refactor: prevent repeating entity hydration in MtM relation (#188)

* refactor: prevent repeating entity hydration in MtM relation

* Apply suggestions from code review

Co-authored-by: Aleksei Gagarin <[email protected]>

* test: replace heap instead of last save call

Co-authored-by: Aleksei Gagarin <[email protected]>

Author: tntrex

src/Iterator.php

@@ -22,21 +22,26 @@ final class Iterator implements \IteratorAggregate
     private $orm;
 
     /** @var string */
-    private $class;
+    private $role;
 
     /** @var iterable */
     private $source;
 
+    /** @var bool */
+    private $tryToFindInHeap;
+
     /**
      * @param ORMInterface $orm
-     * @param string       $class
-     * @param iterable     $source
+     * @param string $class
+     * @param iterable $source
+     * @param bool $tryToFindInHeap
      */
-    public function __construct(ORMInterface $orm, string $class, iterable $source)
+    public function __construct(ORMInterface $orm, string $class, iterable $source, bool $tryToFindInHeap = false)
     {
         $this->orm = $orm;
-        $this->class = $class;
+        $this->role = $this->orm->resolveRole($class);
         $this->source = $source;
+        $this->tryToFindInHeap = $tryToFindInHeap;
     }
 
     /**
@@ -57,7 +62,23 @@ public function getIterator(): \Generator
 
             // add pipeline filter support?
 
-            yield $index => $this->orm->make($this->class, $data, Node::MANAGED);
+            yield $index => $this->getEntity($data);
+        }
+    }
+
+    private function getEntity(array $data)
+    {
+        if ($this->tryToFindInHeap) {
+            $pk = $this->orm->getSchema()->define($this->role, SchemaInterface::PRIMARY_KEY);
+            $id = $data[$pk] ?? null;
+
+            if (null !== $id) {
+                $e = $this->orm->getHeap()->find($this->role, [
+                    $pk => $id,
+                ]);
+            }
         }
+
+        return $e ?? $this->orm->make($this->role, $data, Node::MANAGED);
     }
 }

src/Relation/ManyToMany.php

@@ -56,7 +56,7 @@ public function init(Node $node, array $data): array
         $elements = [];
         $pivotData = new \SplObjectStorage();
 
-        $iterator = new Iterator($this->orm, $this->target, $data);
+        $iterator = new Iterator($this->orm, $this->target, $data, true);
         foreach ($iterator as $pivot => $entity) {
             if (!is_array($pivot)) {
                 // skip partially selected entities (DB level filter)

src/Relation/Pivoted/PivotedPromise.php

@@ -143,7 +143,8 @@ public function __resolve()
 
         $elements = [];
         $pivotData = new \SplObjectStorage();
-        foreach (new Iterator($this->orm, $this->target, $root->getResult()[0]['output']) as $pivot => $entity) {
+        $iterator = new Iterator($this->orm, $this->target, $root->getResult()[0]['output'], true);
+        foreach ($iterator as $pivot => $entity) {
             $pivotData[$entity] = $this->orm->make(
                 $this->relationSchema[Relation::THROUGH_ENTITY],
                 $pivot,

tests/ORM/Fixtures/RbacItemAbstract.php

@@ -14,6 +14,9 @@ class RbacItemAbstract
      */
     public $name;
 
+    /** @var string|null */
+    public $description;
+
     /**
      * @var DoctrineCollection|RbacRole[]|RbacPermission[]
      * @phpstan-var DoctrineCollection<string,RbacRole|RbacPermission>
@@ -26,9 +29,10 @@ class RbacItemAbstract
      */
     public $children;
 
-    public function __construct(string $name)
+    public function __construct(string $name, string $description = null)
     {
         $this->name = $name;
+        $this->description = $description;
 
         $this->parents = new ArrayCollection();
         $this->children = new ArrayCollection();

tests/ORM/ManyToManySingleEntityTest.php

@@ -8,7 +8,6 @@
 use Cycle\ORM\Mapper\Mapper;
 use Cycle\ORM\Mapper\StdMapper;
 use Cycle\ORM\Relation;
-use Cycle\ORM\Relation\Pivoted\PivotedCollection;
 use Cycle\ORM\Schema;
 use Cycle\ORM\Select;
 use Cycle\ORM\Tests\Fixtures\RbacItemAbstract;
@@ -27,6 +26,7 @@ public function setUp(): void
 
         $this->makeTable('rbac_item', [
             'name' => 'string,primary',
+            'description' => 'string,nullable',
             '_type' => 'string,nullable',
         ]);
 
@@ -50,7 +50,7 @@ public function setUp(): void
                 Schema::DATABASE => 'default',
                 Schema::TABLE => 'rbac_item',
                 Schema::PRIMARY_KEY => 'name',
-                Schema::COLUMNS => ['name', '_type'],
+                Schema::COLUMNS => ['name', 'description', '_type'],
                 Schema::RELATIONS => [
                     'parents' => [
                         Relation::TYPE => Relation::MANY_TO_MANY,
@@ -157,4 +157,35 @@ public function testClearAndFillRelation(): void
 
         self::assertTrue(true);
     }
+
+    public function testNotTriggersRehydrate(): void
+    {
+        $role = new RbacRole('superAdmin', 'description');
+
+        $permission = new RbacPermission('writeUser');
+
+        $role->children->add($permission);
+        $permission->parents->add($role);
+
+        $this->save($role);
+
+        unset($role, $permission);
+
+        $this->orm = $this->orm->withHeap(new Heap());
+
+        /** @var RbacRole $fetchedRole */
+        $fetchedRole = (new Select($this->orm, 'rbac_item'))->wherePK('superAdmin')->fetchOne();
+        /** @var RbacPermission $fetchedPermission */
+        $fetchedPermission = (new Select($this->orm, 'rbac_item'))->wherePK('writeUser')->fetchOne();
+
+        $fetchedRole->description = 'updated description';
+
+        // unlink
+        $fetchedRole->children->removeElement($fetchedPermission);
+        $fetchedPermission->parents->removeElement($fetchedRole);
+
+        self::assertSame('updated description', $fetchedRole->description);
+
+        $this->orm = $this->orm->withHeap(new Heap());
+    }
 }