AggregateRoot & Repository responsibilities refactor

[stevehodgkiss]

We discussed this briefly in the last ES brown bag.

Current state:

• the Repository is responsible for getting the events for an aggregate and loading it
• the AggregateRoot is responsible for persisting it's new events in the event sink which is a constructor dependency

This feels wrong to me. Either AggregateRoot should do both saving and loading, or repository should be both saving and loading.

Proposed changes:

• the Repository is responsible for saving aggregates in addition to loading them 😲
• the AggregateRoot exposes it's changes / new events to facilitate this and testing of aggregates

Usage in a command handler would look something like this:

class MyCommandHandler
  include CommandHandler
  # in CommandHandler module
  # def initialize(repository:) # a Repository instance

  def handle(command)
    with_aggregate(Person, command.person_uuid) do |person|
      person.provide_w9_form(...)
    end
  end

  private

  # in CommandHandler module
  def with_aggregate(aggregate_class, id)
    aggregate = repository.load(aggregate_class, command.aggregate_id)
    yield aggregate
    repository.save(aggregate)
  end
end

I dislike referencing the Repository constant from within handlers so the example above has the repository instance constructor injected.

[grassdog]

I like moving the save responsibilities to the Repository. Not sure I'm 100% sold on the interactions between the Repository and the Aggregate though. It does feel like the repository knows a little too much.

I'd like to investigate whether the aggregate could return the new events to persist rather than save them up in an internal array. That way the command handler would coordinate between the repo and the aggregate.

Something like (warning: pseudo code incoming):

class CommandHandler
  def initialize(repository:, aggregate_class:)
    @repo = repository
    # Might makes sense to make this a lazy enumerable if we want to keep the repo out of the aggregate
    events = @repo.load()
    @aggregate = aggregate_class.new.load_state(events)
  end

  def handle(command)
    # This is hand wavy. Would need to nut out the routing to the correct handle method on the aggregate here.
    new_events = @aggregate.handle(command)
    @repo.save(new_events, @aggregate.current_version)
  end
end

Testing the aggregate could be a matter of feeding in a list of events to load_state and checking that it returns expected events when calling a handler.

The repo would only need to test the load and save methods.

Thoughts?

[stevehodgkiss]

@grassdog interesting approach. That would be breaking CQS (handle mutating state and returning data), but I don't think CQS should be blindly followed anyways. The repo.save is pretty much the API on the event store itself, so there wouldn't be much happening in repo.save except delegating to the event store.

The "get changes" "clear changes" approach is probably the most common interface I've seen in .NET/java ES projects (see NEventStore.Domain & Greg's SimpleCQRS.

Merging these changes in now, but let's continue the conversation of alternatives via PR's.