[stable5.3] fix: file share

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
 	 *

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);

lib/Service/UploadedFilesShareService.php

@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Forms\Service;
+
+use OCA\Forms\Constants;
+use OCA\Forms\Db\Form;
+use OCA\Forms\Db\Share;
+use OCA\Forms\Db\ShareMapper;
+use OCA\Forms\Helper\FilePathHelper;
+use OCP\Files\IRootFolder;
+use OCP\Files\NotFoundException;
+use OCP\Share\IManager;
+use OCP\Share\IShare;
+use Psr\Log\LoggerInterface;
+
+class UploadedFilesShareService {
+	public function __construct(
+		private readonly IRootFolder $rootFolder,
+		private readonly FilePathHelper $filePathHelper,
+		private readonly IManager $shareManager,
+		private readonly ShareMapper $shareMapper,
+		private readonly LoggerInterface $logger,
+	) {
+	}
+
+	/**
+	 * remove all linked files shares (oc_share) for collaborators who have 'view responses' on the given form.
+	 */
+	public function removeAllForForm(Form $form): void {
+		$formShares = $this->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]);
+		}
+	}
+}

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,
 		);
 	}
 

tests/Unit/Db/FormMapperTest.php

@@ -0,0 +1,66 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Forms\Tests\Unit\Db;
+
+use OCA\Forms\Db\Form;
+use OCA\Forms\Db\FormMapper;
+use OCA\Forms\Db\QuestionMapper;
+use OCA\Forms\Db\ShareMapper;
+use OCA\Forms\Db\SubmissionMapper;
+use OCA\Forms\Db\UploadedFileMapper;
+use OCA\Forms\Helper\FilePathHelper;
+use OCA\Forms\Service\ConfigService;
+use OCA\Forms\Service\UploadedFilesShareService;
+use OCP\Comments\ICommentsManager;
+use OCP\Files\IRootFolder;
+use OCP\IDBConnection;
+use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
+use Test\TestCase;
+
+class FormMapperTest extends TestCase {
+	private FormMapper $formMapper;
+	private UploadedFilesShareService|MockObject $uploadedFilesShareService;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->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);
+	}
+}

tests/Unit/Service/UploadedFilesShareServiceTest.php

@@ -0,0 +1,99 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Forms\Tests\Unit\Service;
+
+use OCA\Forms\Constants;
+use OCA\Forms\Db\Form;
+use OCA\Forms\Db\Share;
+use OCA\Forms\Db\ShareMapper;
+use OCA\Forms\Helper\FilePathHelper;
+use OCA\Forms\Service\UploadedFilesShareService;
+use OCP\Files\Folder;
+use OCP\Files\IFilenameValidator;
+use OCP\Files\IRootFolder;
+use OCP\Share\IManager;
+use OCP\Share\IShare;
+use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
+use Test\TestCase;
+
+class UploadedFilesShareServiceTest extends TestCase {
+	private UploadedFilesShareService $service;
+	private ShareMapper|MockObject $shareMapper;
+	private IRootFolder|MockObject $rootFolder;
+	private IManager|MockObject $shareManager;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->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);
+	}
+}