From 4ee3fcb78113caff9f2b890cb7d1702dc936d81e Mon Sep 17 00:00:00 2001 From: Sean Molenaar Date: Mon, 16 Nov 2020 20:49:38 +0100 Subject: Refactor User ID fetching and fix non-specific cleanup Signed-off-by: Sean Molenaar --- lib/Controller/Controller.php | 63 +++++++++++ lib/Controller/ExportController.php | 14 ++- lib/Controller/FeedApiController.php | 2 +- lib/Controller/FeedController.php | 45 ++++---- lib/Controller/FolderApiController.php | 2 +- lib/Controller/FolderController.php | 27 +++-- lib/Controller/ItemController.php | 44 ++++---- lib/Controller/PageController.php | 21 ++-- lib/Db/ItemMapperV2.php | 11 ++ lib/Db/NewsMapperV2.php | 22 +++- lib/Service/FeedServiceV2.php | 12 ++- lib/Service/FolderServiceV2.php | 7 +- lib/Service/UpdaterService.php | 4 +- tests/Integration/IntegrationTest.php | 1 - tests/Unit/Controller/ExportControllerTest.php | 24 ++++- tests/Unit/Controller/FeedApiControllerTest.php | 7 +- tests/Unit/Controller/FeedControllerTest.php | 18 ++-- tests/Unit/Controller/FolderControllerTest.php | 16 ++- tests/Unit/Controller/ItemControllerTest.php | 134 ++++++++++-------------- tests/Unit/Controller/PageControllerTest.php | 13 ++- tests/Unit/Service/FolderServiceTest.php | 12 +-- 21 files changed, 296 insertions(+), 203 deletions(-) create mode 100644 lib/Controller/Controller.php diff --git a/lib/Controller/Controller.php b/lib/Controller/Controller.php new file mode 100644 index 000000000..b2b86b007 --- /dev/null +++ b/lib/Controller/Controller.php @@ -0,0 +1,63 @@ + + * @author Bernhard Posselt + * @author David Guillot + * @copyright 2020 Sean Molenaar + */ + +namespace OCA\News\Controller; + +use OCP\AppFramework\Controller as BaseController; +use \OCP\IRequest; +use OCP\IUser; +use \OCP\IUserSession; + +/** + * Class ApiController + * + * @package OCA\News\Controller + */ +class Controller extends BaseController +{ + /** + * @var IUserSession + */ + private $userSession; + + /** + * ApiController constructor. + * + * Stores the user session to be able to leverage the user in further methods + * + * @param string $appName The name of the app + * @param IRequest $request The request + * @param IUserSession $userSession The user session + */ + public function __construct(string $appName, IRequest $request, IUserSession $userSession) + { + parent::__construct($appName, $request); + $this->userSession = $userSession; + } + + /** + * @return IUser + */ + protected function getUser() + { + return $this->userSession->getUser(); + } + + /** + * @return string + */ + protected function getUserId() + { + return $this->getUser()->getUID(); + } +} diff --git a/lib/Controller/ExportController.php b/lib/Controller/ExportController.php index 78f200934..4bd4c667d 100644 --- a/lib/Controller/ExportController.php +++ b/lib/Controller/ExportController.php @@ -19,8 +19,8 @@ use OCA\News\Service\ItemServiceV2; use OCA\News\Service\OpmlService; use OCP\AppFramework\Http\DataDownloadResponse; use \OCP\IRequest; -use \OCP\AppFramework\Controller; use \OCP\AppFramework\Http\JSONResponse; +use OCP\IUserSession; class ExportController extends Controller { @@ -29,7 +29,6 @@ class ExportController extends Controller private $folderService; private $feedService; private $itemService; - private $userId; public function __construct( string $appName, @@ -38,14 +37,13 @@ class ExportController extends Controller FeedServiceV2 $feedService, ItemServiceV2 $itemService, OpmlService $opmlService, - string $UserId + IUserSession $userSession ) { - parent::__construct($appName, $request); + parent::__construct($appName, $request, $userSession); $this->feedService = $feedService; $this->folderService = $folderService; $this->opmlService = $opmlService; $this->itemService = $itemService; - $this->userId = $UserId; } @@ -58,7 +56,7 @@ class ExportController extends Controller $date = date('Y-m-d'); return new DataDownloadResponse( - $this->opmlService->export($this->userId), + $this->opmlService->export($this->getUserId()), "subscriptions-${date}.opml", 'text/xml' ); @@ -71,8 +69,8 @@ class ExportController extends Controller */ public function articles(): JSONResponse { - $feeds = $this->feedService->findAllForUser($this->userId); - $items = $this->itemService->findAllForUser($this->userId, ['unread' => true, 'starred' => true]); + $feeds = $this->feedService->findAllForUser($this->getUserId()); + $items = $this->itemService->findAllForUser($this->getUserId(), ['unread' => true, 'starred' => true]); // build assoc array for fast access $feedsDict = []; diff --git a/lib/Controller/FeedApiController.php b/lib/Controller/FeedApiController.php index ae2f8cdaf..65181f8ac 100644 --- a/lib/Controller/FeedApiController.php +++ b/lib/Controller/FeedApiController.php @@ -117,7 +117,7 @@ class FeedApiController extends ApiController } try { - $this->feedService->purgeDeleted(); + $this->feedService->purgeDeleted($this->getUserId(), time() - 600); $feed = $this->feedService->create($this->getUserId(), $url, $folderId); $result = ['feeds' => $this->serialize($feed)]; diff --git a/lib/Controller/FeedController.php b/lib/Controller/FeedController.php index 76f777a12..5881d7eb9 100644 --- a/lib/Controller/FeedController.php +++ b/lib/Controller/FeedController.php @@ -19,13 +19,11 @@ use OCA\News\Service\FolderServiceV2; use OCP\AppFramework\Http\JSONResponse; use OCP\IRequest; use OCP\IConfig; -use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCA\News\Service\ItemService; use OCA\News\Service\FeedService; use OCA\News\Db\FeedType; -use OCP\IUser; use OCP\IUserSession; class FeedController extends Controller @@ -40,7 +38,9 @@ class FeedController extends Controller * @var FolderServiceV2 */ private $folderService; - private $userId; + /** + * @var IConfig + */ private $settings; public function __construct( @@ -50,14 +50,13 @@ class FeedController extends Controller FeedService $feedService, ItemService $itemService, IConfig $settings, - IUser $user + IUserSession $userSession ) { - parent::__construct($appName, $request); + parent::__construct($appName, $request, $userSession); $this->folderService = $folderService; $this->feedService = $feedService; $this->itemService = $itemService; $this->settings = $settings; - $this->userId = $user->getUID(); } @@ -71,13 +70,13 @@ 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->findAllForUser($this->userId), - 'starred' => $this->itemService->starredCount($this->userId) + 'feeds' => $this->feedService->findAllForUser($this->getUserId()), + 'starred' => $this->itemService->starredCount($this->getUserId()) ]; try { $params['newestItemId'] = - $this->itemService->getNewestItemId($this->userId); + $this->itemService->getNewestItemId($this->getUserId()); // An exception occurs if there is a newest item. If there is none, // simply ignore it and do not add the newestItemId @@ -94,12 +93,12 @@ class FeedController extends Controller public function active(): array { $feedId = (int) $this->settings->getUserValue( - $this->userId, + $this->getUserId(), $this->appName, 'lastViewedFeedId' ); $feedType = $this->settings->getUserValue( - $this->userId, + $this->getUserId(), $this->appName, 'lastViewedFeedType' ); @@ -115,9 +114,9 @@ class FeedController extends Controller if ($feedId === 0) { $feedId = null; } - $this->folderService->find($this->userId, $feedId); + $this->folderService->find($this->getUserId(), $feedId); } elseif ($feedType === FeedType::FEED) { - $this->feedService->find($this->userId, $feedId); + $this->feedService->find($this->getUserId(), $feedId); // if its the first launch, those values will be null } elseif ($feedType === null) { @@ -161,12 +160,12 @@ class FeedController extends Controller try { // we need to purge deleted feeds if a feed is created to // prevent already exists exceptions - $this->feedService->purgeDeleted($this->userId, false); + $this->feedService->purgeDeleted($this->getUserId(), false); $feed = $this->feedService->create( $url, $parentFolderId, - $this->userId, + $this->getUserId(), $title, $user, $password @@ -175,7 +174,7 @@ class FeedController extends Controller try { $params['newestItemId'] = - $this->itemService->getNewestItemId($this->userId); + $this->itemService->getNewestItemId($this->getUserId()); // An exception occurs if there is a newest item. If there is none, // simply ignore it and do not add the newestItemId @@ -201,7 +200,7 @@ class FeedController extends Controller public function delete(int $feedId) { try { - $this->feedService->markDeleted($feedId, $this->userId); + $this->feedService->markDeleted($feedId, $this->getUserId()); } catch (ServiceNotFoundException $ex) { return $this->error($ex, Http::STATUS_NOT_FOUND); } @@ -220,7 +219,7 @@ class FeedController extends Controller public function update(int $feedId) { try { - $feed = $this->feedService->update($this->userId, $feedId); + $feed = $this->feedService->update($this->getUserId(), $feedId); return [ 'feeds' => [ @@ -246,10 +245,10 @@ class FeedController extends Controller */ public function import(array $json): array { - $feed = $this->feedService->importArticles($json, $this->userId); + $feed = $this->feedService->importArticles($json, $this->getUserId()); $params = [ - 'starred' => $this->itemService->starredCount($this->userId) + 'starred' => $this->itemService->starredCount($this->getUserId()) ]; if ($feed) { @@ -269,7 +268,7 @@ class FeedController extends Controller */ public function read(int $feedId, int $highestItemId): array { - $this->itemService->readFeed($feedId, $highestItemId, $this->userId); + $this->itemService->readFeed($feedId, $highestItemId, $this->getUserId()); return [ 'feeds' => [ @@ -292,7 +291,7 @@ class FeedController extends Controller public function restore(int $feedId) { try { - $this->feedService->unmarkDeleted($feedId, $this->userId); + $this->feedService->unmarkDeleted($feedId, $this->getUserId()); } catch (ServiceNotFoundException $ex) { return $this->error($ex, Http::STATUS_NOT_FOUND); } @@ -339,7 +338,7 @@ class FeedController extends Controller ); try { - $this->feedService->patch($feedId, $this->userId, $diff); + $this->feedService->patch($feedId, $this->getUserId(), $diff); } catch (ServiceNotFoundException $ex) { return $this->error($ex, Http::STATUS_NOT_FOUND); } diff --git a/lib/Controller/FolderApiController.php b/lib/Controller/FolderApiController.php index c9a6b67b7..7f7f97525 100644 --- a/lib/Controller/FolderApiController.php +++ b/lib/Controller/FolderApiController.php @@ -72,7 +72,7 @@ class FolderApiController extends ApiController public function create(string $name) { try { - $this->folderService->purgeDeleted(); + $this->folderService->purgeDeleted($this->getUserId(), time() - 600); $folder = $this->folderService->create($this->getUserId(), $name); return ['folders' => $this->serialize($folder)]; } catch (ServiceValidationException $ex) { diff --git a/lib/Controller/FolderController.php b/lib/Controller/FolderController.php index 662d45029..c12c7042f 100644 --- a/lib/Controller/FolderController.php +++ b/lib/Controller/FolderController.php @@ -16,7 +16,6 @@ namespace OCA\News\Controller; use OCA\News\Service\Exceptions\ServiceException; use OCP\AppFramework\Http\JSONResponse; use \OCP\IRequest; -use \OCP\AppFramework\Controller; use \OCP\AppFramework\Http; use \OCA\News\Service\FolderServiceV2; @@ -24,7 +23,7 @@ use \OCA\News\Service\FeedService; use \OCA\News\Service\ItemService; use \OCA\News\Service\Exceptions\ServiceNotFoundException; use \OCA\News\Service\Exceptions\ServiceConflictException; -use OCP\IUser; +use OCP\IUserSession; class FolderController extends Controller { @@ -38,7 +37,6 @@ class FolderController extends Controller private $feedService; //TODO: Remove private $itemService; - private $userId; public function __construct( string $appName, @@ -46,13 +44,12 @@ class FolderController extends Controller FolderServiceV2 $folderService, FeedService $feedService, ItemService $itemService, - IUser $user + IUserSession $userSession ) { - parent::__construct($appName, $request); + parent::__construct($appName, $request, $userSession); $this->folderService = $folderService; $this->feedService = $feedService; $this->itemService = $itemService; - $this->userId = $user->getUID(); } @@ -61,7 +58,7 @@ class FolderController extends Controller */ public function index() { - $folders = $this->folderService->findAllForUser($this->userId); + $folders = $this->folderService->findAllForUser($this->getUserId()); return ['folders' => $this->serialize($folders)]; } @@ -79,7 +76,7 @@ class FolderController extends Controller $folderId = $folderId === 0 ? null : $folderId; try { - $this->folderService->open($this->userId, $folderId, $open); + $this->folderService->open($this->getUserId(), $folderId, $open); } catch (ServiceException $ex) { return $this->error($ex, Http::STATUS_NOT_FOUND); } @@ -98,8 +95,8 @@ class FolderController extends Controller */ public function create(string $folderName, ?int $parent = null) { - $this->folderService->purgeDeleted(); - $folder = $this->folderService->create($this->userId, $folderName, $parent); + $this->folderService->purgeDeleted($this->getUserId(), time() - 600); + $folder = $this->folderService->create($this->getUserId(), $folderName, $parent); return ['folders' => $this->serialize($folder)]; } @@ -118,7 +115,7 @@ class FolderController extends Controller return new JSONResponse([], Http::STATUS_BAD_REQUEST); } try { - $this->folderService->markDelete($this->userId, $folderId, true); + $this->folderService->markDelete($this->getUserId(), $folderId, true); } catch (ServiceNotFoundException $ex) { return $this->error($ex, Http::STATUS_NOT_FOUND); } catch (ServiceConflictException $ex) { @@ -143,7 +140,7 @@ class FolderController extends Controller return new JSONResponse([], Http::STATUS_BAD_REQUEST); } try { - $folder = $this->folderService->rename($this->userId, $folderId, $folderName); + $folder = $this->folderService->rename($this->getUserId(), $folderId, $folderName); return ['folders' => $this->serialize($folder)]; } catch (ServiceConflictException $ex) { @@ -168,9 +165,9 @@ class FolderController extends Controller $this->itemService->readFolder( $folderId, $highestItemId, - $this->userId + $this->getUserId() ); - $feeds = $this->feedService->findAllForUser($this->userId); + $feeds = $this->feedService->findAllForUser($this->getUserId()); return ['feeds' => $this->serialize($feeds)]; } @@ -187,7 +184,7 @@ class FolderController extends Controller $folderId = $folderId === 0 ? null : $folderId; try { - $this->folderService->markDelete($this->userId, $folderId, false); + $this->folderService->markDelete($this->getUserId(), $folderId, false); } catch (ServiceNotFoundException $ex) { return $this->error($ex, Http::STATUS_NOT_FOUND); } catch (ServiceConflictException $ex) { diff --git a/lib/Controller/ItemController.php b/lib/Controller/ItemController.php index 37d34e8ca..be4f707c9 100644 --- a/lib/Controller/ItemController.php +++ b/lib/Controller/ItemController.php @@ -15,13 +15,13 @@ namespace OCA\News\Controller; use \OCP\IRequest; use \OCP\IConfig; -use \OCP\AppFramework\Controller; use \OCP\AppFramework\Http; use \OCA\News\Service\Exceptions\ServiceException; use \OCA\News\Service\Exceptions\ServiceNotFoundException; use \OCA\News\Service\ItemService; use \OCA\News\Service\FeedService; +use OCP\IUserSession; class ItemController extends Controller { @@ -29,7 +29,6 @@ class ItemController extends Controller private $itemService; private $feedService; - private $userId; private $settings; public function __construct( @@ -38,12 +37,11 @@ class ItemController extends Controller FeedService $feedService, ItemService $itemService, IConfig $settings, - $UserId + IUserSession $userSession ) { - parent::__construct($appName, $request); + parent::__construct($appName, $request, $userSession); $this->itemService = $itemService; $this->feedService = $feedService; - $this->userId = $UserId; $this->settings = $settings; } @@ -74,7 +72,7 @@ class ItemController extends Controller // internal state if ($showAll === null) { $showAll = $this->settings->getUserValue( - $this->userId, + $this->getUserId(), $this->appName, 'showAll' ) === '1'; @@ -82,20 +80,20 @@ class ItemController extends Controller if ($oldestFirst === null) { $oldestFirst = $this->settings->getUserValue( - $this->userId, + $this->getUserId(), $this->appName, 'oldestFirst' ) === '1'; } $this->settings->setUserValue( - $this->userId, + $this->getUserId(), $this->appName, 'lastViewedFeedId', $id ); $this->settings->setUserValue( - $this->userId, + $this->getUserId(), $this->appName, 'lastViewedFeedType', $type @@ -118,10 +116,10 @@ class ItemController extends Controller // out of sync if ($offset === 0) { $params['newestItemId'] = - $this->itemService->getNewestItemId($this->userId); - $params['feeds'] = $this->feedService->findAllForUser($this->userId); + $this->itemService->getNewestItemId($this->getUserId()); + $params['feeds'] = $this->feedService->findAllForUser($this->getUserId()); $params['starred'] = - $this->itemService->starredCount($this->userId); + $this->itemService->starredCount($this->getUserId()); } $params['items'] = $this->itemService->findAllItems( @@ -131,7 +129,7 @@ class ItemController extends Controller $offset, $showAll, $oldestFirst, - $this->userId, + $this->getUserId(), $search ); @@ -155,7 +153,7 @@ class ItemController extends Controller public function newItems($type, $id, $lastModified = 0) { $showAll = $this->settings->getUserValue( - $this->userId, + $this->getUserId(), $this->appName, 'showAll' ) === '1'; @@ -164,16 +162,16 @@ class ItemController extends Controller try { $params['newestItemId'] = - $this->itemService->getNewestItemId($this->userId); - $params['feeds'] = $this->feedService->findAllForUser($this->userId); + $this->itemService->getNewestItemId($this->getUserId()); + $params['feeds'] = $this->feedService->findAllForUser($this->getUserId()); $params['starred'] = - $this->itemService->starredCount($this->userId); + $this->itemService->starredCount($this->getUserId()); $params['items'] = $this->itemService->findAllNew( $id, $type, $lastModified, $showAll, - $this->userId + $this->getUserId() ); // this gets thrown if there are no items @@ -200,7 +198,7 @@ class ItemController extends Controller $feedId, $guidHash, $isStarred, - $this->userId + $this->getUserId() ); } catch (ServiceException $ex) { return $this->error($ex, Http::STATUS_NOT_FOUND); @@ -220,7 +218,7 @@ class ItemController extends Controller public function read($itemId, $isRead = true) { try { - $this->itemService->read($itemId, $isRead, $this->userId); + $this->itemService->read($itemId, $isRead, $this->getUserId()); } catch (ServiceException $ex) { return $this->error($ex, Http::STATUS_NOT_FOUND); } @@ -237,8 +235,8 @@ class ItemController extends Controller */ public function readAll($highestItemId) { - $this->itemService->readAll($highestItemId, $this->userId); - return ['feeds' => $this->feedService->findAllForUser($this->userId)]; + $this->itemService->readAll($highestItemId, $this->getUserId()); + return ['feeds' => $this->feedService->findAllForUser($this->getUserId())]; } @@ -251,7 +249,7 @@ class ItemController extends Controller { foreach ($itemIds as $id) { try { - $this->itemService->read($id, true, $this->userId); + $this->itemService->read($id, true, $this->getUserId()); } catch (ServiceNotFoundException $ex) { continue; } diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index ccabd16db..8e55e69ee 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -18,9 +18,7 @@ use OCP\IRequest; use OCP\IConfig; use OCP\IL10N; use OCP\IURLGenerator; -use OCP\AppFramework\Controller; use OCP\AppFramework\Http\TemplateResponse; -use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http; use OCP\AppFramework\Http\ContentSecurityPolicy; @@ -28,6 +26,7 @@ use OCA\News\Service\StatusService; use OCA\News\Explore\RecommendedSites; use OCA\News\Explore\RecommendedSiteNotFoundException; use OCA\News\Db\FeedType; +use OCP\IUserSession; class PageController extends Controller { @@ -43,11 +42,6 @@ class PageController extends Controller */ private $l10n; - /** - * @var string - */ - private $userId; - /** * @var IURLGenerator */ @@ -71,13 +65,12 @@ class PageController extends Controller IL10N $l10n, RecommendedSites $recommendedSites, StatusService $statusService, - ?string $UserId + IUserSession $userSession ) { - parent::__construct($appName, $request); + parent::__construct($appName, $request, $userSession); $this->settings = $settings; $this->urlGenerator = $urlGenerator; $this->l10n = $l10n; - $this->userId = $UserId; $this->recommendedSites = $recommendedSites; $this->statusService = $statusService; } @@ -149,7 +142,7 @@ class PageController extends Controller foreach ($settings as $setting) { $result[$setting] = $this->settings->getUserValue( - $this->userId, + $this->getUserId(), $this->appName, $setting ) === '1'; @@ -185,7 +178,7 @@ class PageController extends Controller foreach ($settings as $setting => $value) { $value = $value ? '1' : '0'; $this->settings->setUserValue( - $this->userId, + $this->getUserId(), $this->appName, $setting, $value @@ -201,13 +194,13 @@ class PageController extends Controller public function explore(string $lang) { $this->settings->setUserValue( - $this->userId, + $this->getUserId(), $this->appName, 'lastViewedFeedId', 0 ); $this->settings->setUserValue( - $this->userId, + $this->getUserId(), $this->appName, 'lastViewedFeedType', FeedType::EXPLORE diff --git a/lib/Db/ItemMapperV2.php b/lib/Db/ItemMapperV2.php index 4f6f82413..4a301fca2 100644 --- a/lib/Db/ItemMapperV2.php +++ b/lib/Db/ItemMapperV2.php @@ -149,4 +149,15 @@ class ItemMapperV2 extends NewsMapperV2 return $this->db->executeQuery($query)->fetch(FetchMode::ASSOCIATIVE); } + + /** + * No-op clear deleted items. + * + * @param string|null $userID + * @param int|null $oldestDelete + */ + public function purgeDeleted(?string $userID, ?int $oldestDelete): void + { + //NO-OP + } } diff --git a/lib/Db/NewsMapperV2.php b/lib/Db/NewsMapperV2.php index 5d1905151..cbfb84fde 100644 --- a/lib/Db/NewsMapperV2.php +++ b/lib/Db/NewsMapperV2.php @@ -64,16 +64,30 @@ abstract class NewsMapperV2 extends QBMapper } /** - * Remove deleted items. + * Remove deleted entities. + * + * @param string|null $userID The user to purge + * @param int|null $oldestDelete The timestamp to purge from * * @return void */ - public function purgeDeleted(): void + public function purgeDeleted(?string $userID, ?int $oldestDelete): void { $builder = $this->db->getQueryBuilder(); $builder->delete($this->tableName) - ->where('deleted_at != 0') - ->execute(); + ->andWhere('deleted_at != 0'); + + if ($userID !== null) { + $builder->andWhere('user_id = :user_id') + ->setParameter(':user_id', $userID); + } + + if ($oldestDelete !== null) { + $builder->andWhere('deleted_at < :deleted_at') + ->setParameter(':deleted_at', $oldestDelete); + } + + $builder->execute(); } /** diff --git a/lib/Service/FeedServiceV2.php b/lib/Service/FeedServiceV2.php index 94878d5fe..16402d0da 100644 --- a/lib/Service/FeedServiceV2.php +++ b/lib/Service/FeedServiceV2.php @@ -328,9 +328,17 @@ class FeedServiceV2 extends Service $this->mapper->delete($feed); } - public function purgeDeleted(): void + /** + * Remove deleted entities. + * + * @param string|null $userID The user to purge + * @param int|null $minTimestamp The timestamp to purge from + * + * @return void + */ + public function purgeDeleted(?string $userID, ?int $minTimestamp): void { - $this->mapper->purgeDeleted(); + $this->mapper->purgeDeleted($userID, $minTimestamp); } public function fetchAll(): void diff --git a/lib/Service/FolderServiceV2.php b/lib/Service/FolderServiceV2.php index 95761b530..ae04c8089 100644 --- a/lib/Service/FolderServiceV2.php +++ b/lib/Service/FolderServiceV2.php @@ -152,10 +152,13 @@ class FolderServiceV2 extends Service /** * Purge all deleted folders. + * + * @param string|null $userID The user to purge + * @param int|null $minTimestamp The timestamp to purge from */ - public function purgeDeleted() + public function purgeDeleted(?string $userID, ?int $minTimestamp) { - $this->mapper->purgeDeleted(); + $this->mapper->purgeDeleted($userID, $minTimestamp); } /** diff --git a/lib/Service/UpdaterService.php b/lib/Service/UpdaterService.php index 7f1b04ddc..bc7c9f562 100644 --- a/lib/Service/UpdaterService.php +++ b/lib/Service/UpdaterService.php @@ -45,8 +45,8 @@ class UpdaterService public function beforeUpdate() { - $this->folderService->purgeDeleted(); - $this->feedService->purgeDeleted(); + $this->folderService->purgeDeleted(null, null); + $this->feedService->purgeDeleted(null, null); } diff --git a/tests/Integration/IntegrationTest.php b/tests/Integration/IntegrationTest.php index bc309b339..7c9acf170 100644 --- a/tests/Integration/IntegrationTest.php +++ b/tests/Integration/IntegrationTest.php @@ -26,7 +26,6 @@ use OCA\News\Tests\Integration\Fixtures\FeedFixture; use OCA\News\Tests\Integration\Fixtures\FolderFixture; use OCA\News\Db\FeedMapper; use OCA\News\Db\ItemMapper; -use OCA\News\Db\FolderMapper; abstract class IntegrationTest extends \Test\TestCase { diff --git a/tests/Unit/Controller/ExportControllerTest.php b/tests/Unit/Controller/ExportControllerTest.php index 563bd987c..276a36abf 100644 --- a/tests/Unit/Controller/ExportControllerTest.php +++ b/tests/Unit/Controller/ExportControllerTest.php @@ -25,6 +25,8 @@ use \OCA\News\Db\Feed; use OCP\AppFramework\Http\DataDownloadResponse; use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; use PHPUnit\Framework\TestCase; class ExportControllerTest extends TestCase @@ -32,6 +34,10 @@ class ExportControllerTest extends TestCase private $controller; private $user; + /** + * @var \PHPUnit\Framework\MockObject\MockObject|IUserSession + */ + private $userSession; /** * @var \PHPUnit\Framework\MockObject\MockObject|FeedServiceV2 */ @@ -55,7 +61,6 @@ class ExportControllerTest extends TestCase public function setUp(): void { $appName = 'news'; - $this->user = 'john'; $this->itemService = $this->getMockBuilder(ItemServiceV2::class) ->disableOriginalConstructor() ->getMock(); @@ -68,6 +73,15 @@ class ExportControllerTest extends TestCase $this->opmlService = $this->getMockBuilder(OpmlService::class) ->disableOriginalConstructor() ->getMock(); + $this->user = $this->getMockBuilder(IUser::class)->getMock(); + $this->user->expects($this->any()) + ->method('getUID') + ->will($this->returnValue('user')); + $this->userSession = $this->getMockBuilder(IUserSession::class) + ->getMock(); + $this->userSession->expects($this->any()) + ->method('getUser') + ->will($this->returnValue($this->user)); $request = $this->getMockBuilder(IRequest::class) ->disableOriginalConstructor() ->getMock(); @@ -78,7 +92,7 @@ class ExportControllerTest extends TestCase $this->feedService, $this->itemService, $this->opmlService, - $this->user + $this->userSession ); } @@ -96,7 +110,7 @@ class ExportControllerTest extends TestCase $this->opmlService->expects($this->once()) ->method('export') - ->with($this->user) + ->with('user') ->will($this->returnValue($opml)); $return = $this->controller->opml(); @@ -126,11 +140,11 @@ class ExportControllerTest extends TestCase $this->feedService->expects($this->once()) ->method('findAllForUser') - ->with($this->user) + ->with('user') ->will($this->returnValue($feeds)); $this->itemService->expects($this->once()) ->method('findAllForUser') - ->with($this->user, ['unread' => true, 'starred' => true]) + ->with('user', ['unread' => true, 'starred' => true]) ->will($this->returnValue($articles)); diff --git a/tests/Unit/Controller/FeedApiControllerTest.php b/tests/Unit/Controller/FeedApiControllerTest.php index ace481a66..0ed65f5c5 100644 --- a/tests/Unit/Controller/FeedApiControllerTest.php +++ b/tests/Unit/Controller/FeedApiControllerTest.php @@ -213,9 +213,10 @@ class FeedApiControllerTest extends TestCase $this->assertEquals( [ - 'feeds' => [$feeds[0]->toAPI()], - 'newestItemId' => 3 - ], $response + 'feeds' => [$feeds[0]->toAPI()], + 'newestItemId' => 3 + ], + $response ); } diff --git a/tests/Unit/Controller/FeedControllerTest.php b/tests/Unit/Controller/FeedControllerTest.php index fb5d0b5b5..6285be9dc 100644 --- a/tests/Unit/Controller/FeedControllerTest.php +++ b/tests/Unit/Controller/FeedControllerTest.php @@ -27,6 +27,7 @@ use OCP\IConfig; use OCP\IRequest; use OCP\IUser; +use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -77,12 +78,6 @@ class FeedControllerTest extends TestCase { $this->appName = 'news'; $this->uid = 'jack'; - $this->user = $this->getMockBuilder(IUser::class) - ->getMock(); - - $this->user->expects($this->once()) - ->method('getUID') - ->willReturn($this->uid); $this->settings = $this->getMockBuilder(IConfig::class) ->disableOriginalConstructor() ->getMock(); @@ -98,6 +93,15 @@ class FeedControllerTest extends TestCase ->getMockBuilder(FolderServiceV2::class) ->disableOriginalConstructor() ->getMock(); + $this->user = $this->getMockBuilder(IUser::class)->getMock(); + $this->user->expects($this->any()) + ->method('getUID') + ->will($this->returnValue($this->uid)); + $this->userSession = $this->getMockBuilder(IUserSession::class) + ->getMock(); + $this->userSession->expects($this->any()) + ->method('getUser') + ->will($this->returnValue($this->user)); $request = $this->getMockBuilder(IRequest::class) ->disableOriginalConstructor() ->getMock(); @@ -108,7 +112,7 @@ class FeedControllerTest extends TestCase $this->feedService, $this->itemService, $this->settings, - $this->user + $this->userSession ); $this->exampleResult = [ 'activeFeed' => [ diff --git a/tests/Unit/Controller/FolderControllerTest.php b/tests/Unit/Controller/FolderControllerTest.php index d0216c70f..ef1b1e965 100644 --- a/tests/Unit/Controller/FolderControllerTest.php +++ b/tests/Unit/Controller/FolderControllerTest.php @@ -27,6 +27,7 @@ use \OCA\News\Service\Exceptions\ServiceValidationException; use OCP\IRequest; use OCP\IUser; +use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -43,6 +44,7 @@ class FolderControllerTest extends TestCase * @var MockObject|IUser */ private $user; + private $userSession; private $class; private $msg; @@ -55,9 +57,6 @@ class FolderControllerTest extends TestCase $appName = 'news'; $this->user = $this->getMockBuilder(IUser::class) ->getMock(); - $this->user->expects($this->once()) - ->method('getUID') - ->willReturn('jack'); $this->folderService = $this->getMockBuilder(FolderServiceV2::class) ->disableOriginalConstructor() ->getMock(); @@ -70,13 +69,22 @@ class FolderControllerTest extends TestCase $request = $this->getMockBuilder(IRequest::class) ->disableOriginalConstructor() ->getMock(); + $this->user = $this->getMockBuilder(IUser::class)->getMock(); + $this->user->expects($this->any()) + ->method('getUID') + ->will($this->returnValue('jack')); + $this->userSession = $this->getMockBuilder(IUserSession::class) + ->getMock(); + $this->userSession->expects($this->any()) + ->method('getUser') + ->will($this->returnValue($this->user)); $this->class = new FolderController( $appName, $request, $this->folderService, $this->feedService, $this->itemService, - $this->user + $this->userSession ); $this->msg = 'ron'; } diff --git a/tests/Unit/Controller/ItemControllerTest.php b/tests/Unit/Controller/ItemControllerTest.php index c015c33a1..9929cde23 100644 --- a/tests/Unit/Controller/ItemControllerTest.php +++ b/tests/Unit/Controller/ItemControllerTest.php @@ -25,6 +25,8 @@ use \OCA\News\Service\Exceptions\ServiceNotFoundException; use OCP\IConfig; use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; use PHPUnit\Framework\TestCase; @@ -36,7 +38,14 @@ class ItemControllerTest extends TestCase private $itemService; private $feedService; private $request; + /** + * @var \PHPUnit\Framework\MockObject\MockObject|IUser + */ private $user; + /** + * @var \PHPUnit\Framework\MockObject\MockObject|IUserSession + */ + private $userSession; private $controller; private $newestItemId; @@ -47,7 +56,6 @@ class ItemControllerTest extends TestCase public function setUp(): void { $this->appName = 'news'; - $this->user = 'jackob'; $this->settings = $this->getMockBuilder(IConfig::class) ->disableOriginalConstructor() ->getMock(); @@ -62,10 +70,22 @@ class ItemControllerTest extends TestCase $this->request = $this->getMockBuilder(IRequest::class) ->disableOriginalConstructor() ->getMock(); + $this->user = $this->getMockBuilder(IUser::class)->getMock(); + $this->user->expects($this->any()) + ->method('getUID') + ->will($this->returnValue('user')); + $this->userSession = $this->getMockBuilder(IUserSession::class) + ->getMock(); + $this->userSession->expects($this->any()) + ->method('getUser') + ->will($this->returnValue($this->user)); $this->controller = new ItemController( - $this->appName, $this->request, - $this->feedService, $this->itemService, $this->settings, - $this->user + $this->appName, + $this->request, + $this->feedService, + $this->itemService, + $this->settings, + $this->userSession ); $this->newestItemId = 12312; } @@ -75,7 +95,7 @@ class ItemControllerTest extends TestCase { $this->itemService->expects($this->once()) ->method('read') - ->with(4, true, $this->user); + ->with(4, true, 'user'); $this->controller->read(4, true); } @@ -92,7 +112,7 @@ class ItemControllerTest extends TestCase $response = $this->controller->read(4); $params = json_decode($response->render(), true); - $this->assertEquals($response->getStatus(), Http::STATUS_NOT_FOUND); + $this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus()); $this->assertEquals($msg, $params['message']); } @@ -102,8 +122,8 @@ class ItemControllerTest extends TestCase $this->itemService->expects($this->exactly(2)) ->method('read') ->withConsecutive( - [2, true, $this->user], - [4, true, $this->user] + [2, true, 'user'], + [4, true, 'user'] ); $this->controller->readMultiple([2, 4]); @@ -116,8 +136,8 @@ class ItemControllerTest extends TestCase $this->itemService->expects($this->exactly(2)) ->method('read') ->withConsecutive( - [2, true, $this->user], - [4, true, $this->user] + [2, true, 'user'], + [4, true, 'user'] ) ->willReturnOnConsecutiveCalls($this->throwException(new ServiceNotFoundException('yo')), null); $this->controller->readMultiple([2, 4]); @@ -128,12 +148,7 @@ class ItemControllerTest extends TestCase { $this->itemService->expects($this->once()) ->method('star') - ->with( - $this->equalTo(4), - $this->equalTo('test'), - $this->equalTo(true), - $this->equalTo($this->user) - ); + ->with(4, 'test', true, 'user'); $this->controller->star(4, 'test', true); } @@ -150,7 +165,7 @@ class ItemControllerTest extends TestCase $response = $this->controller->star(4, 'test', false); $params = json_decode($response->render(), true); - $this->assertEquals($response->getStatus(), Http::STATUS_NOT_FOUND); + $this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus()); $this->assertEquals($msg, $params['message']); } @@ -163,13 +178,10 @@ class ItemControllerTest extends TestCase $this->itemService->expects($this->once()) ->method('readAll') - ->with( - $this->equalTo(5), - $this->equalTo($this->user) - ); + ->with(5, 'user'); $this->feedService->expects($this->once()) ->method('findAllForUser') - ->with($this->equalTo($this->user)) + ->with('user') ->will($this->returnValue([$feed])); $response = $this->controller->readAll(5); @@ -182,15 +194,15 @@ class ItemControllerTest extends TestCase $this->settings->expects($this->exactly(2)) ->method('getUserValue') ->withConsecutive( - [$this->user, $this->appName, 'showAll'], - [$this->user, $this->appName, 'oldestFirst'] + ['user', $this->appName, 'showAll'], + ['user', $this->appName, 'oldestFirst'] ) ->willReturnOnConsecutiveCalls('1', $oldestFirst); $this->settings->expects($this->exactly(2)) ->method('setUserValue') ->withConsecutive( - [$this->user, $this->appName, 'lastViewedFeedId', $id], - [$this->user, $this->appName, 'lastViewedFeedType', $type] + ['user', $this->appName, 'lastViewedFeedId', $id], + ['user', $this->appName, 'lastViewedFeedType', $type] ); } @@ -209,22 +221,22 @@ class ItemControllerTest extends TestCase $this->feedService->expects($this->once()) ->method('findAllForUser') - ->with($this->equalTo($this->user)) + ->with('user') ->will($this->returnValue($feeds)); $this->itemService->expects($this->once()) ->method('getNewestItemId') - ->with($this->equalTo($this->user)) + ->with('user') ->will($this->returnValue($this->newestItemId)); $this->itemService->expects($this->once()) ->method('starredCount') - ->with($this->equalTo($this->user)) + ->with('user') ->will($this->returnValue(3111)); $this->itemService->expects($this->once()) ->method('findAllItems') - ->with(2, FeedType::FEED, 3, 0, true, false, $this->user, []) + ->with(2, FeedType::FEED, 3, 0, true, false, 'user', []) ->will($this->returnValue($result['items'])); $response = $this->controller->index(FeedType::FEED, 2, 3); @@ -246,37 +258,25 @@ class ItemControllerTest extends TestCase $this->feedService->expects($this->once()) ->method('findAllForUser') - ->with($this->equalTo($this->user)) + ->with('user') ->will($this->returnValue($feeds)); $this->itemService->expects($this->once()) ->method('getNewestItemId') - ->with($this->equalTo($this->user)) + ->with('user') ->will($this->returnValue($this->newestItemId)); $this->itemService->expects($this->once()) ->method('starredCount') - ->with($this->equalTo($this->user)) + ->with('user') ->will($this->returnValue(3111)); $this->itemService->expects($this->once()) ->method('findAllItems') - ->with( - $this->equalTo(2), - $this->equalTo(FeedType::FEED), - $this->equalTo(3), - $this->equalTo(0), - $this->equalTo(true), - $this->equalTo(false), - $this->equalTo($this->user), - $this->equalTo(['test', 'search']) - ) + ->with(2, FeedType::FEED, 3, 0, true, false, 'user', ['test', 'search']) ->will($this->returnValue($result['items'])); - $response = $this->controller->index( - FeedType::FEED, 2, 3, - 0, null, null, 'test%20%20search%20' - ); + $response = $this->controller->index(FeedType::FEED, 2, 3, 0, null, null, 'test%20%20search%20'); $this->assertEquals($result, $response); } @@ -289,15 +289,7 @@ class ItemControllerTest extends TestCase $this->itemService->expects($this->once()) ->method('findAllItems') - ->with( - $this->equalTo(2), - $this->equalTo(FeedType::FEED), - $this->equalTo(3), - $this->equalTo(10), - $this->equalTo(true), - $this->equalTo(true), - $this->equalTo($this->user) - ) + ->with(2, FeedType::FEED, 3, 10, true, true, 'user') ->will($this->returnValue($result['items'])); $this->feedService->expects($this->never()) @@ -314,7 +306,7 @@ class ItemControllerTest extends TestCase $this->itemService->expects($this->once()) ->method('getNewestItemId') - ->with($this->equalTo($this->user)) + ->with('user') ->will($this->throwException(new ServiceNotFoundException(''))); $response = $this->controller->index(FeedType::FEED, 2, 3); @@ -334,37 +326,27 @@ class ItemControllerTest extends TestCase $this->settings->expects($this->once()) ->method('getUserValue') - ->with( - $this->equalTo($this->user), - $this->equalTo($this->appName), - $this->equalTo('showAll') - ) + ->with('user', $this->appName, 'showAll') ->will($this->returnValue('1')); $this->feedService->expects($this->once()) ->method('findAllForUser') - ->with($this->equalTo($this->user)) + ->with('user') ->will($this->returnValue($feeds)); $this->itemService->expects($this->once()) ->method('getNewestItemId') - ->with($this->equalTo($this->user)) + ->with('user') ->will($this->returnValue($this->newestItemId)); $this->itemService->expects($this->once()) ->method('starredCount') - ->with($this->equalTo($this->user)) + ->with('user') ->will($this->returnValue(3111)); $this->itemService->expects($this->once()) ->method('findAllNew') - ->with( - $this->equalTo(2), - $this->equalTo(FeedType::FEED), - $this->equalTo(3), - $this->equalTo(true), - $this->equalTo($this->user) - ) + ->with(2, FeedType::FEED, 3, true, 'user') ->will($this->returnValue($result['items'])); $response = $this->controller->newItems(FeedType::FEED, 2, 3); @@ -376,16 +358,12 @@ class ItemControllerTest extends TestCase { $this->settings->expects($this->once()) ->method('getUserValue') - ->with( - $this->equalTo($this->user), - $this->equalTo($this->appName), - $this->equalTo('showAll') - ) + ->with('user', $this->appName, 'showAll') ->will($this->returnValue('1')); $this->itemService->expects($this->once()) ->method('getNewestItemId') - ->with($this->equalTo($this->user)) + ->with('user') ->will($this->throwException(new ServiceNotFoundException(''))); $response = $this->controller->newItems(FeedType::FEED, 2, 3); diff --git a/tests/Unit/Controller/PageControllerTest.php b/tests/Unit/Controller/PageControllerTest.php index 2afe2d590..0c642fe29 100644 --- a/tests/Unit/Controller/PageControllerTest.php +++ b/tests/Unit/Controller/PageControllerTest.php @@ -22,6 +22,8 @@ use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; use OCP\IURLGenerator; +use OCP\IUser; +use OCP\IUserSession; use PHPUnit\Framework\TestCase; @@ -101,6 +103,15 @@ class PageControllerTest extends TestCase $this->status = $this->getMockBuilder(StatusService::class) ->disableOriginalConstructor() ->getMock(); + $this->user = $this->getMockBuilder(IUser::class)->getMock(); + $this->user->expects($this->any()) + ->method('getUID') + ->will($this->returnValue('becka')); + $this->userSession = $this->getMockBuilder(IUserSession::class) + ->getMock(); + $this->userSession->expects($this->any()) + ->method('getUser') + ->will($this->returnValue($this->user)); $this->controller = new PageController( 'news', $this->request, @@ -109,7 +120,7 @@ class PageControllerTest extends TestCase $this->l10n, $this->recommended, $this->status, - 'becka' + $this->userSession ); } diff --git a/tests/Unit/Service/FolderServiceTest.php b/tests/Unit/Service/FolderServiceTest.php index 69934dd5b..d616da736 100644 --- a/tests/Unit/Service/FolderServiceTest.php +++ b/tests/Unit/Service/FolderServiceTest.php @@ -14,22 +14,15 @@ namespace OCA\News\Tests\Unit\Service; use OC\AppFramework\Utility\TimeFactory; -use OC\L10N\L10N; use OCA\News\Db\Feed; use \OCA\News\Db\Folder; -use OCA\News\Db\FolderMapper; use OCA\News\Db\FolderMapperV2; use OCA\News\Service\Exceptions\ServiceNotFoundException; use OCA\News\Service\FeedServiceV2; -use OCA\News\Service\FolderService; use OCA\News\Service\Exceptions\ServiceConflictException; -use OCA\News\Service\Exceptions\ServiceValidationException; use OCA\News\Service\FolderServiceV2; -use OCA\News\Utility\Time; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; -use OCP\IConfig; -use OCP\IL10N; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -271,9 +264,10 @@ class FolderServiceTest extends TestCase public function testPurgeDeleted() { $this->mapper->expects($this->exactly(1)) - ->method('purgeDeleted'); + ->method('purgeDeleted') + ->with('jack', null); - $this->class->purgeDeleted(); + $this->class->purgeDeleted('jack', null); } public function testDelete() -- cgit v1.2.3