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

Add a new release artifact for jakarta.servlet support #9845

Merged
merged 37 commits into from
Dec 6, 2023

Conversation

niloc132
Copy link
Member

@niloc132 niloc132 commented Jul 16, 2023

Draft PR to bring together changes, make a single easier-to-find place to discuss.

Some past discussion can be found at niloc132#3.

Pending work:

  • The maven artifact gwt-servlet-jakarta's sources is missing generated .java files for the new jakarta packages. This is noted in the TODO added to maven/lib-gwt.sh
  • The new package-info.java files should be added to each newly created package manually, explaining that the sources are generated, and not available in the main artifacts, only in gwt-servlet-jakarta.
  • In addition to gwt-servlet, the requestfactory-server jar should also get a forked copy generated.
  • Discuss if this is mergable without also rewriting some/all of javax.validation to jakarta.validation. This will be mostly discussed at Jakarta Validation Support #9844.
  • Merge Add jakarta-servlet-api tools#27, so we can back out the changes to .github/workflows/*-check.yml.
  • Land "unrelated" changes upstream before this merges, to keep this mess at as simple as possible.

Testing notes:

  • Among other testing, we should verify that this is all working with servlet-api 6.0, not just 5.0
  • Javadoc should be verified to work in new and old packages, with javax.servlet vs jakarta.servlet references.

Fixes #9727

niloc132 and others added 27 commits December 1, 2022 19:07
This is a draft patch to add support for Jakarta. It is assumed to only
be used for modular projects, where client and server do not live in the
same classpath. Among other things, this means that validation does not
need to be updated, and old javax.servlet references can be left in
gwt-dev and gwt-user.

This does not correct the sources for deployment to maven, nor does it
fix javadoc.

This does not update the requestfactory-server jar.

Draft for gwtproject#9727
One removes an out of date jar, the other is duplicated elsewhere on the
classpath (from gwt-dev)
This is a draft patch to add support for Jakarta. It is assumed to only
be used for modular projects, where client and server do not live in the
same classpath. Among other things, this means that validation does not
need to be updated, and old javax.servlet references can be left in
gwt-dev and gwt-user.

This does not correct the sources for deployment to maven, nor does it
fix javadoc.

This does not update the requestfactory-server jar.

Draft for gwtproject#9727
One removes an out of date jar, the other is duplicated elsewhere on the
classpath (from gwt-dev)
@@ -121,9 +121,7 @@
">
<src path="super/com/google/gwt/emul/javaemul/internal" />
<classpath>
<pathelement location="${gwt.tools.lib}/gss/2015-10-07/closure-stylesheets-library-20151007-rebased.jar"/>
Copy link
Member Author

Choose a reason for hiding this comment

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

This shouldn't have still been here to begin with, but should probably land separately (ditto the tomcat line below).

<zipfileset src="${gwt.tools.lib}/jscomp/20160315/sourcemap-rebased.jar" />
<zipfileset src="${gwt.tools.lib}/json/android-sdk-19.1/json-android-rebased.jar" />
<zipfileset src="${gwt.tools.lib}/streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-1.5-rebased.jar" />
<zipfileset src="${gwt.tools.lib}/protobuf/protobuf-2.5.0/protobuf-java-rebased-2.5.0.jar" />
Copy link
Member Author

Choose a reason for hiding this comment

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

if #9785 lands, this will need the same change.

doc/build.xml Outdated
@@ -92,6 +110,9 @@
<arg value="-Xdoclint:none" />
<classpath refid="USER_CLASS_PATH" />
<sourcepath refid="USER_SOURCE_PATH" />
<fileset dir="${project.build}/javadoc-src">
Copy link
Member Author

Choose a reason for hiding this comment

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

With the unified approach to sources, this must be removed.

maven/lib-gwt.sh Outdated
[ -d $GWT_EXTRACT_DIR/doc/javadoc ] && jar cf $JAVADOC_FILE_PATH -C $GWT_EXTRACT_DIR/doc/javadoc .

JAVADOC_JAKARTA_FILE_PATH=$RANDOM_DIR/gwt-jakarta-javadoc.jar
Copy link
Member Author

Choose a reason for hiding this comment

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

this should no longer be required

@PriyaKolekar
Copy link

Hello Everyone,

We are using GWT 2.10 version and want to upgrade tomcat to new version Tomcat10. But as mentioned above, GWT jars are not compatible to use as is due to package structure change in Tomcat. I have three queries -

  1. When ready-to-use snapshot version will be available for gwt-user.jar, gwt-servlet.jar, requestfactory-server.jar. Are there any other jars under GWT umbrella affected by Jakarta package structure change.

  2. Is there a plan to merge these changes into main and release new GWT version. If yes, when will be that available.

  3. We had automated tomcat migrator tool which runs on source code and does package level changes (replacing javax.servlet with jakarka.servlet). If such tool is run on existing 2.10 GWT jars, will it work correctly or it might cause compile-time/run-time issues?

@niloc132
Copy link
Member Author

niloc132 commented Aug 8, 2023

  1. When ready-to-use snapshot version will be available for gwt-user.jar, gwt-servlet.jar, requestfactory-server.jar. Are there any other jars under GWT umbrella affected by Jakarta package structure change.

Only gwt-servlet.jar has a modified version at this time, and there is no plan for gwt-user.jar have a jakarta variant. This branch is missing the changes required to produce the required requestfactory-server.jar, but I think that is the main missing piece to be able to merge this. This is at the top of the remaining "TODO" list in the description of the PR.

  1. Is there a plan to merge these changes into main and release new GWT version. If yes, when will be that available.

Once this patch is complete and tested it will be merge. Presently no one is working on it, so there isn't a timeline for this. A month ago I would have said that this would definitely make the 2.11.0 release, but given that it seems no one is interested any longer, I'm not confident in that any more. I've been out for a few weeks for personal reasons, and am hoping to finalize any other patches that intend to make the release so we can get this out the door - if you (or someone else) is interested in pushing this forward, please contact me here/on gitter/via email.

  1. We had automated tomcat migrator tool which runs on source code and does package level changes (replacing javax.servlet with jakarka.servlet). If such tool is run on existing 2.10 GWT jars, will it work correctly or it might cause compile-time/run-time issues?

Its worth a try, but the output won't be exactly compatible with what this patch tries to achieve. That is, instead of changing classes in-place, this patch copies classes to a new package and changes them there. This serves several purposes: the imports are clear (so that having the wrong imports or wrong classpath will produce clear errors instead of confusing runtime problems), and the documentation is clear ("use this package for old servlets, use that package for jakarta"). I don't think that is necessarily a problem, since you could continue to run that same tool with future versions of GWT, and just skip the jakarta-specific artifacts, but if the above concerns seems like they could cause a future headache for you, it might make sense to get this patch finished as-is.

Note that we didn't try to use such a tool to make the changes in this PR, but just a few custom copy/filter operations in ant xml to specifically make new classes with the desired changes.

@DRLovell
Copy link

DRLovell commented Sep 6, 2023

Hello folks,

We use GWT and will need it to work in JBoss EAP 8.0, which uses Jakarta EE 10.

Has there been any further progress?

Excellent platform, by the way!

Many thanks.

@niloc132
Copy link
Member Author

I've just finished a few hours of work to get the remaining development/doc tasks finished - basic tests suggest that it works, but more in depth testing by projects already using jakarta-servlet is required. If you are using gwt-servlet, please try the gwt-servlet-jakarta jar, if you are using requestfactory-server please try the requestfactory-server-jakarta jar.

One important tradeoff to avoid adding requiring client-specific jars also change is to not support the RequestFactoryLogHandler. Due to how RF works, client code depends on shared code which depends on server code, and the server class has an explicit dependency on the RequestFactoryServlet itself. If there is a strong objection here, one could potentially provide a way to share access to to the current threadlocal servlet request in a javax/jakarta-agnostic way to read headers from the incoming request. However, my suspicion is that this is rarely used (even more rarely than RF itself), and the work to fork this type is minimal, and documented roughly at https://www.colinalworth.com/gwt-javadoc-jakarta/javadoc/com/google/web/bindery/requestfactory/server/jakarta/Logging.html. Note also that neither LoggingRequest nor RequestFactoryLogHandler are even present in the requestfactory-client jar anyway.

Javadoc is updated for review at https://www.colinalworth.com/gwt-javadoc-jakarta/javadoc/

Maven repo server has a new snapshot, use repo https://repo.vertispan.com/gwt-snapshot/ and version 2.11.0-jakarta-SNAPSHOT as before. Be sure to test the gwt-servlet-jakarta and requestfactory-server-jakarta artifactIds.

Please note that this is probably the last "big" change that is waiting for review before 2.11 ships - don't assume someone else will test so we can land this. If it is important to you to see this shipped without waiting for 2.12, please help us by testing it promptly.

@niloc132 niloc132 marked this pull request as ready for review September 27, 2023 18:54
@ajkyffin
Copy link

ajkyffin commented Oct 2, 2023

I've tested the latest snapshot (20230928) of gwt-servlet-jakarta and I've had the previous snapshot (20230717) in production for a while, both with no problems.

The only jakarta class I'm using is com.google.gwt.user.server.rpc.jakarta.RemoteServiceServlet. I'm not using requestfactory.

@DRLovell
Copy link

DRLovell commented Oct 2, 2023

Many thanks niloc132 and ajkyffin: this is very much appreciated!

@niloc132
Copy link
Member Author

niloc132 commented Oct 2, 2023

@ajkyffin can you confirm if your use case would support using requestfactory-server-jakarta.jar instead, and if so, could you try using that simpler jar to ensure that this works as well?

@ajkyffin
Copy link

ajkyffin commented Oct 2, 2023

@niloc132 I may be able to to convert it to use RequestFactory instead of RPC. It looks like AutoBeans could do most of the work for me. It would be a few weeks before I have the time to do it tho.

@niloc132
Copy link
Member Author

niloc132 commented Oct 2, 2023

