Single handler for events in code-first sdk?

[efgpinto]

          Nice! Is single handler for events planned in some incoming changes?

Originally posted by @aludwiko in https://github.com/lightbend/kalix-jvm-sdk/pull/1468#pullrequestreview-1291429049

[efgpinto]

@aludwiko @octonato opened this side issue for discussion. I don't have a very good view on what this would entail effort-wise so feel free to expand on your thoughts. I have some questions though?

• would this prevent the possibility of having specific event handlers per type?
• without pattern matching, what will this look like in clients code and what benefits does it bring?

[aludwiko]

Ok, my 2 cents. Currently, we have sth like this:

public class ShowEntity extends EventSourcedEntity<Show> {

  @EventHandler
  public Show onEvent(ShowCreated showCreated) {
    return Show.create(showCreated);
  }

  @EventHandler
  public Show onEvent(SeatReserved seatReserved) {
    return currentState().apply(seatReserved);
  }

  @EventHandler
  public Show onEvent(SeatReservationCancelled seatReservationCancelled) {
    return currentState().apply(seatReservationCancelled);
  }
}

After the change I would expect sth like:

public class ShowEntity extends EventSourcedEntity<Show, ShowEvent> {

  @EventHandler
  public Show onEvent(ShowEvent showEvent) {
    if (showEvent instanceof ShowCreated showCreated){
      return currentState().apply(showCreated);
    }else if (showEvent instanceof SeatReserved seatReserved){
      return currentState().apply(seatReserved);
    }else if (showEvent instanceof SeatReservationCancelled seatReservationCancelled){
      return currentState().apply(seatReservationCancelled);
    }else {
      throw new IllegalStateException("missing handler");
    }
  }
}

Still not great because of the runtime exception, but from my perspective, it's more concise. If we embrace pattern matching in java:

public class ShowEntity extends EventSourcedEntity<Show, ShowEvent> {

    @EventHandler
  public Show onEvent(ShowEvent showEvent) {
    return switch (showEvent) {
      case ShowCreated showCreated -> currentState().apply(showCreated);
      case SeatReserved seatReserved -> currentState().apply(seatReserved);
      case SeatReservationCancelled seatReservationCancelled -> currentState().apply(seatReservationCancelled);
    };
  }
}

Now the compiler will check if we didn't forget anything, and personally, I would move this matching to the domain aggregate itself (why I think it's better is a separate discussion - more than happy to discuss this as well):

public class ShowEntity extends EventSourcedEntity<Show, ShowEvent> {

    @EventHandler
  public Show onEvent(ShowEvent showEvent) {
    return currentState().apply(showEvent);
  }
}

I know that pattern matching is not a fully released Java feature, but it's just a matter of a compilation flag and we are ready to go. It's not that they will abandon this feature, thus we can be ready for it now and give a way to nicely use it for more experience Java devs.

[octonato]

I see your point @aludwiko. I only find it has value with pattern matching. Otherwise, is just a matter of style and not a big deal, IMO.

The possibility to have exhaustive matching has certainly value. The idea of having less methods annotated with @EventHandler is less interesting. With this approach, we don't have many event handlers in the Entity, but we have overloaded methods in domain classes. We just move things around.

It's important to note that we started with a single event handler with if instance statements. And the feedback was that it sucks. And the conclusion was that method overload is what people expects and make it easier for users to follow.

That said, if we take that path, we should be able to continue to support the current approach, (ie: many @EventHandlers).

For that matter, when an event is delivered, we look up the typeUrl in a map of typeUrl -> methodToCall. Each event will have a different typeUrl and we must ensure that the single method receiving the common interface is properly registered in that map.

[aludwiko]

I didn't touch this part of the code for some time, but my gut feeling is that it should be possible to support both styles. Not sure if we should allow to use them simultaneously, e.g. 3 handlers for specific events, and a "default" one with the interface.

[aludwiko]

And the feedback was that it sucks. And the conclusion was that method overload is what people expects and make it easier for users to follow.

Interesting, was it internal or external feedback? I remember that Yoppworks wanted to use a single handler https://github.com/lightbend/kalix-proxy/issues/1738

[octonato]

Internal. And I'm only referring to java users and concerning code-first java with Spring, not scala users.

Scala users would of course prefer pattern matching, but they are not affected by the choices we make for Java/Spring user.

[efgpinto]

We have this for ES entities now and I think we could consider it for Actions and Views. That is, you would be able to declare only a method for the enclosing Type instead of multiple handlers.

[aludwiko]

Yes, I would like to use this feature for any ES subscription.