Remove magic flags from files_external backend params

4 files changed, 83 insertions, 47 deletions

diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js
index 233215a4aa1..26df203091e 100644
--- a/apps/files_external/js/settings.js
+++ b/apps/files_external/js/settings.js
@@ -587,6 +587,19 @@ MountOptionsDropdown.prototype = {
 var MountConfigListView = function($el, options) {
 	this.initialize($el, options);
 };
+
+MountConfigListView.ParameterFlags = {
+	OPTIONAL: 1,
+	USER_PROVIDED: 2
+};
+
+MountConfigListView.ParameterTypes = {
+	TEXT: 0,
+	BOOLEAN: 1,
+	PASSWORD: 2,
+	HIDDEN: 3
+};
+
 /**
  * @memberOf OCA.External.Settings
  */
@@ -961,16 +974,15 @@ MountConfigListView.prototype = _.extend({
 	 */
 	writeParameterInput: function($td, parameter, placeholder, classes) {
 		var hasFlag = function(flag) {
-			return placeholder.indexOf(flag) !== -1;
+			return (placeholder.flags & flag) === flag;
 		};
 		classes = $.isArray(classes) ? classes : [];
 		classes.push('added');
-		if (placeholder.indexOf('&') === 0) {
+		if (hasFlag(MountConfigListView.ParameterFlags.OPTIONAL)) {
 			classes.push('optional');
-			placeholder = placeholder.substring(1);
 		}
 
-		if (hasFlag('@')) {
+		if (hasFlag(MountConfigListView.ParameterFlags.USER_PROVIDED)) {
 			if (this._isPersonal) {
 				classes.push('user_provided');
 			} else {
@@ -980,17 +992,13 @@ MountConfigListView.prototype = _.extend({
 
 		var newElement;
 
-		var trimmedPlaceholder = placeholder;
-		var flags = ['@', '*', '!', '#', '&']; // used to determine what kind of parameter
-		while(flags.indexOf(trimmedPlaceholder[0]) !== -1) {
-			trimmedPlaceholder = trimmedPlaceholder.substr(1);
-		}
-		if (hasFlag('*')) {
+		var trimmedPlaceholder = placeholder.value;
+		if (placeholder.type === MountConfigListView.ParameterTypes.PASSWORD) {
 			newElement = $('<input type="password" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" placeholder="'+ trimmedPlaceholder+'" />');
-		} else if (hasFlag('!')) {
+		} else if (placeholder.type === MountConfigListView.ParameterTypes.BOOLEAN) {
 			var checkboxId = _.uniqueId('checkbox_');
 			newElement = $('<input type="checkbox" id="'+checkboxId+'" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" /><label for="'+checkboxId+'">'+ trimmedPlaceholder+'</label>');
-		} else if (hasFlag('#')) {
+		} else if (placeholder.type === MountConfigListView.ParameterTypes.HIDDEN) {
 			newElement = $('<input type="hidden" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" />');
 		} else {
 			newElement = $('<input type="text" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" placeholder="'+ trimmedPlaceholder+'" />');
diff --git a/apps/files_external/lib/definitionparameter.php b/apps/files_external/lib/definitionparameter.php
index dc7985837f5..27c6af0fcda 100644
--- a/apps/files_external/lib/definitionparameter.php
+++ b/apps/files_external/lib/definitionparameter.php
@@ -131,27 +131,11 @@ class DefinitionParameter implements \JsonSerializable {
 	 * @return string
 	 */
 	public function jsonSerialize() {
-		$prefix = '';
-		switch ($this->getType()) {
-			case self::VALUE_BOOLEAN:
-				$prefix = '!';
-				break;
-			case self::VALUE_PASSWORD:
-				$prefix = '*';
-				break;
-			case self::VALUE_HIDDEN:
-				$prefix = '#';
-				break;
-		}
-
-		if ($this->isFlagSet(self::FLAG_OPTIONAL)) {
-			$prefix = '&' . $prefix;
-		}
-		if ($this->isFlagSet(self::FLAG_USER_PROVIDED)) {
-			$prefix = '@' . $prefix;
-		}
-
-		return $prefix . $this->getText();
+		return [
+			'value' => $this->getText(),
+			'flags' => $this->getFlags(),
+			'type' => $this->getType()
+		];
 	}
 
 	public function isOptional() {
diff --git a/apps/files_external/tests/definitionparameterttest.php b/apps/files_external/tests/definitionparameterttest.php
index dc7c150ec96..e89058d5c78 100644
--- a/apps/files_external/tests/definitionparameterttest.php
+++ b/apps/files_external/tests/definitionparameterttest.php
@@ -27,18 +27,34 @@ class DefinitionParameterTest extends \Test\TestCase {
 
 	public function testJsonSerialization() {
 		$param = new Param('foo', 'bar');
-		$this->assertEquals('bar', $param->jsonSerialize());
+		$this->assertEquals([
+			'value' => 'bar',
+			'flags' => 0,
+			'type' => 0
+		], $param->jsonSerialize());
 
 		$param->setType(Param::VALUE_BOOLEAN);
-		$this->assertEquals('!bar', $param->jsonSerialize());
+		$this->assertEquals([
+			'value' => 'bar',
+			'flags' => 0,
+			'type' => Param::VALUE_BOOLEAN
+		], $param->jsonSerialize());
 
 		$param->setType(Param::VALUE_PASSWORD);
 		$param->setFlag(Param::FLAG_OPTIONAL);
-		$this->assertEquals('&*bar', $param->jsonSerialize());
+		$this->assertEquals([
+			'value' => 'bar',
+			'flags' => Param::FLAG_OPTIONAL,
+			'type' => Param::VALUE_PASSWORD
+		], $param->jsonSerialize());
 
 		$param->setType(Param::VALUE_HIDDEN);
 		$param->setFlags(Param::FLAG_NONE);
-		$this->assertEquals('#bar', $param->jsonSerialize());
+		$this->assertEquals([
+			'value' => 'bar',
+			'flags' => Param::FLAG_NONE,
+			'type' => Param::VALUE_HIDDEN
+		], $param->jsonSerialize());
 	}
 
 	public function validateValueProvider() {
diff --git a/apps/files_external/tests/js/settingsSpec.js b/apps/files_external/tests/js/settingsSpec.js
index 72b186edf06..6f5bb2a8e3e 100644
--- a/apps/files_external/tests/js/settingsSpec.js
+++ b/apps/files_external/tests/js/settingsSpec.js
@@ -58,8 +58,13 @@ describe('OCA.External.Settings tests', function() {
 					'identifier': '\\OC\\TestBackend',
 					'name': 'Test Backend',
 					'configuration': {
-						'field1': 'Display Name 1',
-						'field2': '&Display Name 2'
+						'field1': {
+							'value': 'Display Name 1'
+						},
+						'field2': {
+							'value': 'Display Name 2',
+							'flags': 1
+						}
 					},
 					'authSchemes': {
 						'builtin': true,
@@ -70,8 +75,13 @@ describe('OCA.External.Settings tests', function() {
 					'identifier': '\\OC\\AnotherTestBackend',
 					'name': 'Another Test Backend',
 					'configuration': {
-						'field1': 'Display Name 1',
-						'field2': '&Display Name 2'
+						'field1': {
+							'value': 'Display Name 1'
+						},
+						'field2': {
+							'value': 'Display Name 2',
+							'flags': 1
+						}
 					},
 					'authSchemes': {
 						'builtin': true,
@@ -82,12 +92,30 @@ describe('OCA.External.Settings tests', function() {
 					'identifier': '\\OC\\InputsTestBackend',
 					'name': 'Inputs test backend',
 					'configuration': {
-						'field_text': 'Text field',
-						'field_password': '*Password field',
-						'field_bool': '!Boolean field',
-						'field_hidden': '#Hidden field',
-						'field_text_optional': '&Text field optional',
-						'field_password_optional': '&*Password field optional'
+						'field_text': {
+							'value': 'Text field'
+						},
+						'field_password': {
+							'value': ',Password field',
+							'type': 2
+						},
+						'field_bool': {
+							'value': 'Boolean field',
+							'type': 1
+						},
+						'field_hidden': {
+							'value': 'Hidden field',
+							'type': 3
+						},
+						'field_text_optional': {
+							'value': 'Text field optional',
+							'flags': 1
+						},
+						'field_password_optional': {
+							'value': 'Password field optional',
+							'flags': 1,
+							'type': 2
+						}
 					},
 					'authSchemes': {
 						'builtin': true,