summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSean Molenaar <sean@seanmolenaar.eu>2021-04-05 15:13:30 +0200
committerSean Molenaar <SMillerDev@users.noreply.github.com>2021-04-05 19:18:19 +0200
commit951f3eb7eefae9b7236517a2eca8fcbfe77567b2 (patch)
treefafe57eb7d65515e0d3478bb3066c14524b58feb
parent5264713b50be83d022b15cd0431134972af25c36 (diff)
DB: Allow negative limits
Signed-off-by: Sean Molenaar <sean@seanmolenaar.eu>
-rw-r--r--AUTHORS.md1
-rw-r--r--CHANGELOG.md1
-rw-r--r--lib/Db/ItemMapperV2.php15
-rw-r--r--tests/Unit/Db/ItemMapperPaginatedTest.php223
-rw-r--r--tests/integration/items.bats41
5 files changed, 274 insertions, 7 deletions
diff --git a/AUTHORS.md b/AUTHORS.md
index 6062595a1..56f3aceb1 100644
--- a/AUTHORS.md
+++ b/AUTHORS.md
@@ -151,6 +151,7 @@
* [nexus-uw](mailto:you@example.com)
* [repat](mailto:repat@repat.de)
* [ritchiewilson](mailto:rawilson52@gmail.com)
+* [skiingwiz](mailto:skiingwiz@gmail.com)
* [sonologic](mailto:gmc@sonologic.nl)
* [tflidd](mailto:tflidd@aspekte.net)
* [waffshappen](mailto:44290023+waffshappen@users.noreply.github.com)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c6da0678a..fcdfcbdd1 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
+- Allow negative limits (#1275)
## [15.4.0-beta3] - 2021-04-03
### Changed
diff --git a/lib/Db/ItemMapperV2.php b/lib/Db/ItemMapperV2.php
index c79ac035f..b2e3bcb04 100644
--- a/lib/Db/ItemMapperV2.php
+++ b/lib/Db/ItemMapperV2.php
@@ -467,7 +467,6 @@ class ItemMapperV2 extends NewsMapperV2
->andWhere('items.feed_id = :feedId')
->setParameter('userId', $userId)
->setParameter('feedId', $feedId)
- ->setMaxResults($limit)
->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
@@ -479,6 +478,10 @@ class ItemMapperV2 extends NewsMapperV2
}
}
+ if ($limit >= 1) {
+ $builder->setMaxResults($limit);
+ }
+
if ($offset !== 0) {
$builder->andWhere($this->offsetWhere($oldestFirst))
->setParameter('offset', $offset);
@@ -526,7 +529,6 @@ class ItemMapperV2 extends NewsMapperV2
->andWhere('feeds.deleted_at = 0')
->andWhere($folderWhere)
->setParameter('userId', $userId)
- ->setMaxResults($limit)
->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
@@ -538,6 +540,10 @@ class ItemMapperV2 extends NewsMapperV2
}
}
+ if ($limit >= 1) {
+ $builder->setMaxResults($limit);
+ }
+
if ($offset !== 0) {
$builder->andWhere($this->offsetWhere($oldestFirst))
->setParameter('offset', $offset);
@@ -577,7 +583,6 @@ class ItemMapperV2 extends NewsMapperV2
->andWhere('feeds.user_id = :userId')
->andWhere('feeds.deleted_at = 0')
->setParameter('userId', $userId)
- ->setMaxResults($limit)
->orderBy('items.last_modified', ($oldestFirst ? 'ASC' : 'DESC'))
->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));
@@ -589,6 +594,10 @@ class ItemMapperV2 extends NewsMapperV2
}
}
+ if ($limit >= 1) {
+ $builder->setMaxResults($limit);
+ }
+
if ($offset !== 0) {
$builder->andWhere($this->offsetWhere($oldestFirst))
->setParameter('offset', $offset);
diff --git a/tests/Unit/Db/ItemMapperPaginatedTest.php b/tests/Unit/Db/ItemMapperPaginatedTest.php
index 6bd6906d3..93521f541 100644
--- a/tests/Unit/Db/ItemMapperPaginatedTest.php
+++ b/tests/Unit/Db/ItemMapperPaginatedTest.php
@@ -259,6 +259,75 @@ class ItemMapperPaginatedTest extends MapperTestUtility
$this->assertEquals([Item::fromRow(['id' => 4])], $result);
}
+ public function testFindAllItemsUnreadNoLimit()
+ {
+ $this->db->expects($this->once())
+ ->method('getQueryBuilder')
+ ->willReturn($this->builder);
+
+ $this->builder->expects($this->once())
+ ->method('select')
+ ->with('items.*')
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->once())
+ ->method('from')
+ ->with('news_items', 'items')
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->exactly(1))
+ ->method('innerJoin')
+ ->withConsecutive(['items', 'news_feeds', 'feeds', 'items.feed_id = feeds.id'])
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->exactly(4))
+ ->method('andWhere')
+ ->withConsecutive(
+ ['feeds.user_id = :userId'],
+ ['feeds.deleted_at = 0'],
+ ['items.id < :offset'],
+ ['items.unread = 1']
+ )
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->exactly(2))
+ ->method('setParameter')
+ ->withConsecutive(['userId', 'jack'], ['offset', 10])
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->never())
+ ->method('setMaxResults');
+
+ $this->builder->expects($this->exactly(0))
+ ->method('setFirstResult')
+ ->with(10)
+ ->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();
+
+ $this->builder->expects($this->exactly(1))
+ ->method('execute')
+ ->will($this->returnValue($this->cursor));
+
+ $this->cursor->expects($this->exactly(2))
+ ->method('fetch')
+ ->willReturnOnConsecutiveCalls(
+ ['id' => 4],
+ false
+ );
+
+ $result = $this->class->findAllItems('jack', 6, -1, 10, false, []);
+ $this->assertEquals([Item::fromRow(['id' => 4])], $result);
+ }
+
public function testFindAllItemsStarred()
{
$this->db->expects($this->once())
@@ -482,6 +551,77 @@ class ItemMapperPaginatedTest extends MapperTestUtility
$this->assertEquals([Item::fromRow(['id' => 4])], $result);
}
+ public function testFindAllFeedNoLimit()
+ {
+ $this->db->expects($this->once())
+ ->method('getQueryBuilder')
+ ->willReturn($this->builder);
+
+ $this->builder->expects($this->once())
+ ->method('select')
+ ->with('items.*')
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->once())
+ ->method('from')
+ ->with('news_items', 'items')
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->exactly(1))
+ ->method('innerJoin')
+ ->withConsecutive(['items', 'news_feeds', 'feeds', 'items.feed_id = feeds.id'])
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->exactly(4))
+ ->method('andWhere')
+ ->withConsecutive(
+ ['feeds.deleted_at = 0'],
+ ['feeds.user_id = :userId'],
+ ['items.feed_id = :feedId'],
+ ['items.id < :offset']
+ )
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->exactly(3))
+ ->method('setParameter')
+ ->withConsecutive(['userId', 'jack'], ['feedId', 2], ['offset', 10])
+ ->will($this->returnSelf());
+
+
+ $this->builder->expects($this->never())
+ ->method('setMaxResults');
+
+
+ $this->builder->expects($this->exactly(0))
+ ->method('setFirstResult')
+ ->with(10)
+ ->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();
+
+ $this->builder->expects($this->exactly(1))
+ ->method('execute')
+ ->will($this->returnValue($this->cursor));
+
+ $this->cursor->expects($this->exactly(2))
+ ->method('fetch')
+ ->willReturnOnConsecutiveCalls(
+ ['id' => 4],
+ false
+ );
+
+ $result = $this->class->findAllFeed('jack', 2, -1, 10, false, false, []);
+ $this->assertEquals([Item::fromRow(['id' => 4])], $result);
+ }
+
public function testFindAllFeedInverted()
{
$this->db->expects($this->once())
@@ -796,6 +936,89 @@ class ItemMapperPaginatedTest extends MapperTestUtility
$this->assertEquals([Item::fromRow(['id' => 4])], $result);
}
+ public function testFindAllFolderIdNullNoLimit()
+ {
+ $expr = $this->getMockBuilder(IExpressionBuilder::class)
+ ->getMock();
+
+ $expr->expects($this->once())
+ ->method('isNull')
+ ->with('feeds.folder_id')
+ ->will($this->returnValue('x IS NULL'));
+
+ $this->db->expects($this->once())
+ ->method('getQueryBuilder')
+ ->willReturn($this->builder);
+
+ $this->builder->expects($this->exactly(1))
+ ->method('expr')
+ ->will($this->returnValue($expr));
+
+ $this->builder->expects($this->once())
+ ->method('select')
+ ->with('items.*')
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->once())
+ ->method('from')
+ ->with('news_items', 'items')
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->exactly(1))
+ ->method('innerJoin')
+ ->withConsecutive(['items', 'news_feeds', 'feeds', 'items.feed_id = feeds.id'])
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->exactly(4))
+ ->method('andWhere')
+ ->withConsecutive(
+ ['feeds.user_id = :userId'],
+ ['feeds.deleted_at = 0'],
+ ['x IS NULL'],
+ ['items.id < :offset']
+ )
+ ->will($this->returnSelf());
+
+ $this->builder->expects($this->exactly(2))
+ ->method('setParameter')
+ ->withConsecutive(['userId', 'jack'], ['offset', 10])
+ ->will($this->returnSelf());
+
+
+ $this->builder->expects($this->never(1))
+ ->method('setMaxResults');
+
+
+ $this->builder->expects($this->exactly(0))
+ ->method('setFirstResult')
+ ->with(10)
+ ->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();
+
+ $this->builder->expects($this->exactly(1))
+ ->method('execute')
+ ->will($this->returnValue($this->cursor));
+
+ $this->cursor->expects($this->exactly(2))
+ ->method('fetch')
+ ->willReturnOnConsecutiveCalls(
+ ['id' => 4],
+ false
+ );
+
+ $result = $this->class->findAllFolder('jack', null, -1, 10, false, false, []);
+ $this->assertEquals([Item::fromRow(['id' => 4])], $result);
+ }
+
public function testFindAllFolderHideRead()
{
$expr = $this->getMockBuilder(IExpressionBuilder::class)
diff --git a/tests/integration/items.bats b/tests/integration/items.bats
index e85c8956c..22a4c73c7 100644
--- a/tests/integration/items.bats
+++ b/tests/integration/items.bats
@@ -17,7 +17,7 @@ teardown() {
fi
}
-@test "[$TESTSUITE] List all items in feed" {
+@test "[$TESTSUITE] List 200 items in feed" {
run ./occ news:item:list-feed "$user" "$ID" --limit 200
[ "$status" -eq 0 ]
@@ -28,7 +28,18 @@ teardown() {
fi
}
-@test "[$TESTSUITE] List all items in folder" {
+@test "[$TESTSUITE] List all items in feed" {
+ run ./occ news:item:list-feed "$user" "$ID" --limit 0
+ [ "$status" -eq 0 ]
+
+ if ! echo "$output" | grep "$TAG"; then
+ ret_status=$?
+ echo "Release not found in feed list"
+ return $ret_status
+ fi
+}
+
+@test "[$TESTSUITE] List 200 items in folder" {
run ./occ news:item:list-folder "$user" --limit 200
[ "$status" -eq 0 ]
@@ -39,7 +50,18 @@ teardown() {
fi
}
-@test "[$TESTSUITE] List all items" {
+@test "[$TESTSUITE] List all items in folder" {
+ run ./occ news:item:list-folder "$user" --limit 0
+ [ "$status" -eq 0 ]
+
+ if ! echo "$output" | grep "$TAG"; then
+ ret_status=$?
+ echo "Release not found in folder list"
+ return $ret_status
+ fi
+}
+
+@test "[$TESTSUITE] List 200 items" {
run ./occ news:item:list "$user" --limit 200
[ "$status" -eq 0 ]
@@ -48,4 +70,15 @@ teardown() {
echo "Release not found in list"
return $ret_status
fi
-} \ No newline at end of file
+}
+
+@test "[$TESTSUITE] List all items" {
+ run ./occ news:item:list "$user" --limit 0
+ [ "$status" -eq 0 ]
+
+ if ! echo "$output" | grep "$TAG"; then
+ ret_status=$?
+ echo "Release not found in list"
+ return $ret_status
+ fi
+}