New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support batch-processing of messages #116

Closed
benjamin-hodgson opened this Issue Jul 15, 2015 · 8 comments

Comments

Projects
None yet
5 participants
@benjamin-hodgson
Contributor

benjamin-hodgson commented Jul 15, 2015

I've been thinking about the signatures of the Send<T> and Publish<T> methods. To correctly use these generic methods, you have to know statically the concrete (runtime) type of message at which you wish to invoke it. This makes batch processing of messages hard:

List<Event> events = new List<Event>();
events.Add(new AddressUpdatedEvent(customerId: 123,
                                   newAddress: "Maersk House, Braham Street"));
events.Add(new PhoneNumberChangedEvent(customerId: 123,
                                       newNumber: "01234567890"));

foreach (var @event in events)
{
    // this line silently does nothing:
    // C# has determined that we are invoking Publish<Event>
    // (and Event has no handlers)
    commandProcessor.Publish(@event);
}

If you make your model responsible for deciding what has happened to it (as you should, in my opinion) then each domain object may have a bunch of methods which return a List<Event> to be published by the invoking handler:

class Customer
{
    // ...
    public List<Event> UpdateContactDetails(string address, string phoneNumber)
    {
        var events = new List<Event>();
        if (address != this.Address)
        {
            this.Address = address;
            events.Add(new AddressUpdatedEvent(this.Id, address));
        }
        if (phoneNumber != this.PhoneNumber)
        {
            this.PhoneNumber = phoneNumber;
            events.Add(new PhoneNumberChangedEvent(this.Id, phoneNumber));
        }
        return events;
    }
}

...for which the only solution is to place calls to Publish inside the domain object itself (let's not even go there).

Part of the problem is the way C# type "inference" works. When I write commandProcessor.Publish(new MyEvent()), what it really means is commandProcessor.Publish<MyEvent>(new MyEvent()). C#'s syntax makes it look like I'm using a non-generic method although it actually is generic.

The current work-around is to query the concrete type of an event using GetType before you publish it, and invoke Publish using reflection:

commandProcessor.GetType()
    .GetMethods()
    .Single(m => m.Name == "Publish")
    .MakeGenericMethod(message.GetType())
    .Invoke(commandProcessor, new object[] { message });

I think it's possible to avoid foisting this sort of boilerplate on customers by carefully considering the polymorphic type of those functions. The rubric:

Parametric polymorphism (generics) says "I behave uniformly for all types T". In other words, a parametrically polymorphic function does not inspect the type of its parameters to behave conditionally. In the case of CommandProcessor.Send, this is certainly not true! The type is inspected (in SubscriberRegistry) to look up the handlers. Subtype polymorphism (interfaces), on the other hand, says "this function's behaviour depends on the runtime type of the argument". This is true of the CommandProcessor methods. So a subtyping-oriented API seems more natural to me.

Changing that API obviously wouldn't be backwards compatible. However I think that in nearly every case, source compatibility can be maintained (as long as clients aren't explicitly supplying the type argument: processor.Send<MyEvent>(new MyEvent())). I've implemented a proof-of-concept of that API in a branch on my fork and all the tests passed with no source changes. I can open a pull request with the change so we have something concrete to discuss, if you like.

@george-ayris

This comment has been minimized.

Contributor

george-ayris commented Aug 4, 2015

We've experienced the same problem with publishing failing to call the correct handler because there are no handlers registered to the event type of our base event class.

@holytshirt

This comment has been minimized.

Member

holytshirt commented Aug 4, 2015

@george-ayris that is just bad wiring up

@george-ayris

This comment has been minimized.

Contributor

george-ayris commented Aug 5, 2015

What I mean is that because the list of events is of type List<EventBaseClass> then when commandProcessor.Publish(event) is called we're calling Publish<EventBaseClass>(..) and not Publish<EventConcreteClass>(..) as Ben has described above.

Since it wouldn't make sense to register any handler for EventBaseClass then no handlers are called. We've got around the problem for the minute by using a list of delegates instead of events, so that the type of the events isn't upcast to the base class.

@iancooper

This comment has been minimized.

Member

iancooper commented Oct 13, 2015

I think we are conflating several issues:

  1. Limitations with generics. we can't route without knowing the type of the Event that you raise, so that we can find matching handlers. Trying to write Publish(IEnumerable events) thus proves impossible because we can't know the actual type of the event at dispatch from this list. We looked at alternatives here, but most of them are nasty.
  2. So we may ask why would we want to do it? The answer here is that a domain operation results in a list of notifications.
  3. Generally, I think this is wrong. A single command leads to a single event. UpdateContactDetailsCommand should raise a ContactDetailsUpdatedEvent. I don't believe it should raise multiple events because that creates a fanout issue that makes tracing had. Multiple handlers can subscribe to an event, so this does not preclude muliple listeners. I think arguments around "seperation of concerns" are a specious here. The command handler should not be aware of how its notification should be used, only that it reports it. Because the relationship between the command and the event associated with it is strong, I would suggest that the handler raise the event not the model.
  4. It's worth noting that we should not always privilidge handler implementation to be a domain model. An operation script (transaction script) might be equally appropriate, and it has no domain to raise its events from.

But I think the fundamental problem here is that you don't want to fan out events llike this. You are mixing an event sourcing strategy up with a domain events strategy. They are not the same thing.

@iancooper

This comment has been minimized.

Member

iancooper commented Oct 13, 2015

@benjamin-hodgson and I talked F2F. We agree that the value here lies in using a GetKey() on an IRequest and on IHandleRestues that we use to map a request to a handler and defaulting to RTTI for Command, Event, and RequestHandler that you can override. That would allow us to add a Publish(IEnumerable events) or even Publish(iEnumerable events) that worked because we could override the RTTI to use a unique key value (such as a Guid).

@benjamin-hodgson Not sure if you want to close and raise a new issue? Or just progress this one

@iancooper iancooper closed this Oct 13, 2015

@benjamin-hodgson

This comment has been minimized.

Contributor

benjamin-hodgson commented Oct 16, 2015

@iancooper I'll raise a new issue

@Red-F

This comment has been minimized.

Contributor

Red-F commented Dec 20, 2015

@benjamin-hodgson if you still have that example available in a runnable form, try changing the line

  commandProcessor.Publish(@event)

into

  commandProcessor.Publish((dynamic)@event)

This should work. As a proof of concept I changed the helloworld example from

            commandProcessor.Send(new GreetingCommand("Ian"));

Which prints "Hello Ian", into:

            ICommand command = new GreetingCommand("Ian");
            commandProcessor.Send(command);

Which results in an ArgumentException "No command handler was found for the typeof command paramore.brighter.commandprocessor.ICommand - a command should have exactly one handler.". Adding the cast to dynamic:

            ICommand command = new GreetingCommand("Ian");
            commandProcessor.Send((dynamic)command);

allows the RTTI to be used and the correct command handler is found and it faithfully prints "Hello Ian" again.

It is an elegant solution requiring absolutely no changes to Brighter.

@benjamin-hodgson

This comment has been minimized.

Contributor

benjamin-hodgson commented Dec 20, 2015

Personally I wouldn't call it elegant, but yes, it works 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment