Skip to content

Commit

Permalink
IBX-8562: Fixed flooding content attributes table with duplicates
Browse files Browse the repository at this point in the history
For more details see https://issues.ibexa.co/browse/IBX-8562 and #409

Key changes:

* Fixed replacing only field of Content Type with virtual field
  • Loading branch information
Nattfarinn authored Aug 7, 2024
1 parent 17e78be commit 9a62410
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 25 deletions.
9 changes: 8 additions & 1 deletion eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<int, array<string, \eZ\Publish\SPI\Persistence\Content\Field>>
*/
protected function getFieldMap(array $fields, &$languageCodes = null)
{
Expand Down
26 changes: 24 additions & 2 deletions eZ/Publish/Core/Persistence/Legacy/Content/Handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand All @@ -109,6 +115,7 @@ public function __construct(
UrlAliasGateway $urlAliasGateway,
ContentTypeHandler $contentTypeHandler,
TreeHandler $treeHandler,
LanguageHandler $languageHandler,
LoggerInterface $logger = null
) {
$this->contentGateway = $contentGateway;
Expand All @@ -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();
}

Expand Down Expand Up @@ -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)
{
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
23 changes: 16 additions & 7 deletions eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ public function convertToStorageValue(Field $field)
* @param array<array<string, scalar>> $rows
* @param array<array<string, scalar>> $nameRows
* @param string $prefix
* @param array<string>|null $translations
*
* @return \eZ\Publish\SPI\Persistence\Content[]
*
Expand All @@ -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 = [];

Expand All @@ -245,7 +247,8 @@ public function extractContentFromRows(

$fieldDefinitions = $this->loadCachedVersionFieldDefinitionsPerLanguage(
$rows,
$prefix
$prefix,
$translations
);

foreach ($rows as $row) {
Expand Down Expand Up @@ -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(
Expand All @@ -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 = [];
Expand All @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -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')
Expand All @@ -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(
Expand Down Expand Up @@ -1538,7 +1553,8 @@ protected function getContentHandler()
$this->getSlugConverterMock(),
$this->getUrlAliasGatewayMock(),
$this->getContentTypeHandlerMock(),
$this->getTreeHandlerMock()
$this->getTreeHandlerMock(),
$this->getLanguageHandlerMock(),
);
}

Expand Down Expand Up @@ -1566,6 +1582,7 @@ protected function getPartlyMockedHandler(array $methods)
$this->getUrlAliasGatewayMock(),
$this->getContentTypeHandlerMock(),
$this->getTreeHandlerMock(),
$this->getLanguageHandlerMock(),
]
)
->getMock();
Expand Down Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 9a62410

Please sign in to comment.