[Question][Form] CollectionType

[jvahldick]

Hey guys,

Once I upgraded the Symfony version from 4.1 to 4.2, I noticed a change of behaviour regarding the CollectionType once I have nested arrays (array of arrays) on the payload, which previously I needed just to set the allow_extra_fields and allow_add options to true, and it identified the whole payload. Nowadays, it seems that is identifying just the fields compatible with the entry_type (And if I set the entry_type as CollectionType::class, it identifies the first level of the following one and so on).

Is this change expected?
Can you point me on the code where it was changed? (or the issue number)

I tried to find it on the changelog, but I couldn't find anything related.

Thanks in advance.

[xabbuh]

Can you give an example of how your form looks like and what data you submit?

[jvahldick]

Sorry, let me correct something there I didn't realise when I wrote it. I had added a subscriber to handle the result and it was "skipping" the form errors, but actually it considers the payload invalid in the version 4.2.

Following the correction:

it seems that is identifying just the fields compatible with the entry_type

Actually, it doesn't read the first level of the array, but it gives an error like an invalid payload if the type does not match with the entry_type.

Following below the example:

Form:

public function buildForm(FormBuilderInterface $builder, array $options)
{
    $builder
        ->add(
            'metadata', CollectionType::class, [
            'allow_add' => true,
            'allow_extra_fields' => true,
            'required' => false,
       ])
    ;
}

public function configureOptions(OptionsResolver $resolver)
{
    $resolver->setDefaults([
        'allow_extra_fields' => true,
        'data_class' => MyObject::class,
    ]);
}

So, in the version 4.1

"name": "symfony/form"
"version": "v4.1.4"

My payload is something like this:

{
    "metadata": {
    	"text_type": "text-type",
        "template": {
            "name": "my-template",
            "arguments": {
                "email": "example@example.com",
                "link": "http://example"
            }
        }
    }
}

It results in the form been considered valid, and the metadata property been fulfilled as expected in the object:

$form->isValid(); // it's true
dd($object->getMetadata());

array:2 [
  "text_type" => "text-type"
  "template" => array:2 [
    "name" => "my-template"
    "arguments" => array:2 [
      "email" => "example@example.com"
      "link" => "http://example"
    ]
  ]
]

Then I upgrade the Symfony version at this way:
composer.json

"extra": {
    "symfony": {
        "allow-contrib": false,
        "require": "4.2.*"
    }
}

composer update "symfony/*" --with-all-dependencies

I just ran it for testing right now, and it upgraded it to the version 4.2.9.
So, now when I execute the same payload I get the following result (handling the form errors):

please not that the template, which is an object, was considered invalid

{
    "status": 400,
    "type": "/api/docs/errors#validation_error",
    "title": "There was a validation error",
    "detail": null,
    "invalid-params": [
        {
            "name": "metadata.template",
            "reason": "This value is not valid."
        }
    ]
}

So, let's say we try something different changing the form:

public function buildForm(FormBuilderInterface $builder, array $options)
{
    $builder
        ->add(
            'metadata', CollectionType::class, [
            'allow_add' => true,
            'allow_extra_fields' => true,
            'entry_type' => CollectionType::class,
            'entry_options' => [
                'allow_extra_fields' => true,
                'allow_add' => true,
            ],
            'required' => false,
        ]);
}

Result:
note that now the invalid ones are the scalar/text types

{
    "status": 400,
    "type": "/api/docs/errors#validation_error",
    "title": "There was a validation error",
    "detail": null,
    "invalid-params": [
        {
            "name": "metadata.text_type",
            "reason": "This value is not valid."
        },
        {
            "name": "metadata.template.arguments",
            "reason": "This value is not valid."
        }
    ]
}

As you can notice, it considers invalid the types that don't match specifically with the entry_type.

Please let me know if you need any further information.

[iamlucianojr]

The behaviour has changed from the "symfony/form": "4.1.9" to 4.1.10

[xabbuh]

I didn't manage to look into your example in more details yet. But that's probably related to #29307. So adding the multiple option to the form type will probably help.

[jvahldick]

Indeed, adding multiple => true, and compound => false in a new FormType changes the behaviour of the form to something more similar of what we had before.

class UnstructuredType extends AbstractType
{
    public function configureOptions(OptionsResolver $resolver): array
    {
        $resolver->setDefaults([
            'multiple' => true,
            'compound' => false,
        ]);
    }
}

