Improve index dependency consistency check (#2247)

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]")

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.

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;
+    }
 }

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);
     };
 }

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;
     };
 }

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;
+        }
+    }
 }

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;
     };
 }

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 (...)
         {

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)
         {

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(