From 1d5327bce1c0cf10dc7a4fa2beb545cc4a73a70a Mon Sep 17 00:00:00 2001
From: Andreas Fernandez <a.fernandez@scripting-base.de>
Date: Sun, 11 Mar 2018 22:09:06 +0100
Subject: [PATCH] [TASK] Improve overall recycler performance
To improve the overall performance of the recycler, these things are done:
- Improve how permissions are checked for each record
Instead of running multiple SQL requests per record, the check now
instantly stops if the user is either an admin, or has no permission
to modify a certain table.
- Drop sorting of records by page tree structure
The records get sorted by the page tree structure, to mime the tree in
a flat view. However, this feature is rather useless and also
considered buggy in a huge record set.
Resolves: #84711
Releases: master, 8.7
Change-Id: I0c5177546489ce2a0ba84435fed3879267a5a871
Reviewed-on: https://review.typo3.org/56102
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Mona Muzaffar <mona.muzaffar@gmx.de>
Tested-by: Mona Muzaffar <mona.muzaffar@gmx.de>
Reviewed-by: Jan Helke <typo3@helke.de>
Tested-by: Jan Helke <typo3@helke.de>
---
.../Controller/DeletedRecordsController.php | 7 +-
.../Controller/RecyclerAjaxController.php | 5 +-
.../Classes/Domain/Model/DeletedRecords.php | 23 ------
.../recycler/Classes/Domain/Model/Tables.php | 1 +
.../Classes/Utility/RecyclerUtility.php | 13 +++-
.../Partials/RecordsTable/DeletedRecord.html | 2 +-
.../DataSet/Assertion/deletedPage-3_4_5_7.xml | 17 +++--
.../Unit/Domain/Model/DeletedRecordsTest.php | 73 -------------------
8 files changed, 23 insertions(+), 118 deletions(-)
delete mode 100644 typo3/sysext/recycler/Tests/Unit/Domain/Model/DeletedRecordsTest.php
diff --git a/typo3/sysext/recycler/Classes/Controller/DeletedRecordsController.php b/typo3/sysext/recycler/Classes/Controller/DeletedRecordsController.php
index d6554d39b738..aacbcf3e9ecd 100644
--- a/typo3/sysext/recycler/Classes/Controller/DeletedRecordsController.php
+++ b/typo3/sysext/recycler/Classes/Controller/DeletedRecordsController.php
@@ -50,12 +50,10 @@ class DeletedRecordsController
* Transforms the rows for the deleted records
*
* @param array $deletedRowsArray Array with table as key and array with all deleted rows
- * @param int $totalDeleted Number of deleted records in total
* @return array JSON array
*/
- public function transform($deletedRowsArray, $totalDeleted)
+ public function transform($deletedRowsArray)
{
- $total = 0;
$jsonArray = [
'rows' => []
];
@@ -65,11 +63,9 @@ class DeletedRecordsController
$iconFactory = GeneralUtility::makeInstance(IconFactory::class);
foreach ($deletedRowsArray as $table => $rows) {
- $total += count($deletedRowsArray[$table]);
foreach ($rows as $row) {
$pageTitle = $this->getPageTitle((int)$row['pid']);
$backendUserName = $this->getBackendUser((int)$row[$GLOBALS['TCA'][$table]['ctrl']['cruser_id']]);
-
$userIdWhoDeleted = $this->getUserWhoDeleted($table, (int)$row['uid']);
$jsonArray['rows'][] = [
@@ -92,7 +88,6 @@ class DeletedRecordsController
}
}
}
- $jsonArray['total'] = $totalDeleted;
return $jsonArray;
}
diff --git a/typo3/sysext/recycler/Classes/Controller/RecyclerAjaxController.php b/typo3/sysext/recycler/Classes/Controller/RecyclerAjaxController.php
index 57eb2de11c21..690e87333723 100644
--- a/typo3/sysext/recycler/Classes/Controller/RecyclerAjaxController.php
+++ b/typo3/sysext/recycler/Classes/Controller/RecyclerAjaxController.php
@@ -97,7 +97,7 @@ class RecyclerAjaxController
/* @var $controller DeletedRecordsController */
$controller = GeneralUtility::makeInstance(DeletedRecordsController::class);
- $recordsArray = $controller->transform($deletedRowsArray, $totalDeleted);
+ $recordsArray = $controller->transform($deletedRowsArray);
$allowDelete = $this->getBackendUser()->isAdmin()
?: (bool)($this->getBackendUser()->getTSConfig()['mod.']['recycler.']['allowDelete'] ?? false);
@@ -105,10 +105,9 @@ class RecyclerAjaxController
$view->setTemplatePathAndFilename($extPath . 'Resources/Private/Templates/Ajax/RecordsTable.html');
$view->assign('records', $recordsArray['rows']);
$view->assign('allowDelete', $allowDelete);
- $view->assign('total', $recordsArray['total']);
$content = [
'rows' => $view->render(),
- 'totalItems' => $recordsArray['total']
+ 'totalItems' => $totalDeleted
];
break;
case 'undoRecords':
diff --git a/typo3/sysext/recycler/Classes/Domain/Model/DeletedRecords.php b/typo3/sysext/recycler/Classes/Domain/Model/DeletedRecords.php
index d543913c998a..dac6e7c21a46 100644
--- a/typo3/sysext/recycler/Classes/Domain/Model/DeletedRecords.php
+++ b/typo3/sysext/recycler/Classes/Domain/Model/DeletedRecords.php
@@ -243,8 +243,6 @@ class DeletedRecords
if ($recordsToCheck !== false) {
$this->checkRecordAccess($table, $recordsToCheck);
- $pidList = $this->getTreeList($id, $depth);
- $this->sortDeletedRowsByPidList($pidList);
}
}
$this->label[$table] = $tcaCtrl['label'];
@@ -341,27 +339,6 @@ class DeletedRecords
}
}
- /**
- * @param array $pidList
- */
- protected function sortDeletedRowsByPidList(array $pidList)
- {
- foreach ($this->deletedRows as $table => $rows) {
- // Reset array of deleted rows for current table
- $this->deletedRows[$table] = [];
-
- // Get rows for current pid
- foreach ($pidList as $pid) {
- $rowsForCurrentPid = array_filter($rows, function ($row) use ($pid) {
- return (int)$row['pid'] === (int)$pid;
- });
-
- // Append sorted records to the array again
- $this->deletedRows[$table] = array_merge($this->deletedRows[$table], $rowsForCurrentPid);
- }
- }
- }
-
/************************************************************
* DELETE FUNCTIONS
************************************************************/
diff --git a/typo3/sysext/recycler/Classes/Domain/Model/Tables.php b/typo3/sysext/recycler/Classes/Domain/Model/Tables.php
index e4a59f40d386..3c1cfffde65e 100644
--- a/typo3/sysext/recycler/Classes/Domain/Model/Tables.php
+++ b/typo3/sysext/recycler/Classes/Domain/Model/Tables.php
@@ -36,6 +36,7 @@ class Tables
$lang = $this->getLanguageService();
$tables = [];
$connection = GeneralUtility::makeInstance(ConnectionPool::class);
+
foreach (RecyclerUtility::getModifyableTables() as $tableName) {
$deletedField = RecyclerUtility::getDeletedField($tableName);
if ($deletedField) {
diff --git a/typo3/sysext/recycler/Classes/Utility/RecyclerUtility.php b/typo3/sysext/recycler/Classes/Utility/RecyclerUtility.php
index c5753056b687..1fe5231767f8 100644
--- a/typo3/sysext/recycler/Classes/Utility/RecyclerUtility.php
+++ b/typo3/sysext/recycler/Classes/Utility/RecyclerUtility.php
@@ -34,13 +34,21 @@ class RecyclerUtility
* as well as the table access rights of the user.
*
* @param string $table The table to check access for
- * @param string $row Record array
+ * @param array $row Record array
* @return bool Returns TRUE is the user has access, or FALSE if not
*/
public static function checkAccess($table, $row)
{
$backendUser = static::getBackendUser();
+ if ($backendUser->isAdmin()) {
+ return true;
+ }
+
+ if (!$backendUser->check('tables_modify', $table)) {
+ return false;
+ }
+
// Checking if the user has permissions? (Only working as a precaution, because the final permission check is always down in TCE. But it's good to notify the user on beforehand...)
// First, resetting flags.
$hasAccess = false;
@@ -60,9 +68,6 @@ class RecyclerUtility
$hasAccess = $backendUser->recordEditAccessInternals($table, $calcPRec);
}
}
- if (!$backendUser->check('tables_modify', $table)) {
- $hasAccess = false;
- }
return $hasAccess;
}
diff --git a/typo3/sysext/recycler/Resources/Private/Partials/RecordsTable/DeletedRecord.html b/typo3/sysext/recycler/Resources/Private/Partials/RecordsTable/DeletedRecord.html
index 2f84bfafa79f..7dc56e05b147 100644
--- a/typo3/sysext/recycler/Resources/Private/Partials/RecordsTable/DeletedRecord.html
+++ b/typo3/sysext/recycler/Resources/Private/Partials/RecordsTable/DeletedRecord.html
@@ -30,7 +30,7 @@
</td>
</tr>
<tr class="collapse" id="{record.table}_{record.uid}">
- <td colspan="6">
+ <td colspan="7">
<table class="table">
<thead>
<tr>
diff --git a/typo3/sysext/recycler/Tests/Functional/Recycle/Pages/DataSet/Assertion/deletedPage-3_4_5_7.xml b/typo3/sysext/recycler/Tests/Functional/Recycle/Pages/DataSet/Assertion/deletedPage-3_4_5_7.xml
index 4de2198a5761..4b20657bbbfa 100644
--- a/typo3/sysext/recycler/Tests/Functional/Recycle/Pages/DataSet/Assertion/deletedPage-3_4_5_7.xml
+++ b/typo3/sysext/recycler/Tests/Functional/Recycle/Pages/DataSet/Assertion/deletedPage-3_4_5_7.xml
@@ -16,14 +16,6 @@
<deleted>1</deleted>
<perms_everybody>15</perms_everybody>
</pages>
- <pages>
- <uid>7</uid>
- <pid>1</pid>
- <title>Dummy 1-7 (deleted)</title>
- <doktype>1</doktype>
- <deleted>1</deleted>
- <perms_everybody>0</perms_everybody>
- </pages>
<pages>
<uid>5</uid>
<pid>4</pid>
@@ -32,5 +24,14 @@
<deleted>1</deleted>
<perms_everybody>15</perms_everybody>
</pages>
+
+ <pages>
+ <uid>7</uid>
+ <pid>1</pid>
+ <title>Dummy 1-7 (deleted)</title>
+ <doktype>1</doktype>
+ <deleted>1</deleted>
+ <perms_everybody>0</perms_everybody>
+ </pages>
</dataset>
diff --git a/typo3/sysext/recycler/Tests/Unit/Domain/Model/DeletedRecordsTest.php b/typo3/sysext/recycler/Tests/Unit/Domain/Model/DeletedRecordsTest.php
deleted file mode 100644
index d5ce5fd5ce22..000000000000
--- a/typo3/sysext/recycler/Tests/Unit/Domain/Model/DeletedRecordsTest.php
+++ /dev/null
@@ -1,73 +0,0 @@
-<?php
-declare(strict_types = 1);
-namespace TYPO3\CMS\Recycler\Tests\Unit\Domain\Model;
-
-/*
- * This file is part of the TYPO3 CMS project.
- *
- * It is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License, either version 2
- * of the License, or any later version.
- *
- * For the full copyright and license information, please read the
- * LICENSE.txt file that was distributed with this source code.
- *
- * The TYPO3 project - inspiring people to share!
- */
-
-use TYPO3\CMS\Recycler\Domain\Model\DeletedRecords;
-use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
-
-/**
- * Test case
- */
-class DeletedRecordsTest extends UnitTestCase
-{
- /**
- * @test
- */
- public function recordsOfMultipleTablesAreSortedByPid()
- {
- $deletedRowsData = [
- 'pages' => [
- ['uid' => 1, 'pid' => 1],
- ['uid' => 2, 'pid' => 2],
- ['uid' => 3, 'pid' => 4],
- ['uid' => 4, 'pid' => 7],
- ],
- 'sys_template' => [
- ['uid' => 1, 'pid' => 9],
- ['uid' => 2, 'pid' => 10],
- ['uid' => 3, 'pid' => 1],
- ],
- 'tt_content' => [
- ['uid' => 1, 'pid' => 7],
- ['uid' => 2, 'pid' => 1],
- ]
- ];
-
- $expectedRows = [
- 'pages' => [
- ['uid' => 1, 'pid' => 1],
- ['uid' => 2, 'pid' => 2],
- ['uid' => 4, 'pid' => 7],
- ['uid' => 3, 'pid' => 4],
- ],
- 'sys_template' => [
- ['uid' => 3, 'pid' => 1],
- ['uid' => 2, 'pid' => 10],
- ['uid' => 1, 'pid' => 9],
- ],
- 'tt_content' => [
- ['uid' => 2, 'pid' => 1],
- ['uid' => 1, 'pid' => 7],
- ]
- ];
-
- /** @var \PHPUnit_Framework_MockObject_MockObject|\TYPO3\TestingFramework\Core\AccessibleObjectInterface|DeletedRecords $subject */
- $subject = $this->getAccessibleMock(DeletedRecords::class, ['dummy']);
- $subject->_set('deletedRows', $deletedRowsData);
- $subject->_call('sortDeletedRowsByPidList', [1, 2, 7, 4, 10, 9]);
- static::assertEquals($expectedRows, $subject->getDeletedRows());
- }
-}
--
GitLab