Merge pull request #12674 from nupplaphil/bug/config_typesafe

Config: Improve the node.config.php transformation

src/Core/Config/Util/ConfigFileTransformer.php

@@ -26,67 +26,201 @@ namespace Friendica\Core\Config\Util;
  */
 class ConfigFileTransformer
 {
+	/**
+	 * This method takes an array of config values and applies some standard rules for formatting on it
+	 *
+	 * Beware that the applied rules follow some basic formatting principles for node.config.php
+	 * and doesn't support any custom formatting rules.
+	 *
+	 * f.e. associative array and list formatting are very complex with newlines and indentations, thus there are
+	 * three hardcoded types of formatting for them.
+	 *
+	 * a negative example, what's NOT working:
+	 * key => [ value1, [inner_value1, inner_value2], value2]
+	 * Since this isn't necessary for config values, there's no further logic handling such complex-list-in-list scenarios
+	 *
+	 * @param array $data A full config array
+	 *
+	 * @return string The config stream, which can be saved
+	 */
 	public static function encode(array $data): string
 	{
+		// Add the typical header values
 		$dataString = '<?php' . PHP_EOL . PHP_EOL;
-		$dataString .= 'return [' . PHP_EOL;
+		$dataString .= 'return ';
 
-		$categories = array_keys($data);
+		$dataString .= static::extractArray($data);
 
-		foreach ($categories as $category) {
+		// the last array line, close it with a semicolon
-			if (is_null($data[$category])) {
+		$dataString .= ";" . PHP_EOL;
-				$dataString .= "\t'$category' => null," . PHP_EOL;
-				continue;
-			}
-
-			$dataString .= "\t'$category' => [" . PHP_EOL;
-
-			if (is_array($data[$category])) {
-				$keys = array_keys($data[$category]);
-
-				foreach ($keys as $key) {
-					$dataString .= static::mapConfigValue($key, $data[$category][$key]);
-				}
-			}
-			$dataString .= "\t]," . PHP_EOL;
-		}
-
-		$dataString .= "];" . PHP_EOL;
 
 		return $dataString;
 	}
 
-	protected static function extractArray(array $config, int $level = 0): string
+	/**
+	 * Extracts an inner config array.
+	 * Either as an associative array or as a list
+	 *
+	 * @param array $config The config array which should get extracted
+	 * @param int   $level  The current level of recursion (necessary for tab-indentation calculation)
+	 * @param bool  $inList If true, the current array resides inside another list. Different rules may be applicable
+	 *
+	 * @return string The config string
+	 */
+	protected static function extractArray(array $config, int $level = 0, bool $inList = false): string
+	{
+		if (array_values($config) === $config) {
+			return self::extractList($config, $level, $inList);
+		} else {
+			return self::extractAssociativeArray($config, $level, $inList);
+		}
+	}
+
+	/**
+	 * Extracts a key-value array and save it into a string
+	 * output:
+	 * [
+	 *    'key' => value,
+	 *    'key' => value,
+	 *    ...
+	 * ]
+	 *
+	 * @param array $config The associative/key-value array
+	 * @param int   $level  The current level of recursion (necessary for tab-indentation calculation)
+	 * @param bool  $inList If true, the current array resides inside another list. Different rules may be applicable
+	 *
+	 * @return string The config string
+	 */
+	protected static function extractAssociativeArray(array $config, int $level = 0, bool $inList = false): string
 	{
 		$string = '';
 
+		// Because we're in a list, we have to add a line-break first
+		if ($inList) {
+			$string .= PHP_EOL . str_repeat("\t", $level);
+		}
+
+		// Add a typical Line break for a taxative list of key-value pairs
+		$string .= '[' . PHP_EOL;
+
 		foreach ($config as $configKey => $configValue) {
-			$string .= static::mapConfigValue($configKey, $configValue, $level);
+			$string .= str_repeat("\t", $level + 1) .
+					   "'$configKey' => " .
+					   static::transformConfigValue($configValue, $level) .
+					   ',' . PHP_EOL;
 		}
 
+		$string .= str_repeat("\t", $level) . ']';
+
 		return $string;
 	}
 
-	protected static function mapConfigValue(string $key, $value, $level = 0): string
+	/**
+	 * Extracts a list and save it into a string
+	 * output1 - simple:
+	 * [ value, value, value ]
+	 *
+	 * output2 - complex:
+	 * [
+	 *    [ value, value, value ],
+	 *    value,
+	 *    [
+	 *       key => value,
+	 *       key => value,
+	 *    ],
+	 * ]
+	 *
+	 * @param array $config The list
+	 * @param int   $level  The current level of recursion (necessary for tab-indentation calculation)
+	 * @param bool  $inList If true, the current array resides inside another list. Different rules may be applicable
+	 *
+	 * @return string The config string
+	 */
+	protected static function extractList(array $config, int $level = 0, bool $inList = false): string
 	{
-		$string = str_repeat("\t", $level + 2) . "'$key' => ";
+		$string = '[';
 
-		if (is_null($value)) {
+		$countConfigValues = count($config);
-			$string .= "null,";
+		// multiline defines, if each entry uses a new line
-		} elseif (is_array($value)) {
+		$multiline = false;
-			$string .= "[" . PHP_EOL;
+
-			$string .= static::extractArray($value, ++$level);
+		// Search if any value is an array, because then other formatting rules are applicable
-			$string .= str_repeat("\t", $level + 1) . '],';
+		foreach ($config as $item) {
-		} elseif (is_bool($value)) {
+			if (is_array($item)) {
-			$string .= ($value ? 'true' : 'false') . ",";
+				$multiline = true;
-		} elseif (is_numeric($value)) {
+				break;
-			$string .= $value . ",";
+			}
-		} else {
-			$string .= sprintf('\'%s\',', addcslashes($value, '\'\\'));
 		}
 
-		$string .= PHP_EOL;
+		for ($i = 0; $i < $countConfigValues; $i++) {
+			$isArray = is_array($config[$i]);
+
+			/**
+			 * In case this is an array in an array, directly extract this array again and continue
+			 * Skip any other logic since this isn't applicable for an array in an array
+			 */
+			if ($isArray) {
+				$string .= PHP_EOL . str_repeat("\t", $level + 1);
+				$string .= static::extractArray($config[$i], $level + 1, $inList) . ',';
+				continue;
+			}
+
+			if ($multiline) {
+				$string .= PHP_EOL . str_repeat("\t", $level + 1);
+			}
+
+			$string .= static::transformConfigValue($config[$i], $level, true);
+
+			// add trailing commas or whitespaces for certain config entries
+			if (($i < ($countConfigValues - 1))) {
+				$string .= ',';
+				if (!$multiline) {
+					$string .= ' ';
+				}
+			}
+		}
+
+		// Add a new line for the last bracket as well
+		if ($multiline) {
+			$string .= PHP_EOL . str_repeat("\t", $level);
+		}
+
+		$string .= ']';
 
 		return $string;
 	}
+
+	/**
+	 * Transforms one config value and returns the corresponding text-representation
+	 *
+	 * @param mixed $value  Any value to transform
+	 * @param int   $level  The current level of recursion (necessary for tab-indentation calculation)
+	 * @param bool  $inList If true, the current array resides inside another list. Different rules may be applicable
+	 *
+	 * @return string
+	 */
+	protected static function transformConfigValue($value, int $level = 0, bool $inList = false): string
+	{
+		switch (gettype($value)) {
+			case "boolean":
+				return ($value ? 'true' : 'false');
+			case "integer":
+			case "double":
+				return $value;
+			case "string":
+				return sprintf('\'%s\'', addcslashes($value, '\'\\'));
+			case "array":
+				return static::extractArray($value, ++$level, $inList);
+			case "NULL":
+				return "null";
+			case "object":
+			case "resource":
+			case "resource (closed)":
+				throw new \InvalidArgumentException(sprintf('%s in configs are not supported yet.', gettype($value)));
+			case "unknown type":
+				throw new \InvalidArgumentException(sprintf('%s is an unknown value', $value));
+			default:
+				throw new \InvalidArgumentException(sprintf('%s is currently unsupported', $value));
+		}
+	}
 }

tests/datasets/config/B.node.config.php

@@ -23,9 +23,13 @@ return [
 					'string2' => 'false',
 				],
 			],
