update_engine: Make policy UpdateCheckParams processing easier
Currently there are a ton of arguments from UpdateCheckParams that is
passed around in the udpate_attampter.cc. With this CL UpdateCheckParams
is directy passed and this simplies the logic.
BUG=b:171829801
TEST=cros_workon_make --board reef --test update_enigne
Change-Id: If454f6393fc6e28d41fa5d14d184f0db32e8bd19
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2504453
Commit-Queue: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Jae Hoon Kim <kimjae@chromium.org>
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 60267f0..767bb82 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -172,30 +172,10 @@
explicit UpdateAttempterUnderTest(SystemState* system_state)
: UpdateAttempter(system_state, nullptr) {}
- void Update(const std::string& app_version,
- const std::string& omaha_url,
- const std::string& target_channel,
- const std::string& lts_tag,
- const std::string& target_version_prefix,
- bool rollback_allowed,
- bool rollback_data_save_requested,
- int rollback_allowed_milestones,
- bool rollback_on_channel_downgrade,
- bool obey_proxies,
- bool interactive) override {
+ void Update(const UpdateCheckParams& params) override {
update_called_ = true;
if (do_update_) {
- UpdateAttempter::Update(app_version,
- omaha_url,
- target_channel,
- lts_tag,
- target_version_prefix,
- rollback_allowed,
- rollback_data_save_requested,
- rollback_allowed_milestones,
- rollback_on_channel_downgrade,
- obey_proxies,
- interactive);
+ UpdateAttempter::Update(params);
return;
}
LOG(INFO) << "[TEST] Update() disabled.";
@@ -430,7 +410,7 @@
void UpdateAttempterTest::SessionIdTestChange() {
EXPECT_NE(UpdateStatus::UPDATED_NEED_REBOOT, attempter_.status());
const auto old_session_id = attempter_.session_id_;
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_NE(old_session_id, attempter_.session_id_);
ScheduleQuitMainLoop();
}
@@ -801,7 +781,7 @@
EXPECT_CALL(*processor_, StartProcessing());
}
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
loop_.PostTask(FROM_HERE,
base::Bind(&UpdateAttempterTest::UpdateTestVerify,
base::Unretained(this)));
@@ -1001,7 +981,7 @@
fake_system_state_.set_p2p_manager(&mock_p2p_manager);
mock_p2p_manager.fake().SetP2PEnabled(false);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(0);
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_FALSE(actual_using_p2p_for_downloading_);
EXPECT_FALSE(actual_using_p2p_for_sharing());
ScheduleQuitMainLoop();
@@ -1023,7 +1003,7 @@
mock_p2p_manager.fake().SetEnsureP2PRunningResult(false);
mock_p2p_manager.fake().SetPerformHousekeepingResult(false);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(0);
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_FALSE(actual_using_p2p_for_downloading());
EXPECT_FALSE(actual_using_p2p_for_sharing());
ScheduleQuitMainLoop();
@@ -1046,7 +1026,7 @@
mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
mock_p2p_manager.fake().SetPerformHousekeepingResult(false);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping());
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_FALSE(actual_using_p2p_for_downloading());
EXPECT_FALSE(actual_using_p2p_for_sharing());
ScheduleQuitMainLoop();
@@ -1068,7 +1048,7 @@
mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
mock_p2p_manager.fake().SetPerformHousekeepingResult(true);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping());
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_TRUE(actual_using_p2p_for_downloading());
EXPECT_TRUE(actual_using_p2p_for_sharing());
ScheduleQuitMainLoop();
@@ -1091,17 +1071,7 @@
mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
mock_p2p_manager.fake().SetPerformHousekeepingResult(true);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping());
- attempter_.Update("",
- "",
- "",
- "",
- "",
- false,
- false,
- /*rollback_allowed_milestones=*/0,
- false,
- false,
- /*interactive=*/true);
+ attempter_.Update({.interactive = true});
EXPECT_FALSE(actual_using_p2p_for_downloading());
EXPECT_TRUE(actual_using_p2p_for_sharing());
ScheduleQuitMainLoop();
@@ -1131,7 +1101,7 @@
attempter_.policy_provider_.reset(
new policy::PolicyProvider(std::move(device_policy)));
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_EQ(scatter_factor_in_seconds, attempter_.scatter_factor_.InSeconds());
ScheduleQuitMainLoop();
@@ -1169,7 +1139,7 @@
attempter_.policy_provider_.reset(
new policy::PolicyProvider(std::move(device_policy)));
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_EQ(scatter_factor_in_seconds, attempter_.scatter_factor_.InSeconds());
// Make sure the file still exists.
@@ -1185,7 +1155,7 @@
// However, if the count is already 0, it's not decremented. Test that.
initial_value = 0;
EXPECT_TRUE(fake_prefs.SetInt64(kPrefsUpdateCheckCount, initial_value));
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_TRUE(fake_prefs.Exists(kPrefsUpdateCheckCount));
EXPECT_TRUE(fake_prefs.GetInt64(kPrefsUpdateCheckCount, &new_value));
EXPECT_EQ(initial_value, new_value);
@@ -1232,17 +1202,7 @@
new policy::PolicyProvider(std::move(device_policy)));
// Trigger an interactive check so we can test that scattering is disabled.
- attempter_.Update("",
- "",
- "",
- "",
- "",
- false,
- false,
- /*rollback_allowed_milestones=*/0,
- false,
- false,
- /*interactive=*/true);
+ attempter_.Update({.interactive = true});
EXPECT_EQ(scatter_factor_in_seconds, attempter_.scatter_factor_.InSeconds());
// Make sure scattering is disabled for manual (i.e. user initiated) update
@@ -1294,7 +1254,7 @@
FakePrefs fake_prefs;
SetUpStagingTest(kValidStagingSchedule, &fake_prefs);
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
// Check that prefs have the correct values.
int64_t update_count;
EXPECT_TRUE(fake_prefs.GetInt64(kPrefsUpdateCheckCount, &update_count));
@@ -1351,17 +1311,7 @@
FakePrefs fake_prefs;
SetUpStagingTest(kValidStagingSchedule, &fake_prefs);
- attempter_.Update("",
- "",
- "",
- "",
- "",
- false,
- false,
- 0,
- false,
- false,
- /* interactive = */ true);
+ attempter_.Update({.interactive = true});
CheckStagingOff();
ScheduleQuitMainLoop();
@@ -1381,17 +1331,7 @@
FakePrefs fake_prefs;
SetUpStagingTest(kValidStagingSchedule, &fake_prefs);
- attempter_.Update("",
- "",
- "",
- "",
- "",
- false,
- false,
- 0,
- false,
- false,
- /* interactive = */ true);
+ attempter_.Update({.interactive = true});
CheckStagingOff();
ScheduleQuitMainLoop();
@@ -1719,64 +1659,38 @@
}
TEST_F(UpdateAttempterTest, TargetVersionPrefixSetAndReset) {
- attempter_.CalculateUpdateParams(
- /*app_version=*/"",
- /*omaha_url=*/"",
- /*target_channel=*/"",
- /*lts_tag=*/"",
- /*target_version_prefix=*/"1234",
- /*rollback_allowed=*/false,
- /*rollback_data_save_requested=*/false,
- /*rollback_allowed_milestones=*/4,
- /*rollback_on_channel_downgrade=*/false,
- /*obey_proxies=*/false,
- /*interactive=*/false);
+ UpdateCheckParams params;
+ attempter_.CalculateUpdateParams({.target_version_prefix = "1234"});
EXPECT_EQ("1234",
fake_system_state_.request_params()->target_version_prefix());
- attempter_.CalculateUpdateParams(
- "", "", "", "", "", false, false, 4, false, false, false);
+ attempter_.CalculateUpdateParams({});
EXPECT_TRUE(
fake_system_state_.request_params()->target_version_prefix().empty());
}
TEST_F(UpdateAttempterTest, TargetChannelHintSetAndReset) {
- attempter_.CalculateUpdateParams(
- "", "", "", "hint", "", false, false, 4, false, false, false);
+ attempter_.CalculateUpdateParams({.lts_tag = "hint"});
EXPECT_EQ("hint", fake_system_state_.request_params()->lts_tag());
- attempter_.CalculateUpdateParams(
- "", "", "", "", "", false, false, 4, false, false, false);
+ attempter_.CalculateUpdateParams({});
EXPECT_TRUE(fake_system_state_.request_params()->lts_tag().empty());
}
TEST_F(UpdateAttempterTest, RollbackAllowedSetAndReset) {
- attempter_.CalculateUpdateParams("",
- "",
- "",
- "",
- "1234",
- /*rollback_allowed=*/true,
- /*rollback_data_save_requested=*/false,
- /*rollback_allowed_milestones=*/4,
- /*rollback_on_channel_downgrade=*/false,
- false,
- false);
+ attempter_.CalculateUpdateParams({
+ .target_version_prefix = "1234",
+ .rollback_allowed = true,
+ .rollback_allowed_milestones = 4,
+ });
EXPECT_TRUE(fake_system_state_.request_params()->rollback_allowed());
EXPECT_EQ(4,
fake_system_state_.request_params()->rollback_allowed_milestones());
- attempter_.CalculateUpdateParams("",
- "",
- "",
- "",
- "1234",
- /*rollback_allowed=*/false,
- /*rollback_data_save_requested=*/false,
- /*rollback_allowed_milestones=*/4,
- /*rollback_on_channel_downgrade=*/false,
- false,
- false);
+ attempter_.CalculateUpdateParams({
+ .target_version_prefix = "1234",
+ .rollback_allowed_milestones = 4,
+ });
EXPECT_FALSE(fake_system_state_.request_params()->rollback_allowed());
EXPECT_EQ(4,
fake_system_state_.request_params()->rollback_allowed_milestones());
@@ -1786,17 +1700,9 @@
base::ScopedTempDir tempdir;
ASSERT_TRUE(tempdir.CreateUniqueTempDir());
fake_system_state_.request_params()->set_root(tempdir.GetPath().value());
- attempter_.CalculateUpdateParams(/*app_version=*/"",
- /*omaha_url=*/"",
- /*target_channel=*/"stable-channel",
- /*lts_tag=*/"",
- /*target_version_prefix=*/"",
- /*rollback_allowed=*/false,
- /*rollback_data_save_requested=*/false,
- /*rollback_allowed_milestones=*/4,
- /*rollback_on_channel_downgrade=*/false,
- /*obey_proxies=*/false,
- /*interactive=*/false);
+ attempter_.CalculateUpdateParams({
+ .target_channel = "stable-channel",
+ });
EXPECT_FALSE(fake_system_state_.request_params()->is_powerwash_allowed());
}
@@ -1804,17 +1710,10 @@
base::ScopedTempDir tempdir;
ASSERT_TRUE(tempdir.CreateUniqueTempDir());
fake_system_state_.request_params()->set_root(tempdir.GetPath().value());
- attempter_.CalculateUpdateParams(/*app_version=*/"",
- /*omaha_url=*/"",
- /*target_channel=*/"stable-channel",
- /*lts_tag=*/"",
- /*target_version_prefix=*/"",
- /*rollback_allowed=*/false,
- /*rollback_data_save_requested=*/false,
- /*rollback_allowed_milestones=*/4,
- /*rollback_on_channel_downgrade=*/true,
- /*obey_proxies=*/false,
- /*interactive=*/false);
+ attempter_.CalculateUpdateParams({
+ .rollback_on_channel_downgrade = true,
+ .target_channel = "stable-channel",
+ });
EXPECT_TRUE(fake_system_state_.request_params()->is_powerwash_allowed());
}
@@ -1932,7 +1831,7 @@
SetRollbackHappened(false))
.Times(expected_reset ? 1 : 0);
attempter_.policy_provider_ = std::move(mock_policy_provider);
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
ScheduleQuitMainLoop();
}
@@ -2273,7 +2172,7 @@
.WillOnce(Return(false));
attempter_.policy_provider_.reset(
new policy::PolicyProvider(std::move(device_policy)));
- attempter_.Update("", "", "", "", "", false, false, 0, false, false, false);
+ attempter_.Update({});
EXPECT_EQ(token, attempter_.omaha_request_params_->autoupdate_token());
ScheduleQuitMainLoop();