From dce86be58efad2db2e4a473bbf8dd4d1f281d5b7 Mon Sep 17 00:00:00 2001
From: Philipp <admin@philipp.info>
Date: Wed, 4 Jan 2023 19:55:22 +0100
Subject: [PATCH] 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 e557f712f0..ab3d03a5be 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 296a469c06..26420b0788 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 2eec9b68f3..454c760d4a 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);
+	}
 }