From 821365664cb612af02974c3b9f296b6987e7b2c5 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 28 Nov 2023 11:30:08 +0100 Subject: [PATCH 1/4] Fix type registry examples https://github.com/webonyx/graphql-php/issues/1478 --- docs/schema-definition.md | 49 ++++++---- examples/01-blog/Blog/Types.php | 165 ++++++++++++++++---------------- examples/01-blog/graphql.php | 2 +- 3 files changed, 116 insertions(+), 100 deletions(-) diff --git a/docs/schema-definition.md b/docs/schema-definition.md index 2ac66bdbf..99179a06e 100644 --- a/docs/schema-definition.md +++ b/docs/schema-definition.md @@ -157,43 +157,60 @@ final class AuthorType extends ObjectType // Types.php use GraphQL\Type\Definition\Type; +use GraphQL\Type\Definition\NamedType; final class Types { /** @var array */ private static array $types = []; - /** @return \Closure(): Type&NamedType */ - public static function get(string $classname): \Closure + public static function byTypeName(string $typeName): \Closure { - return static fn () => self::byClassName($classname); - } + if (isset(self::$types[$typeName])) { + return self::$types[$typeName]; + } - public static function byTypeName(string $typeName): Type&NamedType - { - return match ($typeName) { - 'Boolean' => self::boolean(), - 'Float' => self::float(), - 'ID' => self::id(), - 'Int' => self::int(), - default => self::$types[$typeName] ?? throw new \Exception("Unknown GraphQL type: {$typeName}."), + $methodName = match ($typeName) { + 'ID' => 'id', + default => lcfirst($typeName), + }; + if (! method_exists(self::class, $methodName)) { + throw new \Exception("Unknown GraphQL type: {$typeName}."); + } + + $type = self::{$methodName}(); // @phpstan-ignore-line variable static method call + if (is_callable($type)) { + $type = $type(); } + + return self::$types[$typeName] = $type; } - private static function byClassName(string $classname): Type + /** @return Type&NamedType */ + private static function byClassName(string $className): Type { - $parts = \explode('\\', $classname); + $parts = \explode('\\', $className); $typeName = \preg_replace('~Type$~', '', $parts[\count($parts) - 1]); // Type loading is very similar to PHP class loading, but keep in mind // that the **typeLoader** must always return the same instance of a type. // We can enforce that in our type registry by caching known types. - return self::$types[$typeName] ??= new $classname; + return self::$types[$typeName] ??= new $className; + } + + /** @return \Closure(): Type&NamedType */ + private static function get(string $className): \Closure + { + return static fn () => self::byClassName($className); } + public static function boolean(): ScalarType { return Type::boolean(); } + public static function float(): ScalarType { return Type::float(); } + public static function id(): ScalarType { return Type::id(); } + public static function int(): ScalarType { return Type::int(); } + public static function string(): ScalarType { return Type::string(); } public static function author(): callable { return self::get(AuthorType::class); } public static function story(): callable { return self::get(StoryType::class); } - ... } diff --git a/examples/01-blog/Blog/Types.php b/examples/01-blog/Blog/Types.php index cdf389fa3..f7078d733 100644 --- a/examples/01-blog/Blog/Types.php +++ b/examples/01-blog/Blog/Types.php @@ -29,143 +29,142 @@ final class Types /** @var array */ private static array $types = []; - public static function user(): callable + /** + * @throws \Exception + * + * @return Type&NamedType + */ + public static function byTypeName(string $typeName): Type { - return self::get(UserType::class); + if (isset(self::$types[$typeName])) { + return self::$types[$typeName]; + } + + switch ($typeName) { + case 'ID': + $methodName = 'id'; + break; + default: + $methodName = \lcfirst($typeName); + } + if (! method_exists(self::class, $methodName)) { + throw new \Exception("Unknown GraphQL type: {$typeName}."); + } + + $type = self::{$methodName}(); // @phpstan-ignore-line variable static method call + if (is_callable($type)) { + $type = $type(); + } + + return self::$types[$typeName] = $type; } - public static function story(): callable + /** + * @param class-string $classname + * + * @return \Closure(): Type + */ + private static function get(string $classname): \Closure { - return self::get(StoryType::class); + return static fn () => self::byClassName($classname); } - public static function comment(): callable + /** @param class-string $className */ + private static function byClassName(string $className): Type { - return self::get(CommentType::class); + $parts = \explode('\\', $className); + + $typeName = \preg_replace('~Type$~', '', $parts[\count($parts) - 1]); + assert(is_string($typeName), 'regex is statically known to be correct'); + + // Type loading is very similar to PHP class loading, but keep in mind + // that the **typeLoader** must always return the same instance of a type. + // We can enforce that in our type registry by caching known types. + return self::$types[$typeName] ??= new $className(); } - public static function image(): callable + /** @throws InvariantViolation */ + public static function boolean(): ScalarType { - return self::get(ImageType::class); + return Type::boolean(); } - public static function node(): callable + /** @throws InvariantViolation */ + public static function float(): ScalarType { - return self::get(NodeType::class); + return Type::float(); } - public static function mention(): callable + /** @throws InvariantViolation */ + public static function id(): ScalarType { - return self::get(SearchResultType::class); + return Type::id(); } - public static function imageSize(): callable + /** @throws InvariantViolation */ + public static function int(): ScalarType { - return self::get(ImageSizeType::class); + return Type::int(); } - public static function contentFormat(): callable + /** @throws InvariantViolation */ + public static function string(): ScalarType { - return self::get(ContentFormatType::class); + return Type::string(); } - public static function storyAffordances(): callable + public static function user(): callable { - return self::get(StoryAffordancesType::class); + return self::get(UserType::class); } - public static function email(): callable + public static function story(): callable { - return self::get(EmailType::class); + return self::get(StoryType::class); } - public static function url(): callable + public static function comment(): callable { - return self::get(UrlType::class); + return self::get(CommentType::class); } - /** - * @param class-string $classname - * - * @return \Closure(): Type - */ - private static function get(string $classname): \Closure + public static function image(): callable { - return static fn () => self::byClassName($classname); + return self::get(ImageType::class); } - /** @param class-string $classname */ - private static function byClassName(string $classname): Type + public static function node(): callable { - $parts = \explode('\\', $classname); - - $withoutTypePrefix = \preg_replace('~Type$~', '', $parts[\count($parts) - 1]); - assert(is_string($withoutTypePrefix), 'regex is statically known to be correct'); - - $cacheName = \strtolower($withoutTypePrefix); - - if (! isset(self::$types[$cacheName])) { - return self::$types[$cacheName] = new $classname(); - } - - return self::$types[$cacheName]; + return self::get(NodeType::class); } - /** - * @throws \Exception - * - * @return Type&NamedType - */ - public static function byTypeName(string $shortName): Type + public static function mention(): callable { - $cacheName = \strtolower($shortName); - - if (isset(self::$types[$cacheName])) { - return self::$types[$cacheName]; - } - - $method = \lcfirst($shortName); - switch ($method) { - case 'boolean': - return self::boolean(); - case 'float': - return self::float(); - case 'id': - return self::id(); - case 'int': - return self::int(); - } - - throw new \Exception("Unknown graphql type: {$shortName}"); + return self::get(SearchResultType::class); } - /** @throws InvariantViolation */ - public static function boolean(): ScalarType + public static function imageSize(): callable { - return Type::boolean(); + return self::get(ImageSizeType::class); } - /** @throws InvariantViolation */ - public static function float(): ScalarType + public static function contentFormat(): callable { - return Type::float(); + return self::get(ContentFormatType::class); } - /** @throws InvariantViolation */ - public static function id(): ScalarType + public static function storyAffordances(): callable { - return Type::id(); + return self::get(StoryAffordancesType::class); } - /** @throws InvariantViolation */ - public static function int(): ScalarType + public static function email(): callable { - return Type::int(); + return self::get(EmailType::class); } - /** @throws InvariantViolation */ - public static function string(): ScalarType + public static function url(): callable { - return Type::string(); + return self::get(UrlType::class); } } diff --git a/examples/01-blog/graphql.php b/examples/01-blog/graphql.php index e00303638..a3fcaf315 100644 --- a/examples/01-blog/graphql.php +++ b/examples/01-blog/graphql.php @@ -25,7 +25,7 @@ $schema = new Schema( (new SchemaConfig()) ->setQuery(new QueryType()) - ->setTypeLoader([Types::class, 'byTypename']) + ->setTypeLoader([Types::class, 'byTypeName']) ); // Prepare context that will be available in all field resolvers (as 3rd argument): From bb4537990114a4a68a25caceeacba63389e72ecf Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 29 Nov 2023 10:04:53 +0100 Subject: [PATCH 2/4] improve types and naming --- docs/schema-definition.md | 11 +++++---- examples/01-blog/Blog/Types.php | 44 ++++++++++++++++----------------- examples/01-blog/graphql.php | 2 +- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/docs/schema-definition.md b/docs/schema-definition.md index 99179a06e..fd6d6eb9c 100644 --- a/docs/schema-definition.md +++ b/docs/schema-definition.md @@ -164,7 +164,8 @@ final class Types /** @var array */ private static array $types = []; - public static function byTypeName(string $typeName): \Closure + /** @return Type&NamedType */ + public static function load(string $typeName): Type { if (isset(self::$types[$typeName])) { return self::$types[$typeName]; @@ -199,7 +200,7 @@ final class Types } /** @return \Closure(): Type&NamedType */ - private static function get(string $className): \Closure + private static function lazyByClassName(string $className): \Closure { return static fn () => self::byClassName($className); } @@ -209,8 +210,8 @@ final class Types public static function id(): ScalarType { return Type::id(); } public static function int(): ScalarType { return Type::int(); } public static function string(): ScalarType { return Type::string(); } - public static function author(): callable { return self::get(AuthorType::class); } - public static function story(): callable { return self::get(StoryType::class); } + public static function author(): callable { return self::lazyByClassName(AuthorType::class); } + public static function story(): callable { return self::lazyByClassName(StoryType::class); } ... } @@ -231,7 +232,7 @@ $schema = new Schema([ ], ], ]), - 'typeLoader' => Types::byTypeName(...), + 'typeLoader' => Types::load(...), ]); ``` diff --git a/examples/01-blog/Blog/Types.php b/examples/01-blog/Blog/Types.php index f7078d733..626120d4a 100644 --- a/examples/01-blog/Blog/Types.php +++ b/examples/01-blog/Blog/Types.php @@ -34,7 +34,7 @@ final class Types * * @return Type&NamedType */ - public static function byTypeName(string $typeName): Type + public static function load(string $typeName): Type { if (isset(self::$types[$typeName])) { return self::$types[$typeName]; @@ -59,16 +59,6 @@ public static function byTypeName(string $typeName): Type return self::$types[$typeName] = $type; } - /** - * @param class-string $classname - * - * @return \Closure(): Type - */ - private static function get(string $classname): \Closure - { - return static fn () => self::byClassName($classname); - } - /** @param class-string $className */ private static function byClassName(string $className): Type { @@ -83,6 +73,16 @@ private static function byClassName(string $className): Type return self::$types[$typeName] ??= new $className(); } + /** + * @param class-string $classname + * + * @return \Closure(): Type&NamedType + */ + private static function lazyByClassName(string $classname): \Closure + { + return static fn () => self::byClassName($classname); + } + /** @throws InvariantViolation */ public static function boolean(): ScalarType { @@ -115,56 +115,56 @@ public static function string(): ScalarType public static function user(): callable { - return self::get(UserType::class); + return self::lazyByClassName(UserType::class); } public static function story(): callable { - return self::get(StoryType::class); + return self::lazyByClassName(StoryType::class); } public static function comment(): callable { - return self::get(CommentType::class); + return self::lazyByClassName(CommentType::class); } public static function image(): callable { - return self::get(ImageType::class); + return self::lazyByClassName(ImageType::class); } public static function node(): callable { - return self::get(NodeType::class); + return self::lazyByClassName(NodeType::class); } public static function mention(): callable { - return self::get(SearchResultType::class); + return self::lazyByClassName(SearchResultType::class); } public static function imageSize(): callable { - return self::get(ImageSizeType::class); + return self::lazyByClassName(ImageSizeType::class); } public static function contentFormat(): callable { - return self::get(ContentFormatType::class); + return self::lazyByClassName(ContentFormatType::class); } public static function storyAffordances(): callable { - return self::get(StoryAffordancesType::class); + return self::lazyByClassName(StoryAffordancesType::class); } public static function email(): callable { - return self::get(EmailType::class); + return self::lazyByClassName(EmailType::class); } public static function url(): callable { - return self::get(UrlType::class); + return self::lazyByClassName(UrlType::class); } } diff --git a/examples/01-blog/graphql.php b/examples/01-blog/graphql.php index a3fcaf315..09204a659 100644 --- a/examples/01-blog/graphql.php +++ b/examples/01-blog/graphql.php @@ -25,7 +25,7 @@ $schema = new Schema( (new SchemaConfig()) ->setQuery(new QueryType()) - ->setTypeLoader([Types::class, 'byTypeName']) + ->setTypeLoader([Types::class, 'load']) ); // Prepare context that will be available in all field resolvers (as 3rd argument): From fbc429d57db2ff334655641303bb32b4a76a7885 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 29 Nov 2023 10:08:47 +0100 Subject: [PATCH 3/4] fix types --- docs/schema-definition.md | 2 +- examples/01-blog/Blog/Types.php | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/schema-definition.md b/docs/schema-definition.md index fd6d6eb9c..e3c74b5ed 100644 --- a/docs/schema-definition.md +++ b/docs/schema-definition.md @@ -199,7 +199,7 @@ final class Types return self::$types[$typeName] ??= new $className; } - /** @return \Closure(): Type&NamedType */ + /** @return \Closure(): (Type&NamedType) */ private static function lazyByClassName(string $className): \Closure { return static fn () => self::byClassName($className); diff --git a/examples/01-blog/Blog/Types.php b/examples/01-blog/Blog/Types.php index 626120d4a..d9451a4f8 100644 --- a/examples/01-blog/Blog/Types.php +++ b/examples/01-blog/Blog/Types.php @@ -59,7 +59,11 @@ public static function load(string $typeName): Type return self::$types[$typeName] = $type; } - /** @param class-string $className */ + /** + * @param class-string $className + * + * @return Type&NamedType + */ private static function byClassName(string $className): Type { $parts = \explode('\\', $className); @@ -76,7 +80,7 @@ private static function byClassName(string $className): Type /** * @param class-string $classname * - * @return \Closure(): Type&NamedType + * @return \Closure(): (Type&NamedType) */ private static function lazyByClassName(string $classname): \Closure { From 182e3e7dd2aa5cf7faf171358c856359b8eb4ee0 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 30 Nov 2023 09:44:18 +0100 Subject: [PATCH 4/4] explain conventions --- docs/schema-definition.md | 9 +++++++-- examples/01-blog/Blog/Types.php | 10 +++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/schema-definition.md b/docs/schema-definition.md index e3c74b5ed..35b7c2b70 100644 --- a/docs/schema-definition.md +++ b/docs/schema-definition.md @@ -171,6 +171,8 @@ final class Types return self::$types[$typeName]; } + // For every type, this class must define a method with the same name + // but the first letter is in lower case. $methodName = match ($typeName) { 'ID' => 'id', default => lcfirst($typeName), @@ -190,8 +192,11 @@ final class Types /** @return Type&NamedType */ private static function byClassName(string $className): Type { - $parts = \explode('\\', $className); - $typeName = \preg_replace('~Type$~', '', $parts[\count($parts) - 1]); + $classNameParts = explode('\\', $className); + $baseClassName = end($classNameParts); + // All type classes must use the suffix Type. + // This prevents name collisions between types and PHP keywords. + $typeName = preg_replace('~Type$~', '', $baseClassName); // Type loading is very similar to PHP class loading, but keep in mind // that the **typeLoader** must always return the same instance of a type. diff --git a/examples/01-blog/Blog/Types.php b/examples/01-blog/Blog/Types.php index d9451a4f8..d1c108458 100644 --- a/examples/01-blog/Blog/Types.php +++ b/examples/01-blog/Blog/Types.php @@ -40,6 +40,8 @@ public static function load(string $typeName): Type return self::$types[$typeName]; } + // For every type, this class must define a method with the same name + // but the first letter is in lower case. switch ($typeName) { case 'ID': $methodName = 'id'; @@ -66,9 +68,11 @@ public static function load(string $typeName): Type */ private static function byClassName(string $className): Type { - $parts = \explode('\\', $className); - - $typeName = \preg_replace('~Type$~', '', $parts[\count($parts) - 1]); + $classNameParts = \explode('\\', $className); + $baseClassName = end($classNameParts); + // All type classes must use the suffix Type. + // This prevents name collisions between types and PHP keywords. + $typeName = \preg_replace('~Type$~', '', $baseClassName); assert(is_string($typeName), 'regex is statically known to be correct'); // Type loading is very similar to PHP class loading, but keep in mind