Merge lp:~vjsamuel/drizzle/rplugin-drizzle-protocol into lp:~drizzle-trunk/drizzle/development

[Vijay Samuel]

[Monty Taylor (mordred)]

Good work so far. Now that you're hacking in the server, there are a few more new things you're going to need to deal with. (Most of this has to do with error message handling)

First of all: in init(module::Context &context), you're doing validation checks directly at module init time. I'd really prefer to see option value validation happen at option parse time, so that we don't have to get halfway in to server startup before we discover a bad value in a config file. Can you register validation methods for these values when you add them to the context?

Secondly, for reporting error messages, you need to use errmsg_printf rather than cout. This allows them to be processed by the servers error message reporting system, which may or may not be configured to actually print to a console (other options would be syslog or the like)

Just while I'm giving you feedback, if you _were_ to use a cout approach (not a choice here) you should be using cerr instead, and also should be adding a std::endl on to the end.

Finally, you need to wrap these new messages strings you just added in _() so that they'll get translated.

So this:

cout << "Invalid value for buffer_length";

Should become:

errmsg_printf(ERRMSG_LVL_ERROR, _("Invalid value for buffer_length\n"));

review: Needs Fixing

[Monty Taylor (mordred)]

Looks good to me. The code in the core in the module loading section actually comes from me.

review: Approve

[Vijay Samuel (vjsamuel)]

Thank you for that important fix Monty!

=== modified file 'drizzled/module/option_context.cc'
--- drizzled/module/option_context.cc	2010-06-16 22:09:59 +0000
+++ drizzled/module/option_context.cc	2010-07-06 08:14:36 +0000
@@ -36,7 +36,7 @@
 option_context& option_context::operator()(const char* name,
                                            const char* description)
 {
-  const std::string new_name(prepend_name(name));
+  const std::string new_name(prepend_name(module_name, name));
   po_options(new_name.c_str(), description);
   return *this;
 }
@@ -45,7 +45,7 @@
 option_context& option_context::operator()(const char* name,
                                            const po::value_semantic* s)
 {
-  const std::string new_name(prepend_name(name));
+  const std::string new_name(prepend_name(module_name, name));
   po_options(new_name.c_str(), s);
   return *this;
 }
@@ -55,11 +55,40 @@
                              const po::value_semantic* s,
                              const char* description)
 {
-  const std::string new_name(prepend_name(name));
+  const std::string new_name(prepend_name(module_name, name));
   po_options(new_name.c_str(), s, description);
   return *this;
 }
 
+namespace
+{
+
+class SwapUnderscores
+{
+public:
+  char operator()(char a) const
+  {
+    if (a == '_')
+      return '-';
+    return a;
+  }
+};
+
+} /* namespace */
+
+/*
+ * Private methods.
+ */
+std::string option_context::prepend_name(std::string module_name,
+                                         const char *name_in)
+{
+  module_name.push_back('.');
+  std::transform(module_name.begin(), module_name.end(),
+                 module_name.begin(), SwapUnderscores());
+  module_name.append(name_in);
+  return module_name;
+}
+
 
 } /* namespace module */
 } /* namespace drizzled */

=== modified file 'drizzled/module/option_context.h'
--- drizzled/module/option_context.h	2010-06-16 22:09:59 +0000
+++ drizzled/module/option_context.h	2010-07-06 08:14:36 +0000
@@ -47,8 +47,6 @@
   const std::string &module_name;
   po::options_description_easy_init po_options;
 
-  std::string prepend_name(const char *name) const;
-
 public:
 
   option_context(const std::string &module_name_in,
@@ -64,6 +62,9 @@
                              const po::value_semantic* s,
                              const char* description);
 
+  static std::string prepend_name(std::string module_name_in,
+                                  const char *name);
+
 private:
   
   /**
@@ -82,17 +83,6 @@
   option_context& operator=(const option_context &);
 };
 
-/*
- * Private methods.
- */
-inline std::string option_context::prepend_name(const char *name_in) const
-{
-  std::string new_name(module_name);
-  new_name.push_back('.');
-  new_name.append(name_in);
-  return new_name;
-}
-
 } /* namespace module */
 } /* namespace drizzled */
 

