From 05642733a365cd150dd9ebb3b7cf7ec9c55ecab3 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Fri, 28 Mar 2014 14:24:38 +0000 Subject: [PATCH 1/9] Add storage priority support Each storage backend has a default priority, assigned to any system mounts created in ownCloud. mount.json can be manually modified to change these priorities. The priority order is as follows: * Personal * User * Group * Global Within each mount type, the mount with the highest priority is active. The storage backend defaults were chosen to be the following: * Local - 150 * Remote storage - 100 * SMB / CIFS with OC login - 90 --- apps/files_external/appinfo/app.php | 12 +++++++ apps/files_external/lib/config.php | 52 ++++++++++++++++++++++++++--- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/apps/files_external/appinfo/app.php b/apps/files_external/appinfo/app.php index e8ed8950c3..ca164784fb 100644 --- a/apps/files_external/appinfo/app.php +++ b/apps/files_external/appinfo/app.php @@ -32,11 +32,13 @@ OCP\Util::connectHook('OC_User', 'post_login', 'OC\Files\Storage\SMB_OC', 'login OC_Mount_Config::registerBackend('\OC\Files\Storage\Local', array( 'backend' => (string)$l->t('Local'), + 'priority' => 150, 'configuration' => array( 'datadir' => (string)$l->t('Location')))); OC_Mount_Config::registerBackend('\OC\Files\Storage\AmazonS3', array( 'backend' => (string)$l->t('Amazon S3'), + 'priority' => 100, 'configuration' => array( 'key' => (string)$l->t('Key'), 'secret' => '*'.$l->t('Secret'), @@ -45,6 +47,7 @@ OC_Mount_Config::registerBackend('\OC\Files\Storage\AmazonS3', array( OC_Mount_Config::registerBackend('\OC\Files\Storage\AmazonS3', array( 'backend' => (string)$l->t('Amazon S3 and compliant'), + 'priority' => 100, 'configuration' => array( 'key' => (string)$l->t('Access Key'), 'secret' => '*'.$l->t('Secret Key'), @@ -58,6 +61,7 @@ OC_Mount_Config::registerBackend('\OC\Files\Storage\AmazonS3', array( OC_Mount_Config::registerBackend('\OC\Files\Storage\Dropbox', array( 'backend' => 'Dropbox', + 'priority' => 100, 'configuration' => array( 'configured' => '#configured', 'app_key' => (string)$l->t('App key'), @@ -69,6 +73,7 @@ OC_Mount_Config::registerBackend('\OC\Files\Storage\Dropbox', array( OC_Mount_Config::registerBackend('\OC\Files\Storage\FTP', array( 'backend' => 'FTP', + 'priority' => 100, 'configuration' => array( 'host' => (string)$l->t('Host'), 'user' => (string)$l->t('Username'), @@ -79,6 +84,7 @@ OC_Mount_Config::registerBackend('\OC\Files\Storage\FTP', array( OC_Mount_Config::registerBackend('\OC\Files\Storage\Google', array( 'backend' => 'Google Drive', + 'priority' => 100, 'configuration' => array( 'configured' => '#configured', 'client_id' => (string)$l->t('Client ID'), @@ -90,6 +96,7 @@ OC_Mount_Config::registerBackend('\OC\Files\Storage\Google', array( OC_Mount_Config::registerBackend('\OC\Files\Storage\Swift', array( 'backend' => (string)$l->t('OpenStack Object Storage'), + 'priority' => 100, 'configuration' => array( 'user' => (string)$l->t('Username (required)'), 'bucket' => (string)$l->t('Bucket (required)'), @@ -107,6 +114,7 @@ OC_Mount_Config::registerBackend('\OC\Files\Storage\Swift', array( if (!OC_Util::runningOnWindows()) { OC_Mount_Config::registerBackend('\OC\Files\Storage\SMB', array( 'backend' => 'SMB / CIFS', + 'priority' => 100, 'configuration' => array( 'host' => (string)$l->t('Host'), 'user' => (string)$l->t('Username'), @@ -117,6 +125,7 @@ if (!OC_Util::runningOnWindows()) { OC_Mount_Config::registerBackend('\OC\Files\Storage\SMB_OC', array( 'backend' => (string)$l->t('SMB / CIFS using OC login'), + 'priority' => 90, 'configuration' => array( 'host' => (string)$l->t('Host'), 'username_as_share' => '!'.$l->t('Username as share'), @@ -127,6 +136,7 @@ if (!OC_Util::runningOnWindows()) { OC_Mount_Config::registerBackend('\OC\Files\Storage\DAV', array( 'backend' => 'WebDAV', + 'priority' => 100, 'configuration' => array( 'host' => (string)$l->t('URL'), 'user' => (string)$l->t('Username'), @@ -137,6 +147,7 @@ OC_Mount_Config::registerBackend('\OC\Files\Storage\DAV', array( OC_Mount_Config::registerBackend('\OC\Files\Storage\OwnCloud', array( 'backend' => 'ownCloud', + 'priority' => 100, 'configuration' => array( 'host' => (string)$l->t('URL'), 'user' => (string)$l->t('Username'), @@ -147,6 +158,7 @@ OC_Mount_Config::registerBackend('\OC\Files\Storage\OwnCloud', array( OC_Mount_Config::registerBackend('\OC\Files\Storage\SFTP', array( 'backend' => 'SFTP', + 'priority' => 100, 'configuration' => array( 'host' => (string)$l->t('Host'), 'user' => (string)$l->t('Username'), diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index 7a651239cb..e23a72b56f 100755 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -126,6 +126,8 @@ class OC_Mount_Config { $datadir = \OC_Config::getValue("datadirectory", \OC::$SERVERROOT . "/data"); $mount_file = \OC_Config::getValue("mount_file", $datadir . "/mount.json"); + $backends = self::getBackends(); + //move config file to it's new position if (is_file(\OC::$SERVERROOT . '/config/mount.json')) { rename(\OC::$SERVERROOT . '/config/mount.json', $mount_file); @@ -136,7 +138,15 @@ class OC_Mount_Config { if (isset($mountConfig[self::MOUNT_TYPE_GLOBAL])) { foreach ($mountConfig[self::MOUNT_TYPE_GLOBAL] as $mountPoint => $options) { $options['options'] = self::decryptPasswords($options['options']); - $mountPoints[$mountPoint] = $options; + if (!isset($options['priority'])) { + $options['priority'] = $backends[$options['class']]['priority']; + } + + if ( (!isset($mountPoints[$mountPoint])) + || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) ) { + $options['priority_type'] = 'global'; + $mountPoints[$mountPoint] = $options; + } } } if (isset($mountConfig[self::MOUNT_TYPE_GROUP])) { @@ -148,7 +158,16 @@ class OC_Mount_Config { $option = self::setUserVars($user, $option); } $options['options'] = self::decryptPasswords($options['options']); - $mountPoints[$mountPoint] = $options; + if (!isset($options['priority'])) { + $options['priority'] = $backends[$options['class']]['priority']; + } + + if ( (!isset($mountPoints[$mountPoint])) + || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) + || ($mountPoints[$mountPoint]['priority_type'] != 'group') ) { + $options['priority_type'] = 'group'; + $mountPoints[$mountPoint] = $options; + } } } } @@ -162,7 +181,16 @@ class OC_Mount_Config { $option = self::setUserVars($user, $option); } $options['options'] = self::decryptPasswords($options['options']); - $mountPoints[$mountPoint] = $options; + if (!isset($options['priority'])) { + $options['priority'] = $backends[$options['class']]['priority']; + } + + if ( (!isset($mountPoints[$mountPoint])) + || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) + || ($mountPoints[$mountPoint]['priority_type'] != 'user') ) { + $options['priority_type'] = 'user'; + $mountPoints[$mountPoint] = $options; + } } } } @@ -173,7 +201,12 @@ class OC_Mount_Config { if (isset($mountConfig[self::MOUNT_TYPE_USER][$user])) { foreach ($mountConfig[self::MOUNT_TYPE_USER][$user] as $mountPoint => $options) { $options['options'] = self::decryptPasswords($options['options']); - $mountPoints[$mountPoint] = $options; + + if ( (!isset($mountPoints[$mountPoint])) + || ($mountPoints[$mountPoint]['priority_type'] != 'personal') ) { + $options['priority_type'] = 'personal'; + $mountPoints[$mountPoint] = $options; + } } } @@ -244,6 +277,9 @@ class OC_Mount_Config { $mount['class'] = '\OC\Files\Storage\\'.substr($mount['class'], 15); } $mount['options'] = self::decryptPasswords($mount['options']); + if (!isset($mount['priority'])) { + $mount['priority'] = $backends[$mount['class']]['priority']; + } // Remove '/$user/files/' from mount point $mountPoint = substr($mountPoint, 13); @@ -251,6 +287,7 @@ class OC_Mount_Config { 'class' => $mount['class'], 'mountpoint' => $mountPoint, 'backend' => $backends[$mount['class']]['backend'], + 'priority' => $mount['priority'], 'options' => $mount['options'], 'applicable' => array('groups' => array($group), 'users' => array()), 'status' => self::getBackendStatus($mount['class'], $mount['options'], false) @@ -275,12 +312,16 @@ class OC_Mount_Config { $mount['class'] = '\OC\Files\Storage\\'.substr($mount['class'], 15); } $mount['options'] = self::decryptPasswords($mount['options']); + if (!isset($mount['priority'])) { + $mount['priority'] = $backends[$mount['class']]['priority']; + } // Remove '/$user/files/' from mount point $mountPoint = substr($mountPoint, 13); $config = array( 'class' => $mount['class'], 'mountpoint' => $mountPoint, 'backend' => $backends[$mount['class']]['backend'], + 'priority' => $mount['priority'], 'options' => $mount['options'], 'applicable' => array('groups' => array(), 'users' => array($user)), 'status' => self::getBackendStatus($mount['class'], $mount['options'], false) @@ -400,6 +441,9 @@ class OC_Mount_Config { 'options' => self::encryptPasswords($classOptions)) ) ); + if (! $isPersonal) { + $mount[$applicable][$mountPoint]['priority'] = $backends[$class]['priority']; + } $mountPoints = self::readData($isPersonal ? OCP\User::getUser() : NULL); $mountPoints = self::mergeMountPoints($mountPoints, $mount, $mountType); From 9151d7250785eb244a8cfe8082877c231876c671 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Fri, 28 Mar 2014 15:22:35 +0000 Subject: [PATCH 2/9] Treat 'all users' as 'global' A mount applicable to all users is as good a definition of global as can be --- apps/files_external/lib/config.php | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index e23a72b56f..5e5340e910 100755 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -149,6 +149,26 @@ class OC_Mount_Config { } } } + if (isset($mountConfig[self::MOUNT_TYPE_USER]) && isset($mountConfig[self::MOUNT_TYPE_USER]['all'])) { + $mounts = $mountConfig[self::MOUNT_TYPE_USER]['all']; + foreach ($mounts as $mountPoint => $options) { + $mountPoint = self::setUserVars($user, $mountPoint); + foreach ($options as &$option) { + $option = self::setUserVars($user, $option); + } + $options['options'] = self::decryptPasswords($options['options']); + if (!isset($options['priority'])) { + $options['priority'] = $backends[$options['class']]['priority']; + } + + if ( (!isset($mountPoints[$mountPoint])) + || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) + || ($mountPoints[$mountPoint]['priority_type'] != 'global') ) { + $options['priority_type'] = 'global'; + $mountPoints[$mountPoint] = $options; + } + } + } if (isset($mountConfig[self::MOUNT_TYPE_GROUP])) { foreach ($mountConfig[self::MOUNT_TYPE_GROUP] as $group => $mounts) { if (\OC_Group::inGroup($user, $group)) { @@ -174,7 +194,7 @@ class OC_Mount_Config { } if (isset($mountConfig[self::MOUNT_TYPE_USER])) { foreach ($mountConfig[self::MOUNT_TYPE_USER] as $mountUser => $mounts) { - if ($mountUser === 'all' or strtolower($mountUser) === strtolower($user)) { + if (strtolower($mountUser) === strtolower($user)) { foreach ($mounts as $mountPoint => $options) { $mountPoint = self::setUserVars($user, $mountPoint); foreach ($options as &$option) { From e4d3ee786677ff768f41d1adfabd7d558a2c2c0e Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Thu, 3 Apr 2014 14:57:35 +0100 Subject: [PATCH 3/9] Preserve priority if already set --- apps/files_external/lib/config.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index 5e5340e910..223b91788c 100755 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -756,6 +756,11 @@ class OC_Mount_Config { $applicable = key($mountPoint); if (isset($data[$mountType])) { if (isset($data[$mountType][$applicable])) { + if (isset($mountPoints[$mountType][$applicable][$mountPoint]) + && isset($mountPoints[$mountType][$applicable][$mountPoint]['priority'])) { + $mount[$applicable][$mountPoint]['priority'] + = $mountPoints[$mountType][$applicable][$mountPoint]['priority']; + } $data[$mountType][$applicable] = array_merge($data[$mountType][$applicable], $mountPoint[$applicable]); } else { From a0ccb060fac1dfa8d2885ab209bd31b606536e14 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Tue, 29 Apr 2014 10:00:26 +0100 Subject: [PATCH 4/9] Use more strict type comparisons --- apps/files_external/lib/config.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index 223b91788c..48f3c1f4a1 100755 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -163,7 +163,7 @@ class OC_Mount_Config { if ( (!isset($mountPoints[$mountPoint])) || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) - || ($mountPoints[$mountPoint]['priority_type'] != 'global') ) { + || ($mountPoints[$mountPoint]['priority_type'] !== 'global') ) { $options['priority_type'] = 'global'; $mountPoints[$mountPoint] = $options; } @@ -184,7 +184,7 @@ class OC_Mount_Config { if ( (!isset($mountPoints[$mountPoint])) || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) - || ($mountPoints[$mountPoint]['priority_type'] != 'group') ) { + || ($mountPoints[$mountPoint]['priority_type'] !== 'group') ) { $options['priority_type'] = 'group'; $mountPoints[$mountPoint] = $options; } @@ -207,7 +207,7 @@ class OC_Mount_Config { if ( (!isset($mountPoints[$mountPoint])) || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) - || ($mountPoints[$mountPoint]['priority_type'] != 'user') ) { + || ($mountPoints[$mountPoint]['priority_type'] !== 'user') ) { $options['priority_type'] = 'user'; $mountPoints[$mountPoint] = $options; } @@ -223,7 +223,7 @@ class OC_Mount_Config { $options['options'] = self::decryptPasswords($options['options']); if ( (!isset($mountPoints[$mountPoint])) - || ($mountPoints[$mountPoint]['priority_type'] != 'personal') ) { + || ($mountPoints[$mountPoint]['priority_type'] !== 'personal') ) { $options['priority_type'] = 'personal'; $mountPoints[$mountPoint] = $options; } From 2254678a0cb9eb292413f2dca0602d4515077ab5 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Fri, 9 May 2014 20:59:56 +0100 Subject: [PATCH 5/9] Use constants for priority type and add comments --- apps/files_external/lib/config.php | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index 48f3c1f4a1..a54101b442 100755 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -35,6 +35,7 @@ class OC_Mount_Config { const MOUNT_TYPE_GLOBAL = 'global'; const MOUNT_TYPE_GROUP = 'group'; const MOUNT_TYPE_USER = 'user'; + const MOUNT_TYPE_PERSONAL = 'personal'; // whether to skip backend test (for unit tests, as this static class is not mockable) public static $skipTest = false; @@ -135,6 +136,8 @@ class OC_Mount_Config { // Load system mount points $mountConfig = self::readData(); + + // Global mount points (is this redundant?) if (isset($mountConfig[self::MOUNT_TYPE_GLOBAL])) { foreach ($mountConfig[self::MOUNT_TYPE_GLOBAL] as $mountPoint => $options) { $options['options'] = self::decryptPasswords($options['options']); @@ -144,11 +147,12 @@ class OC_Mount_Config { if ( (!isset($mountPoints[$mountPoint])) || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) ) { - $options['priority_type'] = 'global'; + $options['priority_type'] = self::MOUNT_TYPE_GLOBAL; $mountPoints[$mountPoint] = $options; } } } + // All user mount points if (isset($mountConfig[self::MOUNT_TYPE_USER]) && isset($mountConfig[self::MOUNT_TYPE_USER]['all'])) { $mounts = $mountConfig[self::MOUNT_TYPE_USER]['all']; foreach ($mounts as $mountPoint => $options) { @@ -162,13 +166,13 @@ class OC_Mount_Config { } if ( (!isset($mountPoints[$mountPoint])) - || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) - || ($mountPoints[$mountPoint]['priority_type'] !== 'global') ) { - $options['priority_type'] = 'global'; + || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) ) { + $options['priority_type'] = self::MOUNT_TYPE_GLOBAL; $mountPoints[$mountPoint] = $options; } } } + // Group mount points if (isset($mountConfig[self::MOUNT_TYPE_GROUP])) { foreach ($mountConfig[self::MOUNT_TYPE_GROUP] as $group => $mounts) { if (\OC_Group::inGroup($user, $group)) { @@ -184,14 +188,15 @@ class OC_Mount_Config { if ( (!isset($mountPoints[$mountPoint])) || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) - || ($mountPoints[$mountPoint]['priority_type'] !== 'group') ) { - $options['priority_type'] = 'group'; + || ($mountPoints[$mountPoint]['priority_type'] !== self::MOUNT_TYPE_GROUP) ) { + $options['priority_type'] = self::MOUNT_TYPE_GROUP; $mountPoints[$mountPoint] = $options; } } } } } + // User mount points if (isset($mountConfig[self::MOUNT_TYPE_USER])) { foreach ($mountConfig[self::MOUNT_TYPE_USER] as $mountUser => $mounts) { if (strtolower($mountUser) === strtolower($user)) { @@ -207,8 +212,8 @@ class OC_Mount_Config { if ( (!isset($mountPoints[$mountPoint])) || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) - || ($mountPoints[$mountPoint]['priority_type'] !== 'user') ) { - $options['priority_type'] = 'user'; + || ($mountPoints[$mountPoint]['priority_type'] !== self::MOUNT_TYPE_USER) ) { + $options['priority_type'] = self::MOUNT_TYPE_USER; $mountPoints[$mountPoint] = $options; } } @@ -222,11 +227,9 @@ class OC_Mount_Config { foreach ($mountConfig[self::MOUNT_TYPE_USER][$user] as $mountPoint => $options) { $options['options'] = self::decryptPasswords($options['options']); - if ( (!isset($mountPoints[$mountPoint])) - || ($mountPoints[$mountPoint]['priority_type'] !== 'personal') ) { - $options['priority_type'] = 'personal'; - $mountPoints[$mountPoint] = $options; - } + // Always override previous config + $options['priority_type'] = self::MOUNT_TYPE_PERSONAL; + $mountPoints[$mountPoint] = $options; } } From eae45dca71a089aa72cc780ecee7a9d7dd9ab3a1 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Wed, 14 May 2014 20:32:19 +0100 Subject: [PATCH 6/9] Add unit tests --- apps/files_external/lib/config.php | 21 ++-- apps/files_external/tests/mountconfig.php | 117 ++++++++++++++++++++++ 2 files changed, 130 insertions(+), 8 deletions(-) diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index a54101b442..788a136c38 100755 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -427,6 +427,7 @@ class OC_Mount_Config { * @param string $mountType MOUNT_TYPE_GROUP | MOUNT_TYPE_USER * @param string $applicable User or group to apply mount to * @param bool $isPersonal Personal or system mount point i.e. is this being called from the personal or admin page + * @param int|null $priority Mount point priority, null for default * @return boolean */ public static function addMountPoint($mountPoint, @@ -434,7 +435,8 @@ class OC_Mount_Config { $classOptions, $mountType, $applicable, - $isPersonal = false) { + $isPersonal = false, + $priority = null) { $backends = self::getBackends(); $mountPoint = OC\Files\Filesystem::normalizePath($mountPoint); if ($mountPoint === '' || $mountPoint === '/') { @@ -464,12 +466,19 @@ class OC_Mount_Config { 'options' => self::encryptPasswords($classOptions)) ) ); - if (! $isPersonal) { - $mount[$applicable][$mountPoint]['priority'] = $backends[$class]['priority']; + if (! $isPersonal && !is_null($priority)) { + $mount[$applicable][$mountPoint]['priority'] = $priority; } $mountPoints = self::readData($isPersonal ? OCP\User::getUser() : NULL); $mountPoints = self::mergeMountPoints($mountPoints, $mount, $mountType); + + // Set default priority if none set + if (!isset($mountPoints[$mountType][$applicable][$mountPoint]['priority'])) { + $mountPoints[$mountType][$applicable][$mountPoint]['priority'] + = $backends[$class]['priority']; + } + self::writeData($isPersonal ? OCP\User::getUser() : NULL, $mountPoints); return self::getBackendStatus($class, $classOptions, $isPersonal); @@ -757,13 +766,9 @@ class OC_Mount_Config { */ private static function mergeMountPoints($data, $mountPoint, $mountType) { $applicable = key($mountPoint); + $mountPath = key($mountPoint[$applicable]); if (isset($data[$mountType])) { if (isset($data[$mountType][$applicable])) { - if (isset($mountPoints[$mountType][$applicable][$mountPoint]) - && isset($mountPoints[$mountType][$applicable][$mountPoint]['priority'])) { - $mount[$applicable][$mountPoint]['priority'] - = $mountPoints[$mountType][$applicable][$mountPoint]['priority']; - } $data[$mountType][$applicable] = array_merge($data[$mountType][$applicable], $mountPoint[$applicable]); } else { diff --git a/apps/files_external/tests/mountconfig.php b/apps/files_external/tests/mountconfig.php index 14fe1d90b7..33cbd10d9e 100644 --- a/apps/files_external/tests/mountconfig.php +++ b/apps/files_external/tests/mountconfig.php @@ -41,16 +41,22 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { const TEST_USER1 = 'user1'; const TEST_USER2 = 'user2'; const TEST_GROUP1 = 'group1'; + const TEST_GROUP1B = 'group1b'; const TEST_GROUP2 = 'group2'; + const TEST_GROUP2B = 'group2b'; public function setUp() { \OC_User::createUser(self::TEST_USER1, self::TEST_USER1); \OC_User::createUser(self::TEST_USER2, self::TEST_USER2); \OC_Group::createGroup(self::TEST_GROUP1); + \OC_Group::createGroup(self::TEST_GROUP1B); \OC_Group::addToGroup(self::TEST_USER1, self::TEST_GROUP1); + \OC_Group::addToGroup(self::TEST_USER1, self::TEST_GROUP1B); \OC_Group::createGroup(self::TEST_GROUP2); + \OC_Group::createGroup(self::TEST_GROUP2B); \OC_Group::addToGroup(self::TEST_USER2, self::TEST_GROUP2); + \OC_Group::addToGroup(self::TEST_USER2, self::TEST_GROUP2B); \OC_User::setUserId(self::TEST_USER1); $this->userHome = \OC_User::getHome(self::TEST_USER1); @@ -81,7 +87,9 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { \OC_User::deleteUser(self::TEST_USER2); \OC_User::deleteUser(self::TEST_USER1); \OC_Group::deleteGroup(self::TEST_GROUP1); + \OC_Group::deleteGroup(self::TEST_GROUP1B); \OC_Group::deleteGroup(self::TEST_GROUP2); + \OC_Group::deleteGroup(self::TEST_GROUP2B); @unlink($this->dataDir . '/mount.json'); @@ -635,4 +643,113 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { $this->assertEquals('ext', $config[1]['mountpoint']); $this->assertEquals($options2, $config[1]['options']); } + + public function priorityDataProvider() { + return array( + + // test 1 - group vs group + array( + array( + array( + 'isPersonal' => false, + 'mountType' => OC_Mount_Config::MOUNT_TYPE_GROUP, + 'applicable' => self::TEST_GROUP1, + 'priority' => 50 + ), + array( + 'isPersonal' => false, + 'mountType' => OC_Mount_Config::MOUNT_TYPE_GROUP, + 'applicable' => self::TEST_GROUP1B, + 'priority' => 60 + ) + ), + 1 + ), + // test 2 - user vs personal + array( + array( + array( + 'isPersonal' => false, + 'mountType' => OC_Mount_Config::MOUNT_TYPE_USER, + 'applicable' => self::TEST_USER1, + 'priority' => 2000 + ), + array( + 'isPersonal' => true, + 'mountType' => OC_Mount_Config::MOUNT_TYPE_USER, + 'applicable' => self::TEST_USER1, + 'priority' => null + ) + ), + 1 + ), + // test 3 - all vs group vs user + array( + array( + array( + 'isPersonal' => false, + 'mountType' => OC_Mount_Config::MOUNT_TYPE_USER, + 'applicable' => 'all', + 'priority' => 70 + ), + array( + 'isPersonal' => false, + 'mountType' => OC_Mount_Config::MOUNT_TYPE_GROUP, + 'applicable' => self::TEST_GROUP1, + 'priority' => 60 + ), + array( + 'isPersonal' => false, + 'mountType' => OC_Mount_Config::MOUNT_TYPE_USER, + 'applicable' => self::TEST_USER1, + 'priority' => 50 + ) + ), + 2 + ) + + ); + } + + /** + * Ensure priorities are being respected + * Test user is self::TEST_USER1 + * + * @dataProvider priorityDataProvider + * @param array[] $mounts array of associative array of mount parameters: + * bool $isPersonal + * string $mountType + * string $applicable + * int|null $priority null for personal + * @param int $expected index of expected visible mount + */ + public function testPriority($mounts, $expected) { + $mountConfig = array( + 'host' => 'somehost', + 'user' => 'someuser', + 'password' => 'somepassword', + 'root' => 'someroot' + ); + + // Add mount points + foreach($mounts as $i => $mount) { + $this->assertTrue( + OC_Mount_Config::addMountPoint( + '/ext', + '\OC\Files\Storage\SMB', + $mountConfig + array('id' => $i), + $mount['mountType'], + $mount['applicable'], + $mount['isPersonal'], + $mount['priority'] + ) + ); + } + + // Get mount points for user + $mountPoints = OC_Mount_Config::getAbsoluteMountPoints(self::TEST_USER1); + + $this->assertEquals(1, count($mountPoints)); + $this->assertEquals($expected, $mountPoints['/'.self::TEST_USER1.'/files/ext']['options']['id']); + } } From 0a8a3199158a7a52c071054df4b4d740c90db5ac Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Wed, 14 May 2014 22:34:38 +0100 Subject: [PATCH 7/9] Fix priority merging logic and add unit test --- apps/files_external/lib/config.php | 7 ++++ apps/files_external/tests/mountconfig.php | 48 +++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index 788a136c38..d47a2b4547 100755 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -769,6 +769,13 @@ class OC_Mount_Config { $mountPath = key($mountPoint[$applicable]); if (isset($data[$mountType])) { if (isset($data[$mountType][$applicable])) { + // Merge priorities + if (isset($data[$mountType][$applicable][$mountPath]) + && isset($data[$mountType][$applicable][$mountPath]['priority']) + && !isset($mountPoint[$applicable][$mountPath]['priority'])) { + $mountPoint[$applicable][$mountPath]['priority'] + = $data[$mountType][$applicable][$mountPath]['priority']; + } $data[$mountType][$applicable] = array_merge($data[$mountType][$applicable], $mountPoint[$applicable]); } else { diff --git a/apps/files_external/tests/mountconfig.php b/apps/files_external/tests/mountconfig.php index 33cbd10d9e..9b04e200e2 100644 --- a/apps/files_external/tests/mountconfig.php +++ b/apps/files_external/tests/mountconfig.php @@ -752,4 +752,52 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { $this->assertEquals(1, count($mountPoints)); $this->assertEquals($expected, $mountPoints['/'.self::TEST_USER1.'/files/ext']['options']['id']); } + + /** + * Test for persistence of priority when changing mount options + */ + public function testPriorityPersistence() { + $class = '\OC\Files\Storage\SMB'; + $priority = 123; + $mountConfig = array( + 'host' => 'somehost', + 'user' => 'someuser', + 'password' => 'somepassword', + 'root' => 'someroot' + ); + + $this->assertTrue( + OC_Mount_Config::addMountPoint( + '/ext', + $class, + $mountConfig, + OC_Mount_Config::MOUNT_TYPE_USER, + self::TEST_USER1, + false, + $priority + ) + ); + + // Check for correct priority + $mountPoints = OC_Mount_Config::getAbsoluteMountPoints(self::TEST_USER1); + $this->assertEquals($priority, + $mountPoints['/'.self::TEST_USER1.'/files/ext']['priority']); + + // Simulate changed mount options (without priority set) + $this->assertTrue( + OC_Mount_Config::addMountPoint( + '/ext', + $class, + $mountConfig, + OC_Mount_Config::MOUNT_TYPE_USER, + self::TEST_USER1, + false + ) + ); + + // Check for correct priority + $mountPoints = OC_Mount_Config::getAbsoluteMountPoints(self::TEST_USER1); + $this->assertEquals($priority, + $mountPoints['/'.self::TEST_USER1.'/files/ext']['priority']); + } } From da03ef25d99bcc756c69923e347521340487f798 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Wed, 21 May 2014 22:29:16 +0100 Subject: [PATCH 8/9] Add priority overriding comments --- apps/files_external/lib/config.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index d47a2b4547..c69cac874c 100755 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -145,6 +145,7 @@ class OC_Mount_Config { $options['priority'] = $backends[$options['class']]['priority']; } + // Override if priority greater if ( (!isset($mountPoints[$mountPoint])) || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) ) { $options['priority_type'] = self::MOUNT_TYPE_GLOBAL; @@ -165,6 +166,7 @@ class OC_Mount_Config { $options['priority'] = $backends[$options['class']]['priority']; } + // Override if priority greater if ( (!isset($mountPoints[$mountPoint])) || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) ) { $options['priority_type'] = self::MOUNT_TYPE_GLOBAL; @@ -186,6 +188,7 @@ class OC_Mount_Config { $options['priority'] = $backends[$options['class']]['priority']; } + // Override if priority greater or if priority type different if ( (!isset($mountPoints[$mountPoint])) || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) || ($mountPoints[$mountPoint]['priority_type'] !== self::MOUNT_TYPE_GROUP) ) { @@ -210,6 +213,7 @@ class OC_Mount_Config { $options['priority'] = $backends[$options['class']]['priority']; } + // Override if priority greater or if priority type different if ( (!isset($mountPoints[$mountPoint])) || ($options['priority'] >= $mountPoints[$mountPoint]['priority']) || ($mountPoints[$mountPoint]['priority_type'] !== self::MOUNT_TYPE_USER) ) { From 856c48bc25ed3abb99763a1baa4a97efc41f1d10 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Wed, 21 May 2014 22:31:18 +0100 Subject: [PATCH 9/9] Use default priority of 100 if backend default is not set --- apps/files_external/lib/config.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index c69cac874c..21f63bf439 100755 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -479,8 +479,13 @@ class OC_Mount_Config { // Set default priority if none set if (!isset($mountPoints[$mountType][$applicable][$mountPoint]['priority'])) { - $mountPoints[$mountType][$applicable][$mountPoint]['priority'] - = $backends[$class]['priority']; + if (isset($backends[$class]['priority'])) { + $mountPoints[$mountType][$applicable][$mountPoint]['priority'] + = $backends[$class]['priority']; + } else { + $mountPoints[$mountType][$applicable][$mountPoint]['priority'] + = 100; + } } self::writeData($isPersonal ? OCP\User::getUser() : NULL, $mountPoints);