Improve slow searches involving installed items (#5701)

Fixes #5629 
(and probably quite a few more list/upgrade some-query-provided
performance related issues)

While things could still be made better, the good case (where files were
already cached locally) of the previous behavior trying to complete
`winget list Micro` was taking 51 seconds. With this change, it takes 6
seconds on the debug build. Given that there is intentionally little to
no I/O and much of the CPU time is likely spent in our code, that should
speed up more with the release.

## Issue
The indexed `IPackage` implementation for v2 needed to acquire the
intermediate manifest to provide the response for `GetVersionKeys`.
Since this is used extensively to perform correlation between available
packages and local items during a search, any available packages found
by the query would then result in a download/disk read of the
intermediate manifest.

## Change
Introduces `IPackage::GetMultiProperty`, allowing code that just wanted
to get all of these values across all versions to do so more
efficiently. For the v2 index, this is how the data is already stored in
the database. For the other `IPackage` implementations, the data was
also present and is just aggregated from each version's output.

Author: JohnMcPMS

src/AppInstallerCLITests/CompositeSource.cpp

@@ -166,11 +166,26 @@ struct CompositeTestSetup
         Composite.AddAvailableSource(Source{ Available });
     }
 
-    SearchResult Search()
+    SearchResult Search(bool disableDataChecks = false)
     {
+        size_t initialCountOfCallsRequiringVersionData = Available->CountOfCallsRequiringVersionData;
+        size_t initialCountOfCallsRequiringManifestData = Available->CountOfCallsRequiringManifestData;
+
         SearchRequest request;
         request.Query = RequestMatch(MatchType::Exact, s_Everything_Query);
-        return Composite.Search(request);
+        auto result = Composite.Search(request);
+
+        // We want to prevent calls to these functions for Search as they can require network or I/O activity.
+        size_t countOfCallsRequiringVersionData = Available->CountOfCallsRequiringVersionData - initialCountOfCallsRequiringVersionData;
+        size_t countOfCallsRequiringManifestData = Available->CountOfCallsRequiringManifestData - initialCountOfCallsRequiringManifestData;
+        if (!disableDataChecks && (countOfCallsRequiringVersionData || countOfCallsRequiringManifestData))
+        {
+            std::ostringstream stream;
+            stream << "Version data calls [" << countOfCallsRequiringVersionData << "] : Manifest data calls [" << countOfCallsRequiringManifestData << "]";
+            FAIL(stream.str());
+        }
+
+        return result;
     }
 
     TestPackageHelper MakeInstalled(std::shared_ptr<ISource> source)
@@ -572,7 +587,8 @@ TEST_CASE("CompositePackage_AvailableVersions_ChannelFilteredOut", "[CompositeSo
         return result;
     };
 
-    SearchResult result = setup.Search();
+    // Disable data checks as we call one of the data methods as validation in the search
+    SearchResult result = setup.Search(true);
 
     REQUIRE(result.Matches.size() == 1);
     REQUIRE(result.Matches[0].Package->GetAvailable().size() == 1);
@@ -614,7 +630,8 @@ TEST_CASE("CompositePackage_AvailableVersions_NoChannelFilteredOut", "[Composite
         return result;
     };
 
-    SearchResult result = setup.Search();
+    // Disable data checks as we call one of the data methods as validation in the search
+    SearchResult result = setup.Search(true);
 
     REQUIRE(result.Matches.size() == 1);
     REQUIRE(result.Matches[0].Package->GetAvailable().size() == 1);

src/AppInstallerCLITests/SQLiteIndexSource.cpp

@@ -233,3 +233,29 @@ TEST_CASE("SQLiteIndexSource_IsSame", "[sqliteindexsource]")
 
     REQUIRE(result1.Matches[0].Package->GetAvailable()[0]->IsSame(result2.Matches[0].Package->GetAvailable()[0].get()));
 }
+
+TEST_CASE("SQLiteIndexSource_Package_ProductCodes", "[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 results = source->Search(request);
+    REQUIRE(results.Matches.size() == 1);
+    REQUIRE(results.Matches[0].Package);
+
+    auto package = results.Matches[0].Package->GetAvailable()[0];
+
+    auto manifestPCs = manifest.GetProductCodes();
+    auto propertyPCs = package->GetMultiProperty(PackageMultiProperty::ProductCode);
+    REQUIRE(manifestPCs.size() == 1);
+    REQUIRE(propertyPCs.size() == 1);
+    REQUIRE(manifestPCs[0] == propertyPCs[0].get());
+}

src/AppInstallerCLITests/TestSource.cpp

@@ -16,6 +16,19 @@ namespace TestCommon
             static std::atomic_size_t packageId(0);
             return ++packageId;
         }
+
+        TestSource* GetTestSourceFromWeakPtr(const std::weak_ptr<const ISource>& weakSource)
+        {
+            if (auto source = weakSource.lock())
+            {
+                if (auto testSource = const_cast<ISource*>(source.get())->CastTo(TestSource::SourceType))
+                {
+                    return reinterpret_cast<TestSource*>(testSource);
+                }
+            }
+
+            return nullptr;
+        }
     }
 
     TestPackageVersion::TestPackageVersion(const Manifest& manifest, MetadataMap installationMetadata, std::weak_ptr<const ISource> source) :
