Improve correlation by keeping arch info declared in manifest arp DisplayName entry

src/AppInstallerCLIE2ETests/ListCommand.cs

@@ -194,6 +194,34 @@ public void ListWithScopeMsixInstalledAsUser()
             TestCommon.RemoveMsix(Constants.MsixInstallerName);
         }
 
+        /// <summary>
+        /// Test package correlation with same package name but different architecture is correct.
+        /// </summary>
+        [Test]
+        public void ListWithMappingWithArchitecture()
+        {
+            var installDir = TestCommon.GetRandomTestDir();
+            var result = TestCommon.RunAICLICommand("install", "AppInstallerTest.TestMappingWithArchitectureX86");
+            Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
+
+            // List with AppInstallerTest.TestMappingWithArchitectureX64 (from available to installed scenario) will not find the package.
+            result = TestCommon.RunAICLICommand("list", "AppInstallerTest.TestMappingWithArchitectureX64");
+            Assert.AreEqual(Constants.ErrorCode.ERROR_NO_APPLICATIONS_FOUND, result.ExitCode);
+            Assert.False(result.StdOut.Contains("AppInstallerTest.TestMappingWithArchitectureX64"));
+
+            // List with AppInstallerTest.TestMappingWithArchitectureX86 (from available to installed scenario) will find the package.
+            result = TestCommon.RunAICLICommand("list", "AppInstallerTest.TestMappingWithArchitectureX86");
+            Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
+            Assert.True(result.StdOut.Contains("AppInstallerTest.TestMappingWithArchitectureX86"));
+
+            // List with product code (from installed to available scenario) will find the AppInstallerTest.TestMappingWithArchitectureX86 package.
+            result = TestCommon.RunAICLICommand("list", "{0e426f01-b682-4e67-a357-52f9ecb4590d}");
+            Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
+            Assert.True(result.StdOut.Contains("AppInstallerTest.TestMappingWithArchitectureX86"));
+
+            TestCommon.RunCommand(Path.Combine(installDir, Constants.TestExeUninstallerFileName));
+        }
+
         private void ArpVersionMappingTest(string packageIdentifier, string displayNameOverride, string displayVersionOverride, string expectedListVersion, string notExpectedListVersion = "")
         {
             System.Guid guid = System.Guid.NewGuid();

src/AppInstallerCLIE2ETests/TestData/Manifests/TestMappingWithArchitectureX64.yaml

@@ -0,0 +1,18 @@
+PackageIdentifier: AppInstallerTest.TestMappingWithArchitectureX64
+PackageVersion: 1.0
+PackageName: TestMappingWithArchitecture
+PackageLocale: en-US
+Publisher: Microsoft
+License: Test
+ShortDescription: E2E test for mapping with architecture.
+Installers:
+  - Architecture: x64
+    InstallerUrl: https://localhost:5001/TestKit/AppInstallerTestExeInstaller/AppInstallerTestExeInstaller.exe
+    InstallerType: exe
+    InstallerSha256: <EXEHASH>
+    AppsAndFeaturesEntries:
+    - DisplayName: "TestMappingWithArchitecture(X64)"
+    InstallerSwitches:
+      Custom: '/DisplayName TestMappingWithArchitecture(X64) /ProductID {0e426f01-b682-4e67-a357-52f9ecb4590d}'
+ManifestType: singleton
+ManifestVersion: 1.2.0

src/AppInstallerCLIE2ETests/TestData/Manifests/TestMappingWithArchitectureX86.yaml

@@ -0,0 +1,18 @@
+PackageIdentifier: AppInstallerTest.TestMappingWithArchitectureX86
+PackageVersion: 1.0
+PackageName: TestMappingWithArchitecture
+PackageLocale: en-US
+Publisher: Microsoft
+License: Test
+ShortDescription: E2E test for mapping with architecture.
+Installers:
+  - Architecture: x86
+    InstallerUrl: https://localhost:5001/TestKit/AppInstallerTestExeInstaller/AppInstallerTestExeInstaller.exe
+    InstallerType: exe
+    InstallerSha256: <EXEHASH>
+    AppsAndFeaturesEntries:
+    - DisplayName: "TestMappingWithArchitecture(X86)"
+    InstallerSwitches:
+      Custom: '/DisplayName TestMappingWithArchitecture(X86) /ProductID {0e426f01-b682-4e67-a357-52f9ecb4590d}'
+ManifestType: singleton
+ManifestVersion: 1.2.0

src/AppInstallerCLITests/NameNormalization.cpp

@@ -139,3 +139,18 @@ TEST_CASE("NameNorm_Initial_PreserveWhitespace", "[name_norm]")
     REQUIRE(normer.NormalizeName("Some Name").Name() == "Some Name");
     REQUIRE(normer.NormalizePublisher("Some Publisher Corp") == "Some Publisher");
 }
