commit b55a6920eb3a0c6e4f02ecadf0b545f66e7dfde4
parent ea0ca3419d344a8c13bfa9c4fe263ffb80e0e03b
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Fri, 15 May 2020 19:17:06 -0700
Make Id,Version,Channel key for manifest operations case insensitive through use of LIKE (#168)
Diffstat:
6 files changed, 128 insertions(+), 17 deletions(-)
diff --git a/src/AppInstallerCLITests/SQLiteIndex.cpp b/src/AppInstallerCLITests/SQLiteIndex.cpp
@@ -29,7 +29,7 @@ SQLiteIndex SimpleTestSetup(const std::string& filePath, Manifest& manifest, std
{
SQLiteIndex index = SQLiteIndex::CreateNew(filePath, Schema::Version::Latest());
- manifest.Id = "test.id";
+ manifest.Id = "Test.Id";
manifest.Name = "Test Name";
manifest.AppMoniker = "testmoniker";
manifest.Version = "1.0.0";
@@ -157,6 +157,10 @@ TEST_CASE("SQLiteIndexCreateAndAddManifestDuplicate", "[sqliteindex]")
// Attempting to add the same manifest at a different path should fail.
REQUIRE_THROWS_HR(index.AddManifest(manifest, "differentpath.yaml"), HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS));
+ // Attempting to add the same manifest with a differently cased Id at a different path should fail.
+ manifest.Id = ToLower(manifest.Id);
+ REQUIRE_THROWS_HR(index.AddManifest(manifest, "differentpath.yaml"), 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));
@@ -449,6 +453,84 @@ TEST_CASE("SQLiteIndex_UpdateManifestChangePath", "[sqliteindex][V1_0]")
REQUIRE(Schema::V1_0::CommandsTable::IsEmpty(connection));
}
+TEST_CASE("SQLiteIndex_UpdateManifestChangeCase", "[sqliteindex][V1_0]")
+{
+ TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
+ INFO("Using temporary file named: " << tempFile.GetPath());
+
+ std::string manifestPath = "test/id/test.id-1.0.0.yaml";
+ Manifest manifest;
+ manifest.Id = "test.id";
+ manifest.Name = "Test Name";
+ manifest.AppMoniker = "testmoniker";
+ manifest.Version = "1.0.0-test";
+ manifest.Channel = "test";
+ manifest.Tags = { "t1", "t2" };
+ manifest.Commands = { "test1", "test2" };
+
+ {
+ SQLiteIndex index = SQLiteIndex::CreateNew(tempFile, { 1, 0 });
+
+ index.AddManifest(manifest, manifestPath);
+ }
+
+ {
+ SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite);
+
+ manifest.Id = "Test.Id";
+
+ // Update with path update should indicate change
+ REQUIRE(index.UpdateManifest(manifest, manifestPath));
+ }
+
+ {
+ SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite);
+
+ manifest.Version = "1.0.0-Test";
+
+ // Update with path update should indicate change
+ REQUIRE(index.UpdateManifest(manifest, manifestPath));
+ }
+
+ {
+ SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite);
+
+ manifest.Channel = "Test";
+
+ // Update with path update should indicate change
+ REQUIRE(index.UpdateManifest(manifest, manifestPath));
+ }
+
+ {
+ SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite);
+
+ manifest.Name = "test name";
+
+ // Update with path update should indicate change
+ REQUIRE(index.UpdateManifest(manifest, manifestPath));
+ }
+
+ {
+ SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite);
+
+ // Now remove manifest, with unknown path
+ index.RemoveManifest(manifest, "");
+ }
+
+ // 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));
+}
+
TEST_CASE("PathPartTable_EnsurePathExists_Negative_Paths", "[sqliteindex][V1_0]")
{
// Open it directly to directly test pathpart table
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.cpp
@@ -26,21 +26,21 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
// 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);
+ std::optional<SQLite::rowid_t> idId = IdTable::SelectIdByValue(connection, manifest.Id, true);
if (!idId)
{
AICLI_LOG(Repo, Info, << "Did not find an Id { " << manifest.Id << " }");
return {};
}
- std::optional<SQLite::rowid_t> versionId = VersionTable::SelectIdByValue(connection, manifest.Version);
+ std::optional<SQLite::rowid_t> versionId = VersionTable::SelectIdByValue(connection, manifest.Version, true);
if (!versionId)
{
AICLI_LOG(Repo, Info, << "Did not find a Version { " << manifest.Version << " }");
return {};
}
- std::optional<SQLite::rowid_t> channelId = ChannelTable::SelectIdByValue(connection, manifest.Channel);
+ std::optional<SQLite::rowid_t> channelId = ChannelTable::SelectIdByValue(connection, manifest.Channel, true);
if (!channelId)
{
AICLI_LOG(Repo, Info, << "Did not find a Channel { " << manifest.Channel << " }");
@@ -239,16 +239,28 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
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");
+ bool indexModified = false;
+
+ // Id, Version, and Channel may have changed casing. If so, they too need to be updated.
+ if (idInIndex != manifest.Id)
+ {
+ UpdateManifestValueById<IdTable>(connection, manifest.Id, manifestId);
+ indexModified = true;
+ }
+
+ if (versionInIndex != manifest.Version)
+ {
+ UpdateManifestValueById<VersionTable>(connection, manifest.Version, manifestId);
+ indexModified = true;
+ }
+
+ if (channelInIndex != manifest.Channel)
+ {
+ UpdateManifestValueById<ChannelTable>(connection, manifest.Channel, manifestId);
+ indexModified = true;
+ }
- // If these values changed, we need to update them.
if (nameInIndex != manifest.Name)
{
UpdateManifestValueById<NameTable>(connection, manifest.Name, manifestId);
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/OneToOneTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/OneToOneTable.cpp
@@ -22,10 +22,19 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
createTableBuilder.Execute(connection);
}
- std::optional<SQLite::rowid_t> OneToOneTableSelectIdByValue(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, std::string_view value)
+ std::optional<SQLite::rowid_t> OneToOneTableSelectIdByValue(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, std::string_view value, bool useLike)
{
SQLite::Builder::StatementBuilder selectBuilder;
- selectBuilder.Select(SQLite::RowIDName).From(tableName).Where(valueName).Equals(value);
+ selectBuilder.Select(SQLite::RowIDName).From(tableName).Where(valueName);
+
+ if (useLike)
+ {
+ selectBuilder.LikeWithEscape(value);
+ }
+ else
+ {
+ selectBuilder.Equals(value);
+ }
SQLite::Statement select = selectBuilder.Prepare(connection);
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/OneToOneTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/OneToOneTable.h
@@ -16,7 +16,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
void CreateOneToOneTable(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName);
// Selects the value from the table, returning the rowid if it exists.
- std::optional<SQLite::rowid_t> OneToOneTableSelectIdByValue(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, std::string_view value);
+ std::optional<SQLite::rowid_t> OneToOneTableSelectIdByValue(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, std::string_view value, bool useLike = false);
// Selects the value from the table, returning the rowid if it exists.
std::optional<std::string> OneToOneTableSelectValueById(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, SQLite::rowid_t id);
@@ -72,9 +72,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
}
// Selects the value from the table, returning the rowid if it exists.
- static std::optional<SQLite::rowid_t> SelectIdByValue(SQLite::Connection& connection, std::string_view value)
+ static std::optional<SQLite::rowid_t> SelectIdByValue(SQLite::Connection& connection, std::string_view value, bool useLike = false)
{
- return details::OneToOneTableSelectIdByValue(connection, TableInfo::TableName(), TableInfo::ValueName(), value);
+ return details::OneToOneTableSelectIdByValue(connection, TableInfo::TableName(), TableInfo::ValueName(), value, useLike);
}
// Selects the value from the table, returning it if it exists.
diff --git a/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.cpp b/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.cpp
@@ -297,6 +297,12 @@ namespace AppInstaller::Repository::SQLite::Builder
return *this;
}
+ StatementBuilder& StatementBuilder::LikeWithEscape(std::string_view value)
+ {
+ AddBindFunctor(AppendOpAndBinder(Op::Like), EscapeStringForLike(value));
+ return Escape(EscapeCharForLike);
+ }
+
StatementBuilder& StatementBuilder::Like(details::unbound_t)
{
AppendOpAndBinder(Op::Like);
diff --git a/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.h b/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.h
@@ -202,7 +202,9 @@ namespace AppInstaller::Repository::SQLite::Builder
StatementBuilder& Equals(details::unbound_t);
StatementBuilder& Equals(std::nullptr_t);
+ StatementBuilder& LikeWithEscape(std::string_view value);
StatementBuilder& Like(details::unbound_t);
+
StatementBuilder& Escape(std::string_view escapeChar);
StatementBuilder& Not();
