Use DI and add more tests for storage filter

apps/files/appinfo/app.php

@@ -29,6 +29,9 @@
 
 \OC::$server->getSearch()->registerProvider('OC\Search\Provider\File', ['apps' => ['files']]);
 
+// instantiate to make sure services get registered
+$app = new \OCA\Files\AppInfo\Application();
+
 $templateManager = \OC_Helper::getFileTemplateManager();
 $templateManager->registerTemplate('text/html', 'core/templates/filetemplates/template.html');
 $templateManager->registerTemplate('application/vnd.oasis.opendocument.presentation', 'core/templates/filetemplates/template.odp');

apps/files/lib/AppInfo/Application.php

@@ -87,6 +87,13 @@ public function __construct(array $urlParams= []) {
 			);
 		});
 
+		$container->registerService('OCP\Lock\ILockingProvider', function(IContainer $c)  {
+			return $c->query('ServerContainer')->getLockingProvider();
+		});
+		$container->registerService('OCP\Files\IMimeTypeLoader', function(IContainer $c)  {
+			return $c->query('ServerContainer')->getMimeTypeLoader();
+		});
+
 		/*
 		 * Register capabilities
 		 */

apps/files/lib/Command/Scan.php

@@ -43,20 +43,31 @@
 use OC\Migration\ConsoleOutput;
 use OCP\Lock\ILockingProvider;
 use OCP\Lock\LockedException;
