Add heuristics for matching packages to ARP after installing

.github/actions/spelling/excludes.txt

@@ -28,6 +28,7 @@ ignore$
 ^Localization/
 ^NOTICE$
 ^src/AppInstallerCLICore/Commands/ExperimentalCommand\.cpp$
+^src/AppInstallerCLITests/TestData/InputARPData.txt$
 ^src/AppInstallerCLITests/TestData/InputNames.txt$
 ^src/AppInstallerCLITests/TestData/InputPublishers.txt$
 ^src/AppInstallerCLITests/TestData/NormalizationInitialIds.txt$

src/AppInstallerCLICore/ExecutionContextData.h

@@ -47,6 +47,7 @@ namespace AppInstaller::CLI::Execution
         // On import: Sources for the imported packages
         Sources,
         ARPSnapshot,
+        ProductCodeFromARP,
         Dependencies,
         DependencySource,
         AllowedArchitectures,
@@ -190,6 +191,12 @@ namespace AppInstaller::CLI::Execution
             using value_t = std::vector<std::tuple<Utility::LocIndString, Utility::LocIndString, Utility::LocIndString>>;
         };
 
+        template <>
+        struct DataMapping<Data::ProductCodeFromARP>
+        {
+            using value_t = Utility::LocIndString;
+        };
+
         template <>
         struct DataMapping<Data::Dependencies>
         {

src/AppInstallerCLICore/Workflows/InstallFlow.cpp

@@ -12,6 +12,7 @@
 #include "WorkflowBase.h"
 #include "Workflows/DependenciesFlow.h"
 #include <AppInstallerDeployment.h>
+#include <winget/ARPCorrelation.h>
 
 using namespace winrt::Windows::ApplicationModel::Store::Preview::InstallControl;
 using namespace winrt::Windows::Foundation;
@@ -506,164 +507,125 @@ namespace AppInstaller::CLI::Workflow
 
     void ReportARPChanges(Execution::Context& context) try
     {
-        if (context.Contains(Execution::Data::ARPSnapshot))
+        if (!context.Contains(Execution::Data::ARPSnapshot))
         {
-            const auto& entries = context.Get<Execution::Data::ARPSnapshot>();
-
-            // Open it again to get the (potentially) changed ARP entries
-            Source arpSource = context.Reporter.ExecuteWithProgress(
-                [](IProgressCallback& progress)
-                {
-                    Repository::Source result = Repository::Source(PredefinedSource::ARP);
-                    result.Open(progress);
-                    return result;
-                }, true);
-
-            std::vector<ResultMatch> changes;
+            return;
+        }
 
-            for (auto& entry : arpSource.Search({}).Matches)
+        // Open the ARP source again to get the (potentially) changed ARP entries
+        Source arpSource = context.Reporter.ExecuteWithProgress(
+            [](IProgressCallback& progress)
             {
-                auto installed = entry.Package->GetInstalledVersion();
-
-                if (installed)
-                {
-                    auto entryKey = std::make_tuple(
-                        entry.Package->GetProperty(PackageProperty::Id),
-                        installed->GetProperty(PackageVersionProperty::Version),
-                        installed->GetProperty(PackageVersionProperty::Channel));
-
-                    auto itr = std::lower_bound(entries.begin(), entries.end(), entryKey);
-                    if (itr == entries.end() || *itr != entryKey)
-                    {
-                        changes.emplace_back(std::move(entry));
-                    }
-                }
-            }
+                Repository::Source result = Repository::Source(PredefinedSource::ARP);
+                result.Open(progress);
+                return result;
+            }, true);
 
-            // Also attempt to find the entry based on the manifest data
-            const auto& manifest = context.Get<Execution::Data::Manifest>();
-
-            SearchRequest nameAndPublisherRequest;
+        const auto& manifest = context.Get<Execution::Data::Manifest>();
 
-            // The default localization must contain the name or we cannot do this lookup
-            if (manifest.DefaultLocalization.Contains(Localization::PackageName))
+        // Try finding the package by product code in ARP.
+        // If we can find it now, we will be able to find it again later
+        // so we don't need to do anything else here.
+        SearchRequest productCodeSearchRequest;
+        std::vector<std::string> productCodes;
+        for (const auto& installer : manifest.Installers)
+        {
+            if (!installer.ProductCode.empty())
             {
-                AppInstaller::Manifest::Manifest::string_t defaultName = manifest.DefaultLocalization.Get<Localization::PackageName>();
-                AppInstaller::Manifest::Manifest::string_t defaultPublisher;
-                if (manifest.DefaultLocalization.Contains(Localization::Publisher))
+                if (std::find(productCodes.begin(), productCodes.end(), installer.ProductCode) == productCodes.end())
                 {
-                    defaultPublisher = manifest.DefaultLocalization.Get<Localization::Publisher>();
+                    productCodeSearchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::ProductCode, MatchType::Exact, installer.ProductCode));
+                    productCodes.emplace_back(installer.ProductCode);
                 }
+            }
+        }
 
