Skip to content

Commit 6096b5e

Browse files
JohnMcPMSJohnMcPMS
authored
Refactor ProcessMultiplePackages for clarity (#5340)
## Change A refactor of `ProcessMultiplePackages` to increase the clarity of what is being requested as options. This is done by passing a flags enum rather than 5 booleans. Also removes some extraneous `Workflow::` namespaces and improves the logging when going backward in execution phase.
1 parent 511d4f9 commit 6096b5e

File tree

8 files changed

+103
-72
lines changed

8 files changed

+103
-72
lines changed

‎src/AppInstallerCLICore/Commands/InstallCommand.cpp‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -119,47 +119,49 @@ namespace AppInstaller::CLI
119119
{
120120
context.SetFlags(ContextFlag::ShowSearchResultsOnPartialFailure);
121121

122-
context << Workflow::InitializeInstallerDownloadAuthenticatorsMap;
122+
context << InitializeInstallerDownloadAuthenticatorsMap;
123123

124124
if (context.Args.Contains(Execution::Args::Type::Manifest))
125125
{
126126
context <<
127-
Workflow::ReportExecutionStage(ExecutionStage::Discovery) <<
128-
Workflow::GetManifestFromArg <<
129-
Workflow::SelectInstaller <<
130-
Workflow::EnsureApplicableInstaller <<
131-
Workflow::Checkpoint("PreInstallCheckpoint", {}) << // TODO: Capture context data
132-
Workflow::InstallSinglePackage;
127+
ReportExecutionStage(ExecutionStage::Discovery) <<
128+
GetManifestFromArg <<
129+
SelectInstaller <<
130+
EnsureApplicableInstaller <<
131+
Checkpoint("PreInstallCheckpoint", {}) << // TODO: Capture context data
132+
InstallSinglePackage;
133133
}
134134
else
135135
{
136136
context <<
137-
Workflow::ReportExecutionStage(ExecutionStage::Discovery) <<
138-
Workflow::OpenSource();
137+
ReportExecutionStage(ExecutionStage::Discovery) <<
138+
OpenSource();
139139

140140
if (!context.Args.Contains(Execution::Args::Type::Force))
141141
{
142142
context <<
143-
Workflow::OpenCompositeSource(Workflow::DetermineInstalledSource(context), false, Repository::CompositeSearchBehavior::AvailablePackages);
143+
OpenCompositeSource(DetermineInstalledSource(context), false, Repository::CompositeSearchBehavior::AvailablePackages);
144144
}
145145

146146
if (context.Args.Contains(Execution::Args::Type::MultiQuery))
147147
{
148-
bool skipDependencies = Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies);
148+
ProcessMultiplePackages::Flags flags = ProcessMultiplePackages::Flags::None;
149+
if (Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies))
150+
{
151+
flags = ProcessMultiplePackages::Flags::IgnoreDependencies;
152+
}
153+
149154
context <<
150-
Workflow::GetMultiSearchRequests <<
151-
Workflow::SearchSubContextsForSingle() <<
152-
Workflow::ReportExecutionStage(Workflow::ExecutionStage::Execution) <<
153-
Workflow::ProcessMultiplePackages(
154-
Resource::String::PackageRequiresDependencies,
155-
APPINSTALLER_CLI_ERROR_MULTIPLE_INSTALL_FAILED,
156-
{}, true, skipDependencies);
155+
GetMultiSearchRequests <<
156+
SearchSubContextsForSingle() <<
157+
ReportExecutionStage(ExecutionStage::Execution) <<
158+
ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_MULTIPLE_INSTALL_FAILED, flags);
157159
}
158160
else
159161
{
160162
context <<
161-
Workflow::Checkpoint("PreInstallCheckpoint", {}) << // TODO: Capture context data
162-
Workflow::InstallOrUpgradeSinglePackage(OperationType::Install);
163+
Checkpoint("PreInstallCheckpoint", {}) << // TODO: Capture context data
164+
InstallOrUpgradeSinglePackage(OperationType::Install);
163165
}
164166
}
165167
}

‎src/AppInstallerCLICore/Commands/UpgradeCommand.cpp‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,10 @@ namespace AppInstaller::CLI
158158
}
159159

