~singpolyma/biboumi: Refactor that fixes a compilation issue in Release mode

12 files changed, 141 insertions(+), 127 deletions(-)

M src/database/database.cpp
R src/database/{column_escape.cpp => insert_query.cpp}
M src/database/insert_query.hpp
M src/database/row.hpp
A src/database/save.hpp
A src/database/select_query.cpp
M src/database/select_query.hpp
M src/database/sqlite3_engine.cpp
M src/database/table.hpp
M src/database/update_query.hpp
M src/xmpp/biboumi_adhoc_commands.cpp
M tests/database.cpp

M src/database/database.cpp => src/database/database.cpp +26 -31

@@ 1,6 1,8 @@
 #include "biboumi.h"
 #ifdef USE_DATABASE
 
+#include <database/select_query.hpp>
+#include <database/save.hpp>
 #include <database/database.hpp>
 #include <utils/get_first_non_empty.hpp>
 #include <utils/time.hpp>


@@ 63,32 65,28 @@ void Database::open(const std::string& filename)
 
 Database::GlobalOptions Database::get_global_options(const std::string& owner)
 {
-  auto request = Database::global_options.select();
+  auto request = select(Database::global_options);
   request.where() << Owner{} << "=" << owner;
 
-  Database::GlobalOptions options{Database::global_options.get_name()};
   auto result = request.execute(*Database::db);
   if (result.size() == 1)
-    options = result.front();
-  else
-    options.col<Owner>() = owner;
+    return result.front();
+  Database::GlobalOptions options{Database::global_options.get_name()};
+  options.col<Owner>() = owner;
   return options;
 }
 
 Database::IrcServerOptions Database::get_irc_server_options(const std::string& owner, const std::string& server)
 {
-  auto request = Database::irc_server_options.select();
+  auto request = select(Database::irc_server_options);
   request.where() << Owner{} << "=" << owner << " and " << Server{} << "=" << server;
 
-  Database::IrcServerOptions options{Database::irc_server_options.get_name()};
   auto result = request.execute(*Database::db);
   if (result.size() == 1)
-    options = result.front();
-  else
-    {
-      options.col<Owner>() = owner;
-      options.col<Server>() = server;
-    }
+    return result.front();
+  Database::IrcServerOptions options{Database::irc_server_options.get_name()};
+  options.col<Owner>() = owner;
+  options.col<Server>() = server;
   return options;
 }
 


@@ 97,7 95,7 @@ Database::AfterConnectionCommands Database::get_after_connection_commands(const 
   const auto id = server_options.col<Id>();
   if (id == Id::unset_value)
     return {};
-  auto request = Database::after_connection_commands.select();
+  auto request = select(Database::after_connection_commands);
   request.where() << ForeignKey{} << "=" << id;
   return request.execute(*Database::db);
 }


@@ 116,26 114,23 @@ void Database::set_after_connection_commands(const Database::IrcServerOptions& s
   for (auto& command: commands)
     {
       command.col<ForeignKey>() = server_options.col<Id>();
-      command.save(Database::db);
+      save(command, *Database::db);
     }
 }
 
 Database::IrcChannelOptions Database::get_irc_channel_options(const std::string& owner, const std::string& server, const std::string& channel)
 {
-  auto request = Database::irc_channel_options.select();
+  auto request = select(Database::irc_channel_options);
   request.where() << Owner{} << "=" << owner <<\
           " and " << Server{} << "=" << server <<\
           " and " << Channel{} << "=" << channel;
-  Database::IrcChannelOptions options{Database::irc_channel_options.get_name()};
   auto result = request.execute(*Database::db);
   if (result.size() == 1)
-    options = result.front();
-  else
-    {
-      options.col<Owner>() = owner;
-      options.col<Server>() = server;
-      options.col<Channel>() = channel;
-    }
+    return result.front();
+  Database::IrcChannelOptions options{Database::irc_channel_options.get_name()};
+  options.col<Owner>() = owner;
+  options.col<Server>() = server;
+  options.col<Channel>() = channel;
   return options;
 }
 


@@ 191,7 186,7 @@ std::string Database::store_muc_message(const std::string& owner, const std::str
   line.col<Body>() = body;
   line.col<Nick>() = nick;
 
-  line.save(Database::db);
+  save(line, *Database::db);
 
   return uuid;
 }


