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.61k stars 211 forks source link

oneOf + allOf children - fix for union graph types and link objects #425

Closed mzronek closed 2 years ago

mzronek commented 3 years ago

Dear maintainers,

I encountered a problem with the following OpenAPI pattern:

paths:
  '/composite':
    get:
      responses:
        '200':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Composite'
          links:
            testLink:
              operationId: anotherQuery
              parameters:
                someId: $request.path.id
components:
  schemas:
    Composite:
      oneOf:
        - $ref: '#/components/schemas/One'
        - $ref: '#/components/schemas/Two'
    Abstract:
      type: object
      properties:
        commonprop:
          type: object
    OneProps:
      type: object
      properties:
        differentoneprop:
          type: object
    TwoProps:
      type: object
      properties:
        differentoneprop:
          type: object
    One:
      allOf:
        - $ref: '#/components/schemas/Abstract'
        - $ref: '#/components/schemas/OneProps'
    Two:
      allOf:
        - $ref: '#/components/schemas/Abstract'
        - $ref: '#/components/schemas/TwoProps

There are currently two problems in the current implementation when using oneOf in combination with children using allOf:

This following PR fixes this for the API specifications I have in use. I had to move resolveAllOf to the oas tools file as I needed to resolve it when invoking GetOneOfTargetGraphQLType on each child node.

Please let me know if you have any questions. Thanks for your hard work!

mzronek commented 2 years ago

Something failed in the Link checker stage. Is there something missing from my side or anything I can do to help?


/opt/hostedtoolcache/Python/3.9.7/x64/bin/python /home/runner/work/_actions/peter-evans/create-issue-from-file/v2/dist/ciff/create_issue_from_file.py
Traceback (most recent call last):
  File "/home/runner/work/_actions/peter-evans/create-issue-from-file/v2/dist/ciff/create_issue_from_file.py", line 72, in <module>
    issue = repo.create_issue(title=issue_title, body=issue_content)
  File "/opt/hostedtoolcache/Python/3.9.7/x64/lib/python3.9/site-packages/github/Repository.py", line 1213, in create_issue
    headers, data = self._requester.requestJsonAndCheck(
  File "/opt/hostedtoolcache/Python/3.9.7/x64/lib/python3.9/site-packages/github/Requester.py", line 317, in requestJsonAndCheck
    return self.__check(
  File "/opt/hostedtoolcache/Python/3.9.7/x64/lib/python3.9/site-packages/github/Requester.py", line 342, in __check
    raise self.__createException(status, responseHeaders, output)
github.GithubException.GithubException: 403 {"message": "Resource not accessible by integration", "documentation_url": "https://docs.github.com/rest/reference/issues#create-an-issue"}
Error: The process '/opt/hostedtoolcache/Python/3.9.7/x64/bin/python' failed with exit code 1```
Alan-Cha commented 2 years ago

@mzronek This is awesome! Our oneOf and allOf resolution definitely can be improved so this is highly appreciated.

Unfortunately, I've been neglecting this project for a while now but I'm trying to get back into it. I will review this as soon as I can.

Regarding the link checker, please do not worry. I will take care of the problem.

Thanks again for this contribution! This is very helpful!

Alan-Cha commented 2 years ago

@mzronek Sorry for the very long wait... I did take a look initially but I was confused by a few things and then I got caught up in other work and now here we are... 😓

But, I've taken another look and I've gotten to fully appreciate everything you've done. This is a super cool change!!! I didn't think that oneOfs could have link objects that can be trickled down to its member types.

Alan-Cha commented 2 years ago

Anyway, thank you so much for these changes!!! I am very happy that we finally got these in 😄