From 78dce7ffe18665bf083ff69631db8ae128a2b99f Mon Sep 17 00:00:00 2001 From: Sean Molenaar Date: Mon, 1 Mar 2021 21:10:46 +0100 Subject: DB: Updates should use set() Issue GH-1211 Signed-off-by: Sean Molenaar --- CHANGELOG.md | 1 + appinfo/info.xml | 3 + lib/Command/Debug/FeedRead.php | 85 ++++++++++++++++ lib/Command/Debug/FolderRead.php | 84 ++++++++++++++++ lib/Command/Debug/ItemRead.php | 87 ++++++++++++++++ lib/Db/FeedMapperV2.php | 32 ++++-- lib/Db/FolderMapperV2.php | 43 +++++--- lib/Db/ItemMapperV2.php | 36 +++++-- lib/Service/FeedServiceV2.php | 6 +- lib/Service/FolderServiceV2.php | 6 +- lib/Service/ItemServiceV2.php | 6 +- tests/Unit/Db/FeedMapperTest.php | 161 ++++++++++++++++++++++++++---- tests/Unit/Db/FolderMapperTest.php | 160 +++++++++++++++++++++++++---- tests/Unit/Db/ItemMapperAfterTest.php | 20 +--- tests/Unit/Db/ItemMapperPaginatedTest.php | 18 +--- tests/Unit/Db/ItemMapperTest.php | 83 ++++++++++++--- tests/Unit/Db/MapperTestUtility.php | 1 - tests/integration/feeds.bats | 21 +++- tests/integration/folders.bats | 17 +++- 19 files changed, 741 insertions(+), 129 deletions(-) create mode 100644 lib/Command/Debug/FeedRead.php create mode 100644 lib/Command/Debug/FolderRead.php create mode 100644 lib/Command/Debug/ItemRead.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 60ca83327..2f3de31b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is almost based on [Keep a Changelog](https://keepachangelog.com/en/1 ### Fixed - Do not show deleted feeds in item list +- Fix update queries (#1211) ## [15.4.0-beta2] - 2021-02-27 ### Fixed diff --git a/appinfo/info.xml b/appinfo/info.xml index cef07f249..bb828e950 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -73,6 +73,9 @@ Before you update to a new version, [check the changelog](https://github.com/nex OCA\News\Command\Debug\ItemList OCA\News\Command\Debug\FolderItemList OCA\News\Command\Debug\FeedItemList + OCA\News\Command\Debug\ItemRead + OCA\News\Command\Debug\FolderRead + OCA\News\Command\Debug\FeedRead diff --git a/lib/Command/Debug/FeedRead.php b/lib/Command/Debug/FeedRead.php new file mode 100644 index 000000000..eaf7f6b14 --- /dev/null +++ b/lib/Command/Debug/FeedRead.php @@ -0,0 +1,85 @@ +feedService = $feedService; + $this->itemService = $itemService; + } + + /** + * Configure command + * + * @return void + */ + protected function configure() + { + $this->setName('news:feed:read') + ->setDescription('Read feed') + ->addArgument('user-id', InputArgument::REQUIRED, 'User to modify the feed for') + ->addArgument('id', InputArgument::REQUIRED, 'Feed ID'); + } + + /** + * Execute command + * + * @param InputInterface $input + * @param OutputInterface $output + * + * @return int|void + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $user = $input->getArgument('user-id'); + + $id = $input->getArgument('id'); + if (!is_numeric($id)) { + $output->writeln('Invalid id!'); + return 255; + } + + try { + $read = $this->feedService->read($user, intval($id)); + $output->writeln("Marked $read items as read", $output::VERBOSITY_VERBOSE); + } catch (ServiceConflictException | ServiceNotFoundException $e) { + $output->writeln('Failed: ' . $e->getMessage()); + return 0; + } + + return 0; + } +} diff --git a/lib/Command/Debug/FolderRead.php b/lib/Command/Debug/FolderRead.php new file mode 100644 index 000000000..a259aceb6 --- /dev/null +++ b/lib/Command/Debug/FolderRead.php @@ -0,0 +1,84 @@ +folderService = $folderService; + $this->itemService = $itemService; + } + + /** + * Configure command + * + * @return void + */ + protected function configure() + { + $this->setName('news:folder:read') + ->setDescription('Read folder') + ->addArgument('user-id', InputArgument::REQUIRED, 'User to modify the folder for') + ->addArgument('id', InputArgument::REQUIRED, 'Folder ID'); + } + + /** + * Execute command + * + * @param InputInterface $input + * @param OutputInterface $output + * + * @return int|void + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $user = $input->getArgument('user-id'); + + $id = $input->getArgument('id'); + if (!is_numeric($id)) { + $output->writeln('Invalid id!'); + return 255; + } + + try { + $read = $this->folderService->read($user, intval($id)); + $output->writeln("Marked $read items as read", $output::VERBOSITY_VERBOSE); + } catch (ServiceConflictException | ServiceNotFoundException $e) { + $output->writeln('Failed: ' . $e->getMessage()); + return 0; + } + + return 0; + } +} diff --git a/lib/Command/Debug/ItemRead.php b/lib/Command/Debug/ItemRead.php new file mode 100644 index 000000000..a36efe3ec --- /dev/null +++ b/lib/Command/Debug/ItemRead.php @@ -0,0 +1,87 @@ +itemService = $itemService; + } + + /** + * Configure command + * + * @return void + */ + protected function configure() + { + $this->setName('news:item:read') + ->setDescription('Read item') + ->addArgument('user-id', InputArgument::REQUIRED, 'User to modify the item for') + ->addOption('id', 'i', InputOption::VALUE_REQUIRED, 'Item ID') + ->addOption('read', 'r', InputOption::VALUE_NONE, 'Item read state'); + } + + /** + * Execute command + * + * @param InputInterface $input + * @param OutputInterface $output + * + * @return int|void + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $user = $input->getArgument('user-id'); + $read = $input->getOption('read'); + + $id = $input->getOption('id'); + if (!is_null($id) && !is_numeric($id)) { + $output->writeln('Invalid id!'); + return 255; + } + + + try { + if (is_null($id)) { + $readItems = $this->itemService->readAll($user, $this->itemService->newest($user)->getId()); + $output->writeln("Marked $readItems items as read", $output::VERBOSITY_VERBOSE); + } else { + $items = $this->itemService->read($user, intval($id), $read); + $output->writeln(json_encode($this->serialize($items), JSON_PRETTY_PRINT)); + } + } catch (ServiceConflictException | ServiceNotFoundException $e) { + $output->writeln('Failed: ' . $e->getMessage()); + return 1; + } + + return 0; + } +} diff --git a/lib/Db/FeedMapperV2.php b/lib/Db/FeedMapperV2.php index 5e346732c..eaa9030cc 100644 --- a/lib/Db/FeedMapperV2.php +++ b/lib/Db/FeedMapperV2.php @@ -16,6 +16,7 @@ namespace OCA\News\Db; use OCA\News\Utility\Time; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\DB\Exception as DBException; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\AppFramework\Db\Entity; @@ -158,23 +159,40 @@ class FeedMapperV2 extends NewsMapperV2 * @param string $userId * @param int $id * @param int|null $maxItemID + * + * @return int + * @throws DBException + * + * @TODO Update for NC 21 */ - public function read(string $userId, int $id, ?int $maxItemID = null): void + public function read(string $userId, int $id, ?int $maxItemID = null): int { - $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); if ($maxItemID !== null) { - $builder->andWhere('items.id =< :maxItemId') - ->setParameter('maxItemId', $maxItemID); + $idBuilder->andWhere('items.id <= :maxItemId') + ->setParameter('maxItemId', $maxItemID); } - $this->db->executeUpdate($builder->getSQL()); + $idList = array_map(function ($value): int { + return intval($value['id']); + }, $this->db->executeQuery($idBuilder->getSQL(), $idBuilder->getParameters())->fetchAll()); + + $builder = $this->db->getQueryBuilder(); + $builder->update(ItemMapperV2::TABLE_NAME) + ->set('unread', $builder->createParameter('unread')) + ->andWhere('id IN (:idList)') + ->andWhere('unread != :unread') + ->setParameter('unread', false, IQueryBuilder::PARAM_BOOL) + ->setParameter('idList', $idList, IQueryBuilder::PARAM_INT_ARRAY); + + return $this->db->executeUpdate($builder->getSQL(), $builder->getParameters(), $builder->getParameterTypes()); } } diff --git a/lib/Db/FolderMapperV2.php b/lib/Db/FolderMapperV2.php index 12fa26887..d0d0cbec1 100644 --- a/lib/Db/FolderMapperV2.php +++ b/lib/Db/FolderMapperV2.php @@ -15,6 +15,8 @@ namespace OCA\News\Db; use OCA\News\Utility\Time; use OCP\AppFramework\Db\Entity; +use OCP\DB\Exception as DBException; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; /** @@ -101,24 +103,39 @@ class FolderMapperV2 extends NewsMapperV2 * @param int $id * @param int|null $maxItemID * - * @return void + * @return int + * + * @throws DBException + * @TODO Update for NC 21 */ - public function read(string $userId, int $id, ?int $maxItemID = null): void + public function read(string $userId, int $id, ?int $maxItemID = null): int { - $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); + $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); if ($maxItemID !== null) { - $builder->andWhere('items.id =< :maxItemId') - ->setParameter('maxItemId', $maxItemID); + $idBuilder->andWhere('items.id <= :maxItemId') + ->setParameter('maxItemId', $maxItemID); } - $this->db->executeUpdate($builder->getSQL()); + $idList = array_map(function ($value): int { + return intval($value['id']); + }, $this->db->executeQuery($idBuilder->getSQL(), $idBuilder->getParameters())->fetchAll()); + + $builder = $this->db->getQueryBuilder(); + $builder->update(ItemMapperV2::TABLE_NAME) + ->set('unread', $builder->createParameter('unread')) + ->andWhere('id IN (:idList)') + ->andWhere('unread != :unread') + ->setParameter('unread', false, IQueryBuilder::PARAM_BOOL) + ->setParameter('idList', $idList, IQueryBuilder::PARAM_INT_ARRAY); + + return $this->db->executeUpdate($builder->getSQL(), $builder->getParameters(), $builder->getParameterTypes()); } } diff --git a/lib/Db/ItemMapperV2.php b/lib/Db/ItemMapperV2.php index a8b101719..c79ac035f 100644 --- a/lib/Db/ItemMapperV2.php +++ b/lib/Db/ItemMapperV2.php @@ -19,6 +19,7 @@ use OCA\News\Utility\Time; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\DB\Exception as DBException; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; @@ -254,20 +255,35 @@ class ItemMapperV2 extends NewsMapperV2 * @param int $maxItemId * * @TODO: Update this for NC 21 + * + * @return int + * + * @throws DBException */ - public function readAll(string $userId, int $maxItemId): void + public function readAll(string $userId, int $maxItemId): int { - $builder = $this->db->getQueryBuilder(); + $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('items.id <= :maxItemId') + ->setParameter('userId', $userId) + ->setParameter('maxItemId', $maxItemId); - $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') - ->setParameter('maxItemId', $maxItemId) - ->setParameter('userId', $userId); + $idList = array_map(function ($value): int { + return intval($value['id']); + }, $this->db->executeQuery($idBuilder->getSQL(), $idBuilder->getParameters())->fetchAll()); - $this->db->executeUpdate($builder->getSQL()); + $builder = $this->db->getQueryBuilder(); + $builder->update(self::TABLE_NAME) + ->set('unread', $builder->createParameter('unread')) + ->andWhere('id IN (:idList)') + ->andWhere('unread != :unread') + ->setParameter('unread', false, IQueryBuilder::PARAM_BOOL) + ->setParameter('idList', $idList, IQueryBuilder::PARAM_INT_ARRAY); + + return $this->db->executeUpdate($builder->getSQL(), $builder->getParameters(), $builder->getParameterTypes()); } /** diff --git a/lib/Service/FeedServiceV2.php b/lib/Service/FeedServiceV2.php index 9d577db97..d15dfceb7 100644 --- a/lib/Service/FeedServiceV2.php +++ b/lib/Service/FeedServiceV2.php @@ -347,13 +347,15 @@ class FeedServiceV2 extends Service * @param int $id Feed ID * @param int|null $maxItemID Highest item ID to mark as read * + * @return int + * * @throws ServiceConflictException * @throws ServiceNotFoundException */ - public function read(string $userId, int $id, ?int $maxItemID = null): void + public function read(string $userId, int $id, ?int $maxItemID = null): int { $feed = $this->find($userId, $id); - $this->mapper->read($userId, $feed->getId(), $maxItemID); + return $this->mapper->read($userId, $feed->getId(), $maxItemID); } } diff --git a/lib/Service/FolderServiceV2.php b/lib/Service/FolderServiceV2.php index d13b4afc0..17d955876 100644 --- a/lib/Service/FolderServiceV2.php +++ b/lib/Service/FolderServiceV2.php @@ -186,13 +186,15 @@ class FolderServiceV2 extends Service * @param int $id Folder ID * @param int|null $maxItemID Highest item ID to mark as read * + * @return int + * * @throws ServiceConflictException * @throws ServiceNotFoundException */ - public function read(string $userId, int $id, ?int $maxItemID = null): void + public function read(string $userId, int $id, ?int $maxItemID = null): int { $folder = $this->find($userId, $id); - $this->mapper->read($userId, $folder->getId(), $maxItemID); + return $this->mapper->read($userId, $folder->getId(), $maxItemID); } } diff --git a/lib/Service/ItemServiceV2.php b/lib/Service/ItemServiceV2.php index fafd5e558..57804a8c3 100644 --- a/lib/Service/ItemServiceV2.php +++ b/lib/Service/ItemServiceV2.php @@ -217,11 +217,11 @@ class ItemServiceV2 extends Service * @param string $userId Item owner * @param int $maxItemId * - * @return void + * @return int */ - public function readAll(string $userId, int $maxItemId): void + public function readAll(string $userId, int $maxItemId): int { - $this->mapper->readAll($userId, $maxItemId); + return $this->mapper->readAll($userId, $maxItemId); } /** diff --git a/tests/Unit/Db/FeedMapperTest.php b/tests/Unit/Db/FeedMapperTest.php index 780bfbc38..ad2417b53 100644 --- a/tests/Unit/Db/FeedMapperTest.php +++ b/tests/Unit/Db/FeedMapperTest.php @@ -13,12 +13,15 @@ namespace OCA\News\Tests\Unit\Db; +use OC\DB\QueryBuilder\Parameter; +use OC\DB\ResultAdapter; use OCA\News\Db\Feed; use OCA\News\Db\FeedMapperV2; use OCA\News\Utility\Time; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\QueryBuilder\IFunctionBuilder; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\QueryBuilder\IQueryFunction; class FeedMapperTest extends MapperTestUtility @@ -457,39 +460,96 @@ class FeedMapperTest extends MapperTestUtility */ public function testRead() { - $this->db->expects($this->once()) + $selectbuilder = $this->getMockBuilder(IQueryBuilder::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->db->expects($this->exactly(2)) ->method('getQueryBuilder') - ->willReturn($this->builder); + ->willReturnOnConsecutiveCalls($selectbuilder, $this->builder); - $this->builder->expects($this->once()) - ->method('update') + $selectbuilder->expects($this->once()) + ->method('select') + ->with('items.id') + ->will($this->returnSelf()); + + $selectbuilder->expects($this->once()) + ->method('from') ->with('news_items', 'items') ->will($this->returnSelf()); - $this->builder->expects($this->once()) + $selectbuilder->expects($this->once()) ->method('innerJoin') ->with('items', 'news_feeds', 'feeds', 'items.feed_id = feeds.id') ->will($this->returnSelf()); + $selectbuilder->expects($this->exactly(2)) + ->method('andWhere') + ->withConsecutive(['feeds.user_id = :userId'], ['feeds.id = :feedId']) + ->will($this->returnSelf()); + + $selectbuilder->expects($this->exactly(2)) + ->method('setParameter') + ->withConsecutive(['userId', 'admin'], ['feedId', 1]) + ->will($this->returnSelf()); + + $selectbuilder->expects($this->exactly(1)) + ->method('getSQL') + ->will($this->returnValue('SQL QUERY')); + + $selectbuilder->expects($this->exactly(1)) + ->method('getParameters') + ->will($this->returnValue([])); + + $result = $this->getMockBuilder(ResultAdapter::class) + ->disableOriginalConstructor() + ->getMock(); + + $result->expects($this->once()) + ->method('fetchAll') + ->willReturn([['id' => 1], ['id' => 2]]); + + $this->db->expects($this->exactly(1)) + ->method('executeQuery') + ->with('SQL QUERY') + ->willReturn($result); + + $this->builder->expects($this->once()) + ->method('update') + ->with('news_items') + ->will($this->returnSelf()); + + $this->builder->expects($this->once()) + ->method('createParameter') + ->will($this->returnArgument(0)); + $this->builder->expects($this->once()) - ->method('setValue') - ->with('unread', 0) + ->method('set') + ->with('unread', 'unread') ->will($this->returnSelf()); $this->builder->expects($this->exactly(2)) ->method('andWhere') - ->withConsecutive(['feeds.user_id = :userId'], ['feeds.id = :feedId']) + ->withConsecutive(['id IN (:idList)'], ['unread != :unread']) ->will($this->returnSelf()); $this->builder->expects($this->exactly(2)) ->method('setParameter') - ->withConsecutive(['userId', 'admin'], ['feedId', 1]) + ->withConsecutive(['unread', false], ['idList', [1, 2]]) ->will($this->returnSelf()); $this->builder->expects($this->exactly(1)) ->method('getSQL') ->will($this->returnValue('QUERY')); + $this->builder->expects($this->exactly(1)) + ->method('getParameters') + ->will($this->returnValue([])); + + $this->builder->expects($this->exactly(1)) + ->method('getParameterTypes') + ->will($this->returnValue([])); + $this->db->expects($this->exactly(1)) ->method('executeUpdate') ->with('QUERY'); @@ -500,45 +560,102 @@ class FeedMapperTest extends MapperTestUtility /** * @covers \OCA\News\Db\FeedMapperV2::read */ - public function testReadWithMaxId() + public function testReadWithMaxID() { - $this->db->expects($this->once()) + $selectbuilder = $this->getMockBuilder(IQueryBuilder::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->db->expects($this->exactly(2)) ->method('getQueryBuilder') - ->willReturn($this->builder); + ->willReturnOnConsecutiveCalls($selectbuilder, $this->builder); - $this->builder->expects($this->once()) - ->method('update') + $selectbuilder->expects($this->once()) + ->method('select') + ->with('items.id') + ->will($this->returnSelf()); + + $selectbuilder->expects($this->once()) + ->method('from') ->with('news_items', 'items') ->will($this->returnSelf()); - $this->builder->expects($this->once()) + $selectbuilder->expects($this->once()) ->method('innerJoin') ->with('items', 'news_feeds', 'feeds', 'items.feed_id = feeds.id') ->will($this->returnSelf()); + $selectbuilder->expects($this->exactly(3)) + ->method('andWhere') + ->withConsecutive(['feeds.user_id = :userId'], ['feeds.id = :feedId'], ['items.id <= :maxItemId']) + ->will($this->returnSelf()); + + $selectbuilder->expects($this->exactly(3)) + ->method('setParameter') + ->withConsecutive(['userId', 'admin'], ['feedId', 1], ['maxItemId', 4]) + ->will($this->returnSelf()); + + $selectbuilder->expects($this->exactly(1)) + ->method('getSQL') + ->will($this->returnValue('SQL QUERY')); + + $selectbuilder->expects($this->exactly(1)) + ->method('getParameters') + ->will($this->returnValue([])); + + $result = $this->getMockBuilder(ResultAdapter::class) + ->disableOriginalConstructor() + ->getMock(); + + $result->expects($this->once()) + ->method('fetchAll') + ->willReturn([['id' => 1], ['id' => 2]]); + + $this->db->expects($this->exactly(1)) + ->method('executeQuery') + ->with('SQL QUERY') + ->willReturn($result); + + $this->builder->expects($this->once()) + ->method('createParameter') + ->will($this->returnArgument(0)); + + $this->builder->expects($this->once()) + ->method('update') + ->with('news_items') + ->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)) + $this->builder->expects($this->exactly(2)) ->method('andWhere') - ->withConsecutive(['feeds.user_id = :userId'], ['feeds.id = :feedId'], ['items.id =< :maxItemId']) + ->withConsecutive(['id IN (:idList)'], ['unread != :unread']) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(3)) + $this->builder->expects($this->exactly(2)) ->method('setParameter') - ->withConsecutive(['userId', 'admin'], ['feedId', 1], ['maxItemId', 4]) + ->withConsecutive(['unread', false], ['idList', [1, 2]]) ->will($this->returnSelf()); $this->builder->expects($this->exactly(1)) ->method('getSQL') ->will($this->returnValue('QUERY')); + $this->builder->expects($this->exactly(1)) + ->method('getParameters') + ->will($this->returnValue([])); + + $this->builder->expects($this->exactly(1)) + ->method('getParameterTypes') + ->will($this->returnValue([])); + $this->db->expects($this->exactly(1)) ->method('executeUpdate') ->with('QUERY'); $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 026c16bc6..ad44d618f 100644 --- a/tests/Unit/Db/FolderMapperTest.php +++ b/tests/Unit/Db/FolderMapperTest.php @@ -13,11 +13,14 @@ namespace OCA\News\Tests\Unit\Db; +use OC\DB\QueryBuilder\Parameter; +use OC\DB\ResultAdapter; use OCA\News\Db\Folder; use OCA\News\Db\FolderMapperV2; use OCA\News\Utility\Time; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\DB\QueryBuilder\IQueryBuilder; class FolderMapperTest extends MapperTestUtility { @@ -285,39 +288,96 @@ class FolderMapperTest extends MapperTestUtility */ public function testRead() { - $this->db->expects($this->once()) + $selectbuilder = $this->getMockBuilder(IQueryBuilder::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->db->expects($this->exactly(2)) ->method('getQueryBuilder') - ->willReturn($this->builder); + ->willReturnOnConsecutiveCalls($selectbuilder, $this->builder); - $this->builder->expects($this->once()) - ->method('update') + $selectbuilder->expects($this->once()) + ->method('select') + ->with('items.id') + ->will($this->returnSelf()); + + $selectbuilder->expects($this->once()) + ->method('from') ->with('news_items', 'items') ->will($this->returnSelf()); - $this->builder->expects($this->once()) + $selectbuilder->expects($this->once()) ->method('innerJoin') ->with('items', 'news_feeds', 'feeds', 'items.feed_id = feeds.id') ->will($this->returnSelf()); + $selectbuilder->expects($this->exactly(2)) + ->method('andWhere') + ->withConsecutive(['feeds.user_id = :userId'], ['feeds.folder_id = :folderId']) + ->will($this->returnSelf()); + + $selectbuilder->expects($this->exactly(2)) + ->method('setParameter') + ->withConsecutive(['userId', 'admin'], ['folderId', 1]) + ->will($this->returnSelf()); + + $selectbuilder->expects($this->exactly(1)) + ->method('getSQL') + ->will($this->returnValue('SQL QUERY')); + + $selectbuilder->expects($this->exactly(1)) + ->method('getParameters') + ->will($this->returnValue([])); + + $result = $this->getMockBuilder(ResultAdapter::class) + ->disableOriginalConstructor() + ->getMock(); + + $result->expects($this->once()) + ->method('fetchAll') + ->willReturn([['id' => 1], ['id' => 2]]); + + $this->db->expects($this->exactly(1)) + ->method('executeQuery') + ->with('SQL QUERY') + ->willReturn($result); + + $this->builder->expects($this->once()) + ->method('createParameter') + ->will($this->returnArgument(0)); + + $this->builder->expects($this->once()) + ->method('update') + ->with('news_items') + ->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)) ->method('andWhere') - ->withConsecutive(['feeds.user_id = :userId'], ['feeds.folder_id = :folderId']) + ->withConsecutive(['id IN (:idList)'], ['unread != :unread']) ->will($this->returnSelf()); $this->builder->expects($this->exactly(2)) ->method('setParameter') - ->withConsecutive(['userId', 'admin'], ['folderId', 1]) + ->withConsecutive(['unread', false], ['idList', [1, 2]]) ->will($this->returnSelf()); $this->builder->expects($this->exactly(1)) ->method('getSQL') ->will($this->returnValue('QUERY')); + $this->builder->expects($this->exactly(1)) + ->method('getParameters') + ->will($this->returnValue([])); + + $this->builder->expects($this->exactly(1)) + ->method('getParameterTypes') + ->will($this->returnValue([])); + $this->db->expects($this->exactly(1)) ->method('executeUpdate') ->with('QUERY'); @@ -330,43 +390,101 @@ class FolderMapperTest extends MapperTestUtility */ public function testReadWithMaxId() { - $this->db->expects($this->once()) + + $selectbuilder = $this->getMockBuilder(IQueryBuilder::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->db->expects($this->exactly(2)) ->method('getQueryBuilder') - ->willReturn($this->builder); + ->willReturnOnConsecutiveCalls($selectbuilder, $this->builder); - $this->builder->expects($this->once()) - ->method('update') + $selectbuilder->expects($this->once()) + ->method('select') + ->with('items.id') + ->will($this->returnSelf()); + + $selectbuilder->expects($this->once()) + ->method('from') ->with('news_items', 'items') ->will($this->returnSelf()); - $this->builder->expects($this->once()) + $selectbuilder->expects($this->once()) ->method('innerJoin') ->with('items', 'news_feeds', 'feeds', 'items.feed_id = feeds.id') ->will($this->returnSelf()); + $selectbuilder->expects($this->exactly(3)) + ->method('andWhere') + ->withConsecutive(['feeds.user_id = :userId'], ['feeds.folder_id = :folderId'], ['items.id <= :maxItemId']) + ->will($this->returnSelf()); + + $selectbuilder->expects($this->exactly(3)) + ->method('setParameter') + ->withConsecutive(['userId', 'admin'], ['folderId', 1], ['maxItemId', 4]) + ->will($this->returnSelf()); + + $selectbuilder->expects($this->exactly(1)) + ->method('getSQL') + ->will($this->returnValue('SQL QUERY')); + + $selectbuilder->expects($this->exactly(1)) + ->method('getParameters') + ->will($this->returnValue([])); + + $result = $this->getMockBuilder(ResultAdapter::class) + ->disableOriginalConstructor() + ->getMock(); + + $result->expects($this->once()) + ->method('fetchAll') + ->willReturn([['id' => 1], ['id' => 2]]); + + $this->db->expects($this->exactly(1)) + ->method('executeQuery') + ->with('SQL QUERY') + ->willReturn($result); + + $this->builder->expects($this->once()) + ->method('createParameter') + ->will($this->returnArgument(0)); + + $this->builder->expects($this->once()) + ->method('update') + ->with('news_items') + ->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)) + $this->builder->expects($this->exactly(2)) ->method('andWhere') - ->withConsecutive(['feeds.user_id = :userId'], ['feeds.folder_id = :folderId'], ['items.id =< :maxItemId']) + ->withConsecutive(['id IN (:idList)'], ['unread != :unread']) ->will($this->returnSelf()); - $this->builder->expects($this->exactly(3)) + $this->builder->expects($this->exactly(2)) ->method('setParameter') - ->withConsecutive(['userId', 'admin'], ['folderId', 1], ['maxItemId', 4]) + ->withConsecutive(['unread', false], ['idList', [1, 2]]) ->will($this->returnSelf()); $this->builder->expects($this->exactly(1)) ->method('getSQL') ->will($this->returnValue('QUERY')); + $this->builder->expects($this->exactly(1)) + ->method('getParameters') + ->will($this->returnValue([])); + + $this->builder->expects($this->exactly(1)) + ->method('getParameterTypes') + ->will($this->returnValue([])); + $this->db->expects($this->exactly(1)) ->method('executeUpdate') ->with('QUERY'); $this->class->read('admin', 1, 4); } -} \ No newline at end of file +} diff --git a/tests/Unit/Db/ItemMapperAfterTest.php b/tests/Unit/Db/ItemMapperAfterTest.php index 5c536e1bf..5b0475522 100644 --- a/tests/Unit/Db/ItemMapperAfterTest.php +++ b/tests/Unit/Db/ItemMapperAfterTest.php @@ -13,24 +13,10 @@ namespace OCA\News\Tests\Unit\Db; -use OC\DB\QueryBuilder\Literal; -use OCA\News\Db\Feed; -use OCA\News\Db\FeedMapperV2; -use OCA\News\Db\Folder; use OCA\News\Db\Item; use OCA\News\Db\ItemMapperV2; -use OCA\News\Db\NewsMapperV2; use OCA\News\Service\Exceptions\ServiceValidationException; use OCA\News\Utility\Time; -use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Db\MultipleObjectsReturnedException; -use OCP\DB\IResult; -use OCP\DB\QueryBuilder\IExpressionBuilder; -use OCP\DB\QueryBuilder\IFunctionBuilder; -use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\DB\QueryBuilder\IQueryFunction; -use OCP\IDBConnection; -use Test\TestCase; /** * Class ItemMapperTest @@ -40,8 +26,6 @@ use Test\TestCase; class ItemMapperAfterTest extends MapperTestUtility { - /** @var Time */ - private $time; /** @var ItemMapperV2 */ private $class; @@ -51,10 +35,10 @@ class ItemMapperAfterTest extends MapperTestUtility protected function setUp(): void { parent::setUp(); - $this->time = $this->getMockBuilder(Time::class) + $time = $this->getMockBuilder(Time::class) ->getMock(); - $this->class = new ItemMapperV2($this->db, $this->time); + $this->class = new ItemMapperV2($this->db, $time); } public function testFindAllInFeedAfter() diff --git a/tests/Unit/Db/ItemMapperPaginatedTest.php b/tests/Unit/Db/ItemMapperPaginatedTest.php index e242833fb..6bd6906d3 100644 --- a/tests/Unit/Db/ItemMapperPaginatedTest.php +++ b/tests/Unit/Db/ItemMapperPaginatedTest.php @@ -14,23 +14,11 @@ namespace OCA\News\Tests\Unit\Db; use OC\DB\QueryBuilder\Literal; -use OCA\News\Db\Feed; -use OCA\News\Db\FeedMapperV2; -use OCA\News\Db\Folder; use OCA\News\Db\Item; use OCA\News\Db\ItemMapperV2; -use OCA\News\Db\NewsMapperV2; use OCA\News\Service\Exceptions\ServiceValidationException; use OCA\News\Utility\Time; -use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Db\MultipleObjectsReturnedException; -use OCP\DB\IResult; use OCP\DB\QueryBuilder\IExpressionBuilder; -use OCP\DB\QueryBuilder\IFunctionBuilder; -use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\DB\QueryBuilder\IQueryFunction; -use OCP\IDBConnection; -use Test\TestCase; /** * Class ItemMapperTest @@ -40,8 +28,6 @@ use Test\TestCase; class ItemMapperPaginatedTest extends MapperTestUtility { - /** @var Time */ - private $time; /** @var ItemMapperV2 */ private $class; @@ -51,10 +37,10 @@ class ItemMapperPaginatedTest extends MapperTestUtility protected function setUp(): void { parent::setUp(); - $this->time = $this->getMockBuilder(Time::class) + $time = $this->getMockBuilder(Time::class) ->getMock(); - $this->class = new ItemMapperV2($this->db, $this->time); + $this->class = new ItemMapperV2($this->db, $time); } public function testFindAllItemsInvalid() diff --git a/tests/Unit/Db/ItemMapperTest.php b/tests/Unit/Db/ItemMapperTest.php index 490f13e71..5cb42c3fa 100644 --- a/tests/Unit/Db/ItemMapperTest.php +++ b/tests/Unit/Db/ItemMapperTest.php @@ -14,6 +14,8 @@ namespace OCA\News\Tests\Unit\Db; use OC\DB\QueryBuilder\Literal; +use OC\DB\QueryBuilder\Parameter; +use OC\DB\ResultAdapter; use OCA\News\Db\Feed; use OCA\News\Db\FeedMapperV2; use OCA\News\Db\Folder; @@ -472,44 +474,101 @@ class ItemMapperTest extends MapperTestUtility public function testReadAll() { - $this->db->expects($this->once()) + $selectbuilder = $this->getMockBuilder(IQueryBuilder::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->db->expects($this->exactly(2)) ->method('getQueryBuilder') - ->willReturn($this->builder); + ->willReturnOnConsecutiveCalls($selectbuilder, $this->builder); - $this->builder->expects($this->once()) - ->method('update') + $selectbuilder->expects($this->once()) + ->method('select') + ->with('items.id') + ->will($this->returnSelf()); + + $selectbuilder->expects($this->once()) + ->method('from') ->with('news_items', 'items') ->will($this->returnSelf()); - $this->builder->expects($this->once()) + $selectbuilder->expects($this->once()) ->method('innerJoin') ->with('items', 'news_feeds', 'feeds', 'items.feed_id = feeds.id') ->will($this->returnSelf()); + $selectbuilder->expects($this->exactly(2)) + ->method('andWhere') + ->withConsecutive(['feeds.user_id = :userId'], ['items.id <= :maxItemId']) + ->will($this->returnSelf()); + + $selectbuilder->expects($this->exactly(2)) + ->method('setParameter') + ->withConsecutive(['userId', 'admin'], ['maxItemId', 4]) + ->will($this->returnSelf()); + + $selectbuilder->expects($this->exactly(1)) + ->method('getSQL') + ->will($this->returnValue('SQL QUERY')); + + $selectbuilder->expects($this->exactly(1)) + ->method('getParameters') + ->will($this->returnValue([])); + + $result = $this->getMockBuilder(ResultAdapter::class) + ->disableOriginalConstructor() + ->getMock(); + + $result->expects($this->once()) + ->method('fetchAll') + ->willReturn([['id' => 1], ['id' => 2]]); + + $this->db->expects($this->exactly(1)) + ->method('executeQuery') + ->with('SQL QUERY') + ->willReturn($result); + + $this->builder->expects($this->once()) + ->method('createParameter') + ->will($this->returnArgument(0)); + + $this->builder->expects($this->once()) + ->method('update') + ->with('news_items') + ->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)) ->method('andWhere') - ->withConsecutive(['items.id =< :maxItemId'], ['feeds.user_id = :userId']) + ->withConsecutive(['id IN (:idList)'], ['unread != :unread']) ->will($this->returnSelf()); $this->builder->expects($this->exactly(2)) ->method('setParameter') - ->withConsecutive(['maxItemId', 4], ['userId', 'jack']) + ->withConsecutive(['unread', false], ['idList', [1, 2]]) ->will($this->returnSelf()); $this->builder->expects($this->exactly(1)) ->method('getSQL') ->will($this->returnValue('QUERY')); - $this->db->expects($this->once()) + $this->builder->expects($this->exactly(1)) + ->method('getParameters') + ->will($this->returnValue([])); + + $this->builder->expects($this->exactly(1)) + ->method('getParameterTypes') + ->will($this->returnValue([])); + + $this->db->expects($this->exactly(1)) ->method('executeUpdate') ->with('QUERY'); - $this->class->readAll('jack', 4); + $this->class->readAll('admin', 4); } public function testPurgeDeletedEmpty() @@ -1055,4 +1114,4 @@ class ItemMapperTest extends MapperTestUtility $this->assertSame(10, $res); } -} \ No newline at end of file +} diff --git a/tests/Unit/Db/MapperTestUtility.php b/tests/Unit/Db/MapperTestUtility.php index 4a875fde5..b5ef782c2 100644 --- a/tests/Unit/Db/MapperTestUtility.php +++ b/tests/Unit/Db/MapperTestUtility.php @@ -24,7 +24,6 @@ namespace OCA\News\Tests\Unit\Db; use Doctrine\DBAL\Driver\Statement; -use OC\DB\QueryBuilder\QueryBuilder; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; diff --git a/tests/integration/feeds.bats b/tests/integration/feeds.bats index d8f0aad8c..2701cbfa9 100644 --- a/tests/integration/feeds.bats +++ b/tests/integration/feeds.bats @@ -57,6 +57,25 @@ teardown() { fi } +@test "[$TESTSUITE] Read all" { + ./occ news:feed:add "$user" "$NC_FEED" --title "Something-${BATS_SUITE_TEST_NUMBER}" + + run ./occ news:feed:list "$user" + [ "$status" -eq 0 ] + + echo "$output" | grep "Something-${BATS_SUITE_TEST_NUMBER}" + + ID=$(./occ news:feed:list 'admin' | grep "Something-${BATS_SUITE_TEST_NUMBER}" -2 | head -1 | grep -oE '[0-9]*') + run ./occ news:feed:read "$user" "$ID" -v + [ "$status" -eq 0 ] + + if ! echo "$output" | grep "items as read"; then + ret_status=$? + echo "Feed not read" + return $ret_status + fi +} + @test "[$TESTSUITE] Delete all" { ./occ news:feed:add "$user" "$NC_FEED" --title "Something-${BATS_SUITE_TEST_NUMBER}" @@ -68,4 +87,4 @@ teardown() { ID=$(./occ news:feed:list 'admin' | grep "Something-${BATS_SUITE_TEST_NUMBER}" -2 | head -1 | grep -oE '[0-9]*') run ./occ news:feed:delete "$user" "$ID" [ "$status" -eq 0 ] -} \ No newline at end of file +} diff --git a/tests/integration/folders.bats b/tests/integration/folders.bats index 9dde06861..48b15f05b 100644 --- a/tests/integration/folders.bats +++ b/tests/integration/folders.bats @@ -36,6 +36,21 @@ teardown() { fi } +@test "[$TESTSUITE] Read all" { + ./occ news:folder:add "$user" "Something-${BATS_SUITE_TEST_NUMBER}" + + ID=$(./occ news:folder:list 'admin' | grep "Something-${BATS_SUITE_TEST_NUMBER}" -1 | head -1 | grep -oE '[0-9]*') + + run ./occ news:folder:read "$user" "$ID" -v + [ "$status" -eq 0 ] + + if ! echo "$output" | grep "items as read"; then + ret_status=$? + echo "Folder not read" + return $ret_status + fi +} + @test "[$TESTSUITE] Delete all" { ID=$(./occ news:folder:add "$user" "Something-${BATS_SUITE_TEST_NUMBER}" | grep -oE '[0-9]*') @@ -46,4 +61,4 @@ teardown() { run ./occ news:folder:delete "$user" "$ID" [ "$status" -eq 0 ] -} \ No newline at end of file +} -- cgit v1.2.3