From b66caa92ab8cde144969692d27bc2ba80df3d530 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 26 Jul 2017 11:15:25 +0200 Subject: [PATCH 01/13] Repair detached filecache entries through occ files:scan --- apps/files/lib/Command/Scan.php | 43 +- lib/private/Files/Utils/Scanner.php | 7 +- .../Repair/RepairMismatchFileCachePath.php | 475 +++++++++++++++++ .../RepairMismatchFileCachePathTest.php | 496 ++++++++++++++++++ 4 files changed, 1016 insertions(+), 5 deletions(-) create mode 100644 lib/private/Repair/RepairMismatchFileCachePath.php create mode 100644 tests/lib/Repair/RepairMismatchFileCachePathTest.php diff --git a/apps/files/lib/Command/Scan.php b/apps/files/lib/Command/Scan.php index 5693817d3fe4..93042c522585 100644 --- a/apps/files/lib/Command/Scan.php +++ b/apps/files/lib/Command/Scan.php @@ -39,6 +39,10 @@ use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Helper\Table; +use OC\Repair\RepairMismatchFileCachePath; +use OC\Migration\ConsoleOutput; +use OCP\Lock\ILockingProvider; +use OCP\Lock\LockedException; class Scan extends Base { @@ -90,6 +94,12 @@ protected function configure() { null, InputOption::VALUE_NONE, 'will rescan all files of all known users' + ) + ->addOption( + 'repair', + null, + InputOption::VALUE_NONE, + 'will repair detached filecache entries (slow)' )->addOption( 'unscanned', null, @@ -107,9 +117,34 @@ public function checkScanWarning($fullPath, OutputInterface $output) { } } - protected function scanFiles($user, $path, $verbose, OutputInterface $output, $backgroundScan = false) { + protected function scanFiles($user, $path, $verbose, OutputInterface $output, $backgroundScan = false, $shouldRepair = false) { $connection = $this->reconnectToDatabase($output); $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); + } catch (OCP\Lock\LockedException $e) { + $output->writeln("\tStorage \"" . $storage->getCache()->getNumericStorageId() . '" cannot be repaired as it is currently in use, please try again later'); + return; + } + try { + // TODO: use DI + $repairStep = new RepairMismatchFileCachePath( + $connection, + \OC::$server->getMimeTypeLoader() + ); + $repairStep->setStorageNumericId($storage->getCache()->getNumericStorageId()); + $repairStep->setCountOnly(false); + $repairStep->run(new ConsoleOutput($output)); + } finally { + $storage->releaseLock('', ILockingProvider::LOCK_EXCLUSIVE, $lockingProvider); + } + }); + } + # check on each file/folder if there was a user interrupt (ctrl-c) and throw an exception # printout and count if ($verbose) { @@ -156,7 +191,7 @@ protected function scanFiles($user, $path, $verbose, OutputInterface $output, $b if ($backgroundScan) { $scanner->backgroundScan($path); }else { - $scanner->scan($path); + $scanner->scan($path, $shouldRepair); } } catch (ForbiddenException $e) { $output->writeln("Home storage for user $user not writable"); @@ -166,7 +201,7 @@ protected function scanFiles($user, $path, $verbose, OutputInterface $output, $b $output->writeln('Interrupted by user'); return; } catch (\Exception $e) { - $output->writeln('Exception during scan: ' . $e->getMessage() . "\n" . $e->getTraceAsString() . ''); + $output->writeln('Exception during scan: ' . get_class($e) . ': ' . $e->getMessage() . "\n" . $e->getTraceAsString() . ''); } } @@ -225,7 +260,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { if ($verbose) {$output->writeln(""); } $output->writeln("Starting scan for user $user_count out of $users_total ($user)"); # full: printout data if $verbose was set - $this->scanFiles($user, $path, $verbose, $output, $input->getOption('unscanned')); + $this->scanFiles($user, $path, $verbose, $output, $input->getOption('unscanned'), $input->getOption('repair')); } else { $output->writeln("Unknown user $user_count $user"); } diff --git a/lib/private/Files/Utils/Scanner.php b/lib/private/Files/Utils/Scanner.php index b0bd2038590a..6f3490d63540 100644 --- a/lib/private/Files/Utils/Scanner.php +++ b/lib/private/Files/Utils/Scanner.php @@ -196,7 +196,8 @@ public function scan($dir = '') { } // if the home storage isn't writable then the scanner is run as the wrong user - if ($storage->instanceOfStorage('\OC\Files\Storage\Home') and + $isHome = $storage->instanceOfStorage('\OC\Files\Storage\Home'); + if ($isHome and (!$storage->isCreatable('') or !$storage->isCreatable('files')) ) { if ($storage->file_exists('') or $storage->getCache()->inCache('')) { @@ -211,6 +212,9 @@ public function scan($dir = '') { if ($storage->instanceOfStorage('OCA\Files_Sharing\ISharedStorage')) { continue; } + + $this->emit('\OC\Files\Utils\Scanner', 'beforeScanStorage', [$storage]); + $relativePath = $mount->getInternalPath($dir); $scanner = $storage->getScanner(); $scanner->setUseTransactions(false); @@ -247,6 +251,7 @@ public function scan($dir = '') { if ($this->useTransaction) { $this->db->commit(); } + $this->emit('\OC\Files\Utils\Scanner', 'afterScanStorage', [$storage]); } } diff --git a/lib/private/Repair/RepairMismatchFileCachePath.php b/lib/private/Repair/RepairMismatchFileCachePath.php new file mode 100644 index 000000000000..477d0a6a3214 --- /dev/null +++ b/lib/private/Repair/RepairMismatchFileCachePath.php @@ -0,0 +1,475 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Repair; + +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; +use Doctrine\DBAL\Platforms\MySqlPlatform; +use OCP\DB\QueryBuilder\IQueryBuilder; +use Doctrine\DBAL\Platforms\OraclePlatform; +use OCP\Files\IMimeTypeLoader; +use OCP\IDBConnection; + +/** + * Repairs file cache entry which path do not match the parent-child relationship + */ +class RepairMismatchFileCachePath implements IRepairStep { + + const CHUNK_SIZE = 10000; + + /** @var IDBConnection */ + protected $connection; + + /** @var IMimeTypeLoader */ + protected $mimeLoader; + + /** @var int */ + protected $dirMimeTypeId; + + /** @var int */ + protected $dirMimePartId; + + /** @var int|null */ + protected $storageNumericId = null; + + /** @var bool */ + protected $countOnly = true; + + /** + * @param \OCP\IDBConnection $connection + */ + public function __construct(IDBConnection $connection, IMimeTypeLoader $mimeLoader) { + $this->connection = $connection; + $this->mimeLoader = $mimeLoader; + } + + public function getName() { + if ($this->countOnly) { + return 'Detect file cache entries with path that does not match parent-child relationships'; + } else { + return 'Repair file cache entries with path that does not match parent-child relationships'; + } + } + + /** + * Sets the numeric id of the storage to process or null to process all. + * + * @param int $storageNumericId numeric id of the storage + */ + public function setStorageNumericId($storageNumericId) { + $this->storageNumericId = $storageNumericId; + } + + /** + * Sets whether to actually repair or only count entries + * + * @param bool $countOnly count only + */ + public function setCountOnly($countOnly) { + $this->countOnly = $countOnly; + } + + /** + * Fixes the broken entry's path. + * + * @param IOutput $out repair output + * @param int $fileId file id of the entry to fix + * @param string $wrongPath wrong path of the entry to fix + * @param int $correctStorageNumericId numeric idea of the correct storage + * @param string $correctPath value to which to set the path of the entry + * @return bool true for success + */ + private function fixEntryPath(IOutput $out, $fileId, $wrongPath, $correctStorageNumericId, $correctPath) { + // delete target if exists + $qb = $this->connection->getQueryBuilder(); + $qb->delete('filecache') + ->where($qb->expr()->eq('storage', $qb->createNamedParameter($correctStorageNumericId))) + ->andWhere($qb->expr()->eq('path', $qb->createNamedParameter($correctPath))); + $entryExisted = $qb->execute() > 0; + + $qb = $this->connection->getQueryBuilder(); + $qb->update('filecache') + ->set('path', $qb->createNamedParameter($correctPath)) + ->set('path_hash', $qb->createNamedParameter(md5($correctPath))) + ->set('storage', $qb->createNamedParameter($correctStorageNumericId)) + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($fileId))); + $qb->execute(); + + $text = "Fixed file cache entry with fileid $fileId, set wrong path \"$wrongPath\" to \"$correctPath\""; + if ($entryExisted) { + $text = " (replaced an existing entry)"; + } + $out->advance(1, $text); + } + + private function addQueryConditions($qb) { + // thanks, VicDeo! + if ($this->connection->getDatabasePlatform() instanceof MySqlPlatform) { + $concatFunction = $qb->createFunction("CONCAT(fcp.path, '/', fc.name)"); + } else { + $concatFunction = $qb->createFunction("(fcp.`path` || '/' || fc.`name`)"); + } + + if ($this->connection->getDatabasePlatform() instanceof OraclePlatform) { + $emptyPathExpr = $qb->expr()->isNotNull('fcp.path'); + } else { + $emptyPathExpr = $qb->expr()->neq('fcp.path', $qb->expr()->literal('')); + } + + $qb + ->from('filecache', 'fc') + ->from('filecache', 'fcp') + ->where($qb->expr()->eq('fc.parent', 'fcp.fileid')) + ->andWhere( + $qb->expr()->orX( + $qb->expr()->neq( + $qb->createFunction($concatFunction), + 'fc.path' + ), + $qb->expr()->neq('fc.storage', 'fcp.storage') + ) + ) + ->andWhere($emptyPathExpr) + // yes, this was observed in the wild... + ->andWhere($qb->expr()->neq('fc.fileid', 'fcp.fileid')); + + if ($this->storageNumericId !== null) { + // use the source storage of the failed move when filtering + $qb->andWhere( + $qb->expr()->eq('fc.storage', $qb->createNamedParameter($this->storageNumericId)) + ); + } + } + + private function countResultsToProcess() { + $qb = $this->connection->getQueryBuilder(); + $qb->select($qb->createFunction('COUNT(*)')); + $this->addQueryConditions($qb); + $results = $qb->execute(); + $count = $results->fetchColumn(0); + $results->closeCursor(); + return $count; + } + + /** + * Outputs a report about storages that need repairing in the file cache + */ + private function reportAffectedStorages(IOutput $out) { + $qb = $this->connection->getQueryBuilder(); + $qb->selectDistinct('fc.storage'); + $this->addQueryConditions($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 with invalid path values 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. + * + * @param IOutput $out + * @return int number of results that were fixed + */ + private function fixEntriesWithCorrectParentIdButWrongPath(IOutput $out) { + $totalResultsCount = 0; + + // find all entries where the path entry doesn't match the path value that would + // be expected when following the parent-child relationship, basically + // concatenating the parent's "path" value with the name of the child + $qb = $this->connection->getQueryBuilder(); + $qb->select('fc.storage', 'fc.fileid', 'fc.name') + ->selectAlias('fc.path', 'path') + ->selectAlias('fc.parent', 'wrongparentid') + ->selectAlias('fcp.storage', 'parentstorage') + ->selectAlias('fcp.path', 'parentpath'); + $this->addQueryConditions($qb); + $qb->setMaxResults(self::CHUNK_SIZE); + + do { + $results = $qb->execute(); + // since we're going to operate on fetched entry, better cache them + // to avoid DB lock ups + $rows = $results->fetchAll(); + $results->closeCursor(); + + $this->connection->beginTransaction(); + $lastResultsCount = 0; + foreach ($rows as $row) { + $wrongPath = $row['path']; + $correctPath = $row['parentpath'] . '/' . $row['name']; + // make sure the target is on a different subtree + if (substr($correctPath, 0, strlen($wrongPath)) === $wrongPath) { + // the path based parent entry is referencing one of its own children, skipping + // fix the entry's parent id instead + // note: fixEntryParent cannot fail to find the parent entry by path + // here because the reason we reached this code is because we already + // found it + $this->fixEntryParent( + $out, + $row['storage'], + $row['fileid'], + $row['path'], + $row['wrongparentid'], + true + ); + } else { + $this->fixEntryPath( + $out, + $row['fileid'], + $wrongPath, + $row['parentstorage'], + $correctPath + ); + } + $lastResultsCount++; + } + $this->connection->commit(); + + $totalResultsCount += $lastResultsCount; + + // note: this is not pagination but repeating the query over and over again + // until all possible entries were fixed + } while ($lastResultsCount > 0); + + if ($totalResultsCount > 0) { + $out->info("Fixed $totalResultsCount file cache entries with wrong path"); + } + + return $totalResultsCount; + } + + /** + * Gets the file id of the entry. If none exists, create it + * up to the root if needed. + * + * @param int $storageId storage id + * @param string $path path for which to create the parent entry + * @return int file id of the newly created parent + */ + private function getOrCreateEntry($storageId, $path, $reuseFileId = null) { + if ($path === '.') { + $path = ''; + } + // find the correct parent + $qb = $this->connection->getQueryBuilder(); + // select fileid as "correctparentid" + $qb->select('fileid') + // from oc_filecache + ->from('filecache') + // where storage=$storage and path='$parentPath' + ->where($qb->expr()->eq('storage', $qb->createNamedParameter($storageId))) + ->andWhere($qb->expr()->eq('path', $qb->createNamedParameter($path))); + $results = $qb->execute(); + $rows = $results->fetchAll(); + $results->closeCursor(); + + if (!empty($rows)) { + return $rows[0]['fileid']; + } + + if ($path !== '') { + $parentId = $this->getOrCreateEntry($storageId, dirname($path)); + } else { + // root entry missing, create it + $parentId = -1; + } + + $qb = $this->connection->getQueryBuilder(); + $values = [ + 'storage' => $qb->createNamedParameter($storageId), + 'path' => $qb->createNamedParameter($path), + 'path_hash' => $qb->createNamedParameter(md5($path)), + 'name' => $qb->createNamedParameter(basename($path)), + 'parent' => $qb->createNamedParameter($parentId), + 'size' => $qb->createNamedParameter(-1), + 'etag' => $qb->createNamedParameter('zombie'), + 'mimetype' => $qb->createNamedParameter($this->dirMimeTypeId), + 'mimepart' => $qb->createNamedParameter($this->dirMimePartId), + ]; + + if ($reuseFileId !== null) { + // purpose of reusing the fileid of the parent is to salvage potential + // metadata that might have previously been linked to this file id + $values['fileid'] = $qb->createNamedParameter($reuseFileId); + } + $qb->insert('filecache')->values($values); + $qb->execute(); + return $this->connection->lastInsertId('*PREFIX*filecache'); + } + + /** + * Fixes the broken entry's path. + * + * @param IOutput $out repair output + * @param int $storageId storage id of the entry to fix + * @param int $fileId file id of the entry to fix + * @param string $path path from the entry to fix + * @param int $wrongParentId wrong parent id + * @param bool $parentIdExists true if the entry from the $wrongParentId exists (but is the wrong one), + * false if it doesn't + * @return bool true if the entry was fixed, false otherwise + */ + private function fixEntryParent(IOutput $out, $storageId, $fileId, $path, $wrongParentId, $parentIdExists = false) { + if (!$parentIdExists) { + // if the parent doesn't exist, let us reuse its id in case there is metadata to salvage + $correctParentId = $this->getOrCreateEntry($storageId, dirname($path), $wrongParentId); + } else { + // parent exists and is the wrong one, so recreating would need a new fileid + $correctParentId = $this->getOrCreateEntry($storageId, dirname($path)); + } + + $this->connection->beginTransaction(); + + $qb = $this->connection->getQueryBuilder(); + $qb->update('filecache') + ->set('parent', $qb->createNamedParameter($correctParentId)) + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($fileId))); + $qb->execute(); + + $text = "Fixed file cache entry with fileid $fileId, set wrong parent \"$wrongParentId\" to \"$correctParentId\""; + $out->advance(1, $text); + + $this->connection->commit(); + + return true; + } + + /** + * Repair entries where the parent id doesn't point to any existing entry + * by finding the actual parent entry matching the entry's path dirname. + * + * @param IOutput $out output + * @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))); + } + $qb->setMaxResults(self::CHUNK_SIZE); + + $totalResultsCount = 0; + do { + $results = $qb->execute(); + // since we're going to operate on fetched entry, better cache them + // to avoid DB lock ups + $rows = $results->fetchAll(); + $results->closeCursor(); + + $lastResultsCount = 0; + foreach ($rows as $row) { + $this->fixEntryParent( + $out, + $row['storage'], + $row['fileid'], + $row['path'], + $row['parent'], + // in general the parent doesn't exist except + // for the one condition where parent=fileid + $row['parent'] === $row['fileid'] + ); + $lastResultsCount++; + } + + $totalResultsCount += $lastResultsCount; + + // note: this is not pagination but repeating the query over and over again + // until all possible entries were fixed + } while ($lastResultsCount > 0); + + if ($totalResultsCount > 0) { + $out->info("Fixed $totalResultsCount file cache entries with wrong path"); + } + + return $totalResultsCount; + } + + /** + * Run the repair step + * + * @param IOutput $out output + */ + public function run(IOutput $out) { + + $this->dirMimeTypeId = $this->mimeLoader->getId('httpd/unix-directory'); + $this->dirMimePartId = $this->mimeLoader->getId('httpd'); + + if ($this->countOnly) { + $this->reportAffectedStorages($out); + } else { + $brokenPathEntries = $this->countResultsToProcess(); + $out->startProgress($brokenPathEntries); + + $totalFixed = 0; + + /* + * This repair itself might overwrite existing target parent entries and create + * orphans where the parent entry of the parent id doesn't exist but the path matches. + * This needs to be repaired by fixEntriesWithNonExistingParentIdEntry(), this is why + * we need to keep this specific order of repair. + */ + $totalFixed += $this->fixEntriesWithCorrectParentIdButWrongPath($out); + + $totalFixed += $this->fixEntriesWithNonExistingParentIdEntry($out); + $out->finishProgress(); + } + } +} diff --git a/tests/lib/Repair/RepairMismatchFileCachePathTest.php b/tests/lib/Repair/RepairMismatchFileCachePathTest.php new file mode 100644 index 000000000000..e33aa0a06f3a --- /dev/null +++ b/tests/lib/Repair/RepairMismatchFileCachePathTest.php @@ -0,0 +1,496 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace Test\Repair; + + +use OC\Repair\RepairMismatchFileCachePath; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; +use Test\TestCase; +use OCP\Files\IMimeTypeLoader; + +/** + * Tests for repairing mismatch file cache paths + * + * @group DB + * + * @see \OC\Repair\RepairMismatchFileCachePath + */ +class RepairMismatchFileCachePathTest extends TestCase { + + /** @var IRepairStep */ + private $repair; + + /** @var \OCP\IDBConnection */ + private $connection; + + protected function setUp() { + parent::setUp(); + + $this->connection = \OC::$server->getDatabaseConnection(); + + $mimeLoader = $this->createMock(IMimeTypeLoader::class); + $mimeLoader->method('getId') + ->will($this->returnValueMap([ + ['httpd', 1], + ['httpd/unix-directory', 2], + ])); + $this->repair = new RepairMismatchFileCachePath($this->connection, $mimeLoader); + $this->repair->setCountOnly(false); + } + + protected function tearDown() { + $qb = $this->connection->getQueryBuilder(); + $qb->delete('filecache')->execute(); + parent::tearDown(); + } + + private function createFileCacheEntry($storage, $path, $parent = -1) { + $qb = $this->connection->getQueryBuilder(); + $qb->insert('filecache') + ->values([ + 'storage' => $qb->createNamedParameter($storage), + 'path' => $qb->createNamedParameter($path), + 'path_hash' => $qb->createNamedParameter(md5($path)), + 'name' => $qb->createNamedParameter(basename($path)), + 'parent' => $qb->createNamedParameter($parent), + ]); + $qb->execute(); + return $this->connection->lastInsertId('*PREFIX*filecache'); + } + + /** + * Returns autoincrement compliant fileid for an entry that might + * have existed + * + * @return int fileid + */ + private function createNonExistingId() { + // why are we doing this ? well, if we just pick an arbitrary + // value ahead of the autoincrement, this will not reflect real scenarios + // and also will likely cause potential collisions as some newly inserted entries + // might receive said arbitrary id through autoincrement + // + // So instead, we insert a dummy entry and delete it afterwards so we have + // "reserved" the fileid and also somehow simulated whatever might have happened + // on a real system when a file cache entry suddenly disappeared for whatever + // mysterious reasons + $entryId = $this->createFileCacheEntry(1, 'goodbye-entry'); + $qb = $this->connection->getQueryBuilder(); + $qb->delete('filecache') + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($entryId))); + $qb->execute(); + return $entryId; + } + + private function getFileCacheEntry($fileId) { + $qb = $this->connection->getQueryBuilder(); + $qb->select('*') + ->from('filecache') + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($fileId))); + $results = $qb->execute(); + $result = $results->fetch(); + $results->closeCursor(); + return $result; + } + + /** + * Sets the parent of the given file id to the given parent id + * + * @param int $fileId file id of the entry to adjust + * @param int $parentId parent id to set to + */ + private function setFileCacheEntryParent($fileId, $parentId) { + $qb = $this->connection->getQueryBuilder(); + $qb->update('filecache') + ->set('parent', $qb->createNamedParameter($parentId)) + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($fileId))); + $qb->execute(); + } + + public function repairCasesProvider() { + return [ + // same storage, different target dir + [1, 1, 'target'], + // different storage, same target dir name + [1, 2, 'source'], + // different storage, different target dir + [1, 2, 'target'], + + // same storage, different target dir, target exists + [1, 1, 'target', true], + // different storage, same target dir name, target exists + [1, 2, 'source', true], + // different storage, different target dir, target exists + [1, 2, 'target', true], + ]; + } + + /** + * Test repair + * + * @dataProvider repairCasesProvider + */ + public function testRepairEntry($sourceStorageId, $targetStorageId, $targetDir, $targetExists = false) { + /* + * Tree: + * + * source storage: + * - files/ + * - files/source/ + * - files/source/to_move (not created as we simulate that it was already moved) + * - files/source/to_move/content_to_update (bogus entry to fix) + * - files/source/to_move/content_to_update/sub (bogus subentry to fix) + * - files/source/do_not_touch (regular entry outside of the repair scope) + * + * target storage: + * - files/ + * - files/target/ + * - files/target/moved_renamed (already moved target) + * - files/target/moved_renamed/content_to_update (missing until repair) + * + * if $targetExists: pre-create these additional entries: + * - files/target/moved_renamed/content_to_update (will be overwritten) + * - files/target/moved_renamed/content_to_update/sub (will be overwritten) + * - files/target/moved_renamed/content_to_update/unrelated (will be reparented) + * + */ + + // source storage entries + $rootId1 = $this->createFileCacheEntry($sourceStorageId, ''); + $baseId1 = $this->createFileCacheEntry($sourceStorageId, 'files', $rootId1); + if ($sourceStorageId !== $targetStorageId) { + $rootId2 = $this->createFileCacheEntry($targetStorageId, ''); + $baseId2 = $this->createFileCacheEntry($targetStorageId, 'files', $rootId2); + } else { + $rootId2 = $rootId1; + $baseId2 = $baseId1; + } + $sourceId = $this->createFileCacheEntry($sourceStorageId, 'files/source', $baseId1); + + // target storage entries + $targetParentId = $this->createFileCacheEntry($targetStorageId, 'files/' . $targetDir, $baseId2); + + // the move does create the parent in the target + $targetId = $this->createFileCacheEntry($targetStorageId, 'files/' . $targetDir . '/moved_renamed', $targetParentId); + + // bogus entry: any children of the source are not properly updated + $movedId = $this->createFileCacheEntry($sourceStorageId, 'files/source/to_move/content_to_update', $targetId); + $movedSubId = $this->createFileCacheEntry($sourceStorageId, 'files/source/to_move/content_to_update/sub', $movedId); + + if ($targetExists) { + // after the bogus move happened, some code path recreated the parent under a + // different file id + $existingTargetId = $this->createFileCacheEntry($targetStorageId, 'files/' . $targetDir . '/moved_renamed/content_to_update', $targetId); + $existingTargetSubId = $this->createFileCacheEntry($targetStorageId, 'files/' . $targetDir . '/moved_renamed/content_to_update/sub', $existingTargetId); + $existingTargetUnrelatedId = $this->createFileCacheEntry($targetStorageId, 'files/' . $targetDir . '/moved_renamed/content_to_update/unrelated', $existingTargetId); + } + + $doNotTouchId = $this->createFileCacheEntry($sourceStorageId, 'files/source/do_not_touch', $sourceId); + + $outputMock = $this->createMock(IOutput::class); + $this->repair->run($outputMock); + + $entry = $this->getFileCacheEntry($movedId); + $this->assertEquals($targetId, $entry['parent']); + $this->assertEquals((string)$targetStorageId, $entry['storage']); + $this->assertEquals('files/' . $targetDir . '/moved_renamed/content_to_update', $entry['path']); + $this->assertEquals(md5('files/' . $targetDir . '/moved_renamed/content_to_update'), $entry['path_hash']); + + $entry = $this->getFileCacheEntry($movedSubId); + $this->assertEquals($movedId, $entry['parent']); + $this->assertEquals((string)$targetStorageId, $entry['storage']); + $this->assertEquals('files/' . $targetDir . '/moved_renamed/content_to_update/sub', $entry['path']); + $this->assertEquals(md5('files/' . $targetDir . '/moved_renamed/content_to_update/sub'), $entry['path_hash']); + + if ($targetExists) { + $this->assertFalse($this->getFileCacheEntry($existingTargetId)); + $this->assertFalse($this->getFileCacheEntry($existingTargetSubId)); + + // unrelated folder has been reparented + $entry = $this->getFileCacheEntry($existingTargetUnrelatedId); + $this->assertEquals($movedId, $entry['parent']); + $this->assertEquals((string)$targetStorageId, $entry['storage']); + $this->assertEquals('files/' . $targetDir . '/moved_renamed/content_to_update/unrelated', $entry['path']); + $this->assertEquals(md5('files/' . $targetDir . '/moved_renamed/content_to_update/unrelated'), $entry['path_hash']); + } + + // root entries left alone + $entry = $this->getFileCacheEntry($rootId1); + $this->assertEquals(-1, $entry['parent']); + $this->assertEquals((string)$sourceStorageId, $entry['storage']); + $this->assertEquals('', $entry['path']); + $this->assertEquals(md5(''), $entry['path_hash']); + + $entry = $this->getFileCacheEntry($rootId2); + $this->assertEquals(-1, $entry['parent']); + $this->assertEquals((string)$targetStorageId, $entry['storage']); + $this->assertEquals('', $entry['path']); + $this->assertEquals(md5(''), $entry['path_hash']); + + // "do not touch" entry left untouched + $entry = $this->getFileCacheEntry($doNotTouchId); + $this->assertEquals($sourceId, $entry['parent']); + $this->assertEquals((string)$sourceStorageId, $entry['storage']); + $this->assertEquals('files/source/do_not_touch', $entry['path']); + $this->assertEquals(md5('files/source/do_not_touch'), $entry['path_hash']); + + // root entry left alone + $entry = $this->getFileCacheEntry($rootId1); + $this->assertEquals(-1, $entry['parent']); + $this->assertEquals((string)$sourceStorageId, $entry['storage']); + $this->assertEquals('', $entry['path']); + $this->assertEquals(md5(''), $entry['path_hash']); + } + + /** + * Test repair self referencing entries + */ + public function testRepairSelfReferencing() { + /** + * Self-referencing: + * - files/all_your_zombies (parent=fileid must be reparented) + * + * Referencing child one level: + * - files/ref_child1 (parent=fileid of the child) + * - files/ref_child1/child (parent=fileid of the child) + * + * Referencing child two levels: + * - files/ref_child2/ (parent=fileid of the child's child) + * - files/ref_child2/child + * - files/ref_child2/child/child + * + * Referencing child two levels detached: + * - detached/ref_child3/ (parent=fileid of the child, "detached" has no entry) + * - detached/ref_child3/child + */ + $storageId = 1; + $rootId1 = $this->createFileCacheEntry($storageId, ''); + $baseId1 = $this->createFileCacheEntry($storageId, 'files', $rootId1); + + $selfRefId = $this->createFileCacheEntry($storageId, 'files/all_your_zombies', $baseId1); + $this->setFileCacheEntryParent($selfRefId, $selfRefId); + + $refChild1Id = $this->createFileCacheEntry($storageId, 'files/ref_child1', $baseId1); + $refChild1ChildId = $this->createFileCacheEntry($storageId, 'files/ref_child1/child', $refChild1Id); + // make it reference its own child + $this->setFileCacheEntryParent($refChild1Id, $refChild1ChildId); + + $refChild2Id = $this->createFileCacheEntry($storageId, 'files/ref_child2', $baseId1); + $refChild2ChildId = $this->createFileCacheEntry($storageId, 'files/ref_child2/child', $refChild2Id); + $refChild2ChildChildId = $this->createFileCacheEntry($storageId, 'files/ref_child2/child/child', $refChild2ChildId); + // make it reference its own sub child + $this->setFileCacheEntryParent($refChild2Id, $refChild2ChildChildId); + + $refChild3Id = $this->createFileCacheEntry($storageId, 'detached/ref_child3', $baseId1); + $refChild3ChildId = $this->createFileCacheEntry($storageId, 'detached/ref_child3/child', $refChild3Id); + // make it reference its own child + $this->setFileCacheEntryParent($refChild3Id, $refChild3ChildId); + + $outputMock = $this->createMock(IOutput::class); + $this->repair->run($outputMock); + + // self-referencing updated + $entry = $this->getFileCacheEntry($selfRefId); + $this->assertEquals($baseId1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('files/all_your_zombies', $entry['path']); + $this->assertEquals(md5('files/all_your_zombies'), $entry['path_hash']); + + // ref child 1 case was reparented to "files" + $entry = $this->getFileCacheEntry($refChild1Id); + $this->assertEquals($baseId1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('files/ref_child1', $entry['path']); + $this->assertEquals(md5('files/ref_child1'), $entry['path_hash']); + + // ref child 1 child left alone + $entry = $this->getFileCacheEntry($refChild1ChildId); + $this->assertEquals($refChild1Id, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('files/ref_child1/child', $entry['path']); + $this->assertEquals(md5('files/ref_child1/child'), $entry['path_hash']); + + // ref child 2 case was reparented to "files" + $entry = $this->getFileCacheEntry($refChild2Id); + $this->assertEquals($baseId1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('files/ref_child2', $entry['path']); + $this->assertEquals(md5('files/ref_child2'), $entry['path_hash']); + + // ref child 2 child left alone + $entry = $this->getFileCacheEntry($refChild2ChildId); + $this->assertEquals($refChild2Id, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('files/ref_child2/child', $entry['path']); + $this->assertEquals(md5('files/ref_child2/child'), $entry['path_hash']); + + // ref child 2 child child left alone + $entry = $this->getFileCacheEntry($refChild2ChildChildId); + $this->assertEquals($refChild2ChildId, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('files/ref_child2/child/child', $entry['path']); + $this->assertEquals(md5('files/ref_child2/child/child'), $entry['path_hash']); + + // root entry left alone + $entry = $this->getFileCacheEntry($rootId1); + $this->assertEquals(-1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('', $entry['path']); + $this->assertEquals(md5(''), $entry['path_hash']); + + // ref child 3 child left alone + $entry = $this->getFileCacheEntry($refChild3ChildId); + $this->assertEquals($refChild3Id, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('detached/ref_child3/child', $entry['path']); + $this->assertEquals(md5('detached/ref_child3/child'), $entry['path_hash']); + + // ref child 3 case was reparented to a new "detached" entry + $entry = $this->getFileCacheEntry($refChild3Id); + $this->assertTrue(isset($entry['parent'])); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('detached/ref_child3', $entry['path']); + $this->assertEquals(md5('detached/ref_child3'), $entry['path_hash']); + + // entry "detached" was restored + $entry = $this->getFileCacheEntry($entry['parent']); + $this->assertEquals($rootId1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('detached', $entry['path']); + $this->assertEquals(md5('detached'), $entry['path_hash']); + } + + /** + * Test repair wrong parent id + */ + public function testRepairParentIdPointingNowhere() { + /** + * Wrong parent id + * - wrongparentroot + * - files/wrongparent + */ + $storageId = 1; + $rootId1 = $this->createFileCacheEntry($storageId, ''); + $baseId1 = $this->createFileCacheEntry($storageId, 'files', $rootId1); + + $nonExistingParentId = $this->createNonExistingId(); + $wrongParentRootId = $this->createFileCacheEntry($storageId, 'wrongparentroot', $nonExistingParentId); + $wrongParentId = $this->createFileCacheEntry($storageId, 'files/wrongparent', $nonExistingParentId); + + $outputMock = $this->createMock(IOutput::class); + $this->repair->run($outputMock); + + // wrong parent root reparented to actual root + $entry = $this->getFileCacheEntry($wrongParentRootId); + $this->assertEquals($rootId1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('wrongparentroot', $entry['path']); + $this->assertEquals(md5('wrongparentroot'), $entry['path_hash']); + + // wrong parent subdir reparented to "files" + $entry = $this->getFileCacheEntry($wrongParentId); + $this->assertEquals($baseId1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('files/wrongparent', $entry['path']); + $this->assertEquals(md5('files/wrongparent'), $entry['path_hash']); + + // root entry left alone + $entry = $this->getFileCacheEntry($rootId1); + $this->assertEquals(-1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('', $entry['path']); + $this->assertEquals(md5(''), $entry['path_hash']); + } + + /** + * Test repair detached subtree + */ + public function testRepairDetachedSubtree() { + /** + * other: + * - files/missingdir/orphaned1 (orphaned entry as "missingdir" is missing) + * - missingdir/missingdir1/orphaned2 (orphaned entry two levels up to root) + * - noroot (orphaned entry on a storage that has no root entry) + */ + $storageId = 1; + $storageId2 = 2; + $rootId1 = $this->createFileCacheEntry($storageId, ''); + $baseId1 = $this->createFileCacheEntry($storageId, 'files', $rootId1); + + $nonExistingParentId = $this->createNonExistingId(); + $orphanedId1 = $this->createFileCacheEntry($storageId, 'files/missingdir/orphaned1', $nonExistingParentId); + + $nonExistingParentId2 = $this->createNonExistingId(); + $orphanedId2 = $this->createFileCacheEntry($storageId, 'missingdir/missingdir1/orphaned2', $nonExistingParentId2); + + $nonExistingParentId3 = $this->createNonExistingId(); + $orphanedId3 = $this->createFileCacheEntry($storageId2, 'noroot', $nonExistingParentId3); + + $outputMock = $this->createMock(IOutput::class); + $this->repair->run($outputMock); + + // orphaned entry reattached + $entry = $this->getFileCacheEntry($orphanedId1); + $this->assertEquals($nonExistingParentId, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('files/missingdir/orphaned1', $entry['path']); + $this->assertEquals(md5('files/missingdir/orphaned1'), $entry['path_hash']); + + // non existing id exists now + $entry = $this->getFileCacheEntry($entry['parent']); + $this->assertEquals($baseId1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('files/missingdir', $entry['path']); + $this->assertEquals(md5('files/missingdir'), $entry['path_hash']); + + // orphaned entry reattached + $entry = $this->getFileCacheEntry($orphanedId2); + $this->assertEquals($nonExistingParentId2, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('missingdir/missingdir1/orphaned2', $entry['path']); + $this->assertEquals(md5('missingdir/missingdir1/orphaned2'), $entry['path_hash']); + + // non existing id exists now + $entry = $this->getFileCacheEntry($entry['parent']); + $this->assertTrue(isset($entry['parent'])); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('missingdir/missingdir1', $entry['path']); + $this->assertEquals(md5('missingdir/missingdir1'), $entry['path_hash']); + + // non existing id parent exists now + $entry = $this->getFileCacheEntry($entry['parent']); + $this->assertEquals($rootId1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('missingdir', $entry['path']); + $this->assertEquals(md5('missingdir'), $entry['path_hash']); + + // root entry left alone + $entry = $this->getFileCacheEntry($rootId1); + $this->assertEquals(-1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('', $entry['path']); + $this->assertEquals(md5(''), $entry['path_hash']); + + // orphaned entry with no root reattached + $entry = $this->getFileCacheEntry($orphanedId3); + $this->assertTrue(isset($entry['parent'])); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('noroot', $entry['path']); + $this->assertEquals(md5('noroot'), $entry['path_hash']); + + // recreated root entry + $entry = $this->getFileCacheEntry($entry['parent']); + $this->assertEquals(-1, $entry['parent']); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('', $entry['path']); + $this->assertEquals(md5(''), $entry['path_hash']); + } +} + From 45e8a89f578f3c0d0bd1faf8ab65fc0fa07a9550 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 2 Aug 2017 12:00:41 +0200 Subject: [PATCH 02/13] Use DI and add more tests for storage filter --- apps/files/appinfo/app.php | 3 + apps/files/lib/AppInfo/Application.php | 7 ++ apps/files/lib/Command/Scan.php | 21 +++- .../Repair/RepairMismatchFileCachePath.php | 118 ++++++++++++------ lib/private/Server.php | 2 + .../RepairMismatchFileCachePathTest.php | 71 ++++++++--- 6 files changed, 159 insertions(+), 63 deletions(-) diff --git a/apps/files/appinfo/app.php b/apps/files/appinfo/app.php index 26352795800e..9fbbc0a4dfd3 100644 --- a/apps/files/appinfo/app.php +++ b/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'); diff --git a/apps/files/lib/AppInfo/Application.php b/apps/files/lib/AppInfo/Application.php index 020c64178955..7953afc68d0d 100644 --- a/apps/files/lib/AppInfo/Application.php +++ b/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 */ diff --git a/apps/files/lib/Command/Scan.php b/apps/files/lib/Command/Scan.php index 93042c522585..5e90effdd5db 100644 --- a/apps/files/lib/Command/Scan.php +++ b/apps/files/lib/Command/Scan.php @@ -43,11 +43,16 @@ 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 */ @@ -55,8 +60,14 @@ class Scan extends Base { /** @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("\tStorage \"" . $storage->getCache()->getNumericStorageId() . '" cannot be repaired as it is currently in use, please try again later'); 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); } }); } diff --git a/lib/private/Repair/RepairMismatchFileCachePath.php b/lib/private/Repair/RepairMismatchFileCachePath.php index 477d0a6a3214..bca29c155e05 100644 --- a/lib/private/Repair/RepairMismatchFileCachePath.php +++ b/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; diff --git a/lib/private/Server.php b/lib/private/Server.php index 96da071a583a..59fa05491c4e 100644 --- a/lib/private/Server.php +++ b/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(); }); diff --git a/tests/lib/Repair/RepairMismatchFileCachePathTest.php b/tests/lib/Repair/RepairMismatchFileCachePathTest.php index e33aa0a06f3a..320a3acac77b 100644 --- a/tests/lib/Repair/RepairMismatchFileCachePathTest.php +++ b/tests/lib/Repair/RepairMismatchFileCachePathTest.php @@ -117,18 +117,29 @@ private function setFileCacheEntryParent($fileId, $parentId) { public function repairCasesProvider() { return [ // same storage, different target dir - [1, 1, 'target'], + [1, 1, 'target', false, null], + [1, 1, 'target', false, [1]], // different storage, same target dir name - [1, 2, 'source'], + [1, 2, 'source', false, null], + [1, 2, 'source', false, [1, 2]], + [1, 2, 'source', false, [2, 1]], // different storage, different target dir - [1, 2, 'target'], + [1, 2, 'target', false, null], + [1, 2, 'target', false, [1, 2]], + [1, 2, 'target', false, [2, 1]], // same storage, different target dir, target exists - [1, 1, 'target', true], + [1, 1, 'target', true, null], + [1, 1, 'target', true, [1, 2]], + [1, 1, 'target', true, [2, 1]], // different storage, same target dir name, target exists - [1, 2, 'source', true], + [1, 2, 'source', true, null], + [1, 2, 'source', true, [1, 2]], + [1, 2, 'source', true, [2, 1]], // different storage, different target dir, target exists - [1, 2, 'target', true], + [1, 2, 'target', true, null], + [1, 2, 'target', true, [1, 2]], + [1, 2, 'target', true, [2, 1]], ]; } @@ -137,7 +148,7 @@ public function repairCasesProvider() { * * @dataProvider repairCasesProvider */ - public function testRepairEntry($sourceStorageId, $targetStorageId, $targetDir, $targetExists = false) { + public function testRepairEntry($sourceStorageId, $targetStorageId, $targetDir, $targetExists, $repairStoragesOrder) { /* * Tree: * @@ -195,7 +206,16 @@ public function testRepairEntry($sourceStorageId, $targetStorageId, $targetDir, $doNotTouchId = $this->createFileCacheEntry($sourceStorageId, 'files/source/do_not_touch', $sourceId); $outputMock = $this->createMock(IOutput::class); - $this->repair->run($outputMock); + if (is_null($repairStoragesOrder)) { + // no storage selected, full repair + $this->repair->setStorageNumericId(null); + $this->repair->run($outputMock); + } else { + foreach ($repairStoragesOrder as $storageId) { + $this->repair->setStorageNumericId($storageId); + $this->repair->run($outputMock); + } + } $entry = $this->getFileCacheEntry($movedId); $this->assertEquals($targetId, $entry['parent']); @@ -294,6 +314,7 @@ public function testRepairSelfReferencing() { $this->setFileCacheEntryParent($refChild3Id, $refChild3ChildId); $outputMock = $this->createMock(IOutput::class); + $this->repair->setStorageNumericId($storageId); $this->repair->run($outputMock); // self-referencing updated @@ -385,6 +406,7 @@ public function testRepairParentIdPointingNowhere() { $wrongParentId = $this->createFileCacheEntry($storageId, 'files/wrongparent', $nonExistingParentId); $outputMock = $this->createMock(IOutput::class); + $this->repair->setStorageNumericId($storageId); $this->repair->run($outputMock); // wrong parent root reparented to actual root @@ -414,13 +436,10 @@ public function testRepairParentIdPointingNowhere() { */ public function testRepairDetachedSubtree() { /** - * other: - * - files/missingdir/orphaned1 (orphaned entry as "missingdir" is missing) - * - missingdir/missingdir1/orphaned2 (orphaned entry two levels up to root) - * - noroot (orphaned entry on a storage that has no root entry) + * - files/missingdir/orphaned1 (orphaned entry as "missingdir" is missing) + * - missingdir/missingdir1/orphaned2 (orphaned entry two levels up to root) */ $storageId = 1; - $storageId2 = 2; $rootId1 = $this->createFileCacheEntry($storageId, ''); $baseId1 = $this->createFileCacheEntry($storageId, 'files', $rootId1); @@ -430,10 +449,8 @@ public function testRepairDetachedSubtree() { $nonExistingParentId2 = $this->createNonExistingId(); $orphanedId2 = $this->createFileCacheEntry($storageId, 'missingdir/missingdir1/orphaned2', $nonExistingParentId2); - $nonExistingParentId3 = $this->createNonExistingId(); - $orphanedId3 = $this->createFileCacheEntry($storageId2, 'noroot', $nonExistingParentId3); - $outputMock = $this->createMock(IOutput::class); + $this->repair->setStorageNumericId($storageId); $this->repair->run($outputMock); // orphaned entry reattached @@ -477,18 +494,34 @@ public function testRepairDetachedSubtree() { $this->assertEquals((string)$storageId, $entry['storage']); $this->assertEquals('', $entry['path']); $this->assertEquals(md5(''), $entry['path_hash']); + } + + /** + * Test repair missing root + */ + public function testRepairMissingRoot() { + /** + * - noroot (orphaned entry on a storage that has no root entry) + */ + $storageId = 1; + $nonExistingParentId = $this->createNonExistingId(); + $orphanedId = $this->createFileCacheEntry($storageId, 'noroot', $nonExistingParentId); + + $outputMock = $this->createMock(IOutput::class); + $this->repair->setStorageNumericId($storageId); + $this->repair->run($outputMock); // orphaned entry with no root reattached - $entry = $this->getFileCacheEntry($orphanedId3); + $entry = $this->getFileCacheEntry($orphanedId); $this->assertTrue(isset($entry['parent'])); - $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals((string)$storageId, $entry['storage']); $this->assertEquals('noroot', $entry['path']); $this->assertEquals(md5('noroot'), $entry['path_hash']); // recreated root entry $entry = $this->getFileCacheEntry($entry['parent']); $this->assertEquals(-1, $entry['parent']); - $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals((string)$storageId, $entry['storage']); $this->assertEquals('', $entry['path']); $this->assertEquals(md5(''), $entry['path_hash']); } From fbc225e168515a06c94854cdff194a798f3263da Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 16 Aug 2017 11:16:10 +0200 Subject: [PATCH 03/13] Run phase 2 of repair on gathered affected storages from phase 1 When fixing failed cross-storage moves in the file cache and using the storage id filter, we filter by target storage for phase 1. However, we also need to fix the source storages in phase 2. To do so, a list of affected source storages is now gathered in phase 1 to be run on phase 2. --- .../Repair/RepairMismatchFileCachePath.php | 55 +++++++++++-------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/lib/private/Repair/RepairMismatchFileCachePath.php b/lib/private/Repair/RepairMismatchFileCachePath.php index bca29c155e05..ccd30c4d376c 100644 --- a/lib/private/Repair/RepairMismatchFileCachePath.php +++ b/lib/private/Repair/RepairMismatchFileCachePath.php @@ -121,7 +121,7 @@ private function fixEntryPath(IOutput $out, $fileId, $wrongPath, $correctStorage $out->advance(1, $text); } - private function addQueryConditionsParentIdWrongPath($qb) { + private function addQueryConditionsParentIdWrongPath($qb, $storageNumericId) { // thanks, VicDeo! if ($this->connection->getDatabasePlatform() instanceof MySqlPlatform) { $concatFunction = $qb->createFunction("CONCAT(fcp.path, '/', fc.name)"); @@ -153,14 +153,14 @@ private function addQueryConditionsParentIdWrongPath($qb) { ->andWhere($qb->expr()->neq('fc.fileid', 'fcp.fileid')); if ($this->storageNumericId !== null) { - // use the source storage of the failed move when filtering + // use the target storage of the failed move when filtering $qb->andWhere( $qb->expr()->eq('fc.storage', $qb->createNamedParameter($this->storageNumericId)) ); } } - private function addQueryConditionsNonExistingParentIdEntry($qb) { + private function addQueryConditionsNonExistingParentIdEntry($qb, $storageNumericId = null) { // Subquery for parent existence $qbe = $this->connection->getQueryBuilder(); $qbe->select($qbe->expr()->literal('1')) @@ -183,27 +183,26 @@ private function addQueryConditionsNonExistingParentIdEntry($qb) { ) ); - 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))); + if ($storageNumericId !== null) { + // filter on destination storage of a failed move + $qb->andWhere($qb->expr()->eq('fc.storage', $qb->createNamedParameter($storageNumericId))); } } - private function countResultsToProcessParentIdWrongPath() { + private function countResultsToProcessParentIdWrongPath($storageNumericId = null) { $qb = $this->connection->getQueryBuilder(); $qb->select($qb->createFunction('COUNT(*)')); - $this->addQueryConditionsParentIdWrongPath($qb); + $this->addQueryConditionsParentIdWrongPath($qb, $storageNumericId); $results = $qb->execute(); $count = $results->fetchColumn(0); $results->closeCursor(); return $count; } - private function countResultsToProcessNonExistingParentIdEntry() { + private function countResultsToProcessNonExistingParentIdEntry($storageNumericId = null) { $qb = $this->connection->getQueryBuilder(); $qb->select($qb->createFunction('COUNT(*)')); - $this->addQueryConditionsNonExistingParentIdEntry($qb); + $this->addQueryConditionsNonExistingParentIdEntry($qb, $storageNumericId); $results = $qb->execute(); $count = $results->fetchColumn(0); $results->closeCursor(); @@ -268,10 +267,12 @@ private function reportAffectedStoragesNonExistingParentIdEntry(IOutput $out) { * value doesn't match the parent's path. * * @param IOutput $out - * @return int number of results that were fixed + * @param int|null $storageNumericId storage to fix or null for all + * @return int[] storage numeric ids that were targets to a move and needs further fixing */ - private function fixEntriesWithCorrectParentIdButWrongPath(IOutput $out) { + private function fixEntriesWithCorrectParentIdButWrongPath(IOutput $out, $storageNumericId = null) { $totalResultsCount = 0; + $affectedStorages = [$storageNumericId => true]; // find all entries where the path entry doesn't match the path value that would // be expected when following the parent-child relationship, basically @@ -282,7 +283,7 @@ private function fixEntriesWithCorrectParentIdButWrongPath(IOutput $out) { ->selectAlias('fc.parent', 'wrongparentid') ->selectAlias('fcp.storage', 'parentstorage') ->selectAlias('fcp.path', 'parentpath'); - $this->addQueryConditionsParentIdWrongPath($qb); + $this->addQueryConditionsParentIdWrongPath($qb, $storageNumericId); $qb->setMaxResults(self::CHUNK_SIZE); do { @@ -299,7 +300,7 @@ private function fixEntriesWithCorrectParentIdButWrongPath(IOutput $out) { $correctPath = $row['parentpath'] . '/' . $row['name']; // make sure the target is on a different subtree if (substr($correctPath, 0, strlen($wrongPath)) === $wrongPath) { - // the path based parent entry is referencing one of its own children, skipping + // the path based parent entry is referencing one of its own children, // fix the entry's parent id instead // note: fixEntryParent cannot fail to find the parent entry by path // here because the reason we reached this code is because we already @@ -320,6 +321,8 @@ private function fixEntriesWithCorrectParentIdButWrongPath(IOutput $out) { $row['parentstorage'], $correctPath ); + // we also need to fix the target storage + $affectedStorages[$row['parentstorage']] = true; } $lastResultsCount++; } @@ -335,7 +338,7 @@ private function fixEntriesWithCorrectParentIdButWrongPath(IOutput $out) { $out->info("Fixed $totalResultsCount file cache entries with wrong path"); } - return $totalResultsCount; + return array_keys($affectedStorages); } /** @@ -439,11 +442,12 @@ private function fixEntryParent(IOutput $out, $storageId, $fileId, $path, $wrong * by finding the actual parent entry matching the entry's path dirname. * * @param IOutput $out output + * @param int|null $storageNumericId storage to fix or null for all * @return int number of results that were fixed */ - private function fixEntriesWithNonExistingParentIdEntry(IOutput $out) { + private function fixEntriesWithNonExistingParentIdEntry(IOutput $out, $storageNumericId = null) { $qb = $this->connection->getQueryBuilder(); - $this->addQueryConditionsNonExistingParentIdEntry($qb); + $this->addQueryConditionsNonExistingParentIdEntry($qb, $storageNumericId); $qb->setMaxResults(self::CHUNK_SIZE); $totalResultsCount = 0; @@ -496,8 +500,8 @@ public function run(IOutput $out) { $this->reportAffectedStoragesParentIdWrongPath($out); $this->reportAffectedStoragesNonExistingParentIdEntry($out); } else { - $brokenPathEntries = $this->countResultsToProcessParentIdWrongPath(); - $brokenParentIdEntries = $this->countResultsToProcessNonExistingParentIdEntry(); + $brokenPathEntries = $this->countResultsToProcessParentIdWrongPath($this->storageNumericId); + $brokenParentIdEntries = $this->countResultsToProcessNonExistingParentIdEntry($this->storageNumericId); $out->startProgress($brokenPathEntries + $brokenParentIdEntries); $totalFixed = 0; @@ -508,9 +512,16 @@ public function run(IOutput $out) { * This needs to be repaired by fixEntriesWithNonExistingParentIdEntry(), this is why * we need to keep this specific order of repair. */ - $totalFixed += $this->fixEntriesWithCorrectParentIdButWrongPath($out); + $affectedStorages = $this->fixEntriesWithCorrectParentIdButWrongPath($out, $this->storageNumericId); - $totalFixed += $this->fixEntriesWithNonExistingParentIdEntry($out); + if ($this->storageNumericId !== null) { + foreach ($affectedStorages as $storageNumericId) { + $this->fixEntriesWithNonExistingParentIdEntry($out, $storageNumericId); + } + } else { + // just fix all + $this->fixEntriesWithNonExistingParentIdEntry($out); + } $out->finishProgress(); } } From d77a845592d64cc1cafe3527e2560d0a3020c3ed Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 16 Aug 2017 11:24:09 +0200 Subject: [PATCH 04/13] occ files:scan --all --repair repairs all storages at once This instead of iterating over all storages which is way less efficient due to the 1-N nature of potential failed cross-storage moves that we are repairing. If singleuser mode is enabled and "--all --repair" is passed, all storages will be repaired in bulk (no repair filter). If not, it will fall back to iterating over each storage which is slower. --- apps/files/lib/Command/Scan.php | 43 ++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/apps/files/lib/Command/Scan.php b/apps/files/lib/Command/Scan.php index 5e90effdd5db..25adffdd0541 100644 --- a/apps/files/lib/Command/Scan.php +++ b/apps/files/lib/Command/Scan.php @@ -44,6 +44,7 @@ use OCP\Lock\ILockingProvider; use OCP\Lock\LockedException; use OCP\Files\IMimeTypeLoader; +use OCP\IConfig; class Scan extends Base { @@ -53,6 +54,8 @@ class Scan extends Base { private $lockingProvider; /** @var IMimeTypeLoader */ private $mimeTypeLoader; + /** @var IConfig */ + private $config; /** @var float */ protected $execTime = 0; /** @var int */ @@ -63,11 +66,13 @@ class Scan extends Base { public function __construct( IUserManager $userManager, ILockingProvider $lockingProvider, - IMimeTypeLoader $mimeTypeLoader + IMimeTypeLoader $mimeTypeLoader, + IConfig $config ) { $this->userManager = $userManager; $this->lockingProvider = $lockingProvider; $this->mimeTypeLoader = $mimeTypeLoader; + $this->config = $config; parent::__construct(); } @@ -128,6 +133,22 @@ public function checkScanWarning($fullPath, OutputInterface $output) { } } + /** + * Repair all storages at once + * + * @param OutputInterface $output + */ + protected function repairAll(OutputInterface $output) { + $connection = $this->reconnectToDatabase($output); + $repairStep = new RepairMismatchFileCachePath( + $connection, + $this->mimeTypeLoader + ); + $repairStep->setStorageNumericId(null); + $repairStep->setCountOnly(false); + $repairStep->run(new ConsoleOutput($output)); + } + protected function scanFiles($user, $path, $verbose, OutputInterface $output, $backgroundScan = false, $shouldRepair = false) { $connection = $this->reconnectToDatabase($output); $scanner = new \OC\Files\Utils\Scanner($user, $connection, \OC::$server->getLogger()); @@ -143,7 +164,7 @@ protected function scanFiles($user, $path, $verbose, OutputInterface $output, $b try { $repairStep = new RepairMismatchFileCachePath( $connection, - $mimeTypeLoader + $this->mimeTypeLoader ); $repairStep->setStorageNumericId($storage->getCache()->getNumericStorageId()); $repairStep->setCountOnly(false); @@ -217,11 +238,27 @@ protected function scanFiles($user, $path, $verbose, OutputInterface $output, $b protected function execute(InputInterface $input, OutputInterface $output) { $inputPath = $input->getOption('path'); + if ($input->getOption('repair')) { + $shouldRepairStoragesIndividually = true; + } + if ($inputPath) { $inputPath = '/' . trim($inputPath, '/'); list (, $user,) = explode('/', $inputPath, 3); $users = [$user]; } else if ($input->getOption('all')) { + // we can only repair all storages in bulk (more efficient) if singleuser or maintenance mode + // is enabled to prevent concurrent user access + if ($input->getOption('repair') && + ($this->config->getSystemValue('singleuser', false) || $this->config->getSystemValue('maintenance', false))) { + // repair all storages at once + $this->repairAll($output); + // don't fix individually + $shouldRepairStoragesIndividually = false; + } else { + $output->writeln("Repairing every storage individually is slower than repairing in bulk"); + $output->writeln("To repair in bulk, please switch to single user mode first: occ maintenance:singleuser --on"); + } $users = $this->userManager->search(''); } else { $users = $input->getArgument('user_id'); @@ -269,7 +306,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { if ($verbose) {$output->writeln(""); } $output->writeln("Starting scan for user $user_count out of $users_total ($user)"); # full: printout data if $verbose was set - $this->scanFiles($user, $path, $verbose, $output, $input->getOption('unscanned'), $input->getOption('repair')); + $this->scanFiles($user, $path, $verbose, $output, $input->getOption('unscanned'), $shouldRepairStoragesIndividually); } else { $output->writeln("Unknown user $user_count $user"); } From dbc38aa16b8693dcf3cab2ce8af2fe4968edbd79 Mon Sep 17 00:00:00 2001 From: Tom Needham Date: Fri, 18 Aug 2017 12:07:28 +0100 Subject: [PATCH 05/13] Improve output when repairing. Fix non-repair file:scan --- apps/files/lib/Command/Scan.php | 7 +++---- lib/private/Repair/RepairMismatchFileCachePath.php | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/files/lib/Command/Scan.php b/apps/files/lib/Command/Scan.php index 25adffdd0541..3b509559f201 100644 --- a/apps/files/lib/Command/Scan.php +++ b/apps/files/lib/Command/Scan.php @@ -238,9 +238,7 @@ protected function scanFiles($user, $path, $verbose, OutputInterface $output, $b protected function execute(InputInterface $input, OutputInterface $output) { $inputPath = $input->getOption('path'); - if ($input->getOption('repair')) { - $shouldRepairStoragesIndividually = true; - } + $shouldRepairStoragesIndividually = (bool) $input->getOption('repair'); if ($inputPath) { $inputPath = '/' . trim($inputPath, '/'); @@ -304,7 +302,8 @@ protected function execute(InputInterface $input, OutputInterface $output) { if ($this->userManager->userExists($user)) { # add an extra line when verbose is set to optical separate users if ($verbose) {$output->writeln(""); } - $output->writeln("Starting scan for user $user_count out of $users_total ($user)"); + $r = $shouldRepairStoragesIndividually ? ' (and repair)' : ''; + $output->writeln("Starting scan$r for user $user_count out of $users_total ($user)"); # full: printout data if $verbose was set $this->scanFiles($user, $path, $verbose, $output, $input->getOption('unscanned'), $shouldRepairStoragesIndividually); } else { diff --git a/lib/private/Repair/RepairMismatchFileCachePath.php b/lib/private/Repair/RepairMismatchFileCachePath.php index ccd30c4d376c..2fc0cc48680b 100644 --- a/lib/private/Repair/RepairMismatchFileCachePath.php +++ b/lib/private/Repair/RepairMismatchFileCachePath.php @@ -523,6 +523,7 @@ public function run(IOutput $out) { $this->fixEntriesWithNonExistingParentIdEntry($out); } $out->finishProgress(); + $out->info(''); } } } From 1cf9ba8c326401b2e1a18466aa6375f3e6ee21aa Mon Sep 17 00:00:00 2001 From: Tom Needham Date: Tue, 12 Sep 2017 13:54:07 +0100 Subject: [PATCH 06/13] Add parallel non-corrupt storages to filecache repair step to check for isolation --- apps/files/lib/Command/Scan.php | 4 +- lib/private/Files/Utils/Scanner.php | 3 +- .../Repair/RepairMismatchFileCachePath.php | 4 +- .../RepairMismatchFileCachePathTest.php | 221 +++++++++++++++++- 4 files changed, 219 insertions(+), 13 deletions(-) diff --git a/apps/files/lib/Command/Scan.php b/apps/files/lib/Command/Scan.php index 3b509559f201..511d744439a7 100644 --- a/apps/files/lib/Command/Scan.php +++ b/apps/files/lib/Command/Scan.php @@ -254,8 +254,8 @@ protected function execute(InputInterface $input, OutputInterface $output) { // don't fix individually $shouldRepairStoragesIndividually = false; } else { - $output->writeln("Repairing every storage individually is slower than repairing in bulk"); - $output->writeln("To repair in bulk, please switch to single user mode first: occ maintenance:singleuser --on"); + $output->writeln("Repairing every storage individually is slower than repairing in bulk"); + $output->writeln("To repair in bulk, please switch to single user mode first: occ maintenance:singleuser --on"); } $users = $this->userManager->search(''); } else { diff --git a/lib/private/Files/Utils/Scanner.php b/lib/private/Files/Utils/Scanner.php index 6f3490d63540..5314cddcc8a5 100644 --- a/lib/private/Files/Utils/Scanner.php +++ b/lib/private/Files/Utils/Scanner.php @@ -196,8 +196,7 @@ public function scan($dir = '') { } // if the home storage isn't writable then the scanner is run as the wrong user - $isHome = $storage->instanceOfStorage('\OC\Files\Storage\Home'); - if ($isHome and + if ($storage->instanceOfStorage('\OC\Files\Storage\Home') and (!$storage->isCreatable('') or !$storage->isCreatable('files')) ) { if ($storage->file_exists('') or $storage->getCache()->inCache('')) { diff --git a/lib/private/Repair/RepairMismatchFileCachePath.php b/lib/private/Repair/RepairMismatchFileCachePath.php index 2fc0cc48680b..e9f39bee8ebb 100644 --- a/lib/private/Repair/RepairMismatchFileCachePath.php +++ b/lib/private/Repair/RepairMismatchFileCachePath.php @@ -232,7 +232,9 @@ private function reportAffectedStoragesParentIdWrongPath(IOutput $out) { if (!empty($storageIds)) { $out->warning('The file cache contains entries with invalid path values for the following storage numeric ids: ' . implode(' ', $storageIds)); - $out->warning('Please run `occ files:scan --all --repair` to repair all affected storages'); + $out->warning('Please run `occ files:scan --all --repair` to repair' + .'all affected storages or run `occ files:scan userid --repair for ' + .'each user with affected storages'); } } diff --git a/tests/lib/Repair/RepairMismatchFileCachePathTest.php b/tests/lib/Repair/RepairMismatchFileCachePathTest.php index 320a3acac77b..7c4497a8ed15 100644 --- a/tests/lib/Repair/RepairMismatchFileCachePathTest.php +++ b/tests/lib/Repair/RepairMismatchFileCachePathTest.php @@ -261,12 +261,6 @@ public function testRepairEntry($sourceStorageId, $targetStorageId, $targetDir, $this->assertEquals('files/source/do_not_touch', $entry['path']); $this->assertEquals(md5('files/source/do_not_touch'), $entry['path_hash']); - // root entry left alone - $entry = $this->getFileCacheEntry($rootId1); - $this->assertEquals(-1, $entry['parent']); - $this->assertEquals((string)$sourceStorageId, $entry['storage']); - $this->assertEquals('', $entry['path']); - $this->assertEquals(md5(''), $entry['path_hash']); } /** @@ -274,12 +268,16 @@ public function testRepairEntry($sourceStorageId, $targetStorageId, $targetDir, */ public function testRepairSelfReferencing() { /** + * This is the storage tree that is created + * (alongside a normal storage, without corruption, but same structure) + * + * * Self-referencing: * - files/all_your_zombies (parent=fileid must be reparented) * * Referencing child one level: * - files/ref_child1 (parent=fileid of the child) - * - files/ref_child1/child (parent=fileid of the child) + * - files/ref_child1/child * * Referencing child two levels: * - files/ref_child2/ (parent=fileid of the child's child) @@ -289,7 +287,13 @@ public function testRepairSelfReferencing() { * Referencing child two levels detached: * - detached/ref_child3/ (parent=fileid of the child, "detached" has no entry) * - detached/ref_child3/child + * + * Normal files that should be untouched + * - files/untouched_folder + * - files/untouched.file */ + + // Test, corrupt storage $storageId = 1; $rootId1 = $this->createFileCacheEntry($storageId, ''); $baseId1 = $this->createFileCacheEntry($storageId, 'files', $rootId1); @@ -308,11 +312,39 @@ public function testRepairSelfReferencing() { // make it reference its own sub child $this->setFileCacheEntryParent($refChild2Id, $refChild2ChildChildId); - $refChild3Id = $this->createFileCacheEntry($storageId, 'detached/ref_child3', $baseId1); + $willBeOverwritten = -1; + $refChild3Id = $this->createFileCacheEntry($storageId, 'detached/ref_child3', $willBeOverwritten); $refChild3ChildId = $this->createFileCacheEntry($storageId, 'detached/ref_child3/child', $refChild3Id); // make it reference its own child $this->setFileCacheEntryParent($refChild3Id, $refChild3ChildId); + $untouchedFileId = $this->createFileCacheEntry($storageId, 'files/untouched.file', $baseId1); + $untouchedFolderId = $this->createFileCacheEntry($storageId, 'files/untouched_folder', $baseId1); + // End correct storage + + // Parallel, correct, but identical storage - used to check for storage isolation and query scope + $storageId2 = 2; + $rootId2 = $this->createFileCacheEntry($storageId2, ''); + $baseId2 = $this->createFileCacheEntry($storageId2, 'files', $rootId2); + + $selfRefId_parallel = $this->createFileCacheEntry($storageId2, 'files/all_your_zombies', $baseId2); + + $refChild1Id_parallel = $this->createFileCacheEntry($storageId2, 'files/ref_child1', $baseId2); + $refChild1ChildId_parallel = $this->createFileCacheEntry($storageId2, 'files/ref_child1/child', $refChild1Id_parallel); + + $refChild2Id_parallel = $this->createFileCacheEntry($storageId2, 'files/ref_child2', $baseId2); + $refChild2ChildId_parallel = $this->createFileCacheEntry($storageId2, 'files/ref_child2/child', $refChild2Id_parallel); + $refChild2ChildChildId_parallel = $this->createFileCacheEntry($storageId2, 'files/ref_child2/child/child', $refChild2ChildId_parallel); + + $refChild3DetachedId_parallel = $this->createFileCacheEntry($storageId2, 'detached', $rootId2); + $refChild3Id_parallel = $this->createFileCacheEntry($storageId2, 'detached/ref_child3', $refChild3DetachedId_parallel); + $refChild3ChildId_parallel = $this->createFileCacheEntry($storageId2, 'detached/ref_child3/child', $refChild3Id_parallel); + + $untouchedFileId_parallel = $this->createFileCacheEntry($storageId2, 'files/untouched.file', $baseId2); + $untouchedFolderId_parallel = $this->createFileCacheEntry($storageId2, 'files/untouched_folder', $baseId2); + // End parallel storage + + $outputMock = $this->createMock(IOutput::class); $this->repair->setStorageNumericId($storageId); $this->repair->run($outputMock); @@ -386,8 +418,106 @@ public function testRepairSelfReferencing() { $this->assertEquals((string)$storageId, $entry['storage']); $this->assertEquals('detached', $entry['path']); $this->assertEquals(md5('detached'), $entry['path_hash']); + + // untouched file and folder are untouched + $entry = $this->getFileCacheEntry($untouchedFileId); + $this->assertEquals($baseId1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('files/untouched.file', $entry['path']); + $this->assertEquals(md5('files/untouched.file'), $entry['path_hash']); + $entry = $this->getFileCacheEntry($untouchedFolderId); + $this->assertEquals($baseId1, $entry['parent']); + $this->assertEquals((string)$storageId, $entry['storage']); + $this->assertEquals('files/untouched_folder', $entry['path']); + $this->assertEquals(md5('files/untouched_folder'), $entry['path_hash']); + + // check that parallel storage is untouched + // self-referencing updated + $entry = $this->getFileCacheEntry($selfRefId_parallel); + $this->assertEquals($baseId2, $entry['parent']); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('files/all_your_zombies', $entry['path']); + $this->assertEquals(md5('files/all_your_zombies'), $entry['path_hash']); + + // ref child 1 case was reparented to "files" + $entry = $this->getFileCacheEntry($refChild1Id_parallel); + $this->assertEquals($baseId2, $entry['parent']); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('files/ref_child1', $entry['path']); + $this->assertEquals(md5('files/ref_child1'), $entry['path_hash']); + + // ref child 1 child left alone + $entry = $this->getFileCacheEntry($refChild1ChildId_parallel); + $this->assertEquals($refChild1Id_parallel, $entry['parent']); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('files/ref_child1/child', $entry['path']); + $this->assertEquals(md5('files/ref_child1/child'), $entry['path_hash']); + + // ref child 2 case was reparented to "files" + $entry = $this->getFileCacheEntry($refChild2Id_parallel); + $this->assertEquals($baseId2, $entry['parent']); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('files/ref_child2', $entry['path']); + $this->assertEquals(md5('files/ref_child2'), $entry['path_hash']); + + // ref child 2 child left alone + $entry = $this->getFileCacheEntry($refChild2ChildId_parallel); + $this->assertEquals($refChild2Id_parallel, $entry['parent']); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('files/ref_child2/child', $entry['path']); + $this->assertEquals(md5('files/ref_child2/child'), $entry['path_hash']); + + // ref child 2 child child left alone + $entry = $this->getFileCacheEntry($refChild2ChildChildId_parallel); + $this->assertEquals($refChild2ChildId_parallel, $entry['parent']); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('files/ref_child2/child/child', $entry['path']); + $this->assertEquals(md5('files/ref_child2/child/child'), $entry['path_hash']); + + // root entry left alone + $entry = $this->getFileCacheEntry($rootId2); + $this->assertEquals(-1, $entry['parent']); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('', $entry['path']); + $this->assertEquals(md5(''), $entry['path_hash']); + + // ref child 3 child left alone + $entry = $this->getFileCacheEntry($refChild3ChildId_parallel); + $this->assertEquals($refChild3Id_parallel, $entry['parent']); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('detached/ref_child3/child', $entry['path']); + $this->assertEquals(md5('detached/ref_child3/child'), $entry['path_hash']); + + // ref child 3 case was reparented to a new "detached" entry + $entry = $this->getFileCacheEntry($refChild3Id_parallel); + $this->assertTrue(isset($entry['parent'])); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('detached/ref_child3', $entry['path']); + $this->assertEquals(md5('detached/ref_child3'), $entry['path_hash']); + + // entry "detached" was untouched + $entry = $this->getFileCacheEntry($entry['parent']); + $this->assertEquals($rootId2, $entry['parent']); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('detached', $entry['path']); + $this->assertEquals(md5('detached'), $entry['path_hash']); + + // untouched file and folder are untouched + $entry = $this->getFileCacheEntry($untouchedFileId_parallel); + $this->assertEquals($baseId2, $entry['parent']); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('files/untouched.file', $entry['path']); + $this->assertEquals(md5('files/untouched.file'), $entry['path_hash']); + $entry = $this->getFileCacheEntry($untouchedFolderId_parallel); + $this->assertEquals($baseId2, $entry['parent']); + $this->assertEquals((string)$storageId2, $entry['storage']); + $this->assertEquals('files/untouched_folder', $entry['path']); + $this->assertEquals(md5('files/untouched_folder'), $entry['path_hash']); + + // end testing parallel storage } + /** * Test repair wrong parent id */ @@ -439,6 +569,8 @@ public function testRepairDetachedSubtree() { * - files/missingdir/orphaned1 (orphaned entry as "missingdir" is missing) * - missingdir/missingdir1/orphaned2 (orphaned entry two levels up to root) */ + + // Corrupt storage $storageId = 1; $rootId1 = $this->createFileCacheEntry($storageId, ''); $baseId1 = $this->createFileCacheEntry($storageId, 'files', $rootId1); @@ -448,6 +580,18 @@ public function testRepairDetachedSubtree() { $nonExistingParentId2 = $this->createNonExistingId(); $orphanedId2 = $this->createFileCacheEntry($storageId, 'missingdir/missingdir1/orphaned2', $nonExistingParentId2); + // end corrupt storage + + // Parallel test storage + $storageId_parallel = 2; + $rootId1_parallel = $this->createFileCacheEntry($storageId_parallel, ''); + $baseId1_parallel = $this->createFileCacheEntry($storageId_parallel, 'files', $rootId1_parallel); + $notOrphanedFolder_parallel = $this->createFileCacheEntry($storageId_parallel, 'files/missingdir', $baseId1_parallel); + $notOrphanedId1_parallel = $this->createFileCacheEntry($storageId_parallel, 'files/missingdir/orphaned1', $notOrphanedFolder_parallel); + $notOrphanedFolder2_parallel = $this->createFileCacheEntry($storageId_parallel, 'missingdir', $rootId1_parallel); + $notOrphanedFolderChild2_parallel = $this->createFileCacheEntry($storageId_parallel, 'missingdir/missingdir1', $notOrphanedFolder2_parallel); + $notOrphanedId2_parallel = $this->createFileCacheEntry($storageId_parallel, 'missingdir/missingdir1/orphaned2', $notOrphanedFolder2_parallel); + // end parallel test storage $outputMock = $this->createMock(IOutput::class); $this->repair->setStorageNumericId($storageId); @@ -494,6 +638,49 @@ public function testRepairDetachedSubtree() { $this->assertEquals((string)$storageId, $entry['storage']); $this->assertEquals('', $entry['path']); $this->assertEquals(md5(''), $entry['path_hash']); + + // now check the parallel storage is intact + // orphaned entry reattached + $entry = $this->getFileCacheEntry($notOrphanedId1_parallel); + $this->assertEquals($notOrphanedFolder_parallel, $entry['parent']); + $this->assertEquals((string)$storageId_parallel, $entry['storage']); + $this->assertEquals('files/missingdir/orphaned1', $entry['path']); + $this->assertEquals(md5('files/missingdir/orphaned1'), $entry['path_hash']); + + // not orphaned folder still exists + $entry = $this->getFileCacheEntry($notOrphanedFolder_parallel); + $this->assertEquals($baseId1_parallel, $entry['parent']); + $this->assertEquals((string)$storageId_parallel, $entry['storage']); + $this->assertEquals('files/missingdir', $entry['path']); + $this->assertEquals(md5('files/missingdir'), $entry['path_hash']); + + // not orphaned entry still exits + $entry = $this->getFileCacheEntry($notOrphanedId2_parallel); + $this->assertEquals($notOrphanedFolder2_parallel, $entry['parent']); + $this->assertEquals((string)$storageId_parallel, $entry['storage']); + $this->assertEquals('missingdir/missingdir1/orphaned2', $entry['path']); + $this->assertEquals(md5('missingdir/missingdir1/orphaned2'), $entry['path_hash']); + + // non existing id exists now + $entry = $this->getFileCacheEntry($notOrphanedFolderChild2_parallel); + $this->assertEquals($notOrphanedFolder2_parallel, $entry['parent']); + $this->assertEquals((string)$storageId_parallel, $entry['storage']); + $this->assertEquals('missingdir/missingdir1', $entry['path']); + $this->assertEquals(md5('missingdir/missingdir1'), $entry['path_hash']); + + // non existing id parent exists now + $entry = $this->getFileCacheEntry($notOrphanedFolder2_parallel); + $this->assertEquals($rootId1_parallel, $entry['parent']); + $this->assertEquals((string)$storageId_parallel, $entry['storage']); + $this->assertEquals('missingdir', $entry['path']); + $this->assertEquals(md5('missingdir'), $entry['path_hash']); + + // root entry left alone + $entry = $this->getFileCacheEntry($rootId1_parallel); + $this->assertEquals(-1, $entry['parent']); + $this->assertEquals((string)$storageId_parallel, $entry['storage']); + $this->assertEquals('', $entry['path']); + $this->assertEquals(md5(''), $entry['path_hash']); } /** @@ -507,6 +694,12 @@ public function testRepairMissingRoot() { $nonExistingParentId = $this->createNonExistingId(); $orphanedId = $this->createFileCacheEntry($storageId, 'noroot', $nonExistingParentId); + // Test parallel storage which should be untouched by the repair operation + $testStorageId = 2; + $baseId = $this->createFileCacheEntry($testStorageId, ''); + $noRootid = $this->createFileCacheEntry($testStorageId, 'noroot', $baseId); + + $outputMock = $this->createMock(IOutput::class); $this->repair->setStorageNumericId($storageId); $this->repair->run($outputMock); @@ -524,6 +717,18 @@ public function testRepairMissingRoot() { $this->assertEquals((string)$storageId, $entry['storage']); $this->assertEquals('', $entry['path']); $this->assertEquals(md5(''), $entry['path_hash']); + + // Check that the parallel test storage is still intact + $entry = $this->getFileCacheEntry($noRootid); + $this->assertEquals($baseId, $entry['parent']); + $this->assertEquals((string)$testStorageId, $entry['storage']); + $this->assertEquals('noroot', $entry['path']); + $this->assertEquals(md5('noroot'), $entry['path_hash']); + $entry = $this->getFileCacheEntry($baseId); + $this->assertEquals(-1, $entry['parent']); + $this->assertEquals((string)$testStorageId, $entry['storage']); + $this->assertEquals('', $entry['path']); + $this->assertEquals(md5(''), $entry['path_hash']); } } From b6abf78b8f63fadc76afc7c35b4b45017b932683 Mon Sep 17 00:00:00 2001 From: Tom Needham Date: Thu, 14 Sep 2017 12:16:12 +0100 Subject: [PATCH 07/13] Test without parallel storages --- tests/lib/Repair/RepairMismatchFileCachePathTest.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/lib/Repair/RepairMismatchFileCachePathTest.php b/tests/lib/Repair/RepairMismatchFileCachePathTest.php index 7c4497a8ed15..fb6ffd96ec28 100644 --- a/tests/lib/Repair/RepairMismatchFileCachePathTest.php +++ b/tests/lib/Repair/RepairMismatchFileCachePathTest.php @@ -583,6 +583,7 @@ public function testRepairDetachedSubtree() { // end corrupt storage // Parallel test storage + /* $storageId_parallel = 2; $rootId1_parallel = $this->createFileCacheEntry($storageId_parallel, ''); $baseId1_parallel = $this->createFileCacheEntry($storageId_parallel, 'files', $rootId1_parallel); @@ -592,6 +593,7 @@ public function testRepairDetachedSubtree() { $notOrphanedFolderChild2_parallel = $this->createFileCacheEntry($storageId_parallel, 'missingdir/missingdir1', $notOrphanedFolder2_parallel); $notOrphanedId2_parallel = $this->createFileCacheEntry($storageId_parallel, 'missingdir/missingdir1/orphaned2', $notOrphanedFolder2_parallel); // end parallel test storage + */ $outputMock = $this->createMock(IOutput::class); $this->repair->setStorageNumericId($storageId); @@ -599,7 +601,7 @@ public function testRepairDetachedSubtree() { // orphaned entry reattached $entry = $this->getFileCacheEntry($orphanedId1); - $this->assertEquals($nonExistingParentId, $entry['parent']); + $this->assertEquals($nonExistingParentId, $entry['parent']); // this row fails, $entry['parent'] seems to equal a similar but different value $this->assertEquals((string)$storageId, $entry['storage']); $this->assertEquals('files/missingdir/orphaned1', $entry['path']); $this->assertEquals(md5('files/missingdir/orphaned1'), $entry['path_hash']); @@ -640,6 +642,7 @@ public function testRepairDetachedSubtree() { $this->assertEquals(md5(''), $entry['path_hash']); // now check the parallel storage is intact + /* // orphaned entry reattached $entry = $this->getFileCacheEntry($notOrphanedId1_parallel); $this->assertEquals($notOrphanedFolder_parallel, $entry['parent']); @@ -681,6 +684,7 @@ public function testRepairDetachedSubtree() { $this->assertEquals((string)$storageId_parallel, $entry['storage']); $this->assertEquals('', $entry['path']); $this->assertEquals(md5(''), $entry['path_hash']); + */ } /** @@ -695,9 +699,11 @@ public function testRepairMissingRoot() { $orphanedId = $this->createFileCacheEntry($storageId, 'noroot', $nonExistingParentId); // Test parallel storage which should be untouched by the repair operation + /* $testStorageId = 2; $baseId = $this->createFileCacheEntry($testStorageId, ''); $noRootid = $this->createFileCacheEntry($testStorageId, 'noroot', $baseId); + */ $outputMock = $this->createMock(IOutput::class); @@ -713,11 +719,12 @@ public function testRepairMissingRoot() { // recreated root entry $entry = $this->getFileCacheEntry($entry['parent']); - $this->assertEquals(-1, $entry['parent']); + $this->assertEquals(-1, $entry['parent']); // this row fails, it appears to get attached to another entry, not the root (fileid ~ 4000) $this->assertEquals((string)$storageId, $entry['storage']); $this->assertEquals('', $entry['path']); $this->assertEquals(md5(''), $entry['path_hash']); + /* // Check that the parallel test storage is still intact $entry = $this->getFileCacheEntry($noRootid); $this->assertEquals($baseId, $entry['parent']); @@ -729,6 +736,7 @@ public function testRepairMissingRoot() { $this->assertEquals((string)$testStorageId, $entry['storage']); $this->assertEquals('', $entry['path']); $this->assertEquals(md5(''), $entry['path_hash']); + */ } } From f2d55443fbbca2ffdc62569b463b5bb55d4b05bd Mon Sep 17 00:00:00 2001 From: Tom Needham Date: Thu, 14 Sep 2017 14:47:07 +0100 Subject: [PATCH 08/13] Fix lastinsertid for postgres wehn reusing fileid --- lib/private/Repair/RepairMismatchFileCachePath.php | 14 +++++++++++--- lib/public/IDBConnection.php | 3 +++ .../lib/Repair/RepairMismatchFileCachePathTest.php | 8 -------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/private/Repair/RepairMismatchFileCachePath.php b/lib/private/Repair/RepairMismatchFileCachePath.php index e9f39bee8ebb..46e57f64cd2e 100644 --- a/lib/private/Repair/RepairMismatchFileCachePath.php +++ b/lib/private/Repair/RepairMismatchFileCachePath.php @@ -21,10 +21,11 @@ namespace OC\Repair; +use Doctrine\DBAL\Platforms\PostgreSqlPlatform; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Migration\IOutput; use OCP\Migration\IRepairStep; use Doctrine\DBAL\Platforms\MySqlPlatform; -use OCP\DB\QueryBuilder\IQueryBuilder; use Doctrine\DBAL\Platforms\OraclePlatform; use OCP\Files\IMimeTypeLoader; use OCP\IDBConnection; @@ -121,7 +122,7 @@ private function fixEntryPath(IOutput $out, $fileId, $wrongPath, $correctStorage $out->advance(1, $text); } - private function addQueryConditionsParentIdWrongPath($qb, $storageNumericId) { + private function addQueryConditionsParentIdWrongPath($qb) { // thanks, VicDeo! if ($this->connection->getDatabasePlatform() instanceof MySqlPlatform) { $concatFunction = $qb->createFunction("CONCAT(fcp.path, '/', fc.name)"); @@ -399,7 +400,14 @@ private function getOrCreateEntry($storageId, $path, $reuseFileId = null) { } $qb->insert('filecache')->values($values); $qb->execute(); - return $this->connection->lastInsertId('*PREFIX*filecache'); + + // If we reused the fileid then this is the id to return + if($reuseFileId !== null) { + return $reuseFileId; + } else { + // Else we inserted a new row with auto generated id, use that + return $this->connection->lastInsertId('*PREFIX*filecache'); + } } /** diff --git a/lib/public/IDBConnection.php b/lib/public/IDBConnection.php index ac09a680f22d..788aba37e331 100644 --- a/lib/public/IDBConnection.php +++ b/lib/public/IDBConnection.php @@ -92,6 +92,9 @@ public function executeUpdate($query, array $params = [], array $types = []); /** * Used to get the id of the just inserted element + * Note: On postgres platform, this will return the last sequence id which + * may not be the id last inserted if you were reinserting a previously + * used auto_increment id. * @param string $table the name of the table where we inserted the item * @return int the id of the inserted element * @since 6.0.0 diff --git a/tests/lib/Repair/RepairMismatchFileCachePathTest.php b/tests/lib/Repair/RepairMismatchFileCachePathTest.php index fb6ffd96ec28..7c74766187b9 100644 --- a/tests/lib/Repair/RepairMismatchFileCachePathTest.php +++ b/tests/lib/Repair/RepairMismatchFileCachePathTest.php @@ -583,7 +583,6 @@ public function testRepairDetachedSubtree() { // end corrupt storage // Parallel test storage - /* $storageId_parallel = 2; $rootId1_parallel = $this->createFileCacheEntry($storageId_parallel, ''); $baseId1_parallel = $this->createFileCacheEntry($storageId_parallel, 'files', $rootId1_parallel); @@ -593,7 +592,6 @@ public function testRepairDetachedSubtree() { $notOrphanedFolderChild2_parallel = $this->createFileCacheEntry($storageId_parallel, 'missingdir/missingdir1', $notOrphanedFolder2_parallel); $notOrphanedId2_parallel = $this->createFileCacheEntry($storageId_parallel, 'missingdir/missingdir1/orphaned2', $notOrphanedFolder2_parallel); // end parallel test storage - */ $outputMock = $this->createMock(IOutput::class); $this->repair->setStorageNumericId($storageId); @@ -642,7 +640,6 @@ public function testRepairDetachedSubtree() { $this->assertEquals(md5(''), $entry['path_hash']); // now check the parallel storage is intact - /* // orphaned entry reattached $entry = $this->getFileCacheEntry($notOrphanedId1_parallel); $this->assertEquals($notOrphanedFolder_parallel, $entry['parent']); @@ -684,7 +681,6 @@ public function testRepairDetachedSubtree() { $this->assertEquals((string)$storageId_parallel, $entry['storage']); $this->assertEquals('', $entry['path']); $this->assertEquals(md5(''), $entry['path_hash']); - */ } /** @@ -699,11 +695,9 @@ public function testRepairMissingRoot() { $orphanedId = $this->createFileCacheEntry($storageId, 'noroot', $nonExistingParentId); // Test parallel storage which should be untouched by the repair operation - /* $testStorageId = 2; $baseId = $this->createFileCacheEntry($testStorageId, ''); $noRootid = $this->createFileCacheEntry($testStorageId, 'noroot', $baseId); - */ $outputMock = $this->createMock(IOutput::class); @@ -724,7 +718,6 @@ public function testRepairMissingRoot() { $this->assertEquals('', $entry['path']); $this->assertEquals(md5(''), $entry['path_hash']); - /* // Check that the parallel test storage is still intact $entry = $this->getFileCacheEntry($noRootid); $this->assertEquals($baseId, $entry['parent']); @@ -736,7 +729,6 @@ public function testRepairMissingRoot() { $this->assertEquals((string)$testStorageId, $entry['storage']); $this->assertEquals('', $entry['path']); $this->assertEquals(md5(''), $entry['path_hash']); - */ } } From 5d4fb0a1897060656bab99be37e14bcb4fa2cbc8 Mon Sep 17 00:00:00 2001 From: Tom Needham Date: Fri, 15 Sep 2017 17:10:37 +0100 Subject: [PATCH 09/13] Try oracle tests without throwing exception for stray transactions --- tests/lib/TestCase.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index 7b841305e7e5..6167512612b8 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -221,7 +221,11 @@ public static function tearDownAfterClass() { } $dataDir = \OC::$server->getConfig()->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data-autotest'); if (self::$wasDatabaseAllowed && \OC::$server->getDatabaseConnection()) { - $queryBuilder = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $connection = \OC::$server->getDatabaseConnection(); + if ($connection->inTransaction()) { + //throw new \Exception('Stray transaction in test class ' . get_called_class()); + } + $queryBuilder = $connection->getQueryBuilder(); self::tearDownAfterClassCleanShares($queryBuilder); self::tearDownAfterClassCleanStorages($queryBuilder); From bec6482c2b006ff7a7076aee7ff1832d5855757c Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 18 Sep 2017 18:51:50 +0200 Subject: [PATCH 10/13] Oracle hates empty strings --- .../Repair/RepairMismatchFileCachePath.php | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/private/Repair/RepairMismatchFileCachePath.php b/lib/private/Repair/RepairMismatchFileCachePath.php index 46e57f64cd2e..a447ecce8a73 100644 --- a/lib/private/Repair/RepairMismatchFileCachePath.php +++ b/lib/private/Repair/RepairMismatchFileCachePath.php @@ -103,8 +103,13 @@ private function fixEntryPath(IOutput $out, $fileId, $wrongPath, $correctStorage // delete target if exists $qb = $this->connection->getQueryBuilder(); $qb->delete('filecache') - ->where($qb->expr()->eq('storage', $qb->createNamedParameter($correctStorageNumericId))) - ->andWhere($qb->expr()->eq('path', $qb->createNamedParameter($correctPath))); + ->where($qb->expr()->eq('storage', $qb->createNamedParameter($correctStorageNumericId))); + + if ($correctPath === '' && $this->connection->getDatabasePlatform() instanceof OraclePlatform) { + $qb->andWhere($qb->expr()->isNull('path')); + } else { + $qb->andWhere($qb->expr()->eq('path', $qb->createNamedParameter($correctPath))); + } $entryExisted = $qb->execute() > 0; $qb = $this->connection->getQueryBuilder(); @@ -363,8 +368,13 @@ private function getOrCreateEntry($storageId, $path, $reuseFileId = null) { // from oc_filecache ->from('filecache') // where storage=$storage and path='$parentPath' - ->where($qb->expr()->eq('storage', $qb->createNamedParameter($storageId))) - ->andWhere($qb->expr()->eq('path', $qb->createNamedParameter($path))); + ->where($qb->expr()->eq('storage', $qb->createNamedParameter($storageId))); + + if ($path === '' && $this->connection->getDatabasePlatform() instanceof OraclePlatform) { + $qb->andWhere($qb->expr()->isNull('path')); + } else { + $qb->andWhere($qb->expr()->eq('path', $qb->createNamedParameter($path))); + } $results = $qb->execute(); $rows = $results->fetchAll(); $results->closeCursor(); From ffa7cbd8987136155231f16fb05d41d03b6381b3 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 19 Sep 2017 10:44:13 +0200 Subject: [PATCH 11/13] Workaround for Oracle trigger for fileid --- lib/private/Repair/RepairMismatchFileCachePath.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/private/Repair/RepairMismatchFileCachePath.php b/lib/private/Repair/RepairMismatchFileCachePath.php index a447ecce8a73..57473c4b6899 100644 --- a/lib/private/Repair/RepairMismatchFileCachePath.php +++ b/lib/private/Repair/RepairMismatchFileCachePath.php @@ -413,6 +413,20 @@ private function getOrCreateEntry($storageId, $path, $reuseFileId = null) { // If we reused the fileid then this is the id to return if($reuseFileId !== null) { + // with Oracle, the trigger gets in the way and does not let us specify + // a fileid value on insert + if ($this->connection->getDatabasePlatform() instanceof OraclePlatform) { + $lastFileId = $this->connection->lastInsertId('*PREFIX*filecache'); + if ($reuseFileId !== $lastFileId) { + // use update to set it directly + $qb = $this->connection->getQueryBuilder(); + $qb->update('filecache') + ->set('fileid', $qb->createNamedParameter($reuseFileId)) + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($lastFileId))); + $qb->execute(); + } + } + return $reuseFileId; } else { // Else we inserted a new row with auto generated id, use that From c53e00a42c81cc31228941c1d802242ba0544fb7 Mon Sep 17 00:00:00 2001 From: Tom Needham Date: Wed, 20 Sep 2017 11:05:48 +0200 Subject: [PATCH 12/13] Cleanup testcase and comments --- tests/lib/Repair/RepairMismatchFileCachePathTest.php | 4 ++-- tests/lib/TestCase.php | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/lib/Repair/RepairMismatchFileCachePathTest.php b/tests/lib/Repair/RepairMismatchFileCachePathTest.php index 7c74766187b9..7c4497a8ed15 100644 --- a/tests/lib/Repair/RepairMismatchFileCachePathTest.php +++ b/tests/lib/Repair/RepairMismatchFileCachePathTest.php @@ -599,7 +599,7 @@ public function testRepairDetachedSubtree() { // orphaned entry reattached $entry = $this->getFileCacheEntry($orphanedId1); - $this->assertEquals($nonExistingParentId, $entry['parent']); // this row fails, $entry['parent'] seems to equal a similar but different value + $this->assertEquals($nonExistingParentId, $entry['parent']); $this->assertEquals((string)$storageId, $entry['storage']); $this->assertEquals('files/missingdir/orphaned1', $entry['path']); $this->assertEquals(md5('files/missingdir/orphaned1'), $entry['path_hash']); @@ -713,7 +713,7 @@ public function testRepairMissingRoot() { // recreated root entry $entry = $this->getFileCacheEntry($entry['parent']); - $this->assertEquals(-1, $entry['parent']); // this row fails, it appears to get attached to another entry, not the root (fileid ~ 4000) + $this->assertEquals(-1, $entry['parent']); $this->assertEquals((string)$storageId, $entry['storage']); $this->assertEquals('', $entry['path']); $this->assertEquals(md5(''), $entry['path_hash']); diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index 6167512612b8..ffafdd7d58d7 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -222,9 +222,6 @@ public static function tearDownAfterClass() { $dataDir = \OC::$server->getConfig()->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data-autotest'); if (self::$wasDatabaseAllowed && \OC::$server->getDatabaseConnection()) { $connection = \OC::$server->getDatabaseConnection(); - if ($connection->inTransaction()) { - //throw new \Exception('Stray transaction in test class ' . get_called_class()); - } $queryBuilder = $connection->getQueryBuilder(); self::tearDownAfterClassCleanShares($queryBuilder); From 120c9a63348573bb45ae0425c049a22858845a1c Mon Sep 17 00:00:00 2001 From: Tom Needham Date: Tue, 26 Sep 2017 14:10:51 +0200 Subject: [PATCH 13/13] Add repair step to repair list --- lib/private/Repair.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/private/Repair.php b/lib/private/Repair.php index fe9e6ba756a6..62e817b9ff39 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -37,6 +37,7 @@ use OC\Repair\OldGroupMembershipShares; use OC\Repair\RemoveGetETagEntries; use OC\Repair\RemoveRootShares; +use OC\Repair\RepairMismatchFileCachePath; use OC\Repair\SharePropagation; use OC\Repair\SqliteAutoincrement; use OC\Repair\DropOldTables; @@ -126,6 +127,7 @@ public function addStep($repairStep) { public static function getRepairSteps() { return [ new RepairMimeTypes(\OC::$server->getConfig()), + new RepairMismatchFileCachePath(\OC::$server->getDatabaseConnection(), \OC::$server->getMimeTypeLoader()), new FillETags(\OC::$server->getDatabaseConnection()), new CleanTags(\OC::$server->getDatabaseConnection(), \OC::$server->getUserManager()), new DropOldTables(\OC::$server->getDatabaseConnection()),