fix(trashbin): avoid full filecache scan when restoring versions (#31682)

apps/files_trashbin/lib/Trashbin.php

@@ -680,7 +680,7 @@ private static function restoreVersionsFromTrashbin(View $view, $filename, $targ
 					$metaStorage->renameOrCopy('rename', $src . MetaStorage::VERSION_FILE_EXT, $user, $dst . MetaStorage::VERSION_FILE_EXT, $owner);
 				}
 
-				$versions = self::getVersionsFromTrash($filenameOnlyWithoutTimestamp, $timestamp, $user);
+				$versions = self::getVersionsFromTrash($filenameOnlyWithoutTimestamp, $timestamp, $user, $dir);
 				foreach ($versions as $v) {
 					if ($timestamp) {
 						$src = '/files_trashbin/versions/' . $dirAndFilename . '.v' . $v . '.d' . $timestamp;
@@ -846,7 +846,7 @@ private static function deleteVersions(View $view, $file, $user) {
 					$filenameOnlyWithoutTimestamp = $filenameOnly;
 					$dirAndFilename = "{$dir}/{$filenameOnly}";
 				}
-				$versions = self::getVersionsFromTrash($filenameOnlyWithoutTimestamp, $timestamp, $user);
+				$versions = self::getVersionsFromTrash($filenameOnlyWithoutTimestamp, $timestamp, $user, $dir);
 				foreach ($versions as $v) {
 					if ($timestamp) {
 						$size += $view->filesize('/files_trashbin/versions/' . $dirAndFilename . '.v' . $v . '.d' . $timestamp);
@@ -957,9 +957,14 @@ private static function copy_recursive($source, $destination, View $view) {
 	 *
 	 * @param string $filename name of the file which should be restored
 	 * @param int $timestamp timestamp when the file was deleted
+	 * @param string $user
+	 * @param string $dir directory the file lives in, relative to the
+	 *                     "files_trashbin/versions" root ('', '/' or '.' for the
+	 *                     root itself). For deleted files that were inside a
+	 *                     folder, their version files live in that same folder.
 	 * @return array
 	 */
-	private static function getVersionsFromTrash($filename, $timestamp, $user) {
+	private static function getVersionsFromTrash($filename, $timestamp, $user, $dir = '') {
 		$view = new View('/' . $user . '/files_trashbin/versions');
 		$versions = [];
 
@@ -971,24 +976,41 @@ private static function getVersionsFromTrash($filename, $timestamp, $user) {
 			self::$scannedVersions = true;
 		}
 
+		// The version files for a deleted file all live in a single, known
+		// directory under "files_trashbin/versions" (the folder the file was in,
+		// or the versions root). Instead of running a non-sargable
+		// trailing-wildcard name search across the whole filecache (full table
+		// scan on MySQL, see issue #31682), list that one directory using the
+		// index-backed (parent, name) lookup and filter the matching
+		// "<filename>.v<version>[.d<timestamp>]" entries in PHP.
+		$lookupDir = ($dir === '/' || $dir === '.') ? '' : \ltrim($dir, '/');
+		$prefix = $filename . '.v';
+
 		if ($timestamp) {
-			// fetch for old versions
-			$matches = $view->searchRaw($filename . '.v%.d' . $timestamp);
+			// match "<filename>.v<version>.d<timestamp>"
+			$suffix = '.d' . $timestamp;
 			$offset = -\strlen($timestamp) - 2;
 		} else {
-			$matches = $view->searchRaw($filename . '.v%');
+			// match "<filename>.v<version>"
+			$suffix = '';
+			$offset = 0;
 		}
 
-		if (\is_array($matches)) {
-			foreach ($matches as $ma) {
-				if ($timestamp) {
-					$parts = \explode('.v', \substr($ma['path'], 0, $offset));
-					$versions[] = (\end($parts));
-				} else {
-					$parts = \explode('.v', $ma['path']);
-					$versions[] = (\end($parts));
+		foreach ($view->getDirectoryContent($lookupDir) as $entry) {
+			$name = $entry->getName();
+			if (\strpos($name, $prefix) !== 0) {
+				continue;
+			}
+			if ($suffix !== '') {
+				// must end with the deletion timestamp suffix
+				if (\substr($name, $offset) !== $suffix) {
+					continue;
 				}
+				$parts = \explode('.v', \substr($name, 0, $offset));
+			} else {
+				$parts = \explode('.v', $name);
 			}
+			$versions[] = (\end($parts));
 		}
 		return $versions;
 	}

apps/files_trashbin/tests/StorageTest.php

@@ -910,6 +910,72 @@ public function testSingleStorageDeleteFileLoggedOut() {
 		}
 	}
 
+	/**
+	 * Regression test for issue #31682.
+	 *
+	 * Trashbin::getVersionsFromTrash() must return the versions of one specific
+	 * deleted file living in "files_trashbin/versions" and ignore version files
+	 * that belong to other (similarly named) files or other deletion timestamps.
+	 * This is exercised here against the real filecache, independent of the
+	 * lookup strategy (index-backed directory listing instead of a wildcard
+	 * name search).
+	 */
+	public function testGetVersionsFromTrash() {
+		$timestamp = 1234567890;
+		$versionsDir = $this->user . '/files_trashbin/versions';
+		$this->rootView->mkdir($versionsDir);
+
+		// versions of the file we want to look up
+		$this->rootView->file_put_contents($versionsDir . '/test.txt.v1.d' . $timestamp, 'v1');
+		$this->rootView->file_put_contents($versionsDir . '/test.txt.v2.d' . $timestamp, 'v2');
+		$this->rootView->file_put_contents($versionsDir . '/test.txt.v10.d' . $timestamp, 'v10');
+
+		// entries that must be ignored:
+		// - same file name but a different deletion timestamp
+		$this->rootView->file_put_contents($versionsDir . '/test.txt.v3.d999', 'other-ts');
+		// - a different file whose name shares the prefix
+		$this->rootView->file_put_contents($versionsDir . '/test.txt.backup.v9.d' . $timestamp, 'other-file');
+		// - an unrelated file
+		$this->rootView->file_put_contents($versionsDir . '/unrelated.txt.v1.d' . $timestamp, 'unrelated');
+
+		// a file deleted from inside a folder keeps its versions in that same
+		// sub-folder of "files_trashbin/versions" (non-timestamped lookup path).
+		$this->rootView->mkdir($versionsDir . '/folder');
+		$this->rootView->file_put_contents($versionsDir . '/folder/nested.txt.v5', 'nested-v5');
+		$this->rootView->file_put_contents($versionsDir . '/folder/nested.txt.v6', 'nested-v6');
+
+		// make sure the cache knows about the files we just created
+		list($storage, ) = $this->rootView->resolvePath($versionsDir);
+		$storage->getScanner()->scan('files_trashbin/versions');
+
+		$method = new \ReflectionMethod(\OCA\Files_Trashbin\Trashbin::class, 'getVersionsFromTrash');
+		$method->setAccessible(true);
+
+		// reset the per-request "already scanned" guard so the lookup re-scans
+		$scannedVersions = new \ReflectionProperty(\OCA\Files_Trashbin\Trashbin::class, 'scannedVersions');
+		$scannedVersions->setAccessible(true);
+
+		// timestamped lookup in the versions root
+		// (sort() uses SORT_REGULAR, so the numeric version strings sort numerically)
+		$scannedVersions->setValue(null, false);
+		$versions = $method->invoke(null, 'test.txt', $timestamp, $this->user);
+		\sort($versions);
+		$this->assertEquals(['1', '2', '10'], $versions);
+
+		// non-timestamped lookup inside a sub-folder (issue #31682 regression:
+		// the previous whole-cache name search found these, the directory
+		// listing must look in the right sub-folder, not just the root)
+		$scannedVersions->setValue(null, false);
+		$versions = $method->invoke(null, 'nested.txt', null, $this->user, 'folder');
+		\sort($versions);
+		$this->assertEquals(['5', '6'], $versions);
+
+		// a file with no stored versions yields an empty result
+		$scannedVersions->setValue(null, false);
+		$versions = $method->invoke(null, 'no-such-file.txt', $timestamp, $this->user);
+		$this->assertEquals([], $versions);
+	}
+
 	private function markTestSkippedIfStorageHasOwnVersioning() {
 		/** @var Storage $storage */
 		list($storage, $internalPath) = $this->userView->resolvePath('folder/inside.txt');