160160
context <<
161-
Workflow::InitializeInstallerDownloadAuthenticatorsMap <<
162-
Workflow::ReportExecutionStage(ExecutionStage::Discovery) <<
163-
Workflow::OpenSource() <<
164-
Workflow::OpenCompositeSource(Workflow::DetermineInstalledSource(context));
161+
InitializeInstallerDownloadAuthenticatorsMap <<
162+
ReportExecutionStage(ExecutionStage::Discovery) <<
163+
OpenSource() <<
164+
OpenCompositeSource(DetermineInstalledSource(context));
165165

166166
if (ShouldListUpgrade(context.Args))
167167
{
@@ -200,19 +200,21 @@ namespace AppInstaller::CLI
200200
// The remaining case: search for specific packages to update
201201
if (!context.Args.Contains(Execution::Args::Type::MultiQuery))
202202
{
203-
context << Workflow::InstallOrUpgradeSinglePackage(OperationType::Upgrade);
203+
context << InstallOrUpgradeSinglePackage(OperationType::Upgrade);
204204
}
205205
else
206206
{
207-
bool skipDependencies = Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies);
207+
ProcessMultiplePackages::Flags flags = ProcessMultiplePackages::Flags::None;
208+
if (Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies))
209+
{
210+
flags = ProcessMultiplePackages::Flags::IgnoreDependencies;
211+
}
212+
208213
context <<
209-
Workflow::GetMultiSearchRequests <<
210-
Workflow::SearchSubContextsForSingle(OperationType::Upgrade) <<
211-
Workflow::ReportExecutionStage(Workflow::ExecutionStage::Execution) <<
212-
Workflow::ProcessMultiplePackages(
213-
Resource::String::PackageRequiresDependencies,
214-
APPINSTALLER_CLI_ERROR_MULTIPLE_INSTALL_FAILED,
215-
{}, true, skipDependencies);
214+
GetMultiSearchRequests <<
215+
SearchSubContextsForSingle(OperationType::Upgrade) <<
216+
ReportExecutionStage(ExecutionStage::Execution) <<
217+
ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_MULTIPLE_INSTALL_FAILED, flags);
216218
}
217219
}
218220
}

‎src/AppInstallerCLICore/ExecutionContext.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ namespace AppInstaller::CLI::Execution
465465
}
466466
else if (m_executionStage > stage)
467467
{
468-
THROW_HR_MSG(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), "Reporting ExecutionStage to an earlier Stage without allowBackward as true");
468+
THROW_HR_MSG(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), "Reporting ExecutionStage to an earlier Stage: current[%d], new[%d]", ToIntegral(m_executionStage), ToIntegral(stage));
469469
}
470470

471471
m_executionStage = stage;

‎src/AppInstallerCLICore/Workflows/ImportExportFlow.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,9 +319,9 @@ namespace AppInstaller::CLI::Workflow
319319
context.Add<Execution::Data::Dependencies>(allDependencies);
320320

321321
context <<
322-
Workflow::ReportDependencies(Resource::String::ImportCommandReportDependencies) <<
323-
Workflow::ProcessMultiplePackages(
324-
Resource::String::ImportCommandReportDependencies, APPINSTALLER_CLI_ERROR_IMPORT_INSTALL_FAILED, {}, true, true);
322+
ReportDependencies(Resource::String::ImportCommandReportDependencies) <<
323+
ProcessMultiplePackages(
324+
Resource::String::ImportCommandReportDependencies, APPINSTALLER_CLI_ERROR_IMPORT_INSTALL_FAILED, ProcessMultiplePackages::Flags::IgnoreDependencies);
325325

326326
if (context.GetTerminationHR() == APPINSTALLER_CLI_ERROR_IMPORT_INSTALL_FAILED)
327327
{

‎src/AppInstallerCLICore/Workflows/InstallFlow.cpp‎

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -584,32 +584,36 @@ namespace AppInstaller::CLI::Workflow
584584

585585
void InstallDependencies(Execution::Context& context)
586586
{
587+
using Flags = ProcessMultiplePackages::Flags;
588+
587589
if (Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies))
588590
{
589591
context.Reporter.Warn() << Resource::String::DependenciesSkippedMessage << std::endl;
590592
return;
591593
}
592594

593595
context <<
594-
Workflow::GetDependenciesFromInstaller <<
595-
Workflow::ReportDependencies(Resource::String::PackageRequiresDependencies) <<
596-
Workflow::EnableWindowsFeaturesDependencies <<
597-
Workflow::ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES, {}, true, true, true, true);
596+
GetDependenciesFromInstaller <<
597+
ReportDependencies(Resource::String::PackageRequiresDependencies) <<
598+
EnableWindowsFeaturesDependencies <<
599+
ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES, Flags::IgnoreDependencies | Flags::StopOnFailure | Flags::RefreshPathVariable);
598600
}
599601

