From 5155a5288c90eb9ccfb432d051d95e3777653eb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 12 Oct 2016 12:59:13 +0200 Subject: [PATCH] Add CleanupRemoteStorages command cleanup files, address review Fix CleanupRemoteStoragesTest tests Fix test expectation. Added files count to check filecache deletion. Sort by numeric id for deterministic test results Removed precise order test and added storage check Remove inaccurate removal message check which has a different order on Oracle. Added more checks to confirm that existing storages still exist. Signed-off-by: Morris Jobke --- apps/files_sharing/appinfo/info.xml | 4 + .../lib/Command/CleanupRemoteStorages.php | 177 ++++++++++++++++ .../Command/CleanupRemoteStoragesTest.php | 193 ++++++++++++++++++ 3 files changed, 374 insertions(+) create mode 100644 apps/files_sharing/lib/Command/CleanupRemoteStorages.php create mode 100644 apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php diff --git a/apps/files_sharing/appinfo/info.xml b/apps/files_sharing/appinfo/info.xml index e93bc99d54..e2a08d951a 100644 --- a/apps/files_sharing/appinfo/info.xml +++ b/apps/files_sharing/appinfo/info.xml @@ -47,4 +47,8 @@ Turning the feature off removes shared files and folders on the server for all s OCA\Files_Sharing\DeleteOrphanedSharesJob OCA\Files_Sharing\ExpireSharesJob + + + OCA\Files_Sharing\Command\CleanupRemoteStorages + diff --git a/apps/files_sharing/lib/Command/CleanupRemoteStorages.php b/apps/files_sharing/lib/Command/CleanupRemoteStorages.php new file mode 100644 index 0000000000..ec98b46267 --- /dev/null +++ b/apps/files_sharing/lib/Command/CleanupRemoteStorages.php @@ -0,0 +1,177 @@ + + * + * @copyright Copyright (c) 2016, 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 OCA\Files_Sharing\Command; + +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; + +/** + * Cleanup 'shared::' storage entries that have no matching entries in the + * shares_external table. + */ +class CleanupRemoteStorages extends Command { + + /** + * @var IDBConnection + */ + protected $connection; + + public function __construct(IDBConnection $connection) { + $this->connection = $connection; + parent::__construct(); + } + + protected function configure() { + $this + ->setName('sharing:cleanup-remote-storages') + ->setDescription('Cleanup \'shared::\' storage entries that have no matching entries in the shares_external table') + ->addOption( + 'dry-run', + null, + InputOption::VALUE_NONE, + 'only show which storages would be deleted' + ); + } + + public function execute(InputInterface $input, OutputInterface $output) { + + $remoteStorages = $this->getRemoteStorages(); + + $output->writeln(count($remoteStorages) . " remote storage(s) need(s) to be checked"); + + $remoteShareIds = $this->getRemoteShareIds(); + + $output->writeln(count($remoteShareIds) . " remote share(s) exist"); + + foreach ($remoteShareIds as $id => $remoteShareId) { + if (isset($remoteStorages[$remoteShareId])) { + $output->writeln("$remoteShareId belongs to remote share $id"); + unset($remoteStorages[$remoteShareId]); + } else { + $output->writeln("$remoteShareId for share $id has no matching storage, yet"); + } + } + + if (empty($remoteStorages)) { + $output->writeln("no storages deleted"); + } else { + $dryRun = $input->getOption('dry-run'); + foreach ($remoteStorages as $id => $numericId) { + if ($dryRun) { + $output->writeln("$id [$numericId] can be deleted"); + $this->countFiles($numericId, $output); + } else { + $this->deleteStorage($id, $numericId, $output); + } + } + } + } + + public function countFiles ($numericId, OutputInterface $output) { + $queryBuilder = $this->connection->getQueryBuilder(); + $queryBuilder->select($queryBuilder->createFunction('count(fileid)')) + ->from('filecache') + ->where($queryBuilder->expr()->eq( + 'storage', + $queryBuilder->createNamedParameter($numericId, IQueryBuilder::PARAM_STR), + IQueryBuilder::PARAM_STR) + ); + $result = $queryBuilder->execute(); + $count = $result->fetchColumn(); + $output->writeln("$count files can be deleted for storage $numericId"); + } + + public function deleteStorage ($id, $numericId, OutputInterface $output) { + $queryBuilder = $this->connection->getQueryBuilder(); + $queryBuilder->delete('storages') + ->where($queryBuilder->expr()->eq( + 'id', + $queryBuilder->createNamedParameter($id, IQueryBuilder::PARAM_STR), + IQueryBuilder::PARAM_STR) + ); + $output->write("deleting $id [$numericId] ... "); + $count = $queryBuilder->execute(); + $output->writeln("deleted $count"); + $this->deleteFiles($numericId, $output); + } + + public function deleteFiles ($numericId, OutputInterface $output) { + $queryBuilder = $this->connection->getQueryBuilder(); + $queryBuilder->delete('filecache') + ->where($queryBuilder->expr()->eq( + 'storage', + $queryBuilder->createNamedParameter($numericId, IQueryBuilder::PARAM_STR), + IQueryBuilder::PARAM_STR) + ); + $output->write("deleting files for storage $numericId ... "); + $count = $queryBuilder->execute(); + $output->writeln("deleted $count"); + } + + public function getRemoteStorages() { + + $queryBuilder = $this->connection->getQueryBuilder(); + $queryBuilder->select(['id', 'numeric_id']) + ->from('storages') + ->where($queryBuilder->expr()->like( + 'id', + // match all 'shared::' + 32 characters storages + $queryBuilder->createNamedParameter('shared::________________________________', IQueryBuilder::PARAM_STR), + IQueryBuilder::PARAM_STR) + ) + ->andWhere($queryBuilder->expr()->notLike( + 'id', + // but not the ones starting with a '/', they are for normal shares + $queryBuilder->createNamedParameter('shared::/%', IQueryBuilder::PARAM_STR), + IQueryBuilder::PARAM_STR) + )->orderBy('numeric_id'); + $query = $queryBuilder->execute(); + + $remoteStorages = []; + + while ($row = $query->fetch()) { + $remoteStorages[$row['id']] = $row['numeric_id']; + } + + return $remoteStorages; + } + + public function getRemoteShareIds() { + + $queryBuilder = $this->connection->getQueryBuilder(); + $queryBuilder->select(['id', 'share_token', 'remote']) + ->from('share_external'); + $query = $queryBuilder->execute(); + + $remoteShareIds = []; + + while ($row = $query->fetch()) { + $remoteShareIds[$row['id']] = 'shared::' . md5($row['share_token'] . '@' . $row['remote']); + } + + return $remoteShareIds; + } +} diff --git a/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php b/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php new file mode 100644 index 0000000000..5dcf7f8310 --- /dev/null +++ b/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php @@ -0,0 +1,193 @@ + + * + * @copyright Copyright (c) 2016, 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 OCA\Files_Sharing\Tests\Command; + +use OCA\Files_Sharing\Command\CleanupRemoteStorages; +use OCP\DB\QueryBuilder\IQueryBuilder; +use Test\TestCase; + +/** + * Class CleanupRemoteStoragesTest + * + * @group DB + * + * @package OCA\Files_Sharing\Tests\Command + */ +class CleanupRemoteStoragesTest extends TestCase { + + /** + * @var CleanupRemoteStorages + */ + private $command; + + /** + * @var \OCP\IDBConnection + */ + private $connection; + + private $storages = [ + ['id' => 'shared::7b4a322b22f9d0047c38d77d471ce3cf', 'share_token' => 'f2c69dad1dc0649f26976fd210fc62e1', 'remote' => 'https://hostname.tld/owncloud1', 'user' => 'user1'], + ['id' => 'shared::efe3b456112c3780da6155d3a9b9141c', 'share_token' => 'f2c69dad1dc0649f26976fd210fc62e2', 'remote' => 'https://hostname.tld/owncloud2', 'user' => 'user2'], + ['notExistingId' => 'shared::33323d9f4ca416a9e3525b435354bc6f', 'share_token' => 'f2c69dad1dc0649f26976fd210fc62e3', 'remote' => 'https://hostname.tld/owncloud3', 'user' => 'user3'], + ['id' => 'shared::7fe41a07d3f517a923f4b2b599e72cbb', 'files_count' => 2], + ['id' => 'shared::de4aeb2f378d222b6d2c5fd8f4e42f8e', 'share_token' => 'f2c69dad1dc0649f26976fd210fc62e5', 'remote' => 'https://hostname.tld/owncloud5', 'user' => 'user5'], + ['id' => 'shared::af712293ab5eb9e6a1745a13818b99fe', 'files_count' => 3], + ['notExistingId' => 'shared::c34568c143cdac7d2f06e0800b5280f9', 'share_token' => 'f2c69dad1dc0649f26976fd210fc62e7', 'remote' => 'https://hostname.tld/owncloud7', 'user' => 'user7'], + ]; + + protected function setup() { + parent::setUp(); + + $this->connection = \OC::$server->getDatabaseConnection(); + + $storageQuery = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $storageQuery->insert('storages') + ->setValue('id', '?'); + + $shareExternalQuery = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $shareExternalQuery->insert('share_external') + ->setValue('share_token', '?') + ->setValue('remote', '?') + ->setValue('name', '?')->setParameter(2, 'irrelevant') + ->setValue('owner', '?')->setParameter(3, 'irrelevant') + ->setValue('user', '?') + ->setValue('mountpoint', '?')->setParameter(5, 'irrelevant') + ->setValue('mountpoint_hash', '?')->setParameter(6, 'irrelevant'); + + $filesQuery = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $filesQuery->insert('filecache') + ->setValue('storage', '?') + ->setValue('path', '?') + ->setValue('path_hash', '?'); + + foreach ($this->storages as &$storage) { + if (isset($storage['id'])) { + $storageQuery->setParameter(0, $storage['id']); + $storageQuery->execute(); + $storage['numeric_id'] = $this->connection->lastInsertId('*PREFIX*storages'); + } + + if (isset($storage['share_token'])) { + $shareExternalQuery + ->setParameter(0, $storage['share_token']) + ->setParameter(1, $storage['remote']) + ->setParameter(4, $storage['user']); + $shareExternalQuery->execute(); + } + + if (isset($storage['files_count'])) { + for ($i = 0; $i < $storage['files_count']; $i++) { + $filesQuery->setParameter(0, $storage['numeric_id']); + $filesQuery->setParameter(1, 'file' . $i); + $filesQuery->setParameter(2, md5('file' . $i)); + $filesQuery->execute(); + } + } + } + + $this->command = new CleanupRemoteStorages($this->connection); + } + + public function tearDown() { + $storageQuery = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $storageQuery->delete('storages') + ->where($storageQuery->expr()->eq('id', $storageQuery->createParameter('id'))); + + $shareExternalQuery = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $shareExternalQuery->delete('share_external') + ->where($shareExternalQuery->expr()->eq('share_token', $shareExternalQuery->createParameter('share_token'))) + ->andWhere($shareExternalQuery->expr()->eq('remote', $shareExternalQuery->createParameter('remote'))); + + foreach ($this->storages as $storage) { + if (isset($storage['id'])) { + $storageQuery->setParameter('id', $storage['id']); + $storageQuery->execute(); + } + + if (isset($storage['share_token'])) { + $shareExternalQuery->setParameter('share_token', $storage['share_token']); + $shareExternalQuery->setParameter('remote', $storage['remote']); + $shareExternalQuery->execute(); + } + } + + return parent::tearDown(); + } + + private function doesStorageExist($numericId) { + $qb = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $qb->select('*') + ->from('storages') + ->where($qb->expr()->eq('numeric_id', $qb->createNamedParameter($numericId))); + $result = $qb->execute()->fetchAll(); + if (!empty($result)) { + return true; + } + + $qb = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $qb->select('*') + ->from('filecache') + ->where($qb->expr()->eq('storage', $qb->createNamedParameter($numericId))); + $result = $qb->execute()->fetchAll(); + if (!empty($result)) { + return true; + } + + return false; + } + + /** + * Test cleanup of orphaned storages + */ + public function testCleanup() { + $input = $this->getMockBuilder('Symfony\Component\Console\Input\InputInterface') + ->disableOriginalConstructor() + ->getMock(); + $output = $this->getMockBuilder('Symfony\Component\Console\Output\OutputInterface') + ->disableOriginalConstructor() + ->getMock(); + + // + + // parent folder, `files`, ´test` and `welcome.txt` => 4 elements + + $at = 0; + $output + ->expects($this->at($at++)) + ->method('writeln') + ->with('5 remote storage(s) need(s) to be checked'); + $output + ->expects($this->at($at++)) + ->method('writeln') + ->with('5 remote share(s) exist'); + + $this->command->execute($input, $output); + + $this->assertTrue($this->doesStorageExist($this->storages[0]['numeric_id'])); + $this->assertTrue($this->doesStorageExist($this->storages[1]['numeric_id'])); + $this->assertFalse($this->doesStorageExist($this->storages[3]['numeric_id'])); + $this->assertTrue($this->doesStorageExist($this->storages[4]['numeric_id'])); + $this->assertFalse($this->doesStorageExist($this->storages[5]['numeric_id'])); + + } +} +