@@ 202,7 197,7 @@ std::vector<Database::MucLogLine> Database::get_muc_logs(const std::string& owne
   if (limit == 0)
     return {};
 
-  auto request = Database::muc_log_lines.select();
+  auto request = select(Database::muc_log_lines);
   request.where() << Database::Owner{} << "=" << owner << \
           " and " << Database::IrcChanName{} << "=" << chan_name << \
           " and " << Database::IrcServerName{} << "=" << server;


@@ 256,7 251,7 @@ std::vector<Database::MucLogLine> Database::get_muc_logs(const std::string& owne
 Database::MucLogLine Database::get_muc_log(const std::string& owner, const std::string& chan_name, const std::string& server,
                                            const std::string& uuid, const std::string& start, const std::string& end)
 {
-  auto request = Database::muc_log_lines.select();
+  auto request = select(Database::muc_log_lines);
   request.where() << Database::Owner{} << "=" << owner << \
           " and " << Database::IrcChanName{} << "=" << chan_name << \
           " and " << Database::IrcServerName{} << "=" << server << \


@@ 297,7 292,7 @@ void Database::add_roster_item(const std::string& local, const std::string& remo
   roster_item.col<Database::LocalJid>() = local;
   roster_item.col<Database::RemoteJid>() = remote;
 
-  roster_item.save(Database::db);
+  save(roster_item, *Database::db);
 }
 
 void Database::delete_roster_item(const std::string& local, const std::string& remote)


@@ 311,7 306,7 @@ void Database::delete_roster_item(const std::string& local, const std::string& r
 
 bool Database::has_roster_item(const std::string& local, const std::string& remote)
 {
-  auto query = Database::roster.select();
+  auto query = select(Database::roster);
   query.where() << Database::LocalJid{} << "=" << local << \
         " and " << Database::RemoteJid{} << "=" << remote;
 


@@ 322,7 317,7 @@ bool Database::has_roster_item(const std::string& local, const std::string& remo
 
 std::vector<Database::RosterItem> Database::get_contact_list(const std::string& local)
 {
-  auto query = Database::roster.select();
+  auto query = select(Database::roster);
   query.where() << Database::LocalJid{} << "=" << local;
 
   return query.execute(*Database::db);


@@ 330,7 325,7 @@ std::vector<Database::RosterItem> Database::get_contact_list(const std::string& 
 
 std::vector<Database::RosterItem> Database::get_full_roster()
 {
-  auto query = Database::roster.select();
+  auto query = select(Database::roster);
 
   return query.execute(*Database::db);
 }

R src/database/column_escape.cpp => src/database/insert_query.cpp +0 -25

@@ 1,28 1,3 @@
-#include <string>
-
-#include <database/database.hpp>
-#include <database/select_query.hpp>
-
-template <>
-std::string before_column<Database::Date>()
-{
-  if (Database::engine_type() == DatabaseEngine::EngineType::Sqlite3)
-    return "strftime(\"%Y-%m-%dT%H:%M:%SZ\", ";
-  else if (Database::engine_type() == DatabaseEngine::EngineType::Postgresql)
-    return "to_char(";
-  return {};
-}
-
-template <>
-std::string after_column<Database::Date>()
-{
-  if (Database::engine_type() == DatabaseEngine::EngineType::Sqlite3)
-    return ")";
-  else if (Database::engine_type() == DatabaseEngine::EngineType::Postgresql)
-    return R"(, 'YYYY-MM-DD"T"HH24:MM:SS"Z"'))";
-  return {};
-}
-
 #include <database/insert_query.hpp>
 
 template <>

M src/database/insert_query.hpp => src/database/insert_query.hpp +21 -0

@@ 1,10 1,15 @@
 #pragma once
 
 #include <database/statement.hpp>
+#include <database/database.hpp>
 #include <database/column.hpp>
 #include <database/query.hpp>
+#include <database/row.hpp>
+
 #include <logger/logger.hpp>
 
+#include <utils/is_one_of.hpp>
+
 #include <type_traits>
 #include <vector>
 #include <string>


@@ 37,6 42,12 @@ std::string after_value()
   return {};
 }
 
+template <>
+std::string before_value<Database::Date>();
+
+template <>
+std::string after_value<Database::Date>();
+
 struct InsertQuery: public Query
 {
   template <typename... T>


@@ 141,3 152,13 @@ struct InsertQuery: public Query
   insert_col_name(const std::tuple<T...>&)
   {}
 };
+
+template <typename... T>
+void insert(Row<T...>& row, DatabaseEngine& db)
+{
+  InsertQuery query(row.table_name, row.columns);
+  // Ugly workaround for non portable stuff
+  if (is_one_of<Id, T...>)
+    query.body += db.get_returning_id_sql_string(Id::name);
+  query.execute(db, row.columns);
+}

M src/database/row.hpp => src/database/row.hpp +0 -49

@@ 1,9 1,5 @@
 #pragma once
 
-#include <database/insert_query.hpp>
-#include <database/update_query.hpp>
-#include <logger/logger.hpp>
-
 #include <utils/is_one_of.hpp>
 
 #include <type_traits>


@@ 29,52 25,7 @@ struct Row
     return col.value;
   }
 
-  template <bool Coucou=true>
-  void save(std::unique_ptr<DatabaseEngine>& db, typename std::enable_if<!is_one_of<Id, T...> && Coucou>::type* = nullptr)
-  {
-    this->insert(*db);
-  }
-
-  template <bool Coucou=true>
-  void save(std::unique_ptr<DatabaseEngine>& db, typename std::enable_if<is_one_of<Id, T...> && Coucou>::type* = nullptr)
-  {
-    const Id& id = std::get<Id>(this->columns);
-    if (id.value == Id::unset_value)
-      {
-        this->insert(*db);
-        if (db->last_inserted_rowid >= 0)
-          std::get<Id>(this->columns).value = static_cast<Id::real_type>(db->last_inserted_rowid);
-      }
-    else
-      this->update(*db);
-  }
-
- private:
-  template <bool Coucou=true>
-  void insert(DatabaseEngine& db, typename std::enable_if<is_one_of<Id, T...> && Coucou>::type* = nullptr)
-  {
-    InsertQuery query(this->table_name, this->columns);
-    // Ugly workaround for non portable stuff
-    query.body += db.get_returning_id_sql_string(Id::name);
-    query.execute(db, this->columns);
-  }
-
-  template <bool Coucou=true>
-  void insert(DatabaseEngine& db, typename std::enable_if<!is_one_of<Id, T...> && Coucou>::type* = nullptr)
-  {
-    InsertQuery query(this->table_name, this->columns);
-    query.execute(db, this->columns);
-  }
-
-  void update(DatabaseEngine& db)
-  {
-    UpdateQuery query(this->table_name, this->columns);
-
-    query.execute(db, this->columns);
-  }
-
 public:
   std::tuple<T...> columns;
   std::string table_name;
-
 };

A src/database/save.hpp => src/database/save.hpp +31 -0

@@ 0,0 1,31 @@
+#pragma once
+
+#include <database/update_query.hpp>
+#include <database/insert_query.hpp>
+
+#include <database/engine.hpp>
+
+#include <database/row.hpp>
+
+#include <utils/is_one_of.hpp>
+
+template <typename... T, bool Coucou=true>
+void save(Row<T...>& row, DatabaseEngine& db, typename std::enable_if<!is_one_of<Id, T...> && Coucou>::type* = nullptr)
+{
+  insert(row, db);
+}
+
+template <typename... T, bool Coucou=true>
+void save(Row<T...>& row, DatabaseEngine& db, typename std::enable_if<is_one_of<Id, T...> && Coucou>::type* = nullptr)
+{
+  const Id& id = std::get<Id>(row.columns);
+    if (id.value == Id::unset_value)
+      {
+        insert(row, db);
+        if (db.last_inserted_rowid >= 0)
+          std::get<Id>(row.columns).value = static_cast<Id::real_type>(db.last_inserted_rowid);
+      }
+    else
+      update(row, db);
+}
+

A src/database/select_query.cpp => src/database/select_query.cpp +21 -0

@@ 0,0 1,21 @@
+#include <database/select_query.hpp>
+
+template <>
+std::string before_column<Database::Date>()
+{
+  if (Database::engine_type() == DatabaseEngine::EngineType::Sqlite3)
+    return "strftime(\"%Y-%m-%dT%H:%M:%SZ\", ";
+  else if (Database::engine_type() == DatabaseEngine::EngineType::Postgresql)
+    return "to_char(";
+  return {};
+}
+
+template <>
+std::string after_column<Database::Date>()
+{
+  if (Database::engine_type() == DatabaseEngine::EngineType::Sqlite3)
+    return ")";
+  else if (Database::engine_type() == DatabaseEngine::EngineType::Postgresql)
+    return R"(, 'YYYY-MM-DD"T"HH24:MM:SS"Z"'))";
+  return {};
+}

M src/database/select_query.hpp => src/database/select_query.hpp +14 -0

@@ 2,6 2,8 @@
 
 #include <database/engine.hpp>
 
+#include <database/table.hpp>
+#include <database/database.hpp>
 #include <database/statement.hpp>
 #include <utils/datetime.hpp>
 #include <database/query.hpp>


@@ 78,6 80,12 @@ std::string after_column()
   return {};
 }
 
+template <>
+std::string before_column<Database::Date>();
+
+template <>
+std::string after_column<Database::Date>();
+
 template <typename... T>
 struct SelectQuery: public Query
 {


@@ 153,3 161,9 @@ struct SelectQuery: public Query
     const std::string table_name;
 };
 
+template <typename... T>
+auto select(const Table<T...> table)
+{
+  SelectQuery<T...> query(table.name);
+  return query;
+}

M src/database/sqlite3_engine.cpp => src/database/sqlite3_engine.cpp +1 -0

@@ 3,6 3,7 @@
 #ifdef SQLITE3_FOUND
 
 #include <database/database.hpp>
+#include <database/select_query.hpp>
 #include <database/sqlite3_engine.hpp>
 
 #include <database/sqlite3_statement.hpp>

M src/database/table.hpp => src/database/table.hpp +2 -8

@@ 2,7 2,6 @@
 
 #include <database/engine.hpp>
 
-#include <database/select_query.hpp>
 #include <database/delete_query.hpp>
 #include <database/row.hpp>
 


@@ 82,12 81,6 @@ class Table
     return {this->name};
   }
 
-  auto select()
-  {
-    SelectQuery<T...> select(this->name);
-    return select;
-  }
-
   auto del()
   {
     DeleteQuery query(this->name);


@@ 99,6 92,8 @@ class Table
     return this->name;
   }
 
+  const std::string name;
+
  private:
 
   template <std::size_t N=0>


@@ 133,5 128,4 @@ class Table
   add_column_create(DatabaseEngine&, std::string&)
   { }
 
-  const std::string name;
 };

M src/database/update_query.hpp => src/database/update_query.hpp +10 -1

@@ 1,7 1,8 @@
 #pragma once
 
-#include <database/query.hpp>
 #include <database/engine.hpp>
+#include <database/query.hpp>
+#include <database/row.hpp>
 
 using namespace std::string_literals;
 


@@ 102,3 103,11 @@ struct UpdateQuery: public Query
     actual_bind(statement, value.value, sizeof...(T));
   }
 };
