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

Provide contextual error message for incorrect use of @BeanParam #21817

Merged
merged 1 commit into from
Dec 1, 2021
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
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
public class BeanParamInfo implements InjectableBean {
private boolean isFormParamRequired;
private boolean isInjectionRequired;
private int fieldExtractorsCount;

@Override
public boolean isFormParamRequired() {
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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<>();
Expand Down Expand Up @@ -274,6 +286,8 @@ protected InjectableBean scanInjectableBean(ClassInfo currentClassInfo, ClassInf
}
}

currentInjectableBean.setFieldExtractorsCount(fieldExtractors.size());

if (!fieldExtractors.isEmpty() && fieldInjectionHandler != null) {
fieldInjectionHandler.handleFieldInjection(currentTypeName, fieldExtractors, superTypeIsInjectable);
}
Expand Down Expand Up @@ -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() + "'";
Expand Down