Skip to content

Commit 7a198bb

Browse files
achow101achow101
committed
wallet: Introduce DatabaseCursor RAII class for managing cursor
Instead of having DatabaseBatch deal with opening and closing database cursors, have a separate RAII class that deals with those. For now, DatabaseBatch manages DatabaseCursor, but this will change later.
1 parent 69efbc0 commit 7a198bb

File tree

6 files changed

+108
-44
lines changed

6 files changed

+108
-44
lines changed

‎src/wallet/bdb.cpp‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <wallet/bdb.h>
99
#include <wallet/db.h>
1010

11+
#include <util/check.h>
1112
#include <util/strencodings.h>
1213
#include <util/translation.h>
1314

@@ -307,7 +308,7 @@ BerkeleyDatabase::~BerkeleyDatabase()
307308
}
308309
}
309310

310-
BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const bool read_only, bool fFlushOnCloseIn) : pdb(nullptr), activeTxn(nullptr), m_cursor(nullptr), m_database(database)
311+
BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const bool read_only, bool fFlushOnCloseIn) : pdb(nullptr), activeTxn(nullptr), m_database(database)
311312
{
312313
database.AddRef();
313314
database.Open();
@@ -656,16 +657,18 @@ void BerkeleyDatabase::ReloadDbEnv()
656657
env->ReloadDbEnv();
657658
}
658659

659-
bool BerkeleyBatch::StartCursor()
660+
BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database)
660661
{
661-
assert(!m_cursor);
662-
if (!pdb)
663-
return false;
664-
int ret = pdb->cursor(nullptr, &m_cursor, 0);
665-
return ret == 0;
662+
if (!database.m_db.get()) {
663+
throw std::runtime_error(STR_INTERNAL_BUG("BerkeleyDatabase does not exist"));
664+
}
665+
int ret = database.m_db->cursor(nullptr, &m_cursor, 0);
666+
if (ret != 0) {
667+
throw std::runtime_error(STR_INTERNAL_BUG(strprintf("BDB Cursor could not be created. Returned %d", ret).c_str()));
668+
}
666669
}
667670

668-
bool BerkeleyBatch::ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete)
671+
bool BerkeleyCursor::Next(CDataStream& ssKey, CDataStream& ssValue, bool& complete)
669672
{
670673
complete = false;
671674
if (m_cursor == nullptr) return false;
@@ -691,13 +694,19 @@ bool BerkeleyBatch::ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool&
691694
return true;
692695
}
693696

694-
void BerkeleyBatch::CloseCursor()
697+
BerkeleyCursor::~BerkeleyCursor()
695698
{
696699
if (!m_cursor) return;
697700
m_cursor->close();
698701
m_cursor = nullptr;
699702
}
700703

704+
std::unique_ptr<DatabaseCursor> BerkeleyBatch::GetNewCursor()
705+
{
706+
if (!pdb) return nullptr;
707+
return std::make_unique<BerkeleyCursor>(m_database);
708+
}
709+
701710
bool BerkeleyBatch::TxnBegin()
702711
{
703712
if (!pdb || activeTxn)

‎src/wallet/bdb.h‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,18 @@ class SafeDbt final
185185
operator Dbt*();
186186
};
187187

188+
class BerkeleyCursor : public DatabaseCursor
189+
{
190+
private:
191+
Dbc* m_cursor;
192+
193+
public:
194+
explicit BerkeleyCursor(BerkeleyDatabase& database);
195+
~BerkeleyCursor() override;
196+
197+
bool Next(CDataStream& key, CDataStream& value, bool& complete) override;
198+
};
199+
188200
/** RAII class that provides access to a Berkeley database */
189201
class BerkeleyBatch : public DatabaseBatch
190202
{
@@ -198,7 +210,6 @@ class BerkeleyBatch : public DatabaseBatch
198210
Db* pdb;
199211
std::string strFile;
200212
DbTxn* activeTxn;
201-
Dbc* m_cursor;
202213
bool fReadOnly;
203214
bool fFlushOnClose;
204215
BerkeleyEnvironment *env;
@@ -214,9 +225,7 @@ class BerkeleyBatch : public DatabaseBatch
214225
void Flush() override;
215226
void Close() override;
216227

217-
bool StartCursor() override;
218-
bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override;
219-
void CloseCursor() override;
228+
std::unique_ptr<DatabaseCursor> GetNewCursor() override;
220229
bool TxnBegin() override;
221230
bool TxnCommit() override;
222231
bool TxnAbort() override;

‎src/wallet/db.h‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,24 @@ struct bilingual_str;
2222
namespace wallet {
2323
void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std::string& database_filename);
2424

25+
class DatabaseCursor
26+
{
27+
public:
28+
explicit DatabaseCursor() {}
29+
virtual ~DatabaseCursor() {}
30+
31+
DatabaseCursor(const DatabaseCursor&) = delete;
32+
DatabaseCursor& operator=(const DatabaseCursor&) = delete;
33+
34+
virtual bool Next(CDataStream& key, CDataStream& value, bool& complete) { return false; }
35+
};
36+
2537
/** RAII class that provides access to a WalletDatabase */
2638
class DatabaseBatch
2739
{
2840
private:
41+
std::unique_ptr<DatabaseCursor> m_cursor;
42+
2943
virtual bool ReadKey(CDataStream&& key, CDataStream& value) = 0;
3044
virtual bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite=true) = 0;
3145
virtual bool EraseKey(CDataStream&& key) = 0;
@@ -92,9 +106,21 @@ class DatabaseBatch
92106
return HasKey(std::move(ssKey));
93107
}
94108

