refactor(OC_Helper): remove buildNotExistingFileNameForView

Move the functionality in the last place it is used OC\Files\Node\Folder

Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>

apps/files_sharing/lib/External/Manager.php

@@ -8,7 +8,6 @@
 
 namespace OCA\Files_Sharing\External;
 
-use HttpResponse;
 use OC\Files\Filesystem;
 use OC\Files\SetupManager;
 use OC\User\NoUserException;
@@ -28,7 +27,6 @@ use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
 use OCP\Files\Storage\IStorageFactory;
 use OCP\Http\Client\IClientService;
-use OCP\Http\Client\IResponse;
 use OCP\ICertificateManager;
 use OCP\IDBConnection;
 use OCP\IGroup;

lib/private/Files/Node/Folder.php

@@ -391,13 +391,50 @@ class Folder extends Node implements IFolder {
 	/**
 	 * Add a suffix to the name in case the file exists
 	 *
-	 * @param string $name
+	 * @param string $filename
 	 * @return string
 	 * @throws NotPermittedException
 	 */
-	public function getNonExistingName($name) {
-		$uniqueName = \OC_Helper::buildNotExistingFileNameForView($this->getPath(), $name, $this->view);
-		return trim($this->getRelativePath($uniqueName), '/');
+	public function getNonExistingName($filename) {
+		$path = $this->getPath();
+		if ($path === '/') {
+			$path = '';
+		}
+		if ($pos = strrpos($filename, '.')) {
+			$name = substr($filename, 0, $pos);
+			$ext = substr($filename, $pos);
+		} else {
+			$name = $filename;
+			$ext = '';
+		}
+
+		$newpath = $path . '/' . $filename;
+		if ($this->view->file_exists($newpath)) {
+			if (preg_match_all('/\((\d+)\)/', $name, $matches, PREG_OFFSET_CAPTURE)) {
+				/** @var array<int<0, max>, array> $matches */
+				//Replace the last "(number)" with "(number+1)"
+				$last_match = count($matches[0]) - 1;
+				$counter = $matches[1][$last_match][0] + 1;
+				$offset = $matches[0][$last_match][1];
+				$match_length = strlen($matches[0][$last_match][0]);
+			} else {
+				$counter = 2;
+				$match_length = 0;
+				$offset = false;
+			}
+			do {
+				if ($offset) {
+					//Replace the last "(number)" with "(number+1)"
+					$newname = substr_replace($name, '(' . $counter . ')', $offset, $match_length);
+				} else {
+					$newname = $name . ' (' . $counter . ')';
+				}
+				$newpath = $path . '/' . $newname . $ext;
+				$counter++;
+			} while ($this->view->file_exists($newpath));
+		}
+
+		return trim($this->getRelativePath($newpath), '/');
 	}
 
 	/**

lib/private/Files/Node/LazyFolder.php

@@ -493,7 +493,7 @@ class LazyFolder implements Folder {
 	/**
 	 * @inheritDoc
 	 */
-	public function getNonExistingName($name) {
+	public function getNonExistingName($filename) {
 		return $this->__call(__FUNCTION__, func_get_args());
 	}
 

lib/private/legacy/OC_Helper.php

@@ -105,65 +105,6 @@ class OC_Helper {
 		return false;
 	}
 
-	/**
-	 * Adds a suffix to the name in case the file exists
-	 *
-	 * @param string $path
-	 * @param string $filename
-	 * @return string
-	 */
-	public static function buildNotExistingFileName($path, $filename) {
-		$view = \OC\Files\Filesystem::getView();
-		return self::buildNotExistingFileNameForView($path, $filename, $view);
-	}
-
-	/**
-	 * Adds a suffix to the name in case the file exists
-	 *
-	 * @param string $path
-	 * @param string $filename
-	 * @return string
-	 */
-	public static function buildNotExistingFileNameForView($path, $filename, \OC\Files\View $view) {
-		if ($path === '/') {
-			$path = '';
-		}
-		if ($pos = strrpos($filename, '.')) {
-			$name = substr($filename, 0, $pos);
-			$ext = substr($filename, $pos);
-		} else {
-			$name = $filename;
-			$ext = '';
-		}
-
-		$newpath = $path . '/' . $filename;
-		if ($view->file_exists($newpath)) {
-			if (preg_match_all('/\((\d+)\)/', $name, $matches, PREG_OFFSET_CAPTURE)) {
-				//Replace the last "(number)" with "(number+1)"
-				$last_match = count($matches[0]) - 1;
-				$counter = $matches[1][$last_match][0] + 1;
-				$offset = $matches[0][$last_match][1];
-				$match_length = strlen($matches[0][$last_match][0]);
-			} else {
-				$counter = 2;
-				$match_length = 0;
-				$offset = false;
-			}
-			do {
-				if ($offset) {
-					//Replace the last "(number)" with "(number+1)"
-					$newname = substr_replace($name, '(' . $counter . ')', $offset, $match_length);
-				} else {
-					$newname = $name . ' (' . $counter . ')';
-				}
-				$newpath = $path . '/' . $newname . $ext;
-				$counter++;
-			} while ($view->file_exists($newpath));
-		}
-
-		return $newpath;
-	}
-
 	/**
 	 * Checks if a function is available
 	 *

lib/public/Files.php

@@ -126,16 +126,4 @@ class Files {
 		}
 		return $includeResult ? [$count, $result] : $count;
 	}
-
-	/**
-	 * Adds a suffix to the name in case the file exists
-	 * @param string $path
-	 * @param string $filename
-	 * @return string
-	 * @since 5.0.0
-	 * @deprecated 14.0.0 use getNonExistingName of the OCP\Files\Folder object
-	 */
-	public static function buildNotExistingFileName($path, $filename) {
-		return \OC_Helper::buildNotExistingFileName($path, $filename);
-	}
 }

lib/public/Files/Folder.php

@@ -194,12 +194,12 @@ interface Folder extends Node {
 	/**
 	 * Add a suffix to the name in case the file exists
 	 *
-	 * @param string $name
+	 * @param string $filename
 	 * @return string
 	 * @throws NotPermittedException
 	 * @since 8.1.0
 	 */
-	public function getNonExistingName($name);
+	public function getNonExistingName($filename);
 
 	/**
 	 * @param int $limit

tests/lib/Files/Node/FolderTest.php

@@ -1121,4 +1121,102 @@ class FolderTest extends NodeTestCase {
 		$result = $node->getOrCreateFolder($folderName);
 		$this->assertEquals($child, $result);
 	}
+
+	private function callBuildNotExistingFileNameForView(string $path, string $name, View&MockObject $view): string {
+		$rootFolder = $this->createMock(IRootFolder::class);
+		$folder = new Folder($rootFolder, $view, $path);
+		return $path . (str_ends_with('/', $path) ? '' : '/') . $folder->getNonExistingName($name);
+	}
+
+	public function testBuildNotExistingFileNameForView(): void {
+		$viewMock = $this->createMock(View::class);
+		$this->assertEquals('/filename', $this->callBuildNotExistingFileNameForView('/', 'filename', $viewMock));
+		$this->assertEquals('dir/filename.ext', $this->callBuildNotExistingFileNameForView('dir', 'filename.ext', $viewMock));
+
+		$viewMock = $this->createMock(View::class);
+		$viewMock->expects($this->exactly(2))
+			->method('file_exists')
+			->willReturnMap([
+				// Conflict on filename.ext
+				['dir/filename.ext', true],
+				['dir/filename (2).ext', false],
+			]);
+		$this->assertEquals('dir/filename (2).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename.ext', $viewMock));
+
+		$viewMock = $this->createMock(View::class);
+		$viewMock->expects($this->exactly(3))
+			->method('file_exists')
+			->willReturnMap([
+				// Conflict on filename.ext
+				['dir/filename.ext', true],
+				['dir/filename (2).ext', true],
+				['dir/filename (3).ext', false],
+			]);
+		$this->assertEquals('dir/filename (3).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename.ext', $viewMock));
+
+		$viewMock = $this->createMock(View::class);
+		$viewMock->expects($this->exactly(2))
+			->method('file_exists')
+			->willReturnMap([
+				['dir/filename (1).ext', true],
+				['dir/filename (2).ext', false],
+			]);
+		$this->assertEquals('dir/filename (2).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename (1).ext', $viewMock));
+
+		$viewMock = $this->createMock(View::class);
+		$viewMock->expects($this->exactly(2))
+			->method('file_exists')
+			->willReturnMap([
+				['dir/filename (2).ext', true],
+				['dir/filename (3).ext', false],
+			]);
+		$this->assertEquals('dir/filename (3).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename (2).ext', $viewMock));
+
+		$viewMock = $this->createMock(View::class);
+		$viewMock->expects($this->exactly(3))
+			->method('file_exists')
+			->willReturnMap([
+				['dir/filename (2).ext', true],
+				['dir/filename (3).ext', true],
+				['dir/filename (4).ext', false],
+			]);
+		$this->assertEquals('dir/filename (4).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename (2).ext', $viewMock));
+
+		$viewMock = $this->createMock(View::class);
+		$viewMock->expects($this->exactly(2))
+			->method('file_exists')
+			->willReturnMap([
+				['dir/filename(1).ext', true],
+				['dir/filename(2).ext', false],
+			]);
+		$this->assertEquals('dir/filename(2).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename(1).ext', $viewMock));
+
+		$viewMock = $this->createMock(View::class);
+		$viewMock->expects($this->exactly(2))
+			->method('file_exists')
+			->willReturnMap([
+				['dir/filename(1) (1).ext', true],
+				['dir/filename(1) (2).ext', false],
+			]);
+		$this->assertEquals('dir/filename(1) (2).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename(1) (1).ext', $viewMock));
+
+		$viewMock = $this->createMock(View::class);
+		$viewMock->expects($this->exactly(3))
+			->method('file_exists')
+			->willReturnMap([
+				['dir/filename(1) (1).ext', true],
+				['dir/filename(1) (2).ext', true],
+				['dir/filename(1) (3).ext', false],
+			]);
+		$this->assertEquals('dir/filename(1) (3).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename(1) (1).ext', $viewMock));
+
+		$viewMock = $this->createMock(View::class);
+		$viewMock->expects($this->exactly(2))
+			->method('file_exists')
+			->willReturnMap([
+				['dir/filename(1) (2) (3).ext', true],
+				['dir/filename(1) (2) (4).ext', false],
+			]);
+		$this->assertEquals('dir/filename(1) (2) (4).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename(1) (2) (3).ext', $viewMock));
+	}
 }

tests/lib/LegacyHelperTest.php

@@ -1,106 +0,0 @@
-<?php
-
-/**
- * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
- * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
- * SPDX-License-Identifier: AGPL-3.0-or-later
- */
-
-namespace Test;
-
-use OC\Files\View;
-use OC_Helper;
-
-class LegacyHelperTest extends \Test\TestCase {
-	public function testBuildNotExistingFileNameForView(): void {
-		$viewMock = $this->createMock(View::class);
-		$this->assertEquals('/filename', OC_Helper::buildNotExistingFileNameForView('/', 'filename', $viewMock));
-		$this->assertEquals('dir/filename.ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename.ext', $viewMock));
-
-		$viewMock = $this->createMock(View::class);
-		$viewMock->expects($this->exactly(2))
-			->method('file_exists')
-			->willReturnMap([
-				// Conflict on filename.ext
-				['dir/filename.ext', true],
-				['dir/filename (2).ext', false],
-			]);
-		$this->assertEquals('dir/filename (2).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename.ext', $viewMock));
-
-		$viewMock = $this->createMock(View::class);
-		$viewMock->expects($this->exactly(3))
-			->method('file_exists')
-			->willReturnMap([
-				// Conflict on filename.ext
-				['dir/filename.ext', true],
-				['dir/filename (2).ext', true],
-				['dir/filename (3).ext', false],
-			]);
-		$this->assertEquals('dir/filename (3).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename.ext', $viewMock));
-
-		$viewMock = $this->createMock(View::class);
-		$viewMock->expects($this->exactly(2))
-			->method('file_exists')
-			->willReturnMap([
-				['dir/filename (1).ext', true],
-				['dir/filename (2).ext', false],
-			]);
-		$this->assertEquals('dir/filename (2).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename (1).ext', $viewMock));
-
-		$viewMock = $this->createMock(View::class);
-		$viewMock->expects($this->exactly(2))
-			->method('file_exists')
-			->willReturnMap([
-				['dir/filename (2).ext', true],
-				['dir/filename (3).ext', false],
-			]);
-		$this->assertEquals('dir/filename (3).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename (2).ext', $viewMock));
-
-		$viewMock = $this->createMock(View::class);
-		$viewMock->expects($this->exactly(3))
-			->method('file_exists')
-			->willReturnMap([
-				['dir/filename (2).ext', true],
-				['dir/filename (3).ext', true],
-				['dir/filename (4).ext', false],
-			]);
-		$this->assertEquals('dir/filename (4).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename (2).ext', $viewMock));
-
-		$viewMock = $this->createMock(View::class);
-		$viewMock->expects($this->exactly(2))
-			->method('file_exists')
-			->willReturnMap([
-				['dir/filename(1).ext', true],
-				['dir/filename(2).ext', false],
-			]);
-		$this->assertEquals('dir/filename(2).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename(1).ext', $viewMock));
-
-		$viewMock = $this->createMock(View::class);
-		$viewMock->expects($this->exactly(2))
-			->method('file_exists')
-			->willReturnMap([
-				['dir/filename(1) (1).ext', true],
-				['dir/filename(1) (2).ext', false],
-			]);
-		$this->assertEquals('dir/filename(1) (2).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename(1) (1).ext', $viewMock));
-
-		$viewMock = $this->createMock(View::class);
-		$viewMock->expects($this->exactly(3))
-			->method('file_exists')
-			->willReturnMap([
-				['dir/filename(1) (1).ext', true],
-				['dir/filename(1) (2).ext', true],
-				['dir/filename(1) (3).ext', false],
-			]);
-		$this->assertEquals('dir/filename(1) (3).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename(1) (1).ext', $viewMock));
-
-		$viewMock = $this->createMock(View::class);
-		$viewMock->expects($this->exactly(2))
-			->method('file_exists')
-			->willReturnMap([
-				['dir/filename(1) (2) (3).ext', true],
-				['dir/filename(1) (2) (4).ext', false],
-			]);
-		$this->assertEquals('dir/filename(1) (2) (4).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename(1) (2) (3).ext', $viewMock));
-	}
-}