shill: Implement GetEntry on Profile

Implement "GetEntry" on profile_dbus_adaptor.  This change adds
the ability to look up a Service in the Manager by its
StorageIdentifier and then to return its properties to the caller.
If the Service does not exist in the manager, we have to read
directly out of the Profile's store and build a DBus property dict
out of its contents.  This is a pretty gross method, and I've already
created a bug to remove this as soon as we can diverge from the
flimflam Profile API.  crosbug.com/25813

BUG=chromium-os:25542
TEST=New unit tests + Manual: chrome://settings/internet now lists
both visible and unavailable networks under "Remembered Networks"
and clikcing on "Forget Network" purges the network from the profile.

Change-Id: Ib2f0ab772e40a1f615206a7b985be446fc7facde
Reviewed-on: https://gerrit.chromium.org/gerrit/15200
Commit-Ready: Paul Stewart <pstew@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>

19 files changed, 550 insertions, 79 deletions

diff --git a/Makefile b/Makefile
index 8420bd85..f3bc5ab3 100644
--- a/Makefile
+++ b/Makefile
@@ -145,6 +145,7 @@ SHILL_OBJS = \
 	power_manager_proxy.o \
 	profile.o \
 	profile_dbus_adaptor.o \
+	profile_dbus_property_exporter.o \
 	property_store.o \
 	proxy_factory.o \
 	routing_table.o \
@@ -246,6 +247,7 @@ TEST_OBJS = \
 	modem_manager_unittest.o \
 	modem_unittest.o \
 	nice_mock_control.o \
+	profile_dbus_property_exporter_unittest.o \
 	profile_unittest.o \
 	property_accessor_unittest.o \
 	property_store_unittest.o \
diff --git a/key_file_store.cc b/key_file_store.cc
index 9fc2d197..fa823aaa 100644
--- a/key_file_store.cc
+++ b/key_file_store.cc
@@ -29,7 +29,7 @@ void KeyFileStore::ReleaseKeyFile() {
   }
 }
 
-bool KeyFileStore::IsNonEmpty() {
+bool KeyFileStore::IsNonEmpty() const {
   int64 file_size = 0;
   return file_util::GetFileSize(path_, &file_size) && file_size != 0;
 }
@@ -92,7 +92,7 @@ bool KeyFileStore::Flush() {
   return success;
 }
 
-set<string> KeyFileStore::GetGroups() {
+set<string> KeyFileStore::GetGroups() const {
   CHECK(key_file_);
   gsize length = 0;
   gchar **groups = glib_->KeyFileGetGroups(key_file_, &length);
@@ -107,7 +107,7 @@ set<string> KeyFileStore::GetGroups() {
 
 // Returns a set so that caller can easily test whether a particular group
 // is contained within this collection.
-set<string> KeyFileStore::GetGroupsWithKey(const string &key) {
+set<string> KeyFileStore::GetGroupsWithKey(const string &key) const {
   set<string> groups = GetGroups();
   set<string> groups_with_key;
   for (set<string>::iterator it = groups.begin(); it != groups.end(); ++it) {
@@ -118,7 +118,7 @@ set<string> KeyFileStore::GetGroupsWithKey(const string &key) {
   return groups_with_key;
 }
 
-bool KeyFileStore::ContainsGroup(const string &group) {
+bool KeyFileStore::ContainsGroup(const string &group) const {
   CHECK(key_file_);
   return glib_->KeyFileHasGroup(key_file_, group.c_str());
 }
@@ -160,7 +160,7 @@ bool KeyFileStore::SetHeader(const string &header) {
 
 bool KeyFileStore::GetString(const string &group,
                              const string &key,
-                             string *value) {
+                             string *value) const {
   CHECK(key_file_);
   GError *error = NULL;
   gchar *data =
@@ -187,7 +187,7 @@ bool KeyFileStore::SetString(const string &group,
 
 bool KeyFileStore::GetBool(const string &group,
                            const string &key,
-                           bool *value) {
+                           bool *value) const {
   CHECK(key_file_);
   GError *error = NULL;
   gboolean data =
@@ -212,7 +212,8 @@ bool KeyFileStore::SetBool(const string &group, const string &key, bool value) {
   return true;
 }
 
-bool KeyFileStore::GetInt(const string &group, const string &key, int *value) {
+bool KeyFileStore::GetInt(
+    const string &group, const string &key, int *value) const {
   CHECK(key_file_);
   GError *error = NULL;
   gint data =
@@ -236,7 +237,7 @@ bool KeyFileStore::SetInt(const string &group, const string &key, int value) {
 
 bool KeyFileStore::GetStringList(const string &group,
                                  const string &key,
-                                 vector<string> *value) {
+                                 vector<string> *value) const {
   CHECK(key_file_);
   gsize length = 0;
   GError *error = NULL;
diff --git a/key_file_store.h b/key_file_store.h
index 04004682..a8c25e3b 100644
--- a/key_file_store.h
+++ b/key_file_store.h
@@ -26,7 +26,7 @@ class KeyFileStore : public StoreInterface {
   const FilePath &path() const { return path_; }
 
   // Returns true if the store exists and is non-empty.
-  bool IsNonEmpty();
+  bool IsNonEmpty() const;
 
   // Opens the store. Returns true on success. This method must be
   // invoked before using any of the getters or setters.
@@ -43,33 +43,33 @@ class KeyFileStore : public StoreInterface {
 
   // Inherited from StoreInterface.
   virtual bool Flush();
-  virtual std::set<std::string> GetGroups();
-  virtual std::set<std::string> GetGroupsWithKey(const std::string &key);
-  virtual bool ContainsGroup(const std::string &group);
+  virtual std::set<std::string> GetGroups() const;
+  virtual std::set<std::string> GetGroupsWithKey(const std::string &key) const;
+  virtual bool ContainsGroup(const std::string &group) const;
   virtual bool DeleteKey(const std::string &group, const std::string &key);
   virtual bool DeleteGroup(const std::string &group);
   virtual bool SetHeader(const std::string &header);
   virtual bool GetString(const std::string &group,
                          const std::string &key,
-                         std::string *value);
+                         std::string *value) const;
   virtual bool SetString(const std::string &group,
                          const std::string &key,
                          const std::string &value);
   virtual bool GetBool(const std::string &group,
                        const std::string &key,
-                       bool *value);
+                       bool *value) const;
   virtual bool SetBool(const std::string &group,
                        const std::string &key,
                        bool value);
   virtual bool GetInt(const std::string &group,
                       const std::string &key,
-                      int *value);
+                      int *value) const;
   virtual bool SetInt(const std::string &group,
                       const std::string &key,
                       int value);
   virtual bool GetStringList(const std::string &group,
                              const std::string &key,
-                             std::vector<std::string> *value);
+                             std::vector<std::string> *value) const;
   virtual bool SetStringList(const std::string &group,
                              const std::string &key,
                              const std::vector<std::string> &value);
diff --git a/manager.cc b/manager.cc
index de2de611..fbb76d2b 100644
--- a/manager.cc
+++ b/manager.cc
@@ -320,6 +320,22 @@ bool Manager::HandleProfileEntryDeletion(const ProfileRefPtr &profile,
   return moved_services;
 }
 
+ServiceRefPtr Manager::GetServiceWithStorageIdentifier(
+    const ProfileRefPtr &profile, const std::string &entry_name, Error *error) {
+  for (vector<ServiceRefPtr>::iterator it = services_.begin();
+       it != services_.end(); ++it) {
+    if ((*it)->profile().get() == profile.get() &&
+        (*it)->GetStorageIdentifier() == entry_name) {
+      return *it;
+    }
+  }
+
+  Error::PopulateAndLog(error, Error::kNotFound,
+      base::StringPrintf("Entry %s is not registered in the manager",
+                         entry_name.c_str()));
+  return NULL;
+}
+
 const ProfileRefPtr &Manager::ActiveProfile() const {
   DCHECK_NE(profiles_.size(), 0);
   return profiles_.back();
diff --git a/manager.h b/manager.h
index a6ff9002..38a418f6 100644
--- a/manager.h
+++ b/manager.h
@@ -109,6 +109,14 @@ class Manager {
   // the profile.
   virtual bool HandleProfileEntryDeletion(const ProfileRefPtr &profile,
                                           const std::string &entry_name);
+  // Find a service that is both the member of |profile| and has a
+  // storage identifier that matches |entry_name|.  This function is
+  // called by the Profile in order to return a profile entry's properties.
+  virtual ServiceRefPtr GetServiceWithStorageIdentifier(
+      const ProfileRefPtr &profile,
+      const std::string &entry_name,
+      Error *error);
+
 
   virtual DeviceInfo *device_info() { return &device_info_; }
   ModemInfo *modem_info() { return &modem_info_; }
diff --git a/mock_store.h b/mock_store.h
index e42063e8..86dd4f97 100644
--- a/mock_store.h
+++ b/mock_store.h
@@ -22,35 +22,35 @@ class MockStore : public StoreInterface {
   virtual ~MockStore();
 
   MOCK_METHOD0(Flush, bool());
-  MOCK_METHOD0(GetGroups, std::set<std::string>());
-  MOCK_METHOD1(GetGroupsWithKey,
-               std::set<std::string>(const std::string &key));
-  MOCK_METHOD1(ContainsGroup, bool(const std::string &group));
+  MOCK_CONST_METHOD0(GetGroups, std::set<std::string>());
+  MOCK_CONST_METHOD1(GetGroupsWithKey,
+                     std::set<std::string>(const std::string &key));
+  MOCK_CONST_METHOD1(ContainsGroup, bool(const std::string &group));
   MOCK_METHOD2(DeleteKey, bool(const std::string &group,
                                const std::string &key));
   MOCK_METHOD1(DeleteGroup, bool(const std::string &group));
   MOCK_METHOD1(SetHeader, bool(const std::string &header));
-  MOCK_METHOD3(GetString, bool(const std::string &group,
-                               const std::string &key,
-                               std::string *value));
+  MOCK_CONST_METHOD3(GetString, bool(const std::string &group,
+                                     const std::string &key,
+                                     std::string *value));
   MOCK_METHOD3(SetString, bool(const std::string &group,
                                const std::string &key,
                                const std::string &value));
-  MOCK_METHOD3(GetBool, bool(const std::string &group,
-                             const std::string &key,
-                             bool *value));
+  MOCK_CONST_METHOD3(GetBool, bool(const std::string &group,
+                                   const std::string &key,
+                                   bool *value));
   MOCK_METHOD3(SetBool, bool(const std::string &group,
                              const std::string &key,
                              bool value));
-  MOCK_METHOD3(GetInt, bool(const std::string &group,
-                            const std::string &key,
-                            int *value));
+  MOCK_CONST_METHOD3(GetInt, bool(const std::string &group,
+                                  const std::string &key,
+                                  int *value));
   MOCK_METHOD3(SetInt, bool(const std::string &group,
                             const std::string &key,
                             int value));
-  MOCK_METHOD3(GetStringList, bool(const std::string &group,
-                                   const std::string &key,
-                                   std::vector<std::string> *value));
+  MOCK_CONST_METHOD3(GetStringList, bool(const std::string &group,
+                                         const std::string &key,
+                                         std::vector<std::string> *value));
   MOCK_METHOD3(SetStringList, bool(const std::string &group,
                                    const std::string &key,
                                    const std::vector<std::string> &value));
diff --git a/profile.cc b/profile.cc
index fba86dfc..2f034ff2 100644
--- a/profile.cc
+++ b/profile.cc
@@ -162,6 +162,12 @@ void Profile::DeleteEntry(const std::string &entry_name, Error *error) {
     // Otherwise, we need to delete the group ourselves.
     storage_->DeleteGroup(entry_name);
   }
+  Save();
+}
+
+ServiceRefPtr Profile::GetServiceFromEntry(const std::string &entry_name,
+                                           Error *error) {
+  return manager_->GetServiceWithStorageIdentifier(this, entry_name, error);
 }
 
 bool Profile::IsValidIdentifierToken(const string &token) {
diff --git a/profile.h b/profile.h
index 2df6cf63..70ba8a60 100644
--- a/profile.h
+++ b/profile.h
@@ -95,6 +95,10 @@ class Profile : public base::RefCounted<Profile> {
   // any service that uses this profile entry.
   virtual void DeleteEntry(const std::string &entry_name, Error *error);
 
+  // Return a service configured from the given profile entry.
+  virtual ServiceRefPtr GetServiceFromEntry(const std::string &entry_name,
+                                            Error *error);
+
   // Return whether |service| can configure itself from the profile.
   bool ContainsService(const ServiceConstRefPtr &service);
 
@@ -113,6 +117,10 @@ class Profile : public base::RefCounted<Profile> {
   // Returns whether |name| matches this Profile's |name_|.
   virtual bool MatchesIdentifier(const Identifier &name) const;
 
+  // Returns a read-only copy of the backing storage of the profile.
+  // TODO(pstew): Needed by ProfileDBusPropertyExporter crosbug.com/25813
+  const StoreInterface *GetConstStorage() const { return storage_.get(); }
+
  protected:
   // Protected getters
   Manager *manager() const { return manager_; }
diff --git a/profile_dbus_adaptor.cc b/profile_dbus_adaptor.cc
index 42f7d1cf..6ebf8bdc 100644
--- a/profile_dbus_adaptor.cc
+++ b/profile_dbus_adaptor.cc
@@ -13,6 +13,8 @@
 
 #include "shill/error.h"
 #include "shill/profile.h"
+#include "shill/profile_dbus_property_exporter.h"
+#include "shill/service.h"
 
 using std::map;
 using std::string;
@@ -71,9 +73,21 @@ void ProfileDBusAdaptor::SetProperty(const string &name,
 }
 
 map<string, ::DBus::Variant> ProfileDBusAdaptor::GetEntry(
-    const std::string& /*name*/,
-    ::DBus::Error &/*error*/) {
-  return map<string, ::DBus::Variant>();
+    const std::string &name,
+    ::DBus::Error &error) {
+  Error e;
+  ServiceRefPtr service = profile_->GetServiceFromEntry(name, &e);
+  map<string, ::DBus::Variant> properties;
+  if (e.IsSuccess()) {
+    DBusAdaptor::GetProperties(service->store(), &properties, &error);
+    return properties;
+  }
+
+  ProfileDBusPropertyExporter loader(profile_->GetConstStorage(), name);
+  if (!loader.LoadServiceProperties(&properties, &e)) {
+    e.ToDBusError(&error);
+  }
+  return properties;
 }
 
 void ProfileDBusAdaptor::DeleteEntry(const std::string &name,
diff --git a/profile_dbus_property_exporter.cc b/profile_dbus_property_exporter.cc
new file mode 100644
index 00000000..6523955e
--- /dev/null
+++ b/profile_dbus_property_exporter.cc
@@ -0,0 +1,128 @@
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "shill/profile_dbus_property_exporter.h"
+
+#include <string>
+
+#include <base/basictypes.h>
+#include <chromeos/dbus/service_constants.h>
+#include <dbus-c++/dbus.h>
+
+#include "shill/dbus_adaptor.h"
+#include "shill/error.h"
+#include "shill/service.h"
+#include "shill/store_interface.h"
+#include "shill/technology.h"
+#include "shill/wifi_service.h"
+
+using std::string;
+
+namespace shill {
+
+ProfileDBusPropertyExporter::ProfileDBusPropertyExporter(
+    const StoreInterface *storage, const string &entry_name)
+    : storage_(storage), entry_name_(entry_name) {}
+
+ProfileDBusPropertyExporter::~ProfileDBusPropertyExporter() {}
+
+bool ProfileDBusPropertyExporter::LoadServiceProperties(
+    PropertyList *properties, Error *error) {
+  if (!storage_->ContainsGroup(entry_name_)) {
+    Error::PopulateAndLog(
+        error, Error::kNotFound,
+        "Could not find profile entry: " + entry_name_);
+    return false;
+  }
+
+  Technology::Identifier technology =
+      Technology::IdentifierFromStorageGroup(entry_name_);
+
+  if (technology == Technology::kUnknown) {
+    Error::PopulateAndLog(
+        error, Error::kInternalError,
+        "Could not determine technology for entry: " + entry_name_);
+    return false;
+  }
+
+  if (technology == Technology::kWifi) {
+    LoadWiFiServiceProperties(properties, error);
+  }
+
+  LoadBool(properties, Service::kStorageAutoConnect,
+           flimflam::kAutoConnectProperty);
+  LoadString(properties, Service::kStorageError, flimflam::kErrorProperty);
+  LoadString(properties, Service::kStorageGUID, flimflam::kGuidProperty);
+  LoadString(properties, Service::kStorageName, flimflam::kNameProperty);
+  if (!LoadString(properties, Service::kStorageType, flimflam::kTypeProperty)) {
+    SetString(properties, flimflam::kTypeProperty,
+              Technology::NameFromIdentifier(technology));
+  }
+  LoadString(properties, Service::kStorageUIData, flimflam::kUIDataProperty);
+  return true;
+}
+
+bool ProfileDBusPropertyExporter::LoadWiFiServiceProperties(
+    PropertyList *properties,
+    Error */*error*/) {
+  LoadBool(properties, WiFiService::kStorageHiddenSSID,
+             flimflam::kWifiHiddenSsid);
+
+  // Support the old and busted technique for storing "Mode" and "Security"
+  // within the entry name.
+  string address;
+  string mode;
+  string security;
+  bool parsed_identifier = WiFiService::ParseStorageIdentifier(
+      entry_name_, &address, &mode, &security);
+
+  if (!LoadString(properties, WiFiService::kStorageMode,
+                  flimflam::kModeProperty) && parsed_identifier) {
+    SetString(properties, flimflam::kModeProperty, mode);
+  }
+
+  if (!LoadString(properties, WiFiService::kStorageSecurity,
+                  flimflam::kSecurityProperty) && parsed_identifier) {
+    SetString(properties, flimflam::kSecurityProperty, security);
+  }
+  return true;
+}
+
+bool ProfileDBusPropertyExporter::LoadBool(PropertyList *properties,
+                                           const string &storage_name,
+                                           const string &dbus_name) {
+  bool value;
+  if (!storage_->GetBool(entry_name_, storage_name, &value)) {
+    return false;
+  }
+
+  SetBool(properties, dbus_name, value);
+  return true;
+}
+
+bool ProfileDBusPropertyExporter::LoadString(PropertyList *properties,
+                                             const string &storage_name,
+                                             const string &dbus_name) {
+  string value;
+  if (!storage_->GetString(entry_name_, storage_name, &value)) {
+    return false;
+  }
+
+  SetString(properties, dbus_name, value);
+  return true;
+}
+
+void ProfileDBusPropertyExporter::SetBool(PropertyList *properties,
+                                          const string &dbus_name,
+                                          bool value) {
+  (*properties)[dbus_name] = DBusAdaptor::BoolToVariant(value);
+}
+
+void ProfileDBusPropertyExporter::SetString(PropertyList *properties,
+                                            const string &dbus_name,
+                                            const string &value) {
+  (*properties)[dbus_name] = DBusAdaptor::StringToVariant(value);
+}
+
+}  // namespace shill
diff --git a/profile_dbus_property_exporter.h b/profile_dbus_property_exporter.h
new file mode 100644
index 00000000..1e414034
--- /dev/null
+++ b/profile_dbus_property_exporter.h
@@ -0,0 +1,63 @@
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SHILL_PROFILE_DBUS_PROPERTY_EXPORTER_
+#define SHILL_PROFILE_DBUS_PROPERTY_EXPORTER_
+
+#include <map>
+#include <string>
+
+#include <base/basictypes.h>
+#include <dbus-c++/dbus.h>
+
+namespace shill {
+
+class Error;
+class StoreInterface;
+
+// This class is responsible for loading stored profile properties
+// from storage for presentation using the Profile::GetEntry DBus
+// API.  Properties are loaded and presented in much the same way
+// as a live service would present them.  This is troublesome
+// because it needs to duplicate (and stay in sync with) the way
+// properties are loaded and presented in a real service.
+//
+// TODO(pstew): Get rid of this.  It's nasty.  crosbug.com/25813
+class ProfileDBusPropertyExporter {
+ public:
+  typedef std::map<std::string, ::DBus::Variant> PropertyList;
+
+  ProfileDBusPropertyExporter(const StoreInterface *storage,
+                              const std::string &entry_name);
+  virtual ~ProfileDBusPropertyExporter();
+
+  bool LoadServiceProperties(PropertyList *properties,
+                             Error *error);
+
+ private:
+  bool LoadWiFiServiceProperties(PropertyList *properties,
+                                 Error *error);
+
+  bool LoadBool(PropertyList *properties,
+                const std::string &storage_name,
+                const std::string &dbus_name);
+  bool LoadString(PropertyList *properties,
+                  const std::string &storage_name,
+                  const std::string &dbus_name);
+  void SetBool(PropertyList *properties,
+               const std::string &dbus_name,
+               bool value);
+  void SetString(PropertyList *properties,
+                 const std::string &dbus_name,
+                 const std::string &value);
+
+  const StoreInterface *storage_;
+  const std::string entry_name_;
+
+  DISALLOW_COPY_AND_ASSIGN(ProfileDBusPropertyExporter);
+};
+
+}  // namespace shill
+
+#endif  // SHILL_PROFILE_DBUS_PROPERTY_EXPORTER_
diff --git a/profile_dbus_property_exporter_unittest.cc b/profile_dbus_property_exporter_unittest.cc
new file mode 100644
index 00000000..a5d14cb8
--- /dev/null
+++ b/profile_dbus_property_exporter_unittest.cc
@@ -0,0 +1,176 @@
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "shill/profile_dbus_property_exporter.h"
+
+#include <string>
+
+#include <base/memory/scoped_ptr.h>
+#include <chromeos/dbus/service_constants.h>
+#include <gtest/gtest.h>
+
+#include "shill/error.h"
+#include "shill/mock_store.h"
+#include "shill/service.h"
+#include "shill/wifi_service.h"
+
+using std::string;
+using testing::_;
+using testing::DoAll;
+using testing::Return;
+using testing::SetArgumentPointee;
+using testing::StrEq;
+using testing::StrNe;
+using testing::StrictMock;
+
+namespace shill {
+
+class ProfileDBusPropertyExporterTest : public testing::Test {
+ public:
+  ProfileDBusPropertyExporterTest() {}
+
+  virtual void SetUp() {
+    SetUpWithEntryName("entry_name");
+  }
+
+  void SetUpWithEntryName(const string &entry_name) {
+    entry_name_ = entry_name;
+    storage_.reset(new StrictMock<MockStore>());
+    EXPECT_CALL(*storage_.get(), ContainsGroup(StrEq(entry_name_)))
+        .WillRepeatedly(Return(true));
+    EXPECT_CALL(*storage_.get(), ContainsGroup(StrNe(entry_name_)))
+        .WillRepeatedly(Return(false));
+    exporter_.reset(new ProfileDBusPropertyExporter(storage_.get(),
+                                                    entry_name_));
+  }
+
+  void ExpectBoolProperty(const string &key, bool value) {
+    EXPECT_CALL(*storage_.get(), GetBool(StrEq(entry_name_), StrEq(key), _))
+        .WillOnce(DoAll(SetArgumentPointee<2>(value), Return(true)));
+  }
+
+  void ExpectStringProperty(const string &key, const string &value) {
+    EXPECT_CALL(*storage_.get(), GetString(StrEq(entry_name_), StrEq(key), _))
+        .WillOnce(DoAll(SetArgumentPointee<2>(value), Return(true)));
+  }
+
+  void ExpectUnknownProperties() {
+    EXPECT_CALL(*storage_.get(), GetBool(StrEq(entry_name_), _, _))
+        .WillRepeatedly(Return(false));
+    EXPECT_CALL(*storage_.get(), GetString(StrEq(entry_name_), _, _))
+        .WillRepeatedly(Return(false));
+  }
+
+  bool GetBoolProperty(
+      ProfileDBusPropertyExporter::PropertyList *props, const string key) {
+    return (*props)[key].reader().get_bool();
+  }
+
+  string GetStringProperty(
+      ProfileDBusPropertyExporter::PropertyList *props, const string key) {
+    return (*props)[key].reader().get_string();
+  }
+
+ protected:
+  string entry_name_;
+  scoped_ptr<MockStore> storage_;
+  scoped_ptr<ProfileDBusPropertyExporter> exporter_;
+};
+
+TEST_F(ProfileDBusPropertyExporterTest, LoadWrongGroup) {
+  ProfileDBusPropertyExporter exporter(storage_.get(), "not_entry_name");
+  ProfileDBusPropertyExporter::PropertyList props;
+  Error e;
+  EXPECT_FALSE(exporter.LoadServiceProperties(&props, &e));
+  EXPECT_EQ(Error::kNotFound, e.type());
+}
+
+TEST_F(ProfileDBusPropertyExporterTest, InvalidTechnology) {
+  SetUpWithEntryName("unknown_technology");
+  ProfileDBusPropertyExporter::PropertyList props;
+  Error e;
+  EXPECT_FALSE(exporter_->LoadServiceProperties(&props, &e));
+  EXPECT_EQ(Error::kInternalError, e.type());
+}
+
+TEST_F(ProfileDBusPropertyExporterTest, MinimalProperties) {
+  SetUpWithEntryName("ethernet_service");
+  ExpectUnknownProperties();
+  ProfileDBusPropertyExporter::PropertyList props;
+  Error e;
+  EXPECT_TRUE(exporter_->LoadServiceProperties(&props, &e));
+  EXPECT_EQ(1, props.size());
+  EXPECT_EQ(flimflam::kTypeEthernet,
+            GetStringProperty(&props, flimflam::kTypeProperty));
+}
+
+TEST_F(ProfileDBusPropertyExporterTest, OverrideTypeProperty) {
+  SetUpWithEntryName("ethernet_service");
+  const string service_type = "special_type";
+  ExpectUnknownProperties();
+  ExpectStringProperty(Service::kStorageType, service_type);
+  ProfileDBusPropertyExporter::PropertyList props;
+  Error e;
+  EXPECT_TRUE(exporter_->LoadServiceProperties(&props, &e));
+  EXPECT_EQ(1, props.size());
+  EXPECT_EQ(service_type, GetStringProperty(&props, flimflam::kTypeProperty));
+}
+
+TEST_F(ProfileDBusPropertyExporterTest, AllServiceProperties) {
+  SetUpWithEntryName("ethernet_service");
+  const bool auto_connect(true);
+  ExpectBoolProperty(Service::kStorageAutoConnect, auto_connect);
+  const string error("no carrier");
+  ExpectStringProperty(Service::kStorageError, error);
+  const string guid("guid");
+  ExpectStringProperty(Service::kStorageGUID, guid);
+  const string name("fastnet");
+  ExpectStringProperty(Service::kStorageName, name);
+  const string type("100baset");
+  ExpectStringProperty(Service::kStorageType, type);
+  const string uidata("ui data");
+  ExpectStringProperty(Service::kStorageUIData, uidata);
+
+  ProfileDBusPropertyExporter::PropertyList props;
+  Error e;
+  EXPECT_TRUE(exporter_->LoadServiceProperties(&props, &e));
+  EXPECT_EQ(auto_connect, GetBoolProperty(&props,
+                                          flimflam::kAutoConnectProperty));
+  EXPECT_EQ(error, GetStringProperty(&props, flimflam::kErrorProperty));
+  EXPECT_EQ(guid, GetStringProperty(&props, flimflam::kGuidProperty));
+  EXPECT_EQ(name, GetStringProperty(&props, flimflam::kNameProperty));
+  EXPECT_EQ(type, GetStringProperty(&props, flimflam::kTypeProperty));
+  EXPECT_EQ(uidata, GetStringProperty(&props, flimflam::kUIDataProperty));
+}
+
+TEST_F(ProfileDBusPropertyExporterTest, MinimalWiFiServiceProperties) {
+  SetUpWithEntryName("wifi_address_ssid_superfly_unbreakable_crypto");
+  ExpectUnknownProperties();
+  ProfileDBusPropertyExporter::PropertyList props;
+  Error e;
+  EXPECT_TRUE(exporter_->LoadServiceProperties(&props, &e));
+  EXPECT_EQ("superfly", GetStringProperty(&props, flimflam::kModeProperty));
+  EXPECT_EQ("unbreakable_crypto",
+            GetStringProperty(&props, flimflam::kSecurityProperty));
+}
+
+TEST_F(ProfileDBusPropertyExporterTest, AllWiFiServiceProperties) {
+  SetUpWithEntryName("wifi_service");
+  ExpectUnknownProperties();
+  const bool hidden_ssid(true);
+  ExpectBoolProperty(WiFiService::kStorageHiddenSSID, hidden_ssid);
+  const string mode("superfly");
+  ExpectStringProperty(WiFiService::kStorageMode, mode);
+  const string security("unbreakablecrypto");
+  ExpectStringProperty(WiFiService::kStorageSecurity, security);
+
+  ProfileDBusPropertyExporter::PropertyList props;
+  Error e;
+  EXPECT_TRUE(exporter_->LoadServiceProperties(&props, &e));
+  EXPECT_EQ(hidden_ssid, GetBoolProperty(&props, flimflam::kWifiHiddenSsid));
+  EXPECT_EQ(mode, GetStringProperty(&props, flimflam::kModeProperty));
+  EXPECT_EQ(security, GetStringProperty(&props, flimflam::kSecurityProperty));
+}
+
+}  // namespace shill
diff --git a/profile_unittest.cc b/profile_unittest.cc
index 3514a97c..89472752 100644
--- a/profile_unittest.cc
+++ b/profile_unittest.cc
@@ -96,27 +96,35 @@ TEST_F(ProfileTest, DeleteEntry) {
   }
 
   Mock::VerifyAndClearExpectations(storage);
-  EXPECT_CALL(*storage, ContainsGroup(kEntryName))
-      .WillRepeatedly(Return(true));
 
   // If HandleProfileEntryDeletion() returns false, Profile should call
   // DeleteGroup() itself.
+  EXPECT_CALL(*storage, ContainsGroup(kEntryName))
+      .WillOnce(Return(true));
   EXPECT_CALL(*manager.get(), HandleProfileEntryDeletion(_, kEntryName))
       .WillOnce(Return(false));
   EXPECT_CALL(*storage, DeleteGroup(kEntryName))
       .WillOnce(Return(true));
+  EXPECT_CALL(*storage, Flush())
+      .WillOnce(Return(true));
   {
     Error error;
     profile_->DeleteEntry(kEntryName, &error);
     EXPECT_TRUE(error.IsSuccess());
   }
 
+  Mock::VerifyAndClearExpectations(storage);
+
   // If HandleProfileEntryDeletion() returns true, Profile should not call
   // DeleteGroup() itself.
+  EXPECT_CALL(*storage, ContainsGroup(kEntryName))
+      .WillOnce(Return(true));
   EXPECT_CALL(*manager.get(), HandleProfileEntryDeletion(_, kEntryName))
       .WillOnce(Return(true));
   EXPECT_CALL(*storage, DeleteGroup(kEntryName))
       .Times(0);
+  EXPECT_CALL(*storage, Flush())
+      .WillOnce(Return(true));
   {
     Error error;
     profile_->DeleteEntry(kEntryName, &error);
diff --git a/service.cc b/service.cc
index 70cb9cc9..caaaad93 100644
--- a/service.cc
+++ b/service.cc
@@ -57,7 +57,9 @@ const char Service::kStorageEapPassword[] = "EAP.Password";
 const char Service::kStorageEapPrivateKey[] = "EAP.PrivateKey";
 const char Service::kStorageEapPrivateKeyPassword[] = "EAP.PrivateKeyPassword";
 const char Service::kStorageEapUseSystemCAs[] = "EAP.UseSystemCAs";
+const char Service::kStorageError[] = "Error";
 const char Service::kStorageFavorite[] = "Favorite";
+const char Service::kStorageGUID[] = "GUID";
 const char Service::kStorageName[] = "Name";
 const char Service::kStoragePriority[] = "Priority";
 const char Service::kStorageProxyConfig[] = "ProxyConfig";
diff --git a/service.h b/service.h
index 47f6358b..4f904437 100644
--- a/service.h
+++ b/service.h
@@ -46,6 +46,35 @@ class Service : public base::RefCounted<Service> {
   static const char kCheckPortalFalse[];
   static const char kCheckPortalTrue[];
 
+  // TODO(pstew): Storage constants shouldn't need to be public
+  // crosbug.com/25813
+  static const char kStorageAutoConnect[];
+  static const char kStorageCheckPortal[];
+  static const char kStorageEapAnonymousIdentity[];
+  static const char kStorageEapCACert[];
+  static const char kStorageEapCACertID[];
+  static const char kStorageEapCertID[];
+  static const char kStorageEapClientCert[];
+  static const char kStorageEapEap[];
+  static const char kStorageEapIdentity[];
+  static const char kStorageEapInnerEap[];
+  static const char kStorageEapKeyID[];
+  static const char kStorageEapKeyManagement[];
+  static const char kStorageEapPIN[];
+  static const char kStorageEapPassword[];
+  static const char kStorageEapPrivateKey[];
+  static const char kStorageEapPrivateKeyPassword[];
+  static const char kStorageEapUseSystemCAs[];
+  static const char kStorageError[];
+  static const char kStorageFavorite[];
+  static const char kStorageGUID[];
+  static const char kStorageName[];
+  static const char kStoragePriority[];
+  static const char kStorageProxyConfig[];
+  static const char kStorageSaveCredentials[];
+  static const char kStorageType[];
+  static const char kStorageUIData[];
+
   enum ConnectFailure {
     kFailureUnknown,
     kFailureActivationFailure,
@@ -289,8 +318,8 @@ class Service : public base::RefCounted<Service> {
   Metrics *metrics() const { return metrics_; }
 
  private:
-  friend class ServiceAdaptorInterface;
   friend class MetricsTest;
+  friend class ServiceAdaptorInterface;
   FRIEND_TEST(DeviceTest, SelectedService);
   FRIEND_TEST(ManagerTest, SortServicesWithConnection);
   FRIEND_TEST(ServiceTest, Constructor);
@@ -304,31 +333,6 @@ class Service : public base::RefCounted<Service> {
   FRIEND_TEST(ServiceTest, SaveStringEmpty);
   FRIEND_TEST(ServiceTest, Unload);
 
-  static const char kStorageAutoConnect[];
-  static const char kStorageCheckPortal[];
-  static const char kStorageEapAnonymousIdentity[];
-  static const char kStorageEapCACert[];
-  static const char kStorageEapCACertID[];
-  static const char kStorageEapCertID[];
-  static const char kStorageEapClientCert[];
-  static const char kStorageEapEap[];
-  static const char kStorageEapIdentity[];
-  static const char kStorageEapInnerEap[];
-  static const char kStorageEapKeyID[];
-  static const char kStorageEapKeyManagement[];
-  static const char kStorageEapPIN[];
-  static const char kStorageEapPassword[];
-  static const char kStorageEapPrivateKey[];
-  static const char kStorageEapPrivateKeyPassword[];
-  static const char kStorageEapUseSystemCAs[];
-  static const char kStorageFavorite[];
-  static const char kStorageName[];
-  static const char kStoragePriority[];
-  static const char kStorageProxyConfig[];
-  static const char kStorageSaveCredentials[];
-  static const char kStorageType[];
-  static const char kStorageUIData[];
-
   bool GetAutoConnect(Error *error);
   void SetAutoConnect(const bool &connect, Error *error);
 
diff --git a/store_interface.h b/store_interface.h
index 9e20ce04..9d1e7ab0 100644
--- a/store_interface.h
+++ b/store_interface.h
@@ -20,13 +20,14 @@ class StoreInterface {
   virtual bool Flush() = 0;
 
   // Returns a set of all groups contained in the store.
-  virtual std::set<std::string> GetGroups() = 0;
+  virtual std::set<std::string> GetGroups() const = 0;
 
   // Returns the names of all groups that contain the named |key|.
-  virtual std::set<std::string> GetGroupsWithKey(const std::string &key) = 0;
+  virtual std::set<std::string> GetGroupsWithKey(
+      const std::string &key) const = 0;
 
   // Returns true if the store contains |group|, false otherwise.
-  virtual bool ContainsGroup(const std::string &group) = 0;
+  virtual bool ContainsGroup(const std::string &group) const = 0;
 
   // Deletes |group|:|key|. Returns true on success.
   virtual bool DeleteKey(const std::string &group, const std::string &key) = 0;
@@ -42,7 +43,7 @@ class StoreInterface {
   // in the store).
   virtual bool GetString(const std::string &group,
                          const std::string &key,
-                         std::string *value) = 0;
+                         std::string *value) const = 0;
 
   // Associates |group|:|key| with a string |value|. Returns true on success,
   // false otherwise.
@@ -55,7 +56,7 @@ class StoreInterface {
   // present in the store).
   virtual bool GetBool(const std::string &group,
                        const std::string &key,
-                       bool *value) = 0;
+                       bool *value) const = 0;
 
   // Associates |group|:|key| with a boolean |value|. Returns true on success,
   // false otherwise.
@@ -68,7 +69,7 @@ class StoreInterface {
   // present in the store).
   virtual bool GetInt(const std::string &group,
                       const std::string &key,
-                      int *value) = 0;
+                      int *value) const = 0;
 
   // Associates |group|:|key| with an integer |value|. Returns true on success,
   // false otherwise.
@@ -81,7 +82,7 @@ class StoreInterface {
   // in the store).
   virtual bool GetStringList(const std::string &group,
                              const std::string &key,
-                             std::vector<std::string> *value) = 0;
+                             std::vector<std::string> *value) const = 0;
 
   // Associates |group|:|key| with a string list |value|. Returns true on
   // success, false otherwise.
diff --git a/wifi_service.cc b/wifi_service.cc
index 380ff308..e2fd9c67 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -587,12 +587,18 @@ bool WiFiService::ParseStorageIdentifier(const string &storage_name,
                                          string *security) {
   vector<string> wifi_parts;
   base::SplitString(storage_name, '_', &wifi_parts);
-  if (wifi_parts.size() != 5 || wifi_parts[0] != flimflam::kTypeWifi) {
+  if ((wifi_parts.size() != 5 && wifi_parts.size() != 6) ||
+      wifi_parts[0] != flimflam::kTypeWifi) {
     return false;
   }
   *address = wifi_parts[1];
   *mode = wifi_parts[3];
-  *security = wifi_parts[4];
+  if (wifi_parts.size() == 5) {
+    *security = wifi_parts[4];
+  } else {
+    // Account for security type "802_1x" which got split up above.
+    *security = wifi_parts[4] + "_" + wifi_parts[5];
+  }
   return true;
 }
 
diff --git a/wifi_service.h b/wifi_service.h
index ef642eeb..ca28e10e 100644
--- a/wifi_service.h
+++ b/wifi_service.h
@@ -24,6 +24,14 @@ class Metrics;
 
 class WiFiService : public Service {
  public:
+  // TODO(pstew): Storage constants shouldn't need to be public
+  // crosbug.com/25813
+  static const char kStorageHiddenSSID[];
+  static const char kStorageMode[];
+  static const char kStoragePassphrase[];
+  static const char kStorageSecurity[];
+  static const char kStorageSSID[];
+
   WiFiService(ControlInterface *control_interface,
               EventDispatcher *dispatcher,
               Metrics *metrics,
@@ -97,12 +105,6 @@ class WiFiService : public Service {
   FRIEND_TEST(WiFiServiceTest, LoadAndUnloadPassphrase);
   FRIEND_TEST(WiFiServiceTest, Populate8021x);
 
-  static const char kStorageHiddenSSID[];
-  static const char kStorageMode[];
-  static const char kStoragePassphrase[];
-  static const char kStorageSecurity[];
-  static const char kStorageSSID[];
-
   void HelpRegisterDerivedString(
       PropertyStore *store,
       const std::string &name,
diff --git a/wifi_service_unittest.cc b/wifi_service_unittest.cc
index dc008986..219d7e8b 100644
--- a/wifi_service_unittest.cc
+++ b/wifi_service_unittest.cc
@@ -540,7 +540,7 @@ TEST_F(WiFiServiceTest, LoadAndUnloadPassphrase) {
   EXPECT_TRUE(service->need_passphrase_);
 }
 
-TEST_F(WiFiServiceTest, ParseStorageIdentifier) {
+TEST_F(WiFiServiceTest, ParseStorageIdentifierNone) {
   vector<uint8_t> ssid(5);
   ssid.push_back(0xff);
 
@@ -564,6 +564,32 @@ TEST_F(WiFiServiceTest, ParseStorageIdentifier) {
   EXPECT_EQ(flimflam::kSecurityNone, security);
 }
 
+TEST_F(WiFiServiceTest, ParseStorageIdentifier8021x) {
+  // Do a separate test for 802.1x, since kSecurity8021x contains a "_",
+  // which needs to be dealt with specially in the parser.
+  vector<uint8_t> ssid(5);
+  ssid.push_back(0xff);
+
+  WiFiServiceRefPtr service = new WiFiService(control_interface(),
+                                              dispatcher(),
+                                              metrics(),
+                                              manager(),
+                                              wifi(),
+                                              ssid,
+                                              flimflam::kModeManaged,
+                                              flimflam::kSecurity8021x,
+                                              false);
+  const string storage_id = service->GetStorageIdentifier();
+  string address;
+  string mode;
+  string security;
+  EXPECT_TRUE(service->ParseStorageIdentifier(storage_id, &address, &mode,
+                                              &security));
+  EXPECT_EQ(StringToLowerASCII(string(fake_mac)), address);
+  EXPECT_EQ(flimflam::kModeManaged, mode);
+  EXPECT_EQ(flimflam::kSecurity8021x, security);
+}
+
 TEST_F(WiFiServiceTest, Connectable) {
   // Open network should be connectable.
   EXPECT_TRUE(CheckConnectable(flimflam::kSecurityNone, NULL, NULL));