Skip to content

Commit

Permalink
Remove sql::ErrorDelegate.
Browse files Browse the repository at this point in the history 
API cleanup.  Replaced by ErrorCallback.

BUG=none

Review URL: https://chromiumcodereview.appspot.com/16788002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@206299 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
shess@chromium.org committed Jun 14, 2013
1 parent c090470 commit 526b466
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 100 deletions.
11 changes: 2 additions & 9 deletions sql/connection.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ bool StatementID::operator<(const StatementID& other) const {
return strcmp(str_, other.str_) < 0;
}

ErrorDelegate::~ErrorDelegate() {
}

Connection::StatementRef::StatementRef(Connection* connection,
sqlite3_stmt* stmt,
bool was_valid)
Expand Down Expand Up @@ -122,7 +119,8 @@ Connection::Connection()
transaction_nesting_(0),
needs_rollback_(false),
in_memory_(false),
poisoned_(false) {}
poisoned_(false) {
}

Connection::~Connection() {
Close();
Expand Down Expand Up @@ -762,11 +760,6 @@ int Connection::OnSqliteError(int err, sql::Statement *stmt) {
return err;
}

// TODO(shess): Remove |error_delegate_| once everything is
// converted to |error_callback_|.
if (error_delegate_.get())
return error_delegate_->OnError(err, this, stmt);

// The default handling is to assert on debug and to ignore on release.
DLOG(FATAL) << GetErrorMessage();
return err;
Expand Down
45 changes: 0 additions & 45 deletions sql/connection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,30 +78,6 @@ class StatementID {

class Connection;

// ErrorDelegate defines the interface to implement error handling and recovery
// for sqlite operations. This allows the rest of the classes to return true or
// false while the actual error code and causing statement are delivered using
// the OnError() callback.
// The tipical usage is to centralize the code designed to handle database
// corruption, low-level IO errors or locking violations.
class SQL_EXPORT ErrorDelegate {
public:
virtual ~ErrorDelegate();

// |error| is an sqlite result code as seen in sqlite3.h. |connection| is the
// db connection where the error happened and |stmt| is our best guess at the
// statement that triggered the error. Do not store these pointers.
//
// |stmt| MAY BE NULL if there is no statement causing the problem (i.e. on
// initialization).
//
// If the error condition has been fixed and the original statement succesfuly
// re-tried then returning SQLITE_OK is appropriate; otherwise it is
// recommended that you return the original |error| or the appropriate error
// code.
virtual int OnError(int error, Connection* connection, Statement* stmt) = 0;
};

class SQL_EXPORT Connection {
private:
class StatementRef; // Forward declaration, see real one below.
Expand Down Expand Up @@ -145,14 +121,6 @@ class SQL_EXPORT Connection {
//
// If no callback is set, the default action is to crash in debug
// mode or return failure in release mode.
//
// TODO(shess): ErrorDelegate allowed for returning a different
// error. Determine if this is necessary for the callback. In my
// experience, this is not well-tested and probably not safe, and
// current clients always return the same error passed.
// Additionally, most errors don't admit to a clean way to retry the
// failed operation, so converting an error to SQLITE_OK is probably
// not feasible.
typedef base::Callback<void(int, Statement*)> ErrorCallback;
void set_error_callback(const ErrorCallback& callback) {
error_callback_ = callback;
Expand All @@ -161,15 +129,6 @@ class SQL_EXPORT Connection {
error_callback_.Reset();
}

// Sets the object that will handle errors. Recomended that it should be set
// before calling Open(). If not set, the default is to ignore errors on
// release and assert on debug builds.
// Takes ownership of |delegate|.
// NOTE(shess): Deprecated, use set_error_callback().
void set_error_delegate(ErrorDelegate* delegate) {
error_delegate_.reset(delegate);
}

// Set this tag to enable additional connection-type histogramming
// for SQLite error codes and database version numbers.
void set_histogram_tag(const std::string& tag) {
Expand Down Expand Up @@ -529,10 +488,6 @@ class SQL_EXPORT Connection {

ErrorCallback error_callback_;

// This object handles errors resulting from all forms of executing sqlite
// commands or statements. It can be null which means default handling.
scoped_ptr<ErrorDelegate> error_delegate_;

// Tag for auxiliary histograms.
std::string histogram_tag_;

Expand Down
32 changes: 9 additions & 23 deletions sql/sqlite_features_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <string>

#include "base/bind.h"
#include "base/file_util.h"
#include "base/files/scoped_temp_dir.h"
#include "sql/connection.h"
Expand All @@ -15,28 +16,12 @@

namespace {

class StatementErrorHandler : public sql::ErrorDelegate {
public:
StatementErrorHandler(int* error, std::string* sql_text)
: error_(error),
sql_text_(sql_text) {}

virtual ~StatementErrorHandler() {}

virtual int OnError(int error, sql::Connection* connection,
sql::Statement* stmt) OVERRIDE {
*error_ = error;
const char* sql_txt = stmt ? stmt->GetSQLStatement() : NULL;
*sql_text_ = sql_txt ? sql_txt : "no statement available";
return error;
}

private:
int* error_;
std::string* sql_text_;

DISALLOW_COPY_AND_ASSIGN(StatementErrorHandler);
};
void CaptureErrorCallback(int* error_pointer, std::string* sql_text,
int error, sql::Statement* stmt) {
*error_pointer = error;
const char* text = stmt ? stmt->GetSQLStatement() : NULL;
*sql_text = text ? text : "no statement available";
}

class SQLiteFeaturesTest : public testing::Test {
public:
Expand All @@ -48,7 +33,8 @@ class SQLiteFeaturesTest : public testing::Test {

// The error delegate will set |error_| and |sql_text_| when any sqlite
// statement operation returns an error code.
db_.set_error_delegate(new StatementErrorHandler(&error_, &sql_text_));
db_.set_error_callback(base::Bind(&CaptureErrorCallback,
&error_, &sql_text_));
}

virtual void TearDown() {
Expand Down
32 changes: 9 additions & 23 deletions sql/statement_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <string>

#include "base/bind.h"
#include "base/file_util.h"
#include "base/files/scoped_temp_dir.h"
#include "sql/connection.h"
Expand All @@ -13,28 +14,12 @@

namespace {

class StatementErrorHandler : public sql::ErrorDelegate {
public:
StatementErrorHandler(int* error, std::string* sql_text)
: error_(error),
sql_text_(sql_text) {}

virtual ~StatementErrorHandler() {}

virtual int OnError(int error, sql::Connection* connection,
sql::Statement* stmt) OVERRIDE {
*error_ = error;
const char* sql_txt = stmt ? stmt->GetSQLStatement() : NULL;
*sql_text_ = sql_txt ? sql_txt : "no statement available";
return error;
}

private:
int* error_;
std::string* sql_text_;

DISALLOW_COPY_AND_ASSIGN(StatementErrorHandler);
};
void CaptureErrorCallback(int* error_pointer, std::string* sql_text,
int error, sql::Statement* stmt) {
*error_pointer = error;
const char* text = stmt ? stmt->GetSQLStatement() : NULL;
*sql_text = text ? text : "no statement available";
}

class SQLStatementTest : public testing::Test {
public:
Expand All @@ -45,7 +30,8 @@ class SQLStatementTest : public testing::Test {
ASSERT_TRUE(db_.Open(temp_dir_.path().AppendASCII("SQLStatementTest.db")));
// The error delegate will set |error_| and |sql_text_| when any sqlite
// statement operation returns an error code.
db_.set_error_delegate(new StatementErrorHandler(&error_, &sql_text_));
db_.set_error_callback(base::Bind(&CaptureErrorCallback,
&error_, &sql_text_));
}

virtual void TearDown() {
Expand Down

0 comments on commit 526b466

Please sign in to comment.