Skip to content

Commit

Permalink
Avoid using sqlite3_db_filename
Browse files Browse the repository at this point in the history
Ubuntu 12.04 LTS ships with sqlite 3.7.9, which does not include the
sqlite3_db_filename function (which first appeared in 3.7.10).  As we
need to support this release for Travis CI builds, stop using this
non-existent function and instead handle the database filename on our
own.

This is unfortunate because there are a couple of shortcomings.  First,
we cannot determine the name of temporary databases and, second, we
cannot determine the name of databases reopened via the c_gate.  None
of these are used in final Kyua builds so this is not a big deal.
  • Loading branch information
jmmv committed Jul 7, 2015
1 parent a5b7ab2 commit f84f0c5
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 34 deletions.
6 changes: 5 additions & 1 deletion utils/sqlite/c_gate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@

#include "utils/sqlite/c_gate.hpp"

#include "utils/fs/path.hpp"
#include "utils/optional.ipp"
#include "utils/sqlite/database.hpp"

namespace sqlite = utils::sqlite;

using utils::none;


/// Creates a new gateway to an existing C++ SQLite database.
///
Expand Down Expand Up @@ -65,7 +69,7 @@ sqlite::database_c_gate::~database_c_gate(void)
sqlite::database
sqlite::database_c_gate::connect(::sqlite3* raw_database)
{
return database(static_cast< void* >(raw_database), false);
return database(none, static_cast< void* >(raw_database), false);
}


Expand Down
8 changes: 4 additions & 4 deletions utils/sqlite/c_gate_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ ATF_TEST_CASE_BODY(c_database)
ATF_TEST_CASE(database__db_filename);
ATF_TEST_CASE_HEAD(database__db_filename)
{
set_md_var("descr", "Ensure that the value of db_filename cached by the "
"sqlite::database class works after a connect() call");
set_md_var("descr", "The current implementation of db_filename() has no "
"means to access the filename of a database connected to a raw "
"sqlite3 object");
}
ATF_TEST_CASE_BODY(database__db_filename)
{
Expand All @@ -82,8 +83,7 @@ ATF_TEST_CASE_BODY(database__db_filename)
"test.db", &raw_db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, NULL));

sqlite::database database = sqlite::database_c_gate::connect(raw_db);
ATF_REQUIRE_EQ(utils::make_optional(fs::path("test.db").to_absolute()),
database.db_filename());
ATF_REQUIRE(!database.db_filename());
::sqlite3_close(raw_db);
}

Expand Down
41 changes: 21 additions & 20 deletions utils/sqlite/database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,7 @@ using utils::optional;

