[Pkg-owncloud-commits] [owncloud] 01/70: Fix merging of external storage configurations
David Prévot
taffit at moszumanska.debian.org
Thu Aug 21 17:39:58 UTC 2014
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to annotated tag v6.0.5RC1
in repository owncloud.
commit 26ded10052f6cbae48ef4ec765692dfdbc6d7394
Author: Vincent Petry <pvince81 at owncloud.com>
Date: Wed Mar 26 12:10:17 2014 +0100
Fix merging of external storage configurations
Merging of configurations is whenever the same config is available for
multiple users/groups, in which case the config is considered as a
single one by the UI, and shows multiple users/groups selected.
Fixed merging logic to make sure that class, mount point and options are
the same before merging them.
Fixed merging to work correctly when the same mount point path is used
for separate users and configs. These are now correctly shows in the UI
as separate entries.
Backport of e002b72 from master
---
apps/files_external/lib/config.php | 78 +++++++----
apps/files_external/templates/settings.php | 14 +-
apps/files_external/tests/mountconfig.php | 201 ++++++++++++++++++++++++++---
3 files changed, 242 insertions(+), 51 deletions(-)
diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php
index a20a2a0..8f19486 100755
--- a/apps/files_external/lib/config.php
+++ b/apps/files_external/lib/config.php
@@ -294,18 +294,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;
}
}
}
@@ -320,23 +325,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);
}
/**
@@ -356,11 +365,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'])
);
}
@@ -695,4 +705,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 3ca16c3..34778eb 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 0a0bb76..4df4ff3 100644
--- a/apps/files_external/tests/mountconfig.php
+++ b/apps/files_external/tests/mountconfig.php
@@ -188,13 +188,51 @@ 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',
@@ -207,7 +245,7 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase {
OC_Mount_Config::addMountPoint(
'/ext',
'\OC\Files\Storage\SMB',
- $mountConfig,
+ $options,
$mountType,
$applicable,
$isPersonal
@@ -217,12 +255,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));
}
/**
@@ -232,7 +271,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',
@@ -245,7 +284,7 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase {
OC_Mount_Config::addMountPoint(
'/ext',
'\OC\Files\Storage\SMB',
- $mountConfig,
+ $options,
$mountType,
$applicable,
$isPersonal
@@ -255,12 +294,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));
}
/**
@@ -338,7 +377,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);
}
@@ -451,4 +490,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']);
+ }
}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git
More information about the Pkg-owncloud-commits
mailing list