-
Notifications
You must be signed in to change notification settings - Fork 358
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
Allow to use @Inject instead of @Context with CDI #4749
Merged
Merged
Changes from 2 commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<!-- | ||
Copyright (c) 2021 Oracle and/or its affiliates. All rights reserved. | ||
|
||
This program and the accompanying materials are made available under the | ||
terms of the Eclipse Public License v. 2.0, which is available at | ||
http://www.eclipse.org/legal/epl-2.0. | ||
|
||
This Source Code may also be made available under the following Secondary | ||
Licenses when the conditions for such availability set forth in the | ||
Eclipse Public License v. 2.0 are satisfied: GNU General Public License, | ||
version 2 with the GNU Classpath Exception, which is available at | ||
https://www.gnu.org/software/classpath/license.html. | ||
|
||
SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 | ||
|
||
--> | ||
<project xmlns="http://maven.apache.org/POM/4.0.0" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
<parent> | ||
<artifactId>project</artifactId> | ||
<groupId>org.glassfish.jersey.ext.cdi</groupId> | ||
<version>2.34-SNAPSHOT</version> | ||
</parent> | ||
<modelVersion>4.0.0</modelVersion> | ||
|
||
<artifactId>jersey-cdi-inject</artifactId> | ||
<name>jersey-ext-cdi-inject</name> | ||
<description>Allow to annotate by Inject instead of Context</description> | ||
|
||
<dependencies> | ||
<dependency> | ||
<groupId>org.glassfish.jersey.core</groupId> | ||
<artifactId>jersey-common</artifactId> | ||
<version>${project.version}</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.glassfish.jersey.ext.cdi</groupId> | ||
<artifactId>jersey-cdi1x</artifactId> | ||
<version>${project.version}</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>javax.enterprise</groupId> | ||
<artifactId>cdi-api</artifactId> | ||
<scope>provided</scope> | ||
</dependency> | ||
</dependencies> | ||
|
||
</project> |
114 changes: 114 additions & 0 deletions
114
...-inject/src/main/java/org/glassfish/jersey/ext/cdi1x/inject/internal/InjectExtension.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
/* | ||
* Copyright (c) 2021 Oracle and/or its affiliates. All rights reserved. | ||
* | ||
* This program and the accompanying materials are made available under the | ||
* terms of the Eclipse Public License v. 2.0, which is available at | ||
* http://www.eclipse.org/legal/epl-2.0. | ||
* | ||
* This Source Code may also be made available under the following Secondary | ||
* Licenses when the conditions for such availability set forth in the | ||
* Eclipse Public License v. 2.0 are satisfied: GNU General Public License, | ||
* version 2 with the GNU Classpath Exception, which is available at | ||
* https://www.gnu.org/software/classpath/license.html. | ||
* | ||
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 | ||
*/ | ||
|
||
package org.glassfish.jersey.ext.cdi1x.inject.internal; | ||
|
||
import org.glassfish.jersey.ext.cdi1x.internal.CdiComponentProvider; | ||
import org.glassfish.jersey.internal.util.ReflectionHelper; | ||
import org.glassfish.jersey.internal.util.collection.LazyValue; | ||
import org.glassfish.jersey.internal.util.collection.Value; | ||
import org.glassfish.jersey.internal.util.collection.Values; | ||
|
||
import javax.enterprise.event.Observes; | ||
import javax.enterprise.inject.Alternative; | ||
import javax.enterprise.inject.spi.BeanManager; | ||
import javax.enterprise.inject.spi.BeforeBeanDiscovery; | ||
import javax.enterprise.inject.spi.Extension; | ||
import javax.enterprise.inject.spi.ProcessAnnotatedType; | ||
import javax.ws.rs.container.ContainerRequestContext; | ||
import javax.ws.rs.container.ResourceContext; | ||
import javax.ws.rs.container.ResourceInfo; | ||
import javax.ws.rs.core.Application; | ||
import javax.ws.rs.core.Configuration; | ||
import javax.ws.rs.core.HttpHeaders; | ||
import javax.ws.rs.core.Request; | ||
import javax.ws.rs.core.SecurityContext; | ||
import javax.ws.rs.core.UriInfo; | ||
import javax.ws.rs.ext.ParamConverterProvider; | ||
import javax.ws.rs.ext.Providers; | ||
import javax.ws.rs.sse.Sse; | ||
import java.security.AccessController; | ||
import java.util.HashSet; | ||
import java.util.Set; | ||
|
||
/** | ||
* <p> | ||
* A utility class that makes sure {@code @Inject} can be used instead of {@code @Context} for the Jakarta REST API classes | ||
* and interfaces, such as for {@code Configuration}, or {@code Providers}. | ||
* </p> | ||
* <p> | ||
* Note that {@code ContextResolver} can be injected using {@code @Context}, but the Jakarta REST specification does not require | ||
* the implementation to be capable of doing so. Since {@code ContextResolver} is parametrized type, the injection using CDI's | ||
* {@Inject} is not supported. The {@code ContextResolver} can be obtained from {@code Providers}. | ||
* </p> | ||
*/ | ||
@SuppressWarnings("unused") | ||
class InjectExtension implements Extension { | ||
private void processAnnotatedType(@Observes ProcessAnnotatedType<?> processAnnotatedType, BeanManager beanManager) { | ||
final Class<?> baseClass = (Class<?>) processAnnotatedType.getAnnotatedType().getBaseType(); | ||
if (Application.class.isAssignableFrom(baseClass) && Configuration.class.isAssignableFrom(baseClass)) { | ||
if (!baseClass.isAnnotationPresent(Alternative.class)) { | ||
processAnnotatedType.veto(); // Filter bean annotated ResourceConfig | ||
} | ||
} | ||
} | ||
|
||
private void beforeDiscoveryObserver(@Observes final BeforeBeanDiscovery bbf, final BeanManager beanManager) { | ||
final CdiComponentProvider cdiComponentProvider = beanManager.getExtension(CdiComponentProvider.class); | ||
cdiComponentProvider.addHK2DepenendencyCheck(InjectExtension::isHK2Dependency); | ||
} | ||
|
||
private static final boolean isHK2Dependency(Class<?> clazz) { | ||
return JERSEY_BOUND_INJECTABLES.get().contains(clazz); | ||
} | ||
|
||
private static final LazyValue<Set<Class<?>>> JERSEY_BOUND_INJECTABLES | ||
= Values.lazy((Value<Set<Class<?>>>) () -> sumNonJerseyBoundInjectables()); | ||
|
||
private static Set<Class<?>> sumNonJerseyBoundInjectables() { | ||
final Set<Class<?>> injectables = new HashSet<>(); | ||
|
||
//JAX-RS | ||
injectables.add(Application.class); | ||
injectables.add(Configuration.class); | ||
injectables.add(ContainerRequestContext.class); | ||
jansupol marked this conversation as resolved.
Show resolved
Hide resolved
|
||
injectables.add(HttpHeaders.class); | ||
injectables.add(ParamConverterProvider.class); | ||
jansupol marked this conversation as resolved.
Show resolved
Hide resolved
|
||
injectables.add(Providers.class); | ||
injectables.add(Request.class); | ||
injectables.add(ResourceContext.class); | ||
injectables.add(ResourceInfo.class); | ||
injectables.add(SecurityContext.class); | ||
injectables.add(Sse.class); | ||
injectables.add(UriInfo.class); | ||
|
||
jansupol marked this conversation as resolved.
Show resolved
Hide resolved
|
||
//Servlet if available | ||
addOptionally("javax.servlet.http.HttpServletRequest", injectables); | ||
addOptionally("javax.servlet.http.HttpServletResponse", injectables); | ||
addOptionally("javax.servlet.ServletConfig", injectables); | ||
addOptionally("javax.servlet.ServletContext", injectables); | ||
addOptionally("javax.servlet.FilterConfig", injectables); | ||
|
||
return injectables; | ||
} | ||
|
||
private static void addOptionally(String className, Set<Class<?>> set) { | ||
final Class<?> optionalClass = AccessController.doPrivileged(ReflectionHelper.classForNamePA(className)); | ||
if (optionalClass != null) { | ||
set.add(optionalClass); | ||
} | ||
} | ||
} |
1 change: 1 addition & 0 deletions
1
...sey-cdi-inject/src/main/resources/META-INF/services/javax.enterprise.inject.spi.Extension
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
org.glassfish.jersey.ext.cdi1x.inject.internal.InjectExtension |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be nice to get the word
context
in the artifact ID. CDI is already part of the group ID.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the word
cdi
corresponds to other modules, all of them have cdi1x in the artifact id. The 1x is no longer used though, we use 2x (and even 3x in Jersey 3), so I left the 1x behind. I thought aboutjersey-cdi-context-inject
, but I am not sure it is any better. Do you have any particular name in mind?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upon further consideration,
jersey-cdi-jaxrs-inject
seems to be a more descriptive name.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jansupol as "jaxrs" is the old Java EE name, what about considering the new Jakarta name? E.g. "Jakarta Rest" or just "rest" here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arjantijms Thank you to chime in. The naming conventions seem to be the hardest on the overall work. I agree that "jaxrs" is not the best we could use in Jakarta EE. For Jersey 2.x it is far more descriptive than any other name I heard. For 3.x, changing to "rest" sounds a bit ambiguous. One can create REST endpoints with JAX-WS, for instance. "jakarta-rest" on the other hand sounds too long. Ideally, the name should be the same for 2.x and 3.x.
jersey-cdi-rs-inject
perhaps? I am open to any suggestions.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jersey-cdi-rs-inject
it is.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JAX-RS is neither old nor new nor forbidden or obsolete. Most people understand what JAX-RS means and clearly the technology itself is mostly referred to still as JAX-RS. As I explained several times, it is officially still allowed and legal to keep using the word JAX-RS anywhere. One the specification title (hence, reference to the specification, not the technology) must be named "Jakarta RESTful Web Services". So it would have been absolutely ok that you keep "jaxrs" in the name if you like to do that. Just to make things clear, as there is much FUD about this. :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is likely for a discussion with a larger audience. The official Jakarta documents do not like JAX prefix. I know you keep saying you have the approval, but no one has seen it. No matter the legal stuff, the javax based vs jakarta based Jersey differences are better explained to the customers when each has a different name, JAX-RS for Java EE 8, Jakarta REST for Jakarta EE 9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't get me wrong - I am totally OK with your decision. I just wanted to make clear that there is absoutely no legal reason that forces you to get rid of the word "JAX-RS". The document you correctly refer to just says "SHOULD" not "MUST", and it clearly asks for the change only for documents etc, but not for source code and other technical stuff:
The document even allows the use of the "old" acronyms explicitly for technical stuff:
It is completely up to the Jersey committers to decide whether they still use the acronym JAX-RS or not. No instance at the Eclipse Foundation or at Oracle explicitly forbids it. The document you refer to makes this 100% clear, so I am not sure why you say, "no one has seen it": It is written THERE. THAT document IS the final answer. So there is no more FUD. :-)