Support inspection of schema mappings on startup

[nort3x]

i just started exploring this project, i read the documents and couldn't figure this out (so in that case i'm emphasizing on importance of configuration being much more straight forward and possibly default enabled )

for schema files we are supposed to write valid mappings by signature and return type
but if i make any mistake (not annotating Arguments, or methods) it remains silent until a request hit it

i was lucky i had defined not-null return type in schem otherwise i suppose it would remain completely silent

{
  "errors": [
    {
      "message": "The field at path '/uploadTestFile' was declared as a non null type, but the code involved in retrieving data has wrongly returned a null value.  The graphql specification requires that the parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on. The non-nullable type is 'File' within parent type 'Mutation'",
      "path": [
        "uploadTestFile"
      ],
      "extensions": {
        "classification": "NullValueInNonNullableField"
      }
    }
  ],
  "data": null
}

my point is this is not an issue at all!
but when i'm writing schema file i'm going with schema-first approach and it would be much more better experience if these mistakes would bubble up at startup rather than request resolving

[rstoyanchev]

I would expect this to be uncovered by tests, but I see your point, there is an opportunity to validate data fetcher registrations against schema fields.

As far as I can see currently GraphQL Java iterates fields and looks up DataFetcher registrations. That means a registration in RuntimeWiring could remain unused.

We could use a GraphQLTypeVisitor to go the other way, i.e. iterate DataFetcher registrations and cross check against field coordinates. Or is there a better way? It feels like it could be a GraphQL Java feature.

[danielshiplett]

I agree that this is an important feature that is currently lacking the in the Spring for GraphQL implementation. We are looking at switching to using Spring for GraphQL from the GraphQL Kickstart project. We rely heavily on the startup introspection to catch any failures to implement or map resolvers correctly.

Additionally, we use the com.kobylynskyi.graphql plugin to generate the API interfaces for our schema. This allows us to perform some amount of compile time checking of types against our implementation before we even get to the final startup introspection.

It is very important that our implementation 100% matches the GraphQL schema so that we don't respond to clients with GraphQL specific errors that they cannot do anything about (or are unlikely to understand).

[JamesPeters98]

Is there any update on this? I also agree that this is an important feature, especially during initial development it is very easy to miss a resolver and not notice it.

[rstoyanchev]

This is now scheduled for 1.2 M1. We'll see if there is a way to backport it to 1.1.x in the mean time such that it can be enabled if desired.

[nurkiewicz]

I confirm this is a major issue for me. I tried plain Java kickstart and it beautifully shows missing resolvers on startup, e.g.:

Caused by: [...]FieldResolverError: No method or field found as defined in schema [...] 
with any of the following signatures [...], in priority order:

  com.nurkiewicz.graphql.Player.trustworthiness()
  com.nurkiewicz.graphql.Player.getTrustworthiness()
  com.nurkiewicz.graphql.Player.trustworthiness

I'd assume Spring could inherit this feature. I don't want to write tests for things that can be detected on startup automatically.

[bclozel]

We should make this arrangement flexible enough to support sliced tests in Spring Boot.

Right now @GraphQlTest allows you to test a subset of controllers so that you don't have to start the entire application. By definition, there will be missing resolvers with sliced tests and we should not prevent this use case.

[rstoyanchev]

Slice tests should be okay. My understanding of this issue is that GraphQL Java iterates over schema types/fields and then crosschecks against RuntimeWiring registrations, so in effect, a controller method could remain unregistered silently. So it's fine to test one controller at a time, as long as all controller methods correspond to valid schema coordinates.

[rstoyanchev]

It appears my understanding is incorrect. @nort3x, or anyone else here, could you comment on your expectations for this. Do you expect us to very that:

A) Every controller method is mapped to a valid schema field
B) Every schema field is mapped to a controller method, or is otherwise covered by a corresponding object property?

I think B) could be a challenge to do in some cases. For example:

• if an application registers a DataFetcher directly with GraphQL Java, instead of declaring a controller method, we wouldn't know its return type, and wouldn't be able to verify if its properties and structure match the schema without any gaps.
• if a controller method does not specify the exact return type, we would have the same issue, and a controller method may not always be able to be precise about its return type, e.g. with interfaces and unions.