From f21edce874766c6a195f4fb3ed6bc727dd182504 Mon Sep 17 00:00:00 2001
From: Mojmir Fendek <mfendek@silverstripe.com>
Date: Thu, 19 Oct 2023 15:39:14 +1300
Subject: [PATCH] PR fixes.

---
 src/RecursiveStagesService.php | 38 ++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/src/RecursiveStagesService.php b/src/RecursiveStagesService.php
index cbc03a67..9ca75de0 100644
--- a/src/RecursiveStagesService.php
+++ b/src/RecursiveStagesService.php
@@ -72,8 +72,23 @@ protected function determineStagesDifferRecursive(DataObject $object): bool
         // Compare existing content (perform full ownership traversal)
         $models = [$object];
 
+        // We will keep track of inspected models so we don;t inspect the same model multiple times
+        // This prevents some edge cases like infinite loops
+        $identifiers = [];
+
         /** @var DataObject|Versioned $model */
         while ($model = array_shift($models)) {
+            $identifier = $this->getObjectIdentifier($model);
+
+            if (in_array($identifier, $identifiers)) {
+                // We've already inspected this model, so we can skip processing it
+                // This is to prevent potential infinite loops
+                continue;
+            }
+
+            // Mark model as inspected
+            $identifiers[] = $identifier;
+
             if ($model->hasExtension(Versioned::class) && $model->isModifiedOnDraft()) {
                 // Model is dirty,
                 // we can return here as there is no need to check the rest of the hierarchy
@@ -112,10 +127,15 @@ protected function getOwnedIdentifiers(DataObject $object, string $stage): array
             $identifiers = [];
 
             while ($model = array_shift($models)) {
-                // Compose a unique identifier, so we can easily compare models across stages
-                // Note that we can't use getUniqueKey() as that one contains stage fragments
-                // which prevents us from making cross-stage comparison
-                $identifiers[] = $model->ClassName . '-' . $model->ID;
+                $identifier = $this->getObjectIdentifier($model);
+
+                if (in_array($identifier, $identifiers)) {
+                    // We've already inspected this model, so we can skip processing it
+                    // This is to prevent potential infinite loops
+                    continue;
+                }
+
+                $identifiers[] = $identifier;
                 $relatedObjects = $this->getOwnedObjects($model);
                 $models = array_merge($models, $relatedObjects);
             }
@@ -155,4 +175,14 @@ protected function getOwnedObjects(DataObject $object): array
 
         return $this->ownedObjectsCache[$cacheKey];
     }
+
+    /**
+     * This method covers cases where @see DataObject::getUniqueKey() can't be used
+     * For example when we want to compare models across stages we can't use getUniqueKey()
+     * as that one contains stage fragments which prevents us from making cross-stage comparison
+     */
+    private function getObjectIdentifier(DataObject $object): string
+    {
+        return $object->ClassName . '-' . $object->ID;
+    }
 }