Merge pull request #235 from moufmouf/enum_issue

Changing the default name given to enum types in the GraphQL schema

Author: moufmouf

docs/annotations_reference.md

@@ -241,3 +241,15 @@ Attribute      | Compulsory | Type | Definition
 ---------------|------------|------|--------
 *for*          | *yes*      | string | The name of the PHP parameter
 *constraint*   | *yes       | annotation | One (or many) Symfony validation annotations.
+
+## @EnumType annotation
+
+The `@EnumType` annotation is used to change the name of a "Enum" type.
+Note that if you do not want to change the name, the annotation is optionnal. Any object extending `MyCLabs\Enum\Enum`
+is automatically mapped to a GraphQL enum type.
+
+**Applies on**: classes extending the `MyCLabs\Enum\Enum` base class.
+
+Attribute      | Compulsory | Type | Definition
+---------------|------------|------|--------
+name           | *no*       | string | The name of the enum type (in the GraphQL schema)

docs/type_mapping.md

@@ -223,7 +223,7 @@ class StatusEnum extends Enum
  */
 public function users(StatusEnum $status): array
 {
-    if ($status == StatusEum::ON()) {
+    if ($status == StatusEnum::ON()) {
         // Note that the "magic" ON() method returns an instance of the StatusEnum class.
         // Also, note that we are comparing this instance using "==" (using "===" would fail as we have 2 different instances here)
         // ...
@@ -232,6 +232,30 @@ public function users(StatusEnum $status): array
 }
 ```
 
+```graphql
+query users($status: StatusEnum!) {}
+    users(status: $status) {
+        id
+    }
+}
+```
+
+By default, the name of the GraphQL enum type will be the name of the class. If you have a naming conflict (two classes
+that live in different namespaces with the same class name), you can solve it using the `@EnumType` annotation:
+
+```php
+use TheCodingMachine\GraphQLite\Annotations\EnumType;
+
+/**
+ * @EnumType(name="UserStatus")
+ */
+class StatusEnum extends Enum
+{
+    // ...
+}
+```
+
+
 <div class="alert alert-info">There are many enumeration library in PHP and you might be using another library.
 If you want to add support for your own library, this is not extremely difficult to do. You need to register a custom
 "RootTypeMapper" with GraphQLite. You can learn more about <em>type mappers</em> in the <a href="internals.md">"internals" documentation</a>

phpstan.neon

@@ -29,9 +29,13 @@ parameters:
         -
            message: '#Method TheCodingMachine\\GraphQLite\\AnnotationReader::getMethodAnnotations\(\) should return array<int, T of object> but returns array<object>.#'
            path: src/AnnotationReader.php
+        -
+           message: '#Parameter \#1 \$enumClass of method TheCodingMachine\\GraphQLite\\Mappers\\Root\\MyCLabsEnumTypeMapper::getTypeName\(\) expects class-string<MyCLabs\\Enum\\Enum>, class-string<object> given.#'
+           path: src/Mappers/Root/MyCLabsEnumTypeMapper.php
         - '#Call to an undefined method GraphQL\\Error\\ClientAware::getMessage()#'
         # Needed because of a bug in PHP-CS
         - '#PHPDoc tag @param for parameter \$args with type mixed is not subtype of native type array<int, mixed>.#'
+
         #-
         #  message: '#If condition is always true#'
         #  path: src/Middlewares/SecurityFieldMiddleware.php

src/AnnotationReader.php

@@ -7,12 +7,14 @@
 use Doctrine\Common\Annotations\AnnotationException;
 use Doctrine\Common\Annotations\Reader;
 use InvalidArgumentException;
+use MyCLabs\Enum\Enum;
 use ReflectionClass;
 use ReflectionMethod;
 use ReflectionParameter;
 use RuntimeException;
 use TheCodingMachine\GraphQLite\Annotations\AbstractRequest;
 use TheCodingMachine\GraphQLite\Annotations\Decorate;
+use TheCodingMachine\GraphQLite\Annotations\EnumType;
 use TheCodingMachine\GraphQLite\Annotations\Exceptions\ClassNotFoundException;
 use TheCodingMachine\GraphQLite\Annotations\Exceptions\InvalidParameterException;
 use TheCodingMachine\GraphQLite\Annotations\ExtendType;
@@ -85,7 +87,6 @@ public function getTypeAnnotation(ReflectionClass $refClass): ?Type
     {
         try {
             $type = $this->getClassAnnotation($refClass, Type::class);
-            assert($type instanceof Type || $type === null);
             if ($type !== null && $type->isSelfType()) {
                 $type->setClass($refClass->getName());
             }
@@ -105,7 +106,6 @@ public function getExtendTypeAnnotation(ReflectionClass $refClass): ?ExtendType
     {
         try {
             $extendType = $this->getClassAnnotation($refClass, ExtendType::class);
-            assert($extendType instanceof ExtendType || $extendType === null);
         } catch (ClassNotFoundException $e) {
             throw ClassNotFoundException::wrapExceptionForExtendTag($e, $refClass->getName());
         }
@@ -230,7 +230,12 @@ public function getMiddlewareAnnotations(ReflectionMethod $refMethod): Middlewar
     /**
      * Returns a class annotation. Does not look in the parent class.
      *
-     * @param ReflectionClass<T> $refClass
+     * @param ReflectionClass<object> $refClass
+     * @param class-string<T> $annotationClass
+     *
+     * @return T|null
+     *
+     * @throws AnnotationException
      *
      * @template T of object
      */
@@ -239,6 +244,7 @@ private function getClassAnnotation(ReflectionClass $refClass, string $annotatio
         $type = null;
         try {
             $type = $this->reader->getClassAnnotation($refClass, $annotationClass);
+            assert($type === null || $type instanceof $annotationClass);
         } catch (AnnotationException $e) {
             switch ($this->mode) {
                 case self::STRICT_MODE:
@@ -389,4 +395,12 @@ public function getMethodAnnotations(ReflectionMethod $refMethod, string $annota
 
         return $toAddAnnotations;
     }
+
+    /**
+     * @param ReflectionClass<Enum> $refClass
+     */
+    public function getEnumTypeAnnotation(ReflectionClass $refClass): ?EnumType
+    {
+        return $this->getClassAnnotation($refClass, EnumType::class);
+    }
 }

src/Annotations/EnumType.php

@@ -0,0 +1,36 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TheCodingMachine\GraphQLite\Annotations;
+
+/**
+ * The EnumType annotation is useful to change the name of the generated "enum" type.
+ *
+ * @Annotation
+ * @Target({"CLASS"})
+ * @Attributes({
+ *   @Attribute("name", type = "string"),
+ * })
+ */
+class EnumType
+{
+    /** @var string|null */
+    private $name;
+
+    /**
+     * @param mixed[] $attributes
+     */
+    public function __construct(array $attributes = [])
+    {
+        $this->name = $attributes['name'] ?? null;
+    }
+
+    /**
+     * Returns the GraphQL name for this type.
+     */
+    public function getName(): ?string
+    {
+        return $this->name;
+    }
+}

src/Mappers/Root/MyCLabsEnumTypeMapper.php

@@ -13,26 +13,30 @@
 use phpDocumentor\Reflection\DocBlock;
 use phpDocumentor\Reflection\Type;
 use phpDocumentor\Reflection\Types\Object_;
+use ReflectionClass;
 use ReflectionMethod;
+use TheCodingMachine\GraphQLite\AnnotationReader;
 use TheCodingMachine\GraphQLite\Types\MyCLabsEnumType;
 use function is_a;
-use function str_replace;
-use function strpos;
-use function substr;
 
 /**
  * Maps an class extending MyCLabs enums to a GraphQL type
  */
 class MyCLabsEnumTypeMapper implements RootTypeMapperInterface
 {
-    /** @var array<string, EnumType> */
+    /** @var array<class-string<object>, EnumType> */
     private $cache = [];
+    /** @var array<string, EnumType> */
+    private $cacheByName = [];
     /** @var RootTypeMapperInterface */
     private $next;
+    /** @var AnnotationReader */
+    private $annotationReader;
 
-    public function __construct(RootTypeMapperInterface $next)
+    public function __construct(RootTypeMapperInterface $next, AnnotationReader $annotationReader)
     {
         $this->next = $next;
+        $this->annotationReader = $annotationReader;
     }
 
     /**
@@ -74,11 +78,17 @@ private function map(Type $type): ?EnumType
         if ($fqsen === null) {
             return null;
         }
+        /**
+         * @var class-string<object>
+         */
         $enumClass = (string) $fqsen;
 
         return $this->mapByClassName($enumClass);
     }
 
+    /**
+     * @param class-string<object> $enumClass
+     */
     private function mapByClassName(string $enumClass): ?EnumType
     {
         if (! is_a($enumClass, Enum::class, true)) {
@@ -88,7 +98,24 @@ private function mapByClassName(string $enumClass): ?EnumType
             return $this->cache[$enumClass];
         }
 
-        return $this->cache[$enumClass] = new MyCLabsEnumType($enumClass);
+        $type = new MyCLabsEnumType($enumClass, $this->getTypeName($enumClass));
+        return $this->cacheByName[$type->name] = $this->cache[$enumClass] = $type;
+    }
+
+    /**
+     * @param class-string<Enum> $enumClass
+     */
+    private function getTypeName(string $enumClass): string
+    {
+        $refClass = new ReflectionClass($enumClass);
+        $enumType = $this->annotationReader->getEnumTypeAnnotation($refClass);
+        if ($enumType !== null) {
+            $name = $enumType->getName();
+            if ($name !== null) {
+                return $name;
+            }
+        }
+        return $refClass->getShortName();
     }
 
     /**
@@ -100,14 +127,20 @@ private function mapByClassName(string $enumClass): ?EnumType
      */
     public function mapNameToType(string $typeName): NamedType
     {
-        if (strpos($typeName, 'MyCLabsEnum_') === 0) {
+        // This is a hack to make sure "$schema->assertValid()" returns true.
+        // The mapNameToType will fail if the mapByClassName method was not called before.
+        // This is actually not an issue in real life scenarios where enum types are never queried by type name.
+        if (isset($this->cacheByName[$typeName])) {
+            return $this->cacheByName[$typeName];
+        }
+        /*if (strpos($typeName, 'MyCLabsEnum_') === 0) {
             $className = str_replace('__', '\\', substr($typeName, 12));
 
             $type = $this->mapByClassName($className);
             if ($type !== null) {
                 return $type;
             }
-        }
+        }*/
 
         return $this->next->mapNameToType($typeName);
     }

src/SchemaFactory.php

@@ -334,7 +334,7 @@ public function createSchema(): Schema
 
         $errorRootTypeMapper = new FinalRootTypeMapper($recursiveTypeMapper);
         $rootTypeMapper = new BaseTypeMapper($errorRootTypeMapper, $recursiveTypeMapper, $topRootTypeMapper);
-        $rootTypeMapper = new MyCLabsEnumTypeMapper($rootTypeMapper);
+        $rootTypeMapper = new MyCLabsEnumTypeMapper($rootTypeMapper, $annotationReader);
 
         if (! empty($this->rootTypeMapperFactories)) {
             $rootSchemaFactoryContext = new RootTypeMapperFactoryContext(

src/Types/MyCLabsEnumType.php

@@ -7,7 +7,6 @@
 use GraphQL\Type\Definition\EnumType;
 use InvalidArgumentException;
 use MyCLabs\Enum\Enum;
-use function str_replace;
 
 /**
  * An extension of the EnumType to support Myclabs enum.
@@ -17,7 +16,7 @@
  */
 class MyCLabsEnumType extends EnumType
 {
-    public function __construct(string $enumClassName)
+    public function __construct(string $enumClassName, string $typeName)
     {
         $consts         = $enumClassName::toArray();
         $constInstances = [];
@@ -26,7 +25,7 @@ public function __construct(string $enumClassName)
         }
 
         parent::__construct([
-            'name' => 'MyCLabsEnum_' . str_replace('\\', '__', $enumClassName),
+            'name' => $typeName,
             'values' => $constInstances,
         ]);
     }

tests/AbstractQueryProviderTest.php

@@ -329,7 +329,7 @@ protected function buildRootTypeMapper(): RootTypeMapperInterface
 
         $errorRootTypeMapper = new FinalRootTypeMapper($this->getTypeMapper());
         $rootTypeMapper = new BaseTypeMapper($errorRootTypeMapper, $this->getTypeMapper(), $topRootTypeMapper);
-        $rootTypeMapper = new MyCLabsEnumTypeMapper($rootTypeMapper);
+        $rootTypeMapper = new MyCLabsEnumTypeMapper($rootTypeMapper, $this->getAnnotationReader());
         $rootTypeMapper = new CompoundTypeMapper($rootTypeMapper, $topRootTypeMapper, $this->getTypeRegistry(), $this->getTypeMapper());
         $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $topRootTypeMapper);
 

tests/Fixtures/Integration/Models/ProductTypeEnum.php

@@ -3,7 +3,11 @@
 namespace TheCodingMachine\GraphQLite\Fixtures\Integration\Models;
 
 use MyCLabs\Enum\Enum;
+use TheCodingMachine\GraphQLite\Annotations\EnumType;
 
+/**
+ * @EnumType(name="ProductTypes")
+ */
 class ProductTypeEnum extends Enum
 {
     const FOOD = 'food';

tests/Integration/EndToEndTest.php

@@ -226,7 +226,7 @@ public function createContainer(array $overloadedServices = []): ContainerInterf
             'rootTypeMapper' => function(ContainerInterface $container) {
                 $errorRootTypeMapper = new FinalRootTypeMapper($container->get(RecursiveTypeMapperInterface::class));
                 $rootTypeMapper = new BaseTypeMapper($errorRootTypeMapper, $container->get(RecursiveTypeMapperInterface::class), $container->get(RootTypeMapperInterface::class));
-                $rootTypeMapper = new MyCLabsEnumTypeMapper($rootTypeMapper);
+                $rootTypeMapper = new MyCLabsEnumTypeMapper($rootTypeMapper, $container->get(AnnotationReader::class));
                 $rootTypeMapper = new CompoundTypeMapper($rootTypeMapper, $container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class));
                 $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $container->get(RootTypeMapperInterface::class));
                 return $rootTypeMapper;

tests/Mappers/Root/MyCLabsEnumTypeMapperTest.php

@@ -13,7 +13,7 @@ class MyCLabsEnumTypeMapperTest extends AbstractQueryProviderTest
 
     public function testObjectTypeHint(): void
     {
-        $mapper = new MyCLabsEnumTypeMapper(new FinalRootTypeMapper($this->getTypeMapper()));
+        $mapper = new MyCLabsEnumTypeMapper(new FinalRootTypeMapper($this->getTypeMapper()), $this->getAnnotationReader());
 
         $this->expectException(CannotMapTypeException::class);
         $this->expectExceptionMessage("don't know how to handle type object");

tests/Types/MyclabsEnumTypeTest.php

@@ -10,7 +10,7 @@ class MyclabsEnumTypeTest extends TestCase
 {
     public function testException()
     {
-        $enumType = new MyCLabsEnumType(ProductTypeEnum::class);
+        $enumType = new MyCLabsEnumType(ProductTypeEnum::class, 'foo');
         $this->expectException(InvalidArgumentException::class);
         $this->expectExceptionMessage('Expected a Myclabs Enum instance');
         $enumType->serialize('foo');