IBM / openapi-to-graphql

Translate APIs described by OpenAPI Specifications (OAS) into GraphQL
https://developer.ibm.com/open/projects/openapi-to-graphql/
MIT License
1.6k stars 206 forks source link

Add support for query parameters in generated http request to match OAS query parameters serialization expectation #426

Closed eokoneyo closed 2 years ago

eokoneyo commented 2 years ago

Hi, I discovered that the style formatting for query parameters as defined in the Open API Specification is not being respected.

Given the partial definition;

"/api/path": {
  get: {
   "parameters" : [ {
          "name" : "parameters",
          "in" : "query",
          "required" : true,
          "style" : "form",
          "explode" : true,
          "schema" : {
            "$ref" : "#/components/schemas/SomeSchemaInput"
          }
    } ],
  }
}

where SomeSchemaInput has properties of filter and limit for example, I found that the http request made for the generated GraphQL resolver is of the format; http://{baseUrl}/api/path?parameters[limit]=10&parameters[offset]=10.

The http request url should be http://{baseUrl}/api/path?limit=10&offset=10 considering the provided values for style and explode, as defined in Open API specification.

I've added a PR to support this behaviour although both for objects and arrays, I have not made style: form the default as defined here https://swagger.io/docs/specification/serialization/ for queries.

I've also realized that even with the changes I've made some of the standard behaviour of arrays are ignored because of how the query params get constructed for object like primitives as seen here https://github.com/IBM/openapi-to-graphql/blob/master/packages/openapi-to-graphql/src/resolver_builder.ts#L1332-L1335, this related to https://github.com/IBM/openapi-to-graphql/pull/312 where a solution with used the native query string implementation was introduced. I'd be willing to take on the effort to make parameters behaviour in the package match the standard open api behaviour.

Alan-Cha commented 2 years ago

@eokoneyo Sorry for the late reply. Thanks again for this PR and your other ones. This looks very promising! I was wondering if you are still interested in trying to make a more complete solution to match the standard OpenAPI behavior. There is no pressure to do so. If not, we can just get this in. Thanks again!

eokoneyo commented 2 years ago

@Alan-Cha actually it would make sense to use this PR for a the complete solution that implements the standard OpenAPI behaviour, as I've actually stumbled on another scenario that needs some modification too. I'll make changes to this PR to cover all the cases I've stumbled on, and of course there are all non-breaking changes

eokoneyo commented 2 years ago

@Alan-Cha closing this in favour of https://github.com/IBM/openapi-to-graphql/pull/440