Config fixings

- Delete now really overwrites static default/setting.config.php keys
- Delete now really overwrites static default/setting.config.php categories
- The Update::check() routine is added to different places
- Merge the given config file with the new config before writing
- Remove ConfigTransaction::get() because it's no more reliable

src/Core/Config/Model/Config.php

@@ -116,7 +116,7 @@ class Config implements IManageConfigValues
 	/** {@inheritDoc} */
 	public function delete(string $cat, string $key): bool
 	{
-		if ($this->configCache->delete($cat, $key)) {
+		if ($this->configCache->delete($cat, $key, Cache::SOURCE_DATA)) {
 			$this->save();
 			return true;
 		} else {

src/Core/Config/Model/ConfigTransaction.php

@@ -35,37 +35,13 @@ class ConfigTransaction implements ISetConfigValuesTransactionally
 	protected $config;
 	/** @var Cache */
 	protected $cache;
-	/** @var Cache */
-	protected $delCache;
 	/** @var bool field to check if something is to save */
 	protected $changedConfig = false;
 
 	public function __construct(IManageConfigValues $config)
 	{
-		$this->config   = $config;
-		$this->cache    = new Cache();
-		$this->delCache = new Cache();
-	}
-
-	/**
-	 * Get a particular user's config variable given the category name
-	 * ($cat) and a $key from the current transaction.
-	 *
-	 * Isn't part of the interface because of it's rare use case
-	 *
-	 * @param string  $cat        The category of the configuration value
-	 * @param string  $key           The configuration key to query
-	 *
-	 * @return mixed Stored value or null if it does not exist
-	 *
-	 * @throws ConfigPersistenceException In case the persistence layer throws errors
-	 *
-	 */
-	public function get(string $cat, string $key)
-	{
-		return !$this->delCache->get($cat, $key) ?
-			($this->cache->get($cat, $key) ?? $this->config->get($cat, $key)) :
-			null;
+		$this->config = $config;
+		$this->cache  = clone $config->getCache();
 	}
 
 	/** {@inheritDoc} */
@@ -81,8 +57,7 @@ class ConfigTransaction implements ISetConfigValuesTransactionally
 	/** {@inheritDoc} */
 	public function delete(string $cat, string $key): ISetConfigValuesTransactionally
 	{
-		$this->cache->delete($cat, $key);
-		$this->delCache->set($cat, $key, 'deleted');
+		$this->cache->delete($cat, $key, Cache::SOURCE_DATA);
 		$this->changedConfig = true;
 
 		return $this;
@@ -97,13 +72,8 @@ class ConfigTransaction implements ISetConfigValuesTransactionally
 		}
 
 		try {
-			$newCache = $this->config->getCache()->merge($this->cache);
-			$newCache = $newCache->diff($this->delCache);
-			$this->config->load($newCache);
-
-			// flush current cache
-			$this->cache    = new Cache();
-			$this->delCache = new Cache();
+			$this->config->load($this->cache);
+			$this->cache = clone $this->config->getCache();
 		} catch (\Exception $e) {
 			throw new ConfigPersistenceException('Cannot save config', $e);
 		}

src/Core/Config/Util/ConfigFileManager.php

@@ -93,33 +93,33 @@ class ConfigFileManager
 	 * First loads the default value for all the configuration keys, then the legacy configuration files, then the
 	 * expected local.config.php
 	 *
-	 * @param Cache $config The config cache to load to
-	 * @param bool  $raw    Set up the raw config format
+	 * @param Cache $configCache The config cache to load to
+	 * @param bool  $raw         Set up the raw config format
 	 *
 	 * @throws ConfigFileException
 	 */
-	public function setupCache(Cache $config, bool $raw = false)
+	public function setupCache(Cache $configCache, bool $raw = false)
 	{
 		// Load static config files first, the order is important
-		$config->load($this->loadStaticConfig('defaults'), Cache::SOURCE_STATIC);
-		$config->load($this->loadStaticConfig('settings'), Cache::SOURCE_STATIC);
+		$configCache->load($this->loadStaticConfig('defaults'), Cache::SOURCE_STATIC);
+		$configCache->load($this->loadStaticConfig('settings'), Cache::SOURCE_STATIC);
 
 		// try to load the legacy config first
-		$config->load($this->loadLegacyConfig('htpreconfig'), Cache::SOURCE_FILE);
-		$config->load($this->loadLegacyConfig('htconfig'), Cache::SOURCE_FILE);
+		$configCache->load($this->loadLegacyConfig('htpreconfig'), Cache::SOURCE_FILE);
+		$configCache->load($this->loadLegacyConfig('htconfig'), Cache::SOURCE_FILE);
 
 		// Now load every other config you find inside the 'config/' directory
-		$this->loadCoreConfig($config);
+		$this->loadCoreConfig($configCache);
 
 		// Now load the node.config.php file with the node specific config values (based on admin gui/console actions)
-		$this->loadDataConfig($config);
+		$this->loadDataConfig($configCache);
 
-		$config->load($this->loadEnvConfig(), Cache::SOURCE_ENV);
+		$configCache->load($this->loadEnvConfig(), Cache::SOURCE_ENV);
 
 		// In case of install mode, add the found basepath (because there isn't a basepath set yet
-		if (!$raw && empty($config->get('system', 'basepath'))) {
+		if (!$raw && empty($configCache->get('system', 'basepath'))) {
 			// Setting at least the basepath we know
-			$config->set('system', 'basepath', $this->baseDir, Cache::SOURCE_FILE);
+			$configCache->set('system', 'basepath', $this->baseDir, Cache::SOURCE_FILE);
 		}
 	}
 
@@ -139,7 +139,7 @@ class ConfigFileManager
 
 		if (file_exists($configName)) {
 			return $this->loadConfigFile($configName);
-		} elseif (file_exists($iniName)) {
+		} else if (file_exists($iniName)) {
 			return $this->loadINIConfigFile($iniName);
 		} else {
 			return [];
@@ -149,31 +149,31 @@ class ConfigFileManager
 	/**
 	 * Tries to load the specified core-configuration into the config cache.
 	 *
-	 * @param Cache $config The Config cache
+	 * @param Cache $configCache The Config cache
 	 *
 	 * @throws ConfigFileException if the configuration file isn't readable
 	 */
-	private function loadCoreConfig(Cache $config)
+	private function loadCoreConfig(Cache $configCache)
 	{
 		// try to load legacy ini-files first
 		foreach ($this->getConfigFiles(true) as $configFile) {
-			$config->load($this->loadINIConfigFile($configFile), Cache::SOURCE_FILE);
+			$configCache->load($this->loadINIConfigFile($configFile), Cache::SOURCE_FILE);
 		}
 
 		// try to load supported config at last to overwrite it
 		foreach ($this->getConfigFiles() as $configFile) {
-			$config->load($this->loadConfigFile($configFile), Cache::SOURCE_FILE);
+			$configCache->load($this->loadConfigFile($configFile), Cache::SOURCE_FILE);
 		}
 	}
 
 	/**
 	 * Tries to load the data config file with the overridden data
 	 *
-	 * @param Cache $config The Config cache
+	 * @param Cache $configCache The Config cache
 	 *
 	 * @throws ConfigFileException In case the config file isn't loadable
 	 */
-	private function loadDataConfig(Cache $config)
+	private function loadDataConfig(Cache $configCache)
 	{
 		$filename = $this->configDir . '/' . self::CONFIG_DATA_FILE;
 
@@ -194,37 +194,69 @@ class ConfigFileManager
 
 			$dataArray = eval('?>' . $content);
 
-			if (!is_array($dataArray)) {
-				throw new ConfigFileException(sprintf('Error loading config file %s', $filename));
+			if (is_array($dataArray)) {
+				$configCache->load($dataArray, Cache::SOURCE_DATA);
 			}
-
-			$config->load($dataArray, Cache::SOURCE_DATA);
 		}
 	}
 
 	/**
 	 * Saves overridden config entries back into the data.config.phpR
 	 *
-	 * @param Cache $config The config cache
+	 * @param Cache $configCache The config cache
 	 *
 	 * @throws ConfigFileException In case the config file isn't writeable or the data is invalid
 	 */
-	public function saveData(Cache $config)
+	public function saveData(Cache $configCache)
 	{
-		$data = $config->getDataBySource(Cache::SOURCE_DATA);
+		$filename = $this->configDir . '/' . self::CONFIG_DATA_FILE;
 
-		$encodedData = ConfigFileTransformer::encode($data);
-
-		if (!$encodedData) {
-			throw new ConfigFileException('config source cannot get encoded');
+		if (file_exists($filename)) {
+			$fileExists = true;
+		} else {
+			$fileExists = false;
 		}
 
-		$configStream = fopen($this->configDir . '/' . self::CONFIG_DATA_FILE, 'w');
+		$configStream = fopen($filename, 'c+');
 
-		if (flock($configStream, LOCK_EX)) {
-			fwrite($configStream, $encodedData);
-			fflush($configStream);
-			flock($configStream, LOCK_UN);
+		try {
+			if (flock($configStream, LOCK_EX)) {
+
+				if ($fileExists) {
+					clearstatcache(true, $filename);
+					$content = fread($configStream, filesize($filename));
+					rewind($configStream);
+					if (!$content) {
+						throw new ConfigFileException(sprintf('Couldn\'t read file %s', $filename));
+					}
+
+					$dataArray = eval('?>' . $content);
+
+					if (is_array($dataArray)) {
+						$fileConfigCache = new Cache();
+						$fileConfigCache->load($dataArray, Cache::SOURCE_DATA);
+						$configCache = $fileConfigCache->merge($configCache);
+					}
+				}
+
+				$data = $configCache->getDataBySource(Cache::SOURCE_DATA);
+
+				$encodedData = ConfigFileTransformer::encode($data);
+
+				if (!$encodedData) {
+					throw new ConfigFileException('config source cannot get encoded');
+				}
+
+				clearstatcache(true, $filename);
+				if (!ftruncate($configStream, 0) ||
+					!fwrite($configStream, $encodedData) ||
+					!fflush($configStream) ||
+					!flock($configStream, LOCK_UN)) {
+					throw new ConfigFileException(sprintf('Cannot modify locked file %s', $filename));
+				}
+			}
+		} finally {
+			fclose($configStream);
 		}
 	}
 
@@ -242,7 +274,7 @@ class ConfigFileManager
 		$filepath = $this->baseDir . DIRECTORY_SEPARATOR .   // /var/www/html/
 					Addon::DIRECTORY . DIRECTORY_SEPARATOR . // addon/
 					$name . DIRECTORY_SEPARATOR .            // openstreetmap/
-					'config'. DIRECTORY_SEPARATOR . 		 // config/
+					'config' . DIRECTORY_SEPARATOR .         // config/
 					$name . ".config.php";                   // openstreetmap.config.php
 
 		if (file_exists($filepath)) {

src/Core/Config/Util/ConfigFileTransformer.php

@@ -34,6 +34,12 @@ class ConfigFileTransformer
 		$categories = array_keys($data);
 
 		foreach ($categories as $category) {
+
+			if (is_null($data[$category])) {
+				$dataString .= "\t'$category' => null," . PHP_EOL;
+				continue;
+			}
+
 			$dataString .= "\t'$category' => [" . PHP_EOL;
 
 			if (is_array($data[$category])) {
@@ -66,7 +72,9 @@ class ConfigFileTransformer
 	{
 		$string = str_repeat("\t", $level + 2) . "'$key' => ";
 
-		if (is_array($value)) {
+		if (is_null($value)) {
+			$string .= "null,";
+		} elseif (is_array($value)) {
 			$string .= "[" . PHP_EOL;
 			$string .= static::extractArray($value, ++$level);
 			$string .= str_repeat("\t", $level + 1) . '],';

src/Core/Config/ValueObject/Cache.php

@@ -65,7 +65,7 @@ class Cache
 	 * @param bool  $hidePasswordOutput True, if cache variables should take extra care of password values
 	 * @param int   $source             Sets a source of the initial config values
 	 */
-	public function __construct(array $config = [], bool $hidePasswordOutput = true, $source = self::SOURCE_DEFAULT)
+	public function __construct(array $config = [], bool $hidePasswordOutput = true, int $source = self::SOURCE_DEFAULT)
 	{
 		$this->hidePasswordOutput = $hidePasswordOutput;
 		$this->load($config, $source);
@@ -87,11 +87,10 @@ class Cache
 				$keys = array_keys($config[$category]);
 
 				foreach ($keys as $key) {
-					$value = $config[$category][$key];
-					if (isset($value)) {
-						$this->set($category, $key, $value, $source);
-					}
+					$this->set($category, $key, $config[$category][$key] ?? null, $source);
 				}
+			} else {
+				$this->set($category, null, $config[$category], $source);
 			}
 		}
 	}
@@ -150,6 +149,8 @@ class Cache
 						$data[$category][$key] = $this->config[$category][$key];
 					}
 				}
+			} elseif (is_int($this->source[$category])) {
+				$data[$category] = null;
 			}
 		}
 
@@ -159,40 +160,49 @@ class Cache
 	/**
 	 * Sets a value in the config cache. Accepts raw output from the config table
 	 *
-	 * @param string $cat    Config category
-	 * @param string $key    Config key
-	 * @param mixed  $value  Value to set
-	 * @param int    $source The source of the current config key
+	 * @param string  $cat    Config category
+	 * @param ?string $key    Config key
+	 * @param ?mixed  $value  Value to set
+	 * @param int     $source The source of the current config key
 	 *
 	 * @return bool True, if the value is set
 	 */
-	public function set(string $cat, string $key, $value, int $source = self::SOURCE_DEFAULT): bool
+	public function set(string $cat, string $key = null, $value = null, int $source = self::SOURCE_DEFAULT): bool
 	{
-		if (!isset($this->config[$cat])) {
+		if (!isset($this->config[$cat]) && $key !== null) {
 			$this->config[$cat] = [];
 			$this->source[$cat] = [];
 		}
 
-		if (isset($this->source[$cat][$key]) &&
-			$source < $this->source[$cat][$key]) {
+		if ((isset($this->source[$cat][$key]) && $source < $this->source[$cat][$key]) ||
+			(is_int($this->source[$cat] ?? null) && $source < $this->source[$cat])) {
 			return false;
 		}
 
 		if ($this->hidePasswordOutput &&
 			$key == 'password' &&
 			is_string($value)) {
-			$this->config[$cat][$key] = new HiddenString((string)$value);
+			$this->setCatKeyValueSource($cat, $key, new HiddenString((string)$value), $source);
 		} else if (is_string($value)) {
-			$this->config[$cat][$key] = self::toConfigValue($value);
+			$this->setCatKeyValueSource($cat, $key, self::toConfigValue($value), $source);
 		} else {
-			$this->config[$cat][$key] = $value;
+			$this->setCatKeyValueSource($cat, $key, $value, $source);
 		}
 
-		$this->source[$cat][$key] = $source;
-
 		return true;
 	}
 
+	private function setCatKeyValueSource(string $cat, string $key = null, $value = null, int $source = self::SOURCE_DEFAULT)
+	{
+		if (isset($key)) {
+			$this->config[$cat][$key] = $value;
+			$this->source[$cat][$key] = $source;
+		} else {
+			$this->config[$cat] = $value;
+			$this->source[$cat] = $source;
+		}
+	}
+
 	/**
 	 * Formats a DB value to a config value
 	 * - null   = The db-value isn't set
@@ -222,24 +232,27 @@ class Cache
 	/**
 	 * Deletes a value from the config cache.
 	 *
-	 * @param string $cat Config category
-	 * @param string $key Config key
+	 * @param string  $cat Config category
+	 * @param ?string $key Config key (if not set, the whole category will get deleted)
+	 * @param int     $source The source of the current config key
 	 *
 	 * @return bool true, if deleted
 	 */
-	public function delete(string $cat, string $key): bool
+	public function delete(string $cat, string $key = null, int $source = self::SOURCE_DEFAULT): bool
 	{
 		if (isset($this->config[$cat][$key])) {
-			unset($this->config[$cat][$key]);
-			unset($this->source[$cat][$key]);
-			if (count($this->config[$cat]) == 0) {
-				unset($this->config[$cat]);
-				unset($this->source[$cat]);
+			$this->config[$cat][$key] = null;
+			$this->source[$cat][$key] = $source;
+			if (empty(array_filter($this->config[$cat], function($value) { return !is_null($value); }))) {
+				$this->config[$cat] = null;
+				$this->source[$cat] = $source;
 			}
-			return true;
-		} else {
-			return false;
+		} elseif (isset($this->config[$cat])) {
+			$this->config[$cat] = null;
+			$this->source[$cat] = $source;
 		}
+
+		return true;
 	}
 
 	/**
@@ -302,39 +315,9 @@ class Cache
 					$newConfig[$category][$key] = $cache->config[$category][$key];
 					$newSource[$category][$key] = $cache->source[$category][$key];
 				}
-			}
-		}
-
-		$newCache = new Cache();
-		$newCache->config = $newConfig;
-		$newCache->source = $newSource;
-
-		return $newCache;
-	}
-
-
-	/**
-	 * Diffs a new Cache into the existing one and returns the diffed Cache
-	 *
-	 * @param Cache $cache The cache, which should get deleted for the current Cache
-	 *
-	 * @return Cache The diffed Cache
-	 */
-	public function diff(Cache $cache): Cache
-	{
-		$newConfig = $this->config;
-		$newSource = $this->source;
-
-		$categories = array_keys($cache->config);
-
-		foreach ($categories as $category) {
-			if (is_array($cache->config[$category])) {
-				$keys = array_keys($cache->config[$category]);
-
-				foreach ($keys as $key) {
-					unset($newConfig[$category][$key]);
-					unset($newSource[$category][$key]);
-				}
+			} else {
+				$newConfig[$category] = $cache->config[$category];
+				$newSource[$category] = $cache->source[$category];
 			}
 		}