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

Upgrade to Jersey 2.37 and Java 11 #471

Merged
merged 21 commits into from
Nov 14, 2022
Merged

Upgrade to Jersey 2.37 and Java 11 #471

merged 21 commits into from
Nov 14, 2022

Conversation

abcpro1
Copy link
Contributor

@abcpro1 abcpro1 commented Nov 12, 2022

This is an upgrade to Java 11 and Jersey 2.37.

Resolves #380.

Commits are well organized and described, and most changes are easy to follow.

Feel free to ask questions.

The main reference for jersey migration is Migrating from Jersey 1.x to 2.0, but various other references were useful to me, including reading the source code and documentation of Jersey 1.x and 2.x and other dependency packages, in addition to stackoverflow.com.

Finally, to catch and solve a memory leak in Spring; Analysing the heap dump gave a good insight, which helped finding the cause with the help of a debugger, then crafting a solution.


IssueHunt Summary

Referenced issues

This pull request has been submitted to:


POM files for core, lite, and full are upgraded to java 11 and Jersey 2.37.
A few dependencies have been renamed from Jersey 1 to Jersey 2.
A few migrations are said to be simple since they have direct replacements
in Jersey 2 from Jersey 1.

This list includes:

- ServletContainer replaces SpringServlet.
- MultivaluedHashMap replaces MultivaluedMapImpl.
- ClientBuilder.newClient() replaces Client.create().
- WebTarget replaces WebResource.
- LoggingFeature replaces LoggingFilter

As a general reference: https://eclipse-ee4j.github.io/jersey.github.io/documentation/latest/migration.html#mig-1.x
The parameter name has changed from "com.sun.jersey.config.property.packages"
to "jersey.config.server.provider.packages". This parameter defines
the root packages that would be scanned for resources and providers.

The new name is also defined in the constant ServerProperties.PROVIDER_PACKAGES.

Reference: https://eclipse-ee4j.github.io/jersey.github.io/apidocs/2.37/jersey/org/glassfish/jersey/server/ServerProperties.html#PROVIDER_PACKAGES
There is no equivalent to the method queryParams() in WebTarget, but there is
the method queryParam() which adds a single query parameter.
The solution is to add parameters one at a time.

Reference: https://jakartaee.github.io/rest/apidocs/2.1.6/javax/ws/rs/client/WebTarget.html#queryParam(java.lang.String,java.lang.Object...)
The signature has changed for the method ContainerRequestFilter.filter(),
and the ResourceFilter interface along with the ResourceFilters annotation
no longer exist in Jersey 2.

A future commit will address the ResourceFilters annotation for filters
registration.

Reference: https://jakartaee.github.io/rest/apidocs/2.1.6/javax/ws/rs/container/ContainerRequestFilter.html#filter(javax.ws.rs.container.ContainerRequestContext)
There is no direct equivalent of the @ResourceFilters annotation in Jersey 2.
The best alternative to define a custom annotation and use a DynamicFeature provider
to interpret this annotation and register filters.

Why not define a annotation for each filter and use Jersey's @namebinding mechanism?
@ResourceFilters works differently than @namebinding. A @ResourceFilters annotation
on method overrides the class level annotation; In this case, all filters
defined on the class level are discarded. This behaviour can't be achieved
with @namebinding.

References:
https://eclipse-ee4j.github.io/jersey.github.io/documentation/2.37/filters-and-interceptors.html#d0e10127
https://jakartaee.github.io/rest/apidocs/2.1.6/javax/ws/rs/container/DynamicFeature.html
It is a good practice to define priorities for request filters; Otherwise,
the execution order of filters with the same priority is implementation-defined.

References:
https://eclipse-ee4j.github.io/jersey.github.io/documentation/2.37/filters-and-interceptors.html#d0e10195
https://jakartaee.github.io/rest/apidocs/2.1.6/javax/ws/rs/Priorities.html
The exception NotFoundException no longer stores the missing resource URI;
However the latter can is accessible at any time during the request from
the request context.

References:
https://jakartaee.github.io/rest/apidocs/2.1.6/javax/ws/rs/NotFoundException.html
https://jakartaee.github.io/rest/apidocs/2.1.6/javax/ws/rs/container/ContainerRequestContext.html#getUriInfo()
Replace LocaleProvider with LocaleFactory. The semantics are the same except
that Jersey 2 unlike Jersey 1 does not include a native dependency injection
framework; HK2 is the standard dependency injection framework used with Jersey 2.
In this case a class implementing the Factory<Locale> interface is the
replacement of AbstractHttpContextInjectable<Locale> from Jersey 1.

References: https://javaee.github.io/hk2/apidocs/org/glassfish/hk2/api/Factory.html
The is no direct replacement of the method ContainerRequest.getUserPrincipal()
in Jersey 2 was removed, but the logic of the original method is quite simple;
We can inline the Jersey 1.x implementation which used to be:

    if (securityContext == null)
        throw new UnsupportedOperationException();
    return securityContext.getUserPrincipal();

Reference: https://github.com/javaee/jersey-1.x/blob/1.19.3/jersey-server/src/main/java/com/sun/jersey/spi/container/ContainerRequest.java#L905-L910
When building a client request, the method WebTarget.request() must be used
to make a builder that provides the following methods:

