[Sync] Implementation of model association for passwords using sync API

chrome/browser/password_manager/password_syncable_service_unittest.cc

Index: chrome/browser/password_manager/password_syncable_service_unittest.cc
diff --git a/chrome/browser/password_manager/password_syncable_service_unittest.cc b/chrome/browser/password_manager/password_syncable_service_unittest.cc
new file mode 100755
index 0000000000000000000000000000000000000000..ef2adb9c6ca3452660716799784b7c971d329f9d
--- /dev/null
+++ b/chrome/browser/password_manager/password_syncable_service_unittest.cc
@@ -0,0 +1,368 @@
+// 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 <string>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/location.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
+#include "chrome/browser/password_manager/mock_password_store.h"
+#include "chrome/browser/password_manager/password_store_factory.h"
+#include "chrome/browser/password_manager/password_syncable_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/test/base/testing_profile.h"
+#include "sync/api/sync_change_processor.h"
+#include "sync/api/sync_error.h"
+#include "sync/api/sync_error_factory.h"
+#include "sync/protocol/password_specifics.pb.h"
+#include "sync/protocol/sync.pb.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using syncer::SyncChange;
+using syncer::SyncData;
+using syncer::SyncDataList;
+using syncer::SyncError;
+using testing::Invoke;
+using testing::Return;
+using testing::SetArgumentPointee;
+using testing::_;
+
+namespace {

[Nicolas Zea, 2013/10/30 23:57:18]

nit: newline after

[lipalani1, 2013/10/31 19:22:18]

Done.

+typedef std::vector<SyncChange> SyncChangeList;
+
+// This class will be instantiated by the tests rather than the
+// |PasswordSyncableService| as this class will mock away any calls
+// that touch the password db.
+class TestPasswordSyncableService : public PasswordSyncableService {
+ public:
+  explicit TestPasswordSyncableService(PasswordStore* password_store):
+      PasswordSyncableService(password_store) {}
+  virtual ~TestPasswordSyncableService() {}
+  MOCK_METHOD0(NotifyPasswordStore, void());
+};
+
+// Concrete implementation of SyncChangeprocessor. The methods will
+// verify that the |PasswordSyncableService| is calling the
+// |SyncChangeProcessor| with right arguments.
+class TestSyncChangeProcessor : public syncer::SyncChangeProcessor {
+ public:
+  TestSyncChangeProcessor() {}
+  virtual ~TestSyncChangeProcessor() {}
+  virtual SyncError ProcessSyncChanges(
+      const tracked_objects::Location& from_here,
+      const SyncChangeList& change_list) {
+    // Loop through the |change_list| and verify they are present in the
+    // |expected_changes_| list.
+    for (SyncChangeList::const_iterator it = change_list.begin();
+         it != change_list.end();
+         ++it) {
+      SyncChange data = *it;
+      const sync_pb::EntitySpecifics& specifics =
+          data.sync_data().GetSpecifics();
+      const sync_pb::PasswordSpecificsData& password_specifics(
+          specifics.password().client_only_encrypted_data());
+      std::string actual_tag = PasswordSyncableService::MakeTag(
+          password_specifics);
+
+      bool matched = false;
+      for (SyncChangeList::iterator expected_it = expected_changes_.begin();
+           expected_it != expected_changes_.end();
+           ++expected_it) {
+        SyncChange expected_data = *expected_it;
+        const sync_pb::EntitySpecifics& specifics =
+            expected_data.sync_data().GetSpecifics();
+        const sync_pb::PasswordSpecificsData& password_specifics(
+            specifics.password().client_only_encrypted_data());
+        std::string expected_tag = PasswordSyncableService::MakeTag(
+            password_specifics);
+        if (expected_tag == actual_tag) {
+          if (data.change_type() == expected_data.change_type()) {
+            matched = true;
+          }
+          break;
+        }
+      }
+      EXPECT_TRUE(matched);
+    }
+    EXPECT_EQ(change_list.size(), expected_changes_.size());
+    return SyncError();
+  }
+
+  virtual SyncDataList GetAllSyncData(syncer::ModelType type) const {
+    return SyncDataList();
+  }
+
+  // Adds a password entry to the |expected_changes_| list.
+  void AddExpectedChange(const autofill::PasswordForm& password,
+                         SyncChange::SyncChangeType type) {
+    SyncData data = PasswordSyncableService::CreateSyncData(password);
+    SyncChange change(FROM_HERE, type, data);
+    expected_changes_.push_back(change);
+  }
+
+ private:
+  SyncChangeList expected_changes_;
+  DISALLOW_COPY_AND_ASSIGN(TestSyncChangeProcessor);
+};
+
+// Class to verify the arguments passed to |PasswordStore|.
+class PasswordStoreDataVerifier {
+ public:
+  PasswordStoreDataVerifier() {}
+  virtual ~PasswordStoreDataVerifier() {}
+
+  // Adds an expected add change.
+  void AddExpectedAddChange(const SyncData& data) {
+    const sync_pb::EntitySpecifics& specifics = data.GetSpecifics();
+    const sync_pb::PasswordSpecificsData& password_specifics(
+        specifics.password().client_only_encrypted_data());
+
+    autofill::PasswordForm form;
+    PasswordSyncableService::ExtractPasswordFromSpecifics(password_specifics,
+                                                          &form);
+    add_changes_.push_back(form);
+  }
+
+  // Adds an expected update change.
+  void AddExpectedUpdateChange(const autofill::PasswordForm& form) {
+    update_changes_.push_back(form);
+  }
+
+  // Verifies that the |password| is present in the |add_changes_| list.
+  void VerifyAdd(const autofill::PasswordForm& password) {
+    VerifyChange(password, &add_changes_);
+  }
+
+  // Verifies that the |password| is present in the |update_changes_| list.
+  void VerifyUpdate(const autofill::PasswordForm& password) {
+    VerifyChange(password, &update_changes_);
+  }
+
+  int AddChangeCount() const {
+    return add_changes_.size();
+  }
+
+  int UpdateChangeCount() const {
+    return update_changes_.size();
+  }
+
+ private:
+  void VerifyChange(const autofill::PasswordForm& password,
+                std::vector<autofill::PasswordForm>* password_list) {

[Nicolas Zea, 2013/10/30 23:57:18]

fix indent

[lipalani1, 2013/10/31 19:22:18]

Done.

+    bool matched = false;
+    for (std::vector<autofill::PasswordForm>::iterator it
+           = password_list->begin();
+       it != password_list->end();

[Nicolas Zea, 2013/10/30 23:57:18]

here too

[lipalani1, 2013/10/31 19:22:18]

Done.

+       ++it) {
+      if (password == *it) {
+        password_list->erase(it);
+        matched = true;
+        break;
+      }
+    }
+    EXPECT_TRUE(matched);
+  }
+  std::vector<autofill::PasswordForm> add_changes_;
+  std::vector<autofill::PasswordForm> update_changes_;
+  DISALLOW_COPY_AND_ASSIGN(PasswordStoreDataVerifier);
+};
+
+SyncData CreateSyncData(std::string signon_realm) {
+  sync_pb::EntitySpecifics password_data;
+  sync_pb::PasswordSpecificsData* password_specifics =
+      password_data.mutable_password()->mutable_client_only_encrypted_data();
+  password_specifics->set_signon_realm(signon_realm);
+
+  std::string tag = PasswordSyncableService::MakeTag(*password_specifics);
+  return syncer::SyncData::CreateLocalData(tag, tag, password_data);
+}
+
+class PasswordSyncableServiceTest : public testing::Test {
+ public:
+  PasswordSyncableServiceTest() {}
+  ~PasswordSyncableServiceTest() {}
+
+  virtual void SetUp() OVERRIDE {
+    TestingProfile::Builder builder;
+    scoped_ptr<Profile> profile = builder.Build().Pass();
+    password_store_ = static_cast<MockPasswordStore*>(
+        PasswordStoreFactory::GetInstance()->SetTestingFactoryAndUse(
+            profile.get(), MockPasswordStore::Build).get());
+    service_.reset(new TestPasswordSyncableService(password_store_));
+    sync_change_processor_.reset(new TestSyncChangeProcessor);
+  }
+
+  TestPasswordSyncableService* service() {
+    return service_.get();
+  }
+
+  PasswordStoreDataVerifier* verifier() {
+    return &verifier_;
+  }
+
+  scoped_ptr<TestSyncChangeProcessor>* sync_change_processor() {

[Nicolas Zea, 2013/10/30 23:57:18]

no need to return scoped ptr, just return normal pointer
(sync_change_processor_get())

[lipalani1, 2013/10/31 19:22:18]

We have to pass this scoped ptr as a parameter to MergeDataAndStartSyncing(which
is achieved by doing a .Pass)

The pointer ownership transfers to PasswordSyncableService. Another option is we
can have a method here called release_sync_change_processor which returns a
pointer. Let me know what you think.
On 2013/10/30 23:57:18, Nicolas Zea wrote:

[Nicolas Zea, 2013/10/31 22:41:56]

Ah, I see. Yeah, I think two different methods is better. sync_change_processor
should just return the pointer, while ReleaseSyncChangeProcessor() should return
sync_change_processor_.Pass().

+    return &sync_change_processor_;
+  }
+
+  void SetPasswordStoreData(
+      const std::vector<autofill::PasswordForm*>& forms) {
+    EXPECT_CALL(*(password_store_.get()), FillAutofillableLogins(_))
+        .WillOnce(DoAll(SetArgumentPointee<0>(forms), Return(true)));
+    EXPECT_CALL(*(password_store_.get()), FillBlacklistLogins(_))
+        .WillOnce(Return(true));
+  }
+
+ protected:
+  scoped_refptr<MockPasswordStore> password_store_;
+  scoped_ptr<TestPasswordSyncableService> service_;
+  PasswordStoreDataVerifier verifier_;
+  scoped_ptr<TestSyncChangeProcessor> sync_change_processor_;
+};
+
+// Both sync and password db have data that are not present in the other.
+TEST_F(PasswordSyncableServiceTest, AdditionsInBoth) {
+  autofill::PasswordForm *form1 = new autofill::PasswordForm;
+  form1->signon_realm = "abc";
+
+  std::vector<autofill::PasswordForm*> forms;
+  forms.push_back(form1);
+
+  SyncData sync_data = CreateSyncData("def");
+  SyncDataList list;
+  list.push_back(sync_data);
+
+  (*sync_change_processor())->AddExpectedChange(*form1,
+                                                SyncChange::ACTION_ADD);
+
+  verifier()->AddExpectedAddChange(sync_data);
+
+  SetPasswordStoreData(forms);
+  EXPECT_CALL(*(password_store_.get()), AddLoginImpl(_))

[Nicolas Zea, 2013/10/30 23:57:18]

you can just do *password_store_, here and elsewhere

[lipalani1, 2013/10/31 19:22:18]

Done.

+      .WillRepeatedly(Invoke(
+           verifier(), &PasswordStoreDataVerifier::VerifyAdd));
+
+  EXPECT_CALL(*(service()), NotifyPasswordStore());

[Nicolas Zea, 2013/10/30 23:57:18]

no need for parens around (service()), here and elsewhere

[lipalani1, 2013/10/31 19:22:18]

Done.

+
+  service()->MergeDataAndStartSyncing(syncer::PASSWORDS,
+                                      list,
+                                      (*(sync_change_processor())).Pass(),
+                                      scoped_ptr<syncer::SyncErrorFactory>()
+                                          .Pass());
+
+  EXPECT_EQ(0, verifier()->AddChangeCount());
+}
+
+// Sync has data that is not present in the password db.
+TEST_F(PasswordSyncableServiceTest, AdditionOnlyInSync) {
+  std::vector<autofill::PasswordForm*> forms;
+
+  SyncData sync_data = CreateSyncData("def");
+  SyncDataList list;
+  list.push_back(sync_data);
+
+  verifier()->AddExpectedAddChange(sync_data);
+
+  SetPasswordStoreData(forms);
+
+  EXPECT_CALL(*(password_store_.get()), AddLoginImpl(_))
+      .WillRepeatedly(Invoke(
+           verifier(), &PasswordStoreDataVerifier::VerifyAdd));
+
+  EXPECT_CALL(*(service()), NotifyPasswordStore());
+
+  service()->MergeDataAndStartSyncing(syncer::PASSWORDS,
+                                      list,
+                                      (*(sync_change_processor())).Pass(),
+                                      scoped_ptr<syncer::SyncErrorFactory>()
+                                          .Pass());
+
+  EXPECT_EQ(0, verifier()->AddChangeCount());
+}
+
+// Passwords db has data that is not present in sync.
+TEST_F(PasswordSyncableServiceTest, AdditionOnlyInPasswordStore) {
+  autofill::PasswordForm *form1 = new autofill::PasswordForm;
+  form1->signon_realm = "abc";
+
+  std::vector<autofill::PasswordForm*> forms;
+  forms.push_back(form1);
+
+  SyncDataList list;
+
+  (*sync_change_processor())->AddExpectedChange(*form1,
+                                                SyncChange::ACTION_ADD);
+
+  SetPasswordStoreData(forms);
+
+  service()->MergeDataAndStartSyncing(syncer::PASSWORDS,
+                                      list,
+                                      (*(sync_change_processor())).Pass(),
+                                      scoped_ptr<syncer::SyncErrorFactory>()
+                                          .Pass());
+
+  EXPECT_EQ(0, verifier()->AddChangeCount());
+}
+
+// Both passwords db and sync contain the same data.
+TEST_F(PasswordSyncableServiceTest, BothInSync) {
+  autofill::PasswordForm *form1 = new autofill::PasswordForm;
+  form1->signon_realm = "abc";
+
+  std::vector<autofill::PasswordForm*> forms;
+  forms.push_back(form1);
+
+  SyncData sync_data = CreateSyncData("abc");
+  SyncDataList list;
+  list.push_back(sync_data);
+
+  SetPasswordStoreData(forms);
+
+  service()->MergeDataAndStartSyncing(syncer::PASSWORDS,
+                                      list,
+                                      (*(sync_change_processor())).Pass(),
+                                      scoped_ptr<syncer::SyncErrorFactory>()
+                                         .Pass());

[Nicolas Zea, 2013/10/30 23:57:18]

check verifier afterwards?

[lipalani1, 2013/10/31 19:22:18]

Done.

+}
+
+// Both passwords db and sync have the same data but they need to be merged
+// as some fields of the data differ.
+TEST_F(PasswordSyncableServiceTest, Merge) {
+  autofill::PasswordForm *form1 = new autofill::PasswordForm;
+  form1->signon_realm = "abc";
+  form1->action = GURL("http://pie.com");
+
+  std::vector<autofill::PasswordForm*> forms;
+  forms.push_back(form1);
+
+  SyncData sync_data = CreateSyncData("abc");
+  SyncDataList list;
+  list.push_back(sync_data);
+
+  (*sync_change_processor())->AddExpectedChange(*form1,
+                                                SyncChange::ACTION_UPDATE);
+
+  verifier()->AddExpectedUpdateChange(*form1);
+
+  SetPasswordStoreData(forms);
+
+  EXPECT_CALL(*(password_store_.get()), UpdateLoginImpl(_))
+      .WillRepeatedly(Invoke(verifier(),
+                             &PasswordStoreDataVerifier::VerifyUpdate));
+
+  EXPECT_CALL(*(service()), NotifyPasswordStore());
+
+  service()->MergeDataAndStartSyncing(syncer::PASSWORDS,
+                                      list,
+                                      (*(sync_change_processor())).Pass(),
+                                      scoped_ptr<syncer::SyncErrorFactory>()
+                                          .Pass());
+
+  EXPECT_EQ(0, verifier()->UpdateChangeCount());
+}

[Nicolas Zea, 2013/10/30 23:57:18]

nit: newline after

[lipalani1, 2013/10/31 19:22:18]

Done.

+}  // namespace
+