summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSean Molenaar <sean@seanmolenaar.eu>2021-02-23 22:05:03 +0100
committerBenjamin Brahmer <info@b-brahmer.de>2021-02-27 15:12:59 +0100
commit9d5d35ce2396d0a059b4271ffbd454c0eacab0f8 (patch)
treed65637e9c46e24dcfec4dc6963ef58331c9263ee
parent43deed2dbbebb6cc45d1db2b469de106890fc5cf (diff)
DB: Fix offset quotes
Issue GH-1200 Signed-off-by: Sean Molenaar <sean@seanmolenaar.eu>
-rw-r--r--CHANGELOG.md1
-rw-r--r--lib/Db/ItemMapperV2.php55
-rw-r--r--tests/Unit/Db/ItemMapperPaginatedTest.php173
3 files changed, 59 insertions, 170 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c6194af48..bce387bdd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@ The format is almost based on [Keep a Changelog](https://keepachangelog.com/en/1
### Changed
### Fixed
+- Item list not using ID for offset 2 (#1200)
## [15.4.0-beta1] - 2021-02-23
### Changed
diff --git a/lib/Db/ItemMapperV2.php b/lib/Db/ItemMapperV2.php
index fe4bbabc0..33b4611f9 100644
--- a/lib/Db/ItemMapperV2.php
+++ b/lib/Db/ItemMapperV2.php
@@ -403,6 +403,22 @@ 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
@@ -424,21 +440,13 @@ class ItemMapperV2 extends NewsMapperV2
): 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)
->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
@@ -451,6 +459,11 @@ class ItemMapperV2 extends NewsMapperV2
}
}
+ if ($offset !== 0) {
+ $builder->andWhere($this->offsetWhere($oldestFirst))
+ ->setParameter('offset', $offset);
+ }
+
if ($hideRead === true) {
$builder->andWhere('items.unread = 1');
}
@@ -486,20 +499,12 @@ class ItemMapperV2 extends NewsMapperV2
$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)
->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
@@ -512,6 +517,11 @@ class ItemMapperV2 extends NewsMapperV2
}
}
+ if ($offset !== 0) {
+ $builder->andWhere($this->offsetWhere($oldestFirst))
+ ->setParameter('offset', $offset);
+ }
+
if ($hideRead === true) {
$builder->andWhere('items.unread = 1');
}
@@ -540,19 +550,11 @@ class ItemMapperV2 extends NewsMapperV2
): 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)
->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
@@ -565,6 +567,11 @@ class ItemMapperV2 extends NewsMapperV2
}
}
+ if ($offset !== 0) {
+ $builder->andWhere($this->offsetWhere($oldestFirst))
+ ->setParameter('offset', $offset);
+ }
+
switch ($type) {
case ListType::STARRED:
$builder->andWhere('items.starred = 1');
diff --git a/tests/Unit/Db/ItemMapperPaginatedTest.php b/tests/Unit/Db/ItemMapperPaginatedTest.php
index 4db152236..2b4ac0c5f 100644
--- a/tests/Unit/Db/ItemMapperPaginatedTest.php
+++ b/tests/Unit/Db/ItemMapperPaginatedTest.php
@@ -62,13 +62,6 @@ class ItemMapperPaginatedTest extends MapperTestUtility
$this->expectException(ServiceValidationException::class);
$this->expectExceptionMessage('Unexpected Feed type in call');
- $expr = $this->getMockBuilder(IExpressionBuilder::class)
- ->getMock();
-
- $this->builder->expects($this->exactly(1))
- ->method('expr')
- ->will($this->returnValue($expr));
-
$this->db->expects($this->once())
->method('getQueryBuilder')
->willReturn($this->builder);
@@ -142,18 +135,6 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->method('getQueryBuilder')
->willReturn($this->builder);
- $expr = $this->getMockBuilder(IExpressionBuilder::class)
- ->getMock();
-
- $expr->expects($this->once())
- ->method('lt')
- ->with('items.id', ':offset')
- ->will($this->returnValue('x < y'));
-
- $this->builder->expects($this->exactly(1))
- ->method('expr')
- ->will($this->returnValue($expr));
-
$this->builder->expects($this->once())
->method('select')
->with('items.*')
@@ -173,7 +154,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->method('andWhere')
->withConsecutive(
['feeds.user_id = :userId'],
- ['x < y']
+ ['items.id > :offset']
)
->will($this->returnSelf());
@@ -225,18 +206,6 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->method('getQueryBuilder')
->willReturn($this->builder);
- $expr = $this->getMockBuilder(IExpressionBuilder::class)
- ->getMock();
-
- $expr->expects($this->once())
- ->method('gt')
- ->with('items.id', ':offset')
- ->will($this->returnValue('x > y'));
-
- $this->builder->expects($this->exactly(1))
- ->method('expr')
- ->will($this->returnValue($expr));
-
$this->builder->expects($this->once())
->method('select')
->with('items.*')
@@ -256,7 +225,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->method('andWhere')
->withConsecutive(
['feeds.user_id = :userId'],
- ['x > y'],
+ ['items.id < :offset'],
['items.unread = 1']
)
->will($this->returnSelf());
@@ -309,18 +278,6 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->method('getQueryBuilder')
->willReturn($this->builder);
- $expr = $this->getMockBuilder(IExpressionBuilder::class)
- ->getMock();
-
- $expr->expects($this->once())
- ->method('gt')
- ->with('items.id', ':offset')
- ->will($this->returnValue('x > y'));
-
- $this->builder->expects($this->exactly(1))
- ->method('expr')
- ->will($this->returnValue($expr));
-
$this->builder->expects($this->once())
->method('select')
->with('items.*')
@@ -340,7 +297,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->method('andWhere')
->withConsecutive(
['feeds.user_id = :userId'],
- ['x > y'],
+ ['items.id < :offset'],
['items.starred = 1']
)
->will($this->returnSelf());
@@ -395,18 +352,6 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->method('escapeLikeParameter')
->will($this->returnArgument(0));
- $expr = $this->getMockBuilder(IExpressionBuilder::class)
- ->getMock();
-
- $expr->expects($this->once())
- ->method('gt')
- ->with('items.id', ':offset')
- ->will($this->returnValue('x > y'));
-
- $this->builder->expects($this->exactly(1))
- ->method('expr')
- ->will($this->returnValue($expr));
-
$this->builder->expects($this->once())
->method('select')
->with('items.*')
@@ -426,16 +371,16 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->method('andWhere')
->withConsecutive(
['feeds.user_id = :userId'],
- ['x > y'],
['items.search_index LIKE :term0'],
['items.search_index LIKE :term1'],
+ ['items.id < :offset'],
['items.starred = 1']
)
->will($this->returnSelf());
$this->builder->expects($this->exactly(4))
->method('setParameter')
- ->withConsecutive(['userId', 'jack'], ['offset', 10], ['term0', '%key%'], ['term1', '%word%'])
+ ->withConsecutive(['userId', 'jack'], ['term0', '%key%'], ['term1', '%word%'], ['offset', 10])
->will($this->returnSelf());
@@ -481,18 +426,6 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->method('getQueryBuilder')
->willReturn($this->builder);
- $expr = $this->getMockBuilder(IExpressionBuilder::class)
- ->getMock();
-
- $expr->expects($this->once())
- ->method('gt')
- ->with('items.id', ':offset')
- ->will($this->returnValue('x > y'));
-
- $this->builder->expects($this->exactly(1))
- ->method('expr')
- ->will($this->returnValue($expr));
-
$this->builder->expects($this->once())
->method('select')
->with('items.*')
@@ -513,7 +446,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->withConsecutive(
['feeds.user_id = :userId'],
['items.feed_id = :feedId'],
- ['x > y']
+ ['items.id < :offset']
)
->will($this->returnSelf());
@@ -565,18 +498,6 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->method('getQueryBuilder')
->willReturn($this->builder);
- $expr = $this->getMockBuilder(IExpressionBuilder::class)
- ->getMock();
-
- $expr->expects($this->once())
- ->method('lt')
- ->with('items.id', ':offset')
- ->will($this->returnValue('x < y'));
-
- $this->builder->expects($this->exactly(1))
- ->method('expr')
- ->will($this->returnValue($expr));
-
$this->builder->expects($this->once())
->method('select')
->with('items.*')
@@ -597,7 +518,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->withConsecutive(
['feeds.user_id = :userId'],
['items.feed_id = :feedId'],
- ['x < y']
+ ['items.id > :offset']
)
->will($this->returnSelf());
@@ -606,13 +527,11 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->withConsecutive(['userId', 'jack'], ['feedId', 2], ['offset', 10])
->will($this->returnSelf());
-
$this->builder->expects($this->exactly(1))
->method('setMaxResults')
->with(10)
->will($this->returnSelf());
-
$this->builder->expects($this->exactly(0))
->method('setFirstResult')
->with(10)
@@ -649,18 +568,6 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->method('getQueryBuilder')
->willReturn($this->builder);
- $expr = $this->getMockBuilder(IExpressionBuilder::class)
- ->getMock();
-
- $expr->expects($this->once())
- ->method('gt')
- ->with('items.id', ':offset')
- ->will($this->returnValue('x > y'));
-
- $this->builder->expects($this->exactly(1))
- ->method('expr')
- ->will($this->returnValue($expr));
-
$this->builder->expects($this->once())
->method('select')
->with('items.*')
@@ -681,7 +588,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->withConsecutive(
['feeds.user_id = :userId'],
['items.feed_id = :feedId'],
- ['x > y'],
+ ['items.id < :offset'],
['items.unread = 1']
)
->will($this->returnSelf());
@@ -737,18 +644,6 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->method('escapeLikeParameter')
->will($this->returnArgument(0));
- $expr = $this->getMockBuilder(IExpressionBuilder::class)
- ->getMock();
-
- $expr->expects($this->once())
- ->method('gt')
- ->with('items.id', ':offset')
- ->will($this->returnValue('x > y'));
-
- $this->builder->expects($this->exactly(1))
- ->method('expr')
- ->will($this->returnValue($expr));
-
$this->builder->expects($this->once())
->method('select')
->with('items.*')
@@ -769,15 +664,21 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->withConsecutive(
['feeds.user_id = :userId'],
['items.feed_id = :feedId'],
- ['x > y'],
['items.search_index LIKE :term0'],
- ['items.search_index LIKE :term1']
+ ['items.search_index LIKE :term1'],
+ ['items.id < :offset']
)
->will($this->returnSelf());
$this->builder->expects($this->exactly(5))
->method('setParameter')
- ->withConsecutive(['userId', 'jack'], ['feedId', 2], ['offset', 10], ['term0', '%key%'], ['term1', '%word%'])
+ ->withConsecutive(
+ ['userId', 'jack'],
+ ['feedId', 2],
+ ['term0', '%key%'],
+ ['term1', '%word%'],
+ ['offset', 10]
+ )
->will($this->returnSelf());
@@ -827,16 +728,11 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->with('feeds.folder_id')
->will($this->returnValue('x IS NULL'));
- $expr->expects($this->once())
- ->method('gt')
- ->with('items.id', ':offset')
- ->will($this->returnValue('x > y'));
-
$this->db->expects($this->once())
->method('getQueryBuilder')
->willReturn($this->builder);
- $this->builder->expects($this->exactly(2))
+ $this->builder->expects($this->exactly(1))
->method('expr')
->will($this->returnValue($expr));
@@ -860,7 +756,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->withConsecutive(
['feeds.user_id = :userId'],
['x IS NULL'],
- ['x > y']
+ ['items.id < :offset']
)
->will($this->returnSelf());
@@ -916,16 +812,11 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->with('feeds.folder_id')
->will($this->returnValue('x IS NULL'));
- $expr->expects($this->once())
- ->method('gt')
- ->with('items.id', ':offset')
- ->will($this->returnValue('x > y'));
-
$this->db->expects($this->once())
->method('getQueryBuilder')
->willReturn($this->builder);
- $this->builder->expects($this->exactly(2))
+ $this->builder->expects($this->exactly(1))
->method('expr')
->will($this->returnValue($expr));
@@ -949,7 +840,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->withConsecutive(
['feeds.user_id = :userId'],
['x IS NULL'],
- ['x > y'],
+ ['items.id < :offset'],
['items.unread = 1']
)
->will($this->returnSelf());
@@ -1006,16 +897,11 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->with('feeds.folder_id')
->will($this->returnValue('x IS NULL'));
- $expr->expects($this->once())
- ->method('lt')
- ->with('items.id', ':offset')
- ->will($this->returnValue('x < y'));
-
$this->db->expects($this->once())
->method('getQueryBuilder')
->willReturn($this->builder);
- $this->builder->expects($this->exactly(2))
+ $this->builder->expects($this->exactly(1))
->method('expr')
->will($this->returnValue($expr));
@@ -1039,7 +925,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->withConsecutive(
['feeds.user_id = :userId'],
['x IS NULL'],
- ['x < y'],
+ ['items.id > :offset'],
['items.unread = 1']
)
->will($this->returnSelf());
@@ -1091,7 +977,7 @@ class ItemMapperPaginatedTest extends MapperTestUtility
$expr = $this->getMockBuilder(IExpressionBuilder::class)
->getMock();
- $this->builder->expects($this->exactly(2))
+ $this->builder->expects($this->exactly(1))
->method('expr')
->will($this->returnValue($expr));
@@ -1100,11 +986,6 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->with('feeds.folder_id', new Literal(2))
->will($this->returnValue('x = y'));
- $expr->expects($this->once())
- ->method('gt')
- ->with('items.id', ':offset')
- ->will($this->returnValue('x > y'));
-
$this->db->expects($this->once())
->method('getQueryBuilder')
->willReturn($this->builder);
@@ -1132,15 +1013,15 @@ class ItemMapperPaginatedTest extends MapperTestUtility
->withConsecutive(
['feeds.user_id = :userId'],
['x = y'],
- ['x > y'],
['items.search_index LIKE :term0'],
- ['items.search_index LIKE :term1']
+ ['items.search_index LIKE :term1'],
+ ['items.id < :offset']
)
->will($this->returnSelf());
$this->builder->expects($this->exactly(4))
->method('setParameter')
- ->withConsecutive(['userId', 'jack'], ['offset', 10], ['term0', '%key%'], ['term1', '%word%'])
+ ->withConsecutive(['userId', 'jack'], ['term0', '%key%'], ['term1', '%word%'], ['offset', 10])
->will($this->returnSelf());