Skip to content

Commit

Permalink
Merge pull request quarkusio#44642 from aureamunoz/spring-controller-…
Browse files Browse the repository at this point in the history
…43704

Don't ignore Spring Rest controllers interfaces if contains errors
  • Loading branch information
cescoffier authored Dec 16, 2024
2 parents 5f476ac + 6050efd commit 283566b
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package io.quarkus.resteasy.common.spi;

import java.util.function.Predicate;

import org.jboss.jandex.ClassInfo;

import io.quarkus.builder.item.MultiBuildItem;

/**
* A build item that provides a {@link Predicate} to detect and validate classes defining REST endpoints.
* <p>
* This can include resources in RESTEasy or controllers in the Spring ecosystem.
* It acts as a Service Provider Interface (SPI) to allow customization of the validation logic for endpoint detection,
* enabling integration with various frameworks or specific application needs.
* </p>
*
* <p>
* The {@link Predicate} evaluates {@link ClassInfo} instances to determine whether a class defines a REST endpoint
* according to the provided logic.
* </p>
*/
public final class EndpointValidationPredicatesBuildItem extends MultiBuildItem {

private final Predicate<ClassInfo> predicate;

public EndpointValidationPredicatesBuildItem(Predicate<ClassInfo> predicate) {
this.predicate = predicate;
}

public Predicate<ClassInfo> getPredicate() {
return predicate;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import jakarta.ws.rs.ProcessingException;
import jakarta.ws.rs.RuntimeType;
Expand Down Expand Up @@ -177,6 +178,7 @@
import io.quarkus.resteasy.reactive.common.deployment.ResourceScanningResultBuildItem;
import io.quarkus.resteasy.reactive.common.deployment.SerializersUtil;
import io.quarkus.resteasy.reactive.common.runtime.ResteasyReactiveConfig;
import io.quarkus.resteasy.reactive.spi.EndpointValidationPredicatesBuildItem;
import io.quarkus.resteasy.reactive.spi.MessageBodyReaderBuildItem;
import io.quarkus.resteasy.reactive.spi.MessageBodyReaderOverrideBuildItem;
import io.quarkus.resteasy.reactive.spi.MessageBodyWriterBuildItem;
Expand Down Expand Up @@ -289,7 +291,8 @@ void setupClientProxies(JaxrsClientReactiveRecorder recorder,
List<RestClientDefaultConsumesBuildItem> defaultProduces,
List<RestClientDisableSmartDefaultProduces> disableSmartDefaultProduces,
List<RestClientDisableRemovalTrailingSlashBuildItem> disableRemovalTrailingSlashProduces,
List<ParameterContainersBuildItem> parameterContainersBuildItems) {
List<ParameterContainersBuildItem> parameterContainersBuildItems,
List<EndpointValidationPredicatesBuildItem> validationPredicatesBuildItems) {

String defaultConsumesType = defaultMediaType(defaultConsumes, MediaType.APPLICATION_OCTET_STREAM);
String defaultProducesType = defaultMediaType(defaultProduces, MediaType.TEXT_PLAIN);
Expand Down Expand Up @@ -343,6 +346,8 @@ public boolean test(Map<DotName, AnnotationInstance> anns) {
return anns.containsKey(NOT_BODY) || anns.containsKey(URL);
}
})
.setValidateEndpoint(validationPredicatesBuildItems.stream().map(item -> item.getPredicate())
.collect(Collectors.toUnmodifiableList()))
.setResourceMethodCallback(new Consumer<>() {
@Override
public void accept(EndpointIndexer.ResourceMethodCallbackEntry entry) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package io.quarkus.resteasy.reactive.spi;

import java.util.function.Predicate;

import org.jboss.jandex.ClassInfo;

import io.quarkus.builder.item.MultiBuildItem;

/**
* A build item that provides a {@link Predicate} to detect and validate classes defining REST endpoints.
* <p>
* This can include resources in RESTEasy or controllers in the Spring ecosystem.
* It acts as a Service Provider Interface (SPI) to allow customization of the validation logic for endpoint detection,
* enabling integration with various frameworks or specific application needs.
* </p>
*
* <p>
* The {@link Predicate} evaluates {@link ClassInfo} instances to determine whether a class defines a REST endpoint
* according to the provided logic.
* </p>
*/
public final class EndpointValidationPredicatesBuildItem extends MultiBuildItem {

private final Predicate<ClassInfo> predicate;

public EndpointValidationPredicatesBuildItem(Predicate<ClassInfo> predicate) {
this.predicate = predicate;
}

public Predicate<ClassInfo> getPredicate() {
return predicate;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@
import io.quarkus.resteasy.reactive.server.spi.ResumeOn404BuildItem;
import io.quarkus.resteasy.reactive.spi.CustomExceptionMapperBuildItem;
import io.quarkus.resteasy.reactive.spi.DynamicFeatureBuildItem;
import io.quarkus.resteasy.reactive.spi.EndpointValidationPredicatesBuildItem;
import io.quarkus.resteasy.reactive.spi.ExceptionMapperBuildItem;
import io.quarkus.resteasy.reactive.spi.JaxrsFeatureBuildItem;
import io.quarkus.resteasy.reactive.spi.MessageBodyReaderBuildItem;
Expand Down Expand Up @@ -468,7 +469,8 @@ public void setupEndpoints(ApplicationIndexBuildItem applicationIndexBuildItem,
CompiledJavaVersionBuildItem compiledJavaVersionBuildItem,
ResourceInterceptorsBuildItem resourceInterceptorsBuildItem,
Capabilities capabilities,
Optional<AllowNotRestParametersBuildItem> allowNotRestParametersBuildItem) {
Optional<AllowNotRestParametersBuildItem> allowNotRestParametersBuildItem,
List<EndpointValidationPredicatesBuildItem> validationPredicatesBuildItems) {

if (!resourceScanningResultBuildItem.isPresent()) {
// no detected @Path, bail out
Expand Down Expand Up @@ -640,6 +642,8 @@ private boolean hasAnnotation(MethodInfo method, short paramPosition, DotName an
})
.setResteasyReactiveRecorder(recorder)
.setApplicationClassPredicate(applicationClassPredicate)
.setValidateEndpoint(validationPredicatesBuildItems.stream().map(item -> item.getPredicate())
.collect(Collectors.toUnmodifiableList()))
.setTargetJavaVersion(new TargetJavaVersion() {

private final Status result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.function.Predicate;

import jakarta.ws.rs.Priorities;
import jakarta.ws.rs.core.HttpHeaders;
Expand Down Expand Up @@ -34,6 +35,7 @@
import io.quarkus.deployment.builditem.nativeimage.ReflectiveHierarchyIgnoreWarningBuildItem;
import io.quarkus.gizmo.ClassOutput;
import io.quarkus.jaxrs.spi.deployment.AdditionalJaxRsResourceMethodAnnotationsBuildItem;
import io.quarkus.resteasy.common.spi.EndpointValidationPredicatesBuildItem;
import io.quarkus.resteasy.common.spi.ResteasyJaxrsProviderBuildItem;
import io.quarkus.resteasy.reactive.spi.ExceptionMapperBuildItem;

Expand Down Expand Up @@ -86,6 +88,30 @@ public AdditionalJaxRsResourceMethodAnnotationsBuildItem additionalJaxRsResource
return new AdditionalJaxRsResourceMethodAnnotationsBuildItem(MAPPING_ANNOTATIONS);
}

@BuildStep
EndpointValidationPredicatesBuildItem createSpringRestControllerPredicateForClassic() {
Predicate<ClassInfo> predicate = new Predicate<>() {
@Override
public boolean test(ClassInfo classInfo) {
return classInfo
.declaredAnnotation(REST_CONTROLLER_ANNOTATION) == null;
}
};
return new EndpointValidationPredicatesBuildItem(predicate);
}

@BuildStep
io.quarkus.resteasy.reactive.spi.EndpointValidationPredicatesBuildItem createSpringRestControllerPredicateForReactive() {
Predicate<ClassInfo> predicate = new Predicate<>() {
@Override
public boolean test(ClassInfo classInfo) {
return classInfo
.declaredAnnotation(REST_CONTROLLER_ANNOTATION) == null;
}
};
return new io.quarkus.resteasy.reactive.spi.EndpointValidationPredicatesBuildItem(predicate);
}

@BuildStep
public void ignoreReflectionHierarchy(BuildProducer<ReflectiveHierarchyIgnoreWarningBuildItem> ignore) {
ignore.produce(new ReflectiveHierarchyIgnoreWarningBuildItem(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package io.quarkus.spring.web.resteasy.reactive.test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;

import jakarta.ws.rs.QueryParam;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

import io.quarkus.test.QuarkusProdModeTest;

public class UnbuildableSpringRestControllerInterfaceTest {

@RegisterExtension
static final QuarkusProdModeTest config = new QuarkusProdModeTest()
.withApplicationRoot((jar) -> jar
.addClasses(UnbuildableControllerInterface.class))
.setApplicationName("unbuildable-rest-controller-interface")
.setApplicationVersion("0.1-SNAPSHOT")
.assertBuildException(throwable -> {
assertThat(throwable).isInstanceOf(RuntimeException.class);
assertThat(throwable).hasMessageContaining(
"Cannot have more than one of @PathParam, @QueryParam, @HeaderParam, @FormParam, @CookieParam, @BeanParam, @Context on method");
});

@Test
public void testBuildLogs() {
fail("Should not be called");
}

@RestController
@RequestMapping("/unbuildable")
public interface UnbuildableControllerInterface {
@GetMapping("/ping")
String ping();

@PostMapping("/hello")
String hello(@RequestParam(required = false) @QueryParam("dung") String params);

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
import java.nio.charset.StandardCharsets;
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -239,6 +240,7 @@ public abstract class EndpointIndexer<T extends EndpointIndexer<T, PARAM, METHOD
private final Function<ClassInfo, Supplier<Boolean>> isDisabledCreator;

private final Predicate<Map<DotName, AnnotationInstance>> skipMethodParameter;
private final List<Predicate<ClassInfo>> validateEndpoint;
private final boolean skipNotRestParameters;

private SerializerScanningResult serializerScanningResult;
Expand Down Expand Up @@ -267,6 +269,7 @@ protected EndpointIndexer(Builder<T, ?, METHOD> builder) {
this.isDisabledCreator = builder.isDisabledCreator;
this.skipMethodParameter = builder.skipMethodParameter;
this.skipNotRestParameters = builder.skipNotRestParameters;
this.validateEndpoint = builder.defaultPredicate;
}

public Optional<ResourceClass> createEndpoints(ClassInfo classInfo, boolean considerApplication) {
Expand Down Expand Up @@ -324,14 +327,18 @@ public Optional<ResourceClass> createEndpoints(ClassInfo classInfo, boolean cons

return Optional.of(clazz);
} catch (Exception e) {
if (Modifier.isInterface(classInfo.flags()) || Modifier.isAbstract(classInfo.flags())) {
//kinda bogus, but we just ignore failed interfaces for now
//they can have methods that are not valid until they are actually extended by a concrete type
log.debug("Ignoring interface " + classInfo.name(), e);
return Optional.empty();
for (Predicate<ClassInfo> predicate : validateEndpoint) {
if (predicate.test(classInfo)) {
//kinda bogus, but we just ignore failed interfaces for now
//they can have methods that are not valid until they are actually extended by a concrete type
log.debug("Ignoring interface " + classInfo.name(), e);
} else {
throw new RuntimeException(e);
}
}
throw new RuntimeException(e);
return Optional.empty();
}

}

private String sanitizePath(String path) {
Expand Down Expand Up @@ -1708,6 +1715,13 @@ public boolean handleMultipartForReturnType(AdditionalWriters additionalWriters,
private Function<ClassInfo, Supplier<Boolean>> isDisabledCreator = null;

private Predicate<Map<DotName, AnnotationInstance>> skipMethodParameter = null;
private List<Predicate<ClassInfo>> defaultPredicate = Arrays.asList(new Predicate<>() {
@Override
public boolean test(ClassInfo classInfo) {
return Modifier.isInterface(classInfo.flags())
|| Modifier.isAbstract(classInfo.flags());
}
});

private boolean skipNotRestParameters = false;

Expand Down Expand Up @@ -1844,6 +1858,14 @@ public B setSkipMethodParameter(
return (B) this;
}

public B setValidateEndpoint(List<Predicate<ClassInfo>> validateEndpoint) {
List<Predicate<ClassInfo>> predicates = new ArrayList<>(validateEndpoint.size() + 1);
predicates.addAll(validateEndpoint);
predicates.addAll(this.defaultPredicate);
this.defaultPredicate = predicates;
return (B) this;
}

public B skipNotRestParameters(boolean skipNotRestParameters) {
this.skipNotRestParameters = skipNotRestParameters;
return (B) this;
Expand Down

0 comments on commit 283566b

Please sign in to comment.