winget-cli

commit ee19d90cedc7c9b6a793d5b64dfd87dfdaf15768
parent 841496f9cd710ec09eb9a046a7565545652af759
Author: JohnMcPMS <johnmcp@microsoft.com>
Date:   Wed, 11 Mar 2020 19:04:34 -0700

Fix a bug in AddManifest (#51)


Diffstat:
M
src/AppInstallerCLITests/SQLiteIndex.cpp
 | 
19
+++++++++++++++++--
M
src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.cpp
 | 
10
+++++++---

2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/src/AppInstallerCLITests/SQLiteIndex.cpp b/src/AppInstallerCLITests/SQLiteIndex.cpp
@@ -141,9 +141,24 @@ TEST_CASE("SQLiteIndexCreateAndAddManifestFile", "[sqliteindex]")
     std::filesystem::path manifestPath{ "microsoft/msixsdk/microsoft.msixsdk-1.7.32.yml" };
 
     index.AddManifest(manifestFile, manifestPath);
+}
+
+TEST_CASE("SQLiteIndexCreateAndAddManifestDuplicate", "[sqliteindex]")
+{
+    TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
+    INFO("Using temporary file named: " << tempFile.GetPath());
+
+    Manifest manifest;
+    std::string relativePath;
+
+    SQLiteIndex index = SimpleTestSetup(tempFile, manifest, relativePath);
+
+    // Attempting to add the same manifest at a different path should fail.
+    REQUIRE_THROWS_HR(index.AddManifest(manifest, "differentpath.yml"), HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS));
 
-    // Attempting to add again should fail
-    REQUIRE_THROWS_HR(index.AddManifest(manifestFile, manifestPath), HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS));
+    // Attempting to add a different manifest at the same path should fail.
+    manifest.Id += "-new";
+    REQUIRE_THROWS_HR(index.AddManifest(manifest, relativePath), HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS));
 }
 
 TEST_CASE("SQLiteIndex_RemoveManifestFile_NotPresent", "[sqliteindex]")
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.cpp
@@ -23,8 +23,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
 {
     namespace
     {
-        // Gets an existing manifest by its rowid.
-        // The return value contains the path leaf and manifest rowid, if they exist.
+        // Gets an existing manifest by its rowid., if it exists.
         std::optional<SQLite::rowid_t> GetExistingManifestId(SQLite::Connection& connection, const Manifest::Manifest& manifest)
         {
             std::optional<SQLite::rowid_t> idId = IdTable::SelectIdByValue(connection, manifest.Id);
@@ -184,11 +183,16 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
 
     void Interface::AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath)
     {
+        auto manifestResult = GetExistingManifestId(connection, manifest);
+
+        // If this manifest is already present, we can't add it.
+        THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), manifestResult.has_value());
+
         SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "addmanifest_v1_0");
 
         auto [pathAdded, pathLeafId] = PathPartTable::EnsurePathExists(connection, relativePath, true);
 
-        // If we get false from the function, this manifest already exists in the index.
+        // If we get false from the function, this manifest path already exists in the index.
         THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), !pathAdded);
 
         // Ensure that all of the 1:1 data exists.