shill: Service: Return loadable service identifier
Add a Service method which returns the storage identifier from
a given StorageInterface which could be used to configure this
service. Override this method for WiFiService since it uses
a different method than default to decide a loadable profile
entry.
BUG=chromium:235674
TEST=Unit tests
Change-Id: I6d1bc3cc7e472d3598ac9299b3d76d33820ce0fe
Reviewed-on: https://gerrit.chromium.org/gerrit/49269
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/profile.cc b/profile.cc
index f719243..c88d835 100644
--- a/profile.cc
+++ b/profile.cc
@@ -194,7 +194,7 @@
}
bool Profile::ContainsService(const ServiceConstRefPtr &service) {
- return service->IsLoadableFrom(storage_.get());
+ return service->IsLoadableFrom(*storage_.get());
}
void Profile::DeleteEntry(const std::string &entry_name, Error *error) {
diff --git a/service.cc b/service.cc
index 49e2ea4..0301dcc 100644
--- a/service.cc
+++ b/service.cc
@@ -384,8 +384,13 @@
return adaptor_->GetRpcIdentifier();
}
-bool Service::IsLoadableFrom(StoreInterface *storage) const {
- return storage->ContainsGroup(GetStorageIdentifier());
+string Service::GetLoadableStorageIdentifier(
+ const StoreInterface &storage) const {
+ return IsLoadableFrom(storage) ? GetStorageIdentifier() : "";
+}
+
+bool Service::IsLoadableFrom(const StoreInterface &storage) const {
+ return storage.ContainsGroup(GetStorageIdentifier());
}
bool Service::Load(StoreInterface *storage) {
diff --git a/service.h b/service.h
index aabe624..92afd4a 100644
--- a/service.h
+++ b/service.h
@@ -209,8 +209,14 @@
// Returns the unique persistent storage identifier for the service.
virtual std::string GetStorageIdentifier() const = 0;
+ // Returns the identifier within |storage| from which configuration for
+ // this service can be loaded. Returns an empty string if no entry in
+ // |storage| can be used.
+ virtual std::string GetLoadableStorageIdentifier(
+ const StoreInterface &storage) const;
+
// Returns whether the service configuration can be loaded from |storage|.
- virtual bool IsLoadableFrom(StoreInterface *storage) const;
+ virtual bool IsLoadableFrom(const StoreInterface &storage) const;
// Returns true if the service uses 802.1x for key management.
virtual bool Is8021x() const { return false; }
diff --git a/service_unittest.cc b/service_unittest.cc
index 4c672f4..fe78890 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -363,6 +363,24 @@
}
}
+TEST_F(ServiceTest, GetLoadableStorageIdentifier) {
+ NiceMock<MockStore> storage;
+ EXPECT_CALL(storage, ContainsGroup(storage_id_))
+ .WillOnce(Return(false))
+ .WillOnce(Return(true));
+ EXPECT_EQ("", service_->GetLoadableStorageIdentifier(storage));
+ EXPECT_EQ(storage_id_, service_->GetLoadableStorageIdentifier(storage));
+}
+
+TEST_F(ServiceTest, IsLoadableFrom) {
+ NiceMock<MockStore> storage;
+ EXPECT_CALL(storage, ContainsGroup(storage_id_))
+ .WillOnce(Return(false))
+ .WillOnce(Return(true));
+ EXPECT_FALSE(service_->IsLoadableFrom(storage));
+ EXPECT_TRUE(service_->IsLoadableFrom(storage));
+}
+
TEST_F(ServiceTest, Load) {
NiceMock<MockStore> storage;
EXPECT_CALL(storage, ContainsGroup(storage_id_)).WillOnce(Return(true));
diff --git a/wifi_service.cc b/wifi_service.cc
index 70c8310..7cfee73 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -243,8 +243,25 @@
UpdateConnectable();
}
-bool WiFiService::IsLoadableFrom(StoreInterface *storage) const {
- return !storage->GetGroupsWithProperties(GetStorageProperties()).empty();
+string WiFiService::GetLoadableStorageIdentifier(
+ const StoreInterface &storage) const {
+ set<string> groups = storage.GetGroupsWithProperties(GetStorageProperties());
+ if (groups.empty()) {
+ LOG(WARNING) << "Configuration for service "
+ << unique_name()
+ << " is not available in the persistent store";
+ return "";
+ }
+ if (groups.size() > 1) {
+ LOG(WARNING) << "More than one configuration for service "
+ << unique_name()
+ << " is available; choosing the first.";
+ }
+ return *groups.begin();
+}
+
+bool WiFiService::IsLoadableFrom(const StoreInterface &storage) const {
+ return !storage.GetGroupsWithProperties(GetStorageProperties()).empty();
}
bool WiFiService::IsVisible() const {
@@ -255,21 +272,10 @@
}
bool WiFiService::Load(StoreInterface *storage) {
- // First find out which storage identifier is available in priority order
- // of specific, generic.
- set<string> groups = storage->GetGroupsWithProperties(GetStorageProperties());
- if (groups.empty()) {
- LOG(WARNING) << "Configuration for service "
- << unique_name()
- << " is not available in the persistent store";
+ string id = GetLoadableStorageIdentifier(*storage);
+ if (id.empty()) {
return false;
}
- if (groups.size() > 1) {
- LOG(WARNING) << "More than one configuration for service "
- << unique_name()
- << " is available; choosing the first.";
- }
- string id = *groups.begin();
// Set our storage identifier to match the storage name in the Profile.
storage_identifier_ = id;
diff --git a/wifi_service.h b/wifi_service.h
index caeae84..cbb10f5 100644
--- a/wifi_service.h
+++ b/wifi_service.h
@@ -91,9 +91,15 @@
const std::string &bssid() const { return bssid_; }
uint16 physical_mode() const { return physical_mode_; }
+ // WiFi services can load from profile entries other than their current
+ // storage identifier. Override the methods from the parent Service
+ // class which pertain to whether this service may be loaded from |storage|.
+ virtual std::string GetLoadableStorageIdentifier(
+ const StoreInterface &storage) const;
+ virtual bool IsLoadableFrom(const StoreInterface &storage) const;
+
// Overrride Load and Save from parent Service class. We will call
// the parent method.
- virtual bool IsLoadableFrom(StoreInterface *storage) const;
virtual bool Load(StoreInterface *storage);
virtual bool Save(StoreInterface *storage);
virtual bool Unload();
diff --git a/wifi_service_unittest.cc b/wifi_service_unittest.cc
index 4c3e88a..f606eaf 100644
--- a/wifi_service_unittest.cc
+++ b/wifi_service_unittest.cc
@@ -246,10 +246,13 @@
flimflam::kModeManaged,
storage_security)))
.WillRepeatedly(Return(groups));
- bool is_loadable = wifi_service->IsLoadableFrom(&mock_store);
+ bool is_loadable = wifi_service->IsLoadableFrom(mock_store);
EXPECT_EQ(expectation, is_loadable);
bool is_loaded = wifi_service->Load(&mock_store);
EXPECT_EQ(expectation, is_loaded);
+ const string expected_identifier(expectation ? kStorageId : "");
+ EXPECT_EQ(expected_identifier,
+ wifi_service->GetLoadableStorageIdentifier(mock_store));
if (expectation != is_loadable || expectation != is_loaded) {
return false;