ajeans commented 4 years ago

Bug Report Checklist

[X] Have you provided a full/minimal spec to reproduce the issue?
[ ] Have you validated the input using an OpenAPI validator (example)?
[X] What's the version of OpenAPI Generator used?
[X] Have you search for related issues/PRs?
[ ] What's the actual output vs expected output?
[ ] [Optional] Bounty to sponsor the fix (example)

Description

So we have created a REST server based on standard spring framework conventions (RestController), the openapi spec is generated using springdoc-openapi from the server, and I managed to use the gradle openapi-generator plugin to create a "java/resttemplate" client. This is with the 4.3.1 openapi-generator plugin. Using an integration test, this worked fine so :+1:

But then we decided to make the server handle errors as described in RFC-7807 https://tools.ietf.org/html/rfc7807 As soon as I regenerated the openapi spec and the client from it, the test started failing.

Debugging this a bit, it seems that:

Adding the RFC-7807 for the error cases is properly reflected in the openapi spec, HTTP 200 declares a application/json content whereas HTTP 500 declares a application/problem+json content

"paths": {
"/": {
  "get": {
    "tags": [
      "home-controller"
    ],
    "summary": "Request information from the application",
    "operationId": "home",
    "responses": {
      "500": {
        "description": "Internal Server Error",
        "content": {
          "application/problem+json": {
            "schema": {
              "$ref": "#/components/schemas/ErrorDto"
            }
          }
        }
      },
      "200": {
        "description": "OK",
        "content": {
          "application/json": {
            "schema": {
              "$ref": "#/components/schemas/HomeResponseDto"
            }
          }
        }
      }
    }
  }
}
}

The client is properly generated using the gradle plugin, the main change is that each *ControllerApi now knows that it should accept the two media types

     final MultiValueMap<String, String> cookieParams = new LinkedMultiValueMap<String, String>();
     final MultiValueMap formParams = new LinkedMultiValueMap();
-    final String[] accepts = {"application/json"};
+    final String[] accepts = {"application/problem+json", "application/json"};
     final List<MediaType> accept = apiClient.selectHeaderAccept(accepts);
     final String[] contentTypes = {};
     final MediaType contentType = apiClient.selectHeaderContentType(contentTypes);

When launching the integration test however, what happens is that apiClient.selectHeaderAccept transforms {"application/problem+json", "application/json"} into a single MediaType.APPLICATION_PROBLEM_JSON because of its generated implementation.

As application/problem+json comes first, isJsonMime returns true and this is the only media type that is returned.

  public List<MediaType> selectHeaderAccept(String[] accepts) {
    if (accepts.length == 0) {
      return null;
    }
    for (String accept : accepts) {
      MediaType mediaType = MediaType.parseMediaType(accept);
      if (isJsonMime(mediaType)) {
        return Collections.singletonList(mediaType);
      }
    }
    return MediaType.parseMediaTypes(StringUtils.arrayToCommaDelimitedString(accepts));
  }

The client sends an HTTP Header Accept: application/problem+json which is rejected by the server as being an unexpected media type.

org.springframework.web.client.HttpClientErrorException$UnsupportedMediaType: 415 : [{"detail":"Accept type 'application/problem+json' not supported","exception-id":"2a0da9ab-7286-4e1c-ba18-d2c2ada028b3","status":415,"timestamp":"2020-08-05T07:18:16.138179Z","title":"Unsupported Media Type"}]   at org.springframework.web.client.HttpClientErrorException.create(HttpClientErrorException.java:133)
    at org.springframework.web.client.DefaultResponseErrorHandler.handleError(DefaultResponseErrorHandler.java:184)
    at org.springframework.web.client.DefaultResponseErrorHandler.handleError(DefaultResponseErrorHandler.java:125)
    at org.springframework.web.client.ResponseErrorHandler.handleError(ResponseErrorHandler.java:63)
    at org.springframework.web.client.RestTemplate.handleResponse(RestTemplate.java:782)
    at org.springframework.web.client.RestTemplate.doExecute(RestTemplate.java:740)
    at org.springframework.web.client.RestTemplate.exchange(RestTemplate.java:651)

