Skip to content

Commit

Permalink
Prevent duplicate installations in winget upgrade all (#1652)
Browse files Browse the repository at this point in the history
  • Loading branch information
@yao-msft
yao-msft authored Nov 4, 2021
1 parent 902478c commit 134326d
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 14 deletions.
17 changes: 16 additions & 1 deletion src/AppInstallerCLICore/Workflows/UpdateFlow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,21 @@ namespace AppInstaller::CLI::Workflow
{
return (installedVersion < updateVersion || updateVersion.IsLatest());
}

void AddToPackagesToInstallIfNotPresent(std::vector<Execution::PackageToInstall>& packagesToInstall, Execution::PackageToInstall&& package)
{
for (auto const& existing : packagesToInstall)
{
if (existing.Manifest.Id == package.Manifest.Id &&
existing.Manifest.Version == package.Manifest.Version &&
existing.PackageVersion->GetProperty(PackageVersionProperty::SourceIdentifier) == package.PackageVersion->GetProperty(PackageVersionProperty::SourceIdentifier))
{
return;
}
}

packagesToInstall.emplace_back(std::move(package));
}
}

void SelectLatestApplicableUpdate::operator()(Execution::Context& context) const
Expand Down Expand Up @@ -119,7 +134,7 @@ namespace AppInstaller::CLI::Workflow
std::move(updateContext.Get<Execution::Data::Installer>().value()) };
package.PackageSubExecutionId = subExecution.GetCurrentSubExecutionId();

packagesToInstall.emplace_back(std::move(package));
AddToPackagesToInstallIfNotPresent(packagesToInstall, std::move(package));
}

if (!updateAllFoundUpdate)
Expand Down
87 changes: 74 additions & 13 deletions src/AppInstallerCLITests/WorkFlow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,41 @@ namespace
PackageMatchFilter(PackageMatchField::Id, MatchType::Exact, "TestInstallerWithLicenseAgreement")));
}

if (input == "TestUpgradeAllWithDuplicateUpgradeItems")
{
auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Exe.yaml"));
auto manifest2 = YamlParser::CreateFromPath(TestDataFile("UpdateFlowTest_Exe.yaml"));
auto manifest3 = YamlParser::CreateFromPath(TestDataFile(m_upgradeUsesLicenses ? "UpdateFlowTest_Exe_2_LicenseAgreement.yaml" : "UpdateFlowTest_Exe_2.yaml"));
result.Matches.emplace_back(
ResultMatch(
TestPackage::Make(
manifest,
TestPackage::MetadataMap
{
{ PackageVersionMetadata::InstalledType, "Exe" },
{ PackageVersionMetadata::StandardUninstallCommand, "C:\\uninstall.exe" },
{ PackageVersionMetadata::SilentUninstallCommand, "C:\\uninstall.exe /silence" },
},
std::vector<Manifest>{ manifest3, manifest2, manifest },
shared_from_this()
),
PackageMatchFilter(PackageMatchField::Id, MatchType::Exact, "AppInstallerCliTest.TestExeInstaller")));
result.Matches.emplace_back(
ResultMatch(
TestPackage::Make(
manifest2,
TestPackage::MetadataMap
{
{ PackageVersionMetadata::InstalledType, "Exe" },
{ PackageVersionMetadata::StandardUninstallCommand, "C:\\uninstall.exe" },
{ PackageVersionMetadata::SilentUninstallCommand, "C:\\uninstall.exe /silence" },
},
std::vector<Manifest>{ manifest3, manifest2, manifest },
shared_from_this()
),
PackageMatchFilter(PackageMatchField::Id, MatchType::Exact, "AppInstallerCliTest.TestExeInstaller")));
}

return result;
}

Expand All @@ -280,16 +315,18 @@ namespace

