Fix mapper->find and empty user sessions

Signed-off-by: Sean Molenaar <sean@seanmolenaar.eu>
This commit is contained in:
Sean Molenaar 2020-12-16 22:30:19 +01:00 committed by Sean Molenaar
parent 7bebaab86e
commit 11f5904dd5
30 changed files with 101 additions and 87 deletions

View File

@ -9,8 +9,8 @@
* [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)
* [anoy](mailto:anoymouserver+github@mailbox.org)
* [Davide Saurino](mailto:davide.saurino@alcacoop.it)
* [raghunayyar](mailto:me@iraghu.com)
* [bastei](mailto:bastei@users.noreply.github.com)
* [Bernhard Posselt](mailto:bep@foryouandyourcustomers.com)
@ -95,6 +95,7 @@
* [Hanzei](mailto:Hanzei@users.noreply.github.com)
* [Hendrik Leppelsack](mailto:hendrik@leppelsack.de)
* [Jasper Knockaert](mailto:jasper@knockaert.nl)
* [Kevin Decherf](mailto:kevin@kdecherf.com)
* [Maceček Richard](mailto:46937538+macecekrichard@users.noreply.github.com)
* [Marc Cousin](mailto:marc.cousin@people-doc.com)
* [Martin Ferretti](mailto:ferrettimartin@protonmail.com)
@ -132,6 +133,7 @@
* [bjoerns1983](mailto:bjoern@sengotta.net)
* [blackcrack](mailto:blackcrack@blackysgate.de)
* [comradekingu](mailto:epost@anotheragency.no)
* [derritter88](mailto:derritter88@users.noreply.github.com)
* [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)
@ -143,6 +145,7 @@
* [ritchiewilson](mailto:rawilson52@gmail.com)
* [sonologic](mailto:gmc@sonologic.nl)
* [tflidd](mailto:tflidd@aspekte.net)
* [waffshappen](mailto:44290023+waffshappen@users.noreply.github.com)
* [wizdude](mailto:wizdude@users.noreply.github.com)
* [zero77](mailto:zero77@users.noreply.github.com)
* [Éloi Rivard](mailto:eloi.rivard@aquilenet.fr)

View File

@ -3,12 +3,16 @@ All notable changes to this project will be documented in this file.
## Unreleased
### Fixed
- Fix #985
- Fix #981
## 15.1.1-rc1
## Changed
### Changed
- Remove outdated folder DB code
## Fixed
### Fixed
- Export Unread/Starred Articles gives Error Message #963
- Some events don't appear in feed #921

View File

@ -36,13 +36,12 @@ class AdminController extends Controller
/**
* AdminController constructor.
*
* @param string $appName The name of the app
* @param IRequest $request The request
* @param IConfig $config Config for nextcloud
*/
public function __construct(string $appName, IRequest $request, IConfig $config)
public function __construct(IRequest $request, IConfig $config)
{
parent::__construct($appName, $request);
parent::__construct(Application::NAME, $request);
$this->config = $config;
}

View File

@ -15,8 +15,10 @@
namespace OCA\News\Controller;
use OCA\News\AppInfo\Application;
use OCA\News\Controller\Exceptions\NotLoggedInException;
use \OCP\IUser;
use \OCP\IRequest;
use OCP\IUser;
use \OCP\IUserSession;
use \OCP\AppFramework\ApiController as BaseApiController;
@ -28,7 +30,7 @@ use \OCP\AppFramework\ApiController as BaseApiController;
class ApiController extends BaseApiController
{
/**
* @var IUserSession
* @var IUserSession|null
*/
private $userSession;
@ -37,13 +39,12 @@ class ApiController extends BaseApiController
*
* 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
* @param IRequest $request The request
* @param IUserSession|null $userSession The user session
*/
public function __construct(string $appName, IRequest $request, IUserSession $userSession)
public function __construct(IRequest $request, ?IUserSession $userSession)
{
parent::__construct($appName, $request);
parent::__construct(Application::NAME, $request);
$this->userSession = $userSession;
}
@ -52,6 +53,10 @@ class ApiController extends BaseApiController
*/
protected function getUser()
{
if ($this->userSession === null) {
throw new NotLoggedInException();
}
return $this->userSession->getUser();
}

View File

@ -13,9 +13,11 @@
namespace OCA\News\Controller;
use OCA\News\AppInfo\Application;
use OCA\News\Controller\Exceptions\NotLoggedInException;
use OCP\AppFramework\Controller as BaseController;
use \OCP\IUser;
use \OCP\IRequest;
use OCP\IUser;
use \OCP\IUserSession;
/**
@ -26,7 +28,7 @@ use \OCP\IUserSession;
class Controller extends BaseController
{
/**
* @var IUserSession
* @var IUserSession|null
*/
private $userSession;
@ -35,13 +37,13 @@ class Controller extends BaseController
*
* 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
* @param IRequest $request The request
* @param IUserSession|null $userSession The user session
*/
public function __construct(string $appName, IRequest $request, IUserSession $userSession)
public function __construct(IRequest $request, ?IUserSession $userSession)
{
parent::__construct($appName, $request);
parent::__construct(Application::NAME, $request);
$this->userSession = $userSession;
}
@ -50,6 +52,10 @@ class Controller extends BaseController
*/
protected function getUser()
{
if ($this->userSession === null) {
throw new NotLoggedInException();
}
return $this->userSession->getUser();
}

View File

@ -0,0 +1,14 @@
<?php
namespace OCA\News\Controller\Exceptions;
use Throwable;
class NotLoggedInException extends \Exception
{
public function __construct(?string $message = null)
{
parent::__construct($message ?? 'Unauthorized: User is not logged in!', 0, null);
}
}

View File

@ -31,15 +31,14 @@ class ExportController extends Controller
private $itemService;
public function __construct(
string $appName,
IRequest $request,
FolderServiceV2 $folderService,
FeedServiceV2 $feedService,
ItemServiceV2 $itemService,
OpmlService $opmlService,
IUserSession $userSession
?IUserSession $userSession
) {
parent::__construct($appName, $request, $userSession);
parent::__construct($request, $userSession);
$this->feedService = $feedService;
$this->folderService = $folderService;
$this->opmlService = $opmlService;

View File

@ -16,6 +16,7 @@
namespace OCA\News\Controller;
use Exception;
use OCA\News\AppInfo\Application;
use OCA\News\Service\Exceptions\ServiceConflictException;
use OCA\News\Service\Exceptions\ServiceNotFoundException;
use OCA\News\Service\FeedServiceV2;
@ -61,15 +62,14 @@ class FeedApiController extends ApiController
private $serializer;
public function __construct(
string $appName,
IRequest $request,
IUserSession $userSession,
?IUserSession $userSession,
FeedService $oldFeedService,
FeedServiceV2 $feedService,
ItemService $oldItemService,
LoggerInterface $logger
) {
parent::__construct($appName, $request, $userSession);
parent::__construct($request, $userSession);
$this->feedService = $feedService;
$this->oldFeedService = $oldFeedService;
$this->oldItemService = $oldItemService;

View File

@ -44,15 +44,14 @@ class FeedController extends Controller
private $settings;
public function __construct(
string $appName,
IRequest $request,
FolderServiceV2 $folderService,
FeedService $feedService,
ItemService $itemService,
IConfig $settings,
IUserSession $userSession
?IUserSession $userSession
) {
parent::__construct($appName, $request, $userSession);
parent::__construct($request, $userSession);
$this->folderService = $folderService;
$this->feedService = $feedService;
$this->itemService = $itemService;

View File

@ -35,13 +35,12 @@ class FolderApiController extends ApiController
private $itemService;
public function __construct(
string $appName,
IRequest $request,
IUserSession $userSession,
?IUserSession $userSession,
FolderServiceV2 $folderService,
ItemService $itemService
) {
parent::__construct($appName, $request, $userSession);
parent::__construct($request, $userSession);
$this->folderService = $folderService;
$this->itemService = $itemService;

View File

@ -39,14 +39,13 @@ class FolderController extends Controller
private $itemService;
public function __construct(
string $appName,
IRequest $request,
FolderServiceV2 $folderService,
FeedService $feedService,
ItemService $itemService,
IUserSession $userSession
?IUserSession $userSession
) {
parent::__construct($appName, $request, $userSession);
parent::__construct($request, $userSession);
$this->folderService = $folderService;
$this->feedService = $feedService;
$this->itemService = $itemService;

View File

@ -32,13 +32,12 @@ class ItemApiController extends ApiController
private $itemService;
public function __construct(
string $appName,
IRequest $request,
IUserSession $userSession,
?IUserSession $userSession,
ItemService $oldItemService,
ItemServiceV2 $itemService
) {
parent::__construct($appName, $request, $userSession);
parent::__construct($request, $userSession);
$this->oldItemService = $oldItemService;
$this->itemService = $itemService;

View File

@ -32,14 +32,13 @@ class ItemController extends Controller
private $settings;
public function __construct(
$appName,
IRequest $request,
FeedService $feedService,
ItemService $itemService,
IConfig $settings,
IUserSession $userSession
?IUserSession $userSession
) {
parent::__construct($appName, $request, $userSession);
parent::__construct($request, $userSession);
$this->itemService = $itemService;
$this->feedService = $feedService;
$this->settings = $settings;

View File

@ -58,16 +58,15 @@ class PageController extends Controller
private $statusService;
public function __construct(
string $appName,
IRequest $request,
IConfig $settings,
IURLGenerator $urlGenerator,
IL10N $l10n,
RecommendedSites $recommendedSites,
StatusService $statusService,
IUserSession $userSession
?IUserSession $userSession
) {
parent::__construct($appName, $request, $userSession);
parent::__construct($request, $userSession);
$this->settings = $settings;
$this->urlGenerator = $urlGenerator;
$this->l10n = $l10n;

View File

@ -24,11 +24,10 @@ use \OCP\AppFramework\Http;
class UserApiController extends ApiController
{
public function __construct(
string $appName,
IRequest $request,
IUserSession $userSession
?IUserSession $userSession
) {
parent::__construct($appName, $request, $userSession);
parent::__construct($request, $userSession);
}
/**

View File

@ -30,14 +30,13 @@ class UtilityApiController extends ApiController
private $statusService;
public function __construct(
string $appName,
IRequest $request,
IUserSession $userSession,
?IUserSession $userSession,
UpdaterService $updater,
IConfig $settings,
StatusService $statusService
) {
parent::__construct($appName, $request, $userSession);
parent::__construct($request, $userSession);
$this->updaterService = $updater;
$this->settings = $settings;
$this->statusService = $statusService;

View File

@ -40,8 +40,8 @@ abstract class Service
/**
* Service constructor.
*
* @param NewsMapperV2 $mapper
* @param LoggerInterface $logger
* @param NewsMapperV2 $mapper
* @param LoggerInterface $logger
*/
public function __construct($mapper, LoggerInterface $logger)
{
@ -97,7 +97,7 @@ abstract class Service
public function find(string $userId, int $id): Entity
{
try {
return $this->mapper->find($userId, $id);
return $this->mapper->findFromUser($userId, $id);
} catch (DoesNotExistException $ex) {
throw new ServiceNotFoundException($ex->getMessage());
} catch (MultipleObjectsReturnedException $ex) {

View File

@ -62,7 +62,7 @@ class AdminControllerTest extends TestCase
->disableOriginalConstructor()
->getMock();
$this->controller = new AdminController($this->appName, $this->request, $this->config, $this->itemService);
$this->controller = new AdminController($this->request, $this->config, $this->itemService);
}
/**

View File

@ -86,7 +86,6 @@ class ExportControllerTest extends TestCase
->disableOriginalConstructor()
->getMock();
$this->controller = new ExportController(
$appName,
$request,
$this->folderService,
$this->feedService,

View File

@ -91,7 +91,6 @@ class FeedApiControllerTest extends TestCase
->disableOriginalConstructor()
->getMock();
$this->class = new FeedApiController(
$appName,
$request,
$userSession,
$this->oldFeedService,

View File

@ -106,7 +106,6 @@ class FeedControllerTest extends TestCase
->disableOriginalConstructor()
->getMock();
$this->class = new FeedController(
$this->appName,
$request,
$this->folderService,
$this->feedService,

View File

@ -66,7 +66,6 @@ class FolderApiControllerTest extends TestCase
->disableOriginalConstructor()
->getMock();
$this->folderAPI = new FolderApiController(
$appName,
$request,
$userSession,
$this->folderService,

View File

@ -79,7 +79,6 @@ class FolderControllerTest extends TestCase
->method('getUser')
->will($this->returnValue($this->user));
$this->class = new FolderController(
$appName,
$request,
$this->folderService,
$this->feedService,

View File

@ -66,7 +66,6 @@ class ItemApiControllerTest extends TestCase
->disableOriginalConstructor()
->getMock();
$this->class = new ItemApiController(
$this->appName,
$this->request,
$this->userSession,
$this->oldItemService,

View File

@ -80,7 +80,6 @@ class ItemControllerTest extends TestCase
->method('getUser')
->will($this->returnValue($this->user));
$this->controller = new ItemController(
$this->appName,
$this->request,
$this->feedService,
$this->itemService,

View File

@ -113,7 +113,6 @@ class PageControllerTest extends TestCase
->method('getUser')
->will($this->returnValue($this->user));
$this->controller = new PageController(
'news',
$this->request,
$this->settings,
$this->urlGenerator,

View File

@ -52,7 +52,8 @@ class UserApiControllerTest extends TestCase
->disableOriginalConstructor()
->getMock();
$this->controller = new UserApiController(
$this->appName, $this->request, $this->userSession,
$this->request,
$this->userSession,
$this->rootFolder
);

View File

@ -93,7 +93,6 @@ class UtilityApiControllerTest extends TestCase
->disableOriginalConstructor()
->getMock();
$this->newsAPI = new UtilityApiController(
$this->appName,
$this->request,
$this->userSession,
$this->updateService,

View File

@ -349,7 +349,7 @@ class FeedServiceTest extends TestCase
$fetchReturn = [$feed, $items];
$this->feedMapper->expects($this->exactly(2))
->method('find')
->method('findFromUser')
->with($this->user, $feed->getId())
->will($this->returnValue($feed));
$this->fetcher->expects($this->once())
@ -408,7 +408,7 @@ class FeedServiceTest extends TestCase
$fetchReturn = [$feed, $items];
$this->feedMapper->expects($this->exactly(2))
->method('find')
->method('findFromUser')
->with($this->user, $feed->getId())
->will($this->returnValue($feed));
$this->fetcher->expects($this->once())
@ -498,7 +498,7 @@ class FeedServiceTest extends TestCase
$fetchReturn = [$feed, $items];
$this->feedMapper->expects($this->exactly(2))
->method('find')
->method('findFromUser')
->will($this->returnValue($feed));
$this->fetcher->expects($this->once())
->method('fetch')
@ -536,7 +536,7 @@ class FeedServiceTest extends TestCase
$fetchReturn = [$feed, $items];
$this->feedMapper->expects($this->exactly(2))
->method('find')
->method('findFromUser')
->will($this->returnValue($feed));
$this->fetcher->expects($this->once())
->method('fetch')
@ -577,7 +577,7 @@ class FeedServiceTest extends TestCase
$items = [$item];
$this->feedMapper->expects($this->any())
->method('find')
->method('findFromUser')
->will($this->returnValue($existingFeed));
$this->fetcher->expects($this->once())
@ -612,7 +612,7 @@ class FeedServiceTest extends TestCase
$ex = new ReadErrorException('hi');
$this->feedMapper->expects($this->exactly(2))
->method('find')
->method('findFromUser')
->with($this->user, $feed->getId())
->willReturnOnConsecutiveCalls($feed, $expectedFeed);
$this->fetcher->expects($this->once())
@ -640,7 +640,7 @@ class FeedServiceTest extends TestCase
$ex = new DoesNotExistException('');
$this->feedMapper->expects($this->exactly(1))
->method('find')
->method('findFromUser')
->with($this->user, $feed->getId())
->will($this->throwException($ex));
@ -660,7 +660,7 @@ class FeedServiceTest extends TestCase
$ex = new DoesNotExistException('');
$this->feedMapper->expects($this->exactly(1))
->method('find')
->method('findFromUser')
->with($this->user, $feed->getId())
->will($this->returnValue($feed));
@ -699,7 +699,7 @@ class FeedServiceTest extends TestCase
$ex = new DoesNotExistException('');
$this->feedMapper->expects($this->exactly(2))
->method('find')
->method('findFromUser')
->with($this->user, $feed->getId())
->willReturnOnConsecutiveCalls($feed, $this->throwException($ex));
$this->feedMapper->expects($this->exactly(1))
@ -731,7 +731,7 @@ class FeedServiceTest extends TestCase
$feed->setPreventUpdate(true);
$this->feedMapper->expects($this->once())
->method('find')
->method('findFromUser')
->with($this->user, $feed->getId())
->will($this->returnValue($feed));
$this->fetcher->expects($this->never())
@ -750,7 +750,7 @@ class FeedServiceTest extends TestCase
$feed->setUrl('http://example.com');
$this->feedMapper->expects($this->once())
->method('find')
->method('findFromUser')
->with($this->user, $feed->getId())
->will($this->returnValue($feed));
$this->fetcher->expects($this->once())
@ -771,7 +771,7 @@ class FeedServiceTest extends TestCase
$feed->setId($feedId);
$this->feedMapper->expects($this->once())
->method('find')
->method('findFromUser')
->with($this->user, $feedId)
->will($this->returnValue($feed));
@ -796,7 +796,7 @@ class FeedServiceTest extends TestCase
$feed->setId($feedId);
$this->feedMapper->expects($this->once())
->method('find')
->method('findFromUser')
->with($this->equalTo($this->user), $this->equalTo($feedId))
->will($this->returnValue($feed));
@ -968,7 +968,7 @@ class FeedServiceTest extends TestCase
$feed2->setDeletedAt($this->time);
$this->feedMapper->expects($this->once())
->method('find')
->method('findFromUser')
->with($this->equalTo($this->user), $this->equalTo($id))
->will($this->returnValue($feed));
$this->feedMapper->expects($this->once())
@ -987,7 +987,7 @@ class FeedServiceTest extends TestCase
$feed2->setDeletedAt(0);
$this->feedMapper->expects($this->once())
->method('find')
->method('findFromUser')
->with($this->equalTo($this->user), $this->equalTo($id))
->will($this->returnValue($feed));
$this->feedMapper->expects($this->once())
@ -1064,7 +1064,7 @@ class FeedServiceTest extends TestCase
{
$feed = Feed::fromRow(['id' => 3]);
$this->feedMapper->expects($this->once())
->method('find')
->method('findFromUser')
->with(
$this->equalTo($this->user),
$this->equalTo($feed->getId())
@ -1092,7 +1092,7 @@ class FeedServiceTest extends TestCase
);
$feed2 = Feed::fromRow(['id' => 3]);
$this->feedMapper->expects($this->exactly(2))
->method('find')
->method('findFromUser')
->with(
$this->equalTo($this->user),
$this->equalTo($feed->getId())
@ -1116,7 +1116,7 @@ class FeedServiceTest extends TestCase
$this->expectException('OCA\News\Service\Exceptions\ServiceNotFoundException');
$feed = Feed::fromRow(['id' => 3]);
$this->feedMapper->expects($this->once())
->method('find')
->method('findFromUser')
->will($this->throwException(new DoesNotExistException('')));
$this->feedService->patch(3, $this->user);
@ -1127,7 +1127,7 @@ class FeedServiceTest extends TestCase
{
$feed = Feed::fromRow(['id' => 3, 'pinned' => false]);
$this->feedMapper->expects($this->once())
->method('find')
->method('findFromUser')
->with($this->user, $feed->getId())
->will($this->returnValue($feed));

View File

@ -73,7 +73,7 @@ class ServiceTest extends TestCase
->method('delete')
->with($this->equalTo($folder));
$this->mapper->expects($this->once())
->method('find')
->method('findFromUser')
->with($this->equalTo($user), $this->equalTo($id))
->will($this->returnValue($folder));
@ -87,7 +87,7 @@ class ServiceTest extends TestCase
$user = 'ken';
$this->mapper->expects($this->once())
->method('find')
->method('findFromUser')
->with($this->equalTo($user), $this->equalTo($id))
->will($this->returnValue(new Feed()));
@ -100,7 +100,7 @@ class ServiceTest extends TestCase
$ex = new DoesNotExistException('hi');
$this->mapper->expects($this->once())
->method('find')
->method('findFromUser')
->will($this->throwException($ex));
$this->expectException(ServiceNotFoundException::class);
@ -113,7 +113,7 @@ class ServiceTest extends TestCase
$ex = new MultipleObjectsReturnedException('hi');
$this->mapper->expects($this->once())
->method('find')
->method('findFromUser')
->will($this->throwException($ex));
$this->expectException(ServiceNotFoundException::class);