Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(770)

Unified Diff: google_apis/gcm/engine/gservices_settings_unittest.cc

Issue 288433002: G-services settings v3 implementation (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Adding tests for settings diff, addressing CR feedback" Created 6 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: google_apis/gcm/engine/gservices_settings_unittest.cc
diff --git a/google_apis/gcm/engine/gservices_settings_unittest.cc b/google_apis/gcm/engine/gservices_settings_unittest.cc
index 12623e348128c8af6b7ffa78fbdde76bd4749b12..f5466a4fd3a2a03f42de82496f0ef3d85299693a 100644
--- a/google_apis/gcm/engine/gservices_settings_unittest.cc
+++ b/google_apis/gcm/engine/gservices_settings_unittest.cc
@@ -23,6 +23,24 @@ const int64 kDefaultCheckinInterval = 2 * 24 * 60 * 60; // seconds = 2 days.
const char kDefaultCheckinURL[] = "https://android.clients.google.com/checkin";
const char kDefaultRegistrationURL[] =
"https://android.clients.google.com/c2dm/register3";
+const char kDefaultSettingsDigest[] =
+ "1-4C598686664FF131F439C522AB7B355A65819B1F";
+const char kAlternativeSettingsDigest[] =
+ "1-7DA4AA4EB38A8BD3E330E3751CC0899924499134";
+
+void AddSettingsToResponse(
+ checkin_proto::AndroidCheckinResponse& checkin_response,
+ const std::map<std::string, std::string>& settings,
jianli 2014/05/14 22:04:30 use GServicesSettings::SettingsMap
fgorski 2014/05/16 20:48:47 Done.
+ bool settings_diff) {
+ for (std::map<std::string, std::string>::const_iterator iter =
jianli 2014/05/14 22:04:30 ditto
fgorski 2014/05/16 20:48:47 Done.
+ settings.begin();
+ iter != settings.end(); ++iter) {
+ checkin_proto::GservicesSetting* setting = checkin_response.add_setting();
+ setting->set_name(iter->first);
+ setting->set_value(iter->second);
+ }
+ checkin_response.set_settings_diff(settings_diff);
+}
} // namespace
@@ -70,41 +88,48 @@ void GServicesSettingsTest::SetUp() {
void GServicesSettingsTest::CheckAllSetToDefault() {
EXPECT_EQ(base::TimeDelta::FromSeconds(kDefaultCheckinInterval),
- settings().checkin_interval());
- EXPECT_EQ(GURL(kDefaultCheckinURL), settings().checkin_url());
+ settings().GetCheckinInterval());
+ EXPECT_EQ(GURL(kDefaultCheckinURL), settings().GetCheckinURL());
EXPECT_EQ(GURL("https://mtalk.google.com:5228"),
- settings().mcs_main_endpoint());
+ settings().GetMCSMainEndpoint());
EXPECT_EQ(GURL("https://mtalk.google.com:443"),
- settings().mcs_fallback_endpoint());
- EXPECT_EQ(GURL(kDefaultRegistrationURL), settings().registration_url());
+ settings().GetMCSFallbackEndpoint());
+ EXPECT_EQ(GURL(kDefaultRegistrationURL), settings().GetRegistrationURL());
}
void GServicesSettingsTest::CheckAllSetToAlternative() {
EXPECT_EQ(base::TimeDelta::FromSeconds(kAlternativeCheckinInterval),
- settings().checkin_interval());
- EXPECT_EQ(GURL(kAlternativeCheckinURL), settings().checkin_url());
+ settings().GetCheckinInterval());
+ EXPECT_EQ(GURL(kAlternativeCheckinURL), settings().GetCheckinURL());
EXPECT_EQ(GURL("https://alternative.gcm.host:7777"),
- settings().mcs_main_endpoint());
+ settings().GetMCSMainEndpoint());
EXPECT_EQ(GURL("https://alternative.gcm.host:443"),
- settings().mcs_fallback_endpoint());
- EXPECT_EQ(GURL(kAlternativeRegistrationURL), settings().registration_url());
+ settings().GetMCSFallbackEndpoint());
+ EXPECT_EQ(GURL(kAlternativeRegistrationURL), settings().GetRegistrationURL());
}
void GServicesSettingsTest::SetWithAlternativeSettings(
checkin_proto::AndroidCheckinResponse& checkin_response) {
- for (std::map<std::string, std::string>::const_iterator iter =
- alternative_settings_.begin();
- iter != alternative_settings_.end(); ++iter) {
- checkin_proto::GservicesSetting* setting = checkin_response.add_setting();
- setting->set_name(iter->first);
- setting->set_value(iter->second);
- }
+ AddSettingsToResponse(checkin_response, alternative_settings_, false);
}
// Verifies default values of the G-services settings and settings digest.
TEST_F(GServicesSettingsTest, DefaultSettingsAndDigest) {
CheckAllSetToDefault();
- EXPECT_EQ(std::string(), settings().digest());
+ EXPECT_EQ(kDefaultSettingsDigest, settings().digest());
+}
+
+// Verifies digest calculation for default settings.
+TEST_F(GServicesSettingsTest, CalculateDigestForDefaultSettings) {
+ CheckAllSetToDefault();
+ EXPECT_EQ(kDefaultSettingsDigest,
+ GServicesSettings::CalculateDigest(settings().settings_map()));
+}
+
+// Verifies digest calculation for aternative settings.
+TEST_F(GServicesSettingsTest, CalculateDigestForAternativeSettings) {
+ EXPECT_EQ(kAlternativeSettingsDigest,
+ GServicesSettings::CalculateDigest(alternative_settings_));
}
// Verifies that settings are not updated when load result is empty.
@@ -114,7 +139,7 @@ TEST_F(GServicesSettingsTest, UpdateFromEmptyLoadResult) {
settings().UpdateFromLoadResult(result);
CheckAllSetToDefault();
- EXPECT_EQ(std::string(), settings().digest());
+ EXPECT_EQ(kDefaultSettingsDigest, settings().digest());
}
// Verifies that settings are not updated when one of them is missing.
@@ -126,18 +151,18 @@ TEST_F(GServicesSettingsTest, UpdateFromLoadResultWithSettingMissing) {
settings().UpdateFromLoadResult(result);
CheckAllSetToDefault();
- EXPECT_EQ(std::string(), settings().digest());
+ EXPECT_EQ(kDefaultSettingsDigest, settings().digest());
}
// Verifies that the settings are set correctly based on the load result.
TEST_F(GServicesSettingsTest, UpdateFromLoadResult) {
GCMStore::LoadResult result;
result.gservices_settings = alternative_settings();
- result.gservices_digest = "digest_value";
+ result.gservices_digest = kAlternativeSettingsDigest;
settings().UpdateFromLoadResult(result);
CheckAllSetToAlternative();
- EXPECT_EQ("digest_value", settings().digest());
+ EXPECT_EQ(kAlternativeSettingsDigest, settings().digest());
}
// Verifies that the settings are set correctly after parsing a checkin
@@ -145,14 +170,14 @@ TEST_F(GServicesSettingsTest, UpdateFromLoadResult) {
TEST_F(GServicesSettingsTest, UpdateFromCheckinResponse) {
checkin_proto::AndroidCheckinResponse checkin_response;
- checkin_response.set_digest("digest_value");
+ checkin_response.set_digest(kAlternativeSettingsDigest);
SetWithAlternativeSettings(checkin_response);
EXPECT_TRUE(settings().UpdateFromCheckinResponse(checkin_response));
CheckAllSetToAlternative();
- EXPECT_EQ(alternative_settings_, settings().GetSettingsMap());
- EXPECT_EQ("digest_value", settings().digest());
+ EXPECT_EQ(alternative_settings_, settings().settings_map());
+ EXPECT_EQ(kAlternativeSettingsDigest, settings().digest());
}
// Verifies that the checkin interval is updated to minimum if the original
@@ -160,7 +185,8 @@ TEST_F(GServicesSettingsTest, UpdateFromCheckinResponse) {
TEST_F(GServicesSettingsTest, UpdateFromCheckinResponseMinimumCheckinInterval) {
checkin_proto::AndroidCheckinResponse checkin_response;
- checkin_response.set_digest("digest_value");
+ const char digest[] = "1-436646257E0FAC3F0D20C866B0DA46C21DCD1FA2";
+ checkin_response.set_digest(digest);
// Setting the checkin interval to less than minimum.
alternative_settings_["checkin_interval"] = base::IntToString(3600);
SetWithAlternativeSettings(checkin_response);
@@ -168,22 +194,22 @@ TEST_F(GServicesSettingsTest, UpdateFromCheckinResponseMinimumCheckinInterval) {
EXPECT_TRUE(settings().UpdateFromCheckinResponse(checkin_response));
EXPECT_EQ(GServicesSettings::MinimumCheckinInterval(),
- settings().checkin_interval());
- EXPECT_EQ("digest_value", settings().digest());
+ settings().GetCheckinInterval());
+ EXPECT_EQ(digest, settings().digest());
}
// Verifies that settings are not updated when one of them is missing.
TEST_F(GServicesSettingsTest, UpdateFromCheckinResponseWithSettingMissing) {
checkin_proto::AndroidCheckinResponse checkin_response;
- checkin_response.set_digest("digest_value");
+ checkin_response.set_digest(kAlternativeSettingsDigest);
alternative_settings_.erase("gcm_hostname");
SetWithAlternativeSettings(checkin_response);
EXPECT_FALSE(settings().UpdateFromCheckinResponse(checkin_response));
CheckAllSetToDefault();
- EXPECT_EQ(std::string(), settings().digest());
+ EXPECT_EQ(kDefaultSettingsDigest, settings().digest());
}
// Verifies that no update is done, when a checkin response misses digest.
@@ -194,23 +220,88 @@ TEST_F(GServicesSettingsTest, UpdateFromCheckinResponseNoDigest) {
EXPECT_FALSE(settings().UpdateFromCheckinResponse(checkin_response));
CheckAllSetToDefault();
- EXPECT_EQ(std::string(), settings().digest());
+ EXPECT_EQ(kDefaultSettingsDigest, settings().digest());
}
// Verifies that no update is done, when a checkin response digest is the same.
TEST_F(GServicesSettingsTest, UpdateFromCheckinResponseSameDigest) {
GCMStore::LoadResult load_result;
- load_result.gservices_digest = "old_digest";
+ load_result.gservices_digest = kAlternativeSettingsDigest;
load_result.gservices_settings = alternative_settings();
settings().UpdateFromLoadResult(load_result);
checkin_proto::AndroidCheckinResponse checkin_response;
- checkin_response.set_digest("old_digest");
+ checkin_response.set_digest(kAlternativeSettingsDigest);
SetWithAlternativeSettings(checkin_response);
EXPECT_FALSE(settings().UpdateFromCheckinResponse(checkin_response));
CheckAllSetToAlternative();
- EXPECT_EQ("old_digest", settings().digest());
+ EXPECT_EQ(kAlternativeSettingsDigest, settings().digest());
+}
+
+// Update from checkin response should also do incremental update for both cases
+// where some settings are removed or added.
+TEST_F(GServicesSettingsTest, UpdateFromCheckinResponseSettingsDiff) {
+ checkin_proto::AndroidCheckinResponse checkin_response;
+ checkin_response.set_digest(kAlternativeSettingsDigest);
+ SetWithAlternativeSettings(checkin_response);
+ EXPECT_TRUE(settings().UpdateFromCheckinResponse(checkin_response));
+
+ // Only the new settings will be included in the response with settings diff.
+ std::map<std::string, std::string> settings_diff;
+ settings_diff["new_setting_1"] = "new_setting_1_value";
+ settings_diff["new_setting_2"] = "new_setting_2_value";
+
+ // Full settings are necessary to calculate digest.
+ std::map<std::string, std::string> full_settings(alternative_settings());
+ full_settings.insert(settings_diff.begin(), settings_diff.end());
+ std::string digest = GServicesSettings::CalculateDigest(full_settings);
+
+ checkin_response.Clear();
+ AddSettingsToResponse(checkin_response, settings_diff, true);
+ checkin_response.set_digest(digest);
+ EXPECT_TRUE(settings().UpdateFromCheckinResponse(checkin_response));
+ EXPECT_EQ(full_settings, settings().settings_map());
+
+ // Setting up diff removing some of the values.
+ settings_diff.clear();
+ settings_diff["delete_new_setting_1"] = "";
+ settings_diff["new_setting_3"] = "new_setting_3_value";
+
+ // Updating full settings to calculate digest.
+ full_settings.erase(full_settings.find("new_setting_1"));
+ full_settings["new_setting_3"] = "new_setting_3_value";
+ digest = GServicesSettings::CalculateDigest(full_settings);
+
+ checkin_response.Clear();
+ AddSettingsToResponse(checkin_response, settings_diff, true);
+ checkin_response.set_digest(digest);
+ EXPECT_TRUE(settings().UpdateFromCheckinResponse(checkin_response));
+ EXPECT_EQ(full_settings, settings().settings_map());
+}
+
+// Update from checkin response should fail when one of the mandatory settings
+// is deleted.
+TEST_F(GServicesSettingsTest, UpdateFromCheckinResponseRequiredSettings) {
+ checkin_proto::AndroidCheckinResponse checkin_response;
+ checkin_response.set_digest(kAlternativeSettingsDigest);
+ SetWithAlternativeSettings(checkin_response);
+ EXPECT_TRUE(settings().UpdateFromCheckinResponse(checkin_response));
+
+ // Settings diff attempts to remove a mandatory setting.
+ std::map<std::string, std::string> settings_diff;
+ settings_diff["delete_checkin_interval"] = "";
+
+ // Full settings are necessary to calculate digest.
+ std::map<std::string, std::string> full_settings(alternative_settings());
+ full_settings.erase(full_settings.find("checkin_interval"));
+ std::string digest = GServicesSettings::CalculateDigest(full_settings);
+
+ checkin_response.Clear();
+ AddSettingsToResponse(checkin_response, settings_diff, true);
+ checkin_response.set_digest(digest);
+ EXPECT_FALSE(settings().UpdateFromCheckinResponse(checkin_response));
+ EXPECT_EQ(alternative_settings(), settings().settings_map());
}
} // namespace gcm

Powered by Google App Engine
This is Rietveld 408576698