+
+template <typename... T>
+void update(Row<T...>& row, DatabaseEngine& db)
+{
+  UpdateQuery query(row.table_name, row.columns);
+
+  query.execute(db, row.columns);
+}

M src/xmpp/biboumi_adhoc_commands.cpp => src/xmpp/biboumi_adhoc_commands.cpp +4 -3

@@ 14,6 14,7 @@
 
 #ifdef USE_DATABASE
 #include <database/database.hpp>
+#include <database/save.hpp>
 #endif
 
 #ifndef HAS_PUT_TIME


@@ 196,7 197,7 @@ void ConfigureGlobalStep2(XmppComponent& xmpp_component, AdhocSession& session, 
             options.col<Database::GlobalPersistent>() = to_bool(value->get_inner());
         }
 
-      options.save(Database::db);
+      save(options, *Database::db);
 
       command_node.delete_all_children();
       XmlSubNode note(command_node, "note");


@@ 476,7 477,7 @@ void ConfigureIrcServerStep2(XmppComponent&, AdhocSession& session, XmlNode& com
 
         }
       Database::invalidate_encoding_in_cache();
-      options.save(Database::db);
+      save(options, *Database::db);
       Database::set_after_connection_commands(options, commands);
 
       command_node.delete_all_children();


@@ 646,7 647,7 @@ bool handle_irc_channel_configuration_form(XmppComponent& xmpp_component, const 
 
             }
           Database::invalidate_encoding_in_cache(requester.bare(), iid.get_server(), iid.get_local());
