Merge pull request #12674 from nupplaphil/bug/config_typesafe

Config: Improve the node.config.php transformation

diff --git a/src/Core/Config/Util/ConfigFileTransformer.php b/src/Core/Config/Util/ConfigFileTransformer.php
index ac4990df1306155ed9873f57887049b82267d5ee..45e7a5522ba20a57e9755343f9d9b87ab9c1bd6f 100644 (file)
--- a/src/Core/Config/Util/ConfigFileTransformer.php
+++ b/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) {
-                       if (is_null($data[$category])) {
-                               $dataString .= "\t'$category' => null," . PHP_EOL;
-                               continue;
-                       }
-
-                       $dataString .= "\t'$category' => [" . PHP_EOL;
+               // the last array line, close it with a semicolon
+               $dataString .= ";" . PHP_EOL;
 
-                       if (is_array($data[$category])) {
-                               $keys = array_keys($data[$category]);
+               return $dataString;
+       }
 
-                               foreach ($keys as $key) {
-                                       $dataString .= static::mapConfigValue($key, $data[$category][$key]);
-                               }
-                       }
-                       $dataString .= "\t]," . PHP_EOL;
+       /**
+        * 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);
                }
-
-               $dataString .= "];" . PHP_EOL;
-
-               return $dataString;
        }
 
-       protected static function extractArray(array $config, int $level = 0): string
+       /**
+        * 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' => ";
-
-               if (is_null($value)) {
-                       $string .= "null,";
-               } elseif (is_array($value)) {
-                       $string .= "[" . PHP_EOL;
-                       $string .= static::extractArray($value, ++$level);
-                       $string .= str_repeat("\t", $level + 1) . '],';
-               } elseif (is_bool($value)) {
-                       $string .= ($value ? 'true' : 'false') . ",";
-               } elseif (is_numeric($value)) {
-                       $string .= $value . ",";
-               } else {
-                       $string .= sprintf('\'%s\',', addcslashes($value, '\'\\'));
+               $string = '[';
+
+               $countConfigValues = count($config);
+               // multiline defines, if each entry uses a new line
+               $multiline = false;
+
+               // Search if any value is an array, because then other formatting rules are applicable
+               foreach ($config as $item) {
+                       if (is_array($item)) {
+                               $multiline = true;
+                               break;
+                       }
+               }
+
+               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 .= PHP_EOL;
+               $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));
+               }
+       }
 }

diff --git a/tests/datasets/config/B.node.config.php b/tests/datasets/config/B.node.config.php
index 499e092a4546faadaab6154124ef4ba9b5b529ce..6b0f15ad1e468c151519097d3fe723f43b15c986 100644 (file)
--- a/tests/datasets/config/B.node.config.php
+++ b/tests/datasets/config/B.node.config.php
@@ -23,9 +23,13 @@ return [
                                        'string2' => 'false',
                                ],
                        ],
-                       'v' => true,
-                       'v3' => 1,
+                       'bool_true' => true,
+                       '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' => [

diff --git a/tests/datasets/config/C.node.config.php b/tests/datasets/config/C.node.config.php
deleted file mode 100644 (file)
index 6d93d8d..0000000
--- a/tests/datasets/config/C.node.config.php
+++ /dev/null
@@ -1,48 +0,0 @@
-<?php
-
-return [
-       'config' => [
-               'hostname' => 'friendica.local',
-       ],
-       'system' => [
-               'disable_email_validation' => 1,
-               'disable_password_exposed' => 1,
-               'throttle_limit_day' => 100,
-               'throttle_limit_week' => 100,
-               'throttle_limit_month' => 100,
-               'temppath' => '/tmp/friendica.local',
-               'theme' => 'frio',
-               'url' => 'https://friendica.local',
-               'urlpath' => '',
-               'build' => 1508,
-               'maintenance' => false,
-               'dbupdate' => 1,
-               'update' => 0,
-               'spoolpath' => '/tmp/localhost/spool',
-               'actor_name' => 'friendica',
-               'site_prvkey' => '-----BEGIN PRIVATE KEY-----
-MIICeAIBADANBgkqhkiG9w0BAQEFAASCAmIwggJeAgEAAoGBALgypZoZ2X7rYCHT
-pXZRPKZYOJrtzAZoAD6a2FESfax/mW7tGF8/XGcsu4E8a0WUs18CDb09iDlECs0r
-WFkyxOsS55FyDPVNOVheU6ziqmjTNggr1qR8iIpPW2xHAnFjCfvJxgaUfszdoeUV
-mhA++BrxFGRpfcH49O+dVcjisJEVAgMBAAECgYEAq0QsRkSSvjgMgmdQCdsvEVwm
-BafldG9vCsbfK0KOJ73c5A8AAk/fku88yMVs2J2SylwWekakStrBUFNlKkrSXEv3
-r1l0b+dfniaTGJkawqgRh+U/0G9LN+cdZYt5EuhNhCbmIQB+FOI12jAx6COwEENi
-824zrrwn0BU1VTOMwwECQQDnBqq0J9JCJAtjqX+3xd8DvTRYjYvtvXlQ8fwrGxBc
-GwURNG8aMGTaj+sn2kVWNt4wLQqj/CTJ42y0bkKYMJftAkEAzBwSqfuMZD/+nEkM
-wDQb1G2z+BaxLh5JJQo80WX9tORbspOkbEuPgFprO6NB0/vNH5m4AaL3jymokH1p
-JfVoyQJBAN+GSsmOMdf+qepeiA0V7OXgPXJkWXvHtEZGK1bFk7maBvgThF+RbTMu
-xjZD8IwvACEao03wWuPfIEEe4V4Avi0CQCc5FdUYg+gX7CO4XfzphpeR5U29fprw
-MvotN3a99L04TO7KNISjGJZ/ya+SNeo4rzhtX9DgslYOmVf64aPrvxECQQDB79vF
-Kx2HyacBSErXrlqqkPdFo8KxhKCAVhrs0VBU1WPswolzsZvRdFuielhGP49DjjE7
-JV1Als1hl1xTduNb
------END PRIVATE KEY-----
-               ',
-               'site_pubkey' => '-----BEGIN PUBLIC KEY-----
-MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC4MqWaGdl+62Ah06V2UTymWDia
-7cwGaAA+mthREn2sf5lu7RhfP1xnLLuBPGtFlLNfAg29PYg5RArNK1hZMsTrEueR
-cgz1TTlYXlOs4qpo0zYIK9akfIiKT1tsRwJxYwn7ycYGlH7M3aHlFZoQPvga8RRk
-aX3B+PTvnVXI4rCRFQIDAQAB
------END PUBLIC KEY-----
-               ',
-       ],
-];

diff --git a/tests/datasets/config/transformer/C.node.config.php b/tests/datasets/config/transformer/C.node.config.php
new file mode 100644 (file)
index 0000000..6d93d8d
--- /dev/null
+++ b/tests/datasets/config/transformer/C.node.config.php
@@ -0,0 +1,48 @@
+<?php
+
+return [
+       'config' => [
+               'hostname' => 'friendica.local',
+       ],
+       'system' => [
+               'disable_email_validation' => 1,
+               'disable_password_exposed' => 1,
+               'throttle_limit_day' => 100,
+               'throttle_limit_week' => 100,
+               'throttle_limit_month' => 100,
+               'temppath' => '/tmp/friendica.local',
+               'theme' => 'frio',
+               'url' => 'https://friendica.local',
+               'urlpath' => '',
+               'build' => 1508,
+               'maintenance' => false,
+               'dbupdate' => 1,
+               'update' => 0,
+               'spoolpath' => '/tmp/localhost/spool',
+               'actor_name' => 'friendica',
+               'site_prvkey' => '-----BEGIN PRIVATE KEY-----
+MIICeAIBADANBgkqhkiG9w0BAQEFAASCAmIwggJeAgEAAoGBALgypZoZ2X7rYCHT
+pXZRPKZYOJrtzAZoAD6a2FESfax/mW7tGF8/XGcsu4E8a0WUs18CDb09iDlECs0r
+WFkyxOsS55FyDPVNOVheU6ziqmjTNggr1qR8iIpPW2xHAnFjCfvJxgaUfszdoeUV
+mhA++BrxFGRpfcH49O+dVcjisJEVAgMBAAECgYEAq0QsRkSSvjgMgmdQCdsvEVwm
+BafldG9vCsbfK0KOJ73c5A8AAk/fku88yMVs2J2SylwWekakStrBUFNlKkrSXEv3
+r1l0b+dfniaTGJkawqgRh+U/0G9LN+cdZYt5EuhNhCbmIQB+FOI12jAx6COwEENi
+824zrrwn0BU1VTOMwwECQQDnBqq0J9JCJAtjqX+3xd8DvTRYjYvtvXlQ8fwrGxBc
+GwURNG8aMGTaj+sn2kVWNt4wLQqj/CTJ42y0bkKYMJftAkEAzBwSqfuMZD/+nEkM
+wDQb1G2z+BaxLh5JJQo80WX9tORbspOkbEuPgFprO6NB0/vNH5m4AaL3jymokH1p
+JfVoyQJBAN+GSsmOMdf+qepeiA0V7OXgPXJkWXvHtEZGK1bFk7maBvgThF+RbTMu
+xjZD8IwvACEao03wWuPfIEEe4V4Avi0CQCc5FdUYg+gX7CO4XfzphpeR5U29fprw
+MvotN3a99L04TO7KNISjGJZ/ya+SNeo4rzhtX9DgslYOmVf64aPrvxECQQDB79vF
+Kx2HyacBSErXrlqqkPdFo8KxhKCAVhrs0VBU1WPswolzsZvRdFuielhGP49DjjE7
+JV1Als1hl1xTduNb
+-----END PRIVATE KEY-----
+               ',
+               'site_pubkey' => '-----BEGIN PUBLIC KEY-----
+MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC4MqWaGdl+62Ah06V2UTymWDia
+7cwGaAA+mthREn2sf5lu7RhfP1xnLLuBPGtFlLNfAg29PYg5RArNK1hZMsTrEueR
+cgz1TTlYXlOs4qpo0zYIK9akfIiKT1tsRwJxYwn7ycYGlH7M3aHlFZoQPvga8RRk
+aX3B+PTvnVXI4rCRFQIDAQAB
+-----END PUBLIC KEY-----
+               ',
+       ],
+];

diff --git a/tests/datasets/config/transformer/D.node.config.php b/tests/datasets/config/transformer/D.node.config.php
new file mode 100644 (file)
index 0000000..9e5707a
--- /dev/null
+++ b/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',
+       ],
+];

diff --git a/tests/datasets/config/transformer/object.node.config.php b/tests/datasets/config/transformer/object.node.config.php
new file mode 100644 (file)
index 0000000..f180719
--- /dev/null
+++ b/tests/datasets/config/transformer/object.node.config.php
@@ -0,0 +1,7 @@
+<?php
+
+return [
+       'object' => [
+               'objects_not_supported' => new stdClass(),
+       ],
+];

diff --git a/tests/datasets/config/transformer/ressource.node.config.php b/tests/datasets/config/transformer/ressource.node.config.php
new file mode 100644 (file)
index 0000000..b83a139
--- /dev/null
+++ b/tests/datasets/config/transformer/ressource.node.config.php
@@ -0,0 +1,7 @@
+<?php
+
+return [
+       'ressource' => [
+               'ressources_not_allowed' => new \GuzzleHttp\Psr7\AppendStream(),
+       ],
+];

diff --git a/tests/datasets/config/transformer/small_types.node.config.php b/tests/datasets/config/transformer/small_types.node.config.php
new file mode 100644 (file)
index 0000000..4bf92e3
--- /dev/null
+++ b/tests/datasets/config/transformer/small_types.node.config.php
@@ -0,0 +1,12 @@
+<?php
+
+return [
+       'small_cat' => [
+               [
+                       'key' => 'value',
+               ],
+               [
+                       'key2' => 'value2',
+               ],
+       ],
+];

diff --git a/tests/datasets/config/transformer/types.node.config.php b/tests/datasets/config/transformer/types.node.config.php
new file mode 100644 (file)
index 0000000..d2a7dfe
--- /dev/null
+++ b/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',
+               ],
+       ],
+];

diff --git a/tests/src/Core/Config/Util/ConfigFileTransformerTest.php b/tests/src/Core/Config/Util/ConfigFileTransformerTest.php
index 5a0ab96a4674bff440231edc30a25fe77a227b8d..bb156cf2827f49da05ec3f80f67ee98a10ef9851 100644 (file)
--- a/tests/src/Core/Config/Util/ConfigFileTransformerTest.php
+++ b/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);