Some reformation, test cases and first bugfixes

src/cursor.hpp

@@ -195,12 +195,10 @@ void LMDBAL::Cursor<K, V>::terminated () {
     switch (state) {
         case openedPublic:
             storage->_mdbCursorClose(cursor);
-
             state = closed;
             break;
         case openedPrivate:
-            storage->closeCursorTransaction(cursor);
+            storage->closeCursorTransaction(cursor, true);
-
             state = closed;
             break;
         default:
@@ -228,7 +226,7 @@ void LMDBAL::Cursor<K, V>::open () {
 
     switch (state) {
         case closed:
-            storage->openCursorTransaction(&cursor);
+            storage->openCursorTransaction(&cursor, false);
             state = openedPrivate;
         break;
         default:
@@ -297,30 +295,15 @@ void LMDBAL::Cursor<K, V>::renew () {
 
     storage->ensureOpened(renewCursorMethodName);
     switch (state) {
-        case openedPrivate: {
+        case openedPrivate:
-            TransactionID txn = storage->_mdbCursorTxn(cursor);
+            storage->closeCursorTransaction(cursor, false);
-            storage->abortTransaction(txn);
+            storage->openCursorTransaction(&cursor, true);
-            storage->transactionAborted(txn);
+            break;
-
+        case openedPublic:
-            txn = storage->beginReadOnlyTransaction();
-            int result = storage->_mdbCursorRenew(txn, cursor);
-            if (result != MDB_SUCCESS)
-                storage->throwUnknown(result, txn);
-
-            storage->transactionStarted(txn, true);
-            state = openedPrivate;
-        }
-        case openedPublic: {
             storage->disconnectCursorFromTransaction(id, cursor);
-            TransactionID txn = storage->beginReadOnlyTransaction();
+            storage->openCursorTransaction(&cursor, true);
-            int result = storage->_mdbCursorRenew(txn, cursor);
-            if (result != MDB_SUCCESS)
-                storage->throwUnknown(result, txn);
-
-            storage->transactionStarted(txn, true);
-            storage->attachCursorToTransaction(id, cursor, this);
             state = openedPrivate;
-        } break;
+            break;
         default:
             break;
     }
@@ -355,16 +338,14 @@ void LMDBAL::Cursor<K, V>::renew (const Transaction& transaction) {
     TransactionID txn = storage->extractTransactionId(transaction, renewCursorMethodName);
     switch (state) {
         case openedPrivate: {
-            TransactionID txnOld = storage->_mdbCursorTxn(cursor);
+            storage->closeCursorTransaction(cursor, false);
-            storage->abortTransaction(txnOld);
-            storage->transactionAborted(txnOld);
-
             int result = storage->_mdbCursorRenew(txn, cursor);
             if (result != MDB_SUCCESS)
                 storage->throwUnknown(result);
 
+            storage->attachCursorToTransaction(id, cursor, this);
             state = openedPublic;
-        }
+        } break;
         case openedPublic: {
             storage->disconnectCursorFromTransaction(id, cursor);
             int result = storage->_mdbCursorRenew(txn, cursor);
@@ -372,7 +353,6 @@ void LMDBAL::Cursor<K, V>::renew (const Transaction& transaction) {
                 storage->throwUnknown(result);
 
             storage->attachCursorToTransaction(id, cursor, this);
-            state = openedPublic;
         } break;
         default:
             break;
@@ -399,7 +379,7 @@ void LMDBAL::Cursor<K, V>::close () {
             state = closed;
         break;
         case openedPrivate:
-            storage->closeCursorTransaction(cursor);
+            storage->closeCursorTransaction(cursor, true);
 
             state = closed;
         break;

src/storage.cpp

@@ -181,15 +181,21 @@ void LMDBAL::iStorage::attachCursorToTransaction (uint32_t cursorId, MDB_cursor*
  * This method is ment to be called from LMDBAL::Cursor
  *
  * \param[out] cursor   - cursor handle
+ * \param[in]  renew    - true if instead of opening cursor should be renewed
  *
  * \exception LMDBAL::Closed        thrown if you try to open the cursor on a closed database
  * \exception LMDBAL::Unknown       thrown if there was a problem opening the cursor by the lmdb, or to begin a transaction
  */
-void LMDBAL::iStorage::openCursorTransaction (MDB_cursor** cursor) const {
+void LMDBAL::iStorage::openCursorTransaction (MDB_cursor** cursor, bool renew) const {
     ensureOpened(openCursorTransactionMethodName);
 
     TransactionID txn = beginReadOnlyTransaction();
-    int result = _mdbCursorOpen(txn, cursor);
+    int result;
+    if (renew)
+        result = _mdbCursorRenew(txn, *cursor);
+    else
+        result = _mdbCursorOpen(txn, cursor);
+
     if (result != MDB_SUCCESS)
         throwUnknown(result, txn);
 
@@ -202,10 +208,13 @@ void LMDBAL::iStorage::openCursorTransaction (MDB_cursor** cursor) const {
  * This method is ment to be called from LMDBAL::Cursor
  *
  * \param[in] cursor        - cursor handle
+ * \param[in] closeCursor   - true if the cursor should also get closed, false if you wish to leave it open
  */
-void LMDBAL::iStorage::closeCursorTransaction (MDB_cursor* cursor) const {
+void LMDBAL::iStorage::closeCursorTransaction (MDB_cursor* cursor, bool closeCursor) const {
     TransactionID txn = _mdbCursorTxn(cursor);
+    if (closeCursor)
         _mdbCursorClose(cursor);
+
     abortTransaction(txn);
     transactionAborted(txn);
 }

src/storage.h

@@ -76,8 +76,8 @@ protected:
     virtual int drop(TransactionID transaction);
     virtual SizeType count(TransactionID txn) const;
 
-    void openCursorTransaction(MDB_cursor** cursor) const;
+    void openCursorTransaction(MDB_cursor** cursor, bool renew = false) const;
-    void closeCursorTransaction(MDB_cursor* cursor) const;
+    void closeCursorTransaction(MDB_cursor* cursor, bool closeCursor = false) const;
     void attachCursorToTransaction(uint32_t cursorId, MDB_cursor* cursorHandle, iCursor* pointer) const;
     void disconnectCursorFromTransaction(uint32_t cursorId, MDB_cursor* cursorHandle) const;
 

test/cachecursor.cpp

@@ -436,5 +436,40 @@ TEST_F(CacheCursorTest, CornerCases) {
     EXPECT_EQ(element.first, reference->first);
     EXPECT_EQ(element.second, reference->second);
     EXPECT_THROW(cursor.prev(), LMDBAL::NotFound);
+
+    cursor.close();
 }
 
+TEST_F(CacheCursorTest, TerminatedTransaction) {
+    LMDBAL::Cursor<uint64_t, std::string> cr = cache->createCursor();
+
+    {
+        LMDBAL::Transaction txn = db->beginReadOnlyTransaction();
+        cr.open(txn);
+        EXPECT_NO_THROW(cr.first());
+    }
+
+    EXPECT_FALSE(cr.opened());
+
+    LMDBAL::Transaction txn2;
+    {
+        LMDBAL::Transaction txn = db->beginReadOnlyTransaction();
+        EXPECT_TRUE(txn.isActive());
+        EXPECT_FALSE(txn2.isActive());
+
+        cr.open(txn);
+        EXPECT_TRUE(cr.opened());
+
+        txn2 = std::move(txn);
+        EXPECT_FALSE(txn.isActive());
+        EXPECT_TRUE(txn2.isActive());
+        EXPECT_TRUE(cr.opened());
+    }
+
+    EXPECT_TRUE(txn2.isActive());
+    EXPECT_TRUE(cr.opened());
+
+    txn2.terminate();
+    EXPECT_FALSE(txn2.isActive());
+    EXPECT_FALSE(cr.opened());
+}

test/storagecursor.cpp

@@ -419,13 +419,35 @@ TEST_F(StorageCursorTest, CornerCases) {
 }
 
 TEST_F(StorageCursorTest, TerminatedTransaction) {
-    LMDBAL::Cursor<uint64_t, std::string> emptyCursor = table->createCursor();
+    LMDBAL::Cursor<uint64_t, std::string> cr = table->createCursor();
 
     {
         LMDBAL::Transaction txn = db->beginReadOnlyTransaction();
-        cursor.open(txn);
+        cr.open(txn);
-        EXPECT_NO_THROW(cursor.first());
+        EXPECT_NO_THROW(cr.first());
     }
 
-    EXPECT_FALSE(cursor.opened());
+    EXPECT_FALSE(cr.opened());
+
+    LMDBAL::Transaction txn2;
+    {
+        LMDBAL::Transaction txn = db->beginReadOnlyTransaction();
+        EXPECT_TRUE(txn.isActive());
+        EXPECT_FALSE(txn2.isActive());
+
+        cr.open(txn);
+        EXPECT_TRUE(cr.opened());
+
+        txn2 = std::move(txn);
+        EXPECT_FALSE(txn.isActive());
+        EXPECT_TRUE(txn2.isActive());
+        EXPECT_TRUE(cr.opened());
+    }
+
+    EXPECT_TRUE(txn2.isActive());
+    EXPECT_TRUE(cr.opened());
+
+    txn2.terminate();
+    EXPECT_FALSE(txn2.isActive());
+    EXPECT_FALSE(cr.opened());
 }