sync: Implement per-type commit interface

sync/engine/commit_util.cc

Index: sync/engine/commit_util.cc
diff --git a/sync/engine/process_commit_response_command.cc b/sync/engine/commit_util.cc
similarity index 56%
rename from sync/engine/process_commit_response_command.cc
rename to sync/engine/commit_util.cc
index 97f403f795f7f8750e0b77625cb489ee8b120135..dc6991c6690702f9ae05625eca61b87b423a3605 100644
--- a/sync/engine/process_commit_response_command.cc
+++ b/sync/engine/commit_util.cc
@@ -2,222 +2,194 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "sync/engine/process_commit_response_command.h"
+#include "sync/engine/commit_util.h"
 
-#include <cstddef>
+#include <limits>
 #include <set>
 #include <string>
 #include <vector>
 
-#include "base/basictypes.h"
-#include "base/location.h"
+#include "base/strings/string_util.h"
 #include "sync/engine/syncer_proto_util.h"
-#include "sync/engine/syncer_util.h"
 #include "sync/internal_api/public/base/unique_position.h"
+#include "sync/protocol/bookmark_specifics.pb.h"
+#include "sync/protocol/sync.pb.h"
 #include "sync/sessions/sync_session.h"
+#include "sync/syncable/directory.h"
 #include "sync/syncable/entry.h"
 #include "sync/syncable/model_neutral_mutable_entry.h"
-#include "sync/syncable/syncable_model_neutral_write_transaction.h"
+#include "sync/syncable/syncable_base_transaction.h"
+#include "sync/syncable/syncable_base_write_transaction.h"
+#include "sync/syncable/syncable_changes_version.h"
 #include "sync/syncable/syncable_proto_util.h"
-#include "sync/syncable/syncable_read_transaction.h"
 #include "sync/syncable/syncable_util.h"
 #include "sync/util/time.h"
 
 using std::set;
 using std::string;
 using std::vector;