-          options.save(Database::db);
+          save(options, *Database::db);
         }
       return true;
     }

M tests/database.cpp => tests/database.cpp +11 -10

@@ 7,6 7,7 @@
 #include <cstdlib>
 
 #include <database/database.hpp>
+#include <database/save.hpp>
 
 #include <config/config.hpp>
 


@@ 28,11 29,11 @@ TEST_CASE("Database")
     {
       auto o = Database::get_irc_server_options("zouzou@example.com", "irc.example.com");
       CHECK(Database::count(Database::irc_server_options) == 0);
-      o.save(Database::db);
+      save(o, *Database::db);
       CHECK(Database::count(Database::irc_server_options) == 1);
       o.col<Database::Realname>() = "Different realname";
       CHECK(o.col<Database::Realname>() == "Different realname");
-      o.save(Database::db);
+      save(o, *Database::db);
       CHECK(o.col<Database::Realname>() == "Different realname");
       CHECK(Database::count(Database::irc_server_options) == 1);
 


@@ 44,7 45,7 @@ TEST_CASE("Database")
       // inserted
       CHECK(1 == Database::count(Database::irc_server_options));
 
-      b.save(Database::db);
+      save(b, *Database::db);
       CHECK(2 == Database::count(Database::irc_server_options));
 
       CHECK(b.col<Database::Pass>() == "");


@@ 58,7 59,7 @@ TEST_CASE("Database")
       o.col<Database::EncodingIn>() = "ISO-8859-1";
       CHECK(o.col<Database::RecordHistoryOptional>().is_set == false);
       o.col<Database::RecordHistoryOptional>().set_value(false);
-      o.save(Database::db);
+      save(o, *Database::db);
       auto b = Database::get_irc_channel_options("zouzou@example.com", "irc.example.com", "#foo");
       CHECK(o.col<Database::EncodingIn>() == "ISO-8859-1");
       CHECK(o.col<Database::RecordHistoryOptional>().is_set == true);


@@ 77,7 78,7 @@ TEST_CASE("Database")
       GIVEN("An option defined for the channel but not the server")
       {
         c.col<Database::EncodingIn>() = "channelEncoding";
-        c.save(Database::db);
+        save(c, *Database::db);
         WHEN("we fetch that option")
           {
             auto r = Database::get_irc_channel_options_with_server_default(owner, server, chan1);


@@ 88,7 89,7 @@ TEST_CASE("Database")
       GIVEN("An option defined for the server but not the channel")
         {
           s.col<Database::EncodingIn>() = "serverEncoding";
-          s.save(Database::db);
+          save(s, *Database::db);
         WHEN("we fetch that option")
           {
             auto r = Database::get_irc_channel_options_with_server_default(owner, server, chan1);


@@ 99,9 100,9 @@ TEST_CASE("Database")
       GIVEN("An option defined for both the server and the channel")
         {
           s.col<Database::EncodingIn>() = "serverEncoding";
-          s.save(Database::db);
+          save(s, *Database::db);
           c.col<Database::EncodingIn>() = "channelEncoding";
-          c.save(Database::db);
+          save(c, *Database::db);
         WHEN("we fetch that option")
           {
             auto r = Database::get_irc_channel_options_with_server_default(owner, server, chan1);


@@ 129,8 130,8 @@ TEST_CASE("Database")
       auto after_connection_commands =  Database::get_after_connection_commands(soptions);
       CHECK(after_connection_commands.empty());
 
-      soptions.save(Database::db);
-      soptions2.save(Database::db);
+      save(soptions, *Database::db);
+      save(soptions2, *Database::db);
       auto com = Database::after_connection_commands.row();
       com.col<Database::AfterConnectionCommand>() = "first";
       after_connection_commands.push_back(com);