DB: only sort on item IDs

Signed-off-by: Sean Molenaar <sean@seanmolenaar.eu>

5 files changed, 79 insertions, 75 deletions

diff --git a/AUTHORS.md b/AUTHORS.md
index e6d7dca80..afeda70f2 100644
--- a/AUTHORS.md
+++ b/AUTHORS.md
@@ -2,23 +2,28 @@
 * [Bernhard Posselt](mailto:dev@bernhard-posselt.com)
 * [Alessandro Cosentino](mailto:cosenal@gmail.com)
 * [Benjamin Brahmer](mailto:info@b-brahmer.de)
+* [Marco Nassabain](mailto:marco.nassabain@hotmail.com)
 * [Robin Appelman](mailto:icewind@owncloud.com)
 * [Sean Molenaar](mailto:sean@seanmolenaar.eu)
 * [Gregor Tätzner](mailto:gregor@freenet.de)
 * [Sean Molenaar](mailto:SMillerDev@users.noreply.github.com)
 * [Morris Jobke](mailto:hey@morrisjobke.de)
 * [anoy](mailto:anoymouserver+github@mailbox.org)
+* [Jimmy Huynh](mailto:jimmy.huynh@etu.unistra.fr)
+* [Aurélien](mailto:dav.aurelien@gmail.com)
 * [Jan-Christoph Borchardt](mailto:hey@jancborchardt.net)
+* [Paul Tirk](mailto:paultirk@paultirk.com)
 * [Daniel Schaal](mailto:daniel@schaal.email)
 * [Davide Saurino](mailto:davide.saurino@alcacoop.it)
 * [raghunayyar](mailto:me@iraghu.com)
+* [WENDLING NICOLAS](mailto:nicolas.wendling2@etu.unistra.fr)
 * [bastei](mailto:bastei@users.noreply.github.com)
 * [Bernhard Posselt](mailto:bep@foryouandyourcustomers.com)
 * [Thomas Müller](mailto:thomas.mueller@tmit.eu)
 * [Hoàng Đức Hiếu](mailto:hdhoang@zahe.me)
+* [Marco Nassabain](mailto:marco.nassabain@etu.unistra.fr)
 * [rakekniven](mailto:mark.ziegler@rakekniven.de)
 * [Daniel Opitz](mailto:git@copynpaste.de)
-* [Marco Nassabain](mailto:marco.nassabain@hotmail.com)
 * [Sean Molenaar](mailto:sean@m2mobi.com)
 * [David-Development](mailto:david-dev@live.de)
 * [IBBoard](mailto:dev@ibboard.co.uk)
@@ -26,7 +31,7 @@
 * [Lukas Reschke](mailto:lukas@owncloud.com)
 * [Bart Visscher](mailto:bartv@thisnet.nl)
 * [Christian Elmer](mailto:christian@keinkurt.de)
-* [Jimmy Huynh](mailto:jimmy.huynh@etu.unistra.fr)
+* [Nicolas Wendling](mailto:nicolas.wendling1011@gmail.com)
 * [Thomas Tanghus](mailto:thomas@tanghus.net)
 * [Volkan Gezer](mailto:volkangezer@gmail.com)
 * [Xéfir Destiny](mailto:xefir@crystalyx.net)
@@ -39,11 +44,13 @@
 * [Brice Maron](mailto:brice@bmaron.net)
 * [Christoph Stenglein](mailto:christoph@christophstenglein.com)
 * [Daniel Kesselberg](mailto:mail@danielkesselberg.de)
+* [ELHADDAD Hamza](mailto:elhaddadhamza49@gmail.com)
 * [Jakob Sack](mailto:mail@jakobsack.de)
 * [Qingping Hou](mailto:dave2008713@gmail.com)
 * [Roman](mailto:reverse@jamm.me)
 * [b_b](mailto:bruno@eliaz.fr)
 * [heyarne](mailto:arne@schlueter.is)
+* [marco.nassabain@etu.unistra.fr](mailto:marco.nassabain@hotmail.com)
 * [Andreas Fischer](mailto:bantu@owncloud.com)
 * [David Guillot](mailto:david@guillot.me)
 * [Gioele Falcetti](mailto:thegio.f@gmail.com)
@@ -73,6 +80,8 @@
 * [Andrea Boero](mailto:mail@tsumi.it)
 * [Andreas Demmelbauer](mailto:git@notice.at)
 * [Arthur Schiwon](mailto:blizzz@arthur-schiwon.de)
+* [Aurelien DAVID](mailto:aurelien.david@etu.unistra.fr)
+* [Aurelien DAVID](mailto:dav.aurelien@gmail.com)
 * [Benedikt Geißler](mailto:benedikt@g5r.eu)
 * [Bernhard Posselt](mailto:bernhard@desktop.localdomain)
 * [Candid Dauth](mailto:cdauth@cdauth.eu)
@@ -132,6 +141,7 @@
 * [Tilo Spannagel](mailto:development@tilosp.de)
 * [Timo Schmidt](mailto:timo@xinterchange.net)
 * [Tucker McKnight](mailto:tucker.mcknight@gmail.com)
+* [WENDLING NICOLAS](mailto:nicolas.wendling1011@gmail.com)
 * [Welling Guzmán](mailto:WellingGuzman@users.noreply.github.com)
 * [Xaver Maierhofer](mailto:xaver.maierhofer@xwissen.info)
 * [Xemle](mailto:xemle@phtagr.org)
@@ -141,6 +151,7 @@
 * [b_b](mailto:brunobergot@gmail.com)
 * [bjoerns1983](mailto:bjoern@sengotta.net)
 * [blackcrack](mailto:blackcrack@blackysgate.de)
+* [cherguimalih](mailto:ilyes.chergui-malih@etu.unistra.fr)
 * [comradekingu](mailto:epost@anotheragency.no)
 * [derritter88](mailto:derritter88@users.noreply.github.com)
 * [e-alfred](mailto:e-alfred@users.noreply.github.com)
@@ -149,6 +160,7 @@
 * [kesselb](mailto:mail@danielkesselberg.de)
 * [kondou](mailto:kondou@ts.unde.re)
 * [markusj](mailto:markusj@users.noreply.github.com)
