From e11b1e2c9c5434c28934bae268082d95e6e6812f Mon Sep 17 00:00:00 2001 From: amzn-mike <80125227+amzn-mike@users.noreply.github.com> Date: Tue, 2 Nov 2021 12:51:28 -0500 Subject: [PATCH] [LYN-7774] wildcard source dependencies not refreshing with new files (#5054) * Fix handling of absolute path dependencies when a newly added file satisfies a previously added dependency Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Fixed relative path wildcard dependencies matching absolute paths Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Update extra unit test to only run on windows since this problem doesn't apply to other OSes Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> --- .../AssetManager/assetProcessorManager.cpp | 23 +++-- .../AssetProcessorManagerTest.cpp | 88 +++++++++++++++++-- .../assetmanager/AssetProcessorManagerTest.h | 1 + 3 files changed, 102 insertions(+), 10 deletions(-) diff --git a/Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.cpp b/Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.cpp index 5e061491f0..367a296bd4 100644 --- a/Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.cpp +++ b/Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.cpp @@ -4027,30 +4027,43 @@ namespace AssetProcessor // It is generally called when a source file modified in any way, including when it is added or deleted. // note that this is a "reverse" dependency query - it looks up what depends on a file, not what the file depends on using namespace AzToolsFramework::AssetDatabase; - QStringList absoluteSourceFilePathQueue; + QSet absoluteSourceFilePathQueue; QString databasePath; QString scanFolder; - auto callbackFunction = [&](AzToolsFramework::AssetDatabase::SourceFileDependencyEntry& entry) + auto callbackFunction = [this, &absoluteSourceFilePathQueue](SourceFileDependencyEntry& entry) { QString relativeDatabaseName = QString::fromUtf8(entry.m_source.c_str()); QString absolutePath = m_platformConfig->FindFirstMatchingFile(relativeDatabaseName); if (!absolutePath.isEmpty()) { - absoluteSourceFilePathQueue.push_back(absolutePath); + absoluteSourceFilePathQueue.insert(absolutePath); } return true; }; + auto callbackFunctionAbsoluteCheck = [&callbackFunction](SourceFileDependencyEntry& entry) + { + if (AZ::IO::PathView(entry.m_dependsOnSource.c_str()).IsAbsolute()) + { + return callbackFunction(entry); + } + + return true; + }; + // convert to a database path so that the standard function can be called. if (m_platformConfig->ConvertToRelativePath(sourcePath, databasePath, scanFolder)) { m_stateData->QuerySourceDependencyByDependsOnSource(databasePath.toUtf8().constData(), nullptr, SourceFileDependencyEntry::DEP_Any, callbackFunction); - } - return absoluteSourceFilePathQueue; + // We'll also check with the absolute path, because we support absolute path dependencies + m_stateData->QuerySourceDependencyByDependsOnSource( + sourcePath.toUtf8().constData(), nullptr, SourceFileDependencyEntry::DEP_Any, callbackFunctionAbsoluteCheck); + + return absoluteSourceFilePathQueue.values(); } void AssetProcessorManager::AddSourceToDatabase(AzToolsFramework::AssetDatabase::SourceDatabaseEntry& sourceDatabaseEntry, const ScanFolderInfo* scanFolder, QString relativeSourceFilePath) diff --git a/Code/Tools/AssetProcessor/native/tests/assetmanager/AssetProcessorManagerTest.cpp b/Code/Tools/AssetProcessor/native/tests/assetmanager/AssetProcessorManagerTest.cpp index 0b02b168dc..9c20d99243 100644 --- a/Code/Tools/AssetProcessor/native/tests/assetmanager/AssetProcessorManagerTest.cpp +++ b/Code/Tools/AssetProcessor/native/tests/assetmanager/AssetProcessorManagerTest.cpp @@ -5318,6 +5318,18 @@ TEST_F(MetadataFileTest, MetadataFile_SourceFileExtensionDifferentCase) ASSERT_EQ(jobDetails.m_jobEntry.m_pathRelativeToWatchFolder, relFileName); } +AZStd::vector QStringListToVector(const QStringList& qstringList) +{ + AZStd::vector azVector; + // Convert to a vector of AZStd::strings because GTest handles this type better when displaying errors + for (const QString& resolvedPath : qstringList) + { + azVector.emplace_back(resolvedPath.toUtf8().constData()); + } + + return azVector; +} + bool WildcardSourceDependencyTest::Test( const AZStd::string& dependencyPath, AZStd::vector& resolvedPaths) { @@ -5326,15 +5338,18 @@ bool WildcardSourceDependencyTest::Test( AssetBuilderSDK::SourceFileDependency dependency(dependencyPath, AZ::Uuid::CreateNull(), AssetBuilderSDK::SourceFileDependency::SourceFileDependencyType::Wildcards); bool result = m_assetProcessorManager->ResolveSourceFileDependencyPath(dependency, resolvedName, stringlistPaths); - // Convert to a vector of AZStd::strings because GTest handles this type better when displaying errors - for (const QString& resolvedPath : stringlistPaths) - { - resolvedPaths.emplace_back(resolvedPath.toUtf8().constData()); - } + resolvedPaths = QStringListToVector(stringlistPaths); return result; } +AZStd::vector WildcardSourceDependencyTest::FileAddedTest(const QString& path) +{ + auto result = m_assetProcessorManager->GetSourceFilesWhichDependOnSourceFile(path); + + return QStringListToVector(result); +} + void WildcardSourceDependencyTest::SetUp() { AssetProcessorManagerTest::SetUp(); @@ -5357,6 +5372,42 @@ void WildcardSourceDependencyTest::SetUp() // Add a file in the non-recursive scanfolder. Since its not directly in the scan folder, it should always be ignored UnitTestUtils::CreateDummyFile(tempPath.absoluteFilePath("no_recurse/one/two/three/f.foo")); + + AzToolsFramework::AssetDatabase::SourceFileDependencyEntryContainer dependencies; + + // Relative path wildcard dependency + dependencies.push_back(AzToolsFramework::AssetDatabase::SourceFileDependencyEntry( + AZ::Uuid::CreateRandom(), "a.foo", "%a.foo", + AzToolsFramework::AssetDatabase::SourceFileDependencyEntry::DEP_SourceLikeMatch, 0)); + + // Absolute path wildcard dependency + dependencies.push_back(AzToolsFramework::AssetDatabase::SourceFileDependencyEntry( + AZ::Uuid::CreateRandom(), "b.foo", tempPath.absoluteFilePath("%b.foo").toUtf8().constData(), + AzToolsFramework::AssetDatabase::SourceFileDependencyEntry::DEP_SourceLikeMatch, 0)); + + // Test what happens when we have 2 dependencies on the same file + dependencies.push_back(AzToolsFramework::AssetDatabase::SourceFileDependencyEntry( + AZ::Uuid::CreateRandom(), "folder/one/d.foo", "%c.foo", + AzToolsFramework::AssetDatabase::SourceFileDependencyEntry::DEP_SourceLikeMatch, 0)); + + dependencies.push_back(AzToolsFramework::AssetDatabase::SourceFileDependencyEntry( + AZ::Uuid::CreateRandom(), "folder/one/d.foo", tempPath.absoluteFilePath("%c.foo").toUtf8().constData(), + AzToolsFramework::AssetDatabase::SourceFileDependencyEntry::DEP_SourceLikeMatch, 0)); + +#ifdef AZ_PLATFORM_WINDOWS + // Test to make sure a relative wildcard dependency doesn't match an absolute path + // For example, if the input is C:/project/subfolder1/a.foo + // This should not match a wildcard of c%.foo + // Take the first character of the tempPath and append %.foo onto it for this test, which should produce something like c%.foo + // This only applies to windows because on other OSes if the dependency starts with /, then its an abs path dependency + auto test = (tempPath.absolutePath().left(1) + "%.foo"); + dependencies.push_back(AzToolsFramework::AssetDatabase::SourceFileDependencyEntry( + AZ::Uuid::CreateRandom(), "folder/one/d.foo", + (test).toUtf8().constData(), + AzToolsFramework::AssetDatabase::SourceFileDependencyEntry::DEP_SourceLikeMatch, 0)); +#endif + + ASSERT_TRUE(m_assetProcessorManager->m_stateData->SetSourceFileDependencies(dependencies)); } TEST_F(WildcardSourceDependencyTest, Relative_Broad) @@ -5455,3 +5506,30 @@ TEST_F(WildcardSourceDependencyTest, Absolute_NoWildcard) ASSERT_FALSE(Test(tempPath.absoluteFilePath("subfolder1/1a.foo").toUtf8().constData(), resolvedPaths)); ASSERT_THAT(resolvedPaths, ::testing::UnorderedElementsAre()); } + +TEST_F(WildcardSourceDependencyTest, NewFile_MatchesSavedRelativeDependency) +{ + QDir tempPath(m_tempDir.path()); + + auto matches = FileAddedTest(tempPath.absoluteFilePath("subfolder1/1a.foo")); + + ASSERT_THAT(matches, ::testing::UnorderedElementsAre(tempPath.absoluteFilePath("subfolder2/redirected/a.foo").toUtf8().constData())); +} + +TEST_F(WildcardSourceDependencyTest, NewFile_MatchesSavedAbsoluteDependency) +{ + QDir tempPath(m_tempDir.path()); + + auto matches = FileAddedTest(tempPath.absoluteFilePath("subfolder1/1b.foo")); + + ASSERT_THAT(matches, ::testing::UnorderedElementsAre(tempPath.absoluteFilePath("subfolder2/redirected/b.foo").toUtf8().constData())); +} + +TEST_F(WildcardSourceDependencyTest, NewFile_MatchesDuplicatedDependenciesOnce) +{ + QDir tempPath(m_tempDir.path()); + + auto matches = FileAddedTest(tempPath.absoluteFilePath("subfolder2/redirected/folder/one/c.foo")); + + ASSERT_THAT(matches, ::testing::UnorderedElementsAre(tempPath.absoluteFilePath("subfolder2/redirected/folder/one/d.foo").toUtf8().constData())); +} diff --git a/Code/Tools/AssetProcessor/native/tests/assetmanager/AssetProcessorManagerTest.h b/Code/Tools/AssetProcessor/native/tests/assetmanager/AssetProcessorManagerTest.h index 94cc997938..d5afd8520f 100644 --- a/Code/Tools/AssetProcessor/native/tests/assetmanager/AssetProcessorManagerTest.h +++ b/Code/Tools/AssetProcessor/native/tests/assetmanager/AssetProcessorManagerTest.h @@ -135,6 +135,7 @@ struct WildcardSourceDependencyTest : AssetProcessorManagerTest { bool Test(const AZStd::string& dependencyPath, AZStd::vector& resolvedPaths); + AZStd::vector FileAddedTest(const QString& path); void SetUp() override; };