Make manifest retrieval choice more dynamic

microsoft · Oct 6, 2023 · b304a03 · b304a03
1 parent 21de160
commit b304a03
Show file tree

Hide file tree

Showing 9 changed files with 121 additions and 13 deletions.
diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp
@@ -152,6 +152,12 @@ struct TestPackageHelper
         return *this;
     }
 
+    TestPackageHelper& HideSRS(bool value = true)
+    {
+        m_hideSystemReferenceStrings = value;
+        return *this;
+    }
+
     operator std::shared_ptr<IPackage>()
     {
         if (!m_package)
@@ -162,7 +168,7 @@ struct TestPackageHelper
             }
             else
             {
-                m_package = TestPackage::Make(std::vector<Manifest::Manifest>{ m_manifest }, m_source);
+                m_package = TestPackage::Make(std::vector<Manifest::Manifest>{ m_manifest }, m_source, m_hideSystemReferenceStrings);
             }
         }
 
@@ -179,6 +185,7 @@ struct TestPackageHelper
     Manifest::Manifest m_manifest;
     std::shared_ptr<ISource> m_source;
     std::shared_ptr<TestPackage> m_package;
+    bool m_hideSystemReferenceStrings = false;
 };
 
 TestPackageHelper MakeInstalled()
@@ -1475,3 +1482,52 @@ TEST_CASE("CompositeSource_CorrelateToInstalledContainsManifestData", "[Composit
     request.Query = RequestMatch(MatchType::Exact, "NotForEverything");
     SearchResult result = setup.Composite.Search(request);
 }
+
+TEST_CASE("CompositeSource_Respects_FeatureFlag_ManifestMayContainAdditionalSystemReferenceStrings", "[CompositeSource]")
+{
+    std::string id = "Special test ID";
+    std::string productCode1 = "product-code1";
+
+    CompositeTestSetup setup;
+    bool productCodeSearched = false;
+    setup.Installed->SearchFunction = [&](const SearchRequest& request)
+        {
+            for (const auto& inclusion : request.Inclusions)
+            {
+                if (inclusion.Field == PackageMatchField::ProductCode)
+                {
+                    productCodeSearched = true;
+                }
+            }
+
+            return SearchResult{};
+        };
+    setup.Available->SearchFunction = [&](const SearchRequest&)
+        {
+            SearchResult result;
+            result.Matches.emplace_back(MakeAvailable(setup.Available).WithId(id).WithPC(productCode1).HideSRS(), Criteria());
+            return result;
+        };
+
+    SECTION("Feature false")
+    {
+        SearchRequest request;
+        request.Query = RequestMatch(MatchType::Exact, "NotForEverything");
+        SearchResult result = setup.Composite.Search(request);
+
+        REQUIRE(!productCodeSearched);
+    }
+    SECTION("Feature true")
+    {
+        setup.Available->QueryFeatureFlagFunction = [](SourceFeatureFlag flag)
+            {
+                return (flag == SourceFeatureFlag::ManifestMayContainAdditionalSystemReferenceStrings);
+            };
+
+        SearchRequest request;
+        request.Query = RequestMatch(MatchType::Exact, "NotForEverything");
+        SearchResult result = setup.Composite.Search(request);
+
+        REQUIRE(productCodeSearched);
+    }
+}
diff --git a/src/AppInstallerCLITests/TestSource.cpp b/src/AppInstallerCLITests/TestSource.cpp
@@ -12,8 +12,8 @@ namespace TestCommon
     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) {}
