From 6759366d120160b04b50cd12decc65f42df36628 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Mon, 9 May 2022 11:54:24 +0200 Subject: [PATCH 1/2] refs #2338 use '(n)' suffix instead of timestamp prefix to make sure attachment names are unique Signed-off-by: Julien Veyssier --- lib/Service/ImageService.php | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/lib/Service/ImageService.php b/lib/Service/ImageService.php index df22d374f85..69b65a0edf5 100644 --- a/lib/Service/ImageService.php +++ b/lib/Service/ImageService.php @@ -152,7 +152,7 @@ public function uploadImage(int $documentId, string $newFileName, $newFileResour 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, @@ -179,7 +179,7 @@ public function uploadImagePublic(?int $documentId, string $newFileName, $newFil } $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, @@ -221,7 +221,7 @@ public function insertImageFile(int $documentId, string $path, string $userId): private function copyImageFile(File $imageFile, Folder $saveDir, File $textFile): array { $mimeType = $imageFile->getMimeType(); if (in_array($mimeType, ImageController::IMAGE_MIME_TYPES, true)) { - $fileName = (string) time() . '-' . $imageFile->getName(); + $fileName = $this->getUniqueFileName($saveDir, $imageFile->getName()); $targetPath = $saveDir->getPath() . '/' . $fileName; $targetFile = $imageFile->copy($targetPath); // get file type and name @@ -236,6 +236,23 @@ private function copyImageFile(File $imageFile, Folder $saveDir, File $textFile) ]; } + /** + * Get unique file name in a directory. Add '(n)' suffix. + * @param Folder $dir + * @param string $fileName + * @return string + */ + private function getUniqueFileName(Folder $dir, string $fileName): string { + $extension = pathinfo($fileName, PATHINFO_EXTENSION); + $counter = 1; + $uniqueFileName = $fileName; + while ($dir->nodeExists($uniqueFileName)) { + $counter++; + $uniqueFileName = preg_replace('/\.' . $extension . '$/', ' (' . $counter . ').' . $extension, $fileName); + } + return $uniqueFileName; + } + /** * Check if the shared access has write permissions * From c0c29644b51c23824ef86e859e2179e778fef9a6 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Mon, 9 May 2022 18:20:46 +0200 Subject: [PATCH 2/2] refs #2338 improve and test ImageService::getUniqueFileName() Signed-off-by: Julien Veyssier --- lib/Service/ImageService.php | 15 +++++++++++---- tests/TextTest.php | 37 ++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/lib/Service/ImageService.php b/lib/Service/ImageService.php index 69b65a0edf5..8c2bc1ee2c9 100644 --- a/lib/Service/ImageService.php +++ b/lib/Service/ImageService.php @@ -242,13 +242,20 @@ private function copyImageFile(File $imageFile, Folder $saveDir, File $textFile) * @param string $fileName * @return string */ - private function getUniqueFileName(Folder $dir, string $fileName): string { + public static function getUniqueFileName(Folder $dir, string $fileName): string { $extension = pathinfo($fileName, PATHINFO_EXTENSION); $counter = 1; $uniqueFileName = $fileName; - while ($dir->nodeExists($uniqueFileName)) { - $counter++; - $uniqueFileName = preg_replace('/\.' . $extension . '$/', ' (' . $counter . ').' . $extension, $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; } diff --git a/tests/TextTest.php b/tests/TextTest.php index aeaa8d0cc12..161446b8c81 100644 --- a/tests/TextTest.php +++ b/tests/TextTest.php @@ -4,6 +4,7 @@ use OCA\Text\AppInfo\Application; use OCA\Text\Service\ImageService; +use OCP\Files\Folder; class TextTest extends \PHPUnit\Framework\TestCase { public function testDummy() { @@ -34,4 +35,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')); + } }