Merge pull request #28196 from nextcloud/revert-27205-backport/26936/stable21

Revert "[stable21] better cleanup of filecache when deleting an external storage"

5 files changed, 48 insertions, 77 deletions

diff --git a/apps/files_external/lib/Service/StoragesService.php b/apps/files_external/lib/Service/StoragesService.php
index 334b1c5041a..63f0c5d52c5 100644
--- a/apps/files_external/lib/Service/StoragesService.php
+++ b/apps/files_external/lib/Service/StoragesService.php
@@ -479,7 +479,44 @@ abstract class StoragesService {
 		$this->triggerHooks($deletedStorage, Filesystem::signal_delete_mount);
 
 		// delete oc_storages entries and oc_filecache
-		\OC\Files\Cache\Storage::cleanByMountId($id);
+		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();
 	}
 
 	/**
diff --git a/apps/files_external/tests/Service/StoragesServiceTest.php b/apps/files_external/tests/Service/StoragesServiceTest.php
index 1e4707e05b5..9817c779a31 100644
--- a/apps/files_external/tests/Service/StoragesServiceTest.php
+++ b/apps/files_external/tests/Service/StoragesServiceTest.php
@@ -40,11 +40,7 @@ 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 = [];
@@ -283,8 +279,10 @@ abstract class StoragesServiceTest extends \Test\TestCase {
 					'password' => 'testPassword',
 					'root' => 'someroot',
 				],
-				'webdav::test@example.com//someroot/'
+				'webdav::test@example.com//someroot/',
+				0
 			],
+			// special case with $user vars, cannot auto-remove the oc_storages entry
 			[
 				[
 					'host' => 'example.com',
@@ -292,7 +290,8 @@ abstract class StoragesServiceTest extends \Test\TestCase {
 					'password' => 'testPassword',
 					'root' => 'someroot',
 				],
-				'webdav::someone@example.com//someroot/'
+				'webdav::someone@example.com//someroot/',
+				1
 			],
 		];
 	}
@@ -300,7 +299,7 @@ abstract class StoragesServiceTest extends \Test\TestCase {
 	/**
 	 * @dataProvider deleteStorageDataProvider
 	 */
-	public function testDeleteStorage($backendOptions, $rustyStorageId) {
+	public function testDeleteStorage($backendOptions, $rustyStorageId, $expectedCountAfterDeletion) {
 		$backend = $this->backendService->getBackend('identifier:\OCA\Files_External\Lib\Backend\DAV');
 		$authMechanism = $this->backendService->getAuthMechanism('identifier:\Auth\Mechanism');
 		$storage = new StorageConfig(255);
@@ -316,31 +315,6 @@ 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();
 
@@ -364,7 +338,7 @@ abstract class StoragesServiceTest extends \Test\TestCase {
 		$result = $storageCheckQuery->execute();
 		$storages = $result->fetchAll();
 		$result->closeCursor();
-		$this->assertCount(0, $storages, "expected 0 storages, got " . json_encode($storages));
+		$this->assertCount($expectedCountAfterDeletion, $storages, "expected $expectedCountAfterDeletion 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 2ad75c88940..c2f3f42ade8 100644
--- 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) {
+	public function testDeleteStorage($backendOptions, $rustyStorageId, $expectedCountAfterDeletion) {
 		$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 e54fc6d515f..c7bc660c538 100644
--- 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) {
-		parent::testDeleteStorage($backendOptions, $rustyStorageId);
+	public function testDeleteStorage($backendOptions, $rustyStorageId, $expectedCountAfterDeletion) {
+		parent::testDeleteStorage($backendOptions, $rustyStorageId, $expectedCountAfterDeletion);
 
 		// 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 c2ebddbd5e5..9f28926aec5 100644
--- a/lib/private/Files/Cache/Storage.php
+++ b/lib/private/Files/Cache/Storage.php
@@ -29,7 +29,6 @@
 
 namespace OC\Files\Cache;
 
-use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\Files\Storage\IStorage;
 use Psr\Log\LoggerInterface;
 
@@ -220,43 +219,4 @@ 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;
-		}
-	}
 }