Address review comments

SOCI · Sep 1, 2024 · 39e7a10 · 39e7a10
1 parent ea1739e
commit 39e7a10
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 69 deletions.
diff --git a/include/soci/logger.h b/include/soci/logger.h
@@ -11,10 +11,21 @@
 #include "soci/soci-platform.h"
 
 #include <ostream>
+#include <vector>
 
 namespace soci
 {
 
+
+struct SOCI_DECL query_parameter
+{
+    explicit query_parameter(std::string name = {}, std::string value = {})
+        : name(std::move(name)), value(std::move(value)) {}
+
+    std::string name;
+    std::string value;
+};
+
 // Allows to customize the logging of database operations performed by SOCI.
 //
 // To do it, derive your own class from logger_impl and override its pure
@@ -28,12 +39,13 @@ class SOCI_DECL logger_impl
     virtual ~logger_impl();
 
     // Called to indicate that a new query is about to be executed.
-    virtual void start_query(std::string const & query) = 0;
+    virtual void start_query(std::string const & query);
 
     // Called to log a parameter that is bound to the currently active query
-    virtual void add_query_parameter(std::string name, std::string value) = 0;
+    virtual void add_query_parameter(std::string name, std::string value);
 
-    virtual void reset_query_parameter() = 0;
+    // Clears all currently logged query parameters
+    virtual void clear_query_parameters();
 
     logger_impl * clone() const;
 
@@ -43,7 +55,10 @@ class SOCI_DECL logger_impl
     virtual void set_stream(std::ostream * s);
     virtual std::ostream * get_stream() const;
     virtual std::string get_last_query() const;
-    virtual std::string get_last_query_with_context() const;
+    virtual std::string get_last_query_context() const;
+
+protected:
+    std::vector<query_parameter> queryParams_;
 
 private:
     // Override to return a new heap-allocated copy of this object.
@@ -79,13 +94,13 @@ class SOCI_DECL logger
         m_impl->add_query_parameter(std::move(name), std::move(value));
     }
 
-    virtual void reset_query_parameter() { m_impl->reset_query_parameter(); }
+    virtual void clear_query_parameters() { m_impl->clear_query_parameters(); }
 
     // Methods used for the implementation of session basic logging support.
     void set_stream(std::ostream * s) { m_impl->set_stream(s); }
     std::ostream * get_stream() const { return m_impl->get_stream(); }
     std::string get_last_query() const { return m_impl->get_last_query(); }
-    std::string get_last_query_with_context() const { return m_impl->get_last_query_with_context(); }
+    std::string get_last_query_context() const { return m_impl->get_last_query_context(); }
 
 private:
     logger_impl * m_impl;

diff --git a/include/soci/session.h b/include/soci/session.h
@@ -106,10 +106,10 @@ class SOCI_DECL session
     std::ostream * get_log_stream() const;
 
     void log_query(std::string const & query);
-    void reset_query_parameter();
+    void clear_query_parameters();
     void add_query_parameter(std::string name, std::string value);
     std::string get_last_query() const;
-    std::string get_last_query_with_context() const;
+    std::string get_last_query_context() const;
 
     void set_got_data(bool gotData);
     bool got_data() const;

diff --git a/src/core/logger.cpp b/src/core/logger.cpp
@@ -23,6 +23,18 @@ void throw_not_supported()
 } // namespace anonymous
 
 
+void logger_impl::start_query(const std::string &)
+{
+    clear_query_parameters();
+}
+
+void logger_impl::add_query_parameter(std::string name, std::string value)
+{
+    queryParams_.emplace_back(std::move(name), std::move(value));
+}
+
+void logger_impl::clear_query_parameters() { queryParams_.clear(); }
+
 logger_impl * logger_impl::clone() const
 {
     logger_impl * const impl = do_clone();
@@ -57,11 +69,26 @@ std::string logger_impl::get_last_query() const
     SOCI_DUMMY_RETURN(std::string());
 }
 
-std::string logger_impl::get_last_query_with_context() const
+std::string logger_impl::get_last_query_context() const
 {
-    throw_not_supported();
+    std::string context;
 
-    SOCI_DUMMY_RETURN(std::string());
+    bool first = true;
+    for (const query_parameter &param : queryParams_)
+    {
+        if (first)
+        {
+            first = false;
+        }
+        else
+        {
+            context += ", ";
+        }
+
+        context += ":" + param.name + "=" + param.value;
+    }
+
+    return context;
 }
 
 logger::logger(logger_impl * impl)

diff --git a/src/core/session.cpp b/src/core/session.cpp
@@ -26,15 +26,6 @@ void ensureConnected(session_backend * backEnd)
     }
 }
 
