From edbcbe4965ebbf6383f930aaec13b4b09ca2e8af Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Mon, 11 Nov 2024 19:19:34 +0100 Subject: [PATCH 01/24] Add `reset` method to Type, Directive and Introspection See #1424 This library uses a static cache for standard types and the internal directives. While not ideal, this works fine. As long as you don't have a scenario where you want to change them. In my situation, we have 2 schema's with a custom ID type. They are not the same instance. This still works fine, but as soon as we run our whole test suite at once, the static cache is initialized in some test, and another test later expects something else. While I think the better solution would be to remove these static caches completely, and store them on an instance instead of statically. But that will be a much bigger task given the current architecture. With these 2 reset methods, we can manually reset the caches in our test suite. It could also be used for people that run their GraphQL server in tools like RoadRunner and FrankenPHP. --- src/Type/Definition/Directive.php | 13 +++++++++++-- src/Type/Definition/Type.php | 7 ++++++- src/Type/Introspection.php | 5 +++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Type/Definition/Directive.php b/src/Type/Definition/Directive.php index 0a6db42e5..f2c546f93 100644 --- a/src/Type/Definition/Directive.php +++ b/src/Type/Definition/Directive.php @@ -33,7 +33,7 @@ class Directive * * @var array */ - protected static array $internalDirectives; + protected static array $internalDirectives = []; public string $name; @@ -90,7 +90,11 @@ public static function includeDirective(): Directive */ public static function getInternalDirectives(): array { - return self::$internalDirectives ??= [ + if (self::$internalDirectives !== []) { + return self::$internalDirectives; + } + + return self::$internalDirectives = [ 'include' => new self([ 'name' => self::INCLUDE_NAME, 'description' => 'Directs the executor to include this field or fragment only when the `if` argument is true.', @@ -162,4 +166,9 @@ public static function isSpecifiedDirective(Directive $directive): bool { return \array_key_exists($directive->name, self::getInternalDirectives()); } + + public static function reset(): void + { + self::$internalDirectives = []; + } } diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index cd734b99c..e757e7d32 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -31,7 +31,7 @@ abstract class Type implements \JsonSerializable ]; /** @var array */ - protected static array $standardTypes; + protected static array $standardTypes = []; /** * @api @@ -261,4 +261,9 @@ public function jsonSerialize(): string { return $this->toString(); } + + public static function reset(): void + { + static::$standardTypes = []; + } } diff --git a/src/Type/Introspection.php b/src/Type/Introspection.php index 9aaa5341a..9d29dde3d 100644 --- a/src/Type/Introspection.php +++ b/src/Type/Introspection.php @@ -795,4 +795,9 @@ public static function typeNameMetaFieldDef(): FieldDefinition 'resolve' => static fn ($source, array $args, $context, ResolveInfo $info): string => $info->parentType->name, ]); } + + public static function reset(): void + { + self::$map = []; + } } From 7229cf7aec4b84de02c5d0356ac99af511f021f9 Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Mon, 11 Nov 2024 14:02:01 +0100 Subject: [PATCH 02/24] Store built-in types in static property This way, it behaves the same as the standard types. --- src/Type/Definition/Type.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index e757e7d32..6a2a29c52 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -33,6 +33,9 @@ abstract class Type implements \JsonSerializable /** @var array */ protected static array $standardTypes = []; + /** @var array */ + protected static array $builtInTypes = []; + /** * @api * @@ -116,9 +119,11 @@ public static function nonNull($type): NonNull */ public static function builtInTypes(): array { - static $builtInTypes; + if (self::$builtInTypes !== []) { + return self::$builtInTypes; + } - return $builtInTypes ??= \array_merge( + return self::$builtInTypes = \array_merge( Introspection::getTypes(), self::getStandardTypes() ); @@ -265,5 +270,6 @@ public function jsonSerialize(): string public static function reset(): void { static::$standardTypes = []; + static::$builtInTypes = []; } } From a38a5a3c983a2bdba9234b4256ac46f5fdfa392b Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Mon, 11 Nov 2024 19:12:39 +0100 Subject: [PATCH 03/24] Store introspection field definitions as property Since the ReferenceExecutor is an instance, we can keep this cache on the instance. Great! --- src/Executor/ReferenceExecutor.php | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/Executor/ReferenceExecutor.php b/src/Executor/ReferenceExecutor.php index 303287080..224d50aef 100644 --- a/src/Executor/ReferenceExecutor.php +++ b/src/Executor/ReferenceExecutor.php @@ -69,6 +69,12 @@ class ReferenceExecutor implements ExecutorImplementation */ protected \SplObjectStorage $fieldArgsCache; + protected FieldDefinition $schemaMetaFieldDef; + + protected FieldDefinition $typeMetaFieldDef; + + protected FieldDefinition $typeNameMetaFieldDef; + protected function __construct(ExecutionContext $context) { if (! isset(static::$UNDEFINED)) { @@ -701,23 +707,22 @@ protected function resolveField( */ protected function getFieldDef(Schema $schema, ObjectType $parentType, string $fieldName): ?FieldDefinition { - static $schemaMetaFieldDef, $typeMetaFieldDef, $typeNameMetaFieldDef; - $schemaMetaFieldDef ??= Introspection::schemaMetaFieldDef(); - $typeMetaFieldDef ??= Introspection::typeMetaFieldDef(); - $typeNameMetaFieldDef ??= Introspection::typeNameMetaFieldDef(); + $this->schemaMetaFieldDef ??= Introspection::schemaMetaFieldDef(); + $this->typeMetaFieldDef ??= Introspection::typeMetaFieldDef(); + $this->typeNameMetaFieldDef ??= Introspection::typeNameMetaFieldDef(); $queryType = $schema->getQueryType(); - if ($fieldName === $schemaMetaFieldDef->name && $queryType === $parentType) { - return $schemaMetaFieldDef; + if ($fieldName === $this->schemaMetaFieldDef->name && $queryType === $parentType) { + return $this->schemaMetaFieldDef; } - if ($fieldName === $typeMetaFieldDef->name && $queryType === $parentType) { - return $typeMetaFieldDef; + if ($fieldName === $this->typeMetaFieldDef->name && $queryType === $parentType) { + return $this->typeMetaFieldDef; } - if ($fieldName === $typeNameMetaFieldDef->name) { - return $typeNameMetaFieldDef; + if ($fieldName === $this->typeNameMetaFieldDef->name) { + return $this->typeNameMetaFieldDef; } return $parentType->findField($fieldName); From 2c4c7b860c6b1ead31129e233ae129547489941e Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Tue, 12 Nov 2024 08:35:54 +0100 Subject: [PATCH 04/24] Add test This is done in a single test, because if you reset the Type without resetting the rest it will result in different standard types. --- tests/ResetTest.php | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tests/ResetTest.php diff --git a/tests/ResetTest.php b/tests/ResetTest.php new file mode 100644 index 000000000..c2629df10 --- /dev/null +++ b/tests/ResetTest.php @@ -0,0 +1,26 @@ + Date: Tue, 12 Nov 2024 11:19:10 +0100 Subject: [PATCH 05/24] Apply feedback --- src/Type/Definition/Directive.php | 12 ++++-------- src/Type/Definition/Type.php | 14 +++++--------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/Type/Definition/Directive.php b/src/Type/Definition/Directive.php index f2c546f93..4be4fcac3 100644 --- a/src/Type/Definition/Directive.php +++ b/src/Type/Definition/Directive.php @@ -31,9 +31,9 @@ class Directive /** * Lazily initialized. * - * @var array + * @var array|null */ - protected static array $internalDirectives = []; + protected static ?array $internalDirectives = null; public string $name; @@ -90,11 +90,7 @@ public static function includeDirective(): Directive */ public static function getInternalDirectives(): array { - if (self::$internalDirectives !== []) { - return self::$internalDirectives; - } - - return self::$internalDirectives = [ + return self::$internalDirectives ??= [ 'include' => new self([ 'name' => self::INCLUDE_NAME, 'description' => 'Directs the executor to include this field or fragment only when the `if` argument is true.', @@ -169,6 +165,6 @@ public static function isSpecifiedDirective(Directive $directive): bool public static function reset(): void { - self::$internalDirectives = []; + self::$internalDirectives = null; } } diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 6a2a29c52..92473d404 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -31,10 +31,10 @@ abstract class Type implements \JsonSerializable ]; /** @var array */ - protected static array $standardTypes = []; + protected static ?array $standardTypes = []; - /** @var array */ - protected static array $builtInTypes = []; + /** @var array|null */ + protected static ?array $builtInTypes = null; /** * @api @@ -119,11 +119,7 @@ public static function nonNull($type): NonNull */ public static function builtInTypes(): array { - if (self::$builtInTypes !== []) { - return self::$builtInTypes; - } - - return self::$builtInTypes = \array_merge( + return self::$builtInTypes ??= \array_merge( Introspection::getTypes(), self::getStandardTypes() ); @@ -270,6 +266,6 @@ public function jsonSerialize(): string public static function reset(): void { static::$standardTypes = []; - static::$builtInTypes = []; + static::$builtInTypes = null; } } From ca6712ec40cb5b83a3187d7ce92cf84988509864 Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Tue, 12 Nov 2024 18:16:30 +0100 Subject: [PATCH 06/24] Update src/Type/Definition/Type.php Co-authored-by: Benedikt Franke --- src/Type/Definition/Type.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 92473d404..0b74863bf 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -34,7 +34,7 @@ abstract class Type implements \JsonSerializable protected static ?array $standardTypes = []; /** @var array|null */ - protected static ?array $builtInTypes = null; + protected static ?array $builtInTypes; /** * @api From 67d538e9230081d0ebd428153f84c0892499e5fd Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Tue, 12 Nov 2024 18:16:35 +0100 Subject: [PATCH 07/24] Update src/Type/Definition/Type.php Co-authored-by: Benedikt Franke --- src/Type/Definition/Type.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 0b74863bf..2a4513b8b 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -30,7 +30,7 @@ abstract class Type implements \JsonSerializable ...Introspection::TYPE_NAMES, ]; - /** @var array */ + /** @var array|null */ protected static ?array $standardTypes = []; /** @var array|null */ From 2a27348ddf993a520e315ed0934c0085609a1fcb Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Tue, 12 Nov 2024 18:16:44 +0100 Subject: [PATCH 08/24] Update src/Type/Definition/Type.php Co-authored-by: Benedikt Franke --- src/Type/Definition/Type.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 2a4513b8b..1a1143333 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -265,7 +265,7 @@ public function jsonSerialize(): string public static function reset(): void { - static::$standardTypes = []; + static::$standardTypes = null; static::$builtInTypes = null; } } From abdf5fd1a1502847462b491833c83ef0a3af0d13 Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Tue, 12 Nov 2024 18:16:50 +0100 Subject: [PATCH 09/24] Update src/Type/Definition/Type.php Co-authored-by: Benedikt Franke --- src/Type/Definition/Type.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 1a1143333..eeb77c116 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -31,7 +31,7 @@ abstract class Type implements \JsonSerializable ]; /** @var array|null */ - protected static ?array $standardTypes = []; + protected static ?array $standardTypes; /** @var array|null */ protected static ?array $builtInTypes; From 5e23fdc757fe1e4a05b581442a63087b5cfe4f62 Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Tue, 12 Nov 2024 19:11:27 +0100 Subject: [PATCH 10/24] Automatically reset when overriding standard types --- src/Type/Definition/Type.php | 7 +++++ .../Definition/TypeTest.php} | 6 ++-- tests/Type/StandardTypesTest.php | 30 ++++++++++++------- 3 files changed, 28 insertions(+), 15 deletions(-) rename tests/{ResetTest.php => Type/Definition/TypeTest.php} (82%) diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index eeb77c116..35ea64c68 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -150,6 +150,10 @@ public static function getStandardTypes(): array */ public static function overrideStandardTypes(array $types): void { + $standardTypes = static::$standardTypes; + self::reset(); + static::$standardTypes = $standardTypes; + foreach ($types as $type) { // @phpstan-ignore-next-line generic type is not enforced by PHP if (! $type instanceof ScalarType) { @@ -267,5 +271,8 @@ public static function reset(): void { static::$standardTypes = null; static::$builtInTypes = null; + + Introspection::reset(); + Directive::reset(); } } diff --git a/tests/ResetTest.php b/tests/Type/Definition/TypeTest.php similarity index 82% rename from tests/ResetTest.php rename to tests/Type/Definition/TypeTest.php index c2629df10..bac5ff0d3 100644 --- a/tests/ResetTest.php +++ b/tests/Type/Definition/TypeTest.php @@ -1,13 +1,13 @@ */ - private static array $originalStandardTypes; - - public static function setUpBeforeClass(): void - { - self::$originalStandardTypes = Type::getStandardTypes(); - } - public function tearDown(): void { parent::tearDown(); - Type::overrideStandardTypes(self::$originalStandardTypes); + Type::reset(); } public function testAllowsOverridingStandardTypes(): void { $originalTypes = Type::getStandardTypes(); self::assertCount(5, $originalTypes); - self::assertSame(self::$originalStandardTypes, $originalTypes); $newBooleanType = self::createCustomScalarType(Type::BOOLEAN); $newFloatType = self::createCustomScalarType(Type::FLOAT); @@ -65,7 +58,6 @@ public function testPreservesOriginalStandardTypes(): void { $originalTypes = Type::getStandardTypes(); self::assertCount(5, $originalTypes); - self::assertSame(self::$originalStandardTypes, $originalTypes); $newIDType = self::createCustomScalarType(Type::ID); $newStringType = self::createCustomScalarType(Type::STRING); @@ -122,6 +114,22 @@ public function testStandardTypesOverrideDoesSanityChecks($notType, string $expe Type::overrideStandardTypes([$notType]); } + public function testCachesShouldResetWhenOverridingStandardTypes(): void + { + $string = Type::string(); + $schema = Introspection::_schema(); + $directives = Directive::getInternalDirectives(); + + Type::overrideStandardTypes([ + $newString = self::createCustomScalarType(Type::STRING), + ]); + + self::assertNotSame($string, Type::string()); + self::assertSame($newString, Type::string()); + self::assertNotSame($schema, Introspection::_schema()); + self::assertNotSame($directives, Directive::getInternalDirectives()); + } + /** @throws InvariantViolation */ private static function createCustomScalarType(string $name): CustomScalarType { From b066c693c80b5c7b6c5311e98c58820cedc9e00a Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Tue, 12 Nov 2024 19:12:10 +0100 Subject: [PATCH 11/24] Fix --- src/Type/Definition/Type.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 35ea64c68..d59bac423 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -150,7 +150,7 @@ public static function getStandardTypes(): array */ public static function overrideStandardTypes(array $types): void { - $standardTypes = static::$standardTypes; + $standardTypes = static::$standardTypes ?? []; self::reset(); static::$standardTypes = $standardTypes; From 077b5e61f5590dc3861b98e5614d2b0b04abd71f Mon Sep 17 00:00:00 2001 From: ruudk Date: Tue, 12 Nov 2024 18:13:02 +0000 Subject: [PATCH 12/24] Apply php-cs-fixer changes --- tests/Type/StandardTypesTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Type/StandardTypesTest.php b/tests/Type/StandardTypesTest.php index 068bf516e..282cf3600 100644 --- a/tests/Type/StandardTypesTest.php +++ b/tests/Type/StandardTypesTest.php @@ -6,7 +6,6 @@ use GraphQL\Type\Definition\CustomScalarType; use GraphQL\Type\Definition\Directive; use GraphQL\Type\Definition\ObjectType; -use GraphQL\Type\Definition\ScalarType; use GraphQL\Type\Definition\Type; use GraphQL\Type\Introspection; use PHPUnit\Framework\TestCase; From 05069b3863b768bff4072db5abd1a6505a0fb5d4 Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Tue, 12 Nov 2024 19:15:14 +0100 Subject: [PATCH 13/24] Reset when override --- src/Type/Definition/Type.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index d59bac423..5aa0a817b 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -150,9 +150,7 @@ public static function getStandardTypes(): array */ public static function overrideStandardTypes(array $types): void { - $standardTypes = static::$standardTypes ?? []; self::reset(); - static::$standardTypes = $standardTypes; foreach ($types as $type) { // @phpstan-ignore-next-line generic type is not enforced by PHP From 14c4bd210beb4a68d8d43745565b9fa0d03148e5 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 13 Nov 2024 10:03:44 +0100 Subject: [PATCH 14/24] Fix test --- src/Type/Definition/Type.php | 1 - tests/Type/StandardTypesTest.php | 11 ++++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 5aa0a817b..7893847aa 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -267,7 +267,6 @@ public function jsonSerialize(): string public static function reset(): void { - static::$standardTypes = null; static::$builtInTypes = null; Introspection::reset(); diff --git a/tests/Type/StandardTypesTest.php b/tests/Type/StandardTypesTest.php index 282cf3600..3d9e899e6 100644 --- a/tests/Type/StandardTypesTest.php +++ b/tests/Type/StandardTypesTest.php @@ -6,16 +6,25 @@ use GraphQL\Type\Definition\CustomScalarType; use GraphQL\Type\Definition\Directive; use GraphQL\Type\Definition\ObjectType; +use GraphQL\Type\Definition\ScalarType; use GraphQL\Type\Definition\Type; use GraphQL\Type\Introspection; use PHPUnit\Framework\TestCase; final class StandardTypesTest extends TestCase { + /** @var array */ + private static array $originalStandardTypes; + + public static function setUpBeforeClass(): void + { + self::$originalStandardTypes = Type::getStandardTypes(); + } + public function tearDown(): void { parent::tearDown(); - Type::reset(); + Type::overrideStandardTypes(self::$originalStandardTypes); } public function testAllowsOverridingStandardTypes(): void From 31804161563a2cc18180eb075fbefe230d8f4360 Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Wed, 13 Nov 2024 12:57:50 +0100 Subject: [PATCH 15/24] Remove reset method from Type I think end users don't have to care about this, they can use the overrideStandardTypes method for this. --- src/Type/Definition/Type.php | 13 ++++--------- tests/Type/Definition/TypeTest.php | 24 ------------------------ 2 files changed, 4 insertions(+), 33 deletions(-) delete mode 100644 tests/Type/Definition/TypeTest.php diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 7893847aa..7c68285c0 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -150,7 +150,10 @@ public static function getStandardTypes(): array */ public static function overrideStandardTypes(array $types): void { - self::reset(); + static::$builtInTypes = null; + + Introspection::reset(); + Directive::reset(); foreach ($types as $type) { // @phpstan-ignore-next-line generic type is not enforced by PHP @@ -264,12 +267,4 @@ public function jsonSerialize(): string { return $this->toString(); } - - public static function reset(): void - { - static::$builtInTypes = null; - - Introspection::reset(); - Directive::reset(); - } } diff --git a/tests/Type/Definition/TypeTest.php b/tests/Type/Definition/TypeTest.php deleted file mode 100644 index bac5ff0d3..000000000 --- a/tests/Type/Definition/TypeTest.php +++ /dev/null @@ -1,24 +0,0 @@ - Date: Wed, 13 Nov 2024 12:20:25 +0100 Subject: [PATCH 16/24] Add partial property --- src/GraphQL.php | 5 +++-- src/Type/Definition/Type.php | 7 ++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/GraphQL.php b/src/GraphQL.php index 919b09dec..8ce31cc42 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -211,14 +211,15 @@ public static function getStandardTypes(): array * Standard types not listed here remain untouched. * * @param array $types + * @param bool $partial Whether to replace only types listed in $types or all standard types * * @api * * @throws InvariantViolation */ - public static function overrideStandardTypes(array $types): void + public static function overrideStandardTypes(array $types, bool $partial = true): void { - Type::overrideStandardTypes($types); + Type::overrideStandardTypes($types, $partial); } /** diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 7c68285c0..064302049 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -145,16 +145,21 @@ public static function getStandardTypes(): array /** * @param array $types + * @param bool $partial Whether to replace only types listed in $types or all standard types * * @throws InvariantViolation */ - public static function overrideStandardTypes(array $types): void + public static function overrideStandardTypes(array $types, bool $partial = true): void { static::$builtInTypes = null; Introspection::reset(); Directive::reset(); + if (! $partial) { + static::$standardTypes = null; + } + foreach ($types as $type) { // @phpstan-ignore-next-line generic type is not enforced by PHP if (! $type instanceof ScalarType) { From c62f9cbca60c01ea5ddc8ada44e01ca6d8ac6072 Mon Sep 17 00:00:00 2001 From: ruudk Date: Wed, 13 Nov 2024 11:20:58 +0000 Subject: [PATCH 17/24] Prettify docs --- docs/class-reference.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/class-reference.md b/docs/class-reference.md index 26922b7fc..bbd0922c4 100644 --- a/docs/class-reference.md +++ b/docs/class-reference.md @@ -135,12 +135,13 @@ static function getStandardTypes(): array * Standard types not listed here remain untouched. * * @param array $types + * @param bool $partial Whether to replace only types listed in $types or all standard types * * @api * * @throws InvariantViolation */ -static function overrideStandardTypes(array $types): void +static function overrideStandardTypes(array $types, bool $partial = true): void ``` ```php From bf8c1b57abed406dba4bda1af576bd33861256ed Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 13 Nov 2024 14:39:09 +0100 Subject: [PATCH 18/24] Revert "Add partial property" This reverts commit 386c6dd4d2d545e922ce482b66b44e59224c9ebd. --- src/GraphQL.php | 5 ++--- src/Type/Definition/Type.php | 7 +------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/GraphQL.php b/src/GraphQL.php index 8ce31cc42..919b09dec 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -211,15 +211,14 @@ public static function getStandardTypes(): array * Standard types not listed here remain untouched. * * @param array $types - * @param bool $partial Whether to replace only types listed in $types or all standard types * * @api * * @throws InvariantViolation */ - public static function overrideStandardTypes(array $types, bool $partial = true): void + public static function overrideStandardTypes(array $types): void { - Type::overrideStandardTypes($types, $partial); + Type::overrideStandardTypes($types); } /** diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 064302049..7c68285c0 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -145,21 +145,16 @@ public static function getStandardTypes(): array /** * @param array $types - * @param bool $partial Whether to replace only types listed in $types or all standard types * * @throws InvariantViolation */ - public static function overrideStandardTypes(array $types, bool $partial = true): void + public static function overrideStandardTypes(array $types): void { static::$builtInTypes = null; Introspection::reset(); Directive::reset(); - if (! $partial) { - static::$standardTypes = null; - } - foreach ($types as $type) { // @phpstan-ignore-next-line generic type is not enforced by PHP if (! $type instanceof ScalarType) { From 507a2c1efcbe3c93e2b185dfb9ea88fd6be833c5 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 13 Nov 2024 14:59:53 +0100 Subject: [PATCH 19/24] Simplify directive caching --- src/Type/Definition/Directive.php | 120 ++++++++++++++---------------- 1 file changed, 57 insertions(+), 63 deletions(-) diff --git a/src/Type/Definition/Directive.php b/src/Type/Definition/Directive.php index 4be4fcac3..c955ca22e 100644 --- a/src/Type/Definition/Directive.php +++ b/src/Type/Definition/Directive.php @@ -75,14 +75,6 @@ public function __construct(array $config) $this->config = $config; } - /** @throws InvariantViolation */ - public static function includeDirective(): Directive - { - $internal = self::getInternalDirectives(); - - return $internal['include']; - } - /** * @throws InvariantViolation * @@ -90,71 +82,73 @@ public static function includeDirective(): Directive */ public static function getInternalDirectives(): array { - return self::$internalDirectives ??= [ - 'include' => new self([ - 'name' => self::INCLUDE_NAME, - 'description' => 'Directs the executor to include this field or fragment only when the `if` argument is true.', - 'locations' => [ - DirectiveLocation::FIELD, - DirectiveLocation::FRAGMENT_SPREAD, - DirectiveLocation::INLINE_FRAGMENT, - ], - 'args' => [ - self::IF_ARGUMENT_NAME => [ - 'type' => Type::nonNull(Type::boolean()), - 'description' => 'Included when true.', - ], - ], - ]), - 'skip' => new self([ - 'name' => self::SKIP_NAME, - 'description' => 'Directs the executor to skip this field or fragment when the `if` argument is true.', - 'locations' => [ - DirectiveLocation::FIELD, - DirectiveLocation::FRAGMENT_SPREAD, - DirectiveLocation::INLINE_FRAGMENT, - ], - 'args' => [ - self::IF_ARGUMENT_NAME => [ - 'type' => Type::nonNull(Type::boolean()), - 'description' => 'Skipped when true.', - ], - ], - ]), - 'deprecated' => new self([ - 'name' => self::DEPRECATED_NAME, - 'description' => 'Marks an element of a GraphQL schema as no longer supported.', - 'locations' => [ - DirectiveLocation::FIELD_DEFINITION, - DirectiveLocation::ENUM_VALUE, - DirectiveLocation::ARGUMENT_DEFINITION, - DirectiveLocation::INPUT_FIELD_DEFINITION, - ], - 'args' => [ - self::REASON_ARGUMENT_NAME => [ - 'type' => Type::string(), - 'description' => 'Explains why this element was deprecated, usually also including a suggestion for how to access supported similar data. Formatted using the Markdown syntax, as specified by [CommonMark](https://commonmark.org/).', - 'defaultValue' => self::DEFAULT_DEPRECATION_REASON, - ], - ], - ]), + return [ + self::INCLUDE_NAME => self::includeDirective(), + self::SKIP_NAME => self::skipDirective(), + self::DEPRECATED_NAME => self::deprecatedDirective(), ]; } /** @throws InvariantViolation */ - public static function skipDirective(): Directive + public static function includeDirective(): Directive { - $internal = self::getInternalDirectives(); + return self::$internalDirectives[self::INCLUDE_NAME] ??= new self([ + 'name' => self::INCLUDE_NAME, + 'description' => 'Directs the executor to include this field or fragment only when the `if` argument is true.', + 'locations' => [ + DirectiveLocation::FIELD, + DirectiveLocation::FRAGMENT_SPREAD, + DirectiveLocation::INLINE_FRAGMENT, + ], + 'args' => [ + self::IF_ARGUMENT_NAME => [ + 'type' => Type::nonNull(Type::boolean()), + 'description' => 'Included when true.', + ], + ], + ]); + } - return $internal['skip']; + /** @throws InvariantViolation */ + public static function skipDirective(): Directive + { + return self::$internalDirectives[self::SKIP_NAME] ??= new self([ + 'name' => self::SKIP_NAME, + 'description' => 'Directs the executor to skip this field or fragment when the `if` argument is true.', + 'locations' => [ + DirectiveLocation::FIELD, + DirectiveLocation::FRAGMENT_SPREAD, + DirectiveLocation::INLINE_FRAGMENT, + ], + 'args' => [ + self::IF_ARGUMENT_NAME => [ + 'type' => Type::nonNull(Type::boolean()), + 'description' => 'Skipped when true.', + ], + ], + ]); } /** @throws InvariantViolation */ public static function deprecatedDirective(): Directive { - $internal = self::getInternalDirectives(); - - return $internal['deprecated']; + return self::$internalDirectives[self::DEPRECATED_NAME] ??= new self([ + 'name' => self::DEPRECATED_NAME, + 'description' => 'Marks an element of a GraphQL schema as no longer supported.', + 'locations' => [ + DirectiveLocation::FIELD_DEFINITION, + DirectiveLocation::ENUM_VALUE, + DirectiveLocation::ARGUMENT_DEFINITION, + DirectiveLocation::INPUT_FIELD_DEFINITION, + ], + 'args' => [ + self::REASON_ARGUMENT_NAME => [ + 'type' => Type::string(), + 'description' => 'Explains why this element was deprecated, usually also including a suggestion for how to access supported similar data. Formatted using the Markdown syntax, as specified by [CommonMark](https://commonmark.org/).', + 'defaultValue' => self::DEFAULT_DEPRECATION_REASON, + ], + ], + ]); } /** @throws InvariantViolation */ @@ -163,7 +157,7 @@ public static function isSpecifiedDirective(Directive $directive): bool return \array_key_exists($directive->name, self::getInternalDirectives()); } - public static function reset(): void + public static function resetCachedInstances(): void { self::$internalDirectives = null; } From 3e246602c5f2d91d0c66ef3c64273081705e40d7 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 13 Nov 2024 15:00:02 +0100 Subject: [PATCH 20/24] Simplify introspection caching --- src/Type/Introspection.php | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/Type/Introspection.php b/src/Type/Introspection.php index 9d29dde3d..27dd736f1 100644 --- a/src/Type/Introspection.php +++ b/src/Type/Introspection.php @@ -68,8 +68,8 @@ class Introspection self::DIRECTIVE_LOCATION_ENUM_NAME, ]; - /** @var array */ - private static $map = []; + /** @var array|null */ + protected static ?array $cachedInstances; /** * @param IntrospectionOptions $options @@ -253,7 +253,7 @@ public static function getTypes(): array /** @throws InvariantViolation */ public static function _schema(): ObjectType { - return self::$map[self::SCHEMA_OBJECT_NAME] ??= new ObjectType([ + return self::$cachedInstances[self::SCHEMA_OBJECT_NAME] ??= new ObjectType([ 'name' => self::SCHEMA_OBJECT_NAME, 'isIntrospection' => true, 'description' => 'A GraphQL Schema defines the capabilities of a GraphQL ' @@ -293,7 +293,7 @@ public static function _schema(): ObjectType /** @throws InvariantViolation */ public static function _type(): ObjectType { - return self::$map[self::TYPE_OBJECT_NAME] ??= new ObjectType([ + return self::$cachedInstances[self::TYPE_OBJECT_NAME] ??= new ObjectType([ 'name' => self::TYPE_OBJECT_NAME, 'isIntrospection' => true, 'description' => 'The fundamental unit of any GraphQL Schema is the type. There are ' @@ -444,7 +444,7 @@ public static function _type(): ObjectType /** @throws InvariantViolation */ public static function _typeKind(): EnumType { - return self::$map[self::TYPE_KIND_ENUM_NAME] ??= new EnumType([ + return self::$cachedInstances[self::TYPE_KIND_ENUM_NAME] ??= new EnumType([ 'name' => self::TYPE_KIND_ENUM_NAME, 'isIntrospection' => true, 'description' => 'An enum describing what kind of type a given `__Type` is.', @@ -488,7 +488,7 @@ public static function _typeKind(): EnumType /** @throws InvariantViolation */ public static function _field(): ObjectType { - return self::$map[self::FIELD_OBJECT_NAME] ??= new ObjectType([ + return self::$cachedInstances[self::FIELD_OBJECT_NAME] ??= new ObjectType([ 'name' => self::FIELD_OBJECT_NAME, 'isIntrospection' => true, 'description' => 'Object and Interface types are described by a list of Fields, each of ' @@ -542,7 +542,7 @@ public static function _field(): ObjectType /** @throws InvariantViolation */ public static function _inputValue(): ObjectType { - return self::$map[self::INPUT_VALUE_OBJECT_NAME] ??= new ObjectType([ + return self::$cachedInstances[self::INPUT_VALUE_OBJECT_NAME] ??= new ObjectType([ 'name' => self::INPUT_VALUE_OBJECT_NAME, 'isIntrospection' => true, 'description' => 'Arguments provided to Fields or Directives and the input fields of an ' @@ -600,7 +600,7 @@ public static function _inputValue(): ObjectType /** @throws InvariantViolation */ public static function _enumValue(): ObjectType { - return self::$map[self::ENUM_VALUE_OBJECT_NAME] ??= new ObjectType([ + return self::$cachedInstances[self::ENUM_VALUE_OBJECT_NAME] ??= new ObjectType([ 'name' => self::ENUM_VALUE_OBJECT_NAME, 'isIntrospection' => true, 'description' => 'One possible value for a given Enum. Enum values are unique values, not ' @@ -630,7 +630,7 @@ public static function _enumValue(): ObjectType /** @throws InvariantViolation */ public static function _directive(): ObjectType { - return self::$map[self::DIRECTIVE_OBJECT_NAME] ??= new ObjectType([ + return self::$cachedInstances[self::DIRECTIVE_OBJECT_NAME] ??= new ObjectType([ 'name' => self::DIRECTIVE_OBJECT_NAME, 'isIntrospection' => true, 'description' => 'A Directive provides a way to describe alternate runtime execution and ' @@ -669,7 +669,7 @@ public static function _directive(): ObjectType /** @throws InvariantViolation */ public static function _directiveLocation(): EnumType { - return self::$map[self::DIRECTIVE_LOCATION_ENUM_NAME] ??= new EnumType([ + return self::$cachedInstances[self::DIRECTIVE_LOCATION_ENUM_NAME] ??= new EnumType([ 'name' => self::DIRECTIVE_LOCATION_ENUM_NAME, 'isIntrospection' => true, 'description' => 'A Directive can be adjacent to many parts of the GraphQL language, a ' @@ -758,7 +758,7 @@ public static function _directiveLocation(): EnumType /** @throws InvariantViolation */ public static function schemaMetaFieldDef(): FieldDefinition { - return self::$map[self::SCHEMA_FIELD_NAME] ??= new FieldDefinition([ + return self::$cachedInstances[self::SCHEMA_FIELD_NAME] ??= new FieldDefinition([ 'name' => self::SCHEMA_FIELD_NAME, 'type' => Type::nonNull(self::_schema()), 'description' => 'Access the current type schema of this server.', @@ -770,7 +770,7 @@ public static function schemaMetaFieldDef(): FieldDefinition /** @throws InvariantViolation */ public static function typeMetaFieldDef(): FieldDefinition { - return self::$map[self::TYPE_FIELD_NAME] ??= new FieldDefinition([ + return self::$cachedInstances[self::TYPE_FIELD_NAME] ??= new FieldDefinition([ 'name' => self::TYPE_FIELD_NAME, 'type' => self::_type(), 'description' => 'Request the type information of a single type.', @@ -787,7 +787,7 @@ public static function typeMetaFieldDef(): FieldDefinition /** @throws InvariantViolation */ public static function typeNameMetaFieldDef(): FieldDefinition { - return self::$map[self::TYPE_NAME_FIELD_NAME] ??= new FieldDefinition([ + return self::$cachedInstances[self::TYPE_NAME_FIELD_NAME] ??= new FieldDefinition([ 'name' => self::TYPE_NAME_FIELD_NAME, 'type' => Type::nonNull(Type::string()), 'description' => 'The name of the current Object type at runtime.', @@ -796,8 +796,8 @@ public static function typeNameMetaFieldDef(): FieldDefinition ]); } - public static function reset(): void + public static function resetCachedInstances(): void { - self::$map = []; + self::$cachedInstances = null; } } From 30a1c925678bcda38fab3d159298d21d78fa1522 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 13 Nov 2024 15:00:17 +0100 Subject: [PATCH 21/24] Format complex conditionals --- src/Executor/ReferenceExecutor.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Executor/ReferenceExecutor.php b/src/Executor/ReferenceExecutor.php index 224d50aef..1867cbd7b 100644 --- a/src/Executor/ReferenceExecutor.php +++ b/src/Executor/ReferenceExecutor.php @@ -713,11 +713,15 @@ protected function getFieldDef(Schema $schema, ObjectType $parentType, string $f $queryType = $schema->getQueryType(); - if ($fieldName === $this->schemaMetaFieldDef->name && $queryType === $parentType) { + if ($fieldName === $this->schemaMetaFieldDef->name + && $queryType === $parentType + ) { return $this->schemaMetaFieldDef; } - if ($fieldName === $this->typeMetaFieldDef->name && $queryType === $parentType) { + if ($fieldName === $this->typeMetaFieldDef->name + && $queryType === $parentType + ) { return $this->typeMetaFieldDef; } From 6b6284f0b1cb0be7740594508efc6d8213998647 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 13 Nov 2024 15:00:42 +0100 Subject: [PATCH 22/24] Restore assertions --- tests/Type/StandardTypesTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Type/StandardTypesTest.php b/tests/Type/StandardTypesTest.php index 3d9e899e6..279fb2607 100644 --- a/tests/Type/StandardTypesTest.php +++ b/tests/Type/StandardTypesTest.php @@ -31,6 +31,7 @@ public function testAllowsOverridingStandardTypes(): void { $originalTypes = Type::getStandardTypes(); self::assertCount(5, $originalTypes); + self::assertSame(self::$originalStandardTypes, $originalTypes); $newBooleanType = self::createCustomScalarType(Type::BOOLEAN); $newFloatType = self::createCustomScalarType(Type::FLOAT); @@ -66,6 +67,7 @@ public function testPreservesOriginalStandardTypes(): void { $originalTypes = Type::getStandardTypes(); self::assertCount(5, $originalTypes); + self::assertSame(self::$originalStandardTypes, $originalTypes); $newIDType = self::createCustomScalarType(Type::ID); $newStringType = self::createCustomScalarType(Type::STRING); From a19b38fdee3f1f58d8355d25173335b5200eef23 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 13 Nov 2024 15:00:47 +0100 Subject: [PATCH 23/24] Revert docs --- docs/class-reference.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/class-reference.md b/docs/class-reference.md index bbd0922c4..26922b7fc 100644 --- a/docs/class-reference.md +++ b/docs/class-reference.md @@ -135,13 +135,12 @@ static function getStandardTypes(): array * Standard types not listed here remain untouched. * * @param array $types - * @param bool $partial Whether to replace only types listed in $types or all standard types * * @api * * @throws InvariantViolation */ -static function overrideStandardTypes(array $types, bool $partial = true): void +static function overrideStandardTypes(array $types): void ``` ```php From 3fde9d1c6aa28e127e4f0bd2b5c2886a6be3fb32 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 13 Nov 2024 15:00:56 +0100 Subject: [PATCH 24/24] Make code more grokable --- src/Type/Definition/Type.php | 6 +++--- tests/Type/StandardTypesTest.php | 25 ++++++++++++++++--------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 7c68285c0..35224f514 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -150,10 +150,10 @@ public static function getStandardTypes(): array */ public static function overrideStandardTypes(array $types): void { + // Reset caches that might contain instances of standard types static::$builtInTypes = null; - - Introspection::reset(); - Directive::reset(); + Introspection::resetCachedInstances(); + Directive::resetCachedInstances(); foreach ($types as $type) { // @phpstan-ignore-next-line generic type is not enforced by PHP diff --git a/tests/Type/StandardTypesTest.php b/tests/Type/StandardTypesTest.php index 279fb2607..6a569f45e 100644 --- a/tests/Type/StandardTypesTest.php +++ b/tests/Type/StandardTypesTest.php @@ -127,17 +127,24 @@ public function testStandardTypesOverrideDoesSanityChecks($notType, string $expe public function testCachesShouldResetWhenOverridingStandardTypes(): void { $string = Type::string(); - $schema = Introspection::_schema(); - $directives = Directive::getInternalDirectives(); - Type::overrideStandardTypes([ - $newString = self::createCustomScalarType(Type::STRING), - ]); + $typeNameMetaFieldDef = Introspection::typeNameMetaFieldDef(); + self::assertSame($string, Type::getNullableType($typeNameMetaFieldDef->getType())); + + $deprecatedDirective = Directive::deprecatedDirective(); + self::assertSame($string, $deprecatedDirective->args[0]->getType()); + + $newString = self::createCustomScalarType(Type::STRING); + self::assertNotSame($string, $newString); + Type::overrideStandardTypes([$newString]); + + $newTypeNameMetaFieldDef = Introspection::typeNameMetaFieldDef(); + self::assertNotSame($typeNameMetaFieldDef, $newTypeNameMetaFieldDef); + self::assertSame($newString, Type::getNullableType($newTypeNameMetaFieldDef->getType())); - self::assertNotSame($string, Type::string()); - self::assertSame($newString, Type::string()); - self::assertNotSame($schema, Introspection::_schema()); - self::assertNotSame($directives, Directive::getInternalDirectives()); + $newDeprecatedDirective = Directive::deprecatedDirective(); + self::assertNotSame($deprecatedDirective, $newDeprecatedDirective); + self::assertSame($newString, $newDeprecatedDirective->args[0]->getType()); } /** @throws InvariantViolation */