+
+TEST_CASE("NameNorm_GetNormalizedName_GetNormalizedFields", "[name_norm]")
+{
+    NameNormalizer normer(NormalizationVersion::Initial);
+
+    auto normalizedName = normer.NormalizeName("Name(X64)");
+    REQUIRE(normalizedName.GetNormalizedName(NormalizationField::None) == "Name");
+    REQUIRE(normalizedName.GetNormalizedName(NormalizationField::Architecture) == "Name(X64)");
+    REQUIRE(normalizedName.GetNormalizedFields() == NormalizationField::Architecture);
+
+    auto normalizedName2 = normer.NormalizeName("Name");
+    REQUIRE(normalizedName2.GetNormalizedName(NormalizationField::None) == "Name");
+    REQUIRE(normalizedName2.GetNormalizedName(NormalizationField::Architecture) == "Name");
+    REQUIRE(normalizedName2.GetNormalizedFields() == NormalizationField::None);
+}

src/AppInstallerCommonCore/NameNormalization.cpp

@@ -501,4 +501,28 @@ namespace AppInstaller::Utility
     {
         return m_normalizer->NormalizePublisher(publisher);
     }
+
+    std::string NormalizedName::GetNormalizedName(NormalizationField fieldsToInclude) const
+    {
+        std::string result = Name();
+
+        if (WI_IsFlagSet(fieldsToInclude, NormalizationField::Architecture) && m_arch != Utility::Architecture::Unknown)
+        {
+            result += '(' + std::string(Utility::ToString(m_arch)) + ')';
+        }
+
+        return result;
+    }
+
+    NormalizationField NormalizedName::GetNormalizedFields() const
+    {
+        NormalizationField result = NormalizationField::None;
+
+        if (m_arch != Utility::Architecture::Unknown)
+        {
+            result |= NormalizationField::Architecture;
+        }
+
+        return result;
+    }
 }

src/AppInstallerCommonCore/Public/winget/NameNormalization.h

@@ -16,6 +16,16 @@ namespace AppInstaller::Utility
         InitialPreserveWhiteSpace,
     };
 
+    // List of name normalization fields. Architecture, locale, etc.
+    // Currently only architecture is used.
+    enum class NormalizationField : uint32_t
+    {
+        None = 0x0,
+        Architecture = 0x1,
+    };
+
+    DEFINE_ENUM_FLAG_OPERATORS(NormalizationField);
+
     struct NameNormalizer;
 
     // A package publisher and name that has been normalized, allowing direct
@@ -39,6 +49,11 @@ namespace AppInstaller::Utility
         void Publisher(std::string&& publisher) { m_publisher = std::move(publisher); }
         void Publisher(std::string_view publisher) { m_publisher = publisher; }
 
+        // Gets normalized name with additional normalization fields included.
+        std::string GetNormalizedName(NormalizationField fieldsToInclude) const;
+        // Gets a flag indicating the list of fields detected in normalization.
+        NormalizationField GetNormalizedFields() const;
+
     private:
         std::string m_name;
         Utility::Architecture m_arch = Utility::Architecture::Unknown;

src/AppInstallerRepositoryCore/CompositeSource.cpp

@@ -4,6 +4,7 @@
 #include "CompositeSource.h"
 #include "Microsoft/PinningIndex.h"
 #include <winget/ExperimentalFeature.h>
+#include <winget/NameNormalization.h>
 
 using namespace AppInstaller::Repository::Microsoft;
 using namespace AppInstaller::Settings;
@@ -842,7 +843,15 @@ namespace AppInstaller::Repository
             // Data relevant to correlation for a package.
             struct PackageData
             {
+                // ProductCode, PackageFamilyName, etc
                 std::set<SystemReferenceString> SystemReferenceStrings;
+                // Normalized Publisher and Names, with NormalizationField info for creating filtered search request.
+                std::map<SystemReferenceString, Utility::NormalizationField> NameAndPublishers;
+
+                bool Empty()
+                {
+                    return SystemReferenceStrings.empty() && NameAndPublishers.empty();
+                }
 
                 void AddIfNotPresent(SystemReferenceString&& srs)
                 {
@@ -852,18 +861,52 @@ namespace AppInstaller::Repository
                     }
                 }
 
