doctrine / doctrine-laminas-hydrator

Doctrine hydrators for Laminas applications
https://www.doctrine-project.org/projects/doctrine-laminas-hydrator.html
MIT License
33 stars 19 forks source link

hydrate composite PK #9

Closed rodsouto closed 4 years ago

rodsouto commented 4 years ago

Hi! I want to hydrate an entity with a composite PK, the problem is that this line removes those values

https://github.com/rodsouto/doctrine-laminas-hydrator/blob/master/src/DoctrineObject.php#L485

I tried commenting out that line and it works, but there is one test failing with Error: Cannot access protected property DoctrineTest\Laminas\Hydrator\Assets\ByValueDifferentiatorEntity::$id

I think thats happening because in DoctrineObject::hydrateByReference() it is calling $reflProperty->setValue() but $object is not the same class as $metadata->getReflectionClass()

If in line 339 I replace $refl = $metadata->getReflectionClass(); with $refl = new \ReflectionClass($object) then all tests are green again

So, is using $refl = new \ReflectionClass($object) a valid fix to get the composite PK working? or is there a better way?

TomHAnderson commented 4 years ago

Why isn't $metadata->getReflectionClass()? Can we solve that to get the same value as a new \ReflectionClass?

rodsouto commented 4 years ago

Ok, I think the code works ok without line #L485 and the problem is just testHydrateOneToManyAssociationByReferenceUsingIdentifiersArrayForRelations missing a expectation to return the correct mock for $this->objectManager->getClassMetadata(get_class($object)) in https://github.com/doctrine/doctrine-laminas-hydrator/blob/master/src/DoctrineObject.php#L136

TomHAnderson commented 4 years ago

Can you put that into a PR?

rodsouto commented 4 years ago

Yes, I'm trying to figure out how to fix it without changing too much tests

The problem with testHydrateOneToManyAssociationByReferenceUsingIdentifiersArrayForRelations is that if you keep the PK, it calls an aditional hydrate() in https://github.com/doctrine/doctrine-laminas-hydrator/blob/master/src/DoctrineObject.php#L496 (that's why it's failing only in ByReferenceUsingIdentifiersArray)

In this specific test case (by reference + array identifier) we need to mock two metadata in configureObjectManagerForOneToManyEntity (currently it's only mocking $this->metadata for OneToManyEntity but we also need a mock for ByValueDifferentiatorEntity), but I think that requires to do quite a big refactor in the test case :/

TomHAnderson commented 4 years ago

@rodsouto this repo has migrated to Doctrine 3.0. Are you still going to submit a PR?

rodsouto commented 4 years ago

Not in the short term, you can close this if you like