Skip to content

Commit c5fbdd9

Browse files
authored
Refactor IPackage (#4174)
## Change To better reflect the underlying code and continue to move business logic out of the `CompositeSource`, this change refactors the `IPackage` interface into 3: ```C++ // Contains a collection of package versions. struct IPackageVersionCollection { // Gets all versions of this package. // The versions will be returned in sorted, descending order. // Ex. { 4, 3, 2, 1 } virtual std::vector<PackageVersionKey> GetVersionKeys() const = 0; // Gets a specific version of this package. virtual std::shared_ptr<IPackageVersion> GetVersion(const PackageVersionKey& versionKey) const = 0; // A convenience method to effectively call `GetVersion(GetVersionKeys[0])`. virtual std::shared_ptr<IPackageVersion> GetLatestVersion() const = 0; }; // Contains information about a package and its versions from a single source. struct IPackage : public IPackageVersionCollection { // Gets a property of this package. virtual Utility::LocIndString GetProperty(PackageProperty property) const = 0; // Gets the source that this package is from. virtual Source GetSource() const = 0; // Determines if the given IPackage refers to the same package as this one. virtual bool IsSame(const IPackage*) const = 0; // Gets this object as the requested type, or null if it is not the requested type. virtual const void* CastTo(IPackageType type) const = 0; }; // Contains information about the graph of packages related to a search. struct ICompositePackage { // Gets a property of this package result. virtual Utility::LocIndString GetProperty(PackageProperty property) const = 0; // Gets the installed package information. virtual std::shared_ptr<IPackage> GetInstalled() = 0; // Gets all of the available packages for this result. // There will be at most one package per source in this list. virtual std::vector<std::shared_ptr<IPackage>> GetAvailable() = 0; }; ``` `IPackageVersionCollection` is simply a set of `IPackageVersion` objects with only the previous `IPackage` functions to enumerate them. This enables some code that only wants to inspect versions to use a tighter set of APIs and thus some implementations need not worry about things like the appropriate property values to return. The refactored `IPackage` inherits from `IPackageVersionCollection` and is itself a set of package versions that all come from a single source (as in `Source`). It adds the properties and the `Source` that the versions are all from. `ICompositePackage` is the new search result, resembling the previous `IPackage` in what it contains but exposing it more directly. The installed `IPackage` object will eventually be able to contain more than one installed version in a future change. The available packages are exposed directly as `IPackage` objects, rather than mixing the versions together in the `CompositeSource`. The code in `PackageVersionSelection.cpp` extracts the business logic that was inside `CompositeSource` previously, as well as providing a few helpers to make the existing code work more succinctly. The refactoring is not attempting to change behavior, but simply make it possible to change behavior more easily and precisely in the future. I would have preferred to go further by splitting `CompositeSource` out into a public type, allowing the other sources to have a simpler model of only ever returning `IPackage` results from search. This change would have created an even larger churn though and did not directly contribute towards the goal of supporting future side-by-side work.    Also changes the YAML object model to use a non-exceptional function to attempt to convert to an integer.
1 parent a3c686b commit c5fbdd9

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+1140
-801
lines changed

src/AppInstallerCLICore/ConfigurationWingetDscModuleUnitValidation.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,8 @@ namespace AppInstaller::CLI::Configuration
358358
{
359359
if (!package.Version.empty())
360360
{
361-
auto versionKeys = searchResult.Matches.at(0).Package->GetAvailableVersionKeys();
361+
std::shared_ptr<Repository::IPackage> availablePackage = searchResult.Matches.at(0).Package->GetAvailable().at(0);
362+
auto versionKeys = availablePackage->GetVersionKeys();
362363
bool foundVersion = false;
363364
for (auto const& versionKey : versionKeys)
364365
{

src/AppInstallerCLICore/ExecutionContextData.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ namespace AppInstaller::CLI::Execution
105105
template <>
106106
struct DataMapping<Data::Package>
107107
{
108-
using value_t = std::shared_ptr<Repository::IPackage>;
108+
using value_t = std::shared_ptr<Repository::ICompositePackage>;
109109
};
110110

111111
template <>

src/AppInstallerCLICore/Workflows/CompletionFlow.cpp

+13-7
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,14 @@ namespace AppInstaller::CLI::Workflow
7070
const std::string& word = context.Get<Data::CompletionData>().Word();
7171
auto stream = context.Reporter.Completion();
7272

73-
for (const auto& vc : context.Get<Execution::Data::Package>()->GetAvailableVersionKeys())
73+
for (const auto& ap : context.Get<Execution::Data::Package>()->GetAvailable())
7474
{
75-
if (word.empty() || Utility::ICUCaseInsensitiveStartsWith(vc.Version, word))
75+
for (const auto& vc : ap->GetVersionKeys())
7676
{
77-
OutputCompletionString(stream, vc.Version);
77+
if (word.empty() || Utility::ICUCaseInsensitiveStartsWith(vc.Version, word))
78+
{
79+
OutputCompletionString(stream, vc.Version);
80+
}
7881
}
7982
}
8083
}
@@ -86,12 +89,15 @@ namespace AppInstaller::CLI::Workflow
8689

8790
std::vector<std::string> channels;
8891

89-
for (const auto& vc : context.Get<Execution::Data::Package>()->GetAvailableVersionKeys())
92+
for (const auto& ap : context.Get<Execution::Data::Package>()->GetAvailable())
9093
{
91-
if ((word.empty() || Utility::ICUCaseInsensitiveStartsWith(vc.Channel, word)) &&
92-
std::find(channels.begin(), channels.end(), vc.Channel) == channels.end())
94+
for (const auto& vc : ap->GetVersionKeys())
9395
{
94-
channels.emplace_back(vc.Channel);
96+
if ((word.empty() || Utility::ICUCaseInsensitiveStartsWith(vc.Channel, word)) &&
97+
std::find(channels.begin(), channels.end(), vc.Channel) == channels.end())
98+
{
99+
channels.emplace_back(vc.Channel);
100+
}
95101
}
96102
}
97103

src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.cpp

+6-4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "DependencyNodeProcessor.h"
55
#include "ManifestComparator.h"
66
#include <winget/PinningData.h>
7+
#include <winget/PackageVersionSelection.h>
78

89
using namespace AppInstaller::Manifest;
910
using namespace AppInstaller::Repository;
@@ -41,20 +42,21 @@ namespace AppInstaller::CLI::Workflow
4142
const auto& match = matches.at(0);
4243
const auto& package = match.Package;
4344
auto packageId = package->GetProperty(PackageProperty::Id);
44-
m_nodePackageInstalledVersion = package->GetInstalledVersion();
45+
m_nodePackageInstalledVersion = GetInstalledVersion(package);
46+
std::shared_ptr<IPackageVersionCollection> availableVersions = GetAvailableVersionsForInstalledVersion(package);
4547

4648
if (m_context.Args.Contains(Execution::Args::Type::Force))
4749
{
48-
m_nodePackageLatestVersion = package->GetLatestAvailableVersion();
50+
m_nodePackageLatestVersion = availableVersions->GetLatestVersion();
4951
}
5052
else
5153
{
5254
Pinning::PinBehavior pinBehavior = m_context.Args.Contains(Execution::Args::Type::IncludePinned) ? Pinning::PinBehavior::IncludePinned : Pinning::PinBehavior::ConsiderPins;
5355

5456
Pinning::PinningData pinningData{ Pinning::PinningData::Disposition::ReadOnly };
55-
auto evaluator = pinningData.CreatePinStateEvaluator(pinBehavior, package->GetInstalledVersion());
57+
auto evaluator = pinningData.CreatePinStateEvaluator(pinBehavior, m_nodePackageInstalledVersion);
5658

57-
m_nodePackageLatestVersion = evaluator.GetLatestAvailableVersionForPins(package);
59+
m_nodePackageLatestVersion = evaluator.GetLatestAvailableVersionForPins(availableVersions);
5860
}
5961

6062
if (m_nodePackageInstalledVersion && dependencyNode.IsVersionOk(Utility::Version(m_nodePackageInstalledVersion->GetProperty(PackageVersionProperty::Version))))

src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp

+8-5
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "WorkflowBase.h"
1010
#include <winget/RepositorySearch.h>
1111
#include <winget/Runtime.h>
12+
#include <winget/PackageVersionSelection.h>
1213

1314
namespace AppInstaller::CLI::Workflow
1415
{
@@ -60,20 +61,22 @@ namespace AppInstaller::CLI::Workflow
6061
// If requested, checks that the installed version is available and reports a warning if it is not.
6162
std::shared_ptr<IPackageVersion> GetAvailableVersionForInstalledPackage(
6263
Execution::Context& context,
63-
std::shared_ptr<IPackage> package,
64+
std::shared_ptr<ICompositePackage> package,
6465
Utility::LocIndView version,
6566
Utility::LocIndView channel,
6667
bool checkVersion)
6768
{
69+
std::shared_ptr<IPackageVersionCollection> availableVersions = GetAvailableVersionsForInstalledVersion(package);
70+
6871
if (!checkVersion)
6972
{
70-
return package->GetLatestAvailableVersion();
73+
return availableVersions->GetLatestVersion();
7174
}
7275

73-
auto availablePackageVersion = package->GetAvailableVersion({ "", version, channel });
76+
auto availablePackageVersion = availableVersions->GetVersion({ "", version, channel });
7477
if (!availablePackageVersion)
7578
{
76-
availablePackageVersion = package->GetLatestAvailableVersion();
79+
availablePackageVersion = availableVersions->GetLatestVersion();
7780
if (availablePackageVersion)
7881
{
7982
// Warn installed version is not available.
@@ -100,7 +103,7 @@ namespace AppInstaller::CLI::Workflow
100103
auto& exportedSources = exportedPackages.Sources;
101104
for (const auto& packageMatch : searchResult.Matches)
102105
{
103-
auto installedPackageVersion = packageMatch.Package->GetInstalledVersion();
106+
auto installedPackageVersion = GetInstalledVersion(packageMatch.Package);
104107
auto version = installedPackageVersion->GetProperty(PackageVersionProperty::Version);
105108
auto channel = installedPackageVersion->GetProperty(PackageVersionProperty::Channel);
106109

src/AppInstallerCLICore/Workflows/PinFlow.cpp

+16-14
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "TableOutput.h"
77
#include <winget/PinningData.h>
88
#include <winget/RepositorySearch.h>
9+
#include <winget/PackageVersionSelection.h>
910

1011
using namespace AppInstaller::Repository;
1112

@@ -38,21 +39,20 @@ namespace AppInstaller::CLI::Workflow
3839

3940
if (context.Args.Contains(Execution::Args::Type::PinInstalled))
4041
{
41-
auto installedVersion = package->GetInstalledVersion();
42+
auto installedVersion = GetInstalledVersion(package);
4243
if (installedVersion)
4344
{
4445
pinKeys.emplace(Pinning::PinKey::GetPinKeyForInstalled(installedVersion->GetProperty(PackageVersionProperty::Id)));
4546
}
4647
}
4748
else
4849
{
49-
auto packageVersionKeys = package->GetAvailableVersionKeys();
50-
for (const auto& versionKey : packageVersionKeys)
50+
auto availablePackages = package->GetAvailable();
51+
for (const auto& availablePackage : availablePackages)
5152
{
52-
auto availableVersion = package->GetAvailableVersion(versionKey);
5353
pinKeys.emplace(
54-
availableVersion->GetProperty(PackageVersionProperty::Id).get(),
55-
availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get());
54+
availablePackage->GetProperty(PackageProperty::Id).get(),
55+
availablePackage->GetSource().GetIdentifier());
5656
}
5757
}
5858

@@ -140,7 +140,7 @@ namespace AppInstaller::CLI::Workflow
140140
}
141141
else
142142
{
143-
auto availableVersion = package->GetAvailableVersion({ pinKey.SourceId, "", "" });
143+
auto availableVersion = GetAvailablePackageFromSource(package, pinKey.SourceId)->GetLatestVersion();
144144
if (availableVersion)
145145
{
146146
packageNameToReport = availableVersion->GetProperty(PackageVersionProperty::Name);
@@ -198,8 +198,6 @@ namespace AppInstaller::CLI::Workflow
198198
// Note that if a source was specified in the command line,
199199
// that will be the only one we get version keys from.
200200
// So, we remove pins from all sources unless one was provided.
201-
auto packageVersionKeys = package->GetAvailableVersionKeys();
202-
203201
for (const auto& pin : pins)
204202
{
205203
AICLI_LOG(CLI, Info, << "Removing Pin " << pin.GetKey().ToString());
@@ -255,15 +253,19 @@ namespace AppInstaller::CLI::Workflow
255253
else
256254
{
257255
// This ensures we get the info from the right source if it exists on multiple
258-
auto availableVersion = match.Package->GetAvailableVersion({ pinKey.SourceId, "", "" });
259-
if (availableVersion)
256+
auto availablePackage = GetAvailablePackageFromSource(match.Package, pinKey.SourceId);
257+
if (availablePackage)
260258
{
261-
packageName = availableVersion->GetProperty(PackageVersionProperty::Name);
262-
sourceName = availableVersion->GetProperty(PackageVersionProperty::SourceName);
259+
auto availableVersion = availablePackage->GetLatestVersion();
260+
if (availableVersion)
261+
{
262+
packageName = availableVersion->GetProperty(PackageVersionProperty::Name);
263+
sourceName = availableVersion->GetProperty(PackageVersionProperty::SourceName);
264+
}
263265
}
264266
}
265267

266-
auto installedVersion = match.Package->GetInstalledVersion();
268+
auto installedVersion = GetInstalledVersion(match.Package);
267269
if (installedVersion)
268270
{
269271
packageName = installedVersion->GetProperty(PackageVersionProperty::Name);

src/AppInstallerCLICore/Workflows/RepairFlow.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "AppInstallerSynchronization.h"
1515
#include "MSStoreInstallerHandler.h"
1616
#include "ManifestComparator.h"
17+
#include <winget/PackageVersionSelection.h>
1718

1819
using namespace AppInstaller::Manifest;
1920
using namespace AppInstaller::Msix;
@@ -443,10 +444,11 @@ namespace AppInstaller::CLI::Workflow
443444
std::string_view requestedVersion = context.Args.Contains(Execution::Args::Type::Version) ? context.Args.GetArg(Execution::Args::Type::Version) : installedVersion.ToString();
444445
// If it's Store source with only one version unknown, use the unknown version for available version mapping.
445446
const auto& package = context.Get<Execution::Data::Package>();
446-
auto versionKeys = package->GetAvailableVersionKeys();
447+
auto packageVersions = GetAvailableVersionsForInstalledVersion(package, installedPackage);
448+
auto versionKeys = packageVersions->GetVersionKeys();
447449
if (versionKeys.size() == 1)
448450
{
449-
auto packageVersion = package->GetAvailableVersion(versionKeys.at(0));
451+
auto packageVersion = packageVersions->GetVersion(versionKeys.at(0));
450452
if (packageVersion->GetSource().IsWellKnownSource(WellKnownSource::MicrosoftStore) &&
451453
Utility::Version{ packageVersion->GetProperty(PackageVersionProperty::Version) }.IsUnknown())
452454
{

src/AppInstallerCLICore/Workflows/UninstallFlow.cpp

+16-6
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <AppInstallerDeployment.h>
1212
#include <AppInstallerSynchronization.h>
1313
#include <winget/Runtime.h>
14+
#include <winget/PackageVersionSelection.h>
1415

1516
using namespace AppInstaller::CLI::Execution;
1617
using namespace AppInstaller::Manifest;
@@ -34,9 +35,8 @@ namespace AppInstaller::CLI::Workflow
3435
std::string SourceIdentifier;
3536
};
3637

37-
void AddIfRemoteAndNotPresent(const std::shared_ptr<IPackageVersion>& packageVersion)
38+
void AddIfRemoteAndNotPresent(Source&& source, const Utility::LocIndString& identifier)
3839
{
39-
auto source = packageVersion->GetSource();
4040
const auto details = source.GetDetails();
4141
if (!source.ContainsAvailablePackages())
4242
{
@@ -51,7 +51,17 @@ namespace AppInstaller::CLI::Workflow
5151
}
5252
}
5353

54-
Items.emplace_back(Item{ packageVersion->GetProperty(PackageVersionProperty::Id), std::move(source), details.Identifier });
54+
Items.emplace_back(Item{ identifier, std::move(source), details.Identifier });
55+
}
56+
57+
void AddIfRemoteAndNotPresent(const std::shared_ptr<IPackageVersion>& packageVersion)
58+
{
59+
AddIfRemoteAndNotPresent(packageVersion->GetSource(), packageVersion->GetProperty(PackageVersionProperty::Id));
60+
}
61+
62+
void AddIfRemoteAndNotPresent(const std::shared_ptr<IPackage>& package)
63+
{
64+
AddIfRemoteAndNotPresent(package->GetSource(), package->GetProperty(PackageProperty::Id));
5565
}
5666

5767
std::vector<Item> Items;
@@ -311,12 +321,12 @@ namespace AppInstaller::CLI::Workflow
311321
UninstallCorrelatedSources correlatedSources;
312322

313323
// Start with the installed version
314-
correlatedSources.AddIfRemoteAndNotPresent(package->GetInstalledVersion());
324+
correlatedSources.AddIfRemoteAndNotPresent(GetInstalledVersion(package));
315325

316326
// Then look through all available versions
317-
for (const auto& versionKey : package->GetAvailableVersionKeys())
327+
for (const auto& availablePackage : package->GetAvailable())
318328
{
319-
correlatedSources.AddIfRemoteAndNotPresent(package->GetAvailableVersion(versionKey));
329+
correlatedSources.AddIfRemoteAndNotPresent(availablePackage);
320330
}
321331

322332
// Finally record the uninstall for each found value

src/AppInstallerCLICore/Workflows/UpdateFlow.cpp

+6-5
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
3-
43
#include "pch.h"
54
#include "WorkflowBase.h"
65
#include "DependenciesFlow.h"
76
#include "InstallFlow.h"
87
#include "UpdateFlow.h"
98
#include "ManifestComparator.h"
109
#include <winget/PinningData.h>
10+
#include <winget/PackageVersionSelection.h>
1111

1212
using namespace AppInstaller::Repository;
1313
using namespace AppInstaller::Repository::Microsoft;
@@ -72,10 +72,11 @@ namespace AppInstaller::CLI::Workflow
7272
const bool includePinned = m_isSinglePackage || context.Args.Contains(Execution::Args::Type::IncludePinned);
7373

7474
PinningData pinningData{ PinningData::Disposition::ReadOnly };
75-
auto evaluator = pinningData.CreatePinStateEvaluator(includePinned ? PinBehavior::IncludePinned : PinBehavior::ConsiderPins, package->GetInstalledVersion());
75+
auto evaluator = pinningData.CreatePinStateEvaluator(includePinned ? PinBehavior::IncludePinned : PinBehavior::ConsiderPins, GetInstalledVersion(package));
7676

7777
// The version keys should have already been sorted by version
78-
const auto& versionKeys = package->GetAvailableVersionKeys();
78+
auto availableVersions = GetAvailableVersionsForInstalledVersion(package);
79+
const auto& versionKeys = availableVersions->GetVersionKeys();
7980
// Assume that no update versions are applicable
8081
bool upgradeVersionAvailable = false;
8182
for (const auto& key : versionKeys)
@@ -89,7 +90,7 @@ namespace AppInstaller::CLI::Workflow
8990
upgradeVersionAvailable = true;
9091
}
9192

92-
auto packageVersion = package->GetAvailableVersion(key);
93+
auto packageVersion = availableVersions->GetVersion(key);
9394

9495
// Check if the package is pinned
9596
PinType pinType = evaluator.EvaluatePinType(packageVersion);
@@ -217,7 +218,7 @@ namespace AppInstaller::CLI::Workflow
217218
auto updateContextPtr = context.CreateSubContext();
218219
Execution::Context& updateContext = *updateContextPtr;
219220
auto previousThreadGlobals = updateContext.SetForCurrentThread();
220-
auto installedVersion = match.Package->GetInstalledVersion();
221+
auto installedVersion = GetInstalledVersion(match.Package);
221222

222223
updateContext.Add<Execution::Data::Package>(match.Package);
223224

0 commit comments

Comments
 (0)