Skip to content

Commit

Permalink
Correctly handle REST field injection
Browse files Browse the repository at this point in the history
This is done by converting the Resource class to
@RequestScoped when possible and failing the build
when it's not
  • Loading branch information
geoand committed Feb 17, 2025
1 parent 620f645 commit b975725
Show file tree
Hide file tree
Showing 14 changed files with 538 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import java.util.Map;
import java.util.function.Predicate;

import jakarta.enterprise.context.RequestScoped;
import jakarta.enterprise.inject.spi.DeploymentException;
import jakarta.ws.rs.core.MediaType;

import org.jboss.jandex.AnnotationInstance;
Expand All @@ -28,6 +30,7 @@
import org.jboss.resteasy.reactive.server.processor.ServerIndexedParameter;
import org.jboss.resteasy.reactive.server.spi.EndpointInvokerFactory;

import io.quarkus.arc.processor.BuiltinScope;
import io.quarkus.builder.BuildException;
import io.quarkus.deployment.Capabilities;
import io.quarkus.deployment.annotations.BuildProducer;
Expand Down Expand Up @@ -274,4 +277,24 @@ protected void warnAboutMissUsedBodyParameter(DotName httpMethod, MethodInfo met
super.warnAboutMissUsedBodyParameter(httpMethod, methodInfo);
}

/**
* At this point we know exactly which resources will require field injection and therefore are required to be
* {@link RequestScoped}.
* We can't change anything CDI related at this point (because it would create build cycles), so all we can do
* is fail the build if the resource has not already been handled automatically (by the best effort approach performed
* elsewhere)
* or it's not manually set to be {@link RequestScoped}.
*/
@Override
protected void verifyClassThatRequiresFieldInjection(ClassInfo classInfo) {
if (!alreadyHandledRequestScopedResources.contains(classInfo.name())) {
BuiltinScope scope = BuiltinScope.from(classInfo);
if (BuiltinScope.REQUEST != scope) {
throw new DeploymentException(
"Resource classes that use field injection for REST parameters can only be @RequestScoped. Offending class is "
+ classInfo.name());
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,6 @@
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.CONTEXT;
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.PATH;
import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.PROVIDER;

import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import jakarta.ws.rs.BeanParam;

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.AnnotationTarget;
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.DotName;
import org.jboss.jandex.MethodInfo;
import org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames;
import org.jboss.resteasy.reactive.common.processor.scanning.ResourceScanningResult;
import org.jboss.resteasy.reactive.server.injection.ContextProducers;
import org.jboss.resteasy.reactive.server.processor.util.ResteasyReactiveServerDotNames;

import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem;
import io.quarkus.arc.deployment.AutoInjectAnnotationBuildItem;
Expand All @@ -41,6 +21,25 @@
import io.quarkus.resteasy.reactive.server.spi.SubResourcesAsBeansBuildItem;
import io.quarkus.resteasy.reactive.spi.DynamicFeatureBuildItem;
import io.quarkus.resteasy.reactive.spi.JaxrsFeatureBuildItem;
import jakarta.enterprise.context.RequestScoped;
import jakarta.enterprise.inject.spi.DeploymentException;
import jakarta.ws.rs.BeanParam;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.AnnotationTarget;
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.DotName;
import org.jboss.jandex.MethodInfo;
import org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames;
import org.jboss.resteasy.reactive.common.processor.scanning.ResourceScanningResult;
import org.jboss.resteasy.reactive.server.injection.ContextProducers;
import org.jboss.resteasy.reactive.server.processor.util.ResteasyReactiveServerDotNames;

public class ResteasyReactiveCDIProcessor {

Expand All @@ -67,6 +66,47 @@ void beanDefiningAnnotations(BuildProducer<BeanDefiningAnnotationBuildItem> bean
BuiltinScope.SINGLETON.getName()));
}

/**
* The idea here is to make a best effort to find resources that need to be {@link RequestScoped}
* and make them such if no scope has been defined.
* If any other scope has been explicitly defined, the build will fail
*/
@BuildStep
void requestScopedResources(Optional<ResourceScanningResultBuildItem> resourceScanningResultBuildItem,
BuildProducer<AnnotationsTransformerBuildItem> additionalBeanBuildItemBuildProducer) {
if (resourceScanningResultBuildItem.isEmpty()) {
return;
}
Set<DotName> requestScopedResources = resourceScanningResultBuildItem.get().getResult()
.getRequestScopedResources();

additionalBeanBuildItemBuildProducer.produce(new AnnotationsTransformerBuildItem(new AnnotationsTransformer() {
@Override
public boolean appliesTo(AnnotationTarget.Kind kind) {
return kind == AnnotationTarget.Kind.CLASS;
}

@Override
public void transform(TransformationContext transformationContext) {
ClassInfo clazz = transformationContext.getTarget().asClass();
if (requestScopedResources.contains(clazz.name())) {
BuiltinScope builtinScope = BuiltinScope.from(clazz);
if (builtinScope != null) {
if (builtinScope.getName() != BuiltinScope.REQUEST.getName()) {
throw new DeploymentException(
"Resource classes that use field injection for REST parameters can only be @RequestScoped. Offending class is "
+ clazz.name());
} else {
// nothing to do as @RequestScoped was already present
}
} else {
transformationContext.transform().add(RequestScoped.class).done();
}
}
}
}));
}

@BuildStep
void unremovableContextMethodParams(Optional<ResourceScanningResultBuildItem> resourceScanningResultBuildItem,
BuildProducer<UnremovableBeanBuildItem> producer) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,8 @@ public Supplier<Boolean> apply(ClassInfo classInfo) {
boolean disableIfMissing = disableIfMissingValue != null && disableIfMissingValue.asBoolean();
return recorder.disableIfPropertyMatches(propertyName, propertyValue, disableIfMissing);
}
});
})
.alreadyHandledRequestScopedResources(result.getRequestScopedResources());

