Skip to content

Commit

Permalink
[BUGFIX] Only fix versioning PID for moved records
Browse files Browse the repository at this point in the history 
Since the removal of "pid=-1" in TYPO3 v10, it is not necessary
to fix the versioning PID for all versioned records anymore.

The checks in BackendUtility::fixVersioningPid() and
PageRepository->fixVersioningPid() are cleaned up, because:

* The only version type that has a DIFFERENT page ID than
its online pendant is the MOVE_POINTER (t3ver_state=4)
* The MOVE_PLACEHOLDER does not contain a t3ver_oid
and is not evaluated ever in these methods
* All other t3ver_state items have the same PID as the live
version, and should not be evaluated.

Resolves: #92314
Releases: master, 10.4
Change-Id: I0551547bd2bb9f469ed3a68396b74c0d4de1a18d
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/65772
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Benni Mack <benni@typo3.org>
  • Loading branch information
@bmack
bmack committed Sep 18, 2020
1 parent 9ed135c commit 8b5bbd0
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 129 deletions.
82 changes: 46 additions & 36 deletions typo3/sysext/backend/Classes/Utility/BackendUtility.php
View file
Expand Up @@ -3505,7 +3505,7 @@ public static function selectVersionsOfRecord(
* to detect if a record was in fact in a versionized branch.
*
* @param string $table Table name
* @param array $rr Record array passed by reference. As minimum, "pid" and "uid" fields must exist! "t3ver_oid" and "t3ver_wsid" is nice and will save you a DB query.
* @param array $rr Record array passed by reference. As minimum, "pid" and "uid" fields must exist! "t3ver_oid", "t3ver_state" and "t3ver_wsid" is nice and will save you a DB query.
* @param bool $ignoreWorkspaceMatch Ignore workspace match
* @see PageRepository::fixVersioningPid()
* @internal should only be used from within TYPO3 Core
Expand All @@ -3518,43 +3518,53 @@ public static function fixVersioningPid($table, &$rr, $ignoreWorkspaceMatch = fa
if (!static::isTableWorkspaceEnabled($table)) {
return;
}
// Check that the input record is an offline version from a table that supports versioning:
if (is_array($rr)) {
// Check values for t3ver_oid and t3ver_wsid:
if (isset($rr['t3ver_oid']) && isset($rr['t3ver_wsid'])) {
// If "t3ver_oid" is already a field, just set this:
$oid = $rr['t3ver_oid'];
$wsid = $rr['t3ver_wsid'];
} else {
$oid = 0;
$wsid = 0;
// Otherwise we have to expect "uid" to be in the record and look up based on this:
$newPidRec = self::getRecord($table, $rr['uid'], 't3ver_oid,t3ver_wsid');
if (is_array($newPidRec)) {
$oid = $newPidRec['t3ver_oid'];
$wsid = $newPidRec['t3ver_wsid'];
}
}
// If ID of current online version is found, look up the PID value of that:
if ($oid
&& ($ignoreWorkspaceMatch || (static::getBackendUserAuthentication() instanceof BackendUserAuthentication && (int)$wsid === (int)static::getBackendUserAuthentication()->workspace))
) {
$oidRec = self::getRecord($table, $oid, 'pid');
if (is_array($oidRec)) {
$rr['_ORIG_pid'] = $rr['pid'];
$rr['pid'] = $oidRec['pid'];
// Check that the input record is an offline version from a table that supports versioning
if (!is_array($rr)) {
return;
}
$incomingPid = $rr['pid'] ?? null;
// Check values for t3ver_oid and t3ver_wsid:
if (isset($rr['t3ver_oid']) && isset($rr['t3ver_wsid']) && isset($rr['t3ver_state'])) {
// If "t3ver_oid" is already a field, just set this:
$oid = $rr['t3ver_oid'];
$workspaceId = (int)$rr['t3ver_wsid'];
$versionState = (int)$rr['t3ver_state'];
} else {
$oid = 0;
$workspaceId = 0;
$versionState = 0;
// Otherwise we have to expect "uid" to be in the record and look up based on this:
$newPidRec = self::getRecord($table, $rr['uid'], 'pid,t3ver_oid,t3ver_wsid,t3ver_state');
if (is_array($newPidRec)) {
$incomingPid = $newPidRec['pid'];
$oid = $newPidRec['t3ver_oid'];
$workspaceId = $newPidRec['t3ver_wsid'];
$versionState = $newPidRec['t3ver_state'];
}
}
if ($oid && ($ignoreWorkspaceMatch || (static::getBackendUserAuthentication() instanceof BackendUserAuthentication && $workspaceId === (int)static::getBackendUserAuthentication()->workspace))) {
if ($incomingPid === null) {
// This can be removed, as this is the same for all versioned records
$onlineRecord = self::getRecord($table, $oid, 'pid');
if (is_array($onlineRecord)) {
$rr['_ORIG_pid'] = $onlineRecord['pid'];
$rr['pid'] = $onlineRecord['pid'];
}
// Use target PID in case of move pointer
if (
!isset($rr['t3ver_state'])
|| VersionState::cast($rr['t3ver_state'])->equals(VersionState::MOVE_POINTER)
) {
$movePlaceholder = self::getMovePlaceholder($table, $oid, 'pid');
if ($movePlaceholder) {
$rr['_ORIG_pid'] = $rr['pid'];
$rr['pid'] = $movePlaceholder['pid'];
}
} else {
// This can be removed, as this is the same for all versioned records (clearly obvious here)
$rr['_ORIG_pid'] = $incomingPid;
$rr['pid'] = $incomingPid;
}
// Use moved PID in case of move pointer
if ($versionState === VersionState::MOVE_POINTER) {
if ($incomingPid !== null) {
$movedPageIdInWorkspace = $incomingPid;
} else {
$versionedMovePointer = self::getRecord($table, $rr['uid'], 'pid');
$movedPageIdInWorkspace = $versionedMovePointer['pid'];
}
$rr['_ORIG_pid'] = $incomingPid;
$rr['pid'] = $movedPageIdInWorkspace;
}
}
}
Expand Down
1 change: 1 addition & 0 deletions typo3/sysext/backend/Tests/Unit/Utility/BackendUtilityTest.php
View file
Expand Up @@ -1097,6 +1097,7 @@ public function fixVersioningPidDoesNotChangeValuesForNoBeUserAvailable()
'pid' => -1,
't3ver_oid' => 7,
't3ver_wsid' => 42,
't3ver_state' => 0
];
$reference = $rr;
BackendUtility::fixVersioningPid($tableName, $rr);
Expand Down
197 changes: 104 additions & 93 deletions typo3/sysext/core/Classes/Domain/Repository/PageRepository.php
View file
Expand Up @@ -1449,78 +1449,81 @@ public function getMultipleGroupsWhereClause($field, $table)
* ONLY active when backend user is previewing records. MUST NEVER affect a site
* served which is not previewed by backend users!!!
*
* Will look if the "pid" value of the input record is -1 (it is an offline
* version) and if the table supports versioning; if so, it will translate the -1
* PID into the PID of the original record.
* What happens in this method:
* If a record was moved in a workspace, the records' PID might be different. This is only reason
* nowadays why this method exists.
*
* This is checked:
* 1. If the record has a "online pendant" (t3ver_oid > 0), it overrides the "pid" with the one from the online version.
* 2. If a record is a live version, check if there is a moved version in this workspace, and override the LIVE version with the new moved "pid" value.
*
* Used whenever you are tracking something back, like making the root line.
*
* Principle; Record offline! => Find online?
*
* @param string $table Table name
* @param array $rr Record array passed by reference. As minimum, "pid" and "uid" fields must exist! "t3ver_oid" and "t3ver_wsid" is nice and will save you a DB query.
* @param array $rr Record array passed by reference. As minimum, "pid" and "uid" fields must exist! "t3ver_oid", "t3ver_state" and "t3ver_wsid", "t3ver_state" is nice and will save you a DB query.
* @see BackendUtility::fixVersioningPid()
* @see versionOL()
* @see getRootLine()
*/
public function fixVersioningPid($table, &$rr)
{
if ($this->versioningWorkspaceId > 0 && is_array($rr) && $this->hasTableWorkspaceSupport($table)) {
$oid = 0;
$wsid = 0;
// Check values for t3ver_oid and t3ver_wsid:
if (isset($rr['t3ver_oid']) && isset($rr['t3ver_wsid'])) {
// If "t3ver_oid" is already a field, just set this:
$oid = $rr['t3ver_oid'];
$wsid = $rr['t3ver_wsid'];
} else {
// Otherwise we have to expect "uid" to be in the record and look up based
// on this:
$uid = (int)$rr['uid'];
if ($uid > 0) {
$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($table);
$queryBuilder->getRestrictions()
->removeAll()
->add(GeneralUtility::makeInstance(DeletedRestriction::class));
$newPidRec = $queryBuilder->select('t3ver_oid', 't3ver_wsid')
->from($table)
->where($queryBuilder->expr()->eq('uid', $queryBuilder->createNamedParameter($uid, \PDO::PARAM_INT)))
->execute()
->fetch();
if ($this->versioningWorkspaceId <= 0) {
return;
}
if (!is_array($rr)) {
return;
}
if (!$this->hasTableWorkspaceSupport($table)) {
return;
}
$uid = (int)$rr['uid'];
$oid = 0;
$workspaceId = 0;
$versionState = null;
// Check values for t3ver_oid and t3ver_wsid
if (isset($rr['t3ver_oid']) && isset($rr['t3ver_wsid']) && isset($rr['t3ver_state'])) {
// If "t3ver_oid" is already a field, just set this:
$oid = $rr['t3ver_oid'];
$workspaceId = (int)$rr['t3ver_wsid'];
$versionState = (int)$rr['t3ver_state'];
} elseif ($uid > 0) {
// Otherwise we have to expect "uid" to be in the record and look up based
// on this:
$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($table);
$queryBuilder->getRestrictions()
->removeAll()
->add(GeneralUtility::makeInstance(DeletedRestriction::class));
$newPidRec = $queryBuilder->select('t3ver_oid', 't3ver_wsid', 't3ver_state')
->from($table)
->where($queryBuilder->expr()->eq('uid', $queryBuilder->createNamedParameter($uid, \PDO::PARAM_INT)))
->execute()
->fetch();

if (is_array($newPidRec)) {
$oid = $newPidRec['t3ver_oid'];
$wsid = $newPidRec['t3ver_wsid'];
}
}
if (is_array($newPidRec)) {
$oid = $newPidRec['t3ver_oid'];
$workspaceId = (int)$newPidRec['t3ver_wsid'];
$versionState = (int)$newPidRec['t3ver_state'];
}
// If workspace ids matches and ID of current online version is found, look up
// the PID value of that:
if ($oid && (int)$wsid === (int)$this->versioningWorkspaceId) {
$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($table);
$queryBuilder->getRestrictions()
->removeAll()
->add(GeneralUtility::makeInstance(DeletedRestriction::class));
$oidRec = $queryBuilder->select('pid')
->from($table)
->where($queryBuilder->expr()->eq('uid', $queryBuilder->createNamedParameter($oid, \PDO::PARAM_INT)))
->execute()
->fetch();
}

if (is_array($oidRec)) {
// SWAP uid as well? Well no, because when fixing a versioning PID happens it is
// assumed that this is a "branch" type page and therefore the uid should be
// kept (like in versionOL()). However if the page is NOT a branch version it
// should not happen - but then again, direct access to that uid should not
// happen!
$rr['_ORIG_pid'] = $rr['pid'];
$rr['pid'] = $oidRec['pid'];
}
}
// Workspace does not match, so this is skipped
if ($workspaceId !== (int)$this->versioningWorkspaceId) {
return;
}
// Now set the "pid" properly.
// This will only make a difference when handing in move placeholders but is kept for backwards-compat
// however it's always the same for live + versioned record, except for moving, where _ORIG_pid will
// be set to the live PID, and pid to the moved location
if ($oid) {
$rr['_ORIG_pid'] = $rr['pid'];
}
// Changing PID in case of moving pointer:
if ($movePlhRec = $this->getMovePlaceholder($table, $rr['uid'], 'pid')) {
$rr['pid'] = $movePlhRec['pid'];
// Changing PID in case there is a move pointer
// This happens if the $uid is still a live version but the overlay happened (via t3ver_oid) and the t3ver_state was
// Changed to MOVE_POINTER. This logic happens in versionOL(), where the "pid" of the live version is kept.
if ($versionState === VersionState::MOVE_POINTER && $movedPageId = $this->getMovedPidOfVersionedRecord($table, $uid)) {
$rr['_ORIG_pid'] = $rr['pid'];
$rr['pid'] = $movedPageId;
}
}

Expand Down Expand Up @@ -1561,6 +1564,8 @@ public function versionOL($table, &$row, $unsetMovePointers = false, $bypassEnab
// Keep the old (-1) - indicates it was a version...
$wsAlt['_ORIG_pid'] = $wsAlt['pid'];
// Set in the online versions PID.
// Side note: For move pointers, this is set back to the PID of the live record now
// The only place where PID is actually different.
$wsAlt['pid'] = $row['pid'];
// For versions of single elements or page+content, preserve online UID and PID
// (this will produce true "overlay" of element _content_, not any references)
Expand Down Expand Up @@ -1671,52 +1676,58 @@ protected function movePlhOL($table, &$row)
}

/**
* Returns move placeholder of online (live) version
* Returns the PID of the new (moved) location within a version, when a $liveUid is given.
*
* Please note: This is only performed within a workspace.
* This was previously stored in the move placeholder's PID, but move pointer's PID and move placeholder's PID
* are the same since TYPO3 v10, so the MOVE_POINTER is queried.
*
* @param string $table Table name
* @param int $uid Record UID of online version
* @param string $fields Field list, default is *
* @return array If found, the record, otherwise nothing.
* @param int $liveUid Record UID of online version
* @return int|null If found, the Page ID of the moved record, otherwise null.
* @see BackendUtility::getMovePlaceholder()
*/
protected function getMovePlaceholder($table, $uid, $fields = '*')
protected function getMovedPidOfVersionedRecord(string $table, int $liveUid): ?int
{
$workspace = (int)$this->versioningWorkspaceId;
if ($workspace > 0 && $this->hasTableWorkspaceSupport($table)) {
// Select workspace version of record:
$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($table);
$queryBuilder->getRestrictions()
->removeAll()
->add(GeneralUtility::makeInstance(DeletedRestriction::class));
if ($this->versioningWorkspaceId <= 0) {
return null;
}
if (!$this->hasTableWorkspaceSupport($table)) {
return null;
}
// Select workspace version of record
$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($table);
$queryBuilder->getRestrictions()
->removeAll()
->add(GeneralUtility::makeInstance(DeletedRestriction::class));

$row = $queryBuilder->select(...GeneralUtility::trimExplode(',', $fields, true))
->from($table)
->where(
$queryBuilder->expr()->eq(
't3ver_state',
$queryBuilder->createNamedParameter(
(string)VersionState::cast(VersionState::MOVE_PLACEHOLDER),
\PDO::PARAM_INT
)
),
$queryBuilder->expr()->eq(
't3ver_move_id',
$queryBuilder->createNamedParameter($uid, \PDO::PARAM_INT)
),
$queryBuilder->expr()->eq(
't3ver_wsid',
$queryBuilder->createNamedParameter($workspace, \PDO::PARAM_INT)
$row = $queryBuilder->select('pid')
->from($table)
->where(
$queryBuilder->expr()->eq(
't3ver_state',
$queryBuilder->createNamedParameter(
(string)VersionState::cast(VersionState::MOVE_POINTER),
\PDO::PARAM_INT
)
),
$queryBuilder->expr()->eq(
't3ver_oid',
$queryBuilder->createNamedParameter($liveUid, \PDO::PARAM_INT)
),
$queryBuilder->expr()->eq(
't3ver_wsid',
$queryBuilder->createNamedParameter($this->versioningWorkspaceId, \PDO::PARAM_INT)
)
->setMaxResults(1)
->execute()
->fetch();
)
->setMaxResults(1)
->execute()
->fetch();

if (is_array($row)) {
return $row;
}
if (is_array($row)) {
return (int)$row['pid'];
}
return false;
return null;
}

/**
Expand Down
5 changes: 5 additions & 0 deletions typo3/sysext/core/Classes/Routing/PageSlugCandidateProvider.php
View file
Expand Up @@ -231,6 +231,11 @@ protected function getPagesFromDatabaseForCandidates(array $slugCandidates, int

while ($row = $statement->fetch()) {
$mountPageInformation = null;
// This changes the PID value and adds a _ORIG_PID value (only different in move actions)
// In live: This fetches everything in a bad way ! as there is no workspace limitation given, fetching all new and moved placeholders here!
// In a workspace: Filter out versioned records (t3ver_oid=0), leaving effectively the new/move placeholders in place, where the new placeholder
// However, this is checked in $siteFinder->getSiteByPageId() via RootlineUtility where overlays are happening
// so the fixVersioningPid() call is probably irrelevant.
$pageRepository->fixVersioningPid('pages', $row);
$pageIdInDefaultLanguage = (int)($languageId > 0 ? $row['l10n_parent'] : $row['uid']);
// When this page was added before via recursion, this page should be skipped
Expand Down

0 comments on commit 8b5bbd0

Please sign in to comment.