Remove D-Bus dependency from Service
Here are the main changes:
1. Update service to use ServiceAdaptorInterface instead of using
D-Bus adaptor directly.
2. Update D-Bus object registration for Service and Config to be
synchronous, to remove unnecessary complexity. This also
eliminates the need for Manager::CreateService handler to be
asynchronous.
3. Update Service APIs to use the internal Error instead of brillo::Error.
4. Use MockServiceAdaptor for testing, fake version is not needed
since the adaptor properties for Service are not being used in test.
Bug: 24194427
TEST=Start AP service on both Brillo and Chrome OS devices
TEST=Run unittests on both Brillo and Chrome OS
Change-Id: Ib94a4b91ef402415470e0131afdeeef5105817e4
diff --git a/Android.mk b/Android.mk
index 8e4062f..fd816f6 100644
--- a/Android.mk
+++ b/Android.mk
@@ -127,6 +127,7 @@
mock_manager.cc \
mock_process_factory.cc \
mock_service.cc \
+ mock_service_adaptor.cc \
service_unittest.cc \
run_all_tests.cc
LOCAL_STATIC_LIBRARIES := libapmanager libgmock
diff --git a/apmanager.gyp b/apmanager.gyp
index b5f8de5..a261984 100644
--- a/apmanager.gyp
+++ b/apmanager.gyp
@@ -163,6 +163,7 @@
'mock_manager.cc',
'mock_process_factory.cc',
'mock_service.cc',
+ 'mock_service_adaptor.cc',
'service_unittest.cc',
'testrunner.cc',
],
diff --git a/dbus/device_dbus_adaptor.cc b/dbus/device_dbus_adaptor.cc
index 9091641..721d8d5 100644
--- a/dbus/device_dbus_adaptor.cc
+++ b/dbus/device_dbus_adaptor.cc
@@ -39,6 +39,7 @@
device->identifier())),
dbus_object_(object_manager, bus, object_path_) {
// Register D-Bus object.
+ adaptor_.RegisterWithDBusObject(&dbus_object_);
dbus_object_.RegisterAndBlock();
}
diff --git a/dbus/service_dbus_adaptor.cc b/dbus/service_dbus_adaptor.cc
index 2e90ae7..66cce6e 100644
--- a/dbus/service_dbus_adaptor.cc
+++ b/dbus/service_dbus_adaptor.cc
@@ -16,6 +16,7 @@
#include "apmanager/dbus/service_dbus_adaptor.h"
+#include <base/bind.h>
#include <base/strings/stringprintf.h>
#include <dbus_bindings/org.chromium.apmanager.Manager.h>
@@ -40,6 +41,9 @@
service->identifier())),
dbus_object_(object_manager, bus, object_path_),
service_(service) {
+ // Need to initialize Config property with a valid path before registering
+ // to the bus.
+ SetConfig(service_->config());
// Register D-Bus object.
adaptor_.RegisterWithDBusObject(&dbus_object_);
dbus_object_.RegisterAndBlock();
@@ -49,12 +53,16 @@
void ServiceDBusAdaptor::Start(
std::unique_ptr<DBusMethodResponse<>> response) {
- // TODO(zqiu): to be implemented.
+ service_->Start(
+ base::Bind(&ServiceDBusAdaptor::OnStartCompleted,
+ base::Unretained(this),
+ base::Passed(&response)));
}
-bool ServiceDBusAdaptor::Stop(brillo::ErrorPtr* error) {
- // TODO(zqiu): to be implemented.
- return false;
+bool ServiceDBusAdaptor::Stop(brillo::ErrorPtr* dbus_error) {
+ Error error;
+ service_->Stop(&error);
+ return !error.ToDBusError(dbus_error);
}
RPCObjectIdentifier ServiceDBusAdaptor::GetRpcObjectIdentifier() {
@@ -69,4 +77,14 @@
adaptor_.SetState(state);
}
+void ServiceDBusAdaptor::OnStartCompleted(
+ std::unique_ptr<DBusMethodResponse<>> response, const Error& error) {
+ brillo::ErrorPtr dbus_error;
+ if (error.ToDBusError(&dbus_error)) {
+ response->ReplyWithError(dbus_error.get());
+ } else {
+ response->Return();
+ }
+}
+
} // namespace apmanager
diff --git a/dbus/service_dbus_adaptor.h b/dbus/service_dbus_adaptor.h
index 9ca577d..cae077d 100644
--- a/dbus/service_dbus_adaptor.h
+++ b/dbus/service_dbus_adaptor.h
@@ -21,6 +21,7 @@
#include <dbus_bindings/org.chromium.apmanager.Service.h>
+#include "apmanager/error.h"
#include "apmanager/service_adaptor_interface.h"
namespace apmanager {
@@ -39,7 +40,7 @@
void Start(
std::unique_ptr<brillo::dbus_utils::DBusMethodResponse<>>
response) override;
- bool Stop(brillo::ErrorPtr* error) override;
+ bool Stop(brillo::ErrorPtr* dbus_error) override;
// Implementation of ServiceAdaptorInterface.
RPCObjectIdentifier GetRpcObjectIdentifier() override;
@@ -47,6 +48,10 @@
void SetState(const std::string& state) override;
private:
+ void OnStartCompleted(
+ std::unique_ptr<brillo::dbus_utils::DBusMethodResponse<>> response,
+ const Error& error);
+
org::chromium::apmanager::ServiceAdaptor adaptor_;
dbus::ObjectPath object_path_;
brillo::dbus_utils::DBusObject dbus_object_;
diff --git a/dbus_bindings/org.chromium.apmanager.Manager.dbus-xml b/dbus_bindings/org.chromium.apmanager.Manager.dbus-xml
index 591204f..0934d2d 100644
--- a/dbus_bindings/org.chromium.apmanager.Manager.dbus-xml
+++ b/dbus_bindings/org.chromium.apmanager.Manager.dbus-xml
@@ -12,7 +12,6 @@
Service for managing an access point.
</tp:docstring>
</arg>
- <annotation name="org.chromium.DBus.Method.Kind" value="async"/>
<annotation name="org.chromium.DBus.Method.IncludeDBusMessage"
value="true"/>
</method>
diff --git a/manager.cc b/manager.cc
index 00839e9..c5b1cb4 100644
--- a/manager.cc
+++ b/manager.cc
@@ -70,21 +70,13 @@
device_info_.Stop();
}
-void Manager::CreateService(
- std::unique_ptr<DBusMethodResponse<dbus::ObjectPath>> response,
- dbus::Message* message) {
+bool Manager::CreateService(brillo::ErrorPtr* error,
+ dbus::Message* message,
+ dbus::ObjectPath* out_service) {
LOG(INFO) << "Manager::CreateService";
- scoped_refptr<AsyncEventSequencer> sequencer(new AsyncEventSequencer());
std::unique_ptr<Service> service(new Service(this, service_identifier_));
-
- service->RegisterAsync(
- dbus_object_->GetObjectManager().get(), bus_, sequencer.get());
- sequencer->OnAllTasksCompletedCall({
- base::Bind(&Manager::OnServiceRegistered,
- base::Unretained(this),
- base::Passed(&response),
- base::Passed(&service))
- });
+ *out_service = service->adaptor()->GetRpcObjectIdentifier();
+ services_.push_back(std::move(service));
base::Closure on_connection_vanish = base::Bind(
&Manager::OnAPServiceOwnerDisappeared,
@@ -93,13 +85,15 @@
service_watchers_[service_identifier_].reset(
new DBusServiceWatcher{bus_, message->GetSender(), on_connection_vanish});
service_identifier_++;
+
+ return true;
}
bool Manager::RemoveService(brillo::ErrorPtr* error,
dbus::Message* message,
const dbus::ObjectPath& in_service) {
for (auto it = services_.begin(); it != services_.end(); ++it) {
- if ((*it)->dbus_path() == in_service) {
+ if ((*it)->adaptor()->GetRpcObjectIdentifier() == in_service) {
// Verify the owner.
auto watcher = service_watchers_.find((*it)->identifier());
CHECK(watcher != service_watchers_.end())
@@ -177,32 +171,6 @@
firewall_manager_.ReleaseDHCPPortAccess(interface);
}
-void Manager::OnServiceRegistered(
- std::unique_ptr<DBusMethodResponse<dbus::ObjectPath>> response,
- std::unique_ptr<Service> service,
- bool success) {
- LOG(INFO) << "ServiceRegistered";
- // Success should always be true since we've said that failures are fatal.
- CHECK(success) << "Init of one or more objects has failed.";
-
- // Remove this service if the owner doesn't exist anymore. It is theoretically
- // possible to have the owner disappear before the AP service complete its
- // registration with DBus.
- if (service_watchers_.find(service->identifier()) ==
- service_watchers_.end()) {
- LOG(INFO) << "Service " << service->identifier()
- << ": owner doesn't exist anymore";
- service.reset();
- return;
- }
-
- // Add service to the service list and return the service dbus path for the
- // CreateService call.
- dbus::ObjectPath service_path = service->dbus_path();
- services_.push_back(std::move(service));
- response->Return(service_path);
-}
-
void Manager::OnAPServiceOwnerDisappeared(int service_identifier) {
LOG(INFO) << "Owner for service " << service_identifier << " disappeared";
// Remove service watcher.
diff --git a/manager.h b/manager.h
index 5ae8571..06a9c3b 100644
--- a/manager.h
+++ b/manager.h
@@ -45,11 +45,9 @@
// Implementation of ManagerInterface.
// Handles calls to org.chromium.apmanager.Manager.CreateService().
- // This is an asynchronous call, response is invoked when Service and Config
- // dbus objects complete the DBus service registration.
- virtual void CreateService(
- std::unique_ptr<DBusMethodResponse<dbus::ObjectPath>> response,
- dbus::Message* message);
+ virtual bool CreateService(brillo::ErrorPtr* error,
+ dbus::Message* message,
+ dbus::ObjectPath* out_service);
// Handles calls to org.chromium.apmanager.Manager.RemoveService().
virtual bool RemoveService(brillo::ErrorPtr* error,
dbus::Message* message,
@@ -96,17 +94,6 @@
private:
friend class ManagerTest;
- // A callback that will be called when the Service/Config D-Bus
- // objects/interfaces are exported successfully and ready to be used.
- void OnServiceRegistered(
- std::unique_ptr<DBusMethodResponse<dbus::ObjectPath>> response,
- std::unique_ptr<Service> service,
- bool success);
-
- // A callback that will be called when a Device D-Bus object/interface is
- // exported successfully and ready to be used.
- void OnDeviceRegistered(scoped_refptr<Device> device, bool success);
-
// This is invoked when the owner of an AP service disappeared.
void OnAPServiceOwnerDisappeared(int service_identifier);
diff --git a/mock_service_adaptor.cc b/mock_service_adaptor.cc
new file mode 100644
index 0000000..cce51f4
--- /dev/null
+++ b/mock_service_adaptor.cc
@@ -0,0 +1,24 @@
+//
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#include "apmanager/mock_service_adaptor.h"
+
+namespace apmanager {
+
+MockServiceAdaptor::MockServiceAdaptor() {}
+MockServiceAdaptor::~MockServiceAdaptor() {}
+
+} // namespace apmanager
diff --git a/mock_service_adaptor.h b/mock_service_adaptor.h
new file mode 100644
index 0000000..3a1141a
--- /dev/null
+++ b/mock_service_adaptor.h
@@ -0,0 +1,44 @@
+//
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#ifndef APMANAGER_MOCK_SERVICE_ADAPTOR_H_
+#define APMANAGER_MOCK_SERVICE_ADAPTOR_H_
+
+#include <string>
+
+#include <base/macros.h>
+#include <gmock/gmock.h>
+
+#include "apmanager/service_adaptor_interface.h"
+
+namespace apmanager {
+
+class MockServiceAdaptor : public ServiceAdaptorInterface {
+ public:
+ MockServiceAdaptor();
+ ~MockServiceAdaptor() override;
+
+ MOCK_METHOD0(GetRpcObjectIdentifier, RPCObjectIdentifier());
+ MOCK_METHOD1(SetConfig, void(Config* config));
+ MOCK_METHOD1(SetState, void(const std::string& state));
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MockServiceAdaptor);
+};
+
+} // namespace apmanager
+
+#endif // APMANAGER_MOCK_SERVICE_ADAPTOR_H_
diff --git a/service.cc b/service.cc
index 4b5a026..e973da9 100644
--- a/service.cc
+++ b/service.cc
@@ -18,28 +18,23 @@
#include <signal.h>
+#include <base/bind.h>
#include <base/strings/stringprintf.h>
#include <brillo/errors/error.h>
#if !defined(__ANDROID__)
#include <chromeos/dbus/service_constants.h>
#else
-#include "dbus/apmanager/dbus-constants.h"
+#include <dbus/apmanager/dbus-constants.h>
#endif // __ANDROID__
#if defined(__BRILLO__)
-#include <base/bind.h>
-
#include "apmanager/event_dispatcher.h"
#endif // __BRILLO__
-#include "apmanager/error.h"
+#include "apmanager/control_interface.h"
#include "apmanager/manager.h"
-using brillo::dbus_utils::AsyncEventSequencer;
-using brillo::dbus_utils::DBusMethodResponse;
-using brillo::dbus_utils::ExportedObjectManager;
-using org::chromium::apmanager::ManagerAdaptor;
using std::string;
namespace apmanager {
@@ -73,20 +68,15 @@
const char Service::kStateFailed[] = "Failed";
Service::Service(Manager* manager, int service_identifier)
- : org::chromium::apmanager::ServiceAdaptor(this),
- manager_(manager),
+ : manager_(manager),
identifier_(service_identifier),
- service_path_(
- base::StringPrintf("%s/services/%d",
- ManagerAdaptor::GetObjectPath().value().c_str(),
- service_identifier)),
- dbus_path_(dbus::ObjectPath(service_path_)),
config_(new Config(manager, service_identifier)),
+ adaptor_(manager->control_interface()->CreateServiceAdaptor(this)),
dhcp_server_factory_(DHCPServerFactory::GetInstance()),
file_writer_(FileWriter::GetInstance()),
process_factory_(ProcessFactory::GetInstance()) {
- SetConfig(config_->adaptor()->GetRpcObjectIdentifier());
- SetState(kStateIdle);
+ adaptor_->SetConfig(config_.get());
+ adaptor_->SetState(kStateIdle);
// TODO(zqiu): come up with better server address management. This is good
// enough for now.
config_->SetServerAddressIndex(identifier_ & 0xFF);
@@ -104,25 +94,10 @@
}
}
-void Service::RegisterAsync(ExportedObjectManager* object_manager,
- const scoped_refptr<dbus::Bus>& bus,
- AsyncEventSequencer* sequencer) {
- CHECK(!dbus_object_) << "Already registered";
- dbus_object_.reset(
- new brillo::dbus_utils::DBusObject(
- object_manager,
- bus,
- dbus_path_));
- RegisterWithDBusObject(dbus_object_.get());
- dbus_object_->RegisterAsync(
- sequencer->GetHandler("Service.RegisterAsync() failed.", true));
-}
-
-bool Service::StartInternal(brillo::ErrorPtr* error) {
+bool Service::StartInternal(Error* error) {
if (IsHostapdRunning()) {
- brillo::Error::AddTo(
- error, FROM_HERE, brillo::errors::dbus::kDomain, kServiceError,
- "Service already running");
+ Error::PopulateAndLog(
+ error, Error::kInternalError, "Service already running", FROM_HERE);
return false;
}
@@ -131,10 +106,7 @@
// Generate hostapd configuration content.
string config_str;
- Error internal_error;
- if (!config_->GenerateConfigFile(&internal_error, &config_str)) {
- // TODO(zqiu): temporary until D-Bus is decoupled from this class.
- internal_error.ToDBusError(error);
+ if (!config_->GenerateConfigFile(error, &config_str)) {
return false;
}
@@ -142,25 +114,26 @@
string config_file_name = base::StringPrintf(kHostapdConfigPathFormat,
identifier_);
if (!file_writer_->Write(config_file_name, config_str)) {
- brillo::Error::AddTo(
- error, FROM_HERE, brillo::errors::dbus::kDomain, kServiceError,
- "Failed to write configuration to a file");
+ Error::PopulateAndLog(error,
+ Error::kInternalError,
+ "Failed to write configuration to a file",
+ FROM_HERE);
return false;
}
// Claim the device needed for this ap service.
if (!config_->ClaimDevice()) {
- brillo::Error::AddTo(
- error, FROM_HERE, brillo::errors::dbus::kDomain, kServiceError,
- "Failed to claim the device for this service");
+ Error::PopulateAndLog(error,
+ Error::kInternalError,
+ "Failed to claim the device for this service",
+ FROM_HERE);
return false;
}
// Start hostapd process.
if (!StartHostapdProcess(config_file_name)) {
- brillo::Error::AddTo(
- error, FROM_HERE, brillo::errors::dbus::kDomain, kServiceError,
- "Failed to start hostapd");
+ Error::PopulateAndLog(
+ error, Error::kInternalError, "Failed to start hostapd", FROM_HERE);
// Release the device claimed for this service.
config_->ReleaseDevice();
return false;
@@ -172,9 +145,10 @@
dhcp_server_factory_->CreateDHCPServer(config_->GetServerAddressIndex(),
config_->selected_interface()));
if (!dhcp_server_->Start()) {
- brillo::Error::AddTo(
- error, FROM_HERE, brillo::errors::dbus::kDomain, kServiceError,
- "Failed to start DHCP server");
+ Error::PopulateAndLog(error,
+ Error::kInternalError,
+ "Failed to start DHCP server",
+ FROM_HERE);
ReleaseResources();
return false;
}
@@ -192,35 +166,32 @@
hostapd_monitor_->Start();
// Update service state.
- SetState(kStateStarting);
+ adaptor_->SetState(kStateStarting);
return true;
}
-void Service::Start(std::unique_ptr<DBusMethodResponse<>> response) {
- brillo::ErrorPtr error;
+void Service::Start(const base::Callback<void(const Error&)>& result_callback) {
+ Error error;
#if !defined(__BRILLO__)
- if (!StartInternal(&error)) {
- response->ReplyWithError(error.get());
- } else {
- response->Return();
- }
+ StartInternal(&error);
+ result_callback.Run(error);
#else
if (start_in_progress_) {
- brillo::Error::AddTo(
- &error, FROM_HERE, brillo::errors::dbus::kDomain, kServiceError,
- "Start already in progress");
- response->ReplyWithError(error.get());
+ Error::PopulateAndLog(
+ &error, Error::kInternalError, "Start already in progress", FROM_HERE);
+ result_callback.Run(error);
return;
}
string interface_name;
if (!manager_->SetupApModeInterface(&interface_name)) {
- brillo::Error::AddTo(
- &error, FROM_HERE, brillo::errors::dbus::kDomain, kServiceError,
- "Failed to setup AP mode interface");
- response->ReplyWithError(error.get());
+ Error::PopulateAndLog(&error,
+ Error::kInternalError,
+ "Failed to setup AP mode interface",
+ FROM_HERE);
+ result_callback.Run(error);
return;
}
@@ -229,21 +200,21 @@
weak_factory_.GetWeakPtr(),
interface_name,
0, // Initial check count.
- base::Passed(&response)),
+ result_callback),
kAPInterfaceCheckIntervalMilliseconds);
#endif
}
-bool Service::Stop(brillo::ErrorPtr* error) {
+bool Service::Stop(Error* error) {
if (!IsHostapdRunning()) {
- brillo::Error::AddTo(
- error, FROM_HERE, brillo::errors::dbus::kDomain, kServiceError,
- "Service is not currently running");
+ Error::PopulateAndLog(error,
+ Error::kInternalError,
+ "Service is not currently running", FROM_HERE);
return false;
}
ReleaseResources();
- SetState(kStateIdle);
+ adaptor_->SetState(kStateIdle);
return true;
}
@@ -260,8 +231,8 @@
void Service::APInterfaceCheckTask(
const string& interface_name,
int check_count,
- std::unique_ptr<DBusMethodResponse<>> response) {
- brillo::ErrorPtr error;
+ const base::Callback<void(const Error&)>& result_callback) {
+ Error error;
// Check if the AP interface is enumerated.
if (manager_->GetDeviceFromInterfaceName(interface_name)) {
@@ -269,20 +240,19 @@
config_->SetInterfaceName(interface_name);
if (!StartInternal(&error)) {
HandleStartFailure();
- response->ReplyWithError(error.get());
- } else {
- response->Return();
}
+ result_callback.Run(error);
return;
}
check_count++;
if (check_count >= kAPInterfaceCheckMaxAttempts) {
- brillo::Error::AddTo(
- &error, FROM_HERE, brillo::errors::dbus::kDomain, kServiceError,
- "Timeout waiting for AP interface to be enumerated");
+ Error::PopulateAndLog(&error,
+ Error::kInternalError,
+ "Timeout waiting for AP interface to be enumerated",
+ FROM_HERE);
HandleStartFailure();
- response->ReplyWithError(error.get());
+ result_callback.Run(error);
return;
}
@@ -291,7 +261,7 @@
weak_factory_.GetWeakPtr(),
interface_name,
check_count,
- base::Passed(&response)),
+ result_callback),
kAPInterfaceCheckIntervalMilliseconds);
}
#endif // __BRILLO__
@@ -339,10 +309,10 @@
const std::string& data) {
switch (event) {
case HostapdMonitor::kHostapdFailed:
- SetState(kStateFailed);
+ adaptor_->SetState(kStateFailed);
break;
case HostapdMonitor::kHostapdStarted:
- SetState(kStateStarted);
+ adaptor_->SetState(kStateStarted);
break;
case HostapdMonitor::kStationConnected:
LOG(INFO) << "Station connected: " << data;
diff --git a/service.h b/service.h
index 1459229..80dccea 100644
--- a/service.h
+++ b/service.h
@@ -19,16 +19,18 @@
#include <string>
+#include <base/callback.h>
#include <base/macros.h>
#include <base/memory/weak_ptr.h>
#include <brillo/process.h>
#include "apmanager/config.h"
#include "apmanager/dhcp_server_factory.h"
+#include "apmanager/error.h"
#include "apmanager/file_writer.h"
#include "apmanager/hostapd_monitor.h"
#include "apmanager/process_factory.h"
-#include "dbus_bindings/org.chromium.apmanager.Service.h"
+#include "apmanager/service_adaptor_interface.h"
namespace apmanager {
@@ -38,27 +40,20 @@
class EventDispatcher;
#endif // __BRILLO__
-class Service : public org::chromium::apmanager::ServiceAdaptor,
- public org::chromium::apmanager::ServiceInterface {
+class Service {
public:
Service(Manager* manager, int service_identifier);
virtual ~Service();
- // Implementation of org::chromium::apmanager::ServiceInterface.
- void Start(
- std::unique_ptr<brillo::dbus_utils::DBusMethodResponse<>> response);
- bool Stop(brillo::ErrorPtr* error);
-
- // Register Service DBus object.
- void RegisterAsync(
- brillo::dbus_utils::ExportedObjectManager* object_manager,
- const scoped_refptr<dbus::Bus>& bus,
- brillo::dbus_utils::AsyncEventSequencer* sequencer);
-
- const dbus::ObjectPath& dbus_path() const { return dbus_path_; }
+ void Start(const base::Callback<void(const Error&)>& result_callback);
+ bool Stop(Error* error);
int identifier() const { return identifier_; }
+ ServiceAdaptorInterface* adaptor() const { return adaptor_.get(); }
+
+ Config* config() const { return config_.get(); }
+
private:
friend class ServiceTest;
@@ -80,13 +75,13 @@
void APInterfaceCheckTask(
const std::string& interface_name,
int check_count,
- std::unique_ptr<brillo::dbus_utils::DBusMethodResponse<>> response);
+ const base::Callback<void(const Error&)>& result_callback);
// Handle asynchronous service start failures.
void HandleStartFailure();
#endif // __BRILLO__
- bool StartInternal(brillo::ErrorPtr* error);
+ bool StartInternal(Error* error);
// Return true if hostapd process is currently running.
bool IsHostapdRunning();
@@ -107,10 +102,8 @@
Manager* manager_;
int identifier_;
- std::string service_path_;
- dbus::ObjectPath dbus_path_;
std::unique_ptr<Config> config_;
- std::unique_ptr<brillo::dbus_utils::DBusObject> dbus_object_;
+ std::unique_ptr<ServiceAdaptorInterface> adaptor_;
std::unique_ptr<brillo::Process> hostapd_process_;
std::unique_ptr<DHCPServer> dhcp_server_;
DHCPServerFactory* dhcp_server_factory_;
diff --git a/service_unittest.cc b/service_unittest.cc
index 2f571ab..a23e06c 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -27,7 +27,7 @@
#if !defined(__ANDROID__)
#include <chromeos/dbus/service_constants.h>
#else
-#include "dbus/apmanager/dbus-constants.h"
+#include <dbus/apmanager/dbus-constants.h>
#endif // __ANDROID__
#include "apmanager/error.h"
@@ -40,6 +40,7 @@
#include "apmanager/mock_hostapd_monitor.h"
#include "apmanager/mock_manager.h"
#include "apmanager/mock_process_factory.h"
+#include "apmanager/mock_service_adaptor.h"
using brillo::ProcessMock;
using ::testing::_;
@@ -69,6 +70,8 @@
ServiceTest()
: manager_(&control_interface_),
hostapd_monitor_(new MockHostapdMonitor()) {
+ ON_CALL(control_interface_, CreateServiceAdaptorRaw())
+ .WillByDefault(ReturnNew<MockServiceAdaptor>());
ON_CALL(control_interface_, CreateConfigAdaptorRaw())
.WillByDefault(ReturnNew<FakeConfigAdaptor>());
// Defer creation of Service object to allow ControlInterface to
@@ -82,7 +85,7 @@
service_->hostapd_monitor_.reset(hostapd_monitor_);
}
- bool StartService(brillo::ErrorPtr* error) {
+ bool StartService(Error* error) {
return service_->StartInternal(error);
}
@@ -98,6 +101,14 @@
service_->config_.reset(config);
}
+ void VerifyError(const Error& error,
+ Error::Type expected_type,
+ const std::string& expected_message_start) {
+ EXPECT_EQ(expected_type, error.type());
+ EXPECT_TRUE(
+ base::StartsWithASCII(error.message(), expected_message_start, false));
+ }
+
protected:
MockControl control_interface_;
MockManager manager_;
@@ -108,26 +119,19 @@
std::unique_ptr<Service> service_;
};
-MATCHER_P(IsServiceErrorStartingWith, message, "") {
- return arg != nullptr &&
- arg->GetDomain() == brillo::errors::dbus::kDomain &&
- arg->GetCode() == kServiceError &&
- base::EndsWith(arg->GetMessage(), message, false);
-}
-
TEST_F(ServiceTest, StartWhenServiceAlreadyRunning) {
StartDummyProcess();
- brillo::ErrorPtr error;
+ Error error;
EXPECT_FALSE(StartService(&error));
- EXPECT_THAT(error, IsServiceErrorStartingWith("Service already running"));
+ VerifyError(error, Error::kInternalError, "Service already running");
}
TEST_F(ServiceTest, StartWhenConfigFileFailed) {
MockConfig* config = new MockConfig(&manager_);
SetConfig(config);
- brillo::ErrorPtr error;
+ Error error;
EXPECT_CALL(*config, GenerateConfigFile(_, _)).WillOnce(Return(false));
EXPECT_FALSE(StartService(&error));
}
@@ -142,7 +146,7 @@
ProcessMock* process = new ProcessMock();
std::string config_str(kHostapdConfig);
- brillo::ErrorPtr error;
+ Error error;
EXPECT_CALL(*config, GenerateConfigFile(_, _)).WillOnce(
DoAll(SetArgPointee<1>(config_str), Return(true)));
EXPECT_CALL(file_writer_, Write(kHostapdConfigFilePath, kHostapdConfig))
@@ -156,14 +160,14 @@
EXPECT_CALL(manager_, RequestDHCPPortAccess(_));
EXPECT_CALL(*hostapd_monitor_, Start());
EXPECT_TRUE(StartService(&error));
- EXPECT_EQ(nullptr, error);
+ EXPECT_TRUE(error.IsSuccess());
}
TEST_F(ServiceTest, StopWhenServiceNotRunning) {
- brillo::ErrorPtr error;
+ Error error;
EXPECT_FALSE(service_->Stop(&error));
- EXPECT_THAT(error, IsServiceErrorStartingWith(
- "Service is not currently running"));
+ VerifyError(
+ error, Error::kInternalError, "Service is not currently running");
}
TEST_F(ServiceTest, StopSuccess) {
@@ -171,7 +175,7 @@
MockConfig* config = new MockConfig(&manager_);
SetConfig(config);
- brillo::ErrorPtr error;
+ Error error;
EXPECT_CALL(*config, ReleaseDevice()).Times(1);
EXPECT_TRUE(service_->Stop(&error));
Mock::VerifyAndClearExpectations(config);