Changed DB to store node positions as Ordinals.

sync/syncable/directory_backing_store.cc

Index: sync/syncable/directory_backing_store.cc
diff --git a/sync/syncable/directory_backing_store.cc b/sync/syncable/directory_backing_store.cc
index 9b8e0dba855edc22128e0cb4aab84d96ec485da5..fb8fbdc2331e998fcbf5b20148619595710041e2 100644
--- a/sync/syncable/directory_backing_store.cc
+++ b/sync/syncable/directory_backing_store.cc
@@ -22,6 +22,7 @@
 #include "sql/connection.h"
 #include "sql/statement.h"
 #include "sql/transaction.h"
+#include "sync/internal_api/public/base/node_ordinal.h"
 #include "sync/protocol/bookmark_specifics.pb.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/syncable/syncable-inl.h"
@@ -39,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 = 80;
+const int32 kCurrentDBVersion = 81;
 
 // Iterate over the fields of |entry| and bind each to |statement| for
 // updating.  Returns the number of args bound.
@@ -69,6 +70,10 @@ void BindFields(const EntryKernel& entry,
     entry.ref(static_cast<ProtoField>(i)).SerializeToString(&temp);
     statement->BindBlob(index++, temp.data(), temp.length());
   }
+  for( ; i < ORDINAL_FIELDS_END; ++i) {
+    temp = entry.ref(static_cast<OrdinalField>(i)).ToInternalValue();
+    statement->BindBlob(index++, temp.data(), temp.length());
+  }
 }
 
 // The caller owns the returned EntryKernel*.  Assumes the statement currently
@@ -99,6 +104,11 @@ EntryKernel* UnpackEntry(sql::Statement* statement) {
     kernel->mutable_ref(static_cast<ProtoField>(i)).ParseFromArray(
         statement->ColumnBlob(i), statement->ColumnByteLength(i));
   }
+  for( ; i < ORDINAL_FIELDS_END; ++i) {
+    std::string temp;
+    statement->ColumnBlobAsString(i, &temp);
+    kernel->mutable_ref(static_cast<OrdinalField>(i)) = NodeOrdinal(temp);

[akalin, 2012/10/05 00:57:59]

how should we handle invalid NodeOrdinals?  DCHECK/CHECK/unrecoverable error,
create brand new ordinal, something else?  ideas, vishwash / richard?

[rlarocque, 2012/10/05 01:10:50]

One option would be to fail to load the directory and declare it corrupt.  This
would lead to it being deleted and the user's sync data re-downloaded.

I think that Ordinal's methods do a pretty good job of maintaining invariants. 
If we trust that code, then we can assume that invalid ordinals can't/won't be
generated by Chrome, so it's valid to assume something outside chrome caused the
issue.

[vishwath, 2012/10/05 18:34:49]

Done.

+  }
   return kernel;
 }
 
@@ -323,6 +333,13 @@ bool DirectoryBackingStore::InitializeTables() {
       version_on_disk = 80;
   }
 
+  // Version 81 changes the server_position_in_parent_field from an int64
+  // to a blob.
+  if (version_on_disk == 80) {
+    if (MigrateVersion80To81())
+      version_on_disk = 81;
+  }
+
   // 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.
@@ -503,7 +520,7 @@ bool DirectoryBackingStore::SaveEntryToDB(const EntryKernel& entry) {
     values.append("VALUES ");
     const char* separator = "( ";
     int i = 0;
-    for (i = BEGIN_FIELDS; i < PROTO_FIELDS_END; ++i) {
+    for (i = BEGIN_FIELDS; i < ORDINAL_FIELDS_END; ++i) {

[akalin, 2012/10/05 00:57:59]

can you use FIELD_COUNT instead of ORDINAL_FIELDS_END?

(maybe even define FIELDS_END = FIELD_COUNT)

[vishwath, 2012/10/05 18:34:49]

Called it END_FIELDS to match with BEGIN_FIELDS, but essentially done.
On 2012/10/05 00:57:59, akalin wrote:
Done.

       query.append(separator);
       values.append(separator);
       separator = ", ";
@@ -968,6 +985,7 @@ bool DirectoryBackingStore::MigrateVersion78To79() {
   SetVersion(79);
   return true;
 }
+
 bool DirectoryBackingStore::MigrateVersion79To80() {
   if (!db_->Execute(
           "ALTER TABLE share_info ADD COLUMN bag_of_chips BLOB"))
@@ -982,6 +1000,36 @@ bool DirectoryBackingStore::MigrateVersion79To80() {
   return true;
 }
 
+bool DirectoryBackingStore::MigrateVersion80To81() {
+  if(!db_->Execute(
+         "ALTER TABLE metas ADD COLUMN server_ordinal_in_parent BLOB"))
+    return false;
+
+  sql::Statement get_positions(db_->GetUniqueStatement(
+      "SELECT metahandle, server_position_in_parent FROM metas"));
+
+  sql::Statement put_ordinals(db_->GetUniqueStatement(
+      "UPDATE metas SET server_ordinal_in_parent = ?"
+      "WHERE metahandle = ?"));
+
+  while(get_positions.Step()) {
+    int64 metahandle = get_positions.ColumnInt64(0);
+    int64 position = get_positions.ColumnInt64(1);
+
+    std::string ordinal = Int64ToNodeOrdinal(position).ToInternalValue();

[rlarocque, 2012/10/05 19:02:55]

I think you can make this string a const ref.

[vishwath, 2012/10/05 21:05:48]

Done.

+    put_ordinals.BindBlob(0, ordinal.data(), ordinal.length());
+    put_ordinals.BindInt64(1, metahandle);
+
+    if(!put_ordinals.Run())
+      return false;
+    put_ordinals.Reset(true);
+  }
+
+  SetVersion(81);
+  needs_column_refresh_ = true;
+  return true;
+}
+
 bool DirectoryBackingStore::CreateTables() {
   DVLOG(1) << "First run, creating tables";
   // Create two little tables share_version and share_info