Clang: Fix reset for write and execute in the sqlite statement

We were not reset a statement if we got an exception. There are now test
for it.

Change-Id: Ife7b4437fece9369767605ba7387bd0564c1bb8d
Reviewed-by: Ivan Donchevskii <ivan.donchevskii@qt.io>

src/libs/sqlite/sqlitebasestatement.cpp

@@ -54,7 +54,6 @@ BaseStatement::BaseStatement(Utils::SmallStringView sqlStatement, Database &data
 
 void BaseStatement::deleteCompiledStatement(sqlite3_stmt *compiledStatement)
 {
-    if (compiledStatement)
     sqlite3_finalize(compiledStatement);
 }
 
@@ -145,12 +144,6 @@ void BaseStatement::step() const
     next();
 }
 
-void BaseStatement::execute() const
-{
-    next();
-    reset();
-}
-
 int BaseStatement::columnCount() const
 {
     return m_columnCount;

src/libs/sqlite/sqlitebasestatement.h

@@ -60,7 +60,6 @@ public:
 
     bool next() const;
     void step() const;
-    void execute() const;
     void reset() const;
 
     int fetchIntValue(int column) const;
@@ -159,6 +158,13 @@ class StatementImplementation : public BaseStatement
 public:
     using BaseStatement::BaseStatement;
 
+    void execute()
+    {
+        Resetter resetter{*this};
+        BaseStatement::next();
+        resetter.reset();
+    }
+
     void bindValues()
     {
     }
@@ -172,8 +178,10 @@ public:
     template<typename... ValueType>
     void write(const ValueType&... values)
     {
+        Resetter resetter{*this};
         bindValuesByIndex(1, values...);
-        BaseStatement::execute();
+        BaseStatement::next();
+        resetter.reset();
     }
 
     template<typename... ValueType>
@@ -185,8 +193,10 @@ public:
     template<typename... ValueType>
     void writeNamed(const ValueType&... values)
     {
+        Resetter resetter{*this};
         bindValuesByName(values...);
-        BaseStatement::execute();
+        BaseStatement::next();
+        resetter.reset();
     }
 
     template <typename ResultType,

src/libs/sqlite/sqlitedatabasebackend.cpp

@@ -178,7 +178,7 @@ void DatabaseBackend::setLastInsertedRowId(int64_t rowId)
 void DatabaseBackend::execute(Utils::SmallStringView sqlStatement)
 {
     ReadWriteStatement statement(sqlStatement, m_database);
-    statement.step();
+    statement.execute();
 }
 
 void DatabaseBackend::close()

src/libs/sqlite/sqlitetransaction.h

@@ -64,6 +64,7 @@ public:
     {
         m_interface.commit();
         m_isAlreadyCommited = true;
+        m_locker.unlock();
     }
 
 protected:
@@ -76,7 +77,7 @@ protected:
 
 protected:
     TransactionInterface &m_interface;
-    std::lock_guard<TransactionInterface> m_locker{m_interface};
+    std::unique_lock<TransactionInterface> m_locker{m_interface};
     bool m_isAlreadyCommited = false;
     bool m_rollback = false;
 };

tests/unit/unittest/mocksqlitestatement.h

@@ -35,7 +35,6 @@ class BaseMockSqliteStatement
 public:
     MOCK_METHOD0(next, bool ());
     MOCK_METHOD0(step, void ());
-    MOCK_METHOD0(execute, void ());
     MOCK_METHOD0(reset, void ());
 
     MOCK_CONST_METHOD1(fetchIntValue, int (int));
@@ -53,6 +52,7 @@ public:
     MOCK_METHOD2(bind, void (int, double));
     MOCK_METHOD2(bind, void (int, Utils::SmallStringView));
     MOCK_METHOD2(bind, void (int, long));
+    MOCK_CONST_METHOD1(bindingIndexForName, int (Utils::SmallStringView name));
 
     MOCK_METHOD1(prepare, void (Utils::SmallStringView sqlStatement));
 };

tests/unit/unittest/sqlitestatement-test.cpp

@@ -639,6 +639,40 @@ TEST_F(SqliteStatement, ThrowExceptionOnlyInReset)
                  Sqlite::StatementHasError);
 }
 
+TEST_F(SqliteStatement, ResetIfWriteIsThrowingException)
+{
+    MockSqliteStatement mockStatement;
+
+    EXPECT_CALL(mockStatement, bind(1, TypedEq<Utils::SmallStringView>("bar")))
+            .WillOnce(Throw(Sqlite::StatementIsBusy("")));
+    EXPECT_CALL(mockStatement, reset());
+
+    ASSERT_ANY_THROW(mockStatement.write("bar"));
+}
+
+TEST_F(SqliteStatement, ResetIfWriteNamedIsThrowingException)
+{
+    MockSqliteStatement mockStatement;
+
+    EXPECT_CALL(mockStatement, bindingIndexForName(TypedEq<Utils::SmallStringView>("@foo")))
+            .WillOnce(Return(1));
+    EXPECT_CALL(mockStatement, bind(1, TypedEq<Utils::SmallStringView>("bar")))
+            .WillOnce(Throw(Sqlite::StatementIsBusy("")));
+    EXPECT_CALL(mockStatement, reset());
+
+    ASSERT_ANY_THROW(mockStatement.writeNamed("@foo", "bar"));
+}
+
+TEST_F(SqliteStatement, ResetIfExecuteThrowsException)
+{
+    MockSqliteStatement mockStatement;
+
+    EXPECT_CALL(mockStatement, next()).WillOnce(Throw(Sqlite::StatementIsBusy("")));
+    EXPECT_CALL(mockStatement, reset());
+
+    ASSERT_ANY_THROW(mockStatement.execute());
+}
+
 void SqliteStatement::SetUp()
 {
     database.execute("CREATE TABLE test(name TEXT UNIQUE, number NUMERIC, value NUMERIC)");