Merge pull request #10104 from nupplaphil/fix/jit_caching

Fix JIT Config Adapter caching

src/Core/Config/JitConfig.php

@@ -86,7 +86,7 @@ class JitConfig extends BaseConfig
 			$dbvalue = $this->configModel->get($cat, $key);
 
 			if (isset($dbvalue)) {
-				$this->configCache->set($cat, $key, $dbvalue);
+				$this->configCache->set($cat, $key, $dbvalue, Cache::SOURCE_DB);
 				unset($dbvalue);
 			}
 
@@ -105,7 +105,7 @@ class JitConfig extends BaseConfig
 	public function set(string $cat, string $key, $value)
 	{
 		// set the cache first
-		$cached = $this->configCache->set($cat, $key, $value);
+		$cached = $this->configCache->set($cat, $key, $value, Cache::SOURCE_DB);
 
 		// If there is no connected adapter, we're finished
 		if (!$this->configModel->isConnected()) {

src/Core/Config/PreloadConfig.php

@@ -81,7 +81,7 @@ class PreloadConfig extends BaseConfig
 			if ($this->configModel->isConnected()) {
 				$config = $this->configModel->get($cat, $key);
 				if (isset($config)) {
-					$this->configCache->set($cat, $key, $config);
+					$this->configCache->set($cat, $key, $config, Cache::SOURCE_DB);
 				}
 			}
 		}
@@ -102,7 +102,7 @@ class PreloadConfig extends BaseConfig
 		}
 
 		// set the cache first
-		$cached = $this->configCache->set($cat, $key, $value);
+		$cached = $this->configCache->set($cat, $key, $value, Cache::SOURCE_DB);
 
 		// If there is no connected adapter, we're finished
 		if (!$this->configModel->isConnected()) {
@@ -133,4 +133,18 @@ class PreloadConfig extends BaseConfig
 
 		return $cacheRemoved || $storeRemoved;
 	}
+
+	public function testSetDouble()
+	{
+		$this->configModel->shouldReceive('isConnected')
+						  ->andReturn(true);
+
+		// constructor loading
+		$this->configModel->shouldReceive('load')
+						  ->with('config')
+						  ->andReturn(['config' => ['test' => 'it']])
+						  ->once();
+
+		parent::testSetDouble();
+	}
 }

tests/src/Core/Config/CacheTest.php

@@ -319,4 +319,27 @@ class CacheTest extends MockedTest
 		self::assertEquals(23, $configCache->get('database', 'password'));
 		self::assertEmpty($configCache->get('database', 'username'));
 	}
+
+	/**
+	 * Test the set() method with overrides
+	 * @dataProvider dataTests
+	 */
+	public function testSetOverrides($data)
+	{
+
+		$configCache = new Cache();
+		$configCache->load($data, Cache::SOURCE_DB);
+
+		// test with wrong override
+		self::assertFalse($configCache->set('system', 'test', '1234567', Cache::SOURCE_FILE));
+		self::assertEquals($data['system']['test'], $configCache->get('system', 'test'));
+
+		// test with override (equal)
+		self::assertTrue($configCache->set('system', 'test', '8910', Cache::SOURCE_DB));
+		self::assertEquals('8910', $configCache->get('system', 'test'));
+
+		// test with override (over)
+		self::assertTrue($configCache->set('system', 'test', '111213', Cache::SOURCE_ENV));
+		self::assertEquals('111213', $configCache->get('system', 'test'));
+	}
 }

tests/src/Core/Config/ConfigTest.php

@@ -437,4 +437,35 @@ abstract class ConfigTest extends MockedTest
 
 		self::assertEmpty($this->testedConfig->getCache()->getAll());
 	}
