shill: Remove PropertyStoreInspector
Remove PropertyStoreInspector, and move its methods internal to
PropertyStore. Service will soon need to inspect individual
properties, so it is time to move the inspection code out of
unit-test only code.
BUG=chromium-os:34525
TEST=Unit tests
Change-Id: I48fb7e459d2fca631a231700ee3050161b1df79b
Reviewed-on: https://gerrit.chromium.org/gerrit/41540
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/Makefile b/Makefile
index 75a2d94..07a15c6 100644
--- a/Makefile
+++ b/Makefile
@@ -448,7 +448,6 @@
profile_dbus_property_exporter_unittest.o \
profile_unittest.o \
property_accessor_unittest.o \
- property_store_inspector.o \
property_store_unittest.o \
resolver_unittest.o \
routing_table_unittest.o \
diff --git a/l2tp_ipsec_driver_unittest.cc b/l2tp_ipsec_driver_unittest.cc
index cf4a297..9c20ce4 100644
--- a/l2tp_ipsec_driver_unittest.cc
+++ b/l2tp_ipsec_driver_unittest.cc
@@ -20,7 +20,6 @@
#include "shill/mock_process_killer.h"
#include "shill/mock_vpn.h"
#include "shill/mock_vpn_service.h"
-#include "shill/property_store_inspector.h"
#include "shill/vpn.h"
using std::find;
@@ -474,12 +473,12 @@
TEST_F(L2TPIPSecDriverTest, GetProvider) {
PropertyStore store;
driver_->InitPropertyStore(&store);
- PropertyStoreInspector inspector(&store);
{
KeyValueStore props;
+ Error error;
EXPECT_TRUE(
- inspector.GetKeyValueStoreProperty(
- flimflam::kProviderProperty, &props));
+ store.GetKeyValueStoreProperty(
+ flimflam::kProviderProperty, &props, &error));
EXPECT_TRUE(props.LookupBool(flimflam::kPassphraseRequiredProperty, false));
EXPECT_TRUE(
props.LookupBool(flimflam::kL2tpIpsecPskRequiredProperty, false));
@@ -488,9 +487,10 @@
KeyValueStore props;
SetArg(flimflam::kL2tpIpsecPasswordProperty, "random-password");
SetArg(flimflam::kL2tpIpsecPskProperty, "random-psk");
+ Error error;
EXPECT_TRUE(
- inspector.GetKeyValueStoreProperty(
- flimflam::kProviderProperty, &props));
+ store.GetKeyValueStoreProperty(
+ flimflam::kProviderProperty, &props, &error));
EXPECT_FALSE(props.LookupBool(flimflam::kPassphraseRequiredProperty, true));
EXPECT_FALSE(
props.LookupBool(flimflam::kL2tpIpsecPskRequiredProperty, true));
diff --git a/openvpn_driver_unittest.cc b/openvpn_driver_unittest.cc
index eff0db8..8e84d5e 100644
--- a/openvpn_driver_unittest.cc
+++ b/openvpn_driver_unittest.cc
@@ -29,7 +29,6 @@
#include "shill/mock_vpn.h"
#include "shill/mock_vpn_service.h"
#include "shill/nice_mock_control.h"
-#include "shill/property_store_inspector.h"
#include "shill/rpc_task.h"
#include "shill/vpn.h"
#include "shill/vpn_service.h"
@@ -915,20 +914,21 @@
TEST_F(OpenVPNDriverTest, GetProvider) {
PropertyStore store;
driver_->InitPropertyStore(&store);
- PropertyStoreInspector inspector(&store);
{
KeyValueStore props;
+ Error error;
EXPECT_TRUE(
- inspector.GetKeyValueStoreProperty(
- flimflam::kProviderProperty, &props));
+ store.GetKeyValueStoreProperty(
+ flimflam::kProviderProperty, &props, &error));
EXPECT_TRUE(props.LookupBool(flimflam::kPassphraseRequiredProperty, false));
}
{
KeyValueStore props;
SetArg(flimflam::kOpenVPNPasswordProperty, "random-password");
+ Error error;
EXPECT_TRUE(
- inspector.GetKeyValueStoreProperty(
- flimflam::kProviderProperty, &props));
+ store.GetKeyValueStoreProperty(
+ flimflam::kProviderProperty, &props, &error));
EXPECT_FALSE(props.LookupBool(flimflam::kPassphraseRequiredProperty, true));
EXPECT_FALSE(props.ContainsString(flimflam::kOpenVPNPasswordProperty));
}
diff --git a/property_store.cc b/property_store.cc
index bb1a87c..6b2e449 100644
--- a/property_store.cc
+++ b/property_store.cc
@@ -42,6 +42,88 @@
ContainsKey(rpc_identifiers_properties_, prop));
}
+bool PropertyStore::GetBoolProperty(const string &name,
+ bool *value,
+ Error *error) const {
+ return GetProperty(name, value, error, bool_properties_, "a bool");
+}
+
+bool PropertyStore::GetInt16Property(const string &name,
+ int16 *value,
+ Error *error) const {
+ return GetProperty(name, value, error, int16_properties_, "an int16");
+}
+
+bool PropertyStore::GetInt32Property(const string &name,
+ int32 *value,
+ Error *error) const {
+ return GetProperty(name, value, error, int32_properties_, "an int32");
+}
+
+bool PropertyStore::GetKeyValueStoreProperty(const string &name,
+ KeyValueStore *value,
+ Error *error) const {
+ return GetProperty(name, value, error, key_value_store_properties_,
+ "a key value store");
+}
+
+bool PropertyStore::GetRpcIdentifierProperty(const string &name,
+ RpcIdentifier *value,
+ Error *error) const {
+ return GetProperty(name, value, error, rpc_identifier_properties_,
+ "an rpc_identifier");
+}
+
+bool PropertyStore::GetStringProperty(const string &name,
+ string *value,
+ Error *error) const {
+ return GetProperty(name, value, error, string_properties_, "a string");
+}
+
+bool PropertyStore::GetStringmapProperty(const string &name,
+ Stringmap *values,
+ Error *error) const {
+ return GetProperty(name, values, error, stringmap_properties_,
+ "a string map");
+}
+
+bool PropertyStore::GetStringmapsProperty(const string &name,
+ Stringmaps *values,
+ Error *error) const {
+ return GetProperty(name, values, error, stringmaps_properties_,
+ "a string map list");
+}
+
+bool PropertyStore::GetStringsProperty(const string &name,
+ Strings *values,
+ Error *error) const {
+ return GetProperty(name, values, error, strings_properties_, "a string list");
+}
+
+bool PropertyStore::GetUint8Property(const string &name,
+ uint8 *value,
+ Error *error) const {
+ return GetProperty(name, value, error, uint8_properties_, "a uint8");
+}
+
+bool PropertyStore::GetUint16Property(const string &name,
+ uint16 *value,
+ Error *error) const {
+ return GetProperty(name, value, error, uint16_properties_, "a uint16");
+}
+
+bool PropertyStore::GetUint32Property(const string &name,
+ uint32 *value,
+ Error *error) const {
+ return GetProperty(name, value, error, uint32_properties_, "a uint32");
+}
+
+bool PropertyStore::GetUint64Property(const string &name,
+ uint64 *value,
+ Error *error) const {
+ return GetProperty(name, value, error, uint64_properties_, "a uint64");
+}
+
bool PropertyStore::SetBoolProperty(const string &name,
bool value,
Error *error) {
@@ -500,6 +582,36 @@
// private methods
template <class V>
+bool PropertyStore::GetProperty(
+ const string &name,
+ V *value,
+ Error *error,
+ const map< string, std::tr1::shared_ptr<
+ AccessorInterface<V> > >&collection,
+ const string &value_type_english) const {
+ SLOG(Property, 2) << "Getting " << name << " as " << value_type_english
+ << ".";
+ typename map< string, std::tr1::shared_ptr<
+ AccessorInterface<V> > >::const_iterator it = collection.find(name);
+ if (it != collection.end()) {
+ V val = it->second->Get(error);
+ if (error->IsSuccess()) {
+ *value = val;
+ }
+ } else {
+ if (Contains(name)) {
+ error->Populate(
+ Error::kInvalidArguments,
+ "Property " + name + " is not " + value_type_english + ".");
+ } else {
+ error->Populate(
+ Error::kInvalidProperty, "Property " + name + " does not exist.");
+ }
+ }
+ return error->IsSuccess();
+};
+
+template <class V>
bool PropertyStore::SetProperty(
const string &name,
const V &value,
diff --git a/property_store.h b/property_store.h
index 829dcc4..56ade72 100644
--- a/property_store.h
+++ b/property_store.h
@@ -25,6 +25,37 @@
virtual bool Contains(const std::string& property) const;
+ // Methods to allow the getting of properties stored in the referenced
+ // |store_| by name. Upon success, these methods return true and return the
+ // property value in |value|. Upon failure, they return false and
+ // leave |value| untouched.
+ bool GetBoolProperty(const std::string &name, bool *value,
+ Error *error) const;
+ bool GetInt16Property(const std::string &name, int16 *value,
+ Error *error) const;
+ bool GetInt32Property(const std::string &name, int32 *value,
+ Error *error) const;
+ bool GetKeyValueStoreProperty(const std::string &name, KeyValueStore *value,
+ Error *error) const;
+ bool GetStringProperty(const std::string &name, std::string *value,
+ Error *error) const;
+ bool GetStringmapProperty(const std::string &name, Stringmap *values,
+ Error *error) const;
+ bool GetStringmapsProperty(const std::string &name, Stringmaps *values,
+ Error *error) const;
+ bool GetStringsProperty(const std::string &name, Strings *values,
+ Error *error) const;
+ bool GetUint8Property(const std::string &name, uint8 *value,
+ Error *error) const;
+ bool GetUint16Property(const std::string &name, uint16 *value,
+ Error *error) const;
+ bool GetUint32Property(const std::string &name, uint32 *value,
+ Error *error) const;
+ bool GetUint64Property(const std::string &name, uint64 *value,
+ Error *error) const;
+ bool GetRpcIdentifierProperty(const std::string &name, RpcIdentifier *value,
+ Error *error) const;
+
// Methods to allow the setting, by name, of properties stored in this object.
// The property names are declared in chromeos/dbus/service_constants.h,
// so that they may be shared with libcros.
@@ -90,11 +121,6 @@
// Otherwrise, |error| is unchanged, and the method returns true.
virtual bool ClearProperty(const std::string &name, Error *error);
- // We do not provide methods for reading individual properties,
- // because we don't need them to implement the flimflam API. (The flimflam
- // API only allows fetching all properties at once -- not individual
- // properties.)
-
// Accessors for iterators over property maps. Useful for dumping all
// properties.
ReadablePropertyConstIterator<bool> GetBoolPropertiesIter() const;
@@ -182,6 +208,15 @@
private:
template <class V>
+ bool GetProperty(
+ const std::string &name,
+ V *value,
+ Error *error,
+ const std::map< std::string, std::tr1::shared_ptr<
+ AccessorInterface<V> > > &collection,
+ const std::string &value_type_english) const;
+
+ template <class V>
bool SetProperty(
const std::string &name,
const V &value,
diff --git a/property_store_inspector.cc b/property_store_inspector.cc
deleted file mode 100644
index d057d99..0000000
--- a/property_store_inspector.cc
+++ /dev/null
@@ -1,93 +0,0 @@
-// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "shill/property_store_inspector.h"
-
-using std::map;
-using std::string;
-using std::vector;
-
-namespace shill {
-
-PropertyStoreInspector::PropertyStoreInspector() : store_(NULL) {}
-
-PropertyStoreInspector::PropertyStoreInspector(const PropertyStore *store)
- : store_(store) {}
-
-PropertyStoreInspector::~PropertyStoreInspector() {}
-
-bool PropertyStoreInspector::GetBoolProperty(const string &name, bool *value) {
- return GetProperty(name, value, &PropertyStore::GetBoolPropertiesIter);
-}
-
-bool PropertyStoreInspector::GetInt16Property(const string &name,
- int16 *value) {
- return GetProperty(name, value, &PropertyStore::GetInt16PropertiesIter);
-}
-
-bool PropertyStoreInspector::GetInt32Property(const string &name,
- int32 *value) {
- return GetProperty(name, value, &PropertyStore::GetInt32PropertiesIter);
-}
-
-bool PropertyStoreInspector::GetKeyValueStoreProperty(const string &name,
- KeyValueStore *value) {
- return GetProperty(name, value,
- &PropertyStore::GetKeyValueStorePropertiesIter);
-}
-
-bool PropertyStoreInspector::GetStringProperty(const string &name,
- string *value) {
- return GetProperty(name, value, &PropertyStore::GetStringPropertiesIter);
-}
-
-bool PropertyStoreInspector::GetStringmapProperty(const string &name,
- map<string, string> *values) {
- return GetProperty(name, values, &PropertyStore::GetStringmapPropertiesIter);
-}
-
-bool PropertyStoreInspector::GetStringsProperty(const string &name,
- vector<string> *values) {
- return GetProperty(name, values, &PropertyStore::GetStringsPropertiesIter);
-}
-
-bool PropertyStoreInspector::GetUint8Property(const string &name,
- uint8 *value) {
- return GetProperty(name, value, &PropertyStore::GetUint8PropertiesIter);
-}
-
-bool PropertyStoreInspector::GetUint16Property(const string &name,
- uint16 *value) {
- return GetProperty(name, value, &PropertyStore::GetUint16PropertiesIter);
-}
-
-bool PropertyStoreInspector::GetUint32Property(const string &name,
- uint32 *value) {
- return GetProperty(name, value, &PropertyStore::GetUint32PropertiesIter);
-}
-
-bool PropertyStoreInspector::GetRpcIdentifierProperty(const string &name,
- RpcIdentifier *value) {
- return GetProperty(name, value,
- &PropertyStore::GetRpcIdentifierPropertiesIter);
-}
-
-template <class V>
-bool PropertyStoreInspector::GetProperty(
- const string &name,
- V *value,
- ReadablePropertyConstIterator<V>(PropertyStore::*getter)() const) {
- for (ReadablePropertyConstIterator<V> it = ((*store_).*getter)();
- !it.AtEnd(); it.Advance()) {
- if (it.Key() == name) {
- if (value) {
- *value = it.value();
- }
- return true;
- }
- }
- return false;
-};
-
-} // namespace shill
diff --git a/property_store_inspector.h b/property_store_inspector.h
deleted file mode 100644
index b29a9dc..0000000
--- a/property_store_inspector.h
+++ /dev/null
@@ -1,66 +0,0 @@
-// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef SHILL_PROPERTY_STORE_INSPECTOR_H_
-#define SHILL_PROPERTY_STORE_INSPECTOR_H_
-
-#ifndef UNIT_TEST
-#error "Do not use PropertyStoreInspector in non-test code."
-#endif
-
-#include "shill/property_store.h"
-
-namespace shill {
-
-// This class supplies functions to unit tests for inspecting a PropertyStore
-// class to get single attributes. We never need to do this in the real
-// PropertyStore, since the only interface to it over the control API is
-// "GetProperties" which iterates over the entire store's properties.
-//
-// This code is done inefficiently -- if we were ever meant to retrieve
-// single properties in production code, we'd access the map directly.
-// For this reason, this class should only be used in unit tests.
-class PropertyStoreInspector {
- public:
- PropertyStoreInspector();
- explicit PropertyStoreInspector(const PropertyStore *store);
- virtual ~PropertyStoreInspector();
-
- const PropertyStore *store() const { return store_; }
- void set_store(const PropertyStore *store) { store_ = store; }
-
- // Methods to allow the getting of properties stored in the referenced
- // |store_| by name. Upon success, these methods return true and return the
- // property value in |value| if non-NULL. Upon failure, they return false and
- // leave |value| untouched.
- bool GetBoolProperty(const std::string &name, bool *value);
- bool GetInt16Property(const std::string &name, int16 *value);
- bool GetInt32Property(const std::string &name, int32 *value);
- bool GetKeyValueStoreProperty(const std::string &name, KeyValueStore *value);
- bool GetStringProperty(const std::string &name, std::string *value);
- bool GetStringmapProperty(const std::string &name,
- std::map<std::string, std::string> *values);
- bool GetStringsProperty(const std::string &name,
- std::vector<std::string> *values);
- bool GetUint8Property(const std::string &name, uint8 *value);
- bool GetUint16Property(const std::string &name, uint16 *value);
- bool GetUint32Property(const std::string &name, uint32 *value);
- bool GetRpcIdentifierProperty(const std::string &name, RpcIdentifier *value);
-
- private:
- template <class V>
- bool GetProperty(
- const std::string &name,
- V *value,
- ReadablePropertyConstIterator<V>(PropertyStore::*getter)() const);
-
- const PropertyStore *store_;
-
- DISALLOW_COPY_AND_ASSIGN(PropertyStoreInspector);
-};
-
-} // namespace shill
-
-
-#endif // SHILL_PROPERTY_STORE_INSPECTOR_H_
diff --git a/property_store_unittest.cc b/property_store_unittest.cc
index 79239d0..8fa059a 100644
--- a/property_store_unittest.cc
+++ b/property_store_unittest.cc
@@ -247,6 +247,14 @@
EXPECT_TRUE(values[1] == it.value());
it.Advance();
EXPECT_TRUE(it.AtEnd());
+
+ Error errors[2];
+ EXPECT_FALSE(store.GetBoolProperty(keys[0], NULL, &errors[0]));
+ EXPECT_EQ(Error::kPermissionDenied, errors[0].type());
+ bool test_value;
+ EXPECT_TRUE(store.GetBoolProperty(keys[1], &test_value, &errors[1]));
+ EXPECT_TRUE(errors[1].IsSuccess());
+ EXPECT_EQ(values[1], test_value);
}
{
const string keys[] = {"int16p1", "int16p2"};
@@ -260,6 +268,14 @@
EXPECT_EQ(values[1], it.value());
it.Advance();
EXPECT_TRUE(it.AtEnd());
+
+ Error errors[2];
+ EXPECT_FALSE(store.GetInt16Property(keys[0], NULL, &errors[0]));
+ EXPECT_EQ(Error::kPermissionDenied, errors[0].type());
+ int16 test_value;
+ EXPECT_TRUE(store.GetInt16Property(keys[1], &test_value, &errors[1]));
+ EXPECT_TRUE(errors[1].IsSuccess());
+ EXPECT_EQ(values[1], test_value);
}
{
const string keys[] = {"int32p1", "int32p2"};
@@ -273,6 +289,14 @@
EXPECT_EQ(values[1], it.value());
it.Advance();
EXPECT_TRUE(it.AtEnd());
+
+ Error errors[2];
+ EXPECT_FALSE(store.GetInt32Property(keys[0], NULL, &errors[0]));
+ EXPECT_EQ(Error::kPermissionDenied, errors[0].type());
+ int32 test_value;
+ EXPECT_TRUE(store.GetInt32Property(keys[1], &test_value, &errors[1]));
+ EXPECT_TRUE(errors[1].IsSuccess());
+ EXPECT_EQ(values[1], test_value);
}
{
const string keys[] = {"stringp1", "stringp2"};
@@ -286,6 +310,14 @@
EXPECT_EQ(values[1], it.value());
it.Advance();
EXPECT_TRUE(it.AtEnd());
+
+ Error errors[2];
+ EXPECT_FALSE(store.GetStringProperty(keys[0], NULL, &errors[0]));
+ EXPECT_EQ(Error::kPermissionDenied, errors[0].type());
+ string test_value;
+ EXPECT_TRUE(store.GetStringProperty(keys[1], &test_value, &errors[1]));
+ EXPECT_TRUE(errors[1].IsSuccess());
+ EXPECT_EQ(values[1], test_value);
}
{
const string keys[] = {"stringmapp1", "stringmapp2"};
@@ -302,6 +334,14 @@
EXPECT_TRUE(values[1] == it.value());
it.Advance();
EXPECT_TRUE(it.AtEnd());
+
+ Error errors[2];
+ EXPECT_FALSE(store.GetStringmapProperty(keys[0], NULL, &errors[0]));
+ EXPECT_EQ(Error::kPermissionDenied, errors[0].type());
+ Stringmap test_value;
+ EXPECT_TRUE(store.GetStringmapProperty(keys[1], &test_value, &errors[1]));
+ EXPECT_TRUE(errors[1].IsSuccess());
+ EXPECT_TRUE(values[1] == test_value);
}
{
const string keys[] = {"stringmapsp1", "stringmapsp2"};
@@ -322,6 +362,14 @@
EXPECT_TRUE(values[1] == it.value());
it.Advance();
EXPECT_TRUE(it.AtEnd());
+
+ Error errors[2];
+ EXPECT_FALSE(store.GetStringmapsProperty(keys[0], NULL, &errors[0]));
+ EXPECT_EQ(Error::kPermissionDenied, errors[0].type());
+ Stringmaps test_value;
+ EXPECT_TRUE(store.GetStringmapsProperty(keys[1], &test_value, &errors[1]));
+ EXPECT_TRUE(errors[1].IsSuccess());
+ EXPECT_TRUE(values[1] == test_value);
}
{
const string keys[] = {"stringsp1", "stringsp2"};
@@ -341,6 +389,14 @@
EXPECT_TRUE(values[1] == it.value());
it.Advance();
EXPECT_TRUE(it.AtEnd());
+
+ Error errors[2];
+ EXPECT_FALSE(store.GetStringsProperty(keys[0], NULL, &errors[0]));
+ EXPECT_EQ(Error::kPermissionDenied, errors[0].type());
+ Strings test_value;
+ EXPECT_TRUE(store.GetStringsProperty(keys[1], &test_value, &errors[1]));
+ EXPECT_TRUE(errors[1].IsSuccess());
+ EXPECT_TRUE(values[1] == test_value);
}
{
const string keys[] = {"uint8p1", "uint8p2"};
@@ -354,6 +410,14 @@
EXPECT_EQ(values[1], it.value());
it.Advance();
EXPECT_TRUE(it.AtEnd());
+
+ Error errors[2];
+ EXPECT_FALSE(store.GetUint8Property(keys[0], NULL, &errors[0]));
+ EXPECT_EQ(Error::kPermissionDenied, errors[0].type());
+ uint8 test_value;
+ EXPECT_TRUE(store.GetUint8Property(keys[1], &test_value, &errors[1]));
+ EXPECT_TRUE(errors[1].IsSuccess());
+ EXPECT_EQ(values[1], test_value);
}
{
const string keys[] = {"uint16p", "uint16p1"};
@@ -367,6 +431,14 @@
EXPECT_EQ(values[1], it.value());
it.Advance();
EXPECT_TRUE(it.AtEnd());
+
+ Error errors[2];
+ EXPECT_FALSE(store.GetUint16Property(keys[0], NULL, &errors[0]));
+ EXPECT_EQ(Error::kPermissionDenied, errors[0].type());
+ uint16 test_value;
+ EXPECT_TRUE(store.GetUint16Property(keys[1], &test_value, &errors[1]));
+ EXPECT_TRUE(errors[1].IsSuccess());
+ EXPECT_EQ(values[1], test_value);
}
}
diff --git a/service_unittest.cc b/service_unittest.cc
index 3b48932..08b4cd8 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -31,7 +31,6 @@
#include "shill/mock_proxy_factory.h"
#include "shill/mock_store.h"
#include "shill/mock_time.h"
-#include "shill/property_store_inspector.h"
#include "shill/property_store_unittest.h"
#include "shill/service_under_test.h"
@@ -878,8 +877,9 @@
class ReadOnlyServicePropertyTest : public ServiceTest {};
TEST_P(ReadOnlyServicePropertyTest, PropertyWriteOnly) {
string property(GetParam().reader().get_string());
- PropertyStoreInspector inspector(&service_->store());
- EXPECT_FALSE(inspector.GetStringProperty(property, NULL));
+ Error error;
+ EXPECT_FALSE(service_->store().GetStringProperty(property, NULL, &error));
+ EXPECT_EQ(Error::kPermissionDenied, error.type());
}
INSTANTIATE_TEST_CASE_P(
@@ -1231,18 +1231,18 @@
TEST_F(ServiceTest, DiagnosticsProperties) {
const string kWallClock0 = "2012-12-09T12:41:22.234567-0800";
const string kWallClock1 = "2012-12-31T23:59:59.345678-0800";
- PropertyStoreInspector inspector(&service_->store());
Strings values;
PushTimestamp(GetDisconnects(), 0, kWallClock0);
- ASSERT_TRUE(
- inspector.GetStringsProperty(kDiagnosticsDisconnectsProperty, &values));
+ Error unused_error;
+ ASSERT_TRUE(service_->store().GetStringsProperty(
+ kDiagnosticsDisconnectsProperty, &values, &unused_error));
ASSERT_EQ(1, values.size());
EXPECT_EQ(kWallClock0, values[0]);
PushTimestamp(GetMisconnects(), 0, kWallClock1);
- ASSERT_TRUE(
- inspector.GetStringsProperty(kDiagnosticsMisconnectsProperty, &values));
+ ASSERT_TRUE(service_->store().GetStringsProperty(
+ kDiagnosticsMisconnectsProperty, &values, &unused_error));
ASSERT_EQ(1, values.size());
EXPECT_EQ(kWallClock1, values[0]);
}
diff --git a/static_ip_parameters_unittest.cc b/static_ip_parameters_unittest.cc
index 649f5fe..88acf91 100644
--- a/static_ip_parameters_unittest.cc
+++ b/static_ip_parameters_unittest.cc
@@ -10,7 +10,6 @@
#include "shill/ipconfig.h"
#include "shill/mock_store.h"
#include "shill/property_store.h"
-#include "shill/property_store_inspector.h"
using std::string;
using std::vector;
@@ -116,21 +115,30 @@
EXPECT_EQ(kTestAddress, props.address);
EXPECT_EQ(kTestMtu, props.mtu);
- PropertyStoreInspector inspector(&store);
- EXPECT_FALSE(inspector.GetStringProperty("StaticIP.Address", NULL));
+ {
+ Error error;
+ EXPECT_FALSE(store.GetStringProperty("StaticIP.Address", NULL, &error));
+ EXPECT_EQ(Error::kNotFound, error.type());
+ }
string string_value;
- EXPECT_TRUE(inspector.GetStringProperty("StaticIP.Gateway", &string_value));
+ Error unused_error;
+ EXPECT_TRUE(store.GetStringProperty("StaticIP.Gateway", &string_value,
+ &unused_error));
EXPECT_EQ(kGateway, string_value);
- EXPECT_FALSE(inspector.GetInt32Property("StaticIP.Mtu", NULL));
- EXPECT_TRUE(inspector.GetStringProperty("StaticIP.NameServers",
- &string_value));
+ {
+ Error error;
+ EXPECT_FALSE(store.GetInt32Property("StaticIP.Mtu", NULL, &error));
+ EXPECT_EQ(Error::kNotFound, error.type());
+ }
+ EXPECT_TRUE(store.GetStringProperty("StaticIP.NameServers", &string_value,
+ &unused_error));
EXPECT_EQ(kNameServers, string_value);
- EXPECT_TRUE(inspector.GetStringProperty("StaticIP.PeerAddress",
- &string_value));
+ EXPECT_TRUE(store.GetStringProperty("StaticIP.PeerAddress", &string_value,
+ &unused_error));
EXPECT_EQ(kPeerAddress, string_value);
int32 int_value;
- EXPECT_TRUE(inspector.GetInt32Property("StaticIP.Prefixlen",
- &int_value));
+ EXPECT_TRUE(store.GetInt32Property("StaticIP.Prefixlen", &int_value,
+ &unused_error));
EXPECT_EQ(kPrefixLen, int_value);
}
@@ -187,23 +195,26 @@
Populate();
static_params_.ApplyTo(&props_);
- PropertyStoreInspector inspector(&store);
string string_value;
- EXPECT_TRUE(inspector.GetStringProperty("SavedIP.Address", &string_value));
+ Error unused_error;
+ EXPECT_TRUE(store.GetStringProperty("SavedIP.Address", &string_value,
+ &unused_error));
EXPECT_EQ(kAddress, string_value);
- EXPECT_TRUE(inspector.GetStringProperty("SavedIP.Gateway", &string_value));
+ EXPECT_TRUE(store.GetStringProperty("SavedIP.Gateway", &string_value,
+ &unused_error));
EXPECT_EQ(kGateway, string_value);
int32 int_value;
- EXPECT_TRUE(inspector.GetInt32Property("SavedIP.Mtu", &int_value));
+ EXPECT_TRUE(store.GetInt32Property("SavedIP.Mtu", &int_value,
+ &unused_error));
EXPECT_EQ(kMtu, int_value);
- EXPECT_TRUE(inspector.GetStringProperty("SavedIP.NameServers",
- &string_value));
+ EXPECT_TRUE(store.GetStringProperty("SavedIP.NameServers", &string_value,
+ &unused_error));
EXPECT_EQ(kNameServers, string_value);
- EXPECT_TRUE(inspector.GetStringProperty("SavedIP.PeerAddress",
- &string_value));
+ EXPECT_TRUE(store.GetStringProperty("SavedIP.PeerAddress", &string_value,
+ &unused_error));
EXPECT_EQ(kPeerAddress, string_value);
- EXPECT_TRUE(inspector.GetInt32Property("SavedIP.Prefixlen",
- &int_value));
+ EXPECT_TRUE(store.GetInt32Property("SavedIP.Prefixlen", &int_value,
+ &unused_error));
EXPECT_EQ(kPrefixLen, int_value);
store.ClearProperty("StaticIP.Address", &error);
@@ -214,20 +225,23 @@
store.ClearProperty("StaticIP.Prefixlen", &error);
static_params_.ApplyTo(&props_);
- EXPECT_TRUE(inspector.GetStringProperty("SavedIP.Address", &string_value));
+ EXPECT_TRUE(store.GetStringProperty("SavedIP.Address", &string_value,
+ &unused_error));
EXPECT_EQ(kPrefix + kAddress, string_value);
- EXPECT_TRUE(inspector.GetStringProperty("SavedIP.Gateway", &string_value));
+ EXPECT_TRUE(store.GetStringProperty("SavedIP.Gateway", &string_value,
+ &unused_error));
EXPECT_EQ(kPrefix + kGateway, string_value);
- EXPECT_TRUE(inspector.GetInt32Property("SavedIP.Mtu", &int_value));
+ EXPECT_TRUE(store.GetInt32Property("SavedIP.Mtu", &int_value,
+ &unused_error));
EXPECT_EQ(kOffset + kMtu, int_value);
- EXPECT_TRUE(inspector.GetStringProperty("SavedIP.NameServers",
- &string_value));
+ EXPECT_TRUE(store.GetStringProperty("SavedIP.NameServers", &string_value,
+ &unused_error));
EXPECT_EQ(kPrefix + kNameServers, string_value);
- EXPECT_TRUE(inspector.GetStringProperty("SavedIP.PeerAddress",
- &string_value));
+ EXPECT_TRUE(store.GetStringProperty("SavedIP.PeerAddress", &string_value,
+ &unused_error));
EXPECT_EQ(kPrefix + kPeerAddress, string_value);
- EXPECT_TRUE(inspector.GetInt32Property("SavedIP.Prefixlen",
- &int_value));
+ EXPECT_TRUE(store.GetInt32Property("SavedIP.Prefixlen", &int_value,
+ &unused_error));
EXPECT_EQ(kOffset + kPrefixLen, int_value);
}
diff --git a/vpn_driver_unittest.cc b/vpn_driver_unittest.cc
index 83e2e61..fe221f3 100644
--- a/vpn_driver_unittest.cc
+++ b/vpn_driver_unittest.cc
@@ -21,7 +21,6 @@
#include "shill/mock_store.h"
#include "shill/nice_mock_control.h"
#include "shill/property_store.h"
-#include "shill/property_store_inspector.h"
using std::string;
using testing::_;
@@ -116,10 +115,10 @@
bool VPNDriverTest::GetProviderProperty(const PropertyStore &store,
const string &key,
string *value) {
- PropertyStoreInspector inspector(&store);
KeyValueStore provider_properties;
- EXPECT_TRUE(inspector.GetKeyValueStoreProperty(
- flimflam::kProviderProperty, &provider_properties));
+ Error error;
+ EXPECT_TRUE(store.GetKeyValueStoreProperty(
+ flimflam::kProviderProperty, &provider_properties, &error));
if (!provider_properties.ContainsString(key)) {
return false;
}
@@ -208,10 +207,13 @@
// KeyValueStore.
PropertyStore store;
driver_.InitPropertyStore(&store);
- PropertyStoreInspector inspector(&store);
// An un-set property should not be readable.
- EXPECT_FALSE(inspector.GetStringProperty(kPortProperty, NULL));
+ {
+ Error error;
+ EXPECT_FALSE(store.GetStringProperty(kPortProperty, NULL, &error));
+ EXPECT_EQ(Error::kInvalidArguments, error.type());
+ }
EXPECT_FALSE(GetProviderProperty(store, kPortProperty, NULL));
const string kProviderName = "boo";
@@ -222,12 +224,18 @@
// We should not be able to read a property out of the driver args using the
// key to the args directly.
- EXPECT_FALSE(inspector.GetStringProperty(kPortProperty, NULL));
+ {
+ Error error;
+ EXPECT_FALSE(store.GetStringProperty(kPortProperty, NULL, &error));
+ EXPECT_EQ(Error::kInvalidArguments, error.type());
+ }
// We should instead be able to find it within the "Provider" stringmap.
- string value;
- EXPECT_TRUE(GetProviderProperty(store, kPortProperty, &value));
- EXPECT_EQ(kPort, value);
+ {
+ string value;
+ EXPECT_TRUE(GetProviderProperty(store, kPortProperty, &value));
+ EXPECT_EQ(kPort, value);
+ }
// We should be able to read empty properties from the "Provider" stringmap.
{
@@ -238,8 +246,11 @@
// Properties that start with the prefix "Provider." should be mapped to the
// name in the Properties dict with the prefix removed.
- EXPECT_TRUE(GetProviderProperty(store, flimflam::kNameProperty, &value));
- EXPECT_EQ(kProviderName, value);
+ {
+ string value;
+ EXPECT_TRUE(GetProviderProperty(store, flimflam::kNameProperty, &value));
+ EXPECT_EQ(kProviderName, value);
+ }
// If we clear a property, we should no longer be able to find it.
{
diff --git a/wifi_unittest.cc b/wifi_unittest.cc
index c878539..6e26070 100644
--- a/wifi_unittest.cc
+++ b/wifi_unittest.cc
@@ -53,7 +53,6 @@
#include "shill/mock_time.h"
#include "shill/mock_wifi_service.h"
#include "shill/nice_mock_control.h"
-#include "shill/property_store_inspector.h"
#include "shill/property_store_unittest.h"
#include "shill/proxy_factory.h"
#include "shill/wifi_endpoint.h"
@@ -155,9 +154,9 @@
EXPECT_TRUE(device_->bgscan_method_.empty());
string method;
- PropertyStoreInspector inspector(&device_->store());
- EXPECT_TRUE(inspector.GetStringProperty(flimflam::kBgscanMethodProperty,
- &method));
+ Error unused_error;
+ EXPECT_TRUE(device_->store().GetStringProperty(
+ flimflam::kBgscanMethodProperty, &method, &unused_error));
EXPECT_EQ(WiFi::kDefaultBgscanMethod, method);
EXPECT_EQ(wpa_supplicant::kNetworkBgscanMethodSimple, method);
@@ -169,14 +168,14 @@
wpa_supplicant::kNetworkBgscanMethodLearn),
&error));
EXPECT_EQ(wpa_supplicant::kNetworkBgscanMethodLearn, device_->bgscan_method_);
- EXPECT_TRUE(inspector.GetStringProperty(flimflam::kBgscanMethodProperty,
- &method));
+ EXPECT_TRUE(device_->store().GetStringProperty(
+ flimflam::kBgscanMethodProperty, &method, &unused_error));
EXPECT_EQ(wpa_supplicant::kNetworkBgscanMethodLearn, method);
EXPECT_TRUE(DBusAdaptor::ClearProperty(
device_->mutable_store(), flimflam::kBgscanMethodProperty, &error));
- EXPECT_TRUE(inspector.GetStringProperty(flimflam::kBgscanMethodProperty,
- &method));
+ EXPECT_TRUE(device_->store().GetStringProperty(
+ flimflam::kBgscanMethodProperty, &method, &unused_error));
EXPECT_EQ(WiFi::kDefaultBgscanMethod, method);
EXPECT_TRUE(device_->bgscan_method_.empty());
}