struct WorkflowTaskOverride
{
WorkflowTaskOverride(WorkflowTask::Func f, const std::function<void(TestContext&)>& o) :
Target(f), Override(o) {}
WorkflowTaskOverride(WorkflowTask::Func f, const std::function<void(TestContext&)>& o, int expectedUseCount = -1) :
Target(f), Override(o), ExpectedUseCount(expectedUseCount) {}

WorkflowTaskOverride(std::string_view n, const std::function<void(TestContext&)>& o) :
Target(n), Override(o) {}
WorkflowTaskOverride(std::string_view n, const std::function<void(TestContext&)>& o, int expectedUseCount = -1) :
Target(n), Override(o), ExpectedUseCount(expectedUseCount) {}

WorkflowTaskOverride(const WorkflowTask& t, const std::function<void(TestContext&)>& o) :
Target(t), Override(o) {}
WorkflowTaskOverride(const WorkflowTask& t, const std::function<void(TestContext&)>& o, int expectedUseCount = -1) :
Target(t), Override(o), ExpectedUseCount(expectedUseCount) {}

bool Used = false;
// -1 means no check on actual use count, as long as it's used.
int ExpectedUseCount = -1;
int UseCount = 0;
WorkflowTask Target;
std::function<void(TestContext&)> Override;
};
Expand All @@ -306,7 +343,7 @@ namespace
} };

// Mark this one as used so that it doesn't anger the destructor.
wto.Used = true;
wto.UseCount++;

Override(wto);
}
Expand All @@ -324,7 +361,7 @@ namespace
}
else
{
itr->Used = true;
itr->UseCount++;
itr->Override(*this);
return false;
}
Expand All @@ -337,10 +374,14 @@ namespace
{
for (const auto& wto : *m_overrides)
{
if (!wto.Used)
if (wto.UseCount == 0)
{
FAIL_CHECK("Unused override " + wto.Target.GetName());
}
else if (wto.ExpectedUseCount != -1 && wto.ExpectedUseCount != wto.UseCount)
{
FAIL_CHECK("Used override count does not match expected " + wto.Target.GetName());
}
}
}
}
Expand Down Expand Up @@ -434,19 +475,19 @@ void OverrideForCheckExistingInstaller(TestContext& context)
} });
}

void OverrideForShellExecute(TestContext& context)
void OverrideForShellExecute(TestContext& context, int expectedUseCount = -1)
{
OverrideForCheckExistingInstaller(context);

context.Override({ DownloadInstallerFile, [](TestContext& context)
{
context.Add<Data::HashPair>({ {}, {} });
context.Add<Data::InstallerPath>(TestDataFile("AppInstallerTestExeInstaller.exe"));
} });
}, expectedUseCount });

context.Override({ RenameDownloadedInstaller, [](TestContext&)
{
} });
}, expectedUseCount });

OverrideForUpdateInstallerMotw(context);
}
Expand Down Expand Up @@ -1461,6 +1502,26 @@ TEST_CASE("UpdateFlow_UpdateAllApplicable", "[UpdateFlow][workflow]")
REQUIRE(std::filesystem::exists(updateMSStoreResultPath.GetPath()));
}

TEST_CASE("UpdateFlow_UpgradeWithDuplicateUpgradeItemsFound", "[UpdateFlow][workflow]")
{
TestCommon::TempFile updateExeResultPath("TestExeInstalled.txt");

std::ostringstream updateOutput;
TestContext context{ updateOutput, std::cin };
OverrideForCompositeInstalledSource(context);
// Installer should only be run once since the 2 upgrade items are same.
OverrideForShellExecute(context, 1);
context.Args.AddArg(Execution::Args::Type::Query, "TestUpgradeAllWithDuplicateUpgradeItems"sv);
context.Args.AddArg(Execution::Args::Type::All);

UpgradeCommand update({});
update.Execute(context);
INFO(updateOutput.str());

// Verify installers are called.
REQUIRE(std::filesystem::exists(updateExeResultPath.GetPath()));
}

TEST_CASE("UpdateFlow_Dependencies", "[UpdateFlow][workflow][dependencies]")
{
TestCommon::TempFile updateResultPath("TestExeInstalled.txt");
Expand Down

0 comments on commit 134326d

Please sign in to comment.