-                nameAndPublisherRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact, defaultName, defaultPublisher));
+        SearchResult arpFoundByProductCode;
 
-                for (const auto& loc : manifest.Localizations)
-                {
-                    if (loc.Contains(Localization::PackageName) || loc.Contains(Localization::Publisher))
-                    {
-                        nameAndPublisherRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact,
-                            loc.Contains(Localization::PackageName) ? loc.Get<Localization::PackageName>() : defaultName,
-                            loc.Contains(Localization::Publisher) ? loc.Get<Localization::Publisher>() : defaultPublisher));
-                    }
-                }
-            }
+        // Don't execute this search if it would just find everything
+        if (!productCodeSearchRequest.IsForEverything())
+        {
+            arpFoundByProductCode = arpSource.Search(productCodeSearchRequest);
+        }
 
-            std::vector<std::string> productCodes;
-            for (const auto& installer : manifest.Installers)
-            {
-                if (!installer.ProductCode.empty())
-                {
-                    if (std::find(productCodes.begin(), productCodes.end(), installer.ProductCode) == productCodes.end())
-                    {
-                        nameAndPublisherRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::ProductCode, MatchType::Exact, installer.ProductCode));
-                        productCodes.emplace_back(installer.ProductCode);
-                    }
-                }
-            }
+        if (!arpFoundByProductCode.Matches.empty())
+        {
+            // TODO: Would we want to report changes in this case?
+            AICLI_LOG(CLI, Info, << "Installed package can be found in ARP by Product Code");
+            return;
+        }
 
-            SearchResult findByManifest;
+        // The product codes were not enough to find the package.
+        // We need to run some heuristics to try and match it with some ARP entry.
 
-            // Don't execute this search if it would just find everything
-            if (!nameAndPublisherRequest.IsForEverything())
-            {
-                findByManifest = arpSource.Search(nameAndPublisherRequest);
-            }
+        // First format the ARP data appropriately for the heuristic search
+        std::vector<Correlation::ARPEntry> arpEntries;
 
-            // Cross reference the changes with the search results
-            std::vector<std::shared_ptr<IPackage>> packagesInBoth;
+        size_t changedCount = 0;
+        const auto& arpSnapshot = context.Get<Execution::Data::ARPSnapshot>();
+        for (auto& entry : arpSource.Search({}).Matches)
+        {
+            auto installed = entry.Package->GetInstalledVersion();
 
-            for (const auto& change : changes)
+            if (installed)
             {
-                for (const auto& byManifest : findByManifest.Matches)
+                // Compare with the previous snapshot to see if it changed.
+                auto entryKey = std::make_tuple(
+                    entry.Package->GetProperty(PackageProperty::Id),
+                    installed->GetProperty(PackageVersionProperty::Version),
+                    installed->GetProperty(PackageVersionProperty::Channel));
+
+                auto itr = std::lower_bound(arpSnapshot.begin(), arpSnapshot.end(), entryKey);
+                bool isNewOrUpdated = (itr == arpSnapshot.end() || *itr != entryKey);
+                if (isNewOrUpdated)
                 {
-                    if (change.Package->IsSame(byManifest.Package.get()))
-                    {
-                        packagesInBoth.emplace_back(change.Package);
-                        break;
-                    }
+                    ++changedCount;
                 }
-            }
 
-            // We now have all of the package changes; time to report them.
-            // The set of cases we could have for changes to ARP:
-            //  0 packages  ::  No changes were detected to ARP, which could mean that the installer
-            //                  did not write an entry. It could also be a forced reinstall.
-            //  1 package   ::  Golden path; this should be what we installed.
-            //  2+ packages ::  We need to determine which package actually matches the one that we
-            //                  were installing.
-            //
-            // The set of cases we could have for finding packages based on the manifest:
-            //  0 packages  ::  The manifest data does not match the ARP information.
-            //  1 package   ::  Golden path; this should be what we installed.
-            //  2+ packages ::  The data in the manifest is either too broad or we have
-            //                  a problem with our name normalization.
-
-            // Find the package that we are going to log
-            std::shared_ptr<IPackageVersion> toLog;
-
-            // If there is only a single common package (changed and matches), it is almost certainly the correct one.
-            if (packagesInBoth.size() == 1)
-            {
-                toLog = packagesInBoth[0]->GetInstalledVersion();
-            }
-            // If it wasn't changed but we still find a match, that is the best thing to report.
-            else if (findByManifest.Matches.size() == 1)
-            {
-                toLog = findByManifest.Matches[0].Package->GetInstalledVersion();
-            }
-            // If only a single ARP entry was changed and we found no matches, report that.
-            else if (findByManifest.Matches.empty() && changes.size() == 1)
-            {
-                toLog = changes[0].Package->GetInstalledVersion();
+                arpEntries.emplace_back(installed, isNewOrUpdated);
             }
+        }
 
-            IPackageVersion::Metadata toLogMetadata;
-            if (toLog)
-            {
-                toLogMetadata = toLog->GetMetadata();
-            }
+        // Find the best match
+        const auto& correlationMeasure = Correlation::ARPCorrelationMeasure::GetInstance();
+        auto arpEntry = correlationMeasure.GetBestMatchForManifest(manifest, arpEntries)
+            ->Entry; // TODO: Fix; this was modified as a hack for the tests...
 
-            // We can only get the source identifier from an active source
-            std::string sourceIdentifier;
-            if (context.Contains(Execution::Data::PackageVersion))
-            {
-                sourceIdentifier = context.Get<Execution::Data::PackageVersion>()->GetProperty(PackageVersionProperty::SourceIdentifier);
-            }
+        IPackageVersion::Metadata arpEntryMetadata;
+        if (arpEntry)
+        {
+            arpEntryMetadata = arpEntry->GetMetadata();
+        }
 
-            Logging::Telemetry().LogSuccessfulInstallARPChange(
-                sourceIdentifier,
-                manifest.Id,
-                manifest.Version,
-                manifest.Channel,
-                changes.size(),
-                findByManifest.Matches.size(),
-                packagesInBoth.size(),
-                toLog ? static_cast<std::string>(toLog->GetProperty(PackageVersionProperty::Name)) : "",
-                toLog ? static_cast<std::string>(toLog->GetProperty(PackageVersionProperty::Version)) : "",
-                toLog ? static_cast<std::string_view>(toLogMetadata[PackageVersionMetadata::Publisher]) : "",
-                toLog ? static_cast<std::string_view>(toLogMetadata[PackageVersionMetadata::InstalledLocale]) : ""
-            );
+        // We can only get the source identifier from an active source
+        std::string sourceIdentifier;
+        if (context.Contains(Execution::Data::PackageVersion))
+        {
+            sourceIdentifier = context.Get<Execution::Data::PackageVersion>()->GetProperty(PackageVersionProperty::SourceIdentifier);
         }
