From 7f52ed803e5f7c0cdde4c55c8286aa01c369c3c9 Mon Sep 17 00:00:00 2001 From: Auri Munoz Date: Fri, 22 Nov 2024 13:49:46 +0100 Subject: [PATCH] Don't ignore Spring Rest controllers interfaces if contains errors Related to #43704 --- ...SpringRestControllerAnnotationHandler.java | 17 +++++++ ...ableSpringRestControllerInterfaceTest.java | 48 +++++++++++++++++++ .../DefaultEndpointAnnotationHandler.java | 18 +++++++ .../processor/EndpointAnnotationHandler.java | 9 ++++ .../common/processor/EndpointIndexer.java | 10 +++- 5 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 extensions/spring-web/core/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringRestControllerAnnotationHandler.java create mode 100644 extensions/spring-web/resteasy-reactive/tests/src/test/java/io/quarkus/spring/web/resteasy/reactive/test/UnbuildableSpringRestControllerInterfaceTest.java create mode 100644 independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/DefaultEndpointAnnotationHandler.java create mode 100644 independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointAnnotationHandler.java diff --git a/extensions/spring-web/core/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringRestControllerAnnotationHandler.java b/extensions/spring-web/core/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringRestControllerAnnotationHandler.java new file mode 100644 index 00000000000000..0c2ae5b70d4008 --- /dev/null +++ b/extensions/spring-web/core/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringRestControllerAnnotationHandler.java @@ -0,0 +1,17 @@ +package io.quarkus.spring.web.deployment; + +import org.jboss.jandex.ClassInfo; +import org.jboss.jandex.DotName; +import org.jboss.resteasy.reactive.common.processor.EndpointAnnotationHandler; + +public class SpringRestControllerAnnotationHandler implements EndpointAnnotationHandler { + public static final DotName SPRING_REST_CONTROLLER = DotName + .createSimple("org.springframework.web.bind.annotation.RestController"); + + @Override + public boolean isEndpointAnnotationValid(ClassInfo classInfo) { + return classInfo + .declaredAnnotation(SPRING_REST_CONTROLLER) != null; + } + +} diff --git a/extensions/spring-web/resteasy-reactive/tests/src/test/java/io/quarkus/spring/web/resteasy/reactive/test/UnbuildableSpringRestControllerInterfaceTest.java b/extensions/spring-web/resteasy-reactive/tests/src/test/java/io/quarkus/spring/web/resteasy/reactive/test/UnbuildableSpringRestControllerInterfaceTest.java new file mode 100644 index 00000000000000..9cdb6b44c87798 --- /dev/null +++ b/extensions/spring-web/resteasy-reactive/tests/src/test/java/io/quarkus/spring/web/resteasy/reactive/test/UnbuildableSpringRestControllerInterfaceTest.java @@ -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); + + } + +} diff --git a/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/DefaultEndpointAnnotationHandler.java b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/DefaultEndpointAnnotationHandler.java new file mode 100644 index 00000000000000..188530e49e8efe --- /dev/null +++ b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/DefaultEndpointAnnotationHandler.java @@ -0,0 +1,18 @@ +package org.jboss.resteasy.reactive.common.processor; + +import java.lang.reflect.Modifier; + +import org.jboss.jandex.ClassInfo; + +public class DefaultEndpointAnnotationHandler implements EndpointAnnotationHandler { + public DefaultEndpointAnnotationHandler() { + } + + @Override + public boolean isEndpointAnnotationValid(ClassInfo classInfo) { + if (Modifier.isInterface(classInfo.flags()) || Modifier.isAbstract(classInfo.flags())) { + return true; + } + return false; + } +} diff --git a/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointAnnotationHandler.java b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointAnnotationHandler.java new file mode 100644 index 00000000000000..38e545b4ababc5 --- /dev/null +++ b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointAnnotationHandler.java @@ -0,0 +1,9 @@ +package org.jboss.resteasy.reactive.common.processor; + +import org.jboss.jandex.ClassInfo; + +public interface EndpointAnnotationHandler { + + boolean isEndpointAnnotationValid(ClassInfo classInfo); + +} diff --git a/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java index 25633cdd60993d..dda303afc2f4ea 100644 --- a/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java +++ b/independent-projects/resteasy-reactive/common/processor/src/main/java/org/jboss/resteasy/reactive/common/processor/EndpointIndexer.java @@ -97,6 +97,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.ServiceLoader; import java.util.Set; import java.util.function.Consumer; import java.util.function.Function; @@ -324,7 +325,7 @@ public Optional createEndpoints(ClassInfo classInfo, boolean cons return Optional.of(clazz); } catch (Exception e) { - if (Modifier.isInterface(classInfo.flags()) || Modifier.isAbstract(classInfo.flags())) { + if (isEndpointValid(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); @@ -334,6 +335,13 @@ public Optional createEndpoints(ClassInfo classInfo, boolean cons } } + private boolean isEndpointValid(ClassInfo classInfo) { + EndpointAnnotationHandler defaultEndpointAnnotationHandler = new DefaultEndpointAnnotationHandler(); + return ServiceLoader.load(EndpointAnnotationHandler.class).findFirst() + .orElse(defaultEndpointAnnotationHandler).isEndpointAnnotationValid(classInfo); + + } + private String sanitizePath(String path) { // this simply replaces the whitespace characters (not part of a path variable) with %20 // TODO: this might have to be more complex, URL encoding maybe?