winget-cli

Unnamed repository; edit this file 'description' to name the repository.
Log | Files | Refs | README | LICENSE

commit f236fa2fda208e29827f10a50e56968e308e23b4
parent dd557b234dad115c0b59bbc10e04317aeaf8f1bc
Author: JohnMcPMS <johnmcp@microsoft.com>
Date:   Mon,  1 Mar 2021 20:53:57 -0800

Add normalized name and publisher to index and correlation telemetry (#767)

## Change
This change adds the normalized name and publisher to the index, as well as leveraging this to collect telemetry on how well this normalization strategy is doing to correlate a manifest with the ARP entry on the machine.

The normalized name and publisher tables are added to the index as 1:N entries due to the introduction of full localization of name and publisher into the new manifest schema.  These tables don't have any link between them, which would allow a combination of name and publisher from any localization to work.  This tradeoff of precision seems worthwhile over writing more complex code to handle two values at a time, and it also allows the potential for one of the fields to change while the other remains constant and consume less space overall.

Searching is done by sending in the observed values for name and publisher, and the normalization is done by the source before comparing with its internal store.  In this way any type of normalization can occur internal to the source without the workflow logic needing to be aware.

In order to collect telemetry, we first snapshot the set of packages in ARP before executing the install.  Afterward, we compare the set with the snapshot to determine which ones are net new.  We also search the source using the manifest used to install to determine if there are any matches.  Finally, we intersect these two sets of packages.

The telemetry contains information about the package that was being installed `{source, id, version, channel}`, information about the number of packages in each state above `{changed, matching, intersection}`, and information from the ARP entry (if any) that we have decided is the one that represents the main package `{name, version, publisher, language}`.  We will be able to leverage this telemetry to improve both our normalization code and the manifests in winget-pkgs to improve our ability to correlate installed packages with remote ones.
Diffstat:
M.github/actions/spelling/allow.txt | 3+++
M.github/actions/spelling/expect.txt | 1+
Mazure-pipelines.yml | 3+++
Msrc/AppInstallerCLI.sln | 11++++++++++-
Msrc/AppInstallerCLICore/AppInstallerCLICore.vcxproj | 1+
Msrc/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters | 8++++----
Msrc/AppInstallerCLICore/Commands/UpgradeCommand.cpp | 19++-----------------
Msrc/AppInstallerCLICore/ExecutionContext.h | 211++-----------------------------------------------------------------------------
Asrc/AppInstallerCLICore/ExecutionContextData.h | 189+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Msrc/AppInstallerCLICore/Workflows/ImportExportFlow.cpp | 520++++++++++++++++++++++++++++++++++++++++----------------------------------------
Msrc/AppInstallerCLICore/Workflows/InstallFlow.cpp | 244++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
Msrc/AppInstallerCLICore/Workflows/InstallFlow.h | 22++++++++++++++++++++--
Msrc/AppInstallerCLICore/Workflows/UpdateFlow.cpp | 10+---------
Msrc/AppInstallerCLICore/Workflows/WorkflowBase.h | 1+
Msrc/AppInstallerCLIE2ETests/TestData/Manifests/TestExeInstaller.yaml | 2+-
Asrc/AppInstallerCLITests/ARPChanges.cpp | 424+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Msrc/AppInstallerCLITests/AppInstallerCLITests.vcxproj | 1+
Msrc/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters | 3+++
Msrc/AppInstallerCLITests/CompositeSource.cpp | 21++++++++++++++++-----
Msrc/AppInstallerCLITests/PredefinedInstalledSource.cpp | 4++--
Msrc/AppInstallerCLITests/SQLiteIndex.cpp | 141+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
Msrc/AppInstallerCLITests/SQLiteIndexSource.cpp | 22++++++++++++++++++++++
Msrc/AppInstallerCLITests/Sources.cpp | 85++++++++++++++++++++-----------------------------------------------------------
Msrc/AppInstallerCLITests/TestData/InstallFlowTest_Exe.yaml | 2+-
Msrc/AppInstallerCLITests/TestHooks.h | 6++++++
Msrc/AppInstallerCLITests/TestSource.cpp | 65++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
Msrc/AppInstallerCLITests/TestSource.h | 29++++++++++++++++++++++++++++-
Msrc/AppInstallerCLITests/WorkFlow.cpp | 9++++-----
Msrc/AppInstallerCLITests/main.cpp | 13++++++++++++-
Msrc/AppInstallerCommonCore/AppInstallerTelemetry.cpp | 132+++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------
Msrc/AppInstallerCommonCore/NameNormalization.cpp | 47++++++++++++++++++++++++++++++++++++++---------
Msrc/AppInstallerCommonCore/Public/AppInstallerLanguageUtilities.h | 79+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Msrc/AppInstallerCommonCore/Public/AppInstallerStrings.h | 1+
Msrc/AppInstallerCommonCore/Public/AppInstallerTelemetry.h | 75+++++++++++++++++++++++++++++++++++++++++++--------------------------------
Msrc/AppInstallerCommonCore/Public/winget/ManifestLocalization.h | 2+-
Msrc/AppInstallerCommonCore/Public/winget/NameNormalization.h | 11+++++++++++
Msrc/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj | 6++++++
Msrc/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters | 21+++++++++++++++++++++
Msrc/AppInstallerRepositoryCore/CompositeSource.cpp | 32++++++++++++++++++++++++++++++--
Msrc/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp | 98++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------
Msrc/AppInstallerRepositoryCore/Microsoft/ARPHelper.h | 4++++
Msrc/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp | 37++++++++-----------------------------
Msrc/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.h | 3+--
Msrc/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp | 19++++++++++++-------
Msrc/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h | 5+++++
Msrc/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp | 29+++++++++++++++++++++++++++++
Msrc/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h | 3+++
Msrc/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp | 48++++++++++++++++++++++++++++++++----------------
Msrc/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.cpp | 88++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------
Msrc/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.h | 14++++++++------
Msrc/AppInstallerRepositoryCore/Microsoft/Schema/1_0/SearchResultsTable.h | 13+++++++++----
Msrc/AppInstallerRepositoryCore/Microsoft/Schema/1_0/SearchResultsTable_1_0.cpp | 119+++++++++++++++++++++++++++++++++++++++++--------------------------------------
Msrc/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface.h | 2++
Msrc/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp | 122++++++++++++++++++++++++++++++++++++++++++++-----------------------------------
Msrc/AppInstallerRepositoryCore/Microsoft/Schema/1_1/SearchResultsTable.h | 2+-
Msrc/AppInstallerRepositoryCore/Microsoft/Schema/1_1/SearchResultsTable_1_1.cpp | 2+-
Asrc/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h | 34++++++++++++++++++++++++++++++++++
Asrc/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp | 205+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Asrc/AppInstallerRepositoryCore/Microsoft/Schema/1_2/NormalizedPackageNameTable.h | 22++++++++++++++++++++++
Asrc/AppInstallerRepositoryCore/Microsoft/Schema/1_2/NormalizedPackagePublisherTable.h | 22++++++++++++++++++++++
Asrc/AppInstallerRepositoryCore/Microsoft/Schema/1_2/SearchResultsTable.h | 33+++++++++++++++++++++++++++++++++
Asrc/AppInstallerRepositoryCore/Microsoft/Schema/1_2/SearchResultsTable_1_2.cpp | 39+++++++++++++++++++++++++++++++++++++++
Msrc/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h | 5+++++
Msrc/AppInstallerRepositoryCore/Microsoft/Schema/Version.cpp | 9+++++++--
Msrc/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h | 33+++++++++++++++++++++++++++++++--
Msrc/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h | 3+--
Msrc/AppInstallerRepositoryCore/RepositorySource.cpp | 8++------
Msrc/AppInstallerRepositoryCore/SQLiteStatementBuilder.cpp | 14++++++++++++++
Msrc/AppInstallerRepositoryCore/SQLiteStatementBuilder.h | 5++++-
Msrc/AppInstallerTestExeInstaller/main.cpp | 58+++++++++++++++++++++++++++++++++++-----------------------
70 files changed, 2631 insertions(+), 943 deletions(-)

diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt @@ -190,6 +190,7 @@ Linux LOCALAPPDATA localtime LOGPATH +logsql logto LONGLONG LPCGUID @@ -201,6 +202,7 @@ MAKEINTRESOURCE makemsix MANIFESTSCHEMA MANIFESTVERSION +MBs mday metadata microsoft @@ -414,6 +416,7 @@ triaged trunc TRUSTEDPEOPLE tt +ttl typedef typename uap diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt @@ -35,6 +35,7 @@ casemap casemappings cch CDEF +cend centralus certmgr certs diff --git a/azure-pipelines.yml b/azure-pipelines.yml @@ -240,6 +240,7 @@ jobs: - task: VSTest@2 displayName: Run E2E Tests Packaged x64 inputs: + testRunTitle: 'E2E Packaged x64' testSelector: 'testAssemblies' testAssemblyVer2: 'src\x64\Release\AppInstallerCLIE2ETests\AppInstallerCLIE2ETests.dll' runSettingsFile: 'src\x64\Release\AppInstallerCLIE2ETests\Test.runsettings' @@ -265,6 +266,7 @@ jobs: - task: VSTest@2 displayName: Run E2E Tests Packaged x86 inputs: + testRunTitle: 'E2E Packaged x86' testSelector: 'testAssemblies' testAssemblyVer2: 'src\x86\Release\AppInstallerCLIE2ETests\AppInstallerCLIE2ETests.dll' runSettingsFile: 'src\x86\Release\AppInstallerCLIE2ETests\Test.runsettings' @@ -310,6 +312,7 @@ jobs: - task: ComponentGovernanceComponentDetection@0 displayName: Component Governance + continueOnError: true inputs: scanType: 'Register' verbosity: 'Verbose' diff --git a/src/AppInstallerCLI.sln b/src/AppInstallerCLI.sln @@ -17,8 +17,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Project", "Project", "{8D53 ..\azure-pipelines.loc.yml = ..\azure-pipelines.loc.yml ..\azure-pipelines.yml = ..\azure-pipelines.yml ..\cgmanifest.json = ..\cgmanifest.json - ..\README.md = ..\README.md ..\doc\packages.schema.json = ..\doc\packages.schema.json + ..\README.md = ..\README.md ..\doc\Settings.md = ..\doc\Settings.md ..\doc\settings.schema.json = ..\doc\settings.schema.json EndProjectSection @@ -68,6 +68,14 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "Valijson", "Valijson\Valijs EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "ManifestSchema", "ManifestSchema\ManifestSchema.vcxitems", "{7D05F64D-CE5A-42AA-A2C1-E91458F061CF}" EndProject +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "spelling", "spelling", "{2ACDE176-F13F-42FA-8159-C34FA3D37837}" + ProjectSection(SolutionItems) = preProject + ..\.github\actions\spelling\allow.txt = ..\.github\actions\spelling\allow.txt + ..\.github\actions\spelling\excludes.txt = ..\.github\actions\spelling\excludes.txt + ..\.github\actions\spelling\expect.txt = ..\.github\actions\spelling\expect.txt + ..\.github\actions\spelling\patterns.txt = ..\.github\actions\spelling\patterns.txt + EndProjectSection +EndProject Global GlobalSection(SharedMSBuildProjectFiles) = preSolution ManifestSchema\ManifestSchema.vcxitems*{1622da16-914f-4f57-a259-d5169003cc8c}*SharedItemsImports = 4 @@ -429,6 +437,7 @@ Global {3BAF989F-7F65-465B-ACE8-BAFE42D1017E} = {EA8CD934-0702-4911-A2C5-A40600E616DE} {358BC478-0624-4AD1-A933-0422B5292AF8} = {60618CAC-2995-4DF9-9914-45C6FC02C995} {7D05F64D-CE5A-42AA-A2C1-E91458F061CF} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7} + {2ACDE176-F13F-42FA-8159-C34FA3D37837} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {B6FDB70C-A751-422C-ACD1-E35419495857} diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj @@ -193,6 +193,7 @@ <ClInclude Include="Commands\SettingsCommand.h" /> <ClInclude Include="CompletionData.h" /> <ClInclude Include="ExecutionArgs.h" /> + <ClInclude Include="ExecutionContextData.h" /> <ClInclude Include="ExecutionContext.h" /> <ClInclude Include="ExecutionProgress.h" /> <ClInclude Include="ExecutionReporter.h" /> diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters @@ -68,9 +68,6 @@ <ClInclude Include="ExecutionReporter.h"> <Filter>Header Files</Filter> </ClInclude> - <ClInclude Include="ExecutionContext.h"> - <Filter>Header Files</Filter> - </ClInclude> <ClInclude Include="ExecutionArgs.h"> <Filter>Header Files</Filter> </ClInclude> @@ -149,7 +146,10 @@ <ClInclude Include="Commands\ImportCommand.h"> <Filter>Commands</Filter> </ClInclude> - <ClInclude Include="resource.h"> + <ClInclude Include="ExecutionContextData.h"> + <Filter>Header Files</Filter> + </ClInclude> + <ClInclude Include="ExecutionContext.h"> <Filter>Header Files</Filter> </ClInclude> </ItemGroup> diff --git a/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp b/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp @@ -150,20 +150,13 @@ namespace AppInstaller::CLI // --manifest case where new manifest is provided context << GetManifestFromArg << - ReportManifestIdentity << SearchSourceUsingManifest << EnsureOneMatchFromSearchResult(true) << GetInstalledPackageVersion << EnsureUpdateVersionApplicable << SelectInstaller << EnsureApplicableInstaller << - ShowInstallationDisclaimer << - Workflow::ReportExecutionStage(ExecutionStage::Download) << - DownloadInstaller << - Workflow::ReportExecutionStage(ExecutionStage::Execution) << - ExecuteInstaller << - Workflow::ReportExecutionStage(ExecutionStage::PostExecution) << - RemoveInstaller; + InstallPackageInstaller; } else { @@ -171,7 +164,6 @@ namespace AppInstaller::CLI context << SearchSourceForSingle << EnsureOneMatchFromSearchResult(true) << - ReportPackageIdentity << GetInstalledPackageVersion; if (context.Args.Contains(Execution::Args::Type::Version)) @@ -190,14 +182,7 @@ namespace AppInstaller::CLI context << SelectLatestApplicableUpdate(true); } - context << - ShowInstallationDisclaimer << - Workflow::ReportExecutionStage(ExecutionStage::Download) << - DownloadInstaller << - Workflow::ReportExecutionStage(ExecutionStage::Execution) << - ExecuteInstaller << - Workflow::ReportExecutionStage(ExecutionStage::PostExecution) << - RemoveInstaller; + context << InstallPackageInstaller; } } } diff --git a/src/AppInstallerCLICore/ExecutionContext.h b/src/AppInstallerCLICore/ExecutionContext.h @@ -2,20 +2,12 @@ // Licensed under the MIT License. #pragma once #include <AppInstallerLogging.h> -#include <AppInstallerRepositorySearch.h> -#include <AppInstallerRepositorySource.h> -#include <winget/Manifest.h> #include "ExecutionReporter.h" #include "ExecutionArgs.h" +#include "ExecutionContextData.h" #include "CompletionData.h" -#include "PackageCollection.h" -#include <filesystem> -#include <map> -#include <string> -#include <utility> -#include <variant> -#include <vector> +#include <string_view> // Terminates the Context with some logging to indicate the location. @@ -43,38 +35,6 @@ namespace AppInstaller::CLI::Workflow namespace AppInstaller::CLI::Execution { - // Names a piece of data stored in the context by a workflow step. - // Must start at 0 to enable direct access to variant in Context. - // Max must be last and unused. - enum class Data : size_t - { - Source, - SearchResult, - SourceList, - Package, - Manifest, - PackageVersion, - Installer, - HashPair, - InstallerPath, - LogPath, - InstallerArgs, - CompletionData, - InstalledPackageVersion, - ExecutionStage, - UninstallString, - PackageFamilyNames, - ProductCodes, - // On export: A collection of packages to be exported to a file - // On import: A collection of packages read from a file - PackageCollection, - // On import: A collection of specific package versions to install - PackagesToInstall, - // On import: Sources for the imported packages - Sources, - Max - }; - // bit masks used as Context flags enum class ContextFlag : int { @@ -86,149 +46,10 @@ namespace AppInstaller::CLI::Execution DEFINE_ENUM_FLAG_OPERATORS(ContextFlag); - namespace details - { - template <Data D> - struct DataMapping - { - // value_t type specifies the type of this data - }; - - template <> - struct DataMapping<Data::Source> - { - using value_t = std::shared_ptr<Repository::ISource>; - }; - - template <> - struct DataMapping<Data::SearchResult> - { - using value_t = Repository::SearchResult; - }; - - template <> - struct DataMapping<Data::SourceList> - { - using value_t = std::vector<Repository::SourceDetails>; - }; - - template <> - struct DataMapping<Data::Package> - { - using value_t = std::shared_ptr<Repository::IPackage>; - }; - - template <> - struct DataMapping<Data::Manifest> - { - using value_t = Manifest::Manifest; - }; - - template <> - struct DataMapping<Data::PackageVersion> - { - using value_t = std::shared_ptr<Repository::IPackageVersion>; - }; - - template <> - struct DataMapping<Data::Installer> - { - using value_t = std::optional<Manifest::ManifestInstaller>; - }; - - template <> - struct DataMapping<Data::HashPair> - { - using value_t = std::pair<std::vector<uint8_t>, std::vector<uint8_t>>; - }; - - template <> - struct DataMapping<Data::InstallerPath> - { - using value_t = std::filesystem::path; - }; - - template <> - struct DataMapping<Data::LogPath> - { - using value_t = std::filesystem::path; - }; - - template <> - struct DataMapping<Data::InstallerArgs> - { - using value_t = std::string; - }; - - template <> - struct DataMapping<Data::CompletionData> - { - using value_t = CLI::CompletionData; - }; - - template <> - struct DataMapping<Data::InstalledPackageVersion> - { - using value_t = std::shared_ptr<Repository::IPackageVersion>; - }; - - template <> - struct DataMapping<Data::ExecutionStage> - { - using value_t = Workflow::ExecutionStage; - }; - - template <> - struct DataMapping<Data::UninstallString> - { - using value_t = std::string; - }; - - template <> - struct DataMapping<Data::PackageFamilyNames> - { - using value_t = std::vector<Utility::LocIndString>; - }; - - template <> - struct DataMapping<Data::ProductCodes> - { - using value_t = std::vector<Utility::LocIndString>; - }; - - template <> - struct DataMapping<Data::PackageCollection> - { - using value_t = CLI::PackageCollection; - }; - - template <> - struct DataMapping<Data::PackagesToInstall> - { - using value_t = std::vector<std::shared_ptr<Repository::IPackageVersion>>; - }; - - template <> - struct DataMapping<Data::Sources> - { - using value_t = std::vector<std::shared_ptr<Repository::ISource>>; - }; - - // Used to deduce the DataVariant type; making a variant that includes std::monostate and all DataMapping types. - template <size_t... I> - inline auto Deduce(std::index_sequence<I...>) { return std::variant<std::monostate, DataMapping<static_cast<Data>(I)>::value_t...>{}; } - - // Holds data of any type listed in a DataMapping. - using DataVariant = decltype(Deduce(std::make_index_sequence<static_cast<size_t>(Data::Max)>())); - - // Gets the index into the variant for the given Data. - constexpr inline size_t DataIndex(Data d) { return static_cast<size_t>(d) + 1; } - } - // The context within which all commands execute. // Contains input/output via Execution::Reporter and // arguments via Execution::Args. - struct Context + struct Context : EnumBasedVariantMap<Data, details::DataMapping> { Context(std::ostream& out, std::istream& in) : Reporter(out, in) {} @@ -262,31 +83,6 @@ namespace AppInstaller::CLI::Execution // Set the context to the terminated state. void Terminate(HRESULT hr, std::string_view file = {}, size_t line = {}); - // Adds a value to the context data, or overwrites an existing entry. - // This must be used to create the initial data entry, but Get can be used to modify. - template <Data D> - void Add(typename details::DataMapping<D>::value_t&& v) - { - m_data[D].emplace<details::DataIndex(D)>(std::forward<typename details::DataMapping<D>::value_t>(v)); - } - template <Data D> - void Add(const typename details::DataMapping<D>::value_t& v) - { - m_data[D].emplace<details::DataIndex(D)>(v); - } - - // Return a value indicating whether the given data type is stored in the context. - bool Contains(Data d) { return (m_data.find(d) != m_data.end()); } - - // Gets context data; which can be modified in place. - template <Data D> - typename details::DataMapping<D>::value_t& Get() - { - auto itr = m_data.find(D); - THROW_HR_IF_MSG(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), itr == m_data.end(), "Get(%d)", D); - return std::get<details::DataIndex(D)>(itr->second); - } - // Gets context flags ContextFlag GetFlags() const { @@ -314,7 +110,6 @@ namespace AppInstaller::CLI::Execution DestructionToken m_disableCtrlHandlerOnExit = false; bool m_isTerminated = false; HRESULT m_terminationHR = S_OK; - std::map<Data, details::DataVariant> m_data; size_t m_CtrlSignalCount = 0; ContextFlag m_flags = ContextFlag::None; }; diff --git a/src/AppInstallerCLICore/ExecutionContextData.h b/src/AppInstallerCLICore/ExecutionContextData.h @@ -0,0 +1,189 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include <AppInstallerRepositorySearch.h> +#include <AppInstallerRepositorySource.h> +#include <winget/Manifest.h> +#include "CompletionData.h" +#include "PackageCollection.h" +#include "Workflows/WorkflowBase.h" + +#include <filesystem> +#include <map> +#include <string> +#include <utility> +#include <variant> +#include <vector> + + +namespace AppInstaller::CLI::Execution +{ + // Names a piece of data stored in the context by a workflow step. + // Must start at 0 to enable direct access to variant in Context. + // Max must be last and unused. + enum class Data : size_t + { + Source, + SearchResult, + SourceList, + Package, + Manifest, + PackageVersion, + Installer, + HashPair, + InstallerPath, + LogPath, + InstallerArgs, + CompletionData, + InstalledPackageVersion, + ExecutionStage, + UninstallString, + PackageFamilyNames, + ProductCodes, + // On export: A collection of packages to be exported to a file + // On import: A collection of packages read from a file + PackageCollection, + // On import: A collection of specific package versions to install + PackagesToInstall, + // On import: Sources for the imported packages + Sources, + ARPSnapshot, + Max + }; + + namespace details + { + template <Data D> + struct DataMapping + { + // value_t type specifies the type of this data + }; + + template <> + struct DataMapping<Data::Source> + { + using value_t = std::shared_ptr<Repository::ISource>; + }; + + template <> + struct DataMapping<Data::SearchResult> + { + using value_t = Repository::SearchResult; + }; + + template <> + struct DataMapping<Data::SourceList> + { + using value_t = std::vector<Repository::SourceDetails>; + }; + + template <> + struct DataMapping<Data::Package> + { + using value_t = std::shared_ptr<Repository::IPackage>; + }; + + template <> + struct DataMapping<Data::Manifest> + { + using value_t = Manifest::Manifest; + }; + + template <> + struct DataMapping<Data::PackageVersion> + { + using value_t = std::shared_ptr<Repository::IPackageVersion>; + }; + + template <> + struct DataMapping<Data::Installer> + { + using value_t = std::optional<Manifest::ManifestInstaller>; + }; + + template <> + struct DataMapping<Data::HashPair> + { + using value_t = std::pair<std::vector<uint8_t>, std::vector<uint8_t>>; + }; + + template <> + struct DataMapping<Data::InstallerPath> + { + using value_t = std::filesystem::path; + }; + + template <> + struct DataMapping<Data::LogPath> + { + using value_t = std::filesystem::path; + }; + + template <> + struct DataMapping<Data::InstallerArgs> + { + using value_t = std::string; + }; + + template <> + struct DataMapping<Data::CompletionData> + { + using value_t = CLI::CompletionData; + }; + + template <> + struct DataMapping<Data::InstalledPackageVersion> + { + using value_t = std::shared_ptr<Repository::IPackageVersion>; + }; + + template <> + struct DataMapping<Data::ExecutionStage> + { + using value_t = Workflow::ExecutionStage; + }; + + template <> + struct DataMapping<Data::UninstallString> + { + using value_t = std::string; + }; + + template <> + struct DataMapping<Data::PackageFamilyNames> + { + using value_t = std::vector<Utility::LocIndString>; + }; + + template <> + struct DataMapping<Data::ProductCodes> + { + using value_t = std::vector<Utility::LocIndString>; + }; + + template <> + struct DataMapping<Data::PackageCollection> + { + using value_t = CLI::PackageCollection; + }; + + template <> + struct DataMapping<Data::PackagesToInstall> + { + using value_t = std::vector<std::shared_ptr<Repository::IPackageVersion>>; + }; + + template <> + struct DataMapping<Data::Sources> + { + using value_t = std::vector<std::shared_ptr<Repository::ISource>>; + }; + + template <> + struct DataMapping<Data::ARPSnapshot> + { + // Contains the { Id, Version, Channel } + using value_t = std::vector<std::tuple<Utility::LocIndString, Utility::LocIndString, Utility::LocIndString>>; + }; + } +} diff --git a/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp b/src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp @@ -1,19 +1,19 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. -#include "pch.h" -#include "InstallFlow.h" -#include "ImportExportFlow.h" -#include "UpdateFlow.h" -#include "PackageCollection.h" -#include "WorkflowBase.h" -#include "AppInstallerRepositorySearch.h" - -namespace AppInstaller::CLI::Workflow -{ - using namespace AppInstaller::Repository; - - namespace - { +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "InstallFlow.h" +#include "ImportExportFlow.h" +#include "UpdateFlow.h" +#include "PackageCollection.h" +#include "WorkflowBase.h" +#include "AppInstallerRepositorySearch.h" + +namespace AppInstaller::CLI::Workflow +{ + using namespace AppInstaller::Repository; + + namespace + { SourceDetails GetSourceDetails(const SourceDetails& source) { return source; @@ -33,11 +33,11 @@ namespace AppInstaller::CLI::Workflow template<class T> std::function<bool(const T&)> GetSourceDetailsEquivalencePredicate(const SourceDetails& details) { - return [&](const T& source) - { - SourceDetails sourceDetails = GetSourceDetails(source); - return sourceDetails.Type == details.Type && sourceDetails.Identifier == details.Identifier; - }; + return [&](const T& source) + { + SourceDetails sourceDetails = GetSourceDetails(source); + return sourceDetails.Type == details.Type && sourceDetails.Identifier == details.Identifier; + }; } // Finds a source equivalent to the one specified. @@ -68,245 +68,245 @@ namespace AppInstaller::CLI::Workflow return package->GetLatestAvailableVersion(); } - auto availablePackageVersion = package->GetAvailableVersion({ "", version, channel }); - if (!availablePackageVersion) - { - availablePackageVersion = package->GetLatestAvailableVersion(); - if (availablePackageVersion) - { - // Warn installed version is not available. - AICLI_LOG( - CLI, - Info, - << "Installed package version is not available." - << " Package Id [" << availablePackageVersion->GetProperty(PackageVersionProperty::Id) << "], Version [" << version << "], Channel [" << channel << "]" - << ". Found Version [" << availablePackageVersion->GetProperty(PackageVersionProperty::Version) << "], Channel [" << availablePackageVersion->GetProperty(PackageVersionProperty::Version) << "]"); - context.Reporter.Warn() - << Resource::String::InstalledPackageVersionNotAvailable - << ' ' << availablePackageVersion->GetProperty(PackageVersionProperty::Id) - << ' ' << version << ' ' << channel << std::endl; - } - } + auto availablePackageVersion = package->GetAvailableVersion({ "", version, channel }); + if (!availablePackageVersion) + { + availablePackageVersion = package->GetLatestAvailableVersion(); + if (availablePackageVersion) + { + // Warn installed version is not available. + AICLI_LOG( + CLI, + Info, + << "Installed package version is not available." + << " Package Id [" << availablePackageVersion->GetProperty(PackageVersionProperty::Id) << "], Version [" << version << "], Channel [" << channel << "]" + << ". Found Version [" << availablePackageVersion->GetProperty(PackageVersionProperty::Version) << "], Channel [" << availablePackageVersion->GetProperty(PackageVersionProperty::Version) << "]"); + context.Reporter.Warn() + << Resource::String::InstalledPackageVersionNotAvailable + << ' ' << availablePackageVersion->GetProperty(PackageVersionProperty::Id) + << ' ' << version << ' ' << channel << std::endl; + } + } return availablePackageVersion; } - } - - void SelectVersionsToExport(Execution::Context& context) - { - const auto& searchResult = context.Get<Execution::Data::SearchResult>(); - const bool includeVersions = context.Args.Contains(Execution::Args::Type::IncludeVersions); - PackageCollection exportedPackages; - exportedPackages.ClientVersion = Runtime::GetClientVersion().get(); - auto& exportedSources = exportedPackages.Sources; - for (const auto& packageMatch : searchResult.Matches) - { - auto installedPackageVersion = packageMatch.Package->GetInstalledVersion(); - auto version = installedPackageVersion->GetProperty(PackageVersionProperty::Version); - auto channel = installedPackageVersion->GetProperty(PackageVersionProperty::Channel); - - // Find an available version of this package to determine its source. - auto availablePackageVersion = GetAvailableVersionForInstalledPackage(context, packageMatch.Package, version, channel, includeVersions); - if (!availablePackageVersion) - { - // Report package not found and move to next package. - AICLI_LOG(CLI, Warning, << "No available version of package [" << installedPackageVersion->GetProperty(PackageVersionProperty::Name) << "] was found to export"); - context.Reporter.Warn() << Resource::String::InstalledPackageNotAvailable << ' ' << installedPackageVersion->GetProperty(PackageVersionProperty::Name) << std::endl; - continue; - } - - const auto& sourceDetails = availablePackageVersion->GetSource()->GetDetails(); - AICLI_LOG(CLI, Info, - << "Installed package is available. Package Id [" << availablePackageVersion->GetProperty(PackageVersionProperty::Id) << "], Source [" << sourceDetails.Identifier << "]"); - - // Find the exported source for this package - auto sourceItr = FindSource(exportedSources, sourceDetails); - if (sourceItr == exportedSources.end()) - { - exportedSources.emplace_back(sourceDetails); - sourceItr = std::prev(exportedSources.end()); - } - - // Take the Id from the available package because that is the one used in the source, - // but take the exported version from the installed package if needed. - if (includeVersions) - { - sourceItr->Packages.emplace_back( - availablePackageVersion->GetProperty(PackageVersionProperty::Id), - version.get(), - channel.get()); - } - else - { - sourceItr->Packages.emplace_back(availablePackageVersion->GetProperty(PackageVersionProperty::Id)); - } - } - - context.Add<Execution::Data::PackageCollection>(std::move(exportedPackages)); - } - - void WriteImportFile(Execution::Context& context) - { - auto packages = PackagesJson::CreateJson(context.Get<Execution::Data::PackageCollection>()); - - std::filesystem::path outputFilePath{ context.Args.GetArg(Execution::Args::Type::OutputFile) }; - std::ofstream outputFileStream{ outputFilePath }; - outputFileStream << packages; - } - - void ReadImportFile(Execution::Context& context) - { - std::ifstream importFile{ context.Args.GetArg(Execution::Args::Type::ImportFile) }; - THROW_LAST_ERROR_IF(importFile.fail()); - - Json::Value jsonRoot; - Json::CharReaderBuilder builder; - Json::String errors; - if (!Json::parseFromStream(builder, importFile, &jsonRoot, &errors)) - { - AICLI_LOG(CLI, Error, << "Failed to read JSON: " << errors); - context.Reporter.Error() << Resource::String::InvalidJsonFile << std::endl; - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_JSON_INVALID_FILE); - } - - auto packages = PackagesJson::TryParseJson(jsonRoot); - if (!packages.has_value()) - { - context.Reporter.Error() << Resource::String::InvalidJsonFile << std::endl; - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_JSON_INVALID_FILE); - } - - if (packages->Sources.empty()) - { - AICLI_LOG(CLI, Warning, << "No packages to install"); - context.Reporter.Info() << Resource::String::NoPackagesFoundInImportFile << std::endl; - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NO_APPLICATIONS_FOUND); - } - - if (context.Args.Contains(Execution::Args::Type::IgnoreVersions)) - { - // Strip out all the version information as we don't need it. - for (auto& source : packages->Sources) - { - for (auto& package : source.Packages) - { - package.VersionAndChannel = {}; - } - } - } - - context.Add<Execution::Data::PackageCollection>(packages.value()); - } - - void OpenSourcesForImport(Execution::Context& context) - { - auto availableSources = Repository::GetSources(); - for (auto& requiredSource : context.Get<Execution::Data::PackageCollection>().Sources) - { - // Find the installed source matching the one described in the collection. - AICLI_LOG(CLI, Info, << "Looking for source [" << requiredSource.Details.Identifier << "]"); - auto matchingSource = FindSource(availableSources, requiredSource.Details); - if (matchingSource != availableSources.end()) - { - requiredSource.Details.Name = matchingSource->Name; - } - else - { - AICLI_LOG(CLI, Error, << "Missing required source: " << requiredSource.Details.Name); - context.Reporter.Warn() << Resource::String::ImportSourceNotInstalled << ' ' << requiredSource.Details.Name << std::endl; - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_SOURCE_NAME_DOES_NOT_EXIST); - } - - context << Workflow::OpenNamedSourceForSources(requiredSource.Details.Name); - if (context.IsTerminated()) - { - return; - } - } - } - - void SearchPackagesForImport(Execution::Context& context) - { - const auto& sources = context.Get<Execution::Data::Sources>(); - std::vector<std::shared_ptr<IPackageVersion>> packagesToInstall = {}; - bool foundAll = true; - - // Look for the packages needed from each source independently. - // If a package is available from multiple sources, this ensures we will get it from the right one. - for (auto& requiredSource : context.Get<Execution::Data::PackageCollection>().Sources) - { - // Find the required source among the open sources. This must exist as we already found them. - auto sourceItr = FindSource(sources, requiredSource.Details); - if (sourceItr == sources.end()) - { - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INTERNAL_ERROR); - } - - // Search for all the packages in the source. - // Each search is done in a sub context to search everything regardless of previous failures. - auto source = Repository::CreateCompositeSource(context.Get<Execution::Data::Source>(), *sourceItr, CompositeSearchBehavior::AllPackages); - AICLI_LOG(CLI, Info, << "Searching for packages requested from source [" << requiredSource.Details.Identifier << "]"); - for (const auto& packageRequest : requiredSource.Packages) - { - Logging::SubExecutionTelemetryScope subExecution; - AICLI_LOG(CLI, Info, << "Searching for package [" << packageRequest.Id << "]"); - - // Search for the current package - SearchRequest searchRequest; - searchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::Id, MatchType::CaseInsensitive, packageRequest.Id)); - - auto searchContextPtr = context.Clone(); - Execution::Context& searchContext = *searchContextPtr; - searchContext.Add<Execution::Data::Source>(source); - searchContext.Add<Execution::Data::SearchResult>(source->Search(searchRequest)); - - // Find the single version we want is available - searchContext << - Workflow::EnsureOneMatchFromSearchResult(false) << - Workflow::GetManifestWithVersionFromPackage(packageRequest.VersionAndChannel) << - Workflow::GetInstalledPackageVersion; - - if (searchContext.Contains(Execution::Data::InstalledPackageVersion) && searchContext.Get<Execution::Data::InstalledPackageVersion>()) - { - searchContext << Workflow::EnsureUpdateVersionApplicable; - } - - if (searchContext.IsTerminated()) - { - if (searchContext.GetTerminationHR() == APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE) - { - AICLI_LOG(CLI, Info, << "Package is already installed: [" << packageRequest.Id << "]"); - context.Reporter.Info() << Resource::String::ImportPackageAlreadyInstalled << ' ' << packageRequest.Id << std::endl; - continue; - } - else - { - AICLI_LOG(CLI, Info, << "Package not found for import: [" << packageRequest.Id << "], Version " << packageRequest.VersionAndChannel.ToString()); - context.Reporter.Info() << Resource::String::ImportSearchFailed << ' ' << packageRequest.Id << std::endl; - - // Keep searching for the remaining packages and only fail at the end. - foundAll = false; - continue; - } - } - - packagesToInstall.push_back(std::move(searchContext.Get<Execution::Data::PackageVersion>())); - } - } - - if (!foundAll) - { - AICLI_LOG(CLI, Info, << "Could not find one or more packages for import"); - if (context.Args.Contains(Execution::Args::Type::IgnoreUnavailable)) - { - AICLI_LOG(CLI, Info, << "Ignoring unavailable packages due to command line argument"); - } - else - { - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NOT_ALL_PACKAGES_FOUND); - } - } - - context.Add<Execution::Data::PackagesToInstall>(std::move(packagesToInstall)); - } -} + } + + void SelectVersionsToExport(Execution::Context& context) + { + const auto& searchResult = context.Get<Execution::Data::SearchResult>(); + const bool includeVersions = context.Args.Contains(Execution::Args::Type::IncludeVersions); + PackageCollection exportedPackages; + exportedPackages.ClientVersion = Runtime::GetClientVersion().get(); + auto& exportedSources = exportedPackages.Sources; + for (const auto& packageMatch : searchResult.Matches) + { + auto installedPackageVersion = packageMatch.Package->GetInstalledVersion(); + auto version = installedPackageVersion->GetProperty(PackageVersionProperty::Version); + auto channel = installedPackageVersion->GetProperty(PackageVersionProperty::Channel); + + // Find an available version of this package to determine its source. + auto availablePackageVersion = GetAvailableVersionForInstalledPackage(context, packageMatch.Package, version, channel, includeVersions); + if (!availablePackageVersion) + { + // Report package not found and move to next package. + AICLI_LOG(CLI, Warning, << "No available version of package [" << installedPackageVersion->GetProperty(PackageVersionProperty::Name) << "] was found to export"); + context.Reporter.Warn() << Resource::String::InstalledPackageNotAvailable << ' ' << installedPackageVersion->GetProperty(PackageVersionProperty::Name) << std::endl; + continue; + } + + const auto& sourceDetails = availablePackageVersion->GetSource()->GetDetails(); + AICLI_LOG(CLI, Info, + << "Installed package is available. Package Id [" << availablePackageVersion->GetProperty(PackageVersionProperty::Id) << "], Source [" << sourceDetails.Identifier << "]"); + + // Find the exported source for this package + auto sourceItr = FindSource(exportedSources, sourceDetails); + if (sourceItr == exportedSources.end()) + { + exportedSources.emplace_back(sourceDetails); + sourceItr = std::prev(exportedSources.end()); + } + + // Take the Id from the available package because that is the one used in the source, + // but take the exported version from the installed package if needed. + if (includeVersions) + { + sourceItr->Packages.emplace_back( + availablePackageVersion->GetProperty(PackageVersionProperty::Id), + version.get(), + channel.get()); + } + else + { + sourceItr->Packages.emplace_back(availablePackageVersion->GetProperty(PackageVersionProperty::Id)); + } + } + + context.Add<Execution::Data::PackageCollection>(std::move(exportedPackages)); + } + + void WriteImportFile(Execution::Context& context) + { + auto packages = PackagesJson::CreateJson(context.Get<Execution::Data::PackageCollection>()); + + std::filesystem::path outputFilePath{ context.Args.GetArg(Execution::Args::Type::OutputFile) }; + std::ofstream outputFileStream{ outputFilePath }; + outputFileStream << packages; + } + + void ReadImportFile(Execution::Context& context) + { + std::ifstream importFile{ context.Args.GetArg(Execution::Args::Type::ImportFile) }; + THROW_LAST_ERROR_IF(importFile.fail()); + + Json::Value jsonRoot; + Json::CharReaderBuilder builder; + Json::String errors; + if (!Json::parseFromStream(builder, importFile, &jsonRoot, &errors)) + { + AICLI_LOG(CLI, Error, << "Failed to read JSON: " << errors); + context.Reporter.Error() << Resource::String::InvalidJsonFile << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_JSON_INVALID_FILE); + } + + auto packages = PackagesJson::TryParseJson(jsonRoot); + if (!packages.has_value()) + { + context.Reporter.Error() << Resource::String::InvalidJsonFile << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_JSON_INVALID_FILE); + } + + if (packages->Sources.empty()) + { + AICLI_LOG(CLI, Warning, << "No packages to install"); + context.Reporter.Info() << Resource::String::NoPackagesFoundInImportFile << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NO_APPLICATIONS_FOUND); + } + + if (context.Args.Contains(Execution::Args::Type::IgnoreVersions)) + { + // Strip out all the version information as we don't need it. + for (auto& source : packages->Sources) + { + for (auto& package : source.Packages) + { + package.VersionAndChannel = {}; + } + } + } + + context.Add<Execution::Data::PackageCollection>(packages.value()); + } + + void OpenSourcesForImport(Execution::Context& context) + { + auto availableSources = Repository::GetSources(); + for (auto& requiredSource : context.Get<Execution::Data::PackageCollection>().Sources) + { + // Find the installed source matching the one described in the collection. + AICLI_LOG(CLI, Info, << "Looking for source [" << requiredSource.Details.Identifier << "]"); + auto matchingSource = FindSource(availableSources, requiredSource.Details); + if (matchingSource != availableSources.end()) + { + requiredSource.Details.Name = matchingSource->Name; + } + else + { + AICLI_LOG(CLI, Error, << "Missing required source: " << requiredSource.Details.Name); + context.Reporter.Warn() << Resource::String::ImportSourceNotInstalled << ' ' << requiredSource.Details.Name << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_SOURCE_NAME_DOES_NOT_EXIST); + } + + context << Workflow::OpenNamedSourceForSources(requiredSource.Details.Name); + if (context.IsTerminated()) + { + return; + } + } + } + + void SearchPackagesForImport(Execution::Context& context) + { + const auto& sources = context.Get<Execution::Data::Sources>(); + std::vector<std::shared_ptr<IPackageVersion>> packagesToInstall = {}; + bool foundAll = true; + + // Look for the packages needed from each source independently. + // If a package is available from multiple sources, this ensures we will get it from the right one. + for (auto& requiredSource : context.Get<Execution::Data::PackageCollection>().Sources) + { + // Find the required source among the open sources. This must exist as we already found them. + auto sourceItr = FindSource(sources, requiredSource.Details); + if (sourceItr == sources.end()) + { + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INTERNAL_ERROR); + } + + // Search for all the packages in the source. + // Each search is done in a sub context to search everything regardless of previous failures. + auto source = Repository::CreateCompositeSource(context.Get<Execution::Data::Source>(), *sourceItr, CompositeSearchBehavior::AllPackages); + AICLI_LOG(CLI, Info, << "Searching for packages requested from source [" << requiredSource.Details.Identifier << "]"); + for (const auto& packageRequest : requiredSource.Packages) + { + Logging::SubExecutionTelemetryScope subExecution; + AICLI_LOG(CLI, Info, << "Searching for package [" << packageRequest.Id << "]"); + + // Search for the current package + SearchRequest searchRequest; + searchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::Id, MatchType::CaseInsensitive, packageRequest.Id.get())); + + auto searchContextPtr = context.Clone(); + Execution::Context& searchContext = *searchContextPtr; + searchContext.Add<Execution::Data::Source>(source); + searchContext.Add<Execution::Data::SearchResult>(source->Search(searchRequest)); + + // Find the single version we want is available + searchContext << + Workflow::EnsureOneMatchFromSearchResult(false) << + Workflow::GetManifestWithVersionFromPackage(packageRequest.VersionAndChannel) << + Workflow::GetInstalledPackageVersion; + + if (searchContext.Contains(Execution::Data::InstalledPackageVersion) && searchContext.Get<Execution::Data::InstalledPackageVersion>()) + { + searchContext << Workflow::EnsureUpdateVersionApplicable; + } + + if (searchContext.IsTerminated()) + { + if (searchContext.GetTerminationHR() == APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE) + { + AICLI_LOG(CLI, Info, << "Package is already installed: [" << packageRequest.Id << "]"); + context.Reporter.Info() << Resource::String::ImportPackageAlreadyInstalled << ' ' << packageRequest.Id << std::endl; + continue; + } + else + { + AICLI_LOG(CLI, Info, << "Package not found for import: [" << packageRequest.Id << "], Version " << packageRequest.VersionAndChannel.ToString()); + context.Reporter.Info() << Resource::String::ImportSearchFailed << ' ' << packageRequest.Id << std::endl; + + // Keep searching for the remaining packages and only fail at the end. + foundAll = false; + continue; + } + } + + packagesToInstall.push_back(std::move(searchContext.Get<Execution::Data::PackageVersion>())); + } + } + + if (!foundAll) + { + AICLI_LOG(CLI, Info, << "Could not find one or more packages for import"); + if (context.Args.Contains(Execution::Args::Type::IgnoreUnavailable)) + { + AICLI_LOG(CLI, Info, << "Ignoring unavailable packages due to command line argument"); + } + else + { + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NOT_ALL_PACKAGES_FOUND); + } + } + + context.Add<Execution::Data::PackagesToInstall>(std::move(packagesToInstall)); + } +} diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -18,6 +18,25 @@ namespace AppInstaller::CLI::Workflow using namespace AppInstaller::Manifest; using namespace AppInstaller::Repository; + namespace + { + bool MightWriteToARP(InstallerTypeEnum type) + { + switch (type) + { + case InstallerTypeEnum::Exe: + case InstallerTypeEnum::Burn: + case InstallerTypeEnum::Inno: + case InstallerTypeEnum::Msi: + case InstallerTypeEnum::Nullsoft: + case InstallerTypeEnum::Wix: + return true; + default: + return false; + } + } + } + void EnsureApplicableInstaller(Execution::Context& context) { const auto& installer = context.Get<Execution::Data::Installer>(); @@ -357,20 +376,30 @@ namespace AppInstaller::CLI::Workflow } } - void InstallPackageVersion(Execution::Context& context) + void InstallPackageInstaller(Execution::Context& context) { context << - Workflow::SelectInstaller << - Workflow::EnsureApplicableInstaller << + Workflow::ReportManifestIdentity << Workflow::ShowInstallationDisclaimer << Workflow::ReportExecutionStage(ExecutionStage::Download) << Workflow::DownloadInstaller << + Workflow::ReportExecutionStage(ExecutionStage::PreExecution) << + Workflow::SnapshotARPEntries << Workflow::ReportExecutionStage(ExecutionStage::Execution) << Workflow::ExecuteInstaller << Workflow::ReportExecutionStage(ExecutionStage::PostExecution) << + Workflow::ReportARPChanges << Workflow::RemoveInstaller; } + void InstallPackageVersion(Execution::Context& context) + { + context << + Workflow::SelectInstaller << + Workflow::EnsureApplicableInstaller << + Workflow::InstallPackageInstaller; + } + void InstallMultiple(Execution::Context& context) { bool allSucceeded = true; @@ -399,4 +428,213 @@ namespace AppInstaller::CLI::Workflow AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_IMPORT_INSTALL_FAILED); } } + + void SnapshotARPEntries(Execution::Context& context) try + { + // Ensure that installer type might actually write to ARP, otherwise this is a waste of time + auto installer = context.Get<Execution::Data::Installer>(); + + if (installer && MightWriteToARP(installer->InstallerType)) + { + std::shared_ptr<ISource> arpSource = context.Reporter.ExecuteWithProgress( + [](IProgressCallback& progress) + { + return Repository::OpenPredefinedSource(PredefinedSource::ARP, progress); + }, true); + + std::vector<std::tuple<Utility::LocIndString, Utility::LocIndString, Utility::LocIndString>> entries; + + for (const auto& entry : arpSource->Search({}).Matches) + { + auto installed = entry.Package->GetInstalledVersion(); + entries.emplace_back(std::make_tuple( + entry.Package->GetProperty(PackageProperty::Id), + installed->GetProperty(PackageVersionProperty::Version), + installed->GetProperty(PackageVersionProperty::Channel))); + } + + std::sort(entries.begin(), entries.end()); + + context.Add<Execution::Data::ARPSnapshot>(std::move(entries)); + } + } + CATCH_LOG() + + void ReportARPChanges(Execution::Context& context) try + { + if (context.Contains(Execution::Data::ARPSnapshot)) + { + const auto& entries = context.Get<Execution::Data::ARPSnapshot>(); + + // Open it again to get the (potentially) changed ARP entries + std::shared_ptr<ISource> arpSource = context.Reporter.ExecuteWithProgress( + [](IProgressCallback& progress) + { + return Repository::OpenPredefinedSource(PredefinedSource::ARP, progress); + }, true); + + std::vector<ResultMatch> changes; + + for (auto& entry : arpSource->Search({}).Matches) + { + auto installed = entry.Package->GetInstalledVersion(); + auto entryKey = std::make_tuple( + entry.Package->GetProperty(PackageProperty::Id), + installed->GetProperty(PackageVersionProperty::Version), + installed->GetProperty(PackageVersionProperty::Channel)); + + auto itr = std::lower_bound(entries.begin(), entries.end(), entryKey); + if (itr == entries.end() || *itr != entryKey) + { + changes.emplace_back(std::move(entry)); + } + } + + // Also attempt to find the entry based on the manifest data + const auto& manifest = context.Get<Execution::Data::Manifest>(); + + SearchRequest nameAndPublisherRequest; + + // The default localization must contain the name or we cannot do this lookup + if (manifest.DefaultLocalization.Contains(Localization::PackageName)) + { + AppInstaller::Manifest::Manifest::string_t defaultName = manifest.DefaultLocalization.Get<Localization::PackageName>(); + AppInstaller::Manifest::Manifest::string_t defaultPublisher; + if (manifest.DefaultLocalization.Contains(Localization::Publisher)) + { + defaultPublisher = manifest.DefaultLocalization.Get<Localization::Publisher>(); + } + + nameAndPublisherRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact, defaultName, defaultPublisher)); + + for (const auto& loc : manifest.Localizations) + { + if (loc.Contains(Localization::PackageName) || loc.Contains(Localization::Publisher)) + { + nameAndPublisherRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact, + loc.Contains(Localization::PackageName) ? loc.Get<Localization::PackageName>() : defaultName, + loc.Contains(Localization::Publisher) ? loc.Get<Localization::Publisher>() : defaultPublisher)); + } + } + } + + std::vector<std::string> productCodes; + for (const auto& installer : manifest.Installers) + { + if (!installer.ProductCode.empty()) + { + if (std::find(productCodes.begin(), productCodes.end(), installer.ProductCode) == productCodes.end()) + { + nameAndPublisherRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::ProductCode, MatchType::Exact, installer.ProductCode)); + productCodes.emplace_back(installer.ProductCode); + } + } + } + + SearchResult findByManifest; + + // Don't execute this search if it would just find everything + if (!nameAndPublisherRequest.IsForEverything()) + { + findByManifest = arpSource->Search(nameAndPublisherRequest); + } + + // Cross reference the changes with the search results + std::vector<std::shared_ptr<IPackage>> packagesInBoth; + + for (const auto& change : changes) + { + for (const auto& byManifest : findByManifest.Matches) + { + if (change.Package->IsSame(byManifest.Package.get())) + { + packagesInBoth.emplace_back(change.Package); + break; + } + } + } + + // We now have all of the package changes; time to report them. + // The set of cases we could have for changes to ARP: + // 0 packages :: No changes were detected to ARP, which could mean that the installer + // did not write an entry. It could also be a forced reinstall. + // 1 package :: Golden path; this should be what we installed. + // 2+ packages :: We need to determine which package actually matches the one that we + // were installing. + // + // The set of cases we could have for finding packages based on the manifest: + // 0 packages :: The manifest data does not match the ARP information. + // 1 package :: Golden path; this should be what we installed. + // 2+ packages :: The data in the manifest is either too broad or we have + // a problem with our name normalization. + // + // ARP Package changes + // 0 1 N + // +------------------+--------------------+--------------------+ + // M | | | | + // a | Package does not | Manifest data does | Manifest data does | + // n 0 | write to ARP | not match ARP | not match ARP | + // i | Log this fact | Log for fixup | Log for fixup | + // f | | | | + // e +------------------+--------------------+--------------------+ + // s | | | | + // t | Reinstall of | Golden Path! | Treat manifest as | + // 1 | existing version | (assuming match) | main if common | + // r | | | | + // e +------------------+--------------------+--------------------+ + // s | | | | + // u | Not expected | Treat ARP as main | Not expected | + // l N | Log this for | | Log this for | + // t | investigation | | investigation | + // s | | | | + // +------------------+--------------------+--------------------+ + + // Find the package that we are going to log + std::shared_ptr<IPackageVersion> toLog; + + // If no changes found, only log if a single matching package was found by the manifest + if (changes.empty() && findByManifest.Matches.size() == 1) + { + toLog = findByManifest.Matches[0].Package->GetInstalledVersion(); + } + // If only a single ARP entry was changed, always log that + else if (changes.size() == 1) + { + toLog = changes[0].Package->GetInstalledVersion(); + } + // Finally, if there is only a single common package, log that one + else if (packagesInBoth.size() == 1) + { + toLog = packagesInBoth[0]->GetInstalledVersion(); + } + + IPackageVersion::Metadata toLogMetadata; + if (toLog) + { + toLogMetadata = toLog->GetMetadata(); + } + + // We can only get the source identifier from an active source + std::string sourceIdentifier; + if (context.Contains(Execution::Data::PackageVersion)) + { + sourceIdentifier = context.Get<Execution::Data::PackageVersion>()->GetProperty(PackageVersionProperty::SourceIdentifier); + } + + Logging::Telemetry().LogSuccessfulInstallARPChange( + sourceIdentifier, + manifest.Id, + manifest.Version, + manifest.Channel, + changes.size(), + findByManifest.Matches.size(), + packagesInBoth.size(), + toLog ? static_cast<std::string_view>(toLog->GetProperty(PackageVersionProperty::Name)) : "", + toLog ? static_cast<std::string_view>(toLog->GetProperty(PackageVersionProperty::Version)) : "", + toLog ? static_cast<std::string_view>(toLogMetadata[PackageVersionMetadata::Publisher]) : "", + toLog ? static_cast<std::string_view>(toLogMetadata[PackageVersionMetadata::Locale]) : "" + ); + } + } + CATCH_LOG() } diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.h b/src/AppInstallerCLICore/Workflows/InstallFlow.h @@ -77,10 +77,16 @@ namespace AppInstaller::CLI::Workflow // Outputs: None void RemoveInstaller(Execution::Context& context); - // Installs a single package from its manifest + // Installs a specific package installer. + // Required Args: None + // Inputs: Manifest, Installer + // Outputs: None + void InstallPackageInstaller(Execution::Context& context); + + // Installs a specific package version. // Required Args: None // Inputs: Manifest, PackageVersion, Source - // Outputs: Manifest + // Outputs: None void InstallPackageVersion(Execution::Context& context); // Installs multiple packages. @@ -88,4 +94,16 @@ namespace AppInstaller::CLI::Workflow // Inputs: Manifests // Outputs: None void InstallMultiple(Execution::Context& context); + + // Stores the existing set of packages in ARP. + // Required Args: None + // Inputs: Installer + // Outputs: ARPSnapshot + void SnapshotARPEntries(Execution::Context& context); + + // Reports on the changes between the stored ARPSnapshot and the current values. + // Required Args: None + // Inputs: ARPSnapshot?, Manifest, PackageVersion + // Outputs: None + void ReportARPChanges(Execution::Context& context); } diff --git a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp @@ -110,15 +110,7 @@ namespace AppInstaller::CLI::Workflow updateAllFoundUpdate = true; - updateContext << - ReportManifestIdentity << - ShowInstallationDisclaimer << - Workflow::ReportExecutionStage(ExecutionStage::Download) << - DownloadInstaller << - Workflow::ReportExecutionStage(ExecutionStage::Execution) << - ExecuteInstaller << - Workflow::ReportExecutionStage(ExecutionStage::PostExecution) << - RemoveInstaller; + updateContext << InstallPackageInstaller; updateContext.Reporter.Info() << std::endl; diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h @@ -22,6 +22,7 @@ namespace AppInstaller::CLI::Workflow ParseArgs = 1000, Discovery = 2000, Download = 3000, + PreExecution = 3500, Execution = 4000, PostExecution = 5000, }; diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestExeInstaller.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestExeInstaller.yaml @@ -15,6 +15,6 @@ Installers: Silent: /exesilent Interactive: /exeinteractive Language: /exeenus - Log: /exelog <LOGPATH> + Log: /LogFile <LOGPATH> InstallLocation: /InstallDir <INSTALLPATH> ManifestVersion: 0.1.0 diff --git a/src/AppInstallerCLITests/ARPChanges.cpp b/src/AppInstallerCLITests/ARPChanges.cpp @@ -0,0 +1,424 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "TestCommon.h" +#include "TestSource.h" +#include "TestHooks.h" +#include <Workflows/WorkflowBase.h> +#include <Workflows/InstallFlow.h> +#include <winget/Manifest.h> +#include <Microsoft/PredefinedInstalledSourceFactory.h> + +using namespace TestCommon; +using namespace AppInstaller; +using namespace AppInstaller::CLI; +using namespace AppInstaller::CLI::Execution; +using namespace AppInstaller::CLI::Workflow; +using namespace AppInstaller::Logging; + +struct TestTelemetry : public TelemetryTraceLogger +{ + void LogSuccessfulInstallARPChange( + std::string_view sourceIdentifier, + std::string_view packageIdentifier, + std::string_view packageVersion, + std::string_view packageChannel, + size_t changesToARP, + size_t matchesInARP, + size_t countOfIntersectionOfChangesAndMatches, + std::string_view arpName, + std::string_view arpVersion, + std::string_view arpPublisher, + std::string_view arpLanguage) const noexcept override + { + WasLogSuccessfulInstallARPChangeCalled = true; + if (OnLogSuccessfulInstallARPChange) + { + OnLogSuccessfulInstallARPChange( + sourceIdentifier, packageIdentifier, packageVersion, packageChannel, + changesToARP, matchesInARP, countOfIntersectionOfChangesAndMatches, + arpName, arpVersion, arpPublisher, arpLanguage); + } + } + + std::function<void( + std::string_view, std::string_view, std::string_view, std::string_view, + size_t, size_t, size_t, + std::string_view, std::string_view, std::string_view, std::string_view)> OnLogSuccessfulInstallARPChange; + + mutable bool WasLogSuccessfulInstallARPChangeCalled = false; +}; + +struct TestContext : public Context +{ + TestContext(Manifest::InstallerTypeEnum installerType = Manifest::InstallerTypeEnum::Exe) : + Context(OStream, IStream), SourceFactory([this](const SourceDetails&) { return Source; }) + { + // Put installer in to control whether arp change code cares to run + Manifest::ManifestInstaller installer; + installer.InstallerType = installerType; + Add<Data::Installer>(std::move(installer)); + + // Put in an empty manifest by default + Manifest::Manifest manifest; + manifest.Id = "Installing.Id"; + manifest.Version = "Installing.Version"; + manifest.Channel = "Installing.Channel"; + manifest.DefaultLocalization.Add<Manifest::Localization::PackageName>("Installing.Name"); + Add<Data::Manifest>(std::move(manifest)); + + // Set up logger to intercept event + Logger = std::make_shared<TestTelemetry>(); + TestHook_SetTelemetryOverride(Logger); + + Logger->OnLogSuccessfulInstallARPChange = [this]( + std::string_view sourceIdentifier, + std::string_view packageIdentifier, + std::string_view packageVersion, + std::string_view packageChannel, + size_t changesToARP, + size_t matchesInARP, + size_t countOfIntersectionOfChangesAndMatches, + std::string_view arpName, + std::string_view arpVersion, + std::string_view arpPublisher, + std::string_view arpLanguage) + { + SourceIdentifier = sourceIdentifier; + PackageIdentifier = packageIdentifier; + PackageVersion = packageVersion; + PackageChannel = packageChannel; + ChangesToARP = changesToARP; + MatchesInARP = matchesInARP; + CountOfIntersectionOfChangesAndMatches = countOfIntersectionOfChangesAndMatches; + ARPName = arpName; + ARPVersion = arpVersion; + ARPPublisher = arpPublisher; + ARPLanguage = arpLanguage; + }; + + // Inject our source + TestHook_SetSourceFactoryOverride(std::string{ Repository::Microsoft::PredefinedInstalledSourceFactory::Type() }, SourceFactory); + + Source = std::make_shared<TestSource>(); + Source->SearchFunction = [&](const SearchRequest& request) + { + return request.IsForEverything() ? EverythingResult : MatchResult; + }; + + // The package version is used to get the source identifier + Add<Data::PackageVersion>(TestPackageVersion::Make(Get<Data::Manifest>(), Source)); + + // Populate everything result with a few items + AddEverythingResult("Id1", "Name1", "Publisher1", "1.0"); + AddEverythingResult("Id2", "Name2", "Publisher2", "2.0"); + } + + ~TestContext() + { + TestHook_ClearSourceFactoryOverrides(); + TestHook_SetTelemetryOverride({}); + } + + void AddEverythingResult(std::string_view id, std::string_view name, std::string_view publisher, std::string_view version) + { + AddResult(EverythingResult, id, name, publisher, version); + } + + void AddMatchResult(std::string_view id, std::string_view name, std::string_view publisher, std::string_view version) + { + AddResult(MatchResult, id, name, publisher, version); + } + + void ExpectEvent(size_t arpChanges, size_t matches, size_t overlap, IPackage* arpEntry = nullptr) + { + REQUIRE(Logger->WasLogSuccessfulInstallARPChangeCalled); + + const auto& manifest = Get<Data::Manifest>(); + + REQUIRE(Source->GetIdentifier() == SourceIdentifier); + REQUIRE(manifest.Id == PackageIdentifier); + REQUIRE(manifest.Version == PackageVersion); + REQUIRE(manifest.Channel == PackageChannel); + REQUIRE(arpChanges == ChangesToARP); + REQUIRE(matches == MatchesInARP); + REQUIRE(overlap == CountOfIntersectionOfChangesAndMatches); + + if (arpEntry) + { + auto version = arpEntry->GetInstalledVersion(); + REQUIRE(version->GetProperty(PackageVersionProperty::Name) == ARPName); + REQUIRE(version->GetProperty(PackageVersionProperty::Version) == ARPVersion); + + auto metadata = version->GetMetadata(); + REQUIRE(metadata[PackageVersionMetadata::Publisher] == ARPPublisher); + REQUIRE(metadata[PackageVersionMetadata::Locale] == ARPLanguage); + } + else + { + REQUIRE(ARPName.empty()); + REQUIRE(ARPVersion.empty()); + REQUIRE(ARPPublisher.empty()); + REQUIRE(ARPLanguage.empty()); + } + } + + std::ostringstream OStream; + std::istringstream IStream; + std::shared_ptr<TestTelemetry> Logger; + TestSourceFactory SourceFactory; + std::shared_ptr<TestSource> Source; + SearchResult EverythingResult; + SearchResult MatchResult; + + // EventData + std::string SourceIdentifier; + std::string PackageIdentifier; + std::string PackageVersion; + std::string PackageChannel; + size_t ChangesToARP; + size_t MatchesInARP; + size_t CountOfIntersectionOfChangesAndMatches; + std::string ARPName; + std::string ARPVersion; + std::string ARPPublisher; + std::string ARPLanguage; + + private: + void AddResult(SearchResult& result, std::string_view id, std::string_view name, std::string_view publisher, std::string_view version) + { + PackageMatchFilter defaultFilter{ PackageMatchField::Id, MatchType::Exact }; + Manifest::Manifest manifest; + + manifest.Id = id; + manifest.DefaultLocalization.Add<Manifest::Localization::PackageName>(name); + manifest.DefaultLocalization.Add<Manifest::Localization::Publisher>(publisher); + manifest.Version = version; + manifest.Installers.push_back({}); + + TestPackage::MetadataMap metadata; + metadata[PackageVersionMetadata::Publisher] = publisher; + + result.Matches.emplace_back(TestPackage::Make(manifest, std::move(metadata), std::vector<Manifest::Manifest>{}, Source), defaultFilter); + } +}; + + +TEST_CASE("ARPChanges_MSIX_Ignored", "[ARPChanges][workflow]") +{ + TestContext context(Manifest::InstallerTypeEnum::Msix); + + context << SnapshotARPEntries; + + REQUIRE(!context.Contains(Data::ARPSnapshot)); + + context << ReportARPChanges; + + REQUIRE(!context.Logger->WasLogSuccessfulInstallARPChangeCalled); +} + +TEST_CASE("ARPChanges_CheckSnapshot", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + + REQUIRE(context.Contains(Data::ARPSnapshot)); + + auto snapshot = context.Get<Data::ARPSnapshot>(); + + REQUIRE(context.EverythingResult.Matches.size() == snapshot.size()); + + // Destructively match + for (const auto& match : context.EverythingResult.Matches) + { + bool found = false; + for (auto itr = snapshot.begin(); itr != snapshot.end(); ++itr) + { + if (match.Package->GetProperty(PackageProperty::Id) == std::get<0>(*itr)) + { + REQUIRE(match.Package->GetInstalledVersion()->GetProperty(PackageVersionProperty::Version) == std::get<1>(*itr)); + REQUIRE(match.Package->GetInstalledVersion()->GetProperty(PackageVersionProperty::Channel) == std::get<2>(*itr)); + + snapshot.erase(itr); + found = true; + break; + } + } + REQUIRE(found); + } + + REQUIRE(snapshot.empty()); +} + +TEST_CASE("ARPChanges_NoChange_NoMatch", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + REQUIRE(context.Contains(Data::ARPSnapshot)); + + context << ReportARPChanges; + context.ExpectEvent(0, 0, 0); +} + +TEST_CASE("ARPChanges_NoChange_SingleMatch", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + REQUIRE(context.Contains(Data::ARPSnapshot)); + + context.AddMatchResult("MatchId1", "MatchName1", "MatchPublisher1", "MatchVersion1"); + + context << ReportARPChanges; + context.ExpectEvent(0, 1, 0, context.MatchResult.Matches[0].Package.get()); +} + +TEST_CASE("ARPChanges_NoChange_MultiMatch", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + REQUIRE(context.Contains(Data::ARPSnapshot)); + + context.AddMatchResult("MatchId1", "MatchName1", "MatchPublisher1", "MatchVersion1"); + context.AddMatchResult("MatchId2", "MatchName2", "MatchPublisher2", "MatchVersion2"); + + context << ReportARPChanges; + context.ExpectEvent(0, 2, 0); +} + +TEST_CASE("ARPChanges_SingleChange_NoMatch", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + REQUIRE(context.Contains(Data::ARPSnapshot)); + + context.AddEverythingResult("EverythingId1", "EverythingName1", "EverythingPublisher1", "EverythingVersion1"); + + context << ReportARPChanges; + context.ExpectEvent(1, 0, 0, context.EverythingResult.Matches.back().Package.get()); +} + +TEST_CASE("ARPChanges_SingleChange_SingleMatch", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + REQUIRE(context.Contains(Data::ARPSnapshot)); + + context.AddEverythingResult("EverythingId1", "EverythingName1", "EverythingPublisher1", "EverythingVersion1"); + context.AddMatchResult("MatchId1", "MatchName1", "MatchPublisher1", "MatchVersion1"); + + context << ReportARPChanges; + context.ExpectEvent(1, 1, 0, context.EverythingResult.Matches.back().Package.get()); +} + +TEST_CASE("ARPChanges_SingleChange_MultiMatch", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + REQUIRE(context.Contains(Data::ARPSnapshot)); + + context.AddEverythingResult("EverythingId1", "EverythingName1", "EverythingPublisher1", "EverythingVersion1"); + context.AddMatchResult("MatchId1", "MatchName1", "MatchPublisher1", "MatchVersion1"); + context.MatchResult.Matches.emplace_back(context.EverythingResult.Matches.back()); + + context << ReportARPChanges; + context.ExpectEvent(1, 2, 1, context.EverythingResult.Matches.back().Package.get()); +} + +TEST_CASE("ARPChanges_MultiChange_NoMatch", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + REQUIRE(context.Contains(Data::ARPSnapshot)); + + context.AddEverythingResult("EverythingId1", "EverythingName1", "EverythingPublisher1", "EverythingVersion1"); + context.AddEverythingResult("EverythingId2", "EverythingName2", "EverythingPublisher2", "EverythingVersion2"); + + context << ReportARPChanges; + context.ExpectEvent(2, 0, 0); +} + +TEST_CASE("ARPChanges_MultiChange_SingleMatch_NoOverlap", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + REQUIRE(context.Contains(Data::ARPSnapshot)); + + context.AddEverythingResult("EverythingId1", "EverythingName1", "EverythingPublisher1", "EverythingVersion1"); + context.AddEverythingResult("EverythingId2", "EverythingName2", "EverythingPublisher2", "EverythingVersion2"); + context.AddMatchResult("MatchId1", "MatchName1", "MatchPublisher1", "MatchVersion1"); + + context << ReportARPChanges; + context.ExpectEvent(2, 1, 0); +} + +TEST_CASE("ARPChanges_MultiChange_SingleMatch_Overlap", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + REQUIRE(context.Contains(Data::ARPSnapshot)); + + context.AddEverythingResult("EverythingId1", "EverythingName1", "EverythingPublisher1", "EverythingVersion1"); + context.AddEverythingResult("EverythingId2", "EverythingName2", "EverythingPublisher2", "EverythingVersion2"); + context.MatchResult.Matches.emplace_back(context.EverythingResult.Matches.back()); + + context << ReportARPChanges; + context.ExpectEvent(2, 1, 1, context.MatchResult.Matches.back().Package.get()); +} + +TEST_CASE("ARPChanges_MultiChange_MultiMatch_NoOverlap", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + REQUIRE(context.Contains(Data::ARPSnapshot)); + + context.AddEverythingResult("EverythingId1", "EverythingName1", "EverythingPublisher1", "EverythingVersion1"); + context.AddEverythingResult("EverythingId2", "EverythingName2", "EverythingPublisher2", "EverythingVersion2"); + context.AddMatchResult("MatchId1", "MatchName1", "MatchPublisher1", "MatchVersion1"); + context.AddMatchResult("MatchId2", "MatchName2", "MatchPublisher2", "MatchVersion2"); + + context << ReportARPChanges; + context.ExpectEvent(2, 2, 0); +} + +TEST_CASE("ARPChanges_MultiChange_MultiMatch_SingleOverlap", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + REQUIRE(context.Contains(Data::ARPSnapshot)); + + context.AddEverythingResult("EverythingId1", "EverythingName1", "EverythingPublisher1", "EverythingVersion1"); + context.AddEverythingResult("EverythingId2", "EverythingName2", "EverythingPublisher2", "EverythingVersion2"); + context.AddMatchResult("MatchId1", "MatchName1", "MatchPublisher1", "MatchVersion1"); + context.MatchResult.Matches.emplace_back(context.EverythingResult.Matches.back()); + + context << ReportARPChanges; + context.ExpectEvent(2, 2, 1, context.MatchResult.Matches.back().Package.get()); +} + +TEST_CASE("ARPChanges_MultiChange_MultiMatch_MultiOverlap", "[ARPChanges][workflow]") +{ + TestContext context; + + context << SnapshotARPEntries; + REQUIRE(context.Contains(Data::ARPSnapshot)); + + context.AddEverythingResult("EverythingId1", "EverythingName1", "EverythingPublisher1", "EverythingVersion1"); + context.MatchResult.Matches.emplace_back(context.EverythingResult.Matches.back()); + context.AddEverythingResult("EverythingId2", "EverythingName2", "EverythingPublisher2", "EverythingVersion2"); + context.MatchResult.Matches.emplace_back(context.EverythingResult.Matches.back()); + + context << ReportARPChanges; + context.ExpectEvent(2, 2, 2); +} diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -179,6 +179,7 @@ <ClInclude Include="TestSource.h" /> </ItemGroup> <ItemGroup> + <ClCompile Include="ARPChanges.cpp" /> <ClCompile Include="Command.cpp" /> <ClCompile Include="Completion.cpp" /> <ClCompile Include="CompositeSource.cpp" /> diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -122,6 +122,9 @@ <ClCompile Include="Regex.cpp"> <Filter>Source Files</Filter> </ClCompile> + <ClCompile Include="ARPChanges.cpp"> + <Filter>Source Files</Filter> + </ClCompile> </ItemGroup> <ItemGroup> <None Include="PropertySheet.props" /> diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp @@ -27,13 +27,9 @@ struct ComponentTestSource : public TestSource { return Everything; } - else if (SearchFunction) - { - return SearchFunction(request); - } else { - return {}; + return TestSource::Search(request); } } @@ -561,3 +557,18 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_ReverseMatchBoth", "[Composi REQUIRE(result.Matches[0].Package->GetInstalledVersion()); REQUIRE(result.Matches[0].Package->GetAvailableVersionKeys().size() == 1); } + +TEST_CASE("CompositeSource_IsSame", "[CompositeSource]") +{ + CompositeTestSetup setup; + setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPFN("sortof_apfn")), Criteria()); + setup.Available->Everything.Matches.emplace_back(MakeAvailable(WithPFN("sortof_apfn")), Criteria()); + + SearchResult result1 = setup.Search(); + REQUIRE(result1.Matches.size() == 1); + + SearchResult result2 = setup.Search(); + REQUIRE(result2.Matches.size() == 1); + + REQUIRE(result1.Matches[0].Package->IsSame(result2.Matches[0].Package.get())); +} diff --git a/src/AppInstallerCLITests/PredefinedInstalledSource.cpp b/src/AppInstallerCLITests/PredefinedInstalledSource.cpp @@ -115,7 +115,6 @@ void VerifyMetadataString(const SQLiteIndex::MetadataResult& metadata, PackageVe void VerifyEntryAgainstIndex(const SQLiteIndex& index, SQLiteIndex::IdType manifestId, const ARPEntry& entry) { - REQUIRE(index.GetPropertyByManifestId(manifestId, PackageVersionProperty::Id) == entry.EntryName); REQUIRE(index.GetPropertyByManifestId(manifestId, PackageVersionProperty::Name) == entry.DisplayName); REQUIRE(index.GetPropertyByManifestId(manifestId, PackageVersionProperty::Version) == entry.DisplayVersion); @@ -128,6 +127,7 @@ void VerifyEntryAgainstIndex(const SQLiteIndex& index, SQLiteIndex::IdType manif VerifyInstalledType(metadata, entry.WindowsInstaller.value_or(false) ? InstallerTypeEnum::Msi : InstallerTypeEnum::Exe); VerifyTestScope(metadata); + VerifyMetadataString(metadata, PackageVersionMetadata::Publisher, entry.Publisher); VerifyMetadataString(metadata, PackageVersionMetadata::InstalledLocation, entry.InstallLocation); VerifyMetadataString(metadata, PackageVersionMetadata::StandardUninstallCommand, entry.UninstallString); VerifyMetadataString(metadata, PackageVersionMetadata::SilentUninstallCommand, entry.QuietUninstallString); @@ -246,7 +246,7 @@ TEST_CASE("ARPHelper_DetermineVersion_Version", "[arphelper][list]") SetRegistryValue(root.get(), helper.VersionMinor, 14); auto result = helper.DetermineVersion(key); - REQUIRE(result == "2.7.42"); + REQUIRE(result == "3.14"); } TEST_CASE("ARPHelper_DetermineVersion_VersionMajorMinor", "[arphelper][list]") diff --git a/src/AppInstallerCLITests/SQLiteIndex.cpp b/src/AppInstallerCLITests/SQLiteIndex.cpp @@ -32,7 +32,7 @@ SQLiteIndex CreateTestIndex(const std::string& filePath, std::optional<Schema::V // If no specific version requested, then use generator to run against all versions. if (!version) { - version = GENERATE(Schema::Version{ 1, 0 }, Schema::Version::Latest()); + version = GENERATE(Schema::Version{ 1, 0 }, Schema::Version{ 1, 1 }, Schema::Version::Latest()); } return SQLiteIndex::CreateNew(filePath, version.value()); @@ -56,6 +56,16 @@ Schema::Version TestPrepareForRead(SQLiteIndex& index) return { 1, 0 }; } } + else if (index.GetVersion() == Schema::Version{ 1, 2 }) + { + Schema::Version version = GENERATE(Schema::Version{ 1, 0 }, Schema::Version{ 1, 1 }, Schema::Version{ 1, 2 }); + + if (version != Schema::Version{ 1, 2 }) + { + index.ForceVersion(version); + return version; + } + } return index.GetVersion(); } @@ -126,8 +136,35 @@ struct IndexFields ProductCodes(std::move(productCodes)) {} + IndexFields( + std::string id, + std::string name, + std::string publisher, + std::string moniker, + std::string version, + std::string channel, + std::vector<NormalizedString> tags, + std::vector<NormalizedString> commands, + std::string path, + std::vector<NormalizedString> packageFamilyNames, + std::vector<NormalizedString> productCodes + ) : + Id(std::move(id)), + Name(std::move(name)), + Publisher(std::move(publisher)), + Moniker(std::move(moniker)), + Version(std::move(version)), + Channel(std::move(channel)), + Tags(std::move(tags)), + Commands(std::move(commands)), + Path(std::move(path)), + PackageFamilyNames(std::move(packageFamilyNames)), + ProductCodes(std::move(productCodes)) + {} + std::string Id; std::string Name; + std::string Publisher; std::string Moniker; std::string Version; std::string Channel; @@ -148,6 +185,7 @@ SQLiteIndex SearchTestSetup(const std::string& filePath, std::initializer_list<I { manifest.Id = d.Id; manifest.DefaultLocalization.Add<Localization::PackageName>(d.Name); + manifest.DefaultLocalization.Add<Localization::Publisher>(d.Publisher); manifest.Moniker = d.Moniker; manifest.Version = d.Version; manifest.DefaultLocalization.Add<Localization::Tags>(d.Tags); @@ -189,6 +227,12 @@ bool ArePackageFamilyNameAndProductCodeSupported(const SQLiteIndex& index, const return (index.GetVersion() >= Schema::Version{ 1, 1 } && testVersion >= Schema::Version{ 1, 1 }); } +bool AreNormalizedNameAndPublisherSupported(const SQLiteIndex& index, const Schema::Version& testVersion) +{ + UNSCOPED_INFO("Index " << index.GetVersion() << " | Test " << testVersion); + return (index.GetVersion() >= Schema::Version{ 1, 2 } && testVersion >= Schema::Version{ 1, 2 }); +} + bool IsManifestMetadataSupported(const SQLiteIndex& index, const Schema::Version& testVersion) { UNSCOPED_INFO("Index " << index.GetVersion() << " | Test " << testVersion); @@ -1215,11 +1259,16 @@ TEST_CASE("SQLiteIndex_SearchResultsTableSearches", "[sqliteindex][V1_0]") std::string value = "test"; // Perform every type of field and match search + PackageMatchFilter filter(PackageMatchField::Id, MatchType::Exact, value); + for (auto field : { PackageMatchField::Id, PackageMatchField::Name, PackageMatchField::Moniker, PackageMatchField::Tag, PackageMatchField::Command }) { + filter.Field = field; + for (auto match : { MatchType::Exact, MatchType::Fuzzy, MatchType::FuzzySubstring, MatchType::Substring, MatchType::Wildcard }) { - search.SearchOnField(field, match, value); + filter.Type = match; + search.SearchOnField(filter); } } } @@ -1993,3 +2042,91 @@ TEST_CASE("SQLiteIndex_ManifestMetadata", "[sqliteindex]") REQUIRE(index.GetMetadataByManifestId(manifestId2).empty()); } + +TEST_CASE("SQLiteIndex_NormNameAndPublisher_Exact", "[sqliteindex]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + std::string testName = "Name"; + std::string testPublisher = "Publisher"; + + SQLiteIndex index = SearchTestSetup(tempFile, { + { "Id1", testName, testPublisher, "Moniker", "Version", "Channel", { "Tag" }, { "Command" }, "Path1", {}, { "PC1", "PC2" } }, + }); + + Schema::Version testVersion = TestPrepareForRead(index); + + SearchRequest request; + request.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact, testName, testPublisher)); + + auto results = index.Search(request); + + if (AreNormalizedNameAndPublisherSupported(index, testVersion)) + { + REQUIRE(results.Matches.size() == 1); + } + else + { + REQUIRE(results.Matches.empty()); + } +} + +TEST_CASE("SQLiteIndex_NormNameAndPublisher_Simple", "[sqliteindex]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + std::string testName = "Name"; + std::string testPublisher = "Publisher"; + + SQLiteIndex index = SearchTestSetup(tempFile, { + { "Id1", testName, testPublisher, "Moniker", "Version", "Channel", { "Tag" }, { "Command" }, "Path1", {}, { "PC1", "PC2" } }, + }); + + Schema::Version testVersion = TestPrepareForRead(index); + + SearchRequest request; + request.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact, testName + " 1.0", testPublisher + " Corporation")); + + auto results = index.Search(request); + + if (AreNormalizedNameAndPublisherSupported(index, testVersion)) + { + REQUIRE(results.Matches.size() == 1); + } + else + { + REQUIRE(results.Matches.empty()); + } +} + +TEST_CASE("SQLiteIndex_NormNameAndPublisher_Complex", "[sqliteindex]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + std::string testName = "Name"; + std::string testPublisher = "Publisher"; + + SQLiteIndex index = SearchTestSetup(tempFile, { + { "Id1", testName, testPublisher, "Moniker", "Version", "Channel", { "Tag" }, { "Command" }, "Path1", {}, { "PC1", "PC2" } }, + { "Id2", testName, "Different Publisher", "Moniker", "Version", "Channel", { "Tag" }, { "Command" }, "Path2", {}, { "PC1", "PC2" } }, + }); + + Schema::Version testVersion = TestPrepareForRead(index); + + SearchRequest request; + request.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact, testName + " 1.0", testPublisher)); + + auto results = index.Search(request); + + if (AreNormalizedNameAndPublisherSupported(index, testVersion)) + { + REQUIRE(results.Matches.size() == 1); + } + else + { + REQUIRE(results.Matches.empty()); + } +} diff --git a/src/AppInstallerCLITests/SQLiteIndexSource.cpp b/src/AppInstallerCLITests/SQLiteIndexSource.cpp @@ -171,3 +171,25 @@ TEST_CASE("SQLiteIndexSource_GetManifest", "[sqliteindexsource]") auto noResultVersion = package->GetAvailableVersion(PackageVersionKey("", "blargle", "flargle")); REQUIRE(!noResultVersion); } + +TEST_CASE("SQLiteIndexSource_IsSame", "[sqliteindexsource]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + SourceDetails details; + Manifest manifest; + std::string relativePath; + std::shared_ptr<SQLiteIndexSource> source = SimpleTestSetup(tempFile, details, manifest, relativePath); + + SearchRequest request; + request.Query = RequestMatch(MatchType::Exact, manifest.Id); + + auto result1 = source->Search(request); + REQUIRE(result1.Matches.size() == 1); + + auto result2 = source->Search(request); + REQUIRE(result2.Matches.size() == 1); + + REQUIRE(result1.Matches[0].Package->IsSame(result2.Matches[0].Package.get())); +} diff --git a/src/AppInstallerCLITests/Sources.cpp b/src/AppInstallerCLITests/Sources.cpp @@ -12,6 +12,7 @@ #include <AppInstallerErrors.h> #include <winget/Settings.h> +using namespace TestCommon; using namespace AppInstaller; using namespace AppInstaller::Runtime; using namespace AppInstaller::Repository; @@ -110,7 +111,7 @@ Sources: namespace { // Helper to create a simple source. - struct SourcesTestSource : public TestCommon::TestSource + struct SourcesTestSource : public TestSource { SourcesTestSource() = default; SourcesTestSource(const SourceDetails& details) @@ -136,50 +137,6 @@ namespace return result; } }; - - // Helper that allows some lambdas to be wrapped into a source factory. - struct TestSourceFactory : public ISourceFactory - { - using CreateFunctor = std::function<std::shared_ptr<ISource>(const SourceDetails&)>; - using AddFunctor = std::function<void(SourceDetails&)>; - using UpdateFunctor = std::function<void(const SourceDetails&)>; - using RemoveFunctor = std::function<void(const SourceDetails&)>; - - TestSourceFactory() : - m_Create(SourcesTestSource::Create), m_Add([](SourceDetails&) {}), m_Update([](const SourceDetails&) {}), m_Remove([](const SourceDetails&) {}) {} - - // ISourceFactory - std::shared_ptr<ISource> Create(const SourceDetails& details, IProgressCallback&) override - { - return m_Create(details); - } - - void Add(SourceDetails& details, IProgressCallback&) override - { - m_Add(details); - } - - void Update(const SourceDetails& details, IProgressCallback&) override - { - m_Update(details); - } - - void Remove(const SourceDetails& details, IProgressCallback&) override - { - m_Remove(details); - } - - // Make copies of self when requested. - operator std::function<std::unique_ptr<ISourceFactory>()>() - { - return [this]() { return std::make_unique<TestSourceFactory>(*this); }; - } - - CreateFunctor m_Create; - AddFunctor m_Add; - UpdateFunctor m_Update; - RemoveFunctor m_Remove; - }; } @@ -273,8 +230,8 @@ TEST_CASE("RepoSources_AddSource", "[sources]") std::string data = "thisIsTheData"; bool addCalledOnFactory = false; - TestSourceFactory factory; - factory.m_Add = [&](SourceDetails& sd) { addCalledOnFactory = true; sd.Data = data; }; + TestSourceFactory factory{ SourcesTestSource::Create }; + factory.OnAdd = [&](SourceDetails& sd) { addCalledOnFactory = true; sd.Data = data; }; TestHook_SetSourceFactoryOverride(type, factory); ProgressCallback progress; @@ -306,8 +263,8 @@ TEST_CASE("RepoSources_AddMultipleSources", "[sources]") const char* suffix[2] = { "", "2" }; - TestSourceFactory factory1; - factory1.m_Add = [&](SourceDetails& sd) { sd.Data = data; }; + TestSourceFactory factory1{ SourcesTestSource::Create }; + factory1.OnAdd = [&](SourceDetails& sd) { sd.Data = data; }; TestHook_SetSourceFactoryOverride(type, factory1); ProgressCallback progress; @@ -325,8 +282,8 @@ TEST_CASE("RepoSources_AddMultipleSources", "[sources]") REQUIRE(sources[1].Origin == SourceOrigin::Default); - TestSourceFactory factory2; - factory2.m_Add = [&](SourceDetails& sd) { sd.Data = data + suffix[1]; }; + TestSourceFactory factory2{ SourcesTestSource::Create }; + factory2.OnAdd = [&](SourceDetails& sd) { sd.Data = data + suffix[1]; }; TestHook_SetSourceFactoryOverride(type + suffix[1], factory2); AddSource(name + suffix[1], type + suffix[1], arg + suffix[1], progress); @@ -361,8 +318,8 @@ TEST_CASE("RepoSources_UpdateSource", "[sources]") std::string data = "thisIsTheData"; bool addCalledOnFactory = false; - TestSourceFactory factory; - factory.m_Add = [&](SourceDetails& sd) { addCalledOnFactory = true; sd.Data = data; }; + TestSourceFactory factory{ SourcesTestSource::Create }; + factory.OnAdd = [&](SourceDetails& sd) { addCalledOnFactory = true; sd.Data = data; }; TestHook_SetSourceFactoryOverride(type, factory); ProgressCallback progress; @@ -385,7 +342,7 @@ TEST_CASE("RepoSources_UpdateSource", "[sources]") // Reset for a call to update bool updateCalledOnFactory = false; auto now = std::chrono::system_clock::now(); - factory.m_Update = [&](const SourceDetails&) { updateCalledOnFactory = true; }; + factory.OnUpdate = [&](const SourceDetails&) { updateCalledOnFactory = true; }; UpdateSource(name, progress); @@ -413,8 +370,8 @@ TEST_CASE("RepoSources_UpdateSourceRetries", "[sources]") std::string arg = "thisIsTheArg"; std::string data = "thisIsTheData"; - TestSourceFactory factory; - factory.m_Add = [&](SourceDetails& sd) { sd.Data = data; }; + TestSourceFactory factory{ SourcesTestSource::Create }; + factory.OnAdd = [&](SourceDetails& sd) { sd.Data = data; }; TestHook_SetSourceFactoryOverride(type, factory); ProgressCallback progress; @@ -423,7 +380,7 @@ TEST_CASE("RepoSources_UpdateSourceRetries", "[sources]") // Reset for a call to update bool updateShouldThrow = false; bool updateCalledOnFactoryAgain = false; - factory.m_Update = [&](const SourceDetails&) + factory.OnUpdate = [&](const SourceDetails&) { if (updateShouldThrow) { @@ -449,8 +406,8 @@ TEST_CASE("RepoSources_RemoveSource", "[sources]") std::string data = "thisIsTheData"; bool removeCalledOnFactory = false; - TestSourceFactory factory; - factory.m_Remove = [&](const SourceDetails&) { removeCalledOnFactory = true; }; + TestSourceFactory factory{ SourcesTestSource::Create }; + factory.OnRemove = [&](const SourceDetails&) { removeCalledOnFactory = true; }; TestHook_SetSourceFactoryOverride(type, factory); ProgressCallback progress; @@ -477,8 +434,8 @@ TEST_CASE("RepoSources_RemoveDefaultSource", "[sources]") REQUIRE(sources[0].Origin == SourceOrigin::Default); bool removeCalledOnFactory = false; - TestSourceFactory factory; - factory.m_Remove = [&](const SourceDetails&) { removeCalledOnFactory = true; }; + TestSourceFactory factory{ SourcesTestSource::Create }; + factory.OnRemove = [&](const SourceDetails&) { removeCalledOnFactory = true; }; TestHook_SetSourceFactoryOverride(sources[0].Type, factory); ProgressCallback progress; @@ -503,8 +460,8 @@ TEST_CASE("RepoSources_UpdateOnOpen", "[sources]") std::string data = "testData"; bool updateCalledOnFactory = false; - TestSourceFactory factory; - factory.m_Update = [&](const SourceDetails&) { updateCalledOnFactory = true; }; + TestSourceFactory factory{ SourcesTestSource::Create }; + factory.OnUpdate = [&](const SourceDetails&) { updateCalledOnFactory = true; }; TestHook_SetSourceFactoryOverride(type, factory); SetSetting(Streams::UserSources, s_SingleSource); @@ -568,7 +525,7 @@ TEST_CASE("RepoSources_DropAllSources", "[sources]") TEST_CASE("RepoSources_SearchAcrossMultipleSources", "[sources]") { TestHook_ClearSourceFactoryOverrides(); - TestSourceFactory factory; + TestSourceFactory factory{ SourcesTestSource::Create }; TestHook_SetSourceFactoryOverride("testType", factory); SetSetting(Streams::UserSources, s_TwoSource_AggregateSourceTest); diff --git a/src/AppInstallerCLITests/TestData/InstallFlowTest_Exe.yaml b/src/AppInstallerCLITests/TestData/InstallFlowTest_Exe.yaml @@ -1,6 +1,6 @@ Id: AppInstallerCliTest.TestExeInstaller Version: 1.0.0.0 -Name: AppInstaller Test Installer +Name: AppInstaller Test Exe Installer Publisher: Microsoft Corporation AppMoniker: AICLITestExe License: Test diff --git a/src/AppInstallerCLITests/TestHooks.h b/src/AppInstallerCLITests/TestHooks.h @@ -7,6 +7,7 @@ #include <memory> #include <string> +#include <AppInstallerTelemetry.h> #include <AppInstallerRuntime.h> #ifdef AICLI_DISABLE_TEST_HOOKS @@ -26,4 +27,9 @@ namespace AppInstaller void TestHook_SetSourceFactoryOverride(const std::string& type, std::function<std::unique_ptr<ISourceFactory>()>&& factory); void TestHook_ClearSourceFactoryOverrides(); } + + namespace Logging + { + void TestHook_SetTelemetryOverride(std::shared_ptr<TelemetryTraceLogger> ttl); + } } diff --git a/src/AppInstallerCLITests/TestSource.cpp b/src/AppInstallerCLITests/TestSource.cpp @@ -9,8 +9,8 @@ using namespace AppInstaller::Repository; namespace TestCommon { - TestPackageVersion::TestPackageVersion(const Manifest& manifest, MetadataMap installationMetadata) : - VersionManifest(manifest), Metadata(std::move(installationMetadata)) {} + TestPackageVersion::TestPackageVersion(const Manifest& manifest, MetadataMap installationMetadata, std::weak_ptr<const ISource> source) : + VersionManifest(manifest), Metadata(std::move(installationMetadata)), Source(source) {} TestPackageVersion::TestPackageVersion(const Manifest& manifest, std::weak_ptr<const ISource> source) : VersionManifest(manifest), Source(source) {} @@ -27,6 +27,8 @@ namespace TestCommon return LocIndString{ VersionManifest.Version }; case PackageVersionProperty::Channel: return LocIndString{ VersionManifest.Channel }; + case PackageVersionProperty::SourceIdentifier: + return LocIndString{ Source.lock()->GetIdentifier() }; default: return {}; } @@ -92,7 +94,7 @@ namespace TestCommon } TestPackage::TestPackage(const Manifest& installed, MetadataMap installationMetadata, const std::vector<Manifest>& available, std::weak_ptr<const ISource> source) : - InstalledVersion(TestPackageVersion::Make(installed, std::move(installationMetadata))) + InstalledVersion(TestPackageVersion::Make(installed, std::move(installationMetadata), source)) { for (const auto& manifest : available) { @@ -181,6 +183,28 @@ namespace TestCommon return false; } + bool TestPackage::IsSame(const IPackage* other) const + { + const TestPackage* otherAvailable = dynamic_cast<const TestPackage*>(other); + + if (!otherAvailable || + InstalledVersion.get() != otherAvailable->InstalledVersion.get() || + AvailableVersions.size() != otherAvailable->AvailableVersions.size()) + { + return false; + } + + for (size_t i = 0; i < AvailableVersions.size(); ++i) + { + if (AvailableVersions[i].get() != otherAvailable->AvailableVersions[i].get()) + { + return false; + } + } + + return true; + } + const SourceDetails& TestSource::GetDetails() const { return Details; @@ -207,4 +231,39 @@ namespace TestCommon { return Composite; } + + std::shared_ptr<ISource> TestSourceFactory::Create(const SourceDetails& details, IProgressCallback&) + { + return OnCreate(details); + } + + void TestSourceFactory::Add(SourceDetails& details, IProgressCallback&) + { + if (OnAdd) + { + OnAdd(details); + } + } + + void TestSourceFactory::Update(const SourceDetails& details, IProgressCallback&) + { + if (OnUpdate) + { + OnUpdate(details); + } + } + + void TestSourceFactory::Remove(const SourceDetails& details, IProgressCallback&) + { + if (OnRemove) + { + OnRemove(details); + } + } + + // Make copies of self when requested. + TestSourceFactory::operator std::function<std::unique_ptr<ISourceFactory>()>() + { + return [this]() { return std::make_unique<TestSourceFactory>(*this); }; + } } diff --git a/src/AppInstallerCLITests/TestSource.h b/src/AppInstallerCLITests/TestSource.h @@ -3,6 +3,7 @@ #pragma once #include <Public/AppInstallerRepositorySource.h> #include <winget/Manifest.h> +#include <SourceFactory.h> #include <functional> #include <utility> @@ -18,7 +19,7 @@ namespace TestCommon using MetadataMap = AppInstaller::Repository::IPackageVersion::Metadata; TestPackageVersion(const Manifest& manifest, std::weak_ptr<const ISource> source = {}); - TestPackageVersion(const Manifest& manifest, MetadataMap installationMetadata); + TestPackageVersion(const Manifest& manifest, MetadataMap installationMetadata, std::weak_ptr<const ISource> source = {}); template <typename... Args> static std::shared_ptr<TestPackageVersion> Make(Args&&... args) @@ -66,6 +67,7 @@ namespace TestCommon std::shared_ptr<AppInstaller::Repository::IPackageVersion> GetLatestAvailableVersion() const override; std::shared_ptr<AppInstaller::Repository::IPackageVersion> GetAvailableVersion(const AppInstaller::Repository::PackageVersionKey& versionKey) const override; bool IsUpdateAvailable() const override; + bool IsSame(const IPackage* other) const override; std::shared_ptr<AppInstaller::Repository::IPackageVersion> InstalledVersion; std::vector<std::shared_ptr<AppInstaller::Repository::IPackageVersion>> AvailableVersions; @@ -83,4 +85,29 @@ namespace TestCommon std::function<AppInstaller::Repository::SearchResult(const AppInstaller::Repository::SearchRequest& request)> SearchFunction; bool Composite = false; }; + + // An ISourceFactory implementation for use across the test code. + struct TestSourceFactory : public AppInstaller::Repository::ISourceFactory + { + using CreateFunctor = std::function<std::shared_ptr<AppInstaller::Repository::ISource>(const AppInstaller::Repository::SourceDetails&)>; + using AddFunctor = std::function<void(AppInstaller::Repository::SourceDetails&)>; + using UpdateFunctor = std::function<void(const AppInstaller::Repository::SourceDetails&)>; + using RemoveFunctor = std::function<void(const AppInstaller::Repository::SourceDetails&)>; + + TestSourceFactory(CreateFunctor create) : OnCreate(std::move(create)) {} + + // ISourceFactory + std::shared_ptr<AppInstaller::Repository::ISource> Create(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; + void Add(AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; + void Update(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; + void Remove(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; + + // Make copies of self when requested. + operator std::function<std::unique_ptr<AppInstaller::Repository::ISourceFactory>()>(); + + CreateFunctor OnCreate; + AddFunctor OnAdd; + UpdateFunctor OnUpdate; + RemoveFunctor OnRemove; + }; } diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp @@ -709,7 +709,7 @@ TEST_CASE("InstallFlow_SearchFoundMultipleApp", "[InstallFlow][workflow]") REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::MultiplePackagesFound).get()) != std::string::npos); } -TEST_CASE("InstallFlow_SearchAndShowAppInfo", "[ShowFlow][workflow]") +TEST_CASE("ShowFlow_SearchAndShowAppInfo", "[ShowFlow][workflow]") { std::ostringstream showOutput; TestContext context{ showOutput, std::cin }; @@ -722,12 +722,12 @@ TEST_CASE("InstallFlow_SearchAndShowAppInfo", "[ShowFlow][workflow]") // Verify AppInfo is printed REQUIRE(showOutput.str().find("AppInstallerCliTest.TestExeInstaller") != std::string::npos); - REQUIRE(showOutput.str().find("AppInstaller Test Installer") != std::string::npos); + REQUIRE(showOutput.str().find("AppInstaller Test Exe Installer") != std::string::npos); REQUIRE(showOutput.str().find("1.0.0.0") != std::string::npos); REQUIRE(showOutput.str().find("https://ThisIsNotUsed") != std::string::npos); } -TEST_CASE("InstallFlow_SearchAndShowAppVersion", "[ShowFlow][workflow]") +TEST_CASE("ShowFlow_SearchAndShowAppVersion", "[ShowFlow][workflow]") { std::ostringstream showOutput; TestContext context{ showOutput, std::cin }; @@ -1370,4 +1370,4 @@ TEST_CASE("VerifyInstallerTrustLevelAndUpdateInstallerFileMotw", "[DownloadInsta VerifyMotw(testInstallerPath, 3); INFO(updateMotwOutput.str()); -}- \ No newline at end of file +} diff --git a/src/AppInstallerCLITests/main.cpp b/src/AppInstallerCLITests/main.cpp @@ -52,6 +52,7 @@ int main(int argc, char** argv) bool hasSetTestDataBasePath = false; bool waitBeforeReturn = false; + bool keepSQLLogging = false; std::vector<char*> args; for (int i = 0; i < argc; ++i) @@ -86,6 +87,10 @@ int main(int argc, char** argv) { waitBeforeReturn = true; } + else if ("-logsql"s == argv[i]) + { + keepSQLLogging = true; + } else { args.push_back(argv[i]); @@ -105,9 +110,15 @@ int main(int argc, char** argv) } } - // Enable all logging, to force log string building to run. + // Enable logging, to force log string building to run. + // Disable SQL by default, as it generates 10s of MBs of log file and + // increases the the full test run time by 60% or more. // By not creating a log target, it will all be thrown away. Logging::Log().EnableChannel(Logging::Channel::All); + if (!keepSQLLogging) + { + Logging::Log().DisableChannel(Logging::Channel::SQL); + } Logging::Log().SetLevel(Logging::Level::Verbose); Logging::EnableWilFailureTelemetry(); diff --git a/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp b/src/AppInstallerCommonCore/AppInstallerTelemetry.cpp @@ -81,7 +81,7 @@ namespace AppInstaller::Logging return instance; } - void TelemetryTraceLogger::LogFailure(const wil::FailureInfo& failure) noexcept + void TelemetryTraceLogger::LogFailure(const wil::FailureInfo& failure) const noexcept { if (IsTelemetryEnabled()) { @@ -110,7 +110,7 @@ namespace AppInstaller::Logging }()); } - void TelemetryTraceLogger::LogStartup() noexcept + void TelemetryTraceLogger::LogStartup() const noexcept { LocIndString version = Runtime::GetClientVersion(); LocIndString packageVersion; @@ -127,7 +127,7 @@ namespace AppInstaller::Logging nullptr, TraceLoggingCountedString(version->c_str(), static_cast<ULONG>(version->size()), "Version"), TraceLoggingCountedString(packageVersion->c_str(), static_cast<ULONG>(packageVersion->size()), "PackageVersion"), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance|PDT_ProductAndServiceUsage), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } @@ -140,7 +140,7 @@ namespace AppInstaller::Logging } } - void TelemetryTraceLogger::LogCommand(std::string_view commandName) noexcept + void TelemetryTraceLogger::LogCommand(std::string_view commandName) const noexcept { if (IsTelemetryEnabled()) { @@ -149,14 +149,14 @@ namespace AppInstaller::Logging GetActivityId(), nullptr, AICLI_TraceLoggingStringView(commandName, "Command"), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } AICLI_LOG(CLI, Info, << "Leaf command to execute: " << commandName); } - void TelemetryTraceLogger::LogCommandSuccess(std::string_view commandName) noexcept + void TelemetryTraceLogger::LogCommandSuccess(std::string_view commandName) const noexcept { if (IsTelemetryEnabled()) { @@ -172,7 +172,7 @@ namespace AppInstaller::Logging AICLI_LOG(CLI, Info, << "Leaf command succeeded: " << commandName); } - void TelemetryTraceLogger::LogCommandTermination(HRESULT hr, std::string_view file, size_t line) noexcept + void TelemetryTraceLogger::LogCommandTermination(HRESULT hr, std::string_view file, size_t line) const noexcept { if (IsTelemetryEnabled()) { @@ -192,7 +192,7 @@ namespace AppInstaller::Logging AICLI_LOG(CLI, Error, << "Terminating context: 0x" << SetHRFormat << hr << " at " << file << ":" << line); } - void TelemetryTraceLogger::LogException(std::string_view commandName, std::string_view type, std::string_view message) noexcept + void TelemetryTraceLogger::LogException(std::string_view commandName, std::string_view type, std::string_view message) const noexcept { if (IsTelemetryEnabled()) { @@ -212,7 +212,7 @@ namespace AppInstaller::Logging AICLI_LOG(CLI, Error, << "Caught " << type << ": " << message); } - void TelemetryTraceLogger::LogIsManifestLocal(bool isLocalManifest) noexcept + void TelemetryTraceLogger::LogIsManifestLocal(bool isLocalManifest) const noexcept { if (IsTelemetryEnabled()) { @@ -227,7 +227,7 @@ namespace AppInstaller::Logging } } - void TelemetryTraceLogger::LogManifestFields(std::string_view id, std::string_view name, std::string_view version) noexcept + void TelemetryTraceLogger::LogManifestFields(std::string_view id, std::string_view name, std::string_view version) const noexcept { if (IsTelemetryEnabled()) { @@ -239,14 +239,14 @@ namespace AppInstaller::Logging AICLI_TraceLoggingStringView(id, "Id"), AICLI_TraceLoggingStringView(name,"Name"), AICLI_TraceLoggingStringView(version, "Version"), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance|PDT_ProductAndServiceUsage), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } AICLI_LOG(CLI, Info, << "Manifest fields: Name [" << name << "], Version [" << version << ']'); } - void TelemetryTraceLogger::LogNoAppMatch() noexcept + void TelemetryTraceLogger::LogNoAppMatch() const noexcept { if (IsTelemetryEnabled()) { @@ -255,14 +255,14 @@ namespace AppInstaller::Logging GetActivityId(), nullptr, TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } AICLI_LOG(CLI, Info, << "No app found matching input criteria"); } - void TelemetryTraceLogger::LogMultiAppMatch() noexcept + void TelemetryTraceLogger::LogMultiAppMatch() const noexcept { if (IsTelemetryEnabled()) { @@ -271,14 +271,14 @@ namespace AppInstaller::Logging GetActivityId(), nullptr, TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } AICLI_LOG(CLI, Info, << "Multiple apps found matching input criteria"); } - void TelemetryTraceLogger::LogAppFound(std::string_view name, std::string_view id) noexcept + void TelemetryTraceLogger::LogAppFound(std::string_view name, std::string_view id) const noexcept { if (IsTelemetryEnabled()) { @@ -296,7 +296,7 @@ namespace AppInstaller::Logging AICLI_LOG(CLI, Info, << "Found one app. App id: " << id << " App name: " << name); } - void TelemetryTraceLogger::LogSelectedInstaller(int arch, std::string_view url, std::string_view installerType, std::string_view scope, std::string_view language) noexcept + void TelemetryTraceLogger::LogSelectedInstaller(int arch, std::string_view url, std::string_view installerType, std::string_view scope, std::string_view language) const noexcept { if (IsTelemetryEnabled()) { @@ -310,7 +310,7 @@ namespace AppInstaller::Logging AICLI_TraceLoggingStringView(installerType, "InstallerType"), AICLI_TraceLoggingStringView(scope, "Scope"), AICLI_TraceLoggingStringView(language, "Language"), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } @@ -331,7 +331,7 @@ namespace AppInstaller::Logging std::string_view tag, std::string_view command, size_t maximum, - std::string_view request) + std::string_view request) const noexcept { if (IsTelemetryEnabled()) { @@ -354,7 +354,7 @@ namespace AppInstaller::Logging } } - void TelemetryTraceLogger::LogSearchResultCount(uint64_t resultCount) noexcept + void TelemetryTraceLogger::LogSearchResultCount(uint64_t resultCount) const noexcept { if (IsTelemetryEnabled()) { @@ -364,7 +364,7 @@ namespace AppInstaller::Logging nullptr, TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), TraceLoggingUInt64(resultCount, "ResultCount"), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } } @@ -375,7 +375,7 @@ namespace AppInstaller::Logging std::string_view channel, const std::vector<uint8_t>& expected, const std::vector<uint8_t>& actual, - bool overrideHashMismatch) + bool overrideHashMismatch) const noexcept { if (IsTelemetryEnabled()) { @@ -390,7 +390,7 @@ namespace AppInstaller::Logging TraceLoggingBinary(expected.data(), static_cast<ULONG>(expected.size()), "Expected"), TraceLoggingBinary(actual.data(), static_cast<ULONG>(actual.size()), "Actual"), TraceLoggingValue(overrideHashMismatch, "Override"), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } @@ -402,7 +402,7 @@ namespace AppInstaller::Logging << ']'); } - void TelemetryTraceLogger::LogInstallerFailure(std::string_view id, std::string_view version, std::string_view channel, std::string_view type, uint32_t errorCode) + void TelemetryTraceLogger::LogInstallerFailure(std::string_view id, std::string_view version, std::string_view channel, std::string_view type, uint32_t errorCode) const noexcept { if (IsTelemetryEnabled()) { @@ -416,14 +416,14 @@ namespace AppInstaller::Logging AICLI_TraceLoggingStringView(channel, "Channel"), AICLI_TraceLoggingStringView(type, "Type"), TraceLoggingUInt32(errorCode, "ErrorCode"), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } AICLI_LOG(CLI, Error, << type << " installer failed: " << errorCode); } - void TelemetryTraceLogger::LogUninstallerFailure(std::string_view id, std::string_view version, std::string_view type, uint32_t errorCode) + void TelemetryTraceLogger::LogUninstallerFailure(std::string_view id, std::string_view version, std::string_view type, uint32_t errorCode) const noexcept { if (IsTelemetryEnabled()) { @@ -436,32 +436,84 @@ namespace AppInstaller::Logging AICLI_TraceLoggingStringView(version, "Version"), AICLI_TraceLoggingStringView(type, "Type"), TraceLoggingUInt32(errorCode, "ErrorCode"), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } AICLI_LOG(CLI, Error, << type << " uninstaller failed: " << errorCode); } - void TelemetryTraceLogger::LogDuplicateARPEntry(HRESULT hr, std::string_view scope, std::string_view architecture, std::string_view productCode, std::string_view name) + void TelemetryTraceLogger::LogSuccessfulInstallARPChange( + std::string_view sourceIdentifier, + std::string_view packageIdentifier, + std::string_view packageVersion, + std::string_view packageChannel, + size_t changesToARP, + size_t matchesInARP, + size_t countOfIntersectionOfChangesAndMatches, + std::string_view arpName, + std::string_view arpVersion, + std::string_view arpPublisher, + std::string_view arpLanguage) const noexcept { if (IsTelemetryEnabled()) { + size_t languageNumber = 0xFFFF; + + try + { + std::istringstream languageConversion{ std::string{ arpLanguage } }; + languageConversion >> languageNumber; + } + catch (...) {} + TraceLoggingWriteActivity(g_hTelemetryProvider, - "DuplicateARPEntry", + "InstallARPChange", GetActivityId(), nullptr, TraceLoggingUInt32(s_subExecutionId, "SubExecutionId"), - TraceLoggingHResult(hr, "HResult"), - AICLI_TraceLoggingStringView(scope, "Scope"), - AICLI_TraceLoggingStringView(architecture, "Architecture"), - AICLI_TraceLoggingStringView(productCode, "ProductCode"), - AICLI_TraceLoggingStringView(name, "Name"), - TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance | PDT_ProductAndServiceUsage), + AICLI_TraceLoggingStringView(sourceIdentifier, "SourceIdentifier"), + AICLI_TraceLoggingStringView(packageIdentifier, "PackageIdentifier"), + AICLI_TraceLoggingStringView(packageVersion, "PackageVersion"), + AICLI_TraceLoggingStringView(packageChannel, "PackageChannel"), + TraceLoggingUInt64(static_cast<UINT64>(changesToARP), "ChangesToARP"), + TraceLoggingUInt64(static_cast<UINT64>(matchesInARP), "MatchesInARP"), + TraceLoggingUInt64(static_cast<UINT64>(countOfIntersectionOfChangesAndMatches), "ChangesThatMatch"), + AICLI_TraceLoggingStringView(arpName, "ARPName"), + AICLI_TraceLoggingStringView(arpVersion, "ARPVersion"), + AICLI_TraceLoggingStringView(arpPublisher, "ARPPublisher"), + TraceLoggingUInt64(static_cast<UINT64>(languageNumber), "ARPLanguage"), + TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance), TraceLoggingKeyword(MICROSOFT_KEYWORD_CRITICAL_DATA)); } - AICLI_LOG(CLI, Error, << "Ignoring duplicate ARP entry " << scope << '|' << architecture << '|' << productCode << " [" << name << "]"); + AICLI_LOG(CLI, Info, << "During package install, " << changesToARP << " changes to ARP were observed, " + << matchesInARP << " matches were found for the package, and " << countOfIntersectionOfChangesAndMatches << " packages were in both"); + + if (arpName.empty()) + { + AICLI_LOG(CLI, Info, << "No single entry was determined to be associated with the package"); + } + else + { + AICLI_LOG(CLI, Info, << "The entry determined to be associated with the package is '" << arpName << "', with publisher '" << arpPublisher << "'"); + } + } + +#ifndef AICLI_DISABLE_TEST_HOOKS + static std::shared_ptr<TelemetryTraceLogger> s_TelemetryTraceLogger_TestOverride; +#endif + + TelemetryTraceLogger& Telemetry() + { +#ifndef AICLI_DISABLE_TEST_HOOKS + if (s_TelemetryTraceLogger_TestOverride) + { + return *s_TelemetryTraceLogger_TestOverride.get(); + } +#endif + + return TelemetryTraceLogger::GetInstance(); } void EnableWilFailureTelemetry() @@ -500,4 +552,12 @@ namespace AppInstaller::Logging { s_subExecutionId = s_RootExecutionId; } + +#ifndef AICLI_DISABLE_TEST_HOOKS + // Replace this test hook with context telemetry when it gets moved over + void TestHook_SetTelemetryOverride(std::shared_ptr<TelemetryTraceLogger> ttl) + { + s_TelemetryTraceLogger_TestOverride = std::move(ttl); + } +#endif } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/NameNormalization.cpp b/src/AppInstallerCommonCore/NameNormalization.cpp @@ -355,12 +355,7 @@ namespace AppInstaller::Utility return result; } - public: - NormalizationInitial() : Locales(FoldAndSort(LocaleViews)), LegalEntitySuffixes(FoldAndSort(LegalEntitySuffixViews)) - { - } - - InterimNameNormalizationResult NormalizeName(std::string_view name) const + InterimNameNormalizationResult NormalizeNameInternal(std::string_view name) const { InterimNameNormalizationResult result; result.Name = PrepareForValidation(name); @@ -390,7 +385,7 @@ namespace AppInstaller::Utility return result; } - InterimPublisherNormalizationResult NormalizePublisher(std::string_view publisher) const + InterimPublisherNormalizationResult NormalizePublisherInternal(std::string_view publisher) const { InterimPublisherNormalizationResult result; @@ -408,10 +403,15 @@ namespace AppInstaller::Utility return result; } + public: + NormalizationInitial() : Locales(FoldAndSort(LocaleViews)), LegalEntitySuffixes(FoldAndSort(LegalEntitySuffixViews)) + { + } + NormalizedName Normalize(std::string_view name, std::string_view publisher) const override { - InterimNameNormalizationResult nameResult = NormalizeName(name); - InterimPublisherNormalizationResult pubResult = NormalizePublisher(publisher); + InterimNameNormalizationResult nameResult = NormalizeNameInternal(name); + InterimPublisherNormalizationResult pubResult = NormalizePublisherInternal(publisher); NormalizedName result; result.Name(ConvertToUTF8(nameResult.Name)); @@ -421,6 +421,25 @@ namespace AppInstaller::Utility return result; } + + NormalizedName NormalizeName(std::string_view name) const override + { + InterimNameNormalizationResult nameResult = NormalizeNameInternal(name); + + NormalizedName result; + result.Name(ConvertToUTF8(nameResult.Name)); + result.Architecture(nameResult.Architecture); + result.Locale(ConvertToUTF8(nameResult.Locale)); + + return result; + } + + std::string NormalizePublisher(std::string_view publisher) const override + { + InterimPublisherNormalizationResult pubResult = NormalizePublisherInternal(publisher); + + return ConvertToUTF8(pubResult.Publisher); + } }; } @@ -440,4 +459,14 @@ namespace AppInstaller::Utility { return m_normalizer->Normalize(name, publisher); } + + NormalizedName NameNormalizer::NormalizeName(std::string_view name) const + { + return m_normalizer->NormalizeName(name); + } + + std::string NameNormalizer::NormalizePublisher(std::string_view publisher) const + { + return m_normalizer->NormalizePublisher(publisher); + } } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerLanguageUtilities.h b/src/AppInstallerCommonCore/Public/AppInstallerLanguageUtilities.h @@ -1,13 +1,16 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once +#include <wil/result.h> #include <initializer_list> +#include <map> #include <string> #include <string_view> #include <tuple> #include <type_traits> #include <utility> +#include <variant> namespace AppInstaller { @@ -66,6 +69,82 @@ namespace AppInstaller { return static_cast<E>(ut); } + + // Enum based variant helper. + // Enum must be an enum whose first member has the value 0, each subsequent member increases by 1, and the final member is named Max. + // Mapping is a template type that takes one template parameter of type Enum, and whose members define value_t as the type for that enum value. + template <typename Enum, template<Enum> typename Mapping> + struct EnumBasedVariant + { + private: + // Used to deduce the variant type; making a variant that includes std::monostate and all Mapping types. + template <size_t... I> + static inline auto Deduce(std::index_sequence<I...>) { return std::variant<std::monostate, typename Mapping<static_cast<Enum>(I)>::value_t...>{}; } + + public: + // Holds data of any type listed in Mapping. + using variant_t = decltype(Deduce(std::make_index_sequence<static_cast<size_t>(Enum::Max)>())); + + // Gets the index into the variant for the given Data. + static constexpr inline size_t Index(Enum e) { return static_cast<size_t>(e) + 1; } + }; + + // Provides a map of the Enum to the mapped types. + template <typename Enum, template<Enum> typename Mapping> + struct EnumBasedVariantMap + { + using Variant = EnumBasedVariant<Enum, Mapping>; + + template <Enum E> + using mapping_t = typename Mapping<E>::value_t; + + // Adds a value to the map, or overwrites an existing entry. + // This must be used to create the initial data entry, but Get can be used to modify. + template <Enum E> + void Add(mapping_t<E>&& v) + { + m_data[E].emplace<Variant::Index(E)>(std::move(std::forward<mapping_t<E>>(v))); + } + + template <Enum E> + void Add(const mapping_t<E>& v) + { + m_data[E].emplace<Variant::Index(E)>(v); + } + + // Return a value indicating whether the given enum is stored in the map. + bool Contains(Enum e) { return (m_data.find(e) != m_data.end()); } + + // Gets the value. + template <Enum E> + mapping_t<E>& Get() + { + return std::get<Variant::Index(E)>(GetVariant(E)); + } + + template <Enum E> + const mapping_t<E>& Get() const + { + return std::get<Variant::Index(E)>(GetVariant(E)); + } + + private: + typename Variant::variant_t& GetVariant(Enum e) + { + auto itr = m_data.find(e); + THROW_HR_IF_MSG(E_NOT_SET, itr == m_data.end(), "GetVariant(%d)", e); + return itr->second; + } + + const typename Variant::variant_t& GetVariant(Enum e) const + { + auto itr = m_data.find(e); + THROW_HR_IF_MSG(E_NOT_SET, itr == m_data.cend(), "GetVariant(%d)", e); + return itr->second; + } + + std::map<Enum, typename Variant::variant_t> m_data; + }; } // Enable enums to be output generically (as their integral value). diff --git a/src/AppInstallerCommonCore/Public/AppInstallerStrings.h b/src/AppInstallerCommonCore/Public/AppInstallerStrings.h @@ -32,6 +32,7 @@ namespace AppInstaller::Utility NormalizedUTF8(std::string_view sv) : std::string(Normalize(sv, Form)) {} + NormalizedUTF8(std::string& s) : std::string(Normalize(s, Form)) {} NormalizedUTF8(const std::string& s) : std::string(Normalize(s, Form)) {} NormalizedUTF8(std::string&& s) : std::string(Normalize(s, Form)) {} diff --git a/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h b/src/AppInstallerCommonCore/Public/AppInstallerTelemetry.h @@ -15,52 +15,52 @@ namespace AppInstaller::Logging // this should not become a burden. struct TelemetryTraceLogger { - ~TelemetryTraceLogger(); + virtual ~TelemetryTraceLogger(); - TelemetryTraceLogger(const TelemetryTraceLogger&) = delete; - TelemetryTraceLogger& operator=(const TelemetryTraceLogger&) = delete; + TelemetryTraceLogger(const TelemetryTraceLogger&) = default; + TelemetryTraceLogger& operator=(const TelemetryTraceLogger&) = default; - TelemetryTraceLogger(TelemetryTraceLogger&&) = delete; - TelemetryTraceLogger& operator=(TelemetryTraceLogger&&) = delete; + TelemetryTraceLogger(TelemetryTraceLogger&&) = default; + TelemetryTraceLogger& operator=(TelemetryTraceLogger&&) = default; // Gets the singleton instance of this type. static TelemetryTraceLogger& GetInstance(); // Logs the failure info. - void LogFailure(const wil::FailureInfo& failure) noexcept; + void LogFailure(const wil::FailureInfo& failure) const noexcept; // Logs the initial process startup. - void LogStartup() noexcept; + void LogStartup() const noexcept; // Logs the invoked command. - void LogCommand(std::string_view commandName) noexcept; + void LogCommand(std::string_view commandName) const noexcept; // Logs the invoked command success. - void LogCommandSuccess(std::string_view commandName) noexcept; + void LogCommandSuccess(std::string_view commandName) const noexcept; // Logs the invoked command termination. - void LogCommandTermination(HRESULT hr, std::string_view file, size_t line) noexcept; + void LogCommandTermination(HRESULT hr, std::string_view file, size_t line) const noexcept; // Logs the invoked command termination. - void LogException(std::string_view commandName, std::string_view type, std::string_view message) noexcept; + void LogException(std::string_view commandName, std::string_view type, std::string_view message) const noexcept; // Logs whether the manifest used in workflow is local - void LogIsManifestLocal(bool isLocalManifest) noexcept; + void LogIsManifestLocal(bool isLocalManifest) const noexcept; // Logs the Manifest fields. - void LogManifestFields(std::string_view id, std::string_view name, std::string_view version) noexcept; + void LogManifestFields(std::string_view id, std::string_view name, std::string_view version) const noexcept; // Logs when there is no matching App found for search - void LogNoAppMatch() noexcept; + void LogNoAppMatch() const noexcept; // Logs when there is multiple matching Apps found for search - void LogMultiAppMatch() noexcept; + void LogMultiAppMatch() const noexcept; // Logs the name and Id of app found - void LogAppFound(std::string_view name, std::string_view id) noexcept; + void LogAppFound(std::string_view name, std::string_view id) const noexcept; // Logs the selected installer details - void LogSelectedInstaller(int arch, std::string_view url, std::string_view installerType, std::string_view scope, std::string_view language) noexcept; + void LogSelectedInstaller(int arch, std::string_view url, std::string_view installerType, std::string_view scope, std::string_view language) const noexcept; // Logs details of a search request. void LogSearchRequest( @@ -72,10 +72,10 @@ namespace AppInstaller::Logging std::string_view tag, std::string_view command, size_t maximum, - std::string_view request); + std::string_view request) const noexcept; // Logs the Search Result - void LogSearchResultCount(uint64_t resultCount) noexcept; + void LogSearchResultCount(uint64_t resultCount) const noexcept; // Logs a mismatch between the expected and actual hash values. void LogInstallerHashMismatch( @@ -84,27 +84,38 @@ namespace AppInstaller::Logging std::string_view channel, const std::vector<uint8_t>& expected, const std::vector<uint8_t>& actual, - bool overrideHashMismatch); + bool overrideHashMismatch) const noexcept; // Logs a failed installation attempt. - void LogInstallerFailure(std::string_view id, std::string_view version, std::string_view channel, std::string_view type, uint32_t errorCode); + void LogInstallerFailure(std::string_view id, std::string_view version, std::string_view channel, std::string_view type, uint32_t errorCode) const noexcept; // Logs a failed uninstallation attempt. - void LogUninstallerFailure(std::string_view id, std::string_view version, std::string_view type, uint32_t errorCode); - - // Logs a failure to insert a value into the in-memory cache of installed system packages. - // The most likely reason is due to the same key name being used under multiple ARP scope/architecture locations. - void LogDuplicateARPEntry(HRESULT hr, std::string_view scope, std::string_view architecture, std::string_view productCode, std::string_view name); - - private: + void LogUninstallerFailure(std::string_view id, std::string_view version, std::string_view type, uint32_t errorCode) const noexcept; + + // Logs data about the changes that ocurred in the ARP entries based on an install. + // First 4 arguments are well known values for the package that we installed. + // The next 3 are counts of the number of packages in each category. + // The last 4 are the fields directly from the ARP entry that has been determined to be related to the package that + // was installed, or they will be empty if there is no data or ambiguity about which entry should be logged. + virtual void LogSuccessfulInstallARPChange( + std::string_view sourceIdentifier, + std::string_view packageIdentifier, + std::string_view packageVersion, + std::string_view packageChannel, + size_t changesToARP, + size_t matchesInARP, + size_t countOfIntersectionOfChangesAndMatches, + std::string_view arpName, + std::string_view arpVersion, + std::string_view arpPublisher, + std::string_view arpLanguage) const noexcept; + + protected: TelemetryTraceLogger(); }; // Helper to make the call sites look clean. - inline TelemetryTraceLogger& Telemetry() - { - return TelemetryTraceLogger::GetInstance(); - } + TelemetryTraceLogger& Telemetry(); // Turns on wil failure telemetry and logging. void EnableWilFailureTelemetry(); diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestLocalization.h b/src/AppInstallerCommonCore/Public/winget/ManifestLocalization.h @@ -70,7 +70,7 @@ namespace AppInstaller::Manifest } // Return a value indicating whether the given localization type exists. - bool Contains(Localization l) { return (m_data.find(l) != m_data.end()); } + bool Contains(Localization l) const { return (m_data.find(l) != m_data.end()); } // Gets the localization value if exists, otherwise empty for easier access template <Localization L> diff --git a/src/AppInstallerCommonCore/Public/winget/NameNormalization.h b/src/AppInstallerCommonCore/Public/winget/NameNormalization.h @@ -26,6 +26,7 @@ namespace AppInstaller::Utility const std::string& Name() const { return m_name; } void Name(std::string&& name) { m_name = std::move(name); } + void Name(std::string_view name) { m_name = name; } Utility::Architecture Architecture() const { return m_arch; } void Architecture(Utility::Architecture arch) { m_arch = arch; } @@ -35,6 +36,7 @@ namespace AppInstaller::Utility const std::string& Publisher() const { return m_publisher; } void Publisher(std::string&& publisher) { m_publisher = std::move(publisher); } + void Publisher(std::string_view publisher) { m_publisher = publisher; } private: std::string m_name; @@ -50,7 +52,14 @@ namespace AppInstaller::Utility { virtual ~INameNormalizer() = default; + // Normalize both the name and publisher at the same time. virtual NormalizedName Normalize(std::string_view name, std::string_view publisher) const = 0; + + // Normalize only the name. + virtual NormalizedName NormalizeName(std::string_view name) const = 0; + + // Normalize only the publisher. + virtual std::string NormalizePublisher(std::string_view publisher) const = 0; }; } @@ -61,6 +70,8 @@ namespace AppInstaller::Utility NameNormalizer(NormalizationVersion version); NormalizedName Normalize(std::string_view name, std::string_view publisher) const; + NormalizedName NormalizeName(std::string_view name) const; + std::string NormalizePublisher(std::string_view publisher) const; private: std::unique_ptr<details::INameNormalizer> m_normalizer; diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj @@ -195,6 +195,10 @@ <ClInclude Include="Microsoft\Schema\1_1\PackageFamilyNameTable.h" /> <ClInclude Include="Microsoft\Schema\1_1\ProductCodeTable.h" /> <ClInclude Include="Microsoft\Schema\1_1\SearchResultsTable.h" /> + <ClInclude Include="Microsoft\Schema\1_2\Interface.h" /> + <ClInclude Include="Microsoft\Schema\1_2\NormalizedPackageNameTable.h" /> + <ClInclude Include="Microsoft\Schema\1_2\NormalizedPackagePublisherTable.h" /> + <ClInclude Include="Microsoft\Schema\1_2\SearchResultsTable.h" /> <ClInclude Include="Microsoft\Schema\ISQLiteIndex.h" /> <ClInclude Include="Microsoft\Schema\MetadataTable.h" /> <ClInclude Include="Microsoft\Schema\Version.h" /> @@ -232,6 +236,8 @@ <ClCompile Include="Microsoft\Schema\1_1\Interface_1_1.cpp" /> <ClCompile Include="Microsoft\Schema\1_1\ManifestMetadataTable.cpp" /> <ClCompile Include="Microsoft\Schema\1_1\SearchResultsTable_1_1.cpp" /> + <ClCompile Include="Microsoft\Schema\1_2\Interface_1_2.cpp" /> + <ClCompile Include="Microsoft\Schema\1_2\SearchResultsTable_1_2.cpp" /> <ClCompile Include="Microsoft\Schema\MetadataTable.cpp" /> <ClCompile Include="Microsoft\Schema\Version.cpp" /> <ClCompile Include="Microsoft\SQLiteIndex.cpp" /> diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters @@ -31,6 +31,9 @@ <Filter Include="Microsoft\Schema\1_1"> <UniqueIdentifier>{aef8989c-44fd-4848-ae2c-c81d3f2e2c72}</UniqueIdentifier> </Filter> + <Filter Include="Microsoft\Schema\1_2"> + <UniqueIdentifier>{dc8b6163-e9b5-4d05-87bc-d6098afe0f92}</UniqueIdentifier> + </Filter> </ItemGroup> <ItemGroup> <ClInclude Include="pch.h"> @@ -138,6 +141,18 @@ <ClInclude Include="Microsoft\ARPHelper.h"> <Filter>Microsoft</Filter> </ClInclude> + <ClInclude Include="Microsoft\Schema\1_2\Interface.h"> + <Filter>Microsoft\Schema\1_2</Filter> + </ClInclude> + <ClInclude Include="Microsoft\Schema\1_2\SearchResultsTable.h"> + <Filter>Microsoft\Schema\1_2</Filter> + </ClInclude> + <ClInclude Include="Microsoft\Schema\1_2\NormalizedPackageNameTable.h"> + <Filter>Microsoft\Schema\1_2</Filter> + </ClInclude> + <ClInclude Include="Microsoft\Schema\1_2\NormalizedPackagePublisherTable.h"> + <Filter>Microsoft\Schema\1_2</Filter> + </ClInclude> </ItemGroup> <ItemGroup> <ClCompile Include="pch.cpp"> @@ -209,6 +224,12 @@ <ClCompile Include="Microsoft\ARPHelper.cpp"> <Filter>Microsoft</Filter> </ClCompile> + <ClCompile Include="Microsoft\Schema\1_2\Interface_1_2.cpp"> + <Filter>Microsoft\Schema\1_2</Filter> + </ClCompile> + <ClCompile Include="Microsoft\Schema\1_2\SearchResultsTable_1_2.cpp"> + <Filter>Microsoft\Schema\1_2</Filter> + </ClCompile> </ItemGroup> <ItemGroup> <None Include="PropertySheet.props" /> diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -106,6 +106,22 @@ namespace AppInstaller::Repository return (latest && (GetVACFromVersion(installed.get()).IsUpdatedBy(GetVACFromVersion(latest.get())))); } + bool IsSame(const IPackage* other) const override + { + const CompositePackage* otherComposite = dynamic_cast<const CompositePackage*>(other); + + if (!otherComposite || + static_cast<bool>(m_installedPackage) != static_cast<bool>(otherComposite->m_installedPackage) || + m_installedPackage && !m_installedPackage->IsSame(otherComposite->m_installedPackage.get()) || + static_cast<bool>(m_availablePackage) != static_cast<bool>(otherComposite->m_availablePackage) || + m_availablePackage && !m_availablePackage->IsSame(otherComposite->m_availablePackage.get())) + { + return false; + } + + return true; + } + void SetAvailablePackage(std::shared_ptr<IPackage> availablePackage) { m_availablePackage = std::move(availablePackage); @@ -186,6 +202,18 @@ namespace AppInstaller::Repository // Lie here so that list and upgrade will carry on to be able to output the diagnostic information. return true; } + + bool IsSame(const IPackage* other) const override + { + const UnknownAvailablePackage* otherUnknown = dynamic_cast<const UnknownAvailablePackage*>(other); + + if (otherUnknown) + { + return true; + } + + return false; + } }; // The comparator compares the ResultMatch by MatchType first, then Field in a predefined order. @@ -437,7 +465,7 @@ namespace AppInstaller::Repository { for (const auto& srs : installedPackageData.SystemReferenceStrings) { - systemReferenceSearch.Inclusions.emplace_back(PackageMatchFilter(srs.Field, MatchType::Exact, srs.String)); + systemReferenceSearch.Inclusions.emplace_back(PackageMatchFilter(srs.Field, MatchType::Exact, srs.String.get())); } std::shared_ptr<IPackage> availablePackage; @@ -523,7 +551,7 @@ namespace AppInstaller::Repository SearchRequest systemReferenceSearch; for (const auto& srs : packageData->SystemReferenceStrings) { - systemReferenceSearch.Inclusions.emplace_back(PackageMatchFilter(srs.Field, MatchType::Exact, srs.String)); + systemReferenceSearch.Inclusions.emplace_back(PackageMatchFilter(srs.Field, MatchType::Exact, srs.String.get())); } SearchResult installedCrossRef = m_installedSource->Search(systemReferenceSearch); diff --git a/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp b/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp @@ -99,6 +99,7 @@ namespace AppInstaller::Repository::Microsoft std::string ARPHelper::DetermineVersion(const Registry::Key& arpKey) const { + // First check DisplayVersion for a complete version string auto displayVersion = arpKey[DisplayVersion]; if (displayVersion && displayVersion->GetType() == Registry::Value::Type::String) { @@ -109,39 +110,44 @@ namespace AppInstaller::Repository::Microsoft } } - auto version = arpKey[Version]; - if (version && version->GetType() == Registry::Value::Type::DWord) + // Next attempt VersionMajor.VersionMinor, then MajorVersion.MinorVersion + for (const auto& names : { std::make_pair(std::ref(VersionMajor), std::ref(VersionMinor)), std::make_pair(std::ref(MajorVersion), std::ref(MinorVersion)) }) { - uint32_t versionInt = version->GetValue<Registry::Value::Type::DWord>(); - if (versionInt) + auto majorVersion = arpKey[names.first]; + auto minorVersion = arpKey[names.second]; + if (majorVersion || minorVersion) { - std::ostringstream strstr; - strstr << ((versionInt & 0xFF000000) >> 24) << '.' << ((versionInt & 0x00FF0000) >> 16) << '.' << (versionInt & 0x0000FFFF); - return strstr.str(); - } - } + uint32_t majorVersionInt = 0; + uint32_t minorVersionInt = 0; - auto majorVersion = arpKey[VersionMajor]; - auto minorVersion = arpKey[VersionMinor]; - if (majorVersion || minorVersion) - { - uint32_t majorVersionInt = 0; - uint32_t minorVersionInt = 0; + if (majorVersion && majorVersion->GetType() == Registry::Value::Type::DWord) + { + majorVersionInt = majorVersion->GetValue<Registry::Value::Type::DWord>(); + } - if (majorVersion && majorVersion->GetType() == Registry::Value::Type::DWord) - { - majorVersionInt = majorVersion->GetValue<Registry::Value::Type::DWord>(); - } + if (minorVersion && minorVersion->GetType() == Registry::Value::Type::DWord) + { + minorVersionInt = minorVersion->GetValue<Registry::Value::Type::DWord>(); + } - if (minorVersion && minorVersion->GetType() == Registry::Value::Type::DWord) - { - minorVersionInt = minorVersion->GetValue<Registry::Value::Type::DWord>(); + if (majorVersionInt || minorVersionInt) + { + std::ostringstream strstr; + strstr << majorVersionInt << '.' << minorVersionInt; + return strstr.str(); + } } + } - if (majorVersionInt || minorVersionInt) + // Finally attempt to turn the Version DWORD into a version string + auto version = arpKey[Version]; + if (version && version->GetType() == Registry::Value::Type::DWord) + { + uint32_t versionInt = version->GetValue<Registry::Value::Type::DWord>(); + if (versionInt) { std::ostringstream strstr; - strstr << majorVersionInt << '.' << minorVersionInt; + strstr << ((versionInt & 0xFF000000) >> 24) << '.' << ((versionInt & 0x00FF0000) >> 16) << '.' << (versionInt & 0x0000FFFF); return strstr.str(); } } @@ -164,6 +170,12 @@ namespace AppInstaller::Repository::Microsoft { valueString = value->GetValue<Registry::Value::Type::ExpandString>(); } + else if (value->GetType() == Registry::Value::Type::DWord) + { + std::ostringstream strstr; + strstr << value->GetValue<Registry::Value::Type::DWord>(); + valueString = strstr.str(); + } if (!valueString.empty()) { @@ -246,36 +258,38 @@ namespace AppInstaller::Repository::Microsoft if (publisher && publisher->GetType() == Registry::Value::Type::String) { manifest.DefaultLocalization.Add<Manifest::Localization::Publisher>(publisher->GetValue<Registry::Value::Type::String>()); + + // If Publisher is set, change the Id using name normalization + // TODO: Figure out how to actually make this work since there are often instances of the same + // data in x64 and x86 entries that will collide. + //auto normalizedName = index.NormalizeName( + // manifest.DefaultLocalization.Get<Manifest::Localization::PackageName>(), + // manifest.DefaultLocalization.Get<Manifest::Localization::Publisher>()); + //manifest.Id = normalizedName.Publisher() + '.' + normalizedName.Name(); } // TODO: If we want to keep the constructed manifest around to allow for `show` type commands // against installed packages, we should use URLInfoAbout/HelpLink for the Homepage. - // TODO: Pick up Language/InnoSetupLanguage to enable proper selection of language for upgrade. - - // TODO: Determine the best way to handle duplicates, which may very well happen. - // For now, we will attempt to insert and catch, then send failure telemetry. - // In a future where we cache these entries + // TODO: Determine the best way to handle duplicates; sometimes the same package will be listed under + // both x64 and x86 locations for ARP. + // For now, we will attempt to insert and catch. std::optional<SQLiteIndex::IdType> manifestIdOpt; - HRESULT addHr = S_OK; try { // Use the ProductCode as a unique key for the path manifestIdOpt = index.AddManifest(manifest, Utility::ConvertToUTF16(manifest.Installers[0].ProductCode)); } - catch (wil::ResultException& re) - { - addHr = re.GetErrorCode(); - } catch (...) { - addHr = E_FAIL; + // Ignore errors if they occur, they are most likely a duplicate value } if (!manifestIdOpt) { - Logging::Telemetry().LogDuplicateARPEntry(addHr, scope, architecture, productCode, manifest.DefaultLocalization.Get<Manifest::Localization::PackageName>()); + AICLI_LOG(Repo, Warning, + << "Ignoring duplicate ARP entry " << scope << '|' << architecture << '|' << productCode << " [" << manifest.DefaultLocalization.Get<Manifest::Localization::PackageName>() << "]"); continue; } @@ -288,6 +302,14 @@ namespace AppInstaller::Repository::Microsoft // is from it's ARP location, despite it very clearly being a specific architecture. And note that user // scope does not have separate ARP locations, so every architecture would appear as native. + // Publisher is needed for certain scenarios but we don't store it from the manifest + if (manifest.DefaultLocalization.Contains(Manifest::Localization::Publisher)) + { + index.SetMetadataByManifestId( + manifestId, PackageVersionMetadata::Publisher, + manifest.DefaultLocalization.Get<Manifest::Localization::Publisher>()); + } + // Pick up InstallLocation when upgrade supports remove/install to enable this location // to survive across the removal. AddMetadataIfPresent(arpKey, InstallLocation, index, manifestId, PackageVersionMetadata::InstalledLocation); @@ -296,6 +318,10 @@ namespace AppInstaller::Repository::Microsoft AddMetadataIfPresent(arpKey, UninstallString, index, manifestId, PackageVersionMetadata::StandardUninstallCommand); AddMetadataIfPresent(arpKey, QuietUninstallString, index, manifestId, PackageVersionMetadata::SilentUninstallCommand); + // Pick up Language to enable proper selection of language for upgrade. + // TODO: Determine if InnoSetupLanguage represents the same concept and pick it up if language is not present. + AddMetadataIfPresent(arpKey, Language, index, manifestId, PackageVersionMetadata::Locale); + // Pick up WindowsInstaller to determine if this is an MSI install. // TODO: Could also determine Inno (and maybe other types) through detecting other keys here. auto installedType = Manifest::InstallerTypeEnum::Exe; diff --git a/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.h b/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.h @@ -29,6 +29,10 @@ namespace AppInstaller::Repository::Microsoft std::wstring VersionMajor{ L"VersionMajor" }; // REG_DWORD std::wstring VersionMinor{ L"VersionMinor" }; + // REG_DWORD + std::wstring MajorVersion{ L"MajorVersion" }; + // REG_DWORD + std::wstring MinorVersion{ L"MinorVersion" }; // REG_SZ std::wstring URLInfoAbout{ L"URLInfoAbout" }; // REG_SZ diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp @@ -116,26 +116,11 @@ namespace AppInstaller::Repository::Microsoft SQLiteIndex index = SQLiteIndex::CreateNew(SQLITE_MEMORY_DB_CONNECTION_TARGET, Schema::Version::Latest()); // Put installed packages into the index - std::optional<ARPHelper> arpHelper; - - if (filter == PredefinedInstalledSourceFactory::Filter::None || filter == PredefinedInstalledSourceFactory::Filter::ARP_System) - { - if (!arpHelper) - { - arpHelper = ARPHelper(); - } - - arpHelper->PopulateIndexFromARP(index, Manifest::ScopeEnum::Machine); - } - - if (filter == PredefinedInstalledSourceFactory::Filter::None || filter == PredefinedInstalledSourceFactory::Filter::ARP_User) + if (filter == PredefinedInstalledSourceFactory::Filter::None || filter == PredefinedInstalledSourceFactory::Filter::ARP) { - if (!arpHelper) - { - arpHelper = ARPHelper(); - } - - arpHelper->PopulateIndexFromARP(index, Manifest::ScopeEnum::User); + ARPHelper arpHelper; + arpHelper.PopulateIndexFromARP(index, Manifest::ScopeEnum::Machine); + arpHelper.PopulateIndexFromARP(index, Manifest::ScopeEnum::User); } if (filter == PredefinedInstalledSourceFactory::Filter::None || filter == PredefinedInstalledSourceFactory::Filter::MSIX) @@ -172,10 +157,8 @@ namespace AppInstaller::Repository::Microsoft { case AppInstaller::Repository::Microsoft::PredefinedInstalledSourceFactory::Filter::None: return "None"sv; - case AppInstaller::Repository::Microsoft::PredefinedInstalledSourceFactory::Filter::ARP_System: - return "ARP_System"sv; - case AppInstaller::Repository::Microsoft::PredefinedInstalledSourceFactory::Filter::ARP_User: - return "ARP_User"sv; + case AppInstaller::Repository::Microsoft::PredefinedInstalledSourceFactory::Filter::ARP: + return "ARP"sv; case AppInstaller::Repository::Microsoft::PredefinedInstalledSourceFactory::Filter::MSIX: return "MSIX"sv; default: @@ -185,13 +168,9 @@ namespace AppInstaller::Repository::Microsoft PredefinedInstalledSourceFactory::Filter PredefinedInstalledSourceFactory::StringToFilter(std::string_view filter) { - if (filter == FilterToString(Filter::ARP_System)) - { - return Filter::ARP_System; - } - else if (filter == FilterToString(Filter::ARP_User)) + if (filter == FilterToString(Filter::ARP)) { - return Filter::ARP_User; + return Filter::ARP; } else if (filter == FilterToString(Filter::MSIX)) { diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.h b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.h @@ -25,8 +25,7 @@ namespace AppInstaller::Repository::Microsoft enum class Filter { None, - ARP_System, - ARP_User, + ARP, MSIX, }; diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp @@ -133,7 +133,7 @@ namespace AppInstaller::Repository::Microsoft SQLiteIndex::IdType SQLiteIndex::AddManifest(const std::filesystem::path& manifestPath, const std::filesystem::path& relativePath) { - AICLI_LOG(Repo, Info, << "Adding manifest from file [" << manifestPath << "]"); + AICLI_LOG(Repo, Verbose, << "Adding manifest from file [" << manifestPath << "]"); Manifest::Manifest manifest = Manifest::YamlParser::CreateFromPath(manifestPath); return AddManifest(manifest, relativePath); @@ -141,7 +141,7 @@ namespace AppInstaller::Repository::Microsoft SQLiteIndex::IdType SQLiteIndex::AddManifest(const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) { - AICLI_LOG(Repo, Info, << "Adding manifest for [" << manifest.Id << ", " << manifest.Version << "] at relative path [" << relativePath << "]"); + AICLI_LOG(Repo, Verbose, << "Adding manifest for [" << manifest.Id << ", " << manifest.Version << "] at relative path [" << relativePath << "]"); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "sqliteindex_addmanifest"); @@ -156,7 +156,7 @@ namespace AppInstaller::Repository::Microsoft bool SQLiteIndex::UpdateManifest(const std::filesystem::path& manifestPath, const std::filesystem::path& relativePath) { - AICLI_LOG(Repo, Info, << "Updating manifest from file [" << manifestPath << "]"); + AICLI_LOG(Repo, Verbose, << "Updating manifest from file [" << manifestPath << "]"); Manifest::Manifest manifest = Manifest::YamlParser::CreateFromPath(manifestPath); return UpdateManifest(manifest, relativePath); @@ -164,7 +164,7 @@ namespace AppInstaller::Repository::Microsoft bool SQLiteIndex::UpdateManifest(const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) { - AICLI_LOG(Repo, Info, << "Updating manifest for [" << manifest.Id << ", " << manifest.Version << "] at relative path [" << relativePath << "]"); + AICLI_LOG(Repo, Verbose, << "Updating manifest for [" << manifest.Id << ", " << manifest.Version << "] at relative path [" << relativePath << "]"); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "sqliteindex_updatemanifest"); @@ -182,7 +182,7 @@ namespace AppInstaller::Repository::Microsoft void SQLiteIndex::RemoveManifest(const std::filesystem::path& manifestPath, const std::filesystem::path& relativePath) { - AICLI_LOG(Repo, Info, << "Removing manifest from file [" << manifestPath << "]"); + AICLI_LOG(Repo, Verbose, << "Removing manifest from file [" << manifestPath << "]"); Manifest::Manifest manifest = Manifest::YamlParser::CreateFromPath(manifestPath); RemoveManifest(manifest, relativePath); @@ -190,7 +190,7 @@ namespace AppInstaller::Repository::Microsoft void SQLiteIndex::RemoveManifest(const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) { - AICLI_LOG(Repo, Info, << "Removing manifest for [" << manifest.Id << ", " << manifest.Version << "] at relative path [" << relativePath << "]"); + AICLI_LOG(Repo, Verbose, << "Removing manifest for [" << manifest.Id << ", " << manifest.Version << "] at relative path [" << relativePath << "]"); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "sqliteindex_removemanifest"); @@ -221,7 +221,7 @@ namespace AppInstaller::Repository::Microsoft Schema::ISQLiteIndex::SearchResult SQLiteIndex::Search(const SearchRequest& request) const { - AICLI_LOG(Repo, Info, << "Performing search: " << request.ToString()); + AICLI_LOG(Repo, Verbose, << "Performing search: " << request.ToString()); return m_interface->Search(m_dbconn, request); } @@ -256,6 +256,11 @@ namespace AppInstaller::Repository::Microsoft m_interface->SetMetadataByManifestId(m_dbconn, manifestId, metadata, value); } + Utility::NormalizedName SQLiteIndex::NormalizeName(std::string_view name, std::string_view publisher) const + { + return m_interface->NormalizeName(name, publisher); + } + // Recording last write time based on MSDN documentation stating that time returns a POSIX epoch time and thus // should be consistent across systems. void SQLiteIndex::SetLastWriteTime() diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h @@ -8,6 +8,7 @@ #include <AppInstallerLanguageUtilities.h> #include <AppInstallerVersions.h> #include <winget/Manifest.h> +#include <winget/NameNormalization.h> #include <chrono> #include <filesystem> @@ -121,6 +122,10 @@ namespace AppInstaller::Repository::Microsoft // Sets the string for the given metadata and manifest id. void SetMetadataByManifestId(IdType manifestId, PackageVersionMetadata metadata, std::string_view value); + // Normalizes a name using the internal rules used by the index. + // Largely a utility function; should not be used to do work on behalf of the index by the caller. + Utility::NormalizedName NormalizeName(std::string_view name, std::string_view publisher) const; + private: // Constructor used to open an existing index. SQLiteIndex(const std::string& target, SQLite::Connection::OpenDisposition disposition, SQLite::Connection::OpenFlags flags); diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp @@ -177,6 +177,11 @@ namespace AppInstaller::Repository::Microsoft return result; } + bool IsSame(const PackageBase& other) const + { + return m_idId == other.m_idId; + } + protected: std::shared_ptr<IPackageVersion> GetLatestVersionInternal() const { @@ -252,6 +257,18 @@ namespace AppInstaller::Repository::Microsoft { return false; } + + bool IsSame(const IPackage* other) const override + { + const AvailablePackage* otherAvailable = dynamic_cast<const AvailablePackage*>(other); + + if (otherAvailable) + { + return PackageBase::IsSame(*otherAvailable); + } + + return false; + } }; // The IPackage impl for SQLiteIndexSource of Installed packages. @@ -289,6 +306,18 @@ namespace AppInstaller::Repository::Microsoft { return false; } + + bool IsSame(const IPackage* other) const override + { + const InstalledPackage* otherInstalled = dynamic_cast<const InstalledPackage*>(other); + + if (otherInstalled) + { + return PackageBase::IsSame(*otherInstalled); + } + + return false; + } }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h @@ -31,6 +31,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 MetadataResult GetMetadataByManifestId(const SQLite::Connection& connection, SQLite::rowid_t manifestId) const override; void SetMetadataByManifestId(SQLite::Connection& connection, SQLite::rowid_t manifestId, PackageVersionMetadata metadata, std::string_view value) override; + // Version 1.2 + Utility::NormalizedName NormalizeName(std::string_view name, std::string_view publisher) const override; + protected: // Creates the search results table. virtual std::unique_ptr<SearchResultsTable> CreateSearchResultsTable(const SQLite::Connection& connection) const; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp @@ -29,21 +29,21 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 std::optional<SQLite::rowid_t> idId = IdTable::SelectIdByValue(connection, manifest.Id, true); if (!idId) { - AICLI_LOG(Repo, Info, << "Did not find an Id { " << manifest.Id << " }"); + AICLI_LOG(Repo, Verbose, << "Did not find an Id { " << manifest.Id << " }"); return {}; } std::optional<SQLite::rowid_t> versionId = VersionTable::SelectIdByValue(connection, manifest.Version, true); if (!versionId) { - AICLI_LOG(Repo, Info, << "Did not find a Version { " << manifest.Version << " }"); + AICLI_LOG(Repo, Verbose, << "Did not find a Version { " << manifest.Version << " }"); return {}; } std::optional<SQLite::rowid_t> channelId = ChannelTable::SelectIdByValue(connection, manifest.Channel, true); if (!channelId) { - AICLI_LOG(Repo, Info, << "Did not find a Channel { " << manifest.Channel << " }"); + AICLI_LOG(Repo, Verbose, << "Did not find a Channel { " << manifest.Channel << " }"); return {}; } @@ -51,7 +51,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 if (!result) { - AICLI_LOG(Repo, Info, << "Did not find a manifest row for { " << manifest.Id << ", " << manifest.Version << ", " << manifest.Channel << " }"); + AICLI_LOG(Repo, Verbose, << "Did not find a manifest row for { " << manifest.Id << ", " << manifest.Version << ", " << manifest.Channel << " }"); } return result; @@ -413,7 +413,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 SearchResult result; for (SQLite::rowid_t id : ids) { - result.Matches.emplace_back(std::make_pair(id, PackageMatchFilter(PackageMatchField::Id, MatchType::Wildcard, {}))); + result.Matches.emplace_back(std::make_pair(id, PackageMatchFilter(PackageMatchField::Id, MatchType::Wildcard))); } result.Truncated = (request.MaximumResults && IdTable::GetCount(connection) > request.MaximumResults); @@ -438,11 +438,12 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 if (!request.Inclusions.empty()) { - for (const auto& include : request.Inclusions) + for (auto include : request.Inclusions) { for (MatchType match : GetMatchTypeOrder(include.Type)) { - resultsTable->SearchOnField(include.Field, match, include.Value); + include.Type = match; + resultsTable->SearchOnField(include); } } @@ -455,11 +456,12 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 THROW_HR_IF(E_UNEXPECTED, request.Filters.empty()); // Perform search for just the field matching the first filter - const PackageMatchFilter& filter = request.Filters[0]; + PackageMatchFilter filter = request.Filters[0]; for (MatchType match : GetMatchTypeOrder(filter.Type)) { - resultsTable->SearchOnField(filter.Field, match, filter.Value); + filter.Type = match; + resultsTable->SearchOnField(filter); } // Skip the filter as we already know everything matches @@ -472,13 +474,14 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 // Second phase, for remaining filters, flag matching search results, then remove unflagged values. for (size_t i = filterIndex; i < request.Filters.size(); ++i) { - const PackageMatchFilter& filter = request.Filters[i]; + PackageMatchFilter filter = request.Filters[i]; resultsTable->PrepareToFilter(); for (MatchType match : GetMatchTypeOrder(filter.Type)) { - resultsTable->FilterOnField(filter.Field, match, filter.Value); + filter.Type = match; + resultsTable->FilterOnField(filter); } resultsTable->CompleteFilter(); @@ -547,6 +550,14 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 { } + Utility::NormalizedName Interface::NormalizeName(std::string_view name, std::string_view publisher) const + { + Utility::NormalizedName result; + result.Name(name); + result.Publisher(publisher); + return result; + } + std::unique_ptr<SearchResultsTable> Interface::CreateSearchResultsTable(const SQLite::Connection& connection) const { return std::make_unique<SearchResultsTable>(connection); @@ -577,13 +588,18 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 void Interface::PerformQuerySearch(SearchResultsTable& resultsTable, const RequestMatch& query) const { + // Arbitrary values to create a reusable filter with the given value. + PackageMatchFilter filter(PackageMatchField::Id, MatchType::Exact, query.Value); + for (MatchType match : GetMatchTypeOrder(query.Type)) { - resultsTable.SearchOnField(PackageMatchField::Id, match, query.Value); - resultsTable.SearchOnField(PackageMatchField::Name, match, query.Value); - resultsTable.SearchOnField(PackageMatchField::Moniker, match, query.Value); - resultsTable.SearchOnField(PackageMatchField::Command, match, query.Value); - resultsTable.SearchOnField(PackageMatchField::Tag, match, query.Value); + filter.Type = match; + + for (auto field : { PackageMatchField::Id, PackageMatchField::Name, PackageMatchField::Moniker, PackageMatchField::Command, PackageMatchField::Tag }) + { + filter.Field = field; + resultsTable.SearchOnField(filter); + } } } } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.cpp @@ -165,10 +165,10 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return result; } - int ManifestTableBuildSearchStatement( + std::vector<int> ManifestTableBuildSearchStatement( SQLite::Builder::StatementBuilder& builder, - const SQLite::Builder::QualifiedColumn& column, - bool isOneToOne, + std::initializer_list<SQLite::Builder::QualifiedColumn> columns, + std::initializer_list<bool> isOneToOnes, std::string_view manifestAlias, std::string_view valueAlias, bool useLike) @@ -176,40 +176,78 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 using QCol = SQLite::Builder::QualifiedColumn; // Build a statement like: - // SELECT manifest.rowid as m, ids.id as v from manifest join ids on manifest.id = ids.rowid where ids.id = <value> + // SELECT manifest.rowid as m, ids.id as v from manifest + // join ids on manifest.id = ids.rowid + // where ids.id = <value> // OR - // SELECT manifest.rowid as m, tags.tag as v from manifest join tags_map on manifest.rowid = tags_map.manifest - // join tags on tags_map.tag = tags.rowid where tags.tag = <value> + // SELECT manifest.rowid as m, tags.tag as v from manifest + // join tags_map on manifest.rowid = tags_map.manifest + // join tags on tags_map.tag = tags.rowid + // where tags.tag = <value> + // Where the joins and where portions are repeated for each table in question. builder.Select(). - Column(QCol(s_ManifestTable_Table_Name, SQLite::RowIDName)).As(manifestAlias). - Column(column).As(valueAlias); + Column(QCol(s_ManifestTable_Table_Name, SQLite::RowIDName)).As(manifestAlias); - if (isOneToOne) + // Value will be captured for single tables references, and left empty for multi-tables + if (columns.size() == 1) { - builder.From(s_ManifestTable_Table_Name). - Join(column.Table).On(QCol(s_ManifestTable_Table_Name, column.Column), QCol(column.Table, SQLite::RowIDName)). - Where(column); + builder.Column(*columns.begin()); } else { - std::string mapTableName = details::OneToManyTableGetMapTableName(column.Table); - builder.From(s_ManifestTable_Table_Name). - Join(mapTableName).On(QCol(s_ManifestTable_Table_Name, SQLite::RowIDName), QCol(mapTableName, details::OneToManyTableGetManifestColumnName())). - Join(column.Table).On(QCol(mapTableName, column.Column), QCol(column.Table, SQLite::RowIDName)). - Where(column); + builder.LiteralColumn(""); } - int result = 0; - if (useLike) + builder.As(valueAlias).From(s_ManifestTable_Table_Name); + + // Create join clauses + THROW_HR_IF(E_INVALIDARG, columns.size() != isOneToOnes.size()); + auto columnItr = columns.begin(); + auto isOneToOneItr = isOneToOnes.begin(); + + for (; columnItr != columns.end(); ++columnItr, ++isOneToOneItr) { - builder.Like(SQLite::Builder::Unbound); - result = builder.GetLastBindIndex(); - builder.Escape(SQLite::EscapeCharForLike); + const SQLite::Builder::QualifiedColumn& column = *columnItr; + + if (*isOneToOneItr) + { + builder. + Join(column.Table).On(QCol(s_ManifestTable_Table_Name, column.Column), QCol(column.Table, SQLite::RowIDName)); + } + else + { + std::string mapTableName = details::OneToManyTableGetMapTableName(column.Table); + builder. + Join(mapTableName).On(QCol(s_ManifestTable_Table_Name, SQLite::RowIDName), QCol(mapTableName, details::OneToManyTableGetManifestColumnName())). + Join(column.Table).On(QCol(mapTableName, column.Column), QCol(column.Table, SQLite::RowIDName)); + } } - else + + std::vector<int> result; + + // Create where clause + for (const SQLite::Builder::QualifiedColumn& column : columns) { - builder.Equals(SQLite::Builder::Unbound); - result = builder.GetLastBindIndex(); + if (result.empty()) + { + builder.Where(column); + } + else + { + builder.And(column); + } + + if (useLike) + { + builder.Like(SQLite::Builder::Unbound); + result.push_back(builder.GetLastBindIndex()); + builder.Escape(SQLite::EscapeCharForLike); + } + else + { + builder.Equals(SQLite::Builder::Unbound); + result.push_back(builder.GetLastBindIndex()); + } } return result; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/ManifestTable.h @@ -46,10 +46,10 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 std::initializer_list<SQLite::rowid_t> ids); // Builds the search select statement base on the given values. - int ManifestTableBuildSearchStatement( + std::vector<int> ManifestTableBuildSearchStatement( SQLite::Builder::StatementBuilder& builder, - const SQLite::Builder::QualifiedColumn& column, - bool isOneToOne, + std::initializer_list<SQLite::Builder::QualifiedColumn> columns, + std::initializer_list<bool> isOneToOnes, std::string_view manifestAlias, std::string_view valueAlias, bool useLike); @@ -138,10 +138,12 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 } // Builds the search select statement base on the given values. - template <typename Table> - static int BuildSearchStatement(SQLite::Builder::StatementBuilder& builder, std::string_view manifestAlias, std::string_view valueAlias, bool useLike) + // If more than one table is provided, no value will be captured. + // The return value is the bind indices of the values to match against. + template <typename... Table> + static std::vector<int> BuildSearchStatement(SQLite::Builder::StatementBuilder& builder, std::string_view manifestAlias, std::string_view valueAlias, bool useLike) { - return details::ManifestTableBuildSearchStatement(builder, SQLite::Builder::QualifiedColumn{ Table::TableName(), Table::ValueName() }, Table::IsOneToOne(), manifestAlias, valueAlias, useLike); + return details::ManifestTableBuildSearchStatement(builder, { SQLite::Builder::QualifiedColumn{ Table::TableName(), Table::ValueName() }... }, { Table::IsOneToOne()... }, manifestAlias, valueAlias, useLike); } // Update the value of a single column for the manifest with the given rowid. diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/SearchResultsTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/SearchResultsTable.h @@ -25,7 +25,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 SearchResultsTable& operator=(SearchResultsTable&&) = default; // Performs the requested search type on the requested field. - void SearchOnField(PackageMatchField field, MatchType match, std::string_view value); + void SearchOnField(const PackageMatchFilter& filter); // Removes rows with manifest ids whose sort order is below the highest one. void RemoveDuplicateManifestRows(); @@ -34,7 +34,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 void PrepareToFilter(); // Performs the requested filter type on the requested field. - void FilterOnField(PackageMatchField field, MatchType match, std::string_view value); + void FilterOnField(const PackageMatchFilter& filter); // Completes a filtering pass, removing filtered rows. void CompleteFilter(); @@ -44,15 +44,20 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 protected: // Builds the search statement for the specified field and match type. - std::optional<int> BuildSearchStatement(SQLite::Builder::StatementBuilder& builder, PackageMatchField field, MatchType match) const; + std::vector<int> BuildSearchStatement(SQLite::Builder::StatementBuilder& builder, PackageMatchField field, MatchType match) const; - virtual std::optional<int> BuildSearchStatement( + virtual std::vector<int> BuildSearchStatement( SQLite::Builder::StatementBuilder& builder, PackageMatchField field, std::string_view manifestAlias, std::string_view valueAlias, bool useLike) const; + static bool MatchUsesLike(MatchType match); + void BindStatementForMatchType(SQLite::Statement& statement, MatchType match, int bindIndex, std::string_view value); + + virtual void BindStatementForMatchType(SQLite::Statement& statement, const PackageMatchFilter& filter, const std::vector<int>& bindIndex); + private: const SQLite::Connection& m_connection; int m_sortOrdinalValue = 0; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/SearchResultsTable_1_0.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/SearchResultsTable_1_0.cpp @@ -32,49 +32,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 constexpr std::string_view s_SearchResultsTable_SubSelect_TableAlias = "valueTable"sv; constexpr std::string_view s_SearchResultsTable_SubSelect_ManifestAlias = "m"sv; constexpr std::string_view s_SearchResultsTable_SubSelect_ValueAlias = "v"sv; - - bool MatchUsesLike(MatchType match) - { - return (match != MatchType::Exact); - } - - void ExecuteStatementForMatchType(SQLite::Statement& statement, MatchType match, int bindIndex, bool escapeValueForLike, std::string_view value) - { - // TODO: Implement these more complex match types - if (match == MatchType::Wildcard || match == MatchType::Fuzzy || match == MatchType::FuzzySubstring) - { - AICLI_LOG(Repo, Verbose, << "Specific match type not implemented, skipping: " << MatchTypeToString(match)); - return; - } - - std::string valueToUse; - - if (escapeValueForLike) - { - valueToUse = SQLite::EscapeStringForLike(value); - } - else - { - valueToUse = value; - } - - switch (match) - { - case AppInstaller::Repository::MatchType::StartsWith: - valueToUse += '%'; - break; - case AppInstaller::Repository::MatchType::Substring: - valueToUse = "%"s + valueToUse + '%'; - break; - default: - // No changes required for others. - break; - } - - statement.Bind(bindIndex, valueToUse); - - statement.Execute(); - } } SearchResultsTable::SearchResultsTable(const SQLite::Connection& connection) : @@ -113,7 +70,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 } } - void SearchResultsTable::SearchOnField(PackageMatchField field, MatchType match, std::string_view value) + void SearchResultsTable::SearchOnField(const PackageMatchFilter& filter) { using namespace SQLite::Builder; @@ -128,26 +85,27 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 StatementBuilder builder; builder.InsertInto(GetQualifiedName()).Select(). Column(QualifiedColumn(s_SearchResultsTable_SubSelect_TableAlias, s_SearchResultsTable_SubSelect_ManifestAlias)). - Value(field). - Value(match). + Value(filter.Field). + Value(filter.Type). Column(QualifiedColumn(s_SearchResultsTable_SubSelect_TableAlias, s_SearchResultsTable_SubSelect_ValueAlias)). Value(sortOrdinal). Value(false). From().BeginParenthetical(); // Add the field specific portion - std::optional<int> bindIndex = BuildSearchStatement(builder, field, match); + std::vector<int> bindIndex = BuildSearchStatement(builder, filter.Field, filter.Type); - if (!bindIndex) + if (bindIndex.empty()) { - AICLI_LOG(Repo, Verbose, << "PackageMatchField not supported in this version: " << PackageMatchFieldToString(field)); + AICLI_LOG(Repo, Verbose, << "PackageMatchField not supported in this version: " << PackageMatchFieldToString(filter.Field)); return; } builder.EndParenthetical().As(s_SearchResultsTable_SubSelect_TableAlias); SQLite::Statement statement = builder.Prepare(m_connection); - ExecuteStatementForMatchType(statement, match, bindIndex.value(), MatchUsesLike(match), value); + BindStatementForMatchType(statement, filter, bindIndex); + statement.Execute(); AICLI_LOG(Repo, Verbose, << "Search found " << m_connection.GetChanges() << " rows"); } @@ -183,7 +141,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 builder.Execute(m_connection); } - void SearchResultsTable::FilterOnField(PackageMatchField field, MatchType match, std::string_view value) + void SearchResultsTable::FilterOnField(const PackageMatchFilter& filter) { using namespace SQLite::Builder; @@ -200,18 +158,19 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 Select(s_SearchResultsTable_SubSelect_ManifestAlias).From().BeginParenthetical(); // Add the field specific portion - std::optional<int> bindIndex = BuildSearchStatement(builder, field, match); + std::vector<int> bindIndex = BuildSearchStatement(builder, filter.Field, filter.Type); - if (!bindIndex) + if (bindIndex.empty()) { - AICLI_LOG(Repo, Verbose, << "PackageMatchField not supported in this version: " << PackageMatchFieldToString(field)); + AICLI_LOG(Repo, Verbose, << "PackageMatchField not supported in this version: " << PackageMatchFieldToString(filter.Field)); return; } builder.EndParenthetical().EndParenthetical(); SQLite::Statement statement = builder.Prepare(m_connection); - ExecuteStatementForMatchType(statement, match, bindIndex.value(), MatchUsesLike(match), value); + BindStatementForMatchType(statement, filter, bindIndex); + statement.Execute(); AICLI_LOG(Repo, Verbose, << "Filter kept " << m_connection.GetChanges() << " rows"); } @@ -267,12 +226,12 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return result; } - std::optional<int> SearchResultsTable::BuildSearchStatement(SQLite::Builder::StatementBuilder& builder, PackageMatchField field, MatchType match) const + std::vector<int> SearchResultsTable::BuildSearchStatement(SQLite::Builder::StatementBuilder& builder, PackageMatchField field, MatchType match) const { return BuildSearchStatement(builder, field, s_SearchResultsTable_SubSelect_ManifestAlias, s_SearchResultsTable_SubSelect_ValueAlias, MatchUsesLike(match)); } - std::optional<int> SearchResultsTable::BuildSearchStatement( + std::vector<int> SearchResultsTable::BuildSearchStatement( SQLite::Builder::StatementBuilder& builder, PackageMatchField field, std::string_view manifestAlias, @@ -295,4 +254,50 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return {}; } } + + bool SearchResultsTable::MatchUsesLike(MatchType match) + { + return (match != MatchType::Exact); + } + + void SearchResultsTable::BindStatementForMatchType(SQLite::Statement& statement, MatchType match, int bindIndex, std::string_view value) + { + std::string valueToUse; + + if (MatchUsesLike(match)) + { + valueToUse = SQLite::EscapeStringForLike(value); + } + else + { + valueToUse = value; + } + + switch (match) + { + case AppInstaller::Repository::MatchType::StartsWith: + valueToUse += '%'; + break; + case AppInstaller::Repository::MatchType::Substring: + valueToUse = "%"s + valueToUse + '%'; + break; + default: + // No changes required for others. + break; + } + + statement.Bind(bindIndex, valueToUse); + } + + void SearchResultsTable::BindStatementForMatchType(SQLite::Statement& statement, const PackageMatchFilter& filter, const std::vector<int>& bindIndex) + { + // TODO: Implement these more complex match types + if (filter.Type == MatchType::Wildcard || filter.Type == MatchType::Fuzzy || filter.Type == MatchType::FuzzySubstring) + { + AICLI_LOG(Repo, Verbose, << "Specific match type not implemented, skipping: " << MatchTypeToString(filter.Type)); + return; + } + + BindStatementForMatchType(statement, filter.Type, bindIndex[0], filter.Value); + } } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface.h @@ -28,5 +28,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 protected: std::unique_ptr<V1_0::SearchResultsTable> CreateSearchResultsTable(const SQLite::Connection& connection) const override; void PerformQuerySearch(V1_0::SearchResultsTable& resultsTable, const RequestMatch& query) const override; + virtual SearchResult SearchInternal(const SQLite::Connection& connection, SearchRequest& request) const; + virtual void PrepareForPackaging(SQLite::Connection& connection, bool vacuum); }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp @@ -150,34 +150,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 void Interface::PrepareForPackaging(SQLite::Connection& connection) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "prepareforpackaging_v1_1"); - - V1_0::IdTable::PrepareForPackaging(connection); - V1_0::NameTable::PrepareForPackaging(connection); - V1_0::MonikerTable::PrepareForPackaging(connection); - V1_0::VersionTable::PrepareForPackaging(connection); - V1_0::ChannelTable::PrepareForPackaging(connection); - - V1_0::PathPartTable::PrepareForPackaging(connection); - - V1_0::ManifestTable::PrepareForPackaging(connection, { - V1_0::VersionTable::ValueName(), - V1_0::ChannelTable::ValueName(), - V1_0::PathPartTable::ValueName(), - }); - - V1_0::TagsTable::PrepareForPackaging(connection, false); - V1_0::CommandsTable::PrepareForPackaging(connection, false); - PackageFamilyNameTable::PrepareForPackaging(connection, true, true); - ProductCodeTable::PrepareForPackaging(connection, true, true); - - savepoint.Commit(); - - // Force the database to actually shrink the file size. - // This *must* be done outside of an active transaction. - SQLite::Builder::StatementBuilder builder; - builder.Vacuum(); - builder.Execute(connection); + PrepareForPackaging(connection, true); } bool Interface::CheckConsistency(const SQLite::Connection& connection, bool log) const @@ -200,29 +173,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 ISQLiteIndex::SearchResult Interface::Search(const SQLite::Connection& connection, const SearchRequest& request) const { - // Update any system reference strings to be folded - SearchRequest foldedRequest = request; - - auto foldIfNeeded = [](PackageMatchFilter& filter) - { - if ((filter.Field == PackageMatchField::PackageFamilyName || filter.Field == PackageMatchField::ProductCode) && - filter.Type == MatchType::Exact) - { - filter.Value = Utility::FoldCase(filter.Value); - } - }; - - for (auto& inclusion : foldedRequest.Inclusions) - { - foldIfNeeded(inclusion); - } - - for (auto& filter : foldedRequest.Filters) - { - foldIfNeeded(filter); - } - - return V1_0::Interface::Search(connection, foldedRequest); + SearchRequest updatedRequest = request; + return SearchInternal(connection, updatedRequest); } std::vector<std::string> Interface::GetMultiPropertyByManifestId(const SQLite::Connection& connection, SQLite::rowid_t manifestId, PackageVersionMultiProperty property) const @@ -273,11 +225,73 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 { // First, do an exact match search for the folded system reference strings // We do this first because it is exact, and likely won't match anything else if it matches this. - std::string foldedQuery = Utility::FoldCase(query.Value); - resultsTable.SearchOnField(PackageMatchField::PackageFamilyName, MatchType::Exact, foldedQuery); - resultsTable.SearchOnField(PackageMatchField::ProductCode, MatchType::Exact, foldedQuery); + PackageMatchFilter filter(PackageMatchField::PackageFamilyName, MatchType::Exact, Utility::FoldCase(query.Value)); + resultsTable.SearchOnField(filter); + + filter.Field = PackageMatchField::ProductCode; + resultsTable.SearchOnField(filter); // Then do the 1.0 search V1_0::Interface::PerformQuerySearch(resultsTable, query); } + + ISQLiteIndex::SearchResult Interface::SearchInternal(const SQLite::Connection& connection, SearchRequest& request) const + { + // Update any system reference strings to be folded + auto foldIfNeeded = [](PackageMatchFilter& filter) + { + if ((filter.Field == PackageMatchField::PackageFamilyName || filter.Field == PackageMatchField::ProductCode) && + filter.Type == MatchType::Exact) + { + filter.Value = Utility::FoldCase(filter.Value); + } + }; + + for (auto& inclusion : request.Inclusions) + { + foldIfNeeded(inclusion); + } + + for (auto& filter : request.Filters) + { + foldIfNeeded(filter); + } + + return V1_0::Interface::Search(connection, request); + } + + void Interface::PrepareForPackaging(SQLite::Connection& connection, bool vacuum) + { + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "prepareforpackaging_v1_1"); + + V1_0::IdTable::PrepareForPackaging(connection); + V1_0::NameTable::PrepareForPackaging(connection); + V1_0::MonikerTable::PrepareForPackaging(connection); + V1_0::VersionTable::PrepareForPackaging(connection); + V1_0::ChannelTable::PrepareForPackaging(connection); + + V1_0::PathPartTable::PrepareForPackaging(connection); + + V1_0::ManifestTable::PrepareForPackaging(connection, { + V1_0::VersionTable::ValueName(), + V1_0::ChannelTable::ValueName(), + V1_0::PathPartTable::ValueName(), + }); + + V1_0::TagsTable::PrepareForPackaging(connection, false); + V1_0::CommandsTable::PrepareForPackaging(connection, false); + PackageFamilyNameTable::PrepareForPackaging(connection, true, true); + ProductCodeTable::PrepareForPackaging(connection, true, true); + + savepoint.Commit(); + + if (vacuum) + { + // Force the database to actually shrink the file size. + // This *must* be done outside of an active transaction. + SQLite::Builder::StatementBuilder builder; + builder.Vacuum(); + builder.Execute(connection); + } + } } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/SearchResultsTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/SearchResultsTable.h @@ -18,7 +18,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 SearchResultsTable& operator=(SearchResultsTable&&) = default; protected: - std::optional<int> BuildSearchStatement( + std::vector<int> BuildSearchStatement( SQLite::Builder::StatementBuilder& builder, PackageMatchField field, std::string_view manifestAlias, diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/SearchResultsTable_1_1.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/SearchResultsTable_1_1.cpp @@ -11,7 +11,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 { - std::optional<int> SearchResultsTable::BuildSearchStatement( + std::vector<int> SearchResultsTable::BuildSearchStatement( SQLite::Builder::StatementBuilder& builder, PackageMatchField field, std::string_view manifestAlias, diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h @@ -0,0 +1,34 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "Microsoft/Schema/ISQLiteIndex.h" +#include "Microsoft/Schema/1_1/Interface.h" + + +namespace AppInstaller::Repository::Microsoft::Schema::V1_2 +{ + // Interface to this schema version exposed through ISQLiteIndex. + struct Interface : public V1_1::Interface + { + Interface(Utility::NormalizationVersion normVersion = Utility::NormalizationVersion::Initial); + + // Version 1.0 + Schema::Version GetVersion() const override; + void CreateTables(SQLite::Connection& connection) override; + SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; + std::pair<bool, SQLite::rowid_t> UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; + SQLite::rowid_t RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; + bool CheckConsistency(const SQLite::Connection& connection, bool log) const override; + + // Version 1.2 + Utility::NormalizedName NormalizeName(std::string_view name, std::string_view publisher) const override; + + protected: + std::unique_ptr<V1_0::SearchResultsTable> CreateSearchResultsTable(const SQLite::Connection& connection) const override; + SearchResult SearchInternal(const SQLite::Connection& connection, SearchRequest& request) const override; + void PrepareForPackaging(SQLite::Connection& connection, bool vacuum) override; + + // The name normalization utility + Utility::NameNormalizer m_normalizer; + }; +} diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp @@ -0,0 +1,205 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "Microsoft/Schema/1_2/Interface.h" + +#include "Microsoft/Schema/1_2/NormalizedPackageNameTable.h" +#include "Microsoft/Schema/1_2/NormalizedPackagePublisherTable.h" + +#include "Microsoft/Schema/1_2/SearchResultsTable.h" + + +namespace AppInstaller::Repository::Microsoft::Schema::V1_2 +{ + namespace + { + void AddLocalizationNormalizedName(const Utility::NameNormalizer& normalizer, const Manifest::ManifestLocalization& localization, std::vector<Utility::NormalizedString>& out) + { + if (localization.Contains(Manifest::Localization::PackageName)) + { + out.emplace_back(normalizer.NormalizeName(Utility::FoldCase(localization.Get<Manifest::Localization::PackageName>())).Name()); + } + } + + void AddLocalizationNormalizedPublisher(const Utility::NameNormalizer& normalizer, const Manifest::ManifestLocalization& localization, std::vector<Utility::NormalizedString>& out) + { + if (localization.Contains(Manifest::Localization::Publisher)) + { + out.emplace_back(normalizer.NormalizePublisher(Utility::FoldCase(localization.Get<Manifest::Localization::Publisher>()))); + } + } + + std::vector<Utility::NormalizedString> GetNormalizedNames(const Utility::NameNormalizer& normalizer, const Manifest::Manifest& manifest) + { + std::vector<Utility::NormalizedString> result; + + AddLocalizationNormalizedName(normalizer, manifest.DefaultLocalization, result); + for (const auto& loc : manifest.Localizations) + { + AddLocalizationNormalizedName(normalizer, loc, result); + } + + return result; + } + + std::vector<Utility::NormalizedString> GetNormalizedPublishers(const Utility::NameNormalizer& normalizer, const Manifest::Manifest& manifest) + { + std::vector<Utility::NormalizedString> result; + + AddLocalizationNormalizedPublisher(normalizer, manifest.DefaultLocalization, result); + for (const auto& loc : manifest.Localizations) + { + AddLocalizationNormalizedPublisher(normalizer, loc, result); + } + + return result; + } + } + + Interface::Interface(Utility::NormalizationVersion normVersion) : m_normalizer(normVersion) + { + } + + Schema::Version Interface::GetVersion() const + { + return { 1, 2 }; + } + + void Interface::CreateTables(SQLite::Connection& connection) + { + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createtables_v1_2"); + + V1_1::Interface::CreateTables(connection); + + // While the name and publisher should be linked per-locale, we are not implementing that here. + // This will mean that one can match cross locale name and publisher, but the chance that this + // leads to a confusion between packages is very small. More likely would be intentional attempts + // to confuse the correlation, which could be fairly easily carried out even with linked values. + NormalizedPackageNameTable::Create(connection); + NormalizedPackagePublisherTable::Create(connection); + + savepoint.Commit(); + } + + SQLite::rowid_t Interface::AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + { + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "addmanifest_v1_2"); + + SQLite::rowid_t manifestId = V1_1::Interface::AddManifest(connection, manifest, relativePath); + + // Add the new 1.2 data + // These normalized strings are all stored with their cases folded so that they can be + // looked up ordinally; enabling the index to provide efficient searches. + NormalizedPackageNameTable::EnsureExistsAndInsert(connection, GetNormalizedNames(m_normalizer, manifest), manifestId); + NormalizedPackagePublisherTable::EnsureExistsAndInsert(connection, GetNormalizedPublishers(m_normalizer, manifest), manifestId); + + savepoint.Commit(); + + return manifestId; + } + + std::pair<bool, SQLite::rowid_t> Interface::UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + { + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "updatemanifest_v1_2"); + + auto [indexModified, manifestId] = V1_1::Interface::UpdateManifest(connection, manifest, relativePath); + + // Update new 1.2 tables as necessary + indexModified = NormalizedPackageNameTable::UpdateIfNeededByManifestId(connection, GetNormalizedNames(m_normalizer, manifest), manifestId) || indexModified; + indexModified = NormalizedPackagePublisherTable::UpdateIfNeededByManifestId(connection, GetNormalizedPublishers(m_normalizer, manifest), manifestId) || indexModified; + + savepoint.Commit(); + + return { indexModified, manifestId }; + } + + SQLite::rowid_t Interface::RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + { + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removemanifest_v1_2"); + + SQLite::rowid_t manifestId = V1_1::Interface::RemoveManifest(connection, manifest, relativePath); + + // Remove all of the new 1.2 data that is no longer referenced. + NormalizedPackageNameTable::DeleteIfNotNeededByManifestId(connection, manifestId); + NormalizedPackagePublisherTable::DeleteIfNotNeededByManifestId(connection, manifestId); + + savepoint.Commit(); + + return manifestId; + } + + bool Interface::CheckConsistency(const SQLite::Connection& connection, bool log) const + { + bool result = V1_1::Interface::CheckConsistency(connection, log); + + // If the v1.1 index was consistent, or if full logging of inconsistency was requested, check the v1.2 data. + if (result || log) + { + result = NormalizedPackageNameTable::CheckConsistency(connection, log) && result; + } + + if (result || log) + { + result = NormalizedPackagePublisherTable::CheckConsistency(connection, log) && result; + } + + return result; + } + + Utility::NormalizedName Interface::NormalizeName(std::string_view name, std::string_view publisher) const + { + return m_normalizer.Normalize(name, publisher); + } + + std::unique_ptr<V1_0::SearchResultsTable> Interface::CreateSearchResultsTable(const SQLite::Connection& connection) const + { + return std::make_unique<SearchResultsTable>(connection); + } + + ISQLiteIndex::SearchResult Interface::SearchInternal(const SQLite::Connection& connection, SearchRequest& request) const + { + // Update NormalizedNameAndPublisher with normalization and folding + auto updateIfNeeded = [&](PackageMatchFilter& filter) + { + if (filter.Field == PackageMatchField::NormalizedNameAndPublisher && filter.Type == MatchType::Exact) + { + Utility::NormalizedName normalized = m_normalizer.Normalize(Utility::FoldCase(filter.Value), Utility::FoldCase(filter.Additional.value())); + filter.Value = normalized.Name(); + filter.Additional = normalized.Publisher(); + } + }; + + for (auto& inclusion : request.Inclusions) + { + updateIfNeeded(inclusion); + } + + for (auto& filter : request.Filters) + { + updateIfNeeded(filter); + } + + return V1_1::Interface::SearchInternal(connection, request); + } + + void Interface::PrepareForPackaging(SQLite::Connection& connection, bool vacuum) + { + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "prepareforpackaging_v1_2"); + + V1_1::Interface::PrepareForPackaging(connection, false); + + NormalizedPackageNameTable::PrepareForPackaging(connection, true, true); + NormalizedPackagePublisherTable::PrepareForPackaging(connection, true, true); + + savepoint.Commit(); + + if (vacuum) + { + // Force the database to actually shrink the file size. + // This *must* be done outside of an active transaction. + SQLite::Builder::StatementBuilder builder; + builder.Vacuum(); + builder.Execute(connection); + } + } +} diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/NormalizedPackageNameTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/NormalizedPackageNameTable.h @@ -0,0 +1,22 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "Microsoft/Schema/1_0/OneToManyTable.h" + + +namespace AppInstaller::Repository::Microsoft::Schema::V1_2 +{ + namespace details + { + using namespace std::string_view_literals; + + struct NormalizedPackageNameTableInfo + { + inline static constexpr std::string_view TableName() { return "norm_names"sv; } + inline static constexpr std::string_view ValueName() { return "norm_name"sv; } + }; + } + + // The table for NormalizedPackageName. + using NormalizedPackageNameTable = V1_0::OneToManyTable<details::NormalizedPackageNameTableInfo>; +} diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/NormalizedPackagePublisherTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/NormalizedPackagePublisherTable.h @@ -0,0 +1,22 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "Microsoft/Schema/1_0/OneToManyTable.h" + + +namespace AppInstaller::Repository::Microsoft::Schema::V1_2 +{ + namespace details + { + using namespace std::string_view_literals; + + struct NormalizedPackagePublisherTableInfo + { + inline static constexpr std::string_view TableName() { return "norm_publishers"sv; } + inline static constexpr std::string_view ValueName() { return "norm_publisher"sv; } + }; + } + + // The table for NormalizedPackagePublisher. + using NormalizedPackagePublisherTable = V1_0::OneToManyTable<details::NormalizedPackagePublisherTableInfo>; +} diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/SearchResultsTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/SearchResultsTable.h @@ -0,0 +1,33 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "Microsoft/Schema/1_1/SearchResultsTable.h" + + +namespace AppInstaller::Repository::Microsoft::Schema::V1_2 +{ + // Table for holding temporary search results. + struct SearchResultsTable : public V1_1::SearchResultsTable + { + SearchResultsTable(const SQLite::Connection& connection) : V1_1::SearchResultsTable(connection) {} + + SearchResultsTable(const SearchResultsTable&) = delete; + SearchResultsTable& operator=(const SearchResultsTable&) = delete; + + SearchResultsTable(SearchResultsTable&&) = default; + SearchResultsTable& operator=(SearchResultsTable&&) = default; + + protected: + std::vector<int> BuildSearchStatement( + SQLite::Builder::StatementBuilder& builder, + PackageMatchField field, + std::string_view manifestAlias, + std::string_view valueAlias, + bool useLike) const override; + + // Import all overrides of this function + using V1_0::SearchResultsTable::BindStatementForMatchType; + + void BindStatementForMatchType(SQLite::Statement& statement, const PackageMatchFilter& filter, const std::vector<int>& bindIndex) override; + }; +} diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/SearchResultsTable_1_2.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/SearchResultsTable_1_2.cpp @@ -0,0 +1,39 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "pch.h" +#include "SearchResultsTable.h" + +#include "Microsoft/Schema/1_0/ManifestTable.h" +#include "Microsoft/Schema/1_2/NormalizedPackageNameTable.h" +#include "Microsoft/Schema/1_2/NormalizedPackagePublisherTable.h" + + +namespace AppInstaller::Repository::Microsoft::Schema::V1_2 +{ + std::vector<int> SearchResultsTable::BuildSearchStatement( + SQLite::Builder::StatementBuilder& builder, + PackageMatchField field, + std::string_view manifestAlias, + std::string_view valueAlias, + bool useLike) const + { + switch (field) + { + case PackageMatchField::NormalizedNameAndPublisher: + return V1_0::ManifestTable::BuildSearchStatement<NormalizedPackageNameTable, NormalizedPackagePublisherTable>(builder, manifestAlias, valueAlias, useLike); + default: + return V1_1::SearchResultsTable::BuildSearchStatement(builder, field, manifestAlias, valueAlias, useLike); + } + } + + void SearchResultsTable::BindStatementForMatchType(SQLite::Statement& statement, const PackageMatchFilter& filter, const std::vector<int>& bindIndex) + { + V1_0::SearchResultsTable::BindStatementForMatchType(statement, filter, bindIndex); + + if (filter.Field == PackageMatchField::NormalizedNameAndPublisher) + { + BindStatementForMatchType(statement, filter.Type, bindIndex[1], filter.Additional.value()); + } + } +} diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h @@ -6,6 +6,7 @@ #include "Public/AppInstallerRepositorySearch.h" #include <AppInstallerVersions.h> #include <winget/Manifest.h> +#include <winget/NameNormalization.h> #include <filesystem> @@ -80,5 +81,9 @@ namespace AppInstaller::Repository::Microsoft::Schema // Sets the string for the given metadata and manifest id. virtual void SetMetadataByManifestId(SQLite::Connection& connection, SQLite::rowid_t manifestId, PackageVersionMetadata metadata, std::string_view value) = 0; + + // Normalizes a name using the internal rules used by the index. + // Largely a utility function; should not be used to do work on behalf of the index by the caller. + virtual Utility::NormalizedName NormalizeName(std::string_view name, std::string_view publisher) const = 0; }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Version.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Version.cpp @@ -6,6 +6,7 @@ #include "1_0/Interface.h" #include "1_1/Interface.h" +#include "1_2/Interface.h" namespace AppInstaller::Repository::Microsoft::Schema { @@ -34,11 +35,15 @@ namespace AppInstaller::Repository::Microsoft::Schema { return std::make_unique<V1_0::Interface>(); } - else if (*this == Version{ 1, 1 } || + else if (*this == Version{ 1, 1 }) + { + return std::make_unique<V1_1::Interface>(); + } + else if (*this == Version{ 1, 2 } || this->MajorVersion == 1 || this->IsLatest()) { - return std::make_unique<V1_1::Interface>(); + return std::make_unique<V1_2::Interface>(); } // We do not have the capacity to operate on this schema version diff --git a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h @@ -42,6 +42,7 @@ namespace AppInstaller::Repository Tag, PackageFamilyName, ProductCode, + NormalizedNameAndPublisher, }; // A single match to be performed during a search. @@ -49,8 +50,13 @@ namespace AppInstaller::Repository { MatchType Type; Utility::NormalizedString Value; + std::optional<Utility::NormalizedString> Additional; - RequestMatch(MatchType t, std::string_view v) : Type(t), Value(v) {} + RequestMatch(MatchType t) : Type(t) {} + RequestMatch(MatchType t, Utility::NormalizedString& v) : Type(t), Value(v) {} + RequestMatch(MatchType t, const Utility::NormalizedString& v) : Type(t), Value(v) {} + RequestMatch(MatchType t, Utility::NormalizedString&& v) : Type(t), Value(std::move(v)) {} + RequestMatch(MatchType t, std::string_view v1, std::string_view v2) : Type(t), Value(v1), Additional(Utility::NormalizedString{ v2 }) {} }; // A match on a specific field to be performed during a search. @@ -58,7 +64,21 @@ namespace AppInstaller::Repository { PackageMatchField Field; - PackageMatchFilter(PackageMatchField f, MatchType t, std::string_view v) : RequestMatch(t, v), Field(f) {} + PackageMatchFilter(PackageMatchField f, MatchType t) : RequestMatch(t), Field(f) { EnsureRequiredValues(); } + PackageMatchFilter(PackageMatchField f, MatchType t, Utility::NormalizedString& v) : RequestMatch(t, v), Field(f) { EnsureRequiredValues(); } + PackageMatchFilter(PackageMatchField f, MatchType t, const Utility::NormalizedString& v) : RequestMatch(t, v), Field(f) { EnsureRequiredValues(); } + PackageMatchFilter(PackageMatchField f, MatchType t, Utility::NormalizedString&& v) : RequestMatch(t, std::move(v)), Field(f) { EnsureRequiredValues(); } + PackageMatchFilter(PackageMatchField f, MatchType t, std::string_view v1, std::string_view v2) : RequestMatch(t, v1, v2), Field(f) { EnsureRequiredValues(); } + + protected: + void EnsureRequiredValues() + { + // Ensure that the second value always exists when it should + if (Field == PackageMatchField::NormalizedNameAndPublisher && !Additional) + { + Additional = Utility::NormalizedString{}; + } + } }; // Container for data used to filter the available manifests in a source. @@ -122,6 +142,10 @@ namespace AppInstaller::Repository StandardUninstallCommand, // An uninstall command that should be non-interactive SilentUninstallCommand, + // The publisher of the package + Publisher, + // The locale of the package + Locale, }; // Convert a PackageVersionMetadata to a string. @@ -207,6 +231,9 @@ namespace AppInstaller::Repository // Gets a value indicating whether an available version is newer than the installed version. virtual bool IsUpdateAvailable() const = 0; + + // Determines if the given IPackage refers to the same package as this one. + virtual bool IsSame(const IPackage*) const = 0; }; // A single result from the search. @@ -276,6 +303,8 @@ namespace AppInstaller::Repository return "PackageFamilyName"sv; case PackageMatchField::ProductCode: return "ProductCode"sv; + case PackageMatchField::NormalizedNameAndPublisher: + return "NormalizedNameAndPublisher"sv; } return "UnknownMatchField"sv; diff --git a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h @@ -118,8 +118,7 @@ namespace AppInstaller::Repository enum class PredefinedSource { Installed, - ARP_System, - ARP_User, + ARP, MSIX, }; diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -715,13 +715,9 @@ namespace AppInstaller::Repository details.Type = Microsoft::PredefinedInstalledSourceFactory::Type(); details.Arg = Microsoft::PredefinedInstalledSourceFactory::FilterToString(Microsoft::PredefinedInstalledSourceFactory::Filter::None); return CreateSourceFromDetails(details, progress); - case PredefinedSource::ARP_System: + case PredefinedSource::ARP: details.Type = Microsoft::PredefinedInstalledSourceFactory::Type(); - details.Arg = Microsoft::PredefinedInstalledSourceFactory::FilterToString(Microsoft::PredefinedInstalledSourceFactory::Filter::ARP_System); - return CreateSourceFromDetails(details, progress); - case PredefinedSource::ARP_User: - details.Type = Microsoft::PredefinedInstalledSourceFactory::Type(); - details.Arg = Microsoft::PredefinedInstalledSourceFactory::FilterToString(Microsoft::PredefinedInstalledSourceFactory::Filter::ARP_User); + details.Arg = Microsoft::PredefinedInstalledSourceFactory::FilterToString(Microsoft::PredefinedInstalledSourceFactory::Filter::ARP); return CreateSourceFromDetails(details, progress); case PredefinedSource::MSIX: details.Type = Microsoft::PredefinedInstalledSourceFactory::Type(); diff --git a/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.cpp b/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.cpp @@ -323,6 +323,17 @@ namespace AppInstaller::Repository::SQLite::Builder return *this; } + StatementBuilder& StatementBuilder::LiteralColumn(std::string_view value) + { + if (m_needsComma) + { + m_stream << ", "; + } + AddBindFunctor(AppendOpAndBinder(Op::Literal), value); + m_needsComma = true; + return *this; + } + StatementBuilder& StatementBuilder::Escape(std::string_view escapeChar) { THROW_HR_IF(E_INVALIDARG, escapeChar.length() != 1); @@ -773,6 +784,9 @@ namespace AppInstaller::Repository::SQLite::Builder case Op::Escape: m_stream << " ESCAPE ?"; break; + case Op::Literal: + m_stream << " ?"; + break; default: THROW_HR(E_UNEXPECTED); } diff --git a/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.h b/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.h @@ -239,6 +239,8 @@ namespace AppInstaller::Repository::SQLite::Builder StatementBuilder& LikeWithEscape(std::string_view value); StatementBuilder& Like(details::unbound_t); + StatementBuilder& LiteralColumn(std::string_view value); + StatementBuilder& Escape(std::string_view escapeChar); StatementBuilder& Not(); @@ -394,7 +396,8 @@ namespace AppInstaller::Repository::SQLite::Builder { Equals, Like, - Escape + Escape, + Literal, }; // Appends given the operation. diff --git a/src/AppInstallerTestExeInstaller/main.cpp b/src/AppInstallerTestExeInstaller/main.cpp @@ -20,7 +20,7 @@ path GenerateUninstaller(std::wostream& out, const path& installDirectory, const path uninstallerPath = installDirectory; uninstallerPath /= "UninstallTestExe.bat"; - out << "Uninstaller located at path: " << uninstallerPath << '\n'; + out << "Uninstaller located at path: " << uninstallerPath << std::endl; path uninstallerOutputTextFilePath = installDirectory; uninstallerOutputTextFilePath /= "TestExeUninstalled.txt"; @@ -54,7 +54,7 @@ void WriteToUninstallRegistry(std::wostream& out, const std::wstring& productID, // String inputs to registry must be of wide char type const wchar_t* displayName = L"AppInstallerTestExeInstaller"; const wchar_t* publisher = L"Microsoft Corporation"; - const wchar_t* uninstallString = uninstallerPath.c_str(); + std::wstring uninstallString = uninstallerPath.wstring(); DWORD version = 1; std::wstring registryKey{ registrySubkey }; @@ -62,12 +62,12 @@ void WriteToUninstallRegistry(std::wostream& out, const std::wstring& productID, if (!productID.empty()) { registryKey += productID; - out << "Product Code overridden to: " << registryKey << "\n"; + out << "Product Code overridden to: " << registryKey << std::endl; } else { registryKey += defaultProductID; - out << "Default Product Code used: " << registryKey << "\n"; + out << "Default Product Code used: " << registryKey << std::endl; } lReg = RegCreateKeyEx( @@ -83,42 +83,42 @@ void WriteToUninstallRegistry(std::wostream& out, const std::wstring& productID, if (lReg == ERROR_SUCCESS) { - out << "Successfully opened registry key \n"; + out << "Successfully opened registry key" << std::endl; // Set Display Name Property Value if (LONG res = RegSetValueEx(hkey, L"DisplayName", NULL, REG_SZ, (LPBYTE)displayName, (DWORD)(wcslen(displayName) + 1) * sizeof(wchar_t)) != ERROR_SUCCESS) { - out << "Failed to write DisplayName value. Error Code: " << res << "\n"; + out << "Failed to write DisplayName value. Error Code: " << res << std::endl; } // Set Display Version Property Value if (LONG res = RegSetValueEx(hkey, L"DisplayVersion", NULL, REG_SZ, (LPBYTE)displayVersion.c_str(), (DWORD)(displayVersion.length() + 1) * sizeof(wchar_t)) != ERROR_SUCCESS) { - out << "Failed to write DisplayVersion value. Error Code: " << res << "\n"; + out << "Failed to write DisplayVersion value. Error Code: " << res << std::endl; } // Set Publisher Property Value if (LONG res = RegSetValueEx(hkey, L"Publisher", NULL, REG_SZ, (LPBYTE)publisher, (DWORD)(wcslen(publisher) + 1) * sizeof(wchar_t)) != ERROR_SUCCESS) { - out << "Failed to write Publisher value. Error Code: " << res << "\n"; + out << "Failed to write Publisher value. Error Code: " << res << std::endl; } // Set UninstallString Property Value - if (LONG res = RegSetValueEx(hkey, L"UninstallString", NULL, REG_EXPAND_SZ, (LPBYTE)uninstallString, (DWORD)wcslen(uninstallString + 1) * sizeof(wchar_t*)) != ERROR_SUCCESS) + if (LONG res = RegSetValueEx(hkey, L"UninstallString", NULL, REG_EXPAND_SZ, (LPBYTE)uninstallString.c_str(), (DWORD)(uninstallString.length() + 1) * sizeof(wchar_t)) != ERROR_SUCCESS) { - out << "Failed to write UninstallString value. Error Code: " << res << "\n"; + out << "Failed to write UninstallString value. Error Code: " << res << std::endl; } // Set Version Property Value if (LONG res = RegSetValueEx(hkey, L"Version", NULL, REG_DWORD, (LPBYTE)&version, sizeof(version)) != ERROR_SUCCESS) { - out << "Failed to write Version value. Error Code: " << res << "\n"; + out << "Failed to write Version value. Error Code: " << res << std::endl; } - out << "Write to registry key completed \n"; + out << "Write to registry key completed" << std::endl; } else { - out << "Key Creation Failed\n"; + out << "Key Creation Failed" << std::endl; } RegCloseKey(hkey); @@ -141,29 +141,41 @@ int wmain(int argc, const wchar_t** argv) outContent << argv[i] << ' '; // Supports custom install path. - if (_wcsicmp(argv[i], L"/InstallDir") == 0 && ++i < argc) + if (_wcsicmp(argv[i], L"/InstallDir") == 0) { - installDirectory = argv[i]; - outContent << argv[i] << ' '; + if (++i < argc) + { + installDirectory = argv[i]; + outContent << argv[i] << ' '; + } } // Supports custom product code ID - if (_wcsicmp(argv[i], L"/ProductID") == 0 && ++i < argc) + else if (_wcsicmp(argv[i], L"/ProductID") == 0) { - productCode = argv[i]; + if (++i < argc) + { + productCode = argv[i]; + } } // Supports custom version - if (_wcsicmp(argv[i], L"/Version") == 0 && ++i < argc) + else if (_wcsicmp(argv[i], L"/Version") == 0) { - version = argv[i]; + if (++i < argc) + { + version = argv[i]; + } } // Supports log file - if (_wcsicmp(argv[i], L"/LogFile") == 0 && ++i < argc) + else if (_wcsicmp(argv[i], L"/LogFile") == 0) { - logFile = std::wofstream(argv[i], std::wofstream::out | std::wofstream::trunc); - out = &logFile; + if (++i < argc) + { + logFile = std::wofstream(argv[i], std::wofstream::out | std::wofstream::trunc); + out = &logFile; + } } }