Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(162)

Unified Diff: sync/engine/process_updates_command.cc

Issue 38803003: sync: Implement per-type update processing (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fix integration tests Created 7 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: sync/engine/process_updates_command.cc
diff --git a/sync/engine/process_updates_command.cc b/sync/engine/process_updates_command.cc
deleted file mode 100644
index d69c1098c6efc0116327ea7ad1c0c10b4c52ba65..0000000000000000000000000000000000000000
--- a/sync/engine/process_updates_command.cc
+++ /dev/null
@@ -1,348 +0,0 @@
-// 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/engine/process_updates_command.h"
-
-#include <vector>
-
-#include "base/basictypes.h"
-#include "base/location.h"
-#include "sync/engine/syncer.h"
-#include "sync/engine/syncer_proto_util.h"
-#include "sync/engine/syncer_util.h"
-#include "sync/sessions/sync_session.h"
-#include "sync/syncable/directory.h"
-#include "sync/syncable/model_neutral_mutable_entry.h"
-#include "sync/syncable/syncable_model_neutral_write_transaction.h"
-#include "sync/syncable/syncable_proto_util.h"
-#include "sync/syncable/syncable_util.h"
-#include "sync/util/cryptographer.h"
-
-using std::vector;
-
-namespace syncer {
-
-using sessions::SyncSession;
-using sessions::StatusController;
-
-using syncable::GET_BY_ID;
-
-ProcessUpdatesCommand::ProcessUpdatesCommand() {}
-ProcessUpdatesCommand::~ProcessUpdatesCommand() {}
-
-namespace {
-
-// This function attempts to determine whether or not this update is genuinely
-// new, or if it is a reflection of one of our own commits.
-//
-// There is a known inaccuracy in its implementation. If this update ends up
-// being applied to a local item with a different ID, we will count the change
-// as being a non-reflection update. Fortunately, the server usually updates
-// our IDs correctly in its commit response, so a new ID during GetUpdate should
-// be rare.
-//
-// The only secnarios I can think of where this might happen are:
-// - We commit a new item to the server, but we don't persist the
-// server-returned new ID to the database before we shut down. On the GetUpdate
-// following the next restart, we will receive an update from the server that
-// updates its local ID.
-// - When two attempts to create an item with identical UNIQUE_CLIENT_TAG values
-// collide at the server. I have seen this in testing. When it happens, the
-// test server will send one of the clients a response to upate its local ID so
-// that both clients will refer to the item using the same ID going forward. In
-// this case, we're right to assume that the update is not a reflection.
-//
-// For more information, see FindLocalIdToUpdate().
-bool UpdateContainsNewVersion(syncable::BaseTransaction *trans,
- const sync_pb::SyncEntity &update) {
- int64 existing_version = -1; // The server always sends positive versions.
- syncable::Entry existing_entry(trans, GET_BY_ID,
- SyncableIdFromProto(update.id_string()));
- if (existing_entry.good())
- existing_version = existing_entry.GetBaseVersion();
-
- if (!existing_entry.good() && update.deleted()) {
- // There are several possible explanations for this. The most common cases
- // will be first time sync and the redelivery of deletions we've already
- // synced, accepted, and purged from our database. In either case, the
- // update is useless to us. Let's count them all as "not new", even though
- // that may not always be entirely accurate.
- return false;
- }
-
- if (existing_entry.good() &&
- !existing_entry.GetUniqueClientTag().empty() &&
- existing_entry.GetIsDel() &&
- update.deleted()) {
- // Unique client tags will have their version set to zero when they're
- // deleted. The usual version comparison logic won't be able to detect
- // reflections of these items. Instead, we assume any received tombstones
- // are reflections. That should be correct most of the time.
- return false;
- }
-
- return existing_version < update.version();
-}
-
-} // namespace
-
-SyncerError ProcessUpdatesCommand::ExecuteImpl(SyncSession* session) {
- syncable::Directory* dir = session->context()->directory();
-
- syncable::ModelNeutralWriteTransaction trans(
- FROM_HERE, syncable::SYNCER, dir);
-
- sessions::StatusController* status = session->mutable_status_controller();
- const sync_pb::GetUpdatesResponse& updates =
- status->updates_response().get_updates();
- ModelTypeSet requested_types = GetRoutingInfoTypes(
- session->context()->routing_info());
-
- TypeSyncEntityMap updates_by_type;
-
- PartitionUpdatesByType(updates, &updates_by_type);
-
- int update_count = updates.entries().size();
- status->increment_num_updates_downloaded_by(update_count);
-
- DVLOG(1) << update_count << " entries to verify";
- for (TypeSyncEntityMap::iterator it = updates_by_type.begin();
- it != updates_by_type.end(); ++it) {
- for (SyncEntityList::iterator update_it = it->second.begin();
- update_it != it->second.end(); ++update_it) {
- if (!UpdateContainsNewVersion(&trans, **update_it))
- status->increment_num_reflected_updates_downloaded_by(1);
- if ((*update_it)->deleted())
- status->increment_num_tombstone_updates_downloaded_by(1);
- VerifyResult verify_result = VerifyUpdate(
- &trans,
- **update_it,
- requested_types,
- session->context()->routing_info());
- if (verify_result != VERIFY_SUCCESS && verify_result != VERIFY_UNDELETE)
- continue;
- ProcessUpdate(**update_it, dir->GetCryptographer(&trans), &trans);
- }
- }
-
- return SYNCER_OK;
-}
-
-void ProcessUpdatesCommand::PartitionUpdatesByType(
- const sync_pb::GetUpdatesResponse& updates,
- TypeSyncEntityMap* updates_by_type) {
- int update_count = updates.entries().size();
- for (int i = 0; i < update_count; ++i) {
- const sync_pb::SyncEntity& update = updates.entries(i);
- ModelType type = GetModelType(update);
- DCHECK(IsRealDataType(type));
- (*updates_by_type)[type].push_back(&update);
- }
-}
-
-namespace {
-
-// In the event that IDs match, but tags differ AttemptReuniteClient tag
-// will have refused to unify the update.
-// We should not attempt to apply it at all since it violates consistency
-// rules.
-VerifyResult VerifyTagConsistency(
- const sync_pb::SyncEntity& entry,
- const syncable::ModelNeutralMutableEntry& same_id) {
- if (entry.has_client_defined_unique_tag() &&
- entry.client_defined_unique_tag() !=
- same_id.GetUniqueClientTag()) {
- return VERIFY_FAIL;
- }
- return VERIFY_UNDECIDED;
-}
-
-} // namespace
-
-VerifyResult ProcessUpdatesCommand::VerifyUpdate(
- syncable::ModelNeutralWriteTransaction* trans,
- const sync_pb::SyncEntity& entry,
- ModelTypeSet requested_types,
- const ModelSafeRoutingInfo& routes) {
- syncable::Id id = SyncableIdFromProto(entry.id_string());
- VerifyResult result = VERIFY_FAIL;
-
- const bool deleted = entry.has_deleted() && entry.deleted();
- const bool is_directory = IsFolder(entry);
- const ModelType model_type = GetModelType(entry);
-
- if (!id.ServerKnows()) {
- LOG(ERROR) << "Illegal negative id in received updates";
- return result;
- }
- {
- const std::string name = SyncerProtoUtil::NameFromSyncEntity(entry);
- if (name.empty() && !deleted) {
- LOG(ERROR) << "Zero length name in non-deleted update";
- return result;
- }
- }
-
- syncable::ModelNeutralMutableEntry same_id(trans, GET_BY_ID, id);
- result = VerifyNewEntry(entry, &same_id, deleted);
-
- ModelType placement_type = !deleted ? GetModelType(entry)
- : same_id.good() ? same_id.GetModelType() : UNSPECIFIED;
-
- if (VERIFY_UNDECIDED == result) {
- result = VerifyTagConsistency(entry, same_id);
- }
-
- if (VERIFY_UNDECIDED == result) {
- if (deleted) {
- // For deletes the server could send tombostones for items that
- // the client did not request. If so ignore those items.
- if (IsRealDataType(placement_type) &&
- !requested_types.Has(placement_type)) {
- result = VERIFY_SKIP;
- } else {
- result = VERIFY_SUCCESS;
- }
- }
- }
-
- // If we have an existing entry, we check here for updates that break
- // consistency rules.
- if (VERIFY_UNDECIDED == result) {
- result = VerifyUpdateConsistency(trans, entry, deleted,
- is_directory, model_type, &same_id);
- }
-
- if (VERIFY_UNDECIDED == result)
- result = VERIFY_SUCCESS; // No news is good news.
-
- return result; // This might be VERIFY_SUCCESS as well
-}
-
-namespace {
-// Returns true if the entry is still ok to process.
-bool ReverifyEntry(syncable::ModelNeutralWriteTransaction* trans,
- const sync_pb::SyncEntity& entry,
- syncable::ModelNeutralMutableEntry* same_id) {
-
- const bool deleted = entry.has_deleted() && entry.deleted();
- const bool is_directory = IsFolder(entry);
- const ModelType model_type = GetModelType(entry);
-
- return VERIFY_SUCCESS == VerifyUpdateConsistency(trans,
- entry,
- deleted,
- is_directory,
- model_type,
- same_id);
-}
-} // namespace
-
-// Process a single update. Will avoid touching global state.
-void ProcessUpdatesCommand::ProcessUpdate(
- const sync_pb::SyncEntity& update,
- const Cryptographer* cryptographer,
- syncable::ModelNeutralWriteTransaction* const trans) {
- const syncable::Id& server_id = SyncableIdFromProto(update.id_string());
- const std::string name = SyncerProtoUtil::NameFromSyncEntity(update);
-
- // Look to see if there's a local item that should recieve this update,
- // maybe due to a duplicate client tag or a lost commit response.
- syncable::Id local_id = FindLocalIdToUpdate(trans, update);
-
- // FindLocalEntryToUpdate has veto power.
- if (local_id.IsNull()) {
- return; // The entry has become irrelevant.
- }
-
- CreateNewEntry(trans, local_id);
-
- // We take a two step approach. First we store the entries data in the
- // server fields of a local entry and then move the data to the local fields
- syncable::ModelNeutralMutableEntry target_entry(trans, GET_BY_ID, local_id);
-
- // We need to run the Verify checks again; the world could have changed
- // since we last verified.
- if (!ReverifyEntry(trans, update, &target_entry)) {
- return; // The entry has become irrelevant.
- }
-
- // If we're repurposing an existing local entry with a new server ID,
- // change the ID now, after we're sure that the update can succeed.
- if (local_id != server_id) {
- DCHECK(!update.deleted());
- ChangeEntryIDAndUpdateChildren(trans, &target_entry, server_id);
- // When IDs change, versions become irrelevant. Forcing BASE_VERSION
- // to zero would ensure that this update gets applied, but would indicate
- // creation or undeletion if it were committed that way. Instead, prefer
- // forcing BASE_VERSION to entry.version() while also forcing
- // IS_UNAPPLIED_UPDATE to true. If the item is UNSYNCED, it's committable
- // from the new state; it may commit before the conflict resolver gets
- // a crack at it.
- if (target_entry.GetIsUnsynced() || target_entry.GetBaseVersion() > 0) {
- // If either of these conditions are met, then we can expect valid client
- // fields for this entry. When BASE_VERSION is positive, consistency is
- // enforced on the client fields at update-application time. Otherwise,
- // we leave the BASE_VERSION field alone; it'll get updated the first time
- // we successfully apply this update.
- target_entry.PutBaseVersion(update.version());
- }
- // Force application of this update, no matter what.
- target_entry.PutIsUnappliedUpdate(true);
- }
-
- // If this is a newly received undecryptable update, and the only thing that
- // has changed are the specifics, store the original decryptable specifics,
- // (on which any current or future local changes are based) before we
- // overwrite SERVER_SPECIFICS.
- // MTIME, CTIME, and NON_UNIQUE_NAME are not enforced.
-
- bool position_matches = false;
- if (target_entry.ShouldMaintainPosition() && !update.deleted()) {
- std::string update_tag = GetUniqueBookmarkTagFromUpdate(update);
- if (UniquePosition::IsValidSuffix(update_tag)) {
- position_matches = GetUpdatePosition(update, update_tag).Equals(
- target_entry.GetServerUniquePosition());
- } else {
- NOTREACHED();
- }
- } else {
- // If this item doesn't care about positions, then set this flag to true.
- position_matches = true;
- }
-
- if (!update.deleted() && !target_entry.GetServerIsDel() &&
- (SyncableIdFromProto(update.parent_id_string()) ==
- target_entry.GetServerParentId()) &&
- position_matches &&
- update.has_specifics() && update.specifics().has_encrypted() &&
- !cryptographer->CanDecrypt(update.specifics().encrypted())) {
- sync_pb::EntitySpecifics prev_specifics =
- target_entry.GetServerSpecifics();
- // We only store the old specifics if they were decryptable and applied and
- // there is no BASE_SERVER_SPECIFICS already. Else do nothing.
- if (!target_entry.GetIsUnappliedUpdate() &&
- !IsRealDataType(GetModelTypeFromSpecifics(
- target_entry.GetBaseServerSpecifics())) &&
- (!prev_specifics.has_encrypted() ||
- cryptographer->CanDecrypt(prev_specifics.encrypted()))) {
- DVLOG(2) << "Storing previous server specifcs: "
- << prev_specifics.SerializeAsString();
- target_entry.PutBaseServerSpecifics(prev_specifics);
- }
- } else if (IsRealDataType(GetModelTypeFromSpecifics(
- target_entry.GetBaseServerSpecifics()))) {
- // We have a BASE_SERVER_SPECIFICS, but a subsequent non-specifics-only
- // change arrived. As a result, we can't use the specifics alone to detect
- // changes, so we clear BASE_SERVER_SPECIFICS.
- target_entry.PutBaseServerSpecifics(
- sync_pb::EntitySpecifics());
- }
-
- UpdateServerFieldsFromUpdate(&target_entry, update, name);
-
- return;
-}
-
-} // namespace syncer

Powered by Google App Engine
This is Rietveld 408576698