From a4b00a7742bd7f0ecee2d8dc567079440a1e88f0 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Tue, 12 May 2020 12:54:18 +0200 Subject: [PATCH] Sqlite: Add update hook and use it to get the last changed id Sqlite has a function to get the last inserted rowid but very often you want to get the updated rowid too. Change-Id: Ie276a5039682813ad16597433996a2959f54d9ba Reviewed-by: Tim Jenssen --- src/libs/sqlite/lastchangedrowid.h | 71 ++++++++++++ src/libs/sqlite/sqlite-lib.pri | 1 + src/libs/sqlite/sqlitedatabase.h | 7 ++ src/libs/sqlite/sqlitedatabasebackend.cpp | 41 +++++-- src/libs/sqlite/sqlitedatabasebackend.h | 7 +- src/libs/sqlite/sqlitedatabaseinterface.h | 12 ++ src/libs/sqlite/sqliteexception.h | 2 + src/libs/sqlite/sqliteglobal.h | 2 + tests/unit/unittest/lastchangedrowid-test.cpp | 108 ++++++++++++++++++ tests/unit/unittest/mocksqlitedatabase.h | 4 + tests/unit/unittest/sqlitedatabase-test.cpp | 87 ++++++++++++++ tests/unit/unittest/unittest.pro | 1 + 12 files changed, 330 insertions(+), 13 deletions(-) create mode 100644 src/libs/sqlite/lastchangedrowid.h create mode 100644 tests/unit/unittest/lastchangedrowid-test.cpp diff --git a/src/libs/sqlite/lastchangedrowid.h b/src/libs/sqlite/lastchangedrowid.h new file mode 100644 index 00000000000..f2a77b25ace --- /dev/null +++ b/src/libs/sqlite/lastchangedrowid.h @@ -0,0 +1,71 @@ +/**************************************************************************** +** +** Copyright (C) 2020 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of Qt Creator. +** +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-3.0.html. +** +****************************************************************************/ + +#pragma once + +#include "sqlitedatabaseinterface.h" + +#include + +#include + +namespace Sqlite { + +class LastChangedRowId +{ +public: + LastChangedRowId(DatabaseInterface &database, + Utils::SmallStringView databaseName, + Utils::SmallStringView tableName) + : database(database) + , databaseName(databaseName) + , tableName(tableName) + { + callback = [this](ChangeType, char const *database, char const *table, long long rowId) { + if (this->databaseName == database && this->tableName == table) + lastRowId = rowId; + }; + + database.setUpdateHook(callback); + } + + ~LastChangedRowId() { database.resetUpdateHook(); } + + long long takeLastRowId() + { + long long rowId = lastRowId; + lastRowId = -1; + return rowId; + } + +public: + DatabaseInterface &database; + DatabaseInterface::UpdateCallback callback; + Utils::SmallStringView databaseName; + Utils::SmallStringView tableName; + long long lastRowId = -1; +}; + +} // namespace Sqlite diff --git a/src/libs/sqlite/sqlite-lib.pri b/src/libs/sqlite/sqlite-lib.pri index 2fd286f3a8c..7efb3ac2cd0 100644 --- a/src/libs/sqlite/sqlite-lib.pri +++ b/src/libs/sqlite/sqlite-lib.pri @@ -25,6 +25,7 @@ SOURCES += \ $$PWD/sqlitebasestatement.cpp HEADERS += \ $$PWD/createtablesqlstatementbuilder.h \ + $$PWD/lastchangedrowid.h \ $$PWD/sqlitedatabasebackend.h \ $$PWD/sqlitedatabaseinterface.h \ $$PWD/sqliteexception.h \ diff --git a/src/libs/sqlite/sqlitedatabase.h b/src/libs/sqlite/sqlitedatabase.h index 323ac803259..94b0da03ba7 100644 --- a/src/libs/sqlite/sqlitedatabase.h +++ b/src/libs/sqlite/sqlitedatabase.h @@ -114,6 +114,13 @@ public: m_databaseBackend.walCheckpointFull(); } + void setUpdateHook(DatabaseBackend::UpdateCallback &callback) + { + m_databaseBackend.setUpdateHook(callback); + } + + void resetUpdateHook() { m_databaseBackend.resetUpdateHook(); } + private: void deferredBegin() override; void immediateBegin() override; diff --git a/src/libs/sqlite/sqlitedatabasebackend.cpp b/src/libs/sqlite/sqlitedatabasebackend.cpp index 4d5b3508017..a346ab7d066 100644 --- a/src/libs/sqlite/sqlitedatabasebackend.cpp +++ b/src/libs/sqlite/sqlitedatabasebackend.cpp @@ -344,13 +344,11 @@ int indexOfPragma(Utils::SmallStringView pragma, const Utils::SmallStringView (& } } -constexpr const Utils::SmallStringView journalModeStrings[] = { - "delete", - "truncate", - "persist", - "memory", - "wal" -}; +const Utils::SmallStringView journalModeStrings[] = {"delete", + "truncate", + "persist", + "memory", + "wal"}; Utils::SmallStringView DatabaseBackend::journalModeToPragma(JournalMode journalMode) { @@ -367,11 +365,7 @@ JournalMode DatabaseBackend::pragmaToJournalMode(Utils::SmallStringView pragma) return static_cast(index); } -constexpr const Utils::SmallStringView textEncodingStrings[] = { - "UTF-8", - "UTF-16le", - "UTF-16be" -}; +const Utils::SmallStringView textEncodingStrings[] = {"UTF-8", "UTF-16le", "UTF-16be"}; Utils::SmallStringView DatabaseBackend::textEncodingToPragma(TextEncoding textEncoding) { @@ -426,6 +420,29 @@ void DatabaseBackend::walCheckpointFull() } } +namespace { +void updateCallback( + void *callback, int type, char const *database, char const *table, sqlite3_int64 row) +{ + auto &function = *reinterpret_cast(callback); + + function(static_cast(type), database, table, row); +} +} // namespace + +void DatabaseBackend::setUpdateHook(UpdateCallback &callback) +{ + if (callback) + sqlite3_update_hook(m_databaseHandle, updateCallback, &callback); + else + sqlite3_update_hook(m_databaseHandle, nullptr, nullptr); +} + +void DatabaseBackend::resetUpdateHook() +{ + sqlite3_update_hook(m_databaseHandle, nullptr, nullptr); +} + void DatabaseBackend::throwExceptionStatic(const char *whatHasHappens) { throw Exception(whatHasHappens); diff --git a/src/libs/sqlite/sqlitedatabasebackend.h b/src/libs/sqlite/sqlitedatabasebackend.h index 7f3973f8787..7cf814b555c 100644 --- a/src/libs/sqlite/sqlitedatabasebackend.h +++ b/src/libs/sqlite/sqlitedatabasebackend.h @@ -40,6 +40,9 @@ class Database; class SQLITE_EXPORT DatabaseBackend { public: + using UpdateCallback + = std::function; + DatabaseBackend(Database &database); ~DatabaseBackend(); @@ -87,6 +90,9 @@ public: void walCheckpointFull(); + void setUpdateHook(UpdateCallback &callback); + void resetUpdateHook(); + protected: bool databaseIsOpen() const; @@ -128,7 +134,6 @@ private: Database &m_database; sqlite3 *m_databaseHandle; TextEncoding m_cachedTextEncoding; - }; } // namespace Sqlite diff --git a/src/libs/sqlite/sqlitedatabaseinterface.h b/src/libs/sqlite/sqlitedatabaseinterface.h index 61ea3ac928d..1c2a588b7ad 100644 --- a/src/libs/sqlite/sqlitedatabaseinterface.h +++ b/src/libs/sqlite/sqlitedatabaseinterface.h @@ -25,11 +25,23 @@ #pragma once +#include + +#include "sqliteglobal.h" + +#include + namespace Sqlite { class DatabaseInterface { public: + using UpdateCallback + = std::function; + virtual void walCheckpointFull() = 0; + virtual void execute(Utils::SmallStringView sqlStatement) = 0; + virtual void setUpdateHook(UpdateCallback &callback) = 0; + virtual void resetUpdateHook() = 0; protected: ~DatabaseInterface() = default; diff --git a/src/libs/sqlite/sqliteexception.h b/src/libs/sqlite/sqliteexception.h index 3bf2792d9dd..f34755df4d3 100644 --- a/src/libs/sqlite/sqliteexception.h +++ b/src/libs/sqlite/sqliteexception.h @@ -29,6 +29,8 @@ #include +#include + namespace Sqlite { class SQLITE_EXPORT Exception diff --git a/src/libs/sqlite/sqliteglobal.h b/src/libs/sqlite/sqliteglobal.h index 6df0849e18e..625c45a9ee4 100644 --- a/src/libs/sqlite/sqliteglobal.h +++ b/src/libs/sqlite/sqliteglobal.h @@ -84,4 +84,6 @@ enum TextEncoding : char }; +enum class ChangeType : int { Delete = 9, Insert = 18, Update = 23 }; + } // namespace Sqlite diff --git a/tests/unit/unittest/lastchangedrowid-test.cpp b/tests/unit/unittest/lastchangedrowid-test.cpp new file mode 100644 index 00000000000..10b83a97956 --- /dev/null +++ b/tests/unit/unittest/lastchangedrowid-test.cpp @@ -0,0 +1,108 @@ +/**************************************************************************** +** +** Copyright (C) 2020 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of Qt Creator. +** +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-3.0.html. +** +****************************************************************************/ + +#include "googletest.h" + +#include "mocksqlitedatabase.h" + +#include + +namespace { + +class LastChangedRowId : public testing::Test +{ +protected: + NiceMock mockSqliteDatabase; + Sqlite::LastChangedRowId lastRowId{mockSqliteDatabase, "main", "foo"}; +}; + +TEST_F(LastChangedRowId, SetUpdateHookInContructor) +{ + EXPECT_CALL(mockSqliteDatabase, setUpdateHook(_)); + + Sqlite::LastChangedRowId lastRowId{mockSqliteDatabase, "main", "foo"}; +} + +TEST_F(LastChangedRowId, ResetUpdateHookInDestructor) +{ + EXPECT_CALL(mockSqliteDatabase, resetUpdateHook()); +} + +TEST_F(LastChangedRowId, GetMinusOneAsRowIdIfNoCallbackWasCalled) +{ + ASSERT_THAT(lastRowId.lastRowId, -1); +} + +TEST_F(LastChangedRowId, CallbackSetsLastRowId) +{ + lastRowId.callback(Sqlite::ChangeType::Update, "main", "foo", 42); + + ASSERT_THAT(lastRowId.lastRowId, 42); +} + +TEST_F(LastChangedRowId, CallbackChecksDatabaseName) +{ + lastRowId.callback(Sqlite::ChangeType::Update, "temp", "foo", 42); + + ASSERT_THAT(lastRowId.lastRowId, -1); +} + +TEST_F(LastChangedRowId, CallbackChecksTableName) +{ + lastRowId.callback(Sqlite::ChangeType::Update, "main", "bar", 42); + + ASSERT_THAT(lastRowId.lastRowId, -1); +} + +TEST_F(LastChangedRowId, LastCallSetsRowId) +{ + lastRowId.callback(Sqlite::ChangeType::Update, "main", "foo", 42); + lastRowId.callback(Sqlite::ChangeType::Insert, "main", "foo", 33); + + lastRowId.callback(Sqlite::ChangeType::Delete, "main", "foo", 66); + + ASSERT_THAT(lastRowId.lastRowId, 66); +} + +TEST_F(LastChangedRowId, TakeLastRowId) +{ + lastRowId.callback(Sqlite::ChangeType::Update, "main", "foo", 42); + + auto id = lastRowId.takeLastRowId(); + + ASSERT_THAT(id, 42); +} + +TEST_F(LastChangedRowId, TakeLastRowIdResetsRowIdToMinusOne) +{ + lastRowId.callback(Sqlite::ChangeType::Update, "main", "foo", 42); + lastRowId.takeLastRowId(); + + auto id = lastRowId.takeLastRowId(); + + ASSERT_THAT(id, -1); +} + +} // namespace diff --git a/tests/unit/unittest/mocksqlitedatabase.h b/tests/unit/unittest/mocksqlitedatabase.h index 05f6f4e9581..97d5394339b 100644 --- a/tests/unit/unittest/mocksqlitedatabase.h +++ b/tests/unit/unittest/mocksqlitedatabase.h @@ -59,5 +59,9 @@ public: void (bool)); MOCK_METHOD0(walCheckpointFull, void()); + + MOCK_METHOD1(setUpdateHook, void(Sqlite::DatabaseInterface::UpdateCallback &)); + + MOCK_METHOD0(resetUpdateHook, void()); }; diff --git a/tests/unit/unittest/sqlitedatabase-test.cpp b/tests/unit/unittest/sqlitedatabase-test.cpp index 66c1aa83879..ff3436c4592 100644 --- a/tests/unit/unittest/sqlitedatabase-test.cpp +++ b/tests/unit/unittest/sqlitedatabase-test.cpp @@ -71,6 +71,8 @@ protected: QString databaseFilePath{":memory:"}; Sqlite::Database database; Sqlite::TransactionInterface &transactionInterface = database; + MockFunction callbackMock; + Sqlite::Database::UpdateCallback callback = callbackMock.AsStdFunction(); }; TEST_F(SqliteDatabase, SetDatabaseFilePath) @@ -220,4 +222,89 @@ TEST_F(SqliteDatabase, Rollback) ASSERT_NO_THROW(transactionInterface.rollback()); } +TEST_F(SqliteDatabase, SetUpdateHookSet) +{ + database.setUpdateHook(callback); + + EXPECT_CALL(callbackMock, Call(_, _, _, _)); + Sqlite::WriteStatement("INSERT INTO test(name) VALUES (?)", database).write(42); } + +TEST_F(SqliteDatabase, SetNullUpdateHook) +{ + database.setUpdateHook(callback); + Sqlite::Database::UpdateCallback newCallback; + + database.setUpdateHook(newCallback); + + EXPECT_CALL(callbackMock, Call(_, _, _, _)).Times(0); + Sqlite::WriteStatement("INSERT INTO test(name) VALUES (?)", database).write(42); +} + +TEST_F(SqliteDatabase, ResetUpdateHook) +{ + database.setUpdateHook(callback); + Sqlite::Database::UpdateCallback newCallback; + + database.resetUpdateHook(); + + EXPECT_CALL(callbackMock, Call(_, _, _, _)).Times(0); + Sqlite::WriteStatement("INSERT INTO test(name) VALUES (?)", database).write(42); +} + +TEST_F(SqliteDatabase, DeleteUpdateHookCall) +{ + Sqlite::WriteStatement("INSERT INTO test(name) VALUES (?)", database).write(42); + database.setUpdateHook(callback); + + EXPECT_CALL(callbackMock, Call(Eq(Sqlite::ChangeType::Delete), _, _, _)); + + Sqlite::WriteStatement("DELETE FROM test WHERE name = 42", database).execute(); +} + +TEST_F(SqliteDatabase, InsertUpdateHookCall) +{ + database.setUpdateHook(callback); + + EXPECT_CALL(callbackMock, Call(Eq(Sqlite::ChangeType::Insert), _, _, _)); + + Sqlite::WriteStatement("INSERT INTO test(name) VALUES (?)", database).write(42); +} + +TEST_F(SqliteDatabase, UpdateUpdateHookCall) +{ + database.setUpdateHook(callback); + + EXPECT_CALL(callbackMock, Call(Eq(Sqlite::ChangeType::Insert), _, _, _)); + + Sqlite::WriteStatement("INSERT INTO test(name) VALUES (?)", database).write(42); +} + +TEST_F(SqliteDatabase, RowIdUpdateHookCall) +{ + database.setUpdateHook(callback); + + EXPECT_CALL(callbackMock, Call(_, _, _, Eq(42))); + + Sqlite::WriteStatement("INSERT INTO test(rowid, name) VALUES (?,?)", database).write(42, "foo"); +} + +TEST_F(SqliteDatabase, DatabaseUpdateHookCall) +{ + database.setUpdateHook(callback); + + EXPECT_CALL(callbackMock, Call(_, StrEq("main"), _, _)); + + Sqlite::WriteStatement("INSERT INTO test(name) VALUES (?)", database).write(42); +} + +TEST_F(SqliteDatabase, TableUpdateHookCall) +{ + database.setUpdateHook(callback); + + EXPECT_CALL(callbackMock, Call(_, _, StrEq("test"), _)); + + Sqlite::WriteStatement("INSERT INTO test(name) VALUES (?)", database).write(42); +} + +} // namespace diff --git a/tests/unit/unittest/unittest.pro b/tests/unit/unittest/unittest.pro index 6155cf32128..7340f007754 100644 --- a/tests/unit/unittest/unittest.pro +++ b/tests/unit/unittest/unittest.pro @@ -63,6 +63,7 @@ SOURCES += \ filepathview-test.cpp \ gtest-creator-printing.cpp \ gtest-qt-printing.cpp \ + lastchangedrowid-test.cpp \ lineprefixer-test.cpp \ locatorfilter-test.cpp \ matchingtext-test.cpp \