Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move websocket-util classes into websocket-core #5705

Merged
merged 31 commits into from
Dec 2, 2020

Conversation

lachlan-roberts
Copy link
Contributor

  • Move the websocket-util classes into websocket-core-common internal package.
  • Move the websocket-util-server internal classes into the websocket-core-server API.
  • API cleanups of websocket-core
  • WebSocketUpgradeHandler now uses the WebSocketMappings and can upgrade to multiple different FrameHandlers.
  • WebSocketUpgradeFilter now exists in a jar all by itself.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
…shaker instead

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts
Copy link
Contributor Author

@gregw @sbordet There is still something strange about the websocket structure. The websocket-jetty-common module is really just a private implementation but it leaks a bunch of websocket-core classes in its method signatures. It is used as common implementation between websocket-jetty-server and websocket-jetty-client, you are never expected to directly use classes from websocket-jetty-common. This is different to websocket-core-common, you need to use classes directly from this jar as there is no websocket-core-api.

And there is JPMS warnings because these internal classes from websocket-core-common are in the public signatures of websocket-jetty-common and websocket-javax-common, even though these are really just meant to be internal implementation jars.

…websocket-util

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@sbordet
Copy link
Contributor

sbordet commented Nov 24, 2020

@lachlan-roberts there are a bunch of test failures due to the latest changes.

The reason is that now JPMS module org.eclipse.jetty.websocket.jetty.common is only exported to org.eclipse.jetty.websocket.jetty.client and *-server.

However, there are test classes that are in the org.eclipse.jetty.websocket.jetty.common module, and for tests that's the application Lookup object.

However, classes from that package/module are not exported so the Lookup object has allowedModes=0 (which means no permissions), which means that we get an IllegalAccessException when trying to call findVirtual() to find the @OnMessage method.

@sbordet
Copy link
Contributor

sbordet commented Nov 24, 2020

Feels like a JDK bug... I sent an email to jigsaw-dev asking for clarifications.

@sbordet
Copy link
Contributor

sbordet commented Nov 24, 2020

@lachlan-roberts jigsaw-dev replied and classes from org.eclipse.jetty.websocket.jetty.common must be exported unconditionally.

I'm making this change and see if the PR builds fine.

sbordet and others added 3 commits November 24, 2020 21:57
…ng properly.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
olamy and others added 2 commits November 26, 2020 10:47
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
…websocket-util

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Jenkinsfile Outdated Show resolved Hide resolved
olamy and others added 4 commits November 26, 2020 12:24
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
olamy and others added 9 commits November 27, 2020 13:30
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
Copy link
Contributor

@sbordet sbordet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few nibble to fix.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@lachlan-roberts lachlan-roberts merged commit 121d898 into jetty-10.0.x Dec 2, 2020
@lachlan-roberts lachlan-roberts deleted the jetty-10.0.x-websocket-util branch December 2, 2020 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants