diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index 823c0bcbfc18e4fec6a04fb9484c3e863912e866..ddfab43987922933e744984d5ca9834b8d2f2d96 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 342f020d3a9bf6b477ece7240ea5bc9b2e4250fe..f288d02705c59e220e75872ef98552753ed9514d 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); @@ -184,7 +197,13 @@ class Test_Mount_Config extends \Test\TestCase { $applicable = 'all'; $isPersonal = false; - $this->assertEquals(true, OC_Mount_Config::addMountPoint('/ext', '\OC\Files\Storage\SFTP', array(), $mountType, $applicable, $isPersonal)); + $storageOptions = array( + 'host' => 'localhost', + 'user' => 'testuser', + 'password' => '12345', + ); + + $this->assertEquals(true, OC_Mount_Config::addMountPoint('/ext', '\OC\Files\Storage\SFTP', $storageOptions, $mountType, $applicable, $isPersonal)); $config = $this->readGlobalConfig(); $this->assertEquals(1, count($config)); @@ -205,7 +224,13 @@ class Test_Mount_Config extends \Test\TestCase { $applicable = self::TEST_USER1; $isPersonal = true; - $this->assertEquals(true, OC_Mount_Config::addMountPoint('/ext', '\OC\Files\Storage\SFTP', array(), $mountType, $applicable, $isPersonal)); + $storageOptions = array( + 'host' => 'localhost', + 'user' => 'testuser', + 'password' => '12345', + ); + + $this->assertEquals(true, OC_Mount_Config::addMountPoint('/ext', '\OC\Files\Storage\SFTP', $storageOptions, $mountType, $applicable, $isPersonal)); $config = $this->readUserConfig(); $this->assertEquals(1, count($config)); @@ -236,8 +261,14 @@ class Test_Mount_Config extends \Test\TestCase { implode(',', array_keys($this->allBackends)) ); + $storageOptions = array( + 'host' => 'localhost', + 'user' => 'testuser', + 'password' => '12345', + ); + // non-local but forbidden - $this->assertFalse(OC_Mount_Config::addMountPoint('/ext', '\OC\Files\Storage\SFTP', array(), $mountType, $applicable, $isPersonal)); + $this->assertFalse(OC_Mount_Config::addMountPoint('/ext', '\OC\Files\Storage\SFTP', $storageOptions, $mountType, $applicable, $isPersonal)); $this->assertFalse(file_exists($this->userHome . '/mount.json')); } @@ -629,7 +660,8 @@ class Test_Mount_Config extends \Test\TestCase { 'host' => 'someost', 'user' => 'someuser', 'password' => 'somepassword', - 'root' => 'someroot' + 'root' => 'someroot', + 'share' => '', ); // add mount point as "test" user @@ -872,7 +904,8 @@ class Test_Mount_Config extends \Test\TestCase { 'host' => 'somehost', 'user' => 'someuser', 'password' => 'somepassword', - 'root' => 'someroot' + 'root' => 'someroot', + 'share' => '', ); // Add mount points @@ -908,7 +941,8 @@ class Test_Mount_Config extends \Test\TestCase { 'host' => 'somehost', 'user' => 'someuser', 'password' => 'somepassword', - 'root' => 'someroot' + 'root' => 'someroot', + 'share' => '', ); $this->assertTrue( @@ -954,7 +988,8 @@ class Test_Mount_Config extends \Test\TestCase { 'host' => 'somehost', 'user' => 'someuser', 'password' => 'somepassword', - 'root' => 'someroot' + 'root' => 'someroot', + 'share' => '', ); // Create personal mount point @@ -982,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'])); + } }