From 64887e7a9f8874de012f4298e6f9c45ae38aefc5 Mon Sep 17 00:00:00 2001
From: Jan-Hendrik Willms <tleilax+studip@gmail.com>
Date: Fri, 17 Feb 2023 16:11:12 +0000
Subject: [PATCH] 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              | 68 ++++++++++---------
 .../unit/lib/classes/SimpleOrMapNodbTest.php  | 23 ++++++-
 2 files changed, 58 insertions(+), 33 deletions(-)

diff --git a/lib/models/SimpleORMap.class.php b/lib/models/SimpleORMap.class.php
index 7af9dece840..b210c399320 100644
--- a/lib/models/SimpleORMap.class.php
+++ b/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;
diff --git a/tests/unit/lib/classes/SimpleOrMapNodbTest.php b/tests/unit/lib/classes/SimpleOrMapNodbTest.php
index e5c333b3be3..b36451042e1 100644
--- a/tests/unit/lib/classes/SimpleOrMapNodbTest.php
+++ b/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()
+        );
+    }
 }
-- 
GitLab