From 10e8c28feaf6d858948285a291231f651ef74728 Mon Sep 17 00:00:00 2001 From: Sean Molenaar Date: Sun, 4 Oct 2020 20:45:33 +0200 Subject: Add migration with foreign keys Closes #829 Signed-off-by: Sean Molenaar --- .github/workflows/integration-tests.yml | 2 +- AUTHORS.md | 2 +- CHANGELOG.md | 1 + lib/Command/Config/FeedAdd.php | 24 +++- lib/Command/Config/FolderAdd.php | 10 +- lib/Command/Config/FolderDelete.php | 4 +- lib/Controller/FeedApiController.php | 21 ++- lib/Controller/FeedController.php | 32 +++-- lib/Controller/FolderApiController.php | 27 ++-- lib/Controller/FolderController.php | 39 ++++-- lib/Controller/UserApiController.php | 74 ----------- lib/Db/Feed.php | 12 +- lib/Db/FeedMapper.php | 4 +- lib/Db/FeedMapperV2.php | 18 ++- lib/Db/Folder.php | 2 +- lib/Db/FolderMapperV2.php | 1 + lib/Db/ItemMapper.php | 22 +-- lib/Migration/Version150005Date20201009192341.php | 98 ++++++++++++++ lib/Service/FeedService.php | 26 ++-- lib/Service/FeedServiceV2.php | 8 +- lib/Service/FolderService.php | 18 ++- lib/Service/FolderServiceV2.php | 2 +- lib/Service/ItemService.php | 44 +++--- lib/Utility/OPMLExporter.php | 2 +- templates/index.php | 2 +- tests/Integration/Fixtures/FeedFixture.php | 2 +- tests/Integration/Fixtures/FolderFixture.php | 4 +- tests/Integration/IntegrationTest.php | 2 +- tests/Unit/Command/FeedAddTest.php | 4 +- tests/Unit/Command/FolderDeleteTest.php | 2 +- tests/Unit/Controller/UserApiControllerTest.php | 155 ---------------------- tests/Unit/Service/FeedServiceTest.php | 2 +- tests/Unit/Utility/OPMLExporterTest.php | 4 +- 33 files changed, 313 insertions(+), 357 deletions(-) delete mode 100644 lib/Controller/UserApiController.php create mode 100644 lib/Migration/Version150005Date20201009192341.php delete mode 100644 tests/Unit/Controller/UserApiControllerTest.php diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index edab7e2fb..5843149e9 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -140,4 +140,4 @@ jobs: run: make integration-test - name: Feed tests working-directory: ../server/apps/news - run: make feed-test \ No newline at end of file + run: make feed-test diff --git a/AUTHORS.md b/AUTHORS.md index 65abd3e7d..7cb56a2d8 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -5,8 +5,8 @@ * [Robin Appelman](mailto:icewind@owncloud.com) * [Gregor Tätzner](mailto:gregor@freenet.de) * [Morris Jobke](mailto:hey@morrisjobke.de) -* [Sean Molenaar](mailto:SMillerDev@users.noreply.github.com) * [Sean Molenaar](mailto:sean@seanmolenaar.eu) +* [Sean Molenaar](mailto:SMillerDev@users.noreply.github.com) * [Jan-Christoph Borchardt](mailto:hey@jancborchardt.net) * [Daniel Schaal](mailto:daniel@schaal.email) * [Davide Saurino](mailto:davide.saurino@alcacoop.it) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d0e607d3..75f2fdf9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. - Stop overloading DB ids - Unittest commands and utilities - Upload codecoverage to codecov.io +- Use foreign keys in db ## 15.0.6 diff --git a/lib/Command/Config/FeedAdd.php b/lib/Command/Config/FeedAdd.php index 3c4820437..86121ad06 100644 --- a/lib/Command/Config/FeedAdd.php +++ b/lib/Command/Config/FeedAdd.php @@ -2,6 +2,8 @@ declare(strict_types=1); namespace OCA\News\Command\Config; +use OCA\News\Service\Exceptions\ServiceConflictException; +use OCA\News\Service\Exceptions\ServiceNotFoundException; use OCA\News\Service\FeedServiceV2; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -56,13 +58,31 @@ class FeedAdd extends Command { $user = $input->getArgument('user-id'); $url = $input->getArgument('feed'); - $folder = (int) $input->getOption('folder') ?? 0; + $folder = $input->getOption('folder'); $title = $input->getOption('title'); $full_text = (bool) $input->getOption('full-text'); $username = $input->getOption('username'); $password = $input->getOption('password'); - $feed = $this->feedService->create($user, $url, $folder, $full_text, $title, $username, $password); + if ($folder !== null) { + $folder = intval($folder); + } + + try { + $feed = $this->feedService->create( + $user, + $url, + $folder, + $full_text, + $title, + $username, + $password + ); + } catch (ServiceNotFoundException|ServiceConflictException $e) { + $output->write($e->getMessage()); + return 1; + } + $this->feedService->fetch($feed); $output->writeln(json_encode($feed->toAPI(), JSON_PRETTY_PRINT)); diff --git a/lib/Command/Config/FolderAdd.php b/lib/Command/Config/FolderAdd.php index 257cf2dd6..03ad72535 100644 --- a/lib/Command/Config/FolderAdd.php +++ b/lib/Command/Config/FolderAdd.php @@ -47,9 +47,13 @@ class FolderAdd extends Command */ protected function execute(InputInterface $input, OutputInterface $output): int { - $user = $input->getArgument('user-id'); - $name = $input->getArgument('name'); - $parent = (int) $input->getOption('parent') ?? 0; + $user = $input->getArgument('user-id'); + $name = $input->getArgument('name'); + $parent = $input->getOption('parent'); + + if ($parent !== null) { + $parent = intval($parent); + } $this->folderService->create($user, $name, $parent); diff --git a/lib/Command/Config/FolderDelete.php b/lib/Command/Config/FolderDelete.php index bf7608b02..397eda999 100644 --- a/lib/Command/Config/FolderDelete.php +++ b/lib/Command/Config/FolderDelete.php @@ -50,11 +50,11 @@ class FolderDelete extends Command $user = $input->getArgument('user-id'); $id = $input->getArgument('folder-id'); - if ($id === '0') { + if ($id === null) { throw new ServiceException('Can not remove root folder!'); } - $this->folderService->delete($user, $id); + $this->folderService->delete($user, intval($id)); return 0; } diff --git a/lib/Controller/FeedApiController.php b/lib/Controller/FeedApiController.php index cb3d5e645..2c96bbc6e 100644 --- a/lib/Controller/FeedApiController.php +++ b/lib/Controller/FeedApiController.php @@ -27,6 +27,7 @@ use \OCP\AppFramework\Http; use \OCA\News\Service\FeedService; use \OCA\News\Service\ItemService; use Psr\Log\LoggerInterface; +use function GuzzleHttp\Psr7\uri_for; class FeedApiController extends ApiController { @@ -99,13 +100,17 @@ class FeedApiController extends ApiController * @NoCSRFRequired * @CORS * - * @param string $url - * @param int $folderId + * @param string $url + * @param int|null $folderId * * @return array|mixed|JSONResponse */ - public function create(string $url, int $folderId = 0) + public function create(string $url, ?int $folderId = null) { + if ($folderId === 0) { + $folderId = null; + } + try { $this->feedService->purgeDeleted($this->getUserId(), false); @@ -169,13 +174,17 @@ class FeedApiController extends ApiController * @NoCSRFRequired * @CORS * - * @param int $feedId - * @param int $folderId + * @param int $feedId + * @param int|null $folderId * * @return array|JSONResponse */ - public function move(int $feedId, int $folderId) + public function move(int $feedId, ?int $folderId) { + if ($folderId === 0) { + $folderId = null; + } + try { $this->feedService->patch( $feedId, diff --git a/lib/Controller/FeedController.php b/lib/Controller/FeedController.php index c09096bd9..78df7be0c 100644 --- a/lib/Controller/FeedController.php +++ b/lib/Controller/FeedController.php @@ -105,6 +105,9 @@ class FeedController extends Controller // check if feed or folder exists try { if ($feedType === FeedType::FOLDER) { + if ($feedId === 0) { + $feedId = null; + } $this->folderService->find($this->userId, $feedId); } elseif ($feedType === FeedType::FEED) { $this->feedService->find($this->userId, $feedId); @@ -131,20 +134,23 @@ class FeedController extends Controller * @NoAdminRequired * * @param string $url - * @param int $parentFolderId - * @param string $title - * @param string $user - * @param string $password + * @param int|null $parentFolderId + * @param string|null $title + * @param string|null $user + * @param string|null $password * * @return array|JSONResponse */ public function create( string $url, - int $parentFolderId, + ?int $parentFolderId, ?string $title = null, ?string $user = null, ?string $password = null ) { + if ($parentFolderId === 0) { + $parentFolderId = null; + } try { // we need to purge deleted feeds if a feed is created to // prevent already exists exceptions @@ -290,13 +296,13 @@ class FeedController extends Controller /** * @NoAdminRequired * - * @param int $feedId - * @param bool $pinned - * @param bool $fullTextEnabled - * @param int $updateMode - * @param int $ordering - * @param int $folderId - * @param string $title + * @param int $feedId + * @param bool $pinned + * @param bool $fullTextEnabled + * @param int|null $updateMode + * @param int|null $ordering + * @param int|null $folderId + * @param string|null $title * * @return array|JSONResponse */ @@ -315,7 +321,7 @@ class FeedController extends Controller 'updateMode' => $updateMode, 'ordering' => $ordering, 'title' => $title, - 'folderId' => $folderId + 'folderId' => $folderId === 0 ? null : $folderId ]; $diff = array_filter( diff --git a/lib/Controller/FolderApiController.php b/lib/Controller/FolderApiController.php index 1f79d7cf9..691c67139 100644 --- a/lib/Controller/FolderApiController.php +++ b/lib/Controller/FolderApiController.php @@ -90,12 +90,16 @@ class FolderApiController extends ApiController * @NoCSRFRequired * @CORS * - * @param int $folderId + * @param int|null $folderId * * @return array|JSONResponse */ - public function delete(int $folderId) + public function delete(?int $folderId) { + if (empty($folderId)) { + return new JSONResponse([], Http::STATUS_BAD_REQUEST); + } + try { $this->folderService->delete($folderId, $this->getUserId()); } catch (ServiceNotFoundException $ex) { @@ -111,13 +115,17 @@ class FolderApiController extends ApiController * @NoCSRFRequired * @CORS * - * @param int $folderId - * @param string $name + * @param int|null $folderId + * @param string $name * * @return array|JSONResponse */ - public function update(int $folderId, string $name) + public function update(?int $folderId, string $name) { + if (empty($folderId)) { + return new JSONResponse([], Http::STATUS_BAD_REQUEST); + } + try { $this->folderService->rename($folderId, $name, $this->getUserId()); } catch (ServiceValidationException $ex) { @@ -137,11 +145,14 @@ class FolderApiController extends ApiController * @NoCSRFRequired * @CORS * - * @param int $folderId - * @param int $newestItemId + * @param int|null $folderId + * @param int $newestItemId */ - public function read(int $folderId, int $newestItemId): void + public function read(?int $folderId, int $newestItemId): void { + if ($folderId === 0) { + $folderId = null; + } $this->itemService->readFolder($folderId, $newestItemId, $this->getUserId()); } } diff --git a/lib/Controller/FolderController.php b/lib/Controller/FolderController.php index d19726963..09900f5b2 100644 --- a/lib/Controller/FolderController.php +++ b/lib/Controller/FolderController.php @@ -64,13 +64,15 @@ class FolderController extends Controller /** * @NoAdminRequired * - * @param int $folderId - * @param bool $open + * @param int|null $folderId + * @param bool $open * * @return array|JSONResponse */ - public function open(int $folderId, bool $open) + public function open(?int $folderId, bool $open) { + $folderId = $folderId === 0 ? null : $folderId; + try { $this->folderService->open($folderId, $open, $this->userId); } catch (ServiceException $ex) { @@ -108,12 +110,15 @@ class FolderController extends Controller /** * @NoAdminRequired * - * @param int $folderId + * @param int|null $folderId * * @return array|JSONResponse */ - public function delete(int $folderId) + public function delete(?int $folderId) { + if (empty($folderId)) { + return new JSONResponse([], Http::STATUS_BAD_REQUEST); + } try { $this->folderService->markDeleted($folderId, $this->userId); } catch (ServiceNotFoundException $ex) { @@ -127,13 +132,16 @@ class FolderController extends Controller /** * @NoAdminRequired * - * @param string $folderName - * @param int $folderId + * @param string $folderName + * @param int|null $folderId * * @return array|JSONResponse */ - public function rename(string $folderName, int $folderId) + public function rename(string $folderName, ?int $folderId) { + if (empty($folderId)) { + return new JSONResponse([], Http::STATUS_BAD_REQUEST); + } try { $folder = $this->folderService->rename( $folderId, @@ -154,12 +162,15 @@ class FolderController extends Controller /** * @NoAdminRequired * - * @param int $folderId - * @param int $highestItemId + * @param int|null $folderId + * @param int $highestItemId + * * @return array */ - public function read(int $folderId, int $highestItemId): array + public function read(?int $folderId, int $highestItemId): array { + $folderId = $folderId === 0 ? null : $folderId; + $this->itemService->readFolder( $folderId, $highestItemId, @@ -173,12 +184,14 @@ class FolderController extends Controller /** * @NoAdminRequired * - * @param int $folderId + * @param int|null $folderId * * @return array|JSONResponse */ - public function restore(int $folderId) + public function restore(?int $folderId) { + $folderId = $folderId === 0 ? null : $folderId; + try { $this->folderService->unmarkDeleted($folderId, $this->userId); } catch (ServiceNotFoundException $ex) { diff --git a/lib/Controller/UserApiController.php b/lib/Controller/UserApiController.php deleted file mode 100644 index b644ba1f0..000000000 --- a/lib/Controller/UserApiController.php +++ /dev/null @@ -1,74 +0,0 @@ - - * @author Bernhard Posselt - * @author David Guillot - * @copyright 2012 Alessandro Cosentino - * @copyright 2012-2014 Bernhard Posselt - * @copyright 2018 David Guillot - */ - -namespace OCA\News\Controller; - -use \OCP\IRequest; -use \OCP\IUserSession; -use \OCP\IURLGenerator; -use \OCP\Files\IRootFolder; -use \OCP\AppFramework\Http; - -class UserApiController extends ApiController -{ - - private $rootFolder; - - public function __construct( - string $appName, - IRequest $request, - IUserSession $userSession, - IRootFolder $rootFolder - ) { - parent::__construct($appName, $request, $userSession); - $this->rootFolder = $rootFolder; - } - - /** - * @NoAdminRequired - * @NoCSRFRequired - * @CORS - */ - public function index(): array - { - $user = $this->getUser(); - - // find the avatar - $jpgAvatar = '/' . $user->getUID() . '/avatar.jpg'; - $pngAvatar = '/' . $user->getUID() . '/avatar.png'; - $avatar = null; - - if ($this->rootFolder->nodeExists($jpgAvatar)) { - $file = $this->rootFolder->get($jpgAvatar); - $avatar = [ - 'data' => base64_encode($file->getContent()), - 'mime' => 'image/jpeg' - ]; - } elseif ($this->rootFolder->nodeExists($pngAvatar)) { - $file = $this->rootFolder->get($pngAvatar); - $avatar = [ - 'data' => base64_encode($file->getContent()), - 'mime' => 'image/png' - ]; - } - - return [ - 'userId' => $user->getUID(), - 'displayName' => $user->getDisplayName(), - 'lastLoginTimestamp' => $user->getLastLogin(), - 'avatar' => $avatar - ]; - } -} diff --git a/lib/Db/Feed.php b/lib/Db/Feed.php index 852de4c78..473912acc 100644 --- a/lib/Db/Feed.php +++ b/lib/Db/Feed.php @@ -37,7 +37,7 @@ class Feed extends Entity implements IAPI, \JsonSerializable protected $faviconLink = null; /** @var int|null */ protected $added = 0; - /** @var int */ + /** @var int|null */ protected $folderId; /** @var int */ protected $unreadCount; @@ -152,9 +152,9 @@ class Feed extends Entity implements IAPI, \JsonSerializable } /** - * @return int + * @return int|null */ - public function getFolderId(): int + public function getFolderId(): ?int { return $this->folderId; } @@ -416,9 +416,11 @@ class Feed extends Entity implements IAPI, \JsonSerializable } /** - * @param int $folderId + * @param int|null $folderId + * + * @return Feed */ - public function setFolderId(int $folderId): Feed + public function setFolderId(?int $folderId): Feed { if ($this->folderId !== $folderId) { $this->folderId = $folderId; diff --git a/lib/Db/FeedMapper.php b/lib/Db/FeedMapper.php index 867ba982d..193cf1f5d 100644 --- a/lib/Db/FeedMapper.php +++ b/lib/Db/FeedMapper.php @@ -77,7 +77,7 @@ class FeedMapper extends NewsMapper // think twice when changing this 'AND `items`.`unread` = ? ' . 'WHERE `feeds`.`user_id` = ? ' . - 'AND (`feeds`.`folder_id` = 0 ' . + 'AND (`feeds`.`folder_id` IS NULL ' . 'OR `folders`.`deleted_at` = 0 ' . ') ' . 'AND `feeds`.`deleted_at` = 0 ' . @@ -106,7 +106,7 @@ class FeedMapper extends NewsMapper // POSSIBLE SQL INJECTION RISK WHEN MODIFIED WITHOUT THOUGHT. // think twice when changing this 'AND `items`.`unread` = ? ' . - 'WHERE (`feeds`.`folder_id` = 0 ' . + 'WHERE (`feeds`.`folder_id` IS NULL ' . 'OR `folders`.`deleted_at` = 0 ' . ') ' . 'AND `feeds`.`deleted_at` = 0 ' . diff --git a/lib/Db/FeedMapperV2.php b/lib/Db/FeedMapperV2.php index a7edecd88..05b272112 100644 --- a/lib/Db/FeedMapperV2.php +++ b/lib/Db/FeedMapperV2.php @@ -43,6 +43,7 @@ class FeedMapperV2 extends NewsMapperV2 * Find all feeds for a user. * * @param string $userId The user identifier + * @param array $params Filter parameters * * @return Entity[] */ @@ -62,7 +63,7 @@ class FeedMapperV2 extends NewsMapperV2 * Find all feeds for a user. * * @param string $userId The user identifier - * @param int $id The feed identifier + * @param int $id The feed identifier * * @return Entity * @@ -124,17 +125,22 @@ class FeedMapperV2 extends NewsMapperV2 /** * Find all feeds in a folder * - * @param int $id ID of the folder + * @param int|null $id ID of the folder * * @return Feed[] */ - public function findAllFromFolder(int $id): array + public function findAllFromFolder(?int $id): array { $builder = $this->db->getQueryBuilder(); $builder->addSelect('*') - ->from($this->tableName) - ->where('folder_id = :folder_id') - ->setParameter(':folder_id', $id); + ->from($this->tableName); + + if (is_null($id)) { + $builder->where('folder_id IS NULL'); + } else { + $builder->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 2efd65e96..d452bb7d4 100644 --- a/lib/Db/Folder.php +++ b/lib/Db/Folder.php @@ -141,7 +141,7 @@ class Folder extends Entity implements IAPI, \JsonSerializable return $this; } - public function setParentId(int $parentId = 0): self + public function setParentId(?int $parentId = null): self { if ($this->parentId !== $parentId) { $this->parentId = $parentId; diff --git a/lib/Db/FolderMapperV2.php b/lib/Db/FolderMapperV2.php index 7d0536607..913bd9d70 100644 --- a/lib/Db/FolderMapperV2.php +++ b/lib/Db/FolderMapperV2.php @@ -41,6 +41,7 @@ class FolderMapperV2 extends NewsMapperV2 * Find all feeds for a user. * * @param string $userId The user identifier + * @param array $params Filter parameters * * @return Entity[] */ diff --git a/lib/Db/ItemMapper.php b/lib/Db/ItemMapper.php index 270919b44..65c6e0b15 100644 --- a/lib/Db/ItemMapper.php +++ b/lib/Db/ItemMapper.php @@ -55,7 +55,7 @@ class ItemMapper extends NewsMapper $prependTo . 'LEFT OUTER JOIN `*PREFIX*news_folders` `folders` ' . 'ON `folders`.`id` = `feeds`.`folder_id` ' . - 'WHERE `feeds`.`folder_id` = 0 ' . + 'WHERE `feeds`.`folder_id` IS NULL ' . 'OR `folders`.`deleted_at` = 0 ' . 'ORDER BY `items`.`id` ' . $ordering; } @@ -125,7 +125,7 @@ class ItemMapper extends NewsMapper 'AND `items`.`starred` = ? ' . 'LEFT OUTER JOIN `*PREFIX*news_folders` `folders` ' . 'ON `folders`.`id` = `feeds`.`folder_id` ' . - 'WHERE `feeds`.`folder_id` = 0 ' . + 'WHERE `feeds`.`folder_id` IS NULL ' . 'OR `folders`.`deleted_at` = 0'; $params = [$userId, true]; @@ -151,14 +151,15 @@ class ItemMapper extends NewsMapper } - public function readFolder($folderId, $highestItemId, $time, $userId) + public function readFolder(?int $folderId, $highestItemId, $time, $userId) { + $folderWhere = is_null($folderId) ? 'IS' : '='; $sql = 'UPDATE `*PREFIX*news_items` ' . 'SET unread = ? ' . ', `last_modified` = ? ' . 'WHERE `feed_id` IN (' . 'SELECT `id` FROM `*PREFIX*news_feeds` ' . - 'WHERE `folder_id` = ? ' . + "WHERE `folder_id` ${folderWhere} ? " . 'AND `user_id` = ? ' . ') ' . 'AND `id` <= ?'; @@ -207,11 +208,12 @@ class ItemMapper extends NewsMapper } - public function findAllNewFolder($id, $updatedSince, $showAll, $userId) + public function findAllNewFolder(?int $id, $updatedSince, $showAll, $userId) { $sql = $this->buildStatusQueryPart($showAll); - $sql .= 'AND `feeds`.`folder_id` = ? ' . + $folderWhere = is_null($id) ? 'IS' : '='; + $sql .= "AND `feeds`.`folder_id` ${$folderWhere} ? " . 'AND `items`.`last_modified` >= ? '; $sql = $this->makeSelectQuery($sql); $params = [$userId, $id, $updatedSince]; @@ -270,7 +272,7 @@ class ItemMapper extends NewsMapper public function findAllFolder( - $id, + ?int $id, $limit, $offset, $showAll, @@ -285,10 +287,10 @@ class ItemMapper extends NewsMapper $sql = $this->buildStatusQueryPart($showAll); $sql .= $this->buildSearchQueryPart($search); - $sql .= 'AND `feeds`.`folder_id` = ? '; + $folderWhere = is_null($id) ? 'IS' : '='; + $sql .= "AND `feeds`.`folder_id` ${folderWhere} ? "; if ($offset !== 0) { - $sql .= 'AND `items`.`id` ' . - $this->getOperator($oldestFirst) . ' ? '; + $sql .= 'AND `items`.`id` ' . $this->getOperator($oldestFirst) . ' ? '; $params[] = $offset; } $sql = $this->makeSelectQuery($sql, $oldestFirst, $search); diff --git a/lib/Migration/Version150005Date20201009192341.php b/lib/Migration/Version150005Date20201009192341.php new file mode 100644 index 000000000..937d3f50b --- /dev/null +++ b/lib/Migration/Version150005Date20201009192341.php @@ -0,0 +1,98 @@ +connection = $connection; + } + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + */ + public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + $qb = $this->connection->getQueryBuilder(); + + $qb->update('news_feeds') + ->set('folder_id', $qb->createPositionalParameter(null, IQueryBuilder::PARAM_NULL)) + ->where('folder_id = 0') + ->execute(); + + $feed_name = $this->connection->getQueryBuilder()->getTableName('news_feeds'); + $folder_name = $this->connection->getQueryBuilder()->getTableName('news_folders'); + + $items_query = "DELETE FROM ${feed_name} WHERE ${feed_name}.`folder_id` NOT IN (SELECT DISTINCT id FROM ${folder_name}) AND ${feed_name}.`folder_id` IS NOT NULL"; + $this->connection->executeQuery($items_query); + + $item_name = $this->connection->getQueryBuilder()->getTableName('news_items'); + $feed_name = $this->connection->getQueryBuilder()->getTableName('news_feeds'); + + $items_query = "DELETE FROM ${item_name} WHERE ${item_name}.`feed_id` NOT IN (SELECT DISTINCT id FROM ${feed_name})"; + $this->connection->executeQuery($items_query); + } + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + if ($schema->hasTable('news_items') && + !$schema->getTable('news_items')->hasForeignKey('feed')) { + + $schema->getTable('news_items') + ->addForeignKeyConstraint( + $schema->getTable('news_feeds')->getName(), + ['feed_id'], + ['id'], + ['onDelete' => 'CASCADE'], + 'feed' + ); + } + + if ($schema->hasTable('news_feeds') && + !$schema->getTable('news_feeds')->hasForeignKey('folder')) { + + $schema->getTable('news_feeds') + ->addForeignKeyConstraint( + $schema->getTable('news_folders')->getName(), + ['folder_id'], + ['id'], + ['onDelete' => 'CASCADE'], + 'folder' + ); + } + + return $schema; + } + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + */ + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + } +} diff --git a/lib/Service/FeedService.php b/lib/Service/FeedService.php index ab9c18219..c671a035c 100644 --- a/lib/Service/FeedService.php +++ b/lib/Service/FeedService.php @@ -103,21 +103,27 @@ class FeedService extends Service /** * Creates a new feed * - * @param string $feedUrl the url to the feed - * @param int $folderId the folder where it should be put into, 0 for root + * @param string $feedUrl the url to the feed + * @param int|null $folderId the folder where it should be put into, null for root * folder - * @param string $userId for which user the feed should be created - * @param string $title if given, this is used for the opml feed title - * @param string $user if given, basic auth is set for this feed - * @param string $password if given, basic auth is set for this + * @param string $userId for which user the feed should be created + * @param string|null $title if given, this is used for the opml feed title + * @param string|null $user if given, basic auth is set for this feed + * @param string|null $password if given, basic auth is set for this * feed. Ignored if user is null or an empty string * + * @return Feed the newly created feed * @throws ServiceConflictException if the feed exists already * @throws ServiceNotFoundException if the url points to an invalid feed - * @return Feed the newly created feed */ - public function create($feedUrl, $folderId, $userId, $title = null, $user = null, $password = null) - { + public function create( + string $feedUrl, + ?int $folderId, + string $userId, + string $title = null, + string $user = null, + string $password = null + ) { // first try if the feed exists already try { /** @@ -369,7 +375,7 @@ class FeedService extends Service $feed->setUrl($url); $feed->setTitle($this->l10n->t('Articles without feed')); $feed->setAdded($this->timeFactory->getTime()); - $feed->setFolderId(0); + $feed->setFolderId(null); $feed->setPreventUpdate(true); /** @var Feed $feed */ $feed = $this->feedMapper->insert($feed); diff --git a/lib/Service/FeedServiceV2.php b/lib/Service/FeedServiceV2.php index 2304c3286..4be96ebf8 100644 --- a/lib/Service/FeedServiceV2.php +++ b/lib/Service/FeedServiceV2.php @@ -121,11 +121,11 @@ class FeedServiceV2 extends Service } /** - * @param int $id + * @param int|null $id * * @return Feed[] */ - public function findAllFromFolder(int $id): array + public function findAllFromFolder(?int $id): array { return $this->mapper->findAllFromFolder($id); } @@ -182,7 +182,7 @@ class FeedServiceV2 extends Service * * @param string $userId Feed owner * @param string $feedUrl Feed URL - * @param int $folderId Target folder, defaults to root + * @param int|null $folderId Target folder, defaults to root * @param bool $full_text Scrape the feed for full text * @param string|null $title The feed title * @param string|null $user Basic auth username, if set @@ -196,7 +196,7 @@ class FeedServiceV2 extends Service public function create( string $userId, string $feedUrl, - int $folderId = 0, + ?int $folderId = null, bool $full_text = false, ?string $title = null, ?string $user = null, diff --git a/lib/Service/FolderService.php b/lib/Service/FolderService.php index cd8b4b852..db060b050 100644 --- a/lib/Service/FolderService.php +++ b/lib/Service/FolderService.php @@ -101,24 +101,28 @@ class FolderService extends Service * @throws ServiceValidationException if the folder has invalid parameters * @throws ServiceConflictException if name exists already */ - public function create(string $folderName, string $userId, int $parentId = 0) + public function create(string $folderName, string $userId, ?int $parentId = null) { $this->validateFolder($folderName, $userId); $folder = new Folder(); - $folder->setName($folderName); - $folder->setUserId($userId); - $folder->setParentId($parentId); - $folder->setOpened(true); + $folder->setName($folderName) + ->setUserId($userId) + ->setParentId($parentId) + ->setOpened(true); return $this->folderMapper->insert($folder); } /** - * @throws ServiceException if the folder does not exist + * @param int|null $folderId + * @param bool $opened + * @param string $userId + * + * @throws ServiceNotFoundException */ - public function open(int $folderId, bool $opened, string $userId) + public function open(?int $folderId, bool $opened, string $userId) { $folder = $this->find($userId, $folderId); $folder->setOpened($opened); diff --git a/lib/Service/FolderServiceV2.php b/lib/Service/FolderServiceV2.php index cf599456b..784d82f8c 100644 --- a/lib/Service/FolderServiceV2.php +++ b/lib/Service/FolderServiceV2.php @@ -80,7 +80,7 @@ class FolderServiceV2 extends Service return $this->mapper->findAll(); } - public function create(string $userId, string $name, int $parent = 0): Entity + public function create(string $userId, string $name, ?int $parent = null): Entity { $folder = new Folder(); $folder->setUserId($userId) diff --git a/lib/Service/ItemService.php b/lib/Service/ItemService.php index 5f85e0b84..ab5536137 100644 --- a/lib/Service/ItemService.php +++ b/lib/Service/ItemService.php @@ -54,16 +54,17 @@ class ItemService extends Service /** * Returns all new items * - * @param int $id the id of the feed, 0 for starred or all items - * @param int $type the type of the feed - * @param int $updatedSince a timestamp with the last modification date + * @param int|null $id the id of the feed, 0 for starred or all items + * @param int $type the type of the feed + * @param int $updatedSince a timestamp with the last modification date * returns only items with a >= modified * timestamp - * @param boolean $showAll if unread items should also be returned - * @param string $userId the name of the user + * @param boolean $showAll if unread items should also be returned + * @param string $userId the name of the user + * * @return array of items */ - public function findAllNew($id, $type, $updatedSince, $showAll, $userId) + public function findAllNew(?int $id, $type, $updatedSince, $showAll, $userId) { switch ($type) { case FeedType::FEED: @@ -94,20 +95,21 @@ class ItemService extends Service /** * Returns all items * - * @param int $id the id of the feed, 0 for starred or all items - * @param int $type the type of the feed - * @param int $limit how many items should be returned - * @param int $offset the offset - * @param boolean $showAll if unread items should also be returned - * @param boolean $oldestFirst if it should be ordered by oldest first - * @param string $userId the name of the user - * @param string[] $search an array of keywords that the result should + * @param int|null $id the id of the feed, 0 for starred or all items + * @param int $type the type of the feed + * @param int $limit how many items should be returned + * @param int $offset the offset + * @param boolean $showAll if unread items should also be returned + * @param boolean $oldestFirst if it should be ordered by oldest first + * @param string $userId the name of the user + * @param string[] $search an array of keywords that the result should * contain in either the author, title, link * or body + * * @return array of items */ public function findAllItems( - $id, + ?int $id, $type, $limit, $offset, @@ -225,13 +227,13 @@ class ItemService extends Service /** * Set a folder read * - * @param int $folderId the id of the folder that should be marked read - * @param int $highestItemId all items below that are marked read. This is - * used to prevent marking items as read that - * the users hasn't seen yet - * @param string $userId the name of the user + * @param int|null $folderId the id of the folder that should be marked read + * @param int $highestItemId all items below that are marked read. This is + * used to prevent marking items as read that + * the users hasn't seen yet + * @param string $userId the name of the user */ - public function readFolder($folderId, $highestItemId, $userId) + public function readFolder(?int $folderId, $highestItemId, $userId) { $time = $this->timeFactory->getMicroTime(); $this->itemMapper->readFolder( diff --git a/lib/Utility/OPMLExporter.php b/lib/Utility/OPMLExporter.php index 880222da0..aa06f5dbf 100644 --- a/lib/Utility/OPMLExporter.php +++ b/lib/Utility/OPMLExporter.php @@ -69,7 +69,7 @@ class OPMLExporter // feeds without folders foreach ($feeds as $feed) { - if ($feed->getFolderId() === 0) { + if ($feed->getFolderId() === null) { $feedOutline = $this->createFeedOutline($feed, $document); $body->appendChild($feedOutline); } diff --git a/templates/index.php b/templates/index.php index 4b0408af1..f01f246e2 100644 --- a/templates/index.php +++ b/templates/index.php @@ -57,7 +57,7 @@ foreach (Plugin::getScripts() as $appName => $fileName) { inc('part.navigation.unreadfeed')) ?> inc('part.navigation.starredfeed')) ?> inc( - 'part.navigation.feed', ['folderId' => '0'] + 'part.navigation.feed', ['folderId' => null] )) ?> inc('part.navigation.folder')) ?> inc('part.navigation.explore')) ?> diff --git a/tests/Integration/Fixtures/FeedFixture.php b/tests/Integration/Fixtures/FeedFixture.php index f0de6de2d..77abd3ee0 100644 --- a/tests/Integration/Fixtures/FeedFixture.php +++ b/tests/Integration/Fixtures/FeedFixture.php @@ -30,7 +30,7 @@ class FeedFixture extends Feed 'title' => 'title', 'faviconLink' => 'http://feed.com/favicon.ico', 'added' => 3000, - 'folderId' => 0, + 'folderId' => null, 'link' => 'http://feed.com/rss', 'preventUpdate' => false, 'deletedAt' => 0, diff --git a/tests/Integration/Fixtures/FolderFixture.php b/tests/Integration/Fixtures/FolderFixture.php index 19f19041b..09c03e5de 100644 --- a/tests/Integration/Fixtures/FolderFixture.php +++ b/tests/Integration/Fixtures/FolderFixture.php @@ -19,11 +19,11 @@ class FolderFixture extends Folder { use Fixture; - public function __construct(array $defaults=[]) + public function __construct(array $defaults=[]) { $defaults = array_merge( [ - 'parentId' => 0, + 'parentId' => null, 'name' => 'folder', 'userId' => 'test', 'opened' => true, diff --git a/tests/Integration/IntegrationTest.php b/tests/Integration/IntegrationTest.php index 1135bbd2e..f943e2940 100644 --- a/tests/Integration/IntegrationTest.php +++ b/tests/Integration/IntegrationTest.php @@ -141,7 +141,7 @@ abstract class IntegrationTest extends \Test\TestCase } } - protected function loadFeedFixtures(array $feedFixtures = [], $folderId = 0) + protected function loadFeedFixtures(array $feedFixtures = [], $folderId = null) { foreach ($feedFixtures as $feedFixture) { $feed = new FeedFixture($feedFixture); diff --git a/tests/Unit/Command/FeedAddTest.php b/tests/Unit/Command/FeedAddTest.php index c0d4de1bd..0f2d3606f 100644 --- a/tests/Unit/Command/FeedAddTest.php +++ b/tests/Unit/Command/FeedAddTest.php @@ -69,7 +69,7 @@ class FeedAddTest extends TestCase $this->consoleInput->expects($this->exactly(5)) ->method('getOption') ->will($this->returnValueMap([ - ['folder', '0'], + ['folder', null], ['title', 'title'], ['username', 'user'], ['password', 'pass'], @@ -80,7 +80,7 @@ class FeedAddTest extends TestCase $this->service->expects($this->exactly(1)) ->method('create') - ->with('admin', 'http://feed', 0, true, 'title', 'user', 'pass') + ->with('admin', 'http://feed', null, true, 'title', 'user', 'pass') ->willReturn($feed); $this->service->expects($this->exactly(1)) diff --git a/tests/Unit/Command/FolderDeleteTest.php b/tests/Unit/Command/FolderDeleteTest.php index 9db078b33..c87b05acd 100644 --- a/tests/Unit/Command/FolderDeleteTest.php +++ b/tests/Unit/Command/FolderDeleteTest.php @@ -88,7 +88,7 @@ class FolderDeleteTest extends TestCase $this->consoleInput->expects($this->exactly(2)) ->method('getArgument') ->will($this->returnValueMap([ - ['folder-id', '0'], + ['folder-id', null], ['user-id', 'admin'], ])); diff --git a/tests/Unit/Controller/UserApiControllerTest.php b/tests/Unit/Controller/UserApiControllerTest.php deleted file mode 100644 index dd0140c81..000000000 --- a/tests/Unit/Controller/UserApiControllerTest.php +++ /dev/null @@ -1,155 +0,0 @@ - - * @author Bernhard Posselt - * @copyright 2012 Alessandro Cosentino - * @copyright 2012-2014 Bernhard Posselt - */ - -namespace OCA\News\Tests\Unit\Controller; - -use OCA\News\Controller\UserApiController; -use OCP\Files\File; -use OCP\Files\IRootFolder; -use OCP\IRequest; -use OCP\IUser; -use OCP\IUserSession; - -use PHPUnit\Framework\TestCase; - -class UserApiControllerTest extends TestCase -{ - - private $request; - private $appName; - private $rootFolder; - private $userSession; - private $controller; - private $user; - private $file; - - protected function setUp(): void - { - $this->appName = 'news'; - $this->request = $this->getMockBuilder(IRequest::class) - ->disableOriginalConstructor() - ->getMock(); - $this->rootFolder = $this->getMockBuilder(IRootFolder::class) - ->disableOriginalConstructor() - ->getMock(); - $this->file = $this->getMockBuilder(File::class) - ->disableOriginalConstructor() - ->getMock(); - $this->userSession = $this->getMockBuilder(IUserSession::class) - ->disableOriginalConstructor() - ->getMock(); - $this->user = $this->getMockBuilder(IUser::class) - ->disableOriginalConstructor() - ->getMock(); - $this->controller = new UserApiController( - $this->appName, $this->request, $this->userSession, - $this->rootFolder - ); - - - } - - private function expectUser($uid, $displayName, $lastLogin) - { - $this->userSession->expects($this->any()) - ->method('getUser') - ->will($this->returnValue($this->user)); - $this->user->expects($this->any()) - ->method('getUID') - ->will($this->returnValue($uid)); - $this->user->expects($this->any()) - ->method('getLastLogin') - ->will($this->returnValue($lastLogin)); - $this->user->expects($this->any()) - ->method('getDisplayName') - ->will($this->returnValue($displayName)); - } - - private function expectImg($isJpg, $isPng, $user, $exists, $data) - { - $jpg = '/' . $user . '/' . 'avatar.jpg'; - $png = '/' . $user . '/' . 'avatar.png'; - - $this->rootFolder->expects($this->any()) - ->method('nodeExists') - ->will( - $this->returnValueMap( - [ - [$jpg, $isJpg], - [$png, $isPng] - ] - ) - ); - $this->rootFolder->expects($this->any()) - ->method('get') - ->will($this->returnValue($this->file)); - $this->file->expects($this->any()) - ->method('getContent') - ->will($this->returnValue($data)); - } - - public function testGetJpeg() - { - $this->expectUser('john', 'John', 123); - $this->expectImg(true, false, 'john', true, 'hi'); - - $result = $this->controller->index(); - $expected = [ - 'userId' => 'john', - 'displayName' => 'John', - 'lastLoginTimestamp' => 123, - 'avatar' => [ - 'data' => base64_encode('hi'), - 'mime' => 'image/jpeg' - ] - ]; - - $this->assertEquals($expected, $result); - } - - public function testGetPng() - { - $this->expectUser('john', 'John', 123); - $this->expectImg(false, true, 'john', false, 'hi'); - - $result = $this->controller->index(); - $expected = [ - 'userId' => 'john', - 'displayName' => 'John', - 'lastLoginTimestamp' => 123, - 'avatar' => [ - 'data' => base64_encode('hi'), - 'mime' => 'image/png' - ] - ]; - - $this->assertEquals($expected, $result); - } - - public function testNoAvatar() - { - $this->expectUser('john', 'John', 123); - $this->expectImg(false, false, 'john', false, 'hi'); - - $result = $this->controller->index(); - $expected = [ - 'userId' => 'john', - 'displayName' => 'John', - 'lastLoginTimestamp' => 123, - 'avatar' => null - ]; - - $this->assertEquals($expected, $result); - } - -} diff --git a/tests/Unit/Service/FeedServiceTest.php b/tests/Unit/Service/FeedServiceTest.php index 3b39019c1..769453459 100644 --- a/tests/Unit/Service/FeedServiceTest.php +++ b/tests/Unit/Service/FeedServiceTest.php @@ -913,7 +913,7 @@ class FeedServiceTest extends TestCase $insertFeed->setTitle('Articles without feed'); $insertFeed->setAdded($this->time); $insertFeed->setPreventUpdate(true); - $insertFeed->setFolderId(0); + $insertFeed->setFolderId(null); $this->l10n->expects($this->once()) ->method('t') diff --git a/tests/Unit/Utility/OPMLExporterTest.php b/tests/Unit/Utility/OPMLExporterTest.php index 8c7c3c923..bc0fd7bee 100644 --- a/tests/Unit/Utility/OPMLExporterTest.php +++ b/tests/Unit/Utility/OPMLExporterTest.php @@ -37,7 +37,7 @@ class OPMLExporterTest extends TestCase $this->exporter = new OPMLExporter(); $this->folder1 = new Folder(); $this->folder1->setId(3); - $this->folder1->setParentId(0); + $this->folder1->setParentId(null); $this->folder1->setName('Örgendwas'); $this->folder2 = new Folder(); $this->folder2->setId(1); @@ -46,7 +46,7 @@ class OPMLExporterTest extends TestCase $this->feed1 = new Feed(); $this->feed1->setUrl('http://url1'); $this->feed1->setTitle('tötel'); - $this->feed1->setFolderId(0); + $this->feed1->setFolderId(null); $this->feed2 = new Feed(); $this->feed2->setUrl('http://url'); $this->feed2->setTitle('ttel df'); -- cgit v1.2.3