-                SearchRequest CreateInclusionsSearchRequest() const
+                void AddNameAndPublisherIfNotPresent(SystemReferenceString&& srs, Utility::NormalizationField normalizationField)
+                {
+                    if (NameAndPublishers.find(srs) == NameAndPublishers.end())
+                    {
+                        NameAndPublishers.emplace(std::move(srs), normalizationField);
+                    }
+                }
+
+                // If filter is std::nullopt, all values will be included.
+                SearchRequest CreateInclusionsSearchRequest(std::optional<Utility::NormalizationField> filter = std::nullopt) const
                 {
                     SearchRequest result;
                     for (const auto& srs : SystemReferenceStrings)
                     {
                         srs.AddToFilters(result.Inclusions);
                     }
+                    for (const auto& nameAndPublisher : NameAndPublishers)
+                    {
+                        // We specifically use direct compare of NormalizationField here since most likely in the future,
+                        // The matching logic will try narrowing down list of strings step by step.
+                        // For example, try Name|Arch|Locale, then Name|Arch, then Name.
+                        if (filter.has_value() && nameAndPublisher.second != filter.value())
+                        {
+                            continue;
+                        }
+                        nameAndPublisher.first.AddToFilters(result.Inclusions);
+                    }
                     return result;
                 }
+
+                bool PackageNamesContainNormalizationField(Utility::NormalizationField filter)
+                {
+                    for (const auto& nameAndPublisher : NameAndPublishers)
+                    {
+                        if (nameAndPublisher.second == filter)
+                        {
+                            return true;;
+                        }
+                    }
+
+                    return false;
+                }
             };
 
-            // For a given package version, prepares the results for it.
+            // For a given package version, prepares the system reference strings for correlation.
+            // Excluding normalized publisher and name.
             PackageData GetSystemReferenceStrings(IPackageVersion* version)
             {
                 PackageData result;
@@ -1054,14 +1097,17 @@ namespace AppInstaller::Repository
                 auto names = installedVersion->GetMultiProperty(PackageVersionMultiProperty::Name);
                 auto publishers = installedVersion->GetMultiProperty(PackageVersionMultiProperty::Publisher);
 
+                Utility::NameNormalizer normer(Utility::NormalizationVersion::Initial);
+
                 for (size_t i = 0; i < names.size(); ++i)
                 {
+                    auto normalizationField = normer.NormalizeName(names[i]).GetNormalizedFields();
                     for (size_t j = 0; j < publishers.size(); ++j)
                     {
-                        data.AddIfNotPresent(SystemReferenceString{
+                        data.AddNameAndPublisherIfNotPresent(SystemReferenceString{
                             PackageMatchField::NormalizedNameAndPublisher,
                             names[i],
-                            publishers[j] });
+                            publishers[j] }, normalizationField);
                     }
                 }
             }
@@ -1218,11 +1264,8 @@ namespace AppInstaller::Repository
 
                 auto installedPackageData = result.GetSystemReferenceStrings(installedVersion.get());
 
-                // Create a search request to run against all available sources
-                if (!installedPackageData.SystemReferenceStrings.empty())
+                auto tryAddAvailablePackage = [&](const SearchRequest& systemReferenceSearch) -> bool
                 {
-                    SearchRequest systemReferenceSearch = installedPackageData.CreateInclusionsSearchRequest();
-
                     Source trackedSource;
                     std::shared_ptr<IPackage> trackingPackage;
                     std::shared_ptr<IPackageVersion> trackingPackageVersion;
@@ -1306,6 +1349,31 @@ namespace AppInstaller::Repository
                         addedAvailablePackage = true;
                         compositePackage->AddAvailablePackage(std::move(availablePackage));
                     }
+
+                    return addedAvailablePackage;
+                };
+
+                // Create a search request to run against all available sources
+                if (!installedPackageData.Empty())
+                {
+                    // For installed package to available package mapping,
+                    // try the search with NormalizedName with Arch first, if not found, try with all values.
+                    // This can be extended in the future for more granular search requests.
+                    std::vector<SearchRequest> candidateSearches;
+                    if (installedPackageData.PackageNamesContainNormalizationField(Utility::NormalizationField::Architecture))
+                    {
+                        candidateSearches.emplace_back(installedPackageData.CreateInclusionsSearchRequest(Utility::NormalizationField::Architecture));
+                    }
+                    candidateSearches.emplace_back(installedPackageData.CreateInclusionsSearchRequest());
+
+                    for (const auto& candidateSearch : candidateSearches)
+                    {
+                        if (tryAddAvailablePackage(candidateSearch))
+                        {
+                            // If found available package, break. Otherwise try next search request.
+                            break;
+                        }
+                    }
                 }
 
                 // Move the installed result into the composite result
