From 74261d2bace661f10a97fa60824972ac8a5bc76c Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 10 May 2021 17:34:10 +0200 Subject: [PATCH] better cleanup of filecache when deleting an external storage this way it can delete the cache entries even with per-user credentials Signed-off-by: Robin Appelman --- .../lib/Service/StoragesService.php | 39 +----------------- .../tests/Service/StoragesServiceTest.php | 40 +++++++++++++++---- .../Service/UserGlobalStoragesServiceTest.php | 2 +- .../tests/Service/UserStoragesServiceTest.php | 4 +- lib/private/Files/Cache/Storage.php | 40 +++++++++++++++++++ 5 files changed, 77 insertions(+), 48 deletions(-) diff --git a/apps/files_external/lib/Service/StoragesService.php b/apps/files_external/lib/Service/StoragesService.php index ce0b9d61fa40b..fd38e9988adea 100644 --- a/apps/files_external/lib/Service/StoragesService.php +++ b/apps/files_external/lib/Service/StoragesService.php @@ -479,44 +479,7 @@ public function removeStorage($id) { $this->triggerHooks($deletedStorage, Filesystem::signal_delete_mount); // delete oc_storages entries and oc_filecache - try { - $rustyStorageId = $this->getRustyStorageIdFromConfig($deletedStorage); - \OC\Files\Cache\Storage::remove($rustyStorageId); - } catch (\Exception $e) { - // can happen either for invalid configs where the storage could not - // be instantiated or whenever $user vars where used, in which case - // the storage id could not be computed - \OC::$server->getLogger()->logException($e, [ - 'level' => ILogger::ERROR, - 'app' => 'files_external', - ]); - } - } - - /** - * Returns the rusty storage id from oc_storages from the given storage config. - * - * @param StorageConfig $storageConfig - * @return string rusty storage id - */ - private function getRustyStorageIdFromConfig(StorageConfig $storageConfig) { - // if any of the storage options contains $user, it is not possible - // to compute the possible storage id as we don't know which users - // mounted it already (and we certainly don't want to iterate over ALL users) - foreach ($storageConfig->getBackendOptions() as $value) { - if (strpos($value, '$user') !== false) { - throw new \Exception('Cannot compute storage id for deletion due to $user vars in the configuration'); - } - } - - // note: similar to ConfigAdapter->prepateStorageConfig() - $storageConfig->getAuthMechanism()->manipulateStorageConfig($storageConfig); - $storageConfig->getBackend()->manipulateStorageConfig($storageConfig); - - $class = $storageConfig->getBackend()->getStorageClass(); - $storageImpl = new $class($storageConfig->getBackendOptions()); - - return $storageImpl->getId(); + \OC\Files\Cache\Storage::cleanByMountId($id); } /** diff --git a/apps/files_external/tests/Service/StoragesServiceTest.php b/apps/files_external/tests/Service/StoragesServiceTest.php index b1118a571402a..b3722d1dc2f0c 100644 --- a/apps/files_external/tests/Service/StoragesServiceTest.php +++ b/apps/files_external/tests/Service/StoragesServiceTest.php @@ -40,7 +40,11 @@ use OCA\Files_External\Service\DBConfigService; use OCA\Files_External\Service\StoragesService; use OCP\AppFramework\IAppContainer; +use OCP\Files\Cache\ICache; use OCP\Files\Config\IUserMountCache; +use OCP\Files\Mount\IMountPoint; +use OCP\Files\Storage\IStorage; +use OCP\IUser; class CleaningDBConfig extends DBConfigService { private $mountIds = []; @@ -279,10 +283,8 @@ public function deleteStorageDataProvider() { 'password' => 'testPassword', 'root' => 'someroot', ], - 'webdav::test@example.com//someroot/', - 0 + 'webdav::test@example.com//someroot/' ], - // special case with $user vars, cannot auto-remove the oc_storages entry [ [ 'host' => 'example.com', @@ -290,8 +292,7 @@ public function deleteStorageDataProvider() { 'password' => 'testPassword', 'root' => 'someroot', ], - 'webdav::someone@example.com//someroot/', - 1 + 'webdav::someone@example.com//someroot/' ], ]; } @@ -299,7 +300,7 @@ public function deleteStorageDataProvider() { /** * @dataProvider deleteStorageDataProvider */ - public function testDeleteStorage($backendOptions, $rustyStorageId, $expectedCountAfterDeletion) { + public function testDeleteStorage($backendOptions, $rustyStorageId) { $backend = $this->backendService->getBackend('identifier:\OCA\Files_External\Lib\Backend\DAV'); $authMechanism = $this->backendService->getAuthMechanism('identifier:\Auth\Mechanism'); $storage = new StorageConfig(255); @@ -315,6 +316,31 @@ public function testDeleteStorage($backendOptions, $rustyStorageId, $expectedCou // access, which isn't possible within this test $storageCache = new \OC\Files\Cache\Storage($rustyStorageId); + /** @var IUserMountCache $mountCache */ + $mountCache = \OC::$server->get(IUserMountCache::class); + $mountCache->clear(); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('test'); + $cache = $this->createMock(ICache::class); + $storage = $this->createMock(IStorage::class); + $storage->method('getCache')->willReturn($cache); + $mount = $this->createMock(IMountPoint::class); + $mount->method('getStorage') + ->willReturn($storage); + $mount->method('getStorageId') + ->willReturn($rustyStorageId); + $mount->method('getNumericStorageId') + ->willReturn($storageCache->getNumericId()); + $mount->method('getStorageRootId') + ->willReturn(1); + $mount->method('getMountPoint') + ->willReturn('dummy'); + $mount->method('getMountId') + ->willReturn($id); + $mountCache->registerMounts($user, [ + $mount + ]); + // get numeric id for later check $numericId = $storageCache->getNumericId(); @@ -338,7 +364,7 @@ public function testDeleteStorage($backendOptions, $rustyStorageId, $expectedCou $result = $storageCheckQuery->execute(); $storages = $result->fetchAll(); $result->closeCursor(); - $this->assertCount($expectedCountAfterDeletion, $storages, "expected $expectedCountAfterDeletion storages, got " . json_encode($storages)); + $this->assertCount(0, $storages, "expected 0 storages, got " . json_encode($storages)); } protected function actualDeletedUnexistingStorageTest() { diff --git a/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php b/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php index a1e92266e6b6c..396f9c3efbe20 100644 --- a/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php +++ b/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php @@ -204,7 +204,7 @@ public function testNonExistingStorage() { /** * @dataProvider deleteStorageDataProvider */ - public function testDeleteStorage($backendOptions, $rustyStorageId, $expectedCountAfterDeletion) { + public function testDeleteStorage($backendOptions, $rustyStorageId) { $this->expectException(\DomainException::class); $backend = $this->backendService->getBackend('identifier:\OCA\Files_External\Lib\Backend\SMB'); diff --git a/apps/files_external/tests/Service/UserStoragesServiceTest.php b/apps/files_external/tests/Service/UserStoragesServiceTest.php index b8d4dc2f5da46..6388b5c258087 100644 --- a/apps/files_external/tests/Service/UserStoragesServiceTest.php +++ b/apps/files_external/tests/Service/UserStoragesServiceTest.php @@ -150,8 +150,8 @@ public function testUpdateStorage() { /** * @dataProvider deleteStorageDataProvider */ - public function testDeleteStorage($backendOptions, $rustyStorageId, $expectedCountAfterDeletion) { - parent::testDeleteStorage($backendOptions, $rustyStorageId, $expectedCountAfterDeletion); + public function testDeleteStorage($backendOptions, $rustyStorageId) { + parent::testDeleteStorage($backendOptions, $rustyStorageId); // hook called once for user (first one was during test creation) $this->assertHookCall( diff --git a/lib/private/Files/Cache/Storage.php b/lib/private/Files/Cache/Storage.php index ece1df695d7e2..80ad9d1998068 100644 --- a/lib/private/Files/Cache/Storage.php +++ b/lib/private/Files/Cache/Storage.php @@ -30,6 +30,7 @@ namespace OC\Files\Cache; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Storage\IStorage; use Psr\Log\LoggerInterface; @@ -220,4 +221,43 @@ public static function remove($storageId) { $query->execute(); } } + + /** + * remove the entry for the storage by the mount id + * + * @param int $mountId + */ + public static function cleanByMountId(int $mountId) { + $db = \OC::$server->getDatabaseConnection(); + + try { + $db->beginTransaction(); + + $query = $db->getQueryBuilder(); + $query->select('storage_id') + ->from('mounts') + ->where($query->expr()->eq('mount_id', $query->createNamedParameter($mountId, IQueryBuilder::PARAM_INT))); + $storageIds = $query->executeQuery()->fetchAll(\PDO::FETCH_COLUMN); + + $query = $db->getQueryBuilder(); + $query->delete('filecache') + ->where($query->expr()->in('storage', $query->createNamedParameter($storageIds, IQueryBuilder::PARAM_INT_ARRAY))); + $query->executeStatement(); + + $query = $db->getQueryBuilder(); + $query->delete('storages') + ->where($query->expr()->eq('numeric_id', $query->createNamedParameter($storageIds, IQueryBuilder::PARAM_INT_ARRAY))); + $query->executeStatement(); + + $query = $db->getQueryBuilder(); + $query->delete('mounts') + ->where($query->expr()->eq('mount_id', $query->createNamedParameter($mountId, IQueryBuilder::PARAM_INT))); + $query->executeStatement(); + + $db->commit(); + } catch (\Exception $e) { + $db->rollBack(); + throw $e; + } + } }