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

Cache result of validation

Open shmax opened this issue 5 months ago • 4 comments

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.

shmax avatar Jul 11 '25 03:07 shmax

I just found that Lighthouse already implements validation caching, see https://github.com/nuwave/lighthouse/pull/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.

spawnia avatar Jul 23 '25 10:07 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?

shmax avatar Jul 24 '25 03:07 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?

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.

spawnia avatar Jul 25 '25 07:07 spawnia

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.

shmax avatar Jul 25 '25 15:07 shmax