openapi-generator version

This is with 4.3.1. I didn't test the 5.0.0 beta as the mustache template has the same selectHeaderAccept implementation (cf. https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/Java/libraries/resttemplate/ApiClient.mustache)

OpenAPI declaration file content or url

{
  "openapi": "3.0.1",
  "info": {
    "title": "Example",
    "description": "Example for JSON and problem JSON (RFC-7807)",
    "license": {
      "name": "Proprietary"
    },
    "version": "0.1.0-SNAPSHOT"
  },
  "servers": [
    {
      "url": "https://example.com/foo/v0",
      "description": "Generated server url"
    }
  ],
  "paths": {
    "/": {
      "get": {
        "tags": [
          "home-controller"
        ],
        "summary": "Request information from the application",
        "operationId": "home",
        "responses": {
          "500": {
            "description": "Internal Server Error",
            "content": {
              "application/problem+json": {
                "schema": {
                  "$ref": "#/components/schemas/ErrorDto"
                }
              }
            }
          },
          "200": {
            "description": "OK",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/HomeResponseDto"
                }
              }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "ErrorDto": {
        "type": "object",
        "properties": {
          "detail": {
            "type": "string"
          },
          "exception-id": {
            "type": "string",
            "format": "uuid"
          },
          "status": {
            "type": "integer",
            "format": "int32"
          },
          "timestamp": {
            "type": "string",
            "format": "date-time"
          },
          "title": {
            "type": "string"
          }
        }
      },
      "HomeResponseDto": {
        "type": "object",
        "properties": {
          "actuator": {
            "type": "string",
            "format": "url"
          },
          "swagger": {
            "type": "string",
            "format": "url"
          }
        }
      }
    }
  }
}

Generation Details

The gradle configuration is as follows

openApiGenerate {
    // see https://github.com/OpenAPITools/openapi-generator/blob/master/docs/generators/java.md
    generatorName = "java"
    library = "resttemplate" // spring rest template and jackson is what we want
    inputSpec = "$rootDir/$project.name/specs/sign-v3.0.yaml".toString()
    outputDir = "$rootDir/$project.name"
    // remove a lot of stuff that can be generated
    generateApiDocumentation = false
    generateApiTests = false
    generateModelTests = false
    generateModelDocumentation = false
    // define correct groups
    apiPackage = "com.quicksign.cases.worker.esig.client.api"
    modelPackage = "com.quicksign.cases.worker.esig.client.model"
    configOptions = [
      dateLibrary: "java8", // we have the java8 date library
      hideGenerationTimestamp: "true" // simpler diffs when regenerating
    ]
}

Steps to reproduce

I can provide a simplified gradle project if necessary.

Related issues/PRs

440 seems to be in the same group of "how do I deal with multiple mime types"

Suggest a fix

Naively, I would implement another method called isJsonProblemMime and only return early if the media type being tested is isJsonMime(mediaType) && !isJsonProblemMime(mediaType)

Happy to write a PR in that direction (or another) if that helps.

ajeans commented 4 years ago

As a sidenote, for now I workaround this by doing the following bash voodoo after the generation.

find . -name "*ControllerApi.java" | xargs sed -i 's/{"application\/problem+json", "application\/json"}/{"application\/json", "application\/problem+json"}/g'

The standard JSON mediatype will come first and be selected.

pgadura commented 3 years ago

@wing328 I just opened the same bug as this for the webclient generator (https://github.com/OpenAPITools/openapi-generator/issues/9543) & was planning on submitting a PR when I came across this one. Is there a reason this fix wasn't ported to the other client generators?

ajeans commented 3 years ago

@pgadura that was my first PR and I decided to limit the scope of the PR. You're right it would make sense in other clients.