Use a local UpdateFirstSeenAt timestamp instead of UpdatePublishedOn
The existing implementation that used the UpdatePublishedOn doesn't work
well with other AU enterprise policies such as stop AU, target version,
etc. This change will help all those scenarios to work and will make
the co-existence of policies more intuitive for the enterprise admin.
Basically, these scenarios bring out a flaw in the assumption I had
made earlier. i.e. we had assumed that if an update was pushed 5 months
ago, we never have to scatter that because most machines would have
been picked up the update by then. With the other AU policies like
stop AU or version pinning, this assumption is not true and scattering
is still relevant in these cases.
This new UpdateFirstSeenAt timestamp is the first time a Chrome device
hears of an update from Omaha that's eligble to be applied to the device
based on all policies except scattering. It'll use this timestamp instead
of the UpdatePublishedOn from the rule (which is no longer needed) and
everything else remains the same. This UpdateFirstSeenAt value will
also be persisted so that the waiting period can be satisfied over reboots
when an update is not ready to be applied yet.
This timestamp will be cleared (only) after an update has been successfully
applied and the device is waiting to be rebooted.
Also contains a minor fix for avoiding the crash mentioned in 32797.
BUG=chromium-os:32280
TEST=Added new unit tests, tested on ZGB.
Change-Id: I1d7845a11f7eb7fc0a019018c8c4b9a3c68ee2cd
Reviewed-on: https://gerrit.chromium.org/gerrit/28100
Commit-Ready: Jay Srinivasan <jaysri@chromium.org>
Reviewed-by: Jay Srinivasan <jaysri@chromium.org>
Tested-by: Jay Srinivasan <jaysri@chromium.org>
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index d8bef5e..a336bdd 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -27,6 +27,7 @@
using std::vector;
using testing::_;
using testing::AllOf;
+using testing::DoAll;
using testing::Ge;
using testing::Le;
using testing::NiceMock;
@@ -71,7 +72,6 @@
const string& needsadmin,
const string& size,
const string& deadline,
- Time published_on,
const string& max_days_to_scatter) {
return string(
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><gupdate "
@@ -82,7 +82,6 @@
"ChromeOSVersion=\"" + display_version + "\" "
"MoreInfo=\"" + more_info_url + "\" Prompt=\"" + prompt + "\" "
"IsDelta=\"true\" "
- "UpdatePublishedOn=\"" + utils::ToString(published_on) + "\" "
"MaxDaysToScatter=\"" + max_days_to_scatter + "\" "
"codebase=\"" + codebase + "\" "
"hash=\"not-applicable\" "
@@ -111,7 +110,6 @@
needsadmin,
size,
deadline,
- Time::Now(),
"7");
}
@@ -291,6 +289,7 @@
&response,
NULL));
EXPECT_TRUE(response.update_exists);
+ EXPECT_TRUE(response.update_exists);
EXPECT_EQ("1.2.3.4", response.display_version);
EXPECT_EQ("http://code/base", response.codebase);
EXPECT_EQ("http://more/info", response.more_info_url);
@@ -369,7 +368,6 @@
"false", // needs admin
"123", // size
"", // deadline
- Time::Now(), // published on
"7"), // max days to scatter
-1,
false, // ping_only
@@ -410,7 +408,6 @@
"false", // needs admin
"123", // size
"", // deadline
- Time::Now(), // published on
"7"), // max days to scatter
-1,
false, // ping_only
@@ -451,7 +448,6 @@
"false", // needs admin
"123", // size
"", // deadline
- Time::Now(), // published on
"0"), // max days to scatter
-1,
false, // ping_only
@@ -493,7 +489,6 @@
"false", // needs admin
"123", // size
"", // deadline
- Time::Now(), // published on
"7"), // max days to scatter
-1,
false, // ping_only
@@ -538,7 +533,6 @@
"false", // needs admin
"123", // size
"", // deadline
- Time::Now(), // published on
"7"), // max days to scatter
-1,
false, // ping_only
@@ -585,7 +579,6 @@
"false", // needs admin
"123", // size
"", // deadline
- Time::Now(), // published on
"7"), // max days to scatter
-1,
false, // ping_only
@@ -1323,4 +1316,93 @@
EXPECT_FALSE(response.update_exists);
}
+TEST(OmahaRequestActionTest, TestUpdateFirstSeenAtGetsPersistedFirstTime) {
+ OmahaResponse response;
+ OmahaRequestParams params = kDefaultTestParams;
+ params.wall_clock_based_wait_enabled = true;
+ params.waiting_period = TimeDelta().FromDays(1);
+ params.update_check_count_wait_enabled = false;
+
+ string prefs_dir;
+ EXPECT_TRUE(utils::MakeTempDirectory("/tmp/ue_ut_prefs.XXXXXX",
+ &prefs_dir));
+ ScopedDirRemover temp_dir_remover(prefs_dir);
+
+ Prefs prefs;
+ LOG_IF(ERROR, !prefs.Init(FilePath(prefs_dir)))
+ << "Failed to initialize preferences.";
+
+ ASSERT_FALSE(TestUpdateCheck(
+ &prefs, // prefs
+ params,
+ GetUpdateResponse2(OmahaRequestParams::kAppId,
+ "1.2.3.4", // version
+ "http://more/info",
+ "true", // prompt
+ "http://code/base", // dl url
+ "HASH1234=", // checksum
+ "false", // needs admin
+ "123", // size
+ "", // deadline
+ "7"), // max days to scatter
+ -1,
+ false, // ping_only
+ kActionCodeOmahaUpdateDeferredPerPolicy,
+ &response,
+ NULL));
+
+ int64 timestamp = 0;
+ ASSERT_TRUE(prefs.GetInt64(kPrefsUpdateFirstSeenAt, ×tamp));
+ ASSERT_TRUE(timestamp > 0);
+ EXPECT_FALSE(response.update_exists);
+}
+
+TEST(OmahaRequestActionTest, TestUpdateFirstSeenAtGetsUsedIfAlreadyPresent) {
+ OmahaResponse response;
+ OmahaRequestParams params = kDefaultTestParams;
+ params.wall_clock_based_wait_enabled = true;
+ params.waiting_period = TimeDelta().FromDays(1);
+ params.update_check_count_wait_enabled = false;
+
+ string prefs_dir;
+ EXPECT_TRUE(utils::MakeTempDirectory("/tmp/ue_ut_prefs.XXXXXX",
+ &prefs_dir));
+ ScopedDirRemover temp_dir_remover(prefs_dir);
+
+ Prefs prefs;
+ LOG_IF(ERROR, !prefs.Init(FilePath(prefs_dir)))
+ << "Failed to initialize preferences.";
+
+ // Set the timestamp to a very old value such that it exceeds the
+ // waiting period set above.
+ Time t1;
+ Time::FromString("1/1/2012", &t1);
+ ASSERT_TRUE(prefs.SetInt64(kPrefsUpdateFirstSeenAt, t1.ToInternalValue()));
+ ASSERT_TRUE(TestUpdateCheck(
+ &prefs, // prefs
+ params,
+ GetUpdateResponse2(OmahaRequestParams::kAppId,
+ "1.2.3.4", // version
+ "http://more/info",
+ "true", // prompt
+ "http://code/base", // dl url
+ "HASH1234=", // checksum
+ "false", // needs admin
+ "123", // size
+ "", // deadline
+ "7"), // max days to scatter
+ -1,
+ false, // ping_only
+ kActionCodeSuccess,
+ &response,
+ NULL));
+
+ EXPECT_TRUE(response.update_exists);
+
+ // Make sure the timestamp t1 is unchanged showing that it was reused.
+ int64 timestamp = 0;
+ ASSERT_TRUE(prefs.GetInt64(kPrefsUpdateFirstSeenAt, ×tamp));
+ ASSERT_TRUE(timestamp == t1.ToInternalValue());
+}
+
} // namespace chromeos_update_engine