- accept()
- header()
- method()
- get()
- post()
- put()
- delete()

Reference: https://jakartaee.github.io/rest/apidocs/2.1.6/javax/ws/rs/client/WebTarget.html#request()
This commit is a set of simple migrations for client requests made in
unittests.

The javax.ws.rs.core.Response class is the default object returned by
the request methods: get, post, put, and delete; This class can be a replacement
for com.sun.jersey.api.client.ClientResponse from Jersey 1.x.

Why not use org.glassfish.jersey.client.ClientResponse as a replacement instead?
When trying to return a org.glassfish.jersey.client.ClientResponse from the
request methods, the Jersey implementation for some reason attempts to use
the Jackson ObjectMapper to convert the do the conversion which by default will fail
with an exception.

Other simple migrations in this commit include replacing MultivaluedMap with
javax.ws.rs.core.Form for form query parameters; And conforming to the new signature
for put() and post() methods a javax.ws.rs.client.Entity is always supplied.
ProcessingException replaces ClientHandlerException.
WebApplicationException replaces UniformInterfaceException; and

WebApplicationException is only thrown when the response type *is not Response*
and the response status code is not in the 2xx range.

References:
https://jakartaee.github.io/rest/apidocs/2.1.6/javax/ws/rs/client/SyncInvoker.html#method(java.lang.String)
https://jakartaee.github.io/rest/apidocs/2.1.6/javax/ws/rs/client/SyncInvoker.html#method(java.lang.String,java.lang.Class)
Jersey parses URI query parameters in order to expand templates; A template
placeholder is an identifier enclosed in curly braces.
To be able to use curly braces in URI query parameters, an indirection is required.
The query parameter will hold a template placeholder which will expand to
the real value of the query parameter which can contain curly braces;
This value will not be expanded further.

References:
https://jakartaee.github.io/rest/apidocs/2.1.6/javax/ws/rs/client/WebTarget.html#queryParam(java.lang.String,java.lang.Object...)
https://jakartaee.github.io/rest/apidocs/2.1.6/javax/ws/rs/client/WebTarget.html#resolveTemplate(java.lang.String,java.lang.Object)
A few APIs have changed in the class JerseyTest from Jersey 1 to Jersey 2,
but direct replacements are available for most cases albeit the setup is
slightly different.

References:
https://eclipse-ee4j.github.io/jersey.github.io/apidocs/2.32/jersey/org/glassfish/jersey/test/JerseyTest.html
https://eclipse-ee4j.github.io/jersey.github.io/apidocs/2.32/jersey/org/glassfish/jersey/test/ServletDeploymentContext.html
Upgrade CI testing environment from Java 8 to Java 11.
There was a memory leak in org.springframework.web.context.ContextLoader
where objects were being inserted into a map but never removed from it.
The map is [this](https://github.com/spring-projects/spring-framework/blob/v5.3.23/spring-web/src/main/java/org/springframework/web/context/ContextLoader.java#L152-L156).

This map matches a WebApplicationContext to the current thread class loader
The insertion to the map happens during a web application deployment
[here](https://github.com/spring-projects/spring-framework/blob/v5.3.23/spring-web/src/main/java/org/springframework/web/context/ContextLoader.java#L302).
The thread class loader is set prior to this operation by
org.glassfish.grizzly.servlet.WebappContext
[here](https://github.com/eclipse-ee4j/grizzly/blob/2.4.4/modules/http-servlet/src/main/java/org/glassfish/grizzly/servlet/WebappContext.java#L2080)
Finally, when the web application is being destroyed; The WebApplicationContext
inserted earlier is supposed to be removed from the map
[here](https://github.com/spring-projects/spring-framework/blob/v5.3.23/spring-web/src/main/java/org/springframework/web/context/ContextLoader.java#L526).
But, the thread class loader this time is different from when the insertion
occurred.
This is a bug in Grizzly, as you can see
[here](https://github.com/eclipse-ee4j/grizzly/blob/2.4.4/modules/http-servlet/src/main/java/org/glassfish/grizzly/servlet/WebappContext.java#L2106-L2127)
the thread class loader is not set unlike during initialization.
This is the cause of the discrepancy of the thread class loader
between initialization and destruction.

A straightforward solution was implemented in this commit, which is to set
the thread class loader before destruction to the class loader that was
active during initialization.

In the future, this issue needs be solved in the Grizzly project.
@abcpro1 abcpro1 changed the title Upgrader to Jersey 2.37 and Java 11 Upgrade to Jersey 2.37 and Java 11 Nov 12, 2022
@kupietz
Copy link
Member

kupietz commented Nov 13, 2022

Great, thanks! We'll have a closer look tomorrow.

@kupietz
Copy link
Member

kupietz commented Nov 14, 2022

Well done, great work! Many thanks also for the detailed, well-explained and comprehensible commit steps, the additional hints and the fixing of the memory leak!

@kupietz kupietz merged commit f175e6d into KorAP:master Nov 14, 2022
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.

Update to Java 11 and migrate to Jersey >=2.37
2 participants