drupal-graphql / graphql

GraphQL integration for Drupal 9/10
287 stars 202 forks source link

Field resolver validation reports a false positive for array and object types #1341

Open Kingdutch opened 1 year ago

Kingdutch commented 1 year ago

Problem

Given the following schema

schema {
  query: Query
}

type Query {
  someField: TypedField
}

type TypedField {
  one: String!
  two: String!
}

Where the schema is mapped using

    $registry->addFieldResolver('Query', 'someField'
      $builder->fromValue(['one' => 1, 'two' => 2])
    );

The TypedField will automatically be resolved by the GraphQL library's Executor::defaultFieldResolver. However, the validator is not aware of this and will report missing field resolvers for the sub-types, even though the schema will work just fine.

This can lead to bug reports which may put people on the wrong trail such as https://www.drupal.org/project/graphql_address/issues/3354352

Proposed resolution

This is a tricky problem because the schema can not be validated at inspection time, but really only by inspecting the resolved value. The current mapping validation mostly checks "if there's a field resolver" and doesn't capture any information about the resolvers, so we also can't utilise something like reflection to make this process smarter and look at the @return type of Resolver::resolve method (this would require also inspecting the output of the ResolverBuilder's chain).

This would be a good candidate for the providing module to mark the type TypedField as an ignored type. The Validator already has an $ignored_types optional argument which would need to be provided in the ValidationController.

The downside is that if someone uses a different resolver for the someField then it may no longer return a type which actually has the one and two fields. However this will not be caught by the validator because it's ignored.