Although, at this way, IMO the FormType become a bit obsolete for less structured payloads, as we cannot structure at least part of the payload due to the compound option disables the Form of having more fields, or at least structure or validate them.

As a real world example:

class MessageType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder
            ->add('provider', TextType::class, [
                'constraints' => [
                    new NotBlank(),
                    new NotNull(),
                ]
            ])
            ->add(
                'body', TextType::class, [
                'constraints' => [
                    new NotBlank(),
                    new Type([
                        'type' => 'string'
                    ])
                ],
            ])
            ->add('metadata', MetadataCollectionType::class)
        ;
    }

    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults([
            'data_class' => MessageDto::class,
            'csrf_protection' => false,
            'allow_extra_fields' => true,
        ]);
    }
}

class MetadataType extends CollectionType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder
            ->add('delivery_addresses', CollectionType::class, [
                'allow_add' => true,
                'allow_extra_fields' => true,
                'constraints' => [
                    new Type('array')
                ]
            ])
            ->add('template', TemplateType::class)
        ;

        // Remove the fields w/ no data
        $builder->addEventSubscriber(new EmptyFieldsResolverSubscriber());
    }

    public function configureOptions(OptionsResolver $resolver): array
    {
        $resolver->setDefaults([
            'allow_extra_fields' => true,
            'allow_add' => true,
        ]);
    }
}

Let's say that the payload is the following json:

{
    "provider": "sendgrid",
    "body": "My message",
    "metadata": {
        "template": {
            "name": "my-template",
            "arguments": {
                "email": "example@example.com",
                "reset_password_link": "http:\/\/example"
            }
        },
	 "delivery_addresses": [
	        "my-email@email.com"
	 ],
        "force_delivery": true,
    }
}

In this case, on the version 4.1.9 I could structure properly the data coming for the metadata field, and later on, once I need to read these properties I could trust on their types. As you can see, the payload has multiple levels, some must be structured and other other don't. The logic inside the TemplateType is even more complex, but I believe just this example might work.

I saw some other people had a similar issue, but in general they were using TextType to accept the payload. I understand that the idea of the Form component is, in general, to work with well-structured data, which was one of the reasons for changing the behaviour, but I see quite often unstructured payloads and even been controversial with its name, they are logical sometimes.

I believe of the behaviour CollectionType was perfect as it was on the 4.1.9, which I could work with structured and unstructured collections.

My work around on that was essentially adding all the "keys" as "fields" dynamically, which I don't think is correct. It's something like this:

class MapUndefinedFieldsRecursivelySubscriber implements EventSubscriberInterface
{
    /**
     * {@inheritdoc}
     */
    public static function getSubscribedEvents(): array
    {
        return [
            FormEvents::PRE_SUBMIT => ['onPreSubmit', 50]
        ];
    }

    /**
     * Before submitting the form, we loop over all the fields trying to add the
     * string/array types on dynamic payloads.
     *
     * @param FormEvent $event
     *
     * @return void
     */
    public function onPreSubmit(FormEvent $event): void
    {
        $formData = $event->getData();
        $form = $event->getForm();

        if (null === $formData || !is_array($formData)) {
            return;
        }

        foreach ($formData as $fieldName => $value) {
            if (empty($value)) {
                FormHelper::cleanFormFieldReferences($form, $formData, $fieldName);
                continue;
            }

            $this->mapUnmappedField($form, (string) $fieldName, $value);
        }

        $event->setData($formData);
    }

    private function mapUnmappedField(Form $form, string $fieldName, $data): void
    {
        if (is_array($data)) {
            $this->mapArrayDataRecursively($form, $fieldName, $data);
        }

        if (is_string($data)) {
            $this->mapStringData($form, $fieldName);
        }
    }

    private function mapStringData(Form $form, string $fieldName): void
    {
        if ($form->has($fieldName)) {
            return;
        }

        $form->add($fieldName, TextType::class);
    }

    private function mapArrayDataRecursively(Form $form, string $fieldName, array $data): void
    {
        if (!$form->has($fieldName)) {
            $form->add($fieldName, CollectionType::class, [
                'allow_extra_fields' => true,
                'allow_add' => true,
            ]);
        }

        foreach ($data as $key => $value) {
            $collectionForm = $form->get($fieldName);
            $embeddedFieldName = (string) $key;

            if (!is_array($value)) {
                $this->mapStringData($collectionForm, $embeddedFieldName);
                continue;
            }

            $this->mapArrayDataRecursively($collectionForm, $embeddedFieldName, $value);
        }
    }
}

