diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php
index c5b091336e197ad803c602d3c87fa305ff292bd3..f7caafb74aaf5c54b0d80e19b9f53703dfa2bc4e 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'])
);
}
@@ -712,4 +722,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 de44d3c86442180402458abc9be3dac82c6b94d5..5e84fa8a25286969b241ac21c9070564249139d5 100644
--- a/apps/files_external/templates/settings.php
+++ b/apps/files_external/templates/settings.php
@@ -16,17 +16,17 @@
</thead>
<tbody width="100%">
<?php $_['mounts'] = array_merge($_['mounts'], array('' => array())); ?>
- <?php foreach ($_['mounts'] as $mountPoint => $mount): ?>
- <tr <?php print_unescaped(($mountPoint != '') ? 'class="'.OC_Util::sanitizeHTML($mount['class']).'"' : 'id="addMountPoint"'); ?>>
+ <?php foreach ($_['mounts'] as $mount): ?>
+ <tr <?php print_unescaped(($mount['mountpoint'] !== '') ? 'class="'.OC_Util::sanitizeHTML($mount['class']).'"' : 'id="addMountPoint"'); ?>>
<td class="status">
<?php if (isset($mount['status'])): ?>
<span class="<?php p(($mount['status']) ? 'success' : 'error'); ?>"></span>
<?php endif; ?>
</td>
<td class="mountPoint"><input type="text" name="mountPoint"
- value="<?php p($mountPoint); ?>"
+ value="<?php p($mount['mountpoint']); ?>"
placeholder="<?php p($l->t('Folder name')); ?>" /></td>
- <?php if ($mountPoint == ''): ?>
+ <?php if ($mount['mountpoint'] == ''): ?>
<td class="backend">
<select id="selectBackend" data-configurations='<?php print_unescaped(json_encode($_['backends'])); ?>'>
<option value="" disabled selected
@@ -41,8 +41,8 @@
data-class="<?php p($mount['class']); ?>"><?php p($mount['backend']); ?></td>
<?php endif; ?>
<td class ="configuration" width="100%">
- <?php if (isset($mount['configuration'])): ?>
- <?php foreach ($mount['configuration'] as $parameter => $value): ?>
+ <?php if (isset($mount['options'])): ?>
+ <?php foreach ($mount['options'] as $parameter => $value): ?>
<?php if (isset($_['backends'][$mount['class']]['configuration'][$parameter])): ?>
<?php $placeholder = $_['backends'][$mount['class']]['configuration'][$parameter]; ?>
<?php if (strpos($placeholder, '*') !== false): ?>
@@ -108,7 +108,7 @@
</select>
</td>
<?php endif; ?>
- <td <?php if ($mountPoint != ''): ?>class="remove"
+ <td <?php if ($mount['mountpoint'] != ''): ?>class="remove"
<?php else: ?>style="visibility:hidden;"
<?php endif ?>><img alt="<?php p($l->t('Delete')); ?>"
title="<?php p($l->t('Delete')); ?>"
diff --git a/apps/files_external/tests/mountconfig.php b/apps/files_external/tests/mountconfig.php
index bf43bb31c3876c3fe6d108163abcc5646c078058..c89874c94d59ce32cd048b330804fae9875ab830 100644
--- a/apps/files_external/tests/mountconfig.php
+++ b/apps/files_external/tests/mountconfig.php
@@ -211,14 +211,52 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase {
}
+ /**
+ * Provider for testing configurations with different
+ * "applicable" values (all, user, groups)
+ */
+ public function applicableConfigProvider() {
+ return array(
+ // applicable to "all"
+ array(
+ OC_Mount_Config::MOUNT_TYPE_USER,
+ 'all',
+ array(
+ 'users' => 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']);
+ }
}