From c07a7c3766061870624856998aa0f321c07a286f Mon Sep 17 00:00:00 2001
From: daimini <daimini@amazon.com>
Date: Tue, 27 Apr 2021 17:20:22 -0700
Subject: [PATCH] Detect and block instantiations that would generate circular
 dependencies in the instance hierarchy.

---
 .../Prefab/PrefabPublicHandler.cpp            | 33 +++++++++++++++++--
 .../Prefab/PrefabPublicHandler.h              |  8 +++++
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp
index 1e9cc35230..fdcef9bd4b 100644
--- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp
+++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp
@@ -184,6 +184,16 @@ namespace AzToolsFramework
                 instanceToParentUnder = prefabEditorEntityOwnershipInterface->GetRootPrefabInstance();
                 parent = instanceToParentUnder->get().GetContainerEntityId();
             }
+
+            //Detect whether this instantiation would produce a cyclical dependency
+            auto relativePath = m_prefabLoaderInterface->GetRelativePathToProject(filePath);
+            Prefab::TemplateId templateId = m_prefabSystemComponentInterface->GetTemplateIdFromFilePath(relativePath);
+
+            // If the template isn't currently loaded, there's no way for it to be in the hierarchy so we just skip the check.
+            if (templateId != Prefab::InvalidTemplateId && IsPrefabInInstanceAncestorHierarchy(templateId, instanceToParentUnder->get()))
+            {
+                return AZ::Failure(AZStd::string("Instantiate Prefab operation aborted - Instantiation would have introduced a cyclical dependency."));
+            }
             
             {
                 // Initialize Undo Batch object
@@ -194,7 +204,7 @@ namespace AzToolsFramework
                     instanceToParentUnderDomBeforeCreate, instanceToParentUnder->get());
 
                 // Instantiate the Prefab
-                auto instanceToCreate = prefabEditorEntityOwnershipInterface->InstantiatePrefab(filePath, instanceToParentUnder);
+                auto instanceToCreate = prefabEditorEntityOwnershipInterface->InstantiatePrefab(relativePath, instanceToParentUnder);
 
                 if (!instanceToCreate)
                 {
@@ -242,13 +252,30 @@ namespace AzToolsFramework
             {
                 AZ_Assert(
                     false,
-                    "Failed to create prefab : Couldn't get a valid owning instance for the common root entity of the enities provided");
+                    "Failed to create prefab : Couldn't get a valid owning instance for the common root entity of the entities provided");
                 return AZ::Failure(AZStd::string(
-                    "Failed to create prefab : Couldn't get a valid owning instance for the common root entity of the enities provided"));
+                    "Failed to create prefab : Couldn't get a valid owning instance for the common root entity of the entities provided"));
             }
             return AZ::Success();
         }
 
+        bool PrefabPublicHandler::IsPrefabInInstanceAncestorHierarchy(TemplateId prefabTemplateId, const Instance& instance)
+        {
+            const Instance* currentInstance = &instance;
+
+            while (currentInstance != nullptr)
+            {
+                if (currentInstance->GetTemplateId() == prefabTemplateId)
+                {
+                    return true;
+                }
+
+                currentInstance = &currentInstance->GetParentInstance()->get();
+            }
+
+            return false;
+        }
+
         void PrefabPublicHandler::CreateLink(
             const EntityList& topLevelEntities, Instance& sourceInstance, TemplateId targetTemplateId,
             UndoSystem::URSequencePoint* undoBatch, AZ::EntityId commonRootEntityId)
diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h
index e83513dbff..351ffcb71d 100644
--- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h
+++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.h
@@ -96,6 +96,14 @@ namespace AzToolsFramework
                 const AZStd::vector<AZ::EntityId>& entityIds, EntityList& inputEntityList, EntityList& topLevelEntities,
                 AZ::EntityId& commonRootEntityId, InstanceOptionalReference& commonRootEntityOwningInstance);
 
+            /* Detects whether an instance of prefabTemplateId is present in the hierarchy of ancestors of instance.
+             *
+             * \param prefabTemplateId The template id to test for
+             * \param instance The instance whose ancestor hierarchy prefabTemplateId will be tested against.
+             * \return true if an instance of the template of id prefabTemplateId could be found in the ancestor hierarchy of instance, false otherwise.
+             */
+            bool IsPrefabInInstanceAncestorHierarchy(TemplateId prefabTemplateId, const Instance& instance);
+
             static Instance* GetParentInstance(Instance* instance);
             static Instance* GetAncestorOfInstanceThatIsChildOfRoot(const Instance* ancestor, Instance* descendant);
             static void GenerateContainerEntityTransform(const EntityList& topLevelEntities, AZ::Vector3& translation, AZ::Quaternion& rotation);