95-
virtual bool StartCursor() = 0;
96-
virtual bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) = 0;
97-
virtual void CloseCursor() = 0;
109+
virtual std::unique_ptr<DatabaseCursor> GetNewCursor() = 0;
110+
bool StartCursor()
111+
{
112+
m_cursor = GetNewCursor();
113+
return m_cursor != nullptr;
114+
}
115+
bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete)
116+
{
117+
if (!m_cursor) return false;
118+
return m_cursor->Next(ssKey, ssValue, complete);
119+
}
120+
void CloseCursor()
121+
{
122+
m_cursor.reset();
123+
}
98124
virtual bool TxnBegin() = 0;
99125
virtual bool TxnCommit() = 0;
100126
virtual bool TxnAbort() = 0;
@@ -156,6 +182,11 @@ class WalletDatabase
156182
virtual std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) = 0;
157183
};
158184

185+
class DummyCursor : public DatabaseCursor
186+
{
187+
bool Next(CDataStream& key, CDataStream& value, bool& complete) override { return false; }
188+
};
189+
159190
/** RAII class that provides access to a DummyDatabase. Never fails. */
160191
class DummyBatch : public DatabaseBatch
161192
{
@@ -169,9 +200,7 @@ class DummyBatch : public DatabaseBatch
169200
void Flush() override {}
170201
void Close() override {}
171202

172-
bool StartCursor() override { return true; }
173-
bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override { return true; }
174-
void CloseCursor() override {}
203+
std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<DummyCursor>(); }
175204
bool TxnBegin() override { return true; }
176205
bool TxnCommit() override { return true; }
177206
bool TxnAbort() override { return true; }

‎src/wallet/sqlite.cpp‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,6 @@ void SQLiteBatch::SetupSQLStatements()
125125
{&m_insert_stmt, "INSERT INTO main VALUES(?, ?)"},
126126
{&m_overwrite_stmt, "INSERT or REPLACE into main values(?, ?)"},
127127
{&m_delete_stmt, "DELETE FROM main WHERE key = ?"},
128-
{&m_cursor_stmt, "SELECT key, value FROM main"},
129128
};
130129

131130
for (const auto& [stmt_prepared, stmt_text] : statements) {
@@ -374,7 +373,6 @@ void SQLiteBatch::Close()
374373
{&m_insert_stmt, "insert"},
375374
{&m_overwrite_stmt, "overwrite"},
376375
{&m_delete_stmt, "delete"},
377-
{&m_cursor_stmt, "cursor"},
378376
};
379377

380378
for (const auto& [stmt_prepared, stmt_description] : statements) {
@@ -472,27 +470,17 @@ bool SQLiteBatch::HasKey(CDataStream&& key)
472470
return res == SQLITE_ROW;
473471
}
474472

