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

remoting/host/pairing_registry_delegate_linux_unittest.cc

 // Copyright 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 "remoting/host/pairing_registry_delegate_linux.h"
 
 #include "base/file_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/task_runner.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/timer/timer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace remoting {
 
 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();
   }
 };
 
 TEST_F(PairingRegistryDelegateLinuxTest, SaveAndLoad) {
   base::MessageLoop message_loop;
   base::RunLoop run_loop;
 
   // Create a temporary directory in order to get a unique name and use a
   // subdirectory to ensure that the AddPairing method creates the parent
   // 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();
   scoped_ptr<PairingRegistryDelegateLinux> save_delegate(
       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();
 
   base::DeleteFile(temp_dir, true);
 };
 
 }  // namespace remoting