Improve correlation by keeping arch info declared in manifest arp Dis…

…playName entry (#3100)

src/AppInstallerCLIE2ETests/ListCommand.cs

@@ -194,6 +194,35 @@ 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 -l {installDir}");
+            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"));
+
+            // best effort clean up
+            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,19 @@
+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}'
+      InstallLocation: /InstallDir <INSTALLPATH>
+ManifestType: singleton
+ManifestVersion: 1.2.0

src/AppInstallerCLIE2ETests/TestData/Manifests/TestMappingWithArchitectureX86.yaml

@@ -0,0 +1,19 @@
+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}'
+      InstallLocation: /InstallDir <INSTALLPATH>
+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

@@ -820,7 +820,8 @@ namespace AppInstaller::Repository
                     return Field == other.Field && String1 == other.String1 && String2 == other.String2;
                 }
 
-                void AddToFilters(std::vector<PackageMatchFilter>& filters) const
+                void AddToFilters(
+                    std::vector<PackageMatchFilter>& filters) const
                 {
                     switch (Field)
                     {
@@ -852,13 +853,14 @@ namespace AppInstaller::Repository
                     }
                 }
 
-                SearchRequest CreateInclusionsSearchRequest() const
+                SearchRequest CreateInclusionsSearchRequest(SearchPurpose searchPurpose) const
                 {
                     SearchRequest result;
                     for (const auto& srs : SystemReferenceStrings)
                     {
                         srs.AddToFilters(result.Inclusions);
                     }
+                    result.Purpose = searchPurpose;
                     return result;
                 }
             };
@@ -1221,7 +1223,8 @@ namespace AppInstaller::Repository
                 // Create a search request to run against all available sources
                 if (!installedPackageData.SystemReferenceStrings.empty())
                 {
-                    SearchRequest systemReferenceSearch = installedPackageData.CreateInclusionsSearchRequest();
+                    SearchRequest systemReferenceSearch = installedPackageData.CreateInclusionsSearchRequest(SearchPurpose::CorrelationToAvailable);
+                    AICLI_LOG(Repo, Info, << "Finding available package from installed package using system reference search: " << systemReferenceSearch.ToString());
 
                     Source trackedSource;
                     std::shared_ptr<IPackage> trackingPackage;
@@ -1239,8 +1242,8 @@ namespace AppInstaller::Repository
                         std::shared_ptr<IPackage> candidatePackage = GetMatchingPackage(trackingResult.Matches,
                             [&]() {
                                 AICLI_LOG(Repo, Info,
-                                    << "Found multiple matches for installed package [" << installedVersion->GetProperty(PackageVersionProperty::Id) <<
-                                    "] in tracking catalog for source [" << source.GetIdentifier() << "] when searching for [" << systemReferenceSearch.ToString() << "]");
+                                << "Found multiple matches for installed package [" << installedVersion->GetProperty(PackageVersionProperty::Id) <<
+                                "] in tracking catalog for source [" << source.GetIdentifier() << "] when searching for [" << systemReferenceSearch.ToString() << "]");
                             }, [&] {
                                 AICLI_LOG(Repo, Warning, << "  Appropriate tracking package could not be determined");
                             });
@@ -1265,8 +1268,12 @@ namespace AppInstaller::Repository
                     // Directly search for the available package from tracking information.
                     if (trackingPackage)
                     {
-                        addedAvailablePackage = true;
-                        compositePackage->AddAvailablePackage(GetTrackedPackageFromAvailableSource(result, trackedSource, trackingPackage->GetProperty(PackageProperty::Id)));
+                        auto availablePackage = GetTrackedPackageFromAvailableSource(result, trackedSource, trackingPackage->GetProperty(PackageProperty::Id));
+                        if (availablePackage)
+                        {
+                            addedAvailablePackage = true;
+                            compositePackage->AddAvailablePackage(std::move(availablePackage));
+                        }
                         compositePackage->SetTracking(std::move(trackedSource), std::move(trackingPackage), std::move(trackingPackageVersion));
                     }
 
@@ -1297,12 +1304,13 @@ namespace AppInstaller::Repository
                         auto availablePackage = GetMatchingPackage(availableResult.Matches,
                             [&]() {
                                 AICLI_LOG(Repo, Info,
-                                    << "Found multiple matches for installed package [" << installedVersion->GetProperty(PackageVersionProperty::Id) <<
-                                    "] in source [" << source.GetIdentifier() << "] when searching for [" << systemReferenceSearch.ToString() << "]");
+                                << "Found multiple matches for installed package [" << installedVersion->GetProperty(PackageVersionProperty::Id) <<
+                                "] in source [" << source.GetIdentifier() << "] when searching for [" << systemReferenceSearch.ToString() << "]");
                             }, [&] {
                                 AICLI_LOG(Repo, Warning, << "  Appropriate available package could not be determined");
                             });
 
+                        // For non pinning cases. We found some matching packages here, don't keep going.
                         addedAvailablePackage = true;
                         compositePackage->AddAvailablePackage(std::move(availablePackage));
                     }
@@ -1342,9 +1350,9 @@ namespace AppInstaller::Repository
                 // source to create a new composite package entry if we find any packages there.
                 if (packageData && !packageData->SystemReferenceStrings.empty())
                 {
-                    // Create a search request to run against the installed source
-                    SearchRequest systemReferenceSearch = packageData->CreateInclusionsSearchRequest();
+                    SearchRequest systemReferenceSearch = packageData->CreateInclusionsSearchRequest(SearchPurpose::CorrelationToInstalled);
 
+                    AICLI_LOG(Repo, Info, << "Finding installed package from tracking package using system reference search: " << systemReferenceSearch.ToString());
                     // Correlate against installed (allow exceptions out as we own the installed source)
                     SearchResult installedCrossRef = m_installedSource.Search(systemReferenceSearch);
 
@@ -1391,8 +1399,9 @@ namespace AppInstaller::Repository
                 if (packageData && !packageData->SystemReferenceStrings.empty())
                 {
                     // Create a search request to run against the installed source
-                    SearchRequest systemReferenceSearch = packageData->CreateInclusionsSearchRequest();
+                    SearchRequest systemReferenceSearch = packageData->CreateInclusionsSearchRequest(SearchPurpose::CorrelationToInstalled);
 
+                    AICLI_LOG(Repo, Info, << "Finding installed package from available package using system reference search: " << systemReferenceSearch.ToString());
                     // 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;