jcabi / jcabi-manifests

Java library for convenient reading of MANIFEST.MF files available in classpath
https://manifests.jcabi.com
Other
60 stars 22 forks source link

Add support for Jakarta EE 9.1 and higher #60

Closed melbeltagy closed 1 year ago

melbeltagy commented 1 year ago

This PR adds support for Jakarta EE Servlet API as a replacement for the previous JavaEE Servlet API (package change javax.servlet -> jakarta.servlet) It can be used by Jakarta EE 9.1 and higher

Notes:

melbeltagy commented 1 year ago

@yegor256 May I ask your help reviewing this, please?

yegor256 commented 1 year ago

@melbeltagy I'm a bit scared here, since we totally move away from "standard" API. Is it possible to somehow maintain backward compatibility? WDYT?

melbeltagy commented 1 year ago

Thank you @yegor256 for the quick reply. I'm not sure what you mean by "standard" API, to be honest. As you know Jakarta EE is the new standard APIs after Oracle donating JavaEE to Eclipse. So, all javax.* packages from JavaEE have now moved to jakarta.* since Jakarta EE 9.

Regarding backwards compatibility, I am open for any suggestions on how to achieve it. The issue is that javax.servlet package is the old API from JavaEE days. jakarta.servlet is the new package replacement starting from JakartaEE 9. Having dependency on both will bring in unnecessary dependency to the users of the jcabi library.

I think what we can do is have two separate versions:

WDYT?

melbeltagy commented 1 year ago

@yegor256 Any thoughts/updates regarding this PR, please?

melbeltagy commented 1 year ago

@yegor256 I can see that Version 2.0 is created and still doesn't have a Jakarta EE support. The issue is that we're actually using it in our system and we have plans on upgrading to Spring Boot version 3.0 which, as you know, has migrated to Jakarta EE.

We tried using version 1.x of the library with Spring Boot 3.x, but it didn't work due to jcabi-manifests bringing in javax.servlet packages.

Your input/support with this topic is highly appreciated.

yegor256 commented 1 year ago

@melbeltagy how about instead of modifying the existing ServletMfs class, we add a new one called JakartaServletMfs, which will work with new Jakarta API? Also, we make all servlet* dependencies in pom.xml optional. This will allow users to use either old Servlet API or the new one.

melbeltagy commented 1 year ago

@yegor256 That sounds like a good idea. Having both classes, and both dependencies. So, to make sure I understand the idea correctly, users of javax.servlet.* package would use the ServletMfs. Users of jakarta.servlet.* package would use the JakartaServletMfs.

But since these packages cannot be used together on the same classpath, how would we configure the build in this case, please?
This is my first time considering this approach, so any clarification on how it can be done would be much appreciated.

Edit: I guess your hint to the optional dependency is what I needed 😄 . Will play around with it then and update the you afterwards. Thank you.

yegor256 commented 1 year ago

But since these packages cannot be used together on the same classpath

@melbeltagy why not?

I mean, in a webapp they definitely should not stay together on the classpath. But in this library they can easily co-exist on classpath during compilation, I believe.

melbeltagy commented 1 year ago

@yegor256 Thank you so much. The PR is now updated according to your recommendations and uses optional for both javax.* and jakarta.* dependencies.

Also, the deprecated methods in manifests are now removed to limit the dependency on servlet.* package to ServletMfs class only.

Please let me know if something else is missing and requires to be updated. Once again.. thank you :)

yegor256 commented 1 year ago

@rultor merge

rultor commented 1 year ago

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor commented 1 year ago

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 3min)

yegor256 commented 1 year ago

@rultor release, tag is 2.1.0

rultor commented 1 year ago

@rultor release, tag is 2.1.0

@yegor256 OK, I will release it now. Please check the progress here

rultor commented 1 year ago

@rultor release, tag is 2.1.0

@yegor256 Done! FYI, the full log is here (took me 14min)

yegor256 commented 1 year ago

@melbeltagy it's released, thanks for your contribution!