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>
diff --git a/Makefile b/Makefile
index 68463db..c9a075f 100644
--- a/Makefile
+++ b/Makefile
@@ -275,7 +275,6 @@
profile_dbus_adaptor.o \
profile_dbus_property_exporter.o \
property_store.o \
- provider.o \
proxy_factory.o \
routing_table.o \
resolver.o \
@@ -477,7 +476,6 @@
profile_unittest.o \
property_accessor_unittest.o \
property_store_unittest.o \
- provider_unittest.o \
resolver_unittest.o \
result_aggregator_unittest.o \
routing_table_unittest.o \
diff --git a/ethernet_eap_provider.cc b/ethernet_eap_provider.cc
index a26d15f..5a0556b 100644
--- a/ethernet_eap_provider.cc
+++ b/ethernet_eap_provider.cc
@@ -24,13 +24,33 @@
EthernetEapProvider::~EthernetEapProvider() {}
-ServiceRefPtr EthernetEapProvider::GetService(const KeyValueStore &args,
- Error *error) {
+void EthernetEapProvider::CreateServicesFromProfile(
+ const ProfileRefPtr &profile) {
+ // Since the EthernetEapProvider's service is created during Start(),
+ // there is no need to do anything in this method.
+}
+
+ServiceRefPtr EthernetEapProvider::FindSimilarService(const KeyValueStore &args,
+ Error *error) const {
CHECK_EQ(kTypeEthernetEap, args.LookupString(flimflam::kTypeProperty, ""))
<< "Service type must be Ethernet EAP!";
return service();
}
+ServiceRefPtr EthernetEapProvider::GetService(const KeyValueStore &args,
+ Error *error) {
+ return FindSimilarService(args, error);
+}
+
+ServiceRefPtr EthernetEapProvider::CreateTemporaryService(
+ const KeyValueStore &args,
+ Error *error) {
+ return new EthernetEapService(control_interface_,
+ dispatcher_,
+ metrics_,
+ manager_);
+}
+
void EthernetEapProvider::Start() {
if (!service_) {
service_ = new EthernetEapService(control_interface_,
diff --git a/ethernet_eap_provider.h b/ethernet_eap_provider.h
index 3250438..21708f1 100644
--- a/ethernet_eap_provider.h
+++ b/ethernet_eap_provider.h
@@ -9,7 +9,7 @@
#include <base/callback.h>
-#include "shill/provider.h"
+#include "shill/provider_interface.h"
#include "shill/refptr_types.h"
namespace shill {
@@ -22,7 +22,7 @@
class Manager;
class Metrics;
-class EthernetEapProvider : public Provider {
+class EthernetEapProvider : public ProviderInterface {
public:
typedef base::Callback<void()> CredentialChangeCallback;
@@ -33,8 +33,13 @@
virtual ~EthernetEapProvider();
// Called by Manager as a part of the Provider interface.
+ virtual void CreateServicesFromProfile(const ProfileRefPtr &profile) override;
virtual ServiceRefPtr GetService(const KeyValueStore &args,
Error *error) override;
+ virtual ServiceRefPtr FindSimilarService(
+ const KeyValueStore &args, Error *error) const override;
+ virtual ServiceRefPtr CreateTemporaryService(
+ const KeyValueStore &args, Error *error) override;
virtual void Start() override;
virtual void Stop() override;
diff --git a/ethernet_eap_provider_unittest.cc b/ethernet_eap_provider_unittest.cc
index 7409d8d..9adf2ee 100644
--- a/ethernet_eap_provider_unittest.cc
+++ b/ethernet_eap_provider_unittest.cc
@@ -8,6 +8,7 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include "shill/key_value_store.h"
#include "shill/mock_control.h"
#include "shill/mock_ethernet.h"
#include "shill/mock_event_dispatcher.h"
@@ -118,4 +119,32 @@
provider_.OnCredentialsChanged();
}
+TEST_F(EthernetEapProviderTest, ServiceConstructors) {
+ ServiceRefPtr service;
+ EXPECT_CALL(manager_, RegisterService(_)).WillOnce(SaveArg<0>(&service));
+ provider_.Start();
+ KeyValueStore args;
+ args.SetString(flimflam::kTypeProperty, kTypeEthernetEap);
+ {
+ Error error;
+ EXPECT_EQ(service, provider_.GetService(args, &error));
+ EXPECT_TRUE(error.IsSuccess());
+ }
+ {
+ Error error;
+ EXPECT_EQ(service, provider_.FindSimilarService(args, &error));
+ EXPECT_TRUE(error.IsSuccess());
+ }
+ {
+ Error error;
+ Mock::VerifyAndClearExpectations(&manager_);
+ EXPECT_CALL(manager_, RegisterService(_)).Times(0);
+ ServiceRefPtr temp_service = provider_.CreateTemporaryService(args, &error);
+ EXPECT_TRUE(error.IsSuccess());
+ // Returned service should be non-NULL but not the provider's own service.
+ EXPECT_NE(ServiceRefPtr(), temp_service);
+ EXPECT_NE(service, temp_service);
+ }
+}
+
} // namespace shill
diff --git a/manager.h b/manager.h
index 538d89c..8e58dad 100644
--- a/manager.h
+++ b/manager.h
@@ -525,7 +525,7 @@
// Map of technologies to Provider instances. These pointers are owned
// by the respective scoped_reptr objects that are held over the lifetime
// of the Manager object.
- std::map<Technology::Identifier, Provider *> providers_;
+ std::map<Technology::Identifier, ProviderInterface *> providers_;
// List of startup profile names to push on the profile stack on startup.
std::vector<ProfileRefPtr> profiles_;
ProfileRefPtr ephemeral_profile_;
diff --git a/provider.cc b/provider.cc
deleted file mode 100644
index 973555e..0000000
--- a/provider.cc
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright (c) 2013 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/provider.h"
-
-#include "shill/error.h"
-#include "shill/service.h"
-
-namespace shill {
-
-Provider::Provider() {}
-
-void Provider::CreateServicesFromProfile(const ProfileRefPtr &/*profile*/) {}
-
-ServiceRefPtr Provider::FindSimilarService(
- const KeyValueStore &/*args*/, Error *error) const {
- error->Populate(Error::kInternalError,
- "FindSimilarService is not implemented");
- return NULL;
-}
-
-ServiceRefPtr Provider::GetService(const KeyValueStore &/*args*/,
- Error *error) {
- error->Populate(Error::kInternalError, "GetService is not implemented");
- return NULL;
-}
-
-ServiceRefPtr Provider::CreateTemporaryService(
- const KeyValueStore &/*args*/, Error *error) {
- error->Populate(Error::kInternalError,
- "CreateTemporaryService is not implemented");
- return NULL;
-}
-
-void Provider::Start() {}
-
-void Provider::Stop() {}
-
-} // namespace shill
diff --git a/provider.h b/provider_interface.h
similarity index 63%
rename from provider.h
rename to provider_interface.h
index dad9ffa..5de021a 100644
--- a/provider.h
+++ b/provider_interface.h
@@ -12,49 +12,39 @@
class Error;
class KeyValueStore;
-// This is an object that creates and manages service objects. It provides
-// default implementations for each provider method so sublcasses do not
-// need to implement boilerplate unimplemented methods.
-class Provider {
+// This is an interface for objects that creates and manages service objects.
+class ProviderInterface {
public:
- virtual ~Provider() {}
+ virtual ~ProviderInterface() {}
// Creates services from the entries within |profile|.
- virtual void CreateServicesFromProfile(const ProfileRefPtr &profile);
+ virtual void CreateServicesFromProfile(const ProfileRefPtr &profile) = 0;
- // Find a Service with similar properties to |args|. The criteria
+ // Finds a Service with similar properties to |args|. The criteria
// used are specific to the provider subclass. Returns a reference
// to a matching service if one exists. Otherwise it returns a NULL
// reference and populates |error|.
virtual ServiceRefPtr FindSimilarService(
- const KeyValueStore &args, Error *error) const;
+ const KeyValueStore &args, Error *error) const = 0;
// Retrieves (see FindSimilarService) or creates a service with the
// unique attributes in |args|. The remaining attributes will be
// populated (by Manager) via a later call to Service::Configure().
// Returns a NULL reference and populates |error| on failure.
- virtual ServiceRefPtr GetService(const KeyValueStore &args, Error *error);
+ virtual ServiceRefPtr GetService(const KeyValueStore &args, Error *error) = 0;
- // Create a temporary service with the identifying properties populated
+ // Creates a temporary service with the identifying properties populated
// from |args|. Callers outside of the Provider must never register
// this service with the Manager or connect it since it was never added
// to the provider's service list.
virtual ServiceRefPtr CreateTemporaryService(
- const KeyValueStore &args, Error *error);
+ const KeyValueStore &args, Error *error) = 0;
- // Start the provider.
- virtual void Start();
+ // Starts the provider.
+ virtual void Start() = 0;
- // Stop the provider (will de-register all services).
- virtual void Stop();
-
- protected:
- Provider();
-
- private:
- friend class ProviderTest;
-
- DISALLOW_COPY_AND_ASSIGN(Provider);
+ // Stops the provider (will de-register all services).
+ virtual void Stop() = 0;
};
} // namespace shill
diff --git a/provider_unittest.cc b/provider_unittest.cc
deleted file mode 100644
index ef59c86..0000000
--- a/provider_unittest.cc
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright (c) 2013 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/provider.h"
-
-#include <gmock/gmock.h>
-#include <gtest/gtest.h>
-
-#include "shill/error.h"
-#include "shill/key_value_store.h"
-#include "shill/profile.h"
-#include "shill/service.h"
-
-namespace shill {
-
-class ProviderTest : public testing::Test {
- protected:
- Provider provider_;
-};
-
-TEST_F(ProviderTest, Stubs) {
- // Expect that we do not crash.
- provider_.CreateServicesFromProfile(ProfileRefPtr());
-
- KeyValueStore args;
-
- {
- Error error;
- EXPECT_EQ(NULL, provider_.FindSimilarService(args, &error).get());
- EXPECT_EQ(Error::kInternalError, error.type());
- }
- {
- Error error;
- EXPECT_EQ(NULL, provider_.GetService(args, &error).get());
- EXPECT_EQ(Error::kInternalError, error.type());
- }
-}
-
-} // namespace shill
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) {
diff --git a/vpn_provider.h b/vpn_provider.h
index 3fbb847..2ab9520 100644
--- a/vpn_provider.h
+++ b/vpn_provider.h
@@ -11,7 +11,7 @@
#include <base/basictypes.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
-#include "shill/provider.h"
+#include "shill/provider_interface.h"
#include "shill/refptr_types.h"
namespace shill {
@@ -23,7 +23,7 @@
class Manager;
class Metrics;
-class VPNProvider : public Provider {
+class VPNProvider : public ProviderInterface {
public:
VPNProvider(ControlInterface *control_interface,
EventDispatcher *dispatcher,
@@ -35,8 +35,12 @@
// used for matching services for the VPN provider are the ProviderType,
// ProviderHost mode and Name parameters.
virtual void CreateServicesFromProfile(const ProfileRefPtr &profile) override;
+ virtual ServiceRefPtr FindSimilarService(
+ const KeyValueStore &args, Error *error) const override;
virtual ServiceRefPtr GetService(const KeyValueStore &args,
Error *error) override;
+ virtual ServiceRefPtr CreateTemporaryService(
+ const KeyValueStore &args, Error *error) override;
virtual void Start() override;
virtual void Stop() override;
@@ -62,8 +66,15 @@
// Create a service of type |type| and storage identifier |storage_id|
// and initial parameters |args|. Returns a service reference pointer
- // to the newly created service, or popuplates |error| with an the error
+ // to the newly created service, or populates |error| with an the error
// that caused this to fail.
+ VPNServiceRefPtr CreateServiceInner(const std::string &type,
+ const std::string &name,
+ const std::string &storage_id,
+ Error *error);
+
+ // Calls CreateServiceInner above, and on success registers and adds this
+ // service to the provider's list.
VPNServiceRefPtr CreateService(const std::string &type,
const std::string &name,
const std::string &storage_id,
@@ -73,7 +84,17 @@
// Provider.Host property set to |host|.
VPNServiceRefPtr FindService(const std::string &type,
const std::string &name,
- const std::string &host);
+ const std::string &host) const;
+
+ // Populates |type_ptr|, |name_ptr| and |host_ptr| with the appropriate
+ // values from |args|. Returns True on success, otherwise if any of
+ // these arguments are not available, |error| is populated and False is
+ // returned.
+ static bool GetServiceParametersFromArgs(const KeyValueStore &args,
+ std::string *type_ptr,
+ std::string *name_ptr,
+ std::string *host_ptr,
+ Error *error);
ControlInterface *control_interface_;
EventDispatcher *dispatcher_;
diff --git a/vpn_provider_unittest.cc b/vpn_provider_unittest.cc
index 7d8a583..00d12b3 100644
--- a/vpn_provider_unittest.cc
+++ b/vpn_provider_unittest.cc
@@ -58,6 +58,10 @@
return provider_.services_[idx];
}
+ size_t GetServiceCount() const {
+ return provider_.services_.size();
+ }
+
NiceMockControl control_;
MockMetrics metrics_;
MockManager manager_;
@@ -91,27 +95,77 @@
TEST_F(VPNProviderTest, GetService) {
KeyValueStore args;
- Error e;
args.SetString(flimflam::kTypeProperty, flimflam::kTypeVPN);
args.SetString(flimflam::kProviderTypeProperty, flimflam::kProviderOpenVpn);
args.SetString(flimflam::kProviderHostProperty, kHost);
args.SetString(flimflam::kNameProperty, kName);
- EXPECT_CALL(manager_, device_info()).WillOnce(Return(&device_info_));
- EXPECT_CALL(manager_, RegisterService(_));
- ServiceRefPtr service0 = provider_.GetService(args, &e);
- EXPECT_TRUE(e.IsSuccess());
- ASSERT_TRUE(service0);
- EXPECT_EQ("vpn_10_8_0_1_vpn_name", service0->GetStorageIdentifier());
- EXPECT_EQ(kName, GetServiceFriendlyName(service0));
+
+ {
+ Error error;
+ ServiceRefPtr service = provider_.FindSimilarService(args, &error);
+ EXPECT_EQ(Error::kNotFound, error.type());
+ EXPECT_EQ(NULL, service.get());
+ }
+
+ EXPECT_EQ(0, GetServiceCount());
+
+ ServiceRefPtr service;
+ {
+ Error error;
+ EXPECT_CALL(manager_, device_info()).WillOnce(Return(&device_info_));
+ EXPECT_CALL(manager_, RegisterService(_));
+ service = provider_.GetService(args, &error);
+ EXPECT_TRUE(error.IsSuccess());
+ ASSERT_TRUE(service);
+ testing::Mock::VerifyAndClearExpectations(&manager_);
+ }
+
+ EXPECT_EQ("vpn_10_8_0_1_vpn_name", service->GetStorageIdentifier());
+ EXPECT_EQ(kName, GetServiceFriendlyName(service));
+
+ EXPECT_EQ(1, GetServiceCount());
// Configure the service to set its properties (including Provider.Host).
- service0->Configure(args, &e);
- EXPECT_TRUE(e.IsSuccess());
+ {
+ Error error;
+ service->Configure(args, &error);
+ EXPECT_TRUE(error.IsSuccess());
+ }
+
+ // None of the calls below should cause a new service to be registered.
+ EXPECT_CALL(manager_, RegisterService(_)).Times(0);
// A second call should return the same service.
- ServiceRefPtr service1 = provider_.GetService(args, &e);
- EXPECT_TRUE(e.IsSuccess());
- ASSERT_EQ(service0.get(), service1.get());
+ {
+ Error error;
+ ServiceRefPtr get_service = provider_.GetService(args, &error);
+ EXPECT_TRUE(error.IsSuccess());
+ ASSERT_EQ(service, get_service);
+ }
+
+ EXPECT_EQ(1, GetServiceCount());
+
+ // FindSimilarService should also return this service.
+ {
+ Error error;
+ ServiceRefPtr similar_service = provider_.FindSimilarService(args, &error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_EQ(service, similar_service);
+ }
+
+ EXPECT_EQ(1, GetServiceCount());
+
+ // However, CreateTemporaryService should create a different service.
+ {
+ Error error;
+ ServiceRefPtr temporary_service =
+ provider_.CreateTemporaryService(args, &error);
+ EXPECT_TRUE(error.IsSuccess());
+ EXPECT_NE(service, temporary_service);
+
+ // However this service will not be part of the provider.
+ EXPECT_EQ(1, GetServiceCount());
+ }
}
TEST_F(VPNProviderTest, OnDeviceInfoAvailable) {
diff --git a/wifi_provider.h b/wifi_provider.h
index e968d86..b712e6c 100644
--- a/wifi_provider.h
+++ b/wifi_provider.h
@@ -13,7 +13,7 @@
#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include "shill/accessor_interface.h" // for ByteArrays
-#include "shill/provider.h"
+#include "shill/provider_interface.h"
#include "shill/refptr_types.h"
namespace shill {
@@ -32,7 +32,7 @@
// The WiFi Provider is the holder of all WiFi Services. It holds both
// visible (created due to an Endpoint becoming visible) and invisible
// (created due to user or storage configuration) Services.
-class WiFiProvider : public Provider {
+class WiFiProvider : public ProviderInterface {
public:
static const char kStorageFrequencies[];
static const int kMaxStorageFrequencies;
@@ -60,10 +60,10 @@
// used for matching services for the WiFi provider are the SSID, mode and
// security parameters.
virtual void CreateServicesFromProfile(const ProfileRefPtr &profile) override;
- virtual ServiceRefPtr GetService(const KeyValueStore &args,
- Error *error) override;
virtual ServiceRefPtr FindSimilarService(
const KeyValueStore &args, Error *error) const override;
+ virtual ServiceRefPtr GetService(const KeyValueStore &args,
+ Error *error) override;
virtual ServiceRefPtr CreateTemporaryService(
const KeyValueStore &args, Error *error) override;
virtual void Start() override;
diff --git a/wimax_provider.cc b/wimax_provider.cc
index cbe9ca6..b0d0af5 100644
--- a/wimax_provider.cc
+++ b/wimax_provider.cc
@@ -147,20 +147,37 @@
return true;
}
-ServiceRefPtr WiMaxProvider::GetService(const KeyValueStore &args,
- Error *error) {
- SLOG(WiMax, 2) << __func__;
- CHECK_EQ(args.GetString(flimflam::kTypeProperty), flimflam::kTypeWimax);
+// static
+bool WiMaxProvider::GetServiceParametersFromArgs(const KeyValueStore &args,
+ WiMaxNetworkId *id_ptr,
+ string *name_ptr,
+ Error *error) {
WiMaxNetworkId id = args.LookupString(WiMaxService::kNetworkIdProperty, "");
if (id.empty()) {
Error::PopulateAndLog(
error, Error::kInvalidArguments, "Missing WiMAX network id.");
- return NULL;
+ return false;
}
string name = args.LookupString(flimflam::kNameProperty, "");
if (name.empty()) {
Error::PopulateAndLog(
error, Error::kInvalidArguments, "Missing WiMAX service name.");
+ return false;
+ }
+
+ *id_ptr = id;
+ *name_ptr = name;
+
+ return true;
+}
+
+ServiceRefPtr WiMaxProvider::GetService(const KeyValueStore &args,
+ Error *error) {
+ SLOG(WiMax, 2) << __func__;
+ CHECK_EQ(flimflam::kTypeWimax, args.GetString(flimflam::kTypeProperty));
+ WiMaxNetworkId id;
+ string name;
+ if (!GetServiceParametersFromArgs(args, &id, &name, error)) {
return NULL;
}
WiMaxServiceRefPtr service = GetUniqueService(id, name);
@@ -170,6 +187,35 @@
return service;
}
+ServiceRefPtr WiMaxProvider::FindSimilarService(const KeyValueStore &args,
+ Error *error) const {
+ SLOG(WiMax, 2) << __func__;
+ CHECK_EQ(flimflam::kTypeWimax, args.GetString(flimflam::kTypeProperty));
+ WiMaxNetworkId id;
+ string name;
+ if (!GetServiceParametersFromArgs(args, &id, &name, error)) {
+ return NULL;
+ }
+ string storage_id = WiMaxService::CreateStorageIdentifier(id, name);
+ WiMaxServiceRefPtr service = FindService(storage_id);
+ if (!service) {
+ error->Populate(Error::kNotFound, "Matching service was not found");
+ }
+ return service;
+}
+
+ServiceRefPtr WiMaxProvider::CreateTemporaryService(const KeyValueStore &args,
+ Error *error) {
+ SLOG(WiMax, 2) << __func__;
+ CHECK_EQ(flimflam::kTypeWimax, args.GetString(flimflam::kTypeProperty));
+ WiMaxNetworkId id;
+ string name;
+ if (!GetServiceParametersFromArgs(args, &id, &name, error)) {
+ return NULL;
+ }
+ return CreateService(id, name);
+}
+
void WiMaxProvider::CreateServicesFromProfile(const ProfileRefPtr &profile) {
SLOG(WiMax, 2) << __func__;
bool created = false;
@@ -329,7 +375,7 @@
networks_[path] = info;
}
-WiMaxServiceRefPtr WiMaxProvider::FindService(const string &storage_id) {
+WiMaxServiceRefPtr WiMaxProvider::FindService(const string &storage_id) const {
SLOG(WiMax, 2) << __func__ << "(" << storage_id << ")";
map<string, WiMaxServiceRefPtr>::const_iterator find_it =
services_.find(storage_id);
@@ -350,16 +396,23 @@
SLOG(WiMax, 2) << "Service already exists.";
return service;
}
- service = new WiMaxService(control_, dispatcher_, metrics_, manager_);
- service->set_network_id(id);
- service->set_friendly_name(name);
- service->InitStorageIdentifier();
+ service = CreateService(id, name);
services_[service->GetStorageIdentifier()] = service;
manager_->RegisterService(service);
LOG(INFO) << "Registered WiMAX service: " << service->GetStorageIdentifier();
return service;
}
+WiMaxServiceRefPtr WiMaxProvider::CreateService(const WiMaxNetworkId &id,
+ const string &name) {
+ WiMaxServiceRefPtr service =
+ new WiMaxService(control_, dispatcher_, metrics_, manager_);
+ service->set_network_id(id);
+ service->set_friendly_name(name);
+ service->InitStorageIdentifier();
+ return service;
+}
+
void WiMaxProvider::StartLiveServices() {
SLOG(WiMax, 2) << __func__ << "(" << networks_.size() << ")";
for (map<RpcIdentifier, NetworkInfo>::const_iterator nit = networks_.begin();
diff --git a/wimax_provider.h b/wimax_provider.h
index 56838f3..0879a40 100644
--- a/wimax_provider.h
+++ b/wimax_provider.h
@@ -13,7 +13,7 @@
#include "shill/accessor_interface.h"
#include "shill/dbus_manager.h"
-#include "shill/provider.h"
+#include "shill/provider_interface.h"
#include "shill/refptr_types.h"
#include "shill/wimax_network_proxy_interface.h"
@@ -27,7 +27,7 @@
class ProxyFactory;
class WiMaxManagerProxyInterface;
-class WiMaxProvider : public Provider {
+class WiMaxProvider : public ProviderInterface {
public:
WiMaxProvider(ControlInterface *control,
EventDispatcher *dispatcher,
@@ -39,8 +39,12 @@
// used for matching services for the WiMax provider are the NetworkId,
// mode and Name parameters.
virtual void CreateServicesFromProfile(const ProfileRefPtr &profile) override;
+ virtual ServiceRefPtr FindSimilarService(
+ const KeyValueStore &args, Error *error) const override;
virtual ServiceRefPtr GetService(const KeyValueStore &args,
Error *error) override;
+ virtual ServiceRefPtr CreateTemporaryService(
+ const KeyValueStore &args, Error *error) override;
void Start() override;
void Stop() override;
@@ -99,13 +103,26 @@
// Finds and returns the service identified by |storage_id|. Returns NULL if
// the service is not found.
- WiMaxServiceRefPtr FindService(const std::string &storage_id);
+ WiMaxServiceRefPtr FindService(const std::string &storage_id) const;
// Finds or creates a service with the given parameters. The parameters
// uniquely identify a service so no duplicate services will be created.
+ // The service will be registered and a memeber of the provider's
+ // |services_| map.
WiMaxServiceRefPtr GetUniqueService(const WiMaxNetworkId &id,
const std::string &name);
+ // Allocates a service with the given parameters.
+ WiMaxServiceRefPtr CreateService(const WiMaxNetworkId &id,
+ const std::string &name);
+
+ // Populates the |id_ptr| and |name_ptr| from the parameters in |args|.
+ // Returns true on success, otheriwse populates |error| and returns false.
+ static bool GetServiceParametersFromArgs(const KeyValueStore &args,
+ WiMaxNetworkId *id_ptr,
+ std::string *name_ptr,
+ Error *error);
+
// Starts all services with network ids in the current set of live
// networks. This method also creates, registers and starts the default
// service for each live network.
diff --git a/wimax_provider_unittest.cc b/wimax_provider_unittest.cc
index a1bcf78..9cff1c5 100644
--- a/wimax_provider_unittest.cc
+++ b/wimax_provider_unittest.cc
@@ -508,20 +508,39 @@
args.SetString(flimflam::kNameProperty, kName);
static const char kIdentity[] = "joe";
args.SetString(flimflam::kEapIdentityProperty, kIdentity);
- EXPECT_CALL(manager_, RegisterService(_));
+
e.Reset();
+ service = provider_.FindSimilarService(args, &e);
+ EXPECT_EQ(ServiceRefPtr(), service);
+ EXPECT_EQ(Error::kNotFound, e.type());
+
+ e.Reset();
+ EXPECT_CALL(manager_, RegisterService(_));
service = provider_.GetService(args, &e);
EXPECT_TRUE(e.IsSuccess());
ASSERT_TRUE(service);
+ testing::Mock::VerifyAndClearExpectations(&manager_);
// GetService should create a service with only identifying parameters set.
EXPECT_EQ(kName, GetServiceFriendlyName(service));
EXPECT_EQ("", service->eap()->identity());
+ e.Reset();
+ ServiceRefPtr similar_service = provider_.FindSimilarService(args, &e);
+ EXPECT_EQ(service, similar_service);
+ EXPECT_TRUE(e.IsSuccess());
+
// After configuring the service, other parameters should be set.
service->Configure(args, &e);
EXPECT_TRUE(e.IsSuccess());
EXPECT_EQ(kIdentity, service->eap()->identity());
+
+ e.Reset();
+ EXPECT_CALL(manager_, RegisterService(_)).Times(0);
+ ServiceRefPtr temporary_service = provider_.CreateTemporaryService(args, &e);
+ EXPECT_NE(ServiceRefPtr(), temporary_service);
+ EXPECT_NE(service, temporary_service);
+ EXPECT_TRUE(e.IsSuccess());
}
TEST_F(WiMaxProviderTest, SelectCarrier) {