shill: Use carrier based unique id to identify a service's storage.
Current shill stores user data pertaining to a cellular services managed
by CellularCapabilityUniversal by using the IMSI read from the SIM. This
CL fixes this by assigning a unique identifier to each carrier, which is
obtained from CellularOperatorInfo via operator id.
BUG=chrome-os-partner:15268
TEST=Build and run unittests.
Change-Id: I3880daeca65fed4ef441e87eea8657935ee38764
Reviewed-on: https://gerrit.chromium.org/gerrit/41804
Reviewed-by: Ben Chan <benchan@chromium.org>
Commit-Queue: Arman Uguray <armansito@chromium.org>
Tested-by: Arman Uguray <armansito@chromium.org>
diff --git a/cellular.h b/cellular.h
index 800e78b..3aec582 100644
--- a/cellular.h
+++ b/cellular.h
@@ -241,6 +241,7 @@
FRIEND_TEST(CellularCapabilityUniversalTest, UpdateOLP);
FRIEND_TEST(CellularCapabilityUniversalTest, UpdateOperatorInfoViaOperatorId);
FRIEND_TEST(CellularCapabilityUniversalTest, UpdateScanningProperty);
+ FRIEND_TEST(CellularCapabilityUniversalTest, UpdateStorageIdentifier);
FRIEND_TEST(CellularServiceTest, FriendlyName);
FRIEND_TEST(CellularTest, CreateService);
FRIEND_TEST(CellularTest, Connect);
diff --git a/cellular_capability_universal.cc b/cellular_capability_universal.cc
index 2920719..0f0b85e 100644
--- a/cellular_capability_universal.cc
+++ b/cellular_capability_universal.cc
@@ -376,13 +376,33 @@
sim_proxy_.reset();
}
-void CellularCapabilityUniversal::OnServiceCreated() {
- // If IMSI is available, base the service's storage identifier on it.
- if (!imsi_.empty()) {
- cellular()->service()->SetStorageIdentifier(
- string(flimflam::kTypeCellular) + "_" +
- cellular()->address() + "_" + imsi_);
+void CellularCapabilityUniversal::UpdateStorageIdentifier() {
+ if (!cellular()->service().get())
+ return;
+
+ // Lookup the unique identifier assigned to the current network and base the
+ // service's storage identifier on it.
+ const string prefix = "cellular_" + cellular()->address() + "_";
+ string storage_id;
+ if (!operator_id_.empty()) {
+ const CellularOperatorInfo::CellularOperator *provider =
+ cellular()->cellular_operator_info()->
+ GetCellularOperatorByMCCMNC(operator_id_);
+ if (provider && !provider->identifier().empty()) {
+ storage_id = prefix + provider->identifier();
+ }
}
+ // If the above didn't work, append IMSI, if available.
+ if (storage_id.empty() && !imsi_.empty()) {
+ storage_id = prefix + imsi_;
+ }
+ if (!storage_id.empty()) {
+ cellular()->service()->SetStorageIdentifier(storage_id);
+ }
+}
+
+void CellularCapabilityUniversal::OnServiceCreated() {
+ UpdateStorageIdentifier();
bool activation_required = IsServiceActivationRequired();
cellular()->service()->SetActivationState(
activation_required ?
diff --git a/cellular_capability_universal.h b/cellular_capability_universal.h
index c8b36d5..84ef2de 100644
--- a/cellular_capability_universal.h
+++ b/cellular_capability_universal.h
@@ -150,6 +150,7 @@
FRIEND_TEST(CellularCapabilityUniversalTest, StartModem);
FRIEND_TEST(CellularCapabilityUniversalTest, StopModem);
FRIEND_TEST(CellularCapabilityUniversalTest, StopModemConnected);
+ FRIEND_TEST(CellularCapabilityUniversalTest, UpdateStorageIdentifier);
FRIEND_TEST(CellularCapabilityUniversalTest, UpdateOLP);
FRIEND_TEST(CellularCapabilityUniversalTest, UpdateOperatorInfo);
FRIEND_TEST(CellularCapabilityUniversalTest, UpdateOperatorInfoViaOperatorId);
@@ -190,6 +191,9 @@
// Updates |bearer_path_| to match the currently active bearer.
void UpdateBearerPath();
+ // Updates the storage identifier used for the current cellular service.
+ void UpdateStorageIdentifier();
+
// Initializes the |apn_list_| property based on the current |home_provider_|.
void InitAPNList();
diff --git a/cellular_capability_universal_unittest.cc b/cellular_capability_universal_unittest.cc
index 5a2ad5d..ca5e2a2 100644
--- a/cellular_capability_universal_unittest.cc
+++ b/cellular_capability_universal_unittest.cc
@@ -9,6 +9,7 @@
#include <base/bind.h>
#include <base/stringprintf.h>
+#include <base/string_util.h>
#include <chromeos/dbus/service_constants.h>
#include <gtest/gtest.h>
#include <mobile_provider.h>
@@ -86,7 +87,7 @@
&metrics_,
&manager_,
"",
- "",
+ kMachineAddress,
0,
Cellular::kTypeUniversal,
"",
@@ -179,6 +180,7 @@
static const char kActiveBearerPathPrefix[];
static const char kImei[];
static const char kInactiveBearerPathPrefix[];
+ static const char kMachineAddress[];
static const char kSimPath[];
static const uint32 kAccessTechnologies;
@@ -266,6 +268,8 @@
const char CellularCapabilityUniversalTest::kImei[] = "999911110000";
const char CellularCapabilityUniversalTest::kInactiveBearerPathPrefix[] =
"/bearer/inactive";
+const char CellularCapabilityUniversalTest::kMachineAddress[] =
+ "TestMachineAddress";
const char CellularCapabilityUniversalTest::kSimPath[] = "/foo/sim";
const uint32 CellularCapabilityUniversalTest::kAccessTechnologies =
MM_MODEM_ACCESS_TECHNOLOGY_LTE |
@@ -1089,6 +1093,57 @@
EXPECT_FALSE(capability_->scanning_or_searching_);
}
+TEST_F(CellularCapabilityUniversalTest, UpdateStorageIdentifier) {
+ CellularOperatorInfo::CellularOperator provider;
+ cellular_->cellular_operator_info_ = &cellular_operator_info_;
+
+ SetService();
+
+ const string prefix = "cellular_" + string(kMachineAddress) + "_";
+ const string default_identifier_pattern = prefix + "GSMNetwork*";
+
+ // |capability_->operator_id_| is "".
+ capability_->UpdateStorageIdentifier();
+ EXPECT_TRUE(::MatchPattern(cellular_->service()->storage_identifier_,
+ default_identifier_pattern));
+
+ // GetCellularOperatorByMCCMNC returns NULL.
+ capability_->operator_id_ = "1";
+ EXPECT_CALL(cellular_operator_info_,
+ GetCellularOperatorByMCCMNC(capability_->operator_id_))
+ .WillOnce(
+ Return((const CellularOperatorInfo::CellularOperator *)NULL));
+
+ capability_->UpdateStorageIdentifier();
+ EXPECT_TRUE(::MatchPattern(cellular_->service()->storage_identifier_,
+ default_identifier_pattern));
+
+ // |capability_->imsi_| is not ""
+ capability_->imsi_ = "TESTIMSI";
+ EXPECT_CALL(cellular_operator_info_,
+ GetCellularOperatorByMCCMNC(capability_->operator_id_))
+ .WillOnce(
+ Return((const CellularOperatorInfo::CellularOperator *)NULL));
+
+ capability_->UpdateStorageIdentifier();
+ EXPECT_EQ(prefix + "TESTIMSI", cellular_->service()->storage_identifier_);
+
+ EXPECT_CALL(cellular_operator_info_,
+ GetCellularOperatorByMCCMNC(capability_->operator_id_))
+ .Times(2)
+ .WillRepeatedly(Return(&provider));
+
+ // |provider.identifier_| is "".
+ capability_->UpdateStorageIdentifier();
+ EXPECT_EQ(prefix + "TESTIMSI", cellular_->service()->storage_identifier_);
+
+ // Success.
+ provider.identifier_ = "testidentifier";
+ capability_->UpdateStorageIdentifier();
+ EXPECT_EQ(prefix + "testidentifier",
+ cellular_->service()->storage_identifier_);
+}
+
TEST_F(CellularCapabilityUniversalTest, UpdateOLP) {
CellularService::OLP test_olp;
test_olp.SetURL("http://testurl");
diff --git a/cellular_operator_info.cc b/cellular_operator_info.cc
index 2e0daf0..a1a7320 100644
--- a/cellular_operator_info.cc
+++ b/cellular_operator_info.cc
@@ -189,6 +189,7 @@
key_handler_map["apn"] = &CellularOperatorInfo::HandleAPN;
key_handler_map["sid"] = &CellularOperatorInfo::HandleSID;
key_handler_map["olp"] = &CellularOperatorInfo::HandleOLP;
+ key_handler_map["identifier"] = &CellularOperatorInfo::HandleIdentifier;
key_handler_map["country"] = &CellularOperatorInfo::HandleCountry;
string line;
@@ -312,6 +313,16 @@
return true;
}
+bool CellularOperatorInfo::HandleIdentifier(ParserState *state,
+ const string &value) {
+ if (!state->provider) {
+ LOG(ERROR) << "Found \"identifier\" entry without \"provider\".";
+ return false;
+ }
+ state->provider->identifier_ = value;
+ return true;
+}
+
bool CellularOperatorInfo::HandleAPN(ParserState *state,
const string &value) {
if (!state->provider) {
diff --git a/cellular_operator_info.h b/cellular_operator_info.h
index 8e9c0fd..85acd73 100644
--- a/cellular_operator_info.h
+++ b/cellular_operator_info.h
@@ -57,6 +57,7 @@
virtual ~CellularOperator();
const std::string &country() const { return country_; }
+ const std::string &identifier() const { return identifier_; }
const std::vector<std::string> &mccmnc_list() const {
return mccmnc_list_;
}
@@ -70,12 +71,15 @@
bool requires_roaming() const { return requires_roaming_; }
private:
+ friend class CellularCapabilityUniversalTest;
friend class CellularOperatorInfo;
friend class CellularOperatorInfoTest;
+ FRIEND_TEST(CellularCapabilityUniversalTest, UpdateStorageIdentifier);
FRIEND_TEST(CellularOperatorInfoTest, HandleMCCMNC);
FRIEND_TEST(CellularOperatorInfoTest, HandleSID);
std::string country_;
+ std::string identifier_;
std::vector<std::string> mccmnc_list_;
std::vector<std::string> sid_list_;
std::vector<LocalizedName> name_list_;
@@ -130,6 +134,7 @@
FRIEND_TEST(CellularOperatorInfoTest, HandleOLP);
FRIEND_TEST(CellularOperatorInfoTest, HandleProvider);
FRIEND_TEST(CellularOperatorInfoTest, HandleSID);
+ FRIEND_TEST(CellularOperatorInfoTest, HandleIdentifier);
FRIEND_TEST(CellularOperatorInfoTest, ParseKeyValue);
FRIEND_TEST(CellularOperatorInfoTest, ParseNameLine);
FRIEND_TEST(CellularOperatorInfoTest, ParseSuccess);
@@ -164,6 +169,8 @@
const std::string &value);
bool HandleCountry(ParserState *state,
const std::string &value);
+ bool HandleIdentifier(ParserState *state,
+ const std::string &value);
bool ParseNameLine(const std::string &value,
LocalizedName *name);
diff --git a/cellular_operator_info_unittest.cc b/cellular_operator_info_unittest.cc
index 87ec7b8..34e56f4 100644
--- a/cellular_operator_info_unittest.cc
+++ b/cellular_operator_info_unittest.cc
@@ -24,6 +24,7 @@
"\n"
"# TestProvider1\n"
"provider:1,1,0,1\n"
+ "identifier:provider1identifier\n"
"name:,TestProvider1\n"
"mccmnc:000001,0,000002,0\n"
"sid:1,0,2,0,3,0\n"
@@ -34,6 +35,7 @@
"\n"
"# TestProvider2\n"
"provider:1,2,1,0\n"
+ "identifier:provider2identifier\n"
"name:,TestProviderTwo\n"
"name:,TestProvider2\n"
"mccmnc:100001,1,100002,0\n"
@@ -76,6 +78,7 @@
EXPECT_FALSE(provider->is_primary());
EXPECT_TRUE(provider->requires_roaming());
EXPECT_EQ(provider->country(), "us");
+ EXPECT_EQ(provider->identifier(), "provider1identifier");
EXPECT_EQ(provider->name_list().size(), 1);
EXPECT_EQ(provider->name_list()[0].language, "");
EXPECT_EQ(provider->name_list()[0].name, "TestProvider1");
@@ -107,6 +110,7 @@
EXPECT_TRUE(provider2->is_primary());
EXPECT_FALSE(provider2->requires_roaming());
EXPECT_EQ(provider2->country(), "us");
+ EXPECT_EQ(provider2->identifier(), "provider2identifier");
EXPECT_EQ(provider2->name_list().size(), 2);
EXPECT_EQ(provider2->name_list()[0].language, "");
EXPECT_EQ(provider2->name_list()[0].name, "TestProviderTwo");
@@ -421,6 +425,22 @@
EXPECT_EQ(state.provider->sid_to_olp_idx_["3"], 0);
}
+TEST_F(CellularOperatorInfoTest, HandleIdentifier) {
+ CellularOperatorInfo::ParserState state;
+ EXPECT_FALSE(info_.HandleIdentifier(&state, "testidentifier"));
+
+ EXPECT_TRUE(info_.HandleProvider(&state, "1,1,0,0"));
+ EXPECT_TRUE(state.provider);
+ EXPECT_EQ("", state.provider->identifier());
+
+ EXPECT_TRUE(info_.HandleIdentifier(&state, "testidentifier0"));
+ EXPECT_EQ("testidentifier0", state.provider->identifier());
+ EXPECT_TRUE(info_.HandleIdentifier(&state, "testidentifier1"));
+ EXPECT_EQ("testidentifier1", state.provider->identifier());
+ EXPECT_TRUE(info_.HandleIdentifier(&state, "testidentifier2"));
+ EXPECT_EQ("testidentifier2", state.provider->identifier());
+}
+
TEST_F(CellularOperatorInfoTest, HandleOLP) {
CellularOperatorInfo::ParserState state;
EXPECT_FALSE(info_.HandleOLP(&state, ",,"));
diff --git a/cellular_service.h b/cellular_service.h
index fac61a9..8af31e1 100644
--- a/cellular_service.h
+++ b/cellular_service.h
@@ -110,9 +110,11 @@
virtual bool IsAutoConnectable(const char **reason) const;
private:
+ friend class CellularCapabilityUniversalTest;
friend class CellularServiceTest;
FRIEND_TEST(CellularCapabilityGSMTest, SetupApnTryList);
FRIEND_TEST(CellularCapabilityTest, TryApns);
+ FRIEND_TEST(CellularCapabilityUniversalTest, UpdateStorageIdentifier);
FRIEND_TEST(CellularTest, Connect);
FRIEND_TEST(CellularServiceTest, SetApn);
FRIEND_TEST(CellularServiceTest, ClearApn);
diff --git a/data/cellular_operator_info b/data/cellular_operator_info
index 3e5df7d..a77ad25 100644
--- a/data/cellular_operator_info
+++ b/data/cellular_operator_info
@@ -9,6 +9,7 @@
# <provider block 2>
#
# provider:<# of names>,<# of apns>,<primary>,<roaming>
+# identifier:<unique operator identifier>
# name:<lang>,<name 1>
# name:<lang>,<name 2>
# ...
@@ -28,6 +29,7 @@
# Verizon Wireless
provider:1,1,0,0
+identifier:vzw
name:,Verizon Wireless
mccmnc:310995,0,311480,0
sid:2,1,4,1,5,1,6,1,8,1,12,1,15,1,17,1,18,1,20,1,21,1,22,1,26,1,28,1,30,1,32,1,33,1,37,1,40,1,41,1,48,1,51,1,54,1,56,1,58,1,59,1,60,1,64,1,65,1,69,1,73,1,75,1,78,1,80,1,86,1,92,1,93,1,94,1,95,1,96,1,104,1,107,1,110,1,112,1,113,1,119,1,126,1,127,1,133,1,137,1,139,1,143,1,150,1,154,1,162,1,163,1,165,1,170,1,172,1,179,1,180,1,181,1,186,1,189,1,190,1,203,1,213,1,214,1,222,1,224,1,226,1,228,1,241,1,250,1,258,1,262,1,263,1,266,1,272,1,276,1,284,1,286,1,294,1,298,1,299,1,300,1,314,1,316,1,319,1,323,1,328,1,329,1,330,1,349,1,356,1,377,1,385,1,404,1,428,1,443,1,456,1,465,1,482,1,483,1,486,1,490,1,498,1,502,1,506,1,528,1,530,1,532,1,539,1,1015,1,1026,1,1032,1,1034,1,1062,1,1072,1,1074,1,1076,1,1083,1,1086,1,1088,1,1094,1,1103,1,1129,1,1131,1,1137,1,1139,1,1145,1,1151,1,1153,1,1164,1,1166,1,1174,1,1180,1,1189,1,1193,1,1196,1,1220,1,1224,1,1227,1,1230,1,1267,1,1285,1,1330,1,1358,1,1417,1,1429,1,1476,1,1488,1,1492,1,1494,1,1506,1,1510,1,1514,1,1516,1,1517,1,1519,1,1523,1,1548,1,1552,1,1563,1,1567,1,1614,1,1626,1,1630,1,1632,1,1637,1,1639,1,1641,1,1653,1,1679,1,1736,1,1740,1,1749,1,1760,1,1776,1,1780,1,1790,1,1792,1,1824,1,1826,1,1827,1,1830,1,1832,1,1857,1,1910,1,1912,1,1940,1,1969,1,2004,1,2054,1,2058,1,2060,1,2076,1,2115,1,2119,1,2125,1,2127,1,2149,1,3004,1,3008,1,3046,1,3066,1,3216,1,3218,1,3228,1,6709,1,6711,1,7532,1,7536,1,9640,1,9642,1,9644,1
diff --git a/mock_cellular_operator_info.h b/mock_cellular_operator_info.h
index 9346301..3a37085 100644
--- a/mock_cellular_operator_info.h
+++ b/mock_cellular_operator_info.h
@@ -20,7 +20,9 @@
MOCK_METHOD1(Load, bool(const FilePath &info_file_path));
MOCK_METHOD1(GetOLPByMCCMNC,
- const CellularService::OLP *(const std::string &operator_id));
+ const CellularService::OLP *(const std::string &mccmnc));
+ MOCK_METHOD1(GetCellularOperatorByMCCMNC,
+ const CellularOperator *(const std::string &mccmnc));
};
} // namespace shill
diff --git a/service.cc b/service.cc
index 78d15d4..e02e499 100644
--- a/service.cc
+++ b/service.cc
@@ -1234,6 +1234,7 @@
}
void Service::SetAutoConnect(const bool &connect, Error *error) {
+ LOG(INFO) << __func__ << "(" << connect << ")";
set_auto_connect(connect);
}