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

[Spring] can't handle multiple responses #1096

Open
mwoodland opened this issue Sep 24, 2018 · 17 comments
Open

[Spring] can't handle multiple responses #1096

mwoodland opened this issue Sep 24, 2018 · 17 comments

Comments

@mwoodland
Copy link
Contributor

Description

We have an OpenAPI spec file where we've defined multiple possible responses for an operation, however when the Spring code is generated the generated @RequestMapping method is only capable of returning the first response defined.

openapi-generator version

3.2.3

OpenAPI declaration file content or url
openapi: "3.0.0"
info:
  title: "Example"
  version: "1.0.0"
servers:
  - url: "/api/v1"
  - url: "/api/"
tags:
  - name: "example"
paths:
  "/example":
    get:
      tags:
        - "example"
      operationId: "examples"
      summary: "Get examples that match criteria"
      parameters:
      - name: "id"
        in: query
        description: "the identifier to get"
        required: true
      responses:
        200:
          description: "Success"
          content:
            application/json:
              schema:
                $ref: "#components/schemas/ExampleObject"
        404:
          description: "Failed to find the Example Object"
          content:
            application/json:
              schema:
                $ref: "#components/schemas/Errors"
components:
  schemas:
    "ExampleObject":
      type: object
      properties:
        "property1":
          type: string
        "property2":
          type: string
    "Errors":
      type: object
      properties:
        "code":
          type: integer
          format: int32
        "severity":
          type: string
        "message":
          type: string
Command line used for generation
<pluginExecutor>
    <plugin>
        <groupId>org.openapitools</groupId>
        <artifactId>openapi-generator-maven-plugin</artifactId>
        <version>3.2.3</version>
    </plugin>
    <goal>generate</goal>
    <configuration>
        <inputSpec>api-def/spec.yml</inputSpec>
        <generatorName>spring</generatorName>
        <output>api-gen</output>
        <modelNamePrefix>Api</modelNamePrefix>
        <configOptions>
            <basePackage>package</modelPackage>
            <apiPackage>package</apiPackage>
            <groupId>group</groupId>
            <artifactId>api-gen</artifactId>
            <artifactVersion>${project.version}</artifactVersion>
            <artifactDescription>Automatically generated interfaces for APIs</artifactDescription>
            <interfaceOnly>true</interfaceOnly>
        </configOptions>
    </configuration>
</pluginExecutor>
Steps to reproduce

Generate the Spring code using the specified spec file and maven configuration.
The following @RequestMapping is generated:

    @ApiOperation(value = "Get examples that match criteria", nickname = "examples", notes = "", response = ApiExampleObject.class, tags={ "example", })
    @ApiResponses(value = { 
        @ApiResponse(code = 200, message = "Success", response = ApiExampleObject.class),
        @ApiResponse(code = 404, message = "Failed to find the Example Object", response = ApiErrors.class) })
    @RequestMapping(value = "/example",
        produces = { "application/json" }, 
        method = RequestMethod.GET)
    default ResponseEntity<ApiExampleObject> examples(@NotNull @ApiParam(value = "the identifier to get", required = true) @Valid @RequestParam(value = "id", required = true)  ) {
        getRequest().ifPresent(request -> {
            for (MediaType mediaType: MediaType.parseMediaTypes(request.getHeader("Accept"))) {
                if (mediaType.isCompatibleWith(MediaType.valueOf("application/json"))) {
                    ApiUtil.setExampleResponse(request, "application/json", "{  \"property2\" : \"property2\",  \"property1\" : \"property1\"}");
                    break;
                }
            }
        });
        return new ResponseEntity<>(HttpStatus.NOT_IMPLEMENTED);

    }

The return type is of ResponseEntity<ApiExampleObject> which means I can't return an ApiErrors object.

Related issues/PRs

Sounds similar to #316

Suggest a fix/enhancement

Get the method to return an object that can encapsulate one of the types defined in the response.

