From 62c03beb1d2dc133542c088e6c72b201211ccfba Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Mon, 10 Sep 2018 22:33:35 +0200 Subject: Extract logic for webroot into method and add test Signed-off-by: Daniel Kesselberg --- tests/lib/SetupTest.php | 44 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 9 deletions(-) (limited to 'tests') diff --git a/tests/lib/SetupTest.php b/tests/lib/SetupTest.php index e6e9fb5c56c..fe3b1b54d23 100644 --- a/tests/lib/SetupTest.php +++ b/tests/lib/SetupTest.php @@ -10,6 +10,7 @@ namespace Test; use bantu\IniGetWrapper\IniGetWrapper; use OC\Installer; +use OC\Setup; use OC\SystemConfig; use OCP\Defaults; use OCP\IL10N; @@ -45,7 +46,7 @@ class SetupTest extends \Test\TestCase { $this->logger = $this->createMock(ILogger::class); $this->random = $this->createMock(ISecureRandom::class); $this->installer = $this->createMock(Installer::class); - $this->setupClass = $this->getMockBuilder('\OC\Setup') + $this->setupClass = $this->getMockBuilder(Setup::class) ->setMethods(['class_exists', 'is_callable', 'getAvailableDbDriversForPdo']) ->setConstructorArgs([$this->config, $this->iniWrapper, $this->l10n, $this->defaults, $this->logger, $this->random, $this->installer]) ->getMock(); @@ -56,7 +57,7 @@ class SetupTest extends \Test\TestCase { ->expects($this->once()) ->method('getValue') ->will($this->returnValue( - array('sqlite', 'mysql', 'oci') + ['sqlite', 'mysql', 'oci'] )); $this->setupClass ->expects($this->once()) @@ -67,9 +68,9 @@ class SetupTest extends \Test\TestCase { ->method('getAvailableDbDriversForPdo') ->will($this->returnValue(['sqlite'])); $result = $this->setupClass->getSupportedDatabases(); - $expectedResult = array( + $expectedResult = [ 'sqlite' => 'SQLite' - ); + ]; $this->assertSame($expectedResult, $result); } @@ -79,7 +80,7 @@ class SetupTest extends \Test\TestCase { ->expects($this->once()) ->method('getValue') ->will($this->returnValue( - array('sqlite', 'mysql', 'oci', 'pgsql') + ['sqlite', 'mysql', 'oci', 'pgsql'] )); $this->setupClass ->expects($this->any()) @@ -91,7 +92,7 @@ class SetupTest extends \Test\TestCase { ->will($this->returnValue([])); $result = $this->setupClass->getSupportedDatabases(); - $this->assertSame(array(), $result); + $this->assertSame([], $result); } public function testGetSupportedDatabasesWithAllWorking() { @@ -99,7 +100,7 @@ class SetupTest extends \Test\TestCase { ->expects($this->once()) ->method('getValue') ->will($this->returnValue( - array('sqlite', 'mysql', 'pgsql', 'oci') + ['sqlite', 'mysql', 'pgsql', 'oci'] )); $this->setupClass ->expects($this->any()) @@ -110,12 +111,12 @@ class SetupTest extends \Test\TestCase { ->method('getAvailableDbDriversForPdo') ->will($this->returnValue(['sqlite', 'mysql', 'pgsql'])); $result = $this->setupClass->getSupportedDatabases(); - $expectedResult = array( + $expectedResult = [ 'sqlite' => 'SQLite', 'mysql' => 'MySQL/MariaDB', 'pgsql' => 'PostgreSQL', 'oci' => 'Oracle' - ); + ]; $this->assertSame($expectedResult, $result); } @@ -130,4 +131,29 @@ class SetupTest extends \Test\TestCase { ->will($this->returnValue('NotAnArray')); $this->setupClass->getSupportedDatabases(); } + + /** + * @dataProvider findWebRootProvider + */ + public function testFindWebRootCli($url, $webRoot) { + $this->config + ->expects($this->once()) + ->method('getValue') + ->will($this->returnValue($url)); + \OC::$CLI = true; + + $this->assertEquals( + $webRoot, + $this->setupClass::findWebRoot($this->config) + ); + } + + public function findWebRootProvider(): array { + return [ + 'https://www.example.com/nextcloud' => ['https://www.example.com/nextcloud', '/nextcloud'], + 'https://www.example.com/' => ['https://www.example.com/', ''], + 'https://www.example.com' => ['https://www.example.com', false], + 'empty' => ['', false], + ]; + } } -- cgit v1.2.3 From 603a578a1c9baab2c9eda08d5a316ed25a0163bf Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Mon, 10 Sep 2018 22:45:40 +0200 Subject: Change return false to throw new Signed-off-by: Daniel Kesselberg --- lib/private/Setup.php | 17 ++++++++++++----- tests/lib/SetupTest.php | 15 ++++++++++----- 2 files changed, 22 insertions(+), 10 deletions(-) (limited to 'tests') diff --git a/lib/private/Setup.php b/lib/private/Setup.php index a31d746a062..4b6f2d54583 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -43,6 +43,7 @@ namespace OC; use bantu\IniGetWrapper\IniGetWrapper; use Exception; +use InvalidArgumentException; use OC\App\AppStore\Bundles\BundleFetcher; use OC\Authentication\Token\DefaultTokenCleanupJob; use OC\Authentication\Token\DefaultTokenProvider; @@ -434,18 +435,19 @@ class Setup { * Find webroot from config * * @param SystemConfig $config - * @return bool|string + * @return string + * @throws InvalidArgumentException when invalid value for overwrite.cli.url */ - public static function findWebRoot(SystemConfig $config) { + public static function findWebRoot(SystemConfig $config): string { // For CLI read the value from overwrite.cli.url if (\OC::$CLI) { $webRoot = $config->getValue('overwrite.cli.url', ''); if ($webRoot === '') { - return false; + throw new InvalidArgumentException('overwrite.cli.url is empty'); } $webRoot = parse_url($webRoot, PHP_URL_PATH); if ($webRoot === null) { - return false; + throw new InvalidArgumentException('invalid value for overwrite.cli.url'); } $webRoot = rtrim($webRoot, '/'); } else { @@ -463,7 +465,12 @@ class Setup { */ public static function updateHtaccess() { $config = \OC::$server->getSystemConfig(); - $webRoot = self::findWebRoot($config); + + try { + $webRoot = self::findWebRoot($config); + } catch (InvalidArgumentException $e) { + return false; + } $setupHelper = new \OC\Setup( $config, diff --git a/tests/lib/SetupTest.php b/tests/lib/SetupTest.php index fe3b1b54d23..40481de8f1d 100644 --- a/tests/lib/SetupTest.php +++ b/tests/lib/SetupTest.php @@ -134,18 +134,23 @@ class SetupTest extends \Test\TestCase { /** * @dataProvider findWebRootProvider + * @param $url + * @param $expected */ - public function testFindWebRootCli($url, $webRoot) { + public function testFindWebRootCli($url, $expected) { $this->config ->expects($this->once()) ->method('getValue') ->will($this->returnValue($url)); \OC::$CLI = true; - $this->assertEquals( - $webRoot, - $this->setupClass::findWebRoot($this->config) - ); + try { + $webRoot = $this->setupClass::findWebRoot($this->config); + } catch (\InvalidArgumentException $e) { + $webRoot = false; + } + + $this->assertEquals($webRoot, $expected); } public function findWebRootProvider(): array { -- cgit v1.2.3 From fabd3e7ba1a7ff903ccba77c9044b802d073b6ad Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Tue, 11 Sep 2018 17:22:10 +0200 Subject: Restore previous state for OC::$CLI Signed-off-by: Daniel Kesselberg --- tests/lib/SetupTest.php | 3 +++ 1 file changed, 3 insertions(+) (limited to 'tests') diff --git a/tests/lib/SetupTest.php b/tests/lib/SetupTest.php index 40481de8f1d..fc9def6e1c3 100644 --- a/tests/lib/SetupTest.php +++ b/tests/lib/SetupTest.php @@ -138,6 +138,8 @@ class SetupTest extends \Test\TestCase { * @param $expected */ public function testFindWebRootCli($url, $expected) { + $cliState = \OC::$CLI; + $this->config ->expects($this->once()) ->method('getValue') @@ -150,6 +152,7 @@ class SetupTest extends \Test\TestCase { $webRoot = false; } + \OC::$CLI = $cliState; $this->assertEquals($webRoot, $expected); } -- cgit v1.2.3 From 3b7ac0c94d473b9458d39e4d0b1d4d33cd31c379 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Thu, 13 Sep 2018 12:24:06 +0200 Subject: Change visibility to private Signed-off-by: Daniel Kesselberg --- lib/private/Setup.php | 2 +- tests/lib/SetupTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/lib/private/Setup.php b/lib/private/Setup.php index 4b6f2d54583..d5ccde6bba3 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -438,7 +438,7 @@ class Setup { * @return string * @throws InvalidArgumentException when invalid value for overwrite.cli.url */ - public static function findWebRoot(SystemConfig $config): string { + private static function findWebRoot(SystemConfig $config): string { // For CLI read the value from overwrite.cli.url if (\OC::$CLI) { $webRoot = $config->getValue('overwrite.cli.url', ''); diff --git a/tests/lib/SetupTest.php b/tests/lib/SetupTest.php index fc9def6e1c3..628f9393c15 100644 --- a/tests/lib/SetupTest.php +++ b/tests/lib/SetupTest.php @@ -147,7 +147,7 @@ class SetupTest extends \Test\TestCase { \OC::$CLI = true; try { - $webRoot = $this->setupClass::findWebRoot($this->config); + $webRoot = self::invokePrivate($this->setupClass, 'findWebRoot', [$this->config]); } catch (\InvalidArgumentException $e) { $webRoot = false; } -- cgit v1.2.3