Change EntityData and EntityMetadata according to the design doc.

BUG=553638

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

Cr-Commit-Position: refs/heads/master@{#358993}

3 files changed, 135 insertions, 84 deletions

diff --git a/sync/api/entity_data.cc b/sync/api/entity_data.cc
index bfff4c7..fdb9b47 100644
--- a/sync/api/entity_data.cc
+++ b/sync/api/entity_data.cc
@@ -4,54 +4,38 @@
 
 #include "sync/api/entity_data.h"
 
-#include "sync/protocol/entity_metadata.pb.h"
-#include "sync/protocol/sync.pb.h"
-#include "sync/util/time.h"
+#include "base/logging.h"
 
 namespace syncer_v2 {
 
-EntityData::EntityData(const std::string& client_id,
-                       const std::string& non_unique_name,
-                       const sync_pb::EntitySpecifics* specifics,
-                       bool is_deleted)
-    : client_id_(client_id),
-      non_unique_name_(non_unique_name),
-      is_deleted_(is_deleted) {
-  if (!is_deleted) {
-    DCHECK(specifics);
-    specifics_.set_value(*specifics);
-  }
-}
-
+EntityData::EntityData() {}
 EntityData::~EntityData() {}
 
-// Static.
-EntityData EntityData::CreateData(const std::string& client_id,
-                                  const std::string& non_unique_name,
-                                  const sync_pb::EntitySpecifics& specifics) {
-  return EntityData(client_id, non_unique_name, &specifics, false);
-}
+void EntityData::Swap(EntityData* other) {
+  server_id.swap(other->server_id);
+  client_tag_hash.swap(other->client_tag_hash);
+  non_unique_name.swap(other->non_unique_name);
+
+  specifics.Swap(&other->specifics);
+
+  std::swap(creation_time, other->creation_time);
+  std::swap(modification_time, other->modification_time);
 
-// Static.
-EntityData EntityData::CreateDelete(const std::string& sync_tag,
-                                    const std::string& non_unique_name) {
-  return EntityData(sync_tag, non_unique_name, nullptr, true);
+  parent_id.swap(other->parent_id);
+  unique_position.Swap(&other->unique_position);
 }
 
-const sync_pb::EntitySpecifics& EntityData::specifics() const {
-  DCHECK(!is_deleted());
-  return specifics_.value();
+void EntityDataTraits::SwapValue(EntityData* dest, EntityData* src) {
+  dest->Swap(src);
 }
 
-bool EntityData::HasMetadata() const {
-  return metadata_.client_id() == client_id_;
+bool EntityDataTraits::HasValue(const EntityData& value) {
+  return !value.client_tag_hash.empty();
 }
 
-void EntityData::SetMetadata(const sync_pb::EntityMetadata& metadata) {
-  DCHECK(!HasMetadata());
-  DCHECK_EQ(client_id_, metadata.client_id());
-  modification_time_ = syncer::ProtoTimeToTime(metadata.modification_time());
-  metadata_ = metadata;
+const EntityData& EntityDataTraits::DefaultValue() {
+  CR_DEFINE_STATIC_LOCAL(EntityData, default_instance, ());
+  return default_instance;
 }
 
 }  // namespace syncer_v2
diff --git a/sync/api/entity_data.h b/sync/api/entity_data.h
index c5c2de1..5c966b9 100644
--- a/sync/api/entity_data.h
+++ b/sync/api/entity_data.h
@@ -8,70 +8,71 @@
 #include <string>
 #include <vector>
 
+#include "base/macros.h"
 #include "base/time/time.h"
 #include "sync/base/sync_export.h"
 #include "sync/internal_api/public/util/proto_value_ptr.h"
-#include "sync/protocol/entity_metadata.pb.h"
-
-namespace sync_pb {
-class EntityMetadata;
-class EntitySpecifics;
-}  // namespace sync_pb
+#include "sync/protocol/sync.pb.h"
 
 namespace syncer_v2 {
 
-// A light-weight container for sync entity data.
-//
-// EntityData objects either have specifics data or represent a deleted
-// entity that does not have specifics. This component is immutable.
-//
-// EntityData objects can also be modified by adding EntityMetadata to them,
-// which is stored in its serialized form for persistence to storage.
-class SYNC_EXPORT EntityData {
+// A light-weight container for sync entity data which represents either
+// local data created on the ModelTypeService side or remote data created
+// on ModelTypeWorker.
+// EntityData is supposed to be wrapped and passed by reference.
+struct SYNC_EXPORT EntityData {
  public:
+  EntityData();
   ~EntityData();
 
-  // Default copy and assign welcome.
+  // Typically this is a server assigned sync ID, although for a local change
+  // that represents a new entity this field might be either empty or contain
+  // a temporary client sync ID.
+  std::string server_id;
+
+  // A hash based on the client tag and model type.
+  // Used for various map lookups. Should always be available.
+  // Sent to the server as SyncEntity::client_defined_unique_tag.
+  std::string client_tag_hash;
+
+  // Entity name, used mostly for Debug purposes.
+  std::string non_unique_name;
+
+  // Model type specific sync data.
+  sync_pb::EntitySpecifics specifics;
 
-  static EntityData CreateData(const std::string& client_id,
-                               const std::string& non_unique_name,
-                               const sync_pb::EntitySpecifics& specifics);
+  // Entity creation and modification timestamps.
+  base::Time creation_time;
+  base::Time modification_time;
 
-  static EntityData CreateDelete(const std::string& client_id,
-                                 const std::string& non_unique_name);
+  // Sync ID of the parent entity. This is supposed to be set only for
+  // hierarchical datatypes (e.g. Bookmarks).
+  std::string parent_id;
 
-  const std::string& client_id() const { return client_id_; }
-  const std::string& non_unique_name() const { return non_unique_name_; }
-  bool is_deleted() const { return is_deleted_; }
-  const sync_pb::EntitySpecifics& specifics() const;
-  bool HasMetadata() const;
-  void SetMetadata(const sync_pb::EntityMetadata& metadata);
-  // Maybe this should be private with SMTP and ModelTypeWorker as friends?
-  const sync_pb::EntityMetadata& GetMetadata() const { return metadata_; }
+  // Unique position of an entity among its siblings. This is supposed to be
+  // set only for datatypes that support positioning (e.g. Bookmarks).
+  sync_pb::UniquePosition unique_position;
+
+  // True if EntityData represents deleted entity; otherwise false.
+  // Note that EntityData would be considered to represent a deletion if it
+  // specifics hasn't been set.
+  bool is_deleted() { return specifics.ByteSize() == 0; }
 
  private:
-  EntityData(const std::string& client_id,
-             const std::string& non_unique_name,
-             const sync_pb::EntitySpecifics* specifics,
-             bool is_deleted);
-
-  // Fields that are always present and don't change.
-  const std::string client_id_;
-  const std::string non_unique_name_;
-  syncer::syncable::EntitySpecificsPtr specifics_;
-  // TODO(maxbogue): This might be replaced by a ChangeType enum.
-  const bool is_deleted_;
-
-  // Present sometimes.
-  base::Time modification_time_;
-  sync_pb::EntityMetadata metadata_;
-
-  // Only set for bookmarks; should really be in bookmark specifics.
-  // std::string parent_tag_hash_;
-  // sync_pb::UniquePosition unique_position_;
+  friend struct EntityDataTraits;
+  // Used to transfer the data without copying.
+  void Swap(EntityData* other);
+
+  DISALLOW_COPY_AND_ASSIGN(EntityData);
+};
+
+struct SYNC_EXPORT EntityDataTraits {
+  static void SwapValue(EntityData* dest, EntityData* src);
+  static bool HasValue(const EntityData& value);
+  static const EntityData& DefaultValue();
 };
 
-typedef std::vector<EntityData> EntityDataList;
+typedef syncer::ProtoValuePtr<EntityData, EntityDataTraits> EntityDataPtr;
 
 }  // namespace syncer_v2
 
diff --git a/sync/api/entity_data_unittest.cc b/sync/api/entity_data_unittest.cc
new file mode 100644
index 0000000..b61f086
--- /dev/null
+++ b/sync/api/entity_data_unittest.cc
@@ -0,0 +1,66 @@
+// Copyright 2015 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/api/entity_data.h"
+
+#include "sync/internal_api/public/base/model_type.h"
+#include "sync/internal_api/public/base/unique_position.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer_v2 {
+
+class EntityDataTest : public testing::Test {
+ protected:
+  EntityDataTest() {}
+  ~EntityDataTest() override {}
+};
+
+TEST_F(EntityDataTest, IsDeleted) {
+  EntityData data;
+  EXPECT_TRUE(data.is_deleted());
+
+  syncer::AddDefaultFieldValue(syncer::BOOKMARKS, &data.specifics);
+  EXPECT_FALSE(data.is_deleted());
+}
+
+TEST_F(EntityDataTest, Swap) {
+  EntityData data;
+  syncer::AddDefaultFieldValue(syncer::BOOKMARKS, &data.specifics);
+  data.server_id = "server_id";
+  data.client_tag_hash = "client_tag_hash";
+  data.non_unique_name = "non_unique_name";
+  data.creation_time = base::Time::FromTimeT(10);
+  data.modification_time = base::Time::FromTimeT(20);
+  data.parent_id = "parent_id";
+
+  using syncer::UniquePosition;
+  UniquePosition unique_position =
+      UniquePosition::InitialPosition(UniquePosition::RandomSuffix());
+
+  unique_position.ToProto(&data.unique_position);
+
+  // Remember addresses of some data within EntitySpecific and UniquePosition
+  // to make sure that the underlying data isn't copied.
+  const sync_pb::BookmarkSpecifics* bookmark_specifics =
+      &data.specifics.bookmark();
+  const std::string* unique_position_value = &data.unique_position.value();
+
+  EntityDataPtr ptr;
+  ptr.swap_value(&data);
+
+  // Compare addresses of the data wrapped by EntityDataPtr to make sure that
+  // the underlying objects are exactly the same.
+  EXPECT_EQ(bookmark_specifics, &ptr->specifics.bookmark());
+  EXPECT_EQ(unique_position_value, &ptr->unique_position.value());
+
+  // Compare other fields.
+  EXPECT_EQ("server_id", ptr->server_id);
+  EXPECT_EQ("client_tag_hash", ptr->client_tag_hash);
+  EXPECT_EQ("non_unique_name", ptr->non_unique_name);
+  EXPECT_EQ("parent_id", ptr->parent_id);
+  EXPECT_EQ(base::Time::FromTimeT(10), ptr->creation_time);
+  EXPECT_EQ(base::Time::FromTimeT(20), ptr->modification_time);
+}
+
+}  // namespace syncer_v2