From a30f0c09e2613a9936155bf50be82ceaf6719ea0 Mon Sep 17 00:00:00 2001 From: Keith Gable Date: Tue, 24 Jul 2018 15:18:15 -0700 Subject: [PATCH 1/2] Fix connection string parsing, add a test for connection string parsing, use latest Ruby in Travis-CI --- .travis.yml | 59 +++++++++++-- .../connection_adapters/odbc_adapter.rb | 13 +-- test/connection_string_test.rb | 84 +++++++++++++++++++ test/test_helper.rb | 1 + 4 files changed, 146 insertions(+), 11 deletions(-) create mode 100644 test/connection_string_test.rb diff --git a/.travis.yml b/.travis.yml index 08e48209..bf05688a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,24 +3,73 @@ language: ruby cache: bundler matrix: include: - - rvm: 2.3.1 + - rvm: 2.3.7 env: - DB=mysql - CONN_STR='DRIVER=MySQL;SERVER=localhost;DATABASE=odbc_test;USER=root;PASSWORD=;' addons: - mysql: "5.5" apt: packages: - unixodbc - unixodbc-dev - libmyodbc - - mysql-client - - rvm: 2.3.1 + - mysql-client-5.6 + services: + - mysql + - rvm: 2.3.7 env: - DB=postgresql - CONN_STR='DRIVER={PostgreSQL ANSI};SERVER=localhost;PORT=5432;DATABASE=odbc_test;UID=postgres;' addons: - postgresql: "9.1" + postgresql: "9.2" + apt: + packages: + - unixodbc + - unixodbc-dev + - odbc-postgresql + - rvm: 2.4.4 + env: + - DB=mysql + - CONN_STR='DRIVER=MySQL;SERVER=localhost;DATABASE=odbc_test;USER=root;PASSWORD=;' + addons: + apt: + packages: + - unixodbc + - unixodbc-dev + - libmyodbc + - mysql-client-5.6 + services: + - mysql + - rvm: 2.4.4 + env: + - DB=postgresql + - CONN_STR='DRIVER={PostgreSQL ANSI};SERVER=localhost;PORT=5432;DATABASE=odbc_test;UID=postgres;' + addons: + postgresql: "9.2" + apt: + packages: + - unixodbc + - unixodbc-dev + - odbc-postgresql + - rvm: 2.5.1 + env: + - DB=mysql + - CONN_STR='DRIVER=MySQL;SERVER=localhost;DATABASE=odbc_test;USER=root;PASSWORD=;' + addons: + apt: + packages: + - unixodbc + - unixodbc-dev + - libmyodbc + - mysql-client-5.6 + services: + - mysql + - rvm: 2.5.1 + env: + - DB=postgresql + - CONN_STR='DRIVER={PostgreSQL ANSI};SERVER=localhost;PORT=5432;DATABASE=odbc_test;UID=postgres;' + addons: + postgresql: "9.2" apt: packages: - unixodbc diff --git a/lib/active_record/connection_adapters/odbc_adapter.rb b/lib/active_record/connection_adapters/odbc_adapter.rb index c1163245..e4a56340 100644 --- a/lib/active_record/connection_adapters/odbc_adapter.rb +++ b/lib/active_record/connection_adapters/odbc_adapter.rb @@ -30,7 +30,7 @@ def odbc_connection(config) # :nodoc: end dbms = ::ODBCAdapter::DBMS.new(connection) - dbms.adapter_class.new(connection, logger, dbms) + dbms.adapter_class.new(connection, logger, dbms, options) end private @@ -55,7 +55,7 @@ def odbc_conn_str_connection(config) driver.attrs = {} connstr_keyval_pairs.each do |pair| - keyval = pair.split('=') + keyval = pair.split('=', 2) driver.attrs[keyval[0]] = keyval[1] if keyval.length == 2 end @@ -79,10 +79,11 @@ class ODBCAdapter < AbstractAdapter attr_reader :dbms - def initialize(connection, logger, dbms) + def initialize(connection, logger, dbms, options) super(connection, logger) @connection = connection @dbms = dbms + @options = options @visitor = self.class::BindSubstitution.new(self) end @@ -112,10 +113,10 @@ def active? def reconnect! disconnect! @connection = - if options.key?(:dsn) - ODBC.connect(options[:dsn], options[:username], options[:password]) + if @options.key?(:dsn) + ODBC.connect(@options[:dsn], @options[:username], @options[:password]) else - ODBC::Database.new.drvconnect(options[:driver]) + ODBC::Database.new.drvconnect(@options[:driver]) end super end diff --git a/test/connection_string_test.rb b/test/connection_string_test.rb new file mode 100644 index 00000000..68698e41 --- /dev/null +++ b/test/connection_string_test.rb @@ -0,0 +1,84 @@ +require 'test_helper' + +class ConnectionStringTest < Minitest::Test + def setup + end + + def teardown + end + + # Make sure that the connection string is parsed properly when it has an equals sign + def test_odbc_conn_str_connection_with_equals + conn_str = "Foo=Bar;Foo2=Something=with=equals" + + odbc_driver_attrs = {} + odbc_driver_instance_mock = Minitest::Mock.new + odbc_database_instance_mock = Minitest::Mock.new + odbc_connection_instance_mock = Minitest::Mock.new + + # Setup ODBC::Driver instance mocks + odbc_driver_instance_mock.expect(:name=, nil, ['odbc']) + odbc_driver_instance_mock.expect(:attrs=, nil, [{}]) + odbc_driver_instance_mock.expect(:attrs, odbc_driver_attrs) + odbc_driver_instance_mock.expect(:attrs, odbc_driver_attrs) # must be called twice + + # Setup ODBC::Database instance mocks + odbc_database_instance_mock.expect(:drvconnect, odbc_connection_instance_mock, [odbc_driver_instance_mock]) + + # Stub ODBC::Driver.new + ODBC::Driver.stub :new, odbc_driver_instance_mock do + # Stub ODBC::Database.new + ODBC::Database.stub :new, odbc_database_instance_mock do + # Run under our stubs/mocks + ActiveRecord::Base.__send__(:odbc_conn_str_connection, conn_str: conn_str) + end + end + + # Assert we set up the driver properly + assert_equal odbc_driver_attrs['Foo'], 'Bar' + assert_equal odbc_driver_attrs['Foo2'], 'Something=with=equals' + + # make sure we called the methods we expected + odbc_driver_instance_mock.verify + odbc_database_instance_mock.verify + odbc_connection_instance_mock.verify + end + + # Make sure that the connection string is parsed properly when it doesn't have an + # equals sign + def test_odbc_conn_str_connection_without_equals + conn_str = "Foo=Bar;Foo2=Something without equals" + + odbc_driver_attrs = {} + odbc_driver_instance_mock = Minitest::Mock.new + odbc_database_instance_mock = Minitest::Mock.new + odbc_connection_instance_mock = Minitest::Mock.new + + # Setup ODBC::Driver instance mocks + odbc_driver_instance_mock.expect(:name=, nil, ['odbc']) + odbc_driver_instance_mock.expect(:attrs=, nil, [{}]) + odbc_driver_instance_mock.expect(:attrs, odbc_driver_attrs) + odbc_driver_instance_mock.expect(:attrs, odbc_driver_attrs) # must be called twice + + # Setup ODBC::Database instance mocks + odbc_database_instance_mock.expect(:drvconnect, odbc_connection_instance_mock, [odbc_driver_instance_mock]) + + # Stub ODBC::Driver.new + ODBC::Driver.stub :new, odbc_driver_instance_mock do + # Stub ODBC::Database.new + ODBC::Database.stub :new, odbc_database_instance_mock do + # Run under our stubs/mocks + ActiveRecord::Base.__send__(:odbc_conn_str_connection, conn_str: conn_str) + end + end + + # Assert we set up the driver properly + assert_equal odbc_driver_attrs['Foo'], 'Bar' + assert_equal odbc_driver_attrs['Foo2'], 'Something without equals' + + # make sure we called the methods we expected + odbc_driver_instance_mock.verify + odbc_database_instance_mock.verify + odbc_connection_instance_mock.verify + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index e937e615..eb181434 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -2,6 +2,7 @@ require 'odbc_adapter' require 'minitest/autorun' +require 'minitest/mock' require 'pry' options = { adapter: 'odbc' } From f0bf5cce2dfec9379e7597de362e7092c62034b3 Mon Sep 17 00:00:00 2001 From: Keith Gable Date: Tue, 24 Jul 2018 15:20:23 -0700 Subject: [PATCH 2/2] Put this back to the (busted) upstream implementation --- lib/active_record/connection_adapters/odbc_adapter.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/active_record/connection_adapters/odbc_adapter.rb b/lib/active_record/connection_adapters/odbc_adapter.rb index e4a56340..e3241019 100644 --- a/lib/active_record/connection_adapters/odbc_adapter.rb +++ b/lib/active_record/connection_adapters/odbc_adapter.rb @@ -30,7 +30,7 @@ def odbc_connection(config) # :nodoc: end dbms = ::ODBCAdapter::DBMS.new(connection) - dbms.adapter_class.new(connection, logger, dbms, options) + dbms.adapter_class.new(connection, logger, dbms) end private @@ -79,11 +79,10 @@ class ODBCAdapter < AbstractAdapter attr_reader :dbms - def initialize(connection, logger, dbms, options) + def initialize(connection, logger, dbms) super(connection, logger) @connection = connection @dbms = dbms - @options = options @visitor = self.class::BindSubstitution.new(self) end @@ -113,10 +112,10 @@ def active? def reconnect! disconnect! @connection = - if @options.key?(:dsn) - ODBC.connect(@options[:dsn], @options[:username], @options[:password]) + if options.key?(:dsn) + ODBC.connect(options[:dsn], options[:username], options[:password]) else - ODBC::Database.new.drvconnect(@options[:driver]) + ODBC::Database.new.drvconnect(options[:driver]) end super end