commit 8774f0fe0e3265d5e731ae96de9859e039db9190
parent 9056c0fc3fe4a362ef2644ba080f88698836576b
Author: yao-msft <50888816+yao-msft@users.noreply.github.com>
Date: Wed, 22 Jun 2022 11:10:21 -0700
Improve index dependency consistency check (#2247)
Diffstat:
10 files changed, 194 insertions(+), 10 deletions(-)
diff --git a/src/AppInstallerCLITests/SQLiteIndex.cpp b/src/AppInstallerCLITests/SQLiteIndex.cpp
@@ -609,6 +609,54 @@ TEST_CASE("SQLiteIndex_DependenciesTable_CheckConsistency", "[sqliteindex][V1_4]
REQUIRE(!index.CheckConsistency(true));
}
+
+ TempFile tempFile2{ "repolibtest_tempdb"s, ".db"s };
+ INFO("Using temporary file named: " << tempFile2.GetPath());
+
+ {
+ SQLiteIndex index = CreateTestIndex(tempFile2, Schema::Version::Latest());
+
+ Manifest manifest;
+ manifest.Id = "Foo";
+ manifest.Version = "10.0";
+
+ index.AddManifest(manifest, "path");
+
+ REQUIRE(index.CheckConsistency(true));
+
+ // Add dependency that does not require min version
+ Manifest manifestWithDependency1;
+ manifestWithDependency1.Id = "Bar1";
+ manifestWithDependency1.Version = "10.0";
+ manifestWithDependency1.Installers.push_back({});
+ manifestWithDependency1.Installers[0].Dependencies.Add(Dependency(DependencyType::Package, manifest.Id));
+
+ index.AddManifest(manifestWithDependency1, "path1");
+
+ REQUIRE(index.CheckConsistency(true));
+
+ // Add dependency with min version satisfied
+ Manifest manifestWithDependency2;
+ manifestWithDependency2.Id = "Bar2";
+ manifestWithDependency2.Version = "10.0";
+ manifestWithDependency2.Installers.push_back({});
+ manifestWithDependency2.Installers[0].Dependencies.Add(Dependency(DependencyType::Package, manifest.Id, "1.0"));
+
+ index.AddManifest(manifestWithDependency2, "path2");
+
+ REQUIRE(index.CheckConsistency(true));
+
+ // Add dependency with min version not satisfied
+ Manifest manifestWithDependency3;
+ manifestWithDependency3.Id = "Bar3";
+ manifestWithDependency3.Version = "10.0";
+ manifestWithDependency3.Installers.push_back({});
+ manifestWithDependency3.Installers[0].Dependencies.Add(Dependency(DependencyType::Package, manifest.Id, "11.0"));
+
+ index.AddManifest(manifestWithDependency3, "path3");
+
+ REQUIRE_FALSE(index.CheckConsistency(true));
+ }
}
TEST_CASE("SQLiteIndex_RemoveManifestFile_NotPresent", "[sqliteindex]")
diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp
@@ -159,7 +159,12 @@ namespace AppInstaller::Repository
{
auto&& [manifestVersion, arpMinVersion, arpMaxVersion] = std::move(tuple);
Utility::VersionRange arpVersionRange{ Utility::Version(std::move(arpMinVersion)), Utility::Version(std::move(arpMaxVersion)) };
- arpVersionMap.emplace_back(std::make_pair(Utility::Version{ std::move(manifestVersion) }, std::move(arpVersionRange)));
+ Utility::Version manifestVer{ std::move(manifestVersion) };
+ // Skip mapping to unknown version
+ if (!manifestVer.IsUnknown())
+ {
+ arpVersionMap.emplace_back(std::make_pair(std::move(manifestVer), std::move(arpVersionRange)));
+ }
}
// Go through the arp version map and determine what mapping should be performed.
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/DependenciesTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/DependenciesTable.cpp
@@ -426,7 +426,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
.LeftOuterJoin(VersionTable::TableName())
.On(QCol(s_DependenciesTable_Table_Name, s_DependenciesTable_MinVersion_Column_Name), QCol(VersionTable::TableName(), SQLite::RowIDName))
.Where(QCol(ManifestTable::TableName(), SQLite::RowIDName)).IsNull()
- .Or(QCol(VersionTable::TableName(), SQLite::RowIDName)).IsNull()
+ .Or(QCol(VersionTable::TableName(), SQLite::RowIDName)).IsNull().And(QCol(s_DependenciesTable_Table_Name, s_DependenciesTable_MinVersion_Column_Name)).IsNotNull()
.Or(QCol(IdTable::TableName(), SQLite::RowIDName)).IsNull();
SQLite::Statement select = builder.Prepare(connection);
@@ -515,4 +515,46 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
return result;
}
+
+ std::vector<std::pair<SQLite::rowid_t, Utility::NormalizedString>> DependenciesTable::GetAllDependenciesWithMinVersions(const SQLite::Connection& connection)
+ {
+ if (!Exists(connection))
+ {
+ return {};
+ }
+
+ std::vector<std::pair<SQLite::rowid_t, Utility::NormalizedString>> result;
+
+ constexpr std::string_view depTableAlias = "dep";
+ constexpr std::string_view minVersionAlias = "minV";
+
+ StatementBuilder builder;
+
+ // SELECT [dep].[package_id], [minV].[version] FROM [dependencies] AS [dep]
+ // JOIN [versions] AS [minV] ON [minV].[rowid] = [dep].[min_version]
+ builder.Select()
+ .Column(QCol(depTableAlias, s_DependenciesTable_PackageId_Column_Name))
+ .Column(QCol(minVersionAlias, VersionTable::ValueName()))
+ .From({ s_DependenciesTable_Table_Name }).As(depTableAlias)
+ .Join({ VersionTable::TableName() }).As(minVersionAlias)
+ .On(QCol(minVersionAlias, SQLite::RowIDName), QCol(depTableAlias, s_DependenciesTable_MinVersion_Column_Name));
+
+ SQLite::Statement select = builder.Prepare(connection);
+
+ while (select.Step())
+ {
+ Utility::NormalizedString version = "";
+ if (!select.GetColumnIsNull(1))
+ {
+ version = select.GetColumn<std::string>(1);
+ }
+
+ if (!version.empty())
+ {
+ result.emplace_back(std::make_pair(select.GetColumn<SQLite::rowid_t>(0), version));
+ }
+ }
+
+ return result;
+ }
}
\ No newline at end of file
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/DependenciesTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/DependenciesTable.h
@@ -46,8 +46,13 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
// Checks if the row id is present in the column denoted by the value supplied.
static bool IsValueReferenced(const SQLite::Connection& connection, std::string_view valueName, SQLite::rowid_t valueRowId);
+ // The dependencies table and corresponding index are dropped.
static void PrepareForPackaging(SQLite::Connection& connection);
+ // Get all min version values of the given manifest's dependencies, used for VersionTable cleanup when updating or removing a manifest.
static std::vector<SQLite::rowid_t> GetDependenciesMinVersionsRowIdByManifestId(const SQLite::Connection& connection, SQLite::rowid_t manifestRowId);
+
+ // Get all dependencies with min versions in the dependencies table, used during consistency check. Returning a list of <PackageRowId, VersionString> pair.
+ static std::vector<std::pair<SQLite::rowid_t, Utility::NormalizedString>> GetAllDependenciesWithMinVersions(const SQLite::Connection& connection);
};
}
\ No newline at end of file
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/Interface.h
@@ -25,5 +25,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
protected:
bool NotNeeded(const SQLite::Connection& connection, std::string_view tableName, std::string_view valueName, SQLite::rowid_t id) const override;
+
+ private:
+ // Semantic check to validate dependencies with min versions are satisfied.
+ bool ValidateDependenciesWithMinVersions(const SQLite::Connection& connection, bool log) const;
};
}
\ No newline at end of file
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/Interface_1_4.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/Interface_1_4.cpp
@@ -120,6 +120,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
result = DependenciesTable::DependenciesTableCheckConsistency(connection, log) && result;
}
+ if (result || log)
+ {
+ result = ValidateDependenciesWithMinVersions(connection, log) && result;
+ }
+
return result;
}
@@ -132,4 +137,45 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4
{
return DependenciesTable::GetDependentsById(connection, packageId);
}
+
+ bool Interface::ValidateDependenciesWithMinVersions(const SQLite::Connection& connection, bool log) const
+ {
+ try
+ {
+ bool result = true;
+ // A map to store already checked dependency package latest versions.
+ std::map<SQLite::rowid_t, Utility::Version> checkedVersions;
+
+ auto dependencies = DependenciesTable::GetAllDependenciesWithMinVersions(connection);
+ for (auto const& dependency : dependencies)
+ {
+ // If the dependency package has not been checked yet, add to the map.
+ if (checkedVersions.find(dependency.first) == checkedVersions.end())
+ {
+ auto versionKeys = GetVersionKeysById(connection, dependency.first);
+ THROW_HR_IF(E_UNEXPECTED, versionKeys.empty());
+ checkedVersions.emplace(dependency.first, versionKeys[0].GetVersion());
+ }
+
+ // If the latest version is less than min version required, fail the validation.
+ if (checkedVersions[dependency.first] < Utility::Version{ dependency.second })
+ {
+ AICLI_LOG(Repo, Error, << "Dependency with min version not satisfied. Dependency package row id: " << dependency.first << " min version: " << dependency.second);
+ result = false;
+
+ if (!log)
+ {
+ break;
+ }
+ }
+ }
+
+ return result;
+ }
+ catch (...)
+ {
+ AICLI_LOG(Repo, Error, << "ValidateDependenciesWithMinVersions() encountered internal error. Returning false.");
+ return false;
+ }
+ }
}
\ No newline at end of file
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_5/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_5/Interface.h
@@ -28,6 +28,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_5
private:
// Semantic check to validate all arp version ranges within the index
- bool ValidateArpVersionConsistency(const SQLite::Connection& connection) const;
+ bool ValidateArpVersionConsistency(const SQLite::Connection& connection, bool log) const;
};
}
\ No newline at end of file
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_5/Interface_1_5.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_5/Interface_1_5.cpp
@@ -162,7 +162,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_5
if (result || log)
{
- result = ValidateArpVersionConsistency(connection) && result;
+ result = ValidateArpVersionConsistency(connection, log) && result;
}
return result;
@@ -181,10 +181,12 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_5
}
}
- bool Interface::ValidateArpVersionConsistency(const SQLite::Connection& connection) const
+ bool Interface::ValidateArpVersionConsistency(const SQLite::Connection& connection, bool log) const
{
try
{
+ bool result = true;
+
// Search everything
SearchRequest request;
auto searchResult = Search(connection, request);
@@ -215,11 +217,16 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_5
if (Utility::HasOverlapInVersionRanges(ranges))
{
AICLI_LOG(Repo, Error, << "Overlapped Arp version ranges found for package. PackageRowId: " << match.first);
- return false;
+ result = false;
+
+ if (!log)
+ {
+ break;
+ }
}
}
- return true;
+ return result;
}
catch (...)
{
diff --git a/src/WinGetUtil/Exports.cpp b/src/WinGetUtil/Exports.cpp
@@ -307,8 +307,6 @@ extern "C"
}
CATCH_RETURN()
- DEFINE_ENUM_FLAG_OPERATORS(WinGetValidateManifestResult);
-
WINGET_UTIL_API WinGetValidateManifestV3(
WINGET_MANIFEST_HANDLE manifest,
WINGET_SQLITE_INDEX_HANDLE index,
@@ -370,6 +368,27 @@ extern "C"
}
}
+ if (WI_IsFlagSet(option, WinGetValidateManifestOptionV2::InstallerValidation))
+ {
+ try
+ {
+ auto errors = ValidateManifestInstallers(*manifestPtr);
+ if (errors.size() > 0)
+ {
+ // Throw the errors as ManifestExceptions to get processed errors and message.
+ THROW_EXCEPTION(ManifestException({ std::move(errors) }));
+ }
+ }
+ catch (const ManifestException& e)
+ {
+ WI_SetFlagIf(validationResult, WinGetValidateManifestResult::InstallerValidationFailure, !e.IsWarningOnly());
+ if (message)
+ {
+ validationMessage += e.GetManifestErrorMessage();
+ }
+ }
+ }
+
*result = validationResult;
if (message)
{
diff --git a/src/WinGetUtil/WinGetUtil.h b/src/WinGetUtil/WinGetUtil.h
@@ -28,6 +28,8 @@ extern "C"
InstallerValidations = 0x4,
};
+ DEFINE_ENUM_FLAG_OPERATORS(WinGetValidateManifestOption);
+
enum WinGetCreateManifestOption
{
// Just create the manifest without any validation
@@ -43,6 +45,8 @@ extern "C"
ReturnErrorOnVerifiedPublisherFields = 0x1000,
};
+ DEFINE_ENUM_FLAG_OPERATORS(WinGetCreateManifestOption);
+
enum WinGetValidateManifestOptionV2
{
// No validation, caller will get E_INVALIDARG
@@ -55,6 +59,8 @@ extern "C"
InstallerValidation = 0x4,
};
+ DEFINE_ENUM_FLAG_OPERATORS(WinGetValidateManifestOptionV2);
+
enum WinGetValidateManifestOperationType
{
OperationTypeAdd = 0,
@@ -75,13 +81,15 @@ extern "C"
InternalError = 0x1000,
};
+ DEFINE_ENUM_FLAG_OPERATORS(WinGetValidateManifestResult);
+
enum WinGetValidateManifestDependenciesOption
{
DefaultValidation = 0,
ForDelete = 0x1,
};
- DEFINE_ENUM_FLAG_OPERATORS(WinGetValidateManifestOption);
+ DEFINE_ENUM_FLAG_OPERATORS(WinGetValidateManifestDependenciesOption);
// Initializes the logging infrastructure.
WINGET_UTIL_API WinGetLoggingInit(
