commit bc14fb7e10919a39d8e82d5981d94961aa5d4be7
author Paul Stewart <pstew@chromium.org> Tue Jul 30 08:21:58 2013 -0700
committer ChromeBot <chrome-bot@google.com> Wed Jul 31 16:32:16 2013 -0700
tree df1306b66ce365fd7527eaafdc958d6acd6fedef
parent b87d22b774fe15b6580d13638d5f0514bde031fd

shill: *Provider: Flesh out inherited classes

Implement CreateTemporaryService and FindSimilarService for all
remaining service subclasses.  Now that this is done, change
Provider into a pure-virtual ProviderInterface and remove its
implementation and tests.

BUG=chromium:265592
TEST=Unit tests

Change-Id: I4b74ad7b8602d90908b7596bbbb08eddb01e1c17
Reviewed-on: https://gerrit.chromium.org/gerrit/63650
Commit-Queue: Paul Stewart <pstew@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>

vpn_provider.cc

diff --git a/vpn_provider.cc b/vpn_provider.cc
index 0e2474b..959bc0e 100644
--- a/vpn_provider.cc
+++ b/vpn_provider.cc
@@ -39,20 +39,42 @@
 
 void VPNProvider::Stop() {}
 
-ServiceRefPtr VPNProvider::GetService(const KeyValueStore &args,
-                                      Error *error) {
+// static
+bool VPNProvider::GetServiceParametersFromArgs(const KeyValueStore &args,
+                                               string *type_ptr,
+                                               string *name_ptr,
+                                               string *host_ptr,
+                                               Error *error) {
   SLOG(VPN, 2) << __func__;
   string type = args.LookupString(flimflam::kProviderTypeProperty, "");
   if (type.empty()) {
     Error::PopulateAndLog(
         error, Error::kNotSupported, "Missing VPN type property.");
-    return NULL;
+    return false;
   }
 
   string host = args.LookupString(flimflam::kProviderHostProperty, "");
   if (host.empty()) {
     Error::PopulateAndLog(
         error, Error::kNotSupported, "Missing VPN host property.");
+    return false;
+  }
+
+  *type_ptr = type,
+  *host_ptr = host,
+  *name_ptr = args.LookupString(flimflam::kNameProperty, "");
+
+  return true;
+}
+
+ServiceRefPtr VPNProvider::GetService(const KeyValueStore &args,
+                                      Error *error) {
+  SLOG(VPN, 2) << __func__;
+  string type;
+  string name;
+  string host;
+
+  if (!GetServiceParametersFromArgs(args, &type, &name, &host, error)) {
     return NULL;
   }
 
@@ -61,8 +83,6 @@
     return NULL;
   }
 
-  string name = args.LookupString(flimflam::kNameProperty, "");
-
   // Find a service in the provider list which matches these parameters.
   VPNServiceRefPtr service = FindService(type, name, host);
   if (service == NULL) {
@@ -71,6 +91,26 @@
   return service;
 }
 
+ServiceRefPtr VPNProvider::FindSimilarService(const KeyValueStore &args,
+                                              Error *error) const {
+  SLOG(VPN, 2) << __func__;
+  string type;
+  string name;
+  string host;
+
+  if (!GetServiceParametersFromArgs(args, &type, &name, &host, error)) {
+    return NULL;
+  }
+
+  // Find a service in the provider list which matches these parameters.
+  VPNServiceRefPtr service = FindService(type, name, host);
+  if (!service) {
+    error->Populate(Error::kNotFound, "Matching service was not found");
+  }
+
+  return service;
+}
+
 bool VPNProvider::OnDeviceInfoAvailable(const string &link_name,
                                         int interface_index) {
   for (vector<VPNServiceRefPtr>::const_iterator it = services_.begin();
@@ -146,10 +186,10 @@
   }
 }
 
-VPNServiceRefPtr VPNProvider::CreateService(const string &type,
-                                            const string &name,
-                                            const string &storage_id,
-                                            Error *error) {
+VPNServiceRefPtr VPNProvider::CreateServiceInner(const string &type,
+                                                 const string &name,
+                                                 const string &storage_id,
+                                                 Error *error) {
   SLOG(VPN, 2) << __func__ << " type " << type << " name " << name
                << " storage id " << storage_id;
 #if defined(DISABLE_VPN)
@@ -181,17 +221,27 @@
   if (!name.empty()) {
     service->set_friendly_name(name);
   }
-  services_.push_back(service);
-  manager_->RegisterService(service);
-
   return service;
 
 #endif  // DISABLE_VPN
 }
 
+VPNServiceRefPtr VPNProvider::CreateService(const string &type,
+                                            const string &name,
+                                            const string &storage_id,
+                                            Error *error) {
+  VPNServiceRefPtr service = CreateServiceInner(type, name, storage_id, error);
+  if (service) {
+    services_.push_back(service);
+    manager_->RegisterService(service);
+  }
+
+  return service;
+}
+
 VPNServiceRefPtr VPNProvider::FindService(const string &type,
                                           const string &name,
-                                          const string &host) {
+                                          const string &host) const {
   for (vector<VPNServiceRefPtr>::const_iterator it = services_.begin();
        it != services_.end();
        ++it) {
@@ -204,6 +254,24 @@
   return NULL;
 }
 
+ServiceRefPtr VPNProvider::CreateTemporaryService(
+      const KeyValueStore &args, Error *error) {
+  string type;
+  string name;
+  string host;
+
+  if (!GetServiceParametersFromArgs(args, &type, &name, &host, error)) {
+    return NULL;
+  }
+
+  string storage_id = VPNService::CreateStorageIdentifier(args, error);
+  if (storage_id.empty()) {
+    return NULL;
+  }
+
+  return CreateServiceInner(type, name, storage_id, error);
+}
+
 bool VPNProvider::HasActiveService() const {
   for (vector<VPNServiceRefPtr>::const_iterator it = services_.begin();
        it != services_.end(); ++it) {