@ajkyffin my apologies, I misread (and I've been doing that more than a little lately... might be time for a break). If you're using RPC and not RequestFactory (as I thought it said on my first read), it isn't worth the effort of you updating to change both that and your classpath.

@codeart1st
Copy link

codeart1st commented Oct 16, 2023

Started migrating a very large application to Jakarta EE with WildFly and Tomcat support. Looks promising so far. We also needed to migrate vom Jetty-based dev mode to standalone Tomcat (-noserver).

Good job up to here.

Next step for us is to experiment with javax.validation migration. Maybe we give gwt-bean-validators a try.

@DRLovell
Copy link

DRLovell commented Nov 1, 2023

I've migrated our web application to Jakarta EE 10, and with the 28 Sep 2023 GWT Jakarta server JAR for our GWT-RPC servlets it's all behaving great!

Super DevMode works great, too!

Thanks very much all!

@niloc132
Copy link
Member Author

niloc132 commented Nov 1, 2023

Thanks for the feedback everyone - I think we're reasonably confident that gwt-servlet is behaving, at least for GWT-RPC use cases. Two testing questions from production apps:

  • Anyone using the logging service, and server-side stack de-obfuscation: does this work correctly?
  • Anyone using RequestFactory, with either requestfactory-server.jar or gwt-servlet.jar (and please specify which): does this work correctly?

@DeltaPi-Dev
Copy link

DeltaPi-Dev commented Nov 3, 2023

@niloc132:
The remote logging service works for us in our JEE10 / RPC / gwt-servlet-jakarta setup.

I was also able to test the StackTraceDeobfuscator with the following setup in app gwt.xml and by using the GWT compilation option -deploy so that the source maps can be found on the server:
<set-property name="compiler.useSourceMaps" value="true"/>

Here comes an example server side stacktrace received from the client:

ERROR ch.rsnag.cm.simulator.client.templates.partner.PartnerMasterData - *** GWT exception test
com.google.gwt.core.shared.SerializableThrowable: just some test
	at java.lang.Throwable.Throwable(com/google/gwt/emul/java/lang/Throwable.java:68) ~[?:?]
	at java.lang.Exception.Exception(com/google/gwt/emul/java/lang/Exception.java:29) ~[?:?]
	at java.lang.RuntimeException.RuntimeException(com/google/gwt/emul/java/lang/RuntimeException.java:29) ~[?:?]
	at com/google/gwt/emul/java/lang/IllegalArgumentException.java.(com/google/gwt/emul/java/lang/IllegalArgumentException.java:29) ~[?:?]
	at ch/rsnag/cm/simulator/client/templates/partner/PartnerMasterData.java.(ch/rsnag/cm/simulator/client/templates/partner/PartnerMasterData.java:237) ~[?:?]
	at ch.rsnag.cm.simulator.client.SimulatorContentDispatcher.$dispatch(ch/rsnag/cm/simulator/client/SimulatorContentDispatcher.java:74) ~[classes/:?]
	at ch.rsnag.cm.simulator.client.Simulator$5.onValueChange(ch/rsnag/cm/simulator/client/Simulator.java:128) ~[classes/:?]
	at com.google.gwt.event.logical.shared.ValueChangeEvent.dispatch(com/google/gwt/event/logical/shared/ValueChangeEvent.java:128) ~[gwt-servlet-jakarta-2.11.0-jakarta-SNAPSHOT.jar:?]
	at com.google.web.bindery.event.shared.SimpleEventBus.$doFire(com/google/gwt/event/shared/GwtEvent.java:76) ~[gwt-servlet-jakarta-2.11.0-jakarta-SNAPSHOT.jar:?]
[...]

Looks good to me.
But it's the first time I made this working, so I do not know if that used to look differently.

niloc132 pushed a commit to gwtproject/tools that referenced this pull request Nov 14, 2023
@dodgex
Copy link
Contributor

dodgex commented Nov 14, 2023

  • Anyone using RequestFactory, with either requestfactory-server.jar or gwt-servlet.jar (and please specify which): does this work correctly?

Last time I tested the requestfactory jars where not ready for jakarta, but I can see from the changes in the PR that this should have changed. While I also can't currently test those new jars, I at least can confirm, that back then requestfactory with the gwt-servlet.jar with the jakarta classes did work for me.

When things go well, I think I get to the point of working on/testing the gwt changes again in about 2 weeks. But thats sadly nothing I would bet on.

As long there are no other voices, maybe onsider this "ready" as soon as you think and prepare for a 2.11.1 release with fixes if anyone encounters issues.

@codeart1st
Copy link

Among other testing, we should verify that this is all working with servlet-api 6.0, not just 5.0

Our migration test was with servlet-api 6.0 Jars in classpath and WildFly 29

@niloc132 niloc132 merged commit f439c5e into gwtproject:main Dec 6, 2023
3 checks passed
@DRLovell
Copy link

Thanks very much for all your work on this! I've seen 2.11.0 has been released - brilliant!

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.

No support for Jakarta EE 9
8 participants