`Annotated*Client` annotated with `ClientEndpoint` should not be included in the deployment

[jamezp]

Challenged tests

• com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.client.WSCClientIT

TCK Version
Jakarta WebSocket 2.2.0

Description
The com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.client.WSCClientIT includes 3 client classes that are included in the deployment because they're in the com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.client package. These 3 types should not be included as they're not needed in the deployment and only used on the client side.

• com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.client.AnnotatedBinaryClient
• com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.client.AnnotatedTextClient
• com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.client.AnnotatedThrowingClient

The com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.client.AnnotatedThrowingClient in the deployment which does not have a no-arg constructor. This causes the deployment to fail and thus the tests do not run. Per the specification:

The class must have a public no-args constructor,

The other 2 have requirements on the following two types which are not included in the deploy:

• com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.common.AlternativeInputStreamDecoder.class
• com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.common.AlternativeReaderDecoder.class

Note I've updated the description here as I thought we needed to add the above two types to the deployment, see #579. However, it makes more sense to not include the clients in the deployment.

Additional context
The workaround is to remove the types from the deployment.

[markt-asf]

The no-args constructor requirement is necessary if WebSocketContainer.connectToServer(Class<?>, URI) is used.

It isn't necessary (as in the implementation doesn't need to use the no-arg constructor) if WebSocketContainer.connectToServer(Object, URI) is used since the caller has already constructed the object.

The question here is whether the no-arg constructor requirement was intended to apply to the second case.

The second case was added fairly late in the development of the original WebSocket specification. This specific scenario wasn't considered (that I can find a record of) but my sense of reading what I could find was that the no-arg constructor requirement was not intended to apply in this case.

I think we have two options.

1. Work on the basis the no-arg constructor requirement always applies, exclude this test for previously released TCK versions, fix the test (just added a no-arg constructor that throws an ISE would work) and potentially re-release the 2.2.x TCK.

2. Work on the basis that the no-arg constructor requirement only applies if the class is passed in a call to WebSocketContainer.connectToServer(Class<?>, URI) and update the specification text (section 4.2) accordingly.

I'm leaning towards option 2 on the grounds that there is no need to require a no-arg constructor in this case, there is no benefit to requiring a no-arg constructor on this case and we have a number of implementations (since they pass the TCK and have done since WebSocket 1.0 over a decade ago) that don't require a no-arg constructor in this case. The later point meaning there is a chance we would break existing code if we forced those implementations to require a no-arg constructor.

Either way, I think that means you can exclude this test when running the TCK if you wish.

[jamezp]

For the TCK test simply removing it from the deployment is enough as it's only used on the client side of the test. This is what I'm doing for WildFly now with an Arquillian Extension.

As for which option is better, I don't feel I know the specification well enough to comment. From a quick look at Undertow it looks like it always requires a default constructor if a @ClientEntpoint annotation is found.

[brideck]

FWIW, from inspecting the built wsc_ee_jakarta_websocket_session11_client_web.war artifact from the EE 10 Platform TCK, I can see that it doesn't contain any of the Annotated*Client classes from this package. It would seem that the WebSocket 2.2 TCK is packaging more in this instance (the entire session11.client package) than used to be in the application, which would help explain how no implementations have run into this before now.

[jamezp]

Thank you @brideck. I've updated the title and summary to hopefully better explain that as you pointed out. It makes a lot more sense to simply remove the types from the deployment. Thanks for your help on this.

[markt-asf]

Thanks for the additional analysis. The point about requiring the no-arg constructor stands even if it doesn't apply in this case.

While there is no reason for the Annotated*Client classes to be included in the deployment, I am wondering why they are causing a problem. I don't see anything in the specification that requires a container to scan for @ClientEndpoint. The only time that annotation should be checked is if a class is passed to WebSocketContainer.connectToServer(Class<?>, URI) and that isn't happening here.

