Adding a NetworkProfileHandler used by ManagedNetworkConfigurationHandler.

chromeos/network/managed_network_configuration_handler_unittest.cc

 // Copyright (c) 2013 The Chromium 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 "chromeos/network/managed_network_configuration_handler.h"
 
 #include <iostream>
 #include <sstream>
 
 #include "base/message_loop.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/mock_dbus_thread_manager.h"
 #include "chromeos/dbus/mock_shill_manager_client.h"
 #include "chromeos/dbus/mock_shill_profile_client.h"
 #include "chromeos/dbus/mock_shill_service_client.h"
 #include "chromeos/dbus/shill_profile_client_stub.h"
 #include "chromeos/network/network_configuration_handler.h"
+#include "chromeos/network/network_profile_handler_stub.h"
 #include "chromeos/network/onc/onc_test_utils.h"
 #include "chromeos/network/onc/onc_utils.h"
 #include "dbus/object_path.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 using ::testing::Invoke;
 using ::testing::Mock;
 using ::testing::Pointee;
 using ::testing::Return;
 using ::testing::SaveArg;
 using ::testing::StrEq;
 using ::testing::StrictMock;
 using ::testing::_;
 
 namespace test_utils = ::chromeos::onc::test_utils;
 
 namespace {
 
 std::string ValueToString(const base::Value* value) {
   std::stringstream str;
   str << *value;
   return str.str();
 }
 
+const char kSharedProfilePath[] = "/profile/default";
+const char kUser1[] = "user1";
+const char kUser1ProfilePath[] = "/profile/user1/shill";
+
 // Matcher to match base::Value.
 MATCHER_P(IsEqualTo,
           value,
           std::string(negation ? "isn't" : "is") + " equal to " +
           ValueToString(value)) {
   return value->Equals(&arg);
 }
 
 }  // namespace
 
 
 namespace chromeos {
 
 class ManagedNetworkConfigurationHandlerTest : public testing::Test {
  public:
-  ManagedNetworkConfigurationHandlerTest() {
+  ManagedNetworkConfigurationHandlerTest()
+      : stub_profile_client_(NULL /* no ShillManagerClient::TestInterface */ ) {
   }
 
   virtual ~ManagedNetworkConfigurationHandlerTest() {
   }
 
   virtual void SetUp() OVERRIDE {
     MockDBusThreadManager* dbus_thread_manager = new MockDBusThreadManager;
     EXPECT_CALL(*dbus_thread_manager, GetSystemBus())
         .WillRepeatedly(Return(static_cast<dbus::Bus*>(NULL)));
     DBusThreadManager::InitializeForTesting(dbus_thread_manager);
 
     EXPECT_CALL(*dbus_thread_manager, GetShillManagerClient())
         .WillRepeatedly(Return(&mock_manager_client_));
     EXPECT_CALL(*dbus_thread_manager, GetShillServiceClient())
         .WillRepeatedly(Return(&mock_service_client_));
     EXPECT_CALL(*dbus_thread_manager, GetShillProfileClient())
         .WillRepeatedly(Return(&mock_profile_client_));
 
     ON_CALL(mock_profile_client_, GetProperties(_,_,_))
         .WillByDefault(Invoke(&stub_profile_client_,
                               &ShillProfileClientStub::GetProperties));
 
     ON_CALL(mock_profile_client_, GetEntry(_,_,_,_))
         .WillByDefault(Invoke(&stub_profile_client_,
                               &ShillProfileClientStub::GetEntry));
 
     NetworkConfigurationHandler::Initialize();
-    ManagedNetworkConfigurationHandler::Initialize();
+    ManagedNetworkConfigurationHandler::Initialize(&stub_profile_handler_);

[pneubeck (no reviews), 2013/04/29 18:05:51]

I think it would be better, to use the constructor directly instead of using
Initialize(..).

[stevenjb, 2013/04/30 17:42:47]

If we were using a mock or a locally defined stub, I would suggest adding a
setter to ManagedNetworkConfigurationHandler to allow overriding the profile
handler. But since we're using NetworkProfileHandlerStub anwyay, I'd just rather
use the instances created by DbusThreadManager.

     message_loop_.RunUntilIdle();
   }
 
   virtual void TearDown() OVERRIDE {
     ManagedNetworkConfigurationHandler::Shutdown();
     NetworkConfigurationHandler::Shutdown();
     DBusThreadManager::Shutdown();
   }
 
+  void VerifyAndClearExpectations() {
+    Mock::VerifyAndClearExpectations(&mock_manager_client_);
+    Mock::VerifyAndClearExpectations(&mock_service_client_);
+    Mock::VerifyAndClearExpectations(&mock_profile_client_);
+  }
+
+  void InitializeStandardProfiles() {
+    stub_profile_client_.AddProfile(kUser1ProfilePath, kUser1);
+    stub_profile_handler_.AddProfile(NetworkProfile(kUser1ProfilePath, kUser1));
+    stub_profile_handler_.AddProfile(
+        NetworkProfile(kSharedProfilePath, std::string()));
+  }
+
   void SetUpEntry(const std::string& path_to_shill_json,
                   const std::string& profile_path,
                   const std::string& entry_path) {
-    stub_profile_client_.AddEntry(
-        profile_path,
-        entry_path,
-        *test_utils::ReadTestDictionary(path_to_shill_json));
+    scoped_ptr<base::DictionaryValue> entry =
+        test_utils::ReadTestDictionary(path_to_shill_json);
+    entry->SetStringWithoutPathExpansion(flimflam::kProfileProperty,
+                                         profile_path);
+    stub_profile_client_.AddEntry(profile_path, entry_path, *entry);
   }
 
   void SetPolicy(onc::ONCSource onc_source,
+                 const std::string& userhash,
                  const std::string& path_to_onc) {
     scoped_ptr<base::DictionaryValue> policy;
     if (path_to_onc.empty())
       policy = onc::ReadDictionaryFromJson(onc::kEmptyUnencryptedConfiguration);
     else
       policy = test_utils::ReadTestDictionary(path_to_onc);
 
     base::ListValue* network_configs = NULL;
     policy->GetListWithoutPathExpansion(
         onc::toplevel_config::kNetworkConfigurations, &network_configs);
 
-    managed_handler()->SetPolicy(onc::ONC_SOURCE_USER_POLICY, *network_configs);
+    managed_handler()->SetPolicy(
+        onc::ONC_SOURCE_USER_POLICY, userhash, *network_configs);
   }
 
   ManagedNetworkConfigurationHandler* managed_handler() {
     return ManagedNetworkConfigurationHandler::Get();
   }
 
  protected:
   StrictMock<MockShillManagerClient> mock_manager_client_;
   StrictMock<MockShillServiceClient> mock_service_client_;
   StrictMock<MockShillProfileClient> mock_profile_client_;
   ShillProfileClientStub stub_profile_client_;

[stevenjb, 2013/04/30 17:42:47]

Again, mixing mocks and stubs is going to be very difficult to maintain. Also,
if DbusThreadManager is initialized then there will be two stub instances, which
can lead to subtle errors (I've seen that happen).

Please either use a mock or local stub, or just use all of the stubs together as
they are intended.

[pneubeck (no reviews), 2013/05/03 17:32:55]

Why should there be two stubs? The MockDBusThreadManager is used, not the normal
one with ClientStubs.
Anyways, using a local stub now.

+  NetworkProfileHandlerStub stub_profile_handler_;
   MessageLoop message_loop_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ManagedNetworkConfigurationHandlerTest);
 };
 
+TEST_F(ManagedNetworkConfigurationHandlerTest, ProfileInitialization) {
+  InitializeStandardProfiles();
+  message_loop_.RunUntilIdle();
+}
+
 TEST_F(ManagedNetworkConfigurationHandlerTest, SetPolicyManageUnconfigured) {
+  InitializeStandardProfiles();
   scoped_ptr<base::DictionaryValue> expected_shill_properties =
       test_utils::ReadTestDictionary(
           "policy/shill_policy_on_unconfigured_wifi1.json");
 
   EXPECT_CALL(mock_profile_client_,
-              GetProperties(dbus::ObjectPath("/profile/chronos/shill"),
-                            _, _)).Times(1);
+              GetProperties(dbus::ObjectPath(kUser1ProfilePath), _, _));
 
   EXPECT_CALL(mock_manager_client_,
               ConfigureServiceForProfile(
-                  dbus::ObjectPath("/profile/chronos/shill"),
+                  dbus::ObjectPath(kUser1ProfilePath),
                   IsEqualTo(expected_shill_properties.get()),
-                  _, _)).Times(1);
+                  _, _));
 
