[shill] Add code for persisting profiles and services to disk.
BUG=chromium-os:17253
TEST=unit
Change-Id: Ic6dbbcb10543da3f4615cb305a77f6b9b301e8bc
Reviewed-on: http://gerrit.chromium.org/gerrit/7633
Reviewed-by: Darin Petkov <petkov@chromium.org>
Tested-by: Chris Masone <cmasone@chromium.org>
diff --git a/cellular_service.cc b/cellular_service.cc
index bc27c5b..e2de6b7 100644
--- a/cellular_service.cc
+++ b/cellular_service.cc
@@ -57,10 +57,10 @@
cellular_->Activate(carrier, error);
}
-string CellularService::GetStorageIdentifier(const string &mac) {
+string CellularService::GetStorageIdentifier() {
string id = base::StringPrintf("%s_%s_%s",
kServiceType,
- mac.c_str(),
+ cellular_->address().c_str(),
serving_operator_.GetName().c_str());
std::replace_if(id.begin(), id.end(), &Service::LegalChar, '_');
return id;
diff --git a/cellular_service.h b/cellular_service.h
index 2e464ee..b2ab054 100644
--- a/cellular_service.h
+++ b/cellular_service.h
@@ -35,7 +35,7 @@
virtual void ActivateCellularModem(const std::string &carrier, Error *error);
// cellular_<MAC>_<Service_Operator_Name>
- std::string GetStorageIdentifier(const std::string &mac);
+ std::string GetStorageIdentifier();
const std::string &activation_state() const { return activation_state_; }
void set_activation_state(const std::string &state) {
diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index 5fac327..1a3f701 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -91,7 +91,7 @@
flimflam::kAddressProperty,
PropertyStoreTest::kStringV,
&error));
- EXPECT_EQ(invalid_args_, error.name());
+ EXPECT_EQ(invalid_args(), error.name());
}
{
::DBus::Error error;
@@ -99,7 +99,7 @@
flimflam::kCarrierProperty,
PropertyStoreTest::kStringV,
&error));
- EXPECT_EQ(invalid_args_, error.name());
+ EXPECT_EQ(invalid_args(), error.name());
}
{
::DBus::Error error;
@@ -107,7 +107,7 @@
flimflam::kPRLVersionProperty,
PropertyStoreTest::kInt16V,
&error));
- EXPECT_EQ(invalid_args_, error.name());
+ EXPECT_EQ(invalid_args(), error.name());
}
}
diff --git a/dbus_adaptor_unittest.cc b/dbus_adaptor_unittest.cc
index eca91f2..84fed1e 100644
--- a/dbus_adaptor_unittest.cc
+++ b/dbus_adaptor_unittest.cc
@@ -55,13 +55,13 @@
uint32_v_(DBusAdaptor::Uint32ToVariant(ex_uint32_)),
device_(new MockDevice(&control_interface_,
&dispatcher_,
- &manager_,
+ manager(),
"mock",
"addr0",
0)),
service_(new MockService(&control_interface_,
&dispatcher_,
- &manager_)) {
+ manager())) {
ex_stringmap_[ex_string_] = ex_string_;
stringmap_v_ = DBusAdaptor::StringmapToVariant(ex_stringmap_);
diff --git a/default_profile.cc b/default_profile.cc
index 025c258..59e56c5 100644
--- a/default_profile.cc
+++ b/default_profile.cc
@@ -16,11 +16,10 @@
const char DefaultProfile::kDefaultId[] = "default";
DefaultProfile::DefaultProfile(ControlInterface *control,
- GLib *glib,
Manager *manager,
const FilePath &storage_path,
const Manager::Properties &manager_props)
- : Profile(control, glib, manager, Identifier(kDefaultId), "", true),
+ : Profile(control, manager, Identifier(kDefaultId), "", true),
storage_path_(storage_path) {
PropertyStore *store = this->store();
store->RegisterConstString(flimflam::kCheckPortalListProperty,
diff --git a/default_profile.h b/default_profile.h
index 0876ade..803c444 100644
--- a/default_profile.h
+++ b/default_profile.h
@@ -24,13 +24,11 @@
class DefaultProfile : public Profile {
public:
DefaultProfile(ControlInterface *control,
- GLib *glib,
Manager *manager,
const FilePath &storage_path,
const Manager::Properties &manager_props);
virtual ~DefaultProfile();
-
// Sets |path| to the persistent store file path for the default, global
// profile. Returns true on success, and false if unable to determine an
// appropriate file location.
diff --git a/default_profile_unittest.cc b/default_profile_unittest.cc
index 5828875..79e75e7 100644
--- a/default_profile_unittest.cc
+++ b/default_profile_unittest.cc
@@ -25,8 +25,7 @@
public:
DefaultProfileTest()
: profile_(new DefaultProfile(&control_interface_,
- &glib_,
- &manager_,
+ manager(),
FilePath(kTestStoragePath),
properties_)) {
}
@@ -37,7 +36,6 @@
static const char kTestStoragePath[];
ProfileRefPtr profile_;
- GLib glib_;
Manager::Properties properties_;
};
diff --git a/device.h b/device.h
index ed7c413..995517d 100644
--- a/device.h
+++ b/device.h
@@ -76,6 +76,7 @@
std::string GetRpcIdentifier();
std::string GetStorageIdentifier();
+ const std::string &address() const { return hardware_address_; }
const std::string &link_name() const { return link_name_; }
int interface_index() const { return interface_index_; }
const ConnectionRefPtr &connection() const { return connection_; }
diff --git a/device_unittest.cc b/device_unittest.cc
index a2f2a68..96f6890 100644
--- a/device_unittest.cc
+++ b/device_unittest.cc
@@ -107,7 +107,7 @@
flimflam::kAddressProperty,
PropertyStoreTest::kStringV,
&error));
- EXPECT_EQ(invalid_args_, error.name());
+ EXPECT_EQ(invalid_args(), error.name());
}
TEST_F(DeviceTest, TechnologyIs) {
@@ -177,7 +177,7 @@
scoped_refptr<MockService> service(
new StrictMock<MockService>(&control_interface_,
&dispatcher_,
- &manager_));
+ manager()));
device_->SelectService(service);
EXPECT_TRUE(device_->selected_service_.get() == service.get());
diff --git a/dhcp_config_unittest.cc b/dhcp_config_unittest.cc
index d7ffbf5..9ca6592 100644
--- a/dhcp_config_unittest.cc
+++ b/dhcp_config_unittest.cc
@@ -343,7 +343,7 @@
flimflam::kAddressProperty,
PropertyStoreTest::kStringV,
&error));
- EXPECT_EQ(invalid_args_, error.name());
+ EXPECT_EQ(invalid_args(), error.name());
}
} // namespace shill
diff --git a/ephemeral_profile.cc b/ephemeral_profile.cc
index 0469daf..372f4af 100644
--- a/ephemeral_profile.cc
+++ b/ephemeral_profile.cc
@@ -19,15 +19,19 @@
namespace shill {
EphemeralProfile::EphemeralProfile(ControlInterface *control_interface,
- GLib *glib,
Manager *manager)
- : Profile(control_interface, glib, manager, Identifier(), "", false) {
+ : Profile(control_interface, manager, Identifier(), "", false) {
}
EphemeralProfile::~EphemeralProfile() {}
-void EphemeralProfile::Finalize() {
+void EphemeralProfile::Finalize(StoreInterface *storage) {
services()->clear();
}
+bool EphemeralProfile::Save(StoreInterface *storage) {
+ NOTREACHED();
+ return false;
+}
+
} // namespace shill
diff --git a/ephemeral_profile.h b/ephemeral_profile.h
index 20fea1d..066e5a2 100644
--- a/ephemeral_profile.h
+++ b/ephemeral_profile.h
@@ -19,18 +19,23 @@
class ControlInterface;
class Manager;
+class StoreInterface;
// An in-memory profile that is not persisted to disk, but allows the
// promotion of entries contained herein to the currently active profile.
class EphemeralProfile : public Profile {
public:
- EphemeralProfile(ControlInterface *control_interface,
- GLib *glib,
- Manager *manager);
+ EphemeralProfile(ControlInterface *control_interface, Manager *manager);
virtual ~EphemeralProfile();
// Merely stop managing service persistence; flush nothing to disk.
- virtual void Finalize();
+ virtual void Finalize(StoreInterface *storage);
+
+ // Should not be called.
+ virtual bool Save(StoreInterface *storage);
+
+ // Leaves |path| untouched and returns false.
+ virtual bool GetStoragePath(FilePath *path) { return false; }
private:
DISALLOW_COPY_AND_ASSIGN(EphemeralProfile);
diff --git a/ethernet_service.cc b/ethernet_service.cc
index 8db66cb..ee02f6d 100644
--- a/ethernet_service.cc
+++ b/ethernet_service.cc
@@ -52,8 +52,9 @@
return ethernet_->GetRpcIdentifier();
}
-string EthernetService::GetStorageIdentifier(const string &mac) {
- return base::StringPrintf("%s_%s", kServiceType, mac.c_str());
+string EthernetService::GetStorageIdentifier() {
+ return base::StringPrintf("%s_%s",
+ kServiceType, ethernet_->address().c_str());
}
} // namespace shill
diff --git a/ethernet_service.h b/ethernet_service.h
index 88aa8b5..2934c6a 100644
--- a/ethernet_service.h
+++ b/ethernet_service.h
@@ -30,7 +30,7 @@
virtual void Disconnect();
// ethernet_<MAC>
- virtual std::string GetStorageIdentifier(const std::string &mac);
+ virtual std::string GetStorageIdentifier();
private:
static const char kServiceType[];
diff --git a/key_file_store.cc b/key_file_store.cc
index b228e86..2d34d1d 100644
--- a/key_file_store.cc
+++ b/key_file_store.cc
@@ -105,7 +105,7 @@
CHECK(key_file_);
GError *error = NULL;
glib_->KeyFileRemoveKey(key_file_, group.c_str(), key.c_str(), &error);
- if (error) {
+ if (error && error->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND) {
LOG(ERROR) << "Failed to delete (" << group << ":" << key << "): "
<< glib_->ConvertErrorToMessage(error);
return false;
@@ -117,7 +117,7 @@
CHECK(key_file_);
GError *error = NULL;
glib_->KeyFileRemoveGroup(key_file_, group.c_str(), &error);
- if (error) {
+ if (error && error->code != G_KEY_FILE_ERROR_GROUP_NOT_FOUND) {
LOG(ERROR) << "Failed to delete group " << group << ": "
<< glib_->ConvertErrorToMessage(error);
return false;
diff --git a/key_file_store.h b/key_file_store.h
index b448256..2986ba9 100644
--- a/key_file_store.h
+++ b/key_file_store.h
@@ -25,9 +25,20 @@
void set_path(const FilePath &path) { path_ = path; }
const FilePath &path() const { return path_; }
+ // Opens the store. Returns true on success. This method must be
+ // invoked before using any of the getters or setters.
+ // This method does not complete gracefully if invoked on a store
+ // that has been opened already but not closed yet.
+ bool Open();
+
+ // Closes the store and flushes it to persistent storage. Returns true on
+ // success. Note that the store is considered closed even if Close returns
+ // false.
+ // This method does not complete gracefully if invoked on a store
+ // that has not been opened successfully or has been closed already.
+ bool Close();
+
// Inherited from StoreInterface.
- virtual bool Open();
- virtual bool Close();
virtual std::set<std::string> GetGroups();
virtual bool ContainsGroup(const std::string &group);
virtual bool DeleteKey(const std::string &group, const std::string &key);
diff --git a/key_file_store_unittest.cc b/key_file_store_unittest.cc
index 65fa89e..d69d479 100644
--- a/key_file_store_unittest.cc
+++ b/key_file_store_unittest.cc
@@ -127,7 +127,7 @@
kGroup, kKeyDead, kKeyAlive, kValueAlive));
ASSERT_TRUE(store_.Open());
EXPECT_TRUE(store_.DeleteKey(kGroup, kKeyDead));
- EXPECT_FALSE(store_.DeleteKey(kGroup, "random-key"));
+ EXPECT_TRUE(store_.DeleteKey(kGroup, "random-key"));
EXPECT_FALSE(store_.DeleteKey("random-group", kKeyAlive));
ASSERT_TRUE(store_.Close());
EXPECT_EQ(base::StringPrintf("\n"
@@ -148,7 +148,7 @@
kGroupA, kGroupB, kGroupC));
ASSERT_TRUE(store_.Open());
EXPECT_TRUE(store_.DeleteGroup(kGroupB));
- EXPECT_FALSE(store_.DeleteGroup("group-d"));
+ EXPECT_TRUE(store_.DeleteGroup("group-d"));
ASSERT_TRUE(store_.Close());
EXPECT_EQ(base::StringPrintf("\n"
"[%s]\n"
@@ -532,7 +532,7 @@
}
EXPECT_TRUE(store_.DeleteGroup(kGroupA));
- EXPECT_FALSE(store_.DeleteGroup(kGroupA));
+ EXPECT_TRUE(store_.DeleteGroup(kGroupA));
EXPECT_FALSE(store_.ContainsGroup(kGroupA));
EXPECT_TRUE(store_.ContainsGroup(kGroupB));
@@ -552,7 +552,7 @@
vector<string>(1, kValueStringB));
EXPECT_TRUE(store_.DeleteKey(kGroupB, kKeyString));
- EXPECT_FALSE(store_.DeleteKey(kGroupB, kKeyString));
+ EXPECT_TRUE(store_.DeleteKey(kGroupB, kKeyString));
{
string value;
diff --git a/manager.cc b/manager.cc
index 19aadbd..960eaac 100644
--- a/manager.cc
+++ b/manager.cc
@@ -23,6 +23,7 @@
#include "shill/device_info.h"
#include "shill/ephemeral_profile.h"
#include "shill/error.h"
+#include "shill/key_file_store.h"
#include "shill/profile.h"
#include "shill/property_accessor.h"
#include "shill/resolver.h"
@@ -47,7 +48,7 @@
device_info_(control_interface, dispatcher, this),
modem_info_(control_interface, dispatcher, this, glib),
running_(false),
- ephemeral_profile_(new EphemeralProfile(control_interface, glib, this)),
+ ephemeral_profile_(new EphemeralProfile(control_interface, this)),
control_interface_(control_interface),
glib_(glib) {
HelpRegisterDerivedString(flimflam::kActiveProfileProperty,
@@ -86,7 +87,6 @@
// TODO(cmasone): Wire these up once we actually put in profile support.
// known_properties_.push_back(flimflam::kProfilesProperty);
profiles_.push_back(new DefaultProfile(control_interface_,
- glib_,
this,
storage_path_,
props_));
@@ -94,11 +94,7 @@
}
Manager::~Manager() {
- vector<ProfileRefPtr>::iterator it;
- for (it = profiles_.begin(); it != profiles_.end(); ++it) {
- (*it)->Finalize();
- }
- ephemeral_profile_->Finalize();
+ profiles_.clear();
}
void Manager::AddDeviceToBlackList(const string &device_name) {
@@ -121,6 +117,22 @@
void Manager::Stop() {
running_ = false;
+ // Persist profile, device, service information to disk.
+ vector<ProfileRefPtr>::iterator it;
+ for (it = profiles_.begin(); it != profiles_.end(); ++it) {
+ KeyFileStore storage(glib_);
+ FilePath profile_path;
+ CHECK((*it)->GetStoragePath(&profile_path));
+ storage.set_path(profile_path);
+ if (storage.Open()) {
+ (*it)->Finalize(&storage);
+ storage.Close();
+ } else {
+ LOG(ERROR) << "Could not open storage at " << profile_path.value();
+ }
+ }
+ ephemeral_profile_->Finalize(NULL);
+
adaptor_->UpdateRunning();
modem_info_.Stop();
device_info_.Stop();
diff --git a/manager_unittest.cc b/manager_unittest.cc
index fb5f78d..7a60d7e 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -30,29 +30,29 @@
using std::vector;
namespace shill {
-using ::testing::Test;
using ::testing::_;
using ::testing::NiceMock;
using ::testing::Return;
+using ::testing::Test;
class ManagerTest : public PropertyStoreTest {
public:
ManagerTest()
: mock_device_(new NiceMock<MockDevice>(&control_interface_,
&dispatcher_,
- &manager_,
+ manager(),
"null0",
"addr0",
0)),
mock_device2_(new NiceMock<MockDevice>(&control_interface_,
&dispatcher_,
- &manager_,
+ manager(),
"null2",
"addr2",
2)),
mock_device3_(new NiceMock<MockDevice>(&control_interface_,
&dispatcher_,
- &manager_,
+ manager(),
"null3",
"addr3",
3)) {
@@ -61,7 +61,7 @@
bool IsDeviceRegistered(const DeviceRefPtr &device, Device::Technology tech) {
vector<DeviceRefPtr> devices;
- manager_.FilterByTechnology(tech, &devices);
+ manager()->FilterByTechnology(tech, &devices);
return (devices.size() == 1 && devices[0].get() == device.get());
}
@@ -72,8 +72,8 @@
};
TEST_F(ManagerTest, Contains) {
- EXPECT_TRUE(manager_.store()->Contains(flimflam::kStateProperty));
- EXPECT_FALSE(manager_.store()->Contains(""));
+ EXPECT_TRUE(manager()->store()->Contains(flimflam::kStateProperty));
+ EXPECT_FALSE(manager()->store()->Contains(""));
}
TEST_F(ManagerTest, DeviceRegistration) {
@@ -84,9 +84,9 @@
ON_CALL(*mock_device3_.get(), TechnologyIs(Device::kCellular))
.WillByDefault(Return(true));
- manager_.RegisterDevice(mock_device_);
- manager_.RegisterDevice(mock_device2_);
- manager_.RegisterDevice(mock_device3_);
+ manager()->RegisterDevice(mock_device_);
+ manager()->RegisterDevice(mock_device2_);
+ manager()->RegisterDevice(mock_device3_);
EXPECT_TRUE(IsDeviceRegistered(mock_device_, Device::kEthernet));
EXPECT_TRUE(IsDeviceRegistered(mock_device2_, Device::kWifi));
@@ -99,30 +99,38 @@
ON_CALL(*mock_device2_.get(), TechnologyIs(Device::kWifi))
.WillByDefault(Return(true));
- manager_.RegisterDevice(mock_device_.get());
- manager_.RegisterDevice(mock_device2_.get());
+ manager()->RegisterDevice(mock_device_.get());
+ manager()->RegisterDevice(mock_device2_.get());
ASSERT_TRUE(IsDeviceRegistered(mock_device_, Device::kEthernet));
ASSERT_TRUE(IsDeviceRegistered(mock_device2_, Device::kWifi));
EXPECT_CALL(*mock_device_.get(), Stop());
- manager_.DeregisterDevice(mock_device_.get());
+ manager()->DeregisterDevice(mock_device_.get());
EXPECT_FALSE(IsDeviceRegistered(mock_device_, Device::kEthernet));
EXPECT_CALL(*mock_device2_.get(), Stop());
- manager_.DeregisterDevice(mock_device2_.get());
+ manager()->DeregisterDevice(mock_device2_.get());
EXPECT_FALSE(IsDeviceRegistered(mock_device2_, Device::kWifi));
}
TEST_F(ManagerTest, ServiceRegistration) {
+ // It's much easier and safer to use a real GLib for this test.
+ GLib glib;
+ Manager manager(&control_interface_,
+ &dispatcher_,
+ &glib,
+ run_path(),
+ storage_path(),
+ string());
scoped_refptr<MockService> mock_service(
new NiceMock<MockService>(&control_interface_,
&dispatcher_,
- &manager_));
+ &manager));
scoped_refptr<MockService> mock_service2(
new NiceMock<MockService>(&control_interface_,
&dispatcher_,
- &manager_));
+ &manager));
string service1_name(mock_service->UniqueName());
string service2_name(mock_service2->UniqueName());
@@ -131,20 +139,22 @@
EXPECT_CALL(*mock_service2.get(), GetRpcIdentifier())
.WillRepeatedly(Return(service2_name));
// TODO(quiche): make this EXPECT_CALL work (crosbug.com/20154)
- // EXPECT_CALL(*dynamic_cast<ManagerMockAdaptor *>(manager_.adaptor_.get()),
+ // EXPECT_CALL(*dynamic_cast<ManagerMockAdaptor *>(manager.adaptor_.get()),
// EmitRpcIdentifierArrayChanged(flimflam::kServicesProperty, _));
- manager_.RegisterService(mock_service);
- manager_.RegisterService(mock_service2);
+ manager.RegisterService(mock_service);
+ manager.RegisterService(mock_service2);
- vector<string> rpc_ids = manager_.EnumerateAvailableServices();
+ vector<string> rpc_ids = manager.EnumerateAvailableServices();
set<string> ids(rpc_ids.begin(), rpc_ids.end());
EXPECT_EQ(2, ids.size());
EXPECT_TRUE(ContainsKey(ids, mock_service->GetRpcIdentifier()));
EXPECT_TRUE(ContainsKey(ids, mock_service2->GetRpcIdentifier()));
- EXPECT_TRUE(manager_.FindService(service1_name).get() != NULL);
- EXPECT_TRUE(manager_.FindService(service2_name).get() != NULL);
+ EXPECT_TRUE(manager.FindService(service1_name).get() != NULL);
+ EXPECT_TRUE(manager.FindService(service2_name).get() != NULL);
+
+ manager.Stop();
}
TEST_F(ManagerTest, GetProperties) {
@@ -153,10 +163,10 @@
{
::DBus::Error dbus_error;
string expected("portal_list");
- manager_.store()->SetStringProperty(flimflam::kCheckPortalListProperty,
- expected,
- &error);
- DBusAdaptor::GetProperties(manager_.store(), &props, &dbus_error);
+ manager()->store()->SetStringProperty(flimflam::kCheckPortalListProperty,
+ expected,
+ &error);
+ DBusAdaptor::GetProperties(manager()->store(), &props, &dbus_error);
ASSERT_FALSE(props.find(flimflam::kCheckPortalListProperty) == props.end());
EXPECT_EQ(props[flimflam::kCheckPortalListProperty].reader().get_string(),
expected);
@@ -164,10 +174,10 @@
{
::DBus::Error dbus_error;
bool expected = true;
- manager_.store()->SetBoolProperty(flimflam::kOfflineModeProperty,
- expected,
- &error);
- DBusAdaptor::GetProperties(manager_.store(), &props, &dbus_error);
+ manager()->store()->SetBoolProperty(flimflam::kOfflineModeProperty,
+ expected,
+ &error);
+ DBusAdaptor::GetProperties(manager()->store(), &props, &dbus_error);
ASSERT_FALSE(props.find(flimflam::kOfflineModeProperty) == props.end());
EXPECT_EQ(props[flimflam::kOfflineModeProperty].reader().get_bool(),
expected);
@@ -175,13 +185,13 @@
}
TEST_F(ManagerTest, GetDevicesProperty) {
- manager_.RegisterDevice(mock_device_.get());
- manager_.RegisterDevice(mock_device2_.get());
+ manager()->RegisterDevice(mock_device_.get());
+ manager()->RegisterDevice(mock_device2_.get());
{
map<string, ::DBus::Variant> props;
::DBus::Error dbus_error;
bool expected = true;
- DBusAdaptor::GetProperties(manager_.store(), &props, &dbus_error);
+ DBusAdaptor::GetProperties(manager()->store(), &props, &dbus_error);
ASSERT_FALSE(props.find(flimflam::kDevicesProperty) == props.end());
Strings devices =
props[flimflam::kDevicesProperty].operator vector<string>();
@@ -190,29 +200,40 @@
}
TEST_F(ManagerTest, MoveService) {
+ // It's much easier and safer to use a real GLib for this test.
+ GLib glib;
+ Manager manager(&control_interface_,
+ &dispatcher_,
+ &glib,
+ run_path(),
+ storage_path(),
+ string());
+
// I want to ensure that the Profiles are managing this Service object
// lifetime properly, so I can't hold a ref to it here.
- ProfileRefPtr profile(
- new MockProfile(&control_interface_, &glib_, &manager_, ""));
+ ProfileRefPtr profile(new MockProfile(&control_interface_, &manager, ""));
string service_name;
{
- ServiceRefPtr s2(
+ scoped_refptr<MockService> s2(
new MockService(&control_interface_,
&dispatcher_,
- &manager_));
+ &manager));
+ EXPECT_CALL(*s2.get(), Save(_)).WillOnce(Return(true));
+
profile->AdoptService(s2);
s2->set_profile(profile);
service_name = s2->UniqueName();
}
// Now, move the |service| to another profile.
- manager_.MoveToActiveProfile(profile, profile->FindService(service_name));
+ ASSERT_TRUE(manager.MoveToActiveProfile(profile,
+ profile->FindService(service_name)));
// Force destruction of the original Profile, to ensure that the Service
// is kept alive and populated with data.
profile = NULL;
{
- ServiceRefPtr serv(manager_.ActiveProfile()->FindService(service_name));
+ ServiceRefPtr serv(manager.ActiveProfile()->FindService(service_name));
ASSERT_TRUE(serv.get() != NULL);
Error error(Error::kInvalidProperty, "");
::DBus::Error dbus_error;
@@ -226,19 +247,20 @@
EXPECT_EQ(props[flimflam::kAutoConnectProperty].reader().get_bool(),
expected);
}
+ manager.Stop();
}
TEST_F(ManagerTest, Dispatch) {
{
::DBus::Error error;
- EXPECT_TRUE(DBusAdaptor::DispatchOnType(manager_.store(),
+ EXPECT_TRUE(DBusAdaptor::DispatchOnType(manager()->store(),
flimflam::kOfflineModeProperty,
PropertyStoreTest::kBoolV,
&error));
}
{
::DBus::Error error;
- EXPECT_TRUE(DBusAdaptor::DispatchOnType(manager_.store(),
+ EXPECT_TRUE(DBusAdaptor::DispatchOnType(manager()->store(),
flimflam::kCountryProperty,
PropertyStoreTest::kStringV,
&error));
@@ -246,49 +268,49 @@
// Attempt to write with value of wrong type should return InvalidArgs.
{
::DBus::Error error;
- EXPECT_FALSE(DBusAdaptor::DispatchOnType(manager_.store(),
+ EXPECT_FALSE(DBusAdaptor::DispatchOnType(manager()->store(),
flimflam::kCountryProperty,
PropertyStoreTest::kBoolV,
&error));
- EXPECT_EQ(invalid_args_, error.name());
+ EXPECT_EQ(invalid_args(), error.name());
}
{
::DBus::Error error;
- EXPECT_FALSE(DBusAdaptor::DispatchOnType(manager_.store(),
+ EXPECT_FALSE(DBusAdaptor::DispatchOnType(manager()->store(),
flimflam::kOfflineModeProperty,
PropertyStoreTest::kStringV,
&error));
- EXPECT_EQ(invalid_args_, error.name());
+ EXPECT_EQ(invalid_args(), error.name());
}
// Attempt to write R/O property should return InvalidArgs.
{
::DBus::Error error;
EXPECT_FALSE(DBusAdaptor::DispatchOnType(
- manager_.store(),
+ manager()->store(),
flimflam::kEnabledTechnologiesProperty,
PropertyStoreTest::kStringsV,
&error));
- EXPECT_EQ(invalid_args_, error.name());
+ EXPECT_EQ(invalid_args(), error.name());
}
}
TEST_F(ManagerTest, RequestScan) {
{
Error error;
- manager_.RegisterDevice(mock_device_.get());
- manager_.RegisterDevice(mock_device2_.get());
+ manager()->RegisterDevice(mock_device_.get());
+ manager()->RegisterDevice(mock_device2_.get());
EXPECT_CALL(*mock_device_, TechnologyIs(Device::kWifi))
.WillRepeatedly(Return(true));
EXPECT_CALL(*mock_device_, Scan());
EXPECT_CALL(*mock_device2_, TechnologyIs(Device::kWifi))
.WillRepeatedly(Return(false));
EXPECT_CALL(*mock_device2_, Scan()).Times(0);
- manager_.RequestScan(flimflam::kTypeWifi, &error);
+ manager()->RequestScan(flimflam::kTypeWifi, &error);
}
{
Error error;
- manager_.RequestScan("bogus_device_type", &error);
+ manager()->RequestScan("bogus_device_type", &error);
EXPECT_EQ(Error::kInvalidArguments, error.type());
}
}
diff --git a/mock_profile.cc b/mock_profile.cc
index 72e17a5..1c36d8e 100644
--- a/mock_profile.cc
+++ b/mock_profile.cc
@@ -14,21 +14,14 @@
namespace shill {
-class ControlInterface;
-class GLib;
-class Manager;
-
-MockProfile::MockProfile(ControlInterface *control,
- GLib *glib,
- Manager *manager)
- : Profile(control, glib, manager, Identifier("mock"), "", false) {
+MockProfile::MockProfile(ControlInterface *control, Manager *manager)
+ : Profile(control, manager, Identifier("mock"), "", false) {
}
MockProfile::MockProfile(ControlInterface *control,
- GLib *glib,
Manager *manager,
const std::string &identifier)
- : Profile(control, glib, manager, Identifier(identifier), "", false) {
+ : Profile(control, manager, Identifier(identifier), "", false) {
}
MockProfile::~MockProfile() {}
diff --git a/mock_profile.h b/mock_profile.h
index 6d491f6..32381a8 100644
--- a/mock_profile.h
+++ b/mock_profile.h
@@ -11,22 +11,18 @@
#include "shill/profile.h"
-
namespace shill {
-class ControlInterface;
-class GLib;
-class Manager;
-
class MockProfile : public Profile {
public:
- MockProfile(ControlInterface *control, GLib *glib, Manager *manager);
+ MockProfile(ControlInterface *control, Manager *manager);
MockProfile(ControlInterface *control,
- GLib *glib,
Manager *manager,
const std::string &identifier);
virtual ~MockProfile();
+ MOCK_METHOD1(GetStoragePath, bool(FilePath *));
+
private:
DISALLOW_COPY_AND_ASSIGN(MockProfile);
};
diff --git a/mock_service.h b/mock_service.h
index 371ced4..b3a720e 100644
--- a/mock_service.h
+++ b/mock_service.h
@@ -13,10 +13,6 @@
namespace shill {
-class ControlInterface;
-class EventDispatcher;
-class Manager;
-
class MockService : public Service {
public:
// A constructor for the Service object
@@ -34,7 +30,9 @@
MOCK_CONST_METHOD0(failure, ConnectFailure());
MOCK_METHOD0(GetDeviceRpcId, std::string());
MOCK_CONST_METHOD0(GetRpcIdentifier, std::string());
- MOCK_METHOD1(GetStorageIdentifier, std::string(const std::string &));
+ MOCK_METHOD0(GetStorageIdentifier, std::string());
+ MOCK_METHOD1(Load, bool(StoreInterface *));
+ MOCK_METHOD1(Save, bool(StoreInterface *));
private:
DISALLOW_COPY_AND_ASSIGN(MockService);
diff --git a/mock_store.h b/mock_store.h
index 457a762..70259f2 100644
--- a/mock_store.h
+++ b/mock_store.h
@@ -17,8 +17,6 @@
MockStore();
virtual ~MockStore();
- MOCK_METHOD0(Open, bool());
- MOCK_METHOD0(Close, bool());
MOCK_METHOD0(GetGroups, std::set<std::string>());
MOCK_METHOD1(ContainsGroup, bool(const std::string &group));
MOCK_METHOD2(DeleteKey,
diff --git a/profile.cc b/profile.cc
index d0bcd77..3fa0ed1 100644
--- a/profile.cc
+++ b/profile.cc
@@ -19,6 +19,7 @@
#include "shill/manager.h"
#include "shill/property_accessor.h"
#include "shill/service.h"
+#include "shill/store_interface.h"
using std::map;
using std::string;
@@ -27,14 +28,12 @@
namespace shill {
Profile::Profile(ControlInterface *control_interface,
- GLib *glib,
Manager *manager,
const Identifier &name,
const string &user_storage_format,
bool connect_to_rpc)
: manager_(manager),
name_(name),
- storage_(glib),
storage_format_(user_storage_format) {
if (connect_to_rpc)
adaptor_.reset(control_interface->CreateProfileAdaptor(this));
@@ -54,7 +53,9 @@
NULL);
}
-Profile::~Profile() {}
+Profile::~Profile() {
+ DCHECK_EQ(services_.size(), 0);
+}
string Profile::GetFriendlyName() {
return (name_.user.empty() ? "" : name_.user + "/") + name_.identifier;
@@ -103,8 +104,8 @@
return NULL;
}
-void Profile::Finalize() {
- // TODO(cmasone): Flush all of |services_| to disk if needed.
+void Profile::Finalize(StoreInterface *storage) {
+ Save(storage);
services_.clear();
}
@@ -149,6 +150,15 @@
return true;
}
+bool Profile::Load(StoreInterface *storage) {
+ return false;
+}
+
+bool Profile::Save(StoreInterface *storage) {
+ // TODO(cmasone): Persist other profile info to disk.
+ return SaveServices(storage);
+}
+
bool Profile::GetStoragePath(FilePath *path) {
if (name_.user.empty()) {
LOG(ERROR) << "Non-default profiles cannot be stored globally.";
@@ -182,4 +192,15 @@
StringsAccessor(new CustomAccessor<Profile, Strings>(this, get, set)));
}
+bool Profile::SaveServices(StoreInterface *storage) {
+ for (map<string, ServiceRefPtr>::iterator it = services_.begin();
+ it != services_.end();
+ ++it) {
+ VLOG(1) << "Saving service named " << it->first;
+ if (!it->second->Save(storage))
+ return false;
+ }
+ return true;
+}
+
} // namespace shill
diff --git a/profile.h b/profile.h
index 34417fc..cc9fb63 100644
--- a/profile.h
+++ b/profile.h
@@ -12,7 +12,6 @@
#include <base/memory/scoped_ptr.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
-#include "shill/key_file_store.h"
#include "shill/property_store.h"
#include "shill/refptr_types.h"
#include "shill/shill_event.h"
@@ -23,9 +22,9 @@
class ControlInterface;
class Error;
-class GLib;
class Manager;
class ProfileAdaptorInterface;
+class StoreInterface;
class Profile : public base::RefCounted<Profile> {
public:
@@ -41,7 +40,6 @@
};
Profile(ControlInterface *control_interface,
- GLib *glib,
Manager *manager,
const Identifier &name,
const std::string &user_storage_format,
@@ -78,7 +76,7 @@
std::vector<std::string> EnumerateEntries();
// Flush any pending entry info to disk and stop managing service persistence.
- virtual void Finalize();
+ virtual void Finalize(StoreInterface *storage);
// Parses a profile identifier. There're two acceptable forms of the |raw|
// identifier: "identifier" and "~user/identifier". Both "user" and
@@ -86,6 +84,10 @@
// on success.
static bool ParseIdentifier(const std::string &raw, Identifier *parsed);
+ bool Load(StoreInterface *storage);
+
+ virtual bool Save(StoreInterface *storage);
+
// Sets |path| to the persistent store file path for a profile identified by
// |name_|. Returns true on success, and false if unable to determine an
// appropriate file location. |name_| must be a valid identifier,
@@ -116,6 +118,9 @@
Strings(Profile::*get)(void),
bool(Profile::*set)(const Strings&));
+ // Persists |services_| to disk.
+ bool SaveServices(StoreInterface *storage);
+
// Data members shared with subclasses via getter/setters above in the
// protected: section
Manager *manager_;
@@ -127,9 +132,6 @@
// Properties to be gotten via PropertyStore calls.
Identifier name_;
- // Persistent store associated with the profile.
- KeyFileStore storage_;
-
// Format string used to generate paths to user profile directories.
const std::string storage_format_;
diff --git a/profile_unittest.cc b/profile_unittest.cc
index 67002cf..d0643a2 100644
--- a/profile_unittest.cc
+++ b/profile_unittest.cc
@@ -12,12 +12,15 @@
#include "shill/mock_profile.h"
#include "shill/mock_service.h"
+#include "shill/mock_store.h"
#include "shill/property_store_unittest.h"
using std::set;
using std::string;
using std::vector;
+using testing::_;
using testing::Return;
+using testing::SetArgumentPointee;
using testing::StrictMock;
namespace shill {
@@ -25,11 +28,18 @@
class ProfileTest : public PropertyStoreTest {
public:
ProfileTest()
- : profile_(new MockProfile(&control_interface_, &glib_, &manager_, "")) {
+ : profile_(new MockProfile(&control_interface_, manager(), "")) {
+ }
+
+ MockService *CreateMockService() {
+ return new StrictMock<MockService>(&control_interface_,
+ &dispatcher_,
+ manager());
}
protected:
- ProfileRefPtr profile_;
+ MockStore store_;
+ scoped_refptr<MockProfile> profile_;
};
TEST_F(ProfileTest, IsValidIdentifierToken) {
@@ -80,10 +90,10 @@
Profile::Identifier id;
id.identifier = kIdentifier;
ProfileRefPtr profile(
- new Profile(&control_interface_, &glib_, &manager_, id, "", false));
+ new Profile(&control_interface_, manager(), id, "", false));
EXPECT_EQ(kIdentifier, profile->GetFriendlyName());
id.user = kUser;
- profile = new Profile(&control_interface_, &glib_, &manager_, id, "", false);
+ profile = new Profile(&control_interface_, manager(), id, "", false);
EXPECT_EQ(string(kUser) + "/" + kIdentifier, profile->GetFriendlyName());
}
@@ -95,11 +105,11 @@
Profile::Identifier id;
id.identifier = kIdentifier;
ProfileRefPtr profile(
- new Profile(&control_interface_, &glib_, &manager_, id, "", false));
+ new Profile(&control_interface_, manager(), id, "", false));
EXPECT_FALSE(profile->GetStoragePath(&path));
id.user = kUser;
profile =
- new Profile(&control_interface_, &glib_, &manager_, id, kFormat, false);
+ new Profile(&control_interface_, manager(), id, kFormat, false);
EXPECT_TRUE(profile->GetStoragePath(&path));
string suffix = base::StringPrintf("/%s.profile", kIdentifier);
EXPECT_EQ(base::StringPrintf(kFormat, kUser) + suffix, path.value());
@@ -109,14 +119,8 @@
string service1_name;
string service2_name;
{
- ServiceRefPtr service1(
- new StrictMock<MockService>(&control_interface_,
- &dispatcher_,
- &manager_));
- ServiceRefPtr service2(
- new StrictMock<MockService>(&control_interface_,
- &dispatcher_,
- &manager_));
+ ServiceRefPtr service1(CreateMockService());
+ ServiceRefPtr service2(CreateMockService());
service1_name = service1->UniqueName();
service2_name = service2->UniqueName();
ASSERT_TRUE(profile_->AdoptService(service1));
@@ -136,15 +140,27 @@
ASSERT_TRUE(profile_->FindService(service2_name).get() == NULL);
}
+TEST_F(ProfileTest, Finalize) {
+ scoped_refptr<MockService> service1(CreateMockService());
+ scoped_refptr<MockService> service2(CreateMockService());
+ EXPECT_CALL(*service1.get(), Save(_)).WillOnce(Return(true));
+ EXPECT_CALL(*service2.get(), Save(_)).WillOnce(Return(true));
+
+ ASSERT_TRUE(profile_->AdoptService(service1));
+ ASSERT_TRUE(profile_->AdoptService(service2));
+
+ profile_->Finalize(&store_);
+}
+
TEST_F(ProfileTest, EntryEnumeration) {
scoped_refptr<MockService> service1(
new StrictMock<MockService>(&control_interface_,
&dispatcher_,
- &manager_));
+ manager()));
scoped_refptr<MockService> service2(
new StrictMock<MockService>(&control_interface_,
&dispatcher_,
- &manager_));
+ manager()));
string service1_name(service1->UniqueName());
string service2_name(service2->UniqueName());
diff --git a/property_store_unittest.cc b/property_store_unittest.cc
index 2717f7a..560563f 100644
--- a/property_store_unittest.cc
+++ b/property_store_unittest.cc
@@ -62,24 +62,30 @@
DBusAdaptor::Uint32ToVariant(0);
PropertyStoreTest::PropertyStoreTest()
- : manager_(&control_interface_,
+ : invalid_args_(Error::GetName(Error::kInvalidArguments)),
+ invalid_prop_(Error::GetName(Error::kInvalidProperty)),
+ path_(dir_.CreateUniqueTempDir() ? dir_.path().value() : ""),
+ manager_(&control_interface_,
&dispatcher_,
&glib_,
- run_dir_.path().value(),
- storage_dir_.path().value(),
- string()),
- invalid_args_(Error::GetName(Error::kInvalidArguments)),
- invalid_prop_(Error::GetName(Error::kInvalidProperty)) {
+ run_path(),
+ storage_path(),
+ string()) {
}
PropertyStoreTest::~PropertyStoreTest() {}
+void PropertyStoreTest::SetUp() {
+ ASSERT_FALSE(run_path().empty());
+ ASSERT_FALSE(storage_path().empty());
+}
+
TEST_P(PropertyStoreTest, TestProperty) {
// Ensure that an attempt to write unknown properties returns InvalidProperty.
PropertyStore store;
::DBus::Error error;
EXPECT_FALSE(DBusAdaptor::DispatchOnType(&store, "", GetParam(), &error));
- EXPECT_EQ(invalid_prop_, error.name());
+ EXPECT_EQ(invalid_prop(), error.name());
}
INSTANTIATE_TEST_CASE_P(
diff --git a/property_store_unittest.h b/property_store_unittest.h
index 6e451c5..0fb3472 100644
--- a/property_store_unittest.h
+++ b/property_store_unittest.h
@@ -9,6 +9,7 @@
#include <string>
#include <vector>
+#include <base/memory/scoped_ptr.h>
#include <base/memory/scoped_temp_dir.h>
#include <dbus-c++/dbus.h>
#include <gmock/gmock.h>
@@ -46,15 +47,29 @@
PropertyStoreTest();
virtual ~PropertyStoreTest();
+ virtual void SetUp();
+
protected:
- ScopedTempDir run_dir_;
- ScopedTempDir storage_dir_;
+ Manager *manager() { return &manager_; }
+
+ const std::string &run_path() const { return path_; }
+ const std::string &storage_path() const { return path_; }
+
+ const std::string &invalid_args() const { return invalid_args_; }
+ const std::string &invalid_prop() const { return invalid_prop_; }
+
+ // TODO(cmasone): make these private as per http://crosbug.com/19573
MockControl control_interface_;
EventDispatcher dispatcher_;
- Manager manager_;
MockGLib glib_;
- std::string invalid_args_;
- std::string invalid_prop_;
+
+ private:
+ const std::string invalid_args_;
+ const std::string invalid_prop_;
+ ScopedTempDir dir_;
+ const std::string path_;
+ Manager manager_;
+
};
} // namespace shill
diff --git a/service.cc b/service.cc
index d1ed9a4..29b2f3a 100644
--- a/service.cc
+++ b/service.cc
@@ -188,8 +188,8 @@
return adaptor_->GetRpcIdentifier();
}
-bool Service::Load(StoreInterface *storage, const string &id_suffix) {
- const string id = GetStorageIdentifier(id_suffix);
+bool Service::Load(StoreInterface *storage) {
+ const string id = GetStorageIdentifier();
if (!storage->ContainsGroup(id)) {
LOG(WARNING) << "Service is not available in the persistent store: " << id;
return false;
@@ -218,8 +218,8 @@
return true;
}
-bool Service::Save(StoreInterface *storage, const string &id_suffix) {
- const string id = GetStorageIdentifier(id_suffix);
+bool Service::Save(StoreInterface *storage) {
+ const string id = GetStorageIdentifier();
// TODO(petkov): We could choose to simplify the saving code by removing most
// conditionals thus saving even default values.
diff --git a/service.h b/service.h
index 836d38e..88e27b2 100644
--- a/service.h
+++ b/service.h
@@ -113,15 +113,13 @@
virtual std::string GetRpcIdentifier() const;
// Returns the unique persistent storage identifier for the service.
- // |mac| is the associated Device's MAC address to be used in the storage id.
- // We do this for flimflam compatibility, for now.
- virtual std::string GetStorageIdentifier(const std::string &mac) = 0;
+ virtual std::string GetStorageIdentifier() = 0;
// Loads the service from persistent |storage|. Returns true on success.
- virtual bool Load(StoreInterface *storage, const std::string &id_suffix);
+ virtual bool Load(StoreInterface *storage);
// Saves the service to persistent |storage|. Returns true on success.
- virtual bool Save(StoreInterface *storage, const std::string &id_suffix);
+ virtual bool Save(StoreInterface *storage);
bool auto_connect() const { return auto_connect_; }
void set_auto_connect(bool connect) { auto_connect_ = connect; }
@@ -130,6 +128,9 @@
void set_error(const std::string &error) { error_ = error; }
// These are defined in service.cc so that we don't have to include profile.h
+ // TODO(cmasone): right now, these are here only so that we can get the
+ // profile name as a property. Can we store just the name, and then handle
+ // setting the profile for this service via |manager_|?
const ProfileRefPtr &profile() const;
void set_profile(const ProfileRefPtr &p);
@@ -139,6 +140,9 @@
// Returns true if a character is allowed to be in a service storage id.
static bool LegalChar(char a) { return isalnum(a) || a == '_'; }
+ const std::string &name() const { return name_; }
+ void set_name(const std::string &n) { name_ = n; }
+
virtual std::string CalculateState();
void HelpRegisterDerivedBool(const std::string &name,
@@ -222,7 +226,7 @@
EventDispatcher *dispatcher_;
static unsigned int serial_number_;
- const std::string name_;
+ std::string name_;
bool available_;
bool configured_;
Configuration *configuration_;
diff --git a/service_unittest.cc b/service_unittest.cc
index 7d7d4fb..302f504 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -53,7 +53,7 @@
virtual string CalculateState() { return ""; }
virtual string GetRpcIdentifier() const { return ServiceMockAdaptor::kRpcId; }
virtual string GetDeviceRpcId() { return kRpcId; }
- virtual string GetStorageIdentifier(const string &mac) { return kStorageId; }
+ virtual string GetStorageIdentifier() { return kStorageId; }
private:
DISALLOW_COPY_AND_ASSIGN(ServiceUnderTest);
@@ -69,7 +69,8 @@
service_(new ServiceUnderTest(&control_interface_,
&dispatcher_,
&mock_manager_)),
- storage_id_(ServiceUnderTest::kStorageId) {}
+ storage_id_(ServiceUnderTest::kStorageId) {
+ }
virtual ~ServiceTest() {}
@@ -164,7 +165,7 @@
flimflam::kFavoriteProperty,
PropertyStoreTest::kBoolV,
&error));
- EXPECT_EQ(invalid_args_, error.name());
+ EXPECT_EQ(invalid_args(), error.name());
}
}
@@ -174,13 +175,13 @@
EXPECT_CALL(storage, GetString(storage_id_, _, _))
.Times(AtLeast(1))
.WillRepeatedly(Return(true));
- EXPECT_TRUE(service_->Load(&storage, ""));
+ EXPECT_TRUE(service_->Load(&storage));
}
TEST_F(ServiceTest, LoadFail) {
StrictMock<MockStore> storage;
EXPECT_CALL(storage, ContainsGroup(storage_id_)).WillOnce(Return(false));
- EXPECT_FALSE(service_->Load(&storage, ""));
+ EXPECT_FALSE(service_->Load(&storage));
}
TEST_F(ServiceTest, SaveString) {
@@ -225,7 +226,7 @@
EXPECT_CALL(storage, DeleteKey(storage_id_, _))
.Times(AtLeast(1))
.WillRepeatedly(Return(true));
- EXPECT_TRUE(service_->Save(&storage, ""));
+ EXPECT_TRUE(service_->Save(&storage));
}
TEST_F(ServiceTest, State) {
diff --git a/store_interface.h b/store_interface.h
index b03d3c0..283b205 100644
--- a/store_interface.h
+++ b/store_interface.h
@@ -16,18 +16,6 @@
public:
virtual ~StoreInterface() {}
- // Opens the store. Returns true on success. This method must be invoked
- // before using any of the getters or setters. This method is not required
- // complete gracefully if invoked on a store that has been opened already but
- // not closed yet.
- virtual bool Open() = 0;
-
- // Closes the store and flushes it to persistent storage. Returns true on
- // success. Note that the store is considered closed even if Close returns
- // false. This method is not required to complete gracefully if invoked on a
- // store that has not been opened successfully or has been closed already.
- virtual bool Close() = 0;
-
// Returns a set of all groups contained in the store.
virtual std::set<std::string> GetGroups() = 0;
diff --git a/wifi_service.cc b/wifi_service.cc
index 4127b55..f581e7a 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -52,6 +52,10 @@
store->RegisterConstString(flimflam::kWifiHexSsid, &hex_ssid_);
hex_ssid_ = base::HexEncode(&(*ssid_.begin()), ssid_.size());
+ set_name(name() +
+ "_" +
+ string(reinterpret_cast<const char*>(ssid_.data()), ssid_.size()));
+
// TODO(quiche): set based on security properties
need_passphrase_ = false;
// TODO(quiche): figure out when to set true
@@ -76,10 +80,10 @@
// XXX remove from favorite networks list?
}
-string WiFiService::GetStorageIdentifier(const std::string &mac) {
+string WiFiService::GetStorageIdentifier() {
return StringToLowerASCII(base::StringPrintf("%s_%s_%s_%s_%s",
flimflam::kTypeWifi,
- mac.c_str(),
+ wifi_->address().c_str(),
hex_ssid_.c_str(),
mode_.c_str(),
security_.c_str()));
diff --git a/wifi_service.h b/wifi_service.h
index 8e10df2..6784f20 100644
--- a/wifi_service.h
+++ b/wifi_service.h
@@ -35,7 +35,7 @@
virtual void Disconnect();
// wifi_<MAC>_<BSSID>_<mode_string>_<security_string>
- std::string GetStorageIdentifier(const std::string &mac);
+ std::string GetStorageIdentifier();
const std::string &mode() const;
const std::string &key_management() const;
diff --git a/wifi_service_unittest.cc b/wifi_service_unittest.cc
index 0d23d9f..cc5819d 100644
--- a/wifi_service_unittest.cc
+++ b/wifi_service_unittest.cc
@@ -35,16 +35,22 @@
TEST_F(WiFiServiceTest, StorageId) {
vector<uint8_t> ssid(5, 0);
ssid.push_back(0xff);
-
- WiFiServiceRefPtr wifi = new WiFiService(&control_interface_,
- &dispatcher_,
- &manager_,
- NULL,
- ssid,
- flimflam::kModeManaged,
- "none");
static const char fake_mac[] = "AaBBcCDDeeFF";
- string id = wifi->GetStorageIdentifier(fake_mac);
+
+ WiFiRefPtr wifi = new WiFi(&control_interface_,
+ &dispatcher_,
+ manager(),
+ "wifi",
+ fake_mac,
+ 0);
+ WiFiServiceRefPtr wifi_service = new WiFiService(&control_interface_,
+ &dispatcher_,
+ manager(),
+ wifi,
+ ssid,
+ flimflam::kModeManaged,
+ "none");
+ string id = wifi_service->GetStorageIdentifier();
for (uint i = 0; i < id.length(); ++i) {
EXPECT_TRUE(id[i] == '_' ||
isxdigit(id[i]) ||
diff --git a/wifi_unittest.cc b/wifi_unittest.cc
index 5cb2c76..3ffb266 100644
--- a/wifi_unittest.cc
+++ b/wifi_unittest.cc
@@ -94,7 +94,7 @@
flimflam::kScanningProperty,
PropertyStoreTest::kBoolV,
&error));
- EXPECT_EQ(invalid_args_, error.name());
+ EXPECT_EQ(invalid_args(), error.name());
}
}