From cd7c77449bb960b2f341892879fc5ea82a98365b Mon Sep 17 00:00:00 2001 From: Andy Pfister Date: Tue, 9 Sep 2025 14:51:53 +0200 Subject: [PATCH 1/3] `TinyTds::Client.new` now accepts keyword arguments instead of a hash --- CHANGELOG.md | 2 + README.md | 4 +- ext/tiny_tds/client.c | 87 +++++++++++++++--------------------------- lib/tiny_tds/client.rb | 54 +++++++++++++------------- test/client_test.rb | 16 +++----- test/result_test.rb | 3 +- test/test_helper.rb | 6 +-- 7 files changed, 70 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fcc6d41..1317fac8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ * `TinyTds::Result` is now a pure Ruby class * `#execute`: Replaced `opts` hash with keyword arguments * Removed `symbolize_keys` and `cache_rows` from `#default_query_options` +* `TinyTds::Client.new` now accepts keyword arguments instead of a hash +* Renamed `tds_version` and `tds_version_info` to `server_version` and `server_version_info`. ## 3.3.0 diff --git a/README.md b/README.md index 7bc697d1..ce86ecad 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ Connect to a database. client = TinyTds::Client.new username: 'sa', password: 'secret', host: 'mydb.host.net' ``` -Creating a new client takes a hash of options. For valid iconv encoding options, see the output of `iconv -l`. Only a few have been tested, and are highly recommended to leave blank for the UTF-8 default. +Creating a new client takes keyword arguments. For valid iconv encoding options, see the output of `iconv -l`. Only a few have been tested, and are highly recommended to leave blank for the UTF-8 default. * :username - The database server user. * :password - The user password. @@ -106,7 +106,7 @@ Creating a new client takes a hash of options. For valid iconv encoding options, * :host - Used if :dataserver blank. Can be an host name or IP. * :port - Defaults to 1433. Only used if :host is used. * :database - The default database to use. -* :appname - Short string seen in SQL Servers process/activity window. +* :app_name - Short string seen in SQL Servers process/activity window. * :tds_version - TDS version. Defaults to "7.3". * :login_timeout - Seconds to wait for login. Default to 60 seconds. * :timeout - Seconds to wait for a response to a SQL command. Default 5 seconds. Timeouts caused by network failure will raise a timeout error 1 second after the configured timeout limit is hit (see [#481](https://github.com/rails-sqlserver/tiny_tds/pull/481) for details). diff --git a/ext/tiny_tds/client.c b/ext/tiny_tds/client.c index 5e6b2191..e8de1fb1 100644 --- a/ext/tiny_tds/client.c +++ b/ext/tiny_tds/client.c @@ -4,7 +4,6 @@ VALUE cTinyTdsClient; extern VALUE mTinyTds, cTinyTdsError; -static ID sym_username, sym_password, sym_dataserver, sym_database, sym_appname, sym_tds_version, sym_login_timeout, sym_timeout, sym_encoding, sym_azure, sym_contained, sym_use_utf16, sym_message_handler; static ID intern_source_eql, intern_severity_eql, intern_db_error_number_eql, intern_os_error_number_eql; static ID intern_new, intern_dup, intern_transpose_iconv_encoding, intern_local_offset, intern_gsub, intern_call; VALUE opt_escape_regex, opt_escape_dblquote; @@ -470,7 +469,7 @@ static VALUE allocate(VALUE klass) // TinyTds::Client (public) -static VALUE rb_tinytds_tds_version(VALUE self) +static VALUE rb_tinytds_server_version(VALUE self) { GET_CLIENT_WRAPPER(self); return INT2FIX(dbtds(cwrap->client)); @@ -925,25 +924,25 @@ static VALUE rb_tinytds_identity_sql(VALUE self) // TinyTds::Client (protected) -static VALUE rb_tinytds_connect(VALUE self, VALUE opts) +static VALUE rb_tinytds_connect(VALUE self) { /* Parsing options hash to local vars. */ - VALUE user, pass, dataserver, database, app, version, ltimeout, timeout, charset, azure, contained, use_utf16; + VALUE username, password, dataserver, database, app_name, tds_version, login_timeout, timeout, charset, azure, contained, use_utf16; GET_CLIENT_WRAPPER(self); - user = rb_hash_aref(opts, sym_username); - pass = rb_hash_aref(opts, sym_password); - dataserver = rb_hash_aref(opts, sym_dataserver); - database = rb_hash_aref(opts, sym_database); - app = rb_hash_aref(opts, sym_appname); - version = rb_hash_aref(opts, sym_tds_version); - ltimeout = rb_hash_aref(opts, sym_login_timeout); - timeout = rb_hash_aref(opts, sym_timeout); - charset = rb_hash_aref(opts, sym_encoding); - azure = rb_hash_aref(opts, sym_azure); - contained = rb_hash_aref(opts, sym_contained); - use_utf16 = rb_hash_aref(opts, sym_use_utf16); - cwrap->userdata->message_handler = rb_hash_aref(opts, sym_message_handler); + app_name = rb_iv_get(self, "@app_name"); + azure = rb_iv_get(self, "@azure"); + contained = rb_iv_get(self, "@contained"); + database = rb_iv_get(self, "@database"); + dataserver = rb_iv_get(self, "@dataserver"); + charset = rb_iv_get(self, "@encoding"); + login_timeout = rb_iv_get(self, "@login_timeout"); + password = rb_iv_get(self, "@password"); + tds_version = rb_iv_get(self, "@tds_version"); + timeout = rb_iv_get(self, "@timeout"); + username = rb_iv_get(self, "@username"); + use_utf16 = rb_iv_get(self, "@use_utf16"); + cwrap->userdata->message_handler = rb_iv_get(self, "@message_handler"); /* Dealing with options. */ if (dbinit() == FAIL) { @@ -955,24 +954,24 @@ static VALUE rb_tinytds_connect(VALUE self, VALUE opts) dbmsghandle(tinytds_msg_handler); cwrap->login = dblogin(); - if (!NIL_P(version)) { - dbsetlversion(cwrap->login, NUM2INT(version)); + if (!NIL_P(tds_version)) { + dbsetlversion(cwrap->login, NUM2INT(tds_version)); } - if (!NIL_P(user)) { - dbsetluser(cwrap->login, StringValueCStr(user)); + if (!NIL_P(username)) { + dbsetluser(cwrap->login, StringValueCStr(username)); } - if (!NIL_P(pass)) { - dbsetlpwd(cwrap->login, StringValueCStr(pass)); + if (!NIL_P(password)) { + dbsetlpwd(cwrap->login, StringValueCStr(password)); } - if (!NIL_P(app)) { - dbsetlapp(cwrap->login, StringValueCStr(app)); + if (!NIL_P(app_name)) { + dbsetlapp(cwrap->login, StringValueCStr(app_name)); } - if (!NIL_P(ltimeout)) { - dbsetlogintime(NUM2INT(ltimeout)); + if (!NIL_P(login_timeout)) { + dbsetlogintime(NUM2INT(login_timeout)); } if (!NIL_P(charset)) { @@ -981,19 +980,7 @@ static VALUE rb_tinytds_connect(VALUE self, VALUE opts) if (!NIL_P(database)) { if (azure == Qtrue || contained == Qtrue) { - #ifdef DBSETLDBNAME DBSETLDBNAME(cwrap->login, StringValueCStr(database)); - #else - - if (azure == Qtrue) { - rb_warn("TinyTds: :azure option is not supported in this version of FreeTDS.\n"); - } - - if (contained == Qtrue) { - rb_warn("TinyTds: :contained option is not supported in this version of FreeTDS.\n"); - } - - #endif } } @@ -1017,8 +1004,8 @@ static VALUE rb_tinytds_connect(VALUE self, VALUE opts) cwrap->closed = 0; cwrap->charset = charset; - if (!NIL_P(version)) { - dbsetversion(NUM2INT(version)); + if (!NIL_P(tds_version)) { + dbsetversion(NUM2INT(tds_version)); } if (!NIL_P(timeout)) { @@ -1053,7 +1040,7 @@ void init_tinytds_client() cTinyTdsClient = rb_define_class_under(mTinyTds, "Client", rb_cObject); rb_define_alloc_func(cTinyTdsClient, allocate); /* Define TinyTds::Client Public Methods */ - rb_define_method(cTinyTdsClient, "tds_version", rb_tinytds_tds_version, 0); + rb_define_method(cTinyTdsClient, "server_version", rb_tinytds_server_version, 0); rb_define_method(cTinyTdsClient, "close", rb_tinytds_close, 0); rb_define_method(cTinyTdsClient, "closed?", rb_tinytds_closed, 0); rb_define_method(cTinyTdsClient, "canceled?", rb_tinytds_canceled, 0); @@ -1068,21 +1055,7 @@ void init_tinytds_client() rb_define_method(cTinyTdsClient, "return_code", rb_tinytds_return_code, 0); rb_define_method(cTinyTdsClient, "identity_sql", rb_tinytds_identity_sql, 0); /* Define TinyTds::Client Protected Methods */ - rb_define_protected_method(cTinyTdsClient, "connect", rb_tinytds_connect, 1); - /* Symbols For Connect */ - sym_username = ID2SYM(rb_intern("username")); - sym_password = ID2SYM(rb_intern("password")); - sym_dataserver = ID2SYM(rb_intern("dataserver")); - sym_database = ID2SYM(rb_intern("database")); - sym_appname = ID2SYM(rb_intern("appname")); - sym_tds_version = ID2SYM(rb_intern("tds_version")); - sym_login_timeout = ID2SYM(rb_intern("login_timeout")); - sym_timeout = ID2SYM(rb_intern("timeout")); - sym_encoding = ID2SYM(rb_intern("encoding")); - sym_azure = ID2SYM(rb_intern("azure")); - sym_contained = ID2SYM(rb_intern("contained")); - sym_use_utf16 = ID2SYM(rb_intern("use_utf16")); - sym_message_handler = ID2SYM(rb_intern("message_handler")); + rb_define_protected_method(cTinyTdsClient, "connect", rb_tinytds_connect, 0); /* Intern TinyTds::Error Accessors */ intern_source_eql = rb_intern("source="); intern_severity_eql = rb_intern("severity="); diff --git a/lib/tiny_tds/client.rb b/lib/tiny_tds/client.rb index 3631a51a..f9ffd6cc 100644 --- a/lib/tiny_tds/client.rb +++ b/lib/tiny_tds/client.rb @@ -1,5 +1,7 @@ module TinyTds class Client + attr_reader :app_name, :contained, :database, :dataserver, :encoding, :message_handler, :login_timeout, :password, :port, :tds_version, :timeout, :username, :use_utf16 + @default_query_options = { as: :hash, empty_sets: true, @@ -7,7 +9,6 @@ class Client } attr_reader :query_options - attr_reader :message_handler class << self attr_reader :default_query_options @@ -29,37 +30,38 @@ def local_offset # rubocop:disable Metrics/MethodLength # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/PerceivedComplexity - def initialize(opts = {}) - if opts[:dataserver].to_s.empty? && opts[:host].to_s.empty? + def initialize(app_name: "TinyTds", azure: false, contained: false, database: nil, dataserver: nil, encoding: "UTF-8", message_handler: nil, host: nil, login_timeout: 60, password: nil, port: 1433, tds_version: nil, timeout: 5, username: nil, use_utf16: true) + if dataserver.to_s.empty? && host.to_s.empty? raise ArgumentError, "missing :host option if no :dataserver given" end - @message_handler = opts[:message_handler] - if @message_handler && !@message_handler.respond_to?(:call) + if message_handler && !message_handler.respond_to?(:call) raise ArgumentError, ":message_handler must implement `call` (eg, a Proc or a Method)" + else + @message_handler = message_handler end - opts[:username] = parse_username(opts) - opts[:password] = opts[:password].to_s if opts[:password] && opts[:password].to_s.strip != "" - opts[:appname] ||= "TinyTds" - opts[:tds_version] = tds_versions_setter(opts) - opts[:use_utf16] = opts[:use_utf16].nil? || ["true", "1", "yes"].include?(opts[:use_utf16].to_s) - opts[:login_timeout] ||= 60 - opts[:timeout] ||= 5 - opts[:encoding] = (opts[:encoding].nil? || opts[:encoding].casecmp("utf8").zero?) ? "UTF-8" : opts[:encoding].upcase - opts[:port] ||= 1433 - opts[:dataserver] = "#{opts[:host]}:#{opts[:port]}" if opts[:dataserver].to_s.empty? - forced_integer_keys = [:login_timeout, :port, :timeout] - forced_integer_keys.each { |k| opts[k] = opts[k].to_i if opts[k] } - connect(opts) + @app_name = app_name + @database = database + @dataserver = dataserver || "#{host}:#{port}" + @encoding = (encoding.nil? || encoding.casecmp("utf8").zero?) ? "UTF-8" : encoding.upcase + @login_timeout = login_timeout.to_i + @password = password if password && password.to_s.strip != "" + @port = port.to_i + @timeout = timeout.to_i + @tds_version = tds_versions_setter(tds_version:) + @username = parse_username(azure:, host:, username:) + @use_utf16 = use_utf16.nil? || ["true", "1", "yes"].include?(use_utf16.to_s) + + connect end def tds_73? - tds_version >= 11 + server_version >= 11 end - def tds_version_info - info = TDS_VERSIONS_GETTERS[tds_version] + def server_version_info + info = TDS_VERSIONS_GETTERS[server_version] "#{info[:name]} - #{info[:description]}" if info end @@ -69,18 +71,16 @@ def active? private - def parse_username(opts) - host = opts[:host] - username = opts[:username] - return username if username.nil? || !opts[:azure] + def parse_username(azure: false, host: nil, username:) + return username if username.nil? || !azure return username if username.include?("@") && !username.include?("database.windows.net") user, domain = username.split("@") domain ||= host "#{user}@#{domain.split(".").first}" end - def tds_versions_setter(opts = {}) - v = opts[:tds_version] || ENV["TDSVER"] || "7.3" + def tds_versions_setter(tds_version:) + v = tds_version || ENV["TDSVER"] || "7.3" TDS_VERSIONS_SETTERS[v.to_s] end diff --git a/test/client_test.rb b/test/client_test.rb index 5d3ef7f5..e48c5cda 100644 --- a/test/client_test.rb +++ b/test/client_test.rb @@ -26,14 +26,9 @@ class ClientTest < TinyTds::TestCase end end - it "has getters for the tds version information (brittle since conf takes precedence)" do - if @client.tds_73? - assert_equal 11, @client.tds_version - assert_equal "DBTDS_7_3 - Microsoft SQL Server 2008", @client.tds_version_info - else - assert_equal 9, @client.tds_version - assert_equal "DBTDS_7_1/DBTDS_8_0 - Microsoft SQL Server 2000", @client.tds_version_info - end + it "has getters for the server version information (brittle since conf takes precedence)" do + assert_equal 11, @client.server_version + assert_equal "DBTDS_7_3 - Microsoft SQL Server 2008", @client.server_version_info end it "uses UTF-8 client charset/encoding by default" do @@ -82,8 +77,7 @@ class ClientTest < TinyTds::TestCase end it "raises TinyTds exception with undefined :dataserver" do - options = connection_options login_timeout: 1, dataserver: "DOESNOTEXIST" - action = lambda { new_connection(options) } + action = lambda { new_connection(login_timeout: 1, dataserver: "DOESNOTEXIST") } assert_raise_tinytds_error(action) do |e| # Not sure why tese are different. if ruby_darwin? @@ -193,7 +187,7 @@ class ClientTest < TinyTds::TestCase it "raises TinyTds exception with wrong :username" do skip if ENV["CI"] && sqlserver_azure? # Some issue with db_error_number. options = connection_options username: "willnotwork" - action = lambda { new_connection(options) } + action = lambda { new_connection(**options) } assert_raise_tinytds_error(action) do |e| assert_equal 18456, e.db_error_number assert_equal 14, e.severity diff --git a/test/result_test.rb b/test/result_test.rb index 1f8916f1..6babe06c 100644 --- a/test/result_test.rb +++ b/test/result_test.rb @@ -142,9 +142,8 @@ class ResultTest < TinyTds::TestCase it "works in tandem with the client when needing to find out if client has sql sent and result is canceled or not" do # Default state. - @client = TinyTds::Client.new(connection_options) _(@client.sqlsent?).must_equal false - _(@client.canceled?).must_equal false + _(@client.canceled?).must_equal true # With active result before and after cancel. @client.execute(@query1) _(@client.sqlsent?).must_equal false diff --git a/test/test_helper.rb b/test/test_helper.rb index 88982531..b637a931 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -41,8 +41,8 @@ def close_client(client = @client) client.close if defined?(client) && client.is_a?(TinyTds::Client) end - def new_connection(options = {}) - client = TinyTds::Client.new(connection_options(options)) + def new_connection(**options) + client = TinyTds::Client.new(**connection_options(options)) if sqlserver_azure? client.do("SET ANSI_NULLS ON") client.do("SET CURSOR_CLOSE_ON_COMMIT OFF") @@ -71,7 +71,7 @@ def connection_options(options = {}) username: username, password: password, database: ENV["TINYTDS_UNIT_DATABASE"] || "tinytdstest", - appname: "TinyTds Dev", + app_name: "TinyTds Dev", login_timeout: 5, timeout: connection_timeout, azure: sqlserver_azure?}.merge(options) From 206898d4cd6cdb8cbbe2b69a11afcf638e160e2d Mon Sep 17 00:00:00 2001 From: Andy Pfister Date: Tue, 9 Sep 2025 15:14:25 +0200 Subject: [PATCH 2/3] Separate `#new` and `#connect` --- CHANGELOG.md | 4 +++- ext/tiny_tds/client.c | 30 +++++++++++++++++++++++++----- lib/tiny_tds/client.rb | 4 +--- test/client_test.rb | 29 ++++++++++++----------------- test/test_helper.rb | 1 + 5 files changed, 42 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1317fac8..1642c239 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,9 @@ * `#execute`: Replaced `opts` hash with keyword arguments * Removed `symbolize_keys` and `cache_rows` from `#default_query_options` * `TinyTds::Client.new` now accepts keyword arguments instead of a hash -* Renamed `tds_version` and `tds_version_info` to `server_version` and `server_version_info`. +* Renamed `tds_version` and `tds_version_info` to `server_version` and `server_version_info` +* Separate `#new` and `#connect` + * Instead, before running `#do`, `#execute` or `#insert`, `tiny_tds` will check if the connection is active and re-connect if needed. ## 3.3.0 diff --git a/ext/tiny_tds/client.c b/ext/tiny_tds/client.c index e8de1fb1..2ed5b765 100644 --- a/ext/tiny_tds/client.c +++ b/ext/tiny_tds/client.c @@ -5,7 +5,7 @@ VALUE cTinyTdsClient; extern VALUE mTinyTds, cTinyTdsError; static ID intern_source_eql, intern_severity_eql, intern_db_error_number_eql, intern_os_error_number_eql; -static ID intern_new, intern_dup, intern_transpose_iconv_encoding, intern_local_offset, intern_gsub, intern_call; +static ID intern_new, intern_dup, intern_transpose_iconv_encoding, intern_local_offset, intern_gsub, intern_call, intern_active, intern_connect; VALUE opt_escape_regex, opt_escape_dblquote; static ID id_ivar_fields, id_ivar_rows, id_ivar_return_code, id_ivar_affected_rows, id_ivar_default_query_options, intern_bigd, intern_divide; @@ -468,11 +468,15 @@ static VALUE allocate(VALUE klass) // TinyTds::Client (public) - static VALUE rb_tinytds_server_version(VALUE self) { GET_CLIENT_WRAPPER(self); - return INT2FIX(dbtds(cwrap->client)); + + if (rb_funcall(self, intern_active, 0) == Qtrue) { + return INT2FIX(dbtds(cwrap->client)); + } else { + return Qnil; + } } static VALUE rb_tinytds_close(VALUE self) @@ -745,6 +749,11 @@ static VALUE rb_tinytds_execute(int argc, VALUE *argv, VALUE self) } GET_CLIENT_WRAPPER(self); + + if (rb_funcall(self, intern_active, 0) == Qfalse) { + rb_funcall(self, intern_connect, 0); + } + rb_tinytds_send_sql_to_server(cwrap, sql); VALUE result = rb_obj_alloc(cTinyTdsResult); @@ -856,6 +865,11 @@ static VALUE rb_tiny_tds_insert(VALUE self, VALUE sql) { VALUE identity = Qnil; GET_CLIENT_WRAPPER(self); + + if (rb_funcall(self, intern_active, 0) == Qfalse) { + rb_funcall(self, intern_connect, 0); + } + rb_tinytds_send_sql_to_server(cwrap, sql); rb_tinytds_result_exec_helper(cwrap->client); @@ -885,6 +899,11 @@ static VALUE rb_tiny_tds_insert(VALUE self, VALUE sql) static VALUE rb_tiny_tds_do(VALUE self, VALUE sql) { GET_CLIENT_WRAPPER(self); + + if (rb_funcall(self, intern_active, 0) == Qfalse) { + rb_funcall(self, intern_connect, 0); + } + rb_tinytds_send_sql_to_server(cwrap, sql); rb_tinytds_result_exec_helper(cwrap->client); @@ -1054,8 +1073,7 @@ void init_tinytds_client() rb_define_method(cTinyTdsClient, "escape", rb_tinytds_escape, 1); rb_define_method(cTinyTdsClient, "return_code", rb_tinytds_return_code, 0); rb_define_method(cTinyTdsClient, "identity_sql", rb_tinytds_identity_sql, 0); - /* Define TinyTds::Client Protected Methods */ - rb_define_protected_method(cTinyTdsClient, "connect", rb_tinytds_connect, 0); + rb_define_method(cTinyTdsClient, "connect", rb_tinytds_connect, 0); /* Intern TinyTds::Error Accessors */ intern_source_eql = rb_intern("source="); intern_severity_eql = rb_intern("severity="); @@ -1088,6 +1106,8 @@ void init_tinytds_client() intern_timezone = rb_intern("timezone"); intern_utc = rb_intern("utc"); intern_local = rb_intern("local"); + intern_active = rb_intern("active?"); + intern_connect = rb_intern("connect"); cTinyTdsClient = rb_const_get(mTinyTds, rb_intern("Client")); cTinyTdsResult = rb_const_get(mTinyTds, rb_intern("Result")); diff --git a/lib/tiny_tds/client.rb b/lib/tiny_tds/client.rb index f9ffd6cc..c5382d39 100644 --- a/lib/tiny_tds/client.rb +++ b/lib/tiny_tds/client.rb @@ -52,8 +52,6 @@ def initialize(app_name: "TinyTds", azure: false, contained: false, database: ni @tds_version = tds_versions_setter(tds_version:) @username = parse_username(azure:, host:, username:) @use_utf16 = use_utf16.nil? || ["true", "1", "yes"].include?(use_utf16.to_s) - - connect end def tds_73? @@ -71,7 +69,7 @@ def active? private - def parse_username(azure: false, host: nil, username:) + def parse_username(username:, azure: false, host: nil) return username if username.nil? || !azure return username if username.include?("@") && !username.include?("database.windows.net") user, domain = username.split("@") diff --git a/test/client_test.rb b/test/client_test.rb index e48c5cda..57042de7 100644 --- a/test/client_test.rb +++ b/test/client_test.rb @@ -10,9 +10,18 @@ class ClientTest < TinyTds::TestCase @client = new_connection end - it "must not be closed" do - assert !@client.closed? - assert @client.active? + it "is considered close without a connection" do + client = TinyTds::Client.new(**connection_options) + + assert client.closed? + assert !client.active? + end + + it "returns nil values for server version without a connection" do + client = TinyTds::Client.new(**connection_options) + + assert_nil client.server_version + assert_nil client.server_version_info end it "allows client connection to be closed" do @@ -20,10 +29,6 @@ class ClientTest < TinyTds::TestCase assert @client.closed? assert !@client.active? assert @client.dead? - action = lambda { @client.execute("SELECT 1 as [one]").each } - assert_raise_tinytds_error(action) do |e| - assert_match %r{closed connection}i, e.message, "ignore if non-english test run" - end end it "has getters for the server version information (brittle since conf takes precedence)" do @@ -299,15 +304,5 @@ class ClientTest < TinyTds::TestCase assert_client_works(@client) end end - - it "throws an error if client is closed" do - @client.close - assert @client.closed? - - action = lambda { @client.insert("SELECT 1 as [one]") } - assert_raise_tinytds_error(action) do |e| - assert_match %r{closed connection}i, e.message - end - end end end diff --git a/test/test_helper.rb b/test/test_helper.rb index b637a931..854b6733 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -43,6 +43,7 @@ def close_client(client = @client) def new_connection(**options) client = TinyTds::Client.new(**connection_options(options)) + if sqlserver_azure? client.do("SET ANSI_NULLS ON") client.do("SET CURSOR_CLOSE_ON_COMMIT OFF") From 321c66a590fd841b9a73c39f129d4cc1583f39ed Mon Sep 17 00:00:00 2001 From: Andy Pfister Date: Tue, 9 Sep 2025 16:49:15 +0200 Subject: [PATCH 3/3] Rename `encoding` keyword argument to `charset` `encoding` denotes the actual `Encoding` then used to associate strings. --- ext/tiny_tds/client.c | 28 ++++------------------------ lib/tiny_tds/client.rb | 14 +++----------- test/client_test.rb | 10 +++++----- 3 files changed, 12 insertions(+), 40 deletions(-) diff --git a/ext/tiny_tds/client.c b/ext/tiny_tds/client.c index 2ed5b765..f336e180 100644 --- a/ext/tiny_tds/client.c +++ b/ext/tiny_tds/client.c @@ -5,7 +5,7 @@ VALUE cTinyTdsClient; extern VALUE mTinyTds, cTinyTdsError; static ID intern_source_eql, intern_severity_eql, intern_db_error_number_eql, intern_os_error_number_eql; -static ID intern_new, intern_dup, intern_transpose_iconv_encoding, intern_local_offset, intern_gsub, intern_call, intern_active, intern_connect; +static ID intern_new, intern_dup, intern_local_offset, intern_gsub, intern_call, intern_active, intern_connect; VALUE opt_escape_regex, opt_escape_dblquote; static ID id_ivar_fields, id_ivar_rows, id_ivar_return_code, id_ivar_affected_rows, id_ivar_default_query_options, intern_bigd, intern_divide; @@ -15,15 +15,6 @@ static VALUE cTinyTdsResult, cKernel, cDate; rb_encoding *binaryEncoding; VALUE opt_onek, opt_onebil, opt_float_zero, opt_four, opt_tenk; -static void rb_tinytds_client_mark(void *ptr) -{ - tinytds_client_wrapper *cwrap = (tinytds_client_wrapper *)ptr; - - if (cwrap) { - rb_gc_mark(cwrap->charset); - } -} - static void rb_tinytds_client_free(void *ptr) { tinytds_client_wrapper *cwrap = (tinytds_client_wrapper *)ptr; @@ -51,7 +42,7 @@ static size_t tinytds_client_wrapper_size(const void* data) static const rb_data_type_t tinytds_client_wrapper_type = { .wrap_struct_name = "tinytds_client_wrapper", .function = { - .dmark = rb_tinytds_client_mark, + .dmark = NULL, .dfree = rb_tinytds_client_free, .dsize = tinytds_client_wrapper_size, }, @@ -459,7 +450,6 @@ static VALUE allocate(VALUE klass) tinytds_client_wrapper *cwrap; obj = TypedData_Make_Struct(klass, tinytds_client_wrapper, &tinytds_client_wrapper_type, cwrap); cwrap->closed = 1; - cwrap->charset = Qnil; cwrap->userdata = malloc(sizeof(tinytds_client_userdata)); cwrap->userdata->closed = 1; rb_tinytds_client_reset_userdata(cwrap->userdata); @@ -910,12 +900,6 @@ static VALUE rb_tiny_tds_do(VALUE self, VALUE sql) return rb_tinytds_affected_rows(cwrap->client); } -static VALUE rb_tinytds_charset(VALUE self) -{ - GET_CLIENT_WRAPPER(self); - return cwrap->charset; -} - static VALUE rb_tinytds_encoding(VALUE self) { GET_CLIENT_WRAPPER(self); @@ -954,7 +938,7 @@ static VALUE rb_tinytds_connect(VALUE self) contained = rb_iv_get(self, "@contained"); database = rb_iv_get(self, "@database"); dataserver = rb_iv_get(self, "@dataserver"); - charset = rb_iv_get(self, "@encoding"); + charset = rb_iv_get(self, "@charset"); login_timeout = rb_iv_get(self, "@login_timeout"); password = rb_iv_get(self, "@password"); tds_version = rb_iv_get(self, "@tds_version"); @@ -1021,7 +1005,6 @@ static VALUE rb_tinytds_connect(VALUE self) VALUE transposed_encoding, timeout_string; cwrap->closed = 0; - cwrap->charset = charset; if (!NIL_P(tds_version)) { dbsetversion(NUM2INT(tds_version)); @@ -1043,8 +1026,7 @@ static VALUE rb_tinytds_connect(VALUE self) dbuse(cwrap->client, StringValueCStr(database)); } - transposed_encoding = rb_funcall(cTinyTdsClient, intern_transpose_iconv_encoding, 1, charset); - cwrap->encoding = rb_enc_find(StringValueCStr(transposed_encoding)); + cwrap->encoding = rb_enc_find(StringValueCStr(charset)); cwrap->identity_insert_sql = "SELECT CAST(SCOPE_IDENTITY() AS bigint) AS Ident"; } @@ -1068,7 +1050,6 @@ void init_tinytds_client() rb_define_method(cTinyTdsClient, "execute", rb_tinytds_execute, -1); rb_define_method(cTinyTdsClient, "insert", rb_tiny_tds_insert, 1); rb_define_method(cTinyTdsClient, "do", rb_tiny_tds_do, 1); - rb_define_method(cTinyTdsClient, "charset", rb_tinytds_charset, 0); rb_define_method(cTinyTdsClient, "encoding", rb_tinytds_encoding, 0); rb_define_method(cTinyTdsClient, "escape", rb_tinytds_escape, 1); rb_define_method(cTinyTdsClient, "return_code", rb_tinytds_return_code, 0); @@ -1082,7 +1063,6 @@ void init_tinytds_client() /* Intern Misc */ intern_new = rb_intern("new"); intern_dup = rb_intern("dup"); - intern_transpose_iconv_encoding = rb_intern("transpose_iconv_encoding"); intern_local_offset = rb_intern("local_offset"); intern_gsub = rb_intern("gsub"); intern_call = rb_intern("call"); diff --git a/lib/tiny_tds/client.rb b/lib/tiny_tds/client.rb index c5382d39..29626503 100644 --- a/lib/tiny_tds/client.rb +++ b/lib/tiny_tds/client.rb @@ -1,6 +1,6 @@ module TinyTds class Client - attr_reader :app_name, :contained, :database, :dataserver, :encoding, :message_handler, :login_timeout, :password, :port, :tds_version, :timeout, :username, :use_utf16 + attr_reader :app_name, :charset, :contained, :database, :dataserver, :message_handler, :login_timeout, :password, :port, :tds_version, :timeout, :username, :use_utf16 @default_query_options = { as: :hash, @@ -13,14 +13,6 @@ class Client class << self attr_reader :default_query_options - # Most, if not all, iconv encoding names can be found by ruby. Just in case, you can - # overide this method to return a string name that Encoding.find would work with. Default - # is to return the passed encoding. - # - def transpose_iconv_encoding(encoding) - encoding - end - def local_offset ::Time.local(2010).utc_offset.to_r / 86_400 end @@ -30,7 +22,7 @@ def local_offset # rubocop:disable Metrics/MethodLength # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/PerceivedComplexity - def initialize(app_name: "TinyTds", azure: false, contained: false, database: nil, dataserver: nil, encoding: "UTF-8", message_handler: nil, host: nil, login_timeout: 60, password: nil, port: 1433, tds_version: nil, timeout: 5, username: nil, use_utf16: true) + def initialize(app_name: "TinyTds", azure: false, charset: "UTF-8", contained: false, database: nil, dataserver: nil, message_handler: nil, host: nil, login_timeout: 60, password: nil, port: 1433, tds_version: nil, timeout: 5, username: nil, use_utf16: true) if dataserver.to_s.empty? && host.to_s.empty? raise ArgumentError, "missing :host option if no :dataserver given" end @@ -42,9 +34,9 @@ def initialize(app_name: "TinyTds", azure: false, contained: false, database: ni end @app_name = app_name + @charset = (charset.nil? || charset.casecmp("utf8").zero?) ? "UTF-8" : charset.upcase @database = database @dataserver = dataserver || "#{host}:#{port}" - @encoding = (encoding.nil? || encoding.casecmp("utf8").zero?) ? "UTF-8" : encoding.upcase @login_timeout = login_timeout.to_i @password = password if password && password.to_s.strip != "" @port = port.to_i diff --git a/test/client_test.rb b/test/client_test.rb index 57042de7..a7ae7179 100644 --- a/test/client_test.rb +++ b/test/client_test.rb @@ -45,11 +45,11 @@ class ClientTest < TinyTds::TestCase assert_equal "''hello''", @client.escape("'hello'") end - ["CP850", "CP1252", "ISO-8859-1"].each do |encoding| - it "allows valid iconv character set - #{encoding}" do - client = new_connection(encoding: encoding) - assert_equal encoding, client.charset - assert_equal Encoding.find(encoding), client.encoding + ["CP850", "CP1252", "ISO-8859-1"].each do |charset| + it "allows valid iconv character set - #{charset}" do + client = new_connection(charset:) + assert_equal charset, client.charset + assert_equal Encoding.find(charset), client.encoding ensure client&.close end