Refactor to testable container; add tests

Author: JohnMcPMS

src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp

@@ -17,6 +17,7 @@
 #include <AppInstallerStrings.h>
 #include <winget/ExperimentalFeature.h>
 #include <winget/SelfManagement.h>
+#include <winget/PathTree.h>
 #include <winrt/Microsoft.Management.Configuration.h>
 
 using namespace AppInstaller::CLI::Execution;
@@ -1695,49 +1696,14 @@ namespace AppInstaller::CLI::Workflow
 
                 // Units whose location is at this node.
                 std::vector<IConfigurationUnitProcessorDetails> Units;
-
-                // The children of this node.
-                std::map<std::filesystem::path, Node> Children;
             };
 
-            Node m_rootNode;
-
-            Node* FindNode(const std::filesystem::path& path, bool createIfNeeded)
-            {
-                const auto& nodePath = std::filesystem::weakly_canonical(path);
-                Node* currentNode = &m_rootNode;
-
-                for (const auto& pathPart : nodePath)
-                {
-                    auto& children = currentNode->Children;
-
-                    if (createIfNeeded)
-                    {
-                        currentNode = &children[pathPart];
-                    }
-                    else
-                    {
-                        auto itr = children.find(pathPart);
-
-                        if (itr != children.end())
-                        {
-                            currentNode = &itr->second;
-                        }
-                        else
-                        {
-                            // Not found and should not create
-                            return nullptr;
-                        }
-                    }
-                }
-
-                return currentNode;
-            }
+            Filesystem::PathTree<Node> m_pathTree;
 
             Node& FindNodeForFilePath(const winrt::hstring& filePath)
             {
                 std::filesystem::path path{ std::wstring{ filePath } };
-                return *FindNode(path.parent_path(), true);
+                return m_pathTree.FindOrInsert(path.parent_path());
             }
 
         public:
@@ -1756,42 +1722,30 @@ namespace AppInstaller::CLI::Workflow
 
             void PlacePackage(const PackageCollection::Source& source, const PackageCollection::Package& package)
             {
-                Node* node = FindNode(package.InstalledLocation, false);
+                Node* node = m_pathTree.Find(package.InstalledLocation);
                 if (node)
                 {
                     node->Packages.emplace_back(SourceAndPackage{ source, package });
                 }
             }
 
-            std::vector<IConfigurationUnitProcessorDetails> GetResourcesForPackage(const PackageCollection::Package& package)
+            std::vector<IConfigurationUnitProcessorDetails> GetResourcesForPackage(const PackageCollection::Package& package) const
             {
                 std::vector<IConfigurationUnitProcessorDetails> result;
 
-                Node* node = FindNode(package.InstalledLocation, false);
-                if (node)
-                {
-                    std::queue<const Node*> nodes;
-                    nodes.push(node);
-
-                    while (!nodes.empty())
+                m_pathTree.VisitIf(
+                    package.InstalledLocation,
+                    [&](const Node& node)
                     {
-                        const Node* currentNode = nodes.front();
-                        nodes.pop();
-
-                        for (const auto& unit : currentNode->Units)
+                        for (const auto& unit : node.Units)
                         {
                             result.emplace_back(unit);
                         }
-
-                        for (const auto& child : currentNode->Children)
-                        {
-                            if (child.second.Packages.empty())
-                            {
-                                nodes.push(&child.second);
-                            }
-                        }
-                    }
-                }
+                    },
+                    [](const Node& node)
+                    {
+                        return node.Packages.empty();
+                    });
 
                 return result;
             }

src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs

@@ -31,7 +31,9 @@ public void BaseSetup()
             var installDir = TestCommon.GetRandomTestDir();
             TestCommon.RunAICLICommand("install", $"AppInstallerTest.TestPackageExport -v 1.0.0.0 --silent -l {installDir}");
             this.previousPathValue = System.Environment.GetEnvironmentVariable("PATH");
-            System.Environment.SetEnvironmentVariable("PATH", this.previousPathValue + ";" + installDir);
+
+            // The installer puts DSCv3 resources in both locations
+            System.Environment.SetEnvironmentVariable("PATH", this.previousPathValue + ";" + installDir + ";" + installDir + ".SubDirectory");
             DSCv3ResourceTestBase.EnsureTestResourcePresence();
         }
 
