owncloud · phil-davis · Jun 11, 2020 · Jun 10, 2020
diff --git a/apps/files_external/lib/Command/Export.php b/apps/files_external/lib/Command/Export.php
@@ -52,6 +52,7 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 		$listInput->setOption('output', 'json_pretty');
 		$listInput->setOption('show-password', true);
 		$listInput->setOption('full', true);
+		$listInput->setOption('importable-format', true);
 		$listCommand->execute($listInput, $output);
 	}
 }
diff --git a/apps/files_external/lib/Command/ListCommand.php b/apps/files_external/lib/Command/ListCommand.php
@@ -101,6 +101,11 @@ protected function configure() {
 				's',
 				InputOption::VALUE_NONE,
 				'Show only a reduced mount info'
+			)->addOption(
+				'importable-format',
+				'i',
+				InputOption::VALUE_NONE,
+				'Provide output values in a format compatible with files_external:import'
 			);
 		parent::configure();
 	}
@@ -129,7 +134,9 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 	 */
 	public function listMounts($userId, array $mounts, InputInterface $input, OutputInterface $output) {
 		$outputType = $input->getOption('output');
+		$outputIsJson = ($outputType === self::OUTPUT_FORMAT_JSON) || ($outputType === self::OUTPUT_FORMAT_JSON_PRETTY);
 		$shortView = $input->getOption('short');
+		$importableView = $input->getOption('importable-format');
 
 		// check if there are any mounts present
 		if (\count($mounts) === 0) {
@@ -191,7 +198,7 @@ public function listMounts($userId, array $mounts, InputInterface $input, Output
 		$countInvalid = 0;
 		// In case adding array elements, add them only after the first two (Mount ID / Mount Point)
 		// and before the last one entry (Type). Necessary for option -s
-		$rows = \array_map(function (IStorageConfig $config) use ($shortView, $userId, $defaultMountOptions, $full, $showMountOptions, &$countInvalid) {
+		$rows = \array_map(function (IStorageConfig $config) use ($outputIsJson, $shortView, $importableView, $userId, $defaultMountOptions, $full, $showMountOptions, &$countInvalid) {
 			if ($config->getBackend() instanceof InvalidBackend || $config->getAuthMechanism() instanceof InvalidAuth) {
 				$countInvalid++;
 			}
@@ -239,22 +246,38 @@ public function listMounts($userId, array $mounts, InputInterface $input, Output
 					$config->getMountPoint()
 				];
 			} else {
+				$configValue = $configString;
+				if ($importableView) {
+					$storageValue = $config->getBackend()->getStorageClass();
+					$authValue = $config->getAuthMechanism()->getIdentifier();
+					if ($outputIsJson) {
+						$configValue = $storageConfig;
+					}
+				} else {
+					$storageValue = $config->getBackend()->getText();
+					$authValue = $config->getAuthMechanism()->getText();
+				}
 				$values = [
 					$config->getId(),
 					$config->getMountPoint(),
-					$config->getBackend()->getText(),
-					$config->getAuthMechanism()->getText(),
-					$configString,
+					$storageValue,
+					$authValue,
+					$configValue,
 					$optionsString
 				];
 			}
 
-			// output independent on option shortview
+			// output independent of option shortview
 			if (!$userId || $userId === self::ALL) {
-				$applicableUsers = \implode(', ', $config->getApplicableUsers());
-				$applicableGroups = \implode(', ', $config->getApplicableGroups());
-				if ($applicableUsers === '' && $applicableGroups === '') {
-					$applicableUsers = 'All';
+				if ($importableView && $outputIsJson) {
+					$applicableUsers = $config->getApplicableUsers();
+					$applicableGroups = $config->getApplicableGroups();
+				} else {
+					$applicableUsers = \implode(', ', $config->getApplicableUsers());
+					$applicableGroups = \implode(', ', $config->getApplicableGroups());
+					if ($applicableUsers === '' && $applicableGroups === '') {
+						$applicableUsers = 'All';
+					}
 				}
 				$values[] = $applicableUsers;
 				$values[] = $applicableGroups;
@@ -274,7 +297,7 @@ public function listMounts($userId, array $mounts, InputInterface $input, Output
 			return $values;
 		}, $mounts);
 
-		if ($outputType === self::OUTPUT_FORMAT_JSON || $outputType === self::OUTPUT_FORMAT_JSON_PRETTY) {
+		if ($outputIsJson) {
 			$keys = \array_map(function ($header) {
 				return \strtolower(\str_replace(' ', '_', $header));
 			}, $headers);

diff --git a/apps/files_external/tests/Command/ListCommandTest.php b/apps/files_external/tests/Command/ListCommandTest.php
@@ -228,6 +228,79 @@ public function testLongView($options, $expectedResult, $mountOptions) {
 		$input = $this->getInput($instance, ['user_id' => 'user1'], $options);
 		$output = new BufferedOutput();
 
+		$instance->listMounts('user1', [$mount1, $mount2], $input, $output);
+		$output = $output->fetch();
+		if (isset($options['output']) && ($options['output'] === 'json')) {
+			$results = \json_decode($output, true);
+			$countResults = \count($results);
+
+			for ($i = 0; $i < $countResults; $i++) {
+				$this->assertEquals($expectedResult[$i]['mount_id'], $results[$i]['mount_id']);
+				$this->assertEquals($expectedResult[$i]['mount_point'], $results[$i]['mount_point']);
+				$this->assertEquals($expectedResult[$i]['authentication_type'], $results[$i]['authentication_type']);
+			}
+		} else {
+			$this->assertEquals($expectedResult, $output);
+		}
+	}
+	public function providesImportableFormat() {
+		return [
+			[
+				['importable-format' => true, 'short' => false, 'output' => 'json'],
+				[
+					['mount_id' => 1, 'mount_point' => '/ownCloud', 'storage' => 'Local', 'authentication_type' => 'password::password'],
+					['mount_id' => 2, 'mount_point' => '/SFTP', 'storage' => 'Local', 'authentication_type' => 'password::sessioncredentials'],
+				],
+				[
+					['mount_id' => 1, 'mount_point' => '/ownCloud'],
+					['mount_id' => 2, 'mount_point' => '/SFTP'],
+				]
+			],
+			[
+				['importable-format' => true, 'short' => false],
+				<<<EOS
++----------+-------------+-------------------------+------------------------------+---------------+---------+
+| Mount ID | Mount Point | Storage                 | Authentication Type          | Configuration | Options |
++----------+-------------+-------------------------+------------------------------+---------------+---------+
+| 1        | /ownCloud   | \OC\Files\Storage\Local | password::password           |               |         |
+| 2        | /SFTP       | \OC\Files\Storage\Local | password::sessioncredentials |               |         |
++----------+-------------+-------------------------+------------------------------+---------------+---------+
+
+EOS
+				,
+				[
+					['mount_id' => 1, 'mount_point' => '/ownCloud', 'storage' => 'Local'],
+					['mount_id' => 2, 'mount_point' => '/SFTP', 'storage' => 'Local'],
+				]
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider providesImportableFormat
+	 * @param $options
+	 * @param $expectedResult
+	 * @param $mountOptions
+	 */
+	public function testImportableFormat($options, $expectedResult, $mountOptions) {
+		$l10n = $this->createMock('\OCP\IL10N', null, [], '', false);
+		$session = $this->createMock('\OCP\ISession');
+		$crypto = $this->createMock('\OCP\Security\ICrypto');
+		$instance = $this->getInstance();
+		// FIXME: use mock of IStorageConfig
+		$mount1 = new StorageConfig();
+		$mount1->setId($mountOptions[0]['mount_id']);
+		$mount1->setMountPoint($mountOptions[0]['mount_point']);
+		$mount1->setAuthMechanism(new Password());
+		$mount1->setBackend(new Local($l10n, new NullMechanism()));
+		$mount2 = new StorageConfig();
+		$mount2->setId($mountOptions[1]['mount_id']);
+		$mount2->setMountPoint($mountOptions[1]['mount_point']);
+		$mount2->setAuthMechanism(new SessionCredentials($session, $crypto));
+		$mount2->setBackend(new Local($l10n, new NullMechanism()));
+		$input = $this->getInput($instance, ['user_id' => 'user1'], $options);
+		$output = new BufferedOutput();
+
 		$instance->listMounts('user1', [$mount1, $mount2], $input, $output);
 		$output = $output->fetch();
 		if (isset($options['output']) && ($options['output'] === 'json')) {

diff --git a/changelog/unreleased/37054 b/changelog/unreleased/37054
@@ -0,0 +1,7 @@
+Bugfix: Correct files_external:export output so it can be imported
+
+The output of files_external:export was not suitable to be used as input to
+files_external:import. This has been corrected.
+
+https://github.com/owncloud/core/issues/37054
+https://github.com/owncloud/core/pull/37513
diff --git a/tests/acceptance/features/bootstrap/OccContext.php b/tests/acceptance/features/bootstrap/OccContext.php
@@ -1720,12 +1720,29 @@ public function theFollowingLocalStorageShouldBeListed(TableNode $table) {
 						);
 					}
 					if (isset($expectedStorageEntry['Configuration'])) {
-						Assert::assertStringStartsWith(
-							$expectedStorageEntry['Configuration'],
-							$listedStorageEntry->configuration,
-							"Configuration column does not start with the expected value for storage "
-							. $expectedStorageEntry['MountPoint']
-						);
+						if ($expectedStorageEntry['Configuration'] === '') {
+							Assert::assertEquals(
+								'',
+								$listedStorageEntry->configuration,
+								'Configuration column should be empty but is '
+								. $listedStorageEntry->configuration
+							);
+						} else {
+							if (\is_string($listedStorageEntry->configuration)) {
+								Assert::assertStringStartsWith(
+									$expectedStorageEntry['Configuration'],
+									$listedStorageEntry->configuration,
+									"Configuration column does not start with the expected value for storage "
+									. $expectedStorageEntry['Configuration']
+								);
+							} else {
+								$item = \strtok($expectedStorageEntry['Configuration'], ':');
+								Assert::assertTrue(
+									\property_exists($listedStorageEntry->configuration, $item),
+									"$item was not found in the Configuration column"
+								);
+							}
+						}
 					}
 					if (isset($expectedStorageEntry['Options'])) {
 						Assert::assertEquals(
@@ -1736,32 +1753,65 @@ public function theFollowingLocalStorageShouldBeListed(TableNode $table) {
 						);
 					}
 					if (isset($expectedStorageEntry['ApplicableUsers'])) {
-						$listedApplicableUsers = \explode(', ', $listedStorageEntry->applicable_users);
-						$expectedApplicableUsers = \explode(', ', $expectedStorageEntry['ApplicableUsers']);
-						foreach ($expectedApplicableUsers as $expectedApplicableUserEntry) {
-							$expectedApplicableUserEntry = $this->featureContext->getActualUsername($expectedApplicableUserEntry);
-							Assert::assertContains(
-								$expectedApplicableUserEntry,
+						if (\is_string($listedStorageEntry->applicable_users)) {
+							if ($listedStorageEntry->applicable_users === '') {
+								$listedApplicableUsers = [];
+							} else {
+								$listedApplicableUsers = \explode(', ', $listedStorageEntry->applicable_users);
+							}
+						} else {
+							$listedApplicableUsers = $listedStorageEntry->applicable_users;
+						}
+						if ($expectedStorageEntry['ApplicableUsers'] === '') {
+							Assert::assertEquals(
+								[],
 								$listedApplicableUsers,
-								__METHOD__
-								. " '$expectedApplicableUserEntry' is not listed in '"
-								. \implode(', ', $listedApplicableUsers)
-								. "'"
+								"ApplicableUsers was expected to be an empty array but was not empty"
 							);
+						} else {
+							$expectedApplicableUsers = \explode(', ', $expectedStorageEntry['ApplicableUsers']);
+							foreach ($expectedApplicableUsers as $expectedApplicableUserEntry) {
+								$expectedApplicableUserEntry = $this->featureContext->getActualUsername($expectedApplicableUserEntry);
+								Assert::assertContains(
+									$expectedApplicableUserEntry,
+									$listedApplicableUsers,
+									__METHOD__
+									. " '$expectedApplicableUserEntry' is not listed in '"
+									. \implode(', ', $listedApplicableUsers)
+									. "'"
+								);
+							}
 						}
 					}
 					if (isset($expectedStorageEntry['ApplicableGroups'])) {
-						$listedApplicableGroups = \explode(', ', $listedStorageEntry->applicable_groups);
-						$expectedApplicableGroups = \explode(', ', $expectedStorageEntry['ApplicableGroups']);
-						foreach ($expectedApplicableGroups as $expectedApplicableGroupEntry) {
-							Assert::assertContains(
-								$expectedApplicableGroupEntry,
+						if (\is_string($listedStorageEntry->applicable_groups)) {
+							if ($listedStorageEntry->applicable_groups === '') {
+								$listedApplicableGroups = [];
+							} else {
+								$listedApplicableGroups = \explode(', ', $listedStorageEntry->applicable_groups);
+							}
+						} else {
+							$listedApplicableGroups = $listedStorageEntry->applicable_groups;
+						}
+						if ($expectedStorageEntry['ApplicableGroups'] === '') {
+							Assert::assertEquals(
+								[],
 								$listedApplicableGroups,
-								__METHOD__
-								. " '$expectedApplicableGroupEntry' is not listed in '"
-								. \implode(', ', $listedApplicableGroups)
-								. "'"
+								"ApplicableGroups was expected to be an empty array but was not empty"
 							);
+							Assert::assertEquals([], $listedApplicableGroups);
+						} else {
+							$expectedApplicableGroups = \explode(', ', $expectedStorageEntry['ApplicableGroups']);
+							foreach ($expectedApplicableGroups as $expectedApplicableGroupEntry) {
+								Assert::assertContains(
+									$expectedApplicableGroupEntry,
+									$listedApplicableGroups,
+									__METHOD__
+									. " '$expectedApplicableGroupEntry' is not listed in '"
+									. \implode(', ', $listedApplicableGroups)
+									. "'"
+								);
+							}
 						}
 					}
 					if (isset($expectedStorageEntry['Type'])) {
@@ -1965,37 +2015,35 @@ public function theFollowingShouldNotBeIncludedInTheOptionsOfLocalStorage($local
 	 *
 	 * @param string $folder
 	 *
-	 * @return integer
+	 * @return integer|boolean
 	 * @throws Exception
 	 */
 	public function administratorDeletesFolder($folder) {
-		$createdLocalStorage = [];
-		$this->listLocalStorageMount();
-		$commandOutput = \json_decode($this->featureContext->getStdOutOfOccCommand());
-		foreach ($commandOutput as $i) {
-			$createdLocalStorage[$i->mount_id] = \ltrim($i->mount_point, '/');
-		}
-		foreach ($createdLocalStorage as $key => $value) {
-			if ($value === $folder) {
-				$mount_id = $key;
-			}
-		}
-		if (!isset($mount_id)) {
-			throw  new Exception("Id not found for folder to be deleted");
-		}
-		$this->invokingTheCommand('files_external:delete --yes ' . $mount_id);
-		return (int) $mount_id;
+		return $this->deleteLocalStorageFolderUsingTheOccCommand($folder);
 	}
 
 	/**
-	 * @When the administrator has deleted local storage :folder using the occ command
+	 * @Given the administrator has deleted local storage :folder using the occ command
+	 *
+	 * @param string $folder
 	 *
+	 * @return integer|boolean
+	 * @throws Exception
+	 */
+	public function administratorHasDeletedLocalStorageFolderUsingTheOccCommand($folder) {
+		$mount_id = $this->deleteLocalStorageFolderUsingTheOccCommand($folder);
+		$this->theCommandShouldHaveBeenSuccessful();
+		return $mount_id;
+	}
+
+	/**
 	 * @param string $folder
+	 * @param bool $mustExist
 	 *
-	 * @return integer
+	 * @return integer|boolean
 	 * @throws Exception
 	 */
-	public function administratorHasDeletedFolder($folder) {
+	public function deleteLocalStorageFolderUsingTheOccCommand($folder, $mustExist = true) {
 		$createdLocalStorage = [];
 		$this->listLocalStorageMount();
 		$commandOutput = \json_decode($this->featureContext->getStdOutOfOccCommand());
@@ -2008,10 +2056,12 @@ public function administratorHasDeletedFolder($folder) {
 			}
 		}
 		if (!isset($mount_id)) {
-			throw  new Exception("Id not found for folder to be deleted");
+			if ($mustExist) {
+				throw  new Exception("Id not found for folder to be deleted");
+			}
+			return false;
 		}
 		$this->invokingTheCommand('files_external:delete --yes ' . $mount_id);
-		$this->theCommandShouldHaveBeenSuccessful();
 		return (int) $mount_id;
 	}
 
@@ -2822,6 +2872,24 @@ public function resetDAVTechPreview() {
 		}
 	}
 
+	/**
+	 * This will run after EVERY scenario.
+	 * Some local_storage tests import storage from an export file. In that case
+	 * we have not explicitly created the storage, and so we do not explicitly
+	 * know to delete it. So delete the local storage that is known to be used
+	 * in tests.
+	 *
+	 * @AfterScenario @local_storage
+	 *
+	 * @return void
+	 * @throws Exception
+	 */
+	public function removeLocalStorageIfExists() {
+		$this->deleteLocalStorageFolderUsingTheOccCommand('local_storage', false);
+		$this->deleteLocalStorageFolderUsingTheOccCommand('local_storage2', false);
+		$this->deleteLocalStorageFolderUsingTheOccCommand('local_storage3', false);
+	}
+
 	/**
 	 * This will run before EVERY scenario.
 	 * It will set the properties for this object.