Merge pull request #7140 from LibreSign/backport/7139/stable33

[stable33] fix: orphan file delete null nodeid

Author: vitormattos

lib/Service/FileService.php

@@ -46,6 +46,7 @@
 use OCP\IUserManager;
 use Psr\Log\LoggerInterface;
 use stdClass;
+use TypeError;
 
 /**
  * @psalm-import-type LibresignEnvelopeChildFile from ResponseDefinitions
@@ -660,22 +661,22 @@ public function delete(int $fileId, bool $deleteFile = true): void {
 		$this->idDocsMapper->deleteByFileId($file->getId());
 		$this->fileMapper->delete($file);
 		if ($deleteFile) {
-			if ($file->getSignedNodeId()) {
-				try {
-					$signedNextcloudFile = $this->folderService->getFileByNodeId($file->getSignedNodeId());
-					$parentFolder = $signedNextcloudFile->getParent();
-					$signedNextcloudFile->delete();
-					$this->deleteEmptyFolder($parentFolder);
-				} catch (NotFoundException) {
-				}
-			}
-			try {
-				$nextcloudFile = $this->folderService->getFileByNodeId($file->getNodeId());
-				$parentFolder = $nextcloudFile->getParent();
-				$nextcloudFile->delete();
-				$this->deleteEmptyFolder($parentFolder);
-			} catch (NotFoundException) {
-			}
+			$this->deletePhysicalFileIfPossible($file->getSignedNodeId());
+			$this->deletePhysicalFileIfPossible($file->getNodeId());
+		}
+	}
+
+	private function deletePhysicalFileIfPossible(?int $nodeId): void {
+		if ($nodeId === null) {
+			return;
+		}
+
+		try {
+			$nextcloudFile = $this->folderService->getFileByNodeId($nodeId);
+			$parentFolder = $nextcloudFile->getParent();
+			$nextcloudFile->delete();
+			$this->deleteEmptyFolder($parentFolder);
+		} catch (NotFoundException|TypeError) {
 		}
 	}
 

tests/php/Unit/Service/FileServiceTest.php

@@ -340,6 +340,53 @@ public function testDeleteWithoutDeletingFile(): void {
 		$service->delete(1, false);
 	}
 
+	public function testDeleteWithNullNodeIdDoesNotCallFolderService(): void {
+		$file = new \OCA\Libresign\Db\File();
+		$file->setId(1);
+		$file->setNodeType('single');
+		$file->setParentFileId(null);
+		$file->setNodeId(null);
+		$file->setSignedNodeId(null);
+
+		$this->fileMapper->method('getById')->willReturn($file);
+		$this->fileMapper->expects($this->once())->method('delete')->with($file);
+
+		$this->folderService->expects($this->never())->method('getFileByNodeId');
+
+		$this->signRequestMapper->method('getByFileId')->willReturn([]);
+		$this->fileElementService->expects($this->once())->method('deleteVisibleElements');
+		$this->idDocsMapper->expects($this->once())->method('deleteByFileId');
+
+		$service = $this->createFileService();
+
+		$service->delete(1, true);
+	}
+
+	public function testDeleteIgnoresMissingPhysicalFile(): void {
+		$file = new \OCA\Libresign\Db\File();
+		$file->setId(1);
+		$file->setNodeId(100);
+		$file->setSignedNodeId(null);
+		$file->setNodeType('single');
+		$file->setParentFileId(null);
+
+		$this->fileMapper->method('getById')->willReturn($file);
+		$this->fileMapper->expects($this->once())->method('delete')->with($file);
+
+		$this->folderService->expects($this->once())
+			->method('getFileByNodeId')
+			->with(100)
+			->willThrowException(new \OCP\Files\NotFoundException());
+
+		$this->signRequestMapper->method('getByFileId')->willReturn([]);
+		$this->fileElementService->expects($this->once())->method('deleteVisibleElements');
+		$this->idDocsMapper->expects($this->once())->method('deleteByFileId');
+
+		$service = $this->createFileService();
+
+		$service->delete(1, true);
+	}
+
 	#[DataProvider('providerTestVisibleElements')]
 	public function testVisibleElements(bool $showVisibleElementsFlag, bool $expectedInResult): void {
 		$file = new \OCA\Libresign\Db\File();