oneToMany orphanRemoval with unique - constraint violation

[akomm]

I currently do not have time for the test, also because this blocks me: #6738. I try to add one asap.

Issue is the following:

Preconditions:

• You have an OneToMany with orphanRemoval set from the one-side.
• On the many-side you have a unique constraint

You do the following in a single transaction/flush:

1. Remove some entities from the many-side
2. Create new entities on the many-side

When the column value (unique constraint) on the removed entities (1) equals the column value (unique constraint) on the created entities (2) a constraint violation is raised.

Reason:
The created entities (2) are INSERTed before the removed entities (1) are DELETEd.

Found a fixed, similar issue, just ManyToMany: #2310

[lcobucci]

@akomm thanks for reporting the issue, we do need a test though (we're also short on time). Send us something when you're available and we discuss on the PR.

[akomm]

I resolved the #6738 issue but could not make a reproduction "quick" and worked around it removing the unique constraint temporary relying on code as long as the app is not in prod. Want to return to the reproduction test when rest is done, so I can enable the constraint again.

[julienb-allopneus]

Hi,
We're facing the same problem but we won't just remove the constraint ;-)
Anyone working on this topic?

cc @nio-hevea @Myloth

[lcobucci]

@julienb-allopneus could you please send us a failing test case that reproduces that behaviour? It would help us a lot to identify and fix the issue you're describing.

You can find examples on https://github.com/doctrine/doctrine2/tree/388afb46d0cb3ed0c51332e8df0de9e942c2690b/tests/Doctrine/Tests/ORM/Functional/Ticket

[julienb-allopneus]

Hi @lcobucci ,
I'll work on this today, and keep you informed as soon as I can.

[julienb-allopneus]

Hi guys @lcobucci & @akomm ,

I have written a test in the following PR #6838 , please have a look and tell me what's next :-)
FYI, I have no experience in doctrine contribution and doctrine internals .

Thanks,

Julien.

cc @nio-hevea @Myloth

[julienb-allopneus]

Hi all,

Any news/advice on the test I wrote?
@lcobucci , is it ok for you?

Thanks,

Julien.

[julienb-allopneus]

Hi @lcobucci and @akomm ,

Any news on this issue?
I guess that we may remove the "Missing tests" label and requalify the issue.

Thanks,

Julien.

[toby-griffiths]

I'm having this same issue on a OnToOne relationship as well (no matter which the owning side it).

Is this related to this issue, or is there some other documentation someone can point me at?

[Ocramius]

@toby we'd need a test case to verify that.

…

On 6 Mar 2018 18:38, "Toby Griffiths" ***@***.***> wrote: I'm having this same issue on a OnToOne relationship as well (no matter which the owning side it). Is this related to this issue, or is there some other documentation someone can point me at? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#6776 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJakO86kAhc5wRGz95LNkqfQUwTU-acks5tbsl4gaJpZM4P6XxY> .

[lcobucci]

@Ocramius @toby-griffiths we do have a test case in #6838 - it needs to be updated, sure

[toby-griffiths]

@Ocramius Should I add my case details here, or raise a new ticket?

[toby-griffiths]

Or Raise a new Test Case, as per @lcobucci 's comment?

[toby-griffiths]

(Thanks for the prompt reply, btw)

[toby-griffiths]

