From 0615da2006090b98b06a0912bbbbf96aff88d424 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Tue, 28 Nov 2023 10:21:47 +0100 Subject: [PATCH 01/34] IBX-5388: Fixed performance issues of content updates after field changes --- .../Tests/ContentTypeServiceTest.php | 60 ----- .../Tests/NonRedundantFieldSetTest.php | 20 +- .../Legacy/Content/FieldHandler.php | 4 + .../Persistence/Legacy/Content/Mapper.php | 67 +++++- .../Legacy/Content/StorageHandler.php | 2 +- .../Legacy/Content/Type/Handler.php | 1 - .../Type/Update/Handler/DoctrineDatabase.php | 12 +- .../DatabasePlatform/SqliteGateway.php | 32 +-- .../Legacy/Tests/Content/MapperTest.php | 207 ++++++++++++++---- .../Tests/Content/StorageHandlerTest.php | 1 + .../Content/Type/ContentTypeHandlerTest.php | 9 +- .../Type/Gateway/DoctrineDatabaseTest.php | 1 - .../Update/Handler/DoctrineDatabaseTest.php | 27 +-- .../extract_content_from_rows_result.php | 56 +++++ .../SharedGateway/GatewayFactoryTest.php | 2 +- .../Persistence/Legacy/Tests/TestCase.php | 2 +- .../Tests/Content/HandlerContentSortTest.php | 1 + .../Tests/Content/HandlerContentTest.php | 1 + .../storage_engines/legacy/content.yml | 1 + .../storage_engines/legacy/content_type.yml | 1 - .../Tests/FieldType/BaseIntegrationTest.php | 9 +- 21 files changed, 330 insertions(+), 186 deletions(-) diff --git a/eZ/Publish/API/Repository/Tests/ContentTypeServiceTest.php b/eZ/Publish/API/Repository/Tests/ContentTypeServiceTest.php index b76bab4f72..91b70701f8 100644 --- a/eZ/Publish/API/Repository/Tests/ContentTypeServiceTest.php +++ b/eZ/Publish/API/Repository/Tests/ContentTypeServiceTest.php @@ -2021,66 +2021,6 @@ public function testRemoveFieldDefinitionRemovesFieldFromContentRemoved($data) ); } - /** - * @covers \eZ\Publish\API\Repository\ContentTypeService::removeFieldDefinition() - */ - public function testRemoveFieldDefinitionRemovesOrphanedRelations(): void - { - $repository = $this->getRepository(); - - $contentTypeService = $repository->getContentTypeService(); - $contentService = $repository->getContentService(); - - // Create ContentType - $contentTypeDraft = $this->createContentTypeDraft([$this->getRelationFieldDefinition()]); - $contentTypeService->publishContentTypeDraft($contentTypeDraft); - $publishedType = $contentTypeService->loadContentType($contentTypeDraft->id); - - // Create Content with Relation - $contentDraft = $this->createContentDraft(); - $publishedVersion = $contentService->publishVersion($contentDraft->versionInfo); - - $newDraft = $contentService->createContentDraft($publishedVersion->contentInfo); - $updateStruct = $contentService->newContentUpdateStruct(); - $updateStruct->setField('relation', 14, 'eng-US'); - $contentDraft = $contentService->updateContent($newDraft->versionInfo, $updateStruct); - $publishedContent = $contentService->publishVersion($contentDraft->versionInfo); - - // Remove field definition from ContentType - $contentTypeDraft = $contentTypeService->createContentTypeDraft($publishedType); - $relationField = $contentTypeDraft->getFieldDefinition('relation'); - $contentTypeService->removeFieldDefinition($contentTypeDraft, $relationField); - $contentTypeService->publishContentTypeDraft($contentTypeDraft); - - // Load Content - $content = $contentService->loadContent($publishedContent->contentInfo->id); - - $this->assertCount(0, $contentService->loadRelations($content->versionInfo)); - } - - private function getRelationFieldDefinition(): FieldDefinitionCreateStruct - { - $repository = $this->getRepository(); - - $contentTypeService = $repository->getContentTypeService(); - - $relationFieldCreate = $contentTypeService->newFieldDefinitionCreateStruct( - 'relation', - 'ezobjectrelation' - ); - $relationFieldCreate->names = ['eng-US' => 'Relation']; - $relationFieldCreate->descriptions = ['eng-US' => 'Relation to any Content']; - $relationFieldCreate->fieldGroup = 'blog-content'; - $relationFieldCreate->position = 3; - $relationFieldCreate->isTranslatable = false; - $relationFieldCreate->isRequired = false; - $relationFieldCreate->isInfoCollector = false; - $relationFieldCreate->validatorConfiguration = []; - $relationFieldCreate->isSearchable = false; - - return $relationFieldCreate; - } - /** * Test for the addFieldDefinition() method. * diff --git a/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php b/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php index e0232a703c..3793d64551 100644 --- a/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php +++ b/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php @@ -152,13 +152,13 @@ public function testCreateContentEmptyValuesTranslationNotStored() * * @param \eZ\Publish\API\Repository\Values\Content\Content $content */ - public function testCreateContentEmptyValuesTranslationNotStoredFields(Content $content) + public function testCreateContentEmptyValuesTranslationVirtualFields(Content $content) { $emptyValue = $this->getRepository()->getFieldTypeService()->getFieldType('ezstring')->getEmptyValue(); $this->assertCount(1, $content->versionInfo->languageCodes); $this->assertContains('eng-US', $content->versionInfo->languageCodes); - $this->assertCount(4, $content->getFields()); + $this->assertCount(8, $content->getFields()); // eng-US $this->assertContains('eng-US', $content->versionInfo->languageCodes); @@ -167,8 +167,12 @@ public function testCreateContentEmptyValuesTranslationNotStoredFields(Content $ $this->assertEquals($emptyValue, $content->getFieldValue('field3', 'eng-US')); $this->assertEquals($emptyValue, $content->getFieldValue('field4', 'eng-US')); - // ger-DE is not stored! + // ger-DE is not stored and fields are virtual $this->assertNotContains('ger-DE', $content->versionInfo->languageCodes); + $this->assertNull($content->getField('field1', 'ger-DE')->id); + $this->assertNull($content->getField('field2', 'ger-DE')->id); + $this->assertNull($content->getField('field3', 'ger-DE')->id); + $this->assertNull($content->getField('field4', 'ger-DE')->id); } /** @@ -262,13 +266,13 @@ public function testCreateContentTwoLanguagesSecondTranslationNotStored() * * @param \eZ\Publish\API\Repository\Values\Content\Content $content */ - public function testCreateContentTwoLanguagesSecondTranslationNotStoredFields(Content $content) + public function testCreateContentTwoLanguagesSecondTranslationVirtualFields(Content $content) { $emptyValue = $this->getRepository()->getFieldTypeService()->getFieldType('ezstring')->getEmptyValue(); $this->assertCount(1, $content->versionInfo->languageCodes); $this->assertContains('eng-US', $content->versionInfo->languageCodes); - $this->assertCount(4, $content->getFields()); + $this->assertCount(8, $content->getFields()); // eng-US $this->assertEquals($emptyValue, $content->getFieldValue('field1', 'eng-US')); @@ -276,8 +280,12 @@ public function testCreateContentTwoLanguagesSecondTranslationNotStoredFields(Co $this->assertEquals($emptyValue, $content->getFieldValue('field3', 'eng-US')); $this->assertEquals('default value 4', $content->getFieldValue('field4', 'eng-US')); - // ger-DE is not stored! + // ger-DE is not stored and fields are virtual $this->assertNotContains('ger-DE', $content->versionInfo->languageCodes); + $this->assertNull($content->getField('field1', 'ger-DE')->id); + $this->assertNull($content->getField('field2', 'ger-DE')->id); + $this->assertNull($content->getField('field3', 'ger-DE')->id); + $this->assertNull($content->getField('field4', 'ger-DE')->id); } /** diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php b/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php index cd3440db7a..39b3a6d6ba 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php @@ -151,6 +151,10 @@ protected function getEmptyField(FieldDefinition $fieldDefinition, $languageCode public function createExistingFieldsInNewVersion(Content $content) { foreach ($content->fields as $field) { + if ($field->id === null) { + // Virtual field with default value, skip creating field as it has no id + continue; + } $this->createExistingFieldInNewVersion($field, $content); } } diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index 20f3708b93..0ec0437ee7 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -16,6 +16,8 @@ use eZ\Publish\SPI\Persistence\Content\Language\Handler as LanguageHandler; use eZ\Publish\SPI\Persistence\Content\Relation; use eZ\Publish\SPI\Persistence\Content\Relation\CreateStruct as RelationCreateStruct; +use eZ\Publish\SPI\Persistence\Content\Type\FieldDefinition; +use eZ\Publish\SPI\Persistence\Content\Type\Handler as ContentTypeHandler; use eZ\Publish\SPI\Persistence\Content\VersionInfo; /** @@ -39,16 +41,25 @@ class Mapper */ protected $languageHandler; + /** + * @var \eZ\Publish\SPI\Persistence\Content\Type\Handler + */ + private $contentTypeHandler; + /** * Creates a new mapper. * * @param \eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\ConverterRegistry $converterRegistry * @param \eZ\Publish\SPI\Persistence\Content\Language\Handler $languageHandler */ - public function __construct(Registry $converterRegistry, LanguageHandler $languageHandler) - { + public function __construct( + Registry $converterRegistry, + LanguageHandler $languageHandler, + ContentTypeHandler $contentTypeHandler + ) { $this->converterRegistry = $converterRegistry; $this->languageHandler = $languageHandler; + $this->contentTypeHandler = $contentTypeHandler; } /** @@ -182,18 +193,24 @@ public function convertToStorageValue(Field $field) public function extractContentFromRows(array $rows, array $nameRows, $prefix = 'ezcontentobject_') { $versionedNameData = []; + $languageCodes = []; + foreach ($nameRows as $row) { - $contentId = (int)$row['ezcontentobject_name_contentobject_id']; - $versionNo = (int)$row['ezcontentobject_name_content_version']; - $versionedNameData[$contentId][$versionNo][$row['ezcontentobject_name_content_translation']] = $row['ezcontentobject_name_name']; + $contentId = (int)$row["{$prefix}name_contentobject_id"]; + $versionNo = (int)$row["{$prefix}name_content_version"]; + $versionedNameData[$contentId][$versionNo][$row["{$prefix}name_content_translation"]] = $row["{$prefix}name_name"]; + $languageCodes[] = $row["{$prefix}name_content_translation"]; } $contentInfos = []; $versionInfos = []; $fields = []; + $fieldDefinitions = []; foreach ($rows as $row) { $contentId = (int)$row["{$prefix}id"]; + $versionId = (int)$row["{$prefix}version_id"]; + $contentTypeId = (int)$row["{$prefix}contentclass_id"]; if (!isset($contentInfos[$contentId])) { $contentInfos[$contentId] = $this->extractContentInfoFromRow($row, $prefix); } @@ -201,14 +218,25 @@ public function extractContentFromRows(array $rows, array $nameRows, $prefix = ' $versionInfos[$contentId] = []; } - $versionId = (int)$row['ezcontentobject_version_id']; + if (!isset($fieldDefinitions[$contentId][$versionId])) { + $contentType = $this->contentTypeHandler->load($contentTypeId); + foreach ($contentType->fieldDefinitions as $fieldDefinition) { + foreach ($languageCodes as $languageCode) { + $fieldDefinitions[$contentId][$versionId][$languageCode][$fieldDefinition->id] = $fieldDefinition; + } + } + } + if (!isset($versionInfos[$contentId][$versionId])) { $versionInfos[$contentId][$versionId] = $this->extractVersionInfoFromRow($row); } - $fieldId = (int)$row['ezcontentobject_attribute_id']; - if (!isset($fields[$contentId][$versionId][$fieldId])) { + $fieldId = (int)$row["{$prefix}attribute_id"]; + $fieldDefinitionId = (int)$row["{$prefix}attribute_contentclassattribute_id"]; + $languageCode = $row["{$prefix}attribute_language_code"]; + if (!isset($fields[$contentId][$versionId][$fieldId]) && isset($fieldDefinitions[$contentId][$versionId][$languageCode][$fieldDefinitionId])) { $fields[$contentId][$versionId][$fieldId] = $this->extractFieldFromRow($row); + unset($fieldDefinitions[$contentId][$versionId][$languageCode][$fieldDefinitionId]); } } @@ -227,6 +255,17 @@ public function extractContentFromRows(array $rows, array $nameRows, $prefix = ' $content->versionInfo->names = $names; $content->versionInfo->contentInfo = $contentInfo; $content->fields = array_values($fields[$contentId][$versionId]); + + /** @var string $languageCode */ + foreach ($fieldDefinitions[$contentId][$versionId] as $languageCode => $versionFieldDefinitions) { + foreach ($versionFieldDefinitions as $fieldDefinition) { + $content->fields[] = $this->createEmptyField( + $fieldDefinition, + $languageCode + ); + } + } + $results[] = $content; } } @@ -251,7 +290,6 @@ public function extractContentInfoFromRow(array $row, $prefix = '', $treePrefix $contentInfo->contentTypeId = (int)$row["{$prefix}contentclass_id"]; $contentInfo->sectionId = (int)$row["{$prefix}section_id"]; $contentInfo->currentVersionNo = (int)$row["{$prefix}current_version"]; - $contentInfo->isPublished = ($row["{$prefix}status"] == ContentInfo::STATUS_PUBLISHED); $contentInfo->ownerId = (int)$row["{$prefix}owner_id"]; $contentInfo->publicationDate = (int)$row["{$prefix}published"]; $contentInfo->modificationDate = (int)$row["{$prefix}modified"]; @@ -578,4 +616,15 @@ public function createRelationFromCreateStruct(RelationCreateStruct $struct) return $relation; } + + private function createEmptyField(FieldDefinition $fieldDefinition, string $languageCode): Field + { + $field = new Field(); + $field->fieldDefinitionId = $fieldDefinition->id; + $field->type = $fieldDefinition->fieldType; + $field->value = $fieldDefinition->defaultValue; + $field->languageCode = $languageCode; + + return $field; + } } diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php b/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php index 72c4bb7e8d..45fd043be1 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php @@ -79,7 +79,7 @@ public function copyFieldData(VersionInfo $versionInfo, Field $field, Field $ori public function getFieldData(VersionInfo $versionInfo, Field $field) { $storage = $this->storageRegistry->getStorage($field->type); - if ($storage->hasFieldData()) { + if ($storage->hasFieldData() && $field->id !== null) { $storage->getFieldData($versionInfo, $field, $this->context); } } diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Type/Handler.php b/eZ/Publish/Core/Persistence/Legacy/Content/Type/Handler.php index dcb3f423e9..54068a5393 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Type/Handler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Type/Handler.php @@ -592,7 +592,6 @@ public function publish($contentTypeId) try { $fromType = $this->load($contentTypeId, Type::STATUS_DEFINED); - $this->updateHandler->updateContentObjects($fromType, $toType); $this->updateHandler->deleteOldType($fromType); } catch (Exception\TypeNotFound $e) { // If no old type is found, no updates are necessary to it diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Type/Update/Handler/DoctrineDatabase.php b/eZ/Publish/Core/Persistence/Legacy/Content/Type/Update/Handler/DoctrineDatabase.php index 7a5bc6ccc1..9351df00bf 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Type/Update/Handler/DoctrineDatabase.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Type/Update/Handler/DoctrineDatabase.php @@ -8,7 +8,6 @@ namespace eZ\Publish\Core\Persistence\Legacy\Content\Type\Update\Handler; -use eZ\Publish\Core\Persistence\Legacy\Content\Type\ContentUpdater; use eZ\Publish\Core\Persistence\Legacy\Content\Type\Gateway; use eZ\Publish\Core\Persistence\Legacy\Content\Type\Update\Handler; use eZ\Publish\SPI\Persistence\Content\Type; @@ -23,21 +22,14 @@ final class DoctrineDatabase extends Handler /** @var \eZ\Publish\Core\Persistence\Legacy\Content\Type\Gateway */ protected $contentTypeGateway; - /** @var \eZ\Publish\Core\Persistence\Legacy\Content\Type\ContentUpdater */ - protected $contentUpdater; - - public function __construct(Gateway $contentTypeGateway, ContentUpdater $contentUpdater) + public function __construct(Gateway $contentTypeGateway) { $this->contentTypeGateway = $contentTypeGateway; - $this->contentUpdater = $contentUpdater; } public function updateContentObjects(Type $fromType, Type $toType): void { - $this->contentUpdater->applyUpdates( - $fromType->id, - $this->contentUpdater->determineActions($fromType, $toType) - ); + // Do nothing, content objects are no longer updated } public function deleteOldType(Type $fromType): void diff --git a/eZ/Publish/Core/Persistence/Legacy/SharedGateway/DatabasePlatform/SqliteGateway.php b/eZ/Publish/Core/Persistence/Legacy/SharedGateway/DatabasePlatform/SqliteGateway.php index fcb081f759..fea446be49 100644 --- a/eZ/Publish/Core/Persistence/Legacy/SharedGateway/DatabasePlatform/SqliteGateway.php +++ b/eZ/Publish/Core/Persistence/Legacy/SharedGateway/DatabasePlatform/SqliteGateway.php @@ -8,8 +8,6 @@ namespace eZ\Publish\Core\Persistence\Legacy\SharedGateway\DatabasePlatform; -use Doctrine\DBAL\Connection; -use Doctrine\DBAL\FetchMode; use eZ\Publish\Core\Base\Exceptions\DatabaseException; use eZ\Publish\Core\Persistence\Legacy\SharedGateway\Gateway; @@ -20,39 +18,19 @@ final class SqliteGateway implements Gateway */ private const FATAL_ERROR_CODE = 7; - /** @var \Doctrine\DBAL\Connection */ - private $connection; - - /** @var \Doctrine\DBAL\Platforms\AbstractPlatform */ - private $databasePlatform; - - /** @var int[] */ + /** @var array */ private $lastInsertedIds = []; - /** - * @throws \Doctrine\DBAL\DBALException - */ - public function __construct(Connection $connection) - { - $this->connection = $connection; - $this->databasePlatform = $connection->getDatabasePlatform(); - } - public function getColumnNextIntegerValue( string $tableName, string $columnName, string $sequenceName ): ?int { - $query = $this->connection->createQueryBuilder(); - $query - ->select($this->databasePlatform->getMaxExpression($columnName)) - ->from($tableName); - - $lastId = (int)$query->execute()->fetch(FetchMode::COLUMN); + $lastId = $this->lastInsertedIds[$sequenceName] ?? 0; + $nextId = (int)hrtime(true); - $this->lastInsertedIds[$sequenceName] = $lastId + 1; - - return $this->lastInsertedIds[$sequenceName]; + // $lastId === $nextId shouldn't happen using high-resolution time, but better safe than sorry + return $this->lastInsertedIds[$sequenceName] = $lastId === $nextId ? $nextId + 1 : $nextId; } /** diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php index 99a721f147..6a583875b6 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php @@ -149,7 +149,7 @@ public function testConvertToStorageValue() $field->type = 'some-type'; $field->value = new FieldValue(); - $mapper = new Mapper($reg, $this->getLanguageHandler()); + $mapper = new Mapper($reg, $this->getLanguageHandler(), $this->getContentTypeHandler()); $res = $mapper->convertToStorageValue($field); $this->assertInstanceOf( @@ -169,36 +169,110 @@ public function testExtractContentFromRows() $rowsFixture = $this->getContentExtractFixture(); $nameRowsFixture = $this->getNamesExtractFixture(); - $convMock = $this->createMock(Converter::class); - $convMock->expects($this->exactly(count($rowsFixture))) - ->method('toFieldValue') - ->with( - $this->isInstanceOf( - StorageFieldValue::class - ) - )->will( - $this->returnValue( - new FieldValue() - ) - ); + $contentType = $this->getContentTypeFromRows($rowsFixture); + + $contentTypeHandlerMock = $this->getContentTypeHandler(); + $contentTypeHandlerMock->method('load')->willReturn($contentType); - $reg = new Registry( + $reg = $this->getFieldRegistry([ + 'ezauthor', + 'ezstring', + 'ezboolean', + 'ezimage', + 'ezdatetime', + 'ezkeyword', + ], count($rowsFixture)); + + $mapper = new Mapper($reg, $this->getLanguageHandler(), $contentTypeHandlerMock); + $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); + + $this->assertEquals( [ - 'ezauthor' => $convMock, - 'ezstring' => $convMock, - 'ezboolean' => $convMock, - 'ezimage' => $convMock, - 'ezdatetime' => $convMock, - 'ezkeyword' => $convMock, - ] + $this->getContentExtractReference(), + ], + $result ); + } + + public function testExtractContentFromRowsWithNewFieldDefinitions(): void + { + $rowsFixture = $this->getContentExtractFixture(); + $nameRowsFixture = $this->getNamesExtractFixture(); - $mapper = new Mapper($reg, $this->getLanguageHandler()); + $contentType = $this->getContentTypeFromRows($rowsFixture); + $contentType->fieldDefinitions[] = new Content\Type\FieldDefinition([ + 'fieldType' => 'eznumber', + ]); + + $contentTypeHandlerMock = $this->getContentTypeHandler(); + $contentTypeHandlerMock->method('load')->willReturn($contentType); + + $reg = $this->getFieldRegistry([ + 'ezauthor', + 'ezstring', + 'ezboolean', + 'ezimage', + 'ezdatetime', + 'ezkeyword', + 'eznumber', + ], count($rowsFixture)); + + $mapper = new Mapper($reg, $this->getLanguageHandler(), $contentTypeHandlerMock); $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); + $expectedContent = $this->getContentExtractReference(); + $expectedContent->fields[] = new Field([ + 'type' => 'eznumber', + 'languageCode' => 'eng-GB', + 'value' => new FieldValue(), + ]); + $expectedContent->fields[] = new Field([ + 'type' => 'eznumber', + 'languageCode' => 'eng-US', + 'value' => new FieldValue(), + ]); + $this->assertEquals( [ - $this->getContentExtractReference(), + $expectedContent, + ], + $result + ); + } + + public function testExtractContentFromRowsWithRemovedFieldDefinitions(): void + { + $rowsFixture = $this->getContentExtractFixture(); + $nameRowsFixture = $this->getNamesExtractFixture(); + + $contentType = $this->getContentTypeFromRows($rowsFixture); + $contentType->fieldDefinitions = array_filter($contentType->fieldDefinitions, static function (Content\Type\FieldDefinition $fieldDefinition) { + // ref. fixtures, ezauthor + return $fieldDefinition->id !== 185; + }); + + $contentTypeHandlerMock = $this->getContentTypeHandler(); + $contentTypeHandlerMock->method('load')->willReturn($contentType); + + $reg = $this->getFieldRegistry([ + 'ezstring', + 'ezboolean', + 'ezimage', + 'ezdatetime', + 'ezkeyword', + ], count($rowsFixture) - 1); + + $mapper = new Mapper($reg, $this->getLanguageHandler(), $contentTypeHandlerMock); + $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); + + $expectedContent = $this->getContentExtractReference(); + $expectedContent->fields = array_values(array_filter($expectedContent->fields, static function (Field $field) { + return $field->fieldDefinitionId !== 185; + })); + + $this->assertEquals( + [ + $expectedContent, ], $result ); @@ -209,22 +283,20 @@ public function testExtractContentFromRows() */ public function testExtractContentFromRowsMultipleVersions() { - $convMock = $this->createMock(Converter::class); - $convMock->expects($this->any()) - ->method('toFieldValue') - ->will($this->returnValue(new FieldValue())); - - $reg = new Registry( - [ - 'ezstring' => $convMock, - 'ezdatetime' => $convMock, - ] - ); + $reg = $this->getFieldRegistry([ + 'ezstring', + 'ezdatetime', + ]); $rowsFixture = $this->getMultipleVersionsExtractFixture(); $nameRowsFixture = $this->getMultipleVersionsNamesExtractFixture(); - $mapper = new Mapper($reg, $this->getLanguageHandler()); + $contentType = $this->getContentTypeFromRows($rowsFixture); + + $contentTypeHandlerMock = $this->getContentTypeHandler(); + $contentTypeHandlerMock->method('load')->willReturn($contentType); + + $mapper = new Mapper($reg, $this->getLanguageHandler(), $contentTypeHandlerMock); $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); $this->assertCount( @@ -251,6 +323,30 @@ public function testExtractContentFromRowsMultipleVersions() ); } + /** + * @param string[] $fields + */ + private function getFieldRegistry( + array $fields = [], + int $expectedConverterCalls = null + ): Registry { + $convMock = $this->createMock(Converter::class); + $convMock->expects( + $expectedConverterCalls === null + ? $this->any() + : $this->exactly($expectedConverterCalls) + ) + ->method('toFieldValue') + ->will($this->returnValue(new FieldValue())); + + $converters = []; + foreach ($fields as $field) { + $converters[$field] = $convMock; + } + + return new Registry($converters); + } + /** * @covers \eZ\Publish\Core\Persistence\Legacy\Content\Mapper::createCreateStructFromContent */ @@ -380,7 +476,8 @@ public function testExtractContentInfoFromRow(array $fixtures, $prefix) $contentInfoReference = $this->getContentExtractReference()->versionInfo->contentInfo; $mapper = new Mapper( $this->getValueConverterRegistryMock(), - $this->getLanguageHandler() + $this->getLanguageHandler(), + $this->getContentTypeHandler() ); self::assertEquals($contentInfoReference, $mapper->extractContentInfoFromRow($fixtures, $prefix)); } @@ -540,7 +637,8 @@ protected function getMapper($valueConverter = null) { return new Mapper( $this->getValueConverterRegistryMock(), - $this->getLanguageHandler() + $this->getLanguageHandler(), + $this->getContentTypeHandler() ); } @@ -637,4 +735,39 @@ static function ($languageCode) use ($languages) { return $this->languageHandler; } + + /** + * @return Content\Type\Handler|\PHPUnit\Framework\MockObject\MockObject + */ + protected function getContentTypeHandler() + { + return $this->createMock(Content\Type\Handler::class); + } + + /** + * @param array> $rows + */ + protected function getContentTypeFromRows(array $rows): Content\Type + { + $contentType = new Content\Type(); + $fieldDefinitions = []; + + foreach ($rows as $row) { + $fieldDefinitionId = $row['ezcontentobject_attribute_contentclassattribute_id']; + $fieldType = $row['ezcontentobject_attribute_data_type_string']; + + if (isset($fieldDefinitions[$fieldDefinitionId])) { + continue; + } + + $fieldDefinitions[$fieldDefinitionId] = new Content\Type\FieldDefinition([ + 'id' => $fieldDefinitionId, + 'fieldType' => $fieldType, + ]); + } + + $contentType->fieldDefinitions = array_values($fieldDefinitions); + + return $contentType; + } } diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php index a32bf3fc37..48a2f59da2 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php @@ -101,6 +101,7 @@ public function testGetFieldDataAvailable() ->will($this->returnValue($storageMock)); $field = new Field(); + $field->id = 123; $field->type = 'foobar'; $field->value = new FieldValue(); diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Type/ContentTypeHandlerTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Type/ContentTypeHandlerTest.php index 081c62189e..a2eaf589d5 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Type/ContentTypeHandlerTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Type/ContentTypeHandlerTest.php @@ -1037,12 +1037,9 @@ public function testPublish() $this->returnValue(new Type()) ); - $updateHandlerMock->expects($this->once()) - ->method('updateContentObjects') - ->with( - $this->isInstanceOf(Type::class), - $this->isInstanceOf(Type::class) - ); + $updateHandlerMock->expects($this->never()) + ->method('updateContentObjects'); + $updateHandlerMock->expects($this->once()) ->method('deleteOldType') ->with( diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Type/Gateway/DoctrineDatabaseTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Type/Gateway/DoctrineDatabaseTest.php index 45d13e4d56..524a334df8 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Type/Gateway/DoctrineDatabaseTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Type/Gateway/DoctrineDatabaseTest.php @@ -497,7 +497,6 @@ public function testInsertType($column, $expectation) public static function getTypeCreationContentClassNameExpectations() { return [ - ['contentclass_id', [1, 1]], ['contentclass_version', [0, 0]], ['language_id', [3, 4]], ['language_locale', ['eng-US', 'eng-GB']], diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Type/Update/Handler/DoctrineDatabaseTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Type/Update/Handler/DoctrineDatabaseTest.php index df3acf4596..608ad8a80a 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Type/Update/Handler/DoctrineDatabaseTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Type/Update/Handler/DoctrineDatabaseTest.php @@ -40,23 +40,11 @@ public function testUpdateContentObjects() $updaterMock = $this->getContentUpdaterMock(); - $updaterMock->expects($this->once()) - ->method('determineActions') - ->with( - $this->isInstanceOf( - Type::class - ), - $this->isInstanceOf( - Type::class - ) - )->will($this->returnValue([])); - - $updaterMock->expects($this->once()) - ->method('applyUpdates') - ->with( - $this->equalTo(23), - $this->equalTo([]) - ); + $updaterMock->expects($this->never()) + ->method('determineActions'); + + $updaterMock->expects($this->never()) + ->method('applyUpdates'); $types = $this->getTypeFixtures(); @@ -130,10 +118,7 @@ protected function getTypeFixtures() */ protected function getUpdateHandler() { - return new DoctrineDatabase( - $this->getGatewayMock(), - $this->getContentUpdaterMock() - ); + return new DoctrineDatabase($this->getGatewayMock()); } /** diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/_fixtures/extract_content_from_rows_result.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/_fixtures/extract_content_from_rows_result.php index 0bfc326130..55ef4634a8 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/_fixtures/extract_content_from_rows_result.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/_fixtures/extract_content_from_rows_result.php @@ -129,4 +129,60 @@ $content->fields[] = $field; +$field = new Field(); +$field->fieldDefinitionId = 183; +$field->type = 'ezstring'; +$field->value = new FieldValue(); +$field->languageCode = 'eng-GB'; + +$content->fields[] = $field; + +$field = new Field(); +$field->fieldDefinitionId = 184; +$field->type = 'ezstring'; +$field->value = new FieldValue(); +$field->languageCode = 'eng-GB'; + +$content->fields[] = $field; + +$field = new Field(); +$field->fieldDefinitionId = 185; +$field->type = 'ezauthor'; +$field->value = new FieldValue(); +$field->languageCode = 'eng-GB'; + +$content->fields[] = $field; + +$field = new Field(); +$field->fieldDefinitionId = 188; +$field->type = 'ezboolean'; +$field->value = new FieldValue(); +$field->languageCode = 'eng-GB'; + +$content->fields[] = $field; + +$field = new Field(); +$field->fieldDefinitionId = 189; +$field->type = 'ezimage'; +$field->value = new FieldValue(); +$field->languageCode = 'eng-GB'; + +$content->fields[] = $field; + +$field = new Field(); +$field->fieldDefinitionId = 191; +$field->type = 'ezdatetime'; +$field->value = new FieldValue(); +$field->languageCode = 'eng-GB'; + +$content->fields[] = $field; + +$field = new Field(); +$field->fieldDefinitionId = 192; +$field->type = 'ezdatetime'; +$field->value = new FieldValue(); +$field->languageCode = 'eng-GB'; + +$content->fields[] = $field; + return $content; diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/SharedGateway/GatewayFactoryTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/SharedGateway/GatewayFactoryTest.php index 18856557f2..649da896b8 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/SharedGateway/GatewayFactoryTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/SharedGateway/GatewayFactoryTest.php @@ -30,7 +30,7 @@ final class GatewayFactoryTest extends TestCase public function setUp(): void { $gateways = [ - 'sqlite' => new SqliteGateway($this->createMock(Connection::class)), + 'sqlite' => new SqliteGateway(), ]; $this->factory = new GatewayFactory( diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/TestCase.php b/eZ/Publish/Core/Persistence/Legacy/Tests/TestCase.php index 9a66f99734..055796815a 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/TestCase.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/TestCase.php @@ -113,7 +113,7 @@ final public function getSharedGateway(): SharedGateway\Gateway $factory = new SharedGateway\GatewayFactory( new SharedGateway\DatabasePlatform\FallbackGateway($connection), [ - 'sqlite' => new SharedGateway\DatabasePlatform\SqliteGateway($connection), + 'sqlite' => new SharedGateway\DatabasePlatform\SqliteGateway(), ] ); diff --git a/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentSortTest.php b/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentSortTest.php index b8b2e4c7a8..23d5af04cb 100644 --- a/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentSortTest.php +++ b/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentSortTest.php @@ -101,6 +101,7 @@ protected function getContentMapperMock() [ $this->getFieldRegistry(), $this->getLanguageHandler(), + $this->getContentTypeHandler(), ] ) ->setMethods(['extractContentInfoFromRows']) diff --git a/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentTest.php b/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentTest.php index 5701ba1d00..c1bdbcafcb 100644 --- a/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentTest.php +++ b/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentTest.php @@ -200,6 +200,7 @@ protected function getContentMapperMock() [ $this->getConverterRegistry(), $this->getLanguageHandler(), + $this->getContentTypeHandler(), ] ) ->setMethods(['extractContentInfoFromRows']) diff --git a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml index 8c402f1679..d2efee3322 100644 --- a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml +++ b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml @@ -9,6 +9,7 @@ services: arguments: - "@ezpublish.persistence.legacy.field_value_converter.registry" - "@ezpublish.spi.persistence.legacy.language.handler" + - '@ezpublish.spi.persistence.legacy.content_type.handler' ezpublish.persistence.legacy.content.gateway.inner: class: eZ\Publish\Core\Persistence\Legacy\Content\Gateway\DoctrineDatabase diff --git a/eZ/Publish/Core/settings/storage_engines/legacy/content_type.yml b/eZ/Publish/Core/settings/storage_engines/legacy/content_type.yml index b5f0385afd..951cc8f3fb 100644 --- a/eZ/Publish/Core/settings/storage_engines/legacy/content_type.yml +++ b/eZ/Publish/Core/settings/storage_engines/legacy/content_type.yml @@ -38,7 +38,6 @@ services: class: eZ\Publish\Core\Persistence\Legacy\Content\Type\Update\Handler\DoctrineDatabase arguments: - "@ezpublish.persistence.legacy.content_type.gateway" - - "@ezpublish.persistence.legacy.content_type.content_updater" ezpublish.persistence.legacy.content_type.update_handler: alias: ezpublish.persistence.legacy.content_type.update_handler.basic diff --git a/eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php b/eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php index 396d0be03c..8877671b20 100644 --- a/eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php +++ b/eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php @@ -445,10 +445,11 @@ public function testLoadFieldType($content) { $this->assertSame( $this->getTypeName(), - $content->fields[2]->type + $content->fields[1]->type ); - return $content->fields[2]; + + return $content->fields[1]; } /** @@ -506,10 +507,10 @@ public function testUpdateFieldType($content) { $this->assertSame( $this->getTypeName(), - $content->fields[2]->type + $content->fields[1]->type ); - return $content->fields[2]; + return $content->fields[1]; } /** From f2e552c0187ed28ba9e904c7e0bb0bf2a391c09f Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Thu, 14 Dec 2023 12:20:42 +0100 Subject: [PATCH 02/34] fix: Coding standards --- eZ/Publish/API/Repository/Tests/ContentTypeServiceTest.php | 1 - eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php | 1 - 2 files changed, 2 deletions(-) diff --git a/eZ/Publish/API/Repository/Tests/ContentTypeServiceTest.php b/eZ/Publish/API/Repository/Tests/ContentTypeServiceTest.php index 91b70701f8..ad095d5632 100644 --- a/eZ/Publish/API/Repository/Tests/ContentTypeServiceTest.php +++ b/eZ/Publish/API/Repository/Tests/ContentTypeServiceTest.php @@ -15,7 +15,6 @@ use eZ\Publish\API\Repository\Values\ContentType\ContentTypeGroup; use eZ\Publish\API\Repository\Values\ContentType\FieldDefinition; use eZ\Publish\API\Repository\Values\ContentType\FieldDefinitionCollection as APIFieldDefinitionCollection; -use eZ\Publish\API\Repository\Values\ContentType\FieldDefinitionCreateStruct; use eZ\Publish\API\Repository\Values\Translation\Message; use eZ\Publish\Core\FieldType\TextLine\Value as TextLineValue; diff --git a/eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php b/eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php index 8877671b20..c0ca560020 100644 --- a/eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php +++ b/eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php @@ -448,7 +448,6 @@ public function testLoadFieldType($content) $content->fields[1]->type ); - return $content->fields[1]; } From 58f3f9c0c0208bbfed5b77771b2d0a43a204cde1 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Wed, 20 Dec 2023 09:30:06 +0100 Subject: [PATCH 03/34] fix: Code Review --- .../Tests/NonRedundantFieldSetTest.php | 4 +- .../Legacy/Content/FieldHandler.php | 2 +- .../Persistence/Legacy/Content/Mapper.php | 99 ++++++++++++++----- .../Legacy/Content/StorageHandler.php | 2 +- .../Legacy/Tests/Content/MapperTest.php | 31 +++--- .../Tests/Content/StorageHandlerTest.php | 28 ++++++ 6 files changed, 122 insertions(+), 44 deletions(-) diff --git a/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php b/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php index 3793d64551..a12fe13cf7 100644 --- a/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php +++ b/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php @@ -152,7 +152,7 @@ public function testCreateContentEmptyValuesTranslationNotStored() * * @param \eZ\Publish\API\Repository\Values\Content\Content $content */ - public function testCreateContentEmptyValuesTranslationVirtualFields(Content $content) + public function testCreateContentEmptyValuesTranslationVirtualFields(Content $content): void { $emptyValue = $this->getRepository()->getFieldTypeService()->getFieldType('ezstring')->getEmptyValue(); @@ -266,7 +266,7 @@ public function testCreateContentTwoLanguagesSecondTranslationNotStored() * * @param \eZ\Publish\API\Repository\Values\Content\Content $content */ - public function testCreateContentTwoLanguagesSecondTranslationVirtualFields(Content $content) + public function testCreateContentTwoLanguagesSecondTranslationVirtualFields(Content $content): void { $emptyValue = $this->getRepository()->getFieldTypeService()->getFieldType('ezstring')->getEmptyValue(); diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php b/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php index 39b3a6d6ba..dba486b78e 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php @@ -148,7 +148,7 @@ protected function getEmptyField(FieldDefinition $fieldDefinition, $languageCode * * @param \eZ\Publish\SPI\Persistence\Content $content */ - public function createExistingFieldsInNewVersion(Content $content) + public function createExistingFieldsInNewVersion(Content $content): void { foreach ($content->fields as $field) { if ($field->id === null) { diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index 0ec0437ee7..0649b25572 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -46,12 +46,6 @@ class Mapper */ private $contentTypeHandler; - /** - * Creates a new mapper. - * - * @param \eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\ConverterRegistry $converterRegistry - * @param \eZ\Publish\SPI\Persistence\Content\Language\Handler $languageHandler - */ public function __construct( Registry $converterRegistry, LanguageHandler $languageHandler, @@ -187,11 +181,17 @@ public function convertToStorageValue(Field $field) * * @param array $rows * @param array $nameRows + * @param string $prefix * * @return \eZ\Publish\SPI\Persistence\Content[] + * + * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException */ - public function extractContentFromRows(array $rows, array $nameRows, $prefix = 'ezcontentobject_') - { + public function extractContentFromRows( + array $rows, + array $nameRows, + string $prefix = 'ezcontentobject_' + ): array { $versionedNameData = []; $languageCodes = []; @@ -205,28 +205,25 @@ public function extractContentFromRows(array $rows, array $nameRows, $prefix = ' $contentInfos = []; $versionInfos = []; $fields = []; - $fieldDefinitions = []; + + $fieldDefinitions = $this->loadCachedVersionFieldDefinitionsPerLanguage( + $rows, + $languageCodes, + $prefix + ); foreach ($rows as $row) { $contentId = (int)$row["{$prefix}id"]; $versionId = (int)$row["{$prefix}version_id"]; - $contentTypeId = (int)$row["{$prefix}contentclass_id"]; + if (!isset($contentInfos[$contentId])) { $contentInfos[$contentId] = $this->extractContentInfoFromRow($row, $prefix); } + if (!isset($versionInfos[$contentId])) { $versionInfos[$contentId] = []; } - if (!isset($fieldDefinitions[$contentId][$versionId])) { - $contentType = $this->contentTypeHandler->load($contentTypeId); - foreach ($contentType->fieldDefinitions as $fieldDefinition) { - foreach ($languageCodes as $languageCode) { - $fieldDefinitions[$contentId][$versionId][$languageCode][$fieldDefinition->id] = $fieldDefinition; - } - } - } - if (!isset($versionInfos[$contentId][$versionId])) { $versionInfos[$contentId][$versionId] = $this->extractVersionInfoFromRow($row); } @@ -234,21 +231,41 @@ public function extractContentFromRows(array $rows, array $nameRows, $prefix = ' $fieldId = (int)$row["{$prefix}attribute_id"]; $fieldDefinitionId = (int)$row["{$prefix}attribute_contentclassattribute_id"]; $languageCode = $row["{$prefix}attribute_language_code"]; - if (!isset($fields[$contentId][$versionId][$fieldId]) && isset($fieldDefinitions[$contentId][$versionId][$languageCode][$fieldDefinitionId])) { + + if (!isset($fields[$contentId][$versionId][$fieldId]) + && isset($fieldDefinitions[$contentId][$versionId][$languageCode][$fieldDefinitionId]) + ) { $fields[$contentId][$versionId][$fieldId] = $this->extractFieldFromRow($row); unset($fieldDefinitions[$contentId][$versionId][$languageCode][$fieldDefinitionId]); } } + return $this->buildContentObjects( + $contentInfos, + $versionInfos, + $fields, + $fieldDefinitions, + $versionedNameData + ); + } + + /** + * @return \eZ\Publish\SPI\Persistence\Content[] + */ + private function buildContentObjects( + array $contentInfos, + array $versionInfos, + array $fields, + array $fieldDefinitions, + array $versionedNameData + ): array { $results = []; + foreach ($contentInfos as $contentId => $contentInfo) { foreach ($versionInfos[$contentId] as $versionId => $versionInfo) { // Fallback to just main language name if versioned name data is missing - if (isset($versionedNameData[$contentId][$versionInfo->versionNo])) { - $names = $versionedNameData[$contentId][$versionInfo->versionNo]; - } else { - $names = [$contentInfo->mainLanguageCode => $contentInfo->name]; - } + $names = $versionedNameData[$contentId][$versionInfo->versionNo] + ?? [$contentInfo->mainLanguageCode => $contentInfo->name]; $content = new Content(); $content->versionInfo = $versionInfo; @@ -256,7 +273,6 @@ public function extractContentFromRows(array $rows, array $nameRows, $prefix = ' $content->versionInfo->contentInfo = $contentInfo; $content->fields = array_values($fields[$contentId][$versionId]); - /** @var string $languageCode */ foreach ($fieldDefinitions[$contentId][$versionId] as $languageCode => $versionFieldDefinitions) { foreach ($versionFieldDefinitions as $fieldDefinition) { $content->fields[] = $this->createEmptyField( @@ -273,6 +289,37 @@ public function extractContentFromRows(array $rows, array $nameRows, $prefix = ' return $results; } + /** + * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException + */ + private function loadCachedVersionFieldDefinitionsPerLanguage( + array $rows, + array $languageCodes, + string $prefix + ): array { + $fieldDefinitions = []; + $contentTypes = []; + + foreach ($rows as $row) { + $contentId = (int)$row["{$prefix}id"]; + $versionId = (int)$row["{$prefix}version_id"]; + $contentTypeId = (int)$row["{$prefix}contentclass_id"]; + + if (!isset($fieldDefinitions[$contentId][$versionId])) { + $contentType = $contentTypes[$contentTypeId] = $contentTypes[$contentTypeId] + ?? $this->contentTypeHandler->load($contentTypeId); + foreach ($contentType->fieldDefinitions as $fieldDefinition) { + foreach ($languageCodes as $languageCode) { + $id = $fieldDefinition->id; + $fieldDefinitions[$contentId][$versionId][$languageCode][$id] = $fieldDefinition; + } + } + } + } + + return $fieldDefinitions; + } + /** * Extracts a ContentInfo object from $row. * diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php b/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php index 45fd043be1..4101063fad 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php @@ -79,7 +79,7 @@ public function copyFieldData(VersionInfo $versionInfo, Field $field, Field $ori public function getFieldData(VersionInfo $versionInfo, Field $field) { $storage = $this->storageRegistry->getStorage($field->type); - if ($storage->hasFieldData() && $field->id !== null) { + if ($field->id !== null && $storage->hasFieldData()) { $storage->getFieldData($versionInfo, $field, $this->context); } } diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php index 6a583875b6..d2c3830e70 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php @@ -246,10 +246,13 @@ public function testExtractContentFromRowsWithRemovedFieldDefinitions(): void $nameRowsFixture = $this->getNamesExtractFixture(); $contentType = $this->getContentTypeFromRows($rowsFixture); - $contentType->fieldDefinitions = array_filter($contentType->fieldDefinitions, static function (Content\Type\FieldDefinition $fieldDefinition) { - // ref. fixtures, ezauthor - return $fieldDefinition->id !== 185; - }); + $contentType->fieldDefinitions = array_filter( + $contentType->fieldDefinitions, + static function (Content\Type\FieldDefinition $fieldDefinition) { + // ref. fixtures, ezauthor + return $fieldDefinition->id !== 185; + } + ); $contentTypeHandlerMock = $this->getContentTypeHandler(); $contentTypeHandlerMock->method('load')->willReturn($contentType); @@ -324,24 +327,24 @@ public function testExtractContentFromRowsMultipleVersions() } /** - * @param string[] $fields + * @param string[] $fieldTypeIdentifiers */ private function getFieldRegistry( - array $fields = [], - int $expectedConverterCalls = null + array $fieldTypeIdentifiers = [], + ?int $expectedConverterCalls = null ): Registry { - $convMock = $this->createMock(Converter::class); - $convMock->expects( + $converterMock = $this->createMock(Converter::class); + $converterMock->expects( $expectedConverterCalls === null - ? $this->any() - : $this->exactly($expectedConverterCalls) + ? self::any() + : self::exactly($expectedConverterCalls) ) ->method('toFieldValue') - ->will($this->returnValue(new FieldValue())); + ->willReturn(new FieldValue()); $converters = []; - foreach ($fields as $field) { - $converters[$field] = $convMock; + foreach ($fieldTypeIdentifiers as $fieldTypeIdentifier) { + $converters[$fieldTypeIdentifier] = $converterMock; } return new Registry($converters); diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php index 48a2f59da2..36586954a9 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php @@ -128,6 +128,34 @@ public function testGetFieldDataNotAvailable() ->with($this->equalTo('foobar')) ->will($this->returnValue($storageMock)); + $field = new Field(); + $field->id = 123; + $field->type = 'foobar'; + $field->value = new FieldValue(); + + $handler = $this->getStorageHandler(); + $handler->getFieldData($this->getVersionInfoMock(), $field); + } + + /** + * @covers \eZ\Publish\Core\Persistence\Legacy\Content\StorageHandler::getFieldData + */ + public function testGetFieldDataNotAvailableForVirtualField() + { + $storageMock = $this->getStorageMock(); + $storageRegistryMock = $this->getStorageRegistryMock(); + + $storageMock->expects(self::never()) + ->method('hasFieldData'); + + $storageMock->expects(self::never()) + ->method('getFieldData'); + + $storageRegistryMock->expects(self::once()) + ->method('getStorage') + ->with($this->equalTo('foobar')) + ->willReturn($storageMock); + $field = new Field(); $field->type = 'foobar'; $field->value = new FieldValue(); From de49379a1a2fd8980bb5a366e3370d1d45d0c596 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Wed, 20 Dec 2023 12:27:32 +0100 Subject: [PATCH 04/34] fix: Regression and fix invalid integration tests --- .../Tests/NonRedundantFieldSetTest.php | 16 +-- .../Persistence/Legacy/Content/Mapper.php | 5 +- .../Legacy/Tests/Content/MapperTest.php | 17 +-- .../extract_content_from_rows_result.php | 130 +++++++++--------- .../Tests/FieldType/BaseIntegrationTest.php | 8 +- 5 files changed, 82 insertions(+), 94 deletions(-) diff --git a/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php b/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php index a12fe13cf7..b6725f6b96 100644 --- a/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php +++ b/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php @@ -158,7 +158,7 @@ public function testCreateContentEmptyValuesTranslationVirtualFields(Content $co $this->assertCount(1, $content->versionInfo->languageCodes); $this->assertContains('eng-US', $content->versionInfo->languageCodes); - $this->assertCount(8, $content->getFields()); + $this->assertCount(4, $content->getFields()); // eng-US $this->assertContains('eng-US', $content->versionInfo->languageCodes); @@ -167,12 +167,8 @@ public function testCreateContentEmptyValuesTranslationVirtualFields(Content $co $this->assertEquals($emptyValue, $content->getFieldValue('field3', 'eng-US')); $this->assertEquals($emptyValue, $content->getFieldValue('field4', 'eng-US')); - // ger-DE is not stored and fields are virtual + // ger-DE is not stored $this->assertNotContains('ger-DE', $content->versionInfo->languageCodes); - $this->assertNull($content->getField('field1', 'ger-DE')->id); - $this->assertNull($content->getField('field2', 'ger-DE')->id); - $this->assertNull($content->getField('field3', 'ger-DE')->id); - $this->assertNull($content->getField('field4', 'ger-DE')->id); } /** @@ -272,7 +268,7 @@ public function testCreateContentTwoLanguagesSecondTranslationVirtualFields(Cont $this->assertCount(1, $content->versionInfo->languageCodes); $this->assertContains('eng-US', $content->versionInfo->languageCodes); - $this->assertCount(8, $content->getFields()); + $this->assertCount(4, $content->getFields()); // eng-US $this->assertEquals($emptyValue, $content->getFieldValue('field1', 'eng-US')); @@ -280,12 +276,8 @@ public function testCreateContentTwoLanguagesSecondTranslationVirtualFields(Cont $this->assertEquals($emptyValue, $content->getFieldValue('field3', 'eng-US')); $this->assertEquals('default value 4', $content->getFieldValue('field4', 'eng-US')); - // ger-DE is not stored and fields are virtual + // ger-DE is not stored $this->assertNotContains('ger-DE', $content->versionInfo->languageCodes); - $this->assertNull($content->getField('field1', 'ger-DE')->id); - $this->assertNull($content->getField('field2', 'ger-DE')->id); - $this->assertNull($content->getField('field3', 'ger-DE')->id); - $this->assertNull($content->getField('field4', 'ger-DE')->id); } /** diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index 0649b25572..6b7803a0c6 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -208,7 +208,6 @@ public function extractContentFromRows( $fieldDefinitions = $this->loadCachedVersionFieldDefinitionsPerLanguage( $rows, - $languageCodes, $prefix ); @@ -294,21 +293,23 @@ private function buildContentObjects( */ private function loadCachedVersionFieldDefinitionsPerLanguage( array $rows, - array $languageCodes, string $prefix ): array { $fieldDefinitions = []; $contentTypes = []; + $allLanguages = $this->loadAllLanguagesWithIdKey(); foreach ($rows as $row) { $contentId = (int)$row["{$prefix}id"]; $versionId = (int)$row["{$prefix}version_id"]; $contentTypeId = (int)$row["{$prefix}contentclass_id"]; + $languageMask = (int)$row["{$prefix}version_language_mask"]; if (!isset($fieldDefinitions[$contentId][$versionId])) { $contentType = $contentTypes[$contentTypeId] = $contentTypes[$contentTypeId] ?? $this->contentTypeHandler->load($contentTypeId); foreach ($contentType->fieldDefinitions as $fieldDefinition) { + $languageCodes = $this->extractLanguageCodesFromMask($languageMask, $allLanguages); foreach ($languageCodes as $languageCode) { $id = $fieldDefinition->id; $fieldDefinitions[$contentId][$versionId][$languageCode][$id] = $fieldDefinition; diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php index d2c3830e70..13082bd798 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php @@ -181,15 +181,15 @@ public function testExtractContentFromRows() 'ezimage', 'ezdatetime', 'ezkeyword', - ], count($rowsFixture)); + ], count($rowsFixture) - 1); $mapper = new Mapper($reg, $this->getLanguageHandler(), $contentTypeHandlerMock); $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); + $expected = [$this->getContentExtractReference()]; + $this->assertEquals( - [ - $this->getContentExtractReference(), - ], + $expected, $result ); } @@ -215,17 +215,12 @@ public function testExtractContentFromRowsWithNewFieldDefinitions(): void 'ezdatetime', 'ezkeyword', 'eznumber', - ], count($rowsFixture)); + ], count($rowsFixture) - 1); $mapper = new Mapper($reg, $this->getLanguageHandler(), $contentTypeHandlerMock); $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); $expectedContent = $this->getContentExtractReference(); - $expectedContent->fields[] = new Field([ - 'type' => 'eznumber', - 'languageCode' => 'eng-GB', - 'value' => new FieldValue(), - ]); $expectedContent->fields[] = new Field([ 'type' => 'eznumber', 'languageCode' => 'eng-US', @@ -263,7 +258,7 @@ static function (Content\Type\FieldDefinition $fieldDefinition) { 'ezimage', 'ezdatetime', 'ezkeyword', - ], count($rowsFixture) - 1); + ], count($rowsFixture) - 2); $mapper = new Mapper($reg, $this->getLanguageHandler(), $contentTypeHandlerMock); $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/_fixtures/extract_content_from_rows_result.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/_fixtures/extract_content_from_rows_result.php index 55ef4634a8..b339be7d4a 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/_fixtures/extract_content_from_rows_result.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/_fixtures/extract_content_from_rows_result.php @@ -119,70 +119,70 @@ $content->fields[] = $field; -$field = new Field(); -$field->id = 4000; -$field->fieldDefinitionId = 193; -$field->type = 'ezkeyword'; -$field->value = new FieldValue(); -$field->languageCode = 'eng-GB'; -$field->versionNo = 2; - -$content->fields[] = $field; - -$field = new Field(); -$field->fieldDefinitionId = 183; -$field->type = 'ezstring'; -$field->value = new FieldValue(); -$field->languageCode = 'eng-GB'; - -$content->fields[] = $field; - -$field = new Field(); -$field->fieldDefinitionId = 184; -$field->type = 'ezstring'; -$field->value = new FieldValue(); -$field->languageCode = 'eng-GB'; - -$content->fields[] = $field; - -$field = new Field(); -$field->fieldDefinitionId = 185; -$field->type = 'ezauthor'; -$field->value = new FieldValue(); -$field->languageCode = 'eng-GB'; - -$content->fields[] = $field; - -$field = new Field(); -$field->fieldDefinitionId = 188; -$field->type = 'ezboolean'; -$field->value = new FieldValue(); -$field->languageCode = 'eng-GB'; - -$content->fields[] = $field; - -$field = new Field(); -$field->fieldDefinitionId = 189; -$field->type = 'ezimage'; -$field->value = new FieldValue(); -$field->languageCode = 'eng-GB'; - -$content->fields[] = $field; - -$field = new Field(); -$field->fieldDefinitionId = 191; -$field->type = 'ezdatetime'; -$field->value = new FieldValue(); -$field->languageCode = 'eng-GB'; - -$content->fields[] = $field; - -$field = new Field(); -$field->fieldDefinitionId = 192; -$field->type = 'ezdatetime'; -$field->value = new FieldValue(); -$field->languageCode = 'eng-GB'; - -$content->fields[] = $field; +//$field = new Field(); +//$field->id = 4000; +//$field->fieldDefinitionId = 193; +//$field->type = 'ezkeyword'; +//$field->value = new FieldValue(); +//$field->languageCode = 'eng-GB'; +//$field->versionNo = 2; +// +//$content->fields[] = $field; +// +//$field = new Field(); +//$field->fieldDefinitionId = 183; +//$field->type = 'ezstring'; +//$field->value = new FieldValue(); +//$field->languageCode = 'eng-GB'; +// +//$content->fields[] = $field; +// +//$field = new Field(); +//$field->fieldDefinitionId = 184; +//$field->type = 'ezstring'; +//$field->value = new FieldValue(); +//$field->languageCode = 'eng-GB'; +// +//$content->fields[] = $field; +// +//$field = new Field(); +//$field->fieldDefinitionId = 185; +//$field->type = 'ezauthor'; +//$field->value = new FieldValue(); +//$field->languageCode = 'eng-GB'; +// +//$content->fields[] = $field; +// +//$field = new Field(); +//$field->fieldDefinitionId = 188; +//$field->type = 'ezboolean'; +//$field->value = new FieldValue(); +//$field->languageCode = 'eng-GB'; +// +//$content->fields[] = $field; +// +//$field = new Field(); +//$field->fieldDefinitionId = 189; +//$field->type = 'ezimage'; +//$field->value = new FieldValue(); +//$field->languageCode = 'eng-GB'; +// +//$content->fields[] = $field; +// +//$field = new Field(); +//$field->fieldDefinitionId = 191; +//$field->type = 'ezdatetime'; +//$field->value = new FieldValue(); +//$field->languageCode = 'eng-GB'; +// +//$content->fields[] = $field; +// +//$field = new Field(); +//$field->fieldDefinitionId = 192; +//$field->type = 'ezdatetime'; +//$field->value = new FieldValue(); +//$field->languageCode = 'eng-GB'; +// +//$content->fields[] = $field; return $content; diff --git a/eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php b/eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php index c0ca560020..396d0be03c 100644 --- a/eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php +++ b/eZ/Publish/SPI/Tests/FieldType/BaseIntegrationTest.php @@ -445,10 +445,10 @@ public function testLoadFieldType($content) { $this->assertSame( $this->getTypeName(), - $content->fields[1]->type + $content->fields[2]->type ); - return $content->fields[1]; + return $content->fields[2]; } /** @@ -506,10 +506,10 @@ public function testUpdateFieldType($content) { $this->assertSame( $this->getTypeName(), - $content->fields[1]->type + $content->fields[2]->type ); - return $content->fields[1]; + return $content->fields[2]; } /** From bdf66e102e51aa54cb05cbffe9aa3974807d50d8 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Wed, 20 Dec 2023 12:30:28 +0100 Subject: [PATCH 05/34] fix: Revert unnecessary changes to integration test structure --- .../API/Repository/Tests/NonRedundantFieldSetTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php b/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php index b6725f6b96..e0232a703c 100644 --- a/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php +++ b/eZ/Publish/API/Repository/Tests/NonRedundantFieldSetTest.php @@ -152,7 +152,7 @@ public function testCreateContentEmptyValuesTranslationNotStored() * * @param \eZ\Publish\API\Repository\Values\Content\Content $content */ - public function testCreateContentEmptyValuesTranslationVirtualFields(Content $content): void + public function testCreateContentEmptyValuesTranslationNotStoredFields(Content $content) { $emptyValue = $this->getRepository()->getFieldTypeService()->getFieldType('ezstring')->getEmptyValue(); @@ -167,7 +167,7 @@ public function testCreateContentEmptyValuesTranslationVirtualFields(Content $co $this->assertEquals($emptyValue, $content->getFieldValue('field3', 'eng-US')); $this->assertEquals($emptyValue, $content->getFieldValue('field4', 'eng-US')); - // ger-DE is not stored + // ger-DE is not stored! $this->assertNotContains('ger-DE', $content->versionInfo->languageCodes); } @@ -262,7 +262,7 @@ public function testCreateContentTwoLanguagesSecondTranslationNotStored() * * @param \eZ\Publish\API\Repository\Values\Content\Content $content */ - public function testCreateContentTwoLanguagesSecondTranslationVirtualFields(Content $content): void + public function testCreateContentTwoLanguagesSecondTranslationNotStoredFields(Content $content) { $emptyValue = $this->getRepository()->getFieldTypeService()->getFieldType('ezstring')->getEmptyValue(); @@ -276,7 +276,7 @@ public function testCreateContentTwoLanguagesSecondTranslationVirtualFields(Cont $this->assertEquals($emptyValue, $content->getFieldValue('field3', 'eng-US')); $this->assertEquals('default value 4', $content->getFieldValue('field4', 'eng-US')); - // ger-DE is not stored + // ger-DE is not stored! $this->assertNotContains('ger-DE', $content->versionInfo->languageCodes); } From 6d8aac75f9f01d56a572f0c012d7ac57fac6b58b Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Wed, 20 Dec 2023 12:31:57 +0100 Subject: [PATCH 06/34] fix: Remove commented code --- .../extract_content_from_rows_result.php | 66 ------------------- 1 file changed, 66 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/_fixtures/extract_content_from_rows_result.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/_fixtures/extract_content_from_rows_result.php index b339be7d4a..9460576b1c 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/_fixtures/extract_content_from_rows_result.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/_fixtures/extract_content_from_rows_result.php @@ -119,70 +119,4 @@ $content->fields[] = $field; -//$field = new Field(); -//$field->id = 4000; -//$field->fieldDefinitionId = 193; -//$field->type = 'ezkeyword'; -//$field->value = new FieldValue(); -//$field->languageCode = 'eng-GB'; -//$field->versionNo = 2; -// -//$content->fields[] = $field; -// -//$field = new Field(); -//$field->fieldDefinitionId = 183; -//$field->type = 'ezstring'; -//$field->value = new FieldValue(); -//$field->languageCode = 'eng-GB'; -// -//$content->fields[] = $field; -// -//$field = new Field(); -//$field->fieldDefinitionId = 184; -//$field->type = 'ezstring'; -//$field->value = new FieldValue(); -//$field->languageCode = 'eng-GB'; -// -//$content->fields[] = $field; -// -//$field = new Field(); -//$field->fieldDefinitionId = 185; -//$field->type = 'ezauthor'; -//$field->value = new FieldValue(); -//$field->languageCode = 'eng-GB'; -// -//$content->fields[] = $field; -// -//$field = new Field(); -//$field->fieldDefinitionId = 188; -//$field->type = 'ezboolean'; -//$field->value = new FieldValue(); -//$field->languageCode = 'eng-GB'; -// -//$content->fields[] = $field; -// -//$field = new Field(); -//$field->fieldDefinitionId = 189; -//$field->type = 'ezimage'; -//$field->value = new FieldValue(); -//$field->languageCode = 'eng-GB'; -// -//$content->fields[] = $field; -// -//$field = new Field(); -//$field->fieldDefinitionId = 191; -//$field->type = 'ezdatetime'; -//$field->value = new FieldValue(); -//$field->languageCode = 'eng-GB'; -// -//$content->fields[] = $field; -// -//$field = new Field(); -//$field->fieldDefinitionId = 192; -//$field->type = 'ezdatetime'; -//$field->value = new FieldValue(); -//$field->languageCode = 'eng-GB'; -// -//$content->fields[] = $field; - return $content; From 657e809b91a9681d1111336e6924fe9ad0a42cc8 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Wed, 20 Dec 2023 14:50:31 +0100 Subject: [PATCH 07/34] fix: Code review --- .../Persistence/Legacy/Content/Mapper.php | 46 +++++++++++++------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index 6b7803a0c6..d93fd33e30 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -24,6 +24,12 @@ * Mapper for Content Handler. * * Performs mapping of Content objects. + * + * @phpstan-type TVersionedLanguageFieldDefinitionsMap array>>> + * @phpstan-type TVersionedFieldMap array>> + * @phpstan-type TVersionedNameMap array>>> + * @phpstan-type TContentInfoMap array + * @phpstan-type TVersionInfoMap array> */ class Mapper { @@ -193,13 +199,12 @@ public function extractContentFromRows( string $prefix = 'ezcontentobject_' ): array { $versionedNameData = []; - $languageCodes = []; foreach ($nameRows as $row) { $contentId = (int)$row["{$prefix}name_contentobject_id"]; $versionNo = (int)$row["{$prefix}name_content_version"]; - $versionedNameData[$contentId][$versionNo][$row["{$prefix}name_content_translation"]] = $row["{$prefix}name_name"]; - $languageCodes[] = $row["{$prefix}name_content_translation"]; + $languageCode = $row["{$prefix}name_content_translation"]; + $versionedNameData[$contentId][$versionNo][$languageCode] = $row["{$prefix}name_name"]; } $contentInfos = []; @@ -249,21 +254,27 @@ public function extractContentFromRows( } /** + * @phpstan-param TContentInfoMap $contentInfos + * @phpstan-param TVersionInfoMap $versionInfos + * @phpstan-param TVersionedFieldMap $fields + * @phpstan-param TVersionedLanguageFieldDefinitionsMap $missingFieldDefinitions + * @phpstan-param TVersionedNameMap $versionedNames + * * @return \eZ\Publish\SPI\Persistence\Content[] */ private function buildContentObjects( array $contentInfos, array $versionInfos, array $fields, - array $fieldDefinitions, - array $versionedNameData + array $missingFieldDefinitions, + array $versionedNames ): array { $results = []; foreach ($contentInfos as $contentId => $contentInfo) { foreach ($versionInfos[$contentId] as $versionId => $versionInfo) { // Fallback to just main language name if versioned name data is missing - $names = $versionedNameData[$contentId][$versionInfo->versionNo] + $names = $versionedNames[$contentId][$versionInfo->versionNo] ?? [$contentInfo->mainLanguageCode => $contentInfo->name]; $content = new Content(); @@ -272,7 +283,8 @@ private function buildContentObjects( $content->versionInfo->contentInfo = $contentInfo; $content->fields = array_values($fields[$contentId][$versionId]); - foreach ($fieldDefinitions[$contentId][$versionId] as $languageCode => $versionFieldDefinitions) { + $missingVersionFieldDefinitions = $missingFieldDefinitions[$contentId][$versionId]; + foreach ($missingVersionFieldDefinitions as $languageCode => $versionFieldDefinitions) { foreach ($versionFieldDefinitions as $fieldDefinition) { $content->fields[] = $this->createEmptyField( $fieldDefinition, @@ -289,6 +301,8 @@ private function buildContentObjects( } /** + * @phpstan-return TVersionedLanguageFieldDefinitionsMap + * * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException */ private function loadCachedVersionFieldDefinitionsPerLanguage( @@ -306,14 +320,16 @@ private function loadCachedVersionFieldDefinitionsPerLanguage( $languageMask = (int)$row["{$prefix}version_language_mask"]; if (!isset($fieldDefinitions[$contentId][$versionId])) { - $contentType = $contentTypes[$contentTypeId] = $contentTypes[$contentTypeId] - ?? $this->contentTypeHandler->load($contentTypeId); - foreach ($contentType->fieldDefinitions as $fieldDefinition) { - $languageCodes = $this->extractLanguageCodesFromMask($languageMask, $allLanguages); - foreach ($languageCodes as $languageCode) { - $id = $fieldDefinition->id; - $fieldDefinitions[$contentId][$versionId][$languageCode][$id] = $fieldDefinition; - } + continue; + } + + $languageCodes = $this->extractLanguageCodesFromMask($languageMask, $allLanguages); + $contentType = $contentTypes[$contentTypeId] = $contentTypes[$contentTypeId] + ?? $this->contentTypeHandler->load($contentTypeId); + foreach ($contentType->fieldDefinitions as $fieldDefinition) { + foreach ($languageCodes as $languageCode) { + $id = $fieldDefinition->id; + $fieldDefinitions[$contentId][$versionId][$languageCode][$id] = $fieldDefinition; } } } From edf319b6d9950743a937bff6889ba9f74f6d7564 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Wed, 20 Dec 2023 16:32:07 +0100 Subject: [PATCH 08/34] fix: Tests --- eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index d93fd33e30..cbc41cf202 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -319,7 +319,7 @@ private function loadCachedVersionFieldDefinitionsPerLanguage( $contentTypeId = (int)$row["{$prefix}contentclass_id"]; $languageMask = (int)$row["{$prefix}version_language_mask"]; - if (!isset($fieldDefinitions[$contentId][$versionId])) { + if (isset($fieldDefinitions[$contentId][$versionId])) { continue; } From 9ed8d263c3324a591d27528651d4bfe23a740ca9 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Wed, 20 Dec 2023 17:30:22 +0100 Subject: [PATCH 09/34] fix: Prevent fatal error in case of virtual field --- eZ/Publish/API/Repository/Values/Content/Field.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/eZ/Publish/API/Repository/Values/Content/Field.php b/eZ/Publish/API/Repository/Values/Content/Field.php index 83c2717df0..3ce613d4a1 100644 --- a/eZ/Publish/API/Repository/Values/Content/Field.php +++ b/eZ/Publish/API/Repository/Values/Content/Field.php @@ -24,9 +24,10 @@ class Field extends ValueObject /** * The field id. * - * @todo may be not needed + * Value of `null` indicates the field is virtual + * and is not persisted (yet). * - * @var int + * @var int|null */ protected $id; @@ -58,7 +59,7 @@ class Field extends ValueObject */ protected $fieldTypeIdentifier; - public function getId(): int + public function getId(): ?int { return $this->id; } @@ -85,4 +86,9 @@ public function getFieldTypeIdentifier(): string { return $this->fieldTypeIdentifier; } + + public function isVirtual(): bool + { + return null === $this->id; + } } From 8039b52cb956d5c9551d856e7c8c7574e709fed4 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Wed, 20 Mar 2024 10:27:14 +0100 Subject: [PATCH 10/34] fix: Do not rely on fieldDefinition->defaultValue --- .../Core/Persistence/Legacy/Content/Mapper.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index cbc41cf202..5214db116e 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -686,9 +686,19 @@ private function createEmptyField(FieldDefinition $fieldDefinition, string $lang $field = new Field(); $field->fieldDefinitionId = $fieldDefinition->id; $field->type = $fieldDefinition->fieldType; - $field->value = $fieldDefinition->defaultValue; + $field->value = $this->getDefaultValue($fieldDefinition); $field->languageCode = $languageCode; return $field; } + + private function getDefaultValue(FieldDefinition $fieldDefinition): FieldValue + { + $fieldValue = new FieldValue(); + + $converter = $this->converterRegistry->getConverter($fieldDefinition->fieldType); + $converter->toFieldValue(new StorageFieldValue(), $fieldValue); + + return $fieldValue; + } } From a5045db44710b2d499971feee03ca22ea885ceb5 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Thu, 21 Mar 2024 10:00:07 +0100 Subject: [PATCH 11/34] fix: Author field --- .../Persistence/Legacy/Content/Mapper.php | 20 ++++++++++++++++--- .../Legacy/Tests/Content/MapperTest.php | 2 +- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index 5214db116e..775dd195fa 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -694,11 +694,25 @@ private function createEmptyField(FieldDefinition $fieldDefinition, string $lang private function getDefaultValue(FieldDefinition $fieldDefinition): FieldValue { - $fieldValue = new FieldValue(); + $value = clone $fieldDefinition->defaultValue; + $storageValue = $this->getDefaultStorageValue(); $converter = $this->converterRegistry->getConverter($fieldDefinition->fieldType); - $converter->toFieldValue(new StorageFieldValue(), $fieldValue); + $converter->toStorageValue($value, $storageValue); + $converter->toFieldValue($storageValue, $value); - return $fieldValue; + return $value; + } + + private function getDefaultStorageValue(): StorageFieldValue + { + $storageValue = new StorageFieldValue(); + $storageValue->dataFloat = null; + $storageValue->dataInt = null; + $storageValue->dataText = ''; + $storageValue->sortKeyInt = 0; + $storageValue->sortKeyString = ''; + + return $storageValue; } } diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php index 13082bd798..5724492ee8 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php @@ -215,7 +215,7 @@ public function testExtractContentFromRowsWithNewFieldDefinitions(): void 'ezdatetime', 'ezkeyword', 'eznumber', - ], count($rowsFixture) - 1); + ], count($rowsFixture)); $mapper = new Mapper($reg, $this->getLanguageHandler(), $contentTypeHandlerMock); $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); From ec9552c901af608369274163cb4a2f115311e382 Mon Sep 17 00:00:00 2001 From: Slawomir Dolzycki-Uchto Date: Thu, 21 Mar 2024 10:07:43 +0100 Subject: [PATCH 12/34] fix: Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paweł Niedzielski --- .../API/Repository/Values/Content/Field.php | 3 ++ .../Legacy/Content/FieldHandler.php | 2 +- .../Persistence/Legacy/Content/Mapper.php | 35 +++++++++++++++---- .../Legacy/Content/StorageHandler.php | 2 +- .../Legacy/Tests/Content/MapperTest.php | 14 ++++---- .../Tests/Content/StorageHandlerTest.php | 2 +- 6 files changed, 43 insertions(+), 15 deletions(-) diff --git a/eZ/Publish/API/Repository/Values/Content/Field.php b/eZ/Publish/API/Repository/Values/Content/Field.php index 3ce613d4a1..a4af0a3215 100644 --- a/eZ/Publish/API/Repository/Values/Content/Field.php +++ b/eZ/Publish/API/Repository/Values/Content/Field.php @@ -87,6 +87,9 @@ public function getFieldTypeIdentifier(): string return $this->fieldTypeIdentifier; } + /** + * @phpstan-assert-if-true !null $this->getId() + */ public function isVirtual(): bool { return null === $this->id; diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php b/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php index dba486b78e..da92831cc9 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php @@ -151,7 +151,7 @@ protected function getEmptyField(FieldDefinition $fieldDefinition, $languageCode public function createExistingFieldsInNewVersion(Content $content): void { foreach ($content->fields as $field) { - if ($field->id === null) { + if ($field->getId() === null) { // Virtual field with default value, skip creating field as it has no id continue; } diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index 775dd195fa..3341cc449b 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -25,11 +25,35 @@ * * Performs mapping of Content objects. * - * @phpstan-type TVersionedLanguageFieldDefinitionsMap array>>> - * @phpstan-type TVersionedFieldMap array>> - * @phpstan-type TVersionedNameMap array>>> + * @phpstan-type TVersionedLanguageFieldDefinitionsMap array< + int, array< + int, array< + string, array< + int, \eZ\Publish\SPI\Persistence\Content\Type\FieldDefinition, + > + > + > + > + * @phpstan-type TVersionedFieldMap array< + int, array< + int, array< + int, \eZ\Publish\SPI\Persistence\Content\Field, + > + > + > + * @phpstan-type TVersionedNameMap array< + int, array< + int, array< + string, array + > + > + > * @phpstan-type TContentInfoMap array - * @phpstan-type TVersionInfoMap array> + * @phpstan-type TVersionInfoMap array< + int, array< + int, \eZ\Publish\SPI\Persistence\Content\VersionInfo, + > + > */ class Mapper { @@ -324,8 +348,7 @@ private function loadCachedVersionFieldDefinitionsPerLanguage( } $languageCodes = $this->extractLanguageCodesFromMask($languageMask, $allLanguages); - $contentType = $contentTypes[$contentTypeId] = $contentTypes[$contentTypeId] - ?? $this->contentTypeHandler->load($contentTypeId); + $contentType = $contentTypes[$contentTypeId] ??= $this->contentTypeHandler->load($contentTypeId); foreach ($contentType->fieldDefinitions as $fieldDefinition) { foreach ($languageCodes as $languageCode) { $id = $fieldDefinition->id; diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php b/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php index 4101063fad..4654779252 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php @@ -79,7 +79,7 @@ public function copyFieldData(VersionInfo $versionInfo, Field $field, Field $ori public function getFieldData(VersionInfo $versionInfo, Field $field) { $storage = $this->storageRegistry->getStorage($field->type); - if ($field->id !== null && $storage->hasFieldData()) { + if ($field->getId() !== null && $storage->hasFieldData()) { $storage->getFieldData($versionInfo, $field, $this->context); } } diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php index 5724492ee8..4923c188e3 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php @@ -264,9 +264,11 @@ static function (Content\Type\FieldDefinition $fieldDefinition) { $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); $expectedContent = $this->getContentExtractReference(); - $expectedContent->fields = array_values(array_filter($expectedContent->fields, static function (Field $field) { - return $field->fieldDefinitionId !== 185; - })); + $expectedContent->fields = array_values( + array_filter($expectedContent->fields, static function (Field $field): bool { + return $field->fieldDefinitionId !== 185; + }) + ); $this->assertEquals( [ @@ -331,8 +333,8 @@ private function getFieldRegistry( $converterMock = $this->createMock(Converter::class); $converterMock->expects( $expectedConverterCalls === null - ? self::any() - : self::exactly($expectedConverterCalls) + ? self::any() + : self::exactly($expectedConverterCalls) ) ->method('toFieldValue') ->willReturn(new FieldValue()); @@ -735,7 +737,7 @@ static function ($languageCode) use ($languages) { } /** - * @return Content\Type\Handler|\PHPUnit\Framework\MockObject\MockObject + * @return \eZ\Publish\SPI\Persistence\Content\Type\Handler&\PHPUnit\Framework\MockObject\MockObject */ protected function getContentTypeHandler() { diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php index 36586954a9..899d108913 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php @@ -153,7 +153,7 @@ public function testGetFieldDataNotAvailableForVirtualField() $storageRegistryMock->expects(self::once()) ->method('getStorage') - ->with($this->equalTo('foobar')) + ->with(self::equalTo('foobar')) ->willReturn($storageMock); $field = new Field(); From 6f0dd63b2bb47d3bfb163cdc54ece57d3f04020d Mon Sep 17 00:00:00 2001 From: Slawomir Dolzycki-Uchto Date: Thu, 21 Mar 2024 10:08:45 +0100 Subject: [PATCH 13/34] fix: Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paweł Niedzielski --- eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php index 4923c188e3..f70dc66c16 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php @@ -243,7 +243,7 @@ public function testExtractContentFromRowsWithRemovedFieldDefinitions(): void $contentType = $this->getContentTypeFromRows($rowsFixture); $contentType->fieldDefinitions = array_filter( $contentType->fieldDefinitions, - static function (Content\Type\FieldDefinition $fieldDefinition) { + static function (Content\Type\FieldDefinition $fieldDefinition): bool { // ref. fixtures, ezauthor return $fieldDefinition->id !== 185; } From c1d469108e2169edf32da304c3ff4747f9553746 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Thu, 21 Mar 2024 10:44:44 +0100 Subject: [PATCH 14/34] fix: Code review --- eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php | 2 +- eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php b/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php index da92831cc9..dba486b78e 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php @@ -151,7 +151,7 @@ protected function getEmptyField(FieldDefinition $fieldDefinition, $languageCode public function createExistingFieldsInNewVersion(Content $content): void { foreach ($content->fields as $field) { - if ($field->getId() === null) { + if ($field->id === null) { // Virtual field with default value, skip creating field as it has no id continue; } diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php b/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php index 4654779252..4101063fad 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/StorageHandler.php @@ -79,7 +79,7 @@ public function copyFieldData(VersionInfo $versionInfo, Field $field, Field $ori public function getFieldData(VersionInfo $versionInfo, Field $field) { $storage = $this->storageRegistry->getStorage($field->type); - if ($field->getId() !== null && $storage->hasFieldData()) { + if ($field->id !== null && $storage->hasFieldData()) { $storage->getFieldData($versionInfo, $field, $this->context); } } From c59e2637cd4ab75dc752115c0f0630856804a6bd Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Thu, 21 Mar 2024 10:54:37 +0100 Subject: [PATCH 15/34] fix: Cannot use PHP 7.4 features --- eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index 3341cc449b..668eb6762a 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -348,7 +348,8 @@ private function loadCachedVersionFieldDefinitionsPerLanguage( } $languageCodes = $this->extractLanguageCodesFromMask($languageMask, $allLanguages); - $contentType = $contentTypes[$contentTypeId] ??= $this->contentTypeHandler->load($contentTypeId); + $contentTypes[$contentTypeId] = $contentTypes[$contentTypeId] ?? $this->contentTypeHandler->load($contentTypeId); + $contentType = $contentTypes[$contentTypeId]; foreach ($contentType->fieldDefinitions as $fieldDefinition) { foreach ($languageCodes as $languageCode) { $id = $fieldDefinition->id; From 3fe2bebe92f481c506c3601f90ecb202a4542fcb Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Thu, 4 Apr 2024 09:08:21 +0200 Subject: [PATCH 16/34] fix: External storage --- .../Persistence/Legacy/Content/Mapper.php | 26 ++++++++++++++++--- .../storage_engines/legacy/content.yml | 1 + 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index 668eb6762a..67351e809f 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -57,6 +57,8 @@ */ class Mapper { + const EMPTY_FIELD_ID = -1; + /** * FieldValue converter registry. * @@ -76,14 +78,21 @@ class Mapper */ private $contentTypeHandler; + /** + * @var StorageRegistry + */ + private $storageRegistry; + public function __construct( Registry $converterRegistry, LanguageHandler $languageHandler, - ContentTypeHandler $contentTypeHandler + ContentTypeHandler $contentTypeHandler, + StorageRegistry $storageRegistry ) { $this->converterRegistry = $converterRegistry; $this->languageHandler = $languageHandler; $this->contentTypeHandler = $contentTypeHandler; + $this->storageRegistry = $storageRegistry; } /** @@ -310,10 +319,19 @@ private function buildContentObjects( $missingVersionFieldDefinitions = $missingFieldDefinitions[$contentId][$versionId]; foreach ($missingVersionFieldDefinitions as $languageCode => $versionFieldDefinitions) { foreach ($versionFieldDefinitions as $fieldDefinition) { - $content->fields[] = $this->createEmptyField( + $emptyField = $this->createEmptyField( + $versionInfo, $fieldDefinition, $languageCode ); + + $externalStorage = $this->storageRegistry->getStorage($fieldDefinition->fieldType); + if ($externalStorage->hasFieldData()) { + $externalStorage->getFieldData($versionInfo, $emptyField, []); + } + + $emptyField->id = null; + $content->fields[] = $emptyField; } } @@ -705,13 +723,15 @@ public function createRelationFromCreateStruct(RelationCreateStruct $struct) return $relation; } - private function createEmptyField(FieldDefinition $fieldDefinition, string $languageCode): Field + private function createEmptyField(VersionInfo $versionInfo, FieldDefinition $fieldDefinition, string $languageCode): Field { $field = new Field(); + $field->id = self::EMPTY_FIELD_ID; $field->fieldDefinitionId = $fieldDefinition->id; $field->type = $fieldDefinition->fieldType; $field->value = $this->getDefaultValue($fieldDefinition); $field->languageCode = $languageCode; + $field->versionNo = $versionInfo->versionNo; return $field; } diff --git a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml index d2efee3322..98b87859ea 100644 --- a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml +++ b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml @@ -10,6 +10,7 @@ services: - "@ezpublish.persistence.legacy.field_value_converter.registry" - "@ezpublish.spi.persistence.legacy.language.handler" - '@ezpublish.spi.persistence.legacy.content_type.handler' + - "@ezpublish.persistence.external_storage_registry" ezpublish.persistence.legacy.content.gateway.inner: class: eZ\Publish\Core\Persistence\Legacy\Content\Gateway\DoctrineDatabase From be3e6f34772962c740c15aa7b8930e83e2d7cab1 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Tue, 9 Apr 2024 10:20:59 +0200 Subject: [PATCH 17/34] fix: External Storage --- .../Persistence/Legacy/Content/Mapper.php | 66 ++------ .../storage_engines/legacy/content.yml | 8 + .../Event/Mapper/ResolveMissingFieldEvent.php | 69 ++++++++ .../Mapper/ResolveVirtualFieldSubscriber.php | 154 ++++++++++++++++++ 4 files changed, 246 insertions(+), 51 deletions(-) create mode 100644 src/contracts/Event/Mapper/ResolveMissingFieldEvent.php create mode 100644 src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index 67351e809f..b394df0216 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -16,9 +16,10 @@ use eZ\Publish\SPI\Persistence\Content\Language\Handler as LanguageHandler; use eZ\Publish\SPI\Persistence\Content\Relation; use eZ\Publish\SPI\Persistence\Content\Relation\CreateStruct as RelationCreateStruct; -use eZ\Publish\SPI\Persistence\Content\Type\FieldDefinition; use eZ\Publish\SPI\Persistence\Content\Type\Handler as ContentTypeHandler; use eZ\Publish\SPI\Persistence\Content\VersionInfo; +use Ibexa\Contracts\Core\Event\Mapper\ResolveMissingFieldEvent; +use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; /** * Mapper for Content Handler. @@ -79,20 +80,20 @@ class Mapper private $contentTypeHandler; /** - * @var StorageRegistry + * @var EventDispatcherInterface */ - private $storageRegistry; + private $eventDispatcher; public function __construct( Registry $converterRegistry, LanguageHandler $languageHandler, ContentTypeHandler $contentTypeHandler, - StorageRegistry $storageRegistry + EventDispatcherInterface $eventDispatcher ) { $this->converterRegistry = $converterRegistry; $this->languageHandler = $languageHandler; $this->contentTypeHandler = $contentTypeHandler; - $this->storageRegistry = $storageRegistry; + $this->eventDispatcher = $eventDispatcher; } /** @@ -319,19 +320,19 @@ private function buildContentObjects( $missingVersionFieldDefinitions = $missingFieldDefinitions[$contentId][$versionId]; foreach ($missingVersionFieldDefinitions as $languageCode => $versionFieldDefinitions) { foreach ($versionFieldDefinitions as $fieldDefinition) { - $emptyField = $this->createEmptyField( - $versionInfo, - $fieldDefinition, - $languageCode + $event = $this->eventDispatcher->dispatch( + new ResolveMissingFieldEvent( + $content, + $fieldDefinition, + $languageCode + ) ); - $externalStorage = $this->storageRegistry->getStorage($fieldDefinition->fieldType); - if ($externalStorage->hasFieldData()) { - $externalStorage->getFieldData($versionInfo, $emptyField, []); + $field = $event->getField(); + if ($field !== null) { + $content->fields[] = $field; } - $emptyField->id = null; - $content->fields[] = $emptyField; } } @@ -722,41 +723,4 @@ public function createRelationFromCreateStruct(RelationCreateStruct $struct) return $relation; } - - private function createEmptyField(VersionInfo $versionInfo, FieldDefinition $fieldDefinition, string $languageCode): Field - { - $field = new Field(); - $field->id = self::EMPTY_FIELD_ID; - $field->fieldDefinitionId = $fieldDefinition->id; - $field->type = $fieldDefinition->fieldType; - $field->value = $this->getDefaultValue($fieldDefinition); - $field->languageCode = $languageCode; - $field->versionNo = $versionInfo->versionNo; - - return $field; - } - - private function getDefaultValue(FieldDefinition $fieldDefinition): FieldValue - { - $value = clone $fieldDefinition->defaultValue; - $storageValue = $this->getDefaultStorageValue(); - - $converter = $this->converterRegistry->getConverter($fieldDefinition->fieldType); - $converter->toStorageValue($value, $storageValue); - $converter->toFieldValue($storageValue, $value); - - return $value; - } - - private function getDefaultStorageValue(): StorageFieldValue - { - $storageValue = new StorageFieldValue(); - $storageValue->dataFloat = null; - $storageValue->dataInt = null; - $storageValue->dataText = ''; - $storageValue->sortKeyInt = 0; - $storageValue->sortKeyString = ''; - - return $storageValue; - } } diff --git a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml index 98b87859ea..14463e9d9b 100644 --- a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml +++ b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml @@ -10,7 +10,15 @@ services: - "@ezpublish.persistence.legacy.field_value_converter.registry" - "@ezpublish.spi.persistence.legacy.language.handler" - '@ezpublish.spi.persistence.legacy.content_type.handler' + - "@event_dispatcher" + + Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber: + arguments: + - "@ezpublish.persistence.legacy.field_value_converter.registry" - "@ezpublish.persistence.external_storage_registry" + - "@ezpublish.persistence.legacy.content.gateway" + tags: + - { name: kernel.event_subscriber } ezpublish.persistence.legacy.content.gateway.inner: class: eZ\Publish\Core\Persistence\Legacy\Content\Gateway\DoctrineDatabase diff --git a/src/contracts/Event/Mapper/ResolveMissingFieldEvent.php b/src/contracts/Event/Mapper/ResolveMissingFieldEvent.php new file mode 100644 index 0000000000..c36011f24c --- /dev/null +++ b/src/contracts/Event/Mapper/ResolveMissingFieldEvent.php @@ -0,0 +1,69 @@ +content = $content; + $this->fieldDefinition = $fieldDefinition; + $this->languageCode = $languageCode; + $this->context = $context; + $this->field = null; + } + + public function getContent(): Content + { + return $this->content; + } + + public function getFieldDefinition(): FieldDefinition + { + return $this->fieldDefinition; + } + + public function getLanguageCode(): string + { + return $this->languageCode; + } + + public function getContext(): array + { + return $this->context; + } + + public function setField(?Field $field): void + { + $this->field = $field; + } + + public function getField(): ?Field + { + return $this->field; + } +} \ No newline at end of file diff --git a/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php b/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php new file mode 100644 index 0000000000..e517083058 --- /dev/null +++ b/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php @@ -0,0 +1,154 @@ +converterRegistry = $converterRegistry; + $this->storageRegistry = $storageRegistry; + $this->contentGateway = $contentGateway; + } + + public static function getSubscribedEvents(): array + { + return [ + ResolveMissingFieldEvent::class => [ + ['persistExternalStorageField', -100], + ['resolveVirtualField', 0], + ] + ]; + } + + public function resolveVirtualField(ResolveMissingFieldEvent $event): void + { + if ($event->getField()) { + return; + } + + $content = $event->getContent(); + + try { + $emptyField = $this->createEmptyField( + $content->versionInfo, + $event->getFieldDefinition(), + $event->getLanguageCode() + ); + + $event->setField($emptyField); + } catch (NotFound $exception) { + return; + } + } + + public function persistExternalStorageField(ResolveMissingFieldEvent $event): void + { + $field = $event->getField(); + + if ($field && $field->id !== null) { + // Not a virtual field + return; + } + + $fieldDefinition = $event->getFieldDefinition(); + $storage = $this->storageRegistry->getStorage($fieldDefinition->fieldType); + + if ($storage instanceof NullStorage) { + // Not an external storage + return; + } + + $content = $event->getContent(); + + $field->id = $this->contentGateway->insertNewField( + $content, + $field, + $this->getDefaultStorageValue() + ); + + $storage->getFieldData( + $content->versionInfo, + $field, + $event->getContext() + ); + + $event->setField($field); + } + + /** + * @throws NotFound + */ + private function createEmptyField( + VersionInfo $versionInfo, + FieldDefinition $fieldDefinition, + string $languageCode + ): Field { + $field = new Field(); + $field->fieldDefinitionId = $fieldDefinition->id; + $field->type = $fieldDefinition->fieldType; + $field->value = $this->getDefaultValue($fieldDefinition); + $field->languageCode = $languageCode; + $field->versionNo = $versionInfo->versionNo; + + return $field; + } + + /** + * @throws NotFound + */ + private function getDefaultValue(FieldDefinition $fieldDefinition): FieldValue + { + $value = clone $fieldDefinition->defaultValue; + $storageValue = $this->getDefaultStorageValue(); + + $converter = $this->converterRegistry->getConverter($fieldDefinition->fieldType); + $converter->toStorageValue($value, $storageValue); + $converter->toFieldValue($storageValue, $value); + + return $value; + } + + private function getDefaultStorageValue(): StorageFieldValue + { + $storageValue = new StorageFieldValue(); + $storageValue->dataFloat = null; + $storageValue->dataInt = null; + $storageValue->dataText = ''; + $storageValue->sortKeyInt = 0; + $storageValue->sortKeyString = ''; + + return $storageValue; + } +} \ No newline at end of file From 1fac46ae8b9027b5221e206566f87191ed7b560e Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Tue, 9 Apr 2024 10:29:53 +0200 Subject: [PATCH 18/34] fix: CS --- .../Persistence/Legacy/Content/Mapper.php | 7 ++- .../Legacy/Tests/Content/MapperTest.php | 48 ++++++++++++++++--- .../Event/Mapper/ResolveMissingFieldEvent.php | 12 +++-- .../Mapper/ResolveVirtualFieldSubscriber.php | 14 +++--- 4 files changed, 59 insertions(+), 22 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index b394df0216..fe319d365e 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -58,7 +58,7 @@ */ class Mapper { - const EMPTY_FIELD_ID = -1; + public const EMPTY_FIELD_ID = -1; /** * FieldValue converter registry. @@ -80,7 +80,7 @@ class Mapper private $contentTypeHandler; /** - * @var EventDispatcherInterface + * @var \Symfony\Contracts\EventDispatcher\EventDispatcherInterface */ private $eventDispatcher; @@ -329,10 +329,9 @@ private function buildContentObjects( ); $field = $event->getField(); - if ($field !== null) { + if ($field !== null) { $content->fields[] = $field; } - } } diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php index f70dc66c16..d3cf7d52a5 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php @@ -22,6 +22,8 @@ use eZ\Publish\SPI\Persistence\Content\Relation as SPIRelation; use eZ\Publish\SPI\Persistence\Content\Relation\CreateStruct as RelationCreateStruct; use eZ\Publish\SPI\Persistence\Content\VersionInfo; +use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; /** * Test case for Mapper. @@ -149,7 +151,12 @@ public function testConvertToStorageValue() $field->type = 'some-type'; $field->value = new FieldValue(); - $mapper = new Mapper($reg, $this->getLanguageHandler(), $this->getContentTypeHandler()); + $mapper = new Mapper( + $reg, + $this->getLanguageHandler(), + $this->getContentTypeHandler(), + $this->getEventDispatcher(), + ); $res = $mapper->convertToStorageValue($field); $this->assertInstanceOf( @@ -183,7 +190,12 @@ public function testExtractContentFromRows() 'ezkeyword', ], count($rowsFixture) - 1); - $mapper = new Mapper($reg, $this->getLanguageHandler(), $contentTypeHandlerMock); + $mapper = new Mapper( + $reg, + $this->getLanguageHandler(), + $contentTypeHandlerMock, + $this->getEventDispatcher() + ); $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); $expected = [$this->getContentExtractReference()]; @@ -217,7 +229,12 @@ public function testExtractContentFromRowsWithNewFieldDefinitions(): void 'eznumber', ], count($rowsFixture)); - $mapper = new Mapper($reg, $this->getLanguageHandler(), $contentTypeHandlerMock); + $mapper = new Mapper( + $reg, + $this->getLanguageHandler(), + $contentTypeHandlerMock, + $this->getEventDispatcher() + ); $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); $expectedContent = $this->getContentExtractReference(); @@ -260,7 +277,12 @@ static function (Content\Type\FieldDefinition $fieldDefinition): bool { 'ezkeyword', ], count($rowsFixture) - 2); - $mapper = new Mapper($reg, $this->getLanguageHandler(), $contentTypeHandlerMock); + $mapper = new Mapper( + $reg, + $this->getLanguageHandler(), + $contentTypeHandlerMock, + $this->getEventDispatcher() + ); $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); $expectedContent = $this->getContentExtractReference(); @@ -296,7 +318,12 @@ public function testExtractContentFromRowsMultipleVersions() $contentTypeHandlerMock = $this->getContentTypeHandler(); $contentTypeHandlerMock->method('load')->willReturn($contentType); - $mapper = new Mapper($reg, $this->getLanguageHandler(), $contentTypeHandlerMock); + $mapper = new Mapper( + $reg, + $this->getLanguageHandler(), + $contentTypeHandlerMock, + $this->getEventDispatcher() + ); $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); $this->assertCount( @@ -477,7 +504,8 @@ public function testExtractContentInfoFromRow(array $fixtures, $prefix) $mapper = new Mapper( $this->getValueConverterRegistryMock(), $this->getLanguageHandler(), - $this->getContentTypeHandler() + $this->getContentTypeHandler(), + $this->getEventDispatcher() ); self::assertEquals($contentInfoReference, $mapper->extractContentInfoFromRow($fixtures, $prefix)); } @@ -638,7 +666,8 @@ protected function getMapper($valueConverter = null) return new Mapper( $this->getValueConverterRegistryMock(), $this->getLanguageHandler(), - $this->getContentTypeHandler() + $this->getContentTypeHandler(), + $this->getEventDispatcher() ); } @@ -676,6 +705,11 @@ protected function getRelationCreateStructFixture() return $struct; } + protected function getEventDispatcher(): EventDispatcherInterface + { + return new EventDispatcher(); + } + /** * Returns a language handler mock. * diff --git a/src/contracts/Event/Mapper/ResolveMissingFieldEvent.php b/src/contracts/Event/Mapper/ResolveMissingFieldEvent.php index c36011f24c..0dc14deddd 100644 --- a/src/contracts/Event/Mapper/ResolveMissingFieldEvent.php +++ b/src/contracts/Event/Mapper/ResolveMissingFieldEvent.php @@ -1,5 +1,9 @@ field; } -} \ No newline at end of file +} diff --git a/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php b/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php index e517083058..a00ff61f7d 100644 --- a/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php +++ b/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php @@ -23,13 +23,13 @@ final class ResolveVirtualFieldSubscriber implements EventSubscriberInterface { - /** @var ConverterRegistry */ + /** @var \eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\ConverterRegistry */ private $converterRegistry; - /** @var StorageRegistry */ + /** @var \eZ\Publish\Core\Persistence\Legacy\Content\StorageRegistry */ private $storageRegistry; - /** @var ContentGateway */ + /** @var \eZ\Publish\Core\Persistence\Legacy\Content\Gateway */ private $contentGateway; public function __construct( @@ -48,7 +48,7 @@ public static function getSubscribedEvents(): array ResolveMissingFieldEvent::class => [ ['persistExternalStorageField', -100], ['resolveVirtualField', 0], - ] + ], ]; } @@ -108,7 +108,7 @@ public function persistExternalStorageField(ResolveMissingFieldEvent $event): vo } /** - * @throws NotFound + * @throws \eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\Converter\Exception\NotFound */ private function createEmptyField( VersionInfo $versionInfo, @@ -126,7 +126,7 @@ private function createEmptyField( } /** - * @throws NotFound + * @throws \eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\Converter\Exception\NotFound */ private function getDefaultValue(FieldDefinition $fieldDefinition): FieldValue { @@ -151,4 +151,4 @@ private function getDefaultStorageValue(): StorageFieldValue return $storageValue; } -} \ No newline at end of file +} From 80d6e101aa8f3518f7c8fa30fc8c5776231ec5fb Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Tue, 16 Apr 2024 09:16:19 +0200 Subject: [PATCH 19/34] fix: interface DefaultDataFieldStorage --- .../FieldType/DefaultDataFieldStorage.php | 26 +++++++++ .../Mapper/ResolveVirtualFieldSubscriber.php | 55 ++++++++++++++++++- 2 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 src/contracts/FieldType/DefaultDataFieldStorage.php diff --git a/src/contracts/FieldType/DefaultDataFieldStorage.php b/src/contracts/FieldType/DefaultDataFieldStorage.php new file mode 100644 index 0000000000..5ec92f761a --- /dev/null +++ b/src/contracts/FieldType/DefaultDataFieldStorage.php @@ -0,0 +1,26 @@ +value is a {@link \eZ\Publish\SPI\Persistence\Content\FieldValue} object. + * This value holds the data as a {@link \eZ\Publish\Core\FieldType\Value} based object, according to + * the field type (e.g. for TextLine, it will be a {@link \eZ\Publish\Core\FieldType\TextLine\Value} object). + * + * @param \eZ\Publish\SPI\Persistence\Content\VersionInfo $versionInfo + * @param \eZ\Publish\SPI\Persistence\Content\Field $field + */ + public function getDefaultFieldData(VersionInfo $versionInfo, Field $field): void; +} diff --git a/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php b/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php index a00ff61f7d..c04fe5dc9e 100644 --- a/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php +++ b/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php @@ -19,6 +19,7 @@ use eZ\Publish\SPI\Persistence\Content\Type\FieldDefinition; use eZ\Publish\SPI\Persistence\Content\VersionInfo; use Ibexa\Contracts\Core\Event\Mapper\ResolveMissingFieldEvent; +use Ibexa\Contracts\FieldType\DefaultDataFieldStorage; use Symfony\Component\EventDispatcher\EventSubscriberInterface; final class ResolveVirtualFieldSubscriber implements EventSubscriberInterface @@ -47,6 +48,7 @@ public static function getSubscribedEvents(): array return [ ResolveMissingFieldEvent::class => [ ['persistExternalStorageField', -100], + ['resolveVirtualExternalStorageField', -80], ['resolveVirtualField', 0], ], ]; @@ -73,6 +75,9 @@ public function resolveVirtualField(ResolveMissingFieldEvent $event): void } } + /** + * @throws \eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\Converter\Exception\NotFound + */ public function persistExternalStorageField(ResolveMissingFieldEvent $event): void { $field = $event->getField(); @@ -98,13 +103,61 @@ public function persistExternalStorageField(ResolveMissingFieldEvent $event): vo $this->getDefaultStorageValue() ); - $storage->getFieldData( + $result = $storage->storeFieldData( $content->versionInfo, $field, $event->getContext() ); + if ($result === true) { + $storageValue = new StorageFieldValue(); + $converter = $this->converterRegistry->getConverter($fieldDefinition->fieldType); + $converter->toStorageValue( + $field->value, + $storageValue + ); + + $this->contentGateway->updateField( + $field, + $storageValue + ); + } + + $event->setField($field); + } + + public function resolveVirtualExternalStorageField(ResolveMissingFieldEvent $event): void + { + $field = $event->getField(); + + if ($field && $field->id !== null) { + // Not a virtual field + return; + } + + $fieldDefinition = $event->getFieldDefinition(); + $storage = $this->storageRegistry->getStorage($fieldDefinition->fieldType); + + if ($storage instanceof NullStorage) { + // Not an external storage + return; + } + + if (!$storage instanceof DefaultDataFieldStorage) { + return; + } + + $content = $event->getContent(); + + $storage->getDefaultFieldData( + $content->versionInfo, + $field + ); + $event->setField($field); + + // Do not persist the external storage field + $event->stopPropagation(); } /** From 3c738c0b1cd4756c1e6565986220a7973ddd59d7 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Tue, 16 Apr 2024 09:38:15 +0200 Subject: [PATCH 20/34] fix: MapperTest --- .../Legacy/Tests/Content/MapperTest.php | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php index d3cf7d52a5..c19247656a 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php @@ -6,6 +6,9 @@ */ namespace eZ\Publish\Core\Persistence\Legacy\Tests\Content; +use eZ\Publish\Core\Persistence\Legacy\Content\Gateway; +use eZ\Publish\Core\Persistence\Legacy\Content\StorageRegistry; +use Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber; use function count; use eZ\Publish\API\Repository\Values\Content\Relation as RelationValue; use eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\Converter; @@ -227,7 +230,7 @@ public function testExtractContentFromRowsWithNewFieldDefinitions(): void 'ezdatetime', 'ezkeyword', 'eznumber', - ], count($rowsFixture)); + ], count($rowsFixture) - 1); $mapper = new Mapper( $reg, @@ -242,6 +245,7 @@ public function testExtractContentFromRowsWithNewFieldDefinitions(): void 'type' => 'eznumber', 'languageCode' => 'eng-US', 'value' => new FieldValue(), + 'versionNo' => 2, ]); $this->assertEquals( @@ -682,6 +686,10 @@ protected function getValueConverterRegistryMock() $this->valueConverterRegistryMock = $this->getMockBuilder(Registry::class) ->setMethods([]) ->getMock(); + + $this->valueConverterRegistryMock + ->method('getConverter') + ->willReturn($this->createMock(Converter::class)); } return $this->valueConverterRegistryMock; @@ -707,7 +715,16 @@ protected function getRelationCreateStructFixture() protected function getEventDispatcher(): EventDispatcherInterface { - return new EventDispatcher(); + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addSubscriber( + new ResolveVirtualFieldSubscriber( + $this->getValueConverterRegistryMock(), + new StorageRegistry([]), + $this->createMock(Gateway::class) + ) + ); + + return $eventDispatcher; } /** From 8f9296c468d475f24bd4e2fe834efa9c5a3f4174 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Tue, 16 Apr 2024 09:49:00 +0200 Subject: [PATCH 21/34] fix: register Symfony EventDispatcher outside of MVC tests --- eZ/Publish/Core/settings/events.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/eZ/Publish/Core/settings/events.yml b/eZ/Publish/Core/settings/events.yml index 89ec3dd9ab..2161ca21ca 100644 --- a/eZ/Publish/Core/settings/events.yml +++ b/eZ/Publish/Core/settings/events.yml @@ -6,3 +6,7 @@ services: eZ\Publish\Core\Repository\EventSubscriber\: resource: '../Repository/EventSubscriber/*' + + Symfony\Component\EventDispatcher\EventDispatcher: ~ + Symfony\Component\EventDispatcher\EventDispatcherInterface: '@Symfony\Component\EventDispatcher\EventDispatcher' + event_dispatcher: '@Symfony\Component\EventDispatcher\EventDispatcherInterface' From af3fd67ad51776162eda3b403665c2eb85b84c86 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Tue, 16 Apr 2024 09:54:22 +0200 Subject: [PATCH 22/34] fix: missing EventDispatcher --- .../Tests/Content/HandlerContentTest.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentTest.php b/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentTest.php index c1bdbcafcb..18ab3b06fa 100644 --- a/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentTest.php +++ b/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentTest.php @@ -13,12 +13,17 @@ use eZ\Publish\API\Repository\Values\Content\Query\SortClause; use eZ\Publish\Core\Persistence; use eZ\Publish\Core\Persistence\Legacy\Content\FieldHandler; +use eZ\Publish\Core\Persistence\Legacy\Content\Gateway; use eZ\Publish\Core\Persistence\Legacy\Content\Location\Mapper as LocationMapper; use eZ\Publish\Core\Persistence\Legacy\Content\Mapper as ContentMapper; +use eZ\Publish\Core\Persistence\Legacy\Content\StorageRegistry; use eZ\Publish\Core\Search\Legacy\Content; use eZ\Publish\Core\Search\Legacy\Content\Location\Gateway as LocationGateway; use eZ\Publish\SPI\Persistence\Content\ContentInfo; use eZ\Publish\SPI\Persistence\Content\Type; +use Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber; +use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; /** * Content Search test case for ContentSearchHandler. @@ -201,6 +206,7 @@ protected function getContentMapperMock() $this->getConverterRegistry(), $this->getLanguageHandler(), $this->getContentTypeHandler(), + $this->getEventDispatcher(), ] ) ->setMethods(['extractContentInfoFromRows']) @@ -1445,4 +1451,18 @@ public function testGetNonExistingFieldDefinition(): void $this->getContentTypeHandler()->getFieldDefinition(0, Type::STATUS_DEFINED); } + + private function getEventDispatcher(): EventDispatcherInterface + { + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addSubscriber( + new ResolveVirtualFieldSubscriber( + $this->getConverterRegistry(), + new StorageRegistry([]), + $this->createMock(Gateway::class) + ) + ); + + return $eventDispatcher; + } } From 80f09dddab22d1cae72908ff6beba5a078a7b6f5 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Tue, 16 Apr 2024 09:56:57 +0200 Subject: [PATCH 23/34] fix: Coding standards --- .../Legacy/Tests/Content/MapperTest.php | 8 ++++---- .../Legacy/Tests/Content/AbstractTestCase.php | 19 +++++++++++++++++++ .../Tests/Content/HandlerContentTest.php | 19 ------------------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php index c19247656a..68e506095e 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php @@ -6,15 +6,14 @@ */ namespace eZ\Publish\Core\Persistence\Legacy\Tests\Content; -use eZ\Publish\Core\Persistence\Legacy\Content\Gateway; -use eZ\Publish\Core\Persistence\Legacy\Content\StorageRegistry; -use Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber; use function count; use eZ\Publish\API\Repository\Values\Content\Relation as RelationValue; use eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\Converter; use eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\ConverterRegistry as Registry; +use eZ\Publish\Core\Persistence\Legacy\Content\Gateway; use eZ\Publish\Core\Persistence\Legacy\Content\Mapper; use eZ\Publish\Core\Persistence\Legacy\Content\StorageFieldValue; +use eZ\Publish\Core\Persistence\Legacy\Content\StorageRegistry; use eZ\Publish\SPI\Persistence\Content; use eZ\Publish\SPI\Persistence\Content\ContentInfo; use eZ\Publish\SPI\Persistence\Content\CreateStruct; @@ -25,6 +24,7 @@ use eZ\Publish\SPI\Persistence\Content\Relation as SPIRelation; use eZ\Publish\SPI\Persistence\Content\Relation\CreateStruct as RelationCreateStruct; use eZ\Publish\SPI\Persistence\Content\VersionInfo; +use Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; @@ -715,7 +715,7 @@ protected function getRelationCreateStructFixture() protected function getEventDispatcher(): EventDispatcherInterface { - $eventDispatcher = new EventDispatcher(); + $eventDispatcher = new EventDispatcher(); $eventDispatcher->addSubscriber( new ResolveVirtualFieldSubscriber( $this->getValueConverterRegistryMock(), diff --git a/eZ/Publish/Core/Search/Legacy/Tests/Content/AbstractTestCase.php b/eZ/Publish/Core/Search/Legacy/Tests/Content/AbstractTestCase.php index 1b795ae058..eb0ec67daf 100644 --- a/eZ/Publish/Core/Search/Legacy/Tests/Content/AbstractTestCase.php +++ b/eZ/Publish/Core/Search/Legacy/Tests/Content/AbstractTestCase.php @@ -8,12 +8,17 @@ use eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\Converter; use eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\ConverterRegistry; +use eZ\Publish\Core\Persistence\Legacy\Content\Gateway; +use eZ\Publish\Core\Persistence\Legacy\Content\StorageRegistry; use eZ\Publish\Core\Persistence\Legacy\Content\Type\Gateway\DoctrineDatabase as ContentTypeGateway; use eZ\Publish\Core\Persistence\Legacy\Content\Type\Handler as ContentTypeHandler; use eZ\Publish\Core\Persistence\Legacy\Content\Type\Mapper as ContentTypeMapper; use eZ\Publish\Core\Persistence\Legacy\Content\Type\Update\Handler as ContentTypeUpdateHandler; use eZ\Publish\Core\Persistence\Legacy\Tests\Content\LanguageAwareTestCase; use eZ\Publish\SPI\Persistence\Content\Type\Handler as SPIContentTypeHandler; +use Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber; +use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; /** * Abstract test suite for legacy search. @@ -113,4 +118,18 @@ protected function getConverterRegistry() return $this->converterRegistry; } + + protected function getEventDispatcher(): EventDispatcherInterface + { + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addSubscriber( + new ResolveVirtualFieldSubscriber( + $this->getConverterRegistry(), + new StorageRegistry([]), + $this->createMock(Gateway::class) + ) + ); + + return $eventDispatcher; + } } diff --git a/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentTest.php b/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentTest.php index 18ab3b06fa..ef77af8f2f 100644 --- a/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentTest.php +++ b/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentTest.php @@ -13,17 +13,12 @@ use eZ\Publish\API\Repository\Values\Content\Query\SortClause; use eZ\Publish\Core\Persistence; use eZ\Publish\Core\Persistence\Legacy\Content\FieldHandler; -use eZ\Publish\Core\Persistence\Legacy\Content\Gateway; use eZ\Publish\Core\Persistence\Legacy\Content\Location\Mapper as LocationMapper; use eZ\Publish\Core\Persistence\Legacy\Content\Mapper as ContentMapper; -use eZ\Publish\Core\Persistence\Legacy\Content\StorageRegistry; use eZ\Publish\Core\Search\Legacy\Content; use eZ\Publish\Core\Search\Legacy\Content\Location\Gateway as LocationGateway; use eZ\Publish\SPI\Persistence\Content\ContentInfo; use eZ\Publish\SPI\Persistence\Content\Type; -use Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber; -use Symfony\Component\EventDispatcher\EventDispatcher; -use Symfony\Component\EventDispatcher\EventDispatcherInterface; /** * Content Search test case for ContentSearchHandler. @@ -1451,18 +1446,4 @@ public function testGetNonExistingFieldDefinition(): void $this->getContentTypeHandler()->getFieldDefinition(0, Type::STATUS_DEFINED); } - - private function getEventDispatcher(): EventDispatcherInterface - { - $eventDispatcher = new EventDispatcher(); - $eventDispatcher->addSubscriber( - new ResolveVirtualFieldSubscriber( - $this->getConverterRegistry(), - new StorageRegistry([]), - $this->createMock(Gateway::class) - ) - ); - - return $eventDispatcher; - } } From 80904a945b0236f80231b8f3548f3b865fbc24a8 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Tue, 16 Apr 2024 10:11:47 +0200 Subject: [PATCH 24/34] fix: add missing EventDispatcher to Handler test --- .../Core/Search/Legacy/Tests/Content/HandlerContentSortTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentSortTest.php b/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentSortTest.php index 23d5af04cb..9d057cbe4b 100644 --- a/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentSortTest.php +++ b/eZ/Publish/Core/Search/Legacy/Tests/Content/HandlerContentSortTest.php @@ -102,6 +102,7 @@ protected function getContentMapperMock() $this->getFieldRegistry(), $this->getLanguageHandler(), $this->getContentTypeHandler(), + $this->getEventDispatcher(), ] ) ->setMethods(['extractContentInfoFromRows']) From eae6c1b65ef8c6ffc1e230d15b477bf5a63d8dba Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Wed, 17 Apr 2024 09:16:09 +0200 Subject: [PATCH 25/34] fix: replace event_dispatcher with FQCN --- eZ/Publish/Core/settings/events.yml | 4 ---- eZ/Publish/Core/settings/storage_engines/legacy/content.yml | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/eZ/Publish/Core/settings/events.yml b/eZ/Publish/Core/settings/events.yml index 2161ca21ca..89ec3dd9ab 100644 --- a/eZ/Publish/Core/settings/events.yml +++ b/eZ/Publish/Core/settings/events.yml @@ -6,7 +6,3 @@ services: eZ\Publish\Core\Repository\EventSubscriber\: resource: '../Repository/EventSubscriber/*' - - Symfony\Component\EventDispatcher\EventDispatcher: ~ - Symfony\Component\EventDispatcher\EventDispatcherInterface: '@Symfony\Component\EventDispatcher\EventDispatcher' - event_dispatcher: '@Symfony\Component\EventDispatcher\EventDispatcherInterface' diff --git a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml index 14463e9d9b..29374fafa2 100644 --- a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml +++ b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml @@ -10,7 +10,7 @@ services: - "@ezpublish.persistence.legacy.field_value_converter.registry" - "@ezpublish.spi.persistence.legacy.language.handler" - '@ezpublish.spi.persistence.legacy.content_type.handler' - - "@event_dispatcher" + - '@Symfony\Contracts\EventDispatcher\EventDispatcherInterface' Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber: arguments: From 9d104aa355a7fdd0cb6507554cf4063ab75e78aa Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Wed, 17 Apr 2024 09:41:46 +0200 Subject: [PATCH 26/34] fix: register new subscriber in test setup factory --- .../settings/tests/integration_legacy.yml | 1 + .../Mapper/ResolveVirtualFieldSubscriber.php | 40 +++++++++++-------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/eZ/Publish/Core/settings/tests/integration_legacy.yml b/eZ/Publish/Core/settings/tests/integration_legacy.yml index d656f1b935..c2a96cec18 100644 --- a/eZ/Publish/Core/settings/tests/integration_legacy.yml +++ b/eZ/Publish/Core/settings/tests/integration_legacy.yml @@ -33,6 +33,7 @@ services: - ['addSubscriber', ['@eZ\Publish\Core\Search\Common\EventSubscriber\SectionEventSubscriber']] - ['addSubscriber', ['@eZ\Publish\Core\Search\Common\EventSubscriber\TrashEventSubscriber']] - ['addSubscriber', ['@eZ\Publish\Core\Search\Common\EventSubscriber\UserEventSubscriber']] + - ['addSubscriber', ['@Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber']] Doctrine\Common\EventManager: ~ diff --git a/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php b/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php index c04fe5dc9e..c5d1d80cdb 100644 --- a/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php +++ b/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php @@ -103,26 +103,34 @@ public function persistExternalStorageField(ResolveMissingFieldEvent $event): vo $this->getDefaultStorageValue() ); - $result = $storage->storeFieldData( - $content->versionInfo, - $field, - $event->getContext() - ); - - if ($result === true) { - $storageValue = new StorageFieldValue(); - $converter = $this->converterRegistry->getConverter($fieldDefinition->fieldType); - $converter->toStorageValue( - $field->value, - $storageValue - ); - - $this->contentGateway->updateField( + if ($field->value->data !== null) { + $result = $storage->storeFieldData( + $content->versionInfo, $field, - $storageValue + [] ); + + if ($result === true) { + $storageValue = new StorageFieldValue(); + $converter = $this->converterRegistry->getConverter($fieldDefinition->fieldType); + $converter->toStorageValue( + $field->value, + $storageValue + ); + + $this->contentGateway->updateField( + $field, + $storageValue + ); + } } + $storage->getFieldData( + $content->versionInfo, + $field, + [] + ); + $event->setField($field); } From 0f7966d9a91ad63895f1433b0936bbfe58463af4 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Tue, 23 Apr 2024 13:36:46 +0200 Subject: [PATCH 27/34] fix: Code Review --- .../Persistence/Legacy/Content/Mapper.php | 50 ++- .../Legacy/Tests/Content/MapperTest.php | 39 +- .../Tests/Content/StorageHandlerTest.php | 39 +- .../Legacy/Tests/Content/AbstractTestCase.php | 2 +- .../storage_engines/legacy/content.yml | 6 +- .../Event/Mapper/ResolveMissingFieldEvent.php | 8 +- .../FieldType/DefaultDataFieldStorage.php | 5 +- .../Mapper/ResolveVirtualFieldSubscriber.php | 10 +- .../ResolveVirtualFieldSubscriberTest.php | 381 ++++++++++++++++++ 9 files changed, 455 insertions(+), 85 deletions(-) create mode 100644 tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index fe319d365e..e1880eb79a 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -27,39 +27,37 @@ * Performs mapping of Content objects. * * @phpstan-type TVersionedLanguageFieldDefinitionsMap array< - int, array< - int, array< - string, array< - int, \eZ\Publish\SPI\Persistence\Content\Type\FieldDefinition, - > - > - > - > + * int, array< + * int, array< + * string, array< + * int, \eZ\Publish\SPI\Persistence\Content\Type\FieldDefinition, + * > + * > + * > + * > * @phpstan-type TVersionedFieldMap array< - int, array< - int, array< - int, \eZ\Publish\SPI\Persistence\Content\Field, - > - > - > + * int, array< + * int, array< + * int, \eZ\Publish\SPI\Persistence\Content\Field, + * > + * > + * > * @phpstan-type TVersionedNameMap array< - int, array< - int, array< - string, array - > - > - > + * int, array< + * int, array< + * string, array + * > + * > + * > * @phpstan-type TContentInfoMap array * @phpstan-type TVersionInfoMap array< - int, array< - int, \eZ\Publish\SPI\Persistence\Content\VersionInfo, - > - > + * int, array< + * int, \eZ\Publish\SPI\Persistence\Content\VersionInfo, + * > + * > */ class Mapper { - public const EMPTY_FIELD_ID = -1; - /** * FieldValue converter registry. * diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php index 68e506095e..f4e59b9b9b 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/MapperTest.php @@ -8,6 +8,7 @@ use function count; use eZ\Publish\API\Repository\Values\Content\Relation as RelationValue; +use eZ\Publish\Core\Persistence\Legacy\Bookmark\Handler; use eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\Converter; use eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\ConverterRegistry as Registry; use eZ\Publish\Core\Persistence\Legacy\Content\Gateway; @@ -97,8 +98,8 @@ public function testCreateVersionInfoForContent() ], $versionInfo ); - $this->assertGreaterThanOrEqual($time, $versionInfo->creationDate); - $this->assertGreaterThanOrEqual($time, $versionInfo->modificationDate); + self::assertGreaterThanOrEqual($time, $versionInfo->creationDate); + self::assertGreaterThanOrEqual($time, $versionInfo->modificationDate); } /** @@ -162,7 +163,7 @@ public function testConvertToStorageValue() ); $res = $mapper->convertToStorageValue($field); - $this->assertInstanceOf( + self::assertInstanceOf( StorageFieldValue::class, $res ); @@ -203,7 +204,7 @@ public function testExtractContentFromRows() $expected = [$this->getContentExtractReference()]; - $this->assertEquals( + self::assertEquals( $expected, $result ); @@ -248,7 +249,7 @@ public function testExtractContentFromRowsWithNewFieldDefinitions(): void 'versionNo' => 2, ]); - $this->assertEquals( + self::assertEquals( [ $expectedContent, ], @@ -296,7 +297,7 @@ static function (Content\Type\FieldDefinition $fieldDefinition): bool { }) ); - $this->assertEquals( + self::assertEquals( [ $expectedContent, ], @@ -330,25 +331,25 @@ public function testExtractContentFromRowsMultipleVersions() ); $result = $mapper->extractContentFromRows($rowsFixture, $nameRowsFixture); - $this->assertCount( + self::assertCount( 2, $result ); - $this->assertEquals( + self::assertEquals( 11, $result[0]->versionInfo->contentInfo->id ); - $this->assertEquals( + self::assertEquals( 11, $result[1]->versionInfo->contentInfo->id ); - $this->assertEquals( + self::assertEquals( 1, $result[0]->versionInfo->versionNo ); - $this->assertEquals( + self::assertEquals( 2, $result[1]->versionInfo->versionNo ); @@ -390,7 +391,7 @@ public function testCreateCreateStructFromContent() $struct = $mapper->createCreateStructFromContent($content); - $this->assertInstanceOf(CreateStruct::class, $struct); + self::assertInstanceOf(CreateStruct::class, $struct); return [ 'original' => $content, @@ -429,7 +430,7 @@ public function testCreateCreateStructFromContentBasicProperties($data) */ public function testCreateCreateStructFromContentParentLocationsEmpty($data) { - $this->assertEquals( + self::assertEquals( [], $data['result']->locations ); @@ -441,7 +442,7 @@ public function testCreateCreateStructFromContentParentLocationsEmpty($data) */ public function testCreateCreateStructFromContentFieldCount($data) { - $this->assertEquals( + self::assertEquals( count($data['original']->fields), count($data['result']->fields) ); @@ -454,7 +455,7 @@ public function testCreateCreateStructFromContentFieldCount($data) public function testCreateCreateStructFromContentFieldsNoId($data) { foreach ($data['result']->fields as $field) { - $this->assertNull($field->id); + self::assertNull($field->id); } } @@ -466,7 +467,7 @@ public function testExtractRelationsFromRows() $res = $mapper->extractRelationsFromRows($rows); - $this->assertEquals( + self::assertEquals( $this->getRelationExtractReference(), $res ); @@ -485,7 +486,7 @@ public function testCreateCreateStructFromContentWithPreserveOriginalLanguage() $struct = $mapper->createCreateStructFromContent($content, true); - $this->assertInstanceOf(CreateStruct::class, $struct); + self::assertInstanceOf(CreateStruct::class, $struct); $this->assertStructsEqual($content->versionInfo->contentInfo, $struct, ['sectionId', 'ownerId']); self::assertNotEquals($content->versionInfo->contentInfo->remoteId, $struct->remoteId); self::assertSame($content->versionInfo->contentInfo->contentTypeId, $struct->typeId); @@ -719,7 +720,7 @@ protected function getEventDispatcher(): EventDispatcherInterface $eventDispatcher->addSubscriber( new ResolveVirtualFieldSubscriber( $this->getValueConverterRegistryMock(), - new StorageRegistry([]), + $this->createMock(StorageRegistry::class), $this->createMock(Gateway::class) ) ); @@ -790,7 +791,7 @@ static function ($languageCode) use ($languages) { /** * @return \eZ\Publish\SPI\Persistence\Content\Type\Handler&\PHPUnit\Framework\MockObject\MockObject */ - protected function getContentTypeHandler() + protected function getContentTypeHandler(): Content\Type\Handler { return $this->createMock(Content\Type\Handler::class); } diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php index 899d108913..72b6ea4598 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/StorageHandlerTest.php @@ -16,6 +16,8 @@ /** * Test case for Content Handler. + * + * @covers \eZ\Publish\Core\Persistence\Legacy\Content\StorageHandler */ class StorageHandlerTest extends TestCase { @@ -47,10 +49,7 @@ class StorageHandlerTest extends TestCase */ protected $versionInfoMock; - /** - * @covers \eZ\Publish\Core\Persistence\Legacy\Content\StorageHandler::storeFieldData - */ - public function testStoreFieldData() + public function testStoreFieldData(): void { $storageMock = $this->getStorageMock(); $storageRegistryMock = $this->getStorageRegistryMock(); @@ -76,10 +75,7 @@ public function testStoreFieldData() $handler->storeFieldData($this->getVersionInfoMock(), $field); } - /** - * @covers \eZ\Publish\Core\Persistence\Legacy\Content\StorageHandler::getFieldData - */ - public function testGetFieldDataAvailable() + public function testGetFieldDataAvailable(): void { $storageMock = $this->getStorageMock(); $storageRegistryMock = $this->getStorageRegistryMock(); @@ -109,10 +105,7 @@ public function testGetFieldDataAvailable() $handler->getFieldData($this->getVersionInfoMock(), $field); } - /** - * @covers \eZ\Publish\Core\Persistence\Legacy\Content\StorageHandler::getFieldData - */ - public function testGetFieldDataNotAvailable() + public function testGetFieldDataNotAvailable(): void { $storageMock = $this->getStorageMock(); $storageRegistryMock = $this->getStorageRegistryMock(); @@ -137,10 +130,7 @@ public function testGetFieldDataNotAvailable() $handler->getFieldData($this->getVersionInfoMock(), $field); } - /** - * @covers \eZ\Publish\Core\Persistence\Legacy\Content\StorageHandler::getFieldData - */ - public function testGetFieldDataNotAvailableForVirtualField() + public function testGetFieldDataNotAvailableForVirtualField(): void { $storageMock = $this->getStorageMock(); $storageRegistryMock = $this->getStorageRegistryMock(); @@ -164,10 +154,7 @@ public function testGetFieldDataNotAvailableForVirtualField() $handler->getFieldData($this->getVersionInfoMock(), $field); } - /** - * @covers \eZ\Publish\Core\Persistence\Legacy\Content\StorageHandler::deleteFieldData - */ - public function testDeleteFieldData() + public function testDeleteFieldData(): void { $storageMock = $this->getStorageMock(); $storageRegistryMock = $this->getStorageRegistryMock(); @@ -194,7 +181,7 @@ public function testDeleteFieldData() * * @return \eZ\Publish\Core\Persistence\Legacy\Content\StorageHandler */ - protected function getStorageHandler() + protected function getStorageHandler(): StorageHandler { if (!isset($this->storageHandler)) { $this->storageHandler = new StorageHandler( @@ -209,9 +196,9 @@ protected function getStorageHandler() /** * Returns a context mock. * - * @return array + * @return int[] */ - protected function getContextMock() + protected function getContextMock(): array { return [23, 42]; } @@ -221,7 +208,7 @@ protected function getContextMock() * * @return \eZ\Publish\Core\Persistence\Legacy\Content\StorageRegistry */ - protected function getStorageRegistryMock() + protected function getStorageRegistryMock(): StorageRegistry { if (!isset($this->storageRegistryMock)) { $this->storageRegistryMock = $this->getMockBuilder(StorageRegistry::class) @@ -238,7 +225,7 @@ protected function getStorageRegistryMock() * * @return \eZ\Publish\SPI\FieldType\FieldStorage */ - protected function getStorageMock() + protected function getStorageMock(): FieldStorage { if (!isset($this->storageMock)) { $this->storageMock = $this->createMock(FieldStorage::class); @@ -247,7 +234,7 @@ protected function getStorageMock() return $this->storageMock; } - protected function getVersionInfoMock() + protected function getVersionInfoMock(): VersionInfo { if (!isset($this->versionInfoMock)) { $this->versionInfoMock = $this->createMock(VersionInfo::class); diff --git a/eZ/Publish/Core/Search/Legacy/Tests/Content/AbstractTestCase.php b/eZ/Publish/Core/Search/Legacy/Tests/Content/AbstractTestCase.php index eb0ec67daf..ab25ca43a7 100644 --- a/eZ/Publish/Core/Search/Legacy/Tests/Content/AbstractTestCase.php +++ b/eZ/Publish/Core/Search/Legacy/Tests/Content/AbstractTestCase.php @@ -125,7 +125,7 @@ protected function getEventDispatcher(): EventDispatcherInterface $eventDispatcher->addSubscriber( new ResolveVirtualFieldSubscriber( $this->getConverterRegistry(), - new StorageRegistry([]), + $this->createMock(StorageRegistry::class), $this->createMock(Gateway::class) ) ); diff --git a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml index 29374fafa2..f7dd8b7dd0 100644 --- a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml +++ b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml @@ -14,9 +14,9 @@ services: Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber: arguments: - - "@ezpublish.persistence.legacy.field_value_converter.registry" - - "@ezpublish.persistence.external_storage_registry" - - "@ezpublish.persistence.legacy.content.gateway" + $converterRegistry: "@ezpublish.persistence.legacy.field_value_converter.registry" + $storageRegistry: "@ezpublish.persistence.external_storage_registry" + $contentGateway: "@ezpublish.persistence.legacy.content.gateway" tags: - { name: kernel.event_subscriber } diff --git a/src/contracts/Event/Mapper/ResolveMissingFieldEvent.php b/src/contracts/Event/Mapper/ResolveMissingFieldEvent.php index 0dc14deddd..af0f0c2282 100644 --- a/src/contracts/Event/Mapper/ResolveMissingFieldEvent.php +++ b/src/contracts/Event/Mapper/ResolveMissingFieldEvent.php @@ -22,12 +22,15 @@ final class ResolveMissingFieldEvent extends Event /** @var string */ private $languageCode; - /** @var array */ + /** @var array */ private $context; /** @var \eZ\Publish\SPI\Persistence\Content\Field|null */ private $field; + /** + * @param array $context + */ public function __construct( Content $content, FieldDefinition $fieldDefinition, @@ -56,6 +59,9 @@ public function getLanguageCode(): string return $this->languageCode; } + /** + * @return array + */ public function getContext(): array { return $this->context; diff --git a/src/contracts/FieldType/DefaultDataFieldStorage.php b/src/contracts/FieldType/DefaultDataFieldStorage.php index 5ec92f761a..7c8ae9967a 100644 --- a/src/contracts/FieldType/DefaultDataFieldStorage.php +++ b/src/contracts/FieldType/DefaultDataFieldStorage.php @@ -6,7 +6,7 @@ */ declare(strict_types=1); -namespace Ibexa\Contracts\FieldType; +namespace Ibexa\Contracts\Core\FieldType; use eZ\Publish\SPI\Persistence\Content\Field; use eZ\Publish\SPI\Persistence\Content\VersionInfo; @@ -18,9 +18,6 @@ interface DefaultDataFieldStorage * $field->value is a {@link \eZ\Publish\SPI\Persistence\Content\FieldValue} object. * This value holds the data as a {@link \eZ\Publish\Core\FieldType\Value} based object, according to * the field type (e.g. for TextLine, it will be a {@link \eZ\Publish\Core\FieldType\TextLine\Value} object). - * - * @param \eZ\Publish\SPI\Persistence\Content\VersionInfo $versionInfo - * @param \eZ\Publish\SPI\Persistence\Content\Field $field */ public function getDefaultFieldData(VersionInfo $versionInfo, Field $field): void; } diff --git a/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php b/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php index c5d1d80cdb..2443586f34 100644 --- a/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php +++ b/src/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriber.php @@ -19,7 +19,7 @@ use eZ\Publish\SPI\Persistence\Content\Type\FieldDefinition; use eZ\Publish\SPI\Persistence\Content\VersionInfo; use Ibexa\Contracts\Core\Event\Mapper\ResolveMissingFieldEvent; -use Ibexa\Contracts\FieldType\DefaultDataFieldStorage; +use Ibexa\Contracts\Core\FieldType\DefaultDataFieldStorage; use Symfony\Component\EventDispatcher\EventSubscriberInterface; final class ResolveVirtualFieldSubscriber implements EventSubscriberInterface @@ -82,7 +82,7 @@ public function persistExternalStorageField(ResolveMissingFieldEvent $event): vo { $field = $event->getField(); - if ($field && $field->id !== null) { + if ($field !== null && $field->id !== null) { // Not a virtual field return; } @@ -138,7 +138,7 @@ public function resolveVirtualExternalStorageField(ResolveMissingFieldEvent $eve { $field = $event->getField(); - if ($field && $field->id !== null) { + if ($field !== null && $field->id !== null) { // Not a virtual field return; } @@ -169,7 +169,7 @@ public function resolveVirtualExternalStorageField(ResolveMissingFieldEvent $eve } /** - * @throws \eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\Converter\Exception\NotFound + * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException */ private function createEmptyField( VersionInfo $versionInfo, @@ -187,7 +187,7 @@ private function createEmptyField( } /** - * @throws \eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\Converter\Exception\NotFound + * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException */ private function getDefaultValue(FieldDefinition $fieldDefinition): FieldValue { diff --git a/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php b/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php new file mode 100644 index 0000000000..b091612861 --- /dev/null +++ b/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php @@ -0,0 +1,381 @@ +getVersionInfo(); + + $content = new Content(); + $content->versionInfo = $versionInfo; + $content->fields = []; + + return $content; + } + + private function getVersionInfo(): VersionInfo + { + $versionInfo = new VersionInfo(); + $versionInfo->versionNo = 123; + + return $versionInfo; + } + + public function testResolveVirtualField(): void + { + $converterRegistry = $this->createMock(ConverterRegistry::class); + $converterRegistry->method('getConverter') + ->willReturn($this->createMock(Converter::class)); + + $storageRegistry = $this->createMock(StorageRegistry::class); + $storageRegistry->method('getStorage') + ->willReturn(new NullStorage()); + + $contentGateway = $this->createMock(ContentGateway::class); + $contentGateway->expects($this->never()) + ->method('insertNewField'); + + $eventDispatcher = new TraceableEventDispatcher( + new EventDispatcher(), + new Stopwatch() + ); + + $eventDispatcher->addSubscriber( + new ResolveVirtualFieldSubscriber( + $converterRegistry, + $storageRegistry, + $contentGateway + ) + ); + + $content = $this->getContent(); + $fieldDefinition = new FieldDefinition([ + 'id' => 123, + 'identifier' => 'example_field', + 'fieldType' => 'some_type', + 'defaultValue' => new Content\FieldValue(), + ]); + + $event = new ResolveMissingFieldEvent( + $content, + $fieldDefinition, + 'eng-GB' + ); + + $event = $eventDispatcher->dispatch($event); + + $expected = new Content\Field([ + 'id' => null, + 'fieldDefinitionId' => 123, + 'type' => 'some_type', + 'value' => new Content\FieldValue(), + 'languageCode' => 'eng-GB', + 'versionNo' => 123, + ]); + + self::assertEquals( + $expected, + $event->getField() + ); + + self::assertCount(3, $eventDispatcher->getCalledListeners()); + self::assertEquals( + [ + 'Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber::resolveVirtualField', + 'Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber::resolveVirtualExternalStorageField', + 'Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber::persistExternalStorageField', + ], + array_column($eventDispatcher->getCalledListeners(), 'pretty') + ); + } + + public function testResolveVirtualExternalStorageField(): void + { + $converterRegistry = $this->createMock(ConverterRegistry::class); + $converterRegistry->method('getConverter') + ->willReturn($this->createMock(Converter::class)); + + $storageRegistry = $this->createMock(StorageRegistry::class); + $storageRegistry->method('getStorage') + ->willReturn(new class() implements FieldStorage, DefaultDataFieldStorage { + public function getDefaultFieldData(VersionInfo $versionInfo, Field $field): void + { + $field->value->externalData = [ + 'some_default' => 'external_data', + ]; + } + + public function storeFieldData(VersionInfo $versionInfo, Field $field, array $context): void + { + } + + public function getFieldData(VersionInfo $versionInfo, Field $field, array $context): void + { + } + + public function deleteFieldData(VersionInfo $versionInfo, array $fieldIds, array $context): void + { + } + + public function hasFieldData(): void + { + } + + public function getIndexData(VersionInfo $versionInfo, Field $field, array $context): void + { + } + }); + + $eventDispatcher = new TraceableEventDispatcher( + new EventDispatcher(), + new Stopwatch() + ); + + $eventDispatcher->addSubscriber( + new ResolveVirtualFieldSubscriber( + $converterRegistry, + $storageRegistry, + $this->createMock(ContentGateway::class) + ) + ); + + $content = $this->getContent(); + $fieldDefinition = new FieldDefinition([ + 'id' => 123, + 'identifier' => 'example_field', + 'fieldType' => 'external_type_virtual', + 'defaultValue' => new Content\FieldValue(), + ]); + + $event = new ResolveMissingFieldEvent( + $content, + $fieldDefinition, + 'eng-GB' + ); + + $event = $eventDispatcher->dispatch($event); + + $expected = new Content\Field([ + 'id' => null, + 'fieldDefinitionId' => 123, + 'type' => 'external_type_virtual', + 'value' => new Content\FieldValue([ + 'externalData' => [ + 'some_default' => 'external_data', + ], + ]), + 'languageCode' => 'eng-GB', + 'versionNo' => 123, + ]); + + self::assertEquals( + $expected, + $event->getField() + ); + + self::assertCount(1, $eventDispatcher->getNotCalledListeners()); + self::assertEquals( + 'Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber::persistExternalStorageField', + $eventDispatcher->getNotCalledListeners()[0]['pretty'] + ); + } + + public function testPersistEmptyExternalStorageField(): void + { + $converterRegistry = $this->createMock(ConverterRegistry::class); + $converterRegistry->method('getConverter') + ->willReturn($this->createMock(Converter::class)); + + $storage = $this->createMock(FieldStorage::class); + $storage->expects($this->never()) + ->method('storeFieldData'); + + $storage->expects($this->once()) + ->method('getFieldData') + ->willReturnCallback(static function (VersionInfo $versionInfo, Field $field) { + $field->value->externalData = [ + 'some_default' => 'external_data', + ]; + }); + + $storageRegistry = $this->createMock(StorageRegistry::class); + $storageRegistry->method('getStorage') + ->willReturn($storage); + + $contentGateway = $this->createMock(ContentGateway::class); + $contentGateway->expects($this->once()) + ->method('insertNewField') + ->willReturn(567); + + $eventDispatcher = new TraceableEventDispatcher( + new EventDispatcher(), + new Stopwatch() + ); + + $eventDispatcher->addSubscriber( + new ResolveVirtualFieldSubscriber( + $converterRegistry, + $storageRegistry, + $contentGateway, + ) + ); + + $content = $this->getContent(); + $fieldDefinition = new FieldDefinition([ + 'id' => 123, + 'identifier' => 'example_field', + 'fieldType' => 'external_type', + 'defaultValue' => new Content\FieldValue(), + ]); + + $event = new ResolveMissingFieldEvent( + $content, + $fieldDefinition, + 'eng-GB' + ); + + $event = $eventDispatcher->dispatch($event); + + $expected = new Content\Field([ + 'id' => 567, + 'fieldDefinitionId' => 123, + 'type' => 'external_type', + 'value' => new Content\FieldValue([ + 'externalData' => [ + 'some_default' => 'external_data', + ], + ]), + 'languageCode' => 'eng-GB', + 'versionNo' => 123, + ]); + + self::assertEquals( + $expected, + $event->getField() + ); + + self::assertCount(3, $eventDispatcher->getCalledListeners()); + self::assertEquals( + [ + 'Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber::resolveVirtualField', + 'Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber::resolveVirtualExternalStorageField', + 'Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber::persistExternalStorageField', + ], + array_column($eventDispatcher->getCalledListeners(), 'pretty') + ); + } + + public function testPersistExternalStorageField(): void + { + $converterRegistry = $this->createMock(ConverterRegistry::class); + $converterRegistry->method('getConverter') + ->willReturn($this->createMock(Converter::class)); + + $storage = $this->createMock(FieldStorage::class); + $storage->expects($this->once()) + ->method('storeFieldData') + ->willReturnCallback(static function (VersionInfo $versionInfo, Field $field) { + $field->value->externalData = $field->value->data; + }); + + $storage->expects($this->once()) + ->method('getFieldData'); + + $storageRegistry = $this->createMock(StorageRegistry::class); + $storageRegistry->method('getStorage') + ->willReturn($storage); + + $contentGateway = $this->createMock(ContentGateway::class); + $contentGateway->expects($this->once()) + ->method('insertNewField') + ->willReturn(456); + + $eventDispatcher = new TraceableEventDispatcher( + new EventDispatcher(), + new Stopwatch() + ); + + $eventDispatcher->addSubscriber( + new ResolveVirtualFieldSubscriber( + $converterRegistry, + $storageRegistry, + $contentGateway, + ) + ); + + $content = $this->getContent(); + $fieldDefinition = new FieldDefinition([ + 'id' => 123, + 'identifier' => 'example_field', + 'fieldType' => 'external_type', + 'defaultValue' => new Content\FieldValue([ + 'data' => ['some_data' => 'to_be_stored'], + ]), + ]); + + $event = new ResolveMissingFieldEvent( + $content, + $fieldDefinition, + 'eng-GB' + ); + + $event = $eventDispatcher->dispatch($event); + + $expected = new Content\Field([ + 'id' => 456, + 'fieldDefinitionId' => 123, + 'type' => 'external_type', + 'value' => new Content\FieldValue([ + 'data' => [ + 'some_data' => 'to_be_stored', + ], + 'externalData' => [ + 'some_data' => 'to_be_stored', + ], + ]), + 'languageCode' => 'eng-GB', + 'versionNo' => 123, + ]); + + self::assertEquals( + $expected, + $event->getField() + ); + + self::assertCount(3, $eventDispatcher->getCalledListeners()); + self::assertEquals( + [ + 'Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber::resolveVirtualField', + 'Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber::resolveVirtualExternalStorageField', + 'Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber::persistExternalStorageField', + ], + array_column($eventDispatcher->getCalledListeners(), 'pretty') + ); + } +} From 69acebf492f8ab04dd29c8278bcc4a1e19cf22c2 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Tue, 23 Apr 2024 14:45:50 +0200 Subject: [PATCH 28/34] fix: Reduce code duplication --- .../ResolveVirtualFieldSubscriberTest.php | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php b/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php index b091612861..9a4cae3f2b 100644 --- a/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php +++ b/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php @@ -61,11 +61,7 @@ public function testResolveVirtualField(): void $contentGateway->expects($this->never()) ->method('insertNewField'); - $eventDispatcher = new TraceableEventDispatcher( - new EventDispatcher(), - new Stopwatch() - ); - + $eventDispatcher = $this->getEventDispatcher(); $eventDispatcher->addSubscriber( new ResolveVirtualFieldSubscriber( $converterRegistry, @@ -123,6 +119,7 @@ public function testResolveVirtualExternalStorageField(): void $storageRegistry = $this->createMock(StorageRegistry::class); $storageRegistry->method('getStorage') + // Multiple interface mocks are deprecated in PHPUnit 9+ ->willReturn(new class() implements FieldStorage, DefaultDataFieldStorage { public function getDefaultFieldData(VersionInfo $versionInfo, Field $field): void { @@ -133,42 +130,47 @@ public function getDefaultFieldData(VersionInfo $versionInfo, Field $field): voi public function storeFieldData(VersionInfo $versionInfo, Field $field, array $context): void { + // Mock } public function getFieldData(VersionInfo $versionInfo, Field $field, array $context): void { + // Mock } public function deleteFieldData(VersionInfo $versionInfo, array $fieldIds, array $context): void { + // Mock } public function hasFieldData(): void { + // Mock } public function getIndexData(VersionInfo $versionInfo, Field $field, array $context): void { + // Mock } }); - $eventDispatcher = new TraceableEventDispatcher( - new EventDispatcher(), - new Stopwatch() - ); + $contentGateway = $this->createMock(ContentGateway::class); + $contentGateway->expects($this->never()) + ->method('insertNewField'); + $eventDispatcher = $this->getEventDispatcher(); $eventDispatcher->addSubscriber( new ResolveVirtualFieldSubscriber( $converterRegistry, $storageRegistry, - $this->createMock(ContentGateway::class) + $contentGateway ) ); $content = $this->getContent(); $fieldDefinition = new FieldDefinition([ - 'id' => 123, - 'identifier' => 'example_field', + 'id' => 678, + 'identifier' => 'example_external_field', 'fieldType' => 'external_type_virtual', 'defaultValue' => new Content\FieldValue(), ]); @@ -183,7 +185,7 @@ public function getIndexData(VersionInfo $versionInfo, Field $field, array $cont $expected = new Content\Field([ 'id' => null, - 'fieldDefinitionId' => 123, + 'fieldDefinitionId' => 678, 'type' => 'external_type_virtual', 'value' => new Content\FieldValue([ 'externalData' => [ @@ -233,11 +235,7 @@ public function testPersistEmptyExternalStorageField(): void ->method('insertNewField') ->willReturn(567); - $eventDispatcher = new TraceableEventDispatcher( - new EventDispatcher(), - new Stopwatch() - ); - + $eventDispatcher = $this->getEventDispatcher(); $eventDispatcher->addSubscriber( new ResolveVirtualFieldSubscriber( $converterRegistry, @@ -316,11 +314,7 @@ public function testPersistExternalStorageField(): void ->method('insertNewField') ->willReturn(456); - $eventDispatcher = new TraceableEventDispatcher( - new EventDispatcher(), - new Stopwatch() - ); - + $eventDispatcher = $this->getEventDispatcher(); $eventDispatcher->addSubscriber( new ResolveVirtualFieldSubscriber( $converterRegistry, @@ -378,4 +372,12 @@ public function testPersistExternalStorageField(): void array_column($eventDispatcher->getCalledListeners(), 'pretty') ); } + + private function getEventDispatcher(): TraceableEventDispatcher + { + return new TraceableEventDispatcher( + new EventDispatcher(), + new Stopwatch() + ); + } } From 84a16b0d5c0c093314f53b80ff2ea59b555ee933 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Thu, 25 Apr 2024 09:37:01 +0200 Subject: [PATCH 29/34] fix: Reorganize subscriber test --- .../ResolveVirtualFieldSubscriberTest.php | 227 +++++++++--------- 1 file changed, 108 insertions(+), 119 deletions(-) diff --git a/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php b/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php index 9a4cae3f2b..9f7fafff27 100644 --- a/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php +++ b/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php @@ -28,30 +28,9 @@ final class ResolveVirtualFieldSubscriberTest extends TestCase { - private function getContent(): Content - { - $versionInfo = $this->getVersionInfo(); - - $content = new Content(); - $content->versionInfo = $versionInfo; - $content->fields = []; - - return $content; - } - - private function getVersionInfo(): VersionInfo - { - $versionInfo = new VersionInfo(); - $versionInfo->versionNo = 123; - - return $versionInfo; - } - public function testResolveVirtualField(): void { - $converterRegistry = $this->createMock(ConverterRegistry::class); - $converterRegistry->method('getConverter') - ->willReturn($this->createMock(Converter::class)); + $converterRegistry = $this->getConverterRegistry(); $storageRegistry = $this->createMock(StorageRegistry::class); $storageRegistry->method('getStorage') @@ -61,31 +40,25 @@ public function testResolveVirtualField(): void $contentGateway->expects($this->never()) ->method('insertNewField'); - $eventDispatcher = $this->getEventDispatcher(); - $eventDispatcher->addSubscriber( - new ResolveVirtualFieldSubscriber( - $converterRegistry, - $storageRegistry, - $contentGateway - ) + $eventDispatcher = $this->getEventDispatcher( + $converterRegistry, + $storageRegistry, + $contentGateway ); - $content = $this->getContent(); - $fieldDefinition = new FieldDefinition([ - 'id' => 123, - 'identifier' => 'example_field', - 'fieldType' => 'some_type', - 'defaultValue' => new Content\FieldValue(), - ]); - - $event = new ResolveMissingFieldEvent( - $content, - $fieldDefinition, - 'eng-GB' + $event = $eventDispatcher->dispatch( + new ResolveMissingFieldEvent( + $this->getContent(), + new FieldDefinition([ + 'id' => 123, + 'identifier' => 'example_field', + 'fieldType' => 'some_type', + 'defaultValue' => new Content\FieldValue(), + ]), + 'eng-GB' + ) ); - $event = $eventDispatcher->dispatch($event); - $expected = new Content\Field([ 'id' => null, 'fieldDefinitionId' => 123, @@ -113,9 +86,7 @@ public function testResolveVirtualField(): void public function testResolveVirtualExternalStorageField(): void { - $converterRegistry = $this->createMock(ConverterRegistry::class); - $converterRegistry->method('getConverter') - ->willReturn($this->createMock(Converter::class)); + $converterRegistry = $this->getConverterRegistry(); $storageRegistry = $this->createMock(StorageRegistry::class); $storageRegistry->method('getStorage') @@ -158,31 +129,25 @@ public function getIndexData(VersionInfo $versionInfo, Field $field, array $cont $contentGateway->expects($this->never()) ->method('insertNewField'); - $eventDispatcher = $this->getEventDispatcher(); - $eventDispatcher->addSubscriber( - new ResolveVirtualFieldSubscriber( - $converterRegistry, - $storageRegistry, - $contentGateway - ) + $eventDispatcher = $this->getEventDispatcher( + $converterRegistry, + $storageRegistry, + $contentGateway ); - $content = $this->getContent(); - $fieldDefinition = new FieldDefinition([ - 'id' => 678, - 'identifier' => 'example_external_field', - 'fieldType' => 'external_type_virtual', - 'defaultValue' => new Content\FieldValue(), - ]); - - $event = new ResolveMissingFieldEvent( - $content, - $fieldDefinition, - 'eng-GB' + $event = $eventDispatcher->dispatch( + new ResolveMissingFieldEvent( + $this->getContent(), + new FieldDefinition([ + 'id' => 678, + 'identifier' => 'example_external_field', + 'fieldType' => 'external_type_virtual', + 'defaultValue' => new Content\FieldValue(), + ]), + 'eng-GB' + ) ); - $event = $eventDispatcher->dispatch($event); - $expected = new Content\Field([ 'id' => null, 'fieldDefinitionId' => 678, @@ -210,9 +175,7 @@ public function getIndexData(VersionInfo $versionInfo, Field $field, array $cont public function testPersistEmptyExternalStorageField(): void { - $converterRegistry = $this->createMock(ConverterRegistry::class); - $converterRegistry->method('getConverter') - ->willReturn($this->createMock(Converter::class)); + $converterRegistry = $this->getConverterRegistry(); $storage = $this->createMock(FieldStorage::class); $storage->expects($this->never()) @@ -235,31 +198,25 @@ public function testPersistEmptyExternalStorageField(): void ->method('insertNewField') ->willReturn(567); - $eventDispatcher = $this->getEventDispatcher(); - $eventDispatcher->addSubscriber( - new ResolveVirtualFieldSubscriber( - $converterRegistry, - $storageRegistry, - $contentGateway, - ) + $eventDispatcher = $this->getEventDispatcher( + $converterRegistry, + $storageRegistry, + $contentGateway ); - $content = $this->getContent(); - $fieldDefinition = new FieldDefinition([ - 'id' => 123, - 'identifier' => 'example_field', - 'fieldType' => 'external_type', - 'defaultValue' => new Content\FieldValue(), - ]); - - $event = new ResolveMissingFieldEvent( - $content, - $fieldDefinition, - 'eng-GB' + $event = $eventDispatcher->dispatch( + new ResolveMissingFieldEvent( + $this->getContent(), + new FieldDefinition([ + 'id' => 123, + 'identifier' => 'example_field', + 'fieldType' => 'external_type', + 'defaultValue' => new Content\FieldValue(), + ]), + 'eng-GB' + ) ); - $event = $eventDispatcher->dispatch($event); - $expected = new Content\Field([ 'id' => 567, 'fieldDefinitionId' => 123, @@ -291,9 +248,7 @@ public function testPersistEmptyExternalStorageField(): void public function testPersistExternalStorageField(): void { - $converterRegistry = $this->createMock(ConverterRegistry::class); - $converterRegistry->method('getConverter') - ->willReturn($this->createMock(Converter::class)); + $converterRegistry = $this->getConverterRegistry(); $storage = $this->createMock(FieldStorage::class); $storage->expects($this->once()) @@ -314,33 +269,27 @@ public function testPersistExternalStorageField(): void ->method('insertNewField') ->willReturn(456); - $eventDispatcher = $this->getEventDispatcher(); - $eventDispatcher->addSubscriber( - new ResolveVirtualFieldSubscriber( - $converterRegistry, - $storageRegistry, - $contentGateway, - ) + $eventDispatcher = $this->getEventDispatcher( + $converterRegistry, + $storageRegistry, + $contentGateway ); - $content = $this->getContent(); - $fieldDefinition = new FieldDefinition([ - 'id' => 123, - 'identifier' => 'example_field', - 'fieldType' => 'external_type', - 'defaultValue' => new Content\FieldValue([ - 'data' => ['some_data' => 'to_be_stored'], - ]), - ]); - - $event = new ResolveMissingFieldEvent( - $content, - $fieldDefinition, - 'eng-GB' + $event = $eventDispatcher->dispatch( + new ResolveMissingFieldEvent( + $this->getContent(), + new FieldDefinition([ + 'id' => 123, + 'identifier' => 'example_field', + 'fieldType' => 'external_type', + 'defaultValue' => new Content\FieldValue([ + 'data' => ['some_data' => 'to_be_stored'], + ]), + ]), + 'eng-GB' + ) ); - $event = $eventDispatcher->dispatch($event); - $expected = new Content\Field([ 'id' => 456, 'fieldDefinitionId' => 123, @@ -373,11 +322,51 @@ public function testPersistExternalStorageField(): void ); } - private function getEventDispatcher(): TraceableEventDispatcher + private function getContent(): Content { + $versionInfo = $this->getVersionInfo(); + + $content = new Content(); + $content->versionInfo = $versionInfo; + $content->fields = []; + + return $content; + } + + private function getVersionInfo(): VersionInfo + { + $versionInfo = new VersionInfo(); + $versionInfo->versionNo = 123; + + return $versionInfo; + } + + private function getEventDispatcher( + ConverterRegistry $converterRegistry, + StorageRegistry $storageRegistry, + ContentGateway $contentGateway + ): TraceableEventDispatcher { + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addSubscriber( + new ResolveVirtualFieldSubscriber( + $converterRegistry, + $storageRegistry, + $contentGateway, + ) + ); + return new TraceableEventDispatcher( - new EventDispatcher(), + $eventDispatcher, new Stopwatch() ); } + + private function getConverterRegistry(): ConverterRegistry + { + $converterRegistry = $this->createMock(ConverterRegistry::class); + $converterRegistry->method('getConverter') + ->willReturn($this->createMock(Converter::class)); + + return $converterRegistry; + } } From 07d6721b8e006bb3bfbd265a9ff86c3f9ace3280 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Thu, 25 Apr 2024 09:48:00 +0200 Subject: [PATCH 30/34] fix: Code Review --- .../Persistence/Legacy/Content/Mapper.php | 4 +-- .../FieldType/DefaultDataFieldStorage.php | 9 ++++--- .../ResolveVirtualFieldSubscriberTest.php | 27 +++++++------------ 3 files changed, 16 insertions(+), 24 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index e1880eb79a..dafce22757 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -217,8 +217,8 @@ public function convertToStorageValue(Field $field) * * "$tableName_$columnName" * - * @param array $rows - * @param array $nameRows + * @param array> $rows + * @param array> $nameRows * @param string $prefix * * @return \eZ\Publish\SPI\Persistence\Content[] diff --git a/src/contracts/FieldType/DefaultDataFieldStorage.php b/src/contracts/FieldType/DefaultDataFieldStorage.php index 7c8ae9967a..25db0fa9d3 100644 --- a/src/contracts/FieldType/DefaultDataFieldStorage.php +++ b/src/contracts/FieldType/DefaultDataFieldStorage.php @@ -14,10 +14,11 @@ interface DefaultDataFieldStorage { /** - * Populates $field value property with default data based on the external data. - * $field->value is a {@link \eZ\Publish\SPI\Persistence\Content\FieldValue} object. - * This value holds the data as a {@link \eZ\Publish\Core\FieldType\Value} based object, according to - * the field type (e.g. for TextLine, it will be a {@link \eZ\Publish\Core\FieldType\TextLine\Value} object). + * Populates $field value property with default data based on the external data. + * + * $field->value is a {@see \eZ\Publish\SPI\Persistence\Content\FieldValue} object. + * This value holds the data as a {@see \eZ\Publish\Core\FieldType\Value} based object, according to + * the field type (e.g. for TextLine, it will be a {@see \eZ\Publish\Core\FieldType\TextLine\Value} object). */ public function getDefaultFieldData(VersionInfo $versionInfo, Field $field): void; } diff --git a/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php b/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php index 9f7fafff27..85a40c230b 100644 --- a/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php +++ b/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php @@ -33,12 +33,10 @@ public function testResolveVirtualField(): void $converterRegistry = $this->getConverterRegistry(); $storageRegistry = $this->createMock(StorageRegistry::class); - $storageRegistry->method('getStorage') - ->willReturn(new NullStorage()); + $storageRegistry->method('getStorage')->willReturn(new NullStorage()); $contentGateway = $this->createMock(ContentGateway::class); - $contentGateway->expects($this->never()) - ->method('insertNewField'); + $contentGateway->expects($this->never())->method('insertNewField'); $eventDispatcher = $this->getEventDispatcher( $converterRegistry, @@ -126,8 +124,7 @@ public function getIndexData(VersionInfo $versionInfo, Field $field, array $cont }); $contentGateway = $this->createMock(ContentGateway::class); - $contentGateway->expects($this->never()) - ->method('insertNewField'); + $contentGateway->expects($this->never())->method('insertNewField'); $eventDispatcher = $this->getEventDispatcher( $converterRegistry, @@ -178,8 +175,7 @@ public function testPersistEmptyExternalStorageField(): void $converterRegistry = $this->getConverterRegistry(); $storage = $this->createMock(FieldStorage::class); - $storage->expects($this->never()) - ->method('storeFieldData'); + $storage->expects($this->never())->method('storeFieldData'); $storage->expects($this->once()) ->method('getFieldData') @@ -190,12 +186,10 @@ public function testPersistEmptyExternalStorageField(): void }); $storageRegistry = $this->createMock(StorageRegistry::class); - $storageRegistry->method('getStorage') - ->willReturn($storage); + $storageRegistry->method('getStorage')->willReturn($storage); $contentGateway = $this->createMock(ContentGateway::class); - $contentGateway->expects($this->once()) - ->method('insertNewField') + $contentGateway->expects($this->once())->method('insertNewField') ->willReturn(567); $eventDispatcher = $this->getEventDispatcher( @@ -257,16 +251,13 @@ public function testPersistExternalStorageField(): void $field->value->externalData = $field->value->data; }); - $storage->expects($this->once()) - ->method('getFieldData'); + $storage->expects($this->once())->method('getFieldData'); $storageRegistry = $this->createMock(StorageRegistry::class); - $storageRegistry->method('getStorage') - ->willReturn($storage); + $storageRegistry->method('getStorage')->willReturn($storage); $contentGateway = $this->createMock(ContentGateway::class); - $contentGateway->expects($this->once()) - ->method('insertNewField') + $contentGateway->expects($this->once())->method('insertNewField') ->willReturn(456); $eventDispatcher = $this->getEventDispatcher( From 8b9a267b4372983b41183f66afa692f9a35fb8f0 Mon Sep 17 00:00:00 2001 From: Nattfarinn Date: Thu, 25 Apr 2024 09:57:18 +0200 Subject: [PATCH 31/34] fix: code duplication --- .../ResolveVirtualFieldSubscriberTest.php | 90 +++++++++---------- 1 file changed, 43 insertions(+), 47 deletions(-) diff --git a/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php b/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php index 85a40c230b..440f905bb3 100644 --- a/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php +++ b/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php @@ -32,12 +32,12 @@ public function testResolveVirtualField(): void { $converterRegistry = $this->getConverterRegistry(); - $storageRegistry = $this->createMock(StorageRegistry::class); - $storageRegistry->method('getStorage')->willReturn(new NullStorage()); - $contentGateway = $this->createMock(ContentGateway::class); $contentGateway->expects($this->never())->method('insertNewField'); + $storageRegistry = $this->createMock(StorageRegistry::class); + $storageRegistry->method('getStorage')->willReturn(new NullStorage()); + $eventDispatcher = $this->getEventDispatcher( $converterRegistry, $storageRegistry, @@ -45,16 +45,12 @@ public function testResolveVirtualField(): void ); $event = $eventDispatcher->dispatch( - new ResolveMissingFieldEvent( - $this->getContent(), - new FieldDefinition([ - 'id' => 123, - 'identifier' => 'example_field', - 'fieldType' => 'some_type', - 'defaultValue' => new Content\FieldValue(), - ]), - 'eng-GB' - ) + $this->getEvent([ + 'id' => 123, + 'identifier' => 'example_field', + 'fieldType' => 'some_type', + 'defaultValue' => new Content\FieldValue(), + ]) ); $expected = new Content\Field([ @@ -86,6 +82,9 @@ public function testResolveVirtualExternalStorageField(): void { $converterRegistry = $this->getConverterRegistry(); + $contentGateway = $this->createMock(ContentGateway::class); + $contentGateway->expects($this->never())->method('insertNewField'); + $storageRegistry = $this->createMock(StorageRegistry::class); $storageRegistry->method('getStorage') // Multiple interface mocks are deprecated in PHPUnit 9+ @@ -123,9 +122,6 @@ public function getIndexData(VersionInfo $versionInfo, Field $field, array $cont } }); - $contentGateway = $this->createMock(ContentGateway::class); - $contentGateway->expects($this->never())->method('insertNewField'); - $eventDispatcher = $this->getEventDispatcher( $converterRegistry, $storageRegistry, @@ -133,16 +129,12 @@ public function getIndexData(VersionInfo $versionInfo, Field $field, array $cont ); $event = $eventDispatcher->dispatch( - new ResolveMissingFieldEvent( - $this->getContent(), - new FieldDefinition([ - 'id' => 678, - 'identifier' => 'example_external_field', - 'fieldType' => 'external_type_virtual', - 'defaultValue' => new Content\FieldValue(), - ]), - 'eng-GB' - ) + $this->getEvent([ + 'id' => 678, + 'identifier' => 'example_external_field', + 'fieldType' => 'external_type_virtual', + 'defaultValue' => new Content\FieldValue(), + ]) ); $expected = new Content\Field([ @@ -199,16 +191,12 @@ public function testPersistEmptyExternalStorageField(): void ); $event = $eventDispatcher->dispatch( - new ResolveMissingFieldEvent( - $this->getContent(), - new FieldDefinition([ - 'id' => 123, - 'identifier' => 'example_field', - 'fieldType' => 'external_type', - 'defaultValue' => new Content\FieldValue(), - ]), - 'eng-GB' - ) + $this->getEvent([ + 'id' => 123, + 'identifier' => 'example_field', + 'fieldType' => 'external_type', + 'defaultValue' => new Content\FieldValue(), + ]) ); $expected = new Content\Field([ @@ -267,18 +255,14 @@ public function testPersistExternalStorageField(): void ); $event = $eventDispatcher->dispatch( - new ResolveMissingFieldEvent( - $this->getContent(), - new FieldDefinition([ - 'id' => 123, - 'identifier' => 'example_field', - 'fieldType' => 'external_type', - 'defaultValue' => new Content\FieldValue([ - 'data' => ['some_data' => 'to_be_stored'], - ]), + $this->getEvent([ + 'id' => 123, + 'identifier' => 'example_field', + 'fieldType' => 'external_type', + 'defaultValue' => new Content\FieldValue([ + 'data' => ['some_data' => 'to_be_stored'], ]), - 'eng-GB' - ) + ]) ); $expected = new Content\Field([ @@ -360,4 +344,16 @@ private function getConverterRegistry(): ConverterRegistry return $converterRegistry; } + + /** + * @param array $fieldDefinition + */ + private function getEvent(array $fieldDefinition): ResolveMissingFieldEvent + { + return new ResolveMissingFieldEvent( + $this->getContent(), + new FieldDefinition($fieldDefinition), + 'eng-GB' + ); + } } From 0b0ba2249a3e0604adf46aaaf583cca3a27b4c86 Mon Sep 17 00:00:00 2001 From: Andrew Longosz Date: Thu, 25 Apr 2024 13:32:23 +0200 Subject: [PATCH 32/34] Made DefaultDataFieldStorage extending FieldStorage --- src/contracts/FieldType/DefaultDataFieldStorage.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/contracts/FieldType/DefaultDataFieldStorage.php b/src/contracts/FieldType/DefaultDataFieldStorage.php index 25db0fa9d3..ebb00182b5 100644 --- a/src/contracts/FieldType/DefaultDataFieldStorage.php +++ b/src/contracts/FieldType/DefaultDataFieldStorage.php @@ -8,10 +8,11 @@ namespace Ibexa\Contracts\Core\FieldType; +use eZ\Publish\SPI\FieldType\FieldStorage; use eZ\Publish\SPI\Persistence\Content\Field; use eZ\Publish\SPI\Persistence\Content\VersionInfo; -interface DefaultDataFieldStorage +interface DefaultDataFieldStorage extends FieldStorage { /** * Populates $field value property with default data based on the external data. From 90831c63e9aff42f8cbe26ac8b059a6cec4d90f1 Mon Sep 17 00:00:00 2001 From: Andrew Longosz Date: Thu, 25 Apr 2024 13:33:36 +0200 Subject: [PATCH 33/34] [Tests] Improved mocking of DefaultDataFieldStorage --- .../ResolveVirtualFieldSubscriberTest.php | 41 ++++--------------- 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php b/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php index 440f905bb3..67e50f6496 100644 --- a/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php +++ b/tests/lib/Persistence/Legacy/Content/Mapper/ResolveVirtualFieldSubscriberTest.php @@ -85,42 +85,19 @@ public function testResolveVirtualExternalStorageField(): void $contentGateway = $this->createMock(ContentGateway::class); $contentGateway->expects($this->never())->method('insertNewField'); - $storageRegistry = $this->createMock(StorageRegistry::class); - $storageRegistry->method('getStorage') - // Multiple interface mocks are deprecated in PHPUnit 9+ - ->willReturn(new class() implements FieldStorage, DefaultDataFieldStorage { - public function getDefaultFieldData(VersionInfo $versionInfo, Field $field): void - { + $defaultFieldStorageMock = $this->createMock(DefaultDataFieldStorage::class); + $defaultFieldStorageMock + ->method('getDefaultFieldData') + ->willReturnCallback( + static function (VersionInfo $versionInfo, Field $field): void { $field->value->externalData = [ 'some_default' => 'external_data', ]; } - - public function storeFieldData(VersionInfo $versionInfo, Field $field, array $context): void - { - // Mock - } - - public function getFieldData(VersionInfo $versionInfo, Field $field, array $context): void - { - // Mock - } - - public function deleteFieldData(VersionInfo $versionInfo, array $fieldIds, array $context): void - { - // Mock - } - - public function hasFieldData(): void - { - // Mock - } - - public function getIndexData(VersionInfo $versionInfo, Field $field, array $context): void - { - // Mock - } - }); + ); + $storageRegistry = $this->createMock(StorageRegistry::class); + $storageRegistry->method('getStorage') + ->willReturn($defaultFieldStorageMock); $eventDispatcher = $this->getEventDispatcher( $converterRegistry, From 71061b1c250d67289ecbfd49b9f21ef53823d79d Mon Sep 17 00:00:00 2001 From: Andrew Longosz Date: Thu, 25 Apr 2024 14:08:59 +0200 Subject: [PATCH 34/34] [DI][CS] Used single quotes for Symfony DIC config --- eZ/Publish/Core/settings/storage_engines/legacy/content.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml index f7dd8b7dd0..58c0de94b0 100644 --- a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml +++ b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml @@ -14,9 +14,9 @@ services: Ibexa\Core\Persistence\Legacy\Content\Mapper\ResolveVirtualFieldSubscriber: arguments: - $converterRegistry: "@ezpublish.persistence.legacy.field_value_converter.registry" - $storageRegistry: "@ezpublish.persistence.external_storage_registry" - $contentGateway: "@ezpublish.persistence.legacy.content.gateway" + $converterRegistry: '@ezpublish.persistence.legacy.field_value_converter.registry' + $storageRegistry: '@ezpublish.persistence.external_storage_registry' + $contentGateway: '@ezpublish.persistence.legacy.content.gateway' tags: - { name: kernel.event_subscriber }