From 39e7a10bc4ccdcb6dfd6266eff83c945f79b6138 Mon Sep 17 00:00:00 2001 From: Robert Adam Date: Sun, 1 Sep 2024 14:08:34 +0200 Subject: [PATCH] Address review comments --- include/soci/logger.h | 27 +++++++++++++++----- include/soci/session.h | 4 +-- src/core/logger.cpp | 33 +++++++++++++++++++++--- src/core/session.cpp | 57 ++++++------------------------------------ src/core/statement.cpp | 2 +- tests/common-tests.h | 12 +++------ 6 files changed, 66 insertions(+), 69 deletions(-) diff --git a/include/soci/logger.h b/include/soci/logger.h index 302ffc6fe..48740b47e 100644 --- a/include/soci/logger.h +++ b/include/soci/logger.h @@ -11,10 +11,21 @@ #include "soci/soci-platform.h" #include +#include 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 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 index 157080989..b6ca1d3b6 100644 --- 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 index 51520514f..794ea5595 100644 --- 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 ¶m : 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 index 5b703fe1f..18488c665 100644 --- 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 ¶m = 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 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 index d91dc0410..4ae69daf9 100644 --- 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 index 1916520a3..e30e31afb 100644 --- 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 {