[#233] Reactivating the Thymeleaf extension

[jelemux]

This reenables the Thymeleaf extension by using its 3.1.0.M1 milestone release (which uses the jakarta namespace).

To adapt to changes in Thymeleaf, CDIWebContext had to be changed to use the jakarta namespace and the new IWebExchange. Since JakartaServletWebApplication has been established to wrap ServletContext in Thymeleaf, there needed to be a producer to inject it into both DefaultTemplateEngineProducer and ThymeleafViewEngine. But because JakartaServletWebApplication is a final class, a wrapper for it had to be written in order to use CDI.

Fixes #233

[chkal]

I'm currently creating two instances of JakartaServletWebApplication (for DefaultTemplateEngineProducer and ThymeleafViewEngine respectively) which is not pretty, but it should not be the problem here.

Maybe we could simply "produce" an application-scoped instance of JakartaServletWebApplication and use it in both classes? Not sure if this works, as I didn't have a detailed look at the code yet.

The problem is that somehow the request from the ViewEngineContext has a ServletContext different from the one injected into the ViewEngine and Thymeleaf doesn't like that at all.

It is just a guess, but if we use @Inject ServletContext, we will most likely get a CDI proxy class instead of the real ServletContext instance. I guess there are a few ways to get a reference to the context:

// (A) Directly inject via CDI, but I guess you will get a wrapper
@Inject 
private ServletContext servletContext;

// (B) Inject the request and then call request.getServletContext(). Should return the "real" servlet context?
@Inject 
private HttpServletRequest request;

// (C) Like (B) but Krazo spezific and will return the same request as @Context HttpServletRequest
@Inject 
@JaxRsContext
private HttpServletRequest request;

// (D) via ViewEngineContext.
viewEngineContext.getRequest(HttpServletRequest.class).getServletContext()

To make things more complicated: HttpServletRequests can be wrapped via servlet filter. We actually even have some special handling in Krazo to "unwrap" these wrappers. The result is what you will get from ViewEngineContext. See:

https://github.com/eclipse-ee4j/krazo/blob/d2467daaa211ae0b8002af54875c68980812a3a0/core/src/main/java/org/eclipse/krazo/core/ViewableWriter.java#L136-L137

Maybe I'll find some time tomorrow to have a look at your branch.

[jelemux]

Maybe we could simply "produce" an application-scoped instance of JakartaServletWebApplication and use it in both classes? Not sure if this works, as I didn't have a detailed look at the code yet.

JakartaServletWebApplication is a final class so it cannot be proxied by CDI. To solve this, I see three possibilities:

1. Create a wrapper object for JakartaServletWebApplication and produce that.
2. Create an issue/PR with Thymeleaf asking them to remove the final modifier.
3. Create an issue/PR with Thymeleaf including the buildExchange() method (which we need) in IServletWebApplication. That way we could just produce and inject that.
   (This would add the need for a wrapper around HttpServletRequest and HttpServletResponse in Thymeleaf.)

It is just a guess, but if we use @Inject ServletContext, we will most likely get a CDI proxy class instead of the real ServletContext instance.

Thanks, that seems to be the case. My current (local) codebase uses option D and that works.

[jelemux]

I now have it in a working state and pushed the changes. Have a look and tell me what you think.

[jelemux]

I don't like my current solution at all. I'll try solution 3:

3. Create an issue/PR with Thymeleaf including the buildExchange() method (which we need) in IServletWebApplication.

[erdlet]

@chkal after a look into the PR I'm afraid we need a CQ because of the minor update, right? Do you know if we need one for the final 3.1.0 again after creating one for 3.1.0.M1?

[erdlet]

I don't like my current solution at all. I'll try solution 3:

3. Create an issue/PR with Thymeleaf including the buildExchange() method (which we need) in IServletWebApplication.

Good idea, thanks for doing that 🍻

[jelemux]

As we agreed yesterday in the MVC Spec Meeting: Since there is no reaction from Thymeleaf maintainers to my PR, we'll just merge this current solution.

The only open point is the CQ that @erdlet pointed out.

@chkal after a look into the PR I'm afraid we need a CQ because of the minor update, right? Do you know if we need one for the final 3.1.0 again after creating one for 3.1.0.M1?

[erdlet]

CQ https://dev.eclipse.org/ipzilla/show_bug.cgi?id=23960 created.

[erdlet]

CQ is approved, so we can finally merge this PR :)

@jelemux please let me known if you want to squash something before merge or if everything is fine for you.

[jelemux]

I already squashed everything, so should be good to go for the merge