From 74814ad6141f406704cade09ff5f7412d2ec25dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 30 Dec 2020 12:54:19 +0100 Subject: [PATCH] Fix test mocking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .php_cs.dist | 1 + lib/AppInfo/Application20.php | 2 -- lib/Service/AttachmentService.php | 11 ++--------- lib/Service/BoardService.php | 2 +- lib/Service/CardService.php | 2 +- lib/Service/FileService.php | 5 +---- lib/Service/FilesAppService.php | 12 +----------- lib/Service/ICustomAttachmentService.php | 3 --- lib/Sharing/DeckShareProvider.php | 6 ++---- lib/Sharing/Listener.php | 1 - lib/Sharing/ShareAPIHelper.php | 2 -- tests/unit/Service/AttachmentServiceTest.php | 20 +++++++++++++++++--- tests/unit/Service/BoardServiceTest.php | 15 ++------------- tests/unit/controller/PageControllerTest.php | 5 ++++- 14 files changed, 32 insertions(+), 55 deletions(-) diff --git a/.php_cs.dist b/.php_cs.dist index 3b4fa98f1..965ed18ec 100644 --- a/.php_cs.dist +++ b/.php_cs.dist @@ -13,6 +13,7 @@ $config ->notPath('build') ->notPath('l10n') ->notPath('src') + ->notPath('node_modules') ->notPath('vendor') ->in(__DIR__); return $config; diff --git a/lib/AppInfo/Application20.php b/lib/AppInfo/Application20.php index 80cd94b62..06b95930a 100644 --- a/lib/AppInfo/Application20.php +++ b/lib/AppInfo/Application20.php @@ -26,7 +26,6 @@ namespace OCA\Deck\AppInfo; use Closure; use Exception; use OC\EventDispatcher\SymfonyAdapter; -use OC\Share20\ProviderFactory; use OCA\Deck\Activity\CommentEventHandler; use OCA\Deck\Capabilities; use OCA\Deck\Collaboration\Resources\ResourceProvider; @@ -66,7 +65,6 @@ use OCP\IUser; use OCP\IUserManager; use OCP\Notification\IManager as NotificationManager; use OCP\Share\IManager; -use OCP\Share\IProviderFactory; use OCP\Util; use Psr\Container\ContainerInterface; diff --git a/lib/Service/AttachmentService.php b/lib/Service/AttachmentService.php index 1b98b9c11..67b3dd0d2 100644 --- a/lib/Service/AttachmentService.php +++ b/lib/Service/AttachmentService.php @@ -34,16 +34,11 @@ use OCA\Deck\Db\ChangeHelper; use OCA\Deck\InvalidAttachmentType; use OCA\Deck\NoPermissionException; use OCA\Deck\NotFoundException; -use OCA\Deck\Sharing\DeckShareProvider; use OCA\Deck\StatusException; use OCP\AppFramework\Http\Response; -use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\ICache; use OCP\ICacheFactory; -use OCP\IDBConnection; use OCP\IL10N; -use OCP\IPreview; -use OCP\Share\IShare; class AttachmentService { private $attachmentMapper; @@ -63,7 +58,7 @@ class AttachmentService { /** @var ChangeHelper */ private $changeHelper; - public function __construct(AttachmentMapper $attachmentMapper, CardMapper $cardMapper, ChangeHelper $changeHelper, PermissionService $permissionService, Application $application, ICacheFactory $cacheFactory, $userId, IL10N $l10n, ActivityManager $activityManager, DeckShareProvider $shareProvider) { + public function __construct(AttachmentMapper $attachmentMapper, CardMapper $cardMapper, ChangeHelper $changeHelper, PermissionService $permissionService, Application $application, ICacheFactory $cacheFactory, $userId, IL10N $l10n, ActivityManager $activityManager) { $this->attachmentMapper = $attachmentMapper; $this->cardMapper = $cardMapper; $this->permissionService = $permissionService; @@ -156,7 +151,7 @@ class AttachmentService { foreach (array_keys($this->services) as $attachmentType) { $service = $this->getService($attachmentType); if ($service instanceof ICustomAttachmentService) { - $count += $service->getAttachmentCount($cardId); + $count += $service->getAttachmentCount($cardId); } } @@ -212,7 +207,6 @@ class AttachmentService { } $service->extendData($attachment); - } catch (InvalidAttachmentType $e) { // just store the data } @@ -259,7 +253,6 @@ class AttachmentService { } catch (\Exception $e) { throw new NotFoundException(); } - } /** diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index c524606d5..321d69ba4 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -324,7 +324,7 @@ class BoardService { 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false ]); - $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $new_board, ActivityManager::SUBJECT_BOARD_CREATE); + $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $new_board, ActivityManager::SUBJECT_BOARD_CREATE, [], $userId); $this->changeHelper->boardChanged($new_board->getId()); $this->eventDispatcher->dispatch( diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index e7d00277f..fc3cbc005 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -133,7 +133,7 @@ class CardService { return $this->cardMapper->searchRaw($boardIds, $term, $limit, $offset); } - /** + /** * @param $cardId * @return \OCA\Deck\Db\RelationalEntity * @throws \OCA\Deck\NoPermissionException diff --git a/lib/Service/FileService.php b/lib/Service/FileService.php index c6dc0dc8d..2112a30f1 100644 --- a/lib/Service/FileService.php +++ b/lib/Service/FileService.php @@ -40,7 +40,6 @@ use OCP\IConfig; use OCP\IL10N; use OCP\ILogger; use OCP\IRequest; -use OCP\Share\IManager; class FileService implements IAttachmentService { private $l10n; @@ -58,8 +57,7 @@ class FileService implements IAttachmentService { ILogger $logger, IRootFolder $rootFolder, IConfig $config, - AttachmentMapper $attachmentMapper, - IManager $shareManager + AttachmentMapper $attachmentMapper ) { $this->l10n = $l10n; $this->appData = $appData; @@ -68,7 +66,6 @@ class FileService implements IAttachmentService { $this->rootFolder = $rootFolder; $this->config = $config; $this->attachmentMapper = $attachmentMapper; - $this->shareManager = $shareManager; } /** diff --git a/lib/Service/FilesAppService.php b/lib/Service/FilesAppService.php index 10e90c690..9e7a988a2 100644 --- a/lib/Service/FilesAppService.php +++ b/lib/Service/FilesAppService.php @@ -24,25 +24,17 @@ namespace OCA\Deck\Service; use OCA\Deck\Db\Attachment; -use OCA\Deck\Db\AttachmentMapper; use OCA\Deck\Sharing\DeckShareProvider; use OCA\Deck\StatusException; use OCA\Deck\Exceptions\ConflictException; use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\FileDisplayResponse; -use OCP\AppFramework\Http\Response; use OCP\AppFramework\Http\StreamResponse; use OCP\Constants; -use OCP\Files\IAppData; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; -use OCP\Files\NotPermittedException; -use OCP\Files\SimpleFS\ISimpleFile; -use OCP\Files\SimpleFS\ISimpleFolder; -use OCP\IConfig; use OCP\IDBConnection; use OCP\IL10N; -use OCP\ILogger; use OCP\IPreview; use OCP\IRequest; use OCP\Share; @@ -50,7 +42,6 @@ use OCP\Share\IManager; use OCP\Share\IShare; class FilesAppService implements IAttachmentService, ICustomAttachmentService { - private $request; private $rootFolder; private $shareProvider; @@ -100,7 +91,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { }, $shares); } - function getAttachmentCount(int $cardId): int { + public function getAttachmentCount(int $cardId): int { /** @var IDBConnection $qb */ $db = \OC::$server->getDatabaseConnection(); $qb = $db->getQueryBuilder(); @@ -247,7 +238,6 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { return; } } - } public function allowUndo() { diff --git a/lib/Service/ICustomAttachmentService.php b/lib/Service/ICustomAttachmentService.php index 429a11cfd..bc1486c1d 100644 --- a/lib/Service/ICustomAttachmentService.php +++ b/lib/Service/ICustomAttachmentService.php @@ -26,7 +26,6 @@ declare(strict_types=1); namespace OCA\Deck\Service; - /** * Interface to implement in case attachments are handled by a different backend than * then oc_deck_attachments table, e.g. for file sharing. When this interface is used @@ -34,9 +33,7 @@ namespace OCA\Deck\Service; * table and it is up to the implementation to track attachment to card relation. */ interface ICustomAttachmentService { - public function listAttachments(int $cardId): array; public function getAttachmentCount(int $cardId): int; - } diff --git a/lib/Sharing/DeckShareProvider.php b/lib/Sharing/DeckShareProvider.php index b60ae4bc1..bf2b63937 100644 --- a/lib/Sharing/DeckShareProvider.php +++ b/lib/Sharing/DeckShareProvider.php @@ -26,7 +26,6 @@ declare(strict_types=1); namespace OCA\Deck\Sharing; - use OC\Files\Cache\Cache; use OCA\Deck\Db\Acl; use OCA\Deck\Db\Board; @@ -60,7 +59,6 @@ interface IShareProviderBackend { } class DeckShareProvider implements \OCP\Share\IShareProvider { - public const DECK_FOLDER = '/Deck'; public const DECK_FOLDER_PLACEHOLDER = '/{DECK_PLACEHOLDER}'; @@ -117,7 +115,7 @@ class DeckShareProvider implements \OCP\Share\IShareProvider { try { $board = $this->boardMapper->find($boardId); $valid &= !$board->getArchived(); - } catch (DoesNotExistException|MultipleObjectsReturnedException $e) { + } catch (DoesNotExistException | MultipleObjectsReturnedException $e) { $valid = false; } @@ -901,7 +899,7 @@ class DeckShareProvider implements \OCP\Share\IShareProvider { $types = [IShare::TYPE_DECK]; if ($currentAccess) { - $types[] = self::SHARE_TYPE_DECK_USER; + $types[] = self::SHARE_TYPE_DECK_USER; } $qb->select('id', 'parent', 'share_type', 'share_with', 'file_source', 'file_target', 'permissions') diff --git a/lib/Sharing/Listener.php b/lib/Sharing/Listener.php index 3c0c00ee8..580b2e5c6 100644 --- a/lib/Sharing/Listener.php +++ b/lib/Sharing/Listener.php @@ -26,7 +26,6 @@ declare(strict_types=1); namespace OCA\Deck\Sharing; - use OC\Files\Filesystem; use OCA\Deck\Service\ConfigService; use OCP\EventDispatcher\IEventDispatcher; diff --git a/lib/Sharing/ShareAPIHelper.php b/lib/Sharing/ShareAPIHelper.php index 31472000a..e212af61e 100644 --- a/lib/Sharing/ShareAPIHelper.php +++ b/lib/Sharing/ShareAPIHelper.php @@ -26,7 +26,6 @@ declare(strict_types=1); namespace OCA\Deck\Sharing; - use OCA\Deck\Db\Acl; use OCA\Deck\Db\CardMapper; use OCA\Deck\NoPermissionException; @@ -37,7 +36,6 @@ use OCP\IURLGenerator; use OCP\Share\IShare; class ShareAPIHelper { - private $urlGenerator; private $timeFactory; private $cardMapper; diff --git a/tests/unit/Service/AttachmentServiceTest.php b/tests/unit/Service/AttachmentServiceTest.php index 75a861f1d..85863fe18 100644 --- a/tests/unit/Service/AttachmentServiceTest.php +++ b/tests/unit/Service/AttachmentServiceTest.php @@ -42,7 +42,7 @@ use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; /** @internal Just for testing the service registration */ -class MyAttachmentService { +class MyAttachmentService implements IAttachmentService { public function extendData(Attachment $attachment) { } public function display(Attachment $attachment) { @@ -84,6 +84,10 @@ class AttachmentServiceTest extends TestCase { private $l10n; /** @var ChangeHelper */ private $changeHelper; + /** + * @var IAttachmentService|MockObject + */ + private $filesAppServiceImpl; /** * @throws \OCP\AppFramework\QueryException @@ -92,6 +96,8 @@ class AttachmentServiceTest extends TestCase { parent::setUp(); $this->attachmentServiceImpl = $this->createMock(IAttachmentService::class); + $this->filesAppServiceImpl = $this->createMock(IAttachmentService::class); + $this->appContainer = $this->createMock(IAppContainer::class); $this->attachmentMapper = $this->createMock(AttachmentMapper::class); @@ -105,6 +111,8 @@ class AttachmentServiceTest extends TestCase { $this->cacheFactory->expects($this->any())->method('createDistributed')->willReturn($this->cache); $this->appContainer->expects($this->at(0))->method('query')->with(FileService::class)->willReturn($this->attachmentServiceImpl); + $this->appContainer->expects($this->at(1))->method('query')->with(FilesAppService::class)->willReturn($this->filesAppServiceImpl); + $this->application->expects($this->any()) ->method('getContainer') ->willReturn($this->appContainer); @@ -119,8 +127,12 @@ class AttachmentServiceTest extends TestCase { $application = $this->createMock(Application::class); $appContainer = $this->createMock(IAppContainer::class); $fileServiceMock = $this->createMock(FileService::class); - $appContainer->expects($this->at(1))->method('query')->with(MyAttachmentService::class)->willReturn(new MyAttachmentService()); + $fileAppServiceMock = $this->createMock(FilesAppService::class); + $appContainer->expects($this->at(0))->method('query')->with(FileService::class)->willReturn($fileServiceMock); + $appContainer->expects($this->at(1))->method('query')->with(FilesAppService::class)->willReturn($fileAppServiceMock); + $appContainer->expects($this->at(2))->method('query')->with(MyAttachmentService::class)->willReturn(new MyAttachmentService()); + $application->expects($this->any()) ->method('getContainer') ->willReturn($appContainer); @@ -135,8 +147,10 @@ class AttachmentServiceTest extends TestCase { $application = $this->createMock(Application::class); $appContainer = $this->createMock(IAppContainer::class); $fileServiceMock = $this->createMock(FileService::class); + $fileAppServiceMock = $this->createMock(FilesAppService::class); $appContainer->expects($this->at(0))->method('query')->with(FileService::class)->willReturn($fileServiceMock); - $appContainer->expects($this->at(1))->method('query')->with(MyAttachmentService::class)->willReturn(new MyAttachmentService()); + $appContainer->expects($this->at(1))->method('query')->with(FilesAppService::class)->willReturn($fileAppServiceMock); + $appContainer->expects($this->at(2))->method('query')->with(MyAttachmentService::class)->willReturn(new MyAttachmentService()); $application->expects($this->any()) ->method('getContainer') ->willReturn($appContainer); diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index 146c08bc0..a1c177b05 100644 --- a/tests/unit/Service/BoardServiceTest.php +++ b/tests/unit/Service/BoardServiceTest.php @@ -123,20 +123,9 @@ class BoardServiceTest extends TestCase { $b3 = new Board(); $b3->setId(3); $this->boardMapper->expects($this->once()) - ->method('findAllByUser') + ->method('findAllForUser') ->with('admin') - ->willReturn([$b1, $b2]); - $this->stackMapper->expects($this->any()) - ->method('findAll') - ->willReturn([]); - $this->boardMapper->expects($this->once()) - ->method('findAllByGroups') - ->with('admin', ['a', 'b', 'c']) - ->willReturn([$b2, $b3]); - $this->boardMapper->expects($this->once()) - ->method('findAllByCircles') - ->with('admin') - ->willReturn([]); + ->willReturn([$b1, $b2, $b3]); $user = $this->createMock(IUser::class); $this->groupManager->method('getUserGroupIds') ->willReturn(['a', 'b', 'c']); diff --git a/tests/unit/controller/PageControllerTest.php b/tests/unit/controller/PageControllerTest.php index 4cc9e0c8b..508bc9917 100644 --- a/tests/unit/controller/PageControllerTest.php +++ b/tests/unit/controller/PageControllerTest.php @@ -26,6 +26,7 @@ namespace OCA\Deck\Controller; use OCA\Deck\Service\ConfigService; use OCA\Deck\Service\PermissionService; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IInitialStateService; use OCP\IL10N; use OCP\IRequest; @@ -37,6 +38,7 @@ class PageControllerTest extends \Test\TestCase { private $permissionService; private $initialState; private $configService; + private $eventDispatcher; public function setUp(): void { $this->l10n = $this->createMock(IL10N::class); @@ -44,9 +46,10 @@ class PageControllerTest extends \Test\TestCase { $this->permissionService = $this->createMock(PermissionService::class); $this->configService = $this->createMock(ConfigService::class); $this->initialState = $this->createMock(IInitialStateService::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->controller = new PageController( - 'deck', $this->request, $this->permissionService, $this->initialState, $this->configService + 'deck', $this->request, $this->permissionService, $this->initialState, $this->configService, $this->eventDispatcher ); }