From 3a99abc69ba36714e6b195459409e8a7ca83d376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20W=C3=B3js?= Date: Tue, 19 Oct 2021 15:20:06 +0200 Subject: [PATCH 1/5] IBX-1288: Skipped schema generation for invalid Content Type Group / Content Type and Field Definition (according to GraphQL spec) --- .../Domain/Content/ContentDomainIterator.php | 44 ++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/src/Schema/Domain/Content/ContentDomainIterator.php b/src/Schema/Domain/Content/ContentDomainIterator.php index 8a6b18fa..88618979 100644 --- a/src/Schema/Domain/Content/ContentDomainIterator.php +++ b/src/Schema/Domain/Content/ContentDomainIterator.php @@ -4,6 +4,7 @@ * @copyright Copyright (C) eZ Systems AS. All rights reserved. * @license For full copyright and license information view LICENSE file distributed with this source code. */ + namespace EzSystems\EzPlatformGraphQL\Schema\Domain\Content; use eZ\Publish\API\Repository\ContentTypeService; @@ -11,17 +12,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,13 +44,28 @@ 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]; @@ -48,4 +73,13 @@ public function iterate(): Generator } } } + + 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)); + } } From 9991d9e580da4cc051053ecab2432ef206c4e860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20W=C3=B3js?= Date: Fri, 22 Oct 2021 12:08:52 +0200 Subject: [PATCH 2/5] Explicitly configure logger dependency --- src/Resources/config/services/schema.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Resources/config/services/schema.yml b/src/Resources/config/services/schema.yml index fa1bca18..aa64aded 100644 --- 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' From 3ab7421756860c61a11fc89cceb4b9e725d8d0d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20W=C3=B3js?= Date: Fri, 22 Oct 2021 15:40:34 +0200 Subject: [PATCH 3/5] Fixed code style issues --- src/Schema/Domain/Content/ContentDomainIterator.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Schema/Domain/Content/ContentDomainIterator.php b/src/Schema/Domain/Content/ContentDomainIterator.php index 88618979..b71464d4 100644 --- a/src/Schema/Domain/Content/ContentDomainIterator.php +++ b/src/Schema/Domain/Content/ContentDomainIterator.php @@ -4,7 +4,6 @@ * @copyright Copyright (C) eZ Systems AS. All rights reserved. * @license For full copyright and license information view LICENSE file distributed with this source code. */ - namespace EzSystems\EzPlatformGraphQL\Schema\Domain\Content; use eZ\Publish\API\Repository\ContentTypeService; From 04311211c6fd8ed8618446883cc014852ec62521 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20W=C3=B3js?= Date: Mon, 25 Oct 2021 11:15:59 +0200 Subject: [PATCH 4/5] Updated unit tests --- composer.json | 3 +- .../Content/ContentDomainIteratorSpec.php | 50 +++++++++++++------ .../Schema/Domain/NameValidatorSpec.php | 23 +++++++++ src/Schema/Domain/NameValidator.php | 2 +- 4 files changed, 60 insertions(+), 18 deletions(-) create mode 100644 spec/Ibexa/GraphQL/Schema/Domain/NameValidatorSpec.php diff --git a/composer.json b/composer.json index cdbe605c..b5f73b08 100644 --- 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", + "sepc\\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 index cfe63c50..35d600dc 100644 --- a/spec/EzSystems/EzPlatformGraphQL/Schema/Domain/Content/ContentDomainIteratorSpec.php +++ b/spec/EzSystems/EzPlatformGraphQL/Schema/Domain/Content/ContentDomainIteratorSpec.php @@ -1,6 +1,7 @@ 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 new file mode 100644 index 00000000..705af7bd --- /dev/null +++ b/spec/Ibexa/GraphQL/Schema/Domain/NameValidatorSpec.php @@ -0,0 +1,23 @@ +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/Schema/Domain/NameValidator.php b/src/Schema/Domain/NameValidator.php index 4b9070a7..a189b390 100644 --- 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]*$/'; From 513c6a8892b9deb9f02fc417dbd7e46591934740 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20W=C3=B3js?= Date: Mon, 25 Oct 2021 11:36:05 +0200 Subject: [PATCH 5/5] Update composer.json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paweł Niedzielski --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index b5f73b08..283c20a5 100644 --- a/composer.json +++ b/composer.json @@ -36,7 +36,7 @@ "psr-4": { "spec\\EzSystems\\EzPlatformGraphQL\\Tools\\": "spec/Tools", "spec\\EzSystems\\EzPlatformGraphQL\\": "spec/EzSystems/EzPlatformGraphQL", - "sepc\\Ibexa\\GraphQL\\": "spec/Ibexa/GraphQL" + "spec\\Ibexa\\GraphQL\\": "spec/Ibexa/GraphQL" } }, "extra": {