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 <robin@icewind.nl>

diff --git a/apps/files_external/lib/Service/StoragesService.php b/apps/files_external/lib/Service/StoragesService.php
index 63f0c5d52c5abed951230e1e37b7deaf2d6bddb9..334b1c5041a557113b57c2faffd2ca9904cd391a 100644 (file)
--- a/apps/files_external/lib/Service/StoragesService.php
+++ b/apps/files_external/lib/Service/StoragesService.php
@@ -479,44 +479,7 @@ abstract class StoragesService {
                $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 9817c779a311c59365d18f8b504c3fb9634efa9e..1e4707e05b5a9106f047e3073258c84196441628 100644 (file)
--- 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\BackendService;
 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 @@ abstract class StoragesServiceTest extends \Test\TestCase {
                                        '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 @@ abstract class StoragesServiceTest extends \Test\TestCase {
                                        'password' => 'testPassword',
                                        'root' => 'someroot',
                                ],
-                               'webdav::someone@example.com//someroot/',
-                               1
+                               'webdav::someone@example.com//someroot/'
                        ],
                ];
        }
@@ -299,7 +300,7 @@ abstract class StoragesServiceTest extends \Test\TestCase {
        /**
         * @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 @@ abstract class StoragesServiceTest extends \Test\TestCase {
                // 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 @@ abstract class StoragesServiceTest extends \Test\TestCase {
                $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 c2f3f42ade856d6d689012b6864c914cfe6e21ba..2ad75c88940eb3c4db64b8700aa2e410b7ac0b0f 100644 (file)
--- a/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php
+++ b/apps/files_external/tests/Service/UserGlobalStoragesServiceTest.php
@@ -204,7 +204,7 @@ class UserGlobalStoragesServiceTest extends GlobalStoragesServiceTest {
        /**
         * @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 c7bc660c5389f395f392c3f53f24a55b6cb3029f..e54fc6d515ff4d28ac0d22aa8326c9849137255e 100644 (file)
--- a/apps/files_external/tests/Service/UserStoragesServiceTest.php
+++ b/apps/files_external/tests/Service/UserStoragesServiceTest.php
@@ -150,8 +150,8 @@ class UserStoragesServiceTest extends StoragesServiceTest {
        /**
         * @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 9f28926aec558a80d2977646d375bb7d7bd0b816..c2ebddbd5e58a0f972adb7fd0c9c8dcf31b7f63c 100644 (file)
--- a/lib/private/Files/Cache/Storage.php
+++ b/lib/private/Files/Cache/Storage.php
@@ -29,6 +29,7 @@
 
 namespace OC\Files\Cache;
 
+use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\Files\Storage\IStorage;
 use Psr\Log\LoggerInterface;
 
@@ -219,4 +220,43 @@ class Storage {
                        $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;
+               }
+       }
 }