websocket icon indicating copy to clipboard operation
websocket copied to clipboard

Support providing endpoint from a container JAR

Open glassfishrobot opened this issue 10 years ago • 18 comments

For JSF 2.3 / Mojarra I'm currently in progress of adding a standard <f:websocket> tag. As Mojarra is to be designed as a container-provided JAR (not an user-provided JAR), the endpoint needs to be programmatically added in a ServletContextListener, but at that point, servletContext.getAttribute(ServerContainer.class.getName()) returns null when Tyrus is used (Payara/GlassFish). Upon inspection it turns out that the TyrusServletContainerInitializer#onStartup() immediately returns when there are no user-provided endpoints and doesn't register the ServerContainer anymore, causing it to not be placed in ServletContext anymore. See also https://java.net/jira/browse/TYRUS-420

I'm therefore making the following proposals to ensure that the websocket container can forcibly be initialized from the container on regardless of the user-provided endpoints.

1. Proposal #1: require eager initialization.

  • Always initialize container regardless of the onStartup(classes).

2. Proposal #2: check a context param.

  • Check if a context param something like "javax.websocket.ALWAYS_ENABLED" is set.
  • If so, initialize container regardless of the onStartup(classes).

3. Proposal #3: check a context attribute.

  • Check if servletContext.getAttribute(ServerContainer.class.getName()) returns null.
  • If so, check if ServerContainer.class.getName() is present in servletContext.getAttributeNames().
  • If so, initialize container regardless of the onStartup(classes).

glassfishrobot avatar Dec 21 '15 16:12 glassfishrobot

  • Issue Imported From: https://github.com/javaee/websocket-spec/issues/240
  • Original Issue Raised By:@glassfishrobot
  • Original Issue Assigned To: @pavelbucek

glassfishrobot avatar Feb 12 '18 08:02 glassfishrobot

@glassfishrobot Commented Reported by balusc

glassfishrobot avatar Dec 21 '15 16:12 glassfishrobot

@glassfishrobot Commented balusc said: Extension on proposal #2: Apart from a predefined context param, also check a predefined JNDI entry on same name. This enables the opportunity to set it programmatically.

glassfishrobot avatar Jan 19 '16 10:01 glassfishrobot

@glassfishrobot Commented @pavelbucek said: ServletContext param can be set programatically, that's how current "dynamic deploy" feature works:

https://github.com/tyrus-project/tyrus/blob/master/containers/servlet/src/main/java/org/glassfish/tyrus/servlet/TyrusServletContainerInitializer.java#L138

I don't see a reason for involving JNDI..

glassfishrobot avatar Jan 19 '16 12:01 glassfishrobot

@glassfishrobot Commented balusc said: That's a context attribute.

Context params are web.xml entries.

glassfishrobot avatar Jan 19 '16 12:01 glassfishrobot

@glassfishrobot Commented @pavelbucek said: Ok, then the #2 could be:

Check if a context attribute something like "javax.websocket.ALWAYS_ENABLED" is set.
If so, initialize container regardless of the onStartup(classes).

I mean.. if there already is the way how to pass a parameter/attribute within Servlet API, we should use it.

glassfishrobot avatar Jan 19 '16 12:01 glassfishrobot

@glassfishrobot Commented balusc said: That's #3 but then with another name/key. Still, this doesn't cover timing errors. It would still fail when WS container initializer runs before JSF one. Ordering is unspecified. A context param or JNDI entry is more robust. A JNDI entry can be set by CDI extension which in turn can run before container initializer.

glassfishrobot avatar Jan 19 '16 12:01 glassfishrobot

@glassfishrobot Commented arjan_t said:

A JNDI entry can be set by CDI extension which in turn can run before container initializer.

CDI extensions are very important here.

A context param can unfortunately not be set or read by a CDI extension. Manually inspecting of web.xml is not possible either, since it's not yet on the classpath when CDI extensions are processed. There are enhancement requests filed for this, but they don't seem to be going anywhere. See https://issues.jboss.org/browse/CDI-152 and SERVLET_SPEC-9

JNDI could be an option, but perhaps some kind of key in the application scope can work as well.

glassfishrobot avatar Jan 19 '16 12:01 glassfishrobot

@glassfishrobot Commented balusc said: Any progress? The new JSF <f:websocket> tag is as good as finished (https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1396), only in order to get it to run in Tyrus (GlassFish/Payara), a hack is required (create a dummy endpoint in WAR) whereas it works fine in all other containers.

In the meanwhile, I thought about another option, perhaps the best one after all: check for /META-INF/services/javax.websocket.Endpoint via service loader API.

glassfishrobot avatar Mar 07 '16 14:03 glassfishrobot

