Refactored PairingRegistry::Delegate such that it can retrieve/modify for a single client.

remoting/host/pairing_registry_delegate_linux_unittest.cc

Index: remoting/host/pairing_registry_delegate_linux_unittest.cc
diff --git a/remoting/host/pairing_registry_delegate_linux_unittest.cc b/remoting/host/pairing_registry_delegate_linux_unittest.cc
index 0f679eb8f257670f21f596ce95aae7b8d44871e6..86bfb370cc7d077a7b9fdaadec8d50e98252dbef 100644
--- a/remoting/host/pairing_registry_delegate_linux_unittest.cc
+++ b/remoting/host/pairing_registry_delegate_linux_unittest.cc
@@ -19,19 +19,22 @@ using protocol::PairingRegistry;
 class PairingRegistryDelegateLinuxTest : public testing::Test {
  public:
   void SaveComplete(PairingRegistry::Delegate* delegate,
-                    const std::string& expected_json,
+                    PairingRegistry::Pairing expected_pairing,
                     bool success) {
     EXPECT_TRUE(success);
+
     // Load the pairings again to make sure we get what we've just written.

[Jamie, 2013/07/30 21:35:07]

s/pairings/pairing/

[alexeypa (please no reviews), 2013/07/31 21:31:24]

Done.

     delegate->Load(
+        expected_pairing.client_id(),
         base::Bind(&PairingRegistryDelegateLinuxTest::VerifyLoad,
                    base::Unretained(this),
-                   expected_json));
+                   expected_pairing));
   }
 
-  void VerifyLoad(const std::string& expected,
-                  const std::string& actual) {
-    EXPECT_EQ(actual, expected);
+  void VerifyLoad(PairingRegistry::Pairing expected_pairing,
+                  PairingRegistry::Pairing actual_pairing) {
+    EXPECT_EQ(actual_pairing.client_id(), expected_pairing.client_id());
+    EXPECT_EQ(actual_pairing, expected_pairing);
     base::MessageLoop::current()->Quit();
   }
 };
@@ -45,7 +48,7 @@ TEST_F(PairingRegistryDelegateLinuxTest, SaveAndLoad) {
   // directory if it doesn't exist.
   base::FilePath temp_dir;
   file_util::CreateNewTempDirectory("chromoting-test", &temp_dir);
-  base::FilePath temp_file = temp_dir.Append("dir").Append("registry.json");
+  base::FilePath temp_registry = temp_dir.Append("pairing-registry");
 
   scoped_refptr<base::TaskRunner> task_runner =
       base::ThreadTaskRunnerHandle::Get();
@@ -53,20 +56,18 @@ TEST_F(PairingRegistryDelegateLinuxTest, SaveAndLoad) {
       new PairingRegistryDelegateLinux(task_runner));
   scoped_ptr<PairingRegistryDelegateLinux> load_delegate(
       new PairingRegistryDelegateLinux(task_runner));
-  save_delegate->SetFilenameForTesting(temp_file);
-  load_delegate->SetFilenameForTesting(temp_file);
+  save_delegate->SetRegistryPathForTesting(temp_registry);
+  load_delegate->SetRegistryPathForTesting(temp_registry);

[Jamie, 2013/07/30 21:35:07]

I think we need more unit tests now that the interface has more methods (and the
implementation is more complex). For example, we should verify LoadAllPairings
when we've just created the directory (should be empty), LoadPairing with an
unknown client id when the registry is not empty. At a minimum, there should be
a unit test per interface method.

If you want to get started on the Windows implementation, then it's fine to
leave this for now, and I'll implement them.

[alexeypa (please no reviews), 2013/07/31 21:31:24]

Done.

 
   // Save the pairings, then load them using a different delegate to ensure
-  // that the test isn't passing due to cached values. Note that the delegate
-  // doesn't require that the strings it loads and saves are valid JSON, so
-  // we can simplify the test a bit.
-  std::string test_data = "test data";
+  // that the test isn't passing due to cached values.
+  PairingRegistry::Pairing test_pairing(base::Time::Now(), "xxx", "xxx", "xxx");
   save_delegate->Save(
-      test_data,
+      test_pairing,
       base::Bind(&PairingRegistryDelegateLinuxTest::SaveComplete,
                  base::Unretained(this),
                  load_delegate.get(),
-                 test_data));
+                 test_pairing));
 
   run_loop.Run();