=== modified file 'drizzled/module/option_map.h'
--- drizzled/module/option_map.h	2010-06-20 19:05:15 +0000
+++ drizzled/module/option_map.h	2010-07-06 08:14:36 +0000
@@ -26,6 +26,7 @@
 #define DRIZZLED_MODULE_OPTION_MAP_H
 
 #include "drizzled/visibility.h"
+#include "drizzled/module/option_context.h"
 
 #include <boost/program_options.hpp>
 
@@ -53,18 +54,12 @@
 
   const boost::program_options::variable_value& operator[](const std::string &name_in) const
   {
-    std::string new_name(module_name);
-    new_name.push_back('.');
-    new_name.append(name_in);
-    return vm[new_name];
+    return vm[option_context::prepend_name(module_name, name_in.c_str())];
   }
 
   size_t count(const std::string &name_in) const
   {
-    std::string new_name(module_name);
-    new_name.push_back('.');
-    new_name.append(name_in);
-    return vm.count(new_name);
+    return vm.count(option_context::prepend_name(module_name, name_in.c_str()));
   }
 
 private:

=== modified file 'drizzled/option.cc'
--- drizzled/option.cc	2010-04-23 21:37:29 +0000
+++ drizzled/option.cc	2010-07-06 08:14:36 +0000
@@ -562,7 +562,7 @@
   {
     char *ptr, *end;
 
-    ptr= strrchr(cur_arg + 1, '.'); /* Skip the first character */
+    ptr= NULL; //Options with '.' are now supported.
     end= strrchr(cur_arg, '=');
 
     /*

=== modified file 'drizzled/plugin.h'
--- drizzled/plugin.h	2010-06-21 04:53:35 +0000
+++ drizzled/plugin.h	2010-07-06 08:14:36 +0000
@@ -68,7 +68,7 @@
 
 
 #define DRIZZLE_DECLARE_PLUGIN_END
-#define DRIZZLE_PLUGIN(init,system) \
+#define DRIZZLE_PLUGIN(init,system,options) \
   DRIZZLE_DECLARE_PLUGIN \
   { \
     DRIZZLE_VERSION_ID, \
@@ -77,7 +77,7 @@
     STRINGIFY_ARG(PANDORA_MODULE_AUTHOR), \
     STRINGIFY_ARG(PANDORA_MODULE_TITLE), \
     PANDORA_MODULE_LICENSE, \
-    init, system, NULL \
+    init, system, options \
   } 
 
 

=== modified file 'plugin/auth_file/auth_file.cc'
--- plugin/auth_file/auth_file.cc	2010-05-18 18:20:56 +0000
+++ plugin/auth_file/auth_file.cc	2010-07-06 08:14:36 +0000
@@ -237,4 +237,4 @@
 
 } /* namespace auth_file */
 
-DRIZZLE_PLUGIN(auth_file::init, auth_file::sys_variables);
+DRIZZLE_PLUGIN(auth_file::init, auth_file::sys_variables, NULL);

=== modified file 'plugin/auth_ldap/auth_ldap.cc'
--- plugin/auth_ldap/auth_ldap.cc	2010-05-18 18:20:56 +0000
+++ plugin/auth_ldap/auth_ldap.cc	2010-07-06 08:14:36 +0000
@@ -490,4 +490,4 @@
 
 } /* namespace auth_ldap */
 
-DRIZZLE_PLUGIN(auth_ldap::init, auth_ldap::sys_variables);
+DRIZZLE_PLUGIN(auth_ldap::init, auth_ldap::sys_variables, NULL);

=== modified file 'plugin/auth_ldap/test_ldap.sh'
--- plugin/auth_ldap/test_ldap.sh	2010-04-27 20:36:41 +0000
+++ plugin/auth_ldap/test_ldap.sh	2010-07-06 08:14:36 +0000
@@ -19,7 +19,7 @@
   --auth-ldap-base-dn="dc=drizzle,dc=org" \
   --auth-ldap-cache-timeout=1 \
   --mysql-protocol-port=12345 \