if (!serverDefaultProducesHandlers.isEmpty()) {
List<DefaultProducesHandler> handlers = new ArrayList<>(serverDefaultProducesHandlers.size());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package io.quarkus.resteasy.reactive.server.test.injection;

import jakarta.enterprise.inject.spi.DeploymentException;
import jakarta.inject.Singleton;
import jakarta.ws.rs.FormParam;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;

public class FormFieldSingletonScopeTest {

@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.withApplicationRoot(jar -> jar.addClasses(Resource.class))
.assertException(t -> {
org.junit.jupiter.api.Assertions.assertEquals(DeploymentException.class, t.getClass());
});

@Test
public void test() {
Assertions.fail("should never have run");
}

@Path("/test")
@Singleton
public static class Resource {

@FormParam("foo")
String foo;

@GET
@Produces(MediaType.TEXT_PLAIN)
public String hello() {
return "foo: " + foo;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package io.quarkus.resteasy.reactive.server.test.injection;

import jakarta.enterprise.context.Dependent;
import jakarta.enterprise.inject.spi.DeploymentException;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.HeaderParam;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;

import org.assertj.core.api.Assertions;
import org.jboss.resteasy.reactive.RestHeader;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;

public class HeaderFieldInSuperClassDependentScopeTest {

@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.withApplicationRoot(jar -> jar.addClasses(AbstractResource.class, Resource.class))
.assertException(t -> {
org.junit.jupiter.api.Assertions.assertEquals(DeploymentException.class, t.getClass());
});

@Test
public void test() {
Assertions.fail("should never have run");
}

@Path("/test")
@Dependent
public static class Resource extends AbstractResource {

@GET
@Produces(MediaType.TEXT_PLAIN)
public String hello() {
return "foo: " + foo + ", bar: " + bar;
}
}

public static class AbstractResource {
@HeaderParam("foo")
String foo;

@RestHeader
String bar;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package io.quarkus.resteasy.reactive.server.test.injection;

import static io.restassured.RestAssured.given;
import static io.restassured.RestAssured.when;
import static org.hamcrest.CoreMatchers.is;

import jakarta.enterprise.context.RequestScoped;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.HeaderParam;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;

import org.jboss.resteasy.reactive.RestHeader;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;

public class HeaderFieldInSuperClassNoScopeTest {

@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.withApplicationRoot(jar -> jar.addClasses(AbstractResource.class, AbstractAbstractResource.class, Resource.class));

@Test
public void test() {
given()
.header("foo", "f")
.header("bar", "b")
.when()
.get("/test")
.then()
.statusCode(200)
.body(is("foo: f, bar: b"));

when()
.get("/test")
.then()
.statusCode(200)
.body(is("foo: null, bar: null"));
}

@Path("/test")
@RequestScoped
public static class Resource extends AbstractAbstractResource {

@GET
@Produces(MediaType.TEXT_PLAIN)
public String hello() {
return "foo: " + foo + ", bar: " + bar;
}
}

public static class AbstractResource {
@HeaderParam("foo")
String foo;

@RestHeader
String bar;
}

public static class AbstractAbstractResource extends AbstractResource {

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package io.quarkus.resteasy.reactive.server.test.injection;

import static io.restassured.RestAssured.given;
import static io.restassured.RestAssured.when;
import static org.hamcrest.CoreMatchers.is;

import jakarta.enterprise.context.RequestScoped;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.HeaderParam;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;

import org.jboss.resteasy.reactive.RestHeader;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;

public class HeaderFieldInSuperClassRequestScopeTest {

@RegisterExtension
static QuarkusUnitTest runner = new QuarkusUnitTest()
.withApplicationRoot(jar -> jar.addClasses(AbstractResource.class, Resource.class));

@Test
public void test() {
given()
.header("foo", "f")
.header("bar", "b")
.when()
.get("/test")
.then()
.statusCode(200)
.body(is("foo: f, bar: b"));

when()
.get("/test")
.then()
.statusCode(200)
.body(is("foo: null, bar: null"));
}

@Path("/test")
@RequestScoped
public static class Resource extends AbstractResource {

@GET
@Produces(MediaType.TEXT_PLAIN)
public String hello() {
return "foo: " + foo + ", bar: " + bar;
}
}

public static class AbstractResource {
@HeaderParam("foo")
String foo;

@RestHeader
String bar;
}
}
Loading

0 comments on commit b975725

Please sign in to comment.