[NOTICES] Fix InboxNoticeStream caching

Introduce a new property in CachingNoticeStream for always checking if there
are any new elements in the stream.
It would be extremely hard to blow InboxNoticeStream, so instead the database
hit will still occur, but it is be much faster than starting fresh.

This fixes a regression introduced in 36a55d8436

lib/notices/cachingnoticestream.php

@@ -1,66 +1,96 @@
 <?php
+// This file is part of GNU social - https://www.gnu.org/software/social
+//
+// GNU social is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// GNU social is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with GNU social.  If not, see <http://www.gnu.org/licenses/>.
+
 /**
- * StatusNet - the distributed open-source microblogging tool
- * Copyright (C) 2011, StatusNet, Inc.
- *
  * A stream of notices
  *
- * PHP version 5
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
  * @category  Stream
- * @package   StatusNet
+ * @package   GNUsocial
  * @author    Evan Prodromou <evan@status.net>
  * @copyright 2011 StatusNet, Inc.
- * @license   http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0
+ * @license   https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
- * @link      http://status.net/
  */
 
-if (!defined('STATUSNET')) {
+defined('GNUSOCIAL') || die();
-    // This check helps protect against security problems;
-    // your code file can't be executed directly from the web.
-    exit(1);
-}
 
 /**
  * Class for notice streams
  *
  * @category  Stream
- * @package   StatusNet
+ * @package   GNUsocial
  * @author    Evan Prodromou <evan@status.net>
  * @copyright 2011 StatusNet, Inc.
- * @license   http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0
+ * @license   https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
- * @link      http://status.net/
  */
 
 class CachingNoticeStream extends NoticeStream
 {
     const CACHE_WINDOW = 200;
 
-    public $stream   = null;
+    public $stream      = null;
-    public $cachekey = null;
+    public $cachekey    = null;
-    public $useLast  = true;
+    public $useLast     = true;
+    public $alwaysCheck = true;
 
-    function __construct($stream, $cachekey, $useLast = true)
+    public function __construct(
-    {
+        NoticeStream $stream,
-        $this->stream   = $stream;
+        string       $cachekey,
-        $this->cachekey = $cachekey;
+        bool         $useLast = true,
-        $this->useLast  = $useLast;
+        bool         $alwaysCheck = false
+    ) {
+        $this->stream      = $stream;
+        $this->cachekey    = $cachekey;
+        $this->useLast     = $useLast;
+        $this->alwaysCheck = $alwaysCheck;
     }
 
-    function getNoticeIds($offset, $limit, $sinceId, $maxId)
+    private function getCacheNoticeIds(
+        Cache  $cache,
+        string $idkey,
+        bool   $check = false
+    ): ?array {
+        $id_str = $cache->get($idkey);
+
+        if ($id_str === false) {
+            return null;
+        }
+
+        $ids = explode(',', $id_str);
+
+        if ($check) {
+            $latest_id = $ids[0];
+            $new_ids = $this->stream->getNoticeIds(
+                0,
+                self::CACHE_WINDOW,
+                $latest_id,
+                null
+            );
+
+            $ids = array_merge($new_ids, $ids);
+            $ids = array_slice($ids, 0, self::CACHE_WINDOW);
+
+            $new_id_str = implode(',', $ids);
+            if ($id_str !== $new_id_str) {
+                $cache->set($idkey, $new_id_str);
+            }
+        }
+        return $ids;
+    }
+
+    public function getNoticeIds($offset, $limit, $sinceId, $maxId)
     {
         $cache = Cache::instance();
 
@@ -78,13 +108,11 @@ class CachingNoticeStream extends NoticeStream
 
         $idkey = Cache::key($this->cachekey);
 
-        $idstr = $cache->get($idkey);
+        $ids = $this->getCacheNoticeIds($cache, $idkey, $this->alwaysCheck);
 
-        if ($idstr !== false) {
+        if (!is_null($ids)) {
             // Cache hit! Woohoo!
-            $window = explode(',', $idstr);
+            return array_slice($ids, $offset, $limit);
-            $ids = array_slice($window, $offset, $limit);
-            return $ids;
         }
 
         if ($this->useLast) {
@@ -94,43 +122,31 @@ class CachingNoticeStream extends NoticeStream
             // a few at the "tip", which can bound our queries and save lots
             // of time.
 
-            $laststr = $cache->get($idkey.';last');
+            $ids = $this->getCacheNoticeIds($cache, $idkey . ';last', true);
 
-            if ($laststr !== false) {
+            if (!is_null($ids)) {
-                $window = explode(',', $laststr);
+                // Set the actual cache value as well
-                $last_id = $window[0];
+                $id_str = implode(',', $ids);
-                $new_ids = $this->stream->getNoticeIds(0, self::CACHE_WINDOW, $last_id, 0);
+                $cache->set($idkey, $id_str);
 
-                $new_window = array_merge($new_ids, $window);
+                return array_slice($ids, $offset, $limit);
-
-                $new_windowstr = implode(',', $new_window);
-
-                $result = $cache->set($idkey, $new_windowstr);
-                $result = $cache->set($idkey . ';last', $new_windowstr);
-
-                $ids = array_slice($new_window, $offset, $limit);
-
-                return $ids;
             }
         }
 
         // No cache hits :( Generate directly and stick the results
         // into the cache. Note we generate the full cache window.
 
-        $window = $this->stream->getNoticeIds(0, self::CACHE_WINDOW, 0, 0);
+        $window = $this->stream->getNoticeIds(0, self::CACHE_WINDOW, null, null);
 
         $windowstr = implode(',', $window);
 
-        $result = $cache->set($idkey, $windowstr);
+        $cache->set($idkey, $windowstr);
 
         if ($this->useLast) {
-            $result = $cache->set($idkey . ';last', $windowstr);
+            $cache->set($idkey . ';last', $windowstr);
         }
 
         // Return just the slice that was requested
-
+        return array_slice($window, $offset, $limit);
-        $ids = array_slice($window, $offset, $limit);
-
-        return $ids;
     }
 }

lib/notices/inboxnoticestream.php

@@ -46,7 +46,15 @@ class InboxNoticeStream extends ScopingNoticeStream
      */
     public function __construct(Profile $target, Profile $scoped = null)
     {
-        parent::__construct(new CachingNoticeStream(new RawInboxNoticeStream($target), 'profileall:'.$target->getID()), $scoped);
+        parent::__construct(
+            new CachingNoticeStream(
+                new RawInboxNoticeStream($target),
+                'profileall:' . $target->getID(),
+                false,
+                true
+            ),
+            $scoped
+        );
     }
 }