From 7d1506eef6736778feebe0e2b1b8370d4f9fc1f2 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Tue, 7 Nov 2017 07:36:31 +0100
Subject: [PATCH] [TASK] Reduce PHP queries of pagetree SQL
The TYPO3 Backend PageTree, built in TYPO3 4.5, has
some strange quirks resolving DB records, doing
a lot of queries for fetching a single page multiple
times. This is unnecessary because this can be fetched
with one query, which happens anyway. Additionally,
the WSOL should only happen when a workspace is selected.
Explicitly querying for "-1" pid etc. should be handled
via SQL, and not explicitly implemented by the page tree.
Bottom line: Remove one SQL-query per page which is loaded.
As most of the PHP classes will be restructured with the upcoming
ExtJS / ExtDirect removal, the removed methods will be part
of the breaking change of ExtJS removal.
Resolves: #82945
Releases: master
Change-Id: I7b1d79b40d0e9212cc0884c9440e5725e4f74d8e
Reviewed-on: https://review.typo3.org/54574
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
Reviewed-by: Stefan Neufeind <typo3.neufeind@speedpartner.de>
Tested-by: Stefan Neufeind <typo3.neufeind@speedpartner.de>
---
.../Classes/Tree/Pagetree/Commands.php | 17 +-
.../Classes/Tree/Pagetree/DataProvider.php | 165 ++++++++++--------
.../Tree/Pagetree/ExtdirectTreeCommands.php | 2 +-
.../Classes/Utility/BackendUtility.php | 41 +++--
4 files changed, 120 insertions(+), 105 deletions(-)
diff --git a/typo3/sysext/backend/Classes/Tree/Pagetree/Commands.php b/typo3/sysext/backend/Classes/Tree/Pagetree/Commands.php
index 13f80f041832..551e16d31e79 100644
--- a/typo3/sysext/backend/Classes/Tree/Pagetree/Commands.php
+++ b/typo3/sysext/backend/Classes/Tree/Pagetree/Commands.php
@@ -227,7 +227,7 @@ class Commands
*/
public static function getNode($nodeId, $unsetMovePointers = true)
{
- $record = self::getNodeRecord($nodeId, $unsetMovePointers);
+ $record = BackendUtility::getRecordWSOL('pages', $nodeId, '*', '', true, $unsetMovePointers);
return self::getNewNode($record);
}
@@ -252,7 +252,7 @@ class Commands
array_shift($rootline);
$path = [];
foreach ($rootline as $rootlineElement) {
- $record = self::getNodeRecord($rootlineElement['uid']);
+ $record = BackendUtility::getRecordWSOL('pages', $rootlineElement['uid'], 'title, nav_title', '', true, true);
$text = $record['title'];
if (self::$useNavTitle && trim($record['nav_title']) !== '') {
$text = $record['nav_title'];
@@ -262,19 +262,6 @@ class Commands
return '/' . implode('/', $path);
}
- /**
- * Returns a node record from a given id
- *
- * @param int $nodeId
- * @param bool $unsetMovePointers
- * @return array
- */
- public static function getNodeRecord($nodeId, $unsetMovePointers = true)
- {
- $record = BackendUtility::getRecordWSOL('pages', $nodeId, '*', '', true, $unsetMovePointers);
- return $record;
- }
-
/**
* Returns the first configured domain name for a page
*
diff --git a/typo3/sysext/backend/Classes/Tree/Pagetree/DataProvider.php b/typo3/sysext/backend/Classes/Tree/Pagetree/DataProvider.php
index 5618bea16bac..5e86582c0b7b 100644
--- a/typo3/sysext/backend/Classes/Tree/Pagetree/DataProvider.php
+++ b/typo3/sysext/backend/Classes/Tree/Pagetree/DataProvider.php
@@ -127,16 +127,15 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
if (!(int)$GLOBALS['BE_USER']->uc['pageTree_temporaryMountPoint']) {
$mountPoints = array_map('intval', $GLOBALS['BE_USER']->returnWebmounts());
$mountPoints = array_unique($mountPoints);
- if (!in_array(0, $mountPoints)) {
+ if (!in_array(0, $mountPoints, true)) {
// using a virtual root node
// so then return the mount points here as "subpages" of the first node
$isVirtualRootNode = true;
$subpages = [];
foreach ($mountPoints as $webMountPoint) {
- $subpages[] = [
- 'uid' => $webMountPoint,
- 'isMountPoint' => true
- ];
+ $subpage = BackendUtility::getRecordWSOL('pages', $webMountPoint, '*', '', true, true);
+ $subpage['isMountPoint'] = true;
+ $subpages[] = $subpage;
}
}
}
@@ -144,18 +143,14 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
if (is_array($subpages) && !empty($subpages)) {
$lastRootline = [];
foreach ($subpages as $subpage) {
- if (in_array($subpage['uid'], $this->hiddenRecords)) {
+ if (in_array($subpage['t3ver_oid'] ?: $subpage['uid'], $this->hiddenRecords)) {
continue;
}
- // must be calculated above getRecordWithWorkspaceOverlay,
+ // must be calculated before getRecordWSOL(),
// because the information is lost otherwise
$isMountPoint = $subpage['isMountPoint'] === true;
if ($isVirtualRootNode) {
- $mountPoint = (int)$subpage['uid'];
- }
- $subpage = $this->getRecordWithWorkspaceOverlay($subpage['uid'], true);
- if (!$subpage) {
- continue;
+ $mountPoint = (int)$subpage['t3ver_oid'] ?: $subpage['uid'];
}
$subNode = Commands::getNewNode($subpage, $mountPoint);
$subNode->setIsMountPoint($isMountPoint);
@@ -175,7 +170,7 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
$subNode->setChildNodes($childNodes);
$this->nodeCounter += $childNodes->count();
} else {
- $subNode->setLeaf(!$this->hasNodeSubPages($subNode->getId()));
+ $subNode->setLeaf(!$this->hasNodeSubPages((int)$subNode->getId()));
}
if (!$GLOBALS['BE_USER']->isAdmin() && (int)$subpage['editlock'] === 1) {
$subNode->setLabelIsEditable(false);
@@ -190,18 +185,6 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
return $nodeCollection;
}
- /**
- * Wrapper method for \TYPO3\CMS\Backend\Utility\BackendUtility::getRecordWSOL
- *
- * @param int $uid The page id
- * @param bool $unsetMovePointers Whether to unset move pointers
- * @return array
- */
- protected function getRecordWithWorkspaceOverlay($uid, $unsetMovePointers = false)
- {
- return BackendUtility::getRecordWSOL('pages', $uid, '*', '', true, $unsetMovePointers);
- }
-
/**
* Returns a node collection of filtered nodes
*
@@ -214,7 +197,7 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
{
/** @var $nodeCollection \TYPO3\CMS\Backend\Tree\Pagetree\PagetreeNodeCollection */
$nodeCollection = GeneralUtility::makeInstance(\TYPO3\CMS\Backend\Tree\Pagetree\PagetreeNodeCollection::class);
- $records = $this->getSubpages(-1, $searchFilter);
+ $records = $this->getPagesByQuery($searchFilter);
if (!is_array($records) || empty($records)) {
return $nodeCollection;
}
@@ -234,24 +217,29 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
$nodeId = (int)$node->getId();
$processedRecordIds = [];
foreach ($records as $record) {
- if ((int)$record['t3ver_wsid'] !== (int)$GLOBALS['BE_USER']->workspace && (int)$record['t3ver_wsid'] !== 0) {
- continue;
- }
- $liveVersion = BackendUtility::getLiveVersionOfRecord('pages', $record['uid'], 'uid');
- if ($liveVersion !== null) {
- $record = $liveVersion;
- }
-
- $record = Commands::getNodeRecord($record['uid'], false);
- if ((int)$record['pid'] === -1
- || in_array($record['uid'], $this->hiddenRecords)
- || in_array($record['uid'], $processedRecordIds)
+ $uid = (int)$record['t3ver_oid'] ?: $record['uid'];
+ if (in_array($uid, $this->hiddenRecords) || in_array($uid, $processedRecordIds, true)
+ || (
+ (int)$record['pid'] === -1 && (
+ (int)$record['t3ver_wsid'] === 0
+ || (int)$record['t3ver_wsid'] !== (int)$GLOBALS['BE_USER']->workspace
+ )
+ )
) {
continue;
}
- $processedRecordIds[] = $record['uid'];
+ $processedRecordIds[] = $uid;
- $rootline = BackendUtility::BEgetRootLine($record['uid'], '', $GLOBALS['BE_USER']->workspace != 0);
+ $rootline = BackendUtility::BEgetRootLine(
+ $uid,
+ '',
+ $GLOBALS['BE_USER']->workspace != 0,
+ [
+ 'hidden',
+ 'starttime',
+ 'endtime',
+ ]
+ );
$rootline = array_reverse($rootline);
if (!in_array(0, $mountPoints, true)) {
$isInsideMountPoints = false;
@@ -268,6 +256,7 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
$reference = $nodeCollection;
$inFilteredRootline = false;
$amountOfRootlineElements = count($rootline);
+ // render the root line elements up to the search result
for ($i = 0; $i < $amountOfRootlineElements; ++$i) {
$rootlineElement = $rootline[$i];
$rootlineElement['uid'] = (int)$rootlineElement['uid'];
@@ -288,7 +277,6 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
if (!$inFilteredRootline || $rootlineElement['uid'] === $mountPoint) {
continue;
}
- $rootlineElement = Commands::getNodeRecord($rootlineElement['uid'], false);
$ident = (int)$rootlineElement['sorting'] . (int)$rootlineElement['uid'];
if ($reference && $reference->offsetExists($ident)) {
/** @var $refNode \TYPO3\CMS\Backend\Tree\Pagetree\PagetreeNode */
@@ -353,7 +341,7 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
if (!$mountPoints) {
$mountPoints = array_map('intval', $GLOBALS['BE_USER']->returnWebmounts());
$mountPoints = array_unique($mountPoints);
- if (!in_array(0, $mountPoints)) {
+ if (!in_array(0, $mountPoints, true)) {
$rootNodeIsVirtual = true;
// use a virtual root
// the real mountpoints will be fetched in getNodes() then
@@ -370,13 +358,9 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
foreach ($mountPoints as $mountPoint) {
if ($mountPoint === 0) {
- $sitename = 'TYPO3';
- if ($GLOBALS['TYPO3_CONF_VARS']['SYS']['sitename'] !== '') {
- $sitename = $GLOBALS['TYPO3_CONF_VARS']['SYS']['sitename'];
- }
$record = [
'uid' => 0,
- 'title' => $sitename
+ 'title' => $GLOBALS['TYPO3_CONF_VARS']['SYS']['sitename'] ?: 'TYPO3'
];
$subNode = Commands::getNewNode($record);
$subNode->setLabelIsEditable(false);
@@ -391,7 +375,7 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
if (in_array($mountPoint, $this->hiddenRecords)) {
continue;
}
- $record = $this->getRecordWithWorkspaceOverlay($mountPoint);
+ $record = BackendUtility::getRecordWSOL('pages', $mountPoint);
if (!$record) {
continue;
}
@@ -427,25 +411,15 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
* Sets the Doctrine where clause for fetching pages
*
* @param QueryBuilder $queryBuilder
- * @param int $id
* @param string $searchFilter
* @return QueryBuilder
*/
- protected function setWhereClause(QueryBuilder $queryBuilder, $id, $searchFilter = ''): QueryBuilder
+ protected function setWhereClause(QueryBuilder $queryBuilder, $searchFilter = ''): QueryBuilder
{
$expressionBuilder = $queryBuilder->expr();
$queryBuilder->where(
- QueryHelper::stripLogicalOperatorPrefix($GLOBALS['BE_USER']->getPagePermsClause(1))
- );
-
- if (is_numeric($id) && $id >= 0) {
- $queryBuilder->andWhere(
- $expressionBuilder->eq('pid', $queryBuilder->createNamedParameter($id, \PDO::PARAM_INT))
- );
- }
-
- // Only show records in default language
- $queryBuilder->andWhere(
+ QueryHelper::stripLogicalOperatorPrefix($GLOBALS['BE_USER']->getPagePermsClause(1)),
+ // Only show records in default language
$expressionBuilder->eq('sys_language_uid', $queryBuilder->createNamedParameter(0, \PDO::PARAM_INT))
);
@@ -499,13 +473,49 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
}
/**
- * Returns all sub-pages of a given id
+ * Returns all sub-pages of a given ID
*
* @param int $id
+ * @return array
+ */
+ protected function getSubpages(int $id): array
+ {
+ $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('pages');
+ $queryBuilder->getRestrictions()
+ ->removeAll()
+ ->add(GeneralUtility::makeInstance(DeletedRestriction::class))
+ ->add(GeneralUtility::makeInstance(BackendWorkspaceRestriction::class));
+ $result = [];
+ $queryBuilder->select('*')
+ ->from('pages')
+ ->where(
+ QueryHelper::stripLogicalOperatorPrefix($GLOBALS['BE_USER']->getPagePermsClause(1)),
+ // Only show records in default language
+ $queryBuilder->expr()->eq('sys_language_uid', $queryBuilder->createNamedParameter(0, \PDO::PARAM_INT))
+ )
+ ->orderBy('sorting');
+ if ((int)$id >= 0) {
+ $queryBuilder->andWhere(
+ $queryBuilder->expr()->eq('pid', $queryBuilder->createNamedParameter($id, \PDO::PARAM_INT))
+ );
+ }
+ $queryResult = $queryBuilder->execute();
+ while ($row = $queryResult->fetch()) {
+ BackendUtility::workspaceOL('pages', $row, -99, true);
+ if ($row) {
+ $result[] = $row;
+ }
+ }
+ return $result;
+ }
+
+ /**
+ * Returns all pages with a query.
+ *
* @param string $searchFilter
* @return array
*/
- protected function getSubpages($id, $searchFilter = '')
+ protected function getPagesByQuery(string $searchFilter = ''): array
{
$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('pages');
$queryBuilder->getRestrictions()
@@ -513,34 +523,47 @@ class DataProvider extends \TYPO3\CMS\Backend\Tree\AbstractTreeDataProvider
->add(GeneralUtility::makeInstance(DeletedRestriction::class))
->add(GeneralUtility::makeInstance(BackendWorkspaceRestriction::class));
$result = [];
- $queryBuilder = $this->setWhereClause($queryBuilder, $id, $searchFilter);
- $queryResult = $queryBuilder->select('uid', 't3ver_wsid')
+ $queryBuilder = $this->setWhereClause($queryBuilder, $searchFilter);
+ $queryResult = $queryBuilder->select('*')
->from('pages')
->orderBy('sorting')
->execute();
while ($row = $queryResult->fetch()) {
- $result[$row['uid']] = $row;
+ BackendUtility::workspaceOL('pages', $row, -99, true);
+ if ($row) {
+ $result[] = $row;
+ }
}
return $result;
}
/**
- * Returns TRUE if the node has child's
+ * Returns true if the node has children.
*
* @param int $id
* @return bool
*/
- protected function hasNodeSubPages($id)
+ protected function hasNodeSubPages(int $id): bool
{
$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('pages');
$queryBuilder->getRestrictions()
->removeAll()
->add(GeneralUtility::makeInstance(DeletedRestriction::class))
->add(GeneralUtility::makeInstance(BackendWorkspaceRestriction::class));
- $queryBuilder = $this->setWhereClause($queryBuilder, $id);
- $count = $queryBuilder->count('uid')
+ $queryBuilder->count('uid')
->from('pages')
- ->execute()
+ ->where(
+ QueryHelper::stripLogicalOperatorPrefix($GLOBALS['BE_USER']->getPagePermsClause(1)),
+ // Only show records in default language
+ $queryBuilder->expr()->eq('sys_language_uid', $queryBuilder->createNamedParameter(0, \PDO::PARAM_INT))
+ )
+ ->orderBy('sorting');
+ if ((int)$id >= 0) {
+ $queryBuilder->andWhere(
+ $queryBuilder->expr()->eq('pid', $queryBuilder->createNamedParameter($id, \PDO::PARAM_INT))
+ );
+ }
+ $count = $queryBuilder->execute()
->fetchColumn(0);
return (bool)$count;
}
diff --git a/typo3/sysext/backend/Classes/Tree/Pagetree/ExtdirectTreeCommands.php b/typo3/sysext/backend/Classes/Tree/Pagetree/ExtdirectTreeCommands.php
index a611d8411902..b03c4fc0bd2a 100644
--- a/typo3/sysext/backend/Classes/Tree/Pagetree/ExtdirectTreeCommands.php
+++ b/typo3/sysext/backend/Classes/Tree/Pagetree/ExtdirectTreeCommands.php
@@ -86,7 +86,7 @@ class ExtdirectTreeCommands
Commands::deleteNode($node);
$returnValue = [];
if (static::getBackendUser()->workspace) {
- $record = Commands::getNodeRecord($node->getId());
+ $record = BackendUtility::getRecordWSOL('pages', $node->getId(), '*', '', true, true);
if ($record['_ORIG_uid']) {
$newNode = Commands::getNewNode($record);
$returnValue = $newNode->toArray();
diff --git a/typo3/sysext/backend/Classes/Utility/BackendUtility.php b/typo3/sysext/backend/Classes/Utility/BackendUtility.php
index 3b70932c4c05..161fe78e6811 100644
--- a/typo3/sysext/backend/Classes/Utility/BackendUtility.php
+++ b/typo3/sysext/backend/Classes/Utility/BackendUtility.php
@@ -353,14 +353,15 @@ class BackendUtility
* stops the process if we meet a page, the user has no reading access to.
* @param bool $workspaceOL If TRUE, version overlay is applied. This must be requested specifically because it is
* usually only wanted when the rootline is used for visual output while for permission checking you want the raw thing!
+ * @param string[] $additionalFields Additional Fields to select for rootline records
* @return array Root line array, all the way to the page tree root (or as far as $clause allows!)
*/
- public static function BEgetRootLine($uid, $clause = '', $workspaceOL = false)
+ public static function BEgetRootLine($uid, $clause = '', $workspaceOL = false, array $additionalFields = [])
{
static $BEgetRootLine_cache = [];
$output = [];
$pid = $uid;
- $ident = $pid . '-' . $clause . '-' . $workspaceOL;
+ $ident = $pid . '-' . $clause . '-' . $workspaceOL . ($additionalFields ? '-' . implode(',', $additionalFields) : '');
if (is_array($BEgetRootLine_cache[$ident])) {
$output = $BEgetRootLine_cache[$ident];
} else {
@@ -368,7 +369,7 @@ class BackendUtility
$theRowArray = [];
while ($uid != 0 && $loopCheck) {
$loopCheck--;
- $row = self::getPageForRootline($uid, $clause, $workspaceOL);
+ $row = self::getPageForRootline($uid, $clause, $workspaceOL, $additionalFields);
if (is_array($row)) {
$uid = $row['pid'];
$theRowArray[] = $row;
@@ -382,20 +383,22 @@ class BackendUtility
$c = count($theRowArray);
foreach ($theRowArray as $val) {
$c--;
- $output[$c] = [
- 'uid' => $val['uid'],
- 'pid' => $val['pid'],
- 'title' => $val['title'],
- 'doktype' => $val['doktype'],
- 'tsconfig_includes' => $val['tsconfig_includes'],
- 'TSconfig' => $val['TSconfig'],
- 'is_siteroot' => $val['is_siteroot'],
- 't3ver_oid' => $val['t3ver_oid'],
- 't3ver_wsid' => $val['t3ver_wsid'],
- 't3ver_state' => $val['t3ver_state'],
- 't3ver_stage' => $val['t3ver_stage'],
- 'backend_layout_next_level' => $val['backend_layout_next_level']
+ $fields = [
+ 'uid',
+ 'pid',
+ 'title',
+ 'doktype',
+ 'tsconfig_includes',
+ 'TSconfig',
+ 'is_siteroot',
+ 't3ver_oid',
+ 't3ver_wsid',
+ 't3ver_state',
+ 't3ver_stage',
+ 'backend_layout_next_level',
];
+ $fields = array_merge($fields, $additionalFields);
+ $output[$c] = array_intersect_key($val, array_combine($fields, $fields));
if (isset($val['_ORIG_pid'])) {
$output[$c]['_ORIG_pid'] = $val['_ORIG_pid'];
}
@@ -411,10 +414,11 @@ class BackendUtility
* @param int $uid Page id for which to create the root line.
* @param string $clause Clause can be used to select other criteria. It would typically be where-clauses that stops the process if we meet a page, the user has no reading access to.
* @param bool $workspaceOL If TRUE, version overlay is applied. This must be requested specifically because it is usually only wanted when the rootline is used for visual output while for permission checking you want the raw thing!
+ * @param string[] $additionalFields AdditionalFields to fetch from the root line
* @return array Cached page record for the rootline
* @see BEgetRootLine
*/
- protected static function getPageForRootline($uid, $clause, $workspaceOL)
+ protected static function getPageForRootline($uid, $clause, $workspaceOL, array $additionalFields = [])
{
static $getPageForRootline_cache = [];
$ident = $uid . '-' . $clause . '-' . $workspaceOL;
@@ -439,7 +443,8 @@ class BackendUtility
't3ver_wsid',
't3ver_state',
't3ver_stage',
- 'backend_layout_next_level'
+ 'backend_layout_next_level',
+ ...$additionalFields
)
->from('pages')
->where(
--
GitLab