Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Each overwriting messages #1334

Open
tchari opened this issue Feb 5, 2021 · 4 comments
Open

Each overwriting messages #1334

tchari opened this issue Feb 5, 2021 · 4 comments

Comments

@tchari
Copy link

tchari commented Feb 5, 2021

When you validate the elements of an array (or iterable) using each, the messages get "overwritten" when you get the exception message.

    try {
      Validator::notEmpty()->iterableType()->each(
        Validator::key('street', Validator::stringType()->notEmpty())
        ->key('region', Validator::stringType()->notEmpty())
        ->key('country', Validator::stringType()->notEmpty())
        ->key('other', Validator::nullable(Validator::notEmpty()->stringType()), false)
      )->assert(
        [
          ['region' => 'Oregon', 'country' => 'USA', 'other' => 123],
          ['street' => '', 'region' => 'Oregon', 'country' => 'USA'],
          ['street' => 123, 'region' => 'Oregon', 'country' => 'USA'],
        ]
      );
    } catch (NestedValidationException $e) {
      var_dump($e->getMessages());
    }

The above var_dump results in

array(1) {
  ["each"]=>
  string(29) "street must be of type string"
}

However, I would have expected

array(1) {
  ["each"]=>
  array(3) {
    [0]=>
    array(2) {
      ['street']=>
      string(22) "street must be present"
      ['other'] =>
      string(28) "other must be of type string"
    }
    [1]=>
    array(1) {
      ['street']=>
      string(24) "street must not be empty"
    }
    [2]=>
    array(1) {
      ['street']=>
      string(29) "street must be of type string"
    }
  }
}

It is a little alarming that no exception is present for the first erroneous 'other' field.
Is there a different rule for this?

@alganet
Copy link
Member

alganet commented Feb 19, 2023

Unfortunatelly, this and other composite messages are completely broken in 2.x, and there isn't much we can do without further changing the core NestedValidationException.

However, for each specifically, this is a deal braker since it makes it unusable, so I made a hotfix and it should be available on 2.3.

Check out this test to see how the behavior changed.

For 3.0, most messages for composite validators should be reporting using a . notation or some other tree expression equivalent. I haven't settled on it yet, but every message should be available somewhere. It might be a getAllMessages or something, but it will be there.

@delboy1978uk
Copy link

Is this still an issue? I'm upgrading a site here, and I'm trying the following code:

        $v = new V();
        $v->addRule(V::key('id', V::scalarVal()));
        $v->addRule(V::key('fileReference', V::length(4, 35), false));
        $v->addRule(V::key('scenario', V::length(3, 16), false));
        $v->addRule(V::key('totalGrossMass', V::numericVal(), false));
        $v->addRule(V::key('bolNumber', V::length(1, 35)->scalarVal(), false));
        $v->addRule(V::key('arrivedDate', V::dateTime('Y-m-d H:i:s'), false));
        $v->addRule(V::key('packages', V::intVal(), false));
        $v->addRule(V::key('status', V::in(Status::toArray()), false));

        try {
            $v->assert($payload);
        } catch (NestedValidationException $e) {
            $messages = $e->getMessages();
        }

This gives me the following:

[
    "validator" => 'fileReference must have a length between 4 and 35'
]

I would have expected the key to say fileReference and not validator? Is this related or am I using it wrong?

@henriquemoody henriquemoody modified the milestones: 2.3, 3.0 Mar 9, 2024
@henriquemoody
Copy link
Member

henriquemoody commented Dec 13, 2024

In version 3.0, the array of messages will look somehow like this:

[
    'each' => [
        '__root__' => 'Each item in `[["region": "Oregon", "country": "USA", "other": 123], ["street": "", "region": "Oregon", "country": "USA"], ["s ... ]` must be valid',
        'allOf.1' => [
            '__root__' => 'These rules must pass for `["region": "Oregon", "country": "USA", "other": 123]`',
            'street' => 'street must be present',
            'other' => 'other must be a string or must be null',
        ],
        'allOf.2' => 'street must not be empty',
        'allOf.3' => 'street must be a string',
    ],
]

I'm pushing to get the new version released as soon as possible, but it might take some more time. Probably beginning of next year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants