Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APIResponse and APIResponses should target TYPE as well #417

Closed
jvs64893 opened this issue Apr 1, 2020 · 12 comments · Fixed by #524
Closed

APIResponse and APIResponses should target TYPE as well #417

jvs64893 opened this issue Apr 1, 2020 · 12 comments · Fixed by #524

Comments

@jvs64893
Copy link

jvs64893 commented Apr 1, 2020

Currently APIResponse and APIResponses use:
@Target({ ElementType.METHOD })

The Swagger equivalent handles this differently (see
https://github.com/swagger-api/swagger-core/blob/master/modules/swagger-annotations/src/main/java/io/swagger/v3/oas/annotations/responses/ApiResponses.java):
@Target({METHOD, TYPE, ANNOTATION_TYPE})

This makes it possible to add a response to all @Operations in a class. It would be great if this was possible with MP OpenAPI as well!

@EricWittmann
Copy link
Contributor

Can you provide some pseudo/sample code that shows the use-case you're asking for. I believe this is about annotating an entire JAX-RS class with e.g. @APIResponse so that the response is applied to all operations defined in the class. I presume that would be useful for a number of error response conditions - in particular 500. So you don't have to repeatedly tag every method with a "Server Error" api response when of course every method could possibly throw that. Am I correct in my understanding of what this issue is referring to?

@jvs64893
Copy link
Author

@EricWittmann
You're correct, that is exactly the use case.

Sample code would be something like that:

@APIResponses(value = {@APIResponse(responseCode = "500",
        content = @Content(
            schema = @Schema(implementation = Error.class)),
        description = "Error")})
public class Test {

  @APIResponses(value = {
      @APIResponse(responseCode = "200",
          content = @Content(schema = @Schema(
              implementation = Test.class)),
          description = "Operation executed successfully"),
      @APIResponse(responseCode = "400", content = @Content(
          schema = @Schema(implementation = Error.class)),
          description = "Error")})
  @Operation(summary = "Testmethod", description = "Test")
  @GET
  public Response test() {
  ...
  }

}

The generated documentation would add the 500 error to all methods, plus the specific APIResponses that the methods have (200 + 400 in this case).

@EricWittmann
Copy link
Contributor

OK great thanks. I've marked this as a hangout topic for us to discuss on our next specification call.

@phillip-kruger
Copy link
Member

+1

1 similar comment
@EricWittmann
Copy link
Contributor

+1

@EricWittmann
Copy link
Contributor

@MikeEdgar and @arthurdm - this issue gets two resounding +1's from us. I think we should go ahead and tag this as "help wanted" or whatever the next step is to getting this done. Unless either of you has any reservations...

@MikeEdgar
Copy link
Member

+1 from me as well. This is a nice feature.

@arthurdm
Copy link
Member

+1

@jmini
Copy link
Contributor

jmini commented Jul 2, 2020

@jvs64893 will you propose a PR for that?

The annotation should be changed and I think it would be great to have a TCK case for this.

@renzodf
Copy link

renzodf commented Jan 20, 2021

Hello,

It would be great to also add the ANNOTATION_TYPE in Target like Swagger does. The main usecase I see is the definition of custom meta annotation like this one:

@ApiResponse(code="404", message="Resource not found")
public @interface MyResourceNotFound

allowing to save a lot of repetitive code.

The example is extracted from this Swagger topic: swagger-api/swagger-core#690

@szymonblaszczyk
Copy link

Both the original proposal and the extra suggestion by @renzodf would be a great addition to this lib's capacity to reduce the boilerplate written and time taken to document uniform and/or repetitive APIs.

Has any further action taken place in this direction? Is any assistance needed?

@MikeEdgar
Copy link
Member

Hi @szymonblaszczyk - if you have some bandwidth, I certainly think a PR with the suggested annotation changes and TCK test (tests?) to verify the changes to behaviour would be welcome!

I am also curious about feedback from others as well regarding the use of ANNOTATION_TYPE in the annotation's Target. I don't think it's a bad idea, but it might be best to consider something like that generally across all (or several) of the annotations.

CC: @EricWittmann, @jmini, @phillip-kruger

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment