Skip to content

IBX-12033: Fixed content name reverting when translations are published in parallel #779

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.

Sign up for GitHub

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

Open
Sztig wants to merge 2 commits into
base: 4.6
Choose a base branch
from
Open

IBX-12033: Fixed content name reverting when translations are published in parallel #779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b715b6
fixed content name reverting when translations are published in parallel
Sztig Jul 9, 2026
b420e8a
implemented test
Sztig Jul 9, 2026
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
6 changes: 6 additions & 0 deletions src/lib/Repository/ContentService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1528,6 +1528,12 @@ protected function copyNonTranslatableFieldsFromPublishedVersion(APIContent $cur
$versionInfo = $currentVersionContent->getVersionInfo();
$contentType = $currentVersionContent->getContentType();

$currentVersionContent = $this->internalLoadContentById(

@bnowak bnowak Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that reloading content version 2nd time solves the issue completely - it's just narrowing down the time when the issue can happen. Ofc, there's much less chances for that now, but we're no 100% sure it wil work every time.

Instead, I'd rather extend db transaction in publishVersion method to include also loading content in 1st line, and use pessimistic locking to block concurrent processes to load the same content (which is currently publishing). That way, next process should wait until 1st will be finished and will load its new (already published) version.
You can look at https://github.com/ibexa/taxonomy/pull/423 as an example 😉

Also, that would need to be covered by an integration test that checks this lock/block for concurent connection. You can check testTreeRootLockBlocksConcurrentConnection test from mentioned PR

@Sztig Sztig Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the locking approach would fix the problem that we have here as this scenario is completely sequential, each step only starts after the previous one is fully commited. The lock would be acquired instantly and changes nothing.

@alongosz alongosz Jul 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bnowak Taxonomy is a specific case because it uses ORM. Overall in the product and especially in core we use DBAL (that's why I don't know ORM that well btw ;-) ).

That being said, @Sztig maybe it's worth exploring, though I'm not exactly sure how much work that's gonna be. This whole batch of operations is already wrapped by transaction. So I wonder what would happen if we tried to do pessimistic write lock for queries selecting proper rows here too. On DBAL layer \Doctrine\DBAL\Platforms\AbstractPlatform::getWriteLockSQL method returns proper platform-specific portion of SQL, which usually is FOR UPDATE. It's appended as SELECT ... FOR UPDATE to a query which should lock row it applies to (is it content_name relation or version_attribute relation we're talking about here?).

As for tests, we already had some parallel integration test in core, done differently than what I had to do for taxo. Sadly it's legacy integration layer - \Ibexa\Tests\Integration\Core\Repository\Parallel\BaseParallelTestCase. We'd need sth similar extending RepositoryTestCase.

$versionInfo->getContentInfo()->getId(),
null,
$versionInfo->versionNo
);

$publishedContent = $this->internalLoadContentById($versionInfo->getContentInfo()->getId());
$publishedVersionInfo = $publishedContent->getVersionInfo();

Expand Down
71 changes: 71 additions & 0 deletions tests/integration/Core/Repository/ContentServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6573,6 +6573,77 @@ public function testCopyTranslationsFromInvalidPublishedContentToDraft()
);
}

public function testCopyTranslationsFromPublishedToDraftWithNonTranslatableField(): void

@bnowak bnowak Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure that the test is covering the issue? Will it fail if you comment out the change in ContentService::copyNonTranslatableFieldsFromPublishedVersion?

If not, it means that it doesn't fulfil its role and could be removed or rewritten.

@Sztig Sztig Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test fails without the fix, at least locally.

{
$contentTypeService = $this->getRepository()->getContentTypeService();

$contentTypeCreateStruct = $contentTypeService->newContentTypeCreateStruct(
'test_non_translatable_field'
);
$contentTypeCreateStruct->mainLanguageCode = self::ENG_US;
$contentTypeCreateStruct->names = [self::ENG_US => 'Test content type with non-translatable field'];
$contentTypeCreateStruct->nameSchema = '<name>';

$nameField = $contentTypeService->newFieldDefinitionCreateStruct('name', 'ezstring');
$nameField->position = 1;
$nameField->isTranslatable = true;
$contentTypeCreateStruct->addFieldDefinition($nameField);

$integerField = $contentTypeService->newFieldDefinitionCreateStruct('integer', 'ezinteger');
$integerField->position = 2;
$integerField->isTranslatable = false;
$contentTypeCreateStruct->addFieldDefinition($integerField);

$contentTypeService->publishContentTypeDraft(
$contentTypeService->createContentType(
$contentTypeCreateStruct,
[$contentTypeService->loadContentTypeGroupByIdentifier('Content')]
)
);

$contentDraft = $this->createContentDraft(
'test_non_translatable_field',
$this->generateId('location', 2),
[
'name' => 'test',
'integer' => 1,
]
);
$this->contentService->publishVersion($contentDraft->versionInfo);

$translationDraft = $this->contentService->createContentDraft($contentDraft->contentInfo);
$translationUpdateStruct = new ContentUpdateStruct([
'initialLanguageCode' => self::GER_DE,
]);
$translationUpdateStruct->setField('name', 'test de', self::GER_DE);
$this->contentService->updateContent($translationDraft->versionInfo, $translationUpdateStruct);
$publishedContent = $this->contentService->publishVersion($translationDraft->versionInfo);

$usDraft = $this->contentService->createContentDraft($publishedContent->contentInfo);
$deDraft = $this->contentService->createContentDraft($publishedContent->contentInfo);

$usUpdateStruct = new ContentUpdateStruct([
'initialLanguageCode' => self::ENG_US,
]);
$usUpdateStruct->setField('name', 'test updated', self::ENG_US);
$this->contentService->updateContent($usDraft->versionInfo, $usUpdateStruct);
$this->contentService->publishVersion($usDraft->versionInfo, [self::ENG_US]);

$deUpdateStruct = new ContentUpdateStruct([
'initialLanguageCode' => self::GER_DE,
]);
$deUpdateStruct->setField('name', 'test de updated', self::GER_DE);
$this->contentService->updateContent($deDraft->versionInfo, $deUpdateStruct);
$dePublished = $this->contentService->publishVersion($deDraft->versionInfo, [self::GER_DE]);

$expectedNames = [
self::ENG_US => 'test updated',
self::GER_DE => 'test de updated',
];
$this->assertEquals($expectedNames, $dePublished->fields['name']);
$this->assertEquals($expectedNames, $dePublished->getVersionInfo()->getNames());
}

/**
* Create structure of parent folders with Locations to be used for Content hide/reveal tests.
*
Expand Down