[DDC-518] Merging an entity with a one to one association to a MANAGED entity with no id throws 'The given entity has no identity.' Created: 13/Apr/10  Updated: 30/Jul/10  Resolved: 30/Jul/10

Status: Closed
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: None
Fix Version/s: 2.0-BETA3
Security Level: All

Type: Bug Priority: Critical
Reporter: Dave Keen Assignee: Roman S. Borschel
Resolution: Fixed Votes: 0
Labels: None

Attachments: Text File ddc518.patch    

 Description   

Calling merge($entity) where $entity has a one to one association to another entity that has been persisted but not yet flushed (when using auto-generated ids) throws 'The given entity has no identity.'

It looks like it does this because _doMerge in UnitOfWork assumes for one to one associations only that the associated entity has an id and calls registerManaged, which then calls addToIdentityMap)on it.

I think that registeredManaged should only be called if !isset($this->_entityInsertions[spl_object_hash($other)])

Reproduce.php
// This is a new element
$doctor = new \vo\Doctor(); $d1->name = "New doctor";

// This is a detached element which is in the database
$patient = new \vo\Patient(); $p1->name = "Existing patient"; $p1->id = 1;

$doctor->patients->add($patient);
$patient ->doctor = $doctor;

$em->persist($doctor);

// This throws InvalidArgumentException: The given entity has no identity. in D:\Projects\ORM\flextrine2\flextrine\web\lib\Doctrine\ORM\UnitOfWork.php on line 1014
$em->merge($patient);
Doctor.php
class Doctor {

    /** @Id @Column(type="integer") @GeneratedValue(strategy="IDENTITY") */
    public $id;
	
    /** @Column(length=100, type="string") */
    public $name;
	
	/**
     * @OneToMany(targetEntity="Patient", mappedBy="doctor", fetch="EAGER")
     */
	public $patients;
	
	public function __construct() {
		$this->patients = new ArrayCollection();
	}
	
}
Patient.php
class Patient {
	
	var $_explicitType = "vo/Patient";
	
    /** @Id @Column(type="integer") @GeneratedValue(strategy="IDENTITY") */
    public $id;
	
    /** @Column(length=100, type="string") */
    public $name;

	/**
     * @OneToOne(targetEntity="Doctor", inversedBy="patients")
	 * @JoinColumn(name="doctor_id", referencedColumnName="id")
     */
	public $doctor;
	
}


 Comments   
Comment by Roman S. Borschel [ 08/May/10 ]

I think the order of operations in your example is not correct even though the error is misleading.

You are associating a "new doctor" to a "detached patient". That does not seem right, remember that merge() returns a managed copy, thus when merging the patient later, the "new doctor" is still associated with the "detached patient", not with the managed one.

The following should work:

// Merge detached patient
$managedPatient = $em->merge($patient);

// Associate new doctor with patient
$doctor->patients->add($managedPatient);
$managedPatient->doctor = $doctor;

// Persist new doctor and flush
$em->persist($doctor);
$em->flush();
Comment by Dave Keen [ 16/May/10 ]

You are quite right - that does work.

However, I am now trying to implement my use case (turning a tree of detached objects into a tree of managed objects) and implementing the correct order you describe above seems to cause another problem. I am not sure if this should be another ticket, but I'll put it here for the moment.

Note that the part within the stars that creates the unmanaged doctor and the detached patient simulates exactly what is received by Doctrine in my application so I can't put any merges in here.

/********************************************************************/
// This is a new element
$doctor = new \vo\Doctor(); $doctor->name = "New doctor";

// This is a detached element which is in the database
$patient = new \vo\Patient(); $patient->name = "Existing patient"; $patient->id = 1;

$doctor->patients->add($patient); $patient->doctor = $doctor;
/********************************************************************/

// Now replace $patient with its managed version
$managedPatient = $em->merge($patient);
$doctor->patients->set(0, $managedPatient);

// Persist the doctor
$em->persist($doctor);

$em->flush();

This works up to the flush, which throws an error of the form:

Notice: Undefined index: 000000007dd346c3000000005d0908d2 in \Doctrine\ORM\UnitOfWork.php on line 1955

In the database this results in a new doctor being created, but doctor_id in the patients table is set to NULL for the patient that is supposed to be linking to it.

Comment by Benjamin Eberlei [ 06/Jun/10 ]

I think this can't work, because obviously $doctor->patients still points to the unmanaged $patient, not the managed one.

Can you elaborate on why the star part is done by Doctrine? Where is doctrine doing that? what are you doing with the public API?

All the detached entities should be merged before doing anything regarding a NEW entity in my opinion.

Comment by Benjamin Eberlei [ 06/Jun/10 ]

Moved to beta3 for now

Comment by Benjamin Eberlei [ 06/Jun/10 ]

Patch with test-case for this merging scenario

Comment by Roman S. Borschel [ 30/Jul/10 ]

Fixed in http://github.com/doctrine/doctrine2/commit/69073c4b37ee28f988306db4965f512b70f45181

Generated at Fri Aug 01 22:49:06 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.