Just commit config transactions if something changed

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;
 	}

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);

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);
+	}
 }