shill: vpn: Set the friendly VPN service name.
Also, implement a more concise KeyValueStore string lookup method and add unit
tests.
BUG=chromium-os:27775
TEST=unit tests
Change-Id: If6ab38a9110c09275816bcc6ca992a77425859eb
Reviewed-on: https://gerrit.chromium.org/gerrit/18106
Commit-Ready: Darin Petkov <petkov@chromium.org>
Reviewed-by: Darin Petkov <petkov@chromium.org>
Tested-by: Darin Petkov <petkov@chromium.org>
diff --git a/Makefile b/Makefile
index 60badd4..a83d76e 100644
--- a/Makefile
+++ b/Makefile
@@ -227,6 +227,7 @@
ip_address_unittest.o \
ipconfig_unittest.o \
key_file_store_unittest.o \
+ key_value_store_unittest.o \
manager_unittest.o \
metrics_unittest.o \
mock_adaptors.o \
diff --git a/key_value_store.cc b/key_value_store.cc
index faba479..26e2b3b 100644
--- a/key_value_store.cc
+++ b/key_value_store.cc
@@ -56,4 +56,9 @@
uint_properties_[name] = value;
}
+string KeyValueStore::LookupString(const string &name,
+ const string &default_value) const {
+ return ContainsString(name) ? GetString(name) : default_value;
+}
+
} // namespace shill
diff --git a/key_value_store.h b/key_value_store.h
index 6d03343..f5f0ba0 100644
--- a/key_value_store.h
+++ b/key_value_store.h
@@ -41,6 +41,11 @@
void SetString(const std::string& name, const std::string& value);
void SetUint(const std::string &name, uint32 value);
+ // If |name| is in this store returns its value, otherwise returns
+ // |default_value|.
+ std::string LookupString(const std::string &name,
+ const std::string &default_value) const;
+
const std::map<std::string, bool> &bool_properties() const {
return bool_properties_;
}
diff --git a/key_value_store_unittest.cc b/key_value_store_unittest.cc
new file mode 100644
index 0000000..03964da
--- /dev/null
+++ b/key_value_store_unittest.cc
@@ -0,0 +1,27 @@
+// 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/key_value_store.h"
+
+#include <gtest/gtest.h>
+
+using testing::Test;
+
+namespace shill {
+
+class KeyValueStoreTest : public Test {
+ public:
+ KeyValueStoreTest() {}
+
+ protected:
+ KeyValueStore store_;
+};
+
+TEST_F(KeyValueStoreTest, LookupString) {
+ EXPECT_EQ("bar", store_.LookupString("foo", "bar"));
+ store_.SetString("foo", "zoo");
+ EXPECT_EQ("zoo", store_.LookupString("foo", "bar"));
+}
+
+} // namespace shill
diff --git a/openvpn_driver.cc b/openvpn_driver.cc
index 41d465a..80fba60 100644
--- a/openvpn_driver.cc
+++ b/openvpn_driver.cc
@@ -306,10 +306,7 @@
}
void OpenVPNDriver::InitOptions(vector<string> *options, Error *error) {
- string vpnhost;
- if (args_.ContainsString(flimflam::kProviderHostProperty)) {
- vpnhost = args_.GetString(flimflam::kProviderHostProperty);
- }
+ string vpnhost = args_.LookupString(flimflam::kProviderHostProperty, "");
if (vpnhost.empty()) {
Error::PopulateAndLog(
error, Error::kInvalidArguments, "VPN host not specified.");
@@ -379,10 +376,8 @@
<< "not implemented yet.";
// TLS suport.
- string remote_cert_tls;
- if (args_.ContainsString(flimflam::kOpenVPNRemoteCertTLSProperty)) {
- remote_cert_tls = args_.GetString(flimflam::kOpenVPNRemoteCertTLSProperty);
- }
+ string remote_cert_tls =
+ args_.LookupString(flimflam::kOpenVPNRemoteCertTLSProperty, "");
if (remote_cert_tls.empty()) {
remote_cert_tls = "server";
}
@@ -435,10 +430,7 @@
void OpenVPNDriver::AppendValueOption(
const string &property, const string &option, vector<string> *options) {
- string value;
- if (args_.ContainsString(property)) {
- value = args_.GetString(property);
- }
+ string value = args_.LookupString(property, "");
if (!value.empty()) {
options->push_back(option);
options->push_back(value);
diff --git a/service.h b/service.h
index 50b75ca..7917924 100644
--- a/service.h
+++ b/service.h
@@ -248,6 +248,7 @@
// Setter is deliberately omitted; use MakeFavorite.
const std::string &friendly_name() const { return friendly_name_; }
+ void set_friendly_name(const std::string &n) { friendly_name_ = n; }
int32 priority() const { return priority_; }
void set_priority(int32 priority) { priority_ = priority; }
@@ -298,8 +299,6 @@
// Returns true if a character is disallowed to be in a service storage id.
static bool IllegalChar(char a) { return !LegalChar(a); }
- void set_friendly_name(const std::string &n) { friendly_name_ = n; }
-
virtual std::string CalculateState(Error *error);
// Returns whether this service is in a state conducive to auto-connect.
diff --git a/vpn_provider.cc b/vpn_provider.cc
index be3c466..8ac7b38 100644
--- a/vpn_provider.cc
+++ b/vpn_provider.cc
@@ -35,7 +35,8 @@
VPNServiceRefPtr VPNProvider::GetService(const KeyValueStore &args,
Error *error) {
VLOG(2) << __func__;
- if (!args.ContainsString(flimflam::kProviderTypeProperty)) {
+ string type = args.LookupString(flimflam::kProviderTypeProperty, "");
+ if (type.empty()) {
Error::PopulateAndLog(
error, Error::kNotSupported, "Missing VPN type property.");
return NULL;
@@ -46,7 +47,6 @@
return NULL;
}
- const string &type = args.GetString(flimflam::kProviderTypeProperty);
scoped_ptr<VPNDriver> driver;
if (type == flimflam::kProviderOpenVpn) {
driver.reset(new OpenVPNDriver(
@@ -61,6 +61,10 @@
VPNServiceRefPtr service = new VPNService(
control_interface_, dispatcher_, metrics_, manager_, driver.release());
service->set_storage_id(storage_id);
+ string name = args.LookupString(flimflam::kProviderNameProperty, "");
+ if (!name.empty()) {
+ service->set_friendly_name(name);
+ }
services_.push_back(service);
manager_->RegisterService(service);
return service;
diff --git a/vpn_provider_unittest.cc b/vpn_provider_unittest.cc
index 1f7c398..e65bff8 100644
--- a/vpn_provider_unittest.cc
+++ b/vpn_provider_unittest.cc
@@ -79,6 +79,7 @@
EXPECT_TRUE(e.IsSuccess());
ASSERT_TRUE(service);
EXPECT_EQ("vpn_10_8_0_1_vpn_name", service->GetStorageIdentifier());
+ EXPECT_EQ(kName, service->friendly_name());
}
TEST_F(VPNProviderTest, OnDeviceInfoAvailable) {
diff --git a/vpn_service.cc b/vpn_service.cc
index 3e3a7b9..6264cb8 100644
--- a/vpn_service.cc
+++ b/vpn_service.cc
@@ -47,19 +47,13 @@
// static
string VPNService::CreateStorageIdentifier(const KeyValueStore &args,
Error *error) {
- string host;
- if (args.ContainsString(flimflam::kProviderHostProperty)) {
- host = args.GetString(flimflam::kProviderHostProperty);
- }
+ string host = args.LookupString(flimflam::kProviderHostProperty, "");
if (host.empty()) {
Error::PopulateAndLog(
error, Error::kInvalidProperty, "Missing VPN host.");
return "";
}
- string name;
- if (args.ContainsString(flimflam::kProviderNameProperty)) {
- name = args.GetString(flimflam::kProviderNameProperty);
- }
+ string name = args.LookupString(flimflam::kProviderNameProperty, "");
if (name.empty()) {
Error::PopulateAndLog(
error, Error::kNotSupported, "Missing VPN name.");