Fix handling of dependency cycles.

Updated dependency builder to allow multiple dependencies

Signed-off-by: amzn-mike <[email protected]>

Code/Tools/AssetProcessor/native/ui/ProductDependencyTreeModel.cpp

@@ -30,7 +30,7 @@ namespace AssetProcessor
         , m_productFilterModel(productFilterModel)
         , m_treeType(treeType)
         , m_fileIcon(QIcon(QStringLiteral(":/AssetProcessor_goto.svg")))
-    {        
+    {
         m_root.reset(new ProductDependencyTreeItem(AZStd::make_shared<ProductDependencyTreeItemData>("", "")));
     }
 
@@ -213,10 +213,10 @@ namespace AssetProcessor
         switch (m_treeType)
         {
         case DependencyTreeType::Outgoing:
-            PopulateOutgoingProductDependencies(productDependencies, productItemData->m_databaseInfo.m_productID);
+            PopulateOutgoingProductDependencies(productDependencies, productItemData->m_databaseInfo.m_productID, {});
             break;
         case DependencyTreeType::Incoming:
-            PopulateIncomingProductDependencies(productDependencies, productItemData->m_databaseInfo.m_productID);
+            PopulateIncomingProductDependencies(productDependencies, productItemData->m_databaseInfo.m_productID, {});
             break;
         }
         endResetModel();
@@ -233,7 +233,7 @@ namespace AssetProcessor
         AZ::s64 m_productId;
     };
 
-    void ProductDependencyTreeModel::PopulateIncomingProductDependencies(ProductDependencyTreeItem* parent, AZ::s64 parentProductId)
+    void ProductDependencyTreeModel::PopulateIncomingProductDependencies(ProductDependencyTreeItem* parent, AZ::s64 parentProductId, QSet<AZ::s64> visitedDependencies)
     {
         AZ::Data::AssetId assetId;
         m_sharedDbConnection->QueryProductByProductID(
@@ -252,7 +252,7 @@ namespace AssetProcessor
                 return true;
             });
 
-        
+
         AZStd::string platform;
         m_sharedDbConnection->QueryJobByProductID(
             parentProductId,
@@ -262,13 +262,20 @@ namespace AssetProcessor
                 return true;
             });
 
-        
+
         AZStd::vector<ProductDependencyChild> pendingChildren;
 
         m_sharedDbConnection->QueryDirectReverseProductDependenciesBySourceGuidSubId(
             assetId.m_guid, assetId.m_subId,
             [&](AzToolsFramework::AssetDatabase::ProductDatabaseEntry& incomingDependency)
             {
+                // Make sure we haven't already encountered this product before
+                // If we have, that means there's a dependency loop, so just abort
+                if (visitedDependencies.contains(incomingDependency.m_productID))
+                {
+                    return true;
+                }
+
                 bool platformMatches = false;
                 m_sharedDbConnection->QueryJobByJobID(
                     incomingDependency.m_jobPK,
@@ -285,6 +292,8 @@ namespace AssetProcessor
                     return true;
                 }
 
+                visitedDependencies.insert(incomingDependency.m_productID);
+
                 AZ::IO::Path productNamePath(incomingDependency.m_productName, AZ::IO::PosixPathSeparator);
                 const AZ::IO::PathView filename = productNamePath.Filename();
                 AZStd::shared_ptr<ProductDependencyTreeItemData> productDepTreeItemData = AZStd::make_shared<ProductDependencyTreeItemData>(
@@ -298,11 +307,11 @@ namespace AssetProcessor
 
         for (auto& child : pendingChildren)
         {
-            PopulateIncomingProductDependencies(child.m_treeItem, child.m_productId);
+            PopulateIncomingProductDependencies(child.m_treeItem, child.m_productId, visitedDependencies);
         }
     }
 
-    void ProductDependencyTreeModel::PopulateOutgoingProductDependencies(ProductDependencyTreeItem* parent, AZ::s64 parentProductId)
+    void ProductDependencyTreeModel::PopulateOutgoingProductDependencies(ProductDependencyTreeItem* parent, AZ::s64 parentProductId, QSet<AZ::s64> visitedDependencies)
     {
         AZStd::string platform;
         m_sharedDbConnection->QueryJobByProductID(
@@ -326,7 +335,9 @@ namespace AssetProcessor
                     dependency.m_dependencySourceGuid, dependency.m_dependencySubID,
                     [&](AzToolsFramework::AssetDatabase::ProductDatabaseEntry& product)
                     {
-                        if (m_trackedProductIds.contains(product.m_productID))
+                        // Make sure we haven't already encountered this product before
+                        // If we have, that means there's a dependency loop, so just abort
+                        if (visitedDependencies.contains(product.m_productID))
                         {
                             return true;
                         }
@@ -346,6 +357,8 @@ namespace AssetProcessor
                             return true;
                         }
 
+                        visitedDependencies.insert(product.m_productID);
+
                         AZ::IO::Path productNamePath(product.m_productName, AZ::IO::PosixPathSeparator);
                         const AZ::IO::PathView filename = productNamePath.Filename();
                         AZStd::shared_ptr<ProductDependencyTreeItemData> productDepTreeItemData =
@@ -360,7 +373,8 @@ namespace AssetProcessor
             });
         for (auto& child : pendingChildren)
         {
-            PopulateOutgoingProductDependencies(child.m_treeItem, child.m_productId);
+            PopulateOutgoingProductDependencies(child.m_treeItem, child.m_productId, visitedDependencies);
         }
     }
+
 } // namespace AssetProcessor

Code/Tools/AssetProcessor/native/ui/ProductDependencyTreeModel.h

@@ -50,8 +50,9 @@ namespace AssetProcessor
         void AssetDataSelectionChanged(const QItemSelection& selected, const QItemSelection& deselected);
     protected:
 
-        void PopulateOutgoingProductDependencies(ProductDependencyTreeItem* parent, AZ::s64 parentProductId);
-        void PopulateIncomingProductDependencies(ProductDependencyTreeItem* parent, AZ::s64 parentProductId);
+        // visitedDependencies is intentionally a copy because we need to independently track the chain for each branch in the graph
+        void PopulateOutgoingProductDependencies(ProductDependencyTreeItem* parent, AZ::s64 parentProductId, QSet<AZ::s64> visitedDependencies);
+        void PopulateIncomingProductDependencies(ProductDependencyTreeItem* parent, AZ::s64 parentProductId, QSet<AZ::s64> visitedDependencies);
 
         AZStd::shared_ptr<AzToolsFramework::AssetDatabase::AssetDatabaseConnection> m_sharedDbConnection;
         

Gems/TestAssetBuilder/Code/Source/Builder/TestDependencyBuilderComponent.cpp

@@ -48,7 +48,9 @@ namespace TestAssetBuilder
 
         if (serialize)
         {
-            serialize->Class<TestAsset, AZ::Data::AssetData>()->Field("ReferencedAsset", &TestAsset::m_referencedAsset);
+            serialize->Class<TestAsset, AZ::Data::AssetData>()
+                ->Version(1)
+                ->Field("ReferencedAssets", &TestAsset::m_referencedAssets);
         }
     }
 
@@ -154,21 +156,33 @@ namespace TestAssetBuilder
 
         if (!buffer.empty())
         {
-            bool result = false;
-            AZ::Data::AssetInfo assetInfo;
-            AZStd::string watchFolder;
-            AzToolsFramework::AssetSystemRequestBus::BroadcastResult(result, &AzToolsFramework::AssetSystemRequestBus::Events::GetSourceInfoBySourcePath, buffer.c_str(), assetInfo, watchFolder);
+            AZStd::vector<AZStd::string> tokens;
+            AZ::StringFunc::Tokenize(buffer, tokens, "|");
 
-            if (!result || !assetInfo.m_assetId.IsValid())
+            for (const AZStd::string& path : tokens)
             {
-                AZ_Error("TestDependencyBuilderComponent", false, "GetSourceInfoBySourcePath failed for %s", buffer.c_str());
-                return;
+                bool result = false;
+                AZ::Data::AssetInfo assetInfo;
+                AZStd::string watchFolder;
+                AzToolsFramework::AssetSystemRequestBus::BroadcastResult(
+                    result,
+                    &AzToolsFramework::AssetSystemRequestBus::Events::GetSourceInfoBySourcePath,
+                    path.c_str(),
+                    assetInfo,
+                    watchFolder);
+
+                if (!result || !assetInfo.m_assetId.IsValid())
+                {
+                    AZ_Error("TestDependencyBuilderComponent", false, "GetSourceInfoBySourcePath failed for %s", path.c_str());
+                    return;
+                }
+
+                // It's not technically correct to use the source AssetId as a product asset reference, however we know the output will have
+                // a subId of 0 (the default) so we don't actually need that bit of data, we just need the UUID
+                auto assetRef = AZ::Data::Asset<TestAsset>(assetInfo.m_assetId, azrtti_typeid<TestAsset>(), path);
+                assetRef.SetAutoLoadBehavior(AZ::Data::PreLoad);
+                outputAsset.m_referencedAssets.push_back(AZStd::move(assetRef));
             }
-
-            // It's not technically correct to use the source AssetId as a product asset reference, however we know the output will have a
-            // subId of 0 (the default) so we don't actually need that bit of data, we just need the UUID
-            outputAsset.m_referencedAsset = AZ::Data::Asset<TestAsset>(assetInfo.m_assetId, azrtti_typeid<TestAsset>(), buffer);
-            outputAsset.m_referencedAsset.SetAutoLoadBehavior(AZ::Data::PreLoad);
         }
 
         auto outputPath = AZ::IO::Path(request.m_tempDirPath) / request.m_sourceFile;

Gems/TestAssetBuilder/Code/Source/Builder/TestDependencyBuilderComponent.h

@@ -29,7 +29,7 @@ namespace TestAssetBuilder
         TestAsset() = default;
         virtual ~TestAsset() = default;
 
-        AZ::Data::Asset<TestAsset> m_referencedAsset;
+        AZStd::vector<AZ::Data::Asset<TestAsset>> m_referencedAssets;
     };
 
     //! This builder is intended for automated tests which need an asset that can reference other assets.