From ff86443846239157d03a74d1b57115b5d6152ad1 Mon Sep 17 00:00:00 2001
From: JohnMcPMS <johnmcp@microsoft.com>
Date: Wed, 13 Nov 2024 15:58:07 -0800
Subject: [PATCH] Don't put ARP version data into tracking catalog (#4964)

## Change
A previous change was made to block overlapping ARP ranges being put
into the index. The fact that this was potentially impactful to the
tracking catalog was put off until now.

This change removes ARP version information (when present) from the
manifest before placing it into the tracking catalog. We are not using
this data, and I don't see a reason that we would. Attempting to
preserve it would probably require removing all existing ARP version
information when adding/updating the tracking manifest.

## Validation
Added a test that to insert an overlapping range into the tracking
catalog.
---
 .../PackageTrackingCatalog.cpp                | 42 +++++++++++++++++++
 .../PackageTrackingCatalog.cpp                | 11 ++++-
 .../Public/winget/PackageTrackingCatalog.h    |  2 +-
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/src/AppInstallerCLITests/PackageTrackingCatalog.cpp b/src/AppInstallerCLITests/PackageTrackingCatalog.cpp
index 520304d779..4dc4ae4ac2 100644
--- a/src/AppInstallerCLITests/PackageTrackingCatalog.cpp
+++ b/src/AppInstallerCLITests/PackageTrackingCatalog.cpp
@@ -191,3 +191,45 @@ TEST_CASE("TrackingCatalog_Uninstall", "[tracking_catalog]")
     SearchResult resultAfter = catalog.Search(request);
     REQUIRE(resultAfter.Matches.size() == 0);
 }
+
+TEST_CASE("TrackingCatalog_Overlapping_ARP_Range", "[tracking_catalog]")
+{
+    TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
+    INFO("Using temporary file named: " << tempFile.GetPath());
+
+    SourceDetails details;
+    Manifest manifest;
+    std::string relativePath;
+    auto source = SimpleTestSetup(tempFile, details, manifest, relativePath);
+
+    REQUIRE(manifest.Installers.size() >= 2);
+    AppsAndFeaturesEntry appEntry{};
+    appEntry.DisplayVersion = "1.23";
+    manifest.Installers[0].AppsAndFeaturesEntries.emplace_back(appEntry);
+    appEntry.DisplayVersion = "1.24";
+    manifest.Installers[1].AppsAndFeaturesEntries.emplace_back(appEntry);
+
+    PackageTrackingCatalog catalog = CreatePackageTrackingCatalogForSource(source);
+
+    SearchRequest request;
+    request.Filters.emplace_back(PackageMatchField::Id, MatchType::Exact, manifest.Id);
+
+    catalog.RecordInstall(manifest, manifest.Installers[0], false);
+
+    SearchResult resultBefore = catalog.Search(request);
+    REQUIRE(resultBefore.Matches.size() == 1);
+    REQUIRE(resultBefore.Matches[0].Package->GetAvailable().size() == 1);
+    REQUIRE(resultBefore.Matches[0].Package->GetAvailable()[0]->GetLatestVersion()->GetProperty(PackageVersionProperty::Version) ==
+        manifest.Version);
+
+    // Change version
+    manifest.Version = "99.1.2.3";
+
+    catalog.RecordInstall(manifest, manifest.Installers[0], true);
+
+    SearchResult resultAfter = catalog.Search(request);
+    REQUIRE(resultAfter.Matches.size() == 1);
+    REQUIRE(resultAfter.Matches[0].Package->GetAvailable().size() == 1);
+    REQUIRE(resultAfter.Matches[0].Package->GetAvailable()[0]->GetLatestVersion()->GetProperty(PackageVersionProperty::Version) ==
+        manifest.Version);
+}
diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp
index 8c7cf0f7b2..2fcbee0484 100644
--- a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp
+++ b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp
@@ -214,7 +214,7 @@ namespace AppInstaller::Repository
     }
 
     PackageTrackingCatalog::Version PackageTrackingCatalog::RecordInstall(
-        const Manifest::Manifest& manifest,
+        Manifest::Manifest& manifest,
         const Manifest::ManifestInstaller& installer,
         bool isUpgrade)
     {
@@ -223,6 +223,15 @@ namespace AppInstaller::Repository
 
         auto& index = m_implementation->Source->GetIndex();
 
+        // Strip ARP version information from the manifest if it is present
+        for (auto& arpRangeRemovedInstaller : manifest.Installers)
+        {
+            for (auto& arpRangeRemovedEntry : arpRangeRemovedInstaller.AppsAndFeaturesEntries)
+            {
+                arpRangeRemovedEntry.DisplayVersion.clear();
+            }
+        }
+
         // Check for an existing manifest that matches this one (could be reinstalling)
         auto manifestIdOpt = index.GetManifestIdByManifest(manifest);
 
diff --git a/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h b/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h
index 6b049f1cf7..f8963d37f8 100644
--- a/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h
+++ b/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h
@@ -59,7 +59,7 @@ namespace AppInstaller::Repository
         };
 
         // Records an installation of the given package.
-        Version RecordInstall(const Manifest::Manifest& manifest, const Manifest::ManifestInstaller& installer, bool isUpgrade);
+        Version RecordInstall(Manifest::Manifest& manifest, const Manifest::ManifestInstaller& installer, bool isUpgrade);
 
         // Records an uninstall of the given package.
         void RecordUninstall(const Utility::LocIndString& packageIdentifier);