nette / schema

📐 Validating data structures against a given Schema.
https://doc.nette.org/schema
Other
906 stars 26 forks source link

arrayOf, listOf defaults are always in array #13

Closed MartkCz closed 3 years ago

MartkCz commented 5 years ago

Version: 1.0.0

$schema = Expect::listOf('string')->default([
    'foo',
    'bar',
]);

$processor = new Processor();

$array = $processor->process($schema, [
    'foo',
    'bar'
]);

var_dump($array);

dumps:

array(4) {
  [0]=>
  string(3) "foo"
  [1]=>
  string(3) "bar"
  [2]=>
  string(3) "foo"
  [3]=>
  string(3) "bar"
}
$schema = Expect::arrayOf('string')->default([
    'foo',
    'bar',
]);

$processor = new Processor();

$array = $processor->process($schema, [
    'foo',
    'bar'
]);

var_dump($array);

dumps:

array(4) {
  [0]=>
  string(3) "foo"
  [1]=>
  string(3) "bar"
  [2]=>
  string(3) "foo"
  [3]=>
  string(3) "bar"
}

Is it the right behavior?

dg commented 5 years ago

It is an intention, although I realize that it is not completely understandable. It is used for example for the configuration of http > headers.

mabar commented 5 years ago

@MartkCz Lists are not supported by PHP, so result is same. Difference is in validation - lists does not support keys https://github.com/nette/schema/blob/master/tests/Schema/Expect.list.phpt#L14-L22

Edit: I see, default values are always included in result array, sorry

MartkCz commented 5 years ago

@mabar It's issue only about default values for array/list :)

@dg I undestand

dg commented 5 years ago

I will leave it open as a reminder that this is not ideal.

AdryanSignor commented 4 years ago

Hi, @MartkCz, @dg, a temporary "solution" to applying the default is to use the before method:

colinodell commented 4 years ago

This workaround doesn't seem to work when the option is nested within a Structure and no value is provided by the user:

test('listOf workaround from #13', function () {
    // Test current behavior of always merging
    $schema = Expect::listOf('int')->default([1, 2, 3]);

    Assert::same([1, 2, 3], (new Processor)->process($schema, null));
    Assert::same([1, 2, 3], (new Processor)->process($schema, []));
    Assert::same([1, 2, 3, 42], (new Processor)->process($schema, [42])); // Not the behavior I want; I don't want the defaults here

    // Test workaround from #13
    $schema = Expect::listOf('int')->before(function($value) {
        return $value === null || $value === [] ? [1, 2, 3] : $value;
    });

    Assert::same([1, 2, 3], (new Processor)->process($schema, null));
    Assert::same([1, 2, 3], (new Processor)->process($schema, []));
    Assert::same([42], (new Processor)->process($schema, [42])); // Workaround seems to function fine in this particular case

    // Test current behavior of always merging within a nested structure
    $schema = Expect::structure([
        'test' => Expect::listOf('int')->default([1, 2, 3])
    ])->castTo('array');

    Assert::same(['test' => [1, 2, 3]], (new Processor)->process($schema, null));
    Assert::same(['test' => [1, 2, 3]], (new Processor)->process($schema, []));
    Assert::same(['test' => [1, 2, 3]], (new Processor)->process($schema, ['test' => null]));
    Assert::same(['test' => [1, 2, 3]], (new Processor)->process($schema, ['test' => []]));
    Assert::same(['test' => [1, 2, 3, 42]], (new Processor)->process($schema, ['test' => [42]])); // Not the behavior I want; I don't want the defaults here

    // Test workaround from #13 within a nested structure
    $schema = Expect::structure([
        'test' => Expect::listOf('int')->before(function($value) {
            return $value === null || $value === [] ? [1, 2, 3] : $value;
        })
    ])->castTo('array');

    Assert::same(['test' => []], (new Processor)->process($schema, null)); // Not the behavior I want (defaults should ideally be applied if option missing)
    Assert::same(['test' => []], (new Processor)->process($schema, [])); // Not the behavior I want (defaults should ideally be applied if option missing)
    Assert::same(['test' => [1, 2, 3]], (new Processor)->process($schema, ['test' => null]));
    Assert::same(['test' => [1, 2, 3]], (new Processor)->process($schema, ['test' => []]));
    Assert::same(['test' => [42]], (new Processor)->process($schema, ['test' => [42]]));
});
AdryanSignor commented 4 years ago

Hi @colinodell, I agree. Sometimes I had to use array_replace_recursive on user's data, for example:

$schema = Expect::structure([
    'foo' => Expect::listOf('string')->before(function ($v) {
        return $v === null || $v === [] ? ['bar'] : $v;
    })
]);

$defaultStructure = [
    'foo' => null
];

$userData = [];

// Excepts
// class stdClass#11 (1) {
//  public $foo =>
// array(1) {
//    [0] =>
//    string(3) "bar"
//  }
// }

$processor->process($schema, $userData); // Doesn't work
$processor->process($schema, array_replace_recursive($defaultStructure, $userData)); // Works

I tested your solution (#28) on my projects, replacing before method by default([...])->preventMergingDefaults() and removing $defaultStructure from user's data. Much better :satisfied:.

:rocket::rocket::rocket: