Skip to content

Conversation

@shmax
Copy link
Contributor

@shmax shmax commented Jul 11, 2025

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 shmax changed the title cache result of validation Cache result of validation Jul 11, 2025
@shmax shmax force-pushed the validation-cache branch from 2d38364 to a2c38f4 Compare July 11, 2025 03:55
@shmax shmax force-pushed the validation-cache branch from 9ffbbd7 to 6782bad Compare July 11, 2025 04:07
@shmax shmax force-pushed the validation-cache branch from e3bc588 to 6317931 Compare July 12, 2025 00:05
@shmax shmax force-pushed the validation-cache branch from 1a59209 to 73c9774 Compare July 12, 2025 00:40
@shmax shmax force-pushed the validation-cache branch from 5e59b9c to 6d7ace0 Compare July 12, 2025 00:41
@shmax shmax force-pushed the validation-cache branch from 59f0ea0 to 708c85b Compare July 12, 2025 00:45
@shmax shmax force-pushed the validation-cache branch from a12854e to f35cb3c Compare July 12, 2025 01:53
@shmax shmax force-pushed the validation-cache branch from e6fa9a4 to d71e123 Compare July 12, 2025 04:00
@shmax shmax force-pushed the validation-cache branch from d1f3597 to c2a82a4 Compare July 14, 2025 14:26
@shmax shmax marked this pull request as ready for review July 14, 2025 17:32
Copy link
Collaborator

@spawnia spawnia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@shmax shmax force-pushed the validation-cache branch from 8fc2b53 to b5fdd3e Compare July 23, 2025 03:16
Copy link
Collaborator

@spawnia spawnia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Collaborator

spawnia commented Jul 23, 2025

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
Copy link
Contributor Author

shmax commented Jul 24, 2025

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
Copy link
Collaborator

spawnia commented Jul 25, 2025

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
Copy link
Contributor Author

shmax commented Jul 25, 2025

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.

spawnia added a commit to nuwave/lighthouse that referenced this pull request Dec 29, 2025
Implement the ValidationCache interface from webonyx/graphql-php#1730
to improve validation result caching with automatic cache invalidation.

Cache key now includes:
- Library versions (webonyx/graphql-php and nuwave/lighthouse)
- Schema hash
- Query hash
- Rule configuration hash (max_query_depth, disable_introspection)

This eliminates the need for manual cache clearing when upgrading
graphql-php or lighthouse, as the cache auto-invalidates on version
changes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
spawnia added a commit to nuwave/lighthouse that referenced this pull request Dec 29, 2025
Temporarily depend on the validation-cache branch from webonyx/graphql-php
to test the ValidationCache interface integration.

This will be updated to a proper version constraint once
webonyx/graphql-php#1730 is merged and released.

Note: This will require a major version bump for Lighthouse.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@shmax
Copy link
Contributor Author

shmax commented Dec 29, 2025

I see you're working on stuff around this, which is great. Holler if I can be of any help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants