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

Persisting entities with OneToOne and OneToMany/ManyToOne relationship between them. #6499

Closed
Frikkle opened this issue Jun 12, 2017 · 9 comments · Fixed by #10547
Closed
Assignees
Labels

Comments

@Frikkle
Copy link
Contributor

Frikkle commented Jun 12, 2017

Last week I encountered an issue in one of our projects where we had two entities, call them A and B having a unidirectional OneToOne A->B (with the join column not nullable) and a OneToMany(A->B)/ManyToOne(B->A) bidirectional relationship between them.

The problem lies in the order of persisting them; when first persisting A, then B, a constraint violation is thrown: SQLSTATE[23000]: Integrity constraint violation: 19 NOT NULL constraint failed: a.b_id, indicating that the join column isn't allowed to be NULL, even though it should have been set.

I reworked my code to a minimal example:

Entity A

/**
 * Class A
 * @ORM\Entity()
 */
class A
{
    /**
     * @ORM\Id()
     * @ORM\GeneratedValue(strategy="AUTO")
     * @ORM\Column(name="id", type="integer")
     */
    private $id;

    /**
     * @ORM\OneToMany(targetEntity="AppBundle\Entity\B", mappedBy="a", cascade={"persist", "remove"}, orphanRemoval=true)
     */
    private $bs;

    /**
     * @ORM\OneToOne(targetEntity="AppBundle\Entity\B", cascade={"persist"})
     * @ORM\JoinColumn(nullable=false)
     */
    private $b;

    /**
     * @return int
     */
    public function getId()
    {
        return $this->id;
    }

    /**
     * @return B[]|ArrayCollection
     */
    public function getBs()
    {
        return $this->bs;
    }

    /**
     * @param B $b
     */
    public function addB(B $b)
    {
        if ($this->bs->contains($b)) return;

        $this->bs->add($b);

        // Update owning side
        $b->setA($this);
    }

    /**
     * @param B $b
     */
    public function removeB(B $b)
    {
        if (!$this->bs->contains($b)) return;

        $this->bs->removeElement($b);

        // Not updating owning side due to orphan removal
    }

    /**
     * @return B
     */
    public function getB()
    {
        return $this->b;
    }

    /**
     * @param B $b
     */
    public function setB(B $b)
    {
        $this->b = $b;
    }
}

Entity B

/**
 * Class B
 * @ORM\Entity()
 */
class B
{
    /**
     * @ORM\Id()
     * @ORM\GeneratedValue(strategy="AUTO")
     * @ORM\Column(name="id", type="integer")
     */
    private $id;

    /**
     * @ORM\ManyToOne(targetEntity="AppBundle\Entity\A", inversedBy="bs", cascade={"persist"})
     */
    private $a;

    /**
     * @return int
     */
    public function getId()
    {
        return $this->id;
    }

    /**
     * @return A
     */
    public function getA()
    {
        return $this->a;
    }

    /**
     * @param A $a
     */
    public function setA(A $a)
    {
        $this->a = $a;
    }
}

My persist code

$a = new A();
$entityManager->persist($a);

$b = new B();
$a->setB($b);
$entityManager->persist($b);

$entityManager->flush();

The code above throws the constraint violation error. At first I thought it might be related to circular cascade persists. But if I remove all cascade annotation and the orphanRemoval clauses as well, it still throws the ConstraintViolationException.

What does work?
It does however seem to work when I revert the persist order, like this:

$a = new A();
$b = new B();
$a->setB($b);

$entityManager->persist($b);
$entityManager->persist($a);

$entityManager->flush();

And, it also works when wrapping the whole operation in a transaction. But somehow I have the feeling that it should work.

Possibly related issues
When looking through the issue list I came across #4090 which seems to be a similar issue. Maybe this helps to shine light on that as well?

Our environment
We're working in Symfony 2.8 LTS using the doctrine/doctrine-bundle version 1.6.4 and doctrine/orm version 2.5.5. After upgradring both of them to the latest version, the problem persisted.

@lcobucci
Copy link
Member

@triasinformatica-gabe could you check if this issue happens on master? Commit order has been fixed after a major refactoring and probably solves this issue.

@Frikkle
Copy link
Contributor Author

Frikkle commented Jun 23, 2017

@lcobucci I will check this later today and get back to you with the findings. Thansk for the reply.

@lcobucci
Copy link
Member

@triasinformatica-gabe awesome! Please keep in mind that dependencies versions for v2.6 (master) have been bumped up (PHP and packages).

@Frikkle
Copy link
Contributor Author

Frikkle commented Jun 28, 2017

@lcobucci I've just tested the minimal example against ^2.6/master and the issue still occurs.

In order to help pinpoint the issue I've created a repository with the minimal example (Symfony 2.8.*, DoctrineBundle, Doctrine and PHPUnit). I added the example entities A and B mentioned above as well as two simple PHPUnit tests indicating the problematic scenario.

Setting it up would be a matter of running the following commands, given you have a database connection set-up:

