diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index ffbab7bca80..384ab410e9c 100755 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -304,18 +304,23 @@ class OC_Mount_Config { $mount['options'] = self::decryptPasswords($mount['options']); // Remove '/$user/files/' from mount point $mountPoint = substr($mountPoint, 13); - // Merge the mount point into the current mount points - if (isset($system[$mountPoint]) && $system[$mountPoint]['configuration'] == $mount['options']) { - $system[$mountPoint]['applicable']['groups'] - = array_merge($system[$mountPoint]['applicable']['groups'], array($group)); + + $config = array( + 'class' => $mount['class'], + 'mountpoint' => $mountPoint, + 'backend' => $backends[$mount['class']]['backend'], + 'options' => $mount['options'], + 'applicable' => array('groups' => array($group), 'users' => array()), + 'status' => self::getBackendStatus($mount['class'], $mount['options']) + ); + $hash = self::makeConfigHash($config); + // If an existing config exists (with same class, mountpoint and options) + if (isset($system[$hash])) { + // add the groups into that config + $system[$hash]['applicable']['groups'] + = array_merge($system[$hash]['applicable']['groups'], array($group)); } else { - $system[$mountPoint] = array( - 'class' => $mount['class'], - 'backend' => $backends[$mount['class']]['backend'], - 'configuration' => $mount['options'], - 'applicable' => array('groups' => array($group), 'users' => array()), - 'status' => self::getBackendStatus($mount['class'], $mount['options']) - ); + $system[$hash] = $config; } } } @@ -330,23 +335,27 @@ class OC_Mount_Config { $mount['options'] = self::decryptPasswords($mount['options']); // Remove '/$user/files/' from mount point $mountPoint = substr($mountPoint, 13); - // Merge the mount point into the current mount points - if (isset($system[$mountPoint]) && $system[$mountPoint]['configuration'] == $mount['options']) { - $system[$mountPoint]['applicable']['users'] - = array_merge($system[$mountPoint]['applicable']['users'], array($user)); + $config = array( + 'class' => $mount['class'], + 'mountpoint' => $mountPoint, + 'backend' => $backends[$mount['class']]['backend'], + 'options' => $mount['options'], + 'applicable' => array('groups' => array(), 'users' => array($user)), + 'status' => self::getBackendStatus($mount['class'], $mount['options']) + ); + $hash = self::makeConfigHash($config); + // If an existing config exists (with same class, mountpoint and options) + if (isset($system[$hash])) { + // add the users into that config + $system[$hash]['applicable']['users'] + = array_merge($system[$hash]['applicable']['users'], array($user)); } else { - $system[$mountPoint] = array( - 'class' => $mount['class'], - 'backend' => $backends[$mount['class']]['backend'], - 'configuration' => $mount['options'], - 'applicable' => array('groups' => array(), 'users' => array($user)), - 'status' => self::getBackendStatus($mount['class'], $mount['options']) - ); + $system[$hash] = $config; } } } } - return $system; + return array_values($system); } /** @@ -366,11 +375,12 @@ class OC_Mount_Config { $mount['class'] = '\OC\Files\Storage\\'.substr($mount['class'], 15); } $mount['options'] = self::decryptPasswords($mount['options']); - // Remove '/uid/files/' from mount point - $personal[substr($mountPoint, strlen($uid) + 8)] = array( + $personal[] = array( 'class' => $mount['class'], + // Remove '/uid/files/' from mount point + 'mountpoint' => substr($mountPoint, strlen($uid) + 8), 'backend' => $backends[$mount['class']]['backend'], - 'configuration' => $mount['options'], + 'options' => $mount['options'], 'status' => self::getBackendStatus($mount['class'], $mount['options']) ); } @@ -707,4 +717,20 @@ class OC_Mount_Config { $cipher->setKey(\OCP\Config::getSystemValue('passwordsalt')); return $cipher; } + + /** + * Computes a hash based on the given configuration. + * This is mostly used to find out whether configurations + * are the same. + */ + private static function makeConfigHash($config) { + $data = json_encode( + array( + 'c' => $config['class'], + 'm' => $config['mountpoint'], + 'o' => $config['options'] + ) + ); + return hash('md5', $data); + } } diff --git a/apps/files_external/templates/settings.php b/apps/files_external/templates/settings.php index de44d3c8644..5e84fa8a252 100644 --- a/apps/files_external/templates/settings.php +++ b/apps/files_external/templates/settings.php @@ -16,17 +16,17 @@ array())); ?> - $mount): ?> - > + + > - + - class="remove" + class="remove" style="visibility:hidden;" ><?php p($l->t('Delete')); ?> array('all'), + 'groups' => array() + ) + ), + // applicable to single user + array( + OC_Mount_Config::MOUNT_TYPE_USER, + self::TEST_USER1, + array( + 'users' => array(self::TEST_USER1), + 'groups' => array() + ) + ), + // applicable to single group + array( + OC_Mount_Config::MOUNT_TYPE_GROUP, + self::TEST_GROUP1, + array( + 'users' => array(), + 'groups' => array(self::TEST_GROUP1) + ) + ), + ); + } + /** * Test reading and writing global config + * + * @dataProvider applicableConfigProvider */ - public function testReadWriteGlobalConfig() { - $mountType = OC_Mount_Config::MOUNT_TYPE_USER; - $applicable = 'all'; + public function testReadWriteGlobalConfig($mountType, $applicable, $expectApplicableArray) { + $mountType = $mountType; + $applicable = $applicable; $isPersonal = false; - $mountConfig = array( + $options = array( 'host' => 'smbhost', 'user' => 'smbuser', 'password' => 'smbpassword', @@ -231,7 +269,7 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { OC_Mount_Config::addMountPoint( '/ext', '\OC\Files\Storage\SMB', - $mountConfig, + $options, $mountType, $applicable, $isPersonal @@ -241,12 +279,13 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { // re-read config $config = OC_Mount_Config::getSystemMountPoints(); $this->assertEquals(1, count($config)); - $this->assertTrue(isset($config['ext'])); - $this->assertEquals('\OC\Files\Storage\SMB', $config['ext']['class']); - $savedMountConfig = $config['ext']['configuration']; - $this->assertEquals($mountConfig, $savedMountConfig); + $this->assertEquals('\OC\Files\Storage\SMB', $config[0]['class']); + $this->assertEquals('ext', $config[0]['mountpoint']); + $this->assertEquals($expectApplicableArray, $config[0]['applicable']); + $savedOptions = $config[0]['options']; + $this->assertEquals($options, $savedOptions); // key order needs to be preserved for the UI... - $this->assertEquals(array_keys($mountConfig), array_keys($savedMountConfig)); + $this->assertEquals(array_keys($options), array_keys($savedOptions)); } /** @@ -256,7 +295,7 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { $mountType = OC_Mount_Config::MOUNT_TYPE_USER; $applicable = self::TEST_USER1; $isPersonal = true; - $mountConfig = array( + $options = array( 'host' => 'smbhost', 'user' => 'smbuser', 'password' => 'smbpassword', @@ -269,7 +308,7 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { OC_Mount_Config::addMountPoint( '/ext', '\OC\Files\Storage\SMB', - $mountConfig, + $options, $mountType, $applicable, $isPersonal @@ -279,12 +318,12 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { // re-read config $config = OC_Mount_Config::getPersonalMountPoints(); $this->assertEquals(1, count($config)); - $this->assertTrue(isset($config['ext'])); - $this->assertEquals('\OC\Files\Storage\SMB', $config['ext']['class']); - $savedMountConfig = $config['ext']['configuration']; - $this->assertEquals($mountConfig, $savedMountConfig); + $this->assertEquals('\OC\Files\Storage\SMB', $config[0]['class']); + $this->assertEquals('ext', $config[0]['mountpoint']); + $savedOptions = $config[0]['options']; + $this->assertEquals($options, $savedOptions); // key order needs to be preserved for the UI... - $this->assertEquals(array_keys($mountConfig), array_keys($savedMountConfig)); + $this->assertEquals(array_keys($options), array_keys($savedOptions)); } /** @@ -362,7 +401,7 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { // re-read config, password was read correctly $config = OC_Mount_Config::getPersonalMountPoints(); - $savedMountConfig = $config['ext']['configuration']; + $savedMountConfig = $config[0]['options']; $this->assertEquals($mountConfig, $savedMountConfig); } @@ -475,4 +514,130 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { $this->assertEquals(0, count($mountPoints)); } } + + /** + * Test the same config for multiple users. + * The config will be merged by getSystemMountPoints(). + */ + public function testConfigMerging() { + $mountType = OC_Mount_Config::MOUNT_TYPE_USER; + $isPersonal = false; + $options = array( + 'host' => 'smbhost', + 'user' => 'smbuser', + 'password' => 'smbpassword', + 'share' => 'smbshare', + 'root' => 'smbroot' + ); + + // write config + $this->assertTrue( + OC_Mount_Config::addMountPoint( + '/ext', + '\OC\Files\Storage\SMB', + $options, + OC_Mount_Config::MOUNT_TYPE_USER, + self::TEST_USER1, + $isPersonal + ) + ); + + $this->assertTrue( + OC_Mount_Config::addMountPoint( + '/ext', + '\OC\Files\Storage\SMB', + $options, + OC_Mount_Config::MOUNT_TYPE_USER, + self::TEST_USER2, + $isPersonal + ) + ); + + $this->assertTrue( + OC_Mount_Config::addMountPoint( + '/ext', + '\OC\Files\Storage\SMB', + $options, + OC_Mount_Config::MOUNT_TYPE_GROUP, + self::TEST_GROUP2, + $isPersonal + ) + ); + + $this->assertTrue( + OC_Mount_Config::addMountPoint( + '/ext', + '\OC\Files\Storage\SMB', + $options, + OC_Mount_Config::MOUNT_TYPE_GROUP, + self::TEST_GROUP1, + $isPersonal + ) + ); + + // re-read config + $config = OC_Mount_Config::getSystemMountPoints(); + $this->assertEquals(1, count($config)); + $this->assertEquals('\OC\Files\Storage\SMB', $config[0]['class']); + $this->assertEquals('ext', $config[0]['mountpoint']); + $this->assertEquals($options, $config[0]['options']); + $this->assertEquals(array(self::TEST_USER1, self::TEST_USER2), $config[0]['applicable']['users']); + $this->assertEquals(array(self::TEST_GROUP2, self::TEST_GROUP1), $config[0]['applicable']['groups']); + } + + /** + * Create then re-read mount points configs where the mount points + * have the same path, the config must NOT be merged. + */ + public function testRereadMountpointWithSamePath() { + $mountType = OC_Mount_Config::MOUNT_TYPE_USER; + $isPersonal = false; + $options1 = array( + 'host' => 'smbhost', + 'user' => 'smbuser', + 'password' => 'smbpassword', + 'share' => 'smbshare', + 'root' => 'smbroot' + ); + + // write config + $this->assertTrue( + OC_Mount_Config::addMountPoint( + '/ext', + '\OC\Files\Storage\SMB', + $options1, + $mountType, + self::TEST_USER1, + $isPersonal + ) + ); + + $options2 = array( + 'host' => 'anothersmbhost', + 'user' => 'anothersmbuser', + 'password' => 'anothersmbpassword', + 'share' => 'anothersmbshare', + 'root' => 'anothersmbroot' + ); + $this->assertTrue( + OC_Mount_Config::addMountPoint( + '/ext', + '\OC\Files\Storage\SMB', + $options2, + $mountType, + self::TEST_USER2, + $isPersonal + ) + ); + + // re-read config + $config = OC_Mount_Config::getSystemMountPoints(); + $this->assertEquals(2, count($config)); + $this->assertEquals('\OC\Files\Storage\SMB', $config[0]['class']); + $this->assertEquals('ext', $config[0]['mountpoint']); + $this->assertEquals($options1, $config[0]['options']); + $this->assertEquals('\OC\Files\Storage\SMB', $config[1]['class']); + $this->assertEquals('ext', $config[1]['mountpoint']); + $this->assertEquals($options2, $config[1]['options']); + } }