DB: Use ID as offset in item queries

Signed-off-by: Sean Molenaar <sean@seanmolenaar.eu> Signed-off-by: Marco Nassabain <marco.nassabain@hotmail.com>
Team-Forward · Feb 23, 2021 · 8e5b406 · 8e5b406
1 parent c3a3fc1
commit 8e5b406
Show file tree

Hide file tree

Showing 5 changed files with 1,805 additions and 1,326 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ The format is almost based on [Keep a Changelog](https://keepachangelog.com/en/1
 - Item list throwing error for folder and "all items"
 - Articles with high IDs can be placed lower than articles with low IDs (#1147)
 - Feeds are accidentally moved on rename
+- Item list not using ID for offset
 
 ## [15.3.2] - 2021-02-10
 No changes compared to RC2

diff --git a/lib/Db/ItemMapperV2.php b/lib/Db/ItemMapperV2.php
@@ -403,13 +403,13 @@ public function findAllAfter(string $userId, int $feedType, int $updatedSince):
     }
 
     /**
-     * @param string $userId
-     * @param int    $feedId
-     * @param int    $limit
-     * @param int    $offset
-     * @param bool   $hideRead
-     * @param bool   $oldestFirst
-     * @param array  $search
+     * @param string $userId      User identifier
+     * @param int    $feedId      Feed identifier
+     * @param int    $limit       Max items to retrieve
+     * @param int    $offset      First item ID to retrieve
+     * @param bool   $hideRead    Hide read items
+     * @param bool   $oldestFirst Chronological sort
+     * @param array  $search      Search terms
      *
      * @return Item[]
      */
@@ -424,15 +424,22 @@ public function findAllFeed(
     ): array {
         $builder = $this->db->getQueryBuilder();
 
+        if ($oldestFirst === true) {
+            $offsetWhere = $builder->expr()->lt('items.id', ':offset');
+        } else {
+            $offsetWhere = $builder->expr()->gt('items.id', ':offset');
+        }
+
         $builder->select('items.*')
             ->from($this->tableName, 'items')
             ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
             ->andWhere('feeds.user_id = :userId')
             ->andWhere('items.feed_id = :feedId')
+            ->andWhere($offsetWhere)
             ->setParameter('userId', $userId)
             ->setParameter('feedId', $feedId)
+            ->setParameter('offset', $offset)
             ->setMaxResults($limit)
-            ->setFirstResult($offset)
             ->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
             ->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
 
@@ -452,13 +459,13 @@ public function findAllFeed(
     }
 
     /**
-     * @param string   $userId
-     * @param int|null $folderId
-     * @param int      $limit
-     * @param int      $offset
-     * @param bool     $hideRead
-     * @param bool     $oldestFirst
-     * @param array    $search
+     * @param string   $userId      User identifier
+     * @param int|null $folderId    Folder identifier (null for root)
+     * @param int      $limit       Max items to retrieve
+     * @param int      $offset      First item ID to retrieve
+     * @param bool     $hideRead    Hide read items
+     * @param bool     $oldestFirst Chronological sort
+     * @param array    $search      Search terms
      *
      * @return Item[]
      */
@@ -479,14 +486,21 @@ public function findAllFolder(
             $folderWhere = $builder->expr()->eq('feeds.folder_id', new Literal($folderId), IQueryBuilder::PARAM_INT);
         }
 
+        if ($oldestFirst === true) {
+            $offsetWhere = $builder->expr()->lt('items.id', ':offset');
+        } else {
+            $offsetWhere = $builder->expr()->gt('items.id', ':offset');
+        }
+
         $builder->select('items.*')
             ->from($this->tableName, 'items')
             ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
             ->andWhere('feeds.user_id = :userId')
             ->andWhere($folderWhere)
+            ->andWhere($offsetWhere)
             ->setParameter('userId', $userId)
+            ->setParameter('offset', $offset)
             ->setMaxResults($limit)
-            ->setFirstResult($offset)
             ->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
             ->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
 
@@ -506,12 +520,12 @@ public function findAllFolder(
     }
 
     /**
-     * @param string $userId
-     * @param int    $type
-     * @param int    $limit
-     * @param int    $offset
-     * @param bool   $oldestFirst
-     * @param array  $search
+     * @param string $userId      User identifier
+     * @param int    $type        Type of items to retrieve
+     * @param int    $limit       Max items to retrieve
+     * @param int    $offset      First item ID to retrieve
+     * @param bool   $oldestFirst Chronological sort
+     * @param array  $search      Search terms
      *
      * @return Item[]
      * @throws ServiceValidationException
@@ -526,13 +540,20 @@ public function findAllItems(
     ): array {
         $builder = $this->db->getQueryBuilder();
 
+        if ($oldestFirst === true) {
+            $offsetWhere = $builder->expr()->lt('items.id', ':offset');
+        } else {
+            $offsetWhere = $builder->expr()->gt('items.id', ':offset');
+        }
+
         $builder->select('items.*')
             ->from($this->tableName, 'items')
             ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
             ->andWhere('feeds.user_id = :userId')
+            ->andWhere($offsetWhere)
             ->setParameter('userId', $userId)
+            ->setParameter('offset', $offset)
             ->setMaxResults($limit)
-            ->setFirstResult($offset)
             ->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
             ->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));