diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index c4450a156d..8c8706af3f 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -11687,12 +11687,6 @@ parameters: count: 1 path: src/lib/Service/ContentTypeService.php - - - message: '#^Access to an undefined property Ibexa\\Contracts\\Core\\Repository\\Values\\ValueObject\:\:\$fieldDefinitions\.$#' - identifier: property.notFound - count: 1 - path: src/lib/Service/MetaFieldType/MetaFieldDefinitionService.php - - message: '#^Access to an undefined property Ibexa\\Contracts\\Core\\Repository\\Values\\User\\Policy\:\:\$originalId\.$#' identifier: property.notFound diff --git a/src/lib/Service/MetaFieldType/MetaFieldDefinitionService.php b/src/lib/Service/MetaFieldType/MetaFieldDefinitionService.php index 06a10178df..b27da7deee 100644 --- a/src/lib/Service/MetaFieldType/MetaFieldDefinitionService.php +++ b/src/lib/Service/MetaFieldType/MetaFieldDefinitionService.php @@ -11,6 +11,8 @@ use Ibexa\AdminUi\Config\AdminUiForms\ContentTypeFieldTypesResolverInterface; use Ibexa\Bundle\AdminUi\DependencyInjection\Configuration\Parser\AdminUiForms; use Ibexa\Contracts\Core\Repository\ContentTypeService; +use Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException; +use Ibexa\Contracts\Core\Repository\FieldTypeService; use Ibexa\Contracts\Core\Repository\LanguageService; use Ibexa\Contracts\Core\Repository\Values\Content\Language; use Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeCreateStruct; @@ -18,6 +20,7 @@ use Ibexa\Contracts\Core\Repository\Values\ContentType\FieldDefinitionCreateStruct; use Ibexa\Contracts\Core\Repository\Values\ValueObject; use Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface; +use Ibexa\Core\Base\Exceptions\NotFound\FieldTypeNotFoundException; use Ibexa\Core\Helper\FieldsGroups\FieldsGroupsList; use Ibexa\Core\MVC\Symfony\Locale\LocaleConverterInterface; use JMS\TranslationBundle\Annotation\Ignore; @@ -34,6 +37,8 @@ final class MetaFieldDefinitionService implements MetaFieldDefinitionServiceInte private ContentTypeService $contentTypeService; + private FieldTypeService $fieldTypeService; + private FieldsGroupsList $fieldsGroupsList; private LanguageService $languageService; @@ -46,6 +51,7 @@ public function __construct( ConfigResolverInterface $configResolver, ContentTypeFieldTypesResolverInterface $contentTypeFieldTypesResolver, ContentTypeService $contentTypeService, + FieldTypeService $fieldTypeService, FieldsGroupsList $fieldsGroupsList, LanguageService $languageService, LocaleConverterInterface $localeConverter, @@ -54,12 +60,16 @@ public function __construct( $this->configResolver = $configResolver; $this->contentTypeFieldTypesResolver = $contentTypeFieldTypesResolver; $this->contentTypeService = $contentTypeService; + $this->fieldTypeService = $fieldTypeService; $this->fieldsGroupsList = $fieldsGroupsList; $this->languageService = $languageService; $this->localeConverter = $localeConverter; $this->translator = $translator; } + /** + * @param \Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeCreateStruct|\Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeDraft $contentType + */ public function addMetaFieldDefinitions(ValueObject $contentType, ?Language $language = null): void { $metaFieldTypes = $this->contentTypeFieldTypesResolver->getMetaFieldTypes(); @@ -70,8 +80,15 @@ public function addMetaFieldDefinitions(ValueObject $contentType, ?Language $lan foreach ($metaFieldTypes as $metaFieldTypeIdentifier => $metaFieldTypeSettings) { $fieldGroup = $this->getDefaultMetaDataFieldTypeGroup() ?? $this->fieldsGroupsList->getDefaultGroup(); + try { + $isSingular = $this->fieldTypeService->getFieldType($metaFieldTypeIdentifier)->isSingular(); + } catch (NotFoundException|FieldTypeNotFoundException $e) { + continue; + } + + $fieldTypeGroup = $isSingular === true ? null : $fieldGroup; - if ($this->metaFieldDefinitionExists($metaFieldTypeIdentifier, $fieldGroup, $contentType)) { + if ($this->metaFieldDefinitionExists($metaFieldTypeIdentifier, $fieldTypeGroup, $contentType)) { continue; } @@ -120,15 +137,18 @@ public function createMetaFieldDefinitionCreateStruct( return $fieldDefinitionCreateStruct; } + /** + * @param \Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeCreateStruct|\Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeDraft $contentType + */ public function metaFieldDefinitionExists( string $fieldTypeIdentifier, - string $fieldTypeGroup, + ?string $fieldTypeGroup, ValueObject $contentType ): bool { foreach ($contentType->fieldDefinitions as $fieldDefinition) { if ( $fieldDefinition->fieldTypeIdentifier === $fieldTypeIdentifier - && $fieldDefinition->fieldGroup === $fieldTypeGroup + && ($fieldTypeGroup === null || $fieldDefinition->fieldGroup === $fieldTypeGroup) ) { return true; } diff --git a/src/lib/Service/MetaFieldType/MetaFieldDefinitionServiceInterface.php b/src/lib/Service/MetaFieldType/MetaFieldDefinitionServiceInterface.php index 9728c865e0..e9e85a5e55 100644 --- a/src/lib/Service/MetaFieldType/MetaFieldDefinitionServiceInterface.php +++ b/src/lib/Service/MetaFieldType/MetaFieldDefinitionServiceInterface.php @@ -17,6 +17,9 @@ */ interface MetaFieldDefinitionServiceInterface { + /** + * @param \Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeCreateStruct|\Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeDraft $contentType + */ public function addMetaFieldDefinitions( ValueObject $contentType, ?Language $language = null @@ -29,9 +32,12 @@ public function createMetaFieldDefinitionCreateStruct( int $position ): FieldDefinitionCreateStruct; + /** + * @param \Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeCreateStruct|\Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeDraft $contentType + */ public function metaFieldDefinitionExists( string $fieldTypeIdentifier, - string $fieldTypeGroup, + ?string $fieldTypeGroup, ValueObject $contentType ): bool; diff --git a/tests/lib/Service/MetaFieldType/MetaFieldDefinitionServiceTest.php b/tests/lib/Service/MetaFieldType/MetaFieldDefinitionServiceTest.php new file mode 100644 index 0000000000..230268e96e --- /dev/null +++ b/tests/lib/Service/MetaFieldType/MetaFieldDefinitionServiceTest.php @@ -0,0 +1,312 @@ +contentTypeService = $this->createMock(ContentTypeService::class); + $this->fieldTypeService = $this->createMock(FieldTypeService::class); + $this->contentTypeFieldTypesResolver = $this->createStub(ContentTypeFieldTypesResolverInterface::class); + $this->fieldsGroupsList = $this->createStub(FieldsGroupsList::class); + $this->languageService = $this->createStub(LanguageService::class); + $this->configResolver = $this->createStub(ConfigResolverInterface::class); + + $this->configResolver + ->method('hasParameter') + ->willReturn(false); + + $this->fieldsGroupsList + ->method('getDefaultGroup') + ->willReturn(self::DEFAULT_FIELD_GROUP); + + $this->languageService + ->method('getDefaultLanguageCode') + ->willReturn('eng-GB'); + $this->languageService + ->method('loadLanguage') + ->willReturn(new Language(['languageCode' => 'eng-GB'])); + + $this->contentTypeService + ->method('newFieldDefinitionCreateStruct') + ->willReturnCallback( + static fn (string $identifier, string $fieldTypeIdentifier): FieldDefinitionCreateStruct => new FieldDefinitionCreateStruct([ + 'identifier' => $identifier, + 'fieldTypeIdentifier' => $fieldTypeIdentifier, + ]) + ); + + $localeConverter = $this->createStub(LocaleConverterInterface::class); + $localeConverter->method('convertToPOSIX')->willReturn('en_GB'); + + $translator = $this->createStub(TranslatorInterface::class); + $translator->method('trans')->willReturn('Label'); + + $this->metaFieldDefinitionService = new MetaFieldDefinitionService( + $this->configResolver, + $this->contentTypeFieldTypesResolver, + $this->contentTypeService, + $this->fieldTypeService, + $this->fieldsGroupsList, + $this->languageService, + $localeConverter, + $translator + ); + } + + /** + * @dataProvider provideFieldGroupsForExistenceCheck + */ + public function testMetaFieldDefinitionExists( + string $fieldTypeIdentifier, + string $existingFieldGroup, + ?string $queryFieldGroup, + bool $expectedResult + ): void { + $contentType = $this->createContentTypeDraft([ + $this->createFieldDefinition($fieldTypeIdentifier, $existingFieldGroup), + ]); + + self::assertSame( + $expectedResult, + $this->metaFieldDefinitionService->metaFieldDefinitionExists( + $fieldTypeIdentifier, + $queryFieldGroup, + $contentType + ) + ); + } + + /** + * @return iterable + */ + public static function provideFieldGroupsForExistenceCheck(): iterable + { + yield 'singular field ignores field group' => [ + self::SINGULAR_FIELD_TYPE_IDENTIFIER, + self::OTHER_FIELD_GROUP, + null, + true, + ]; + + yield 'non-singular field in different group is not found' => [ + self::NON_SINGULAR_FIELD_TYPE_IDENTIFIER, + self::OTHER_FIELD_GROUP, + self::DEFAULT_FIELD_GROUP, + false, + ]; + + yield 'non-singular field in matching group is found' => [ + self::NON_SINGULAR_FIELD_TYPE_IDENTIFIER, + self::OTHER_FIELD_GROUP, + self::OTHER_FIELD_GROUP, + true, + ]; + } + + public function testAddMetaFieldDefinitionsDoesNotDuplicateSingularFieldAlreadyPresentInDifferentGroup(): void + { + $contentType = $this->createContentTypeDraft([ + $this->createFieldDefinition(self::SINGULAR_FIELD_TYPE_IDENTIFIER, self::OTHER_FIELD_GROUP), + ]); + + $this->contentTypeFieldTypesResolver + ->method('getMetaFieldTypes') + ->willReturn([ + self::SINGULAR_FIELD_TYPE_IDENTIFIER => ['meta' => true, 'position' => 1], + ]); + + $this->fieldTypeService + ->expects(self::once()) + ->method('getFieldType') + ->with(self::SINGULAR_FIELD_TYPE_IDENTIFIER) + ->willReturn($this->createFieldType(true)); + + $this->contentTypeService + ->expects(self::never()) + ->method('addFieldDefinition'); + + $this->metaFieldDefinitionService->addMetaFieldDefinitions($contentType); + } + + public function testAddMetaFieldDefinitionsSkipsMetaFieldTypeWhenFieldTypeIsNotFound(): void + { + $contentType = $this->createContentTypeDraft([]); + + $this->contentTypeFieldTypesResolver + ->method('getMetaFieldTypes') + ->willReturn([ + self::MISSING_FIELD_TYPE_IDENTIFIER => ['meta' => true, 'position' => 1], + self::NON_SINGULAR_FIELD_TYPE_IDENTIFIER => ['meta' => true, 'position' => 2], + ]); + + $this->fieldTypeService + ->expects(self::exactly(2)) + ->method('getFieldType') + ->with(self::logicalOr(self::MISSING_FIELD_TYPE_IDENTIFIER, self::NON_SINGULAR_FIELD_TYPE_IDENTIFIER)) + ->willReturnCallback( + function (string $fieldTypeIdentifier): FieldType { + if ($fieldTypeIdentifier === self::MISSING_FIELD_TYPE_IDENTIFIER) { + throw new NotFoundException('FieldType', $fieldTypeIdentifier); + } + + return $this->createFieldType(false); + } + ); + + $this->contentTypeService + ->expects(self::once()) + ->method('addFieldDefinition') + ->with( + $contentType, + self::callback( + static fn (FieldDefinitionCreateStruct $struct): bool => $struct->fieldTypeIdentifier === self::NON_SINGULAR_FIELD_TYPE_IDENTIFIER + ) + ); + + $this->metaFieldDefinitionService->addMetaFieldDefinitions($contentType); + } + + /** + * @dataProvider provideNonDuplicateAdditionScenarios + * + * @param array $existingFieldDefinitions + */ + public function testAddMetaFieldDefinitionsAddsFieldWhenNotAlreadyPresentInMatchingGroup( + array $existingFieldDefinitions, + string $fieldTypeIdentifier, + bool $isSingular + ): void { + $contentType = $this->createContentTypeDraft(array_map( + fn (array $fieldDefinition): FieldDefinition => $this->createFieldDefinition(...$fieldDefinition), + $existingFieldDefinitions + )); + + $this->contentTypeFieldTypesResolver + ->method('getMetaFieldTypes') + ->willReturn([ + $fieldTypeIdentifier => ['meta' => true, 'position' => 1], + ]); + + $this->fieldTypeService + ->expects(self::once()) + ->method('getFieldType') + ->with($fieldTypeIdentifier) + ->willReturn($this->createFieldType($isSingular)); + + $this->contentTypeService + ->expects(self::once()) + ->method('addFieldDefinition') + ->with( + $contentType, + self::callback( + static fn (FieldDefinitionCreateStruct $struct): bool => $struct->fieldTypeIdentifier === $fieldTypeIdentifier + ) + ); + + $this->metaFieldDefinitionService->addMetaFieldDefinitions($contentType); + } + + /** + * @return iterable, string, bool}> + */ + public static function provideNonDuplicateAdditionScenarios(): iterable + { + yield 'non-singular field present only in different group is still added' => [ + [[self::NON_SINGULAR_FIELD_TYPE_IDENTIFIER, self::OTHER_FIELD_GROUP]], + self::NON_SINGULAR_FIELD_TYPE_IDENTIFIER, + false, + ]; + + yield 'singular field not yet present is added' => [ + [], + self::SINGULAR_FIELD_TYPE_IDENTIFIER, + true, + ]; + } + + /** + * @param array<\Ibexa\Contracts\Core\Repository\Values\ContentType\FieldDefinition> $fieldDefinitions + */ + private function createContentTypeDraft(array $fieldDefinitions): ContentTypeDraftStub + { + return new ContentTypeDraftStub(new FieldDefinitionCollection($fieldDefinitions)); + } + + private function createFieldDefinition(string $fieldTypeIdentifier, string $fieldGroup): FieldDefinition + { + return new FieldDefinition([ + 'identifier' => $fieldTypeIdentifier, + 'fieldTypeIdentifier' => $fieldTypeIdentifier, + 'fieldGroup' => $fieldGroup, + ]); + } + + /** + * @return \Ibexa\Contracts\Core\Repository\FieldType&\PHPUnit\Framework\MockObject\Stub + */ + private function createFieldType(bool $isSingular): FieldType + { + $fieldType = $this->createStub(FieldType::class); + $fieldType + ->method('isSingular') + ->willReturn($isSingular); + + return $fieldType; + } +} diff --git a/tests/lib/Service/MetaFieldType/Stub/ContentTypeDraftStub.php b/tests/lib/Service/MetaFieldType/Stub/ContentTypeDraftStub.php new file mode 100644 index 0000000000..f482f5037d --- /dev/null +++ b/tests/lib/Service/MetaFieldType/Stub/ContentTypeDraftStub.php @@ -0,0 +1,70 @@ +fieldDefinitionCollection = $fieldDefinitionCollection; + } + + public function __get($property) + { + if ('fieldDefinitions' === $property) { + return $this->getFieldDefinitions(); + } + + return parent::__get($property); + } + + public function getFieldDefinitions(): FieldDefinitionCollection + { + return $this->fieldDefinitionCollection; + } + + /** + * @return array<\Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeGroup> + */ + public function getContentTypeGroups(): array + { + return []; + } + + public function getNames(): array + { + return []; + } + + public function getName($languageCode = null): ?string + { + return null; + } + + public function getDescriptions(): array + { + return []; + } + + public function getDescription($languageCode = null): ?string + { + return null; + } +}