-  SetPolicy(onc::ONC_SOURCE_USER_POLICY, "policy/policy_wifi1.onc");
+  SetPolicy(onc::ONC_SOURCE_USER_POLICY, kUser1, "policy/policy_wifi1.onc");
   message_loop_.RunUntilIdle();
 }
 
 TEST_F(ManagedNetworkConfigurationHandlerTest, SetPolicyIgnoreUnmodified) {
-  EXPECT_CALL(mock_profile_client_,
-              GetProperties(_, _, _)).Times(1);
+  InitializeStandardProfiles();
+  EXPECT_CALL(mock_profile_client_, GetProperties(_, _, _));
 
-  EXPECT_CALL(mock_manager_client_,
-              ConfigureServiceForProfile(_, _, _, _)).Times(1);
+  EXPECT_CALL(mock_manager_client_, ConfigureServiceForProfile(_, _, _, _));
 
-  SetPolicy(onc::ONC_SOURCE_USER_POLICY, "policy/policy_wifi1.onc");
+  SetPolicy(onc::ONC_SOURCE_USER_POLICY, kUser1, "policy/policy_wifi1.onc");
   message_loop_.RunUntilIdle();
-  Mock::VerifyAndClearExpectations(&mock_profile_client_);
-  Mock::VerifyAndClearExpectations(&mock_manager_client_);
+  VerifyAndClearExpectations();
 
   SetUpEntry("policy/shill_policy_on_unmanaged_user_wifi1.json",
-             "/profile/chronos/shill",
+             kUser1ProfilePath,
              "some_entry_path");
 
-  EXPECT_CALL(mock_profile_client_,
-              GetProperties(_, _, _)).Times(1);
+  EXPECT_CALL(mock_profile_client_, GetProperties(_, _, _));
 
   EXPECT_CALL(mock_profile_client_,
-              GetEntry(dbus::ObjectPath("/profile/chronos/shill"),
+              GetEntry(dbus::ObjectPath(kUser1ProfilePath),
                        "some_entry_path",
-                       _, _)).Times(1);
+                       _, _));
 
-  SetPolicy(onc::ONC_SOURCE_USER_POLICY, "policy/policy_wifi1.onc");
+  SetPolicy(onc::ONC_SOURCE_USER_POLICY, kUser1, "policy/policy_wifi1.onc");
   message_loop_.RunUntilIdle();
 }
 
 TEST_F(ManagedNetworkConfigurationHandlerTest, SetPolicyManageUnmanaged) {
+  InitializeStandardProfiles();
   SetUpEntry("policy/shill_unmanaged_user_wifi1.json",
-             "/profile/chronos/shill",
+             kUser1ProfilePath,
              "old_entry_path");
 
   scoped_ptr<base::DictionaryValue> expected_shill_properties =
       test_utils::ReadTestDictionary(
           "policy/shill_policy_on_unmanaged_user_wifi1.json");
 
   EXPECT_CALL(mock_profile_client_,
-              GetProperties(dbus::ObjectPath("/profile/chronos/shill"),
-                            _, _)).Times(1);
+              GetProperties(dbus::ObjectPath(kUser1ProfilePath), _, _));
 
   EXPECT_CALL(mock_profile_client_,
-              GetEntry(dbus::ObjectPath("/profile/chronos/shill"),
+              GetEntry(dbus::ObjectPath(kUser1ProfilePath),
                        "old_entry_path",
-                       _, _)).Times(1);
+                       _, _));
 
   EXPECT_CALL(mock_manager_client_,
               ConfigureServiceForProfile(
-                  dbus::ObjectPath("/profile/chronos/shill"),
+                  dbus::ObjectPath(kUser1ProfilePath),
                   IsEqualTo(expected_shill_properties.get()),
-                  _, _)).Times(1);
+                  _, _));
 
