shill: Implement more of Profile DBus interface
Return a DBus path from Manager.GetActiveProfile().
Implement the "Profiles" property on the manager Manager.
Fix the "Entries" property on the Profile to only report group
identifiers that correspond to technologies (not ipconfig,
devices, etc).
Fix the "Services" Profile property, to only appear as a property
of the active profile.
BUG=chromium-os:25538, chromium-os:23702
TEST=Manual: Running "list-profiles" from the flimflam test suite now
works correctly.
Change-Id: I3120fe54f02662822186ac033fab0b3566449705
Reviewed-on: https://gerrit.chromium.org/gerrit/14904
Commit-Ready: Paul Stewart <pstew@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/manager.cc b/manager.cc
index 459c4ec..e743d8b 100644
--- a/manager.cc
+++ b/manager.cc
@@ -71,7 +71,7 @@
metrics_(metrics),
glib_(glib) {
HelpRegisterDerivedString(flimflam::kActiveProfileProperty,
- &Manager::GetActiveProfileName,
+ &Manager::GetActiveProfileRpcIdentifier,
NULL);
HelpRegisterDerivedStrings(flimflam::kAvailableTechnologiesProperty,
&Manager::AvailableTechnologies,
@@ -93,6 +93,9 @@
NULL);
store_.RegisterBool(flimflam::kOfflineModeProperty, &props_.offline_mode);
store_.RegisterString(flimflam::kPortalURLProperty, &props_.portal_url);
+ HelpRegisterDerivedStrings(flimflam::kProfilesProperty,
+ &Manager::EnumerateProfiles,
+ NULL);
store_.RegisterString(shill::kHostNameProperty, &props_.host_name);
HelpRegisterDerivedString(flimflam::kStateProperty,
&Manager::CalculateState,
@@ -104,8 +107,6 @@
&Manager::EnumerateWatchedServices,
NULL);
- // TODO(cmasone): Wire these up once we actually put in profile support.
- // known_properties_.push_back(flimflam::kProfilesProperty);
VLOG(2) << "Manager initialized.";
}
@@ -310,11 +311,16 @@
PopProfileInternal();
}
-const ProfileRefPtr &Manager::ActiveProfile() {
+const ProfileRefPtr &Manager::ActiveProfile() const {
DCHECK_NE(profiles_.size(), 0);
return profiles_.back();
}
+bool Manager::IsActiveProfile(const ProfileRefPtr &profile) const {
+ return (profiles_.size() > 0 &&
+ ActiveProfile().get() == profile.get());
+}
+
bool Manager::MoveServiceToProfile(const ServiceRefPtr &to_move,
const ProfileRefPtr &destination) {
const ProfileRefPtr from = to_move->profile();
@@ -633,6 +639,16 @@
return device_rpc_ids;
}
+vector<string> Manager::EnumerateProfiles(Error */*error*/) {
+ vector<string> profile_rpc_ids;
+ for (vector<ProfileRefPtr>::const_iterator it = profiles_.begin();
+ it != profiles_.end();
+ ++it) {
+ profile_rpc_ids.push_back((*it)->GetRpcIdentifier());
+ }
+ return profile_rpc_ids;
+}
+
vector<string> Manager::EnumerateAvailableServices(Error */*error*/) {
vector<string> service_rpc_ids;
for (vector<ServiceRefPtr>::const_iterator it = services_.begin();
@@ -648,8 +664,8 @@
return EnumerateAvailableServices(error);
}
-string Manager::GetActiveProfileName(Error */*error*/) {
- return ActiveProfile()->GetFriendlyName();
+string Manager::GetActiveProfileRpcIdentifier(Error */*error*/) {
+ return ActiveProfile()->GetRpcIdentifier();
}
// called via RPC (e.g., from ManagerDBusAdaptor)
diff --git a/manager.h b/manager.h
index 085abf3..f61d4ce 100644
--- a/manager.h
+++ b/manager.h
@@ -56,7 +56,8 @@
void Start();
void Stop();
- const ProfileRefPtr &ActiveProfile();
+ const ProfileRefPtr &ActiveProfile() const;
+ bool IsActiveProfile(const ProfileRefPtr &profile) const;
bool MoveServiceToProfile(const ServiceRefPtr &to_move,
const ProfileRefPtr &destination);
@@ -119,13 +120,14 @@
private:
friend class ManagerAdaptorInterface;
friend class ManagerTest;
- FRIEND_TEST(ManagerTest, DeviceRegistrationAndStart);
- FRIEND_TEST(ManagerTest, PushPopProfile);
- FRIEND_TEST(ManagerTest, SortServices);
- FRIEND_TEST(ManagerTest, SortServicesWithConnection);
FRIEND_TEST(ManagerTest, AvailableTechnologies);
FRIEND_TEST(ManagerTest, ConnectedTechnologies);
FRIEND_TEST(ManagerTest, DefaultTechnology);
+ FRIEND_TEST(ManagerTest, DeviceRegistrationAndStart);
+ FRIEND_TEST(ManagerTest, EnumerateProfiles);
+ FRIEND_TEST(ManagerTest, PushPopProfile);
+ FRIEND_TEST(ManagerTest, SortServices);
+ FRIEND_TEST(ManagerTest, SortServicesWithConnection);
static const char kManagerErrorNoDevice[];
@@ -137,9 +139,10 @@
std::string DefaultTechnology(Error *error);
std::vector<std::string> EnabledTechnologies(Error *error);
std::vector<std::string> EnumerateDevices(Error *error);
+ std::vector<std::string> EnumerateProfiles(Error *error);
// TODO(cmasone): This should be implemented by filtering |services_|.
std::vector<std::string> EnumerateWatchedServices(Error *error);
- std::string GetActiveProfileName(Error *error);
+ std::string GetActiveProfileRpcIdentifier(Error *error);
void HelpRegisterDerivedString(
const std::string &name,
diff --git a/manager_unittest.cc b/manager_unittest.cc
index 411bf51..534f7a0 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -591,7 +591,7 @@
Error error;
// Active profile should be the last one we pushed.
- EXPECT_EQ(kProfile1, "~" + manager.GetActiveProfileName(&error));
+ EXPECT_EQ(kProfile1, "~" + manager.ActiveProfile()->GetFriendlyName());
// Make sure a profile name that doesn't exist fails.
const char kProfile2Id[] = "profile2";
@@ -1109,6 +1109,26 @@
manager()->UpdateService(service);
}
+TEST_F(ManagerTest, EnumerateProfiles) {
+ vector<string> profile_paths;
+ for (size_t i = 0; i < 10; i++) {
+ scoped_refptr<MockProfile> profile(
+ new StrictMock<MockProfile>(control_interface(), manager(), ""));
+ profile_paths.push_back(base::StringPrintf("/profile/%d", i));
+ EXPECT_CALL(*profile.get(), GetRpcIdentifier())
+ .WillOnce(Return(profile_paths.back()));
+ AdoptProfile(manager(), profile);
+ }
+
+ Error error;
+ vector<string> returned_paths = manager()->EnumerateProfiles(&error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(profile_paths.size(), returned_paths.size());
+ for (size_t i = 0; i < profile_paths.size(); i++) {
+ EXPECT_EQ(profile_paths[i], returned_paths[i]);
+ }
+}
+
TEST_F(ManagerTest, AutoConnectOnRegister) {
MockServiceRefPtr service = MakeAutoConnectableService();
EXPECT_CALL(*service.get(), AutoConnect());
diff --git a/mock_service.cc b/mock_service.cc
index 1e1aca2..d4d3888 100644
--- a/mock_service.cc
+++ b/mock_service.cc
@@ -42,7 +42,7 @@
MockService::~MockService() {}
bool MockService::FauxSave(StoreInterface *store) {
- return store->SetString(UniqueName(), "dummy", "dummy");
+ return store->SetString(GetStorageIdentifier(), "dummy", "dummy");
}
} // namespace shill
diff --git a/profile.cc b/profile.cc
index 4390d23..8ff5ff0 100644
--- a/profile.cc
+++ b/profile.cc
@@ -211,17 +211,27 @@
}
vector<string> Profile::EnumerateAvailableServices(Error *error) {
- // TOOD(quiche): This list should be based on the information we
- // have about services, rather than the Manager's service
- // list. (crosbug.com/23702)
- return manager_->EnumerateAvailableServices(error);
+ // We should return the Manager's service list if this is the active profile.
+ if (manager_->IsActiveProfile(this)) {
+ return manager_->EnumerateAvailableServices(error);
+ } else {
+ return vector<string>();
+ }
}
vector<string> Profile::EnumerateEntries(Error */*error*/) {
- // TODO(someone): Determine if we care about this wasteful copying; consider
- // making GetGroups return a vector.
set<string> groups(storage_->GetGroups());
- return vector<string>(groups.begin(), groups.end());
+ vector<string> service_groups;
+
+ // Filter this list down to only entries that correspond
+ // to a technology. (wifi_*, etc)
+ for (set<string>::iterator it = groups.begin();
+ it != groups.end(); ++it) {
+ if (Technology::IdentifierFromStorageGroup(*it) != Technology::kUnknown)
+ service_groups.push_back(*it);
+ }
+
+ return service_groups;
}
void Profile::HelpRegisterDerivedStrings(
diff --git a/profile_unittest.cc b/profile_unittest.cc
index d7ef54f..9a1fa7c 100644
--- a/profile_unittest.cc
+++ b/profile_unittest.cc
@@ -219,28 +219,33 @@
TEST_F(ProfileTest, EntryEnumeration) {
scoped_refptr<MockService> service1(CreateMockService());
scoped_refptr<MockService> service2(CreateMockService());
+ string service1_storage_name = Technology::NameFromIdentifier(
+ Technology::kCellular) + "_1";
+ string service2_storage_name = Technology::NameFromIdentifier(
+ Technology::kCellular) + "_2";
EXPECT_CALL(*service1.get(), Save(_))
.WillRepeatedly(Invoke(service1.get(), &MockService::FauxSave));
EXPECT_CALL(*service2.get(), Save(_))
.WillRepeatedly(Invoke(service2.get(), &MockService::FauxSave));
+ EXPECT_CALL(*service1.get(), GetStorageIdentifier())
+ .WillRepeatedly(Return(service1_storage_name));
+ EXPECT_CALL(*service2.get(), GetStorageIdentifier())
+ .WillRepeatedly(Return(service2_storage_name));
string service1_name(service1->UniqueName());
string service2_name(service2->UniqueName());
- Error error;
ASSERT_TRUE(profile_->AdoptService(service1));
ASSERT_TRUE(profile_->AdoptService(service2));
- ASSERT_EQ(profile_->EnumerateEntries(&error).size(), 2);
+ Error error;
+ ASSERT_EQ(2, profile_->EnumerateEntries(&error).size());
ASSERT_TRUE(profile_->AbandonService(service1));
- ASSERT_EQ(profile_->EnumerateEntries(&error)[0], service2_name);
-
- ASSERT_TRUE(profile_->AbandonService(service1));
- ASSERT_EQ(profile_->EnumerateEntries(&error)[0], service2_name);
+ ASSERT_EQ(service2_storage_name, profile_->EnumerateEntries(&error)[0]);
ASSERT_TRUE(profile_->AbandonService(service2));
- ASSERT_EQ(profile_->EnumerateEntries(&error).size(), 0);
+ ASSERT_EQ(0, profile_->EnumerateEntries(&error).size());
}
TEST_F(ProfileTest, MatchesIdentifier) {
diff --git a/technology.cc b/technology.cc
index 9e42618..9a87d85 100644
--- a/technology.cc
+++ b/technology.cc
@@ -5,17 +5,20 @@
#include "shill/technology.h"
#include <string>
+#include <vector>
+#include <base/string_split.h>
#include <chromeos/dbus/service_constants.h>
namespace shill {
using std::string;
+using std::vector;
const char Technology::kUnknownName[] = "Unknown";
// static
-Technology::Identifier Technology::IdentifierFromName(const std::string &name) {
+Technology::Identifier Technology::IdentifierFromName(const string &name) {
if (name == flimflam::kTypeEthernet) {
return kEthernet;
} else if (name == flimflam::kTypeWifi) {
@@ -28,7 +31,7 @@
}
// static
-std::string Technology::NameFromIdentifier(Technology::Identifier id) {
+string Technology::NameFromIdentifier(Technology::Identifier id) {
if (id == kEthernet) {
return flimflam::kTypeEthernet;
} else if (id == kWifi) {
@@ -40,4 +43,15 @@
}
}
+// static
+Technology::Identifier Technology::IdentifierFromStorageGroup(
+ const string &group) {
+ vector<string> group_parts;
+ base::SplitString(group, '_', &group_parts);
+ if (group_parts.empty()) {
+ return kUnknown;
+ }
+ return IdentifierFromName(group_parts[0]);
+}
+
} // namespace shill
diff --git a/technology.h b/technology.h
index 827d048..2fa6c6c 100644
--- a/technology.h
+++ b/technology.h
@@ -22,6 +22,7 @@
static Identifier IdentifierFromName(const std::string &name);
static std::string NameFromIdentifier(Identifier id);
+ static Identifier IdentifierFromStorageGroup(const std::string &group);
private:
static const char kUnknownName[];