shill: Implement write-only properties
Certain properties (e.g. WIFI Passphrase) are write only and must
not be returned when Service.GetProperties() is called over D-Bus.
This CL implements WriteOnlyProperties, a write-only analog of the
read-only ConstProperties.
Also add a ReadablePropertyConstIterator which only returns the
readable properties. Switch over DBus adaptor and PropertyStore
to use that.
BUG=chromium-os:21196
TEST=Added 2 new unittests.
Change-Id: I52815cc395650e0b49e1acac8d4954deeebcee5d
Reviewed-on: https://gerrit.chromium.org/gerrit/11402
Commit-Ready: Gaurav Shah <gauravsh@chromium.org>
Reviewed-by: Gaurav Shah <gauravsh@chromium.org>
Tested-by: Gaurav Shah <gauravsh@chromium.org>
diff --git a/accessor_interface.h b/accessor_interface.h
index 68b2837..efb85ef 100644
--- a/accessor_interface.h
+++ b/accessor_interface.h
@@ -28,8 +28,8 @@
AccessorInterface() {}
virtual ~AccessorInterface() {}
- // Provides read-only access.
- virtual const T &Get() = 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.
virtual void Set(const T &value, Error *error) = 0;
diff --git a/cellular.cc b/cellular.cc
index 85097a5..c3ab9b1 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -712,7 +712,7 @@
}
}
-StrIntPair Cellular::SimLockStatusToProperty() {
+StrIntPair Cellular::SimLockStatusToProperty(Error */*error*/) {
return StrIntPair(make_pair(flimflam::kSIMLockTypeProperty,
sim_lock_status_.lock_type),
make_pair(flimflam::kSIMLockRetriesLeftProperty,
@@ -721,7 +721,7 @@
void Cellular::HelpRegisterDerivedStrIntPair(
const string &name,
- StrIntPair(Cellular::*get)(void),
+ StrIntPair(Cellular::*get)(Error *),
void(Cellular::*set)(const StrIntPair&, Error *)) {
mutable_store()->RegisterDerivedStrIntPair(
name,
diff --git a/cellular.h b/cellular.h
index 5f280cd..9cea7d7 100644
--- a/cellular.h
+++ b/cellular.h
@@ -264,11 +264,11 @@
// to the network-connected state and bring the network interface up.
void EstablishLink();
- StrIntPair SimLockStatusToProperty();
+ StrIntPair SimLockStatusToProperty(Error *error);
void HelpRegisterDerivedStrIntPair(
const std::string &name,
- StrIntPair(Cellular::*get)(void),
+ StrIntPair(Cellular::*get)(Error *),
void(Cellular::*set)(const StrIntPair&, Error *));
void InitCapability();
diff --git a/cellular_service.cc b/cellular_service.cc
index 8011bc6..2a5ff93 100644
--- a/cellular_service.cc
+++ b/cellular_service.cc
@@ -68,7 +68,7 @@
return id;
}
-string CellularService::GetDeviceRpcId() {
+string CellularService::GetDeviceRpcId(Error */*error*/) {
return cellular_->GetRpcIdentifier();
}
diff --git a/cellular_service.h b/cellular_service.h
index 4f1b3b3..d304999 100644
--- a/cellular_service.h
+++ b/cellular_service.h
@@ -64,7 +64,7 @@
private:
static const char kServiceType[];
- virtual std::string GetDeviceRpcId();
+ virtual std::string GetDeviceRpcId(Error *error);
// Properties
std::string activation_state_;
diff --git a/dbus_adaptor.cc b/dbus_adaptor.cc
index cf98f8f..67d4d73 100644
--- a/dbus_adaptor.cc
+++ b/dbus_adaptor.cc
@@ -87,50 +87,53 @@
bool DBusAdaptor::GetProperties(const PropertyStore &store,
map<string, ::DBus::Variant> *out,
::DBus::Error */*error*/) {
+ Error e;
{
- PropertyConstIterator<bool> it = store.GetBoolPropertiesIter();
+ ReadablePropertyConstIterator<bool> it = store.GetBoolPropertiesIter();
for ( ; !it.AtEnd(); it.Advance())
- (*out)[it.Key()] = BoolToVariant(it.Value());
+ (*out)[it.Key()] = BoolToVariant(it.Value(&e));
}
{
- PropertyConstIterator<int16> it = store.GetInt16PropertiesIter();
+ ReadablePropertyConstIterator<int16> it = store.GetInt16PropertiesIter();
for ( ; !it.AtEnd(); it.Advance())
- (*out)[it.Key()] = Int16ToVariant(it.Value());
+ (*out)[it.Key()] = Int16ToVariant(it.Value(&e));
}
{
- PropertyConstIterator<int32> it = store.GetInt32PropertiesIter();
+ ReadablePropertyConstIterator<int32> it = store.GetInt32PropertiesIter();
for ( ; !it.AtEnd(); it.Advance())
- (*out)[it.Key()] = Int32ToVariant(it.Value());
+ (*out)[it.Key()] = Int32ToVariant(it.Value(&e));
}
{
- PropertyConstIterator<string> it = store.GetStringPropertiesIter();
+ ReadablePropertyConstIterator<string> it = store.GetStringPropertiesIter();
for ( ; !it.AtEnd(); it.Advance())
- (*out)[it.Key()] = StringToVariant(it.Value());
+ (*out)[it.Key()] = StringToVariant(it.Value(&e));
}
{
- PropertyConstIterator<Stringmap> it = store.GetStringmapPropertiesIter();
+ ReadablePropertyConstIterator<Stringmap> it =
+ store.GetStringmapPropertiesIter();
for ( ; !it.AtEnd(); it.Advance())
- (*out)[it.Key()] = StringmapToVariant(it.Value());
+ (*out)[it.Key()]= StringmapToVariant(it.Value(&e));
}
{
- PropertyConstIterator<Strings> it = store.GetStringsPropertiesIter();
+ ReadablePropertyConstIterator<Strings> it =
+ store.GetStringsPropertiesIter();
for ( ; !it.AtEnd(); it.Advance())
- (*out)[it.Key()] = StringsToVariant(it.Value());
+ (*out)[it.Key()] = StringsToVariant(it.Value(&e));
}
{
- PropertyConstIterator<uint8> it = store.GetUint8PropertiesIter();
+ ReadablePropertyConstIterator<uint8> it = store.GetUint8PropertiesIter();
for ( ; !it.AtEnd(); it.Advance())
- (*out)[it.Key()] = ByteToVariant(it.Value());
+ (*out)[it.Key()] = ByteToVariant(it.Value(&e));
}
{
- PropertyConstIterator<uint16> it = store.GetUint16PropertiesIter();
+ ReadablePropertyConstIterator<uint16> it = store.GetUint16PropertiesIter();
for ( ; !it.AtEnd(); it.Advance())
- (*out)[it.Key()] = Uint16ToVariant(it.Value());
+ (*out)[it.Key()] = Uint16ToVariant(it.Value(&e));
}
{
- PropertyConstIterator<uint32> it = store.GetUint32PropertiesIter();
+ ReadablePropertyConstIterator<uint32> it = store.GetUint32PropertiesIter();
for ( ; !it.AtEnd(); it.Advance())
- (*out)[it.Key()] = Uint32ToVariant(it.Value());
+ (*out)[it.Key()] = Uint32ToVariant(it.Value(&e));
}
return true;
}
@@ -139,7 +142,7 @@
void DBusAdaptor::ArgsToKeyValueStore(
const map<string, ::DBus::Variant> &args,
KeyValueStore *out,
- Error *error) { // XXX should be ::DBus::Error?
+ Error *error) { // TODO(quiche): Should be ::DBus::Error?
for (map<string, ::DBus::Variant>::const_iterator it = args.begin();
it != args.end();
++it) {
@@ -152,7 +155,7 @@
out->SetBool(it->first, it->second.reader().get_bool());
} else {
error->Populate(Error::kInternalError);
- return; // skip remaining args after error
+ return; // Skip remaining args after error.
}
}
}
diff --git a/device.cc b/device.cc
index 056b827..e389056 100644
--- a/device.cc
+++ b/device.cc
@@ -245,7 +245,7 @@
void Device::HelpRegisterDerivedStrings(
const string &name,
- Strings(Device::*get)(void),
+ Strings(Device::*get)(Error *),
void(Device::*set)(const Strings&, Error *)) {
store_.RegisterDerivedStrings(
name,
@@ -307,7 +307,7 @@
return StringPrintf("%s:%s", suffix.c_str(), ipconfig_->type().c_str());
}
-vector<string> Device::AvailableIPConfigs() {
+vector<string> Device::AvailableIPConfigs(Error */*error*/) {
string id = (ipconfig_.get() ? ipconfig_->GetRpcIdentifier() : string());
return vector<string>(1, id);
}
diff --git a/device.h b/device.h
index f61f1ef..4f3d3c0 100644
--- a/device.h
+++ b/device.h
@@ -133,7 +133,7 @@
void SetServiceFailure(Service::ConnectFailure failure_state);
void HelpRegisterDerivedStrings(const std::string &name,
- Strings(Device::*get)(void),
+ Strings(Device::*get)(Error *),
void(Device::*set)(const Strings&, Error *));
// Property getters reserved for subclasses
@@ -158,7 +158,7 @@
// |suffix| is injected into the storage identifier used for the configs.
std::string SerializeIPConfigs(const std::string &suffix);
- std::vector<std::string> AvailableIPConfigs();
+ std::vector<std::string> AvailableIPConfigs(Error *error);
std::string GetRpcConnectionIdentifier();
// Properties
diff --git a/error.cc b/error.cc
index 5ab022e..5fd4ec4 100644
--- a/error.cc
+++ b/error.cc
@@ -42,7 +42,7 @@
const char Error::kInterfaceName[] = SHILL_INTERFACE;
Error::Error() {
- Populate(kSuccess);
+ Reset();
}
Error::Error(Type type) {
@@ -65,6 +65,10 @@
message_ = message;
}
+void Error::Reset() {
+ Populate(kSuccess);
+}
+
bool Error::ToDBusError(::DBus::Error *error) const {
if (IsFailure()) {
error->set(GetName(type_).c_str(), message_.c_str());
diff --git a/error.h b/error.h
index a8159a0..bee2465 100644
--- a/error.h
+++ b/error.h
@@ -49,6 +49,8 @@
void Populate(Type type); // Uses the default message for |type|.
void Populate(Type type, const std::string &message);
+ void Reset();
+
// Sets the DBus |error| and returns true if Error represents failure.
// Leaves |error| unchanged, and returns false, otherwise.
bool ToDBusError(::DBus::Error *error) const;
diff --git a/error_unittest.cc b/error_unittest.cc
index aa84dbc..fa67b29 100644
--- a/error_unittest.cc
+++ b/error_unittest.cc
@@ -34,6 +34,13 @@
EXPECT_EQ(kMessage, e.message());
}
+TEST_F(ErrorTest, Reset) {
+ Error e(Error::kAlreadyExists);
+ e.Reset();
+ EXPECT_EQ(Error::kSuccess, e.type());
+ EXPECT_EQ(Error::GetDefaultMessage(Error::kSuccess), e.message());
+}
+
TEST_F(ErrorTest, PopulateDefaultMessage) {
Error e;
e.Populate(Error::kInternalError);
diff --git a/ethernet_service.cc b/ethernet_service.cc
index 7a8f363..a450460 100644
--- a/ethernet_service.cc
+++ b/ethernet_service.cc
@@ -49,7 +49,7 @@
return ethernet_->TechnologyIs(type);
}
-std::string EthernetService::GetDeviceRpcId() {
+std::string EthernetService::GetDeviceRpcId(Error */*error*/) {
return ethernet_->GetRpcIdentifier();
}
diff --git a/ethernet_service.h b/ethernet_service.h
index 4a38958..fdee17a 100644
--- a/ethernet_service.h
+++ b/ethernet_service.h
@@ -36,7 +36,7 @@
private:
static const char kServiceType[];
- std::string GetDeviceRpcId();
+ std::string GetDeviceRpcId(Error *error);
EthernetRefPtr ethernet_;
DISALLOW_COPY_AND_ASSIGN(EthernetService);
diff --git a/manager.cc b/manager.cc
index bd73bd7..b61f7aa 100644
--- a/manager.cc
+++ b/manager.cc
@@ -388,7 +388,7 @@
void Manager::HelpRegisterDerivedString(
const string &name,
- string(Manager::*get)(void),
+ string(Manager::*get)(Error *),
void(Manager::*set)(const string&, Error *)) {
store_.RegisterDerivedString(
name,
@@ -397,7 +397,7 @@
void Manager::HelpRegisterDerivedStrings(
const string &name,
- Strings(Manager::*get)(void),
+ Strings(Manager::*get)(Error *),
void(Manager::*set)(const Strings &, Error *)) {
store_.RegisterDerivedStrings(
name,
@@ -408,27 +408,27 @@
sort(services_.begin(), services_.end(), ServiceSorter(technology_order_));
}
-string Manager::CalculateState() {
+string Manager::CalculateState(Error */*error*/) {
return flimflam::kStateOffline;
}
-vector<string> Manager::AvailableTechnologies() {
+vector<string> Manager::AvailableTechnologies(Error */*error*/) {
return vector<string>();
}
-vector<string> Manager::ConnectedTechnologies() {
+vector<string> Manager::ConnectedTechnologies(Error */*error*/) {
return vector<string>();
}
-string Manager::DefaultTechnology() {
+string Manager::DefaultTechnology(Error */*error*/) {
return "";
}
-vector<string> Manager::EnabledTechnologies() {
+vector<string> Manager::EnabledTechnologies(Error */*error*/) {
return vector<string>();
}
-vector<string> Manager::EnumerateDevices() {
+vector<string> Manager::EnumerateDevices(Error */*error*/) {
vector<string> device_rpc_ids;
for (vector<DeviceRefPtr>::const_iterator it = devices_.begin();
it != devices_.end();
@@ -438,7 +438,7 @@
return device_rpc_ids;
}
-vector<string> Manager::EnumerateAvailableServices() {
+vector<string> Manager::EnumerateAvailableServices(Error */*error*/) {
vector<string> service_rpc_ids;
for (vector<ServiceRefPtr>::const_iterator it = services_.begin();
it != services_.end();
@@ -448,12 +448,12 @@
return service_rpc_ids;
}
-vector<string> Manager::EnumerateWatchedServices() {
+vector<string> Manager::EnumerateWatchedServices(Error *error) {
// TODO(cmasone): Filter this list for services in appropriate states.
- return EnumerateAvailableServices();
+ return EnumerateAvailableServices(error);
}
-string Manager::GetActiveProfileName() {
+string Manager::GetActiveProfileName(Error */*error*/) {
return ActiveProfile()->GetFriendlyName();
}
diff --git a/manager.h b/manager.h
index 71c5a0f..5df868c 100644
--- a/manager.h
+++ b/manager.h
@@ -68,7 +68,7 @@
std::vector<DeviceRefPtr> *found);
ServiceRefPtr FindService(const std::string& name);
- std::vector<std::string> EnumerateAvailableServices();
+ std::vector<std::string> EnumerateAvailableServices(Error *error);
// called via RPC (e.g., from ManagerDBusAdaptor)
WiFiServiceRefPtr GetWifiService(const KeyValueStore &args, Error *error);
@@ -101,23 +101,23 @@
static const char kManagerErrorNoDevice[];
- std::string CalculateState();
- std::vector<std::string> AvailableTechnologies();
- std::vector<std::string> ConnectedTechnologies();
- std::string DefaultTechnology();
- std::vector<std::string> EnabledTechnologies();
- std::vector<std::string> EnumerateDevices();
+ std::string CalculateState(Error *error);
+ std::vector<std::string> AvailableTechnologies(Error *error);
+ std::vector<std::string> ConnectedTechnologies(Error *error);
+ std::string DefaultTechnology(Error *error);
+ std::vector<std::string> EnabledTechnologies(Error *error);
+ std::vector<std::string> EnumerateDevices(Error *error);
// TODO(cmasone): This should be implemented by filtering |services_|.
- std::vector<std::string> EnumerateWatchedServices();
- std::string GetActiveProfileName();
+ std::vector<std::string> EnumerateWatchedServices(Error *error);
+ std::string GetActiveProfileName(Error *error);
void HelpRegisterDerivedString(
const std::string &name,
- std::string(Manager::*get)(void),
+ std::string(Manager::*get)(Error *),
void(Manager::*set)(const std::string&, Error *));
void HelpRegisterDerivedStrings(
const std::string &name,
- Strings(Manager::*get)(void),
+ Strings(Manager::*get)(Error *),
void(Manager::*set)(const Strings&, Error *));
void PopProfileInternal();
diff --git a/manager_unittest.cc b/manager_unittest.cc
index 05b2765..c88897b 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -229,7 +229,8 @@
manager.RegisterService(mock_service);
manager.RegisterService(mock_service2);
- vector<string> rpc_ids = manager.EnumerateAvailableServices();
+ Error error;
+ vector<string> rpc_ids = manager.EnumerateAvailableServices(&error);
set<string> ids(rpc_ids.begin(), rpc_ids.end());
EXPECT_EQ(2, ids.size());
EXPECT_TRUE(ContainsKey(ids, mock_service->GetRpcIdentifier()));
@@ -453,8 +454,9 @@
EXPECT_EQ(Error::kAlreadyExists, TestPushProfile(&manager, kProfile0));
EXPECT_EQ(Error::kAlreadyExists, TestPushProfile(&manager, kProfile1));
+ Error error;
// Active profile should be the last one we pushed.
- EXPECT_EQ(kProfile1, "~" + manager.GetActiveProfileName());
+ EXPECT_EQ(kProfile1, "~" + manager.GetActiveProfileName(&error));
// Make sure a profile name that doesn't exist fails.
const char kProfile2Id[] = "profile2";
diff --git a/mock_service.h b/mock_service.h
index 7de6948..a9d738a 100644
--- a/mock_service.h
+++ b/mock_service.h
@@ -23,14 +23,14 @@
MOCK_METHOD1(Connect, void(Error *error));
MOCK_METHOD0(Disconnect, void());
- MOCK_METHOD0(CalculateState, std::string());
+ MOCK_METHOD1(CalculateState, std::string(Error *error));
MOCK_CONST_METHOD1(TechnologyIs,
bool(const Technology::Identifier technology));
MOCK_METHOD1(SetState, void(ConnectState state));
MOCK_CONST_METHOD0(state, ConnectState());
MOCK_METHOD1(SetFailure, void(ConnectFailure failure));
MOCK_CONST_METHOD0(failure, ConnectFailure());
- MOCK_METHOD0(GetDeviceRpcId, std::string());
+ MOCK_METHOD1(GetDeviceRpcId, std::string(Error *error));
MOCK_CONST_METHOD0(GetRpcIdentifier, std::string());
MOCK_CONST_METHOD0(GetStorageIdentifier, std::string());
MOCK_METHOD1(Load, bool(StoreInterface *store_interface));
diff --git a/profile.cc b/profile.cc
index 7ba3aef..f922d39 100644
--- a/profile.cc
+++ b/profile.cc
@@ -202,11 +202,11 @@
return true;
}
-vector<string> Profile::EnumerateAvailableServices() {
- return manager_->EnumerateAvailableServices();
+vector<string> Profile::EnumerateAvailableServices(Error *error) {
+ return manager_->EnumerateAvailableServices(error);
}
-vector<string> Profile::EnumerateEntries() {
+vector<string> Profile::EnumerateEntries(Error */*error*/) {
// TODO(someone): Determine if we care about this wasteful copying; consider
// making GetGroups return a vector.
set<string> groups(storage_->GetGroups());
@@ -215,7 +215,7 @@
void Profile::HelpRegisterDerivedStrings(
const string &name,
- Strings(Profile::*get)(void),
+ Strings(Profile::*get)(Error *),
void(Profile::*set)(const Strings&, Error *)) {
store_.RegisterDerivedStrings(
name,
diff --git a/profile.h b/profile.h
index b694eef..6d56ef3 100644
--- a/profile.h
+++ b/profile.h
@@ -89,8 +89,8 @@
// Return whether |service| can configure itself from the profile.
bool ContainsService(const ServiceConstRefPtr &service);
- std::vector<std::string> EnumerateAvailableServices();
- std::vector<std::string> EnumerateEntries();
+ std::vector<std::string> EnumerateAvailableServices(Error *error);
+ std::vector<std::string> EnumerateEntries(Error *error);
// Write all in-memory state to disk via |storage_|.
virtual bool Save();
@@ -129,7 +129,7 @@
void HelpRegisterDerivedStrings(
const std::string &name,
- Strings(Profile::*get)(void),
+ Strings(Profile::*get)(Error *),
void(Profile::*set)(const Strings&, Error *));
// Data members shared with subclasses via getter/setters above in the
diff --git a/profile_unittest.cc b/profile_unittest.cc
index f352671..e014df2 100644
--- a/profile_unittest.cc
+++ b/profile_unittest.cc
@@ -224,19 +224,20 @@
string service1_name(service1->UniqueName());
string service2_name(service2->UniqueName());
+ Error error;
ASSERT_TRUE(profile_->AdoptService(service1));
ASSERT_TRUE(profile_->AdoptService(service2));
- ASSERT_EQ(profile_->EnumerateEntries().size(), 2);
+ ASSERT_EQ(profile_->EnumerateEntries(&error).size(), 2);
ASSERT_TRUE(profile_->AbandonService(service1));
- ASSERT_EQ(profile_->EnumerateEntries()[0], service2_name);
+ ASSERT_EQ(profile_->EnumerateEntries(&error)[0], service2_name);
ASSERT_TRUE(profile_->AbandonService(service1));
- ASSERT_EQ(profile_->EnumerateEntries()[0], service2_name);
+ ASSERT_EQ(profile_->EnumerateEntries(&error)[0], service2_name);
ASSERT_TRUE(profile_->AbandonService(service2));
- ASSERT_EQ(profile_->EnumerateEntries().size(), 0);
+ ASSERT_EQ(profile_->EnumerateEntries(&error).size(), 0);
}
TEST_F(ProfileTest, MatchesIdentifier) {
diff --git a/property_accessor.h b/property_accessor.h
index 542fd8d..306f9c8 100644
--- a/property_accessor.h
+++ b/property_accessor.h
@@ -7,6 +7,7 @@
#include <base/basictypes.h>
#include <base/logging.h>
+#include <gtest/gtest_prod.h> // for FRIEND_TEST.
#include "shill/accessor_interface.h"
#include "shill/error.h"
@@ -34,7 +35,7 @@
}
virtual ~PropertyAccessor() {}
- const T &Get() { return *property_; }
+ T Get(Error */*error*/) { return *property_; }
void Set(const T &value, Error */*error*/) {
*property_ = value;
}
@@ -52,7 +53,7 @@
}
virtual ~ConstPropertyAccessor() {}
- const T &Get() { return *property_; }
+ T Get(Error */*error*/) { return *property_; }
void Set(const T &/*value*/, Error *error) {
// TODO(quiche): check if this is the right error.
// (maybe Error::kPermissionDenied instead?)
@@ -64,6 +65,31 @@
DISALLOW_COPY_AND_ASSIGN(ConstPropertyAccessor);
};
+template <class T>
+class WriteOnlyPropertyAccessor : public AccessorInterface<T> {
+ public:
+ explicit WriteOnlyPropertyAccessor(T *property) : property_(property) {
+ DCHECK(property);
+ }
+ virtual ~WriteOnlyPropertyAccessor() {}
+
+ T Get(Error *error) {
+ error->Populate(Error::kPermissionDenied, "Property is write-only");
+ return T();
+ }
+ void Set(const T &value, Error */*error*/) {
+ *property_ = value;
+ }
+
+ private:
+ FRIEND_TEST(PropertyAccessorTest, SignedIntCorrectness);
+ FRIEND_TEST(PropertyAccessorTest, UnsignedIntCorrectness);
+ FRIEND_TEST(PropertyAccessorTest, StringCorrectness);
+
+ T * const property_;
+ 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.
@@ -75,17 +101,23 @@
// attempts to set via the accessor.
// It is an error to pass NULL for either of the other two arguments.
CustomAccessor(C *target,
- T(C::*getter)(),
+ T(C::*getter)(Error *),
void(C::*setter)(const T&, Error *))
: target_(target),
getter_(getter),
setter_(setter) {
DCHECK(target);
- DCHECK(getter);
}
virtual ~CustomAccessor() {}
- const T &Get() { return storage_ = (target_->*getter_)(); }
+ T Get(Error *error) {
+ if (getter_)
+ return storage_ = (target_->*getter_)(error);
+
+ error->Populate(Error::kPermissionDenied, "Property is write-only");
+ return T();
+ }
+
void Set(const T &value, Error *error) {
if (setter_) {
(target_->*setter_)(value, error);
@@ -99,7 +131,7 @@
// Get() returns a const&, so we need to have internal storage to which to
// return a reference.
T storage_;
- T(C::*getter_)(void);
+ T(C::*getter_)(Error *);
void(C::*setter_)(const T&, Error *);
DISALLOW_COPY_AND_ASSIGN(CustomAccessor);
};
diff --git a/property_accessor_unittest.cc b/property_accessor_unittest.cc
index 4303d13..e65c576 100644
--- a/property_accessor_unittest.cc
+++ b/property_accessor_unittest.cc
@@ -28,29 +28,49 @@
{
Error error;
Int32Accessor accessor(new PropertyAccessor<int32>(&int_store));
- EXPECT_EQ(int_store, accessor->Get());
+ EXPECT_EQ(int_store, accessor->Get(&error));
int32 expected_int32 = 127;
accessor->Set(expected_int32, &error);
ASSERT_TRUE(error.IsSuccess());
- EXPECT_EQ(expected_int32, accessor->Get());
+ EXPECT_EQ(expected_int32, accessor->Get(&error));
int_store = std::numeric_limits<int32>::max();
- EXPECT_EQ(std::numeric_limits<int32>::max(), accessor->Get());
+ EXPECT_EQ(std::numeric_limits<int32>::max(), accessor->Get(&error));
}
{
Error error;
Int32Accessor accessor(new ConstPropertyAccessor<int32>(&int_store));
- EXPECT_EQ(int_store, accessor->Get());
+ EXPECT_EQ(int_store, accessor->Get(&error));
int32 expected_int32 = 127;
accessor->Set(expected_int32, &error);
ASSERT_FALSE(error.IsSuccess());
EXPECT_EQ(Error::kInvalidArguments, error.type());
- EXPECT_EQ(int_store, accessor->Get());
+ EXPECT_EQ(int_store, accessor->Get(&error));
int_store = std::numeric_limits<int32>::max();
- EXPECT_EQ(std::numeric_limits<int32>::max(), accessor->Get());
+ EXPECT_EQ(std::numeric_limits<int32>::max(), accessor->Get(&error));
+ }
+ {
+ Error error;
+ Int32Accessor accessor(new WriteOnlyPropertyAccessor<int32>(&int_store));
+ accessor->Get(&error);
+ ASSERT_TRUE(error.IsFailure());
+ EXPECT_EQ(Error::kPermissionDenied, error.type());
+ }
+ {
+ Error error;
+ int32 expected_int32 = 127;
+ WriteOnlyPropertyAccessor<int32> accessor(&expected_int32);
+ accessor.Set(expected_int32, &error);
+ ASSERT_TRUE(error.IsSuccess());
+ EXPECT_EQ(expected_int32, *accessor.property_);
+ EXPECT_EQ(int32(), accessor.Get(&error));
+ ASSERT_FALSE(error.IsSuccess());
+
+ expected_int32 = std::numeric_limits<int32>::max();
+ EXPECT_EQ(std::numeric_limits<int32>::max(), *accessor.property_);
}
}
@@ -59,29 +79,49 @@
{
Error error;
Uint32Accessor accessor(new PropertyAccessor<uint32>(&int_store));
- EXPECT_EQ(int_store, accessor->Get());
+ EXPECT_EQ(int_store, accessor->Get(&error));
uint32 expected_uint32 = 127;
accessor->Set(expected_uint32, &error);
ASSERT_TRUE(error.IsSuccess());
- EXPECT_EQ(expected_uint32, accessor->Get());
+ EXPECT_EQ(expected_uint32, accessor->Get(&error));
int_store = std::numeric_limits<uint32>::max();
- EXPECT_EQ(std::numeric_limits<uint32>::max(), accessor->Get());
+ EXPECT_EQ(std::numeric_limits<uint32>::max(), accessor->Get(&error));
}
{
Error error;
Uint32Accessor accessor(new ConstPropertyAccessor<uint32>(&int_store));
- EXPECT_EQ(int_store, accessor->Get());
+ EXPECT_EQ(int_store, accessor->Get(&error));
uint32 expected_uint32 = 127;
accessor->Set(expected_uint32, &error);
ASSERT_FALSE(error.IsSuccess());
EXPECT_EQ(Error::kInvalidArguments, error.type());
- EXPECT_EQ(int_store, accessor->Get());
+ EXPECT_EQ(int_store, accessor->Get(&error));
int_store = std::numeric_limits<uint32>::max();
- EXPECT_EQ(std::numeric_limits<uint32>::max(), accessor->Get());
+ EXPECT_EQ(std::numeric_limits<uint32>::max(), accessor->Get(&error));
+ }
+ {
+ Error error;
+ Uint32Accessor accessor(new WriteOnlyPropertyAccessor<uint32>(&int_store));
+ accessor->Get(&error);
+ ASSERT_TRUE(error.IsFailure());
+ EXPECT_EQ(Error::kPermissionDenied, error.type());
+ }
+ {
+ Error error;
+ uint32 expected_uint32 = 127;
+ WriteOnlyPropertyAccessor<uint32> accessor(&expected_uint32);
+ accessor.Set(expected_uint32, &error);
+ ASSERT_TRUE(error.IsSuccess());
+ EXPECT_EQ(expected_uint32, *accessor.property_);
+ EXPECT_EQ(uint32(), accessor.Get(&error));
+ ASSERT_FALSE(error.IsSuccess());
+
+ expected_uint32 = std::numeric_limits<uint32>::max();
+ EXPECT_EQ(std::numeric_limits<uint32>::max(), *accessor.property_);
}
}
@@ -90,29 +130,49 @@
{
Error error;
StringAccessor accessor(new PropertyAccessor<string>(&storage));
- EXPECT_EQ(storage, accessor->Get());
+ EXPECT_EQ(storage, accessor->Get(&error));
string expected_string("what");
accessor->Set(expected_string, &error);
ASSERT_TRUE(error.IsSuccess());
- EXPECT_EQ(expected_string, accessor->Get());
+ EXPECT_EQ(expected_string, accessor->Get(&error));
storage = "nooooo";
- EXPECT_EQ(storage, accessor->Get());
+ EXPECT_EQ(storage, accessor->Get(&error));
}
{
Error error;
StringAccessor accessor(new ConstPropertyAccessor<string>(&storage));
- EXPECT_EQ(storage, accessor->Get());
+ EXPECT_EQ(storage, accessor->Get(&error));
string expected_string("what");
accessor->Set(expected_string, &error);
ASSERT_FALSE(error.IsSuccess());
EXPECT_EQ(Error::kInvalidArguments, error.type());
- EXPECT_EQ(storage, accessor->Get());
+ EXPECT_EQ(storage, accessor->Get(&error));
storage = "nooooo";
- EXPECT_EQ(storage, accessor->Get());
+ EXPECT_EQ(storage, accessor->Get(&error));
+ }
+ {
+ Error error;
+ StringAccessor accessor(new WriteOnlyPropertyAccessor<string>(&storage));
+ accessor->Get(&error);
+ ASSERT_TRUE(error.IsFailure());
+ EXPECT_EQ(Error::kPermissionDenied, error.type());
+ }
+ {
+ Error error;
+ string expected_string = "what";
+ WriteOnlyPropertyAccessor<string> accessor(&expected_string);
+ accessor.Set(expected_string, &error);
+ ASSERT_TRUE(error.IsSuccess());
+ EXPECT_EQ(expected_string, *accessor.property_);
+ EXPECT_EQ(string(), accessor.Get(&error));
+ ASSERT_FALSE(error.IsSuccess());
+
+ expected_string = "nooooo";
+ EXPECT_EQ("nooooo", *accessor.property_);
}
}
diff --git a/property_iterator.h b/property_iterator.h
index 8851b20..c1c47de 100644
--- a/property_iterator.h
+++ b/property_iterator.h
@@ -9,28 +9,37 @@
#include <string>
#include "shill/accessor_interface.h"
+#include "shill/error.h"
+
+class Error;
namespace shill {
// An iterator wrapper class to hide the details of what kind of data structure
// we're using to store key/value pairs for properties.
// Intended for use with PropertyStore.
+
+// TODO(gauravsh): Consider getting rid of PropertyConstIterator, and just
+// keeping ReadablePropertyConstIterator since it doesn't look like the caller
+// is ever interested in anything but readable properties.
template <class V>
class PropertyConstIterator {
public:
virtual ~PropertyConstIterator() {}
- void Advance() { ++it_; }
+ virtual void Advance() {
+ if (!AtEnd())
+ ++it_;
+ }
bool AtEnd() { return it_ == collection_.end(); }
const std::string &Key() const { return it_->first; }
- const V &Value() const { return it_->second->Get(); }
+ V Value(Error *error) const { return it_->second->Get(error); }
- private:
- friend class PropertyStore;
+ protected:
typedef std::tr1::shared_ptr<AccessorInterface<V> > VAccessorPtr;
explicit PropertyConstIterator(
@@ -39,11 +48,50 @@
it_(collection_.begin()) {
}
+ private:
+ friend class PropertyStore;
+
const typename std::map<std::string, VAccessorPtr> &collection_;
typename std::map<std::string, VAccessorPtr>::const_iterator it_;
};
+// A version of the iterator that always advances to the next readable
+// property.
+template <class V>
+class ReadablePropertyConstIterator : public PropertyConstIterator<V> {
+ public:
+ virtual ~ReadablePropertyConstIterator() {}
+ virtual void Advance() {
+ Error error;
+ while (!PropertyConstIterator<V>::AtEnd()) {
+ PropertyConstIterator<V>::Advance();
+ if (PropertyConstIterator<V>::AtEnd())
+ return;
+ error.Reset();
+ PropertyConstIterator<V>::Value(&error);
+ if (error.IsSuccess())
+ break;
+ }
+ }
+
+ private:
+ friend class PropertyStore;
+
+ typedef std::tr1::shared_ptr<AccessorInterface<V> > VAccessorPtr;
+
+ explicit ReadablePropertyConstIterator(
+ const typename std::map<std::string, VAccessorPtr> &collection)
+ : PropertyConstIterator<V>(collection) {
+ if (PropertyConstIterator<V>::AtEnd())
+ return;
+ Error error;
+ PropertyConstIterator<V>::Value(&error);
+ // Start at the next readable property (if any).
+ if (!error.IsSuccess())
+ Advance();
+ }
+};
} // namespace shill
diff --git a/property_store.cc b/property_store.cc
index 8cc67b1..72c3ea8 100644
--- a/property_store.cc
+++ b/property_store.cc
@@ -95,47 +95,60 @@
return SetProperty(name, value, error, uint32_properties_, "a uint32");
}
-PropertyConstIterator<bool> PropertyStore::GetBoolPropertiesIter() const {
- return PropertyConstIterator<bool>(bool_properties_);
-}
-
-PropertyConstIterator<int16> PropertyStore::GetInt16PropertiesIter() const {
- return PropertyConstIterator<int16>(int16_properties_);
-}
-
-PropertyConstIterator<int32> PropertyStore::GetInt32PropertiesIter() const {
- return PropertyConstIterator<int32>(int32_properties_);
-}
-
-PropertyConstIterator<std::string> PropertyStore::GetStringPropertiesIter()
+ReadablePropertyConstIterator<bool> PropertyStore::GetBoolPropertiesIter()
const {
- return PropertyConstIterator<std::string>(string_properties_);
+ return ReadablePropertyConstIterator<bool>(bool_properties_);
}
-PropertyConstIterator<Stringmap> PropertyStore::GetStringmapPropertiesIter()
+ReadablePropertyConstIterator<int16> PropertyStore::GetInt16PropertiesIter()
const {
- return PropertyConstIterator<Stringmap>(stringmap_properties_);
+ return ReadablePropertyConstIterator<int16>(int16_properties_);
}
-PropertyConstIterator<Strings> PropertyStore::GetStringsPropertiesIter() const {
- return PropertyConstIterator<Strings>(strings_properties_);
-}
-
-PropertyConstIterator<StrIntPair> PropertyStore::GetStrIntPairPropertiesIter()
+ReadablePropertyConstIterator<int32> PropertyStore::GetInt32PropertiesIter()
const {
- return PropertyConstIterator<StrIntPair>(strintpair_properties_);
+ return ReadablePropertyConstIterator<int32>(int32_properties_);
}
-PropertyConstIterator<uint8> PropertyStore::GetUint8PropertiesIter() const {
- return PropertyConstIterator<uint8>(uint8_properties_);
+ReadablePropertyConstIterator<std::string>
+PropertyStore::GetStringPropertiesIter() const {
+ return ReadablePropertyConstIterator<std::string>(string_properties_);
}
-PropertyConstIterator<uint16> PropertyStore::GetUint16PropertiesIter() const {
- return PropertyConstIterator<uint16>(uint16_properties_);
+ReadablePropertyConstIterator<Stringmap>
+PropertyStore::GetStringmapPropertiesIter() const {
+ return ReadablePropertyConstIterator<Stringmap>(stringmap_properties_);
}
-PropertyConstIterator<uint32> PropertyStore::GetUint32PropertiesIter() const {
- return PropertyConstIterator<uint32>(uint32_properties_);
+ReadablePropertyConstIterator<Stringmaps>
+PropertyStore::GetStringmapsPropertiesIter()
+ const {
+ return ReadablePropertyConstIterator<Stringmaps>(stringmaps_properties_);
+}
+
+ReadablePropertyConstIterator<Strings> PropertyStore::GetStringsPropertiesIter()
+ const {
+ return ReadablePropertyConstIterator<Strings>(strings_properties_);
+}
+
+ReadablePropertyConstIterator<StrIntPair>
+PropertyStore::GetStrIntPairPropertiesIter() const {
+ return ReadablePropertyConstIterator<StrIntPair>(strintpair_properties_);
+}
+
+ReadablePropertyConstIterator<uint8> PropertyStore::GetUint8PropertiesIter()
+ const {
+ return ReadablePropertyConstIterator<uint8>(uint8_properties_);
+}
+
+ReadablePropertyConstIterator<uint16> PropertyStore::GetUint16PropertiesIter()
+ const {
+ return ReadablePropertyConstIterator<uint16>(uint16_properties_);
+}
+
+ReadablePropertyConstIterator<uint32> PropertyStore::GetUint32PropertiesIter()
+ const {
+ return ReadablePropertyConstIterator<uint32>(uint32_properties_);
}
void PropertyStore::RegisterBool(const string &name, bool *prop) {
@@ -146,6 +159,11 @@
bool_properties_[name] = BoolAccessor(new ConstPropertyAccessor<bool>(prop));
}
+void PropertyStore::RegisterWriteOnlyBool(const string &name, bool *prop) {
+ bool_properties_[name] = BoolAccessor(
+ new WriteOnlyPropertyAccessor<bool>(prop));
+}
+
void PropertyStore::RegisterInt16(const string &name, int16 *prop) {
int16_properties_[name] = Int16Accessor(new PropertyAccessor<int16>(prop));
}
@@ -155,6 +173,10 @@
Int16Accessor(new ConstPropertyAccessor<int16>(prop));
}
+void PropertyStore::RegisterWriteOnlyInt16(const string &name, int16 *prop) {
+ int16_properties_[name] =
+ Int16Accessor(new WriteOnlyPropertyAccessor<int16>(prop));
+}
void PropertyStore::RegisterInt32(const string &name, int32 *prop) {
int32_properties_[name] = Int32Accessor(new PropertyAccessor<int32>(prop));
}
@@ -164,6 +186,11 @@
Int32Accessor(new ConstPropertyAccessor<int32>(prop));
}
+void PropertyStore::RegisterWriteOnlyInt32(const string &name, int32 *prop) {
+ int32_properties_[name] =
+ Int32Accessor(new WriteOnlyPropertyAccessor<int32>(prop));
+}
+
void PropertyStore::RegisterString(const string &name, string *prop) {
string_properties_[name] = StringAccessor(new PropertyAccessor<string>(prop));
}
@@ -174,6 +201,11 @@
StringAccessor(new ConstPropertyAccessor<string>(prop));
}
+void PropertyStore::RegisterWriteOnlyString(const string &name, string *prop) {
+ string_properties_[name] =
+ StringAccessor(new WriteOnlyPropertyAccessor<string>(prop));
+}
+
void PropertyStore::RegisterStringmap(const string &name, Stringmap *prop) {
stringmap_properties_[name] =
StringmapAccessor(new PropertyAccessor<Stringmap>(prop));
@@ -185,6 +217,12 @@
StringmapAccessor(new ConstPropertyAccessor<Stringmap>(prop));
}
+void PropertyStore::RegisterWriteOnlyStringmap(const string &name,
+ Stringmap *prop) {
+ stringmap_properties_[name] =
+ StringmapAccessor(new WriteOnlyPropertyAccessor<Stringmap>(prop));
+}
+
void PropertyStore::RegisterStringmaps(const string &name, Stringmaps *prop) {
stringmaps_properties_[name] =
StringmapsAccessor(new PropertyAccessor<Stringmaps>(prop));
@@ -196,6 +234,12 @@
StringmapsAccessor(new ConstPropertyAccessor<Stringmaps>(prop));
}
+void PropertyStore::RegisterWriteOnlyStringmaps(const string &name,
+ Stringmaps *prop) {
+ stringmaps_properties_[name] =
+ StringmapsAccessor(new WriteOnlyPropertyAccessor<Stringmaps>(prop));
+}
+
void PropertyStore::RegisterStrings(const string &name, Strings *prop) {
strings_properties_[name] =
StringsAccessor(new PropertyAccessor<Strings>(prop));
@@ -207,6 +251,12 @@
StringsAccessor(new ConstPropertyAccessor<Strings>(prop));
}
+void PropertyStore::RegisterWriteOnlyStrings(const string &name,
+ Strings *prop) {
+ strings_properties_[name] =
+ StringsAccessor(new WriteOnlyPropertyAccessor<Strings>(prop));
+}
+
void PropertyStore::RegisterUint8(const string &name, uint8 *prop) {
uint8_properties_[name] = Uint8Accessor(new PropertyAccessor<uint8>(prop));
}
@@ -216,6 +266,11 @@
Uint8Accessor(new ConstPropertyAccessor<uint8>(prop));
}
+void PropertyStore::RegisterWriteOnlyUint8(const string &name, uint8 *prop) {
+ uint8_properties_[name] =
+ Uint8Accessor(new WriteOnlyPropertyAccessor<uint8>(prop));
+}
+
void PropertyStore::RegisterUint16(const std::string &name, uint16 *prop) {
uint16_properties_[name] = Uint16Accessor(new PropertyAccessor<uint16>(prop));
}
@@ -226,6 +281,11 @@
Uint16Accessor(new ConstPropertyAccessor<uint16>(prop));
}
+void PropertyStore::RegisterWriteOnlyUint16(const string &name, uint16 *prop) {
+ uint16_properties_[name] =
+ Uint16Accessor(new WriteOnlyPropertyAccessor<uint16>(prop));
+}
+
void PropertyStore::RegisterDerivedBool(const std::string &name,
const BoolAccessor &accessor) {
bool_properties_[name] = accessor;
diff --git a/property_store.h b/property_store.h
index 696ea17..caa1491 100644
--- a/property_store.h
+++ b/property_store.h
@@ -75,36 +75,45 @@
// Accessors for iterators over property maps. Useful for dumping all
// properties.
- PropertyConstIterator<bool> GetBoolPropertiesIter() const;
- PropertyConstIterator<int16> GetInt16PropertiesIter() const;
- PropertyConstIterator<int32> GetInt32PropertiesIter() const;
- PropertyConstIterator<std::string> GetStringPropertiesIter() const;
- PropertyConstIterator<Stringmap> GetStringmapPropertiesIter() const;
- PropertyConstIterator<Stringmaps> GetStringmapsPropertiesIter() const;
- PropertyConstIterator<StrIntPair> GetStrIntPairPropertiesIter() const;
- PropertyConstIterator<Strings> GetStringsPropertiesIter() const;
- PropertyConstIterator<uint8> GetUint8PropertiesIter() const;
- PropertyConstIterator<uint16> GetUint16PropertiesIter() const;
- PropertyConstIterator<uint32> GetUint32PropertiesIter() const;
+ ReadablePropertyConstIterator<bool> GetBoolPropertiesIter() const;
+ ReadablePropertyConstIterator<int16> GetInt16PropertiesIter() const;
+ ReadablePropertyConstIterator<int32> GetInt32PropertiesIter() const;
+ ReadablePropertyConstIterator<std::string> GetStringPropertiesIter() const;
+ ReadablePropertyConstIterator<Stringmap> GetStringmapPropertiesIter() const;
+ ReadablePropertyConstIterator<Stringmaps> GetStringmapsPropertiesIter() const;
+ ReadablePropertyConstIterator<StrIntPair> GetStrIntPairPropertiesIter() const;
+ ReadablePropertyConstIterator<Strings> GetStringsPropertiesIter() const;
+ ReadablePropertyConstIterator<uint8> GetUint8PropertiesIter() const;
+ ReadablePropertyConstIterator<uint16> GetUint16PropertiesIter() const;
+ ReadablePropertyConstIterator<uint32> GetUint32PropertiesIter() const;
void RegisterBool(const std::string &name, bool *prop);
void RegisterConstBool(const std::string &name, const bool *prop);
+ void RegisterWriteOnlyBool(const std::string &name, bool *prop);
void RegisterInt16(const std::string &name, int16 *prop);
void RegisterConstInt16(const std::string &name, const int16 *prop);
+ void RegisterWriteOnlyInt16(const std::string &name, int16 *prop);
void RegisterInt32(const std::string &name, int32 *prop);
void RegisterConstInt32(const std::string &name, const int32 *prop);
+ void RegisterWriteOnlyInt32(const std::string &name, int32 *prop);
void RegisterString(const std::string &name, std::string *prop);
void RegisterConstString(const std::string &name, const std::string *prop);
+ void RegisterWriteOnlyString(const std::string &name, std::string *prop);
void RegisterStringmap(const std::string &name, Stringmap *prop);
void RegisterConstStringmap(const std::string &name, const Stringmap *prop);
+ void RegisterWriteOnlyStringmap(const std::string &name, Stringmap *prop);
void RegisterStringmaps(const std::string &name, Stringmaps *prop);
void RegisterConstStringmaps(const std::string &name, const Stringmaps *prop);
+ void RegisterWriteOnlyStringmaps(const std::string &name, Stringmaps *prop);
void RegisterStrings(const std::string &name, Strings *prop);
void RegisterConstStrings(const std::string &name, const Strings *prop);
+ void RegisterWriteOnlyStrings(const std::string &name, Strings *prop);
void RegisterUint8(const std::string &name, uint8 *prop);
void RegisterConstUint8(const std::string &name, const uint8 *prop);
+ void RegisterWriteOnlyUint8(const std::string &name, uint8 *prop);
void RegisterUint16(const std::string &name, uint16 *prop);
void RegisterConstUint16(const std::string &name, const uint16 *prop);
+ void RegisterWriteOnlyUint16(const std::string &name, uint16 *prop);
void RegisterDerivedBool(const std::string &name,
const BoolAccessor &accessor);
diff --git a/property_store_unittest.cc b/property_store_unittest.cc
index c35b2c3..8c15039 100644
--- a/property_store_unittest.cc
+++ b/property_store_unittest.cc
@@ -111,4 +111,162 @@
EXPECT_EQ(internal_error(), error.name());
}
+TEST_F(PropertyStoreTest, WriteOnlyProperties) {
+ // Test that properties registered as write-only are not returned
+ // when using Get*PropertiesIter().
+ PropertyStore store;
+ Error error;
+ {
+ const string keys[] = {"boolp1", "boolp2"};
+ bool values[] = {true, true};
+ store.RegisterWriteOnlyBool(keys[0], &values[0]);
+ store.RegisterBool(keys[1], &values[1]);
+
+ ReadablePropertyConstIterator<bool> it = store.GetBoolPropertiesIter();
+ error.Reset();
+ EXPECT_FALSE(it.AtEnd());
+ EXPECT_EQ(keys[1], it.Key());
+ EXPECT_TRUE(values[1] == it.Value(&error));
+ EXPECT_TRUE(error.IsSuccess());
+ it.Advance();
+ EXPECT_TRUE(it.AtEnd());
+ }
+ {
+ const string keys[] = {"int16p1", "int16p2"};
+ int16 values[] = {127, 128};
+ store.RegisterWriteOnlyInt16(keys[0], &values[0]);
+ store.RegisterInt16(keys[1], &values[1]);
+
+ ReadablePropertyConstIterator<int16> it = store.GetInt16PropertiesIter();
+ error.Reset();
+ EXPECT_FALSE(it.AtEnd());
+ EXPECT_EQ(keys[1], it.Key());
+ EXPECT_EQ(values[1], it.Value(&error));
+ EXPECT_TRUE(error.IsSuccess());
+ it.Advance();
+ EXPECT_TRUE(it.AtEnd());
+ }
+ {
+ const string keys[] = {"int32p1", "int32p2"};
+ int32 values[] = {127, 128};
+ store.RegisterWriteOnlyInt32(keys[0], &values[0]);
+ store.RegisterInt32(keys[1], &values[1]);
+
+ ReadablePropertyConstIterator<int32> it = store.GetInt32PropertiesIter();
+ error.Reset();
+ EXPECT_FALSE(it.AtEnd());
+ EXPECT_EQ(keys[1], it.Key());
+ EXPECT_EQ(values[1], it.Value(&error));
+ EXPECT_TRUE(error.IsSuccess());
+ it.Advance();
+ EXPECT_TRUE(it.AtEnd());
+ }
+ {
+ const string keys[] = {"stringp1", "stringp2"};
+ string values[] = {"noooo", "yesss"};
+ store.RegisterWriteOnlyString(keys[0], &values[0]);
+ store.RegisterString(keys[1], &values[1]);
+
+ ReadablePropertyConstIterator<string> it = store.GetStringPropertiesIter();
+ error.Reset();
+ EXPECT_FALSE(it.AtEnd());
+ EXPECT_EQ(keys[1], it.Key());
+ EXPECT_EQ(values[1], it.Value(&error));
+ EXPECT_TRUE(error.IsSuccess());
+ it.Advance();
+ EXPECT_TRUE(it.AtEnd());
+ }
+ {
+ const string keys[] = {"stringmapp1", "stringmapp2"};
+ Stringmap values[2];
+ values[0]["noooo"] = "yesss";
+ values[1]["yesss"] = "noooo";
+ store.RegisterWriteOnlyStringmap(keys[0], &values[0]);
+ store.RegisterStringmap(keys[1], &values[1]);
+
+ ReadablePropertyConstIterator<Stringmap> it =
+ store.GetStringmapPropertiesIter();
+ error.Reset();
+ EXPECT_FALSE(it.AtEnd());
+ EXPECT_EQ(keys[1], it.Key());
+ EXPECT_TRUE(values[1] == it.Value(&error));
+ EXPECT_TRUE(error.IsSuccess());
+ it.Advance();
+ EXPECT_TRUE(it.AtEnd());
+ }
+ {
+ const string keys[] = {"stringmapsp1", "stringmapsp2"};
+ Stringmaps values[2];
+ Stringmap element;
+ element["noooo"] = "yesss";
+ values[0].push_back(element);
+ element["yesss"] = "noooo";
+ values[1].push_back(element);
+
+ store.RegisterWriteOnlyStringmaps(keys[0], &values[0]);
+ store.RegisterStringmaps(keys[1], &values[1]);
+
+ ReadablePropertyConstIterator<Stringmaps> it =
+ store.GetStringmapsPropertiesIter();
+ error.Reset();
+ EXPECT_FALSE(it.AtEnd());
+ EXPECT_EQ(keys[1], it.Key());
+ EXPECT_TRUE(values[1] == it.Value(&error));
+ EXPECT_TRUE(error.IsSuccess());
+ it.Advance();
+ EXPECT_TRUE(it.AtEnd());
+ }
+ {
+ const string keys[] = {"stringsp1", "stringsp2"};
+ Strings values[2];
+ string element;
+ element = "noooo";
+ values[0].push_back(element);
+ element = "yesss";
+ values[1].push_back(element);
+ store.RegisterWriteOnlyStrings(keys[0], &values[0]);
+ store.RegisterStrings(keys[1], &values[1]);
+
+ ReadablePropertyConstIterator<Strings> it =
+ store.GetStringsPropertiesIter();
+ error.Reset();
+ EXPECT_FALSE(it.AtEnd());
+ EXPECT_EQ(keys[1], it.Key());
+ EXPECT_TRUE(values[1] == it.Value(&error));
+ EXPECT_TRUE(error.IsSuccess());
+ it.Advance();
+ EXPECT_TRUE(it.AtEnd());
+ }
+ {
+ const string keys[] = {"uint8p1", "uint8p2"};
+ uint8 values[] = {127, 128};
+ store.RegisterWriteOnlyUint8(keys[0], &values[0]);
+ store.RegisterUint8(keys[1], &values[1]);
+
+ ReadablePropertyConstIterator<uint8> it = store.GetUint8PropertiesIter();
+ error.Reset();
+ EXPECT_FALSE(it.AtEnd());
+ EXPECT_EQ(keys[1], it.Key());
+ EXPECT_EQ(values[1], it.Value(&error));
+ EXPECT_TRUE(error.IsSuccess());
+ it.Advance();
+ EXPECT_TRUE(it.AtEnd());
+ }
+ {
+ const string keys[] = {"uint16p", "uint16p1"};
+ uint16 values[] = {127, 128};
+ store.RegisterWriteOnlyUint16(keys[0], &values[0]);
+ store.RegisterUint16(keys[1], &values[1]);
+
+ ReadablePropertyConstIterator<uint16> it = store.GetUint16PropertiesIter();
+ error.Reset();
+ EXPECT_FALSE(it.AtEnd());
+ EXPECT_EQ(keys[1], it.Key());
+ EXPECT_EQ(values[1], it.Value(&error));
+ EXPECT_TRUE(error.IsSuccess());
+ it.Advance();
+ EXPECT_TRUE(it.AtEnd());
+ }
+}
+
} // namespace shill
diff --git a/property_store_unittest.h b/property_store_unittest.h
index 8b800b7..9679aab 100644
--- a/property_store_unittest.h
+++ b/property_store_unittest.h
@@ -31,7 +31,7 @@
// can be ordering issues if your constructors have side effects.
// These constructors don't, and declaring these as static lets me
// autogenerate a bunch of unit test code that I would otherwise need to
- // copypasta. So I think it's safe and worth it.
+ // copypaste. So I think it's safe and worth it.
static const ::DBus::Variant kBoolV;
static const ::DBus::Variant kByteV;
static const ::DBus::Variant kInt16V;
diff --git a/service.cc b/service.cc
index ccba272..97cb2f4 100644
--- a/service.cc
+++ b/service.cc
@@ -171,7 +171,7 @@
return false;
}
-bool Service::IsActive() {
+bool Service::IsActive(Error */*error*/) {
return state_ != kStateUnknown &&
state_ != kStateIdle &&
state_ != kStateFailure;
@@ -187,7 +187,8 @@
failure_ = kFailureUnknown;
}
manager_->UpdateService(this);
- adaptor_->EmitStringChanged(flimflam::kStateProperty, CalculateState());
+ Error error;
+ adaptor_->EmitStringChanged(flimflam::kStateProperty, CalculateState(&error));
}
void Service::SetFailure(ConnectFailure failure) {
@@ -330,7 +331,7 @@
void Service::set_profile(const ProfileRefPtr &p) { profile_ = p; }
-string Service::CalculateState() {
+string Service::CalculateState(Error */*error*/) {
switch (state_) {
case kStateConnected:
return flimflam::kStateReady;
@@ -342,7 +343,7 @@
void Service::HelpRegisterDerivedBool(
const string &name,
- bool(Service::*get)(void),
+ bool(Service::*get)(Error *),
void(Service::*set)(const bool&, Error *)) {
store_.RegisterDerivedBool(
name,
@@ -351,7 +352,7 @@
void Service::HelpRegisterDerivedString(
const string &name,
- string(Service::*get)(void),
+ string(Service::*get)(Error *),
void(Service::*set)(const string&, Error *)) {
store_.RegisterDerivedString(
name,
diff --git a/service.h b/service.h
index 2cb6411..bbc37ad 100644
--- a/service.h
+++ b/service.h
@@ -101,7 +101,7 @@
// Base method always returns false.
virtual bool TechnologyIs(const Technology::Identifier type) const;
- virtual bool IsActive();
+ virtual bool IsActive(Error *error);
virtual ConnectState state() const { return state_; }
// Updates the state of the Service and alerts the manager. Also
@@ -177,15 +177,15 @@
const std::string &friendly_name() const { return friendly_name_; }
void set_friendly_name(const std::string &n) { friendly_name_ = n; }
- virtual std::string CalculateState();
+ virtual std::string CalculateState(Error *error);
void HelpRegisterDerivedBool(
const std::string &name,
- bool(Service::*get)(void),
+ bool(Service::*get)(Error *),
void(Service::*set)(const bool&, Error *));
void HelpRegisterDerivedString(
const std::string &name,
- std::string(Service::*get)(void),
+ std::string(Service::*get)(Error *),
void(Service::*set)(const std::string&, Error *));
// Assigns |value| to |key| in |storage| if |value| is non-empty and |save| is
@@ -239,9 +239,9 @@
static const char kStorageProxyConfig[];
static const char kStorageSaveCredentials[];
- virtual std::string GetDeviceRpcId() = 0;
+ virtual std::string GetDeviceRpcId(Error *error) = 0;
- std::string GetProfileRpcId() {
+ std::string GetProfileRpcId(Error */*error*/) {
return ""; // Will need to call Profile to get this.
}
diff --git a/service_under_test.cc b/service_under_test.cc
index d747df0..dd915da 100644
--- a/service_under_test.cc
+++ b/service_under_test.cc
@@ -29,13 +29,13 @@
void ServiceUnderTest::Disconnect() {}
-string ServiceUnderTest::CalculateState() { return ""; }
+string ServiceUnderTest::CalculateState(Error */*error*/) { return ""; }
string ServiceUnderTest::GetRpcIdentifier() const {
return ServiceMockAdaptor::kRpcId;
}
-string ServiceUnderTest::GetDeviceRpcId() { return kRpcId; }
+string ServiceUnderTest::GetDeviceRpcId(Error */*error*/) { return kRpcId; }
string ServiceUnderTest::GetStorageIdentifier() const { return kStorageId; }
diff --git a/service_under_test.h b/service_under_test.h
index 015305a..451e974 100644
--- a/service_under_test.h
+++ b/service_under_test.h
@@ -26,9 +26,9 @@
virtual void Connect(Error */*error*/);
virtual void Disconnect();
- virtual std::string CalculateState();
+ virtual std::string CalculateState(Error */*error*/);
virtual std::string GetRpcIdentifier() const;
- virtual std::string GetDeviceRpcId();
+ virtual std::string GetDeviceRpcId(Error */*error*/);
virtual std::string GetStorageIdentifier() const;
private:
diff --git a/wifi_service.cc b/wifi_service.cc
index 9f61866..3dd3362 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -237,7 +237,7 @@
wifi_->ConnectTo(this, params);
}
-string WiFiService::GetDeviceRpcId() {
+string WiFiService::GetDeviceRpcId(Error */*error*/) {
return wifi_->GetRpcIdentifier();
}
diff --git a/wifi_service.h b/wifi_service.h
index 7e316ae..5f7b1e9 100644
--- a/wifi_service.h
+++ b/wifi_service.h
@@ -67,7 +67,7 @@
void ConnectTask();
- std::string GetDeviceRpcId();
+ std::string GetDeviceRpcId(Error *error);
static std::string ParseWEPPassphrase(const std::string &passphrase,
Error *error);