-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Jetty-12 Structure #7638
Comments
@joakime @olamy I've assigned to you to think about the mechanics of this, including how the deployer might work. |
The code reads pretty well, particularly for those coming from previous versions of jetty. Do you intend that the I'll have to have a further think about the implications for eg |
@janbartel in this vision of things there is no core! |
I always envisioned this as the "end-game" of Step 1: prove out jetty-core-server |
I would encourage us to move away from URLClassLoader, as the changes coming in on newer JDK releases means we will have a new class of file locking issues on windows. |
Something not clear, in my mind my understanding was this mix will not be possible in embedded mode but only when using distribution which may have a different mechanism of class loading.
most of the tools (Maven, Gradle etc..) will not be able to manage to have both and build projects with such dependencies. |
@olamy if they have a need for multiple ee levels at the same time, they'll need to manage it from the the raw jar level on disk, not a formal dependency on a build tool. Think something like ...
Their main-server/ will have to use something like Those kinds of advanced setups will be more work, but not impossible. |
Sure everything is possible but what a complicated stuff to managed for those users.. and definitely not a build trick to recommend. |
@olamy I think |
So this is the structure I think we should go for, which is pretty much jetty-11 with extra:
The idea being that we leave as many modules in place as possible with same coordinates.
Ideally with the common modules, if we previously had an I'm wondering if it is best to start over with a 11 branch for this? |
For the jetty-12-hackathone I think we should try to make the following example work: DELETED THIS EXAMPLE (with classloaders). See next comment instead. public class Jetty12Example
{
// deleted. See next comment instead
} |
Some good points here. Let's finish exorcising servlet from current |
Actaully, let's do this initial goal without classloader complication, because "javax." and "jakarta." can coexist we can do ee10 and ee8 together like: public class Jetty12Example
{
public static void main(String[] args) throws Exception
{
Server server = new Server();
// HTTP
ServerConnector http2 = new ServerConnector(server);
server.addConnector(http2);
// HTTPS & H2
HttpConfiguration sslConfiguration = new HttpConfiguration();
SecureRequestCustomizer secureRequestCustomizer = new SecureRequestCustomizer();
sslConfiguration.addCustomizer(secureRequestCustomizer);
SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
File keystoreFile = sslContextFactory.getKeyStoreResource().getFile();
if (!keystoreFile.exists())
throw new FileNotFoundException(keystoreFile.getAbsolutePath());
sslContextFactory.setKeyStorePassword("storepwd");
ServerConnector https = new ServerConnector(server,
new SslConnectionFactory(sslContextFactory, HttpVersion.HTTP_1_1.asString()),
new HttpConnectionFactory(sslConfiguration),
new HTTP2CServerConnectionFactory(sslConfiguration));
server.addConnector(https);
// Declare server handler collection
ContextHandlerCollection contexts =
new ContextHandlerCollection();
server.setHandler(contexts);
// Add an embedded jetty-12 context
ContextHandler embedded = new ContextHandler("/embedded");
embedded.setHandler(new Handler.Processor()
{
public void process(Request request,
Response response,
Callback callback) throws Exception
{
response.setStatus(200);
response.setContentType(MimeTypes.Type.TEXT_PLAIN_UTF_8.asString());
response.write(true, callback, "Hello World");
}
});
contexts.addHandler(embedded);
// Add an EE10 ServletContext
ContextHandler ee10Context = new org.eclipse.jetty.ee10.servlet.ServletContextHandler("/ee10");
Handler.Wrapper ee10Session = new org.eclipse.jetty.ee10.session.SessionHandler();
org.eclipse.jetty.ee10.servlet.ServletHandler ee10Servlet = new org.eclipse.jetty.ee10.servlet.ServletHandler();
ee10Context.setHandler(ee10Session);
ee10Session.setHandler(ee10Servlet);
contexts.addHandler(ee10Context);
ee10Servlet.addServletWithMapping("org.acme.TestServlet", "/");
// Add an EE8 Webapp
ContextHandler ee8Context = new org.eclipse.jetty.ee8.webapp.WebAppContext("/ee8");
contexts.addHandler(ee8Context);
server.start();
server.dumpStdErr();
server.join();
} @joakime & @olamy what we need to achive with this from a git perspective (not sure if it is possible) is:
So I really think we need to start over from a fresh jetty-11. Somehow import the jetty-10 modules with history to be under ee8, and then pull apart jetty-servlet into parts that stay in place as common and parts that move to ee10. Only at the very end will we bring in ee9 |
@joakime missed your comment. Yes, let's take it 1 step at a time... continue working in the core structure for now... and plan what we want some more before acting again. |
Note, I'm giving up on the source code compatible idea for porting embedded applications from jetty-9/10/11 to 12. Whilst that would be possible, it would result in our main classes having strange package/class names, whilst our legacy stuff squatted on the good package/class names. We'd forever have the problem of same package/class in multiple jars. Moving the legacy code to ee8 and ee9 means some code modifications, but they should be trivial. It will give us a much cleaner base going forward. |
Contains code dependent on
Needs to be in
Needs to be in
Needs to be in
Contains some tests eg sessions that are dependent on
Has direct dependencies eg on
I think our split into ee8/9/10 might break this - not sure the tycho plugin will cope. Might be possible to do
|
I have created https://github.com/eclipse/jetty.project/tree/jetty-12.0.x-hackathon restarted from jetty-11 I've copied the servlet/webapp to an ee9 structure and made them all compile, so now we have both I will do the same with ee10 soon. Need to work out how the same files from jetty-10 can end up in ee8 with a direct git history that doesn't go via jakarta name space. Once the structure is good, only then will I copy over code from current 12.0.x branch, which will break some things. |
ee10 has been added |
I see
|
How are we structuring the various "big" categories now?
The Core and EE# layers will definitely need individual boms. |
There is now no core. Just jetty server in it's normal location. Ultimately anything dependent on a servlet api will need to move under a jetty-eeX module. I've only done the servlet/webApps for now as that's what we should work on next week. Things like jetty-home will stay top level. I'm not sure why you think 12.0.x is mangled? It's not been touched by this whilst i trial these ideas Ina different branch. |
Preserving the maven coordinates for those modules that will now have servlet removed will be awkward to explain/support. But the "compat" layers (within ee10) that we talked about at the old coordinates does make sense. So, that means |
I don't see a problem with the preserved maven coordinates:
There will never be something like |
I've now pushed the new
So yes the build is now horribly broken. Nothing above jetty-server will build. Can't make an omlette without breaking some eggs! Hackathon goals are to fix the disabled tests in |
OK, but there's 2 things that still need to happen. With this structure, that ALSO means /jetty-home/ and /tests/ cannot have servlet references, even transitively.
This is 100% ee specific, and should live in the appropriate |
Exactly
But jetty-home will still need to exist at the top level. We will still have it as our primary distribution. Ultimately ee8/ee9/ee10 become jetty modules that can be deployed into jetty-home, adding the right servlet-api in the right places.
Perhaps not 100%. JuliLog is not servlet-api specific.... but it may not be worth having a common module for one little class like that. That is why there is now an apache-ee9-jsp and an apache-ee10-jsp |
Target Jetty version(s)
Jetty-12
Enhancement Description
On consideration, I don't think we have the structure in jetty-12 correct yet. Specifically I don't think we should move the
Server
and coreHandler
classes to acore
package. TheServer
needs to be our primary front line class and should live where it always has atorg.eclipse.jetty.server.Server
. The consequence of this is that any jetty-10 or jetty-11 handlers that are ported to run in jetty-12 will need to be moved to packages likeorg.eclipse.jetty.ee8.server.Handler
, being passed aorg.eclipse.jetty.ee8.server.Request
etc.Below is an example of how I see us assembling a server that runs a jetty-12 embedded context, an ee10 ServletContext and an ee8 Webapp in the same server. I have left all the package names fully qualified for "clarity". Note that I have used classloaders to isolate the ee8 and ee10 contexts - this is not strictly needed as the servlet API for ee8 and ee10 can coexist in the same classloader ("javax." vs "jakarta."), but that approach would not work if we included ee9 as well as we'd have 2 versions of "jakarta.servlet.*". So classloader isolation has been added, even if strictly not need for this example:
So at some stage, I think we need to restructure again.... but let's do some more thought experiments on this before putting the effort in.
The text was updated successfully, but these errors were encountered: