diff --git a/core/modules/comment/comment.module b/core/modules/comment/comment.module index 6d7e051..a6ae9a2 100644 --- a/core/modules/comment/comment.module +++ b/core/modules/comment/comment.module @@ -70,17 +70,6 @@ const COMMENT_OPEN = 2; /** - * The time cutoff for comments marked as read for entity types other node. - * - * Comments changed before this time are always marked as read. - * Comments changed after this time may be marked new, updated, or read, - * depending on their state for the current user. Defaults to 30 days ago. - * - * @todo Remove when http://drupal.org/node/1029708 lands. - */ -define('COMMENT_NEW_LIMIT', REQUEST_TIME - 30 * 24 * 60 * 60); - -/** * Implements hook_help(). */ function comment_help($path, $arg) { @@ -563,7 +552,7 @@ function comment_entity_view(EntityInterface $entity, EntityViewDisplayInterface // Embed the metadata for the "X new comments" link (if any) on this node. $entity->content['links']['#post_render_cache']['history_attach_timestamp'] = array( - array('node_id' => $entity->id()), + array('entity_type' => $entity->getEntityTypeId(), 'entity_id' => $entity->id()), ); $entity->content['links']['#post_render_cache']['Drupal\comment\CommentViewBuilder::attachNewCommentsLinkMetadata'] = array( array('entity_type' => $entity->getEntityTypeId(), 'entity_id' => $entity->id(), 'field_name' => $field_name), @@ -1156,30 +1145,20 @@ function comment_load($cid, $reset = FALSE) { * Entity type of the entity to which the comments are attached. * @param string $field_name * (optional) The field_name to count comments for. Defaults to NULL. - * @param $timestamp - * Time to count from (defaults to time of last user access to node). + * @param int $timestamp + * (optional) Time to count from (defaults to time of last user access to + * entity). * * @return int|false * The number of new comments or FALSE if the user is not logged in. */ function comment_num_new($entity_id, $entity_type, $field_name = NULL, $timestamp = 0) { - if (\Drupal::currentUser()->isAuthenticated() && \Drupal::moduleHandler()->moduleExists('history')) { + $account = \Drupal::currentUser(); + if ($account->isAuthenticated() && \Drupal::moduleHandler()->moduleExists('history')) { // Retrieve the timestamp at which the current user last viewed this entity. if (!$timestamp) { - if ($entity_type == 'node') { - $timestamp = history_read($entity_id); - } - else { - $function = $entity_type . '_last_viewed'; - if (function_exists($function)) { - $timestamp = $function($entity_id); - } - else { - // Default to 30 days ago. - // @todo Remove once http://drupal.org/node/1029708 lands. - $timestamp = COMMENT_NEW_LIMIT; - } - } + $timestamp = \Drupal::service('history.repository')->getLastViewed($entity_type, array($entity_id), $account); + $timestamp = $timestamp[$entity_id]; } $timestamp = ($timestamp > HISTORY_READ_LIMIT ? $timestamp : HISTORY_READ_LIMIT); @@ -1198,10 +1177,7 @@ function comment_num_new($entity_id, $entity_type, $field_name = NULL, $timestam return $query->execute() ->fetchField(); } - else { - return FALSE; - } - + return FALSE; } /** diff --git a/core/modules/comment/lib/Drupal/comment/CommentViewBuilder.php b/core/modules/comment/lib/Drupal/comment/CommentViewBuilder.php index 53d0ad7..eb39d03 100644 --- a/core/modules/comment/lib/Drupal/comment/CommentViewBuilder.php +++ b/core/modules/comment/lib/Drupal/comment/CommentViewBuilder.php @@ -143,7 +143,7 @@ public function buildContent(array $entities, array $displays, $view_mode, $lang // Embed the metadata for the comment "new" indicators on this node. $entity->content['#post_render_cache']['history_attach_timestamp'] = array( - array('node_id' => $commented_entity->id()), + array('entity_type' => $commented_entity->getEntityTypeId(), 'entity_id' => $commented_entity->id()), ); } } diff --git a/core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.php b/core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.php index 78f81f9..35b1530 100644 --- a/core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.php +++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.php @@ -155,6 +155,10 @@ function setEnvironment(array $info) { )); $comment->save(); $this->comment = $comment; + + // comment_num_new() relies on history_read(), so ensure that no one has + // seen the node of this comment. + \Drupal::service('history.repository')->deleteByEntity($this->node); } else { $cids = db_query("SELECT cid FROM {comment}")->fetchCol(); diff --git a/core/modules/forum/forum.services.yml b/core/modules/forum/forum.services.yml index f27ffc0..e8ec76e 100644 --- a/core/modules/forum/forum.services.yml +++ b/core/modules/forum/forum.services.yml @@ -1,7 +1,7 @@ services: forum_manager: class: Drupal\forum\ForumManager - arguments: ['@config.factory', '@entity.manager', '@database', '@field.info', '@string_translation'] + arguments: ['@config.factory', '@entity.manager', '@database', '@field.info', '@string_translation', '@history.repository'] forum.breadcrumb: class: Drupal\forum\ForumBreadcrumbBuilder arguments: ['@entity.manager', '@config.factory', '@forum_manager'] diff --git a/core/modules/forum/lib/Drupal/forum/ForumManager.php b/core/modules/forum/lib/Drupal/forum/ForumManager.php index 5c530c7..a4e6acd 100644 --- a/core/modules/forum/lib/Drupal/forum/ForumManager.php +++ b/core/modules/forum/lib/Drupal/forum/ForumManager.php @@ -13,6 +13,7 @@ use Drupal\Core\StringTranslation\TranslationInterface; use Drupal\comment\CommentInterface; use Drupal\field\FieldInfo; +use Drupal\history\HistoryRepositoryInterface; use Drupal\node\NodeInterface; /** @@ -111,6 +112,13 @@ class ForumManager implements ForumManagerInterface { protected $translationManager; /** + * The history repository service. + * + * @var \Drupal\history\HistoryRepositoryInterface + */ + protected $historyRepository; + + /** * Constructs the forum manager service. * * @param \Drupal\Core\Config\ConfigFactory $config_factory @@ -123,13 +131,16 @@ class ForumManager implements ForumManagerInterface { * The field info service. * @param \Drupal\Core\StringTranslation\TranslationInterface $translation_manager * The translation manager service. + * @param \Drupal\history\HistoryRepositoryInterface $history_repository + * The history repository service. */ - public function __construct(ConfigFactory $config_factory, EntityManagerInterface $entity_manager, Connection $connection, FieldInfo $field_info, TranslationInterface $translation_manager) { + public function __construct(ConfigFactory $config_factory, EntityManagerInterface $entity_manager, Connection $connection, FieldInfo $field_info, TranslationInterface $translation_manager, HistoryRepositoryInterface $history_repository) { $this->configFactory = $config_factory; $this->entityManager = $entity_manager; $this->connection = $connection; $this->fieldInfo = $field_info; $this->translationManager = $translation_manager; + $this->historyRepository = $history_repository; } /** @@ -238,7 +249,7 @@ public function getTopics($tid) { $topic->new = 0; } else { - $history = $this->lastVisit($topic->id()); + $history = $this->lastVisit($topic->id(), $user); $topic->new_replies = $this->numberNew($topic->id(), $history); $topic->new = $topic->new_replies || ($topic->last_comment_timestamp > $history); } @@ -322,24 +333,23 @@ protected function numberNew($nid, $timestamp) { * * @param int $nid * The node ID. + * @param \Drupal\Core\Session\AccountInterface $account + * The user account to retrieve history for. * * @return int * The timestamp when the user last viewed this node, if the user has * previously viewed the node; otherwise HISTORY_READ_LIMIT. */ - protected function lastVisit($nid) { - $user = \Drupal::currentUser(); - + protected function lastVisit($nid, $account) { + if (isset($this->history[$nid])) { + return $this->history[$nid]; + } + $this->history += $this->historyRepository->getLastViewed('node', array($nid), $account); if (empty($this->history[$nid])) { - $result = $this->connection->select('history', 'h') - ->fields('h', array('nid', 'timestamp')) - ->condition('uid', $user->id()) - ->execute(); - foreach ($result as $t) { - $this->history[$t->nid] = $t->timestamp > HISTORY_READ_LIMIT ? $t->timestamp : HISTORY_READ_LIMIT; - } + $this->history[$nid] = HISTORY_READ_LIMIT; } - return isset($this->history[$nid]) ? $this->history[$nid] : HISTORY_READ_LIMIT; + + return $this->history[$nid]; } /** @@ -499,7 +509,7 @@ public function checkNodeType(NodeInterface $node) { public function unreadTopics($term, $uid) { $query = $this->connection->select('node_field_data', 'n'); $query->join('forum', 'f', 'n.vid = f.vid AND f.tid = :tid', array(':tid' => $term)); - $query->leftJoin('history', 'h', 'n.nid = h.nid AND h.uid = :uid', array(':uid' => $uid)); + $query->leftJoin('history', 'h', "n.nid = h.entity_id AND h.entity_type = 'node' AND h.uid = :uid", array(':uid' => $uid)); $query->addExpression('COUNT(n.nid)', 'count'); return $query ->condition('status', 1) @@ -507,7 +517,7 @@ public function unreadTopics($term, $uid) { // field language and just fall back to the default language. ->condition('n.default_langcode', 1) ->condition('n.created', HISTORY_READ_LIMIT, '>') - ->isNull('h.nid') + ->isNull('h.entity_id') ->addTag('node_access') ->execute() ->fetchField(); diff --git a/core/modules/forum/tests/Drupal/forum/Tests/ForumManagerTest.php b/core/modules/forum/tests/Drupal/forum/Tests/ForumManagerTest.php index 8d3be5f..2672897 100644 --- a/core/modules/forum/tests/Drupal/forum/Tests/ForumManagerTest.php +++ b/core/modules/forum/tests/Drupal/forum/Tests/ForumManagerTest.php @@ -73,12 +73,17 @@ public function testGetIndex() { ->disableOriginalConstructor() ->getMock(); + $history_repository = $this->getMockBuilder('\Drupal\history\HistoryRepository') + ->disableOriginalConstructor() + ->getMock(); + $manager = $this->getMock('\Drupal\forum\ForumManager', array('getChildren'), array( $config_factory, $entity_manager, $connection, $field_info, $translation_manager, + $history_repository )); $manager->expects($this->once()) diff --git a/core/modules/history/history.install b/core/modules/history/history.install index dcfd871..f9ab637 100644 --- a/core/modules/history/history.install +++ b/core/modules/history/history.install @@ -10,16 +10,23 @@ */ function history_schema() { $schema['history'] = array( - 'description' => 'A record of which {users} have read which {node}s.', + 'description' => 'A record of which {users} have read which entities.', 'fields' => array( 'uid' => array( - 'description' => 'The {users}.uid that read the {node} nid.', + 'description' => 'The {users}.uid that read the {history}.entity_id entity.', 'type' => 'int', 'not null' => TRUE, 'default' => 0, ), - 'nid' => array( - 'description' => 'The {node}.nid that was read.', + 'entity_type' => array( + 'description' => 'The type of the entity that was read.', + 'type' => 'varchar', + 'not null' => TRUE, + 'default' => '', + 'length' => 255, + ), + 'entity_id' => array( + 'description' => 'The ID of the entity that was read.', 'type' => 'int', 'not null' => TRUE, 'default' => 0, @@ -31,11 +38,19 @@ function history_schema() { 'default' => 0, ), ), - 'primary key' => array('uid', 'nid'), + 'primary key' => array( + array('entity_type', 32), + 'uid', + 'entity_id', + ), 'indexes' => array( - 'nid' => array('nid'), + 'history_entity' => array( + array('entity_type', 32), + 'entity_id', + ), ), ); return $schema; } + diff --git a/core/modules/history/history.module b/core/modules/history/history.module index 88a60c7..a0dd96c 100644 --- a/core/modules/history/history.module +++ b/core/modules/history/history.module @@ -11,6 +11,7 @@ use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\Display\EntityViewDisplayInterface; +use Drupal\Core\Session\AccountInterface; /** * Entities changed before this time are always shown as read. @@ -41,9 +42,12 @@ function history_help($path, $arg) { * @return int * If a node has been previously viewed by the user, the timestamp in seconds * of when the last view occurred; otherwise, zero. + * + * @deprecated Use \Drupal\history\HistoryRepositoryInterface::getLastViewed() instead. */ function history_read($nid) { - $history = history_read_multiple(array($nid)); + $account = \Drupal::currentUser(); + $history = \Drupal::service('history.repository')->getLastViewed('node', array($nid), $account); return $history[$nid]; } @@ -57,75 +61,38 @@ function history_read($nid) { * Array of timestamps keyed by node ID. If a node has been previously viewed * by the user, the timestamp in seconds of when the last view occurred; * otherwise, zero. + * + * @deprecated Use \Drupal\history\HistoryRepositoryInterface::getLastViewed() instead. */ function history_read_multiple($nids) { - $history = &drupal_static(__FUNCTION__, array()); - - $return = array(); - - $nodes_to_read = array(); - foreach ($nids as $nid) { - if (isset($history[$nid])) { - $return[$nid] = $history[$nid]; - } - else { - // Initialize value if current user has not viewed the node. - $nodes_to_read[$nid] = 0; - } - } - - if (empty($nodes_to_read)) { - return $return; - } - - $result = db_query('SELECT nid, timestamp FROM {history} WHERE uid = :uid AND nid IN(:nids)', array( - ':uid' => \Drupal::currentUser()->id(), - ':nids' => array_keys($nodes_to_read), - )); - foreach ($result as $row) { - $nodes_to_read[$row->nid] = (int) $row->timestamp; - } - $history += $nodes_to_read; - - return $return + $nodes_to_read; + $account = \Drupal::currentUser(); + return \Drupal::service('history.repository')->getLastViewed('node', $nids, $account); } /** * Updates 'last viewed' timestamp of the specified entity for the current user. * - * @param $nid + * @param int $nid * The node ID that has been read. - * @param $account + * @param \Drupal\Core\Session\AccountInterface $account * (optional) The user account to update the history for. Defaults to the * current user. + * + * @deprecated Use \Drupal\history\HistoryRepositoryInterface::updateLastViewed() instead. */ -function history_write($nid, $account = NULL) { - +function history_write($nid, AccountInterface $account = NULL) { if (!isset($account)) { $account = \Drupal::currentUser(); } - - if ($account->isAuthenticated()) { - db_merge('history') - ->key(array( - 'uid' => $account->id(), - 'nid' => $nid, - )) - ->fields(array('timestamp' => REQUEST_TIME)) - ->execute(); - // Update static cache. - $history = &drupal_static('history_read_multiple', array()); - $history[$nid] = REQUEST_TIME; - } + $node = entity_load('node', $nid); + \Drupal::service('history.repository')->updateLastViewed($node, $account); } /** * Implements hook_cron(). */ function history_cron() { - db_delete('history') - ->condition('timestamp', HISTORY_READ_LIMIT, '<') - ->execute(); + \Drupal::service('history.repository')->purge(); } /** @@ -156,9 +123,7 @@ function history_node_view_alter(&$build, EntityInterface $node, EntityViewDispl * Implements hook_node_delete(). */ function history_node_delete(EntityInterface $node) { - db_delete('history') - ->condition('nid', $node->id()) - ->execute(); + \Drupal::service('history.repository')->deleteByEntity($node); } /** @@ -167,9 +132,7 @@ function history_node_delete(EntityInterface $node) { function history_user_cancel($edit, $account, $method) { switch ($method) { case 'user_cancel_reassign': - db_delete('history') - ->condition('uid', $account->id()) - ->execute(); + \Drupal::service('history.repository')->deleteByUser($account); break; } } @@ -178,9 +141,7 @@ function history_user_cancel($edit, $account, $method) { * Implements hook_user_delete(). */ function history_user_delete($account) { - db_delete('history') - ->condition('uid', $account->id()) - ->execute(); + \Drupal::service('history.repository')->deleteByUser($account); } /** @@ -213,18 +174,20 @@ function history_library_info() { * - #attached * @param array $context * An array with the following keys: - * - node_id: the node ID for which to attach the last read timestamp. + * - entity_id: the entity ID for which to attach the last read timestamp. * * @return array $element * The updated $element. */ function history_attach_timestamp(array $element, array $context) { + $account = \Drupal::currentUser(); + $history = \Drupal::service('history.repository')->getLastViewed($context['entity_type'], array($context['entity_id']), $account); $element['#attached']['js'][] = array( 'type' => 'setting', 'data' => array( 'history' => array( 'lastReadTimestamps' => array( - $context['node_id'] => (int) history_read($context['node_id']), + $context['node_id'] => $history[$context['entity_id']], ) ), ), diff --git a/core/modules/history/history.services.yml b/core/modules/history/history.services.yml new file mode 100644 index 0000000..64fcc21 --- /dev/null +++ b/core/modules/history/history.services.yml @@ -0,0 +1,4 @@ +services: + history.repository: + class: Drupal\history\HistoryRepository + arguments: ['@database', '@current_user'] diff --git a/core/modules/history/history.views.inc b/core/modules/history/history.views.inc index 50e12aa..571d7f3 100644 --- a/core/modules/history/history.views.inc +++ b/core/modules/history/history.views.inc @@ -22,8 +22,9 @@ function history_views_data() { 'node' => array( 'table' => 'history', 'left_field' => 'nid', - 'field' => 'nid', + 'field' => 'entity_id', 'extra' => array( + array('field' => 'entity_type', 'value' => 'node'), array('field' => 'uid', 'value' => '***CURRENT_USER***', 'numeric' => TRUE), ), ), diff --git a/core/modules/history/lib/Drupal/history/Controller/HistoryController.php b/core/modules/history/lib/Drupal/history/Controller/HistoryController.php index b605e6e..6d51320 100644 --- a/core/modules/history/lib/Drupal/history/Controller/HistoryController.php +++ b/core/modules/history/lib/Drupal/history/Controller/HistoryController.php @@ -7,11 +7,13 @@ namespace Drupal\history\Controller; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Drupal\Core\Controller\ControllerBase; +use Drupal\history\HistoryRepositoryInterface; use Drupal\node\NodeInterface; /** @@ -20,16 +22,45 @@ class HistoryController extends ControllerBase { /** + * The history repository service. + * + * @var \Drupal\history\HistoryRepositoryInterface + */ + protected $historyRepository; + + /** + * Constructs a HistoryController object. + * + * @param \Drupal\history\HistoryRepositoryInterface $history_repository + * The history repository service. + */ + public function __construct(HistoryRepositoryInterface $history_repository) { + $this->historyRepository = $history_repository; + } + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container) { + return new static( + $container->get('history.repository') + ); + } + + /** * Returns a set of nodes' last read timestamps. * * @param \Symfony\Component\HttpFoundation\Request $request * The request of the page. * - * @return Symfony\Component\HttpFoundation\JsonResponse + * @return \Symfony\Component\HttpFoundation\JsonResponse * The JSON response. + * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException + * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException */ public function getNodeReadTimestamps(Request $request) { - if ($this->currentUser()->isAnonymous()) { + $account = $this->currentUser(); + if ($account->isAnonymous()) { throw new AccessDeniedHttpException(); } @@ -37,7 +68,7 @@ public function getNodeReadTimestamps(Request $request) { if (!isset($nids)) { throw new NotFoundHttpException(); } - return new JsonResponse(history_read_multiple($nids)); + return new JsonResponse($this->historyManager->getLastViewed('node', $nids, $account)); } /** @@ -47,16 +78,22 @@ public function getNodeReadTimestamps(Request $request) { * The request of the page. * @param \Drupal\node\NodeInterface $node * The node whose "last read" timestamp should be updated. + * + * @return \Symfony\Component\HttpFoundation\JsonResponse + * The JSON response. + * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException */ public function readNode(Request $request, NodeInterface $node) { - if ($this->currentUser()->isAnonymous()) { + $account = $this->currentUser(); + if ($account->isAnonymous()) { throw new AccessDeniedHttpException(); } // Update the history table, stating that this user viewed this node. - history_write($node->id()); + $this->historyManager->updateLastViewed($node, $account); - return new JsonResponse((int)history_read($node->id())); + $history = $this->historyManager->getLastViewed('node', array($node->id()), $account); + return new JsonResponse($history[$node->id()]); } } diff --git a/core/modules/history/lib/Drupal/history/HistoryRepository.php b/core/modules/history/lib/Drupal/history/HistoryRepository.php new file mode 100644 index 0000000..e91ba67 --- /dev/null +++ b/core/modules/history/lib/Drupal/history/HistoryRepository.php @@ -0,0 +1,150 @@ +connection = $connection; + $this->currentUser = $current_user; + } + + /** + * {@inheritdoc} + */ + public function getLastViewed($entity_type, $entity_ids, AccountInterface $account) { + $return = array(); + + $entities_to_read = array(); + foreach ($entity_ids as $entity_id) { + if (isset($this->history[$entity_type][$entity_id])) { + $return[$entity_id] = $this->history[$entity_type][$entity_id]; + } + else { + $entities_to_read[$entity_id] = 0; + } + } + + if (empty($entities_to_read)) { + return $return; + } + + $result = $this->connection->select('history', 'h') + ->fields('h', array('entity_id', 'timestamp')) + ->condition('uid', $account->id()) + ->condition('entity_type', $entity_type) + ->condition('entity_id', array_keys($entities_to_read), 'IN') + ->execute(); + + foreach ($result as $row) { + $entities_to_read[$row->entity_id] = (int) $row->timestamp; + } + if (!isset($this->history[$entity_type])) { + $this->history[$entity_type] = array(); + } + $this->history[$entity_type] += $entities_to_read; + + return $return + $entities_to_read; + } + + /** + * {@inheritdoc} + */ + public function updateLastViewed(EntityInterface $entity, AccountInterface $account) { + if ($account->isAuthenticated()) { + $this->connection->merge('history') + ->key(array( + 'uid' => $account->id(), + 'entity_id' => $entity->id(), + 'entity_type' => $entity->getEntityTypeId(), + )) + ->fields(array('timestamp' => REQUEST_TIME)) + ->execute(); + // Update cached value. + $this->history[$entity->getEntityTypeId()][$entity->id()] = REQUEST_TIME; + } + } + + /** + * {@inheritdoc} + */ + public function purge() { + $this->connection->delete('history') + ->condition('timestamp', HISTORY_READ_LIMIT, '<') + ->execute(); + // Clean static cache. + $this->resetCache(); + } + + /** + * {@inheritdoc} + */ + public function deleteByUser(AccountInterface $account) { + $this->connection->delete('history') + ->condition('uid', $account->id()) + ->execute(); + // Clean static cache. + $this->resetCache(); + } + + /** + * {@inheritdoc} + */ + public function deleteByEntity(EntityInterface $entity) { + $this->connection->delete('history') + ->condition('entity_id', $entity->id()) + ->condition('entity_type', $entity->getEntityTypeId()) + ->execute(); + // Clean static cache. + unset($this->history[$entity->getEntityTypeId()][$entity->id()]); + } + + /** + * {@inheritdoc} + */ + public function resetCache() { + $this->history = array(); + } + +} diff --git a/core/modules/history/lib/Drupal/history/HistoryRepositoryInterface.php b/core/modules/history/lib/Drupal/history/HistoryRepositoryInterface.php new file mode 100644 index 0000000..7f9f0dc --- /dev/null +++ b/core/modules/history/lib/Drupal/history/HistoryRepositoryInterface.php @@ -0,0 +1,71 @@ +fields(array( 'uid' => $account->id(), - 'nid' => $nodes[0]->id(), + 'entity_id' => $nodes[0]->id(), + 'entity_type' => 'node', 'timestamp' => REQUEST_TIME - 100, ))->execute(); db_insert('history') ->fields(array( 'uid' => $account->id(), - 'nid' => $nodes[1]->id(), + 'entity_id' => $nodes[1]->id(), + 'entity_type' => 'node', 'timestamp' => REQUEST_TIME + 100, ))->execute();