Fail when detecting collections in Resteasy Reactive JAXB

Fix quarkusio#33865
Sgitario · Jun 22, 2023 · 20d286d · 20d286d
1 parent 94c34c2
commit 20d286d
Show file tree

Hide file tree

Showing 5 changed files with 201 additions and 12 deletions.
diff --git a/...main/java/io/quarkus/resteasy/reactive/jaxb/deployment/ResteasyReactiveJaxbProcessor.java b/...main/java/io/quarkus/resteasy/reactive/jaxb/deployment/ResteasyReactiveJaxbProcessor.java
@@ -1,5 +1,7 @@
 package io.quarkus.resteasy.reactive.jaxb.deployment;
 
+import static org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames.REST_RESPONSE;
+
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -8,6 +10,7 @@
 import java.util.Locale;
 import java.util.Set;
 
+import jakarta.enterprise.inject.spi.DeploymentException;
 import jakarta.ws.rs.Priorities;
 import jakarta.ws.rs.RuntimeType;
 import jakarta.ws.rs.core.MediaType;
@@ -19,7 +22,10 @@
 import org.jboss.jandex.FieldInfo;
 import org.jboss.jandex.IndexView;
 import org.jboss.jandex.MethodInfo;
+import org.jboss.jandex.MethodParameterInfo;
 import org.jboss.jandex.Type;
+import org.jboss.resteasy.reactive.common.model.MethodParameter;
+import org.jboss.resteasy.reactive.common.model.ParameterType;
 import org.jboss.resteasy.reactive.common.model.ResourceMethod;
 import org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames;
 
