commit 7b0866c2d594939e9ba9a8078551c02e8c26be3d
parent 066616193477388f3390121ad90d10e25a45836a
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Thu, 25 Jun 2020 17:14:20 -0700
Case insensitivity improvements (#459)
Makes version and channel case insensitive for lookups from the user. They already were required to be case insensitive unique, so this shouldn't have any effect other than making use easier.
Causes an index modification (add / update) to overwrite the existing Id value with the incoming one if the casing is different. So last writer wins on Id casing. Other fields can have distinct casing per manifest revision for now, but since Id needs to be the thing that ties them together, it needs to be a single value.
Diffstat:
4 files changed, 140 insertions(+), 11 deletions(-)
diff --git a/src/AppInstallerCLITests/SQLiteIndex.cpp b/src/AppInstallerCLITests/SQLiteIndex.cpp
@@ -531,6 +531,90 @@ TEST_CASE("SQLiteIndex_UpdateManifestChangeCase", "[sqliteindex][V1_0]")
REQUIRE(Schema::V1_0::CommandsTable::IsEmpty(connection));
}
+TEST_CASE("SQLiteIndex_IdCaseInsensitivity", "[sqliteindex][V1_0]")
+{
+ TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
+ INFO("Using temporary file named: " << tempFile.GetPath());
+
+ std::string manifest1Path = "test/id/test.id-1.0.0.yaml";
+ Manifest manifest1;
+ manifest1.Id = "test.id";
+ manifest1.Name = "Test Name";
+ manifest1.AppMoniker = "testmoniker";
+ manifest1.Version = "1.0.0";
+ manifest1.Tags = { "t1", "t2" };
+ manifest1.Commands = { "test1", "test2" };
+
+ std::string manifest2Path = "test/id/test.id-2.0.0.yaml";
+ Manifest manifest2 = manifest1;
+ manifest2.Id = "Test.Id";
+ manifest1.Version = "2.0.0";
+
+ {
+ SQLiteIndex index = SQLiteIndex::CreateNew(tempFile, { 1, 0 });
+
+ index.AddManifest(manifest1, manifest1Path);
+
+ auto results = index.Search({});
+ REQUIRE(results.Matches.size() == 1);
+ REQUIRE(manifest1.Id == index.GetIdStringById(results.Matches[0].first));
+ }
+
+ {
+ SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite);
+
+ index.AddManifest(manifest2, manifest2Path);
+
+ auto results = index.Search({});
+ REQUIRE(results.Matches.size() == 1);
+ REQUIRE(manifest2.Id == index.GetIdStringById(results.Matches[0].first));
+ }
+
+ {
+ SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite);
+
+ manifest1.Id = "TEST.ID";
+
+ REQUIRE(index.UpdateManifest(manifest1, manifest1Path));
+
+ auto results = index.Search({});
+ REQUIRE(results.Matches.size() == 1);
+ REQUIRE(manifest1.Id == index.GetIdStringById(results.Matches[0].first));
+ }
+
+ {
+ SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite);
+
+ index.RemoveManifest(manifest1, manifest1Path);
+
+ auto results = index.Search({});
+ REQUIRE(results.Matches.size() == 1);
+ REQUIRE(manifest1.Id == index.GetIdStringById(results.Matches[0].first));
+ }
+
+ {
+ SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite);
+
+ index.RemoveManifest(manifest2, manifest2Path);
+
+ auto results = index.Search({});
+ REQUIRE(results.Matches.empty());
+ }
+
+ // 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
@@ -853,6 +937,38 @@ TEST_CASE("SQLiteIndex_PathString_VersionSorting", "[sqliteindex]")
REQUIRE(!result.has_value());
}
+TEST_CASE("SQLiteIndex_PathString_CaseInsensitive", "[sqliteindex]")
+{
+ TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
+ INFO("Using temporary file named: " << tempFile.GetPath());
+
+ SQLiteIndex index = SearchTestSetup(tempFile, {
+ { "Id", "Name", "Moniker", "14.0.0", "", { "foot" }, { "com34" }, "Path1" },
+ { "Id", "Name", "Moniker", "16.0.0", "alpha", { "floor" }, { "com3" }, "Path2" },
+ { "Id", "Name", "Moniker", "15.0.0", "", {}, { "Command" }, "Path3" },
+ { "Id", "Name", "Moniker", "13.2.0-BUGFIX", "", {}, { "Command" }, "Path4" },
+ { "Id", "Name", "Moniker", "15.1.0", "beta", { "foo" }, { "com3" }, "Path5" },
+ { "Id", "Name", "Moniker", "15.8.0", "alpha", { "foo" }, { "com3" }, "Path6" },
+ { "Id", "Name", "Moniker", "13.2.0-bugfix", "beta", { "foo" }, { "com3" }, "Path7" },
+ { "Id", "Name", "Moniker", "13.0.0", "", { "foo" }, { "com3" }, "Path8" },
+ });
+
+ SearchRequest request;
+ request.Filters.emplace_back(ApplicationMatchField::Id, MatchType::Exact, "Id");
+
+ auto results = index.Search(request);
+ REQUIRE(results.Matches.size() == 1);
+
+ auto result = index.GetPathStringByKey(results.Matches[0].first, "", "Alpha");
+ REQUIRE(result.has_value());
+
+ result = index.GetPathStringByKey(results.Matches[0].first, "13.2.0-BugFix", "");
+ REQUIRE(result.has_value());
+
+ result = index.GetPathStringByKey(results.Matches[0].first, "13.2.0-BugFix", "BETA");
+ REQUIRE(!result.has_value());
+}
+
TEST_CASE("SQLiteIndex_SearchResultsTableSearches", "[sqliteindex][V1_0]")
{
TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.cpp
@@ -60,7 +60,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
// Gets a manifest id by the given key values.
std::optional<SQLite::rowid_t> GetManifestIdByKey(SQLite::Connection& connection, SQLite::rowid_t id, std::string_view version = "", std::string_view channel = "")
{
- std::optional<SQLite::rowid_t> channelIdOpt = ChannelTable::SelectIdByValue(connection, channel);
+ std::optional<SQLite::rowid_t> channelIdOpt = ChannelTable::SelectIdByValue(connection, channel, true);
if (!channelIdOpt && !channel.empty())
{
// If an empty channel was given but none was found, we will just not filter on channel.
@@ -104,7 +104,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
}
else
{
- versionIdOpt = VersionTable::SelectIdByValue(connection, version);
+ versionIdOpt = VersionTable::SelectIdByValue(connection, version, true);
}
if (!versionIdOpt)
@@ -125,11 +125,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
// 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)
+ void UpdateManifestValueById(SQLite::Connection& connection, const typename Table::value_t& value, SQLite::rowid_t manifestId, bool overwriteLikeMatch = false)
{
auto [oldValueId] = ManifestTable::GetIdsById<Table>(connection, manifestId);
- SQLite::rowid_t newValueId = Table::EnsureExists(connection, value);
+ SQLite::rowid_t newValueId = Table::EnsureExists(connection, value, overwriteLikeMatch);
ManifestTable::UpdateValueIdById<Table>(connection, manifestId, newValueId);
@@ -204,7 +204,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), !pathAdded);
// Ensure that all of the 1:1 data exists.
- SQLite::rowid_t idId = IdTable::EnsureExists(connection, manifest.Id);
+ SQLite::rowid_t idId = IdTable::EnsureExists(connection, manifest.Id, true);
SQLite::rowid_t nameId = NameTable::EnsureExists(connection, manifest.Name);
SQLite::rowid_t monikerId = MonikerTable::EnsureExists(connection, manifest.AppMoniker);
SQLite::rowid_t versionId = VersionTable::EnsureExists(connection, manifest.Version);
@@ -245,7 +245,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
// 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);
+ UpdateManifestValueById<IdTable>(connection, manifest.Id, manifestId, true);
indexModified = true;
}
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/OneToOneTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/OneToOneTable.cpp
@@ -85,11 +85,24 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
return result;
}
- SQLite::rowid_t OneToOneTableEnsureExists(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, std::string_view value)
+ SQLite::rowid_t OneToOneTableEnsureExists(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, std::string_view value, bool overwriteLikeMatch)
{
- auto selectResult = OneToOneTableSelectIdByValue(connection, tableName, valueName, value);
+ auto selectResult = OneToOneTableSelectIdByValue(connection, tableName, valueName, value, overwriteLikeMatch);
if (selectResult)
{
+ if (overwriteLikeMatch)
+ {
+ // If the value in the table is not an exact match, overwrite it with the incoming value
+ auto tableValue = OneToOneTableSelectValueById(connection, tableName, valueName, selectResult.value());
+ if (tableValue.value() != value)
+ {
+ SQLite::Builder::StatementBuilder updateBuilder;
+ updateBuilder.Update(tableName).Set().Column(valueName).Equals(value).Where(SQLite::RowIDName).Equals(selectResult);
+
+ updateBuilder.Execute(connection);
+ }
+ }
+
return selectResult.value();
}
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/OneToOneTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/OneToOneTable.h
@@ -25,7 +25,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
std::vector<SQLite::rowid_t> OneToOneTableGetAllRowIds(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, size_t limit);
// Ensures that the values exists in the table.
- SQLite::rowid_t OneToOneTableEnsureExists(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, std::string_view value);
+ SQLite::rowid_t OneToOneTableEnsureExists(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, std::string_view value, bool overwriteLikeMatch = false);
// Removes the given row by its rowid if it is no longer referenced.
void OneToOneTableDeleteIfNotNeededById(SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, SQLite::rowid_t id);
@@ -90,9 +90,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
}
// Ensures that the given value exists in the table, returning the rowid.
- static SQLite::rowid_t EnsureExists(SQLite::Connection& connection, std::string_view value)
+ static SQLite::rowid_t EnsureExists(SQLite::Connection& connection, std::string_view value, bool overwriteLikeMatch = false)
{
- return details::OneToOneTableEnsureExists(connection, TableInfo::TableName(), TableInfo::ValueName(), value);
+ return details::OneToOneTableEnsureExists(connection, TableInfo::TableName(), TableInfo::ValueName(), value, overwriteLikeMatch);
}
// Removes the given row by its rowid if it is no longer referenced.
