sync: Move unique_client_tag hashing code

This is one of many changes that will lead up to the introduction of
the bookmark UniquePosition algorithm.

We eventually want to use the hashing code to create a unique tag for
bookmarks.  This tag will be initialized at bookmark creation time in
MutableEntry.  This patch moves the hashing code out of BaseNode and
into syncable_util so it can be accessed from the syncable layer.

BUG=145412,126505


Review URL: https://chromiumcodereview.appspot.com/11571092

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@174449 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 74 insertions, 53 deletions

diff --git a/sync/internal_api/base_node.cc b/sync/internal_api/base_node.cc
index 19de572..f640532 100644
--- a/sync/internal_api/base_node.cc
+++ b/sync/internal_api/base_node.cc
@@ -6,8 +6,6 @@
 
 #include <stack>
 
-#include "base/base64.h"
-#include "base/sha1.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
@@ -65,21 +63,6 @@ BaseNode::BaseNode() : password_data_(new sync_pb::PasswordSpecificsData) {}
 
 BaseNode::~BaseNode() {}
 
-std::string BaseNode::GenerateSyncableHash(
-    ModelType model_type, const std::string& client_tag) {
-  // Blank PB with just the field in it has termination symbol,
-  // handy for delimiter.
-  sync_pb::EntitySpecifics serialized_type;
-  AddDefaultFieldValue(model_type, &serialized_type);
-  std::string hash_input;
-  serialized_type.AppendToString(&hash_input);
-  hash_input.append(client_tag);
-
-  std::string encode_output;
-  CHECK(base::Base64Encode(base::SHA1HashString(hash_input), &encode_output));
-  return encode_output;
-}
-
 bool BaseNode::DecryptIfNecessary() {
   if (!GetEntry()->Get(syncable::UNIQUE_SERVER_TAG).empty())
       return true;  // Ignore unique folders.
diff --git a/sync/internal_api/public/base_node.h b/sync/internal_api/public/base_node.h
index 8afbeaa..b3946fb 100644
--- a/sync/internal_api/public/base_node.h
+++ b/sync/internal_api/public/base_node.h
@@ -200,11 +200,6 @@ class SYNC_EXPORT BaseNode {
  protected:
   BaseNode();
   virtual ~BaseNode();
-  // The server has a size limit on client tags, so we generate a fixed length
-  // hash locally. This also ensures that ModelTypes have unique namespaces.
-  static std::string GenerateSyncableHash(
-      ModelType model_type,
-      const std::string& client_tag);
 
   // Determines whether part of the entry is encrypted, and if so attempts to
   // decrypt it. Unless decryption is necessary and fails, this will always
diff --git a/sync/internal_api/read_node.cc b/sync/internal_api/read_node.cc
index 2873b0d..5419665 100644
--- a/sync/internal_api/read_node.cc
+++ b/sync/internal_api/read_node.cc
@@ -8,6 +8,7 @@
 #include "sync/internal_api/public/base_transaction.h"
 #include "sync/syncable/entry.h"
 #include "sync/syncable/syncable_base_transaction.h"
+#include "sync/syncable/syncable_util.h"
 
 namespace syncer {
 
@@ -57,7 +58,7 @@ BaseNode::InitByLookupResult ReadNode::InitByClientTagLookup(
   if (tag.empty())
     return INIT_FAILED_PRECONDITION;
 
-  const std::string hash = GenerateSyncableHash(model_type, tag);
+  const std::string hash = syncable::GenerateSyncableHash(model_type, tag);
 
   entry_ = new syncable::Entry(transaction_->GetWrappedTrans(),
                                syncable::GET_BY_CLIENT_TAG, hash);
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index d27b56e..b4ca259 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -63,6 +63,7 @@
 #include "sync/syncable/nigori_util.h"
 #include "sync/syncable/syncable_id.h"
 #include "sync/syncable/syncable_read_transaction.h"
+#include "sync/syncable/syncable_util.h"
 #include "sync/syncable/syncable_write_transaction.h"
 #include "sync/test/callback_counter.h"
 #include "sync/test/engine/fake_sync_scheduler.h"
@@ -279,22 +280,6 @@ TEST_F(SyncApiTest, BasicTagWrite) {
   }
 }
 
-TEST_F(SyncApiTest, GenerateSyncableHash) {
-  EXPECT_EQ("OyaXV5mEzrPS4wbogmtKvRfekAI=",
-      BaseNode::GenerateSyncableHash(BOOKMARKS, "tag1"));
-  EXPECT_EQ("iNFQtRFQb+IZcn1kKUJEZDDkLs4=",
-      BaseNode::GenerateSyncableHash(PREFERENCES, "tag1"));
-  EXPECT_EQ("gO1cPZQXaM73sHOvSA+tKCKFs58=",
-      BaseNode::GenerateSyncableHash(AUTOFILL, "tag1"));
-
-  EXPECT_EQ("A0eYIHXM1/jVwKDDp12Up20IkKY=",
-      BaseNode::GenerateSyncableHash(BOOKMARKS, "tag2"));
-  EXPECT_EQ("XYxkF7bhS4eItStFgiOIAU23swI=",
-      BaseNode::GenerateSyncableHash(PREFERENCES, "tag2"));
-  EXPECT_EQ("GFiWzo5NGhjLlN+OyCfhy28DJTQ=",
-      BaseNode::GenerateSyncableHash(AUTOFILL, "tag2"));
-}
-
 TEST_F(SyncApiTest, ModelTypesSiloed) {
   {
     WriteTransaction trans(FROM_HERE, test_user_share_.user_share());
@@ -956,7 +941,7 @@ class SyncManagerTest : public testing::Test,
     UserShare* share = sync_manager_.GetUserShare();
     syncable::WriteTransaction trans(
         FROM_HERE, syncable::UNITTEST, share->directory.get());
-    const std::string hash = BaseNode::GenerateSyncableHash(type, client_tag);
+    const std::string hash = syncable::GenerateSyncableHash(type, client_tag);
     syncable::MutableEntry entry(&trans, syncable::GET_BY_CLIENT_TAG,
                                  hash);
     EXPECT_TRUE(entry.good());
@@ -2197,7 +2182,7 @@ TEST_F(SyncManagerTest, UpdateEntryWithEncryption) {
   entity_specifics.mutable_bookmark()->set_url("url");
   entity_specifics.mutable_bookmark()->set_title("title");
   MakeServerNode(sync_manager_.GetUserShare(), BOOKMARKS, client_tag,
-                 BaseNode::GenerateSyncableHash(BOOKMARKS,
+                 syncable::GenerateSyncableHash(BOOKMARKS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
@@ -2346,7 +2331,7 @@ TEST_F(SyncManagerTest, UpdatePasswordSetEntitySpecificsNoChange) {
             mutable_encrypted());
   }
   MakeServerNode(sync_manager_.GetUserShare(), PASSWORDS, client_tag,
-                 BaseNode::GenerateSyncableHash(PASSWORDS,
+                 syncable::GenerateSyncableHash(PASSWORDS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
@@ -2381,7 +2366,7 @@ TEST_F(SyncManagerTest, UpdatePasswordSetPasswordSpecifics) {
             mutable_encrypted());
   }
   MakeServerNode(sync_manager_.GetUserShare(), PASSWORDS, client_tag,
-                 BaseNode::GenerateSyncableHash(PASSWORDS,
+                 syncable::GenerateSyncableHash(PASSWORDS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
@@ -2432,7 +2417,7 @@ TEST_F(SyncManagerTest, UpdatePasswordNewPassphrase) {
         entity_specifics.mutable_password()->mutable_encrypted());
   }
   MakeServerNode(sync_manager_.GetUserShare(), PASSWORDS, client_tag,
-                 BaseNode::GenerateSyncableHash(PASSWORDS,
+                 syncable::GenerateSyncableHash(PASSWORDS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
@@ -2471,7 +2456,7 @@ TEST_F(SyncManagerTest, UpdatePasswordReencryptEverything) {
         entity_specifics.mutable_password()->mutable_encrypted());
   }
   MakeServerNode(sync_manager_.GetUserShare(), PASSWORDS, client_tag,
-                 BaseNode::GenerateSyncableHash(PASSWORDS,
+                 syncable::GenerateSyncableHash(PASSWORDS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
@@ -2495,7 +2480,7 @@ TEST_F(SyncManagerTest, SetBookmarkTitle) {
   entity_specifics.mutable_bookmark()->set_url("url");
   entity_specifics.mutable_bookmark()->set_title("title");
   MakeServerNode(sync_manager_.GetUserShare(), BOOKMARKS, client_tag,
-                 BaseNode::GenerateSyncableHash(BOOKMARKS,
+                 syncable::GenerateSyncableHash(BOOKMARKS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
@@ -2531,7 +2516,7 @@ TEST_F(SyncManagerTest, SetBookmarkTitleWithEncryption) {
   entity_specifics.mutable_bookmark()->set_url("url");
   entity_specifics.mutable_bookmark()->set_title("title");
   MakeServerNode(sync_manager_.GetUserShare(), BOOKMARKS, client_tag,
-                 BaseNode::GenerateSyncableHash(BOOKMARKS,
+                 syncable::GenerateSyncableHash(BOOKMARKS,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
@@ -2590,7 +2575,7 @@ TEST_F(SyncManagerTest, SetNonBookmarkTitle) {
   MakeServerNode(sync_manager_.GetUserShare(),
                  PREFERENCES,
                  client_tag,
-                 BaseNode::GenerateSyncableHash(PREFERENCES,
+                 syncable::GenerateSyncableHash(PREFERENCES,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
@@ -2628,7 +2613,7 @@ TEST_F(SyncManagerTest, SetNonBookmarkTitleWithEncryption) {
   MakeServerNode(sync_manager_.GetUserShare(),
                  PREFERENCES,
                  client_tag,
-                 BaseNode::GenerateSyncableHash(PREFERENCES,
+                 syncable::GenerateSyncableHash(PREFERENCES,
                                                 client_tag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
@@ -2699,7 +2684,7 @@ TEST_F(SyncManagerTest, SetPreviouslyEncryptedSpecifics) {
     AddDefaultFieldValue(BOOKMARKS, &entity_specifics);
   }
   MakeServerNode(sync_manager_.GetUserShare(), BOOKMARKS, client_tag,
-                 BaseNode::GenerateSyncableHash(BOOKMARKS,
+                 syncable::GenerateSyncableHash(BOOKMARKS,
                                                 client_tag),
                  entity_specifics);
 
@@ -2764,7 +2749,7 @@ TEST_F(SyncManagerTest, IncrementTransactionVersion) {
   entity_specifics.mutable_bookmark()->set_url("url");
   entity_specifics.mutable_bookmark()->set_title("title");
   MakeServerNode(sync_manager_.GetUserShare(), BOOKMARKS, client_tag,
-                 BaseNode::GenerateSyncableHash(BOOKMARKS,
+                 syncable::GenerateSyncableHash(BOOKMARKS,
                                                 client_tag),
                  entity_specifics);
 
diff --git a/sync/internal_api/write_node.cc b/sync/internal_api/write_node.cc
index 6f70b3d..0c1c2ad 100644
--- a/sync/internal_api/write_node.cc
+++ b/sync/internal_api/write_node.cc
@@ -19,6 +19,7 @@
 #include "sync/protocol/typed_url_specifics.pb.h"
 #include "sync/syncable/mutable_entry.h"
 #include "sync/syncable/nigori_util.h"
+#include "sync/syncable/syncable_util.h"
 #include "sync/util/cryptographer.h"
 
 using std::string;
@@ -292,7 +293,7 @@ BaseNode::InitByLookupResult WriteNode::InitByClientTagLookup(
   if (tag.empty())
     return INIT_FAILED_PRECONDITION;
 
-  const std::string hash = GenerateSyncableHash(model_type, tag);
+  const std::string hash = syncable::GenerateSyncableHash(model_type, tag);
 
   entry_ = new syncable::MutableEntry(transaction_->GetWrappedWriteTrans(),
                                       syncable::GET_BY_CLIENT_TAG, hash);
@@ -380,7 +381,7 @@ WriteNode::InitUniqueByCreationResult WriteNode::InitUniqueByCreation(
     return INIT_FAILED_EMPTY_TAG;
   }
 
-  const std::string hash = GenerateSyncableHash(model_type, tag);
+  const std::string hash = syncable::GenerateSyncableHash(model_type, tag);
 
   syncable::Id parent_id = parent.GetEntry()->Get(syncable::ID);
 
diff --git a/sync/sync.gyp b/sync/sync.gyp
index aacc629..17671bc 100644
--- a/sync/sync.gyp
+++ b/sync/sync.gyp
@@ -687,6 +687,7 @@
           'syncable/syncable_enum_conversions_unittest.cc',
           'syncable/syncable_id_unittest.cc',
           'syncable/syncable_unittest.cc',
+          'syncable/syncable_util_unittest.cc',
           'util/cryptographer_unittest.cc',
           'util/data_encryption_win_unittest.cc',
           'util/data_type_histogram_unittest.cc',
diff --git a/sync/syncable/syncable_util.cc b/sync/syncable/syncable_util.cc
index 36d08d0..0597109 100644
--- a/sync/syncable/syncable_util.cc
+++ b/sync/syncable/syncable_util.cc
@@ -4,8 +4,10 @@
 
 #include "sync/syncable/syncable_util.h"
 
+#include "base/base64.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/sha1.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/entry.h"
 #include "sync/syncable/mutable_entry.h"
@@ -102,5 +104,20 @@ bool SyncAssert(bool condition,
   return true;
 }
 
+std::string GenerateSyncableHash(
+    ModelType model_type, const std::string& client_tag) {
+  // Blank PB with just the field in it has termination symbol,
+  // handy for delimiter.
+  sync_pb::EntitySpecifics serialized_type;
+  AddDefaultFieldValue(model_type, &serialized_type);
+  std::string hash_input;
+  serialized_type.AppendToString(&hash_input);
+  hash_input.append(client_tag);
+
+  std::string encode_output;
+  CHECK(base::Base64Encode(base::SHA1HashString(hash_input), &encode_output));
+  return encode_output;
+}
+
 }  // namespace syncable
 }  // namespace syncer
diff --git a/sync/syncable/syncable_util.h b/sync/syncable/syncable_util.h
index 425621e..add3e2d 100644
--- a/sync/syncable/syncable_util.h
+++ b/sync/syncable/syncable_util.h
@@ -5,9 +5,11 @@
 #ifndef SYNC_SYNCABLE_SYNCABLE_UTIL_H_
 #define SYNC_SYNCABLE_SYNCABLE_UTIL_H_
 
+#include <string>
 #include <vector>
 
 #include "base/basictypes.h"
+#include "sync/internal_api/public/base/model_type.h"
 
 namespace tracked_objects {
 class Location;
@@ -35,6 +37,10 @@ bool SyncAssert(bool condition,
 int GetUnsyncedEntries(BaseTransaction* trans,
                        std::vector<int64> *handles);
 
+// Generates a fixed-length tag for the given string under the given model_type.
+std::string GenerateSyncableHash(
+    ModelType model_type, const std::string& client_tag);
+
 }  // namespace syncable
 }  // namespace syncer
 
diff --git a/sync/syncable/syncable_util_unittest.cc b/sync/syncable/syncable_util_unittest.cc
new file mode 100644
index 0000000..8f818f0
--- /dev/null
+++ b/sync/syncable/syncable_util_unittest.cc
@@ -0,0 +1,32 @@
+// Copyright 2012 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 "sync/internal_api/public/base/model_type.h"
+#include "sync/syncable/syncable_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer {
+namespace syncable {
+namespace {
+
+// Tests that the hashing algorithm has not changed.
+TEST(SyncableUtilTest, GenerateSyncableHash) {
+  EXPECT_EQ("OyaXV5mEzrPS4wbogmtKvRfekAI=",
+            GenerateSyncableHash(BOOKMARKS, "tag1"));
+  EXPECT_EQ("iNFQtRFQb+IZcn1kKUJEZDDkLs4=",
+            GenerateSyncableHash(PREFERENCES, "tag1"));
+  EXPECT_EQ("gO1cPZQXaM73sHOvSA+tKCKFs58=",
+            GenerateSyncableHash(AUTOFILL, "tag1"));
+
+  EXPECT_EQ("A0eYIHXM1/jVwKDDp12Up20IkKY=",
+            GenerateSyncableHash(BOOKMARKS, "tag2"));
+  EXPECT_EQ("XYxkF7bhS4eItStFgiOIAU23swI=",
+            GenerateSyncableHash(PREFERENCES, "tag2"));
+  EXPECT_EQ("GFiWzo5NGhjLlN+OyCfhy28DJTQ=",
+            GenerateSyncableHash(AUTOFILL, "tag2"));
+}
+
+}  // namespace
+}  // namespace syncer
+}  // namespace syncable