search

puniverse / comsat

Fibers and actors for web development
docs.paralleluniverse.co/comsat
Other
598 stars 103 forks source link

javax.websocket classes shouldn't be madatory to be in the classpath #81

Open victornoel opened 7 years ago

victornoel commented 7 years ago

This is a kind of spinoff of #74.

Basically, because WebActorInitializer refers to javax.websocket.server.ServerContainer and other classes from the javax.websocket package, when the JVM tries to load WebActorInitializer, it also tries to load the websocket ones, which must then be in the classpath even tough one (such as me :) would want them not to!

It seems to me that the simplest way to solve that (if we want to of course, an alternative would be to simply add the javax.websocket API only to the classpath) would be to externalize websocket related code in other classes (that wouldn't then be loaded until needed). I've made some tests and it works.

The problem is that it means then it becomes a bit complex to maintain and not reintroduce problems after… not sure how hard it is to make a test around that…

What is your opinion @pron ?

pron commented 7 years ago

What do you mean by "it becomes a bit complex to maintain and not reintroduce problems"? Can you be a bit more specific?

victornoel commented 7 years ago

@pron I means that if in 1 year, you, without noticing, reintroduce direct references to javax.websocket in WebActorInitializer or any other class where we would like to avoid it, then nothing will warn you.

pron commented 7 years ago

There's a tool called Macker, which I used a long time ago for this kind of thing. You may want to give it a try. But not everything needs to be automated. I assume you'll warn me.

I'd certainly consider a PR.

victornoel commented 7 years ago

Ok, I will make a PR very soon :)

victornoel commented 7 years ago

@pron the same question arises with the class co.paralleluniverse.comsat.webactors.servlet.ServletWebActors: it contains both websocket and non-websocket utilities.

I don't think we can simply extract the websocket-dependent method because it would break API, but what do you think of making them deprecated for now (and delegating to another class) and then removing them in the next major version? Just an idea, maybe it is starting to be a bit complex just for not including a simple api dependency…

pron commented 7 years ago

Yeah, that would be overkill. Are you sure, though, that ServletWebActors loads the websocket classes even if the relevant methods are never called?

victornoel commented 7 years ago

I just checked and yes it does happen :/ The more I think about it, and the more I think it's a lot of work just to avoid a dependency: I'm ok to make a PR, but if you agree that all of this is too much, let's abandon the idea :)

pron commented 7 years ago

Yeah, I think we'll wait until there's stronger motivation that just including an API JAR, considering how many unused JARs are in most applications anyway.