-struct query_parameter
-{
-    query_parameter(std::string name = {}, std::string value = {})
-        : name(std::move(name)), value(std::move(value)) {}
-
-    std::string name;
-    std::string value;
-};
-
 // Standard logger class used by default.
 class standard_logger_impl : public logger_impl
 {
@@ -46,22 +37,16 @@ class standard_logger_impl : public logger_impl
 
     virtual void start_query(std::string const & query)
     {
+        logger_impl::start_query(query);
+
         if (logStream_ != NULL)
         {
             *logStream_ << query << '\n';
         }
 
         lastQuery_ = query;
-        reset_query_parameter();
     }
 
-    virtual void add_query_parameter(std::string name, std::string value)
-    {
-        queryParams_.emplace_back(std::move(name), std::move(value));
-    }
-
-    virtual void reset_query_parameter() { queryParams_.clear(); }
-
     virtual void set_stream(std::ostream * s)
     {
         logStream_ = s;
@@ -77,31 +62,6 @@ class standard_logger_impl : public logger_impl
         return lastQuery_;
     }
 
-    virtual std::string get_last_query_with_context() const
-    {
-        if (queryParams_.empty()) {
-            return get_last_query();
-        }
-
-        std::string query = get_last_query();
-
-        query += " with ";
-
-        for (std::size_t i = 0; i < queryParams_.size(); ++i)
-        {
-            const query_parameter &param = queryParams_[i];
-
-            query += ":" + param.name + "=" + param.value;
-
-            if (i + 1  < queryParams_.size())
-            {
-                query += ", ";
-            }
-        }
-
-        return query;
-    }
-
 private:
     virtual logger_impl* do_clone() const
     {
@@ -110,7 +70,6 @@ class standard_logger_impl : public logger_impl
 
     std::ostream * logStream_;
     std::string lastQuery_;
-    std::vector<query_parameter> queryParams_;
 };
 
 } // namespace anonymous
@@ -492,15 +451,15 @@ void session::log_query(std::string const & query)
     }
 }
 
-void session::reset_query_parameter()
+void session::clear_query_parameters()
 {
     if (isFromPool_)
     {
-        pool_->at(poolPosition_).reset_query_parameter();
+        pool_->at(poolPosition_).clear_query_parameters();
     }
     else
     {
-        logger_.reset_query_parameter();
+        logger_.clear_query_parameters();
     }
 }
 
@@ -528,15 +487,15 @@ std::string session::get_last_query() const
     }
 }
 
-std::string session::get_last_query_with_context() const
+std::string session::get_last_query_context() const
 {
     if (isFromPool_)
     {
-        return pool_->at(poolPosition_).get_last_query_with_context();
+        return pool_->at(poolPosition_).get_last_query_context();
     }
     else
     {
-        return logger_.get_last_query_with_context();
+        return logger_.get_last_query_context();
     }
 }
 

diff --git a/src/core/statement.cpp b/src/core/statement.cpp
@@ -590,7 +590,7 @@ void statement_impl::pre_fetch()
 
 void statement_impl::pre_use()
 {
-    session_.reset_query_parameter();
+    session_.clear_query_parameters();
 
     std::size_t const usize = uses_.size();
     for (std::size_t i = 0; i != usize; ++i)

diff --git a/tests/common-tests.h b/tests/common-tests.h
@@ -3640,7 +3640,7 @@ TEST_CASE_METHOD(common_tests, "Basic logging support", "[core][logging]")
     catch (...) {}
 
     CHECK(sql.get_last_query() == "drop table soci_test1");
-    CHECK(sql.get_last_query() == sql.get_last_query_with_context());
+    CHECK(sql.get_last_query_context() == "");
 
     sql.set_log_stream(NULL);
 
@@ -3652,7 +3652,7 @@ TEST_CASE_METHOD(common_tests, "Basic logging support", "[core][logging]")
         sql << "insert into soci_test (name,id) values (:name,:id)", use(name, "name"), use(id, "id");
 
         CHECK(sql.get_last_query() == "insert into soci_test (name,id) values (:name,:id)");
-        CHECK(sql.get_last_query_with_context() == "insert into soci_test (name,id) values (:name,:id) with :name=\"b\", :id=1");
+        CHECK(sql.get_last_query_context() == R"(:name="b", :id=1)");
 
         statement stmt = (sql.prepare << "insert into soci_test(name, id) values (:name, :id)");
         {
@@ -3664,7 +3664,7 @@ TEST_CASE_METHOD(common_tests, "Basic logging support", "[core][logging]")
             stmt.execute(true);
             stmt.bind_clean_up();
             CHECK(sql.get_last_query() == "insert into soci_test(name, id) values (:name, :id)");
-            CHECK(sql.get_last_query_with_context() == "insert into soci_test(name, id) values (:name, :id) with :name=\"alice\", :id=5");
+            CHECK(sql.get_last_query_context() == R"(:name="alice", :id=5)");
         }
         {
             id = 42;
@@ -3675,7 +3675,7 @@ TEST_CASE_METHOD(common_tests, "Basic logging support", "[core][logging]")
             stmt.execute(true);
             stmt.bind_clean_up();
             CHECK(sql.get_last_query() == "insert into soci_test(name, id) values (:name, :id)");
-            CHECK(sql.get_last_query_with_context() == "insert into soci_test(name, id) values (:name, :id) with :name=\"bob\", :id=42");
+            CHECK(sql.get_last_query_context() == R"(:name="bob", :id=42)");
         }
 
     }
@@ -7003,10 +7003,6 @@ TEST_CASE_METHOD(common_tests, "Logger", "[core][log]")
             m_logbuf.push_back(query);
         }
 
-        virtual void reset_query_parameter() {}
-
-        virtual void add_query_parameter(std::string /*name*/, std::string /*value*/) {}
-
     private:
         virtual logger_impl* do_clone() const
         {