+* [mnassabain](mailto:34754819+mnassabain@users.noreply.github.com)
 * [nexus-uw](mailto:you@example.com)
 * [repat](mailto:repat@repat.de)
 * [ritchiewilson](mailto:rawilson52@gmail.com)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cf5f29691..bc44acd27 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@ The format is almost based on [Keep a Changelog](https://keepachangelog.com/en/1
 ### Fixed
 - Check category label for null (#1282)
 - Do not return non-matching search items
+- Resolve an issue with webservices missing items
 
 ## [15.4.0-beta3] - 2021-04-03
 ### Fixed
diff --git a/lib/Db/ItemMapperV2.php b/lib/Db/ItemMapperV2.php
index 44d1740cd..6e50a5fb5 100644
--- a/lib/Db/ItemMapperV2.php
+++ b/lib/Db/ItemMapperV2.php
@@ -62,7 +62,7 @@ class ItemMapperV2 extends NewsMapperV2
                 ->setParameter('user_id', $userId, IQueryBuilder::PARAM_STR);
 
         foreach ($params as $key => $value) {
-            $builder->andWhere("${key} = " . $builder->createNamedParameter($value));
+            $builder->andWhere("$key = " . $builder->createNamedParameter($value));
         }
 
         return $this->findEntities($builder);
@@ -201,7 +201,6 @@ class ItemMapperV2 extends NewsMapperV2
             ->from($this->tableName)
             ->where('feed_id = :feedId')
             ->andWhere('starred = false')
-            ->orderBy('last_modified', 'DESC')
             ->addOrderBy('id', 'DESC');
 
         if ($removeUnread === false) {
@@ -426,22 +425,6 @@ class ItemMapperV2 extends NewsMapperV2
     }
 
     /**
-     * Generate an expression for the offset.
-     *
-     * @param bool $oldestFirst Sorting direction
-     *
-     * @return string
-     */
-    private function offsetWhere(bool $oldestFirst): string
-    {
-        if ($oldestFirst === true) {
-            return 'items.id > :offset';
-        }
-
-        return 'items.id < :offset';
-    }
-
-    /**
      * @param string $userId      User identifier
      * @param int    $feedId      Feed identifier
      * @param int    $limit       Max items to retrieve
@@ -471,16 +454,9 @@ class ItemMapperV2 extends NewsMapperV2
             ->andWhere('items.feed_id = :feedId')
             ->setParameter('userId', $userId)
             ->setParameter('feedId', $feedId)
-            ->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
             ->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
 
-        if ($search !== []) {
-            foreach ($search as $key => $term) {
-                $term = $this->db->escapeLikeParameter($term);
-                $builder->andWhere("items.search_index LIKE :term${key}")
-                        ->setParameter("term${key}", "%$term%");
-            }
-        }
+        $builder = $this->addSearch($builder, $search);
 
         if ($limit >= 1) {
             $builder->setMaxResults($limit);
@@ -534,16 +510,9 @@ class ItemMapperV2 extends NewsMapperV2
             ->andWhere('feeds.deleted_at = 0')
             ->andWhere($folderWhere)
             ->setParameter('userId', $userId)
-            ->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
             ->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
 
-        if ($search !== []) {
-            foreach ($search as $key => $term) {
-                $term = $this->db->escapeLikeParameter($term);
-                $builder->andWhere("items.search_index LIKE :term${key}")
-                        ->setParameter("term${key}", "%$term%");
-            }
-        }
+        $builder = $this->addSearch($builder, $search);
 
         if ($limit >= 1) {
             $builder->setMaxResults($limit);
@@ -589,17 +558,8 @@ class ItemMapperV2 extends NewsMapperV2
             ->andWhere('feeds.user_id = :userId')
             ->andWhere('feeds.deleted_at = 0')
             ->setParameter('userId', $userId)
-            ->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
             ->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
 
-        if ($search !== []) {
-            foreach ($search as $key => $term) {
-                $term = $this->db->escapeLikeParameter($term);
-                $builder->andWhere("items.search_index LIKE :term${key}")
-                        ->setParameter("term${key}", "%$term%");
-            }
-        }
-
         if ($limit >= 1) {
             $builder->setMaxResults($limit);
         }
@@ -609,6 +569,8 @@ class ItemMapperV2 extends NewsMapperV2
                     ->setParameter('offset', $offset);
         }
 
+        $builder = $this->addSearch($builder, $search);
+
         switch ($type) {
             case ListType::STARRED:
                 $builder->andWhere('items.starred = :starred')
@@ -626,4 +588,43 @@ class ItemMapperV2 extends NewsMapperV2
 
         return $this->findEntities($builder);
     }
+
+    /**
+     * Add search parameters.
+     *
+     * @param IQueryBuilder $builder
+     * @param array         $terms
+     *
+     * @return IQueryBuilder
+     */
+    private function addSearch(IQueryBuilder $builder, array $terms): IQueryBuilder
+    {
+        if ($terms === []) {
+            return $builder;
+        }
+
+        foreach ($terms as $key => $term) {
+            $term = $this->db->escapeLikeParameter($term);
+            $builder->andWhere("items.search_index LIKE :term$key")
+                ->setParameter("term$key", "%$term%");
+        }
+
+        return $builder;
+    }
+
+    /**
+     * Generate an expression for the offset.
+     *
+     * @param bool $oldestFirst Sorting direction
+     *
+     * @return string
+     */
+    private function offsetWhere(bool $oldestFirst): string
+    {
+        if ($oldestFirst === true) {
+            return 'items.id > :offset';
+        }
+
+        return 'items.id < :offset';
+    }
 }
diff --git a/tests/Unit/Db/ItemMapperPaginatedTest.php b/tests/Unit/Db/ItemMapperPaginatedTest.php
index 448622280..652e2fa98 100644
--- a/tests/Unit/Db/ItemMapperPaginatedTest.php
+++ b/tests/Unit/Db/ItemMapperPaginatedTest.php
@@ -93,11 +93,6 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->will($this->returnSelf());
 
         $this->builder->expects($this->once())
-            ->method('orderBy')
-            ->with('items.last_modified', 'DESC')
-            ->will($this->returnSelf());
-
-        $this->builder->expects($this->once())
             ->method('addOrderBy')
             ->with('items.id', 'DESC')
             ->willReturnSelf();
@@ -164,11 +159,6 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->will($this->returnSelf());
 
         $this->builder->expects($this->once())
-            ->method('orderBy')
-            ->with('items.last_modified', 'ASC')
-            ->will($this->returnSelf());
-
-        $this->builder->expects($this->once())
             ->method('addOrderBy')
             ->with('items.id', 'ASC')
             ->willReturnSelf();
@@ -234,7 +224,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'DESC')
             ->will($this->returnSelf());
