[settings] postToSettings

Trying to keep the weird POST-formatting to a minimum, using real types where possible

ext/setup/main.php

@@ -16,11 +16,11 @@ require_once "config.php";
 class ConfigSaveEvent extends Event
 {
     public Config $config;
-    /** @var array<string, mixed> $values */
+    /** @var array<string, null|string|int|boolean|array<string>> $values */
     public array $values;
 
     /**
-     * @param array<string, mixed> $values
+     * @param array<string, null|string|int|boolean|array<string>> $values
      */
     public function __construct(Config $config, array $values)
     {
@@ -28,6 +28,55 @@ class ConfigSaveEvent extends Event
         $this->config = $config;
         $this->values = $values;
     }
+
+    /**
+     * Convert POST data to settings data, eg
+     *
+     *     $_POST = [
+     *         "_type_mynull" => "string",
+     *         "_type_mystring" => "string",
+     *         "_config_mystring" => "hello world!",
+     *         "_type_myint" => "int",
+     *         "_config_myint" => "42KB",
+     *     ]
+     *
+     * becomes
+     *
+     *     $config = [
+     *         "mynull" => null,
+     *         "mystring" => "hello world!",
+     *         "myint" => 43008,
+     *     ]
+     *
+     * @param array<string, string|string[]> $post
+     * @return array<string, null|string|int|boolean|array<string>>
+     */
+    public static function postToSettings(array $post): array
+    {
+        $settings = [];
+        foreach ($post as $key => $type) {
+            if (str_starts_with($key, "_type_")) {
+                $key = str_replace("_type_", "", $key);
+                $value = $post["_config_$key"] ?? null;
+                if ($type === "string") {
+                    $settings[$key] = $value;
+                } elseif ($type === "int") {
+                    assert(is_string($value));
+                    $settings[$key] = $value ? parse_shorthand_int($value) : null;
+                } elseif ($type === "bool") {
+                    $settings[$key] = $value === "on";
+                } elseif ($type === "array") {
+                    $settings[$key] = $value;
+                } else {
+                    if (is_array($value)) {
+                        $value = implode(", ", $value);
+                    }
+                    throw new InvalidInput("Invalid type '$value' for key '$key'");
+                }
+            }
+        }
+        return $settings;
+    }
 }
 
 /*
@@ -343,7 +392,7 @@ class Setup extends Extension
             send_event(new SetupBuildingEvent($panel));
             $this->theme->display_page($page, $panel);
         } elseif ($event->page_matches("setup/save", method: "POST", permission: Permissions::CHANGE_SETTING)) {
-            send_event(new ConfigSaveEvent($config, $event->POST));
+            send_event(new ConfigSaveEvent($config, ConfigSaveEvent::postToSettings($event->POST)));
             $page->flash("Config saved");
             $page->set_mode(PageMode::REDIRECT);
             $page->set_redirect(make_link("setup"));
@@ -385,35 +434,14 @@ class Setup extends Extension
     public function onConfigSave(ConfigSaveEvent $event): void
     {
         $config = $event->config;
-        foreach ($event->values as $_name => $junk) {
+        foreach ($event->values as $key => $value) {
-            if (substr($_name, 0, 6) == "_type_") {
+            match(true) {
-                $name = substr($_name, 6);
+                is_null($value) => $config->delete($key),
-                $type = $event->values["_type_$name"];
+                is_string($value) => $config->set_string($key, $value),
-                if (isset($event->values["_config_$name"])) {
+                is_int($value) => $config->set_int($key, $value),
-                    $value = $event->values["_config_$name"];
+                is_bool($value) => $config->set_bool($key, $value),
-                    switch ($type) {
+                is_array($value) => $config->set_array($key, $value),
-                        case "string":
+            };
-                            $config->set_string($name, $value);
-                            break;
-                        case "int":
-                            $config->set_int($name, parse_shorthand_int((string)$value));
-                            break;
-                        case "bool":
-                            $config->set_bool($name, bool_escape($value));
-                            break;
-                        case "array":
-                            $config->set_array($name, $value);
-                            break;
-                    }
-                } else {
-                    // browsers don't send empty checkboxes, false value must be stored in case default is true
-                    if ($type == "bool") {
-                        $config->set_bool($name, false);
-                    } else {
-                        $config->delete($name);
-                    }
-                }
-            }
         }
         log_warning("setup", "Configuration updated");
     }

ext/setup/test.php

@@ -6,6 +6,45 @@ namespace Shimmie2;
 
 class SetupTest extends ShimmiePHPUnitTestCase
 {
+    public function testParseSettings(): void
+    {
+        $this->assertEquals(
+            [
+                "mynull" => null,
+                "mystring" => "hello world!",
+                "myint" => 42 * 1024,
+                "mybool_true" => true,
+                "mybool_false" => false,
+                "myarray" => ["hello", "world"],
+            ],
+            ConfigSaveEvent::postToSettings([
+                // keys in POST that don't start with _type or _config are ignored
+                "some_post" => "value",
+                // _type with no _config means the value is null
+                "_type_mynull" => "string",
+                // strings left as-is
+                "_type_mystring" => "string",
+                "_config_mystring" => "hello world!",
+                // ints parsed from human-readable form
+                "_type_myint" => "int",
+                "_config_myint" => "42KB",
+                // HTML booleans (HTML checkboxes are "on" or undefined, there is no "off")
+                "_type_mybool_true" => "bool",
+                "_config_mybool_true" => "on",
+                "_type_mybool_false" => "bool",
+                // Arrays are... passed as arrays? Does this work?
+                "_type_myarray" => "array",
+                "_config_myarray" => ["hello", "world"],
+            ])
+        );
+
+        $this->assertException(InvalidInput::class, function () {
+            ConfigSaveEvent::postToSettings([
+                "_type_myint" => "cake",
+                "_config_myint" => "tasty",
+            ]);
+        });
+    }
     public function testNiceUrlsTest(): void
     {
         # XXX: this only checks that the text is "ok", to check

ext/user_config/main.php

@@ -146,7 +146,7 @@ class UserConfig extends Extension
             }
 
             $target_config = UserConfig::get_for_user($duser->id);
-            send_event(new ConfigSaveEvent($target_config, $event->POST));
+            send_event(new ConfigSaveEvent($target_config, ConfigSaveEvent::postToSettings($event->POST)));
             $page->flash("Config saved");
             $page->set_mode(PageMode::REDIRECT);
             $page->set_redirect(make_link("user_config"));