From a0fc0b0d0ba4fe0ba8e347a511deb8a2f344a38d Mon Sep 17 00:00:00 2001 From: xjm Date: Wed, 20 Jul 2016 13:14:24 -0500 Subject: [PATCH] Issue #2746033 by amateescu, mohit_aghera, kamalrajsahu21, dawehner, xjm, catch, rfay: NodeController::revisionOverview() does not have a pager, which results in unlimited queries --- .../node/src/Controller/NodeController.php | 29 +++++++-- .../node/src/Tests/NodeRevisionsAllTest.php | 61 +++++++++++++++---- 2 files changed, 74 insertions(+), 16 deletions(-) diff --git a/core/modules/node/src/Controller/NodeController.php b/core/modules/node/src/Controller/NodeController.php index aeb25dbddd8..89a6d7e435e 100644 --- a/core/modules/node/src/Controller/NodeController.php +++ b/core/modules/node/src/Controller/NodeController.php @@ -8,6 +8,7 @@ use Drupal\Core\Datetime\DateFormatterInterface; use Drupal\Core\DependencyInjection\ContainerInjectionInterface; use Drupal\Core\Render\RendererInterface; use Drupal\Core\Url; +use Drupal\node\NodeStorageInterface; use Drupal\node\NodeTypeInterface; use Drupal\node\NodeInterface; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -170,12 +171,9 @@ class NodeController extends ControllerBase implements ContainerInjectionInterfa $delete_permission = (($account->hasPermission("delete $type revisions") || $account->hasPermission('delete all revisions') || $account->hasPermission('administer nodes')) && $node->access('delete')); $rows = array(); - - $vids = $node_storage->revisionIds($node); - $default_revision = $node->getRevisionId(); - foreach (array_reverse($vids) as $vid) { + foreach ($this->getRevisionIds($node, $node_storage) as $vid) { /** @var \Drupal\node\NodeInterface $revision */ $revision = $node_storage->loadRevision($vid); // Only show revisions that are affected by the language that is being @@ -265,6 +263,8 @@ class NodeController extends ControllerBase implements ContainerInjectionInterfa '#attributes' => ['class' => 'node-revision-table'], ); + $build['pager'] = array('#type' => 'pager'); + return $build; } @@ -281,4 +281,25 @@ class NodeController extends ControllerBase implements ContainerInjectionInterfa return $this->t('Create @name', array('@name' => $node_type->label())); } + /** + * Gets a list of node revision IDs for a specific node. + * + * @param \Drupal\node\NodeInterface + * The node entity. + * @param \Drupal\node\NodeStorageInterface $node_storage + * The node storage handler. + * + * @return int[] + * Node revision IDs (in descending order). + */ + protected function getRevisionIds(NodeInterface $node, NodeStorageInterface $node_storage) { + $result = $node_storage->getQuery() + ->allRevisions() + ->condition($node->getEntityType()->getKey('id'), $node->id()) + ->sort($node->getEntityType()->getKey('revision'), 'DESC') + ->pager(50) + ->execute(); + return array_keys($result); + } + } diff --git a/core/modules/node/src/Tests/NodeRevisionsAllTest.php b/core/modules/node/src/Tests/NodeRevisionsAllTest.php index 582ca5bef7c..7963aa039ae 100644 --- a/core/modules/node/src/Tests/NodeRevisionsAllTest.php +++ b/core/modules/node/src/Tests/NodeRevisionsAllTest.php @@ -2,6 +2,8 @@ namespace Drupal\node\Tests; +use Drupal\node\NodeInterface; + /** * Create a node with revisions and test viewing, saving, reverting, and * deleting revisions for user with access to all. @@ -15,7 +17,7 @@ class NodeRevisionsAllTest extends NodeTestBase { protected function setUp() { parent::setUp(); - $node_storage = $this->container->get('entity.manager')->getStorage('node'); + // Create and log in user. $web_user = $this->drupalCreateUser( array( @@ -45,17 +47,7 @@ class NodeRevisionsAllTest extends NodeTestBase { for ($i = 0; $i < $revision_count; $i++) { $logs[] = $node->revision_log = $this->randomMachineName(32); - // Create revision with a random title and body and update variables. - $node->title = $this->randomMachineName(); - $node->body = array( - 'value' => $this->randomMachineName(32), - 'format' => filter_default_format(), - ); - $node->setNewRevision(); - $node->save(); - - $node_storage->resetCache(array($node->id())); - $node = $node_storage->load($node->id()); // Make sure we get revision information. + $node = $this->createNodeRevision($node); $nodes[] = clone $node; } @@ -63,6 +55,28 @@ class NodeRevisionsAllTest extends NodeTestBase { $this->revisionLogs = $logs; } + /** + * Creates a new revision for a given node. + * + * @param \Drupal\node\NodeInterface $node + * A node object. + * + * @return \Drupal\node\NodeInterface + * A node object with up to date revision information. + */ + protected function createNodeRevision(NodeInterface $node) { + // Create revision with a random title and body and update variables. + $node->title = $this->randomMachineName(); + $node->body = array( + 'value' => $this->randomMachineName(32), + 'format' => filter_default_format(), + ); + $node->setNewRevision(); + $node->save(); + + return $node; + } + /** * Checks node revision operations. */ @@ -145,6 +159,29 @@ class NodeRevisionsAllTest extends NodeTestBase { '%title' => $nodes[2]->getTitle(), '%revision-date' => format_date($old_revision_date), ))); + + // Create 50 more revisions in order to trigger paging on the revisions + // overview screen. + $node = $nodes[0]; + for ($i = 0; $i < 50; $i++) { + $logs[] = $node->revision_log = $this->randomMachineName(32); + + $node = $this->createNodeRevision($node); + $nodes[] = clone $node; + } + + $this->drupalGet('node/' . $node->id() . '/revisions'); + + // Check that the pager exists. + $this->assertRaw('page=1'); + + // Check that the last revision is displayed on the first page. + $this->assertText(end($logs)); + + // Go to the second page and check that one of the initial three revisions + // is displayed. + $this->clickLink(t('Page 2')); + $this->assertText($logs[2]); } }