Add FailState to RequestQueueStore

Bug: 824931
Change-Id: I7ef1247593ee8a1a50672972076ddb0d14f26d74
Reviewed-on: https://chromium-review.googlesource.com/985555
Commit-Queue: Candice Sy <cmsy@google.com>
Reviewed-by: Cathy Li <chili@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547294}

components/offline_pages/core/background/request_queue_store_sql.cc

@@ -47,7 +47,8 @@ bool CreateRequestQueueTable(sql::Connection* db) {
                       " client_namespace VARCHAR NOT NULL,"
                       " client_id VARCHAR NOT NULL,"
                       " original_url VARCHAR NOT NULL DEFAULT '',"
-                      " request_origin VARCHAR NOT NULL DEFAULT ''"
+                      " request_origin VARCHAR NOT NULL DEFAULT '',"
+                      " fail_state INTEGER NOT NULL DEFAULT 0"
                       ")";
   return db->Execute(kSql);
 }
@@ -92,6 +93,20 @@ bool UpgradeFrom58(sql::Connection* db) {
   return UpgradeWithQuery(db, kSql);
 }
 
+bool UpgradeFrom61(sql::Connection* db) {
+  const char kSql[] =
+      "INSERT INTO " REQUEST_QUEUE_TABLE_NAME
+      " (request_id, creation_time, activation_time, last_attempt_time, "
+      "started_attempt_count, completed_attempt_count, state, url, "
+      "client_namespace, client_id, original_url, request_origin) "
+      "SELECT "
+      "request_id, creation_time, activation_time, last_attempt_time, "
+      "started_attempt_count, completed_attempt_count, state, url, "
+      "client_namespace, client_id, original_url, request_origin "
+      "FROM temp_" REQUEST_QUEUE_TABLE_NAME;
+  return UpgradeWithQuery(db, kSql);
+}
+
 bool CreateSchema(sql::Connection* db) {
   sql::Transaction transaction(db);
   if (!transaction.Begin())
@@ -116,6 +131,9 @@ bool CreateSchema(sql::Connection* db) {
   } else if (!db->DoesColumnExist(REQUEST_QUEUE_TABLE_NAME, "request_origin")) {
     if (!UpgradeFrom58(db))
       return false;
+  } else if (!db->DoesColumnExist(REQUEST_QUEUE_TABLE_NAME, "fail_state")) {
+    if (!UpgradeFrom61(db))
+      return false;
   }
 
   // This would be a great place to add indices when we need them.
@@ -141,6 +159,8 @@ std::unique_ptr<SavePageRequest> MakeSavePageRequest(
                            statement.ColumnString(9));
   const GURL original_url(statement.ColumnString(10));
   const std::string request_origin(statement.ColumnString(11));
+  const FailState fail_state =
+      static_cast<FailState>(statement.ColumnInt64(12));
 
   DVLOG(2) << "making save page request - id " << id << " url " << url
            << " client_id " << client_id.name_space << "-" << client_id.id
@@ -156,6 +176,7 @@ std::unique_ptr<SavePageRequest> MakeSavePageRequest(
   request->set_request_state(state);
   request->set_original_url(original_url);
   request->set_request_origin(request_origin);
+  request->set_fail_state(fail_state);
   return request;
 }
 
@@ -165,7 +186,8 @@ std::unique_ptr<SavePageRequest> GetOneRequest(sql::Connection* db,
   const char kSql[] =
       "SELECT request_id, creation_time, activation_time,"
       " last_attempt_time, started_attempt_count, completed_attempt_count,"
-      " state, url, client_namespace, client_id, original_url, request_origin"
+      " state, url, client_namespace, client_id, original_url, request_origin,"
+      " fail_state"
       " FROM " REQUEST_QUEUE_TABLE_NAME " WHERE request_id=?";
 
   sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));
