Skip to content

Commit

Permalink
better cleanup of filecache when deleting an external storage
Browse files Browse the repository at this point in the history
this way it can delete the cache entries even with per-user credentials

Signed-off-by: Robin Appelman <robin@icewind.nl>
  • Loading branch information
icewind1991 authored and skjnldsv committed Jul 28, 2021
1 parent 90f50ae commit 7344770
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 48 deletions.
39 changes: 1 addition & 38 deletions apps/files_external/lib/Service/StoragesService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
40 changes: 33 additions & 7 deletions apps/files_external/tests/Service/StoragesServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand Down Expand Up @@ -279,27 +283,24 @@ 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',
'user' => '$user',
'password' => 'testPassword',
'root' => 'someroot',
],
'webdav::someone@example.com//someroot/',
1
'webdav::someone@example.com//someroot/'
],
];
}

/**
* @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);
Expand All @@ -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();

Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
4 changes: 2 additions & 2 deletions apps/files_external/tests/Service/UserStoragesServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
40 changes: 40 additions & 0 deletions lib/private/Files/Cache/Storage.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

namespace OC\Files\Cache;

use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Files\Storage\IStorage;
use Psr\Log\LoggerInterface;

Expand Down Expand Up @@ -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;
}
}
}

0 comments on commit 7344770

Please sign in to comment.