mrmorrandir / EventSourcing

An event sourcing library with an EF Core based persistence (in order to use MSSQL and other relational databases that are available in corporate scopes).
MIT License
1 stars 0 forks source link

EventRepository.GetAsync<T> returns Success, when using StreamId of wrong AggregateType #1

Closed mrmorrandir closed 5 months ago

mrmorrandir commented 9 months ago

When using a streamId (here request.Id) of an Aggregate type that differs from the type parameter, the EventRespository should return a IsFailed result, but returns IsSuccess.

Example: We want to request a ProcessAggregate from the EventRepository but the reqeuest.Id is that of a ProcessStepAggregate (which does exist in the event store)

var processResult = await _eventRepository.GetAsync<ProcessAggregate>(request.Id, cancellationToken);
if (processResult.IsFailed)
    return Result.Fail(_messages.ErrorProcessCouldNotBeStarted(request.Id).CausedBy(processResult.Errors));

// ... we should not get here ...
mrmorrandir commented 5 months ago

We have different scanarios this "bug" would be possible:

  1. Some thinking and testing brought me to the solution, that this is a constructed case, since the main reason this error would be possible is a collision of Guids. The "bug" was part of a test-scenario, where more than one aggregate was created (by hand) with the same id. This should/would never happen in the field under normal circumstances.
  2. Some developer using the library would request an aggregate from the EventRespository with the id of another Aggregate type that he might have used before.

In both scenarios it would only be possible to return Result.IsFailed when the type of the aggregate would be stored in the EventStore. Meaning that the EventData entity would need an StreamType together with the StreamId to identify the stream. (The current Type property only identifies the event type)

In most cases the protected abstract void AggregateRoot.Apply method would/should throw an exception when an event with unknown event-type should be applied.

public sealed class ProcessAggregate : AggregateRoot
{

   // ... properites, constructors, methods ...

    protected override void Apply(IEvent @event)
    {
        switch (@event)
        {
            case ProcessCreated e:
                Id = e.Id;
                Created = e.Created;
                Country = e.Country;
                Plant = e.Plant;
                Line = e.Line;
                Name = e.Name;
                Equipment = e.Equipment;
                Material = e.Material;
                MaterialText = e.MaterialText;
                Order = e.Order;
                State = ProcessState.Pending;
                Result = ProcessResult.Unknown;
                Data = e.Data;
                break;
            case ProcessStarted e:
                State = ProcessState.Active;
                break;
            case ProcessFinished e:
                State = ProcessState.Finished;
                Finished = e.Finished;
                Result = e.Result;
                break;
            case ProcessCancelled e:
                State = ProcessState.Cancelled;
                break;

            // This default case would be needed to get an error
            default:
                throw new UnkownEventException($"The event type {e.GetType()) could not be processed by {GetType) (Aggregate)!");
        }
    }
}

Implementing the default: case would be in the responsibility of the developer which seems legitimate to me.