From f3402cd52867623d1e9b23827c80909552aedac0 Mon Sep 17 00:00:00 2001 From: Maciej Malarz Date: Sun, 23 Feb 2020 11:38:11 +0100 Subject: [PATCH 1/7] Add test case --- .../ORM/Functional/Ticket/GH8031Test.php | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php new file mode 100644 index 00000000000..3ff242ac4b6 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php @@ -0,0 +1,114 @@ +setUpEntitySchema([ + GH8031Invoice::class, + ]); + } + + public function testEntityIsFetched() + { + $entity = new GH8031Invoice(new GH8031InvoiceCode(1, 2020)); + $this->_em->persist($entity); + $this->_em->flush(); + $this->_em->clear(); + + /** @var GH8031Invoice $fetched */ + $fetched = $this->_em->find(GH8031Invoice::class, $entity->getId()); + $this->assertInstanceOf(GH8031Invoice::class, $fetched); + $this->assertSame(1, $fetched->getCode()->getNumber()); + $this->assertSame(2020, $fetched->getCode()->getYear()); + + $this->_em->clear(); + $this->assertCount( + 1, + $this->_em->getRepository(GH8031Invoice::class)->findBy([], ['code.number' => 'ASC']) + ); + } +} + +/** + * @Embeddable + */ +class GH8031InvoiceCode extends GH8031AbstractYearSequenceValue +{ +} + +/** + * @Embeddable + */ +abstract class GH8031AbstractYearSequenceValue +{ + /** + * @Column(type="integer", name="number", length=6) + * @var int + */ + protected $number; + + /** + * @Column(type="smallint", name="year", length=4) + * @var int + */ + protected $year; + + public function __construct(int $number, int $year) + { + $this->number = $number; + $this->year = $year; + } + + public function getNumber() : int + { + return $this->number; + } + + public function getYear() : int + { + return $this->year; + } +} + +/** + * @Entity + */ +class GH8031Invoice +{ + /** + * @Id + * @GeneratedValue + * @Column(type="integer") + */ + private $id; + + /** + * @Embedded(class=GH8031InvoiceCode::class) + * @var GH8031InvoiceCode + */ + private $code; + + public function __construct(GH8031InvoiceCode $code) + { + $this->code = $code; + } + + public function getId() + { + return $this->id; + } + + public function getCode() : GH8031InvoiceCode + { + return $this->code; + } +} From 39dfce6c52d2a07bd37f0fa2434efd7d040a8eb5 Mon Sep 17 00:00:00 2001 From: Maciej Malarz Date: Mon, 24 Feb 2020 22:24:15 +0100 Subject: [PATCH 2/7] Treat parent embeddables as mapped superclasses --- lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php | 5 +++-- lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index 8410ce5b0de..f4015129e22 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -401,7 +401,7 @@ private function getShortName($className) private function addInheritedFields(ClassMetadata $subClass, ClassMetadata $parentClass) { foreach ($parentClass->fieldMappings as $mapping) { - if ( ! isset($mapping['inherited']) && ! $parentClass->isMappedSuperclass) { + if ( ! isset($mapping['inherited']) && ! $parentClass->isMappedSuperclass && ! $parentClass->isEmbeddedClass) { $mapping['inherited'] = $parentClass->name; } if ( ! isset($mapping['declared'])) { @@ -780,7 +780,8 @@ protected function getDriver() */ protected function isEntity(ClassMetadataInterface $class) { - return isset($class->isMappedSuperclass) && $class->isMappedSuperclass === false; + return isset($class->isMappedSuperclass) && $class->isMappedSuperclass === false + && isset($class->isEmbeddedClass) && $class->isEmbeddedClass === false; } /** diff --git a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php index bafd5d27499..28cf958a315 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php @@ -277,6 +277,8 @@ public function loadMetadataForClass($className, ClassMetadata $metadata) /* @var $property \ReflectionProperty */ foreach ($class->getProperties() as $property) { if ($metadata->isMappedSuperclass && ! $property->isPrivate() + || + $metadata->isEmbeddedClass && $property->getDeclaringClass()->getName() !== $class->getName() || $metadata->isInheritedField($property->name) || From 961b20baea94bdb4d5673f05561c5f0ddfbc7779 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 1 Mar 2020 17:05:36 +0100 Subject: [PATCH 3/7] [GH-8031] Bugfix: Get working again on nested embeddables in inherited embeddables. --- .../ORM/Mapping/ClassMetadataFactory.php | 4 --- .../ORM/Functional/Ticket/GH8031Test.php | 32 +++++++++++++++++-- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index f4015129e22..9f379739e6a 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -469,10 +469,6 @@ private function addInheritedEmbeddedClasses(ClassMetadata $subClass, ClassMetad private function addNestedEmbeddedClasses(ClassMetadata $subClass, ClassMetadata $parentClass, $prefix) { foreach ($subClass->embeddedClasses as $property => $embeddableClass) { - if (isset($embeddableClass['inherited'])) { - continue; - } - $embeddableMetadata = $this->getMetadataFor($embeddableClass['class']); $parentClass->mapEmbedded( diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php index 3ff242ac4b6..7b432ff4ac2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php @@ -19,7 +19,7 @@ protected function setUp() public function testEntityIsFetched() { - $entity = new GH8031Invoice(new GH8031InvoiceCode(1, 2020)); + $entity = new GH8031Invoice(new GH8031InvoiceCode(1, 2020, new GH8031Nested(10))); $this->_em->persist($entity); $this->_em->flush(); $this->_em->clear(); @@ -38,6 +38,28 @@ public function testEntityIsFetched() } } +/** + * @Embeddable + */ +class GH8031Nested +{ + /** + * @Column(type="integer", name="number", length=6) + * @var int + */ + protected $number; + + public function __construct(int $number) + { + $this->number = $number; + } + + public function getNumber() : int + { + return $this->number; + } +} + /** * @Embeddable */ @@ -62,10 +84,16 @@ abstract class GH8031AbstractYearSequenceValue */ protected $year; - public function __construct(int $number, int $year) + /** + * @Embedded(class=GH8031Nested::class) + */ + protected $nested; + + public function __construct(int $number, int $year, GH8031Nested $nested) { $this->number = $number; $this->year = $year; + $this->nested = $nested; } public function getNumber() : int From 5f5b3a5dd6e1180483119cf9ec84dd1f677ccd9c Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 1 Mar 2020 17:21:29 +0100 Subject: [PATCH 4/7] Housekeeping: CS --- lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php | 4 ++-- tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index 9f379739e6a..5e57b05d83a 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -401,10 +401,10 @@ private function getShortName($className) private function addInheritedFields(ClassMetadata $subClass, ClassMetadata $parentClass) { foreach ($parentClass->fieldMappings as $mapping) { - if ( ! isset($mapping['inherited']) && ! $parentClass->isMappedSuperclass && ! $parentClass->isEmbeddedClass) { + if (! isset($mapping['inherited']) && ! $parentClass->isMappedSuperclass && ! $parentClass->isEmbeddedClass) { $mapping['inherited'] = $parentClass->name; } - if ( ! isset($mapping['declared'])) { + if (! isset($mapping['declared'])) { $mapping['declared'] = $parentClass->name; } $subClass->addInheritedFieldMapping($mapping); diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php index 7b432ff4ac2..5815791ef99 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php @@ -84,9 +84,7 @@ abstract class GH8031AbstractYearSequenceValue */ protected $year; - /** - * @Embedded(class=GH8031Nested::class) - */ + /** @Embedded(class=GH8031Nested::class) */ protected $nested; public function __construct(int $number, int $year, GH8031Nested $nested) From a8cd2d407498ad8d41f8cd0cd2db2cd38f207ec1 Mon Sep 17 00:00:00 2001 From: Maciej Malarz Date: Mon, 2 Mar 2020 21:04:38 +0100 Subject: [PATCH 5/7] Update note on limitations --- docs/en/tutorials/embeddables.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/en/tutorials/embeddables.rst b/docs/en/tutorials/embeddables.rst index 483e58d9da6..c49bdfc65af 100644 --- a/docs/en/tutorials/embeddables.rst +++ b/docs/en/tutorials/embeddables.rst @@ -8,7 +8,9 @@ or address are the primary use case for this feature. .. note:: - Embeddables can only contain properties with basic ``@Column`` mapping. + Embeddables can not contain references to entities. They can however compose + other embeddables in addition to holding properties with basic ``@Column`` + mapping. For the purposes of this tutorial, we will assume that you have a ``User`` class in your application and you would like to store an address in From 27575b8abb3bde3dfc34ec73b81b35b0510d2159 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Mon, 2 Mar 2020 23:29:27 +0100 Subject: [PATCH 6/7] [GH-8031] Verify assocations still do not work with Embeddables. --- .../ORM/Mapping/ClassMetadataFactory.php | 5 ++-- .../ORM/Functional/Ticket/GH8031Test.php | 27 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index 5e57b05d83a..4f6a2edd532 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -776,8 +776,9 @@ protected function getDriver() */ protected function isEntity(ClassMetadataInterface $class) { - return isset($class->isMappedSuperclass) && $class->isMappedSuperclass === false - && isset($class->isEmbeddedClass) && $class->isEmbeddedClass === false; + assert($class instanceof ClassMetadata); + + return $class->isMappedSuperclass === false && $class->isEmbeddedClass === false; } /** diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php index 5815791ef99..37dd4d38b50 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php @@ -36,6 +36,28 @@ public function testEntityIsFetched() $this->_em->getRepository(GH8031Invoice::class)->findBy([], ['code.number' => 'ASC']) ); } + + public function testEmbeddableWithAssociationNotAllowed() + { + $cm = $this->_em->getClassMetadata(GH8031EmbeddableWithAssociation::class); + + $this->assertArrayHasKey('invoice', $cm->associationMappings); + + $cm = $this->_em->getClassMetadata(GH8031Invoice::class); + + $this->assertCount(0, $cm->associationMappings); + } +} + +/** + * @Embeddable + */ +class GH8031EmbeddableWithAssociation +{ + /** + * @ManyToOne(targetEntity=GH8031Invoice::class) + */ + public $invoice; } /** @@ -123,6 +145,11 @@ class GH8031Invoice */ private $code; + /** + * @Embedded(class=GH8031EmbeddableWithAssociation::class) + */ + private $embeddedAssoc; + public function __construct(GH8031InvoiceCode $code) { $this->code = $code; From e2ead2e7e05388abdd25933833e3fc56d5614d18 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Mon, 2 Mar 2020 23:47:21 +0100 Subject: [PATCH 7/7] Housekeeping: CS --- lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php | 1 + tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php | 8 ++------ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index 4f6a2edd532..e1142e41bf1 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -31,6 +31,7 @@ use Doctrine\ORM\Id\IdentityGenerator; use Doctrine\ORM\ORMException; use ReflectionException; +use function assert; /** * The ClassMetadataFactory is used to create ClassMetadata objects that contain all the diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php index 37dd4d38b50..7ae17d3f2f8 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8031Test.php @@ -54,9 +54,7 @@ public function testEmbeddableWithAssociationNotAllowed() */ class GH8031EmbeddableWithAssociation { - /** - * @ManyToOne(targetEntity=GH8031Invoice::class) - */ + /** @ManyToOne(targetEntity=GH8031Invoice::class) */ public $invoice; } @@ -145,9 +143,7 @@ class GH8031Invoice */ private $code; - /** - * @Embedded(class=GH8031EmbeddableWithAssociation::class) - */ + /** @Embedded(class=GH8031EmbeddableWithAssociation::class) */ private $embeddedAssoc; public function __construct(GH8031InvoiceCode $code)