clone records before fetching into them so that the after_initialize will...

clone records before fetching into them so that the after_initialize will always be called on a fresh object

Closes #1081

Merge request studip/studip!646

lib/models/SimpleORMap.class.php

@@ -615,26 +615,27 @@ class SimpleORMap implements ArrayAccess, Countable, IteratorAggregate
     public static function findBySQL($sql, $params = [])
     {
         $db_table = static::db_table();
-        $class = get_called_class();
-        $record = new $class();
-        $db = DBManager::get();
+
         $has_join = stripos($sql, 'JOIN ');
         if ($has_join === false || $has_join > 10) {
             $sql = 'WHERE ' . $sql;
         }
         $sql = "SELECT `" . $db_table . "`.* FROM `" . $db_table . "` " . $sql;
-        $ret = [];
         $stmt = DBManager::get()->prepare($sql);
         $stmt->execute($params);
-        $stmt->setFetchMode(PDO::FETCH_INTO , $record);
-        $record->setNew(false);
-        while ($record = $stmt->fetch()) {
-            // Reset all relations
-            $record->cleanup();
 
-            $record->applyCallbacks('after_initialize');
-            $ret[] = clone $record;
-        }
+        $record = static::build([], false);
+
+        $ret = [];
+        do  {
+            $clone = clone $record;
+            $stmt->setFetchMode(PDO::FETCH_INTO, $clone);
+
+            if ($clone = $stmt->fetch()) {
+                $clone->applyCallbacks('after_initialize');
+                $ret[] = $clone;
+            }
+        } while ($clone);
         return $ret;
     }
 
@@ -670,24 +671,25 @@ class SimpleORMap implements ArrayAccess, Countable, IteratorAggregate
         $assoc_foreign_key = $options['assoc_foreign_key'];
 
         $db_table = static::db_table();
-        $class = get_called_class();
-        $record = new $class();
+
         $sql = "SELECT `$db_table`.* FROM `$thru_table`
         INNER JOIN `$db_table` ON `$thru_table`.`$thru_assoc_key` = `$db_table`.`$assoc_foreign_key`
         WHERE `$thru_table`.`$thru_key` = ? " . ($options['order_by'] ?? '');
-        $db = DBManager::get();
-        $st = $db->prepare($sql);
+        $st = DBManager::get()->prepare($sql);
         $st->execute([$foreign_key_value]);
+
+        $record = static::build([], false);
+
         $ret = [];
-        $st->setFetchMode(PDO::FETCH_INTO , $record);
-        $record->setNew(false);
-        while ($record = $st->fetch()) {
-            // Reset all relations
-            $record->cleanup();
+        do {
+            $clone = clone $record;
+            $st->setFetchMode(PDO::FETCH_INTO, $clone);
 
-            $record->applyCallbacks('after_initialize');
-            $ret[] = clone $record;
-        }
+            if ($clone = $st->fetch()) {
+                $clone->applyCallbacks('after_initialize');
+                $ret[] = $clone;
+            }
+        } while ($clone);
         return $ret;
     }
 
@@ -713,20 +715,22 @@ class SimpleORMap implements ArrayAccess, Countable, IteratorAggregate
         $db_table = static::db_table();
         $st = DBManager::get()->prepare("SELECT `{$db_table}`.* FROM `{$db_table}` {$sql}");
         $st->execute($params);
-        $st->setFetchMode(PDO::FETCH_INTO , $record);
 
         // Indicate that we are performing a batch operation
         static::$performs_batch_operation = true;
 
+        $record = static::build([], false);
+
         $ret = 0;
-        while ($record = $st->fetch()) {
-            // Reset all relations
-            $record->cleanup();
-            $record->applyCallbacks('after_initialize');
+        do {
+            $clone = clone $record;
+            $st->setFetchMode(PDO::FETCH_INTO, $clone);
 
-            // Execute callable on current record
-            $callable(clone $record, $ret++);
-        }
+            if ($clone = $st->fetch()) {
+                $clone->applyCallbacks('after_initialize');
+                $callable($clone, $ret++);
+            }
+        } while ($clone);
 
         // Reset batch operation indicator
         static::$performs_batch_operation = false;

tests/unit/lib/classes/SimpleOrMapNodbTest.php

@@ -56,7 +56,7 @@ class SimpleOrMapNodbTest extends \Codeception\Test\Unit
         StudipTestHelper::tear_down_tables();
     }
 
-    public function testConstruct()
+    public function testConstruct(): auth_user_md5
     {
         $a = new auth_user_md5();
         $this->assertInstanceOf('SimpleOrMap', $a);
@@ -288,4 +288,25 @@ class SimpleOrMapNodbTest extends \Codeception\Test\Unit
 
         $this->assertEquals($a->toArray(), $unserialized->toArray());
     }
+
+    /**
+     * @depends testConstruct
+     * @covers SimpleORMap::__clone
+     */
+    public function testClone(auth_user_md5 $a)
+    {
+        $queue = new SplStack();
+        $queue->push(1);
+        $queue->push(2);
+        $queue->push(3);
+
+        $a->additional_dummy_data = $queue;
+
+        $b = clone $a;
+
+        $this->assertEquals(
+            $a->additional_dummy_data->count(),
+            $b->additional_dummy_data->count()
+        );
+    }
 }