Add support for Jetty 12

[justin-tay]

The support for the CdiDecoratingListener in the JettyContainer does not work in Jetty 12 as the attribute names now also contain the environment names of the module.

Module	Attribute	Attribute Name
ee9-cdi-decorate	JETTY_CDI_ATTRIBUTE	org.eclipse.jetty.ee9.cdi
	CDI_DECORATING_LISTENER_ATTRIBUTE	org.eclipse.jetty.ee9.cdi.decorator
ee10-cdi-decorate	JETTY_CDI_ATTRIBUTE	org.eclipse.jetty.ee10.cdi
	CDI_DECORATING_LISTENER_ATTRIBUTE	org.eclipse.jetty.ee10.cdi.decorator

Existing Codes:

core/environments/servlet/core/src/main/java/org/jboss/weld/environment/jetty/JettyContainer.java

Line 46 in b57d3e9

public static final String JETTY_CDI_ATTRIBUTE = "org.eclipse.jetty.cdi";

core/environments/servlet/core/src/main/java/org/jboss/weld/environment/jetty/JettyContainer.java

Line 49 in b57d3e9

public static final String CDI_DECORATING_LISTENER_ATTRIBUTE = "org.eclipse.jetty.cdi.decorator";

References:

• https://github.com/eclipse/jetty.project/blob/28100e8da711e44c0722ed10bd413ae862497539/jetty-ee9/jetty-ee9-cdi/src/main/java/org/eclipse/jetty/ee9/cdi/CdiServletContainerInitializer.java#L45
• https://github.com/eclipse/jetty.project/blob/28100e8da711e44c0722ed10bd413ae862497539/jetty-ee9/jetty-ee9-cdi/src/main/java/org/eclipse/jetty/ee9/cdi/CdiDecoratingListener.java#L25
• https://github.com/eclipse/jetty.project/blob/28100e8da711e44c0722ed10bd413ae862497539/jetty-ee10/jetty-ee10-cdi/src/main/java/org/eclipse/jetty/ee10/cdi/CdiServletContainerInitializer.java#L45
• https://github.com/eclipse/jetty.project/blob/28100e8da711e44c0722ed10bd413ae862497539/jetty-ee10/jetty-ee10-cdi/src/main/java/org/eclipse/jetty/ee10/cdi/CdiDecoratingListener.java#L25

[manovotn]

Hello and thanks for the PR.

Before merging it, we should probably also take a look at Jetty servlet testing.
We used to have a module for that but it's been dead since EE 10 due to (IIRC) some incompatible changes and Arq. adapter not existing so it's currently excluded.
I've noticed that the adapter just had a release which might be what we need.

If you want to take a look at that as well, be my guest. If not, I'll poke around but it might take me a while as my jetty understanding is very low :)

[manovotn]

I took a look but Jetty 12 requires JDK 17 which the first issue - Weld 5 is compatible with EE 10 and as such needs to be able to run with JDK 11. This way, we couldn't run servlet tests on JDK 11.
But setting that aside, I didn't manage to get it running for 17 either due to an Arq. exception that I wasn't able to get past:

java.lang.IllegalArgumentException: ArquillianServletRunnerEE9 not found. Could not determine ContextRoot from ProtocolMetadata, please contact DeployableContainer developer.

Code I've changed can be seen here (on top of your commit) - https://github.com/weld/core/compare/master...manovotn:core:jetty12?expand=1

@justin-tay what did you test your proposed change with? Can you share it?

[justin-tay]

I have built an example project for testing at https://github.com/justin-tay/jetty-weld-war-example.

Jetty 12 EE10 does the CDI setup in org.eclipse.jetty.ee10.cdi.CdiServletContainerInitializer.

Currently Weld is set to the snapshot in the POM but if you change it to the release you should see the issue.

[justin-tay]

I have managed to get the Jetty tests to run and most of them to pass on this branch https://github.com/justin-tay/core/tree/test_jetty12_master.

[INFO]
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR]   ConfigWithoutJettyEnvTest>ConfigTestBase.testDOS:63->ConfigTestBase.assertBeans:56 Null bean manager
[ERROR]   ListenerSymmetryTest.testListenerCalledSymmetrically:62 expected:<[OK]> but was:<[FAILURE, Size:4]>
[ERROR] Errors:
[ERROR]   CrossContextForwardTest.testCrossContextForward:77 » FailingHttpStatusCode 500...
[INFO]
[ERROR] Tests run: 56, Failures: 2, Errors: 1, Skipped: 1

[justin-tay]

It looks like the changes to the JettyContainer might not be needed after all.

I had reported a separate issue with the ee10-cdi-spi mode with the Jetty project and it looks like they may be removing the environment prefix from the attribute names.

• Deploying WAR with ee10-cdi-spi fails with Weld 5/CDI 4 jetty/jetty.project#10356
• Issue #10356 Update Weld integration jetty/jetty.project#10359

[manovotn]

I have built an example project for testing at https://github.com/justin-tay/jetty-weld-war-example.

Thanks, will check out.

I have managed to get the Jetty tests to run and most of them to pass on this branch https://github.com/justin-tay/core/tree/test_jetty12_master.

Why do you need to add CdiDecoratingListenerWebAppContextProcessor? I can see that it sets two init params, but no such config was previously needed. Or is it something that was needed but was instead done on the Arq. container level?

It looks like the changes to the JettyContainer might not be needed after all.

I see, I will wait for resolution of the linked PR, thanks for the heads-up.

[justin-tay]

Why do you need to add CdiDecoratingListenerWebAppContextProcessor? I can see that it sets two init params, but no such config was previously needed. Or is it something that was needed but was instead done on the Arq. container level?

The params is to use the mode ee10-cdi-decorate. I see from the Jetty PR that they also need to set the ordering attribute as this is running embedded so the order would be random. I think if deployed on the WAR the container initializers will be ordered first if there is no explicit order.

I'm not sure which mode the tests were previously using however I'm guessing that previously it was the JettyLegacyContainer that used the LegacyWeldDecorator that was doing the decorating. I think this would now fail because the class org.eclipse.jetty.servlet.ServletContextHandler is now org.eclipse.jetty.ee10.servlet.ServletContextHandler.

core/environments/servlet/core/src/main/java/org/jboss/weld/environment/jetty/LegacyWeldDecorator.java

Line 23 in 5e47006

import org.eclipse.jetty.servlet.ServletContextHandler;

The other possibility is if the org.eclipse.jetty.ee10.webapp.DecoratingListener was registered, but I don't really see anywhere that would automatically register it.

[janbartel]

Any of the Jetty *Legacy* classes reference classes that are no longer in jetty-12. I'm still working on Jetty PR jetty/jetty.project#10359, but hope to have that ready for review in the next day or so.

[manovotn]

Closing this PR as the proposed changes are no longer needed thanks to linked Jetty PR.
Feel free to reopen/comment if there is still some work to be done in this regard.