Skip to content

Commit

Permalink
Don't put ARP version data into tracking catalog (#4964)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
JohnMcPMS authored Nov 13, 2024
1 parent 1105343 commit ff86443
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 2 deletions.
42 changes: 42 additions & 0 deletions src/AppInstallerCLITests/PackageTrackingCatalog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
11 changes: 10 additions & 1 deletion src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ namespace AppInstaller::Repository
}

PackageTrackingCatalog::Version PackageTrackingCatalog::RecordInstall(
const Manifest::Manifest& manifest,
Manifest::Manifest& manifest,
const Manifest::ManifestInstaller& installer,
bool isUpgrade)
{
Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit ff86443

Please sign in to comment.