+    TestPackageVersion::TestPackageVersion(const Manifest& manifest, std::weak_ptr<const ISource> source, bool hideSystemReferenceStrings) :
+        VersionManifest(manifest), Source(source), HideSystemReferenceStrings(hideSystemReferenceStrings) {}
 
     TestPackageVersion::LocIndString TestPackageVersion::GetProperty(PackageVersionProperty property) const
     {
@@ -47,16 +47,22 @@ namespace TestCommon
         switch (property)
         {
         case PackageVersionMultiProperty::PackageFamilyName:
-            for (const auto& installer : VersionManifest.Installers)
+            if (!HideSystemReferenceStrings)
             {
-                AddIfHasValueAndNotPresent(installer.PackageFamilyName, result, true);
+                for (const auto& installer : VersionManifest.Installers)
+                {
+                    AddIfHasValueAndNotPresent(installer.PackageFamilyName, result, true);
+                }
             }
             break;
         case PackageVersionMultiProperty::ProductCode:
-            for (const auto& installer : VersionManifest.Installers)
+            if (!HideSystemReferenceStrings)
             {
-                bool shouldFoldCaseForNonPortable = installer.EffectiveInstallerType() != AppInstaller::Manifest::InstallerTypeEnum::Portable;
-                AddIfHasValueAndNotPresent(installer.ProductCode, result, shouldFoldCaseForNonPortable);
+                for (const auto& installer : VersionManifest.Installers)
+                {
+                    bool shouldFoldCaseForNonPortable = installer.EffectiveInstallerType() != AppInstaller::Manifest::InstallerTypeEnum::Portable;
+                    AddIfHasValueAndNotPresent(installer.ProductCode, result, shouldFoldCaseForNonPortable);
+                }
             }
             break;
         case PackageVersionMultiProperty::Name:
@@ -111,11 +117,11 @@ namespace TestCommon
         }
     }
 
-    TestPackage::TestPackage(const std::vector<Manifest>& available, std::weak_ptr<const ISource> source)
+    TestPackage::TestPackage(const std::vector<Manifest>& available, std::weak_ptr<const ISource> source, bool hideSystemReferenceStringsOnVersion)
     {
         for (const auto& manifest : available)
         {
-            AvailableVersions.emplace_back(TestPackageVersion::Make(manifest, source));
+            AvailableVersions.emplace_back(TestPackageVersion::Make(manifest, source, hideSystemReferenceStringsOnVersion));
         }
     }
 
@@ -261,6 +267,11 @@ namespace TestCommon
         return Information;
     }
 
+    bool TestSource::QueryFeatureFlag(SourceFeatureFlag flag) const
+    {
+        return (QueryFeatureFlagFunction ? QueryFeatureFlagFunction(flag) : false);
+    }
+
     SearchResult TestSource::Search(const SearchRequest& request) const
     {
         if (SearchFunction)

diff --git a/src/AppInstallerCLITests/TestSource.h b/src/AppInstallerCLITests/TestSource.h
@@ -18,7 +18,7 @@ namespace TestCommon
         using LocIndString = AppInstaller::Utility::LocIndString;
         using MetadataMap = AppInstaller::Repository::IPackageVersion::Metadata;
 
-        TestPackageVersion(const Manifest& manifest, std::weak_ptr<const ISource> source = {});
+        TestPackageVersion(const Manifest& manifest, std::weak_ptr<const ISource> source = {}, bool hideSystemReferenceStrings = false);
         TestPackageVersion(const Manifest& manifest, MetadataMap installationMetadata, std::weak_ptr<const ISource> source = {});
 
         template <typename... Args>
@@ -36,6 +36,7 @@ namespace TestCommon
         Manifest VersionManifest;
         MetadataMap Metadata;
         std::weak_ptr<const ISource> Source;
+        bool HideSystemReferenceStrings = false;
 
     protected:
         static void AddIfHasValueAndNotPresent(const AppInstaller::Utility::NormalizedString& value, std::vector<LocIndString>& target, bool folded = false);
@@ -52,7 +53,7 @@ namespace TestCommon
         using MetadataMap = TestPackageVersion::MetadataMap;
 
         // Create a package with only available versions using these manifests.
-        TestPackage(const std::vector<Manifest>& available, std::weak_ptr<const ISource> source = {});
+        TestPackage(const std::vector<Manifest>& available, std::weak_ptr<const ISource> source = {}, bool hideSystemReferenceStringsOnVersion = false);
 
         // Create a package with an installed version, metadata, and optionally available versions.
         TestPackage(const Manifest& installed, MetadataMap installationMetadata, const std::vector<Manifest>& available = {}, std::weak_ptr<const ISource> source = {});
@@ -86,6 +87,9 @@ namespace TestCommon
         const std::string& GetIdentifier() const override;
         AppInstaller::Repository::SourceInformation GetInformation() const override;
 
+        bool QueryFeatureFlag(AppInstaller::Repository::SourceFeatureFlag flag) const override;
+        std::function<bool(AppInstaller::Repository::SourceFeatureFlag)> QueryFeatureFlagFunction;
+
         AppInstaller::Repository::SearchResult Search(const AppInstaller::Repository::SearchRequest& request) const override;
         void* CastTo(AppInstaller::Repository::ISourceType type) override;
 

diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp
@@ -1519,13 +1519,14 @@ namespace AppInstaller::Repository
             }
 
             SearchResult availableResult = result.SearchAndHandleFailures(source, request);
