Split up ForumManager to create ForumIndexStorage

ForumRepository? Why not the forum storage controller??

Well, we're renaming FooStorageController to FooStorage, so that would be the same conflict :-/

#2188613: Rename EntityStorageController to EntityStorage

Sure.

1. +++ b/core/modules/forum/lib/Drupal/forum/ForumRepository.php
   @@ -0,0 +1,170 @@
   +  public function updateIndex(NodeInterface $node) {
   ...
   +    $count = $this->database->query("SELECT COUNT(cid) FROM {comment} c INNER JOIN {forum_index} i ON c.entity_id = i.nid WHERE c.entity_id = :nid AND c.field_id = 'node__comment_forum' AND c.entity_type = 'node' AND c.status = :status", array(
   ...
   +    if ($count > 0) {
   +      // Comments exist.
   +      $last_reply = $this->database->queryRange("SELECT cid, name, created, uid FROM {comment} WHERE entity_id = :nid AND field_id = 'node__comment_forum' AND entity_type = 'node' AND status = :status ORDER BY cid DESC", 0, 1, array(
   ...
   +      $this->database->update('forum_index')
   ...
   +          'comment_count' => $count,
   +          'last_comment_timestamp' => $last_reply->created,
   

   The comment exists, because count>0.
   So second query needs to fetch "created" only

2. +++ b/core/modules/forum/lib/Drupal/forum/ForumRepository.php
   @@ -0,0 +1,170 @@
   +      // Comments do not exist.
   +      // @todo This should be actually filtering on the desired node language
   +      //   and just fall back to the default language.
   +      $node = $this->database->query('SELECT uid, created FROM {node_field_data} WHERE nid = :nid AND default_langcode = 1', array(':nid' => $nid))->fetchObject();
   +      $this->database->update('forum_index')
   ...
   +          'comment_count' => 0,
   +          'last_comment_timestamp' => $node->created,
   

   The $node is already passed as argument, suppose query could be skipped

NW as per #10.

The patch has been rerolled.

continue the work on the reroll

Attached the updated patch.

Updated the patch according to comment #10

Removed deprecated comment.

This issue is a part of #2068325: [META] Convert entity SQL queries to the Entity Query API
Suppose that's should be enough because all queries here are encapsulated in single service and the most of them used to change data.
Queries that fetches data are highly optimized and does not need any alter for translation

The issue summary says that ForumManagerInterface is removed - does not appear to be.

I guess the plan wrt to change notices is to expand https://drupal.org/node/2079767?

+++ b/core/modules/forum/lib/Drupal/forum/ForumManager.php
@@ -513,44 +513,6 @@ public function unreadTopics($term, $uid) {
-      ':status' => CommentInterface::PUBLISHED,

Can remove the use ... CommentInterface from the manager.

rtbc again

+++ b/core/modules/forum/lib/Drupal/forum/ForumManagerInterface.php
@@ -96,12 +96,4 @@ public function checkNodeType(NodeInterface $node);
   public function unreadTopics($term, $uid);

+++ b/core/modules/forum/lib/Drupal/forum/ForumRepositoryInterface.php
@@ -0,0 +1,94 @@
+ * Handles CRUD operations to {forum_index} table.
...
+interface ForumRepositoryInterface {

Naming things is hard, indeed. :) I have no idea what a forum repository is. If it's not ForumStorage because it's not an entity storage handler... can we give it a longer and more descriptive name? ForumIndexSomething would be better since that makes it clear it manages the {forum_index} table.

so "naming" resolved

FWIW and referring to #2/#3:
I am going to assume that the suffix 'Storage' is not off limits for anyone creating their non-entity storage.

Which I agree with - and which is a larger thing than just this issue. I mean why would entities monopolize the term 'Storage'? Are we going to forbid e.g. metatag.module to call a class MetatagStorage?

(...so this class could have been named ForumStorage too :p but I don't mind one way or the other.)

Just noting this, as the guy who is probably going to take a stab at creating TrackerStorage...

Re: #36: Well the thing is that it's not the actual forum storage... it's a lookup table. :) So ForumIndexStorage is indeed a clear/correct name.

The change record https://drupal.org/node/2079767 does not have a reference to this issue yet; should it? Will it need an update?

Thanks @larowlan. I've edited https://drupal.org/node/2079767 to add a reference to this issue.

re-roll

proper merge

+++ b/core/modules/forum/forum.services.yml
@@ -12,3 +12,6 @@ services:
+  forum.repository:
+    class: Drupal\forum\ForumIndexStorage
+    arguments: ['@database', '@forum_manager']

But we're still calling the service forum.repository? In my mind this should forum.index_storage

Reroll to PSR-4 and fixed service name.

Looks good if green.

Committed 1b34836 and pushed to 8.x. Thanks!