-  --drizzle-protocol-port=12346 \
+  --drizzle-protocol.port=12346 \
   --pid-file=pid &
 
 sleep 3

=== modified file 'plugin/auth_test/auth_test.cc'
--- plugin/auth_test/auth_test.cc	2010-05-18 18:20:56 +0000
+++ plugin/auth_test/auth_test.cc	2010-07-06 08:14:36 +0000
@@ -106,4 +106,4 @@
 
 } /* namespace auth_test */
 
-DRIZZLE_PLUGIN(auth_test::init, NULL);
+DRIZZLE_PLUGIN(auth_test::init, NULL, NULL);

=== modified file 'plugin/blitzdb/ha_blitz.cc'
--- plugin/blitzdb/ha_blitz.cc	2010-06-29 21:22:35 +0000
+++ plugin/blitzdb/ha_blitz.cc	2010-07-06 08:14:36 +0000
@@ -1457,4 +1457,4 @@
   return pos + skip_len + sizeof(uint16_t);
 }
 
-DRIZZLE_PLUGIN(blitz_init, NULL);
+DRIZZLE_PLUGIN(blitz_init, NULL, NULL);

=== modified file 'plugin/crc32/crc32udf.cc'
--- plugin/crc32/crc32udf.cc	2010-05-18 18:20:56 +0000
+++ plugin/crc32/crc32udf.cc	2010-07-06 08:14:36 +0000
@@ -78,4 +78,4 @@
   return 0;
 }
 
-DRIZZLE_PLUGIN(initialize, NULL);
+DRIZZLE_PLUGIN(initialize, NULL, NULL);

=== modified file 'plugin/default_replicator/default_replicator.cc'
--- plugin/default_replicator/default_replicator.cc	2010-05-18 18:20:56 +0000
+++ plugin/default_replicator/default_replicator.cc	2010-07-06 08:14:36 +0000
@@ -66,4 +66,4 @@
   return 0;
 }
 
-DRIZZLE_PLUGIN(init,  NULL);
+DRIZZLE_PLUGIN(init,  NULL, NULL);

=== modified file 'plugin/drizzle_protocol/drizzle_protocol.cc'
--- plugin/drizzle_protocol/drizzle_protocol.cc	2010-05-18 18:20:56 +0000
+++ plugin/drizzle_protocol/drizzle_protocol.cc	2010-07-06 08:14:36 +0000
@@ -26,7 +26,9 @@
 #include "drizzled/internal/my_sys.h"
 #include "drizzled/internal/m_string.h"
 #include <algorithm>
-
+#include <iostream>
+#include <boost/program_options.hpp>
+#include <drizzled/module/option_map.h>
 #include "pack.h"
 #include "errmsg.h"
 #include "drizzle_protocol.h"
@@ -38,7 +40,7 @@
 {
 extern uint32_t global_thread_id;
 }
-
+namespace po= boost::program_options;
 using namespace drizzled;
 using namespace std;
 
@@ -812,12 +814,74 @@
   drizzleclient_net_write(&net, buff, 5);
 }
 
-static ListenDrizzleProtocol *listen_obj= NULL;
-
 static int init(module::Context &context)
 {
-  listen_obj= new ListenDrizzleProtocol("drizzle_protocol", false);
-  context.add(listen_obj); 
+  const module::option_map &vm= context.getOptions();
+  if (vm.count("port"))
+  { 
+    if (port > 65535)
+    {
+      errmsg_printf(ERRMSG_LVL_ERROR, _("Invalid value of port\n"));
+      exit(-1);
+    }
+  }
+
+  if (vm.count("connect-timeout"))
+  {
+    if (connect_timeout < 1 || connect_timeout > 300)
+    {
+      errmsg_printf(ERRMSG_LVL_ERROR, _("Invalid value for connect_timeout\n"));
+      exit(-1);
+    }
+  }
+
+  if (vm.count("read-timeout"))
+  {
+    if (read_timeout < 1 || read_timeout > 300)
+    {
+      errmsg_printf(ERRMSG_LVL_ERROR, _("Invalid value for read_timeout\n"));
+      exit(-1);
+    }
+  }
+
+  if (vm.count("write-timeout"))
+  {
+    if (write_timeout < 1 || write_timeout > 300)
+    {
+      errmsg_printf(ERRMSG_LVL_ERROR, _("Invalid value for write_timeout\n"));
+      exit(-1);
+    }
+  }
+
+  if (vm.count("retry-count"))
+  {
+    if (retry_count < 1 || retry_count > 100)
+    {
+      errmsg_printf(ERRMSG_LVL_ERROR, _("Invalid value for retry_count\n"));
+      exit(-1);
+    }
+  }
+
+  if (vm.count("buffer-length"))
+  {
+    if (buffer_length < 1024 || buffer_length > 1024*1024)
+    {
+      errmsg_printf(ERRMSG_LVL_ERROR, _("Invalid value for buffer_length\n"));
+      exit(-1);
+    }
+  }
+
+  if (vm.count("bind-address"))
+  {
+    bind_address= strdup(vm["bind-address"].as<string>().c_str());
+  }
+
+  else
+  {
+    bind_address= NULL;
+  }
+  
+  context.add(new ListenDrizzleProtocol("drizzle_protocol", false)); 
   return 0;
 }
 
@@ -842,6 +906,34 @@
 static DRIZZLE_SYSVAR_STR(bind_address, bind_address, PLUGIN_VAR_READONLY,
                           N_("Address to bind to."), NULL, NULL, NULL);
 
+static void init_options(drizzled::module::option_context &context)
+{
+  context("port",
+          po::value<uint32_t>(&port)->default_value(0),
+          N_("Port number to use for connection or 0 for "
+                              "default to, in order of "
+                              "preference, drizzle.cnf, $DRIZZLE_TCP_PORT, "
+                              "built-in default (4427)."));
+  context("connect-timeout",
+          po::value<uint32_t>(&connect_timeout)->default_value(10),
+          N_("Connect Timeout."));
+  context("read-timeout",
+          po::value<uint32_t>(&read_timeout)->default_value(30),
+          N_("Read Timeout."));
+  context("write-timeout",
+          po::value<uint32_t>(&write_timeout)->default_value(60),
+          N_("Write Timeout."));
+  context("retry-count",
+          po::value<uint32_t>(&retry_count)->default_value(10),
+          N_("Retry Count."));
+  context("buffer-length",
+          po::value<uint32_t>(&buffer_length)->default_value(16384),
+          N_("Buffer length."));
+  context("bind-address",
+          po::value<string>(),
+          N_("Address to bind to."));
+}
+
 static drizzle_sys_var* sys_variables[]= {
   DRIZZLE_SYSVAR(port),
   DRIZZLE_SYSVAR(connect_timeout),
@@ -855,4 +947,4 @@
 
 } /* namespace drizzle_protocol */
 
-DRIZZLE_PLUGIN(drizzle_protocol::init, drizzle_protocol::sys_variables);
+DRIZZLE_PLUGIN(drizzle_protocol::init, drizzle_protocol::sys_variables, drizzle_protocol::init_options);

=== modified file 'plugin/errmsg_notify/errmsg_notify.cc'
--- plugin/errmsg_notify/errmsg_notify.cc	2010-05-18 18:20:56 +0000
+++ plugin/errmsg_notify/errmsg_notify.cc	2010-07-06 08:14:36 +0000
@@ -103,4 +103,4 @@
   return 0;
 }
 
-DRIZZLE_PLUGIN(plugin_init, NULL);
+DRIZZLE_PLUGIN(plugin_init, NULL, NULL);

=== modified file 'plugin/filtered_replicator/filtered_replicator.cc'
--- plugin/filtered_replicator/filtered_replicator.cc	2010-05-18 18:20:56 +0000
+++ plugin/filtered_replicator/filtered_replicator.cc	2010-07-06 08:14:36 +0000
@@ -611,4 +611,4 @@
   NULL
 };
 
