Commit 3478634d authored by Vincent Petry's avatar Vincent Petry
Browse files

Merge pull request #13707 from owncloud/extstorage-fixincompletestorageconfig

Allow saving incomplete external storage config
parents 4d90fd93 37645153
......@@ -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;
}
......
......@@ -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']));
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment