diff --git a/core/lib/Drupal/Core/Database/Connection.php b/core/lib/Drupal/Core/Database/Connection.php index 3f194e1aa052..a5c78acc445d 100644 --- a/core/lib/Drupal/Core/Database/Connection.php +++ b/core/lib/Drupal/Core/Database/Connection.php @@ -11,6 +11,7 @@ use Drupal\Core\Database\Query\Select; use Drupal\Core\Database\Query\Truncate; use Drupal\Core\Database\Query\Update; use Drupal\Core\Database\Query\Upsert; +use Drupal\Core\Pager\PagerManagerInterface; /** * Base Database API class. @@ -1968,4 +1969,17 @@ abstract class Connection { return ($first === 'Drupal' && strtolower($second) === $second) ? $second : 'core'; } + /** + * Get the pager manager service, if available. + * + * @return \Drupal\Core\Pager\PagerManagerInterface + * The pager manager service, if available. + * + * @throws \Drupal\Core\DependencyInjection\ContainerNotInitializedException + * If the container has not been initialized yet. + */ + public function getPagerManager(): PagerManagerInterface { + return \Drupal::service('pager.manager'); + } + } diff --git a/core/lib/Drupal/Core/Database/Query/PagerSelectExtender.php b/core/lib/Drupal/Core/Database/Query/PagerSelectExtender.php index f32b1c733cf8..1a53ec860143 100644 --- a/core/lib/Drupal/Core/Database/Query/PagerSelectExtender.php +++ b/core/lib/Drupal/Core/Database/Query/PagerSelectExtender.php @@ -20,6 +20,11 @@ class PagerSelectExtender extends SelectExtender { * The highest element we've autogenerated so far. * * @var int + * + * @deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. Use + * \Drupal::service('pager.manager')->getMaxPagerElementId() instead. + * + * @see https://www.drupal.org/node/3194594 */ public static $maxElement = 0; @@ -44,6 +49,14 @@ class PagerSelectExtender extends SelectExtender { */ protected $customCountQuery = FALSE; + /** + * Constructs a PagerSelectExtender object. + * + * @param \Drupal\Core\Database\Query\SelectInterface $query + * Select query object. + * @param \Drupal\Core\Database\Connection $connection + * Database connection object. + */ public function __construct(SelectInterface $query, Connection $connection) { parent::__construct($query, $connection); @@ -73,9 +86,7 @@ class PagerSelectExtender extends SelectExtender { $this->ensureElement(); $total_items = $this->getCountQuery()->execute()->fetchField(); - /** @var \Drupal\Core\Pager\PagerManagerInterface $pager_manager */ - $pager_manager = \Drupal::service('pager.manager'); - $pager = $pager_manager->createPager($total_items, $this->limit, $this->element); + $pager = $this->connection->getPagerManager()->createPager($total_items, $this->limit, $this->element); $this->range($pager->getCurrentPage() * $this->limit, $this->limit); // Now that we've added our pager-based range instructions, run the query normally. @@ -84,15 +95,13 @@ class PagerSelectExtender extends SelectExtender { /** * Ensure that there is an element associated with this query. - * If an element was not specified previously, then the value of the - * $maxElement counter is taken, after which the counter is incremented. * * After running this method, access $this->element to get the element for this * query. */ protected function ensureElement() { if (!isset($this->element)) { - $this->element = self::$maxElement++; + $this->element($this->connection->getPagerManager()->getMaxPagerElementId() + 1); } } @@ -151,9 +160,6 @@ class PagerSelectExtender extends SelectExtender { * whatever reason you want to explicitly define an element for a given query, * you may do so here. * - * Setting the element here also increments the static $maxElement counter, - * which is used for determining the $element when there's none specified. - * * Note that no collision detection is done when setting an element ID * explicitly, so it is possible for two pagers to end up using the same ID * if both are set explicitly. @@ -163,10 +169,23 @@ class PagerSelectExtender extends SelectExtender { */ public function element($element) { $this->element = $element; - if ($element >= self::$maxElement) { - self::$maxElement = $element + 1; - } + $this->connection->getPagerManager()->reservePagerElementId($this->element); return $this; } + /** + * Gets the element ID for this pager query. + * + * The element is used to differentiate different pager queries on the same + * page so that they may be operated independently. + * + * @return int + * Element ID that is used to differentiate between different pager + * queries. + */ + public function getElement(): int { + $this->ensureElement(); + return $this->element; + } + } diff --git a/core/lib/Drupal/Core/Entity/Query/QueryBase.php b/core/lib/Drupal/Core/Entity/Query/QueryBase.php index a1e5e2c92311..6c43c919eef1 100644 --- a/core/lib/Drupal/Core/Entity/Query/QueryBase.php +++ b/core/lib/Drupal/Core/Entity/Query/QueryBase.php @@ -2,7 +2,6 @@ namespace Drupal\Core\Entity\Query; -use Drupal\Core\Database\Query\PagerSelectExtender; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Utility\TableSort; @@ -289,10 +288,7 @@ abstract class QueryBase implements QueryInterface { // Even when not using SQL, storing the element PagerSelectExtender is as // good as anywhere else. if (!isset($element)) { - $element = PagerSelectExtender::$maxElement++; - } - elseif ($element >= PagerSelectExtender::$maxElement) { - PagerSelectExtender::$maxElement = $element + 1; + $element = \Drupal::service('pager.manager')->getMaxPagerElementId() + 1; } $this->pager = [ diff --git a/core/lib/Drupal/Core/Pager/PagerManager.php b/core/lib/Drupal/Core/Pager/PagerManager.php index 5e29724dc1d9..6a9cb7c9b82a 100644 --- a/core/lib/Drupal/Core/Pager/PagerManager.php +++ b/core/lib/Drupal/Core/Pager/PagerManager.php @@ -2,6 +2,7 @@ namespace Drupal\Core\Pager; +use Drupal\Core\Database\Query\PagerSelectExtender; use Drupal\Core\DependencyInjection\DependencySerializationTrait; /** @@ -31,6 +32,13 @@ class PagerManager implements PagerManagerInterface { */ protected $pagers; + /** + * The highest pager ID created so far. + * + * @var int + */ + protected $maxPagerElementId = -1; + /** * Construct a PagerManager object. * @@ -90,13 +98,20 @@ class PagerManager implements PagerManagerInterface { } /** - * Gets the extent of the pager page element IDs. - * - * @return int - * The maximum element ID available, -1 if there are no elements. + * {@inheritdoc} */ - protected function getMaxPagerElementId() { - return empty($this->pagers) ? -1 : max(array_keys($this->pagers)); + public function getMaxPagerElementId() { + return $this->maxPagerElementId; + } + + /** + * {@inheritdoc} + */ + public function reservePagerElementId(int $element): void { + $this->maxPagerElementId = max($element, $this->maxPagerElementId); + // BC for PagerSelectExtender::$maxElement. + // @todo remove the line below in D10. + PagerSelectExtender::$maxElement = $this->getMaxPagerElementId(); } /** @@ -108,6 +123,7 @@ class PagerManager implements PagerManagerInterface { * The pager index. */ protected function setPager(Pager $pager, $element = 0) { + $this->maxPagerElementId = max($element, $this->maxPagerElementId); $this->pagers[$element] = $pager; } diff --git a/core/lib/Drupal/Core/Pager/PagerManagerInterface.php b/core/lib/Drupal/Core/Pager/PagerManagerInterface.php index 94db9320ac33..113fcb843025 100644 --- a/core/lib/Drupal/Core/Pager/PagerManagerInterface.php +++ b/core/lib/Drupal/Core/Pager/PagerManagerInterface.php @@ -33,7 +33,7 @@ interface PagerManagerInterface { * before executing it. For example: * @code * $query = $connection->select('some_table') - * ->extend('Drupal\Core\Database\Query\PagerSelectExtender'); + * ->extend(PagerSelectExtender::class); * @endcode * * However, if you are using a different method for generating the items to be @@ -163,4 +163,26 @@ interface PagerManagerInterface { */ public function getUpdatedParameters(array $query, $element, $index); + /** + * Gets the extent of the pager page element IDs. + * + * @return int + * The maximum element ID available, -1 if there are no elements. + */ + public function getMaxPagerElementId(); + + /** + * Reserve a pager element ID. + * + * Calling code may need to reserve the ID of a pager before actually creating + * it. This methods allows to do so ensuring no collision occurs with + * ::getMaxPagerElementId(). + * + * @param int $element + * The ID of the pager to be reserved. + * + * @see \Drupal\Core\Database\Query\PagerSelectExtender::element() + */ + public function reservePagerElementId(int $element): void; + } diff --git a/core/modules/system/tests/src/Functional/Database/SelectPagerDefaultTest.php b/core/modules/system/tests/src/Functional/Database/SelectPagerDefaultTest.php index 3819f45b7c2f..c5e2e38816b1 100644 --- a/core/modules/system/tests/src/Functional/Database/SelectPagerDefaultTest.php +++ b/core/modules/system/tests/src/Functional/Database/SelectPagerDefaultTest.php @@ -144,34 +144,46 @@ class SelectPagerDefaultTest extends DatabaseTestBase { \Drupal::getContainer()->get('request_stack')->push($request); $connection = Database::getConnection(); - $name = $connection->select('test', 't') + $query = $connection->select('test', 't') ->extend(PagerSelectExtender::class) ->element(2) ->fields('t', ['name']) ->orderBy('age') - ->limit(1) - ->execute() + ->limit(1); + $this->assertSame(2, $query->getElement()); + // BC for PagerSelectExtender::$maxElement. + // @todo remove the assertion below in D10. + $this->assertSame(2, PagerSelectExtender::$maxElement); + $name = $query->execute() ->fetchField(); $this->assertEqual('Paul', $name, 'Pager query #1 with a specified element ID returned the correct results.'); - // Setting an element smaller than the previous one - // should not overwrite the pager $maxElement with a smaller value. - $name = $connection->select('test', 't') + // Setting an element smaller than the previous one should not collide with + // the existing pager. + $query = $connection->select('test', 't') ->extend(PagerSelectExtender::class) ->element(1) ->fields('t', ['name']) ->orderBy('age') - ->limit(1) - ->execute() + ->limit(1); + $this->assertSame(1, $query->getElement()); + // BC for PagerSelectExtender::$maxElement. + // @todo remove the assertion below in D10. + $this->assertSame(2, PagerSelectExtender::$maxElement); + $name = $query->execute() ->fetchField(); $this->assertEqual('George', $name, 'Pager query #2 with a specified element ID returned the correct results.'); - $name = $connection->select('test', 't') + $query = $connection->select('test', 't') ->extend(PagerSelectExtender::class) ->fields('t', ['name']) ->orderBy('age') - ->limit(1) - ->execute() + ->limit(1); + $this->assertSame(3, $query->getElement()); + // BC for PagerSelectExtender::$maxElement. + // @todo remove the assertion below in D10. + $this->assertSame(3, PagerSelectExtender::$maxElement); + $name = $query->execute() ->fetchField(); $this->assertEqual('John', $name, 'Pager query #3 with a generated element ID returned the correct results.'); diff --git a/core/tests/Drupal/Tests/Core/Database/SelectTest.php b/core/tests/Drupal/KernelTests/Core/Database/SelectExtenderTest.php similarity index 92% rename from core/tests/Drupal/Tests/Core/Database/SelectTest.php rename to core/tests/Drupal/KernelTests/Core/Database/SelectExtenderTest.php index 867cc61989fe..5a23f9a2500a 100644 --- a/core/tests/Drupal/Tests/Core/Database/SelectTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/SelectExtenderTest.php @@ -1,20 +1,20 @@ addPsr4("Drupal\\corefake\\Driver\\Database\\corefake\\", __DIR__ . "/../../../../../tests/fixtures/database_drivers/module/corefake/src/Driver/Database/corefake"); $additional_class_loader->addPsr4("Drupal\\corefake\\Driver\\Database\\corefakeWithAllCustomClasses\\", __DIR__ . "/../../../../../tests/fixtures/database_drivers/module/corefake/src/Driver/Database/corefakeWithAllCustomClasses"); @@ -130,7 +130,7 @@ class SelectTest extends UnitTestCase { // Get an instance of the class \Drupal\Core\Database\Query\SelectExtender. $select_extender = $connection->select('test')->extend(SelectExtender::class); - $this->assertEquals('Drupal\Core\Database\Query\SelectExtender', get_class($select_extender)); + $this->assertEquals(SelectExtender::class, get_class($select_extender)); // Tests the method \Drupal\Core\Database\Query\SelectExtender::extend(). $select_extender_extended = $select_extender->extend($extend); diff --git a/core/tests/Drupal/KernelTests/Core/Pager/PagerManagerTest.php b/core/tests/Drupal/KernelTests/Core/Pager/PagerManagerTest.php index 3f1442141cc8..2f5d31161aad 100644 --- a/core/tests/Drupal/KernelTests/Core/Pager/PagerManagerTest.php +++ b/core/tests/Drupal/KernelTests/Core/Pager/PagerManagerTest.php @@ -53,4 +53,36 @@ class PagerManagerTest extends KernelTestBase { $this->assertEquals(10, $pager_manager->findPage(1)); } + /** + * @covers ::getMaxPagerElementId + * + * @dataProvider providerTestGetMaxPagerElementId + */ + public function testGetMaxPagerElementId(array $elements, int $expected_max_element_id): void { + /* @var $pager_manager \Drupal\Core\Pager\PagerManagerInterface */ + $pager_manager = $this->container->get('pager.manager'); + + foreach ($elements as $element) { + $pager_manager->createPager(30, 10, $element); + } + + $this->assertEquals($expected_max_element_id, $pager_manager->getMaxPagerElementId()); + } + + /** + * Provides test cases for PagerManagerTest::testGetMaxPagerElementId(). + * + * @return array + * An array of test cases, each which the following values: + * - Array of elements to pass to PagerManager::createPager(). + * - The expected value returned by PagerManager::getMaxPagerElementId(). + */ + public function providerTestGetMaxPagerElementId(): array { + return [ + 'no_pager' => [[], -1], + 'single_pager' => [[0], 0], + 'multiple_pagers' => [[30, 10, 20], 30], + ]; + } + }