Move client classes and dependencies out from websocket-core #2174
Comments
|
I remain unconvinced we need this complexity. Simplicity is worth a lot an needless fine grained modules can create a lot of complexity and confusion.
In fact, I think we could go the other way and remove/merge modules such as I think for each foobar API we should have foobar-websocket-common, foobar-websocket-server, foobar-websocket-client, foobar-websocket-test. Everything else should either be in core or the foobar commons. |
|
I had Joakim walk me through this a bit earlier because something doesn't smell right, why should this websocket goop be any different then the rest of Jetty structure wise? You have a jetty-client that depends on jetty-io and has no concept of the servlet-api or the jetty-server itself, you have a jetty-server that depends on jetty-io and no concept of a jetty-client but does care about the servlet-api and stuff like that. We have bent over backwards historically to ensure a clean separation of concerns exists and this seems like websocket is something different then just a protocol with a client and a server and some shared io classes I am not seeing what benefit there is to glomming everything together into a -core module (which is a new term?) that tries to be all things to all people. IMO:
I can see how it got to this point out of convenience for development which is what you have been advocating for months, don't get hung up on the project structure, focus on API's, but once you guys have the refactor fleshed out I would think a good test of that would be at least validating separation of concerns like the other protocol modules in Jetty. Unless I am missing something fundamental here? |
|
I can live with But having a few client/server specific classes in
We don't have But also note that things like I agree that I think that In summary, I think the hierarchy should be nice and simple: And finally, as I keep saying... perhaps once we are done a requirement may emerge that will require us to change how we decompose the impl into modules. However, no such requirement is currently apparent, so if one does emerge, any preemptive decomposition is likely to be wrong. |
|
@gregw , in theory it looks nice and straight forward and so long as the dependencies are not trying to push a mess of <excludes> and <provides> magic onto users that is fine. I stopped with @joakime when I saw a mess of ugly osgi includes and excludes that i believe were automagically generated from the maven build. Actually, if you updated your graphic with the intended jetty-* dependencies that would help, like websocket-io should only depend on jetty-io in my mind, and in turn jetty-util. |
|
pushing my ascii-art foo to its max: dotted lines are provided dependencies |
|
Man, I don't like provided dependencies at all for an io library like that. Is there that much code in there that you can't just put client and server dependencies in the end oriented modules and do away with jetty-websocket and javax-websocket? I don't see why there needs to be circles in there at all, purity wise it should be jetty-http -> websocket-io then branch to [ws-client | ws-server] and then javax-websocket-client builds on top of that ws-client and then javax-websocket-client is a dependency on javax-websocket-server....that way it is all pure straight lines and makes good maven sense (and osgi sense). |
Maybe ws-client and ws-server have the stuff needed to act as the jetty specific api version in them as well and it is just ignored when being used in the javax context. |
|
This issue has been automatically marked as stale because it has been a full year without activit. It will be closed if no further activity occurs. Thank you for your contributions. |
|
@lachlan-roberts can this be closed? ie are we happy with the current package structure? |
|
Build a minimal websocket client only project. (you should have no server dependencies) |
|
@joakime I don't get it. If you want to know the dependencies for a core client: or for a javax client: There are classes in those dependencies that will not be used and splitting core into more packages won't change that. If you really want an absolute minimal set of classes, then some dynamic analysis is required that will not be helped by more core packages? |
|
The last time I tried, there were more lines of |
|
There was also a bunch of |
|
I'm just not understanding the problem? Just use core dependency with no exclusions. OK it does depend on jetty-server, which is probably not really needed... but there are always going to be unused classes in the classpath unless you do a dynamic analysis and trim to specific classes. I thinking adding packages is just too much unnecessary complexity to save a few 100kB of jars |
|
The design of websocket-core is just antithetical to how to properly create a jar in maven, osgi, and java 9+ (JPMS). The reason this issue exists is to fix that. Then there's the maintenance angle as well. |
|
I still don't get the strong concern. We have split the packages into client/server within the module, so this should be pretty trivial to implement. Let's see a PR and see if the extra complexity is worth it. |
|
I'm fine with the current websocket-core structure, but will leave this and #2173 open for discussion. |
Currently,
websocket-corehas client classes and dependencies.These are not appropriate for
websocket-coreas they contaminate the dependency tree and complicate the WebAppClassloader for pure websocket servers. (currently Jetty Native WebSockets, not JSR356, and the future proposed Reactive streams server)These classes, test cases, and dependencies should be moved out of
websocket-coreThe text was updated successfully, but these errors were encountered: