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

Reboot Projection API Model #3052

Open
jeremydmiller opened this issue Mar 17, 2024 · 14 comments
Open

Reboot Projection API Model #3052

jeremydmiller opened this issue Mar 17, 2024 · 14 comments

Comments

@jeremydmiller
Copy link
Member

jeremydmiller commented Mar 17, 2024

Medium to long term things here folks, so nobody get too upset or anxious about anything here yet. I'm just thinking out loud here.

Problems:

  • Code generation approach and the conventional Create() / Apply() is successful for simple workflows, but breaks down badly for any kind of remotely complicated workflow
  • Some people just don't like the conventional approach period, and now that C# has pattern matching (yes F# folks, I realize that was never an issue for y'all), there's maybe less advantage in the conventional approach
  • Explicit code via CustomProjection is a bit clumsy to use today. Doesn't work for live aggregations. Would help if this was an easier escape hatch from the conventional/codegen approach
  • "Slicing" API is less than obvious to use. That might be more of a documentation/example issue
  • Would like to do more to incorporate event/stream archiving soon, especially when the Event Store Partitioning is done
  • There's no easy way in the conventional approach to say "apply this logic on the last encountered event", and that's come up quite often lately

Possible Solutions

  • Consider using source generators for the conventional application of events instead of the JasperFx.CodeGeneration / JasperFx.Compiler model? Not sure if that's enough value to justify the effort, but it's also a way to dip the toes into it
  • Push through issues with CustomProjection so that it can be used for live aggregations
  • New "explicit aggregation projection" model where the user gets the current aggregate (or not if it's new) in a method, and returns a response object that's something like Store/Delete/UnDelete/Archive/Partials telling Marten what to do next. Vague hope here is to do something that's easy to test
  • Reevaluate the whole event slicing API. Ick, ick, ick.

Independently, I'd like to move this one up into a release soon: #2533

@jeremydmiller
Copy link
Member Author

I'm shutting this down for now. The "grouping/slicing" stuff probably gets re-evaluated as part of Marten 8.

@jeremydmiller
Copy link
Member Author

Nope, client work today made me go into the guts of the projection code, and now's a decent time to re-think this for Marten 8

@jeremydmiller jeremydmiller removed the 7.x label Sep 12, 2024
@jeremydmiller
Copy link
Member Author

jeremydmiller commented Sep 21, 2024

Current Thinking (September 21st, 2024)

  • For Marten 8, pull everything about the event store and even the async daemon that isn't specific to PostgreSQL or maybe even just for a database period into a separate library ("JasperFx.EventStore" is the most likely contender right now)
  • Somewhat deprecate the conventional method approach
  • Make it as easy and quick as possible to just write explicit code for the aggregation code
  • Make it as easy as possible for users to just write explicit code for projections in addition to the current conventional method approach. This should also include the “event slicing” model
    Streamline the sheer number of projection aggregation options.
  • I’d like to consider collapsing SingleStreamProjection/MultiStreamProjection/CustomProjection/ExperimentalCrossStreamAggregation down to just a couple types. In all cases you should be able to use either conventional methods or explicit code.
  • Pull the projection model – and inevitably quite a bit of the IEvent / StreamAction code too – into a shared library that could be reused across Marten, Ermine, and a future CosmosDb event store.
  • Another option – that went over like a lead balloon when I asked about it – would be to make Marten actually depend directly on Wolverine
  • Somehow, some way, make the event slicing model easier to use
  • Reevaluate the daemon internals yet again
  • Reevaluate ILiveAggregator interface
  • Reevaluate IProjection as it's a terrible API that's wrapped too much around being Inline for single stream projections
  • Reevaluate all the aggregation code
  • Consider reversing some of the API renames that happened in Marten 6 around the projection type names. This author doesn't believe they added any value whatsoever and make the code less intention revealing

More details to come.

@jeremydmiller jeremydmiller added this to the Marten 8.0 milestone Oct 31, 2024
@jeremydmiller
Copy link
Member Author

jeremydmiller commented Nov 10, 2024

Notes on Nov. 10th

Current state of IProjectionSource:

public interface IProjectionSource: IReadOnlyProjectionData
{
    AsyncOptions Options { get; }

    /// <summary>
    ///     This is *only* a hint to Marten about what projected document types
    ///     are published by this projection to aid the "generate ahead" model
    /// </summary>
    /// <returns></returns>
    IEnumerable<Type> PublishedTypes();

    IReadOnlyList<AsyncProjectionShard> AsyncProjectionShards(DocumentStore store);

    ValueTask<EventRangeGroup> GroupEvents(DocumentStore store, IMartenDatabase daemonDatabase,
        EventRange range,
        CancellationToken cancellationToken);

    IProjection Build(DocumentStore store);

    ISubscriptionExecution BuildExecution(AsyncProjectionShard shard, DocumentStore store, IMartenDatabase database,
        ILogger logger);

    /// <summary>
    /// Specify that this projection is a non 1 version of the original projection definition to opt
    /// into Marten's parallel blue/green deployment of this projection.
    /// </summary>
    public uint ProjectionVersion { get; set; }

    bool TryBuildReplayExecutor(DocumentStore store, IMartenDatabase database, out IReplayExecutor executor);
}

Notes:

  • Rename IProjection to IInlineProjection. New IProjection that's just:
public interface IProjection
{
    ValueTask ApplyAsync(IDocumentOperations, IReadOnlyList<IEvent>, CancellationToken);
}
  • IProjection Build(DocumentStore store); should become IInlineProjection Build(DocumentStore)
  • Keep BuildExecution(), and this is for async projections
  • Merge in what's now ILiveAggregatorSource<T>? Or make that optional, and you have to find it to use it. Kinda the way it is today anyway
  • Kill off GroupEvents(). Should be buried in the details of BuildExecution()
  • Merge ISubscriptionSource in, kind of. Make ISubscriptionSource implement a subset of IProjectionSource and expose the BuildExecution() : ISubscriptionExecution method
  • Make AsyncProjectionShard an interface (IAsyncShard ?) and embed construction of the subscription agent
  • Above bullet implies that IProjectionSource / ISubscriptionSource will have a reference to DocumentStore and the relevant IMartenDatabase

SnapshotLifecycle

I hate the Snapshot nomenclature in the Marten 6+ projection registrations. I'd honestly like to rewind back to Marten 5 nomenclature and nuke SnapshotLifecycle. Any opinions on that?

User Entrypoints

So what users will actually use to create projections to Marten / Ermine / future critters?

  • EventProjection -- I know I floated the idea of killing it, but I think I change my mind. More below.
  • Aggregation<T> and Aggregation<TDoc, TId>, with these being sorta replacements for SingleStreamProjection, MultiStreamProjection, and CustomProjection. Plus ExperimentalMultiStreamProjection that I'm not sure anyone ever used. I think that we'd make SingleStream & MultiStream inherit from the new unified model. More below.
  • FlatTableProjection pretty well as it is today
  • Still support the existing model of putting Apply/ShouldDelete/Create methods directly on projected aggregate types. Little more below

EventProjection

  • Still support the Project<T>() inline lambdas, but this time, I'd like to say that you have to either do all that, or all conventional methods, but not mix
  • Support the conventional method approach, but this time it's all or nothing. Either all explicit, or all Project<T>(). And only support public methods. Can we get away with that?
  • Allow you to override a ValueTask ApplyAsync(IDocumentOperations, IReadOnlyList<IEvent>, CancellationToken); method and/or a void Apply(IDocumentOperations, IReadOnlyList<IEvent>); method. But, if you do that, you cannot use Project() or conventional methods. This is meant to be the trap door to "just do whatever you want with explicit code". Using EventProjection as a base still allows us to keep all the optional filtering stuff and async options stuff

"Self Aggregates"

This is the old "self aggregating" approach where you throw Apply() / Create() methods directly onto the projected aggregate. I think I'm proposing to keep this as is, but also add a new convention to alternatively use a Build(IEnumerable<object>) method or take in IEnumerable<IEvent>. This is somewhat driven by a JasperFx client where they wrote their own event sourcing before switching to Marten:-)

Aggregation

  • Allow for the same conventional methods as before
  • Or, override an ValueTask ApplyChangesAsync(DocumentSessionBase session, EventSlice<TDoc, TId> slice, CancellationToken cancellation, ProjectionLifecycle lifecycle = ProjectionLifecycle.Inline) method (like CustomProjection today)
  • Or more simply, override this: public virtual ValueTask<TDoc> BuildAsync(IQuerySession session, TDoc? snapshot, IReadOnlyList<IEvent> events) if you have simpler needs
  • Still support the Project<T>() methods

Through validation, make every single one of these options be mutually exclusive. You gotta do one or the other, but not mix

Aggregation<TDoc, TId>

Same rules and stuff as Aggregation<T>, but we've got a new issue of needing to figure out grouping (ick)

Options:

  1. Use the declarative Identity, Identities, Fanout methods, but make sure every single one can use IEvent<T> as well as just the event type T
  2. Kill off IAggregateGrouper<TId> and MultiStreamProjection.CustomGrouper<TId>. Just kill it.
  3. Allow for a virtual method called TId : IdentityFor(IEvent) as a simple, explicit recipe
  4. Allow for a virtual method that can be overridden with this signature:
    ValueTask<IReadOnlyList<EventSliceGroup<TDoc, TId>>> SliceAsyncEvents(
        IQuerySession querySession,
        List<IEvent> events);

That last signature gives you the ability to do anything. Event data enrichment, use other data for slicing, you name it. Special logic, whatever. And again, all of the above options are mutually exclusive.

In all cases, I'd like to push users toward explicit code whenever the logic or workflow or data lookup needs are anything other than simplistic

@erdtsieck
Copy link
Contributor

I would suggest that if you want explicit projections to become the default, then the most common cases should be very easy.

So maybe have a plain and simple Projection class that you can inherit from, that has abstract methods create and apply.

public interface IProjection<T> 
{
    ValueTask<T> ApplyAsync(IDocumentOperations ops, IReadOnlyList<IEvent> events, CancellationToken cancellationToken);
}

public class Projection<T> 
{
    public virtual ValueTask<T> ApplyAsync(IDocumentOperations ops, IReadOnlyList<IEvent> events, CancellationToken cancellationToken)
    {
         // default implementation that you could override 
         // first call create on all events until aggregate is not null
         // then call Apply on all events 
    }
    
    public abstract ValueTask<T> Create(IDocumentOperations ops, IEvent @event);
    
    public abstract ValueTask<T> Apply(IDocumentOperations ops, T aggregate, IEvent @event);
}

Why split Create and Apply? Because of nullability. We and the compiler can now assume that in the Apply method the aggregate is never null.

@jeremydmiller
Copy link
Member Author

jeremydmiller commented Dec 2, 2024

Proposed API -- Edited Dec. 3rd

Entry Points

By "entry points", I mean the interfaces or base classes that users would implement or override in order to implement a projection of some sort.

Directly Implement ILiveAggregator

This is already in the guts of Marten, but for simple aggregations where you never delete anything, just implement this:

public interface ILiveAggregator<T>
{
    ValueTask<T> BuildAsync(
        IReadOnlyList<IEvent> events,
        IQuerySession session,
        T? snapshot,
        CancellationToken cancellation);
}

The general idea is that Marten (really JasperFx) would have adapters that let this run as either an inline projection, an async projection, or as is for live aggregations

Directly Implement IProjection

In this generation, IProjection is just another mechanism for simplistic, non-aggregation projections. It gets simplified a bit in Marten 8 / Ermine 1 to:

public interface IProjection
{
    Task ApplyAsync(IDocumentOperations operations, IReadOnlyList<IEvent> events, CancellationToken cancellation);
}

EventProjection ultimately builds an object instance that implements the interface above

Subclassing EventProjection

This has the previous role of "just let me do stuff with matched events", but done so with the understanding that it does
not have the performance optimizations (think parallelization) of the aggregation projections recipes. I want to recommend that the "new" EventProjection get used in one of two mutually exclusive ways. First, you can override this
method:

    public virtual ValueTask ApplyAsync(IDocumentOperations operations, IReadOnlyList<IEvent> events,
        CancellationToken token)
    {
        return new ValueTask();
    }

For optimized usage within async projections, we'd encourage folks to register event filters as well, but that's only meaningful for async projections

Or, you can use the previous named method conventions. In this version of the projections, I'd like to enforce that all methods have to be public to simplify the codegen guts

In this version, I'd like to completely eliminate the mix and matching of these:

    [MartenIgnore]
    public void Project<TEvent>(Action<TEvent, IDocumentOperations> project)
    {
        _projectMethods.AddLambda(project, typeof(TEvent));
    }

    [MartenIgnore]
    public void ProjectAsync<TEvent>(Func<TEvent, IDocumentOperations, Task> project)
    {
        _projectMethods.AddLambda(project, typeof(TEvent));
    }

Those puppies are a remnant of the ancient Marten 2/3 era ViewProjection. With the advent of easy pattern matching in C# (or people using F# in the first place), I'd rather push for explicit code. In a subsequent version of Marten 7.*, I'd like to mark the methods above as [Obsolete] and add the first version as a path for folks to get off of the now obsolete APIs.

Directly implement IInlineProjection

I don't think this will be common

This will be a new interface (it's a cut down version of the existing IProjection that's optimized for Inline execution):

public interface IInlineProjection<TOperations>
{
    /// <summary>
    ///     Apply inline projections during asynchronous operations
    /// </summary>
    /// <param name="operations"></param>
    /// <param name="streams"></param>
    /// <param name="cancellation"></param>
    /// <returns></returns>
    Task ApplyAsync(IDocumentOperations operations, IReadOnlyList<StreamAction> streams,
        CancellationToken cancellation);
}

Marten itself will use this interface at runtime during the IDocumentSession.SaveChangesAsync(). Most projection types
running Inline will end up exposing an adapter implementation of the interface above.

Aggregations

Alright, just to get this out there, I hated the Marten 6 API renames of AggregationProjection to SingleStreamProjection and MultiStreamProjection.

At a minimum, I'd like to collapse the various aggregation options down to:

  • SingleStreamProjection<T> (don't really want to bother changing the name)
  • MultiStreamProjection<TDoc, TId> -- takes over for all the existing CustomProjection, MultiStreamProjection, and the ExperimentalMultiStreamProjection. The runtime behavior will change a bit depending on the overridden methods or the presence of conventional methods. You will not be allowed to do both though, one or the other.

With both of the aggregation recipes, you can either directly override this method for complex workflows where you
might delete, un-delete, or even patch the document:

    public virtual async ValueTask ApplyChangesAsync(DocumentSessionBase session, EventSlice<TDoc, TId> slice,
        CancellationToken cancellation,
        ProjectionLifecycle lifecycle = ProjectionLifecycle.Inline)

or this for simpler workflows where you don't do anything but build or update the aggregate document from new events:

 public virtual ValueTask<TDoc> BuildAsync(IQuerySession session, TDoc? snapshot, IReadOnlyList<IEvent> events)

or this even simpler method:

public virtual TDoc Apply(TDoc? snapshot, IReadOnlyList<IEvent> events)

The projection validation would reject if you implement more than one of those methods.

Or you can use the existing Apply / Create / constructor function conventions and Marten will codegen the actual implementations of ILiveAggregator<T> and other innards

See the next section for "slicing"

Slicing APIs

The analysis on this is heavily in flight, but I think we can agree that:

  • There's room for improvement over the existing model
  • We should probably aim for more explicit code
  • The current class names in the API are dreadful

"Self Aggregates"

The old aggregation formula where users just stick Apply or Create methods directly on the aggregated document. This
will still create a SingleStreamProjection<T> where T is the aggregate type

Subclass FlatTableProjection

I don't see this model changing much for Marten 8 except to get a bug fix or two

@jeremydmiller
Copy link
Member Author

jeremydmiller commented Dec 3, 2024

Slicing APIs -- Dec 3rd

I originally thought that Marten 8 wouldn't have very much change in the public API, but the main goal is to get common stuff pulled out to use in Ermine or any other future event store critters, so maybe there's room to consider actual disruptive changes. And I will freely admit that I think the event slicing API is about the absolute least usable API in the entire Critter Stack and I've always been unhappy with it outside of easy multi-stream aggregations. I'm also aware that widespread API changes will lead to the need for a comprehensive migration guide that might be time consuming -- but also, the documentation on multi-stream projections isn't terribly good today anyway, so ¯_(ツ)_/¯.

Enough, let's get started, here's a static view of the base data holders for slicing in Marten 8 so far

classDiagram

class EventSliceGroup {
    string TenantId
}

EventSliceGroup ..|> IEventGrouping

EventSliceGroup *.. EventSlice
EventSlice o.. IEvent
Loading

Just know that most of the types have generic arguments for <TDoc, TId>, with IEventGrouping<TId> being a little simpler, and IEvent being unchanged from Marten 7.

IEventGrouping would be just a mechanism to sort events into "slices" by aggregate identity that can be processed through aggregation. My proposal for this interface right now is:

public interface IEventGrouping<TId>
{
    void AddEvent(TId id, IEvent @event);

    void AddEvents(TId id, IEnumerable<IEvent> events);

    // This time it would be smart enough to wall paper over a T of either
    // InvoiceAdded or IEvent<InvoiceAdded>
    void AddEvents<TEvent>(Func<TEvent, TId> singleIdSource, IEnumerable<IEvent> events);

    // This time it would be smart enough to wall paper over a T of either
    // InvoiceAdded or IEvent<InvoiceAdded>
    void AddEvents<TEvent>(Func<TEvent, IEnumerable<TId>> multipleIdSource, IEnumerable<IEvent> events);
    
    void FanOutOnEach<TSource, TChild>(Func<TSource, IEnumerable<TChild>> fanOutFunc);
}

By making it smart enough to understand the different mechanics from T vs IEvent<T>, I think we can shrink down this interface over what was EventSliceGroup<TDoc, TId> in Marten 7

IEventSlicer

The JasperFx interface to express slicing will be this:

public interface IEventSlicer<TDoc, TId>
{
    // Making a big assumption here that the multi-tenancy sorting -- if any -- happens
    // independently of this slicing
    ValueTask SliceAsync(IEventGrouping<TId> grouping, IReadOnlyList<IEvent> events);
}

The general idea is that you'll get a group of events, then place them -- or derived, virtual events -- in the grouping by the right id.

The Marten specific version that plugs into that would be:

public interface IMartenEventSlicer<TDoc, TId>
{
    // Making a big assumption here that the multi-tenancy sorting -- if any -- happens
    // independently of this slicing
    ValueTask SliceAsync(IQuerySession session, IEventGrouping<TId> grouping, IReadOnlyList<IEvent> events);
}

Creating slicing rules

I guess that both MultiStreamProjection and maybe FlatTableProjection will have the same behavior here.

The first option for defining slicing behavior would be to keep the existing methods for simplistic slicing:

*. Identity<T>(Func<T, TId>), but this time the T could be either the event type or IEvent<T>

  • Identities<T>(Func<T, IEnumerable<TId>>), same note as above
  • FanOut -- keep from before

The second option would be to override this new method:

public virtual TId IdentityFor(IEvent e)

The third option would be to override this new method for really custom slicing where you may also want to do event enrichment in a batch:

public virtual ValueTask SliceAsync(IQuerySession, IEventGrouping<TId>, IReadOnlyList<IEvent>);

And a fourth, rarer model to just call:

RollupOnTenant()

that works as it does today.

I would vote to delete IAggregateGrouper and probably eliminate any need to roll custom IEventSlicer from scratch.

@jeremydmiller
Copy link
Member Author

jeremydmiller commented Dec 6, 2024

Nevermind, we can keep this so we don't break backward compatibility

Eliminate the support for ShouldDelete() in the conventional method approach. Make it all easier. Keep the Delete events, but not anything conditional. Validate that you can no longer use this. Push users toward explicit logic instead with any workflow at all.

@erdtsieck
Copy link
Contributor

For the IReadOnlyList<IEvent> events parameter, naming or documentation can be improved by making the purpose and content of the parameter more explicit. Here are some suggestions:

Naming Suggestions

  1. currentEventSlice: Indicates this is the current "slice" or batch of events being processed.
  2. eventBatch: A common term for a collection of events processed together.
  3. loadedEvents: Suggests that these are the events loaded into the session for processing.

Documentation Suggestions

  • Brief Description: Add XML documentation explaining what the events parameter represents:
/// <summary>
/// A read-only list of events that are part of the current slice being processed by the projection.
/// </summary>
/// <param name="events">The events included in the current processing context, possibly pre-filtered or grouped.</param>

Of course, I will help you write explicit documentation on this (even if you do not implement any of the above suggestions)

@erdtsieck
Copy link
Contributor

Just to try the ILiveAggregator. I think for this ShoppingCart example it makes for a simplified design:

public class ShoppingCart
{
    public Guid Id { get; set; }
    public List<string> Items { get; set; } = new();
    public bool IsCheckedOut { get; set; } = false;
}

public class ShoppingCartAggregator : ILiveAggregator<ShoppingCart>
{
    public ShoppingCart Build(ShoppingCart? current, IReadOnlyList<IEvent> events)
    {
        // Initialize the aggregate if it doesn't exist
        var cart = current ?? new ShoppingCart();

        foreach (var @event in events)
        {
            switch (@event.Data)
            {
                case ItemAdded itemAdded:
                    cart.Items.Add(itemAdded.ItemName);
                    break;

                case ItemRemoved itemRemoved:
                    cart.Items.Remove(itemRemoved.ItemName);
                    break;

                case CartCheckedOut _:
                    cart.IsCheckedOut = true;
                    break;

                default:
                    throw new InvalidOperationException($"Unknown event type: {@event.GetType()}");
            }
        }

        return cart;
    }
}

public record ItemAdded(string ItemName);
public record ItemRemoved(string ItemName);
public record CartCheckedOut();

You have a lot of power over creation proces and maybe some interaction between events

@erdtsieck
Copy link
Contributor

erdtsieck commented Dec 9, 2024

Proposed ILiveAggregator Design with Immutable Records

In this approach, the aggregate (ShoppingCart) is implemented as a C# record with init-only setters, ensuring immutability. The aggregate can only be created via a Create method, and state transitions are handled through Apply methods on the record. The ILiveAggregator.Build method orchestrates the application of events to the aggregate, ensuring a clean and functional approach to state management.

Example Implementation

public record ShoppingCart
{
    public Guid Id { get; init; }
    public List<string> Items { get; init; } = new();
    public bool IsCheckedOut { get; init; }

    public static ShoppingCart Create(Guid id) =>
        new ShoppingCart { Id = id };

    public ShoppingCart Apply(ItemAdded itemAdded) =>
        this with { Items = Items.Append(itemAdded.ItemName).ToList() };

    public ShoppingCart Apply(ItemRemoved itemRemoved) =>
        this with { Items = Items.Where(item => item != itemRemoved.ItemName).ToList() };

    public ShoppingCart Apply(CartCheckedOut _) =>
        this with { IsCheckedOut = true };
}

public class ShoppingCartAggregator : ILiveAggregator<ShoppingCart>
{
    public ShoppingCart Build(ShoppingCart? current, IReadOnlyList<IEvent> events)
    {
        var cart = current;

        foreach (var @event in events)
        {
            cart = @event.Data switch
            {
                CartCreated created => ShoppingCart.Create(created.Id),
                ItemAdded itemAdded => cart?.Apply(itemAdded) 
                    ?? throw new InvalidOperationException("Cannot add items to a non-existent cart."),
                ItemRemoved itemRemoved => cart?.Apply(itemRemoved) 
                    ?? throw new InvalidOperationException("Cannot remove items from a non-existent cart."),
                CartCheckedOut checkedOut => cart?.Apply(checkedOut) 
                    ?? throw new InvalidOperationException("Cannot check out a non-existent cart."),
                _ => throw new InvalidOperationException($"Unknown event type: {@event.GetType()}")
            };
        }

        return cart ?? throw new InvalidOperationException("Cart was not created during the event stream.");
    }
}

public record CartCreated(Guid Id) : IEvent;
public record ItemAdded(string ItemName) : IEvent;
public record ItemRemoved(string ItemName) : IEvent;
public record CartCheckedOut() : IEvent;

Pros of this Design

  1. Immutable State: The use of records with init-only setters ensures that the state of the aggregate cannot be modified unintentionally after creation.
  2. Clear Aggregate Lifecycle: The Create method makes it explicit when and how the aggregate is instantiated. This avoids implicit or accidental state creation.
  3. Functional Design: The Apply methods combined with the with keyword offer a clean and functional way to handle state transitions.
  4. Validation at Runtime: The Build method includes guards against applying events to a null aggregate, reducing the risk of invalid operations.
  5. Separation of Concerns: The aggregate's state management is encapsulated within its Apply methods, simplifying the logic in the ILiveAggregator.

Cons of this Design

  1. Null-Checking in Event Application: The cart?.Apply(event) pattern introduces uncertainty for the compiler about whether the aggregate is null or not. This can lead to repeated null-checks or the need for additional guard clauses, potentially cluttering the code.
  2. Performance Considerations: The creation of new instances using the with keyword for every state transition may have a performance impact, particularly with large aggregates or high-frequency events.

@erdtsieck
Copy link
Contributor

One way to deal with the null checks is of course:

public class ShoppingCartAggregator : ILiveAggregator<ShoppingCart>
{
    public ShoppingCart Build(ShoppingCart? current, IReadOnlyList<IEvent> events)
    {
        ShoppingCart cart = current ?? events.OfType<IEvent<CartCreated>>().FirstOrDefault() switch
        {
            IEvent<CartCreated> created => ShoppingCart.Create(created.Data.Id),
            _ => throw new InvalidOperationException("Cart must be created before other events can be applied.")
        };

        foreach (var @event in events)
        {
            cart = @event.Data switch
            {
                ItemAdded itemAdded => cart.Apply(itemAdded),
                ItemRemoved itemRemoved => cart.Apply(itemRemoved),
                CartCheckedOut checkedOut => cart.Apply(checkedOut),
                _ => throw new InvalidOperationException($"Unknown event type: {@event.GetType()}")
            };
        }

        return cart;
    }
}

But there is the performance penalty for the initial event filtering on OfType.

Might not be a biggy, since it should be event zero. So with some documentation on that, this might work.

@erdtsieck
Copy link
Contributor

erdtsieck commented Dec 9, 2024

Suggested Improvement: Helper Methods for Null Handling and Initialization

To reduce boilerplate and improve readability in ILiveAggregator implementations, I propose introducing Helper Methods for common patterns such as:

  1. Null Checking: Ensuring the aggregate is initialized before applying events.
  2. Aggregate Initialization from Events: Creating an aggregate from a specific event in the event stream.

These helper methods would simplify event processing logic, making implementations more concise and less error-prone.


Proposed Helper Methods

public static class AggregatorHelpers
{
    /// <summary>
    /// Ensures that the aggregate is not null. Throws an exception if it is null.
    /// </summary>
    public static T EnsureExists<T>(T? aggregate, string errorMessage) where T : class
    {
        return aggregate ?? throw new InvalidOperationException(errorMessage);
    }

    /// <summary>
    /// Initializes the aggregate from an event, or throws if the event is not found.
    /// </summary>
    public static T InitializeFromEvent<T, TEvent>(T? current, IReadOnlyList<IEvent> events, Func<IEvent<TEvent>, T> create)
        where TEvent : class
    {
        if (current is not null)
            return current;

        var createEvent = events.OfType<IEvent<TEvent>>().FirstOrDefault();
        return createEvent != null
            ? create(createEvent)
            : throw new InvalidOperationException($"Missing initialization event of type {typeof(TEvent).Name}");
    }
}

Example: Refactored ShoppingCartAggregator Using Helpers

public class ShoppingCartAggregator : ILiveAggregator<ShoppingCart>
{
    public ShoppingCart Build(ShoppingCart? current, IReadOnlyList<IEvent> events)
    {
        var cart = AggregatorHelpers.InitializeFromEvent(
            current,
            events,
            (IEvent<CartCreated> created) => ShoppingCart.Create(created.Data.Id)
        );

        foreach (var @event in events)
        {
            cart = @event.Data switch
            {
                ItemAdded itemAdded => cart.Apply(itemAdded),
                ItemRemoved itemRemoved => cart.Apply(itemRemoved),
                CartCheckedOut checkedOut => cart.Apply(checkedOut),
                _ => throw new InvalidOperationException($"Unknown event type: {@event.GetType()}")
            };
        }

        return cart;
    }
}

Why Use Helper Methods?

Pros:

  1. Reduced Boilerplate: Reusable logic eliminates repetitive patterns across multiple aggregators.
  2. Cleaner Code: Keeps Build methods focused on domain-specific logic.
  3. Clearer Error Messages: Explicit exceptions improve runtime error diagnostics.
  4. Improved Maintainability: Centralized methods simplify refactoring if validation rules change.

Cons:

  1. Extra Indirection: May introduce slight complexity if the helpers are used inconsistently.
  2. Customization Overhead: Custom exceptions would require method overrides or additional parameters.
  3. Learning Curve: New developers might need time to grasp the added level of abstraction.

Additional Use Case: Null-Safe Apply

We can also use EnsureExists within the event application loop, further reducing the chance of null-related bugs:

foreach (var @event in events)
{
    cart = AggregatorHelpers.EnsureExists(cart, "Cart must be created before events can be applied.") switch
    {
        ItemAdded itemAdded => cart.Apply(itemAdded),
        ItemRemoved itemRemoved => cart.Apply(itemRemoved),
        CartCheckedOut checkedOut => cart.Apply(checkedOut),
        _ => throw new InvalidOperationException($"Unknown event type: {@event.GetType()}")
    };
}

By using these helper methods, ILiveAggregator implementations can become clearer, more maintainable, and less error-prone. Let me know if you’d like additional examples or expansions on this approach!

@jeremydmiller
Copy link
Member Author

@erdtsieck The conventional approaches to the aggregation already make sure that the snapshot exists before calling into the Apply methods. Helpers for writing purely explicit projections sounds good to me though.

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

No branches or pull requests

2 participants