Doctrine 2 - ORM

UnitOfWork tryGetById method is always called with the rootEntityName

Details

  • Type: Bug Bug
  • Status: Resolved 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

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
Benjamin Eberlei added a comment -

Fixed.

Show
Benjamin Eberlei added a comment - Fixed.

People

  • Assignee:
    Benjamin Eberlei
    Reporter:
    Couragier Sébastien
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: