[DDC-3378] Support merging entities with composite identities defined through to-one associations

[adrienbrault]

I hope that I'm doing something wrong and that this is not a bug ...

[doctrinebot]

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-3378

We use Jira to track the state of pull requests and the versions they got
included in.

[Ocramius]

@adrienbrault there is a class naming collision in the test case.

[adrienbrault]

Note that the following works:

$country = new MergeCompositeToOneKeyCountry();
$country->country = 'US';
$state = new MergeCompositeToOneKeyState();
$state->state = 'CA';
$state->country = $country;

$newCountry = $this->_em->merge($country);
$state->country = $newCountry;
$this->_em->merge($state);

[stof]

Well, merge() does not make the argument it receives managed. It returns a new instance which is managed (because it might already have it in its identity map).

So you issue is a wrong usage of merge()

[stof]

note that Doctrine can be configured to cascade the merge operation, which could help you in this case

[adrienbrault]

@stof I've updated the test to a valid use case

[adrienbrault]

The issue is that the cascading merges happens after uow tries to get the State id, and at that point the Country is not managed

[adrienbrault]

@Ocramius Updated

[stof]

removing this check looks wrong to me. It is expected to enforce using managed entities

[stof]

The issue when you remove this is that you now allow using unmanaged entities in identifiers even in contexts where it should not be allowed. Fixing the retrieval of the id when merging should not be done by opening all other cases for bugs.

[Ocramius]

I think this is actually correct though - identities deriving from associations are still valid, even for un-managed entities.

[stof]

hmm, indeed. The relation itslef will still be required to use a managed entity in other places

[hjr3]

I can reproduce this with the following use case:

    /** @var $obj \Doctrine\Tests\ORM\Functional\MergeCompositeToOneKeyState */
    $doctrine->detach($obj); 
    $em->clear(); 
    $doctrine->merge($obj);

The merge code tries to flatten a composite key with a foreign identifier but requires the entity identifier values that were just cleared.

[Ocramius]

@hjr3 does this apply also to the codebase after @adrienbrault's patch?

[hjr3]

@Ocramius the patch from @adrienbrault does address my issue.

[Ocramius]

@adrienbrault can you check @stof's comment please?

[Ocramius]

@adrienbrault merged, thanks!

master: b889e18