shill: cellular: Unify UpdateStorageIdentifier.
Before this CL, |CellularService::SetStorageIdentifier| was called from
different code paths in three of the capabilities. This CL unifies these code
paths to be triggered when the ServingOperator is determined in |Cellular|.
BUG=chromium:352243
TEST=- Manually test that sensible storage identifiers are used for different
operators, and that they are persistent across service reboots.
- Run shill unit-tests.
Change-Id: I5837465589667e4d4feb75eeeeeb6f793fb5450a
Reviewed-on: https://chromium-review.googlesource.com/197602
Commit-Queue: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index bbeb124..4182d9d 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -134,6 +134,7 @@
sim_present_(false),
prl_version_(0),
modem_info_(modem_info),
+ type_(type),
proxy_factory_(proxy_factory),
ppp_device_factory_(PPPDeviceFactory::GetInstance()),
allow_roaming_(false),
@@ -622,7 +623,48 @@
CHECK(!service_.get());
service_ = new CellularService(modem_info_, this);
capability_->OnServiceCreated();
+
+ // Storage identifier must be set only once, and before registering the
+ // service with the manager, since we key off of this identifier to
+ // determine the profile to load.
+ // TODO(pprabhu) Make profile matching more robust (crbug.com/369755)
+ string service_id;
+ if (home_provider_info_->IsMobileNetworkOperatorKnown() &&
+ !home_provider_info_->uuid().empty()){
+ service_id = home_provider_info_->uuid();
+ } else if (serving_operator_info_->IsMobileNetworkOperatorKnown() &&
+ !serving_operator_info_->uuid().empty()) {
+ service_id = serving_operator_info_->uuid();
+ } else {
+ switch (type_) {
+ case kTypeGSM:
+ case kTypeUniversal:
+ if (!sim_identifier().empty()) {
+ service_id = sim_identifier();
+ }
+ break;
+
+ case kTypeCDMA:
+ case kTypeUniversalCDMA:
+ if (!meid().empty()) {
+ service_id = meid();
+ }
+ break;
+
+ default:
+ NOTREACHED();
+ }
+ }
+
+ if (!service_id.empty()) {
+ string storage_id = base::StringPrintf(
+ "%s_%s_%s",
+ kTypeCellular, address().c_str(), service_id.c_str());
+ service()->SetStorageIdentifier(storage_id);
+ }
+
manager()->RegisterService(service_);
+
// We might have missed a property update because the service wasn't created
// ealier.
UpdateScanning();
diff --git a/cellular.h b/cellular.h
index acef14e..7dc5206 100644
--- a/cellular.h
+++ b/cellular.h
@@ -358,8 +358,6 @@
FRIEND_TEST(CellularCapabilityUniversalMainTest,
UpdateServiceActivationState);
FRIEND_TEST(CellularCapabilityUniversalMainTest, UpdateServiceName);
- FRIEND_TEST(CellularCapabilityUniversalMainTest, UpdateStorageIdentifier);
- FRIEND_TEST(CellularServiceTest, FriendlyName);
FRIEND_TEST(CellularTest, ChangeServiceState);
FRIEND_TEST(CellularTest, ChangeServiceStatePPP);
FRIEND_TEST(CellularTest, CreateService);
@@ -409,6 +407,7 @@
FRIEND_TEST(CellularTest, StopModemCallbackFail);
FRIEND_TEST(CellularTest, StopPPPOnDisconnect);
FRIEND_TEST(CellularTest, StopPPPOnTermination);
+ FRIEND_TEST(CellularTest, StorageIdentifier);
FRIEND_TEST(CellularTest, StartConnected);
FRIEND_TEST(CellularTest, StartCDMARegister);
FRIEND_TEST(CellularTest, StartGSMRegister);
@@ -562,6 +561,7 @@
// ///////////////////////////////////////////////////////////////////////////
ModemInfo *modem_info_;
+ const Type type_;
ProxyFactory *proxy_factory_;
PPPDeviceFactory *ppp_device_factory_;
diff --git a/cellular_capability_gsm.cc b/cellular_capability_gsm.cc
index 4291a87..e1ae2a1 100644
--- a/cellular_capability_gsm.cc
+++ b/cellular_capability_gsm.cc
@@ -209,12 +209,6 @@
}
void CellularCapabilityGSM::OnServiceCreated() {
- // If IMSI is available, base the service's storage identifier on it.
- if (!cellular()->imsi().empty()) {
- cellular()->service()->SetStorageIdentifier(
- string(kTypeCellular) + "_" + cellular()->address() + "_" +
- cellular()->imsi());
- }
cellular()->service()->SetActivationState(kActivationStateActivated);
UpdateServingOperator();
}
diff --git a/cellular_capability_gsm_unittest.cc b/cellular_capability_gsm_unittest.cc
index 68f2d42..3658b7c 100644
--- a/cellular_capability_gsm_unittest.cc
+++ b/cellular_capability_gsm_unittest.cc
@@ -712,18 +712,6 @@
EXPECT_EQ(kRoamingStateUnknown, capability_->GetRoamingStateString());
}
-TEST_F(CellularCapabilityGSMTest, SetStorageIdentifier) {
- SetService();
- capability_->OnServiceCreated();
- EXPECT_EQ(string(kTypeCellular) + "_" + kAddress + "_" +
- cellular_->service()->friendly_name(),
- cellular_->service()->GetStorageIdentifier());
- cellular_->set_imsi(kIMSI);
- capability_->OnServiceCreated();
- EXPECT_EQ(string(kTypeCellular) + "_" + kAddress + "_" + kIMSI,
- cellular_->service()->GetStorageIdentifier());
-}
-
MATCHER_P(KeyValueStoreEq, value, "") {
bool match = value.bool_properties() == arg.bool_properties() &&
value.int_properties() == arg.int_properties() &&
diff --git a/cellular_capability_universal.cc b/cellular_capability_universal.cc
index a9a7dcf..6caa172 100644
--- a/cellular_capability_universal.cc
+++ b/cellular_capability_universal.cc
@@ -647,32 +647,6 @@
sim_proxy_.reset();
}
-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 kPrefix =
- string(shill::kTypeCellular) + "_" + cellular()->address() + "_";
- string storage_id;
- if (!operator_id_.empty()) {
- const CellularOperatorInfo::CellularOperator *provider =
- modem_info()->cellular_operator_info()->GetCellularOperatorByMCCMNC(
- operator_id_);
- if (provider && !provider->identifier().empty()) {
- storage_id = kPrefix + provider->identifier();
- }
- }
- // If the above didn't work, append IMSI, if available.
- if (storage_id.empty() && !cellular()->imsi().empty()) {
- storage_id = kPrefix + cellular()->imsi();
- }
- if (!storage_id.empty()) {
- cellular()->service()->SetStorageIdentifier(storage_id);
- }
-}
-
void CellularCapabilityUniversal::UpdateServiceActivationState() {
if (!cellular()->service().get())
return;
@@ -713,7 +687,6 @@
}
void CellularCapabilityUniversal::OnServiceCreated() {
- UpdateStorageIdentifier();
UpdateServiceActivationState();
UpdateServingOperator();
UpdateOLP();
diff --git a/cellular_capability_universal.h b/cellular_capability_universal.h
index a82c1a7..513b223 100644
--- a/cellular_capability_universal.h
+++ b/cellular_capability_universal.h
@@ -130,9 +130,6 @@
// Post-payment activation handlers.
virtual void UpdatePendingActivationState();
- // Updates the storage identifier used for the current cellular service.
- virtual void UpdateStorageIdentifier();
-
// Returns the operator-specific form of |mdn|, which is passed to the online
// payment portal of a cellular operator.
std::string GetMdnForOLP(
@@ -181,8 +178,6 @@
friend class CellularCapabilityUniversalCDMATest;
FRIEND_TEST(CellularCapabilityUniversalCDMAMainTest, PropertiesChanged);
FRIEND_TEST(CellularCapabilityUniversalCDMAMainTest, UpdateOLP);
- FRIEND_TEST(CellularCapabilityUniversalCDMAMainTest,
- UpdateStorageIdentifier);
FRIEND_TEST(CellularCapabilityUniversalMainTest, AllowRoaming);
FRIEND_TEST(CellularCapabilityUniversalMainTest,
ActivationWaitForRegisterTimeout);
@@ -241,7 +236,6 @@
UpdateRegistrationStateModemNotConnected);
FRIEND_TEST(CellularCapabilityUniversalMainTest,
UpdateServiceActivationState);
- FRIEND_TEST(CellularCapabilityUniversalMainTest, UpdateStorageIdentifier);
FRIEND_TEST(CellularCapabilityUniversalMainTest, UpdateOLP);
FRIEND_TEST(CellularCapabilityUniversalMainTest, UpdateOperatorInfo);
FRIEND_TEST(CellularCapabilityUniversalMainTest,
diff --git a/cellular_capability_universal_cdma.cc b/cellular_capability_universal_cdma.cc
index 6959db2..09f7967 100644
--- a/cellular_capability_universal_cdma.cc
+++ b/cellular_capability_universal_cdma.cc
@@ -184,28 +184,8 @@
return (activation_state_ == MM_MODEM_CDMA_ACTIVATION_STATE_ACTIVATED);
}
-void CellularCapabilityUniversalCDMA::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 CellularOperatorInfo::CellularOperator *provider =
- modem_info()->cellular_operator_info()->GetCellularOperatorBySID(
- UintToString(sid_));
- if (!provider || provider->identifier().empty())
- // Don't update the identifier if a better one could not be built. The
- // default identifier initialized in cellular_service.cc still applies
- // here.
- return;
- cellular()->service()->SetStorageIdentifier(
- string(shill::kTypeCellular) + "_" + cellular()->address() +
- "_" + provider->identifier());
-}
-
void CellularCapabilityUniversalCDMA::OnServiceCreated() {
SLOG(Cellular, 2) << __func__;
- UpdateStorageIdentifier();
UpdateServiceActivationStateProperty();
UpdateServingOperator();
HandleNewActivationStatus(MM_CDMA_ACTIVATION_ERROR_NONE);
diff --git a/cellular_capability_universal_cdma.h b/cellular_capability_universal_cdma.h
index c9d2a16..958f9d7 100644
--- a/cellular_capability_universal_cdma.h
+++ b/cellular_capability_universal_cdma.h
@@ -73,9 +73,6 @@
// Post-payment activation handlers.
virtual void UpdatePendingActivationState() override;
- // Updates the storage identifier used for the current cellular service.
- virtual void UpdateStorageIdentifier() override;
-
private:
friend class CellularCapabilityUniversalCDMATest;
FRIEND_TEST(CellularCapabilityUniversalCDMADispatcherTest,
@@ -92,8 +89,6 @@
FRIEND_TEST(CellularCapabilityUniversalCDMAMainTest, UpdateOperatorInfo);
FRIEND_TEST(CellularCapabilityUniversalCDMAMainTest,
UpdateServiceActivationStateProperty);
- FRIEND_TEST(CellularCapabilityUniversalCDMAMainTest,
- UpdateStorageIdentifier);
// CDMA property change handlers
virtual void OnModemCDMAPropertiesChanged(
diff --git a/cellular_capability_universal_cdma_unittest.cc b/cellular_capability_universal_cdma_unittest.cc
index 1f324cc..f527f10 100644
--- a/cellular_capability_universal_cdma_unittest.cc
+++ b/cellular_capability_universal_cdma_unittest.cc
@@ -578,45 +578,6 @@
EXPECT_STREQ("#777", map["number"].reader().get_string());
}
-TEST_F(CellularCapabilityUniversalCDMAMainTest, UpdateStorageIdentifier) {
- ClearService();
- EXPECT_FALSE(cellular_->service().get());
- capability_->UpdateStorageIdentifier();
- EXPECT_FALSE(cellular_->service().get());
-
- SetService();
- EXPECT_TRUE(cellular_->service().get());
-
- const string kPrefix =
- string(shill::kTypeCellular) + "_" + string(kMachineAddress) + "_";
-
- // GetCellularOperatorBySID returns NULL.
- EXPECT_CALL(*modem_info_.mock_cellular_operator_info(),
- GetCellularOperatorBySID(_))
- .WillOnce(Return(nullptr));
- capability_->UpdateStorageIdentifier();
- EXPECT_EQ(kPrefix + cellular_->service()->friendly_name(),
- cellular_->service()->GetStorageIdentifier());
- Mock::VerifyAndClearExpectations(modem_info_.mock_cellular_operator_info());
-
- CellularOperatorInfo::CellularOperator provider;
- EXPECT_CALL(*modem_info_.mock_cellular_operator_info(),
- GetCellularOperatorBySID(_))
- .Times(2)
- .WillRepeatedly(Return(&provider));
-
- // |provider.identifier_| is empty.
- capability_->UpdateStorageIdentifier();
- EXPECT_EQ(kPrefix + cellular_->service()->friendly_name(),
- cellular_->service()->GetStorageIdentifier());
-
- // Success.
- provider.identifier_ = "testidentifier";
- capability_->UpdateStorageIdentifier();
- EXPECT_EQ(kPrefix + "testidentifier",
- cellular_->service()->GetStorageIdentifier());
-}
-
TEST_F(CellularCapabilityUniversalCDMADispatcherTest,
UpdatePendingActivationState) {
capability_->activation_state_ = MM_MODEM_CDMA_ACTIVATION_STATE_ACTIVATED;
diff --git a/cellular_capability_universal_unittest.cc b/cellular_capability_universal_unittest.cc
index dbdf55c..d9f65c3 100644
--- a/cellular_capability_universal_unittest.cc
+++ b/cellular_capability_universal_unittest.cc
@@ -79,7 +79,7 @@
device_adaptor_(NULL),
cellular_(new Cellular(&modem_info_,
"",
- kMachineAddress,
+ "00:01:02:03:04:05",
0,
Cellular::kTypeUniversal,
"",
@@ -212,7 +212,6 @@
static const char kActiveBearerPathPrefix[];
static const char kImei[];
static const char kInactiveBearerPathPrefix[];
- static const char kMachineAddress[];
static const char kSimPath[];
static const uint32 kAccessTechnologies;
static const char kTestMobileProviderDBPath[];
@@ -343,8 +342,6 @@
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 |
@@ -1564,61 +1561,6 @@
EXPECT_TRUE(cellular_->provider_requires_roaming());
}
-TEST_F(CellularCapabilityUniversalMainTest, UpdateStorageIdentifier) {
- CellularOperatorInfo::CellularOperator provider;
-
- ClearService();
- EXPECT_FALSE(cellular_->service().get());
- capability_->UpdateStorageIdentifier();
- EXPECT_FALSE(cellular_->service().get());
-
- SetService();
- EXPECT_TRUE(cellular_->service().get());
-
- const string prefix = "cellular_" + string(kMachineAddress) + "_";
-
- // |capability_->operator_id_| is "".
- capability_->UpdateStorageIdentifier();
- EXPECT_EQ(prefix + cellular_->service()->friendly_name(),
- cellular_->service()->GetStorageIdentifier());
-
- // GetCellularOperatorByMCCMNC returns NULL.
- capability_->operator_id_ = "1";
- EXPECT_CALL(*modem_info_.mock_cellular_operator_info(),
- GetCellularOperatorByMCCMNC(capability_->operator_id_))
- .WillOnce(Return(nullptr));
-
- capability_->UpdateStorageIdentifier();
- EXPECT_EQ(prefix + cellular_->service()->friendly_name(),
- cellular_->service()->GetStorageIdentifier());
- Mock::VerifyAndClearExpectations(modem_info_.mock_cellular_operator_info());
-
- // |cellular_->imsi_| is not ""
- cellular_->set_imsi("TESTIMSI");
- EXPECT_CALL(*modem_info_.mock_cellular_operator_info(),
- GetCellularOperatorByMCCMNC(capability_->operator_id_))
- .WillOnce(Return(nullptr));
-
- capability_->UpdateStorageIdentifier();
- EXPECT_EQ(prefix + "TESTIMSI", cellular_->service()->storage_identifier_);
- Mock::VerifyAndClearExpectations(modem_info_.mock_cellular_operator_info());
-
- EXPECT_CALL(*modem_info_.mock_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(CellularCapabilityUniversalMainTest, GetMdnForOLP) {
CellularOperatorInfo::CellularOperator cellular_operator;
diff --git a/cellular_service.cc b/cellular_service.cc
index be41575..78607fc 100644
--- a/cellular_service.cc
+++ b/cellular_service.cc
@@ -131,7 +131,7 @@
set_friendly_name(cellular_->CreateDefaultFriendlyServiceName());
SetStorageIdentifier(string(kTypeCellular) + "_" +
- cellular_->address() + "_" + friendly_name());
+ cellular_->address() + "_" + friendly_name());
// Assume we are not performing any out-of-credits detection.
// The capability can reinitialize with the appropriate type later.
InitOutOfCreditsDetection(OutOfCreditsDetector::OOCTypeNone);
@@ -382,6 +382,7 @@
}
void CellularService::SetStorageIdentifier(const string &identifier) {
+ SLOG(Cellular, 3) << __func__ << ": " << identifier;
storage_identifier_ = identifier;
std::replace_if(storage_identifier_.begin(),
storage_identifier_.end(),
diff --git a/cellular_service.h b/cellular_service.h
index 6e271f4..910763b 100644
--- a/cellular_service.h
+++ b/cellular_service.h
@@ -143,7 +143,6 @@
FRIEND_TEST(CellularCapabilityUniversalMainTest,
UpdatePendingActivationState);
FRIEND_TEST(CellularCapabilityUniversalMainTest, UpdateServiceName);
- FRIEND_TEST(CellularCapabilityUniversalMainTest, UpdateStorageIdentifier);
FRIEND_TEST(CellularTest, Connect);
FRIEND_TEST(CellularTest, GetLogin); // ppp_username_, ppp_password_
FRIEND_TEST(CellularTest, OnConnectionHealthCheckerResult);
diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index 19c88dd..df15be9 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -767,21 +767,21 @@
}
TEST_F(CellularTest, ServiceFriendlyName) {
+ // Test that the name created for the service is sensible under different
+ // scenarios w.r.t. information about the mobile network operator.
SetMockMobileOperatorInfoObjects();
CHECK(mock_home_provider_info_);
CHECK(mock_serving_operator_info_);
- // Test that the name created for the service is sensible under different
- // scenarios w.r.t. information about the mobile network operator.
const string home_provider_name {"HomeProviderName"};
const string serving_operator_name {"ServingOperatorName"};
SetCellularType(Cellular::kTypeCDMA);
// (1) Service created, MNO not known => Default name.
EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
- .WillOnce(Return(false));
+ .WillRepeatedly(Return(false));
EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
- .WillOnce(Return(false));
+ .WillRepeatedly(Return(false));
device_->CreateService();
// Compare substrings explicitly using EXPECT_EQ for better error message.
size_t prefix_len = strlen(Cellular::kGenericServiceNamePrefix);
@@ -793,16 +793,18 @@
// (2) Service created, then home provider determined => Name provided by
// home provider.
- mock_home_provider_info_->SetEmptyDefaultsForProperties();
- EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
- .WillOnce(Return(false))
- .WillOnce(Return(true));
EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
.WillRepeatedly(Return(false));
- EXPECT_CALL(*mock_home_provider_info_, operator_name())
- .WillRepeatedly(ReturnRef(home_provider_name));
+ EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(false));
device_->CreateService();
// Now emulate an event for updated home provider information.
+ Mock::VerifyAndClearExpectations(mock_home_provider_info_);
+ mock_home_provider_info_->SetEmptyDefaultsForProperties();
+ EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_home_provider_info_, operator_name())
+ .WillRepeatedly(ReturnRef(home_provider_name));
device_->mobile_operator_info_observer_->OnOperatorChanged();
EXPECT_EQ(home_provider_name, device_->service_->friendly_name());
Mock::VerifyAndClearExpectations(mock_home_provider_info_);
@@ -811,16 +813,18 @@
// (3) Service created, then serving operator determined => Name provided by
// serving operator.
- mock_serving_operator_info_->SetEmptyDefaultsForProperties();
- EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
- .WillOnce(Return(false))
- .WillOnce(Return(true));
EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
.WillRepeatedly(Return(false));
- EXPECT_CALL(*mock_serving_operator_info_, operator_name())
- .WillRepeatedly(ReturnRef(serving_operator_name));
+ EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(false));
device_->CreateService();
// Now emulate an event for updated serving operator information.
+ Mock::VerifyAndClearExpectations(mock_serving_operator_info_);
+ mock_serving_operator_info_->SetEmptyDefaultsForProperties();
+ EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_serving_operator_info_, operator_name())
+ .WillRepeatedly(ReturnRef(serving_operator_name));
device_->mobile_operator_info_observer_->OnOperatorChanged();
EXPECT_EQ(serving_operator_name, device_->service_->friendly_name());
Mock::VerifyAndClearExpectations(mock_home_provider_info_);
@@ -829,22 +833,26 @@
// (4) Service created, then home provider determined, then serving operator
// determined => final name is serving operator.
- mock_home_provider_info_->SetEmptyDefaultsForProperties();
- mock_serving_operator_info_->SetEmptyDefaultsForProperties();
- EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
- .WillOnce(Return(false))
- .WillOnce(Return(true))
- .WillOnce(Return(true));
EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
- .WillOnce(Return(false))
- .WillOnce(Return(false))
- .WillOnce(Return(true));
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(false));
+ device_->CreateService();
+ // Now emulate an event for updated home provider information.
+ Mock::VerifyAndClearExpectations(mock_home_provider_info_);
+ mock_home_provider_info_->SetEmptyDefaultsForProperties();
+ EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(true));
EXPECT_CALL(*mock_home_provider_info_, operator_name())
.WillRepeatedly(ReturnRef(home_provider_name));
+ device_->mobile_operator_info_observer_->OnOperatorChanged();
+ // Now emulate an event for updated serving operator information.
+ Mock::VerifyAndClearExpectations(mock_serving_operator_info_);
+ mock_serving_operator_info_->SetEmptyDefaultsForProperties();
+ EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(true));
EXPECT_CALL(*mock_serving_operator_info_, operator_name())
.WillRepeatedly(ReturnRef(serving_operator_name));
- device_->CreateService();
- device_->mobile_operator_info_observer_->OnOperatorChanged();
device_->mobile_operator_info_observer_->OnOperatorChanged();
EXPECT_EQ(serving_operator_name, device_->service_->friendly_name());
Mock::VerifyAndClearExpectations(mock_home_provider_info_);
@@ -853,22 +861,26 @@
// (5) Service created, then serving operator determined, then home provider
// determined => final name is serving operator.
- mock_home_provider_info_->SetEmptyDefaultsForProperties();
+ EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(false));
+ device_->CreateService();
+ // Now emulate an event for updated serving operator information.
+ Mock::VerifyAndClearExpectations(mock_serving_operator_info_);
mock_serving_operator_info_->SetEmptyDefaultsForProperties();
EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
- .WillOnce(Return(false))
- .WillOnce(Return(false))
- .WillOnce(Return(true));
- EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
- .WillOnce(Return(false))
- .WillOnce(Return(true))
- .WillOnce(Return(true));
- EXPECT_CALL(*mock_home_provider_info_, operator_name())
- .WillRepeatedly(ReturnRef(home_provider_name));
+ .WillRepeatedly(Return(true));
EXPECT_CALL(*mock_serving_operator_info_, operator_name())
.WillRepeatedly(ReturnRef(serving_operator_name));
- device_->CreateService();
device_->mobile_operator_info_observer_->OnOperatorChanged();
+ // Now emulate an event for updated home provider information.
+ Mock::VerifyAndClearExpectations(mock_home_provider_info_);
+ mock_home_provider_info_->SetEmptyDefaultsForProperties();
+ EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_home_provider_info_, operator_name())
+ .WillRepeatedly(ReturnRef(home_provider_name));
device_->mobile_operator_info_observer_->OnOperatorChanged();
EXPECT_EQ(serving_operator_name, device_->service_->friendly_name());
Mock::VerifyAndClearExpectations(mock_home_provider_info_);
@@ -880,9 +892,9 @@
mock_home_provider_info_->SetEmptyDefaultsForProperties();
mock_serving_operator_info_->SetEmptyDefaultsForProperties();
EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
- .WillOnce(Return(true));
+ .WillRepeatedly(Return(true));
EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
- .WillOnce(Return(true));
+ .WillRepeatedly(Return(true));
EXPECT_CALL(*mock_home_provider_info_, operator_name())
.WillRepeatedly(ReturnRef(home_provider_name));
EXPECT_CALL(*mock_serving_operator_info_, operator_name())
@@ -891,6 +903,88 @@
EXPECT_EQ(serving_operator_name, device_->service_->friendly_name());
}
+static bool IllegalChar(char a) {
+ return !(isalnum(a) || a == '_');
+}
+
+TEST_F(CellularTest, StorageIdentifier) {
+ // Test that the storage identifier name used by the service is sensible under
+ // different scenarios w.r.t. information about the mobile network operator.
+ SetMockMobileOperatorInfoObjects();
+ mock_home_provider_info_->SetEmptyDefaultsForProperties();
+ mock_serving_operator_info_->SetEmptyDefaultsForProperties();
+ CHECK(mock_home_provider_info_);
+ CHECK(mock_serving_operator_info_);
+
+ // See cellular_service.cc
+ string prefix = string(kTypeCellular) + "_" +
+ string(kTestDeviceAddress) + "_";
+ // Service replaces ':' with '_'
+ std::replace_if(prefix.begin(), prefix.end(), &IllegalChar, '_');
+ const string kUuidHomeProvider = "uuidHomeProvider";
+ const string kUuidServingOperator = "uuidServingOperator";
+ const string kSimIdentifier = "12345123451234512345";
+
+ SetCellularType(Cellular::kTypeCDMA);
+ ON_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
+ .WillByDefault(Return(false));
+
+ // (1) Service created, both home provider and serving operator known =>
+ // home provider used.
+ mock_home_provider_info_->SetEmptyDefaultsForProperties();
+ mock_serving_operator_info_->SetEmptyDefaultsForProperties();
+ EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_home_provider_info_, uuid())
+ .WillRepeatedly(ReturnRef(kUuidHomeProvider));
+ EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_serving_operator_info_, uuid())
+ .WillRepeatedly(ReturnRef(kUuidServingOperator));
+ device_->CreateService();
+ EXPECT_EQ(prefix + kUuidHomeProvider,
+ device_->service()->GetStorageIdentifier());
+ Mock::VerifyAndClearExpectations(mock_home_provider_info_);
+ Mock::VerifyAndClearExpectations(mock_serving_operator_info_);
+ device_->DestroyService();
+
+ // Common expectation for following tests:
+ EXPECT_CALL(*mock_home_provider_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(false));
+
+ // (2) Service created, no extra information => Default storage_id;
+ EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(false));
+ device_->CreateService();
+ EXPECT_EQ(prefix + device_->service()->friendly_name(),
+ device_->service()->GetStorageIdentifier());
+ Mock::VerifyAndClearExpectations(mock_serving_operator_info_);
+ device_->DestroyService();
+
+ // (3) Service created, serving operator known, uuid known.
+ mock_serving_operator_info_->SetEmptyDefaultsForProperties();
+ EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_serving_operator_info_, uuid())
+ .WillRepeatedly(ReturnRef(kUuidServingOperator));
+ device_->CreateService();
+ EXPECT_EQ(prefix + kUuidServingOperator,
+ device_->service()->GetStorageIdentifier());
+ Mock::VerifyAndClearExpectations(mock_serving_operator_info_);
+ device_->DestroyService();
+
+ // (4) Service created, serving operator known, uuid not known, iccid known.
+ mock_serving_operator_info_->SetEmptyDefaultsForProperties();
+ EXPECT_CALL(*mock_serving_operator_info_, IsMobileNetworkOperatorKnown())
+ .WillRepeatedly(Return(true));
+ device_->set_sim_identifier(kSimIdentifier);
+ device_->CreateService();
+ EXPECT_EQ(prefix + kSimIdentifier,
+ device_->service()->GetStorageIdentifier());
+ Mock::VerifyAndClearExpectations(mock_serving_operator_info_);
+ device_->DestroyService();
+}
+
namespace {
MATCHER(ContainsPhoneNumber, "") {
diff --git a/mobile_operator_info.h b/mobile_operator_info.h
index dd0d444..a180127 100644
--- a/mobile_operator_info.h
+++ b/mobile_operator_info.h
@@ -142,7 +142,7 @@
// The unique identifier of this carrier. This is primarily used to
// identify the user profile in store for each carrier. This identifier is
// access technology agnostic and should be the same across 3GPP and CDMA.
- const std::string &uuid() const;
+ virtual const std::string &uuid() const;
virtual const std::string &operator_name() const;
const std::string &country() const;
diff --git a/mock_mobile_operator_info.cc b/mock_mobile_operator_info.cc
index b17a9e2..c4b6ab9 100644
--- a/mock_mobile_operator_info.cc
+++ b/mock_mobile_operator_info.cc
@@ -17,6 +17,7 @@
ON_CALL(*this, mccmnc()).WillByDefault(ReturnRef(empty_mccmnc_));
ON_CALL(*this, operator_name())
.WillByDefault(ReturnRef(empty_operator_name_));
+ ON_CALL(*this, uuid()).WillByDefault(ReturnRef(empty_uuid_));
}
} // namespace shill
diff --git a/mock_mobile_operator_info.h b/mock_mobile_operator_info.h
index 44c6b60..49cbb2a 100644
--- a/mock_mobile_operator_info.h
+++ b/mock_mobile_operator_info.h
@@ -24,6 +24,7 @@
MOCK_CONST_METHOD0(mccmnc, const std::string &());
MOCK_CONST_METHOD0(operator_name, const std::string &());
+ MOCK_CONST_METHOD0(uuid, const std::string &());
// Sets up the mock object to return empty strings/vectors etc for all
// propeties.
@@ -32,6 +33,7 @@
private:
std::string empty_mccmnc_;
std::string empty_operator_name_;
+ std::string empty_uuid_;
};
} // namespace shill
diff --git a/service.h b/service.h
index df38cc7..0d7f04a 100644
--- a/service.h
+++ b/service.h
@@ -583,9 +583,6 @@
friend void TestNamePropertyChange(ServiceRefPtr, ServiceMockAdaptor *);
FRIEND_TEST(AllMockServiceTest, AutoConnectWithFailures);
FRIEND_TEST(CellularCapabilityGSMTest, SetStorageIdentifier);
- FRIEND_TEST(CellularCapabilityUniversalCDMAMainTest,
- UpdateStorageIdentifier);
- FRIEND_TEST(CellularCapabilityUniversalMainTest, UpdateStorageIdentifier);
FRIEND_TEST(CellularServiceTest, IsAutoConnectable);
FRIEND_TEST(DeviceTest, IPConfigUpdatedFailureWithStatic);
FRIEND_TEST(ManagerTest, ConnectToBestServices);
