Merge pull request #1973 from soyuka/merge-2.2

Merge 2.2 (couple eager loading fixes + conflicts due to cs fixes)

Author: soyuka

features/main/circular_reference.feature

@@ -60,10 +60,29 @@ Feature: Circular references handling
           "@type": "CircularReference",
           "parent": "/circular_references/1",
           "children": [
-              "/circular_references/1",
-              "/circular_references/2"
+              "/circular_references/1"
           ]
       },
       "children": []
     }
     """
+    And I send a "GET" request to "/circular_references/1"
+    Then the response status code should be 200
+    And the JSON should be equal to:
+    """
+    {
+        "@context": "/contexts/CircularReference",
+        "@id": "/circular_references/1",
+        "@type": "CircularReference",
+        "parent": "/circular_references/1",
+        "children": [
+            "/circular_references/1",
+            {
+                "@id": "/circular_references/2",
+                "@type": "CircularReference",
+                "parent": "/circular_references/1",
+                "children": []
+            }
+        ]
+    }
+    """

features/main/operation.feature

@@ -12,3 +12,24 @@ Feature: Operation support
     """
     "This is a custom action for 42."
     """
+
+  @createSchema
+  @dropSchema
+  Scenario: Select a resource and it's embedded data
+    Given there are 1 embedded dummy objects
+    When I send a "GET" request to "/embedded_dummies_groups/1"
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
+    And the JSON should be equal to:
+    """
+    {
+        "@context": "/contexts/EmbeddedDummy",
+        "@id": "/embedded_dummies/1",
+        "@type": "EmbeddedDummy",
+        "name": "Dummy #1",
+        "embeddedDummy": {
+            "dummyName": "Dummy #1"
+        }
+    }
+    """

src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php

@@ -207,9 +207,13 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt
                 $queryBuilder->addSelect($associationAlias);
             }
 
-            // Avoid recursion
+            // Avoid recursive joins
             if ($mapping['targetEntity'] === $resourceClass) {
-                $queryBuilder->addSelect($associationAlias);
+                // Avoid joining the same association twice (see #1959)
+                if (!\in_array($associationAlias, $queryBuilder->getAllAliases(), true)) {
+                    $queryBuilder->addSelect($associationAlias);
+                }
+
                 continue;
             }
 
@@ -245,15 +249,17 @@ private function addSelect(QueryBuilder $queryBuilder, string $entity, string $a
                 continue;
             }
 
-            //the field test allows to add methods to a Resource which do not reflect real database fields
-            if ($targetClassMetadata->hasField($property) && (true === $propertyMetadata->getAttribute('fetchable') || $propertyMetadata->isReadable())) {
-                $select[] = $property;
-            }
-
+            // If it's an embedded property see below
             if (!array_key_exists($property, $targetClassMetadata->embeddedClasses)) {
+                //the field test allows to add methods to a Resource which do not reflect real database fields
+                if ($targetClassMetadata->hasField($property) && (true === $propertyMetadata->getAttribute('fetchable') || $propertyMetadata->isReadable())) {
+                    $select[] = $property;
+                }
+
                 continue;
             }
 
+            // It's an embedded property, select relevent subfields
             foreach ($this->propertyNameCollectionFactory->create($targetClassMetadata->embeddedClasses[$property]['class']) as $embeddedProperty) {
                 $propertyMetadata = $this->propertyMetadataFactory->create($entity, $property, $propertyMetadataOptions);
                 $propertyName = "$property.$embeddedProperty";

tests/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php

@@ -97,8 +97,8 @@ public function testApplyToCollection()
         $relatedClassMetadataProphecy = $this->prophesize(ClassMetadata::class);
 
         foreach ($relatedNameCollection as $property) {
-            if ('id' !== $property) {
-                $relatedClassMetadataProphecy->hasField($property)->willReturn(!\in_array($property, ['notindatabase', 'embeddedDummy'], true))->shouldBeCalled();
+            if ('id' !== $property && 'embeddedDummy' !== $property) {
+                $relatedClassMetadataProphecy->hasField($property)->willReturn(!\in_array($property, ['notindatabase'], true))->shouldBeCalled();
             }
         }
         $relatedClassMetadataProphecy->hasField('embeddedDummy.name')->willReturn(true)->shouldBeCalled();
@@ -183,8 +183,8 @@ public function testApplyToItem()
         $relatedClassMetadataProphecy = $this->prophesize(ClassMetadata::class);
 
         foreach ($relatedNameCollection as $property) {
-            if ('id' !== $property) {
-                $relatedClassMetadataProphecy->hasField($property)->willReturn(!\in_array($property, ['notindatabase', 'embeddedDummy'], true))->shouldBeCalled();
+            if ('id' !== $property && 'embeddedDummy' !== $property) {
+                $relatedClassMetadataProphecy->hasField($property)->willReturn(!\in_array($property, ['notindatabase'], true))->shouldBeCalled();
             }
         }
         $relatedClassMetadataProphecy->hasField('embeddedDummy.name')->willReturn(true)->shouldBeCalled();

tests/Fixtures/TestBundle/Entity/EmbeddableDummy.php

@@ -30,6 +30,7 @@ class EmbeddableDummy
      * @var string The dummy name
      *
      * @ORM\Column(nullable=true)
+     * @Groups({"embed"})
      */
     private $dummyName;
 

tests/Fixtures/TestBundle/Entity/EmbeddedDummy.php

@@ -15,14 +15,18 @@
 
 use ApiPlatform\Core\Annotation\ApiResource;
 use Doctrine\ORM\Mapping as ORM;
+use Symfony\Component\Serializer\Annotation\Groups;
 use Symfony\Component\Validator\Constraints as Assert;
 
 /**
  * Embedded Dummy.
  *
  * @author Jordan Samouh <[email protected]>
  *
- * @ApiResource(attributes={"filters"={"my_dummy.search", "my_dummy.order", "my_dummy.date", "my_dummy.range", "my_dummy.boolean", "my_dummy.numeric"}})
+ * @ApiResource(
+ *     attributes={"filters"={"my_dummy.search", "my_dummy.order", "my_dummy.date", "my_dummy.range", "my_dummy.boolean", "my_dummy.numeric"}},
+ *     itemOperations={"get", "put", "delete", "groups"={"method"="GET", "path"="/embedded_dummies_groups/{id}", "normalization_context"={"groups"={"embed"}}}}
+ * )
  * @ORM\Entity
  */
 class EmbeddedDummy
@@ -40,6 +44,7 @@ class EmbeddedDummy
      * @var string The dummy name
      *
      * @ORM\Column(nullable=true)
+     * @Groups({"embed"})
      */
     private $name;
 
@@ -55,6 +60,7 @@ class EmbeddedDummy
      * @var EmbeddableDummy
      *
      * @ORM\Embedded(class="EmbeddableDummy")
+     * @Groups({"embed"})
      */
     public $embeddedDummy;