Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jakarta servlet version 5 #639

Merged
merged 8 commits into from
Mar 22, 2023
Merged

Jakarta servlet version 5 #639

merged 8 commits into from
Mar 22, 2023

Conversation

gkresic
Copy link
Contributor

@gkresic gkresic commented Feb 21, 2023

WIP.

Due to the changes in the way Jetty handles WebSockets, I need an access to Jetty Server instance from within JettyWebSocketFilter.init

What would be the best way to access it from there? Provided FilterConfig does not hold any reference to Server.

Currently, I've hardcoded server access in ro.pippo.jetty.websocket.JettyWebSocketFilter: https://github.com/gkresic/pippo/blob/jakarta/pippo-server-parent/pippo-jetty/src/main/java/ro/pippo/jetty/websocket/JettyWebSocketFilter.java

jakarta.servlet namespace!
Pippo is not designed to be restarted (PippoFilter destroys Application instance).
@gkresic gkresic mentioned this pull request Feb 21, 2023
/*
* Alternative is JettyWebSocketServletContainerInitializer.configure
*/
WebSocketComponents components = WebSocketServerComponents.ensureWebSocketComponents(JettyServer._get_DONT_USE(), filterConfig.getServletContext());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we need somehow to add support for WebSocket in JettyServer not in JettyWebSocketFilter.
In https://github.com/pippo-java/pippo/blob/master/pippo-server-parent/pippo-jetty/src/main/java/ro/pippo/jetty/JettyServer.java#L223 we know if the Jetty is configured with webSocket support or not (a jetty websocket class is present in classpath or not).
If the user doesn't add jetty websocket as dependency then we don't need to force the part with websocket. In many cases I use jetty without websocket.

Copy link
Contributor Author

@gkresic gkresic Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After taking a second look at this, I figured out that cleanest approach would be to pass Jetty's Server instance as constructor parameter to JettyWebSocketFilter.

Reasons:

  • Adding any reference to WebSocket classes to JettyServer would make it dependent to websocket-jetty-server stuff. Instead, turn dependencies other way around and make JettyWebSocketFilter dependent on Jetty's Server which is already implicitly the case.
  • Even if we accept depending JettyServer on WebSocket classes, JettyWebSocketFilter internally needs instances of JettyWebSocketServerContainer and JettyWebSocketCreator, so WebSocket initialization can not be performed solely in JettyServer and something would have to be passed to JettyWebSocketFilter anyway. By keeping dynamic instantiation of JettyWebSocketFilter in JettyServer and passing only Jetty's Server instance to it, we keep JettyServer free of WebSocket dependencies and isolate them to JettyWebSocketFilter class.

On a side note, I'm not a Maven expert, but shouldn't websocket-jetty-server artifact in pippo-jetty be marked as optional?

pippo-server-parent/pippo-jetty/pom.xml Show resolved Hide resolved
@@ -33,11 +35,23 @@
<scope>compile</scope>
</dependency>

<dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this dependency in pippo-test module. This module is used also by users that go with other web server (tomcat, undertow)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dependency can be moved in pippo-jetty module and added test scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As previously discussed, instead of moving WebSocket test client to another project, I've extracted WebSocket-related test infrastructure from PippoExtension to PippoWebSocketExtension and made websocket-jetty-client dependency optional.

pippo-test/pom.xml Outdated Show resolved Hide resolved

private final Pippo pippo;
private final WebSocketClient webSocketClient;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add this instance in Jetty related test classes.
I prefer to not touch this class because as I mentioned is a core class used by developers to test pippo applications.
I think that start & stop pippo (server, ..) for each test (the existed behavior) is fine because this process is very fast. But if we decide that is not OK and we need to start & stop pippo before all tests and after all tests, we will address this problem in a future PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things are relevant here:

WebSocketClient used here does belong to Jetty project, but it's unrelated to Jetty server that is an optional Pippo component. It is intended to test WebSockets on any server implementation (although I wrote tests only for JettyServer). I could as well use any other WebSocket client lib, but since I was working with Jetty server-side WebSockets API, this one came handy. If I move it to Jetty related tests, this functionality will need to be duplicated for Tomcat and Undertow related tests, too. With this in mind, do you still want me to move it somewhere else?