-			'v' => true,
+			'bool_true' => true,
-			'v3' => 1,
+			'bool_false' => false,
+			'int_1_not_true' => 1,
+			'int_0_not_false' => 0,
 			'v4' => 5.6443,
+			'string_1_not_true' => '1',
+			'string_0_not_false' => '0',
 		],
 	],
 	'system' => [

tests/datasets/config/transformer/D.node.config.php

@@ -0,0 +1,8 @@
+<?php
+
+return [
+	'string_int_values' => [
+		'string_1_not_true' => '1',
+		'string_0_not_false' => '0',
+	],
+];

tests/datasets/config/transformer/object.node.config.php

@@ -0,0 +1,7 @@
+<?php
+
+return [
+	'object' => [
+		'objects_not_supported' => new stdClass(),
+	],
+];

tests/datasets/config/transformer/ressource.node.config.php

@@ -0,0 +1,7 @@
+<?php
+
+return [
+	'ressource' => [
+		'ressources_not_allowed' => new \GuzzleHttp\Psr7\AppendStream(),
+	],
+];

tests/datasets/config/transformer/small_types.node.config.php

@@ -0,0 +1,12 @@
+<?php
+
+return [
+	'small_cat' => [
+		[
+			'key' => 'value',
+		],
+		[
+			'key2' => 'value2',
+		],
+	],
+];

tests/datasets/config/transformer/types.node.config.php

@@ -0,0 +1,75 @@
+<?php
+
+return [
+	'type_test' => [
+		'bool_true' => true,
+		'bool_false' => false,
+		'int_1' => 1,
+		'int_0' => 2,
+		'int_12345' => 12345,
+		'float' => 1.234,
+		'double_E+' => 1.24E+20,
+		'double_E-' => 7.0E-10,
+		'null' => null,
+		'array' => [1, '2', '3', 4.0E-10, 12345, 0, false, 'true', true],
+		'array_keys' => [
+			'int_1' => 1,
+			'string_2' => '2',
+			'string_3' => '3',
+			'double' => 4.0E-10,
+			'int' => 12345,
+			'int_0' => 0,
+			'false' => false,
+			'string_true' => 'true',
+			'true' => true,
+		],
+		'array_extended' => [
+			[
+				'key_1' => 'value_1',
+				'key_2' => 'value_2',
+				'key_3' => [
+					'inner_key' => 'inner_value',
+				],
+			],
+			[
+				'key_2' => false,
+				'0' => [
+					'is_that' => true,
+					'0' => [
+						'working' => '?',
+					],
+				],
+				'inner_array' => [
+					[
+						'key' => 'value',
+						'key2' => 12,
+					],
+				],
+				'key_3' => true,
+			],
+			['value', 'value2'],
+			[
+				[
+					'key' => 123,
+				],
+				'test',
+				'test52',
+				'test23',
+				[
+					'key' => 456,
+				],
+			],
+		],
+	],
+	'other_cat' => [
+		'key' => 'value',
+	],
+	'other_cat2' => [
+		[
+			'key' => 'value',
+		],
+		[
+			'key2' => 'value2',
+		],
+	],
+];

tests/src/Core/Config/Util/ConfigFileTransformerTest.php

@@ -36,8 +36,25 @@ class ConfigFileTransformerTest extends MockedTest
 				'configFile' => (dirname(__DIR__, 4) . '/datasets/config/B.node.config.php'),
 			],
 			'friendica.local' => [
-				'configFile' => (dirname(__DIR__, 4) . '/datasets/config/C.node.config.php'),
+				'configFile' => (dirname(__DIR__, 4) . '/datasets/config/transformer/C.node.config.php'),
 			],
