Skip to content

Commit

Permalink
[BUGFIX] Workspace and reference index fixes
Browse files Browse the repository at this point in the history
This drops all left over calls that suppress the refindex
integrity checks in DataHandler functional tests, except those
within ManyToMany tests: This area needs more work before the
reference index can be fixed.

Two bugs indirectly related to the reference index are fixed
along the way, those can't be separated as standalone patches:

* When a translated page is deleted in workspaces, the
  DataHandler deleted live records on this page, too. This
  is fixed by adding a proper db restriction. This also
  fixes the reference index state in this scenario.

* When publishing a delete placeholder that has inline children,
  workspace delete placeholders are created for the children, which
  is wrong. This happens because the user is not set to the live
  workspace during the publish operation. Temporarily changing the
  user workspace to live not only fixes the reference index, but
  does not create the bogus inline children placeholder records
  anymore, too.

Change-Id: I897f6a93b1d5a579bfa5c52e93e65119a018e4aa
Resolves: #92589
Related: #92467
Releases: master, 10.4
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/66152
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Benni Mack <benni@typo3.org>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Benni Mack <benni@typo3.org>
Reviewed-by: David Steeb <david.steeb@b13.com>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
  • Loading branch information
lolli42 committed Oct 19, 2020
1 parent 17f5519 commit b02172f
Show file tree
Hide file tree
Showing 49 changed files with 161 additions and 317 deletions.
33 changes: 33 additions & 0 deletions typo3/sysext/core/Classes/DataHandling/DataHandler.php
Expand Up @@ -3576,6 +3576,15 @@ public function copyRecord_raw($table, $uid, $pid, $overrideArray = [], array $w
}
// Do the copy by internal function
$theNewSQLID = $this->insertNewCopyVersion($table, $row, $pid);

// When a record is copied in workspace (eg. to create a delete placeholder record for a live record), records
// pointing to that record need a reference index update. This is for instance the case in FAL, if a sys_file_reference
// for a eg. tt_content record is marked as deleted. The tt_content record then needs a reference index update.
// This scenario seems to currently only show up if in workspaces, so the refindex update is restricted to this for now.
if (!empty($workspaceOptions)) {
$this->referenceIndexUpdater->registerUpdateForReferencesToItem($table, (int)$row['uid'], (int)$this->BE_USER->workspace);
}

if ($theNewSQLID) {
$this->dbAnalysisStoreExec();
$this->dbAnalysisStore = [];
Expand Down Expand Up @@ -4716,11 +4725,23 @@ public function deleteVersionsForRecord($table, $uid, $forceHardDelete)
if (is_array($versions)) {
foreach ($versions as $verRec) {
if (!$verRec['_CURRENT_VERSION']) {
$currentUserWorkspace = null;
if ((int)$verRec['t3ver_wsid'] !== (int)$this->BE_USER->workspace) {
// If deleting records from 'foreign' / 'other' workspaces, the be user must be put into
// this workspace temporarily so stuff like refindex updating is registered for this workspace
// when deleting records in there.
$currentUserWorkspace = $this->BE_USER->workspace;
$this->BE_USER->workspace = (int)$verRec['t3ver_wsid'];
}
if ($table === 'pages') {
$this->deletePages($verRec['uid'], true, $forceHardDelete);
} else {
$this->deleteRecord($table, $verRec['uid'], true, $forceHardDelete);
}
if ($currentUserWorkspace !== null) {
// Switch back workspace
$this->BE_USER->workspace = $currentUserWorkspace;
}
}
}
}
Expand Down Expand Up @@ -4974,6 +4995,18 @@ public function deleteSpecificPage($uid, $forceHardDelete = false, bool $deleteR
)
);
}

$currentUserWorkspace = (int)$this->BE_USER->workspace;
if ($currentUserWorkspace !== 0 && BackendUtility::isTableWorkspaceEnabled($table)) {
// If we are in a workspace, make sure only records of this workspace are deleted.
$queryBuilder->andWhere(
$queryBuilder->expr()->eq(
't3ver_wsid',
$queryBuilder->createNamedParameter($currentUserWorkspace, \PDO::PARAM_INT)
)
);
}

$statement = $queryBuilder->execute();

