[Java] custom response wrapper

Add new configuration option customResponseWrapper to java generators. There are cases, when user might want to wrap all responses with common object that can represent information common to all responses(like status, custom message, common error format etc). For such case I've added this new option.

Now user can create & maintain custom response wrapper and add it via new option

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh

  Commit all changed files. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*. For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master (6.1.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Java	@bbdouglas (2017/07) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01) @karismann (2019/03) @Zomzog (2019/04) @lwlee2608 (2019/10)
Java Spring	@cachescrubber (2022/02) @welshm (2022/02) @MelleD (2022/02) @atextor (2022/02) @manedev79 (2022/02) @javisst (2022/02) @borsch (2022/02) @banlevente (2022/02) @Zomzog (2022/09)

Created by: welshm

Couldn't the same behavior be achieved by creating a response wrapper component in the API itself?

Wouldn't this have issues of generated clients not sharing the same response type and therefore not being able to parse the response?

@welshm

Couldn't the same behavior be achieved by creating a response wrapper component in the API itself?

I don't think this is possible as for now. Only approach that I can think of is to create a base class and later using multiple allOf to extend those classes. Also this is not the same since it will extend class, while my approach is to wrap them

Wouldn't this have issues of generated clients not sharing the same response type and therefore not being able to parse the response?

Not sure that I got you. If you talk about other client libraries then probably we can apply this feature to them as well

Created by: welshm

@welshm

Couldn't the same behavior be achieved by creating a response wrapper component in the API itself?

I don't think this is possible as for now. Only approach that I can think of is to create a base class and later using multiple allOf to extend those classes. Also this is not the same since it will extend class, while my approach is to wrap them

I'll give this a bit more thought and try to create an example - but I think using additionalProperties to allow a dictionary of different response values to be set would be an option.

Wouldn't this have issues of generated clients not sharing the same response type and therefore not being able to parse the response?

Not sure that I got you. If you talk about other client libraries then probably we can apply this feature to them as well

My concern is that if this is enabled on a server, no client generated from the specification will succeed in parsing responses from the server because the specification does not include the wrapping class - so they will fail to decode any response from the server unless they have the same wrapping class defined. Because of that, I feel like it should be part of the specification - otherwise you're requiring that server and client keep their wrappers the same but without a shared specification. However, this is an optional flag so I suppose it doesn't need to be used at all if someone doesn't want to.

Created by: welshm

@welshm

Couldn't the same behavior be achieved by creating a response wrapper component in the API itself?

I don't think this is possible as for now. Only approach that I can think of is to create a base class and later using multiple allOf to extend those classes. Also this is not the same since it will extend class, while my approach is to wrap them

I'll give this a bit more thought and try to create an example - but I think using additionalProperties to allow a dictionary of different response values to be set would be an option.

Wouldn't this have issues of generated clients not sharing the same response type and therefore not being able to parse the response?

Not sure that I got you. If you talk about other client libraries then probably we can apply this feature to them as well

My concern is that if this is enabled on a server, no client generated from the specification will succeed in parsing responses from the server because the specification does not include the wrapping class - so they will fail to decode any response from the server unless they have the same wrapping class defined. Because of that, I feel like it should be part of the specification - otherwise you're requiring that server and client keep their wrappers the same but without a shared specification. However, this is an optional flag so I suppose it doesn't need to be used at all if someone doesn't want to.

@borsch

I would probably try to use oneOf mappings to achieve this with a common response object.

    RequestTypeEnum:
      type: string
      enum: [REQUEST_A, REQUEST_B]
     RequestTypeOneOfEnumMappingDisc:
      type: object
      properties:
        requestType:
          $ref: "#/components/schemas/RequestTypeEnum"
      required: requestType
      oneOf:
        - $ref: '#/components/schemas/RequestA'
        - $ref: '#/components/schemas/RequestB'
      discriminator:
        propertyName: requestType
        mapping:
          REQUEST_A: '#/components/schemas/RequestA'
          REQUEST_B: '#/components/schemas/RequestB'
    RequestA:
      type: object
      required:
        - seeds
      properties:
        seeds:
          type: integer
    RequestB:
      type: object
      required:
        - length
      properties:
        length:
          type: integer

However, like I said, this is an option that can be used and doesn't need to be used - if you see value in it, I don't see any reason to not merge this code

Created by: welshm

Review: Approved

is the custom response wrapper similar to ApiResponse object, e.g. https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/ApiResponse.mustache ?

Created by: cachescrubber

I agree with @welshm. One could use allOf or oneOf to abstract the actual type of a return value.

@borsch is the custom response wrappers replacement or alternative to Springs ResponseEntity<?>. Or would we get signatures like ResponseEntity<my.company.CustomeResponseWrapper>?

is the custom response wrapper similar to ApiResponse object, e.g. https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/ApiResponse.mustache ?

@wing328 yes, idea is similar, but this new wrapper is something that would be controlled by client. Using this wrapper client would be able to add additional and common fields to all responses

I agree with @welshm. One could use allOf or oneOf to abstract the actual type of a return value. @borsch is the custom response wrappers replacement or alternative to Springs ResponseEntity<?>. Or would we get signatures like ResponseEntity<my.company.CustomeResponseWrapper>?

1. it would be additional wrapper. check this test assert - http://metis.lti.cs.cmu.edu:8023/OpenAPITools/openapi-generator/-/merge_requests/13873/files#diff-4ce5fb739894dacffe829d7ec59dea6513fb16d9e2cfd2ab495098302037382fR1312
2. I understand the idea with allOf/oneOf, but I think this is too complicated when you have at least 20 different response types(DTOs)

Note: I understand that this is something that is not controlled by specification and therefore might cause issues, but since this is a custom/optional change nobody is obligated to use it

yes, idea is similar, but this new wrapper is something that would be controlled by client. Using this wrapper client would be able to add additional and common fields to all responses

Can you please elaborate on the use case to add "additional and common fields to all responses"?

I think that's one of the primary goal of having ApiReponse, which stores the stats code, headers etc in the HTTP response from the server.

Can you please elaborate on the use case to add "additional and common fields to all responses"?

I think that's one of the primary goal of having ApiReponse, which stores the stats code, headers etc in the HTTP response from the server.

That is something pretty specific to my current project and probably doesn't make any sense to 98-99% of projects

closed