@@ -100,6 +113,11 @@ namespace TestCommon
 
     TestPackageVersion::Manifest TestPackageVersion::GetManifest()
     {
+        if (auto source = GetTestSource())
+        {
+            source->IncrementCountOfCallsRequiringManifestData();
+        }
+
         return VersionManifest;
     }
 
@@ -126,6 +144,11 @@ namespace TestCommon
         }
     }
 
+    TestSource* TestPackageVersion::GetTestSource() const
+    {
+        return GetTestSourceFromWeakPtr(Source);
+    }
+
     TestPackage::TestPackage(const std::vector<Manifest>& available, std::weak_ptr<const ISource> source, bool hideSystemReferenceStringsOnVersion) :
         Source(source)
     {
@@ -168,8 +191,34 @@ namespace TestCommon
         }
     }
 
+    std::vector<TestPackage::LocIndString> TestPackage::GetMultiProperty(PackageMultiProperty property) const
+    {
+        std::vector<LocIndString> result;
+        PackageVersionMultiProperty mappedProperty = PackageMultiPropertyToPackageVersionMultiProperty(property);
+
+        for (const auto& version : Versions)
+        {
+            for (auto&& string : version->GetMultiProperty(mappedProperty))
+            {
+                auto itr = std::lower_bound(result.begin(), result.end(), string);
+
+                if (itr == result.end() || *itr != string)
+                {
+                    result.emplace(itr, std::move(string));
+                }
+            }
+        }
+
+        return result;
+    }
+
     std::vector<PackageVersionKey> TestPackage::GetVersionKeys() const
     {
+        if (auto source = GetTestSource())
+        {
+            source->IncrementCountOfCallsRequiringVersionData();
+        }
+
         std::vector<PackageVersionKey> result;
         for (const auto& version : Versions)
         {
@@ -190,6 +239,14 @@ namespace TestCommon
 
     std::shared_ptr<IPackageVersion> TestPackage::GetVersion(const PackageVersionKey& versionKey) const
     {
+        if (!versionKey.IsDefaultLatest())
+        {
+            if (auto source = GetTestSource())
+            {
+                source->IncrementCountOfCallsRequiringVersionData();
+            }
+        }
+
         for (const auto& version : Versions)
         {
             if ((versionKey.Version.empty() || versionKey.Version == version->GetProperty(PackageVersionProperty::Version).get()) && 
@@ -234,6 +291,11 @@ namespace TestCommon
         return nullptr;
     }
 
+    TestSource* TestPackage::GetTestSource() const
+    {
+        return GetTestSourceFromWeakPtr(Source);
+    }
+
     TestCompositePackage::TestCompositePackage(const std::vector<Manifest>& available, std::weak_ptr<const ISource> source, bool hideSystemReferenceStringsOnVersion)
     {
         if (!available.empty())
@@ -332,6 +394,16 @@ namespace TestCommon
         return nullptr;
     }
 
+    void TestSource::IncrementCountOfCallsRequiringVersionData()
+    {
+        ++CountOfCallsRequiringVersionData;
+    }
+
+    void TestSource::IncrementCountOfCallsRequiringManifestData()
+    {
+        ++CountOfCallsRequiringManifestData;
+    }
+
     std::string_view TestSourceFactory::TypeName() const
     {
         return "*TestSource"sv;

src/AppInstallerCLITests/TestSource.h

@@ -10,6 +10,8 @@
 
 namespace TestCommon
 {
+    struct TestSource;
+
     // IPackageVersion for TestSource
     struct TestPackageVersion : public AppInstaller::Repository::IPackageVersion
     {
@@ -40,6 +42,7 @@ namespace TestCommon
 
     protected:
         static void AddIfHasValueAndNotPresent(const AppInstaller::Utility::NormalizedString& value, std::vector<LocIndString>& target, bool folded = false);
+        TestSource* GetTestSource() const;
     };
 
     // IPackage for TestSource
@@ -65,6 +68,7 @@ namespace TestCommon
         }
 
         AppInstaller::Utility::LocIndString GetProperty(AppInstaller::Repository::PackageProperty property) const override;
+        std::vector<AppInstaller::Utility::LocIndString> GetMultiProperty(AppInstaller::Repository::PackageMultiProperty property) const override;
         std::vector<AppInstaller::Repository::PackageVersionKey> GetVersionKeys() const override;
         std::shared_ptr<AppInstaller::Repository::IPackageVersion> GetLatestVersion() const override;
         std::shared_ptr<AppInstaller::Repository::IPackageVersion> GetVersion(const AppInstaller::Repository::PackageVersionKey& versionKey) const override;
@@ -76,6 +80,9 @@ namespace TestCommon
         std::weak_ptr<const ISource> Source;
         size_t DefaultIsSameIdentity = 0;
         std::function<bool(const IPackage*, const IPackage*)> IsSameOverride;
+
+    protected:
+        TestSource* GetTestSource() const;
     };
 
     // ICompositePackage for TestSource
@@ -127,6 +134,13 @@ namespace TestCommon
 
         TestSource() = default;
         TestSource(const AppInstaller::Repository::SourceDetails& details) : Details(details) {}
+
+        // Tracking for potential network impacts
+        void IncrementCountOfCallsRequiringVersionData();
+        size_t CountOfCallsRequiringVersionData = 0;
+
+        void IncrementCountOfCallsRequiringManifestData();
+        size_t CountOfCallsRequiringManifestData = 0;
     };
 
     struct TestSourceReference : public AppInstaller::Repository::ISourceReference