Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. 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.'

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0-BETA3
    • Component/s: ORM
    • Security Level: All
    • Labels:
      None

      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;
      	
      }
      
      1. ddc518.patch
        3 kB
        Benjamin Eberlei

        Activity

        Hide
        Roman S. Borschel added a comment -

        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();
        
        Show
        Roman S. Borschel added a comment - 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();
        Hide
        Dave Keen added a comment -

        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.

        Show
        Dave Keen added a comment - 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.
        Hide
        Benjamin Eberlei added a comment -

        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.

        Show
        Benjamin Eberlei added a comment - 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.
        Hide
        Benjamin Eberlei added a comment -

        Moved to beta3 for now

        Show
        Benjamin Eberlei added a comment - Moved to beta3 for now
        Hide
        Benjamin Eberlei added a comment -

        Patch with test-case for this merging scenario

        Show
        Benjamin Eberlei added a comment - Patch with test-case for this merging scenario
        Show
        Roman S. Borschel added a comment - Fixed in http://github.com/doctrine/doctrine2/commit/69073c4b37ee28f988306db4965f512b70f45181

          People

          • Assignee:
            Roman S. Borschel
            Reporter:
            Dave Keen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: