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

diff --git a/src/Core/Config/Model/Config.php b/src/Core/Config/Model/Config.php
index c96750694becdf0fae0216dedb60d25a0b84596c..5258033a754eaef1987b63b2f8984253733c0c00 100644 (file)
--- a/src/Core/Config/Model/Config.php
+++ b/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 {

diff --git a/src/Core/Config/ValueObject/Cache.php b/src/Core/Config/ValueObject/Cache.php
index db972431acb516ac5422dcf317eeb460638a0f96..be42ae1181f9689ce34af18ae056b793d0dd3584 100644 (file)
--- a/src/Core/Config/ValueObject/Cache.php
+++ b/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;

diff --git a/tests/src/Core/Config/Cache/CacheTest.php b/tests/src/Core/Config/Cache/CacheTest.php
index a4e12e23a63abb2aa1cddbafc381eadd92e87ea3..c4e536871be2c71b3b5e5bd5df837e4ebd1a2c26 100644 (file)
--- a/tests/src/Core/Config/Cache/CacheTest.php
+++ b/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'));
+       }
 }

diff --git a/tests/src/Core/Config/Cache/ConfigFileManagerTest.php b/tests/src/Core/Config/Cache/ConfigFileManagerTest.php
index 6c2c41ef65163f2a7a7d7c561a69dfa7dd8662d6..e5b630a200dac77ed45df3a90937faf81718c097 100644 (file)
--- a/tests/src/Core/Config/Cache/ConfigFileManagerTest.php
+++ b/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'));
-       }
-
-       /**
-        * 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'));
+               self::assertEquals('UTC', $configCache->get('system', 'default_timezone'));
        }
 
        /**

diff --git a/tests/src/Core/Config/ConfigTest.php b/tests/src/Core/Config/ConfigTest.php
index f7d7c070d3d4e5835cc1d465386bd80d5690d35f..f244044743e0c3a2ae4e86cf4a575f403f0fc947 100644 (file)
--- a/tests/src/Core/Config/ConfigTest.php
+++ b/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'));
+       }
 }