Merge pull request #8163 from Xenopathic/files_external_priority

Priority on external storage mounts

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'),

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;
@@ -126,6 +127,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);
@@ -133,12 +136,45 @@ 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']);
-				$mountPoints[$mountPoint] = $options;
+				if (!isset($options['priority'])) {
+					$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;
+					$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) {
+				$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'];
+				}
+
+				// Override if priority greater
+				if ( (!isset($mountPoints[$mountPoint]))
+					|| ($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)) {
@@ -148,21 +184,42 @@ 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'];
+						}
+
+						// 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) ) {
+							$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 ($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) {
 							$option = self::setUserVars($user, $option);
 						}
 						$options['options'] = self::decryptPasswords($options['options']);
-						$mountPoints[$mountPoint] = $options;
+						if (!isset($options['priority'])) {
+							$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) ) {
+							$options['priority_type'] = self::MOUNT_TYPE_USER;
+							$mountPoints[$mountPoint] = $options;
+						}
 					}
 				}
 			}
@@ -173,6 +230,9 @@ 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']);
+
+				// Always override previous config
+				$options['priority_type'] = self::MOUNT_TYPE_PERSONAL;
 				$mountPoints[$mountPoint] = $options;
 			}
 		}
@@ -244,6 +304,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 +314,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 +339,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)
@@ -363,6 +431,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,
@@ -370,7 +439,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 === '/') {
@@ -400,9 +470,24 @@ class OC_Mount_Config {
 				'options' => self::encryptPasswords($classOptions))
 			)
 		);
+		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'])) {
+			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);
 
 		return self::getBackendStatus($class, $classOptions, $isPersonal);
@@ -690,8 +775,16 @@ 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])) {
+				// 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 {

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,161 @@ 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']);
+	}
+
+	/**
+	 * 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']);
+	}
 }