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

Fail when using collections in Resteasy Reactive JAXB #34237

Merged
merged 1 commit into from
Jun 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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,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;
Expand All @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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");
}
}
}
Original file line number Diff line number Diff line change
@@ -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<>());
}
}
}
Original file line number Diff line number Diff line change
@@ -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";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1334,6 +1334,7 @@ && isParameterContainerType(paramType.asClassType())) {
if (field) {
return builder;
}
builder.setName(sourceName);
geoand marked this conversation as resolved.
Show resolved Hide resolved
builder.setType(ParameterType.BODY);
}
}
Expand Down