From 90702dd297bf8dac6ac5baa7eb25ecf35f0a7d25 Mon Sep 17 00:00:00 2001 From: Sean Molenaar Date: Tue, 6 Apr 2021 12:29:36 +0200 Subject: DB: Use boolean parameter for unread/starred Issue GH-1278 Signed-off-by: Sean Molenaar --- CHANGELOG.md | 1 + lib/Db/ItemMapperV2.php | 34 +++++++++++++--------- tests/Unit/Db/ItemMapperAfterTest.php | 28 +++++++++++++++--- tests/Unit/Db/ItemMapperPaginatedTest.php | 48 +++++++++++++++++-------------- 4 files changed, 73 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcdfcbdd1..531a2dc35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is almost based on [Keep a Changelog](https://keepachangelog.com/en/1 ### Fixed - Allow negative limits (#1275) +- Use boolean to check bool fields (#1278) ## [15.4.0-beta3] - 2021-04-03 ### Changed diff --git a/lib/Db/ItemMapperV2.php b/lib/Db/ItemMapperV2.php index b2e3bcb04..44d1740cd 100644 --- a/lib/Db/ItemMapperV2.php +++ b/lib/Db/ItemMapperV2.php @@ -342,7 +342,8 @@ class ItemMapperV2 extends NewsMapperV2 ->addOrderBy('items.id', 'DESC'); if ($hideRead === true) { - $builder->andWhere('items.unread = 1'); + $builder->andWhere('items.unread = :unread') + ->setParameter('unread', true); } return $this->findEntities($builder); @@ -377,7 +378,8 @@ class ItemMapperV2 extends NewsMapperV2 ->addOrderBy('items.id', 'DESC'); if ($hideRead === true) { - $builder->andWhere('items.unread = 1'); + $builder->andWhere('items.unread = :unread') + ->setParameter('unread', true); } return $this->findEntities($builder); @@ -407,10 +409,12 @@ class ItemMapperV2 extends NewsMapperV2 switch ($feedType) { case ListType::STARRED: - $builder->andWhere('items.starred = 1'); + $builder->andWhere('items.starred = :starred') + ->setParameter('starred', true); break; case ListType::UNREAD: - $builder->andWhere('items.unread = 1'); + $builder->andWhere('items.unread = :unread') + ->setParameter('unread', true); break; case ListType::ALL_ITEMS: break; @@ -474,7 +478,7 @@ class ItemMapperV2 extends NewsMapperV2 foreach ($search as $key => $term) { $term = $this->db->escapeLikeParameter($term); $builder->andWhere("items.search_index LIKE :term${key}") - ->setParameter("term${key}", "%$term%"); + ->setParameter("term${key}", "%$term%"); } } @@ -484,11 +488,12 @@ class ItemMapperV2 extends NewsMapperV2 if ($offset !== 0) { $builder->andWhere($this->offsetWhere($oldestFirst)) - ->setParameter('offset', $offset); + ->setParameter('offset', $offset); } if ($hideRead === true) { - $builder->andWhere('items.unread = 1'); + $builder->andWhere('items.unread = :unread') + ->setParameter('unread', true); } return $this->findEntities($builder); @@ -536,7 +541,7 @@ class ItemMapperV2 extends NewsMapperV2 foreach ($search as $key => $term) { $term = $this->db->escapeLikeParameter($term); $builder->andWhere("items.search_index LIKE :term${key}") - ->setParameter("term${key}", "%$term%"); + ->setParameter("term${key}", "%$term%"); } } @@ -546,11 +551,12 @@ class ItemMapperV2 extends NewsMapperV2 if ($offset !== 0) { $builder->andWhere($this->offsetWhere($oldestFirst)) - ->setParameter('offset', $offset); + ->setParameter('offset', $offset); } if ($hideRead === true) { - $builder->andWhere('items.unread = 1'); + $builder->andWhere('items.unread = :unread') + ->setParameter('unread', true); } return $this->findEntities($builder); @@ -600,15 +606,17 @@ class ItemMapperV2 extends NewsMapperV2 if ($offset !== 0) { $builder->andWhere($this->offsetWhere($oldestFirst)) - ->setParameter('offset', $offset); + ->setParameter('offset', $offset); } switch ($type) { case ListType::STARRED: - $builder->andWhere('items.starred = 1'); + $builder->andWhere('items.starred = :starred') + ->setParameter('starred', true); break; case ListType::UNREAD: - $builder->andWhere('items.unread = 1'); + $builder->andWhere('items.unread = :unread') + ->setParameter('unread', true); break; case ListType::ALL_ITEMS: break; diff --git a/tests/Unit/Db/ItemMapperAfterTest.php b/tests/Unit/Db/ItemMapperAfterTest.php index 5b0475522..68bad19dc 100644 --- a/tests/Unit/Db/ItemMapperAfterTest.php +++ b/tests/Unit/Db/ItemMapperAfterTest.php @@ -134,7 +134,7 @@ class ItemMapperAfterTest extends MapperTestUtility ['feeds.user_id = :userId'], ['feeds.id = :feedId'], ['feeds.deleted_at = 0'], - ['items.unread = 1'] + ['items.unread = :unread'] ) ->will($this->returnSelf()); @@ -147,6 +147,11 @@ class ItemMapperAfterTest extends MapperTestUtility ]) ->will($this->returnSelf()); + $this->builder->expects($this->exactly(1)) + ->method('setParameter') + ->with('unread', true) + ->will($this->returnSelf()); + $this->builder->expects($this->once()) ->method('orderBy') ->with('items.last_modified', 'DESC') @@ -271,7 +276,7 @@ class ItemMapperAfterTest extends MapperTestUtility ['feeds.user_id = :userId'], ['feeds.deleted_at = 0'], ['folders.id = :folderId'], - ['items.unread = 1'] + ['items.unread = :unread'] ) ->will($this->returnSelf()); @@ -284,6 +289,11 @@ class ItemMapperAfterTest extends MapperTestUtility ]) ->will($this->returnSelf()); + $this->builder->expects($this->exactly(1)) + ->method('setParameter') + ->with('unread', true) + ->will($this->returnSelf()); + $this->builder->expects($this->once()) ->method('orderBy') ->with('items.last_modified', 'DESC') @@ -336,7 +346,7 @@ class ItemMapperAfterTest extends MapperTestUtility ['items.last_modified >= :updatedSince'], ['feeds.deleted_at = 0'], ['feeds.user_id = :userId'], - ['items.unread = 1'] + ['items.unread = :unread'] ) ->will($this->returnSelf()); @@ -348,6 +358,11 @@ class ItemMapperAfterTest extends MapperTestUtility ]) ->will($this->returnSelf()); + $this->builder->expects($this->exactly(1)) + ->method('setParameter') + ->with('unread', true) + ->will($this->returnSelf()); + $this->builder->expects($this->once()) ->method('orderBy') ->with('items.last_modified', 'DESC') @@ -400,7 +415,7 @@ class ItemMapperAfterTest extends MapperTestUtility ['items.last_modified >= :updatedSince'], ['feeds.deleted_at = 0'], ['feeds.user_id = :userId'], - ['items.starred = 1'] + ['items.starred = :starred'] ) ->will($this->returnSelf()); @@ -412,6 +427,11 @@ class ItemMapperAfterTest extends MapperTestUtility ]) ->will($this->returnSelf()); + $this->builder->expects($this->exactly(1)) + ->method('setParameter') + ->with('starred', true) + ->will($this->returnSelf()); + $this->builder->expects($this->once()) ->method('orderBy') ->with('items.last_modified', 'DESC') diff --git a/tests/Unit/Db/ItemMapperPaginatedTest.php b/tests/Unit/Db/ItemMapperPaginatedTest.php index 93521f541..448622280 100644 --- a/tests/Unit/Db/ItemMapperPaginatedTest.php +++ b/tests/Unit/Db/ItemMapperPaginatedTest.php @@ -215,13 +215,13 @@ class ItemMapperPaginatedTest extends MapperTestUtility ['feeds.user_id = :userId'], ['feeds.deleted_at = 0'], ['items.id < :offset'], - ['items.unread = 1'] + ['items.unread = :unread'] ) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(2)) + $this->builder->expects($this->exactly(3)) ->method('setParameter') - ->withConsecutive(['userId', 'jack'], ['offset', 10]) + ->withConsecutive(['userId', 'jack'], ['offset', 10], ['unread', true]) ->will($this->returnSelf()); $this->builder->expects($this->exactly(1)) @@ -286,13 +286,13 @@ class ItemMapperPaginatedTest extends MapperTestUtility ['feeds.user_id = :userId'], ['feeds.deleted_at = 0'], ['items.id < :offset'], - ['items.unread = 1'] + ['items.unread = :unread'] ) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(2)) + $this->builder->expects($this->exactly(3)) ->method('setParameter') - ->withConsecutive(['userId', 'jack'], ['offset', 10]) + ->withConsecutive(['userId', 'jack'], ['offset', 10], ['unread', true]) ->will($this->returnSelf()); $this->builder->expects($this->never()) @@ -355,13 +355,13 @@ class ItemMapperPaginatedTest extends MapperTestUtility ['feeds.user_id = :userId'], ['feeds.deleted_at = 0'], ['items.id < :offset'], - ['items.starred = 1'] + ['items.starred = :starred'] ) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(2)) + $this->builder->expects($this->exactly(3)) ->method('setParameter') - ->withConsecutive(['userId', 'jack'], ['offset', 10]) + ->withConsecutive(['userId', 'jack'], ['offset', 10], ['starred', true]) ->will($this->returnSelf()); @@ -432,13 +432,19 @@ class ItemMapperPaginatedTest extends MapperTestUtility ['items.search_index LIKE :term0'], ['items.search_index LIKE :term1'], ['items.id < :offset'], - ['items.starred = 1'] + ['items.starred = :starred'] ) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(4)) + $this->builder->expects($this->exactly(5)) ->method('setParameter') - ->withConsecutive(['userId', 'jack'], ['term0', '%key%'], ['term1', '%word%'], ['offset', 10]) + ->withConsecutive( + ['userId', 'jack'], + ['term0', '%key%'], + ['term1', '%word%'], + ['offset', 10], + ['starred', true] + ) ->will($this->returnSelf()); @@ -721,13 +727,13 @@ class ItemMapperPaginatedTest extends MapperTestUtility ['feeds.user_id = :userId'], ['items.feed_id = :feedId'], ['items.id < :offset'], - ['items.unread = 1'] + ['items.unread = :unread'] ) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(3)) + $this->builder->expects($this->exactly(4)) ->method('setParameter') - ->withConsecutive(['userId', 'jack'], ['feedId', 2], ['offset', 10]) + ->withConsecutive(['userId', 'jack'], ['feedId', 2], ['offset', 10], ['unread', true]) ->will($this->returnSelf()); @@ -1059,13 +1065,13 @@ class ItemMapperPaginatedTest extends MapperTestUtility ['feeds.deleted_at = 0'], ['x IS NULL'], ['items.id < :offset'], - ['items.unread = 1'] + ['items.unread = :unread'] ) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(2)) + $this->builder->expects($this->exactly(3)) ->method('setParameter') - ->withConsecutive(['userId', 'jack'], ['offset', 10]) + ->withConsecutive(['userId', 'jack'], ['offset', 10], ['unread', true]) ->will($this->returnSelf()); @@ -1145,13 +1151,13 @@ class ItemMapperPaginatedTest extends MapperTestUtility ['feeds.deleted_at = 0'], ['x IS NULL'], ['items.id > :offset'], - ['items.unread = 1'] + ['items.unread = :unread'] ) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(2)) + $this->builder->expects($this->exactly(3)) ->method('setParameter') - ->withConsecutive(['userId', 'jack'], ['offset', 10]) + ->withConsecutive(['userId', 'jack'], ['offset', 10], ['unread', true]) ->will($this->returnSelf()); -- cgit v1.2.3