Fix ID generation of foreign keys #7054
Conversation
|
@cb8 thanks for your patch, we just merged in the fix for 2.6 and were porting it to master. I'll check your changes to see if it works better for the new version of the code. |
|
thank you for the review. the deferred IDs were inadvertently flattened when they were added to the original data of the entity. as far as i could see, the failing tests on Travis are not caused by the patch (but don't hold me to that ;)). |
| /** @var LocalColumnMetadata[] $generatedProperties */ | ||
| $generatedProperties = []; | ||
| /** @var ValueGenerationExecutor[] */ | ||
| $executors = []; |
There was a problem hiding this comment.
We could extract the generation of this variable to a private method, it would make things more readable
| use Doctrine\ORM\Mapping\LocalColumnMetadata; | ||
| use Doctrine\ORM\Sequencing\Generator; | ||
|
|
||
| class AssocationValueGeneratorExecutor implements ValueGenerationExecutor |
There was a problem hiding this comment.
Typo here
- class AssocationValueGeneratorExecutor
+ class AssociationValueGeneratorExecutorThere was a problem hiding this comment.
This class could be final, we definitely don't want people inheriting from it
|
|
||
| class AssocationValueGeneratorExecutor implements ValueGenerationExecutor | ||
| { | ||
| public function execute(EntityManagerInterface $entityManager, /*object*/ $entity) : array |
There was a problem hiding this comment.
Since we use PHP 7.2 we can use object as type... I've fixed the interface in #7092
|
|
||
| $this->entityIdentifiers[$oid] = $id; | ||
| $this->entityIdentifiers[$oid] = $this->em->getIdentifierFlattener()->flattenIdentifier($class, $id); |
There was a problem hiding this comment.
@guilhermeblanco this looks ok, but I'd like your eyes here
Tests are based on examples from "Composite and Foreign Keys as Primary Key" tutorial: http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/tutorials/composite-primary-keys.html
When setting deferred ID values, the original data should contain complete objects (as it would with non-deferred IDs).
|
@cb8 🚢 thanks for your contribution! |
this patch is an alternate approach to #7003 which tries to fix #7002. however, it applies to the
masterbranch. so far, i didn't clean up my test cases. in case this patch will get merged despite/instead of #7003, i will push them for review.