From ba019b025681fc6f623ab07f62488b509f10bb4a Mon Sep 17 00:00:00 2001 From: "Thiago Marcos P. Santos" Date: Fri, 23 Feb 2018 11:16:41 +0100 Subject: [qt] Fix issue if resources not being found on the database Once again QVariant getting confused about its contents datatype. With this patch we use QString directly and copy the contents, which should be cheap with Qt implicity sharing. --- platform/qt/src/sqlite3.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'platform/qt/src/sqlite3.cpp') diff --git a/platform/qt/src/sqlite3.cpp b/platform/qt/src/sqlite3.cpp index 80efd6a326..eb4a798043 100644 --- a/platform/qt/src/sqlite3.cpp +++ b/platform/qt/src/sqlite3.cpp @@ -270,20 +270,15 @@ void Statement::bind(int offset, optional value) { } } -void Statement::bind(int offset, const char* value, std::size_t length, bool retain) { +void Statement::bind(int offset, const char* value, std::size_t length, bool /* retain */) { assert(impl); if (length > std::numeric_limits::max()) { // Kept for consistence with the default implementation. throw std::range_error("value too long"); } - // Qt SQLite driver treats QByteArray as blob: we need to explicitly - // declare the variant type as string. - QVariant text(QVariant::Type::String); - text.setValue(retain ? QByteArray(value, length) : QByteArray::fromRawData(value, length)); - // Field numbering starts at 0. - impl->query.bindValue(offset - 1, std::move(text), QSql::In); + impl->query.bindValue(offset - 1, QString(QByteArray(value, length)), QSql::In); checkQueryError(impl->query); } -- cgit v1.2.1 From 5ca38bbde93d273a2a4febb42ff5de53b90e1350 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Tue, 13 Feb 2018 14:52:49 +0100 Subject: [core] refactor SQLite error/status codes --- platform/qt/src/sqlite3.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'platform/qt/src/sqlite3.cpp') diff --git a/platform/qt/src/sqlite3.cpp b/platform/qt/src/sqlite3.cpp index eb4a798043..09a3a16002 100644 --- a/platform/qt/src/sqlite3.cpp +++ b/platform/qt/src/sqlite3.cpp @@ -24,11 +24,11 @@ namespace mapbox { namespace sqlite { // https://www.sqlite.org/rescode.html#ok -static_assert(mbgl::underlying_type(Exception::OK) == 0, "error"); +static_assert(mbgl::underlying_type(ResultCode::OK) == 0, "error"); // https://www.sqlite.org/rescode.html#cantopen -static_assert(mbgl::underlying_type(Exception::CANTOPEN) == 14, "error"); +static_assert(mbgl::underlying_type(ResultCode::CantOpen) == 14, "error"); // https://www.sqlite.org/rescode.html#notadb -static_assert(mbgl::underlying_type(Exception::NOTADB) == 26, "error"); +static_assert(mbgl::underlying_type(ResultCode::NotADB) == 26, "error"); void checkQueryError(const QSqlQuery& query) { QSqlError lastError = query.lastError(); @@ -57,7 +57,7 @@ void checkDatabaseOpenError(const QSqlDatabase &db) { // always returns -1 for `nativeErrorCode()` on database errors. QSqlError lastError = db.lastError(); if (lastError.type() != QSqlError::NoError) { - throw Exception { Exception::Code::CANTOPEN, "Error opening the database." }; + throw Exception { ResultCode::CantOpen, "Error opening the database." }; } } @@ -74,7 +74,7 @@ public: : connectionName(QString::number(uint64_t(QThread::currentThread())) + incrementCounter()) { if (!QSqlDatabase::drivers().contains("QSQLITE")) { - throw Exception { Exception::Code::CANTOPEN, "SQLite driver not found." }; + throw Exception { ResultCode::CantOpen, "SQLite driver not found." }; } assert(!QSqlDatabase::contains(connectionName)); -- cgit v1.2.1 From 136e536159a1e22aa4a92c4e6463893600b809d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Tue, 6 Feb 2018 17:55:50 +0100 Subject: [core, qt] move self-resetting Statement/Query object to shared header --- platform/qt/src/sqlite3.cpp | 198 +++++++++++++++++++++++--------------------- 1 file changed, 103 insertions(+), 95 deletions(-) (limited to 'platform/qt/src/sqlite3.cpp') diff --git a/platform/qt/src/sqlite3.cpp b/platform/qt/src/sqlite3.cpp index 09a3a16002..4bcaea0e31 100644 --- a/platform/qt/src/sqlite3.cpp +++ b/platform/qt/src/sqlite3.cpp @@ -186,74 +186,82 @@ void Database::exec(const std::string &sql) { } } -Statement Database::prepare(const char *query) { - return Statement(this, query); -} - -Statement::Statement(Database *db, const char *sql) - : impl(std::make_unique(QString(sql), QSqlDatabase::database(db->impl->connectionName))) { +Statement::Statement(Database& db, const char* sql) + : impl(std::make_unique(QString(sql), + QSqlDatabase::database(db.impl->connectionName))) { assert(impl); } -Statement::Statement(Statement &&other) - : impl(std::move(other.impl)) { - assert(impl); +Statement::~Statement() { +#ifndef NDEBUG + // Crash if we're destructing this object while we know a Query object references this. + assert(!used); +#endif } -Statement &Statement::operator=(Statement &&other) { - assert(impl); - std::swap(impl, other.impl); - return *this; +Query::Query(Statement& stmt_) : stmt(stmt_) { + assert(stmt.impl); + +#ifndef NDEBUG + assert(!stmt.used); + stmt.used = true; +#endif } -Statement::~Statement() { +Query::~Query() { + reset(); + clearBindings(); + +#ifndef NDEBUG + stmt.used = false; +#endif } -template void Statement::bind(int, int64_t); +template void Query::bind(int, int64_t); template -void Statement::bind(int offset, T value) { - assert(impl); +void Query::bind(int offset, T value) { + assert(stmt.impl); // Field numbering starts at 0. - impl->query.bindValue(offset - 1, QVariant::fromValue(value), QSql::In); - checkQueryError(impl->query); + stmt.impl->query.bindValue(offset - 1, QVariant::fromValue(value), QSql::In); + checkQueryError(stmt.impl->query); } template <> -void Statement::bind(int offset, std::nullptr_t) { - assert(impl); +void Query::bind(int offset, std::nullptr_t) { + assert(stmt.impl); // Field numbering starts at 0. - impl->query.bindValue(offset - 1, QVariant(QVariant::Invalid), QSql::In); - checkQueryError(impl->query); + stmt.impl->query.bindValue(offset - 1, QVariant(QVariant::Invalid), QSql::In); + checkQueryError(stmt.impl->query); } template <> -void Statement::bind(int offset, int32_t value) { +void Query::bind(int offset, int32_t value) { bind(offset, static_cast(value)); } template <> -void Statement::bind(int offset, bool value) { +void Query::bind(int offset, bool value) { bind(offset, static_cast(value)); } template <> -void Statement::bind(int offset, int8_t value) { +void Query::bind(int offset, int8_t value) { bind(offset, static_cast(value)); } template <> -void Statement::bind(int offset, uint8_t value) { +void Query::bind(int offset, uint8_t value) { bind(offset, static_cast(value)); } template <> -void Statement::bind(int offset, mbgl::Timestamp value) { +void Query::bind(int offset, mbgl::Timestamp value) { bind(offset, std::chrono::system_clock::to_time_t(value)); } template <> -void Statement::bind(int offset, optional value) { +void Query::bind(int offset, optional value) { if (value) { bind(offset, *value); } else { @@ -262,7 +270,7 @@ void Statement::bind(int offset, optional value) { } template <> -void Statement::bind(int offset, optional value) { +void Query::bind(int offset, optional value) { if (value) { bind(offset, *value); } else { @@ -270,25 +278,25 @@ void Statement::bind(int offset, optional value) { } } -void Statement::bind(int offset, const char* value, std::size_t length, bool /* retain */) { - assert(impl); +void Query::bind(int offset, const char* value, std::size_t length, bool /* retain */) { + assert(stmt.impl); if (length > std::numeric_limits::max()) { // Kept for consistence with the default implementation. throw std::range_error("value too long"); } // Field numbering starts at 0. - impl->query.bindValue(offset - 1, QString(QByteArray(value, length)), QSql::In); + stmt.impl->query.bindValue(offset - 1, QString(QByteArray(value, length)), QSql::In); - checkQueryError(impl->query); + checkQueryError(stmt.impl->query); } -void Statement::bind(int offset, const std::string& value, bool retain) { +void Query::bind(int offset, const std::string& value, bool retain) { bind(offset, value.data(), value.size(), retain); } -void Statement::bindBlob(int offset, const void* value_, std::size_t length, bool retain) { - assert(impl); +void Query::bindBlob(int offset, const void* value_, std::size_t length, bool retain) { + assert(stmt.impl); const char* value = reinterpret_cast(value_); if (length > std::numeric_limits::max()) { // Kept for consistence with the default implementation. @@ -296,123 +304,123 @@ void Statement::bindBlob(int offset, const void* value_, std::size_t length, boo } // Field numbering starts at 0. - impl->query.bindValue(offset - 1, retain ? QByteArray(value, length) : + stmt.impl->query.bindValue(offset - 1, retain ? QByteArray(value, length) : QByteArray::fromRawData(value, length), QSql::In | QSql::Binary); - checkQueryError(impl->query); + checkQueryError(stmt.impl->query); } -void Statement::bindBlob(int offset, const std::vector& value, bool retain) { +void Query::bindBlob(int offset, const std::vector& value, bool retain) { bindBlob(offset, value.data(), value.size(), retain); } -bool Statement::run() { - assert(impl); +bool Query::run() { + assert(stmt.impl); - if (!impl->query.isValid()) { - if (impl->query.exec()) { - impl->lastInsertRowId = impl->query.lastInsertId().value(); - impl->changes = impl->query.numRowsAffected(); + if (!stmt.impl->query.isValid()) { + if (stmt.impl->query.exec()) { + stmt.impl->lastInsertRowId = stmt.impl->query.lastInsertId().value(); + stmt.impl->changes = stmt.impl->query.numRowsAffected(); } else { - checkQueryError(impl->query); + checkQueryError(stmt.impl->query); } } - const bool hasNext = impl->query.next(); - if (!hasNext) impl->query.finish(); + const bool hasNext = stmt.impl->query.next(); + if (!hasNext) stmt.impl->query.finish(); return hasNext; } -template bool Statement::get(int); -template int Statement::get(int); -template int64_t Statement::get(int); -template double Statement::get(int); +template bool Query::get(int); +template int Query::get(int); +template int64_t Query::get(int); +template double Query::get(int); -template T Statement::get(int offset) { - assert(impl && impl->query.isValid()); - QVariant value = impl->query.value(offset); - checkQueryError(impl->query); +template T Query::get(int offset) { + assert(stmt.impl && stmt.impl->query.isValid()); + QVariant value = stmt.impl->query.value(offset); + checkQueryError(stmt.impl->query); return value.value(); } -template <> std::vector Statement::get(int offset) { - assert(impl && impl->query.isValid()); - QByteArray byteArray = impl->query.value(offset).toByteArray(); - checkQueryError(impl->query); +template <> std::vector Query::get(int offset) { + assert(stmt.impl && stmt.impl->query.isValid()); + QByteArray byteArray = stmt.impl->query.value(offset).toByteArray(); + checkQueryError(stmt.impl->query); std::vector blob(byteArray.begin(), byteArray.end()); return blob; } -template <> mbgl::Timestamp Statement::get(int offset) { - assert(impl && impl->query.isValid()); - QVariant value = impl->query.value(offset); - checkQueryError(impl->query); +template <> mbgl::Timestamp Query::get(int offset) { + assert(stmt.impl && stmt.impl->query.isValid()); + QVariant value = stmt.impl->query.value(offset); + checkQueryError(stmt.impl->query); return std::chrono::time_point_cast( std::chrono::system_clock::from_time_t(value.value<::time_t>())); } -template <> optional Statement::get(int offset) { - assert(impl && impl->query.isValid()); - QVariant value = impl->query.value(offset); - checkQueryError(impl->query); +template <> optional Query::get(int offset) { + assert(stmt.impl && stmt.impl->query.isValid()); + QVariant value = stmt.impl->query.value(offset); + checkQueryError(stmt.impl->query); if (value.isNull()) return {}; return { value.value() }; } -template <> optional Statement::get(int offset) { - assert(impl && impl->query.isValid()); - QVariant value = impl->query.value(offset); - checkQueryError(impl->query); +template <> optional Query::get(int offset) { + assert(stmt.impl && stmt.impl->query.isValid()); + QVariant value = stmt.impl->query.value(offset); + checkQueryError(stmt.impl->query); if (value.isNull()) return {}; return { value.value() }; } -template <> std::string Statement::get(int offset) { - assert(impl && impl->query.isValid()); - QByteArray value = impl->query.value(offset).toByteArray(); - checkQueryError(impl->query); +template <> std::string Query::get(int offset) { + assert(stmt.impl && stmt.impl->query.isValid()); + QByteArray value = stmt.impl->query.value(offset).toByteArray(); + checkQueryError(stmt.impl->query); return std::string(value.constData(), value.size()); } -template <> optional Statement::get(int offset) { - assert(impl && impl->query.isValid()); - QByteArray value = impl->query.value(offset).toByteArray(); - checkQueryError(impl->query); +template <> optional Query::get(int offset) { + assert(stmt.impl && stmt.impl->query.isValid()); + QByteArray value = stmt.impl->query.value(offset).toByteArray(); + checkQueryError(stmt.impl->query); if (value.isNull()) return {}; return { std::string(value.constData(), value.size()) }; } -template <> optional Statement::get(int offset) { - assert(impl && impl->query.isValid()); - QVariant value = impl->query.value(offset); - checkQueryError(impl->query); +template <> optional Query::get(int offset) { + assert(stmt.impl && stmt.impl->query.isValid()); + QVariant value = stmt.impl->query.value(offset); + checkQueryError(stmt.impl->query); if (value.isNull()) return {}; return { std::chrono::time_point_cast( std::chrono::system_clock::from_time_t(value.value<::time_t>())) }; } -void Statement::reset() { - assert(impl); - impl->query.finish(); +void Query::reset() { + assert(stmt.impl); + stmt.impl->query.finish(); } -void Statement::clearBindings() { +void Query::clearBindings() { // no-op } -int64_t Statement::lastInsertRowId() const { - assert(impl); - return impl->lastInsertRowId; +int64_t Query::lastInsertRowId() const { + assert(stmt.impl); + return stmt.impl->lastInsertRowId; } -uint64_t Statement::changes() const { - assert(impl); - return (impl->changes < 0 ? 0 : impl->changes); +uint64_t Query::changes() const { + assert(stmt.impl); + return (stmt.impl->changes < 0 ? 0 : stmt.impl->changes); } Transaction::Transaction(Database& db_, Mode mode) -- cgit v1.2.1 From a1195f6a9dc57910f7c3e6c9217e3041929a01fb Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 8 May 2018 10:40:35 -0700 Subject: Avoid exceptions for flow control during database creation Unfortuntely, it's difficult to avoid all exceptions, because sqlite3_open_v2 does not reliably return SQLITE_NOTADB if the file is not a database. However, this should avoid cases where developers misinterpret the SQLITE_CANTOPEN exception as a crash, which is the common case. --- platform/qt/src/sqlite3.cpp | 85 +++++++++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 38 deletions(-) (limited to 'platform/qt/src/sqlite3.cpp') diff --git a/platform/qt/src/sqlite3.cpp b/platform/qt/src/sqlite3.cpp index 4bcaea0e31..351991f881 100644 --- a/platform/qt/src/sqlite3.cpp +++ b/platform/qt/src/sqlite3.cpp @@ -52,15 +52,6 @@ void checkDatabaseError(const QSqlDatabase &db) { } } -void checkDatabaseOpenError(const QSqlDatabase &db) { - // Assume every error when opening the data as CANTOPEN. Qt - // always returns -1 for `nativeErrorCode()` on database errors. - QSqlError lastError = db.lastError(); - if (lastError.type() != QSqlError::NoError) { - throw Exception { ResultCode::CantOpen, "Error opening the database." }; - } -} - namespace { QString incrementCounter() { static QAtomicInt count = 0; @@ -70,32 +61,9 @@ namespace { class DatabaseImpl { public: - DatabaseImpl(const char* filename, int flags) - : connectionName(QString::number(uint64_t(QThread::currentThread())) + incrementCounter()) + DatabaseImpl(QString connectionName_) + : connectionName(std::move(connectionName_)) { - if (!QSqlDatabase::drivers().contains("QSQLITE")) { - throw Exception { ResultCode::CantOpen, "SQLite driver not found." }; - } - - assert(!QSqlDatabase::contains(connectionName)); - auto db = QSqlDatabase::addDatabase("QSQLITE", connectionName); - - QString connectOptions = db.connectOptions(); - if (flags & OpenFlag::ReadOnly) { - if (!connectOptions.isEmpty()) connectOptions.append(';'); - connectOptions.append("QSQLITE_OPEN_READONLY"); - } - if (flags & OpenFlag::SharedCache) { - if (!connectOptions.isEmpty()) connectOptions.append(';'); - connectOptions.append("QSQLITE_ENABLE_SHARED_CACHE"); - } - - db.setConnectOptions(connectOptions); - db.setDatabaseName(QString(filename)); - - if (!db.open()) { - checkDatabaseOpenError(db); - } } ~DatabaseImpl() { @@ -127,12 +95,51 @@ public: template using optional = std::experimental::optional; +mapbox::util::variant Database::tryOpen(const std::string &filename, int flags) { + if (!QSqlDatabase::drivers().contains("QSQLITE")) { + return Exception { ResultCode::CantOpen, "SQLite driver not found." }; + } -Database::Database(const std::string& file, int flags) - : impl(std::make_unique(file.c_str(), flags)) { - assert(impl); + QString connectionName = QString::number(uint64_t(QThread::currentThread())) + incrementCounter(); + + assert(!QSqlDatabase::contains(connectionName)); + auto db = QSqlDatabase::addDatabase("QSQLITE", connectionName); + + QString connectOptions = db.connectOptions(); + if (flags & OpenFlag::ReadOnly) { + if (!connectOptions.isEmpty()) connectOptions.append(';'); + connectOptions.append("QSQLITE_OPEN_READONLY"); + } + if (flags & OpenFlag::SharedCache) { + if (!connectOptions.isEmpty()) connectOptions.append(';'); + connectOptions.append("QSQLITE_ENABLE_SHARED_CACHE"); + } + + db.setConnectOptions(connectOptions); + db.setDatabaseName(QString(filename.c_str())); + + if (!db.open()) { + // Assume every error when opening the data as CANTOPEN. Qt + // always returns -1 for `nativeErrorCode()` on database errors. + return Exception { ResultCode::CantOpen, "Error opening the database." }; + } + + return Database(std::make_unique(connectionName)); +} + +Database Database::open(const std::string &filename, int flags) { + auto result = tryOpen(filename, flags); + if (result.is()) { + throw result.get(); + } else { + return std::move(result.get()); + } } +Database::Database(std::unique_ptr impl_) + : impl(std::move(impl_)) +{} + Database::Database(Database &&other) : impl(std::move(other.impl)) { assert(impl); @@ -165,7 +172,9 @@ void Database::setBusyTimeout(std::chrono::milliseconds timeout) { } db.setConnectOptions(connectOptions); if (!db.open()) { - checkDatabaseOpenError(db); + // Assume every error when opening the data as CANTOPEN. Qt + // always returns -1 for `nativeErrorCode()` on database errors. + throw Exception { ResultCode::CantOpen, "Error opening the database." }; } } -- cgit v1.2.1