I am starting to think the challenge should be rejected (although I've no objection to cleaning up the deployment).

[joakime]

While there is no reason for the Annotated*Client classes to be included in the deployment, I am wondering why they are causing a problem. I don't see anything in the specification that requires a container to scan for @ClientEndpoint. The only time that annotation should be checked is if a class is passed to WebSocketContainer.connectToServer(Class<?>, URI) and that isn't happening here.

Good thought. There's no bytecode scanning requirement for clientside.
For server side, the jakarta websocket implementations should have a ServletContainerInitializer with a ...

@HandlesTypes( { 
    jakarta.websocket.server.ServerApplicationConfig.class, 
    jakarta.websocket.server.ServerEndpoint.class, 
    jakarta.websocket.Endpoint.class } )

I guess, on server side, if you have a class like ...

@ClientEndpoint
public class MyEndpoint extends Endpoint {
    // only constructor (doesn't have a no-args constructor)
    public MyEndpoint(String mode) {
    }
}

That one would get caught up in the scanning on the server side.

[markt-asf]

I don't think it should. It will be picked up in the scan because it extends Endpoint but, unless there is a ServerApplicationConfig that references it, it should be ignored. I don't see a ServerApplicationConfig in this test so those classes should be ignored.

[jamezp]

The initial issue I saw, which is why I had filed #579 originally, was com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.common.TypeEnum was missing from the deployment so I ended up with a CNFE:

07:40:41,932 ERROR [org.jboss.msc.service.fail] (ServerService Thread Pool -- 79) MSC000001: Failed to start service jboss.deployment.unit."wsc_ee_jakarta_websocket_session11_client_web.war".undertow-deployment: org.jboss.msc.service.StartException in service jboss.deployment.unit."wsc_ee_jakarta_websocket_session11_client_web.war".undertow-deployment: java.lang.NoClassDefFoundError: com/sun/ts/tests/websocket/ee/jakarta/websocket/session11/common/TypeEnum
    at org.wildfly.extension.undertow@39.0.0.Beta1-SNAPSHOT//org.wildfly.extension.undertow.deployment.UndertowDeploymentService$1.run(UndertowDeploymentService.java:73)
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
    at org.jboss.threads@3.9.1//org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
    at org.jboss.threads@3.9.1//org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2651)
    at org.jboss.threads@3.9.1//org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2630)
    at org.jboss.threads@3.9.1//org.jboss.threads.EnhancedQueueExecutor.runThreadBody(EnhancedQueueExecutor.java:1622)
    at org.jboss.threads@3.9.1//org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1589)
    at java.base/java.lang.Thread.run(Thread.java:1583)
    at org.jboss.threads@3.9.1//org.jboss.threads.JBossThread.run(JBossThread.java:501)
Caused by: java.lang.NoClassDefFoundError: com/sun/ts/tests/websocket/ee/jakarta/websocket/session11/common/TypeEnum
    at java.base/java.lang.Class.getDeclaredConstructors0(Native Method)
    at java.base/java.lang.Class.privateGetDeclaredConstructors(Class.java:3551)
    at java.base/java.lang.Class.getConstructor0(Class.java:3756)
    at java.base/java.lang.Class.getDeclaredConstructor(Class.java:2932)
    at org.jboss.as.ee@39.0.0.Beta1-SNAPSHOT//org.jboss.as.ee.component.ReflectiveClassIntrospector.createFactory(ReflectiveClassIntrospector.java:62)
    at org.jboss.as.ee@39.0.0.Beta1-SNAPSHOT//org.jboss.as.ee.component.ComponentRegistry.createInstanceFactory(ComponentRegistry.java:64)
    at org.jboss.as.ee@39.0.0.Beta1-SNAPSHOT//org.jboss.as.ee.component.ComponentRegistry.createInstanceFactory(ComponentRegistry.java:57)
    at org.wildfly.extension.undertow@39.0.0.Beta1-SNAPSHOT//org.wildfly.extension.undertow.deployment.UndertowDeploymentInfoService$ComponentClassIntrospector.createInstanceFactory(UndertowDeploymentInfoService.java:1203)
    at io.undertow.websocket@2.0.0.Alpha2-SNAPSHOT//io.undertow.websockets.jsr.ServerWebSocketContainer.addEndpointInternal(ServerWebSocketContainer.java:708)
    at io.undertow.websocket@2.0.0.Alpha2-SNAPSHOT//io.undertow.websockets.jsr.ServerWebSocketContainer.addEndpoint(ServerWebSocketContainer.java:636)
    at io.undertow.websocket@2.0.0.Alpha2-SNAPSHOT//io.undertow.websockets.jsr.Bootstrap.handleDeployment(Bootstrap.java:87)
    at io.undertow.servlet@2.0.0.Alpha2-SNAPSHOT//io.undertow.servlet.core.DeploymentManagerImpl.handleExtensions(DeploymentManagerImpl.java:282)
    at io.undertow.servlet@2.0.0.Alpha2-SNAPSHOT//io.undertow.servlet.core.DeploymentManagerImpl.deploy(DeploymentManagerImpl.java:156)
    at org.wildfly.extension.undertow@39.0.0.Beta1-SNAPSHOT//org.wildfly.extension.undertow.deployment.UndertowDeploymentService.startContext(UndertowDeploymentService.java:88)
    at org.wildfly.extension.undertow@39.0.0.Beta1-SNAPSHOT//org.wildfly.extension.undertow.deployment.UndertowDeploymentService$1.run(UndertowDeploymentService.java:70)
    ... 9 more
Caused by: java.lang.ClassNotFoundException: com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.common.TypeEnum from [Module "deployment.wsc_ee_jakarta_websocket_session11_client_web.war" from Service Module Loader]
    at org.jboss.modules.ModuleClassLoader.findClass(ModuleClassLoader.java:201)
    at org.jboss.modules.ConcurrentClassLoader.performLoadClassUnchecked(ConcurrentClassLoader.java:408)
    at org.jboss.modules.ConcurrentClassLoader.performLoadClass(ConcurrentClassLoader.java:396)
    at org.jboss.modules.ConcurrentClassLoader.loadClass(ConcurrentClassLoader.java:115)
    ... 24 more

Adding the class to the deployment and I see a different issue.

07:42:33,564 ERROR [org.jboss.msc.service.fail] (ServerService Thread Pool -- 21) MSC000001: Failed to start service jboss.deployment.unit."wsc_ee_jakarta_websocket_session11_client_web.war".undertow-deployment: org.jboss.msc.service.StartException in service jboss.deployment.unit."wsc_ee_jakarta_websocket_session11_client_web.war".undertow-deployment: java.lang.RuntimeException: jakarta.websocket.DeploymentException: UT003024: Web socket deployment failed
    at org.wildfly.extension.undertow@39.0.0.Beta1-SNAPSHOT//org.wildfly.extension.undertow.deployment.UndertowDeploymentService$1.run(UndertowDeploymentService.java:73)
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
    at org.jboss.threads@3.9.1//org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
    at org.jboss.threads@3.9.1//org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2651)
    at org.jboss.threads@3.9.1//org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2630)
    at org.jboss.threads@3.9.1//org.jboss.threads.EnhancedQueueExecutor.runThreadBody(EnhancedQueueExecutor.java:1622)
    at org.jboss.threads@3.9.1//org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1589)
    at java.base/java.lang.Thread.run(Thread.java:1583)
    at org.jboss.threads@3.9.1//org.jboss.threads.JBossThread.run(JBossThread.java:501)
Caused by: java.lang.RuntimeException: jakarta.websocket.DeploymentException: UT003024: Web socket deployment failed
    at io.undertow.websocket@2.0.0.Alpha2-SNAPSHOT//io.undertow.websockets.jsr.Bootstrap.handleDeployment(Bootstrap.java:93)
    at io.undertow.servlet@2.0.0.Alpha2-SNAPSHOT//io.undertow.servlet.core.DeploymentManagerImpl.handleExtensions(DeploymentManagerImpl.java:282)
    at io.undertow.servlet@2.0.0.Alpha2-SNAPSHOT//io.undertow.servlet.core.DeploymentManagerImpl.deploy(DeploymentManagerImpl.java:156)
    at org.wildfly.extension.undertow@39.0.0.Beta1-SNAPSHOT//org.wildfly.extension.undertow.deployment.UndertowDeploymentService.startContext(UndertowDeploymentService.java:88)
    at org.wildfly.extension.undertow@39.0.0.Beta1-SNAPSHOT//org.wildfly.extension.undertow.deployment.UndertowDeploymentService$1.run(UndertowDeploymentService.java:70)
    ... 9 more
Caused by: jakarta.websocket.DeploymentException: UT003024: Web socket deployment failed
    at io.undertow.websocket@2.0.0.Alpha2-SNAPSHOT//io.undertow.websockets.jsr.ServerWebSocketContainer.addEndpointInternal(ServerWebSocketContainer.java:714)
    at io.undertow.websocket@2.0.0.Alpha2-SNAPSHOT//io.undertow.websockets.jsr.ServerWebSocketContainer.addEndpoint(ServerWebSocketContainer.java:636)
    at io.undertow.websocket@2.0.0.Alpha2-SNAPSHOT//io.undertow.websockets.jsr.Bootstrap.handleDeployment(Bootstrap.java:87)
    ... 13 more
Caused by: java.lang.RuntimeException: java.lang.NoSuchMethodException: com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.client.AnnotatedThrowingClient.<init>()
    at org.jboss.as.ee@39.0.0.Beta1-SNAPSHOT//org.jboss.as.ee.component.ReflectiveClassIntrospector.createFactory(ReflectiveClassIntrospector.java:64)
    at org.jboss.as.ee@39.0.0.Beta1-SNAPSHOT//org.jboss.as.ee.component.ComponentRegistry.createInstanceFactory(ComponentRegistry.java:64)
    at org.jboss.as.ee@39.0.0.Beta1-SNAPSHOT//org.jboss.as.ee.component.ComponentRegistry.createInstanceFactory(ComponentRegistry.java:57)
    at org.wildfly.extension.undertow@39.0.0.Beta1-SNAPSHOT//org.wildfly.extension.undertow.deployment.UndertowDeploymentInfoService$ComponentClassIntrospector.createInstanceFactory(UndertowDeploymentInfoService.java:1203)
    at io.undertow.websocket@2.0.0.Alpha2-SNAPSHOT//io.undertow.websockets.jsr.ServerWebSocketContainer.addEndpointInternal(ServerWebSocketContainer.java:708)
    ... 15 more
Caused by: java.lang.NoSuchMethodException: com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.client.AnnotatedThrowingClient.<init>()
    at java.base/java.lang.Class.getConstructor0(Class.java:3763)
    at java.base/java.lang.Class.getDeclaredConstructor(Class.java:2932)
    at org.jboss.as.ee@39.0.0.Beta1-SNAPSHOT//org.jboss.as.ee.component.ReflectiveClassIntrospector.createFactory(ReflectiveClassIntrospector.java:62)
    ... 19 more

It looks like WildFly eagerly looks for @ClientEndpoint types and then Undertow attempts to initialize them according to section 4.2 of the specification.. However, it's been like this since 2013 :)

Are you suggesting that https://jakarta.ee/specifications/websocket/2.2/jakarta-websocket-spec-2.2#application-deployment-on-web-containers means that we need to be lazily initializing the endpoints? Please excuse my ignorance here. I'm not familiar with the spec at all, but it appears I'll be learning :)

[markt-asf]

My reading of that language is that a container should (via a ServletContainerInitializer) find all:

• classes annotated with @ServerEndpoint
• classes that extent Endpoint (this is how Annotated*Client classes are included)
• classes that implement ServerApplicationConfig (there are none in this case)

At this point nothing should be instantiated by the container.

Since no ServerApplicationConfig instances were found only the annotated endpoints (those annotated @ServerEndpoint) with should be deployed.

The @ClientEndpoint annotations are irrelevant in this case.

It appears that the container is incorrectly trying to deploy at least one of the Annotated*Client endpoints. That is, by my reading, not specification compliant behaviour.

The test wasn't looking to identify this non-compliant behaviour. It appears to be an accident that the test uncovered it.

[jamezp]

Okay thank you. I've opened a conversation on [WildFly's Zulip channel](#wildfly-developers > WildFly Jakarta WebSocket Endpoint Initialization). Hopefully, I can get some feedback there.

In either case though we either need to remove the clients or add the com.sun.ts.tests.websocket.ee.jakarta.websocket.session11.common.TypeEnum to the deployment.

[markt-asf]

I don't think we do need to remove the clients. While there were not (as far I can tell) deliberately added to the deployment, their addition should have been a noop. That the addition was not a noop for an implementation has been useful in that it has identified an bug.

Given this I am intended to close this challenge as invalid and without making any changes to the deployment. I do think it is worth while adding a comment to the code summarising this discussion to explain why the clients have been included, why they should be a noop and that they are effectively testing to ensure an implementation does not incorrectly try to deploy them.