shill: PropertyAccessor: Create a const ReadOnly accessor
Provide a means to create an Accessor child class which can be
constructed with a const method. This is useful for read-only
properties which won't modify the called object's state. Make
use of this new feature by transforming the HelpRegisterConst*
in the Service object to take const methods and to register
read only accessors. The bulk of this CL is the change to all
the subclasses to constify methods which were only non-const
due to this issue.
BUG=chromium:325603
TEST=Unit test
Change-Id: I79c6211e9e0907869c2885937dff58c1faf2ca4a
Reviewed-on: https://chromium-review.googlesource.com/178698
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/cellular_service.cc b/cellular_service.cc
index 9e5bf4a..c76ccd7 100644
--- a/cellular_service.cc
+++ b/cellular_service.cc
@@ -399,7 +399,7 @@
return storage_identifier_;
}
-string CellularService::GetDeviceRpcId(Error */*error*/) {
+string CellularService::GetDeviceRpcId(Error */*error*/) const {
return cellular_->GetRpcIdentifier();
}
diff --git a/cellular_service.h b/cellular_service.h
index 9bdbf80..cf02bac 100644
--- a/cellular_service.h
+++ b/cellular_service.h
@@ -180,7 +180,7 @@
bool(CellularService::*get)(Error *),
bool(CellularService::*set)(const bool&, Error *));
- virtual std::string GetDeviceRpcId(Error *error);
+ virtual std::string GetDeviceRpcId(Error *error) const;
Stringmap GetApn(Error *error);
bool SetApn(const Stringmap &value, Error *error);
diff --git a/ethernet_eap_service.cc b/ethernet_eap_service.cc
index 59e410f..08f418b 100644
--- a/ethernet_eap_service.cc
+++ b/ethernet_eap_service.cc
@@ -34,7 +34,7 @@
Technology::NameFromIdentifier(technology()).c_str());
}
-string EthernetEapService::GetDeviceRpcId(Error */*error*/) {
+string EthernetEapService::GetDeviceRpcId(Error */*error*/) const {
return "/";
}
diff --git a/ethernet_eap_service.h b/ethernet_eap_service.h
index 8148df7..7e2d7f0 100644
--- a/ethernet_eap_service.h
+++ b/ethernet_eap_service.h
@@ -25,7 +25,7 @@
virtual ~EthernetEapService();
// Inherited from Service.
- virtual std::string GetDeviceRpcId(Error *error);
+ virtual std::string GetDeviceRpcId(Error *error) const;
virtual std::string GetStorageIdentifier() const;
virtual bool Is8021x() const { return true; }
virtual bool IsVisible() const { return false; }
diff --git a/ethernet_service.cc b/ethernet_service.cc
index 8fab092..7ff6b47 100644
--- a/ethernet_service.cc
+++ b/ethernet_service.cc
@@ -55,7 +55,7 @@
ethernet_->DisconnectFrom(this);
}
-std::string EthernetService::GetDeviceRpcId(Error */*error*/) {
+std::string EthernetService::GetDeviceRpcId(Error */*error*/) const {
return ethernet_->GetRpcIdentifier();
}
diff --git a/ethernet_service.h b/ethernet_service.h
index d7c0b63..7c0c117 100644
--- a/ethernet_service.h
+++ b/ethernet_service.h
@@ -41,7 +41,7 @@
private:
static const char kServiceType[];
- std::string GetDeviceRpcId(Error *error);
+ std::string GetDeviceRpcId(Error *error) const;
EthernetRefPtr ethernet_;
DISALLOW_COPY_AND_ASSIGN(EthernetService);
diff --git a/mock_ethernet_service.h b/mock_ethernet_service.h
index a5039bf..1ef0de9 100644
--- a/mock_ethernet_service.h
+++ b/mock_ethernet_service.h
@@ -22,7 +22,7 @@
MOCK_METHOD2(Configure, void(const KeyValueStore &args, Error *error));
MOCK_METHOD2(DisconnectWithFailure,
void(ConnectFailure failure, Error *error));
- MOCK_METHOD1(GetDeviceRpcId, std::string(Error *error));
+ MOCK_CONST_METHOD1(GetDeviceRpcId, std::string(Error *error));
MOCK_CONST_METHOD0(GetStorageIdentifier, std::string());
MOCK_CONST_METHOD0(Is8021xConnectable, bool());
MOCK_CONST_METHOD0(IsConnected, bool());
diff --git a/mock_service.h b/mock_service.h
index 330d278..7dd77f6 100644
--- a/mock_service.h
+++ b/mock_service.h
@@ -42,7 +42,7 @@
MOCK_CONST_METHOD0(IsVisible, bool());
MOCK_METHOD1(SetFailure, void(ConnectFailure failure));
MOCK_CONST_METHOD0(failure, ConnectFailure());
- MOCK_METHOD1(GetDeviceRpcId, std::string(Error *error));
+ MOCK_CONST_METHOD1(GetDeviceRpcId, std::string(Error *error));
MOCK_CONST_METHOD0(GetRpcIdentifier, std::string());
MOCK_CONST_METHOD0(GetStorageIdentifier, std::string());
MOCK_CONST_METHOD1(GetLoadableStorageIdentifier,
diff --git a/property_accessor.h b/property_accessor.h
index a12ef56..99df286 100644
--- a/property_accessor.h
+++ b/property_accessor.h
@@ -249,6 +249,38 @@
DISALLOW_COPY_AND_ASSIGN(CustomWriteOnlyAccessor);
};
+// CustomReadOnlyAccessor<> allows a custom getter method to be provided.
+// Set and Clear return errors automatically.
+template<class C, class T>
+class CustomReadOnlyAccessor : public AccessorInterface<T> {
+ public:
+ // |target| is the object on which to call the |getter| method.
+ // |getter| is a const method. If a non-const method needs to be used,
+ // use the CustomAccessor with a NULL setter instead.
+ CustomReadOnlyAccessor(C *target, T(C::*getter)(Error *error) const)
+ : target_(target), getter_(getter) {
+ DCHECK(target);
+ DCHECK(getter);
+ }
+ virtual ~CustomReadOnlyAccessor() {}
+
+ void Clear(Error *error) {
+ error->Populate(Error::kInvalidArguments, "Property is read-only");
+ }
+ T Get(Error *error) {
+ return (target_->*getter_)(error);
+ }
+ bool Set(const T &value, Error *error) {
+ error->Populate(Error::kInvalidArguments, "Property is read-only");
+ return false;
+ }
+
+ private:
+ C *const target_;
+ T(C::*const getter_)(Error *error) const;
+ DISALLOW_COPY_AND_ASSIGN(CustomReadOnlyAccessor);
+};
+
// CustomMappedAccessor<> passes an argument to the getter and setter
// so that a generic method can be used, for example one that accesses the
// property in a map.
diff --git a/property_accessor_unittest.cc b/property_accessor_unittest.cc
index fe618d9..ea335d2 100644
--- a/property_accessor_unittest.cc
+++ b/property_accessor_unittest.cc
@@ -271,6 +271,9 @@
string Get(Error */*error*/) {
return value_;
}
+ string ConstGet(Error */*error*/) const {
+ return value_;
+ }
bool Set(const string &value, Error */*error*/) {
if (value_ == value) {
return false;
@@ -437,6 +440,40 @@
}
}
+TEST(PropertyAccessorTest, CustomReadOnlyAccessor) {
+ StringWrapper wrapper;
+ CustomReadOnlyAccessor<StringWrapper, string> accessor(
+ &wrapper, &StringWrapper::ConstGet);
+ const string orig_value = wrapper.value_ = "original value";
+ {
+ // Test reading.
+ Error error;
+ EXPECT_EQ(orig_value, accessor.Get(&error));
+ EXPECT_TRUE(error.IsSuccess());
+ }
+ {
+ // Test writing.
+ Error error;
+ EXPECT_FALSE(accessor.Set("new value", &error));
+ EXPECT_EQ(Error::kInvalidArguments, error.type());
+ EXPECT_EQ(orig_value, accessor.Get(&error));
+ }
+ {
+ // Test writing original value -- this also fails.
+ Error error;
+ EXPECT_FALSE(accessor.Set(orig_value, &error));
+ EXPECT_EQ(Error::kInvalidArguments, error.type());
+ EXPECT_EQ(orig_value, accessor.Get(&error));
+ }
+ {
+ // Test clearing.
+ Error error;
+ accessor.Clear(&error);
+ EXPECT_EQ(Error::kInvalidArguments, error.type());
+ EXPECT_EQ(orig_value, accessor.Get(&error));
+ }
+}
+
class StringMapWrapper {
public:
void Clear(const string &key, Error */*error*/) {
diff --git a/service.cc b/service.cc
index 37797af..488583e 100644
--- a/service.cc
+++ b/service.cc
@@ -1030,7 +1030,7 @@
explicitly_disconnected_ = false;
}
-string Service::GetIPConfigRpcIdentifier(Error *error) {
+string Service::GetIPConfigRpcIdentifier(Error *error) const {
if (!connection_) {
error->Populate(Error::kNotFound);
return DBusAdaptor::kNullPath;
@@ -1165,26 +1165,33 @@
void Service::HelpRegisterConstDerivedRpcIdentifier(
const string &name,
- RpcIdentifier(Service::*get)(Error *)) {
+ RpcIdentifier(Service::*get)(Error *) const) {
store_.RegisterDerivedRpcIdentifier(
name,
- RpcIdentifierAccessor(new CustomAccessor<Service, RpcIdentifier>(
- this, get, NULL)));
+ RpcIdentifierAccessor(new CustomReadOnlyAccessor<Service, RpcIdentifier>(
+ this, get)));
}
void Service::HelpRegisterConstDerivedUint16(
const string &name,
- uint16(Service::*get)(Error *)) {
+ uint16(Service::*get)(Error *) const) {
store_.RegisterDerivedUint16(
name,
- Uint16Accessor(new CustomAccessor<Service, uint16>(this, get, NULL)));
+ Uint16Accessor(new CustomReadOnlyAccessor<Service, uint16>(this, get)));
}
void Service::HelpRegisterConstDerivedStrings(
- const string &name, Strings(Service::*get)(Error *error)) {
+ const string &name, Strings(Service::*get)(Error *error) const) {
store_.RegisterDerivedStrings(
name,
- StringsAccessor(new CustomAccessor<Service, Strings>(this, get, NULL)));
+ StringsAccessor(new CustomReadOnlyAccessor<Service, Strings>(this, get)));
+}
+
+void Service::HelpRegisterConstDerivedString(
+ const string &name, string(Service::*get)(Error *error) const) {
+ store_.RegisterDerivedString(
+ name,
+ StringAccessor(new CustomReadOnlyAccessor<Service, string>(this, get)));
}
// static
@@ -1361,7 +1368,7 @@
return (profile_ != old_profile);
}
-uint16 Service::GetHTTPProxyPort(Error */*error*/) {
+uint16 Service::GetHTTPProxyPort(Error */*error*/) const {
if (http_proxy_.get()) {
return static_cast<uint16>(http_proxy_->proxy_port());
}
@@ -1391,11 +1398,11 @@
return strings;
}
-Strings Service::GetDisconnectsProperty(Error */*error*/) {
+Strings Service::GetDisconnectsProperty(Error */*error*/) const {
return ExtractWallClockToStrings(disconnects_);
}
-Strings Service::GetMisconnectsProperty(Error */*error*/) {
+Strings Service::GetMisconnectsProperty(Error */*error*/) const {
return ExtractWallClockToStrings(misconnects_);
}
diff --git a/service.h b/service.h
index 9b713d8..ad4aea3 100644
--- a/service.h
+++ b/service.h
@@ -502,12 +502,14 @@
bool(Service::*set)(const std::string &value, Error *error));
void HelpRegisterConstDerivedUint16(
const std::string &name,
- uint16(Service::*get)(Error *error));
+ uint16(Service::*get)(Error *error) const);
void HelpRegisterConstDerivedRpcIdentifier(
const std::string &name,
- std::string(Service::*get)(Error *));
+ std::string(Service::*get)(Error *) const);
void HelpRegisterConstDerivedStrings(
- const std::string &name, Strings(Service::*get)(Error *error));
+ const std::string &name, Strings(Service::*get)(Error *error) const);
+ void HelpRegisterConstDerivedString(
+ const std::string &name, std::string(Service::*get)(Error *error) const);
ServiceAdaptorInterface *adaptor() const { return adaptor_.get(); }
@@ -655,9 +657,9 @@
std::string GetGuid(Error *error);
- virtual std::string GetDeviceRpcId(Error *error) = 0;
+ virtual std::string GetDeviceRpcId(Error *error) const = 0;
- std::string GetIPConfigRpcIdentifier(Error *error);
+ std::string GetIPConfigRpcIdentifier(Error *error) const;
std::string GetNameProperty(Error *error);
// The base implementation asserts that |name| matches the current Name
@@ -670,15 +672,15 @@
bool SetProfileRpcId(const std::string &profile, Error *error);
// Returns TCP port of service's HTTP proxy in host order.
- uint16 GetHTTPProxyPort(Error *error);
+ uint16 GetHTTPProxyPort(Error *error) const;
std::string GetProxyConfig(Error *error);
bool SetProxyConfig(const std::string &proxy_config, Error *error);
static Strings ExtractWallClockToStrings(
const std::deque<Timestamp> ×tamps);
- Strings GetDisconnectsProperty(Error *error);
- Strings GetMisconnectsProperty(Error *error);
+ Strings GetDisconnectsProperty(Error *error) const;
+ Strings GetMisconnectsProperty(Error *error) const;
void ReEnableAutoConnectTask();
// Disables autoconnect and posts a task to re-enable it after a cooldown.
diff --git a/service_under_test.cc b/service_under_test.cc
index f945cc6..93c0593 100644
--- a/service_under_test.cc
+++ b/service_under_test.cc
@@ -32,7 +32,9 @@
return ServiceMockAdaptor::kRpcId;
}
-string ServiceUnderTest::GetDeviceRpcId(Error */*error*/) { return kRpcId; }
+string ServiceUnderTest::GetDeviceRpcId(Error */*error*/) const {
+ return kRpcId;
+}
string ServiceUnderTest::GetStorageIdentifier() const { return kStorageId; }
diff --git a/service_under_test.h b/service_under_test.h
index 329676d..98ce17f 100644
--- a/service_under_test.h
+++ b/service_under_test.h
@@ -29,7 +29,7 @@
virtual ~ServiceUnderTest();
virtual std::string GetRpcIdentifier() const;
- virtual std::string GetDeviceRpcId(Error *error);
+ virtual std::string GetDeviceRpcId(Error *error) const;
virtual std::string GetStorageIdentifier() const;
// Getter and setter for a string array property for use in testing.
diff --git a/vpn_service.cc b/vpn_service.cc
index 94a518f..f97d7d1 100644
--- a/vpn_service.cc
+++ b/vpn_service.cc
@@ -96,7 +96,7 @@
return id;
}
-string VPNService::GetDeviceRpcId(Error *error) {
+string VPNService::GetDeviceRpcId(Error *error) const {
error->Populate(Error::kNotSupported);
return "/";
}
diff --git a/vpn_service.h b/vpn_service.h
index 9800ddc..e7a08de 100644
--- a/vpn_service.h
+++ b/vpn_service.h
@@ -58,7 +58,7 @@
static const char kAutoConnNeverConnected[];
static const char kAutoConnVPNAlreadyActive[];
- virtual std::string GetDeviceRpcId(Error *error);
+ virtual std::string GetDeviceRpcId(Error *error) const;
// Returns the Type name of the lowest connection (presumably the "physical"
// connection) that this service depends on.
diff --git a/wifi_service.cc b/wifi_service.cc
index d8102a9..ce0bb49 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -608,7 +608,7 @@
wifi_->DisconnectFrom(this);
}
-string WiFiService::GetDeviceRpcId(Error *error) {
+string WiFiService::GetDeviceRpcId(Error *error) const {
if (!wifi_) {
error->Populate(Error::kNotFound, "Not associated with a device");
return DBusAdaptor::kNullPath;
diff --git a/wifi_service.h b/wifi_service.h
index 1f78693..5c53b96 100644
--- a/wifi_service.h
+++ b/wifi_service.h
@@ -205,7 +205,7 @@
void(WiFiService::*clear)(Error *error),
const std::string *default_value);
- std::string GetDeviceRpcId(Error *error);
+ std::string GetDeviceRpcId(Error *error) const;
void ClearPassphrase(Error *error);
void UpdateConnectable();
diff --git a/wimax_service.cc b/wimax_service.cc
index 968e436..c6b6fb4 100644
--- a/wimax_service.cc
+++ b/wimax_service.cc
@@ -167,7 +167,7 @@
return storage_id_;
}
-string WiMaxService::GetDeviceRpcId(Error *error) {
+string WiMaxService::GetDeviceRpcId(Error *error) const {
if (!device_) {
error->Populate(Error::kNotFound, "Not associated with a device");
return DBusAdaptor::kNullPath;
diff --git a/wimax_service.h b/wimax_service.h
index c796acb..9f0ef46 100644
--- a/wimax_service.h
+++ b/wimax_service.h
@@ -87,7 +87,7 @@
FRIEND_TEST(WiMaxServiceTest, StartStop);
// Inherited from Service.
- virtual std::string GetDeviceRpcId(Error *error);
+ virtual std::string GetDeviceRpcId(Error *error) const;
virtual bool IsAutoConnectable(const char **reason) const;
void OnSignalStrengthChanged(int strength);