fraktalio / fmodel-ts

Functional Domain Modeling with Typescript
https://fraktalio.com/fmodel-ts
Other
105 stars 9 forks source link

Orchestrating Aggregate saga provides wrong events to decider/incorrect state hydration. #798

Closed barnesoir closed 2 weeks ago

barnesoir commented 3 weeks ago

When using then event orchestrating aggregate & a saga triggeres an cmd on another aggregate it provides all current events to re-hydrate state. Meaning any state check in the decider for any saga raised cmd's won't be correct.

Code that I'm using now instead:

    protected async computeNewEvents(
        events: readonly E[],
        command: C,
        fetch: (c: C) => Promise<readonly E[]>
    ): Promise<readonly E[]> {
        // Compute the initial events from the command
        let resultingEvents = this.computeNewEventsInternally(events, command);

        // Collect saga-generated commands
        const sagaCommands: C[] = resultingEvents.flatMap((evt) => this.saga.react(evt));

        // Process saga-generated commands separately
        for (const sagaCommand of sagaCommands) {
            const cmdEvts = await fetch(sagaCommand);
            const newEvents = await this.computeNewEvents(
                cmdEvts.map((evt) => evt as E),
                sagaCommand,
                fetch
            );
            resultingEvents = resultingEvents.concat(newEvents);
        }

        return resultingEvents;
    }
idugalic commented 3 weeks ago

Hi @barnesoir. Thanks for opening the issue. Before we recognize this as an issue, we can discuss whether this is a feature.

When using then event orchestrating aggregate & a saga triggeres an cmd on another aggregate it provides all current events to re-hydrate state.

YES, it will provide all previously created events to the next Decider/Aggregate to take them into account. That Decider should be robust to recognize IF these Events should be used to build its state (check in your Decider eveolve/decide functions if the identity/ID of State matches matching identity/ID of Event). Identity is an important part of the core domain IMHO and you should check it there, not letting/trusting the outside layer (fetch/store) make that decision for you. The good thing about this is that it will work in more complex situations where you have Saga communicating two Deciders/Aggregates A and B forward and back: A-B-A. In this case, you would need to have newly created events of A from the first step included in the state of the Third command that is targeting the same A decider.

I hope it makes sense. Let me know, please. My idea was to provide these newly created events as input for the next command (not relying on just fetching events at this point) by default. I believe that you can not know upfront if these are going to be needed for that next command/decider. Decider can/should check this.

Best, Ivan

idugalic commented 3 weeks ago

In case you prefer your option better and let infra layer provide you with the right events in any possible case I would suggest considering specifying the shape of your types for commands and events:

export type CommandShape = {
  decider: string; // The name of the decider/aggregate/entity that the command is targeting
  id: string; // The ID of the decider/aggregate/entity that the command is targeting
  kind: string; // The kind/type/name of the command
};

export type EventShape = {
  decider: string; // The name of the decider/aggregate/entity that produced the event
  id: string; // The ID of the decider/aggregate/entity that produced the event
  kind: string; // The kind/type/name of the event
  version: number; // The version of the event (used for evolving the event schema)
};

You can extend the Fmodel aggregate to use this information:

MyEventSourcingOrchestratingAggregate<
  C extends CommandShape,
  S,
  E extends EventShape,
  StreamVersion,
  CommandMetadata,
  EventMetadata
>;

Now, your application layer is more robust and can override any method (eg. computeNewEvents) in a type-safe way. You just created a small framework based on the Fmodel ;) My point is, FModel is too general to specify the shape of your API, but nobody is stopping you to specialize it for your case.

barnesoir commented 3 weeks ago

@idugalic

Hi Ivan, thank you for taking the time to provide such a detailed response. I really appreciate your answer, a little hard for me to see at first but your 100% right on not letting your store/infra dictate your events & enforcing identify checks in decide & evolve, though I little odd looking at first glance completely makes sense.

So with identity being at the core of the domain, does raising events for another aggregate seem appropriate in the decider, eg: gist. Each user gets 8 points when being created, and halves points up the chain to 1 (3 parents). UserA => Userb => UserC => UserD = 1,2,4,8 points respectively IE: Using RewardUserParent cmd to check for invitedBy, then raising an event for the invitedBy aggregate

Thanks again for your time and effort.

idugalic commented 2 weeks ago

Hi,

It is not a good idea for one decider/aggregate to publish event(s) that another decider/aggregate will use to source its state (it will couple your components). Saga/React should be used to react to event(s) from one decider/aggregate and publish commands to another. Effectively, Saga maps Events into Commands, and it is useful as an integration pattern.

idugalic commented 2 weeks ago

I will close this issue. Feel free to open a new Discussion topic @barnesoir . Thanks for your questions!