600602
void DownloadPackageDependencies(Execution::Context& context)
601603
{
604+
using Flags = ProcessMultiplePackages::Flags;
605+
602606
if (Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies))
603607
{
604608
context.Reporter.Warn() << Resource::String::DependenciesSkippedMessage << std::endl;
605609
return;
606610
}
607611

608612
context <<
609-
Workflow::GetDependenciesFromInstaller <<
610-
Workflow::ReportDependencies(Resource::String::PackageRequiresDependencies) <<
611-
Workflow::CreateDependencySubContexts(Resource::String::PackageRequiresDependencies) <<
612-
Workflow::ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_DOWNLOAD_DEPENDENCIES, {}, true, true, true, false, true);
613+
GetDependenciesFromInstaller <<
614+
ReportDependencies(Resource::String::PackageRequiresDependencies) <<
615+
CreateDependencySubContexts(Resource::String::PackageRequiresDependencies) <<
616+
ProcessMultiplePackages(Resource::String::PackageRequiresDependencies, APPINSTALLER_CLI_ERROR_DOWNLOAD_DEPENDENCIES, Flags::IgnoreDependencies | Flags::StopOnFailure | Flags::DownloadOnly);
613617
}
614618

615619
void InstallSinglePackage(Execution::Context& context)
@@ -675,6 +679,25 @@ namespace AppInstaller::CLI::Workflow
675679
Workflow::EnsureValidNestedInstallerMetadataForArchiveInstall;
676680
}
677681

682+
ProcessMultiplePackages::ProcessMultiplePackages(
683+
StringResource::StringId dependenciesReportMessage,
684+
HRESULT resultOnFailure,
685+
Flags flags,
686+
std::vector<HRESULT>&& ignorableInstallResults) :
687+
WorkflowTask("ProcessMultiplePackages"),
688+
m_dependenciesReportMessage(dependenciesReportMessage),
689+
m_resultOnFailure(resultOnFailure),
690+
m_ignorableInstallResults(std::move(ignorableInstallResults))
691+
{
692+
// Inverted
693+
m_ensurePackageAgreements = !WI_IsFlagSet(flags, Flags::SkipPackageAgreements);
694+
695+
m_ignorePackageDependencies = WI_IsFlagSet(flags, Flags::IgnoreDependencies);
696+
m_stopOnFailure = WI_IsFlagSet(flags, Flags::StopOnFailure);
697+
m_refreshPathVariable = WI_IsFlagSet(flags, Flags::RefreshPathVariable);
698+
m_downloadOnly = WI_IsFlagSet(flags, Flags::DownloadOnly);
699+
}
700+
678701
void ProcessMultiplePackages::operator()(Execution::Context& context) const
679702
{
680703
if (!context.Contains(Execution::Data::PackageSubContexts))
@@ -692,11 +715,11 @@ namespace AppInstaller::CLI::Workflow
692715
return;
693716
}
694717

718+
auto& packageSubContexts = context.Get<Execution::Data::PackageSubContexts>();
719+
695720
// Report dependencies
696721
if (!m_ignorePackageDependencies)
697722
{
698-
auto& packageSubContexts = context.Get<Execution::Data::PackageSubContexts>();
699-
700723
DependencyList allDependencies;
701724

702725
for (auto& packageContext : packageSubContexts)
@@ -721,10 +744,10 @@ namespace AppInstaller::CLI::Workflow
721744
}
722745

723746
bool allSucceeded = true;
724-
size_t packagesCount = context.Get<Execution::Data::PackageSubContexts>().size();
747+
size_t packagesCount = packageSubContexts.size();
725748
size_t packagesProgress = 0;
726749

727-
for (auto& packageContext : context.Get<Execution::Data::PackageSubContexts>())
750+
for (auto& packageContext : packageSubContexts)
728751
{
729752
packagesProgress++;
730753
context.Reporter.Info() << '(' << packagesProgress << '/' << packagesCount << ") "_liv;
@@ -744,7 +767,7 @@ namespace AppInstaller::CLI::Workflow
744767
currentContext <<
745768
Workflow::EnableWindowsFeaturesDependencies <<
746769
Workflow::CreateDependencySubContexts(m_dependenciesReportMessage) <<
747-
Workflow::ProcessMultiplePackages(m_dependenciesReportMessage, APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES, {}, true, true, true, true);
770+
Workflow::ProcessMultiplePackages(m_dependenciesReportMessage, APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES, Flags::IgnoreDependencies | Flags::StopOnFailure | Flags::RefreshPathVariable);
748771
}
749772

