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

Minimize Jetty Coupling #2164

Open
zugazagoitia opened this issue Feb 29, 2024 · 6 comments
Open

Minimize Jetty Coupling #2164

zugazagoitia opened this issue Feb 29, 2024 · 6 comments

Comments

@zugazagoitia
Copy link
Member

While we leverage the Servlet API for compatibility, some areas of our codebase are tightly coupled to Jetty-specific implementations (like #2157). This issue aims to identify these dependencies to improve flexibility and maintainability.

Goals:

  • Keep Servlet Compatibility: Ensure our project can work with servlets following Jakarta EE specifications.
  • Abstraction: Introduce an abstraction layer or design pattern to decouple core servlet functionality from the specific HTTP server implementation.
  • Jetty Integration: Provide Jetty as an initial, supported HTTP server.
  • Flexibility: Enable the possibility of switching to other servlet-compatible HTTP servers with minimal code changes (potentially providing modules for it).

Discussion Points:

  • Interfaces?: Should we define our interfaces abstracted from servlet specs or leverage an existing abstraction?
  • Minimal Dependency: How can we limit Jetty-specific dependencies within our core logic?
  • Alternative Servers: What other viable servlet-compatible HTTP server options should we consider? What are the pros and cons of each?
  • Potential Challenges: Are there foreseeable challenges or limitations to this approach?

Identified Coupling Points

  • Static Files API: Our static files API is directly coupled to Jetty's resource handler. This creates complexity and hinders migration efforts (Explore Jetty-free implementation of resource handler #2157, investigate jetty 12 #2158).
    • Requirements: The replacement implementation should:
      • Be fully compatible with the current API.
      • Address security concerns and filter malicious requests.
      • Not depend on Jetty or Jakarta EE servlet APIs.
  • Jetty 12 Migration: We've encountered breaking changes in the upgrade to Jetty 12. Specific examples include:
    • Changes and removals in the MimeTypes class.
    • Refactoring of ResourceHandler, Resource, and related classes.
    • Modifications to Session API.
    • Potential issues with nested Jetty classes (org.eclipse.jetty.ee9.nested.*).
@cowwoc
Copy link

cowwoc commented Feb 29, 2024

Speaking for myself: Why add this functionality?

I get the feeling that people are coding against Javalin. They don't care what the underlying server is. Why would they want to swap it? What would the benefit be?

On the negative side, adding a layer of abstraction will introduce complexity and weight to Javalin. Sometimes it's best to just "lean in". For example, how often do people swap database types? Almost never. One of the reasons that Hibernate is a heavy beast is that they attempt to abstract away the database. If they were to "lean in" the library would be lighter, faster, and could use implementation-specific functionality that is not available on other databases.

@tipsy
Copy link
Member

tipsy commented Feb 29, 2024

I largely agree with @cowwoc, this is very expensive for what it brings to the average Javalin developer. Someone in the community would have to step up and commit to this.

@zugazagoitia
Copy link
Member Author

zugazagoitia commented Feb 29, 2024

I see this simply as technical debt, right now the very reason why we can't update to Jetty 12 is this, we're using internal jetty classes and APIs that have break Javalin during the update. This is not new, we had the very same issue with the HTTP/2 ALPN situation in #2084.

We're relying on some Jetty internals that weren't designed or expected to be relied upon, something that one of their developers pointed out in the jetty 12 (#2158) PR.

I don't intend to stop using Jetty, but most of our functionality is already independent from it, and the Servlet we create is quite functional.

@cowwoc
Copy link

cowwoc commented Feb 29, 2024

With respect to using internal classes, given that Jetty uses Java Modules they shouldn't be exporting any internal classes. If they are, I would consider it a bug in their end... you might want to clarify with them what happened in this case to avoid future problems.

@dzikoysk
Copy link
Member

Maybe I can help to clarify a few things here, because this is quite high level overview of the Jetty related issues, and it touches several areas. Just before we start, I'd like to highlight that this request is not about removing Jetty support or anything like that - I want to make that clear, because I know some people reading this thread may miss the point here and be a bit worried :)

Speaking for myself: Why add this functionality?

I'd personally say that the initial/main cause that leads us into this whole discussion is not about adding new functionality, but about maintenance, technical aspects related to the mixed implementation we have in the core module and separation of concerns. If we remove any references to alternative http server from this ticket description, the motivation behind implementing part of these changes is still valid.

On the negative side, adding a layer of abstraction will introduce complexity and weight to Javalin. Sometimes it's best to just "lean in".

I think there are different types of abstractions and blending it all together does not reflect the desired state. Adding the abstraction layer may lead to increased complexity when you're trying to support some sort of functionality in a very generic way etc., but in terms of this refactor, we'd like to just move Jetty-related complexity out of the core module, so we can actually keep our implementation as simple as possible.

For instance, using Jetty's resource handler and our own request handler forces us to implement static files as a special edge case of the whole request handler. You could probably say "it's just one place, probably good enough"... well, the problem here is that if we're mixing Javalin "way" of doing things on our side, and party doing it via Jetty's implementation, we're kinda desynchronizing the actual state and propagating this to upper layers. It's sometimes hard to implement features that are fully safe by design, because we're sometimes uncertain about the current state.

I get the feeling that people are coding against Javalin. They don't care what the underlying server is. Why would they want to swap it? What would the benefit be?

Assuming that the support for an alternative http server is a thing, I'd say it's encouraging people to stay with Javalin, because you can use Javalin's API without being bounded to a specific server. Speaking about the benefits - it simply depends on the use-case and what's your goal or issues you have with the existing server. It'd require to go deeper into more technical details, so maybe I'll stop on mentioning those 2 completely random examples:

  1. Green threads (via Loom) are quite cool topic, because it changes the way we can handle async tasks on JVM. For most of the existing http servers (also libs and frameworks), the only viable way to support it is to just replace currently used thread pools, but it's a shortcut, and it's not really utilizing it properly. But here it is, the Helidon Nima project that builds a new server on top of the Loom from scratch (yay).
  2. "I don't like dependencies" gang that could try to launch Javalin on top of the JDK's http server :shipit:

For example, how often do people swap database types? Almost never.

I'm not sure if this is the case here. It wouldn't be about swapping a server for existing applications, but rather a possibility to actually start using Javalin in a project where, for some reason, you need a specific http server due to its benefits.

One of the reasons that Hibernate is a heavy beast is that they attempt to abstract away the database. If they were to "lean in" the library would be lighter, faster, and could use implementation-specific functionality that is not available on other databases.

I think Hibernate is just bad, and generalized access to sql dialect is one of the least of its problems 💔

@patton73
Copy link

My two cents in this discussion.
Honestly i think that coding abstraction layers when you have clear that javalin is based over jetty is not only useless but also very harmful.
Lightweight means precisely "lightweight". So no abstraction layers, no useless interfaces, simply go directly with the provided api.
And to me this should be the real "power" of javalin.
Light, clean, easy to use, no magic, no useless annotations, no hidden features.

Please keep it as light as possible.
Thanks anyway for the huge effort you put in the project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants