[Offline Pages]: Add PrefetchStoreSchema for schema creation and upgrades.

Moves schema creation and migration code out of PrefetchStore and into
the newly created PrefetchStoreSchema. This is a step towards having a
generic storage architecture for all of Offline Pages features.

This also introduces the first migration code that will upgrade the
schema from version 1 to version 2. This will ensure that all users have
the latest version of the items table, for which the default |file_size|
column value is -1.

PrefetchStoreSchemaTest has unit tests for the schema management class
itself and for SQLite preconditions that ensure its logic works.

Bug: 701939
Change-Id: Ica8c2905bbb4f9bf76721c83e69d2b61044fa95d
Reviewed-on: https://chromium-review.googlesource.com/656402
Commit-Queue: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504157}

components/offline_pages/core/prefetch/BUILD.gn

@@ -74,6 +74,8 @@ static_library("prefetch") {
     "store/prefetch_downloader_quota.h",
     "store/prefetch_store.cc",
     "store/prefetch_store.h",
+    "store/prefetch_store_schema.cc",
+    "store/prefetch_store_schema.h",
     "store/prefetch_store_utils.cc",
     "store/prefetch_store_utils.h",
     "suggested_articles_observer.cc",
@@ -182,6 +184,7 @@ source_set("unit_tests") {
     "sent_get_operation_cleanup_task_unittest.cc",
     "stale_entry_finalizer_task_unittest.cc",
     "store/prefetch_downloader_quota_unittest.cc",
+    "store/prefetch_store_schema_unittest.cc",
     "store/prefetch_store_unittest.cc",
     "suggested_articles_observer_unittest.cc",
     "test_download_client.cc",

components/offline_pages/core/prefetch/prefetch_item_unittest.cc

@@ -9,7 +9,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "components/offline_pages/core/prefetch/prefetch_types.h"
-#include "components/offline_pages/core/prefetch/store/prefetch_store.h"
+#include "components/offline_pages/core/prefetch/store/prefetch_store_schema.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
@@ -61,7 +61,8 @@ void PrefetchItemTest::CheckAllFieldsWereTested() {
 
 // Computes the number of columns the SQL table has.
 std::size_t PrefetchItemTest::GetTableColumnsCount() {
-  std::string tableCreationSql(PrefetchStore::GetTableCreationSqlForTesting());
+  std::string tableCreationSql =
+      PrefetchStoreSchema::GetItemTableCreationSqlForTesting();
   std::vector<std::string> create_statement_split = base::SplitString(
       tableCreationSql, "()", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
   EXPECT_EQ(3U, create_statement_split.size());

components/offline_pages/core/prefetch/store/prefetch_store.cc

@@ -9,104 +9,22 @@
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/logging.h"
-#include "base/metrics/histogram_macros.h"
 #include "base/sequenced_task_runner.h"
 #include "base/task_runner_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/offline_pages/core/offline_store_types.h"
-#include "components/offline_pages/core/prefetch/store/prefetch_store_utils.h"
+#include "components/offline_pages/core/prefetch/store/prefetch_store_schema.h"
 #include "sql/connection.h"
-#include "sql/meta_table.h"
-#include "sql/statement.h"
-#include "sql/transaction.h"
 
 namespace offline_pages {
 namespace {
 
-// Name of the table with prefetch items.
-const char kPrefetchItemsTableName[] = "prefetch_items";
-const char kPrefetchQuotaTableName[] = "prefetch_downloader_quota";
 const char kPrefetchStoreFileName[] = "PrefetchStore.db";
 
-const int kCurrentVersion = 1;
-const int kCompatibleVersion = 1;
-
 using InitializeCallback =
     base::Callback<void(InitializationStatus,
                         std::unique_ptr<sql::Connection>)>;
 
-// IMPORTANT #1: when making changes to these columns please also reflect them
-// into:
-// - PrefetchItem: update existing fields and all method implementations
-//   (operator=, operator<<, ToString, etc).
-// - PrefetchItemTest, PrefetchStoreTestUtil: update test related code to cover
-//   the changed set of columns and PrefetchItem members.
-// - MockPrefetchItemGenerator: so that its generated items consider all fields.
-// IMPORTANT #2: the ordering of column types is important in SQLite 3 tables to
-// simplify data retrieval. Columns with fixed length types must come first and
-// variable length types must come later.
-static const char kTableCreationSql[] =
-    "CREATE TABLE prefetch_items"
-    // Fixed length columns come first.
-    "(offline_id INTEGER PRIMARY KEY NOT NULL,"
-    " state INTEGER NOT NULL DEFAULT 0,"
-    " generate_bundle_attempts INTEGER NOT NULL DEFAULT 0,"
-    " get_operation_attempts INTEGER NOT NULL DEFAULT 0,"
-    " download_initiation_attempts INTEGER NOT NULL DEFAULT 0,"
-    " archive_body_length INTEGER_NOT_NULL DEFAULT -1,"
-    " creation_time INTEGER NOT NULL,"
-    " freshness_time INTEGER NOT NULL,"
-    " error_code INTEGER NOT NULL DEFAULT 0,"
-    " file_size INTEGER NOT NULL DEFAULT -1,"
-    // Variable length columns come later.
-    " guid VARCHAR NOT NULL DEFAULT '',"
-    " client_namespace VARCHAR NOT NULL DEFAULT '',"
-    " client_id VARCHAR NOT NULL DEFAULT '',"
-    " requested_url VARCHAR NOT NULL DEFAULT '',"
-    " final_archived_url VARCHAR NOT NULL DEFAULT '',"
-    " operation_name VARCHAR NOT NULL DEFAULT '',"
-    " archive_body_name VARCHAR NOT NULL DEFAULT '',"
-    " title VARCHAR NOT NULL DEFAULT '',"
-    " file_path VARCHAR NOT NULL DEFAULT ''"
-    ")";
-
-bool CreatePrefetchItemsTable(sql::Connection* db) {
-  return db->Execute(kTableCreationSql);
-}
-
-bool CreatePrefetchQuotaTable(sql::Connection* db) {
-  static const char kSql[] =
-      "CREATE TABLE prefetch_downloader_quota"
-      "(quota_id INTEGER PRIMARY KEY NOT NULL DEFAULT 1,"
-      " update_time INTEGER NOT NULL,"
-      " available_quota INTEGER NOT NULL DEFAULT 0)";
-  return db->Execute(kSql);
-}
-
-bool CreateSchema(sql::Connection* db) {
-  // If you create a transaction but don't Commit() it is automatically
-  // rolled back by its destructor when it falls out of scope.
-  sql::Transaction transaction(db);
-  if (!transaction.Begin())
-    return false;
-
-  if (!db->DoesTableExist(kPrefetchItemsTableName)) {
-    if (!CreatePrefetchItemsTable(db))
-      return false;
-  }
-
-  if (!db->DoesTableExist(kPrefetchQuotaTableName)) {
-    if (!CreatePrefetchQuotaTable(db))
-      return false;
-  }
-
-  sql::MetaTable meta_table;
-  meta_table.Init(db, kCurrentVersion, kCompatibleVersion);
-
-  // This would be a great place to add indices when we need them.
-  return transaction.Commit();
-}
-
 bool PrepareDirectory(const base::FilePath& path) {
   base::File::Error error = base::File::FILE_OK;
   if (!base::DirectoryExists(path.DirName())) {
@@ -147,7 +65,7 @@ bool InitializeSync(sql::Connection* db,
   }
   db->Preload();
 
-  return CreateSchema(db);
+  return PrefetchStoreSchema::CreateOrUpgradeIfNeeded(db);
 }
 
 }  // namespace
@@ -188,6 +106,7 @@ void PrefetchStore::Initialize(base::OnceClosure pending_command) {
 
 void PrefetchStore::OnInitializeDone(base::OnceClosure pending_command,
                                      bool success) {
+  // TODO(carlosk): Add initializing error reporting here.
   DCHECK_EQ(initialization_status_, InitializationStatus::INITIALIZING);
   initialization_status_ =
       success ? InitializationStatus::SUCCESS : InitializationStatus::FAILURE;
@@ -202,9 +121,4 @@ void PrefetchStore::OnInitializeDone(base::OnceClosure pending_command,
     initialization_status_ = InitializationStatus::NOT_INITIALIZED;
 }
 
-// static
-const char* PrefetchStore::GetTableCreationSqlForTesting() {
-  return kTableCreationSql;
-}
-
 }  // namespace offline_pages

components/offline_pages/core/prefetch/store/prefetch_store_schema.cc

+// Copyright 2017 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 "components/offline_pages/core/prefetch/store/prefetch_store_schema.h"
+
+#include "sql/connection.h"
+#include "sql/meta_table.h"
+#include "sql/transaction.h"
+
+namespace offline_pages {
+
+// Schema versions changelog:
+// * 1: Initial version with prefetch_items and prefetch_downloader_quota
+//   tables.
+// * 2: Changes prefetch_items.file_size to have a default value of -1 (instead
+//   of 0).
+
+// static
+const int PrefetchStoreSchema::kCurrentVersion = 2;
+// static
+const int PrefetchStoreSchema::kCompatibleVersion = 1;
+
+namespace {
+
+// TODO(https://crbug.com/765282): remove MetaTable internal values and helper
+// methods once its getters and setters for version information allow the caller
+// to be informed about internal errors.
+
+// From MetaTable internals.
+const char kVersionKey[] = "version";
+const char kCompatibleVersionKey[] = "last_compatible_version";
+
+const int kVersionError = -1;
+
+bool SetVersionNumber(sql::MetaTable* meta_table, int version) {
+  return meta_table->SetValue(kVersionKey, version);
+}
+
+bool SetCompatibleVersionNumber(sql::MetaTable* meta_table, int version) {
+  return meta_table->SetValue(kCompatibleVersionKey, version);
+}
+
+int GetVersionNumber(sql::MetaTable* meta_table) {
+  int version;
+  if (meta_table->GetValue(kVersionKey, &version))
+    return version;
+  return kVersionError;
+}
+
+int GetCompatibleVersionNumber(sql::MetaTable* meta_table) {
+  int version;
+  if (meta_table->GetValue(kCompatibleVersionKey, &version))
+    return version;
+  return kVersionError;
+}
+
+// IMPORTANT #1: when making changes to these columns please also reflect them
+// into:
+// - PrefetchItem: update existing fields and all method implementations
+//   (operator=, operator<<, ToString, etc).
+// - PrefetchItemTest, PrefetchStoreTestUtil: update test related code to cover
+//   the changed set of columns and PrefetchItem members.
+// - MockPrefetchItemGenerator: so that its generated items consider all fields.
+// IMPORTANT #2: the ordering of column types is important in SQLite 3 tables to
+// simplify data retrieval. Columns with fixed length types must come first and
+// variable length types must come later.
+static const char kItemsTableCreationSql[] =
+    "CREATE TABLE IF NOT EXISTS prefetch_items "
+    // Fixed length columns come first.
+    "(offline_id INTEGER PRIMARY KEY NOT NULL,"
+    " state INTEGER NOT NULL DEFAULT 0,"
+    " generate_bundle_attempts INTEGER NOT NULL DEFAULT 0,"
+    " get_operation_attempts INTEGER NOT NULL DEFAULT 0,"
+    " download_initiation_attempts INTEGER NOT NULL DEFAULT 0,"
+    " archive_body_length INTEGER_NOT_NULL DEFAULT -1,"
+    " creation_time INTEGER NOT NULL,"
+    " freshness_time INTEGER NOT NULL,"
+    " error_code INTEGER NOT NULL DEFAULT 0,"
+    " file_size INTEGER NOT NULL DEFAULT -1,"
+    // Variable length columns come later.
+    " guid VARCHAR NOT NULL DEFAULT '',"
+    " client_namespace VARCHAR NOT NULL DEFAULT '',"
+    " client_id VARCHAR NOT NULL DEFAULT '',"
+    " requested_url VARCHAR NOT NULL DEFAULT '',"
+    " final_archived_url VARCHAR NOT NULL DEFAULT '',"
+    " operation_name VARCHAR NOT NULL DEFAULT '',"
+    " archive_body_name VARCHAR NOT NULL DEFAULT '',"
+    " title VARCHAR NOT NULL DEFAULT '',"
+    " file_path VARCHAR NOT NULL DEFAULT ''"
+    ")";
+
+bool CreatePrefetchItemsTable(sql::Connection* db) {
+  return db->Execute(kItemsTableCreationSql);
+}
+
+static const char kQuotaTableCreationSql[] =
+    "CREATE TABLE IF NOT EXISTS prefetch_downloader_quota "
+    "(quota_id INTEGER PRIMARY KEY NOT NULL DEFAULT 1,"
+    " update_time INTEGER NOT NULL,"
+    " available_quota INTEGER NOT NULL DEFAULT 0)";
+
+bool CreatePrefetchQuotaTable(sql::Connection* db) {
+  return db->Execute(kQuotaTableCreationSql);
+}
+
+bool CreateLatestSchema(sql::Connection* db) {
+  sql::Transaction transaction(db);
+  if (!transaction.Begin())
+    return false;
+
+  if (!CreatePrefetchItemsTable(db) || !CreatePrefetchQuotaTable(db))
+    return false;
+
+  // This would be a great place to add indices when we need them.
+  return transaction.Commit();
+}
+
+}  // namespace
+
+// static
+bool PrefetchStoreSchema::CreateOrUpgradeIfNeeded(sql::Connection* db) {
+  DCHECK_GE(kCurrentVersion, kCompatibleVersion);
+  DCHECK(db);
+  if (!db)
+    return false;
+
+  sql::MetaTable meta_table;
+  if (!meta_table.Init(db, kCurrentVersion, kCompatibleVersion))
+    return false;
+
+  const int compatible_version = GetCompatibleVersionNumber(&meta_table);
+  const int current_version = GetVersionNumber(&meta_table);
+  if (current_version == kVersionError || compatible_version == kVersionError)
+    return false;
+  DCHECK_GE(current_version, compatible_version);
+
+  // Stored database version is newer and incompatible with the current running
+  // code (Chrome was downgraded). The DB will never work until Chrome is
+  // re-upgraded.
+  if (compatible_version > kCurrentVersion)
+    return false;
+
+  // Database is already at the latest version or has just been created. Create
+  // any missing tables and return.
+  if (current_version == kCurrentVersion)
+    return CreateLatestSchema(db);
+
+  // Versions 0 and below are unexpected.
+  if (current_version <= 0)
+    return false;
+
+  // Schema upgrade code starts here.
+  // Note: A series of if-else blocks was chosen to allow for more flexibility
+  // in the upgrade logic than a single switch-case block would.
+
+  if (current_version == 1) {
+    sql::Transaction transaction(db);
+    if (!transaction.Begin())
+      return false;
+
+    if (!db->Execute("ALTER TABLE prefetch_items RENAME TO prefetch_items_old"))
+      return false;
+
+    if (!CreatePrefetchItemsTable(db))
+      return false;
+
+    const char kMigrateItemsSql[] =
+        "INSERT INTO prefetch_items "
+        " (offline_id, state, generate_bundle_attempts, get_operation_attempts,"
+        "  download_initiation_attempts, archive_body_length, creation_time,"
+        "  freshness_time, error_code, file_size, guid, client_namespace,"
+        "  client_id, requested_url, final_archived_url, operation_name,"
+        "  archive_body_name, title, file_path)"
+        " SELECT "
+        "  offline_id, state, generate_bundle_attempts, get_operation_attempts,"
+        "  download_initiation_attempts, archive_body_length, creation_time,"
+        "  freshness_time, error_code, file_size, guid, client_namespace,"
+        "  client_id, requested_url, final_archived_url, operation_name,"
+        "  archive_body_name, title, file_path"
+        " FROM prefetch_items_old";
+    if (!db->Execute(kMigrateItemsSql))
+      return false;
+
+    if (!db->Execute("DROP TABLE prefetch_items_old"))
+      return false;
+
+    if (!SetVersionNumber(&meta_table, kCurrentVersion) ||
+        !SetCompatibleVersionNumber(&meta_table, kCompatibleVersion)) {
+      return false;
+    }
+
+    if (!transaction.Commit())
+      return false;
+  }
+
+  return true;
+}
+
+// static
+std::string PrefetchStoreSchema::GetItemTableCreationSqlForTesting() {
+  return kItemsTableCreationSql;
+}
+
+}  // namespace offline_pages

components/offline_pages/core/prefetch/store/prefetch_store_schema.h

+// Copyright 2017 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.
+
+#ifndef COMPONENTS_OFFLINE_PAGES_CORE_PREFETCH_STORE_PREFETCH_STORE_SCHEMA_H_
+#define COMPONENTS_OFFLINE_PAGES_CORE_PREFETCH_STORE_PREFETCH_STORE_SCHEMA_H_
+
+#include <string>
+
+namespace sql {
+class Connection;
+}
+
+namespace offline_pages {
+
+// Maintains the schema of the prefetch database, ensuring creation and upgrades
+// from any and all previous database versions to the latest.
+class PrefetchStoreSchema {
+ public:
+  static const int kCurrentVersion;
+  static const int kCompatibleVersion;
+
+  // Creates or upgrade the database schema as needed from information stored in
+  // a metadata table. Returns |true| if the database is ready to be used,
+  // |false| if creation or upgrades failed.
+  static bool CreateOrUpgradeIfNeeded(sql::Connection* db);
+
+  // Returns the current items table creation SQL command for test usage.
+  static std::string GetItemTableCreationSqlForTesting();
+};
+
+}  // namespace offline_pages
+
+#endif  // COMPONENTS_OFFLINE_PAGES_CORE_PREFETCH_STORE_PREFETCH_STORE_SCHEMA_H_