From ae319dd75322c5503b1649bf70710b79a789891e Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Mon, 22 Sep 2014 15:33:42 +0100 Subject: [PATCH] Issue #2225283 by Primsi, martin107, slashrsm | fgm: Make Book storage independent. --- core/modules/book/book.services.yml | 6 +- core/modules/book/src/BookManager.php | 157 ++++---------- core/modules/book/src/BookOutlineStorage.php | 193 ++++++++++++++++++ .../book/src/BookOutlineStorageInterface.php | 156 ++++++++++++++ .../src/Plugin/Block/BookNavigationBlock.php | 8 +- .../book/tests/src/Unit/BookManagerTest.php | 20 +- 6 files changed, 402 insertions(+), 138 deletions(-) create mode 100644 core/modules/book/src/BookOutlineStorage.php create mode 100644 core/modules/book/src/BookOutlineStorageInterface.php diff --git a/core/modules/book/book.services.yml b/core/modules/book/book.services.yml index b86f44f61475..ce5f3cd2fcbe 100644 --- a/core/modules/book/book.services.yml +++ b/core/modules/book/book.services.yml @@ -6,7 +6,7 @@ services: - { name: breadcrumb_builder, priority: 701 } book.manager: class: Drupal\book\BookManager - arguments: ['@database', '@entity.manager', '@string_translation', '@config.factory'] + arguments: ['@entity.manager', '@string_translation', '@config.factory', '@book.outline_storage'] tags: - { name: backend_overridable } book.outline: @@ -15,7 +15,9 @@ services: book.export: class: Drupal\book\BookExport arguments: ['@entity.manager', '@book.manager'] - + book.outline_storage: + class: Drupal\book\BookOutlineStorage + arguments: ['@database'] access_check.book.removable: class: Drupal\book\Access\BookNodeIsRemovableAccessCheck arguments: ['@book.manager'] diff --git a/core/modules/book/src/BookManager.php b/core/modules/book/src/BookManager.php index ad906ed7cbc9..0035cd34f093 100644 --- a/core/modules/book/src/BookManager.php +++ b/core/modules/book/src/BookManager.php @@ -9,7 +9,6 @@ use Drupal\Component\Utility\Unicode; use Drupal\Core\Cache\Cache; -use Drupal\Core\Database\Connection; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Session\AccountInterface; @@ -29,13 +28,6 @@ class BookManager implements BookManagerInterface { */ const BOOK_MAX_DEPTH = 9; - /** - * Database Service Object. - * - * @var \Drupal\Core\Database\Connection - */ - protected $connection; - /** * Entity manager Service Object. * @@ -57,6 +49,13 @@ class BookManager implements BookManagerInterface { */ protected $books; + /** + * Book outline storage. + * + * @var \Drupal\book\BookOutlineStorageInterface + */ + protected $bookOutlineStorage; + /** * Stores flattened book trees. * @@ -67,11 +66,11 @@ class BookManager implements BookManagerInterface { /** * Constructs a BookManager object. */ - public function __construct(Connection $connection, EntityManagerInterface $entity_manager, TranslationInterface $translation, ConfigFactoryInterface $config_factory) { - $this->connection = $connection; + public function __construct(EntityManagerInterface $entity_manager, TranslationInterface $translation, ConfigFactoryInterface $config_factory, BookOutlineStorageInterface $book_outline_storage) { $this->entityManager = $entity_manager; $this->stringTranslation = $translation; $this->configFactory = $config_factory; + $this->bookOutlineStorage = $book_outline_storage; } /** @@ -89,16 +88,10 @@ public function getAllBooks() { */ protected function loadBooks() { $this->books = array(); - $nids = $this->connection->query("SELECT DISTINCT(bid) FROM {book}")->fetchCol(); + $nids = $this->bookOutlineStorage->getBooks(); if ($nids) { - $query = $this->connection->select('book', 'b', array('fetch' => \PDO::FETCH_ASSOC)); - $query->fields('b'); - $query->condition('b.nid', $nids); - $query->addTag('node_access'); - $query->addMetaData('base_table', 'book'); - $book_links = $query->execute(); - + $book_links = $this->bookOutlineStorage->loadMultiple($nids); $nodes = $this->entityManager->getStorage('node')->loadMultiple($nids); // @todo: Sort by weight and translated title. @@ -148,21 +141,7 @@ public function getParentDepthLimit(array $book_link) { * the passed book link. */ protected function findChildrenRelativeDepth(array $book_link) { - $query = $this->connection->select('book'); - $query->addField('book', 'depth'); - $query->condition('bid', $book_link['bid']); - $query->orderBy('depth', 'DESC'); - $query->range(0, 1); - - $i = 1; - $p = 'p1'; - while ($i <= static::BOOK_MAX_DEPTH && $book_link[$p]) { - $query->condition($p, $book_link[$p]); - $p = 'p' . ++$i; - } - - $max_depth = $query->execute()->fetchField(); - + $max_depth = $this->bookOutlineStorage->getChildRelativeDepth($book_link, static::BOOK_MAX_DEPTH); return ($max_depth > $book_link['depth']) ? $max_depth - $book_link['depth'] : 0; } @@ -436,14 +415,12 @@ public function getTableOfContents($bid, $depth_limit, array $exclude = array()) */ public function deleteFromBook($nid) { $original = $this->loadBookLink($nid, FALSE); - $this->connection->delete('book') - ->condition('nid', $nid) - ->execute(); + $this->bookOutlineStorage->delete($nid); + if ($nid == $original['bid']) { // Handle deletion of a top-level post. - $result = $this->connection->query("SELECT * FROM {book} WHERE pid = :nid", array( - ':nid' => $nid - ))->fetchAllAssoc('nid', \PDO::FETCH_ASSOC); + $result = $this->bookOutlineStorage->loadBookChildren($nid); + foreach ($result as $child) { $child['bid'] = $child['nid']; $this->updateOutline($child); @@ -630,32 +607,11 @@ protected function doBookTreeBuild($bid, array $parameters = array()) { } if (!isset($trees[$tree_cid])) { - $query = $this->connection->select('book'); - $query->fields('book'); - for ($i = 1; $i <= static::BOOK_MAX_DEPTH; $i++) { - $query->orderBy('p' . $i, 'ASC'); - } - $query->condition('bid', $bid); - if (!empty($parameters['expanded'])) { - $query->condition('pid', $parameters['expanded'], 'IN'); - } $min_depth = (isset($parameters['min_depth']) ? $parameters['min_depth'] : 1); - if ($min_depth != 1) { - $query->condition('depth', $min_depth, '>='); - } - if (isset($parameters['max_depth'])) { - $query->condition('depth', $parameters['max_depth'], '<='); - } - // Add custom query conditions, if any were passed. - if (isset($parameters['conditions'])) { - foreach ($parameters['conditions'] as $column => $value) { - $query->condition($column, $value); - } - } + $result = $this->bookOutlineStorage->getBookMenuTree($bid, $parameters, $min_depth, static::BOOK_MAX_DEPTH); // Build an ordered array of links using the query result object. $links = array(); - $result = $query->execute(); foreach ($result as $link) { $link = (array) $link; $links[$link['nid']] = $link; @@ -734,7 +690,7 @@ public function loadBookLink($nid, $translate = TRUE) { * {@inheritdoc} */ public function loadBookLinks($nids, $translate = TRUE) { - $result = $this->connection->query("SELECT * FROM {book} WHERE nid IN (:nids)", array(':nids' => $nids), array('fetch' => \PDO::FETCH_ASSOC)); + $result = $this->bookOutlineStorage->loadMultiple($nids); $links = array(); foreach ($result as $link) { if ($translate) { @@ -755,14 +711,9 @@ public function saveBookLink(array $link, $new) { $link += $this->getLinkDefaults($link['nid']); if ($new) { // Insert new. - $this->connection->insert('book') - ->fields(array( - 'nid' => $link['nid'], - 'bid' => $link['bid'], - 'pid' => $link['pid'], - 'weight' => $link['weight'], - ) + $this->getBookParents($link, (array) $this->loadBookLink($link['pid'], FALSE))) - ->execute(); + $parents = $this->getBookParents($link, (array) $this->loadBookLink($link['pid'], FALSE)); + $this->bookOutlineStorage->insert($link, $parents); + // Update the has_children status of the parent. $this->updateParent($link); } @@ -797,10 +748,11 @@ public function saveBookLink(array $link, $new) { $this->updateParent($link); } // Update the weight and pid. - $query = $this->connection->update('book'); - $query->fields(array('weight' => $link['weight'], 'pid' => $link['pid'], 'bid' => $link['bid'])); - $query->condition('nid', $link['nid']); - $query->execute(); + $this->bookOutlineStorage->update($link['nid'], array( + 'weight' => $link['weight'], + 'pid' => $link['pid'], + 'bid' => $link['bid'], + )); } foreach ($affected_bids as $bid) { \Drupal::cache('data')->deleteTags(array('bid' => $bid)); @@ -817,10 +769,6 @@ public function saveBookLink(array $link, $new) { * The original parent of $link. */ protected function moveChildren(array $link, array $original) { - $query = $this->connection->update('book'); - - $query->fields(array('bid' => $link['bid'])); - $p = 'p1'; $expressions = array(); for ($i = 1; $i <= $link['depth']; $p = 'p' . ++$i) { @@ -842,18 +790,8 @@ protected function moveChildren(array $link, array $original) { // @see http://dev.mysql.com/doc/refman/5.0/en/update.html $expressions = array_reverse($expressions); } - foreach ($expressions as $expression) { - $query->expression($expression[0], $expression[1], $expression[2]); - } - - $query->expression('depth', 'depth + :depth', array(':depth' => $shift)); - $query->condition('bid', $original['bid']); - $p = 'p1'; - for ($i = 1; !empty($original[$p]); $p = 'p' . ++$i) { - $query->condition($p, $original[$p]); - } - $query->execute(); + $this->bookOutlineStorage->updateMovedChildren($link['bid'], $original, $expressions, $shift); } /** @@ -875,10 +813,7 @@ protected function updateParent(array $link) { // Nothing to update. return TRUE; } - $query = $this->connection->update('book'); - $query->fields(array('has_children' => 1)) - ->condition('nid', $link['pid']); - return $query->execute(); + return $this->bookOutlineStorage->update($link['pid'], array('has_children' => 1)); } /** @@ -901,22 +836,13 @@ protected function updateOriginalParent(array $original) { return TRUE; } // Check if $original had at least one child. - $original_number_of_children = $this->connection->select('book', 'b') - ->condition('bid', $original['bid']) - ->condition('pid', $original['pid']) - ->condition('nid', $original['nid'], '<>') - ->countQuery() - ->execute() - ->fetchField(); + $original_number_of_children = $this->bookOutlineStorage->countOriginalLinkChildren($original); $parent_has_children = ((bool) $original_number_of_children) ? 1 : 0; // Update the parent. If the original link did not have children, then the // parent now does not have children. If the original had children, then the // the parent has children now (still). - $query = $this->connection->update('book'); - $query->fields(array('has_children' => $parent_has_children)) - ->condition('nid', $original['pid']); - return $query->execute(); + return $this->bookOutlineStorage->update($original['pid'], array('has_children' => $parent_has_children)); } /** @@ -949,15 +875,14 @@ public function bookTreeCheckAccess(&$tree, $node_links = array()) { if ($node_links) { // @todo Extract that into its own method. $nids = array_keys($node_links); - $select = $this->connection->select('node_field_data', 'n'); - $select->addField('n', 'nid'); + // @todo This should be actually filtering on the desired node status field // language and just fall back to the default language. - $select->condition('n.status', 1); + $nids = \Drupal::entityQuery('node') + ->condition('nid', $nids) + ->condition('status', 1) + ->execute(); - $select->condition('n.nid', $nids, 'IN'); - $select->addTag('node_access'); - $nids = $select->execute()->fetchCol(); foreach ($nids as $nid) { foreach ($node_links[$nid] as $mlid => $link) { $node_links[$nid][$mlid]['access'] = TRUE; @@ -1103,17 +1028,9 @@ public function bookSubtreeData($link) { // If the subtree data was not in the cache, $data will be NULL. if (!isset($data)) { - $query = db_select('book', 'b', array('fetch' => \PDO::FETCH_ASSOC)); - $query->fields('b'); - $query->condition('b.bid', $link['bid']); - for ($i = 1; $i <= static::BOOK_MAX_DEPTH && $link["p$i"]; ++$i) { - $query->condition("p$i", $link["p$i"]); - } - for ($i = 1; $i <= static::BOOK_MAX_DEPTH; ++$i) { - $query->orderBy("p$i"); - } + $result = $this->bookOutlineStorage->getBookSubtree($link, static::BOOK_MAX_DEPTH); $links = array(); - foreach ($query->execute() as $item) { + foreach ($result as $item) { $links[] = $item; } $data['tree'] = $this->buildBookOutlineData($links, array(), $link['depth']); diff --git a/core/modules/book/src/BookOutlineStorage.php b/core/modules/book/src/BookOutlineStorage.php new file mode 100644 index 000000000000..cfc281a94308 --- /dev/null +++ b/core/modules/book/src/BookOutlineStorage.php @@ -0,0 +1,193 @@ +<?php + +/** + * @file + * Definition of Drupal\book\BookOutlineStorage. + */ + +namespace Drupal\book; + +use Drupal\Core\Database\Connection; + +/** + * Defines a storage class for books outline. + */ +class BookOutlineStorage implements BookOutlineStorageInterface { + + /** + * Database Service Object. + * + * @var \Drupal\Core\Database\Connection + */ + protected $connection; + + /** + * Constructs a BookOutlineStorage object. + */ + public function __construct(Connection $connection) { + $this->connection = $connection; + } + + /** + * {@inheritdoc} + */ + public function getBooks() { + return $this->connection->query("SELECT DISTINCT(bid) FROM {book}")->fetchCol(); + } + + /** + * {@inheritdoc} + */ + public function loadMultiple($nids) { + $query = $this->connection->select('book', 'b', array('fetch' => \PDO::FETCH_ASSOC)); + $query->fields('b'); + $query->condition('b.nid', $nids); + $query->addTag('node_access'); + $query->addMetaData('base_table', 'book'); + return $query->execute(); + } + + /** + * {@inheritdoc} + */ + public function getChildRelativeDepth($book_link, $max_depth) { + $query = $this->connection->select('book'); + $query->addField('book', 'depth'); + $query->condition('bid', $book_link['bid']); + $query->orderBy('depth', 'DESC'); + $query->range(0, 1); + + $i = 1; + $p = 'p1'; + while ($i <= $max_depth && $book_link[$p]) { + $query->condition($p, $book_link[$p]); + $p = 'p' . ++$i; + } + + return $query->execute()->fetchField(); + } + + /** + * {@inheritdoc} + */ + public function delete($nid) { + return $this->connection->delete('book') + ->condition('nid', $nid) + ->execute(); + } + + /** + * {@inheritdoc} + */ + public function loadBookChildren($pid) { + return $this->connection + ->query("SELECT * FROM {book} WHERE pid = :pid", array(':pid' => $pid)) + ->fetchAllAssoc('nid', \PDO::FETCH_ASSOC); + } + + /** + * {@inheritdoc} + */ + public function getBookMenuTree($bid, $parameters, $min_depth, $max_depth) { + $query = $this->connection->select('book'); + $query->fields('book'); + for ($i = 1; $i <= $max_depth; $i++) { + $query->orderBy('p' . $i, 'ASC'); + } + $query->condition('bid', $bid); + if (!empty($parameters['expanded'])) { + $query->condition('pid', $parameters['expanded'], 'IN'); + } + if ($min_depth != 1) { + $query->condition('depth', $min_depth, '>='); + } + if (isset($parameters['max_depth'])) { + $query->condition('depth', $parameters['max_depth'], '<='); + } + // Add custom query conditions, if any were passed. + if (isset($parameters['conditions'])) { + foreach ($parameters['conditions'] as $column => $value) { + $query->condition($column, $value); + } + } + + return $query->execute(); + } + + /** + * {@inheritdoc} + */ + public function insert($link, $parents) { + return $this->connection + ->insert('book') + ->fields(array( + 'nid' => $link['nid'], + 'bid' => $link['bid'], + 'pid' => $link['pid'], + 'weight' => $link['weight'], + ) + $parents + ) + ->execute(); + } + + /** + * {@inheritdoc} + */ + public function update($nid, $fields) { + return $this->connection + ->update('book') + ->fields($fields) + ->condition('nid', $nid) + ->execute(); + } + + /** + * {@inheritdoc} + */ + public function updateMovedChildren($bid, $original, $expressions, $shift) { + $query = $this->connection->update('book'); + $query->fields(array('bid' => $bid)); + + foreach ($expressions as $expression) { + $query->expression($expression[0], $expression[1], $expression[2]); + } + + $query->expression('depth', 'depth + :depth', array(':depth' => $shift)); + $query->condition('bid', $original['bid']); + $p = 'p1'; + for ($i = 1; !empty($original[$p]); $p = 'p' . ++$i) { + $query->condition($p, $original[$p]); + } + + return $query->execute(); + } + + /** + * {@inheritdoc} + */ + public function countOriginalLinkChildren($original) { + return $this->connection->select('book', 'b') + ->condition('bid', $original['bid']) + ->condition('pid', $original['pid']) + ->condition('nid', $original['nid'], '<>') + ->countQuery() + ->execute()->fetchField(); + } + + /** + * {@inheritdoc} + */ + public function getBookSubtree($link, $max_depth) { + $query = db_select('book', 'b', array('fetch' => \PDO::FETCH_ASSOC)); + $query->fields('b'); + $query->condition('b.bid', $link['bid']); + + for ($i = 1; $i <= $max_depth && $link["p$i"]; ++$i) { + $query->condition("p$i", $link["p$i"]); + } + for ($i = 1; $i <= $max_depth; ++$i) { + $query->orderBy("p$i"); + } + return $query->execute(); + } +} diff --git a/core/modules/book/src/BookOutlineStorageInterface.php b/core/modules/book/src/BookOutlineStorageInterface.php new file mode 100644 index 000000000000..8a27592057c2 --- /dev/null +++ b/core/modules/book/src/BookOutlineStorageInterface.php @@ -0,0 +1,156 @@ +<?php + +/** + * @file + * Contains \Drupal\book\BookOutlineStorageInterface. +*/ + +namespace Drupal\book; + +/** + * Defines a common interface for book outline storage classes. + */ +interface BookOutlineStorageInterface { + + /** + * Gets books (the highest positioned book links). + * + * @return array + * An array of book IDs. + */ + public function getBooks(); + + /** + * Loads books. + * + * @param array $nids + * An array of node IDs. + * + * @return array + * Array of loaded book items. + */ + public function loadMultiple($nids); + + /** + * Gets child relative depth. + * + * @param array $book_link + * The book link. + * + * @param int $max_depth + * The maximum supported depth of the book tree. + * + * @return int + * The depth of the searched book. + */ + public function getChildRelativeDepth($book_link, $max_depth); + + /** + * Deletes a book entry. + * + * @param int $nid + * Deletes a book entry. + * + * @return mixed + * Number of deleted book entries. + */ + public function delete($nid); + + /** + * Loads book's children using it's parent ID. + * + * @param int $pid + * The book's parent ID. + * + * @return array + * Array of loaded book items. + */ + public function loadBookChildren($pid); + + /** + * Builds tree data used for the menu tree. + * + * @param int $bid + * The ID of the book that we are building the tree for. + * @param array $parameters + * An associative array of build parameters. For info about individual + * parameters see BookManager::bookTreeBuild(). + * @param int $min_depth + * The minimum depth of book links in the resulting tree. + * @param int $max_depth + * The maximum supported depth of the book tree. + * + * @return array + * Array of loaded book links. + */ + public function getBookMenuTree($bid, $parameters, $min_depth, $max_depth); + + /** + * Inserts a book link. + * + * @param array $link + * The link array to be inserted in the database. + * @param array $parents + * The array of parent ids for the link to be inserted. + * + * @return mixed + * The last insert ID of the query, if one exists. + */ + public function insert($link, $parents); + + + /** + * Updates book reference for links that were moved between books. + * + * @param int $nid + * The nid of the book entry to be updated. + * @param array $fields + * The array of fields to be updated. + * + * @return mixed + * The number of rows matched by the update query. + */ + public function update($nid, $fields); + + /** + * Update the book ID of the book link that it's being moved. + * + * @param int $bid + * The ID of the book whose children we move. + * @param array $original + * The original parent of the book link. + * @param array $expressions + * Array of expressions to be added to the query. + * @param int $shift + * The difference in depth between the old and the new position of the + * element being moved. + * + * @return mixed + * The number of rows matched by the update query. + */ + public function updateMovedChildren($bid, $original, $expressions, $shift); + + /** + * Count the number of original link children. + * + * @param array $original + * The book link array. + * + * @return int + * Number of children. + */ + public function countOriginalLinkChildren($original); + + /** + * Get book subtree. + * + * @param array $link + * A fully loaded book link. + * @param int $max_depth + * The maximum supported depth of the book tree. + * + * @return array + * Array of unordered subtree book items. + */ + public function getBookSubtree($link, $max_depth); +} diff --git a/core/modules/book/src/Plugin/Block/BookNavigationBlock.php b/core/modules/book/src/Plugin/Block/BookNavigationBlock.php index 441d05255623..4c2fd14dc758 100644 --- a/core/modules/book/src/Plugin/Block/BookNavigationBlock.php +++ b/core/modules/book/src/Plugin/Block/BookNavigationBlock.php @@ -149,11 +149,9 @@ public function build() { } elseif ($current_bid) { // Only display this block when the user is browsing a book. - $select = db_select('node', 'n') - ->fields('n', array('nid')) - ->condition('n.nid', $node->book['bid']) - ->addTag('node_access'); - $nid = $select->execute()->fetchField(); + $query = \Drupal::entityQuery('node'); + $nid = $query->condition('nid', $node->book['bid'], '=')->execute(); + // Only show the block if the user has view access for the top-level node. if ($nid) { $tree = $this->bookManager->bookTreeAllData($node->book['bid'], $node->book); diff --git a/core/modules/book/tests/src/Unit/BookManagerTest.php b/core/modules/book/tests/src/Unit/BookManagerTest.php index d43a4a66822b..2c12385c16d8 100644 --- a/core/modules/book/tests/src/Unit/BookManagerTest.php +++ b/core/modules/book/tests/src/Unit/BookManagerTest.php @@ -16,13 +16,6 @@ */ class BookManagerTest extends UnitTestCase { - /** - * The mocked database connection. - * - * @var \Drupal\Core\Database\Connection|\PHPUnit_Framework_MockObject_MockObject - */ - protected $connection; - /** * The mocked entity manager. * @@ -51,17 +44,22 @@ class BookManagerTest extends UnitTestCase { */ protected $bookManager; + /** + * Book outline storage. + * + * @var \Drupal\book\BookOutlineStorageInterface + */ + protected $bookOutlineStorage; + /** * {@inheritdoc} */ protected function setUp() { - $this->connection = $this->getMockBuilder('Drupal\Core\Database\Connection') - ->disableOriginalConstructor() - ->getMock(); $this->entityManager = $this->getMock('Drupal\Core\Entity\EntityManagerInterface'); $this->translation = $this->getStringTranslationStub(); $this->configFactory = $this->getConfigFactoryStub(array()); - $this->bookManager = new BookManager($this->connection, $this->entityManager, $this->translation, $this->configFactory); + $this->bookOutlineStorage = $this->getMock('Drupal\book\BookOutlineStorageInterface'); + $this->bookManager = new BookManager($this->entityManager, $this->translation, $this->configFactory, $this->bookOutlineStorage); } /** -- GitLab