@@ -1340,10 +1408,15 @@ namespace AppInstaller::Repository
 
                 // If no package was found that was already in the results, do a correlation lookup with the installed
                 // source to create a new composite package entry if we find any packages there.
-                if (packageData && !packageData->SystemReferenceStrings.empty())
+                if (packageData && !packageData->Empty())
                 {
                     // Create a search request to run against the installed source
-                    SearchRequest systemReferenceSearch = packageData->CreateInclusionsSearchRequest();
+                    // For available package to installed package mapping, only one try is needed.
+                    // For example, if arp DisplayName contains arch, then the local packages's arp DisplayName should also include arch.
+                    SearchRequest systemReferenceSearch =
+                        packageData->PackageNamesContainNormalizationField(Utility::NormalizationField::Architecture) ?
+                        packageData->CreateInclusionsSearchRequest(Utility::NormalizationField::Architecture) :
+                        packageData->CreateInclusionsSearchRequest();
 
                     // Correlate against installed (allow exceptions out as we own the installed source)
                     SearchResult installedCrossRef = m_installedSource.Search(systemReferenceSearch);
@@ -1388,10 +1461,15 @@ namespace AppInstaller::Repository
                 // If no package was found that was already in the results, do a correlation lookup with the installed
                 // source to create a new composite package entry if we find any packages there.
                 bool foundInstalledMatch = false;
-                if (packageData && !packageData->SystemReferenceStrings.empty())
+                if (packageData && !packageData->Empty())
                 {
                     // Create a search request to run against the installed source
-                    SearchRequest systemReferenceSearch = packageData->CreateInclusionsSearchRequest();
+                    // For available package to installed package mapping, only one try is needed.
+                    // For example, if arp DisplayName contains arch, then the local packages's arp DisplayName should also include arch.
+                    SearchRequest systemReferenceSearch =
+                        packageData->PackageNamesContainNormalizationField(Utility::NormalizationField::Architecture) ?
+                        packageData->CreateInclusionsSearchRequest(Utility::NormalizationField::Architecture) :
+                        packageData->CreateInclusionsSearchRequest();
 
                     // Correlate against installed (allow exceptions out as we own the installed source)
                     SearchResult installedCrossRef = m_installedSource.Search(systemReferenceSearch);

src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp

@@ -363,13 +363,19 @@ namespace AppInstaller::Repository::Microsoft
                     continue;
                 }
                 auto displayNameValue = displayName->GetValue<Registry::Value::Type::String>();
-                manifest.DefaultLocalization.Add<Manifest::Localization::PackageName>(displayNameValue);
                 if (displayNameValue.empty())
                 {
                     AICLI_LOG(Repo, Verbose, << "Skipping " << productCode << " because DisplayName is empty");
                     continue;
                 }
 
+                manifest.DefaultLocalization.Add<Manifest::Localization::PackageName>(displayNameValue);
+                // Add DisplayName to ARP entries too
+                // This is to help normalized publisher and name correlation where ARP DisplayName matching
+                // will be getting improved in future iterations.
+                manifest.Installers[0].AppsAndFeaturesEntries.emplace_back();
+                manifest.Installers[0].AppsAndFeaturesEntries[0].DisplayName = displayNameValue;
+
                 // If no version can be determined, ignore this entry
                 manifest.Version = DetermineVersion(arpKey);
                 if (manifest.Version.empty())
@@ -404,7 +410,6 @@ namespace AppInstaller::Repository::Microsoft
                     auto upgradeCodeItr = upgradeCodes.find(productCode);
                     if (upgradeCodeItr != upgradeCodes.end())
                     {
-                        manifest.Installers[0].AppsAndFeaturesEntries.emplace_back();
                         manifest.Installers[0].AppsAndFeaturesEntries[0].UpgradeCode = upgradeCodeItr->second;
                     }
                 }

src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp

@@ -30,7 +30,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1
         std::vector<Utility::NormalizedString> GetSystemReferenceStrings(
             const Manifest::Manifest& manifest,
             std::function<const Utility::NormalizedString&(const Manifest::ManifestInstaller&)> extractStringFromInstaller,
-            std::function<const Utility::NormalizedString& (const Manifest::AppsAndFeaturesEntry&)> extractStringFromAppsAndFeaturesEntry = {})
+            std::function<const Utility::NormalizedString&(const Manifest::AppsAndFeaturesEntry&)> extractStringFromAppsAndFeaturesEntry = {})
         {
             std::set<Utility::NormalizedString> set;