Asset Processor: Zero analysis dependency fingerprint verification (#7292)

* Move modtime scanning tests out of APM tests file and into its own file.

Changes were kept to a minimum to get things compiling, this is just a move of code

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

* Fix rebase compile errors

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

* Verify dependency fingerprints during Zero Analysis.

This fixes an issue where dependencies that weren't finished processing when AP shuts down would not resume when AP is started back up due to Zero Analysis ignoring dependencies when determining files to skip.
Added unit test for verification

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

* Fix compile error, make 17 a constexpr

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

* Fix compile error

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

Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.cpp

@@ -2942,11 +2942,20 @@ namespace AssetProcessor
             // File is a source file that has been processed before
             AZStd::string fingerprintFromDatabase = sourceFileItr->m_analysisFingerprint.toUtf8().data();
             AZStd::string_view builderEntries(fingerprintFromDatabase.begin() + s_lengthOfUuid + 1, fingerprintFromDatabase.end());
+            AZStd::string_view dependencyFingerprint(fingerprintFromDatabase.begin(), fingerprintFromDatabase.begin() + s_lengthOfUuid);
             int numBuildersEmittingSourceDependencies = 0;
 
             if (!fingerprintFromDatabase.empty() && AreBuildersUnchanged(builderEntries, numBuildersEmittingSourceDependencies))
             {
                 // Builder(s) have not changed since last time
+                AZStd::string currentFingerprint = ComputeRecursiveDependenciesFingerprint(
+                    fileInfo.m_filePath.toUtf8().constData(), sourceFileItr->m_sourceDatabaseName.toUtf8().constData());
+
+                if(dependencyFingerprint != currentFingerprint)
+                {
+                    // Dependencies have changed
+                    return false;
+                }
                 // Success - we can skip this file, nothing has changed!
 
                 // Remove it from the list of to-be-processed files, otherwise the AP will assume the file was deleted
@@ -4311,6 +4320,32 @@ namespace AssetProcessor
         }
     }
 
+    AZStd::string AssetProcessorManager::ComputeRecursiveDependenciesFingerprint(const AZStd::string& fileAbsolutePath, const AZStd::string& fileDatabaseName)
+    {
+        AZStd::string concatenatedFingerprints;
+
+        // QSet is not ordered.
+        SourceFilesForFingerprintingContainer knownDependenciesAbsolutePaths;
+        // this automatically adds the input file to the list:
+        QueryAbsolutePathDependenciesRecursive(QString::fromUtf8(fileDatabaseName.c_str()), knownDependenciesAbsolutePaths,
+            AzToolsFramework::AssetDatabase::SourceFileDependencyEntry::DEP_Any, false);
+        AddMetadataFilesForFingerprinting(QString::fromUtf8(fileAbsolutePath.c_str()), knownDependenciesAbsolutePaths);
+
+        // reserve 17 chars for each since its a 64 bit hex number, and then one more for the dash inbetween each.
+        constexpr int bytesPerFingerprint = (sizeof(AZ::u64) * 2) + 1; // 2 HEX characters per byte +1 for the `-` we will add between each fingerprint
+        concatenatedFingerprints.reserve((knownDependenciesAbsolutePaths.size() * bytesPerFingerprint));
+
+        for (const auto& element : knownDependenciesAbsolutePaths)
+        {
+            // if its a placeholder then don't bother hitting the disk to find it.
+            concatenatedFingerprints.append(AssetUtilities::GetFileFingerprint(element.first, element.second));
+            concatenatedFingerprints.append("-");
+        }
+
+        // to keep this from growing out of hand, we don't use the full string, we use a hash of it:
+        return AZ::Uuid::CreateName(concatenatedFingerprints.c_str()).ToString<AZStd::string>();
+    }
+
     void AssetProcessorManager::FinishAnalysis(AZStd::string fileToCheck)
     {
         using namespace AzToolsFramework::AssetDatabase;
@@ -4378,29 +4413,9 @@ namespace AssetProcessor
         if (found)
         {
             // construct the analysis fingerprint
-            // the format for this data is "modtimefingerprint:builder0:builder1:builder2:...:buildern"
-            source.m_analysisFingerprint.clear();
-            // compute mod times:
-            // get the appropriate modtimes:
-            AZStd::string modTimeArray;
+            // the format for this data is "hashfingerprint:builder0:builder1:builder2:...:buildern"
+            source.m_analysisFingerprint = ComputeRecursiveDependenciesFingerprint(fileToCheck, analysisTracker->m_databaseSourceName);
 
-            // QSet is not ordered.
-            SourceFilesForFingerprintingContainer knownDependenciesAbsolutePaths;
-            // this automatically adds the input file to the list:
-            QueryAbsolutePathDependenciesRecursive(QString::fromUtf8(analysisTracker->m_databaseSourceName.c_str()), knownDependenciesAbsolutePaths, SourceFileDependencyEntry::DEP_Any, false);
-            AddMetadataFilesForFingerprinting(QString::fromUtf8(fileToCheck.c_str()), knownDependenciesAbsolutePaths);
-
-            // reserve 17 chars for each since its a 64 bit hex number, and then one more for the dash inbetween each.
-            modTimeArray.reserve((knownDependenciesAbsolutePaths.size() * 17));
-
-            for (const auto& element : knownDependenciesAbsolutePaths)
-            {
-                // if its a placeholder then don't bother hitting the disk to find it.
-                modTimeArray.append(AssetUtilities::GetFileFingerprint(element.first, element.second));
-                modTimeArray.append("-");
-            }
-            // to keep this from growing out of hand, we don't use the full string, we use a hash of it:
-            source.m_analysisFingerprint = AZ::Uuid::CreateName(modTimeArray.c_str()).ToString<AZStd::string>();
             for (const AZ::Uuid& builderID : analysisTracker->m_buildersInvolved)
             {
                 source.m_analysisFingerprint.append(":");
@@ -4423,7 +4438,7 @@ namespace AssetProcessor
             const ScanFolderInfo* scanFolder = m_platformConfig->GetScanFolderForFile(fileToCheck.c_str());
 
             scanFolderPk = aznumeric_cast<int>(scanFolder->ScanFolderID());
-            m_platformConfig->ConvertToRelativePath(fileToCheck.c_str(), scanFolder, databaseSourceName);
+            PlatformConfiguration::ConvertToRelativePath(fileToCheck.c_str(), scanFolder, databaseSourceName);
         }
 
         // Record the modtime for the file so we know we processed it

Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.h

@@ -485,6 +485,7 @@ namespace AssetProcessor
         };
 
         void ComputeBuilderDirty();
+        AZStd::string ComputeRecursiveDependenciesFingerprint(const AZStd::string& fileAbsolutePath, const AZStd::string& fileDatabaseName);
         AZStd::unordered_map<AZ::Uuid, BuilderData>  m_builderDataCache;
         bool m_buildersAddedOrRemoved = true; //< true if any new builders exist.  If this happens we actually need to re-analyze everything.
         bool m_anyBuilderChange = true;

Code/Tools/AssetProcessor/native/tests/assetmanager/ModtimeScanningTests.cpp

@@ -516,6 +516,79 @@ namespace UnitTests
         ASSERT_EQ(m_data->m_processResults.size(), 2);
     }
 
+     TEST_F(ModtimeScanningTest, AssetProcessorIsRestartedBeforeDependencyIsProcessed_DependencyIsProcessedOnStart)
+    {
+        using namespace AzToolsFramework::AssetSystem;
+        auto theFile = m_data->m_absolutePath[1].toUtf8();
+        const char* theFileString = theFile.constData();
+
+        SetFileContents(theFileString, "hello world");
+
+        // Enable the features we're testing
+        m_assetProcessorManager->SetEnableModtimeSkippingFeature(true);
+        AssetUtilities::SetUseFileHashOverride(true, true);
+
+        QSet<AssetFileInfo> filePaths = BuildFileSet();
+        SimulateAssetScanner(filePaths);
+
+        // Even though we're only updating one file, we're expecting 2 createJob calls because our test file is a dependency that triggers
+        // the other test file to process as well
+        ExpectWork(2, 2);
+
+        // Sort the results and process the first one, which should always be the modtimeTestDependency.txt file
+        // which is the same file we modified above.  modtimeTestFile.txt depends on this file but we're not going to process it yet.
+        {
+            std::sort(
+                m_data->m_processResults.begin(), m_data->m_processResults.end(),
+                [](decltype(m_data->m_processResults[0])& left, decltype(left)& right)
+                {
+                    return left.m_jobEntry.m_databaseSourceName < right.m_jobEntry.m_databaseSourceName;
+                });
+
+            const auto& processResult = m_data->m_processResults[0];
+            auto file =
+                QDir(processResult.m_destinationPath).absoluteFilePath(processResult.m_jobEntry.m_databaseSourceName.toLower() + ".arc1");
+            m_data->m_productPaths.emplace(
+                QDir(processResult.m_jobEntry.m_watchFolderPath)
+                    .absoluteFilePath(processResult.m_jobEntry.m_databaseSourceName)
+                    .toUtf8()
+                    .constData(),
+                file);
+
+            // Create the file on disk
+            ASSERT_TRUE(UnitTestUtils::CreateDummyFile(file, "products."));
+
+            AssetBuilderSDK::ProcessJobResponse response;
+            response.m_resultCode = AssetBuilderSDK::ProcessJobResult_Success;
+            response.m_outputProducts.push_back(AssetBuilderSDK::JobProduct(file.toUtf8().constData(), AZ::Uuid::CreateNull(), 1));
+
+            using JobEntry = AssetProcessor::JobEntry;
+
+            QMetaObject::invokeMethod(
+                m_assetProcessorManager.get(), "AssetProcessed", Qt::QueuedConnection, Q_ARG(JobEntry, processResult.m_jobEntry),
+                Q_ARG(AssetBuilderSDK::ProcessJobResponse, response));
+        }
+
+        ASSERT_TRUE(BlockUntilIdle(5000));
+
+        // Shutdown and restart the APM
+        m_assetProcessorManager.reset();
+        m_assetProcessorManager = AZStd::make_unique<AssetProcessorManager_Test>(m_config.get());
+
+        SetUpAssetProcessorManager();
+
+        m_data->m_mockBuilderInfoHandler.m_createJobsCount = 0;
+        m_data->m_processResults.clear();
+        m_data->m_deletedSources.clear();
+
+        // Re-run the scanner on our files
+        filePaths = BuildFileSet();
+        SimulateAssetScanner(filePaths);
+
+        // Expect processing to resume on the job we didn't process before
+        ExpectWork(1, 1);
+    }
+
     void DeleteTest::SetUp()
     {
         AssetProcessorManagerTest::SetUp();