diff --git a/apps/files_external/lib/definitionparameter.php b/apps/files_external/lib/definitionparameter.php index 4b560908b6..7f883e5fad 100644 --- a/apps/files_external/lib/definitionparameter.php +++ b/apps/files_external/lib/definitionparameter.php @@ -154,22 +154,31 @@ class DefinitionParameter implements \JsonSerializable { /** * Validate a parameter value against this + * Convert type as necessary * * @param mixed $value Value to check * @return bool success */ - public function validateValue($value) { - if ($this->getFlags() & self::FLAG_OPTIONAL) { - return true; - } + public function validateValue(&$value) { + $optional = $this->getFlags() & self::FLAG_OPTIONAL; + switch ($this->getType()) { case self::VALUE_BOOLEAN: if (!is_bool($value)) { - return false; + switch ($value) { + case 'true': + $value = true; + break; + case 'false': + $value = false; + break; + default: + return false; + } } break; default: - if (empty($value)) { + if (!$value && !$optional) { return false; } break; diff --git a/apps/files_external/lib/frontenddefinitiontrait.php b/apps/files_external/lib/frontenddefinitiontrait.php index 4b826372d2..a5fb1a3f62 100644 --- a/apps/files_external/lib/frontenddefinitiontrait.php +++ b/apps/files_external/lib/frontenddefinitiontrait.php @@ -134,12 +134,12 @@ trait FrontendDefinitionTrait { * @return bool */ public function validateStorageDefinition(StorageConfig $storage) { - $options = $storage->getBackendOptions(); foreach ($this->getParameters() as $name => $parameter) { - $value = isset($options[$name]) ? $options[$name] : null; + $value = $storage->getBackendOption($name); if (!$parameter->validateValue($value)) { return false; } + $storage->setBackendOption($name, $value); } return true; } diff --git a/apps/files_external/lib/ftp.php b/apps/files_external/lib/ftp.php index aeca17c4f4..f3631e53fa 100644 --- a/apps/files_external/lib/ftp.php +++ b/apps/files_external/lib/ftp.php @@ -45,11 +45,7 @@ class FTP extends \OC\Files\Storage\StreamWrapper{ $this->user=$params['user']; $this->password=$params['password']; if (isset($params['secure'])) { - if (is_string($params['secure'])) { - $this->secure = ($params['secure'] === 'true'); - } else { - $this->secure = (bool)$params['secure']; - } + $this->secure = $params['secure']; } else { $this->secure = false; } diff --git a/apps/files_external/service/storagesservice.php b/apps/files_external/service/storagesservice.php index f655d99045..63e7162778 100644 --- a/apps/files_external/service/storagesservice.php +++ b/apps/files_external/service/storagesservice.php @@ -118,6 +118,8 @@ abstract class StoragesService { $applicableGroups[] = $applicable; $storageConfig->setApplicableGroups($applicableGroups); } + + return $storageConfig; } @@ -238,6 +240,12 @@ abstract class StoragesService { $this->setRealStorageIds($storages, $storagesWithConfigHash); } + // convert parameter values + foreach ($storages as $storage) { + $storage->getBackend()->validateStorageDefinition($storage); + $storage->getAuthMechanism()->validateStorageDefinition($storage); + } + return $storages; } diff --git a/apps/files_external/tests/definitionparameterttest.php b/apps/files_external/tests/definitionparameterttest.php index 6be0050869..22e0b84225 100644 --- a/apps/files_external/tests/definitionparameterttest.php +++ b/apps/files_external/tests/definitionparameterttest.php @@ -49,6 +49,9 @@ class DefinitionParameterTest extends \Test\TestCase { [Param::VALUE_BOOLEAN, Param::FLAG_NONE, false, true], [Param::VALUE_BOOLEAN, Param::FLAG_NONE, 123, false], + // conversion from string to boolean + [Param::VALUE_BOOLEAN, Param::FLAG_NONE, 'false', true, false], + [Param::VALUE_BOOLEAN, Param::FLAG_NONE, 'true', true, true], [Param::VALUE_PASSWORD, Param::FLAG_NONE, 'foobar', true], [Param::VALUE_PASSWORD, Param::FLAG_NONE, '', false], @@ -60,11 +63,14 @@ class DefinitionParameterTest extends \Test\TestCase { /** * @dataProvider validateValueProvider */ - public function testValidateValue($type, $flags, $value, $success) { + public function testValidateValue($type, $flags, $value, $success, $expectedValue = null) { $param = new Param('foo', 'bar'); $param->setType($type); $param->setFlags($flags); $this->assertEquals($success, $param->validateValue($value)); + if (isset($expectedValue)) { + $this->assertEquals($expectedValue, $value); + } } } diff --git a/apps/files_external/tests/frontenddefinitiontraittest.php b/apps/files_external/tests/frontenddefinitiontraittest.php index 871a87d4c5..d92d02b985 100644 --- a/apps/files_external/tests/frontenddefinitiontraittest.php +++ b/apps/files_external/tests/frontenddefinitiontraittest.php @@ -70,9 +70,11 @@ class FrontendDefinitionTraitTest extends \Test\TestCase { $storageConfig = $this->getMockBuilder('\OCA\Files_External\Lib\StorageConfig') ->disableOriginalConstructor() ->getMock(); - $storageConfig->expects($this->once()) - ->method('getBackendOptions') - ->willReturn([]); + $storageConfig->expects($this->any()) + ->method('getBackendOption') + ->willReturn(null); + $storageConfig->expects($this->any()) + ->method('setBackendOption'); $trait = $this->getMockForTrait('\OCA\Files_External\Lib\FrontendDefinitionTrait'); $trait->setText('test'); @@ -80,4 +82,35 @@ class FrontendDefinitionTraitTest extends \Test\TestCase { $this->assertEquals($expectedSuccess, $trait->validateStorageDefinition($storageConfig)); } + + public function testValidateStorageSet() { + $param = $this->getMockBuilder('\OCA\Files_External\Lib\DefinitionParameter') + ->disableOriginalConstructor() + ->getMock(); + $param->method('getName') + ->willReturn('param'); + $param->expects($this->once()) + ->method('validateValue') + ->will($this->returnCallback(function(&$value) { + $value = 'foobar'; + return true; + })); + + $storageConfig = $this->getMockBuilder('\OCA\Files_External\Lib\StorageConfig') + ->disableOriginalConstructor() + ->getMock(); + $storageConfig->expects($this->once()) + ->method('getBackendOption') + ->with('param') + ->willReturn('barfoo'); + $storageConfig->expects($this->once()) + ->method('setBackendOption') + ->with('param', 'foobar'); + + $trait = $this->getMockForTrait('\OCA\Files_External\Lib\FrontendDefinitionTrait'); + $trait->setText('test'); + $trait->addParameter($param); + + $this->assertEquals(true, $trait->validateStorageDefinition($storageConfig)); + } } diff --git a/apps/files_external/tests/service/globalstoragesservicetest.php b/apps/files_external/tests/service/globalstoragesservicetest.php index 05585d2c06..2bc480ca31 100644 --- a/apps/files_external/tests/service/globalstoragesservicetest.php +++ b/apps/files_external/tests/service/globalstoragesservicetest.php @@ -789,6 +789,13 @@ class GlobalStoragesServiceTest extends StoragesServiceTest { file_put_contents($configFile, json_encode($json)); + $this->backendService->getBackend('identifier:\OCA\Files_External\Lib\Backend\SMB') + ->expects($this->exactly(4)) + ->method('validateStorageDefinition'); + $this->backendService->getAuthMechanism('identifier:\Auth\Mechanism') + ->expects($this->exactly(4)) + ->method('validateStorageDefinition'); + $allStorages = $this->service->getAllStorages(); $this->assertCount(4, $allStorages);