-  SetPolicy(onc::ONC_SOURCE_USER_POLICY, "policy/policy_wifi1.onc");
+  SetPolicy(onc::ONC_SOURCE_USER_POLICY, kUser1, "policy/policy_wifi1.onc");
   message_loop_.RunUntilIdle();
 }
 
 TEST_F(ManagedNetworkConfigurationHandlerTest, SetPolicyUpdateManagedNewGUID) {
+  InitializeStandardProfiles();
   SetUpEntry("policy/shill_managed_wifi1.json",
-             "/profile/chronos/shill",
+             kUser1ProfilePath,
              "old_entry_path");
 
   scoped_ptr<base::DictionaryValue> expected_shill_properties =
       test_utils::ReadTestDictionary(
           "policy/shill_policy_on_unmanaged_user_wifi1.json");
 
   // The passphrase isn't sent again, because it's configured by the user and
   // Shill doesn't sent it on GetProperties calls.
   expected_shill_properties->RemoveWithoutPathExpansion(
       flimflam::kPassphraseProperty, NULL);
 
   EXPECT_CALL(mock_profile_client_,
-              GetProperties(dbus::ObjectPath("/profile/chronos/shill"),
-                            _, _)).Times(1);
+              GetProperties(dbus::ObjectPath(kUser1ProfilePath), _, _));
 
   EXPECT_CALL(mock_profile_client_,
-              GetEntry(dbus::ObjectPath("/profile/chronos/shill"),
+              GetEntry(dbus::ObjectPath(kUser1ProfilePath),
                        "old_entry_path",
-                       _, _)).Times(1);
+                       _, _));
 
   EXPECT_CALL(mock_manager_client_,
               ConfigureServiceForProfile(
-                  dbus::ObjectPath("/profile/chronos/shill"),
+                  dbus::ObjectPath(kUser1ProfilePath),
                   IsEqualTo(expected_shill_properties.get()),
-                  _, _)).Times(1);
+                  _, _));
 
-  SetPolicy(onc::ONC_SOURCE_USER_POLICY, "policy/policy_wifi1.onc");
+  SetPolicy(onc::ONC_SOURCE_USER_POLICY, kUser1, "policy/policy_wifi1.onc");
   message_loop_.RunUntilIdle();
 }
 
 TEST_F(ManagedNetworkConfigurationHandlerTest, SetPolicyReapplyToManaged) {
+  InitializeStandardProfiles();
   SetUpEntry("policy/shill_policy_on_unmanaged_user_wifi1.json",
-             "/profile/chronos/shill",
+             kUser1ProfilePath,
              "old_entry_path");
 
   scoped_ptr<base::DictionaryValue> expected_shill_properties =
       test_utils::ReadTestDictionary(
           "policy/shill_policy_on_unmanaged_user_wifi1.json");
 
   // The passphrase isn't sent again, because it's configured by the user and
   // Shill doesn't sent it on GetProperties calls.
   expected_shill_properties->RemoveWithoutPathExpansion(
       flimflam::kPassphraseProperty, NULL);
 
   EXPECT_CALL(mock_profile_client_,
-              GetProperties(dbus::ObjectPath("/profile/chronos/shill"),
-                            _, _)).Times(1);
+              GetProperties(dbus::ObjectPath(kUser1ProfilePath), _, _));
 
   EXPECT_CALL(mock_profile_client_,
-              GetEntry(dbus::ObjectPath("/profile/chronos/shill"),
+              GetEntry(dbus::ObjectPath(kUser1ProfilePath),
                        "old_entry_path",
-                       _, _)).Times(1);
+                       _, _));
 
   EXPECT_CALL(mock_manager_client_,
               ConfigureServiceForProfile(
-                  dbus::ObjectPath("/profile/chronos/shill"),
+                  dbus::ObjectPath(kUser1ProfilePath),
                   IsEqualTo(expected_shill_properties.get()),
-                  _, _)).Times(1);
+                  _, _));
 
