Assure that deleted cat/keys are working as expected

- A deleted cache-key would delete a merged cache-key as well
- Deleting a key in the Model results in reloading the config to assure any value from underlying files

src/Core/Config/Model/Config.php

@@ -71,6 +71,8 @@ class Config implements IManageConfigValues
 	{
 		try {
 			$this->configFileManager->saveData($this->configCache);
+			// reload after the save to possible reload default values of lower source-priorities again
+			$this->reload();
 		} catch (ConfigFileException $e) {
 			throw new ConfigPersistenceException('Cannot save config', $e);
 		}
@@ -116,7 +118,7 @@ class Config implements IManageConfigValues
 	/** {@inheritDoc} */
 	public function delete(string $cat, string $key): bool
 	{
-		if ($this->configCache->delete($cat, $key, Cache::SOURCE_DATA)) {
+		if ($this->configCache->delete($cat, $key)) {
 			$this->save();
 			return true;
 		} else {

src/Core/Config/ValueObject/Cache.php

@@ -55,6 +55,11 @@ class Cache
 	 */
 	private $source = [];
 
+	/**
+	 * @var array
+	 */
+	private $delConfig = [];
+
 	/**
 	 * @var bool
 	 */
@@ -232,6 +237,7 @@ class Cache
 		if (isset($this->config[$cat][$key])) {
 			unset($this->config[$cat][$key]);
 			unset($this->source[$cat][$key]);
+			$this->delConfig[$cat][$key] = true;
 			if (count($this->config[$cat]) == 0) {
 				unset($this->config[$cat]);
 				unset($this->source[$cat]);
@@ -313,6 +319,19 @@ class Cache
 			}
 		}
 
+		$delCategories = array_keys($cache->delConfig);
+
+		foreach ($delCategories as $category) {
+			if (is_array($cache->delConfig[$category])) {
+				$keys = array_keys($cache->delConfig[$category]);
+
+				foreach ($keys as $key) {
+					unset($newConfig[$category][$key]);
+					unset($newSource[$category][$key]);
+				}
+			}
+		}
+
 		$newCache = new Cache();
 		$newCache->config = $newConfig;
 		$newCache->source = $newSource;

tests/src/Core/Config/Cache/CacheTest.php

@@ -576,4 +576,25 @@ class CacheTest extends MockedTest
 		$cache->delete('config', 'a');
 		self::assertArrayNotHasKey('config', $cache->getAll());
 	}
+
+	/**
+	 * Test that deleted keys are working with merge
+	 *
+	 * @dataProvider dataTests
+	 */
+	public function testDeleteAndMergeWithDefault($data)
+	{
+		$cache = new Cache();
+		$cache->load($data, Cache::SOURCE_FILE);
+
+		$cache2 = new Cache();
+		$cache2->set('system', 'test', 'overrride');
+		$cache2->delete('system', 'test');
+
+		self::assertEquals('it', $cache->get('system', 'test'));
+		self::assertNull($cache2->get('system', 'test'));
+
+		$mergedCache = $cache->merge($cache2);
+		self::assertNull($mergedCache->get('system', 'test'));
+	}
 }

tests/src/Core/Config/Cache/ConfigFileManagerTest.php

@@ -443,7 +443,7 @@ class ConfigFileManagerTest extends MockedTest
 	}
 
 	/**
-	 * If we delete something with the Cache::delete() functionality, be sure to override the underlying source as well
+	 * If we delete something with the Cache::delete() functionality, be sure to probably reset it to the underlying key
 	 */
 	public function testDeleteKeyOverwrite()
 	{
@@ -465,51 +465,16 @@ class ConfigFileManagerTest extends MockedTest
 
 		$configFileManager->setupCache($configCache);
 
-		$configCache->delete('system', 'default_timezone', Cache::SOURCE_DATA);
+		$configCache->delete('system', 'default_timezone');
 
 		$configFileManager->saveData($configCache);
 
-		// assert that system.default_timezone is now null, even it's set with settings.conf.php
+		// assert that system.default_timezone is now the restored 'UTC' from the defaults
 		$configCache       = new Cache();
 
 		$configFileManager->setupCache($configCache);
 
-		self::assertNull($configCache->get('system', 'default_timezone'));
+		self::assertEquals('UTC', $configCache->get('system', 'default_timezone'));
-	}
-
-	/**
-	 * If we delete something with the Cache::delete() functionality, be sure to override the underlying source as well
-	 */
-	public function testDeleteCategoryOverwrite()
-	{
-		$this->delConfigFile('node.config.php');
-
-		$fileDir = dirname(__DIR__) . DIRECTORY_SEPARATOR .
-				   '..' . DIRECTORY_SEPARATOR .
-				   '..' . DIRECTORY_SEPARATOR .
-				   '..' . DIRECTORY_SEPARATOR .
-				   'datasets' . DIRECTORY_SEPARATOR .
-				   'config' . DIRECTORY_SEPARATOR;
-
-		vfsStream::newFile('B.config.php')
-				 ->at($this->root->getChild('config'))
-				 ->setContent(file_get_contents($fileDir . 'B.config.php'));
-
-		$configFileManager = (new Config())->createConfigFileManager($this->root->url());
-		$configCache       = new Cache();
-
-		$configFileManager->setupCache($configCache);
-
-		$configCache->delete('system');
-
-		$configFileManager->saveData($configCache);
-
-		// assert that system.default_timezone is now null, even it's set with settings.conf.php
-		$configCache       = new Cache();
-
-		$configFileManager->setupCache($configCache);
-
-		self::assertNull($configCache->get('system', 'default_timezone'));
 	}
 
 	/**

tests/src/Core/Config/ConfigTest.php

@@ -363,8 +363,6 @@ class ConfigTest extends MockedTest
 		self::assertTrue($this->testedConfig->delete('test', 'it'));
 		self::assertNull($this->testedConfig->get('test', 'it'));
 		self::assertNull($this->testedConfig->getCache()->get('test', 'it'));
-
-		self::assertEquals(['test' => null], $this->testedConfig->getCache()->getAll());
 	}
 
 	/**
@@ -532,4 +530,22 @@ class ConfigTest extends MockedTest
 
 		self::assertEquals($assertion, $config->get($category));
 	}
+
+	/**
+	 * Tests, if an overwritten value of an existing key will reset to it's default after deletion
+	 */
+	public function testDeleteReturnsDefault()
+	{
+		vfsStream::newFile(ConfigFileManager::CONFIG_DATA_FILE)
+				 ->at($this->root->getChild('config'))
+				 ->setContent(ConfigFileTransformer::encode([
+					 'config' => ['sitename' => 'overritten'],
+				 ]));
+
+		$config = $this->getInstance();
+		self::assertEquals('overritten', $config->get('config', 'sitename'));
+
+		$config->delete('config', 'sitename');
+		self::assertEquals('Friendica Social Network', $config->get('config', 'sitename'));
+	}
 }