@lcobucci / @Ocramius is it possible to add a test case file to that existing Test Case PR (#6838)? Or should I raise a separate PR to include the OneToOne test case?

[Ocramius]

@toby-griffiths yeah, you'd need to send a PR against that fork

[toby-griffiths]

OK. Will do when I can find a moment.

[artem328]

Any updates on this issue?

[toby-griffiths]

Please advise if this PR needs modifying (& how) as I was unable to submit the PR to the julienb-allopneus/doctrine2 fork.

[elkangaroo]

Is this still being worked on? I am a bit lost about the status here, but this bug is quiet annoying.
Please tell me if there is anything I can do to help.

[SenseException]

Failing tests were added and a bugfix PR is needed to make the tests work.

[vincentbab]

I'm also facing this bug and it's quite annoying.
I took a look at doctrine's internals but could not find an easy fix for this bug.
Doctrine always execute INSERTs before DELETEs.
Any ideas on how to fix this ?

[sannek8552]

@vincentbab seems like they don't want to fix that. I am olso faced with that issue and trying to find workaround

[akomm]

@sannek8552 can't blame them, they do it in their free time, there is no paid core team supported by some company AFAIK, like in many other OOS projects.

[egor-dev]

I'm also facing with this :(

[beberlei]

This is a long standing bug that should have an even older ticket (I remember it was open during 2.0 beta already). The problem is that we couldn't re-order insert after deletes for some other reason that I forgot at the moment.

[kirkmadera]

I had a similar use case. I think it's better to handle it manually, until Doctrine is able to handle this more gracefully.

This method prevents deletes/inserts when an update will do the job also.

1. Loop through all existing records, find matches and update them
2. Remove all unmatched records
3. Loop through all updates and create records for the ones that were not matched.

Example dealing with currencies:

            foreach ($currency->getExchangeRates() as $exchangeRate) {
                foreach ($data['exchangeRates'] as $exchangeRateData) {
                    if ($exchangeRate->getCurrencyTo()->getCode() == $exchangeRateData['currencyTo']) {
                        $exchangeRate->setRate($exchangeRateData['rate']);
                        continue 2;
                    }
                }

                $currency->removeExchangeRate($exchangeRate);
                $this->entityManager->remove($exchangeRate);
            }

            foreach ($data['exchangeRates'] as $exchangeRateData) {
                foreach ($currency->getExchangeRates() as $exchangeRate) {
                    if ($exchangeRate->getCurrencyTo()->getCode() == $exchangeRateData['currencyTo']) {
                        continue 2;
                    }
                }

                $exchangeRate = new CurrencyExchangeRate();
                $exchangeRate->setCurrencyTo($indexedCurrencies[$exchangeRateData['currencyTo']]);
                $exchangeRate->setRate($exchangeRateData['rate']);
                $currency->addExchangeRate($exchangeRate);
            }

[Nek-]

Since nobody posted it, here you go. This issue is related to #5109 and you should be aware of the long discussion there.

[mpdude]

It seems that having the DELETEs before the INSERTs would be a fix for this case. But, #10809 adds two examples that show why this cannot easily be done, at least not in general.

[jurchiks]

I have found only one fix for this extremely annoying issue that took me many tries and several hours to work around, and that is, that I have to manually filter out removed orphans BEFORE adding new ones or updating existing ones, AND I have to flush the removed ones too:

private function setChildren(EntityManager $em, ParentEntity $parent, array $submittedChildren): void
{
    $existingChildren = $parent->getChildren();

    // region Workaround for 7 year old Doctrine bug.
    // ChildEntity has a unique index.
    // If a ChildEntity is deleted (i.e. ID not in $submittedChildren),
    // and a new one is added with the same values from the unique index columns,
    // Doctrine first tries to add the new one - and catches a UniqueConstraintViolationException,
    // never even reaching the orphanRemoval part.
    // Thus, we are forced to manually remove the orphans ourselves...
    $existingChildIds = $existingChildren->getKeys(); // indexBy="id"
    $updatedChildIds = array_filter(array_column($submittedChildren, 'id'));
    $deletedChildIds = array_diff($existingChildIds, $updatedChildIds);

    foreach ($deletedChildIds as $deletedId) {
        $em->remove($existingChildren[$deletedId]);
    }

    $em->flush(); // Doesn't work without this.
    // endregion

    foreach ($submittedChildren as ['id' => $childId, 'fieldA' => $fieldA, 'fieldB' => $fieldB]) {
        $child = $existingChildren[$childId] ?? new ChildEntity();
        $child->setFieldA($fieldA);
        $child->setFieldB($fieldB);

        if (!$child->getId()) {
            $parent->addChild($child);
        }
    }
}

Personally I would suggest adding a flag to the OneToMany relation, something like deleteBeforeInsertion=true. I presume for those to whom this situation applies, many would want exactly this, and those that wouldn't most likely require a completely custom solution anyway.