Skip to content

Commit

Permalink
Change meaning of AddOrUpdate return bool (microsoft#4885)
Browse files Browse the repository at this point in the history
## Issue
The requestor of this new (1.10) API needs to know whether the operation
was an add or an update. At the same time, the "was the index modified"
return value from Update has always been true for any changes since the
manifest hash was added.

## Change
Return `true` when adding and `false` when updating.
  • Loading branch information
JohnMcPMS authored Oct 16, 2024
1 parent fa62b8b commit 9bfeedb
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 14 deletions.
8 changes: 4 additions & 4 deletions src/AppInstallerCLITests/SQLiteIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3918,20 +3918,20 @@ TEST_CASE("SQLiteIndex_AddOrUpdateManifest", "[sqliteindex]")
{
SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteStorageBase::OpenDisposition::ReadWrite);

// Update with no updates should return false
// Update should return false
REQUIRE(!index.AddOrUpdateManifest(manifest, manifestPath));

manifest.DefaultLocalization.Add<Localization::Description>("description2");

// Update with no indexed updates should return false
// Update should return false
REQUIRE(!index.AddOrUpdateManifest(manifest, manifestPath));

// Update with indexed changes
// Update with indexed changes should still return false
manifest.DefaultLocalization.Add<Localization::PackageName>("Test Name2");
manifest.Moniker = "testmoniker2";
manifest.DefaultLocalization.Add<Localization::Tags>({ "t1", "t2", "t3" });
manifest.Installers[0].Commands = {};

REQUIRE(index.AddOrUpdateManifest(manifest, manifestPath));
REQUIRE(!index.AddOrUpdateManifest(manifest, manifestPath));
}
}
3 changes: 2 additions & 1 deletion src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@ namespace AppInstaller::Repository::Microsoft

if (m_interface->GetManifestIdByManifest(m_dbconn, manifest))
{
return UpdateManifestInternalHoldingLock(manifest, relativePath);
UpdateManifestInternalHoldingLock(manifest, relativePath);
return false;
}
else
{
Expand Down
6 changes: 3 additions & 3 deletions src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,15 @@ namespace AppInstaller::Repository::Microsoft
bool UpdateManifest(const Manifest::Manifest& manifest);

// Adds or updates the manifest with matching { Id, Version, Channel } in the index.
// The return value indicates whether the index was modified by the function.
// The return value indicates whether the manifest was added (true) or updated (false).
bool AddOrUpdateManifest(const std::filesystem::path& manifestPath, const std::filesystem::path& relativePath);

// Updates the manifest with matching { Id, Version, Channel } in the index.
// The return value indicates whether the index was modified by the function.
// The return value indicates whether the manifest was added (true) or updated (false).
bool AddOrUpdateManifest(const Manifest::Manifest& manifest, const std::filesystem::path& relativePath);

// Updates the manifest with matching { Id, Version, Channel } in the index.
// The return value indicates whether the index was modified by the function.
// The return value indicates whether the manifest was added (true) or updated (false).
bool AddOrUpdateManifest(const Manifest::Manifest& manifest);

// Removes the manifest with matching { Id, Version, Channel } from the index.
Expand Down
2 changes: 1 addition & 1 deletion src/WinGetUtil/WinGetUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ extern "C"
BOOL* indexModified);

// Adds or Updates the manifest with matching { Id, Version, Channel } in the index.
// The return value indicates whether the index was modified by the function.
// The return value indicates whether the manifest was added (true) or updated (false).
WINGET_UTIL_API WinGetSQLiteIndexAddOrUpdateManifest(
WINGET_SQLITE_INDEX_HANDLE index,
WINGET_STRING manifestPath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,13 @@ public void AddOrUpdateManifest()
{
this.CreateIndexHelperForIndexTest((wrapper) =>
{
// Add manifest.
// Add manifest; should return true.
string addManifest = Path.Combine(this.indexTestDataPath, PackageTest);
wrapper.AddOrUpdateManifest(addManifest, PackageTestRelativePath);
Assert.True(wrapper.AddOrUpdateManifest(addManifest, PackageTestRelativePath));

// Update manifest. name is different, should return true.
// Update manifest. name is different, should return false.
string updateManifest = Path.Combine(this.indexTestDataPath, PackageTestNewName);
Assert.True(wrapper.AddOrUpdateManifest(updateManifest, PackageTestRelativePath));
Assert.False(wrapper.AddOrUpdateManifest(updateManifest, PackageTestRelativePath));

return true;
});
Expand Down
2 changes: 1 addition & 1 deletion src/WinGetUtilInterop/Interfaces/IWinGetSQLiteIndex.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public interface IWinGetSQLiteIndex : IDisposable
/// </summary>
/// <param name="manifestPath">Path to manifest.</param>
/// <param name="relativePath">Path of the manifest in the repository.</param>
/// <returns>True if index was modified.</returns>
/// <returns>True if added; false if updated.</returns>
bool AddOrUpdateManifest(string manifestPath, string relativePath);

/// <summary>
Expand Down

0 comments on commit 9bfeedb

Please sign in to comment.