Your opinion wanted - aggregate event handler parameter types

Imported from Google Groups
#1

Hello all,

every once in a while, I swap things around on this mailinglist. This time, it’s me asking the questions ;-).

As you might know (and if not, you’ll know from now on), you can use all sorts of parameters in @CommandHandler and @EventHandler annotated methods. If you use Spring, you can even use parameters where Axon will automatically inject Spring beans.
Recently, I had received feedback (mostly from OSGi users) that injecting the ParameterResolverFactory (ultimately responsible for providing the values to use in parameters) would be better than the current singleton approach. I have to agree with that and changes are on the way in 2.1.

There is one challenge, though: the Aggregates. Unlike all other components, creation of aggregates is not (always) managed by Axon. So, somehow, the Aggregate instance needs to get access to the handler mechanism by Axon.
I see a few options to make each Aggregate instance “aware” of the ParameterResolverFactory it should use: Note that this problem doesn’t occur on @CommandHandler methods in Aggregates, just the @EventHandler ones.

  1. Use a singleton holder that keeps a reference to the ParameterResolverFactory for the aggregates to use (thus single instance per VM)

  2. A variation of the above where an (additional) configuration can be provided per aggregate type

  3. Have the Repository configure the right ParameterResolverFactory. The fact that these extra parameters aren’t available when creating an aggregate is not a problem.

  4. Use the UnitOfWork to hold a reference to the ParameterResolverFactory instance to use while executing that command

  5. Don’t worry about custom parameters. The default ones are good enough on Aggregate Event Handlers

  6. I know another way: …
    Note that options 1 through 4 will be automatically configured for Spring users when using <axon:annotation-config …>.
    Please cast your votes.

Cheers,

Allard

#2

Please please PLEASE do not go with option 5. I think custom parameters are one of the most elegant and powerful parts of Axon and I’d probably stop using the framework if they were removed.

As long as the API doesn’t change (as in, my @CommandHandler and @EventHandler methods on my AggregateRoots don’t look any different), then I don’t have a strong opinion how this is implemented.

I think I need a little more information to weigh in on designs 1 through 4, would you be able to post some code snippets to aide your explanation for how parameter resolution is currently happening?

#3

Hi William,

that’s clear. I expected this to happen, but I wanted to put an unbiased list of options there.
The API’s don’t change. The defaults will work fine, and if you use Spring, the Axon-Spring components will take care of the extra configuration required.

Here are some code snippets that I had in mind with 1 through 4.
1:
AnnotationConfiguration.getDefaultConfiguration().setParameterResolverFactory(new MyCustomParameterResolverFactory());

2:
Same as the previous (for the defaults), but also allows configuration to be overridden for a specific aggregate:
AnnotationConfiguration.getConfigurationFor(MyAggregateRoot.class).setParameterResolverFactory(new MyCustomParameterResolverFactory());
This setting sets the parameter resolver for the whole aggregate, not only the root class.

3:
EventSourcingRepository repo = new EventSourcingRepository(…);
repo.setParameterResolverFactory(new MyCustomParameterResolverFactory());
// obviously, commands that do Aggregate myaggregate = new MyAggregate(); cannot use the custom parameter resolver factory.

  1. An interceptor would have to do:
    CurrentUnitOfWork.get().registerResource(ParameterResolverFactory.class, new MyParameterResolverFactory());

The aggregate will check for a resource under the key “ParameterResolverFactory.class”, and use that, when found.

Note that the default ParameterResolverFactory instance is one that looks for custom instances on the classpath, just like the current implementation does. So it is still possible to provide your own resolvers. I have changed the implementation so that it should work better in OSGi containers (although that hasn’t been tested yet). It now uses the target class’ classloader to find the custom parameter resolvers.

Cheers,

Allard

#4

I was going to suggest option 3 was the best since it avoided statics, and people who want to instantiate AggregateRoots manually may wire in this dependency manually via a setter. But then it got me thinking that the ParameterResolverFactory is a pretty strange and techy abstraction to be injecting in to your AggregateRoots.

This made me think that it might be better to inject something like “eventPublisher” instead. The apply(event) method on the AbstractAnnotatedAggregateRoot would then call this, and it would be the eventPublishers responsibility for ultimately calling back on to the AggregateRoot to apply the event, and it may use the ParameterResolverFactory to do so.

This should also enable you to remove some of the code out of AbstractAnnotatedAggregateRoot, including the existing use to static calls to AggregateAnnotationInspector which is how you currently hook in to Spring I believe.

It also enables people that want to do finer grained unit testing (in addition or instead of the GivenWhenThen testing) against their AggregateRoots because they may use a mock eventPublisher.

#5

Hi William,

