Cache query AST as an array in store mode - #2785
Open
dxiiren wants to merge 1 commit into
Open
Conversation
Cache stores serialize their values, and Laravel 13 lets applications limit which classes unserialize() will accept via cache.serializable_classes. Storing the DocumentNode itself meant a restricted list turned the cached AST back into __PHP_Incomplete_Class, so fromStoreOrParse() failed its return type. Store the array form instead, mirroring what the opcache mode already does, and reparse whatever else is found under the key so existing caches recover on their own.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #2782
Changes
query_cache.mode: storehanded theDocumentNodestraight to the cache store:Cache stores serialize what they are given. Laravel 13 lets an application limit which
classes come back out again through
cache.serializable_classes, which every serializingstore reads before unserializing:
Once that list is set and does not name every AST node class, the cached query comes back
as
__PHP_Incomplete_ClassandfromStoreOrParse()fails its return type on the secondrequest. Whitelisting the AST is not a practical workaround, since it would mean naming
every node class in
webonyx/graphql-phpand keeping that list current.So this caches the array form instead of the object, which is what
mode: opcachealreadydoes via
opcacheFileContents()andrequireOPcacheFile(). Arrays are unaffected byallowed_classes, so the AST survives regardless of how a store is configured.The
remember()call is replaced with the explicitget()/put()pair thatfromHybridOrParse()already uses, so anything found under the key that is not an arrayis simply reparsed and overwritten. Caches populated by an older version therefore recover
by themselves on the next request, with no flush needed on upgrade.
Verified on PHP 8.4.20 / Laravel 13.23.0 / webonyx/graphql-php 15.37.1. The added test
fails on current
masterwith exactly the reported error and passes with this change:It reproduces through the
arraystore withserializeenabled, so it needs no Redis ordatabase. The rest of
QueryCacheTestis unchanged by this commit.Breaking changes
None. The cache key is unchanged, entries written by older versions are detected and
reparsed rather than failing, and the public API is untouched.