@@ -303,7 +293,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'DESC')
             ->will($this->returnSelf());
@@ -375,7 +365,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'DESC')
             ->will($this->returnSelf());
@@ -429,9 +419,9 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->withConsecutive(
                 ['feeds.user_id = :userId'],
                 ['feeds.deleted_at = 0'],
+                ['items.id < :offset'],
                 ['items.search_index LIKE :term0'],
                 ['items.search_index LIKE :term1'],
-                ['items.id < :offset'],
                 ['items.starred = :starred']
             )
             ->will($this->returnSelf());
@@ -440,9 +430,9 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->method('setParameter')
             ->withConsecutive(
                 ['userId', 'jack'],
+                ['offset', 10],
                 ['term0', '%key%'],
                 ['term1', '%word%'],
-                ['offset', 10],
                 ['starred', true]
             )
             ->will($this->returnSelf());
@@ -459,7 +449,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'DESC')
             ->will($this->returnSelf());
@@ -532,7 +522,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'DESC')
             ->will($this->returnSelf());
@@ -603,7 +593,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'DESC')
             ->will($this->returnSelf());
@@ -674,7 +664,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'ASC')
             ->will($this->returnSelf());
@@ -748,7 +738,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'DESC')
             ->will($this->returnSelf());
@@ -832,7 +822,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'DESC')
             ->will($this->returnSelf());
@@ -917,7 +907,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'DESC')
             ->will($this->returnSelf());
@@ -991,7 +981,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->will($this->returnSelf());
 
 
-        $this->builder->expects($this->never(1))
+        $this->builder->expects($this->never())
             ->method('setMaxResults');
 
 
@@ -1000,7 +990,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'DESC')
             ->will($this->returnSelf());
@@ -1086,7 +1076,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'DESC')
             ->will($this->returnSelf());
@@ -1172,7 +1162,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'ASC')
             ->will($this->returnSelf());
@@ -1262,7 +1252,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
             ->with(10)
             ->will($this->returnSelf());
 
-        $this->builder->expects($this->once())
+        $this->builder->expects($this->never())
             ->method('orderBy')
             ->with('items.last_modified', 'DESC')
             ->will($this->returnSelf());
diff --git a/tests/Unit/Db/ItemMapperTest.php b/tests/Unit/Db/ItemMapperTest.php
index 5cb42c3fa..12c022893 100644
--- a/tests/Unit/Db/ItemMapperTest.php
+++ b/tests/Unit/Db/ItemMapperTest.php
@@ -752,7 +752,7 @@ class ItemMapperTest extends MapperTestUtility
                  ->with('starred = false')
                  ->willReturnSelf();
 
-        $builder2->expects($this->once())
+        $builder2->expects($this->never())
                  ->method('orderBy')
                  ->with('last_modified', 'DESC')
                  ->willReturnSelf();
@@ -910,7 +910,7 @@ class ItemMapperTest extends MapperTestUtility
                  ->withConsecutive(['starred = false'], ['unread = false'])
                  ->willReturnSelf();
 
-        $builder2->expects($this->once())
+        $builder2->expects($this->never())
                  ->method('orderBy')
                  ->with('last_modified', 'DESC')
                  ->willReturnSelf();
@@ -1067,7 +1067,7 @@ class ItemMapperTest extends MapperTestUtility
                  ->withConsecutive(['starred = false'], ['unread = false'])
                  ->willReturnSelf();
 
-        $builder2->expects($this->once())
+        $builder2->expects($this->never())
                  ->method('orderBy')
                  ->with('last_modified', 'DESC')
                  ->willReturnSelf();