diff --git a/lib/Service/AttachmentService.php b/lib/Service/AttachmentService.php index 820b846cf..7ff427b5f 100644 --- a/lib/Service/AttachmentService.php +++ b/lib/Service/AttachmentService.php @@ -118,7 +118,7 @@ class AttachmentService { /** @var IAttachmentService $service */ $service = $this->getService($attachmentType); if ($service instanceof ICustomAttachmentService) { - $attachments = array_merge($attachments, $service->listAttachments($cardId)); + $attachments = array_merge($attachments, $service->listAttachments((int)$cardId)); } } @@ -151,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((int)$cardId); } } diff --git a/lib/Service/FilesAppService.php b/lib/Service/FilesAppService.php index 460a6b669..b84ebc1bc 100644 --- a/lib/Service/FilesAppService.php +++ b/lib/Service/FilesAppService.php @@ -77,11 +77,11 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { $shares = array_filter($shares, function ($share) { return $share->getPermissions() > 0; }); - return array_map(function (IShare $share) use ($cardId, $userFolder) { + return array_map(function (IShare $share) use ($cardId) { $file = $share->getNode(); $attachment = new Attachment(); $attachment->setType('file'); - $attachment->setId($share->getId()); + $attachment->setId((int)$share->getId()); $attachment->setCardId($cardId); $attachment->setCreatedBy($share->getSharedBy()); $attachment->setData($file->getName()); @@ -179,18 +179,16 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { throw new StatusException('Could not read file'); } $target->putContent($content); - if (is_resource($content)) { - fclose($content); - } + fclose($content); $share = $this->shareManager->newShare(); $share->setNode($target); - $share->setShareType(Share::SHARE_TYPE_DECK); + $share->setShareType(ISHARE::TYPE_DECK); $share->setSharedWith((string)$attachment->getCardId()); $share->setPermissions(Constants::PERMISSION_READ); $share->setSharedBy($this->userId); $share = $this->shareManager->createShare($share); - $attachment->setId($share->getId()); + $attachment->setId((int)$share->getId()); $attachment->setData($target->getName()); return $attachment; } @@ -237,9 +235,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { throw new StatusException('Could not read file'); } $target->putContent($content); - if (is_resource($content)) { - fclose($content); - } + fclose($content); $attachment->setLastModified(time()); return $attachment; @@ -249,9 +245,6 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { $share = $this->shareProvider->getShareById($attachment->getId()); $file = $share->getNode(); $attachment->setData($file->getName()); - if ($file === null) { - throw new NotFoundException('File not found'); - } if ($file->getOwner() !== null && $file->getOwner()->getUID() === $this->userId) { $file->delete(); diff --git a/lib/Sharing/DeckShareProvider.php b/lib/Sharing/DeckShareProvider.php index 3b2aa460b..d19493abf 100644 --- a/lib/Sharing/DeckShareProvider.php +++ b/lib/Sharing/DeckShareProvider.php @@ -114,7 +114,7 @@ class DeckShareProvider implements \OCP\Share\IShareProvider { try { $board = $this->boardMapper->find($boardId); - $valid &= !$board->getArchived(); + $valid = $valid && !$board->getArchived(); } catch (DoesNotExistException | MultipleObjectsReturnedException $e) { $valid = false; } @@ -234,7 +234,7 @@ class DeckShareProvider implements \OCP\Share\IShareProvider { */ private function createShareObject(array $data): IShare { $share = $this->shareManager->newShare(); - $share->setId((int)$data['id']) + $share->setId($data['id']) ->setShareType((int)$data['share_type']) ->setPermissions((int)$data['permissions']) ->setTarget($data['file_target']) @@ -401,7 +401,7 @@ class DeckShareProvider implements \OCP\Share\IShareProvider { $qb->execute(); - return $this->getShareById($share->getId(), $recipient); + return $this->getShareById((int)$share->getId(), $recipient); } /** @@ -456,6 +456,7 @@ class DeckShareProvider implements \OCP\Share\IShareProvider { /** * @inheritDoc + * @returns */ public function getSharesInFolder($userId, Folder $node, $reshares) { $qb = $this->dbConnection->getQueryBuilder(); @@ -755,14 +756,13 @@ class DeckShareProvider implements \OCP\Share\IShareProvider { /** * Get shared with the card * - * @param string $userId get shares where this user is the recipient + * @param int $cardId * @param int $shareType - * @param Node|null $node * @param int $limit The max number of entries returned, -1 for all * @param int $offset * @return IShare[] */ - public function getSharedWithByType(string $cardId, int $shareType, $limit, $offset): array { + public function getSharedWithByType(int $cardId, int $shareType, $limit, $offset): array { /** @var IShare[] $shares */ $shares = []; diff --git a/lib/Sharing/ShareAPIHelper.php b/lib/Sharing/ShareAPIHelper.php index e212af61e..d086f2fdd 100644 --- a/lib/Sharing/ShareAPIHelper.php +++ b/lib/Sharing/ShareAPIHelper.php @@ -32,6 +32,7 @@ use OCA\Deck\NoPermissionException; use OCA\Deck\Service\PermissionService; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IL10N; use OCP\IURLGenerator; use OCP\Share\IShare; @@ -40,12 +41,14 @@ class ShareAPIHelper { private $timeFactory; private $cardMapper; private $permissionService; + private $l10n; - public function __construct(IURLGenerator $urlGenerator, ITimeFactory $timeFactory, CardMapper $cardMapper, PermissionService $permissionService) { + public function __construct(IURLGenerator $urlGenerator, ITimeFactory $timeFactory, CardMapper $cardMapper, PermissionService $permissionService, IL10N $l10n) { $this->urlGenerator = $urlGenerator; $this->timeFactory = $timeFactory; $this->cardMapper = $cardMapper; $this->permissionService = $permissionService; + $this->l10n = $l10n; } public function formatShare(IShare $share): array { @@ -67,7 +70,7 @@ class ShareAPIHelper { $expireDate = $this->parseDate($expireDate); $share->setExpirationDate($expireDate); } catch (\Exception $e) { - throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD')); + throw new OCSNotFoundException($this->l10n->t('Invalid date, date format must be YYYY-MM-DD')); } } } @@ -90,10 +93,6 @@ class ShareAPIHelper { throw new \Exception('Invalid date. Format must be YYYY-MM-DD'); } - if ($date === false) { - throw new \Exception('Invalid date. Format must be YYYY-MM-DD'); - } - $date->setTime(0, 0, 0); return $date; diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 9e0a0aa93..f8ab41ada 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -4,18 +4,6 @@ $message !== null - - getArchived - getBoardId - getBoardId - getBoardId - getBoardId - getCardId - getCardId - getStackId - getTitle - getTitle - @@ -28,23 +16,9 @@ - - IBootstrap - - - - - listen - listen - - - - - getAcl - getOwner - getTitle - getTitle - + + method_exists($shareManager, 'registerShareProvider') + @@ -90,11 +64,9 @@ - - Application - Application - Application - + + LoadSidebar + @@ -104,21 +76,6 @@ Util - - - Job - - - - - Job - - - - - Job - - ExternalCalendar @@ -140,18 +97,6 @@ NotFound - - - IWidget - - - - - getPermissionEdit - getPermissionManage - getPermissionShare - - $aclId @@ -161,32 +106,12 @@ $cardId - - getParticipant - getParticipant - getParticipant - getParticipant - getParticipant - getParticipant - getType - getType - getType - - - getCardId - getCardId - $query - - - getLastModified - - $boardId @@ -194,53 +119,20 @@ \OCA\Circles\Api\v1\Circles - - setAcl - setLabels - VCalendar VCalendar - - getArchived - getArchived - getDescription - getLabels - getLabels - getLastModified - getLastModified - getStackId - getTitle - - - $qb->createNamedParameter($boardIds, IQueryBuilder::PARAM_INT_ARRAY) - $entity->getId() $cardId - - getDescription - getDescription - getDuedate - setCreatedAt - setDatabaseType - setDatabaseType - setDescription - setDescription - setLabels - setLastModified - setLastModified - setNotified - setNotified - @@ -269,69 +161,26 @@ \OCA\Circles\Model\Circle - - - getLastModified - - $labelId - - setLastModified - setLastModified - - - - - getETag - VCalendar VCalendar - - getLastModified - getTitle - $stackId - - getBoardId - - - - - BeforeTemplateRenderedEvent - - - - - getParticipant - getParticipant - getType - getType - $board->getId() - - Application - - - getTitle - getTitle - getTitle - getTitle - @@ -348,42 +197,12 @@ [] - - IndexDocument - SearchTemplate - - - - - SearchResultEntry - - - - - SearchResultEntry - - - - - IProvider - $cardId $cardId - - getParticipant - getParticipant - getParticipant - getType - getType - getType - setCardId - setParticipant - setType - $this->currentUser @@ -391,86 +210,11 @@ $this->currentUser - - - Application - - - getCardId - getCardId - getCardId - getData - getType - getType - setCardId - setCreatedAt - setCreatedBy - setData - setLastModified - setLastModified - setType - - - - '\OCA\Deck\Board::onCreate' - '\OCA\Deck\Board::onDelete' - '\OCA\Deck\Board::onDelete' - '\OCA\Deck\Board::onShareEdit' - '\OCA\Deck\Board::onUpdate' - '\OCA\Deck\Board::onUpdate' - - - Application - Application - Application - findAll findAll - - getAcl - getAcl - getAcl - getBoardId - getBoardId - getBoardId - getBoardId - getParticipant - getType - setBoardId - setBoardId - setBoardId - setBoardId - setColor - setColor - setColor - setColor - setColor - setLabels - setOwner - setOwner - setParticipant - setPermissionEdit - setPermissionEdit - setPermissionManage - setPermissionManage - setPermissionShare - setPermissionShare - setPermissions - setPermissions - setPermissions - setPermissions - setSettings - setTitle - setTitle - setTitle - setTitle - setTitle - setTitle - setType - @@ -479,52 +223,6 @@ \OCP\AppFramework\Db\ - - getArchived - getArchived - getArchived - getArchived - getArchived - getDescription - getDescription - getDescription - getDescriptionPrev - getDescriptionPrev - getLastEditor - getLastEditor - getLastEditor - getOrder - setArchived - setArchived - setArchived - setAssignedUsers - setAssignedUsers - setAttachmentCount - setAttachments - setCommentsUnread - setDeletedAt - setDeletedAt - setDescription - setDescription - setDescriptionPrev - setDescriptionPrev - setDuedate - setDuedate - setLabels - setLastEditor - setOrder - setOrder - setOwner - setOwner - setStackId - setStackId - setStackId - setTitle - setTitle - setTitle - setType - setType - @@ -533,13 +231,6 @@ - - Application - Application - Application - Application - Application - $this->cardMapper $this->permissionService @@ -555,25 +246,7 @@ $this->permissionService - - - (int)$value - - - Application - Application - Application - Application - Application - Application - Application - - - - Application - Application - $color === false || $color === null $color === null @@ -597,94 +270,46 @@ is_resource($content) is_resource($content) - - getCardId - getCardId - getCardId - getData - getData - setData - setData - setDeletedAt - setExtendedData - setLastModified - - - - DocumentAccess - IndexDocument - - - getDescription - getDescription - getTitle - getTitle - - - - - getBoardId - getBoardId - getBoardId - setBoardId - setColor - setColor - setTitle - setTitle - - - - - setAssignedUsers - setAttachmentCount - setCommentsUnread - setLabels - + + + $this->rootFolder + $this->rootFolder + IRootFolder + Share\Exceptions\ShareNotFound + \OCA\Circles\Api\v1\Circles \OCA\Circles\Api\v1\Circles - - getAcl - getParticipant - getParticipant - getParticipant - getParticipant - getType - getType - getType - getType - getType - getType - - - '\OCA\Deck\Stack::onCreate' - '\OCA\Deck\Stack::onDelete' - '\OCA\Deck\Stack::onUpdate' - BadRquestException - - getBoardId - getBoardId - getBoardId - getBoardId - getOrder - setBoardId - setBoardId - setCards - setDeletedAt - setDeletedAt - setOrder - setOrder - setTitle - setTitle - + + + + $shares + + + getSharesInFolder + + + GenericShareException + GenericShareException + ShareNotFound + ShareNotFound + ShareNotFound + ShareNotFound + ShareNotFound + + + + + [self::class, 'listenPreShare'] + diff --git a/tests/unit/Activity/ActivityManagerTest.php b/tests/unit/Activity/ActivityManagerTest.php index e91ca6cae..922e780a0 100644 --- a/tests/unit/Activity/ActivityManagerTest.php +++ b/tests/unit/Activity/ActivityManagerTest.php @@ -316,10 +316,6 @@ class ActivityManagerTest extends TestCase { $stack->setBoardId(999); $board = new Board(); $board->setId(999); - $this->attachmentMapper->expects($this->once()) - ->method('find') - ->with(777) - ->willReturn($attachment); $this->cardMapper->expects($this->once()) ->method('find') ->with(555) @@ -340,7 +336,7 @@ class ActivityManagerTest extends TestCase { 'archived' => $card->getArchived() ], 'attachment' => $attachment - ], $this->invokePrivate($this->activityManager, 'findDetailsForAttachment', [777])); + ], $this->invokePrivate($this->activityManager, 'findDetailsForAttachment', [$attachment])); } public function invokePrivate(&$object, $methodName, array $parameters = []) { diff --git a/tests/unit/Service/AttachmentServiceTest.php b/tests/unit/Service/AttachmentServiceTest.php index 85863fe18..d76bb2c52 100644 --- a/tests/unit/Service/AttachmentServiceTest.php +++ b/tests/unit/Service/AttachmentServiceTest.php @@ -270,7 +270,7 @@ class AttachmentServiceTest extends TestCase { ->method('display') ->with($attachment) ->willReturn($response); - $actual = $this->attachmentService->display(1); + $actual = $this->attachmentService->display(1, 1); $this->assertEquals($response, $actual); } @@ -286,8 +286,8 @@ class AttachmentServiceTest extends TestCase { $this->attachmentServiceImpl->expects($this->once()) ->method('display') ->with($attachment) - ->will($this->throwException(new InvalidAttachmentType('deck_file'))); - $this->attachmentService->display(1); + ->will($this->throwException(new NotFoundException('deck_file'))); + $this->attachmentService->display(1, 1); } public function testUpdate() { $attachment = $this->createAttachment('deck_file', 'file_name.jpg'); @@ -309,7 +309,7 @@ class AttachmentServiceTest extends TestCase { $a->setExtendedData(['mime' => 'image/jpeg']); }); - $actual = $this->attachmentService->update(1, 'file_name.jpg'); + $actual = $this->attachmentService->update(1, 1, 'file_name.jpg'); $expected->setExtendedData(['mime' => 'image/jpeg']); $expected->setLastModified($attachment->getLastModified()); @@ -333,7 +333,7 @@ class AttachmentServiceTest extends TestCase { $this->attachmentMapper->expects($this->once()) ->method('delete') ->willReturn($attachment); - $actual = $this->attachmentService->delete(1); + $actual = $this->attachmentService->delete(1, 1); $this->assertEquals($expected, $actual); } @@ -358,7 +358,7 @@ class AttachmentServiceTest extends TestCase { ->method('update') ->willReturn($attachment); $expected->setDeletedAt(23); - $actual = $this->attachmentService->delete(1); + $actual = $this->attachmentService->delete(1, 1); $this->assertEquals($expected, $actual); } @@ -378,7 +378,7 @@ class AttachmentServiceTest extends TestCase { ->method('update') ->willReturn($attachment); $expected->setDeletedAt(0); - $actual = $this->attachmentService->restore(1); + $actual = $this->attachmentService->restore(1, 1); $this->assertEquals($expected, $actual); } @@ -395,6 +395,6 @@ class AttachmentServiceTest extends TestCase { $this->attachmentServiceImpl->expects($this->once()) ->method('allowUndo') ->willReturn(false); - $actual = $this->attachmentService->restore(1); + $actual = $this->attachmentService->restore(1, 1); } }