Migrate old Shortcuts DB data to conform to new type values

chrome/browser/history/shortcuts_database.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 "chrome/browser/history/shortcuts_database.h"
 
 #include <string>
 #include <vector>
 
 #include "base/guid.h"
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
+#include "sql/meta_table.h"
 #include "sql/statement.h"
 #include "sql/transaction.h"
 
 namespace {
 
+// Current version number. We write databases at the "current" version number,
+// but any previous version that can read the "compatible" one can make do with
+// or database without *too* many bad effects.

[Peter Kasting, 2014/02/12 00:27:50]

Nit: or -> our

[Anuj, 2014/02/12 23:54:40]

Done.

+const int kCurrentVersionNumber = 1;
+const int kCompatibleVersionNumber = 1;
+
 void BindShortcutToStatement(
     const history::ShortcutsBackend::Shortcut& shortcut,
     sql::Statement* s) {
   DCHECK(base::IsValidGUID(shortcut.id));
   s->BindString(0, shortcut.id);
   s->BindString16(1, shortcut.text);
   s->BindString16(2, shortcut.match_core.fill_into_edit);
   s->BindString(3, shortcut.match_core.destination_url.spec());
   s->BindString16(4, shortcut.match_core.contents);
   s->BindString(5, AutocompleteMatch::ClassificationsToString(
@@ skipping 141 matching lines @@
             "keyword VARCHAR, last_access_time INTEGER, "
             "number_of_hits INTEGER)");
   }
 
   // The first version of the shortcuts table lacked the fill_into_edit,
   // transition, type, and keyword columns.
   if (!db_.DoesColumnExist("omni_box_shortcuts", "fill_into_edit")) {
     // Perform the upgrade in a transaction to ensure it doesn't happen
     // incompletely.
     sql::Transaction transaction(&db_);
     transaction.Begin();

[Peter Kasting, 2014/02/12 00:27:50]

Change this block to something like:

  if (!transaction.Begin() || !db_.Execute() ...)
    return false;

This way, we'll go ahead and create the meta table correctly for such databases.

This will also check the return value of Begin(), which we erroneously weren't
doing before.

[Anuj, 2014/02/12 23:54:40]

Done. I will let compiler deal with Mr. De Morgan.

     return
         db_.Execute("ALTER TABLE omni_box_shortcuts "
             "ADD COLUMN fill_into_edit VARCHAR") &&
         db_.Execute("UPDATE omni_box_shortcuts SET fill_into_edit = url") &&
         db_.Execute("ALTER TABLE omni_box_shortcuts "
             "ADD COLUMN transition INTEGER") &&
         db_.Execute(base::StringPrintf(
             "UPDATE omni_box_shortcuts SET transition = %d",
             static_cast<int>(content::PAGE_TRANSITION_TYPED)).c_str()) &&
         db_.Execute("ALTER TABLE omni_box_shortcuts ADD COLUMN type INTEGER") &&
         db_.Execute(base::StringPrintf(
             "UPDATE omni_box_shortcuts SET type = %d",
             static_cast<int>(AutocompleteMatchType::HISTORY_TITLE)).c_str()) &&
         db_.Execute("ALTER TABLE omni_box_shortcuts "
             "ADD COLUMN keyword VARCHAR") &&
         transaction.Commit();
   }
 
+  if (!sql::MetaTable::DoesTableExist(&db_)) {
+    meta_table_.Init(&db_, kCurrentVersionNumber, kCompatibleVersionNumber);
+    sql::Transaction transaction(&db_);
+    transaction.Begin();

[Peter Kasting, 2014/02/12 00:27:50]

Nit: Include this call in the return statement

You may want to also structure this like a "if (!...) return false;" that falls
through to the subsequent code (which is today just a "return true") in order to
be less likely to break if we add more migration steps in the future.

[Anuj, 2014/02/12 23:54:40]

Done.

+    return
+      // Migrate old SEARCH_OTHER_ENGINE values to the new type value.
+      db_.Execute(base::StringPrintf("UPDATE omni_box_shortcuts "
+          "SET type = 13 WHERE type = 9").c_str()) &&
+      // Migrate old EXTENSION_APP values to the new type value.
+      db_.Execute(base::StringPrintf("UPDATE omni_box_shortcuts "
+           "SET type = 14 WHERE type = 10").c_str()) &&
+      // Migrate old CONTACT values to the new type value.
+      db_.Execute(base::StringPrintf("UPDATE omni_box_shortcuts "
+           "SET type = 15 WHERE type = 11").c_str()) &&
+      // Migrate old BOOKMARK_TITLE values to the new type value.
+      db_.Execute(base::StringPrintf("UPDATE omni_box_shortcuts "
+           "SET type = 16 WHERE type = 12").c_str()) &&
+      transaction.Commit();
+  }
   return true;
 }
 
 }  // namespace history