Enumeration usage

[dgaspar]

An Order HasOne OrderStatus (Enumeration class). https://github.com/dotnet-architecture/eShopOnContainers/blob/c3f7178631d3ab1e9e39810e782272e888317a87/src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/Order.cs#L24-L25

When updating that status, I noticed that only the underlying _orderStatusId was modified by the methods but OrderStatus remained untouched (and out of sync with the underlying value) https://github.com/dotnet-architecture/eShopOnContainers/blob/c3f7178631d3ab1e9e39810e782272e888317a87/src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/Order.cs#L148

And if I try to also set order.OrderStatus = OrderStatus.Shipped;... well, those Enumeration instances are static properties and are untracked, therefore, EF Core tries to insert them during SaveChanges() and the operation fails due to key dup errors.

Question: wouldn't it make sense to establish that all Enumeration are EntityState.Unchanged, and then we can safely assign them around to entities?

public override async Task<int> SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken ct = new CancellationToken())
{
   foreach (var entity in ChangeTracker.Entries<IEnumeration>()) entity.State = EntityState.Unchanged;

   return await base.SaveChangesAsync(acceptAllChangesOnSuccess, ct);
}

[sughosneo]

Hi @dgaspar, thank you for reaching out.

Like other methods in \Services\Ordering\Ordering.Domain\AggregatesModel\OrderAggregate\Order.cs class, SetShippedStatus() method also track the status using Id for e.g OrderStatus.Shipped.Id That's because it has already been declared as a static object as public static OrderStatus Shipped = new OrderStatus(5, nameof(Shipped).ToLowerInvariant()); in OrderStatus class. Yes, you could alternatively use the status as string as well but that wouldn't make much of a difference as in DomainEvent we are dealing with an object. If you notice the class OrderShippedDomainEventHandler you would notice primarily it gets wrapped as OrderStatusChangedToShippedIntegrationEvent object where instead of Id, OrderStatus.Name has been used.

To my opinion in a distributed system, Entity state change actually gets tracked through by publishing an event. From the Domain Drive Design point of view, it's possibly the best way to keep track of a state of an entity . For e.g : Instead of keeping track of OrderStatus.Modified it would be easy to keep a watch for an event of OrderStatus.Confirmed or OrderStatus.Cancelled. Yes, in terms of saving the event state through EF Core module (For e.g : SaveChangesAsync()) you could either use local database transaction within a service or you could implement a retry mechanism through a queue.

You could futher refer the implementation of OrderingIntegrationEventService and IntegrationEventLogService class more details.

Hope this helps !

[dgaspar]

Hi, I was able to follow the events sequence, thanks for pointing that out.

The event is fired with this as argument, which is now an "inconsistent" Order entity (because this.OrderStatus didn't get updated, only this._orderStatusId did). https://github.com/dotnet-architecture/eShopOnContainers/blob/43fe719e98bb7e004c697d5724a975f5ecb2191b/src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/Order.cs#L150

All this is working because the domain event Notification Handler decides to disregard the incoming Order object. It only takes into account the id orderShippedDomainEvent.Order.Id, to then reload the Order: https://github.com/dotnet-architecture/eShopOnContainers/blob/23992ed3249cd257ac5fc42b23f2864b2ddf347b/src/Services/Ordering/Ordering.API/Application/DomainEventHandlers/OrderShipped/OrderShippedDomainEventHandler.cs#L34-L43

But if the developer had naïvely relied on the event argument orderShippedDomainEvent.Order.OrderStatus (for instance, when dispatching the Integration event, or when doing logging), then those would have been bugs.

If I got this right, it's best practice to distrust the incoming entity, and instead reload it from the database. (In this case, isn't it more honest to only inform the order Id as the event argument?)