+
+        // Store the ARP entry found to match the package to record it in the tracking catalog later
+        if (arpEntry)
+        {
+            // We use the product code as the ID in the ARP source.
+            context.Add<Data::ProductCodeFromARP>(arpEntry->GetProperty(PackageVersionProperty::Id));
+        }
+
+        // TODO: Revisit removed checks
+
+        Logging::Telemetry().LogSuccessfulInstallARPChange(
+            sourceIdentifier,
+            manifest.Id,
+            manifest.Version,
+            manifest.Channel,
+            changedCount,
+            0, // TODO findByManifest.Matches.size(),
+            0, // TODO packagesInBoth.size(),
+            arpEntry ? static_cast<std::string>(arpEntry->GetProperty(PackageVersionProperty::Name)) : "",
+            arpEntry ? static_cast<std::string>(arpEntry->GetProperty(PackageVersionProperty::Version)) : "",
+            arpEntry ? static_cast<std::string_view>(arpEntryMetadata[PackageVersionMetadata::Publisher]) : "",
+            arpEntry ? static_cast<std::string_view>(arpEntryMetadata[PackageVersionMetadata::InstalledLocale]) : ""
+        );
     }
     CATCH_LOG();
 
@@ -677,10 +639,21 @@ namespace AppInstaller::CLI::Workflow
             return;
         }
 
+        auto manifest = context.Get<Data::Manifest>();
+
+        // If we have determined an ARP entry matches the installed package,
+        // we set its product code in the manifest we record to ensure we can
+        // find it in the future.
+        // Note that this may overwrite existing information.
+        if (context.Contains(Data::ProductCodeFromARP))
+        {
+            manifest.DefaultInstallerInfo.ProductCode = context.Get<Data::ProductCodeFromARP>().get();
+        }
+
         auto trackingCatalog = context.Get<Data::PackageVersion>()->GetSource().GetTrackingCatalog();
 
         trackingCatalog.RecordInstall(
-            context.Get<Data::Manifest>(),
+            manifest,
             context.Get<Data::Installer>().value(),
             WI_IsFlagSet(context.GetFlags(), ContextFlag::InstallerExecutionUseUpdate));
     }

src/AppInstallerCLICore/Workflows/InstallFlow.h

@@ -167,15 +167,16 @@ namespace AppInstaller::CLI::Workflow
     // Outputs: ARPSnapshot
     void SnapshotARPEntries(Execution::Context& context);
 
-    // Reports on the changes between the stored ARPSnapshot and the current values.
+    // Reports on the changes between the stored ARPSnapshot and the current values,
+    // and stores the product code of the ARP entry found for the package.
     // Required Args: None
     // Inputs: ARPSnapshot?, Manifest, PackageVersion
-    // Outputs: None
+    // Outputs: ProductCodeFromARP?
     void ReportARPChanges(Execution::Context& context);
 
     // Records the installation to the tracking catalog.
     // Required Args: None
-    // Inputs: PackageVersion?, Manifest, Installer
+    // Inputs: PackageVersion?, Manifest, Installer, ProductCodeFromARP?
     // Outputs: None
     void RecordInstall(Execution::Context& context);
 }

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj

@@ -188,6 +188,7 @@
     <ClCompile Include="Command.cpp" />
     <ClCompile Include="Completion.cpp" />
     <ClCompile Include="CompositeSource.cpp" />
+    <ClCompile Include="Correlation.cpp" />
     <ClCompile Include="CustomHeader.cpp" />
     <ClCompile Include="Dependencies.cpp" />
     <ClCompile Include="Downloader.cpp" />
@@ -542,6 +543,9 @@
     <CopyFileToFolders Include="TestData\InputPublishers.txt">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InputARPData.txt">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\NormalizationInitialIds.txt">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>

src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters

@@ -188,6 +188,9 @@
     <ClCompile Include="PackageTrackingCatalog.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="Correlation.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <None Include="PropertySheet.props" />
@@ -555,5 +558,8 @@
     <CopyFileToFolders Include="TestData\Installer_Exe_DependenciesMultideclaration.yaml">
       <Filter>TestData</Filter>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InputARPData.txt">
+      <Filter>TestData</Filter>
+    </CopyFileToFolders>
   </ItemGroup>
 </Project>