@glassfishrobot Commented @pavelbucek said: We cannot make any progress without doing Maintenance or Full spec release. Unfortunately, as far as I know, websocket spec is not planned to update in Java EE 8 just yet; as far as I know, websocket spec might do maintenance release if required by Servlet 4 / HTTP 2 integration, but since HTTP 2 / WebSocket relationship is not defined yet...

glassfishrobot avatar Mar 14 '16 15:03 glassfishrobot

@glassfishrobot Commented arjan_t said:

Unfortunately, as far as I know, websocket spec is not planned to update in Java EE 8 just yet;

I know it's probably not that easy in practice, but would it in any way be possible to start the discussions about planning that update? Thanks!

glassfishrobot avatar Mar 14 '16 23:03 glassfishrobot

@glassfishrobot Commented balusc said: Given that you also maintain Tyrus, could you otherwise improve it in Tyrus end? Requesting Tyrus users (GlassFish/Payara users) to manually add a hack in order to use JSF-native websocket support really goes overboard.

glassfishrobot avatar Mar 15 '16 08:03 glassfishrobot

@glassfishrobot Commented @pavelbucek said:

I know it's probably not that easy in practice, but would it in any way be possible to start the discussions about planning that update? Thanks!

I'm afraid that in this case, it's not up to me/WebSocket spec. You need to talk to JSF spec lead and he needs to convince Java EE architecture group that this update is required / blocking other spec.

Given that you also maintain Tyrus, could you otherwise improve it in Tyrus end? Requesting Tyrus users (GlassFish/Payara users) to manually add a hack in order to use JSF-native websocket support really goes overboard.

The general problem with "hacks" like this is that they are not that easy to remove. I very much prefer clean spec-compliant way. (also, just a Tyrus hack doesn't really solve the issue, since JSF doesn't want to depend on Tyrus – the dependency needs to be only on WebSocket spec).

glassfishrobot avatar Mar 15 '16 19:03 glassfishrobot

@glassfishrobot Commented arjan_t said:

You need to talk to JSF spec lead and he needs to convince Java EE architecture group that this update is required / blocking other spec.

Okay, so let's start with that then. Thanks!

Tyrus hack doesn't really solve the issue, since JSF doesn't want to depend on Tyrus – the dependency needs to be only on WebSocket spec

Principally true, of course. Mojarra though can depend, more or less, an another GlassFish project if I'm not mistaken. I'm fairly sure that the MVC RI (Ozark) depends on Jersey and does not work out of the box on say JBoss, which uses RestEasy.

The question is, are we talking about a "hack", or just a proprietary (product specific) way to do something? Hacks should of course be avoided, they indeed don't go away anymore. But a product specific way should not be that bad. There's quite an amount of precedents for that in Java EE. Something gets done in a product specific way first, then if that appears to work it's standardised in a later version of the spec.

From the JSF side we could maybe look into defining a small SPI for this where we abstract this activation. For GlassFish we then use the Tyrus specific way to activate this, while vendors wishing to combine Mojarra with another WebSocket implementation have to do it in their specific way.

Would that work for you?

glassfishrobot avatar Mar 15 '16 21:03 glassfishrobot

@glassfishrobot Commented This issue was imported from java.net JIRA WEBSOCKET_SPEC-240

glassfishrobot avatar Apr 24 '17 11:04 glassfishrobot

Currently section 6.2 of the spec says:

Java EE containers are not required to support deployment of websocket endpoints if they are not packaged in a WAR file as described above.

I think the simplest option is a version of option 1. Essentially, if the SCI is executed then the ServerContainer is always added to the ServletContext. The Servlet specification provides a mechanism for an application developer to disable an SCI the developer does not want to be load (8.2.2. b).

markt-asf avatar Apr 16 '20 19:04 markt-asf

This is the first time i've seen this particular issue.

Just thinking, wouldn't a container implementation of javax.websocket.server.ServerApplicationConfig be sufficient to have Mojarra auto-register it's endpoints?

joakime avatar Apr 16 '20 19:04 joakime

It depends on what the WebSocket implementation scans. Section 6.2 of the WebSocket spec allows the implementation to limit itself just to the current WAR. As does the Servlet spec when it comes to annotation scanning. However the Servlet spec does indicate that SCIs can be provided by the container. And, contrary to what I wrote above, there is no spec provided mechanism (there may be be a container specific one) to exclude container provided SCIs.

I'm tempted to still go with the variation I proposed above. If the SCI executes, the ServerContainer is added to the ServletContext. If that is a problem, containers can provide container specific mechanisms to exclude the SCI for some/all web applications. It seems like the simplest option.

markt-asf avatar Apr 21 '20 17:04 markt-asf

Fixing #211 in WebSocket 2.1 essentially requires option 1 as WebSocket endpoints may all be added after deployment has completed. I don;t think there is anything left for the WebSocket specification to do here.

markt-asf avatar Jun 29 '23 12:06 markt-asf