Changed DB to store node positions as Ordinals.

sync/syncable/directory_backing_store_unittest.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 "testing/gtest/include/gtest/gtest.h"
 
 #include <string>
 
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/scoped_temp_dir.h"
 #include "base/stl_util.h"
 #include "base/string_number_conversions.h"
 #include "sql/connection.h"
 #include "sql/statement.h"
 #include "sync/protocol/bookmark_specifics.pb.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/syncable/directory_backing_store.h"
 #include "sync/syncable/on_disk_directory_backing_store.h"
 #include "sync/syncable/syncable-inl.h"
 #include "sync/test/test_directory_backing_store.h"
 #include "sync/util/time.h"
 #include "testing/gtest/include/gtest/gtest-param-test.h"
+#include "sync/internal_api/public/base/ordinal.h"
+#include "sync/internal_api/public/base/node_ordinal.h"
 
 namespace syncer {
 namespace syncable {
 
 extern const int32 kCurrentDBVersion;
 
 class MigrationTest : public testing::TestWithParam<int> {
  public:
   virtual void SetUp() {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
@@ skipping 21 matching lines @@
   void SetUpVersion70Database(sql::Connection* connection);
   void SetUpVersion71Database(sql::Connection* connection);
   void SetUpVersion72Database(sql::Connection* connection);
   void SetUpVersion73Database(sql::Connection* connection);
   void SetUpVersion74Database(sql::Connection* connection);
   void SetUpVersion75Database(sql::Connection* connection);
   void SetUpVersion76Database(sql::Connection* connection);
   void SetUpVersion77Database(sql::Connection* connection);
   void SetUpVersion78Database(sql::Connection* connection);
   void SetUpVersion79Database(sql::Connection* connection);
+  void SetUpVersion80Database(sql::Connection* connection);
 
   void SetUpCurrentDatabaseAndCheckVersion(sql::Connection* connection) {
     SetUpVersion77Database(connection);  // Prepopulates data.
     scoped_ptr<TestDirectoryBackingStore> dbs(
         new TestDirectoryBackingStore(GetUsername(), connection));
 
     ASSERT_TRUE(LoadAndIgnoreReturnedData(dbs.get()));
     ASSERT_FALSE(dbs->needs_column_refresh_);
     ASSERT_EQ(kCurrentDBVersion, dbs->GetVersion());
   }
@@ skipping 1552 matching lines @@
           "9010788312004066376x-6609234393368420856x',NULL);"
           ));
   ASSERT_TRUE(connection->CommitTransaction());
 }
 
 void MigrationTest::SetUpVersion79Database(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',78);"
+      "INSERT INTO 'share_version' VALUES('nick@chromium.org',79);"
       "CREATE TABLE models (model_id BLOB primary key, progress_marker BLOB, in"
           "itial_sync_ended BOOLEAN default 0);"
       "INSERT INTO 'models' VALUES(X'C2881000',X'0888810218B605',1);"
       "CREATE TABLE 'metas'(metahandle bigint primary key ON CONFLICT FAIL,base"
           "_version bigint default -1,server_version bigint default 0,server_po"
           "sition_in_parent 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"
@@ skipping 69 matching lines @@
       "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);"
       "INSERT INTO 'share_info' VALUES('nick@chromium.org','nick@chromium.org',"
           "'c27e9f59-08ca-46f8-b0cc-f16a2ed778bb','Unknown',1263522064,"
           "-131078,'9010788312004066376x-6609234393368420856x',NULL);"
           ));
   ASSERT_TRUE(connection->CommitTransaction());
 }
 
+void MigrationTest::SetUpVersion80Database(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',80);"
+      "CREATE TABLE models (model_id BLOB primary key, progress_marker BLOB, in"
+          "itial_sync_ended BOOLEAN default 0);"
+      "INSERT INTO 'models' VALUES(X'C2881000',X'0888810218B605',1);"
+      "CREATE TABLE 'metas'(metahandle bigint primary key ON CONFLICT FAIL,base"
+          "_version bigint default -1,server_version bigint default 0,server_po"
+          "sition_in_parent 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"
+          ");"
+      "INSERT INTO 'metas' VALUES(1,-1,0,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,-2097152,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,-3145728,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,1048576,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,-4194304,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,1048576,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,1048576,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,1048576,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,2097152,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,-1048576,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,0,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,-917504,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,1048576,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);"
+      "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);"
+          ));
+  ASSERT_TRUE(connection->CommitTransaction());
+}
+
 TEST_F(DirectoryBackingStoreTest, MigrateVersion67To68) {
   sql::Connection connection;
   ASSERT_TRUE(connection.OpenInMemory());
 
   SetUpVersion67Database(&connection);
 
   // Columns existing before version 67.
   ASSERT_TRUE(connection.DoesColumnExist("metas", "name"));
   ASSERT_TRUE(connection.DoesColumnExist("metas", "unsanitized_name"));
   ASSERT_TRUE(connection.DoesColumnExist("metas", "server_name"));
@@ skipping 334 matching lines @@
   Directory::KernelLoadInfo load_info;
 
   ASSERT_TRUE(dbs->Load(&entry_bucket, &load_info));
   // Check that the initial value is the serialization of an empty ChipBag.
   sync_pb::ChipBag chip_bag;
   std::string serialized_chip_bag;
   ASSERT_TRUE(chip_bag.SerializeToString(&serialized_chip_bag));
   EXPECT_EQ(serialized_chip_bag, load_info.kernel_info.bag_of_chips);
 }
 
