[Validator] Any constraint on an array property causes traversing it

[corphi]

Q	A
Bug report?	yes
Feature request?	no
BC Break report?	well…
RFC?	no
Symfony version	all since 2.5.0

While #23032 also deals with arrays, this bug is specific to traversing mixed hierarchies of arrays and objects. The issue is that arrays are added to the todo list for traversing if they carry any constraint. This behaviour has been introduced in the original implementation of RecursiveContextualValidator back in 2.5.0. Older versions behaved correctly (tested with latest 2.3). This is a BC break with unsupported versions, I’m not sure if it qualifies.

Minimal test scenario:

<?php
class Foo
{
     /** @Assert\Count(min=1, max=1) */
    public $bars;
    public function __construct()
    {
        $this->bars = array(new Bar());
    }
}
class Bar
{
    /** @Assert\NotBlank */
    public $baz;
}
$validator->validate(new Foo());

If the Count constraint is removed, validation passes, as validation does not traverse $bars by default.

I created a failing test that also works below 2.5.

[spackmat]

I can confirm that for at least version 4.1.0 and 4.1.1. In my case, adding a Count Constraint on a Collection form field containing EntityType fields causes the chosen Entities to be validated, very weird:

$builder->add('myEntities', CollectionType::class, [
    'entry_type' => MyEntityType::class,
    'label' => 'myForm.myEntities.label',
    'allow_add' => true,
    'allow_delete' => true,
    'delete_empty' => true,
    'prototype' => true,
    'constraints' => [
        new Count(['min' => 1, 'groups' => ['caseA', 'caseC']]), // remove this to solve the problem
    ],
    'error_bubbling' => false,
];

And:

class MyEntityType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $builder->add('myEntity', EntityType::class, [
            'class' => MyEntity::class,
            'required' => true,
            'label' => 'Form.Type.MyEntityType.myEntity.label',
            'constraints' => [
                new NotNull(),
            ],
        ]);
    }

    /**
     * @param OptionsResolver $resolver
     */
    public function configureOptions(OptionsResolver $resolver)
    {
        $resolver
            ->setDefaults([
                'error_bubbling' => false,
                'error_mapping' => array(
                    '.' => 'myEntity', // needed because validation is based on array level within database, should be validated with a DTO instead
                ),
            ])
        ;
    }
}

If a chosen Entity within the collection is not valid by itself, its error messages are mapped to the myEntity field and are shown (without error_mapping they are not visible to the user at all). Removing the Count() constraint from the myEntities field solves the validation traversal.

I don't have any ideas for a workaround and the Count constraint on the Collection is crucial in my case.

[geoffrey-brier]

I'm using symfony 3.4.15 and I can confirm this as well.

It happens for the EntityType & CollectionType types ... in my case, I have no constraints set at all (Assert\Valid nor Assert*) yet the validation component validates the whole entity behind.

I just upgraded from 2.8.45 where it worked, I don't really know what to do for the moment except playing with validation groups ... which is not really doable as the app I'm maintaining has hundreds of forms.

[HeahDude]

Having an array validated by cascade is an expected behavior, but not expecting that each entry of a validated array value are not valid as well looks weird. If an entry is an object it's validated in this context as any other type of required entry.

The proposed solution is another BC break that may lead to invalid values.

Maybe the proper work around is to make the entries constraints optional instead of fixing the core.

[corphi]

In a nutshell: Traverse the array property and cascade validation if:

Annotation	specification	pre-2.5 implementation	2.5 implementation
(none)	No	No	No
@Valid	Yes	Yes	Yes
@NotNull	No	No	Yes (incorrect)

The BC break has already been done in 2.5 in the opposite direction. I was actually affected (and found a workaround - wrap it into a Traversable), but couldn’t pinpoint it until recently. Also, this is an edge case, because few things in the Symfony ecosystem actually use PHP arrays - usually there is a Traversable implementation at work, for example when used with forms or Doctrine entities.

[spackmat]

Just to leave a workaround for people getting strange validation errors on array data: Sometimes a typecast to an object for that array data avoids the properties from being traversed. So instead of

$form = $this->createFormBuilder(['relatedEntityMaybeInvalid' => $myRelatedEntity])->…

which will lead to (for users) unskippable form errors, when an invalid entity is selected in the relatedEntityMaybeInvalid field of the form (which is seriously annoying), one could write

$form = $this->createFormBuilder((object)['relatedEntityMaybeInvalid' => $myRelatedEntity])->…

and voilà: The validator doesn't traverse and the form fails only on its own validation rules again.

This raises the question: why are the validators traversed at all for array data?