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

Allow to use @Inject instead of @Context with CDI #4749

Merged
merged 3 commits into from
Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 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
Expand Down Expand Up @@ -30,6 +30,7 @@

import org.glassfish.jersey.internal.BootstrapBag;
import org.glassfish.jersey.internal.BootstrapConfigurator;
import org.glassfish.jersey.internal.PropertiesDelegate;
import org.glassfish.jersey.internal.inject.AbstractBinder;
import org.glassfish.jersey.internal.inject.InjectionManager;
import org.glassfish.jersey.process.internal.RequestScoped;
Expand Down Expand Up @@ -132,7 +133,7 @@ protected void configure() {

// Bind proxiable HttpHeaders, Request and ContainerRequestContext injection injection points
bindFactory(ContainerRequestFactory.class)
.to(HttpHeaders.class).to(Request.class)
.to(HttpHeaders.class).to(Request.class).to(PropertiesDelegate.class)
.proxy(true).proxyForSameScope(false)
.in(RequestScoped.class);

Expand Down
50 changes: 50 additions & 0 deletions ext/cdi/jersey-cdi-inject/pom.xml
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>
Copy link
Contributor

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.

Copy link
Contributor Author

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 about jersey-cdi-context-inject, but I am not sure it is any better. Do you have any particular name in mind?

Copy link
Contributor Author

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.

Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Member

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. :-)

Copy link
Contributor Author

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.

Copy link
Member

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:

For names of Jakarta EE projects, specifications, test suites, and other related materials, the full project name/title and any scope statement SHOULD not incorporate an identified acronym.

The document even allows the use of the "old" acronyms explicitly for technical stuff:

A URL, repository name, package name, class name, or method name MAY include the acronym to the right of a string identifying Eclipse or Jakarta as the origin.

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. :-)

<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>
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* 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 java.lang.reflect.Type;
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 Type baseType = processAnnotatedType.getAnnotatedType().getBaseType();
if (Application.class.isAssignableFrom((Class<?>) baseType)
&& Configuration.class.isAssignableFrom((Class<?>) baseType)) {
if (!((Class<?>) baseType).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(javax.ws.rs.ext.Providers.class);
injectables.add(Request.class);
injectables.add(ResourceContext.class);
injectables.add(ResourceInfo.class);
injectables.add(SecurityContext.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);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
org.glassfish.jersey.ext.cdi1x.inject.internal.InjectExtension
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2014, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2014, 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
Expand All @@ -19,11 +19,14 @@
import java.lang.annotation.Annotation;
import java.util.Iterator;
import java.util.NoSuchElementException;
import java.util.Set;

import javax.enterprise.context.spi.Contextual;
import javax.enterprise.context.spi.CreationalContext;
import javax.enterprise.inject.spi.AnnotatedType;
import javax.enterprise.inject.spi.Bean;
import javax.enterprise.inject.spi.BeanManager;
import javax.enterprise.inject.spi.InjectionPoint;
import javax.enterprise.inject.spi.InjectionTarget;
import javax.enterprise.inject.spi.InjectionTargetFactory;

Expand Down Expand Up @@ -83,11 +86,17 @@ public void preDestroy(final T instance) {
public T getInstance(final Class<T> clazz) {
final CreationalContext<T> creationalContext = beanManager.createCreationalContext(null);
final T instance = injectionTarget.produce(creationalContext);
injectionTarget.inject(instance, creationalContext);
if (injectionManager != null) {
injectionManager.inject(instance, CdiComponentProvider.CDI_CLASS_ANALYZER);
}
injectionTarget.postConstruct(instance);
final CdiComponentProvider cdiComponentProvider = beanManager.getExtension(CdiComponentProvider.class);
final CdiComponentProvider.InjectionManagerInjectedCdiTarget hk2managedTarget =
cdiComponentProvider.new InjectionManagerInjectedCdiTarget(injectionTarget) {
@Override
public Set<InjectionPoint> getInjectionPoints() {
return injectionTarget.getInjectionPoints();
}
};
hk2managedTarget.setInjectionManager(injectionManager);
hk2managedTarget.inject(instance, creationalContext);
hk2managedTarget.postConstruct(instance);
return instance;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2020 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2021 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018 Payara Foundation and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
Expand Down Expand Up @@ -29,10 +29,12 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand Down Expand Up @@ -110,6 +112,8 @@ public class CdiComponentProvider implements ComponentProvider, Extension {
private final Set<Type> jaxrsInjectableTypes = new HashSet<>();
private final Set<Type> hk2ProvidedTypes = Collections.synchronizedSet(new HashSet<Type>());
private final Set<Type> jerseyVetoedTypes = Collections.synchronizedSet(new HashSet<Type>());
private final Set<DependencyPredicate> jerseyOrDependencyTypes = Collections.synchronizedSet(new LinkedHashSet<>());
private final ThreadLocal<InjectionManager> threadInjectionManagers = new ThreadLocal<>();

/**
* set of request scoped components
Expand Down Expand Up @@ -138,6 +142,7 @@ public Boolean apply(final Class<?> clazz) {
public CdiComponentProvider() {
customHk2TypesProvider = CdiUtil.lookupService(Hk2CustomBoundTypesProvider.class);
injectionManagerStore = CdiUtil.createHk2InjectionManagerStore();
addHK2DepenendencyCheck(CdiComponentProvider::isJerseyOrDependencyType);
}

@Override
Expand Down Expand Up @@ -449,7 +454,7 @@ private Set<InjectionPoint> filterHk2InjectionPointsOut(final Set<InjectionPoint
} else {
if (injectedType instanceof Class<?>) {
final Class<?> injectedClass = (Class<?>) injectedType;
if (isJerseyOrDependencyType(injectedClass)) {
if (testDependencyType(injectedClass)) {
//remember the type, we would need to mock it's CDI binding at runtime
hk2ProvidedTypes.add(injectedType);
} else {
Expand Down Expand Up @@ -598,6 +603,15 @@ private static boolean isJerseyOrDependencyType(final Class<?> clazz) {
&& !pkgName.startsWith("com.sun.jersey.tests")));
}

private boolean testDependencyType(Class<?> clazz) {
for (Predicate<Class<?>> predicate : jerseyOrDependencyTypes) {
if (predicate.test(clazz)) {
return true;
}
}
return false;
}

private void bindHk2ClassAnalyzer() {
ClassAnalyzer defaultClassAnalyzer =
injectionManager.getInstance(ClassAnalyzer.class, ClassAnalyzer.DEFAULT_IMPLEMENTATION_NAME);
Expand Down Expand Up @@ -628,7 +642,7 @@ private StringBuilder listElements(final StringBuilder logMsgBuilder, final Coll
}

@SuppressWarnings("unchecked")
private abstract class InjectionManagerInjectedCdiTarget implements InjectionManagerInjectedTarget {
/* package */ abstract class InjectionManagerInjectedCdiTarget implements InjectionManagerInjectedTarget {

private final InjectionTarget delegate;
private volatile InjectionManager effectiveInjectionManager;
Expand All @@ -642,16 +656,19 @@ public InjectionManagerInjectedCdiTarget(InjectionTarget delegate) {

@Override
public void inject(final Object t, final CreationalContext cc) {
delegate.inject(t, cc);

InjectionManager injectingManager = getEffectiveInjectionManager();
if (injectingManager == null) {
injectingManager = effectiveInjectionManager;
threadInjectionManagers.set(injectingManager);
}

delegate.inject(t, cc); // here the injection manager is used in HK2Bean

if (injectingManager != null) {
injectingManager.inject(t, CdiComponentProvider.CDI_CLASS_ANALYZER);
}

threadInjectionManagers.remove();
}

@Override
Expand Down Expand Up @@ -705,7 +722,12 @@ public boolean isNullable() {

@Override
public Object create(final CreationalContext creationalContext) {
return getEffectiveInjectionManager().getInstance(t);
InjectionManager injectionManager = getEffectiveInjectionManager();
if (injectionManager == null) {
injectionManager = threadInjectionManagers.get();
}

return injectionManager.getInstance(t);
}

@Override
Expand Down Expand Up @@ -824,5 +846,39 @@ private void beforeBeanDiscovery(@Observes final BeforeBeanDiscovery beforeBeanD
"Jersey " + ProcessJAXRSAnnotatedTypes.class.getName()
);
}

/**
* Add a predicate to test HK2 dependency to create a CDI bridge bean to HK2 for it.
* @param predicate to test whether given class is a HK2 dependency.
*/
public void addHK2DepenendencyCheck(Predicate<Class<?>> predicate) {
jerseyOrDependencyTypes.add(new DependencyPredicate(predicate));
}

private final class DependencyPredicate implements Predicate<Class<?>> {
private final Predicate<Class<?>> predicate;

public DependencyPredicate(Predicate<Class<?>> predicate) {
this.predicate = predicate;
}

@Override
public boolean test(Class<?> aClass) {
return predicate.test(aClass);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
DependencyPredicate that = (DependencyPredicate) o;
return predicate.getClass().equals(that.predicate);
}

@Override
public int hashCode() {
return predicate.getClass().hashCode();
}
}
}

Loading