Bug fix: PackageTrackingCatalog and SQLiteIndexUpdate (#1780)

* Bug fix: PackageTrackingCatalog and SQLiteIndexUpdate

* Addressed PR comments

Co-authored-by: Akinwale Alagbe <[email protected]>

src/AppInstallerCLITests/PackageTrackingCatalog.cpp

@@ -18,7 +18,7 @@ namespace
 {
     static Source SimpleTestSetup(const std::string& filePath, SourceDetails& details, Manifest& manifest, std::string& relativePath)
     {
-        SQLiteIndex index = SQLiteIndex::CreateNew(filePath, Schema::Version::Latest());
+        SQLiteIndex index = SQLiteIndex::CreateNew(filePath, Schema::Version::Latest(), SQLiteIndex::CreateOptions::SupportPathless | SQLiteIndex::CreateOptions::DisableDependenciesSupport);
 
         TestDataFile testManifest("Manifest-Good.yaml");
         manifest = YamlParser::CreateFromPath(testManifest);

src/AppInstallerCLITests/SQLiteIndex.cpp

@@ -509,7 +509,7 @@ TEST_CASE("SQLiteIndex_AddManifestWithDependencies_EmptyManifestVersion", "[sqli
     index.AddManifest(manifest, GetPathFromManifest(manifest));
 }
 
-TEST_CASE("SQLiteIndex_DependenciesTable_CheckConsistency", "[sqliteindex][V1_1]")
+TEST_CASE("SQLiteIndex_DependenciesTable_CheckConsistency", "[sqliteindex][V1_4]")
 {
     TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
     INFO("Using temporary file named: " << tempFile.GetPath());
@@ -965,7 +965,7 @@ TEST_CASE("SQLiteIndex_RemoveManifestFile", "[sqliteindex][V1_0]")
     REQUIRE(Schema::V1_0::CommandsTable::IsEmpty(connection));
 }
 
-TEST_CASE("SQLiteIndex_UpdateManifest", "[sqliteindex][V1_0]")
+TEST_CASE("SQLiteIndex_UpdateManifest", "[sqliteindex][V1_4]")
 {
     TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
     INFO("Using temporary file named: " << tempFile.GetPath());
@@ -981,8 +981,10 @@ TEST_CASE("SQLiteIndex_UpdateManifest", "[sqliteindex][V1_0]")
     manifest.DefaultLocalization.Add<Localization::Tags>({ "t1", "t2" });
     manifest.Installers[0].Commands = { "test1", "test2" };
 
+
     {
-        SQLiteIndex index = SQLiteIndex::CreateNew(tempFile, { 1, 0 });
+        auto version = GENERATE(Schema::Version{ 1, 0 }, Schema::Version::Latest());
+        SQLiteIndex index = SQLiteIndex::CreateNew(tempFile, version);
 
         index.AddManifest(manifest, manifestPath);
     }

src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/DependenciesTable.cpp

@@ -93,12 +93,13 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
             return dependencies;
         }
 
-        void RemoveDependenciesByRowIds(SQLite::Connection& connection, std::vector<DependencyTableRow> dependencyTableRows)
+        bool RemoveDependenciesByRowIds(SQLite::Connection& connection, std::vector<DependencyTableRow> dependencyTableRows)
         {
             using namespace SQLite::Builder;
+            bool tableUpdated = false;
             if (dependencyTableRows.empty())
             {
-                return;
+                return tableUpdated;
             }
             SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, std::string{ s_DependenciesTable_Table_Name } + "remove_dependencies_by_rowid");
 
@@ -108,31 +109,36 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
                 .Where(s_DependenciesTable_PackageId_Column_Name).Equals(Unbound)
                 .And(s_DependenciesTable_Manifest_Column_Name).Equals(Unbound);
 
+
             SQLite::Statement deleteStmt = builder.Prepare(connection);
             for (auto row : dependencyTableRows)
             {
                 deleteStmt.Reset();
                 deleteStmt.Bind(1, row.m_packageRowId);
                 deleteStmt.Bind(2, row.m_manifestRowId);
                 deleteStmt.Execute(true);
+                tableUpdated = true;
             }
 
             savepoint.Commit();
+            return tableUpdated;
         }
 
-        void InsertManifestDependencies(
+        bool InsertManifestDependencies(
             SQLite::Connection& connection,
             std::set<DependencyTableRow>& dependenciesTableRows)
         {
             using namespace SQLite::Builder;
             using namespace Schema::V1_0;
+            bool tableUpdated = false;
 
             StatementBuilder insertBuilder;
             insertBuilder.InsertInto(s_DependenciesTable_Table_Name)
                 .Columns({ s_DependenciesTable_Manifest_Column_Name, s_DependenciesTable_MinVersion_Column_Name, s_DependenciesTable_PackageId_Column_Name })
                 .Values(Unbound, Unbound, Unbound);
             SQLite::Statement insert = insertBuilder.Prepare(connection);
 
+
             for (const auto& dep : dependenciesTableRows)
             {
                 insert.Reset();
@@ -150,7 +156,10 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
                 insert.Bind(3, dep.m_packageRowId);
 
                 insert.Execute(true);
+                tableUpdated = true;
             }
+
+            return tableUpdated;
         }
     }
 
@@ -230,6 +239,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
 
     void DependenciesTable::AddDependencies(SQLite::Connection& connection, const Manifest::Manifest& manifest, SQLite::rowid_t manifestRowId)
     {
+        if (!Exists(connection))
+        {
+            return;
+        }
+
         SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, std::string{ s_DependenciesTable_Table_Name } + "add_dependencies_v1_4");
 
         auto dependencies = GetAndLinkDependencies(connection, manifest, manifestRowId, Manifest::DependencyType::Package);
@@ -245,6 +259,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
 
     bool DependenciesTable::UpdateDependencies(SQLite::Connection& connection, const Manifest::Manifest& manifest, SQLite::rowid_t manifestRowId)
     {
+        if (!Exists(connection))
+        {
+            return false;
+        }
+
         SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, std::string{ s_DependenciesTable_Table_Name } + "update_dependencies_v1_4");
 
         const auto dependencies = GetAndLinkDependencies(connection, manifest, manifestRowId, Manifest::DependencyType::Package);
@@ -277,16 +296,22 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
             }
         );
 