[jvahldick]

Hi @xabbuh

Did you have any time to review this?
The line that caused the issue on the CollectionType from the 4.1.9 to 4.1.10 specifically was this:
https://github.com/symfony/symfony/blob/v4.1.10/src/Symfony/Component/Form/Form.php#L538

I couldn't find exactly the piece of code on the latest versions.
If you need any further detail or a way I can please let me know.

[HeahDude]

The problem is that the nested collection is using the default TextType as entry. Try:

class MetadataType extends CollectionType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder
            ->add('delivery_addresses', CollectionType::class, [
                'entry_type' => FormType::class, // <== THIS IS THE FIX
                'allow_add' => true,
                'allow_extra_fields' => true,
                'constraints' => [
                    new Type('array')
                ]
            ])
            ->add('template', TemplateType::class)
        ;

        // Remove the fields w/ no data
        $builder->addEventSubscriber(new EmptyFieldsResolverSubscriber());
    }

    public function configureOptions(OptionsResolver $resolver): array
    {
        $resolver->setDefaults([
            'allow_extra_fields' => true,
            'allow_add' => true,
        ]);
    }
}

Basically, a TextType is a FormType but it's not compound.
So FormType should be used instead of TextType to handle array data, instead of hacking the multiple option meant for FileType or ChoiceType (select input) that are legit input submitting array when the multiple attribute is set.

[jvahldick]

Hi @HeahDude,

Unfortunately, it still doesn't work.
I have tried a couple of combinations, which the last one was the code below.

public function buildForm(FormBuilderInterface $builder, array $options): void
{
    $builder
        ->add('settings', CollectionType::class, [
            'allow_add' => true,
            'entry_type' => CollectionType::class,
            'entry_options' => [
                'allow_extra_fields' => true,
                'allow_add' => true,
                'entry_type' => FormType::class,
            ],
            'allow_extra_fields' => true,
        ])
    ;
}

It works for 1-level of depth on the collection, but not for unstructured arrays. The results I got with the new attempts were:

• form should not contain extra fields
• or simply the settings without any value

[HeahDude]

With the configuration you used each second level entry must be an array and cannot be a string, in you just want to bind a submitted array to the settings without predefining nested named fields in it then you just need to use the settings field as a form type like this:

public function buildForm(FormBuilderInterface $builder, array $options): void
{
    $builder
        ->add('settings', FormType::class, [
            'allow_extra_fields' => true,
        ])
    ;
}

[jvahldick]

Sorry, I thought it was on the entry_type of the CollectionType. I'll try earlier morning UTC. Thanks for the quick feedback.

[jvahldick]

I could not get it working, it binds the property with an empty array. The other attempt was setting form type as Collection and the entry_type as FormType, then I got just last key of the first level as key with an empty value.

The payload

"anything": {
	"a": "b",
	"c": {
		"A": "B",
		"crop": {
			"width": 200,
			"height": 500
		}
	}
}

// with 
->add('anything', FormType::class, [
    'allow_extra_fields' => true,
])

// the result is []


// with
->add('anything', CollectionType::class, [
    'allow_extra_fields' => true,
    'allow_add' => true,
    'entry_type' => FormType::class
])

// the result is anything = ["c" => []]

[redheadedstep]

I have a similar problem. I have a pagination type that is used to pass in page number, items per page, and search parameters. The search parameter can be simple like a name or complex like an array of states. So the search parameter needed to receive either a string or an array.

The problem is like above, but it is caused by validation in the symfony/Form/Form.php file. On line 541 for simple types, there is a check that fails if an array is passed in and the Type is not compound or multiple. But then on line 572, there is an additional check that fails if the Type is compound and is not an array.

The only way around those two opposite assertions was to create a new Type that was not compound (to skip the second assertion on line 572) but could still be an array (to skip the first check on line 541).

The magic way to do it was to use the multiple option (which is used in the first check but not the second).

<?php

namespace App\Form;

use Symfony\Component\Form\AbstractType;
use Symfony\Component\OptionsResolver\OptionsResolver;

