Add ability to customize graphql schema with OAS extensions

[elsmr]

Hi there 👋

First of all, thanks for the awesome library. We've been using it at @apideck-io with great success!

Recently we forked the repo because we needed granular control over the naming of certain schemas/operations.
It's not always possible to use title fields or similar, since we have other tooling using those as well. We needed a way to customise naming specifically for GraphQL.

We came up with a solution using OpenAPI extensions. We think it could be useful to other users of openapi-to-graphql so we're contributing it back 🎉

Let me know if you're interested in having something like this, otherwise we'll keep it in our fork 😄

Here's an API spec I used for testing: https://gist.github.com/elsmr/ac35f5dc7dbf7a42e60276f688955640. I also included the graphql schema output before & after.

There's also more features that could be added using this approach, for example a property whitelist for schemas etc.

Cheers

[Alan-Cha]

@elsmr Wow! This is an awesome feature! Thank you so much for sharing this and making this available for other users!

Producing consistent names in OtG has been a long-standing issue (#127, #131, #179, #303, #335) so I am glad that we finally have a solution. This change will allow OtG to be much more predictable and usable.

---

I like the direction that these changes are going but I have some additional concerns.

1. Naming of fields vs types and args and enum values.

Currently, this change seems to affect both fields and types but I wonder if they should be separated.

Consider this:

{
  "paths": {
    "/cat123": {
      "get": {
        "responses": {
          "200": {
            "content": {
              "application/json": {
                "schema": {
                  "allOf": [
                    {
                      "$ref": "#/components/schemas/pet"
                    },
                    {
                      "x-graphql-field-name": "cat"
                    }
                  ]
                }
              }
            }
          }
        }
      }
    },
    "/dog123": {
      "get": {
        "responses": {
          "200": {
            "content": {
              "application/json": {
                "schema": {
                  "allOf": [
                    {
                      "$ref": "#/components/schemas/pet"
                    },
                    {
                      "x-graphql-field-name": "dog"
                    }
                  ]
                }
              }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "pet": {
        "type": "object",
        "properties": {
          "petName": {
            "type": "string"
          },
          "favoriteFood": {
            "type": "string"
          }
        },
        "x-graphql-type-name": "dog"
      }
    }
  }
}

By using x-graphql-field-name and x-graphql-type-name extensions, this would ideally produce a dog and cat field, both of type Pet. What do you think about this proposal?

As mentioned previously, creating valid names for all GraphQL components, not just fields and types, has been a recurring problem for OtG. I think we should also consider what kind of precedent this change will have and how can we best design it so we can build off of it in the future. Therefore, we must also think about argument names and enum values.

If I recall correctly, argument names are handled in the same way fields are but I need to check.

Enum values are more complicated however. Perhaps what we need is a new extension like x-graphql-enum-mapping that is a map between existing enum values and proposed GraphQL enum values.

---

2. Naming of links

Creating nested objects via link objects is one of OtG's biggest features so we want to make sure that we can control these names as well.

I think there is a relatively straight forward solution if we check for an extension like x-graphql-field-name in a link object.

---

3. Naming prioritization, collisions, and error handling

The current behavior seems to use the preexisting name resolution in the case that there is a collision. I wonder if we should do more, like throw an error or even break. I would assume that users would expect that the field name would be changed as desired and if we just silently swallow it, it may lead to some unforeseen consequences.

---

4. External configuration

I wonder if we can utilize an external configuration to customize names. Similar to the selectQueryOrMutationField or customResolvers options, I was originally thinking we could do a triply nested object that allows users to map operations to field names however after some more consideration, this seems that limiting. Mapping schemas to type names and enum values will be cumbersome. I need to do some more thinking but an external configuration has its advantages; it's not always practical to manually add the extensions to an OAS and an external configuration can be reused.

[Alan-Cha]

We do not necessarily need to resolve all of these points in this PR. In the very minimum, I think we should consider doing x-graphql-field-name and x-graphql-type-name but I am curious if you have any opinions on the other points I brought up.

[elsmr]

Happy to hear you like it! Great remarks as well 😄

---

1. Naming of fields vs types and args and enum values.

Not sure I fully understand the example 😅. Is the following schema what you would expect?

type Query {
  cat: Pet
  dog: Pet
}

type Pet {
  favoriteFood: String
  petName: String
}

If yes, you can achieve that in this branch by adding extensions to the schema like I did in this gist.

Still, more specific naming such as x-graphql-type-name and x-graphql-field-name is probably a good idea.

I like your proposal for handling enums 👍

"direction": {
  "type": "string",
  "enum": ["asc", "desc"],
  "x-graphql-enum-mapping": {
    "asc": "ASCENDING",
    "desc": "DESCENDING"
  }
}

---

2. Naming of links

Haven't used links before, but just looked into it and that should be pretty straightforward to implement like you said 👍

---

3. Naming prioritization, collisions, and error handling

This is probably where this PR still needs a bit of work. I agree that if extension-provided names collide, openapi-to-graphql should throw. Currently it's using the existing logic, trying to avoid collisions by renaming.

---

4. External configuration

I don't love this. I feel like the config would be pretty complex because of the nesting. Having the gql names right in the spec is much easier to read because they are right next to the schema they're linked to.

Still if there is a use case for it, I don't see why not but could be separate from this PR 😄

[Alan-Cha]

Is the following schema what you would expect?

Yes, this is what I meant.

If yes, you can achieve that in this branch by adding extensions to the schema like I did in this gist.
Still, more specific naming such as x-graphql-type-name and x-graphql-field-name is probably a good idea.

Ah sorry, I did not realize that there was already this functionality. I think having a x-graphql-type-name and x-graphql-field-name would make things clearer.

I agree that if extension-provided names collide, openapi-to-graphql should throw.

Sounds like a plan.

I feel like the config would be pretty complex because of the nesting.

I agree so I was wondering if we could offer it in a limited fashion where we can only rename top level fields (operations) and types from component schemas. Maybe it's more effort than it's worth and may cause confusion between the external configuration and the extensions.

And agreed, it can always be added in a separate PR.

---

Are you interested in further working on this PR? If not, then I will take a crack at adding x-graphql-type-name/x-graphql-field-name extensions, link support, and also adding documentation.

[elsmr]

Feel free to have a crack at it. Also happy to help out if you want 😄

[elsmr]

@Alan-Cha if you don't have bandwidth to work on this one currently, lmk, I can pick it up 😄

[Alan-Cha]

@elsmr Thank you so much for offering! Yes, unfortunately, I haven't had any time to follow up on this. If you would like to give it a go, that would be highly appreciated.

[elsmr]

@Alan-Cha Hey there, been a while 😄 I finished up this PR

• Added errors for naming conflicts (when x-graphql- is used)
• Tests for errors and schema naming
• Links x-graphql-field-name

I think this is ready for review, I guess some docs would also be nice, happy to help 😄

[Alan-Cha]

@elsmr This is awesome! Thank you so much!!! I'll try to take a look at this today! Again, this is a fantastic feature!

[Alan-Cha]

I know I mentioned that I would take a look at this. I'm going to see if I can do this week. I'm very sorry about the delay.

[elsmr]

No worries 😉

[Gdewilde]

Hi @Alan-Cha! Any update on this?

[Alan-Cha]

@elsmr Hey! It's finally in! There were still quite a few things I wanted to change and I wasn't able to make any changes to your fork so I had to create my own off of yours. You can see the changes I made here: #399.

I added some docs and refactored a lot of code. Something to note is that I changed the way x-graphql-type-name affects input object types (parameters). Previous, OtG would use x-graphql-type-name for input object types and all other custom types. Currently, we append Input to the end of the name for input object types. This is important because otherwise, we may create types with the same name and that is illegal in GraphQL.

I'm sorry it took so long to review this. I was really struggling to find time to work on OtG with all the other work I had on my hands. Thank you so much for this incredible contribution! I was also very impressed with the test cases. They were very extensive!

[Alan-Cha]

@Gdewilde It's in now!

[Alan-Cha]

Closed: see #399

[Gdewilde]

Thanks, Alan!

…

Sent from my iPhone

On 28 May 2021, at 23:34, Alan Cha ***@***.***> wrote:  @Gdewilde It's in now! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[elsmr]

@Alan-Cha awesome! Thanks for finishing it 😄 happy to see it merged 👏