Fix inserting order in regular cases

CHANGELOG.md

@@ -1,5 +1,9 @@
 # CHANGELOG
 
+v2.7.1 (13.02.2024)
+--------------------
+- Fix inserting order in regular cases by @roxblnfk and @gam6itko (#381)
+
 v2.7.0 (08.02.2024)
 --------------------
 - Add Generated Fields option into ORM Schema by @roxblnfk (#462)

CONTRIBUTING.md

@@ -26,7 +26,7 @@ $ ./vendor/bin/phpunit
 To run quick test suite:
 
 ```bash
-$ ./vendor/bin/phpunit tests/ORM/Driver/SQLite
+$ ./vendor/bin/phpunit tests/ORM/Functional/Driver/SQLite
 ```
 
 ## Help Needed In

phpunit.xml

@@ -11,8 +11,8 @@
          convertWarningsToExceptions="true"
          convertDeprecationsToExceptions="true"
          processIsolation="false"
-         stopOnFailure="true"
-         stopOnError="true"
+         stopOnFailure="false"
+         stopOnError="false"
 >
     <coverage>
         <include>

src/Transaction/Pool.php

@@ -10,7 +10,6 @@
 use Cycle\ORM\ORMInterface;
 use Cycle\ORM\Reference\ReferenceInterface;
 use JetBrains\PhpStorm\ExpectedValues;
-use SplObjectStorage;
 use Traversable;
 
 /**
@@ -21,14 +20,9 @@
  */
 final class Pool implements \Countable
 {
-    /** @var SplObjectStorage<object, Tuple> */
-    private SplObjectStorage $storage;
-
-    /** @var SplObjectStorage<object, Tuple> */
-    private SplObjectStorage $all;
-
-    /** @var SplObjectStorage<object, Tuple> */
-    private SplObjectStorage $priorityStorage;
+    private TupleStorage $storage;
+    private TupleStorage $all;
+    private TupleStorage $priorityStorage;
 
     /**
      * @var Tuple[]
@@ -49,8 +43,8 @@ final class Pool implements \Countable
     public function __construct(
         private ORMInterface $orm
     ) {
-        $this->storage = new SplObjectStorage();
-        $this->all = new SplObjectStorage();
+        $this->storage = new TupleStorage();
+        $this->all = new TupleStorage();
     }
 
     public function someHappens(): void
@@ -93,13 +87,13 @@ public function attach(
     private function smartAttachTuple(Tuple $tuple, bool $highPriority = false, bool $snap = false): Tuple
     {
         if ($tuple->status === Tuple::STATUS_PROCESSED) {
-            $this->all->attach($tuple->entity, $tuple);
+            $this->all->attach($tuple);
             return $tuple;
         }
         if ($tuple->status === Tuple::STATUS_PREPARING && $this->all->contains($tuple->entity)) {
-            return $this->all->offsetGet($tuple->entity);
+            return $this->all->getTuple($tuple->entity);
         }
-        $this->all->attach($tuple->entity, $tuple);
+        $this->all->attach($tuple);
 
         if ($this->iterating || $snap) {
             $this->snap($tuple);
@@ -109,9 +103,9 @@ private function smartAttachTuple(Tuple $tuple, bool $highPriority = false, bool
             $tuple->state->setStatus(Node::SCHEDULED_DELETE);
         }
         if (($this->priorityAutoAttach || $highPriority) && $tuple->status === Tuple::STATUS_PREPARING) {
-            $this->priorityStorage->attach($tuple->entity, $tuple);
+            $this->priorityStorage->attach($tuple);
         } else {
-            $this->storage->attach($tuple->entity, $tuple);
+            $this->storage->attach($tuple);
         }
         return $tuple;
     }
@@ -138,7 +132,7 @@ public function attachDelete(
 
     public function offsetGet(object $entity): ?Tuple
     {
-        return $this->all->contains($entity) ? $this->all->offsetGet($entity) : null;
+        return $this->all->contains($entity) ? $this->all->getTuple($entity) : null;
     }
 
     /**
@@ -156,14 +150,11 @@ public function openIterator(): Traversable
         $this->unprocessed = [];
 
         // Snap all entities before store
-        while ($this->storage->valid()) {
-            /** @var Tuple $tuple */
-            $tuple = $this->storage->getInfo();
+        /** @var object $entity */
+        foreach ($this->storage as $entity => $tuple) {
             $this->snap($tuple);
             if (!isset($tuple->node)) {
-                $this->storage->detach($this->storage->current());
-            } else {
-                $this->storage->next();
+                $this->storage->detach($entity);
             }
         }
 
@@ -172,8 +163,7 @@ public function openIterator(): Traversable
             // High priority first
             if ($this->priorityStorage->count() > 0) {
                 $priorityStorage = $this->priorityStorage;
-                foreach ($priorityStorage as $entity) {
-                    $tuple = $priorityStorage->offsetGet($entity);
+                foreach ($priorityStorage as $entity => $tuple) {
                     yield $entity => $tuple;
                     $this->trashIt($entity, $tuple, $priorityStorage);
                 }
@@ -184,39 +174,29 @@ public function openIterator(): Traversable
                 break;
             }
             $pool = $this->storage;
-            if (!$pool->valid() && $pool->count() > 0) {
-                $pool->rewind();
-            }
             if ($stage === 0) {
-                // foreach ($pool as $entity) {
-                while ($pool->valid()) {
-                    /** @var Tuple $tuple */
-                    $entity = $pool->current();
-                    $tuple = $pool->getInfo();
-                    $pool->next();
+                foreach ($this->storage as $entity => $tuple) {
                     if ($tuple->status !== Tuple::STATUS_PREPARING) {
                         continue;
                     }
+
                     yield $entity => $tuple;
-                    $this->trashIt($entity, $tuple, $this->storage);
+                    $this->trashIt($entity, $tuple, $pool);
                     // Check priority
                     if ($this->priorityStorage->count() > 0) {
                         continue 2;
                     }
                 }
                 $this->priorityAutoAttach = true;
                 $stage = 1;
-                $this->storage->rewind();
             }
             if ($stage === 1) {
-                while ($pool->valid()) {
-                    /** @var Tuple $tuple */
-                    $entity = $pool->current();
-                    $tuple = $pool->getInfo();
-                    $pool->next();
+                /** @var object $entity */
+                foreach ($this->storage as $entity => $tuple) {
                     if ($tuple->status !== Tuple::STATUS_WAITING || $tuple->task === Tuple::TASK_DELETE) {
                         continue;
                     }
+
                     $tuple->status = Tuple::STATUS_WAITED;
                     yield $entity => $tuple;
                     $this->trashIt($entity, $tuple, $this->storage);
@@ -226,14 +206,11 @@ public function openIterator(): Traversable
                     }
                 }
                 $stage = 2;
-                $this->storage->rewind();
             }
             if ($stage === 2) {
                 $this->happens = 0;
-                while ($pool->valid()) {
-                    /** @var Tuple $tuple */
-                    $entity = $pool->current();
-                    $tuple = $pool->getInfo();
+                /** @var object $entity */
+                foreach ($this->storage as $entity => $tuple) {
                     if ($tuple->task === Tuple::TASK_DELETE) {
                         $tuple->task = Tuple::TASK_FORCE_DELETE;
                     }
@@ -242,7 +219,6 @@ public function openIterator(): Traversable
                     } elseif ($tuple->status === Tuple::STATUS_DEFERRED) {
                         $tuple->status = Tuple::STATUS_PROPOSED;
                     }
-                    $pool->next();
                     yield $entity => $tuple;
                     $this->trashIt($entity, $tuple, $this->storage);
                     // Check priority
@@ -254,7 +230,7 @@ public function openIterator(): Traversable
                 if ($this->happens !== 0 && $hasUnresolved) {
                     /** @psalm-suppress InvalidIterator */
                     foreach ($this->unprocessed as $item) {
-                        $this->storage->attach($item->entity, $item);
+                        $this->storage->attach($item);
                     }
                     $this->unprocessed = [];
                     continue;
@@ -290,8 +266,7 @@ public function getUnresolved(): iterable
      */
     public function getAllTuples(): iterable
     {
-        foreach ($this->all as $entity) {
-            $tuple = $this->all->offsetGet($entity);
+        foreach ($this->all as $entity => $tuple) {
             if (isset($tuple->node)) {
                 yield $entity => $tuple;
             }
@@ -307,7 +282,6 @@ public function closeIterator(): void
         $this->priorityEnabled = false;
         $this->priorityAutoAttach = false;
         unset($this->priorityStorage, $this->unprocessed);
-        // $this->all = new SplObjectStorage();
     }
 
     /**
@@ -346,17 +320,17 @@ private function snap(Tuple $tuple, bool $forceUpdateState = false): void
         }
     }
 
-    private function trashIt(object $entity, Tuple $tuple, SplObjectStorage $storage): void
+    private function trashIt(object $entity, Tuple $tuple, TupleStorage $storage): void
     {
-        $storage->detach($entity);
-
         if ($tuple->status === Tuple::STATUS_UNPROCESSED) {
+            $storage->detach($entity);
             $tuple->status = Tuple::STATUS_PREPROCESSED;
             $this->unprocessed[] = $tuple;
             return;
         }
 
         if ($tuple->status >= Tuple::STATUS_PREPROCESSED) {
+            $storage->detach($entity);
             $tuple->status = Tuple::STATUS_PROCESSED;
             ++$this->happens;
             return;
@@ -366,7 +340,11 @@ private function trashIt(object $entity, Tuple $tuple, SplObjectStorage $storage
             ++$tuple->status;
             ++$this->happens;
         }
-        $this->storage->attach($tuple->entity, $tuple);
+
+        if ($storage !== $this->storage) {
+            $storage->detach($entity);
+            $this->storage->attach($tuple);
+        }
     }
 
     private function activatePriorityStorage(): void
@@ -375,7 +353,7 @@ private function activatePriorityStorage(): void
             return;
         }
         $this->priorityEnabled = true;
-        $this->priorityStorage = new SplObjectStorage();
+        $this->priorityStorage = new TupleStorage();
     }
 
     private function updateTuple(Tuple $tuple, int $task, ?int $status, bool $cascade, ?Node $node, ?State $state): void

src/Transaction/TupleStorage.php

@@ -0,0 +1,89 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Cycle\ORM\Transaction;
+
+use Countable;
+use IteratorAggregate;
+
+/**
+ * @internal
+ * @implements IteratorAggregate<object, Tuple>
+ */
+final class TupleStorage implements IteratorAggregate, Countable
+{
+    /** @var array<int, Tuple> */
+    private array $storage = [];
+
+    private array $iterators = [];
+
+    /**
+     * @return \Traversable<object, Tuple>
+     */
+    public function getIterator(): \Traversable
+    {
+        $iterator = $this->storage;
+        // When the generator is destroyed, the reference to the iterator is removed from the collection.
+        $cleaner = new class () {
+            public array $iterators;
+            public function __destruct()
+            {
+                unset($this->iterators[\spl_object_id($this)]);
+            }
+        };
+        /** @psalm-suppress UnsupportedPropertyReferenceUsage */
+        $cleaner->iterators = &$this->iterators;
+        $this->iterators[\spl_object_id($cleaner)] = &$iterator;
+
+        while (\count($iterator) > 0) {
+            $tuple = \current($iterator);
+            unset($iterator[\key($iterator)]);
+            yield $tuple->entity => $tuple;
+        }
+    }
+
+    /**
+     * Returns {@see Tuple} if exists, throws an exception otherwise.
+     *
+     * @throws \Throwable if the entity is not found in the storage
+     */
+    public function getTuple(object $entity): Tuple
+    {
+        return $this->storage[\spl_object_id($entity)] ?? throw new \RuntimeException('Tuple not found');
+    }
+
+    public function attach(Tuple $tuple): void
+    {
+        if ($this->contains($tuple->entity)) {
+            return;
+        }
+
+        $this->storage[\spl_object_id($tuple->entity)] = $tuple;
+        foreach ($this->iterators as &$collection) {
+            $collection[\spl_object_id($tuple->entity)] = $tuple;
+        }
+    }
+
+    public function contains(object $entity): bool
+    {
+        return \array_key_exists(\spl_object_id($entity), $this->storage);
+    }
+
+    public function detach(object $entity): void
+    {
+        $id = \spl_object_id($entity);
+        unset($this->storage[$id]);
+        foreach ($this->iterators as &$collection) {
+            unset($collection[$id]);
+        }
+    }
+
+    /**
+     * @return int<0, max>
+     */
+    public function count(): int
+    {
+        return \count($this->storage);
+    }
+}

tests/ORM/Functional/Driver/Common/Integration/Case321/CaseTest.php

@@ -84,7 +84,6 @@ private function makeTables(): void
             'id' => 'primary', // autoincrement
         ]);
 
-        $this->logger->display();
         $this->makeTable('user4', [
             'id' => 'primary',
             'counter' => 'int',