Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBX-5388: Fixed performance issues of content updates after field changes #397

Merged
merged 34 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
0615da2
IBX-5388: Fixed performance issues of content updates after field cha…
Nattfarinn Nov 28, 2023
f2e552c
fix: Coding standards
Nattfarinn Dec 14, 2023
58f3f9c
fix: Code Review
Nattfarinn Dec 20, 2023
de49379
fix: Regression and fix invalid integration tests
Nattfarinn Dec 20, 2023
bdf66e1
fix: Revert unnecessary changes to integration test structure
Nattfarinn Dec 20, 2023
6d8aac7
fix: Remove commented code
Nattfarinn Dec 20, 2023
657e809
fix: Code review
Nattfarinn Dec 20, 2023
edf319b
fix: Tests
Nattfarinn Dec 20, 2023
9ed8d26
fix: Prevent fatal error in case of virtual field
Nattfarinn Dec 20, 2023
8039b52
fix: Do not rely on fieldDefinition->defaultValue
Nattfarinn Mar 20, 2024
a5045db
fix: Author field
Nattfarinn Mar 21, 2024
ec9552c
fix: Apply suggestions from code review
Nattfarinn Mar 21, 2024
6f0dd63
fix: Apply suggestions from code review
Nattfarinn Mar 21, 2024
c1d4691
fix: Code review
Nattfarinn Mar 21, 2024
c59e263
fix: Cannot use PHP 7.4 features
Nattfarinn Mar 21, 2024
3fe2beb
fix: External storage
Nattfarinn Apr 4, 2024
be3e6f3
fix: External Storage
Nattfarinn Apr 9, 2024
1fac46a
fix: CS
Nattfarinn Apr 9, 2024
80d6e10
fix: interface DefaultDataFieldStorage
Nattfarinn Apr 16, 2024
3c738c0
fix: MapperTest
Nattfarinn Apr 16, 2024
8f9296c
fix: register Symfony EventDispatcher outside of MVC tests
Nattfarinn Apr 16, 2024
af3fd67
fix: missing EventDispatcher
Nattfarinn Apr 16, 2024
80f09dd
fix: Coding standards
Nattfarinn Apr 16, 2024
80904a9
fix: add missing EventDispatcher to Handler test
Nattfarinn Apr 16, 2024
eae6c1b
fix: replace event_dispatcher with FQCN
Nattfarinn Apr 17, 2024
9d104aa
fix: register new subscriber in test setup factory
Nattfarinn Apr 17, 2024
0f7966d
fix: Code Review
Nattfarinn Apr 23, 2024
69acebf
fix: Reduce code duplication
Nattfarinn Apr 23, 2024
84a16b0
fix: Reorganize subscriber test
Nattfarinn Apr 25, 2024
07d6721
fix: Code Review
Nattfarinn Apr 25, 2024
8b9a267
fix: code duplication
Nattfarinn Apr 25, 2024
0b0ba22
Made DefaultDataFieldStorage extending FieldStorage
alongosz Apr 25, 2024
90831c6
[Tests] Improved mocking of DefaultDataFieldStorage
alongosz Apr 25, 2024
71061b1
[DI][CS] Used single quotes for Symfony DIC config
alongosz Apr 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 0 additions & 61 deletions eZ/Publish/API/Repository/Tests/ContentTypeServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -2021,66 +2020,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.
*
Expand Down
15 changes: 12 additions & 3 deletions eZ/Publish/API/Repository/Values/Content/Field.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -58,7 +59,7 @@ class Field extends ValueObject
*/
protected $fieldTypeIdentifier;

public function getId(): int
public function getId(): ?int
{
return $this->id;
}
Expand All @@ -85,4 +86,12 @@ public function getFieldTypeIdentifier(): string
{
return $this->fieldTypeIdentifier;
}

