From d00d1ab2a28f428223e52b17052c072c64784016 Mon Sep 17 00:00:00 2001 From: Sean Molenaar Date: Sat, 29 Aug 2020 23:39:35 +0200 Subject: [PATCH] Create V2 mapper, Service and management commands Signed-off-by: Sean Molenaar --- .github/workflows/integration-tests.yml | 7 + AUTHORS.md | 1 + appinfo/info.xml | 7 + composer.json | 3 +- composer.lock | 15 +- lib/Command/Config/FeedAdd.php | 72 ++++ lib/Command/Config/FeedDelete.php | 56 +++ lib/Command/Config/FeedList.php | 61 ++++ lib/Command/Config/FolderAdd.php | 58 +++ lib/Command/Config/FolderDelete.php | 61 ++++ lib/Command/Config/FolderList.php | 62 ++++ lib/Command/ExploreGenerator.php | 3 +- lib/Command/ShowFeed.php | 3 +- lib/Command/Updater/AfterUpdate.php | 34 +- lib/Command/Updater/AllFeeds.php | 20 +- lib/Command/Updater/BeforeUpdate.php | 20 +- lib/Command/Updater/UpdateFeed.php | 23 +- lib/Controller/ApiPayloadTrait.php | 36 ++ lib/Controller/EntityApiSerializer.php | 6 + lib/Controller/FeedApiController.php | 35 +- lib/Controller/FeedController.php | 12 +- lib/Controller/FolderApiController.php | 6 +- lib/Controller/FolderController.php | 6 +- lib/Controller/ItemApiController.php | 10 +- lib/Controller/ItemController.php | 6 +- lib/Controller/JSONHttpError.php | 7 +- lib/Controller/UtilityApiController.php | 21 +- lib/Cron/{Updater.php => UpdaterJob.php} | 18 +- lib/Db/Feed.php | 115 ++++-- lib/Db/FeedMapper.php | 29 +- lib/Db/FeedMapperV2.php | 141 ++++++++ lib/Db/Folder.php | 7 +- lib/Db/FolderMapper.php | 38 +- lib/Db/FolderMapperV2.php | 87 +++++ lib/Db/Item.php | 6 + lib/Db/ItemMapper.php | 52 ++- lib/Db/ItemMapperV2.php | 124 +++++++ lib/Db/MapperFactory.php | 6 + lib/Db/Mysql/ItemMapper.php | 6 + lib/Db/NewsMapper.php | 84 ++++- lib/Db/NewsMapperV2.php | 108 ++++++ lib/Fetcher/FeedFetcher.php | 2 +- lib/Fetcher/IFeedFetcher.php | 2 +- lib/Fetcher/YoutubeFetcher.php | 8 +- .../ServiceConflictException.php | 2 +- .../{ => Exceptions}/ServiceException.php | 2 +- .../ServiceNotFoundException.php | 2 +- .../ServiceValidationException.php | 2 +- lib/Service/FeedService.php | 61 ++-- lib/Service/FeedServiceV2.php | 336 ++++++++++++++++++ lib/Service/FolderService.php | 97 +++-- lib/Service/FolderServiceV2.php | 102 ++++++ lib/Service/ItemService.php | 30 +- lib/Service/ItemServiceV2.php | 104 ++++++ lib/Service/Service.php | 70 +++- lib/Service/StatusService.php | 6 +- .../UpdaterService.php} | 20 +- lib/Utility/OPMLExporter.php | 15 +- lib/Utility/Time.php | 4 +- tests/Integration/Db/FeedMapperTest.php | 16 +- tests/Integration/Db/ItemMapperTest.php | 52 +-- tests/Integration/IntegrationTest.php | 30 +- .../Unit/Controller/FeedApiControllerTest.php | 36 +- tests/Unit/Controller/FeedControllerTest.php | 16 +- .../Controller/FolderApiControllerTest.php | 23 +- .../Unit/Controller/FolderControllerTest.php | 14 +- .../Unit/Controller/ItemApiControllerTest.php | 2 +- tests/Unit/Controller/ItemControllerTest.php | 2 +- .../Controller/UtilityApiControllerTest.php | 59 ++- tests/Unit/Db/FeedTest.php | 25 +- tests/Unit/Db/FolderMapperTest.php | 6 +- tests/Unit/Db/FolderTest.php | 9 +- tests/Unit/Db/MapperFactoryTest.php | 4 +- tests/Unit/Fetcher/FeedFetcherTest.php | 32 +- tests/Unit/Service/FeedServiceTest.php | 168 ++++----- tests/Unit/Service/FolderServiceTest.php | 30 +- tests/Unit/Service/ItemServiceTest.php | 61 +++- tests/Unit/Service/ServiceTest.php | 39 +- .../Unit/{Utility => Service}/UpdaterTest.php | 38 +- 79 files changed, 2436 insertions(+), 563 deletions(-) create mode 100644 lib/Command/Config/FeedAdd.php create mode 100644 lib/Command/Config/FeedDelete.php create mode 100644 lib/Command/Config/FeedList.php create mode 100644 lib/Command/Config/FolderAdd.php create mode 100644 lib/Command/Config/FolderDelete.php create mode 100644 lib/Command/Config/FolderList.php create mode 100644 lib/Controller/ApiPayloadTrait.php rename lib/Cron/{Updater.php => UpdaterJob.php} (77%) create mode 100644 lib/Db/FeedMapperV2.php create mode 100644 lib/Db/FolderMapperV2.php create mode 100644 lib/Db/ItemMapperV2.php create mode 100644 lib/Db/NewsMapperV2.php rename lib/Service/{ => Exceptions}/ServiceConflictException.php (93%) rename lib/Service/{ => Exceptions}/ServiceException.php (93%) rename lib/Service/{ => Exceptions}/ServiceNotFoundException.php (93%) rename lib/Service/{ => Exceptions}/ServiceValidationException.php (93%) create mode 100644 lib/Service/FeedServiceV2.php create mode 100644 lib/Service/FolderServiceV2.php create mode 100644 lib/Service/ItemServiceV2.php rename lib/{Utility/Updater.php => Service/UpdaterService.php} (70%) rename tests/Unit/{Utility => Service}/UpdaterTest.php (66%) diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 14244ab2a..b94f01f8e 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -100,9 +100,16 @@ jobs: run: | cd ../server ./occ news:generate-explore --votes 100 "https://nextcloud.com/blog/feed/" + ./occ news:folder:add 'admin' 'Something' + ./occ news:folder:list 'admin' | grep 'Something' + ./occ news:folder:delete 'admin' $(./occ news:folder:list 'admin' | grep 'Something' -1 | head -1 | grep -oE '[0-9]*') + ./occ news:feed:add 'admin' "https://nextcloud.com/blog/feed/" + ./occ news:feed:list 'admin' | grep 'nextcloud\.com' + ./occ news:feed:delete 'admin' $(./occ news:feed:list 'admin' | grep 'nextcloud\.com' -1 | head -1 | grep -oE '[0-9]*') - name: Prep PHP tests run: cd ../server/apps/news && make php-test-dependencies - name: Integration tests run: cd ../server/apps/news && make integration-test + diff --git a/AUTHORS.md b/AUTHORS.md index 187c491c4..25264f040 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -130,6 +130,7 @@ * [blackcrack](mailto:blackcrack@blackysgate.de) * [comradekingu](mailto:epost@anotheragency.no) * [e-alfred](mailto:e-alfred@users.noreply.github.com) +* [fran-penedo](mailto:fran@franpenedo.com) * [joeplus](mailto:joerg@honululu.Speedport_W_723V_1_32_000) * [kesselb](mailto:mail@danielkesselberg.de) * [kondou](mailto:kondou@ts.unde.re) diff --git a/appinfo/info.xml b/appinfo/info.xml index a9eb039f5..c11dc7f20 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -58,6 +58,13 @@ Before you update to a new version, [check the changelog](https://github.com/nex OCA\News\Command\Updater\UpdateFeed OCA\News\Command\Updater\BeforeUpdate OCA\News\Command\Updater\AfterUpdate + OCA\News\Command\Config\FolderList + OCA\News\Command\Config\FolderAdd + OCA\News\Command\Config\FolderDelete + OCA\News\Command\Config\FeedList + OCA\News\Command\Config\FeedAdd + OCA\News\Command\Config\FeedDelete + OCA\News\Command\Config\FeedDelete diff --git a/composer.json b/composer.json index 25d0db908..0acc198ae 100644 --- a/composer.json +++ b/composer.json @@ -50,7 +50,8 @@ "ext-json": "*", "ext-simplexml": "*", "ext-libxml": "*", - "andreskrey/readability.php": "^2.1" + "andreskrey/readability.php": "^2.1", + "ext-dom": "*" }, "require-dev": { "phpunit/phpunit": "9.2.6", diff --git a/composer.lock b/composer.lock index 7b31b1ac9..ada2160ad 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "d026de8b34993e486d928ef4c509676d", + "content-hash": "0f4161d1df33ea0b72224da925f2f6b5", "packages": [ { "name": "andreskrey/readability.php", @@ -826,16 +826,16 @@ }, { "name": "phpstan/phpstan", - "version": "0.12.44", + "version": "0.12.45", "source": { "type": "git", "url": "https://github.com/phpstan/phpstan.git", - "reference": "330b45776ea77f167b150e24787412414a8fa469" + "reference": "90c67259212ed891ee86604a9368ef7d7bead3a4" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpstan/phpstan/zipball/330b45776ea77f167b150e24787412414a8fa469", - "reference": "330b45776ea77f167b150e24787412414a8fa469", + "url": "https://api.github.com/repos/phpstan/phpstan/zipball/90c67259212ed891ee86604a9368ef7d7bead3a4", + "reference": "90c67259212ed891ee86604a9368ef7d7bead3a4", "shasum": "" }, "require": { @@ -878,7 +878,7 @@ "type": "tidelift" } ], - "time": "2020-09-24T15:28:47+00:00" + "time": "2020-09-26T19:19:00+00:00" }, { "name": "phpunit/php-code-coverage", @@ -2422,7 +2422,8 @@ "php": "^7.2", "ext-json": "*", "ext-simplexml": "*", - "ext-libxml": "*" + "ext-libxml": "*", + "ext-dom": "*" }, "platform-dev": [], "plugin-api-version": "1.1.0" diff --git a/lib/Command/Config/FeedAdd.php b/lib/Command/Config/FeedAdd.php new file mode 100644 index 000000000..d21f448c0 --- /dev/null +++ b/lib/Command/Config/FeedAdd.php @@ -0,0 +1,72 @@ +feedService = $feedService; + } + + /** + * Configure command + */ + protected function configure() + { + $this->setName('news:feed:add') + ->setDescription('Add a feed') + ->addArgument('userID', InputArgument::REQUIRED, 'User to add the feed for') + ->addArgument('feed', InputArgument::REQUIRED, 'Feed to parse') + ->addOption('folder', null, InputOption::VALUE_OPTIONAL, 'Folder ID') + ->addOption('title', null, InputOption::VALUE_OPTIONAL, 'Feed title') + ->addOption('full-text', null, InputOption::VALUE_OPTIONAL, 'Scrape item URLs', false) + ->addOption('username', null, InputOption::VALUE_OPTIONAL, 'Basic auth username') + ->addOption('password', null, InputOption::VALUE_OPTIONAL, 'Basic auth password'); + } + + /** + * Execute command + * + * @param InputInterface $input + * @param OutputInterface $output + * + * @return int + */ + protected function execute(InputInterface $input, OutputInterface $output): int + { + $url = $input->getArgument('feed'); + $user = $input->getArgument('userID'); + $folder = (int) $input->getOption('folder') ?? 0; + $title = $input->getOption('title'); + $username = $input->getOption('username'); + $full_text = $input->getOption('full-text'); + $password = $input->getOption('password'); + + $feed = $this->feedService->create($user, $url, $folder, $full_text, $title, $username, $password); + $this->feedService->fetch($feed, true); + + $output->writeln(json_encode($feed->toAPI(), JSON_PRETTY_PRINT)); + + return 0; + } +} diff --git a/lib/Command/Config/FeedDelete.php b/lib/Command/Config/FeedDelete.php new file mode 100644 index 000000000..c848b1fd4 --- /dev/null +++ b/lib/Command/Config/FeedDelete.php @@ -0,0 +1,56 @@ +feedService = $feedService; + } + + /** + * Configure command + */ + protected function configure() + { + $this->setName('news:feed:delete') + ->setDescription('Remove a feed') + ->addArgument('userID', InputArgument::REQUIRED, 'User to remove the feed from') + ->addArgument('id', InputArgument::REQUIRED, 'Feed ID', null); + } + + /** + * Execute command + * + * @param InputInterface $input + * @param OutputInterface $output + * + * @return int + */ + protected function execute(InputInterface $input, OutputInterface $output): int + { + $user = $input->getArgument('userID'); + $id = $input->getArgument('id'); + + $this->feedService->delete($user, $id); + + return 0; + } +} diff --git a/lib/Command/Config/FeedList.php b/lib/Command/Config/FeedList.php new file mode 100644 index 000000000..57e14d339 --- /dev/null +++ b/lib/Command/Config/FeedList.php @@ -0,0 +1,61 @@ +feedService = $feedService; + } + + /** + * Configure command + */ + protected function configure() + { + $this->setName('news:feed:list') + ->setDescription('List all feeds') + ->addArgument('userID', InputArgument::REQUIRED, 'User to list the feeds for') + ->addOption('recursive', null, InputOption::VALUE_NONE, 'Fetch the feed recursively'); + } + + /** + * Execute command + * + * @param InputInterface $input + * @param OutputInterface $output + * + * @return int|void + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $user = $input->getArgument('userID'); + $recursive = $input->getOption('recursive'); + + if ($recursive !== false) { + $feeds = $this->feedService->findAllForUserRecursive($user); + } else { + $feeds = $this->feedService->findAllForUser($user); + } + + $output->writeln(json_encode($this->serialize($feeds), JSON_PRETTY_PRINT)); + } +} diff --git a/lib/Command/Config/FolderAdd.php b/lib/Command/Config/FolderAdd.php new file mode 100644 index 000000000..5a7155e0c --- /dev/null +++ b/lib/Command/Config/FolderAdd.php @@ -0,0 +1,58 @@ +folderService = $folderService; + } + + /** + * Configure command + */ + protected function configure() + { + $this->setName('news:folder:add') + ->setDescription('Add a folder') + ->addArgument('userID', InputArgument::REQUIRED, 'User to add the folder for') + ->addArgument('name', InputArgument::REQUIRED, 'Folder name', null) + ->addOption('parent', null, InputOption::VALUE_OPTIONAL, 'Parent folder'); + } + + /** + * Execute command + * + * @param InputInterface $input + * @param OutputInterface $output + * + * @return int + */ + protected function execute(InputInterface $input, OutputInterface $output): int + { + $user = $input->getArgument('userID'); + $name = $input->getArgument('name'); + $parent = $input->getOption('parent') ?? 0; + + $this->folderService->create($user, $name, $parent); + + return 0; + } +} diff --git a/lib/Command/Config/FolderDelete.php b/lib/Command/Config/FolderDelete.php new file mode 100644 index 000000000..c441bd615 --- /dev/null +++ b/lib/Command/Config/FolderDelete.php @@ -0,0 +1,61 @@ +folderService = $folderService; + } + + /** + * Configure command + */ + protected function configure() + { + $this->setName('news:folder:delete') + ->setDescription('Remove a folder') + ->addArgument('userID', InputArgument::REQUIRED, 'User to remove the folder from') + ->addArgument('id', InputArgument::REQUIRED, 'Folder ID', null); + } + + /** + * Execute command + * + * @param InputInterface $input + * @param OutputInterface $output + * + * @return int + */ + protected function execute(InputInterface $input, OutputInterface $output): int + { + $user = $input->getArgument('userID'); + $id = $input->getArgument('id'); + + if ($id === '0') { + throw new ServiceException('Can not remove root folder!'); + } + + $this->folderService->delete($user, $id); + + return 0; + } +} diff --git a/lib/Command/Config/FolderList.php b/lib/Command/Config/FolderList.php new file mode 100644 index 000000000..7a2d33ab5 --- /dev/null +++ b/lib/Command/Config/FolderList.php @@ -0,0 +1,62 @@ +folderService = $folderService; + } + + /** + * Configure command + */ + protected function configure() + { + $this->setName('news:folder:list') + ->setDescription('List all folders') + ->addArgument('userID', InputArgument::REQUIRED, 'User to list the folders for') + ->addOption('recursive', null, InputOption::VALUE_NONE, 'Fetch the folder recursively'); + } + + /** + * Execute command + * + * @param InputInterface $input + * @param OutputInterface $output + * + * @return int|void + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $user = $input->getArgument('userID'); + $recursive = $input->getOption('recursive'); + + if ($recursive !== false) { + $folders = $this->folderService->findAllForUserRecursive($user); + } else { + $folders = $this->folderService->findAllForUser($user); + } + + $output->writeln(json_encode($this->serialize($folders), JSON_PRETTY_PRINT)); + } +} diff --git a/lib/Command/ExploreGenerator.php b/lib/Command/ExploreGenerator.php index 2e1b38e91..65c206c5e 100644 --- a/lib/Command/ExploreGenerator.php +++ b/lib/Command/ExploreGenerator.php @@ -64,7 +64,7 @@ class ExploreGenerator extends Command ->addOption('votes', null, InputOption::VALUE_OPTIONAL, 'Votes for the feed, defaults to 100'); } - protected function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): int { $url = $input->getArgument('feed'); $votes = $input->getOption('votes'); @@ -85,6 +85,7 @@ class ExploreGenerator extends Command ]; $output->writeln(json_encode($result, JSON_PRETTY_PRINT)); + return 0; } catch (\Throwable $ex) { $output->writeln('Failed to fetch feed info:'); $output->writeln($ex->getMessage()); diff --git a/lib/Command/ShowFeed.php b/lib/Command/ShowFeed.php index 878b71123..1218279e6 100644 --- a/lib/Command/ShowFeed.php +++ b/lib/Command/ShowFeed.php @@ -60,7 +60,7 @@ class ShowFeed extends Command ->addOption('full-text', 'f', InputOption::VALUE_NONE, 'Usa a scraper to get full text'); } - protected function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): int { $url = $input->getArgument('feed'); $user = $input->getOption('user'); @@ -71,6 +71,7 @@ class ShowFeed extends Command list($feed, $items) = $this->feedFetcher->fetch($url, true, null, $fullTextEnabled, $user, $password); $output->writeln("Feed: " . json_encode($feed, JSON_PRETTY_PRINT)); $output->writeln("Items: " . json_encode($items, JSON_PRETTY_PRINT)); + return 0; } catch (\Throwable $ex) { $output->writeln('Failed to fetch feed info:'); $output->writeln($ex->getMessage()); diff --git a/lib/Command/Updater/AfterUpdate.php b/lib/Command/Updater/AfterUpdate.php index c80913fab..307dece99 100644 --- a/lib/Command/Updater/AfterUpdate.php +++ b/lib/Command/Updater/AfterUpdate.php @@ -11,35 +11,43 @@ namespace OCA\News\Command\Updater; -use Exception; - +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\Output\OutputInterface; -use \OCA\News\Utility\Updater; - class AfterUpdate extends Command { - private $updater; + /** + * @var ItemServiceV2 + */ + private $itemService; - public function __construct(Updater $updater) + /** + * AfterUpdate constructor. + * + * @param ItemServiceV2 $itemService + */ + public function __construct(ItemServiceV2 $itemService) { parent::__construct(); - $this->updater = $updater; + $this->itemService = $itemService; } protected function configure() { $this->setName('news:updater:after-update') - ->setDescription( - 'This is used to clean up the database. It ' . - 'removes old read articles which are not starred' - ); + ->setDescription('removes old read articles which are not starred') + ->addArgument('purge_count', InputArgument::OPTIONAL, 'The amount of items to purge'); } - protected function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): int { - $this->updater->afterUpdate(); + $count = $input->getArgument('id'); + + echo $this->itemService->purgeOverThreshold($count); + + return 0; } } diff --git a/lib/Command/Updater/AllFeeds.php b/lib/Command/Updater/AllFeeds.php index 93ef4e59a..6993d51ea 100644 --- a/lib/Command/Updater/AllFeeds.php +++ b/lib/Command/Updater/AllFeeds.php @@ -11,19 +11,24 @@ namespace OCA\News\Command\Updater; -use Exception; - +use OCA\News\Service\FeedServiceV2; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -use OCA\News\Service\FeedService; - class AllFeeds extends Command { + /** + * @var FeedServiceV2 Feed service + */ private $feedService; - public function __construct(FeedService $feedService) + /** + * AllFeeds constructor. + * + * @param FeedServiceV2 $feedService + */ + public function __construct(FeedServiceV2 $feedService) { parent::__construct(); $this->feedService = $feedService; @@ -35,6 +40,7 @@ class AllFeeds extends Command $this->setName('news:updater:all-feeds') ->setDescription( + 'DEPRECATED: use news:feed:list instead.' . PHP_EOL . 'Prints a JSON string which contains all feed ' . 'ids and user ids, e.g.: ' . $json ); @@ -42,7 +48,7 @@ class AllFeeds extends Command protected function execute(InputInterface $input, OutputInterface $output) { - $feeds = $this->feedService->findAllFromAllUsers(); + $feeds = $this->feedService->findAll(); $result = ['feeds' => []]; foreach ($feeds as $feed) { @@ -53,6 +59,6 @@ class AllFeeds extends Command ]; } - print(json_encode($result)); + $output->write(json_encode($result)); } } diff --git a/lib/Command/Updater/BeforeUpdate.php b/lib/Command/Updater/BeforeUpdate.php index 3a0b1ca72..787125c32 100644 --- a/lib/Command/Updater/BeforeUpdate.php +++ b/lib/Command/Updater/BeforeUpdate.php @@ -11,22 +11,22 @@ namespace OCA\News\Command\Updater; -use Exception; - +use OCA\News\Service\UpdaterService; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -use \OCA\News\Utility\Updater; - class BeforeUpdate extends Command { - private $updater; + /** + * @var UpdaterService Updater + */ + private $updaterService; - public function __construct(Updater $updater) + public function __construct(UpdaterService $updater) { parent::__construct(); - $this->updater = $updater; + $this->updaterService = $updater; } protected function configure() @@ -39,8 +39,10 @@ class BeforeUpdate extends Command ); } - protected function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): int { - $this->updater->beforeUpdate(); + $this->updaterService->beforeUpdate(); + + return 0; } } diff --git a/lib/Command/Updater/UpdateFeed.php b/lib/Command/Updater/UpdateFeed.php index f5cda22ad..5078e92a4 100644 --- a/lib/Command/Updater/UpdateFeed.php +++ b/lib/Command/Updater/UpdateFeed.php @@ -13,6 +13,7 @@ namespace OCA\News\Command\Updater; use Exception; +use OCA\News\Service\FeedServiceV2; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -22,9 +23,12 @@ use OCA\News\Service\FeedService; class UpdateFeed extends Command { + /** + * @var FeedServiceV2 Feed service + */ private $feedService; - public function __construct(FeedService $feedService) + public function __construct(FeedServiceV2 $feedService) { parent::__construct(); $this->feedService = $feedService; @@ -33,25 +37,26 @@ class UpdateFeed extends Command protected function configure() { $this->setName('news:updater:update-feed') - ->addArgument( - 'feed-id', - InputArgument::REQUIRED, - 'feed id, integer' - ) ->addArgument( 'user-id', InputArgument::REQUIRED, 'user id of a user, string' ) + ->addArgument( + 'feed-id', + InputArgument::REQUIRED, + 'feed id, integer' + ) ->setDescription('Console API for updating a single user\'s feed'); } - protected function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): int { $feedId = $input->getArgument('feed-id'); $userId = $input->getArgument('user-id'); try { - $this->feedService->update($feedId, $userId); + $feed = $this->feedService->findForUser($userId, $feedId); + $this->feedService->fetch($feed); } catch (Exception $e) { $output->writeln( 'Could not update feed with id ' . $feedId . @@ -59,5 +64,7 @@ class UpdateFeed extends Command ' ' ); } + + return 0; } } diff --git a/lib/Controller/ApiPayloadTrait.php b/lib/Controller/ApiPayloadTrait.php new file mode 100644 index 000000000..2bb31784e --- /dev/null +++ b/lib/Controller/ApiPayloadTrait.php @@ -0,0 +1,36 @@ +toAPI()]; + } + + if (!is_array($data)) { + return $return; + } + + foreach ($data as $entity) { + if ($entity instanceof IAPI) { + $return[] = $entity->toAPI(); + } + } + return $return; + } +} diff --git a/lib/Controller/EntityApiSerializer.php b/lib/Controller/EntityApiSerializer.php index 78a9b1031..c7fdb84e5 100644 --- a/lib/Controller/EntityApiSerializer.php +++ b/lib/Controller/EntityApiSerializer.php @@ -13,6 +13,12 @@ namespace OCA\News\Controller; use \OCA\News\Db\IAPI; +/** + * Class EntityApiSerializer + * + * @package OCA\News\Controller + * @deprecated use ApiPayloadTrait + */ class EntityApiSerializer { diff --git a/lib/Controller/FeedApiController.php b/lib/Controller/FeedApiController.php index 160a8bcd6..f2b77c72b 100644 --- a/lib/Controller/FeedApiController.php +++ b/lib/Controller/FeedApiController.php @@ -15,6 +15,9 @@ namespace OCA\News\Controller; +use OCA\News\Service\Exceptions\ServiceConflictException; +use OCA\News\Service\Exceptions\ServiceNotFoundException; +use OCA\News\Utility\PsrLogger; use \OCP\IRequest; use \OCP\ILogger; use \OCP\IUserSession; @@ -22,17 +25,30 @@ use \OCP\AppFramework\Http; use \OCA\News\Service\FeedService; use \OCA\News\Service\ItemService; -use \OCA\News\Service\ServiceNotFoundException; -use \OCA\News\Service\ServiceConflictException; +use Psr\Log\LoggerInterface; class FeedApiController extends ApiController { use JSONHttpError; + /** + * @var ItemService + */ private $itemService; + + /** + * @var FeedService + */ private $feedService; + + /** + * @var LoggerInterface + */ private $logger; - private $loggerParams; + + /** + * @var EntityApiSerializer + */ private $serializer; public function __construct( @@ -41,14 +57,12 @@ class FeedApiController extends ApiController IUserSession $userSession, FeedService $feedService, ItemService $itemService, - ILogger $logger, - $LoggerParameters + LoggerInterface $logger ) { parent::__construct($appName, $request, $userSession); $this->feedService = $feedService; $this->itemService = $itemService; $this->logger = $logger; - $this->loggerParams = $LoggerParameters; $this->serializer = new EntityApiSerializer('feeds'); } @@ -63,7 +77,7 @@ class FeedApiController extends ApiController $result = [ 'starredCount' => $this->itemService->starredCount($this->getUserId()), - 'feeds' => $this->feedService->findAll($this->getUserId()) + 'feeds' => $this->feedService->findAllForUser($this->getUserId()) ]; @@ -226,13 +240,10 @@ class FeedApiController extends ApiController public function update($userId, $feedId) { try { - $this->feedService->update($feedId, $userId); + $this->feedService->update($userId, $feedId); // ignore update failure } catch (\Exception $ex) { - $this->logger->debug( - 'Could not update feed ' . $ex->getMessage(), - $this->loggerParams - ); + $this->logger->debug('Could not update feed ' . $ex->getMessage()); } } } diff --git a/lib/Controller/FeedController.php b/lib/Controller/FeedController.php index 32e9bd232..40aef909a 100644 --- a/lib/Controller/FeedController.php +++ b/lib/Controller/FeedController.php @@ -13,6 +13,8 @@ namespace OCA\News\Controller; +use OCA\News\Service\Exceptions\ServiceConflictException; +use OCA\News\Service\Exceptions\ServiceNotFoundException; use OCP\IRequest; use OCP\IConfig; use OCP\AppFramework\Controller; @@ -21,8 +23,6 @@ use OCP\AppFramework\Http; use OCA\News\Service\ItemService; use OCA\News\Service\FeedService; use OCA\News\Service\FolderService; -use OCA\News\Service\ServiceNotFoundException; -use OCA\News\Service\ServiceConflictException; use OCA\News\Db\FeedType; class FeedController extends Controller @@ -63,7 +63,7 @@ class FeedController extends Controller // because of this we also pass the starred count and the newest // item id which will be used for marking feeds read $params = [ - 'feeds' => $this->feedService->findAll($this->userId), + 'feeds' => $this->feedService->findAllForUser($this->userId), 'starred' => $this->itemService->starredCount($this->userId) ]; @@ -104,9 +104,9 @@ class FeedController extends Controller // check if feed or folder exists try { if ($feedType === FeedType::FOLDER) { - $this->folderService->find($feedId, $this->userId); + $this->folderService->find($this->userId, $feedId); } elseif ($feedType === FeedType::FEED) { - $this->feedService->find($feedId, $this->userId); + $this->feedService->find($this->userId, $feedId); // if its the first launch, those values will be null } elseif ($feedType === null) { @@ -203,7 +203,7 @@ class FeedController extends Controller public function update($feedId) { try { - $feed = $this->feedService->update($feedId, $this->userId); + $feed = $this->feedService->update($this->userId, $feedId); return [ 'feeds' => [ diff --git a/lib/Controller/FolderApiController.php b/lib/Controller/FolderApiController.php index eb98b8107..3bafd81a0 100644 --- a/lib/Controller/FolderApiController.php +++ b/lib/Controller/FolderApiController.php @@ -21,9 +21,9 @@ use \OCP\AppFramework\Http; use \OCA\News\Service\FolderService; use \OCA\News\Service\ItemService; -use \OCA\News\Service\ServiceNotFoundException; -use \OCA\News\Service\ServiceConflictException; -use \OCA\News\Service\ServiceValidationException; +use \OCA\News\Service\Exceptions\ServiceNotFoundException; +use \OCA\News\Service\Exceptions\ServiceConflictException; +use \OCA\News\Service\Exceptions\ServiceValidationException; class FolderApiController extends ApiController { diff --git a/lib/Controller/FolderController.php b/lib/Controller/FolderController.php index d3089178d..22baf8db6 100644 --- a/lib/Controller/FolderController.php +++ b/lib/Controller/FolderController.php @@ -20,9 +20,9 @@ use \OCP\AppFramework\Http; use \OCA\News\Service\FolderService; use \OCA\News\Service\FeedService; use \OCA\News\Service\ItemService; -use \OCA\News\Service\ServiceNotFoundException; -use \OCA\News\Service\ServiceConflictException; -use \OCA\News\Service\ServiceValidationException; +use \OCA\News\Service\Exceptions\ServiceNotFoundException; +use \OCA\News\Service\Exceptions\ServiceConflictException; +use \OCA\News\Service\Exceptions\ServiceValidationException; class FolderController extends Controller { diff --git a/lib/Controller/ItemApiController.php b/lib/Controller/ItemApiController.php index cf4c7c730..d5b1de680 100644 --- a/lib/Controller/ItemApiController.php +++ b/lib/Controller/ItemApiController.php @@ -20,7 +20,7 @@ use \OCP\IUserSession; use \OCP\AppFramework\Http; use \OCA\News\Service\ItemService; -use \OCA\News\Service\ServiceNotFoundException; +use \OCA\News\Service\Exceptions\ServiceNotFoundException; class ItemApiController extends ApiController { @@ -223,7 +223,7 @@ class ItemApiController extends ApiController * @NoCSRFRequired * @CORS * - * @param int[] item ids + * @param int[] $items item ids */ public function readMultiple($items) { @@ -236,7 +236,7 @@ class ItemApiController extends ApiController * @NoCSRFRequired * @CORS * - * @param int[] item ids + * @param int[] $items item ids */ public function unreadMultiple($items) { @@ -266,7 +266,7 @@ class ItemApiController extends ApiController * @NoCSRFRequired * @CORS * - * @param int[] item ids + * @param int[] $items item ids */ public function starMultiple($items) { @@ -279,7 +279,7 @@ class ItemApiController extends ApiController * @NoCSRFRequired * @CORS * - * @param int[] item ids + * @param int[] $items item ids */ public function unstarMultiple($items) { diff --git a/lib/Controller/ItemController.php b/lib/Controller/ItemController.php index 156f4d1d4..658e92883 100644 --- a/lib/Controller/ItemController.php +++ b/lib/Controller/ItemController.php @@ -18,8 +18,8 @@ use \OCP\IConfig; use \OCP\AppFramework\Controller; use \OCP\AppFramework\Http; -use \OCA\News\Service\ServiceException; -use \OCA\News\Service\ServiceNotFoundException; +use \OCA\News\Service\Exceptions\ServiceException; +use \OCA\News\Service\Exceptions\ServiceNotFoundException; use \OCA\News\Service\ItemService; use \OCA\News\Service\FeedService; @@ -245,7 +245,7 @@ class ItemController extends Controller /** * @NoAdminRequired * - * @param int[] item ids + * @param int[] $itemIds item ids */ public function readMultiple($itemIds) { diff --git a/lib/Controller/JSONHttpError.php b/lib/Controller/JSONHttpError.php index 16d80f857..6a66f9c29 100644 --- a/lib/Controller/JSONHttpError.php +++ b/lib/Controller/JSONHttpError.php @@ -18,10 +18,9 @@ trait JSONHttpError /** - * @param \Exception $exception the message that is returned taken from the - * exception - * @param int $code the http error code - * @return \OCP\AppFramework\Http\JSONResponse + * @param \Exception $exception The exception to report + * @param int $code The http error code + * @return JSONResponse */ public function error(\Exception $exception, $code) { diff --git a/lib/Controller/UtilityApiController.php b/lib/Controller/UtilityApiController.php index ee9ca0900..23956f149 100644 --- a/lib/Controller/UtilityApiController.php +++ b/lib/Controller/UtilityApiController.php @@ -15,18 +15,17 @@ namespace OCA\News\Controller; +use OCA\News\Service\UpdaterService; use \OCP\IRequest; use \OCP\IConfig; use \OCP\IUserSession; -use \OCP\AppFramework\Http; -use \OCA\News\Utility\Updater; use \OCA\News\Service\StatusService; class UtilityApiController extends ApiController { - private $updater; + private $updaterService; private $settings; private $statusService; @@ -34,12 +33,12 @@ class UtilityApiController extends ApiController $appName, IRequest $request, IUserSession $userSession, - Updater $updater, + UpdaterService $updater, IConfig $settings, StatusService $statusService ) { parent::__construct($appName, $request, $userSession); - $this->updater = $updater; + $this->updaterService = $updater; $this->settings = $settings; $this->statusService = $statusService; } @@ -50,7 +49,7 @@ class UtilityApiController extends ApiController * @NoCSRFRequired * @CORS */ - public function version() + public function version(): array { $version = $this->settings->getAppValue( $this->appName, @@ -64,9 +63,9 @@ class UtilityApiController extends ApiController * @NoCSRFRequired * @CORS */ - public function beforeUpdate() + public function beforeUpdate(): void { - $this->updater->beforeUpdate(); + $this->updaterService->beforeUpdate(); } @@ -74,9 +73,9 @@ class UtilityApiController extends ApiController * @NoCSRFRequired * @CORS */ - public function afterUpdate() + public function afterUpdate(): void { - $this->updater->afterUpdate(); + $this->updaterService->afterUpdate(); } @@ -85,7 +84,7 @@ class UtilityApiController extends ApiController * @NoCSRFRequired * @NoAdminRequired */ - public function status() + public function status(): array { return $this->statusService->getStatus(); } diff --git a/lib/Cron/Updater.php b/lib/Cron/UpdaterJob.php similarity index 77% rename from lib/Cron/Updater.php rename to lib/Cron/UpdaterJob.php index 3d9336df7..d2dbde149 100644 --- a/lib/Cron/Updater.php +++ b/lib/Cron/UpdaterJob.php @@ -15,10 +15,10 @@ use OC\BackgroundJob\TimedJob; use OCA\News\AppInfo\Application; use OCA\News\Service\StatusService; -use OCA\News\Utility\Updater as UpdaterService; +use OCA\News\Service\UpdaterService; use OCP\IConfig; -class Updater extends TimedJob +class UpdaterJob extends TimedJob { /** @@ -28,7 +28,7 @@ class Updater extends TimedJob /** * @var StatusService */ - private $status; + private $statusService; /** * @var UpdaterService */ @@ -40,7 +40,7 @@ class Updater extends TimedJob UpdaterService $updaterService ) { $this->config = $config; - $this->status = $status; + $this->statusService = $status; $this->updaterService = $updaterService; $interval = $this->config->getAppValue( @@ -60,10 +60,12 @@ class Updater extends TimedJob Application::DEFAULT_SETTINGS['useCronUpdates'] ); - if ($uses_cron && $this->status->isProperlyConfigured()) { - $this->updaterService->beforeUpdate(); - $this->updaterService->update(); - $this->updaterService->afterUpdate(); + if (!$uses_cron || !$this->statusService->isProperlyConfigured()) { + return; } + + $this->updaterService->beforeUpdate(); + $this->updaterService->update(); + $this->updaterService->afterUpdate(); } } diff --git a/lib/Db/Feed.php b/lib/Db/Feed.php index 246ae9886..ae01c3d6c 100644 --- a/lib/Db/Feed.php +++ b/lib/Db/Feed.php @@ -15,6 +15,12 @@ namespace OCA\News\Db; use OCP\AppFramework\Db\Entity; +/** + * Class Feed + * + * @package OCA\News\Db + * @Embeddable + */ class Feed extends Entity implements IAPI, \JsonSerializable { use EntityJSONSerializer; @@ -67,6 +73,8 @@ class Feed extends Entity implements IAPI, \JsonSerializable protected $basicAuthUser = ''; /** @var string|null */ protected $basicAuthPassword = ''; + /** @var Item[] */ + public $items = []; /** * @return int|null @@ -135,7 +143,7 @@ class Feed extends Entity implements IAPI, \JsonSerializable /** * @return string|null */ - public function getHttpEtag() + public function getHttpEtag(): ?string { return $this->httpEtag; } @@ -143,7 +151,7 @@ class Feed extends Entity implements IAPI, \JsonSerializable /** * @return string|null */ - public function getHttpLastModified() + public function getHttpLastModified(): ?string { return $this->httpLastModified; } @@ -313,250 +321,294 @@ class Feed extends Entity implements IAPI, \JsonSerializable /** * @param int|null $added */ - public function setAdded(int $added = null) + public function setAdded(int $added = null): Feed { if ($this->added !== $added) { $this->added = $added; $this->markFieldUpdated('added'); } + + return $this; } /** * @param int $articlesPerUpdate */ - public function setArticlesPerUpdate(int $articlesPerUpdate) + public function setArticlesPerUpdate(int $articlesPerUpdate): Feed { if ($this->articlesPerUpdate !== $articlesPerUpdate) { $this->articlesPerUpdate = $articlesPerUpdate; $this->markFieldUpdated('articlesPerUpdate'); } + + return $this; } /** * @param string|null $basicAuthPassword */ - public function setBasicAuthPassword(string $basicAuthPassword = null) + public function setBasicAuthPassword(string $basicAuthPassword = null): Feed { if ($this->basicAuthPassword !== $basicAuthPassword) { $this->basicAuthPassword = $basicAuthPassword; $this->markFieldUpdated('basicAuthPassword'); } + + return $this; } /** * @param string|null $basicAuthUser */ - public function setBasicAuthUser(string $basicAuthUser = null) + public function setBasicAuthUser(string $basicAuthUser = null): Feed { if ($this->basicAuthUser !== $basicAuthUser) { $this->basicAuthUser = $basicAuthUser; $this->markFieldUpdated('basicAuthUser'); } + + return $this; } /** * @param int|null $deletedAt */ - public function setDeletedAt(int $deletedAt = null) + public function setDeletedAt(int $deletedAt = null): Feed { if ($this->deletedAt !== $deletedAt) { $this->deletedAt = $deletedAt; $this->markFieldUpdated('deletedAt'); } + + return $this; } /** * @param string|null $faviconLink */ - public function setFaviconLink(string $faviconLink = null) + public function setFaviconLink(string $faviconLink = null): Feed { if ($this->faviconLink !== $faviconLink) { $this->faviconLink = $faviconLink; $this->markFieldUpdated('faviconLink'); } + + return $this; } /** * @param int $folderId */ - public function setFolderId(int $folderId) + public function setFolderId(int $folderId): Feed { if ($this->folderId !== $folderId) { $this->folderId = $folderId; $this->markFieldUpdated('folderId'); } + + return $this; } /** * @param bool $fullTextEnabled */ - public function setFullTextEnabled(bool $fullTextEnabled) + public function setFullTextEnabled(bool $fullTextEnabled): Feed { if ($this->fullTextEnabled !== $fullTextEnabled) { $this->fullTextEnabled = $fullTextEnabled; $this->markFieldUpdated('fullTextEnabled'); } + + return $this; } /** * @param string|null $httpEtag */ - public function setHttpEtag(string $httpEtag = null) + public function setHttpEtag(string $httpEtag = null): Feed { if ($this->httpEtag !== $httpEtag) { $this->httpEtag = $httpEtag; $this->markFieldUpdated('httpEtag'); } + + return $this; } /** * @param string|null $httpLastModified */ - public function setHttpLastModified(string $httpLastModified = null) + public function setHttpLastModified(string $httpLastModified = null): Feed { if ($this->httpLastModified !== $httpLastModified) { $this->httpLastModified = $httpLastModified; $this->markFieldUpdated('httpLastModified'); } + + return $this; } /** * @param int $id */ - public function setId(int $id) + public function setId(int $id): Feed { if ($this->id !== $id) { $this->id = $id; $this->markFieldUpdated('id'); } + + return $this; } /** * @param string|null $lastModified */ - public function setLastModified(string $lastModified = null) + public function setLastModified(string $lastModified = null): Feed { if ($this->lastModified !== $lastModified) { $this->lastModified = $lastModified; $this->markFieldUpdated('lastModified'); } + + return $this; } /** * @param string|null $lastUpdateError */ - public function setLastUpdateError(string $lastUpdateError = null) + public function setLastUpdateError(string $lastUpdateError = null): Feed { if ($this->lastUpdateError !== $lastUpdateError) { $this->lastUpdateError = $lastUpdateError; $this->markFieldUpdated('lastUpdateError'); } + + return $this; } /** * @param string|null $link */ - public function setLink(string $link = null) + public function setLink(string $link = null): Feed { $link = trim($link); if (strpos($link, 'http') === 0 && $this->link !== $link) { $this->link = $link; $this->markFieldUpdated('link'); } + + return $this; } /** * @param string|null $location */ - public function setLocation(string $location = null) + public function setLocation(string $location = null): Feed { if ($this->location !== $location) { $this->location = $location; $this->markFieldUpdated('location'); } + + return $this; } /** * @param int $ordering */ - public function setOrdering(int $ordering) + public function setOrdering(int $ordering): Feed { if ($this->ordering !== $ordering) { $this->ordering = $ordering; $this->markFieldUpdated('ordering'); } + + return $this; } /** * @param bool $pinned */ - public function setPinned(bool $pinned) + public function setPinned(bool $pinned): Feed { if ($this->pinned !== $pinned) { $this->pinned = $pinned; $this->markFieldUpdated('pinned'); } + + return $this; } /** * @param bool $preventUpdate */ - public function setPreventUpdate(bool $preventUpdate) + public function setPreventUpdate(bool $preventUpdate): Feed { if ($this->preventUpdate !== $preventUpdate) { $this->preventUpdate = $preventUpdate; $this->markFieldUpdated('preventUpdate'); } + + return $this; } /** * @param string $title */ - public function setTitle(string $title) + public function setTitle(string $title): Feed { if ($this->title !== $title) { $this->title = $title; $this->markFieldUpdated('title'); } + + return $this; } /** * @param int $unreadCount */ - public function setUnreadCount(int $unreadCount) + public function setUnreadCount(int $unreadCount): Feed { if ($this->unreadCount !== $unreadCount) { $this->unreadCount = $unreadCount; $this->markFieldUpdated('unreadCount'); } + + return $this; } /** * @param int $updateErrorCount */ - public function setUpdateErrorCount(int $updateErrorCount) + public function setUpdateErrorCount(int $updateErrorCount): Feed { if ($this->updateErrorCount !== $updateErrorCount) { $this->updateErrorCount = $updateErrorCount; $this->markFieldUpdated('updateErrorCount'); } + + return $this; } /** * @param int $updateMode */ - public function setUpdateMode(int $updateMode) + public function setUpdateMode(int $updateMode): Feed { if ($this->updateMode !== $updateMode) { $this->updateMode = $updateMode; $this->markFieldUpdated('updateMode'); } + + return $this; } /** * @param string $url */ - public function setUrl(string $url) + public function setUrl(string $url): Feed { $url = trim($url); if (strpos($url, 'http') === 0 && $this->url !== $url) { @@ -564,28 +616,34 @@ class Feed extends Entity implements IAPI, \JsonSerializable $this->setUrlHash(md5($url)); $this->markFieldUpdated('url'); } + + return $this; } /** * @param string $urlHash */ - public function setUrlHash(string $urlHash) + public function setUrlHash(string $urlHash): Feed { if ($this->urlHash !== $urlHash) { $this->urlHash = $urlHash; $this->markFieldUpdated('urlHash'); } + + return $this; } /** * @param string $userId */ - public function setUserId(string $userId) + public function setUserId(string $userId): Feed { if ($this->userId !== $userId) { $this->userId = $userId; $this->markFieldUpdated('userId'); } + + return $this; } public function toAPI(): array @@ -603,7 +661,8 @@ class Feed extends Entity implements IAPI, \JsonSerializable 'link', 'pinned', 'updateErrorCount', - 'lastUpdateError' + 'lastUpdateError', + 'items' ] ); } diff --git a/lib/Db/FeedMapper.php b/lib/Db/FeedMapper.php index b23ced239..867ba982d 100644 --- a/lib/Db/FeedMapper.php +++ b/lib/Db/FeedMapper.php @@ -14,20 +14,28 @@ namespace OCA\News\Db; use OCA\News\Utility\Time; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\IDBConnection; use OCP\AppFramework\Db\Entity; +/** + * Class LegacyFeedMapper + * + * @package OCA\News\Db + * @deprecated use FeedMapper + */ class FeedMapper extends NewsMapper { - + const TABLE_NAME = 'news_feeds'; public function __construct(IDBConnection $db, Time $time) { - parent::__construct($db, 'news_feeds', Feed::class, $time); + parent::__construct($db, $time, Feed::class); } - public function find($id, $userId) + public function find(string $userId, int $id) { $sql = 'SELECT `feeds`.*, `item_numbers`.`unread_count` ' . 'FROM `*PREFIX*news_feeds` `feeds` ' . @@ -52,7 +60,7 @@ class FeedMapper extends NewsMapper } - public function findAllFromUser($userId) + public function findAllFromUser(string $userId): array { $sql = 'SELECT `feeds`.*, `item_numbers`.`unread_count` ' . 'FROM `*PREFIX*news_feeds` `feeds` ' . @@ -82,7 +90,7 @@ class FeedMapper extends NewsMapper } - public function findAll() + public function findAll(): array { $sql = 'SELECT `feeds`.*, `item_numbers`.`unread_count` ' . 'FROM `*PREFIX*news_feeds` `feeds` ' . @@ -135,15 +143,15 @@ class FeedMapper extends NewsMapper } - public function delete(Entity $entity) + public function delete(Entity $entity): Entity { - parent::delete($entity); - // someone please slap me for doing this manually :P // we needz CASCADE + FKs please $sql = 'DELETE FROM `*PREFIX*news_items` WHERE `feed_id` = ?'; $params = [$entity->getId()]; $this->execute($sql, $params); + + return parent::delete($entity); } @@ -186,4 +194,9 @@ class FeedMapper extends NewsMapper $sql = 'DELETE FROM `*PREFIX*news_feeds` WHERE `user_id` = ?'; $this->execute($sql, [$userId]); } + + public function findFromUser(string $userId, int $id): Entity + { + return $this->find($id, $userId); + } } diff --git a/lib/Db/FeedMapperV2.php b/lib/Db/FeedMapperV2.php new file mode 100644 index 000000000..2b4ff8f10 --- /dev/null +++ b/lib/Db/FeedMapperV2.php @@ -0,0 +1,141 @@ + + * @author Bernhard Posselt + * @copyright 2012 Alessandro Cosentino + * @copyright 2012-2014 Bernhard Posselt + */ + +namespace OCA\News\Db; + +use OCA\News\Utility\Time; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\IDBConnection; +use OCP\AppFramework\Db\Entity; + +/** + * Class FeedMapper + * + * @package OCA\News\Db + */ +class FeedMapperV2 extends NewsMapperV2 +{ + const TABLE_NAME = 'news_feeds'; + + /** + * FeedMapper constructor. + * + * @param IDBConnection $db + * @param Time $time + */ + public function __construct(IDBConnection $db, Time $time) + { + parent::__construct($db, $time, Feed::class); + } + + /** + * Find all feeds for a user. + * + * @param string $userId The user identifier + * + * @return Entity[] + */ + public function findAllFromUser(string $userId): array + { + $builder = $this->db->getQueryBuilder(); + $builder->addSelect('*') + ->from($this->tableName) + ->where('user_id = :user_id') + ->andWhere('deleted_at = 0') + ->setParameter(':user_id', $userId); + + return $this->findEntities($builder); + } + + /** + * Find all feeds for a user. + * + * @param string $userId The user identifier + * @param int $id The feed identifier + * + * @return Entity + * + * @throws DoesNotExistException + * @throws MultipleObjectsReturnedException + */ + public function findFromUser(string $userId, int $id): Entity + { + $builder = $this->db->getQueryBuilder(); + $builder->addSelect('*') + ->from($this->tableName) + ->where('user_id = :user_id') + ->where('id = :id') + ->setParameter(':user_id', $userId) + ->setParameter(':id', $id); + + return $this->findEntity($builder); + } + + /** + * Find all items + * + * @return Entity[] + */ + public function findAll(): array + { + $builder = $this->db->getQueryBuilder(); + $builder->select('*') + ->from($this->tableName) + ->where('deleted_at = 0'); + + return $this->findEntities($builder); + } + + /** + * Find feed by URL + * + * @param string $userId The user to find in. + * @param string $url The URL to find + * + * @return Entity + * + * @throws DoesNotExistException If not found + * @throws MultipleObjectsReturnedException If multiple found + */ + public function findByURL(string $userId, string $url): Entity + { + $builder = $this->db->getQueryBuilder(); + $builder->addSelect('*') + ->from($this->tableName) + ->where('user_id = :user_id') + ->andWhere('url = :url') + ->setParameter(':user_id', $userId) + ->setParameter(':url', $url); + + return $this->findEntity($builder); + } + + /** + * Find all feeds in a folder + * + * @param int $id ID of the folder + * + * @return Feed[] + */ + public function findAllFromFolder(int $id): array + { + $builder = $this->db->getQueryBuilder(); + $builder->addSelect('*') + ->from($this->tableName) + ->where('folder_id = :folder_id') + ->setParameter(':folder_id', $id); + + return $this->findEntities($builder); + } +} diff --git a/lib/Db/Folder.php b/lib/Db/Folder.php index 4f54524a8..674c9fabc 100644 --- a/lib/Db/Folder.php +++ b/lib/Db/Folder.php @@ -31,6 +31,8 @@ class Folder extends Entity implements IAPI, \JsonSerializable protected $deletedAt = 0; /** @var string|null */ protected $lastModified = '0'; + /** @var Feed[] */ + public $feeds = []; /** * @return int|null @@ -134,7 +136,7 @@ class Folder extends Entity implements IAPI, \JsonSerializable } } - public function setParentId(int $parentId = null) + public function setParentId(int $parentId = 0) { if ($this->parentId !== $parentId) { $this->parentId = $parentId; @@ -155,7 +157,8 @@ class Folder extends Entity implements IAPI, \JsonSerializable return $this->serializeFields( [ 'id', - 'name' + 'name', + 'feeds' ] ); } diff --git a/lib/Db/FolderMapper.php b/lib/Db/FolderMapper.php index fe73093a9..75b749974 100644 --- a/lib/Db/FolderMapper.php +++ b/lib/Db/FolderMapper.php @@ -14,18 +14,28 @@ namespace OCA\News\Db; use OCA\News\Utility\Time; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\IDBConnection; use OCP\AppFramework\Db\Entity; +/** + * Class LegacyFolderMapper + * + * @package OCA\News\Db + * @deprecated use FolderMapper + */ class FolderMapper extends NewsMapper { + const TABLE_NAME = 'news_folders'; + public function __construct(IDBConnection $db, Time $time) { - parent::__construct($db, 'news_folders', Folder::class, $time); + parent::__construct($db, $time, Folder::class); } - public function find($id, $userId) + public function find(string $userId, int $id) { $sql = 'SELECT * FROM `*PREFIX*news_folders` ' . 'WHERE `id` = ? ' . @@ -35,7 +45,7 @@ class FolderMapper extends NewsMapper } - public function findAllFromUser($userId) + public function findAllFromUser(string $userId): array { $sql = 'SELECT * FROM `*PREFIX*news_folders` ' . 'WHERE `user_id` = ? ' . @@ -46,7 +56,7 @@ class FolderMapper extends NewsMapper } - public function findByName($folderName, $userId) + public function findByName(string $folderName, string $userId) { $sql = 'SELECT * FROM `*PREFIX*news_folders` ' . 'WHERE `name` = ? ' . @@ -57,7 +67,7 @@ class FolderMapper extends NewsMapper } - public function delete(Entity $entity) + public function delete(Entity $entity): Entity { parent::delete($entity); @@ -73,6 +83,8 @@ class FolderMapper extends NewsMapper $stmt = $this->execute($sql); $stmt->closeCursor(); + + return $entity; } @@ -109,9 +121,23 @@ class FolderMapper extends NewsMapper * * @param string $userId the name of the user */ - public function deleteUser($userId) + public function deleteUser(string $userId) { $sql = 'DELETE FROM `*PREFIX*news_folders` WHERE `user_id` = ?'; $this->execute($sql, [$userId]); } + + /** + * NO-OP + * @return array + */ + public function findAll(): array + { + return []; + } + + public function findFromUser(string $userId, int $id): Entity + { + return $this->find($id, $userId); + } } diff --git a/lib/Db/FolderMapperV2.php b/lib/Db/FolderMapperV2.php new file mode 100644 index 000000000..d684e5af2 --- /dev/null +++ b/lib/Db/FolderMapperV2.php @@ -0,0 +1,87 @@ + + * @author Bernhard Posselt + * @copyright 2012 Alessandro Cosentino + * @copyright 2012-2014 Bernhard Posselt + */ + +namespace OCA\News\Db; + +use OCA\News\Utility\Time; +use OCP\AppFramework\Db\Entity; +use OCP\IDBConnection; + +/** + * Class FolderMapper + * + * @package OCA\News\Db + */ +class FolderMapperV2 extends NewsMapperV2 +{ + const TABLE_NAME = 'news_folders'; + + /** + * FolderMapper constructor. + * + * @param IDBConnection $db + * @param Time $time + */ + public function __construct(IDBConnection $db, Time $time) + { + parent::__construct($db, $time, Folder::class); + } + + /** + * Find all feeds for a user. + * + * @param string $userId The user identifier + * + * @return Entity[] + */ + public function findAllFromUser($userId): array + { + $builder = $this->db->getQueryBuilder(); + $builder->select('*') + ->from($this->tableName) + ->where('user_id = :user_id') + ->where('deleted_at = 0') + ->setParameter(':user_id', $userId); + + return $this->findEntities($builder); + } + + /** + * Find all items + * + * @return Entity[] + */ + public function findAll(): array + { + $builder = $this->db->getQueryBuilder(); + $builder->select('*') + ->from($this->tableName) + ->where('deleted_at = 0'); + + return $this->findEntities($builder); + } + + public function findFromUser(string $userId, int $id): Entity + { + $builder = $this->db->getQueryBuilder(); + $builder->select('*') + ->from($this->tableName) + ->where('user_id = :user_id') + ->where('id = :id') + ->where('deleted_at = 0') + ->setParameter(':user_id', $userId) + ->setParameter(':id', $id); + + return $this->findEntity($builder); + } +} diff --git a/lib/Db/Item.php b/lib/Db/Item.php index 4cf376366..7d3924597 100644 --- a/lib/Db/Item.php +++ b/lib/Db/Item.php @@ -15,6 +15,12 @@ namespace OCA\News\Db; use OCP\AppFramework\Db\Entity; +/** + * Class Item + * + * @package OCA\News\Db + * @Embeddable + */ class Item extends Entity implements IAPI, \JsonSerializable { use EntityJSONSerializer; diff --git a/lib/Db/ItemMapper.php b/lib/Db/ItemMapper.php index 2e08471db..270919b44 100644 --- a/lib/Db/ItemMapper.php +++ b/lib/Db/ItemMapper.php @@ -15,15 +15,25 @@ namespace OCA\News\Db; use Exception; use OCA\News\Utility\Time; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\Entity; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; +/** + * Class LegacyItemMapper + * + * @package OCA\News\Db + * @deprecated use ItemMapper + */ class ItemMapper extends NewsMapper { + const TABLE_NAME = 'news_items'; public function __construct(IDBConnection $db, Time $time) { - parent::__construct($db, 'news_items', Item::class, $time); + parent::__construct($db, $time, Item::class); } private function makeSelectQuery( @@ -99,13 +109,13 @@ class ItemMapper extends NewsMapper * @param string $userId * @return \OCA\News\Db\Item */ - public function find($id, $userId) + public function find(string $userId, int $id) { $sql = $this->makeSelectQuery('AND `items`.`id` = ? '); return $this->findEntity($sql, [$userId, $id]); } - public function starredCount($userId) + public function starredCount(string $userId) { $sql = 'SELECT COUNT(*) AS size FROM `*PREFIX*news_items` `items` ' . 'JOIN `*PREFIX*news_feeds` `feeds` ' . @@ -126,7 +136,7 @@ class ItemMapper extends NewsMapper } - public function readAll($highestItemId, $time, $userId) + public function readAll(int $highestItemId, $time, string $userId) { $sql = 'UPDATE `*PREFIX*news_items` ' . 'SET unread = ? ' . @@ -221,7 +231,7 @@ class ItemMapper extends NewsMapper } - private function findEntitiesIgnoringNegativeLimit($sql, $params, $limit) + private function findEntitiesIgnoringNegativeLimit($sql, $params, $limit): array { // ignore limit if negative to offer a way to return all feeds if ($limit >= 0) { @@ -286,7 +296,7 @@ class ItemMapper extends NewsMapper } - public function findAll( + public function findAllItems( $limit, $offset, $type, @@ -294,7 +304,7 @@ class ItemMapper extends NewsMapper $oldestFirst, $userId, $search = [] - ) { + ): array { $params = [$userId]; $params = array_merge($params, $this->buildLikeParameters($search)); $sql = $this->buildStatusQueryPart($showAll, $type); @@ -457,7 +467,7 @@ class ItemMapper extends NewsMapper public function readItem($itemId, $isRead, $lastModified, $userId) { - $item = $this->find($itemId, $userId); + $item = $this->find($userId, $itemId); // reading an item should set all of the same items as read, whereas // marking an item as unread should only mark the selected instance @@ -479,4 +489,30 @@ class ItemMapper extends NewsMapper $this->update($item); } } + + /** + * NO-OP + * + * @param string $userId + * + * @return array + */ + public function findAllFromUser(string $userId): array + { + return []; + } + + public function findFromUser(string $userId, int $id): Entity + { + return $this->find($id, $userId); + } + + /** + * NO-OP + * @return array + */ + public function findAll(): array + { + return []; + } } diff --git a/lib/Db/ItemMapperV2.php b/lib/Db/ItemMapperV2.php new file mode 100644 index 000000000..ba840fda7 --- /dev/null +++ b/lib/Db/ItemMapperV2.php @@ -0,0 +1,124 @@ + + * @author Bernhard Posselt + * @copyright 2020 Sean Molenaar + */ + +namespace OCA\News\Db; + +use Doctrine\DBAL\FetchMode; +use OCA\News\Utility\Time; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\Entity; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; + +/** + * Class ItemMapper + * + * @package OCA\News\Db + */ +class ItemMapperV2 extends NewsMapperV2 +{ + const TABLE_NAME = 'news_items'; + + /** + * ItemMapper constructor. + * + * @param IDBConnection $db + * @param Time $time + */ + public function __construct(IDBConnection $db, Time $time) + { + parent::__construct($db, $time, Item::class); + } + + /** + * Find all feeds for a user. + * + * @param string $userId The user identifier + * + * @return Entity[] + */ + public function findAllFromUser($userId): array + { + $builder = $this->db->getQueryBuilder(); + $builder->select('items.*') + ->from($this->tableName) + ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id') + ->where('feeds.user_id = :user_id') + ->andWhere('deleted_at = 0') + ->setParameter(':user_id', $userId, IQueryBuilder::PARAM_STR); + + return $this->findEntities($builder); + } + + /** + * Find all items + * + * @return Entity[] + */ + public function findAll(): array + { + $builder = $this->db->getQueryBuilder(); + $builder->addSelect('*') + ->from($this->tableName) + ->andWhere('deleted_at = 0'); + + return $this->findEntities($builder); + } + + public function findFromUser(string $userId, int $id): Entity + { + $builder = $this->db->getQueryBuilder(); + $builder->select('items.*') + ->from($this->tableName) + ->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id') + ->where('feeds.user_id = :user_id') + ->andWhere('items.id = :item_id') + ->andWhere('deleted_at = 0') + ->setParameter(':user_id', $userId, IQueryBuilder::PARAM_STR) + ->setParameter(':item_id', $id, IQueryBuilder::PARAM_STR); + + return $this->findEntity($builder); + } + + public function findByGuidHash(string $getGuidHash): Item + { + throw new DoesNotExistException('fasi'); + } + + public function findAllForFeed(int $feedId) + { + $builder = $this->db->getQueryBuilder(); + $builder->addSelect('*') + ->from($this->tableName) + ->andWhere('feed_id = :feed_id') + ->setParameter(':feed_id', $feedId, IQueryBuilder::PARAM_INT); + + return $this->findEntities($builder); + } + + /** + * Delete items from feed that are over the max item threshold + * + * @param int $threshold Deletion threshold + */ + public function deleteOverThreshold($threshold) + { + $builder = $this->db->getQueryBuilder(); + + $query = $builder->addSelect('COUNT(*)') + ->from($this->tableName) + ->groupBy('feed_id') + ->where(''); + + return $this->db->executeQuery($query)->fetch(FetchMode::ASSOCIATIVE); + } +} diff --git a/lib/Db/MapperFactory.php b/lib/Db/MapperFactory.php index 635003c64..35d587a6b 100644 --- a/lib/Db/MapperFactory.php +++ b/lib/Db/MapperFactory.php @@ -19,6 +19,12 @@ use OCP\IDBConnection; use OCA\News\Db\Mysql\ItemMapper as MysqlItemMapper; use OCA\News\DependencyInjection\IFactory; +/** + * Class LegacyMapperFactory + * + * @package OCA\News\Db + * @deprecated not needed in modern system + */ class MapperFactory implements IFactory { diff --git a/lib/Db/Mysql/ItemMapper.php b/lib/Db/Mysql/ItemMapper.php index 42c3b44e1..88c87038e 100644 --- a/lib/Db/Mysql/ItemMapper.php +++ b/lib/Db/Mysql/ItemMapper.php @@ -16,6 +16,12 @@ namespace OCA\News\Db\Mysql; use OCA\News\Utility\Time; use OCP\IDBConnection; +/** + * Class LegacyItemMapper + * + * @package OCA\News\Db\Mysql + * @deprecated use normal ItemMapper + */ class ItemMapper extends \OCA\News\Db\ItemMapper { diff --git a/lib/Db/NewsMapper.php b/lib/Db/NewsMapper.php index 4fa001f31..14913c1be 100644 --- a/lib/Db/NewsMapper.php +++ b/lib/Db/NewsMapper.php @@ -14,47 +14,101 @@ namespace OCA\News\Db; use OCA\News\Utility\Time; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\AppFramework\Db\QBMapper; use OCP\IDBConnection; use OCP\AppFramework\Db\Mapper; use OCP\AppFramework\Db\Entity; +/** + * Class NewsMapper + * + * @package OCA\News\Db + */ abstract class NewsMapper extends Mapper { + const TABLE_NAME = ''; /** * @var Time */ private $time; + /** + * NewsMapper constructor. + * + * @param IDBConnection $db Database connection + * @param Time $time Time class + * @param string $entity Entity class + */ public function __construct( IDBConnection $db, - $table, - $entity, - Time $time + Time $time, + string $entity ) { - parent::__construct($db, $table, $entity); + parent::__construct($db, static::TABLE_NAME, $entity); $this->time = $time; } - public function update(Entity $entity) + public function update(Entity $entity): Entity { $entity->setLastModified($this->time->getMicroTime()); return parent::update($entity); } - public function insert(Entity $entity) + public function insert(Entity $entity): Entity { $entity->setLastModified($this->time->getMicroTime()); return parent::insert($entity); } /** - * @param int $id the id of the feed - * @param string $userId the id of the user + * Remove deleted items. * - * @return \OCP\AppFramework\Db\Entity + * @return void */ - abstract public function find($id, $userId); + public function purgeDeleted(): void + { + $builder = $this->db->getQueryBuilder(); + $builder->delete($this->tableName) + ->where('deleted_at != 0') + ->execute() + ->execute(); + } + + abstract public function find(string $userId, int $id); + + /** + * Find all items. + * + * @return Entity[] + */ + abstract public function findAll(): array; + + /** + * Find all items for a user. + * + * @param string $userId ID of the user + * + * @return Entity[] + */ + abstract public function findAllFromUser(string $userId): array; + + /** + * Find item for a user. + * + * @param string $userId ID of the user + * @param int $id ID of the item + * + * @return Feed + * + * @throws DoesNotExistException The item is not found + * @throws MultipleObjectsReturnedException Multiple items found + */ + abstract public function findFromUser(string $userId, int $id): Entity; + + /** * Performs a SELECT query with all arguments appened to the WHERE clause @@ -64,12 +118,14 @@ abstract class NewsMapper extends Mapper * Important: This method does not filter marked as deleted rows! * * @param array $search an assoc array from property to filter value - * @param int $limit + * @param int|null $limit Output limit + * @param int|null $offset Output offset + * + * @depreacted Legacy function * - * @paran int $offset * @return array */ - public function where(array $search = [], $limit = null, $offset = null) + public function where(array $search = [], ?int $limit = null, ?int $offset = null) { $entity = new $this->entityClass(); @@ -80,7 +136,7 @@ abstract class NewsMapper extends Mapper // accidental Sql injection if (!property_exists($entity, $property)) { $msg = 'Property ' . $property . ' does not exist on ' - . $this->entityClass; + . $this->entityClass; throw new \BadFunctionCallException($msg); } diff --git a/lib/Db/NewsMapperV2.php b/lib/Db/NewsMapperV2.php new file mode 100644 index 000000000..be5695598 --- /dev/null +++ b/lib/Db/NewsMapperV2.php @@ -0,0 +1,108 @@ + + * @author Bernhard Posselt + * @copyright 2012 Alessandro Cosentino + * @copyright 2012-2014 Bernhard Posselt + */ + +namespace OCA\News\Db; + +use OCA\News\Utility\Time; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\AppFramework\Db\QBMapper; +use OCP\IDBConnection; +use OCP\AppFramework\Db\Mapper; +use OCP\AppFramework\Db\Entity; + +/** + * Class NewsMapper + * + * @package OCA\News\Db + */ +abstract class NewsMapperV2 extends QBMapper +{ + const TABLE_NAME = ''; + + /** + * @var Time + */ + private $time; + + /** + * NewsMapper constructor. + * + * @param IDBConnection $db Database connection + * @param Time $time Time class + * @param string $entity Entity class + */ + public function __construct( + IDBConnection $db, + Time $time, + string $entity + ) { + parent::__construct($db, static::TABLE_NAME, $entity); + $this->time = $time; + } + + public function update(Entity $entity): Entity + { + $entity->setLastModified($this->time->getMicroTime()); + return parent::update($entity); + } + + public function insert(Entity $entity): Entity + { + $entity->setLastModified($this->time->getMicroTime()); + return parent::insert($entity); + } + + /** + * Remove deleted items. + * + * @return void + */ + public function purgeDeleted(): void + { + $builder = $this->db->getQueryBuilder(); + $builder->delete($this->tableName) + ->where('deleted_at != 0') + ->execute() + ->execute(); + } + + /** + * Find all items. + * + * @return Entity[] + */ + abstract public function findAll(): array; + + /** + * Find all items for a user. + * + * @param string $userId ID of the user + * + * @return Entity[] + */ + abstract public function findAllFromUser(string $userId): array; + + /** + * Find item for a user. + * + * @param string $userId ID of the user + * @param int $id ID of the item + * + * @return Feed + * + * @throws DoesNotExistException The item is not found + * @throws MultipleObjectsReturnedException Multiple items found + */ + abstract public function findFromUser(string $userId, int $id): Entity; +} diff --git a/lib/Fetcher/FeedFetcher.php b/lib/Fetcher/FeedFetcher.php index 99d3d90ab..4e93424ba 100755 --- a/lib/Fetcher/FeedFetcher.php +++ b/lib/Fetcher/FeedFetcher.php @@ -64,7 +64,7 @@ class FeedFetcher implements IFeedFetcher * * @return bool */ - public function canHandle($url): bool + public function canHandle(string $url): bool { return true; } diff --git a/lib/Fetcher/IFeedFetcher.php b/lib/Fetcher/IFeedFetcher.php index 6500c2690..ce44ec104 100644 --- a/lib/Fetcher/IFeedFetcher.php +++ b/lib/Fetcher/IFeedFetcher.php @@ -43,5 +43,5 @@ interface IFeedFetcher * @return boolean if the fetcher can handle the url. This fetcher will be * used exclusively to fetch the feed and the items of the page */ - public function canHandle($url): bool; + public function canHandle(string $url): bool; } diff --git a/lib/Fetcher/YoutubeFetcher.php b/lib/Fetcher/YoutubeFetcher.php index 56adaae03..85aad5d59 100644 --- a/lib/Fetcher/YoutubeFetcher.php +++ b/lib/Fetcher/YoutubeFetcher.php @@ -37,9 +37,13 @@ class YoutubeFetcher implements IFeedFetcher /** - * This fetcher handles all the remaining urls therefore always returns true + * Check if the URL is a youtube URL by reformatting it. + * + * @param string $url the url that should be fetched + * + * @return bool */ - public function canHandle($url): bool + public function canHandle(string $url): bool { return $this->buildUrl($url) !== $url; } diff --git a/lib/Service/ServiceConflictException.php b/lib/Service/Exceptions/ServiceConflictException.php similarity index 93% rename from lib/Service/ServiceConflictException.php rename to lib/Service/Exceptions/ServiceConflictException.php index 95955eecb..4a1a80801 100644 --- a/lib/Service/ServiceConflictException.php +++ b/lib/Service/Exceptions/ServiceConflictException.php @@ -11,7 +11,7 @@ * @copyright 2012-2014 Bernhard Posselt */ -namespace OCA\News\Service; +namespace OCA\News\Service\Exceptions; class ServiceConflictException extends ServiceException { diff --git a/lib/Service/ServiceException.php b/lib/Service/Exceptions/ServiceException.php similarity index 93% rename from lib/Service/ServiceException.php rename to lib/Service/Exceptions/ServiceException.php index ef1aa1102..7ee53bef9 100644 --- a/lib/Service/ServiceException.php +++ b/lib/Service/Exceptions/ServiceException.php @@ -11,7 +11,7 @@ * @copyright 2012-2014 Bernhard Posselt */ -namespace OCA\News\Service; +namespace OCA\News\Service\Exceptions; class ServiceException extends \Exception { diff --git a/lib/Service/ServiceNotFoundException.php b/lib/Service/Exceptions/ServiceNotFoundException.php similarity index 93% rename from lib/Service/ServiceNotFoundException.php rename to lib/Service/Exceptions/ServiceNotFoundException.php index 65ba092d7..6a9bae192 100644 --- a/lib/Service/ServiceNotFoundException.php +++ b/lib/Service/Exceptions/ServiceNotFoundException.php @@ -11,7 +11,7 @@ * @copyright 2012-2014 Bernhard Posselt */ -namespace OCA\News\Service; +namespace OCA\News\Service\Exceptions; class ServiceNotFoundException extends ServiceException { diff --git a/lib/Service/ServiceValidationException.php b/lib/Service/Exceptions/ServiceValidationException.php similarity index 93% rename from lib/Service/ServiceValidationException.php rename to lib/Service/Exceptions/ServiceValidationException.php index 20485642b..8d1bf09c8 100644 --- a/lib/Service/ServiceValidationException.php +++ b/lib/Service/Exceptions/ServiceValidationException.php @@ -11,7 +11,7 @@ * @copyright 2012-2014 Bernhard Posselt */ -namespace OCA\News\Service; +namespace OCA\News\Service\Exceptions; class ServiceValidationException extends ServiceException { diff --git a/lib/Service/FeedService.php b/lib/Service/FeedService.php index 6d57c8027..55807b45a 100644 --- a/lib/Service/FeedService.php +++ b/lib/Service/FeedService.php @@ -18,7 +18,9 @@ use HTMLPurifier; use OCA\News\AppInfo\Application; use OCP\IConfig; -use OCP\ILogger; +use OCA\News\Service\Exceptions\ServiceConflictException; +use OCA\News\Service\Exceptions\ServiceNotFoundException; +use OCP\AppFramework\Db\Entity; use OCP\IL10N; use OCP\AppFramework\Db\DoesNotExistException; @@ -28,14 +30,20 @@ use OCA\News\Db\FeedMapper; use OCA\News\Db\ItemMapper; use OCA\News\Fetcher\Fetcher; use OCA\News\Utility\Time; +use Psr\Log\LoggerInterface; +/** + * Class LegacyFeedService + * + * @package OCA\News\Service + * @deprecated use FeedServiceV2 + */ class FeedService extends Service { private $feedFetcher; private $itemMapper; private $feedMapper; - private $logger; private $l10n; private $timeFactory; private $autoPurgeMinimumInterval; @@ -43,18 +51,19 @@ class FeedService extends Service private $loggerParams; public function __construct( - FeedMapper $feedMapper, + FeedMapper $legacyFeedMapper, Fetcher $feedFetcher, - ItemMapper $itemMapper, - ILogger $logger, + ItemMapper $legacyItemMapper, + LoggerInterface $logger, IL10N $l10n, Time $timeFactory, IConfig $config, HTMLPurifier $purifier ) { - parent::__construct($feedMapper); + parent::__construct($legacyFeedMapper, $logger); $this->feedFetcher = $feedFetcher; - $this->itemMapper = $itemMapper; + $this->feedMapper = $legacyFeedMapper; + $this->itemMapper = $legacyItemMapper; $this->logger = $logger; $this->l10n = $l10n; $this->timeFactory = $timeFactory; @@ -64,7 +73,6 @@ class FeedService extends Service Application::DEFAULT_SETTINGS['autoPurgeMinimumInterval'] ); $this->purifier = $purifier; - $this->feedMapper = $feedMapper; $this->loggerParams = ['app' => Application::NAME]; } @@ -75,7 +83,7 @@ class FeedService extends Service * * @return Feed[] */ - public function findAll($userId) + public function findAllForUser($userId): array { return $this->feedMapper->findAllFromUser($userId); } @@ -88,7 +96,7 @@ class FeedService extends Service */ public function findAllFromAllUsers() { - return $this->feedMapper->findAll(); + return $this->findAll(); } @@ -202,17 +210,17 @@ class FeedService extends Service /** * Updates a single feed * - * @param int $feedId the id of the feed that should be updated * @param string $userId the id of the user + * @param int $feedId the id of the feed that should be updated * @param bool $forceUpdate update even if the article exists already * * @throws ServiceNotFoundException if the feed does not exist * @return Feed the updated feed entity */ - public function update($feedId, $userId, $forceUpdate = false) + public function update(string $userId, int $feedId, $forceUpdate = false) { /** @var Feed $existingFeed */ - $existingFeed = $this->find($feedId, $userId); + $existingFeed = $this->find($userId, $feedId); if ($existingFeed->getPreventUpdate() === true) { return $existingFeed; @@ -316,7 +324,7 @@ class FeedService extends Service $this->feedMapper->update($existingFeed); - return $this->find($feedId, $userId); + return $this->find($userId, $feedId); } /** @@ -333,7 +341,7 @@ class FeedService extends Service $urlHash = md5($url); // build assoc array for fast access - $feeds = $this->findAll($userId); + $feeds = $this->findAllForUser($userId); $feedsDict = []; foreach ($feeds as $feed) { $feedsDict[$feed->getLink()] = $feed; @@ -402,9 +410,9 @@ class FeedService extends Service * * @throws ServiceNotFoundException when feed does not exist */ - public function markDeleted($feedId, $userId) + public function markDeleted(int $feedId, string $userId) { - $feed = $this->find($feedId, $userId); + $feed = $this->find($userId, $feedId); $feed->setDeletedAt($this->timeFactory->getTime()); $this->feedMapper->update($feed); } @@ -418,9 +426,9 @@ class FeedService extends Service * * @throws ServiceNotFoundException when feed does not exist */ - public function unmarkDeleted($feedId, $userId) + public function unmarkDeleted(int $feedId, string $userId) { - $feed = $this->find($feedId, $userId); + $feed = $this->find($userId, $feedId); $feed->setDeletedAt(0); $this->feedMapper->update($feed); } @@ -463,9 +471,9 @@ class FeedService extends Service } /** - * @param string $feedId ID of the feed. + * @param int $feedId ID of the feed. * @param string $userId ID of the user. - * @param array $diff An array containing the fields to update, e.g.: + * @param array $diff An array containing the fields to update, e.g.: * * [ * 'ordering' => 1, @@ -479,9 +487,9 @@ class FeedService extends Service * @throws ServiceNotFoundException if feed does not exist * @return Feed The patched feed */ - public function patch($feedId, $userId, $diff = []) + public function patch(int $feedId, string $userId, array $diff = []) { - $feed = $this->find($feedId, $userId); + $feed = $this->find($userId, $feedId); foreach ($diff as $attribute => $value) { $method = 'set' . ucfirst($attribute); @@ -494,9 +502,14 @@ class FeedService extends Service $feed->setHttpEtag(''); $feed->setHttpLastModified(0); $this->feedMapper->update($feed); - return $this->update($feedId, $userId, true); + return $this->update($userId, $feedId, true); } return $this->feedMapper->update($feed); } + + public function findAll(): array + { + return $this->feedMapper->findAll(); + } } diff --git a/lib/Service/FeedServiceV2.php b/lib/Service/FeedServiceV2.php new file mode 100644 index 000000000..58217a1ce --- /dev/null +++ b/lib/Service/FeedServiceV2.php @@ -0,0 +1,336 @@ + + * @author Bernhard Posselt + * @copyright 2012 Alessandro Cosentino + * @copyright 2012-2014 Bernhard Posselt + */ + +namespace OCA\News\Service; + +use FeedIo\Reader\ReadErrorException; +use HTMLPurifier; + +use OCA\News\Db\FeedMapperV2; +use OCA\News\Fetcher\FeedFetcher; +use OCA\News\Service\Exceptions\ServiceConflictException; +use OCA\News\Service\Exceptions\ServiceNotFoundException; +use OCP\AppFramework\Db\Entity; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\ILogger; +use OCP\IL10N; +use OCP\AppFramework\Db\DoesNotExistException; + +use OCA\News\Db\Feed; +use OCA\News\Db\Item; +use OCA\News\Db\FeedMapper; +use OCA\News\Db\ItemMapper; +use OCA\News\Fetcher\Fetcher; +use OCA\News\Config\Config; +use OCA\News\Utility\Time; +use Psr\Log\LoggerInterface; + +/** + * Class FeedService + * + * @package OCA\News\Service + */ +class FeedServiceV2 extends Service +{ + /** + * Class to fetch feeds. + * @var FeedFetcher + */ + protected $feedFetcher; + /** + * Items service. + * + * @var ItemServiceV2 + */ + protected $itemService; + /** + * HTML Purifier + * @var HTMLPurifier + */ + protected $purifier; + + /** + * FeedService constructor. + * + * @param FeedMapperV2 $mapper DB layer for feeds + * @param FeedFetcher $feedFetcher FeedIO interface + * @param ItemServiceV2 $itemService Service to manage items + * @param HTMLPurifier $purifier HTML Purifier + * @param LoggerInterface $logger Logger + */ + public function __construct( + FeedMapperV2 $mapper, + FeedFetcher $feedFetcher, + ItemServiceV2 $itemService, + HTMLPurifier $purifier, + LoggerInterface $logger + ) { + parent::__construct($mapper, $logger); + + $this->feedFetcher = $feedFetcher; + $this->itemService = $itemService; + $this->purifier = $purifier; + } + + /** + * Finds all feeds of a user + * + * @param string $userId the name of the user + * + * @return Feed[] + */ + public function findAllForUser(string $userId): array + { + return $this->mapper->findAllFromUser($userId); + } + + /** + * Finds a feed of a user + * + * @param string $userId the name of the user + * @param string $id the id of the feed + * + * @return Feed + * + * @throws DoesNotExistException + * @throws MultipleObjectsReturnedException + */ + public function findForUser(string $userId, string $id): Feed + { + return $this->mapper->findFromUser($userId, $id); + } + + /** + * @param int $id + * + * @return Feed[] + */ + public function findAllFromFolder(int $id): array + { + return $this->mapper->findAllFromFolder($id); + } + + /** + * Finds all feeds of a user and all items in it + * + * @param string $userId the name of the user + * + * @return Feed[] + */ + public function findAllForUserRecursive(string $userId): array + { + $feeds = $this->mapper->findAllFromUser($userId); + + foreach ($feeds as &$feed) { + $items = $this->itemService->findAllForFeed($feed->getId()); + $feed->items = $items; + } + return $feeds; + } + + /** + * Finds all feeds + * + * @return Feed[] + */ + public function findAll(): array + { + return $this->mapper->findAll(); + } + + /** + * Check if a feed exists for a user + * + * @param string $userID the name of the user + * @param string $url the feed URL + * + * @return bool + */ + public function existsForUser(string $userID, string $url): bool + { + try { + $this->mapper->findByURL($userID, $url); + return true; + } catch (DoesNotExistException $e) { + return false; + } + } + + + /** + * Creates a new feed + * + * @param string $userId Feed owner + * @param string $feedUrl Feed URL + * @param int $folderId Target folder, defaults to root + * @param string|null $title The OPML feed title + * @param string|null $user Basic auth username, if set + * @param string|null $password Basic auth password if username is set + * + * @return Feed the newly created feed + * + * @throws ServiceConflictException The feed already exists + * @throws ServiceNotFoundException The url points to an invalid feed + */ + public function create( + string $userId, + string $feedUrl, + int $folderId = 0, + bool $full_text = false, + ?string $title = null, + ?string $user = null, + ?string $password = null + ): Feed { + if ($this->existsForUser($userId, $feedUrl)) { + throw new ServiceConflictException('Feed with this URL exists'); + } + + try { + /** + * @var Feed $feed + * @var Item[] $items + */ + list($feed, $items) = $this->feedFetcher->fetch($feedUrl, true, $full_text, false, $user, $password); + if ($feed === null) { + throw new ServiceNotFoundException('Failed to fetch feed'); + } + + $feed->setFolderId($folderId) + ->setUserId($userId) + ->setArticlesPerUpdate(count($items)); + + if (!is_null($title)) { + $feed->setTitle($title); + } + + if (!is_null($user)) { + $feed->setBasicAuthUser($user) + ->setBasicAuthUser($password); + } + + $feed = $this->mapper->insert($feed); + + return $feed; + } catch (ReadErrorException $ex) { + $this->logger->debug($ex->getMessage()); + throw new ServiceNotFoundException($ex->getMessage()); + } + } + + + /** + * Update a feed + * + * @param Feed $feed Feed item + * @param bool $force update even if the article exists already + * + * @return Feed|Entity Database feed entity + */ + public function fetch(Feed $feed, bool $force = false) + { + if ($feed->getPreventUpdate() === true) { + return $feed; + } + + // for backwards compability it can be that the location is not set + // yet, if so use the url + $location = $feed->getLocation() ?? $feed->getUrl(); + + try { + /** + * @var Feed $feed + * @var Item[] $items + */ + list($fetchedFeed, $items) = $this->feedFetcher->fetch( + $location, + false, + $feed->getHttpLastModified(), + $feed->getFullTextEnabled(), + $feed->getBasicAuthUser(), + $feed->getBasicAuthPassword() + ); + + // if there is no feed it means that no update took place + if (!$fetchedFeed) { + return $feed; + } + + // update number of articles on every feed update + $itemCount = count($items); + + // this is needed to adjust to updates that add more items + // than when the feed was created. You can't update the count + // if it's lower because it may be due to the caching headers + // that were sent as the request and it might cause unwanted + // deletion and reappearing of feeds + if ($itemCount > $feed->getArticlesPerUpdate()) { + $feed->setArticlesPerUpdate($itemCount); + } + + $feed->setHttpLastModified($fetchedFeed->getHttpLastModified()); + $feed->setHttpEtag($fetchedFeed->getHttpEtag()); + $feed->setLocation($fetchedFeed->getLocation()); + + // insert items in reverse order because the first one is + // usually the newest item + for ($i = $itemCount - 1; $i >= 0; $i--) { + $item = $items[$i]; + $item->setFeedId($feed->getId()); + + $item->setTitle($item->getTitle()); + $item->setUrl($item->getUrl()); + $item->setAuthor($item->getAuthor()); + $item->setSearchIndex($item->getSearchIndex()); + $item->setRtl($item->getRtl()); + $item->setLastModified($item->getLastModified()); + $item->setPubDate($item->getPubDate()); + $item->setUpdatedDate($item->getUpdatedDate()); + $item->setEnclosureMime($item->getEnclosureMime()); + $item->setEnclosureLink($item->getEnclosureLink()); + $item->setBody($this->purifier->purify($item->getBody())); + + // update modes: 0 nothing, 1 set unread + if ($feed->getUpdateMode() === 1) { + $item->setUnread(true); + } + + $this->itemService->insertOrUpdate($item); + } + + // mark feed as successfully updated + $feed->setUpdateErrorCount(0); + $feed->setLastUpdateError(null); + } catch (ReadErrorException $ex) { + $feed->setUpdateErrorCount($feed->getUpdateErrorCount() + 1); + $feed->setLastUpdateError($ex->getMessage()); + } + + return $this->mapper->update($feed); + } + + public function delete(string $user, int $id) + { + $feed = $this->mapper->findFromUser($user, $id); + $this->mapper->delete($feed); + } + + public function purgeDeleted() + { + $this->mapper->purgeDeleted(); + } + + public function fetchAll() + { + return $this->mapper->findAll(); + } +} diff --git a/lib/Service/FolderService.php b/lib/Service/FolderService.php index 35eb77f95..ed95f81f4 100644 --- a/lib/Service/FolderService.php +++ b/lib/Service/FolderService.php @@ -15,11 +15,23 @@ namespace OCA\News\Service; use OCA\News\AppInfo\Application; use OCP\IConfig; +use OCA\News\Service\Exceptions\ServiceConflictException; +use OCA\News\Service\Exceptions\ServiceException; +use OCA\News\Service\Exceptions\ServiceNotFoundException; +use OCA\News\Service\Exceptions\ServiceValidationException; +use OCP\AppFramework\Db\Entity; use OCP\IL10N; use OCA\News\Db\Folder; use OCA\News\Db\FolderMapper; use OCA\News\Utility\Time; +use Psr\Log\LoggerInterface; +/** + * Class LegacyFolderService + * + * @package OCA\News\Service + * @deprecated use FolderServiceV2 + */ class FolderService extends Service { @@ -32,35 +44,37 @@ class FolderService extends Service FolderMapper $folderMapper, IL10N $l10n, Time $timeFactory, - IConfig $config + IConfig $config, + LoggerInterface $logger ) { - parent::__construct($folderMapper); - $this->l10n = $l10n; - $this->timeFactory = $timeFactory; + parent::__construct($folderMapper, $logger); + $this->l10n = $l10n; + $this->timeFactory = $timeFactory; + $this->folderMapper = $folderMapper; $this->autoPurgeMinimumInterval = $config->getAppValue( Application::NAME, 'autoPurgeMinimumInterval', Application::DEFAULT_SETTINGS['autoPurgeMinimumInterval'] ); - $this->folderMapper = $folderMapper; } /** - * Returns all folders of a user + * Finds all folders of a user * - * @param string $userId the name of the user - * @return array of folders + * @param string $userId the name of the user + * + * @return Folder[] */ - public function findAll($userId) + public function findAllForUser(string $userId): array { return $this->folderMapper->findAllFromUser($userId); } - private function validateFolder($folderName, $userId) + private function validateFolder(string $folderName, string $userId) { - $existingFolders = - $this->folderMapper->findByName($folderName, $userId); + $existingFolders + = $this->folderMapper->findByName($folderName, $userId); if (count($existingFolders) > 0) { throw new ServiceConflictException( $this->l10n->t('Can not add folder: Exists already') @@ -78,15 +92,16 @@ class FolderService extends Service /** * Creates a new folder * - * @param string $folderName the name of the folder - * @param string $userId the name of the user for whom it should be created - * @param int $parentId the parent folder id, deprecated we don't nest + * @param string $folderName the name of the folder + * @param string $userId the name of the user for whom it should be created + * @param int $parentId the parent folder id, deprecated we don't nest * folders - * @throws ServiceConflictException if name exists already + * + * @return Folder|Entity the newly created folder * @throws ServiceValidationException if the folder has invalid parameters - * @return Folder the newly created folder + * @throws ServiceConflictException if name exists already */ - public function create($folderName, $userId, $parentId = 0) + public function create(string $folderName, string $userId, int $parentId = 0) { $this->validateFolder($folderName, $userId); @@ -103,9 +118,9 @@ class FolderService extends Service /** * @throws ServiceException if the folder does not exist */ - public function open($folderId, $opened, $userId) + public function open(int $folderId, bool $opened, string $userId) { - $folder = $this->find($folderId, $userId); + $folder = $this->find($userId, $folderId); $folder->setOpened($opened); $this->folderMapper->update($folder); } @@ -114,19 +129,20 @@ class FolderService extends Service /** * Renames a folder * - * @param int $folderId the id of the folder that should be deleted - * @param string $folderName the new name of the folder - * @param string $userId the name of the user for security reasons - * @throws ServiceConflictException if name exists already + * @param int $folderId the id of the folder that should be deleted + * @param string $folderName the new name of the folder + * @param string $userId the name of the user for security reasons + * + * @return Folder the updated folder * @throws ServiceValidationException if the folder has invalid parameters * @throws ServiceNotFoundException if the folder does not exist - * @return Folder the updated folder + * @throws ServiceConflictException if name exists already */ - public function rename($folderId, $folderName, $userId) + public function rename(int $folderId, string $folderName, string $userId) { $this->validateFolder($folderName, $userId); - $folder = $this->find($folderId, $userId); + $folder = $this->find($userId, $folderId); $folder->setName($folderName); return $this->folderMapper->update($folder); @@ -136,13 +152,14 @@ class FolderService extends Service /** * Use this to mark a folder as deleted. That way it can be un-deleted * - * @param int $folderId the id of the folder that should be deleted - * @param string $userId the name of the user for security reasons + * @param int $folderId the id of the folder that should be deleted + * @param string $userId the name of the user for security reasons + * * @throws ServiceNotFoundException when folder does not exist */ - public function markDeleted($folderId, $userId) + public function markDeleted(int $folderId, string $userId) { - $folder = $this->find($folderId, $userId); + $folder = $this->find($userId, $folderId); $folder->setDeletedAt($this->timeFactory->getTime()); $this->folderMapper->update($folder); } @@ -151,13 +168,14 @@ class FolderService extends Service /** * Use this to restore a folder * - * @param int $folderId the id of the folder that should be restored - * @param string $userId the name of the user for security reasons + * @param int $folderId the id of the folder that should be restored + * @param string $userId the name of the user for security reasons + * * @throws ServiceNotFoundException when folder does not exist */ - public function unmarkDeleted($folderId, $userId) + public function unmarkDeleted(int $folderId, string $userId) { - $folder = $this->find($folderId, $userId); + $folder = $this->find($userId, $folderId); $folder->setDeletedAt(0); $this->folderMapper->update($folder); } @@ -171,7 +189,7 @@ class FolderService extends Service * entries in a given interval to give the user a chance to undo the * deletion */ - public function purgeDeleted($userId = null, $useInterval = true) + public function purgeDeleted(?string $userId = null, bool $useInterval = true) { $deleteOlderThan = null; @@ -193,8 +211,13 @@ class FolderService extends Service * * @param string $userId the name of the user */ - public function deleteUser($userId) + public function deleteUser(string $userId) { $this->folderMapper->deleteUser($userId); } + + public function findAll(): array + { + return $this->mapper->findAll(); + } } diff --git a/lib/Service/FolderServiceV2.php b/lib/Service/FolderServiceV2.php new file mode 100644 index 000000000..0e15ddde5 --- /dev/null +++ b/lib/Service/FolderServiceV2.php @@ -0,0 +1,102 @@ + + * @author Bernhard Posselt + * @copyright 2012 Alessandro Cosentino + * @copyright 2012-2014 Bernhard Posselt + */ + +namespace OCA\News\Service; + +use OCA\News\Db\Feed; +use OCA\News\Db\FeedMapperV2; +use OCA\News\Db\Folder; +use OCA\News\Db\FolderMapperV2; +use Psr\Log\LoggerInterface; + +/** + * Class FolderService + * + * @package OCA\News\Service + */ +class FolderServiceV2 extends Service +{ + /** + * @var FeedServiceV2 + */ + private $feedService; + + public function __construct( + FolderMapperV2 $mapper, + FeedServiceV2 $feedService, + LoggerInterface $logger + ) { + parent::__construct($mapper, $logger); + $this->feedService = $feedService; + } + + /** + * Finds all folders of a user + * + * @param string $userId the name of the user + * + * @return Folder[] + */ + public function findAllForUser(string $userId): array + { + return $this->mapper->findAllFromUser($userId); + } + + /** + * @param string $userId + * + * @return Folder[] + */ + public function findAllForUserRecursive(string $userId): array + { + $folders = $this->findAllForUser($userId); + foreach ($folders as &$folder) { + $feeds = $this->feedService->findAllFromFolder($folder->getId()); + $folder->feeds = $feeds; + } + + return $folders; + } + + /** + * Find all folders. + * + * @return Folder[] + */ + public function findAll(): array + { + return $this->mapper->findAll(); + } + + public function create(string $userId, string $name, int $parent = 0): void + { + $folder = new Folder(); + $folder->setUserId($userId); + $folder->setName($name); + $folder->setParentId($parent); + + $this->mapper->insert($folder); + } + + public function delete(string $user, int $id) + { + $entity = $this->mapper->findFromUser($user, $id); + + $this->mapper->delete($entity); + } + + public function purgeDeleted() + { + $this->mapper->purgeDeleted(); + } +} diff --git a/lib/Service/ItemService.php b/lib/Service/ItemService.php index 5e1660cc0..3a0270658 100644 --- a/lib/Service/ItemService.php +++ b/lib/Service/ItemService.php @@ -15,13 +15,22 @@ namespace OCA\News\Service; use OCA\News\AppInfo\Application; use OCA\News\Db\Item; +use OCA\News\Service\Exceptions\ServiceNotFoundException; +use OCP\AppFramework\Db\Entity; use OCP\IConfig; use OCP\AppFramework\Db\DoesNotExistException; use OCA\News\Db\ItemMapper; use OCA\News\Db\FeedType; use OCA\News\Utility\Time; +use Psr\Log\LoggerInterface; +/** + * Class LegacyItemService + * + * @package OCA\News\Service + * @deprecated use ItemServiceV2 + */ class ItemService extends Service { @@ -32,12 +41,13 @@ class ItemService extends Service public function __construct( ItemMapper $itemMapper, Time $timeFactory, - IConfig $config + IConfig $config, + LoggerInterface $logger ) { - parent::__construct($itemMapper); + parent::__construct($itemMapper, $logger); + $this->config = $config; $this->timeFactory = $timeFactory; $this->itemMapper = $itemMapper; - $this->config = $config; } @@ -96,7 +106,7 @@ class ItemService extends Service * or body * @return array of items */ - public function findAll( + public function findAllItems( $id, $type, $limit, @@ -128,7 +138,7 @@ class ItemService extends Service $search ); default: - return $this->itemMapper->findAll( + return $this->itemMapper->findAllItems( $limit, $offset, $type, @@ -140,6 +150,11 @@ class ItemService extends Service } } + public function findAllForUser(string $userId): array + { + return $this->itemMapper->findAllFromUser($userId); + } + /** * Star or unstar an item @@ -320,4 +335,9 @@ class ItemService extends Service { $this->itemMapper->updateSearchIndices(); } + + public function findAll(): array + { + return $this->mapper->findAll(); + } } diff --git a/lib/Service/ItemServiceV2.php b/lib/Service/ItemServiceV2.php new file mode 100644 index 000000000..879b2908f --- /dev/null +++ b/lib/Service/ItemServiceV2.php @@ -0,0 +1,104 @@ + + * @author Bernhard Posselt + * @copyright 2020 Sean Molenaar + */ + +namespace OCA\News\Service; + +use OCA\News\AppInfo\Application; +use OCA\News\Db\Item; +use OCA\News\Db\ItemMapperV2; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IConfig; +use Psr\Log\LoggerInterface; + +/** + * Class ItemService + * + * @package OCA\News\Service + */ +class ItemServiceV2 extends Service +{ + + /** + * @var IConfig + */ + protected $config; + + /** + * ItemService constructor. + * + * @param ItemMapperV2 $mapper + * @param IConfig $config + * @param LoggerInterface $logger + */ + public function __construct( + ItemMapperV2 $mapper, + IConfig $config, + LoggerInterface $logger + ) { + parent::__construct($mapper, $logger); + $this->config = $config; + } + + /** + * Finds all items of a user + * + * @param string $userId the name of the user + * + * @return Item[] + */ + public function findAllForUser($userId): array + { + return $this->mapper->findAllFromUser($userId); + } + + /** + * Find all items + * + * @return Item[] + */ + public function findAll(): array + { + return $this->mapper->findAll(); + } + + public function insertOrUpdate(Item $item) + { + try { + $db_item = $this->mapper->findByGuidHash($item->getGuidHash()); + $item->setId($db_item->getId()); + $this->mapper->update($item); + } catch (DoesNotExistException $exception) { + $this->mapper->insert($item); + } + } + + public function findAllForFeed(int $feedId): array + { + return $this->mapper->findAllForFeed($feedId); + } + + public function purgeOverThreshold($threshold = null) + { + + $threshold = (int) $threshold ?? $this->config->getAppValue( + Application::NAME, + 'autoPurgeCount', + Application::DEFAULT_SETTINGS['autoPurgeCount'] + ); + + if ($threshold === 0) { + return ''; + } + + return $this->mapper->deleteOverThreshold($threshold); + } +} diff --git a/lib/Service/Service.php b/lib/Service/Service.php index ac9b6e1a3..33397bc0f 100644 --- a/lib/Service/Service.php +++ b/lib/Service/Service.php @@ -13,33 +13,72 @@ namespace OCA\News\Service; -use \OCP\AppFramework\Db\DoesNotExistException; -use \OCP\AppFramework\Db\MultipleObjectsReturnedException; - -use \OCA\News\Db\NewsMapper; +use OCA\News\Db\NewsMapper; +use OCA\News\Db\NewsMapperV2; +use OCA\News\Service\Exceptions\ServiceNotFoundException; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\Entity; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use Psr\Log\LoggerInterface; +/** + * Class Service + * + * @package OCA\News\Service + */ abstract class Service { - + /** + * @var NewsMapper|NewsMapperV2 + */ protected $mapper; + /** + * @var LoggerInterface + */ + protected $logger; - public function __construct(NewsMapper $mapper) + /** + * Service constructor. + * + * @param NewsMapper|NewsMapperV2 $mapper + * @param LoggerInterface $logger + */ + public function __construct($mapper, LoggerInterface $logger) { $this->mapper = $mapper; + $this->logger = $logger; } + /** + * Finds all items of a user + * + * @param string $userId the name of the user + * + * @return Entity[] + */ + abstract public function findAllForUser(string $userId): array; + + /** + * Finds all items + * + * @return Entity[] + */ + abstract public function findAll(): array; + /** * Delete an entity * - * @param int $id the id of the entity - * @param string $userId the name of the user for security reasons + * @param int $id the id of the entity + * @param string $userId the name of the user for security reasons + * * @throws ServiceNotFoundException if the entity does not exist, or there * are more than one of it */ - public function delete($id, $userId) + public function delete(string $userId, int $id) { - $entity = $this->find($id, $userId); + $entity = $this->find($userId, $id); + $this->mapper->delete($entity); } @@ -47,16 +86,17 @@ abstract class Service /** * Finds an entity by id * - * @param int $id the id of the entity - * @param string $userId the name of the user for security reasons + * @param int $id the id of the entity + * @param string $userId the name of the user for security reasons + * + * @return \OCP\AppFramework\Db\Entity the entity * @throws ServiceNotFoundException if the entity does not exist, or there * are more than one of it - * @return \OCP\AppFramework\Db\Entity the entity */ - public function find($id, $userId) + public function find(string $userId, int $id): Entity { try { - return $this->mapper->find($id, $userId); + return $this->mapper->find($userId, $id); } catch (DoesNotExistException $ex) { throw new ServiceNotFoundException($ex->getMessage()); } catch (MultipleObjectsReturnedException $ex) { diff --git a/lib/Service/StatusService.php b/lib/Service/StatusService.php index f926258b6..c2daa0c0b 100644 --- a/lib/Service/StatusService.php +++ b/lib/Service/StatusService.php @@ -29,14 +29,14 @@ class StatusService public function __construct( IConfig $settings, IDBConnection $connection, - $AppName + string $AppName ) { $this->settings = $settings; $this->appName = $AppName; $this->connection = $connection; } - public function isProperlyConfigured() + public function isProperlyConfigured(): bool { $cronMode = $this->settings->getSystemValue('backgroundjobs_mode'); $cronOff = !$this->settings->getAppValue( @@ -50,7 +50,7 @@ class StatusService } - public function getStatus() + public function getStatus(): array { $version = $this->settings->getAppValue( $this->appName, diff --git a/lib/Utility/Updater.php b/lib/Service/UpdaterService.php similarity index 70% rename from lib/Utility/Updater.php rename to lib/Service/UpdaterService.php index 01e07291b..dadd667f5 100644 --- a/lib/Utility/Updater.php +++ b/lib/Service/UpdaterService.php @@ -12,13 +12,17 @@ */ -namespace OCA\News\Utility; +namespace OCA\News\Service; -use \OCA\News\Service\FolderService; +use OCA\News\Db\ItemMapperV2; +use OCA\News\Service\FeedServiceV2; +use OCA\News\Service\FolderServiceV2; +use OCA\News\Service\ItemServiceV2; +use \OCA\News\Service\LegacyFolderService; use \OCA\News\Service\FeedService; use \OCA\News\Service\ItemService; -class Updater +class UpdaterService { /** @@ -37,9 +41,9 @@ class Updater private $itemService; public function __construct( - FolderService $folderService, - FeedService $feedService, - ItemService $itemService + FolderServiceV2 $folderService, + FeedServiceV2 $feedService, + ItemServiceV2 $itemService ) { $this->folderService = $folderService; $this->feedService = $feedService; @@ -56,12 +60,12 @@ class Updater public function update() { - $this->feedService->updateAll(); + $this->feedService->fetchAll(); } public function afterUpdate() { - $this->itemService->autoPurgeOld(); + $this->itemService->purgeOverThreshold(null); } } diff --git a/lib/Utility/OPMLExporter.php b/lib/Utility/OPMLExporter.php index 189cf8d1e..8c21fa204 100644 --- a/lib/Utility/OPMLExporter.php +++ b/lib/Utility/OPMLExporter.php @@ -13,6 +13,8 @@ namespace OCA\News\Utility; +use \DOMDocument; +use \DOMElement; use OCA\News\Db\Feed; /** @@ -26,11 +28,11 @@ class OPMLExporter * * @param \OCA\News\Db\Folder[] $folders * @param \OCA\News\Db\Feed[] $feeds - * @return \DomDocument the document + * @return DOMDocument the document */ - public function build($folders, $feeds) + public function build(array $folders, array $feeds) { - $document = new \DomDocument('1.0', 'UTF-8'); + $document = new DOMDocument('1.0', 'UTF-8'); $document->formatOutput = true; $root = $document->createElement('opml'); @@ -81,10 +83,10 @@ class OPMLExporter /** * @param Feed $feed - * @param \DOMDocument $document - * @return \DOMElement + * @param DOMDocument $document + * @return DOMElement */ - protected function createFeedOutline($feed, $document) + protected function createFeedOutline(Feed $feed, DOMDocument $document) { $feedOutline = $document->createElement('outline'); $feedOutline->setAttribute('title', $feed->getTitle()); @@ -92,6 +94,7 @@ class OPMLExporter $feedOutline->setAttribute('type', 'rss'); $feedOutline->setAttribute('xmlUrl', $feed->getUrl()); $feedOutline->setAttribute('htmlUrl', $feed->getLink()); + return $feedOutline; } } diff --git a/lib/Utility/Time.php b/lib/Utility/Time.php index 739204f6d..aee1e6358 100644 --- a/lib/Utility/Time.php +++ b/lib/Utility/Time.php @@ -13,7 +13,7 @@ namespace OCA\News\Utility; class Time { - public function getTime() + public function getTime(): int { return time(); } @@ -21,7 +21,7 @@ class Time /** * @return int the current unix time in miliseconds */ - public function getMicroTime() + public function getMicroTime(): int { list($millisecs, $secs) = explode(" ", microtime()); return $secs . substr($millisecs, 2, 6); diff --git a/tests/Integration/Db/FeedMapperTest.php b/tests/Integration/Db/FeedMapperTest.php index 626b8f9a9..f07103d10 100644 --- a/tests/Integration/Db/FeedMapperTest.php +++ b/tests/Integration/Db/FeedMapperTest.php @@ -25,7 +25,7 @@ class FeedMapperTest extends IntegrationTest $feed = new FeedFixture(); $feed = $this->feedMapper->insert($feed); - $fetched = $this->feedMapper->find($feed->getId(), $this->user); + $fetched = $this->feedMapper->find($this->user, $feed->getId()); $this->assertInstanceOf(Feed::class, $fetched); $this->assertEquals($feed->getLink(), $fetched->getLink()); @@ -34,7 +34,7 @@ class FeedMapperTest extends IntegrationTest public function testFindNotExisting() { $this->expectException('OCP\AppFramework\Db\DoesNotExistException'); - $this->feedMapper->find(0, $this->user); + $this->feedMapper->find($this->user, 0); } @@ -252,22 +252,14 @@ class FeedMapperTest extends IntegrationTest $this->assertCount(4, $this->feedMapper->findAllFromUser($this->user)); - $items = $this->itemMapper->findAll(100, 0, 0, true, false, $this->user); + $items = $this->itemMapper->findAllItems(100, 0, 0, true, false, $this->user); $this->assertCount(9, $items); $this->feedMapper->deleteUser($this->user); $this->assertCount(0, $this->feedMapper->findAllFromUser($this->user)); - $items = $this->itemMapper->findAll(100, 0, 0, true, false, $this->user); + $items = $this->itemMapper->findAllItems(100, 0, 0, true, false, $this->user); $this->assertCount(0, $items); } - - /** - * @coversNothing - */ - public function testDeleteUserNotExisting() - { - $this->feedMapper->deleteUser('notexistinguser'); - } } diff --git a/tests/Integration/Db/ItemMapperTest.php b/tests/Integration/Db/ItemMapperTest.php index 026834e6b..28a6c3e11 100644 --- a/tests/Integration/Db/ItemMapperTest.php +++ b/tests/Integration/Db/ItemMapperTest.php @@ -27,7 +27,7 @@ class ItemMapperTest extends IntegrationTest $item = $this->itemMapper->insert($item); - $fetched = $this->itemMapper->find($item->getId(), $this->user); + $fetched = $this->itemMapper->find($this->user, $item->getId()); $this->assertEquals($item->getTitle(), $fetched->getTitle()); } @@ -44,7 +44,7 @@ class ItemMapperTest extends IntegrationTest } /** - * @expectedException OCP\AppFramework\Db\DoesNotExistException + * @expectedException \OCP\AppFramework\Db\DoesNotExistException */ public function testFindNotFoundWhenDeletedFeed() { @@ -52,12 +52,12 @@ class ItemMapperTest extends IntegrationTest $this->loadFixtures('default'); $id = $this->whereTitleId('not found feed'); - $this->itemMapper->find($id, $this->user); + $this->itemMapper->find($this->user, $id); } /** - * @expectedException OCP\AppFramework\Db\DoesNotExistException + * @expectedException \OCP\AppFramework\Db\DoesNotExistException */ public function testFindNotFoundWhenDeletedFolder() { @@ -66,7 +66,7 @@ class ItemMapperTest extends IntegrationTest $id = $this->whereTitleId('not found folder'); - $this->itemMapper->find($id, $this->user); + $this->itemMapper->find($this->user, $id); } @@ -76,15 +76,15 @@ class ItemMapperTest extends IntegrationTest $this->itemMapper->deleteReadOlderThanThreshold(1); - $this->itemMapper->find($this->whereTitleId('a title1'), $this->user); - $this->itemMapper->find($this->whereTitleId('a title2'), $this->user); - $this->itemMapper->find($this->whereTitleId('a title3'), $this->user); - $this->itemMapper->find($this->whereTitleId('del3'), $this->user); - $this->itemMapper->find($this->whereTitleId('del4'), $this->user); + $this->itemMapper->find($this->user, $this->whereTitleId('a title1')); + $this->itemMapper->find($this->user, $this->whereTitleId('a title2')); + $this->itemMapper->find($this->user, $this->whereTitleId('a title3')); + $this->itemMapper->find($this->user, $this->whereTitleId('del3')); + $this->itemMapper->find($this->user, $this->whereTitleId('del4')); } /** - * @expectedException OCP\AppFramework\Db\DoesNotExistException + * @expectedException \OCP\AppFramework\Db\DoesNotExistException */ public function testDeleteOlderThanThresholdOne() { @@ -94,11 +94,11 @@ class ItemMapperTest extends IntegrationTest $this->deleteReadOlderThanThreshold(); - $this->itemMapper->find($id, $this->user); + $this->itemMapper->find($this->user, $id); } /** - * @expectedException OCP\AppFramework\Db\DoesNotExistException + * @expectedException \OCP\AppFramework\Db\DoesNotExistException */ public function testDeleteOlderThanThresholdTwo() { @@ -108,7 +108,7 @@ class ItemMapperTest extends IntegrationTest $this->deleteReadOlderThanThreshold(); - $this->itemMapper->find($id, $this->user); + $this->itemMapper->find($this->user, $id); } @@ -127,24 +127,24 @@ class ItemMapperTest extends IntegrationTest $this->itemMapper->readAll(PHP_INT_MAX, 10, $this->user); - $items = $this->itemMapper->findAll( + $items = $this->itemMapper->findAllItems( 30, 0, 0, false, false, $this->user ); $this->assertEquals(0, count($items)); $itemId = $this->whereTitleId('a title1'); - $item = $this->itemMapper->find($itemId, $this->user); + $item = $this->itemMapper->find($this->user, $itemId); $this->assertEquals(10, $item->getLastModified()); $itemId = $this->whereTitleId('a title3'); - $item = $this->itemMapper->find($itemId, $this->user); + $item = $this->itemMapper->find($this->user, $itemId); $this->assertEquals(10, $item->getLastModified()); $itemId = $this->whereTitleId('a title9'); - $item = $this->itemMapper->find($itemId, $this->user); + $item = $this->itemMapper->find($this->user, $itemId); $this->assertEquals(10, $item->getLastModified()); } @@ -159,24 +159,24 @@ class ItemMapperTest extends IntegrationTest $folderId, PHP_INT_MAX, 10, $this->user ); - $items = $this->itemMapper->findAll( + $items = $this->itemMapper->findAllItems( 30, 0, 0, false, false, $this->user ); $this->assertEquals(1, count($items)); $item = $this->findItemByTitle('a title1'); - $item = $this->itemMapper->find($item->getId(), $this->user); + $item = $this->itemMapper->find($this->user, $item->getId()); $this->assertEquals(10, $item->getLastModified()); $item = $this->findItemByTitle('a title3'); - $item = $this->itemMapper->find($item->getId(), $this->user); + $item = $this->itemMapper->find($this->user, $item->getId()); $this->assertEquals(10, $item->getLastModified()); $item = $this->findItemByTitle('a title9'); - $item = $this->itemMapper->find($item->getId(), $this->user); + $item = $this->itemMapper->find($this->user, $item->getId()); $this->assertTrue($item->isUnread()); } @@ -191,24 +191,24 @@ class ItemMapperTest extends IntegrationTest $feedId, PHP_INT_MAX, 10, $this->user ); - $items = $this->itemMapper->findAll( + $items = $this->itemMapper->findAllItems( 30, 0, 0, false, false, $this->user ); $this->assertEquals(2, count($items)); $item = $this->findItemByTitle('a title9'); - $item = $this->itemMapper->find($item->getId(), $this->user); + $item = $this->itemMapper->find($this->user, $item->getId()); $this->assertEquals(10, $item->getLastModified()); $item = $this->findItemByTitle('a title3'); - $item = $this->itemMapper->find($item->getId(), $this->user); + $item = $this->itemMapper->find($this->user, $item->getId()); $this->assertTrue($item->isUnread()); $item = $this->findItemByTitle('a title1'); - $item = $this->itemMapper->find($item->getId(), $this->user); + $item = $this->itemMapper->find($this->user, $item->getId()); $this->assertTrue($item->isUnread()); } diff --git a/tests/Integration/IntegrationTest.php b/tests/Integration/IntegrationTest.php index 13969e7c3..276c6c394 100644 --- a/tests/Integration/IntegrationTest.php +++ b/tests/Integration/IntegrationTest.php @@ -39,22 +39,22 @@ abstract class IntegrationTest extends \Test\TestCase protected $userPassword = 'test'; /** - * @var ItemMapper + * @var ItemMapper */ protected $itemMapper; /** - * @var FeedMapper + * @var FeedMapper */ protected $feedMapper; /** - * @var FolderMapper + * @var FolderMapper */ protected $folderMapper; /** - * @var IAppContainer + * @var IAppContainer */ protected $container; @@ -72,7 +72,7 @@ abstract class IntegrationTest extends \Test\TestCase $this->folderMapper = $this->container->query(FolderMapper::class); } - protected function findItemByTitle($title) + protected function findItemByTitle($title) { // db logic in app code, negligible since its a test $items = $this->itemMapper->where(['title' => $title]); @@ -97,7 +97,7 @@ abstract class IntegrationTest extends \Test\TestCase return $result; } - protected function findFolderByName($name) + protected function findFolderByName($name) { return $this->folderMapper->where( [ @@ -107,7 +107,7 @@ abstract class IntegrationTest extends \Test\TestCase )[0]; } - protected function findFeedByTitle($title) + protected function findFeedByTitle($title) { return $this->feedMapper->where( [ @@ -120,7 +120,7 @@ abstract class IntegrationTest extends \Test\TestCase /** * @param string $name loads fixtures from a given file */ - protected function loadFixtures($name) + protected function loadFixtures($name) { $fixtures = include __DIR__ . '/Fixtures/data/' . $name . '.php'; if (array_key_exists('folders', $fixtures)) { @@ -131,7 +131,7 @@ abstract class IntegrationTest extends \Test\TestCase } } - protected function loadFolderFixtures(array $folderFixtures=[]) + protected function loadFolderFixtures(array $folderFixtures = []) { foreach ($folderFixtures as $folderFixture) { $folder = new FolderFixture($folderFixture); @@ -140,7 +140,7 @@ abstract class IntegrationTest extends \Test\TestCase } } - protected function loadFeedFixtures(array $feedFixtures=[], $folderId=0) + protected function loadFeedFixtures(array $feedFixtures = [], $folderId = 0) { foreach ($feedFixtures as $feedFixture) { $feed = new FeedFixture($feedFixture); @@ -153,7 +153,7 @@ abstract class IntegrationTest extends \Test\TestCase } } - protected function loadItemFixtures(array $itemFixtures=[], $feedId) + protected function loadItemFixtures(array $itemFixtures, $feedId) { foreach ($itemFixtures as $itemFixture) { $item = new ItemFixture($itemFixture); @@ -168,7 +168,7 @@ abstract class IntegrationTest extends \Test\TestCase * @param Entity $fixture * @return int the id */ - protected function loadFixture(Entity $fixture) + protected function loadFixture(Entity $fixture) { if ($fixture instanceof FeedFixture) { return $this->feedMapper->insert($fixture)->getId(); @@ -187,7 +187,7 @@ abstract class IntegrationTest extends \Test\TestCase * @param $user * @param $password */ - protected function setupUser($user, $password) + protected function setupUser($user, $password) { $userManager = $this->container->query(IUserManager::class); $userManager->createUser($user, $password); @@ -200,7 +200,7 @@ abstract class IntegrationTest extends \Test\TestCase * * @param $user */ - protected function tearDownUser($user) + protected function tearDownUser($user) { $userManager = $this->container->query(IUserManager::class); @@ -216,7 +216,7 @@ abstract class IntegrationTest extends \Test\TestCase * * @param string $user */ - protected function clearUserNewsDatabase($user) + protected function clearUserNewsDatabase($user) { $sql = [ 'DELETE FROM `*PREFIX*news_items` WHERE `feed_id` IN diff --git a/tests/Unit/Controller/FeedApiControllerTest.php b/tests/Unit/Controller/FeedApiControllerTest.php index c889a54b3..1dbac6f42 100644 --- a/tests/Unit/Controller/FeedApiControllerTest.php +++ b/tests/Unit/Controller/FeedApiControllerTest.php @@ -18,10 +18,11 @@ namespace OCA\News\Tests\Unit\Controller; use OCA\News\Controller\FeedApiController; use OCA\News\Service\FeedService; use OCA\News\Service\ItemService; +use OCA\News\Utility\PsrLogger; use \OCP\AppFramework\Http; -use \OCA\News\Service\ServiceNotFoundException; -use \OCA\News\Service\ServiceConflictException; +use \OCA\News\Service\Exceptions\ServiceNotFoundException; +use \OCA\News\Service\Exceptions\ServiceConflictException; use \OCA\News\Db\Feed; use OCP\ILogger; use OCP\IRequest; @@ -29,6 +30,7 @@ use OCP\IUser; use OCP\IUserSession; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class FeedApiControllerTest extends TestCase { @@ -36,10 +38,7 @@ class FeedApiControllerTest extends TestCase private $feedService; private $itemService; private $feedAPI; - private $appName; - private $userSession; private $user; - private $request; private $msg; private $logger; private $loggerParams; @@ -47,20 +46,20 @@ class FeedApiControllerTest extends TestCase protected function setUp(): void { $this->loggerParams = ['hi']; - $this->logger = $this->getMockBuilder(ILogger::class) + $this->logger = $this->getMockBuilder(LoggerInterface::class) ->disableOriginalConstructor() ->getMock(); - $this->appName = 'news'; - $this->request = $this->getMockBuilder(IRequest::class) + $appName = 'news'; + $request = $this->getMockBuilder(IRequest::class) ->disableOriginalConstructor() ->getMock(); - $this->userSession = $this->getMockBuilder(IUserSession::class) + $userSession = $this->getMockBuilder(IUserSession::class) ->disableOriginalConstructor() ->getMock(); $this->user = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() ->getMock(); - $this->userSession->expects($this->any()) + $userSession->expects($this->any()) ->method('getUser') ->will($this->returnValue($this->user)); $this->user->expects($this->any()) @@ -73,9 +72,9 @@ class FeedApiControllerTest extends TestCase ->disableOriginalConstructor() ->getMock(); $this->feedAPI = new FeedApiController( - $this->appName, - $this->request, - $this->userSession, + $appName, + $request, + $userSession, $this->feedService, $this->itemService, $this->logger, @@ -100,7 +99,7 @@ class FeedApiControllerTest extends TestCase ->with($this->equalTo($this->user->getUID())) ->will($this->returnValue($newestItemId)); $this->feedService->expects($this->once()) - ->method('findAll') + ->method('findAllForUser') ->with($this->equalTo($this->user->getUID())) ->will($this->returnValue($feeds)); @@ -130,7 +129,7 @@ class FeedApiControllerTest extends TestCase ->with($this->equalTo($this->user->getUID())) ->will($this->throwException(new ServiceNotFoundException(''))); $this->feedService->expects($this->once()) - ->method('findAll') + ->method('findAllForUser') ->with($this->equalTo($this->user->getUID())) ->will($this->returnValue($feeds)); @@ -377,7 +376,7 @@ class FeedApiControllerTest extends TestCase $this->feedService->expects($this->once()) ->method('update') - ->with($this->equalTo($feedId), $this->equalTo($userId)); + ->with($userId, $feedId); $this->feedAPI->update($userId, $feedId); } @@ -392,10 +391,7 @@ class FeedApiControllerTest extends TestCase ->will($this->throwException(new \Exception($this->msg))); $this->logger->expects($this->once()) ->method('debug') - ->with( - $this->equalTo('Could not update feed ' . $this->msg), - $this->equalTo($this->loggerParams) - ); + ->with('Could not update feed ' . $this->msg); $this->feedAPI->update($userId, $feedId); diff --git a/tests/Unit/Controller/FeedControllerTest.php b/tests/Unit/Controller/FeedControllerTest.php index 9a3114da0..7498d0ccb 100644 --- a/tests/Unit/Controller/FeedControllerTest.php +++ b/tests/Unit/Controller/FeedControllerTest.php @@ -21,8 +21,8 @@ use OCP\AppFramework\Http; use OCA\News\Db\Feed; use OCA\News\Db\FeedType; -use OCA\News\Service\ServiceNotFoundException; -use OCA\News\Service\ServiceConflictException; +use OCA\News\Service\Exceptions\ServiceNotFoundException; +use OCA\News\Service\Exceptions\ServiceConflictException; use OCP\IConfig; use OCP\IRequest; @@ -93,7 +93,7 @@ class FeedControllerTest extends TestCase 'starred' => 13 ]; $this->feedService->expects($this->once()) - ->method('findAll') + ->method('findAllForUser') ->with($this->equalTo($this->user)) ->will($this->returnValue($result['feeds'])); $this->itemService->expects($this->once()) @@ -121,7 +121,7 @@ class FeedControllerTest extends TestCase 'newestItemId' => 5 ]; $this->feedService->expects($this->once()) - ->method('findAll') + ->method('findAllForUser') ->with($this->equalTo($this->user)) ->will($this->returnValue($result['feeds'])); $this->itemService->expects($this->once()) @@ -189,7 +189,7 @@ class FeedControllerTest extends TestCase $this->feedService->expects($this->once()) ->method('find') - ->with($this->equalTo($id), $this->equalTo($this->user)) + ->with($this->user, $id) ->will($this->throwException($ex)); $this->activeInitMocks($id, $type); @@ -209,7 +209,7 @@ class FeedControllerTest extends TestCase $this->folderService->expects($this->once()) ->method('find') - ->with($this->equalTo($id), $this->equalTo($this->user)) + ->with($this->user, $id) ->will($this->throwException($ex)); $this->activeInitMocks($id, $type); @@ -374,7 +374,7 @@ class FeedControllerTest extends TestCase $this->feedService->expects($this->once()) ->method('update') - ->with($this->equalTo(4), $this->equalTo($this->user)) + ->with($this->equalTo($this->user), $this->equalTo(4)) ->will($this->returnValue($feed)); $response = $this->controller->update(4); @@ -387,7 +387,7 @@ class FeedControllerTest extends TestCase { $this->feedService->expects($this->once()) ->method('update') - ->with($this->equalTo(4), $this->equalTo($this->user)) + ->with($this->equalTo($this->user), $this->equalTo(4)) ->will($this->throwException(new ServiceNotFoundException('NO!'))); $response = $this->controller->update(4); diff --git a/tests/Unit/Controller/FolderApiControllerTest.php b/tests/Unit/Controller/FolderApiControllerTest.php index 311212169..de62b887a 100644 --- a/tests/Unit/Controller/FolderApiControllerTest.php +++ b/tests/Unit/Controller/FolderApiControllerTest.php @@ -20,9 +20,9 @@ use OCA\News\Service\FolderService; use OCA\News\Service\ItemService; use \OCP\AppFramework\Http; -use \OCA\News\Service\ServiceNotFoundException; -use \OCA\News\Service\ServiceConflictException; -use \OCA\News\Service\ServiceValidationException; +use \OCA\News\Service\Exceptions\ServiceNotFoundException; +use \OCA\News\Service\Exceptions\ServiceConflictException; +use \OCA\News\Service\Exceptions\ServiceValidationException; use \OCA\News\Db\Folder; use OCP\IRequest; @@ -38,25 +38,22 @@ class FolderApiControllerTest extends TestCase private $folderService; private $itemService; private $folderAPI; - private $appName; - private $userSession; private $user; - private $request; private $msg; protected function setUp(): void { - $this->appName = 'news'; - $this->request = $this->getMockBuilder(IRequest::class) + $appName = 'news'; + $request = $this->getMockBuilder(IRequest::class) ->disableOriginalConstructor() ->getMock(); - $this->userSession = $this->getMockBuilder(IUserSession::class) + $userSession = $this->getMockBuilder(IUserSession::class) ->disableOriginalConstructor() ->getMock(); $this->user = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() ->getMock(); - $this->userSession->expects($this->any()) + $userSession->expects($this->any()) ->method('getUser') ->will($this->returnValue($this->user)); $this->user->expects($this->any()) @@ -69,9 +66,9 @@ class FolderApiControllerTest extends TestCase ->disableOriginalConstructor() ->getMock(); $this->folderAPI = new FolderApiController( - $this->appName, - $this->request, - $this->userSession, + $appName, + $request, + $userSession, $this->folderService, $this->itemService ); diff --git a/tests/Unit/Controller/FolderControllerTest.php b/tests/Unit/Controller/FolderControllerTest.php index abe1ebd7a..ea3454656 100644 --- a/tests/Unit/Controller/FolderControllerTest.php +++ b/tests/Unit/Controller/FolderControllerTest.php @@ -21,9 +21,9 @@ use \OCP\AppFramework\Http; use \OCA\News\Db\Folder; use \OCA\News\Db\Feed; -use \OCA\News\Service\ServiceNotFoundException; -use \OCA\News\Service\ServiceConflictException; -use \OCA\News\Service\ServiceValidationException; +use \OCA\News\Service\Exceptions\ServiceNotFoundException; +use \OCA\News\Service\Exceptions\ServiceConflictException; +use \OCA\News\Service\Exceptions\ServiceValidationException; use OCP\IRequest; use PHPUnit\Framework\TestCase; @@ -32,11 +32,9 @@ use PHPUnit\Framework\TestCase; class FolderControllerTest extends TestCase { - private $appName; private $folderService; private $itemService; private $feedService; - private $request; private $controller; private $msg; @@ -46,7 +44,7 @@ class FolderControllerTest extends TestCase */ public function setUp(): void { - $this->appName = 'news'; + $appName = 'news'; $this->user = 'jack'; $this->folderService = $this->getMockBuilder(FolderService::class) ->disableOriginalConstructor() @@ -57,11 +55,11 @@ class FolderControllerTest extends TestCase $this->itemService = $this->getMockBuilder(ItemService::class) ->disableOriginalConstructor() ->getMock(); - $this->request = $this->getMockBuilder(IRequest::class) + $request = $this->getMockBuilder(IRequest::class) ->disableOriginalConstructor() ->getMock(); $this->controller = new FolderController( - $this->appName, $this->request, + $appName, $request, $this->folderService, $this->feedService, $this->itemService, diff --git a/tests/Unit/Controller/ItemApiControllerTest.php b/tests/Unit/Controller/ItemApiControllerTest.php index 412b9dd51..1360ad872 100644 --- a/tests/Unit/Controller/ItemApiControllerTest.php +++ b/tests/Unit/Controller/ItemApiControllerTest.php @@ -19,7 +19,7 @@ use OCA\News\Controller\ItemApiController; use OCA\News\Service\ItemService; use \OCP\AppFramework\Http; -use \OCA\News\Service\ServiceNotFoundException; +use \OCA\News\Service\Exceptions\ServiceNotFoundException; use \OCA\News\Db\Item; use OCP\IRequest; use OCP\IUser; diff --git a/tests/Unit/Controller/ItemControllerTest.php b/tests/Unit/Controller/ItemControllerTest.php index 9dbcf15ce..a0780cecb 100644 --- a/tests/Unit/Controller/ItemControllerTest.php +++ b/tests/Unit/Controller/ItemControllerTest.php @@ -21,7 +21,7 @@ use \OCP\AppFramework\Http; use \OCA\News\Db\Item; use \OCA\News\Db\Feed; use \OCA\News\Db\FeedType; -use \OCA\News\Service\ServiceNotFoundException; +use \OCA\News\Service\Exceptions\ServiceNotFoundException; use OCP\IConfig; use OCP\IRequest; diff --git a/tests/Unit/Controller/UtilityApiControllerTest.php b/tests/Unit/Controller/UtilityApiControllerTest.php index 32a66b2e8..127618288 100644 --- a/tests/Unit/Controller/UtilityApiControllerTest.php +++ b/tests/Unit/Controller/UtilityApiControllerTest.php @@ -17,7 +17,7 @@ namespace OCA\News\Tests\Unit\Controller; use OCA\News\Controller\UtilityApiController; use OCA\News\Service\StatusService; -use OCA\News\Utility\Updater; +use OCA\News\Service\UpdaterService; use OCP\IConfig; use OCP\IRequest; use OCP\IUser; @@ -28,21 +28,52 @@ use PHPUnit\Framework\TestCase; class UtilityApiControllerTest extends TestCase { + /** + * @var \PHPUnit\Framework\MockObject\MockObject|IConfig + */ private $settings; + + /** + * @var \PHPUnit\Framework\MockObject\MockObject|IRequest + */ private $request; + + /** + * @var \PHPUnit\Framework\MockObject\MockObject|IUserSession + */ private $userSession; + + /** + * @var \PHPUnit\Framework\MockObject\MockObject|IUser + */ private $user; + + /** + * @var UtilityApiController + */ private $newsAPI; - private $updater; + + /** + * @var string + */ private $appName; + + /** + * @var \PHPUnit\Framework\MockObject\MockObject|StatusService + */ private $status; + /** + * @var \PHPUnit\Framework\MockObject\MockObject|UpdaterService + */ + private $updateService; + protected function setUp(): void { $this->appName = 'news'; $this->settings = $this->getMockBuilder(IConfig::class) - ->disableOriginalConstructor() - ->getMock(); + ->disableOriginalConstructor() + ->getMock(); $this->request = $this->getMockBuilder(IRequest::class) ->disableOriginalConstructor() ->getMock(); @@ -55,15 +86,19 @@ class UtilityApiControllerTest extends TestCase $this->userSession->expects($this->any()) ->method('getUser') ->will($this->returnValue($this->user)); - $this->updater = $this->getMockBuilder(Updater::class) - ->disableOriginalConstructor() - ->getMock(); $this->status = $this->getMockBuilder(StatusService::class) ->disableOriginalConstructor() ->getMock(); + $this->updateService = $this->getMockBuilder(UpdaterService::class) + ->disableOriginalConstructor() + ->getMock(); $this->newsAPI = new UtilityApiController( - $this->appName, $this->request, $this->userSession, - $this->updater, $this->settings, $this->status + $this->appName, + $this->request, + $this->userSession, + $this->updateService, + $this->settings, + $this->status ); } @@ -87,7 +122,7 @@ class UtilityApiControllerTest extends TestCase public function testBeforeUpdate() { - $this->updater->expects($this->once()) + $this->updateService->expects($this->once()) ->method('beforeUpdate'); $this->newsAPI->beforeUpdate(); } @@ -95,7 +130,7 @@ class UtilityApiControllerTest extends TestCase public function testAfterUpdate() { - $this->updater->expects($this->once()) + $this->updateService->expects($this->once()) ->method('afterUpdate'); $this->newsAPI->afterUpdate(); } @@ -103,7 +138,7 @@ class UtilityApiControllerTest extends TestCase public function testStatus() { - $in = 'hi'; + $in = ['hi']; $this->status->expects($this->once()) ->method('getStatus') ->will($this->returnValue($in)); diff --git a/tests/Unit/Db/FeedTest.php b/tests/Unit/Db/FeedTest.php index 47595633d..1812b7164 100644 --- a/tests/Unit/Db/FeedTest.php +++ b/tests/Unit/Db/FeedTest.php @@ -51,18 +51,19 @@ class FeedTest extends TestCase $this->assertEquals( [ - 'id' => 3, - 'url' => 'http://google.com/some/weird/path', - 'title' => 'title', - 'faviconLink' => 'favicon', - 'added' => 123, - 'folderId' => 1, - 'unreadCount' => 321, - 'ordering' => 2, - 'pinned' => true, - 'link' => 'https://www.google.com/some/weird/path', - 'updateErrorCount' => 2, - 'lastUpdateError' => 'hi' + 'id' => 3, + 'url' => 'http://google.com/some/weird/path', + 'title' => 'title', + 'faviconLink' => 'favicon', + 'added' => 123, + 'folderId' => 1, + 'unreadCount' => 321, + 'ordering' => 2, + 'pinned' => true, + 'link' => 'https://www.google.com/some/weird/path', + 'updateErrorCount' => 2, + 'lastUpdateError' => 'hi', + 'items' => [], ], $feed->toAPI() ); } diff --git a/tests/Unit/Db/FolderMapperTest.php b/tests/Unit/Db/FolderMapperTest.php index f8544da65..deb235ff7 100644 --- a/tests/Unit/Db/FolderMapperTest.php +++ b/tests/Unit/Db/FolderMapperTest.php @@ -62,7 +62,7 @@ class FolderMapperTest extends MapperTestUtility $this->setMapperResult($sql, [$id, $userId], $rows); - $result = $this->folderMapper->find($id, $userId); + $result = $this->folderMapper->find($userId, $id); $this->assertEquals($this->folders[0], $result); } @@ -79,7 +79,7 @@ class FolderMapperTest extends MapperTestUtility $this->setMapperResult($sql, [$id, $userId]); $this->expectException(DoesNotExistException::class); - $this->folderMapper->find($id, $userId); + $this->folderMapper->find($userId, $id); } @@ -95,7 +95,7 @@ class FolderMapperTest extends MapperTestUtility $this->setMapperResult($sql, [$id, $userId], $rows); $this->expectException(MultipleObjectsReturnedException::class); - $this->folderMapper->find($id, $userId); + $this->folderMapper->find($userId, $id); } diff --git a/tests/Unit/Db/FolderTest.php b/tests/Unit/Db/FolderTest.php index a3445ea2e..7fdb1ae07 100644 --- a/tests/Unit/Db/FolderTest.php +++ b/tests/Unit/Db/FolderTest.php @@ -20,7 +20,7 @@ class FolderTest extends TestCase { - public function testToAPI() + public function testToAPI() { $folder = new Folder(); $folder->setId(3); @@ -28,14 +28,15 @@ class FolderTest extends TestCase $this->assertEquals( [ - 'id' => 3, - 'name' => 'name' + 'id' => 3, + 'name' => 'name', + 'feeds' => [], ], $folder->toAPI() ); } - public function testSerialize() + public function testSerialize() { $folder = new Folder(); $folder->setId(3); diff --git a/tests/Unit/Db/MapperFactoryTest.php b/tests/Unit/Db/MapperFactoryTest.php index 97680e20b..1c4e2f4b6 100644 --- a/tests/Unit/Db/MapperFactoryTest.php +++ b/tests/Unit/Db/MapperFactoryTest.php @@ -26,8 +26,10 @@ use OCA\News\Db\Mysql\ItemMapper as MysqlMapper; class MapperFactoryTest extends TestCase { + /** + * @var \PHPUnit\Framework\MockObject\MockObject|IDBConnection + */ private $db; - private $settings; public function setUp(): void { diff --git a/tests/Unit/Fetcher/FeedFetcherTest.php b/tests/Unit/Fetcher/FeedFetcherTest.php index 6360973de..38a7c2f61 100644 --- a/tests/Unit/Fetcher/FeedFetcherTest.php +++ b/tests/Unit/Fetcher/FeedFetcherTest.php @@ -14,18 +14,22 @@ namespace OCA\News\Tests\Unit\Fetcher; use DateTime; +use Favicon\Favicon; +use FeedIo\Adapter\ResponseInterface; use FeedIo\Feed\Item\Author; use FeedIo\Feed\Item\MediaInterface; use FeedIo\Feed\ItemInterface; use FeedIo\FeedInterface; +use FeedIo\FeedIo; +use FeedIo\Reader\Result; use OC\L10N\L10N; use \OCA\News\Db\Feed; use \OCA\News\Db\Item; use OCA\News\Scraper\Scraper; use OCA\News\Fetcher\FeedFetcher; -use OCA\News\Utility\PsrLogger; -use OCP\ILogger; +use OCA\News\Utility\Time; +use OCP\IL10N; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; @@ -46,26 +50,26 @@ class FeedFetcherTest extends TestCase /** * Feed reader * - * @var \FeedIo\FeedIo + * @var FeedIo */ private $reader; /** * Feed reader result * - * @var \FeedIo\Reader\Result + * @var Result */ private $result; /** * Feed reader result object * - * @var \FeedIo\Adapter\ResponseInterface + * @var ResponseInterface */ private $response; /** - * @var \Favicon\Favicon + * @var Favicon */ private $favicon; @@ -132,32 +136,32 @@ class FeedFetcherTest extends TestCase protected function setUp(): void { - $this->l10n = $this->getMockBuilder(\OCP\IL10N::class) + $this->l10n = $this->getMockBuilder(IL10N::class) ->disableOriginalConstructor() ->getMock(); - $this->reader = $this->getMockBuilder(\FeedIo\FeedIo::class) + $this->reader = $this->getMockBuilder(FeedIo::class) ->disableOriginalConstructor() ->getMock(); - $this->favicon = $this->getMockBuilder(\Favicon\Favicon::class) + $this->favicon = $this->getMockBuilder(Favicon::class) ->disableOriginalConstructor() ->getMock(); - $this->result = $this->getMockBuilder(\FeedIo\Reader\Result::class) + $this->result = $this->getMockBuilder(Result::class) ->disableOriginalConstructor() ->getMock(); - $this->response = $this->getMockBuilder(\FeedIo\Adapter\ResponseInterface::class) + $this->response = $this->getMockBuilder(ResponseInterface::class) ->disableOriginalConstructor() ->getMock(); - $this->item_mock = $this->getMockBuilder(\FeedIo\Feed\ItemInterface::class) + $this->item_mock = $this->getMockBuilder(ItemInterface::class) ->disableOriginalConstructor() ->getMock(); - $this->feed_mock = $this->getMockBuilder(\FeedIo\FeedInterface::class) + $this->feed_mock = $this->getMockBuilder(FeedInterface::class) ->disableOriginalConstructor() ->getMock(); $this->time = 2323; - $timeFactory = $this->getMockBuilder(\OCA\News\Utility\Time::class) + $timeFactory = $this->getMockBuilder(Time::class) ->disableOriginalConstructor() ->getMock(); $timeFactory->expects($this->any()) diff --git a/tests/Unit/Service/FeedServiceTest.php b/tests/Unit/Service/FeedServiceTest.php index 84c2121ed..f93a1e286 100644 --- a/tests/Unit/Service/FeedServiceTest.php +++ b/tests/Unit/Service/FeedServiceTest.php @@ -20,7 +20,7 @@ use OC\L10N\L10N; use OCA\News\Db\FeedMapper; use OCA\News\Db\ItemMapper; use OCA\News\Service\FeedService; -use OCA\News\Service\ServiceNotFoundException; +use OCA\News\Service\Exceptions\ServiceNotFoundException; use OCA\News\Utility\Time; use OCP\AppFramework\Db\DoesNotExistException; @@ -30,9 +30,9 @@ use OCA\News\Fetcher\Fetcher; use OCA\News\Fetcher\FetcherException; use OCP\IConfig; use OCP\IL10N; -use OCP\ILogger; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class FeedServiceTest extends TestCase @@ -82,16 +82,15 @@ class FeedServiceTest extends TestCase private $l10n; /** - * @var \PHPUnit\Framework\MockObject\MockObject|ILogger + * @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface */ private $logger; protected function setUp(): void { - $this->logger = $this->getMockBuilder(ILogger::class) + $this->logger = $this->getMockBuilder(LoggerInterface::class) ->disableOriginalConstructor() ->getMock(); - $loggerParams = ['hi']; $this->time = 222; $this->autoPurgeMinimumInterval = 10; $timeFactory = $this->getMockBuilder(Time::class) @@ -130,7 +129,7 @@ class FeedServiceTest extends TestCase $this->feedService = new FeedService( $this->feedMapper, $this->fetcher, $this->itemMapper, $this->logger, $this->l10n, - $timeFactory, $config, $this->purifier, $loggerParams + $timeFactory, $config, $this->purifier ); $this->user = 'jack'; } @@ -140,12 +139,13 @@ class FeedServiceTest extends TestCase */ public function testFindAll() { + $this->response = []; $this->feedMapper->expects($this->once()) ->method('findAllFromUser') ->with($this->user) ->will($this->returnValue([])); - $result = $this->feedService->findAll($this->user); + $result = $this->feedService->findAllForUser($this->user); $this->assertEquals([], $result); } @@ -199,10 +199,14 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->once()) ->method('insert') ->with($this->equalTo($createdFeed)) - ->will($this->returnCallback(function() use ($createdFeed) { - $createdFeed->setId(4); - return $createdFeed; - })); + ->will( + $this->returnCallback( + function () use ($createdFeed) { + $createdFeed->setId(4); + return $createdFeed; + } + ) + ); $this->itemMapper->expects($this->at(0)) ->method('findByGuidHash') ->with( @@ -281,10 +285,14 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->once()) ->method('insert') ->with($this->equalTo($createdFeed)) - ->will($this->returnCallback(function() use ($createdFeed) { - $createdFeed->setId(5); - return $createdFeed; - })); + ->will( + $this->returnCallback( + function () use ($createdFeed) { + $createdFeed->setId(5); + return $createdFeed; + } + ) + ); $this->itemMapper->expects($this->at(0)) ->method('findByGuidHash') ->with( @@ -358,10 +366,7 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->at(0)) ->method('find') - ->with( - $this->equalTo($feed->getId()), - $this->equalTo($this->user) - ) + ->with($this->user, $feed->getId()) ->will($this->returnValue($feed)); $this->fetcher->expects($this->once()) ->method('fetch') @@ -394,11 +399,11 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->at(2)) ->method('find') - ->with($feed->getId(), $this->user) + ->with($this->user, $feed->getId()) ->will($this->returnValue($feed)); - $return = $this->feedService->update($feed->getId(), $this->user); + $return = $this->feedService->update($this->user, $feed->getId()); $this->assertEquals($return, $feed); } @@ -425,10 +430,7 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->at(0)) ->method('find') - ->with( - $this->equalTo($feed->getId()), - $this->equalTo($this->user) - ) + ->with($this->user, $feed->getId()) ->will($this->returnValue($feed)); $this->fetcher->expects($this->once()) ->method('fetch') @@ -461,11 +463,11 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->at(2)) ->method('find') - ->with($feed->getId(), $this->user) + ->with($this->user, $feed->getId()) ->will($this->returnValue($feed)); - $return = $this->feedService->update($feed->getId(), $this->user, true); + $return = $this->feedService->update($this->user, $feed->getId(), true); $this->assertEquals($return, $feed); } @@ -543,7 +545,7 @@ class FeedServiceTest extends TestCase ->will($this->returnValue($feed)); - $return = $this->feedService->update($feed->getId(), $this->user); + $return = $this->feedService->update($this->user, $feed->getId()); $this->assertEquals($return, $feed); } @@ -583,7 +585,7 @@ class FeedServiceTest extends TestCase ->method('find') ->will($this->returnValue($feed)); - $return = $this->feedService->update($feed->getId(), $this->user); + $return = $this->feedService->update($this->user, $feed->getId()); $this->assertEquals($return, $feed); @@ -622,7 +624,7 @@ class FeedServiceTest extends TestCase ->method('findByGuidHash') ->will($this->returnValue($item)); - $this->feedService->update($feed->getId(), $this->user); + $this->feedService->update($this->user, $feed->getId()); } public function testUpdateFails() @@ -643,10 +645,7 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->at(0)) ->method('find') - ->with( - $this->equalTo($feed->getId()), - $this->equalTo($this->user) - ) + ->with($this->user, $feed->getId()) ->will($this->returnValue($feed)); $this->fetcher->expects($this->once()) ->method('fetch') @@ -661,10 +660,10 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->at(2)) ->method('find') - ->with($feed->getId(), $this->user) + ->with($this->user, $feed->getId()) ->will($this->returnValue($exptectedFeed)); - $return = $this->feedService->update($feed->getId(), $this->user); + $return = $this->feedService->update($this->user, $feed->getId()); $this->assertEquals($return, $exptectedFeed); } @@ -679,14 +678,11 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->at(0)) ->method('find') - ->with( - $this->equalTo($feed->getId()), - $this->equalTo($this->user) - ) + ->with($this->user, $feed->getId()) ->will($this->throwException($ex)); $this->expectException(ServiceNotFoundException::class); - $this->feedService->update($feed->getId(), $this->user); + $this->feedService->update($this->user, $feed->getId()); } @@ -702,10 +698,7 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->at(0)) ->method('find') - ->with( - $this->equalTo($feed->getId()), - $this->equalTo($this->user) - ) + ->with($this->user, $feed->getId()) ->will($this->returnValue($feed)); $this->fetcher->expects($this->once()) @@ -719,7 +712,7 @@ class FeedServiceTest extends TestCase ->will($this->throwException($ex)); $this->expectException(DoesNotExistException::class); - $this->feedService->update($feed->getId(), $this->user); + $this->feedService->update($this->user, $feed->getId()); } @@ -744,10 +737,7 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->at(0)) ->method('find') - ->with( - $this->equalTo($feed->getId()), - $this->equalTo($this->user) - ) + ->with($this->user, $feed->getId()) ->will($this->returnValue($feed)); $this->feedMapper->expects($this->at(1)) ->method('update') @@ -766,14 +756,11 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->at(2)) ->method('find') - ->with( - $this->equalTo($feed->getId()), - $this->equalTo($this->user) - ) + ->with($this->user, $feed->getId()) ->will($this->throwException($ex)); $this->expectException(ServiceNotFoundException::class); - $this->feedService->update($feed->getId(), $this->user); + $this->feedService->update($this->user, $feed->getId()); } @@ -787,15 +774,12 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->once()) ->method('find') - ->with( - $this->equalTo($feedId), - $this->equalTo($this->user) - ) + ->with($this->user, $feed->getId()) ->will($this->returnValue($feed)); $this->fetcher->expects($this->never()) ->method('fetch'); - $this->feedService->update($feedId, $this->user); + $this->feedService->update($this->user, $feedId); } @@ -809,16 +793,13 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->once()) ->method('find') - ->with( - $this->equalTo($feedId), - $this->equalTo($this->user) - ) + ->with($this->user, $feed->getId()) ->will($this->returnValue($feed)); $this->fetcher->expects($this->once()) ->method('fetch') ->will($this->returnValue([null, null])); - $return = $this->feedService->update($feedId, $this->user); + $return = $this->feedService->update($this->user, $feedId); $this->assertEquals($feed, $return); } @@ -833,14 +814,16 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->once()) ->method('find') - ->with($this->equalTo($feedId), $this->equalTo($this->user)) + ->with($this->user, $feedId) ->will($this->returnValue($feed)); $this->feedMapper->expects($this->once()) ->method('update') ->with($this->equalTo($feed)); - $this->feedService->patch($feedId, $this->user, ['folderId' => $folderId]); + $this->feedService->patch( + $feedId, $this->user, ['folderId' => $folderId] + ); $this->assertEquals($folderId, $feed->getFolderId()); } @@ -856,14 +839,16 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->once()) ->method('find') - ->with($this->equalTo($feedId), $this->equalTo($this->user)) + ->with($this->equalTo($this->user), $this->equalTo($feedId)) ->will($this->returnValue($feed)); $this->feedMapper->expects($this->once()) ->method('update') ->with($this->equalTo($feed)); - $this->feedService->patch($feedId, $this->user, ['title' => $feedTitle]); + $this->feedService->patch( + $feedId, $this->user, ['title' => $feedTitle] + ); $this->assertEquals($feedTitle, $feed->getTitle()); } @@ -982,10 +967,14 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->once()) ->method('insert') ->with($this->equalTo($insertFeed)) - ->will($this->returnCallback(function() use ($insertFeed) { - $insertFeed->setId(3); - return $insertFeed; - })); + ->will( + $this->returnCallback( + function () use ($insertFeed) { + $insertFeed->setId(3); + return $insertFeed; + } + ) + ); $this->itemMapper->expects($this->at(0)) @@ -1025,7 +1014,7 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->once()) ->method('find') - ->with($this->equalTo($id), $this->equalTo($this->user)) + ->with($this->equalTo($this->user), $this->equalTo($id)) ->will($this->returnValue($feed)); $this->feedMapper->expects($this->once()) ->method('update') @@ -1044,7 +1033,7 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->once()) ->method('find') - ->with($this->equalTo($id), $this->equalTo($this->user)) + ->with($this->equalTo($this->user), $this->equalTo($id)) ->will($this->returnValue($feed)); $this->feedMapper->expects($this->once()) ->method('update') @@ -1103,7 +1092,7 @@ class FeedServiceTest extends TestCase public function testfindAllFromAllUsers() { - $expected = 'hi'; + $expected = ['hi']; $this->feedMapper->expects($this->once()) ->method('findAll') ->will($this->returnValue($expected)); @@ -1128,8 +1117,8 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->once()) ->method('find') ->with( - $this->equalTo($feed->getId()), - $this->equalTo($this->user) + $this->equalTo($this->user), + $this->equalTo($feed->getId()) ) ->will($this->returnValue($feed)); @@ -1146,18 +1135,18 @@ class FeedServiceTest extends TestCase { $feed = Feed::fromRow( [ - 'id' => 3, - 'http_etag' => 'a', - 'http_last_modified' => 1, - 'full_text_enabled' => false + 'id' => 3, + 'http_etag' => 'a', + 'http_last_modified' => 1, + 'full_text_enabled' => false ] ); $feed2 = Feed::fromRow(['id' => 3]); $this->feedMapper->expects($this->at(0)) ->method('find') ->with( - $this->equalTo($feed->getId()), - $this->equalTo($this->user) + $this->equalTo($this->user), + $this->equalTo($feed->getId()) ) ->will($this->returnValue($feed)); @@ -1171,8 +1160,8 @@ class FeedServiceTest extends TestCase $this->feedMapper->expects($this->at(2)) ->method('find') ->with( - $this->equalTo($feed->getId()), - $this->equalTo($this->user) + $this->equalTo($this->user), + $this->equalTo($feed->getId()) ) ->will($this->throwException(new DoesNotExistException(''))); @@ -1183,7 +1172,7 @@ class FeedServiceTest extends TestCase public function testPatchDoesNotExist() { - $this->expectException('OCA\News\Service\ServiceNotFoundException'); + $this->expectException('OCA\News\Service\Exceptions\ServiceNotFoundException'); $feed = Feed::fromRow(['id' => 3]); $this->feedMapper->expects($this->once()) ->method('find') @@ -1198,10 +1187,7 @@ class FeedServiceTest extends TestCase $feed = Feed::fromRow(['id' => 3, 'pinned' => false]); $this->feedMapper->expects($this->once()) ->method('find') - ->with( - $this->equalTo($feed->getId()), - $this->equalTo($this->user) - ) + ->with($this->user, $feed->getId()) ->will($this->returnValue($feed)); $feed->setPinned(true); diff --git a/tests/Unit/Service/FolderServiceTest.php b/tests/Unit/Service/FolderServiceTest.php index 4086f55e1..deca27f08 100644 --- a/tests/Unit/Service/FolderServiceTest.php +++ b/tests/Unit/Service/FolderServiceTest.php @@ -17,13 +17,14 @@ use OC\L10N\L10N; use \OCA\News\Db\Folder; use OCA\News\Db\FolderMapper; use OCA\News\Service\FolderService; -use OCA\News\Service\ServiceConflictException; -use OCA\News\Service\ServiceValidationException; +use OCA\News\Service\Exceptions\ServiceConflictException; +use OCA\News\Service\Exceptions\ServiceValidationException; use OCA\News\Utility\Time; use OCP\IConfig; use OCP\IL10N; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class FolderServiceTest extends TestCase @@ -49,6 +50,11 @@ class FolderServiceTest extends TestCase */ private $user; + /** + * @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface + */ + private $logger; + /** * @var int */ @@ -78,12 +84,15 @@ class FolderServiceTest extends TestCase $config = $this->getMockBuilder(IConfig::class) ->disableOriginalConstructor() ->getMock(); + $this->logger = $this->getMockBuilder(LoggerInterface::class) + ->disableOriginalConstructor() + ->getMock(); $config->expects($this->any()) ->method('getAppValue') ->with('news', 'autoPurgeMinimumInterval') ->will($this->returnValue($this->autoPurgeMinimumInterval)); $this->folderService = new FolderService( - $this->folderMapper, $this->l10n, $timeFactory, $config + $this->folderMapper, $this->l10n, $timeFactory, $config, $this->logger ); $this->user = 'hi'; } @@ -92,13 +101,13 @@ class FolderServiceTest extends TestCase public function testFindAll() { $userId = 'jack'; - $return = 'hi'; + $return = []; $this->folderMapper->expects($this->once()) ->method('findAllFromUser') ->with($this->equalTo($userId)) ->will($this->returnValue($return)); - $result = $this->folderService->findAll($userId); + $result = $this->folderService->findAllForUser($userId); $this->assertEquals($return, $result); } @@ -146,8 +155,7 @@ class FolderServiceTest extends TestCase public function testCreateThrowsExWhenFolderNameEmpty() { - $this->expectException('OCA\News\Service\ServiceValidationException'); - + $this->expectException('\OCA\News\Service\Exceptions\ServiceValidationException'); $folderName = ''; $this->folderMapper->expects($this->once()) @@ -165,7 +173,7 @@ class FolderServiceTest extends TestCase $this->folderMapper->expects($this->once()) ->method('find') - ->with($this->equalTo(3)) + ->with('', 3) ->will($this->returnValue($folder)); $this->folderMapper->expects($this->once()) @@ -186,7 +194,7 @@ class FolderServiceTest extends TestCase $this->folderMapper->expects($this->once()) ->method('find') - ->with($this->equalTo(3)) + ->with('', 3) ->will($this->returnValue($folder)); $this->folderMapper->expects($this->once()) @@ -244,7 +252,7 @@ class FolderServiceTest extends TestCase $this->folderMapper->expects($this->once()) ->method('find') - ->with($this->equalTo($id), $this->equalTo($this->user)) + ->with($this->equalTo($this->user), $this->equalTo($id)) ->will($this->returnValue($folder)); $this->folderMapper->expects($this->once()) ->method('update') @@ -263,7 +271,7 @@ class FolderServiceTest extends TestCase $this->folderMapper->expects($this->once()) ->method('find') - ->with($this->equalTo($id), $this->equalTo($this->user)) + ->with($this->equalTo($this->user), $this->equalTo($id)) ->will($this->returnValue($folder)); $this->folderMapper->expects($this->once()) ->method('update') diff --git a/tests/Unit/Service/ItemServiceTest.php b/tests/Unit/Service/ItemServiceTest.php index e56add152..c98ac2c33 100644 --- a/tests/Unit/Service/ItemServiceTest.php +++ b/tests/Unit/Service/ItemServiceTest.php @@ -13,9 +13,11 @@ namespace OCA\News\Tests\Unit\Service; +use OC\Log; use OCA\News\Db\ItemMapper; use OCA\News\Service\ItemService; -use OCA\News\Service\ServiceNotFoundException; +use OCA\News\Service\Exceptions\ServiceNotFoundException; +use OCA\News\Utility\PsrLogger; use OCA\News\Utility\Time; use \OCP\AppFramework\Db\DoesNotExistException; @@ -24,6 +26,7 @@ use \OCA\News\Db\FeedType; use OCP\IConfig; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class ItemServiceTest extends TestCase @@ -38,15 +41,31 @@ class ItemServiceTest extends TestCase */ private $itemService; + /** + * @var \PHPUnit\Framework\MockObject\MockObject|IConfig + */ + private $config; + + /** + * @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface + */ + private $logger; + + /** + * @var \PHPUnit\Framework\MockObject\MockObject|Time + */ + private $timeFactory; + + /** + * @var int + */ + private $newestItemId; + /** * @var int */ private $time; - /** - * @var \PHPUnit\Framework\MockObject\MockObject|IConfig - */ - private $config; protected function setUp(): void { @@ -66,7 +85,24 @@ class ItemServiceTest extends TestCase $this->config = $this->getMockBuilder(IConfig::class) ->disableOriginalConstructor() ->getMock(); - $this->itemService = new ItemService($this->mapper, $this->timeFactory, $this->config); + + $this->logger = $this->getMockBuilder(LoggerInterface::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->itemService = new ItemService( + $this->mapper, + $this->timeFactory, + $this->config, + $this->logger + ); + $this->user = 'jack'; + $this->id = 3; + $this->updatedSince = 20333; + $this->showAll = true; + $this->offset = 5; + $this->limit = 20; + $this->newestItemId = 4; } @@ -143,7 +179,7 @@ class ItemServiceTest extends TestCase ) ->will($this->returnValue(['val'])); - $result = $this->itemService->findAll( + $result = $this->itemService->findAllItems( 3, $type, 20, 5, true, false, 'jack' ); @@ -167,7 +203,7 @@ class ItemServiceTest extends TestCase ) ->will($this->returnValue(['val'])); - $result = $this->itemService->findAll( + $result = $this->itemService->findAllItems( 3, $type, 20, 5, true, true, 'jack' ); @@ -179,7 +215,7 @@ class ItemServiceTest extends TestCase { $type = FeedType::STARRED; $this->mapper->expects($this->once()) - ->method('findAll') + ->method('findAllItems') ->with( $this->equalTo(20), $this->equalTo(5), @@ -191,7 +227,7 @@ class ItemServiceTest extends TestCase ) ->will($this->returnValue(['val'])); - $result = $this->itemService->findAll( + $result = $this->itemService->findAllItems( 3, $type, 20, 5, true, true, 'jack' ); @@ -203,8 +239,9 @@ class ItemServiceTest extends TestCase { $type = FeedType::STARRED; $search = ['test']; + $this->mapper->expects($this->once()) - ->method('findAll') + ->method('findAllItems') ->with( $this->equalTo(20), $this->equalTo(5), @@ -216,7 +253,7 @@ class ItemServiceTest extends TestCase ) ->will($this->returnValue(['val'])); - $result = $this->itemService->findAll( + $result = $this->itemService->findAllItems( 3, $type, 20, 5, true, true, 'jack', $search ); diff --git a/tests/Unit/Service/ServiceTest.php b/tests/Unit/Service/ServiceTest.php index 8f601be46..e243efb3f 100644 --- a/tests/Unit/Service/ServiceTest.php +++ b/tests/Unit/Service/ServiceTest.php @@ -13,21 +13,33 @@ namespace OCA\News\Tests\Unit\Service; +use OCA\News\Db\Feed; use OCA\News\Db\ItemMapper; +use OCA\News\Service\Exceptions\ServiceNotFoundException; use OCA\News\Service\Service; -use OCA\News\Service\ServiceNotFoundException; use \OCP\AppFramework\Db\DoesNotExistException; use \OCP\AppFramework\Db\MultipleObjectsReturnedException; use \OCA\News\Db\Folder; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; -class TestService extends Service +class TestLegacyService extends Service { - public function __construct($mapper) + public function __construct($mapper, $logger) { - parent::__construct($mapper); + parent::__construct($mapper, $logger); + } + + public function findAllForUser(string $userId): array + { + // TODO: Implement findAllForUser() method. + } + + public function findAll(): array + { + // TODO: Implement findAll() method. } } @@ -35,6 +47,7 @@ class ServiceTest extends TestCase { protected $mapper; + protected $logger; protected $newsService; protected function setUp(): void @@ -42,7 +55,10 @@ class ServiceTest extends TestCase $this->mapper = $this->getMockBuilder(ItemMapper::class) ->disableOriginalConstructor() ->getMock(); - $this->newsService = new TestService($this->mapper); + $this->logger = $this->getMockBuilder(LoggerInterface::class) + ->disableOriginalConstructor() + ->getMock(); + $this->newsService = new TestLegacyService($this->mapper, $this->logger); } @@ -58,10 +74,10 @@ class ServiceTest extends TestCase ->with($this->equalTo($folder)); $this->mapper->expects($this->once()) ->method('find') - ->with($this->equalTo($id), $this->equalTo($user)) + ->with($this->equalTo($user), $this->equalTo($id)) ->will($this->returnValue($folder)); - $this->newsService->delete($id, $user); + $this->newsService->delete($user, $id); } @@ -72,9 +88,10 @@ class ServiceTest extends TestCase $this->mapper->expects($this->once()) ->method('find') - ->with($this->equalTo($id), $this->equalTo($user)); + ->with($this->equalTo($user), $this->equalTo($id)) + ->will($this->returnValue(new Feed())); - $this->newsService->find($id, $user); + $this->newsService->find($user, $id); } @@ -87,7 +104,7 @@ class ServiceTest extends TestCase ->will($this->throwException($ex)); $this->expectException(ServiceNotFoundException::class); - $this->newsService->find(1, ''); + $this->newsService->find('', 1); } @@ -100,7 +117,7 @@ class ServiceTest extends TestCase ->will($this->throwException($ex)); $this->expectException(ServiceNotFoundException::class); - $this->newsService->find(1, ''); + $this->newsService->find('', 1); } } diff --git a/tests/Unit/Utility/UpdaterTest.php b/tests/Unit/Service/UpdaterTest.php similarity index 66% rename from tests/Unit/Utility/UpdaterTest.php rename to tests/Unit/Service/UpdaterTest.php index 3cd81de02..b4be0a7cc 100644 --- a/tests/Unit/Utility/UpdaterTest.php +++ b/tests/Unit/Service/UpdaterTest.php @@ -11,35 +11,49 @@ * @copyright 2012-2014 Bernhard Posselt */ -namespace OCA\News\Tests\Unit\Utility; +namespace OCA\News\Tests\Unit\Service; - -use OCA\News\Service\FeedService; -use OCA\News\Service\FolderService; -use OCA\News\Service\ItemService; -use OCA\News\Utility\Updater; +use OCA\News\Service\FeedServiceV2; +use OCA\News\Service\FolderServiceV2; +use OCA\News\Service\ItemServiceV2; +use OCA\News\Service\UpdaterService; use PHPUnit\Framework\TestCase; class UpdaterTest extends TestCase { + /** + * @var \PHPUnit\Framework\MockObject\MockObject|FolderServiceV2 + */ private $folderService; + + /** + * @var \PHPUnit\Framework\MockObject\MockObject|FeedServiceV2 + */ private $feedService; + + /** + * @var \PHPUnit\Framework\MockObject\MockObject|ItemServiceV2 + */ private $itemService; + + /** + * @var UpdaterService + */ private $updater; protected function setUp(): void { - $this->folderService = $this->getMockBuilder(FolderService::class) + $this->folderService = $this->getMockBuilder(FolderServiceV2::class) ->disableOriginalConstructor() ->getMock(); - $this->feedService = $this->getMockBuilder(FeedService::class) + $this->feedService = $this->getMockBuilder(FeedServiceV2::class) ->disableOriginalConstructor() ->getMock(); - $this->itemService = $this->getMockBuilder(ItemService::class) + $this->itemService = $this->getMockBuilder(ItemServiceV2::class) ->disableOriginalConstructor() ->getMock(); - $this->updater = new Updater( + $this->updater = new UpdaterService( $this->folderService, $this->feedService, $this->itemService @@ -59,14 +73,14 @@ class UpdaterTest extends TestCase public function testAfterUpdate() { $this->itemService->expects($this->once()) - ->method('autoPurgeOld'); + ->method('purgeOverThreshold'); $this->updater->afterUpdate(); } public function testUpdate() { $this->feedService->expects($this->once()) - ->method('updateAll'); + ->method('fetchAll'); $this->updater->update(); } } \ No newline at end of file