@@ -193,10 +215,10 @@ ItemActionStatus Insert(sql::Connection* db, const SavePageRequest& request) {
       "INSERT OR IGNORE INTO " REQUEST_QUEUE_TABLE_NAME
       " (request_id, creation_time, activation_time,"
       " last_attempt_time, started_attempt_count, completed_attempt_count,"
-      " state, url, client_namespace, client_id, original_url, "
-      "request_origin)"
+      " state, url, client_namespace, client_id, original_url, request_origin,"
+      " fail_state)"
       " VALUES "
-      " (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
+      " (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
 
   sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));
   statement.BindInt64(0, request.request_id());
@@ -212,6 +234,7 @@ ItemActionStatus Insert(sql::Connection* db, const SavePageRequest& request) {
   statement.BindString(9, request.client_id().id);
   statement.BindString(10, request.original_url().spec());
   statement.BindString(11, request.request_origin());
+  statement.BindInt64(12, static_cast<int64_t>(request.fail_state()));
 
   if (!statement.Run())
     return ItemActionStatus::STORE_ERROR;
@@ -226,7 +249,7 @@ ItemActionStatus Update(sql::Connection* db, const SavePageRequest& request) {
       " SET creation_time = ?, activation_time = ?, last_attempt_time = ?,"
       " started_attempt_count = ?, completed_attempt_count = ?, state = ?,"
       " url = ?, client_namespace = ?, client_id = ?, original_url = ?,"
-      "request_origin = ?"
+      " request_origin = ?, fail_state = ?"
       " WHERE request_id = ?";
 
   sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));
@@ -242,7 +265,8 @@ ItemActionStatus Update(sql::Connection* db, const SavePageRequest& request) {
   statement.BindString(8, request.client_id().id);
   statement.BindString(9, request.original_url().spec());
   statement.BindString(10, request.request_origin());
-  statement.BindInt64(11, request.request_id());
+  statement.BindInt64(11, static_cast<int64_t>(request.fail_state()));
+  statement.BindInt64(12, request.request_id());
 
   if (!statement.Run())
     return ItemActionStatus::STORE_ERROR;
@@ -305,7 +329,8 @@ void GetRequestsSync(sql::Connection* db,
   const char kSql[] =
       "SELECT request_id, creation_time, activation_time,"
       " last_attempt_time, started_attempt_count, completed_attempt_count,"
-      " state, url, client_namespace, client_id, original_url, request_origin"
+      " state, url, client_namespace, client_id, original_url, request_origin,"
+      " fail_state"
       " FROM " REQUEST_QUEUE_TABLE_NAME;
 
   sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));

components/offline_pages/core/background/request_queue_store_sql.h

@@ -29,11 +29,13 @@ namespace offline_pages {
 // Original schema was delivered in M57. Since then the following changes
 // happened:
 // * In M58 original_url was added.
+// * In M61 request_origin was added.
+// * In M67 fail_state was added.
 //
 // TODO(romax): remove all activation_time related code the next we change the
 // schema.
 //
-// Looking for procesure to update the schema, please refer to
+// Looking for procedure to update the schema, please refer to
 // offline_page_metadata_store_sql.h
 class RequestQueueStoreSQL : public RequestQueueStore {
  public:

components/offline_pages/core/background/request_queue_store_unittest.cc

@@ -134,6 +134,55 @@ void BuildTestStoreWithSchemaFromM58(const base::FilePath& file) {
       connection.DoesColumnExist(REQUEST_QUEUE_TABLE_NAME, "request_origin"));
 }
 
+void BuildTestStoreWithSchemaFromM61(const base::FilePath& file) {
+  sql::Connection connection;
+  ASSERT_TRUE(
+      connection.Open(file.Append(FILE_PATH_LITERAL("RequestQueue.db"))));
+  ASSERT_TRUE(connection.is_open());
+  ASSERT_TRUE(connection.BeginTransaction());
+  ASSERT_TRUE(
+      connection.Execute("CREATE TABLE " REQUEST_QUEUE_TABLE_NAME
+                         " (request_id INTEGER PRIMARY KEY NOT NULL,"
+                         " creation_time INTEGER NOT NULL,"
+                         " activation_time INTEGER NOT NULL DEFAULT 0,"
+                         " last_attempt_time INTEGER NOT NULL DEFAULT 0,"
+                         " started_attempt_count INTEGER NOT NULL,"
+                         " completed_attempt_count INTEGER NOT NULL,"
+                         " state INTEGER NOT NULL DEFAULT 0,"
+                         " url VARCHAR NOT NULL,"
+                         " client_namespace VARCHAR NOT NULL,"
+                         " client_id VARCHAR NOT NULL,"
+                         " original_url VARCHAR NOT NULL DEFAULT '',"
+                         " request_origin VARCHAR NOT NULL DEFAULT ''"
+                         ")"));
+
+  ASSERT_TRUE(connection.CommitTransaction());
+  sql::Statement statement(connection.GetUniqueStatement(
+      "INSERT OR IGNORE INTO " REQUEST_QUEUE_TABLE_NAME
+      " (request_id, creation_time, activation_time,"
+      " last_attempt_time, started_attempt_count, completed_attempt_count,"
+      " state, url, client_namespace, client_id, original_url, request_origin)"
+      " VALUES "
+      " (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"));
+
+  statement.BindInt64(0, kRequestId);
+  statement.BindInt64(1, 0);
+  statement.BindInt64(2, 0);
+  statement.BindInt64(3, 0);
+  statement.BindInt64(4, 0);
+  statement.BindInt64(5, 0);
+  statement.BindInt64(6, 0);
+  statement.BindString(7, kUrl.spec());
+  statement.BindString(8, kClientId.name_space);
+  statement.BindString(9, kClientId.id);
+  statement.BindString(10, kUrl2.spec());
+  statement.BindString(11, kRequestOrigin);
+  ASSERT_TRUE(statement.Run());
+  ASSERT_TRUE(connection.DoesTableExist(REQUEST_QUEUE_TABLE_NAME));
+  ASSERT_FALSE(
+      connection.DoesColumnExist(REQUEST_QUEUE_TABLE_NAME, "fail_state"));
+}
+
 }  // namespace
 
 // Class that serves as a base for testing different implementations of the
@@ -285,6 +334,8 @@ class RequestQueueStoreSQLFactory : public RequestQueueStoreFactory {
       BuildTestStoreWithSchemaFromM57(path);
     } else if (version == 58) {
       BuildTestStoreWithSchemaFromM58(path);
+    } else if (version == 61) {
+      BuildTestStoreWithSchemaFromM61(path);
     }
 
     RequestQueueStore* store =
@@ -364,6 +415,25 @@ TYPED_TEST(RequestQueueStoreTest, UpgradeFromVersion58Store) {
   EXPECT_EQ("", this->last_requests()[0]->request_origin());
 }
 
+TYPED_TEST(RequestQueueStoreTest, UpgradeFromVersion61Store) {
+  std::unique_ptr<RequestQueueStore> store(this->BuildStoreWithOldSchema(61));
+  // In-memory store does not support upgrading.
+  if (!store)
+    return;
+  this->InitializeStore(store.get());
+
+  store->GetRequests(base::Bind(&RequestQueueStoreTestBase::GetRequestsDone,
+                                base::Unretained(this)));
+  this->PumpLoop();
+  ASSERT_EQ(LastResult::RESULT_TRUE, this->last_result());
+  ASSERT_EQ(1u, this->last_requests().size());
+  EXPECT_EQ(kRequestId, this->last_requests()[0]->request_id());
+  EXPECT_EQ(kUrl, this->last_requests()[0]->url());
+  EXPECT_EQ(kUrl2, this->last_requests()[0]->original_url());
+  EXPECT_EQ(kRequestOrigin, this->last_requests()[0]->request_origin());
+  EXPECT_EQ(0, static_cast<int>(this->last_requests()[0]->fail_state()));
+}
+
 TYPED_TEST(RequestQueueStoreTest, GetRequestsEmpty) {
   std::unique_ptr<RequestQueueStore> store(this->BuildStore());
   this->InitializeStore(store.get());