-DRIZZLE_PLUGIN(init, filtered_replicator_system_variables);
+DRIZZLE_PLUGIN(init, filtered_replicator_system_variables, NULL);

=== modified file 'plugin/logging_stats/tests/t/command-master.opt'
--- plugin/logging_stats/tests/t/command-master.opt	2010-04-09 01:58:28 +0000
+++ plugin/logging_stats/tests/t/command-master.opt	2010-07-06 08:14:36 +0000
@@ -1,1 +1,1 @@
---plugin-add=logging_stats --logging_stats_enable --logging_stats_max_user_count=503
+--plugin-add=logging_stats --logging-stats-enable --logging-stats-max-user-count=503

=== modified file 'plugin/logging_stats/tests/t/max_session-master.opt'
--- plugin/logging_stats/tests/t/max_session-master.opt	2010-04-09 01:58:28 +0000
+++ plugin/logging_stats/tests/t/max_session-master.opt	2010-07-06 08:14:36 +0000
@@ -1,1 +1,1 @@
---plugin-add=logging_stats --logging_stats_enable --logging_stats_scoreboard_size=10 --logging_stats_bucket_count=5 --logging_stats_max_user_count=502
+--plugin-add=logging_stats --logging-stats-enable --logging-stats-scoreboard-size=10 --logging-stats-bucket-count=5 --logging-stats-max-user-count=502

=== modified file 'plugin/logging_stats/tests/t/max_user-master.opt'
--- plugin/logging_stats/tests/t/max_user-master.opt	2010-04-09 01:58:28 +0000
+++ plugin/logging_stats/tests/t/max_user-master.opt	2010-07-06 08:14:36 +0000
@@ -1,1 +1,1 @@
---plugin-add=logging_stats --logging_stats_enable --logging_stats_max_user_count=505
+--plugin-add=logging_stats --logging-stats-enable --logging-stats-max-user-count=505

=== modified file 'plugin/logging_stats/tests/t/slap-master.opt'
--- plugin/logging_stats/tests/t/slap-master.opt	2010-04-09 01:58:28 +0000
+++ plugin/logging_stats/tests/t/slap-master.opt	2010-07-06 08:14:36 +0000
@@ -1,1 +1,1 @@
---plugin-add=logging_stats --logging_stats_enable --logging_stats_max_user_count=501
+--plugin-add=logging_stats --logging-stats-enable --logging-stats-max-user-count=501

=== modified file 'plugin/rabbitmq/rabbitmq_log.cc'
--- plugin/rabbitmq/rabbitmq_log.cc	2010-05-21 16:24:37 +0000
+++ plugin/rabbitmq/rabbitmq_log.cc	2010-07-06 08:14:36 +0000
@@ -263,5 +263,5 @@
   NULL
 };
 
-DRIZZLE_PLUGIN(init, system_variables);
+DRIZZLE_PLUGIN(init, system_variables, NULL);
 

=== modified file 'plugin/rand_function/rand_function.cc'
--- plugin/rand_function/rand_function.cc	2010-06-04 09:05:44 +0000
+++ plugin/rand_function/rand_function.cc	2010-07-06 08:14:36 +0000
@@ -126,4 +126,4 @@
   return 0;
 }
 
-DRIZZLE_PLUGIN(initialize, NULL);
+DRIZZLE_PLUGIN(initialize, NULL, NULL);

=== modified file 'plugin/replication_dictionary/module.cc'
--- plugin/replication_dictionary/module.cc	2010-05-18 18:20:56 +0000
+++ plugin/replication_dictionary/module.cc	2010-07-06 08:14:36 +0000
@@ -53,4 +53,4 @@
   }
 }
 
-DRIZZLE_PLUGIN(init, NULL);
+DRIZZLE_PLUGIN(init, NULL, NULL);

=== modified file 'plugin/rot13/rot13.cc'
--- plugin/rot13/rot13.cc	2010-05-18 18:20:56 +0000
+++ plugin/rot13/rot13.cc	2010-07-06 08:14:36 +0000
@@ -93,4 +93,4 @@
 
 } /* namespace rot13 */
 
-DRIZZLE_PLUGIN(rot13::init, NULL);
+DRIZZLE_PLUGIN(rot13::init, NULL, NULL);

=== modified file 'plugin/simple_user_policy/module.cc'
--- plugin/simple_user_policy/module.cc	2010-05-18 18:20:56 +0000
+++ plugin/simple_user_policy/module.cc	2010-07-06 08:14:36 +0000
@@ -36,4 +36,4 @@
 
 } /* namespace simple_user_policy */
 
-DRIZZLE_PLUGIN(simple_user_policy::init, NULL);
+DRIZZLE_PLUGIN(simple_user_policy::init, NULL, NULL);

=== modified file 'plugin/sleep/sleep.cc'
--- plugin/sleep/sleep.cc	2010-05-18 18:20:56 +0000
+++ plugin/sleep/sleep.cc	2010-07-06 08:14:36 +0000
@@ -136,4 +136,4 @@
   return 0;
 }
 
-DRIZZLE_PLUGIN(sleep_plugin_init, NULL);
+DRIZZLE_PLUGIN(sleep_plugin_init, NULL, NULL);

=== modified file 'plugin/syslog/module.cc'
--- plugin/syslog/module.cc	2010-06-24 03:15:21 +0000
+++ plugin/syslog/module.cc	2010-07-06 08:14:36 +0000
@@ -157,4 +157,4 @@
 
 } // namespace syslog_module
 
-DRIZZLE_PLUGIN(syslog_module::init, syslog_module::system_variables);
+DRIZZLE_PLUGIN(syslog_module::init, syslog_module::system_variables, NULL);

=== modified file 'plugin/transaction_log/module.cc'
--- plugin/transaction_log/module.cc	2010-05-18 18:20:56 +0000
+++ plugin/transaction_log/module.cc	2010-07-06 08:14:36 +0000
@@ -292,4 +292,4 @@
   NULL
 };
 
-DRIZZLE_PLUGIN(init, sys_variables);
+DRIZZLE_PLUGIN(init, sys_variables, NULL);

=== modified file 'plugin/version/versionudf.cc'
--- plugin/version/versionudf.cc	2010-05-18 18:20:56 +0000
+++ plugin/version/versionudf.cc	2010-07-06 08:14:36 +0000
@@ -60,4 +60,4 @@
   return 0;
 }
 
-DRIZZLE_PLUGIN(initialize, NULL);
+DRIZZLE_PLUGIN(initialize, NULL, NULL);

=== modified file 'tests/test-run.pl'
--- tests/test-run.pl	2010-06-19 19:28:47 +0000
+++ tests/test-run.pl	2010-07-06 08:14:36 +0000
@@ -1497,7 +1497,7 @@
   $ENV{'SLAVE_MYPORT1'}=      $slave->[1]->{'port'};
   $ENV{'SLAVE_MYPORT2'}=      $slave->[2]->{'port'};
   $ENV{'MC_PORT'}=            $opt_memc_myport;
-  $ENV{'DRIZZLE_TCP_PORT'}=     $mysqld_variables{'drizzle-protocol-port'};
+  $ENV{'DRIZZLE_TCP_PORT'}=     $mysqld_variables{'drizzle-protocol.port'};
 
   $ENV{'MTR_BUILD_THREAD'}=      $opt_mtr_build_thread;
 
@@ -2574,7 +2574,7 @@
   mtr_add_arg($args, "%s--mysql-protocol-port=%d", $prefix,
               $mysqld->{'port'});
 
-  mtr_add_arg($args, "%s--drizzle-protocol-port=%d", $prefix,
+  mtr_add_arg($args, "%s--drizzle-protocol.port=%d", $prefix,
               $mysqld->{'secondary_port'});
 
   mtr_add_arg($args, "%s--datadir=%s", $prefix,
@@ -2739,6 +2739,7 @@
 
   if ( defined $exe )
   {
+    mtr_verbose("running Drizzle with: $exe @$args");
     $pid= mtr_spawn($exe, $args, "",
 		    $mysqld->{'path_myerr'},
 		    $mysqld->{'path_myerr'},