Direct and meta-annotations are mutual exclusive, but should be merged

[anma]

Meta-annotations can be used to group annotations like @ApiResponse. However, when you mix direct and meta annotations, only the direct annotations are considered and the meta-annotations are ignored.

For example, with the following code the generated documentation contains only the responses for code 200. The responses from the @DefaultApiResponses meta-annotations are ignored. If you remove the two direct @ApiResponse annotations (marked with XXX), then the documentation contains the responses for 400, 401, and 403 as expected. But you cannot have both.

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import com.swisscom.ais.managementapi.providers.Error;

import io.swagger.v3.oas.annotations.media.Content;
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.responses.ApiResponse;

@Target({ ElementType.METHOD, ElementType.TYPE })
@Retention(RetentionPolicy.RUNTIME)
@ApiResponse(responseCode = "400", description = "Bad request: missing or invalid parameters")
@ApiResponse(responseCode = "401", description = "Unauthorized: missing or invalid credentials")
@ApiResponse(responseCode = "403", description = "Access denied: missing required authority")
@interface DefaultApiResponses { }

@GET
@Operation(summary = "Return all accounts")
@ApiResponse(responseCode = "200", description = "Success",
    content = @Content(array = @ArraySchema(schema = @Schema(implementation = Acount.class)))) // XXX
@DefaultApiResponses
public Collection<Account> getAcounts() {
    // TODO return accounts
}

@GET
@Path("{id}")
@Operation(summary = "Return account for the given ID")
@ApiResponse(responseCode = "200", description = "Success",
    content = @Content(schema = @Schema(implementation = Acount.class))) // XXX
@DefaultApiResponses
public Account getAccount(@PathParam("id") long id) {
    // TODO return account with given ID
}

The cause of the problem is the implementation of io.swagger.v3.core.util.ReflectionUtils.getRepeatableAnnotations(Method, Class<A>), which considers meta-annotations only if the given method has no direct annotations. Mergin the direct annotations and meta-annotations would make meta-annotations much more useful here.

[ghost]

To second this, we have almost exactly the same use case, but have hit a different aspect of the same problem. We want to define a meta-annotation, but then allow specific methods to override particular responses. So for example, if a meta-annotation defining 200, 403, and 404 codes is on an endpoint and we add an @ApiResponse with a code of 200, the resulting schema should have the 403 and 404 from the meta-annotation and the 200 from the direct annotation.

That is not what happens now.

However, our experience doesn't quite match that of @anma .

An example from our [test] codebase:

@RestController
public class FakeController
{

    @GetMapping("/something")
    @ApiResponse(responseCode = "403", 
                content = @Content(schema = @Schema(oneOf = String.class)))
    @DefaultStatusCodes
    public void test()
    { /* do something here */ }
}

@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@Forbidden
// @NotFound // etc. etc.
public @interface DefaultStatusCodes {}

@Retention(RetentionPolicy.RUNTIME)
@ApiResponse(responseCode = "403", description = "Access Denied",
            content = @Content(schema = @Schema(accessMode = AccessMode.READ_ONLY, oneOf = Problem.class)))
public @interface Forbidden {}

This produces a spec which appears to ignore the directly-present annotation and use the one from the meta-annotation.

Swap the @ApiResponse and @DefaultStatusCodes lines, however, and the spec will reflect the meta-annotation and not the directly-present one.

:confounded: :angry:

---

As for fixing this, ideally I'd like to see:

1. Consume all annotations "on" the method (inherited, meta-annotations, direct, etc.)
2. Document what "consume" means above. Specifically, document the order of precedence when merging sets of annotations which overlap.
3. Now that it's a documented part of the API surface, add tests to ensure that the order of precedence remains stable going forward.
4. Update the Wiki with an example of a use case like the ones in this issue, explaining what approaches can be taken (ex: meta-annotation vs. annotated interfaces), and why a developer might want to use one over the other.

[ghost]

I'm even more confused now. Recompiling the test classes in question seems to change the behavior depending on what's compiled when.

So the overall bug "merging overlapping annotations is broken" still persists... but it's not clear that there's just one root cause of the behavior that we and @anma are encountering.

As far as fixing this, it seems like our sources of response information are:

• Annotations on the method.
• Meta-annotations on the method.
• Annotations on the interface that the method implements (if any).
• Annotations on the type

When reconciling overlapping statuses defined in the above sources, I would propose the following order of precedence:

1. Annotations directly present on the method.
2. Meta-annotations, ordered by object graph distance (such that @ApiResponse used in a meta-annotation takes precedence over one used in another annotation that is in turn used in the meta annotation.)
3. Annotations on the interface (same graph distance rule applies as above.)
4. Annotations on the type.

That first one is a bit of a problem in the case of multiple overlapping annotations. Annotation order is preserved in the bytecode, but AFAIK it's not firmly specified that reflection will return them in that order (and I haven't tested that it does.) Not sure what the right thing is here if order isn't reliable.