merge '(n)' suffix PR

Signed-off-by: Julien Veyssier <eneiluj@posteo.net>

Author: Julien Veyssier

cypress/integration/images.spec.js

@@ -66,12 +66,12 @@ const clickOnImageAction = (actionIndex, callback) => {
  */
 const checkImage = (documentId, imageName, imageId) => {
 	cy.log('Check the image is visible and well formed')
-	cy.get('#editor .ProseMirror div.image[data-src="text://image?imageFileName=' + imageName + '"]')
+	cy.get('#editor .ProseMirror div.image[data-src="text://media?mediaFileName=' + imageName + '"]')
 		.should('be.visible')
 		.find('img')
 			.should('have.attr', 'src')
-				.should('contain', 'apps/text/image?documentId=' + documentId)
-				.should('contain', 'imageFileName=' + imageName)
+				.should('contain', 'apps/text/media?documentId=' + documentId)
+				.should('contain', 'mediaFileName=' + imageName)
 	// keep track that we have created this image in the attachment dir
 	attachmentFileNameToId[imageName] = imageId
 }

lib/Service/MediaService.php

@@ -147,7 +147,7 @@ public function uploadMediaFile(int $documentId, string $newFileName, $newFileRe
 			throw new NotPermittedException('No write permissions');
 		}
 		$saveDir = $this->getAttachmentDirectoryForFile($textFile, true);
-		$fileName = (string) time() . '-' . $newFileName;
+		$fileName = $this->getUniqueFileName($saveDir, $newFileName);
 		$savedFile = $saveDir->newFile($fileName, $newFileResource);
 		return [
 			'name' => $fileName,
@@ -174,7 +174,7 @@ public function uploadMediaFilePublic(?int $documentId, string $newFileName, $ne
 		}
 		$textFile = $this->getTextFilePublic($documentId, $shareToken);
 		$saveDir = $this->getAttachmentDirectoryForFile($textFile, true);
-		$fileName = (string) time() . '-' . $newFileName;
+		$fileName = $this->getUniqueFileName($saveDir, $newFileName);
 		$savedFile = $saveDir->newFile($fileName, $newFileResource);
 		return [
 			'name' => $fileName,
@@ -216,7 +216,7 @@ public function insertMediaFile(int $documentId, string $mediaFilePath, string $
 	private function copyMediaFile(File $mediaFile, Folder $saveDir, File $textFile): array {
 		$mimeType = $mediaFile->getMimeType();
 		if (in_array($mimeType, MediaController::IMAGE_MIME_TYPES, true)) {
-			$fileName = (string) time() . '-' . $mediaFile->getName();
+			$fileName = $this->getUniqueFileName($saveDir, $mediaFile->getName());
 			$targetPath = $saveDir->getPath() . '/' . $fileName;
 			$targetFile = $mediaFile->copy($targetPath);
 			// get file type and name
@@ -232,6 +232,30 @@ private function copyMediaFile(File $mediaFile, Folder $saveDir, File $textFile)
 		];
 	}
 
+	/**
+	 * Get unique file name in a directory. Add '(n)' suffix.
+	 * @param Folder $dir
+	 * @param string $fileName
+	 * @return string
+	 */
+	public static function getUniqueFileName(Folder $dir, string $fileName): string {
+		$extension = pathinfo($fileName, PATHINFO_EXTENSION);
+		$counter = 1;
+		$uniqueFileName = $fileName;
+		if ($extension !== '') {
+			while ($dir->nodeExists($uniqueFileName)) {
+				$counter++;
+				$uniqueFileName = preg_replace('/\.' . $extension . '$/', ' (' . $counter . ').' . $extension, $fileName);
+			}
+		} else {
+			while ($dir->nodeExists($uniqueFileName)) {
+				$counter++;
+				$uniqueFileName = preg_replace('/$/', ' (' . $counter . ')', $fileName);
+			}
+		}
+		return $uniqueFileName;
+	}
+
 	/**
 	 * Check if the shared access has write permissions
 	 *

tests/TextTest.php

@@ -51,4 +51,40 @@ public function testGetAttachmentNamesFromContent() {
 			$this->assertContains($contentName, $computedNames);
 		}
 	}
+
+	public function testGetUniqueFileName() {
+		$fileNameList = [
+			'foo.png',
+			'bar',
+			'plop.png',
+			'plop (2).png',
+			'lala.png',
+			'lala (2).png',
+			'lala (3).png',
+			'yay.png',
+			'yay (2).png',
+			'yay (3).png',
+			'yay (5).png',
+			'file.ext.ext',
+		];
+
+		$folder = $this->createMock(Folder::class);
+		$folder->expects(self::any())
+			->method('nodeExists')
+			->willReturnCallback(function ($name) use ($fileNameList) {
+				return in_array($name, $fileNameList);
+			});
+
+		// files that do not exist yet
+		$this->assertEquals('doesNotExistYet', ImageService::getUniqueFileName($folder, 'doesNotExistYet'));
+		$this->assertEquals('doesNotExistYet.png', ImageService::getUniqueFileName($folder, 'doesNotExistYet.png'));
+
+		// files that already exist
+		$this->assertEquals('foo (2).png', ImageService::getUniqueFileName($folder, 'foo.png'));
+		$this->assertEquals('bar (2)', ImageService::getUniqueFileName($folder, 'bar'));
+		$this->assertEquals('plop (3).png', ImageService::getUniqueFileName($folder, 'plop.png'));
+		$this->assertEquals('lala (4).png', ImageService::getUniqueFileName($folder, 'lala.png'));
+		$this->assertEquals('yay (4).png', ImageService::getUniqueFileName($folder, 'yay.png'));
+		$this->assertEquals('file.ext (2).ext', ImageService::getUniqueFileName($folder, 'file.ext.ext'));
+	}
 }

tests/psalm-baseline.xml

@@ -177,7 +177,7 @@
       <code>null</code>
     </NullableReturnStatement>
   </file>
-  <file src="lib/Service/ImageService.php">
+  <file src="lib/Service/MediaService.php">
     <InvalidCatch occurrences="2"/>
     <MissingDependency occurrences="8">
       <code>$this-&gt;rootFolder</code>