From f84f0c548067c410641b1783358826e6ba26118d Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Tue, 7 Jul 2015 17:00:31 -0400 Subject: [PATCH] Avoid using sqlite3_db_filename 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. --- utils/sqlite/c_gate.cpp | 6 ++++- utils/sqlite/c_gate_test.cpp | 8 +++---- utils/sqlite/database.cpp | 41 ++++++++++++++++---------------- utils/sqlite/database.hpp | 2 +- utils/sqlite/database_test.cpp | 11 ++++++++- utils/sqlite/exceptions.cpp | 2 +- utils/sqlite/exceptions_test.cpp | 14 ++++++----- 7 files changed, 50 insertions(+), 34 deletions(-) diff --git a/utils/sqlite/c_gate.cpp b/utils/sqlite/c_gate.cpp index 909f05b2..e89ac533 100644 --- a/utils/sqlite/c_gate.cpp +++ b/utils/sqlite/c_gate.cpp @@ -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. /// @@ -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); } diff --git a/utils/sqlite/c_gate_test.cpp b/utils/sqlite/c_gate_test.cpp index 8afcaf1b..edf46f76 100644 --- a/utils/sqlite/c_gate_test.cpp +++ b/utils/sqlite/c_gate_test.cpp @@ -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) { @@ -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); } diff --git a/utils/sqlite/database.cpp b/utils/sqlite/database.cpp index 32792ff6..41935c3b 100644 --- a/utils/sqlite/database.cpp +++ b/utils/sqlite/database.cpp @@ -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. @@ -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. @@ -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"); } @@ -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_)) { } @@ -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); } @@ -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); } @@ -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); } diff --git a/utils/sqlite/database.hpp b/utils/sqlite/database.hpp index 75bff024..84228b57 100644 --- a/utils/sqlite/database.hpp +++ b/utils/sqlite/database.hpp @@ -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: diff --git a/utils/sqlite/database_test.cpp b/utils/sqlite/database_test.cpp index bbf39fc4..70f057b9 100644 --- a/utils/sqlite/database_test.cpp +++ b/utils/sqlite/database_test.cpp @@ -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()); } @@ -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); diff --git a/utils/sqlite/exceptions.cpp b/utils/sqlite/exceptions.cpp index 222299b4..cc2d42ca 100644 --- a/utils/sqlite/exceptions.cpp +++ b/utils/sqlite/exceptions.cpp @@ -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"; } diff --git a/utils/sqlite/exceptions_test.cpp b/utils/sqlite/exceptions_test.cpp index 27d62c54..d9e81038 100644 --- a/utils/sqlite/exceptions_test.cpp +++ b/utils/sqlite/exceptions_test.cpp @@ -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()); } @@ -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()); } @@ -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()); } @@ -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()); } @@ -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()); }