Upgrade prefetch db schema: 4 new fields

Added thumbnail_url, favicon_url, snippet, and attribution. These are not yet
set by any code. We anticipate wanting these fields for experiments on the net
error page, and for integration with Jardin.

Change-Id: I5cd7eb0f20a82e9d6af87f2f03252de80113acbc
Reviewed-on: https://chromium-review.googlesource.com/c/1185914
Commit-Queue: Dan H <harringtond@google.com>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603529}

components/offline_pages/core/prefetch/prefetch_item.h

@@ -61,6 +61,18 @@ struct PrefetchItem {
   // left empty if they are the same.
   GURL final_archived_url;
 
+  // The URL to the thumbnail image representing the article.
+  GURL thumbnail_url;
+
+  // The URL to the favicon image of the article's hosting web site.
+  GURL favicon_url;
+
+  // A snippet of the article's contents.
+  std::string snippet;
+
+  // The publisher name/web site the article is attributed to.
+  std::string attribution;
+
   // Number of attempts to request OPS to generate an archive for this item.
   int generate_bundle_attempts = 0;
 

components/offline_pages/core/prefetch/prefetch_item_unittest.cc

@@ -97,6 +97,18 @@ TEST_F(PrefetchItemTest, OperatorEqualsCopyConstructorAndToString) {
   item1.final_archived_url = GURL("http://test.com/final");
   CheckFieldAndResetItem(item1, "final_archived_url");
 
+  item1.thumbnail_url = GURL("http://thumbnail");
+  CheckFieldAndResetItem(item1, "thumbnail_url");
+
+  item1.favicon_url = GURL("http://favicon");
+  CheckFieldAndResetItem(item1, "favicon_url");
+
+  item1.snippet = "snippet";
+  CheckFieldAndResetItem(item1, "snippet");
+
+  item1.attribution = "attribution";
+  CheckFieldAndResetItem(item1, "attribution");
+
   item1.generate_bundle_attempts = 10;
   CheckFieldAndResetItem(item1, "generate_bundle_attempts");
 

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

@@ -15,6 +15,7 @@ namespace offline_pages {
 //   tables.
 // * 2: Changes prefetch_items.file_size to have a default value of -1 (instead
 //   of 0).
+// * 3: Add thumbnail_url, favicon_url, snippet, and attribution.
 
 // static
 constexpr int PrefetchStoreSchema::kCurrentVersion;
@@ -58,13 +59,12 @@ int GetCompatibleVersionNumber(sql::MetaTable* meta_table) {
 // 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).
+//   (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.
+// IMPORTANT #2: Commonly used columns should appear first, as SQLite can stop
+//  reading the row early if later columns are not being read.
 static const char kItemsTableCreationSql[] =
     // Fixed length columns come first.
     R"sql(
@@ -87,7 +87,11 @@ 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 ''
+file_path VARCHAR NOT NULL DEFAULT '',
+thumbnail_url VARCHAR NOT NULL DEFAULT '',
+favicon_url VARCHAR NOT NULL DEFAULT '',
+snippet VARCHAR NOT NULL DEFAULT '',
+attribution VARCHAR NOT NULL DEFAULT ''
 )
 )sql";
 
@@ -121,6 +125,9 @@ bool CreateLatestSchema(sql::Database* db) {
 }
 
 int MigrateFromVersion1To2(sql::Database* db, sql::MetaTable* meta_table) {
+  // Version 2 simply changes the default value of file_size from 0 to -1.
+  // Because SQLite doesn't support removing or modifying columns, we create
+  // a new table and insert data from the old table.
   const int target_version = 2;
   const int target_compatible_version = 1;
   // 1. Rename the existing items table.
@@ -181,39 +188,71 @@ DROP TABLE prefetch_items_old;
   return kVersionError;
 }
 
-}  // namespace
-
-// static
-bool PrefetchStoreSchema::CreateOrUpgradeIfNeeded(sql::Database* db) {
-  DCHECK_GE(kCurrentVersion, kCompatibleVersion);
-  DCHECK(db);
-  if (!db)
-    return false;
+int MigrateFromVersion2To3(sql::Database* db, sql::MetaTable* meta_table) {
+  const int target_version = 3;
+  const int target_compatible_version = 1;
+  static const char k2To3Sql[] = R"sql(
+ALTER TABLE prefetch_items ADD COLUMN thumbnail_url VARCHAR NOT NULL DEFAULT '';
+ALTER TABLE prefetch_items ADD COLUMN favicon_url VARCHAR NOT NULL DEFAULT '';
+ALTER TABLE prefetch_items ADD COLUMN snippet VARCHAR NOT NULL DEFAULT '';
+ALTER TABLE prefetch_items ADD COLUMN attribution VARCHAR NOT NULL DEFAULT '';
+)sql";
 
-  sql::MetaTable meta_table;
-  if (!meta_table.Init(db, kCurrentVersion, kCompatibleVersion))
-    return false;
+  sql::Transaction transaction(db);
+  if (transaction.Begin() && db->Execute(k2To3Sql) &&
+      SetVersionNumber(meta_table, target_version) &&
+      SetCompatibleVersionNumber(meta_table, target_compatible_version) &&
+      transaction.Commit())
+    return target_version;
 
-  const int compatible_version = GetCompatibleVersionNumber(&meta_table);
-  int current_version = GetVersionNumber(&meta_table);
-  if (current_version == kVersionError || compatible_version == kVersionError)
-    return false;
-  DCHECK_GE(current_version, compatible_version);
+  return kVersionError;
+}
 
-  // 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)
+// Returns true if the database has previously been initialized to a compatible
+// version.
+bool DatabaseIsValid(sql::Database* db,
+                     sql::MetaTable* meta_table,
+                     int* current_version,
+                     int* compatible_version) {
+  if (!sql::MetaTable::DoesTableExist(db) ||
+      !meta_table->Init(db, PrefetchStoreSchema::kCurrentVersion,
+                        PrefetchStoreSchema::kCompatibleVersion))
     return false;
+  *compatible_version = GetCompatibleVersionNumber(meta_table);
+  *current_version = GetVersionNumber(meta_table);
+  return  // Sanity checks.
+      *current_version != kVersionError &&
+      *compatible_version != kVersionError && *current_version >= 1 &&
+      *compatible_version >= 1 &&
+      // This can be false when Chrome is downgraded after a db change that's
+      // not backwards-compatible.
+      *compatible_version <= PrefetchStoreSchema::kCurrentVersion;
+}
 
-  // 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);
+}  // namespace
 
