graphql-java icon indicating copy to clipboard operation
graphql-java copied to clipboard

Schema introspection in JSON can return directive definition many times

Open paullarionov opened this issue 1 year ago • 6 comments

Describe the bug

Version com.graphql-java:graphql-java:21.3

I faced that JSON schema introspection sometimes returns the same directive many times. This breaks code generation in API clients and many tools like Altair.

It happens both if the directive definition is single in sources, and when there are copies of it.

{
  "data": {
    "__schema": {
      "queryType": {
        "name": "Query"
      },
      "mutationType": null,
      "subscriptionType": null,
      "types": [],
      "directives": [
        {
          "name": "exampleDirective",
          "description": null,
          "locations": [
            "FIELD_DEFINITION"
          ],
          "args": []
        },
        {
          "name": "exampleDirective",
          "description": null,
          "locations": [
            "FIELD_DEFINITION"
          ],
          "args": []
        }
      ]
    }
  }
}

In my production project I have 1 directive, but get 2 copies in JSON introspection.

  • What can be the reason for this?
  • Did you see similar situations?
  • Suppose that it is incorrect behaviour

To Reproduce

  1. Example code: https://github.com/paullarionov/test-graphql-java
  2. Fetch of JSON schema:
./gradlew downloadApolloSchema  --endpoint="http://localhost:8080/graphql"  --schema="schema.json"
  1. Example of broken schema: https://github.com/paullarionov/test-graphql-java/blob/main/schema.json#L1022-L1037

paullarionov avatar Feb 16 '24 16:02 paullarionov

I will look into this moore but at a quick glance this seems wrong

directive @exampleDirective on FIELD_DEFINITION
directive @exampleDirective on FIELD_DEFINITION

type Query {
    hello: String @exampleDirective
}

At schema definition time we should not allow the same directive to be defined twice. I am pretty sure thats illegal and should have been caught at schema creation time. The introspection problem is a follow on problem from that

bbakerman avatar Feb 17 '24 01:02 bbakerman

Thank you for the answer @bbakerman. This example is just a demo that this output is possible. In my production project I have 1 directive, but get 2 copies in JSON introspection for some unknown reason.

So I propose:

  1. Eliminate any copies in introspection output
  2. Check schema at schema creation time

Have a nice day!

paullarionov avatar Feb 17 '24 07:02 paullarionov

Hello, at schema creation time, GraphQL Java does check for directive redefinitions. The schema will not be built if a directive redefinition is detected, see test in this PR https://github.com/graphql-java/graphql-java/pull/3454/files

Here is the directive redefinition error in the source: https://github.com/graphql-java/graphql-java/blob/e0837b6e6b69c305b3fe56affb9d44da911ba31a/src/main/java/graphql/schema/idl/errors/DirectiveRedefinitionError.java#L11

I'm not familiar with the downloadApolloSchema task. It would be worth checking if this Gradle task is directly downloading your schema file without running the validation checks that GraphQL Java does on schema creation.

I'm also not familiar with the graphql-java-kickstart project you have in your sample, but when I tried the same schema with Spring for GraphQL, the application does not boot because the schema is invalid. You might like to investigate how to add a similar check to your app. Sample repo: https://github.com/dondonz/directiveRedefinitionSample.

dondonz avatar Feb 18 '24 11:02 dondonz

I had a look into this more - as Donna said there is code in graphql-java that will prevent multiple directives being defined with the same name

But this runs if the schema is created via the SchemaParser / SchemaGenerator doorway.

if the schema was programmatically created say, then this does not run.

I can see an argument that it should run on schema creation (via graphql.schema.validation.SchemaValidator say) to catch other paths

I am not sure how graphql-kickstart creates schema from a SDL file. It must not be using the SchemaParser / SchemaGenerator doorway.

bbakerman avatar Feb 20 '24 22:02 bbakerman

I'm not familiar with the downloadApolloSchema task. It would be worth checking if this Gradle task is directly downloading your schema file without running the validation checks that GraphQL Java does on schema creation.

This is reproduced not only with downloadApolloSchema in Gradle, but also Altair plugin in Google Chrome and iOS Apollo client, so the issue is not in download.

Agree with this:

I can see an argument that it should run on schema creation (via graphql.schema.validation.SchemaValidator say) to catch other paths

paullarionov avatar Feb 27 '24 11:02 paullarionov

Hello, this issue has been inactive for 60 days, so we're marking it as stale. If you would like to continue this discussion, please comment within the next 30 days or we'll close the issue.

github-actions[bot] avatar May 05 '24 00:05 github-actions[bot]