From 68f5e7856ce0a75d4b22b363d9c2e3e6ee8447ad Mon Sep 17 00:00:00 2001 From: Brian Hanson Date: Tue, 11 Oct 2022 12:40:57 -0500 Subject: [PATCH 1/5] Don't factor `$context` in to cache key Previously we were serializing `$context` into the cache key for graphql queries. While this was good for uniqueness it could make the caches _too_ unique. When the cache is first being generated, fields are collected and memoized which altered the `$context` variable. When two queries were run right after each other, the first cache would miss, generate the cache, alter the context and cause the second query to miss the cache as well. This solves that by ignoring the `$context` all together when generating the `$cacheKey`. The `$cacheKey` is already made up of a hash of the query, serialized variables and the operation name. The `$context` can't be altered without a plugin so it seems overkill to include within the `$cacheKey`. Worth noting that this could cause some confusing situations in development if a developer is changing the context via module or plugin but doesn't change the query. With this change and caching enabled they would get stale results. --- src/services/Gql.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/services/Gql.php b/src/services/Gql.php index 807c159c136..aab61c7e92f 100644 --- a/src/services/Gql.php +++ b/src/services/Gql.php @@ -489,7 +489,6 @@ public function executeQuery( $schema, $event->query, $event->rootValue, - $event->context, $event->variables, $event->operationName ); @@ -1234,7 +1233,6 @@ public function prepareFieldDefinitions(array $fields, string $typeName): array * @param GqlSchema $schema * @param string $query * @param mixed $rootValue - * @param mixed $context * @param array|null $variables * @param string|null $operationName * @return string|null @@ -1243,7 +1241,6 @@ private function _getCacheKey( GqlSchema $schema, string $query, mixed $rootValue, - mixed $context, ?array $variables = null, ?string $operationName = null, ): ?string { @@ -1270,7 +1267,6 @@ private function _getCacheKey( '::' . $schema->uid . '::' . md5($query) . '::' . serialize($rootValue) . - '::' . serialize($context) . '::' . serialize($variables) . ($operationName ? "::$operationName" : ''); } catch (Throwable $e) { @@ -1278,6 +1274,9 @@ private function _getCacheKey( $cacheKey = null; } + echo '
';
+        var_dump($cacheKey);
+        echo '
'; return $cacheKey; } From bf4be1b1fd78983aad7fc59723a3c146a71de6ca Mon Sep 17 00:00:00 2001 From: Brian Hanson Date: Tue, 11 Oct 2022 12:51:08 -0500 Subject: [PATCH 2/5] Whoops --- src/services/Gql.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/services/Gql.php b/src/services/Gql.php index aab61c7e92f..881ece511ed 100644 --- a/src/services/Gql.php +++ b/src/services/Gql.php @@ -1274,9 +1274,6 @@ private function _getCacheKey( $cacheKey = null; } - echo '
';
-        var_dump($cacheKey);
-        echo '
'; return $cacheKey; } From a2bd4f5031f218f26187f97c2c7b42ad87a8ba36 Mon Sep 17 00:00:00 2001 From: Brian Hanson Date: Wed, 12 Oct 2022 13:42:56 -0500 Subject: [PATCH 3/5] Add config version to cacheKey --- src/services/Gql.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/Gql.php b/src/services/Gql.php index 881ece511ed..de5ef05b535 100644 --- a/src/services/Gql.php +++ b/src/services/Gql.php @@ -1266,6 +1266,7 @@ private function _getCacheKey( '::' . Craft::$app->getSites()->getCurrentSite()->id . '::' . $schema->uid . '::' . md5($query) . + '::' . Craft::$app->getInfo()->configVersion . '::' . serialize($rootValue) . '::' . serialize($variables) . ($operationName ? "::$operationName" : ''); From 6b5aac451081dcf7e9a37eee3503d38782e5a1f7 Mon Sep 17 00:00:00 2001 From: Brian Hanson Date: Wed, 12 Oct 2022 13:43:31 -0500 Subject: [PATCH 4/5] Better diff --- src/services/Gql.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/Gql.php b/src/services/Gql.php index de5ef05b535..0ece9701be9 100644 --- a/src/services/Gql.php +++ b/src/services/Gql.php @@ -1266,8 +1266,8 @@ private function _getCacheKey( '::' . Craft::$app->getSites()->getCurrentSite()->id . '::' . $schema->uid . '::' . md5($query) . - '::' . Craft::$app->getInfo()->configVersion . '::' . serialize($rootValue) . + '::' . Craft::$app->getInfo()->configVersion . '::' . serialize($variables) . ($operationName ? "::$operationName" : ''); } catch (Throwable $e) { From 2353e68b7cf4ad9100c4a6b871972220fbc76a08 Mon Sep 17 00:00:00 2001 From: brandonkelly Date: Mon, 17 Oct 2022 17:37:36 -0700 Subject: [PATCH 5/5] Release note [ci skip] --- CHANGELOG-WIP.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG-WIP.md b/CHANGELOG-WIP.md index 75c62f3c20a..d607a004be6 100644 --- a/CHANGELOG-WIP.md +++ b/CHANGELOG-WIP.md @@ -110,6 +110,7 @@ - Element queries now support passing ambiguous column names (e.g. `dateCreated`) and field handles into `select()`. ([#11790](https://github.com/craftcms/cms/pull/11790), [#11800](https://github.com/craftcms/cms/pull/11800)) - `{% cache %}` tags now store any HTML registered with `{% html %}` tags. ([#11811](https://github.com/craftcms/cms/discussions/11811)) - `{% cache %}` tags and GraphQL query caches now get a max cache duration based on the fetched/referenced entries’ expiry dates. ([#8525](https://github.com/craftcms/cms/discussions/8525), [#11901](https://github.com/craftcms/cms/pull/11901)) +- Improved GraphQL cache reliability. ([#11994](https://github.com/craftcms/cms/issues/11994), [#12086](https://github.com/craftcms/cms/pull/12086)) - Control panel `.twig` templates are now prioritized over `.html`. ([#11809](https://github.com/craftcms/cms/discussions/11809), [#11840](https://github.com/craftcms/cms/pull/11840)) - `craft\elements\db\ElementQuery::collect()` and `craft\base\Element::getEagerLoadedElements()` now return `craft\elements\ElementCollection` instances. ([#12113](https://github.com/craftcms/cms/discussions/12113)) - `craft\events\DraftEvent::$creatorId` is now nullable. ([#11904](https://github.com/craftcms/cms/issues/11904))