From e49444d5355dc2e9f707a2db84d51ae4bbcf534d Mon Sep 17 00:00:00 2001
From: Peter <peterrolanddias@gmail.com>
Date: Mon, 29 Apr 2019 13:49:42 +0800
Subject: [PATCH] MDL-65429 mod_forum: Indicate subs fetch completion with no
 subs

Unit tests updated to make sure only a single query is executed after
multiple reads
---
 mod/forum/classes/subscriptions.php    |  3 +++
 mod/forum/tests/subscriptions_test.php | 30 ++++++++++++++++++++++++--
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/mod/forum/classes/subscriptions.php b/mod/forum/classes/subscriptions.php
index ada9b4d9e96..27b19dc8131 100644
--- a/mod/forum/classes/subscriptions.php
+++ b/mod/forum/classes/subscriptions.php
@@ -512,9 +512,12 @@ class subscriptions {
                         'userid' => $userid,
                         'forum' => $forumid,
                     ), null, 'id, discussion, preference');
+
+                    self::$forumdiscussioncache[$userid][$forumid] = array();
                     foreach ($subscriptions as $id => $data) {
                         self::add_to_discussion_cache($forumid, $userid, $data->discussion, $data->preference);
                     }
+
                     $subscriptions->close();
                 }
             } else {
diff --git a/mod/forum/tests/subscriptions_test.php b/mod/forum/tests/subscriptions_test.php
index 57035c2ef25..46673e02a92 100644
--- a/mod/forum/tests/subscriptions_test.php
+++ b/mod/forum/tests/subscriptions_test.php
@@ -1036,7 +1036,7 @@ class mod_forum_subscriptions_testcase extends advanced_testcase {
         // Create a course, with a forum.
         $course = $this->getDataGenerator()->create_course();
 
-        $options = array('course' => $course->id, 'forcesubscribe' => FORUM_INITIALSUBSCRIBE);
+        $options = array('course' => $course->id, 'forcesubscribe' => FORUM_FORCESUBSCRIBE);
         $forum = $this->getDataGenerator()->create_module('forum', $options);
 
         // Create some users.
@@ -1045,6 +1045,8 @@ class mod_forum_subscriptions_testcase extends advanced_testcase {
         // Post some discussions to the forum.
         $discussions = array();
         $author = $users[0];
+        $userwithnosubs = $users[1];
+
         for ($i = 0; $i < 20; $i++) {
             list($discussion, $post) = $this->helper_post_to_forum($forum, $author);
             $discussions[] = $discussion;
@@ -1053,15 +1055,20 @@ class mod_forum_subscriptions_testcase extends advanced_testcase {
         // Unsubscribe half the users from the half the discussions.
         $forumcount = 0;
         $usercount = 0;
+        $userwithsubs = null;
         foreach ($discussions as $data) {
+            // Unsubscribe user from all discussions.
+            \mod_forum\subscriptions::unsubscribe_user_from_discussion($userwithnosubs->id, $data);
+
             if ($forumcount % 2) {
                 continue;
             }
             foreach ($users as $user) {
                 if ($usercount % 2) {
+                    $userwithsubs = $user;
                     continue;
                 }
-                \mod_forum\subscriptions::unsubscribe_user_from_discussion($user->id, $discussion);
+                \mod_forum\subscriptions::unsubscribe_user_from_discussion($user->id, $data);
                 $usercount++;
             }
             $forumcount++;
@@ -1071,6 +1078,25 @@ class mod_forum_subscriptions_testcase extends advanced_testcase {
         \mod_forum\subscriptions::reset_forum_cache();
         \mod_forum\subscriptions::reset_discussion_cache();
 
+        // A user with no subscriptions should only be fetched once.
+        $this->assertNull(\mod_forum\subscriptions::fill_discussion_subscription_cache($forum->id, $userwithnosubs->id));
+        $startcount = $DB->perf_get_reads();
+        $this->assertNull(\mod_forum\subscriptions::fill_discussion_subscription_cache($forum->id, $userwithnosubs->id));
+        $this->assertEquals($startcount, $DB->perf_get_reads());
+
+        // Confirm subsequent calls properly tries to fetch subs.
+        $this->assertNull(\mod_forum\subscriptions::fill_discussion_subscription_cache($forum->id, $userwithsubs->id));
+        $this->assertNotEquals($startcount, $DB->perf_get_reads());
+
+        // Another read should be performed to get all subscriptions for the forum.
+        $startcount = $DB->perf_get_reads();
+        $this->assertNull(\mod_forum\subscriptions::fill_discussion_subscription_cache($forum->id));
+        $this->assertNotEquals($startcount, $DB->perf_get_reads());
+
+        // Reset the subscription caches.
+        \mod_forum\subscriptions::reset_forum_cache();
+        \mod_forum\subscriptions::reset_discussion_cache();
+
         // Filling the discussion subscription cache should only use a single query.
         $startcount = $DB->perf_get_reads();
         $this->assertNull(\mod_forum\subscriptions::fill_discussion_subscription_cache($forum->id));