Adding a NetworkProfileHandler used by ManagedNetworkConfigurationHandler.

chromeos/network/managed_network_configuration_handler_unittest.cc

Index: chromeos/network/managed_network_configuration_handler_unittest.cc
diff --git a/chromeos/network/managed_network_configuration_handler_unittest.cc b/chromeos/network/managed_network_configuration_handler_unittest.cc
index def9204afc87df29a6198bfa4f3453b7f5a05bf5..5c24d0bedbf8a191e3ccd981240eb768e032fd0c 100644
--- a/chromeos/network/managed_network_configuration_handler_unittest.cc
+++ b/chromeos/network/managed_network_configuration_handler_unittest.cc
@@ -15,6 +15,7 @@
 #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"
@@ -41,6 +42,10 @@ std::string ValueToString(const base::Value* 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,
@@ -56,7 +61,8 @@ namespace chromeos {
 
 class ManagedNetworkConfigurationHandlerTest : public testing::Test {
  public:
-  ManagedNetworkConfigurationHandlerTest() {
+  ManagedNetworkConfigurationHandlerTest()
+      : stub_profile_client_(NULL /* no ShillManagerClient::TestInterface */ ) {
   }
 
   virtual ~ManagedNetworkConfigurationHandlerTest() {
@@ -84,7 +90,7 @@ class ManagedNetworkConfigurationHandlerTest : public testing::Test {
                               &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();
   }
 
@@ -94,16 +100,31 @@ class ManagedNetworkConfigurationHandlerTest : public testing::Test {
     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())
@@ -115,7 +136,8 @@ class ManagedNetworkConfigurationHandlerTest : public testing::Test {
     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() {
@@ -127,62 +149,66 @@ class ManagedNetworkConfigurationHandlerTest : public testing::Test {
   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 =
@@ -190,27 +216,27 @@ TEST_F(ManagedNetworkConfigurationHandlerTest, SetPolicyManageUnmanaged) {
           "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 =
@@ -223,27 +249,27 @@ TEST_F(ManagedNetworkConfigurationHandlerTest, SetPolicyUpdateManagedNewGUID) {
       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 =
@@ -256,62 +282,84 @@ TEST_F(ManagedNetworkConfigurationHandlerTest, SetPolicyReapplyToManaged) {
       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, 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()),
+                  _, _));
 
-  SetPolicy(onc::ONC_SOURCE_USER_POLICY, "");
+  InitializeStandardProfiles();
   message_loop_.RunUntilIdle();
 }