Make BaseURL check/save transactional and make the whole process easier

diff --git a/src/App/BaseURL.php b/src/App/BaseURL.php
index 89dded300289d798c63a0693fea63d6e54f073f9..e557f712f0078fa868db105ad0ff9dc9d32365c3 100644 (file)
--- a/src/App/BaseURL.php
+++ b/src/App/BaseURL.php
@@ -172,54 +172,37 @@ class BaseURL
         */
        public function save($hostname = null, $sslPolicy = null, $urlPath = null): bool
        {
-               $currHostname  = $this->hostname;
-               $currSSLPolicy = $this->sslPolicy;
-               $currURLPath   = $this->urlPath;
-               $currUrl       = $this->url;
+               $currUrl = $this->url;
+
+               $configTransaction = $this->config->beginTransaction();
+               $savable           = false;
 
                if (!empty($hostname) && $hostname !== $this->hostname) {
-                       if ($this->config->set('config', 'hostname', $hostname)) {
-                               $this->hostname = $hostname;
-                       } else {
-                               return false;
-                       }
+                       $configTransaction->set('config', 'hostname', $hostname);
+                       $this->hostname = $hostname;
+                       $savable        = true;
                }
 
                if (isset($sslPolicy) && $sslPolicy !== $this->sslPolicy) {
-                       if ($this->config->set('system', 'ssl_policy', $sslPolicy)) {
-                               $this->sslPolicy = $sslPolicy;
-                       } else {
-                               $this->hostname = $currHostname;
-                               $this->config->set('config', 'hostname', $this->hostname);
-                               return false;
-                       }
+                       $configTransaction->set('system', 'ssl_policy', $sslPolicy);
+                       $this->sslPolicy = $sslPolicy;
+                       $savable         = true;
                }
 
                if (isset($urlPath) && $urlPath !== $this->urlPath) {
-                       if ($this->config->set('system', 'urlpath', $urlPath)) {
-                               $this->urlPath = $urlPath;
-                       } else {
-                               $this->hostname  = $currHostname;
-                               $this->sslPolicy = $currSSLPolicy;
-                               $this->config->set('config', 'hostname', $this->hostname);
-                               $this->config->set('system', 'ssl_policy', $this->sslPolicy);
-                               return false;
-                       }
+                       $configTransaction->set('system', 'urlpath', $urlPath);
+                       $this->urlPath = $urlPath;
+                       $savable       = true;
                }
 
                $this->determineBaseUrl();
                if ($this->url !== $currUrl) {
-                       if (!$this->config->set('system', 'url', $this->url)) {
-                               $this->hostname  = $currHostname;
-                               $this->sslPolicy = $currSSLPolicy;
-                               $this->urlPath   = $currURLPath;
-                               $this->determineBaseUrl();
+                       $configTransaction->set('system', 'url', $this->url);
+                       $savable = true;
+               }
 
-                               $this->config->set('config', 'hostname', $this->hostname);
-                               $this->config->set('system', 'ssl_policy', $this->sslPolicy);
-                               $this->config->set('system', 'urlpath', $this->urlPath);
-                               return false;
-                       }
+               if ($savable) {
+                       $configTransaction->commit();
                }
 
                return true;

diff --git a/src/Core/Config/Capability/ISetConfigValuesTransactionally.php b/src/Core/Config/Capability/ISetConfigValuesTransactionally.php
index 501e24f738a4d6d350a3b7612b05db53c9a4a914..a9fe36b022578b20f32aa9d10a617b5c2852e01d 100644 (file)
--- a/src/Core/Config/Capability/ISetConfigValuesTransactionally.php
+++ b/src/Core/Config/Capability/ISetConfigValuesTransactionally.php
@@ -41,8 +41,6 @@ interface ISetConfigValuesTransactionally
         * @param mixed  $value  The value to store
         *
         * @return static the current instance
-        *
-        * @throws ConfigPersistenceException In case the persistence layer throws errors
         */
        public function set(string $cat, string $key, $value): self;
 
@@ -54,8 +52,6 @@ interface ISetConfigValuesTransactionally
         *
         * @return static the current instance
         *
-        * @throws ConfigPersistenceException In case the persistence layer throws errors
-        *
         */
        public function delete(string $cat, string $key): self;
 

diff --git a/tests/src/Util/BaseURLTest.php b/tests/src/Util/BaseURLTest.php
index e108385f055b485436b51b3ebe227f24386fbef9..b9c23a1d8b12d3b7d24dc6b9be9d556b772372d1 100644 (file)
--- a/tests/src/Util/BaseURLTest.php
+++ b/tests/src/Util/BaseURLTest.php
@@ -23,10 +23,25 @@ namespace Friendica\Test\src\Util;
 
 use Friendica\App\BaseURL;
 use Friendica\Core\Config\Capability\IManageConfigValues;
+use Friendica\Core\Config\Capability\ISetConfigValuesTransactionally;
+use Friendica\Core\Config\Model\Config;
+use Friendica\Core\Config\Model\ConfigTransaction;
+use Friendica\Core\Config\Util\ConfigFileManager;
+use Friendica\Core\Config\ValueObject\Cache;
 use Friendica\Test\MockedTest;
+use Friendica\Test\Util\VFSTrait;
 
 class BaseURLTest extends MockedTest
 {
+       use VFSTrait;
+
+       protected function setUp(): void
+       {
+               parent::setUp();
+
+               $this->setUpVfsDir();
+       }
+
        public function dataDefault()
        {
                return [
@@ -330,30 +345,20 @@ class BaseURLTest extends MockedTest
         */
        public function testSave($input, $save, $url)
        {
-               $configMock = \Mockery::mock(IManageConfigValues::class);
-               $configMock->shouldReceive('get')->with('config', 'hostname')->andReturn($input['hostname']);
-               $configMock->shouldReceive('get')->with('system', 'urlpath')->andReturn($input['urlPath']);
-               $configMock->shouldReceive('get')->with('system', 'ssl_policy')->andReturn($input['sslPolicy']);
-               $configMock->shouldReceive('get')->with('system', 'url')->andReturn($input['url']);
-               $configMock->shouldReceive('get')->with('system', 'force_ssl')->andReturn($input['force_ssl']);
-
-               $baseUrl = new BaseURL($configMock, []);
-
-               if (isset($save['hostname']) && ($save['hostname'] !== $input['hostname'])) {
-                       $configMock->shouldReceive('set')->with('config', 'hostname', $save['hostname'])->andReturn(true)->once();
-               }
-
-               if (isset($save['urlPath']) && ($save['urlPath'] !== $input['urlPath'])) {
-                       $configMock->shouldReceive('set')->with('system', 'urlpath', $save['urlPath'])->andReturn(true)->once();
-               }
-
-               if (isset($save['sslPolicy']) && ($save['sslPolicy'] !== $input['sslPolicy'])) {
-                       $configMock->shouldReceive('set')->with('system', 'ssl_policy', $save['sslPolicy'])->andReturn(true)->once();
-               }
+               $configFileManager = new ConfigFileManager($this->root->url(), $this->root->url() . '/config/', $this->root->url() . '/static/');
+               $config = new Config($configFileManager, new Cache([
+                       'config' => [
+                               'hostname' => $input['hostname'] ?? null,
+                       ],
+                       'system' => [
+                               'urlpath' => $input['urlPath'] ?? null,
+                               'ssl_policy' => $input['sslPolicy'] ?? null,
+                               'url' => $input['url'] ?? null,
+                               'force_ssl' => $input['force_ssl'] ?? null,
+                       ],
+               ]));
 
-               if ($input['url'] !== $url) {
-                       $configMock->shouldReceive('set')->with('system', 'url', $url)->andReturn(true)->once();
-               }
+               $baseUrl = new BaseURL($config, []);
 
                $baseUrl->save($save['hostname'], $save['sslPolicy'], $save['urlPath']);
 
@@ -370,30 +375,20 @@ class BaseURLTest extends MockedTest
         */
        public function testSaveByUrl($input, $save, $url)
        {
-               $configMock = \Mockery::mock(IManageConfigValues::class);
-               $configMock->shouldReceive('get')->with('config', 'hostname')->andReturn($input['hostname']);
-               $configMock->shouldReceive('get')->with('system', 'urlpath')->andReturn($input['urlPath']);
-               $configMock->shouldReceive('get')->with('system', 'ssl_policy')->andReturn($input['sslPolicy']);
-               $configMock->shouldReceive('get')->with('system', 'url')->andReturn($input['url']);
-               $configMock->shouldReceive('get')->with('system', 'force_ssl')->andReturn($input['force_ssl']);
-
-               $baseUrl = new BaseURL($configMock, []);
-
-               if (isset($save['hostname']) && ($save['hostname'] !== $input['hostname'])) {
-                       $configMock->shouldReceive('set')->with('config', 'hostname', $save['hostname'])->andReturn(true)->once();
-               }
-
-               if (isset($save['urlPath']) && ($save['urlPath'] !== $input['urlPath'])) {
-                       $configMock->shouldReceive('set')->with('system', 'urlpath', $save['urlPath'])->andReturn(true)->once();
-               }
-
-               if (isset($save['sslPolicy']) && ($save['sslPolicy'] !== $input['sslPolicy'])) {
-                       $configMock->shouldReceive('set')->with('system', 'ssl_policy', $save['sslPolicy'])->andReturn(true)->once();
-               }
+               $configFileManager = new ConfigFileManager($this->root->url(), $this->root->url() . '/config/', $this->root->url() . '/static/');
+               $config = new Config($configFileManager, new Cache([
+                       'config' => [
+                               'hostname' => $input['hostname'] ?? null,
+                       ],
+                       'system' => [
+                               'urlpath' => $input['urlPath'] ?? null,
+                               'ssl_policy' => $input['sslPolicy'] ?? null,
+                               'url' => $input['url'] ?? null,
+                               'force_ssl' => $input['force_ssl'] ?? null,
+                       ],
+               ]));
 
-               if ($input['url'] !== $url) {
-                       $configMock->shouldReceive('set')->with('system', 'url', $url)->andReturn(true)->once();
-               }
+               $baseUrl = new BaseURL($config, []);
 
                $baseUrl->saveByURL($url);
 
@@ -517,65 +512,4 @@ class BaseURLTest extends MockedTest
 
                self::assertEquals($redirect, $baseUrl->checkRedirectHttps());
        }
-
-       public function dataWrongSave()
-       {
-               return [
-                       'wrongHostname' => [
-                               'fail' => 'hostname',
-                       ],
-                       'wrongSSLPolicy' => [
-                               'fail' => 'sslPolicy',
-                       ],
-                       'wrongURLPath' => [
-                               'fail' => 'urlPath',
-                       ],
-                       'wrongURL' => [
-                               'fail' => 'url',
-                       ],
-               ];
-       }
-
-       /**
-        * Test the save() method with wrong parameters
-        * @dataProvider dataWrongSave
-        */
-       public function testWrongSave($fail)
-       {
-               $configMock = \Mockery::mock(IManageConfigValues::class);
-               $configMock->shouldReceive('get')->with('config', 'hostname')->andReturn('friendica.local');
-               $configMock->shouldReceive('get')->with('system', 'urlpath')->andReturn('new/test');
-               $configMock->shouldReceive('get')->with('system', 'ssl_policy')->andReturn(BaseURL::DEFAULT_SSL_SCHEME);
-               $configMock->shouldReceive('get')->with('system', 'url')->andReturn('http://friendica.local/new/test');
-
-               switch ($fail) {
-                       case 'hostname':
-                               $configMock->shouldReceive('set')->with('config', 'hostname', \Mockery::any())->andReturn(false)->once();
-                               break;
-                       case 'sslPolicy':
-                               $configMock->shouldReceive('set')->with('config', 'hostname', \Mockery::any())->andReturn(true)->twice();
-                               $configMock->shouldReceive('set')->with('system', 'ssl_policy', \Mockery::any())->andReturn(false)->once();
-                               break;
-                       case 'urlPath':
-                               $configMock->shouldReceive('set')->with('config', 'hostname', \Mockery::any())->andReturn(true)->twice();
-                               $configMock->shouldReceive('set')->with('system', 'ssl_policy', \Mockery::any())->andReturn(true)->twice();
-                               $configMock->shouldReceive('set')->with('system', 'urlpath', \Mockery::any())->andReturn(false)->once();
-                               break;
-                       case 'url':
-                               $configMock->shouldReceive('set')->with('config', 'hostname', \Mockery::any())->andReturn(true)->twice();
-                               $configMock->shouldReceive('set')->with('system', 'ssl_policy', \Mockery::any())->andReturn(true)->twice();
-                               $configMock->shouldReceive('set')->with('system', 'urlpath', \Mockery::any())->andReturn(true)->twice();
-                               $configMock->shouldReceive('set')->with('system', 'url', \Mockery::any())->andReturn(false)->once();
-                               break;
-               }
-
-               $baseUrl = new BaseURL($configMock, []);
-               self::assertFalse($baseUrl->save('test', 10, 'nope'));
-
-               // nothing should have changed because we never successfully saved anything
-               self::assertEquals('friendica.local', $baseUrl->getHostname());
-               self::assertEquals('new/test', $baseUrl->getUrlPath());
-               self::assertEquals(BaseURL::DEFAULT_SSL_SCHEME, $baseUrl->getSSLPolicy());
-               self::assertEquals('http://friendica.local/new/test', $baseUrl->get());
-       }
 }