while ($row = $statement->fetch()) {
Expand Down
Expand Up @@ -15,8 +15,10 @@

namespace TYPO3\CMS\Core\Tests\Functional\DataHandling\Regular;

use TYPO3\CMS\Core\Database\ReferenceIndex;
use TYPO3\CMS\Core\Migrations\TcaMigration;
use TYPO3\CMS\Core\Tests\Functional\DataHandling\AbstractDataHandlerActionTestCase;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Core\Utility\StringUtility;

/**
Expand Down Expand Up @@ -191,10 +193,17 @@ public function localizeContentWithEmptyTcaIntegrityColumns()
foreach ($integrityFieldNames as $integrityFieldName) {
unset($GLOBALS['TCA'][self::TABLE_Content]['columns'][$integrityFieldName]);
}
// After TCA changes, refindex is not ok anymore for imported rows. Update it before performing other actions.
$referenceIndex = GeneralUtility::makeInstance(ReferenceIndex::class);
$referenceIndex->updateIndex(false);

// explicitly call TcaMigration (which was executed already earlier in functional testing bootstrap)
$GLOBALS['TCA'] = (new TcaMigration())->migrate($GLOBALS['TCA']);
// create translated page first
$this->actionService->copyRecordToLanguage(self::TABLE_Page, self::VALUE_PageId, self::VALUE_LanguageId);
// perform actions to be tested
self::localizeContent();
$localizedTableIds = $this->actionService->localizeRecord(self::TABLE_Content, self::VALUE_ContentIdSecond, self::VALUE_LanguageId);
$this->recordIds['localizedContentId'] = $localizedTableIds[self::TABLE_Content][self::VALUE_ContentIdSecond];
}

public function localizeContentWithLanguageSynchronization()
Expand Down Expand Up @@ -399,6 +408,8 @@ public function localizePageAndContentsAndDeletePageLocalization()
$this->recordIds['localizedPageId'] = $localizedTableIds[self::TABLE_Page][self::VALUE_PageId];
$localizedTableIds = $this->actionService->localizeRecord(self::TABLE_Content, self::VALUE_ContentIdSecond, self::VALUE_LanguageId);
$this->recordIds['localizedContentId'] = $localizedTableIds[self::TABLE_Content][self::VALUE_ContentIdSecond];
// Deleting the localized page should also delete its localized records
$this->actionService->deleteRecord(self::TABLE_Page, $this->recordIds['localizedPageId']);
}

public function localizeNestedPagesAndContents()
Expand Down
Expand Up @@ -269,18 +269,12 @@ public function localizeContent()
*/
public function localizeContentWithEmptyTcaIntegrityColumns()
{
// Create translated page first
$this->actionService->copyRecordToLanguage(self::TABLE_Page, self::VALUE_PageId, self::VALUE_LanguageId);
parent::localizeContentWithEmptyTcaIntegrityColumns();
$this->assertAssertionDataSet('localizeContentWithEmptyTcaIntegrityColumns');

$responseSections = $this->getFrontendResponse(self::VALUE_PageId, self::VALUE_LanguageId)->getResponseSections();
self::assertThat($responseSections, $this->getRequestSectionHasRecordConstraint()
->setTable(self::TABLE_Content)->setField('header')->setValues('[Translate to Dansk:] Regular Element #1', '[Translate to Dansk:] Regular Element #2'));

// Due to the changed TCA columns passthrough vs select, reference index updates are performed
// differently for this test. We disable the 'clean refindex' test in tearDown() here.
$this->assertCleanReferenceIndex = false;
}

/**
Expand Down Expand Up @@ -716,9 +710,6 @@ public function localizePageAndContentsAndDeletePageLocalization()
{
// Create localized page and localize content elements first
parent::localizePageAndContentsAndDeletePageLocalization();

// Deleting the localized page should also delete its localized records
$this->actionService->deleteRecord(self::TABLE_Page, $this->recordIds['localizedPageId']);
$this->assertAssertionDataSet('localizePageAndContentsAndDeletePageLocalization');
}

Expand Down
Expand Up @@ -11,8 +11,5 @@
"sys_refindex",,,,,,,,,,,,,
,"hash","tablename","recuid","field","flexpointer","softref_key","softref_id","sorting","deleted","workspace","ref_table","ref_uid","ref_string"
,"8765176f1ce58edcb5efdcabd59ca123","sys_category",31,"parent",,,,0,0,0,"sys_category",28,
,"acac9935bf9b184ae25ee925d8457c25","tt_content",300,"l18n_parent",,,,0,0,0,"tt_content",299,
,"eb360eb09940bbb656509c7f6fda9b05","tt_content",301,"l18n_parent",,,,0,0,0,"tt_content",297,
,"b6e55a3d99888c9a7007226ad685306c","tt_content",302,"l18n_parent",,,,0,0,0,"tt_content",297,
,"7495604375ab4caae25ea3da5e76f6c3","pages",91,"l10n_parent",,,,0,0,0,"pages",89,
,"885fa8b0049daa6a9f6989e7bc55ae11","pages",91,"sys_language_uid",,,,0,0,0,"sys_language",1,
15 changes: 11 additions & 4 deletions typo3/sysext/workspaces/Classes/Hook/DataHandlerHook.php
Expand Up @@ -711,12 +711,19 @@ protected function version_swap($table, $id, $swapWith, DataHandler $dataHandler
// Register swapped ids for later remapping:
$this->remappedIds[$table][$id] = $swapWith;
$this->remappedIds[$table][$swapWith] = $id;
// Checking for delete:
// Delete only if new/deleted placeholders are there.
if (((int)$t3ver_state['swapVersion'] === VersionState::NEW_PLACEHOLDER || (int)$t3ver_state['swapVersion'] === VersionState::DELETE_PLACEHOLDER)) {
// Force delete
if ((int)$t3ver_state['swapVersion'] === VersionState::NEW_PLACEHOLDER) {
// Delete t3ver_state = 1 record as t3ver_state = -1 record is going to be live
$dataHandler->deleteEl($table, $id, true);
}
if ((int)$t3ver_state['swapVersion'] === VersionState::DELETE_PLACEHOLDER) {
// We're publishing a delete placeholder t3ver_state = 2. This means the live record should
// be set to deleted. We're currently in some workspace and deal with a live record here. Thus,
// we temporarily set backend user workspace to 0 so all operations happen as in live.
$currentUserWorkspace = $dataHandler->BE_USER->workspace;
$dataHandler->BE_USER->workspace = 0;
$dataHandler->deleteEl($table, $id, true);
$dataHandler->BE_USER->workspace = $currentUserWorkspace;
}
if ($dataHandler->enableLogging) {
$dataHandler->log($table, $id, SystemLogGenericAction::UNDEFINED, 0, SystemLogErrorClassification::MESSAGE, 'Publishing successful for table "' . $table . '" uid ' . $id . '=>' . $swapWith, -1, [], $dataHandler->eventPid($table, $id, $swapVersion['pid']));
}
Expand Down
Expand Up @@ -228,9 +228,6 @@ public function modifyContentAndDeleteFileReference()
self::assertThat($responseSections, $this->getRequestSectionStructureDoesNotHaveRecordConstraint()
->setRecordIdentifier(self::TABLE_Content . ':' . self::VALUE_ContentIdLast)->setRecordField(self::FIELD_ContentImage)
->setTable(self::TABLE_FileReference)->setField('title')->setValues('Taken at T3BOARD'));

// @todo: reference index not clean after this test. Needs investigation.
$this->assertCleanReferenceIndex = false;
}

/**
Expand All @@ -245,9 +242,6 @@ public function modifyContentAndDeleteAllFileReference()
self::assertThat($responseSections, $this->getRequestSectionStructureDoesNotHaveRecordConstraint()
->setRecordIdentifier(self::TABLE_Content . ':' . self::VALUE_ContentIdLast)->setRecordField(self::FIELD_ContentImage)
->setTable(self::TABLE_FileReference)->setField('title')->setValues('Taken at T3BOARD', 'This is Kasper'));

// @todo: reference index not clean after this test. Needs investigation.
$this->assertCleanReferenceIndex = false;
}

/**
Expand Down
Expand Up @@ -30,7 +30,7 @@
,"fc208fc9d55a71b2faa9f4e4d4fa941d","tt_content",330,"image",,,,1,0,0,"sys_file_reference",126,,,,,,,
,"48540de7710e3082f347bdd65ca340f2","tt_content",331,"image",,,,0,0,0,"sys_file_reference",128,,,,,,,
,"ceaebd2148901a6d7b0a52c546aa5218","tt_content",331,"image",,,,1,0,0,"sys_file_reference",129,,,,,,,
,"9b3ba47f25f09e445354845b22e252b5","tt_content",332,"image",,,,0,0,1,"sys_file_reference",128,,,,,,,
,"e76fc3ad92291277bca43e6f234f7527","tt_content",332,"image",,,,1,0,1,"sys_file_reference",129,,,,,,,
,"89012eac2e5e5270bf76a165f513d70b","tt_content",332,"image",,,,0,0,1,"sys_file_reference",131,,,,,,,
,"ad864b7fbebc8d3d065e8481daf769d2","tt_content",332,"image",,,,1,0,1,"sys_file_reference",130,,,,,,,
,"0980446befe1793ccdfce909b8e4b21e","sys_file_reference",130,"uid_local",,,,0,0,1,"sys_file",1,,,,,,,
,"cdd72700845a23a1296c927245325605","sys_file_reference",131,"uid_local",,,,0,0,1,"sys_file",21,,,,,,,
Expand Up @@ -31,6 +31,6 @@
,"48540de7710e3082f347bdd65ca340f2","tt_content",331,"image",,,,0,0,0,"sys_file_reference",128,,,,,,,
,"ceaebd2148901a6d7b0a52c546aa5218","tt_content",331,"image",,,,1,0,0,"sys_file_reference",129,,,,,,,
,"0980446befe1793ccdfce909b8e4b21e","sys_file_reference",130,"uid_local",,,,0,0,1,"sys_file",1,,,,,,,
,"9b3ba47f25f09e445354845b22e252b5","tt_content",332,"image",,,,0,0,1,"sys_file_reference",128,,,,,,,
,"ad864b7fbebc8d3d065e8481daf769d2","tt_content",332,"image",,,,1,0,1,"sys_file_reference",130,,,,,,,
,"2dd6f68f7a7d8474b2700bb709c4c988","tt_content",332,"image",,,,0,0,1,"sys_file_reference",130,,,,,,,
,"15ecf1ccb07af00b78a32622bdd995f7","tt_content",332,"image",,,,1,0,1,"sys_file_reference",131,,,,,,,
,"cdd72700845a23a1296c927245325605","sys_file_reference",131,"uid_local",,,,0,0,1,"sys_file",21,,,,,,,
Expand Up @@ -4,6 +4,8 @@
,127,89,0,0,0,0,0,0,0,21,330,"tt_content","image",1,"sys_file","Kasper",,,
,128,89,1,0,0,0,0,0,0,21,331,"tt_content","image",1,"sys_file","Taken at T3BOARD",,,
,129,89,1,0,0,0,0,0,0,1,331,"tt_content","image",2,"sys_file","This is Kasper",,,
,130,89,1,0,0,1,2,0,128,21,331,"tt_content","image",1,"sys_file","Taken at T3BOARD",,,
,131,89,1,0,0,1,2,0,129,1,331,"tt_content","image",2,"sys_file","This is Kasper",,,
"tt_content",,,,,,,,,,,,,,,,,,,
,"uid","pid","sorting","deleted","sys_language_uid","l18n_parent","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","header","image",,,,,,,
,330,89,256,0,0,0,0,0,0,0,"Regular Element #1",2,,,,,,,
Expand All @@ -25,3 +27,5 @@
,"fc208fc9d55a71b2faa9f4e4d4fa941d","tt_content",330,"image",,,,1,0,0,"sys_file_reference",126,,,,,,,
,"0b7edf2205371519698cdfbf6e0cc6e7","sys_file_reference",128,"uid_local",,,,0,1,0,"sys_file",21,,,,,,,
,"fb266d0b94cee39b87fe03b9b7875c25","sys_file_reference",129,"uid_local",,,,0,1,0,"sys_file",1,,,,,,,
,"901c0e3cec70fba54579674ab0fab258","sys_file_reference",130,"uid_local",,,,0,1,1,"sys_file",21,,,,,,,
,"eab0cafc17f8d63c1ebd4994d7ab7413","sys_file_reference",131,"uid_local",,,,0,1,1,"sys_file",1,,,,,,,
Expand Up @@ -4,6 +4,8 @@
,127,89,0,0,0,0,0,0,0,21,330,"tt_content","image",1,"sys_file","Kasper",,,
,128,89,1,0,0,0,0,0,0,21,331,"tt_content","image",1,"sys_file","Taken at T3BOARD",,,
,129,89,1,0,0,0,0,0,0,1,331,"tt_content","image",2,"sys_file","This is Kasper",,,
,130,89,1,0,0,1,2,0,128,21,331,"tt_content","image",1,"sys_file","Taken at T3BOARD",,,
,131,89,1,0,0,1,2,0,129,1,331,"tt_content","image",2,"sys_file","This is Kasper",,,
"tt_content",,,,,,,,,,,,,,,,,,,
,"uid","pid","sorting","deleted","sys_language_uid","l18n_parent","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","header","image",,,,,,,
,330,89,256,0,0,0,0,0,0,0,"Regular Element #1",2,,,,,,,
Expand All @@ -25,3 +27,5 @@
,"fc208fc9d55a71b2faa9f4e4d4fa941d","tt_content",330,"image",,,,1,0,0,"sys_file_reference",126,,,,,,,
,"0b7edf2205371519698cdfbf6e0cc6e7","sys_file_reference",128,"uid_local",,,,0,1,0,"sys_file",21,,,,,,,
,"fb266d0b94cee39b87fe03b9b7875c25","sys_file_reference",129,"uid_local",,,,0,1,0,"sys_file",1,,,,,,,
,"901c0e3cec70fba54579674ab0fab258","sys_file_reference",130,"uid_local",,,,0,1,1,"sys_file",21,,,,,,,
,"eab0cafc17f8d63c1ebd4994d7ab7413","sys_file_reference",131,"uid_local",,,,0,1,1,"sys_file",1,,,,,,,
Expand Up @@ -221,9 +221,6 @@ public function deletePage()
(new InternalRequest())->withPageId(self::VALUE_PageId)
);
self::assertEquals(404, $response->getStatusCode());

// @todo: reference index not clean after this test. Needs investigation.
$this->assertCleanReferenceIndex = false;
}

/**
Expand Down

0 comments on commit b02172f

Please sign in to comment.