shill: teach property accessors how to clear properties,
and update code that instantiates property accessors
accordingly.
BUG=chromium-os:24814
TEST=new unit tests
Change-Id: Iae385c331648e74916c2eb2b69c41ccc9cdcafdf
Reviewed-on: https://gerrit.chromium.org/gerrit/15289
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Tested-by: mukesh agrawal <quiche@chromium.org>
Commit-Ready: mukesh agrawal <quiche@chromium.org>
diff --git a/accessor_interface.h b/accessor_interface.h
index 86843e7..0419dd1 100644
--- a/accessor_interface.h
+++ b/accessor_interface.h
@@ -30,6 +30,8 @@
AccessorInterface() {}
virtual ~AccessorInterface() {}
+ // Reset the property to its default value. Sets |error| on failure.
+ virtual void Clear(Error *error) = 0;
// Provides read-only access. Sets |error| on failure.
virtual T Get(Error *error) = 0;
// Attempts to set the wrapped value. Sets |error| on failure.
diff --git a/property_accessor.h b/property_accessor.h
index 03fa7c0..39cd611 100644
--- a/property_accessor.h
+++ b/property_accessor.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// 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.
@@ -15,9 +15,12 @@
namespace shill {
// Templated implementations of AccessorInterface<>.
-// PropertyAccessor<> and ConstPropertyAccessor<> respectively provide
-// R/W and R/O access to the value pointed to by |property|.
-// this allows a class to easily map strings to member variables, so that
+//
+// PropertyAccessor<>, ConstPropertyAccessor<>, and
+// WriteOnlyPropertyAccessor<> provide R/W, R/O, and W/O access
+// (respectively) to the value pointed to by |property|.
+//
+// This allows a class to easily map strings to member variables, so that
// pieces of state stored in the class can be queried or updated by name.
//
// bool foo = true;
@@ -27,14 +30,18 @@
// accessors["foo"]->Set(false); // returns true, because setting is allowed.
// // foo == false, new_foo == true
// new_foo = accessors["foo"]->Get(); // new_foo == false
+// // Clear resets |foo| to its value when the PropertyAccessor was created.
+// accessors["foo"]->Clear(); // foo == true
template <class T>
class PropertyAccessor : public AccessorInterface<T> {
public:
- explicit PropertyAccessor(T *property) : property_(property) {
+ explicit PropertyAccessor(T *property)
+ : property_(property), default_value_(*property) {
DCHECK(property);
}
virtual ~PropertyAccessor() {}
+ void Clear(Error *error) { Set(default_value_, error); }
T Get(Error */*error*/) { return *property_; }
void Set(const T &value, Error */*error*/) {
*property_ = value;
@@ -42,6 +49,7 @@
private:
T * const property_;
+ const T default_value_;
DISALLOW_COPY_AND_ASSIGN(PropertyAccessor);
};
@@ -53,6 +61,11 @@
}
virtual ~ConstPropertyAccessor() {}
+ void Clear(Error *error) {
+ // TODO(quiche): check if this is the right error.
+ // (maybe Error::kInvalidProperty instead?)
+ error->Populate(Error::kInvalidArguments, "Property is read-only");
+ }
T Get(Error */*error*/) { return *property_; }
void Set(const T &/*value*/, Error *error) {
// TODO(quiche): check if this is the right error.
@@ -68,11 +81,13 @@
template <class T>
class WriteOnlyPropertyAccessor : public AccessorInterface<T> {
public:
- explicit WriteOnlyPropertyAccessor(T *property) : property_(property) {
+ explicit WriteOnlyPropertyAccessor(T *property)
+ : property_(property), default_value_(*property) {
DCHECK(property);
}
virtual ~WriteOnlyPropertyAccessor() {}
+ void Clear(Error *error) { Set(default_value_, error); }
T Get(Error *error) {
error->Populate(Error::kPermissionDenied, "Property is write-only");
return T();
@@ -87,12 +102,16 @@
FRIEND_TEST(PropertyAccessorTest, StringCorrectness);
T * const property_;
+ const T default_value_;
DISALLOW_COPY_AND_ASSIGN(WriteOnlyPropertyAccessor);
};
// CustomAccessor<> allows custom getter and setter methods to be provided.
-// Thus, if the state to be returned is to be derived on-demand, we can
-// still fit it into the AccessorInterface<> framework.
+// Thus, if the state to be returned is to be derived on-demand, or if
+// setting the property requires validation, we can still fit it into the
+// AccessorInterface<> framework.
+//
+// If the property is write-only, use CustomWriteOnlyAccessor instead.
template<class C, class T>
class CustomAccessor : public AccessorInterface<T> {
public:
@@ -104,20 +123,22 @@
T(C::*getter)(Error *error),
void(C::*setter)(const T &value, Error *error))
: target_(target),
+ default_value_(),
getter_(getter),
setter_(setter) {
DCHECK(target);
+ DCHECK(getter); // otherwise, use CustomWriteOnlyAccessor
+ if (setter_) {
+ Error e;
+ default_value_ = Get(&e);
+ }
}
virtual ~CustomAccessor() {}
+ void Clear(Error *error) { Set(default_value_, error); }
T Get(Error *error) {
- if (getter_)
- return storage_ = (target_->*getter_)(error);
-
- error->Populate(Error::kPermissionDenied, "Property is write-only");
- return T();
+ return (target_->*getter_)(error);
}
-
void Set(const T &value, Error *error) {
if (setter_) {
(target_->*setter_)(value, error);
@@ -127,15 +148,70 @@
}
private:
- C * const target_;
- // Get() returns a const&, so we need to have internal storage to which to
- // return a reference.
- T storage_;
- T(C::*getter_)(Error *error);
- void(C::*setter_)(const T &value, Error *error);
+ C *const target_;
+ // |default_value_| is non-const because it can't be initialized in
+ // the initializer list.
+ T default_value_;
+ T(C::*const getter_)(Error *error);
+ void(C::*const setter_)(const T &value, Error *error);
DISALLOW_COPY_AND_ASSIGN(CustomAccessor);
};
+// CustomWriteOnlyAccessor<> allows a custom writer method to be provided.
+// Get returns an error automatically. Clear resets the value to a
+// default value.
+template<class C, class T>
+class CustomWriteOnlyAccessor : public AccessorInterface<T> {
+ public:
+ // |target| is the object on which to call |setter| and |clearer|.
+ //
+ // |target| and |setter| must be non-NULL.
+ //
+ // Either |clearer| or |default_value|, but not both, must be non-NULL.
+ // Whichever is non-NULL is used to clear the property.
+ CustomWriteOnlyAccessor(C *target,
+ void(C::*setter)(const T &value, Error *error),
+ void(C::*clearer)(Error *error),
+ const T *default_value)
+ : target_(target),
+ setter_(setter),
+ clearer_(clearer),
+ default_value_() {
+ DCHECK(target);
+ DCHECK(setter);
+ DCHECK(clearer || default_value);
+ DCHECK(!clearer || !default_value);
+ if (default_value) {
+ default_value_ = *default_value;
+ }
+ }
+ virtual ~CustomWriteOnlyAccessor() {}
+
+ void Clear(Error *error) {
+ if (clearer_) {
+ (target_->*clearer_)(error);
+ } else {
+ Set(default_value_, error);
+ }
+ }
+ T Get(Error *error) {
+ error->Populate(Error::kPermissionDenied, "Property is write-only");
+ return T();
+ }
+ void Set(const T &value, Error *error) {
+ (target_->*setter_)(value, error);
+ }
+
+ private:
+ C *const target_;
+ void(C::*const setter_)(const T &value, Error *error);
+ void(C::*const clearer_)(Error *error);
+ // |default_value_| is non-const because it can't be initialized in
+ // the initializer list.
+ T default_value_;
+ DISALLOW_COPY_AND_ASSIGN(CustomWriteOnlyAccessor);
+};
+
} // namespace shill
#endif // SHILL_PROPERTY_ACCESSOR_
diff --git a/property_accessor_unittest.cc b/property_accessor_unittest.cc
index e65c576..56586ac 100644
--- a/property_accessor_unittest.cc
+++ b/property_accessor_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// 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.
@@ -27,14 +27,19 @@
int32 int_store = 0;
{
Error error;
+ int32 orig_value = int_store;
Int32Accessor accessor(new PropertyAccessor<int32>(&int_store));
EXPECT_EQ(int_store, accessor->Get(&error));
int32 expected_int32 = 127;
accessor->Set(expected_int32, &error);
- ASSERT_TRUE(error.IsSuccess());
+ EXPECT_TRUE(error.IsSuccess());
EXPECT_EQ(expected_int32, accessor->Get(&error));
+ accessor->Clear(&error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(orig_value, accessor->Get(&error));
+
int_store = std::numeric_limits<int32>::max();
EXPECT_EQ(std::numeric_limits<int32>::max(), accessor->Get(&error));
}
@@ -54,9 +59,15 @@
}
{
Error error;
+ Int32Accessor accessor(new ConstPropertyAccessor<int32>(&int_store));
+ accessor->Clear(&error);
+ ASSERT_FALSE(error.IsSuccess());
+ }
+ {
+ Error error;
Int32Accessor accessor(new WriteOnlyPropertyAccessor<int32>(&int_store));
accessor->Get(&error);
- ASSERT_TRUE(error.IsFailure());
+ EXPECT_TRUE(error.IsFailure());
EXPECT_EQ(Error::kPermissionDenied, error.type());
}
{
@@ -64,7 +75,7 @@
int32 expected_int32 = 127;
WriteOnlyPropertyAccessor<int32> accessor(&expected_int32);
accessor.Set(expected_int32, &error);
- ASSERT_TRUE(error.IsSuccess());
+ EXPECT_TRUE(error.IsSuccess());
EXPECT_EQ(expected_int32, *accessor.property_);
EXPECT_EQ(int32(), accessor.Get(&error));
ASSERT_FALSE(error.IsSuccess());
@@ -72,20 +83,35 @@
expected_int32 = std::numeric_limits<int32>::max();
EXPECT_EQ(std::numeric_limits<int32>::max(), *accessor.property_);
}
+ {
+ Error error;
+ int32 orig_value = int_store = 0;
+ WriteOnlyPropertyAccessor<int32> accessor(&int_store);
+
+ accessor.Set(127, &error);
+ accessor.Clear(&error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(orig_value, *accessor.property_);
+ }
}
TEST(PropertyAccessorTest, UnsignedIntCorrectness) {
uint32 int_store = 0;
{
Error error;
+ uint32 orig_value = int_store;
Uint32Accessor accessor(new PropertyAccessor<uint32>(&int_store));
EXPECT_EQ(int_store, accessor->Get(&error));
uint32 expected_uint32 = 127;
accessor->Set(expected_uint32, &error);
- ASSERT_TRUE(error.IsSuccess());
+ EXPECT_TRUE(error.IsSuccess());
EXPECT_EQ(expected_uint32, accessor->Get(&error));
+ accessor->Clear(&error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(orig_value, accessor->Get(&error));
+
int_store = std::numeric_limits<uint32>::max();
EXPECT_EQ(std::numeric_limits<uint32>::max(), accessor->Get(&error));
}
@@ -105,9 +131,15 @@
}
{
Error error;
+ Uint32Accessor accessor(new ConstPropertyAccessor<uint32>(&int_store));
+ accessor->Clear(&error);
+ ASSERT_FALSE(error.IsSuccess());
+ }
+ {
+ Error error;
Uint32Accessor accessor(new WriteOnlyPropertyAccessor<uint32>(&int_store));
accessor->Get(&error);
- ASSERT_TRUE(error.IsFailure());
+ EXPECT_TRUE(error.IsFailure());
EXPECT_EQ(Error::kPermissionDenied, error.type());
}
{
@@ -115,7 +147,7 @@
uint32 expected_uint32 = 127;
WriteOnlyPropertyAccessor<uint32> accessor(&expected_uint32);
accessor.Set(expected_uint32, &error);
- ASSERT_TRUE(error.IsSuccess());
+ EXPECT_TRUE(error.IsSuccess());
EXPECT_EQ(expected_uint32, *accessor.property_);
EXPECT_EQ(uint32(), accessor.Get(&error));
ASSERT_FALSE(error.IsSuccess());
@@ -123,20 +155,35 @@
expected_uint32 = std::numeric_limits<uint32>::max();
EXPECT_EQ(std::numeric_limits<uint32>::max(), *accessor.property_);
}
+ {
+ Error error;
+ uint32 orig_value = int_store = 0;
+ WriteOnlyPropertyAccessor<uint32> accessor(&int_store);
+
+ accessor.Set(127, &error);
+ accessor.Clear(&error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(orig_value, *accessor.property_);
+ }
}
TEST(PropertyAccessorTest, StringCorrectness) {
string storage;
{
Error error;
+ string orig_value = storage;
StringAccessor accessor(new PropertyAccessor<string>(&storage));
EXPECT_EQ(storage, accessor->Get(&error));
string expected_string("what");
accessor->Set(expected_string, &error);
- ASSERT_TRUE(error.IsSuccess());
+ EXPECT_TRUE(error.IsSuccess());
EXPECT_EQ(expected_string, accessor->Get(&error));
+ accessor->Clear(&error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(orig_value, accessor->Get(&error));
+
storage = "nooooo";
EXPECT_EQ(storage, accessor->Get(&error));
}
@@ -156,9 +203,15 @@
}
{
Error error;
+ StringAccessor accessor(new ConstPropertyAccessor<string>(&storage));
+ accessor->Clear(&error);
+ ASSERT_FALSE(error.IsSuccess());
+ }
+ {
+ Error error;
StringAccessor accessor(new WriteOnlyPropertyAccessor<string>(&storage));
accessor->Get(&error);
- ASSERT_TRUE(error.IsFailure());
+ EXPECT_TRUE(error.IsFailure());
EXPECT_EQ(Error::kPermissionDenied, error.type());
}
{
@@ -166,7 +219,7 @@
string expected_string = "what";
WriteOnlyPropertyAccessor<string> accessor(&expected_string);
accessor.Set(expected_string, &error);
- ASSERT_TRUE(error.IsSuccess());
+ EXPECT_TRUE(error.IsSuccess());
EXPECT_EQ(expected_string, *accessor.property_);
EXPECT_EQ(string(), accessor.Get(&error));
ASSERT_FALSE(error.IsSuccess());
@@ -174,6 +227,155 @@
expected_string = "nooooo";
EXPECT_EQ("nooooo", *accessor.property_);
}
+ {
+ Error error;
+ string orig_value = storage = "original value";
+ WriteOnlyPropertyAccessor<string> accessor(&storage);
+ accessor.Set("new value", &error);
+ accessor.Clear(&error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(orig_value, *accessor.property_);
+ }
+}
+
+class StringWrapper {
+ public:
+ string Get(Error */*error*/) {
+ return value_;
+ }
+ void Set(const string &value, Error */*error*/) {
+ value_ = value;
+ }
+ void Clear(Error */*error*/) {
+ value_.clear();
+ }
+
+ string value_;
+};
+
+TEST(PropertyAccessorTest, CustomAccessorCorrectness) {
+ StringWrapper wrapper;
+ {
+ // Custom accessor: read, write, clear, read-updated.
+ Error error;
+ const string orig_value = wrapper.value_ = "original value";
+ CustomAccessor<StringWrapper, string> accessor(&wrapper,
+ &StringWrapper::Get,
+ &StringWrapper::Set);
+ EXPECT_EQ(orig_value, accessor.Get(&error));
+ EXPECT_TRUE(error.IsSuccess());
+
+ const string expected_string = "new value";
+ accessor.Set(expected_string, &error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(expected_string, accessor.Get(&error));
+
+ accessor.Clear(&error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(orig_value, accessor.Get(&error));
+
+ wrapper.value_ = "nooooo";
+ EXPECT_EQ(wrapper.value_, accessor.Get(&error));
+ }
+ {
+ // Custom read-only accessor: read, write, read-updated.
+ Error error;
+ CustomAccessor<StringWrapper, string> accessor(&wrapper,
+ &StringWrapper::Get,
+ NULL);
+ EXPECT_EQ(wrapper.value_, accessor.Get(&error));
+
+ const string expected_string = "what";
+ accessor.Set(expected_string, &error);
+ ASSERT_FALSE(error.IsSuccess());
+ EXPECT_EQ(Error::kInvalidArguments, error.type());
+ EXPECT_EQ(wrapper.value_, accessor.Get(&error));
+
+ wrapper.value_ = "nooooo";
+ EXPECT_EQ(wrapper.value_, accessor.Get(&error));
+ }
+ {
+ // Custom read-only accessor: clear.
+ Error error;
+ CustomAccessor<StringWrapper, string> accessor(&wrapper,
+ &StringWrapper::Get,
+ NULL);
+ accessor.Clear(&error);
+ ASSERT_FALSE(error.IsSuccess());
+ }
+}
+
+TEST(PropertyAccessorTest, CustomWriteOnlyAccessorWithDefault) {
+ StringWrapper wrapper;
+ {
+ // Test reading.
+ Error error;
+ const string default_value = "default value";
+ CustomWriteOnlyAccessor<StringWrapper, string> accessor(
+ &wrapper, &StringWrapper::Set, NULL, &default_value);
+ wrapper.value_ = "can't read this";
+ EXPECT_EQ(string(), accessor.Get(&error));
+ EXPECT_TRUE(error.IsFailure());
+ EXPECT_EQ(Error::kPermissionDenied, error.type());
+ }
+ {
+ // Test writing.
+ Error error;
+ const string default_value = "default value";
+ const string expected_string = "what";
+ CustomWriteOnlyAccessor<StringWrapper, string> accessor(
+ &wrapper, &StringWrapper::Set, NULL, &default_value);
+ accessor.Set(expected_string, &error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(expected_string, wrapper.value_);
+ }
+ {
+ // Test clearing.
+ Error error;
+ const string default_value = "default value";
+ CustomWriteOnlyAccessor<StringWrapper, string> accessor(
+ &wrapper, &StringWrapper::Set, NULL, &default_value);
+ accessor.Set("new value", &error);
+ EXPECT_EQ("new value", wrapper.value_);
+ accessor.Clear(&error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(default_value, wrapper.value_);
+ }
+}
+
+TEST(PropertyAccessorTest, CustomWriteOnlyAccessorWithClear) {
+ StringWrapper wrapper;
+ {
+ // Test reading.
+ Error error;
+ CustomWriteOnlyAccessor<StringWrapper, string> accessor(
+ &wrapper, &StringWrapper::Set, &StringWrapper::Clear, NULL);
+ wrapper.value_ = "can't read this";
+ EXPECT_EQ(string(), accessor.Get(&error));
+ EXPECT_TRUE(error.IsFailure());
+ EXPECT_EQ(Error::kPermissionDenied, error.type());
+ }
+ {
+ // Test writing.
+ Error error;
+ const string expected_string = "what";
+ CustomWriteOnlyAccessor<StringWrapper, string> accessor(
+ &wrapper, &StringWrapper::Set, &StringWrapper::Clear, NULL);
+ accessor.Set(expected_string, &error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(expected_string, wrapper.value_);
+ }
+ {
+ // Test clearing.
+ Error error;
+ CustomWriteOnlyAccessor<StringWrapper, string> accessor(
+ &wrapper, &StringWrapper::Set, &StringWrapper::Clear, NULL);
+ accessor.Set("new value", &error);
+ EXPECT_EQ("new value", wrapper.value_);
+ accessor.Clear(&error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ("", wrapper.value_);
+ }
}
} // namespace shill
diff --git a/service.cc b/service.cc
index caaaad9..e6eccd1 100644
--- a/service.cc
+++ b/service.cc
@@ -125,14 +125,18 @@
store_.RegisterString(flimflam::kEAPClientCertProperty, &eap_.client_cert);
store_.RegisterString(flimflam::kEAPCertIDProperty, &eap_.cert_id);
store_.RegisterString(flimflam::kEapPrivateKeyProperty, &eap_.private_key);
- HelpRegisterDerivedString(flimflam::kEapPrivateKeyPasswordProperty, NULL,
- &Service::SetEAPPrivateKeyPassword);
+ HelpRegisterWriteOnlyDerivedString(flimflam::kEapPrivateKeyPasswordProperty,
+ &Service::SetEAPPrivateKeyPassword,
+ NULL,
+ &eap_.private_key_password);
store_.RegisterString(flimflam::kEAPKeyIDProperty, &eap_.key_id);
store_.RegisterString(flimflam::kEapCaCertProperty, &eap_.ca_cert);
store_.RegisterString(flimflam::kEapCaCertIDProperty, &eap_.ca_cert_id);
store_.RegisterString(flimflam::kEAPPINProperty, &eap_.pin);
- HelpRegisterDerivedString(flimflam::kEapPasswordProperty, NULL,
- &Service::SetEAPPassword);
+ HelpRegisterWriteOnlyDerivedString(flimflam::kEapPasswordProperty,
+ &Service::SetEAPPassword,
+ NULL,
+ &eap_.password);
store_.RegisterString(flimflam::kEapKeyMgmtProperty, &eap_.key_management);
store_.RegisterBool(flimflam::kEapUseSystemCAsProperty, &eap_.use_system_cas);
@@ -599,6 +603,18 @@
Uint16Accessor(new CustomAccessor<Service, uint16>(this, get, set)));
}
+void Service::HelpRegisterWriteOnlyDerivedString(
+ const string &name,
+ void(Service::*set)(const string &, Error *),
+ void(Service::*clear)(Error *),
+ const string *default_value) {
+ store_.RegisterDerivedString(
+ name,
+ StringAccessor(
+ new CustomWriteOnlyAccessor<Service, string>(
+ this, set, clear, default_value)));
+}
+
void Service::SaveString(StoreInterface *storage,
const string &id,
const string &key,
diff --git a/service.h b/service.h
index 4f90443..f00e99b 100644
--- a/service.h
+++ b/service.h
@@ -280,6 +280,11 @@
// this service is busy with another connection.
virtual bool IsAutoConnectable() const;
+ // HelpRegisterDerived*: Expose a property over RPC, with the name |name|.
+ //
+ // Reads of the property will be handled by invoking |get|.
+ // Writes to the property will be handled by invoking |set|.
+ // Clearing the property will be handled by PropertyStore.
void HelpRegisterDerivedBool(
const std::string &name,
bool(Service::*get)(Error *error),
@@ -292,6 +297,20 @@
const std::string &name,
uint16(Service::*get)(Error *error),
void(Service::*set)(const uint16 &value, Error *error));
+ // Expose a property over RPC, with the name |name|.
+ //
+ // Reads of the property will be handled by invoking |get|.
+ // Writes to the property will be handled by invoking |set|.
+ //
+ // Clearing the property will be handled by invoking |clear|, or
+ // calling |set| with |default_value| (whichever is non-NULL). It
+ // is an error to call this method with both |clear| and
+ // |default_value| non-NULL.
+ void HelpRegisterWriteOnlyDerivedString(
+ const std::string &name,
+ void(Service::*set)(const std::string &value, Error *error),
+ void(Service::*clear)(Error *error),
+ const std::string *default_value);
ServiceAdaptorInterface *adaptor() const { return adaptor_.get(); }
diff --git a/wifi_service.cc b/wifi_service.cc
index b4ef8bb..6ef3dbb 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -60,10 +60,10 @@
ssid_(ssid) {
PropertyStore *store = this->mutable_store();
store->RegisterConstString(flimflam::kModeProperty, &mode_);
- HelpRegisterDerivedString(store,
- flimflam::kPassphraseProperty,
- NULL,
- &WiFiService::SetPassphrase);
+ HelpRegisterWriteOnlyDerivedString(flimflam::kPassphraseProperty,
+ &WiFiService::SetPassphrase,
+ &WiFiService::ClearPassphrase,
+ NULL);
store->RegisterBool(flimflam::kPassphraseRequiredProperty, &need_passphrase_);
store->RegisterConstString(flimflam::kSecurityProperty, &security_);
@@ -206,6 +206,13 @@
UpdateConnectable();
}
+// ClearPassphrase is separate from SetPassphrase, because the default
+// value for |passphrase_| would not pass validation.
+void WiFiService::ClearPassphrase(Error */*error*/) {
+ passphrase_.clear();
+ UpdateConnectable();
+}
+
bool WiFiService::IsLoadableFrom(StoreInterface *storage) const {
return storage->ContainsGroup(GetGenericStorageIdentifier()) ||
storage->ContainsGroup(GetSpecificStorageIdentifier());
@@ -335,14 +342,16 @@
}
// private methods
-void WiFiService::HelpRegisterDerivedString(
- PropertyStore *store,
- const std::string &name,
- std::string(WiFiService::*get)(Error *),
- void(WiFiService::*set)(const std::string&, Error *)) {
- store->RegisterDerivedString(
+void WiFiService::HelpRegisterWriteOnlyDerivedString(
+ const string &name,
+ void(WiFiService::*set)(const string &, Error *),
+ void(WiFiService::*clear)(Error *),
+ const string *default_value) {
+ mutable_store()->RegisterDerivedString(
name,
- StringAccessor(new CustomAccessor<WiFiService, string>(this, get, set)));
+ StringAccessor(
+ new CustomWriteOnlyAccessor<WiFiService, string>(
+ this, set, clear, default_value)));
}
void WiFiService::ConnectTask() {
diff --git a/wifi_service.h b/wifi_service.h
index ca28e10..bce9e52 100644
--- a/wifi_service.h
+++ b/wifi_service.h
@@ -105,16 +105,19 @@
FRIEND_TEST(WiFiServiceTest, LoadAndUnloadPassphrase);
FRIEND_TEST(WiFiServiceTest, Populate8021x);
- void HelpRegisterDerivedString(
- PropertyStore *store,
+ // Override the base clase implementation, because we need to allow
+ // arguments that aren't base class methods.
+ void HelpRegisterWriteOnlyDerivedString(
const std::string &name,
- std::string(WiFiService::*get)(Error *error),
- void(WiFiService::*set)(const std::string &value, Error *error));
+ void(WiFiService::*set)(const std::string &value, Error *error),
+ void(WiFiService::*clear)(Error *error),
+ const std::string *default_value);
void ConnectTask();
void DisconnectTask();
std::string GetDeviceRpcId(Error *error);
+ void ClearPassphrase(Error *error);
void UpdateConnectable();
static void ValidateWEPPassphrase(const std::string &passphrase,