The form type always triggers an update query when used on doctrine entity property

[stollr]

I have a doctrine entity with a property of mapping type phone_number, which is provided by this bundle. And I use the form type of this bundle to edit the phone number of that entity.

Now I realized that the form submission always triggers an update query on the database, even if nothing changed. Usually this may be not a big issue. But in my case it also generates log entries in the database each time, because I am using the logging feature of gedmo/doctrine-extensions.

After some hours of searching I found out, that Symfony's form component clones the PhoneNumber instance of the entity and assigns it to the the entity. Then Doctrine compares the new instance and the original with === and because the objects are not identical it executes an update query.

The reason why Symfony uses a clone is that the PhoneNumberType defines false as default value for the by_reference option (see here).

I am not completely sure why this was done. But thanks to git's history I found out that this setting was introduced in this commit when the country choice form widget was added. So I assume that it was needed because it uses two child forms.

Now I wonder if I should create a PR to change the default value of the option back to true for the default widget or if I should just override the option in my project.

What do you think?

[stollr]

Setting by_reference to true is not enough to fix the issue, because the data transformer also returns another instance when the string number is parsed.

I found a workaround to get around this issue. I added this to my setter method:

    public function setMobile(?PhoneNumber $mobile): self
    {
+        if ($this->mobile && $mobile && $this->mobile->equals($mobile)) {
+            return $this;
+        }
+
        $this->mobile = $mobile;

        return $this;
    }

[maxhelias]

Hi,
I'm not sure, but I think the by_reference should stay like this.
On the other hand your workaround, should be documented as a good practice on setter phone number. WDYT @Nek- ?

[Nek-]

Thank you very much for highlighting this issue and reporting @stollr. It helps a lot.

First of all, using doctrine events for domain stuff is not a good idea. Ref [1], [2], [3].

That said, I think the behavior here is a little unexpected anyway, so you're right @stollr . And maybe we have something to fix, but this is tricky and may have side effects. (fixing this would be a bc break for the exact same reason it's a problem) I'm in favor of trying something. Maybe inside the transformer?

In any case, I don't think this is a "good practice" to make this fix. It looks far more like (it actually is) a hack to prevent something unexpected to happen. This is why I'm more in favor of finding what's wrong (and maybe giving a better alternative) or fixing it definitely.

[stollr]

Thanks for your reply. I agree with you. Changing the by_reference is not the correct way.

I'm in favor of trying something. Maybe inside the transformer?

That was my first idea, too. But there is no clean way to access the model value in the Transformer.

Just for the records. This is a general issue with objects comparison in Doctrine ()see doctrine/orm#5542). There is a discussion happening, but without a real solution.

Maybe there's a way to solve this issue with a form event. I'll have a look at this.

[JcDecour]

Hi you can fix the problem by using the PRE_SUBMIT event when submitting your form

I applied it on a collectionType but it works on the PhoneNumberType code to adapt

` $builder->get('phones')->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) {
$phonesData = $event->getData();
$form = $event->getForm();

        /** @var Third $third */
        $third = $form->getParent()->getData();

        $existingPhoneNumbers = array_map(function (Phone $phone) {
            return $phone->getNumber()->__toString();
        }, $third->getPhones()->toArray());

        foreach ($phonesData as $key => $phoneData) {
            if (in_array($phoneData['number'], $existingPhoneNumbers, true)) {
                unset($phonesData[$key]);
            }
        }

        $event->setData($phonesData);
    });`