akka / akka

A platform to build and run apps that are elastic, agile, and resilient. SDK, libraries, and hosted environments.
https://doc.akka.io
Other
13.06k stars 3.59k forks source link

Clarify that eventHandler must be free of side effects #25720

Closed patriknw closed 5 years ago

patriknw commented 6 years ago

@viktorklang suggested that we should rename eventHandler because a handler has too much association with side effects.

One proposal is stateUpdater.

Originally I think we named them onCommand and onEvent.

viktorklang commented 6 years ago

Perhaps too verbose: stateAfterEvent: (State, Event) => State

patriknw commented 6 years ago

I think I used applyEvent somewhere and kind of like that, but I think there were other opinions.

viktorklang commented 6 years ago

What about aggregator?

patriknw commented 6 years ago

I understand your thinking, and it’s good. Unfortunately “aggregate” is already taken in DDD that is not so far from what we do here, but it has a different meaning.

viktorklang commented 6 years ago

Well, technically, if it creates the state (the aggregate (root)) then it by very definition is an aggregator :)

patriknw commented 6 years ago

The DDD Aggregate is more about boundaries and invariants, while here it's about summarizing (aggregating) the events into a current state.

Excerpt from http://domainlanguage.com/wp-content/uploads/2016/05/DDD_Reference_2015-03.pdf

Cluster the entities and value objects into aggregates and define boundaries around each. Choose one entity to be the root of each aggregate, and allow external objects to hold references to the root only (references to internal members passed out for use within a single operation only). Define properties and invariants for the aggregate as a whole and give enforcement responsibility to the root or some designated framework mechanism.

viktorklang commented 6 years ago

«A DDD aggregate is a cluster of domain objects that can be treated as a single unit.» <- https://www.martinfowler.com/bliki/DDD_Aggregate.html

the aggregator will be used to construct that cluster of domain objects? :)

TimMoore commented 6 years ago

I like the applyEvent suggestion.

WadeWaldron commented 6 years ago

I think, if I try to eliminate any personal biases, and imagine I was implementing this from scratch, I would probably end up with something like calculateState or determineState. The reason is that it is a function that takes some parameters and then returns a state. I think those names reflect the fact that the function is intended to be side effect free. In that respect, stateUpdater seems the most similar out of the suggestions. However, I don't really like it because when I think update I think about updating something in place. It doesn't make me think of a pure function.

Now, having said all that, if I put the personal biases back in, then I will say that I like the symmetry of commandHandler/eventHandler. To be honest, I am not sure that the term handler really creates any more confusion than would exist with any other solution unless we name it something like sideEffectFreeEventHandler (And I am not suggesting we do). When I actually write these things in existing Akka code, I usually extract them into functions like handle(command) and handle(event). So commandHandler and eventHandler just feels like an extension of what I am already doing. My next choice would be applyEvent but only because it sounds more like eventHandler, and I would be tempted to implement that with an apply(event) or similar. There is a certain amount of satisfaction with the idea of implementing handle(command) and apply(event). That has a nice symmetry. However, I worry about apply since it has special meaning in Scala.

viktorklang commented 6 years ago

We could remove this naming problem altogether by having a State[Event] type which has an apply(Event) method.

WadeWaldron commented 6 years ago

Well, to be honest, that might be how I would actually implement it. I would leverage a state object that has functions to transform it based on the event. So in that respect the idea has merit.

I imagine the concern that would be raised is that those state objects are then bound to the implementation details of Akka. Ideally those state objects would be independent of any specific technology concerns.

Another concern would be that apply(event) is not as intent revealing as it could be. For example if the result of applying the event is to deposit money into a bank account, then a more intent revealing implementation would be something like deposit(amount). It gets a bit better with some better naming so instead of apply(event) if you write it as apply(depositEvent) then it looks a little more intent revealing. But you can't get as much help from your IDE.

viktorklang commented 6 years ago

@WadeWaldron

I imagine the concern that would be raised is that those state objects are then bound to the implementation details of Akka. Ideally those state objects would be independent of any specific technology concerns.

In that case, it would be trivial to wrap an akka-agnostic aggregate into a State[Event]

Another concern would be that apply(event) is not as intent revealing as it could be. For example if the result of applying the event is to deposit money into a bank account, then a more intent revealing implementation would be something like deposit(amount). It gets a bit better with some better naming so instead of apply(event) if you write it as apply(depositEvent) then it looks a little more intent revealing. But you can't get as much help from your IDE.

In the case of obtaining Events, you could do:

def deposit(m: Money): DepositEvent // throws on failure?

and

def apply(ae: AcoountEvent): Account apply(depositEvent)

WadeWaldron commented 6 years ago

@viktorklang Actually, I think if you follow through with your first idea, then you would have your State[Event] wrapper implement the apply(event) logic. Then your domain class would implement the deposit(m: Money). So you domain class is intent revealing, and Akka agnostic. Then you have an Akka specific layer over top that is focused around the events.

Of course, then the complaint would be that there is too much boilerplate. But I am okay with that. If you don't want boilerplate, then eliminate the middleman. Have your domain class implement the Akka State[Event] class directly. The extra step of eliminating the middleman is for the purists, and yes, it requires more boilerplate.

viktorklang commented 6 years ago

@WadeWaldron Yeah, that resonates well with me.

octonato commented 6 years ago

As said in another forum I don't think we should step out of eventHandler.

Not only it has symmetry with commandHandler, but that's also the term used in CQRS/ES. We should not change this.

I don't know why the word "handler" is being associated with side-effects. There is nothing, absolutely nothing in it that refers to side-effects. A handler is something that reacts (handles) to input. In the case of an event handler on the write-model, it reacts by updating the state. That's a common idiom in ES. Everybody refers to it as an event handler, we should not come up with new terms.

In its most basic form both handlers are side-effect free and have the following signature: (types are just indicative and are used here as an illustration)

CommandHandler: State => Command => Try[List[Event]]
// Try[List[Event]] this is just to express that it can be a success or a failure 
// and that it can emit zero or many events. This can be expressed in many different forms.

EventHandler: State => Event => State

In Akka Typed, the command handler has a different shape, State => Command => Effect, which includes not only the concept of persisting but also side-effects to be run after persistence, but the place where to declare the persistence and the side-effects are well-defined by the API. The user is guided to define it in the right place.

patriknw commented 6 years ago

I agree with @renatocaval. I haven't seen a problem with calling it event handler, because I don't associate handler with side effects.

viktorklang commented 6 years ago

Perhaps it is due to me having done quite a bit of "event handling" code outside of EventSourcing: https://en.wikipedia.org/wiki/Event-driven_programming#Event_handlers

Most things like GUI Event Handlers etc are highly side-effecting. So I think keeping eventHandler means putting some pretty clear documentation to set expectations straight for newcomers to eventsourcing.

patriknw commented 6 years ago

Yeah, documenting that event handling must be free of side effects is almost #1 concept to learn when it comes to event sourcing