Sqlite: Add unoptional value fetcher

There is now value, valueWithTransaction, optionalValue and
optionalValueWithTransaction. Sometimes you want to return
the default constructed value as default. So this is now possible.
This simplifies some code but can be even more optimal.

Change-Id: Ibbb6b5ca47803344ed646bfa257602b6db52e0fe
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

src/libs/clangsupport/filepathstorage.h

@@ -87,7 +87,7 @@ public:
     {
         auto &statement = m_statementFactory.selectDirectoryIdFromDirectoriesByDirectoryPath;
 
-        return statement.template value<int>(directoryPath);
+        return statement.template optionalValue<int>(directoryPath);
     }
 
     int writeDirectoryId(Utils::SmallStringView directoryPath)
@@ -106,7 +106,7 @@ public:
 
             auto &statement = m_statementFactory.selectDirectoryPathFromDirectoriesByDirectoryId;
 
-            auto optionalDirectoryPath = statement.template value<Utils::PathString>(directoryPathId);
+            auto optionalDirectoryPath = statement.template optionalValue<Utils::PathString>(directoryPathId);
 
             if (!optionalDirectoryPath)
                 throw DirectoryPathIdDoesNotExists();
@@ -176,7 +176,7 @@ public:
     {
         auto &statement = m_statementFactory.selectSourceIdFromSourcesByDirectoryIdAndSourceName;
 
-        return statement.template value<int>(directoryId, sourceName);
+        return statement.template optionalValue<int>(directoryId, sourceName);
     }
 
     Sources::SourceNameAndDirectoryId fetchSourceNameAndDirectoryId(int sourceId)
@@ -186,7 +186,7 @@ public:
 
             auto &statement = m_statementFactory.selectSourceNameAndDirectoryIdFromSourcesBySourceId;
 
