eclipse-ee4j / krazo

Apache License 2.0
50 stars 19 forks source link

Krazo does not work with OpenLiberty 22.* beta (Jakarta EE 10) #364

Open mthmulders opened 1 year ago

mthmulders commented 1 year ago

Recently, I was experimenting with Krazo 3.0.1 on Jakarta EE 10, using OpenLiberty 22.0.0.13-beta. I stumbled upon the same error message as reported in #282. I thought it had been resolved with #288, so I checked if the LibertyHttpCommunicationUnwrapper got invoked - it did.

So I dug a little further to find the ServletUtil#unwrapRequest method of OpenLiberty. As pointed out by @jesse-gallagher in #282, Liberty expects to be able to get an instance of their IExtendedRequest type from the passed-in request object. The passed-in request object (supplied by Krazo) is a HttpServletRequestWrapper instance backed by a Weld proxy for an SRTServletRequest60 instance. The unwrapRequest method doesn't handle this object properly, and hence throws the "SRV.8.2: RequestWrapper objects must extend ServletRequestWrapper or HttpServletRequestWrapper".

(Aside) the SRTServletRequest60 does in fact implement the IExtendedRequest interface. I think the isInstance() check in Liberty fails due to the Weld proxy around it. I've logged an issue for that: https://github.com/OpenLiberty/open-liberty/issues/23774.

Anyway, when Krazo invokes RequestDispatcher#forward from ServletViewEngine#forwardRequest(), it passes a HttpServletRequestWrapper instance that wraps "the original request". That "original request" comes from the ViewEngineContext, which was constructed by the ViewableWriter. The ViewableWriter in turn gets it @Inject-ed, which explains the Weld proxy.

In my own experiment, I could work around this by implementing a WeldHttpCommunicationUnwrapper, which works pretty much the same as the existing LibertyHttpCommunicationUnwrapper. Using reflection, it checks if the request implements WeldClientProxy. If so, it invokes WeldClientProxy#getMetadata() and then WeldClientProxy.Metadata#getContextualInstance() to get the actual SRTServletRequest60 instance.

Would you be interested in having this in Krazo's code base?

erdlet commented 1 year ago

Hi @mthmulders,

thanks a lot for testing and providing this detailed analysis. It'd be great when you vould provide your solution to Krazo, since we're a little bit short of time at the moment and Liberty makes the most ptoblems

jesse-gallagher commented 1 year ago

I don't have much to add other than: thanks for adding this! I hadn't yet had an occasion to move to JEE 10, but I did today, hit this problem again, built a snapshot build, and it works great once more.