From f8d368c81c3fd9fb1d815f836c6140e3a307dd72 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Fri, 11 May 2018 14:27:01 +0200 Subject: [PATCH 1/2] - Ignore files which are deleted from disk - Show user to which the scanned file belongs --- apps/files/lib/Command/VerifyChecksums.php | 24 +++++++++++++++---- .../tests/Command/VerifyChecksumsTest.php | 6 ++--- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/apps/files/lib/Command/VerifyChecksums.php b/apps/files/lib/Command/VerifyChecksums.php index 88757e24351b..b248fc912ea9 100644 --- a/apps/files/lib/Command/VerifyChecksums.php +++ b/apps/files/lib/Command/VerifyChecksums.php @@ -49,7 +49,6 @@ class VerifyChecksums extends Command { * @var IRootFolder */ private $rootFolder; - /** * @var IUserManager */ @@ -94,19 +93,24 @@ public function execute(InputInterface $input, OutputInterface $output) { $walkFunction = function (Node $node) use ($input, $output) { $path = $node->getInternalPath(); $currentChecksums = $node->getChecksum(); + $owner = $node->getOwner()->getUID(); + + if (!self::fileExistsOnDisk($node)) { + $output->writeln("Skipping $owner/$path => File is in file-cache but doesn`t exist on storage/disk", OutputInterface::VERBOSITY_VERBOSE); + return; + } // Files without calculated checksum can't cause checksum errors if (empty($currentChecksums)) { - $output->writeln("Skipping $path => No Checksum", OutputInterface::VERBOSITY_VERBOSE); + $output->writeln("Skipping $owner/$path => No Checksum", OutputInterface::VERBOSITY_VERBOSE); return; } - $output->writeln("Checking $path => $currentChecksums", OutputInterface::VERBOSITY_VERBOSE); + $output->writeln("Checking $owner/$path => $currentChecksums", OutputInterface::VERBOSITY_VERBOSE); $actualChecksums = self::calculateActualChecksums($path, $node->getStorage()); - if ($actualChecksums !== $currentChecksums) { $output->writeln( - "Mismatch for $path:\n Filecache:\t$currentChecksums\n Actual:\t$actualChecksums" + "Mismatch for $owner/$path:\n Filecache:\t$currentChecksums\n Actual:\t$actualChecksums" ); $this->exitStatus = self::EXIT_CHECKSUM_ERRORS; @@ -178,6 +182,16 @@ private function updateChecksumsForNode(Node $node, $correctChecksum) { ); } + /** + * + * @param Node $node + * @return bool + */ + private static function fileExistsOnDisk(Node $node) { + $statResult = $node->stat(); + return \is_array($statResult) && isset($statResult['size']) && $statResult['size'] !== false; + } + /** * @param $path * @param IStorage $storage diff --git a/apps/files/tests/Command/VerifyChecksumsTest.php b/apps/files/tests/Command/VerifyChecksumsTest.php index e6f178433360..ecb919834a23 100644 --- a/apps/files/tests/Command/VerifyChecksumsTest.php +++ b/apps/files/tests/Command/VerifyChecksumsTest.php @@ -90,7 +90,7 @@ private function createFileForUser($uid, $path, $content) { if (!empty($subDir)) { $currentDir = "$currentDir/$subDir"; if (!$userFolder->nodeExists($currentDir)) { - $userFolder->newFolder("$currentDir"); + $userFolder->newFolder($currentDir); } } } @@ -189,8 +189,8 @@ public function testFilesWithBrokenChecksumsAreDisplayed() { $output = $this->cmd->getDisplay(); - $this->assertContains("Mismatch for {$file1->getInternalPath()}", $output); - $this->assertContains("Mismatch for {$file2->getInternalPath()}", $output); + $this->assertContains($file1->getInternalPath(), $output); + $this->assertContains($file2->getInternalPath(), $output); $this->assertContains(self::BROKEN_CHECKSUM_STRING, $output); $this->assertContains($this->testFiles[4]['expectedChecksums'](), $output); $this->assertContains($this->testFiles[7]['expectedChecksums'](), $output); From 948c5179ac78d61ae99ca3652366b42992fc961e Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Thu, 17 May 2018 14:53:37 +0200 Subject: [PATCH 2/2] Skip SharedStorage and FailedStorage --- apps/files/lib/Command/VerifyChecksums.php | 16 ++++- .../tests/Command/VerifyChecksumsTest.php | 60 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/apps/files/lib/Command/VerifyChecksums.php b/apps/files/lib/Command/VerifyChecksums.php index b248fc912ea9..abb0703888b9 100644 --- a/apps/files/lib/Command/VerifyChecksums.php +++ b/apps/files/lib/Command/VerifyChecksums.php @@ -22,7 +22,9 @@ namespace OCA\Files\Command; use OC\Files\FileInfo; +use OC\Files\Storage\FailedStorage; use OC\Files\Storage\Wrapper\Checksum; +use OCA\Files_Sharing\ISharedStorage; use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\Files\NotFoundException; @@ -94,9 +96,19 @@ public function execute(InputInterface $input, OutputInterface $output) { $path = $node->getInternalPath(); $currentChecksums = $node->getChecksum(); $owner = $node->getOwner()->getUID(); + $storage = $node->getStorage(); + + if ($storage->instanceOfStorage(ISharedStorage::class) || $storage->instanceOfStorage(FailedStorage::class)) { + return; + } if (!self::fileExistsOnDisk($node)) { - $output->writeln("Skipping $owner/$path => File is in file-cache but doesn`t exist on storage/disk", OutputInterface::VERBOSITY_VERBOSE); + $output->writeln("Skipping $owner/$path => File is in file-cache but doesn't exist on storage/disk", OutputInterface::VERBOSITY_VERBOSE); + return; + } + + if (!$node->isReadable()) { + $output->writeln("Skipping $owner/$path => File not readable", OutputInterface::VERBOSITY_VERBOSE); return; } @@ -188,7 +200,7 @@ private function updateChecksumsForNode(Node $node, $correctChecksum) { * @return bool */ private static function fileExistsOnDisk(Node $node) { - $statResult = $node->stat(); + $statResult = @$node->stat(); return \is_array($statResult) && isset($statResult['size']) && $statResult['size'] !== false; } diff --git a/apps/files/tests/Command/VerifyChecksumsTest.php b/apps/files/tests/Command/VerifyChecksumsTest.php index ecb919834a23..d8b287702cf3 100644 --- a/apps/files/tests/Command/VerifyChecksumsTest.php +++ b/apps/files/tests/Command/VerifyChecksumsTest.php @@ -6,6 +6,7 @@ use OC\Files\Storage\Wrapper\Checksum; use OCA\Files\Command\VerifyChecksums; use OCP\IUser; +use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Tester\CommandTester; use Test\TestCase; @@ -270,6 +271,7 @@ public function testOnlyFilesInGivenPathArgumentAreRepaired() { } public function testOnlyFilesOfAGivenUserAreRepaired() { + /** @var File $file1 */ $file1 = $this->testFiles[0]['file']; /** @var File $file2 */ @@ -299,4 +301,62 @@ public function testAllFilesCanBeRepaired() { $this->assertChecksumsAreCorrect($this->testFiles); } + + public function testFileWithoutChecksumIsIgnored() { + /** @var File $file */ + $file = $this->testFiles[0]['file']; + $file->getStorage()->getCache()->update( + $file->getId(), + ['checksum' => ''] + ); + + $this->cmd->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); + $output = $this->cmd->getDisplay(); + + $this->assertContains('somefile.txt => No Checksum', $output); + $this->assertContains('Skipping', $output); + } + + public function testFileInCacheButNotOnDiskIsIgnored() { + /** @var File $file */ + $file = $this->testFiles[0]['file']; + $file->getStorage()->getCache()->update( + $file->getId(), + ['path' => '/does/not/exist', 'name' => 'x-file.txt'] + ); + + $this->cmd->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); + $output = $this->cmd->getDisplay(); + + $this->assertContains('x-file.txt => File is in file-cache but doesn\'t exist on storage/disk', $output); + } + + public function testInvalidArgs() { + $this->cmd->execute(['-p' => '/does/not/exist']); + + $this->assertEquals( + VerifyChecksums::EXIT_INVALID_ARGS, + $this->cmd->getStatusCode(), + 'Not existing path must return invalid args status code' + ); + + $this->cmd->execute(['-u' => 'doesnotexist@example.com']); + + $this->assertEquals( + VerifyChecksums::EXIT_INVALID_ARGS, + $this->cmd->getStatusCode(), + 'Not existing user must return invalid args status code' + ); + + $this->cmd->execute([ + '-u' => 'foo@example.com', + '-p' => '/some/path' + ]); + + $this->assertEquals( + VerifyChecksums::EXIT_INVALID_ARGS, + $this->cmd->getStatusCode(), + 'User and path must return invalid args status code when combined' + ); + } }