Don't send machine and user ID to Omaha anymore. Send a/r pings instead.
This avoids sending a unique ID in order to track active user counts.
Note that this CL doesn't remove the machine/user/Omaha ID/file from
the params object -- it just makes them unused/obsolete. Removal will
be done in a subsequent CL in an effort to make this CL smaller.
BUG=1439
TEST=unit tests, x86-generic, arm-generic, gmerged and inspected logs
Review URL: http://codereview.chromium.org/2856070
diff --git a/SConstruct b/SConstruct
index cf51f47..a24586b 100644
--- a/SConstruct
+++ b/SConstruct
@@ -294,7 +294,7 @@
http_server_cmd = env.Program('test_http_server', 'test_http_server.cc')
unittest_env = env.Clone()
-unittest_env.Append(LIBS=['gtest'])
+unittest_env.Append(LIBS=['gmock', 'gtest'])
unittest_cmd = unittest_env.Program('update_engine_unittests',
unittest_sources + unittest_main)
Clean(unittest_cmd, Glob('*.gcda') + Glob('*.gcno') + Glob('*.gcov') +
diff --git a/main.cc b/main.cc
index 204c525..bdd216c 100644
--- a/main.cc
+++ b/main.cc
@@ -14,6 +14,7 @@
#include "metrics/metrics_library.h"
#include "update_engine/dbus_constants.h"
#include "update_engine/dbus_service.h"
+#include "update_engine/prefs.h"
#include "update_engine/update_attempter.h"
extern "C" {
@@ -105,11 +106,16 @@
// Create the single GMainLoop
GMainLoop* loop = g_main_loop_new(g_main_context_default(), FALSE);
+ chromeos_update_engine::Prefs prefs;
+ LOG_IF(ERROR, !prefs.Init(FilePath("/var/lib/update_engine/prefs")))
+ << "Failed to initialize preferences.";
+
MetricsLibrary metrics_lib;
metrics_lib.Init();
// Create the update attempter:
- chromeos_update_engine::UpdateAttempter update_attempter(&metrics_lib);
+ chromeos_update_engine::UpdateAttempter update_attempter(&prefs,
+ &metrics_lib);
// Create the dbus service object:
dbus_g_object_type_install_info(UPDATE_ENGINE_TYPE_SERVICE,
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index cca2094..7eca9e0 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -11,11 +11,15 @@
#include <libxml/xpathInternals.h>
#include "base/string_util.h"
+#include "base/time.h"
#include "chromeos/obsolete_logging.h"
#include "update_engine/action_pipe.h"
#include "update_engine/omaha_request_params.h"
+#include "update_engine/prefs_interface.h"
#include "update_engine/utils.h"
+using base::Time;
+using base::TimeDelta;
using std::string;
namespace chromeos_update_engine {
@@ -54,13 +58,43 @@
}
};
+// Returns true if |ping_days| has a value that needs to be sent,
+// false otherwise.
+bool ShouldPing(int ping_days) {
+ return ping_days > 0 || ping_days == OmahaRequestAction::kNeverPinged;
+}
+
+// Returns an XML ping element attribute assignment with attribute
+// |name| and value |ping_days| if |ping_days| has a value that needs
+// to be sent, or an empty string otherwise.
+string GetPingAttribute(const string& name, int ping_days) {
+ if (ShouldPing(ping_days)) {
+ return StringPrintf(" %s=\"%d\"", name.c_str(), ping_days);
+ }
+ return "";
+}
+
+// Returns an XML ping element if any of the elapsed days need to be
+// sent, or an empty string otherwise.
+string GetPingBody(int ping_active_days, int ping_roll_call_days) {
+ string ping_active = GetPingAttribute("a", ping_active_days);
+ string ping_roll_call = GetPingAttribute("r", ping_roll_call_days);
+ if (!ping_active.empty() || !ping_roll_call.empty()) {
+ return StringPrintf(" <o:ping%s%s></o:ping>\n",
+ ping_active.c_str(),
+ ping_roll_call.c_str());
+ }
+ return "";
+}
+
string FormatRequest(const OmahaEvent* event,
- const OmahaRequestParams& params) {
+ const OmahaRequestParams& params,
+ int ping_active_days,
+ int ping_roll_call_days) {
string body;
if (event == NULL) {
- body = string(
- " <o:ping active=\"0\"></o:ping>\n"
- " <o:updatecheck></o:updatecheck>\n");
+ body = GetPingBody(ping_active_days, ping_roll_call_days) +
+ " <o:updatecheck></o:updatecheck>\n";
} else {
// The error code is an optional attribute so append it only if
// the result is not success.
@@ -72,13 +106,11 @@
" <o:event eventtype=\"%d\" eventresult=\"%d\"%s></o:event>\n",
event->type, event->result, error_code.c_str());
}
- return string("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
- "<o:gupdate xmlns:o=\"http://www.google.com/update2/request\" "
- "version=\"" + XmlEncode(kGupdateVersion) + "\" "
- "updaterversion=\"" + XmlEncode(kGupdateVersion) + "\" "
- "protocol=\"2.0\" "
- "machineid=\"") + XmlEncode(params.machine_id) +
- "\" ismachine=\"1\" userid=\"" + XmlEncode(params.user_id) + "\">\n"
+ return "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+ "<o:gupdate xmlns:o=\"http://www.google.com/update2/request\" "
+ "version=\"" + XmlEncode(kGupdateVersion) + "\" "
+ "updaterversion=\"" + XmlEncode(kGupdateVersion) + "\" "
+ "protocol=\"2.0\" ismachine=\"1\">\n"
" <o:os version=\"" + XmlEncode(params.os_version) + "\" platform=\"" +
XmlEncode(params.os_platform) + "\" sp=\"" +
XmlEncode(params.os_sp) + "\"></o:os>\n"
@@ -91,6 +123,7 @@
" </o:app>\n"
"</o:gupdate>\n";
}
+
} // namespace {}
// Encodes XML entities in a given string with libxml2. input must be
@@ -109,18 +142,58 @@
return string(reinterpret_cast<const char *>(str.get()));
}
-OmahaRequestAction::OmahaRequestAction(const OmahaRequestParams& params,
+OmahaRequestAction::OmahaRequestAction(PrefsInterface* prefs,
+ const OmahaRequestParams& params,
OmahaEvent* event,
HttpFetcher* http_fetcher)
- : params_(params),
+ : prefs_(prefs),
+ params_(params),
event_(event),
- http_fetcher_(http_fetcher) {}
+ http_fetcher_(http_fetcher),
+ ping_active_days_(0),
+ ping_roll_call_days_(0) {}
OmahaRequestAction::~OmahaRequestAction() {}
+// Calculates the value to use for the ping days parameter.
+int OmahaRequestAction::CalculatePingDays(const string& key) {
+ int days = kNeverPinged;
+ int64_t last_ping = 0;
+ if (prefs_->GetInt64(key, &last_ping) && last_ping >= 0) {
+ days = (Time::Now() - Time::FromInternalValue(last_ping)).InDays();
+ if (days < 0) {
+ // If |days| is negative, then the system clock must have jumped
+ // back in time since the ping was sent. Mark the value so that
+ // it doesn't get sent to the server but we still update the
+ // last ping daystart preference. This way the next ping time
+ // will be correct, hopefully.
+ days = kPingTimeJump;
+ LOG(WARNING) <<
+ "System clock jumped back in time. Resetting ping daystarts.";
+ }
+ }
+ return days;
+}
+
+void OmahaRequestAction::InitPingDays() {
+ // We send pings only along with update checks, not with events.
+ if (IsEvent()) {
+ return;
+ }
+ // TODO(petkov): Figure a way to distinguish active use pings
+ // vs. roll call pings. Currently, the two pings are identical. A
+ // fix needs to change this code as well as UpdateLastPingDays.
+ ping_active_days_ = CalculatePingDays(kPrefsLastActivePingDay);
+ ping_roll_call_days_ = CalculatePingDays(kPrefsLastRollCallPingDay);
+}
+
void OmahaRequestAction::PerformAction() {
http_fetcher_->set_delegate(this);
- string request_post(FormatRequest(event_.get(), params_));
+ InitPingDays();
+ string request_post(FormatRequest(event_.get(),
+ params_,
+ ping_active_days_,
+ ping_roll_call_days_));
http_fetcher_->SetPostData(request_post.data(), request_post.size());
LOG(INFO) << "Posting an Omaha request to " << params_.update_url;
LOG(INFO) << "Request: " << request_post;
@@ -199,6 +272,39 @@
}
return ret;
}
+
+// Update the last ping day preferences based on the server daystart
+// response. Returns true on success, false otherwise.
+bool UpdateLastPingDays(xmlDoc* doc, PrefsInterface* prefs) {
+ static const char kNamespace[] = "x";
+ static const char kDaystartNodeXpath[] = "/x:gupdate/x:daystart";
+ static const char kNsUrl[] = "http://www.google.com/update2/response";
+
+ scoped_ptr_malloc<xmlXPathObject, ScopedPtrXmlXPathObjectFree>
+ xpath_nodeset(GetNodeSet(doc,
+ ConstXMLStr(kDaystartNodeXpath),
+ ConstXMLStr(kNamespace),
+ ConstXMLStr(kNsUrl)));
+ TEST_AND_RETURN_FALSE(xpath_nodeset.get());
+ xmlNodeSet* nodeset = xpath_nodeset->nodesetval;
+ TEST_AND_RETURN_FALSE(nodeset && nodeset->nodeNr >= 1);
+ xmlNode* daystart_node = nodeset->nodeTab[0];
+ TEST_AND_RETURN_FALSE(xmlHasProp(daystart_node,
+ ConstXMLStr("elapsed_seconds")));
+
+ int64_t elapsed_seconds = 0;
+ TEST_AND_RETURN_FALSE(StringToInt64(XmlGetProperty(daystart_node,
+ "elapsed_seconds"),
+ &elapsed_seconds));
+ TEST_AND_RETURN_FALSE(elapsed_seconds >= 0);
+
+ // Remember the local time that matches the server's last midnight
+ // time.
+ Time daystart = Time::Now() - TimeDelta::FromSeconds(elapsed_seconds);
+ prefs->SetInt64(kPrefsLastActivePingDay, daystart.ToInternalValue());
+ prefs->SetInt64(kPrefsLastRollCallPingDay, daystart.ToInternalValue());
+ return true;
+}
} // namespace {}
// If the transfer was successful, this uses libxml2 to parse the response
@@ -236,6 +342,16 @@
return;
}
+ // If a ping was sent, update the last ping day preferences based on
+ // the server daystart response.
+ if (ShouldPing(ping_active_days_) ||
+ ShouldPing(ping_roll_call_days_) ||
+ ping_active_days_ == kPingTimeJump ||
+ ping_roll_call_days_ == kPingTimeJump) {
+ LOG_IF(ERROR, !UpdateLastPingDays(doc.get(), prefs_))
+ << "Failed to update the last ping day preferences!";
+ }
+
static const char* kNamespace("x");
static const char* kUpdatecheckNodeXpath("/x:gupdate/x:app/x:updatecheck");
static const char* kNsUrl("http://www.google.com/update2/response");
diff --git a/omaha_request_action.h b/omaha_request_action.h
index b487a3b..c5f4d19 100644
--- a/omaha_request_action.h
+++ b/omaha_request_action.h
@@ -85,6 +85,7 @@
class NoneType;
class OmahaRequestAction;
struct OmahaRequestParams;
+class PrefsInterface;
template<>
class ActionTraits<OmahaRequestAction> {
@@ -99,6 +100,9 @@
class OmahaRequestAction : public Action<OmahaRequestAction>,
public HttpFetcherDelegate {
public:
+ static const int kNeverPinged = -1;
+ static const int kPingTimeJump = -2;
+
// The ctor takes in all the parameters that will be used for making
// the request to Omaha. For some of them we have constants that
// should be used.
@@ -113,7 +117,8 @@
// OmahaRequestAction(..., new OmahaEvent(...), new WhateverHttpFetcher);
// or
// OmahaRequestAction(..., NULL, new WhateverHttpFetcher);
- OmahaRequestAction(const OmahaRequestParams& params,
+ OmahaRequestAction(PrefsInterface* prefs,
+ const OmahaRequestParams& params,
OmahaEvent* event,
HttpFetcher* http_fetcher);
virtual ~OmahaRequestAction();
@@ -135,6 +140,18 @@
bool IsEvent() const { return event_.get() != NULL; }
private:
+ // If this is an update check request, initializes
+ // |ping_active_days_| and |ping_roll_call_days_| to values that may
+ // be sent as pings to Omaha.
+ void InitPingDays();
+
+ // Based on the perstitent preference store values, calculates the
+ // number of days since the last ping sent for |key|.
+ int CalculatePingDays(const std::string& key);
+
+ // Access to the preferences store.
+ PrefsInterface* prefs_;
+
// These are data that are passed in the request to the Omaha server.
const OmahaRequestParams& params_;
@@ -147,6 +164,12 @@
// Stores the response from the omaha server
std::vector<char> response_buffer_;
+ // Initialized by InitPingDays to values that may be sent to Omaha
+ // as part of a ping message. Note that only positive values and -1
+ // are sent to Omaha.
+ int ping_active_days_;
+ int ping_roll_call_days_;
+
DISALLOW_COPY_AND_ASSIGN(OmahaRequestAction);
};
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index a4ecb29..5ac0811 100755
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -8,22 +8,46 @@
#include <glib.h>
#include "base/string_util.h"
+#include "base/time.h"
#include "gtest/gtest.h"
#include "update_engine/action_pipe.h"
#include "update_engine/mock_http_fetcher.h"
#include "update_engine/omaha_hash_calculator.h"
#include "update_engine/omaha_request_action.h"
#include "update_engine/omaha_request_params.h"
+#include "update_engine/prefs_mock.h"
#include "update_engine/test_utils.h"
+using base::Time;
+using base::TimeDelta;
using std::string;
using std::vector;
+using testing::_;
+using testing::AllOf;
+using testing::Ge;
+using testing::Le;
+using testing::Return;
+using testing::SetArgumentPointee;
namespace chromeos_update_engine {
class OmahaRequestActionTest : public ::testing::Test { };
namespace {
+const OmahaRequestParams kDefaultTestParams(
+ "machine_id",
+ "user_id",
+ OmahaRequestParams::kOsPlatform,
+ OmahaRequestParams::kOsVersion,
+ "service_pack",
+ "x86-generic",
+ OmahaRequestParams::kAppId,
+ "0.1.0.0",
+ "en-US",
+ "unittest",
+ false, // delta okay
+ "http://url");
+
string GetNoUpdateResponse(const string& app_id) {
return string(
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><gupdate "
@@ -83,7 +107,6 @@
processor->StartProcessing();
return FALSE;
}
-
} // namespace {}
class OutputObjectCollectorAction;
@@ -120,11 +143,13 @@
OmahaResponse omaha_response_;
};
-// returns true iff an output response was obtained from the
-// OmahaRequestAction. out_response may be NULL.
-// out_post_data may be null; if non-null, the post-data received by the
-// mock HttpFetcher is returned.
-bool TestUpdateCheck(const OmahaRequestParams& params,
+// Returns true iff an output response was obtained from the
+// OmahaRequestAction. |prefs| may be NULL, in which case a local
+// PrefsMock is used. out_response may be NULL. out_post_data may be
+// null; if non-null, the post-data received by the mock HttpFetcher
+// is returned.
+bool TestUpdateCheck(PrefsInterface* prefs,
+ const OmahaRequestParams& params,
const string& http_response,
ActionExitCode expected_code,
OmahaResponse* out_response,
@@ -132,7 +157,11 @@
GMainLoop* loop = g_main_loop_new(g_main_context_default(), FALSE);
MockHttpFetcher* fetcher = new MockHttpFetcher(http_response.data(),
http_response.size());
- OmahaRequestAction action(params, NULL, fetcher);
+ PrefsMock local_prefs;
+ OmahaRequestAction action(prefs ? prefs : &local_prefs,
+ params,
+ NULL,
+ fetcher);
OmahaRequestActionTestProcessorDelegate delegate;
delegate.loop_ = loop;
delegate.expected_code_ = expected_code;
@@ -165,7 +194,8 @@
GMainLoop* loop = g_main_loop_new(g_main_context_default(), FALSE);
MockHttpFetcher* fetcher = new MockHttpFetcher(http_response.data(),
http_response.size());
- OmahaRequestAction action(params, event, fetcher);
+ PrefsMock prefs;
+ OmahaRequestAction action(&prefs, params, event, fetcher);
OmahaRequestActionTestProcessorDelegate delegate;
delegate.loop_ = loop;
ActionProcessor processor;
@@ -180,21 +210,10 @@
}
TEST(OmahaRequestActionTest, NoUpdateTest) {
- OmahaRequestParams params("", // machine_id
- "", // user_id
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "", // os_sp
- "x86-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest",
- false, // delta okay
- ""); // url
OmahaResponse response;
ASSERT_TRUE(
- TestUpdateCheck(params,
+ TestUpdateCheck(NULL, // prefs
+ kDefaultTestParams,
GetNoUpdateResponse(OmahaRequestParams::kAppId),
kActionCodeSuccess,
&response,
@@ -203,21 +222,10 @@
}
TEST(OmahaRequestActionTest, ValidUpdateTest) {
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "arm-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- ""); // url
OmahaResponse response;
ASSERT_TRUE(
- TestUpdateCheck(params,
+ TestUpdateCheck(NULL, // prefs
+ kDefaultTestParams,
GetUpdateResponse(OmahaRequestParams::kAppId,
"1.2.3.4", // version
"http://more/info",
@@ -241,23 +249,12 @@
}
TEST(OmahaRequestActionTest, NoOutputPipeTest) {
- OmahaRequestParams params("", // machine_id
- "", // usr_id
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "", // os_sp
- "", // os_board
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest",
- false, // delta okay
- ""); // url
const string http_response(GetNoUpdateResponse(OmahaRequestParams::kAppId));
GMainLoop *loop = g_main_loop_new(g_main_context_default(), FALSE);
- OmahaRequestAction action(params, NULL,
+ PrefsMock prefs;
+ OmahaRequestAction action(&prefs, kDefaultTestParams, NULL,
new MockHttpFetcher(http_response.data(),
http_response.size()));
OmahaRequestActionTestProcessorDelegate delegate;
@@ -273,21 +270,10 @@
}
TEST(OmahaRequestActionTest, InvalidXmlTest) {
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "x86-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- "http://url");
OmahaResponse response;
ASSERT_FALSE(
- TestUpdateCheck(params,
+ TestUpdateCheck(NULL, // prefs
+ kDefaultTestParams,
"invalid xml>",
kActionCodeError,
&response,
@@ -296,21 +282,10 @@
}
TEST(OmahaRequestActionTest, MissingStatusTest) {
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "x86-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- "http://url");
OmahaResponse response;
ASSERT_FALSE(TestUpdateCheck(
- params,
+ NULL, // prefs
+ kDefaultTestParams,
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><gupdate "
"xmlns=\"http://www.google.com/update2/response\" protocol=\"2.0\"><app "
"appid=\"foo\" status=\"ok\"><ping "
@@ -322,21 +297,10 @@
}
TEST(OmahaRequestActionTest, InvalidStatusTest) {
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "x86-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- "http://url");
OmahaResponse response;
ASSERT_FALSE(TestUpdateCheck(
- params,
+ NULL, // prefs
+ kDefaultTestParams,
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><gupdate "
"xmlns=\"http://www.google.com/update2/response\" protocol=\"2.0\"><app "
"appid=\"foo\" status=\"ok\"><ping "
@@ -348,21 +312,10 @@
}
TEST(OmahaRequestActionTest, MissingNodesetTest) {
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "x86-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- "http://url");
OmahaResponse response;
ASSERT_FALSE(TestUpdateCheck(
- params,
+ NULL, // prefs
+ kDefaultTestParams,
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><gupdate "
"xmlns=\"http://www.google.com/update2/response\" protocol=\"2.0\"><app "
"appid=\"foo\" status=\"ok\"><ping "
@@ -374,20 +327,9 @@
}
TEST(OmahaRequestActionTest, MissingFieldTest) {
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "x86-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- "http://url");
OmahaResponse response;
- ASSERT_TRUE(TestUpdateCheck(params,
+ ASSERT_TRUE(TestUpdateCheck(NULL, // prefs
+ kDefaultTestParams,
string("<?xml version=\"1.0\" "
"encoding=\"UTF-8\"?><gupdate "
"xmlns=\"http://www.google.com/"
@@ -436,22 +378,11 @@
} // namespace {}
TEST(OmahaRequestActionTest, TerminateTransferTest) {
- OmahaRequestParams params("", // machine_id
- "", // usr_id
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "", // os_sp
- "", // os_board
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest",
- false, // delta okay
- "http://url");
string http_response("doesn't matter");
GMainLoop *loop = g_main_loop_new(g_main_context_default(), FALSE);
- OmahaRequestAction action(params, NULL,
+ PrefsMock prefs;
+ OmahaRequestAction action(&prefs, kDefaultTestParams, NULL,
new MockHttpFetcher(http_response.data(),
http_response.size()));
TerminateEarlyTestProcessorDelegate delegate;
@@ -480,47 +411,36 @@
OmahaRequestParams::kOsPlatform,
OmahaRequestParams::kOsVersion,
"testtheservice_pack>",
- "x86 generic",
+ "x86 generic<id",
OmahaRequestParams::kAppId,
"0.1.0.0",
"en-US",
- "unittest_track",
+ "unittest_track<",
false, // delta okay
"http://url");
OmahaResponse response;
ASSERT_FALSE(
- TestUpdateCheck(params,
+ TestUpdateCheck(NULL, // prefs
+ params,
"invalid xml>",
kActionCodeError,
&response,
&post_data));
// convert post_data to string
string post_str(&post_data[0], post_data.size());
- EXPECT_NE(post_str.find("testthemachine<id"), string::npos);
- EXPECT_EQ(post_str.find("testthemachine<id"), string::npos);
- EXPECT_NE(post_str.find("testtheuser_id&lt;"), string::npos);
- EXPECT_EQ(post_str.find("testtheuser_id<"), string::npos);
EXPECT_NE(post_str.find("testtheservice_pack>"), string::npos);
EXPECT_EQ(post_str.find("testtheservice_pack>"), string::npos);
- EXPECT_NE(post_str.find("x86 generic"), string::npos);
+ EXPECT_NE(post_str.find("x86 generic<id"), string::npos);
+ EXPECT_EQ(post_str.find("x86 generic<id"), string::npos);
+ EXPECT_NE(post_str.find("unittest_track&lt;"), string::npos);
+ EXPECT_EQ(post_str.find("unittest_track<"), string::npos);
}
TEST(OmahaRequestActionTest, XmlDecodeTest) {
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "x86-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- "http://url");
OmahaResponse response;
ASSERT_TRUE(
- TestUpdateCheck(params,
+ TestUpdateCheck(NULL, // prefs
+ kDefaultTestParams,
GetUpdateResponse(OmahaRequestParams::kAppId,
"1.2.3.4", // version
"testthe<url", // more info
@@ -538,21 +458,10 @@
}
TEST(OmahaRequestActionTest, ParseIntTest) {
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "the_board",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- "http://url");
OmahaResponse response;
ASSERT_TRUE(
- TestUpdateCheck(params,
+ TestUpdateCheck(NULL, // prefs
+ kDefaultTestParams,
GetUpdateResponse(OmahaRequestParams::kAppId,
"1.2.3.4", // version
"theurl", // more info
@@ -571,27 +480,15 @@
TEST(OmahaRequestActionTest, FormatUpdateCheckOutputTest) {
vector<char> post_data;
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "x86-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- "http://url");
- OmahaResponse response;
- ASSERT_FALSE(TestUpdateCheck(params,
+ ASSERT_FALSE(TestUpdateCheck(NULL, // prefs
+ kDefaultTestParams,
"invalid xml>",
kActionCodeError,
- &response,
+ NULL, // response
&post_data));
// convert post_data to string
string post_str(&post_data[0], post_data.size());
- EXPECT_NE(post_str.find(" <o:ping active=\"0\"></o:ping>\n"
+ EXPECT_NE(post_str.find(" <o:ping a=\"-1\" r=\"-1\"></o:ping>\n"
" <o:updatecheck></o:updatecheck>\n"),
string::npos);
EXPECT_EQ(post_str.find("o:event"), string::npos);
@@ -599,19 +496,7 @@
TEST(OmahaRequestActionTest, FormatSuccessEventOutputTest) {
vector<char> post_data;
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "x86-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- "http://url");
- TestEvent(params,
+ TestEvent(kDefaultTestParams,
new OmahaEvent(OmahaEvent::kTypeUpdateDownloadStarted),
"invalid xml>",
&post_data);
@@ -622,24 +507,13 @@
OmahaEvent::kTypeUpdateDownloadStarted,
OmahaEvent::kResultSuccess);
EXPECT_NE(post_str.find(expected_event), string::npos);
+ EXPECT_EQ(post_str.find("o:ping"), string::npos);
EXPECT_EQ(post_str.find("o:updatecheck"), string::npos);
}
TEST(OmahaRequestActionTest, FormatErrorEventOutputTest) {
vector<char> post_data;
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "x86-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- "http://url");
- TestEvent(params,
+ TestEvent(kDefaultTestParams,
new OmahaEvent(OmahaEvent::kTypeDownloadComplete,
OmahaEvent::kResultError,
kActionCodeError),
@@ -659,19 +533,7 @@
TEST(OmahaRequestActionTest, FormatEventOutputTest) {
vector<char> post_data;
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "x86-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- "http://url");
- TestEvent(params,
+ TestEvent(kDefaultTestParams,
new OmahaEvent(OmahaEvent::kTypeDownloadComplete,
OmahaEvent::kResultError,
kActionCodeError),
@@ -691,28 +553,18 @@
TEST(OmahaRequestActionTest, IsEventTest) {
string http_response("doesn't matter");
- OmahaRequestParams params("machine_id",
- "user_id",
- OmahaRequestParams::kOsPlatform,
- OmahaRequestParams::kOsVersion,
- "service_pack",
- "x86-generic",
- OmahaRequestParams::kAppId,
- "0.1.0.0",
- "en-US",
- "unittest_track",
- false, // delta okay
- "http://url");
-
+ PrefsMock prefs;
OmahaRequestAction update_check_action(
- params,
+ &prefs,
+ kDefaultTestParams,
NULL,
new MockHttpFetcher(http_response.data(),
http_response.size()));
EXPECT_FALSE(update_check_action.IsEvent());
OmahaRequestAction event_action(
- params,
+ &prefs,
+ kDefaultTestParams,
new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
new MockHttpFetcher(http_response.data(),
http_response.size()));
@@ -736,7 +588,8 @@
"unittest_track",
delta_okay,
"http://url");
- ASSERT_FALSE(TestUpdateCheck(params,
+ ASSERT_FALSE(TestUpdateCheck(NULL, // prefs
+ params,
"invalid xml>",
kActionCodeError,
NULL,
@@ -768,4 +621,182 @@
EXPECT_EQ(kActionCodeError, error_event.error_code);
}
+TEST(OmahaRequestActionTest, PingTest) {
+ PrefsMock prefs;
+ // Add a few hours to the day difference to test no rounding, etc.
+ int64_t five_days_ago =
+ (Time::Now() - TimeDelta::FromHours(5 * 24 + 13)).ToInternalValue();
+ int64_t six_days_ago =
+ (Time::Now() - TimeDelta::FromHours(6 * 24 + 11)).ToInternalValue();
+ EXPECT_CALL(prefs, GetInt64(kPrefsLastActivePingDay, _))
+ .WillOnce(DoAll(SetArgumentPointee<1>(six_days_ago), Return(true)));
+ EXPECT_CALL(prefs, GetInt64(kPrefsLastRollCallPingDay, _))
+ .WillOnce(DoAll(SetArgumentPointee<1>(five_days_ago), Return(true)));
+ vector<char> post_data;
+ ASSERT_TRUE(
+ TestUpdateCheck(&prefs,
+ kDefaultTestParams,
+ GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ kActionCodeSuccess,
+ NULL,
+ &post_data));
+ string post_str(&post_data[0], post_data.size());
+ EXPECT_NE(post_str.find("<o:ping a=\"6\" r=\"5\"></o:ping>"), string::npos);
+}
+
+TEST(OmahaRequestActionTest, ActivePingTest) {
+ PrefsMock prefs;
+ int64_t three_days_ago =
+ (Time::Now() - TimeDelta::FromHours(3 * 24 + 12)).ToInternalValue();
+ int64_t now = Time::Now().ToInternalValue();
+ EXPECT_CALL(prefs, GetInt64(kPrefsLastActivePingDay, _))
+ .WillOnce(DoAll(SetArgumentPointee<1>(three_days_ago), Return(true)));
+ EXPECT_CALL(prefs, GetInt64(kPrefsLastRollCallPingDay, _))
+ .WillOnce(DoAll(SetArgumentPointee<1>(now), Return(true)));
+ vector<char> post_data;
+ ASSERT_TRUE(
+ TestUpdateCheck(&prefs,
+ kDefaultTestParams,
+ GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ kActionCodeSuccess,
+ NULL,
+ &post_data));
+ string post_str(&post_data[0], post_data.size());
+ EXPECT_NE(post_str.find("<o:ping a=\"3\"></o:ping>"), string::npos);
+}
+
+TEST(OmahaRequestActionTest, RollCallPingTest) {
+ PrefsMock prefs;
+ int64_t four_days_ago =
+ (Time::Now() - TimeDelta::FromHours(4 * 24)).ToInternalValue();
+ int64_t now = Time::Now().ToInternalValue();
+ EXPECT_CALL(prefs, GetInt64(kPrefsLastActivePingDay, _))
+ .WillOnce(DoAll(SetArgumentPointee<1>(now), Return(true)));
+ EXPECT_CALL(prefs, GetInt64(kPrefsLastRollCallPingDay, _))
+ .WillOnce(DoAll(SetArgumentPointee<1>(four_days_ago), Return(true)));
+ vector<char> post_data;
+ ASSERT_TRUE(
+ TestUpdateCheck(&prefs,
+ kDefaultTestParams,
+ GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ kActionCodeSuccess,
+ NULL,
+ &post_data));
+ string post_str(&post_data[0], post_data.size());
+ EXPECT_NE(post_str.find("<o:ping r=\"4\"></o:ping>\n"), string::npos);
+}
+
+TEST(OmahaRequestActionTest, NoPingTest) {
+ PrefsMock prefs;
+ int64_t one_hour_ago =
+ (Time::Now() - TimeDelta::FromHours(1)).ToInternalValue();
+ EXPECT_CALL(prefs, GetInt64(kPrefsLastActivePingDay, _))
+ .WillOnce(DoAll(SetArgumentPointee<1>(one_hour_ago), Return(true)));
+ EXPECT_CALL(prefs, GetInt64(kPrefsLastRollCallPingDay, _))
+ .WillOnce(DoAll(SetArgumentPointee<1>(one_hour_ago), Return(true)));
+ EXPECT_CALL(prefs, SetInt64(kPrefsLastActivePingDay, _)).Times(0);
+ EXPECT_CALL(prefs, SetInt64(kPrefsLastRollCallPingDay, _)).Times(0);
+ vector<char> post_data;
+ ASSERT_TRUE(
+ TestUpdateCheck(&prefs,
+ kDefaultTestParams,
+ GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ kActionCodeSuccess,
+ NULL,
+ &post_data));
+ string post_str(&post_data[0], post_data.size());
+ EXPECT_EQ(post_str.find("o:ping"), string::npos);
+}
+
+TEST(OmahaRequestActionTest, BackInTimePingTest) {
+ PrefsMock prefs;
+ int64_t future =
+ (Time::Now() + TimeDelta::FromHours(3 * 24 + 4)).ToInternalValue();
+ EXPECT_CALL(prefs, GetInt64(kPrefsLastActivePingDay, _))
+ .WillOnce(DoAll(SetArgumentPointee<1>(future), Return(true)));
+ EXPECT_CALL(prefs, GetInt64(kPrefsLastRollCallPingDay, _))
+ .WillOnce(DoAll(SetArgumentPointee<1>(future), Return(true)));
+ EXPECT_CALL(prefs, SetInt64(kPrefsLastActivePingDay, _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(prefs, SetInt64(kPrefsLastRollCallPingDay, _))
+ .WillOnce(Return(true));
+ vector<char> post_data;
+ ASSERT_TRUE(
+ TestUpdateCheck(&prefs,
+ kDefaultTestParams,
+ "<?xml version=\"1.0\" encoding=\"UTF-8\"?><gupdate "
+ "xmlns=\"http://www.google.com/update2/response\" "
+ "protocol=\"2.0\"><daystart elapsed_seconds=\"100\"/>"
+ "<app appid=\"foo\" status=\"ok\"><ping status=\"ok\"/>"
+ "<updatecheck status=\"noupdate\"/></app></gupdate>",
+ kActionCodeSuccess,
+ NULL,
+ &post_data));
+ string post_str(&post_data[0], post_data.size());
+ EXPECT_EQ(post_str.find("o:ping"), string::npos);
+}
+
+TEST(OmahaRequestActionTest, LastPingDayUpdateTest) {
+ // This test checks that the action updates the last ping day to now
+ // minus 200 seconds with a slack for 5 seconds. Therefore, the test
+ // may fail if it runs for longer than 5 seconds. It shouldn't run
+ // that long though.
+ int64_t midnight =
+ (Time::Now() - TimeDelta::FromSeconds(200)).ToInternalValue();
+ int64_t midnight_slack =
+ (Time::Now() - TimeDelta::FromSeconds(195)).ToInternalValue();
+ PrefsMock prefs;
+ EXPECT_CALL(prefs, SetInt64(kPrefsLastActivePingDay,
+ AllOf(Ge(midnight), Le(midnight_slack))))
+ .WillOnce(Return(true));
+ EXPECT_CALL(prefs, SetInt64(kPrefsLastRollCallPingDay,
+ AllOf(Ge(midnight), Le(midnight_slack))))
+ .WillOnce(Return(true));
+ ASSERT_TRUE(
+ TestUpdateCheck(&prefs,
+ kDefaultTestParams,
+ "<?xml version=\"1.0\" encoding=\"UTF-8\"?><gupdate "
+ "xmlns=\"http://www.google.com/update2/response\" "
+ "protocol=\"2.0\"><daystart elapsed_seconds=\"200\"/>"
+ "<app appid=\"foo\" status=\"ok\"><ping status=\"ok\"/>"
+ "<updatecheck status=\"noupdate\"/></app></gupdate>",
+ kActionCodeSuccess,
+ NULL,
+ NULL));
+}
+
+TEST(OmahaRequestActionTest, NoElapsedSecondsTest) {
+ PrefsMock prefs;
+ EXPECT_CALL(prefs, SetInt64(kPrefsLastActivePingDay, _)).Times(0);
+ EXPECT_CALL(prefs, SetInt64(kPrefsLastRollCallPingDay, _)).Times(0);
+ ASSERT_TRUE(
+ TestUpdateCheck(&prefs,
+ kDefaultTestParams,
+ "<?xml version=\"1.0\" encoding=\"UTF-8\"?><gupdate "
+ "xmlns=\"http://www.google.com/update2/response\" "
+ "protocol=\"2.0\"><daystart blah=\"200\"/>"
+ "<app appid=\"foo\" status=\"ok\"><ping status=\"ok\"/>"
+ "<updatecheck status=\"noupdate\"/></app></gupdate>",
+ kActionCodeSuccess,
+ NULL,
+ NULL));
+}
+
+TEST(OmahaRequestActionTest, BadElapsedSecondsTest) {
+ PrefsMock prefs;
+ EXPECT_CALL(prefs, SetInt64(kPrefsLastActivePingDay, _)).Times(0);
+ EXPECT_CALL(prefs, SetInt64(kPrefsLastRollCallPingDay, _)).Times(0);
+ ASSERT_TRUE(
+ TestUpdateCheck(&prefs,
+ kDefaultTestParams,
+ "<?xml version=\"1.0\" encoding=\"UTF-8\"?><gupdate "
+ "xmlns=\"http://www.google.com/update2/response\" "
+ "protocol=\"2.0\"><daystart elapsed_seconds=\"x\"/>"
+ "<app appid=\"foo\" status=\"ok\"><ping status=\"ok\"/>"
+ "<updatecheck status=\"noupdate\"/></app></gupdate>",
+ kActionCodeSuccess,
+ NULL,
+ NULL));
+}
+
} // namespace chromeos_update_engine
diff --git a/prefs.cc b/prefs.cc
index 38129c9..39f5908 100644
--- a/prefs.cc
+++ b/prefs.cc
@@ -13,6 +13,9 @@
namespace chromeos_update_engine {
+const char kPrefsLastActivePingDay[] = "last-active-ping-day";
+const char kPrefsLastRollCallPingDay[] = "last-roll-call-ping-day";
+
bool Prefs::Init(const FilePath& prefs_dir) {
prefs_dir_ = prefs_dir;
return true;
diff --git a/prefs.h b/prefs.h
index 5926d97..cccfed5 100644
--- a/prefs.h
+++ b/prefs.h
@@ -17,6 +17,8 @@
class Prefs : public PrefsInterface {
public:
+ Prefs() {}
+
// Initializes the store by associating this object with |prefs_dir|
// as the preference store directory. Returns true on success, false
// otherwise.
@@ -39,6 +41,8 @@
// Preference store directory.
FilePath prefs_dir_;
+
+ DISALLOW_COPY_AND_ASSIGN(Prefs);
};
} // namespace chromeos_update_engine
diff --git a/prefs_interface.h b/prefs_interface.h
index 5f75479..dbfa68a 100644
--- a/prefs_interface.h
+++ b/prefs_interface.h
@@ -9,6 +9,9 @@
namespace chromeos_update_engine {
+extern const char kPrefsLastActivePingDay[];
+extern const char kPrefsLastRollCallPingDay[];
+
// The prefs interface allows access to a persistent preferences
// store. The two reasons for providing this as an interface are
// testing as well as easier switching to a new implementation in the
@@ -33,6 +36,8 @@
// Associates |key| with an int64 |value|. Returns true on success,
// false otherwise.
virtual bool SetInt64(const std::string& key, const int64_t value) = 0;
+
+ virtual ~PrefsInterface() {}
};
} // namespace chromeos_update_engine
diff --git a/prefs_mock.h b/prefs_mock.h
new file mode 100644
index 0000000..cf030f0
--- /dev/null
+++ b/prefs_mock.h
@@ -0,0 +1,24 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_PREFS_MOCK_H__
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_PREFS_MOCK_H__
+
+#include "gmock/gmock.h"
+#include "update_engine/prefs_interface.h"
+
+namespace chromeos_update_engine {
+
+class PrefsMock : public PrefsInterface {
+ public:
+ MOCK_METHOD2(GetString, bool(const std::string& key, std::string* value));
+ MOCK_METHOD2(SetString, bool(const std::string& key,
+ const std::string& value));
+ MOCK_METHOD2(GetInt64, bool(const std::string& key, int64_t* value));
+ MOCK_METHOD2(SetInt64, bool(const std::string& key, const int64_t value));
+};
+
+} // namespace chromeos_update_engine
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_PREFS_MOCK_H__
diff --git a/prefs_unittest.cc b/prefs_unittest.cc
index fc492fb..87116e5 100644
--- a/prefs_unittest.cc
+++ b/prefs_unittest.cc
@@ -91,12 +91,11 @@
TEST_F(PrefsTest, SetStringCreateDir) {
const char kKey[] = "a-test-key";
const char kValue[] = "test value";
- EXPECT_TRUE(prefs_.Init(FilePath(prefs_dir_.Append("subdir"))));
+ FilePath subdir = prefs_dir_.Append("subdir1").Append("subdir2");
+ EXPECT_TRUE(prefs_.Init(subdir));
EXPECT_TRUE(prefs_.SetString(kKey, kValue));
string value;
- EXPECT_TRUE(
- file_util::ReadFileToString(prefs_dir_.Append("subdir").Append(kKey),
- &value));
+ EXPECT_TRUE(file_util::ReadFileToString(subdir.Append(kKey), &value));
EXPECT_EQ(kValue, value);
}
diff --git a/update_attempter.cc b/update_attempter.cc
index 939e074..50c59ab 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -134,7 +134,8 @@
// Actions:
shared_ptr<OmahaRequestAction> update_check_action(
- new OmahaRequestAction(omaha_request_params_,
+ new OmahaRequestAction(prefs_,
+ omaha_request_params_,
NULL,
new LibcurlHttpFetcher));
shared_ptr<OmahaResponseHandlerAction> response_handler_action(
@@ -144,14 +145,16 @@
shared_ptr<FilesystemCopierAction> kernel_filesystem_copier_action(
new FilesystemCopierAction(true));
shared_ptr<OmahaRequestAction> download_started_action(
- new OmahaRequestAction(omaha_request_params_,
+ new OmahaRequestAction(prefs_,
+ omaha_request_params_,
new OmahaEvent(
OmahaEvent::kTypeUpdateDownloadStarted),
new LibcurlHttpFetcher));
shared_ptr<DownloadAction> download_action(
new DownloadAction(new LibcurlHttpFetcher));
shared_ptr<OmahaRequestAction> download_finished_action(
- new OmahaRequestAction(omaha_request_params_,
+ new OmahaRequestAction(prefs_,
+ omaha_request_params_,
new OmahaEvent(
OmahaEvent::kTypeUpdateDownloadFinished),
new LibcurlHttpFetcher));
@@ -162,7 +165,8 @@
shared_ptr<PostinstallRunnerAction> postinstall_runner_action_postcommit(
new PostinstallRunnerAction(false));
shared_ptr<OmahaRequestAction> update_complete_action(
- new OmahaRequestAction(omaha_request_params_,
+ new OmahaRequestAction(prefs_,
+ omaha_request_params_,
new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
new LibcurlHttpFetcher));
@@ -389,7 +393,8 @@
return false;
shared_ptr<OmahaRequestAction> error_event_action(
- new OmahaRequestAction(omaha_request_params_,
+ new OmahaRequestAction(prefs_,
+ omaha_request_params_,
error_event_.release(), // Pass ownership.
new LibcurlHttpFetcher));
actions_.push_back(shared_ptr<AbstractAction>(error_event_action));
diff --git a/update_attempter.h b/update_attempter.h
index cd7f480..6fd4c16 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -38,8 +38,9 @@
class UpdateAttempter : public ActionProcessorDelegate,
public DownloadActionDelegate {
public:
- UpdateAttempter(MetricsLibraryInterface* metrics_lib)
+ UpdateAttempter(PrefsInterface* prefs, MetricsLibraryInterface* metrics_lib)
: dbus_service_(NULL),
+ prefs_(prefs),
metrics_lib_(metrics_lib),
status_(UPDATE_STATUS_IDLE),
download_progress_(0.0),
@@ -120,6 +121,9 @@
// pointer to the OmahaResponseHandlerAction in the actions_ vector;
std::tr1::shared_ptr<OmahaResponseHandlerAction> response_handler_action_;
+ // Pointer to the preferences store interface.
+ PrefsInterface* prefs_;
+
// Pointer to the UMA metrics collection library.
MetricsLibraryInterface* metrics_lib_;