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$

.github/actions/spelling/expect.txt

@@ -158,6 +158,7 @@ hre
 hresults
 htm
 IAttachment
+IARP
 IConfiguration
 idx
 IFACEMETHODIMP
@@ -378,6 +379,7 @@ TStatus
 UCase
 ucasemap
 UChars
+ucnv
 uec
 uild
 uintptr

src/AppInstallerCLICore/ExecutionContextData.h

@@ -3,6 +3,7 @@
 #pragma once
 #include <winget/RepositorySource.h>
 #include <winget/Manifest.h>
+#include <winget/ARPCorrelation.h>
 #include "CompletionData.h"
 #include "PackageCollection.h"
 #include "Workflows/WorkflowBase.h"
@@ -47,6 +48,7 @@ namespace AppInstaller::CLI::Execution
         // On import: Sources for the imported packages
         Sources,
         ARPSnapshot,
+        CorrelatedAppsAndFeaturesEntries,
         Dependencies,
         DependencySource,
         AllowedArchitectures,
@@ -186,8 +188,13 @@ namespace AppInstaller::CLI::Execution
         template <>
         struct DataMapping<Data::ARPSnapshot>
         {
-            // Contains the { Id, Version, Channel }
-            using value_t = std::vector<std::tuple<Utility::LocIndString, Utility::LocIndString, Utility::LocIndString>>;
+            using value_t = std::vector<Repository::Correlation::ARPEntrySnapshot>;
+        };
+
+        template <>
+        struct DataMapping<Data::CorrelatedAppsAndFeaturesEntries>
+        {
+            using value_t = std::vector<Manifest::AppsAndFeaturesEntry>;
         };
 
         template <>

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,173 +507,78 @@ 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;
-
-            for (auto& entry : arpSource.Search({}).Matches)
-            {
-                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));
-                    }
-                }
-            }
-
-            // Also attempt to find the entry based on the manifest data
-            const auto& manifest = context.Get<Execution::Data::Manifest>();
-
-            SearchRequest manifestSearchRequest;
-            AppInstaller::Manifest::Manifest::string_t defaultPublisher;
-            if (manifest.DefaultLocalization.Contains(Localization::Publisher))
-            {
-                defaultPublisher = manifest.DefaultLocalization.Get<Localization::Publisher>();
-            }
-
-            // The default localization must contain the name or we cannot do this lookup
-            if (manifest.DefaultLocalization.Contains(Localization::PackageName))
-            {
-                AppInstaller::Manifest::Manifest::string_t defaultName = manifest.DefaultLocalization.Get<Localization::PackageName>();
-                manifestSearchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact, defaultName, defaultPublisher));
+            return;
+        }
 
-                for (const auto& loc : manifest.Localizations)
-                {
-                    if (loc.Contains(Localization::PackageName) || loc.Contains(Localization::Publisher))
-                    {
-                        manifestSearchRequest.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));
-                    }
-                }
-            }
+        const auto& manifest = context.Get<Execution::Data::Manifest>();
+        const auto& arpSnapshot = context.Get<Execution::Data::ARPSnapshot>();
 
-            std::vector<std::string> productCodes;
-            for (const auto& installer : manifest.Installers)
+        // Open the ARP source again to get the (potentially) changed ARP entries
+        Source arpSource = context.Reporter.ExecuteWithProgress(
+            [](IProgressCallback& progress)
             {
-                if (!installer.ProductCode.empty())
-                {
-                    if (std::find(productCodes.begin(), productCodes.end(), installer.ProductCode) == productCodes.end())
-                    {
-                        manifestSearchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::ProductCode, MatchType::Exact, installer.ProductCode));
-                        productCodes.emplace_back(installer.ProductCode);
-                    }
-                }
+                Repository::Source result = Repository::Source(PredefinedSource::ARP);
+                result.Open(progress);
+                return result;
+            }, true);
 
-                for (const auto& appsAndFeaturesEntry : installer.AppsAndFeaturesEntries)
-                {
-                    if (!appsAndFeaturesEntry.DisplayName.empty())
-                    {
-                        manifestSearchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact,
-                            appsAndFeaturesEntry.DisplayName,
-                            appsAndFeaturesEntry.Publisher.empty() ? defaultPublisher : appsAndFeaturesEntry.Publisher));
-                    }
-                }
-            }
+        auto correlationResult = Correlation::FindARPEntryForNewlyInstalledPackage(manifest, arpSnapshot, arpSource);
 
