Skip to content

Commit

Permalink
Don't ignore Spring Rest controllers interfaces if contains errors
Browse files Browse the repository at this point in the history
Related to quarkusio#43704
  • Loading branch information
aureamunoz committed Dec 10, 2024
1 parent c326bec commit a7dd34c
Show file tree
Hide file tree
Showing 13 changed files with 203 additions and 8 deletions.
5 changes: 5 additions & 0 deletions bom/application/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3780,6 +3780,11 @@
<artifactId>quarkus-resteasy-common-spi</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy-common-deployment-spi</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>io.reactivex.rxjava2</groupId>
<artifactId>rxjava</artifactId>
Expand Down
1 change: 1 addition & 0 deletions extensions/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@

<!-- REST services -->
<module>resteasy-classic</module>
<module>resteasy-common</module>
<module>smallrye-openapi-common</module>
<module>smallrye-openapi</module>
<module>swagger-ui</module>
Expand Down
27 changes: 27 additions & 0 deletions extensions/resteasy-common/deployment-spi/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<parent>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy-common-spi-parent</artifactId>
<version>999-SNAPSHOT</version>
</parent>

<artifactId>quarkus-resteasy-common-deployment-spi</artifactId>
<name>Quarkus - Resteasy - SPI - Deployment</name>

<dependencies>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-core-deployment</artifactId>
</dependency>
<dependency>
<groupId>io.smallrye</groupId>
<artifactId>jandex</artifactId>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package io.quarkus.resteasy.common.deployment;

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;
}
}
21 changes: 21 additions & 0 deletions extensions/resteasy-common/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<parent>
<artifactId>quarkus-extensions-parent</artifactId>
<groupId>io.quarkus</groupId>
<version>999-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>quarkus-resteasy-common-spi-parent</artifactId>
<name>Quarkus - RESTEasy Common SPI - Parent</name>
<description>This module provides reusable abstraction for use with both RESTEasy Classic and Quarkus REST (formerly RESTEasy Reactive)</description>
<packaging>pom</packaging>
<modules>
<module>deployment-spi</module>
</modules>

</project>
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
<groupId>io.quarkus</groupId>
<artifactId>quarkus-rest-spi-deployment</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy-common-deployment-spi</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-rest-client-jaxrs</artifactId>
Expand Down
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 @@ -170,6 +171,7 @@
import io.quarkus.jaxrs.client.reactive.runtime.RestClientBase;
import io.quarkus.jaxrs.client.reactive.runtime.ToObjectArray;
import io.quarkus.jaxrs.client.reactive.runtime.impl.MultipartResponseDataBase;
import io.quarkus.resteasy.common.deployment.EndpointValidationPredicatesBuildItem;
import io.quarkus.resteasy.reactive.common.deployment.ApplicationResultBuildItem;
import io.quarkus.resteasy.reactive.common.deployment.ParameterContainersBuildItem;
import io.quarkus.resteasy.reactive.common.deployment.QuarkusFactoryCreator;
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
4 changes: 4 additions & 0 deletions extensions/resteasy-reactive/rest/deployment/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@
<groupId>io.quarkus</groupId>
<artifactId>quarkus-security-spi</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy-common-deployment-spi</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-jsonp-deployment</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import java.io.File;
import java.io.IOException;
import java.lang.reflect.Modifier;
import java.nio.file.Path;
import java.util.ArrayDeque;
import java.util.ArrayList;
Expand Down Expand Up @@ -159,6 +160,7 @@
import io.quarkus.gizmo.MethodCreator;
import io.quarkus.gizmo.MethodDescriptor;
import io.quarkus.netty.deployment.MinNettyAllocatorMaxOrderBuildItem;
import io.quarkus.resteasy.common.deployment.EndpointValidationPredicatesBuildItem;
import io.quarkus.resteasy.reactive.common.deployment.AggregatedParameterContainersBuildItem;
import io.quarkus.resteasy.reactive.common.deployment.ApplicationResultBuildItem;
import io.quarkus.resteasy.reactive.common.deployment.FactoryUtils;
Expand Down Expand Up @@ -468,7 +470,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 +643,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 Expand Up @@ -849,6 +854,18 @@ private FilterClassIntrospector createFilterClassIntrospector() {
return ab.get();
}

@BuildStep
EndpointValidationPredicatesBuildItem createSpringRestControllerPredicate() {
Predicate<ClassInfo> predicate = new Predicate<ClassInfo>() {
@Override
public boolean test(ClassInfo classInfo) {
return Modifier.isInterface(classInfo.flags())
|| Modifier.isAbstract(classInfo.flags());
}
};
return new EndpointValidationPredicatesBuildItem(predicate);
}

// We want to add @Typed to resources, beanparams and providers so that they can be resolved as CDI bean using purely their
// class as a bean type. This removes any ambiguity that potential subclasses may have.
@BuildStep
Expand Down
4 changes: 4 additions & 0 deletions extensions/spring-web/core/deployment/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy-common-spi</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy-common-deployment-spi</artifactId>
</dependency>
</dependencies>

<build>
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.deployment.EndpointValidationPredicatesBuildItem;
import io.quarkus.resteasy.common.spi.ResteasyJaxrsProviderBuildItem;
import io.quarkus.resteasy.reactive.spi.ExceptionMapperBuildItem;

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

@BuildStep
EndpointValidationPredicatesBuildItem createSpringRestControllerPredicate() {
Predicate<ClassInfo> predicate = new Predicate<>() {
@Override
public boolean test(ClassInfo classInfo) {
return classInfo
.declaredAnnotation(REST_CONTROLLER_ANNOTATION) == null;
}
};
return new 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 @@ -239,6 +239,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 +268,7 @@ protected EndpointIndexer(Builder<T, ?, METHOD> builder) {
this.isDisabledCreator = builder.isDisabledCreator;
this.skipMethodParameter = builder.skipMethodParameter;
this.skipNotRestParameters = builder.skipNotRestParameters;
this.validateEndpoint = builder.validateEndpoint;
}

public Optional<ResourceClass> createEndpoints(ClassInfo classInfo, boolean considerApplication) {
Expand Down Expand Up @@ -324,14 +326,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 +1714,7 @@ public boolean handleMultipartForReturnType(AdditionalWriters additionalWriters,
private Function<ClassInfo, Supplier<Boolean>> isDisabledCreator = null;

private Predicate<Map<DotName, AnnotationInstance>> skipMethodParameter = null;
private List<Predicate<ClassInfo>> validateEndpoint = null;

private boolean skipNotRestParameters = false;

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

public B setValidateEndpoint(List<Predicate<ClassInfo>> validateEndpoint) {
this.validateEndpoint = validateEndpoint;
return (B) this;
}

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

0 comments on commit a7dd34c

Please sign in to comment.