Remove initial_sync_ended bits

sync/syncable/directory_backing_store.cc

 // Copyright (c) 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/syncable/directory_backing_store.h"
 
 #include "build/build_config.h"
 
 #include <limits>
 
@@ skipping 22 matching lines @@
 
 namespace syncer {
 namespace syncable {
 
 // This just has to be big enough to hold an UPDATE or INSERT statement that
 // modifies all the columns in the entry table.
 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 = 83;
+const int32 kCurrentDBVersion = 84;
 
 // Iterate over the fields of |entry| and bind each to |statement| for
 // updating.  Returns the number of args bound.
 void BindFields(const EntryKernel& entry,
                 sql::Statement* statement) {
   int index = 0;
   int i = 0;
   for (i = BEGIN_FIELDS; i < INT64_FIELDS_END; ++i) {
     statement->BindInt64(index++, entry.ref(static_cast<Int64Field>(i)));
   }
@@ skipping 136 matching lines @@
 }
 
 bool DirectoryBackingStore::SaveChanges(
     const Directory::SaveChangesSnapshot& snapshot) {
   DCHECK(CalledOnValidThread());
   DCHECK(db_->is_open());
 
   // Back out early if there is nothing to write.
   bool save_info =
     (Directory::KERNEL_SHARE_INFO_DIRTY == snapshot.kernel_info_status);
-  if (snapshot.dirty_metas.size() < 1 && !save_info)
+  if (snapshot.dirty_metas.empty()
+      && snapshot.metahandles_to_purge.empty()
+      && !save_info) {
     return true;
+  }
 
   sql::Transaction transaction(db_.get());
   if (!transaction.Begin())
     return false;
 
   for (EntryKernelSet::const_iterator i = snapshot.dirty_metas.begin();
        i != snapshot.dirty_metas.end(); ++i) {
     DCHECK(i->is_dirty());
     if (!SaveEntryToDB(*i))
       return false;
@@ skipping 17 matching lines @@
                    info.notification_state.size());
     s1.BindBlob(3, info.bag_of_chips.data(), info.bag_of_chips.size());
 
     if (!s1.Run())
       return false;
     DCHECK_EQ(db_->GetLastChangeCount(), 1);
 
     sql::Statement s2(db_->GetCachedStatement(
             SQL_FROM_HERE,
             "INSERT OR REPLACE "
-            "INTO models (model_id, progress_marker, initial_sync_ended, "
-            "             transaction_version) "
-            "VALUES (?, ?, ?, ?)"));
+            "INTO models (model_id, progress_marker, transaction_version) "
+            "VALUES (?, ?, ?)"));
 
     for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) {
       // We persist not ModelType but rather a protobuf-derived ID.
       string model_id = ModelTypeEnumToModelId(ModelTypeFromInt(i));
       string progress_marker;
       info.download_progress[i].SerializeToString(&progress_marker);
       s2.BindBlob(0, model_id.data(), model_id.length());
       s2.BindBlob(1, progress_marker.data(), progress_marker.length());
-      s2.BindBool(2, info.initial_sync_ended.Has(ModelTypeFromInt(i)));
-      s2.BindInt64(3, info.transaction_version[i]);
+      s2.BindInt64(2, info.transaction_version[i]);
       if (!s2.Run())
         return false;
       DCHECK_EQ(db_->GetLastChangeCount(), 1);
       s2.Reset(true);
     }
   }
 
   return transaction.Commit();
 }
 
@@ skipping 95 matching lines @@
     if (MigrateVersion81To82())
       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;
   }
 
+  // Version 84 migration removes the initial_sync_ended bits.
+  if (version_on_disk == 83) {
+    if (MigrateVersion83To84())
+      version_on_disk = 84;
+  }
+
   // 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.
   if (version_on_disk == kCurrentDBVersion && needs_column_refresh_) {
     if (!RefreshColumns())
       version_on_disk = 0;
   }
 
   // A final, alternative catch-all migration to simply re-sync everything.
   //
   // TODO(rlarocque): It's wrong to recreate the database here unless the higher

[rlarocque, 2012/12/12 01:19:16]

