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

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

eokoneyo
Copy link
Contributor

@eokoneyo eokoneyo commented Oct 21, 2021

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.

  • EDIT

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 #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.

@eokoneyo eokoneyo marked this pull request as ready for review October 21, 2021 13:28
@eokoneyo eokoneyo changed the title Add support to OAS query parameter serialization expectation in generated http request Add support for query parameters in generated http request to match OAS query parameters serialization expectation Oct 21, 2021
@Alan-Cha
Copy link
Collaborator

@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
Copy link
Contributor Author

@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
Copy link
Contributor Author

@Alan-Cha closing this in favour of #440

@eokoneyo eokoneyo closed this Jan 23, 2022
@eokoneyo eokoneyo deleted the patch/support-OAS-query-parameter-serialization branch January 23, 2022 17:18
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

Successfully merging this pull request may close these issues.

2 participants