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.

1 files changed, 183 insertions, 18 deletions

diff --git a/apps/files_external/tests/mountconfig.php b/apps/files_external/tests/mountconfig.php
index bf43bb31c38..c89874c94d5 100644
--- a/apps/files_external/tests/mountconfig.php
+++ b/apps/files_external/tests/mountconfig.php
@@ -212,13 +212,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',
@@ -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']);
+	}
 }