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

EE9 Compatible Release #304

Closed
codylerum opened this issue Sep 27, 2021 · 88 comments
Closed

EE9 Compatible Release #304

codylerum opened this issue Sep 27, 2021 · 88 comments

Comments

@codylerum
Copy link
Contributor

Are there plans for an EE9 compatible release which targets the jakarta namespace?

@lincolnthree
Copy link
Member

Hey @codylerum ! Nice to hear from you :D It's been a while.

So, I haven't had a chance to actually look at anything in EE9, and I'd love to support this but am not really sure what it would entail. Any details you can provide?

How complex would the migration be, and would it be backwards compatible? A PR would of course be welcome, but again, not sure of the complexity.

@codylerum
Copy link
Contributor Author

@lincolnthree Not sure either of the scope, but I think the entire servlet API is moving from javax.servlet to jakarta.servlet so Imagine it would be a separate major version.

EE8
https://jakarta.ee/specifications/servlet/4.0/apidocs/

EE9
https://jakarta.ee/specifications/servlet/5.0/apidocs/

@lincolnthree
Copy link
Member

Yikes. I wonder why they opted to do that. Maybe an easier way to deprecate and force replacement of ancient APIs.

@lincolnthree
Copy link
Member

I'd be grateful if you wanted to take a stab at this!

@codylerum
Copy link
Contributor Author

codylerum commented Sep 29, 2021

I believe it ended up being a copyright issue when Oracle donated it to Eclipse. So there is going to be a firm demarcation between EE8 and EE9+

https://www.infoq.com/news/2019/05/end-of-javax-package/

Anyway I'll see if I can take a stab at it as I get closer to migrating my app to EE9. Honestly though I'm only using rewrite for a couple very simple things so I may just bring that in house and drop the rewrite dependency.

@larsgrefer
Copy link
Contributor

Adding my two cents here:

This definitely requires a new major version of rewrite. The Spring Team is going the same step with Spring Framework 6 and Spring Boot 3.

This new major version of rewrite would be the perfect opportunity to address things like #240 and get rid of some very old stuff.

As the first step for this, I suggest opening a "develop", "next" or "4.x" branch at which pull requests could then be targeted

@BorisYellnikov
Copy link

So, I've changed POM and namespaces and I was able to compile Rewrite, excluding tests. After changing maven properties I had to implements 2 or 3 methods but I didn't know how to implements these methods :). So, it seems all the work is to implements 2 or 3 methods and changing POM to support Java 17 and EE 9. I think it would be not to much work to update Rewrite to EE 9.

@lincolnthree
Copy link
Member

@BorisYellnikov Hey! Sorry, I missed this message. Would you mind sending a PR for what you've done so far? And let me know what is still incomplete / which methods are needing implementation? I'd love to make this happen, I just haven't had a lot of time to do the research to figure out where to start. (Since I'm not working with Java EE at the moment and my current project has been really all-consuming.)

I could most likely run with it and release a new version if you get me started! The tests are in sad shape :( Arquillian seems broken / unmaintained these days, unless I'm missing something.

@BorisYellnikov
Copy link

I did some time ago, so I don't remember exactly what i've done :). First what is to do is to change all namespace to jakarta. Next is to change in pom:

jakarta.platform
jakarta.jakartaee-api
9.0.0
provided


jakarta.servlet
jakarta.servlet-api
5.0.0
provided


jakarta.inject
jakarta.inject-api
2.0.0

I don't remember if I had to change source code in rewrite or in my HttpConfigurationProvider. I remove also tests and run manven with -DskipTests=true.

I've ran mavan today and this is result. I don't know why maven claims on dependencies.

