sync: Refactor NonBlockingTypeProcessor tests

sync/engine/non_blocking_type_processor_unittest.cc

Index: sync/engine/non_blocking_type_processor_unittest.cc
diff --git a/sync/engine/non_blocking_type_processor_unittest.cc b/sync/engine/non_blocking_type_processor_unittest.cc
index 3a3ca5c59e725d2cd010da4479a943fb8eedfbad..20ecb240cb0d93535b5f19783f5bc1cfefc09d77 100644
--- a/sync/engine/non_blocking_type_processor_unittest.cc
+++ b/sync/engine/non_blocking_type_processor_unittest.cc
@@ -10,103 +10,13 @@
 #include "sync/internal_api/public/sync_core_proxy.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/syncable/syncable_util.h"
+#include "sync/test/engine/injectable_sync_core_proxy.h"
+#include "sync/test/engine/mock_non_blocking_type_processor_core.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace syncer {
 
-namespace {
-
-class MockNonBlockingTypeProcessorCore
-    : public NonBlockingTypeProcessorCoreInterface {
- public:
-  MockNonBlockingTypeProcessorCore();
-  virtual ~MockNonBlockingTypeProcessorCore();
-
-  virtual void RequestCommits(const CommitRequestDataList& list) OVERRIDE;
-
-  bool is_connected_;
-
-  std::vector<CommitRequestDataList> commit_request_lists_;
-};
-
-MockNonBlockingTypeProcessorCore::MockNonBlockingTypeProcessorCore()
-    : is_connected_(false) {
-}
-
-MockNonBlockingTypeProcessorCore::~MockNonBlockingTypeProcessorCore() {
-}
-
-void MockNonBlockingTypeProcessorCore::RequestCommits(
-    const CommitRequestDataList& list) {
-  commit_request_lists_.push_back(list);
-}
-
-class MockSyncCoreProxy : public syncer::SyncCoreProxy {
- public:
-  MockSyncCoreProxy();
-  virtual ~MockSyncCoreProxy();
-
-  virtual void ConnectTypeToCore(
-      syncer::ModelType type,
-      const DataTypeState& data_type_state,
-      base::WeakPtr<syncer::NonBlockingTypeProcessor> type_processor) OVERRIDE;
-  virtual void Disconnect(syncer::ModelType type) OVERRIDE;
-  virtual scoped_ptr<SyncCoreProxy> Clone() const OVERRIDE;
-
-  MockNonBlockingTypeProcessorCore* GetMockProcessorCore();
-
- private:
-  explicit MockSyncCoreProxy(MockNonBlockingTypeProcessorCore*);
-
-  // The NonBlockingTypeProcessor's contract expects that it gets to own this
-  // object, so we can retain only a non-owned pointer to it.
-  //
-  // This is very unsafe, but we can get away with it since these tests are not
-  // exercising the processor <-> processor_core connection code.
-  MockNonBlockingTypeProcessorCore* mock_core_;
-};
-
-MockSyncCoreProxy::MockSyncCoreProxy()
-    : mock_core_(new MockNonBlockingTypeProcessorCore) {
-}
-
-MockSyncCoreProxy::MockSyncCoreProxy(MockNonBlockingTypeProcessorCore* core)
-    : mock_core_(core) {
-}
-
-MockSyncCoreProxy::~MockSyncCoreProxy() {
-}
-
-void MockSyncCoreProxy::ConnectTypeToCore(
-    syncer::ModelType type,
-    const DataTypeState& data_type_state,
-    base::WeakPtr<syncer::NonBlockingTypeProcessor> type_processor) {
-  // This class is allowed to participate in only one connection.
-  DCHECK(!mock_core_->is_connected_);
-  mock_core_->is_connected_ = true;
-
-  // Hands off ownership of our member to the type_processor, while keeping
-  // an unsafe pointer to it.  This is why we can only connect once.
-  scoped_ptr<NonBlockingTypeProcessorCoreInterface> core(mock_core_);
-
-  type_processor->OnConnect(core.Pass());
-}
-
-void MockSyncCoreProxy::Disconnect(syncer::ModelType type) {
-  // This mock object is not meant for connect and disconnect tests.
-  NOTREACHED() << "Not implemented";
-}
-
-scoped_ptr<SyncCoreProxy> MockSyncCoreProxy::Clone() const {
-  // There's no sensible way to clone this MockSyncCoreProxy.
-  return scoped_ptr<SyncCoreProxy>(new MockSyncCoreProxy(mock_core_));
-}
-
-MockNonBlockingTypeProcessorCore* MockSyncCoreProxy::GetMockProcessorCore() {
-  return mock_core_;
-}
-
-}  // namespace
+static const ModelType kModelType = PREFERENCES;
 
 // Tests the sync engine parts of NonBlockingTypeProcessor.
 //
@@ -169,37 +79,32 @@ class NonBlockingTypeProcessorTest : public ::testing::Test {
   void SuccessfulCommitResponse(const CommitRequestData& request_data);
 
  private:
-  std::string GenerateId(const std::string& tag) const;
-  std::string GenerateTagHash(const std::string& tag) const;
-  sync_pb::EntitySpecifics GenerateSpecifics(const std::string& tag,
-                                             const std::string& value) const;
+  static std::string GenerateTagHash(const std::string& tag);
+  static sync_pb::EntitySpecifics GenerateSpecifics(const std::string& tag,
+                                                    const std::string& value);
 
   int64 GetServerVersion(const std::string& tag);
   void SetServerVersion(const std::string& tag, int64 version);
 
-  const ModelType type_;
-
-  scoped_ptr<MockSyncCoreProxy> mock_sync_core_proxy_;
-  scoped_ptr<NonBlockingTypeProcessor> processor_;
   MockNonBlockingTypeProcessorCore* mock_processor_core_;
+  scoped_ptr<InjectableSyncCoreProxy> injectable_sync_core_proxy_;
+  scoped_ptr<NonBlockingTypeProcessor> processor_;
 
   DataTypeState data_type_state_;
-
-  std::map<const std::string, int64> server_versions_;
 };
 
 NonBlockingTypeProcessorTest::NonBlockingTypeProcessorTest()
