Commit a22be10b authored by Benni Mack's avatar Benni Mack Committed by Christian Kuhn
Browse files

[BUGFIX] Improve loading of related records in Backend UI

This change uses RelationHandler for foreign_table / allowed and MM
in BackendUtility::getProcessedValue in a consistent way.

In addition, the RelationHandler now fetches the full
record by default and adds this to the processed array, making
fewer queries to the database by fetching each record
fully directly.

Some bugs are fixed with that:
* FAL images (as in all inline fields) in list module now have the correct order
* Fewer DB queries when fetching relations in Group field in FormEngine

Setting the option in RelationHandler to fetch all fields by default
avoids unneeded calls in general when fetching the records.

Resolves: #94651
Releases: master
Change-Id: I8261a0c2657b9b71ae7cdc686ee43eaa7461c8b6
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/70117

Tested-by: core-ci's avatarcore-ci <typo3@b13.com>
Tested-by: Oliver Bartsch's avatarOliver Bartsch <bo@cedev.de>
Tested-by: Christian Kuhn's avatarChristian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Oliver Bartsch's avatarOliver Bartsch <bo@cedev.de>
Reviewed-by: Christian Kuhn's avatarChristian Kuhn <lolli@schwarzbu.ch>
parent 2d04af91
......@@ -90,8 +90,8 @@ class TcaGroup implements FormDataProviderInterface
$relations = $relationHandler->getResolvedItemArray();
foreach ($relations as $relation) {
$tableName = $relation['table'];
$uid = $relation['uid'];
$record = BackendUtility::getRecordWSOL($tableName, $uid);
$record = $relation['record'];
BackendUtility::workspaceOL($tableName, $record);
$title = BackendUtility::getRecordTitle($tableName, $record, false, false);
$items[] = [
'table' => $tableName,
......
......@@ -1569,77 +1569,18 @@ class BackendUtility
$l = $lang->sL($l);
break;
case 'inline':
if ($uid) {
$finalValues = static::resolveRelationLabels($theColConf, $table, $uid, $value, $noRecordLookup);
$l = implode(', ', $finalValues);
} else {
$l = $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_common.xlf:notAvailableAbbreviation');
}
break;
case 'select':
if (!empty($theColConf['MM'])) {
if ($uid) {
// Display the title of MM related records in lists
if ($noRecordLookup) {
$MMfields = [];
$MMfields[] = $theColConf['foreign_table'] . '.uid';
} else {
$MMfields = [$theColConf['foreign_table'] . '.' . $GLOBALS['TCA'][$theColConf['foreign_table']]['ctrl']['label']];
if (isset($GLOBALS['TCA'][$theColConf['foreign_table']]['ctrl']['label_alt'])) {
foreach (GeneralUtility::trimExplode(
',',
$GLOBALS['TCA'][$theColConf['foreign_table']]['ctrl']['label_alt'],
true
) as $f) {
$MMfields[] = $theColConf['foreign_table'] . '.' . $f;
}
}
}
/** @var RelationHandler $dbGroup */
$dbGroup = GeneralUtility::makeInstance(RelationHandler::class);
$dbGroup->start(
$value,
$theColConf['foreign_table'],
$theColConf['MM'],
$uid,
$table,
$theColConf
);
$selectUids = $dbGroup->tableArray[$theColConf['foreign_table']];
if (is_array($selectUids) && !empty($selectUids)) {
$queryBuilder = static::getQueryBuilderForTable($theColConf['foreign_table']);
$queryBuilder->getRestrictions()
->removeAll()
->add(GeneralUtility::makeInstance(DeletedRestriction::class));
$result = $queryBuilder
->select('uid', ...$MMfields)
->from($theColConf['foreign_table'])
->where(
$queryBuilder->expr()->in(
'uid',
$queryBuilder->createNamedParameter($selectUids, Connection::PARAM_INT_ARRAY)
)
)
->execute();
$mmlA = [];
while ($MMrow = $result->fetchAssociative()) {
// Keep sorting of $selectUids
$selectedUid = array_search($MMrow['uid'], $selectUids);
$mmlA[$selectedUid] = $MMrow['uid'];
if (!$noRecordLookup) {
$mmlA[$selectedUid] = static::getRecordTitle(
$theColConf['foreign_table'],
$MMrow,
false,
$forceResult
);
}
}
if (!empty($mmlA)) {
ksort($mmlA);
$l = implode('; ', $mmlA);
} else {
$l = $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_common.xlf:notAvailableAbbreviation');
}
} else {
$l = $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_common.xlf:notAvailableAbbreviation');
}
$finalValues = static::resolveRelationLabels($theColConf, $table, $uid, $value, $noRecordLookup);
$l = implode(', ', $finalValues);
} else {
$l = $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_common.xlf:notAvailableAbbreviation');
}
......@@ -1656,65 +1597,11 @@ class BackendUtility
if ($noRecordLookup) {
$l = $value;
} else {
$rParts = [];
if ($uid && isset($theColConf['foreign_field']) && $theColConf['foreign_field'] !== '') {
$queryBuilder = static::getQueryBuilderForTable($theColConf['foreign_table']);
$queryBuilder->getRestrictions()
->removeAll()
->add(GeneralUtility::makeInstance(DeletedRestriction::class))
->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, static::getBackendUserAuthentication()->workspace));
$constraints = [
$queryBuilder->expr()->eq(
$theColConf['foreign_field'],
$queryBuilder->createNamedParameter($uid, \PDO::PARAM_INT)
)
];
if (!empty($theColConf['foreign_table_field'])) {
$constraints[] = $queryBuilder->expr()->eq(
$theColConf['foreign_table_field'],
$queryBuilder->createNamedParameter($table, \PDO::PARAM_STR)
);
}
// Add additional where clause if foreign_match_fields are defined
$foreignMatchFields = [];
if (is_array($theColConf['foreign_match_fields'] ?? false)) {
$foreignMatchFields = $theColConf['foreign_match_fields'];
}
foreach ($foreignMatchFields as $matchField => $matchValue) {
$constraints[] = $queryBuilder->expr()->eq(
$matchField,
$queryBuilder->createNamedParameter($matchValue)
);
}
$result = $queryBuilder
->select('*')
->from($theColConf['foreign_table'])
->where(...$constraints)
->execute();
while ($record = $result->fetchAssociative()) {
$rParts[] = $record['uid'];
}
}
if (empty($rParts)) {
$rParts = GeneralUtility::trimExplode(',', $value, true);
}
$lA = [];
foreach ($rParts as $rVal) {
$rVal = (int)$rVal;
$r = self::getRecordWSOL($theColConf['foreign_table'], $rVal);
if (is_array($r)) {
$lA[] = $lang->sL($theColConf['foreign_table_prefix'] ?? '')
. self::getRecordTitle($theColConf['foreign_table'], $r, false, $forceResult);
} else {
$lA[] = $rVal ? '[' . $rVal . '!]' : '';
}
$finalValues = [];
if ($uid) {
$finalValues = static::resolveRelationLabels($theColConf, $table, $uid, $value, $noRecordLookup);
}
$l = implode(', ', $lA);
$l = implode(', ', $finalValues);
}
}
if (empty($l) && !empty($value)) {
......@@ -1726,99 +1613,11 @@ class BackendUtility
case 'group':
// resolve the titles for DB records
if (isset($theColConf['internal_type']) && $theColConf['internal_type'] === 'db') {
if (isset($theColConf['MM']) && $theColConf['MM']) {
if ($uid) {
// Display the title of MM related records in lists
if ($noRecordLookup) {
$MMfields = [];
$MMfields[] = $theColConf['foreign_table'] . '.uid';
} else {
$MMfields = [$theColConf['foreign_table'] . '.' . $GLOBALS['TCA'][$theColConf['foreign_table']]['ctrl']['label']];
$altLabelFields = explode(
',',
$GLOBALS['TCA'][$theColConf['foreign_table']]['ctrl']['label_alt']
);
foreach ($altLabelFields as $f) {
$f = trim($f);
if ($f !== '') {
$MMfields[] = $theColConf['foreign_table'] . '.' . $f;
}
}
}
/** @var RelationHandler $dbGroup */
$dbGroup = GeneralUtility::makeInstance(RelationHandler::class);
$dbGroup->start(
$value,
$theColConf['foreign_table'],
$theColConf['MM'],
$uid,
$table,
$theColConf
);
$selectUids = $dbGroup->tableArray[$theColConf['foreign_table']];
if (!empty($selectUids) && is_array($selectUids)) {
$queryBuilder = static::getQueryBuilderForTable($theColConf['foreign_table']);
$queryBuilder->getRestrictions()
->removeAll()
->add(GeneralUtility::makeInstance(DeletedRestriction::class));
$result = $queryBuilder
->select('uid', ...$MMfields)
->from($theColConf['foreign_table'])
->where(
$queryBuilder->expr()->in(
'uid',
$queryBuilder->createNamedParameter(
$selectUids,
Connection::PARAM_INT_ARRAY
)
)
)
->execute();
$mmlA = [];
while ($MMrow = $result->fetchAssociative()) {
// Keep sorting of $selectUids
$selectedUid = array_search($MMrow['uid'], $selectUids);
$mmlA[$selectedUid] = $MMrow['uid'];
if (!$noRecordLookup) {
$mmlA[$selectedUid] = static::getRecordTitle(
$theColConf['foreign_table'],
$MMrow,
false,
$forceResult
);
}
}
if (!empty($mmlA)) {
ksort($mmlA);
$l = implode('; ', $mmlA);
} else {
$l = $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_common.xlf:notAvailableAbbreviation');
}
} else {
$l = $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_common.xlf:notAvailableAbbreviation');
}
} else {
$l = $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_common.xlf:notAvailableAbbreviation');
}
} else {
$finalValues = [];
$relationTableName = $theColConf['allowed'];
$explodedValues = GeneralUtility::trimExplode(',', $value, true);
foreach ($explodedValues as $explodedValue) {
if (MathUtility::canBeInterpretedAsInteger($explodedValue)) {
$relationTableNameForField = $relationTableName;
} else {
[$relationTableNameForField, $explodedValue] = self::splitTable_Uid($explodedValue);
}
$relationRecord = static::getRecordWSOL($relationTableNameForField, $explodedValue);
$finalValues[] = static::getRecordTitle($relationTableNameForField, $relationRecord);
}
$finalValues = static::resolveRelationLabels($theColConf, $table, $uid, $value, $noRecordLookup);
if ($finalValues !== []) {
$l = implode(', ', $finalValues);
} else {
$l = $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_common.xlf:notAvailableAbbreviation');
}
} else {
$l = implode(', ', GeneralUtility::trimExplode(',', $value, true));
......@@ -1962,6 +1761,53 @@ class BackendUtility
return $l;
}
/**
* Helper method to fetch all labels for all relations of processed Values.
*
* @param array $theColConf
* @param string $table
* @param string|int|null $recordId
* @param string|int $value
* @param bool $noRecordLookup
* @return array
*/
protected static function resolveRelationLabels(array $theColConf, string $table, $recordId, $value, bool $noRecordLookup): array
{
$finalValues = [];
$relationHandler = GeneralUtility::makeInstance(RelationHandler::class);
$relationHandler->registerNonTableValues = (bool)($theColConf['allowNonIdValues'] ?? false);
$relationHandler->start(
$value,
$theColConf['allowed'] ?? $theColConf['foreign_table'] ?? '',
$theColConf['MM'] ?? '',
$recordId,
$table,
$theColConf
);
if ($noRecordLookup) {
$finalValues = array_column($relationHandler->itemArray, 'id');
} else {
$relationHandler->getFromDB();
foreach ($relationHandler->getResolvedItemArray() as $item) {
$relationRecord = $item['record'];
static::workspaceOL($item['table'], $relationRecord);
if (!is_array($relationRecord)) {
$finalValues[] = '[' . $item['uid'] . ']';
} else {
$title = static::getRecordTitle($item['table'], $relationRecord);
if ($theColConf['foreign_table_prefix'] ?? null) {
$title = static::getLanguageService()->sL($theColConf['foreign_table_prefix']) . $title;
}
$finalValues[] = $title;
}
}
}
return $finalValues;
}
/**
* Same as ->getProcessedValue() but will go easy on fields like "tstamp" and "pid" which are not configured in TCA - they will be formatted by this function instead.
*
......
......@@ -167,10 +167,16 @@ class TcaGroupTest extends UnitTestCase
[
'table' => 'aForeignTable',
'uid' => 1,
'record' => [
'uid' => 1
]
],
[
'table' => 'aForeignTable',
'uid' => 2,
'record' => [
'uid' => 2
]
],
]);
......@@ -178,15 +184,19 @@ class TcaGroupTest extends UnitTestCase
$expected['databaseRow']['aField'] = [
[
'table' => 'aForeignTable',
'uid' => null,
'uid' => 1,
'title' => '',
'row' => null,
'row' => [
'uid' => 1
],
],
[
'table' => 'aForeignTable',
'uid' => null,
'uid' => 2,
'title' => '',
'row' => null,
'row' => [
'uid' => 2
],
]
];
$expected['processedTca']['columns']['aField']['config']['clipboardElements'] = [];
......
......@@ -15,26 +15,17 @@
namespace TYPO3\CMS\Backend\Tests\Unit\Utility;
use Doctrine\DBAL\Statement;
use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy;
use Psr\EventDispatcher\EventDispatcherInterface;
use TYPO3\CMS\Backend\Configuration\TypoScript\ConditionMatching\ConditionMatcher;
use TYPO3\CMS\Backend\Tests\Unit\Utility\Fixtures\LabelFromItemListMergedReturnsCorrectFieldsFixture;
use TYPO3\CMS\Backend\Tests\Unit\Utility\Fixtures\ProcessedValueForGroupWithMultipleAllowedTablesFixture;
use TYPO3\CMS\Backend\Tests\Unit\Utility\Fixtures\ProcessedValueForGroupWithOneAllowedTableFixture;
use TYPO3\CMS\Backend\Tests\Unit\Utility\Fixtures\ProcessedValueForSelectWithMMRelationFixture;
use TYPO3\CMS\Backend\Utility\BackendUtility;
use TYPO3\CMS\Core\Cache\CacheManager;
use TYPO3\CMS\Core\Cache\Frontend\FrontendInterface;
use TYPO3\CMS\Core\Configuration\Event\ModifyLoadedPageTsConfigEvent;
use TYPO3\CMS\Core\Configuration\Loader\PageTsConfigLoader;
use TYPO3\CMS\Core\Configuration\Parser\PageTsConfigParser;
use TYPO3\CMS\Core\Database\Connection;
use TYPO3\CMS\Core\Database\ConnectionPool;
use TYPO3\CMS\Core\Database\Query\Expression\ExpressionBuilder;
use TYPO3\CMS\Core\Database\Query\QueryBuilder;
use TYPO3\CMS\Core\Database\Query\Restriction\DefaultRestrictionContainer;
use TYPO3\CMS\Core\Database\RelationHandler;
use TYPO3\CMS\Core\Localization\LanguageService;
use TYPO3\CMS\Core\Site\Entity\Site;
......@@ -210,9 +201,51 @@ class BackendUtilityTest extends UnitTestCase
],
],
],
'pages' => [
'ctrl' => [
'label' => 'title'
],
'columns' => [
'title' => [
'config' => [
'type' => 'input'
]
]
]
]
];
$GLOBALS['LANG'] = [];
self::assertSame('Page 1, Page 2', ProcessedValueForGroupWithOneAllowedTableFixture::getProcessedValue('tt_content', 'pages', '1,2'));
$languageServiceProphecy = $this->prophesize(LanguageService::class);
$languageServiceProphecy->sL(Argument::cetera())->willReturnArgument(0);
$GLOBALS['LANG'] = $languageServiceProphecy->reveal();
/** @var RelationHandler|ObjectProphecy $relationHandlerProphet */
$relationHandlerProphet = $this->prophesize(RelationHandler::class);
$relationHandlerProphet->start(Argument::cetera())->shouldBeCalled();
$relationHandlerProphet->getFromDB()->willReturn([]);
$relationHandlerProphet->getResolvedItemArray()->willReturn([
[
'table' => 'pages',
'uid' => 1,
'record' => [
'uid' => 1,
'pid' => 0,
'title' => 'Page 1'
]
],
[
'table' => 'pages',
'uid' => 2,
'record' => [
'uid' => 2,
'pid' => 0,
'title' => 'Page 2'
]
]
]);
GeneralUtility::addInstance(RelationHandler::class, $relationHandlerProphet->reveal());
self::assertSame('Page 1, Page 2', BackendUtility::getProcessedValue('tt_content', 'pages', '1,2'));
}
/**
......@@ -222,7 +255,15 @@ class BackendUtilityTest extends UnitTestCase
{
$GLOBALS['TCA'] = [
'index_config' => [
'ctrl' => [
'label' => 'title'
],
'columns' => [
'title' => [
'config' => [
'type' => 'input'
]
],
'indexcfgs' => [
'config' => [
'type' => 'group',
......@@ -233,49 +274,50 @@ class BackendUtilityTest extends UnitTestCase
],
],
],
'pages' => [
'ctrl' => [
'label' => 'title'
],
'columns' => [
'title' => [
'config' => [
'type' => 'input'
]
]
]
]
];
$GLOBALS['LANG'] = [];
self::assertSame('Page 1, Configuration 2', ProcessedValueForGroupWithMultipleAllowedTablesFixture::getProcessedValue('index_config', 'indexcfgs', 'pages_1,index_config_2'));
}
/**
* Prepare a mock database setup for a Doctrine connection
* and return an array of all prophets to set expectations upon.
*
* @param string $tableName
* @return array
*/
protected function mockDatabaseConnection($tableName = 'sys_category')
{
$connectionProphet = $this->prophesize(Connection::class);
$connectionProphet->quote(Argument::cetera())->will(function ($arguments) {
return "'" . $arguments[0] . "'";
});
$connectionProphet->quoteIdentifier(Argument::cetera())->will(function ($arguments) {
return '`' . $arguments[0] . '`';
});
$restrictionProphet = $this->prophesize(DefaultRestrictionContainer::class);
$restrictionProphet->removeAll()->willReturn($restrictionProphet->reveal());
$restrictionProphet->add(Argument::cetera())->willReturn($restrictionProphet->reveal());
$queryBuilderProphet = $this->prophesize(QueryBuilder::class);
$queryBuilderProphet->expr()->willReturn(
GeneralUtility::makeInstance(ExpressionBuilder::class, $connectionProphet->reveal())
);
$queryBuilderProphet->getRestrictions()->willReturn($restrictionProphet->reveal());
$queryBuilderProphet->quoteIdentifier(Argument::cetera())->will(function ($arguments) {
return '`' . $arguments[0] . '`';
});
$connectionPoolProphet = $this->prophesize(ConnectionPool::class);
$connectionPoolProphet->getConnectionForTable($tableName)
->willReturn($connectionProphet->reveal());
$connectionPoolProphet->getQueryBuilderForTable($tableName)
->shouldBeCalled()
->willReturn($queryBuilderProphet->reveal());
return [$queryBuilderProphet, $connectionPoolProphet, $connectionProphet, $restrictionProphet];
$languageServiceProphecy = $this->prophesize(LanguageService::class);
$languageServiceProphecy->sL(Argument::cetera())->willReturnArgument(0);
$GLOBALS['LANG'] = $languageServiceProphecy->reveal();
/** @var RelationHandler|ObjectProphecy $relationHandlerProphet */
$relationHandlerProphet = $this->prophesize(RelationHandler::class);
$relationHandlerProphet->start(Argument::cetera())->shouldBeCalled();
$relationHandlerProphet->getFromDB()->willReturn([]);
$relationHandlerProphet->getResolvedItemArray()->willReturn([
[
'table' => 'pages',
'uid' => 1,
'record' => [
'uid' => 1,
'pid' => 0,
'title' => 'Page 1'
]
],
[
'table' => 'index_config',
'uid' => 2,
'record' => [
'uid' => 2,
'pid' => 0,
'title' => 'Configuration 2'
]
]
]);
GeneralUtility::addInstance(RelationHandler::class, $relationHandlerProphet->reveal());
self::assertSame('Page 1, Configuration 2', BackendUtility::getProcessedValue('index_config', 'indexcfgs', 'pages_1,index_config_2'));
}
/**
......@@ -286,33 +328,32 @@ class BackendUtilityTest extends UnitTestCase
/** @var RelationHandler|ObjectProphecy $relationHandlerProphet */
$relationHandlerProphet = $this->prophesize(RelationHandler::class);
$relationHandlerProphet->start(Argument::cetera())->shouldBeCalled();
$relationHandlerInstance = $relationHandlerProphet->reveal();
$relationHandlerInstance->tableArray['sys_category'] = [1, 2];
[$queryBuilderProphet, $connectionPoolProphet] = $this->mockDatabaseConnection('sys_category');
$statementProphet = $this->prophesize(Statement::class);
$statementProphet->fetchAssociative()->shouldBeCalled()->willReturn(
$relationHandlerProphet->getFromDB()->willReturn([]);
$relationHandlerProphet->getResolvedItemArray()->willReturn([
[
'table' => 'sys_category',
'uid' => 1,
'title' => 'Category 1',
'record' => [
'uid' => 2,