Properly read multiple nested/composed @APIResponse annotations (#2419)

* Properly read multiple nested/composed `@APIResponse` annotations

Signed-off-by: Michael Edgar <[email protected]>

* Remove unused APIResponsesIO method body, improve response comments

Signed-off-by: Michael Edgar <[email protected]>

---------

Signed-off-by: Michael Edgar <[email protected]>

Author: MikeEdgar

core/src/main/java/io/smallrye/openapi/runtime/io/responses/APIResponseIO.java

@@ -10,6 +10,7 @@
 import org.eclipse.microprofile.openapi.models.responses.APIResponse;
 import org.eclipse.microprofile.openapi.models.responses.APIResponses;
 import org.jboss.jandex.AnnotationInstance;
+import org.jboss.jandex.AnnotationTarget;
 import org.jboss.jandex.DotName;
 import org.jboss.jandex.Type;
 
@@ -40,6 +41,11 @@ public APIResponseIO(IOContext<V, A, O, AB, OB> context) {
         super(context, Names.API_RESPONSE, DotName.createSimple(APIResponse.class));
     }
 
+    @Override
+    public Map<String, APIResponse> readMap(AnnotationTarget target) {
+        return readMap(target, this::responseCode);
+    }
+
     @Override
     public APIResponse read(AnnotationInstance annotation) {
         IoLogging.logger.singleAnnotation("@APIResponse");

core/src/main/java/io/smallrye/openapi/runtime/io/responses/APIResponsesIO.java

@@ -1,7 +1,5 @@
 package io.smallrye.openapi.runtime.io.responses;
 
-import java.util.Arrays;
-import java.util.Collections;
 import java.util.Map;
 import java.util.Optional;
 
@@ -23,36 +21,26 @@ public APIResponsesIO(IOContext<V, A, O, AB, OB> context) {
         super(context, Names.API_RESPONSES, Names.create(APIResponses.class));
     }
 
-    public Map<String, APIResponse> readSingle(AnnotationTarget target) {
-        return Optional.ofNullable(apiResponseIO().getAnnotation(target))
-                .map(Collections::singleton)
-                .map(annotations -> apiResponseIO().readMap(annotations, apiResponseIO()::responseCode))
+    @Override
+    public APIResponses read(AnnotationTarget target) {
+        Map<String, APIResponse> responseMap = apiResponseIO().readMap(target);
+        Map<String, Object> extensions = Optional.ofNullable(getAnnotation(target))
+                .map(extensionIO()::readExtensible)
                 .orElse(null);
-    }
 
-    public Map<String, APIResponse> readAll(AnnotationTarget target) {
-        return apiResponseIO().readMap(target, apiResponseIO()::responseCode);
+        if (!responseMap.isEmpty() || extensions != null) {
+            APIResponses responses = OASFactory.createAPIResponses();
+            responseMap.forEach(responses::addAPIResponse);
+            responses.setExtensions(extensions);
+            return responses;
+        }
+
+        return null;
     }
 
     @Override
     public APIResponses read(AnnotationInstance annotation) {
-        AnnotationTarget target = annotation.target();
-
-        return Optional.ofNullable(annotation)
-                .map(AnnotationInstance::value)
-                /*
-                 * Begin - copy target to clones of nested annotations to support @Extension on
-                 * method being applied to @APIReponse. Remove when no longer supporting TCK
-                 * 3.1.1 and earlier.
-                 */
-                .map(AnnotationValue::asNestedArray)
-                .map(annotations -> Arrays.stream(annotations)
-                        .map(a -> AnnotationInstance.create(a.name(), target, a.values()))
-                        .toArray(AnnotationInstance[]::new))
-                // End
-                .map(this::read)
-                .map(responses -> responses.extensions(extensionIO().readExtensible(annotation)))
-                .orElse(null);
+        throw new UnsupportedOperationException();
     }
 
     @Override

core/src/main/java/io/smallrye/openapi/runtime/scanner/spi/AnnotationScanner.java

@@ -376,25 +376,25 @@ default void processResponse(final AnnotationScannerContext context, final Class
 
         setJsonViewContext(context, context.annotations().getAnnotationValue(method, JacksonConstants.JSON_VIEW));
 
-        Optional<APIResponses> classResponses = Optional.ofNullable(context.io().apiResponsesIO().read(resourceClass));
-        Map<String, APIResponse> classResponse = context.io().apiResponsesIO().readSingle(resourceClass);
-        addResponses(operation, classResponses, classResponse, false);
+        // Add responses from the class
+        addResponses(operation, context.io().apiResponsesIO().read(resourceClass), false);
 
         // Method annotations override class annotations
-        Optional<APIResponses> methodResponses = Optional.ofNullable(context.io().apiResponsesIO().read(method));
-        Map<String, APIResponse> methodResponse = context.io().apiResponsesIO().readSingle(method);
-        addResponses(operation, methodResponses, methodResponse, true);
+        addResponses(operation, context.io().apiResponsesIO().read(method), true);
 
         context.io().apiResponsesIO().readResponseSchema(method)
                 .ifPresent(responseSchema -> addApiReponseSchemaFromAnnotation(responseSchema, method, operation));
 
         /*
-         * If there is no response from annotations, try to create one from the method return value.
-         * Do not generate a response if the app has used an empty @ApiResponses annotation. This
-         * provides a way for the application to indicate that responses will be supplied some other
+         * If an empty @APIResponses annotation is not present on the method,
+         * attempt to generate additional response information based on the
+         * HTTP method and the method's return type.
+         *
+         * The presence of an empty @APIResponses provides a way for the
+         * application to indicate that responses will be supplied some other
          * way (i.e. static file).
          */
-        if (methodResponses.isPresent() || context.io().apiResponsesIO().getAnnotation(method) == null) {
+        if (!hasEmptyAPIResponsesAnnotation(context, method)) {
             createResponseFromRestMethod(context, method, operation);
         }
 
@@ -418,34 +418,50 @@ default void processResponse(final AnnotationScannerContext context, final Class
         clearJsonViewContext(context);
     }
 
-    default void addResponses(Operation operation, Optional<APIResponses> responses, Map<String, APIResponse> singleResponse,
-            boolean includeExtensions) {
-        responses.ifPresent(resp -> {
-            resp.getAPIResponses().forEach(ModelUtil.responses(operation)::addAPIResponse);
-            if (includeExtensions && resp.getExtensions() != null) {
-                resp.getExtensions().forEach(ModelUtil.responses(operation)::addExtension);
+    private boolean hasEmptyAPIResponsesAnnotation(AnnotationScannerContext context, MethodInfo method) {
+        AnnotationInstance responsesAnno = context.io().apiResponsesIO().getAnnotation(method);
+
+        if (responsesAnno == null) {
+            return false;
+        }
+
+        AnnotationInstance[] responsesAnnoValue = context.annotations().value(responsesAnno);
+        return (responsesAnnoValue == null || responsesAnnoValue.length == 0);
+    }
+
+    private void addResponses(Operation operation, APIResponses responses, boolean includeExtensions) {
+        if (responses != null) {
+            APIResponses operationResponses = ModelUtil.responses(operation);
+            responses.getAPIResponses().forEach(operationResponses::addAPIResponse);
+
+            if (includeExtensions) {
+                operationResponses.setExtensions(responses.getExtensions());
             }
-        });
-        if (singleResponse != null) {
-            singleResponse.forEach(ModelUtil.responses(operation)::addAPIResponse);
         }
     }
 
     /**
-     * Called when a scanner (jax-rs, spring) method's APIResponse annotations have all been processed but
-     * no response was actually created for the operation.This method will create a response
-     * from the method information and add it to the given operation. It will try to do this
-     * by examining the method's return value and the type of operation (GET, PUT, POST, DELETE).
+     * Called when a scanner (jax-rs, spring) method's APIResponse annotations
+     * have all been processed but no response content was created for the
+     * operation.
      *
-     * If there is a return value of some kind (a non-void return type) then the response code
-     * is assumed to be 200.
+     * This method will create a response from the method information
+     * and add it to the given operation. It will try to do this by examining
+     * the method's return value and the type of operation (GET, PUT, POST,
+     * DELETE).
      *
-     * If there not a return value (void return type) then either a 201 or 204 is returned,
-     * depending on the type of request.
+     * If there is a return value of some kind (a non-void return type) then the
+     * response code is assumed to be 200.
      *
-     * @param context the scanning context
-     * @param method the current method
-     * @param operation the current operation
+     * If there not a return value (void return type) then either a 201 or 204
+     * is returned, depending on the type of request.
+     *
+     * @param context
+     *        the scanning context
+     * @param method
+     *        the current method
+     * @param operation
+     *        the current operation
      */
     default void createResponseFromRestMethod(final AnnotationScannerContext context,
             final MethodInfo method,

extension-jaxrs/src/main/java/io/smallrye/openapi/jaxrs/JaxRsAnnotationScanner.java

@@ -323,7 +323,7 @@ private Map<DotName, Map<String, APIResponse>> processExceptionMappers() {
             MethodInfo mapperMethod = mapperAnnotation.target().asMethod();
             if (mapperMethod.parametersCount() == 1) {
                 DotName exceptionName = mapperMethod.parameterType(0).name();
-                exceptionMappers.put(exceptionName, context.io().apiResponsesIO().readAll(mapperMethod));
+                exceptionMappers.put(exceptionName, context.io().apiResponseIO().readMap(mapperMethod));
             }
         }
 
@@ -349,14 +349,14 @@ private Map<DotName, Map<String, APIResponse>> exceptionResponseAnnotations(Clas
                 .of(classInfo.method(JaxRsConstants.TO_RESPONSE_METHOD_NAME, exceptionType))
                 .filter(Objects::nonNull)
                 .flatMap(m -> context.io()
-                        .apiResponsesIO()
-                        .readAll(m)
+                        .apiResponseIO()
+                        .readMap(m)
                         .entrySet()
                         .stream());
 
         Stream<Entry<String, APIResponse>> classAnnotations = context.io()
-                .apiResponsesIO()
-                .readAll(classInfo)
+                .apiResponseIO()
+                .readMap(classInfo)
                 .entrySet()
                 .stream();
 

extension-jaxrs/src/test/java/io/smallrye/openapi/runtime/scanner/ApiResponseTests.java

@@ -1,6 +1,10 @@
 package io.smallrye.openapi.runtime.scanner;
 
 import java.io.IOException;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -11,6 +15,7 @@
 import org.eclipse.microprofile.openapi.annotations.media.Schema;
 import org.eclipse.microprofile.openapi.annotations.responses.APIResponse;
 import org.eclipse.microprofile.openapi.annotations.responses.APIResponseSchema;
+import org.eclipse.microprofile.openapi.annotations.responses.APIResponses;
 import org.json.JSONException;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -454,4 +459,48 @@ public jakarta.ws.rs.core.Response handle() {
                 (java.io.InputStream) null,
                 DataApi.class));
     }
+
+    @Target(ElementType.METHOD)
+    @Retention(RetentionPolicy.RUNTIME)
+    @APIResponses({ // NOSONAR - both @APIResponses + @APIResponse wanted directly
+            @APIResponse(responseCode = "400", description = "Really Bad Request"),
+            @APIResponse(responseCode = "406", description = "Not Acceptable")
+    })
+    @APIResponse(responseCode = "404", description = "Not found")
+    @interface WithClientErrors {
+    }
+
+    @Target(ElementType.METHOD)
+    @Retention(RetentionPolicy.RUNTIME)
+    @APIResponse(responseCode = "500", description = "Internal server error")
+    @APIResponses({ // NOSONAR - both @APIResponses + @APIResponse wanted directly
+            @APIResponse(responseCode = "502", description = "Bad Gateway"),
+            @APIResponse(responseCode = "504", description = "Gateway Timeout")
+    })
+    @interface WithServerErrors {
+    }
+
+    @Test
+    void testAnnotationComposition() throws IOException, JSONException {
+
+        @jakarta.ws.rs.Path("hello")
+        class HelloApi {
+
+            @jakarta.ws.rs.GET
+            @jakarta.ws.rs.Produces(jakarta.ws.rs.core.MediaType.TEXT_PLAIN)
+            @APIResponses({ // NOSONAR - both @APIResponses + @APIResponse wanted directly on method
+                    @APIResponse(responseCode = "200", description = "OK"),
+                    @APIResponse(responseCode = "301", description = "See Other")
+            })
+            @WithClientErrors
+            @WithServerErrors
+            @APIResponse(responseCode = "403", description = "Not Authorized")
+            public String sayHello() {
+                return "Goodbye";
+            }
+        }
+
+        assertJsonEquals("responses.multi-location-composition.json", HelloApi.class, WithClientErrors.class,
+                WithServerErrors.class);
+    }
 }

extension-jaxrs/src/test/resources/io/smallrye/openapi/runtime/scanner/responses.multi-location-composition.json

@@ -0,0 +1,45 @@
+{
+  "openapi" : "3.1.0",
+  "paths" : {
+    "/hello" : {
+      "get" : {
+        "responses" : {
+          "403" : {
+            "description" : "Not Authorized"
+          },
+          "404" : {
+            "description" : "Not found"
+          },
+          "500" : {
+            "description" : "Internal server error"
+          },
+          "200" : {
+            "description" : "OK",
+            "content" : {
+              "text/plain" : {
+                "schema" : {
+                  "type" : "string"
+                }
+              }
+            }
+          },
+          "301" : {
+            "description" : "See Other"
+          },
+          "400" : {
+            "description" : "Really Bad Request"
+          },
+          "406" : {
+            "description" : "Not Acceptable"
+          },
+          "502" : {
+            "description" : "Bad Gateway"
+          },
+          "504" : {
+            "description" : "Gateway Timeout"
+          }
+        }
+      }
+    }
+  }
+}