-        InsertManifestDependencies(connection, toAddDependencies);
-        RemoveDependenciesByRowIds(connection, toRemoveDependencies);
+        bool tableUpdated = InsertManifestDependencies(connection, toAddDependencies);
+        tableUpdated = RemoveDependenciesByRowIds(connection, toRemoveDependencies) || tableUpdated;
         savepoint.Commit();
 
-        return true;
+        return tableUpdated;
     }
 
     void DependenciesTable::RemoveDependencies(SQLite::Connection& connection, SQLite::rowid_t manifestRowId)
     {
+        if (!Exists(connection))
+        {
+            return;
+        }
+
         SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, std::string{ s_DependenciesTable_Table_Name } + "remove_dependencies_by_manifest_v1_4");
+
         SQLite::Builder::StatementBuilder builder;
         builder.DeleteFrom(s_DependenciesTable_Table_Name).Where(s_DependenciesTable_Manifest_Column_Name).Equals(manifestRowId);
 
@@ -425,6 +450,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
 
     bool DependenciesTable::IsValueReferenced(const SQLite::Connection& connection, std::string_view tableName, SQLite::rowid_t valueRowId)
     {
+        if (!Exists(connection))
+        {
+            return false;
+        }
+
         StatementBuilder builder;
 
         if (tableName != V1_0::VersionTable::TableName())
@@ -454,6 +484,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
 
     std::vector<SQLite::rowid_t> DependenciesTable::GetDependenciesMinVersionsRowIdByManifestId(const SQLite::Connection& connection, SQLite::rowid_t manifestRowId)
     {
+        if (!Exists(connection))
+        {
+            return {};
+        }
+
         StatementBuilder builder;
 
         std::vector<SQLite::rowid_t> result;

src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/Interface_1_4.cpp

@@ -24,7 +24,10 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
 
         V1_3::Interface::CreateTables(connection, options);
 
-        DependenciesTable::Create(connection);
+        if (WI_IsFlagClear(options, CreateOptions::DisableDependenciesSupport))
+        {
+            DependenciesTable::Create(connection);
+        }
 
         savepoint.Commit();
     }

src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h

@@ -42,9 +42,11 @@ namespace AppInstaller::Repository::Microsoft::Schema
         enum class CreateOptions
         {
             // Standard
-            None,
+            None = 0x0,
             // Enable support for passing in nullopt values to Add/UpdateManifest
-            SupportPathless,
+            SupportPathless = 0x1,
+            // Disable support for dependencies
+            DisableDependenciesSupport = 0x2,
         };
 
         // Creates all of the version dependent tables within the database.

src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp

@@ -58,7 +58,7 @@ namespace AppInstaller::Repository
                     if (!std::filesystem::exists(trackingDB))
                     {
                         std::filesystem::create_directories(trackingDB.parent_path());
-                        SQLiteIndex::CreateNew(trackingDB.u8string(), Schema::Version::Latest(), SQLiteIndex::CreateOptions::SupportPathless);
+                        SQLiteIndex::CreateNew(trackingDB.u8string(), Schema::Version::Latest(), SQLiteIndex::CreateOptions::SupportPathless | SQLiteIndex::CreateOptions::DisableDependenciesSupport);
                     }
                 }