TinyTds::Client.new now accepts keyword arguments instead of a hash

Author: andyundso

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
 

README.md

@@ -98,15 +98,15 @@ 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.
 * :dataserver - Can be the name for your data server as defined in freetds.conf. Raw hostname or hostname:port will work here too. FreeTDS says that a named instance like 'localhost\SQLEXPRESS' will work too, but I highly suggest that you use the :host and :port options below. [Google how to find your host port if you are using named instances](http://bit.ly/xAf2jm) or [go here](http://msdn.microsoft.com/en-us/library/ms181087.aspx).
 * :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).

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=");

lib/tiny_tds/client.rb

@@ -1,13 +1,14 @@
 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,
       timezone: :local
     }
 
     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
 

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

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

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)