Just commit config transactions if something changed

diff --git a/src/App/BaseURL.php b/src/App/BaseURL.php
index e557f712f0078fa868db105ad0ff9dc9d32365c3..ab3d03a5bee8fc2cab4af8116d06a5db915b0283 100644 (file)
--- 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 296a469c06a6beefaa773dc88ff0a96735924531..26420b0788c079fbc784fcd51eec4ee52dc2be6e 100644 (file)
--- 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 2eec9b68f3b5f8f71e7f99794acfbf4dd1af77ad..454c760d4ae64145f9cb0e08a7c71a165f1e5287 100644 (file)
--- 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);
+       }
 }