From 9a6241085f50c9f42cd1b1fd258fba0e46505df1 Mon Sep 17 00:00:00 2001 From: Slawomir Dolzycki-Uchto Date: Wed, 7 Aug 2024 11:03:50 +0200 Subject: [PATCH] IBX-8562: Fixed flooding content attributes table with duplicates For more details see https://issues.ibexa.co/browse/IBX-8562 and https://github.com/ezsystems/ezplatform-kernel/pull/409 Key changes: * Fixed replacing only field of Content Type with virtual field --- .../Legacy/Content/FieldHandler.php | 9 +++++- .../Persistence/Legacy/Content/Handler.php | 26 ++++++++++++++-- .../Persistence/Legacy/Content/Mapper.php | 23 +++++++++----- .../Tests/Content/ContentHandlerTest.php | 31 ++++++++++++++++++- .../Legacy/Tests/Content/FieldHandlerTest.php | 22 +++++-------- .../storage_engines/legacy/content.yml | 1 + 6 files changed, 87 insertions(+), 25 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php b/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php index dba486b78e..5bb2dc0460 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php @@ -358,6 +358,13 @@ public function updateFields(Content $content, UpdateStruct $updateStruct, Type // also update copied field data // Register for processing after all given fields are updated $nonTranslatableCopiesUpdateSet[$fieldDefinition->id][] = $languageCode; + } elseif (isset($contentFieldMap[$fieldDefinition->id][$languageCode])) { + $field = clone $contentFieldMap[$fieldDefinition->id][$languageCode]; + $field->versionNo = $content->versionInfo->versionNo; + // Persist virtual field + if (null === $field->id) { + $this->createNewField($field, $content); + } } // If no above conditions were met - do nothing @@ -417,7 +424,7 @@ protected function updateCopiedField(Field $field, Field $updateField, Field $or * @param \eZ\Publish\SPI\Persistence\Content\Field[] $fields * @param array $languageCodes * - * @return \eZ\Publish\SPI\Persistence\Content\Field[][] + * @return array> */ protected function getFieldMap(array $fields, &$languageCodes = null) { diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Handler.php b/eZ/Publish/Core/Persistence/Legacy/Content/Handler.php index 0fb6e39a2b..430f3b7b8a 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Handler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Handler.php @@ -15,6 +15,7 @@ use eZ\Publish\SPI\Persistence\Content\CreateStruct; use eZ\Publish\SPI\Persistence\Content\Field; use eZ\Publish\SPI\Persistence\Content\Handler as BaseContentHandler; +use eZ\Publish\SPI\Persistence\Content\Language\Handler as LanguageHandler; use eZ\Publish\SPI\Persistence\Content\MetadataUpdateStruct; use eZ\Publish\SPI\Persistence\Content\Relation\CreateStruct as RelationCreateStruct; use eZ\Publish\SPI\Persistence\Content\Type\Handler as ContentTypeHandler; @@ -84,6 +85,11 @@ class Handler implements BaseContentHandler */ protected $treeHandler; + /** + * @var \eZ\Publish\SPI\Persistence\Content\Language\Handler + */ + protected $languageHandler; + /** @var \Psr\Log\LoggerInterface */ private $logger; @@ -109,6 +115,7 @@ public function __construct( UrlAliasGateway $urlAliasGateway, ContentTypeHandler $contentTypeHandler, TreeHandler $treeHandler, + LanguageHandler $languageHandler, LoggerInterface $logger = null ) { $this->contentGateway = $contentGateway; @@ -119,6 +126,7 @@ public function __construct( $this->urlAliasGateway = $urlAliasGateway; $this->contentTypeHandler = $contentTypeHandler; $this->treeHandler = $treeHandler; + $this->languageHandler = $languageHandler; $this->logger = null !== $logger ? $logger : new NullLogger(); } @@ -255,6 +263,8 @@ public function publish($contentId, $versionNo, MetadataUpdateStruct $metaDataUp * @param string|null $languageCode * * @return \eZ\Publish\SPI\Persistence\Content + * + * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException */ public function createDraftFromVersion($contentId, $srcVersion, $userId, ?string $languageCode = null) { @@ -275,6 +285,14 @@ public function createDraftFromVersion($contentId, $srcVersion, $userId, ?string // Clone fields from previous version and append them to the new one $this->fieldHandler->createExistingFieldsInNewVersion($content); + // Persist virtual fields + $contentType = $this->contentTypeHandler->load($content->versionInfo->contentInfo->contentTypeId); + $this->fieldHandler->updateFields($content, new UpdateStruct([ + 'initialLanguageId' => $this->languageHandler->loadByLanguageCode( + $content->versionInfo->initialLanguageCode + )->id, + ]), $contentType); + // Create relations for new version $relations = $this->contentGateway->loadRelations($contentId, $srcVersion); foreach ($relations as $relation) { @@ -320,7 +338,9 @@ public function load($id, $version = null, array $translations = null) $this->contentGateway->loadVersionedNameData([[ 'id' => $id, 'version' => $rows[0]['ezcontentobject_version_version'], - ]]) + ]]), + 'ezcontentobject_', + $translations ); $content = $contentObjects[0]; unset($rows, $contentObjects); @@ -371,7 +391,9 @@ public function loadContentList(array $contentIds, array $translations = null): try { $contentList = $this->mapper->extractContentFromRows( $contentItemsRow, - $contentItemNameData[$contentId] + $contentItemNameData[$contentId], + 'ezcontentobject_', + $translations ); $contentItems[$contentId] = $contentList[0]; } catch (Exception $e) { diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php index dafce22757..ed9e8a8dbc 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php @@ -220,6 +220,7 @@ public function convertToStorageValue(Field $field) * @param array> $rows * @param array> $nameRows * @param string $prefix + * @param array|null $translations * * @return \eZ\Publish\SPI\Persistence\Content[] * @@ -228,7 +229,8 @@ public function convertToStorageValue(Field $field) public function extractContentFromRows( array $rows, array $nameRows, - string $prefix = 'ezcontentobject_' + string $prefix = 'ezcontentobject_', + ?array $translations = null ): array { $versionedNameData = []; @@ -245,7 +247,8 @@ public function extractContentFromRows( $fieldDefinitions = $this->loadCachedVersionFieldDefinitionsPerLanguage( $rows, - $prefix + $prefix, + $translations ); foreach ($rows as $row) { @@ -313,9 +316,10 @@ private function buildContentObjects( $content->versionInfo = $versionInfo; $content->versionInfo->names = $names; $content->versionInfo->contentInfo = $contentInfo; - $content->fields = array_values($fields[$contentId][$versionId]); + $content->fields = array_values($fields[$contentId][$versionId] ?? []); + + $missingVersionFieldDefinitions = $missingFieldDefinitions[$contentId][$versionId] ?? []; - $missingVersionFieldDefinitions = $missingFieldDefinitions[$contentId][$versionId]; foreach ($missingVersionFieldDefinitions as $languageCode => $versionFieldDefinitions) { foreach ($versionFieldDefinitions as $fieldDefinition) { $event = $this->eventDispatcher->dispatch( @@ -341,13 +345,16 @@ private function buildContentObjects( } /** + * @param string[]|null $translations + * * @phpstan-return TVersionedLanguageFieldDefinitionsMap * * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException */ private function loadCachedVersionFieldDefinitionsPerLanguage( array $rows, - string $prefix + string $prefix, + ?array $translations = null ): array { $fieldDefinitions = []; $contentTypes = []; @@ -363,12 +370,14 @@ private function loadCachedVersionFieldDefinitionsPerLanguage( continue; } - $languageCodes = $this->extractLanguageCodesFromMask($languageMask, $allLanguages); + $allLanguagesCodes = $this->extractLanguageCodesFromMask($languageMask, $allLanguages); + $languageCodes = empty($translations) ? $allLanguagesCodes : array_intersect($translations, $allLanguagesCodes); $contentTypes[$contentTypeId] = $contentTypes[$contentTypeId] ?? $this->contentTypeHandler->load($contentTypeId); $contentType = $contentTypes[$contentTypeId]; foreach ($contentType->fieldDefinitions as $fieldDefinition) { foreach ($languageCodes as $languageCode) { - $id = $fieldDefinition->id; + $id = (int)$fieldDefinition->id; + $languageCode = (string)$languageCode; $fieldDefinitions[$contentId][$versionId][$languageCode][$id] = $fieldDefinition; } } diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/ContentHandlerTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/ContentHandlerTest.php index ce2720aab0..133e8904f5 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/ContentHandlerTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/ContentHandlerTest.php @@ -11,6 +11,7 @@ use eZ\Publish\Core\Persistence\Legacy\Content\FieldHandler; use eZ\Publish\Core\Persistence\Legacy\Content\Gateway as ContentGateway; use eZ\Publish\Core\Persistence\Legacy\Content\Handler; +use eZ\Publish\Core\Persistence\Legacy\Content\Language\Handler as LanguageHandler; use eZ\Publish\Core\Persistence\Legacy\Content\Location\Gateway as LocationGateway; use eZ\Publish\Core\Persistence\Legacy\Content\Mapper; use eZ\Publish\Core\Persistence\Legacy\Content\TreeHandler; @@ -108,6 +109,11 @@ class ContentHandlerTest extends TestCase */ protected $contentTypeHandlerMock; + /** + * @var \PHPUnit\Framework\MockObject\MockObject&\eZ\Publish\Core\Persistence\Legacy\Content\Language\Handler + */ + private $languageHandlerMock; + /** * @covers \eZ\Publish\Core\Persistence\Legacy\Content\Handler::create * @@ -384,6 +390,8 @@ public function testCreateDraftFromVersion() $mapperMock = $this->getMapperMock(); $gatewayMock = $this->getGatewayMock(); $fieldHandlerMock = $this->getFieldHandlerMock(); + $languageHandlerMock = $this->getLanguageHandlerMock(); + $contentTypeHandlerMock = $this->getContentTypeHandlerMock(); $handler->expects($this->once()) ->method('load') @@ -402,11 +410,18 @@ public function testCreateDraftFromVersion() [ 'names' => [], 'versionNo' => 3, + 'contentInfo' => new ContentInfo(), ] ) ) ); + $languageHandlerMock->method('loadByLanguageCode') + ->willReturn(new Content\Language()); + + $contentTypeHandlerMock->method('load') + ->willReturn(new Type()); + $gatewayMock->expects($this->once()) ->method('insertVersion') ->with( @@ -1538,7 +1553,8 @@ protected function getContentHandler() $this->getSlugConverterMock(), $this->getUrlAliasGatewayMock(), $this->getContentTypeHandlerMock(), - $this->getTreeHandlerMock() + $this->getTreeHandlerMock(), + $this->getLanguageHandlerMock(), ); } @@ -1566,6 +1582,7 @@ protected function getPartlyMockedHandler(array $methods) $this->getUrlAliasGatewayMock(), $this->getContentTypeHandlerMock(), $this->getTreeHandlerMock(), + $this->getLanguageHandlerMock(), ] ) ->getMock(); @@ -1599,6 +1616,18 @@ protected function getContentTypeHandlerMock() return $this->contentTypeHandlerMock; } + /** + * @return \PHPUnit\Framework\MockObject\MockObject&\eZ\Publish\Core\Persistence\Legacy\Content\Language\Handler + */ + protected function getLanguageHandlerMock(): LanguageHandler + { + if (!isset($this->languageHandlerMock)) { + $this->languageHandlerMock = $this->createMock(LanguageHandler::class); + } + + return $this->languageHandlerMock; + } + /** * Returns a FieldHandler mock. * diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/FieldHandlerTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/FieldHandlerTest.php index d784832cad..93b010529c 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/FieldHandlerTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/FieldHandlerTest.php @@ -633,7 +633,7 @@ public function assertUpdateFieldsForInitialLanguage($storageHandlerUpdatesField $callNo = 0; $fieldValue = new FieldValue(); $fieldsToCopy = []; - foreach ([1, 2, 3] as $fieldDefinitionId) { + foreach ([1, 2, 3] as $id => $fieldDefinitionId) { $field = new Field( [ 'fieldDefinitionId' => $fieldDefinitionId, @@ -646,6 +646,7 @@ public function assertUpdateFieldsForInitialLanguage($storageHandlerUpdatesField // These fields are copied from main language if ($fieldDefinitionId == 2 || $fieldDefinitionId == 3) { $originalField = clone $field; + $originalField->id = $fieldDefinitionId; $originalField->languageCode = 'eng-GB'; $fieldsToCopy[] = [ 'copy' => clone $field, @@ -845,20 +846,13 @@ protected function getContentSingleLanguageFixture() $field->value = new FieldValue(); $field->languageCode = 'eng-GB'; - $firstField = clone $field; - $firstField->fieldDefinitionId = 1; - - $secondField = clone $field; - $secondField->fieldDefinitionId = 2; - - $thirdField = clone $field; - $thirdField->fieldDefinitionId = 3; + foreach ([1, 2, 3] as $id) { + $contentField = clone $field; + $contentField->id = $id; + $contentField->fieldDefinitionId = $id; - $content->fields = [ - $firstField, - $secondField, - $thirdField, - ]; + $content->fields[] = $contentField; + } return $content; } diff --git a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml index 58c0de94b0..2a956461b7 100644 --- a/eZ/Publish/Core/settings/storage_engines/legacy/content.yml +++ b/eZ/Publish/Core/settings/storage_engines/legacy/content.yml @@ -68,5 +68,6 @@ services: - "@ezpublish.persistence.legacy.url_alias.gateway" - "@ezpublish.spi.persistence.legacy.content_type.handler" - "@ezpublish.persistence.legacy.tree_handler" + - "@ezpublish.spi.persistence.legacy.language.handler" - "@logger" lazy: true