> composer install
> php app/console doctrine:schema:update --force
> phpunit -c app/

If there's anything else I can do to contribute to this, please let me know.

@lcobucci
Copy link
Member

@triasinformatica-gabe what would really helps us is a PR with that failing test. Could you please send it?

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

@Frikkle
Copy link
Contributor Author

Frikkle commented Jun 28, 2017

@lcobucci Sure, I'll look into it.

@Frikkle
Copy link
Contributor Author

Frikkle commented Jun 29, 2017

@lcobucci I created PR #6533 as per your request. I also found the cause of the issue and provided a fix. It seems like a logic error that resulted from code optimization.

@Ocramius Ocramius added the Bug label Aug 11, 2017
@Ocramius Ocramius added this to the 2.5.7 milestone Aug 11, 2017
@Ocramius Ocramius self-assigned this Aug 11, 2017
Ocramius added a commit that referenced this issue Aug 11, 2017
Ocramius added a commit that referenced this issue Aug 11, 2017
Ocramius added a commit that referenced this issue Aug 11, 2017
Ocramius added a commit that referenced this issue Aug 11, 2017
Ocramius added a commit that referenced this issue Aug 11, 2017
…r makes it *SOMEWHAT* more readable (no miracles)
@Ocramius Ocramius modified the milestones: 2.6.0, 2.5.7 Aug 11, 2017
Ocramius added a commit that referenced this issue Aug 11, 2017
…nsider-all-join-column-fields'"

This reverts commit 2a58645, reversing
changes made to 6d428c9.
@Ocramius Ocramius removed this from the 2.6.0 milestone Aug 11, 2017
@Ocramius Ocramius assigned Frikkle and unassigned Ocramius Aug 11, 2017
@Ocramius Ocramius reopened this Aug 11, 2017
@Ocramius
Copy link
Member

The patch related to this was reverted, as foreign key integrity issues occur when dealing with deletes.

This needs additional testing and inspection.

rvanlaak added a commit to rvanlaak/orm that referenced this issue May 20, 2021
rvanlaak added a commit to rvanlaak/orm that referenced this issue May 20, 2021
rvanlaak added a commit to rvanlaak/orm that referenced this issue May 26, 2021
rvanlaak added a commit to rvanlaak/orm that referenced this issue May 26, 2021
rvanlaak added a commit to rvanlaak/orm that referenced this issue May 26, 2021
rvanlaak added a commit to rvanlaak/orm that referenced this issue Jun 30, 2021
rvanlaak added a commit to rvanlaak/orm that referenced this issue Jun 30, 2021
…bug on non-nullable cascading relationship
rvanlaak added a commit to rvanlaak/orm that referenced this issue Jun 30, 2021
…bug on non-nullable cascading relationship
@rvanlaak
Copy link
Contributor

@lcobucci would PR #8703 prove whether the persistence ordering works correctly?

rvanlaak added a commit to rvanlaak/orm that referenced this issue Jul 1, 2021
…bug on non-nullable cascading relationship
mpdude pushed a commit to mpdude/doctrine2 that referenced this issue Feb 27, 2023
mpdude pushed a commit to mpdude/doctrine2 that referenced this issue Feb 27, 2023
…bug on non-nullable cascading relationship
mpdude pushed a commit to mpdude/doctrine2 that referenced this issue Feb 27, 2023
mpdude pushed a commit to mpdude/doctrine2 that referenced this issue Feb 27, 2023
…bug on non-nullable cascading relationship
mpdude pushed a commit to mpdude/doctrine2 that referenced this issue Feb 28, 2023
mpdude pushed a commit to mpdude/doctrine2 that referenced this issue Feb 28, 2023
…bug on non-nullable cascading relationship
mpdude pushed a commit to mpdude/doctrine2 that referenced this issue Mar 11, 2023
mpdude pushed a commit to mpdude/doctrine2 that referenced this issue Mar 11, 2023
…bug on non-nullable cascading relationship
mpdude pushed a commit to mpdude/doctrine2 that referenced this issue Mar 11, 2023
mpdude pushed a commit to mpdude/doctrine2 that referenced this issue Mar 11, 2023
…bug on non-nullable cascading relationship
mpdude added a commit to mpdude/doctrine2 that referenced this issue Jun 2, 2023
@Frikkle was the first to propose these tests in doctrine#6533.
@rvanlaak followed up in doctrine#8703, making some adjustments.
mpdude added a commit to mpdude/doctrine2 that referenced this issue Jun 2, 2023
@Frikkle was the first to propose these tests in doctrine#6533.
@rvanlaak followed up in doctrine#8703, making some adjustments.

Co-authored-by: Gabe van der Weijde <[email protected]>
Co-authored-by: Richard van Laak <[email protected]>
Co-authored-by: Grégoire Paris <[email protected]>
greg0ire added a commit that referenced this issue Jun 2, 2023
Add tests to show #6499 has been fixed
@greg0ire greg0ire closed this as completed Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment