diff --git a/lib/Controller/ShareApiController.php b/lib/Controller/ShareApiController.php index 544d55fd7..3300d74cf 100644 --- a/lib/Controller/ShareApiController.php +++ b/lib/Controller/ShareApiController.php @@ -19,6 +19,7 @@ use OCA\Forms\Service\CirclesService; use OCA\Forms\Service\ConfigService; use OCA\Forms\Service\FormsService; +use OCA\Forms\Service\UploadedFilesShareService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\IMapperException; use OCP\AppFramework\Http; @@ -62,6 +63,7 @@ public function __construct( private readonly IRootFolder $rootFolder, private readonly FilePathHelper $filePathHelper, private readonly IManager $shareManager, + private readonly UploadedFilesShareService $uploadedFilesShareService, ) { parent::__construct($appName, $request); } @@ -289,7 +291,7 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da $this->shareManager->createShare($folderShare); } else { - $this->removeUploadedFilesShare($form, $formShare); + $this->uploadedFilesShareService->removeForCollaborator($form, $formShare); } } @@ -341,7 +343,7 @@ public function deleteShare(int $formId, int $shareId): DataResponse { // Revoke any linked Files share before deleting the Forms share if (in_array(Constants::PERMISSION_RESULTS, $share->getPermissions(), true)) { - $this->removeUploadedFilesShare($form, $share); + $this->uploadedFilesShareService->removeForCollaborator($form, $share); } $this->shareMapper->delete($share); @@ -350,26 +352,6 @@ public function deleteShare(int $formId, int $shareId): DataResponse { return new DataResponse($shareId); } - private function removeUploadedFilesShare(Form $form, Share $formShare): void { - if (!in_array($formShare->getShareType(), [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_USERGROUP, IShare::TYPE_CIRCLE], true)) { - return; - } - - $userFolder = $this->rootFolder->getUserFolder($form->getOwnerId()); - $uploadedFilesFolderPath = $this->filePathHelper->getFormUploadedFilesFolderPath($form); - try { - $folder = $userFolder->get($uploadedFilesFolderPath); - } catch (NotFoundException) { - return; - } - $folderShares = $this->shareManager->getSharesBy($form->getOwnerId(), $formShare->getShareType(), $folder, false, -1); - foreach ($folderShares as $folderShare) { - if ($folderShare->getSharedWith() === $formShare->getShareWith()) { - $this->shareManager->deleteShare($folderShare); - } - } - } - /** * Validate user given permission array * diff --git a/lib/Db/FormMapper.php b/lib/Db/FormMapper.php index adfdbc4c2..54061c6d5 100644 --- a/lib/Db/FormMapper.php +++ b/lib/Db/FormMapper.php @@ -10,6 +10,7 @@ use OCA\Forms\Constants; use OCA\Forms\Helper\FilePathHelper; use OCA\Forms\Service\ConfigService; +use OCA\Forms\Service\UploadedFilesShareService; use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\QBMapper; use OCP\Comments\ICommentsManager; @@ -39,6 +40,7 @@ public function __construct( private readonly FilePathHelper $filePathHelper, private readonly UploadedFileMapper $uploadedFileMapper, private readonly IRootFolder $rootFolder, + private readonly UploadedFilesShareService $uploadedFilesShareService, private readonly LoggerInterface $logger, ) { parent::__construct($db, 'forms_v2_forms', Form::class); @@ -228,6 +230,7 @@ public function findAllByOwnerId(string $ownerId, ?string $queryTerm = null): ar public function deleteForm(Form $form): void { // Delete Submissions(incl. Answers), Questions(incl. Options), Shares and Form. $formId = $form->getId(); + $this->uploadedFilesShareService->removeAllForForm($form); $this->submissionMapper->deleteByForm($formId); $this->shareMapper->deleteByForm($formId); $this->questionMapper->deleteByForm($formId); diff --git a/lib/Service/UploadedFilesShareService.php b/lib/Service/UploadedFilesShareService.php new file mode 100644 index 000000000..ded293870 --- /dev/null +++ b/lib/Service/UploadedFilesShareService.php @@ -0,0 +1,72 @@ +shareMapper->findByForm($form->getId()); + foreach ($formShares as $formShare) { + if (in_array(Constants::PERMISSION_RESULTS, $formShare->getPermissions(), true)) { + $this->removeForCollaborator($form, $formShare); + } + } + } + + /** + * remove the linked files share (oc_share) for one collaborator (either user, group, or team) on the given form. + */ + public function removeForCollaborator(Form $form, Share $formShare): void { + if (!in_array($formShare->getShareType(), [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_USERGROUP, IShare::TYPE_CIRCLE], true)) { + return; + } + + try { + $userFolder = $this->rootFolder->getUserFolder($form->getOwnerId()); + $uploadedFilesFolderPath = $this->filePathHelper->getFormUploadedFilesFolderPath($form); + try { + $folder = $userFolder->get($uploadedFilesFolderPath); + } catch (NotFoundException) { + return; + } + + $folderShares = $this->shareManager->getSharesBy($form->getOwnerId(), $formShare->getShareType(), $folder, false, -1); + foreach ($folderShares as $folderShare) { + if ($folderShare->getSharedWith() === $formShare->getShareWith()) { + $this->shareManager->deleteShare($folderShare); + } + } + } catch (\Throwable $e) { + $this->logger->warning('Failed to remove uploaded files share for form', ['exception' => $e]); + } + } +} diff --git a/tests/Unit/Controller/ShareApiControllerTest.php b/tests/Unit/Controller/ShareApiControllerTest.php index 372e16348..56a21e7df 100644 --- a/tests/Unit/Controller/ShareApiControllerTest.php +++ b/tests/Unit/Controller/ShareApiControllerTest.php @@ -20,6 +20,7 @@ use OCA\Forms\Service\CirclesService; use OCA\Forms\Service\ConfigService; use OCA\Forms\Service\FormsService; +use OCA\Forms\Service\UploadedFilesShareService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\IMapperException; use OCP\AppFramework\Http; @@ -64,6 +65,7 @@ class ShareApiControllerTest extends TestCase { private IRootFolder|MockObject $rootFolder; private IFilenameValidator|MockObject $filenameValidator; private IManager|MockObject $shareManager; + private UploadedFilesShareService $uploadedFilesShareService; public function setUp(): void { $this->formMapper = $this->createMock(FormMapper::class); @@ -83,6 +85,13 @@ public function setUp(): void { ->willReturnCallback(static fn (string $fileName, string $replacement): string => str_replace(['/', '\\'], $replacement, trim($fileName))); $this->filePathHelper = new FilePathHelper($this->rootFolder, $this->filenameValidator); $this->shareManager = $this->createMock(IManager::class); + $this->uploadedFilesShareService = new UploadedFilesShareService( + $this->rootFolder, + $this->filePathHelper, + $this->shareManager, + $this->shareMapper, + $this->logger, + ); $this->shareApiController = new ShareApiController( 'forms', @@ -99,6 +108,7 @@ public function setUp(): void { $this->rootFolder, $this->filePathHelper, $this->shareManager, + $this->uploadedFilesShareService, ); } diff --git a/tests/Unit/Db/FormMapperTest.php b/tests/Unit/Db/FormMapperTest.php new file mode 100644 index 000000000..2fc86de63 --- /dev/null +++ b/tests/Unit/Db/FormMapperTest.php @@ -0,0 +1,66 @@ +uploadedFilesShareService = $this->createMock(UploadedFilesShareService::class); + + $this->formMapper = $this->getMockBuilder(FormMapper::class) + ->setConstructorArgs([ + $this->createMock(IDBConnection::class), + $this->createMock(QuestionMapper::class), + $this->createMock(ShareMapper::class), + $this->createMock(SubmissionMapper::class), + $this->createMock(ConfigService::class), + $this->createMock(ICommentsManager::class), + $this->createMock(FilePathHelper::class), + $this->createMock(UploadedFileMapper::class), + $this->createMock(IRootFolder::class), + $this->uploadedFilesShareService, + $this->createMock(LoggerInterface::class), + ]) + ->onlyMethods(['delete']) + ->getMock(); + } + + public function testDeleteFormCleansUpUploadedFilesShares(): void { + $form = new Form(); + $form->setId(2); + $form->setOwnerId('alice'); + + $this->uploadedFilesShareService->expects($this->once()) + ->method('removeAllForForm') + ->with($form); + + $this->formMapper->deleteForm($form); + } +} diff --git a/tests/Unit/Service/UploadedFilesShareServiceTest.php b/tests/Unit/Service/UploadedFilesShareServiceTest.php new file mode 100644 index 000000000..8d957a2bd --- /dev/null +++ b/tests/Unit/Service/UploadedFilesShareServiceTest.php @@ -0,0 +1,99 @@ +shareMapper = $this->createMock(ShareMapper::class); + $this->rootFolder = $this->createMock(IRootFolder::class); + $this->shareManager = $this->createMock(IManager::class); + $fileNameValidator = $this->createMock(IFilenameValidator::class); + $fileNameValidator->method('sanitizeFilename')->willReturnArgument(0); + $filePathHelper = new FilePathHelper( + $this->rootFolder, + $fileNameValidator, + ); + + $this->service = new UploadedFilesShareService( + $this->rootFolder, + $filePathHelper, + $this->shareManager, + $this->shareMapper, + $this->createMock(LoggerInterface::class), + ); + } + + public function testRemoveAllForFormCleansUpResultsShares(): void { + $form = new Form(); + $form->setId(2); + $form->setTitle('test'); + $form->setOwnerId('alice'); + + $resultsShare = new Share(); + $resultsShare->setShareType(IShare::TYPE_USER); + $resultsShare->setShareWith('bob'); + $resultsShare->setPermissions([Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS]); + + $submitOnlyShare = new Share(); + $submitOnlyShare->setShareType(IShare::TYPE_USER); + $submitOnlyShare->setShareWith('carol'); + $submitOnlyShare->setPermissions([Constants::PERMISSION_SUBMIT]); + + $this->shareMapper->expects($this->once()) + ->method('findByForm') + ->with(2) + ->willReturn([$resultsShare, $submitOnlyShare]); + + $folder = $this->createMock(Folder::class); + $userFolder = $this->createMock(Folder::class); + $userFolder->expects($this->once()) + ->method('get') + ->with('Forms/2 - test') + ->willReturn($folder); + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('alice') + ->willReturn($userFolder); + + $fileShare = $this->createMock(IShare::class); + $fileShare->method('getSharedWith')->willReturn('bob'); + $this->shareManager->expects($this->once()) + ->method('getSharesBy') + ->with('alice', IShare::TYPE_USER, $folder, false, -1) + ->willReturn([$fileShare]); + $this->shareManager->expects($this->once()) + ->method('deleteShare') + ->with($fileShare); + + $this->service->removeAllForForm($form); + } +}