Merge pull request #5803 from geoand/#5796

Fix various usability issues in Spring @RestControllerAdvice handling

extensions/spring-web/deployment/src/main/java/io/quarkus/spring/web/deployment/AbstractExceptionMapperGenerator.java

@@ -11,6 +11,7 @@
 import org.jboss.jandex.AnnotationValue;
 import org.jboss.jandex.DotName;
 
+import io.quarkus.deployment.util.HashUtil;
 import io.quarkus.gizmo.ClassCreator;
 import io.quarkus.gizmo.ClassOutput;
 import io.quarkus.gizmo.MethodCreator;
@@ -33,9 +34,10 @@ abstract class AbstractExceptionMapperGenerator {
     abstract void generateMethodBody(MethodCreator toResponse);
 
     String generate() {
-        String generatedClassName = "io.quarkus.spring.web.mappers." + exceptionDotName.withoutPackagePrefix() + "Mapper";
+        String generatedClassName = "io.quarkus.spring.web.mappers." + exceptionDotName.withoutPackagePrefix() + "_Mapper_"
+                + HashUtil.sha1(exceptionDotName.toString());
         String generatedSubtypeClassName = "io.quarkus.spring.web.mappers.Subtype" + exceptionDotName.withoutPackagePrefix()
-                + "Mapper";
+                + "Mapper_" + HashUtil.sha1(exceptionDotName.toString());
         String exceptionClassName = exceptionDotName.toString();
 
         try (ClassCreator cc = ClassCreator.builder()

extensions/spring-web/deployment/src/main/java/io/quarkus/spring/web/deployment/ControllerAdviceAbstractExceptionMapperGenerator.java

@@ -9,6 +9,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.ws.rs.core.Context;
+import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 
 import org.jboss.jandex.AnnotationInstance;
@@ -31,6 +32,7 @@
 class ControllerAdviceAbstractExceptionMapperGenerator extends AbstractExceptionMapperGenerator {
 
     private static final DotName RESPONSE_ENTITY = DotName.createSimple("org.springframework.http.ResponseEntity");
+    private static final DotName STRING = DotName.createSimple(String.class.getName());
 
     private final MethodInfo controllerAdviceMethod;
     private final TypesUtil typesUtil;
@@ -135,17 +137,39 @@ void generateMethodBody(MethodCreator toResponse) {
 
             ResultHandle response;
             if (RESPONSE_ENTITY.equals(returnType.name())) {
+                /*
+                 * By default we will send JSON back unless the ResponseEntity has a String body
+                 */
+                boolean addDefaultJsonContentType = true;
+                if (returnType.kind() == Type.Kind.PARAMETERIZED_TYPE) {
+                    if (returnType.asParameterizedType().arguments().size() == 1) {
+                        Type responseEntityParameterType = returnType.asParameterizedType().arguments().get(0);
+                        if (STRING.equals(responseEntityParameterType.name())) {
+                            addDefaultJsonContentType = false;
+                        }
+                    }
+                }
+
                 // convert Spring's ResponseEntity to JAX-RS Response
                 response = toResponse.invokeStaticMethod(
                         MethodDescriptor.ofMethod(ResponseEntityConverter.class.getName(), "toResponse",
-                                Response.class.getName(), RESPONSE_ENTITY.toString()),
-                        exceptionHandlerMethodResponse);
+                                Response.class.getName(), RESPONSE_ENTITY.toString(), boolean.class.getName()),
+                        exceptionHandlerMethodResponse, toResponse.load(addDefaultJsonContentType));
             } else {
                 ResultHandle status = toResponse.load(getStatus(controllerAdviceMethod.annotation(RESPONSE_STATUS)));
 
                 ResultHandle responseBuilder = toResponse.invokeStaticMethod(
                         MethodDescriptor.ofMethod(Response.class, "status", Response.ResponseBuilder.class, int.class),
                         status);
+                /*
+                 * By default we will send JSON back unless the ResponseEntity has a String body
+                 */
+                if (!STRING.equals(returnType.name())) {
+                    toResponse.invokeVirtualMethod(
+                            MethodDescriptor.ofMethod(Response.ResponseBuilder.class, "type", Response.ResponseBuilder.class,
+                                    String.class),
+                            responseBuilder, toResponse.load(MediaType.APPLICATION_JSON));
+                }
 
                 toResponse.invokeVirtualMethod(
                         MethodDescriptor.ofMethod(Response.ResponseBuilder.class, "entity", Response.ResponseBuilder.class,
@@ -190,9 +214,7 @@ private ResultHandle exceptionHandlerMethodResponse(MethodCreator toResponse) {
     }
 
     private ResultHandle controllerAdviceInstance(MethodCreator toResponse) {
-        ResultHandle controllerAdviceClass = toResponse.invokeStaticMethod(
-                MethodDescriptor.ofMethod(Class.class, "forName", Class.class, String.class),
-                toResponse.load(declaringClassName));
+        ResultHandle controllerAdviceClass = toResponse.loadClass(declaringClassName);
 
         ResultHandle container = toResponse
                 .invokeStaticMethod(MethodDescriptor.ofMethod(Arc.class, "container", ArcContainer.class));

extensions/spring-web/deployment/src/main/java/io/quarkus/spring/web/deployment/SpringWebProcessor.java

@@ -338,7 +338,7 @@ public void generateExceptionMapperProviders(BeanArchiveIndexBuildItem beanArchi
         // Look for all exception classes that are annotated with @ResponseStatus
 
         IndexView index = beanArchiveIndexBuildItem.getIndex();
-        ClassOutput classOutput = new GeneratedClassGizmoAdaptor(generatedExceptionMappers, false);
+        ClassOutput classOutput = new GeneratedClassGizmoAdaptor(generatedExceptionMappers, true);
         generateMappersForResponseStatusOnException(providersProducer, index, classOutput, typesUtil);
         generateMappersForExceptionHandlerInControllerAdvice(providersProducer, reflectiveClassProducer, index, classOutput,
                 typesUtil);

extensions/spring-web/runtime/src/main/java/io/quarkus/spring/web/runtime/ResponseEntityConverter.java

@@ -4,25 +4,37 @@
 import java.util.List;
 import java.util.Map;
 
+import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 
 import org.jboss.resteasy.core.Headers;
 import org.jboss.resteasy.specimpl.BuiltResponse;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.ResponseEntity;
 
+/**
+ * This is only used in the generated ExceptionMappers when the Spring @RestControllerAdvice method returns a ResponseEntity
+ */
 public class ResponseEntityConverter {
 
-    public static Response toResponse(ResponseEntity responseEntity) {
+    public static Response toResponse(ResponseEntity responseEntity, boolean addJsonContentTypeIfNotSet) {
         return new BuiltResponse(
-                responseEntity.getStatusCodeValue(), toHeaders(responseEntity.getHeaders()), responseEntity.getBody(),
+                responseEntity.getStatusCodeValue(), toHeaders(responseEntity.getHeaders(), addJsonContentTypeIfNotSet),
+                responseEntity.getBody(),
                 new Annotation[0]);
     }
 
-    private static Headers<Object> toHeaders(HttpHeaders springHeaders) {
+    private static Headers<Object> toHeaders(HttpHeaders springHeaders, boolean addJsonContentTypeIfNotSet) {
         Headers<Object> jaxRsHeaders = new Headers<>();
         for (Map.Entry<String, List<String>> entry : springHeaders.entrySet()) {
-            jaxRsHeaders.addAll(entry.getKey(), entry.getValue());
+            jaxRsHeaders.addAll(entry.getKey(), entry.getValue().toArray(new Object[0]));
+        }
+        /*
+         * We add the default application/json content type if no content type is specified
+         * since this is the default value when returning an object from a Spring RestController
+         */
+        if (addJsonContentTypeIfNotSet && !jaxRsHeaders.containsKey(javax.ws.rs.core.HttpHeaders.CONTENT_TYPE)) {
+            jaxRsHeaders.add(javax.ws.rs.core.HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON);
         }
         return jaxRsHeaders;
     }

integration-tests/spring-web/src/main/java/io/quarkus/it/spring/web/CustomAdvice.java

@@ -26,7 +26,7 @@ public void unannotatedException() {
     @ExceptionHandler(IllegalStateException.class)
     public ResponseEntity<Error> handleIllegalStateException(IllegalStateException e,
             HttpServletRequest request, HttpServletResponse response) {
-        return ResponseEntity.status(HttpStatus.PAYMENT_REQUIRED)
+        return ResponseEntity.status(HttpStatus.PAYMENT_REQUIRED).header("custom-header", "custom-value")
                 .body(new Error(request.getRequestURI() + ":" + e.getMessage()));
     }
 

integration-tests/spring-web/src/main/java/io/quarkus/it/spring/web/ExceptionThrowingController.java

@@ -35,11 +35,21 @@ public Greeting handledByResponseEntity() {
         throw new IllegalStateException("bad state");
     }
 
+    @GetMapping("/responseEntityFromVoidReturningMethod")
+    public void handledByResponseEntityFromVoidReturningMethod() {
+        throw new IllegalStateException("bad state");
+    }
+
     @GetMapping("/pojo")
     public Greeting greetingWithIllegalArgumentException() {
         throw new IllegalArgumentException("hello from error");
     }
 
+    @GetMapping("/pojoWithVoidReturnType")
+    public void greetingWithIllegalArgumentExceptionAndVoidReturnType() {
+        throw new IllegalArgumentException("hello from error");
+    }
+
     @GetMapping("/re")
     public ResponseEntity<Greeting> responseEntityWithIllegalArgumentException() {
         throw new IllegalStateException("hello from error");

integration-tests/spring-web/src/test/java/io/quarkus/it/spring/web/SpringControllerTest.java

@@ -133,7 +133,17 @@ public void testExceptionHandlerResponseEntityType() {
         RestAssured.when().get("/exception/responseEntity").then()
                 .contentType("application/json")
                 .body(containsString("bad state"), containsString("responseEntity"))
-                .statusCode(402);
+                .statusCode(402)
+                .header("custom-header", "custom-value");
+    }
+
+    @Test
+    public void testExceptionHandlerResponseEntityTypeFromVoidReturningMethod() {
+        RestAssured.when().get("/exception/responseEntityFromVoidReturningMethod").then()
+                .contentType("application/json")
+                .body(containsString("bad state"), containsString("responseEntity"))
+                .statusCode(402)
+                .header("custom-header", "custom-value");
     }
 
     @Test
@@ -144,6 +154,14 @@ public void testExceptionHandlerPojoEntityType() {
                 .statusCode(417);
     }
 
+    @Test
+    public void testControllerMethodWithVoidReturnTypeAndExceptionHandlerPojoEntityType() {
+        RestAssured.when().get("/exception/pojoWithVoidReturnType").then()
+                .contentType("application/json")
+                .body(containsString("hello from error"))
+                .statusCode(417);
+    }
+
     @Test
     public void testRestControllerWithoutRequestMapping() {
         RestAssured.when().get("/hello").then()