-            auto optionalSourceName = statement.template value<Sources::SourceNameAndDirectoryId>(
+            auto optionalSourceName = statement.template optionalValue<Sources::SourceNameAndDirectoryId>(
                 sourceId);
 
             if (!optionalSourceName)
@@ -207,7 +207,7 @@ public:
 
             auto &statement = m_statementFactory.selectDirectoryIdFromSourcesBySourceId;
 
-            auto optionalDirectoryId = statement.template value<int>(sourceId);
+            auto optionalDirectoryId = statement.template optionalValue<int>(sourceId);
 
             if (!optionalDirectoryId)
                 throw SourceNameIdDoesNotExists();

src/libs/clangsupport/projectpartsstorage.h

@@ -77,7 +77,7 @@ public:
 
     bool preCompiledHeaderWasGenerated(ProjectPartId projectPartId) const
     {
-        auto value = fetchProjectPrecompiledHeaderBuildTimeStatement.template value<long long>(
+        auto value = fetchProjectPrecompiledHeaderBuildTimeStatement.template optionalValue<long long>(
             projectPartId.projectPathId);
 
         return value && *value > 0;
@@ -92,7 +92,7 @@ public:
             Sqlite::DeferredTransaction transaction{database};
 
             for (ProjectPartId projectPartId : projectPartIds) {
-                auto value = fetchProjectPartByIdStatement.template value<ProjectPartContainer>(
+                auto value = fetchProjectPartByIdStatement.template optionalValue<ProjectPartContainer>(
                     projectPartId.projectPathId);
                 if (value) {
                     value->headerPathIds = fetchHeaders(projectPartId);
@@ -112,7 +112,7 @@ public:
 
     ProjectPartId fetchProjectPartIdUnguarded(Utils::SmallStringView projectPartName) const override
     {
-        auto optionalProjectPartId = fetchProjectPartIdStatement.template value<ProjectPartId>(
+        auto optionalProjectPartId = fetchProjectPartIdStatement.template optionalValue<ProjectPartId>(
             projectPartName);
 
         if (optionalProjectPartId) {
@@ -147,8 +147,9 @@ public:
         try {
             Sqlite::DeferredTransaction transaction{database};
 
-            auto optionalProjectPartName = fetchProjectPartNameStatement.template value<Utils::PathString>(
+            auto optionalProjectPartName = fetchProjectPartNameStatement
-                projectPartId.projectPathId);
+                                               .template optionalValue<Utils::PathString>(
+                                                   projectPartId.projectPathId);
 
             transaction.commit();
 
@@ -246,7 +247,7 @@ public:
 
             auto &statement = getProjectPartArtefactsBySourceId;
 
-            auto value = statement.template value<ProjectPartArtefact>(sourceId.filePathId);
+            auto value = statement.template optionalValue<ProjectPartArtefact>(sourceId.filePathId);
 
             transaction.commit();
 
@@ -263,7 +264,8 @@ public:
 
             auto &statement = getProjectPartArtefactsByProjectPartId;
 
-            auto value = statement.template value<ProjectPartArtefact>(projectPartId.projectPathId);
+            auto value = statement.template optionalValue<ProjectPartArtefact>(
+                projectPartId.projectPathId);
 
             transaction.commit();
 

src/libs/sqlite/sqlitebasestatement.h

@@ -216,6 +216,22 @@ public:
 
     template<typename ResultType, typename... QueryTypes>
     auto value(const QueryTypes &...queryValues)
+    {
+        Resetter resetter{this};
+        ResultType resultValue;
+
+        bindValues(queryValues...);
+
+        if (BaseStatement::next())
+            resultValue = createValue<ResultType>();
+
+        resetter.reset();
+
+        return resultValue;
+    }
+
+    template<typename ResultType, typename... QueryTypes>
+    auto optionalValue(const QueryTypes &...queryValues)
     {
         Resetter resetter{this};
         Utils::optional<ResultType> resultValue;

src/libs/sqlite/sqlitereadstatement.h

@@ -42,6 +42,7 @@ public:
         Base::checkColumnCount(ResultCount);
     }
 
+    using Base::optionalValue;
     using Base::range;
     using Base::rangeWithTransaction;
     using Base::readCallback;
@@ -62,6 +63,18 @@ public:
         return resultValue;
     }
 
+    template<typename ResultType, typename... QueryTypes>
+    auto optionalValueWithTransaction(const QueryTypes &...queryValues)
+    {
+        DeferredTransaction transaction{Base::database()};
+
+        auto resultValue = Base::template optionalValue<ResultType>(queryValues...);
+
+        transaction.commit();
+
+        return resultValue;
+    }
+
     template<typename ResultType, typename... QueryTypes>
     auto valuesWithTransaction(std::size_t reserveSize, const QueryTypes &...queryValues)
     {

src/libs/sqlite/sqlitereadwritestatement.h

@@ -43,6 +43,7 @@ public:
     }
 
     using Base::execute;
+    using Base::optionalValue;
     using Base::readCallback;
     using Base::readTo;
     using Base::toValue;
@@ -62,6 +63,18 @@ public:
         return resultValue;
     }
 
+    template<typename ResultType, typename... QueryTypes>
+    auto optionalValueWithTransaction(const QueryTypes &...queryValues)
+    {
+        ImmediateTransaction transaction{Base::database()};
+
+        auto resultValue = Base::template optionalValue<ResultType>(queryValues...);
+
+        transaction.commit();
+
+        return resultValue;
+    }
+
     template<typename ResultType, typename... QueryTypes>
     auto valuesWithTransaction(std::size_t reserveSize, const QueryTypes &...queryValues)
     {

src/plugins/clangrefactoring/symbolquery.h

@@ -152,7 +152,7 @@ public:
     {
         auto &statement = m_statementFactory.selectLocationOfSymbol;
 
-        return statement.template value<SourceLocation>(symbolId, int(kind));
+        return statement.template optionalValue<SourceLocation>(symbolId, int(kind));
     }
 private:
     StatementFactory &m_statementFactory;

src/plugins/qmldesigner/designercore/imagecache/imagecachestorage.h

@@ -58,7 +58,7 @@ public:
         try {
             Sqlite::DeferredTransaction transaction{database};
 
-            auto optionalBlob = selectImageStatement.template value<Sqlite::ByteArrayBlob>(
+            auto optionalBlob = selectImageStatement.template optionalValue<Sqlite::ByteArrayBlob>(
                 name, minimumTimeStamp.value);
 
             transaction.commit();
@@ -78,7 +78,7 @@ public:
         try {
             Sqlite::DeferredTransaction transaction{database};
 
-            auto optionalBlob = selectSmallImageStatement.template value<Sqlite::ByteArrayBlob>(
+            auto optionalBlob = selectSmallImageStatement.template optionalValue<Sqlite::ByteArrayBlob>(
                 name, minimumTimeStamp.value);
 
             transaction.commit();
@@ -98,7 +98,7 @@ public:
         try {
             Sqlite::DeferredTransaction transaction{database};
 
-            auto optionalBlob = selectIconStatement.template value<Sqlite::ByteArrayBlob>(
+            auto optionalBlob = selectIconStatement.template optionalValue<Sqlite::ByteArrayBlob>(
                 name, minimumTimeStamp.value);
 
             transaction.commit();

src/tools/clangpchmanagerbackend/source/builddependenciesstorage.h

@@ -102,7 +102,7 @@ public:
     {
         auto &statement = getLowestLastModifiedTimeOfDependencies;
 
-        return statement.template value<long long>(sourceId.filePathId).value_or(0);
+        return statement.template optionalValue<long long>(sourceId.filePathId).value_or(0);
     }
 
     void insertOrUpdateUsedMacros(const UsedMacros &usedMacros) override
@@ -131,7 +131,8 @@ public:
 
     ProjectPartId fetchProjectPartId(Utils::SmallStringView projectPartName) override
     {
-        auto projectPartId = fetchProjectPartIdStatement.template value<ProjectPartId>(projectPartName);
+        auto projectPartId = fetchProjectPartIdStatement.template optionalValue<ProjectPartId>(
+            projectPartName);
 
         if (projectPartId)
             return *projectPartId;

src/tools/clangpchmanagerbackend/source/precompiledheaderstorage.h

@@ -145,7 +145,7 @@ public:
         try {
             Sqlite::DeferredTransaction transaction{database};
 
-            auto value = fetchSystemPrecompiledHeaderPathStatement.template value<FilePath>(
+            auto value = fetchSystemPrecompiledHeaderPathStatement.template optionalValue<FilePath>(
                 projectPartId.projectPathId);
 
             transaction.commit();
@@ -165,7 +165,7 @@ public:
         try {
             Sqlite::DeferredTransaction transaction{database};
 
-            auto value = fetchPrecompiledHeaderStatement.template value<FilePath>(
+            auto value = fetchPrecompiledHeaderStatement.template optionalValue<FilePath>(
                 projectPartId.projectPathId);
 
             transaction.commit();
@@ -185,7 +185,7 @@ public:
         try {
             Sqlite::DeferredTransaction transaction{database};
 
-            auto value = fetchPrecompiledHeadersStatement.template value<PchPaths>(
+            auto value = fetchPrecompiledHeadersStatement.template optionalValue<PchPaths>(
                 projectPartId.projectPathId);
 
             transaction.commit();
@@ -205,7 +205,7 @@ public:
         try {
             Sqlite::DeferredTransaction transaction{database};
 
-            auto value = fetchTimeStampsStatement.template value<PrecompiledHeaderTimeStamps>(
+            auto value = fetchTimeStampsStatement.template optionalValue<PrecompiledHeaderTimeStamps>(
                 projectPartId.projectPathId);
 
             transaction.commit();

tests/unit/unittest/sqlitereadstatementmock.h

@@ -161,7 +161,7 @@ public:
                 (int projectPartId));
 
     template<typename ResultType, typename... QueryTypes>
-    auto value(const QueryTypes &...queryValues)
+    auto optionalValue(const QueryTypes &...queryValues)
     {
         if constexpr (std::is_same_v<ResultType, Sqlite::ByteArrayBlob>)
             return valueReturnBlob(queryValues...);
@@ -196,6 +196,19 @@ public:
                           "SqliteReadStatementMock::value does not handle result type!");
     }
 
+    template<typename ResultType, typename... QueryTypes>
+    auto value(const QueryTypes &...queryValues)
+    {
+        static_assert(!std::is_same_v<ResultType, ResultType>,
+                      "SqliteReadStatementMock::value does not handle result type!");
+    }
+
+    template<typename ResultType, typename... QueryTypes>
+    auto optionalValueWithTransaction(const QueryTypes &...queryValues)
+    {
+        return optionalValue<ResultType>(queryValues...);
+    }
+
     template<typename ResultType, typename... QueryType>
     auto values(std::size_t reserveSize, const QueryType &...queryValues)
     {

tests/unit/unittest/sqlitestatement-test.cpp

@@ -133,8 +133,11 @@ protected:
 
 struct Output
 {
-    Output(Utils::SmallStringView name, Utils::SmallStringView number, long long value)
+    explicit Output() = default;
-        : name(name), number(number), value(value)
+    explicit Output(Utils::SmallStringView name, Utils::SmallStringView number, long long value)
+        : name(name)
+        , number(number)
+        , value(value)
     {}
 
     Utils::SmallString name;
@@ -559,7 +562,7 @@ TEST_F(SqliteStatement, WriteSqliteBlobValue)
 
     statement.write(Sqlite::Value{bytes});
 
-    ASSERT_THAT(readStatement.template value<Sqlite::Blob>(),
+    ASSERT_THAT(readStatement.template optionalValue<Sqlite::Blob>(),
                 Optional(Field(&Sqlite::Blob::bytes, Eq(bytes))));
 }
 
@@ -611,7 +614,7 @@ TEST_F(SqliteStatement, WriteSqliteBlobValueView)
 
     statement.write(Sqlite::ValueView::create(bytes));
 
-    ASSERT_THAT(readStatement.template value<Sqlite::Blob>(),
+    ASSERT_THAT(readStatement.template optionalValue<Sqlite::Blob>(),
                 Optional(Field(&Sqlite::Blob::bytes, Eq(bytes))));
 }
 
@@ -648,7 +651,7 @@ TEST_F(SqliteStatement, WriteBlobs)
 
     statement.write(bytes);
 
-    ASSERT_THAT(readStatement.template value<Sqlite::Blob>(),
+    ASSERT_THAT(readStatement.template optionalValue<Sqlite::Blob>(),
                 Optional(Field(&Sqlite::Blob::bytes, Eq(bytes))));
 }
 
@@ -1049,29 +1052,29 @@ TEST_F(SqliteStatement, GetBlobValues)
     ASSERT_THAT(values, ElementsAre(Field(&Sqlite::Blob::bytes, Eq(bytes))));
 }
 
-TEST_F(SqliteStatement, GetEmptyBlobValueForInteger)
+TEST_F(SqliteStatement, GetEmptyOptionalBlobValueForInteger)
 {
     ReadStatement<1> statement("SELECT value FROM test WHERE name='poo'", database);
 
-    auto value = statement.value<Sqlite::Blob>();
+    auto value = statement.optionalValue<Sqlite::Blob>();
 
     ASSERT_THAT(value, Optional(Field(&Sqlite::Blob::bytes, IsEmpty())));
 }
 
-TEST_F(SqliteStatement, GetEmptyBlobValueForFloat)
+TEST_F(SqliteStatement, GetEmptyOptionalBlobValueForFloat)
 {
     ReadStatement<1> statement("SELECT number FROM test WHERE name='foo'", database);
 
-    auto value = statement.value<Sqlite::Blob>();
+    auto value = statement.optionalValue<Sqlite::Blob>();
 
     ASSERT_THAT(value, Optional(Field(&Sqlite::Blob::bytes, IsEmpty())));
 }
 
-TEST_F(SqliteStatement, GetEmptyBlobValueForText)
+TEST_F(SqliteStatement, GetEmptyOptionalBlobValueForText)
 {
     ReadStatement<1> statement("SELECT number FROM test WHERE name='bar'", database);
 
-    auto value = statement.value<Sqlite::Blob>();
+    auto value = statement.optionalValue<Sqlite::Blob>();
 
     ASSERT_THAT(value, Optional(Field(&Sqlite::Blob::bytes, IsEmpty())));
 }
@@ -1081,7 +1084,7 @@ TEST_F(SqliteStatement, GetOptionalSingleValueAndMultipleQueryValue)
     ReadStatement<1> statement("SELECT name FROM test WHERE name=? AND number=? AND value=?",
                                database);
 
-    auto value = statement.value<Utils::SmallString>("bar", "blah", 1);
+    auto value = statement.optionalValue<Utils::SmallString>("bar", "blah", 1);
 
     ASSERT_THAT(value.value(), Eq("bar"));
 }
@@ -1091,7 +1094,7 @@ TEST_F(SqliteStatement, GetOptionalOutputValueAndMultipleQueryValue)
     ReadStatement<3> statement(
         "SELECT name, number, value FROM test WHERE name=? AND number=? AND value=?", database);
 
-    auto value = statement.value<Output>("bar", "blah", 1);
+    auto value = statement.optionalValue<Output>("bar", "blah", 1);
 
     ASSERT_THAT(value.value(), Eq(Output{"bar", "blah", 1}));
 }
@@ -1102,7 +1105,7 @@ TEST_F(SqliteStatement, GetOptionalTupleValueAndMultipleQueryValue)
     ReadStatement<3> statement(
         "SELECT name, number, value FROM test WHERE name=? AND number=? AND value=?", database);
 
-    auto value = statement.value<Tuple>("bar", "blah", 1);
+    auto value = statement.optionalValue<Tuple>("bar", "blah", 1);
 
     ASSERT_THAT(value.value(), Eq(Tuple{"bar", "blah", 1}));
 }
@@ -1113,7 +1116,7 @@ TEST_F(SqliteStatement, GetOptionalValueCallsReset)
 
     EXPECT_CALL(mockStatement, reset());
 
-    mockStatement.value<int>("bar");
+    mockStatement.optionalValue<int>("bar");
 }
 
 TEST_F(SqliteStatement, GetOptionalValueCallsResetIfExceptionIsThrown)
@@ -1123,6 +1126,56 @@ TEST_F(SqliteStatement, GetOptionalValueCallsResetIfExceptionIsThrown)
 
     EXPECT_CALL(mockStatement, reset());
 
+    EXPECT_THROW(mockStatement.optionalValue<int>("bar"), Sqlite::StatementHasError);
+}
+
+TEST_F(SqliteStatement, GetSingleValueAndMultipleQueryValue)
+{
+    ReadStatement<1> statement("SELECT name FROM test WHERE name=? AND number=? AND value=?",
+                               database);
+
+    auto value = statement.value<Utils::SmallString>("bar", "blah", 1);
+
+    ASSERT_THAT(value, Eq("bar"));
+}
+
+TEST_F(SqliteStatement, GetOutputValueAndMultipleQueryValue)
+{
+    ReadStatement<3> statement(
+        "SELECT name, number, value FROM test WHERE name=? AND number=? AND value=?", database);
+
+    auto value = statement.value<Output>("bar", "blah", 1);
+
+    ASSERT_THAT(value, Eq(Output{"bar", "blah", 1}));
+}
+
+TEST_F(SqliteStatement, GetTupleValueAndMultipleQueryValue)
+{
+    using Tuple = std::tuple<Utils::SmallString, Utils::SmallString, long long>;
+    ReadStatement<3> statement(
+        "SELECT name, number, value FROM test WHERE name=? AND number=? AND value=?", database);
+
+    auto value = statement.value<Tuple>("bar", "blah", 1);
+
+    ASSERT_THAT(value, Eq(Tuple{"bar", "blah", 1}));
+}
+
+TEST_F(SqliteStatement, GetValueCallsReset)
+{
+    MockSqliteStatement mockStatement{databaseMock};
+
+    EXPECT_CALL(mockStatement, reset());
+
+    mockStatement.value<int>("bar");
+}
+
+TEST_F(SqliteStatement, GetValueCallsResetIfExceptionIsThrown)
+{
+    MockSqliteStatement mockStatement{databaseMock};
+    ON_CALL(mockStatement, next()).WillByDefault(Throw(Sqlite::StatementHasError("")));
+
+    EXPECT_CALL(mockStatement, reset());
+
     EXPECT_THROW(mockStatement.value<int>("bar"), Sqlite::StatementHasError);
 }
 
@@ -1402,6 +1455,19 @@ TEST_F(SqliteStatement, ReadStatementValueWithTransactions)
 
     auto value = statement.valueWithTransaction<Tuple>("bar", "blah");
 
+    ASSERT_THAT(value, Eq(Tuple{"bar", "blah", 1}));
+    database.lock();
+}
+
+TEST_F(SqliteStatement, ReadStatementOptionalValueWithTransactions)
+{
+    using Tuple = std::tuple<Utils::SmallString, Utils::SmallString, long long>;
+    ReadStatement<3> statement("SELECT name, number, value FROM test WHERE name=? AND number=?",
+                               database);
+    database.unlock();
+
+    auto value = statement.optionalValueWithTransaction<Tuple>("bar", "blah");
+
     ASSERT_THAT(*value, Eq(Tuple{"bar", "blah", 1}));
     database.lock();
 }
@@ -1456,6 +1522,19 @@ TEST_F(SqliteStatement, ReadWriteStatementValueWithTransactions)
 
     auto value = statement.valueWithTransaction<Tuple>("bar", "blah");
 
+    ASSERT_THAT(value, Eq(Tuple{"bar", "blah", 1}));
+    database.lock();
+}
+
+TEST_F(SqliteStatement, ReadWriteStatementOptionalValueWithTransactions)
+{
+    using Tuple = std::tuple<Utils::SmallString, Utils::SmallString, long long>;
+    ReadWriteStatement<3> statement(
+        "SELECT name, number, value FROM test WHERE name=? AND number=?", database);
+    database.unlock();
+
+    auto value = statement.optionalValueWithTransaction<Tuple>("bar", "blah");
+
     ASSERT_THAT(*value, Eq(Tuple{"bar", "blah", 1}));
     database.lock();
 }