diff options
| -rw-r--r-- | net/tools/testserver/chromiumsync.py | 56 | ||||
| -rwxr-xr-x | net/tools/testserver/chromiumsync_test.py | 14 | ||||
| -rw-r--r-- | sync/engine/apply_control_data_updates.cc | 124 | ||||
| -rw-r--r-- | sync/engine/apply_control_data_updates.h | 13 | ||||
| -rw-r--r-- | sync/engine/apply_control_data_updates_unittest.cc | 96 | ||||
| -rw-r--r-- | sync/engine/syncer.cc | 2 | ||||
| -rw-r--r-- | sync/engine/syncer_util.cc | 5 | ||||
| -rw-r--r-- | sync/internal_api/public/test/test_entry_factory.h | 2 | ||||
| -rw-r--r-- | sync/internal_api/test/test_entry_factory.cc | 14 | ||||
| -rw-r--r-- | sync/syncable/model_type.cc | 6 |
10 files changed, 241 insertions, 91 deletions
diff --git a/net/tools/testserver/chromiumsync.py b/net/tools/testserver/chromiumsync.py index 62156b2..92542e3 100644 --- a/net/tools/testserver/chromiumsync.py +++ b/net/tools/testserver/chromiumsync.py @@ -50,6 +50,8 @@ ALL_TYPES = ( AUTOFILL, AUTOFILL_PROFILE, BOOKMARK, + DEVICE_INFO, + EXPERIMENTS, EXTENSIONS, HISTORY_DELETE_DIRECTIVE, NIGORI, @@ -59,7 +61,7 @@ ALL_TYPES = ( SESSION, THEME, TYPED_URL, - EXTENSION_SETTINGS) = range(17) + EXTENSION_SETTINGS) = range(19) # An eumeration on the frequency at which the server should send errors # to the client. This would be specified by the url that triggers the error. @@ -82,6 +84,8 @@ SYNC_TYPE_TO_DESCRIPTOR = { AUTOFILL: SYNC_TYPE_FIELDS['autofill'], AUTOFILL_PROFILE: SYNC_TYPE_FIELDS['autofill_profile'], BOOKMARK: SYNC_TYPE_FIELDS['bookmark'], + DEVICE_INFO: SYNC_TYPE_FIELDS['device_info'], + EXPERIMENTS: SYNC_TYPE_FIELDS['experiments'], EXTENSION_SETTINGS: SYNC_TYPE_FIELDS['extension_setting'], EXTENSIONS: SYNC_TYPE_FIELDS['extension'], HISTORY_DELETE_DIRECTIVE: SYNC_TYPE_FIELDS['history_delete_directive'], @@ -415,10 +419,15 @@ class SyncDataModel(object): # Specify all the permanent items that a model might need. _PERMANENT_ITEM_SPECS = [ - PermanentItem('google_chrome', name='Google Chrome', - parent_tag=ROOT_ID, sync_type=TOP_LEVEL), + PermanentItem('google_chrome_apps', name='Apps', + parent_tag=ROOT_ID, sync_type=APPS), + PermanentItem('google_chrome_app_notifications', name='App Notifications', + parent_tag=ROOT_ID, sync_type=APP_NOTIFICATION), + PermanentItem('google_chrome_app_settings', + name='App Settings', + parent_tag=ROOT_ID, sync_type=APP_SETTINGS), PermanentItem('google_chrome_bookmarks', name='Bookmarks', - parent_tag='google_chrome', sync_type=BOOKMARK), + parent_tag=ROOT_ID, sync_type=BOOKMARK), PermanentItem('bookmark_bar', name='Bookmark Bar', parent_tag='google_chrome_bookmarks', sync_type=BOOKMARK), PermanentItem('synced_bookmarks', name='Mobile Bookmarks', @@ -428,40 +437,37 @@ class SyncDataModel(object): PermanentItem('synced_bookmarks', name='Synced Bookmarks', parent_tag='google_chrome_bookmarks', sync_type=BOOKMARK, create_by_default=False), - PermanentItem('google_chrome_preferences', name='Preferences', - parent_tag='google_chrome', sync_type=PREFERENCE), PermanentItem('google_chrome_autofill', name='Autofill', - parent_tag='google_chrome', sync_type=AUTOFILL), + parent_tag=ROOT_ID, sync_type=AUTOFILL), PermanentItem('google_chrome_autofill_profiles', name='Autofill Profiles', - parent_tag='google_chrome', sync_type=AUTOFILL_PROFILE), - PermanentItem('google_chrome_app_settings', - name='App Settings', - parent_tag='google_chrome', sync_type=APP_SETTINGS), + parent_tag=ROOT_ID, sync_type=AUTOFILL_PROFILE), + PermanentItem('google_chrome_device_info', name='Device Info', + parent_tag=ROOT_ID, sync_type=DEVICE_INFO), + PermanentItem('google_chrome_experiments', name='Experiments', + parent_tag=ROOT_ID, sync_type=EXPERIMENTS), PermanentItem('google_chrome_extension_settings', name='Extension Settings', - parent_tag='google_chrome', sync_type=EXTENSION_SETTINGS), + parent_tag=ROOT_ID, sync_type=EXTENSION_SETTINGS), PermanentItem('google_chrome_extensions', name='Extensions', - parent_tag='google_chrome', sync_type=EXTENSIONS), + parent_tag=ROOT_ID, sync_type=EXTENSIONS), PermanentItem('google_chrome_history_delete_directives', name='History Delete Directives', - parent_tag='google_chrome', + parent_tag=ROOT_ID, sync_type=HISTORY_DELETE_DIRECTIVE), + PermanentItem('google_chrome_nigori', name='Nigori', + parent_tag=ROOT_ID, sync_type=NIGORI), PermanentItem('google_chrome_passwords', name='Passwords', - parent_tag='google_chrome', sync_type=PASSWORD), + parent_tag=ROOT_ID, sync_type=PASSWORD), + PermanentItem('google_chrome_preferences', name='Preferences', + parent_tag=ROOT_ID, sync_type=PREFERENCE), PermanentItem('google_chrome_search_engines', name='Search Engines', - parent_tag='google_chrome', sync_type=SEARCH_ENGINE), + parent_tag=ROOT_ID, sync_type=SEARCH_ENGINE), PermanentItem('google_chrome_sessions', name='Sessions', - parent_tag='google_chrome', sync_type=SESSION), + parent_tag=ROOT_ID, sync_type=SESSION), PermanentItem('google_chrome_themes', name='Themes', - parent_tag='google_chrome', sync_type=THEME), + parent_tag=ROOT_ID, sync_type=THEME), PermanentItem('google_chrome_typed_urls', name='Typed URLs', - parent_tag='google_chrome', sync_type=TYPED_URL), - PermanentItem('google_chrome_nigori', name='Nigori', - parent_tag='google_chrome', sync_type=NIGORI), - PermanentItem('google_chrome_apps', name='Apps', - parent_tag='google_chrome', sync_type=APPS), - PermanentItem('google_chrome_app_notifications', name='App Notifications', - parent_tag='google_chrome', sync_type=APP_NOTIFICATION), + parent_tag=ROOT_ID, sync_type=TYPED_URL), ] def __init__(self): diff --git a/net/tools/testserver/chromiumsync_test.py b/net/tools/testserver/chromiumsync_test.py index e680d41..7a68b40 100755 --- a/net/tools/testserver/chromiumsync_test.py +++ b/net/tools/testserver/chromiumsync_test.py @@ -50,7 +50,8 @@ class SyncDataModelTest(unittest.TestCase): declared_specs.add(spec.tag) unique_datatypes = set([x.sync_type for x in specs]) - self.assertEqual(unique_datatypes, set(chromiumsync.ALL_TYPES), + self.assertEqual(unique_datatypes, + set(chromiumsync.ALL_TYPES[1:]), 'Every sync datatype should have a permanent folder ' 'associated with it') @@ -70,19 +71,17 @@ class SyncDataModelTest(unittest.TestCase): def testCreatePermanentItems(self): self.model._CreateDefaultPermanentItems(chromiumsync.ALL_TYPES) - self.assertEqual(len(chromiumsync.ALL_TYPES) + 2, + self.assertEqual(len(chromiumsync.ALL_TYPES) + 1, len(self.model._entries)) def ExpectedPermanentItemCount(self, sync_type): if sync_type == chromiumsync.BOOKMARK: if self._expect_synced_bookmarks_folder: - return 5 - else: return 4 - elif sync_type == chromiumsync.TOP_LEVEL: - return 1 + else: + return 3 else: - return 2 + return 1 def testGetChangesFromTimestampZeroForEachType(self): all_types = chromiumsync.ALL_TYPES[1:] @@ -96,7 +95,6 @@ class SyncDataModelTest(unittest.TestCase): expected_count = self.ExpectedPermanentItemCount(sync_type) self.assertEqual(expected_count, version) self.assertEqual(expected_count, len(changes)) - self.assertEqual('google_chrome', changes[0].server_defined_unique_tag) for change in changes: self.assertTrue(change.HasField('server_defined_unique_tag')) self.assertEqual(change.version, change.sync_timestamp) diff --git a/sync/engine/apply_control_data_updates.cc b/sync/engine/apply_control_data_updates.cc index d0f182d..2354c48 100644 --- a/sync/engine/apply_control_data_updates.cc +++ b/sync/engine/apply_control_data_updates.cc @@ -24,15 +24,74 @@ using syncable::SERVER_SPECIFICS; using syncable::SPECIFICS; using syncable::SYNCER; -void ApplyControlDataUpdates(syncable::Directory* dir) { +void ApplyControlDataUpdates(sessions::SyncSession* session) { + syncable::Directory* dir = session->context()->directory(); syncable::WriteTransaction trans(FROM_HERE, SYNCER, dir); - if (ApplyNigoriUpdates(&trans, dir->GetCryptographer(&trans))) { - dir->set_initial_sync_ended_for_type(NIGORI, true); + std::vector<int64> handles; + dir->GetUnappliedUpdateMetaHandles( + &trans, ToFullModelTypeSet(ControlTypes()), &handles); + + // First, go through and manually apply any new top level datatype nodes (so + // that we don't have to worry about hitting a CONFLICT_HIERARCHY with an + // entry because we haven't applied its parent yet). + // TODO(sync): if at some point we support control datatypes with actual + // hierarchies we'll need to revisit this logic. + for (ModelTypeSet::Iterator iter = ControlTypes().First(); iter.Good(); + iter.Inc()) { + syncable::MutableEntry entry(&trans, + syncable::GET_BY_SERVER_TAG, + ModelTypeToRootTag(iter.Get())); + if (!entry.good()) + continue; + if (!entry.Get(syncable::IS_UNAPPLIED_UPDATE)) + continue; + + ModelType type = entry.GetServerModelType(); + if (type == NIGORI) { + // Nigori node applications never fail. + ApplyNigoriUpdate(&trans, + &entry, + dir->GetCryptographer(&trans)); + } else { + ApplyControlUpdate(&trans, + &entry, + dir->GetCryptographer(&trans)); + } + } + + // Go through the rest of the unapplied control updates, skipping over any + // top level folders. + for (std::vector<int64>::const_iterator iter = handles.begin(); + iter != handles.end(); ++iter) { + syncable::MutableEntry entry(&trans, syncable::GET_BY_HANDLE, *iter); + CHECK(entry.good()); + ModelType type = entry.GetServerModelType(); + CHECK(ControlTypes().Has(type)); + if (!entry.Get(syncable::UNIQUE_SERVER_TAG).empty()) { + // We should have already applied all top level control nodes. + DCHECK(!entry.Get(syncable::IS_UNAPPLIED_UPDATE)); + continue; + } + + ApplyControlUpdate(&trans, + &entry, + dir->GetCryptographer(&trans)); + } + + // Set initial sync ended bits for all control types requested. + for (ModelTypeSet::Iterator it = + session->status_controller().updates_request_types().First(); + it.Good(); it.Inc()) { + if (!IsControlType(it.Get())) + continue; + + // This gets persisted to the directory's backing store. + dir->set_initial_sync_ended_for_type(it.Get(), true); } } -// Update the sync encryption handler with the server's nigori node. +// Update the nigori handler with the server's nigori node. // // If we have a locally modified nigori node, we merge them manually. This // handles the case where two clients both set a different passphrase. The @@ -43,24 +102,18 @@ void ApplyControlDataUpdates(syncable::Directory* dir) { // passphrase, the cryptographer will preserve the encryption keys based on the // local passphrase, while the nigori node will preserve the server encryption // keys. -bool ApplyNigoriUpdates(syncable::WriteTransaction* trans, - Cryptographer* cryptographer) { - syncable::MutableEntry nigori_node(trans, GET_BY_SERVER_TAG, - ModelTypeToRootTag(NIGORI)); - - // Mainly for unit tests. We should have a Nigori node by this point. - if (!nigori_node.good()) { - return false; - } - - if (!nigori_node.Get(IS_UNAPPLIED_UPDATE)) { - return true; - } +void ApplyNigoriUpdate(syncable::WriteTransaction* const trans, + syncable::MutableEntry* const entry, + Cryptographer* cryptographer) { + DCHECK(entry->Get(IS_UNAPPLIED_UPDATE)); // We apply the nigori update regardless of whether there's a conflict or // not in order to preserve any new encrypted types or encryption keys. + // TODO(zea): consider having this return a bool reflecting whether it was a + // valid update or not, and in the case of invalid updates not overwrite the + // local data. const sync_pb::NigoriSpecifics& nigori = - nigori_node.Get(SERVER_SPECIFICS).nigori(); + entry->Get(SERVER_SPECIFICS).nigori(); trans->directory()->GetNigoriHandler()->ApplyNigoriUpdate(nigori, trans); // Make sure any unsynced changes are properly encrypted as necessary. @@ -82,19 +135,19 @@ bool ApplyNigoriUpdates(syncable::WriteTransaction* trans, syncable::ProcessUnsyncedChangesForEncryption(trans); } - if (!nigori_node.Get(IS_UNSYNCED)) { // Update only. - UpdateLocalDataFromServerData(trans, &nigori_node); + if (!entry->Get(IS_UNSYNCED)) { // Update only. + UpdateLocalDataFromServerData(trans, entry); } else { // Conflict. const sync_pb::EntitySpecifics& server_specifics = - nigori_node.Get(SERVER_SPECIFICS); + entry->Get(SERVER_SPECIFICS); const sync_pb::NigoriSpecifics& server_nigori = server_specifics.nigori(); const sync_pb::EntitySpecifics& local_specifics = - nigori_node.Get(SPECIFICS); + entry->Get(SPECIFICS); const sync_pb::NigoriSpecifics& local_nigori = local_specifics.nigori(); // We initialize the new nigori with the server state, and will override // it as necessary below. - sync_pb::EntitySpecifics new_specifics = nigori_node.Get(SERVER_SPECIFICS); + sync_pb::EntitySpecifics new_specifics = entry->Get(SERVER_SPECIFICS); sync_pb::NigoriSpecifics* new_nigori = new_specifics.mutable_nigori(); // If the cryptographer is not ready, another client set a new encryption @@ -145,18 +198,35 @@ bool ApplyNigoriUpdates(syncable::WriteTransaction* trans, new_nigori, trans); - nigori_node.Put(SPECIFICS, new_specifics); + entry->Put(SPECIFICS, new_specifics); DVLOG(1) << "Resolving simple conflict, merging nigori nodes: " - << nigori_node; + << entry; - conflict_util::OverwriteServerChanges(&nigori_node); + conflict_util::OverwriteServerChanges(entry); UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict", ConflictResolver::NIGORI_MERGE, ConflictResolver::CONFLICT_RESOLUTION_SIZE); } +} + +void ApplyControlUpdate(syncable::WriteTransaction* const trans, + syncable::MutableEntry* const entry, + Cryptographer* cryptographer) { + DCHECK_NE(entry->GetServerModelType(), NIGORI); + DCHECK(entry->Get(IS_UNAPPLIED_UPDATE)); + if (entry->Get(IS_UNSYNCED)) { + // We just let the server win all conflicts with control types. + DVLOG(1) << "Ignoring local changes for control update."; + conflict_util::IgnoreLocalChanges(entry); + UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict", + ConflictResolver::OVERWRITE_LOCAL, + ConflictResolver::CONFLICT_RESOLUTION_SIZE); + } - return true; + UpdateAttemptResponse response = AttemptToUpdateEntry( + trans, entry, cryptographer); + DCHECK_EQ(SUCCESS, response); } } // namespace syncer diff --git a/sync/engine/apply_control_data_updates.h b/sync/engine/apply_control_data_updates.h index efe2cc8..8d19f47 100644 --- a/sync/engine/apply_control_data_updates.h +++ b/sync/engine/apply_control_data_updates.h @@ -9,13 +9,22 @@ namespace syncer { class Cryptographer; +namespace sessions { +class SyncSession; +} + namespace syncable { class Directory; +class MutableEntry; class WriteTransaction; } -void ApplyControlDataUpdates(syncable::Directory* dir); -bool ApplyNigoriUpdates(syncable::WriteTransaction* trans, +void ApplyControlDataUpdates(sessions::SyncSession* session); +void ApplyNigoriUpdate(syncable::WriteTransaction* trans, + syncable::MutableEntry* const entry, + Cryptographer* cryptographer); +void ApplyControlUpdate(syncable::WriteTransaction* const trans, + syncable::MutableEntry* const entry, Cryptographer* cryptographer); } // namespace syncer diff --git a/sync/engine/apply_control_data_updates_unittest.cc b/sync/engine/apply_control_data_updates_unittest.cc index aa00865..09c5e11 100644 --- a/sync/engine/apply_control_data_updates_unittest.cc +++ b/sync/engine/apply_control_data_updates_unittest.cc @@ -41,12 +41,14 @@ class ApplyControlDataUpdatesTest : public SyncerCommandTest { workers()->push_back(make_scoped_refptr(new FakeModelWorker(GROUP_UI))); workers()->push_back( make_scoped_refptr(new FakeModelWorker(GROUP_PASSWORD))); - (*mutable_routing_info())[BOOKMARKS] = GROUP_UI; - (*mutable_routing_info())[PASSWORDS] = GROUP_PASSWORD; (*mutable_routing_info())[NIGORI] = GROUP_PASSIVE; + (*mutable_routing_info())[EXPERIMENTS] = GROUP_PASSIVE; SyncerCommandTest::SetUp(); entry_factory_.reset(new TestEntryFactory(directory())); + session()->mutable_status_controller()->set_updates_request_types( + ControlTypes()); + syncable::ReadTransaction trans(FROM_HERE, directory()); } @@ -90,7 +92,7 @@ TEST_F(ApplyControlDataUpdatesTest, NigoriUpdate) { ModelTypeToRootTag(NIGORI), specifics, true); EXPECT_FALSE(cryptographer->has_pending_keys()); - ApplyControlDataUpdates(directory()); + ApplyControlDataUpdates(session()); EXPECT_FALSE(cryptographer->is_ready()); EXPECT_TRUE(cryptographer->has_pending_keys()); @@ -170,7 +172,7 @@ TEST_F(ApplyControlDataUpdatesTest, EncryptUnsyncedChanges) { EXPECT_EQ(2*batch_s+1, handles.size()); } - ApplyControlDataUpdates(directory()); + ApplyControlDataUpdates(session()); EXPECT_FALSE(cryptographer->has_pending_keys()); EXPECT_TRUE(cryptographer->is_ready()); @@ -198,7 +200,7 @@ TEST_F(ApplyControlDataUpdatesTest, EncryptUnsyncedChanges) { entry.Put(syncable::IS_UNAPPLIED_UPDATE, true); } - ApplyControlDataUpdates(directory()); + ApplyControlDataUpdates(session()); EXPECT_FALSE(cryptographer->has_pending_keys()); EXPECT_TRUE(cryptographer->is_ready()); @@ -285,7 +287,7 @@ TEST_F(ApplyControlDataUpdatesTest, CannotEncryptUnsyncedChanges) { EXPECT_EQ(2*batch_s+1, handles.size()); } - ApplyControlDataUpdates(directory()); + ApplyControlDataUpdates(session()); EXPECT_FALSE(cryptographer->is_ready()); EXPECT_TRUE(cryptographer->has_pending_keys()); @@ -363,7 +365,7 @@ TEST_F(ApplyControlDataUpdatesTest, EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); - ApplyControlDataUpdates(directory()); + ApplyControlDataUpdates(session()); EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); @@ -442,7 +444,7 @@ TEST_F(ApplyControlDataUpdatesTest, EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); - ApplyControlDataUpdates(directory()); + ApplyControlDataUpdates(session()); EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); @@ -516,7 +518,7 @@ TEST_F(ApplyControlDataUpdatesTest, EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); - ApplyControlDataUpdates(directory()); + ApplyControlDataUpdates(session()); EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); @@ -596,7 +598,7 @@ TEST_F(ApplyControlDataUpdatesTest, EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); - ApplyControlDataUpdates(directory()); + ApplyControlDataUpdates(session()); EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); @@ -679,7 +681,7 @@ TEST_F(ApplyControlDataUpdatesTest, EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); - ApplyControlDataUpdates(directory()); + ApplyControlDataUpdates(session()); EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); @@ -760,7 +762,7 @@ TEST_F(ApplyControlDataUpdatesTest, EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); - ApplyControlDataUpdates(directory()); + ApplyControlDataUpdates(session()); EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); @@ -841,7 +843,7 @@ TEST_F(ApplyControlDataUpdatesTest, EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); - ApplyControlDataUpdates(directory()); + ApplyControlDataUpdates(session()); EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); @@ -868,4 +870,72 @@ TEST_F(ApplyControlDataUpdatesTest, } } +// Check that we can apply a simple control datatype node successfully. +TEST_F(ApplyControlDataUpdatesTest, ControlApply) { + EXPECT_FALSE(directory()->initial_sync_ended_types().Has(EXPERIMENTS)); + + std::string experiment_id = "experiment"; + sync_pb::EntitySpecifics specifics; + specifics.mutable_experiments()->mutable_keystore_encryption()-> + set_enabled(true); + int64 experiment_handle = entry_factory_->CreateUnappliedNewItem( + experiment_id, specifics, false); + ApplyControlDataUpdates(session()); + + EXPECT_TRUE(directory()->initial_sync_ended_types().Has(EXPERIMENTS)); + EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(experiment_handle)); + EXPECT_TRUE( + entry_factory_->GetLocalSpecificsForItem(experiment_handle). + experiments().keystore_encryption().enabled()); +} + +// Verify that we apply top level folders before their children. +TEST_F(ApplyControlDataUpdatesTest, ControlApplyParentBeforeChild) { + EXPECT_FALSE(directory()->initial_sync_ended_types().Has(EXPERIMENTS)); + + std::string parent_id = "parent"; + std::string experiment_id = "experiment"; + sync_pb::EntitySpecifics specifics; + specifics.mutable_experiments()->mutable_keystore_encryption()-> + set_enabled(true); + int64 experiment_handle = entry_factory_->CreateUnappliedNewItemWithParent( + experiment_id, specifics, parent_id); + int64 parent_handle = entry_factory_->CreateUnappliedNewItem( + parent_id, specifics, true); + ApplyControlDataUpdates(session()); + + EXPECT_TRUE(directory()->initial_sync_ended_types().Has(EXPERIMENTS)); + EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(parent_handle)); + EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(experiment_handle)); + EXPECT_TRUE( + entry_factory_->GetLocalSpecificsForItem(experiment_handle). + experiments().keystore_encryption().enabled()); +} + +// Verify that we handle control datatype conflicts by preserving the server +// data. +TEST_F(ApplyControlDataUpdatesTest, ControlConflict) { + EXPECT_FALSE(directory()->initial_sync_ended_types().Has(EXPERIMENTS)); + + std::string experiment_id = "experiment"; + sync_pb::EntitySpecifics local_specifics, server_specifics; + server_specifics.mutable_experiments()->mutable_keystore_encryption()-> + set_enabled(true); + local_specifics.mutable_experiments()->mutable_keystore_encryption()-> + set_enabled(false); + int64 experiment_handle = entry_factory_->CreateSyncedItem( + experiment_id, EXPERIMENTS, false); + entry_factory_->SetServerSpecificsForItem(experiment_handle, + server_specifics); + entry_factory_->SetLocalSpecificsForItem(experiment_handle, + local_specifics); + ApplyControlDataUpdates(session()); + + EXPECT_TRUE(directory()->initial_sync_ended_types().Has(EXPERIMENTS)); + EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(experiment_handle)); + EXPECT_TRUE( + entry_factory_->GetLocalSpecificsForItem(experiment_handle). + experiments().keystore_encryption().enabled()); +} + } // namespace syncer diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc index 22010ee..90d193a 100644 --- a/sync/engine/syncer.cc +++ b/sync/engine/syncer.cc @@ -144,7 +144,7 @@ bool Syncer::SyncShare(sessions::SyncSession* session, } case APPLY_UPDATES: { // These include encryption updates that should be applied early. - ApplyControlDataUpdates(session->context()->directory()); + ApplyControlDataUpdates(session); ApplyUpdatesAndResolveConflictsCommand apply_updates; apply_updates.Execute(session); diff --git a/sync/engine/syncer_util.cc b/sync/engine/syncer_util.cc index cdc1b7a..b88bc7c 100644 --- a/sync/engine/syncer_util.cc +++ b/sync/engine/syncer_util.cc @@ -209,7 +209,7 @@ UpdateAttemptResponse AttemptToUpdateEntry( // We can't decrypt this node yet. DVLOG(1) << "Received an undecryptable " << ModelTypeToString(entry->GetServerModelType()) - << " update, returning encryption_conflict."; + << " update, returning conflict_encryption."; return CONFLICT_ENCRYPTION; } else if (specifics.has_password() && entry->Get(UNIQUE_SERVER_TAG).empty()) { @@ -217,7 +217,7 @@ UpdateAttemptResponse AttemptToUpdateEntry( const sync_pb::PasswordSpecifics& password = specifics.password(); if (!cryptographer->CanDecrypt(password.encrypted())) { DVLOG(1) << "Received an undecryptable password update, returning " - << "encryption_conflict."; + << "conflict_encryption."; return CONFLICT_ENCRYPTION; } } @@ -232,6 +232,7 @@ UpdateAttemptResponse AttemptToUpdateEntry( // different ways we deal with it once here to reduce the amount of code and // potential errors. if (!parent.good() || parent.Get(IS_DEL) || !parent.Get(IS_DIR)) { + DVLOG(1) << "Entry has bad parent, returning conflict_hierarchy."; return CONFLICT_HIERARCHY; } if (entry->Get(PARENT_ID) != new_parent) { diff --git a/sync/internal_api/public/test/test_entry_factory.h b/sync/internal_api/public/test/test_entry_factory.h index 1661a33..d5bc911 100644 --- a/sync/internal_api/public/test/test_entry_factory.h +++ b/sync/internal_api/public/test/test_entry_factory.h @@ -24,7 +24,7 @@ class TestEntryFactory { ~TestEntryFactory(); // Create a new unapplied folder node with a parent. - void CreateUnappliedNewItemWithParent( + int64 CreateUnappliedNewItemWithParent( const std::string& item_id, const sync_pb::EntitySpecifics& specifics, const std::string& parent_id); diff --git a/sync/internal_api/test/test_entry_factory.cc b/sync/internal_api/test/test_entry_factory.cc index 3fa0d67..3cf0dcf 100644 --- a/sync/internal_api/test/test_entry_factory.cc +++ b/sync/internal_api/test/test_entry_factory.cc @@ -27,7 +27,7 @@ TestEntryFactory::TestEntryFactory(syncable::Directory *dir) TestEntryFactory::~TestEntryFactory() { } -void TestEntryFactory::CreateUnappliedNewItemWithParent( +int64 TestEntryFactory::CreateUnappliedNewItemWithParent( const string& item_id, const sync_pb::EntitySpecifics& specifics, const string& parent_id) { @@ -42,6 +42,7 @@ void TestEntryFactory::CreateUnappliedNewItemWithParent( entry.Put(syncable::SERVER_PARENT_ID, Id::CreateFromServerId(parent_id)); entry.Put(syncable::SERVER_IS_DIR, true); entry.Put(syncable::SERVER_SPECIFICS, specifics); + return entry.Get(syncable::META_HANDLE); } int64 TestEntryFactory::CreateUnappliedNewItem( @@ -56,10 +57,12 @@ int64 TestEntryFactory::CreateUnappliedNewItem( entry.Put(syncable::IS_UNAPPLIED_UPDATE, true); entry.Put(syncable::SERVER_NON_UNIQUE_NAME, item_id); entry.Put(syncable::SERVER_PARENT_ID, syncable::GetNullId()); - entry.Put(syncable::SERVER_IS_DIR, false); + entry.Put(syncable::SERVER_IS_DIR, is_unique); entry.Put(syncable::SERVER_SPECIFICS, specifics); - if (is_unique) // For top-level nodes. - entry.Put(syncable::UNIQUE_SERVER_TAG, item_id); + if (is_unique) { // For top-level nodes. + entry.Put(syncable::UNIQUE_SERVER_TAG, + ModelTypeToRootTag(GetModelTypeFromSpecifics(specifics))); + } return entry.Get(syncable::META_HANDLE); } @@ -119,8 +122,7 @@ int64 TestEntryFactory::CreateUnappliedAndUnsyncedItem( } int64 TestEntryFactory::CreateSyncedItem( - const std::string& name, ModelType - model_type, bool is_folder) { + const std::string& name, ModelType model_type, bool is_folder) { WriteTransaction trans(FROM_HERE, UNITTEST, directory_); syncable::Id parent_id(TestIdFactory::root()); diff --git a/sync/syncable/model_type.cc b/sync/syncable/model_type.cc index 08094d0..546ac0c8 100644 --- a/sync/syncable/model_type.cc +++ b/sync/syncable/model_type.cc @@ -281,12 +281,6 @@ ModelTypeSet ControlTypes() { set.Put(ModelTypeFromInt(i)); } - // TODO(rlarocque): Re-enable this when the server supports it. - set.Remove(DEVICE_INFO); - - // TODO(rlarocque): Re-enable this when the server supports it. - set.Remove(EXPERIMENTS); - return set; } |