@@ -175,6 +177,10 @@ public void ExportAll()
             Assert.True(showResult.StdOut.Contains("AppInstallerTest/TestResource"));
             Assert.True(showResult.StdOut.Contains($"Dependencies: {Constants.TestSourceName}_AppInstallerTest.TestPackageExport"));
             Assert.True(showResult.StdOut.Contains("data: TestData"));
+
+            Assert.True(showResult.StdOut.Contains("AppInstallerTest/TestResource.SubDirectory"));
+            Assert.True(showResult.StdOut.Contains($"Dependencies: {Constants.TestSourceName}_AppInstallerTest.TestPackageExport"));
+            Assert.True(showResult.StdOut.Contains("data: TestData"));
         }
 
         /// <summary>

src/AppInstallerCLITests/Filesystem.cpp

@@ -3,6 +3,7 @@
 #include "pch.h"
 #include "TestCommon.h"
 #include <winget/Filesystem.h>
+#include <winget/PathTree.h>
 #include <AppInstallerStrings.h>
 
 using namespace AppInstaller::Utility;
@@ -113,3 +114,112 @@ TEST_CASE("GetExecutablePathForProcess", "[filesystem]")
     REQUIRE(thisExecutable.has_extension());
     REQUIRE(thisExecutable.filename() == L"AppInstallerCLITests.exe");
 }
