shill: Add "IPConfig" property for Services
Add an IPConfig property to services so it can be directly referenced
here instead of indirectly through the device. A side effect of this
is that one can query the IPConfig RPCIdentifier for a VPN service
even though the device is not registered. While we are at it, fix
all the PropertyStore values to be read-only since we never want to
allow these fields to be modified on the IPConfig itself. Later we
will provide a method for individual IPConfig fields to be overridden
but these will be Service properties.
BUG=chromium-os:29540
TEST=Manual: Over DBus, ensure that IPConfig property points at a
readable IPConfig instance, both for OpenVPN, Ethernet and WiFi.
Ensure that in the Chrome UI, the "Network Info" is still available
(basically checking to make sure IPConfigs are still displayable
using the old method) even thought the parameters are now set to
be "Const".
Change-Id: Ib01297ef8dc9bec828ed57361873e9eebc0354bd
Reviewed-on: https://gerrit.chromium.org/gerrit/20983
Commit-Ready: Paul Stewart <pstew@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/connection.cc b/connection.cc
index 71cec11..f836f2d 100644
--- a/connection.cc
+++ b/connection.cc
@@ -110,6 +110,8 @@
dns_domain_search_ = config->properties().domain_search;
}
+ ipconfig_rpc_identifier_ = config->GetRpcIdentifier();
+
if (is_default_) {
resolver_->SetDNSFromIPConfig(config);
}
diff --git a/connection.h b/connection.h
index ff638cf..e892569 100644
--- a/connection.h
+++ b/connection.h
@@ -47,6 +47,10 @@
return dns_servers_;
}
+ virtual const std::string &ipconfig_rpc_identifier() const {
+ return ipconfig_rpc_identifier_;
+ }
+
// Request to accept traffic routed to this connection even if it is not
// the default. This request is ref-counted so the caller must call
// ReleaseRouting() when they no longer need this facility.
@@ -82,6 +86,7 @@
Technology::Identifier technology_;
std::vector<std::string> dns_servers_;
std::vector<std::string> dns_domain_search_;
+ std::string ipconfig_rpc_identifier_;
// Store cached copies of singletons for speed/ease of testing
const DeviceInfo *device_info_;
diff --git a/dhcp_config.cc b/dhcp_config.cc
index ee9ec53..8708f12 100644
--- a/dhcp_config.cc
+++ b/dhcp_config.cc
@@ -64,8 +64,6 @@
root_("/"),
dispatcher_(dispatcher),
glib_(glib) {
- mutable_store()->RegisterConstString(flimflam::kAddressProperty,
- &(properties().address));
SLOG(DHCP, 2) << __func__ << ": " << device_name;
}
diff --git a/dhcp_config_unittest.cc b/dhcp_config_unittest.cc
index ed39b2c..115b0f7 100644
--- a/dhcp_config_unittest.cc
+++ b/dhcp_config_unittest.cc
@@ -373,10 +373,6 @@
}
TEST_F(DHCPConfigTest, SetProperty) {
- EXPECT_TRUE(DBusAdaptor::SetProperty(config_->mutable_store(),
- flimflam::kMtuProperty,
- PropertyStoreTest::kInt32V,
- NULL));
::DBus::Error error;
// Ensure that an attempt to write a R/O property returns InvalidArgs error.
EXPECT_FALSE(DBusAdaptor::SetProperty(config_->mutable_store(),
diff --git a/ipconfig.cc b/ipconfig.cc
index 5f7147b..2c510fe 100644
--- a/ipconfig.cc
+++ b/ipconfig.cc
@@ -45,22 +45,21 @@
}
void IPConfig::Init() {
- // Address might be R/O or not, depending on the type of IPconfig, so
- // we'll leave this up to the subclasses.
- // Register(Const?)String(flimflam::kAddressProperty, &properties_.address);
- store_.RegisterString(flimflam::kBroadcastProperty,
- &properties_.broadcast_address);
- store_.RegisterString(flimflam::kDomainNameProperty,
- &properties_.domain_name);
- store_.RegisterString(flimflam::kGatewayProperty, &properties_.gateway);
+ store_.RegisterConstString(flimflam::kAddressProperty,
+ &properties_.address);
+ store_.RegisterConstString(flimflam::kBroadcastProperty,
+ &properties_.broadcast_address);
+ store_.RegisterConstString(flimflam::kDomainNameProperty,
+ &properties_.domain_name);
+ store_.RegisterConstString(flimflam::kGatewayProperty, &properties_.gateway);
store_.RegisterConstString(flimflam::kMethodProperty, &properties_.method);
- store_.RegisterInt32(flimflam::kMtuProperty, &properties_.mtu);
- store_.RegisterStrings(flimflam::kNameServersProperty,
- &properties_.dns_servers);
- store_.RegisterString(flimflam::kPeerAddressProperty,
- &properties_.peer_address);
- store_.RegisterInt32(flimflam::kPrefixlenProperty,
- &properties_.subnet_prefix);
+ store_.RegisterConstInt32(flimflam::kMtuProperty, &properties_.mtu);
+ store_.RegisterConstStrings(flimflam::kNameServersProperty,
+ &properties_.dns_servers);
+ store_.RegisterConstString(flimflam::kPeerAddressProperty,
+ &properties_.peer_address);
+ store_.RegisterConstInt32(flimflam::kPrefixlenProperty,
+ &properties_.subnet_prefix);
// TODO(cmasone): Does anyone use this?
// store_.RegisterStrings(flimflam::kSearchDomainsProperty,
// &properties_.domain_search);
diff --git a/mock_connection.h b/mock_connection.h
index 5092ef0..620ae60 100644
--- a/mock_connection.h
+++ b/mock_connection.h
@@ -20,6 +20,7 @@
MOCK_METHOD1(UpdateFromIPConfig, void(const IPConfigRefPtr &config));
MOCK_CONST_METHOD0(is_default, bool());
MOCK_METHOD1(SetIsDefault, void(bool is_default));
+ MOCK_CONST_METHOD0(ipconfig_rpc_identifier, const std::string &());
MOCK_METHOD0(RequestRouting, void());
MOCK_METHOD0(ReleaseRouting, void());
MOCK_CONST_METHOD0(interface_name, const std::string &());
diff --git a/service.cc b/service.cc
index f6b6947..b2f83cd 100644
--- a/service.cc
+++ b/service.cc
@@ -173,6 +173,9 @@
HelpRegisterDerivedUint16(shill::kHTTPProxyPortProperty,
&Service::GetHTTPProxyPort,
NULL);
+ HelpRegisterDerivedRpcIdentifier(shill::kIPConfigProperty,
+ &Service::GetIPConfigRpcIdentifier,
+ NULL);
HelpRegisterDerivedBool(flimflam::kIsActiveProperty,
&Service::IsActive,
NULL);
@@ -664,6 +667,23 @@
}
}
+string Service::GetIPConfigRpcIdentifier(Error *error) {
+ if (!connection_) {
+ error->Populate(Error::kNotFound);
+ return "/";
+ }
+
+ string id = connection_->ipconfig_rpc_identifier();
+
+ if (id.empty()) {
+ // Do not return an empty IPConfig.
+ error->Populate(Error::kNotFound);
+ return "/";
+ }
+
+ return id;
+}
+
void Service::set_connectable(bool connectable) {
connectable_ = connectable;
adaptor_->EmitBoolChanged(flimflam::kConnectableProperty, connectable_);
diff --git a/service.h b/service.h
index 0a81f89..614e5c4 100644
--- a/service.h
+++ b/service.h
@@ -409,6 +409,7 @@
FRIEND_TEST(ServiceTest, ConfigureIgnoredProperty);
FRIEND_TEST(ServiceTest, ConfigureStringProperty);
FRIEND_TEST(ServiceTest, Constructor);
+ FRIEND_TEST(ServiceTest, GetIPConfigRpcIdentifier);
FRIEND_TEST(ServiceTest, GetProperties);
FRIEND_TEST(ServiceTest, IsAutoConnectable);
FRIEND_TEST(ServiceTest, Save);
@@ -442,6 +443,8 @@
virtual std::string GetDeviceRpcId(Error *error) = 0;
+ std::string GetIPConfigRpcIdentifier(Error *error);
+
std::string GetNameProperty(Error *error);
void AssertTrivialSetNameProperty(const std::string &name, Error *error);
diff --git a/service_unittest.cc b/service_unittest.cc
index 2d94615..56f6034 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -20,6 +20,8 @@
#include "shill/manager.h"
#include "shill/mock_adaptors.h"
#include "shill/mock_control.h"
+#include "shill/mock_connection.h"
+#include "shill/mock_device_info.h"
#include "shill/mock_manager.h"
#include "shill/mock_profile.h"
#include "shill/mock_store.h"
@@ -36,6 +38,7 @@
using testing::DoAll;
using testing::NiceMock;
using testing::Return;
+using testing::ReturnRef;
using testing::StrictMock;
using testing::SetArgumentPointee;
using testing::Test;
@@ -462,4 +465,44 @@
DBusAdaptor::StringToVariant(flimflam::kEapPrivateKeyPasswordProperty),
DBusAdaptor::StringToVariant(flimflam::kEapPasswordProperty)));
+
+TEST_F(ServiceTest, GetIPConfigRpcIdentifier) {
+ {
+ Error error;
+ EXPECT_EQ("/", service_->GetIPConfigRpcIdentifier(&error));
+ EXPECT_EQ(Error::kNotFound, error.type());
+ }
+
+ scoped_ptr<MockDeviceInfo> mock_device_info(
+ new NiceMock<MockDeviceInfo>(control_interface(), dispatcher(), metrics(),
+ &mock_manager_));
+ scoped_refptr<MockConnection> mock_connection(
+ new NiceMock<MockConnection>(mock_device_info.get()));
+
+ service_->connection_ = mock_connection;
+
+ {
+ Error error;
+ const string empty_string;
+ EXPECT_CALL(*mock_connection, ipconfig_rpc_identifier())
+ .WillOnce(ReturnRef(empty_string));
+ EXPECT_EQ("/", service_->GetIPConfigRpcIdentifier(&error));
+ EXPECT_EQ(Error::kNotFound, error.type());
+ }
+
+ {
+ Error error;
+ const string nonempty_string("/ipconfig/path");
+ EXPECT_CALL(*mock_connection, ipconfig_rpc_identifier())
+ .WillOnce(ReturnRef(nonempty_string));
+ EXPECT_EQ(nonempty_string, service_->GetIPConfigRpcIdentifier(&error));
+ EXPECT_EQ(Error::kSuccess, error.type());
+ }
+
+ // Assure orderly destruction of the Connection before DeviceInfo.
+ service_->connection_ = NULL;
+ mock_connection = NULL;
+ mock_device_info.reset();
+}
+
} // namespace shill