Or a simpler option is to get the method to return a ResponseEntity<Object>.

@tilowestermann
Copy link

I wonder that this issue has such a low activity. Is there any recommended workaround, or do you plan to fix it in an upcoming release?

@alexjoybc
Copy link

Is there any update on this issue? we are facing the same problem where we have to return different objects depending on the status of the response....

@pmatysek
Copy link

Any news on it?
The only solution I've found is to edit api.mustache and add something like:
{{#responses.1}}ResponseEntity<?>{{/responses.1}} {{^responses.1}}{{>returnTypes}}{{/responses.1}}

@tigerinus
Copy link

tigerinus commented Aug 4, 2021

Would be nice to support ResponseEntity<? extends Order> when we have allOf inheritance in the schema:

    OrderResponseOK:
      description: OK
      content:
        application/json:
          schema:
            allOf:
              - $ref: "#/components/schemas/Order"
            properties:
              message:
                readOnly: true
                type: string
                example: "OK"

@tloubrieu-jpl
Copy link

tloubrieu-jpl commented Feb 17, 2023

Hello,

I was coming back here to see if there is any progress on that.

If that can help, our workaround so far is to add a type 'object' to all of the possible response types so that the API end-point takes ResponseEntity as the default returned type.

We do it this way:

The end-point definition:

 /products/{identifier}/collections/latest:
    get:
      tags:
        - deprecated
      summary: deprecated
      operationId: products-lidvid-collections-latest
      responses:
        '200':
          $ref: "#/components/responses/Plural"
        '400':
          $ref: "#/components/responses/Error"
        '404':
          $ref: "#/components/responses/Error"
       ...

And:

responses:
   Error:
     description: Unsuccessful request
     content:
       "*":
         schema:
           type: object
       "*/*":
         schema:
           $ref: '#/components/schemas/errorMessage'
       application/csv:
         schema:
           $ref: '#/components/schemas/errorMessage'
       ...
   Plural:
     description: Successful request
     content:
       "*":
         schema:
           type: object
       "*/*":
         schema:
           $ref: '#/components/schemas/pdsProducts'
       application/csv:
         schema:
           $ref: '#/components/schemas/wyriwygProducts'
    ....

This does not look good, but we did not find a better work-around...

@massahud
Copy link

massahud commented May 10, 2023

One workaround is to define all responses as Response and the schema like this:

    SuccessResponse:
      type: object
      properties:
        message:
          type: string

    ErrorResponse:
      type: object
      properties:
        errors:
          type: array
          items:
            type: string

    Response:
      oneOf:
        - $ref: '#/components/schemas/SuccessResponse'
        - $ref: '#/components/schemas/ErrorResponse'

SuccessResponse and ErrorResponse models will implement Response, generated as an empty interface.

@davidmoten
Copy link

davidmoten commented May 15, 2023

So I guess the controller method is typed based on what it thinks is the primary response. You can trick it to return something else because of generics type erasure:

 return (ResponseEntity<ApiExampleObject>)(ResponseEntity<?>) differentResponse;

I'm curious, why did the project owners not answer this? It's been open since 2018 and is a very important use-case with a simple answer.

@massahud
Copy link

@davidmoten why the thumbs down? Is there a problem with the workaround?

@davidmoten
Copy link

davidmoten commented May 17, 2023

@davidmoten why the thumbs down? Is there a problem with the workaround?

No it's a valid workaround, I removed the thumbs down. Easier and generally more practical just to learn the casting trick but I do like the extra type safety your workaround gives. Not practical to rework large complex third-party apis just to get that feature though. It could be done by the openapi-generator generator where it creates a oneOf type class that also checks validity of type codes (without you having to do it).

@denis-dbm
Copy link

Up

Does this have preview of planning/release? This feature is useful for good design, code and api.

@axel7083
Copy link

To solve the problems of being able to return multiple type of object while keeping the existing logic, we could take advantage of the fact that, other response than 200 are related to a problem (aka an Exception.) Therefore we could use the java native extension system to be able to return the type corresponding to the code we want.

For example give

  /dummy:
    get:
      description: dummy get
      operationId: getDummy
      responses:
        '200':
          $ref: '#/components/responses/ValidResponse'
        '400':
          $ref: '#/components/responses/WrongResponse'
        '404':
          $ref: '#/components/responses/NotFoundResponse'

In the current generator we generate something like

@Override
public ResponseEntity<ValidResponse> getDummy() {
    // our implementation
}

Using java exception, for example, the openapi generator could generate a class

public class GetDummyException extends Exception {
    public GetDummyException(WrongResponse) { /*handle*/ }
    public GetDummyException(NotFoundResponse) { /*handle*/ }
}

This would for example change the signature of our getDummy method to

@Override
public ResponseEntity<ValidResponse> getDummy() throws GetDummyException   {
    // our implementation
}

What do you think @davidmoten

@davidmoten
Copy link

davidmoten commented Nov 14, 2023

I like that @axel7083. It's like the oneOf return but isolated to the non-2xx responses.

  • I'm not sure exactly what we do with an error response that is not listed in the definition though (like a 500), a different exception perhaps in that case.
  • Suppose all my methods had the same Error responses. I might want to handle them all in the same way. To help, this new exception should implement an interface that gets the deserialised response object. That's an easy addition.
  • A RuntimeException might be better than a checked Exception

@axel7083
Copy link

I like that @axel7083. It's like the oneOf return but isolated to the non-2xx responses.

  • I'm not sure exactly what we do with an error response that is not listed in the definition though (like a 500), a different exception perhaps in that case.
  • Suppose all my methods had the same Error responses. I might want to handle them all in the same way. To help, this new exception should implement an interface that gets the deserialised response object. That's an easy addition.
  • A RuntimeException might be better than a checked Exception

Thanks for the anwser! Glad to see it got some interest.

In our team, we face very frequently the issue of dealing with returning something for non-2XX response, aiming to provide more informations to the api clients when something got wrong.

I would be interested to contribute on it, as we would benefits from it!

@axel7083
Copy link

I like that @axel7083. It's like the oneOf return but isolated to the non-2xx responses.

  • I'm not sure exactly what we do with an error response that is not listed in the definition though (like a 500), a different exception perhaps in that case.
  • Suppose all my methods had the same Error responses. I might want to handle them all in the same way. To help, this new exception should implement an interface that gets the deserialised response object. That's an easy addition.
  • A RuntimeException might be better than a checked Exception

@davidmoten I gave it a try!

Based on the discussion I took a look at the api.mustache for JavaSpring (version 6.5.0).

First I created a custom Exception class inside the interface (why not)

    public class {{classname}}Exception extends Exception {
        HttpStatus status;
        Object res;
        public ResponseEntity<?> getResponseEntity() {
            return new ResponseEntity(res, status);
        }
    {{#responses}}
        public {{classname}}Exception({{{baseType}}} response) {
            status = HttpStatus.resolve({{code}});
            res = response;
        }
    {{/responses}}
    }

The idea mainly is to use the delegatePattern to create a function that will try/catch the user overwrited method.

E.g.

Given a very basic endpoint

paths:
  /test:
    get:
      summary: test
      operationId: test
      parameters:
        - $ref: "#/components/parameters/TestCode"
      responses:
        "200":
          $ref: "#/components/responses/Test200"
        "404":
          $ref: "#/components/responses/Test404"
        "500":
          $ref: "#/components/responses/Test500"

TestApiException

We generate a TestApi interface, and inside a TestApiException defined as followed (it has been generated from the mustache snippet given above.

public class TestApiException extends Exception {
        HttpStatus status;
        Object res;
        public ResponseEntity<?> getResponseEntity() {
            return new ResponseEntity(res, status);
        }
        public TestApiException(S200Test response) {
            status = HttpStatus.resolve(200);
            res = response;
        }
        public TestApiException(S404Test response) {
            status = HttpStatus.resolve(404);
            res = response;
        }
        public TestApiException(S500Test response) {
            status = HttpStatus.resolve(500);
            res = response;
        }
    }

TestApi methods

The delegate method _test will be generated, and will look like something like that:

Delegate

The big change is that the ResponseEntity has generic parameter ? to ensure no issue while returning specific type.

.... header
default ResponseEntity<?> _test(
        @NotNull @Parameter(name = "code", description = "", required = true, in = ParameterIn.QUERY) @Valid @RequestParam(value = "code", required = true) Integer code
    ) throws Exception {
        try {
            return test(code);
        } catch (TestApiException e) {
            return e.getResponseEntity();
        }
    }

Method to override by user

The base method to override keep the type of the 200 response code. This is to avoid regression and keep logic the use of exception for non-200 response code.

// Override this method
default  ResponseEntity<S200Test> test(Integer code) throws Exception {
    return new ResponseEntity<>(HttpStatus.NOT_IMPLEMENTED);
}

Here is on our example the user can thrown exception, and the proper http code will be used.

This behavior is probably not wanted for all if we want to use the same object for multiple response code, nothing is handling it here. (We could simply in this case add the possbility to set the http status when throwing the exception.

@Controller
public class TestController implements TestApi {
    @Override
    public ResponseEntity<S200Test> test(Integer code) throws Exception {
        return switch (code) {
            case 200 -> new ResponseEntity<>(new S200Test(), HttpStatus.OK);
            case 404 -> throw new TestApiException(new S404Test());
            case 500 -> throw new TestApiException(new S500Test());
            default -> throw new RuntimeException("Bad code");
        };
    }
}

I am uploading the api.mustache I used (based on 6.5.0) I am far from being good at editing such things, and some elements are not working, like returning array object, the List attribute is not used in the exception generated.
api.mustache.txt

@josephcuellar2662
Copy link

Will 2024 be the year they resolve this defect? 🤔

@bas-kirill
Copy link

Will 2024 be the year they resolve this defect? 🤔

See it in 2025 🙂 (i hope)

@Guillaume-d-o
Copy link

Hi everyone, I also had this problem. after a day working on that issue I think the openApi vision was to create an interface with return value "Strict". For the case with error => you must use ControllerAdvices. And the fact is that it is simpler to do that for an app.

For example :

`@ControllerAdvice
@RestControllerAdvice
public class GlobalExceptionHandler {

@ExceptionHandler(InternalServerException.class)
public ResponseEntity<ErrorResponse> handleInternalServerException(InternalServerException ex) {
    ErrorResponse errorResponse = new ErrorResponse("",ex.getMessage(), ex.getMessage());
    return new ResponseEntity<>(errorResponse, HttpStatus.INTERNAL_SERVER_ERROR);
}

}`
(in this example i only show one of three, but i did the same for 404 and 409). Be carefull to use the ErrorResponse custom class generated by openApi, otherwise you will have only basic 500 errors if you use the bad one.

@Override public ResponseEntity<Data> getData(@NotEmpty @PathVariable("uuid") UUID uuid) { Data result = service.searchDataset(uuid); return new ResponseEntity<>(result, HttpStatus.OK);

responses:
  200:
    description: Data
    content:
      application/json:
        schema:
          $ref: 'browse-schemas.yaml#/Data'
  409:
    description: Conflict
    content:
      application/json:
        schema:
          $ref: 'browse-schemas.yaml#/ErrorResponse'
  500:
    description: Internal error
    content:
      application/json:
        schema:
          $ref: 'browse-schemas.yaml#/ErrorResponse'
  404:
    description: Not Found
    content:
      application/json:
        schema:
          $ref: 'browse-schemas.yaml#/ErrorResponse'

I test all exception and they all work.
Exception must extends directly or not of RuntimeException

If you don't understand spring exception management you can also watch this video : https://www.youtube.com/watch?v=PzK4ZXa2Tbc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests