fix: clean up file shares on collaborator removal

lib/Controller/ShareApiController.php

@@ -10,6 +10,7 @@
 namespace OCA\Forms\Controller;
 
 use OCA\Forms\Constants;
+use OCA\Forms\Db\Form;
 use OCA\Forms\Db\FormMapper;
 use OCA\Forms\Db\Share;
 use OCA\Forms\Db\ShareMapper;
@@ -30,6 +31,7 @@
 use OCP\AppFramework\OCS\OCSNotFoundException;
 use OCP\AppFramework\OCSController;
 use OCP\Files\IRootFolder;
+use OCP\Files\NotFoundException;
 use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\IRequest;
@@ -270,16 +272,16 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da
 		$formShare = $this->shareMapper->update($formShare);
 
 		if (in_array($formShare->getShareType(), [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_USERGROUP, IShare::TYPE_CIRCLE], true)) {
-			$userFolder = $this->rootFolder->getUserFolder($form->getOwnerId());
-			$uploadedFilesFolderPath = $this->formsService->getFormUploadedFilesFolderPath($form);
-			if ($userFolder->nodeExists($uploadedFilesFolderPath)) {
-				$folder = $userFolder->get($uploadedFilesFolderPath);
-			} else {
-				$folder = $userFolder->newFolder($uploadedFilesFolderPath);
-			}
-			/** @var \OCP\Files\Folder $folder */
-
 			if (in_array(Constants::PERMISSION_RESULTS, $keyValuePairs['permissions'], true)) {
+				$userFolder = $this->rootFolder->getUserFolder($form->getOwnerId());
+				$uploadedFilesFolderPath = $this->formsService->getFormUploadedFilesFolderPath($form);
+				try {
+					/** @var \OCP\Files\Folder $folder */
+					$folder = $userFolder->get($uploadedFilesFolderPath);
+				} catch (NotFoundException $e) {
+					$folder = $userFolder->newFolder($uploadedFilesFolderPath);
+				}
+
 				$folderShare = $this->shareManager->newShare();
 				$folderShare->setShareType($formShare->getShareType());
 				$folderShare->setSharedWith($formShare->getShareWith());
@@ -290,12 +292,7 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da
 
 				$this->shareManager->createShare($folderShare);
 			} else {
-				$folderShares = $this->shareManager->getSharesBy($form->getOwnerId(), $formShare->getShareType(), $folder);
-				foreach ($folderShares as $folderShare) {
-					if ($folderShare->getSharedWith() === $formShare->getShareWith()) {
-						$this->shareManager->deleteShare($folderShare);
-					}
-				}
+				$this->removeUploadedFilesShare($form, $formShare);
 			}
 		}
 
@@ -345,12 +342,37 @@ public function deleteShare(int $formId, int $shareId): DataResponse {
 
 		$this->formsService->obtainFormLock($form);
 
+		// Revoke any linked Files share before deleting the Forms share
+		if (in_array(Constants::PERMISSION_RESULTS, $share->getPermissions(), true)) {
+			$this->removeUploadedFilesShare($form, $share);
+		}
+
 		$this->shareMapper->delete($share);
 		$this->formMapper->update($form);
 
 		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->formsService->getFormUploadedFilesFolderPath($form);
+		try {
+			$folder = $userFolder->get($uploadedFilesFolderPath);
+		} catch (NotFoundException $e) {
+			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
 	 *

tests/Unit/Controller/ShareApiControllerTest.php

@@ -539,6 +539,101 @@ public function testDeleteShare() {
 		$this->assertEquals($response, $this->shareApiController->deleteShare(5, 8));
 	}
 
+	/**
+	 * Delete share that had results permission — Files share must be cleaned up.
+	 */
+	public function testDeleteShare_cleansUpFileShare(): void {
+		$share = new Share();
+		$share->setId(8);
+		$share->setFormId(5);
+		$share->setShareType(IShare::TYPE_USER);
+		$share->setShareWith('collaborator');
+		$share->setPermissions([Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS]);
+
+		$this->shareMapper->expects($this->once())
+			->method('findById')
+			->with('8')
+			->willReturn($share);
+
+		$form = new Form();
+		$form->setId('5');
+		$form->setOwnerId('currentUser');
+		$this->formsService->expects($this->once())
+			->method('getFormIfAllowed')
+			->with('5')
+			->willReturn($form);
+
+		// Mock the uploaded files folder lookup
+		$folder = $this->createMock(Folder::class);
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->expects($this->once())
+			->method('get')
+			->willReturn($folder);
+		$this->storage->expects($this->once())
+			->method('getUserFolder')
+			->with('currentUser')
+			->willReturn($userFolder);
+		$this->formsService->expects($this->once())
+			->method('getFormUploadedFilesFolderPath')
+			->with($form)
+			->willReturn('Forms/my-form');
+
+		// Mock finding and deleting the matching Files share
+		$fileShare = $this->createMock(IShare::class);
+		$fileShare->method('getSharedWith')->willReturn('collaborator');
+		$this->shareManager->expects($this->once())
+			->method('getSharesBy')
+			->with('currentUser', IShare::TYPE_USER, $folder)
+			->willReturn([$fileShare]);
+		$this->shareManager->expects($this->once())
+			->method('deleteShare')
+			->with($fileShare);
+
+		$this->shareMapper->expects($this->once())
+			->method('delete')
+			->with($share);
+
+		$response = new DataResponse(8);
+		$this->assertEquals($response, $this->shareApiController->deleteShare(5, 8));
+	}
+
+	/**
+	 * Delete share without results permission — no file share cleanup needed.
+	 */
+	public function testDeleteShare_noResultsPermission_skipsFileShareCleanup(): void {
+		$share = new Share();
+		$share->setId(8);
+		$share->setFormId(5);
+		$share->setShareType(IShare::TYPE_USER);
+		$share->setShareWith('collaborator');
+		$share->setPermissions([Constants::PERMISSION_SUBMIT]);
+
+		$this->shareMapper->expects($this->once())
+			->method('findById')
+			->with('8')
+			->willReturn($share);
+
+		$form = new Form();
+		$form->setId('5');
+		$form->setOwnerId('currentUser');
+		$this->formsService->expects($this->once())
+			->method('getFormIfAllowed')
+			->with('5')
+			->willReturn($form);
+
+		// No file share interactions expected
+		$this->storage->expects($this->never())->method('getUserFolder');
+		$this->shareManager->expects($this->never())->method('getSharesBy');
+		$this->shareManager->expects($this->never())->method('deleteShare');
+
+		$this->shareMapper->expects($this->once())
+			->method('delete')
+			->with($share);
+
+		$response = new DataResponse(8);
+		$this->assertEquals($response, $this->shareApiController->deleteShare(5, 8));
+	}
+
 	/**
 	 * Delete Non-existing share.
 	 */
@@ -766,9 +861,6 @@ public function testUpdateShare(array $share, string $formOwner, array $keyValue
 			->willReturn($this->createMock(IUser::class));
 
 		$userFolder = $this->createMock(Folder::class);
-		$userFolder->expects($this->any())
-			->method('nodeExists')
-			->willReturn(true);
 
 		$file = $this->createMock(File::class);
 		$file->expects($this->any())