-  SetPolicy(onc::ONC_SOURCE_USER_POLICY, "policy/policy_wifi1.onc");
+  SetPolicy(onc::ONC_SOURCE_USER_POLICY, kUser1, "policy/policy_wifi1.onc");
   message_loop_.RunUntilIdle();
 }
 
 TEST_F(ManagedNetworkConfigurationHandlerTest, SetPolicyUnmanageManaged) {
+  InitializeStandardProfiles();
   SetUpEntry("policy/shill_policy_on_unmanaged_user_wifi1.json",
-             "/profile/chronos/shill",
+             kUser1ProfilePath,
              "old_entry_path");
 
   EXPECT_CALL(mock_profile_client_,
-              GetProperties(dbus::ObjectPath("/profile/chronos/shill"),
-                            _, _)).Times(1);
+              GetProperties(dbus::ObjectPath(kUser1ProfilePath), _, _));
 
   EXPECT_CALL(mock_profile_client_,
-              GetEntry(dbus::ObjectPath("/profile/chronos/shill"),
+              GetEntry(dbus::ObjectPath(kUser1ProfilePath),
                        "old_entry_path",
-                       _, _)).Times(1);
+                       _, _));
 
   EXPECT_CALL(mock_profile_client_,
-              DeleteEntry(dbus::ObjectPath("/profile/chronos/shill"),
+              DeleteEntry(dbus::ObjectPath(kUser1ProfilePath),
                           "old_entry_path",
-                          _, _)).Times(1);
+                          _, _));
 
-  SetPolicy(onc::ONC_SOURCE_USER_POLICY, "");
+  SetPolicy(onc::ONC_SOURCE_USER_POLICY, kUser1, "");
   message_loop_.RunUntilIdle();
 }
 
 TEST_F(ManagedNetworkConfigurationHandlerTest, SetPolicyIgnoreUnmanaged) {
+  InitializeStandardProfiles();
   SetUpEntry("policy/shill_unmanaged_user_wifi1.json",
-             "/profile/chronos/shill",
+             kUser1ProfilePath,
              "old_entry_path");
 
   EXPECT_CALL(mock_profile_client_,
-              GetProperties(dbus::ObjectPath("/profile/chronos/shill"),
-                            _, _)).Times(1);
+              GetProperties(dbus::ObjectPath(kUser1ProfilePath), _, _));
 
   EXPECT_CALL(mock_profile_client_,
-              GetEntry(dbus::ObjectPath("/profile/chronos/shill"),
+              GetEntry(dbus::ObjectPath(kUser1ProfilePath),
                        "old_entry_path",
-                       _, _)).Times(1);
+                       _, _));
 
-  SetPolicy(onc::ONC_SOURCE_USER_POLICY, "");
+  SetPolicy(onc::ONC_SOURCE_USER_POLICY, kUser1, "");
   message_loop_.RunUntilIdle();
 }
 
+TEST_F(ManagedNetworkConfigurationHandlerTest, LateProfileLoading) {
+  SetPolicy(onc::ONC_SOURCE_USER_POLICY, kUser1, "policy/policy_wifi1.onc");
+
+  message_loop_.RunUntilIdle();
+  VerifyAndClearExpectations();
+
+  scoped_ptr<base::DictionaryValue> expected_shill_properties =
+      test_utils::ReadTestDictionary(
+          "policy/shill_policy_on_unconfigured_wifi1.json");
+
+  EXPECT_CALL(mock_profile_client_,
+              GetProperties(dbus::ObjectPath(kUser1ProfilePath), _, _));
+
+  EXPECT_CALL(mock_manager_client_,
+              ConfigureServiceForProfile(
+                  dbus::ObjectPath(kUser1ProfilePath),
+                  IsEqualTo(expected_shill_properties.get()),
+                  _, _));
+
+  InitializeStandardProfiles();
+  message_loop_.RunUntilIdle();
+}
+
 }  // namespace chromeos