Merge pull request #123 from ezsystems/ibx-2155-handle-invalid-enum-v…

…alues IBX-2155: Handled invalid enum values
ezsystems · Feb 23, 2022 · 32b02a8 · 32b02a8
2 parents df004cd + 216549e
commit 32b02a8
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 35 deletions.
diff --git a/spec/EzSystems/EzPlatformGraphQL/Schema/Domain/Content/ContentDomainIteratorSpec.php b/spec/EzSystems/EzPlatformGraphQL/Schema/Domain/Content/ContentDomainIteratorSpec.php
@@ -1,6 +1,7 @@
 <?php
 namespace spec\EzSystems\EzPlatformGraphQL\Schema\Domain\Content;
 
+use Ibexa\GraphQL\Schema\Domain\NameValidator;
 use spec\EzSystems\EzPlatformGraphQL\Tools\TypeArgument;
 use eZ\Publish\API\Repository\ContentTypeService;
 use eZ\Publish\Core\Repository\Values\ContentType\ContentType;
@@ -13,9 +14,11 @@
 
 class ContentDomainIteratorSpec extends ObjectBehavior
 {
-    public function let(ContentTypeService $contentTypeService)
-    {
-        $this->beConstructedWith($contentTypeService);
+    public function let(
+        ContentTypeService $contentTypeService,
+        NameValidator $nameValidator
+    ) {
+        $this->beConstructedWith($contentTypeService, $nameValidator);
     }
 
     function it_is_initializable()
@@ -54,8 +57,11 @@ function it_yields_content_type_groups(ContentTypeService $contentTypeService)
     }
 
     function it_yields_content_types_with_their_group_from_a_content_type_group(
-        ContentTypeService $contentTypeService
+        ContentTypeService $contentTypeService,
+        NameValidator $nameValidator
     ) {
+        $nameValidator->isValidName(Argument::any())->willReturn(true);
+
         $contentTypeService->loadContentTypeGroups()->willReturn([
             $group = new ContentTypeGroup(['identifier' => 'Group']),
         ]);
@@ -76,8 +82,11 @@ function it_yields_content_types_with_their_group_from_a_content_type_group(
     }
 
     function it_yields_fields_definitions_with_their_content_types_and_group_from_a_content_type(
-        ContentTypeService $contentTypeService
+        ContentTypeService $contentTypeService,
+        NameValidator $nameValidator
     ) {
+        $nameValidator->isValidName(Argument::any())->willReturn(true);
+
         $contentTypeService->loadContentTypeGroups()->willReturn([
             $group = new ContentTypeGroup(['identifier' => 'Group']),
         ]);
@@ -104,8 +113,11 @@ function it_yields_fields_definitions_with_their_content_types_and_group_from_a_
     }
 
     function it_only_yields_fields_definitions_from_the_current_content_type(
-        ContentTypeService $contentTypeService
+        ContentTypeService $contentTypeService,
+        NameValidator $nameValidator
     ) {
+        $nameValidator->isValidName(Argument::any())->willReturn(true);
+
         $contentTypeService->loadContentTypeGroups()->willReturn([
             $group = new ContentTypeGroup([
                 'identifier' => 'group'

diff --git a/src/Resources/config/services/schema.yml b/src/Resources/config/services/schema.yml
@@ -19,11 +19,7 @@ services:
 
     EzSystems\EzPlatformGraphQL\Schema\Builder\SchemaBuilder:
         arguments:
-            - '@Ibexa\GraphQL\Schema\Domain\NameValidator'
-        calls:
-            - method: setLogger
-              arguments:
-                  - '@logger'
+            $nameValidator: '@Ibexa\GraphQL\Schema\Domain\NameValidator'
 
     EzSystems\EzPlatformGraphQL\Schema\Domain\Content\Mapper\FieldDefinition\FieldDefinitionMapper:
         alias: EzSystems\EzPlatformGraphQL\Schema\Domain\Content\Mapper\FieldDefinition\DefaultFieldDefinitionMapper
@@ -87,9 +83,15 @@ services:
 
     EzSystems\EzPlatformGraphQL\Schema\Domain\Content\NameHelper: ~
 
-    Ibexa\GraphQL\Schema\Domain\NameValidator: ~
+    Ibexa\GraphQL\Schema\Domain\NameValidator:
+        calls:
+            - method: setLogger
+              arguments:
+                  - '@logger'
 
-    EzSystems\EzPlatformGraphQL\Schema\Domain\ImageVariationDomain: ~
+    EzSystems\EzPlatformGraphQL\Schema\Domain\ImageVariationDomain:
+        arguments:
+            $nameValidator: '@Ibexa\GraphQL\Schema\Domain\NameValidator'
 
     EzSystems\EzPlatformGraphQL\Schema\Domain\Content\ContentDomainIterator: ~
 

diff --git a/src/Schema/Builder/SchemaBuilder.php b/src/Schema/Builder/SchemaBuilder.php
@@ -8,14 +8,9 @@
 
 use EzSystems\EzPlatformGraphQL\Schema\Builder as SchemaBuilderInterface;
 use Ibexa\GraphQL\Schema\Domain\NameValidator;
-use Psr\Log\LoggerAwareInterface;
-use Psr\Log\LoggerAwareTrait;
-use Psr\Log\NullLogger;
 
-class SchemaBuilder implements SchemaBuilderInterface, LoggerAwareInterface
+class SchemaBuilder implements SchemaBuilderInterface
 {
-    use LoggerAwareTrait;
-
     private $schema = [];
 
     /** @var \Ibexa\GraphQL\Schema\Domain\NameValidator */
@@ -24,7 +19,6 @@ class SchemaBuilder implements SchemaBuilderInterface, LoggerAwareInterface
     public function __construct(NameValidator $nameValidator)
     {
         $this->nameValidator = $nameValidator;
-        $this->logger = new NullLogger();
     }
 
     public function getSchema(): array
@@ -35,7 +29,7 @@ public function getSchema(): array
     public function addType(Input\Type $typeInput)
     {
         if (!$this->nameValidator->isValidName($typeInput->name)) {
-            $this->generateInvalidGraphQLNameWarning($typeInput->type, $typeInput->name);
+            $this->nameValidator->generateInvalidNameWarning($typeInput->type, $typeInput->name);
 
             return;
         }
@@ -65,7 +59,7 @@ public function addType(Input\Type $typeInput)
     public function addFieldToType($type, Input\Field $fieldInput)
     {
         if (!$this->nameValidator->isValidName($fieldInput->name)) {
-            $this->generateInvalidGraphQLNameWarning($fieldInput->type, $fieldInput->name);
+            $this->nameValidator->generateInvalidNameWarning($fieldInput->type, $fieldInput->name);
 
             return;
         }
@@ -177,12 +171,4 @@ public function hasEnum($enum): bool
     {
         return $this->hasType($enum);
     }
-
-    private function generateInvalidGraphQLNameWarning(string $type, string $name): void
-    {
-        $message = "Skipping schema generation for %s with identifier '%s' as it stands against GraphQL specification. "
-            . 'For more details see http://spec.graphql.org/[latest-release]/#sec-Names.';
-
-        $this->logger->warning(sprintf($message, $type, $name));
-    }
 }
diff --git a/src/Schema/Domain/Content/ContentDomainIterator.php b/src/Schema/Domain/Content/ContentDomainIterator.php
@@ -11,15 +11,22 @@
 use EzSystems\EzPlatformGraphQL\Schema\Builder\Input;
 use EzSystems\EzPlatformGraphQL\Schema\Domain\Iterator;
 use Generator;
+use Ibexa\GraphQL\Schema\Domain\NameValidator;
 
 class ContentDomainIterator implements Iterator
 {
     /** @var \eZ\Publish\API\Repository\ContentTypeService */
     private $contentTypeService;
 
-    public function __construct(ContentTypeService $contentTypeService)
-    {
+    /** @var \Ibexa\GraphQL\Schema\Domain\NameValidator */
+    private $nameValidator;
+
+    public function __construct(
+        ContentTypeService $contentTypeService,
+        NameValidator $nameValidator
+    ) {
         $this->contentTypeService = $contentTypeService;
+        $this->nameValidator = $nameValidator;
     }
 
     public function init(Builder $schema)
@@ -35,6 +42,12 @@ public function iterate(): Generator
             yield ['ContentTypeGroup' => $contentTypeGroup];
 
             foreach ($this->contentTypeService->loadContentTypes($contentTypeGroup) as $contentType) {
+                if (!$this->nameValidator->isValidName($contentType->identifier)) {
+                    $this->nameValidator->generateInvalidNameWarning('Content Type', $contentType->identifier);
+
+                    continue;
+                }
+
                 yield ['ContentTypeGroup' => $contentTypeGroup]
                     + ['ContentType' => $contentType];
 

diff --git a/src/Schema/Domain/ImageVariationDomain.php b/src/Schema/Domain/ImageVariationDomain.php
@@ -11,6 +11,7 @@
 use EzSystems\EzPlatformGraphQL\Schema\Builder;
 use EzSystems\EzPlatformGraphQL\Schema\Domain;
 use Generator;
+use Ibexa\GraphQL\Schema\Domain\NameValidator;
 
 /**
  * Adds configured image variations to the ImageVariationIdentifier type.
@@ -23,14 +24,26 @@ class ImageVariationDomain implements Domain\Iterator, Schema\Worker
     /** @var \eZ\Publish\Core\MVC\ConfigResolverInterface */
     private $configResolver;
 
-    public function __construct(ConfigResolverInterface $configResolver)
-    {
+    /** @var \Ibexa\GraphQL\Schema\Domain\NameValidator */
+    private $nameValidator;
+
+    public function __construct(
+        ConfigResolverInterface $configResolver,
+        NameValidator $nameValidator
+    ) {
         $this->configResolver = $configResolver;
+        $this->nameValidator = $nameValidator;
     }
 
     public function iterate(): Generator
     {
         foreach ($this->configResolver->getParameter('image_variations') as $identifier => $variation) {
+            if (!$this->nameValidator->isValidName($identifier)) {
+                $this->nameValidator->generateInvalidNameWarning('Image Variation', $identifier);
+
+                continue;
+            }
+
             yield [self::ARG => ['identifier' => $identifier, 'variation' => $variation]];
         }
     }

diff --git a/src/Schema/Domain/NameValidator.php b/src/Schema/Domain/NameValidator.php
@@ -8,15 +8,34 @@
 
 namespace Ibexa\GraphQL\Schema\Domain;
 
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerAwareTrait;
+use Psr\Log\NullLogger;
+
 /**
  * Validates given name according to GraphQL specification. See http://spec.graphql.org/June2018/#sec-Names.
  */
-class NameValidator
+class NameValidator implements LoggerAwareInterface
 {
+    use LoggerAwareTrait;
+
     private const NAME_PATTERN = '/^[_a-zA-Z][_a-zA-Z0-9]*$/';
 
+    public function __construct()
+    {
+        $this->logger = new NullLogger();
+    }
+
     public function isValidName(string $name): bool
     {
         return preg_match(self::NAME_PATTERN, $name) === 1;
     }
+
+    public function generateInvalidNameWarning(string $type, string $name): void
+    {
+        $message = "Skipping schema generation for %s with identifier '%s' as it stands against GraphQL specification. "
+            . 'For more details see http://spec.graphql.org/[latest-release]/#sec-Names.';
+
+        $this->logger->warning(sprintf($message, $type, $name));
+    }
 }