Secondly, I changed starting/stopping Pippo before/after all tests simply because Pippo doesn't support restarting it once stop() method is called: PippoFilter destroys provided application (and clears internal field to null):

https://github.com/pippo-java/pippo/blob/master/pippo-core/src/main/java/ro/pippo/core/PippoFilter.java#L165

So, next time you try to call Pippo.start() on stopped instance, you will get an exception that application is not set. I discovered this when my tests started to fail with that message after I merged changes from master that replaced JUnit 4 with 5. If I commented any test, other one would pass, but two tests can not be run on same instance of PippoExtension. Note that JUnit 4 tests also did start/stop Pippo before/after all tests, so this change basically keeps backward compatibility.

I agree that instances of Pippo should be restartable, though. But I assume that change will be better suited for a dedicated PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this in mind, do you still want me to move it somewhere else?

Thanks for the clarification! Now it's more clear for me and I think that we can keep websocket-jetty-client as dependency in pippo-test module. Maybe it's better to add a new PippoWebSocketExtension that extends PippoExtension and override some methods to inject part with create, start and stop websocket client.
Personal, I use WebSocketClient from https://github.com/TooTallNate/Java-WebSocket but I don't see a problem to use the Jetty implementation. At the moment use the library you are comfortable with.

So, next time you try to call Pippo.start() on stopped instance, you will get an exception that application is not set.

I will investigate this issue, probably it's a regression.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will investigate this issue, probably it's a regression.

I cannot replicate the problem with what is available in master branch
image

Copy link
Contributor Author

@gkresic gkresic Feb 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may depend on type of server you are using.

I modified only test in master that uses PippoExtension:

https://github.com/pippo-java/pippo/blob/master/pippo-controller-parent/pippo-weld/src/test/java/ro/pippo/weld/ControllerInterceptorTest.java

and added copy of only existing test in that class:

@Test
public void testControllerWithInterceptorDuplicate() {
    when().get("/").then().statusCode(200);
}

I got it to fail with two different types of exceptions:

  1. if run as configured, using pippo-undertow, it fails with:
velj 27, 2023 11:23:44 AM org.jboss.weld.environment.deployment.discovery.ReflectionDiscoveryStrategy processAnnotatedDiscovery
INFO: WELD-ENV-000014: Falling back to Java Reflection for bean-discovery-mode="annotated" discovery. Add org.jboss:jandex to the classpath to speed-up startup.
velj 27, 2023 11:23:44 AM org.jboss.weld.bootstrap.WeldStartup startContainer
INFO: WELD-000101: Transactional services not available. Injection of @Inject UserTransaction not available. Transactional observers will be invoked synchronously.
velj 27, 2023 11:23:44 AM io.undertow.servlet.api.LoggingExceptionHandler handleThrowable
ERROR: UT005023: Exception handling request to /
javax.servlet.ServletException: java.lang.IllegalStateException: WELD-ENV-002000: Weld SE container STATIC_INSTANCE is already running!
	at ro.pippo.core.PippoFilter.init(PippoFilter.java:115)
	at ro.pippo.undertow.websocket.UndertowWebSocketFilter.init(UndertowWebSocketFilter.java:51)
	at io.undertow.servlet.core.LifecyleInterceptorInvocation.proceed(LifecyleInterceptorInvocation.java:111)
	at io.undertow.servlet.core.ManagedFilter.createFilter(ManagedFilter.java:80)
	at io.undertow.servlet.core.ManagedFilter.forceInit(ManagedFilter.java:125)
	at io.undertow.servlet.handlers.ServletChain.forceInit(ServletChain.java:113)
	at io.undertow.servlet.handlers.ServletChain$1.handleRequest(ServletChain.java:60)
	at io.undertow.servlet.handlers.ServletDispatchingHandler.handleRequest(ServletDispatchingHandler.java:36)
	at io.undertow.servlet.handlers.security.SSLInformationAssociationHandler.handleRequest(SSLInformationAssociationHandler.java:132)
	at io.undertow.servlet.handlers.security.ServletAuthenticationCallHandler.handleRequest(ServletAuthenticationCallHandler.java:57)
	at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
	at io.undertow.security.handlers.AbstractConfidentialityHandler.handleRequest(AbstractConfidentialityHandler.java:46)
	at io.undertow.servlet.handlers.security.ServletConfidentialityConstraintHandler.handleRequest(ServletConfidentialityConstraintHandler.java:64)
	at io.undertow.security.handlers.AuthenticationMechanismsHandler.handleRequest(AuthenticationMechanismsHandler.java:60)
	at io.undertow.servlet.handlers.security.CachedAuthenticatedSessionHandler.handleRequest(CachedAuthenticatedSessionHandler.java:77)
	at io.undertow.security.handlers.AbstractSecurityContextAssociationHandler.handleRequest(AbstractSecurityContextAssociationHandler.java:43)
	at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
	at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
	at io.undertow.servlet.handlers.ServletInitialHandler.handleFirstRequest(ServletInitialHandler.java:292)
	at io.undertow.servlet.handlers.ServletInitialHandler.access$100(ServletInitialHandler.java:81)
	at io.undertow.servlet.handlers.ServletInitialHandler$2.call(ServletInitialHandler.java:138)
	at io.undertow.servlet.handlers.ServletInitialHandler$2.call(ServletInitialHandler.java:135)
	at io.undertow.servlet.core.ServletRequestContextThreadSetupAction$1.call(ServletRequestContextThreadSetupAction.java:48)
	at io.undertow.servlet.core.ContextClassLoaderSetupAction$1.call(ContextClassLoaderSetupAction.java:43)
	at io.undertow.servlet.handlers.ServletInitialHandler.dispatchRequest(ServletInitialHandler.java:272)
	at io.undertow.servlet.handlers.ServletInitialHandler.access$000(ServletInitialHandler.java:81)
	at io.undertow.servlet.handlers.ServletInitialHandler$1.handleRequest(ServletInitialHandler.java:104)
	at io.undertow.server.Connectors.executeRootHandler(Connectors.java:336)
	at io.undertow.server.HttpServerExchange$1.run(HttpServerExchange.java:830)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: java.lang.IllegalStateException: WELD-ENV-002000: Weld SE container STATIC_INSTANCE is already running!
	at org.jboss.weld.environment.se.WeldContainer.initialize(WeldContainer.java:138)
	at org.jboss.weld.environment.se.Weld.initialize(Weld.java:562)
	at ro.pippo.weld.ControllerInterceptorTest$PippoApplication.onInit(ControllerInterceptorTest.java:98)
	at ro.pippo.core.Application.init(Application.java:118)
	at ro.pippo.core.route.RouteDispatcher.init(RouteDispatcher.java:71)
	at ro.pippo.core.PippoFilter.init(PippoFilter.java:108)
	... 31 more


java.lang.AssertionError: 1 expectation failed.
Expected status code <200> but was <500>.
  1. if I change server (in pom.xml) to pipo-jetty I get an exception I was mentioning:
/usr/lib/jvm/java-11-openjdk-amd64/bin/java -agentlib:jdwp=transport=dt_socket,address=127.0.0.1:37431,suspend=y,server=n -ea -Didea.test.cyclic.buffer.size=10240000 -javaagent:/home/gkresic/.local/share/JetBrains/Toolbox/apps/IDEA-C/ch-0/223.8617.56/plugins/Groovy/lib/agent/gragent.jar -javaagent:/home/gkresic/.local/share/JetBrains/Toolbox/apps/IDEA-C/ch-0/223.8617.56/plugins/java/lib/rt/debugger-agent.jar -Dfile.encoding=UTF-8 -classpath /home/gkresic/.m2/repository/org/junit/platform/junit-platform-launcher/1.9.2/junit-platform-launcher-1.9.2.jar:/home/gkresic/.local/share/JetBrains/Toolbox/apps/IDEA-C/ch-0/223.8617.56/lib/idea_rt.jar:/home/gkresic/.local/share/JetBrains/Toolbox/apps/IDEA-C/ch-0/223.8617.56/plugins/junit/lib/junit5-rt.jar:/home/gkresic/.local/share/JetBrains/Toolbox/apps/IDEA-C/ch-0/223.8617.56/plugins/junit/lib/junit-rt.jar:/home/gkresic/Sources/pippo/pippo-controller-parent/pippo-weld/target/test-classes:/home/gkresic/Sources/pippo/pippo-controller-parent/pippo-weld/target/classes:/home/gkresic/Sources/pippo/pippo-controller-parent/pippo-controller/target/classes:/home/gkresic/Sources/pippo/pippo-core/target/classes:/home/gkresic/.m2/repository/org/slf4j/slf4j-api/1.7.25/slf4j-api-1.7.25.jar:/home/gkresic/.m2/repository/javax/xml/bind/jaxb-api/2.3.1/jaxb-api-2.3.1.jar:/home/gkresic/.m2/repository/javax/activation/javax.activation-api/1.2.0/javax.activation-api-1.2.0.jar:/home/gkresic/.m2/repository/org/jboss/weld/se/weld-se/2.3.1.Final/weld-se-2.3.1.Final.jar:/home/gkresic/Sources/pippo/pippo-test/target/classes:/home/gkresic/.m2/repository/io/rest-assured/rest-assured/5.3.0/rest-assured-5.3.0.jar:/home/gkresic/.m2/repository/org/apache/groovy/groovy/4.0.6/groovy-4.0.6.jar:/home/gkresic/.m2/repository/org/apache/groovy/groovy-xml/4.0.6/groovy-xml-4.0.6.jar:/home/gkresic/.m2/repository/org/apache/httpcomponents/httpclient/4.5.13/httpclient-4.5.13.jar:/home/gkresic/.m2/repository/org/apache/httpcomponents/httpcore/4.4.13/httpcore-4.4.13.jar:/home/gkresic/.m2/repository/commons-logging/commons-logging/1.2/commons-logging-1.2.jar:/home/gkresic/.m2/repository/commons-codec/commons-codec/1.11/commons-codec-1.11.jar:/home/gkresic/.m2/repository/org/apache/httpcomponents/httpmime/4.5.13/httpmime-4.5.13.jar:/home/gkresic/.m2/repository/org/hamcrest/hamcrest/2.1/hamcrest-2.1.jar:/home/gkresic/.m2/repository/org/ccil/cowan/tagsoup/tagsoup/1.2.1/tagsoup-1.2.1.jar:/home/gkresic/.m2/repository/io/rest-assured/json-path/5.3.0/json-path-5.3.0.jar:/home/gkresic/.m2/repository/org/apache/groovy/groovy-json/4.0.6/groovy-json-4.0.6.jar:/home/gkresic/.m2/repository/io/rest-assured/rest-assured-common/5.3.0/rest-assured-common-5.3.0.jar:/home/gkresic/.m2/repository/io/rest-assured/xml-path/5.3.0/xml-path-5.3.0.jar:/home/gkresic/.m2/repository/org/apache/commons/commons-lang3/3.11/commons-lang3-3.11.jar:/home/gkresic/.m2/repository/org/junit/jupiter/junit-jupiter-engine/5.9.2/junit-jupiter-engine-5.9.2.jar:/home/gkresic/.m2/repository/org/junit/platform/junit-platform-engine/1.9.2/junit-platform-engine-1.9.2.jar:/home/gkresic/.m2/repository/org/opentest4j/opentest4j/1.2.0/opentest4j-1.2.0.jar:/home/gkresic/.m2/repository/org/junit/platform/junit-platform-commons/1.9.2/junit-platform-commons-1.9.2.jar:/home/gkresic/.m2/repository/org/junit/jupiter/junit-jupiter-api/5.9.2/junit-jupiter-api-5.9.2.jar:/home/gkresic/.m2/repository/org/apiguardian/apiguardian-api/1.1.2/apiguardian-api-1.1.2.jar:/home/gkresic/Sources/pippo/pippo-server-parent/pippo-jetty/target/classes:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-server/9.4.44.v20210927/jetty-server-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/javax/servlet/javax.servlet-api/3.1.0/javax.servlet-api-3.1.0.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-http/9.4.44.v20210927/jetty-http-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-util/9.4.44.v20210927/jetty-util-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-io/9.4.44.v20210927/jetty-io-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-annotations/9.4.44.v20210927/jetty-annotations-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-plus/9.4.44.v20210927/jetty-plus-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-jndi/9.4.44.v20210927/jetty-jndi-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-webapp/9.4.44.v20210927/jetty-webapp-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-xml/9.4.44.v20210927/jetty-xml-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/javax/annotation/javax.annotation-api/1.3.2/javax.annotation-api-1.3.2.jar:/home/gkresic/.m2/repository/org/ow2/asm/asm/9.2/asm-9.2.jar:/home/gkresic/.m2/repository/org/ow2/asm/asm-commons/9.2/asm-commons-9.2.jar:/home/gkresic/.m2/repository/org/ow2/asm/asm-tree/9.2/asm-tree-9.2.jar:/home/gkresic/.m2/repository/org/ow2/asm/asm-analysis/9.2/asm-analysis-9.2.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/websocket/websocket-server/9.4.44.v20210927/websocket-server-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/websocket/websocket-common/9.4.44.v20210927/websocket-common-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/websocket/websocket-api/9.4.44.v20210927/websocket-api-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/websocket/websocket-client/9.4.44.v20210927/websocket-client-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-client/9.4.44.v20210927/jetty-client-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/websocket/websocket-servlet/9.4.44.v20210927/websocket-servlet-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-servlet/9.4.44.v20210927/jetty-servlet-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-security/9.4.44.v20210927/jetty-security-9.4.44.v20210927.jar:/home/gkresic/.m2/repository/org/eclipse/jetty/jetty-util-ajax/9.4.44.v20210927/jetty-util-ajax-9.4.44.v20210927.jar com.intellij.rt.junit.JUnitStarter -ideVersion5 -junit5 @w@/tmp/idea_working_dirs_junit.tmp @/tmp/idea_junit.tmp -socket45897
Connected to the target VM, address: '127.0.0.1:37431', transport: 'socket'
velj 27, 2023 11:25:58 AM org.jboss.weld.bootstrap.WeldStartup <clinit>
INFO: WELD-000900: 2.3.1 (Final)
velj 27, 2023 11:25:58 AM org.jboss.weld.environment.deployment.discovery.ReflectionDiscoveryStrategy processAnnotatedDiscovery
INFO: WELD-ENV-000014: Falling back to Java Reflection for bean-discovery-mode="annotated" discovery. Add org.jboss:jandex to the classpath to speed-up startup.
velj 27, 2023 11:25:58 AM org.jboss.weld.bootstrap.WeldStartup startContainer
INFO: WELD-000101: Transactional services not available. Injection of @Inject UserTransaction not available. Transactional observers will be invoked synchronously.
velj 27, 2023 11:25:58 AM org.jboss.weld.interceptor.util.InterceptionTypeRegistry <clinit>
WARN: WELD-001700: Interceptor annotation class javax.ejb.PostActivate not found, interception based on it is not enabled
velj 27, 2023 11:25:58 AM org.jboss.weld.interceptor.util.InterceptionTypeRegistry <clinit>
WARN: WELD-001700: Interceptor annotation class javax.ejb.PrePassivate not found, interception based on it is not enabled
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.jboss.classfilewriter.ClassFile$1 (file:/home/gkresic/.m2/repository/org/jboss/weld/se/weld-se/2.3.1.Final/weld-se-2.3.1.Final.jar) to method java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int)
WARNING: Please consider reporting this to the maintainers of org.jboss.classfilewriter.ClassFile$1
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
velj 27, 2023 11:25:59 AM org.jboss.weld.environment.se.WeldContainer initialize
INFO: WELD-ENV-002003: Weld SE container STATIC_INSTANCE initialized