+use OCP\Files\IMimeTypeLoader;
 
 class Scan extends Base {
 
 	/** @var IUserManager $userManager */
 	private $userManager;
+	/** @var ILockingProvider */
+	private $lockingProvider;
+	/** @var IMimeTypeLoader */
+	private $mimeTypeLoader;
 	/** @var float */
 	protected $execTime = 0;
 	/** @var int */
 	protected $foldersCounter = 0;
 	/** @var int */
 	protected $filesCounter = 0;
 
-	public function __construct(IUserManager $userManager) {
+	public function __construct(
+		IUserManager $userManager,
+		ILockingProvider $lockingProvider,
+		IMimeTypeLoader $mimeTypeLoader
+	) {
 		$this->userManager = $userManager;
+		$this->lockingProvider = $lockingProvider;
+		$this->mimeTypeLoader = $mimeTypeLoader;
 		parent::__construct();
 	}
 
@@ -122,25 +133,23 @@ protected function scanFiles($user, $path, $verbose, OutputInterface $output, $b
 		$scanner = new \OC\Files\Utils\Scanner($user, $connection, \OC::$server->getLogger());
 		if ($shouldRepair) {
 			$scanner->listen('\OC\Files\Utils\Scanner', 'beforeScanStorage', function ($storage) use ($output, $connection) {
-				$lockingProvider = \OC::$server->getLockingProvider();
 				try {
 					// FIXME: this will lock the storage even if there is nothing to repair
-					$storage->acquireLock('', ILockingProvider::LOCK_EXCLUSIVE, $lockingProvider);
+					$storage->acquireLock('', ILockingProvider::LOCK_EXCLUSIVE, $this->lockingProvider);
 				} catch (OCP\Lock\LockedException $e) {
 					$output->writeln("\t<error>Storage \"" . $storage->getCache()->getNumericStorageId() . '" cannot be repaired as it is currently in use, please try again later</error>');
 					return;
 				}
 				try {
-					// TODO: use DI
 					$repairStep = new RepairMismatchFileCachePath(
 						$connection,
-						\OC::$server->getMimeTypeLoader()
+						$mimeTypeLoader
 					);
 					$repairStep->setStorageNumericId($storage->getCache()->getNumericStorageId());
 					$repairStep->setCountOnly(false);
 					$repairStep->run(new ConsoleOutput($output));
 				} finally {
-					$storage->releaseLock('', ILockingProvider::LOCK_EXCLUSIVE, $lockingProvider);
+					$storage->releaseLock('', ILockingProvider::LOCK_EXCLUSIVE, $this->lockingProvider);
 				}
 			});
 		}

lib/private/Repair/RepairMismatchFileCachePath.php

@@ -121,7 +121,7 @@ private function fixEntryPath(IOutput $out, $fileId, $wrongPath, $correctStorage
 		$out->advance(1, $text);
 	}
 
-	private function addQueryConditions($qb) {
+	private function addQueryConditionsParentIdWrongPath($qb) {
 		// thanks, VicDeo!
 		if ($this->connection->getDatabasePlatform() instanceof MySqlPlatform) {
 			$concatFunction = $qb->createFunction("CONCAT(fcp.path, '/', fc.name)");
@@ -160,23 +160,64 @@ private function addQueryConditions($qb) {
 		}
 	}
 
-	private function countResultsToProcess() {
+	private function addQueryConditionsNonExistingParentIdEntry($qb) {
+		// Subquery for parent existence
+		$qbe = $this->connection->getQueryBuilder();
+		$qbe->select($qbe->expr()->literal('1'))
+			->from('filecache', 'fce')
+			->where($qbe->expr()->eq('fce.fileid', 'fc.parent'));
+
+		// Find entries to repair
+		// select fc.storage,fc.fileid,fc.parent as "wrongparent",fc.path,fc.etag
+		// and not exists (select 1 from oc_filecache fc2 where fc2.fileid = fc.parent)
+		$qb->select('storage', 'fileid', 'path', 'parent')
+			// from oc_filecache fc
+			->from('filecache', 'fc')
+			// where fc.parent <> -1
+			->where($qb->expr()->neq('fc.parent', $qb->createNamedParameter(-1)))
+			// and not exists (select 1 from oc_filecache fc2 where fc2.fileid = fc.parent)
+			->andWhere(
+				$qb->expr()->orX(
+					$qb->expr()->eq('fc.fileid', 'fc.parent'),
+					$qb->createFunction('NOT EXISTS (' . $qbe->getSQL() . ')')
+				)
+			);
+
+		if ($this->storageNumericId !== null) {
+			// filter by storage but make sure we cover both the potential
+			// source and destination of a failed move
+			$qb->andWhere($qb->expr()->eq('fc.storage', $qb->createNamedParameter($this->storageNumericId)));
+		}
+	}
+
+	private function countResultsToProcessParentIdWrongPath() {
+		$qb = $this->connection->getQueryBuilder();
+		$qb->select($qb->createFunction('COUNT(*)'));
+		$this->addQueryConditionsParentIdWrongPath($qb);
+		$results = $qb->execute();
+		$count = $results->fetchColumn(0);
+		$results->closeCursor();
+		return $count;
+	}
+
+	private function countResultsToProcessNonExistingParentIdEntry() {
 		$qb = $this->connection->getQueryBuilder();
 		$qb->select($qb->createFunction('COUNT(*)'));
-		$this->addQueryConditions($qb);
+		$this->addQueryConditionsNonExistingParentIdEntry($qb);
 		$results = $qb->execute();
 		$count = $results->fetchColumn(0);
 		$results->closeCursor();
 		return $count;
 	}
 
+
 	/**
-	 * Outputs a report about storages that need repairing in the file cache
+	 * Outputs a report about storages with wrong path that need repairing in the file cache
 	 */
-	private function reportAffectedStorages(IOutput $out) {
+	private function reportAffectedStoragesParentIdWrongPath(IOutput $out) {
 		$qb = $this->connection->getQueryBuilder();
 		$qb->selectDistinct('fc.storage');
-		$this->addQueryConditions($qb);
+		$this->addQueryConditionsParentIdWrongPath($qb);
 
 		// TODO: max results + paginate ?
 		// TODO: join with oc_storages / oc_mounts to deliver user id ?
@@ -196,6 +237,32 @@ private function reportAffectedStorages(IOutput $out) {
 		}
 	}
 
+	/**
+	 * Outputs a report about storages with non existing parents that need repairing in the file cache
+	 */
+	private function reportAffectedStoragesNonExistingParentIdEntry(IOutput $out) {
+		$qb = $this->connection->getQueryBuilder();
+		$qb->selectDistinct('fc.storage');
+		$this->addQueryConditionsNonExistingParentIdEntry($qb);
+
+		// TODO: max results + paginate ?
+		// TODO: join with oc_storages / oc_mounts to deliver user id ?
+
+		$results = $qb->execute();
+		$rows = $results->fetchAll();
+		$results->closeCursor();
+
+		$storageIds = [];
+		foreach ($rows as $row) {
+			$storageIds[] = $row['storage'];
+		}
+
+		if (!empty($storageIds)) {
+			$out->warning('The file cache contains entries where the parent id does not point to any existing entry for the following storage numeric ids: ' . implode(' ', $storageIds));
+			$out->warning('Please run `occ files:scan --all --repair` to repair all affected storages');
+		}
+	}
+
 	/**
 	 * Repair all entries for which the parent entry exists but the path
 	 * value doesn't match the parent's path.
@@ -215,7 +282,7 @@ private function fixEntriesWithCorrectParentIdButWrongPath(IOutput $out) {
 			->selectAlias('fc.parent', 'wrongparentid')
 			->selectAlias('fcp.storage', 'parentstorage')
 			->selectAlias('fcp.path', 'parentpath');
-		$this->addQueryConditions($qb);
+		$this->addQueryConditionsParentIdWrongPath($qb);
 		$qb->setMaxResults(self::CHUNK_SIZE);
 
 		do {
@@ -375,35 +442,8 @@ private function fixEntryParent(IOutput $out, $storageId, $fileId, $path, $wrong
 	 * @return int number of results that were fixed
 	 */
 	private function fixEntriesWithNonExistingParentIdEntry(IOutput $out) {
-		// Subquery for parent existence
-		$qbe = $this->connection->getQueryBuilder();
-		$qbe->select($qbe->expr()->literal('1'))
-			->from('filecache', 'fce')
-			->where($qbe->expr()->eq('fce.fileid', 'fc.parent'));
-
 		$qb = $this->connection->getQueryBuilder();
-
-		// Find entries to repair
-		// select fc.storage,fc.fileid,fc.parent as "wrongparent",fc.path,fc.etag
-		// and not exists (select 1 from oc_filecache fc2 where fc2.fileid = fc.parent)
-		$qb->select('storage', 'fileid', 'path', 'parent')
-			// from oc_filecache fc
-			->from('filecache', 'fc')
-			// where fc.parent <> -1
-			->where($qb->expr()->neq('fc.parent', $qb->createNamedParameter(-1)))
-			// and not exists (select 1 from oc_filecache fc2 where fc2.fileid = fc.parent)
-			->andWhere(
-				$qb->expr()->orX(
-					$qb->expr()->eq('fc.fileid', 'fc.parent'),
-					$qb->createFunction('NOT EXISTS (' . $qbe->getSQL() . ')')
-				)
-			);
-
-		if ($this->storageNumericId !== null) {
-			// filter by storage but make sure we cover both the potential
-			// source and destination of a failed move
-			$qb->andWhere($qb->expr()->eq('fc.storage', $qb->createNamedParameter($this->storageNumericId)));
-		}
+		$this->addQueryConditionsNonExistingParentIdEntry($qb);
 		$qb->setMaxResults(self::CHUNK_SIZE);
 
 		$totalResultsCount = 0;
@@ -453,10 +493,12 @@ public function run(IOutput $out) {
 		$this->dirMimePartId = $this->mimeLoader->getId('httpd');
 
 		if ($this->countOnly) {
-			$this->reportAffectedStorages($out);
+			$this->reportAffectedStoragesParentIdWrongPath($out);
+			$this->reportAffectedStoragesNonExistingParentIdEntry($out);
 		} else {
-			$brokenPathEntries = $this->countResultsToProcess();
-			$out->startProgress($brokenPathEntries);
+			$brokenPathEntries = $this->countResultsToProcessParentIdWrongPath();
+			$brokenParentIdEntries = $this->countResultsToProcessNonExistingParentIdEntry();
+			$out->startProgress($brokenPathEntries + $brokenParentIdEntries);
 
 			$totalFixed = 0;
 

lib/private/Server.php

@@ -652,6 +652,7 @@ public function __construct($webRoot, \OC\Config $config) {
 			}
 			return new NoopLockingProvider();
 		});
+		$this->registerAlias('OCP\Lock\ILockingProvider', 'LockingProvider');
 		$this->registerService('MountManager', function () {
 			return new \OC\Files\Mount\Manager();
 		});
@@ -667,6 +668,7 @@ public function __construct($webRoot, \OC\Config $config) {
 				$c->getDatabaseConnection()
 			);
 		});
+		$this->registerAlias('OCP\Files\IMimeTypeLoader', 'MimeTypeLoader');
 		$this->registerService('NotificationManager', function () {
 			return new Manager();
 		});