avoiding statics is generally a good idea, mainly for testability of the code. But for use in a framework, I don’t think it’s a very bad option. Mainly because it will be hidden to most of the developers. Spring users don’t even have to see it at all, since I am planning to automatically wire all available ParameterResolverFactories in the Spring context. Since the default factory does a lookup on the classpath, it is still

When it comes to testing, the @EventHandler methods themselves can always be invoked directly. So fine-grained tests that don’t use the given-when-then fixtures will still be able to work. The test fixtures will also automatically use a ParameterResolverFactory that allows for customization.

I have also had some offline discussion about this topic, and I am thinking about applying the 2nd option. It might not be the absolute perfect solution in all aspects, but compared to the current situation, it is an improvement in terms of customization and compatibility (with OSGi, mainly).

Cheers,

Allard

#6

I agree that using your approach that @EventHandler methods can be invoked directly for unit testing, however the same cannot be said for @CommandHandler methods that publish/apply events.

The reason my suggestion excited me a lot is it opens up the opportunity to completely remove the need to extend a base-class. Imagine something like this

`
@AggregateRoot // is this annotation even needed?
public class MyAggregate {
private final EventPublisher eventPublisher;

@CommandHandler(CreateMyAggregate.class)
public MyAggregate(CreateMyAggregate command, EventPublisher eventPublisher) {
this(eventPublisher);
// validate some things
eventPublisher.apply(new MyAggregateCreated());
}

// for axon to hydrate this later
private MyAggregate(EventPublisher eventPublisher) {
this.eventPublisher = eventPublisher;
}

@CommandHandler(MutateMyAggregate.class)
public void mutateMyAggregate(MutateMyAggregate command) {
// validate some other things
eventPublisher.apply(new MyAggregateMutated());
}

@EventHandler
public void on(MyAggregateCreated event) {
// set up some state
}

@EventHandler
public void on(MyAggregateMutated event) {
// set up some other state
}
}

`

The reason this is so cool is no more parent class means way less conceptual bulk and strange things happening above you. The old approach of extending AbstractAnnotatedAggregateRoot will still work, and is available as on option for people and for backwards compatibility. This class is now testable at the unit level for both commandhandlers and eventhandlers but using a mocked eventPublisher - however again, the existing GivenWhenThen integration tests will also still work. It gives people more options and provides a lot more flexibility than the other approaches. It is also a tonne more Axon agnostic and paves the way for plugging in another CQRS framework if one day JSR standard CQRS/Eventsourcing annotations exist :). This is also in line with other persistence mechanisms like javax.persistence where no base-class is required, in fact you can extend OTHER arbitrary classes if you want to, and it also let’s us escape that kind of “active record” feel that we have at the moment when we extend the abstract class. I also find it a lot more transparent having the eventPublisher present as a field, it exposes the fact that there is a dependency going on, and again you can still use the old approach if you don’t want to know about it.

What do you think?

#7

I’d love to provide a pull request as an example but I’m currently abroad so time is limited

#8

Hi William,

this looks pretty cool. I have been looking for ways to remove the required extending of an Axon class and this might be a good way to do it.

I see a chicken and egg problem here, however. Probably not one we can’t solve, though. To inject the EventPublisher, we need a ParameterResolverFactory. But the EventPublisher also needs a ParameterResolverFactory to be able to invoke all the event handler methods. For snapshotting and caching to work properly, an aggregate needs to be serializable. This means all referenced objects should also be serializable (or transient). To make the EventPublisher serializable, it should not really serialize the ParameterResolverFactories it has been initialized with. So to do that, it probably needs to be able to do a (singleton/static) lookup to initialize itself after deserialization. It should really only serialize the reference of the aggregate it has been injected to, so that it knows where to dispatch applied events.

I’m definitely going to look into this further for future versions (maybe Axon 3?). However, it doesn’t seem to remove the need for another way of configuring the ParameterResolverFactories available to an aggregate.

So for 2.1, I’m still opting to use the static style of configuration. Obviously, other solutions are most welcome.

Cheers,

Allard

#9

I don’t think there is a circular dependency problem here. The ParameterResolverFactory get’s instantiated first, then the EventPublisher gets instantiated second, injected with the former. Then when we need to instantiate our aggregates, the factory (or whatever it is) uses the ParameterResolverFactory to inject the EventPublisher in to the aggregate. The factory would depend on both. I think this will work ok.

The serialization problem is legitimate though, I guess you may need to use some clever use of statics hidden inside the EventPublisher to get around this. Atleast the client is hidden from it. I mean it is the same as any other dependency the aggregates might want to have. In theory the EventPublisher could be @Autowired in at the field level just like any other dependency, but I guess any fields like this have the serialization problem. I don’t have a solution to this problem really. Maybe using an alternative to serializing for snapshots, like manually providing a memento?