opensourcemirror
/
nextcloud-news
mirror of https://github.com/nextcloud/news.git
1
0
Fork 0
Code Issues Releases Wiki Activity

DB: Fix offset quotes 

Issue GH-1200

Signed-off-by: Sean Molenaar <sean@seanmolenaar.eu>
This commit is contained in:
Sean Molenaar 2021-02-23 22:05:03 +01:00 committed by Benjamin Brahmer
parent 43deed2dbb
commit 9d5d35ce23
3 changed files with 59 additions and 170 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -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

55
lib/Db/ItemMapperV2.php
View File

@ -402,6 +402,22 @@ class ItemMapperV2 extends NewsMapperV2
return $this->findEntities($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';
}
/**
* @param string $userId User identifier
* @param int $feedId Feed identifier
@ -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');

173
tests/Unit/Db/ItemMapperPaginatedTest.php
View File

@ -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());