/// Internal implementation for sqlite::database.
struct utils::sqlite::database::impl : utils::noncopyable {
/// Path to the database as seen by sqlite3_db_filename after construction.
///
/// We need to hold a copy of this value ourselves because we want to be
/// able to identify the database connection even after close() has been
/// called, but sqlite3_db_filename returns NULL after the connection is
/// closed.
/// Path to the database as seen at construction time.
optional< fs::path > db_filename;

/// The SQLite 3 internal database.
Expand All @@ -70,18 +65,17 @@ struct utils::sqlite::database::impl : utils::noncopyable {

/// Constructor.
///
/// \param db_filename_ The path to the database as seen at construction
/// time, if any, or none for in-memory databases. We should use
/// sqlite3_db_filename instead, but this function appeared in 3.7.10
/// and Ubuntu 12.04 LTS (which we support for Travis CI builds as of
/// 2015-07-07) ships with 3.7.9.
/// \param db_ The SQLite internal database.
/// \param owned_ Whether this object owns the db_ object or not. If it
/// does, the internal db_ will be released during destruction.
impl(::sqlite3* db_, const bool owned_) :
db(db_),
owned(owned_)
impl(optional< fs::path > db_filename_, ::sqlite3* db_, const bool owned_) :
db_filename(db_filename_), db(db_), owned(owned_)
{
const char* raw_db_filename = ::sqlite3_db_filename(db, "main");
if (raw_db_filename == NULL || std::strlen(raw_db_filename) == 0)
db_filename = none;
else
db_filename = utils::make_optional(fs::path(raw_db_filename));
}

/// Destructor.
Expand Down Expand Up @@ -117,7 +111,8 @@ struct utils::sqlite::database::impl : utils::noncopyable {
if (db == NULL)
throw std::bad_alloc();
else {
sqlite::database error_db(db, true);
sqlite::database error_db(utils::make_optional(fs::path(file)),
db, true);
throw sqlite::api_error::from_database(error_db,
"sqlite3_open_v2");
}
Expand Down Expand Up @@ -147,10 +142,14 @@ struct utils::sqlite::database::impl : utils::noncopyable {
/// SQLite session. As soon as the object is destroyed, the session is
/// terminated.
///
/// \param db_filename_ The path to the database as seen at construction
/// time, if any, or none for in-memory databases.
/// \param db_ Raw pointer to the C SQLite 3 object.
/// \param owned_ Whether this instance will own the pointer or not.
sqlite::database::database(void* db_, const bool owned_) :
_pimpl(new impl(static_cast< ::sqlite3* >(db_), owned_))
sqlite::database::database(
const utils::optional< utils::fs::path >& db_filename_, void* db_,
const bool owned_) :
_pimpl(new impl(db_filename_, static_cast< ::sqlite3* >(db_), owned_))
{
}

Expand All @@ -174,7 +173,8 @@ sqlite::database::~database(void)
sqlite::database
sqlite::database::in_memory(void)
{
return database(impl::safe_open(":memory:", SQLITE_OPEN_READWRITE), true);
return database(none, impl::safe_open(":memory:", SQLITE_OPEN_READWRITE),
true);
}


Expand Down Expand Up @@ -210,7 +210,8 @@ sqlite::database::open(const fs::path& file, int open_flags)
}
PRE(open_flags == 0);

return database(impl::safe_open(file.c_str(), flags), true);
return database(utils::make_optional(file),
impl::safe_open(file.c_str(), flags), true);
}


Expand All @@ -223,7 +224,7 @@ sqlite::database::open(const fs::path& file, int open_flags)
sqlite::database
sqlite::database::temporary(void)
{
return database(impl::safe_open("", SQLITE_OPEN_READWRITE), true);
return database(none, impl::safe_open("", SQLITE_OPEN_READWRITE), true);
}


Expand Down
2 changes: 1 addition & 1 deletion utils/sqlite/database.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class database {
std::shared_ptr< impl > _pimpl;

friend class database_c_gate;
database(void*, const bool);
database(const utils::optional< utils::fs::path >&, void*, const bool);
void* raw_database(void);

public:
Expand Down
11 changes: 10 additions & 1 deletion utils/sqlite/database_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,15 @@ ATF_TEST_CASE_BODY(db_filename__file)
const sqlite::database db = sqlite::database::open(fs::path("test.db"),
sqlite::open_readwrite | sqlite::open_create);
ATF_REQUIRE(db.db_filename());
ATF_REQUIRE_EQ(fs::path("test.db").to_absolute(), db.db_filename().get());
ATF_REQUIRE_EQ(fs::path("test.db"), db.db_filename().get());
}


ATF_TEST_CASE_WITHOUT_HEAD(db_filename__temporary);
ATF_TEST_CASE_BODY(db_filename__temporary)
{
const sqlite::database db = sqlite::database::temporary();
ATF_REQUIRE(!db.db_filename());
}


Expand Down Expand Up @@ -264,6 +272,7 @@ ATF_INIT_TEST_CASES(tcs)

ATF_ADD_TEST_CASE(tcs, db_filename__in_memory);
ATF_ADD_TEST_CASE(tcs, db_filename__file);
ATF_ADD_TEST_CASE(tcs, db_filename__temporary);
ATF_ADD_TEST_CASE(tcs, db_filename__ok_after_close);

ATF_ADD_TEST_CASE(tcs, exec__ok);
Expand Down
2 changes: 1 addition & 1 deletion utils/sqlite/exceptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ format_db_filename(const optional< fs::path >& db_filename)
if (db_filename)
return db_filename.get().str();
else
return "in-memory";
return "in-memory or temporary";
}


Expand Down
14 changes: 8 additions & 6 deletions utils/sqlite/exceptions_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ ATF_TEST_CASE_BODY(error__no_filename)
{
const sqlite::database db = sqlite::database::in_memory();
const sqlite::error e(db.db_filename(), "Some text");
ATF_REQUIRE_EQ("Some text (sqlite db: in-memory)", std::string(e.what()));
ATF_REQUIRE_EQ("Some text (sqlite db: in-memory or temporary)",
std::string(e.what()));
ATF_REQUIRE_EQ(db.db_filename(), e.db_filename());
}

Expand All @@ -64,8 +65,7 @@ ATF_TEST_CASE_BODY(error__with_filename)
const sqlite::database db = sqlite::database::open(
fs::path("test.db"), sqlite::open_readwrite | sqlite::open_create);
const sqlite::error e(db.db_filename(), "Some text");
ATF_REQUIRE_MATCH("Some text \\(sqlite db: .*/test.db\\)",
std::string(e.what()));
ATF_REQUIRE_EQ("Some text (sqlite db: test.db)", std::string(e.what()));
ATF_REQUIRE_EQ(db.db_filename(), e.db_filename());
}

Expand All @@ -75,7 +75,8 @@ ATF_TEST_CASE_BODY(api_error__explicit)
{
const sqlite::api_error e(none, "some_function", "Some text");
ATF_REQUIRE_EQ(
"Some text (sqlite op: some_function) (sqlite db: in-memory)",
"Some text (sqlite op: some_function) "
"(sqlite db: in-memory or temporary)",
std::string(e.what()));
ATF_REQUIRE_EQ("some_function", e.api_function());
}
Expand All @@ -99,7 +100,7 @@ ATF_TEST_CASE_BODY(api_error__from_database)
const sqlite::api_error e = sqlite::api_error::from_database(
db, "real_function");
ATF_REQUIRE_MATCH(
".*ABCDE.*\\(sqlite op: real_function\\) \\(sqlite db: .*/test.db\\)",
".*ABCDE.*\\(sqlite op: real_function\\) \\(sqlite db: test.db\\)",
std::string(e.what()));
ATF_REQUIRE_EQ("real_function", e.api_function());
}
Expand All @@ -109,7 +110,8 @@ ATF_TEST_CASE_WITHOUT_HEAD(invalid_column_error);
ATF_TEST_CASE_BODY(invalid_column_error)
{
const sqlite::invalid_column_error e(none, "some_name");
ATF_REQUIRE_EQ("Unknown column 'some_name' (sqlite db: in-memory)",
ATF_REQUIRE_EQ("Unknown column 'some_name' "
"(sqlite db: in-memory or temporary)",
std::string(e.what()));
ATF_REQUIRE_EQ("some_name", e.column_name());
}
Expand Down

0 comments on commit f84f0c5

Please sign in to comment.