NFO] Reactor Summary for rewrite-parent 3.4.5-SNAPSHOT:
[INFO]
[INFO] rewrite-parent ..................................... SUCCESS [ 0.957 s]
[INFO] rewrite-test-base .................................. SUCCESS [ 1.834 s]
[INFO] rewrite-api ........................................ SUCCESS [ 2.622 s]
[INFO] rewrite-api-el ..................................... SUCCESS [ 0.686 s]
[INFO] rewrite-impl ....................................... SUCCESS [ 0.340 s]
[INFO] rewrite-addressbuilder ............................. SUCCESS [ 0.771 s]
[INFO] rewrite-api-servlet ................................ SUCCESS [ 1.071 s]
[INFO] rewrite-impl-servlet-2.5 ........................... SUCCESS [ 0.603 s]
[INFO] rewrite-impl-servlet-3.0 ........................... SUCCESS [ 0.527 s]
[INFO] rewrite-impl-servlet ............................... SUCCESS [ 0.868 s]
[INFO] rewrite-test-harness ............................... SUCCESS [ 2.031 s]
[INFO] rewrite-config-servlet ............................. SUCCESS [ 2.254 s]
[INFO] rewrite-api-tests .................................. SUCCESS [ 1.162 s]
[INFO] rewrite-impl-tests ................................. SUCCESS [ 0.784 s]
[INFO] rewrite-annotations-api ............................ SUCCESS [ 0.386 s]
[INFO] rewrite-annotations-impl ........................... SUCCESS [ 1.301 s]
[INFO] rewrite-integration-cdi ............................ SUCCESS [ 1.433 s]
[INFO] rewrite-config-annotations ......................... SUCCESS [ 1.679 s]
[INFO] rewrite-config-jodatime ............................ SUCCESS [ 0.905 s]
[INFO] rewrite-integration-faces .......................... SUCCESS [ 1.838 s]
[INFO] rewrite-config-prettyfaces ......................... SUCCESS [ 4.439 s]
[INFO] rewrite-config-prettyfaces-tests ................... SUCCESS [ 1.777 s]
[INFO] rewrite-config-proxy ............................... SUCCESS [ 0.760 s]
[INFO] rewrite-config-servlet-tests ....................... SUCCESS [ 1.143 s]
[INFO] rewrite-impl-servlet-tests ......................... SUCCESS [ 1.196 s]
[INFO] rewrite-transform .................................. SUCCESS [ 1.097 s]
[INFO] rewrite-transform-less ............................. SUCCESS [ 0.912 s]
[INFO] rewrite-transform-minify ........................... SUCCESS [ 1.576 s]
[INFO] rewrite-transform-markup ........................... SUCCESS [ 2.384 s]
[INFO] rewrite-integration-cdi-tests ...................... SUCCESS [ 0.987 s]
[INFO] rewrite-security-integration-shiro ................. SUCCESS [ 1.016 s]
[INFO] rewrite-servlet .................................... FAILURE [ 0.390 s]
[INFO] rewrite-distribution ............................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 42.991 s
[INFO] Finished at: 2022-02-08T12:05:07+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal on project rewrite-servlet: Could not resolve dependencies for project org.ocpsoft.rewrite:rewrite-servlet:jar:3.4.5-SNAPSHOT: The following artifacts could not be resolved: org.ocpsoft.rewrite:rewrite-api:jar:sources:3.4.5-SNAPSHOT, org.ocpsoft.rewrite:rewrite-api-el:jar:sources:3.4.5-SNAPSHOT, org.ocpsoft.rewrite:rewrite-impl:jar:sources:3.4.5-SNAPSHOT, org.ocpsoft.rewrite:rewrite-config-servlet:jar:sources:3.4.5-SNAPSHOT, org.ocpsoft.rewrite:rewrite-api-servlet:jar:sources:3.4.5-SNAPSHOT, org.ocpsoft.rewrite:rewrite-impl-servlet:jar:sources:3.4.5-SNAPSHOT, org.ocpsoft.rewrite:rewrite-config-annotations:jar:sources:3.4.5-SNAPSHOT, org.ocpsoft.rewrite:rewrite-annotations-api:jar:sources:3.4.5-SNAPSHOT, org.ocpsoft.rewrite:rewrite-annotations-impl:jar:sources:3.4.5-SNAPSHOT: Could not find artifact org.ocpsoft.rewrite:rewrite-api:jar:sources:3.4.5-SNAPSHOT -> [Help 1]

@larsgrefer
Copy link
Contributor

@lincolnthree Could you prepare a "develop"-branch for rewrite 4.0.0-SNAPSHOT which we can target our pull requests against?

@lincolnthree
Copy link
Member

@BorisYellnikov Thanks! When you say "change all namespace to jakarta", can you give me an example?

That's an interesting error, I'm not sure what would cause the sources jars to be missing in the build. Or why it would fail on them. I'll look into it.

@larsgrefer Yes, I can do this tomorrow. Putting it on my todo list so I don't forget.

@BorisYellnikov
Copy link

This is old original namespaces
package org.ocpsoft.rewrite.servlet.http;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.ocpsoft.rewrite.event.Rewrite;
import org.ocpsoft.rewrite.servlet.ServletRewriteProvider;
import org.ocpsoft.rewrite.servlet.http.event.HttpServletRewrite;

/**

  • A {@link org.ocpsoft.rewrite.spi.RewriteProvider} that only operates on {@link HttpServletRequest} and

  • {@link HttpServletResponse} request cycle types.

  • @author Lincoln Baxter, III
    */
    public abstract class HttpRewriteProvider extends ServletRewriteProvider
    {
    @OverRide
    public boolean handles(final Rewrite event)
    {
    return event instanceof HttpServletRewrite;
    }

    @OverRide
    public final void rewrite(Rewrite event)
    {
    if (event instanceof HttpServletRewrite)
    rewriteHttp((HttpServletRewrite) event);
    }

    /**

    • Handle the current {@link HttpServletRewrite} event.
      */
      public abstract void rewriteHttp(HttpServletRewrite event);
      }

And new

package org.ocpsoft.rewrite.servlet.http;

import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;

import org.ocpsoft.rewrite.event.Rewrite;
import org.ocpsoft.rewrite.servlet.ServletRewriteProvider;
import org.ocpsoft.rewrite.servlet.http.event.HttpServletRewrite;

/**

  • A {@link org.ocpsoft.rewrite.spi.RewriteProvider} that only operates on {@link HttpServletRequest} and

  • {@link HttpServletResponse} request cycle types.

  • @author Lincoln Baxter, III
    */
    public abstract class HttpRewriteProvider extends ServletRewriteProvider
    {
    @OverRide
    public boolean handles(final Rewrite event)
    {
    return event instanceof HttpServletRewrite;
    }

    @OverRide
    public final void rewrite(Rewrite event)
    {
    if (event instanceof HttpServletRewrite)
    rewriteHttp((HttpServletRewrite) event);
    }

    /**

    • Handle the current {@link HttpServletRewrite} event.
      */
      public abstract void rewriteHttp(HttpServletRewrite event);
      }

In general. All namespaces with javax should be change to jakarta in all EE specification namespaces. I think the most work will be to change all pom properties. Source code of rewrite should compile without problems without changes.

@lincolnthree
Copy link
Member

@BorisYellnikov Ok thanks. This makes a lot of sense.

I created a #develop branch for this work.

@lincolnthree
Copy link
Member

Started looking at dependencies and pom issues 😭

@lincolnthree
Copy link
Member

I'll fix the snapshot version later.

@larsgrefer
Copy link
Contributor

@lincolnthree I've opened #310 as first attempt to make the project buildable again.
There are still some errors regarding the tests: org.jboss.arquillian.container.spi.client.container.LifecycleException: Could not start container

I'm not sure if its worth fixing the current containers, as we'll need JEE9 compatible ones anyways

@lincolnthree
Copy link
Member

Thanks @larsgrefer - Merged. I agree about the containers. I also think deleting some of the very outdated integrations is probably in order. I think we can safely delete:

Apache Shiro
Google GWT
Tuckey URLrewrite config parser
Servlet 2.5
Servlet 3.0

Thoughts?

I got part way through the POM updates but had to stop. Planning on continuing over the weekend.

@lincolnthree
Copy link
Member

PS. Does anyone know if there are any Arquillian updates for modern Jakarta EE containers? Is there a new equivalent test harness project we could use?

@larsgrefer
Copy link
Contributor

@lincolnthree Servlet 2.5 and Servlet 3.0 can definitely go, since we'll have Servlet 5 baseline anyway.

With the minimum Java Version being 1.8 now, Joda Time should be redundant, too.

@lincolnthree
Copy link
Member

Hey guys. Sorry, I got pulled into some stuff over the weekend. I have not forgotten about this. It might take me a bit but this is happening. My schedule is absolutely wild right now.

@larsgrefer
Copy link
Contributor

PS. Does anyone know if there are any Arquillian updates for modern Jakarta EE containers? Is there a new equivalent test harness project we could use?

At least Tomcat 10 seems to be supported: arquillian/arquillian-container-tomcat#87

@lincolnthree
Copy link
Member

@larsgrefer That's good to know. Merging your new PRs now. Still crazy over here. Thank you for continuing to work on this. I am really sorry I am not able to be more helpful just yet.

@lincolnthree
Copy link
Member

@larsgrefer Wow, I am blown away by the incredible work you have put in to the past few PRs. Thank you, seriously. What maven command are you running the updated tests with? (E.g. container specification - if any - etc.)

@larsgrefer
Copy link
Contributor

Just ./mvnw test should run the tests.

In #316 I added a simple embedded Tomcat 9 to test-harness which fixes most of the Arquillian based tests.
I choose the embedded Tomcat because it was very easy to set up.

Some tests are still broken, but I'm not sure yet, why.
At least one test has a classloading issue which seems to be caused by the fact that the test itself and the Tomcat under test are running with different classloaders in the same JVM.

