From 2b8123d07197a61cac0185899e0a0f144f66d0ec Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Sat, 5 Nov 2022 22:11:59 +0100
Subject: [PATCH] [BUGFIX] Speed up FormEngine with selective workspaceOL()
calls
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
BackendUtility::getPossibleWorkspaceVersionIdsOfLiveRecordIds()
is added to check for workspace versions within FormEngine
foreign_field logic to first fetch a list of all records
that have a versioned record within the workspace, and only
apply BackendUtility::workspaceOL() for these cases.
Without the patch, each live record triggers a database
query to resolve a possible workspace overlay. With the
patch, one query is done to see which records have
workspace overlays, only those are queried for overlays.
This reduces the number of performed queries a lot, since
typically only a small number of records are overlaid.
This only affects editing records in a workspace.
Resolves: #99007
Releases: main, 11.5
Change-Id: Ia7c47e5de0082ca0ff2c68b30d9ff1bc5312981b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/76446
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Benni Mack <benni@typo3.org>
---
.../FormDataProvider/AbstractItemProvider.php | 13 +-
.../Classes/Utility/BackendUtility.php | 37 +++++
.../FormDataProvider/TcaSelectItemsTest.php | 143 ++++++++++--------
.../TcaSelectTreeItemsTest.php | 2 +-
4 files changed, 126 insertions(+), 69 deletions(-)
diff --git a/typo3/sysext/backend/Classes/Form/FormDataProvider/AbstractItemProvider.php b/typo3/sysext/backend/Classes/Form/FormDataProvider/AbstractItemProvider.php
index a61da37d3ff0..2151e24f8d18 100644
--- a/typo3/sysext/backend/Classes/Form/FormDataProvider/AbstractItemProvider.php
+++ b/typo3/sysext/backend/Classes/Form/FormDataProvider/AbstractItemProvider.php
@@ -294,8 +294,17 @@ abstract class AbstractItemProvider
$fileRepository = GeneralUtility::makeInstance(FileRepository::class);
$iconFactory = GeneralUtility::makeInstance(IconFactory::class);
- while ($foreignRow = $queryResult->fetchAssociative()) {
- BackendUtility::workspaceOL($foreignTable, $foreignRow);
+ $allForeignRows = $queryResult->fetchAllAssociative();
+ // Find all possible versioned records of the current IDs, so we do not need to overlay each record
+ // This way, workspaceOL() does not need to be called for each record.
+ $workspaceId = $this->getBackendUser()->workspace;
+ $doOverlaysForRecords = BackendUtility::getPossibleWorkspaceVersionIdsOfLiveRecordIds($foreignTable, array_column($allForeignRows, 'uid'), $workspaceId);
+
+ foreach ($allForeignRows as $foreignRow) {
+ // Only do workspace overlays when a versioned record exists.
+ if (isset($foreignRow['uid']) && isset($doOverlaysForRecords[(int)$foreignRow['uid']])) {
+ BackendUtility::workspaceOL($foreignTable, $foreignRow, $workspaceId);
+ }
// Only proceed in case the row was not unset and we don't deal with a delete placeholder
if (is_array($foreignRow)
&& !VersionState::cast($foreignRow['t3ver_state'] ?? 0)->equals(VersionState::DELETE_PLACEHOLDER)
diff --git a/typo3/sysext/backend/Classes/Utility/BackendUtility.php b/typo3/sysext/backend/Classes/Utility/BackendUtility.php
index 07a5fefd7c26..4fd641d6bf88 100644
--- a/typo3/sysext/backend/Classes/Utility/BackendUtility.php
+++ b/typo3/sysext/backend/Classes/Utility/BackendUtility.php
@@ -33,6 +33,7 @@ use TYPO3\CMS\Core\Context\DateTimeAspect;
use TYPO3\CMS\Core\Core\Environment;
use TYPO3\CMS\Core\Database\Connection;
use TYPO3\CMS\Core\Database\ConnectionPool;
+use TYPO3\CMS\Core\Database\Platform\PlatformInformation;
use TYPO3\CMS\Core\Database\Query\QueryBuilder;
use TYPO3\CMS\Core\Database\Query\QueryHelper;
use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
@@ -3099,6 +3100,42 @@ class BackendUtility
return false;
}
+ /**
+ * Use this function if you have a large set of IDs to find out which ones have a counterpart within a workspace.
+ * Within a workspace, this is one additional query, so use it only if you have a set of > 2 to find out if you
+ * really need to call BackendUtility::workspaceOL() all the time.
+ *
+ * If you have 1000 records, but only have two 2 records which have been modified in a workspace, only 2 items
+ * are returned.
+ *
+ * @return array<int, int> keys contain the live record ID, values the versioned record ID
+ * @internal this method is not public API and might change, as you really should know what you are doing.
+ */
+ public static function getPossibleWorkspaceVersionIdsOfLiveRecordIds(string $table, array $liveRecordIds, int $workspaceId): array
+ {
+ if ($liveRecordIds === [] || $workspaceId === 0 || !self::isTableWorkspaceEnabled($table)) {
+ return [];
+ }
+ $doOverlaysForRecords = [];
+ $connection = self::getConnectionForTable($table);
+ $maxChunk = PlatformInformation::getMaxBindParameters($connection->getDatabasePlatform());
+ foreach (array_chunk($liveRecordIds, (int)floor($maxChunk / 2)) as $liveRecordIdChunk) {
+ $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($table);
+ $doOverlaysForRecordsStatement = $queryBuilder
+ ->select('t3ver_oid', 'uid')
+ ->from($table)
+ ->where(
+ $queryBuilder->expr()->eq('t3ver_wsid', $queryBuilder->createNamedParameter($workspaceId, Connection::PARAM_INT)),
+ $queryBuilder->expr()->in('t3ver_oid', $queryBuilder->quoteArrayBasedValueListToIntegerList($liveRecordIdChunk))
+ )
+ ->executeQuery();
+ while ($recordWithVersionedRecord = $doOverlaysForRecordsStatement->fetchNumeric()) {
+ $doOverlaysForRecords[(int)$recordWithVersionedRecord[0]] = (int)$recordWithVersionedRecord[1];
+ }
+ }
+ return $doOverlaysForRecords;
+ }
+
/**
* Returns live version of record
*
diff --git a/typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/TcaSelectItemsTest.php b/typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/TcaSelectItemsTest.php
index a141d51795e3..1b63b3adb8ff 100644
--- a/typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/TcaSelectItemsTest.php
+++ b/typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/TcaSelectItemsTest.php
@@ -126,7 +126,7 @@ class TcaSelectItemsTest extends UnitTestCase
[$queryBuilderProphet, $connectionPoolProphet] = $this->mockDatabaseConnection('foreignTable');
$statementProphet = $this->prophesize(Result::class);
- $statementProphet->fetchAssociative()->shouldBeCalled();
+ $statementProphet->fetchAllAssociative()->shouldBeCalled();
$queryBuilderProphet->select('foreignTable.uid')
->shouldBeCalled()
@@ -1002,6 +1002,7 @@ class TcaSelectItemsTest extends UnitTestCase
[$queryBuilderProphet, $connectionPoolProphet] = $this->mockDatabaseConnection();
$statementProphet = $this->prophesize(Result::class);
+ $statementProphet->fetchAllAssociative()->shouldBeCalled()->willReturn([]);
$queryBuilderProphet->select('fTable.uid')->shouldBeCalled()->willReturn($queryBuilderProphet->reveal());
$queryBuilderProphet->from('fTable')->shouldBeCalled()->willReturn($queryBuilderProphet->reveal());
@@ -1096,6 +1097,7 @@ class TcaSelectItemsTest extends UnitTestCase
[$queryBuilderProphet, $connectionPoolProphet] = $this->mockDatabaseConnection();
$statementProphet = $this->prophesize(Result::class);
+ $statementProphet->fetchAllAssociative()->shouldBeCalled()->willReturn([]);
$queryBuilderProphet->select('fTable.uid')->shouldBeCalled()->willReturn($queryBuilderProphet->reveal());
$queryBuilderProphet->from('fTable')->shouldBeCalled()->willReturn($queryBuilderProphet->reveal());
@@ -1264,19 +1266,20 @@ class TcaSelectItemsTest extends UnitTestCase
GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
- $counter = 0;
- $statementProphet->fetchAssociative()->shouldBeCalled()->will(static function ($args) use (&$counter) {
- $counter++;
- if ($counter >= 3) {
- return false;
- }
- return [
- 'uid' => $counter,
+ $statementProphet->fetchAllAssociative()->shouldBeCalled()->willReturn([
+ [
+ 'uid' => 1,
'pid' => 23,
'labelField' => 'aLabel',
'aValue' => 'bar,',
- ];
- });
+ ],
+ [
+ 'uid' => 2,
+ 'pid' => 23,
+ 'labelField' => 'aLabel',
+ 'aValue' => 'bar,',
+ ],
+ ]);
$expected = $input;
$expected['processedTca']['columns']['aField']['config']['items'] = [
@@ -1372,18 +1375,23 @@ class TcaSelectItemsTest extends UnitTestCase
GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
- $counter = 0;
- $statementProphet->fetchAssociative()->shouldBeCalled()->will(static function ($args) use (&$counter) {
- $counter++;
- if ($counter >= 3) {
- return false;
- }
- return [
- 'uid' => $counter,
+ $statementProphet->fetchAllAssociative()->shouldBeCalled()->willReturn([
+ [
+ 'uid' => 1,
'pid' => 0,
'labelField' => 'storageFolderLabel',
- ];
- });
+ ],
+ [
+ 'uid' => 2,
+ 'pid' => 0,
+ 'labelField' => 'storageFolderLabel',
+ ],
+ [
+ 'uid' => 3,
+ 'pid' => 0,
+ 'labelField' => 'storageFolderLabel',
+ ],
+ ]);
$expected = $input;
$expected['processedTca']['columns']['aField']['config']['items'] = [
@@ -1469,7 +1477,7 @@ class TcaSelectItemsTest extends UnitTestCase
'pid' => 23,
'icon' => 'foo.jpg',
];
- $statementProphet->fetchAssociative()->shouldBeCalled()->willReturn($foreignTableRowResultOne, false);
+ $statementProphet->fetchAllAssociative()->shouldBeCalled()->willReturn([$foreignTableRowResultOne]);
$expected = $input;
$expected['processedTca']['columns']['aField']['config']['items'] = [
@@ -2111,20 +2119,22 @@ class TcaSelectItemsTest extends UnitTestCase
GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
- $counter = 0;
- $statementProphet->fetchAssociative()->shouldBeCalled()->will(static function ($args) use (&$counter) {
- $counter++;
- if ($counter >= 3) {
- return false;
- }
- return [
- 'uid' => $counter,
+ $statementProphet->fetchAllAssociative()->willReturn([
+ [
+ 'uid' => 1,
'pid' => 23,
- 'labelField' => 'aLabel_' . $counter,
+ 'labelField' => 'aLabel_1',
'aValue' => 'bar,',
'dbfield' => 'some data',
- ];
- });
+ ],
+ [
+ 'uid' => 2,
+ 'pid' => 23,
+ 'labelField' => 'aLabel_2',
+ 'aValue' => 'bar,',
+ 'dbfield' => 'some data',
+ ],
+ ]);
$expected = $input;
$expected['processedTca']['columns']['aField']['config']['items'] = [
@@ -2231,22 +2241,23 @@ class TcaSelectItemsTest extends UnitTestCase
GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
- $counter = 0;
-
// simulates foreign_table containing two rows
- $statementProphet->fetchAssociative()->shouldBeCalled()->will(static function ($args) use (&$counter) {
- $counter++;
- if ($counter >= 3) {
- return false;
- }
- return [
- 'uid' => $counter,
+ $statementProphet->fetchAllAssociative()->shouldBeCalled()->willReturn([
+ [
+ 'uid' => 1,
'pid' => 23,
'labelField' => 'aLabel will be replaced since the label column is not configured',
'aValue' => 'bar, irrelevant',
'dbfield' => 'some random data, irrelevant',
- ];
- });
+ ],
+ [
+ 'uid' => 2,
+ 'pid' => 23,
+ 'labelField' => 'aLabel will be replaced since the label column is not configured',
+ 'aValue' => 'bar, irrelevant',
+ 'dbfield' => 'some random data, irrelevant',
+ ],
+ ]);
$expected = $input;
$expected['processedTca']['columns']['aField']['config'] = [
@@ -2360,21 +2371,21 @@ class TcaSelectItemsTest extends UnitTestCase
GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
- $counter = 0;
-
// simulates foreign_table containing two rows
- $statementProphet->fetchAssociative()->shouldBeCalled()->will(static function ($args) use (&$counter) {
- $counter++;
- if ($counter >= 3) {
- return false;
- }
- return [
- 'uid' => $counter,
+ $statementProphet->fetchAllAssociative()->shouldBeCalled()->willReturn([
+ [
+ 'uid' => 1,
'pid' => 23,
'labelField' => 'aLabel will be replaced since the label column is not configured',
'randomDbField' => 'bar, irrelevant',
- ];
- });
+ ],
+ [
+ 'uid' => 2,
+ 'pid' => 23,
+ 'labelField' => 'aLabel will be replaced since the label column is not configured',
+ 'randomDbField' => 'bar, irrelevant',
+ ],
+ ]);
$expected = $input;
$expected['processedTca']['columns']['aField']['config'] = [
@@ -2490,21 +2501,21 @@ class TcaSelectItemsTest extends UnitTestCase
GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
- $counter = 0;
-
// simulates foreign_table containing two rows
- $statementProphet->fetchAssociative()->shouldBeCalled()->will(static function ($args) use (&$counter) {
- $counter++;
- if ($counter >= 3) {
- return false;
- }
- return [
- 'uid' => $counter,
+ $statementProphet->fetchAllAssociative()->shouldBeCalled()->willReturn([
+ [
+ 'uid' => 1,
'pid' => 23,
'labelField' => 'aLabel will be replaced since the label column is not configured',
'randomDbField' => 'bar, irrelevant',
- ];
- });
+ ],
+ [
+ 'uid' => 2,
+ 'pid' => 23,
+ 'labelField' => 'aLabel will be replaced since the label column is not configured',
+ 'randomDbField' => 'bar, irrelevant',
+ ],
+ ]);
$expected = $input;
$expected['processedTca']['columns']['aField']['config'] = [
diff --git a/typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/TcaSelectTreeItemsTest.php b/typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/TcaSelectTreeItemsTest.php
index 3850b77de2c8..82cacca8567c 100644
--- a/typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/TcaSelectTreeItemsTest.php
+++ b/typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/TcaSelectTreeItemsTest.php
@@ -64,7 +64,7 @@ class TcaSelectTreeItemsTest extends UnitTestCase
});
$statementProphet = $this->prophesize(Result::class);
- $statementProphet->fetchAssociative()->shouldBeCalled();
+ $statementProphet->fetchAllAssociative()->shouldBeCalled();
$restrictionProphet = $this->prophesize(DefaultRestrictionContainer::class);
$restrictionProphet->removeAll()->willReturn($restrictionProphet->reveal());
--
GitLab