Enabling "Push" functionality breaks annotated websocket servers

[kevinross]

The solution in #69 is intended to trigger this line so that Quarkus knows that websockets are used. Unfortunately, this conditional forces the builder to ignore any annotated server classes that are not also referenced by a config class (such as, but not limited to, the EnableWebsockets one). Therefore, if you have an annotated server class without also including a config class for it, quarkus drops your server from the build.

I can't do this myself here, but I've found a few breadcrumbs that might help with fixing it:

• here is where server annotated endpoints are discovered
• here is where those are used
• it should be sufficient to produce an annotated endpoint builditem pointing to the atmosphere handler, to get the same result as EnableWebsockets

[mcollovati]

Atmoshpere endpoint is handled internally by Atmoshpere, so adding a build item may not be the best way. Perhpase we perform a scan for ServerApplicationConfig implementor and ServerEndpoint annotated classes, and somehow emit EnableWebsockets olnly if none exists.

[kevinross]

Atmoshpere endpoint is handled internally by Atmoshpere

This bug report is external to this, ie the internal details of how the endpoint works isn't at issue here. What is at issue is how atmosphere's "external interface" (ie the part that other servlet containers (eg tomcat) scan for on boot) is registered into quarkus.

As described in #69, EnableWebsockets was added as a "marker interface" of sorts, to trigger quarkus to include the websocket serving machinery. and it does this by including a ServerApplicationConfig. This forces quarkus to exclude annotation-based registrations entirely (if you use ServerApplicationConfig instead of annotations, you're okay).

I don't think scanning for implementors and annotated classes is "good enough", simply because there's a thousand permutations you could have and you'd be playing a game of whack-a-mole trying to avoid stepping on other library/framework/etc's toes.

Instead, use quarkus' build items. It's not implementing websockets by using them, it's just quarkus' version of a marker interface for registration. Include the right build item -> you get the websocket serving machinery included, and without closing off annotation-based configs. A different road to the same destination, if you will.

[mcollovati]

Atmosphere JSR356Endpoint is not annotated and is registered programmatically by JSR356AsyncSupport, which in turn requires ServerContainer to be present in ServletContext.

If I try to produce an AnnotatedWebsocketEndpointBuildItem for JSR356Endpoint I get an error during startup

Caused by: javax.websocket.DeploymentException: UT003027: Class class org.atmosphere.container.JSR356Endpoint was not annotated with @ClientEndpoint or @ServerEndpoint
    at io.undertow.websockets.ServerWebSocketContainer.addEndpointInternal(ServerWebSocketContainer.java:650)
    at io.undertow.websockets.ServerWebSocketContainer.addEndpoint(ServerWebSocketContainer.java:550)
    at io.quarkus.websockets.client.runtime.WebsocketCoreRecorder.createServerContainer(WebsocketCoreRecorder.java:210)
    at io.quarkus.deployment.steps.WebsocketClientProcessor$deploy1070688649.deploy_0(Unknown Source)
    at io.quarkus.deployment.steps.WebsocketClientProcessor$deploy1070688649.deploy(Unknown Source)

[kevinross]

Is it possible to wrap the JSR356Endpoint class in an annotated impl and use the same builditem for that one?

[kevinross]

One potential idea that might solve the problem, although it's a bit of a hack:

@BuildStep
void patchAnnotatedEndpointsIntoEnableWebsocketsClass(
  List<AnnotatedWebsocketEndpointBuildItem> websocketEndpoints,
  BuildProducer<BytecodeTransformerBuildItem> bytecodeProducer) {
    // add a method body to getAnnotatedEndpointClasses with the class names in websocketEndpoints
}

The idea being that we have quarkus discover all annotated endpoints for us, then write that into the class bytecode directly. I call it "a hack" because it should really be handled entirely by quarkus, and vaadin probably should have an annotated wrapper as I mentioned above. This way (annotated wrapper), if quarkus does anything else with websockets (other than gathering and registering endpoints), vaadin isn't stepping on any toes that it might be by doing this hack.

[mcollovati]

@kevinross thanks for the suggestions. I'm trying a solution similar to the proposed one, but currently getting a ChainBuildException: Cycle detected error, so further investigation is needed. For the PUSH endpoint, we need to let Atmosphere handle it because communication is configurable by developer and can even be set to use HTTP long-polling instead of web sockets

[kevinross]

If it's (push vs long polling) something that is designed to be configured exclusively at build time, then you could use the jandex index that quarkus builds to find out what the config is (assuming the only way to do that config is via annotations).

[mcollovati]

Transport can also be changed at runtime

[kevinross]

That kinda breaks the assumptions that quarkus has with respect to compile-time vs runtime config. It's way outside my call to make, but if it were my call, I'd settle on making that knob a compile-time exclusive one for quarkus (ie if you need to switch transports back and forth "on a whim" (for lack of a better phrase) at runtime, quarkus may not be the framework for you).

[mcollovati]

@kevinross the solution may be easier than expected Maybe making EnableWebsocket.getAnnotatedEndpointClasses(Set<Class<?>> scanned) return the input value instead of null would make the trick. Not sure, need to test it

[kevinross]

Actually... Yeah. I think that's not just the solution, but also the intended solution as well. Although the spec/javadocs say that it would be given the list of implementations in the current jar, and return a (possibly modified) list from there, so it wouldn't work for this because $jarA != $jarB, so EnableWebsockets would never get anything other than annotated endpoints in vaadin-quarkus-$ver.jar... but quarkus implements it such that all annotated endpoints are included in the set passed to getAnnotatedEndpointClasses, so it should work for our purposes.