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

OpenAPi handler creates types with the same properties as separate types #1414

Closed
chompy18 opened this issue Jan 13, 2021 · 6 comments
Closed
Assignees

Comments

@chompy18
Copy link

chompy18 commented Jan 13, 2021

Given a swagger definition, the OpenApi handler creates multiple types that contain the same properties.
Out of those types, they DO use the same 3rd type internally.
This makes using Fragments impossible, since fragments require to be defined on a specific type.

type Query {
  # get translations
  #
  # Equivalent to GET /translations/{translation_id}
  getTranslationById(translationId: String): Translation

  # get translations list
  #
  # Equivalent to GET /translations/list
  getTranslationsList(
    # Auto-generated argument that limits the size of returned list of objects/list, selecting the first `n` elements of the list
    limit: Int
  ): [TranslationsListListItem]
}

type Translation {
  displayName: DisplayName
  id: String
  key: String
  platform: [String]
  title: String
}

type DisplayName {
  en: String
  he: String
}

type TranslationsListListItem {
  displayName: DisplayName
  id: String
  key: String
  platform: [String]
  title: String
}

Note, that both TranslationsListListItem and Translation have exactly the same structure, but are separate types.
Moreover, they both use the same DisplayName type internally.

Talking with @ardatan , he suggested opening an issue because that's an inconsistency in the handler

@ardatan
Copy link
Owner

ardatan commented Jan 13, 2021

Thanks for opening this issue!
Could you share your swagger file? At least the part has those types?

@chompy18
Copy link
Author

Here are the relevant paths

    "/translations/{translation_id}": {
      "get": {
        "tags": [
          "translations"
        ],
        "summary": "get translations",
        "operationId": "get|translations",
        "parameters": [
          {
            "name": "translation_id",
            "in": "path",
            "schema": {
              "type": "string"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "OK",
            "content": {
              "application/json": {
                "schema": {
                  "properties": {
                    "display_name": {
                      "type": "object",
                      "properties": {
                        "en": {
                          "type": "string"
                        },
                        "he": {
                          "type": "string"
                        }
                      }
                    },
                    "platform": {
                      "type": "array",
                      "items": {
                        "type": "string"
                      }
                    },
                    "_id": {
                      "type": "string"
                    },
                    "title": {
                      "type": "string"
                    },
                    "key": {
                      "type": "string"
                    }
                  }
                }
              }
            }
          }
        }
      },
      "delete": {
        "tags": [
          "translations"
        ],
        "summary": "delete translations",
        "operationId": "delete|translations",
        "parameters": [
          {
            "name": "translation_id",
            "in": "path",
            "schema": {
              "type": "string"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "OK",
            "content": {
              "application/json": {
                "schema": {
                  "properties": {
                    "message": {
                      "type": "string"
                    }
                  }
                }
              }
            }
          }
        }
      }
    },
    "/translations/list": {
      "get": {
        "tags": [
          "translations",
          "list"
        ],
        "summary": "get translations list",
        "operationId": "get|translations|list",
        "responses": {
          "200": {
            "description": "OK",
            "content": {
              "application/json": {
                "schema": {
                  "type": "array",
                  "items": {
                    "type": "object",
                    "properties": {
                      "display_name": {
                        "type": "object",
                        "properties": {
                          "en": {
                            "type": "string"
                          },
                          "he": {
                            "type": "string"
                          }
                        }
                      },
                      "platform": {
                        "type": "array",
                        "items": {
                          "type": "string"
                        }
                      },
                      "_id": {
                        "type": "string"
                      },
                      "title": {
                        "type": "string"
                      },
                      "key": {
                        "type": "string"
                      }
                    }
                  }
                }
              }
            }
          }
        }
      }
    },
    "/translations/": {
      "post": {
        "tags": [
          "translations",
          ""
        ],
        "summary": "post translations ",
        "operationId": "post|translations|",
        "requestBody": {
          "content": {
            "application/json": {
              "schema": {
                "properties": {
                  "data": {
                    "type": "object",
                    "properties": {
                      "_id": {
                        "type": "string"
                      },
                      "title": {
                        "type": "string"
                      },
                      "key": {
                        "type": "string"
                      },
                      "display_name": {
                        "type": "object",
                        "properties": {
                          "en": {
                            "type": "string"
                          },
                          "he": {
                            "type": "string"
                          }
                        }
                      },
                      "platform": {
                        "type": "array",
                        "items": {
                          "type": "string"
                        }
                      },
                      "updated": {
                        "type": "string"
                      }
                    }
                  }
                }
              }
            }
          }
        },
        "responses": {
          "200": {
            "description": "OK",
            "content": {
              "application/json": {
                "schema": {
                  "properties": {
                    "display_name": {
                      "type": "object",
                      "properties": {
                        "en": {
                          "type": "string"
                        },
                        "he": {
                          "type": "string"
                        }
                      }
                    },
                    "platform": {
                      "type": "array",
                      "items": {
                        "type": "string"
                      }
                    },
                    "_id": {
                      "type": "string"
                    },
                    "title": {
                      "type": "string"
                    },
                    "key": {
                      "type": "string"
                    }
                  }
                }
              }
            }
          }
        },
        "parameters": {}
      }
    }

@ardatan
Copy link
Owner

ardatan commented Jan 13, 2021

Thanks! @chompy18 We'll take a look and get back to you soon!

@chompy18
Copy link
Author

Cool, thanks.
But I think that regardless, #1415 can easily solve the fragments issue

@chompy18
Copy link
Author

chompy18 commented Jan 14, 2021

Another note that might be important here - when using $ref instead of in-line, mesh identifies the types and reuses them as expected, which allows for fragments to be used.

@ardatan
Copy link
Owner

ardatan commented Aug 3, 2022

We are currently rewriting the OpenAPI handler and releasing it as newOpenapi
https://github.com/Urigo/graphql-mesh/blob/master/packages/handlers/new-openapi/yaml-config.graphql

In this new handler, we resolve entire schema by dereferencing $refs then recreate definitions with $ref so this helps us to deduplicate the types that have the same properties.. I'd recommend you to check it :)

So we can close this issue as it is covered by the handler.

@ardatan ardatan closed this as completed Aug 3, 2022
@ardatan ardatan self-assigned this Aug 3, 2022
@Urigo Urigo mentioned this issue Aug 11, 2022
55 tasks
klippx pushed a commit to klippx/graphql-mesh that referenced this issue Oct 9, 2024
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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

No branches or pull requests

2 participants