Merge pull request #19610 from owncloud/ext-deletestoragefromdatabase

Remove oc_storages entries after removing ext storages

3 files changed, 94 insertions, 4 deletions

diff --git a/apps/files_external/service/storagesservice.php b/apps/files_external/service/storagesservice.php
index d5260a066cd..41bb0ca9b80 100644
--- a/apps/files_external/service/storagesservice.php
+++ b/apps/files_external/service/storagesservice.php
@@ -550,6 +550,21 @@ abstract class StoragesService {
 		$this->writeConfig($allStorages);
 
 		$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
+			\OCP\Util::writeLog(
+				'files_external',
+				'Exception: "' . $e->getMessage() . '"',
+				\OCP\Util::ERROR
+			);
+		}
 	}
 
 	/**
@@ -570,4 +585,30 @@ abstract class StoragesService {
 		return (max(array_keys($allStorages)) + 1);
 	}
 
+	/**
+	 * 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 1bec543ff4c..7487ba459af 100644
--- a/apps/files_external/tests/service/storagesservicetest.php
+++ b/apps/files_external/tests/service/storagesservicetest.php
@@ -216,18 +216,57 @@ abstract class StoragesServiceTest extends \Test\TestCase {
 		$this->service->updateStorage($storage);
 	}
 
-	public function testDeleteStorage() {
+	public function deleteStorageDataProvider() {
+		return [
+			// regular case, can properly delete the oc_storages entry
+			[
+				[
+					'share' => 'share',
+					'host' => 'example.com',
+					'user' => 'test',
+					'password' => 'testPassword',
+					'root' => 'someroot',
+				],
+				'smb::test@example.com//share//someroot/',
+				0
+			],
+			// special case with $user vars, cannot auto-remove the oc_storages entry
+			[
+				[
+					'share' => 'share',
+					'host' => 'example.com',
+					'user' => '$user',
+					'password' => 'testPassword',
+					'root' => 'someroot',
+				],
+				'smb::someone@example.com//share//someroot/',
+				1
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider deleteStorageDataProvider
+	 */
+	public function testDeleteStorage($backendOptions, $rustyStorageId, $expectedCountAfterDeletion) {
 		$backend = $this->backendService->getBackend('identifier:\OCA\Files_External\Lib\Backend\SMB');
 		$authMechanism = $this->backendService->getAuthMechanism('identifier:\Auth\Mechanism');
 		$storage = new StorageConfig(255);
 		$storage->setMountPoint('mountpoint');
 		$storage->setBackend($backend);
 		$storage->setAuthMechanism($authMechanism);
-		$storage->setBackendOptions(['password' => 'testPassword']);
+		$storage->setBackendOptions($backendOptions);
 
 		$newStorage = $this->service->addStorage($storage);
 		$this->assertEquals(1, $newStorage->getId());
 
+		// manually trigger storage entry because normally it happens on first
+		// access, which isn't possible within this test
+		$storageCache = new \OC\Files\Cache\Storage($rustyStorageId);
+
+		// get numeric id for later check
+		$numericId = $storageCache->getNumericId();
+
 		$newStorage = $this->service->removeStorage(1);
 
 		$caught = false;
@@ -238,6 +277,13 @@ abstract class StoragesServiceTest extends \Test\TestCase {
 		}
 
 		$this->assertTrue($caught);
+
+		// storage id was removed from oc_storages
+		$qb = \OC::$server->getDatabaseConnection()->getQueryBuilder();
+		$storageCheckQuery = $qb->select('*')
+			->from('storages')
+			->where($qb->expr()->eq('numeric_id', $qb->expr()->literal($numericId)));
+		$this->assertCount($expectedCountAfterDeletion, $storageCheckQuery->execute()->fetchAll());
 	}
 
 	/**
diff --git a/apps/files_external/tests/service/userstoragesservicetest.php b/apps/files_external/tests/service/userstoragesservicetest.php
index f829f73f050..78f9231c3d1 100644
--- a/apps/files_external/tests/service/userstoragesservicetest.php
+++ b/apps/files_external/tests/service/userstoragesservicetest.php
@@ -138,8 +138,11 @@ class UserStoragesServiceTest extends StoragesServiceTest {
 		$this->assertEmpty(self::$hookCalls);
 	}
 
-	public function testDeleteStorage() {
-		parent::testDeleteStorage();
+	/**
+	 * @dataProvider deleteStorageDataProvider
+	 */
+	public function testDeleteStorage($backendOptions, $rustyStorageId, $expectedCountAfterDeletion) {
+		parent::testDeleteStorage($backendOptions, $rustyStorageId, $expectedCountAfterDeletion);
 
 		// hook called once for user (first one was during test creation)
 		$this->assertHookCall(