Merge IBX-1565: Refactor schema's types' names validation

spec/Schema/Builder/SchemaBuilderSpec.php

@@ -1,29 +1,42 @@
 <?php
 
+/**
+ * @copyright Copyright (C) Ibexa AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
 namespace Ibexa\Spec\GraphQL\Schema\Builder;
 
 use Ibexa\GraphQL\Schema\Builder\Input;
 use Ibexa\GraphQL\Schema\Builder\SchemaBuilder;
+use Ibexa\GraphQL\Schema\Domain\NameValidator;
 use PhpSpec\ObjectBehavior;
+use Prophecy\Argument;
 
 class SchemaBuilderSpec extends ObjectBehavior
 {
-    const TYPE = 'Test';
-    const TYPE_TYPE = 'object';
+    public const TYPE = 'Test';
+    public const TYPE_TYPE = 'object';
 
-    const FIELD = 'field';
-    const FIELD_TYPE = 'string';
+    public const FIELD = 'field';
+    public const FIELD_TYPE = 'string';
 
-    const ARG = 'arg';
-    const ARG_TYPE = 'Boolean';
+    public const ARG = 'arg';
+    public const ARG_TYPE = 'Boolean';
 
-    function it_is_initializable()
+    public function let(NameValidator $nameValidator)
+    {
+        $this->beConstructedWith($nameValidator);
+    }
+
+    public function it_is_initializable()
     {
         $this->shouldHaveType(SchemaBuilder::class);
     }
 
-    function it_adds_a_type_to_the_schema()
+    public function it_adds_a_type_to_the_schema(NameValidator $nameValidator)
     {
+        $nameValidator->isValidName(Argument::any())->willReturn(true);
+
         $this->addType($this->inputType('Parent', 'Interface'));
 
         $schema = $this->getSchema();
@@ -33,10 +46,13 @@ function it_adds_a_type_to_the_schema()
         $schema->shouldHaveGraphQLTypeThatImplements('Interface');
     }
 
-    function it_adds_a_field_to_an_existing_type()
+    public function it_adds_a_field_to_an_existing_type(NameValidator $nameValidator)
     {
+        $nameValidator->isValidName(Argument::any())->willReturn(true);
+
         $this->addType($this->inputType());
-        $this->addFieldToType(self::TYPE,
+        $this->addFieldToType(
+            self::TYPE,
             $this->inputField('Description', '@=resolver("myresolver")')
         );
 
@@ -47,8 +63,10 @@ function it_adds_a_field_to_an_existing_type()
         $schema->shouldHaveGraphQLTypeFieldWithResolve('@=resolver("myresolver")');
     }
 
-    function it_adds_an_argument_to_an_existing_type_field()
+    public function it_adds_an_argument_to_an_existing_type_field(NameValidator $nameValidator)
     {
+        $nameValidator->isValidName(Argument::any())->willReturn(true);
+
         $this->addType($this->inputType());
         $this->addFieldToType(self::TYPE, $this->inputField());
         $this->addArgToField(self::TYPE, self::FIELD, $this->inputArg('Description'));
@@ -63,42 +81,42 @@ function it_adds_an_argument_to_an_existing_type_field()
     public function getMatchers(): array
     {
         return [
-            'haveGraphQLType' => function (array $schema) {
+            'haveGraphQLType' => static function (array $schema) {
                 return
                     isset($schema[self::TYPE]['type'])
                     && $schema[self::TYPE]['type'] === self::TYPE_TYPE;
             },
-            'haveGraphQLTypeThatInherits' => function (array $schema, $inherits) {
+            'haveGraphQLTypeThatInherits' => static function (array $schema, $inherits) {
                 return
                     isset($schema[self::TYPE]['inherits'])
                     && in_array($inherits, $schema[self::TYPE]['inherits']);
             },
-            'haveGraphQLTypeThatImplements' => function (array $schema, $interface) {
+            'haveGraphQLTypeThatImplements' => static function (array $schema, $interface) {
                 return
                     isset($schema[self::TYPE]['config']['interfaces'])
                     && in_array($interface, $schema[self::TYPE]['config']['interfaces']);
             },
-            'haveGraphQLTypeField' => function (array $schema) {
+            'haveGraphQLTypeField' => static function (array $schema) {
                 return
                     isset($schema[self::TYPE]['config']['fields'][self::FIELD]['type'])
                     && $schema[self::TYPE]['config']['fields'][self::FIELD]['type'] === self::FIELD_TYPE;
             },
-            'haveGraphQLTypeFieldWithDescription' => function (array $schema, $description) {
+            'haveGraphQLTypeFieldWithDescription' => static function (array $schema, $description) {
                 return
                     isset($schema[self::TYPE]['config']['fields'][self::FIELD]['description'])
                     && $schema[self::TYPE]['config']['fields'][self::FIELD]['description'] === $description;
             },
-            'haveGraphQLTypeFieldWithResolve' => function (array $schema, $resolve) {
+            'haveGraphQLTypeFieldWithResolve' => static function (array $schema, $resolve) {
                 return
                     isset($schema[self::TYPE]['config']['fields'][self::FIELD]['description'])
                     && $schema[self::TYPE]['config']['fields'][self::FIELD]['resolve'] === $resolve;
             },
-            'haveGraphQLTypeFieldArg' => function (array $schema) {
+            'haveGraphQLTypeFieldArg' => static function (array $schema) {
                 return
                     isset($schema[self::TYPE]['config']['fields'][self::FIELD]['args'][self::ARG]['type'])
                     && $schema[self::TYPE]['config']['fields'][self::FIELD]['args'][self::ARG]['type'] === self::ARG_TYPE;
             },
-            'haveGraphQLTypeFieldArgWithDescription' => function (array $schema, $description) {
+            'haveGraphQLTypeFieldArgWithDescription' => static function (array $schema, $description) {
                 return
                     isset($schema[self::TYPE]['config']['fields'][self::FIELD]['args'][self::ARG]['description'])
                     && $schema[self::TYPE]['config']['fields'][self::FIELD]['args'][self::ARG]['description'] === $description;
@@ -109,10 +127,11 @@ public function getMatchers(): array
     protected function inputType($inherits = [], $interfaces = []): Input\Type
     {
         return new Input\Type(
-            self::TYPE, self::TYPE_TYPE,
+            self::TYPE,
+            self::TYPE_TYPE,
             [
                 'inherits' => $inherits,
-                'interfaces' => $interfaces
+                'interfaces' => $interfaces,
             ]
         );
     }

spec/Schema/Domain/Content/ContentDomainIteratorSpec.php

@@ -1,33 +1,35 @@
 <?php
+
+/**
+ * @copyright Copyright (C) Ibexa AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
 namespace Ibexa\Spec\GraphQL\Schema\Domain\Content;
 
 use Ibexa\Contracts\Core\Repository\ContentTypeService;
-use Ibexa\Core\Repository\Values\ContentType\FieldDefinitionCollection;
 use Ibexa\Core\Repository\Values\ContentType\ContentType;
 use Ibexa\Core\Repository\Values\ContentType\ContentTypeGroup;
 use Ibexa\Core\Repository\Values\ContentType\FieldDefinition;
-use PhpSpec\ObjectBehavior;
-use Prophecy\Argument;
-use Ibexa\GraphQL\Schema\Domain\NameValidator;
-use Ibexa\GraphQL\Schema\Domain;
+use Ibexa\Core\Repository\Values\ContentType\FieldDefinitionCollection;
 use Ibexa\GraphQL\Schema\Builder;
+use Ibexa\GraphQL\Schema\Domain;
 use Ibexa\Spec\GraphQL\Tools\TypeArgument;
+use PhpSpec\ObjectBehavior;
+use Prophecy\Argument;
 
 class ContentDomainIteratorSpec extends ObjectBehavior
 {
-    public function let(
-        ContentTypeService $contentTypeService,
-        NameValidator $nameValidator
-    ) {
-        $this->beConstructedWith($contentTypeService, $nameValidator);
+    public function let(ContentTypeService $contentTypeService)
+    {
+        $this->beConstructedWith($contentTypeService);
     }
 
-    function it_is_initializable()
+    public function it_is_initializable()
     {
         $this->shouldHaveType(Domain\Iterator::class);
     }
 
-    function it_initializes_the_schema_with_the_Platform_root_type(Builder $schema)
+    public function it_initializes_the_schema_with_the_Platform_root_type(Builder $schema)
     {
         $this->init($schema);
 
@@ -39,10 +41,8 @@ function it_initializes_the_schema_with_the_Platform_root_type(Builder $schema)
         )->shouldHaveBeenCalled();
     }
 
-    function it_yields_content_type_groups(ContentTypeService $contentTypeService, NameValidator $nameValidator)
+    public function it_yields_content_type_groups(ContentTypeService $contentTypeService)
     {
-        $nameValidator->isValidName(Argument::any())->willReturn(true);
-
         $contentTypeService->loadContentTypeGroups()->willReturn([
             $group1 = new ContentTypeGroup(['identifier' => 'Group 1']),
             $group2 = new ContentTypeGroup(['identifier' => 'Group 2']),
@@ -59,12 +59,9 @@ function it_yields_content_type_groups(ContentTypeService $contentTypeService, N
         );
     }
 
-    function it_yields_content_types_with_their_group_from_a_content_type_group(
-        ContentTypeService $contentTypeService,
-        NameValidator $nameValidator
+    public function it_yields_content_types_with_their_group_from_a_content_type_group(
+        ContentTypeService $contentTypeService
     ) {
-        $nameValidator->isValidName(Argument::any())->willReturn(true);
-
         $contentTypeService->loadContentTypeGroups()->willReturn([
             $group = new ContentTypeGroup(['identifier' => 'Group']),
         ]);
@@ -84,12 +81,9 @@ 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,
-        NameValidator $nameValidator
+    public function it_yields_fields_definitions_with_their_content_types_and_group_from_a_content_type(
+        ContentTypeService $contentTypeService
     ) {
-        $nameValidator->isValidName(Argument::any())->willReturn(true);
-
         $contentTypeService->loadContentTypeGroups()->willReturn([
             $group = new ContentTypeGroup(['identifier' => 'Group']),
         ]);
@@ -100,7 +94,7 @@ function it_yields_fields_definitions_with_their_content_types_and_group_from_a_
                     'field1' => $field1 = new FieldDefinition(['identifier' => 'foo']),
                     'field2' => $field2 = new FieldDefinition(['identifier' => 'bar']),
                     'field3' => $field3 = new FieldDefinition(['identifier' => 'faz']),
-                ])
+                ]),
             ]),
         ]);
 
@@ -115,15 +109,12 @@ 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,
-        NameValidator $nameValidator
+    public function it_only_yields_fields_definitions_from_the_current_content_type(
+        ContentTypeService $contentTypeService
     ) {
-        $nameValidator->isValidName(Argument::any())->willReturn(true);
-
         $contentTypeService->loadContentTypeGroups()->willReturn([
             $group = new ContentTypeGroup([
-                'identifier' => 'group'
+                'identifier' => 'group',
             ]),
         ]);
 
@@ -132,13 +123,13 @@ function it_only_yields_fields_definitions_from_the_current_content_type(
                 'identifier' => 'type1',
                 'fieldDefinitions' => new FieldDefinitionCollection([
                     'type1_field1' => ($type1field1 = new FieldDefinition(['identifier' => 'foo'])),
-                ])
+                ]),
             ]),
             $type2 = new ContentType([
                 'identifier' => 'type2',
                 'fieldDefinitions' => new FieldDefinitionCollection([
                     'type2_field1' => ($type2field1 = new FieldDefinition(['identifier' => 'bar'])),
-                ])
+                ]),
             ]),
         ]);
 

src/bundle/Resources/config/services/schema.yaml

@@ -20,7 +20,13 @@ services:
     Ibexa\Contracts\GraphQL\Schema\Domain\Content\Mapper\FieldDefinition\FieldDefinitionMapper:
         alias: Ibexa\GraphQL\Schema\Domain\Content\Mapper\FieldDefinition\DefaultFieldDefinitionMapper
 
-    Ibexa\GraphQL\Schema\Builder\SchemaBuilder: ~
+    Ibexa\GraphQL\Schema\Builder\SchemaBuilder:
+        arguments:
+            - '@Ibexa\GraphQL\Schema\Domain\NameValidator'
+        calls:
+            - method: setLogger
+              arguments:
+                  - '@logger'
 
     Ibexa\GraphQL\Schema\Domain\Content\Mapper\FieldDefinition\DefaultFieldDefinitionMapper: ~
 

src/lib/Schema/Builder/SchemaBuilder.php

@@ -7,18 +7,39 @@
 namespace Ibexa\GraphQL\Schema\Builder;
 
 use Ibexa\GraphQL\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
+class SchemaBuilder implements SchemaBuilderInterface, LoggerAwareInterface
 {
+    use LoggerAwareTrait;
+
     private $schema = [];
 
+    /** @var \Ibexa\GraphQL\Schema\Domain\NameValidator */
+    private $nameValidator;
+
+    public function __construct(NameValidator $nameValidator)
+    {
+        $this->nameValidator = $nameValidator;
+        $this->logger = new NullLogger();
+    }
+
     public function getSchema(): array
     {
         return $this->schema;
     }
 
     public function addType(Input\Type $typeInput)
     {
+        if (!$this->nameValidator->isValidName($typeInput->name)) {
+            $this->generateInvalidGraphQLNameWarning($typeInput->type, $typeInput->name);
+
+            return;
+        }
+
         if ($this->hasType($typeInput->name)) {
             throw new \Exception("The type $typeInput->name is already defined");
         }
@@ -43,6 +64,12 @@ 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);
+
+            return;
+        }
+
         if (!$this->hasType($type)) {
             throw new \Exception("Expected type $type to be defined, but it was not");
         }
@@ -150,6 +177,14 @@ 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));
+    }
 }
 
 class_alias(SchemaBuilder::class, 'EzSystems\EzPlatformGraphQL\Schema\Builder\SchemaBuilder');