From 296a53dbedffabfd14b60f43e1a7abae0200cb75 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 7 Nov 2023 19:05:12 -0800 Subject: [PATCH] Performance improvements (#3808) ## Change This change has several performance improvements based on profiling. ### Installed package caching A global process cache is stored for the unfiltered install data. It is very basic for updates right now to make it more reasonable to service, but future changes could improve things to examine the system data and only do delta updates. It could also be saved to disk to improve the performance of future processes. The cache itself is not used by any callers, but instead in-memory copies are handed out. The cache sets up event listeners for the current data sources and waits for these events before it will attempt to update again. A full cache hit requires only copying the memory database, which SQLite has an efficient method for. The only performance gain to the update path that is provided by this implementation is to reuse the names of MSIX packages, but since this accounts for a huge amount of creating the index it results in ~50% time reduction in creating an updated installed source (without any MSIX changes). ### ICU regex caching The regular expressions that we use were being compiled repeatedly. Since this is a fixed set of expressions, they are all now cached and copies are used for actual operation. ### Version lookup (string) The version lookup by string (such as `winget show FOO -v VER`) was doing a full table scan. This change shifts to pulling all of the version for the given identifier out and using `Version` object comparisons to find the proper result. This has the side effect of fixing a somewhat annoying user interaction where trailing 0s in a version were still required to find from the command line. One can now provide the smaller version string and still find the expected package version (eg `1.2` will now find version `1.2.0`). ### Version lookup (available version) The round trip from retrieving the available version information to getting the manifest has been improved by including the manifest id value directly in the available version data. The available version package object now caches the version string to manifest id data as well, meaning that any usage of a `PackageVersionKey` that was retrieved from `GetAvailableVersions` will directly return the package version object without querying the index. ### Let property lookups handle an unknown manifest id The `GetProperty` code was enforcing that the manifest id was valid and then reusing existing code that did not handle a missing manifest. Creating new methods that can handle the missing manifest allows us to remove the initial check. --- .github/actions/spelling/expect.txt | 1 + azure-pipelines.yml | 8 +- .../Workflows/InstallFlow.cpp | 2 + .../Workflows/SourceFlow.cpp | 6 + .../Workflows/SourceFlow.h | 6 + src/AppInstallerCLIE2ETests/Constants.cs | 1 + .../Interop/InstallInterop.cs | 67 +++++ .../AppInstallerCLITests.vcxproj | 1 + .../AppInstallerCLITests.vcxproj.filters | 3 + src/AppInstallerCLITests/ArpHelper.cpp | 71 ++++++ .../PredefinedInstalledSource.cpp | 81 ++++++ src/AppInstallerCLITests/SQLiteIndex.cpp | 39 +-- src/AppInstallerCLITests/TestHooks.h | 18 ++ src/AppInstallerCommonCore/Regex.cpp | 66 ++++- .../ArpVersionValidation.cpp | 100 ++++---- .../Microsoft/ARPHelper.cpp | 51 +++- .../Microsoft/ARPHelper.h | 5 + .../PredefinedInstalledSourceFactory.cpp | 237 +++++++++++++++--- .../PredefinedInstalledSourceFactory.h | 2 + .../Microsoft/SQLiteIndex.cpp | 19 +- .../Microsoft/SQLiteIndex.h | 17 +- .../Microsoft/SQLiteIndexSource.cpp | 77 +++++- .../Microsoft/Schema/1_0/Interface.h | 2 +- .../Microsoft/Schema/1_0/Interface_1_0.cpp | 91 +++---- .../Microsoft/Schema/1_0/ManifestTable.cpp | 30 ++- .../Microsoft/Schema/1_0/ManifestTable.h | 49 +++- .../Microsoft/Schema/1_3/Interface_1_3.cpp | 4 +- .../Microsoft/Schema/1_4/Interface_1_4.cpp | 2 +- .../Microsoft/Schema/1_5/Interface_1_5.cpp | 22 +- .../Microsoft/Schema/ISQLiteIndex.h | 11 +- .../PackageDependenciesValidation.cpp | 25 +- .../PackageTrackingCatalog.cpp | 7 +- .../Public/winget/RepositorySource.h | 3 + .../RepositorySource.cpp | 4 + src/AppInstallerRepositoryCore/pch.h | 1 + .../Public/AppInstallerVersions.h | 3 + .../Public/winget/Registry.h | 1 + .../Public/winget/SQLiteMetadataTable.h | 1 + .../Public/winget/SQLiteStorageBase.h | 5 + .../Public/winget/SQLiteWrapper.h | 22 ++ .../SQLiteStorageBase.cpp | 23 +- src/AppInstallerSharedLib/SQLiteWrapper.cpp | 46 +++- src/AppInstallerSharedLib/Versions.cpp | 12 +- 43 files changed, 1006 insertions(+), 236 deletions(-) create mode 100644 src/AppInstallerCLITests/ArpHelper.cpp diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 2916d7951c..8206c8bae5 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -131,6 +131,7 @@ ENDSESSION EPester epth EQU +errcode errmsg ERRORONEXIT ESource diff --git a/azure-pipelines.yml b/azure-pipelines.yml index c4aa4ba1f4..2a3389ee37 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -123,9 +123,11 @@ jobs: /p:UapAppxPackageBuildMode=SideloadOnly' - task: CopyFiles@2 - displayName: 'Copy WindowsPackageManager.dll Symbols to artifacts folder' + displayName: 'Copy Symbols to artifacts folder' inputs: - Contents: '$(buildOutDir)\WindowsPackageManager\WindowsPackageManager.pdb' + Contents: | + $(buildOutDir)\WindowsPackageManager\WindowsPackageManager.pdb + $(buildOutDir)\Microsoft.Management.Configuration\Microsoft.Management.Configuration.pdb TargetFolder: '$(artifactsDir)' condition: succeededOrFailed() @@ -306,7 +308,7 @@ jobs: platform: '$(buildPlatform)' configuration: '$(BuildConfiguration)' diagnosticsEnabled: true - condition: succeededOrFailed() + condition: and(succeededOrFailed(), eq(variables.buildPlatform, 'x86')) - task: PowerShell@2 displayName: Prepare for Microsoft.Management.Configuration.UnitTests (OutOfProc) diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index 9060ede725..3afb876ab8 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -16,6 +16,7 @@ #include "WorkflowBase.h" #include "DependenciesFlow.h" #include "PromptFlow.h" +#include "SourceFlow.h" #include #include #include @@ -553,6 +554,7 @@ namespace AppInstaller::CLI::Workflow Workflow::ReportExecutionStage(ExecutionStage::PostExecution) << Workflow::ReportARPChanges << Workflow::RecordInstall << + Workflow::ForceInstalledCacheUpdate << Workflow::RemoveInstaller << Workflow::DisplayInstallationNotes; } diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp index c0a9a6fb6c..6dbc951f87 100644 --- a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp @@ -301,4 +301,10 @@ namespace AppInstaller::CLI::Workflow } } } + + void ForceInstalledCacheUpdate(Execution::Context&) + { + // Creating this object is currently sufficient to mark the cache as needing an update for the next time it is opened. + Repository::Source ignore{ Repository::PredefinedSource::InstalledForceCacheUpdate }; + } } diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.h b/src/AppInstallerCLICore/Workflows/SourceFlow.h index fb1fee5e0d..f3d651148c 100644 --- a/src/AppInstallerCLICore/Workflows/SourceFlow.h +++ b/src/AppInstallerCLICore/Workflows/SourceFlow.h @@ -77,4 +77,10 @@ namespace AppInstaller::CLI::Workflow // Inputs: SourceList // Outputs: None void ExportSourceList(Execution::Context& context); + + // Forces an update to the cache of installed packages. + // Required Args: None + // Inputs: None + // Outputs: None + void ForceInstalledCacheUpdate(Execution::Context& context); } diff --git a/src/AppInstallerCLIE2ETests/Constants.cs b/src/AppInstallerCLIE2ETests/Constants.cs index ae7ac3f6a2..a578331548 100644 --- a/src/AppInstallerCLIE2ETests/Constants.cs +++ b/src/AppInstallerCLIE2ETests/Constants.cs @@ -82,6 +82,7 @@ public class Constants public const string PortableExePackageId = "AppInstallerTest.TestPortableExe"; public const string PortableExeWithCommandPackageId = "AppInstallerTest.TestPortableExeWithCommand"; + public const string ExeInstalledDefaultProductCode = "{A499DD5E-8DC5-4AD2-911A-BCD0263295E9}"; public const string MsiInstallerProductCode = "{A5D36CF1-1993-4F63-BFB4-3ACD910D36A1}"; public const string MsixInstallerName = "6c6338fe-41b7-46ca-8ba6-b5ad5312bb0e"; public const string MsixInstallerPackageFamilyName = "6c6338fe-41b7-46ca-8ba6-b5ad5312bb0e_8wekyb3d8bbwe"; diff --git a/src/AppInstallerCLIE2ETests/Interop/InstallInterop.cs b/src/AppInstallerCLIE2ETests/Interop/InstallInterop.cs index 4e3e284e82..72eea928ef 100644 --- a/src/AppInstallerCLIE2ETests/Interop/InstallInterop.cs +++ b/src/AppInstallerCLIE2ETests/Interop/InstallInterop.cs @@ -657,5 +657,72 @@ public void GetApplicableInstaller() Assert.AreEqual(PackageInstallerScope.User, packageInstallerInfo.Scope); Assert.AreEqual("en-US", packageInstallerInfo.Locale); } + + /// + /// Install exe and verify that we can find it as installed after. + /// + /// A representing the asynchronous unit test. + [Test] + public async Task InstallExe_VerifyInstalledCatalog() + { + var installedCatalogReference = this.packageManager.GetLocalPackageCatalog(LocalPackageCatalog.InstalledPackages); + + // Ensure package is not installed + var installedResult = this.FindAllPackages(installedCatalogReference, PackageMatchField.ProductCode, PackageFieldMatchOption.Equals, Constants.ExeInstalledDefaultProductCode); + Assert.IsNotNull(installedResult); + Assert.AreEqual(0, installedResult.Count); + + // Find package + var searchResult = this.FindOnePackage(this.testSource, PackageMatchField.Id, PackageFieldMatchOption.Equals, "AppInstallerTest.TestExeInstaller"); + + // Configure installation + var installOptions = this.TestFactory.CreateInstallOptions(); + installOptions.PackageInstallMode = PackageInstallMode.Silent; + installOptions.PreferredInstallLocation = this.installDir; + installOptions.AcceptPackageAgreements = true; + + // Install + var installResult = await this.packageManager.InstallPackageAsync(searchResult.CatalogPackage, installOptions); + + // Assert + Assert.AreEqual(InstallResultStatus.Ok, installResult.Status); + + // Check installed catalog after + this.FindOnePackage(installedCatalogReference, PackageMatchField.ProductCode, PackageFieldMatchOption.Equals, Constants.ExeInstalledDefaultProductCode); + + Assert.True(TestCommon.VerifyTestExeInstalledAndCleanup(this.installDir)); + } + + /// + /// Install msix and verify that we can find it as installed after. + /// + /// A representing the asynchronous unit test. + [Test] + public async Task InstallMSIX_VerifyInstalledCatalog() + { + var installedCatalogReference = this.packageManager.GetLocalPackageCatalog(LocalPackageCatalog.InstalledPackages); + + // Ensure package is not installed + var installedResult = this.FindAllPackages(installedCatalogReference, PackageMatchField.PackageFamilyName, PackageFieldMatchOption.Equals, Constants.MsixInstallerPackageFamilyName); + Assert.IsNotNull(installedResult); + Assert.AreEqual(0, installedResult.Count); + + // Find package + var searchResult = this.FindOnePackage(this.testSource, PackageMatchField.Name, PackageFieldMatchOption.Equals, "TestMsixInstaller"); + + // Configure installation + var installOptions = this.TestFactory.CreateInstallOptions(); + + // Install + var installResult = await this.packageManager.InstallPackageAsync(searchResult.CatalogPackage, installOptions); + + // Assert + Assert.AreEqual(InstallResultStatus.Ok, installResult.Status); + + // Check installed catalog after + this.FindOnePackage(installedCatalogReference, PackageMatchField.PackageFamilyName, PackageFieldMatchOption.Equals, Constants.MsixInstallerPackageFamilyName); + + Assert.True(TestCommon.VerifyTestMsixInstalledAndCleanup()); + } } } \ No newline at end of file diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj index 301a25001b..165bb05fcb 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -197,6 +197,7 @@ + diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters index 0f1bc81a88..c7cd69655c 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -326,6 +326,9 @@ Source Files\Common + + Source Files\Repository + diff --git a/src/AppInstallerCLITests/ArpHelper.cpp b/src/AppInstallerCLITests/ArpHelper.cpp new file mode 100644 index 0000000000..fdbfd2cb4c --- /dev/null +++ b/src/AppInstallerCLITests/ArpHelper.cpp @@ -0,0 +1,71 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "TestCommon.h" +#include "TestHooks.h" +#include "Microsoft/ARPHelper.h" +#include "AppInstallerStrings.h" + +using namespace AppInstaller::Manifest; +using namespace AppInstaller::Repository::Microsoft; +using namespace AppInstaller::Utility; +using namespace AppInstaller::Registry; + + +TEST_CASE("ARPHelper_Watcher", "[ARPHelper]") +{ + ARPHelper helper; + + wil::unique_event callbackEvent; + callbackEvent.create(); + + ScopeEnum scopeCallback = ScopeEnum::Unknown; + Architecture architectureCallback = Architecture::Unknown; + + ScopeEnum scopeTarget = ScopeEnum::Machine; + Architecture architectureTarget = Architecture::X86; + + auto fakeRoot = TestCommon::RegCreateVolatileTestRoot(); + TestHook::SetGetARPKey_Override arpOverride([&](ScopeEnum scope, Architecture arch) + { + if (scope == scopeTarget && arch == architectureTarget) + { + return Key(fakeRoot.get(), L""); + } + else + { + return Key{}; + } + }); + + auto watchers = helper.CreateRegistryWatchers(scopeTarget, [&](ScopeEnum scope, Architecture arch, wil::RegistryChangeKind) + { + scopeCallback = scope; + architectureCallback = arch; + callbackEvent.SetEvent(); + }); + + auto arpKey = helper.GetARPKey(scopeTarget, architectureTarget); + REQUIRE(!!arpKey); + + GUID guid; + std::ignore = CoCreateGuid(&guid); + std::ostringstream stream; + stream << guid; + + auto testKey = TestCommon::RegCreateVolatileSubKey(arpKey, ConvertToUTF16(stream.str())); + + REQUIRE(callbackEvent.wait(1000)); + REQUIRE(scopeTarget == scopeCallback); + REQUIRE(architectureTarget == architectureCallback); + + // Reset for changing a value + scopeCallback = ScopeEnum::Unknown; + architectureCallback = Architecture::Unknown; + + TestCommon::SetRegistryValue(testKey.get(), L"testValue", L"valueValue"); + + REQUIRE(callbackEvent.wait(1000)); + REQUIRE(scopeTarget == scopeCallback); + REQUIRE(architectureTarget == architectureCallback); +} diff --git a/src/AppInstallerCLITests/PredefinedInstalledSource.cpp b/src/AppInstallerCLITests/PredefinedInstalledSource.cpp index cfdd731edb..23efc240bf 100644 --- a/src/AppInstallerCLITests/PredefinedInstalledSource.cpp +++ b/src/AppInstallerCLITests/PredefinedInstalledSource.cpp @@ -7,6 +7,7 @@ #include #include #include +#include using namespace std::string_literals; using namespace std::string_view_literals; @@ -18,6 +19,7 @@ using namespace AppInstaller::Runtime; using namespace AppInstaller::Utility; using SQLiteIndex = AppInstaller::Repository::Microsoft::SQLiteIndex; +using SQLiteIndexSource = AppInstaller::Repository::Microsoft::SQLiteIndexSource; using Factory = AppInstaller::Repository::Microsoft::PredefinedInstalledSourceFactory; using ARPHelper = AppInstaller::Repository::Microsoft::ARPHelper; @@ -400,3 +402,82 @@ TEST_CASE("PredefinedInstalledSource_Search", "[installed][list]") REQUIRE_FALSE(results.Matches.empty()); } + +std::string GetDatabaseIdentifier(const std::shared_ptr& source) +{ + return reinterpret_cast(source->CastTo(ISourceType::SQLiteIndexSource))->GetIndex().GetDatabaseIdentifier(); +} + +void RequirePackagesHaveSameNames(std::shared_ptr& source1, std::shared_ptr& source2) +{ + auto result1 = source1->Search({}); + REQUIRE(!result1.Matches.empty()); + + // Ensure that all packages have the same name values + for (const auto& match : result1.Matches) + { + std::string packageId = match.Package->GetProperty(PackageProperty::Id).get(); + INFO(packageId); + + SearchRequest id2; + id2.Inclusions.emplace_back(PackageMatchFilter{ PackageMatchField::Id, MatchType::CaseInsensitive, packageId }); + auto result2 = source2->Search(id2); + REQUIRE(result2.Matches.size() == 1); + REQUIRE(match.Package->GetProperty(PackageProperty::Name) == result2.Matches[0].Package->GetProperty(PackageProperty::Name)); + } +} + +TEST_CASE("PredefinedInstalledSource_Create_Cached", "[installed][list][installed-cache]") +{ + auto source1 = CreatePredefinedInstalledSource(); + auto source2 = CreatePredefinedInstalledSource(); + + // Ensure the same identifier (which should mean the cache was not updated) + REQUIRE( + GetDatabaseIdentifier(source1) + == + GetDatabaseIdentifier(source2) + ); + + RequirePackagesHaveSameNames(source1, source2); + RequirePackagesHaveSameNames(source2, source1); +} + +TEST_CASE("PredefinedInstalledSource_Create_ForceCacheUpdate", "[installed][list][installed-cache]") +{ + auto source1 = CreatePredefinedInstalledSource(); + auto source2 = CreatePredefinedInstalledSource(Factory::Filter::NoneWithForcedCacheUpdate); + + // Ensure different identifier (which should mean the cache was updated) + REQUIRE( + GetDatabaseIdentifier(source1) + != + GetDatabaseIdentifier(source2) + ); + + RequirePackagesHaveSameNames(source1, source2); + RequirePackagesHaveSameNames(source2, source1); +} + +TEST_CASE("PredefinedInstalledSource_Create_ForceCacheUpdate_StillCached", "[installed][list][installed-cache]") +{ + auto source1 = CreatePredefinedInstalledSource(); + auto source2 = CreatePredefinedInstalledSource(Factory::Filter::NoneWithForcedCacheUpdate); + auto source3 = CreatePredefinedInstalledSource(); + + CAPTURE(GetDatabaseIdentifier(source1), GetDatabaseIdentifier(source2), GetDatabaseIdentifier(source3)); + + // Ensure different identifier (which should mean the cache was updated) + REQUIRE( + GetDatabaseIdentifier(source1) + != + GetDatabaseIdentifier(source2) + ); + + // Ensure the same identifier (which should mean the cache was not updated) + REQUIRE( + GetDatabaseIdentifier(source2) + == + GetDatabaseIdentifier(source3) + ); +} diff --git a/src/AppInstallerCLITests/SQLiteIndex.cpp b/src/AppInstallerCLITests/SQLiteIndex.cpp index f8b9112b1c..35c61695e9 100644 --- a/src/AppInstallerCLITests/SQLiteIndex.cpp +++ b/src/AppInstallerCLITests/SQLiteIndex.cpp @@ -339,6 +339,13 @@ bool IsMapDataFolded(const SQLiteIndex& index) return (index.GetVersion() >= SQLiteVersion{ 1, 7 }); } +std::string GetPropertyStringByKey(const SQLiteIndex& index, rowid_t manifestId, PackageVersionProperty property) +{ + auto result = index.GetPropertyByManifestId(manifestId, property); + REQUIRE(result); + return result.value(); +} + std::string GetPropertyStringByKey(const SQLiteIndex& index, rowid_t id, PackageVersionProperty property, std::string_view version, std::string_view channel) { auto manifestId = index.GetManifestIdByKey(id, version, channel); @@ -352,7 +359,7 @@ std::string GetPropertyStringById(const SQLiteIndex& index, rowid_t id, PackageV { auto versions = index.GetVersionKeysById(id); REQUIRE(!versions.empty()); - return GetPropertyStringByKey(index, id, property, versions[0].GetVersion().ToString(), versions[0].GetChannel().ToString()); + return GetPropertyStringByKey(index, versions[0].ManifestId, property); } std::string GetIdStringById(const SQLiteIndex& index, rowid_t id) @@ -1839,8 +1846,8 @@ TEST_CASE("SQLiteIndex_Versions", "[sqliteindex]") auto result = index.GetVersionKeysById(results.Matches[0].first); REQUIRE(result.size() == 1); - REQUIRE(result[0].GetVersion().ToString() == manifest.Version); - REQUIRE(result[0].GetChannel().ToString() == manifest.Channel); + REQUIRE(result[0].VersionAndChannel.GetVersion().ToString() == manifest.Version); + REQUIRE(result[0].VersionAndChannel.GetChannel().ToString() == manifest.Channel); } TEST_CASE("SQLiteIndex_Search_VersionSorting", "[sqliteindex]") @@ -1885,7 +1892,7 @@ TEST_CASE("SQLiteIndex_Search_VersionSorting", "[sqliteindex]") for (size_t i = 0; i < result.size(); ++i) { const VersionAndChannel& sortedVAC = sortedList[i]; - const VersionAndChannel& resultVAC = result[i]; + const VersionAndChannel& resultVAC = result[i].VersionAndChannel; INFO(i); REQUIRE(sortedVAC.GetVersion().ToString() == resultVAC.GetVersion().ToString()); @@ -1973,7 +1980,7 @@ TEST_CASE("SQLiteIndex_PathString_CaseInsensitive", "[sqliteindex]") REQUIRE(result.has_value()); result = index.GetManifestIdByKey(results.Matches[0].first, "13.2.0-BugFix", "BETA"); - REQUIRE(!result.has_value()); + REQUIRE(result.has_value()); } TEST_CASE("SQLiteIndex_SearchResultsTableSearches", "[sqliteindex][V1_0]") @@ -3270,14 +3277,11 @@ TEST_CASE("SQLiteIndex_MapDataFolding_PFNs", "[sqliteindex][mapdatafolding]") auto versionKeys = index.GetVersionKeysById(results1.Matches[0].first); REQUIRE(versionKeys.size() == 2); - auto manifestId1 = index.GetManifestIdByKey(results1.Matches[0].first, versionKeys[0].GetVersion().ToString(), versionKeys[0].GetChannel().ToString()); - auto manifestId2 = index.GetManifestIdByKey(results1.Matches[0].first, versionKeys[1].GetVersion().ToString(), versionKeys[1].GetChannel().ToString()); + auto manifestId1 = versionKeys[0].ManifestId; + auto manifestId2 = versionKeys[1].ManifestId; - REQUIRE(manifestId1.has_value()); - REQUIRE(manifestId2.has_value()); - - auto pfnValues1 = index.GetMultiPropertyByManifestId(manifestId1.value(), PackageVersionMultiProperty::PackageFamilyName); - auto pfnValues2 = index.GetMultiPropertyByManifestId(manifestId2.value(), PackageVersionMultiProperty::PackageFamilyName); + auto pfnValues1 = index.GetMultiPropertyByManifestId(manifestId1, PackageVersionMultiProperty::PackageFamilyName); + auto pfnValues2 = index.GetMultiPropertyByManifestId(manifestId2, PackageVersionMultiProperty::PackageFamilyName); if (IsMapDataFoldingSupported(index, testVersion)) { @@ -3341,14 +3345,11 @@ TEST_CASE("SQLiteIndex_MapDataFolding_ProductCodes", "[sqliteindex][mapdatafoldi auto versionKeys = index.GetVersionKeysById(results1.Matches[0].first); REQUIRE(versionKeys.size() == 2); - auto manifestId1 = index.GetManifestIdByKey(results1.Matches[0].first, versionKeys[0].GetVersion().ToString(), versionKeys[0].GetChannel().ToString()); - auto manifestId2 = index.GetManifestIdByKey(results1.Matches[0].first, versionKeys[1].GetVersion().ToString(), versionKeys[1].GetChannel().ToString()); - - REQUIRE(manifestId1.has_value()); - REQUIRE(manifestId2.has_value()); + auto manifestId1 = versionKeys[0].ManifestId; + auto manifestId2 = versionKeys[1].ManifestId; - auto pcValues1 = index.GetMultiPropertyByManifestId(manifestId1.value(), PackageVersionMultiProperty::ProductCode); - auto pcValues2 = index.GetMultiPropertyByManifestId(manifestId2.value(), PackageVersionMultiProperty::ProductCode); + auto pcValues1 = index.GetMultiPropertyByManifestId(manifestId1, PackageVersionMultiProperty::ProductCode); + auto pcValues2 = index.GetMultiPropertyByManifestId(manifestId2, PackageVersionMultiProperty::ProductCode); REQUIRE(pcValues1.size() == 1); REQUIRE(pcValues2.size() == 1); diff --git a/src/AppInstallerCLITests/TestHooks.h b/src/AppInstallerCLITests/TestHooks.h index c0085bcb4f..78317d0170 100644 --- a/src/AppInstallerCLITests/TestHooks.h +++ b/src/AppInstallerCLITests/TestHooks.h @@ -39,6 +39,9 @@ namespace AppInstaller namespace Repository::Microsoft { void TestHook_SetPinningIndex_Override(std::optional&& indexPath); + + using GetARPKeyFunc = std::function; + void SetGetARPKeyOverride(GetARPKeyFunc value); } namespace Logging @@ -177,4 +180,19 @@ namespace TestHook private: bool m_status; }; + + struct SetGetARPKey_Override + { + SetGetARPKey_Override(std::function function) + { + AppInstaller::Repository::Microsoft::SetGetARPKeyOverride(function); + } + + ~SetGetARPKey_Override() + { + AppInstaller::Repository::Microsoft::SetGetARPKeyOverride({}); + } + + private: + }; } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Regex.cpp b/src/AppInstallerCommonCore/Regex.cpp index 74d4a0d12b..934be59b39 100644 --- a/src/AppInstallerCommonCore/Regex.cpp +++ b/src/AppInstallerCommonCore/Regex.cpp @@ -4,6 +4,7 @@ #include "Public/winget/Regex.h" #include "Public/AppInstallerErrors.h" #include "Public/AppInstallerLogging.h" +#include "Public/AppInstallerLanguageUtilities.h" #define WINGET_THROW_REGEX_ERROR_IF_FAILED(_err_,_func_) \ if (U_FAILURE(_err_)) \ @@ -20,6 +21,69 @@ namespace AppInstaller::Regex using uregex_ptr = wil::unique_any; using utext_ptr = wil::unique_any; + // Create caches the original ICU regex objects in a static map and hands out copies of them + // when requested. Since we have a limited set, this is a very simple cache-all-forever pattern. + static std::unique_ptr Create(std::string_view pattern, Options options) + { + struct key + { + std::string pattern; + Options options = Options::None; + + bool operator<(const key& other) const + { + if (pattern < other.pattern) + { + return true; + } + else if (pattern == other.pattern) + { + return ToIntegral(options) < ToIntegral(other.options); + } + else + { + return false; + } + } + }; + + struct statics + { + std::map map; + wil::srwlock lock; + }; + + static statics s_regex_cache; + + key requested; + requested.pattern = pattern; + requested.options = options; + + { + // Attempt to find in the cache + auto sharedLock = s_regex_cache.lock.lock_shared(); + + auto itr = s_regex_cache.map.find(requested); + if (itr != s_regex_cache.map.end()) + { + return std::make_unique(itr->second); + } + } + + auto exclusiveLock = s_regex_cache.lock.lock_exclusive(); + + // Check if another thread created it while we waited for the lock. + auto itr = s_regex_cache.map.find(requested); + if (itr != s_regex_cache.map.end()) + { + return std::make_unique(itr->second); + } + else + { + return std::make_unique(s_regex_cache.map.emplace(std::move(requested), impl{ pattern, options }).first->second); + } + } + impl(std::string_view pattern, Options options) { UErrorCode uec = U_ZERO_ERROR; @@ -167,7 +231,7 @@ namespace AppInstaller::Regex Expression::Expression() = default; - Expression::Expression(std::string_view pattern, Options options) : pImpl(std::make_unique(pattern, options)) {} + Expression::Expression(std::string_view pattern, Options options) : pImpl(impl::Create(pattern, options)) {} Expression::Expression(const Expression& other) { diff --git a/src/AppInstallerRepositoryCore/ArpVersionValidation.cpp b/src/AppInstallerRepositoryCore/ArpVersionValidation.cpp index 8f567750de..cbff934dd7 100644 --- a/src/AppInstallerRepositoryCore/ArpVersionValidation.cpp +++ b/src/AppInstallerRepositoryCore/ArpVersionValidation.cpp @@ -1,49 +1,45 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. -#pragma once -#include "pch.h" -#include "ArpVersionValidation.h" -#include - -namespace AppInstaller::Repository -{ - namespace - { - std::vector GetArpVersionRangesByPackageRowId(const Microsoft::SQLiteIndex* index, Microsoft::SQLiteIndex::IdType packageRowId, const Utility::VersionAndChannel& excludeVersionAndChannel = {}) - { - std::vector result; - - auto versionKeys = index->GetVersionKeysById(packageRowId); - for (auto const& versionKey : versionKeys) - { - // For manifest update, the manifest to be updated does not need to be checked. - // In unlikely cases if both version 1.0.0 and 1.0 of the same package exist, we compare raw values here as what sqlite index does. - if (versionKey.GetVersion().ToString() == excludeVersionAndChannel.GetVersion().ToString() && - versionKey.GetChannel().ToString() == excludeVersionAndChannel.GetChannel().ToString()) - { - continue; - } - - std::optional manifestRowId = index->GetManifestIdByKey(packageRowId, versionKey.GetVersion().ToString(), versionKey.GetChannel().ToString()); - if (manifestRowId) +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "pch.h" +#include "ArpVersionValidation.h" +#include + +namespace AppInstaller::Repository +{ + namespace + { + std::vector GetArpVersionRangesByPackageRowId(const Microsoft::SQLiteIndex* index, Microsoft::SQLiteIndex::IdType packageRowId, const Utility::VersionAndChannel& excludeVersionAndChannel = {}) + { + std::vector result; + + auto versionKeys = index->GetVersionKeysById(packageRowId); + for (auto const& versionKey : versionKeys) + { + // For manifest update, the manifest to be updated does not need to be checked. + // In unlikely cases if both version 1.0.0 and 1.0 of the same package exist, we compare raw values here as what sqlite index does. + if (versionKey.VersionAndChannel.GetVersion().ToString() == excludeVersionAndChannel.GetVersion().ToString() && + versionKey.VersionAndChannel.GetChannel().ToString() == excludeVersionAndChannel.GetChannel().ToString()) { - auto arpMinVersion = index->GetPropertyByManifestId(manifestRowId.value(), PackageVersionProperty::ArpMinVersion).value_or(""); - auto arpMaxVersion = index->GetPropertyByManifestId(manifestRowId.value(), PackageVersionProperty::ArpMaxVersion).value_or(""); + continue; + } + + auto arpMinVersion = index->GetPropertyByManifestId(versionKey.ManifestId, PackageVersionProperty::ArpMinVersion).value_or(""); + auto arpMaxVersion = index->GetPropertyByManifestId(versionKey.ManifestId, PackageVersionProperty::ArpMaxVersion).value_or(""); - // Either both empty or both not empty - THROW_HR_IF(E_UNEXPECTED, arpMinVersion.empty() != arpMaxVersion.empty()); + // Either both empty or both not empty + THROW_HR_IF(E_UNEXPECTED, arpMinVersion.empty() != arpMaxVersion.empty()); + + if (!arpMinVersion.empty() && !arpMaxVersion.empty()) + { + result.emplace_back(Utility::VersionRange{ Utility::Version{ std::move(arpMinVersion) }, Utility::Version{ std::move(arpMaxVersion) } }); + } + } + + return result; + } + } - if (!arpMinVersion.empty() && !arpMaxVersion.empty()) - { - result.emplace_back(Utility::VersionRange{ Utility::Version{ std::move(arpMinVersion) }, Utility::Version{ std::move(arpMaxVersion) } }); - } - } - } - - return result; - } - } - void ValidateManifestArpVersion(const Microsoft::SQLiteIndex* index, const Manifest::Manifest& manifest) { try @@ -54,14 +50,14 @@ namespace AppInstaller::Repository return; } - SearchRequest request; - request.Filters.emplace_back(PackageMatchField::Id, MatchType::CaseInsensitive, manifest.Id); - auto searchResult = index->Search(request); - if (searchResult.Matches.empty()) - { - return; - } - + SearchRequest request; + request.Filters.emplace_back(PackageMatchField::Id, MatchType::CaseInsensitive, manifest.Id); + auto searchResult = index->Search(request); + if (searchResult.Matches.empty()) + { + return; + } + auto arpVersionRangesInIndex = GetArpVersionRangesByPackageRowId(index, searchResult.Matches[0].first, { Utility::Version{ manifest.Version }, Utility::Channel{ manifest.Channel } }); for (auto const& arpInIndex : arpVersionRangesInIndex) { @@ -89,5 +85,5 @@ namespace AppInstaller::Repository { Manifest::ValidationError(Manifest::ManifestError::ArpVersionValidationInternalError) }, APPINSTALLER_CLI_ERROR_DEPENDENCIES_VALIDATION_FAILED)); } - } + } } \ No newline at end of file diff --git a/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp b/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp index 26a60f56a1..5bb9ef32ed 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp @@ -108,15 +108,32 @@ namespace AppInstaller::Repository::Microsoft CATCH_LOG_MSG("Failed to read upgrade code: %hs", keyName.c_str()); } } - } + } CATCH_LOG_MSG("Failed to read upgrade codes."); return upgradeCodes; } } +#ifndef AICLI_DISABLE_TEST_HOOKS + using GetARPKeyFunc = std::function; + static GetARPKeyFunc s_GetARPKey_Override; + + void SetGetARPKeyOverride(GetARPKeyFunc value) + { + s_GetARPKey_Override = value; + } +#endif + Registry::Key ARPHelper::GetARPKey(Manifest::ScopeEnum scope, Utility::Architecture architecture) const { +#ifndef AICLI_DISABLE_TEST_HOOKS + if (s_GetARPKey_Override) + { + return s_GetARPKey_Override(scope, architecture); + } +#endif + HKEY rootKey = NULL; switch (scope) @@ -368,7 +385,7 @@ namespace AppInstaller::Repository::Microsoft void ARPHelper::PopulateIndexFromKey(SQLiteIndex& index, const Registry::Key& key, std::string_view scope, std::string_view architecture, const std::map& upgradeCodes) const { - AICLI_LOG(Repo, Info, << "Examining ARP entries for " << scope << " | " << architecture); + AICLI_LOG(Repo, Verbose, << "Examining ARP entries for " << scope << " | " << architecture); for (const auto& arpEntry : key) { @@ -532,4 +549,34 @@ namespace AppInstaller::Repository::Microsoft } } } + + std::vector ARPHelper::CreateRegistryWatchers(Manifest::ScopeEnum scope, std::function callback) + { + std::vector result; + + auto addToResult = [&](Manifest::ScopeEnum scopeToUse) + { + for (auto architecture : Utility::GetApplicableArchitectures()) + { + Registry::Key arpRootKey = GetARPKey(scopeToUse, architecture); + + if (arpRootKey) + { + result.emplace_back(wil::make_registry_watcher(arpRootKey, L"", true, [scopeToUse, architecture, callback](wil::RegistryChangeKind change) { callback(scopeToUse, architecture, change); })); + } + } + }; + + if (scope == Manifest::ScopeEnum::Unknown) + { + addToResult(Manifest::ScopeEnum::User); + addToResult(Manifest::ScopeEnum::Machine); + } + else + { + addToResult(scope); + } + + return result; + } } diff --git a/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.h b/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.h index 3c0b4ac77b..e0afa19924 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.h +++ b/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.h @@ -5,9 +5,11 @@ #include #include #include +#include #include #include +#include namespace AppInstaller::Repository::Microsoft { @@ -86,5 +88,8 @@ namespace AppInstaller::Repository::Microsoft // This entry point is primarily to allow unit tests to operate of arbitrary keys; // product code should use PopulateIndexFromARP. void PopulateIndexFromKey(SQLiteIndex& index, const Registry::Key& key, std::string_view scope, std::string_view architecture, const std::map& upgradeCodes = {}) const; + + // Creates registry watchers for the given scope + std::vector CreateRegistryWatchers(Manifest::ScopeEnum scope, std::function callback); }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp index d3c6740cb6..84d09a8cad 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp @@ -17,13 +17,54 @@ namespace AppInstaller::Repository::Microsoft { namespace { + std::optional GetCachedMSIXName(const Utility::NormalizedString& id, const Utility::Version& version, SQLiteIndex& cacheData) + { + SearchRequest searchRequest; + searchRequest.Inclusions.emplace_back(PackageMatchField::Id, MatchType::Exact, id); + + SQLiteIndex::SearchResult searchResult = cacheData.Search(searchRequest); + + if (searchResult.Matches.empty()) + { + return std::nullopt; + } + + if (searchResult.Matches.size() != 1) + { + // This is very unexpected, but just log it and carry on + AICLI_LOG(Repo, Warning, << "Found multiple (" << searchResult.Matches.size() << ") cache entries for: " << id); + return std::nullopt; + } + + auto versionKeys = cacheData.GetVersionKeysById(searchResult.Matches[0].first); + const SQLiteIndex::VersionKey* versionKey = nullptr; + + for (const auto& key : versionKeys) + { + if (key.VersionAndChannel.GetVersion() == version) + { + versionKey = &key; + break; + } + } + + if (!versionKey) + { + return std::nullopt; + } + + return cacheData.GetPropertyByManifestId(versionKey->ManifestId, PackageVersionProperty::Name); + } + // Populates the index with the entries from MSIX. - void PopulateIndexFromMSIX(SQLiteIndex& index, Manifest::ScopeEnum scope) + void PopulateIndexFromMSIX(SQLiteIndex& index, Manifest::ScopeEnum scope, SQLiteIndex* cacheData = nullptr) { using namespace winrt::Windows::ApplicationModel; using namespace winrt::Windows::Management::Deployment; using namespace winrt::Windows::Foundation::Collections; + AICLI_LOG(Repo, Verbose, << "Examining MSIX entries for " << ScopeToString(scope)); + IIterable packages; PackageManager packageManager; if (scope == Manifest::ScopeEnum::Machine) @@ -63,10 +104,31 @@ namespace AppInstaller::Repository::Microsoft manifest.Id = familyName; + // Get version + std::ostringstream strstr; + auto packageVersion = packageId.Version(); + strstr << packageVersion.Major << '.' << packageVersion.Minor << '.' << packageVersion.Build << '.' << packageVersion.Revision; + + manifest.Version = strstr.str(); + + // Determine package name bool isPackageNameSet = false; + + // Look for the name in the cache data first + if (cacheData) + { + std::optional cachedName = GetCachedMSIXName(manifest.Id, manifest.Version, *cacheData); + + if (cachedName) + { + manifest.DefaultLocalization.Add(cachedName.value()); + isPackageNameSet = true; + } + } + // Attempt to get the DisplayName. Since this will retrieve the localized value, it has a chance to fail. // Rather than completely skip this package in that case, we will simply fall back to using the package name below. - if (!Runtime::IsRunningAsSystem()) + if (!isPackageNameSet && !Runtime::IsRunningAsSystem()) { try { @@ -79,12 +141,12 @@ namespace AppInstaller::Repository::Microsoft } catch (const winrt::hresult_error& hre) { - AICLI_LOG(Repo, Info, << "winrt::hresult_error[0x" << Logging::SetHRFormat << hre.code() << ": " << + AICLI_LOG(Repo, Warning, << "winrt::hresult_error[0x" << Logging::SetHRFormat << hre.code() << ": " << Utility::ConvertToUTF8(hre.message()) << "] exception thrown when getting DisplayName for " << familyName); } catch (...) { - AICLI_LOG(Repo, Info, << "Unknown exception thrown when getting DisplayName for " << familyName); + AICLI_LOG(Repo, Warning, << "Unknown exception thrown when getting DisplayName for " << familyName); } } @@ -93,12 +155,6 @@ namespace AppInstaller::Repository::Microsoft manifest.DefaultLocalization.Add(Utility::ConvertToUTF8(packageId.Name())); } - std::ostringstream strstr; - auto packageVersion = packageId.Version(); - strstr << packageVersion.Major << '.' << packageVersion.Minor << '.' << packageVersion.Build << '.' << packageVersion.Revision; - - manifest.Version = strstr.str(); - manifest.Installers[0].PackageFamilyName = familyName; // Use the full name as a unique key for the path @@ -109,11 +165,126 @@ namespace AppInstaller::Repository::Microsoft } } + SQLiteIndex CreateAndPopulateIndex(PredefinedInstalledSourceFactory::Filter filter) + { + AICLI_LOG(Repo, Verbose, << "Creating PredefinedInstalledSource with filter [" << PredefinedInstalledSourceFactory::FilterToString(filter) << ']'); + + // Create an in memory index + SQLiteIndex index = SQLiteIndex::CreateNew(SQLITE_MEMORY_DB_CONNECTION_TARGET, SQLite::Version::Latest()); + + // Put installed packages into the index + if (filter == PredefinedInstalledSourceFactory::Filter::None || filter == PredefinedInstalledSourceFactory::Filter::ARP || + filter == PredefinedInstalledSourceFactory::Filter::User || filter == PredefinedInstalledSourceFactory::Filter::Machine) + { + ARPHelper arpHelper; + if (filter != PredefinedInstalledSourceFactory::Filter::User) + { + arpHelper.PopulateIndexFromARP(index, Manifest::ScopeEnum::Machine); + } + if (filter != PredefinedInstalledSourceFactory::Filter::Machine) + { + arpHelper.PopulateIndexFromARP(index, Manifest::ScopeEnum::User); + } + } + + if (filter == PredefinedInstalledSourceFactory::Filter::None || + filter == PredefinedInstalledSourceFactory::Filter::MSIX || + filter == PredefinedInstalledSourceFactory::Filter::User) + { + PopulateIndexFromMSIX(index, Manifest::ScopeEnum::User); + } + else if (filter == PredefinedInstalledSourceFactory::Filter::Machine) + { + PopulateIndexFromMSIX(index, Manifest::ScopeEnum::Machine); + } + + AICLI_LOG(Repo, Verbose, << " ... finished creating PredefinedInstalledSource"); + + return index; + } + + struct CachedInstalledIndex + { + CachedInstalledIndex() + { + ARPHelper arpHelper; + m_registryWatchers = arpHelper.CreateRegistryWatchers(Manifest::ScopeEnum::Unknown, + [this](Manifest::ScopeEnum, Utility::Architecture, wil::RegistryChangeKind) { ForceNextUpdate(); }); + + m_catalog = winrt::Windows::ApplicationModel::PackageCatalog::OpenForCurrentUser(); + m_eventRevoker = m_catalog.PackageStatusChanged(winrt::auto_revoke, [this](auto...) { ForceNextUpdate(); }); + } + + void UpdateIndexIfNeeded() + { + auto sharedLock = m_lock.lock_shared(); + if (CheckForUpdate()) + { + // Upgrade to exclusive + sharedLock.reset(); + auto exclusiveLock = m_lock.lock_exclusive(); + + if (CheckForUpdate()) + { + // TODO: To support servicing, the initial implementation of update will simply leverage + // some data from the existing index to speed up the MSIX populate function. + // In a larger update, we may want to make it possible to actually update the cache directly. + // We may even persist the cache to disk to speed things up further. + + // Set the update indicator to false before we start reading so that an external change can + // reindicate a need to update in the middle. But in the event that we error here, set it back to true + // to prevent an error from blocking further attempts. + m_forceNextUpdate = false; + auto scopeExit = wil::scope_exit([&]() { m_forceNextUpdate = true; }); + + // Populate from ARP using standard mechanism. + SQLiteIndex update = CreateAndPopulateIndex(PredefinedInstalledSourceFactory::Filter::ARP); + + // Populate from MSIX, using localization data from the existing index if applicable. + PopulateIndexFromMSIX(update, Manifest::ScopeEnum::User, m_index.get()); + + m_index = std::make_unique(std::move(update)); + scopeExit.release(); + } + } + } + + SQLiteIndex GetCopy() + { + auto lock = m_lock.lock_shared(); + THROW_HR_IF(E_POINTER, !m_index); + return SQLiteIndex::CopyFrom(SQLITE_MEMORY_DB_CONNECTION_TARGET, *m_index); + } + + void ForceNextUpdate() + { + m_forceNextUpdate = true; + } + + private: + bool CheckForUpdate() + { + return (!m_index || m_forceNextUpdate.load()); + } + + wil::srwlock m_lock; + std::atomic_bool m_forceNextUpdate{ false }; + std::unique_ptr m_index; + std::vector m_registryWatchers; + winrt::Windows::ApplicationModel::PackageCatalog m_catalog = nullptr; + decltype(winrt::Windows::ApplicationModel::PackageCatalog{ nullptr }.PackageStatusChanged(winrt::auto_revoke, nullptr)) m_eventRevoker; + }; + struct PredefinedInstalledSourceReference : public ISourceReference { PredefinedInstalledSourceReference(const SourceDetails& details) : m_details(details) { m_details.Identifier = "*PredefinedInstalledSource"; + + if (PredefinedInstalledSourceFactory::StringToFilter(m_details.Arg) == PredefinedInstalledSourceFactory::Filter::NoneWithForcedCacheUpdate) + { + GetCachedInstalledIndex().ForceNextUpdate(); + } } std::string GetIdentifier() override { return m_details.Identifier; } @@ -127,41 +298,27 @@ namespace AppInstaller::Repository::Microsoft // Determine the filter PredefinedInstalledSourceFactory::Filter filter = PredefinedInstalledSourceFactory::StringToFilter(m_details.Arg); - AICLI_LOG(Repo, Info, << "Creating PredefinedInstalledSource with filter [" << PredefinedInstalledSourceFactory::FilterToString(filter) << ']'); - - // Create an in memory index - SQLiteIndex index = SQLiteIndex::CreateNew(SQLITE_MEMORY_DB_CONNECTION_TARGET, SQLite::Version::Latest()); - // Put installed packages into the index - if (filter == PredefinedInstalledSourceFactory::Filter::None || filter == PredefinedInstalledSourceFactory::Filter::ARP || - filter == PredefinedInstalledSourceFactory::Filter::User || filter == PredefinedInstalledSourceFactory::Filter::Machine) + // Only cache for the unfiltered install data + if (filter == PredefinedInstalledSourceFactory::Filter::None || filter == PredefinedInstalledSourceFactory::Filter::NoneWithForcedCacheUpdate) { - ARPHelper arpHelper; - if (filter != PredefinedInstalledSourceFactory::Filter::User) - { - arpHelper.PopulateIndexFromARP(index, Manifest::ScopeEnum::Machine); - } - if (filter != PredefinedInstalledSourceFactory::Filter::Machine) - { - arpHelper.PopulateIndexFromARP(index, Manifest::ScopeEnum::User); - } - } - - if (filter == PredefinedInstalledSourceFactory::Filter::None || - filter == PredefinedInstalledSourceFactory::Filter::MSIX || - filter == PredefinedInstalledSourceFactory::Filter::User) - { - PopulateIndexFromMSIX(index, Manifest::ScopeEnum::User); + CachedInstalledIndex& cachedIndex = GetCachedInstalledIndex(); + cachedIndex.UpdateIndexIfNeeded(); + return std::make_shared(m_details, cachedIndex.GetCopy(), true); } - else if (filter == PredefinedInstalledSourceFactory::Filter::Machine) + else { - PopulateIndexFromMSIX(index, Manifest::ScopeEnum::Machine); + return std::make_shared(m_details, CreateAndPopulateIndex(filter), true); } - - return std::make_shared(m_details, std::move(index), true); } private: + CachedInstalledIndex& GetCachedInstalledIndex() + { + static CachedInstalledIndex s_installedIndex; + return s_installedIndex; + } + SourceDetails m_details; }; @@ -214,6 +371,8 @@ namespace AppInstaller::Repository::Microsoft return "User"sv; case AppInstaller::Repository::Microsoft::PredefinedInstalledSourceFactory::Filter::Machine: return "Machine"sv; + case AppInstaller::Repository::Microsoft::PredefinedInstalledSourceFactory::Filter::NoneWithForcedCacheUpdate: + return "NoneWithForcedCacheUpdate"sv; default: return "Unknown"sv; } @@ -237,6 +396,10 @@ namespace AppInstaller::Repository::Microsoft { return Filter::Machine; } + else if (filter == FilterToString(Filter::NoneWithForcedCacheUpdate)) + { + return Filter::NoneWithForcedCacheUpdate; + } else { return Filter::None; diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.h b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.h index 3a6bcc216f..8c10648be2 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.h +++ b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.h @@ -34,6 +34,8 @@ namespace AppInstaller::Repository::Microsoft User, // Contains machine ARP and machine MSIX Machine, + // Same as None but creating the source reference causes the next Open to always update the cache + NoneWithForcedCacheUpdate, }; // Converts a filter to its string. diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp index 1b6b2e13d3..4e43153ebd 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp @@ -36,6 +36,16 @@ namespace AppInstaller::Repository::Microsoft return result; } + SQLiteIndex SQLiteIndex::Open(const std::string& filePath, OpenDisposition disposition, Utility::ManagedFile&& indexFile) + { + return { filePath, disposition, std::move(indexFile) }; + } + + SQLiteIndex SQLiteIndex::CopyFrom(const std::string& filePath, SQLiteIndex& source) + { + return { filePath, source }; + } + std::unique_ptr SQLiteIndex::CreateISQLiteIndex(const SQLite::Version& version) { using namespace Schema; @@ -78,6 +88,13 @@ namespace AppInstaller::Repository::Microsoft THROW_HR_IF(APPINSTALLER_CLI_ERROR_CANNOT_WRITE_TO_UPLEVEL_INDEX, disposition == SQLiteStorageBase::OpenDisposition::ReadWrite && m_version != m_interface->GetVersion()); } + SQLiteIndex::SQLiteIndex(const std::string& target, SQLiteIndex& source) : + SQLiteStorageBase(target, source) + { + m_dbconn.EnableICU(); + m_interface = CreateISQLiteIndex(m_version); + } + #ifndef AICLI_DISABLE_TEST_HOOKS void SQLiteIndex::ForceVersion(const SQLite::Version& version) { @@ -249,7 +266,7 @@ namespace AppInstaller::Repository::Microsoft return m_interface->GetManifestIdByManifest(m_dbconn, manifest); } - std::vector SQLiteIndex::GetVersionKeysById(IdType id) const + std::vector SQLiteIndex::GetVersionKeysById(IdType id) const { std::lock_guard lockInterface{ *m_interfaceLock }; return m_interface->GetVersionKeysById(m_dbconn, id); diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h index 8ab21488a4..136867e3b0 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h @@ -37,6 +37,9 @@ namespace AppInstaller::Repository::Microsoft // Options for creating a new index. using CreateOptions = Schema::ISQLiteIndex::CreateOptions; + // The type of version keys. + using VersionKey = Schema::ISQLiteIndex::VersionKey; + SQLiteIndex(const SQLiteIndex&) = delete; SQLiteIndex& operator=(const SQLiteIndex&) = delete; @@ -47,10 +50,10 @@ namespace AppInstaller::Repository::Microsoft static SQLiteIndex CreateNew(const std::string& filePath, SQLite::Version version = SQLite::Version::Latest(), CreateOptions options = CreateOptions::None); // Opens an existing SQLiteIndex database. - static SQLiteIndex Open(const std::string& filePath, OpenDisposition disposition, Utility::ManagedFile&& indexFile = {}) - { - return { filePath, disposition, std::move(indexFile) }; - } + static SQLiteIndex Open(const std::string& filePath, OpenDisposition disposition, Utility::ManagedFile&& indexFile = {}); + + // Creates a copy of the given index. + static SQLiteIndex CopyFrom(const std::string& filePath, SQLiteIndex& source); #ifndef AICLI_DISABLE_TEST_HOOKS // Changes the version of the interface being used to operate on the database. @@ -124,7 +127,7 @@ namespace AppInstaller::Repository::Microsoft std::optional GetManifestIdByManifest(const Manifest::Manifest& manifest) const; // Gets all versions and channels for the given id. - std::vector GetVersionKeysById(IdType id) const; + std::vector GetVersionKeysById(IdType id) const; // Gets the string for the given metadata and manifest id, if present. MetadataResult GetMetadataByManifestId(SQLite::rowid_t manifestId) const; @@ -139,6 +142,7 @@ namespace AppInstaller::Repository::Microsoft // Get all the dependencies for a specific manifest. std::set> GetDependenciesByManifestRowId(SQLite::rowid_t manifestRowId) const; std::vector> GetDependentsById(AppInstaller::Manifest::string_t packageId) const; + private: // Constructor used to create a new index. SQLiteIndex(const std::string& target, const SQLite::Version& version); @@ -146,6 +150,9 @@ namespace AppInstaller::Repository::Microsoft // Constructor used to open an existing index. SQLiteIndex(const std::string& target, SQLiteStorageBase::OpenDisposition disposition, Utility::ManagedFile&& indexFile); + // Constructor used to copy the given index. + SQLiteIndex(const std::string& target, SQLiteIndex& source); + // Internal functions to normalize on the relativePath being present. IdType AddManifestInternal(const Manifest::Manifest& manifest, const std::optional& relativePath); bool UpdateManifestInternal(const Manifest::Manifest& manifest, const std::optional& relativePath); diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp index b8c6b4dcbe..013e331637 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp @@ -288,14 +288,34 @@ namespace AppInstaller::Repository::Microsoft std::vector GetAvailableVersionKeys(PinBehavior) const override { std::shared_ptr source = GetReferenceSource(); - std::vector versions = source->GetIndex().GetVersionKeysById(m_idId); - std::vector result; - for (const auto& vac : versions) { - result.emplace_back(source->GetIdentifier(), vac.GetVersion().ToString(), vac.GetChannel().ToString()); + auto sharedLock = m_availableVersionKeysLock.lock_shared(); + + if (!m_availableVersionKeys.empty()) + { + return m_availableVersionKeys; + } } - return result; + + auto exclusiveLock = m_availableVersionKeysLock.lock_exclusive(); + + if (!m_availableVersionKeys.empty()) + { + return m_availableVersionKeys; + } + + std::vector versions = source->GetIndex().GetVersionKeysById(m_idId); + + for (const auto& vk : versions) + { + std::string version = vk.VersionAndChannel.GetVersion().ToString(); + std::string channel = vk.VersionAndChannel.GetChannel().ToString(); + m_availableVersionKeys.emplace_back(source->GetIdentifier(), version, channel); + m_availableVersionKeysMap.emplace(MapKey{ std::move(version), std::move(channel) }, vk.ManifestId); + } + + return m_availableVersionKeys; } std::shared_ptr GetLatestAvailableVersion(PinBehavior) const override @@ -313,7 +333,23 @@ namespace AppInstaller::Repository::Microsoft return {}; } - std::optional manifestId = source->GetIndex().GetManifestIdByKey(m_idId, versionKey.Version, versionKey.Channel); + std::optional manifestId; + + { + MapKey requested{ versionKey.Version, versionKey.Channel }; + auto sharedLock = m_availableVersionKeysLock.lock_shared(); + + auto itr = m_availableVersionKeysMap.find(requested); + if (itr != m_availableVersionKeysMap.end()) + { + manifestId = itr->second; + } + } + + if (!manifestId) + { + manifestId = source->GetIndex().GetManifestIdByKey(m_idId, versionKey.Version, versionKey.Channel); + } if (manifestId) { @@ -349,6 +385,35 @@ namespace AppInstaller::Repository::Microsoft return nullptr; } + + private: + // Contains the information needed to map a version key to it's rows. + struct MapKey + { + Utility::NormalizedString Version; + Utility::NormalizedString Channel; + + bool operator<(const MapKey& other) const + { + if (Version < other.Version) + { + return true; + } + else if (Version == other.Version) + { + return Channel < other.Channel; + } + else + { + return false; + } + } + }; + + // To avoid removing const from the interface + mutable wil::srwlock m_availableVersionKeysLock; + mutable std::vector m_availableVersionKeys; + mutable std::map m_availableVersionKeysMap; }; // The IPackage impl for SQLiteIndexSource of Installed packages. diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h index 42e9ca89eb..bcda1869ef 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h @@ -28,7 +28,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 std::vector GetMultiPropertyByManifestId(const SQLite::Connection& connection, SQLite::rowid_t manifestId, PackageVersionMultiProperty property) const override; std::optional GetManifestIdByKey(const SQLite::Connection& connection, SQLite::rowid_t id, std::string_view version, std::string_view channel) const override; std::optional GetManifestIdByManifest(const SQLite::Connection& connection, const Manifest::Manifest& manifest) const override; - std::vector GetVersionKeysById(const SQLite::Connection& connection, SQLite::rowid_t id) const override; + std::vector GetVersionKeysById(const SQLite::Connection& connection, SQLite::rowid_t id) const override; // Version 1.1 MetadataResult GetMetadataByManifestId(const SQLite::Connection& connection, SQLite::rowid_t manifestId) const override; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp index 31b9de701b..8a68fb1fa8 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp @@ -63,53 +63,60 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 std::optional channelIdOpt = ChannelTable::SelectIdByValue(connection, channel, true); if (!channelIdOpt && !channel.empty()) { - // If a non-empty channel was given but none was found, we will just not filter on channel. AICLI_LOG(Repo, Info, << "Did not find a Channel { " << channel << " }"); return {}; } std::optional versionIdOpt; + std::vector> versionStrings; - if (version.empty()) + if (channelIdOpt) { - std::vector versionStrings; - - if (channelIdOpt) - { - versionStrings = ManifestTable::GetAllValuesByIds(connection, { id, channelIdOpt.value() }); - } - else - { - versionStrings = ManifestTable::GetAllValuesByIds(connection, { id }); - } + versionStrings = ManifestTable::GetAllValuesByIds(connection, { id, channelIdOpt.value() }); + } + else + { + versionStrings = ManifestTable::GetAllValuesByIds(connection, { id }); + } - if (versionStrings.empty()) - { - AICLI_LOG(Repo, Info, << "Did not find any Versions { " << id << ", " << channel << " }"); - return {}; - } + if (versionStrings.empty()) + { + AICLI_LOG(Repo, Info, << "Did not find any Versions { " << id << ", " << channel << " }"); + return {}; + } - // Convert the strings to Versions and sort them - std::vector versions; - for (std::string& v : versionStrings) - { - versions.emplace_back(std::move(v)); - } + // Convert the strings to Versions and sort them + struct VersionAndRow + { + SQLite::rowid_t Row = 0; + Utility::Version Version; - std::sort(versions.begin(), versions.end()); + bool operator<(const VersionAndRow& other) const { return Version < other.Version; } + }; + + std::vector versions; + for (auto& v : versionStrings) + { + versions.emplace_back(VersionAndRow{ v.first, std::move(v.second) }); + } - // Get the last version in the list (the highest version) and its rowid - const std::string& latestVersion = versions.back().ToString(); - versionIdOpt = VersionTable::SelectIdByValue(connection, latestVersion); + std::sort(versions.begin(), versions.end()); - if (!versionIdOpt) - { - AICLI_LOG(Repo, Warning, << "Did not find a Version row for the latest version { " << latestVersion << " }"); - } + if (version.empty()) + { + // Get the last version in the list (the highest version) + versionIdOpt = versions.back().Row; } else { - versionIdOpt = VersionTable::SelectIdByValue(connection, version, true); + VersionAndRow requested; + requested.Version = Utility::Version{ std::string(version) }; + + auto itr = std::lower_bound(versions.begin(), versions.end(), requested); + if (itr != versions.end() && itr->Version == requested.Version) + { + versionIdOpt = itr->Row; + } } if (!versionIdOpt) @@ -510,12 +517,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 std::optional Interface::GetPropertyByManifestId(const SQLite::Connection& connection, SQLite::rowid_t manifestId, PackageVersionProperty property) const { - if (!ManifestTable::ExistsById(connection, manifestId)) - { - AICLI_LOG(Repo, Info, << "Did not find manifest by id: " << manifestId); - return {}; - } - return GetPropertyByManifestIdInternal(connection, manifestId, property); } @@ -544,15 +545,15 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return {}; } - std::vector Interface::GetVersionKeysById(const SQLite::Connection& connection, SQLite::rowid_t id) const + std::vector Interface::GetVersionKeysById(const SQLite::Connection& connection, SQLite::rowid_t id) const { auto versionsAndChannels = ManifestTable::GetAllValuesById(connection, id); - std::vector result; + std::vector result; result.reserve(versionsAndChannels.size()); for (auto&& vac : versionsAndChannels) { - result.emplace_back(Utility::Version{ std::move(std::get<0>(vac)) }, Utility::Channel{ std::move(std::get<1>(vac)) }); + result.emplace_back(ISQLiteIndex::VersionKey{ Utility::VersionAndChannel{ Utility::Version{ std::move(std::get<1>(vac)) }, Utility::Channel{ std::move(std::get<2>(vac)) } }, std::get<0>(vac) }); } std::sort(result.begin(), result.end()); @@ -627,13 +628,13 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 switch (property) { case AppInstaller::Repository::PackageVersionProperty::Id: - return std::get<0>(ManifestTable::GetValuesById(connection, manifestId)); + return ManifestTable::GetValueById(connection, manifestId); case AppInstaller::Repository::PackageVersionProperty::Name: - return std::get<0>(ManifestTable::GetValuesById(connection, manifestId)); + return ManifestTable::GetValueById(connection, manifestId); case AppInstaller::Repository::PackageVersionProperty::Version: - return std::get<0>(ManifestTable::GetValuesById(connection, manifestId)); + return ManifestTable::GetValueById(connection, manifestId); case AppInstaller::Repository::PackageVersionProperty::Channel: - return std::get<0>(ManifestTable::GetValuesById(connection, manifestId)); + return ManifestTable::GetValueById(connection, manifestId); case AppInstaller::Repository::PackageVersionProperty::RelativePath: return PathPartTable::GetPathById(connection, std::get<0>(ManifestTable::GetIdsById(connection, manifestId))); default: diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.cpp index 795222e0fd..8a1f791f35 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.cpp @@ -63,14 +63,18 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 SQLite::Statement ManifestTableGetIdsById_Statement( const SQLite::Connection& connection, SQLite::rowid_t id, - std::initializer_list values) + std::initializer_list values, + bool stepAndVerify) { SQLite::Builder::StatementBuilder builder; builder.Select(values).From(s_ManifestTable_Table_Name).Where(SQLite::RowIDName).Equals(id); SQLite::Statement result = builder.Prepare(connection); - THROW_HR_IF(E_NOT_SET, !result.Step()); + if (stepAndVerify) + { + THROW_HR_IF(E_NOT_SET, !result.Step()); + } return result; } @@ -84,7 +88,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 const SQLite::Connection& connection, SQLite::rowid_t id, std::initializer_list columns, - std::initializer_list manifestColumnNames) + std::initializer_list manifestColumnNames, + bool stepAndVerify) { THROW_HR_IF(E_UNEXPECTED, manifestColumnNames.size() != columns.size()); @@ -108,7 +113,10 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 SQLite::Statement result = builder.Prepare(connection); - THROW_HR_IF(E_NOT_SET, !result.Step()); + if (stepAndVerify) + { + THROW_HR_IF(E_NOT_SET, !result.Step()); + } return result; } @@ -116,6 +124,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 SQLite::Statement ManifestTableGetAllValuesByIds_Statement( const SQLite::Connection& connection, std::initializer_list valueColumns, + std::initializer_list joinColumns, std::initializer_list idColumns, std::initializer_list ids) { @@ -126,9 +135,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 SQLite::Builder::StatementBuilder builder; builder.Select(valueColumns).From(s_ManifestTable_Table_Name); - for (const auto& valueColumn : valueColumns) + for (const auto& joinColumn : joinColumns) { - builder.Join(valueColumn.Table).On(QCol{ s_ManifestTable_Table_Name, valueColumn.Column }, QCol{ valueColumn.Table, SQLite::RowIDName }); + builder.Join(joinColumn.Table).On(QCol{ s_ManifestTable_Table_Name, joinColumn.Column }, QCol{ joinColumn.Table, SQLite::RowIDName }); } bool isFirst = true; @@ -157,18 +166,19 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return select; } - std::vector ManifestTableGetAllValuesByIds( + std::vector> ManifestTableGetAllValuesByIds( const SQLite::Connection& connection, std::initializer_list valueColumns, + std::initializer_list joinColumns, std::initializer_list idColumns, std::initializer_list ids) { - auto select = ManifestTableGetAllValuesByIds_Statement(connection, valueColumns, idColumns, ids); + auto select = ManifestTableGetAllValuesByIds_Statement(connection, valueColumns, joinColumns, idColumns, ids); - std::vector result; + std::vector> result; while (select.Step()) { - result.emplace_back(select.GetColumn(0)); + result.emplace_back(select.GetColumn(0), select.GetColumn(1)); } return result; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.h index cdc664e09a..0bf12be1bd 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.h @@ -7,6 +7,7 @@ #include #include #include +#include #include @@ -37,26 +38,30 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 SQLite::Statement ManifestTableGetIdsById_Statement( const SQLite::Connection& connection, SQLite::rowid_t id, - std::initializer_list values); + std::initializer_list values, + bool stepAndVerify = true); // Gets the requested values for the manifest with the given rowid. SQLite::Statement ManifestTableGetValuesById_Statement( const SQLite::Connection& connection, SQLite::rowid_t id, std::initializer_list columns, - std::initializer_list manifestColumnNames); + std::initializer_list manifestColumnNames, + bool stepAndVerify = true); // Gets all values for rows that match the given ids. SQLite::Statement ManifestTableGetAllValuesByIds_Statement( const SQLite::Connection& connection, std::initializer_list valueColumns, + std::initializer_list joinColumns, std::initializer_list idColumns, std::initializer_list ids); // Gets all values for rows that match the given ids. - std::vector ManifestTableGetAllValuesByIds( + std::vector> ManifestTableGetAllValuesByIds( const SQLite::Connection& connection, std::initializer_list valueColumns, + std::initializer_list joinColumns, std::initializer_list idColumns, std::initializer_list ids); @@ -137,6 +142,15 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return details::ManifestTableGetIdsById_Statement(connection, id, { details::GetManifestTableColumnName()...}).GetRow(); } + // Gets the id requested for the manifest with the given rowid, if it exists. + template + static std::optional GetIdById(const SQLite::Connection& connection, SQLite::rowid_t id) + { + auto statement = details::ManifestTableGetIdsById_Statement(connection, id, { details::GetManifestTableColumnName() }, false); + if (statement.Step()) { return statement.GetColumn(0); } + else { return std::nullopt; } + } + // Gets the values requested for the manifest with the given rowid. template static auto GetValuesById(const SQLite::Connection& connection, SQLite::rowid_t id) @@ -144,22 +158,37 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return details::ManifestTableGetValuesById_Statement(connection, id, { SQLite::Builder::QualifiedColumn{ Tables::TableName(), Tables::ValueName() }... }, { details::GetManifestTableColumnName()... }).GetRow(); } - // Gets the values for rows that match the given ids. + // Gets the value requested for the manifest with the given rowid, if it exists. + template + static std::optional GetValueById(const SQLite::Connection& connection, SQLite::rowid_t id) + { + auto statement = details::ManifestTableGetValuesById_Statement(connection, id, { SQLite::Builder::QualifiedColumn{ Table::TableName(), Table::ValueName() } }, { details::GetManifestTableColumnName
() }, false); + if (statement.Step()) { return statement.GetColumn(0); } + else { return std::nullopt; } + } + + // Gets the row ids and values for rows that match the given ids. template - static std::vector GetAllValuesByIds(const SQLite::Connection& connection, std::initializer_list ids) + static std::vector> GetAllValuesByIds(const SQLite::Connection& connection, std::initializer_list ids) { - return details::ManifestTableGetAllValuesByIds(connection, { SQLite::Builder::QualifiedColumn{ ValueTable::TableName(), ValueTable::ValueName() } }, { IdTables::ValueName()... }, ids); + return details::ManifestTableGetAllValuesByIds(connection, + { SQLite::Builder::QualifiedColumn{ ValueTable::TableName(), SQLite::RowIDName }, SQLite::Builder::QualifiedColumn{ ValueTable::TableName(), ValueTable::ValueName() } }, + { SQLite::Builder::QualifiedColumn{ ValueTable::TableName(), ValueTable::ValueName() } }, + { IdTables::ValueName()... }, ids); } // Gets all values for rows that match the given id. template - static std::vector> GetAllValuesById(const SQLite::Connection& connection, SQLite::rowid_t id) + static std::vector> GetAllValuesById(const SQLite::Connection& connection, SQLite::rowid_t id) { - auto stmt = details::ManifestTableGetAllValuesByIds_Statement(connection, { SQLite::Builder::QualifiedColumn{ ValueTables::TableName(), ValueTables::ValueName() }... }, { IdTable::ValueName() }, { id }); - std::vector> result; + auto stmt = details::ManifestTableGetAllValuesByIds_Statement(connection, + { SQLite::Builder::QualifiedColumn{ TableName(), SQLite::RowIDName }, SQLite::Builder::QualifiedColumn{ ValueTables::TableName(), ValueTables::ValueName() }... }, + { SQLite::Builder::QualifiedColumn{ ValueTables::TableName(), ValueTables::ValueName() }... }, + { IdTable::ValueName() }, { id }); + std::vector> result; while (stmt.Step()) { - result.emplace_back(stmt.GetRow()); + result.emplace_back(stmt.GetRow()); } return result; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface_1_3.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface_1_3.cpp index de08243d2c..383cc93e35 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface_1_3.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface_1_3.cpp @@ -80,8 +80,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_3 { case AppInstaller::Repository::PackageVersionProperty::ManifestSHA256Hash: { - SQLite::blob_t hash = std::get<0>(V1_0::ManifestTable::GetIdsById(connection, manifestId)); - return hash.empty() ? std::optional{} : Utility::SHA256::ConvertToString(hash); + std::optional hash = V1_0::ManifestTable::GetIdById(connection, manifestId); + return (!hash || hash->empty()) ? std::optional{} : Utility::SHA256::ConvertToString(hash.value()); } default: return V1_2::Interface::GetPropertyByManifestIdInternal(connection, manifestId, property); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/Interface_1_4.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/Interface_1_4.cpp index 9e93c11684..cc7fa96f35 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/Interface_1_4.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_4/Interface_1_4.cpp @@ -150,7 +150,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_4 { auto versionKeys = GetVersionKeysById(connection, dependency.first); THROW_HR_IF(E_UNEXPECTED, versionKeys.empty()); - checkedVersions.emplace(dependency.first, versionKeys[0].GetVersion()); + checkedVersions.emplace(dependency.first, versionKeys[0].VersionAndChannel.GetVersion()); } // If the latest version is less than min version required, fail the validation. diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_5/Interface_1_5.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_5/Interface_1_5.cpp index e1199027f7..321c79e80c 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_5/Interface_1_5.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_5/Interface_1_5.cpp @@ -173,9 +173,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_5 switch (property) { case AppInstaller::Repository::PackageVersionProperty::ArpMinVersion: - return std::get<0>(V1_0::ManifestTable::GetValuesById(connection, manifestId)); + return V1_0::ManifestTable::GetValueById(connection, manifestId); case AppInstaller::Repository::PackageVersionProperty::ArpMaxVersion: - return std::get<0>(V1_0::ManifestTable::GetValuesById(connection, manifestId)); + return V1_0::ManifestTable::GetValueById(connection, manifestId); default: return V1_4::Interface::GetPropertyByManifestIdInternal(connection, manifestId, property); } @@ -197,19 +197,15 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_5 auto versionKeys = GetVersionKeysById(connection, match.first); for (auto const& versionKey : versionKeys) { - auto manifestRowId = GetManifestIdByKey(connection, match.first, versionKey.GetVersion().ToString(), versionKey.GetChannel().ToString()); - if (manifestRowId) - { - auto arpMinVersion = GetPropertyByManifestId(connection, manifestRowId.value(), PackageVersionProperty::ArpMinVersion).value_or(""); - auto arpMaxVersion = GetPropertyByManifestId(connection, manifestRowId.value(), PackageVersionProperty::ArpMaxVersion).value_or(""); + auto arpMinVersion = GetPropertyByManifestId(connection, versionKey.ManifestId, PackageVersionProperty::ArpMinVersion).value_or(""); + auto arpMaxVersion = GetPropertyByManifestId(connection, versionKey.ManifestId, PackageVersionProperty::ArpMaxVersion).value_or(""); - // Either both empty or both not empty - THROW_HR_IF(E_UNEXPECTED, arpMinVersion.empty() != arpMaxVersion.empty()); + // Either both empty or both not empty + THROW_HR_IF(E_UNEXPECTED, arpMinVersion.empty() != arpMaxVersion.empty()); - if (!arpMinVersion.empty() && !arpMaxVersion.empty()) - { - ranges.emplace_back(Utility::VersionRange{ Utility::Version{ std::move(arpMinVersion) }, Utility::Version{ std::move(arpMaxVersion) } }); - } + if (!arpMinVersion.empty() && !arpMaxVersion.empty()) + { + ranges.emplace_back(Utility::VersionRange{ Utility::Version{ std::move(arpMinVersion) }, Utility::Version{ std::move(arpMaxVersion) } }); } } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h index aab15d17de..7334b48222 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h @@ -46,6 +46,15 @@ namespace AppInstaller::Repository::Microsoft::Schema DisableDependenciesSupport = 0x2, }; + // Contains both the object representation of the version key and the rows. + struct VersionKey + { + Utility::VersionAndChannel VersionAndChannel; + SQLite::rowid_t ManifestId; + + bool operator<(const VersionKey& other) const { return VersionAndChannel < other.VersionAndChannel; } + }; + // Creates all of the version dependent tables within the database. virtual void CreateTables(SQLite::Connection& connection, CreateOptions options) = 0; @@ -89,7 +98,7 @@ namespace AppInstaller::Repository::Microsoft::Schema virtual std::optional GetManifestIdByManifest(const SQLite::Connection& connection, const Manifest::Manifest& manifest) const = 0; // Gets all versions and channels for the given id. - virtual std::vector GetVersionKeysById(const SQLite::Connection& connection, SQLite::rowid_t id) const = 0; + virtual std::vector GetVersionKeysById(const SQLite::Connection& connection, SQLite::rowid_t id) const = 0; // Version 1.1 diff --git a/src/AppInstallerRepositoryCore/PackageDependenciesValidation.cpp b/src/AppInstallerRepositoryCore/PackageDependenciesValidation.cpp index dca5f8261b..d6a5945b15 100644 --- a/src/AppInstallerRepositoryCore/PackageDependenciesValidation.cpp +++ b/src/AppInstallerRepositoryCore/PackageDependenciesValidation.cpp @@ -33,7 +33,7 @@ namespace AppInstaller::Repository return depList; } - std::optional> GetPackageLatestVersion( + std::optional GetPackageLatestVersion( SQLiteIndex* index, Manifest::string_t packageId, std::set exclusions = {}) { SearchRequest request; @@ -54,31 +54,28 @@ namespace AppInstaller::Repository return {}; } - Utility::VersionAndChannel maxVersion(Utility::Version::CreateUnknown(), Utility::Channel("")); + SQLiteIndex::VersionKey maxVersion{ Utility::VersionAndChannel{ Utility::Version::CreateUnknown(), Utility::Channel("") } }; for (auto& v : vac) { - auto currentVersion = v.GetVersion(); + auto currentVersion = v.VersionAndChannel.GetVersion(); if (exclusions.find(currentVersion) != exclusions.end()) { continue; } - if (currentVersion > maxVersion.GetVersion()) + if (currentVersion > maxVersion.VersionAndChannel.GetVersion()) { maxVersion = v; } } - if (maxVersion.GetVersion().IsUnknown()) + if (maxVersion.VersionAndChannel.GetVersion().IsUnknown()) { return {}; } - auto manifestRowId = index->GetManifestIdByKey( - packageRowId, maxVersion.GetVersion().ToString(), maxVersion.GetChannel().ToString()); - - return std::make_pair(manifestRowId.value(), maxVersion.GetVersion()); + return maxVersion; } void ThrowOnManifestValidationFailed( @@ -130,14 +127,14 @@ namespace AppInstaller::Repository return depList; } - if (node.MinVersion > packageLatest.value().second) + if (node.MinVersion > packageLatest->VersionAndChannel.GetVersion()) { dependenciesError.emplace_back(ManifestError::NoSuitableMinVersionDependency, "PackageIdentifier", node.Id()); foundErrors = true; return depList; } - auto packageLatestDependencies = index->GetDependenciesByManifestRowId(packageLatest.value().first); + auto packageLatestDependencies = index->GetDependenciesByManifestRowId(packageLatest->ManifestId); std::for_each( packageLatestDependencies.begin(), packageLatestDependencies.end(), @@ -201,13 +198,13 @@ namespace AppInstaller::Repository THROW_HR(APPINSTALLER_CLI_ERROR_MISSING_PACKAGE); } - if (Utility::Version(manifest.Version) < packageLatest.value().second) + if (Utility::Version(manifest.Version) < packageLatest->VersionAndChannel.GetVersion()) { // all good, since it's min version the criteria is still satisfied. return true; } - auto nextLatestAfterDelete = GetPackageLatestVersion(index, manifest.Id, { packageLatest.value().second }); + auto nextLatestAfterDelete = GetPackageLatestVersion(index, manifest.Id, { packageLatest->VersionAndChannel.GetVersion() }); if (!nextLatestAfterDelete.has_value()) { @@ -224,7 +221,7 @@ namespace AppInstaller::Repository std::back_inserter(breakingManifests), [&](std::pair current) { - return current.second > nextLatestAfterDelete.value().second; + return current.second > nextLatestAfterDelete->VersionAndChannel.GetVersion(); } ); diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp index 338ab2d7c3..8c7cf0f7b2 100644 --- a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp +++ b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp @@ -273,12 +273,7 @@ namespace AppInstaller::Repository for (const auto& version : versions) { - auto manifestId = index.GetManifestIdByKey(match.first, version.GetVersion().ToString(), version.GetChannel().ToString()); - - if (manifestId) - { - index.RemoveManifestById(manifestId.value()); - } + index.RemoveManifestById(version.ManifestId); } } } diff --git a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h index ff68e41bab..fc0b9d5737 100644 --- a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h +++ b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h @@ -68,6 +68,9 @@ namespace AppInstaller::Repository ARP, MSIX, Installing, + // Same as `Installed`, but creating the source reference for this is sufficient to cause the cache to be updated + // on next Open of any `Installed` or `InstalledForceCacheUpdate`. + InstalledForceCacheUpdate, }; // A well known source. diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp index 0c514fe074..b9fc498f42 100644 --- a/src/AppInstallerRepositoryCore/RepositorySource.cpp +++ b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -162,6 +162,10 @@ namespace AppInstaller::Repository details.Type = Microsoft::PredefinedInstalledSourceFactory::Type(); details.Arg = Microsoft::PredefinedInstalledSourceFactory::FilterToString(Microsoft::PredefinedInstalledSourceFactory::Filter::None); return details; + case PredefinedSource::InstalledForceCacheUpdate: + details.Type = Microsoft::PredefinedInstalledSourceFactory::Type(); + details.Arg = Microsoft::PredefinedInstalledSourceFactory::FilterToString(Microsoft::PredefinedInstalledSourceFactory::Filter::NoneWithForcedCacheUpdate); + return details; case PredefinedSource::InstalledUser: details.Type = Microsoft::PredefinedInstalledSourceFactory::Type(); details.Arg = Microsoft::PredefinedInstalledSourceFactory::FilterToString(Microsoft::PredefinedInstalledSourceFactory::Filter::User); diff --git a/src/AppInstallerRepositoryCore/pch.h b/src/AppInstallerRepositoryCore/pch.h index e6ed263750..512990f9bd 100644 --- a/src/AppInstallerRepositoryCore/pch.h +++ b/src/AppInstallerRepositoryCore/pch.h @@ -14,6 +14,7 @@ #pragma warning( push ) #pragma warning ( disable : 6001 6340 6387 6388 26495 28196 ) #include +#include #include #include #include diff --git a/src/AppInstallerSharedLib/Public/AppInstallerVersions.h b/src/AppInstallerSharedLib/Public/AppInstallerVersions.h index aa98c64c3f..9c2ee43849 100644 --- a/src/AppInstallerSharedLib/Public/AppInstallerVersions.h +++ b/src/AppInstallerSharedLib/Public/AppInstallerVersions.h @@ -93,6 +93,9 @@ namespace AppInstaller::Utility uint64_t Integer = 0; std::string Other; + + private: + std::string m_foldedOther; }; // Gets the part breakdown for a given version. diff --git a/src/AppInstallerSharedLib/Public/winget/Registry.h b/src/AppInstallerSharedLib/Public/winget/Registry.h index 82bc2a3605..88c69b3bc6 100644 --- a/src/AppInstallerSharedLib/Public/winget/Registry.h +++ b/src/AppInstallerSharedLib/Public/winget/Registry.h @@ -282,6 +282,7 @@ namespace AppInstaller::Registry ValueList Values() const; operator bool() const { return m_key.operator bool(); } + operator HKEY() const { return m_key.get(); } // Open a Key; will return an empty Key if the subkey does not exist. static Key OpenIfExists(HKEY key, std::string_view subKey = {}, DWORD options = 0, REGSAM access = KEY_READ); diff --git a/src/AppInstallerSharedLib/Public/winget/SQLiteMetadataTable.h b/src/AppInstallerSharedLib/Public/winget/SQLiteMetadataTable.h index e939bd5bc2..036a2bafc4 100644 --- a/src/AppInstallerSharedLib/Public/winget/SQLiteMetadataTable.h +++ b/src/AppInstallerSharedLib/Public/winget/SQLiteMetadataTable.h @@ -10,6 +10,7 @@ namespace AppInstaller::SQLite { using namespace std::string_view_literals; + static constexpr std::string_view s_MetadataValueName_DatabaseIdentifier = "databaseIdentifier"sv; static constexpr std::string_view s_MetadataValueName_MajorVersion = "majorVersion"sv; static constexpr std::string_view s_MetadataValueName_MinorVersion = "minorVersion"sv; static constexpr std::string_view s_MetadataValueName_LastWriteTime = "lastwritetime"sv; diff --git a/src/AppInstallerSharedLib/Public/winget/SQLiteStorageBase.h b/src/AppInstallerSharedLib/Public/winget/SQLiteStorageBase.h index 8b26f6c48b..fa0e54cbe3 100644 --- a/src/AppInstallerSharedLib/Public/winget/SQLiteStorageBase.h +++ b/src/AppInstallerSharedLib/Public/winget/SQLiteStorageBase.h @@ -25,6 +25,9 @@ namespace AppInstaller::SQLite // Gets the last write time for the database. std::chrono::system_clock::time_point GetLastWriteTime(); + // Gets the identifier written to the database when it was created. + std::string GetDatabaseIdentifier(); + // Gets the schema version of the database. Version GetVersion() const { return m_version; } @@ -33,6 +36,8 @@ namespace AppInstaller::SQLite SQLiteStorageBase(const std::string& filePath, SQLiteStorageBase::OpenDisposition disposition, Utility::ManagedFile&& indexFile); + SQLiteStorageBase(const std::string& target, SQLiteStorageBase& source); + // Sets the last write time metadata value in the database. void SetLastWriteTime(); diff --git a/src/AppInstallerSharedLib/Public/winget/SQLiteWrapper.h b/src/AppInstallerSharedLib/Public/winget/SQLiteWrapper.h index 32a65639cb..68182182e3 100644 --- a/src/AppInstallerSharedLib/Public/winget/SQLiteWrapper.h +++ b/src/AppInstallerSharedLib/Public/winget/SQLiteWrapper.h @@ -317,6 +317,28 @@ namespace AppInstaller::SQLite Statement m_release; }; + // A SQLite backup operation. + struct Backup + { + // Creates a backup. + static Backup Create(Connection& destination, const std::string& destinationName, Connection& source, const std::string& sourceName); + + Backup(const Backup&) = delete; + Backup& operator=(const Backup&) = delete; + + Backup(Backup&&) = default; + Backup& operator=(Backup&&) = default; + + // Performs some or all of the backup. + // Returns true if the backup is completed, false if not. + bool Step(int pages = -1); + + private: + Backup(Connection& destination, const std::string& destinationName, Connection& source, const std::string& sourceName); + + wil::unique_any m_backup; + }; + // The escape character used in the EscapeStringForLike function. extern std::string_view EscapeCharForLike; diff --git a/src/AppInstallerSharedLib/SQLiteStorageBase.cpp b/src/AppInstallerSharedLib/SQLiteStorageBase.cpp index dd6fe57b4c..f835925221 100644 --- a/src/AppInstallerSharedLib/SQLiteStorageBase.cpp +++ b/src/AppInstallerSharedLib/SQLiteStorageBase.cpp @@ -40,6 +40,11 @@ namespace AppInstaller::SQLite return Utility::ConvertUnixEpochToSystemClock(lastWriteTime); } + std::string SQLiteStorageBase::GetDatabaseIdentifier() + { + return MetadataTable::TryGetNamedValue(m_dbconn, s_MetadataValueName_DatabaseIdentifier).value_or(std::string{}); + } + SQLiteStorageBase::SQLiteStorageBase(const std::string& filePath, OpenDisposition disposition, Utility::ManagedFile&& file) : m_indexFile(std::move(file)) { @@ -111,5 +116,21 @@ namespace AppInstaller::SQLite { m_version = version; MetadataTable::Create(m_dbconn); + + // Write a new identifier for this database + GUID databaseIdentifier; + THROW_IF_FAILED(CoCreateGuid(&databaseIdentifier)); + std::ostringstream stream; + stream << databaseIdentifier; + MetadataTable::SetNamedValue(m_dbconn, s_MetadataValueName_DatabaseIdentifier, stream.str()); + } + + SQLiteStorageBase::SQLiteStorageBase(const std::string& target, SQLiteStorageBase& source) : + m_dbconn(SQLite::Connection::Create(target, SQLite::Connection::OpenDisposition::Create)), + m_version(source.m_version) + { + std::string mainDatabase = "main"; + Backup backup = Backup::Create(m_dbconn, mainDatabase, source.m_dbconn, mainDatabase); + backup.Step(); } -} \ No newline at end of file +} diff --git a/src/AppInstallerSharedLib/SQLiteWrapper.cpp b/src/AppInstallerSharedLib/SQLiteWrapper.cpp index b827ec0bcb..43834c27bd 100644 --- a/src/AppInstallerSharedLib/SQLiteWrapper.cpp +++ b/src/AppInstallerSharedLib/SQLiteWrapper.cpp @@ -18,14 +18,14 @@ using namespace std::string_view_literals; #define THROW_SQLITE(_error_,_connection_) \ do { \ - int _ts_sqliteReturnValue = _error_; \ - sqlite3* _ts_sqliteConnection = _connection_; \ + int _ts_sqliteReturnValue = (_error_); \ + sqlite3* _ts_sqliteConnection = (_connection_); \ THROW_EXCEPTION_MSG(SQLiteException(_ts_sqliteReturnValue), _ts_sqliteConnection ? sqlite3_errmsg(_ts_sqliteConnection) : sqlite3_errstr(_ts_sqliteReturnValue)); \ } while (0,0) #define THROW_IF_SQLITE_FAILED(_statement_,_connection_) \ do { \ - int _tisf_sqliteReturnValue = _statement_; \ + int _tisf_sqliteReturnValue = (_statement_); \ if (_tisf_sqliteReturnValue != SQLITE_OK) \ { \ THROW_SQLITE(_tisf_sqliteReturnValue,_connection_); \ @@ -350,6 +350,46 @@ namespace AppInstaller::SQLite } } + Backup::Backup(Connection& destination, const std::string& destinationName, Connection& source, const std::string& sourceName) + { + m_backup.reset(sqlite3_backup_init(destination, destinationName.c_str(), source, sourceName.c_str())); + + if (!m_backup) + { + THROW_SQLITE(sqlite3_errcode(destination), destination); + } + } + + Backup Backup::Create(Connection& destination, const std::string& destinationName, Connection& source, const std::string& sourceName) + { + return { destination, destinationName, source, sourceName }; + } + + bool Backup::Step(int pages) + { + int stepResult = sqlite3_backup_step(m_backup.get(), pages); + + if (stepResult == SQLITE_OK) + { + // A negative number of pages should finish the operation + if (pages < 0) + { + THROW_HR(E_UNEXPECTED); + } + + // Success but not done + return false; + } + else if (stepResult == SQLITE_DONE) + { + return true; + } + else + { + THROW_SQLITE(stepResult, nullptr); + } + } + std::string_view EscapeCharForLike = "'"sv; std::string EscapeStringForLike(std::string_view value) diff --git a/src/AppInstallerSharedLib/Versions.cpp b/src/AppInstallerSharedLib/Versions.cpp index abb097b724..2b84e33a52 100644 --- a/src/AppInstallerSharedLib/Versions.cpp +++ b/src/AppInstallerSharedLib/Versions.cpp @@ -295,10 +295,15 @@ namespace AppInstaller::Utility { Other = end; } + + m_foldedOther = Utility::FoldCase(static_cast(Other)); } Version::Part::Part(uint64_t integer, std::string other) : - Integer(integer), Other(std::move(other)) {} + Integer(integer), Other(std::move(other)) + { + m_foldedOther = Utility::FoldCase(static_cast(Other)); + } bool Version::Part::operator<(const Part& other) const { @@ -320,8 +325,9 @@ namespace AppInstaller::Utility // If the other Other is empty and this is not, this is less. return true; } - else if (Other < other.Other) + else if (m_foldedOther < other.m_foldedOther) { + // Compare the folded versions return true; } @@ -331,7 +337,7 @@ namespace AppInstaller::Utility bool Version::Part::operator==(const Part& other) const { - return Integer == other.Integer && Other == other.Other; + return Integer == other.Integer && m_foldedOther == other.m_foldedOther; } bool Version::Part::operator!=(const Part& other) const