Use MetadataChangeList and EntityChangeList in SharedModelTypeProcessor

sync/internal_api/shared_model_type_processor.cc

Index: sync/internal_api/shared_model_type_processor.cc
diff --git a/sync/internal_api/shared_model_type_processor.cc b/sync/internal_api/shared_model_type_processor.cc
index a6ed683bfcb17dc93bb37882dbaa7eea20417461..f6573bec9e038b1960a020242688c8dd2343a030 100644
--- a/sync/internal_api/shared_model_type_processor.cc
+++ b/sync/internal_api/shared_model_type_processor.cc
@@ -151,16 +151,15 @@ void SharedModelTypeProcessor::OnConnect(scoped_ptr<CommitQueue> worker) {
 void SharedModelTypeProcessor::Put(const std::string& client_tag,
                                    scoped_ptr<EntityData> entity_data,
                                    MetadataChangeList* metadata_change_list) {
-  // TODO(skym): Add metadata to persist to MetadataChangeList, crbug/569636.
-
   DCHECK(entity_data.get());
   DCHECK(!entity_data->is_deleted());
   DCHECK(!entity_data->non_unique_name.empty());
   DCHECK_EQ(type_, syncer::GetModelTypeFromSpecifics(entity_data->specifics));
 
   // If the service specified an overriding hash, use that, otherwise generate
-  // one from the tag. TODO(skym): This behavior should be delayed, once
-  // crbug/561818 is fixed we will only perform this logic in the create case.
+  // one from the tag.
+  // TODO(skym): This behavior should be delayed, once crbug.com/561818 is fixed
+  // we will only perform this logic in the create case.
   const std::string client_tag_hash(
       entity_data->client_tag_hash.empty()
           ? syncer::syncable::GenerateSyncableHash(type_, client_tag)
@@ -169,9 +168,9 @@ void SharedModelTypeProcessor::Put(const std::string& client_tag,
   base::Time now = base::Time::Now();
 
   ModelTypeEntity* entity = nullptr;
-  // TODO(stanisc): crbug/561818: Search by client_tag rather than
+  // TODO(stanisc): crbug.com/561818: Search by client_tag rather than
   // client_tag_hash.
-  EntityMap::const_iterator it = entities_.find(client_tag_hash);
+  auto it = entities_.find(client_tag_hash);
   if (it == entities_.end()) {
     // The service is creating a new entity.
     scoped_ptr<ModelTypeEntity> scoped_entity = ModelTypeEntity::CreateNew(
@@ -182,32 +181,39 @@ void SharedModelTypeProcessor::Put(const std::string& client_tag,
   } else {
     // The service is updating an existing entity.
     entity = it->second.get();
+    DCHECK_EQ(client_tag, entity->client_key());
   }
 
+  // TODO(stanisc): crbug.com/561829: Avoid committing a change if there is no
+  // actual change.
   entity->MakeLocalChange(std::move(entity_data), now);
+  metadata_change_list->UpdateMetadata(client_tag, entity->metadata());
+
   FlushPendingCommitRequests();
 }
 
 void SharedModelTypeProcessor::Delete(
     const std::string& client_key,
     MetadataChangeList* metadata_change_list) {
-  // TODO(skym): Add metadata to persist to MetadataChangeList, crbug/569636.
-
   const std::string client_tag_hash(
       syncer::syncable::GenerateSyncableHash(type_, client_key));
 
-  // TODO(skym): crbug/561818: Search by client_tag rather than client_tag_hash.
-  EntityMap::const_iterator it = entities_.find(client_tag_hash);
+  // TODO(skym): crbug.com/561818: Search by client_tag rather than
+  // client_tag_hash.
+  auto it = entities_.find(client_tag_hash);
   if (it == entities_.end()) {
     // That's unusual, but not necessarily a bad thing.
     // Missing is as good as deleted as far as the model is concerned.
     DLOG(WARNING) << "Attempted to delete missing item."
                   << " client tag: " << client_key;
-  } else {
-    ModelTypeEntity* entity = it->second.get();
-    entity->Delete();
+    return;
   }
 
+  ModelTypeEntity* entity = it->second.get();
+  entity->Delete();
+
+  metadata_change_list->UpdateMetadata(client_key, entity->metadata());
+
   FlushPendingCommitRequests();
 }
 
@@ -223,8 +229,7 @@ void SharedModelTypeProcessor::FlushPendingCommitRequests() {
     return;
 
   // TODO(rlarocque): Do something smarter than iterate here.
-  for (EntityMap::const_iterator it = entities_.begin(); it != entities_.end();
-       ++it) {
+  for (auto it = entities_.begin(); it != entities_.end(); ++it) {
     if (it->second->RequiresCommitRequest()) {
       CommitRequestData request;
       it->second->InitializeCommitRequestData(&request);
@@ -240,14 +245,18 @@ void SharedModelTypeProcessor::FlushPendingCommitRequests() {
 void SharedModelTypeProcessor::OnCommitCompleted(
     const DataTypeState& type_state,
     const CommitResponseDataList& response_list) {
+  scoped_ptr<MetadataChangeList> change_list =
+      service_->CreateMetadataChangeList();
+
   data_type_state_ = type_state;
+  change_list->UpdateDataTypeState(data_type_state_);
 
-  for (CommitResponseDataList::const_iterator list_it = response_list.begin();
-       list_it != response_list.end(); ++list_it) {
+  for (auto list_it = response_list.begin(); list_it != response_list.end();
+       ++list_it) {
     const CommitResponseData& response_data = *list_it;
     const std::string& client_tag_hash = response_data.client_tag_hash;
 
-    EntityMap::const_iterator it = entities_.find(client_tag_hash);
+    auto it = entities_.find(client_tag_hash);
     if (it == entities_.end()) {
       NOTREACHED() << "Received commit response for missing item."
                    << " type: " << type_ << " client_tag: " << client_tag_hash;
@@ -256,21 +265,35 @@ void SharedModelTypeProcessor::OnCommitCompleted(
       it->second->ReceiveCommitResponse(
           response_data.id, response_data.sequence_number,
           response_data.response_version, data_type_state_.encryption_key_name);
+      // TODO(stanisc): crbug.com/573333: Delete case.
+      // This might be the right place to clear a metadata entry that has
+      // been deleted locally and confirmed deleted by the server.
+      change_list->UpdateMetadata(it->second->client_key(),
+                                  it->second->metadata());
     }
   }
+
+  // TODO(stanisc): What is the right method to submit metadata changes to the
+  // service? Is using empty EntityChangeList OK?
+  // TODO(stanisc): crbug.com/570085: Error handling.
+  service_->ApplySyncChanges(std::move(change_list), EntityChangeList());
 }
 
 void SharedModelTypeProcessor::OnUpdateReceived(
     const DataTypeState& data_type_state,
     const UpdateResponseDataList& response_list,
     const UpdateResponseDataList& pending_updates) {
+  scoped_ptr<MetadataChangeList> metadata_changes =
+      service_->CreateMetadataChangeList();
+  EntityChangeList entity_changes;
+
+  metadata_changes->UpdateDataTypeState(data_type_state);
   bool got_new_encryption_requirements = data_type_state_.encryption_key_name !=
                                          data_type_state.encryption_key_name;
-
   data_type_state_ = data_type_state;
 
-  for (UpdateResponseDataList::const_iterator list_it = response_list.begin();
-       list_it != response_list.end(); ++list_it) {
+  for (auto list_it = response_list.begin(); list_it != response_list.end();
+       ++list_it) {
     const UpdateResponseData& response_data = *list_it;
     const EntityData& data = response_data.entity.value();
     const std::string& client_tag_hash = data.client_tag_hash;
@@ -280,24 +303,47 @@ void SharedModelTypeProcessor::OnUpdateReceived(
     pending_updates_map_.erase(client_tag_hash);
 
     ModelTypeEntity* entity = nullptr;
-    EntityMap::const_iterator it = entities_.find(client_tag_hash);
+    auto it = entities_.find(client_tag_hash);
     if (it == entities_.end()) {
+      if (data.is_deleted()) {
+        DLOG(WARNING) << "Received remote delete for a non-existing item."
+                      << " client_tag_hash: " << client_tag_hash;
+        continue;
+      }
+
       // Let the service define |client_tag| based on the entity data.
-      std::string client_tag = service_->GetClientTag(data);
+      std::string client_key = service_->GetClientTag(data);
 
       scoped_ptr<ModelTypeEntity> scoped_entity = ModelTypeEntity::CreateNew(
-          client_tag, client_tag_hash, data.id, data.creation_time);
+          client_key, client_tag_hash, data.id, data.creation_time);
       entity = scoped_entity.get();
       entities_.insert(
           std::make_pair(client_tag_hash, std::move(scoped_entity)));
 
+      entity_changes.push_back(
+          EntityChange::CreateAdd(client_key, response_data.entity));
+
     } else {
       entity = it->second.get();
+      if (data.is_deleted()) {
+        entity_changes.push_back(
+            EntityChange::CreateDelete(entity->client_key()));
+      } else {
+        // TODO(stanisc): crbug.com/561829: Avoid sending an update to the
+        // service if there is no actual change.
+        entity_changes.push_back(EntityChange::CreateUpdate(
+            entity->client_key(), response_data.entity));
+      }
     }
 
     entity->ApplyUpdateFromServer(response_data);
+    // TODO(stanisc): crbug.com/573333: Delete case.
+    // This might be the right place to clear metadata entry instead of
+    // updating it.
+    metadata_changes->UpdateMetadata(entity->client_key(), entity->metadata());
 
-    // TODO(stanisc): Do something special when conflicts are detected.
+    // TODO(stanisc): crbug.com/521867: Do something special when conflicts are
+    // detected.
 
     // If the received entity has out of date encryption, we schedule another
     // commit to fix it.
@@ -306,20 +352,20 @@ void SharedModelTypeProcessor::OnUpdateReceived(
       DVLOG(2) << ModelTypeToString(type_) << ": Requesting re-encrypt commit "
                << response_data.encryption_key_name << " -> "
                << data_type_state_.encryption_key_name;
-      EntityMap::const_iterator it2 = entities_.find(client_tag_hash);
+      auto it2 = entities_.find(client_tag_hash);
       it2->second->UpdateDesiredEncryptionKey(
           data_type_state_.encryption_key_name);
     }
   }
 
+  // TODO: crbug.com/529498: stop saving pending updates.
   // Save pending updates in the appropriate data structure.
-  for (UpdateResponseDataList::const_iterator list_it = pending_updates.begin();
-       list_it != pending_updates.end(); ++list_it) {
+  for (auto list_it = pending_updates.begin(); list_it != pending_updates.end();
+       ++list_it) {
     const UpdateResponseData& update = *list_it;
     const std::string& client_tag_hash = update.entity->client_tag_hash;
 
-    UpdateMap::const_iterator lookup_it =
-        pending_updates_map_.find(client_tag_hash);
+    auto lookup_it = pending_updates_map_.find(client_tag_hash);
     if (lookup_it == pending_updates_map_.end()) {
       pending_updates_map_.insert(std::make_pair(
           client_tag_hash, make_scoped_ptr(new UpdateResponseData(update))));
@@ -333,43 +379,43 @@ void SharedModelTypeProcessor::OnUpdateReceived(
   }
 
   if (got_new_encryption_requirements) {
-    for (EntityMap::const_iterator it = entities_.begin();
-         it != entities_.end(); ++it) {
+    for (auto it = entities_.begin(); it != entities_.end(); ++it) {
       it->second->UpdateDesiredEncryptionKey(
           data_type_state_.encryption_key_name);
     }
   }
 
+  // Inform the service of the new or updated data.
+  // TODO(stanisc): crbug.com/570085: Error handling.
+  service_->ApplySyncChanges(std::move(metadata_changes), entity_changes);
+
   // We may have new reasons to commit by the time this function is done.
   FlushPendingCommitRequests();
-
-  // TODO(rlarocque): Inform the model of the new or updated data.
-  // TODO(rlarocque): Persist the new data on disk.
 }
 
 UpdateResponseDataList SharedModelTypeProcessor::GetPendingUpdates() {
   UpdateResponseDataList pending_updates_list;
-  for (UpdateMap::const_iterator it = pending_updates_map_.begin();
-       it != pending_updates_map_.end(); ++it) {
+  for (auto it = pending_updates_map_.begin(); it != pending_updates_map_.end();
+       ++it) {
     pending_updates_list.push_back(*it->second);
   }
   return pending_updates_list;
 }
 
 void SharedModelTypeProcessor::ClearTransientSyncState() {
-  for (EntityMap::const_iterator it = entities_.begin(); it != entities_.end();
-       ++it) {
+  for (auto it = entities_.begin(); it != entities_.end(); ++it) {
     it->second->ClearTransientSyncState();
   }
 }
 
 void SharedModelTypeProcessor::ClearSyncState() {
-  for (EntityMap::const_iterator it = entities_.begin(); it != entities_.end();
-       ++it) {
+  for (auto it = entities_.begin(); it != entities_.end(); ++it) {
     it->second->ClearSyncState();
   }
   pending_updates_map_.clear();
   data_type_state_ = DataTypeState();
+  // TODO(stanisc): crbug.com/561830, crbug.com/573333: Update the service to
+  // let it know that all metadata need to be cleared from the storage.
 }
 
 }  // namespace syncer_v2