From eaf80c8d8ee5dd481bc85c8ebfcfa4c329b2e250 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Fri, 27 Jan 2012 16:06:39 -0800 Subject: [PATCH 1/4] Issue #233 refactor mysql_options calls and add an options method to the class. --- ext/mysql2/client.c | 73 ++++++++++++++++++++++++++++----------------- ext/mysql2/client.h | 2 +- 2 files changed, 47 insertions(+), 28 deletions(-) diff --git a/ext/mysql2/client.c b/ext/mysql2/client.c index 28f9cedf7..69e48e275 100644 --- a/ext/mysql2/client.c +++ b/ext/mysql2/client.c @@ -530,6 +530,49 @@ static VALUE rb_mysql_client_real_escape(VALUE self, VALUE str) { } } +static VALUE _mysql_client_options(VALUE self, int opt, VALUE value) { + int result; + void *retval = NULL; + unsigned int intval = 0; + my_bool boolean; + + GET_CLIENT(self); + + REQUIRE_OPEN_DB(wrapper); + + if (NIL_P(value)) + return Qfalse; + + switch(opt) { + case MYSQL_OPT_CONNECT_TIMEOUT: + intval = NUM2INT(value); + retval = &intval; + break; + + case MYSQL_OPT_LOCAL_INFILE: + case MYSQL_OPT_RECONNECT: + intval = (value == Qfalse ? 0 : 1); + retval = &intval; + break; + + default: + return Qfalse; + } + + result = mysql_options(wrapper->client, opt, retval); + if (result) + rb_warn("%s\n", mysql_error(wrapper->client)); + + // Zero means success + return INT2NUM(result == 0); +} + +static VALUE rb_mysql_client_options(VALUE self, VALUE option, VALUE value) { + Check_Type(option, T_FIXNUM); + int opt = NUM2INT(option); + return _mysql_client_options(self, opt, value); +} + static VALUE rb_mysql_client_info(VALUE self) { VALUE version, client_info; #ifdef HAVE_RUBY_ENCODING_H @@ -645,37 +688,12 @@ static VALUE rb_mysql_client_encoding(VALUE self) { #endif static VALUE set_reconnect(VALUE self, VALUE value) { - my_bool reconnect; - GET_CLIENT(self); + return _mysql_client_options(self, MYSQL_OPT_RECONNECT, value); - if(!NIL_P(value)) { - reconnect = value == Qfalse ? 0 : 1; - - wrapper->reconnect_enabled = reconnect; - /* set default reconnect behavior */ - if (mysql_options(wrapper->client, MYSQL_OPT_RECONNECT, &reconnect)) { - /* TODO: warning - unable to set reconnect behavior */ - rb_warn("%s\n", mysql_error(wrapper->client)); - } - } - return value; } static VALUE set_connect_timeout(VALUE self, VALUE value) { - unsigned int connect_timeout = 0; - GET_CLIENT(self); - - if(!NIL_P(value)) { - connect_timeout = NUM2INT(value); - if(0 == connect_timeout) return value; - - /* set default connection timeout behavior */ - if (mysql_options(wrapper->client, MYSQL_OPT_CONNECT_TIMEOUT, &connect_timeout)) { - /* TODO: warning - unable to set connection timeout */ - rb_warn("%s\n", mysql_error(wrapper->client)); - } - } - return value; + return _mysql_client_options(self, MYSQL_OPT_CONNECT_TIMEOUT, value); } static VALUE set_charset_name(VALUE self, VALUE value) { @@ -769,6 +787,7 @@ void init_mysql2_client() { rb_define_method(cMysql2Client, "affected_rows", rb_mysql_client_affected_rows, 0); rb_define_method(cMysql2Client, "thread_id", rb_mysql_client_thread_id, 0); rb_define_method(cMysql2Client, "ping", rb_mysql_client_ping, 0); + rb_define_method(cMysql2Client, "options", rb_mysql_client_options, 1); #ifdef HAVE_RUBY_ENCODING_H rb_define_method(cMysql2Client, "encoding", rb_mysql_client_encoding, 0); #endif diff --git a/ext/mysql2/client.h b/ext/mysql2/client.h index af4f76339..c86a2cec5 100644 --- a/ext/mysql2/client.h +++ b/ext/mysql2/client.h @@ -39,4 +39,4 @@ typedef struct { MYSQL *client; } mysql_client_wrapper; -#endif \ No newline at end of file +#endif From 54d452b6b182823139804bab5c30c98b0464739c Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Fri, 27 Jan 2012 16:08:18 -0800 Subject: [PATCH 2/4] Issue #233 add option :local_infile to the constructor. --- ext/mysql2/client.c | 4 ++++ lib/mysql2/client.rb | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/ext/mysql2/client.c b/ext/mysql2/client.c index 69e48e275..415232047 100644 --- a/ext/mysql2/client.c +++ b/ext/mysql2/client.c @@ -689,7 +689,10 @@ static VALUE rb_mysql_client_encoding(VALUE self) { static VALUE set_reconnect(VALUE self, VALUE value) { return _mysql_client_options(self, MYSQL_OPT_RECONNECT, value); +} +static VALUE set_local_infile(VALUE self, VALUE value) { + return _mysql_client_options(self, MYSQL_OPT_LOCAL_INFILE, value); } static VALUE set_connect_timeout(VALUE self, VALUE value) { @@ -794,6 +797,7 @@ void init_mysql2_client() { rb_define_private_method(cMysql2Client, "reconnect=", set_reconnect, 1); rb_define_private_method(cMysql2Client, "connect_timeout=", set_connect_timeout, 1); + rb_define_private_method(cMysql2Client, "local_infile=", set_local_infile, 1); rb_define_private_method(cMysql2Client, "charset_name=", set_charset_name, 1); rb_define_private_method(cMysql2Client, "ssl_set", set_ssl_options, 5); rb_define_private_method(cMysql2Client, "init_connection", init_connection, 0); diff --git a/lib/mysql2/client.rb b/lib/mysql2/client.rb index 6fb693ef4..93fbcf77c 100644 --- a/lib/mysql2/client.rb +++ b/lib/mysql2/client.rb @@ -19,10 +19,12 @@ def initialize(opts = {}) init_connection - [:reconnect, :connect_timeout].each do |key| + # Set MySQL connection options (each one is a call to mysql_options()) + [:reconnect, :connect_timeout, :local_infile].each do |key| next unless opts.key?(key) send(:"#{key}=", opts[key]) end + # force the encoding to utf8 self.charset_name = opts[:encoding] || 'utf8' From 67a5e63804648c4fb98a0046eb31c85509021fe6 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Fri, 27 Jan 2012 16:41:29 -0800 Subject: [PATCH 3/4] List of valid Mysql2::Client options. --- README.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/README.md b/README.md index 111e2d1a1..91733b11b 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,27 @@ results.each(:as => :array) do |row| end ``` +## Connection options + +You may set the following connection options in Mysql2::Client.new(...): + +``` ruby +Mysql2::Client.new( + :host, + :username, + :password, + :port, + :database, + :socket = '/path/to/mysql.sock', + :flags = REMEMBER_OPTIONS | LONG_PASSWORD | LONG_FLAG | TRANSACTIONS | PROTOCOL_41 | SECURE_CONNECTION, + :encoding = 'utf8', + :read_timeout = seconds, + :connect_timeout = seconds, + :reconnect = true/false, + :local_infile = true/false, + ) +``` + ## Cascading config The default config hash is at: From ab19d053f4e4d3f716e15f3d2f6c361eebf68b48 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Wed, 4 Apr 2012 16:05:24 -0700 Subject: [PATCH 4/4] Issue #233 fixes to the options refactor plus tests now pass. --- ext/mysql2/client.c | 78 +++++++++++++++++++++++++------------- ext/mysql2/client.h | 5 ++- lib/mysql2/client.rb | 2 +- spec/mysql2/client_spec.rb | 10 ++++- 4 files changed, 64 insertions(+), 31 deletions(-) diff --git a/ext/mysql2/client.c b/ext/mysql2/client.c index 0b7b33fb4..120764fe0 100644 --- a/ext/mysql2/client.c +++ b/ext/mysql2/client.c @@ -12,11 +12,23 @@ static VALUE intern_encoding_from_charset; static VALUE sym_id, sym_version, sym_async, sym_symbolize_keys, sym_as, sym_array, sym_stream; static ID intern_merge, intern_error_number_eql, intern_sql_state_eql; -#define REQUIRE_OPEN_DB(wrapper) \ - if(!wrapper->reconnect_enabled && wrapper->closed) { \ +#define REQUIRE_INITIALIZED(wrapper) \ + if (!wrapper->initialized) { \ + rb_raise(cMysql2Error, "MySQL client is not initialized"); \ + } + +#define REQUIRE_CONNECTED(wrapper) \ + REQUIRE_INITIALIZED(wrapper) \ + if (!wrapper->connected && !wrapper->reconnect_enabled) { \ rb_raise(cMysql2Error, "closed MySQL connection"); \ } +#define REQUIRE_NOT_CONNECTED(wrapper) \ + REQUIRE_INITIALIZED(wrapper) \ + if (wrapper->connected) { \ + rb_raise(cMysql2Error, "MySQL connection is already open"); \ + } + #define MARK_CONN_INACTIVE(conn) \ wrapper->active = 0 @@ -137,8 +149,8 @@ static VALUE nogvl_close(void *ptr) { int flags; #endif wrapper = ptr; - if (!wrapper->closed) { - wrapper->closed = 1; + if (wrapper->connected) { + wrapper->connected = 0; wrapper->active = 0; /* * we'll send a QUIT message to the server, but that message is more of a @@ -175,9 +187,10 @@ static VALUE allocate(VALUE klass) { mysql_client_wrapper * wrapper; obj = Data_Make_Struct(klass, mysql_client_wrapper, rb_mysql_client_mark, rb_mysql_client_free, wrapper); wrapper->encoding = Qnil; - wrapper->active = 0; wrapper->reconnect_enabled = 0; - wrapper->closed = 1; + wrapper->active = 0; // active: means that a query is active + wrapper->connected = 0; // means that a database connection is open + wrapper->initialized = 0; // means that that the wrapper is initialized wrapper->client = (MYSQL*)xmalloc(sizeof(MYSQL)); return obj; } @@ -231,6 +244,7 @@ static VALUE rb_connect(VALUE self, VALUE user, VALUE pass, VALUE host, VALUE po return rb_raise_mysql2_error(wrapper); } + wrapper->connected = 1; return self; } @@ -243,7 +257,7 @@ static VALUE rb_connect(VALUE self, VALUE user, VALUE pass, VALUE host, VALUE po static VALUE rb_mysql_client_close(VALUE self) { GET_CLIENT(self); - if (!wrapper->closed) { + if (wrapper->connected) { rb_thread_blocking_region(nogvl_close, wrapper, RUBY_UBF_IO, 0); } @@ -330,7 +344,7 @@ static VALUE rb_mysql_client_async_result(VALUE self) { if (!wrapper->active) return Qnil; - REQUIRE_OPEN_DB(wrapper); + REQUIRE_CONNECTED(wrapper); if (rb_thread_blocking_region(nogvl_read_query_result, wrapper->client, RUBY_UBF_IO, 0) == Qfalse) { // an error occurred, mark this connection inactive MARK_CONN_INACTIVE(self); @@ -373,7 +387,7 @@ struct async_query_args { static VALUE disconnect_and_raise(VALUE self, VALUE error) { GET_CLIENT(self); - wrapper->closed = 1; + wrapper->connected = 0; wrapper->active = 0; // manually close the socket for read/write @@ -470,7 +484,7 @@ static VALUE rb_mysql_client_query(int argc, VALUE * argv, VALUE self) { #endif GET_CLIENT(self); - REQUIRE_OPEN_DB(wrapper); + REQUIRE_CONNECTED(wrapper); args.mysql = wrapper->client; @@ -541,7 +555,7 @@ static VALUE rb_mysql_client_real_escape(VALUE self, VALUE str) { #endif GET_CLIENT(self); - REQUIRE_OPEN_DB(wrapper); + REQUIRE_CONNECTED(wrapper); Check_Type(str, T_STRING); #ifdef HAVE_RUBY_ENCODING_H default_internal_enc = rb_default_internal_encoding(); @@ -575,11 +589,11 @@ static VALUE _mysql_client_options(VALUE self, int opt, VALUE value) { int result; void *retval = NULL; unsigned int intval = 0; - my_bool boolean; + my_bool boolval; GET_CLIENT(self); - REQUIRE_OPEN_DB(wrapper); + REQUIRE_NOT_CONNECTED(wrapper); if (NIL_P(value)) return Qfalse; @@ -591,20 +605,30 @@ static VALUE _mysql_client_options(VALUE self, int opt, VALUE value) { break; case MYSQL_OPT_LOCAL_INFILE: - case MYSQL_OPT_RECONNECT: intval = (value == Qfalse ? 0 : 1); retval = &intval; break; + case MYSQL_OPT_RECONNECT: + boolval = (value == Qfalse ? 0 : 1); + retval = &boolval; + break; + default: return Qfalse; } result = mysql_options(wrapper->client, opt, retval); - if (result) - rb_warn("%s\n", mysql_error(wrapper->client)); // Zero means success + if (result != 0) { + rb_warn("%s\n", mysql_error(wrapper->client)); + } else { + // Special case for reconnect, this option is also stored in the wrapper struct + if (opt == MYSQL_OPT_RECONNECT) + wrapper->reconnect_enabled = boolval; + } + return INT2NUM(result == 0); } @@ -658,7 +682,7 @@ static VALUE rb_mysql_client_server_info(VALUE self) { #endif GET_CLIENT(self); - REQUIRE_OPEN_DB(wrapper); + REQUIRE_CONNECTED(wrapper); #ifdef HAVE_RUBY_ENCODING_H default_internal_enc = rb_default_internal_encoding(); conn_enc = rb_to_encoding(wrapper->encoding); @@ -685,7 +709,7 @@ static VALUE rb_mysql_client_server_info(VALUE self) { static VALUE rb_mysql_client_socket(VALUE self) { GET_CLIENT(self); #ifndef _WIN32 - REQUIRE_OPEN_DB(wrapper); + REQUIRE_CONNECTED(wrapper); int fd_set_fd = wrapper->client->net.fd; return INT2NUM(fd_set_fd); #else @@ -701,7 +725,7 @@ static VALUE rb_mysql_client_socket(VALUE self) { */ static VALUE rb_mysql_client_last_id(VALUE self) { GET_CLIENT(self); - REQUIRE_OPEN_DB(wrapper); + REQUIRE_CONNECTED(wrapper); return ULL2NUM(mysql_insert_id(wrapper->client)); } @@ -715,7 +739,7 @@ static VALUE rb_mysql_client_affected_rows(VALUE self) { my_ulonglong retVal; GET_CLIENT(self); - REQUIRE_OPEN_DB(wrapper); + REQUIRE_CONNECTED(wrapper); retVal = mysql_affected_rows(wrapper->client); if (retVal == (my_ulonglong)-1) { rb_raise_mysql2_error(wrapper); @@ -732,7 +756,7 @@ static VALUE rb_mysql_client_thread_id(VALUE self) { unsigned long retVal; GET_CLIENT(self); - REQUIRE_OPEN_DB(wrapper); + REQUIRE_CONNECTED(wrapper); retVal = mysql_thread_id(wrapper->client); return ULL2NUM(retVal); } @@ -757,7 +781,7 @@ static VALUE rb_mysql_client_select_db(VALUE self, VALUE db) struct nogvl_select_db_args args; GET_CLIENT(self); - REQUIRE_OPEN_DB(wrapper); + REQUIRE_CONNECTED(wrapper); args.mysql = wrapper->client; args.db = StringValuePtr(db); @@ -785,7 +809,7 @@ static VALUE nogvl_ping(void *ptr) { static VALUE rb_mysql_client_ping(VALUE self) { GET_CLIENT(self); - if (wrapper->closed) { + if (!wrapper->connected) { return Qfalse; } else { return rb_thread_blocking_region(nogvl_ping, wrapper->client, RUBY_UBF_IO, 0); @@ -918,7 +942,7 @@ static VALUE set_ssl_options(VALUE self, VALUE key, VALUE cert, VALUE ca, VALUE return self; } -static VALUE init_connection(VALUE self) { +static VALUE initialize_ext(VALUE self) { GET_CLIENT(self); if (rb_thread_blocking_region(nogvl_init, wrapper->client, RUBY_UBF_IO, 0) == Qfalse) { @@ -926,7 +950,7 @@ static VALUE init_connection(VALUE self) { return rb_raise_mysql2_error(wrapper); } - wrapper->closed = 0; + wrapper->initialized = 1; return self; } @@ -968,7 +992,7 @@ void init_mysql2_client() { rb_define_method(cMysql2Client, "affected_rows", rb_mysql_client_affected_rows, 0); rb_define_method(cMysql2Client, "thread_id", rb_mysql_client_thread_id, 0); rb_define_method(cMysql2Client, "ping", rb_mysql_client_ping, 0); - rb_define_method(cMysql2Client, "options", rb_mysql_client_options, 1); + rb_define_method(cMysql2Client, "options", rb_mysql_client_options, 2); rb_define_method(cMysql2Client, "select_db", rb_mysql_client_select_db, 1); rb_define_method(cMysql2Client, "more_results", rb_mysql_client_more_results, 0); rb_define_method(cMysql2Client, "next_result", rb_mysql_client_next_result, 0); @@ -982,7 +1006,7 @@ void init_mysql2_client() { rb_define_private_method(cMysql2Client, "local_infile=", set_local_infile, 1); rb_define_private_method(cMysql2Client, "charset_name=", set_charset_name, 1); rb_define_private_method(cMysql2Client, "ssl_set", set_ssl_options, 5); - rb_define_private_method(cMysql2Client, "init_connection", init_connection, 0); + rb_define_private_method(cMysql2Client, "initialize_ext", initialize_ext, 0); rb_define_private_method(cMysql2Client, "connect", rb_connect, 7); intern_encoding_from_charset = rb_intern("encoding_from_charset"); diff --git a/ext/mysql2/client.h b/ext/mysql2/client.h index 813520cab..c49f0cfa2 100644 --- a/ext/mysql2/client.h +++ b/ext/mysql2/client.h @@ -33,9 +33,10 @@ void init_mysql2_client(); typedef struct { VALUE encoding; - int active; int reconnect_enabled; - int closed; + int active; + int connected; + int initialized; MYSQL *client; } mysql_client_wrapper; diff --git a/lib/mysql2/client.rb b/lib/mysql2/client.rb index 93fbcf77c..d7b4f01cd 100644 --- a/lib/mysql2/client.rb +++ b/lib/mysql2/client.rb @@ -17,7 +17,7 @@ def initialize(opts = {}) @query_options = @@default_query_options.dup @query_options.merge! opts - init_connection + initialize_ext # Set MySQL connection options (each one is a call to mysql_options()) [:reconnect, :connect_timeout, :local_infile].each do |key| diff --git a/spec/mysql2/client_spec.rb b/spec/mysql2/client_spec.rb index dd45ab37f..351099098 100644 --- a/spec/mysql2/client_spec.rb +++ b/spec/mysql2/client_spec.rb @@ -254,8 +254,16 @@ def connect *args result = @client.async_result result.class.should eql(Mysql2::Result) end + + it "should not allow options to be set on an open connection" do + lambda { + @client.escape "" + @client.query("SELECT 1") + @client.options(0, 0) + }.should raise_error(Mysql2::Error) + end end - + context "Multiple results sets" do before(:each) do @multi_client = Mysql2::Client.new( :flags => Mysql2::Client::MULTI_STATEMENTS)