Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-1041

UnitOfWork tryGetById method is always called with the rootEntityName

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0.2, 2.1
    • Component/s: ORM
    • Security Level: All
    • Labels:
      None
    • Environment:
      LAMP

      Description

      My problem : i have a class table inheritance, with only 3 class, one abstract and two concrete.
      says AbstractClass, ConcretClassA and ConcreteClassB
      All three are declared with @entity

      if i already have in my identity map the ConcretClassA with id = 1
      when i do an entityManager->find('ConcreteClassB', 1) the identityMap returns me the ConcretClassA with id = 1
      that's not correct !
      it should return a null value instead

      That's because the entityRepository (and all the other doctrine class) call the UnitOfWork tryGetById with the rootEntityName, which in my case is AbstractClass.
      See the first line of the entityRepository's find method :

      $this->_em->getUnitOfWork()->tryGetById($id, $this->_class->rootEntityName)
      

      if i change this line to :

      $this->_em->getUnitOfWork()->tryGetById($id, $this->_class->name)
      

      the find method return the expected null value.

      So, why the UnitOfWork tryGetById method is always called with the rootEntityName ?

        Activity

        Hide
        Benjamin Eberlei added a comment -

        The identity map HAS to work by root entity name.

        Otherwise think of an inheritance hierachy A > B. Then both $em>find('A', 1); and $em->find('B', 1); should return the same instance.

        If this does not hold you are screwed.

        Your problem is more subtle, i am not sure what the expected behavior should be here.

        Show
        Benjamin Eberlei added a comment - The identity map HAS to work by root entity name. Otherwise think of an inheritance hierachy A > B. Then both $em >find('A', 1); and $em->find('B', 1); should return the same instance. If this does not hold you are screwed. Your problem is more subtle, i am not sure what the expected behavior should be here.
        Hide
        Couragier Sébastien added a comment -

        In my sense, the client code should exactly knows the inheritance hierachy, and exactly know what it wants too, so if it asks for an 'A' instance, you can't return a 'B' instance, although A inherit from B.

        Show
        Couragier Sébastien added a comment - In my sense, the client code should exactly knows the inheritance hierachy, and exactly know what it wants too, so if it asks for an 'A' instance, you can't return a 'B' instance, although A inherit from B.
        Hide
        Benjamin Eberlei added a comment -

        I agree with you except for the last sentence part.

        If A->B and you query for B with Id 1 it should treturn an A if it exists. But if B->C and A->C then find(A) should only ever return As or Cs never, Bs.

        Show
        Benjamin Eberlei added a comment - I agree with you except for the last sentence part. If A->B and you query for B with Id 1 it should treturn an A if it exists. But if B->C and A->C then find(A) should only ever return As or Cs never, Bs.
        Hide
        Couragier Sébastien added a comment -

        I agree.
        Perhaps my problem is more specific to the class table inheritance strategy with an abstract class at the top of it.
        Here's my identityMap's dump (with my first comment inheritance example) at the time the problem occurs :

        'AbstractClass' =>
        array
        1 => string 'ConcretClassA'
        2 => string 'ConcretClassA'
        3 => string 'ConcretClassA'
        367 => string 'ConcretClassB'
        368 => string 'ConcretClassB'
        369 => string 'ConcretClassB'

        And, at this time, in my code, i would like to know if the id 1 is a ConcretClassB. If the id 1 is a ConcretClassA, i must throw an exception.
        So, how could i test if the id 1 is a ConcretClassB or not, if the find('ConcretClassB', 1) method returns me the ConcretClassA 1 ?

        thx for your help

        Show
        Couragier Sébastien added a comment - I agree. Perhaps my problem is more specific to the class table inheritance strategy with an abstract class at the top of it. Here's my identityMap's dump (with my first comment inheritance example) at the time the problem occurs : 'AbstractClass' => array 1 => string 'ConcretClassA' 2 => string 'ConcretClassA' 3 => string 'ConcretClassA' 367 => string 'ConcretClassB' 368 => string 'ConcretClassB' 369 => string 'ConcretClassB' And, at this time, in my code, i would like to know if the id 1 is a ConcretClassB. If the id 1 is a ConcretClassA, i must throw an exception. So, how could i test if the id 1 is a ConcretClassB or not, if the find('ConcretClassB', 1) method returns me the ConcretClassA 1 ? thx for your help
        Hide
        Benjamin Eberlei added a comment -

        Cant you just check instanceof?

        $a = $em->find('ConcretClassA', $aId);
        if ($a instanceof ConcretClassB) {
           throw new Exception();
        }
        
        Show
        Benjamin Eberlei added a comment - Cant you just check instanceof? $a = $em->find('ConcretClassA', $aId); if ($a instanceof ConcretClassB) { throw new Exception(); }
        Hide
        Couragier Sébastien added a comment -

        Of course i can.
        But i thought Doctrine should already have to do this check internally for us. Because we must now be very careful with the find method's returns...

        Nevermind, i don't have all the uses case Doctrine have to handle with inheritance in mind; so if you say it is to the client code to check, and it is a completely normal behavior, then it's okay for me.

        Show
        Couragier Sébastien added a comment - Of course i can. But i thought Doctrine should already have to do this check internally for us. Because we must now be very careful with the find method's returns... Nevermind, i don't have all the uses case Doctrine have to handle with inheritance in mind; so if you say it is to the client code to check, and it is a completely normal behavior, then it's okay for me.
        Hide
        Benjamin Eberlei added a comment -

        i will change this behavior in the future, i am just making suggestions for you to fix it now. Using instanceof will also be forwards compatible since "null instanceof Class" is always false.

        In any case the level of having to be careful is the same, using null instead of an object can get you into troubles aswell.

        Show
        Benjamin Eberlei added a comment - i will change this behavior in the future, i am just making suggestions for you to fix it now. Using instanceof will also be forwards compatible since "null instanceof Class" is always false. In any case the level of having to be careful is the same, using null instead of an object can get you into troubles aswell.
        Hide
        Couragier Sébastien added a comment -

        Perhaps throw a \Doctrine\ORM\EntityNotFoundException would be a better option than return a null value ?

        Show
        Couragier Sébastien added a comment - Perhaps throw a \Doctrine\ORM\EntityNotFoundException would be a better option than return a null value ?
        Hide
        Benjamin Eberlei added a comment -

        Fixed.

        Show
        Benjamin Eberlei added a comment - Fixed.

          People

          • Assignee:
            Benjamin Eberlei
            Reporter:
            Couragier Sébastien
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: