diff --git a/CHANGELOG.md b/CHANGELOG.md index d596471..7862d0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,12 @@ # Changelog -## LMDBAL 0.5.2 (October 21, 2023) +## LMDBAL 0.5.2 (November 01, 2023) ### Improvements - RAII cursors +- operation set for cursors + +### Bug fixes +- error beginning transaction is now correctly handled and doesn't segfault ## LMDBAL 0.5.1 (October 21, 2023) ### Improvements diff --git a/src/base.cpp b/src/base.cpp index 81f9bdb..149343b 100644 --- a/src/base.cpp +++ b/src/base.cpp @@ -402,10 +402,9 @@ void LMDBAL::Base::commitTransaction(LMDBAL::TransactionID id, const std::string LMDBAL::TransactionID LMDBAL::Base::beginPrivateReadOnlyTransaction(const std::string& storageName) const { MDB_txn* txn; int rc = mdb_txn_begin(environment, NULL, MDB_RDONLY, &txn); - if (rc != MDB_SUCCESS) { - mdb_txn_abort(txn); + if (rc != MDB_SUCCESS) throw Unknown(name, mdb_strerror(rc), storageName); - } + return txn; } diff --git a/src/cursor.h b/src/cursor.h index 2a5114b..c3ff362 100644 --- a/src/cursor.h +++ b/src/cursor.h @@ -73,6 +73,7 @@ public: private: void dropped(); + void freed(); void terminated(); void operateCursorRead(K& key, V& value, MDB_cursor_op operation, const std::string& methodName, const std::string& operationName) const; @@ -91,6 +92,7 @@ private: inline static const std::string nextMethodName = "next"; /**<\brief member function name, just for exceptions*/ inline static const std::string prevMethodName = "prev"; /**<\brief member function name, just for exceptions*/ inline static const std::string currentMethodName = "current"; /**<\brief member function name, just for exceptions*/ + inline static const std::string setMethodName = "set"; /**<\brief member function name, just for exceptions*/ inline static const std::string firstOperationName = "Cursor::first"; /**<\brief member function name, just for exceptions*/ inline static const std::string lastOperationName = "Cursor::last"; /**<\brief member function name, just for exceptions*/ diff --git a/src/cursor.hpp b/src/cursor.hpp index 027c6fd..bfcc44a 100644 --- a/src/cursor.hpp +++ b/src/cursor.hpp @@ -81,13 +81,11 @@ LMDBAL::Cursor::Cursor(Cursor&& other): state(other.state), id(other.id) { + other.terminated(); if (id != 0) storage->cursors[id] = this; - other.cursor = nullptr; - other.storage = nullptr; - other.id = 0; - other.state = closed; + other.freed(); } /** @@ -109,9 +107,7 @@ LMDBAL::Cursor& LMDBAL::Cursor::operator = (Cursor&& other) { id = other.id; if (id != 0) { - other.cursor = nullptr; - other.storage = nullptr; - other.id = 0; + other.freed(); other.state = closed; storage->cursors[id] = this; @@ -146,20 +142,29 @@ void LMDBAL::Cursor::drop () { if (id != 0) storage->cursors.erase(id); - cursor = nullptr; - storage = nullptr; - id = 0; + freed(); } /** * \brief A private method that turns cursor into an empty one * - * This function is called from LMDBAL::Storage, when it gets destroyed, but still has some valid. + * This function is called from LMDBAL::Storage, when it gets destroyed, but still has some valid cursors. * Those cursors will become empty, and can't be used anymore */ template void LMDBAL::Cursor::dropped () { terminated(); + freed(); +} + +/** + * \brief A private method that turns cursor into an empty one (submethod) + * + * This function is called from LMDBAL::Storage, when the cursor is getting destoryed. + * Those cursors will become empty, and can't be used anymore + */ +template +void LMDBAL::Cursor::freed () { cursor = nullptr; storage = nullptr; id = 0; @@ -585,6 +590,34 @@ std::pair LMDBAL::Cursor::current () const { return result; } +/** + * \brief Sets cursors to the defined position + * + * If exactly the same element wasn't found it sets the cursor to the first + * element greater then the key and returns false + * + * \param[in] key a key of the element you would like the cursor to be + * \returns true if the exact value was found, false otherwise + * + * \exception LMDBAL::CursorNotReady thrown if you try to call this method on a closed cursor + * \exception LMDBAL::Unknown thrown if there was some unexpected problem + */ +template +bool LMDBAL::Cursor::set (const K& key) { + if (state == closed) + storage->throwCursorNotReady(setMethodName); + + MDB_val mdbKey = storage->keySerializer.setData(key); + int result = mdb_cursor_get(cursor, &mdbKey, nullptr, MDB_SET); + if (result == MDB_SUCCESS) + return true; + else if (result == MDB_NOTFOUND) + return false; + + storage->throwUnknown(result); + return false; //unreachable, just to suppress the warning +} + /** * \brief a private mothod that actually doing all the reading * diff --git a/src/storage.hpp b/src/storage.hpp index f533217..1bab63b 100644 --- a/src/storage.hpp +++ b/src/storage.hpp @@ -1022,6 +1022,9 @@ void LMDBAL::Storage::close() { /** * \brief Creates cursor * + * This is a legitimate way to aquire cursor to a storage. + * Aquired cursor is RAII safe, automatic destructor will free everything it occupied. + * * \returns LMDBAL::Cursor for this storage and returs you a pointer to a created cursor */ template @@ -1029,6 +1032,29 @@ LMDBAL::Cursor LMDBAL::Storage::createCursor() { return Cursor(this); } +/** + * \brief Frees cursor + * + * This is a legitimate way to free cursor. + * You don't actually need to do it manually, + * you can just reassign cursor, let it be destroyed by leaving the scope + * or call LMDBAL::Cursor::drop, but you may if you wish. + * + * \param[in] cursor cursor you wish to destroy + * + * \exception LMDBAL::Unknown thrown if you try to destroy a cursor this storage didn't create + */ +template +void LMDBAL::Storage::destroyCursor(LMDBAL::Cursor& cursor) { + typename std::map*>::iterator itr = cursors.find(cursor.id); + if (itr == cursors.end()) + throwUnknown("An attempt to destroy a cursor the storage doesn't own"); + + cursor.close(); + cursors.erase(itr); + cursor.freed(); +} + /** * \brief Reads current storage flags it was opened with * diff --git a/test/cachecursor.cpp b/test/cachecursor.cpp index c988bc1..7d9d440 100644 --- a/test/cachecursor.cpp +++ b/test/cachecursor.cpp @@ -23,7 +23,12 @@ protected: } } + int getCacheCursorsSize() const { + return cache->cursors.size(); + } + static void TearDownTestSuite() { + cursor.drop(); transaction.terminate(); db->close(); db->removeDirectory(); @@ -62,7 +67,9 @@ TEST_F(CacheCursorTest, PopulatingTheTable) { } TEST_F(CacheCursorTest, Creation) { + EXPECT_EQ(getCacheCursorsSize(), 0); cursor = cache->createCursor(); + EXPECT_EQ(getCacheCursorsSize(), 1); EXPECT_THROW(cursor.first(), LMDBAL::CursorNotReady); EXPECT_THROW(cursor.last(), LMDBAL::CursorNotReady); @@ -74,6 +81,7 @@ TEST_F(CacheCursorTest, Creation) { } TEST_F(CacheCursorTest, FirstPrivate) { + EXPECT_EQ(getCacheCursorsSize(), 1); EXPECT_EQ(cache->count(), data.size()); std::pair element = cursor.first(); @@ -85,6 +93,7 @@ TEST_F(CacheCursorTest, FirstPrivate) { } TEST_F(CacheCursorTest, NextPrivate) { + EXPECT_EQ(getCacheCursorsSize(), 1); std::map::const_iterator reference = data.begin(); reference++; @@ -107,6 +116,7 @@ TEST_F(CacheCursorTest, NextPrivate) { } TEST_F(CacheCursorTest, LastPrivate) { + EXPECT_EQ(getCacheCursorsSize(), 1); EXPECT_EQ(cache->count(), data.size()); std::pair element = cursor.last(); @@ -118,6 +128,7 @@ TEST_F(CacheCursorTest, LastPrivate) { } TEST_F(CacheCursorTest, PrevPrivate) { + EXPECT_EQ(getCacheCursorsSize(), 1); std::map::const_reverse_iterator reference = data.rbegin(); reference++; @@ -140,6 +151,7 @@ TEST_F(CacheCursorTest, PrevPrivate) { } TEST_F(CacheCursorTest, CurrentPrivate) { + EXPECT_EQ(getCacheCursorsSize(), 1); std::pair element = cursor.first(); std::map::const_iterator reference = data.begin(); @@ -172,8 +184,42 @@ TEST_F(CacheCursorTest, CurrentPrivate) { EXPECT_EQ(cache->count(), data.size()); } +TEST_F(CacheCursorTest, SettingPrivate) { + EXPECT_EQ(getCacheCursorsSize(), 1); + + EXPECT_FALSE(cursor.set(6684)); + EXPECT_EQ(cursor.current().second, "tanned inmate"); + + std::map::const_iterator reference = data.begin(); + std::advance(reference, 5); + EXPECT_TRUE(cursor.set(reference->first)); + EXPECT_EQ(cursor.current().second, reference->second); + + ++reference; + cursor.next(); + EXPECT_EQ(cursor.current().second, reference->second); + + ++reference; + cursor.next(); + EXPECT_EQ(cursor.current().second, reference->second); + + --reference; + cursor.prev(); + EXPECT_EQ(cursor.current().second, reference->second); + + --reference; + cursor.prev(); + EXPECT_EQ(cursor.current().second, reference->second); + + --reference; + cursor.prev(); + EXPECT_EQ(cursor.current().second, reference->second); +} + TEST_F(CacheCursorTest, Destruction) { + EXPECT_EQ(getCacheCursorsSize(), 1); cursor.close(); + EXPECT_EQ(getCacheCursorsSize(), 1); EXPECT_THROW(cursor.first(), LMDBAL::CursorNotReady); EXPECT_THROW(cursor.last(), LMDBAL::CursorNotReady); @@ -182,11 +228,14 @@ TEST_F(CacheCursorTest, Destruction) { EXPECT_THROW(cursor.current(), LMDBAL::CursorNotReady); cursor = LMDBAL::Cursor(); + EXPECT_EQ(getCacheCursorsSize(), 0); cursor = cache->createCursor(); + EXPECT_EQ(getCacheCursorsSize(), 1); } TEST_F(CacheCursorTest, FirstPublic) { + EXPECT_EQ(getCacheCursorsSize(), 1); transaction = db->beginTransaction(); cursor.open(transaction); @@ -199,6 +248,7 @@ TEST_F(CacheCursorTest, FirstPublic) { } TEST_F(CacheCursorTest, NextPublic) { + EXPECT_EQ(getCacheCursorsSize(), 1); std::map::const_iterator reference = data.begin(); reference++; @@ -220,6 +270,7 @@ TEST_F(CacheCursorTest, NextPublic) { } TEST_F(CacheCursorTest, LastPublic) { + EXPECT_EQ(getCacheCursorsSize(), 1); std::pair element = cursor.last(); std::map::const_reverse_iterator reference = data.rbegin(); @@ -229,6 +280,7 @@ TEST_F(CacheCursorTest, LastPublic) { } TEST_F(CacheCursorTest, PrevPublic) { + EXPECT_EQ(getCacheCursorsSize(), 1); std::map::const_reverse_iterator reference = data.rbegin(); reference++; @@ -250,6 +302,7 @@ TEST_F(CacheCursorTest, PrevPublic) { } TEST_F(CacheCursorTest, CurrentPublic) { + EXPECT_EQ(getCacheCursorsSize(), 1); std::pair element = cursor.first(); std::map::const_iterator reference = data.begin(); @@ -282,6 +335,71 @@ TEST_F(CacheCursorTest, CurrentPublic) { EXPECT_EQ(cache->count(), data.size()); } +TEST_F(CacheCursorTest, SettingPublic) { + EXPECT_EQ(getCacheCursorsSize(), 1); + + EXPECT_FALSE(cursor.set(557)); + EXPECT_EQ(cursor.current().second, "resilent pick forefront"); + + std::map::const_iterator reference = data.begin(); + std::advance(reference, 3); + EXPECT_TRUE(cursor.set(reference->first)); + EXPECT_EQ(cursor.current().second, reference->second); + + ++reference; + cursor.next(); + EXPECT_EQ(cursor.current().second, reference->second); + + ++reference; + cursor.next(); + EXPECT_EQ(cursor.current().second, reference->second); + + --reference; + cursor.prev(); + EXPECT_EQ(cursor.current().second, reference->second); + + --reference; + cursor.prev(); + EXPECT_EQ(cursor.current().second, reference->second); + + --reference; + cursor.prev(); + EXPECT_EQ(cursor.current().second, reference->second); +} + +TEST_F(CacheCursorTest, CursorRAIIBehaviour) { + int initialiCursorsAmount = getCacheCursorsSize(); + { + LMDBAL::Cursor cur = cache->createCursor(); + EXPECT_EQ(initialiCursorsAmount + 1, getCacheCursorsSize()); + cur.open(transaction); + EXPECT_NO_THROW(cur.first()); + } + + EXPECT_EQ(initialiCursorsAmount, getCacheCursorsSize()); + LMDBAL::Cursor cur; + EXPECT_EQ(initialiCursorsAmount, getCacheCursorsSize()); + EXPECT_EQ(cur.empty(), true); + cur = cache->createCursor(); + EXPECT_EQ(cur.empty(), false); + EXPECT_EQ(initialiCursorsAmount + 1, getCacheCursorsSize()); + EXPECT_THROW(emptyCache->destroyCursor(cur), LMDBAL::Unknown); + cache->destroyCursor(cur); + EXPECT_EQ(cur.empty(), true); + EXPECT_EQ(initialiCursorsAmount, getCacheCursorsSize()); + + cur = cache->createCursor(); + EXPECT_EQ(initialiCursorsAmount + 1, getCacheCursorsSize()); + EXPECT_EQ(cur.empty(), false); + + cur.drop(); + EXPECT_EQ(cur.empty(), true); + EXPECT_EQ(initialiCursorsAmount, getCacheCursorsSize()); + + EXPECT_NO_THROW(cur.drop()); + EXPECT_THROW(cache->destroyCursor(cur), LMDBAL::Unknown); +} + TEST_F(CacheCursorTest, CornerCases) { transaction.terminate(); EXPECT_THROW(cursor.current(), LMDBAL::Unknown); diff --git a/test/storagecursor.cpp b/test/storagecursor.cpp index b8de67c..dde8d82 100644 --- a/test/storagecursor.cpp +++ b/test/storagecursor.cpp @@ -22,7 +22,12 @@ protected: } } + int getTableCursorsSize() const { + return table->cursors.size(); + } + static void TearDownTestSuite() { + cursor.drop(); transaction.terminate(); db->close(); db->removeDirectory(); @@ -61,7 +66,9 @@ TEST_F(StorageCursorTest, PopulatingTheTable) { } TEST_F(StorageCursorTest, Creation) { + EXPECT_EQ(getTableCursorsSize(), 0); cursor = table->createCursor(); + EXPECT_EQ(getTableCursorsSize(), 1); EXPECT_THROW(cursor.first(), LMDBAL::CursorNotReady); EXPECT_THROW(cursor.last(), LMDBAL::CursorNotReady); @@ -73,6 +80,7 @@ TEST_F(StorageCursorTest, Creation) { } TEST_F(StorageCursorTest, FirstPrivate) { + EXPECT_EQ(getTableCursorsSize(), 1); std::pair element = cursor.first(); std::map::const_iterator reference = data.begin(); @@ -81,6 +89,7 @@ TEST_F(StorageCursorTest, FirstPrivate) { } TEST_F(StorageCursorTest, NextPrivate) { + EXPECT_EQ(getTableCursorsSize(), 1); std::map::const_iterator reference = data.begin(); reference++; @@ -100,6 +109,7 @@ TEST_F(StorageCursorTest, NextPrivate) { } TEST_F(StorageCursorTest, LastPrivate) { + EXPECT_EQ(getTableCursorsSize(), 1); std::pair element = cursor.last(); std::map::const_reverse_iterator reference = data.rbegin(); @@ -108,6 +118,7 @@ TEST_F(StorageCursorTest, LastPrivate) { } TEST_F(StorageCursorTest, PrevPrivate) { + EXPECT_EQ(getTableCursorsSize(), 1); std::map::const_reverse_iterator reference = data.rbegin(); reference++; @@ -127,6 +138,7 @@ TEST_F(StorageCursorTest, PrevPrivate) { } TEST_F(StorageCursorTest, CurrentPrivate) { + EXPECT_EQ(getTableCursorsSize(), 1); std::pair element = cursor.first(); std::map::const_iterator reference = data.begin(); @@ -157,8 +169,42 @@ TEST_F(StorageCursorTest, CurrentPrivate) { EXPECT_EQ(element.second, reference->second); } +TEST_F(StorageCursorTest, SettingPrivate) { + EXPECT_EQ(getTableCursorsSize(), 1); + + EXPECT_FALSE(cursor.set(6684)); + EXPECT_EQ(cursor.current().second, "tanned inmate"); + + std::map::const_iterator reference = data.begin(); + std::advance(reference, 5); + EXPECT_TRUE(cursor.set(reference->first)); + EXPECT_EQ(cursor.current().second, reference->second); + + ++reference; + cursor.next(); + EXPECT_EQ(cursor.current().second, reference->second); + + ++reference; + cursor.next(); + EXPECT_EQ(cursor.current().second, reference->second); + + --reference; + cursor.prev(); + EXPECT_EQ(cursor.current().second, reference->second); + + --reference; + cursor.prev(); + EXPECT_EQ(cursor.current().second, reference->second); + + --reference; + cursor.prev(); + EXPECT_EQ(cursor.current().second, reference->second); +} + TEST_F(StorageCursorTest, Destruction) { + EXPECT_EQ(getTableCursorsSize(), 1); cursor.close(); + EXPECT_EQ(getTableCursorsSize(), 1); EXPECT_THROW(cursor.first(), LMDBAL::CursorNotReady); EXPECT_THROW(cursor.last(), LMDBAL::CursorNotReady); @@ -167,11 +213,14 @@ TEST_F(StorageCursorTest, Destruction) { EXPECT_THROW(cursor.current(), LMDBAL::CursorNotReady); cursor = LMDBAL::Cursor(); + EXPECT_EQ(getTableCursorsSize(), 0); cursor = table->createCursor(); + EXPECT_EQ(getTableCursorsSize(), 1); } TEST_F(StorageCursorTest, FirstPublic) { + EXPECT_EQ(getTableCursorsSize(), 1); transaction = db->beginReadOnlyTransaction(); cursor.open(transaction); @@ -183,6 +232,7 @@ TEST_F(StorageCursorTest, FirstPublic) { } TEST_F(StorageCursorTest, NextPublic) { + EXPECT_EQ(getTableCursorsSize(), 1); std::map::const_iterator reference = data.begin(); reference++; @@ -202,6 +252,7 @@ TEST_F(StorageCursorTest, NextPublic) { } TEST_F(StorageCursorTest, LastPublic) { + EXPECT_EQ(getTableCursorsSize(), 1); std::pair element = cursor.last(); std::map::const_reverse_iterator reference = data.rbegin(); @@ -210,6 +261,7 @@ TEST_F(StorageCursorTest, LastPublic) { } TEST_F(StorageCursorTest, PrevPublic) { + EXPECT_EQ(getTableCursorsSize(), 1); std::map::const_reverse_iterator reference = data.rbegin(); reference++; @@ -229,6 +281,7 @@ TEST_F(StorageCursorTest, PrevPublic) { } TEST_F(StorageCursorTest, CurrentPublic) { + EXPECT_EQ(getTableCursorsSize(), 1); std::pair element = cursor.first(); std::map::const_iterator reference = data.begin(); @@ -259,7 +312,73 @@ TEST_F(StorageCursorTest, CurrentPublic) { EXPECT_EQ(element.second, reference->second); } +TEST_F(StorageCursorTest, SettingPublic) { + EXPECT_EQ(getTableCursorsSize(), 1); + + EXPECT_FALSE(cursor.set(557)); + EXPECT_EQ(cursor.current().second, "resilent pick forefront"); + + std::map::const_iterator reference = data.begin(); + std::advance(reference, 3); + EXPECT_TRUE(cursor.set(reference->first)); + EXPECT_EQ(cursor.current().second, reference->second); + + ++reference; + cursor.next(); + EXPECT_EQ(cursor.current().second, reference->second); + + ++reference; + cursor.next(); + EXPECT_EQ(cursor.current().second, reference->second); + + --reference; + cursor.prev(); + EXPECT_EQ(cursor.current().second, reference->second); + + --reference; + cursor.prev(); + EXPECT_EQ(cursor.current().second, reference->second); + + --reference; + cursor.prev(); + EXPECT_EQ(cursor.current().second, reference->second); +} + +TEST_F(StorageCursorTest, CursorRAIIBehaviour) { + int initialiCursorsAmount = getTableCursorsSize(); + { + LMDBAL::Cursor cur = table->createCursor(); + EXPECT_EQ(initialiCursorsAmount + 1, getTableCursorsSize()); + cur.open(transaction); + EXPECT_NO_THROW(cur.first()); + } + + EXPECT_EQ(initialiCursorsAmount, getTableCursorsSize()); + LMDBAL::Cursor cur; + EXPECT_EQ(initialiCursorsAmount, getTableCursorsSize()); + EXPECT_EQ(cur.empty(), true); + cur = table->createCursor(); + EXPECT_EQ(cur.empty(), false); + EXPECT_EQ(initialiCursorsAmount + 1, getTableCursorsSize()); + EXPECT_THROW(emptyTable->destroyCursor(cur), LMDBAL::Unknown); + table->destroyCursor(cur); + EXPECT_EQ(cur.empty(), true); + EXPECT_EQ(initialiCursorsAmount, getTableCursorsSize()); + + cur = table->createCursor(); + EXPECT_EQ(initialiCursorsAmount + 1, getTableCursorsSize()); + EXPECT_EQ(cur.empty(), false); + + cur.drop(); + EXPECT_EQ(cur.empty(), true); + EXPECT_EQ(initialiCursorsAmount, getTableCursorsSize()); + + EXPECT_NO_THROW(cur.drop()); + EXPECT_THROW(table->destroyCursor(cur), LMDBAL::Unknown); +} + TEST_F(StorageCursorTest, CornerCases) { + EXPECT_EQ(getTableCursorsSize(), 1); transaction.terminate(); EXPECT_THROW(cursor.current(), LMDBAL::Unknown); cursor.close();