diff --git a/CHANGELOG.md b/CHANGELOG.md index a7aff9a6d9..a02dacd0b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is almost based on [Keep a Changelog](https://keepachangelog.com/en/1 - Update serialization of item to include categories (#1248) ### Fixed +- Fix update queries (#1211) ## [15.4.0-beta2] - 2021-02-27 ### Fixed diff --git a/lib/Db/FeedMapperV2.php b/lib/Db/FeedMapperV2.php index 5e346732c3..b8a6d51134 100644 --- a/lib/Db/FeedMapperV2.php +++ b/lib/Db/FeedMapperV2.php @@ -13,12 +13,14 @@ namespace OCA\News\Db; +use OC\DB\QueryBuilder\Parameter; use OCA\News\Utility\Time; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\AppFramework\Db\Entity; +use PDO; /** * Class FeedMapper @@ -158,23 +160,34 @@ public function findAllFromFolder(?int $id): array * @param string $userId * @param int $id * @param int|null $maxItemID + * + * @throws \OCP\DB\Exception + * + * @TODO Update for NC 21 */ public function read(string $userId, int $id, ?int $maxItemID = null): void { - $builder = $this->db->getQueryBuilder(); - $builder->update(ItemMapperV2::TABLE_NAME, 'items') + $idBuilder = $this->db->getQueryBuilder(); + $idBuilder->select('items.id') + ->from(ItemMapperV2::TABLE_NAME, 'items') ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id') - ->setValue('unread', 0) ->andWhere('feeds.user_id = :userId') - ->andWhere('feeds.id = :feedId') - ->setParameter('userId', $userId) - ->setParameter('feedId', $id); + ->andWhere('feeds.id = :feedId'); + + $idList = $this->db->executeQuery($idBuilder->getSQL(), $idBuilder->getParameters()) + ->fetch(PDO::FETCH_COLUMN); + + $builder = $this->db->getQueryBuilder(); + $builder->update(ItemMapperV2::TABLE_NAME) + ->set('unread', $builder->createParameter('unread')) + ->where($builder->expr()->in('id', $idList)) + ->setParameter('unread', false); if ($maxItemID !== null) { - $builder->andWhere('items.id =< :maxItemId') + $builder->andWhere('id <= :maxItemId') ->setParameter('maxItemId', $maxItemID); } - $this->db->executeUpdate($builder->getSQL()); + $this->db->executeUpdate($builder->getSQL(), $builder->getParameters()); } } diff --git a/lib/Db/FolderMapperV2.php b/lib/Db/FolderMapperV2.php index 12fa268875..8759c6c2cd 100644 --- a/lib/Db/FolderMapperV2.php +++ b/lib/Db/FolderMapperV2.php @@ -16,6 +16,7 @@ use OCA\News\Utility\Time; use OCP\AppFramework\Db\Entity; use OCP\IDBConnection; +use PDO; /** * Class FolderMapper @@ -102,23 +103,35 @@ public function findFromUser(string $userId, int $id): Entity * @param int|null $maxItemID * * @return void + * @throws \OCP\DB\Exception + * + * @TODO Update for NC 21 */ public function read(string $userId, int $id, ?int $maxItemID = null): void { + $idBuilder = $this->db->getQueryBuilder(); + $idBuilder->select('items.id') + ->from(ItemMapperV2::TABLE_NAME, 'items') + ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id') + ->andWhere('feeds.user_id = :userId') + ->andWhere('feeds.folder_id = :folderId') + ->setParameter('userId', $userId) + ->setParameter('folderId', $id); + + $idList = $this->db->executeQuery($idBuilder->getSQL(), $idBuilder->getParameters()) + ->fetch(PDO::FETCH_COLUMN); + $builder = $this->db->getQueryBuilder(); - $builder->update(ItemMapperV2::TABLE_NAME, 'items') - ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id') - ->setValue('unread', 0) - ->andWhere('feeds.user_id = :userId') - ->andWhere('feeds.folder_id = :folderId') - ->setParameter('userId', $userId) - ->setParameter('folderId', $id); + $builder->update(ItemMapperV2::TABLE_NAME) + ->set('unread', $builder->createParameter('unread')) + ->where($builder->expr()->in('id', $idList)) + ->setParameter('unread', false); if ($maxItemID !== null) { - $builder->andWhere('items.id =< :maxItemId') + $builder->andWhere('id <= :maxItemId') ->setParameter('maxItemId', $maxItemID); } - $this->db->executeUpdate($builder->getSQL()); + $this->db->executeUpdate($builder->getSQL(), $builder->getParameters()); } } diff --git a/lib/Db/ItemMapperV2.php b/lib/Db/ItemMapperV2.php index 33b4611f91..b342e07e83 100644 --- a/lib/Db/ItemMapperV2.php +++ b/lib/Db/ItemMapperV2.php @@ -13,6 +13,7 @@ namespace OCA\News\Db; use OC\DB\QueryBuilder\Literal; +use OC\DB\QueryBuilder\Parameter; use OCA\News\Service\Exceptions\ServiceValidationException; use Doctrine\DBAL\FetchMode; use OCA\News\Utility\Time; @@ -254,20 +255,32 @@ public function purgeDeleted(?string $userID, ?int $oldestDelete): void * @param int $maxItemId * * @TODO: Update this for NC 21 + * + * @throws \OCP\DB\Exception */ public function readAll(string $userId, int $maxItemId): void { + $idBuilder = $this->db->getQueryBuilder(); + $idBuilder->select('id') + ->from(ItemMapperV2::TABLE_NAME, 'items') + ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id') + ->andWhere('feeds.user_id = :userId') + ->setParameter('userId', $userId); + + $idList = $this->db->executeQuery($idBuilder->getSQL(), $idBuilder->getParameters()) + ->fetch(PDO::FETCH_COLUMN); + $builder = $this->db->getQueryBuilder(); $builder->update($this->tableName, 'items') ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id') - ->setValue('unread', 0) - ->andWhere('items.id =< :maxItemId') - ->andWhere('feeds.user_id = :userId') + ->set('unread', $builder->createParameter('unread')) + ->andWhere('items.id <= :maxItemId') + ->where($builder->expr()->in('id', $idList)) ->setParameter('maxItemId', $maxItemId) - ->setParameter('userId', $userId); + ->setParameter('unread', false); - $this->db->executeUpdate($builder->getSQL()); + $this->db->executeUpdate($builder->getSQL(), $builder->getParameters()); } /** diff --git a/tests/Unit/Db/FeedMapperTest.php b/tests/Unit/Db/FeedMapperTest.php index 780bfbc386..4f0d90b0e0 100644 --- a/tests/Unit/Db/FeedMapperTest.php +++ b/tests/Unit/Db/FeedMapperTest.php @@ -13,6 +13,7 @@ namespace OCA\News\Tests\Unit\Db; +use OC\DB\QueryBuilder\Parameter; use OCA\News\Db\Feed; use OCA\News\Db\FeedMapperV2; use OCA\News\Utility\Time; @@ -466,14 +467,19 @@ public function testRead() ->with('news_items', 'items') ->will($this->returnSelf()); + $this->builder->expects($this->once()) + ->method('createParameter') + ->with('unread') + ->will($this->returnArgument(0)); + $this->builder->expects($this->once()) ->method('innerJoin') ->with('items', 'news_feeds', 'feeds', 'items.feed_id = feeds.id') ->will($this->returnSelf()); $this->builder->expects($this->once()) - ->method('setValue') - ->with('unread', 0) + ->method('set') + ->with('unread', 'unread') ->will($this->returnSelf()); $this->builder->expects($this->exactly(2)) @@ -481,9 +487,9 @@ public function testRead() ->withConsecutive(['feeds.user_id = :userId'], ['feeds.id = :feedId']) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(2)) + $this->builder->expects($this->exactly(3)) ->method('setParameter') - ->withConsecutive(['userId', 'admin'], ['feedId', 1]) + ->withConsecutive(['userId', 'admin'], ['feedId', 1], ['unread', false]) ->will($this->returnSelf()); $this->builder->expects($this->exactly(1)) @@ -506,6 +512,11 @@ public function testReadWithMaxId() ->method('getQueryBuilder') ->willReturn($this->builder); + $this->builder->expects($this->once()) + ->method('createParameter') + ->with('unread') + ->will($this->returnArgument(0)); + $this->builder->expects($this->once()) ->method('update') ->with('news_items', 'items') @@ -517,8 +528,8 @@ public function testReadWithMaxId() ->will($this->returnSelf()); $this->builder->expects($this->once()) - ->method('setValue') - ->with('unread', 0) + ->method('set') + ->with('unread', 'unread') ->will($this->returnSelf()); $this->builder->expects($this->exactly(3)) @@ -526,9 +537,9 @@ public function testReadWithMaxId() ->withConsecutive(['feeds.user_id = :userId'], ['feeds.id = :feedId'], ['items.id =< :maxItemId']) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(3)) + $this->builder->expects($this->exactly(4)) ->method('setParameter') - ->withConsecutive(['userId', 'admin'], ['feedId', 1], ['maxItemId', 4]) + ->withConsecutive(['userId', 'admin'], ['feedId', 1], ['unread', false], ['maxItemId', 4]) ->will($this->returnSelf()); $this->builder->expects($this->exactly(1)) @@ -541,4 +552,4 @@ public function testReadWithMaxId() $this->class->read('admin', 1, 4); } -} \ No newline at end of file +} diff --git a/tests/Unit/Db/FolderMapperTest.php b/tests/Unit/Db/FolderMapperTest.php index 026c16bc68..9eabc6f996 100644 --- a/tests/Unit/Db/FolderMapperTest.php +++ b/tests/Unit/Db/FolderMapperTest.php @@ -13,6 +13,7 @@ namespace OCA\News\Tests\Unit\Db; +use OC\DB\QueryBuilder\Parameter; use OCA\News\Db\Folder; use OCA\News\Db\FolderMapperV2; use OCA\News\Utility\Time; @@ -289,6 +290,11 @@ public function testRead() ->method('getQueryBuilder') ->willReturn($this->builder); + $this->builder->expects($this->once()) + ->method('createParameter') + ->with('unread') + ->will($this->returnArgument(0)); + $this->builder->expects($this->once()) ->method('update') ->with('news_items', 'items') @@ -300,8 +306,8 @@ public function testRead() ->will($this->returnSelf()); $this->builder->expects($this->once()) - ->method('setValue') - ->with('unread', 0) + ->method('set') + ->with('unread', 'unread') ->will($this->returnSelf()); $this->builder->expects($this->exactly(2)) @@ -309,9 +315,9 @@ public function testRead() ->withConsecutive(['feeds.user_id = :userId'], ['feeds.folder_id = :folderId']) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(2)) + $this->builder->expects($this->exactly(3)) ->method('setParameter') - ->withConsecutive(['userId', 'admin'], ['folderId', 1]) + ->withConsecutive(['userId', 'admin'], ['folderId', 1], ['unread', false]) ->will($this->returnSelf()); $this->builder->expects($this->exactly(1)) @@ -334,6 +340,11 @@ public function testReadWithMaxId() ->method('getQueryBuilder') ->willReturn($this->builder); + $this->builder->expects($this->once()) + ->method('createParameter') + ->with('unread') + ->will($this->returnArgument(0)); + $this->builder->expects($this->once()) ->method('update') ->with('news_items', 'items') @@ -345,8 +356,8 @@ public function testReadWithMaxId() ->will($this->returnSelf()); $this->builder->expects($this->once()) - ->method('setValue') - ->with('unread', 0) + ->method('set') + ->with('unread', 'unread') ->will($this->returnSelf()); $this->builder->expects($this->exactly(3)) @@ -354,9 +365,9 @@ public function testReadWithMaxId() ->withConsecutive(['feeds.user_id = :userId'], ['feeds.folder_id = :folderId'], ['items.id =< :maxItemId']) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(3)) + $this->builder->expects($this->exactly(4)) ->method('setParameter') - ->withConsecutive(['userId', 'admin'], ['folderId', 1], ['maxItemId', 4]) + ->withConsecutive(['userId', 'admin'], ['folderId', 1], ['unread', false], ['maxItemId', 4]) ->will($this->returnSelf()); $this->builder->expects($this->exactly(1)) @@ -369,4 +380,4 @@ public function testReadWithMaxId() $this->class->read('admin', 1, 4); } -} \ No newline at end of file +} diff --git a/tests/Unit/Db/ItemMapperTest.php b/tests/Unit/Db/ItemMapperTest.php index 490f13e712..d5def69ea8 100644 --- a/tests/Unit/Db/ItemMapperTest.php +++ b/tests/Unit/Db/ItemMapperTest.php @@ -14,6 +14,7 @@ namespace OCA\News\Tests\Unit\Db; use OC\DB\QueryBuilder\Literal; +use OC\DB\QueryBuilder\Parameter; use OCA\News\Db\Feed; use OCA\News\Db\FeedMapperV2; use OCA\News\Db\Folder; @@ -476,6 +477,11 @@ public function testReadAll() ->method('getQueryBuilder') ->willReturn($this->builder); + $this->builder->expects($this->once()) + ->method('createParameter') + ->with('unread') + ->will($this->returnArgument(0)); + $this->builder->expects($this->once()) ->method('update') ->with('news_items', 'items') @@ -487,8 +493,8 @@ public function testReadAll() ->will($this->returnSelf()); $this->builder->expects($this->once()) - ->method('setValue') - ->with('unread', 0) + ->method('set') + ->with('unread', 'unread') ->will($this->returnSelf()); $this->builder->expects($this->exactly(2)) @@ -496,9 +502,9 @@ public function testReadAll() ->withConsecutive(['items.id =< :maxItemId'], ['feeds.user_id = :userId']) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(2)) + $this->builder->expects($this->exactly(3)) ->method('setParameter') - ->withConsecutive(['maxItemId', 4], ['userId', 'jack']) + ->withConsecutive(['maxItemId', 4], ['userId', 'jack'], ['unread', false]) ->will($this->returnSelf()); $this->builder->expects($this->exactly(1)) @@ -1055,4 +1061,4 @@ public function testDeleteOverThresholdSuccessUnreadSkipsIfUnderThreshold() $this->assertSame(10, $res); } -} \ No newline at end of file +}