diff options
author | haitaol@chromium.org <haitaol@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-10 05:20:11 +0000 |
---|---|---|
committer | haitaol@chromium.org <haitaol@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-10 05:20:11 +0000 |
commit | fc290d614c7fab02dcdaf88b992a0204be23aa90 (patch) | |
tree | 10400ca878d8c999249c694758006aa1d380333a /sync | |
parent | db06cc16af9cb0eaa766603f2e56f490041665db (diff) | |
download | chromium_src-fc290d614c7fab02dcdaf88b992a0204be23aa90.zip chromium_src-fc290d614c7fab02dcdaf88b992a0204be23aa90.tar.gz chromium_src-fc290d614c7fab02dcdaf88b992a0204be23aa90.tar.bz2 |
Populate versions on individual nodes in sync model and native bookmark model.
Update transaction versions of changed sync models and entries in
syncable::WriteTransaction after change delegate calculates changes and returns
handles of changed entries. For syncer changes, updated version is passed to
change processor and set on native model and nodes. For sync API changes, model
processor queries for new version and set on native model and nodes after write
transaction closes.
BUG=154858
Review URL: https://chromiumcodereview.appspot.com/11341048
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@167061 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync')
-rw-r--r-- | sync/internal_api/public/write_transaction.h | 8 | ||||
-rw-r--r-- | sync/internal_api/sync_manager_impl.cc | 81 | ||||
-rw-r--r-- | sync/internal_api/sync_manager_impl.h | 23 | ||||
-rw-r--r-- | sync/internal_api/write_transaction.cc | 10 | ||||
-rw-r--r-- | sync/syncable/directory_backing_store.cc | 21 | ||||
-rw-r--r-- | sync/syncable/directory_backing_store.h | 1 | ||||
-rw-r--r-- | sync/syncable/directory_backing_store_unittest.cc | 127 | ||||
-rw-r--r-- | sync/syncable/directory_change_delegate.h | 8 | ||||
-rw-r--r-- | sync/syncable/entry_kernel.h | 1 | ||||
-rw-r--r-- | sync/syncable/syncable_columns.h | 1 | ||||
-rw-r--r-- | sync/syncable/syncable_enum_conversions.cc | 7 | ||||
-rw-r--r-- | sync/syncable/syncable_unittest.cc | 7 | ||||
-rw-r--r-- | sync/syncable/write_transaction.cc | 46 | ||||
-rw-r--r-- | sync/syncable/write_transaction.h | 18 | ||||
-rw-r--r-- | sync/test/null_directory_change_delegate.cc | 25 | ||||
-rw-r--r-- | sync/test/null_directory_change_delegate.h | 6 | ||||
-rw-r--r-- | sync/test/test_directory_backing_store.h | 1 |
17 files changed, 318 insertions, 73 deletions
diff --git a/sync/internal_api/public/write_transaction.h b/sync/internal_api/public/write_transaction.h index df27cad..8c11570 100644 --- a/sync/internal_api/public/write_transaction.h +++ b/sync/internal_api/public/write_transaction.h @@ -30,6 +30,12 @@ class WriteTransaction : public BaseTransaction { // Start a new read/write transaction. WriteTransaction(const tracked_objects::Location& from_here, UserShare* share); + // |transaction_version| stores updated model and nodes version if model + // is changed by the transaction, or syncer::syncable::kInvalidTransaction + // if not after transaction is closed. This constructor is used for model + // types that support embassy data. + WriteTransaction(const tracked_objects::Location& from_here, + UserShare* share, int64* transaction_version); virtual ~WriteTransaction(); // Provide access to the syncable transaction from the API WriteNode. @@ -40,7 +46,7 @@ class WriteTransaction : public BaseTransaction { WriteTransaction() {} void SetTransaction(syncable::WriteTransaction* trans) { - transaction_ = trans; + transaction_ = trans; } private: diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc index 37c1a01..f7abea4 100644 --- a/sync/internal_api/sync_manager_impl.cc +++ b/sync/internal_api/sync_manager_impl.cc @@ -270,14 +270,6 @@ bool SyncManagerImpl::VisiblePropertiesDiffer( return false; } -bool SyncManagerImpl::ChangeBuffersAreEmpty() { - for (int i = 0; i < MODEL_TYPE_COUNT; ++i) { - if (!change_buffers_[i].IsEmpty()) - return false; - } - return true; -} - void SyncManagerImpl::ThrowUnrecoverableError() { DCHECK(thread_checker_.CalledOnValidThread()); ReadTransaction trans(FROM_HERE, GetUserShare()); @@ -788,7 +780,7 @@ SyncManagerImpl::HandleTransactionEndingChangeEvent( // This notification happens immediately before a syncable WriteTransaction // falls out of scope. It happens while the channel mutex is still held, // and while the transaction mutex is held, so it cannot be re-entrant. - if (!change_delegate_ || ChangeBuffersAreEmpty()) + if (!change_delegate_ || change_records_.empty()) return ModelTypeSet(); // This will continue the WriteTransaction using a read only wrapper. @@ -798,40 +790,28 @@ SyncManagerImpl::HandleTransactionEndingChangeEvent( ReadTransaction read_trans(GetUserShare(), trans); ModelTypeSet models_with_changes; - for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) { - const ModelType type = ModelTypeFromInt(i); - if (change_buffers_[type].IsEmpty()) - continue; - - ImmutableChangeRecordList ordered_changes; - // TODO(akalin): Propagate up the error further (see - // http://crbug.com/100907). - CHECK(change_buffers_[type].GetAllChangesInTreeOrder(&read_trans, - &ordered_changes)); - if (!ordered_changes.Get().empty()) { - // Increment transaction version so that change processor can read - // updated value and set it in native model after changes are applied. - trans->directory()->IncrementTransactionVersion(type); - - change_delegate_-> - OnChangesApplied(type, - trans->directory()->GetTransactionVersion(type), - &read_trans, ordered_changes); - change_observer_.Call(FROM_HERE, - &SyncManager::ChangeObserver::OnChangesApplied, - type, write_transaction_info.Get().id, ordered_changes); - models_with_changes.Put(type); - } - change_buffers_[i].Clear(); + for (ChangeRecordMap::const_iterator it = change_records_.begin(); + it != change_records_.end(); ++it) { + DCHECK(!it->second.Get().empty()); + ModelType type = ModelTypeFromInt(it->first); + change_delegate_-> + OnChangesApplied(type, trans->directory()->GetTransactionVersion(type), + &read_trans, it->second); + change_observer_.Call(FROM_HERE, + &SyncManager::ChangeObserver::OnChangesApplied, + type, write_transaction_info.Get().id, it->second); + models_with_changes.Put(type); } + change_records_.clear(); return models_with_changes; } void SyncManagerImpl::HandleCalculateChangesChangeEventFromSyncApi( const ImmutableWriteTransactionInfo& write_transaction_info, - syncable::BaseTransaction* trans) { + syncable::BaseTransaction* trans, + std::vector<int64>* entries_changed) { // We have been notified about a user action changing a sync model. - LOG_IF(WARNING, !ChangeBuffersAreEmpty()) << + LOG_IF(WARNING, !change_records_.empty()) << "CALCULATE_CHANGES called with unapplied old changes."; // The mutated model type, or UNSPECIFIED if nothing was mutated. @@ -855,6 +835,7 @@ void SyncManagerImpl::HandleCalculateChangesChangeEventFromSyncApi( // Found real mutation. if (model_type != UNSPECIFIED) { mutated_model_types.Put(model_type); + entries_changed->push_back(it->second.mutated.ref(syncable::META_HANDLE)); } } @@ -903,12 +884,15 @@ void SyncManagerImpl::SetExtraChangeRecordData(int64 id, void SyncManagerImpl::HandleCalculateChangesChangeEventFromSyncer( const ImmutableWriteTransactionInfo& write_transaction_info, - syncable::BaseTransaction* trans) { + syncable::BaseTransaction* trans, + std::vector<int64>* entries_changed) { // We only expect one notification per sync step, so change_buffers_ should // contain no pending entries. - LOG_IF(WARNING, !ChangeBuffersAreEmpty()) << + LOG_IF(WARNING, !change_records_.empty()) << "CALCULATE_CHANGES called with unapplied old changes."; + ChangeReorderBuffer change_buffers[MODEL_TYPE_COUNT]; + Cryptographer* crypto = directory()->GetCryptographer(trans); const syncable::ImmutableEntryKernelMutationMap& mutations = write_transaction_info.Get().mutations; @@ -925,18 +909,31 @@ void SyncManagerImpl::HandleCalculateChangesChangeEventFromSyncer( int64 handle = it->first; if (exists_now && !existed_before) - change_buffers_[type].PushAddedItem(handle); + change_buffers[type].PushAddedItem(handle); else if (!exists_now && existed_before) - change_buffers_[type].PushDeletedItem(handle); + change_buffers[type].PushDeletedItem(handle); else if (exists_now && existed_before && VisiblePropertiesDiffer(it->second, crypto)) { - change_buffers_[type].PushUpdatedItem( + change_buffers[type].PushUpdatedItem( handle, VisiblePositionsDiffer(it->second)); } - SetExtraChangeRecordData(handle, type, &change_buffers_[type], crypto, + SetExtraChangeRecordData(handle, type, &change_buffers[type], crypto, it->second.original, existed_before, exists_now); } + + ReadTransaction read_trans(GetUserShare(), trans); + for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) { + if (!change_buffers[i].IsEmpty()) { + if (change_buffers[i].GetAllChangesInTreeOrder(&read_trans, + &(change_records_[i]))) { + for (size_t j = 0; j < change_records_[i].Get().size(); ++j) + entries_changed->push_back((change_records_[i].Get())[j].id); + } + if (change_records_[i].Get().empty()) + change_records_.erase(i); + } + } } TimeDelta SyncManagerImpl::GetNudgeDelayTimeDelta( diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h index 6037e8c..6accdab 100644 --- a/sync/internal_api/sync_manager_impl.h +++ b/sync/internal_api/sync_manager_impl.h @@ -165,10 +165,12 @@ class SyncManagerImpl : syncable::BaseTransaction* trans) OVERRIDE; virtual void HandleCalculateChangesChangeEventFromSyncApi( const syncable::ImmutableWriteTransactionInfo& write_transaction_info, - syncable::BaseTransaction* trans) OVERRIDE; + syncable::BaseTransaction* trans, + std::vector<int64>* entries_changed) OVERRIDE; virtual void HandleCalculateChangesChangeEventFromSyncer( const syncable::ImmutableWriteTransactionInfo& write_transaction_info, - syncable::BaseTransaction* trans) OVERRIDE; + syncable::BaseTransaction* trans, + std::vector<int64>* entries_changed) OVERRIDE; // InvalidationHandler implementation. virtual void OnInvalidatorStateChange(InvalidatorState state) OVERRIDE; @@ -227,8 +229,6 @@ class SyncManagerImpl : const syncable::EntryKernelMutation& mutation, Cryptographer* cryptographer) const; - bool ChangeBuffersAreEmpty(); - // Open the directory named with username_for_share bool OpenDirectory(); @@ -335,13 +335,14 @@ class SyncManagerImpl : // sync components. AllStatus allstatus_; - // Each element of this array is a store of change records produced by - // HandleChangeEvent during the CALCULATE_CHANGES step. The changes are - // segregated by model type, and are stored here to be processed and - // forwarded to the observer slightly later, at the TRANSACTION_ENDING - // step by HandleTransactionEndingChangeEvent. The list is cleared in the - // TRANSACTION_COMPLETE step by HandleTransactionCompleteChangeEvent. - ChangeReorderBuffer change_buffers_[MODEL_TYPE_COUNT]; + // Each element of this map is a store of change records produced by + // HandleChangeEventFromSyncer during the CALCULATE_CHANGES step. The changes + // are grouped by model type, and are stored here in tree order to be + // forwarded to the observer slightly later, at the TRANSACTION_ENDING step + // by HandleTransactionEndingChangeEvent. The list is cleared after observer + // finishes processing. + typedef std::map<int, ImmutableChangeRecordList> ChangeRecordMap; + ChangeRecordMap change_records_; SyncManager::ChangeDelegate* change_delegate_; diff --git a/sync/internal_api/write_transaction.cc b/sync/internal_api/write_transaction.cc index fe11adf..cf0f121 100644 --- a/sync/internal_api/write_transaction.cc +++ b/sync/internal_api/write_transaction.cc @@ -18,6 +18,16 @@ WriteTransaction::WriteTransaction(const tracked_objects::Location& from_here, share->directory.get()); } +WriteTransaction::WriteTransaction(const tracked_objects::Location& from_here, + UserShare* share, + int64* new_model_version) + : BaseTransaction(share), + transaction_(NULL) { + transaction_ = new syncable::WriteTransaction(from_here, + share->directory.get(), + new_model_version); +} + WriteTransaction::~WriteTransaction() { delete transaction_; } diff --git a/sync/syncable/directory_backing_store.cc b/sync/syncable/directory_backing_store.cc index 4f9febf..e9ba0ac 100644 --- a/sync/syncable/directory_backing_store.cc +++ b/sync/syncable/directory_backing_store.cc @@ -40,7 +40,7 @@ static const string::size_type kUpdateStatementBufferSize = 2048; // Increment this version whenever updating DB tables. extern const int32 kCurrentDBVersion; // Global visibility for our unittest. -const int32 kCurrentDBVersion = 82; +const int32 kCurrentDBVersion = 83; // Iterate over the fields of |entry| and bind each to |statement| for // updating. Returns the number of args bound. @@ -358,6 +358,12 @@ bool DirectoryBackingStore::InitializeTables() { version_on_disk = 82; } + // Version 83 migration added transaction_version column per sync entry. + if (version_on_disk == 82) { + if (MigrateVersion82To83()) + version_on_disk = 83; + } + // If one of the migrations requested it, drop columns that aren't current. // It's only safe to do this after migrating all the way to the current // version. @@ -1072,6 +1078,19 @@ bool DirectoryBackingStore::MigrateVersion81To82() { return true; } +bool DirectoryBackingStore::MigrateVersion82To83() { + // Version 83 added transaction_version on sync node. + if (!db_->Execute( + "ALTER TABLE metas ADD COLUMN transaction_version BIGINT default 0")) + return false; + sql::Statement update(db_->GetUniqueStatement( + "UPDATE metas SET transaction_version = 0")); + if (!update.Run()) + return false; + SetVersion(83); + return true; +} + bool DirectoryBackingStore::CreateTables() { DVLOG(1) << "First run, creating tables"; // Create two little tables share_version and share_info diff --git a/sync/syncable/directory_backing_store.h b/sync/syncable/directory_backing_store.h index ae2b189..a3e17d8 100644 --- a/sync/syncable/directory_backing_store.h +++ b/sync/syncable/directory_backing_store.h @@ -157,6 +157,7 @@ class DirectoryBackingStore : public base::NonThreadSafe { bool MigrateVersion79To80(); bool MigrateVersion80To81(); bool MigrateVersion81To82(); + bool MigrateVersion82To83(); scoped_ptr<sql::Connection> db_; sql::Statement save_entry_statement_; diff --git a/sync/syncable/directory_backing_store_unittest.cc b/sync/syncable/directory_backing_store_unittest.cc index 3cd53c1..f7be920 100644 --- a/sync/syncable/directory_backing_store_unittest.cc +++ b/sync/syncable/directory_backing_store_unittest.cc @@ -66,6 +66,7 @@ class MigrationTest : public testing::TestWithParam<int> { void SetUpVersion79Database(sql::Connection* connection); void SetUpVersion80Database(sql::Connection* connection); void SetUpVersion81Database(sql::Connection* connection); + void SetUpVersion82Database(sql::Connection* connection); void SetUpCurrentDatabaseAndCheckVersion(sql::Connection* connection) { SetUpVersion77Database(connection); // Prepopulates data. @@ -1961,6 +1962,112 @@ void MigrationTest::SetUpVersion81Database(sql::Connection* connection) { ASSERT_TRUE(connection->CommitTransaction()); } +void MigrationTest::SetUpVersion82Database(sql::Connection* connection) { + ASSERT_TRUE(connection->is_open()); + ASSERT_TRUE(connection->BeginTransaction()); + ASSERT_TRUE(connection->Execute( + "CREATE TABLE share_version (id VARCHAR(128) primary key, data INT);" + "INSERT INTO 'share_version' VALUES('nick@chromium.org',81);" + "CREATE TABLE models (model_id BLOB primary key, progress_marker BLOB, in" + "itial_sync_ended BOOLEAN default 0, transaction_version BIGINT " + "default 0);" + "INSERT INTO 'models' VALUES(X'C2881000',X'0888810218B605',1, 1);" + "CREATE TABLE 'metas'(metahandle bigint primary key ON CONFLICT FAIL,base" + "_version bigint default -1,server_version bigint default 0, " + "local_external_id bigint default 0" + ",mtime bigint default 0,server_mtime bigint default 0,ctime bigint d" + "efault 0,server_ctime bigint default 0,id varchar(255) default 'r',p" + "arent_id varchar(255) default 'r',server_parent_id varchar(255) defa" + "ult 'r',prev_id varchar(255) default 'r',next_id varchar(255) defaul" + "t 'r',is_unsynced bit default 0,is_unapplied_update bit default 0,is" + "_del bit default 0,is_dir bit default 0,server_is_dir bit default 0," + "server_is_del bit default 0,non_unique_name varchar,server_non_uniqu" + "e_name varchar(255),unique_server_tag varchar,unique_client_tag varc" + "har,specifics blob,server_specifics blob, base_server_specifics BLOB" + ", server_ordinal_in_parent blob);" + "CREATE TABLE 'share_info' (id TEXT primary key, name TEXT, store_birthda" + "y TEXT, db_create_version TEXT, db_create_time INT, next_id INT defa" + "ult -2, cache_guid TEXT , notification_state BLOB, bag_of_chips " + "blob);" + "INSERT INTO 'share_info' VALUES('nick@chromium.org','nick@chromium.org'," + "'c27e9f59-08ca-46f8-b0cc-f16a2ed778bb','Unknown',1263522064," + "-131078,'9010788312004066376x-6609234393368420856x',NULL, NULL);")); + + const char* insert_stmts[V80_ROW_COUNT] = { + "INSERT INTO 'metas' VALUES(1,-1,0,0," META_PROTO_TIMES_VALS(1) ",'r','" + "r','r','r','r',0,0,0,1,0,0,NULL,NULL,NULL,NULL,X'',X'',NULL,?);", + "INSERT INTO 'metas' VALUES(2,669,669,4," + META_PROTO_TIMES_VALS(2) ",'s_ID_2','s_ID_9','s_ID_9','s_ID_2','s_ID_" + "2',0,0,1,0,0,1,'Deleted Item','Deleted Item',NULL,NULL,X'C28810220A1" + "6687474703A2F2F7777772E676F6F676C652E636F6D2F12084141534741534741',X" + "'C28810260A17687474703A2F2F7777772E676F6F676C652E636F6D2F32120B41534" + "14447414447414447',NULL,?);", + "INSERT INTO 'metas' VALUES(4,681,681,3," + META_PROTO_TIMES_VALS(4) ",'s_ID_4','s_ID_9','s_ID_9','s_ID_4','s_ID_" + "4',0,0,1,0,0,1,'Welcome to Chromium','Welcome to Chromium',NULL,NULL" + ",X'C28810350A31687474703A2F2F7777772E676F6F676C652E636F6D2F6368726F6" + "D652F696E746C2F656E2F77656C636F6D652E68746D6C1200',X'C28810350A31687" + "474703A2F2F7777772E676F6F676C652E636F6D2F6368726F6D652F696E746C2F656" + "E2F77656C636F6D652E68746D6C1200',NULL,?);", + "INSERT INTO 'metas' VALUES(5,677,677,7," + META_PROTO_TIMES_VALS(5) ",'s_ID_5','s_ID_9','s_ID_9','s_ID_5','s_ID_" + "5',0,0,1,0,0,1,'Google','Google',NULL,NULL,X'C28810220A16687474703A2" + "F2F7777772E676F6F676C652E636F6D2F12084147415347415347',X'C28810220A1" + "6687474703A2F2F7777772E676F6F676C652E636F6D2F12084147464447415347',N" + "ULL,?);", + "INSERT INTO 'metas' VALUES(6,694,694,6," + META_PROTO_TIMES_VALS(6) ",'s_ID_6','s_ID_9','s_ID_9','r','r',0,0,0,1" + ",1,0,'The Internet','The Internet',NULL,NULL,X'C2881000',X'C2881000'" + ",NULL,?);", + "INSERT INTO 'metas' VALUES(7,663,663,0," + META_PROTO_TIMES_VALS(7) ",'s_ID_7','r','r','r','r',0,0,0,1,1,0,'Goog" + "le Chrome','Google Chrome','google_chrome',NULL,NULL,NULL,NULL,?);", + "INSERT INTO 'metas' VALUES(8,664,664,0," + META_PROTO_TIMES_VALS(8) ",'s_ID_8','s_ID_7','s_ID_7','r','r',0,0,0,1" + ",1,0,'Bookmarks','Bookmarks','google_chrome_bookmarks',NULL,X'C28810" + "00',X'C2881000',NULL,?);", + "INSERT INTO 'metas' VALUES(9,665,665,1," + META_PROTO_TIMES_VALS(9) ",'s_ID_9','s_ID_8','s_ID_8','r','s_ID_10',0" + ",0,0,1,1,0,'Bookmark Bar','Bookmark Bar','bookmark_bar',NULL,X'C2881" + "000',X'C2881000',NULL,?);", + "INSERT INTO 'metas' VALUES(10,666,666,2," + META_PROTO_TIMES_VALS(10) ",'s_ID_10','s_ID_8','s_ID_8','s_ID_9','r'," + "0,0,0,1,1,0,'Other Bookmarks','Other Bookmarks','other_bookmarks',NU" + "LL,X'C2881000',X'C2881000',NULL,?);", + "INSERT INTO 'metas' VALUES(11,683,683,8," + META_PROTO_TIMES_VALS(11) ",'s_ID_11','s_ID_6','s_ID_6','r','s_ID_13'" + ",0,0,0,0,0,0,'Home (The Chromium Projects)','Home (The Chromium Proj" + "ects)',NULL,NULL,X'C28810220A18687474703A2F2F6465762E6368726F6D69756" + "D2E6F72672F1206414741545741',X'C28810290A1D687474703A2F2F6465762E636" + "8726F6D69756D2E6F72672F6F7468657212084146414756415346',NULL,?);", + "INSERT INTO 'metas' VALUES(12,685,685,9," + META_PROTO_TIMES_VALS(12) ",'s_ID_12','s_ID_6','s_ID_6','s_ID_13','s_" + "ID_14',0,0,0,1,1,0,'Extra Bookmarks','Extra Bookmarks',NULL,NULL,X'C" + "2881000',X'C2881000',NULL,?);", + "INSERT INTO 'metas' VALUES(13,687,687,10," + META_PROTO_TIMES_VALS(13) ",'s_ID_13','s_ID_6','s_ID_6','s_ID_11','s_" + "ID_12',0,0,0,0,0,0,'ICANN | Internet Corporation for Assigned Names " + "and Numbers','ICANN | Internet Corporation for Assigned Names and Nu" + "mbers',NULL,NULL,X'C28810240A15687474703A2F2F7777772E6963616E6E2E636" + "F6D2F120B504E474158463041414646',X'C28810200A15687474703A2F2F7777772" + "E6963616E6E2E636F6D2F120744414146415346',NULL,?);", + "INSERT INTO 'metas' VALUES(14,692,692,11," + META_PROTO_TIMES_VALS(14) ",'s_ID_14','s_ID_6','s_ID_6','s_ID_12','r'" + ",0,0,0,0,0,0,'The WebKit Open Source Project','The WebKit Open Sourc" + "e Project',NULL,NULL,X'C288101A0A12687474703A2F2F7765626B69742E6F726" + "72F1204504E4758',X'C288101C0A13687474703A2F2F7765626B69742E6F72672F7" + "81205504E473259',NULL,?);" }; + + for (int i = 0; i < V80_ROW_COUNT; i++) { + sql::Statement s(connection->GetUniqueStatement(insert_stmts[i])); + std::string ord = V81_Ordinal(i); + s.BindBlob(0, ord.data(), ord.length()); + ASSERT_TRUE(s.Run()); + s.Reset(true); + } + ASSERT_TRUE(connection->CommitTransaction()); +} + TEST_F(DirectoryBackingStoreTest, MigrateVersion67To68) { sql::Connection connection; ASSERT_TRUE(connection.OpenInMemory()); @@ -2388,6 +2495,20 @@ TEST_F(DirectoryBackingStoreTest, MigrateVersion81To82) { ASSERT_TRUE(connection.DoesColumnExist("models", "transaction_version")); } +TEST_F(DirectoryBackingStoreTest, MigrateVersion82To83) { + sql::Connection connection; + ASSERT_TRUE(connection.OpenInMemory()); + SetUpVersion82Database(&connection); + ASSERT_FALSE(connection.DoesColumnExist("metas", "transaction_version")); + + scoped_ptr<TestDirectoryBackingStore> dbs( + new TestDirectoryBackingStore(GetUsername(), &connection)); + ASSERT_TRUE(dbs->MigrateVersion82To83()); + ASSERT_EQ(83, dbs->GetVersion()); + + ASSERT_TRUE(connection.DoesColumnExist("metas", "transaction_version")); +} + TEST_P(MigrationTest, ToCurrentVersion) { sql::Connection connection; ASSERT_TRUE(connection.OpenInMemory()); @@ -2437,6 +2558,9 @@ TEST_P(MigrationTest, ToCurrentVersion) { case 81: SetUpVersion81Database(&connection); break; + case 82: + SetUpVersion82Database(&connection); + break; default: // If you see this error, it may mean that you've increased the // database version number but you haven't finished adding unit tests @@ -2519,6 +2643,9 @@ TEST_P(MigrationTest, ToCurrentVersion) { // Column added in version 82. ASSERT_TRUE(connection.DoesColumnExist("models", "transaction_version")); + // Column added in version 83. + ASSERT_TRUE(connection.DoesColumnExist("metas", "transaction_version")); + // Check download_progress state (v75 migration) ASSERT_EQ(694, dir_info.kernel_info.download_progress[BOOKMARKS] diff --git a/sync/syncable/directory_change_delegate.h b/sync/syncable/directory_change_delegate.h index 2afd621..725a344 100644 --- a/sync/syncable/directory_change_delegate.h +++ b/sync/syncable/directory_change_delegate.h @@ -23,12 +23,16 @@ namespace syncable { // Note that these methods may be called on *any* thread. class DirectoryChangeDelegate { public: + // Returns the handles of changed entries in |entry_changed|. virtual void HandleCalculateChangesChangeEventFromSyncApi( const ImmutableWriteTransactionInfo& write_transaction_info, - BaseTransaction* trans) = 0; + BaseTransaction* trans, + std::vector<int64>* entries_changed) = 0; + // Returns the handles of changed entries in |entry_changed|. virtual void HandleCalculateChangesChangeEventFromSyncer( const ImmutableWriteTransactionInfo& write_transaction_info, - BaseTransaction* trans) = 0; + BaseTransaction* trans, + std::vector<int64>* entries_changed) = 0; // Must return the set of all ModelTypes that were modified in the // transaction. virtual ModelTypeSet HandleTransactionEndingChangeEvent( diff --git a/sync/syncable/entry_kernel.h b/sync/syncable/entry_kernel.h index e8c0924..842aae4 100644 --- a/sync/syncable/entry_kernel.h +++ b/sync/syncable/entry_kernel.h @@ -50,6 +50,7 @@ enum Int64Field { SERVER_VERSION = BASE_VERSION + 1, LOCAL_EXTERNAL_ID, // ID of an item in the external local storage that this // entry is associated with. (such as bookmarks.js) + TRANSACTION_VERSION, INT64_FIELDS_END }; diff --git a/sync/syncable/syncable_columns.h b/sync/syncable/syncable_columns.h index f5d48e0..6eda058 100644 --- a/sync/syncable/syncable_columns.h +++ b/sync/syncable/syncable_columns.h @@ -25,6 +25,7 @@ static const ColumnSpec g_metas_columns[] = { {"server_version", "bigint default 0"}, // This is the item ID that we store for the embedding application. {"local_external_id", "bigint default 0"}, + {"transaction_version", "bigint default 0"}, // These timestamps are kept in the same format as that of the // protocol (ms since Unix epoch). {"mtime", "bigint default 0"}, diff --git a/sync/syncable/syncable_enum_conversions.cc b/sync/syncable/syncable_enum_conversions.cc index 3fdd7a8..b4425a6 100644 --- a/sync/syncable/syncable_enum_conversions.cc +++ b/sync/syncable/syncable_enum_conversions.cc @@ -45,11 +45,12 @@ const char* GetBaseVersionString(BaseVersion base_version) { } const char* GetInt64FieldString(Int64Field int64_field) { - ASSERT_ENUM_BOUNDS(SERVER_VERSION, LOCAL_EXTERNAL_ID, + ASSERT_ENUM_BOUNDS(SERVER_VERSION, TRANSACTION_VERSION, BASE_VERSION + 1, INT64_FIELDS_END - 1); switch (int64_field) { ENUM_CASE(SERVER_VERSION); ENUM_CASE(LOCAL_EXTERNAL_ID); + ENUM_CASE(TRANSACTION_VERSION); case INT64_FIELDS_END: break; } NOTREACHED(); @@ -57,8 +58,8 @@ const char* GetInt64FieldString(Int64Field int64_field) { } const char* GetTimeFieldString(TimeField time_field) { - ASSERT_ENUM_BOUNDS(SERVER_VERSION, LOCAL_EXTERNAL_ID, - BASE_VERSION + 1, INT64_FIELDS_END - 1); + ASSERT_ENUM_BOUNDS(MTIME, SERVER_CTIME, + TIME_FIELDS_BEGIN, TIME_FIELDS_END - 1); switch (time_field) { ENUM_CASE(MTIME); ENUM_CASE(SERVER_MTIME); diff --git a/sync/syncable/syncable_unittest.cc b/sync/syncable/syncable_unittest.cc index 65cedc3..8a49ba4 100644 --- a/sync/syncable/syncable_unittest.cc +++ b/sync/syncable/syncable_unittest.cc @@ -1676,6 +1676,7 @@ TEST_F(OnDiskSyncableDirectoryTest, specifics.mutable_bookmark()->set_favicon("PNG"); specifics.mutable_bookmark()->set_url("http://nowhere"); create.Put(SPECIFICS, specifics); + update.Put(SPECIFICS, specifics); create_pre_save = create.GetKernelCopy(); update_pre_save = update.GetKernelCopy(); create_id = create.Get(ID); @@ -1702,10 +1703,12 @@ TEST_F(OnDiskSyncableDirectoryTest, } int i = BEGIN_FIELDS; for ( ; i < INT64_FIELDS_END ; ++i) { - EXPECT_EQ(create_pre_save.ref((Int64Field)i), + EXPECT_EQ(create_pre_save.ref((Int64Field)i) + + (i == TRANSACTION_VERSION ? 1 : 0), create_post_save.ref((Int64Field)i)) << "int64 field #" << i << " changed during save/load"; - EXPECT_EQ(update_pre_save.ref((Int64Field)i), + EXPECT_EQ(update_pre_save.ref((Int64Field)i) + + (i == TRANSACTION_VERSION ? 1 : 0), update_post_save.ref((Int64Field)i)) << "int64 field #" << i << " changed during save/load"; } diff --git a/sync/syncable/write_transaction.cc b/sync/syncable/write_transaction.cc index 09759b6..bbd9dc4 100644 --- a/sync/syncable/write_transaction.cc +++ b/sync/syncable/write_transaction.cc @@ -6,16 +6,30 @@ #include "sync/syncable/directory.h" #include "sync/syncable/directory_change_delegate.h" +#include "sync/syncable/mutable_entry.h" #include "sync/syncable/transaction_observer.h" #include "sync/syncable/write_transaction_info.h" namespace syncer { namespace syncable { +const int64 kInvalidTransactionVersion = -1; + WriteTransaction::WriteTransaction(const tracked_objects::Location& location, WriterTag writer, Directory* directory) - : BaseTransaction(location, "WriteTransaction", writer, directory) { + : BaseTransaction(location, "WriteTransaction", writer, directory), + transaction_version_(NULL) { + Lock(); +} + +WriteTransaction::WriteTransaction(const tracked_objects::Location& location, + Directory* directory, + int64* transaction_version) + : BaseTransaction(location, "WriteTransaction", SYNCAPI, directory), + transaction_version_(transaction_version) { Lock(); + if (transaction_version_) + *transaction_version_ = kInvalidTransactionVersion; } void WriteTransaction::SaveOriginal(const EntryKernel* entry) { @@ -82,13 +96,15 @@ ModelTypeSet WriteTransaction::NotifyTransactionChangingAndEnding( ImmutableWriteTransactionInfo immutable_write_transaction_info( &write_transaction_info); DirectoryChangeDelegate* const delegate = directory_->kernel_->delegate; + std::vector<int64> entry_changed; if (writer_ == syncable::SYNCAPI) { delegate->HandleCalculateChangesChangeEventFromSyncApi( - immutable_write_transaction_info, this); + immutable_write_transaction_info, this, &entry_changed); } else { delegate->HandleCalculateChangesChangeEventFromSyncer( - immutable_write_transaction_info, this); + immutable_write_transaction_info, this, &entry_changed); } + UpdateTransactionVersion(entry_changed); ModelTypeSet models_with_changes = delegate->HandleTransactionEndingChangeEvent( @@ -107,6 +123,30 @@ void WriteTransaction::NotifyTransactionComplete( models_with_changes); } +void WriteTransaction::UpdateTransactionVersion( + const std::vector<int64>& entry_changed) { + syncer::ModelTypeSet type_seen; + for (uint32 i = 0; i < entry_changed.size(); ++i) { + MutableEntry entry(this, GET_BY_HANDLE, entry_changed[i]); + if (entry.good()) { + ModelType type = GetModelTypeFromSpecifics(entry.Get(SPECIFICS)); + if (type < FIRST_REAL_MODEL_TYPE) + continue; + if (!type_seen.Has(type)) { + directory_->IncrementTransactionVersion(type); + type_seen.Put(type); + } + entry.Put(TRANSACTION_VERSION, directory_->GetTransactionVersion(type)); + } + } + + if (!type_seen.Empty() && transaction_version_) { + DCHECK_EQ(1u, type_seen.Size()); + *transaction_version_ = directory_->GetTransactionVersion( + type_seen.First().Get()); + } +} + WriteTransaction::~WriteTransaction() { const ImmutableEntryKernelMutationMap& mutations = RecordMutations(); directory()->CheckInvariantsOnTransactionClose(this, mutations.Get()); diff --git a/sync/syncable/write_transaction.h b/sync/syncable/write_transaction.h index 17264c4..6a36963 100644 --- a/sync/syncable/write_transaction.h +++ b/sync/syncable/write_transaction.h @@ -11,12 +11,22 @@ namespace syncer { namespace syncable { +extern const int64 kInvalidTransactionVersion; + // Locks db in constructor, unlocks in destructor. class WriteTransaction : public BaseTransaction { public: WriteTransaction(const tracked_objects::Location& from_here, WriterTag writer, Directory* directory); + // Constructor used for getting back transaction version after making sync + // API changes to one model. If model is changed by the transaction, + // the new transaction version of the model and modified nodes will be saved + // in |transaction_version| upon destruction of the transaction. If model is + // not changed, |transaction_version| will be kInvalidTransactionVersion. + WriteTransaction(const tracked_objects::Location& from_here, + Directory* directory, int64* transaction_version); + virtual ~WriteTransaction(); void SaveOriginal(const EntryKernel* entry); @@ -36,10 +46,18 @@ class WriteTransaction : public BaseTransaction { ModelTypeSet NotifyTransactionChangingAndEnding( const ImmutableEntryKernelMutationMap& mutations); + // Increment versions of the models whose entries are modified and set the + // version on the changed entries. + void UpdateTransactionVersion(const std::vector<int64>& entry_changed); + // Only the original fields are filled in until |RecordMutations()|. // We use a mutation map instead of a kernel set to avoid copying. EntryKernelMutationMap mutations_; + // Stores new transaction version of changed model and nodes if model is + // indeed changed. kInvalidTransactionVersion otherwise. Not owned. + int64* transaction_version_; + DISALLOW_COPY_AND_ASSIGN(WriteTransaction); }; diff --git a/sync/test/null_directory_change_delegate.cc b/sync/test/null_directory_change_delegate.cc index e28fc6a..5e2a93d 100644 --- a/sync/test/null_directory_change_delegate.cc +++ b/sync/test/null_directory_change_delegate.cc @@ -11,16 +11,29 @@ NullDirectoryChangeDelegate::~NullDirectoryChangeDelegate() {} void NullDirectoryChangeDelegate::HandleCalculateChangesChangeEventFromSyncApi( const ImmutableWriteTransactionInfo& write_transaction_info, - BaseTransaction* trans) {} + BaseTransaction* trans, + std::vector<int64>* entries_changed) { + for (EntryKernelMutationMap::const_iterator it = + write_transaction_info.Get().mutations.Get().begin(); + it != write_transaction_info.Get().mutations.Get().end(); ++it) { + entries_changed->push_back(it->first); + } +} void NullDirectoryChangeDelegate::HandleCalculateChangesChangeEventFromSyncer( const ImmutableWriteTransactionInfo& write_transaction_info, - BaseTransaction* trans) {} + BaseTransaction* trans, + std::vector<int64>* entries_changed) { + for (EntryKernelMutationMap::const_iterator it = + write_transaction_info.Get().mutations.Get().begin(); + it != write_transaction_info.Get().mutations.Get().end(); ++it) { + entries_changed->push_back(it->first); + } +} -ModelTypeSet - NullDirectoryChangeDelegate::HandleTransactionEndingChangeEvent( - const ImmutableWriteTransactionInfo& write_transaction_info, - BaseTransaction* trans) { +ModelTypeSet NullDirectoryChangeDelegate::HandleTransactionEndingChangeEvent( + const ImmutableWriteTransactionInfo& write_transaction_info, + BaseTransaction* trans) { return ModelTypeSet(); } diff --git a/sync/test/null_directory_change_delegate.h b/sync/test/null_directory_change_delegate.h index 4fdba374..25ddd1b 100644 --- a/sync/test/null_directory_change_delegate.h +++ b/sync/test/null_directory_change_delegate.h @@ -18,10 +18,12 @@ class NullDirectoryChangeDelegate : public DirectoryChangeDelegate { virtual void HandleCalculateChangesChangeEventFromSyncApi( const ImmutableWriteTransactionInfo& write_transaction_info, - BaseTransaction* trans) OVERRIDE; + BaseTransaction* trans, + std::vector<int64>* entries_changed) OVERRIDE; virtual void HandleCalculateChangesChangeEventFromSyncer( const ImmutableWriteTransactionInfo& write_transaction_info, - BaseTransaction* trans) OVERRIDE; + BaseTransaction* trans, + std::vector<int64>* entries_changed) OVERRIDE; virtual ModelTypeSet HandleTransactionEndingChangeEvent( const ImmutableWriteTransactionInfo& write_transaction_info, BaseTransaction* trans) OVERRIDE; diff --git a/sync/test/test_directory_backing_store.h b/sync/test/test_directory_backing_store.h index bb90c65..6d2acb4 100644 --- a/sync/test/test_directory_backing_store.h +++ b/sync/test/test_directory_backing_store.h @@ -44,6 +44,7 @@ class TestDirectoryBackingStore : public DirectoryBackingStore { FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, MigrateVersion79To80); FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, MigrateVersion80To81); FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, MigrateVersion81To82); + FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, MigrateVersion82To83); FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, DetectInvalidOrdinal); FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, ModelTypeIds); FRIEND_TEST_ALL_PREFIXES(DirectoryBackingStoreTest, Corruption); |