shill: EthernetService: Fix auto-connect-by-default

Fix a nasty typo which prevented EthernetService from overriding
the parent class IsAutoConnectByDefault.  Add unit tests.  Add
belt-and-suspenders by also overriding the SetAutoConnect RPC
settter to prevent AutoConnect from being set over the control
interface.

BUG=chromium-os:32709
TEST=Unit tests

Change-Id: I683333b915cb747ccb241512f66d2fb3f1efca3a
Reviewed-on: https://gerrit.chromium.org/gerrit/30557
Commit-Ready: 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 b0d559b..fe6597f 100644
--- a/Makefile
+++ b/Makefile
@@ -258,6 +258,7 @@
 	dhcp_provider_unittest.o \
 	dns_client_unittest.o \
 	error_unittest.o \
+	ethernet_service_unittest.o \
 	hook_table_unittest.o \
 	http_proxy_unittest.o \
 	http_request_unittest.o \
@@ -289,6 +290,7 @@
 	mock_dhcp_provider.o \
 	mock_dhcp_proxy.o \
 	mock_dns_client.o \
+	mock_ethernet.o \
 	mock_event_dispatcher.o \
 	mock_glib.o \
 	mock_http_request.o \
diff --git a/ethernet_service.cc b/ethernet_service.cc
index cc163c8..837bde8 100644
--- a/ethernet_service.cc
+++ b/ethernet_service.cc
@@ -54,4 +54,14 @@
                             kServiceType, ethernet_->address().c_str());
 }
 
+void EthernetService::SetAutoConnect(const bool &connect, Error *error) {
+  if (!connect) {
+    Error::PopulateAndLog(
+        error, Error::kInvalidArguments,
+        "Auto-connect on Ethernet services must not be disabled.");
+    return;
+  }
+  Service::SetAutoConnect(connect, error);
+}
+
 }  // namespace shill
diff --git a/ethernet_service.h b/ethernet_service.h
index 599b1cb..82c156d 100644
--- a/ethernet_service.h
+++ b/ethernet_service.h
@@ -29,7 +29,8 @@
 
   // ethernet_<MAC>
   virtual std::string GetStorageIdentifier() const;
-  virtual bool IsAutoDetectByDefault() const { return true; }
+  virtual bool IsAutoConnectByDefault() const { return true; }
+  virtual void SetAutoConnect(const bool &connect, Error *error);
 
  private:
   static const char kServiceType[];