475-
bool SQLiteBatch::StartCursor()
476-
{
477-
assert(!m_cursor_init);
478-
if (!m_database.m_db) return false;
479-
m_cursor_init = true;
480-
return true;
481-
}
482-
483-
bool SQLiteBatch::ReadAtCursor(CDataStream& key, CDataStream& value, bool& complete)
473+
bool SQLiteCursor::Next(CDataStream& key, CDataStream& value, bool& complete)
484474
{
485475
complete = false;
486476

487-
if (!m_cursor_init) return false;
488-
489477
int res = sqlite3_step(m_cursor_stmt);
490478
if (res == SQLITE_DONE) {
491479
complete = true;
492480
return true;
493481
}
494482
if (res != SQLITE_ROW) {
495-
LogPrintf("SQLiteBatch::ReadAtCursor: Unable to execute cursor step: %s\n", sqlite3_errstr(res));
483+
LogPrintf("%s: Unable to execute cursor step: %s\n", __func__, sqlite3_errstr(res));
496484
return false;
497485
}
498486

@@ -506,10 +494,29 @@ bool SQLiteBatch::ReadAtCursor(CDataStream& key, CDataStream& value, bool& compl
506494
return true;
507495
}
508496

509-
void SQLiteBatch::CloseCursor()
497+
SQLiteCursor::~SQLiteCursor()
510498
{
511499
sqlite3_reset(m_cursor_stmt);
512-
m_cursor_init = false;
500+
int res = sqlite3_finalize(m_cursor_stmt);
501+
if (res != SQLITE_OK) {
502+
LogPrintf("%s: cursor closed but could not finalize cursor statement: %s\n",
503+
__func__, sqlite3_errstr(res));
504+
}
505+
}
506+
507+
std::unique_ptr<DatabaseCursor> SQLiteBatch::GetNewCursor()
508+
{
509+
if (!m_database.m_db) return nullptr;
510+
auto cursor = std::make_unique<SQLiteCursor>();
511+
512+
const char* stmt_text = "SELECT key, value FROM main";
513+
int res = sqlite3_prepare_v2(m_database.m_db, stmt_text, -1, &cursor->m_cursor_stmt, nullptr);
514+
if (res != SQLITE_OK) {
515+
throw std::runtime_error(strprintf(
516+
"%s: Failed to setup cursor SQL statement: %s\n", __func__, sqlite3_errstr(res)));
517+
}
518+
519+
return cursor;
513520
}
514521

515522
bool SQLiteBatch::TxnBegin()

‎src/wallet/sqlite.h‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,27 @@ struct bilingual_str;
1414
namespace wallet {
1515
class SQLiteDatabase;
1616

17+
class SQLiteCursor : public DatabaseCursor
18+
{
19+
public:
20+
sqlite3_stmt* m_cursor_stmt{nullptr};
21+
22+
explicit SQLiteCursor() {}
23+
~SQLiteCursor() override;
24+
25+
bool Next(CDataStream& key, CDataStream& value, bool& complete) override;
26+
};
27+
1728
/** RAII class that provides access to a WalletDatabase */
1829
class SQLiteBatch : public DatabaseBatch
1930
{
2031
private:
2132
SQLiteDatabase& m_database;
2233

23-
bool m_cursor_init = false;
24-
2534
sqlite3_stmt* m_read_stmt{nullptr};
2635
sqlite3_stmt* m_insert_stmt{nullptr};
2736
sqlite3_stmt* m_overwrite_stmt{nullptr};
2837
sqlite3_stmt* m_delete_stmt{nullptr};
29-
sqlite3_stmt* m_cursor_stmt{nullptr};
3038

3139
void SetupSQLStatements();
3240

@@ -44,9 +52,7 @@ class SQLiteBatch : public DatabaseBatch
4452

4553
void Close() override;
4654

47-
bool StartCursor() override;
48-
bool ReadAtCursor(CDataStream& key, CDataStream& value, bool& complete) override;
49-
void CloseCursor() override;
55+
std::unique_ptr<DatabaseCursor> GetNewCursor() override;
5056
bool TxnBegin() override;
5157
bool TxnCommit() override;
5258
bool TxnAbort() override;

‎src/wallet/test/wallet_tests.cpp‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,12 @@ BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup)
867867
TestUnloadWallet(std::move(wallet));
868868
}
869869

870+
class FailCursor : public DatabaseCursor
871+
{
872+
public:
873+
bool Next(CDataStream& key, CDataStream& value, bool& complete) override { return false; }
874+
};
875+
870876
/** RAII class that provides access to a FailDatabase. Which fails if needed. */
871877
class FailBatch : public DatabaseBatch
872878
{
@@ -882,9 +888,7 @@ class FailBatch : public DatabaseBatch
882888
void Flush() override {}
883889
void Close() override {}
884890

885-
bool StartCursor() override { return true; }
886-
bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override { return false; }
887-
void CloseCursor() override {}
891+
std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<FailCursor>(); }
888892
bool TxnBegin() override { return false; }
889893
bool TxnCommit() override { return false; }
890894
bool TxnAbort() override { return false; }

0 commit comments

Comments
 (0)