Allow saving incomplete external storage config

This is needed for Dropbox and others that need a token.

2 files changed, 67 insertions, 2 deletions

diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php
index 823c0bcbfc1..ddfab439879 100644
--- a/apps/files_external/lib/config.php
+++ b/apps/files_external/lib/config.php
@@ -882,6 +882,11 @@ class OC_Mount_Config {
 		return hash('md5', $data);
 	}
 
+	/**
+	 * Add storage id to the storage configurations that did not have any.
+	 *
+	 * @param string $user user for which to process storage configs
+	 */
 	private static function addStorageIdToConfig($user) {
 		$config = self::readData($user);
 
@@ -899,13 +904,35 @@ class OC_Mount_Config {
 		}
 	}
 
+	/**
+	 * Get storage id from the numeric storage id and set
+	 * it into the given options argument. Only do this
+	 * if there was no storage id set yet.
+	 *
+	 * This might also fail if a storage wasn't fully configured yet
+	 * and couldn't be mounted, in which case this will simply return false.
+	 *
+	 * @param array $options storage options
+	 *
+	 * @return bool true if the storage id was added, false otherwise
+	 */
 	private static function addStorageId(&$options) {
 		if (isset($options['storage_id'])) {
 			return false;
 		}
+
 		$class = $options['class'];
-		/** @var \OC\Files\Storage\Storage $storage */
-		$storage = new $class($options['options']);
+		try {
+			/** @var \OC\Files\Storage\Storage $storage */
+			$storage = new $class($options['options']);
+			// TODO: introduce StorageConfigException
+		} catch (\Exception $e) {
+			// storage might not be fully configured yet (ex: Dropbox)
+			// note that storage instances aren't supposed to open any connections
+			// in the constructor, so this exception is likely to be a config exception
+			return false;
+		}
+
 		$options['storage_id'] = $storage->getCache()->getNumericStorageId();
 		return true;
 	}
diff --git a/apps/files_external/tests/mountconfig.php b/apps/files_external/tests/mountconfig.php
index 24e32390a98..f288d02705c 100644
--- a/apps/files_external/tests/mountconfig.php
+++ b/apps/files_external/tests/mountconfig.php
@@ -21,6 +21,12 @@
  */
 
 class Test_Mount_Config_Dummy_Storage {
+	public function __construct($params) {
+		if (isset($params['simulateFail']) && $params['simulateFail'] == true) {
+			throw new \Exception('Simulated config validation fail');
+		}
+	}
+
 	public function test() {
 		return true;
 	}
@@ -82,6 +88,13 @@ class Test_Mount_Config extends \Test\TestCase {
 	protected function setUp() {
 		parent::setUp();
 
+		OC_Mount_Config::registerBackend('Test_Mount_Config_Dummy_Storage', array(
+				'backend' => 'dummy',
+				'priority' => 150,
+				'configuration' => array()
+			)
+		);
+
 		\OC_User::createUser(self::TEST_USER1, self::TEST_USER1);
 		\OC_User::createUser(self::TEST_USER2, self::TEST_USER2);
 
@@ -1004,4 +1017,29 @@ class Test_Mount_Config extends \Test\TestCase {
 		$this->assertEquals($mountConfig,
 			$mountPointsOther['/'.self::TEST_USER1.'/files/ext']['options']);
 	}
+
+	public function testAllowWritingIncompleteConfigIfStorageContructorFails() {
+		$storageClass = 'Test_Mount_Config_Dummy_Storage';
+		$mountType = 'user';
+		$applicable = 'all';
+		$isPersonal = false;
+
+		$this->assertTrue(
+			OC_Mount_Config::addMountPoint(
+				'/ext',
+				$storageClass,
+				array('simulateFail' => true),
+				$mountType,
+				$applicable,
+				$isPersonal
+			)
+		);
+
+		// config can be retrieved afterwards
+		$mounts = OC_Mount_Config::getSystemMountPoints();
+		$this->assertEquals(1, count($mounts));
+
+		// no storage id was set
+		$this->assertFalse(isset($mounts[0]['storage_id']));
+	}
 }