Merge pull request #21817 from Sgitario/reactive_beanparam_classcast

Provide contextual error message for incorrect use of @BeanParam
quarkusio · Dec 1, 2021 · 5d8262a · 5d8262a
2 parents d1c31b6 + ce7aed3
commit 5d8262a
Show file tree

Hide file tree

Showing 6 changed files with 166 additions and 4 deletions.
diff --git a/.../resteasy/reactive/server/test/beanparam/FailWithAnnotationsInAMethodOfBeanParamTest.java b/.../resteasy/reactive/server/test/beanparam/FailWithAnnotationsInAMethodOfBeanParamTest.java
@@ -0,0 +1,54 @@
+package io.quarkus.resteasy.reactive.server.test.beanparam;
+
+import javax.enterprise.inject.spi.DeploymentException;
+import javax.ws.rs.BeanParam;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.MediaType;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusUnitTest;
+
+public class FailWithAnnotationsInAMethodOfBeanParamTest {
+
+    @RegisterExtension
+    static QuarkusUnitTest test = new QuarkusUnitTest()
+            .setExpectedException(DeploymentException.class)
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(GreetingResource.class, NoQueryParamsInFieldsNameData.class));
+
+    @Test
+    void shouldBeanParamWorkWithoutFieldsAnnotatedWithQueryParam() {
+        Assertions.fail("The test case should not be invoked as it should fail with a deployment exception.");
+    }
+
+    @Path("/greeting")
+    public static class GreetingResource {
+
+        @GET
+        @Produces(MediaType.TEXT_PLAIN)
+        public String hello(@BeanParam NoQueryParamsInFieldsNameData request) {
+            return "Hello, " + request.getName();
+        }
+    }
+
+    public static class NoQueryParamsInFieldsNameData {
+        private String name;
+
+        @QueryParam("name")
+        public String getName() {
+            return name;
+        }
+
+        public void setName(String name) {
+            this.name = name;
+        }
+    }
+}
diff --git a/...quarkus/resteasy/reactive/server/test/beanparam/FailWithNoAnnotationsInBeanParamTest.java b/...quarkus/resteasy/reactive/server/test/beanparam/FailWithNoAnnotationsInBeanParamTest.java
@@ -0,0 +1,52 @@
+package io.quarkus.resteasy.reactive.server.test.beanparam;
+
+import javax.enterprise.inject.spi.DeploymentException;
+import javax.ws.rs.BeanParam;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusUnitTest;
+
+public class FailWithNoAnnotationsInBeanParamTest {
+
+    @RegisterExtension
+    static QuarkusUnitTest test = new QuarkusUnitTest()
+            .setExpectedException(DeploymentException.class)
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(GreetingResource.class, NoQueryParamsInFieldsNameData.class));
+
+    @Test
+    void shouldBeanParamWorkWithoutFieldsAnnotatedWithQueryParam() {
+        Assertions.fail("The test case should not be invoked as it should fail with a deployment exception.");
+    }
+
+    @Path("/greeting")
+    public static class GreetingResource {
+
+        @GET
+        @Produces(MediaType.TEXT_PLAIN)
+        public String hello(@BeanParam NoQueryParamsInFieldsNameData request) {
+            return "Hello, " + request.getName();
+        }
+    }
+
+    public static class NoQueryParamsInFieldsNameData {
+        private String name;
+
+        public String getName() {
+            return name;
+        }
+
+        public void setName(String name) {
+            this.name = name;
+        }
+    }
+}
diff --git a/.../src/main/java/org/jboss/resteasy/reactive/common/processor/ResteasyReactiveDotNames.java b/.../src/main/java/org/jboss/resteasy/reactive/common/processor/ResteasyReactiveDotNames.java
@@ -231,6 +231,11 @@ public final class ResteasyReactiveDotNames {
     public static final Set<DotName> RESOURCE_CTOR_PARAMS_THAT_NEED_HANDLING = new HashSet<>(
             Arrays.asList(QUERY_PARAM, HEADER_PARAM, PATH_PARAM, MATRIX_PARAM, COOKIE_PARAM));
 
+    public static final Set<DotName> JAX_RS_ANNOTATIONS_FOR_FIELDS = new HashSet<>(
+            Arrays.asList(BEAN_PARAM, MULTI_PART_FORM_PARAM, PATH_PARAM, QUERY_PARAM, HEADER_PARAM, FORM_PARAM, MATRIX_PARAM,
+                    COOKIE_PARAM, REST_PATH_PARAM, REST_QUERY_PARAM, REST_HEADER_PARAM, REST_FORM_PARAM, REST_MATRIX_PARAM,
+                    REST_COOKIE_PARAM, CONTEXT, DEFAULT_VALUE, SUSPENDED));
+
     public static final DotName ENCODED = DotName.createSimple(Encoded.class.getName());
 
     public static final DotName QUARKUS_REST_CONTAINER_RESPONSE_FILTER = DotName

diff --git a/.../common/runtime/src/main/java/org/jboss/resteasy/reactive/common/model/BeanParamInfo.java b/.../common/runtime/src/main/java/org/jboss/resteasy/reactive/common/model/BeanParamInfo.java
@@ -3,6 +3,7 @@
 public class BeanParamInfo implements InjectableBean {
     private boolean isFormParamRequired;
     private boolean isInjectionRequired;
+    private int fieldExtractorsCount;
 
     @Override
     public boolean isFormParamRequired() {
@@ -25,4 +26,14 @@ public InjectableBean setInjectionRequired(boolean isInjectionRequired) {
         this.isInjectionRequired = isInjectionRequired;
         return this;
     }
+
+    @Override
+    public int getFieldExtractorsCount() {
+        return fieldExtractorsCount;
+    }
+
+    @Override
+    public void setFieldExtractorsCount(int fieldExtractorsCount) {
+        this.fieldExtractorsCount = fieldExtractorsCount;
+    }
 }
diff --git a/...common/runtime/src/main/java/org/jboss/resteasy/reactive/common/model/InjectableBean.java b/...common/runtime/src/main/java/org/jboss/resteasy/reactive/common/model/InjectableBean.java
@@ -8,14 +8,21 @@ public interface InjectableBean {
     /**
      * @return true if we have a FORM injectable field, either directly or in supertypes
      */
-    public boolean isFormParamRequired();
+    boolean isFormParamRequired();
 
-    public InjectableBean setFormParamRequired(boolean isFormParamRequired);
+    InjectableBean setFormParamRequired(boolean isFormParamRequired);
 
     /**
      * @return true if we have injectable fields, either directly or in supertypes
      */
-    public boolean isInjectionRequired();
+    boolean isInjectionRequired();
 
-    public InjectableBean setInjectionRequired(boolean isInjectionRequired);
+    InjectableBean setInjectionRequired(boolean isInjectionRequired);
+
+    /**
+     * @return the number of field extractors.
+     */
+    int getFieldExtractorsCount();
+
+    void setFieldExtractorsCount(int fieldExtractorsCount);
 }
diff --git a/...sor/src/main/java/org/jboss/resteasy/reactive/server/processor/ServerEndpointIndexer.java b/...sor/src/main/java/org/jboss/resteasy/reactive/server/processor/ServerEndpointIndexer.java
@@ -3,6 +3,7 @@
 import static javax.ws.rs.core.MediaType.APPLICATION_FORM_URLENCODED;
 import static javax.ws.rs.core.MediaType.APPLICATION_JSON;
 import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.DATE_FORMAT;
+import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.JAX_RS_ANNOTATIONS_FOR_FIELDS;
 import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.JSONP_JSON_ARRAY;
 import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.JSONP_JSON_NUMBER;
 import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.JSONP_JSON_OBJECT;
@@ -28,9 +29,11 @@
 import java.util.Set;
 import java.util.function.Supplier;
 import java.util.regex.PatternSyntaxException;
+import javax.enterprise.inject.spi.DeploymentException;
 import javax.ws.rs.core.MultivaluedMap;
 import javax.ws.rs.core.PathSegment;
 import org.jboss.jandex.AnnotationInstance;
+import org.jboss.jandex.AnnotationTarget;
 import org.jboss.jandex.AnnotationValue;
 import org.jboss.jandex.ClassInfo;
 import org.jboss.jandex.DotName;
@@ -164,6 +167,12 @@ protected boolean handleBeanParam(ClassInfo actualEndpointInfo, Type paramType,
         InjectableBean injectableBean = scanInjectableBean(beanParamClassInfo,
                 actualEndpointInfo,
                 existingConverters, additionalReaders, injectableBeans, hasRuntimeConverters);
+        if (injectableBean.getFieldExtractorsCount() == 0) {
+            throw new DeploymentException(String.format("No annotations found on fields at '%s'. "
+                    + "Annotations like `@QueryParam` should be used in fields, not in methods.",
+                    beanParamClassInfo.name()));
+        }
+
         return injectableBean.isFormParamRequired();
     }
 
@@ -223,6 +232,9 @@ protected InjectableBean scanInjectableBean(ClassInfo currentClassInfo, ClassInf
         currentInjectableBean = new BeanParamInfo();
         injectableBeans.put(currentTypeName, currentInjectableBean);
 
+        // validate methods
+        validateMethodsForInjectableBean(currentClassInfo);
+
         // LinkedHashMap the TCK expects that fields annotated with @BeanParam are handled last
         Map<FieldInfo, ServerIndexedParameter> fieldExtractors = new LinkedHashMap<>();
         Map<FieldInfo, ServerIndexedParameter> beanParamFields = new LinkedHashMap<>();
@@ -274,6 +286,8 @@ protected InjectableBean scanInjectableBean(ClassInfo currentClassInfo, ClassInf
             }
         }
 
+        currentInjectableBean.setFieldExtractorsCount(fieldExtractors.size());
+
         if (!fieldExtractors.isEmpty() && fieldInjectionHandler != null) {
             fieldInjectionHandler.handleFieldInjection(currentTypeName, fieldExtractors, superTypeIsInjectable);
         }
@@ -389,6 +403,25 @@ protected void handleTemporalParam(ServerIndexedParameter builder, DotName param
                 contextualizeErrorMessage("Unable to handle temporal type '" + paramType + "'", currentMethodInfo));
     }
 
+    private void validateMethodsForInjectableBean(ClassInfo currentClassInfo) {
+        for (MethodInfo method : currentClassInfo.methods()) {
+            for (AnnotationInstance annotation : method.annotations()) {
+                if (annotation.target().kind() == AnnotationTarget.Kind.METHOD) {
+                    for (DotName annotationForField : JAX_RS_ANNOTATIONS_FOR_FIELDS) {
+                        if (annotation.name().equals(annotationForField)) {
+                            throw new DeploymentException(String.format(
+                                    "Method '%s' of class '%s' is annotated with @%s annotation which is prohibited. "
+                                            + "Classes uses as @BeanParam parameters must have a JAX-RS parameter annotation on "
+                                            + "fields only.",
+                                    method.name(), currentClassInfo.name().toString(),
+                                    annotation.name().withoutPackagePrefix()));
+                        }
+                    }
+                }
+            }
+        }
+    }
+
     private String contextualizeErrorMessage(String errorMessage, MethodInfo currentMethodInfo) {
         errorMessage += ". Offending method if '" + currentMethodInfo.name() + "' of class '"
                 + currentMethodInfo.declaringClass().name() + "'";