-  // Versions 0 and below are unexpected.
-  if (current_version <= 0)
-    return false;
+// static
+bool PrefetchStoreSchema::CreateOrUpgradeIfNeeded(sql::Database* db) {
+  // TODO(harringtond): Add UMA to track errors and important actions here.
+  DCHECK(db);
+  sql::MetaTable meta_table;
+  int current_version, compatible_version;
+  if (!DatabaseIsValid(db, &meta_table, &current_version,
+                       &compatible_version)) {
+    // Raze the database to get back to a working state. Note that this is
+    // considered dangerous, and may lose data that could be recovered later.
+    // The benefit of this is that we won't be stuck in a bad state. For
+    // prefetch, loss of the data in the database is not terrible: prefetching
+    // is best effort, and the user will get more prefetch suggestions later.
+    if (!db->Raze())
+      return false;
+    sql::Transaction transaction(db);
+    sql::MetaTable new_meta;
+    return transaction.Begin() &&
+           new_meta.Init(db, PrefetchStoreSchema::kCurrentVersion,
+                         PrefetchStoreSchema::kCompatibleVersion) &&
+           CreateLatestSchema(db) && transaction.Commit();
+  }
 
   // Schema upgrade code starts here.
   //
@@ -224,9 +263,11 @@ bool PrefetchStoreSchema::CreateOrUpgradeIfNeeded(sql::Database* db) {
   // should not. For instance, one should never refer to kCurrentVersion or
   // kCompatibleVersion when setting values for the current and compatible
   // versions as these are definitely going to change with each schema change.
-  if (current_version == 1) {
+  if (current_version == 1)
     current_version = MigrateFromVersion1To2(db, &meta_table);
-  }
+
+  if (current_version == 2)
+    current_version = MigrateFromVersion2To3(db, &meta_table);
 
   return current_version == kCurrentVersion;
 }

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

@@ -17,8 +17,11 @@ namespace offline_pages {
 // from any and all previous database versions to the latest.
 class PrefetchStoreSchema {
  public:
-  static constexpr int kCurrentVersion = 2;
+  // See version_schemas for a history of schema versions.
+  static constexpr int kCurrentVersion = 3;
   static constexpr int kCompatibleVersion = 1;
+  static_assert(kCurrentVersion >= kCompatibleVersion,
+                "compatible version shouldn't be greater than the current one");
 
   // 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,

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

@@ -247,6 +247,18 @@ TEST(PrefetchStoreSchemaPreconditionTest,
   EXPECT_TRUE(db.DoesColumnExist("some_table", "value"));
 }
 
+TEST(PrefetchStoreSchemaTest, TestInvalidMetaTable) {
+  // Verify CreateOrUpgradeIfNeeded will raze the db if it can't be used.
+  sql::Database db;
+  ASSERT_TRUE(db.OpenInMemory());
+  sql::MetaTable meta;
+  meta.Init(&db, 100, 99);  // Some future version.
+  ASSERT_TRUE(db.Execute("CREATE TABLE prefetch_items (x INTEGER DEFAULT 1);"));
+  ASSERT_TRUE(PrefetchStoreSchema::CreateOrUpgradeIfNeeded(&db));
+
+  ExpectDbIsCurrent(&db);
+}
+
 // Verify the latest v#.sql accurately represents the current schema.
 //
 // Note: We keep the creation code for the current schema version duplicated in

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

@@ -45,12 +45,17 @@ const char* kSqlAllColumnNames =
     "archive_body_name, "
     "title, "
     "file_path, "
-    "file_size";
+    "file_size, "
+    "thumbnail_url, "
+    "favicon_url, "
+    "snippet, "
+    "attribution";
 
 bool InsertPrefetchItemSync(const PrefetchItem& item, sql::Database* db) {
   static const std::string kSql = base::StringPrintf(
       "INSERT INTO prefetch_items (%s)"
-      " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)",
+      " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,"
+      " ?, ?, ?)",
       kSqlAllColumnNames);
   sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql.c_str()));
   statement.BindInt64(0, item.offline_id);
@@ -72,6 +77,10 @@ bool InsertPrefetchItemSync(const PrefetchItem& item, sql::Database* db) {
   statement.BindString16(16, item.title);
   statement.BindString(17, store_utils::ToDatabaseFilePath(item.file_path));
   statement.BindInt64(18, item.file_size);
+  statement.BindString(19, item.thumbnail_url.spec());
+  statement.BindString(20, item.favicon_url.spec());
+  statement.BindString(21, item.snippet);
+  statement.BindString(22, item.attribution);
 
   return statement.Run();
 }
@@ -89,7 +98,7 @@ int CountPrefetchItemsSync(sql::Database* db) {
 // Populates the PrefetchItem with the data from the current row of the passed
 // in statement following the natural column ordering.
 void PopulatePrefetchItem(const sql::Statement& statement, PrefetchItem* item) {
-  DCHECK_EQ(19, statement.ColumnCount());
+  DCHECK_EQ(23, statement.ColumnCount());
   DCHECK(item);
 
   // Fields are assigned to the item in the order they are stored in the SQL
@@ -115,6 +124,10 @@ void PopulatePrefetchItem(const sql::Statement& statement, PrefetchItem* item) {
   item->file_path =
       store_utils::FromDatabaseFilePath(statement.ColumnString(17));
   item->file_size = statement.ColumnInt64(18);
+  item->thumbnail_url = GURL(statement.ColumnString(19));
+  item->favicon_url = GURL(statement.ColumnString(20));
+  item->snippet = statement.ColumnString(21);
+  item->attribution = statement.ColumnString(22);
 }
 
 std::unique_ptr<PrefetchItem> GetPrefetchItemSync(int64_t offline_id,

components/offline_pages/core/prefetch/test_util.cc

@@ -16,6 +16,9 @@ std::string PrefetchItem::ToString() const {
     << client_id << ", state=" << state
     << ", url=" << url.possibly_invalid_spec()
     << ", final_url=" << final_archived_url.possibly_invalid_spec()
+    << ", thumbnail_url=" << thumbnail_url.possibly_invalid_spec()
+    << ", favicon_url=" << favicon_url.possibly_invalid_spec()
+    << ", snippet=" << snippet << ", attribution=" << attribution
     << ", gb_attempts=" << generate_bundle_attempts
     << ", get_attempts=" << get_operation_attempts
     << ", dl_attempts=" << download_initiation_attempts
@@ -43,6 +46,9 @@ bool PrefetchItem::operator==(const PrefetchItem& other) const {
   return offline_id == other.offline_id && guid == other.guid &&
          client_id == other.client_id && state == other.state &&
          url == other.url && final_archived_url == other.final_archived_url &&
+         thumbnail_url == other.thumbnail_url &&
+         favicon_url == other.favicon_url && snippet == other.snippet &&
+         attribution == other.attribution &&
          generate_bundle_attempts == other.generate_bundle_attempts &&
          get_operation_attempts == other.get_operation_attempts &&
          download_initiation_attempts == other.download_initiation_attempts &&

components/test/data/offline_pages/prefetch/version_schemas/v1.data

@@ -24,3 +24,7 @@ operation_name: "operation_name"
 archive_body_name: "archive_body_name"
 title: "title"
 file_path: "file_path"
+thumbnail_url: ""
+favicon_url: ""
+snippet: ""
+attribution: ""

components/test/data/offline_pages/prefetch/version_schemas/v2.data

@@ -24,3 +24,7 @@ operation_name: "operation_name"
 archive_body_name: "archive_body_name"
 title: "title"
 file_path: "file_path"
+thumbnail_url: ""
+favicon_url: ""
+snippet: ""
+attribution: ""

components/test/data/offline_pages/prefetch/version_schemas/v3.data

+-- TABLE prefetch_downloader_quota --
+--- ROW 0 ---
+quota_id: "1"
+update_time: "2"
+available_quota: "3"
+-- TABLE prefetch_items --
+--- ROW 0 ---
+offline_id: "1"
+state: "2"
+generate_bundle_attempts: "3"
+get_operation_attempts: "4"
+download_initiation_attempts: "5"
+archive_body_length: "6"
+creation_time: "7"
+freshness_time: "8"
+error_code: "9"
+file_size: "10"
+guid: "guid"
+client_namespace: "client_namespace"
+client_id: "client_id"
+requested_url: "requested_url"
+final_archived_url: "final_archived_url"
+operation_name: "operation_name"
+archive_body_name: "archive_body_name"
+title: "title"
+file_path: "file_path"
+thumbnail_url: "thumbnail_url"
+favicon_url: "favicon_url"
+snippet: "snippet"
+attribution: "attribution"

components/test/data/offline_pages/prefetch/version_schemas/v3.sql

+INSERT OR REPLACE INTO meta (key, value)
+VALUES ("version", 3), ("last_compatible_version", 1);
+
+CREATE TABLE IF NOT EXISTS prefetch_items
+(
+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,
+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 '',
+thumbnail_url VARCHAR NOT NULL DEFAULT '',
+favicon_url VARCHAR NOT NULL DEFAULT '',
+snippet VARCHAR NOT NULL DEFAULT '',
+attribution VARCHAR NOT NULL DEFAULT ''
+);
+
+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
+);
+
+
+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,
+thumbnail_url,
+favicon_url,
+snippet,
+attribution
+)
+VALUES
+(
+1,  -- offline_id
+2,  -- state
+3,  -- generate_bundle_attempts
+4,  -- get_operation_attempts
+5,  -- download_initiation_attempts
+6,  -- archive_body_length
+7,  -- creation_time
+8,  -- freshness_time
+9,  -- error_code
+10,  -- file_size
+'guid',  -- guid
+'client_namespace',  -- client_namespace
+'client_id',  -- client_id
+'requested_url',  -- requested_url
+'final_archived_url',  -- final_archived_url
+'operation_name',  -- operation_name
+'archive_body_name',  -- archive_body_name
+'title',  -- title
+'file_path',  -- file_path
+'thumbnail_url',  -- thumbnail_url
+'favicon_url',  -- favicon_url
+'snippet',  -- snippet
+'attribution'  -- attribution
+);
+
+INSERT INTO prefetch_downloader_quota
+(
+quota_id,
+update_time,
+available_quota
+)
+VALUES
+(
+1,
+2,
+3
+);