Generate storage config ids when missing

When reading in old mount.json files, they do not contain config ids.
Since these are needed to be able to use the UI and the new service
classes, these will be generated automatically.

The config grouping is based on a config hash.

8 files changed, 317 insertions, 43 deletions

diff --git a/apps/files_external/controller/globalstoragescontroller.php b/apps/files_external/controller/globalstoragescontroller.php
index e5aff4f95a2..815f24ee2be 100644
--- a/apps/files_external/controller/globalstoragescontroller.php
+++ b/apps/files_external/controller/globalstoragescontroller.php
@@ -15,6 +15,7 @@ namespace OCA\Files_External\Controller;
 use \OCP\IConfig;
 use \OCP\IUserSession;
 use \OCP\IRequest;
+use \OCP\IL10N;
 use \OCP\AppFramework\Http\DataResponse;
 use \OCP\AppFramework\Controller;
 use \OCP\AppFramework\Http;
@@ -24,15 +25,17 @@ use \OCA\Files_external\Lib\StorageConfig;
 
 class GlobalStoragesController extends StoragesController {
 	/**
-	 * @param string $appName
-	 * @param IRequest $request
-	 * @param IConfig $config
-	 * @param GlobalStoragesService $globalStoragesService
+	 * Creates a new global storages controller.
+	 *
+	 * @param string $AppName application name
+	 * @param IRequest $request request object
+	 * @param IL10N $l10n l10n service
+	 * @param GlobalStoragesService $globalStoragesService storage service
 	 */
 	public function __construct(
 		$AppName,
 		IRequest $request,
-		\OCP\IL10N $l10n,
+		IL10N $l10n,
 		GlobalStoragesService $globalStoragesService
 	){
 		parent::__construct(
diff --git a/apps/files_external/controller/userstoragescontroller.php b/apps/files_external/controller/userstoragescontroller.php
index 64202b5e542..ed7ec453ccd 100644
--- a/apps/files_external/controller/userstoragescontroller.php
+++ b/apps/files_external/controller/userstoragescontroller.php
@@ -15,6 +15,7 @@ namespace OCA\Files_External\Controller;
 use \OCP\IConfig;
 use \OCP\IUserSession;
 use \OCP\IRequest;
+use \OCP\IL10N;
 use \OCP\AppFramework\Http\DataResponse;
 use \OCP\AppFramework\Controller;
 use \OCP\AppFramework\Http;
@@ -24,15 +25,17 @@ use \OCA\Files_external\Lib\StorageConfig;
 
 class UserStoragesController extends StoragesController {
 	/**
-	 * @param string $appName
-	 * @param IRequest $request
-	 * @param IConfig $config
-	 * @param UserStoragesService $userStoragesService
+	 * Creates a new user storages controller.
+	 *
+	 * @param string $AppName application name
+	 * @param IRequest $request request object
+	 * @param IL10N $l10n l10n service
+	 * @param UserStoragesService $userStoragesService storage service
 	 */
 	public function __construct(
 		$AppName,
 		IRequest $request,
-		\OCP\IL10N $l10n,
+		IL10N $l10n,
 		UserStoragesService $userStoragesService
 	){
 		parent::__construct(
diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php
index 23750dbb3fa..884e596bdd4 100644
--- a/apps/files_external/lib/config.php
+++ b/apps/files_external/lib/config.php
@@ -894,12 +894,14 @@ class OC_Mount_Config {
 	 * This is mostly used to find out whether configurations
 	 * are the same.
 	 */
-	private static function makeConfigHash($config) {
+	public static function makeConfigHash($config) {
 		$data = json_encode(
 			array(
 				'c' => $config['class'],
 				'm' => $config['mountpoint'],
-				'o' => $config['options']
+				'o' => $config['options'],
+				'p' => isset($config['priority']) ? $config['priority'] : -1,
+				'mo' => isset($config['mountOptions']) ? $config['mountOptions'] : [],
 			)
 		);
 		return hash('md5', $data);
diff --git a/apps/files_external/personal.php b/apps/files_external/personal.php
index a279163ff70..8b435442259 100644
--- a/apps/files_external/personal.php
+++ b/apps/files_external/personal.php
@@ -24,9 +24,29 @@ OCP\Util::addScript('files_external', 'settings');
 OCP\Util::addStyle('files_external', 'settings');
 $backends = OC_Mount_Config::getPersonalBackends();
 
+$mounts = OC_Mount_Config::getPersonalMountPoints();
+$hasId = true;
+foreach ($mounts as $mount) {
+	if (!isset($mount['id'])) {
+		// some mount points are missing ids
+		$hasId = false;
+		break;
+	}
+}
+
+if (!$hasId) {
+	$service = new \OCA\Files_external\Service\UserStoragesService(\OC::$server->getUserSession());
+	// this will trigger the new storage code which will automatically
+	// generate storage config ids
+	$service->getAllStorages();
+	// re-read updated config
+	$mounts = OC_Mount_Config::getPersonalMountPoints();
+	// TODO: use the new storage config format in the template
+}
+
 $tmpl = new OCP\Template('files_external', 'settings');
 $tmpl->assign('isAdminPage', false);
-$tmpl->assign('mounts', OC_Mount_Config::getPersonalMountPoints());
+$tmpl->assign('mounts', $mounts);
 $tmpl->assign('dependencies', OC_Mount_Config::checkDependencies());
 $tmpl->assign('backends', $backends);
 return $tmpl->fetchPage();
diff --git a/apps/files_external/service/storagesservice.php b/apps/files_external/service/storagesservice.php
index 46a485a169c..73a0ae76475 100644
--- a/apps/files_external/service/storagesservice.php
+++ b/apps/files_external/service/storagesservice.php
@@ -30,6 +30,39 @@ abstract class StoragesService {
 	}
 
 	/**
+	 * Copy legacy storage options into the given storage config object.
+	 *
+	 * @param StorageConfig $storageConfig storage config to populate
+	 * @param string $mountType mount type
+	 * @param string $applicable applicable user or group
+	 * @param array $storageOptions legacy storage options
+	 * @return StorageConfig populated storage config
+	 */
+	protected function populateStorageConfigWithLegacyOptions(&$storageConfig, $mountType, $applicable, $storageOptions) {
+		$storageConfig->setBackendClass($storageOptions['class']);
+		$storageConfig->setBackendOptions($storageOptions['options']);
+		if (isset($storageOptions['mountOptions'])) {
+			$storageConfig->setMountOptions($storageOptions['mountOptions']);
+		}
+		if (isset($storageOptions['priority'])) {
+			$storageConfig->setPriority($storageOptions['priority']);
+		}
+
+		if ($mountType === \OC_Mount_Config::MOUNT_TYPE_USER) {
+			$applicableUsers = $storageConfig->getApplicableUsers();
+			if ($applicable !== 'all') {
+				$applicableUsers[] = $applicable;
+				$storageConfig->setApplicableUsers($applicableUsers);
+			}
+		} else if ($mountType === \OC_Mount_Config::MOUNT_TYPE_GROUP) {
+			$applicableGroups = $storageConfig->getApplicableGroups();
+			$applicableGroups[] = $applicable;
+			$storageConfig->setApplicableGroups($applicableGroups);
+		}
+		return $storageConfig;
+	}
+
+	/**
 	 * Read the external storages config
 	 *
 	 * @return array map of storage id to storage config
@@ -55,9 +88,25 @@ abstract class StoragesService {
 
 		// group by storage id
 		$storages = [];
+
+		// for storages without id (legacy), group by config hash for
+		// later processing
+		$storagesWithConfigHash = [];
+
 		foreach ($mountPoints as $mountType => $applicables) {
 			foreach ($applicables as $applicable => $mountPaths) {
 				foreach ($mountPaths as $rootMountPath => $storageOptions) {
+					$currentStorage = null;
+
+					/**
+					 * Flag whether the config that was read already has an id.
+					 * If not, it will use a config hash instead and generate
+					 * a proper id later
+					 *
+					 * @var boolean
+					 */
+					$hasId = false;
+
 					// the root mount point is in the format "/$user/files/the/mount/point"
 					// we remove the "/$user/files" prefix
 					$parts = explode('/', trim($rootMountPath, '/'), 3);
@@ -73,46 +122,60 @@ abstract class StoragesService {
 
 					$relativeMountPath = $parts[2];
 
-					$configId = (int)$storageOptions['id'];
-					if (isset($storages[$configId])) {
-						$currentStorage = $storages[$configId];
+					// note: we cannot do this after the loop because the decrypted config
+					// options might be needed for the config hash
+					$storageOptions['options'] = \OC_Mount_Config::decryptPasswords($storageOptions['options']);
+
+					if (isset($storageOptions['id'])) {
+						$configId = (int)$storageOptions['id'];
+						if (isset($storages[$configId])) {
+							$currentStorage = $storages[$configId];
+						}
+						$hasId = true;
 					} else {
+						// missing id in legacy config, need to generate
+						// but at this point we don't know the max-id, so use
+						// first group it by config hash
+						$storageOptions['mountpoint'] = $rootMountPath;
+						$configId = \OC_Mount_Config::makeConfigHash($storageOptions);
+						if (isset($storagesWithConfigHash[$configId])) {
+							$currentStorage = $storagesWithConfigHash[$configId];
+						}
+					}
+
+					if (is_null($currentStorage)) {
+						// create new
 						$currentStorage = new StorageConfig($configId);
 						$currentStorage->setMountPoint($relativeMountPath);
 					}
 
-					$currentStorage->setBackendClass($storageOptions['class']);
-					$currentStorage->setBackendOptions($storageOptions['options']);
-					if (isset($storageOptions['mountOptions'])) {
-						$currentStorage->setMountOptions($storageOptions['mountOptions']);
-					}
-					if (isset($storageOptions['priority'])) {
-						$currentStorage->setPriority($storageOptions['priority']);
-					}
+					$this->populateStorageConfigWithLegacyOptions(
+						$currentStorage,
+						$mountType,
+						$applicable,
+						$storageOptions
+					);
 
-					if ($mountType === \OC_Mount_Config::MOUNT_TYPE_USER) {
-						$applicableUsers = $currentStorage->getApplicableUsers();
-						if ($applicable !== 'all') {
-							$applicableUsers[] = $applicable;
-							$currentStorage->setApplicableUsers($applicableUsers);
-						}
-					} else if ($mountType === \OC_Mount_Config::MOUNT_TYPE_GROUP) {
-						$applicableGroups = $currentStorage->getApplicableGroups();
-						$applicableGroups[] = $applicable;
-						$currentStorage->setApplicableGroups($applicableGroups);
+					if ($hasId) {
+						$storages[$configId] = $currentStorage;
+					} else {
+						$storagesWithConfigHash[$configId] = $currentStorage;
 					}
-					$storages[$configId] = $currentStorage;
 				}
 			}
 		}
 
-		// decrypt passwords
-		foreach ($storages as &$storage) {
-			$storage->setBackendOptions(
-				\OC_Mount_Config::decryptPasswords(
-					$storage->getBackendOptions()
-				)
-			);
+		// process storages with config hash, they must get a real id
+		if (!empty($storagesWithConfigHash)) {
+			$nextId = $this->generateNextId($storages);
+			foreach ($storagesWithConfigHash as $storage) {
+				$storage->setId($nextId);
+				$storages[$nextId] = $storage;
+				$nextId++;
+			}
+
+			// re-save the config with the generated ids
+			$this->writeConfig($storages);
 		}
 
 		return $storages;
@@ -177,6 +240,15 @@ abstract class StoragesService {
 	}
 
 	/**
+	 * Gets all storages
+	 *
+	 * @return array array of storage configs
+	 */
+	public function getAllStorages() {
+		return $this->readConfig();
+	}
+
+	/**
 	 * Add new storage to the configuration
 	 *
 	 * @param array $newStorage storage attributes
diff --git a/apps/files_external/settings.php b/apps/files_external/settings.php
index dec329e82a2..3144e1a2ab6 100644
--- a/apps/files_external/settings.php
+++ b/apps/files_external/settings.php
@@ -42,9 +42,29 @@ foreach ($backends as $class => $backend)
 	}
 }
 
+$mounts = OC_Mount_Config::getSystemMountPoints();
+$hasId = true;
+foreach ($mounts as $mount) {
+	if (!isset($mount['id'])) {
+		// some mount points are missing ids
+		$hasId = false;
+		break;
+	}
+}
+
+if (!$hasId) {
+	$service = new \OCA\Files_external\Service\GlobalStoragesService();
+	// this will trigger the new storage code which will automatically
+	// generate storage config ids
+	$service->getAllStorages();
+	// re-read updated config
+	$mounts = OC_Mount_Config::getSystemMountPoints();
+	// TODO: use the new storage config format in the template
+}
+
 $tmpl = new OCP\Template('files_external', 'settings');
 $tmpl->assign('isAdminPage', true);
-$tmpl->assign('mounts', OC_Mount_Config::getSystemMountPoints());
+$tmpl->assign('mounts', $mounts);
 $tmpl->assign('backends', $backends);
 $tmpl->assign('personal_backends', $personal_backends);
 $tmpl->assign('dependencies', OC_Mount_Config::checkDependencies());
diff --git a/apps/files_external/tests/service/globalstoragesservicetest.php b/apps/files_external/tests/service/globalstoragesservicetest.php
index f5cdcfa3907..4c038bc489e 100644
--- a/apps/files_external/tests/service/globalstoragesservicetest.php
+++ b/apps/files_external/tests/service/globalstoragesservicetest.php
@@ -708,4 +708,108 @@ class GlobalStoragesServiceTest extends StoragesServiceTest {
 		}
 	}
 
+	/**
+	 * Test reading in a legacy config and generating config ids.
+	 */
+	public function testReadLegacyConfigAndGenerateConfigId() {
+		$configFile = $this->dataDir . '/mount.json';
+
+		$legacyBackendOptions = [
+			'user' => 'someuser',
+			'password' => 'somepassword',
+		];
+		$legacyBackendOptions = \OC_Mount_Config::encryptPasswords($legacyBackendOptions);
+
+		$legacyConfig = [
+			'class' => '\OC\Files\Storage\SMB',
+			'options' => $legacyBackendOptions,
+			'mountOptions' => ['preview' => false],
+		];
+		// different mount options
+		$legacyConfig2 = [
+			'class' => '\OC\Files\Storage\SMB',
+			'options' => $legacyBackendOptions,
+			'mountOptions' => ['preview' => true],
+		];
+
+		$legacyBackendOptions2 = $legacyBackendOptions;
+		$legacyBackendOptions2 = ['user' => 'someuser2', 'password' => 'somepassword2'];
+		$legacyBackendOptions2 = \OC_Mount_Config::encryptPasswords($legacyBackendOptions2);
+
+		// different config
+		$legacyConfig3 = [
+			'class' => '\OC\Files\Storage\SMB',
+			'options' => $legacyBackendOptions2,
+			'mountOptions' => ['preview' => true],
+		];
+
+		$json = [
+			'user' => [
+				'user1' => [
+					'/$user/files/somemount' => $legacyConfig,
+				],
+				// same config
+				'user2' => [
+					'/$user/files/somemount' => $legacyConfig,
+				],
+				// different mountOptions
+				'user3' => [
+					'/$user/files/somemount' => $legacyConfig2,
+				],
+				// different mount point
+				'user4' => [
+					'/$user/files/anothermount' => $legacyConfig,
+				],
+				// different storage config
+				'user5' => [
+					'/$user/files/somemount' => $legacyConfig3,
+				],
+			],
+			'group' => [
+				'group1' => [
+					// will get grouped with user configs
+					'/$user/files/somemount' => $legacyConfig,
+				],
+			],
+		];
+
+		file_put_contents($configFile, json_encode($json));
+
+		$allStorages = $this->service->getAllStorages();
+
+		$this->assertCount(4, $allStorages);
+
+		$storage1 = $allStorages[1];
+		$storage2 = $allStorages[2];
+		$storage3 = $allStorages[3];
+		$storage4 = $allStorages[4];
+
+		$this->assertEquals('/somemount', $storage1->getMountPoint());
+		$this->assertEquals('someuser', $storage1->getBackendOptions()['user']);
+		$this->assertEquals('somepassword', $storage1->getBackendOptions()['password']);
+		$this->assertEquals(['user1', 'user2'], $storage1->getApplicableUsers());
+		$this->assertEquals(['group1'], $storage1->getApplicableGroups());
+		$this->assertEquals(['preview' => false], $storage1->getMountOptions());
+
+		$this->assertEquals('/somemount', $storage2->getMountPoint());
+		$this->assertEquals('someuser', $storage2->getBackendOptions()['user']);
+		$this->assertEquals('somepassword', $storage2->getBackendOptions()['password']);
+		$this->assertEquals(['user3'], $storage2->getApplicableUsers());
+		$this->assertEquals([], $storage2->getApplicableGroups());
+		$this->assertEquals(['preview' => true], $storage2->getMountOptions());
+
+		$this->assertEquals('/anothermount', $storage3->getMountPoint());
+		$this->assertEquals('someuser', $storage3->getBackendOptions()['user']);
+		$this->assertEquals('somepassword', $storage3->getBackendOptions()['password']);
+		$this->assertEquals(['user4'], $storage3->getApplicableUsers());
+		$this->assertEquals([], $storage3->getApplicableGroups());
+		$this->assertEquals(['preview' => false], $storage3->getMountOptions());
+
+		$this->assertEquals('/somemount', $storage4->getMountPoint());
+		$this->assertEquals('someuser2', $storage4->getBackendOptions()['user']);
+		$this->assertEquals('somepassword2', $storage4->getBackendOptions()['password']);
+		$this->assertEquals(['user5'], $storage4->getApplicableUsers());
+		$this->assertEquals([], $storage4->getApplicableGroups());
+		$this->assertEquals(['preview' => true], $storage4->getMountOptions());
+	}
 }
diff --git a/apps/files_external/tests/service/userstoragesservicetest.php b/apps/files_external/tests/service/userstoragesservicetest.php
index 77b3842b318..dd3f9e1b92c 100644
--- a/apps/files_external/tests/service/userstoragesservicetest.php
+++ b/apps/files_external/tests/service/userstoragesservicetest.php
@@ -201,4 +201,54 @@ class UserStoragesServiceTest extends StoragesServiceTest {
 		$this->assertEquals('', $backendOptions['password']);
 		$this->assertNotEmpty($backendOptions['password_encrypted']);
 	}
+
+	/**
+	 * Test reading in a legacy config and generating config ids.
+	 */
+	public function testReadLegacyConfigAndGenerateConfigId() {
+		$configFile = $this->dataDir . '/' . $this->userId . '/mount.json';
+
+		$legacyBackendOptions = [
+			'user' => 'someuser',
+			'password' => 'somepassword',
+		];
+		$legacyBackendOptions = \OC_Mount_Config::encryptPasswords($legacyBackendOptions);
+
+		$legacyConfig = [
+			'class' => '\OC\Files\Storage\SMB',
+			'options' => $legacyBackendOptions,
+			'mountOptions' => ['preview' => false],
+		];
+		// different mount options
+		$legacyConfig2 = [
+			'class' => '\OC\Files\Storage\SMB',
+			'options' => $legacyBackendOptions,
+			'mountOptions' => ['preview' => true],
+		];
+
+		$json = ['user' => []];
+		$json['user'][$this->userId] = [
+			'/$user/files/somemount' => $legacyConfig,
+			'/$user/files/anothermount' => $legacyConfig2,
+		];
+
+		file_put_contents($configFile, json_encode($json));
+
+		$allStorages = $this->service->getAllStorages();
+
+		$this->assertCount(2, $allStorages);
+
+		$storage1 = $allStorages[1];
+		$storage2 = $allStorages[2];
+
+		$this->assertEquals('/somemount', $storage1->getMountPoint());
+		$this->assertEquals('someuser', $storage1->getBackendOptions()['user']);
+		$this->assertEquals('somepassword', $storage1->getBackendOptions()['password']);
+		$this->assertEquals(['preview' => false], $storage1->getMountOptions());
+
+		$this->assertEquals('/anothermount', $storage2->getMountPoint());
+		$this->assertEquals('someuser', $storage2->getBackendOptions()['user']);
+		$this->assertEquals('somepassword', $storage2->getBackendOptions()['password']);
+		$this->assertEquals(['preview' => true], $storage2->getMountOptions());
+	}
 }