+
+TEST_CASE("PathTree_InsertAndFind", "[filesystem][pathtree]")
+{
+    PathTree<bool> pathTree;
+
+    std::filesystem::path path1 = L"C:\\test";
+    std::filesystem::path path1sub = L"C:\\test\\sub";
+    std::filesystem::path path2 = L"C:\\diff";
+    std::filesystem::path path3 = L"D:\\test";
+
+    REQUIRE(nullptr == pathTree.Find(path1));
+    pathTree.FindOrInsert(path1) = true;
+
+    REQUIRE(nullptr != pathTree.Find(path1));
+    REQUIRE(*pathTree.Find(path1));
+
+    REQUIRE(nullptr == pathTree.Find(path1sub));
+    REQUIRE(nullptr == pathTree.Find(path2));
+    REQUIRE(nullptr == pathTree.Find(path3));
+}
+
+TEST_CASE("PathTree_InsertAndFind_Negative", "[filesystem][pathtree]")
+{
+    PathTree<bool> pathTree;
+    pathTree.FindOrInsert(L"C:\\a\\aa\\aaa");
+
+    REQUIRE(nullptr == pathTree.Find({}));
+    REQUIRE_THROWS_HR(pathTree.FindOrInsert({}), E_INVALIDARG);
+}
+
+size_t CountVisited(const PathTree<bool>& pathTree, const std::filesystem::path& path, std::function<bool(const bool&)> predicate)
+{
+    size_t result = 0;
+    pathTree.VisitIf(path, [&](const bool&) { ++result; }, predicate);
+    return result;
+}
+
+TEST_CASE("PathTree_VisitIf_Count", "[filesystem][pathtree]")
+{
+    PathTree<bool> pathTree;
+
+    pathTree.FindOrInsert(L"C:\\a\\aa\\aaa") = true;
+    pathTree.FindOrInsert(L"C:\\a\\aa\\bbb") = true;
+    pathTree.FindOrInsert(L"C:\\a\\aa\\ccc") = false;
+    pathTree.FindOrInsert(L"C:\\a\\aa") = true;
+
+    pathTree.FindOrInsert(L"C:\\a\\bb\\aaa") = false;
+    pathTree.FindOrInsert(L"C:\\a\\bb\\bbb") = true;
+    pathTree.FindOrInsert(L"C:\\a\\bb\\ccc") = false;
+    pathTree.FindOrInsert(L"C:\\a\\bb") = true;
+
+    pathTree.FindOrInsert(L"C:\\a\\cc\\aaa") = true;
+    pathTree.FindOrInsert(L"C:\\a\\cc\\bbb") = false;
+    pathTree.FindOrInsert(L"C:\\a\\cc\\ccc") = false;
+    pathTree.FindOrInsert(L"C:\\a\\cc") = false;
+
+    pathTree.FindOrInsert(L"C:\\a") = true;
+    pathTree.FindOrInsert(L"C:\\b") = false;
+    pathTree.FindOrInsert(L"D:\\a") = false;
+
+    auto always = [](const bool&) { return true; };
+    auto never = [](const bool&) { return false; };
+    auto if_input = [](const bool& b) { return b; };
+
+    REQUIRE(0 == CountVisited(pathTree, {}, always));
+
+    REQUIRE(15 == CountVisited(pathTree, L"C:\\", always));
+    REQUIRE(2 == CountVisited(pathTree, L"D:\\", always));
+    REQUIRE(0 == CountVisited(pathTree, L"E:\\", always));
+
+    REQUIRE(1 == CountVisited(pathTree, L"C:\\", never));
+    REQUIRE(1 == CountVisited(pathTree, L"D:\\", never));
+    REQUIRE(0 == CountVisited(pathTree, L"E:\\", never));
+
+    REQUIRE(7 == CountVisited(pathTree, L"C:\\", if_input));
+    REQUIRE(6 == CountVisited(pathTree, L"C:\\a", if_input));
+    REQUIRE(2 == CountVisited(pathTree, L"C:\\a\\cc", if_input));
+    REQUIRE(1 == CountVisited(pathTree, L"D:\\", if_input));
+    REQUIRE(0 == CountVisited(pathTree, L"E:\\", if_input));
+}
+
+TEST_CASE("PathTree_VisitIf_Correct", "[filesystem][pathtree]")
+{
+    PathTree<std::pair<bool, bool>> pathTree;
+
+    pathTree.FindOrInsert(L"C:\\a\\aa\\aaa") = { true, true };
+    pathTree.FindOrInsert(L"C:\\a\\aa\\bbb") = { true, true };
+    pathTree.FindOrInsert(L"C:\\a\\aa\\ccc") = { false, false };
+    pathTree.FindOrInsert(L"C:\\a\\aa") = { true, true };
+
+    pathTree.FindOrInsert(L"C:\\a\\bb\\aaa") = { false, false };
+    pathTree.FindOrInsert(L"C:\\a\\bb\\bbb") = { true, true };
+    pathTree.FindOrInsert(L"C:\\a\\bb\\ccc") = { false, false };
+    pathTree.FindOrInsert(L"C:\\a\\bb") = { true, true };
+
+    pathTree.FindOrInsert(L"C:\\a\\cc\\aaa") = { true, true };
+    pathTree.FindOrInsert(L"C:\\a\\cc\\bbb") = { false, false };
+    pathTree.FindOrInsert(L"C:\\a\\cc\\ccc") = { false, false };
+    pathTree.FindOrInsert(L"C:\\a\\cc") = { false, false };
+
+    pathTree.FindOrInsert(L"C:\\a") = { true, true };
+    pathTree.FindOrInsert(L"C:\\b") = { false, false };
+    pathTree.FindOrInsert(L"C:") = { true, false };
+
+    auto check_input = [](const std::pair<bool, bool>& p) { REQUIRE(p.first); };
+    auto if_input = [](const std::pair<bool, bool>& p) { return p.second; };
+
+    pathTree.VisitIf(L"C:", check_input, if_input);
+}

src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj

@@ -346,6 +346,7 @@
     <ClInclude Include="Public\winget\LocIndependent.h" />
     <ClInclude Include="Public\winget\ManagedFile.h" />
     <ClInclude Include="Public\winget\ModuleCountBase.h" />
+    <ClInclude Include="Public\winget\PathTree.h" />
     <ClInclude Include="Public\winget\Registry.h" />
     <ClInclude Include="Public\winget\Resources.h" />
     <ClInclude Include="Public\winget\Runtime.h" />

src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj.filters

@@ -146,6 +146,9 @@
     <ClInclude Include="Public\winget\DetectMismatch.h">
       <Filter>Public\winget</Filter>
     </ClInclude>
+    <ClInclude Include="Public\winget\PathTree.h">
+      <Filter>Public\winget</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="pch.cpp">