From 1f29adb08ecb13dab2dc69b97dff33ca2ca00663 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Thu, 17 Feb 2022 12:54:09 +0100 Subject: [PATCH 1/2] IBX-2155: Handled invalid enum values --- src/Resources/config/services/schema.yml | 16 +++++++------- src/Schema/Builder/SchemaBuilder.php | 20 +++--------------- .../Domain/Content/ContentDomainIterator.php | 17 +++++++++++++-- src/Schema/Domain/ImageVariationDomain.php | 17 +++++++++++++-- src/Schema/Domain/NameValidator.php | 21 ++++++++++++++++++- 5 files changed, 62 insertions(+), 29 deletions(-) diff --git a/src/Resources/config/services/schema.yml b/src/Resources/config/services/schema.yml index 83a9d50..fb4eacf 100644 --- 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 index 6c48728..d571998 100644 --- 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 index 58debb3..4ad0d98 100644 --- 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 index de05e67..0e95b22 100644 --- 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 index a189b39..a43e652 100644 --- 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)); + } } From 216549eab5af3e56ccdbb01837c997bc10443775 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Thu, 17 Feb 2022 13:15:25 +0100 Subject: [PATCH 2/2] IBX-2155: Updated tests --- .../Content/ContentDomainIteratorSpec.php | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/spec/EzSystems/EzPlatformGraphQL/Schema/Domain/Content/ContentDomainIteratorSpec.php b/spec/EzSystems/EzPlatformGraphQL/Schema/Domain/Content/ContentDomainIteratorSpec.php index fc3d33d..c3d3aff 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); + 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'