Make client tag IDs consistent in sync fake server

Modifies the sync fake server so it derives the IDs for client-tagged
items based on their model type and unique client tag.

This fixes a race where two clients could simultaneously commit two
client-tagged items with version == 0, and the two committed items would
be assigned different IDs.  The correct behavior, and that of the real
sync server, is to assign the same IDs to items that have the same
client tags.

BUG=396859

Review URL: https://codereview.chromium.org/414953002

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

4 files changed, 19 insertions, 31 deletions

diff --git a/sync/test/fake_server/fake_server.cc b/sync/test/fake_server/fake_server.cc
index b73ef1a..854eabe 100644
--- a/sync/test/fake_server/fake_server.cc
+++ b/sync/test/fake_server/fake_server.cc
@@ -336,13 +336,7 @@ string FakeServer::CommitEntity(
         client_entity,
         entities_[client_entity.id_string()]);
   } else if (client_entity.has_client_defined_unique_tag()) {
-    if (entities_.find(client_entity.id_string()) != entities_.end()) {
-      entity = UniqueClientEntity::CreateUpdatedVersion(
-          client_entity,
-          entities_[client_entity.id_string()]);
-    } else {
-      entity = UniqueClientEntity::CreateNew(client_entity);
-    }
+    entity = UniqueClientEntity::Create(client_entity);
   } else {
     // TODO(pvalenzuela): Validate entity's parent ID.
     if (entities_.find(client_entity.id_string()) != entities_.end()) {
diff --git a/sync/test/fake_server/fake_server_entity.cc b/sync/test/fake_server/fake_server_entity.cc
index 602a266..f3fc604 100644
--- a/sync/test/fake_server/fake_server_entity.cc
+++ b/sync/test/fake_server/fake_server_entity.cc
@@ -27,7 +27,11 @@ using std::vector;
 using syncer::ModelType;
 
 // The separator used when formatting IDs.
-const char kIdSeparator[] = "/";
+//
+// We chose the underscore character because it doesn't conflict with the
+// special characters used by base/base64.h's encoding, which is also used in
+// the construction of some IDs.
+const char kIdSeparator[] = "_";
 
 namespace fake_server {
 
diff --git a/sync/test/fake_server/unique_client_entity.cc b/sync/test/fake_server/unique_client_entity.cc
index 39d3a4c..11b37d5 100644
--- a/sync/test/fake_server/unique_client_entity.cc
+++ b/sync/test/fake_server/unique_client_entity.cc
@@ -22,15 +22,13 @@ namespace fake_server {
 UniqueClientEntity::~UniqueClientEntity() { }
 
 // static
-FakeServerEntity* UniqueClientEntity::CreateNew(
+FakeServerEntity* UniqueClientEntity::Create(
     const sync_pb::SyncEntity& client_entity) {
   CHECK(client_entity.has_client_defined_unique_tag())
       << "A UniqueClientEntity must have a client-defined unique tag.";
   ModelType model_type =
       syncer::GetModelTypeFromSpecifics(client_entity.specifics());
-  string id = client_entity.version() == 0 ?
-      FakeServerEntity::CreateId(model_type, base::GenerateGUID()) :
-      client_entity.id_string();
+  string id = EffectiveIdForClientTaggedEntity(client_entity);
   return new UniqueClientEntity(id,
                                 model_type,
                                 client_entity.version(),
@@ -42,17 +40,11 @@ FakeServerEntity* UniqueClientEntity::CreateNew(
 }
 
 // static
-FakeServerEntity* UniqueClientEntity::CreateUpdatedVersion(
-    const sync_pb::SyncEntity& client_entity,
-    FakeServerEntity* current_server_entity) {
-  return new UniqueClientEntity(client_entity.id_string(),
-                                current_server_entity->GetModelType(),
-                                client_entity.version(),
-                                client_entity.name(),
-                                client_entity.client_defined_unique_tag(),
-                                client_entity.specifics(),
-                                client_entity.ctime(),
-                                client_entity.mtime());
+std::string UniqueClientEntity::EffectiveIdForClientTaggedEntity(
+    const sync_pb::SyncEntity& entity) {
+  return FakeServerEntity::CreateId(
+      syncer::GetModelTypeFromSpecifics(entity.specifics()),
+      entity.client_defined_unique_tag());
 }
 
 UniqueClientEntity::UniqueClientEntity(
diff --git a/sync/test/fake_server/unique_client_entity.h b/sync/test/fake_server/unique_client_entity.h
index fecf044..56fa465 100644
--- a/sync/test/fake_server/unique_client_entity.h
+++ b/sync/test/fake_server/unique_client_entity.h
@@ -19,14 +19,12 @@ class UniqueClientEntity : public FakeServerEntity {
  public:
   virtual ~UniqueClientEntity();
 
-  // Factory function for UniqueClientEntity.
-  static FakeServerEntity* CreateNew(const sync_pb::SyncEntity& client_entity);
-
-  // Factory function for creating a new version of an existing
-  // UniqueClientEntity.
-  static FakeServerEntity* CreateUpdatedVersion(
-      const sync_pb::SyncEntity& client_entity,
-      FakeServerEntity* current_server_entity);
+  // Factory function for creating a UniqueClientEntity.
+  static FakeServerEntity* Create(const sync_pb::SyncEntity& client_entity);
+
+  // Derives an ID from a unique client tagged entity.
+  static std::string EffectiveIdForClientTaggedEntity(
+      const sync_pb::SyncEntity& entity);
 
   // FakeServerEntity implementation.
   virtual std::string GetParentId() const OVERRIDE;