Allow saving incomplete external storage config

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

diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php
index 823c0bcbfc18e4fec6a04fb9484c3e863912e866..ddfab43987922933e744984d5ca9834b8d2f2d96 100644 (file)
--- 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 24e32390a980c63e9d90e3d44c0123e5de92dd06..f288d02705c59e220e75872ef98552753ed9514d 100644 (file)
--- 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']));
+       }
 }