+            bool downloadManifests = source.QueryFeatureFlag(SourceFeatureFlag::ManifestMayContainAdditionalSystemReferenceStrings);
 
             for (auto&& match : availableResult.Matches)
             {
                 // Check for a package already in the result that should have been correlated already.
                 // In cases that PackageData will be created, also download manifests for system reference strings
                 // when search result is small (currently limiting to 1).
-                auto packageData = result.CheckForExistingResultFromAvailablePackageMatch(match, availableResult.Matches.size() == 1);
+                auto packageData = result.CheckForExistingResultFromAvailablePackageMatch(match, downloadManifests && availableResult.Matches.size() == 1);
 
                 // If found existing package in the result, continue
                 if (!packageData)

diff --git a/src/AppInstallerRepositoryCore/ISource.h b/src/AppInstallerRepositoryCore/ISource.h
@@ -35,6 +35,10 @@ namespace AppInstaller::Repository
         // Get the source's information after the source is opened.
         virtual SourceInformation GetInformation() const { return {}; };
 
+        // Query the value of the given feature flag.
+        // The default state of any new flag is false.
+        virtual bool QueryFeatureFlag(SourceFeatureFlag) const { return false; }
+
         // Execute a search on the source.
         virtual SearchResult Search(const SearchRequest& request) const = 0;
 

diff --git a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h
@@ -168,6 +168,15 @@ namespace AppInstaller::Repository
         std::vector<std::string> RequiredQueryParameters;
     };
 
+    // Allows calling code to inquire about specific features of an ISource implementation.
+    // The default state of any new flag is false.
+    enum class SourceFeatureFlag
+    {
+        // If true, the manifests for this source may contain more data than is available from just the
+        // version information found from a search.
+        ManifestMayContainAdditionalSystemReferenceStrings,
+    };
+
     // Represents a source which would be interacted from outside of repository lib.
     struct Source
     {
@@ -216,6 +225,10 @@ namespace AppInstaller::Repository
         // Get the source's information.
         SourceInformation GetInformation() const;
 
+        // Query the value of the given feature flag.
+        // The default state of any new flag is false.
+        bool QueryFeatureFlag(SourceFeatureFlag flag) const;
+
         // Returns true if the origin type can contain available packages.
         bool ContainsAvailablePackages() const;
 

diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp
@@ -563,6 +563,12 @@ namespace AppInstaller::Repository
         }
     }
 
+    bool Source::QueryFeatureFlag(SourceFeatureFlag flag) const
+    {
+        THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_source);
+        return m_source->QueryFeatureFlag(flag);
+    }
+
     bool Source::ContainsAvailablePackages() const
     {
         THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), IsComposite());

diff --git a/src/AppInstallerRepositoryCore/Rest/RestSource.cpp b/src/AppInstallerRepositoryCore/Rest/RestSource.cpp
@@ -436,6 +436,17 @@ namespace AppInstaller::Repository::Rest
         return m_information;
     }
 
+    bool RestSource::QueryFeatureFlag(SourceFeatureFlag flag) const
+    {
+        switch (flag)
+        {
+        case SourceFeatureFlag::ManifestMayContainAdditionalSystemReferenceStrings:
+            return true;
+        }
+
+        return false;
+    }
+
     SearchResult RestSource::Search(const SearchRequest& request) const
     {
         IRestClient::SearchResult results = m_restClient.Search(request);

diff --git a/src/AppInstallerRepositoryCore/Rest/RestSource.h b/src/AppInstallerRepositoryCore/Rest/RestSource.h
@@ -31,6 +31,8 @@ namespace AppInstaller::Repository::Rest
 
         SourceInformation GetInformation() const override;
 
+        bool QueryFeatureFlag(SourceFeatureFlag flag) const override;
+
         // Execute a search on the source.
         SearchResult Search(const SearchRequest& request) const override;