ro.pippo.core.PippoRuntimeException: javax.servlet.ServletException: Cannot found application class name

	at ro.pippo.jetty.JettyServer.start(JettyServer.java:84)
	at ro.pippo.core.Pippo.start(Pippo.java:158)
	at ro.pippo.test.PippoExtension.startPippo(PippoExtension.java:73)
	at ro.pippo.test.PippoExtension.beforeEach(PippoExtension.java:83)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeBeforeEachCallbacks$2(TestMethodTestDescriptor.java:166)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeBeforeMethodsOrCallbacksUntilExceptionOccurs$6(TestMethodTestDescriptor.java:202)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeBeforeMethodsOrCallbacksUntilExceptionOccurs(TestMethodTestDescriptor.java:202)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeBeforeEachCallbacks(TestMethodTestDescriptor.java:165)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:132)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:68)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:147)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:127)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:90)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:55)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:102)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
	at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
	at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:53)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:57)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
	at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)
Caused by: javax.servlet.ServletException: Cannot found application class name
	at ro.pippo.core.PippoFilter.createApplication(PippoFilter.java:263)
	at ro.pippo.core.PippoFilter.init(PippoFilter.java:77)
	at ro.pippo.jetty.websocket.JettyWebSocketFilter.init(JettyWebSocketFilter.java:43)
	at org.eclipse.jetty.servlet.FilterHolder.initialize(FilterHolder.java:140)
	at org.eclipse.jetty.servlet.ServletHandler.lambda$initialize$0(ServletHandler.java:731)
	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
	at java.base/java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:734)
	at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:658)
	at org.eclipse.jetty.servlet.ServletHandler.initialize(ServletHandler.java:755)
	at org.eclipse.jetty.servlet.ServletContextHandler.startContext(ServletContextHandler.java:379)
	at org.eclipse.jetty.server.handler.ContextHandler.doStart(ContextHandler.java:910)
	at org.eclipse.jetty.servlet.ServletContextHandler.doStart(ServletContextHandler.java:288)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:73)
	at org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:169)
	at org.eclipse.jetty.server.Server.start(Server.java:423)
	at org.eclipse.jetty.util.component.ContainerLifeCycle.doStart(ContainerLifeCycle.java:110)
	at org.eclipse.jetty.server.handler.AbstractHandler.doStart(AbstractHandler.java:97)
	at org.eclipse.jetty.server.Server.doStart(Server.java:387)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:73)
	at ro.pippo.jetty.JettyServer.start(JettyServer.java:81)
	... 57 more