-            SearchResult findByManifest;
+        // Store the ARP entry found to match the package to record it in the tracking catalog later
+        if (correlationResult.Package)
+        {
+            std::vector<AppsAndFeaturesEntry> entries;
 
-            // Don't execute this search if it would just find everything
-            if (!manifestSearchRequest.IsForEverything())
-            {
-                findByManifest = arpSource.Search(manifestSearchRequest);
-            }
+            auto metadata = correlationResult.Package->GetMetadata();
 
-            // Cross reference the changes with the search results
-            std::vector<std::shared_ptr<IPackage>> packagesInBoth;
+            AppsAndFeaturesEntry baseEntry;
 
-            for (const auto& change : changes)
-            {
-                for (const auto& byManifest : findByManifest.Matches)
-                {
-                    if (change.Package->IsSame(byManifest.Package.get()))
-                    {
-                        packagesInBoth.emplace_back(change.Package);
-                        break;
-                    }
-                }
-            }
+            // Display name and publisher are also available as multi properties, but
+            // for ARP there will always be only 0 or 1 values.
+            baseEntry.DisplayName = correlationResult.Package->GetProperty(PackageVersionProperty::Name).get();
+            baseEntry.Publisher = correlationResult.Package->GetProperty(PackageVersionProperty::Publisher).get();
+            baseEntry.DisplayVersion = correlationResult.Package->GetProperty(PackageVersionProperty::Version).get();
+            baseEntry.InstallerType = Manifest::ConvertToInstallerTypeEnum(metadata[PackageVersionMetadata::InstalledType]);
 
-            // 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)
+            auto productCodes = correlationResult.Package->GetMultiProperty(PackageVersionMultiProperty::ProductCode);
+            for (auto&& productCode : productCodes)
             {
-                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();
+                AppsAndFeaturesEntry entry = baseEntry;
+                entry.ProductCode = std::move(productCode).get();
+                entries.push_back(std::move(entry));
             }
 
-            IPackageVersion::Metadata toLogMetadata;
-            if (toLog)
-            {
-                toLogMetadata = toLog->GetMetadata();
-            }
+            context.Add<Data::CorrelatedAppsAndFeaturesEntries>(std::move(entries));
+        }
 
-            // 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);
-            }
+        // 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);
+        }
 
-            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]) : ""
-            );
+        IPackageVersion::Metadata arpEntryMetadata;
+        if (correlationResult.Package)
+        {
+            arpEntryMetadata = correlationResult.Package->GetMetadata();
         }
+
+        Logging::Telemetry().LogSuccessfulInstallARPChange(
+            sourceIdentifier,
+            manifest.Id,
+            manifest.Version,
+            manifest.Channel,
+            correlationResult.ChangesToARP,
+            correlationResult.MatchesInARP,
+            correlationResult.CountOfIntersectionOfChangesAndMatches,
+            correlationResult.Package ? static_cast<std::string>(correlationResult.Package->GetProperty(PackageVersionProperty::Name)) : "",
+            correlationResult.Package ? static_cast<std::string>(correlationResult.Package->GetProperty(PackageVersionProperty::Version)) : "",
+            correlationResult.Package ? static_cast<std::string>(correlationResult.Package->GetProperty(PackageVersionProperty::Publisher)) : "",
+            correlationResult.Package ? static_cast<std::string_view>(arpEntryMetadata[PackageVersionMetadata::InstalledLocale]) : ""
+        );
     }
     CATCH_LOG();
 
@@ -686,10 +592,23 @@ 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::CorrelatedAppsAndFeaturesEntries))
+        {
+            // Use a new Installer entry
+            manifest.Installers.emplace_back();
+            manifest.Installers.back().AppsAndFeaturesEntries = context.Get<Data::CorrelatedAppsAndFeaturesEntries>();
+        }
+
         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: CorrelatedAppsAndFeaturesEntries?
     void ReportARPChanges(Execution::Context& context);
 
     // Records the installation to the tracking catalog.
     // Required Args: None
-    // Inputs: PackageVersion?, Manifest, Installer
+    // Inputs: PackageVersion?, Manifest, Installer, CorrelatedAppsAndFeaturesEntries?
     // Outputs: None
     void RecordInstall(Execution::Context& context);
 }