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 Jan 29, 2025
1 parent b6ed424 commit 4399602
Show file tree
Hide file tree
Showing 14 changed files with 514 additions and 3 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 @@ -11,12 +11,19 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Predicate;

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

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.AnnotationTarget;
import org.jboss.jandex.AnnotationTransformation;
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.Declaration;
import org.jboss.jandex.DotName;
import org.jboss.jandex.MethodInfo;
import org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames;
Expand Down Expand Up @@ -67,6 +74,51 @@ 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 io.quarkus.arc.deployment.AnnotationsTransformerBuildItem(
AnnotationTransformation.builder().whenDeclaration(
new Predicate<>() {
@Override
public boolean test(Declaration declaration) {
return declaration.kind() == AnnotationTarget.Kind.CLASS;
}
}).transform(new Consumer<>() {
@Override
public void accept(AnnotationTransformation.TransformationContext context) {
if (context.declaration().kind() == AnnotationTarget.Kind.CLASS) {
ClassInfo clazz = context.declaration().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 {
context.add(RequestScoped.class);
}
}
}
}
})));
}

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

serverEndpointIndexerBuilder.skipNotRestParameters(allowNotRestParametersBuildItem.isPresent());

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 4399602

Please sign in to comment.