commit 676e60cd38e61fb04f9f7d1b8af1b714319f7888 parent 264eaf340bb4dc922e2a6dbd0ae55cf31a3c08f5 Author: JohnMcPMS <johnmcp@microsoft.com> Date: Mon, 10 Feb 2020 12:27:13 -0800 Implement update functionality in SQLiteIndex (#32) Diffstat:
24 files changed, 546 insertions(+), 90 deletions(-)
diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp @@ -27,7 +27,9 @@ protected: std::future<void> ExecuteInstallerAsync(const Uri& uri) override { - std::ofstream file("TestMsixInstalled.txt", std::ofstream::out); + std::filesystem::path temp = std::filesystem::temp_directory_path(); + temp /= "TestMsixInstalled.txt"; + std::ofstream file(temp, std::ofstream::out); file << AppInstaller::Utility::ConvertToUTF8(uri.ToString()); diff --git a/src/AppInstallerCLITests/SQLiteIndex.cpp b/src/AppInstallerCLITests/SQLiteIndex.cpp @@ -222,6 +222,109 @@ TEST_CASE("SQLiteIndex_RemoveManifestFile", "[sqliteindex]") REQUIRE(Schema::V1_0::ExtensionsTable::IsEmpty(connection)); } +TEST_CASE("SQLiteIndex_UpdateManifest", "[sqliteindex]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + std::string manifestPath = "test/id/test.id-1.0.0.yml"; + Manifest manifest; + manifest.Id = "test.id"; + manifest.Name = "Test Name"; + manifest.AppMoniker = "testmoniker"; + manifest.Version = "1.0.0"; + manifest.Channel = "test"; + manifest.Tags = { "t1", "t2" }; + manifest.Commands = { "test1", "test2" }; + manifest.Protocols = { "htttest" }; + manifest.FileExtensions = { "tst", "test", "testy" }; + + { + SQLiteIndex index = SQLiteIndex::CreateNew(tempFile, { 1, 0 }); + + index.AddManifest(manifest, manifestPath); + } + + { + // Open it directly to directly test table state + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); + + REQUIRE(!Schema::V1_0::ManifestTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::IdTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::NameTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::MonikerTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::VersionTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::ChannelTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::PathPartTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::TagsTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::CommandsTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::ProtocolsTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::ExtensionsTable::IsEmpty(connection)); + } + + { + SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite); + + // Update with no updates should return false + REQUIRE(!index.UpdateManifest(manifest, manifestPath)); + + manifest.Description = "description2"; + + // Update with no indexed updates should return false + REQUIRE(!index.UpdateManifest(manifest, manifestPath)); + + // Update with indexed changes + manifest.Name = "Test Name2"; + manifest.AppMoniker = "testmoniker2"; + manifest.Tags = { "t1", "t2", "t3" }; + manifest.Commands = { "test1", "test3" }; + manifest.Protocols = {}; + manifest.FileExtensions = { "tst", "test", "testy" }; + + REQUIRE(index.UpdateManifest(manifest, manifestPath)); + } + + { + // Open it directly to directly test table state + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); + + REQUIRE(!Schema::V1_0::ManifestTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::IdTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::NameTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::MonikerTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::VersionTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::ChannelTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::PathPartTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::TagsTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::CommandsTable::IsEmpty(connection)); + // The update removed all protocols + REQUIRE(Schema::V1_0::ProtocolsTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::ExtensionsTable::IsEmpty(connection)); + } + + { + SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite); + + // Now remove manifest2 + index.RemoveManifest(manifest, manifestPath); + } + + // Open it directly to directly test table state + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); + + REQUIRE(Schema::V1_0::ManifestTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::IdTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::NameTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::MonikerTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::VersionTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::ChannelTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::PathPartTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::TagsTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::CommandsTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::ProtocolsTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::ExtensionsTable::IsEmpty(connection)); +} + TEST_CASE("PathPartTable_EnsurePathExists_Negative_Paths", "[sqliteindex][V1_0]") { // Open it directly to directly test pathpart table diff --git a/src/AppInstallerCLITests/SQLiteWrapper.cpp b/src/AppInstallerCLITests/SQLiteWrapper.cpp @@ -11,6 +11,7 @@ using namespace std::string_literals; static const char* s_firstColumn = "first"; static const char* s_secondColumn = "second"; static const char* s_tableName = "simpletest"; +static const char* s_savepoint = "simplesave"; static const char* s_CreateSimpleTestTableSQL = R"( CREATE TABLE [main].[simpletest]( @@ -49,6 +50,13 @@ void InsertIntoSimpleTestTable(Connection& connection, int firstVal, const std:: REQUIRE(insert.GetState() == Statement::State::Completed); } +void UpdateSimpleTestTable(Connection& connection, int firstVal, const std::string& secondVal) +{ + Builder::StatementBuilder update; + update.Update(s_tableName).Set().Column(s_firstColumn).Equals(firstVal).Column(s_secondColumn).Equals(secondVal); + update.Execute(connection); +} + void InsertIntoSimpleTestTableWithNull(Connection& connection, int firstVal) { Builder::StatementBuilder builder; @@ -188,6 +196,70 @@ TEST_CASE("SQLiteWrapperSavepointCommit", "[sqlitewrapper]") SelectFromSimpleTestTableOnlyOneRow(connection, firstVal, secondVal); } +TEST_CASE("SQLiteWrapperSavepointReuse", "[sqlitewrapper]") +{ + TestCommon::TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + int firstVal = 1; + std::string secondVal = "test"; + + // Create the DB and some data + { + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::Create); + + CreateSimpleTestTable(connection); + + InsertIntoSimpleTestTable(connection, firstVal, secondVal); + } + + // Reopen the DB and update with a single savepoint + { + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); + + Savepoint savepoint = Savepoint::Create(connection, s_savepoint); + + firstVal = 2; + secondVal = "test2"; + UpdateSimpleTestTable(connection, firstVal, secondVal); + + savepoint.Commit(); + } + + { + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); + SelectFromSimpleTestTableOnlyOneRow(connection, firstVal, secondVal); + } + + // Reopen the DB and update with a multiple savepoint + { + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); + + { + Savepoint savepoint = Savepoint::Create(connection, s_savepoint); + + firstVal = 3; + secondVal = "test3"; + UpdateSimpleTestTable(connection, firstVal, secondVal); + } + + { + Savepoint savepoint = Savepoint::Create(connection, s_savepoint); + + firstVal = 4; + secondVal = "test4"; + UpdateSimpleTestTable(connection, firstVal, secondVal); + + savepoint.Commit(); + } + } + + { + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); + SelectFromSimpleTestTableOnlyOneRow(connection, firstVal, secondVal); + } +} + TEST_CASE("SQLBuilder_SimpleSelectBind", "[sqlbuilder]") { Connection connection = Connection::Create(SQLITE_MEMORY_DB_CONNECTION_TARGET, Connection::OpenDisposition::Create); @@ -306,6 +378,27 @@ TEST_CASE("SQLBuilder_SimpleSelectOptional", "[sqlbuilder]") } } +TEST_CASE("SQLBuilder_Update", "[sqlbuilder]") +{ + Connection connection = Connection::Create(SQLITE_MEMORY_DB_CONNECTION_TARGET, Connection::OpenDisposition::Create); + + CreateSimpleTestTable(connection); + + int firstVal = 1; + std::string secondVal = "test"; + + InsertIntoSimpleTestTable(connection, firstVal, secondVal); + + SelectFromSimpleTestTableOnlyOneRow(connection, firstVal, secondVal); + + firstVal = 2; + secondVal = "testing"; + + UpdateSimpleTestTable(connection, firstVal, secondVal); + + SelectFromSimpleTestTableOnlyOneRow(connection, firstVal, secondVal); +} + TEST_CASE("SQLBuilder_CreateTable", "[sqlbuilder]") { Connection connection = Connection::Create(SQLITE_MEMORY_DB_CONNECTION_TARGET, Connection::OpenDisposition::Create); diff --git a/src/AppInstallerCLITests/TestCommon.cpp b/src/AppInstallerCLITests/TestCommon.cpp @@ -28,7 +28,8 @@ namespace TestCommon return tempFilePath; } - static bool s_TempFileDestructorKeepsFile{}; + static TempFileDestructionBehavior s_TempFileDestructorBehavior = TempFileDestructionBehavior::Delete; + static std::vector<std::filesystem::path> s_TempFilesOnFile; static std::filesystem::path s_TestDataFileBasePath{}; } @@ -44,7 +45,15 @@ namespace TestCommon TempFile::TempFile(const std::filesystem::path& filePath, bool deleteFileOnConstruction) { - _filepath = filePath; + if (filePath.is_relative()) + { + _filepath = std::filesystem::temp_directory_path(); + _filepath /= filePath; + } + else + { + _filepath = filePath; + } if (deleteFileOnConstruction) { std::filesystem::remove(_filepath); @@ -53,15 +62,42 @@ namespace TestCommon TempFile::~TempFile() { - if (!s_TempFileDestructorKeepsFile) + switch (s_TempFileDestructorBehavior) { + case TempFileDestructionBehavior::Delete: std::filesystem::remove(_filepath); + break; + case TempFileDestructionBehavior::Keep: + break; + case TempFileDestructionBehavior::ShellExecuteOnFailure: + s_TempFilesOnFile.emplace_back(std::move(_filepath)); + break; } } - void TempFile::SetDestructorBehavior(bool keepFilesOnDestruction) + void TempFile::SetDestructorBehavior(TempFileDestructionBehavior behavior) + { + s_TempFileDestructorBehavior = behavior; + } + + void TempFile::SetTestFailed(bool failed) { - s_TempFileDestructorKeepsFile = keepFilesOnDestruction; + if (failed) + { + for (const auto& path : s_TempFilesOnFile) + { + SHELLEXECUTEINFOW seinfo{}; + seinfo.cbSize = sizeof(seinfo); + seinfo.lpVerb = L"open"; + seinfo.lpFile = path.c_str(); + + ShellExecuteExW(&seinfo); + } + } + else + { + s_TempFilesOnFile.clear(); + } } std::filesystem::path TestDataFile::GetPath() const diff --git a/src/AppInstallerCLITests/TestCommon.h b/src/AppInstallerCLITests/TestCommon.h @@ -9,6 +9,13 @@ namespace TestCommon { + enum class TempFileDestructionBehavior + { + Delete, + Keep, + ShellExecuteOnFailure, + }; + // Use this to create a temporary file for testing. struct TempFile { @@ -26,7 +33,9 @@ namespace TestCommon const std::filesystem::path& GetPath() const { return _filepath; } operator const std::string () const { return _filepath.u8string(); } - static void SetDestructorBehavior(bool keepFilesOnDestruction); + static void SetDestructorBehavior(TempFileDestructionBehavior behavior); + + static void SetTestFailed(bool failed); private: std::filesystem::path _filepath; diff --git a/src/AppInstallerCLITests/main.cpp b/src/AppInstallerCLITests/main.cpp @@ -22,8 +22,23 @@ struct LoggingBreakListener : public Catch::TestEventListenerBase void testCaseStarting(const Catch::TestCaseInfo& info) override { + Catch::TestEventListenerBase::testCaseStarting(info); AICLI_LOG(Test, Info, << "========== Test Case Begins :: " << info.name << " =========="); + TestCommon::TempFile::SetTestFailed(false); } + + void testCaseEnded(const Catch::TestCaseStats& testCaseStats) override + { + AICLI_LOG(Test, Info, << "========== Test Case Ends :: " << currentTestCaseInfo->name << " =========="); + if (!testCaseStats.totals.delta(lastTotals).testCases.allOk()) + { + TestCommon::TempFile::SetTestFailed(true); + } + lastTotals = testCaseStats.totals; + Catch::TestEventListenerBase::testCaseEnded(testCaseStats); + } + + Catch::Totals lastTotals{}; }; CATCH_REGISTER_LISTENER(LoggingBreakListener); @@ -38,7 +53,11 @@ int main(int argc, char** argv) { if ("-ktf"s == argv[i]) { - TestCommon::TempFile::SetDestructorBehavior(true); + TestCommon::TempFile::SetDestructorBehavior(TestCommon::TempFileDestructionBehavior::Keep); + } + else if ("-seof"s == argv[i]) + { + TestCommon::TempFile::SetDestructorBehavior(TestCommon::TempFileDestructionBehavior::ShellExecuteOnFailure); } else if ("-log"s == argv[i]) { diff --git a/src/AppInstallerCLITests/pch.h b/src/AppInstallerCLITests/pch.h @@ -4,6 +4,7 @@ #define NOMINMAX #include <Windows.h> #include <WinInet.h> +#include <shellapi.h> #include <catch.hpp> diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp @@ -143,23 +143,21 @@ namespace AppInstaller::Repository::Microsoft savepoint.Commit(); } - bool SQLiteIndex::UpdateManifest(const std::filesystem::path& oldManifestPath, const std::filesystem::path& oldRelativePath, const std::filesystem::path& newManifestPath, const std::filesystem::path& newRelativePath) + bool SQLiteIndex::UpdateManifest(const std::filesystem::path& manifestPath, const std::filesystem::path& relativePath) { - AICLI_LOG(Repo, Info, << "Updating manifest from file [" << oldManifestPath << "] to file [" << newManifestPath << "]"); + AICLI_LOG(Repo, Info, << "Updating manifest from file [" << manifestPath << "]"); - Manifest::Manifest oldManifest = Manifest::Manifest::CreateFromPath(oldManifestPath); - Manifest::Manifest newManifest = Manifest::Manifest::CreateFromPath(newManifestPath); - return UpdateManifest(oldManifest, oldRelativePath, newManifest, newRelativePath); + Manifest::Manifest manifest = Manifest::Manifest::CreateFromPath(manifestPath); + return UpdateManifest(manifest, relativePath); } - bool SQLiteIndex::UpdateManifest(const Manifest::Manifest& oldManifest, const std::filesystem::path& oldRelativePath, const Manifest::Manifest& newManifest, const std::filesystem::path& newRelativePath) + bool SQLiteIndex::UpdateManifest(const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) { - AICLI_LOG(Repo, Info, << "Updating manifest from [" << oldManifest.Id << ", " << oldManifest.Version << "] to [" << newManifest.Id << ", " << newManifest.Version << - "] at relative path [" << oldRelativePath << "] to [" << newRelativePath << "]"); + AICLI_LOG(Repo, Info, << "Updating manifest for [" << manifest.Id << ", " << manifest.Version << "] at relative path [" << relativePath << "]"); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "sqliteindex_updatemanifest"); - bool result = m_interface->UpdateManifest(m_dbconn, oldManifest, oldRelativePath, newManifest, newRelativePath); + bool result = m_interface->UpdateManifest(m_dbconn, manifest, relativePath); if (result) { diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h @@ -55,11 +55,11 @@ namespace AppInstaller::Repository::Microsoft // Updates the manifest at the repository relative path in the index. // The return value indicates whether the index was modified by the function. - bool UpdateManifest(const std::filesystem::path& oldManifestPath, const std::filesystem::path& oldRelativePath, const std::filesystem::path& newManifestPath, const std::filesystem::path& newRelativePath); + bool UpdateManifest(const std::filesystem::path& manifestPath, const std::filesystem::path& relativePath); // Updates the manifest at the repository relative path in the index. // The return value indicates whether the index was modified by the function. - bool UpdateManifest(const Manifest::Manifest& oldManifest, const std::filesystem::path& oldRelativePath, const Manifest::Manifest& newManifest, const std::filesystem::path& newRelativePath); + bool UpdateManifest(const Manifest::Manifest& manifest, const std::filesystem::path& relativePath); // Removes the manifest at the repository relative path from the index. void RemoveManifest(const std::filesystem::path& manifestPath, const std::filesystem::path& relativePath); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.cpp @@ -77,6 +77,19 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return result; } + + // Updates the manifest column and related table based on the given value. + template <typename Table> + void UpdateManifestValueById(SQLite::Connection& connection, const typename Table::value_t& value, SQLite::rowid_t manifestId) + { + auto [oldValueId] = ManifestTable::GetIdsById<Table>(connection, manifestId); + + SQLite::rowid_t newValueId = Table::EnsureExists(connection, value); + + ManifestTable::UpdateValueIdById<Table>(connection, manifestId, newValueId); + + Table::DeleteIfNotNeededById(connection, oldValueId); + } } Schema::Version Interface::GetVersion() const @@ -148,22 +161,53 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 savepoint.Commit(); } - bool Interface::UpdateManifest(SQLite::Connection& connection, - const Manifest::Manifest& oldManifest, const std::filesystem::path& oldRelativePath, - const Manifest::Manifest& newManifest, const std::filesystem::path& newRelativePath) + bool Interface::UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) { - UNREFERENCED_PARAMETER(connection); - UNREFERENCED_PARAMETER(oldManifest); - UNREFERENCED_PARAMETER(oldRelativePath); - UNREFERENCED_PARAMETER(newManifest); - UNREFERENCED_PARAMETER(newRelativePath); - THROW_HR(E_NOTIMPL); + ExistingManifestInfo manifestInfo = GetExistingManifestId(connection, manifest, relativePath); + + // If the manifest doesn't actually exist, fail the update. + THROW_HR_IF(E_NOT_SET, !manifestInfo.Manifest); + + SQLite::rowid_t manifestId = manifestInfo.Manifest.value(); + + auto [idInIndex, nameInIndex, monikerInIndex, versionInIndex, channelInIndex] = + ManifestTable::GetValuesById<IdTable, NameTable, MonikerTable, VersionTable, ChannelTable>(connection, manifestId); + + // We know that the Id, Version, and Channel did not change based on GetExistingManifestId, + // but we still verify that here in the event that the code there changed. + THROW_HR_IF(E_UNEXPECTED, idInIndex != manifest.Id); + THROW_HR_IF(E_UNEXPECTED, versionInIndex != manifest.Version); + THROW_HR_IF(E_UNEXPECTED, channelInIndex != manifest.Channel); + + bool indexModified = false; + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "updatemanifest_v1_0"); + + // If these values changed, we need to update them. + if (nameInIndex != manifest.Name) + { + UpdateManifestValueById<NameTable>(connection, manifest.Name, manifestId); + indexModified = true; + } + + if (monikerInIndex != manifest.AppMoniker) + { + UpdateManifestValueById<MonikerTable>(connection, manifest.AppMoniker, manifestId); + indexModified = true; + } + + // Update all 1:N tables as necessary + indexModified = TagsTable::UpdateIfNeededByManifestId(connection, manifest.Tags, manifestId) || indexModified; + indexModified = CommandsTable::UpdateIfNeededByManifestId(connection, manifest.Commands, manifestId) || indexModified; + indexModified = ProtocolsTable::UpdateIfNeededByManifestId(connection, manifest.Protocols, manifestId) || indexModified; + indexModified = ExtensionsTable::UpdateIfNeededByManifestId(connection, manifest.FileExtensions, manifestId) || indexModified; + + savepoint.Commit(); + + return indexModified; } void Interface::RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removemanifest_v1_0"); - ExistingManifestInfo manifestInfo = GetExistingManifestId(connection, manifest, relativePath); // If the manifest doesn't actually exist, fail the remove. @@ -175,6 +219,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 auto [idId, nameId, monikerId, versionId, channelId] = ManifestTable::GetIdsById<IdTable, NameTable, MonikerTable, VersionTable, ChannelTable>(connection, manifestId); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removemanifest_v1_0"); + // Remove the manifest row ManifestTable::DeleteById(connection, manifestId); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h @@ -13,9 +13,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 Schema::Version GetVersion() const override; void CreateTables(SQLite::Connection& connection) override; void AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; - bool UpdateManifest(SQLite::Connection& connection, - const Manifest::Manifest& oldManifest, const std::filesystem::path& oldRelativePath, - const Manifest::Manifest& newManifest, const std::filesystem::path& newRelativePath) override; + bool UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; void RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.cpp @@ -75,6 +75,14 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return result; } + + void ManifestTableUpdateValueIdById(SQLite::Connection& connection, std::string_view valueName, SQLite::rowid_t value, SQLite::rowid_t id) + { + SQLite::Builder::StatementBuilder builder; + builder.Update(s_ManifestTable_Table_Name).Set().Column(valueName).Equals(value).Where(SQLite::RowIDName).Equals(id); + + builder.Execute(connection); + } } void ManifestTable::Create(SQLite::Connection& connection, std::initializer_list<ManifestColumnInfo> values) diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.h @@ -26,6 +26,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 SQLite::Connection& connection, SQLite::rowid_t id, std::initializer_list<SQLite::Builder::QualifiedColumn> columns); + + // Update the value of a single column for the manifest with the given rowid. + void ManifestTableUpdateValueIdById(SQLite::Connection& connection, std::string_view valueName, SQLite::rowid_t value, SQLite::rowid_t id); } // Info on the manifest columns. @@ -73,6 +76,13 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return details::ManifestTableGetValuesById_Statement(connection, id, { SQLite::Builder::QualifiedColumn{ Tables::TableName(), Tables::ValueName() }... }).GetRow<Tables::value_t...>(); } + // Update the value of a single column for the manifest with the given rowid. + template <typename Table> + static void UpdateValueIdById(SQLite::Connection& connection, SQLite::rowid_t id, SQLite::rowid_t value) + { + details::ManifestTableUpdateValueIdById(connection, Table::ValueName(), value, id); + } + // Deletes the manifest row with the given rowid. static void DeleteById(SQLite::Connection& connection, SQLite::rowid_t id); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/OneToManyTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/OneToManyTable.cpp @@ -14,11 +14,80 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 static constexpr std::string_view s_OneToManyTable_MapTable_ManifestName = "manifest"sv; static constexpr std::string_view s_OneToManyTable_MapTable_Suffix = "_map"sv; + namespace + { + // Create the mapping table insert statement for multiple use. + // Bind the rowid of the value to 2. + SQLite::Statement CreateMappingInsertStatementForManifestId(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, SQLite::rowid_t manifestId) + { + SQLite::Builder::StatementBuilder insertMappingBuilder; + insertMappingBuilder.InsertInto({ tableName, s_OneToManyTable_MapTable_Suffix }). + Columns({ s_OneToManyTable_MapTable_ManifestName, valueName }).Values(manifestId, SQLite::Builder::Unbound); + + return insertMappingBuilder.Prepare(connection); + } + + // Get a collection of the value ids associated with the given manifest id. + std::vector<SQLite::rowid_t> GetValueIdsByManifestId(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, SQLite::rowid_t manifestId) + { + std::vector<SQLite::rowid_t> result; + + SQLite::Builder::StatementBuilder selectMappingBuilder; + selectMappingBuilder.Select(valueName).From({ tableName, s_OneToManyTable_MapTable_Suffix }).Where(s_OneToManyTable_MapTable_ManifestName).Equals(manifestId); + + SQLite::Statement selectMappingStatement = selectMappingBuilder.Prepare(connection); + + while (selectMappingStatement.Step()) + { + result.push_back(selectMappingStatement.GetColumn<SQLite::rowid_t>(0)); + } + + return result; + } + + struct DeleteValueIfNotNeededStatements + { + DeleteValueIfNotNeededStatements(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName) + { + SQLite::Builder::StatementBuilder selectValueMappingBuilder; + selectValueMappingBuilder.Select(s_OneToManyTable_MapTable_ManifestName).From({ tableName, s_OneToManyTable_MapTable_Suffix }).Where(valueName).Equals(SQLite::Builder::Unbound).Limit(1); + + SelectIfAnyMappingsByValueId = selectValueMappingBuilder.Prepare(connection); + + SQLite::Builder::StatementBuilder deleteValueBuilder; + deleteValueBuilder.DeleteFrom(tableName).Where(SQLite::RowIDName).Equals(SQLite::Builder::Unbound); + + DeleteValueById = deleteValueBuilder.Prepare(connection); + } + + void Execute(SQLite::rowid_t valueId) + { + SelectIfAnyMappingsByValueId.Reset(); + SelectIfAnyMappingsByValueId.Bind(1, valueId); + + // If no rows are found, we can delete the data. + if (!SelectIfAnyMappingsByValueId.Step()) + { + DeleteValueById.Reset(); + DeleteValueById.Bind(1, valueId); + + DeleteValueById.Execute(); + } + } + + private: + // Bind valid rowid to 1. + SQLite::Statement SelectIfAnyMappingsByValueId; + // Bind valid rowid to 1. + SQLite::Statement DeleteValueById; + }; + } + void CreateOneToManyTable(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName) { using namespace SQLite::Builder; - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, std::string{ tableName } +"_create_v1_0"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, std::string{ tableName } + "_create_v1_0"); // Create the data table as a 1:1 CreateOneToOneTable(connection, tableName, valueName); @@ -40,14 +109,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 std::string_view tableName, std::string_view valueName, const std::vector<std::string>& values, SQLite::rowid_t manifestId) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, std::string{ tableName } +"_ensureandinsert_v1_0"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, std::string{ tableName } + "_ensureandinsert_v1_0"); - // Create the mapping table insert statement for multiple use - SQLite::Builder::StatementBuilder insertMappingBuilder; - insertMappingBuilder.InsertInto({ tableName, s_OneToManyTable_MapTable_Suffix }). - Columns({ s_OneToManyTable_MapTable_ManifestName, valueName }).Values(manifestId, SQLite::Builder::Unbound); - - SQLite::Statement insertMapping = insertMappingBuilder.Prepare(connection); + SQLite::Statement insertMapping = CreateMappingInsertStatementForManifestId(connection, tableName, valueName, manifestId); for (const std::string& value : values) { @@ -64,23 +128,68 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 savepoint.Commit(); } - void OneToManyTableDeleteIfNotNeededByManifestId(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, SQLite::rowid_t manifestId) + bool OneToManyTableUpdateIfNeededByManifestId(SQLite::Connection& connection, + std::string_view tableName, std::string_view valueName, + const std::vector<std::string>& values, SQLite::rowid_t manifestId) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, std::string{ tableName } +"_deleteifnotneeded_v1_0"); + std::vector<SQLite::rowid_t> oldValueIds = GetValueIdsByManifestId(connection, tableName, valueName, manifestId); + bool modificationNeeded = false; - // Get values referenced by the manifest id. - std::vector<SQLite::rowid_t> values; + SQLite::Statement insertMapping = CreateMappingInsertStatementForManifestId(connection, tableName, valueName, manifestId); + + for (const std::string& value : values) + { + SQLite::rowid_t valueId = OneToOneTableEnsureExists(connection, tableName, valueName, value); + + auto itr = std::find(oldValueIds.begin(), oldValueIds.end(), valueId); + if (itr != oldValueIds.end()) + { + oldValueIds.erase(itr); + } + else + { + modificationNeeded = true; - SQLite::Builder::StatementBuilder selectMappingBuilder; - selectMappingBuilder.Select(valueName).From({ tableName, s_OneToManyTable_MapTable_Suffix }).Where(s_OneToManyTable_MapTable_ManifestName).Equals(manifestId); + insertMapping.Reset(); + insertMapping.Bind(2, valueId); - SQLite::Statement selectMappingStatement = selectMappingBuilder.Prepare(connection); + insertMapping.Execute(); + } + } + + // All incoming values are now present, we just need to delete the remaining old ones. + SQLite::Builder::StatementBuilder deleteBuilder; + deleteBuilder.DeleteFrom({ tableName, s_OneToManyTable_MapTable_Suffix }). + Where(s_OneToManyTable_MapTable_ManifestName).Equals(manifestId).And(valueName).Equals(SQLite::Builder::Unbound); - while (selectMappingStatement.Step()) + SQLite::Statement deleteStatement = deleteBuilder.Prepare(connection); + + DeleteValueIfNotNeededStatements dvinns(connection, tableName, valueName); + + for (SQLite::rowid_t valueId : oldValueIds) { - values.push_back(selectMappingStatement.GetColumn<SQLite::rowid_t>(0)); + modificationNeeded = true; + + // First, delete the mapping + deleteStatement.Reset(); + deleteStatement.Bind(2, valueId); + + deleteStatement.Execute(); + + // Second, delete the value itself if not needed + dvinns.Execute(valueId); } + return modificationNeeded; + } + + void OneToManyTableDeleteIfNotNeededByManifestId(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, SQLite::rowid_t manifestId) + { + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, std::string{ tableName } + "_deleteifnotneeded_v1_0"); + + // Get values referenced by the manifest id. + std::vector<SQLite::rowid_t> values = GetValueIdsByManifestId(connection, tableName, valueName, manifestId); + // Delete the mapping table rows with the manifest id. SQLite::Builder::StatementBuilder deleteBuilder; deleteBuilder.DeleteFrom({ tableName, s_OneToManyTable_MapTable_Suffix }).Where(s_OneToManyTable_MapTable_ManifestName).Equals(manifestId); @@ -88,29 +197,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 deleteBuilder.Execute(connection); // For each value, see if any references exist - SQLite::Builder::StatementBuilder selectValueMappingBuilder; - selectValueMappingBuilder.Select(s_OneToManyTable_MapTable_ManifestName).From({ tableName, s_OneToManyTable_MapTable_Suffix }).Where(valueName).Equals(SQLite::Builder::Unbound).Limit(1); - - SQLite::Statement selectValueMappingStatement = selectValueMappingBuilder.Prepare(connection); - - SQLite::Builder::StatementBuilder deleteValueBuilder; - deleteValueBuilder.DeleteFrom(tableName).Where(SQLite::RowIDName).Equals(SQLite::Builder::Unbound); - - SQLite::Statement deleteValueStatement = deleteValueBuilder.Prepare(connection); + DeleteValueIfNotNeededStatements dvinns(connection, tableName, valueName); for (SQLite::rowid_t value : values) { - selectValueMappingStatement.Reset(); - selectValueMappingStatement.Bind(1, value); - - // If no rows are found, we can delete the data. - if (!selectValueMappingStatement.Step()) - { - deleteValueStatement.Reset(); - deleteValueStatement.Bind(1, value); - - deleteValueStatement.Execute(); - } + dvinns.Execute(value); } savepoint.Commit(); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/OneToManyTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/OneToManyTable.h @@ -19,6 +19,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 std::string_view tableName, std::string_view valueName, const std::vector<std::string>& values, SQLite::rowid_t manifestId); + // Updates the mapping table to represent the given values for the manifest. + bool OneToManyTableUpdateIfNeededByManifestId(SQLite::Connection& connection, + std::string_view tableName, std::string_view valueName, + const std::vector<std::string>& values, SQLite::rowid_t manifestId); + // Deletes the mapping rows for the given manifest, then removes any unused data rows. void OneToManyTableDeleteIfNotNeededByManifestId(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, SQLite::rowid_t manifestId); @@ -42,6 +47,12 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 details::OneToManyTableEnsureExistsAndInsert(connection, TableInfo::TableName(), TableInfo::ValueName(), values, manifestId); } + // Updates the mapping table to represent the given values for the manifest. + static bool UpdateIfNeededByManifestId(SQLite::Connection& connection, const std::vector<std::string>& values, SQLite::rowid_t manifestId) + { + return details::OneToManyTableUpdateIfNeededByManifestId(connection, TableInfo::TableName(), TableInfo::ValueName(), values, manifestId); + } + // Deletes the mapping rows for the given manifest, then removes any unused data rows. static void DeleteIfNotNeededByManifestId(SQLite::Connection& connection, SQLite::rowid_t manifestId) { diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h @@ -30,7 +30,7 @@ namespace AppInstaller::Repository::Microsoft::Schema virtual void AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) = 0; // Updates the manifest at the repository relative path in the index. - virtual bool UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& oldManifest, const std::filesystem::path& oldRelativePath, const Manifest::Manifest& newManifest, const std::filesystem::path& newRelativePath) = 0; + virtual bool UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) = 0; // Removes the manifest at the repository relative path from the index. virtual void RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) = 0; diff --git a/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.cpp b/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.cpp @@ -440,6 +440,25 @@ namespace AppInstaller::Repository::SQLite::Builder return *this; } + StatementBuilder& StatementBuilder::Update(std::string_view table) + { + OutputOperationAndTable(m_stream, "UPDATE", table); + return *this; + } + + StatementBuilder& StatementBuilder::Update(std::initializer_list<std::string_view> table) + { + OutputOperationAndTable(m_stream, "UPDATE", table); + return *this; + } + + StatementBuilder& StatementBuilder::Set() + { + m_stream << " SET "; + m_needsComma = false; + return *this; + } + Statement StatementBuilder::Prepare(Connection& connection, bool persistent) { Statement result = Statement::Create(connection, m_stream.str(), persistent); diff --git a/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.h b/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.h @@ -252,6 +252,14 @@ namespace AppInstaller::Repository::SQLite::Builder StatementBuilder& DeleteFrom(std::string_view table); StatementBuilder& DeleteFrom(std::initializer_list<std::string_view> table); + // Begin an update statement. + // The initializer_list form enables the table name to be constructed from multiple parts. + StatementBuilder& Update(std::string_view table); + StatementBuilder& Update(std::initializer_list<std::string_view> table); + + // Output the set portion of an update statement. + StatementBuilder& Set(); + // Prepares and returns the statement, applying any bindings that were requested. Statement Prepare(Connection& connection, bool persistent = false); diff --git a/src/AppInstallerRepositoryCore/SQLiteWrapper.cpp b/src/AppInstallerRepositoryCore/SQLiteWrapper.cpp @@ -181,8 +181,8 @@ namespace AppInstaller::Repository::SQLite using namespace std::string_literals; Statement begin = Statement::Create(connection, "SAVEPOINT ["s + m_name + "]"); - m_rollback = Statement::Create(connection, "ROLLBACK TO ["s + m_name + "]", true); - m_commit = Statement::Create(connection, "RELEASE ["s + m_name + "]", true); + m_rollbackTo = Statement::Create(connection, "ROLLBACK TO ["s + m_name + "]", true); + m_release = Statement::Create(connection, "RELEASE ["s + m_name + "]", true); AICLI_LOG(SQL, Info, << "Begin savepoint: " << m_name); begin.Step(); @@ -203,7 +203,11 @@ namespace AppInstaller::Repository::SQLite if (m_inProgress) { AICLI_LOG(SQL, Info, << "Roll back savepoint: " << m_name); - m_rollback.Step(true); + m_rollbackTo.Step(true); + // 'ROLLBACK TO' *DOES NOT* remove the savepoint from the transaction stack. + // In order to remove it, we must RELEASE. Since we just invoked a ROLLBACK TO + // this should have the effect of 'committing' nothing. + m_release.Step(true); m_inProgress = false; } } @@ -213,7 +217,7 @@ namespace AppInstaller::Repository::SQLite if (m_inProgress) { AICLI_LOG(SQL, Info, << "Commit savepoint: " << m_name); - m_commit.Step(true); + m_release.Step(true); m_inProgress = false; } } diff --git a/src/AppInstallerRepositoryCore/SQLiteWrapper.h b/src/AppInstallerRepositoryCore/SQLiteWrapper.h @@ -245,7 +245,7 @@ namespace AppInstaller::Repository::SQLite std::string m_name; DestructionToken m_inProgress = true; - Statement m_rollback; - Statement m_commit; + Statement m_rollbackTo; + Statement m_release; }; } diff --git a/src/AppInstallerRepositoryCore/pch.h b/src/AppInstallerRepositoryCore/pch.h @@ -17,6 +17,7 @@ #include <winrt/Windows.Foundation.h> +#include <algorithm> #include <filesystem> #include <initializer_list> #include <iomanip> diff --git a/src/AppInstallerSQLiteIndexUtil/AppInstallerSQLiteIndexUtil.h b/src/AppInstallerSQLiteIndexUtil/AppInstallerSQLiteIndexUtil.h @@ -48,12 +48,10 @@ extern "C" // Updates the manifest at the repository relative path in the index. // The out value indicates whether the index was modified by the function. - APPINSTALLER_SQLITE_INDEX_API AppInstallerSQLiteIndexUpdateManifest( + APPINSTALLER_SQLITE_INDEX_API ApdateManifest( APPINSTALLER_SQLITE_INDEX_HANDLE index, - APPINSTALLER_SQLITE_INDEX_STRING oldManifestPath, - APPINSTALLER_SQLITE_INDEX_STRING oldRelativePath, - APPINSTALLER_SQLITE_INDEX_STRING newManifestPath, - APPINSTALLER_SQLITE_INDEX_STRING newRelativePath, + APPINSTALLER_SQLITE_INDEX_STRING manifestPath, + APPINSTALLER_SQLITE_INDEX_STRING relativePath, bool* indexModified); // Removes the manifest at the repository relative path from the index. diff --git a/src/AppInstallerSQLiteIndexUtil/Exports.cpp b/src/AppInstallerSQLiteIndexUtil/Exports.cpp @@ -105,17 +105,15 @@ extern "C" APPINSTALLER_SQLITE_INDEX_API AppInstallerSQLiteIndexUpdateManifest( APPINSTALLER_SQLITE_INDEX_HANDLE index, - APPINSTALLER_SQLITE_INDEX_STRING oldManifestPath, APPINSTALLER_SQLITE_INDEX_STRING oldRelativePath, - APPINSTALLER_SQLITE_INDEX_STRING newManifestPath, APPINSTALLER_SQLITE_INDEX_STRING newRelativePath, + APPINSTALLER_SQLITE_INDEX_STRING manifestPath, + APPINSTALLER_SQLITE_INDEX_STRING relativePath, bool* indexModified) try { THROW_HR_IF(E_INVALIDARG, !index); - THROW_HR_IF(E_INVALIDARG, !oldManifestPath); - THROW_HR_IF(E_INVALIDARG, !oldRelativePath); - THROW_HR_IF(E_INVALIDARG, !newManifestPath); - THROW_HR_IF(E_INVALIDARG, !newRelativePath); + THROW_HR_IF(E_INVALIDARG, !manifestPath); + THROW_HR_IF(E_INVALIDARG, !relativePath); - bool result = reinterpret_cast<SQLiteIndex*>(index)->UpdateManifest(oldManifestPath, oldRelativePath, newManifestPath, newRelativePath); + bool result = reinterpret_cast<SQLiteIndex*>(index)->UpdateManifest(manifestPath, relativePath); if (indexModified) { *indexModified = result; diff --git a/src/AppInstallerTestExeInstaller/main.cpp b/src/AppInstallerTestExeInstaller/main.cpp @@ -3,10 +3,13 @@ #include <iostream> #include <fstream> +#include <filesystem> int main(int argc, const char** argv) { - std::ofstream file("TestExeInstalled.txt", std::ofstream::out); + std::filesystem::path temp = std::filesystem::temp_directory_path(); + temp /= "TestExeInstalled.txt"; + std::ofstream file(temp, std::ofstream::out); for (int i = 1; i < argc; i++) {