Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-evaluate the multi-stream event slicing/grouping APIs #3443

Open
jeremydmiller opened this issue Sep 24, 2024 · 2 comments
Open

Re-evaluate the multi-stream event slicing/grouping APIs #3443

jeremydmiller opened this issue Sep 24, 2024 · 2 comments
Milestone

Comments

@jeremydmiller
Copy link
Member

It's horrible. Worst API in Marten imho. My thinking right now is to push it toward making the users write more explicit code. This is more of a placeholder for conversation than anything immediately actionable.

@jeremydmiller jeremydmiller added this to the Marten 8.0 milestone Sep 24, 2024
@erdtsieck
Copy link
Contributor

Hey Jeremy,

I completely agree with your assessment of the multi-stream event slicing/grouping API. As it stands, there’s a lot of implicit behavior that makes it difficult to reason about what’s actually happening. This often leads to complex and redundant code, especially when implementing custom slicers or groupers.

One specific pain point is the need to implement both SliceInlineActions and SliceAsyncEvents. The two methods end up duplicating a lot of logic, and while I understand the separation between synchronous and asynchronous operations, in practice it creates unnecessary overhead.

For example, here's a part of my own code where I had to implement a custom slicer, and you can see how much duplication occurs between the two methods:

public class CustomSlicer: IEventSlicer<AantalConsultatiesPerStatus, string>
{
    public async ValueTask<IReadOnlyList<EventSlice<AantalConsultatiesPerStatus, string>>> SliceInlineActions(
        IQuerySession querySession, IEnumerable<StreamAction> streams)
    {
        var allEvents = streams.SelectMany(x => x.Events).ToList();
        var group = await TenantSliceGroup(querySession, allEvents);
        return group.Slices.ToList();
    }

    public async ValueTask<IReadOnlyList<TenantSliceGroup<AantalConsultatiesPerStatus, string>>> SliceAsyncEvents(
        IQuerySession querySession, List<IEvent> events)
    {
        var group = await TenantSliceGroup(querySession, events);
        return new List<TenantSliceGroup<AantalConsultatiesPerStatus, string>> { group };
    }

    private static async Task<TenantSliceGroup<AantalConsultatiesPerStatus, string>> TenantSliceGroup(IQuerySession querySession, List<IEvent> events)
    {
        // Event processing logic here...
    }
}

Both methods end up doing almost the exact same thing—managing a group of events, yet I'm forced to handle them separately for synchronous and asynchronous operations. In this case, it feels like I’m writing the same logic twice, just in different contexts. This goes against the idea of reducing boilerplate and improving maintainability. Having a unified API for event slicing that abstracts away the difference between sync and async operations would significantly reduce this kind of duplication.

Another issue is the ambiguity in List<IEvent> events. It’s not always clear whether this list contains all events for a given aggregate or just a subset within the current session. More clarity or context around what this list represents would make the API easier to work with and reduce bugs caused by assumptions about the event data.

Furthermore, if it stays a complex thing, then we just need to add a lot more examples, maybe even an example page.

Lastly, I think the term "slicing" is a bit too vague. It doesn’t immediately convey its purpose, and something more descriptive like PartitionEvents or DistributeEvents would make it easier to understand. Clearer names would definitely help developers better understand how the events are being grouped or distributed, especially for those who are new to the framework.

In summary, streamlining the API to make it more explicit and reducing the redundant logic between sync and async handling would go a long way toward improving the developer experience. I’d be happy to help with further discussions or contribute to these changes.

@erdtsieck
Copy link
Contributor

erdtsieck commented Sep 25, 2024

I’ve been thinking about the current slicing/grouping API as well, and I wanted to offer some concrete suggestions that might help reduce the complexity and make things more explicit.

1. Unify SliceInlineActions and SliceAsyncEvents

Right now, having to implement both methods often leads to duplicated logic. For example, in my own use case, I end up writing the same event partitioning code twice—once for sync and once for async. Here’s a snippet from my project where the duplication happens:

public class CustomSlicer: IEventSlicer<AantalConsultatiesPerStatus, string>
{
    public async ValueTask<IReadOnlyList<EventSlice<AantalConsultatiesPerStatus, string>>> SliceInlineActions(
        IQuerySession querySession, IEnumerable<StreamAction> streams)
    {
        var allEvents = streams.SelectMany(x => x.Events).ToList();
        var group = await TenantSliceGroup(querySession, allEvents);
        return group.Slices.ToList();
    }

    public async ValueTask<IReadOnlyList<TenantSliceGroup<AantalConsultatiesPerStatus, string>>> SliceAsyncEvents(
        IQuerySession querySession, List<IEvent> events)
    {
        var group = await TenantSliceGroup(querySession, events);
        return new List<TenantSliceGroup<AantalConsultatiesPerStatus, string>> { group };
    }
}

In both methods, I’m essentially doing the same thing, just adapting for sync vs. async behavior. I think we could simplify this by unifying the two methods into one, like SliceEvents, and letting the framework handle whether the operation is sync or async based on the context.

Proposed Approach:

public class CustomSlicer: IEventSlicer<MyProjection, string>
{
    public async ValueTask<IReadOnlyList<EventSlice<MyProjection, string>>> SliceEvents(
        IQuerySession querySession, IEnumerable<IEvent> events, SliceContext context)
    {
        // Unified slicing logic for both sync and async
        var eventList = events.ToList();
        var group = await CreateEventGroup(querySession, eventList);

        return context.IsAsync
            ? new List<EventSlice<MyProjection, string>> { group }
            : group.Slices.ToList();
    }

    private async Task<TenantSliceGroup<MyProjection, string>> CreateEventGroup(
        IQuerySession querySession, List<IEvent> events)
    {
        // Shared event processing logic
    }
}

In this design, a new SliceContext object could be introduced to provide metadata like IsAsync to the method, allowing the framework to determine whether it should run the operation synchronously or asynchronously. This reduces the need for separate methods, avoids duplicated code, and still maintains flexibility.

2. Built-In Event Transformation

Lastly, transforming events (like fanning out child events or reshaping data) is another area where we could reduce boilerplate. I think having a WithTransformation() method, or something similar, could help developers apply transformations declaratively without having to implement the same logic over and over.

Hope this helps, or starts a discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants