Fixes a number of issues discovered with Asset Processor and material/materialtype jobs (#19100)

* Fixes material jobs depending on the wrong platform

When a job is emitted by a builder, it can depend on other jobs.
If it depends on other jobs, it waits for those jobs to finish before it
starts its own work.

When your job specifies a dependency, it specifies a tuple of
(platform, job key, source file).  The job key is just the type of job,
a constant string like "Material Builder".

When asset processor tells a builder to emit jobs, it supplies
it a list of platforms that are currently enabled for the project,
for example "pc", "android", "ios", etc.

The Material builder and Material Type builder were creating ONE
job descriptor and then emitting it for each platform as a copy.

The builder was trying to update the platform field in the dependency
but unfortunately it only checked if the platform field was blank.

So in a loop where we go through eahc platform, it would set
the job being emitted to the platform of the first job, and then
on each subsequent platform, it would not update the platform field
because it was not blank anymore, and thus would be emitting jobs
for one platform, but with dependencies for a different platform.

This caused those other jobs to get started before the actual files
they depend on were actually ready (since they would only wait) for
the original platform's job to finish.

This change fixes that by always setting the platform field
to the current platform, so that the dependencies are always
correctly set to the platform of the job being emitted.

It also adds a check to the Asset Processor Manager to reject and
auto fail jobs that have dependencies that are not for the current platform
to prevent this from happening in the future.

It also adds several new automated tests to make sure that the
triggering of dependent jobs works correctly, with, and without the
subids being specified.

Signed-off-by: Nicholas Lawson <[email protected]>

* Fix tabs!

Signed-off-by: Nicholas Lawson <[email protected]>

* Fix comment error

Signed-off-by: Nicholas Lawson <[email protected]>

---------

Signed-off-by: Nicholas Lawson <[email protected]>

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

@@ -1348,17 +1348,25 @@ namespace AssetProcessor
                 newLegacySubIDs.push_back(product.m_legacySubIDs);
             }
 
-            // To find the set of products that were either new, or updated, this code starts with the new products, and erases
-            // the prior products that are exactly the same from that list.
-            // Note that because it uses operator==, it includes comparing the hash.  This means that if the file data has changed
-            // it won't count as being the same, and will not remove it from the list.  This results in 'updatedProducts' containing
-            // the list of new products that were EITHER literally new, or, had data that was new/changed.
+            // if we take the original list of new products
+            // and we subtract priorProducts from it, we end up with the list of new products that were EITHER literally new,
+            // or, had data that was new/changed.  This is because newProducts is the list of ALL emitted stuff from this job
+            // note that it uses operator== to compare products, which means it compares the full product entry, including the hash of the product file.
+            // flags.
             auto updatedProducts = newProducts;
-            if(!updatedProducts.empty())
+            if (!updatedProducts.empty())
             {
                 for (const auto& priorProductEntry : priorProducts)
                 {
-                    updatedProducts.erase(AZStd::remove_if(updatedProducts.begin(), updatedProducts.end(), [&priorProductEntry](const auto& pair){ return pair.first == priorProductEntry; }), updatedProducts.end());
+                    updatedProducts.erase(
+                        AZStd::remove_if(
+                            updatedProducts.begin(),
+                            updatedProducts.end(),
+                            [&priorProductEntry](const auto& pair)
+                            {
+                                return pair.first == priorProductEntry;
+                            }),
+                        updatedProducts.end());
                 }
             }
 
@@ -1382,6 +1390,7 @@ namespace AssetProcessor
                 }
             }
 