750773
currentContext << Workflow::DownloadInstaller;

‎src/AppInstallerCLICore/Workflows/InstallFlow.h‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -167,24 +167,22 @@ namespace AppInstaller::CLI::Workflow
167167
// Outputs: None
168168
struct ProcessMultiplePackages : public WorkflowTask
169169
{
170+
// Flags to signal change from default behavior of the task.
171+
enum class Flags : uint32_t
172+
{
173+
None = 0x00,
174+
SkipPackageAgreements = 0x01,
175+
IgnoreDependencies = 0x02,
176+
StopOnFailure = 0x04,
177+
RefreshPathVariable = 0x08,
178+
DownloadOnly = 0x10,
179+
};
180+
170181
ProcessMultiplePackages(
171182
StringResource::StringId dependenciesReportMessage,
172183
HRESULT resultOnFailure,
173-
std::vector<HRESULT>&& ignorableInstallResults = {},
174-
bool ensurePackageAgreements = true,
175-
bool ignoreDependencies = false,
176-
bool stopOnFailure = false,
177-
bool refreshPathVariable = false,
178-
bool downloadOnly = false):
179-
WorkflowTask("ProcessMultiplePackages"),
180-
m_dependenciesReportMessage(dependenciesReportMessage),
181-
m_resultOnFailure(resultOnFailure),
182-
m_ignorableInstallResults(std::move(ignorableInstallResults)),
183-
m_ignorePackageDependencies(ignoreDependencies),
184-
m_ensurePackageAgreements(ensurePackageAgreements),
185-
m_stopOnFailure(stopOnFailure),
186-
m_refreshPathVariable(refreshPathVariable),
187-
m_downloadOnly(downloadOnly){}
184+
Flags flags = Flags::None,
185+
std::vector<HRESULT>&& ignorableInstallResults = {});
188186

189187
void operator()(Execution::Context& context) const override;
190188

@@ -199,6 +197,8 @@ namespace AppInstaller::CLI::Workflow
199197
bool m_downloadOnly;
200198
};
201199

200+
DEFINE_ENUM_FLAG_OPERATORS(ProcessMultiplePackages::Flags);
201+
202202
// Stores the existing set of packages in ARP.
203203
// Required Args: None
204204
// Inputs: Installer

‎src/AppInstallerCLICore/Workflows/UpdateFlow.cpp‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,13 +269,19 @@ namespace AppInstaller::CLI::Workflow
269269
{
270270
context.Add<Execution::Data::PackageSubContexts>(std::move(packageSubContexts));
271271
context.Reporter.Info() << std::endl;
272-
bool skipDependencies = Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies);
272+
273+
ProcessMultiplePackages::Flags flags = ProcessMultiplePackages::Flags::None;
274+
if (Settings::User().Get<Settings::Setting::InstallSkipDependencies>() || context.Args.Contains(Execution::Args::Type::SkipDependencies))
275+
{
276+
flags = ProcessMultiplePackages::Flags::IgnoreDependencies;
277+
}
278+
273279
context <<
274280
ProcessMultiplePackages(
275281
Resource::String::PackageRequiresDependencies,
276282
APPINSTALLER_CLI_ERROR_UPDATE_ALL_HAS_FAILURE,
277-
{ APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE },
278-
true, skipDependencies);
283+
flags,
284+
{ APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE });
279285
}
280286

281287
if (packagesWithUnknownVersionSkipped > 0)

‎src/AppInstallerCLITests/InstallDependenciesFlow.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,10 @@ void OverrideOpenSourceForDependencies(TestContext& context)
3030

3131
void OverrideForProcessMultiplePackages(TestContext& context)
3232
{
33-
context.Override({ Workflow::ProcessMultiplePackages(
33+
context.Override({ ProcessMultiplePackages(
3434
Resource::String::PackageRequiresDependencies,
3535
APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES,
36-
{},
37-
false,
38-
true), [](TestContext&)
36+
ProcessMultiplePackages::Flags::SkipPackageAgreements | ProcessMultiplePackages::Flags::IgnoreDependencies), [](TestContext&)
3937
{
4038

4139
} });

0 commit comments

Comments
 (0)