mfelicio / NDomain

.NET framework that simplifies software development using DDD, Event Sourcing and CQRS based architectures.
MIT License
54 stars 19 forks source link

First try #1

Closed jamesholcomb closed 7 years ago

jamesholcomb commented 9 years ago

Hello,

Great start on the framework. I like the fact that it is lightweight, testable, pluggable and not intrusive.

When giving the OpenStore sample app a spin, I ran into an issue sending commands.

Sending CreateAuction to background-worker
14:27:29 Failed to process message "CreateAuction" with id "create-auction" Syst
em.Collections.Generic.KeyNotFoundException: The given key was not present in th
e dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at NDomain.EventSourcing.EventStoreSerializer.Serialize(IAggregateEvent event
) in C:\Users\jamesholcomb\code\NDomain\source\NDomain\EventSourcing\EventStoreS
erializer.cs:line 49
   at NDomain.EventSourcing.EventStore.<Append>b__6_0(IAggregateEvent e) in C:\U
sers\jamesholcomb\code\NDomain\source\NDomain\EventSourcing\EventStore.cs:line 5
5
   at System.Linq.Enumerable.WhereSelectListIterator`2.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at NDomain.EventSourcing.EventStore.<Append>d__6.MoveNext() in C:\Users\james
holcomb\code\NDomain\source\NDomain\EventSourcing\EventStore.cs:line 55
context.CommandBus
    .Send(new Command<CreateAuction>("create-auction",
        new CreateAuction()
        {
            SellerId = "seller1",
            MinPrice = 1.0m,
            AuctionId = "auction1",
            Item = new Item() { Name = "book" },
            WindowUtc = new Window() { Start = DateTime.Now, End = DateTime.Now.AddDays(1) }
        }));

Not sure I understand how the command id is used in this context.

mfelicio commented 9 years ago

Hi,

Thanks for giving it a try. I'm still creating the OpenStore samples and so far haven't added tests for the command handlers.

There's nothing wrong with the command id there. The problem is that in order to persist aggregate events, the aggregates must be registered first. So when you're configuring the DomainContext's EventSourcing you can use:

.EventSourcing(e => e.BindAggregate<Auction>())

I should update this on the README main page to be more clear. I plan to add a sample project that uses the OpenStore domain, so that it's easier to get started.

Hope this helps!

jamesholcomb commented 9 years ago

Ah that worked. I see now in your EventBusTests that you configure EventSourcing in this way.

Thanks!

mfelicio commented 9 years ago

Great! If you find any issue please report :) I'll be happy to help.

mfelicio commented 9 years ago

I'm planning to add some extension methods to automatically register all aggregates using reflection on assemblies, but that could potentially cause slower startup times. Maybe this will be the default strategy if there's no explicit BingAggregate call.

jamesholcomb commented 9 years ago

Do you foresee an alternative way to configure EventSourcing such that there is no hard dependency on binding to an IAggregate? For example, with NEventStore, they use an ICommit, which is just a collection of events resulting from a UOW. I could see a scenario where only selective components of NDomain would be used for commanding/querying + event/message pub/sub in non-DDD apps. For instance, refactoring the messaging bits of a legacy SOA event driven app that used NEventStore + NServiceBus, ServiceStack MQServer, etc.

mfelicio commented 9 years ago

You can use the messaging bits including the message bus, command bus and event bus (these are built on top of the message bus) without configuring event sourcing or aggregates. Take a look at the CommandBusTests.

Currently EventStore returns events whose Payload property is fully typed, so it needs to be able to deserialize a JObject into the original event type. It uses an IEventStoreDeserializer for that. Currently it is depending on aggregates being registered because I'm building the event serializer/deserializer functions on startup by retrieving all known events types used to rebuild aggregate states. However, to be honest EventStore only needs to know about event types, not really about aggregates. See below how events are deserialized:

https://github.com/mfelicio/NDomain/blob/master/source/NDomain/EventSourcing/EventStoreSerializer.cs

However, I'm not sure that this is the best design, as to be honest I only need events with a Payload fully typed on the AggregateFactory level when rebuilding aggregates from the events. What exactly is your use case? Simply handling stored events without rebuilding aggregates?

So I see only two options:

Option 1 is not pretty, but safe. Option 2 is prettier but not so safe, because there could be legacy events stored that are not used in the application anymore and thus the deserializer doesn't know about them. Perhaps for these cases it would leave the Payload as JObject and only deserialize the known event types.. but wouldn't be very a consistent interface.

Do you have other ideas?

jamesholcomb commented 9 years ago

That's right....serializing and de-serializing a collection of events related to an arbitrary UOW rather than specific aggregates (there is no aggregate in this scenario).

The NEventStore project recently implemented a recipe for handling domains on top of the base EventStore. I think that would be a good approach to explore.

https://github.com/NEventStore/NEventStore.Domain

mfelicio commented 9 years ago

I've seen NEventStore.Domain and there's a few things I don't like. There's a lot of interfaces without default implementation that's left for the developer to implement. But yes, those are recipes..meaning you'll have to cook a lot until you see something..and you may not like what you see.

NEventStore is a great and mature project but I feel is a bit over engineered and misses the simplicity I'm trying to achieve with NDomain. But both have very different goals.. it would be possible to use NEventStore as the IEventStore implementation used in NDomain.

jamesholcomb commented 8 years ago

Do you have a helper similar to CQRSExtensions.RegisterHandler() to register all event/command handlers when configuring the processor?

mfelicio commented 8 years ago

I have thought about that, but I still haven't figured out what would be the cleanest way to do that.

To register all event/command handlers I would have to inspect all referenced assemblies and look for method signatures and eventually use some naming convention like classes whose name end in 'Handler'. That should be easy to do, the problem is that in some cases you may not want all handlers to be registered in the same processor and I need to find a clean way to support that.

You can have more than one processor in the same process, each with its own input queue, and have command handlers in one processor and event handlers (rebuilding read models) in a different processor. For deployment purposes, they can all be hosted in the same process although they're separated logically.

My initial thoughts on this was to have an extension method for the processor config and pass the Assembly instance where the handlers are. That wouldn't break anything. But you would still have to call it for every assembly where you have handlers. What are your thoughts on this?

jamesholcomb commented 8 years ago

An extension that registers all handlers in an assembly works for me. Logical separation can still be achieved with your current APIs.

And a follow up...

In your sample auction controller to create auctions,

await this.commandBus.Send(command);

If an error occurs in Handle(ICommand<CreateAuction> command) (outside of the http request) how do you correlate a response back to the client?

mfelicio commented 8 years ago

That hasn't been addressed yet, but the idea is that there will be a CommandStatusStore, where you can query the status for a given command Id.

Any HTTP api that sends a command to be processed asynchronously, should return a 202 Accepted with a Location header containing the url of an endpoint that can be used to query the status of the command. That's what the client can use to know whether the command was processed successfully or not. Similar to this: http://farazdagi.com/blog/2014/rest-long-running-jobs/