@larsgrefer
Copy link
Contributor

@lincolnthree I think https://github.com/ocpsoft/common and https://github.com/ocpsoft/parent could use some updates to.

Are there any other ocpsoft projects which still use common and parent or can these be merged into the rewrite project?

@lincolnthree
Copy link
Member

@larsgrefer A few projects use them. I can cut some releases if you want to send PRs to do what's needed! Probably best to increment the major version for these as well.

@larsgrefer
Copy link
Contributor

common already got two PRs.

For new releases I agree with new major versions for each.
Since common uses parent we should release parent first, so common can use the new parent release

@lincolnthree
Copy link
Member

@larsgrefer I completely agree. I think this makes total sense. I'll work on updating versions today.

@larsgrefer
Copy link
Contributor

Seems like the automatic snapshot deployments are now working: #332 https://github.com/ocpsoft/rewrite/runs/6864680645?check_suite_focus=true

So after you set the correct version numbers, we should forward-merge main into develop and then develop into develop-jakartaee-9 in order publish snapshots for those versions, too.

@lincolnthree
Copy link
Member

@larsgrefer Awesome! Version numbers are set. I had to force push and correct a simple mistake in the develop branch (within the span of about 2 minutes), so just in case you pulled during that time, that's what happened.

@larsgrefer
Copy link
Contributor

@lincolnthree How would you like to continue with the two develop into develop-jakartaee-9 branches?

Everything happening in develop should happen in develop-jakartaee-9, too, but we have 2 ways of doing this:

  1. Doing everything twice: One PR for the change in develop and then a second PR for the same change in develop-jakartaee-9
  2. Frequently forward-merging develop into develop-jakartaee-9 in order to keep both branches in sync.

I think Option 2 is "cleaner" and more elegant, but the forward-merges should be done by someone with direct write access to the repository. Forward merges via PR (like #336) are a bit tedious

@lincolnthree
Copy link
Member

lincolnthree commented Jun 27, 2022

@larsgrefer Sorry for the delay. Getting caught back up in firefighting work stuff again. My guess is that this will probably be mostly an issue doing bugfixes, because I think the framework is fairly feature complete, but yes, I tend to agree.

I think Option 2 is probably a bit cleaner, and I agree about a maintainer doing the work being easier, not sure I'd consider it a requirement though. This should be fairly painless assuming the two branches don't get too out of sync.

I think Option 1 is fine if the contributor wants to go through that effort. I guess I don't really have a strong preference.

@lincolnthree
Copy link
Member

@larsgrefer Can you update me on where the current PRs stand? There are a few open and I don't want to screw it up :)

Do you want me to merge the PRs targeted at develop so you can update your forward merge PR? Or do you want me to leave them alone for now?

@larsgrefer
Copy link
Contributor

#337 and #339 would be up next

@lincolnthree
Copy link
Member

@larsgrefer Merged. Those both look good. Thanks for splitting the integration tests. I honestly have not learned how GitHub actions work yet, so I really appreciate this.

@lincolnthree
Copy link
Member

@larsgrefer You are incredible. Thank you for all of this work. I owe you many beers. What's next? All tests appear to be green. Is it time to cut a preliminary release?

@larsgrefer
Copy link
Contributor

Releasing a 4.0.0-m1 and 5.0.0-m1 sounds like a good idea

@lincolnthree
Copy link
Member

4.0.0.Alpha1 and 5.0.0.Alpha1 have been released in Sonatype Nexus and should sync to central by tomorrow at latest :) Thanks you @larsgrefer, @pdurbin, @codylerum, and @BorisYellnikov for all of your amazing work in making this happen! And also thank you for your patience with me 😆 Apologies if I forgot anyone, I think @pdurbin was also submitting on behalf of a colleague.

Please let me know how it goes during testing.

poikilotherm added a commit to IQSS/dataverse that referenced this issue Jul 25, 2022
With the release of prettyfaces/rewrite 5.0.0.Alpha1 they finally
released a version compatible with at least Jakarta EE 9.

This includes the patches @pdurbin applied to our local build, too.
Removing the local built, too.

See also: ocpsoft/rewrite#304
poikilotherm added a commit to IQSS/dataverse that referenced this issue Jul 25, 2022
With the release of prettyfaces/rewrite 5.0.0.Alpha1 they finally
released a version compatible with at least Jakarta EE 9.

This includes the patches @pdurbin applied to our local build, too.
Removing the local built, too.

See also: ocpsoft/rewrite#304
@pdurbin
Copy link

pdurbin commented Jul 25, 2022