/**
* @phpstan-assert-if-true !null $this->getId()
*/
public function isVirtual(): bool
Nattfarinn marked this conversation as resolved.
Show resolved Hide resolved
{
return null === $this->id;
}
}
6 changes: 5 additions & 1 deletion eZ/Publish/Core/Persistence/Legacy/Content/FieldHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,13 @@ 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) {
Nattfarinn marked this conversation as resolved.
Show resolved Hide resolved
// Virtual field with default value, skip creating field as it has no id
continue;
}
$this->createExistingFieldInNewVersion($field, $content);
}
}
Expand Down
186 changes: 164 additions & 22 deletions eZ/Publish/Core/Persistence/Legacy/Content/Mapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,45 @@
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\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.
*
* Performs mapping of Content objects.
*
* @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<int, string>
* >
* >
* >
* @phpstan-type TContentInfoMap array<int, \eZ\Publish\SPI\Persistence\Content\ContentInfo>
* @phpstan-type TVersionInfoMap array<
* int, array<
* int, \eZ\Publish\SPI\Persistence\Content\VersionInfo,
* >
* >
*/
class Mapper
{
Expand All @@ -40,15 +73,25 @@ class Mapper
protected $languageHandler;

/**
* Creates a new mapper.
*
* @param \eZ\Publish\Core\Persistence\Legacy\Content\FieldValue\ConverterRegistry $converterRegistry
* @param \eZ\Publish\SPI\Persistence\Content\Language\Handler $languageHandler
* @var \eZ\Publish\SPI\Persistence\Content\Type\Handler
*/
public function __construct(Registry $converterRegistry, LanguageHandler $languageHandler)
{
private $contentTypeHandler;

/**
* @var \Symfony\Contracts\EventDispatcher\EventDispatcherInterface
*/
private $eventDispatcher;

public function __construct(
Registry $converterRegistry,
LanguageHandler $languageHandler,
ContentTypeHandler $contentTypeHandler,
EventDispatcherInterface $eventDispatcher
) {
$this->converterRegistry = $converterRegistry;
$this->languageHandler = $languageHandler;
$this->contentTypeHandler = $contentTypeHandler;
$this->eventDispatcher = $eventDispatcher;
}

/**
Expand Down Expand Up @@ -174,66 +217,166 @@ public function convertToStorageValue(Field $field)
*
* "$tableName_$columnName"
*
* @param array $rows
* @param array $nameRows
* @param array<array<string, scalar>> $rows
* @param array<array<string, scalar>> $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 = [];

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"];
$languageCode = $row["{$prefix}name_content_translation"];
$versionedNameData[$contentId][$versionNo][$languageCode] = $row["{$prefix}name_name"];
}

$contentInfos = [];
$versionInfos = [];
$fields = [];

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

foreach ($rows as $row) {
$contentId = (int)$row["{$prefix}id"];
$versionId = (int)$row["{$prefix}version_id"];

if (!isset($contentInfos[$contentId])) {
$contentInfos[$contentId] = $this->extractContentInfoFromRow($row, $prefix);
}

if (!isset($versionInfos[$contentId])) {
$versionInfos[$contentId] = [];
}

$versionId = (int)$row['ezcontentobject_version_id'];
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]);
}
}

return $this->buildContentObjects(
$contentInfos,
$versionInfos,
$fields,
$fieldDefinitions,
$versionedNameData
);
}

/**
* @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 $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
if (isset($versionedNameData[$contentId][$versionInfo->versionNo])) {
$names = $versionedNameData[$contentId][$versionInfo->versionNo];
} else {
$names = [$contentInfo->mainLanguageCode => $contentInfo->name];
}
$names = $versionedNames[$contentId][$versionInfo->versionNo]
?? [$contentInfo->mainLanguageCode => $contentInfo->name];

$content = new Content();
$content->versionInfo = $versionInfo;
$content->versionInfo->names = $names;
$content->versionInfo->contentInfo = $contentInfo;
$content->fields = array_values($fields[$contentId][$versionId]);

$missingVersionFieldDefinitions = $missingFieldDefinitions[$contentId][$versionId];
foreach ($missingVersionFieldDefinitions as $languageCode => $versionFieldDefinitions) {
foreach ($versionFieldDefinitions as $fieldDefinition) {
$event = $this->eventDispatcher->dispatch(
new ResolveMissingFieldEvent(
$content,
$fieldDefinition,
$languageCode
)
);

$field = $event->getField();
if ($field !== null) {
$content->fields[] = $field;
}
}
}

$results[] = $content;
}
}

return $results;
}

/**
* @phpstan-return TVersionedLanguageFieldDefinitionsMap
*
* @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
alongosz marked this conversation as resolved.
Show resolved Hide resolved
*/
private function loadCachedVersionFieldDefinitionsPerLanguage(
array $rows,
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])) {
continue;
}

$languageCodes = $this->extractLanguageCodesFromMask($languageMask, $allLanguages);
$contentTypes[$contentTypeId] = $contentTypes[$contentTypeId] ?? $this->contentTypeHandler->load($contentTypeId);
$contentType = $contentTypes[$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.
*
Expand All @@ -251,7 +394,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"];
Expand Down
Loading
Loading