-using sync_pb::CommitResponse;
 
 namespace syncer {
 
-using sessions::OrderedCommitSet;
-using sessions::StatusController;
 using sessions::SyncSession;
-using syncable::ModelNeutralWriteTransaction;
-using syncable::ModelNeutralMutableEntry;
 using syncable::Entry;
-using syncable::BASE_VERSION;
-using syncable::GET_BY_ID;
-using syncable::GET_BY_HANDLE;
-using syncable::ID;
 using syncable::IS_DEL;
-using syncable::IS_DIR;
 using syncable::IS_UNAPPLIED_UPDATE;
 using syncable::IS_UNSYNCED;
-using syncable::PARENT_ID;
-using syncable::SERVER_IS_DEL;
-using syncable::SERVER_PARENT_ID;
-using syncable::SERVER_VERSION;
-using syncable::SYNCER;
-using syncable::SYNCING;
-
-ProcessCommitResponseCommand::ProcessCommitResponseCommand(
-      const sessions::OrderedCommitSet& commit_set,
-      const sync_pb::ClientToServerMessage& commit_message,
-      const sync_pb::ClientToServerResponse& commit_response)
-  : commit_set_(commit_set),
-    commit_message_(commit_message),
-    commit_response_(commit_response) {
-}
-
-ProcessCommitResponseCommand::~ProcessCommitResponseCommand() {}
-
-SyncerError ProcessCommitResponseCommand::ExecuteImpl(SyncSession* session) {
-  syncable::Directory* dir = session->context()->directory();
-  StatusController* status = session->mutable_status_controller();
-  const CommitResponse& cr = commit_response_.commit();
-  const sync_pb::CommitMessage& commit_message = commit_message_.commit();
-
-  int transient_error_commits = 0;
-  int conflicting_commits = 0;
-  int error_commits = 0;
-  int successes = 0;
-
-  set<syncable::Id> deleted_folders;
-
-  { // Scope for ModelNeutralWriteTransaction.
-    ModelNeutralWriteTransaction trans(FROM_HERE, SYNCER, dir);
-    for (size_t i = 0; i < commit_set_.Size(); i++) {
-      CommitResponse::ResponseType response_type = ProcessSingleCommitResponse(
-          &trans,
-          cr.entryresponse(i),
-          commit_message.entries(i),
-          commit_set_.GetCommitHandleAt(i),
-          &deleted_folders);
-      switch (response_type) {
-        case CommitResponse::INVALID_MESSAGE:
-          ++error_commits;
-          break;
-        case CommitResponse::CONFLICT:
-          ++conflicting_commits;
-          status->increment_num_server_conflicts();
-          break;
-        case CommitResponse::SUCCESS:
-          // TODO(sync): worry about sync_rate_ rate calc?
-          ++successes;
-          if (commit_set_.GetModelTypeAt(i) == BOOKMARKS)
-            status->increment_num_successful_bookmark_commits();
-          status->increment_num_successful_commits();
-          break;
-        case CommitResponse::OVER_QUOTA:
-          // We handle over quota like a retry, which is same as transient.
-        case CommitResponse::RETRY:
-        case CommitResponse::TRANSIENT_ERROR:
-          ++transient_error_commits;
-          break;
-        default:
-          LOG(FATAL) << "Bad return from ProcessSingleCommitResponse";
-      }
-    }
-
-    MarkDeletedChildrenSynced(dir, &trans, &deleted_folders);
+using syncable::Id;
+using syncable::SPECIFICS;
+using syncable::UNIQUE_POSITION;
+
+namespace commit_util {
+
+void AddExtensionsActivityToMessage(
+    ExtensionsActivity* activity,
+    ExtensionsActivity::Records* extensions_activity_buffer,
+    sync_pb::CommitMessage* message) {
+  // This isn't perfect, since the set of extensions activity may not correlate
+  // exactly with the items being committed.  That's OK as long as we're looking
+  // for a rough estimate of extensions activity, not an precise mapping of
+  // which commits were triggered by which extension.
+  //
+  // We will push this list of extensions activity back into the
+  // ExtensionsActivityMonitor if this commit fails.  That's why we must keep a
+  // copy of these records in the session.
+  activity->GetAndClearRecords(extensions_activity_buffer);
+
+  const ExtensionsActivity::Records& records = *extensions_activity_buffer;
+  for (ExtensionsActivity::Records::const_iterator it =
+       records.begin();
+       it != records.end(); ++it) {
+    sync_pb::ChromiumExtensionsActivity* activity_message =
+        message->add_extensions_activity();
+    activity_message->set_extension_id(it->second.extension_id);
+    activity_message->set_bookmark_writes_since_last_commit(
+        it->second.bookmark_write_count);
   }
+}
 
-  int commit_count = static_cast<int>(commit_set_.Size());
-  if (commit_count == successes) {
-    return SYNCER_OK;
-  } else if (error_commits > 0) {
-    return SERVER_RETURN_UNKNOWN_ERROR;
-  } else if (transient_error_commits > 0) {
-    return SERVER_RETURN_TRANSIENT_ERROR;
-  } else if (conflicting_commits > 0) {
-    // This means that the server already has an item with this version, but
-    // we haven't seen that update yet.
-    //
-    // A well-behaved client should respond to this by proceeding to the
-    // download updates phase, fetching the conflicting items, then attempting
-    // to resolve the conflict.  That's not what this client does.
-    //
-    // We don't currently have any code to support that exceptional control
-    // flow.  Instead, we abort the current sync cycle and start a new one.  The
-    // end result is the same.
-    return SERVER_RETURN_CONFLICT;
-  } else {
-    LOG(FATAL) << "Inconsistent counts when processing commit response";
-    return SYNCER_OK;
+void AddClientConfigParamsToMessage(
+    ModelTypeSet enabled_types,
+    sync_pb::CommitMessage* message) {
+  sync_pb::ClientConfigParams* config_params = message->mutable_config_params();
+  for (ModelTypeSet::Iterator it = enabled_types.First(); it.Good(); it.Inc()) {
+    if (ProxyTypes().Has(it.Get()))
+      continue;
+    int field_number = GetSpecificsFieldNumberFromModelType(it.Get());
+    config_params->mutable_enabled_type_ids()->Add(field_number);
   }
+  config_params->set_tabs_datatype_enabled(
+      enabled_types.Has(syncer::PROXY_TABS));
 }
 
-void LogServerError(const sync_pb::CommitResponse_EntryResponse& res) {
-  if (res.has_error_message())
-    LOG(WARNING) << "  " << res.error_message();
-  else
-    LOG(WARNING) << "  No detailed error message returned from server";
-}
+namespace {
+void SetEntrySpecifics(const Entry& meta_entry,
+                       sync_pb::SyncEntity* sync_entry) {
+  // Add the new style extension and the folder bit.
+  sync_entry->mutable_specifics()->CopyFrom(meta_entry.GetSpecifics());
+  sync_entry->set_folder(meta_entry.GetIsDir());
 
-CommitResponse::ResponseType
-ProcessCommitResponseCommand::ProcessSingleCommitResponse(
-    syncable::ModelNeutralWriteTransaction* trans,
-    const sync_pb::CommitResponse_EntryResponse& server_entry,
-    const sync_pb::SyncEntity& commit_request_entry,
-    const int64 metahandle,
-    set<syncable::Id>* deleted_folders) {
-  ModelNeutralMutableEntry local_entry(trans, GET_BY_HANDLE, metahandle);
-  CHECK(local_entry.good());
-  bool syncing_was_set = local_entry.GetSyncing();
-  local_entry.PutSyncing(false);
-
-  CommitResponse::ResponseType response = (CommitResponse::ResponseType)
-      server_entry.response_type();
-  if (!CommitResponse::ResponseType_IsValid(response)) {
-    LOG(ERROR) << "Commit response has unknown response type! Possibly out "
-               "of date client?";
-    return CommitResponse::INVALID_MESSAGE;
-  }
-  if (CommitResponse::TRANSIENT_ERROR == response) {
-    DVLOG(1) << "Transient Error Committing: " << local_entry;
-    LogServerError(server_entry);
-    return CommitResponse::TRANSIENT_ERROR;
-  }
-  if (CommitResponse::INVALID_MESSAGE == response) {
-    LOG(ERROR) << "Error Commiting: " << local_entry;
-    LogServerError(server_entry);
-    return response;
-  }
-  if (CommitResponse::CONFLICT == response) {
-    DVLOG(1) << "Conflict Committing: " << local_entry;
-    return response;
+  CHECK(!sync_entry->specifics().password().has_client_only_encrypted_data());
+  DCHECK_EQ(meta_entry.GetModelType(), GetModelType(*sync_entry));
+}
+}  // namespace
+
+void BuildCommitItem(
+    const syncable::Entry& meta_entry,
+    sync_pb::SyncEntity* sync_entry) {
+  syncable::Id id = meta_entry.GetId();
+  sync_entry->set_id_string(SyncableIdToProto(id));
+
+  string name = meta_entry.GetNonUniqueName();
+  CHECK(!name.empty());  // Make sure this isn't an update.
+  // Note: Truncation is also performed in WriteNode::SetTitle(..). But this
+  // call is still necessary to handle any title changes that might originate
+  // elsewhere, or already be persisted in the directory.
+  TruncateUTF8ToByteSize(name, 255, &name);
+  sync_entry->set_name(name);
+
+  // Set the non_unique_name.  If we do, the server ignores
+  // the |name| value (using |non_unique_name| instead), and will return
+  // in the CommitResponse a unique name if one is generated.
+  // We send both because it may aid in logging.
+  sync_entry->set_non_unique_name(name);
+
+  if (!meta_entry.GetUniqueClientTag().empty()) {
+    sync_entry->set_client_defined_unique_tag(
+        meta_entry.GetUniqueClientTag());
   }
-  if (CommitResponse::RETRY == response) {
-    DVLOG(1) << "Retry Committing: " << local_entry;
-    return response;
+
+  // Deleted items with server-unknown parent ids can be a problem so we set
+  // the parent to 0. (TODO(sync): Still true in protocol?).
+  Id new_parent_id;
+  if (meta_entry.GetIsDel() &&
+      !meta_entry.GetParentId().ServerKnows()) {
+    new_parent_id = syncable::BaseTransaction::root_id();
+  } else {
+    new_parent_id = meta_entry.GetParentId();
   }
-  if (CommitResponse::OVER_QUOTA == response) {
-    LOG(WARNING) << "Hit deprecated OVER_QUOTA Committing: " << local_entry;
-    return response;
+  sync_entry->set_parent_id_string(SyncableIdToProto(new_parent_id));
+
+  // If our parent has changed, send up the old one so the server
+  // can correctly deal with multiple parents.
+  // TODO(nick): With the server keeping track of the primary sync parent,
+  // it should not be necessary to provide the old_parent_id: the version
+  // number should suffice.
+  if (new_parent_id != meta_entry.GetServerParentId() &&
+      0 != meta_entry.GetBaseVersion() &&
+      syncable::CHANGES_VERSION != meta_entry.GetBaseVersion()) {
+    sync_entry->set_old_parent_id(
+        SyncableIdToProto(meta_entry.GetServerParentId()));
   }
-  if (!server_entry.has_id_string()) {
-    LOG(ERROR) << "Commit response has no id";
-    return CommitResponse::INVALID_MESSAGE;
+
+  int64 version = meta_entry.GetBaseVersion();
+  if (syncable::CHANGES_VERSION == version || 0 == version) {
+    // Undeletions are only supported for items that have a client tag.
+    DCHECK(!id.ServerKnows() ||
+           !meta_entry.GetUniqueClientTag().empty())
+        << meta_entry;
+
+    // Version 0 means to create or undelete an object.
+    sync_entry->set_version(0);
+  } else {
+    DCHECK(id.ServerKnows()) << meta_entry;
+    sync_entry->set_version(meta_entry.GetBaseVersion());
   }
+  sync_entry->set_ctime(TimeToProtoTime(meta_entry.GetCtime()));
+  sync_entry->set_mtime(TimeToProtoTime(meta_entry.GetMtime()));
 
-  // Implied by the IsValid call above, but here for clarity.
-  DCHECK_EQ(CommitResponse::SUCCESS, response) << response;
-  // Check to see if we've been given the ID of an existing entry. If so treat
-  // it as an error response and retry later.
-  const syncable::Id& server_entry_id =
-      SyncableIdFromProto(server_entry.id_string());
-  if (local_entry.GetId() != server_entry_id) {
-    Entry e(trans, GET_BY_ID, server_entry_id);
-    if (e.good()) {
-      LOG(ERROR)
-          << "Got duplicate id when commiting id: "
-          << local_entry.GetId()
-          << ". Treating as an error return";
-      return CommitResponse::INVALID_MESSAGE;
+  // Deletion is final on the server, let's move things and then delete them.
+  if (meta_entry.GetIsDel()) {
+    sync_entry->set_deleted(true);
+  } else {
+    if (meta_entry.GetSpecifics().has_bookmark()) {
+      // Both insert_after_item_id and position_in_parent fields are set only
+      // for legacy reasons.  See comments in sync.proto for more information.
+      const Id& prev_id = meta_entry.GetPredecessorId();
+      string prev_id_string =
+          prev_id.IsRoot() ? string() : prev_id.GetServerId();
+      sync_entry->set_insert_after_item_id(prev_id_string);
+      sync_entry->set_position_in_parent(
+          meta_entry.GetUniquePosition().ToInt64());
+      meta_entry.GetUniquePosition().ToProto(
+          sync_entry->mutable_unique_position());
     }
+    SetEntrySpecifics(meta_entry, sync_entry);
   }
+}
 
-  if (server_entry.version() == 0) {
-    LOG(WARNING) << "Server returned a zero version on a commit response.";
-  }
 
-  ProcessSuccessfulCommitResponse(commit_request_entry, server_entry,
-      local_entry.GetId(), &local_entry, syncing_was_set, deleted_folders);
-  return response;
+// Helpers for ProcessSingleCommitResponse.
+namespace {
+
+void LogServerError(const sync_pb::CommitResponse_EntryResponse& res) {
+  if (res.has_error_message())
+    LOG(WARNING) << "  " << res.error_message();
+  else
+    LOG(WARNING) << "  No detailed error message returned from server";
 }
 
-const string& ProcessCommitResponseCommand::GetResultingPostCommitName(
+const string& GetResultingPostCommitName(
     const sync_pb::SyncEntity& committed_entry,
     const sync_pb::CommitResponse_EntryResponse& entry_response) {
   const string& response_name =
@@ -227,7 +199,7 @@ const string& ProcessCommitResponseCommand::GetResultingPostCommitName(
   return SyncerProtoUtil::NameFromSyncEntity(committed_entry);
 }
 
-bool ProcessCommitResponseCommand::UpdateVersionAfterCommit(
+bool UpdateVersionAfterCommit(
     const sync_pb::SyncEntity& committed_entry,
     const sync_pb::CommitResponse_EntryResponse& entry_response,
     const syncable::Id& pre_commit_id,
@@ -264,7 +236,7 @@ bool ProcessCommitResponseCommand::UpdateVersionAfterCommit(
   return true;
 }
 
-bool ProcessCommitResponseCommand::ChangeIdAfterCommit(
+bool ChangeIdAfterCommit(
     const sync_pb::CommitResponse_EntryResponse& entry_response,
     const syncable::Id& pre_commit_id,
     syncable::ModelNeutralMutableEntry* local_entry) {
@@ -278,7 +250,10 @@ bool ProcessCommitResponseCommand::ChangeIdAfterCommit(
       DVLOG(1) << " ID changed while committing an old entry. "
                << pre_commit_id << " became " << entry_response_id << ".";
     }
-    ModelNeutralMutableEntry same_id(trans, GET_BY_ID, entry_response_id);
+    syncable::ModelNeutralMutableEntry same_id(
+        trans,
+        syncable::GET_BY_ID,
+        entry_response_id);
     // We should trap this before this function.
     if (same_id.good()) {
       LOG(ERROR) << "ID clash with id " << entry_response_id
@@ -291,7 +266,7 @@ bool ProcessCommitResponseCommand::ChangeIdAfterCommit(
   return true;
 }
 
-void ProcessCommitResponseCommand::UpdateServerFieldsAfterCommit(
+void UpdateServerFieldsAfterCommit(
     const sync_pb::SyncEntity& committed_entry,
     const sync_pb::CommitResponse_EntryResponse& entry_response,
     syncable::ModelNeutralMutableEntry* local_entry) {
@@ -342,7 +317,7 @@ void ProcessCommitResponseCommand::UpdateServerFieldsAfterCommit(
   }
 }
 
-void ProcessCommitResponseCommand::ProcessSuccessfulCommitResponse(
+void ProcessSuccessfulCommitResponse(
     const sync_pb::SyncEntity& committed_entry,
     const sync_pb::CommitResponse_EntryResponse& entry_response,
     const syncable::Id& pre_commit_id,
@@ -384,4 +359,82 @@ void ProcessCommitResponseCommand::ProcessSuccessfulCommitResponse(
   }
 }
 
+}  // namespace
+
+sync_pb::CommitResponse::ResponseType
+ProcessSingleCommitResponse(
+    syncable::BaseWriteTransaction* trans,
+    const sync_pb::CommitResponse_EntryResponse& server_entry,
+    const sync_pb::SyncEntity& commit_request_entry,
+    int64 metahandle,
+    set<syncable::Id>* deleted_folders) {
+  syncable::ModelNeutralMutableEntry local_entry(
+      trans,
+      syncable::GET_BY_HANDLE,
+      metahandle);
+  CHECK(local_entry.good());
+  bool syncing_was_set = local_entry.GetSyncing();
+  local_entry.PutSyncing(false);
+
+  sync_pb::CommitResponse::ResponseType response = server_entry.response_type();
+  if (!sync_pb::CommitResponse::ResponseType_IsValid(response)) {
+    LOG(ERROR) << "Commit response has unknown response type! Possibly out "
+               "of date client?";
+    return sync_pb::CommitResponse::INVALID_MESSAGE;
+  }
+  if (sync_pb::CommitResponse::TRANSIENT_ERROR == response) {
+    DVLOG(1) << "Transient Error Committing: " << local_entry;
+    LogServerError(server_entry);
+    return sync_pb::CommitResponse::TRANSIENT_ERROR;
+  }
+  if (sync_pb::CommitResponse::INVALID_MESSAGE == response) {
+    LOG(ERROR) << "Error Commiting: " << local_entry;
+    LogServerError(server_entry);
+    return response;
+  }
+  if (sync_pb::CommitResponse::CONFLICT == response) {
+    DVLOG(1) << "Conflict Committing: " << local_entry;
+    return response;
+  }
+  if (sync_pb::CommitResponse::RETRY == response) {
+    DVLOG(1) << "Retry Committing: " << local_entry;
+    return response;
+  }
+  if (sync_pb::CommitResponse::OVER_QUOTA == response) {
+    LOG(WARNING) << "Hit deprecated OVER_QUOTA Committing: " << local_entry;
+    return response;
+  }
+  if (!server_entry.has_id_string()) {
+    LOG(ERROR) << "Commit response has no id";
+    return sync_pb::CommitResponse::INVALID_MESSAGE;
+  }
+
+  // Implied by the IsValid call above, but here for clarity.
+  DCHECK_EQ(sync_pb::CommitResponse::SUCCESS, response) << response;
+  // Check to see if we've been given the ID of an existing entry. If so treat
+  // it as an error response and retry later.
+  const syncable::Id& server_entry_id =
+      SyncableIdFromProto(server_entry.id_string());
+  if (local_entry.GetId() != server_entry_id) {
+    Entry e(trans, syncable::GET_BY_ID, server_entry_id);
+    if (e.good()) {
+      LOG(ERROR)
+          << "Got duplicate id when commiting id: "
+          << local_entry.GetId()
+          << ". Treating as an error return";
+      return sync_pb::CommitResponse::INVALID_MESSAGE;
+    }
+  }
+
+  if (server_entry.version() == 0) {
+    LOG(WARNING) << "Server returned a zero version on a commit response.";
+  }
+
+  ProcessSuccessfulCommitResponse(commit_request_entry, server_entry,
+      local_entry.GetId(), &local_entry, syncing_was_set, deleted_folders);
+  return response;
+}
+
+}  // namespace commit_util
+
 }  // namespace syncer