sizovs / PipelinR

PipelinR is a lightweight command processing pipeline ❍ ⇢ ❍ ⇢ ❍ for your Java awesome app.
https://github.com/sizovs/PipelinR
MIT License
420 stars 59 forks source link

Command Handler as CDI bean with @Transactional fails in Handler.matches #28

Closed dstutz closed 1 year ago

dstutz commented 1 year ago

I am refactoring an existing Jakarta EE application to utilize Pipelinr. I have already implemented several commands and handlers and things were working well until I added a handler with the handle method (or class) annotated with CDI's @Transactional. It appears the proxy object is messing things up. The Command.Handler.matches goes into FirstGenericArgOf.isAssignableFrom and on line 15 gets the generic interfaces. For the normal handlers this ends up being an array of 1 and the type is indeed ParameterizedType so the cast on line 20 works. When I have a handler with @Transactional the call to getGenericInterfaces returns a Class[] instead of Type[] and I get the following error:

java.lang.ClassCastException: class java.lang.Class cannot be cast to class java.lang.reflect.ParameterizedType (java.lang.Class and java.lang.reflect.ParameterizedType are in module java.base of loader 'bootstrap') at deployment.example.war//an.awesome.pipelinr.FirstGenericArgOf.isAssignableFrom(FirstGenericArgOf.java:20) at deployment.example.war//an.awesome.pipelinr.Command$Handler.matches(Command.java:18)

The working case: image

with @Transactional: image

All of my command handlers are container managed and are populated using the technique from #16 :

@Produces
    @ApplicationScoped
    public Pipeline getPipeLine(Instance<Command.Handler<?, ?>> handlers, Instance<Command.Middleware> middlewares) {
        List<Command.Handler> commandHandlers = handlers.stream().map(r -> (Command.Handler) r).toList();
        return new Pipelinr()
                .with(middlewares::stream)
                .with(commandHandlers::stream);
    }

Is there any way to work around this?

sizovs commented 1 year ago

Hi @dstutz

I'll look into this early next week. For now, the workaround is overriding the matches method so that works for your custom proxy. If you have many handlers – you can create an abstract class.

Since you'll have to deal with generics, Guava can help a bit:

default boolean matches(C command) {
   TypeToken<C> typeToken = new TypeToken<C>(getClass()) { // available in Guava 12+.
      return typeToken.isSupertypeOf(command.getClass());
   }
 }

– Eduards

dstutz commented 1 year ago

In the command handler with the @Transactional I added:

 @Override
    public boolean matches(TransactionalCommand command) {
        TypeToken<TransactionalCommand > typeToken = new TypeToken<TransactionalCommand>(getClass()) {};
        return typeToken.isSupertypeOf(command.getClass());
    }

Now I get in the logs:

java.lang.ClassCastException: class com.example.query.NoTransactionQuery cannot be cast to class com.example.command.TransactionalCommand (com.example.query.NoTransactionQuery and com.example.command.TransactionalCommand are in unnamed module of loader 'deployment.example.war' @6ee6489e)
    at deployment.example.war//com.example.command.TransactionalCommand Handler.matches(TransactionalCommandHandler.java:35)
    at deployment.example.war//com.example.command.TransactionalCommandHandler$Proxy$_$$_WeldSubclass.matches(Unknown Source)
    at deployment.example.war//an.awesome.pipelinr.Pipelinr$ToFirstMatching.lambda$route$0(Pipelinr.java:170)

Where NoTransactionQuery I think is just the first handler the pipeline is attempting to compare? I was attempting to write a matches method yesterday but was getting the CCE no matter what I did in this matches method, even if it was return false;.

I also noticed the line number (35) the CCE is reported at is the start of the class public class TransactionalCommandHandler implements Command.Handler<TransactionalCommand , Voidy> { NOT line 63 where the matches method is.

sizovs commented 1 year ago

I am now running a masterclass, and I will look at it early next week. Hope you'll survive by then : )

For now, you can try eliminating all generics and just hard-code the command type in every matches.

dstutz commented 1 year ago

Something that works, but isn't our preferred solution, is adding @javax.ejb.Stateless to the handler class. That gives the methods transactions, but doesn't cause a problem with generics in the matches methods.

sizovs commented 1 year ago

Hi @dstutz

Sometimes Weld is creating hairy proxies that require sophisticated handling. Fixed and works with PipelinR 0.8.

Thanks for reporting!

dstutz commented 1 year ago

Well that's awesome, thanks for taking care of that so fast. I was talking with co-worker and we might look to re-implement the transaction handling using middlewares similar to how you do in your Bank example project as it would end up a bit more technology agnostic. That said, I tested with @Transactional and everything is working great.