openapi-processor / openapi-processor-core

moved into openapi-processor-base
Apache License 2.0
7 stars 5 forks source link

better response type handling #68

Closed hauner closed 3 years ago

hauner commented 3 years ago

If an api has multiple response content types (success & errors) the java response type of the controller method will be Object to allow return values with different java types.

If the errors are handled by throwing an exception there is no need to have an Object response and the controller method could use the type of the success response to improve readability of the interface.

mapping could be like this, using a globally applied option:

# mapping.yaml
openapi-processor-mapping: v2

options:
  response-type: all    # all (default) or success

or like this, allowing endpoint & endpoint/method configuration:

# mapping.yaml
openapi-processor-mapping: v2

map:
  response-type: all    # all (default) or success

all: consider all response types to select the method return type success: consider only the success response type, i.e. usually 200

JonasGroeger commented 3 years ago

In the Spring module this is also annoying since even if you put

map:
  result: org.springframework.http.ResponseEntity

it will generate ResponseEntity<?> and not ResponseEntity<SuccessResponse>.

Could you give me some hints on how you would implement it? I'm willing to write an implementation.

Reference: https://docs.openapiprocessor.io/spring/2021.3/mapping/result.html

hauner commented 3 years ago

Yes, I think that has the same cause.

Thanks for the offer to help -)

Here are a few notes to get you started.

I did a few lines on this to get the new option but that's just a minimal start that does not yet do anything. It is on this branch: https://github.com/openapi-processor/openapi-processor-core/tree/response-type-handling

Current idea is to replace the return type of the controller method in the MethodWriter here:

https://github.com/openapi-processor/openapi-processor-core/blob/e25f7ab3c450143edc5b4abec21a9da1e0fbefe7/src/main/kotlin/io/openapiprocessor/core/writer/java/MethodWriter.kt#L47

This is the line that adds the response type to the method. Replacing ${endpointResponse.responseType} with a method that does select the return type based on the new option should solve the issue. If the new option is all use the current behaviour if it is success use the success response type and ignore the error responses.

The code to get the wanted type exists and is currently private in the EndpointResponse:

https://github.com/openapi-processor/openapi-processor-core/blob/e25f7ab3c450143edc5b4abec21a9da1e0fbefe7/src/main/kotlin/io/openapiprocessor/core/model/EndpointResponse.kt#L37

Note that the identical selection is required to create the import list.

Getting that option is probably the harder part. Converting the mapping.yaml to the internal format requires a couple of steps and is more complex than I like.

If the options is configured like this

# mapping.yaml
openapi-processor-mapping: v2

map:
  response-type: all    # all (default) or success

the MethodWriter will need the MappingFinder to get access to it and then call a similiar function to https://github.com/openapi-processor/openapi-processor-core/blob/e25f7ab3c450143edc5b4abec21a9da1e0fbefe7/src/main/kotlin/io/openapiprocessor/core/converter/mapping/MappingFinder.kt#L122 to get the value of new mapping option.

The maps: part of the mapping.yaml is converted internally to Mappings. The MappingFinder uses them to find & select mappings.

The new option will need a new implementation of the Mapping interface: https://github.com/openapi-processor/openapi-processor-core/blob/response-type-handling/src/main/kotlin/io/openapiprocessor/core/converter/mapping/Mapping.kt

Converting the yaml to the Mapping happens here: https://github.com/openapi-processor/openapi-processor-core/blob/response-type-handling/src/main/kotlin/io/openapiprocessor/core/processor/mapping/v2/MappingConverter.kt

I hope this helps and i didn't miss any annoying detail that breaks this approach :)

hauner commented 3 years ago

collecting the import happens here https://github.com/openapi-processor/openapi-processor-core/blob/e25f7ab3c450143edc5b4abec21a9da1e0fbefe7/src/main/kotlin/io/openapiprocessor/core/writer/java/InterfaceWriter.kt#L65

hauner commented 3 years ago

@JonasGroeger Hi, I have it nearly running.... I hope you didn't waste to much on on it.

JonasGroeger commented 3 years ago

@JonasGroeger Hi, I have it nearly running.... I hope you didn't waste to much on on it.

Please don't feel obligated to implement something just because somebody wants it. It's open source after all.

In case you didnt: all good ;)

hauner commented 3 years ago

It something I like to see myself :-) A colleague mentioned it and then your comment on the issue. Funny is, that it all happend in a couple of days. ;-)