diff --git a/ethernet_service_unittest.cc b/ethernet_service_unittest.cc
new file mode 100644
index 0000000..ba85d7d
--- /dev/null
+++ b/ethernet_service_unittest.cc
@@ -0,0 +1,72 @@
+// 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/ethernet_service.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "shill/mock_ethernet.h"
+#include "shill/property_store_unittest.h"
+#include "shill/refptr_types.h"
+
+using ::testing::NiceMock;
+
+namespace shill {
+
+class EthernetServiceTest : public PropertyStoreTest {
+ public:
+  EthernetServiceTest()
+      : ethernet_(
+            new NiceMock<MockEthernet>(control_interface(),
+                                       dispatcher(),
+                                       metrics(),
+                                       manager(),
+                                       "ethernet",
+                                       fake_mac,
+                                       0)),
+        service_(
+            new EthernetService(control_interface(),
+                                dispatcher(),
+                                metrics(),
+                                manager(),
+                                ethernet_)) {}
+  virtual ~EthernetServiceTest() {}
+
+ protected:
+  static const char fake_mac[];
+
+  bool GetAutoConnect() {
+    return service_->GetAutoConnect(NULL);
+  }
+
+  void SetAutoConnect(const bool connect, Error *error) {
+    return service_->SetAutoConnect(connect, error);
+  }
+
+  scoped_refptr<MockEthernet> ethernet_;
+  EthernetServiceRefPtr service_;
+};
+
+// static
+const char EthernetServiceTest::fake_mac[] = "AaBBcCDDeeFF";
+
+TEST_F(EthernetServiceTest, AutoConnect) {
+  EXPECT_TRUE(service_->IsAutoConnectByDefault());
+  EXPECT_TRUE(GetAutoConnect());
+  {
+    Error error;
+    SetAutoConnect(false, &error);
+    EXPECT_FALSE(error.IsSuccess());
+  }
+  EXPECT_TRUE(GetAutoConnect());
+  {
+    Error error;
+    SetAutoConnect(true, &error);
+    EXPECT_TRUE(error.IsSuccess());
+  }
+  EXPECT_TRUE(GetAutoConnect());
+}
+
+}  // namespace shill
diff --git a/mock_ethernet.cc b/mock_ethernet.cc
new file mode 100644
index 0000000..1a02672
--- /dev/null
+++ b/mock_ethernet.cc
@@ -0,0 +1,30 @@
+// 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/mock_ethernet.h"
+
+#include <string>
+
+namespace shill {
+
+using std::string;
+
+MockEthernet::MockEthernet(ControlInterface *control_interface,
+                           EventDispatcher *dispatcher,
+                           Metrics *metrics,
+                           Manager *manager,
+                           const string &link_name,
+                           const string &address,
+                           int interface_index)
+    : Ethernet(control_interface,
+               dispatcher,
+               metrics,
+               manager,
+               link_name,
+               address,
+               interface_index) {}
+
+MockEthernet::~MockEthernet() {}
+
+}  // namespace shill
diff --git a/mock_ethernet.h b/mock_ethernet.h
new file mode 100644
index 0000000..75ff789
--- /dev/null
+++ b/mock_ethernet.h
@@ -0,0 +1,43 @@
+// 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.
+
+#ifndef SHILL_MOCK_ETHERNET_H_
+#define SHILL_MOCK_ETHERNET_H_
+
+#include <string>
+
+#include <gmock/gmock.h>
+
+#include "shill/refptr_types.h"
+#include "shill/ethernet.h"
+
+namespace shill {
+
+class ControlInterface;
+class Error;
+class EventDispatcher;
+
+class MockEthernet : public Ethernet {
+ public:
+  MockEthernet(ControlInterface *control_interface,
+               EventDispatcher *dispatcher,
+               Metrics *metrics,
+               Manager *manager,
+               const std::string &link_name,
+               const std::string &address,
+               int interface_index);
+  virtual ~MockEthernet();
+
+  MOCK_METHOD2(Start, void(Error *error,
+                           const EnabledStateChangedCallback &callback));
+  MOCK_METHOD2(Stop, void(Error *error,
+                          const EnabledStateChangedCallback &callback));
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockEthernet);
+};
+
+}  // namespace shill
+
+#endif  // SHILL_MOCK_ETHERNET_H_
diff --git a/service.h b/service.h
index 4a808d0..fc09ee1 100644
--- a/service.h
+++ b/service.h
@@ -472,6 +472,9 @@
   // failure_ enum.
   void UpdateErrorProperty();
 
+  // RPC setter for the the "AutoConnect" property.
+  virtual void SetAutoConnect(const bool &connect, Error *error);
+
   // Property accessors reserved for subclasses
   EventDispatcher *dispatcher() const { return dispatcher_; }
   const std::string &GetEAPKeyManagement() const;
@@ -484,6 +487,7 @@
   void set_favorite(bool favorite) { favorite_ = favorite; }
 
  private:
+  friend class EthernetServiceTest;
   friend class MetricsTest;
   friend class ServiceAdaptorInterface;
   friend class VPNServiceTest;
@@ -532,7 +536,6 @@
   static const char kServiceSortUniqueName[];
 
   bool GetAutoConnect(Error *error);
-  void SetAutoConnect(const bool &connect, Error *error);
 
   std::string GetCheckPortal(Error *error);
   void SetCheckPortal(const std::string &check_portal, Error *error);