class PaginationSearchType extends AbstractType
{
    /**
     * {@inheritdoc}
     */
    public function configureOptions(OptionsResolver $resolver)
    {
        $resolver->setDefaults([
            'multiple' => true, // <-- makes text fields allow arrays
            'compound' => false // <-- makes text fields with arrays accept strings
        ]);
    }
}

The implementation of this class if very simple:

<?php

namespace App\Form;

use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\CollectionType;
use Symfony\Component\Form\Extension\Core\Type\IntegerType;
use Symfony\Component\Form\Extension\Core\Type\PaginationSearchType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;

class SearchType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
            /**
             * By using a CollectionType, we can add entries to the 'search' parameter.
             * The PaginationSearchType::class will accept an array or string or a 
             * combination of the two
             */
           $builder->add('search', CollectionType::class, [
                'entry_type' => PaginationSearchType::class,
                'allow_add' => true
            ])
        ;
    }
}

When submitting data, we can submit it like:

post($url, [
    'search' => [
      'q' => 'Test Name', // parameter in CollectionType will be a string
      'states' => [ // parameter in CollectionType will be an array
        0 => 'CA',
        1 => 'NV',
        2 => 'UT'
      ]
   ]
])

[xabbuh]

@redheadedstep In your example I do not understand why an array should be submitted to each single PaginationSearchType. The array will be submitted for the CollectionType, but each single entry should receive a scalar.

[redheadedstep]

@xabbuh Normally the CollectionType has an entry_type which would be set as TextType (strings) or possibly another CollectionType or something else. But you can't pass in a sub-array to a TextType (because it has to be a string, not an array) and you can't pass a string to a CollectionType (because it has to be an array, not a string). And the entry_type option on a CollectionType only allows a single type (i.e. all elements in a CollectionType need to be strings or all elements needs be arrays)

So the PaginationSearchType really is just a Type that can accept a string or an array. It can be renamed to whatever you want, but it's what allows you to submit a multi-dimensional array with some leaf nodes as scalar strings and some leaf nodes as sub-arrays.

On the example, I tried to show that the search parameter was a Collection that could receive a string parameter (q='Test Name') and an array parameter (states=['CA', 'NV', 'UT']).

[jvahldick]

@xabbuh can't it be confirmed as a bug?
It's been a long time since this issue is opened, but it was never confirmed by Symfony's team.

If it's not a bug, can't at least we know if it was an architecture decision and the reason for that?

[xabbuh]

@jvahldick If #29307 causes issues for you, I don't think there is a bug but you need to adapt your code accordingly. But honestly I have have some trouble understanding what your code actually looks like right now following your conversation with @HeahDude.

[redheadedstep]

It's not really a bug. It's just that Symfony comes with some preset Types to use in forms. Most of them are simple (like DateType, TextType, or RadioType) and some are more complex (like CollectionType or ChoiceType). But there is validation in symfony/Form/Form.php that enforces that a type that accepts a scalar should not be an array and a type that is compound has to be an array.

So from what I've seen, there is no Type that can be an array or a string, which is what OP had a problem with in the original post. OPs code was to submit a CollectionType to handle the top level of the array and use allow_extra_fields to handle all the other levels of the array. But this changed and that's what broke.

So a user can either explicitly define every level of an CollectionType, set up a CollectionType to only be 1 level deep with strings (or dates, or ints, but not a combination of them), or setup a CollectionType to accept an array (but not anything that isn't an array).

What would be nice is if symfony shipped with a default Type that could be an array or a string that can could be used as the entry_type of CollectionType. Users could use that Type for when data is being submitted, but the user doesn't know what the data will be beforehand (for instance...search parameters or sorting fields).

But until symfony is shipped with this default StringOrArrayType, we'll just have to create our own...which is what the PaginationSearchType class does.

[carsonbot]

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

[carsonbot]

Could I get an answer? If I do not hear anything I will assume this issue is resolved or abandoned. Please get back to me <3

[jvahldick]

Hey @carsonbot

In my opinion, that is a relevant bug because the collection type does not handle unmapped fields very well. I believe I already saw a couple of suggestions to create some specific type to handle unmapped fields. As it is a Collection type, I believe this use case should be handled by it.

A simple example of the issue can be seen here:

As mentioned in the following comment, I found an alternative way to handle this using an event listener. I added it into a common library, so I am using it in my projects.

Keep safe

[carsonbot]

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

[carsonbot]

Friendly reminder that this issue exists. If I don't hear anything I'll close this.

[carsonbot]

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!