update_engine: Move ConnectionManager to an abstract Interface.
MockConnectionManager required to use one of ConnectionManager
constructors passing pointers that won't be use by the mock. This
patch moves the interface to its own ConnectionManagerInterface class.
BUG=None
TEST=unittests still pass.
Change-Id: I9ed09daf8e4256304be7dab30cfbe751901dc24b
Reviewed-on: https://chromium-review.googlesource.com/290120
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
Trybot-Ready: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
diff --git a/connection_manager.cc b/connection_manager.cc
index 62bb11e..45b6467 100644
--- a/connection_manager.cc
+++ b/connection_manager.cc
@@ -253,8 +253,9 @@
}
}
+// static
const char* ConnectionManager::StringForConnectionType(
- NetworkConnectionType type) const {
+ NetworkConnectionType type) {
switch (type) {
case NetworkConnectionType::kEthernet:
return shill::kTypeEthernet;
@@ -272,8 +273,8 @@
return "Unknown";
}
-const char* ConnectionManager::StringForTethering(
- NetworkTethering tethering) const {
+// static
+const char* ConnectionManager::StringForTethering(NetworkTethering tethering) {
switch (tethering) {
case NetworkTethering::kNotDetected:
return shill::kTetheringNotDetectedState;
diff --git a/connection_manager.h b/connection_manager.h
index 687759e..551c4f3 100644
--- a/connection_manager.h
+++ b/connection_manager.h
@@ -7,59 +7,36 @@
#include <base/macros.h>
+#include "update_engine/connection_manager_interface.h"
#include "update_engine/dbus_wrapper_interface.h"
namespace chromeos_update_engine {
-enum class NetworkConnectionType {
- kEthernet = 0,
- kWifi,
- kWimax,
- kBluetooth,
- kCellular,
- kUnknown
-};
-
-enum class NetworkTethering {
- kNotDetected = 0,
- kSuspected,
- kConfirmed,
- kUnknown
-};
-
class SystemState;
-// This class exposes a generic interface to the connection manager
-// (e.g FlimFlam, Shill, etc.) to consolidate all connection-related
-// logic in update_engine.
-class ConnectionManager {
+// This class implements the concrete class that talks with the connection
+// manager (shill) over DBus.
+class ConnectionManager : public ConnectionManagerInterface {
public:
- // Constructs a new ConnectionManager object initialized with the
- // given system state.
- explicit ConnectionManager(SystemState* system_state);
- virtual ~ConnectionManager() = default;
-
- // Populates |out_type| with the type of the network connection
- // that we are currently connected and |out_tethering| with the estimate of
- // whether that network is being tethered. The dbus_iface is used to query
- // the real connection manager (e.g shill).
- virtual bool GetConnectionProperties(DBusWrapperInterface* dbus_iface,
- NetworkConnectionType* out_type,
- NetworkTethering* out_tethering) const;
-
- // Returns true if we're allowed to update the system when we're
- // connected to the internet through the given network connection type and the
- // given tethering state.
- virtual bool IsUpdateAllowedOver(NetworkConnectionType type,
- NetworkTethering tethering) const;
-
// Returns the string representation corresponding to the given
// connection type.
- virtual const char* StringForConnectionType(NetworkConnectionType type) const;
+ static const char* StringForConnectionType(NetworkConnectionType type);
// Returns the string representation corresponding to the given tethering
// state.
- virtual const char* StringForTethering(NetworkTethering tethering) const;
+ static const char* StringForTethering(NetworkTethering tethering);
+
+ // Constructs a new ConnectionManager object initialized with the
+ // given system state.
+ explicit ConnectionManager(SystemState* system_state);
+ ~ConnectionManager() override = default;
+
+ // ConnectionManagerInterface overrides
+ bool GetConnectionProperties(DBusWrapperInterface* dbus_iface,
+ NetworkConnectionType* out_type,
+ NetworkTethering* out_tethering) const override;
+ bool IsUpdateAllowedOver(NetworkConnectionType type,
+ NetworkTethering tethering) const override;
private:
// The global context for update_engine
diff --git a/connection_manager_interface.h b/connection_manager_interface.h
new file mode 100644
index 0000000..aa04946
--- /dev/null
+++ b/connection_manager_interface.h
@@ -0,0 +1,60 @@
+// Copyright 2015 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.
+
+#ifndef UPDATE_ENGINE_CONNECTION_MANAGER_INTERFACE_H_
+#define UPDATE_ENGINE_CONNECTION_MANAGER_INTERFACE_H_
+
+#include <base/macros.h>
+
+#include "update_engine/dbus_wrapper_interface.h"
+
+namespace chromeos_update_engine {
+
+enum class NetworkConnectionType {
+ kEthernet,
+ kWifi,
+ kWimax,
+ kBluetooth,
+ kCellular,
+ kUnknown
+};
+
+enum class NetworkTethering {
+ kNotDetected,
+ kSuspected,
+ kConfirmed,
+ kUnknown
+};
+
+// This class exposes a generic interface to the connection manager
+// (e.g FlimFlam, Shill, etc.) to consolidate all connection-related
+// logic in update_engine.
+class ConnectionManagerInterface {
+ public:
+ virtual ~ConnectionManagerInterface() = default;
+
+ // Populates |out_type| with the type of the network connection
+ // that we are currently connected and |out_tethering| with the estimate of
+ // whether that network is being tethered.
+ virtual bool GetConnectionProperties(
+ DBusWrapperInterface* dbus_iface,
+ NetworkConnectionType* out_type,
+ NetworkTethering* out_tethering) const = 0;
+
+ // Returns true if we're allowed to update the system when we're
+ // connected to the internet through the given network connection type and the
+ // given tethering state.
+ virtual bool IsUpdateAllowedOver(NetworkConnectionType type,
+ NetworkTethering tethering) const = 0;
+
+ protected:
+ ConnectionManagerInterface() = default;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(ConnectionManagerInterface);
+};
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_CONNECTION_MANAGER_INTERFACE_H_
diff --git a/dbus_service.cc b/dbus_service.cc
index 0dbff6b..d77ea31 100644
--- a/dbus_service.cc
+++ b/dbus_service.cc
@@ -13,7 +13,7 @@
#include <policy/device_policy.h>
#include "update_engine/clock_interface.h"
-#include "update_engine/connection_manager.h"
+#include "update_engine/connection_manager_interface.h"
#include "update_engine/dbus_constants.h"
#include "update_engine/hardware_interface.h"
#include "update_engine/omaha_request_params.h"
@@ -408,7 +408,7 @@
UpdateEngineService* self,
gboolean* allowed,
GError **error) {
- chromeos_update_engine::ConnectionManager* cm =
+ chromeos_update_engine::ConnectionManagerInterface* cm =
self->system_state_->connection_manager();
// The device_policy is loaded in a lazy way before an update check and is
diff --git a/fake_system_state.cc b/fake_system_state.cc
index 8747a59..c10dca1 100644
--- a/fake_system_state.cc
+++ b/fake_system_state.cc
@@ -9,8 +9,7 @@
// Mock the SystemStateInterface so that we could lie that
// OOBE is completed even when there's no such marker file, etc.
FakeSystemState::FakeSystemState()
- : mock_connection_manager_(this),
- mock_update_attempter_(this, &dbus_),
+ : mock_update_attempter_(this, &dbus_),
mock_request_params_(this),
fake_update_manager_(&fake_clock_),
clock_(&fake_clock_),
diff --git a/fake_system_state.h b/fake_system_state.h
index 84c37fc..1f3f58f 100644
--- a/fake_system_state.h
+++ b/fake_system_state.h
@@ -45,7 +45,7 @@
return device_policy_;
}
- inline ConnectionManager* connection_manager() override {
+ inline ConnectionManagerInterface* connection_manager() override {
return connection_manager_;
}
@@ -89,7 +89,8 @@
clock_ = clock ? clock : &fake_clock_;
}
- inline void set_connection_manager(ConnectionManager* connection_manager) {
+ inline void set_connection_manager(
+ ConnectionManagerInterface* connection_manager) {
connection_manager_ = (connection_manager ? connection_manager :
&mock_connection_manager_);
}
@@ -215,7 +216,7 @@
// Pointers to objects that client code can override. They are initialized to
// the default implementations above.
ClockInterface* clock_;
- ConnectionManager* connection_manager_;
+ ConnectionManagerInterface* connection_manager_;
HardwareInterface* hardware_;
MetricsLibraryInterface* metrics_lib_;
PrefsInterface* prefs_;
diff --git a/mock_connection_manager.h b/mock_connection_manager.h
index 9530865..609b51d 100644
--- a/mock_connection_manager.h
+++ b/mock_connection_manager.h
@@ -7,17 +7,16 @@
#include <gmock/gmock.h>
-#include "update_engine/connection_manager.h"
+#include "update_engine/connection_manager_interface.h"
namespace chromeos_update_engine {
// This class mocks the generic interface to the connection manager
// (e.g FlimFlam, Shill, etc.) to consolidate all connection-related
// logic in update_engine.
-class MockConnectionManager : public ConnectionManager {
+class MockConnectionManager : public ConnectionManagerInterface {
public:
- explicit MockConnectionManager(SystemState* system_state)
- : ConnectionManager(system_state) {}
+ MockConnectionManager() = default;
MOCK_CONST_METHOD3(GetConnectionProperties,
bool(DBusWrapperInterface* dbus_iface,
@@ -26,12 +25,6 @@
MOCK_CONST_METHOD2(IsUpdateAllowedOver, bool(NetworkConnectionType type,
NetworkTethering tethering));
-
- MOCK_CONST_METHOD1(StringForConnectionType,
- const char*(NetworkConnectionType type));
-
- MOCK_CONST_METHOD1(StringForTethering,
- const char*(NetworkTethering tethering));
};
} // namespace chromeos_update_engine
diff --git a/mock_http_fetcher.h b/mock_http_fetcher.h
index 89f5d21..36b6a1d 100644
--- a/mock_http_fetcher.h
+++ b/mock_http_fetcher.h
@@ -40,8 +40,7 @@
timeout_id_(chromeos::MessageLoop::kTaskIdNull),
paused_(false),
fail_transfer_(false),
- never_use_(false),
- mock_connection_manager_(&fake_system_state_) {
+ never_use_(false) {
fake_system_state_.set_connection_manager(&mock_connection_manager_);
data_.insert(data_.end(), data, data + size);
}
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index dff8ad3..2f361e5 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -21,6 +21,7 @@
#include <expat.h>
#include "update_engine/action_pipe.h"
+#include "update_engine/connection_manager.h"
#include "update_engine/constants.h"
#include "update_engine/hardware_interface.h"
#include "update_engine/omaha_hash_calculator.h"
@@ -1448,7 +1449,8 @@
NetworkConnectionType type;
NetworkTethering tethering;
RealDBusWrapper dbus_iface;
- ConnectionManager* connection_manager = system_state_->connection_manager();
+ ConnectionManagerInterface* connection_manager =
+ system_state_->connection_manager();
if (!connection_manager->GetConnectionProperties(&dbus_iface,
&type, &tethering)) {
LOG(INFO) << "We could not determine our connection type. "
@@ -1457,7 +1459,7 @@
}
bool is_allowed = connection_manager->IsUpdateAllowedOver(type, tethering);
LOG(INFO) << "We are connected via "
- << connection_manager->StringForConnectionType(type)
+ << ConnectionManager::StringForConnectionType(type)
<< ", Updates allowed: " << (is_allowed ? "Yes" : "No");
return is_allowed;
}
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 8f4dc87..5b8fba9 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -445,7 +445,7 @@
OmahaResponse response;
// Set up a connection manager that doesn't allow a valid update over
// the current ethernet connection.
- MockConnectionManager mock_cm(nullptr);
+ MockConnectionManager mock_cm;
fake_system_state_.set_connection_manager(&mock_cm);
EXPECT_CALL(mock_cm, GetConnectionProperties(_, _, _)).WillRepeatedly(
@@ -454,9 +454,6 @@
Return(true)));
EXPECT_CALL(mock_cm, IsUpdateAllowedOver(NetworkConnectionType::kEthernet, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(mock_cm,
- StringForConnectionType(NetworkConnectionType::kEthernet))
- .WillRepeatedly(Return(shill::kTypeEthernet));
ASSERT_FALSE(
TestUpdateCheck(nullptr, // request_params
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 67639cc..6c7c166 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -10,7 +10,7 @@
#include <base/strings/string_util.h>
#include <policy/device_policy.h>
-#include "update_engine/connection_manager.h"
+#include "update_engine/connection_manager_interface.h"
#include "update_engine/constants.h"
#include "update_engine/delta_performer.h"
#include "update_engine/hardware_interface.h"
diff --git a/payload_state.cc b/payload_state.cc
index ed7b775..a15d68b 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -10,7 +10,6 @@
#include <base/logging.h>
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
-#include <base/format_macros.h>
#include <policy/device_policy.h>
#include "update_engine/clock.h"
@@ -176,7 +175,8 @@
NetworkConnectionType network_connection_type;
NetworkTethering tethering;
RealDBusWrapper dbus_iface;
- ConnectionManager* connection_manager = system_state_->connection_manager();
+ ConnectionManagerInterface* connection_manager =
+ system_state_->connection_manager();
if (!connection_manager->GetConnectionProperties(&dbus_iface,
&network_connection_type,
&tethering)) {
diff --git a/real_system_state.h b/real_system_state.h
index 7e0c16d..4e97b95 100644
--- a/real_system_state.h
+++ b/real_system_state.h
@@ -47,7 +47,7 @@
inline ClockInterface* clock() override { return &clock_; }
- inline ConnectionManager* connection_manager() override {
+ inline ConnectionManagerInterface* connection_manager() override {
return &connection_manager_;
}
diff --git a/system_state.h b/system_state.h
index 21aeabd..65d4e48 100644
--- a/system_state.h
+++ b/system_state.h
@@ -25,7 +25,7 @@
// any circular references in header file inclusion. Hence forward-declaring
// the required classes.
class ClockInterface;
-class ConnectionManager;
+class ConnectionManagerInterface;
class GpioHandler;
class HardwareInterface;
class OmahaRequestParams;
@@ -54,7 +54,7 @@
virtual ClockInterface* clock() = 0;
// Gets the connection manager object.
- virtual ConnectionManager* connection_manager() = 0;
+ virtual ConnectionManagerInterface* connection_manager() = 0;
// Gets the hardware interface object.
virtual HardwareInterface* hardware() = 0;
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 8c6fa3d..07ba451 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -100,7 +100,6 @@
protected:
UpdateAttempterTest()
: attempter_(&fake_system_state_, &dbus_),
- mock_connection_manager(&fake_system_state_),
fake_dbus_system_bus_(reinterpret_cast<DBusGConnection*>(1)),
fake_dbus_debugd_proxy_(reinterpret_cast<DBusGProxy*>(2)) {
// Override system state members.
diff --git a/utils.h b/utils.h
index b661b43..73d6eb9 100644
--- a/utils.h
+++ b/utils.h
@@ -18,13 +18,13 @@
#include <base/files/file_path.h>
#include <base/posix/eintr_wrapper.h>
#include <base/time/time.h>
-#include <chromeos/secure_blob.h>
#include <chromeos/key_value_store.h>
+#include <chromeos/secure_blob.h>
#include "metrics/metrics_library.h"
#include "update_engine/action.h"
#include "update_engine/action_processor.h"
-#include "update_engine/connection_manager.h"
+#include "update_engine/connection_manager_interface.h"
#include "update_engine/constants.h"
#include "update_engine/file_descriptor.h"
#include "update_engine/metrics.h"