IBX-1288: Skipped schema generation for invalid Content Type Group / …

…Content Type and Field Definition (#109) Co-authored-by: Paweł Niedzielski <[email protected]>
ezsystems · Oct 25, 2021 · 8e8c8c5 · 8e8c8c5
1 parent 0ac91cc
commit 8e8c8c5
Show file tree

Hide file tree

Showing 6 changed files with 104 additions and 23 deletions.
diff --git a/composer.json b/composer.json
@@ -35,7 +35,8 @@
     "autoload-dev": {
         "psr-4": {
             "spec\\EzSystems\\EzPlatformGraphQL\\Tools\\": "spec/Tools",
-            "spec\\EzSystems\\EzPlatformGraphQL\\": "spec/EzSystems/EzPlatformGraphQL"
+            "spec\\EzSystems\\EzPlatformGraphQL\\": "spec/EzSystems/EzPlatformGraphQL",
+            "spec\\Ibexa\\GraphQL\\": "spec/Ibexa/GraphQL"
         }
     },
     "extra": {

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;
@@ -14,9 +15,10 @@
 class ContentDomainIteratorSpec extends ObjectBehavior
 {
     public function let(
-        ContentTypeService $contentTypeService
+        ContentTypeService $contentTypeService,
+        NameValidator $nameValidator
     ) {
-        $this->beConstructedWith($contentTypeService);
+        $this->beConstructedWith($contentTypeService, $nameValidator);
     }
 
     function it_is_initializable()
@@ -36,8 +38,10 @@ function it_initializes_the_schema_with_the_Platform_root_type(Builder $schema)
         )->shouldHaveBeenCalled();
     }
 
-    function it_yields_content_type_groups(ContentTypeService $contentTypeService)
+    function it_yields_content_type_groups(ContentTypeService $contentTypeService, NameValidator $nameValidator)
     {
+        $nameValidator->isValidName(Argument::any())->willReturn(true);
+
         $contentTypeService->loadContentTypeGroups()->willReturn([
             $group1 = new ContentTypeGroup(['identifier' => 'Group 1']),
             $group2 = new ContentTypeGroup(['identifier' => 'Group 2']),
@@ -54,8 +58,12 @@ function it_yields_content_type_groups(ContentTypeService $contentTypeService)
         );
     }
 
-    function it_yields_content_types_with_their_group_from_a_content_type_group(ContentTypeService $contentTypeService)
-    {
+    function it_yields_content_types_with_their_group_from_a_content_type_group(
+        ContentTypeService $contentTypeService,
+        NameValidator $nameValidator
+    ) {
+        $nameValidator->isValidName(Argument::any())->willReturn(true);
+
         $contentTypeService->loadContentTypeGroups()->willReturn([
             $group = new ContentTypeGroup(['identifier' => 'Group']),
         ]);
@@ -75,18 +83,22 @@ function it_yields_content_types_with_their_group_from_a_content_type_group(Cont
         );
     }
 
-    function it_yields_fields_definitions_with_their_content_types_and_group_from_a_content_type(ContentTypeService $contentTypeService)
-    {
+    function it_yields_fields_definitions_with_their_content_types_and_group_from_a_content_type(
+        ContentTypeService $contentTypeService,
+        NameValidator $nameValidator
+    ) {
+        $nameValidator->isValidName(Argument::any())->willReturn(true);
+
         $contentTypeService->loadContentTypeGroups()->willReturn([
-            $group = new ContentTypeGroup(),
+            $group = new ContentTypeGroup(['identifier' => 'Group']),
         ]);
         $contentTypeService->loadContentTypes(Argument::any())->willReturn([
             $type = new ContentType([
                 'identifier' => 'type',
                 'fieldDefinitions' => [
-                    'field1' => $field1 = new FieldDefinition(),
-                    'field2' => $field2 = new FieldDefinition(),
-                    'field3' => $field3 = new FieldDefinition(),
+                    'field1' => $field1 = new FieldDefinition(['identifier' => 'foo']),
+                    'field2' => $field2 = new FieldDefinition(['identifier' => 'bar']),
+                    'field3' => $field3 = new FieldDefinition(['identifier' => 'faz']),
                 ]
             ]),
         ]);
@@ -102,23 +114,29 @@ 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)
-    {
+    function it_only_yields_fields_definitions_from_the_current_content_type(
+        ContentTypeService $contentTypeService,
+        NameValidator $nameValidator
+    ) {
+        $nameValidator->isValidName(Argument::any())->willReturn(true);
+
         $contentTypeService->loadContentTypeGroups()->willReturn([
-            $group = new ContentTypeGroup(),
+            $group = new ContentTypeGroup([
+                'identifier' => 'group'
+            ]),
         ]);
 
         $contentTypeService->loadContentTypes(Argument::any())->willReturn([
             $type1 = new ContentType([
                 'identifier' => 'type1',
                 'fieldDefinitions' => [
-                    'type1_field1' => ($type1field1 = new FieldDefinition()),
+                    'type1_field1' => ($type1field1 = new FieldDefinition(['identifier' => 'foo'])),
                 ]
             ]),
             $type2 = new ContentType([
                 'identifier' => 'type2',
                 'fieldDefinitions' => [
-                    'type2_field1' => ($type2field1 = new FieldDefinition()),
+                    'type2_field1' => ($type2field1 = new FieldDefinition(['identifier' => 'bar'])),
                 ]
             ]),
         ]);

diff --git a/spec/Ibexa/GraphQL/Schema/Domain/NameValidatorSpec.php b/spec/Ibexa/GraphQL/Schema/Domain/NameValidatorSpec.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace spec\Ibexa\GraphQL\Schema\Domain;
+
+use Ibexa\GraphQL\Schema\Domain\NameValidator;
+use PhpSpec\ObjectBehavior;
+
+final class NameValidatorSpec extends ObjectBehavior
+{
+    function it_is_initializable()
+    {
+        $this->shouldHaveType(NameValidator::class);
+    }
+
+    function it_validates_names()
+    {
+        $this->isValidName('777')->shouldBe(false);
+        $this->isValidName('foo')->shouldBe(true);
+        $this->isValidName('foo_213')->shouldBe(true);
+    }
+}
diff --git a/src/Resources/config/services/schema.yml b/src/Resources/config/services/schema.yml
@@ -57,3 +57,9 @@ services:
             - method: setLogger
               arguments:
                   - '@logger'
+
+    EzSystems\EzPlatformGraphQL\Schema\Domain\Content\ContentDomainIterator:
+        calls:
+            - method: setLogger
+              arguments:
+                  - '@logger'
diff --git a/src/Schema/Domain/Content/ContentDomainIterator.php b/src/Schema/Domain/Content/ContentDomainIterator.php
@@ -11,17 +11,26 @@
 use EzSystems\EzPlatformGraphQL\Schema\Builder\Input;
 use EzSystems\EzPlatformGraphQL\Schema\Domain\Iterator;
 use Generator;
+use Ibexa\GraphQL\Schema\Domain\NameValidator;
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerAwareTrait;
+use Psr\Log\NullLogger;
 
-class ContentDomainIterator implements Iterator
+class ContentDomainIterator implements Iterator, LoggerAwareInterface
 {
-    /**
-     * @var ContentTypeService
-     */
+    use LoggerAwareTrait;
+
+    /** @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;
+        $this->logger = new NullLogger();
     }
 
     public function init(Builder $schema)
@@ -34,18 +43,42 @@ public function init(Builder $schema)
     public function iterate(): Generator
     {
         foreach ($this->contentTypeService->loadContentTypeGroups() as $contentTypeGroup) {
+            if (!$this->nameValidator->isValidName($contentTypeGroup->identifier)) {
+                $this->generateInvalidGraphQLNameWarning('Content Type Group', $contentTypeGroup->identifier);
+                continue;
+            }
+
             yield ['ContentTypeGroup' => $contentTypeGroup];
 
             foreach ($this->contentTypeService->loadContentTypes($contentTypeGroup) as $contentType) {
+                if (!$this->nameValidator->isValidName($contentType->identifier)) {
+                    $this->generateInvalidGraphQLNameWarning('Content Type', $contentType->identifier);
+                    continue;
+                }
+
                 yield ['ContentTypeGroup' => $contentTypeGroup]
                     + ['ContentType' => $contentType];
 
                 foreach ($contentType->getFieldDefinitions() as $fieldDefinition) {
+                    if (!$this->nameValidator->isValidName($fieldDefinition->identifier)) {
+                        $this->generateInvalidGraphQLNameWarning('Field Definition', $fieldDefinition->identifier);
+                        continue;
+                    }
+
                     yield ['ContentTypeGroup' => $contentTypeGroup]
                         + ['ContentType' => $contentType]
                         + ['FieldDefinition' => $fieldDefinition];
                 }
             }
         }
     }
+
+    private function generateInvalidGraphQLNameWarning(string $type, string $name): void
+    {
+        $message = "Skipped schema generation for %s with identifier '%s'. "
+            . 'Please rename given %s according to GraphQL specification '
+            . '(http://spec.graphql.org/June2018/#sec-Names)';
+
+        $this->logger->warning(sprintf($message, $type, $name, $type));
+    }
 }
diff --git a/src/Schema/Domain/NameValidator.php b/src/Schema/Domain/NameValidator.php
@@ -11,7 +11,7 @@
 /**
  * Validates given name according to GraphQL specification. See http://spec.graphql.org/June2018/#sec-Names.
  */
-final class NameValidator
+class NameValidator
 {
     private const NAME_PATTERN = '/^[_a-zA-Z][_a-zA-Z0-9]*$/';