Cache result of validation

[shmax]

Heyo, I remember there was some discussion about possibly disabling document validation checks a while back. I finally got around to looking into it and realized to my horror that it was indeed gobbling up a lot of resources in my app.

I don't know if you folks ever really agreed on a plan, but I thought I'd float this simple caching solution that leaves it up to the user.

Let me know what you think. If you like the general direction I can do a little polish and write better tests.

[spawnia]

I think we are good with the implementation in its current form. Can you add documentation to docs/executing-queries.md? I think a section after Custom Validation Rules would work fine.

[spawnia]

There are some tricky details to figure out when implementing this. I really value putting in the time now to properly document them now to validate the design. I am going to try implenting the validation cache in https://github.com/nuwave/lighthouse and use that to battle-test it in one of my projects to see if anything else comes up.

[spawnia]

Do you mean to expose this class to end users? The directory /tests and subsequently the namespace GraphQL\Tests is excluded from the composer package through .gitattributes. My impression was that it serves only as a vehicle for our own unit tests and perhaps as an example implementation that we can point to.

I would probably recommend users to implement the interface concretely, perhaps we can just add something like this?

Suggested change

	use GraphQL\GraphQL;
	use GraphQL\Tests\PsrValidationCacheAdapter;
	$validationCache = new PsrValidationCacheAdapter();
	use GraphQL\GraphQL;
	final class MyValidationCache implements ValidationCache { ... }
	$validationCache = new MyValidationCache();

[shmax]

Oh, yeah, this is just meant to be a sample. I'll touch that up.

[spawnia]

Good catch on not overwriting $rules 👍

[spawnia]

Due to /tests not being exported, this reference will lead nowhere for users that installed this package.

[shmax]

I figure devs will be savvy enough to either clone the repo or look on github, but I can remove it for now if you like.

[spawnia]

I just found that Lighthouse already implements validation caching, see nuwave/lighthouse#2603. In general, the approach to hashing to schema and hashing the query string is the same, but there are some differences:

1. Lighthouse offers special consideration for the QueryComplexity rule, which requires variables to work. Generally, validation rules do not have access to variable values, but this one has special treatment even in webonyx/graphql-php. The solution in Lighthouse splits validation in two phases, one where the cacheable rules (everything except QueryComplexity) are run/cached, then QueryComplexity maybe runs afterwards.
2. Lighthouse has no considerations for the used library version of webonyx/graphql-php or nuwave/lighthouse itself, but probably should consider that.
3. Lighthouse does not allow dynamically passing $rules ad-hoc, but users may overwrite the interface ProvidesCacheableValidationRules and change which validation rules are used. However, I found that using just the keys or class names of the used validation rules array is not sufficient. The validation rules for security such as QueryDepth are configurable and behave differently based on the configuration.

To resolve 1., perhaps we should add multi-phase validation to this library or some kind of special treatment for QueryComplexity.
Points 2. and 3. seem like something where Lighthouse is currently lacking but could be improved.

I am going to try adding a pull request to Lighthouse soon that implements its validation cache feature using the facilities offered through this pull request and perhaps extend it where its lacking.

[shmax]

Not sure I follow. I don't use Lighthouse, and I'm not familiar with it. What does it have to do with what I'm doing in this PR?

[spawnia]

Not sure I follow. I don't use Lighthouse, and I'm not familiar with it. What does it have to do with what I'm doing in this PR?

Lighthouse is built atop this library. It already implements a validation cache that has proven to be effective. I would like to take the lessons from the implementation effort we made there to inform the implementation in this library. This is to ensure that I can replace the custom implementation in Lighthouse with what we provide here, and we are not missing anything.

[shmax]

I see. Lighthouse implements its own high-level query flow, and uses DocumentValidator:validate directly. In the meantime, I modified my own code to just disable validation on production (I use persisted queries), so I'm not blocked.

I'll leave you to it.