+TEST_F(DirectoryBackingStoreTest, MigrateVersion80To81) {
+  sql::Connection connection;
+  ASSERT_TRUE(connection.OpenInMemory());
+  SetUpVersion80Database(&connection);
+
+  sql::Statement s(connection.GetUniqueStatement(

[rlarocque, 2012/09/29 01:13:42]

You might be able to implement this with PRAGMA TABLE_INFO.  I'm unsure whether
or not that's cleaner than this, though it does have the advantage that it would
work even on an empty table.

Maybe we should extend the connection.h API to have a function for this, similar
to DoesColumnExist?

[vishwath, 2012/10/01 20:39:12]

Done.

+      "SELECT metahandle, server_position_in_parent "
+      "FROM metas WHERE unique_server_tag = 'google_chrome'"));
+  ASSERT_TRUE(s.Step());
+  ASSERT_EQ(sql::COLUMN_TYPE_INTEGER, s.ColumnType(1));
+
+  scoped_ptr<TestDirectoryBackingStore> dbs(
+      new TestDirectoryBackingStore(GetUsername(), &connection));
+  ASSERT_TRUE(dbs->MigrateVersion80To81());
+
+  // First test that a normal ordinal value is preserved correctly.
+  sql::Statement new_s(connection.GetUniqueStatement(
+      "SELECT metahandle, server_position_in_parent "
+      "FROM metas WHERE unique_server_tag = 'google_chrome'"));
+  ASSERT_TRUE(new_s.Step());
+  ASSERT_EQ(sql::COLUMN_TYPE_BLOB, new_s.ColumnType(1));
+
+  std::string expected_ordinal = Int64ToNodeOrdinal(1048576).ToInternalValue();
+  std::string actual_ordinal;
+  new_s.ColumnBlobAsString(1, &actual_ordinal);
+  ASSERT_EQ(expected_ordinal, actual_ordinal);
+
+  // Next, test that null characters in the

[rlarocque, 2012/09/29 01:13:42]

I think you should split this out into a separate test, if possible.

In fact, I think it would work better if it were to write it at a higher level. 
syncable_unittest.cc has a helper function called SimulateSaveAndReloadDir(). 
If we could contrive an int64 value known to produce NULL values in its ordinal
representation, and to reload differently if they are misinterpreted, then we
could test that save and restore works using the actual SQL statements used by
the DirectoryBackingStore in practice.

[vishwath, 2012/10/01 20:39:12]

Done.

+  // server_position_in_parent_field are handled correctly
+  expected_ordinal = "This\0should\0read\0completely";
+  sql::Statement s2(connection.GetUniqueStatement(
+      "UPDATE metas SET server_position_in_parent = ?"
+      "WHERE metahandle = ?"));
+  s2.BindBlob(0, expected_ordinal.data(), expected_ordinal.length());
+  s2.BindInt64(1, new_s.ColumnInt64(0));
+  ASSERT_TRUE(s2.Run());
+
+  new_s.Reset(true);
+  ASSERT_TRUE(new_s.Step());
+  new_s.ColumnBlobAsString(1, &actual_ordinal);
+  ASSERT_EQ(expected_ordinal, actual_ordinal);
+}
+
 TEST_P(MigrationTest, ToCurrentVersion) {
   sql::Connection connection;
   ASSERT_TRUE(connection.OpenInMemory());
   switch (GetParam()) {
     case 67:
       SetUpVersion67Database(&connection);
       break;
     case 68:
       SetUpVersion68Database(&connection);
       break;
@@ skipping 23 matching lines @@
       break;
     case 77:
       SetUpVersion77Database(&connection);
       break;
     case 78:
       SetUpVersion78Database(&connection);
       break;
     case 79:
       SetUpVersion79Database(&connection);
       break;
+    case 80:
+      SetUpVersion80Database(&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
       // for the database migration code.  You need to need to supply a
       // SetUpVersionXXDatabase function with a dump of the test database
       // at the old schema.  Here's one way to do that:
       //   1. Start on a clean tree (with none of your pending schema changes).
       //   2. Set a breakpoint in this function and run the unit test.
       //   3. Allow this test to run to completion (step out of the call),
       //      without allowing ~MigrationTest to execute.
@@ skipping 317 matching lines @@
   const std::string& guid2 = TestDirectoryBackingStore::GenerateCacheGUID();
   EXPECT_EQ(24U, guid1.size());
   EXPECT_EQ(24U, guid2.size());
   // In theory this test can fail, but it won't before the universe
   // dies of heat death.
   EXPECT_NE(guid1, guid2);
 }
 
 }  // namespace syncable
 }  // namespace syncer