@lincolnthree et al. thanks for the release and for reaching out! @poikilotherm has been digging in. Unfortunately, we're pretty sure even 5.0.0.Alpha1 isn't compatible with EE 10 so we opened #345. Please take a look when you get a chance. Thanks!

poikilotherm added a commit to IQSS/dataverse that referenced this issue Jul 26, 2022
With the release of prettyfaces/rewrite 5.0.0.Alpha1 they finally
released a version compatible with at least Jakarta EE 9.

This includes the patches @pdurbin applied to our local build, too.
Removing the local built, too.

See also: ocpsoft/rewrite#304
poikilotherm added a commit to IQSS/dataverse that referenced this issue Jul 27, 2022
With the release of prettyfaces/rewrite 5.0.0.Alpha1 they finally
released a version compatible with at least Jakarta EE 9.

This includes the patches @pdurbin applied to our local build, too.
Removing the local built, too.

See also: ocpsoft/rewrite#304
@lincolnthree
Copy link
Member

Has anyone had a chance to test the Alpha releases for EE8 and 9 support? What was the outcome?

@pdurbin
Copy link

pdurbin commented Nov 3, 2022

Our next target is EE 10 so we're really only testing that. That is, we're looking at #345. We don't plan to use EE 9. Thanks.

@rzo1
Copy link

rzo1 commented May 23, 2023

Has anyone had a chance to test the Alpha releases for EE8 and 9 support? What was the outcome?

I tested 5.0.0.Alpha1 on our jakarta migration branch and it worked fine.

@raphisuter
Copy link

Has anyone had a chance to test the Alpha releases for EE8 and 9 support? What was the outcome?

Hi @lincolnthree
I don't know if this is still relevant but we are migrating a big platform from java EE7 to jakarta EE 9.1 and we are using rewrite. We are using 5.0.0.Alpha1. Can keep you updated if you're intressted.

@lincolnthree
Copy link
Member

Hey @raphisuter ! Definitely! Sorry the official "Final" release has been lagging behind. As I understands it, the Alpha seems to work (Final would just be a version bump unless someone finds an issue). I am trying to make time to get caught up on this and cut a release in the next few weeks.

So please do let me know how it goes!

@raphisuter
Copy link

raphisuter commented Aug 23, 2023

Hi @lincolnthree
Sadly at this point we are far of the JavaEE Standard and need to sort out a lot of dependencies. It is challenging to keep the overview of all the things popping up.
At this point I start to try and compile client after client and sort out all issues occurring module by module. As I'm no at the point of trying to compile the first view related module using rewrite I got into troubles because it's missing a spring-framework dependency.

Failed to execute goal on project view-client: Could not resolve dependencies for project ch.qualitas:view-client:jar:0.0-local: Failed to collect dependencies at org.ocpsoft.rewrite:rewrite-servlet:jar:5.0.0.Alpha1: Failed to read artifact descriptor for org.ocpsoft.rewrite:rewrite-servlet:jar:5.0.0.Alpha1: Could not find artifact org.springframework:spring-framework-bom:pom:6.0.0-M4 in nexus (inhouse nexus url) -> [Help 1]

I'm not sure if I have to include this dependency if we are not using the spring integration. In the module which fails to build we use following dependencies

<dependency>
  <groupId>org.ocpsoft.rewrite</groupId>
  <artifactId>rewrite-servlet</artifactId>
</dependency>
<dependency>
  <groupId>org.ocpsoft.rewrite</groupId>
  <artifactId>rewrite-integration-faces</artifactId>
</dependency>

Any advice?

Update
The reason why I dont get the dependency is because we are having a single mirror configured in maven settings which has no access to the spring repo. I'm gonna sort this out and continue trying to compile the modules. The question still stands if I really need this dependency.

Update 2
I sorted out all occurring issues and was able to mvn compile all 82 modules (8 clients, 74 modules). I added a new proxy on our nexus server to get access to the spring milestone repo and now as mentioned compiling is ok.

@pdurbin
Copy link

pdurbin commented Aug 24, 2023

@lincolnthree hi! I see chatter here about 5.0.0.Alpha1 but should I inquire about 6.0.0.Alpha1 over in #345?

@lincolnthree
Copy link
Member

This is released as rewrite Version 9.0.0.Final! Thanks everyone for their patience and encouragement. This has been a momentous and multi-year effort. Thanks to @pdurbin @codylerum @larsgrefer @poikilotherm and everyone else.

image

@lincolnthree
Copy link
Member

Closing this as COMPLETED!

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

No branches or pull requests

8 participants