illuminatech / config

Manage Laravel configuration by persistent storage
Other
150 stars 14 forks source link

Saving and validation not working for keys with dots #4

Closed SDKiller closed 5 years ago

SDKiller commented 5 years ago

As dots are internally replaced by php with undercores in $_POST

See https://www.php.net/variables.external https://github.com/symfony/symfony/issues/9009

As a result we have:

in form:

<input type="text" name="foo.bar" value="10">

in $request->all():

[
    "foo_bar" => "10"
]

in \Illuminatech\Config\PersistentRepository::makeValidator rules:

["foo->bar"]=>
  array(2) {
    [0]=>
    string(9) "sometimes"
    [1]=>
    string(7) "integer"
  }

Thus validation rules are not applied and value not saved.

SDKiller commented 5 years ago

I ended up just avoiding dot notation in keys.

However, this problem at the moment makes package useless for 'Creating configuration web interface' for the purpose of replacing default config variables, stored in files, like mail.from.address until there is no way to reliably replace and then restore dots. The same is with passing array in post - no way to reliably determine if we meant array or 'dotted' parameter.

Probably parsing php://input (which preserves original input names) instead of using build-in $request?

klimov-paul commented 5 years ago

This matter relates not only to the dots, but to any non-alphanumeric characters like -, + and so on.

This is the reason Item::$id exists. Its value should be used in the forms and for the input. You should manually set Item::$id with the value, which does not contain special chars.

$persistentConfigRepository = (new PersistentRepository($sourceConfigRepository, $storage))
    ->setItems([
        'foo.name' => ['id' => 'foo_name'],
        'bar.enabled' => ['id' => 'bar_enabled'],
    ]);

Note, that in case you updating configuration via REST API in JSON format or using real PUT method, you'll not experience this problem.

Automatic conversion of 'key' into 'id' with special chars strip or replace is risk prone, as 2 different keys may produce the same id.

Although docs should be enhanced mentioning this topic.

klimov-paul commented 5 years ago

Docs updated 9a5f2cd060208eef66f64cc297ab6f12e266cc18

Thank you for the reminder.