UpdateManager: Move logic from UpdateCanStart to UpdateCheckAllowed.
The following should be part of the policy that is consulted before
performing an update check:
1) Whether updates are disabled by device policy.
2) Whether a specific target channel is dictated by the device policy.
This CL moves it from UpdateCanStart into UpdateCheckAllowed.
Another change is renaming the output construct of UpdateCanStart into
'UpdateDownloadParams'; this is in line with the naming of the output
struct of UpdateCheckAllowed, and reflects the fact that it contains
information regarding the download of an update, to be used by the
caller.
BUG=chromium:388386
TEST=Unit tests.
Change-Id: I0631a4464800db77807d7da9a2a2c256b519c5c3
Reviewed-on: https://chromium-review.googlesource.com/205728
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
diff --git a/update_manager/chromeos_policy_unittest.cc b/update_manager/chromeos_policy_unittest.cc
index 0d19720..eef2250 100644
--- a/update_manager/chromeos_policy_unittest.cc
+++ b/update_manager/chromeos_policy_unittest.cc
@@ -205,6 +205,38 @@
&Policy::UpdateCheckAllowed, &result);
}
+TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedWithAttributes) {
+ // Update check is allowed, reponse includes attributes for use in the
+ // request.
+ SetUpdateCheckAllowed(true);
+
+ // Override specific device policy attributes.
+ fake_state_.device_policy_provider()->var_release_channel_delegated()->
+ reset(new bool(false));
+ fake_state_.device_policy_provider()->var_release_channel()->
+ reset(new string("foo-channel"));
+
+ UpdateCheckParams result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCheckAllowed, &result);
+ EXPECT_TRUE(result.updates_enabled);
+ EXPECT_EQ("foo-channel", result.target_channel);
+}
+
+TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedUpdatesDisabled) {
+ // UpdateCheckAllowed should return kAskMeAgainLater because a device policy
+ // is loaded and prohibits updates.
+
+ SetUpdateCheckAllowed(false);
+ fake_state_.device_policy_provider()->var_update_disabled()->reset(
+ new bool(true));
+
+ // Check that the UpdateCanStart returns false.
+ UpdateCheckParams result;
+ ExpectPolicyStatus(EvalStatus::kAskMeAgainLater,
+ &Policy::UpdateCheckAllowed, &result);
+}
+
TEST_F(UmChromeOSPolicyTest, UpdateCanStartFailsCheckAllowedError) {
// The UpdateCanStart policy fails, not being able to query
// UpdateCheckAllowed.
@@ -214,7 +246,7 @@
// Check that the UpdateCanStart fails.
UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromMinutes(10));
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kFailed,
&Policy::UpdateCanStart, &result, false, update_state);
}
@@ -227,7 +259,7 @@
// Check that the UpdateCanStart returns false.
UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromMinutes(10));
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded,
&Policy::UpdateCanStart, &result, false, update_state);
EXPECT_FALSE(result.update_can_start);
@@ -243,12 +275,11 @@
// Check that the UpdateCanStart returns true with no further attributes.
UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromMinutes(10));
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded,
&Policy::UpdateCanStart, &result, false, update_state);
EXPECT_TRUE(result.update_can_start);
EXPECT_FALSE(result.p2p_allowed);
- EXPECT_TRUE(result.target_channel.empty());
EXPECT_EQ(0, result.download_url_idx);
EXPECT_EQ(0, result.download_url_num_failures);
}
@@ -261,34 +292,15 @@
// Check that the UpdateCanStart returns true.
UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromMinutes(10));
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded,
&Policy::UpdateCanStart, &result, false, update_state);
EXPECT_TRUE(result.update_can_start);
EXPECT_FALSE(result.p2p_allowed);
- EXPECT_TRUE(result.target_channel.empty());
EXPECT_EQ(0, result.download_url_idx);
EXPECT_EQ(0, result.download_url_num_failures);
}
-TEST_F(UmChromeOSPolicyTest, UpdateCanStartNotAllowedUpdatesDisabled) {
- // The UpdateCanStart should return false (kAskMeAgainlater) because a device
- // policy is loaded and prohibits updates.
-
- SetUpdateCheckAllowed(false);
- fake_state_.device_policy_provider()->var_update_disabled()->reset(
- new bool(true));
-
- // Check that the UpdateCanStart returns false.
- UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromMinutes(10));
- UpdateCanStartResult result;
- ExpectPolicyStatus(EvalStatus::kAskMeAgainLater,
- &Policy::UpdateCanStart, &result, false, update_state);
- EXPECT_FALSE(result.update_can_start);
- EXPECT_EQ(UpdateCannotStartReason::kDisabledByPolicy,
- result.cannot_start_reason);
-}
-
TEST_F(UmChromeOSPolicyTest, UpdateCanStartFailsScatteringFailed) {
// The UpdateCanStart policy fails because the UpdateScattering policy it
// depends on fails (unset variable).
@@ -301,7 +313,7 @@
// Check that the UpdateCanStart fails.
UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromSeconds(1));
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kFailed,
&Policy::UpdateCanStart, &result, false, update_state);
}
@@ -321,7 +333,7 @@
// Check that the UpdateCanStart returns false and a new wait period
// generated.
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
false, update_state);
EXPECT_FALSE(result.update_can_start);
@@ -345,7 +357,7 @@
// Check that the UpdateCanStart returns false and a new wait period
// generated.
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kAskMeAgainLater, &Policy::UpdateCanStart,
&result, false, update_state);
EXPECT_FALSE(result.update_can_start);
@@ -371,7 +383,7 @@
update_state.scatter_check_threshold_max = 5;
// Check that the UpdateCanStart returns false.
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
false, update_state);
EXPECT_FALSE(result.update_can_start);
@@ -395,7 +407,7 @@
update_state.scatter_check_threshold_max = 5;
// Check that the UpdateCanStart returns false.
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
false, update_state);
EXPECT_FALSE(result.update_can_start);
@@ -420,7 +432,7 @@
update_state.scatter_check_threshold_max = 5;
// Check that the UpdateCanStart returns true.
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
false, update_state);
EXPECT_TRUE(result.update_can_start);
@@ -446,7 +458,7 @@
update_state.scatter_check_threshold_max = 5;
// Check that the UpdateCanStart returns true.
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
true, update_state);
EXPECT_TRUE(result.update_can_start);
@@ -473,7 +485,7 @@
update_state.scatter_check_threshold_max = 5;
// Check that the UpdateCanStart returns true.
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
true, update_state);
EXPECT_TRUE(result.update_can_start);
@@ -494,19 +506,14 @@
new bool(true));
fake_state_.device_policy_provider()->var_au_p2p_enabled()->reset(
new bool(true));
- fake_state_.device_policy_provider()->var_release_channel_delegated()->
- reset(new bool(false));
- fake_state_.device_policy_provider()->var_release_channel()->
- reset(new string("foo-channel"));
// Check that the UpdateCanStart returns true.
UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromMinutes(10));
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
false, update_state);
EXPECT_TRUE(result.update_can_start);
EXPECT_TRUE(result.p2p_allowed);
- EXPECT_EQ("foo-channel", result.target_channel);
EXPECT_EQ(0, result.download_url_idx);
EXPECT_EQ(0, result.download_url_num_failures);
}
@@ -523,7 +530,7 @@
// Check that the UpdateCanStart returns true.
UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromMinutes(10));
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
false, update_state);
EXPECT_TRUE(result.update_can_start);
@@ -548,7 +555,7 @@
// Check that the UpdateCanStart returns true.
UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromMinutes(10));
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
false, update_state);
EXPECT_TRUE(result.update_can_start);
@@ -572,7 +579,7 @@
update_state.download_urls.push_back("https://secure/url/");
// Check that the UpdateCanStart returns true.
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
false, update_state);
EXPECT_TRUE(result.update_can_start);
@@ -601,7 +608,7 @@
ErrorCode::kDownloadWriteError);
// Check that the UpdateCanStart returns true.
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
false, update_state);
EXPECT_TRUE(result.update_can_start);
@@ -625,7 +632,7 @@
ErrorCode::kPayloadHashMismatchError);
// Check that the UpdateCanStart returns true.
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
false, update_state);
EXPECT_TRUE(result.update_can_start);
@@ -650,7 +657,7 @@
ErrorCode::kPayloadHashMismatchError);
// Check that the UpdateCanStart returns true.
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
false, update_state);
EXPECT_TRUE(result.update_can_start);
@@ -671,7 +678,7 @@
// Check that the UpdateCanStart returns false.
UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromMinutes(10));
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kFailed, &Policy::UpdateCanStart, &result,
false, update_state);
}
@@ -691,7 +698,7 @@
// Check that the UpdateCanStart returns true.
UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromMinutes(10));
- UpdateCanStartResult result;
+ UpdateDownloadParams result;
ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
false, update_state);
EXPECT_TRUE(result.update_can_start);