danielcirket / OpenEventSourcing

MIT License
3 stars 0 forks source link

Enhancement: Split domain vs published events #33

Closed danielcirket closed 3 years ago

danielcirket commented 4 years ago

I have a strong dislike for the current implementation of the IEvent.UpdateFrom(ICommand command) implementation. Events should be immutable and this doesn't conform to that "requirement".

My current thoughts are to to remove the following properties and methods from the IEvent interface:

Guid? CorrelationId { get; }
Guid? CausationId { get; }
string UserId { get; }

void UpdateFrom(ICommand command);

This will separate the additional metadata from the actual event data, meaning that there will no longer be the requirement to call UpdateFrom on each event from an aggregate. If this is the case then does IEvent need renaming? (Probably not unless we introduce another event type which carries this additional metadata)

Should we decide to split this properties out, we still need a way to deal with attaching this metadata to a stored and published event, so we need to determine what should be responsible for that- I'd prefer at this point if this was as transparent to the consumer as possible, and the consumer shouldn't have to do any manual translation.

Currently the IEventStore.SaveAsync(IEnumerable<IEvent> events) simply maps the properties from the IEvent into columns on the table. This could potentially be changed more toward these or similar:

IEventStore.SaveAsync(IEnumerable<IEvent> events, Guid? correlationId, Guid? causationId, string userId);
IEventStore.SaveAsync(IEnumerable<IEvent> events, ICommand causation);
IEventStore.SaveAsync(IEnumerable<IEvent> events, IEvent causation);

and similarly in the publisher:

Task PublishAsync<TEvent>(TEvent @event, Guid? correlationId, Guid? causationId, string userId) where TEvent : IEvent;
Task PublishAsync<TEvent>(TEvent @event, ICommand causation) where TEvent : IEvent;
Task PublishAsync<TEvent>(TEvent @event, IEvent causation) where TEvent : IEvent;
Task PublishAsync(IEnumerable<IEvent> events, Guid? correlationId, Guid? causationId, string userId);
Task PublishAsync(IEnumerable<IEvent> events, ICommand causation);
Task PublishAsync(IEnumerable<IEvent> events, IEvent causation);

Are there any better solutions here?

I think I currently prefer the above to having two different event types, e.g. DomainEvent and IntegrationEvent where the consumer needs to map between them when saving and publishing events, e.g.:

IEventStore.SaveAsync(IEnumerable<IntegrationEvent> events, Guid? correlationId, Guid? causationId, string userId);
IEventStore.SaveAsync(IEnumerable<IntegrationEvent> events, ICommand causation);
IEventStore.SaveAsync(IEnumerable<IntegrationEvent> events, IEvent causation);

However this does not account for reading the event streams, as you may likely want the meta data when consuming the event so perhaps we do need a separate IntegrationEvent type or similar which are returned from the store/bus changing signatures to be something like:

Task<Page> GetEventsAsync(long offset);
Task<IEnumerable<IntegrationEvent>> GetEventsAsync(Guid aggregateId);
Task<IEnumerable<IntegrationEvent>> GetEventsAsync(Guid aggregateId, long offset);
interface IntegrationEvent
{
    Guid? CorrelationId { get; }
    Guid? CausationId { get; }
    string UserId { get; }
    DateTimeOffset Timestamp { get; }
    IEvent Event { get; }
    // ...
}

Lots to think about.

/cc @AtLeastITry

AtLeastITry commented 4 years ago

@danielcirket IMO i think it makes sense to remove the UpdateFrom(ICommand) stuff and implement new overloads for saving events, having the library handle the mapping of meta data. I dont think we should have two types of event that require the consumer to map but having an IntegrationEvent when retrieveing events makes perfect sense to me. It would be much easier to have the following:

someAggregate.DoSomething(args....);
_aggregateRepository.Save(aggregate, command);

than the current:

someAggregate.DoSomething(args....);

var events = someAggregate..GetUncommitedEvents().ToArray();
foreach(var @event in events)
   @event.UpdateFrom(command);

_aggregateRepository.Save(aggregate);

and then with the IntegrationEvent type it would still be super easy to poll events from projections

it would just become:

var page = await _eventStore.GetEventsAsync(state.Position);
var projection = _scope.ServiceProvider.GetRequiredService<TProjection>();

foreach (var integrationEvent in page.Events)
   await _projection.HandleAsync(integrationEvent.Event);

With these changes it makes consuming the whole flow much nicer,and as a bonus it also allows consumers to publish events without using commands or without having create a new constructor that takes in the extra params on every event. So thats a win for trying to make everything as "Opt-In" as possible

danielcirket commented 4 years ago

Started work on this, but gone in a different direction - see PR #34

Essentially the main changes are the following:

Remove the following from IEvent:

Guid? CorrelationId { get; }
Guid? CausationId { get; }
string UserId { get; }

void UpdateFrom(ICommand command);

Add IEventContext<TEvent>:

public interface IEventContext<out TEvent> where TEvent : IEvent
{
    Guid? CorrelationId { get; }
    Guid? CausationId { get; }
    TEvent Payload { get; }
    DateTimeOffset Timestamp { get; }
    string UserId { get; }
}

Update the IEventPublisher:

- Task PublishAsync<TEvent>(TEvent @event) where TEvent : IEvent;
- Task PublishAsync(IEnumerable<IEvent> events);
+ Task PublishAsync<TEvent>(IEventContext<TEvent> @event) where TEvent : IEvent;
+ Task PublishAsync(IEnumerable<IEventContext<IEvent>> events);

Update the IEventDispatcher:

- Task DispatchAsync<TEvent>(TEvent @event) where TEvent : IEvent;
- Task DispatchAsync(IEvent @event);
+ Task DispatchAsync<TEvent>(IEventContext<TEvent> context) where TEvent : IEvent;
+ Task DispatchAsync(IEventContext<IEvent> context);

Update the IEventHandler<TEvent>:

- Task HandleAsync(TEvent @event);
+ Task HandleAsync(IEventContext<TEvent> context);

Update the IEventStore:

- Task<IEnumerable<IEvent>> GetEventsAsync(Guid aggregateId);
- Task<IEnumerable<IEvent>> GetEventsAsync(Guid aggregateId, long offset);
- Task SaveAsync(IEnumerable<IEvent> events);
+ Task<IEnumerable<IEventContext<IEvent>>> GetEventsAsync(Guid aggregateId);
+ Task<IEnumerable<IEventContext<IEvent>>> GetEventsAsync(Guid aggregateId, long offset);
+ Task SaveAsync(IEnumerable<IEventContext<IEvent>> events);

@AtLeastITry thoughts?

danielcirket commented 4 years ago

Further to the last comment, I'm also considering renaming some properties for events in general and their associated types and adding additional properties to the context:

From:

interface IEvent 
{
    Guid Id { get; }
    Guid AggregateId { get; }
}

interface IEventContext<out TEvent> where TEvent : IEvent
{
    Guid? CorrelationId { get; }
    Guid? CausationId { get; }
}

To:

interface IEvent 
{
    string Id { get; }
    string Subject { get; }
}

interface IEventContext<out TEvent> where TEvent : IEvent
{
    string CorrelationId { get; }
    string CausationId { get; }
    string ContentType { get; } // e.g. application/json
    string Source { get; } // e.g. /system/module
}

Rationale:

AggregateId -> Subject: Aggregates aren't always a concept within a system, and are considered opt-in. Renaming to Subject removes the notion that events are tied to aggregates when in reality you can have a system that generates events outside of aggregates.

Guid -> String type is to support flexibility, event streams and identifiers and their format should be determined by the system usecase and not locked into a GUID format.

e.g. deae5f76-5804-4d96-a545-1536fb067ef7 vs customer-12345

CorrelationId and CausationId:

Guid -> String type is to support flexibility, event streams and identifiers and their format should be determined by the system usecase and not locked into a GUID format.

ContentType:

Although at the time of writing only JSON serialization is used the implementation is swappable, should this ever change it would be useful to understand the content type for deserialization purposes.

Source:

Used to identify the context in which an event happened. This could include various information such as the organisation, the type of event source and the process that produced the event.

e.g. myorg/service-a/alerts

@AtLeastITry any thoughts?