@@ -87,6 +93,13 @@ void registerClassesToBeBound(ResteasyReactiveResourceMethodEntriesBuildItem res
             if (effectiveReturnType != null) {
                 // When using "application/xml", the return type needs to be registered
                 if (producesXml(resourceInfo)) {
+                    if (!isTypeCompatibleWithJaxb(methodInfo.returnType())) {
+                        throw new DeploymentException(
+                                "Cannot directly return collections or arrays using JAXB. You need to wrap it "
+                                        + "into a root element class. Problematic method is '"
+                                        + entry.getActualClassInfo().name() + "." + methodInfo.name() + "'");
+                    }
+
                     classesInfo.add(effectiveReturnType);
                 }
 
@@ -100,8 +113,16 @@ void registerClassesToBeBound(ResteasyReactiveResourceMethodEntriesBuildItem res
             boolean consumesXml = consumesXml(resourceInfo);
             boolean consumesMultipart = consumesMultipart(resourceInfo);
             if (consumesXml || consumesMultipart) {
-                for (Type parameter : methodInfo.parameterTypes()) {
-                    ClassInfo effectiveParameter = getEffectiveClassInfo(parameter, indexView);
+                for (MethodParameterInfo parameter : methodInfo.parameters()) {
+                    if (isParameterBody(parameter, resourceInfo) && !isTypeCompatibleWithJaxb(parameter.type())) {
+                        throw new DeploymentException(
+                                "Cannot handle collections or arrays as parameters using JAXB. You need to wrap it "
+                                        + "into a root element class. Problematic parameter is '" + parameter.name()
+                                        + "' in the method '" + entry.getActualClassInfo().name() + "." + methodInfo.name()
+                                        + "'");
+                    }
+
+                    ClassInfo effectiveParameter = getEffectiveClassInfo(parameter.type(), indexView);
                     if (effectiveParameter != null) {
                         if (consumesXml) {
                             classesInfo.add(effectiveParameter);
@@ -154,23 +175,16 @@ private ClassInfo getEffectiveClassInfo(Type type, IndexView indexView) {
         }
 
         Type effectiveType = type;
-        if (effectiveType.name().equals(ResteasyReactiveDotNames.REST_RESPONSE) ||
-                effectiveType.name().equals(ResteasyReactiveDotNames.UNI) ||
-                effectiveType.name().equals(ResteasyReactiveDotNames.COMPLETABLE_FUTURE) ||
-                effectiveType.name().equals(ResteasyReactiveDotNames.COMPLETION_STAGE) ||
-                effectiveType.name().equals(ResteasyReactiveDotNames.REST_MULTI) ||
-                effectiveType.name().equals(ResteasyReactiveDotNames.MULTI)) {
+        if (isContainerType(effectiveType)) {
             if (effectiveType.kind() != Type.Kind.PARAMETERIZED_TYPE) {
                 return null;
             }
 
             effectiveType = type.asParameterizedType().arguments().get(0);
         }
-        if (effectiveType.name().equals(ResteasyReactiveDotNames.SET) ||
-                effectiveType.name().equals(ResteasyReactiveDotNames.COLLECTION) ||
-                effectiveType.name().equals(ResteasyReactiveDotNames.LIST)) {
+        if (isCollectionType(effectiveType)) {
             effectiveType = effectiveType.asParameterizedType().arguments().get(0);
-        } else if (effectiveType.name().equals(ResteasyReactiveDotNames.MAP)) {
+        } else if (isMapType(effectiveType)) {
             effectiveType = effectiveType.asParameterizedType().arguments().get(1);
         }
 
@@ -220,4 +234,53 @@ private List<String> toClasses(Collection<ClassInfo> classesInfo) {
 
         return classes;
     }
+
+    private boolean isParameterBody(MethodParameterInfo parameter, ResourceMethod resourceInfo) {
+        for (MethodParameter parameterInfo : resourceInfo.getParameters()) {
+            if (parameterInfo.name != null && parameterInfo.name.equals(parameter.name())) {
+                return parameterInfo.parameterType == ParameterType.BODY;
+            }
+        }
+
+        return false;
+    }
+
+    private boolean isCollectionType(Type type) {
+        return type.name().equals(ResteasyReactiveDotNames.SET) ||
+                type.name().equals(ResteasyReactiveDotNames.COLLECTION) ||
+                type.name().equals(ResteasyReactiveDotNames.LIST);
+    }
+
+    private boolean isMapType(Type type) {
+        return type.name().equals(ResteasyReactiveDotNames.MAP);
+    }
+
+    private boolean isContainerType(Type type) {
+        return type.name().equals(REST_RESPONSE) ||
+                type.name().equals(ResteasyReactiveDotNames.UNI) ||
+                type.name().equals(ResteasyReactiveDotNames.COMPLETABLE_FUTURE) ||
+                type.name().equals(ResteasyReactiveDotNames.COMPLETION_STAGE) ||
+                type.name().equals(ResteasyReactiveDotNames.REST_MULTI) ||
+                type.name().equals(ResteasyReactiveDotNames.MULTI);
+    }
+
+    private boolean isTypeCompatibleWithJaxb(Type type) {
+        if (type.kind() == Type.Kind.PRIMITIVE) {
+            return true;
+        }
+
+        if (type.kind() == Type.Kind.ARRAY || isCollectionType(type) || isMapType(type)) {
+            return false;
+        }
+
+        if (isContainerType(type)) {
+            if (type.kind() != Type.Kind.PARAMETERIZED_TYPE) {
+                return true;
+            }
+
+            return isTypeCompatibleWithJaxb(type.asParameterizedType().arguments().get(0));
+        }
+
+        return true;
+    }
 }
diff --git a/...c/test/java/io/quarkus/resteasy/reactive/jaxb/deployment/test/FailWhenReturnListTest.java b/...c/test/java/io/quarkus/resteasy/reactive/jaxb/deployment/test/FailWhenReturnListTest.java
@@ -0,0 +1,41 @@
+package io.quarkus.resteasy.reactive.jaxb.deployment.test;
+
+import java.util.List;
+
+import jakarta.enterprise.inject.spi.DeploymentException;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.Produces;
+import jakarta.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 FailWhenReturnListTest {
+
+    @RegisterExtension
+    static QuarkusUnitTest test = new QuarkusUnitTest()
+            .setExpectedException(DeploymentException.class)
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(GreetingResource.class));
+
+    @Test
+    void shouldFailWithDeploymentException() {
+        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.APPLICATION_XML)
+        public List<String> hello() {
+            return List.of("1", "2", "3");
+        }
+    }
+}
diff --git a/...ava/io/quarkus/resteasy/reactive/jaxb/deployment/test/FailWhenReturnRestResponseTest.java b/...ava/io/quarkus/resteasy/reactive/jaxb/deployment/test/FailWhenReturnRestResponseTest.java
@@ -0,0 +1,43 @@
+package io.quarkus.resteasy.reactive.jaxb.deployment.test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import jakarta.enterprise.inject.spi.DeploymentException;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.Produces;
+import jakarta.ws.rs.core.MediaType;
+
+import org.jboss.resteasy.reactive.RestResponse;
+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 FailWhenReturnRestResponseTest {
+
+    @RegisterExtension
+    static QuarkusUnitTest test = new QuarkusUnitTest()
+            .setExpectedException(DeploymentException.class)
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(GreetingResource.class));
+
+    @Test
+    void shouldFailWithDeploymentException() {
+        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.APPLICATION_XML)
+        public RestResponse<Map<String, String>> hello() {
+            return RestResponse.ok(new HashMap<>());
+        }
+    }
+}
diff --git a/...test/java/io/quarkus/resteasy/reactive/jaxb/deployment/test/FailWhenUseListParamTest.java b/...test/java/io/quarkus/resteasy/reactive/jaxb/deployment/test/FailWhenUseListParamTest.java
@@ -0,0 +1,41 @@
+package io.quarkus.resteasy.reactive.jaxb.deployment.test;
+
+import java.util.List;
+
+import jakarta.enterprise.inject.spi.DeploymentException;
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.Path;
+import jakarta.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 FailWhenUseListParamTest {
+
+    @RegisterExtension
+    static QuarkusUnitTest test = new QuarkusUnitTest()
+            .setExpectedException(DeploymentException.class)
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(GreetingResource.class));
+
+    @Test
+    void shouldFailWithDeploymentException() {
+        Assertions.fail("The test case should not be invoked as it should fail with a deployment exception.");
+    }
+
+    @Path("/greeting")
+    public static class GreetingResource {
+
+        @POST
+        @Consumes(MediaType.APPLICATION_XML)
+        public String hello(List<String> items) {
+            return "ok";
+        }
+    }
+}
diff --git a/...processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java b/...processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java
@@ -1334,6 +1334,7 @@ && isParameterContainerType(paramType.asClassType())) {
                 if (field) {
                     return builder;
                 }
+                builder.setName(sourceName);
                 builder.setType(ParameterType.BODY);
             }
         }