+			'friendica.local.2' => [
+				'configFile' => (dirname(__DIR__, 4) . '/datasets/config/transformer/D.node.config.php'),
+			],
+			'object_invalid' => [
+				'configFile' => (dirname(__DIR__, 4) . '/datasets/config/transformer/object.node.config.php'),
+				'assertException' => true,
+			],
+			'ressource_invalid' => [
+				'configFile' => (dirname(__DIR__, 4) . '/datasets/config/transformer/ressource.node.config.php'),
+				'assertException' => true,
+			],
+			'test_types' => [
+				'configFile' => (dirname(__DIR__, 4) . '/datasets/config/transformer/types.node.config.php'),
+			],
+			'small_types' => [
+				'configFile' => (dirname(__DIR__, 4) . '/datasets/config/transformer/small_types.node.config.php'),
+			]
 		];
 	}
 
@@ -46,10 +63,14 @@ class ConfigFileTransformerTest extends MockedTest
 	 *
 	 * @dataProvider dataTests
 	 */
-	public function testConfigFile(string $configFile)
+	public function testConfigFile(string $configFile, bool $assertException = false)
 	{
 		$dataArray = include $configFile;
 
+		if ($assertException) {
+			self::expectException(\InvalidArgumentException::class);
+		}
+
 		$newConfig = ConfigFileTransformer::encode($dataArray);
 
 		self::assertEquals(file_get_contents($configFile), $newConfig);