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

Updating entity with combined relation identifier fails #6461

Closed
digilist opened this issue May 22, 2017 · 9 comments
Closed

Updating entity with combined relation identifier fails #6461

digilist opened this issue May 22, 2017 · 9 comments
Assignees

Comments

@digilist
Copy link

Consider the following schema:

  • Entity Owner which has items
  • Entity Field
  • Entity Item which belong to an owner and a field

I am creating a new owner with a new item (that belongs to an existing field) which is persisted and flushed into the database. When I now want to change a field on Item and flush again, that change is not saved correctly, because of the combined identifier. The following SQL statement is generated (I already inserted the parameters):

UPDATE Item SET string = "test" WHERE owner_id = null AND field_id = 1

So Doctrine tries to execute the Update with owner_id = null, even though the owner is set and was persisted / flushed before (the owner on the item has an id).

I will add a functional test case in a comment that you can use to reproduce this behavior, which hopefully explains what I mean.

@digilist
Copy link
Author

<?php

namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection;

/**
 * @group DDC-6461
 */
class DDC6461Test extends \Doctrine\Tests\OrmFunctionalTestCase
{
    public function setUp()
    {
        parent::setUp();
        parent::setUpEntitySchema(array
        (
            __NAMESPACE__ . '\DDC6461Owner',
            __NAMESPACE__ . '\DDC6461Item',
            __NAMESPACE__ . '\DDC6461Field',
        ));
    }
    /**
     * @group DDC-1234
     */
    public function testCombinedRelation()
    {
        $field = new DDC6461Field();
        $this->_em->persist($field);
        $this->_em->flush();

        $owner = new DDC6461Owner();
        $owner->addItem($item = new DDC6461Item($field));
        $item->string = 'hello world';

        $this->_em->persist($owner);
        $this->_em->flush();

        $item->string = 'test';
        $this->_em->flush();

        $this->_em->clear(); // Force reloading the item from the database.
        $owner = $this->_em->find(DDC6461Owner::class, $owner->id);
        $this->assertEquals('test', $owner->items[0]->string); // String is still hello world
    }
}

/**
 * @Entity()
 */
class DDC6461Owner
{
    /**
     * @Id()
     * @Column(type="integer")
     * @GeneratedValue()
     * @var int
     */
    public $id;

    /**
     * @OneToMany(targetEntity="DDC6461Item", mappedBy="owner", cascade={"persist"})
     * @var DDC6461Item[]|Collection
     */
    public $items;

    public function __construct()
    {
        $this->items = new ArrayCollection();
    }

    public function addItem(DDC6461Item $item)
    {
        $item->owner = $this;
        $this->items->add($item);
    }
}

/**
 * @Entity()
 */
class DDC6461Item
{
    /**
     * @Id()
     * @ManyToOne(targetEntity="DDC6461Owner", inversedBy="items")
     * @var DDC6461Owner
     */
    public $owner;

    /**
     * @Id()
     * @ManyToOne(targetEntity="DDC6461Field")
     * @var DDC6461Field
     */
    public $field;

    /**
     * @Column()
     * @var string
     */
    public $string = '';

    /**
     * @param DDC6461Field $field
     */
    public function __construct(DDC6461Field $field)
    {
        $this->field = $field;
    }
}

/**
 * @Entity()
 */
class DDC6461Field
{
    /**
     * @Id()
     * @Column(type="integer")
     * @GeneratedValue()
     * @var int
     */
    public $id;
}

@lcobucci
Copy link
Member

lcobucci commented May 22, 2017

@digilist this is the exact same case of #4584. What happens is that while persisting the identifier is inexistent, since it's generated AFTER the insert.

As explained on the last comment of that issue, I think that it's a limitation and if users want to use entities as identifier they should make sure that the ID is generated before of the insertion (using a UUID for example).

Maybe we could do something about that on v3.x but I'd say we shouldn't change that behaviour on v2.x.

@digilist
Copy link
Author

Thank you. It looks similar, but not the same. In my case, the insertion into the database works properly, but updating the element later fails.

If this could not be fixed in 2.x anymore, I'd be happy if this can be considered for 3.x.

@lcobucci
Copy link
Member

@digilist well I debugged the test case you provided and it fails while processing the cascade insertion of DDC6461Owner, since DDC6461Item has it as part of the identifier (and a generated value):

$owner = new DDC6461Owner();
$owner->addItem($item = new DDC6461Item($field));
$item->string = 'hello world';
$this->_em->persist($owner);

That's why I said it was the same case 😉

@digilist
Copy link
Author

Interesting, here it worked properly and failed only at the assertion. I tested it with the 2.5 branch, as master is the current 3.0 development, isn't it?

@lcobucci
Copy link
Member

lcobucci commented May 23, 2017 via email

@digilist
Copy link
Author

Oh, I see. I just tested it again, and it looks like my code is working (until the Item is saved into the database) with 2.5 but not with 2.6 anymore.

Isn't this a BC break then? (But I see the problem here, so I'd understand if this was accepted).

@Ocramius
Copy link
Member

Isn't this a BC break then?

The code wouldn't work anyway, it's just that you get a better/more meaningful behavior in 2.6 (you see a failure)

@ostrolucky
Copy link
Member

Since issue can no longer be reproduced in newer versions, closing.

@ostrolucky ostrolucky self-assigned this Aug 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants