diff --git a/core/includes/pager.inc b/core/includes/pager.inc index 80971a6..42ea60c 100644 --- a/core/includes/pager.inc +++ b/core/includes/pager.inc @@ -22,13 +22,13 @@ * displays the third page of search results at that URL. * * @deprecated as of Drupal 8.3.x, will be removed before Drupal 9.0.0. - * Use \Drupal::service('pager.factory')->get($element)->getCurrentPage() + * Use \Drupal::service('pager.factory')->findCurrentPage($element) * instead. * - * @see \Drupal\Core\Pager\PagerInterface::init() + * @see \Drupal\Core\Pager\PagerFactoryInterface::findCurrentPage() */ function pager_find_page($element = 0) { - return \Drupal::service('pager.factory')->get($element)->getCurrentPage(); + return \Drupal::service('pager.factory')->findCurrentPage($element); } /** @@ -54,11 +54,11 @@ function pager_find_page($element = 0) { * The following example shows how this function can be used in a controller * that invokes an external datastore with an SQL-like syntax: * @code - * // First find the total number of items and initialize the pager. + * // First find the total number of items and set the pager. * $where = "status = 1"; * $total = mymodule_select("SELECT COUNT(*) FROM data " . $where)->result(); * $num_per_page = \Drupal::config('mymodule.settings')->get('num_per_page'); - * $page = \Drupal::service('pager.factory')->get(0)->init($total, $num_per_page)->getCurrentPage(); + * $page = \Drupal::service('pager.factory')->set(0, $total, $num_per_page)->getCurrentPage(); * * // Next, retrieve the items for the current page and put them into a * // render array. @@ -78,23 +78,20 @@ function pager_find_page($element = 0) { * A second example involves a controller that invokes an external search * service where the total number of matching results is provided as part of * the returned set (so that we do not need a separate query in order to obtain - * this information). Here, we call pager_find_page() to calculate the desired - * offset before the search is invoked: + * this information). Here, we call PagerFactory::findCurrentPage() to + * calculate the desired offset before the search is invoked: * @code - * // First, get a pager for the element '0'. - * $pager = \Drupal::service('pager.factory')->get(0); - * * // Perform the query, using the requested offset from - * // $pager->getCurrentPage(). This comes from a URL parameter, so here we - * // are assuming that the URL parameter corresponds to an actual page of - * // results that will exist within the set. - * $page = $pager->getCurrentPage(); + * // PagerFactory::findCurrentPage(). This comes from a URL parameter, so + * // here we are assuming that the URL parameter corresponds to an actual + * // page of results that will exist within the set. + * $page = \Drupal::service('pager.factory')->findCurrentPage(0); * $num_per_page = \Drupal::config('mymodule.settings')->get('num_per_page'); * $offset = $num_per_page * $page; * $result = mymodule_remote_search($keywords, $offset, $num_per_page); * - * // Now that we have the total number of results, initialize the pager. - * $pager->init($result->total, $num_per_page); + * // Now that we have the total number of results, set the pager. + * \Drupal::service('pager.factory')->set(0, $result->total, $num_per_page); * * // Create a render array with the search results. * $render = []; @@ -125,13 +122,13 @@ function pager_find_page($element = 0) { * result set. * * @deprecated as of Drupal 8.3.x, will be removed before Drupal 9.0.0. - * Use \Drupal::service('pager.factory')->get($element)->init($total, $limit) - * to initialize the pager, and + * Use \Drupal::service('pager.factory')->set($element, $total, $limit) + * to set the pager, and * \Drupal::service('pager.factory')->get($element)->getCurrentPage() to * retrieve the current page. */ function pager_default_initialize($total, $limit, $element = 0) { - return \Drupal::service('pager.factory')->get($element)->init($total, $limit)->getCurrentPage(); + return \Drupal::service('pager.factory')->set($element, $total, $limit)->getCurrentPage(); } /** @@ -179,11 +176,11 @@ function template_preprocess_pager(&$variables) { $route_name = $variables['pager']['#route_name']; $route_parameters = isset($variables['pager']['#route_parameters']) ? $variables['pager']['#route_parameters'] : []; - // Prepare the pager object. + // Get the pager object. $pager = \Drupal::service('pager.factory')->get($element); // Nothing to do if there is only one page. - if ($pager->getTotalPages() <= 1) { + if ($pager === NULL || $pager->getTotalPages() <= 1) { return; } diff --git a/core/lib/Drupal/Core/Pager/Pager.php b/core/lib/Drupal/Core/Pager/Pager.php index 2acda0a..cc57d49 100644 --- a/core/lib/Drupal/Core/Pager/Pager.php +++ b/core/lib/Drupal/Core/Pager/Pager.php @@ -58,49 +58,29 @@ class Pager implements PagerInterface { /** * Constructs a new Pager object. * - * @param int $element - * The pager element that uniquely identifies this pager object. * @param \Drupal\Core\Pager\PagerFactoryInterface $pager_factory * The pager factory. + * @param int $element + * The pager element that uniquely identifies this pager object. + * @param int $total + * The total number of items to be paged. + * @param int $limit + * The number of items the calling code will display per page. */ - public function __construct($element, PagerFactoryInterface $pager_factory) { - $this->element = $element; + public function __construct(PagerFactoryInterface $pager_factory, $element, $total, $limit) { $this->factory = $pager_factory; + $this->element = $element; // @todo: Start of BC layer. This maps the pager properties to the global // pager variables of D8. When removing the global variables in D9, remove // the following lines. global $pager_page_array, $pager_total, $pager_total_items, $pager_limits; - if (!isset($pager_page_array[$element])) { - $pager_page_array[$element] = NULL; - } $this->currentPage = &$pager_page_array[$element]; - if (!isset($pager_total[$element])) { - $pager_total[$element] = NULL; - } $this->totalPages = &$pager_total[$element]; - if (!isset($pager_total_items[$element])) { - $pager_total_items[$element] = NULL; - } $this->totalItems = &$pager_total_items[$element]; - if (!isset($pager_limits[$element])) { - $pager_limits[$element] = NULL; - } $this->limit = &$pager_limits[$element]; // @todo: End of BC layer. - } - /** - * {@inheritdoc} - */ - public function getElement() { - return $this->element; - } - - /** - * {@inheritdoc} - */ - public function init($total, $limit) { // The total number of pages to be managed by the pager is given by the // total number of items ($total) divided by the number of items in each // page ($limit). We round up to the next integer to accommodate any @@ -110,22 +90,13 @@ public function init($total, $limit) { ->setTotalPages((int) ceil($this->getTotalItems() / $limit)) ->setCurrentPage(max(0, min($this->getCurrentPage(), ((int) $this->getTotalPages() - 1)))) ->setLimit($limit); - return $this; } /** - * Gets the current page (0-index), from the request query parameters. - * - * @return int - * The page to which the pager is currently positioned to. + * {@inheritdoc} */ - protected function getCurrentPageFromRequest() { - $page_query = $this->factory->getCurrentRequestQueryParameter($this->factory->getPagerParameterName()); - $page_array = explode(',', $page_query); - if (!isset($page_array[$this->element])) { - $page_array[$this->element] = 0; - } - return (int) $page_array[$this->element]; + public function getElement() { + return $this->element; } /** @@ -133,15 +104,21 @@ protected function getCurrentPageFromRequest() { */ public function getCurrentPage() { if (!isset($this->currentPage) || $this->currentPage === NULL) { - $this->setCurrentPage($this->getCurrentPageFromRequest()); + $this->setCurrentPage($this->factory->findCurrentPage($this->getElement())); } return $this->currentPage; } /** - * {@inheritdoc} + * Sets the current page for this pager (0-index). + * + * @param int $page + * The page to which the pager is currently positioned to. + * + * @return \Drupal\Core\Pager\PagerInterface + * The Pager object. */ - public function setCurrentPage($page) { + protected function setCurrentPage($page) { $this->currentPage = (int) $page; return $this; } @@ -154,9 +131,15 @@ public function getTotalPages() { } /** - * {@inheritdoc} + * Sets total pages in the pager. + * + * @param int $total_pages + * The total number of pages managed by the pager. + * + * @return \Drupal\Core\Pager\PagerInterface + * The Pager object. */ - public function setTotalPages($total_pages) { + protected function setTotalPages($total_pages) { $this->totalPages = (int) $total_pages; return $this; } @@ -176,9 +159,15 @@ public function getTotalItems() { } /** - * {@inheritdoc} + * Sets total items in the pager. + * + * @param int $total_items + * The total number of items (records) managed by the pager. + * + * @return \Drupal\Core\Pager\PagerInterface + * The Pager object. */ - public function setTotalItems($total_items) { + protected function setTotalItems($total_items) { $this->totalItems = (int) $total_items; return $this; } @@ -191,9 +180,15 @@ public function getLimit() { } /** - * {@inheritdoc} + * Sets the items per page. + * + * @param int $limit + * The number of items (records) in each page. + * + * @return \Drupal\Core\Pager\PagerInterface + * The Pager object. */ - public function setLimit($limit) { + protected function setLimit($limit) { $this->limit = (int) $limit; return $this; } diff --git a/core/lib/Drupal/Core/Pager/PagerFactory.php b/core/lib/Drupal/Core/Pager/PagerFactory.php index 23eb74a..ef6fb2a 100644 --- a/core/lib/Drupal/Core/Pager/PagerFactory.php +++ b/core/lib/Drupal/Core/Pager/PagerFactory.php @@ -39,20 +39,24 @@ public function __construct(RequestStack $request_stack) { * * @param int $element * The pager element. + * @param int $total + * The total number of items to be paged. + * @param int $limit + * The number of items the calling code will display per page. * * @return \Drupal\Core\Pager\PagerInterface * The pager object. */ - protected function create($element) { - return new Pager($element, $this); + protected function create($element, $total, $limit) { + return new Pager($this, $element, $total, $limit); } /** * {@inheritdoc} */ - public function get($element) { + public function set($element, $total, $limit) { if (!isset($this->pagers[$element])) { - $this->pagers[$element] = $this->create($element); + $this->pagers[$element] = $this->create($element, $total, $limit); } return $this->pagers[$element]; } @@ -60,6 +64,20 @@ public function get($element) { /** * {@inheritdoc} */ + public function get($element) { + // @todo: BC layer. When removing the globals, remove the following and + // return $this->pagers directly. + global $pager_total, $pager_total_items, $pager_limits; + if (!isset($this->pagers[$element]) && isset($pager_total[$element])) { + $this->set($element, $pager_total_items[$element], $pager_limits[$element]); + } + // @todo: end of BC layer. + return isset($this->pagers[$element]) ? $this->pagers[$element] : NULL; + } + + /** + * {@inheritdoc} + */ public function getFirstAvailableElement() { // @todo: BC layer. When removing the globals, return element from // max(array_keys($this->pagers)) directly. @@ -73,12 +91,13 @@ public function getFirstAvailableElement() { public function all() { // @todo: BC layer. When removing the globals, remove the following and // return $this->pagers directly. - global $pager_total; + global $pager_total, $pager_total_items, $pager_limits; if (!empty($pager_total) && is_array($pager_total)) { foreach (array_keys($pager_total) as $element) { $this->get($element); } } + // @todo: end of BC layer. return $this->pagers; } @@ -124,4 +143,16 @@ public function getCurrentRequestQueryParameters(array $filter = []) { return $query; } + /** + * {@inheritdoc} + */ + public function findCurrentPage($element) { + $page_query = $this->getCurrentRequestQueryParameter($this->getPagerParameterName()); + $page_array = explode(',', $page_query); + if (!isset($page_array[$element])) { + $page_array[$element] = 0; + } + return (int) $page_array[$element]; + } + } diff --git a/core/lib/Drupal/Core/Pager/PagerFactoryInterface.php b/core/lib/Drupal/Core/Pager/PagerFactoryInterface.php index 0f1e17b..b2c8d31 100644 --- a/core/lib/Drupal/Core/Pager/PagerFactoryInterface.php +++ b/core/lib/Drupal/Core/Pager/PagerFactoryInterface.php @@ -8,12 +8,27 @@ interface PagerFactoryInterface { /** - * Returns the pager object for the specified pager element. + * Sets the pager object for the specified pager element. * * The pager gets created if not already existing. * * @param int $element * The pager element. + * @param int $total + * The total number of items to be paged. + * @param int $limit + * The number of items the calling code will display per page. + * + * @return \Drupal\Core\Pager\Pager + * The pager object. + */ + public function set($element, $total, $limit); + + /** + * Returns the pager object for the specified pager element. + * + * @param int $element + * The pager element. * * @return \Drupal\Core\Pager\Pager * The pager object. @@ -75,4 +90,15 @@ public function getCurrentRequestQueryParameter($parameter); */ public function getCurrentRequestQueryParameters(array $filter = []); + /** + * Finds the current page, from the request query parameters. + * + * @param int $element + * The pager element. + * + * @return int + * The page (0-index) to which the pager is currently positioned to. + */ + public function findCurrentPage($element); + } diff --git a/core/lib/Drupal/Core/Pager/PagerInterface.php b/core/lib/Drupal/Core/Pager/PagerInterface.php index f29f401..73817f5 100644 --- a/core/lib/Drupal/Core/Pager/PagerInterface.php +++ b/core/lib/Drupal/Core/Pager/PagerInterface.php @@ -27,19 +27,6 @@ public function getElement(); /** - * Initializes the pager. - * - * @param int $total - * The total number of items to be paged. - * @param int $limit - * The number of items the calling code will display per page. - * - * @return \Drupal\Core\Pager\PagerInterface - * The Pager object. - */ - public function init($total, $limit); - - /** * Gets the current page (0-index). * * @return int @@ -48,17 +35,6 @@ public function init($total, $limit); public function getCurrentPage(); /** - * Sets the current page for this pager (0-index). - * - * @param int $page - * The page to which the pager is currently positioned to. - * - * @return \Drupal\Core\Pager\PagerInterface - * The Pager object. - */ - public function setCurrentPage($page); - - /** * Gets total pages in the pager. * * @return int @@ -67,17 +43,6 @@ public function setCurrentPage($page); public function getTotalPages(); /** - * Sets total pages in the pager. - * - * @param int $total_pages - * The total number of pages managed by the pager. - * - * @return \Drupal\Core\Pager\PagerInterface - * The Pager object. - */ - public function setTotalPages($total_pages); - - /** * Gets last page in the pager (0-index). * * @return int @@ -94,17 +59,6 @@ public function getLastPage(); public function getTotalItems(); /** - * Sets total items in the pager. - * - * @param int $total_items - * The total number of items (records) managed by the pager. - * - * @return \Drupal\Core\Pager\PagerInterface - * The Pager object. - */ - public function setTotalItems($total_items); - - /** * Gets the items per page. * * @return int @@ -113,17 +67,6 @@ public function setTotalItems($total_items); public function getLimit(); /** - * Sets the items per page. - * - * @param int $limit - * The number of items (records) in each page. - * - * @return \Drupal\Core\Pager\PagerInterface - * The Pager object. - */ - public function setLimit($limit); - - /** * Gets the query parameters for a pager URL. * * @param int $page diff --git a/core/modules/taxonomy/src/Form/OverviewTerms.php b/core/modules/taxonomy/src/Form/OverviewTerms.php index cdc6619..742f9a5 100644 --- a/core/modules/taxonomy/src/Form/OverviewTerms.php +++ b/core/modules/taxonomy/src/Form/OverviewTerms.php @@ -77,7 +77,7 @@ public function getFormId() { public function buildForm(array $form, FormStateInterface $form_state, VocabularyInterface $taxonomy_vocabulary = NULL) { // @todo Remove global variables when https://www.drupal.org/node/2044435 is // in. - global $pager_page_array, $pager_total, $pager_total_items; + global $pager_page_array, $pager_total, $pager_total_items, $pager_limits; $form_state->set(['taxonomy', 'vocabulary'], $taxonomy_vocabulary); $parent_fields = FALSE; @@ -173,6 +173,7 @@ public function buildForm(array $form, FormStateInterface $form_state, Vocabular $pager_total_items[0] = $total_entries; $pager_page_array[0] = $page; $pager_total[0] = ceil($total_entries / $page_increment); + $pager_limits[0] = $page_increment; // If this form was already submitted once, it's probably hit a validation // error. Ensure the form is rebuilt in the same order as the user diff --git a/core/modules/views/views.theme.inc b/core/modules/views/views.theme.inc index 4f5ee62..9fb86dd 100644 --- a/core/modules/views/views.theme.inc +++ b/core/modules/views/views.theme.inc @@ -1018,6 +1018,9 @@ function template_preprocess_views_mini_pager(&$variables) { // Prepare the pager object. $pager = \Drupal::service('pager.factory')->get($element); + if ($pager === NULL || $pager->getTotalPages() <= 1) { + return; + } // Current is the page we are currently paged to. $variables['items']['current'] = $pager->getCurrentPage() + 1; diff --git a/core/tests/Drupal/Tests/Core/Pager/PagerFactoryTest.php b/core/tests/Drupal/Tests/Core/Pager/PagerFactoryTest.php index 587076d..42fcba7 100644 --- a/core/tests/Drupal/Tests/Core/Pager/PagerFactoryTest.php +++ b/core/tests/Drupal/Tests/Core/Pager/PagerFactoryTest.php @@ -37,7 +37,7 @@ protected function setUp() { $pager_page_array = $pager_total = $pager_total_items = $pager_limits = NULL; $request_stack = $this->getMockBuilder(RequestStack::class)->getMock(); - $query = new ParameterBag(['foo' => 'bar', 'qux' => 'dee']); + $query = new ParameterBag(['page' => '10,15', 'foo' => 'bar', 'qux' => 'dee']); $this->pagerFactory = $this ->getMockBuilder(PagerFactory::class) @@ -51,13 +51,29 @@ protected function setUp() { } /** + * @covers ::set + */ + public function testSet() { + // Set pager 0. + $pager = $this->pagerFactory->set(0, 100, 10); + // Pager element should be 0. + $this->assertSame(0, $pager->getElement()); + // Set pager 2. + $pager = $this->pagerFactory->set(2, 200, 10); + // Pager element should be 2. + $this->assertSame(2, $pager->getElement()); + } + + /** * @covers ::get */ public function testGet() { // Get pager 0. $pager = $this->pagerFactory->get(0); - // Pager element should be 0. - $this->assertSame(0, $pager->getElement()); + // Pager should be NULL since not set. + $this->assertNull($pager); + // Set pager 2. + $this->pagerFactory->set(2, 200, 10); // Get pager 2. $pager = $this->pagerFactory->get(2); // Pager element should be 2. @@ -70,12 +86,12 @@ public function testGet() { public function testGetFirstAvailableElement() { // No pagers set yet, first available element should be 0. $this->assertSame(0, $this->pagerFactory->getFirstAvailableElement()); - // Get pager 0. - $pager = $this->pagerFactory->get(0); + // Set pager 0. + $this->pagerFactory->set(0, 100, 10); // Now first available element should be 1. $this->assertSame(1, $this->pagerFactory->getFirstAvailableElement()); - // Get pager 4. - $pager = $this->pagerFactory->get(4); + // Set pager 4. + $this->pagerFactory->set(4, 400, 10); // Now first available element should be 5. $this->assertSame(5, $this->pagerFactory->getFirstAvailableElement()); } @@ -84,12 +100,9 @@ public function testGetFirstAvailableElement() { * @covers ::all */ public function testAll() { - // Get pager 4. - $pager = $this->pagerFactory->get(4); - $this->assertSame(4, $pager->getElement()); - // Get pager 6. - $pager = $this->pagerFactory->get(6); - $this->assertSame(6, $pager->getElement()); + // Set pagers 4 and 6. + $this->pagerFactory->set(4, 400, 10); + $this->pagerFactory->set(6, 600, 10); // We should get two pagers for elements 4 and 6. $this->assertSame(2, count($this->pagerFactory->all())); $this->assertSame([4, 6], array_keys($this->pagerFactory->all())); @@ -106,17 +119,26 @@ public function testGetPagerParameterName() { * @covers ::getCurrentRequestQueryParameters */ public function testGetCurrentRequestQueryParameters() { - $this->assertSame(['foo' => 'bar', 'qux' => 'dee'], $this->pagerFactory->getCurrentRequestQueryParameters()); + $this->assertSame(['page' => '10,15', 'foo' => 'bar', 'qux' => 'dee'], $this->pagerFactory->getCurrentRequestQueryParameters()); } /** * @covers ::getCurrentRequestQueryParameter */ public function testGetCurrentRequestQueryParameter() { + $this->assertSame('10,15', $this->pagerFactory->getCurrentRequestQueryParameter('page')); $this->assertSame('bar', $this->pagerFactory->getCurrentRequestQueryParameter('foo')); $this->assertSame('dee', $this->pagerFactory->getCurrentRequestQueryParameter('qux')); // Missing parameter returns empty string. $this->assertSame('', $this->pagerFactory->getCurrentRequestQueryParameter('zot')); } + /** + * @covers ::findCurrentPage + */ + public function testFindCurrentPage() { + $this->assertSame(10, $this->pagerFactory->findCurrentPage(0)); + $this->assertSame(15, $this->pagerFactory->findCurrentPage(1)); + } + } diff --git a/core/tests/Drupal/Tests/Core/Pager/PagerTest.php b/core/tests/Drupal/Tests/Core/Pager/PagerTest.php index aa3e77f..3dd2403 100644 --- a/core/tests/Drupal/Tests/Core/Pager/PagerTest.php +++ b/core/tests/Drupal/Tests/Core/Pager/PagerTest.php @@ -45,13 +45,17 @@ protected function setUp() { $pager_factory->expects($this->any()) ->method('getCurrentRequestQueryParameter') ->will($this->returnValue(',,,,,,11')); + // Set page query parameter current page to 11 (0-indexed). + $pager_factory->expects($this->any()) + ->method('findCurrentPage') + ->will($this->returnValue(11)); // Set other dummy query parameters. $pager_factory->expects($this->any()) ->method('getCurrentRequestQueryParameters') ->will($this->returnValue(['rec' => 'lop', 'ste' => 'cik'])); // Pager for element 6. - $this->pager = new Pager(6, $pager_factory); + $this->pager = new Pager($pager_factory, 6, 513, 28); // Set factory to return the test pager via ::all(). $pager_factory->expects($this->any()) @@ -68,53 +72,33 @@ public function testGetElement() { } /** - * @covers ::init - */ - public function testInit() { - $this->pager->init(513, 28); - $this->assertSame(513, $this->pager->getTotalItems()); - $this->assertSame(19, $this->pager->getTotalPages()); - $this->assertSame(11, $this->pager->getCurrentPage()); - $this->assertSame(28, $this->pager->getLimit()); - } - - /** - * @covers ::setCurrentPage * @covers ::getCurrentPage */ public function testCurrentPage() { $this->assertSame(11, $this->pager->getCurrentPage()); - $this->pager->setCurrentPage(200); - $this->assertSame(200, $this->pager->getCurrentPage()); } /** - * @covers ::setTotalPages * @covers ::getTotalPages * @covers ::getLastPage */ public function testTotalPages() { - $this->pager->setTotalPages(40); - $this->assertSame(40, $this->pager->getTotalPages()); - $this->assertSame(39, $this->pager->getLastPage()); + $this->assertSame(19, $this->pager->getTotalPages()); + $this->assertSame(18, $this->pager->getLastPage()); } /** - * @covers ::setTotalItems * @covers ::getTotalItems */ public function testTotalItems() { - $this->pager->setTotalItems(600); - $this->assertSame(600, $this->pager->getTotalItems()); + $this->assertSame(513, $this->pager->getTotalItems()); } /** - * @covers ::setLimit * @covers ::getLimit */ public function testLimit() { - $this->pager->setLimit(45); - $this->assertSame(45, $this->pager->getLimit()); + $this->assertSame(28, $this->pager->getLimit()); } /**