I've been meaning to delete this comment for a while.  It's been obsolete for ~6
months.

   // layers were expecting us to do so.  See crbug.com/103824.  We must leave
   // this code as is for now because this is the code that ends up creating the
   // database in the first time sync case, where the higher layers are expecting
   // us to create a fresh database.  The solution to this should be to implement
   // crbug.com/105018.
   if (version_on_disk != kCurrentDBVersion) {
     if (version_on_disk > kCurrentDBVersion)
       return false;
 
     // Fallback (re-sync everything) migration path.
@@ skipping 107 matching lines @@
     s.ColumnBlobAsString(4, &(info->kernel_info.bag_of_chips));
 
     // Verify there was only one row returned.
     DCHECK(!s.Step());
     DCHECK(s.Succeeded());
   }
 
   {
     sql::Statement s(
         db_->GetUniqueStatement(
-            "SELECT model_id, progress_marker, initial_sync_ended, "
+            "SELECT model_id, progress_marker, "
             "transaction_version FROM models"));
 
     while (s.Step()) {
       ModelType type = ModelIdToModelTypeEnum(s.ColumnBlob(0),
                                               s.ColumnByteLength(0));
       if (type != UNSPECIFIED && type != TOP_LEVEL_FOLDER) {
         info->kernel_info.download_progress[type].ParseFromArray(
             s.ColumnBlob(1), s.ColumnByteLength(1));
-        if (s.ColumnBool(2))
-          info->kernel_info.initial_sync_ended.Put(type);
-        info->kernel_info.transaction_version[type] = s.ColumnInt64(3);
+        info->kernel_info.transaction_version[type] = s.ColumnInt64(2);
       }
     }
     if (!s.Succeeded())
       return false;
   }
   {
     sql::Statement s(
         db_->GetUniqueStatement(
             "SELECT MAX(metahandle) FROM metas"));
     if (!s.Step())
@@ skipping 374 matching lines @@
   //     boolean initial_sync_ended
   // In version 75, we deprecated the integer-valued last_download_timestamp,
   // using insted a protobuf-valued progress_marker field:
   //     blob progress_marker
   // The progress_marker values are initialized from the value of
   // last_download_timestamp, thereby preserving the download state.
 
   // Move aside the old table and create a new empty one at the current schema.
   if (!db_->Execute("ALTER TABLE models RENAME TO temp_models"))
     return false;
-  if (!CreateModelsTable())
+  if (!CreateV75ModelsTable())
     return false;
 
   sql::Statement query(db_->GetUniqueStatement(
           "SELECT model_id, last_download_timestamp, initial_sync_ended "
           "FROM temp_models"));
 
   sql::Statement update(db_->GetUniqueStatement(
           "INSERT INTO models (model_id, "
           "progress_marker, initial_sync_ended) VALUES (?, ?, ?)"));
 
@@ skipping 130 matching lines @@
       return false;
     put_ordinals.Reset(true);
   }
 
   SetVersion(81);
   needs_column_refresh_ = true;
   return true;
 }
 
 bool DirectoryBackingStore::MigrateVersion81To82() {
   // Version 82 added transaction_version to kernel info. But if user is

[rlarocque, 2012/12/12 01:19:16]

This is the wrong way to handle a change in the CreateModelsTable() function. 
The right way is to modify the Version74->75 migration to use a different
CreateModelsTable() function, one that's frozen in time and won't be affected by
future changes.  That's what I've done in this change.

   // migrating from 74 or before, 74->75 migration would recreate models table
   // that already has transaction_version column.
   if (db_->DoesColumnExist("models", "transaction_version")) {
     SetVersion(82);
     return true;
   }
 
   if (!db_->Execute(
       "ALTER TABLE models ADD COLUMN transaction_version BIGINT default 0"))
     return false;
@@ skipping 11 matching lines @@
       "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::MigrateVersion83To84() {
+  // Version 84 removes the initial_sync_ended flag.
+  if (!db_->Execute("ALTER TABLE models RENAME TO temp_models"))
+    return false;
+  if (!CreateModelsTable())
+    return false;
+  if (!db_->Execute("INSERT INTO models SELECT "
+                    "model_id, progress_marker, transaction_version "
+                    "FROM temp_models")) {
+    return false;
+  }
+  SafeDropTable("temp_models");
+  SetVersion(84);
+  return true;
+}
+
 bool DirectoryBackingStore::CreateTables() {
   DVLOG(1) << "First run, creating tables";
   // Create two little tables share_version and share_info
   if (!db_->Execute(
           "CREATE TABLE share_version ("
           "id VARCHAR(128) primary key, data INT)")) {
     return false;
   }
 
   {
@@ skipping 77 matching lines @@
   return db_->Execute(
       "CREATE TABLE models ("
       "model_id BLOB primary key, "
       "last_download_timestamp INT, "
       // Gets set if the syncer ever gets updates from the
       // server and the server returns 0.  Lets us detect the
       // end of the initial sync.
       "initial_sync_ended BOOLEAN default 0)");
 }
 
+bool DirectoryBackingStore::CreateV75ModelsTable() {
+  // This is an old schema for the Models table, used from versions 75 to 83.
+  return db_->Execute(
+      "CREATE TABLE models ("
+      "model_id BLOB primary key, "
+      "progress_marker BLOB, "
+      // Gets set if the syncer ever gets updates from the
+      // server and the server returns 0.  Lets us detect the
+      // end of the initial sync.
+      "initial_sync_ended BOOLEAN default 0, "
+      "transaction_version BIGINT default 0)");
+}
+
 bool DirectoryBackingStore::CreateModelsTable() {
   // This is the current schema for the Models table, from version 81
   // onward.  If you change the schema, you'll probably want to double-check
-  // the use of this function in the v74-v75 migration.
+  // the use of this function in the v83-v84 migration.
   return db_->Execute(
       "CREATE TABLE models ("
       "model_id BLOB primary key, "
       "progress_marker BLOB, "
       // Gets set if the syncer ever gets updates from the
       // server and the server returns 0.  Lets us detect the
       // end of the initial sync.
-      "initial_sync_ended BOOLEAN default 0, "
       "transaction_version BIGINT default 0)");
 }
 
 bool DirectoryBackingStore::CreateShareInfoTable(bool is_temporary) {
   const char* name = is_temporary ? "temp_share_info" : "share_info";
   string query = "CREATE TABLE ";
   query.append(name);
   // This is the current schema for the ShareInfo table, from version 76
   // onward.
   query.append(" ("
@@ skipping 53 matching lines @@
     bool prev_exists = (ids_set.find(entry->ref(PREV_ID).value()) != end);
     bool parent_exists = (ids_set.find(entry->ref(PARENT_ID).value()) != end);
     bool next_exists = (ids_set.find(entry->ref(NEXT_ID).value()) != end);
     is_ok = is_ok && prev_exists && parent_exists && next_exists;
   }
   return is_ok;
 }
 
 }  // namespace syncable
 }  // namespace syncer