summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSean Molenaar <sean@seanmolenaar.eu>2021-03-01 21:10:46 +0100
committerSean Molenaar <SMillerDev@users.noreply.github.com>2021-03-31 22:39:00 +0200
commit78dce7ffe18665bf083ff69631db8ae128a2b99f (patch)
tree1f4e61970b7461afd968552f4bccef53daedbb93
parent17f773b723538eedf5bd2efcb8d2c85783d7050f (diff)
DB: Updates should use set()
Issue GH-1211 Signed-off-by: Sean Molenaar <sean@seanmolenaar.eu>
-rw-r--r--CHANGELOG.md1
-rw-r--r--appinfo/info.xml3
-rw-r--r--lib/Command/Debug/FeedRead.php85
-rw-r--r--lib/Command/Debug/FolderRead.php84
-rw-r--r--lib/Command/Debug/ItemRead.php87
-rw-r--r--lib/Db/FeedMapperV2.php32
-rw-r--r--lib/Db/FolderMapperV2.php43
-rw-r--r--lib/Db/ItemMapperV2.php36
-rw-r--r--lib/Service/FeedServiceV2.php6
-rw-r--r--lib/Service/FolderServiceV2.php6
-rw-r--r--lib/Service/ItemServiceV2.php6
-rw-r--r--tests/Unit/Db/FeedMapperTest.php161
-rw-r--r--tests/Unit/Db/FolderMapperTest.php160
-rw-r--r--tests/Unit/Db/ItemMapperAfterTest.php20
-rw-r--r--tests/Unit/Db/ItemMapperPaginatedTest.php18
-rw-r--r--tests/Unit/Db/ItemMapperTest.php83
-rw-r--r--tests/Unit/Db/MapperTestUtility.php1
-rw-r--r--tests/integration/feeds.bats21
-rw-r--r--tests/integration/folders.bats17
19 files changed, 741 insertions, 129 deletions
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
<command>OCA\News\Command\Debug\ItemList</command>
<command>OCA\News\Command\Debug\FolderItemList</command>
<command>OCA\News\Command\Debug\FeedItemList</command>
+ <command>OCA\News\Command\Debug\ItemRead</command>
+ <command>OCA\News\Command\Debug\FolderRead</command>
+ <command>OCA\News\Command\Debug\FeedRead</command>
</commands>
<settings>
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 @@
+<?php
+declare(strict_types=1);
+
+namespace OCA\News\Command\Debug;
+
+use OCA\News\Controller\ApiPayloadTrait;
+use OCA\News\Db\ListType;
+use OCA\News\Service\Exceptions\ServiceConflictException;
+use OCA\News\Service\Exceptions\ServiceNotFoundException;
+use OCA\News\Service\FeedServiceV2;
+use OCA\News\Service\FolderServiceV2;
+use OCA\News\Service\ItemServiceV2;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Output\OutputInterface;
+
+/**
+ * Class ItemRead
+ *
+ * @package OCA\News\Command
+ */
+class FeedRead extends Command
+{
+ /**
+ * @var FeedServiceV2 service for the folders.
+ */
+ protected $feedService;
+
+ /**
+ * @var ItemServiceV2 service for the items.
+ */
+ protected $itemService;
+
+ public function __construct(FeedServiceV2 $feedService, ItemServiceV2 $itemService)
+ {
+ parent::__construct();
+
+ $this->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 @@
+<?php
+declare(strict_types=1);
+
+namespace OCA\News\Command\Debug;
+
+use OCA\News\Controller\ApiPayloadTrait;
+use OCA\News\Db\ListType;
+use OCA\News\Service\Exceptions\ServiceConflictException;
+use OCA\News\Service\Exceptions\ServiceNotFoundException;
+use OCA\News\Service\FolderServiceV2;
+use OCA\News\Service\ItemServiceV2;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Output\OutputInterface;
+
+/**
+ * Class ItemRead
+ *
+ * @package OCA\News\Command
+ */
+class FolderRead extends Command
+{
+ /**
+ * @var FolderServiceV2 service for the folders.
+ */
+ protected $folderService;
+
+ /**
+ * @var ItemServiceV2 service for the items.
+ */
+ protected $itemService;
+
+ public function __construct(FolderServiceV2 $folderService, ItemServiceV2 $itemService)
+ {
+ parent::__construct();
+
+ $this->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 @@
+<?php
+declare(strict_types=1);
+
+namespace OCA\News\Command\Debug;
+
+use OCA\News\Controller\ApiPayloadTrait;
+use OCA\News\Db\ListType;
+use OCA\News\Service\Exceptions\ServiceConflictException;
+use OCA\News\Service\Exceptions\ServiceNotFoundException;
+use OCA\News\Service\ItemServiceV2;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Output\OutputInterface;
+
+/**
+ * Class ItemRead
+ *
+ * @package OCA\News\Command
+ */
+class ItemRead extends Command
+{
+ use ApiPayloadTrait;
+
+ /**
+ * @var ItemServiceV2 service for the items.
+ */
+ protected $itemService;
+
+ public function __construct(ItemServiceV2 $itemService)
+ {
+ parent::__construct();
+
+ $this->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')