From 4c2fc3ea384d5aeb2705bc634448442deb9e0ac6 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 3 Jan 2023 23:55:51 +0100 Subject: [PATCH 1/7] Reduce config->set() load for worker executions --- src/App/BaseURL.php | 21 +++++++++++--------- tests/src/Util/BaseURLTest.php | 35 ++++++++++++++++++++++++++-------- 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/src/App/BaseURL.php b/src/App/BaseURL.php index 20fd54916..89dded300 100644 --- a/src/App/BaseURL.php +++ b/src/App/BaseURL.php @@ -175,6 +175,7 @@ class BaseURL $currHostname = $this->hostname; $currSSLPolicy = $this->sslPolicy; $currURLPath = $this->urlPath; + $currUrl = $this->url; if (!empty($hostname) && $hostname !== $this->hostname) { if ($this->config->set('config', 'hostname', $hostname)) { @@ -207,16 +208,18 @@ class BaseURL } $this->determineBaseUrl(); - if (!$this->config->set('system', 'url', $this->url)) { - $this->hostname = $currHostname; - $this->sslPolicy = $currSSLPolicy; - $this->urlPath = $currURLPath; - $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(); - $this->config->set('config', 'hostname', $this->hostname); - $this->config->set('system', 'ssl_policy', $this->sslPolicy); - $this->config->set('system', 'urlpath', $this->urlPath); - return false; + $this->config->set('config', 'hostname', $this->hostname); + $this->config->set('system', 'ssl_policy', $this->sslPolicy); + $this->config->set('system', 'urlpath', $this->urlPath); + return false; + } } return true; diff --git a/tests/src/Util/BaseURLTest.php b/tests/src/Util/BaseURLTest.php index 0be83be0a..e108385f0 100644 --- a/tests/src/Util/BaseURLTest.php +++ b/tests/src/Util/BaseURLTest.php @@ -231,6 +231,21 @@ class BaseURLTest extends MockedTest public function dataSave() { return [ + 'no_change' => [ + 'input' => [ + 'hostname' => 'friendica.local', + 'urlPath' => 'path', + 'sslPolicy' => BaseURL::SSL_POLICY_FULL, + 'url' => 'https://friendica.local/path', + 'force_ssl' => true, + ], + 'save' => [ + 'hostname' => 'friendica.local', + 'urlPath' => 'path', + 'sslPolicy' => BaseURL::SSL_POLICY_FULL, + ], + 'url' => 'https://friendica.local/path', + ], 'default' => [ 'input' => [ 'hostname' => 'friendica.old', @@ -324,19 +339,21 @@ class BaseURLTest extends MockedTest $baseUrl = new BaseURL($configMock, []); - if (isset($save['hostname'])) { + if (isset($save['hostname']) && ($save['hostname'] !== $input['hostname'])) { $configMock->shouldReceive('set')->with('config', 'hostname', $save['hostname'])->andReturn(true)->once(); } - if (isset($save['urlPath'])) { + if (isset($save['urlPath']) && ($save['urlPath'] !== $input['urlPath'])) { $configMock->shouldReceive('set')->with('system', 'urlpath', $save['urlPath'])->andReturn(true)->once(); } - if (isset($save['sslPolicy'])) { + if (isset($save['sslPolicy']) && ($save['sslPolicy'] !== $input['sslPolicy'])) { $configMock->shouldReceive('set')->with('system', 'ssl_policy', $save['sslPolicy'])->andReturn(true)->once(); } - $configMock->shouldReceive('set')->with('system', 'url', $url)->andReturn(true)->once(); + if ($input['url'] !== $url) { + $configMock->shouldReceive('set')->with('system', 'url', $url)->andReturn(true)->once(); + } $baseUrl->save($save['hostname'], $save['sslPolicy'], $save['urlPath']); @@ -362,19 +379,21 @@ class BaseURLTest extends MockedTest $baseUrl = new BaseURL($configMock, []); - if (isset($save['hostname'])) { + if (isset($save['hostname']) && ($save['hostname'] !== $input['hostname'])) { $configMock->shouldReceive('set')->with('config', 'hostname', $save['hostname'])->andReturn(true)->once(); } - if (isset($save['urlPath'])) { + if (isset($save['urlPath']) && ($save['urlPath'] !== $input['urlPath'])) { $configMock->shouldReceive('set')->with('system', 'urlpath', $save['urlPath'])->andReturn(true)->once(); } - if (isset($save['sslPolicy'])) { + if (isset($save['sslPolicy']) && ($save['sslPolicy'] !== $input['sslPolicy'])) { $configMock->shouldReceive('set')->with('system', 'ssl_policy', $save['sslPolicy'])->andReturn(true)->once(); } - $configMock->shouldReceive('set')->with('system', 'url', $url)->andReturn(true)->once(); + if ($input['url'] !== $url) { + $configMock->shouldReceive('set')->with('system', 'url', $url)->andReturn(true)->once(); + } $baseUrl->saveByURL($url); From 317c525cbe55e5c087d656de6a6dade39824c7d1 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 3 Jan 2023 23:58:33 +0100 Subject: [PATCH 2/7] Fix keyValue() call at daemon.php --- bin/daemon.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/daemon.php b/bin/daemon.php index f9ed693f3..577e884eb 100755 --- a/bin/daemon.php +++ b/bin/daemon.php @@ -126,7 +126,7 @@ if ($mode == 'status') { unlink($pidfile); - DI::config()->set('system', 'worker_daemon_mode', false); + DI::keyValue()->set('worker_daemon_mode', false); die("Daemon process $pid isn't running.\n"); } From 2292263780000a50e1e7583bc170cca619d86f42 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 3 Jan 2023 23:58:55 +0100 Subject: [PATCH 3/7] Add more special chars at tests --- tests/src/Core/Config/Cache/ConfigFileManagerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/Core/Config/Cache/ConfigFileManagerTest.php b/tests/src/Core/Config/Cache/ConfigFileManagerTest.php index 99049426b..55dff2f4a 100644 --- a/tests/src/Core/Config/Cache/ConfigFileManagerTest.php +++ b/tests/src/Core/Config/Cache/ConfigFileManagerTest.php @@ -407,7 +407,7 @@ class ConfigFileManagerTest extends MockedTest $configFileLoader->setupCache($configCache); - $specialChars = '!"§$%&/()(/&%$\'>set('system', 'test', 'it', Cache::SOURCE_DATA); From 17105cf7d126dc9765975dd54d2daf261a1c18a9 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 3 Jan 2023 23:06:17 +0100 Subject: [PATCH 4/7] Fix config read/write locking --- src/Core/Config/Util/ConfigFileManager.php | 25 +++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Core/Config/Util/ConfigFileManager.php b/src/Core/Config/Util/ConfigFileManager.php index 378d7fbd4..a3084b832 100644 --- a/src/Core/Config/Util/ConfigFileManager.php +++ b/src/Core/Config/Util/ConfigFileManager.php @@ -173,7 +173,21 @@ class ConfigFileManager $filename = $this->configDir . '/' . self::CONFIG_DATA_FILE; if (file_exists($filename)) { - $dataArray = include $filename; + + $content = '' . $content); if (!is_array($dataArray)) { throw new ConfigFileException(sprintf('Error loading config file %s', $filename)); @@ -200,8 +214,13 @@ class ConfigFileManager throw new ConfigFileException('config source cannot get encoded'); } - if (!file_put_contents($this->configDir . '/' . self::CONFIG_DATA_FILE, $encodedData)) { - throw new ConfigFileException(sprintf('Cannot save data to file %s/%s', $this->configDir, self::CONFIG_DATA_FILE)); + $configStream = fopen($this->configDir . '/' . self::CONFIG_DATA_FILE, 'w+'); + + if (flock($configStream, LOCK_EX)) { + ftruncate($configStream, 0); + fwrite($configStream, $encodedData); + fflush($configStream); + flock($configStream, LOCK_UN); } } From c057954896ee9984513473acb4041751ecad6339 Mon Sep 17 00:00:00 2001 From: Philipp Date: Wed, 4 Jan 2023 08:14:00 +0100 Subject: [PATCH 5/7] Make BaseURL check/save transactional and make the whole process easier --- src/App/BaseURL.php | 53 +++---- .../ISetConfigValuesTransactionally.php | 4 - tests/src/Util/BaseURLTest.php | 148 +++++------------- 3 files changed, 59 insertions(+), 146 deletions(-) diff --git a/src/App/BaseURL.php b/src/App/BaseURL.php index 89dded300..e557f712f 100644 --- 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 501e24f73..a9fe36b02 100644 --- 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 e108385f0..b9c23a1d8 100644 --- 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']); + $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, + ], + ])); - $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(); - } - - 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']); + $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, + ], + ])); - $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(); - } - - 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()); - } } From aabe39220dfbe2d914edc746799a8c74b443f986 Mon Sep 17 00:00:00 2001 From: Philipp Date: Wed, 4 Jan 2023 08:16:40 +0100 Subject: [PATCH 6/7] Make flock writing easier --- src/Core/Config/Util/ConfigFileManager.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Core/Config/Util/ConfigFileManager.php b/src/Core/Config/Util/ConfigFileManager.php index a3084b832..cc264ea26 100644 --- a/src/Core/Config/Util/ConfigFileManager.php +++ b/src/Core/Config/Util/ConfigFileManager.php @@ -214,10 +214,9 @@ class ConfigFileManager throw new ConfigFileException('config source cannot get encoded'); } - $configStream = fopen($this->configDir . '/' . self::CONFIG_DATA_FILE, 'w+'); + $configStream = fopen($this->configDir . '/' . self::CONFIG_DATA_FILE, 'w'); if (flock($configStream, LOCK_EX)) { - ftruncate($configStream, 0); fwrite($configStream, $encodedData); fflush($configStream); flock($configStream, LOCK_UN); From dce86be58efad2db2e4a473bbf8dd4d1f281d5b7 Mon Sep 17 00:00:00 2001 From: Philipp Date: Wed, 4 Jan 2023 19:55:22 +0100 Subject: [PATCH 7/7] Just commit config transactions if something changed --- src/App/BaseURL.php | 9 +------ src/Core/Config/Model/ConfigTransaction.php | 9 +++++++ .../src/Core/Config/ConfigTransactionTest.php | 24 +++++++++++++++++++ 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/App/BaseURL.php b/src/App/BaseURL.php index e557f712f..ab3d03a5b 100644 --- a/src/App/BaseURL.php +++ b/src/App/BaseURL.php @@ -175,35 +175,28 @@ class BaseURL $currUrl = $this->url; $configTransaction = $this->config->beginTransaction(); - $savable = false; if (!empty($hostname) && $hostname !== $this->hostname) { $configTransaction->set('config', 'hostname', $hostname); $this->hostname = $hostname; - $savable = true; } if (isset($sslPolicy) && $sslPolicy !== $this->sslPolicy) { $configTransaction->set('system', 'ssl_policy', $sslPolicy); $this->sslPolicy = $sslPolicy; - $savable = true; } if (isset($urlPath) && $urlPath !== $this->urlPath) { $configTransaction->set('system', 'urlpath', $urlPath); $this->urlPath = $urlPath; - $savable = true; } $this->determineBaseUrl(); if ($this->url !== $currUrl) { $configTransaction->set('system', 'url', $this->url); - $savable = true; } - if ($savable) { - $configTransaction->commit(); - } + $configTransaction->commit(); return true; } diff --git a/src/Core/Config/Model/ConfigTransaction.php b/src/Core/Config/Model/ConfigTransaction.php index 296a469c0..26420b078 100644 --- a/src/Core/Config/Model/ConfigTransaction.php +++ b/src/Core/Config/Model/ConfigTransaction.php @@ -37,6 +37,8 @@ class ConfigTransaction implements ISetConfigValuesTransactionally protected $cache; /** @var Cache */ protected $delCache; + /** @var bool field to check if something is to save */ + protected $changedConfig = false; public function __construct(IManageConfigValues $config) { @@ -70,6 +72,7 @@ class ConfigTransaction implements ISetConfigValuesTransactionally public function set(string $cat, string $key, $value): ISetConfigValuesTransactionally { $this->cache->set($cat, $key, $value, Cache::SOURCE_DATA); + $this->changedConfig = true; return $this; } @@ -80,6 +83,7 @@ class ConfigTransaction implements ISetConfigValuesTransactionally { $this->cache->delete($cat, $key); $this->delCache->set($cat, $key, 'deleted'); + $this->changedConfig = true; return $this; } @@ -87,6 +91,11 @@ class ConfigTransaction implements ISetConfigValuesTransactionally /** {@inheritDoc} */ public function commit(): void { + // If nothing changed, just do nothing :) + if (!$this->changedConfig) { + return; + } + try { $newCache = $this->config->getCache()->merge($this->cache); $newCache = $newCache->diff($this->delCache); diff --git a/tests/src/Core/Config/ConfigTransactionTest.php b/tests/src/Core/Config/ConfigTransactionTest.php index 2eec9b68f..454c760d4 100644 --- a/tests/src/Core/Config/ConfigTransactionTest.php +++ b/tests/src/Core/Config/ConfigTransactionTest.php @@ -9,6 +9,7 @@ use Friendica\Core\Config\Util\ConfigFileManager; use Friendica\Core\Config\ValueObject\Cache; use Friendica\Test\MockedTest; use Friendica\Test\Util\VFSTrait; +use Mockery\Exception\InvalidCountException; class ConfigTransactionTest extends MockedTest { @@ -108,4 +109,27 @@ class ConfigTransactionTest extends MockedTest // the whole category should be gone self::assertNull($tempData['delete'] ?? null); } + + /** + * This test asserts that in empty transactions, no saveData is called, thus no config file writing was performed + */ + public function testNothingToDo() + { + $this->configFileManager = \Mockery::spy(ConfigFileManager::class); + + $config = new Config($this->configFileManager, new Cache()); + $configTransaction = new ConfigTransaction($config); + + // commit empty transaction + $configTransaction->commit(); + + try { + $this->configFileManager->shouldNotHaveReceived('saveData'); + } catch (InvalidCountException $exception) { + self::fail($exception); + } + + // If not failed, the test ends successfully :) + self::assertTrue(true); + } }