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 Nov 29, 2024
1 parent 0f88242 commit 7f52ed8
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -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;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package io.quarkus.spring.web.resteasy.reactive.test;

Check failure on line 1 in extensions/spring-web/resteasy-reactive/tests/src/test/java/io/quarkus/spring/web/resteasy/reactive/test/UnbuildableSpringRestControllerInterfaceTest.java

View workflow job for this annotation

GitHub Actions / Build summary for 7f52ed803e5f7c0cdde4c55c8286aa01c369c3c9

JVM Tests - JDK 17

org.opentest4j.AssertionFailedError: The build was expected to fail at io.quarkus.test.QuarkusProdModeTest.beforeAll(QuarkusProdModeTest.java:470) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Raw output
org.opentest4j.AssertionFailedError: The build was expected to fail
	at io.quarkus.test.QuarkusProdModeTest.beforeAll(QuarkusProdModeTest.java:470)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Check failure on line 1 in extensions/spring-web/resteasy-reactive/tests/src/test/java/io/quarkus/spring/web/resteasy/reactive/test/UnbuildableSpringRestControllerInterfaceTest.java

View workflow job for this annotation

GitHub Actions / Build summary for 7f52ed803e5f7c0cdde4c55c8286aa01c369c3c9

JVM Tests - JDK 17 Windows

org.opentest4j.AssertionFailedError: The build was expected to fail at io.quarkus.test.QuarkusProdModeTest.beforeAll(QuarkusProdModeTest.java:470) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Raw output
org.opentest4j.AssertionFailedError: The build was expected to fail
	at io.quarkus.test.QuarkusProdModeTest.beforeAll(QuarkusProdModeTest.java:470)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Check failure on line 1 in extensions/spring-web/resteasy-reactive/tests/src/test/java/io/quarkus/spring/web/resteasy/reactive/test/UnbuildableSpringRestControllerInterfaceTest.java

View workflow job for this annotation

GitHub Actions / Build summary for 7f52ed803e5f7c0cdde4c55c8286aa01c369c3c9

JVM Tests - JDK 21

org.opentest4j.AssertionFailedError: The build was expected to fail at io.quarkus.test.QuarkusProdModeTest.beforeAll(QuarkusProdModeTest.java:470) at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Raw output
org.opentest4j.AssertionFailedError: The build was expected to fail
	at io.quarkus.test.QuarkusProdModeTest.beforeAll(QuarkusProdModeTest.java:470)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

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
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package org.jboss.resteasy.reactive.common.processor;

import org.jboss.jandex.ClassInfo;

public interface EndpointAnnotationHandler {

boolean isEndpointAnnotationValid(ClassInfo classInfo);

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -324,7 +325,7 @@ public Optional<ResourceClass> 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);
Expand All @@ -334,6 +335,13 @@ public Optional<ResourceClass> 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?
Expand Down

0 comments on commit 7f52ed8

Please sign in to comment.