fix: better error message when trying to scan a folder that is already being scanned

Signed-off-by: Robin Appelman <robin@icewind.nl>

Author: icewind1991

apps/files/lib/Command/Scan.php

@@ -24,6 +24,7 @@
 use OCP\Files\StorageNotAvailableException;
 use OCP\FilesMetadata\IFilesMetadataManager;
 use OCP\IUserManager;
+use OCP\Lock\LockedException;
 use OCP\Server;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\Console\Helper\Table;
@@ -165,6 +166,12 @@ protected function scanFiles(string $user, string $path, ?string $scanMetadata,
 		} catch (NotFoundException $e) {
 			$output->writeln('<error>Path not found: ' . $e->getMessage() . '</error>');
 			++$this->errorsCounter;
+		} catch (LockedException $e) {
+			if (str_starts_with($e->getPath(), 'scanner::')) {
+				$output->writeln('<error>Another process is already scanning \'' . substr($e->getPath(), strlen('scanner::')) . '\'</error>');
+			} else {
+				throw $e;
+			}
 		} catch (\Exception $e) {
 			$output->writeln('<error>Exception during scan: ' . $e->getMessage() . '</error>');
 			$output->writeln('<error>' . $e->getTraceAsString() . '</error>');

apps/files_external/lib/Command/Scan.php

@@ -11,6 +11,7 @@
 use OC\Files\Cache\Scanner;
 use OCA\Files_External\Service\GlobalStoragesService;
 use OCP\IUserManager;
+use OCP\Lock\LockedException;
 use Symfony\Component\Console\Helper\Table;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
@@ -82,7 +83,19 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			$this->abortIfInterrupted();
 		});
 
-		$scanner->scan($path);
+		try {
+			$scanner->scan($path);
+		} catch (LockedException $e) {
+			if (is_string($e->getReadablePath()) && str_starts_with($e->getReadablePath(), 'scanner::')) {
+				if ($e->getReadablePath() === 'scanner::') {
+					$output->writeln('<error>Another process is already scanning this storage</error>');
+				} else {
+					$output->writeln('<error>Another process is already scanning \'' . substr($e->getReadablePath(), strlen('scanner::')) . '\' in this storage</error>');
+				}
+			} else {
+				throw $e;
+			}
+		}
 
 		$this->presentStats($output);
 

lib/private/Files/Cache/Scanner.php

@@ -210,7 +210,7 @@ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData =
 					 * @var \OC\Files\Cache\CacheEntry $cacheData
 					 */
 					$newData = $this->array_diff_assoc_multi($data, $cacheData->getData());
-					
+
 					// make it known to the caller that etag has been changed and needs propagation
 					if (isset($newData['etag'])) {
 						$data['etag_changed'] = true;
@@ -312,6 +312,7 @@ public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $loc
 		}
 
 		if ($lock && $this->storage->instanceOfStorage(ILockingStorage::class)) {
+			$this->storage->acquireLock('scanner::' . $path, ILockingProvider::LOCK_EXCLUSIVE, $this->lockingProvider);
 			$this->storage->acquireLock('scanner::' . $path, ILockingProvider::LOCK_EXCLUSIVE, $this->lockingProvider);
 			$this->storage->acquireLock($path, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
 		}
@@ -351,23 +352,23 @@ public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $loc
 	 *
 	 */
 	protected function array_diff_assoc_multi(array $array1, array $array2) {
-		
+
 		$result = [];
 
 		foreach ($array1 as $key => $value) {
-		
+
 			// if $array2 doesn't have the same key, that's a result
 			if (!array_key_exists($key, $array2)) {
 				$result[$key] = $value;
 				continue;
 			}
-		
+
 			// if $array2's value for the same key is different, that's a result
 			if ($array2[$key] !== $value && !is_array($value)) {
 				$result[$key] = $value;
 				continue;
 			}
-		
+
 			if (is_array($value)) {
 				$nestedDiff = $this->array_diff_assoc_multi($value, $array2[$key]);
 				if (!empty($nestedDiff)) {

lib/private/Files/Utils/Scanner.php

@@ -29,6 +29,7 @@
 use OCP\Files\StorageNotAvailableException;
 use OCP\IDBConnection;
 use OCP\Lock\ILockingProvider;
+use OCP\Lock\LockedException;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -260,7 +261,15 @@ public function scan($dir = '', $recursive = \OC\Files\Cache\Scanner::SCAN_RECUR
 			try {
 				$propagator = $storage->getPropagator();
 				$propagator->beginBatch();
-				$scanner->scan($relativePath, $recursive, \OC\Files\Cache\Scanner::REUSE_ETAG | \OC\Files\Cache\Scanner::REUSE_SIZE);
+				try {
+					$scanner->scan($relativePath, $recursive, \OC\Files\Cache\Scanner::REUSE_ETAG | \OC\Files\Cache\Scanner::REUSE_SIZE);
+				} catch (LockedException $e) {
+					if (is_string($e->getReadablePath()) && str_starts_with($e->getReadablePath(), 'scanner::')) {
+						throw new LockedException("scanner::$dir", $e, $e->getExistingLock());
+					} else {
+						throw $e;
+					}
+				}
 				$cache = $storage->getCache();
 				if ($cache instanceof Cache) {
 					// only re-calculate for the root folder we scanned, anything below that is taken care of by the scanner

lib/public/Lock/LockedException.php

@@ -24,6 +24,8 @@ class LockedException extends \Exception {
 	/** @var string|null */
 	private $existingLock;
 
+	private ?string $readablePath;
+
 	/**
 	 * LockedException constructor.
 	 *
@@ -34,6 +36,7 @@ class LockedException extends \Exception {
 	 * @since 8.1.0
 	 */
 	public function __construct(string $path, ?\Exception $previous = null, ?string $existingLock = null, ?string $readablePath = null) {
+		$this->readablePath = $readablePath;
 		if ($readablePath) {
 			$message = "\"$path\"(\"$readablePath\") is locked";
 		} else {
@@ -62,4 +65,13 @@ public function getPath(): string {
 	public function getExistingLock(): ?string {
 		return $this->existingLock;
 	}
+
+	/**
+	 * @return ?string
+	 * @since 32.0.0
+	 */
+	public function getReadablePath(): ?string {
+		return $this->readablePath;
+	}
+
 }