[Question] Why is the validation added via directive wiring and not instrumentation?

[quinlam]

This is a question that I think would be helpful in the FAQ, as while I am sure there are good reasons, its not immediately obvious why (at least to me and those I have spoken with), and I think informing users will help them navigate the usage of the graphql-java library itself with more intent.

For example the Instrumentation interface provides a beginExecuteOperation method and has a FieldValidationInstrumentation implementation allowing the validation of fields via implementation of a FieldValidation

The FieldValidation provides a list of fields/arguments by path that can than be individually validated. I suspect the reason it may not be useful is because of these lines

only fields that have arguments make any sense to placed in play since only they have variable input

Which at face value makes sense to me, but maybe I am missing something here and that is where the nuance lies? If so would love an explanation, and if not then I would really value from understanding more. I would ask that the answer is added to the end of the README. Happy to contribute in this regard when I understand more.

[bbakerman]

You right that this could have been done via Instrumentation but not via graphql.execution.instrumentation.fieldvalidation.FieldValidation say

graphql.execution.instrumentation.fieldvalidation.FieldValidation is a weak validation mechanism. It does NOT run when all the field values have been coerced into java values and all field instances have been merged. graphql.execution.instrumentation.fieldvalidation.FieldValidation was an early attempt at validation (circa 2017) but its not very powerful. graphql.execution.instrumentation.fieldvalidation.FieldValidation allows you to ABORT the operation as a whole which in retrospect is no a great lever - its all or nothing.

Validating inside a DataFetcher is more powerful. All variables have been coerced, all merged fields have been done and there is a mechanism for adding errors to a response for a field and only that field.

We need to "wrap" the DataFetcher to allow this validation to happen. We could have done this via two mechanisms.

Either graphql.schema.idl.SchemaDirectiveWiring which is done once at schema build time (and hence the DataFetcher is only wrapped once),

Or via graphql.execution.instrumentation.Instrumentation#instrumentDataFetcher which is done on EVERY request. This is way less efficient since its wrapping with allocated code every request.

Since all the "mechanisms" that control how to do the validation are directives it makes sense to use graphql.schema.idl.SchemaDirectiveWiring to detect directives and "influence via wrapping" how DataFetchers run.

I am going to close this issue as done in the sense there is no bug but rather a design question.