-    : type_(PREFERENCES),
-      mock_sync_core_proxy_(new MockSyncCoreProxy()),
-      processor_(new NonBlockingTypeProcessor(type_)) {
+    : mock_processor_core_(new MockNonBlockingTypeProcessorCore()),
+      injectable_sync_core_proxy_(
+          new InjectableSyncCoreProxy(mock_processor_core_)),
+      processor_(new NonBlockingTypeProcessor(kModelType)) {
 }
 
 NonBlockingTypeProcessorTest::~NonBlockingTypeProcessorTest() {
 }
 
 void NonBlockingTypeProcessorTest::FirstTimeInitialize() {
-  processor_->Enable(mock_sync_core_proxy_->Clone());
-  mock_processor_core_ = mock_sync_core_proxy_->GetMockProcessorCore();
+  processor_->Enable(injectable_sync_core_proxy_->Clone());
 }
 
 void NonBlockingTypeProcessorTest::InitializeToReadyState() {
@@ -232,27 +137,11 @@ void NonBlockingTypeProcessorTest::UpdateFromServer(int64 version_offset,
                                                     const std::string& tag,
                                                     const std::string& value) {
   const std::string tag_hash = GenerateTagHash(tag);
-
-  // Overwrite the existing server version if this is the new highest version.
-  int64 old_version = GetServerVersion(tag_hash);
-  int64 version = old_version + version_offset;
-  if (version > old_version) {
-    SetServerVersion(tag_hash, version);
-  }
-
-  UpdateResponseData data;
-  data.id = GenerateId(tag_hash);
-  data.client_tag_hash = tag_hash;
-  data.response_version = version;
-  data.ctime = base::Time::UnixEpoch() + base::TimeDelta::FromDays(1);
-  data.mtime = data.ctime + base::TimeDelta::FromSeconds(version);
-  data.non_unique_name = tag;
-  data.deleted = false;
-  data.specifics = GenerateSpecifics(tag, value);
+  UpdateResponseData data = mock_processor_core_->UpdateFromServer(
+      version_offset, tag_hash, GenerateSpecifics(tag, value));
 
   UpdateResponseDataList list;
   list.push_back(data);
-
   processor_->OnUpdateReceived(data_type_state_, list);
 }
 
@@ -260,85 +149,30 @@ void NonBlockingTypeProcessorTest::TombstoneFromServer(int64 version_offset,
                                                        const std::string& tag) {
   // Overwrite the existing server version if this is the new highest version.
   std::string tag_hash = GenerateTagHash(tag);
-  int64 old_version = GetServerVersion(tag_hash);
-  int64 version = old_version + version_offset;
-  if (version > old_version) {
-    SetServerVersion(tag_hash, version);
-  }
-
-  UpdateResponseData data;
-  data.id = GenerateId(tag_hash);
-  data.client_tag_hash = tag_hash;
-  data.response_version = version;
-  data.ctime = base::Time::UnixEpoch() + base::TimeDelta::FromDays(1);
-  data.mtime = data.ctime + base::TimeDelta::FromSeconds(version);
-  data.non_unique_name = tag;
-  data.deleted = true;
+
+  UpdateResponseData data =
+      mock_processor_core_->TombstoneFromServer(version_offset, tag_hash);
 
   UpdateResponseDataList list;
   list.push_back(data);
-
   processor_->OnUpdateReceived(data_type_state_, list);
 }
 
 void NonBlockingTypeProcessorTest::SuccessfulCommitResponse(
     const CommitRequestData& request_data) {
-  const std::string& client_tag_hash = request_data.client_tag_hash;
-  CommitResponseData response_data;
-
-  if (request_data.base_version == 0) {
-    // Server assigns new ID to newly committed items.
-    DCHECK(request_data.id.empty());
-    response_data.id = request_data.id;
-  } else {
-    // Otherwise we reuse the ID from the request.
-    response_data.id = GenerateId(client_tag_hash);
-  }
-
-  response_data.client_tag_hash = client_tag_hash;
-  response_data.sequence_number = request_data.sequence_number;
-
-  // Increment the server version on successful commit.
-  int64 version = GetServerVersion(client_tag_hash);
-  version++;
-  SetServerVersion(client_tag_hash, version);
-
-  response_data.response_version = version;
-
   CommitResponseDataList list;
-  list.push_back(response_data);
-
+  list.push_back(mock_processor_core_->SuccessfulCommitResponse(request_data));
   processor_->OnCommitCompletion(data_type_state_, list);
 }
 
-int64 NonBlockingTypeProcessorTest::GetServerVersion(const std::string& tag) {
-  std::map<const std::string, int64>::const_iterator it;
-  it = server_versions_.find(tag);
-  if (it == server_versions_.end()) {
-    return 0;
-  } else {
-    return it->second;
-  }
-}
-
-void NonBlockingTypeProcessorTest::SetServerVersion(const std::string& tag_hash,
-                                                    int64 version) {
-  server_versions_[tag_hash] = version;
-}
-
-std::string NonBlockingTypeProcessorTest::GenerateId(
-    const std::string& tag) const {
-  return "FakeId:" + tag;
-}
-
 std::string NonBlockingTypeProcessorTest::GenerateTagHash(
-    const std::string& tag) const {
-  return syncable::GenerateSyncableHash(type_, tag);
+    const std::string& tag) {
+  return syncable::GenerateSyncableHash(kModelType, tag);
 }
 
 sync_pb::EntitySpecifics NonBlockingTypeProcessorTest::GenerateSpecifics(
     const std::string& tag,
-    const std::string& value) const {
+    const std::string& value) {
   sync_pb::EntitySpecifics specifics;
   specifics.mutable_preference()->set_name(tag);
   specifics.mutable_preference()->set_value(value);
@@ -346,63 +180,24 @@ sync_pb::EntitySpecifics NonBlockingTypeProcessorTest::GenerateSpecifics(
 }
 
 size_t NonBlockingTypeProcessorTest::GetNumCommitRequestLists() {
-  return mock_processor_core_->commit_request_lists_.size();
+  return mock_processor_core_->GetNumCommitRequestLists();
 }
 
 CommitRequestDataList NonBlockingTypeProcessorTest::GetNthCommitRequestList(
     size_t n) {
-  DCHECK_LT(n, GetNumCommitRequestLists());
-  return mock_processor_core_->commit_request_lists_[n];
+  return mock_processor_core_->GetNthCommitRequestList(n);
 }
 
 bool NonBlockingTypeProcessorTest::HasCommitRequestForTag(
     const std::string& tag) {
   const std::string tag_hash = GenerateTagHash(tag);
-  const std::vector<CommitRequestDataList>& lists =
-      mock_processor_core_->commit_request_lists_;
-
-  // Iterate backward through the sets of commit requests to find the most
-  // recent one that applies to the specified tag.
-  for (std::vector<CommitRequestDataList>::const_reverse_iterator lists_it =
-           lists.rbegin();
-       lists_it != lists.rend();
-       ++lists_it) {
-    for (CommitRequestDataList::const_iterator it = lists_it->begin();
-         it != lists_it->end();
-         ++it) {
-      if (it->client_tag_hash == tag_hash) {
-        return true;
-      }
-    }
-  }
-
-  return false;
+  return mock_processor_core_->HasCommitRequestForTagHash(tag_hash);
 }
 
 CommitRequestData NonBlockingTypeProcessorTest::GetLatestCommitRequestForTag(
     const std::string& tag) {
   const std::string tag_hash = GenerateTagHash(tag);
-  const std::vector<CommitRequestDataList>& lists =
-      mock_processor_core_->commit_request_lists_;
-
-  // Iterate backward through the sets of commit requests to find the most
-  // recent one that applies to the specified tag.
-  for (std::vector<CommitRequestDataList>::const_reverse_iterator lists_it =
-           lists.rbegin();
-       lists_it != lists.rend();
-       ++lists_it) {
-    for (CommitRequestDataList::const_iterator it = lists_it->begin();
-         it != lists_it->end();
-         ++it) {
-      if (it->client_tag_hash == tag_hash) {
-        return *it;
-      }
-    }
-  }
-
-  NOTREACHED() << "Could not find any commits for given tag " << tag << ". "
-               << "Test should have checked HasCommitRequestForTag() first.";
-  return CommitRequestData();
+  return mock_processor_core_->GetLatestCommitRequestForTagHash(tag_hash);
 }
 
 // Creates a new item locally.