Refactoring AutocompleteActionPredictor Database.

chrome/browser/predictors/autocomplete_action_predictor_table.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/predictors/autocomplete_action_predictor_table.h"
+
+#include "base/logging.h"
+#include "base/metrics/histogram.h"
+#include "base/stringprintf.h"
+#include "base/utf_string_conversions.h"
+#include "chrome/common/guid.h"
+#include "content/public/browser/browser_thread.h"
+#include "sql/statement.h"
+
+namespace {
+
+// TODO(shishir): Rename the table for consistency.

[dominich, 2012/05/08 20:35:28]

Why not do this? we'll lose two weeks of data but that will only take two weeks
to build up again.

[Shishir, 2012/05/08 21:49:03]

We talked about this last time. The idea is that we will also migrate the data
and delete the old table. Which is why I am not doing this now.

+const char kAutocompletePredictorTableName[] = "network_action_predictor";
+
+// The maximum length allowed for strings in the database.
+const size_t kMaxDataLength = 2048;
+
+void BindRowToStatement(
+    const predictors::AutocompleteActionPredictorTable::Row& row,
+    sql::Statement* statement) {
+  DCHECK(guid::IsValidGUID(row.id));
+  statement->BindString(0, row.id);
+  statement->BindString16(1, row.user_text.substr(0, kMaxDataLength));
+  statement->BindString(2, row.url.spec().substr(0, kMaxDataLength));
+  statement->BindInt(3, row.number_of_hits);
+  statement->BindInt(4, row.number_of_misses);
+}
+
+bool StepAndInitializeRow(
+    sql::Statement* statement,
+    predictors::AutocompleteActionPredictorTable::Row* row) {
+  if (!statement->Step())
+    return false;
+
+  row->id = statement->ColumnString(0);
+  row->user_text = statement->ColumnString16(1);
+  row->url = GURL(statement->ColumnString(2));
+  row->number_of_hits = statement->ColumnInt(3);
+  row->number_of_misses = statement->ColumnInt(4);
+  return true;
+}
+
+}  // namespace
+
+namespace predictors {
+
+AutocompleteActionPredictorTable::Row::Row()
+    : number_of_hits(0),
+      number_of_misses(0) {
+}
+
+AutocompleteActionPredictorTable::Row::Row(const Row::Id& id,
+                                           const string16& user_text,
+                                           const GURL& url,
+                                           int number_of_hits,
+                                           int number_of_misses)
+    : id(id),
+      user_text(user_text),
+      url(url),
+      number_of_hits(number_of_hits),
+      number_of_misses(number_of_misses) {
+}
+
+AutocompleteActionPredictorTable::Row::Row(const Row& row)
+    : id(row.id),
+      user_text(row.user_text),
+      url(row.url),
+      number_of_hits(row.number_of_hits),
+      number_of_misses(row.number_of_misses) {
+}
+
+
+void AutocompleteActionPredictorTable::GetRow(const Row::Id& id, Row* row) {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::DB));
+  if (cancelled_.IsSet() || !DB())

[dominich, 2012/05/08 20:35:28]

where does cancelled_ get set?

[dominich, 2012/05/08 20:35:28]

can this check be combined into a single check? TableBase::IsCancelled() maybe?

[Shishir, 2012/05/08 21:49:03]

The predictor_database sets it when it is asked to shutdown.

[Shishir, 2012/05/08 21:49:03]

The semantics might be unclear. The DB is set to null when the database is
unusable (say the create table failed) or when it is not ready.

[dominich, 2012/05/08 22:49:39]

I understand. But throughout this class you have 

if (cancelled_.IsSet() || !DB())

I'm suggesting

if (IsCancelled())

which is defined in TableBase and is defined as 

{ return (cancelled_.IsSet() || !DB()); }

[Shishir, 2012/05/09 17:40:10]

Done.

+    return;
+
+  sql::Statement statement(DB()->GetCachedStatement(SQL_FROM_HERE,
+      base::StringPrintf("SELECT * FROM %s WHERE id=?",
+                         kAutocompletePredictorTableName).c_str()));
+  statement.BindString(0, id);
+
+  bool success = StepAndInitializeRow(&statement, row);
+  DCHECK(success) << "Failed to get row " << id << " from "
+                  << kAutocompletePredictorTableName;
+}
+
+void AutocompleteActionPredictorTable::GetAllRows(Rows* row_buffer) {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::DB));
+  if (cancelled_.IsSet() || !DB())
+    return;
+
+  row_buffer->clear();
+
+  sql::Statement statement(DB()->GetCachedStatement(SQL_FROM_HERE,
+      base::StringPrintf(
+          "SELECT * FROM %s", kAutocompletePredictorTableName).c_str()));
+
+  Row row;
+  while (StepAndInitializeRow(&statement, &row))
+    row_buffer->push_back(row);
+}
+
+void AutocompleteActionPredictorTable::AddRow(
+    const AutocompleteActionPredictorTable::Row& row) {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::DB));
+  if (cancelled_.IsSet() || !DB())
+    return;
+
+  AddAndUpdateRows(Rows(1, row), Rows());
+}
+
+void AutocompleteActionPredictorTable::UpdateRow(
+    const AutocompleteActionPredictorTable::Row& row) {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::DB));
+  if (cancelled_.IsSet() || !DB())
+    return;
+
+  AddAndUpdateRows(Rows(), Rows(1, row));
+}
+
+void AutocompleteActionPredictorTable::AddAndUpdateRows(
+    const Rows& rows_to_add,
+    const Rows& rows_to_update) {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::DB));
+  if (cancelled_.IsSet() || !DB())
+    return;
+
+  if (!DB()->BeginTransaction())
+    return;
+  for (Rows::const_iterator it = rows_to_add.begin();
+       it != rows_to_add.end(); ++it) {
+    sql::Statement statement(DB()->GetCachedStatement(SQL_FROM_HERE,
+        base::StringPrintf(
+            "INSERT INTO %s "
+            "(id, user_text, url, number_of_hits, number_of_misses) "
+            "VALUES (?,?,?,?,?)", kAutocompletePredictorTableName).c_str()));
+    BindRowToStatement(*it, &statement);
+    if (!statement.Run()) {
+      DB()->RollbackTransaction();

[dominich, 2012/05/08 20:35:28]

change of behaviour: in the old code, we never rolled back the transaction but
instead DCHECKed on every statement. Now we would fail silently.

Please put the DCHECK back.

[Shishir, 2012/05/08 21:49:03]

statement.Run() will DCHECK.

+      return;
+    }
+  }
+  for (Rows::const_iterator it = rows_to_update.begin();
+       it != rows_to_update.end(); ++it) {
+    sql::Statement statement(DB()->GetCachedStatement(SQL_FROM_HERE,
+        base::StringPrintf(
+            "UPDATE %s "
+            "SET id=?, user_text=?, url=?, number_of_hits=?, number_of_misses=?"
+            " WHERE id=?1", kAutocompletePredictorTableName).c_str()));
+    BindRowToStatement(*it, &statement);
+    if (!statement.Run()) {
+      DB()->RollbackTransaction();

[dominich, 2012/05/08 20:35:28]

same as above: DCHECK.

[Shishir, 2012/05/08 21:49:03]

statement.Run() will DCHECK.

+      return;
+    }
+    DCHECK_GT(DB()->GetLastChangeCount(), 0);
+  }
+  DB()->CommitTransaction();
+}
+
+void AutocompleteActionPredictorTable::DeleteRows(
+    const std::vector<Row::Id>& id_list) {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::DB));
+  if (cancelled_.IsSet() || !DB())
+    return;
+
+  sql::Statement statement(DB()->GetUniqueStatement(base::StringPrintf(
+      "DELETE FROM %s WHERE id=?",
+      kAutocompletePredictorTableName).c_str()));
+
+  if (!DB()->BeginTransaction())
+    return;
+  for (std::vector<Row::Id>::const_iterator it = id_list.begin();
+       it != id_list.end(); ++it) {
+    statement.BindString(0, *it);
+    if (!statement.Run()) {
+      DB()->RollbackTransaction();

[dominich, 2012/05/08 20:35:28]

ditto: DCHECK

[Shishir, 2012/05/08 21:49:03]

statement.Run() will DCHECK.

+      return;
+    }
+    statement.Reset(true);
+  }
+  DB()->CommitTransaction();
+}
+
+void AutocompleteActionPredictorTable::DeleteAllRows() {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::DB));
+  if (cancelled_.IsSet() || !DB())
+    return;
+
+  sql::Statement statement(DB()->GetCachedStatement(SQL_FROM_HERE,
+      base::StringPrintf("DELETE FROM %s",
+                         kAutocompletePredictorTableName).c_str()));
+
+  statement.Run();
+}
+
+AutocompleteActionPredictorTable::AutocompleteActionPredictorTable()
+    : PredictorTableBase() {
+}
+
+AutocompleteActionPredictorTable::~AutocompleteActionPredictorTable() {
+}
+
+void AutocompleteActionPredictorTable::CreateTableIfNonExistent() {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::DB));
+  if (cancelled_.IsSet() || !DB())
+    return;
+
+  if (DB()->DoesTableExist(kAutocompletePredictorTableName))
+    return;
+
+  bool success = DB()->Execute(base::StringPrintf(
+      "CREATE TABLE %s ( "
+      "id TEXT PRIMARY KEY, "
+      "user_text TEXT, "
+      "url TEXT, "
+      "number_of_hits INTEGER, "
+      "number_of_misses INTEGER)", kAutocompletePredictorTableName).c_str());
+  if (!success)
+    ResetDB();
+}
+
+void AutocompleteActionPredictorTable::LogDatabaseStats()  {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::DB));
+  if (cancelled_.IsSet() || !DB())
+    return;
+
+  sql::Statement count_statement(DB()->GetUniqueStatement(
+      base::StringPrintf("SELECT count(id) FROM %s",
+                         kAutocompletePredictorTableName).c_str()));
+  if (!count_statement.Step())
+    return;
+  UMA_HISTOGRAM_COUNTS("AutocompleteActionPredictor.DatabaseRowCount",
+                       count_statement.ColumnInt(0));
+}
+
+}  // namespace predictors