diff options
| -rw-r--r-- | src/common/ownsql.cpp | 25 | ||||
| -rw-r--r-- | src/common/ownsql.h | 32 | ||||
| -rw-r--r-- | src/common/syncjournaldb.cpp | 56 | ||||
| -rw-r--r-- | test/testownsql.cpp | 15 |
4 files changed, 87 insertions, 41 deletions
diff --git a/src/common/ownsql.cpp b/src/common/ownsql.cpp index bd180cbd1..1b3c8c643 100644 --- a/src/common/ownsql.cpp +++ b/src/common/ownsql.cpp @@ -236,9 +236,6 @@ SqlQuery::~SqlQuery() if (_stmt) { finish(); } - if (_sqldb) { - _sqldb->_queries.remove(this); - } } SqlQuery::SqlQuery(const QByteArray &sql, SqlDatabase &db) @@ -270,17 +267,22 @@ int SqlQuery::prepare(const QByteArray &sql, bool allow_failure) _error = QString::fromUtf8(sqlite3_errmsg(_db)); qCWarning(lcSql) << "Sqlite prepare statement error:" << _error << "in" << _sql; ENFORCE(allow_failure, "SQLITE Prepare error"); + } else { + ASSERT(_stmt); + _sqldb->_queries.insert(this); } - _sqldb->_queries.insert(this); } return _errId; } -// There is no overloads to QByteArray::startWith that takes Qt::CaseInsensitive -template <int N> -static bool startsWithInsensitive(const QByteArray &a, const char (&cal)[N]) +/** + * There is no overloads to QByteArray::startWith that takes Qt::CaseInsensitive. + * Returns true if 'a' starts with 'b' in a case insensitive way + */ +static bool startsWithInsensitive(const QByteArray &a, const char *b) { - return a.size() >= N - 1 && qstrnicmp(a.constData(), cal, N - 1) == 0; + int len = strlen(b); + return a.size() >= len && qstrnicmp(a.constData(), b, len) == 0; } bool SqlQuery::isSelect() @@ -457,8 +459,13 @@ int SqlQuery::numRowsAffected() void SqlQuery::finish() { + if (!_stmt) + return; SQLITE_DO(sqlite3_finalize(_stmt)); _stmt = 0; + if (_sqldb) { + _sqldb->_queries.remove(this); + } } void SqlQuery::reset_and_clear_bindings() @@ -469,7 +476,7 @@ void SqlQuery::reset_and_clear_bindings() } } -bool SqlQuery::init(const QByteArray &sql, OCC::SqlDatabase &db) +bool SqlQuery::initOrReset(const QByteArray &sql, OCC::SqlDatabase &db) { ENFORCE(!_sqldb || &db == _sqldb); _sqldb = &db; diff --git a/src/common/ownsql.h b/src/common/ownsql.h index 45c9aed15..250fa109b 100644 --- a/src/common/ownsql.h +++ b/src/common/ownsql.h @@ -73,6 +73,26 @@ private: /** * @brief The SqlQuery class * @ingroup libsync + * + * There is basically 3 ways to initialize and use a query: + * + SqlQuery q1; + [...] + q1.initOrReset(...); + q1.bindValue(...); + q1.exec(...) + * + SqlQuery q2(db); + q2.prepare(...); + [...] + q2.reset_and_clear_bindings(); + q2.bindValue(...); + q2.exec(...) + * + SqlQuery q3("...", db); + q3.bindValue(...); + q3.exec(...) + * */ class OCSYNC_EXPORT SqlQuery { @@ -82,11 +102,17 @@ public: explicit SqlQuery(SqlDatabase &db); explicit SqlQuery(const QByteArray &sql, SqlDatabase &db); /** - * Prepare the SQLQuery if it was not prepared yet. + * Prepare the SqlQuery if it was not prepared yet. * Otherwise, clear the results and the bindings. * return false if there is an error */ - bool init(const QByteArray &sql, SqlDatabase &db); + bool initOrReset(const QByteArray &sql, SqlDatabase &db); + /** + * Prepare the SqlQuery. + * If the query was already prepared, this will first call finish(), and re-prepare it. + * This function must only be used if the constructor was setting a SqlDatabase + */ + int prepare(const QByteArray &sql, bool allow_failure = false); ~SqlQuery(); QString error() const; @@ -99,11 +125,9 @@ public: int intValue(int index); quint64 int64Value(int index); QByteArray baValue(int index); - bool isSelect(); bool isPragma(); bool exec(); - int prepare(const QByteArray &sql, bool allow_failure = false); bool next(); void bindValue(int pos, const QVariant &value); QString lastQuery() const; diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index a4f21d4be..41c4762fa 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -559,11 +559,11 @@ bool SyncJournalDb::checkConnect() forceRemoteDiscoveryNextSyncLocked(); } - if (!_deleteDownloadInfoQuery.init("DELETE FROM downloadinfo WHERE path=?1", _db)) { + if (!_deleteDownloadInfoQuery.initOrReset("DELETE FROM downloadinfo WHERE path=?1", _db)) { return sqlFail("prepare _deleteDownloadInfoQuery", _deleteDownloadInfoQuery); } - if (!_deleteUploadInfoQuery.init("DELETE FROM uploadinfo WHERE path=?1", _db)) { + if (!_deleteUploadInfoQuery.initOrReset("DELETE FROM uploadinfo WHERE path=?1", _db)) { return sqlFail("prepare _deleteUploadInfoQuery", _deleteUploadInfoQuery); } @@ -574,7 +574,7 @@ bool SyncJournalDb::checkConnect() // case insensitively sql += " COLLATE NOCASE"; } - if (!_getErrorBlacklistQuery.init(sql, _db)) { + if (!_getErrorBlacklistQuery.initOrReset(sql, _db)) { return sqlFail("prepare _getErrorBlacklistQuery", _getErrorBlacklistQuery); } @@ -856,7 +856,7 @@ bool SyncJournalDb::setFileRecord(const SyncJournalFileRecord &_record) parseChecksumHeader(record._checksumHeader, &checksumType, &checksum); int contentChecksumTypeId = mapChecksumType(checksumType); - if (!_setFileRecordQuery.init(QByteArrayLiteral( + if (!_setFileRecordQuery.initOrReset(QByteArrayLiteral( "INSERT OR REPLACE INTO metadata " "(phash, pathlen, path, inode, uid, gid, mode, modtime, type, md5, fileid, remotePerm, filesize, ignoredChildrenRemote, contentChecksum, contentChecksumTypeId) " "VALUES (?1 , ?2, ?3 , ?4 , ?5 , ?6 , ?7, ?8 , ?9 , ?10, ?11, ?12, ?13, ?14, ?15, ?16);"), _db)) { @@ -903,7 +903,7 @@ bool SyncJournalDb::deleteFileRecord(const QString &filename, bool recursively) // if (!recursively) { // always delete the actual file. - if (!_deleteFileRecordPhash.init(QByteArrayLiteral("DELETE FROM metadata WHERE phash=?1"), _db)) + if (!_deleteFileRecordPhash.initOrReset(QByteArrayLiteral("DELETE FROM metadata WHERE phash=?1"), _db)) return false; qlonglong phash = getPHash(filename.toUtf8()); @@ -913,7 +913,7 @@ bool SyncJournalDb::deleteFileRecord(const QString &filename, bool recursively) return false; if (recursively) { - if (!_deleteFileRecordRecursively.init(QByteArrayLiteral("DELETE FROM metadata WHERE " IS_PREFIX_PATH_OF("?1", "path")), _db)) + if (!_deleteFileRecordRecursively.initOrReset(QByteArrayLiteral("DELETE FROM metadata WHERE " IS_PREFIX_PATH_OF("?1", "path")), _db)) return false; _deleteFileRecordRecursively.bindValue(1, filename); if (!_deleteFileRecordRecursively.exec()) { @@ -944,7 +944,7 @@ bool SyncJournalDb::getFileRecord(const QByteArray &filename, SyncJournalFileRec return false; if (!filename.isEmpty()) { - if (!_getFileRecordQuery.init(QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE phash=?1"), _db)) + if (!_getFileRecordQuery.initOrReset(QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE phash=?1"), _db)) return false; _getFileRecordQuery.bindValue(1, getPHash(filename)); @@ -1021,7 +1021,7 @@ bool SyncJournalDb::getFileRecordByInode(quint64 inode, SyncJournalFileRecord *r if (!checkConnect()) return false; - if (!_getFileRecordQueryByInode.init(QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE inode=?1"), _db)) + if (!_getFileRecordQueryByInode.initOrReset(QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE inode=?1"), _db)) return false; _getFileRecordQueryByInode.bindValue(1, inode); @@ -1045,7 +1045,7 @@ bool SyncJournalDb::getFileRecordsByFileId(const QByteArray &fileId, const std:: if (!checkConnect()) return false; - if (!_getFileRecordQueryByFileId.init(QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE fileid=?1"), _db)) + if (!_getFileRecordQueryByFileId.initOrReset(QByteArrayLiteral(GET_FILE_RECORD_QUERY " WHERE fileid=?1"), _db)) return false; _getFileRecordQueryByFileId.bindValue(1, fileId); @@ -1080,13 +1080,13 @@ bool SyncJournalDb::getFilesBelowPath(const QByteArray &path, const std::functio // and find nothing. So, unfortunately, we have to use a different query for // retrieving the whole tree. - if (!_getAllFilesQuery.init(QByteArrayLiteral( GET_FILE_RECORD_QUERY " ORDER BY path||'/' ASC"), _db)) + if (!_getAllFilesQuery.initOrReset(QByteArrayLiteral( GET_FILE_RECORD_QUERY " ORDER BY path||'/' ASC"), _db)) return false; query = &_getAllFilesQuery; } else { // This query is used to skip discovery and fill the tree from the // database instead - if (!_getFilesBelowPathQuery.init(QByteArrayLiteral( + if (!_getFilesBelowPathQuery.initOrReset(QByteArrayLiteral( GET_FILE_RECORD_QUERY " WHERE " IS_PREFIX_PATH_OF("?1", "path") // We want to ensure that the contents of a directory are sorted @@ -1199,7 +1199,7 @@ bool SyncJournalDb::updateFileRecordChecksum(const QString &filename, int checksumTypeId = mapChecksumType(contentChecksumType); - if (!_setFileRecordChecksumQuery.init(QByteArrayLiteral( + if (!_setFileRecordChecksumQuery.initOrReset(QByteArrayLiteral( "UPDATE metadata" " SET contentChecksum = ?2, contentChecksumTypeId = ?3" " WHERE phash == ?1;"), _db)) { @@ -1226,7 +1226,7 @@ bool SyncJournalDb::updateLocalMetadata(const QString &filename, } - if (!_setFileRecordLocalMetadataQuery.init(QByteArrayLiteral( + if (!_setFileRecordLocalMetadataQuery.initOrReset(QByteArrayLiteral( "UPDATE metadata" " SET inode=?2, modtime=?3, filesize=?4" " WHERE phash == ?1;"), _db)) { @@ -1299,7 +1299,7 @@ SyncJournalDb::DownloadInfo SyncJournalDb::getDownloadInfo(const QString &file) if (checkConnect()) { - if (!_getDownloadInfoQuery.init(QByteArrayLiteral( + if (!_getDownloadInfoQuery.initOrReset(QByteArrayLiteral( "SELECT tmpfile, etag, errorcount FROM downloadinfo WHERE path=?1"), _db)) { return res; } @@ -1329,7 +1329,7 @@ void SyncJournalDb::setDownloadInfo(const QString &file, const SyncJournalDb::Do if (i._valid) { - if (!_setDownloadInfoQuery.init(QByteArrayLiteral( + if (!_setDownloadInfoQuery.initOrReset(QByteArrayLiteral( "INSERT OR REPLACE INTO downloadinfo " "(path, tmpfile, etag, errorcount) " "VALUES ( ?1 , ?2, ?3, ?4 )"), _db)) { @@ -1408,7 +1408,7 @@ SyncJournalDb::UploadInfo SyncJournalDb::getUploadInfo(const QString &file) UploadInfo res; if (checkConnect()) { - if (!_getUploadInfoQuery.init(QByteArrayLiteral( + if (!_getUploadInfoQuery.initOrReset(QByteArrayLiteral( "SELECT chunk, transferid, errorcount, size, modtime, contentChecksum FROM " "uploadinfo WHERE path=?1"), _db)) { return res; @@ -1442,7 +1442,7 @@ void SyncJournalDb::setUploadInfo(const QString &file, const SyncJournalDb::Uplo } if (i._valid) { - if (!_setUploadInfoQuery.init(QByteArrayLiteral( + if (!_setUploadInfoQuery.initOrReset(QByteArrayLiteral( "INSERT OR REPLACE INTO uploadinfo " "(path, chunk, transferid, errorcount, size, modtime, contentChecksum) " "VALUES ( ?1 , ?2, ?3 , ?4 , ?5, ?6 , ?7 )"), _db)) { @@ -1641,7 +1641,7 @@ void SyncJournalDb::setErrorBlacklistEntry(const SyncJournalErrorBlacklistRecord return; } - if (!_setErrorBlacklistQuery.init(QByteArrayLiteral( + if (!_setErrorBlacklistQuery.initOrReset(QByteArrayLiteral( "INSERT OR REPLACE INTO blacklist " "(path, lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget, errorCategory) " "VALUES ( ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)"), _db)) { @@ -1719,7 +1719,7 @@ QStringList SyncJournalDb::getSelectiveSyncList(SyncJournalDb::SelectiveSyncList return result; } - if (!_getSelectiveSyncListQuery.init(QByteArrayLiteral("SELECT path FROM selectivesync WHERE type=?1"), _db)) { + if (!_getSelectiveSyncListQuery.initOrReset(QByteArrayLiteral("SELECT path FROM selectivesync WHERE type=?1"), _db)) { *ok = false; return result; } @@ -1843,7 +1843,7 @@ QByteArray SyncJournalDb::getChecksumType(int checksumTypeId) // Retrieve the id auto &query = _getChecksumTypeQuery; - if (!query.init(QByteArrayLiteral("SELECT name FROM checksumtype WHERE id=?1"), _db)) + if (!query.initOrReset(QByteArrayLiteral("SELECT name FROM checksumtype WHERE id=?1"), _db)) return {}; query.bindValue(1, checksumTypeId); if (!query.exec()) { @@ -1864,7 +1864,7 @@ int SyncJournalDb::mapChecksumType(const QByteArray &checksumType) } // Ensure the checksum type is in the db - if (!_insertChecksumTypeQuery.init(QByteArrayLiteral("INSERT OR IGNORE INTO checksumtype (name) VALUES (?1)"), _db)) + if (!_insertChecksumTypeQuery.initOrReset(QByteArrayLiteral("INSERT OR IGNORE INTO checksumtype (name) VALUES (?1)"), _db)) return 0; _insertChecksumTypeQuery.bindValue(1, checksumType); if (!_insertChecksumTypeQuery.exec()) { @@ -1872,7 +1872,7 @@ int SyncJournalDb::mapChecksumType(const QByteArray &checksumType) } // Retrieve the id - if (!_getChecksumTypeIdQuery.init(QByteArrayLiteral("SELECT id FROM checksumtype WHERE name=?1"), _db)) + if (!_getChecksumTypeIdQuery.initOrReset(QByteArrayLiteral("SELECT id FROM checksumtype WHERE name=?1"), _db)) return 0; _getChecksumTypeIdQuery.bindValue(1, checksumType); if (!_getChecksumTypeIdQuery.exec()) { @@ -1893,7 +1893,7 @@ QByteArray SyncJournalDb::dataFingerprint() return QByteArray(); } - if (!_getDataFingerprintQuery.init(QByteArrayLiteral("SELECT fingerprint FROM datafingerprint"), _db)) + if (!_getDataFingerprintQuery.initOrReset(QByteArrayLiteral("SELECT fingerprint FROM datafingerprint"), _db)) return QByteArray(); if (!_getDataFingerprintQuery.exec()) { @@ -1913,8 +1913,8 @@ void SyncJournalDb::setDataFingerprint(const QByteArray &dataFingerprint) return; } - if (!_setDataFingerprintQuery1.init(QByteArrayLiteral("DELETE FROM datafingerprint;"), _db) - || !_setDataFingerprintQuery2.init(QByteArrayLiteral("INSERT INTO datafingerprint (fingerprint) VALUES (?1);"), _db)) { + if (!_setDataFingerprintQuery1.initOrReset(QByteArrayLiteral("DELETE FROM datafingerprint;"), _db) + || !_setDataFingerprintQuery2.initOrReset(QByteArrayLiteral("INSERT INTO datafingerprint (fingerprint) VALUES (?1);"), _db)) { return; } @@ -1931,7 +1931,7 @@ void SyncJournalDb::setConflictRecord(const ConflictRecord &record) return; auto &query = _setConflictRecordQuery; - ASSERT(query.init(QByteArrayLiteral( + ASSERT(query.initOrReset(QByteArrayLiteral( "INSERT OR REPLACE INTO conflicts " "(path, baseFileId, baseModtime, baseEtag) " "VALUES (?1, ?2, ?3, ?4);"), @@ -1951,7 +1951,7 @@ ConflictRecord SyncJournalDb::conflictRecord(const QByteArray &path) if (!checkConnect()) return entry; auto &query = _getConflictRecordQuery; - ASSERT(query.init(QByteArrayLiteral("SELECT baseFileId, baseModtime, baseEtag FROM conflicts WHERE path=?1;"), _db)); + ASSERT(query.initOrReset(QByteArrayLiteral("SELECT baseFileId, baseModtime, baseEtag FROM conflicts WHERE path=?1;"), _db)); query.bindValue(1, path); ASSERT(query.exec()); if (!query.next()) @@ -1970,7 +1970,7 @@ void SyncJournalDb::deleteConflictRecord(const QByteArray &path) if (!checkConnect()) return; - ASSERT(_deleteConflictRecordQuery.init("DELETE FROM conflicts WHERE path=?1;", _db)); + ASSERT(_deleteConflictRecordQuery.initOrReset("DELETE FROM conflicts WHERE path=?1;", _db)); _deleteConflictRecordQuery.bindValue(1, path); ASSERT(_deleteConflictRecordQuery.exec()); } diff --git a/test/testownsql.cpp b/test/testownsql.cpp index 77541794a..86ec77d28 100644 --- a/test/testownsql.cpp +++ b/test/testownsql.cpp @@ -126,6 +126,21 @@ private slots: } } + void testDestructor() + { + // This test make sure that the destructor of SqlQuery works even if the SqlDatabase + // is destroyed before + QScopedPointer<SqlDatabase> db(new SqlDatabase()); + SqlQuery q1(_db); + SqlQuery q2(_db); + q2.prepare("SELECT * FROM addresses"); + SqlQuery q3("SELECT * FROM addresses", _db); + SqlQuery q4; + SqlQuery q5; + q5.initOrReset("SELECT * FROM addresses", _db); + db.reset(); + } + private: SqlDatabase _db; }; |