diff --git a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp index 4568b3ecea..1c9f78b4ba 100644 --- a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp +++ b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp @@ -51,7 +51,7 @@ namespace AppInstaller::CLI::Workflow MachineArchitectureComparator(std::vector allowedArchitectures) : details::ComparisonField("Machine Architecture"), m_allowedArchitectures(std::move(allowedArchitectures)) { - AICLI_LOG(CLI, Verbose, << "Architecture Comparator created with allowed architectures: " << GetAllowedArchitecturesString()); + AICLI_LOG(CLI, Verbose, << "Architecture Comparator created with allowed architectures: " << Utility::ConvertContainerToString(m_allowedArchitectures, Utility::ToString)); } // TODO: At some point we can do better about matching the currently installed architecture @@ -169,22 +169,6 @@ namespace AppInstaller::CLI::Workflow return unsupportedItr != installer.UnsupportedOSArchitectures.end(); } - std::string GetAllowedArchitecturesString() - { - std::stringstream result; - bool prependComma = false; - for (Utility::Architecture architecture : m_allowedArchitectures) - { - if (prependComma) - { - result << ", "; - } - result << Utility::ToString(architecture); - prependComma = true; - } - return result.str(); - } - std::vector m_allowedArchitectures; }; @@ -290,7 +274,7 @@ namespace AppInstaller::CLI::Workflow std::string ExplainInapplicable(const Manifest::ManifestInstaller& installer) override { - std::string result = "Installer scope does not matched currently installed scope: "; + std::string result = "Installer scope does not match currently installed scope: "; result += Manifest::ScopeToString(installer.Scope); result += " != "; result += Manifest::ScopeToString(m_requirement); @@ -429,8 +413,8 @@ namespace AppInstaller::CLI::Workflow LocaleComparator(std::vector preference, std::vector requirement) : details::ComparisonField("Locale"), m_preference(std::move(preference)), m_requirement(std::move(requirement)) { - m_requirementAsString = GetLocalesListAsString(m_requirement); - m_preferenceAsString = GetLocalesListAsString(m_preference); + m_requirementAsString = Utility::ConvertContainerToString(m_requirement); + m_preferenceAsString = Utility::ConvertContainerToString(m_preference); AICLI_LOG(CLI, Verbose, << "Locale Comparator created with Required Locales: " << m_requirementAsString << " , Preferred Locales: " << m_preferenceAsString); } @@ -521,30 +505,61 @@ namespace AppInstaller::CLI::Workflow std::vector m_requirement; std::string m_requirementAsString; std::string m_preferenceAsString; + }; - std::string GetLocalesListAsString(const std::vector& locales) + struct MarketFilter : public details::FilterField + { + MarketFilter(Manifest::string_t market) : details::FilterField("Market"), m_market(market) { - std::string result = "["; + AICLI_LOG(CLI, Verbose, << "Market Filter created with market: " << m_market); + } - bool first = true; - for (auto const& locale : locales) + static std::unique_ptr Create() + { + return std::make_unique(Runtime::GetOSRegion()); + } + + InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override + { + // If both allowed and excluded lists are provided, we only need to check the allowed markets. + if (!installer.Markets.AllowedMarkets.empty()) { - if (first) + // Inapplicable if NOT found + if (!IsMarketInList(installer.Markets.AllowedMarkets)) { - first = false; + return InapplicabilityFlags::Market; } - else + } + else if (!installer.Markets.ExcludedMarkets.empty()) + { + // Inapplicable if found + if (IsMarketInList(installer.Markets.ExcludedMarkets)) { - result += ", "; + return InapplicabilityFlags::Market; } - - result += locale; } - result += ']'; + return InapplicabilityFlags::None; + } + std::string ExplainInapplicable(const Manifest::ManifestInstaller& installer) override + { + std::string result = "Current market '" + m_market + "' does not match installer markets." + + " Allowed markets: " + Utility::ConvertContainerToString(installer.Markets.AllowedMarkets) + + " Excluded markets: " + Utility::ConvertContainerToString(installer.Markets.ExcludedMarkets); return result; } + + private: + bool IsMarketInList(const std::vector markets) + { + return markets.end() != std::find_if( + markets.begin(), + markets.end(), + [&](const auto& m) { return Utility::CaseInsensitiveEquals(m, m_market); }); + } + + Manifest::string_t m_market; }; } @@ -552,6 +567,7 @@ namespace AppInstaller::CLI::Workflow { AddFilter(std::make_unique()); AddFilter(InstalledScopeFilter::Create(installationMetadata)); + AddFilter(MarketFilter::Create()); // Filter order is not important, but comparison order determines priority. // TODO: There are improvements to be made here around ordering, especially in the context of implicit vs explicit vs command line preferences. diff --git a/src/AppInstallerCLICore/Workflows/ManifestComparator.h b/src/AppInstallerCLICore/Workflows/ManifestComparator.h index d08d5c988b..ff83105441 100644 --- a/src/AppInstallerCLICore/Workflows/ManifestComparator.h +++ b/src/AppInstallerCLICore/Workflows/ManifestComparator.h @@ -24,6 +24,7 @@ namespace AppInstaller::CLI::Workflow Locale = 0x10, Scope = 0x20, MachineArchitecture = 0x40, + Market = 0x80, }; DEFINE_ENUM_FLAG_OPERATORS(InapplicabilityFlags); diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index 4968c0511b..75b0054fba 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -999,6 +999,7 @@ namespace AppInstaller::CLI::Workflow context.Add({ optionalArchitectures.begin(), optionalArchitectures.end() }); } } + ManifestComparator manifestComparator(context, installationMetadata); auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(context.Get()); diff --git a/src/AppInstallerCLITests/ManifestComparator.cpp b/src/AppInstallerCLITests/ManifestComparator.cpp index b73706ed7d..d928d74f5a 100644 --- a/src/AppInstallerCLITests/ManifestComparator.cpp +++ b/src/AppInstallerCLITests/ManifestComparator.cpp @@ -31,7 +31,8 @@ const ManifestInstaller& AddInstaller( ScopeEnum scope = ScopeEnum::Unknown, std::string minOSVersion = {}, std::string locale = {}, - std::vector unsupportedOSArchitectures = {}) + std::vector unsupportedOSArchitectures = {}, + MarketsInfo markets = {}) { ManifestInstaller toAdd; toAdd.Arch = architecture; @@ -40,12 +41,24 @@ const ManifestInstaller& AddInstaller( toAdd.MinOSVersion = minOSVersion; toAdd.Locale = locale; toAdd.UnsupportedOSArchitectures = unsupportedOSArchitectures; + toAdd.Markets = markets; manifest.Installers.emplace_back(std::move(toAdd)); return manifest.Installers.back(); } +template +void RequireVectorsEqual(const std::vector& actual, const std::vector& expected) +{ + REQUIRE(actual.size() == expected.size()); + + for (std::size_t i = 0; i < actual.size(); ++i) + { + REQUIRE(actual[i] == expected[i]); + } +} + void RequireInstaller(const std::optional& actual, const ManifestInstaller& expected) { REQUIRE(actual); @@ -54,16 +67,14 @@ void RequireInstaller(const std::optional& actual, const Mani REQUIRE(actual->Scope == expected.Scope); REQUIRE(actual->MinOSVersion == expected.MinOSVersion); REQUIRE(actual->Locale == expected.Locale); + + RequireVectorsEqual(actual->Markets.AllowedMarkets, expected.Markets.AllowedMarkets); + RequireVectorsEqual(actual->Markets.ExcludedMarkets, expected.Markets.ExcludedMarkets); } void RequireInapplicabilities(const std::vector& inapplicabilities, const std::vector& expected) { - REQUIRE(inapplicabilities.size() == expected.size()); - - for (std::size_t i = 0; i < inapplicabilities.size(); i++) - { - REQUIRE(inapplicabilities[i] == expected[i]); - } + RequireVectorsEqual(inapplicabilities, expected); } TEST_CASE("ManifestComparator_OSFilter_Low", "[manifest_comparator]") @@ -580,3 +591,52 @@ TEST_CASE("ManifestComparator_Inapplicabilities", "[manifest_comparator]") inapplicabilities, { InapplicabilityFlags::OSVersion | InapplicabilityFlags::InstalledType | InapplicabilityFlags::Locale | InapplicabilityFlags::Scope | InapplicabilityFlags::MachineArchitecture }); } + +TEST_CASE("ManifestComparator_MarketFilter", "[manifest_comparator]") +{ + Manifest manifest; + + // Get current market. + winrt::Windows::Globalization::GeographicRegion region; + Manifest::string_t currentMarket{ region.CodeTwoLetter() }; + + SECTION("Applicable") + { + MarketsInfo markets; + SECTION("Allowed") + { + markets.AllowedMarkets = { currentMarket }; + } + SECTION("Not excluded") + { + markets.ExcludedMarkets = { "XX" }; + } + + ManifestInstaller installer = AddInstaller(manifest, Architecture::X86, InstallerTypeEnum::Exe, {}, {}, {}, {}, markets); + ManifestComparator mc(ManifestComparatorTestContext{}, {}); + auto [result, inapplicabilities] = mc.GetPreferredInstaller(manifest); + + RequireInstaller(result, installer); + REQUIRE(inapplicabilities.empty()); + } + + SECTION("Inapplicable") + { + MarketsInfo markets; + SECTION("Excluded") + { + markets.ExcludedMarkets = { currentMarket }; + } + SECTION("Not allowed") + { + markets.AllowedMarkets = { "XX" }; + } + + ManifestInstaller installer = AddInstaller(manifest, Architecture::X86, InstallerTypeEnum::Exe, {}, {}, {}, {}, markets); + ManifestComparator mc(ManifestComparatorTestContext{}, {}); + auto [result, inapplicabilities] = mc.GetPreferredInstaller(manifest); + + REQUIRE(!result); + RequireInapplicabilities(inapplicabilities, { InapplicabilityFlags::Market}); + } +} diff --git a/src/AppInstallerCLITests/pch.h b/src/AppInstallerCLITests/pch.h index a00a6a8312..53306b7156 100644 --- a/src/AppInstallerCLITests/pch.h +++ b/src/AppInstallerCLITests/pch.h @@ -13,6 +13,7 @@ #include #include +#include #include #include diff --git a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h index 3ec9abb023..fdf9ec48bb 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h @@ -24,6 +24,7 @@ namespace AppInstaller::Runtime Utility::LocIndString GetOSVersion(); // Gets the OS region. + // This can be used as the current market. std::string GetOSRegion(); // A path to be retrieved based on the runtime. diff --git a/src/AppInstallerCommonCore/Public/AppInstallerStrings.h b/src/AppInstallerCommonCore/Public/AppInstallerStrings.h index 3890b68966..8806a92280 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerStrings.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerStrings.h @@ -145,4 +145,36 @@ namespace AppInstaller::Utility // Gets the file name part of the given URI. std::filesystem::path GetFileNameFromURI(std::string_view uri); + + // Converts a container to a string representation of it. + template + std::string ConvertContainerToString(const T& container, U toString) + { + std::ostringstream strstr; + strstr << '['; + + bool firstItem = true; + for (const auto& item : container) + { + if (firstItem) + { + firstItem = false; + } + else + { + strstr << ", "; + } + + strstr << toString(item); + } + + strstr << ']'; + return strstr.str(); // We need C++20 to get std::move(strstr).str() to extract the string from inside the stream + } + + template + std::string ConvertContainerToString(const T& container) + { + return ConvertContainerToString(container, [](const auto& item) { return item; }); + } }