In both cases either test succeeds iff it is the only test in a class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My test is based on Jetty server. I will try again with other servers, too.
In the end I don't want to block the entire process, we can go with one server start/stop life-cycle, for the entire test class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My test is based on Jetty server. I will try again with other servers, too.

Filed #641 and provided #642 for reproducing it. Interesting thing is that all four new tests behave differently - one even works :)

In the end I don't want to block the entire process, we can go with one server start/stop life-cycle, for the entire test class.

Don't worry, this is not blocked by you, but by my duties this week. Will continue tomorrow.

Remove empty lines, move field declarations before methods.
JettyWebSocketFilter needs own instance of JettyWebSocketServerContainer and JettyWebSocketCreator,
so do all WebSocket-related initialization there and do not pollute JettyServer with WebSocket dependencies.
Extract WebSocket-related test infrastructure from PippoExtension to PippoWebSocketExtension
and make websocket-jetty-client dependency optional.
@decebals
Copy link
Member

@gkresic If I can help on this topic please let me know.

@decebals decebals marked this pull request as draft March 20, 2023 20:59
@gkresic
Copy link
Contributor Author

gkresic commented Mar 21, 2023

@gkresic If I can help on this topic please let me know.

Well, I committed all the changes we talked about, to the best of my abilities.

On topics where I didn't implement changes exactly as you requested, I left conversations opened (unresolved), so you can verify latest commit and comment if something should be further improved.

@decebals decebals marked this pull request as ready for review March 22, 2023 18:40
@decebals
Copy link
Member

There is clearly a lack of communication between us. I thought it was in draft and I was waiting for you to request a review.

@decebals decebals merged commit 6bb5d3e into pippo-java:master Mar 22, 2023
@gkresic gkresic deleted the jakarta branch March 22, 2023 19:24
@gkresic
Copy link
Contributor Author

gkresic commented Mar 22, 2023

I thought it was in draft and I was waiting for you to request a review.

Well, it seems that my GitHub-fu is weak: I didn't know I should re-request a review after I modified PR (commit something more into it). For anyone else reading this: https://github.blog/changelog/2019-02-21-re-request-review-on-a-pull-request/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants