fix(AttachmentService): keep files referenced by id in cleanupAttachments

Signed-off-by: Peter Birrer <peter.birrer@optonic.com>

Author: pbirrer

lib/Service/AttachmentService.php

@@ -352,10 +352,10 @@ public function createAttachmentFile(int $documentId, string $newFileName, strin
 		$fileName = self::getUniqueFileName($saveDir, $newFileName);
 		$newFile = $saveDir->newFile($fileName);
 		return [
-			'name' => $fileName,
+			'name' => $newFile->getName(),
 			'dirname' => $saveDir->getName(),
 			'id' => $newFile->getId(),
-			'documentId' => $newFile->getId(),
+			'documentId' => $textFile->getId(),
 			'mimetype' => $newFile->getMimetype(),
 		];
 	}
@@ -581,23 +581,44 @@ public function cleanupAttachments(int $fileId): int {
 					// this only happens if the attachment dir was deleted by the user while editing the document
 					return 0;
 				}
-				$attachmentsByName = [];
-				foreach ($attachmentDir->getDirectoryListing() as $attNode) {
-					$attachmentsByName[$attNode->getName()] = $attNode;
-				}
-
+				$contentAttachmentFileIds = self::getAttachmentIdsFromContent($textFile->getContent());
 				$contentAttachmentNames = self::getAttachmentNamesFromContent($textFile->getContent(), $fileId);
 
-				$toDelete = array_diff(array_keys($attachmentsByName), $contentAttachmentNames);
-				foreach ($toDelete as $name) {
-					$attachmentsByName[$name]->delete();
+				$toDelete = array_filter($attachmentDir->getDirectoryListing(),
+					function ($node) use ($contentAttachmentFileIds, $contentAttachmentNames) {
+						return !in_array($node->getName(), $contentAttachmentNames) &&
+							!in_array($node->getId(), $contentAttachmentFileIds);
+					}
+				);
+				foreach ($toDelete as $node) {
+					$node->delete();
 				}
 				return count($toDelete);
 			}
 		}
 		return 0;
 	}
 
+	/**
+	 * Get attachment file ids listed in the markdown file content
+	 *
+	 * @param string $content
+	 *
+	 * @return array
+	 */
+	public static function getAttachmentIdsFromContent(string $content): array {
+		$matches = [];
+		// matches [ANY_CONSIDERED_CORRECT_BY_PHP-MARKDOWN](ANY_URL/f/FILE_ID[ (preview)]) and captures FILE_ID
+		preg_match_all(
+			'/\[(?>[^\[\]]+|\[(?>[^\[\]]+|\[(?>[^\[\]]+|\[(?>[^\[\]]+|\[(?>[^\[\]]+|\[(?>[^\[\]]+|\[\])*\])*\])*\])*\])*\])*\]\(\S+\/f\/(\d+)(?: \(preview\))?\)/',
+			$content,
+			$matches,
+			PREG_SET_ORDER
+		);
+		return array_map(static function (array $match) {
+			return intval($match[1]);
+		}, $matches);
+	}
 
 	/**
 	 * Get attachment file names listed in the markdown file content

tests/unit/Service/AttachmentServiceTest.php

@@ -42,6 +42,33 @@ public function testGetAttachmentNamesFromContent() {
 		}
 	}
 
+	public function testGetAttachmentIdsFromContent() {
+		$urls = [
+			'www.example.com',
+			'http://example.com',
+			'https://www.example.com/path/to/page',
+			'http://sub.domain.co.uk/index.php',
+			'https://1.2.3.4:8080/path',
+			'http://localhost:3000/',
+			'https://[2001:db8::1]/ipv6-check',
+		];
+
+		$id = 1;
+		$content = "some content\n";
+		foreach (self::$attachmentNames as $name) {
+			$linkText = preg_replace('/[[\]]/', '', $name);
+			foreach ($urls as $url) {
+				$addon = $id % 2 ? ' (preview)' : '';
+				$content .= "[{$linkText}]({$url}/f/{$id}{$addon})\n";
+				$id++;
+			}
+		}
+		$content .= 'some content';
+
+		$computedIds = AttachmentService::getAttachmentIdsFromContent($content);
+		$this->assertEquals(range(1, $id - 1), $computedIds);
+	}
+
 	public function testGetUniqueFileName() {
 		$fileNameList = [
 			'foo.png',