commit 53b9ad881a438d6d1c5aa741b98e1db2b608d56c parent fb5446bd7a3c35145de84afe45ffdfc78008c929 Author: JohnMcPMS <johnmcp@microsoft.com> Date: Tue, 11 May 2021 21:58:47 -0700 Name an Publisher correlation in CompositeSource (#955) This change leverages the normalized name and publisher to increase the number of packages that can be matched. It also reworks the way the the correlation occurs internally, albeit only slightly. All operations that rely on this data should now work better; this includes `import`, `list`, `upgrade`, and `uninstall`. Diffstat:
13 files changed, 507 insertions(+), 330 deletions(-)
diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp @@ -63,6 +63,7 @@ struct CompositeTestSetup struct Criteria : public PackageMatchFilter { Criteria() : PackageMatchFilter(PackageMatchField::Id, MatchType::Wildcard, ""sv) {} + Criteria(PackageMatchField field) : PackageMatchFilter(field, MatchType::Wildcard, ""sv) {} }; Manifest::Manifest MakeDefaultManifest() @@ -78,28 +79,87 @@ Manifest::Manifest MakeDefaultManifest() return result; } -std::shared_ptr<TestPackage> MakeInstalled(std::function<void(Manifest::Manifest&)> op) +struct TestPackageHelper { - Manifest::Manifest manifest = MakeDefaultManifest(); - op(manifest); - return TestPackage::Make(manifest, TestPackage::MetadataMap{}); -} + TestPackageHelper(bool isInstalled) : m_isInstalled(isInstalled), m_manifest(MakeDefaultManifest()) {} + + TestPackageHelper& WithId(const std::string& id) + { + m_manifest.Id = id; + return *this; + } + + TestPackageHelper& WithChannel(const std::string& channel) + { + m_manifest.Channel = channel; + return *this; + } + + TestPackageHelper& WithDefaultName(const std::string& name) + { + m_manifest.DefaultLocalization.Add<Manifest::Localization::PackageName>(name); + return *this; + } + + TestPackageHelper& WithPFN(const std::string& pfn) + { + m_manifest.Installers[0].PackageFamilyName = pfn; + return *this; + } -std::shared_ptr<TestPackage> MakeAvailable(std::function<void(Manifest::Manifest&)> op) + TestPackageHelper& WithPC(const std::string& pc) + { + m_manifest.Installers[0].ProductCode = pc; + return *this; + } + + operator std::shared_ptr<IPackage>() + { + if (!m_package) + { + if (m_isInstalled) + { + m_package = TestPackage::Make(m_manifest, TestPackage::MetadataMap{}); + } + else + { + m_package = TestPackage::Make(std::vector<Manifest::Manifest>{ m_manifest }); + } + } + + return m_package; + } + +private: + bool m_isInstalled; + Manifest::Manifest m_manifest; + std::shared_ptr<TestPackage> m_package; +}; + +TestPackageHelper MakeInstalled() { - Manifest::Manifest manifest = MakeDefaultManifest(); - op(manifest); - return TestPackage::Make(std::vector<Manifest::Manifest>{ manifest }); + return { true }; } -std::function<void(Manifest::Manifest&)> WithPFN(const std::string& pfn) +TestPackageHelper MakeAvailable() { - return [pfn](Manifest::Manifest& m) { m.Installers[0].PackageFamilyName = pfn; }; + return { false }; } -std::function<void(Manifest::Manifest&)> WithPC(const std::string& pc) +void RequireIncludes(const std::vector<PackageMatchFilter>& filters, PackageMatchField field, MatchType type, std::optional<std::string> value = {}) { - return [pc](Manifest::Manifest& m) { m.Installers[0].ProductCode = pc; }; + bool found = false; + + for (const PackageMatchFilter& filter : filters) + { + if (filter.Field == field && filter.Type == type && + (!value || filter.Value == value.value())) + { + found = true; + } + } + + REQUIRE(found); } TEST_CASE("CompositeSource_PackageFamilyName_NotAvailable", "[CompositeSource]") @@ -108,7 +168,7 @@ TEST_CASE("CompositeSource_PackageFamilyName_NotAvailable", "[CompositeSource]") std::string pfn = "sortof_apfn"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPFN(pfn)), Criteria()); + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); SearchResult result = setup.Search(); @@ -122,14 +182,13 @@ TEST_CASE("CompositeSource_PackageFamilyName_Available", "[CompositeSource]") std::string pfn = "sortof_apfn"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPFN(pfn)), Criteria()); + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pfn); + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), Criteria()); + result.Matches.emplace_back(MakeAvailable().WithPFN(pfn), Criteria()); return result; }; @@ -145,7 +204,7 @@ TEST_CASE("CompositeSource_ProductCode_NotAvailable", "[CompositeSource]") std::string pc = "thiscouldbeapc"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPC(pc)), Criteria()); + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPC(pc), Criteria()); SearchResult result = setup.Search(); @@ -159,14 +218,13 @@ TEST_CASE("CompositeSource_ProductCode_Available", "[CompositeSource]") std::string pc = "thiscouldbeapc"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPC(pc)), Criteria()); + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPC(pc), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pc); + RequireIncludes(request.Inclusions, PackageMatchField::ProductCode, MatchType::Exact, pc); SearchResult result; - result.Matches.emplace_back(MakeAvailable(WithPC(pc)), Criteria()); + result.Matches.emplace_back(MakeAvailable().WithPC(pc), Criteria()); return result; }; @@ -177,17 +235,37 @@ TEST_CASE("CompositeSource_ProductCode_Available", "[CompositeSource]") REQUIRE(result.Matches[0].Package->GetAvailableVersionKeys().size() == 1); } -TEST_CASE("CompositeSource_MultiMatch_FindsId", "[CompositeSource]") +TEST_CASE("CompositeSource_NameAndPublisher_Match", "[CompositeSource]") +{ + CompositeTestSetup setup; + setup.Installed->Everything.Matches.emplace_back(MakeInstalled(), Criteria()); + setup.Available->SearchFunction = [&](const SearchRequest& request) + { + RequireIncludes(request.Inclusions, PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact); + + SearchResult result; + result.Matches.emplace_back(MakeAvailable(), Criteria()); + return result; + }; + + SearchResult result = setup.Search(); + + REQUIRE(result.Matches.size() == 1); + REQUIRE(result.Matches[0].Package->GetInstalledVersion()); + REQUIRE(result.Matches[0].Package->GetAvailableVersionKeys().size() == 1); +} + +TEST_CASE("CompositeSource_MultiMatch_FindsStrongMatch", "[CompositeSource]") { std::string name = "MatchingName"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPFN("sortof_apfn")), Criteria()); + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN("sortof_apfn"), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) { SearchResult result; - result.Matches.emplace_back(MakeAvailable([](Manifest::Manifest& m) { m.Id = "A different ID"; }), Criteria()); - result.Matches.emplace_back(MakeAvailable([&](Manifest::Manifest& m) { m.DefaultLocalization.Add<Manifest::Localization::PackageName>(name); }), Criteria()); + result.Matches.emplace_back(MakeAvailable().WithId("A different ID"), Criteria(PackageMatchField::NormalizedNameAndPublisher)); + result.Matches.emplace_back(MakeAvailable().WithDefaultName(name), Criteria(PackageMatchField::PackageFamilyName)); return result; }; @@ -200,15 +278,15 @@ TEST_CASE("CompositeSource_MultiMatch_FindsId", "[CompositeSource]") REQUIRE(!Version(result.Matches[0].Package->GetLatestAvailableVersion()->GetProperty(PackageVersionProperty::Version)).IsUnknown()); } -TEST_CASE("CompositeSource_MultiMatch_DoesNotFindId", "[CompositeSource]") +TEST_CASE("CompositeSource_MultiMatch_DoesNotFindStrongMatch", "[CompositeSource]") { CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPFN("sortof_apfn")), Criteria()); + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN("sortof_apfn"), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) { SearchResult result; - result.Matches.emplace_back(MakeAvailable([](Manifest::Manifest& m) { m.Id = "A different ID"; }), Criteria()); - result.Matches.emplace_back(MakeAvailable([&](Manifest::Manifest& m) { m.Id = "Another diff ID"; }), Criteria()); + result.Matches.emplace_back(MakeAvailable().WithId("A different ID"), Criteria(PackageMatchField::NormalizedNameAndPublisher)); + result.Matches.emplace_back(MakeAvailable().WithId("Another diff ID"), Criteria(PackageMatchField::NormalizedNameAndPublisher)); return result; }; @@ -216,34 +294,34 @@ TEST_CASE("CompositeSource_MultiMatch_DoesNotFindId", "[CompositeSource]") REQUIRE(result.Matches.size() == 1); REQUIRE(result.Matches[0].Package->GetInstalledVersion()); - REQUIRE(result.Matches[0].Package->GetAvailableVersionKeys().size() == 1); - REQUIRE(Version(result.Matches[0].Package->GetLatestAvailableVersion()->GetProperty(PackageVersionProperty::Version)).IsUnknown()); + REQUIRE(result.Matches[0].Package->GetAvailableVersionKeys().size() == 0); } TEST_CASE("CompositeSource_FoundByBothRootSearches", "[CompositeSource]") { std::string pfn = "sortof_apfn"; + auto installedPackage = MakeInstalled().WithPFN(pfn); + auto availablePackage = MakeAvailable().WithPFN(pfn); + CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPFN(pfn)), Criteria()); + setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Installed->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pfn); + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), Criteria()); + result.Matches.emplace_back(installedPackage, Criteria()); return result; }; - setup.Available->Everything.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), Criteria()); + setup.Available->Everything.Matches.emplace_back(availablePackage, Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pfn); + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), Criteria()); + result.Matches.emplace_back(availablePackage, Criteria()); return result; }; @@ -261,22 +339,20 @@ TEST_CASE("CompositeSource_OnlyAvailableFoundByRootSearch", "[CompositeSource]") CompositeTestSetup setup; setup.Installed->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pfn); + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeInstalled(WithPFN(pfn)), Criteria()); + result.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); return result; }; - setup.Available->Everything.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), Criteria()); + setup.Available->Everything.Matches.emplace_back(MakeAvailable().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pfn); + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), Criteria()); + result.Matches.emplace_back(MakeAvailable().WithPFN(pfn), Criteria()); return result; }; @@ -292,14 +368,13 @@ TEST_CASE("CompositeSource_FoundByAvailableRootSearch_NotInstalled", "[Composite std::string pfn = "sortof_apfn"; CompositeTestSetup setup; - setup.Available->Everything.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), Criteria()); + setup.Available->Everything.Matches.emplace_back(MakeAvailable().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pfn); + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), Criteria()); + result.Matches.emplace_back(MakeAvailable().WithPFN(pfn), Criteria()); return result; }; @@ -314,16 +389,18 @@ TEST_CASE("CompositeSource_UpdateWithBetterMatchCriteria", "[CompositeSource]") MatchType originalType = MatchType::Wildcard; MatchType type = MatchType::Exact; + auto installedPackage = MakeInstalled().WithPFN(pfn); + auto availablePackage = MakeAvailable().WithPFN(pfn); + CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPFN(pfn)), Criteria()); + setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pfn); + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), Criteria()); + result.Matches.emplace_back(availablePackage, Criteria()); return result; }; @@ -337,15 +414,14 @@ TEST_CASE("CompositeSource_UpdateWithBetterMatchCriteria", "[CompositeSource]") // Now make the source root search find it with a better criteria setup.Installed->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pfn); + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), Criteria()); + result.Matches.emplace_back(installedPackage, Criteria()); return result; }; - setup.Available->Everything.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), PackageMatchFilter(PackageMatchField::Id, type, ""sv)); + setup.Available->Everything.Matches.emplace_back(availablePackage, PackageMatchFilter(PackageMatchField::Id, type, ""sv)); result = setup.Search(); @@ -360,7 +436,7 @@ TEST_CASE("CompositePackage_PropertyFromInstalled", "[CompositeSource]") std::string id = "Special test ID"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled([&](Manifest::Manifest& m) { m.Id = id; }), Criteria()); + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithId(id), Criteria()); SearchResult result = setup.Search(); @@ -374,11 +450,11 @@ TEST_CASE("CompositePackage_PropertyFromAvailable", "[CompositeSource]") std::string pfn = "sortof_apfn"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPFN(pfn)), Criteria()); + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) { SearchResult result; - result.Matches.emplace_back(MakeAvailable([&](Manifest::Manifest& m) { m.Id = id; }), Criteria()); + result.Matches.emplace_back(MakeAvailable().WithId(id), Criteria()); return result; }; @@ -394,7 +470,7 @@ TEST_CASE("CompositePackage_AvailableVersions_ChannelFilteredOut", "[CompositeSo std::string channel = "Channel"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPFN(pfn)), Criteria()); + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) { Manifest::Manifest noChannel = MakeDefaultManifest(); @@ -430,7 +506,7 @@ TEST_CASE("CompositePackage_AvailableVersions_NoChannelFilteredOut", "[Composite std::string channel = "Channel"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled([&](Manifest::Manifest& m) { m.Installers[0].PackageFamilyName = pfn; m.Channel = channel; }), Criteria()); + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn).WithChannel(channel), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) { Manifest::Manifest noChannel = MakeDefaultManifest(); @@ -470,25 +546,23 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_MatchFirst", "[CompositeSour std::shared_ptr<ComponentTestSource> secondAvailable = std::make_shared<ComponentTestSource>(); setup.Composite.AddAvailableSource(secondAvailable); - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPFN(pfn)), Criteria()); + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pfn); + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable([&](Manifest::Manifest& m) { m.DefaultLocalization.Add<Manifest::Localization::PackageName>(firstName); }), Criteria()); + result.Matches.emplace_back(MakeAvailable().WithDefaultName(firstName), Criteria()); return result; }; secondAvailable->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pfn); + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable([&](Manifest::Manifest& m) { m.DefaultLocalization.Add<Manifest::Localization::PackageName>(secondName); }), Criteria()); + result.Matches.emplace_back(MakeAvailable().WithDefaultName(secondName), Criteria()); return result; }; @@ -510,15 +584,14 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_MatchSecond", "[CompositeSou std::shared_ptr<ComponentTestSource> secondAvailable = std::make_shared<ComponentTestSource>(); setup.Composite.AddAvailableSource(secondAvailable); - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(WithPFN(pfn)), Criteria()); + setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); secondAvailable->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pfn); + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable([&](Manifest::Manifest& m) { m.DefaultLocalization.Add<Manifest::Localization::PackageName>(secondName); }), Criteria()); + result.Matches.emplace_back(MakeAvailable().WithDefaultName(secondName), Criteria()); return result; }; @@ -534,22 +607,23 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_ReverseMatchBoth", "[Composi { std::string pfn = "sortof_apfn"; + auto installedPackage = MakeInstalled().WithPFN(pfn); + CompositeTestSetup setup; std::shared_ptr<ComponentTestSource> secondAvailable = std::make_shared<ComponentTestSource>(); setup.Composite.AddAvailableSource(secondAvailable); setup.Installed->SearchFunction = [&](const SearchRequest& request) { - REQUIRE(request.Inclusions.size() == 1); - REQUIRE(request.Inclusions[0].Value == pfn); + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeInstalled(WithPFN(pfn)), Criteria()); + result.Matches.emplace_back(installedPackage, Criteria()); return result; }; - setup.Available->Everything.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), Criteria()); - secondAvailable->Everything.Matches.emplace_back(MakeAvailable(WithPFN(pfn)), Criteria()); + setup.Available->Everything.Matches.emplace_back(MakeAvailable().WithPFN(pfn), Criteria()); + secondAvailable->Everything.Matches.emplace_back(MakeAvailable().WithPFN(pfn), Criteria()); SearchResult result = setup.Search(); @@ -561,8 +635,8 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_ReverseMatchBoth", "[Composi 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()); + 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); diff --git a/src/AppInstallerCLITests/TestSource.cpp b/src/AppInstallerCLITests/TestSource.cpp @@ -9,6 +9,27 @@ using namespace AppInstaller::Repository; namespace TestCommon { + namespace + { + template<AppInstaller::Manifest::Localization Field> + void BuildPackageVersionMultiPropertyWithFallback(std::vector<Utility::LocIndString>& result, const Manifest::Manifest& VersionManifest) + { + result.emplace_back(VersionManifest.DefaultLocalization.Get<Field>()); + for (const auto& loc : VersionManifest.Localizations) + { + auto f = loc.Get<Field>(); + if (f.empty()) + { + result.emplace_back(loc.Get<Field>()); + } + else + { + result.emplace_back(std::move(f)); + } + } + } + } + TestPackageVersion::TestPackageVersion(const Manifest& manifest, MetadataMap installationMetadata, std::weak_ptr<const ISource> source) : VersionManifest(manifest), Metadata(std::move(installationMetadata)), Source(source) {} @@ -52,6 +73,19 @@ namespace TestCommon AddFoldedIfHasValueAndNotPresent(installer.ProductCode, result); } break; + case PackageVersionMultiProperty::Name: + BuildPackageVersionMultiPropertyWithFallback<AppInstaller::Manifest::Localization::PackageName>(result, VersionManifest); + break; + case PackageVersionMultiProperty::Publisher: + BuildPackageVersionMultiPropertyWithFallback<AppInstaller::Manifest::Localization::Publisher>(result, VersionManifest); + break; + case PackageVersionMultiProperty::Locale: + result.emplace_back(VersionManifest.DefaultLocalization.Locale); + for (const auto& loc : VersionManifest.Localizations) + { + result.emplace_back(loc.Locale); + } + break; } return result; diff --git a/src/AppInstallerCommonCore/Public/winget/LocIndependent.h b/src/AppInstallerCommonCore/Public/winget/LocIndependent.h @@ -33,6 +33,8 @@ namespace AppInstaller::Utility LocIndString(LocIndString&&) = default; LocIndString& operator=(LocIndString&&) = default; + bool empty() const { return m_value.empty(); } + const std::string& get() const { return m_value; } operator const std::string& () const { return m_value; } @@ -41,6 +43,7 @@ namespace AppInstaller::Utility const std::string* operator->() const { return &m_value; } bool operator==(std::string_view sv) const { return m_value == sv; } + bool operator!=(const LocIndString& other) const { return m_value != other.m_value; } bool operator<(const LocIndString& other) const { return m_value < other.m_value; } diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -17,6 +17,19 @@ namespace AppInstaller::Repository }; } + // Returns true for fields that provide a strong match; one that is not based on a heuristic. + bool IsStrongMatchField(PackageMatchField field) + { + switch (field) + { + case AppInstaller::Repository::PackageMatchField::PackageFamilyName: + case AppInstaller::Repository::PackageMatchField::ProductCode: + return true; + } + + return false; + } + // A composite package for the CompositeSource. struct CompositePackage : public IPackage { @@ -112,9 +125,9 @@ namespace AppInstaller::Repository if (!otherComposite || static_cast<bool>(m_installedPackage) != static_cast<bool>(otherComposite->m_installedPackage) || - m_installedPackage && !m_installedPackage->IsSame(otherComposite->m_installedPackage.get()) || + (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())) + (m_availablePackage && !m_availablePackage->IsSame(otherComposite->m_availablePackage.get()))) { return false; } @@ -122,106 +135,34 @@ namespace AppInstaller::Repository return true; } - void SetAvailablePackage(std::shared_ptr<IPackage> availablePackage) + const std::shared_ptr<IPackage>& GetInstalledPackage() { - m_availablePackage = std::move(availablePackage); + return m_installedPackage; } - private: - std::shared_ptr<IPackage> m_installedPackage; - Utility::LocIndString m_installedChannel; - std::shared_ptr<IPackage> m_availablePackage; - }; - - // A sentinel package with an unknown version. - struct UnknownAvailablePackage : public IPackage - { - static constexpr std::string_view Version = "Unknown"sv; - - struct UnknownAvailablePackageVersion : public IPackageVersion - { - Utility::LocIndString GetProperty(PackageVersionProperty property) const override - { - switch (property) - { - case AppInstaller::Repository::PackageVersionProperty::Version: - return Utility::LocIndString{ Version }; - default: - return {}; - } - } - - std::vector<Utility::LocIndString> GetMultiProperty(PackageVersionMultiProperty) const override - { - return {}; - }; - - Manifest::Manifest GetManifest() override - { - return {}; - } - - std::shared_ptr<const ISource> GetSource() const override - { - return {}; - } - - IPackageVersion::Metadata GetMetadata() const override - { - return {}; - } - }; - - Utility::LocIndString GetProperty(PackageProperty) const override - { - return {}; - } - - std::shared_ptr<IPackageVersion> GetInstalledVersion() const override - { - return {}; - } - - std::vector<PackageVersionKey> GetAvailableVersionKeys() const override + const std::shared_ptr<IPackage>& GetAvailablePackage() { - return { { {}, Version, {} } }; + return m_availablePackage; } - std::shared_ptr<IPackageVersion> GetLatestAvailableVersion() const override - { - return std::make_shared<UnknownAvailablePackageVersion>(); - } - - std::shared_ptr<IPackageVersion> GetAvailableVersion(const PackageVersionKey&) const override - { - return std::make_shared<UnknownAvailablePackageVersion>(); - } - - bool IsUpdateAvailable() const override + void SetAvailablePackage(std::shared_ptr<IPackage> availablePackage) { - // Lie here so that list and upgrade will carry on to be able to output the diagnostic information. - return true; + m_availablePackage = std::move(availablePackage); } - bool IsSame(const IPackage* other) const override - { - const UnknownAvailablePackage* otherUnknown = dynamic_cast<const UnknownAvailablePackage*>(other); - - if (otherUnknown) - { - return true; - } - - return false; - } + private: + std::shared_ptr<IPackage> m_installedPackage; + Utility::LocIndString m_installedChannel; + std::shared_ptr<IPackage> m_availablePackage; }; // The comparator compares the ResultMatch by MatchType first, then Field in a predefined order. struct ResultMatchComparator { + template <typename U, typename V> bool operator() ( - const ResultMatch& match1, - const ResultMatch& match2) + const U& match1, + const V& match2) { if (match1.MatchCriteria.Type != match2.MatchCriteria.Type) { @@ -237,14 +178,32 @@ namespace AppInstaller::Repository } }; + template <typename T> + void SortResultMatches(std::vector<T>& matches) + { + std::stable_sort(matches.begin(), matches.end(), ResultMatchComparator()); + } + + // A copy of the standard match that holds a CompositePackage instead. + struct CompositeResultMatch + { + std::shared_ptr<CompositePackage> Package; + PackageMatchFilter MatchCriteria; + + CompositeResultMatch(std::shared_ptr<CompositePackage> p, PackageMatchFilter f) : Package(std::move(p)), MatchCriteria(std::move(f)) {} + }; + // Stores data to enable correlation between installed and available packages. - struct CompositeResult : public SearchResult + struct CompositeResult { // A system reference string. struct SystemReferenceString { SystemReferenceString(PackageMatchField field, Utility::LocIndString string) : - Field(field), String(string) {} + Field(field), String1(string) {} + + SystemReferenceString(PackageMatchField field, Utility::LocIndString string1, Utility::LocIndString string2) : + Field(field), String1(string1), String2(string2) {} bool operator<(const SystemReferenceString& other) const { @@ -253,151 +212,169 @@ namespace AppInstaller::Repository return Field < other.Field; } - return String < other.String; + if (String1 != other.String1) + { + return String1 < other.String1; + } + + return String2 < other.String2; } bool operator==(const SystemReferenceString& other) const { - return Field == other.Field && String == other.String; + return Field == other.Field && String1 == other.String1 && String2 == other.String2; } + void AddToFilters(std::vector<PackageMatchFilter>& filters) const + { + switch (Field) + { + case PackageMatchField::NormalizedNameAndPublisher: + filters.emplace_back(PackageMatchFilter(Field, MatchType::Exact, String1.get(), String2.get())); + break; + + default: + filters.emplace_back(PackageMatchFilter(Field, MatchType::Exact, String1.get())); + } + } + + private: PackageMatchField Field; - Utility::LocIndString String; + Utility::LocIndString String1; + Utility::LocIndString String2; }; // Data relevant to correlation for a package. struct PackageData { - std::vector<SystemReferenceString> SystemReferenceStrings; - }; + std::set<SystemReferenceString> SystemReferenceStrings; - // Data relevant to correlation for an installed package. - struct InstalledPackageData : public PackageData - { - size_t MatchIndex = 0; + void AddIfNotPresent(SystemReferenceString&& srs) + { + if (SystemReferenceStrings.find(srs) == SystemReferenceStrings.end()) + { + SystemReferenceStrings.emplace(std::move(srs)); + } + } }; // For a given package version, prepares the results for it. - InstalledPackageData ReserveInstalledPackageSlot(IPackageVersion* installedVersion) + PackageData GetSystemReferenceStrings(IPackageVersion* version) { - InstalledPackageData result; - result.MatchIndex = Matches.size(); - - HandleSystemReferenceStringTypeForReserveInstalledPackageSlot( - installedVersion, - PackageVersionMultiProperty::PackageFamilyName, - PackageMatchField::PackageFamilyName, - "package family name"sv, - result); - - HandleSystemReferenceStringTypeForReserveInstalledPackageSlot( - installedVersion, - PackageVersionMultiProperty::ProductCode, - PackageMatchField::ProductCode, - "product code"sv, - result); - + PackageData result; + AddSystemReferenceStrings(version, result); return result; } // Check for a package already in the result that should have been correlated already. // If we find one, see if we should upgrade it's match criteria. // If we don't, return package data for further use. - std::optional<PackageData> CheckForExistingResultFromAvailablePackageMatch(const ResultMatch& match) + std::optional<PackageData> CheckForExistingResultFromAvailablePackageMatch(const ResultMatch& availableMatch) { - bool foundExistingPackage = false; + for (auto& match : Matches) + { + const std::shared_ptr<IPackage>& availablePackage = match.Package->GetAvailablePackage(); + if (availablePackage && availablePackage->IsSame(availableMatch.Package.get())) + { + if (ResultMatchComparator{}(availableMatch, match)) + { + match.MatchCriteria = availableMatch.MatchCriteria; + } + + return {}; + } + } + PackageData result; + for (auto const& versionKey : availableMatch.Package->GetAvailableVersionKeys()) + { + auto packageVersion = availableMatch.Package->GetAvailableVersion(versionKey); + AddSystemReferenceStrings(packageVersion.get(), result); + } + return result; + } - for (auto const& versionKey : match.Package->GetAvailableVersionKeys()) + // Determines if the results contain the given installed package. + bool ContainsInstalledPackage(const IPackage* installedPackage) + { + for (auto& match : Matches) { - auto packageVersion = match.Package->GetAvailableVersion(versionKey); - - foundExistingPackage = HandleSystemReferenceStringTypeForCheckForExistingResultFromAvailablePackageMatch( - match, - packageVersion.get(), - PackageVersionMultiProperty::PackageFamilyName, - PackageMatchField::PackageFamilyName, - "package family name"sv, - result); - - foundExistingPackage = HandleSystemReferenceStringTypeForCheckForExistingResultFromAvailablePackageMatch( - match, - packageVersion.get(), - PackageVersionMultiProperty::ProductCode, - PackageMatchField::ProductCode, - "product code"sv, - result) || foundExistingPackage; - - if (foundExistingPackage) + const std::shared_ptr<IPackage>& matchPackage = match.Package->GetInstalledPackage(); + if (matchPackage && matchPackage->IsSame(installedPackage)) { - return {}; + return true; } } + return false; + } + + // Destructively converts the result to the standard variant. + operator SearchResult() && + { + SearchResult result; + + result.Matches.reserve(Matches.size()); + for (auto& match : Matches) + { + result.Matches.emplace_back(std::move(match.Package), std::move(match.MatchCriteria)); + } + + result.Truncated = Truncated; + return result; } + std::vector<CompositeResultMatch> Matches; + bool Truncated = false; + private: - void HandleSystemReferenceStringTypeForReserveInstalledPackageSlot( + void AddSystemReferenceStrings(IPackageVersion* version, PackageData& data) + { + GetSystemReferenceStrings( + version, + PackageVersionMultiProperty::PackageFamilyName, + PackageMatchField::PackageFamilyName, + data); + + GetSystemReferenceStrings( + version, + PackageVersionMultiProperty::ProductCode, + PackageMatchField::ProductCode, + data); + + GetNameAndPublisher( + version, + data); + } + + void GetSystemReferenceStrings( IPackageVersion* installedVersion, PackageVersionMultiProperty prop, PackageMatchField field, - std::string_view logType, - InstalledPackageData& data) + PackageData& data) { for (auto&& string : installedVersion->GetMultiProperty(prop)) { - SystemReferenceString srs(field, std::move(string)); - - if (m_systemReferenceMap.find(srs) != m_systemReferenceMap.end()) - { - AICLI_LOG(Repo, Warning, << "Multiple installed packages found with " << logType << " [" << srs.String << "], ignoring secondary packages for correlation."); - } - else - { - data.SystemReferenceStrings.emplace_back(srs); - m_systemReferenceMap.emplace(std::move(srs), data.MatchIndex); - } + data.AddIfNotPresent(SystemReferenceString{ field, std::move(string) }); } } - bool HandleSystemReferenceStringTypeForCheckForExistingResultFromAvailablePackageMatch( - const ResultMatch& match, - IPackageVersion* availableVersion, - PackageVersionMultiProperty prop, - PackageMatchField field, - std::string_view logType, + void GetNameAndPublisher( + IPackageVersion* installedVersion, PackageData& data) { - bool foundExistingPackage = false; + auto names = installedVersion->GetMultiProperty(PackageVersionMultiProperty::Name); + auto publishers = installedVersion->GetMultiProperty(PackageVersionMultiProperty::Publisher); - for (auto&& string : availableVersion->GetMultiProperty(prop)) + for (size_t i = 0; i < names.size() && i < publishers.size(); ++i) { - SystemReferenceString srs(field, std::move(string)); - - auto itr = m_systemReferenceMap.find(srs); - if (itr != m_systemReferenceMap.end()) - { - foundExistingPackage = true; - - if (ResultMatchComparator{}(match, Matches[itr->second])) - { - AICLI_LOG(Repo, Verbose, << "Found existing result by " << logType << " [" << srs.String << "], increasing match criteria."); - Matches[itr->second].MatchCriteria = match.MatchCriteria; - } - } - - if (std::find(data.SystemReferenceStrings.begin(), data.SystemReferenceStrings.end(), srs) == data.SystemReferenceStrings.end()) - { - data.SystemReferenceStrings.emplace_back(std::move(srs)); - } + data.AddIfNotPresent(SystemReferenceString{ + PackageMatchField::NormalizedNameAndPublisher, + std::move(names[i]), + std::move(publishers[i]) }); } - - return foundExistingPackage; } - - // Maps for storing quick references to results based on their system reference string. - std::map<SystemReferenceString, size_t> m_systemReferenceMap; }; } @@ -468,13 +445,13 @@ namespace AppInstaller::Repository SearchRequest systemReferenceSearch; auto installedVersion = compositePackage->GetInstalledVersion(); - auto installedPackageData = result.ReserveInstalledPackageSlot(installedVersion.get()); + auto installedPackageData = result.GetSystemReferenceStrings(installedVersion.get()); if (!installedPackageData.SystemReferenceStrings.empty()) { for (const auto& srs : installedPackageData.SystemReferenceStrings) { - systemReferenceSearch.Inclusions.emplace_back(PackageMatchFilter(srs.Field, MatchType::Exact, srs.String.get())); + srs.AddToFilters(systemReferenceSearch.Inclusions); } std::shared_ptr<IPackage> availablePackage; @@ -482,12 +459,6 @@ namespace AppInstaller::Repository // Search sources and add to result for (const auto& source : m_availableSources) { - // See if a previous iteration found a package - if (availablePackage) - { - break; - } - SearchResult availableResult = source->Search(systemReferenceSearch); if (availableResult.Matches.empty()) @@ -498,36 +469,42 @@ namespace AppInstaller::Repository if (availableResult.Matches.size() == 1) { availablePackage = std::move(availableResult.Matches[0].Package); - break; } else // availableResult.Matches.size() > 1 { - auto id = installedVersion->GetProperty(PackageVersionProperty::Id); - AICLI_LOG(Repo, Info, - << "Found multiple matches for installed package [" << id << "] in source [" << source->GetIdentifier() << "] when searching for [" << systemReferenceSearch.ToString() << "]"); + << "Found multiple matches for installed package [" << installedVersion->GetProperty(PackageVersionProperty::Id) << + "] in source [" << source->GetIdentifier() << "] when searching for [" << systemReferenceSearch.ToString() << "]"); // More than one match found for the system reference; run some heuristics to check for a match for (auto&& availableMatch : availableResult.Matches) { - auto matchId = availableMatch.Package->GetLatestAvailableVersion()->GetProperty(PackageVersionProperty::Id); + AICLI_LOG(Repo, Info, << " Checking match with package id: " << + availableMatch.Package->GetLatestAvailableVersion()->GetProperty(PackageVersionProperty::Id)); - AICLI_LOG(Repo, Info, << " Checking system reference match with package id: " << matchId); - - if (Utility::ICUCaseInsensitiveEquals(id, matchId)) + if (IsStrongMatchField(availableMatch.MatchCriteria.Field)) { - availablePackage = std::move(availableMatch.Package); - break; + if (!availablePackage) + { + availablePackage = std::move(availableMatch.Package); + } + else + { + AICLI_LOG(Repo, Info, << " Found multiple packages with strong match fields"); + availablePackage.reset(); + break; + } } } - // We did not find an exact match on Id in the results if (!availablePackage) { - AICLI_LOG(Repo, Warning, << " Appropriate available package could not be determined, setting availability state to unknown"); - availablePackage = std::make_shared<UnknownAvailablePackage>(); + AICLI_LOG(Repo, Warning, << " Appropriate available package could not be determined"); } } + + // We found some matching packages here, don't keep going + break; } compositePackage->SetAvailablePackage(std::move(availablePackage)); @@ -540,46 +517,48 @@ namespace AppInstaller::Repository // Optimization for the "everything installed" case, no need to allow for reverse correlations if (request.IsForEverything()) { - return result; + return std::move(result); } // Search available sources - auto availableResult = SearchAvailable(request); - - for (auto&& match : availableResult.Matches) + for (const auto& source : m_availableSources) { - // Check for a package already in the result that should have been correlated already. - auto packageData = result.CheckForExistingResultFromAvailablePackageMatch(match); + auto availableResult = source->Search(request); - // If no package was found that was already in the results, do a correlation lookup with the installed - // source to create a new composite package entry if we find any packages there. - bool foundInstalledMatch = false; - if (packageData && !packageData->SystemReferenceStrings.empty()) + for (auto&& match : availableResult.Matches) { - // Create a search request to run against the installed source - SearchRequest systemReferenceSearch; - for (const auto& srs : packageData->SystemReferenceStrings) - { - systemReferenceSearch.Inclusions.emplace_back(PackageMatchFilter(srs.Field, MatchType::Exact, srs.String.get())); - } - - SearchResult installedCrossRef = m_installedSource->Search(systemReferenceSearch); + // Check for a package already in the result that should have been correlated already. + auto packageData = result.CheckForExistingResultFromAvailablePackageMatch(match); - for (auto&& crossRef : installedCrossRef.Matches) + // If no package was found that was already in the results, do a correlation lookup with the installed + // source to create a new composite package entry if we find any packages there. + bool foundInstalledMatch = false; + if (packageData && !packageData->SystemReferenceStrings.empty()) { - // Ensure that we don't pick up the same package from two available sources by recording it in the map. - auto installedVersion = crossRef.Package->GetInstalledVersion(); - auto installedPackageData = result.ReserveInstalledPackageSlot(installedVersion.get()); + // Create a search request to run against the installed source + SearchRequest systemReferenceSearch; + for (const auto& srs : packageData->SystemReferenceStrings) + { + srs.AddToFilters(systemReferenceSearch.Inclusions); + } + + SearchResult installedCrossRef = m_installedSource->Search(systemReferenceSearch); - foundInstalledMatch = true; - result.Matches.emplace_back(std::make_shared<CompositePackage>(std::move(crossRef.Package), std::move(match.Package)), match.MatchCriteria); + for (auto&& crossRef : installedCrossRef.Matches) + { + if (!result.ContainsInstalledPackage(crossRef.Package.get())) + { + foundInstalledMatch = true; + result.Matches.emplace_back(std::make_shared<CompositePackage>(std::move(crossRef.Package), std::move(match.Package)), match.MatchCriteria); + } + } } - } - // If there was no correlation for this package, add it without one. - if (m_searchBehavior == CompositeSearchBehavior::AllPackages && !foundInstalledMatch) - { - result.Matches.push_back(std::move(match)); + // If there was no correlation for this package, add it without one. + if (m_searchBehavior == CompositeSearchBehavior::AllPackages && !foundInstalledMatch) + { + result.Matches.emplace_back(std::make_shared<CompositePackage>(std::shared_ptr<IPackage>{}, std::move(match.Package)), match.MatchCriteria); + } } } @@ -591,7 +570,7 @@ namespace AppInstaller::Repository result.Matches.erase(result.Matches.begin() + request.MaximumResults, result.Matches.end()); } - return result; + return std::move(result); } // An available search goes through each source, searching individually and then sorting the full result set. @@ -621,9 +600,4 @@ namespace AppInstaller::Repository return result; } - - void CompositeSource::SortResultMatches(std::vector<ResultMatch>& matches) - { - std::stable_sort(matches.begin(), matches.end(), ResultMatchComparator()); - } } diff --git a/src/AppInstallerRepositoryCore/CompositeSource.h b/src/AppInstallerRepositoryCore/CompositeSource.h @@ -51,9 +51,6 @@ namespace AppInstaller::Repository // Performs a search when no installed source is present. SearchResult SearchAvailable(const SearchRequest& request) const; - // Sorts a vector of results. - static void SortResultMatches(std::vector<ResultMatch>& matches); - std::shared_ptr<ISource> m_installedSource; std::vector<std::shared_ptr<ISource>> m_availableSources; SourceDetails m_details; diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp @@ -179,7 +179,7 @@ namespace AppInstaller::Repository::Microsoft bool IsSame(const PackageBase& other) const { - return m_idId == other.m_idId; + return GetReferenceSource()->IsSame(other.GetReferenceSource().get()) && m_idId == other.m_idId; } protected: @@ -361,4 +361,9 @@ namespace AppInstaller::Repository::Microsoft result.Truncated = indexResults.Truncated; return result; } + + bool SQLiteIndexSource::IsSame(const SQLiteIndexSource* other) const + { + return (other && GetIdentifier() == other->GetIdentifier()); + } } diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.h b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.h @@ -37,6 +37,9 @@ namespace AppInstaller::Repository::Microsoft // Gets the index. const SQLiteIndex& GetIndex() const { return m_index; } + // Determines if the other source refers to the same as this. + bool IsSame(const SQLiteIndexSource* other) const; + private: SourceDetails m_details; Synchronization::CrossProcessReaderWriteLock m_lock; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h @@ -19,6 +19,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_2 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; + std::vector<std::string> GetMultiPropertyByManifestId(const SQLite::Connection& connection, SQLite::rowid_t manifestId, PackageVersionMultiProperty property) const override; // Version 1.2 Utility::NormalizedName NormalizeName(std::string_view name, std::string_view publisher) const override; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp @@ -154,6 +154,20 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_2 return result; } + std::vector<std::string> Interface::GetMultiPropertyByManifestId(const SQLite::Connection& connection, SQLite::rowid_t manifestId, PackageVersionMultiProperty property) const + { + switch (property) + { + // These values are not right, as they are normalized. But they are good enough for now and all we have. + case PackageVersionMultiProperty::Name: + return NormalizedPackageNameTable::GetValuesByManifestId(connection, manifestId); + case PackageVersionMultiProperty::Publisher: + return NormalizedPackagePublisherTable::GetValuesByManifestId(connection, manifestId); + default: + return V1_1::Interface::GetMultiPropertyByManifestId(connection, manifestId, property); + } + } + Utility::NormalizedName Interface::NormalizeName(std::string_view name, std::string_view publisher) const { return m_normalizer.Normalize(name, publisher); diff --git a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h @@ -125,8 +125,18 @@ namespace AppInstaller::Repository // A property of a package version that can have multiple values. enum class PackageVersionMultiProperty { + // The package family names (PFN) associated with the package version PackageFamilyName, + // The product codes associated with the package version. ProductCode, + // TODO: Fully implement these 3; the data is not yet in the index source (name and publisher are hacks and locale is not present) + // The package names for the version; these must match in number and order with both Publisher and Locale. + Name, + // The publisher values for the version; these must match in number and order with both Name and Locale. + Publisher, + // The locale of the matching Name and Publisher values; these must match in number and order with both Name and Publisher. + // May be empty if there is only a single value for Name and Publisher. + Locale, }; // A metadata item of a package version. diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -1123,7 +1123,12 @@ namespace AppInstaller::Repository for (const auto& include : Inclusions) { - result << " Inclusions:" << PackageMatchFieldToString(include.Field) << "='" << include.Value << "'[" << MatchTypeToString(include.Type) << "]"; + result << " Include:" << PackageMatchFieldToString(include.Field) << "='" << include.Value << "'"; + if (include.Additional) + { + result << "+'" << include.Additional.value() << "'"; + } + result << "[" << MatchTypeToString(include.Type) << "]"; } for (const auto& filter : Filters) diff --git a/src/AppInstallerRepositoryCore/Rest/RestSource.cpp b/src/AppInstallerRepositoryCore/Rest/RestSource.cpp @@ -34,7 +34,7 @@ namespace AppInstaller::Repository::Rest { PackageVersion( const std::shared_ptr<const RestSource>& source, IRestClient::PackageInfo packageInfo, IRestClient::VersionInfo versionInfo) - : SourceReference(source), m_packageInfo(packageInfo), m_versionInfo(versionInfo) {} + : SourceReference(source), m_packageInfo(std::move(packageInfo)), m_versionInfo(std::move(versionInfo)) {} // Inherited via IPackageVersion Utility::LocIndString GetProperty(PackageVersionProperty property) const override @@ -75,6 +75,36 @@ namespace AppInstaller::Repository::Rest result.emplace_back(Utility::LocIndString{ productCode }); } break; + case PackageVersionMultiProperty::Name: + if (m_versionInfo.Manifest) + { + BuildPackageVersionMultiPropertyWithFallback<AppInstaller::Manifest::Localization::PackageName>(result); + } + else + { + result.emplace_back(m_packageInfo.PackageName); + } + break; + case PackageVersionMultiProperty::Publisher: + if (m_versionInfo.Manifest) + { + BuildPackageVersionMultiPropertyWithFallback<AppInstaller::Manifest::Localization::Publisher>(result); + } + else + { + result.emplace_back(m_packageInfo.Publisher); + } + break; + case PackageVersionMultiProperty::Locale: + if (m_versionInfo.Manifest) + { + result.emplace_back(m_versionInfo.Manifest->DefaultLocalization.Locale); + for (const auto& loc : m_versionInfo.Manifest->Localizations) + { + result.emplace_back(loc.Locale); + } + } + break; } return result; @@ -114,6 +144,24 @@ namespace AppInstaller::Repository::Rest } private: + template<AppInstaller::Manifest::Localization Field> + void BuildPackageVersionMultiPropertyWithFallback(std::vector<Utility::LocIndString>& result) const + { + result.emplace_back(m_versionInfo.Manifest->DefaultLocalization.Get<Field>()); + for (const auto& loc : m_versionInfo.Manifest->Localizations) + { + auto f = loc.Get<Field>(); + if (f.empty()) + { + result.emplace_back(loc.Get<Field>()); + } + else + { + result.emplace_back(std::move(f)); + } + } + } + IRestClient::PackageInfo m_packageInfo; IRestClient::VersionInfo m_versionInfo; }; @@ -254,7 +302,8 @@ namespace AppInstaller::Repository::Rest if (otherAvailablePackage) { - return Utility::CaseInsensitiveEquals(m_package.PackageInformation.PackageIdentifier, otherAvailablePackage->m_package.PackageInformation.PackageIdentifier); + return GetReferenceSource()->IsSame(otherAvailablePackage->GetReferenceSource().get()) && + Utility::CaseInsensitiveEquals(m_package.PackageInformation.PackageIdentifier, otherAvailablePackage->m_package.PackageInformation.PackageIdentifier); } return false; @@ -301,4 +350,9 @@ namespace AppInstaller::Repository::Rest return searchResult; } + + bool RestSource::IsSame(const RestSource* other) const + { + return (other && GetIdentifier() == other->GetIdentifier()); + } } diff --git a/src/AppInstallerRepositoryCore/Rest/RestSource.h b/src/AppInstallerRepositoryCore/Rest/RestSource.h @@ -33,6 +33,9 @@ namespace AppInstaller::Repository::Rest // Execute a search on the source. SearchResult Search(const SearchRequest& request) const override; + // Determines if the other source refers to the same as this. + bool IsSame(const RestSource* other) const; + private: SourceDetails m_details; RestClient m_restClient;