+
             // we need to delete these product files from the disk as they no longer exist and inform everyone we did so
             for (const auto& priorProduct : priorProducts)
             {
@@ -4249,7 +4258,7 @@ namespace AssetProcessor
             }
             else
             {
-                // if we get here, we succeeded.
+                // if we get here, we succeeded (as in, the job was at least emitted without the builder crashing).
                 {
                     // if we succeeded, we can erase any jobs that had failed createjobs last time for this builder:
                     AzToolsFramework::AssetSystem::JobInfo jobInfo;
@@ -4269,6 +4278,7 @@ namespace AssetProcessor
 
                     const AssetBuilderSDK::PlatformInfo* const infoForPlatform = m_platformConfig->GetPlatformByIdentifier(jobDescriptor.GetPlatformIdentifier().c_str());
 
+                    // do some basic validation
                     if (!infoForPlatform)
                     {
                         AZ_Warning(AssetProcessor::ConsoleChannel, infoForPlatform,
@@ -4278,6 +4288,45 @@ namespace AssetProcessor
                         continue;
                     }
 
+                    bool jobPlatformValidationFailed = false;
+                    for (auto& jobDependency : jobDescriptor.m_jobDependencyList)
+                    {
+                        if (jobDependency.m_platformIdentifier.compare(AssetBuilderSDK::CommonPlatformName) != 0)
+                        {
+                            // you can only depend on the common platform, or other jobs of the same platform.
+                            if (jobDescriptor.GetPlatformIdentifier() != jobDependency.m_platformIdentifier)
+                            {
+                                AZStd::string failureMessage = AZStd::string::format(
+                                    "Invalid Job Dependency emitted for %s.\n"
+                                    "    The builder (%s, %s) emitted a job for one platform that depends on a job for a different "
+                                    "platform\n"
+                                    "    Jobs can only depend on the \"%s\" platform or other jobs for the same platform.\n"
+                                    "    This is a code error, not a problem with the assets - please modify the builder to emit job\n"
+                                    "    dependencies correctly.\n"
+                                    "      Source Job: (platform \"%s\", job Key: \"%s\")\n"
+                                    "      Depends on: (platform \"%s\", job Key: \"%s\", source file: \"%s\")",
+                                    sourceAsset.AbsolutePath().c_str(),
+                                    builderInfo.m_name.c_str(),
+                                    builderInfo.m_busId.ToString<AZStd::string>().c_str(),
+                                    AssetBuilderSDK::CommonPlatformName,
+                                    jobDescriptor.GetPlatformIdentifier().c_str(),
+                                    jobDescriptor.m_jobKey.c_str(),
+                                    jobDependency.m_platformIdentifier.c_str(),
+                                    jobDependency.m_jobKey.c_str(),
+                                    jobDependency.m_sourceFile.ToString().c_str());
+
+                                jobPlatformValidationFailed = true;
+                                JobEntry failingJob(sourceAsset, builderInfo.m_busId, *infoForPlatform, jobDescriptor.m_jobKey.c_str(), 0, GenerateNewJobRunKey(),sourceUUID);
+                                AutoFailJob(AZStd::string::format("Invalid Job Dependency: %s.\n", sourceAsset.AbsolutePath().c_str()), failureMessage, failingJob, "");
+                                break;
+                            }
+                        }
+                    }
+                    if (jobPlatformValidationFailed)
+                    {
+                        continue; // discard the job, its already been added as an auto-fail job.
+                    }
+
                     {
                         JobDetails newJob;
                         newJob.m_assetBuilderDesc = builderInfo;
@@ -4357,7 +4406,12 @@ namespace AssetProcessor
                 {
                     if ((builderInfo.m_flags & AssetBuilderSDK::AssetBuilderDesc::BF_EmitsNoDependencies) != 0)
                     {
-                        AZ_WarningOnce(ConsoleChannel, false, "Asset builder '%s' registered itself using BF_EmitsNoDependencies flag, but actually emitted dependencies.  This will cause rebuilds to be inconsistent.\n", builderInfo.m_name.c_str());
+                        AZ_WarningOnce(
+                            ConsoleChannel,
+                            false,
+                            "Asset builder '%s' registered itself using BF_EmitsNoDependencies flag, but actually emitted dependencies.  "
+                            "This will cause rebuilds to be inconsistent.\n",
+                            builderInfo.m_name.c_str());
                     }
 
                     // remember which builder emitted each dependency:

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

@@ -4674,6 +4674,80 @@ TEST_F(AssetProcessorManagerTest, JobDependencyOrderOnce_MultipleJobs_EmitOK)
     EXPECT_STREQ(jobDetails[1].m_jobDependencyList[0].m_jobDependency.m_sourceFile.m_sourceFileDependencyPath.c_str(), secondSourceFile.toUtf8().constData()); // there should only be one job dependency
 }
 
+TEST_F(AssetProcessorManagerTest, JobDependency_WrongPlatformEmitted_FailsJob)
+{
+    using namespace AssetProcessor;
+    using namespace AssetBuilderSDK;
+
+    QString watchFolderPath = m_assetRootDir.absoluteFilePath("subfolder1");
+    const ScanFolderInfo* scanFolder = m_config->GetScanFolderByPath(watchFolderPath);
+    ASSERT_NE(scanFolder, nullptr);
+    const char relSourceFileName[] = "a.dummy";
+    const char secondRelSourceFile[] = "b.dummy";
+    QString sourceFileName = m_assetRootDir.absoluteFilePath("subfolder1/a.dummy");
+    QString secondSourceFile = m_assetRootDir.absoluteFilePath("subfolder1/b.dummy");
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(sourceFileName, QString("tempdata\n")));
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(secondSourceFile, QString("tempdata\n")));
+
+    AssetBuilderSDK::AssetBuilderDesc builderDescriptor;
+    builderDescriptor.m_name = "Test Dummy Builder";
+    builderDescriptor.m_patterns.push_back(
+        AssetBuilderSDK::AssetBuilderPattern("*.dummy", AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard));
+    builderDescriptor.m_busId = AZ::Uuid::CreateRandom();
+    builderDescriptor.m_createJobFunction =
+        [&](const AssetBuilderSDK::CreateJobsRequest& request, AssetBuilderSDK::CreateJobsResponse& response)
+    {
+        AssetBuilderSDK::JobDescriptor jobDescriptor;
+        jobDescriptor.m_jobKey = builderDescriptor.m_name;
+
+        // create a job for PC
+        jobDescriptor.SetPlatformIdentifier("pc");
+        if (AzFramework::StringFunc::EndsWith(request.m_sourceFile.c_str(), relSourceFileName))
+        {
+            AssetBuilderSDK::SourceFileDependency dep = { secondRelSourceFile, AZ::Uuid::CreateNull() };
+
+            // -                                                          SEE BELOW     this is the crucial part of the test
+            AssetBuilderSDK::JobDependency jobDep(builderDescriptor.m_name, "ios", AssetBuilderSDK::JobDependencyType::Order, dep);
+            jobDescriptor.m_jobDependencyList.emplace_back(jobDep);
+        }
+        response.m_createJobOutputs.emplace_back(jobDescriptor);
+        response.m_result = AssetBuilderSDK::CreateJobsResultCode::Success;
+    };
+    builderDescriptor.m_processJobFunction =
+        [](const AssetBuilderSDK::ProcessJobRequest& /*request*/, AssetBuilderSDK::ProcessJobResponse& response)
+    {
+        response.m_resultCode = AssetBuilderSDK::ProcessJobResultCode::ProcessJobResult_Success;
+    };
+
+    MockApplicationManager::BuilderFilePatternMatcherAndBuilderDesc builderFilePatternMatcher;
+    builderFilePatternMatcher.m_builderDesc = builderDescriptor;
+    builderFilePatternMatcher.m_internalBuilderName = builderDescriptor.m_name;
+    builderFilePatternMatcher.m_internalUuid = builderDescriptor.m_busId;
+    builderFilePatternMatcher.m_matcherBuilderPattern =
+        AssetUtilities::BuilderFilePatternMatcher(builderDescriptor.m_patterns.back(), builderDescriptor.m_busId);
+    m_mockApplicationManager->m_matcherBuilderPatterns.emplace_back(builderFilePatternMatcher);
+
+    // Capture the job details as the APM inspects the file.
+    AZStd::vector<JobDetails> jobDetails;
+    auto connection = QObject::connect(
+        m_assetProcessorManager.get(),
+        &AssetProcessorManager::AssetToProcess,
+        [&jobDetails](JobDetails job)
+        {
+            jobDetails.emplace_back(job);
+        });
+
+    // Tell the APM about the file:
+    m_isIdling = false;
+    QMetaObject::invokeMethod(m_assetProcessorManager.get(), "AssessModifiedFile", Qt::QueuedConnection, Q_ARG(QString, sourceFileName));
+    QMetaObject::invokeMethod(m_assetProcessorManager.get(), "AssessModifiedFile", Qt::QueuedConnection, Q_ARG(QString, secondSourceFile));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+
+    // the job should auto fail.
+    EXPECT_EQ(jobDetails.size(), 2);
+    EXPECT_EQ(jobDetails[0].m_jobEntry.m_sourceAssetReference.AbsolutePath().c_str(), sourceFileName);
+    EXPECT_EQ(jobDetails[0].m_autoFail, true); // the job should be auto-failed because the dependency is for a different platform
+}
 
 TEST_F(AssetProcessorManagerTest, JobDependencyOrderOnly_MultipleJobs_EmitOK)
 {
@@ -4814,6 +4888,338 @@ TEST_F(AssetProcessorManagerTest, JobDependencyOrderOnly_MultipleJobs_EmitOK)
     EXPECT_EQ(jobDetails[0].m_jobEntry.m_sourceAssetReference.AbsolutePath().c_str(), secondSourceFile);
 }
 
+TEST_F(AssetProcessorManagerTest, JobDependencyOrdered_CausesReprocessingIfDependencyChanges)
+{
+    using namespace AssetProcessor;
+    using namespace AssetBuilderSDK;
+
+    QString watchFolderPath = m_assetRootDir.absoluteFilePath("subfolder1");
+    const ScanFolderInfo* scanFolder = m_config->GetScanFolderByPath(watchFolderPath);
+    ASSERT_NE(scanFolder, nullptr);
+
+    // A.dummy depends on B.dummy, as a JOB dep ordered
+    const char relSourceFileName[] = "a.dummy";
+    const char secondRelSourceFile[] = "b.dummy";
+    QString sourceFileName = m_assetRootDir.absoluteFilePath("subfolder1/a.dummy");
+    QString secondSourceFile = m_assetRootDir.absoluteFilePath("subfolder1/b.dummy");
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(sourceFileName, QString("tempdata\n")));
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(secondSourceFile, QString("tempdata\n")));
+
+    AssetBuilderSDK::AssetBuilderDesc builderDescriptor;
+    builderDescriptor.m_name = "Test Dummy Builder";
+    builderDescriptor.m_patterns.push_back(
+        AssetBuilderSDK::AssetBuilderPattern("*.dummy", AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard));
+    builderDescriptor.m_busId = AZ::Uuid::CreateRandom();
+    builderDescriptor.m_createJobFunction =
+        [&](const AssetBuilderSDK::CreateJobsRequest& request, AssetBuilderSDK::CreateJobsResponse& response)
+    {
+        AssetBuilderSDK::JobDescriptor jobDescriptor;
+        jobDescriptor.m_jobKey = builderDescriptor.m_name;
+        jobDescriptor.SetPlatformIdentifier("pc");
+        if (AzFramework::StringFunc::EndsWith(request.m_sourceFile.c_str(), relSourceFileName))
+        {
+            AssetBuilderSDK::SourceFileDependency dep = { secondRelSourceFile, AZ::Uuid::CreateNull() };
+            AssetBuilderSDK::JobDependency jobDep(builderDescriptor.m_name, "pc", AssetBuilderSDK::JobDependencyType::Order, dep);
+            jobDescriptor.m_jobDependencyList.emplace_back(jobDep);
+        }
+        response.m_createJobOutputs.emplace_back(jobDescriptor);
+        response.m_result = AssetBuilderSDK::CreateJobsResultCode::Success;
+    };
+    builderDescriptor.m_processJobFunction =
+        [](const AssetBuilderSDK::ProcessJobRequest& /*request*/, AssetBuilderSDK::ProcessJobResponse& response)
+    {
+        response.m_resultCode = AssetBuilderSDK::ProcessJobResultCode::ProcessJobResult_Success;
+    };
+
+    MockApplicationManager::BuilderFilePatternMatcherAndBuilderDesc builderFilePatternMatcher;
+    builderFilePatternMatcher.m_builderDesc = builderDescriptor;
+    builderFilePatternMatcher.m_internalBuilderName = builderDescriptor.m_name;
+    builderFilePatternMatcher.m_internalUuid = builderDescriptor.m_busId;
+    builderFilePatternMatcher.m_matcherBuilderPattern =
+        AssetUtilities::BuilderFilePatternMatcher(builderDescriptor.m_patterns.back(), builderDescriptor.m_busId);
+    m_mockApplicationManager->m_matcherBuilderPatterns.emplace_back(builderFilePatternMatcher);
+
+    // Capture the job details as the APM inspects the file.
+    AZStd::vector<JobDetails> jobDetails;
+    auto connection = QObject::connect(
+        m_assetProcessorManager.get(),
+        &AssetProcessorManager::AssetToProcess,
+        [&jobDetails](JobDetails job)
+        {
+            jobDetails.emplace_back(job);
+        });
+
+    // Tell the APM about both files.
+    m_isIdling = false;
+    QMetaObject::invokeMethod(m_assetProcessorManager.get(), "AssessModifiedFile", Qt::QueuedConnection, Q_ARG(QString, sourceFileName));
+    QMetaObject::invokeMethod(m_assetProcessorManager.get(), "AssessModifiedFile", Qt::QueuedConnection, Q_ARG(QString, secondSourceFile));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+
+    // Although we have processed a.dummy first, APM should send us notification of b.dummy job first and than of a.dummy job
+    EXPECT_EQ(jobDetails.size(), 2);
+    EXPECT_EQ(jobDetails[0].m_jobEntry.m_sourceAssetReference.AbsolutePath().c_str(), secondSourceFile);
+    EXPECT_EQ(jobDetails[1].m_jobEntry.m_sourceAssetReference.AbsolutePath().c_str(), sourceFileName);
+    EXPECT_EQ(jobDetails[1].m_jobDependencyList.size(), 1); // there should only be one job dependency
+    EXPECT_EQ(
+        jobDetails[1].m_jobDependencyList[0].m_jobDependency.m_sourceFile.m_sourceFileDependencyPath,
+        secondSourceFile.toUtf8().constData()); // there should only be one job dependency
+
+    // Process jobs in APM
+    auto destination = jobDetails[0].m_cachePath;
+    QString productAFileName = (destination / "aoutput.txt").AsPosix().c_str();
+    QString productBFileName = (destination / "boutput.txt").AsPosix().c_str();
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(productBFileName, QString("tempdata\n")));
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(productAFileName, QString("tempdata\n")));
+
+    AssetBuilderSDK::ProcessJobResponse responseB;
+    responseB.m_resultCode = AssetBuilderSDK::ProcessJobResult_Success;
+    responseB.m_outputProducts.push_back(AssetBuilderSDK::JobProduct("boutput.txt", AZ::Uuid::CreateNull(), 1));
+
+    AssetBuilderSDK::ProcessJobResponse responseA;
+    responseA.m_resultCode = AssetBuilderSDK::ProcessJobResult_Success;
+    responseA.m_outputProducts.push_back(AssetBuilderSDK::JobProduct("aoutput.txt", AZ::Uuid::CreateNull(), 1));
+
+    m_isIdling = false;
+    QMetaObject::invokeMethod(m_assetProcessorManager.get(), "AssetProcessed", Qt::QueuedConnection,
+        Q_ARG(JobEntry, jobDetails[0].m_jobEntry),  Q_ARG(AssetBuilderSDK::ProcessJobResponse, responseB));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+
+    m_isIdling = false;
+    QMetaObject::invokeMethod(m_assetProcessorManager.get(), "AssetProcessed", Qt::QueuedConnection,
+        Q_ARG(JobEntry, jobDetails[1].m_jobEntry),
+        Q_ARG(AssetBuilderSDK::ProcessJobResponse, responseA));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+
+    jobDetails.clear();
+    m_isIdling = false;
+
+    // Modify source file b.dummy, a.dummy should auto process since a job order dependency exists on it.
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(secondSourceFile, QString("temp\n")));
+    QMetaObject::invokeMethod(m_assetProcessorManager.get(), "AssessModifiedFile", Qt::QueuedConnection, Q_ARG(QString, secondSourceFile));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+    // both files should have been reprocesed.
+    // Although we have processed a.dummy first, APM should send us notification of b.dummy job first and than of a.dummy job
+    ASSERT_EQ(jobDetails.size(), 2);
+    EXPECT_EQ(jobDetails[0].m_jobEntry.m_sourceAssetReference.AbsolutePath().c_str(), secondSourceFile);
+    EXPECT_EQ(jobDetails[1].m_jobEntry.m_sourceAssetReference.AbsolutePath().c_str(), sourceFileName);
+    ASSERT_EQ(jobDetails[1].m_jobDependencyList.size(), 1); // there should only be one job dependency
+    EXPECT_EQ(jobDetails[1].m_jobDependencyList[0].m_jobDependency.m_sourceFile.m_sourceFileDependencyPath,
+        secondSourceFile.toUtf8().constData()); // there should only be one job dependency
+
+    // Ack both files as having processed.
+    m_isIdling = false;
+    QMetaObject::invokeMethod(
+        m_assetProcessorManager.get(),
+        "AssetProcessed",
+        Qt::QueuedConnection,
+        Q_ARG(JobEntry, jobDetails[0].m_jobEntry),
+        Q_ARG(AssetBuilderSDK::ProcessJobResponse, responseB));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+
+    m_isIdling = false;
+    QMetaObject::invokeMethod(
+        m_assetProcessorManager.get(),
+        "AssetProcessed",
+        Qt::QueuedConnection,
+        Q_ARG(JobEntry, jobDetails[1].m_jobEntry),
+        Q_ARG(AssetBuilderSDK::ProcessJobResponse, responseA));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+
+    jobDetails.clear();
+
+    // Modify source file a.dummy, we should only see one job with source file a.dummy getting processed in this case.
+    // Becuase b.dummy does not depend on it, the dependency goes the other way.
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(sourceFileName, QString("temp\n")));
+    QMetaObject::invokeMethod(m_assetProcessorManager.get(), "AssessModifiedFile", Qt::QueuedConnection, Q_ARG(QString, sourceFileName));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+    EXPECT_EQ(jobDetails.size(), 1);
+    EXPECT_EQ(jobDetails[0].m_jobEntry.m_sourceAssetReference.AbsolutePath().c_str(), sourceFileName);
+    EXPECT_EQ(
+        jobDetails[0].m_jobDependencyList.size(),
+        1); // there should be one job dependency since APM has already processed b.dummy before but a.dummy has OrderOnly dependency on it.
+
+    m_isIdling = false;
+    QMetaObject::invokeMethod(
+        m_assetProcessorManager.get(),
+        "AssetProcessed",
+        Qt::QueuedConnection,
+        Q_ARG(JobEntry, jobDetails[0].m_jobEntry),
+        Q_ARG(AssetBuilderSDK::ProcessJobResponse, responseA));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+
+    jobDetails.clear();
+    m_isIdling = false;
+}
+
+// this test has
+// a.dummy
+//    depends on b.dummy as a job dependency on b.dummy subid 0
+// b.dummy outputs subid 1
+//    a should not reprocess
+// b.dummy outputs subid 0
+//    a should reprocess.
+
+TEST_F(AssetProcessorManagerTest, JobDependencyOrdered_CausesReprocessingIfDependencyChanges_UsingSubID)
+{
+    using namespace AssetProcessor;
+    using namespace AssetBuilderSDK;
+
+    QString watchFolderPath = m_assetRootDir.absoluteFilePath("subfolder1");
+    const ScanFolderInfo* scanFolder = m_config->GetScanFolderByPath(watchFolderPath);
+    ASSERT_NE(scanFolder, nullptr);
+
+    // A.dummy depends on B.dummy, as a JOB dep ordered
+    const char relSourceFileName[] = "a.dummy";
+    const char secondRelSourceFile[] = "b.dummy";
+    QString sourceFileName = m_assetRootDir.absoluteFilePath("subfolder1/a.dummy");
+    QString secondSourceFile = m_assetRootDir.absoluteFilePath("subfolder1/b.dummy");
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(sourceFileName, QString("tempdata\n")));
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(secondSourceFile, QString("tempdata\n")));
+
+    AssetBuilderSDK::AssetBuilderDesc builderDescriptor;
+    builderDescriptor.m_name = "Test Dummy Builder";
+    builderDescriptor.m_patterns.push_back(
+        AssetBuilderSDK::AssetBuilderPattern("*.dummy", AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard));
+    builderDescriptor.m_busId = AZ::Uuid::CreateRandom();
+    builderDescriptor.m_createJobFunction =
+        [&](const AssetBuilderSDK::CreateJobsRequest& request, AssetBuilderSDK::CreateJobsResponse& response)
+    {
+        AssetBuilderSDK::JobDescriptor jobDescriptor;
+        jobDescriptor.m_jobKey = builderDescriptor.m_name;
+        jobDescriptor.SetPlatformIdentifier("pc");
+        if (AzFramework::StringFunc::EndsWith(request.m_sourceFile.c_str(), relSourceFileName))
+        {
+            AssetBuilderSDK::SourceFileDependency dep = { secondRelSourceFile, AZ::Uuid::CreateNull() };
+            AssetBuilderSDK::JobDependency jobDep(builderDescriptor.m_name, "pc", AssetBuilderSDK::JobDependencyType::Order, dep);
+
+            // specify that we only depend on subid 0 of b.dummy, not the entire job so only reprocess if subid 0 changes
+            jobDep.m_productSubIds.push_back(0); // <------------------------------------------------- KEY LINE OF TEST DIFFERING FROM PRIOR TEST
+            jobDescriptor.m_jobDependencyList.emplace_back(jobDep);
+        }
+        response.m_createJobOutputs.emplace_back(jobDescriptor);
+        response.m_result = AssetBuilderSDK::CreateJobsResultCode::Success;
+    };
+    builderDescriptor.m_processJobFunction =
+        [](const AssetBuilderSDK::ProcessJobRequest& /*request*/, AssetBuilderSDK::ProcessJobResponse& response)
+    {
+        response.m_resultCode = AssetBuilderSDK::ProcessJobResultCode::ProcessJobResult_Success;
+    };
+
+    MockApplicationManager::BuilderFilePatternMatcherAndBuilderDesc builderFilePatternMatcher;
+    builderFilePatternMatcher.m_builderDesc = builderDescriptor;
+    builderFilePatternMatcher.m_internalBuilderName = builderDescriptor.m_name;
+    builderFilePatternMatcher.m_internalUuid = builderDescriptor.m_busId;
+    builderFilePatternMatcher.m_matcherBuilderPattern =
+        AssetUtilities::BuilderFilePatternMatcher(builderDescriptor.m_patterns.back(), builderDescriptor.m_busId);
+    m_mockApplicationManager->m_matcherBuilderPatterns.emplace_back(builderFilePatternMatcher);
+
+    // Capture the job details as the APM inspects the file.
+    AZStd::vector<JobDetails> jobDetails;
+    auto connection = QObject::connect(
+        m_assetProcessorManager.get(),
+        &AssetProcessorManager::AssetToProcess,
+        [&jobDetails](JobDetails job)
+        {
+            jobDetails.emplace_back(job);
+        });
+
+    // Tell the APM about both files.
+    m_isIdling = false;
+    QMetaObject::invokeMethod(m_assetProcessorManager.get(), "AssessModifiedFile", Qt::QueuedConnection, Q_ARG(QString, sourceFileName));
+    QMetaObject::invokeMethod(m_assetProcessorManager.get(), "AssessModifiedFile", Qt::QueuedConnection, Q_ARG(QString, secondSourceFile));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+
+    // Although we have processed a.dummy first, APM should send us notification of b.dummy job first and than of a.dummy job
+    EXPECT_EQ(jobDetails.size(), 2);
+    EXPECT_EQ(jobDetails[0].m_jobEntry.m_sourceAssetReference.AbsolutePath().c_str(), secondSourceFile);
+    EXPECT_EQ(jobDetails[1].m_jobEntry.m_sourceAssetReference.AbsolutePath().c_str(), sourceFileName);
+    EXPECT_EQ(jobDetails[1].m_jobDependencyList.size(), 1); // there should only be one job dependency
+    EXPECT_EQ(
+        jobDetails[1].m_jobDependencyList[0].m_jobDependency.m_sourceFile.m_sourceFileDependencyPath,
+        secondSourceFile.toUtf8().constData()); // there should only be one job dependency
+
+    // Process jobs in APM
+    auto destination = jobDetails[0].m_cachePath;
+    QString productAFileName = (destination / "aoutput.txt").AsPosix().c_str();
+    QString productBFileName = (destination / "boutput.txt").AsPosix().c_str();
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(productBFileName, QString("tempdata\n")));
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(productAFileName, QString("tempdata\n")));
+
+    AssetBuilderSDK::ProcessJobResponse responseB;
+    responseB.m_resultCode = AssetBuilderSDK::ProcessJobResult_Success;
+    responseB.m_outputProducts.push_back(AssetBuilderSDK::JobProduct("boutput.txt", AZ::Uuid::CreateNull(), 1)); // <-- note we are updating subid 1 only
+
+    AssetBuilderSDK::ProcessJobResponse responseA;
+    responseA.m_resultCode = AssetBuilderSDK::ProcessJobResult_Success;
+    responseA.m_outputProducts.push_back(AssetBuilderSDK::JobProduct("aoutput.txt", AZ::Uuid::CreateNull(), 1)); // <-- note we are updating subid 1 only
+
+    m_isIdling = false;
+    QMetaObject::invokeMethod(
+        m_assetProcessorManager.get(),
+        "AssetProcessed",
+        Qt::QueuedConnection,
+        Q_ARG(JobEntry, jobDetails[0].m_jobEntry),
+        Q_ARG(AssetBuilderSDK::ProcessJobResponse, responseB));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+
+    m_isIdling = false;
+    QMetaObject::invokeMethod(
+        m_assetProcessorManager.get(),
+        "AssetProcessed",
+        Qt::QueuedConnection,
+        Q_ARG(JobEntry, jobDetails[1].m_jobEntry),
+        Q_ARG(AssetBuilderSDK::ProcessJobResponse, responseA));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+
+    jobDetails.clear();
+    m_isIdling = false;
+    // Modify source file b.dummy, a.dummy should not process unless b.dummy outputs a file of subid 0.
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(secondSourceFile, QString("temp\n")));
+    QMetaObject::invokeMethod(m_assetProcessorManager.get(), "AssessModifiedFile", Qt::QueuedConnection, Q_ARG(QString, secondSourceFile));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+    ASSERT_EQ(jobDetails.size(), 1);
+    EXPECT_EQ(jobDetails[0].m_jobEntry.m_sourceAssetReference.AbsolutePath().c_str(), secondSourceFile);
+
+    // Ack processing to return to idle state.
+    m_isIdling = false;
+    QMetaObject::invokeMethod(
+        m_assetProcessorManager.get(),
+        "AssetProcessed",
+        Qt::QueuedConnection,
+        Q_ARG(JobEntry, jobDetails[0].m_jobEntry),
+        Q_ARG(AssetBuilderSDK::ProcessJobResponse, responseB));    // <-- note we are updating subid 1 only  This should not result in reprocessing anything
+    jobDetails.clear();
+    m_isIdling = false;
+    ASSERT_TRUE(BlockUntilIdle(5000));
+    ASSERT_EQ(jobDetails.size(), 0); // no jobs should have been emitted even though b.dummy finished, as it only updated subid 1 and it depends on subid 0
+    jobDetails.clear();
+    m_isIdling = false;
+
+    // KEY PART OF TEST
+    // modify source file b but this time, then output subid 0, which is the one the other job depends on.
+    ASSERT_TRUE(UnitTestUtils::CreateDummyFile(secondSourceFile, QString("temp2\n")));
+    QMetaObject::invokeMethod(m_assetProcessorManager.get(), "AssessModifiedFile", Qt::QueuedConnection, Q_ARG(QString, secondSourceFile));
+    ASSERT_TRUE(BlockUntilIdle(5000));
+
+    // We should only see one job so far, to process the file that has just changed.  Notifications of aditional files wait until the job finishes.
+    ASSERT_EQ(jobDetails.size(), 1);
+    EXPECT_EQ(jobDetails[0].m_jobEntry.m_sourceAssetReference.AbsolutePath().c_str(), secondSourceFile);
+
+    // Ack processing to return to idle state.  This should also trigger processing of the original file with the dep.
+    responseB.m_outputProducts[0].m_productSubID = 0;
+    m_isIdling = false;
+    QMetaObject::invokeMethod(
+        m_assetProcessorManager.get(),
+        "AssetProcessed",
+        Qt::QueuedConnection,
+        Q_ARG(JobEntry, jobDetails[0].m_jobEntry),
+        Q_ARG(AssetBuilderSDK::ProcessJobResponse, responseB));
+    jobDetails.clear();
+    ASSERT_TRUE(BlockUntilIdle(5000));
+    ASSERT_EQ(jobDetails.size(), 1);
+    EXPECT_EQ(jobDetails[0].m_jobEntry.m_sourceAssetReference.AbsolutePath().c_str(), sourceFileName);
+}
+
 
 TEST_F(AssetProcessorManagerTest, SourceFile_With_NonASCII_Characters_Job_OK)
 {

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

@@ -12,6 +12,19 @@
 
 namespace UnitTests
 {
+    // sets up data such that:
+
+    // SOURCE FILES DATABASE TABLE
+    //    source1 = parent.txt
+    //    source2 = child.txt
+    // JOB DATABASE TABLE
+    //    job1 "Mock Job" (completed)
+    // PRODUCT DATABASE TABLE
+    //    product.txt (subid 0) for job1       - sets hash to input "hashA"
+    //    procuct777.txt (subid 77) for job1   - sets hash to input "HashB"
+    // SOURCE FILE DEPENDENCY TABLE
+    //    JobToJob Dependency - child.txt DEPENDS ON parent.txt - SUBID is either empty, or 777
+
     void JobDependencySubIdTest::CreateTestData(AZ::u64 hashA, AZ::u64 hashB, bool useSubId)
     {
         using namespace AzToolsFramework::AssetDatabase;

Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp

@@ -190,7 +190,12 @@ namespace AZ
 
                 for (auto& jobDependency : outputJobDescriptor.m_jobDependencyList)
                 {
-                    if (jobDependency.m_platformIdentifier.empty())
+                    // we pre-populated these dependencies without any platform to depend on, ie, its blank.
+                    // Jobs depend on other jobs via a unique triplicate, which is (platform, job key, source file)
+                    // e.g. ("android", "Material Type Builder", "blah/whatever/foo.materialtype")
+                    // Anything can depend on the common platform (used for intermediate assets) but other platforms
+                    // should only depend on other assets from the same platform
+                    if (jobDependency.m_platformIdentifier.compare(AssetBuilderSDK::CommonPlatformName) != 0)
                     {
                         jobDependency.m_platformIdentifier = platformInfo.m_identifier;
                     }

Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialTypeBuilder.cpp

@@ -273,7 +273,12 @@ namespace AZ
 
                 for (auto& jobDependency : outputJobDescriptor.m_jobDependencyList)
                 {
-                    if (jobDependency.m_platformIdentifier.empty())
+                    // we pre-populated these dependencies without any platform to depend on, ie, its blank.
+                    // Jobs depend on other jobs via a unique triplicate, which is (platform, job key, source file)
+                    // e.g. ("android", "Material Type Builder", "blah/whatever/foo.materialtype")
+                    // Anything can depend on the common platform (used for intermediate assets) but other platforms
+                    // should only depend on other assets from the same platform.
+                    if (jobDependency.m_platformIdentifier.compare(AssetBuilderSDK::CommonPlatformName) != 0)
                     {
                         jobDependency.m_platformIdentifier = platformInfo.m_identifier;
                     }