+
+	/**
+	 * Test the configuration get() and set() method where the db value has a higher prio than the config file
+	 */
+	public function testSetGetHighPrio()
+	{
+		$this->testedConfig = $this->getInstance();
+		self::assertInstanceOf(Cache::class, $this->testedConfig->getCache());
+
+		$this->testedConfig->getCache()->set('config', 'test', 'prio', Cache::SOURCE_FILE);
+		self::assertEquals('prio', $this->testedConfig->get('config', 'test'));
+
+		// now you have to get the new variable entry because of the new set the get refresh succeed as well
+		self::assertTrue($this->testedConfig->set('config', 'test', '123'));
+		self::assertEquals('123', $this->testedConfig->get('config', 'test', '', true));
+	}
+
+	/**
+	 * Test the configuration get() and set() method where the db value has a lower prio than the env
+	 */
+	public function testSetGetLowPrio()
+	{
+		$this->testedConfig = $this->getInstance();
+		self::assertInstanceOf(Cache::class, $this->testedConfig->getCache());
+		self::assertEquals('it', $this->testedConfig->get('config', 'test'));
+
+		$this->testedConfig->getCache()->set('config', 'test', 'prio', Cache::SOURCE_ENV);
+		// now you have to get the env variable entry as output, even with a new set (which failed) and a get refresh
+		self::assertFalse($this->testedConfig->set('config', 'test', '123'));
+		self::assertEquals('prio', $this->testedConfig->get('config', 'test', '', true));
+	}
 }

tests/src/Core/Config/JitConfigTest.php

@@ -198,4 +198,58 @@ class JitConfigTest extends ConfigTest
 
 		parent::testDeleteWithDB();
 	}
+
+	public function testSetGetHighPrio()
+	{
+		$this->configModel->shouldReceive('isConnected')
+						  ->andReturn(true);
+
+		// constructor loading
+		$this->configModel->shouldReceive('load')
+						  ->with('config')
+						  ->andReturn(['config' => []])
+						  ->once();
+
+		$this->configModel->shouldReceive('get')
+						  ->with('config', 'test')
+						  ->andReturn('prio')
+						  ->once();
+
+		$this->configModel->shouldReceive('set')
+						  ->with('config', 'test', '123')
+						  ->andReturn(true)
+						  ->once();
+
+		$this->configModel->shouldReceive('get')
+						  ->with('config', 'test')
+						  ->andReturn('123')
+						  ->once();
+
+		parent::testSetGetHighPrio();
+	}
+
+	public function testSetGetLowPrio()
+	{
+		$this->configModel->shouldReceive('isConnected')
+						  ->andReturn(true);
+
+		// constructor loading
+		$this->configModel->shouldReceive('load')
+						  ->with('config')
+						  ->andReturn(['config' => ['test' => 'it']])
+						  ->once();
+
+		$this->configModel->shouldReceive('set')
+						  ->with('config', 'test', '123')
+						  ->andReturn(true)
+						  ->once();
+
+		// mocking one get without result
+		$this->configModel->shouldReceive('get')
+						  ->with('config', 'test')
+						  ->andReturn('it')
+						  ->once();
+
+		parent::testSetGetLowPrio();
+	}
 }

tests/src/Core/Config/PreloadConfigTest.php

@@ -162,4 +162,52 @@ class PreloadConfigTest extends ConfigTest
 
 		parent::testDeleteWithDB();
 	}
+
+
+	public function testSetGetHighPrio()
+	{
+		$this->configModel->shouldReceive('isConnected')
+						  ->andReturn(true);
+
+		// constructor loading
+		$this->configModel->shouldReceive('load')
+						  ->andReturn(['config' => []])
+						  ->once();
+
+		$this->configModel->shouldReceive('set')
+						  ->with('config', 'test', '123')
+						  ->andReturn(true)
+						  ->once();
+
+		$this->configModel->shouldReceive('get')
+						  ->with('config', 'test')
+						  ->andReturn('123')
+						  ->once();
+
+		parent::testSetGetHighPrio();
+	}
+
+	public function testSetGetLowPrio()
+	{
+		$this->configModel->shouldReceive('isConnected')
+						  ->andReturn(true);
+
+		// constructor loading
+		$this->configModel->shouldReceive('load')
+						  ->andReturn(['config' => ['test' => 'it']])
+						  ->once();
+
+		$this->configModel->shouldReceive('set')
+						  ->with('config', 'test', '123')
+						  ->andReturn(true)
+						  ->once();
+
+		// mocking one get without result
+		$this->configModel->shouldReceive('get')
+						  ->with('config', 'test')
+						  ->andReturn('it')
+						  ->once();
+
+		parent::testSetGetLowPrio();
+	}
 }