From 3e1796dca4a80e111ae0155f7b67599fa0d1e5bc Mon Sep 17 00:00:00 2001 From: Ingolf Steinhardt Date: Mon, 9 Feb 2026 20:34:49 +0100 Subject: [PATCH 1/2] Speed up pagination performance --- src/MetaModel.php | 119 ++++++++++++++++++++++++++++++---------------- 1 file changed, 79 insertions(+), 40 deletions(-) diff --git a/src/MetaModel.php b/src/MetaModel.php index 69d339436..b9cbf0967 100644 --- a/src/MetaModel.php +++ b/src/MetaModel.php @@ -3,7 +3,7 @@ /** * This file is part of MetaModels/core. * - * (c) 2012-2025 The MetaModels team. + * (c) 2012-2026 The MetaModels team. * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. @@ -21,7 +21,7 @@ * @author David Molineus * @author Sven Baumann * @author Ingolf Steinhardt - * @copyright 2012-2025 The MetaModels team. + * @copyright 2012-2026 The MetaModels team. * @license https://github.com/MetaModels/core/blob/master/LICENSE LGPL-3.0-or-later * @filesource */ @@ -166,6 +166,7 @@ public function __construct( * * @deprecated Inject services via constructor or setter. */ + #[\Override] public function getServiceContainer() { // @codingStandardsIgnoreStart @@ -254,6 +255,7 @@ protected function tryUnserialize(mixed $value): mixed /** * {@inheritdoc} */ + #[\Override] public function addAttribute(IAttribute $objAttribute) { if ($objAttribute instanceof ITranslated && !$this instanceof ITranslatedMetaModel) { @@ -279,6 +281,7 @@ public function addAttribute(IAttribute $objAttribute) /** * {@inheritdoc} */ + #[\Override] public function hasAttribute($strAttributeName) { return \array_key_exists($strAttributeName, $this->arrAttributes); @@ -385,11 +388,37 @@ protected function getMatchingIds($objFilter) } } + return $this->getMatchingIdsAuthoritative($objFilter); + } + + /** + * Narrow down the list of Ids that match the given filter. + * + * @param IFilter|null $objFilter The filter to search the matching ids for. + * + * @return list all matching Ids. + */ + protected function getMatchingIdsAuthoritative(?IFilter $objFilter): array + { + $builder = $this->getConnection()->createQueryBuilder(); + $builder + ->select('t.id') + ->from($this->getTableName(), 't'); + + if ($objFilter) { + $arrFilteredIds = $objFilter->getMatchingIds(); + if ($arrFilteredIds !== null) { + $builder + ->where($builder->expr()->in('t.id', ':values')) + ->setParameter('values', $arrFilteredIds, ArrayParameterType::STRING); + } + } + + // FIXME: Cache list is possible, if cachable filter interface is implemented by all filter rules. + // Either no filter object or all ids allowed => return all ids. // if no id filter is passed, we assume all ids are provided. - return $this->getConnection()->createQueryBuilder() - ->select('t.id') - ->from($this->getTableName(), 't') + return $builder ->executeQuery() ->fetchFirstColumn(); } @@ -609,6 +638,7 @@ protected function copyFilter($objFilter) /** * {@inheritdoc} */ + #[\Override] public function get($strKey) { // Try to retrieve via getter method. @@ -628,6 +658,7 @@ public function get($strKey) /** * {@inheritdoc} */ + #[\Override] public function getTableName() { return ($this->arrData['tableName'] ?? ''); @@ -636,6 +667,7 @@ public function getTableName() /** * {@inheritdoc} */ + #[\Override] public function getName() { return $this->arrData['name']; @@ -644,6 +676,7 @@ public function getName() /** * {@inheritdoc} */ + #[\Override] public function getAttributes() { return $this->arrAttributes; @@ -652,6 +685,7 @@ public function getAttributes() /** * {@inheritdoc} */ + #[\Override] public function getInVariantAttributes() { $arrAttributes = $this->getAttributes(); @@ -674,6 +708,7 @@ public function getInVariantAttributes() * * @deprecated Please implement ITranslatedMetaModel instead. */ + #[\Override] public function isTranslated(bool $deprecation = true) { if (!$deprecation) { @@ -684,7 +719,7 @@ public function isTranslated(bool $deprecation = true) // @codingStandardsIgnoreStart @\trigger_error( \sprintf('The method "%s" is deprecated since MetaModels 2.2 and to be removed in 3.0. ' . - 'Please use "instanceof \MetaModels\ITranslatedMetaModel" instead.', __METHOD__), + 'Please use "instanceof \MetaModels\ITranslatedMetaModel" instead.', __METHOD__), E_USER_DEPRECATED ); // @codingStandardsIgnoreEnd @@ -710,7 +745,7 @@ public function isTranslated(bool $deprecation = true) // @codingStandardsIgnoreStart @\trigger_error( \sprintf('The method "%s" is deprecated since MetaModels 2.2 and to be removed in 3.0. ' . - 'Please use "instanceof \MetaModels\ITranslatedMetaModel" instead.', __METHOD__), + 'Please use "instanceof \MetaModels\ITranslatedMetaModel" instead.', __METHOD__), E_USER_DEPRECATED ); // @codingStandardsIgnoreEnd @@ -721,6 +756,7 @@ public function isTranslated(bool $deprecation = true) /** * {@inheritdoc} */ + #[\Override] public function hasVariants() { return $this->arrData['varsupport']; @@ -731,13 +767,14 @@ public function hasVariants() * * @deprecated Please implement ITranslatedMetaModel instead. */ + #[\Override] public function getAvailableLanguages() { if ($this instanceof ITranslatedMetaModel) { // @codingStandardsIgnoreStart @\trigger_error( \sprintf('The method "%s" is deprecated since MetaModels 2.2 and to be removed in 3.0. ' . - 'Please use "\MetaModels\ITranslatedMetaModel::getLanguages" instead.', __METHOD__), + 'Please use "\MetaModels\ITranslatedMetaModel::getLanguages" instead.', __METHOD__), E_USER_DEPRECATED ); // @codingStandardsIgnoreEnd @@ -749,8 +786,8 @@ public function getAvailableLanguages() // @codingStandardsIgnoreStart @\trigger_error( \sprintf('The method "%s" is deprecated since MetaModels 2.2 and to be removed in 3.0. ' . - 'Please test for "instanceof "\MetaModels\ITranslatedMetaModel" and use '. - '"\MetaModels\ITranslatedMetaModel::getLanguages" instead.', __METHOD__), + 'Please test for "instanceof "\MetaModels\ITranslatedMetaModel" and use '. + '"\MetaModels\ITranslatedMetaModel::getLanguages" instead.', __METHOD__), E_USER_DEPRECATED ); // @codingStandardsIgnoreEnd @@ -760,7 +797,7 @@ public function getAvailableLanguages() // @codingStandardsIgnoreStart @\trigger_error( \sprintf('The method "%s" is deprecated since MetaModels 2.2 and to be removed in 3.0. ' . - 'Please test for "instanceof \MetaModels\ITranslatedMetaModel".', __METHOD__), + 'Please test for "instanceof \MetaModels\ITranslatedMetaModel".', __METHOD__), E_USER_DEPRECATED ); // @codingStandardsIgnoreEnd @@ -773,13 +810,14 @@ public function getAvailableLanguages() * * @deprecated Please implement ITranslatedMetaModel instead. */ + #[\Override] public function getFallbackLanguage() { if ($this instanceof ITranslatedMetaModel) { // @codingStandardsIgnoreStart @\trigger_error( \sprintf('The method "%s" is deprecated since MetaModels 2.2 and to be removed in 3.0. ' . - 'Please use "\MetaModels\ITranslatedMetaModel::getMainLanguage" instead.', __METHOD__), + 'Please use "\MetaModels\ITranslatedMetaModel::getMainLanguage" instead.', __METHOD__), E_USER_DEPRECATED ); @@ -791,8 +829,8 @@ public function getFallbackLanguage() // @codingStandardsIgnoreStart @\trigger_error( \sprintf('The method "%s" is deprecated since MetaModels 2.2 and to be removed in 3.0. ' . - 'Please implement interface "\MetaModels\ITranslatedMetaModel" and use ' . - '"\MetaModels\ITranslatedMetaModel::getMainLanguage" instead.', __METHOD__), + 'Please implement interface "\MetaModels\ITranslatedMetaModel" and use ' . + '"\MetaModels\ITranslatedMetaModel::getMainLanguage" instead.', __METHOD__), E_USER_DEPRECATED ); // @codingStandardsIgnoreEnd @@ -805,8 +843,8 @@ public function getFallbackLanguage() // @codingStandardsIgnoreStart @\trigger_error( \sprintf('The method "%s" is deprecated since MetaModels 2.2 and to be removed in 3.0. ' . - 'Please test for translations via "instanceof \MetaModels\ITranslatedMetaModel" and call ' . - '"\MetaModels\ITranslatedMetaModel::getMainLanguage" instead.', __METHOD__), + 'Please test for translations via "instanceof \MetaModels\ITranslatedMetaModel" and call ' . + '"\MetaModels\ITranslatedMetaModel::getMainLanguage" instead.', __METHOD__), E_USER_DEPRECATED ); // @codingStandardsIgnoreEnd @@ -824,14 +862,15 @@ public function getFallbackLanguage() * * @deprecated Please implement ITranslatedMetaModel instead. */ + #[\Override] public function getActiveLanguage() { if ($this instanceof ITranslatedMetaModel) { // @codingStandardsIgnoreStart @\trigger_error( \sprintf('The method "%s" is deprecated since MetaModels 2.2 and to be removed in 3.0. ' . - 'Please use "\MetaModels\ITranslatedMetaModel::getLanguage" and ' . - '"\MetaModels\ITranslatedMetaModel::selectLanguage" instead.', __METHOD__), + 'Please use "\MetaModels\ITranslatedMetaModel::getLanguage" and ' . + '"\MetaModels\ITranslatedMetaModel::selectLanguage" instead.', __METHOD__), E_USER_DEPRECATED ); // @codingStandardsIgnoreEnd @@ -839,7 +878,7 @@ public function getActiveLanguage() // @codingStandardsIgnoreStart @\trigger_error( \sprintf('The method "%s" is deprecated since MetaModels 2.2 and to be removed in 3.0. ' . - 'Please use "\MetaModels\ITranslatedMetaModel::getLanguage" instead.', __METHOD__), + 'Please use "\MetaModels\ITranslatedMetaModel::getLanguage" instead.', __METHOD__), E_USER_DEPRECATED ); // @codingStandardsIgnoreEnd @@ -852,6 +891,7 @@ public function getActiveLanguage() /** * {@inheritdoc} */ + #[\Override] public function getAttribute($strAttributeName) { $arrAttributes = $this->getAttributes(); @@ -862,6 +902,7 @@ public function getAttribute($strAttributeName) /** * {@inheritdoc} */ + #[\Override] public function getAttributeById($intId) { foreach ($this->getAttributes() as $objAttribute) { @@ -900,6 +941,7 @@ protected function getAttributeByNames($attrNames = []) /** * {@inheritdoc} */ + #[\Override] public function findById($intId, $arrAttrOnly = []) { if (!$intId) { @@ -917,6 +959,7 @@ public function findById($intId, $arrAttrOnly = []) /** * {@inheritdoc} */ + #[\Override] public function findByFilter( $objFilter, $strSortBy = '', @@ -942,6 +985,7 @@ public function findByFilter( * * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ + #[\Override] public function getIdsFromFilter($objFilter, $strSortBy = '', $intOffset = 0, $intLimit = 0, $strSortOrder = 'ASC') { if ([] === $arrFilteredIds = \array_values(\array_filter($this->getMatchingIds($objFilter)))) { @@ -974,6 +1018,7 @@ public function getIdsFromFilter($objFilter, $strSortBy = '', $intOffset = 0, $i return \array_values($cacheResult); } + // FIXME: implementation incomplete! Came with 2.0.5. // Merge the already known and the new one. $fullIdList = \array_merge((array) $this->existingIds[$sortKey], $arrFilteredIds); $fullIdList = \array_keys(\array_flip($fullIdList)); @@ -1008,27 +1053,18 @@ public function getIdsFromFilter($objFilter, $strSortBy = '', $intOffset = 0, $i /** * {@inheritdoc} */ + #[\Override] public function getCount($objFilter) { - $arrFilteredIds = $this->getMatchingIds($objFilter); - if (0 === \count($arrFilteredIds)) { - return 0; - } + $arrFilteredIds = $this->getMatchingIdsAuthoritative($objFilter); - $builder = $this->getConnection()->createQueryBuilder(); - - return (int) $builder - ->select('COUNT(t.id)') - ->from($this->getTableName(), 't') - ->where($builder->expr()->in('t.id', ':values')) - ->setParameter('values', $arrFilteredIds, ArrayParameterType::STRING) - ->executeQuery() - ->fetchOne(); + return \count($arrFilteredIds); } /** * {@inheritdoc} */ + #[\Override] public function findVariantBase($objFilter) { $objNewFilter = $this->copyFilter($objFilter); @@ -1050,6 +1086,7 @@ public function findVariantBase($objFilter) /** * {@inheritdoc} */ + #[\Override] public function findVariants($arrIds, $objFilter) { if (!$arrIds) { @@ -1077,6 +1114,7 @@ public function findVariants($arrIds, $objFilter) /** * {@inheritdoc} */ + #[\Override] public function findVariantsWithBase($arrIds, $objFilter) { if (!$arrIds) { @@ -1104,6 +1142,7 @@ public function findVariantsWithBase($arrIds, $objFilter) /** * {@inheritdoc} */ + #[\Override] public function getAttributeOptions($strAttribute, $objFilter = null) { $objAttribute = $this->getAttribute($strAttribute); @@ -1268,6 +1307,7 @@ protected function createNewItem($item) /** * {@inheritdoc} */ + #[\Override] public function saveItem($objItem, $timestamp = null) { if (null === $timestamp) { @@ -1289,8 +1329,8 @@ public function saveItem($objItem, $timestamp = null) // @codingStandardsIgnoreStart @\trigger_error( \sprintf('Support for translated MetaModels not implementing "\MetaModels\ITranslatedMetaModel" '. - 'is %s deprecated since MetaModels 2.2 and to be removed in 3.0. ' . - 'Please implement interface "\MetaModels\ITranslatedMetaModel".', __METHOD__), + 'is %s deprecated since MetaModels 2.2 and to be removed in 3.0. ' . + 'Please implement interface "\MetaModels\ITranslatedMetaModel".', __METHOD__), E_USER_DEPRECATED ); // @codingStandardsIgnoreEnd @@ -1322,6 +1362,7 @@ public function saveItem($objItem, $timestamp = null) /** * {@inheritdoc} */ + #[\Override] public function delete(IItem $objItem) { $arrIds = [$objItem->get('id')]; @@ -1355,6 +1396,7 @@ public function delete(IItem $objItem) /** * {@inheritdoc} */ + #[\Override] public function getEmptyFilter() { return new Filter($this); @@ -1363,6 +1405,7 @@ public function getEmptyFilter() /** * {@inheritdoc} */ + #[\Override] public function prepareFilter($intFilterSettings, $arrFilterUrl) { // @codingStandardsIgnoreStart @@ -1385,6 +1428,7 @@ public function prepareFilter($intFilterSettings, $arrFilterUrl) /** * {@inheritdoc} */ + #[\Override] public function getView($intViewId = 0) { // @codingStandardsIgnoreStart @@ -1399,12 +1443,7 @@ public function getView($intViewId = 0) return $this->getServiceContainer()->getRenderSettingFactory()->createCollection($this, (string) $intViewId); } - /** - * Obtain the doctrine connection. - * - * @return Connection - */ - private function getConnection() + private function getConnection(): Connection { return $this->connection; } From b6facd3e620aa8c9fb47b1330dd03a46de32f73d Mon Sep 17 00:00:00 2001 From: Ingolf Steinhardt Date: Mon, 9 Feb 2026 22:34:52 +0100 Subject: [PATCH 2/2] Speed up pagination performance --- psalm.xml | 5 --- tests/MetaModelsTest.php | 78 +++++++--------------------------------- 2 files changed, 12 insertions(+), 71 deletions(-) diff --git a/psalm.xml b/psalm.xml index 9d25b2300..c38e88615 100644 --- a/psalm.xml +++ b/psalm.xml @@ -27,11 +27,6 @@ - - - - - diff --git a/tests/MetaModelsTest.php b/tests/MetaModelsTest.php index 4f91d560a..0c1229e48 100644 --- a/tests/MetaModelsTest.php +++ b/tests/MetaModelsTest.php @@ -3,7 +3,7 @@ /** * This file is part of MetaModels/core. * - * (c) 2012-2021 The MetaModels team. + * (c) 2012-2026 The MetaModels team. * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. @@ -13,13 +13,15 @@ * @package MetaModels/core * @author Christian Schiffler * @author Sven Baumann - * @copyright 2012-2021 The MetaModels team. + * @author Ingolf Steinhardt + * @copyright 2012-2026 The MetaModels team. * @license https://github.com/MetaModels/core/blob/master/LICENSE LGPL-3.0-or-later * @filesource */ namespace MetaModels\Test; +use Doctrine\DBAL\ArrayParameterType; use Doctrine\DBAL\Connection; use Doctrine\DBAL\Query\Expression\ExpressionBuilder; use Doctrine\DBAL\Query\QueryBuilder; @@ -99,7 +101,6 @@ public function testBuildDatabaseParameterList(): void ); $reflection = new \ReflectionMethod($metaModel, 'buildDatabaseParameterList'); - $reflection->setAccessible(true); self::assertEquals('?', $reflection->invoke($metaModel, [1])); self::assertEquals('?,?', $reflection->invoke($metaModel, [1, 2])); self::assertEquals('?,?,?,?,?,?', $reflection->invoke($metaModel, [1, 2, 'fooo', 'bar', null, 'test'])); @@ -159,7 +160,7 @@ public function testRetrieveSystemColumns(): void $builder ->expects($this->once()) ->method('setParameter') - ->with('values', [1], Connection::PARAM_STR_ARRAY) + ->with('values', [1], ArrayParameterType::STRING) ->willReturn($builder); $builder @@ -278,7 +279,7 @@ public function testGetIdsFromFilterSortedByPid(): void $builder ->expects($this->once()) ->method('setParameter') - ->with('values', [4, 3, 2, 1], Connection::PARAM_STR_ARRAY) + ->with('values', [4, 3, 2, 1], ArrayParameterType::STRING) ->willReturn($builder); $builder @@ -366,7 +367,7 @@ public function testGetIdsFromFilterSortedByPidWithCache(): void $builder ->expects($this->once()) ->method('setParameter') - ->with('values', [4, 3, 2, 1], Connection::PARAM_STR_ARRAY) + ->with('values', [4, 3, 2, 1], ArrayParameterType::STRING) ->willReturn($builder); $builder @@ -409,7 +410,7 @@ public function testGetCountForEmptyList(): void { $metaModel = $this ->getMockBuilder(MetaModel::class) - ->onlyMethods(['getMatchingIds']) + ->onlyMethods(['getMatchingIdsAuthoritative']) ->setConstructorArgs( [ ['tableName' => 'mm_test_retrieve'], @@ -420,7 +421,7 @@ public function testGetCountForEmptyList(): void ->getMock(); $metaModel ->expects(self::once()) - ->method('getMatchingIds') + ->method('getMatchingIdsAuthoritative') ->willReturn([]); /** @var MetaModel $metaModel */ @@ -433,71 +434,16 @@ public function testGetCountForEmptyList(): void public function testGetCountForNonEmptyList(): void { $metaModel = $this->getMockBuilder(MetaModel::class) - ->onlyMethods(['getMatchingIds']) + ->onlyMethods(['getMatchingIdsAuthoritative']) ->setConstructorArgs([ ['tableName' => 'mm_test_retrieve'], $this->getMockForAbstractClass(EventDispatcherInterface::class), - $this->mockConnection( - (function () { - $builder = $this - ->getMockBuilder(QueryBuilder::class) - ->disableOriginalConstructor() - ->getMock(); - $builder - ->expects($this->once()) - ->method('select') - ->with('COUNT(t.id)') - ->willReturn($builder); - $builder - ->expects($this->once()) - ->method('from') - ->with('mm_test_retrieve', 't') - ->willReturn($builder); - - $expr = $this - ->getMockBuilder(ExpressionBuilder::class) - ->disableOriginalConstructor() - ->onlyMethods([]) - ->getMock(); - - $builder - ->expects($this->once()) - ->method('expr') - ->willReturn($expr); - - $builder - ->expects($this->once()) - ->method('where') - ->with('t.id IN (:values)') - ->willReturn($builder); - - $builder - ->expects($this->once()) - ->method('setParameter') - ->with('values', [4, 3, 2, 1], Connection::PARAM_STR_ARRAY) - ->willReturn($builder); - - $result = $this - ->getMockBuilder(Result::class) - ->disableOriginalConstructor() - ->getMock(); - $result - ->expects($this->once()) - ->method('fetchOne') - ->willReturn(4); - $builder - ->expects($this->once()) - ->method('executeQuery') - ->willReturn($result); - - return $builder; - })->__invoke() - ) + $this->mockConnection() ]) ->getMock(); $metaModel ->expects(self::once()) - ->method('getMatchingIds') + ->method('getMatchingIdsAuthoritative') ->willReturn([4, 3, 2, 1]); self::assertEquals(4, $metaModel->getCount($metaModel->getEmptyFilter()));