Merge branch '51-stable' into 52-stable

Author: enebo

.gitignore

@@ -2,6 +2,7 @@
 *.tgz
 *~
 *.log
+/log/
 patches*
 *#
 TAGS
@@ -30,4 +31,4 @@ Gemfile.lock
 .idea
 .settings
 activerecord-jdbc.iml
-lib/arjdbc/jdbc/adapter_java.jar
+lib/arjdbc/jdbc/adapter_java.jar

lib/arjdbc/abstract/database_statements.rb

@@ -13,10 +13,10 @@ def exec_insert(sql, name = nil, binds = NO_BINDS, pk = nil, sequence_name = nil
         binds = convert_legacy_binds_to_attributes(binds) if binds.first.is_a?(Array)
 
         if without_prepared_statement?(binds)
-          log(sql, name) { @connection.execute_insert(sql) }
+          log(sql, name) { @connection.execute_insert_pk(sql, pk) }
         else
           log(sql, name, binds) do
-            @connection.execute_insert(sql, binds)
+            @connection.execute_insert_pk(sql, binds, pk)
           end
         end
       end

lib/arjdbc/mysql/connection_methods.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 ArJdbc::ConnectionMethods.module_eval do
   def mysql_connection(config)
+    config = config.deep_dup
     # NOTE: this isn't "really" necessary but Rails (in tests) assumes being able to :
     #   ActiveRecord::Base.mysql2_connection ActiveRecord::Base.configurations['arunit'].merge(database: ...)
     config = symbolize_keys_if_necessary(config)

lib/arjdbc/postgresql/connection_methods.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 ArJdbc::ConnectionMethods.module_eval do
   def postgresql_connection(config)
+    config = config.deep_dup
     # NOTE: this isn't "really" necessary but Rails (in tests) assumes being able to :
     #   ActiveRecord::Base.postgresql_connection ActiveRecord::Base.configurations['arunit'].merge(:insert_returning => false)
     # ... while using symbols by default but than configurations returning string keys ;(

lib/arjdbc/sqlite3/connection_methods.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 ArJdbc::ConnectionMethods.module_eval do
   def sqlite3_connection(config)
+    config = config.deep_dup
     config[:adapter_spec] ||= ::ArJdbc::SQLite3
     config[:adapter_class] = ActiveRecord::ConnectionAdapters::SQLite3Adapter unless config.key?(:adapter_class)
 

rakelib/rails.rake

@@ -51,7 +51,7 @@ namespace :rails do
       ruby_opts_string += " -C \"#{ar_path}\""
       ruby_opts_string += " -rbundler/setup"
       ruby_opts_string += " -rminitest -rminitest/excludes" unless ENV['NO_EXCLUDES'].eql?('true')
-      file_list = ENV["TEST"] ? FileList[ ENV["TEST"] ] : test_files_finder.call
+      file_list = ENV["TEST"] ? FileList[ ENV["TEST"].split(',') ] : test_files_finder.call
       file_list_string = file_list.map { |fn| "\"#{fn}\"" }.join(' ')
       # test_loader_code = "-e \"ARGV.each{|f| require f}\"" # :direct
       option_list = ( ENV["TESTOPTS"] || ENV["TESTOPT"] || ENV["TEST_OPTS"] || '' )

src/java/arjdbc/jdbc/RubyJdbcConnection.java

@@ -727,7 +727,10 @@ public synchronized IRubyObject reconnect(final ThreadContext context) {
 
     private void connectImpl(final boolean forceConnection) throws SQLException {
         setConnection( forceConnection ? newConnection() : null );
-        if ( forceConnection ) configureConnection();
+        if (forceConnection) {
+            if (getConnectionImpl() == null) throw new SQLException("Didn't get a connection. Wrong URL?");
+            configureConnection();
+        }
     }
 
     @JRubyMethod(name = "read_only?")
@@ -885,23 +888,45 @@ protected IRubyObject mapExecuteResult(final ThreadContext context,
         return mapQueryResult(context, connection, resultSet);
     }
 
+    private static String[] createStatementPk(IRubyObject pk) {
+        String[] statementPk;
+        if (pk instanceof RubyArray) {
+            RubyArray ary = (RubyArray) pk;
+            int size = ary.size();
+            statementPk = new String[size];
+            for (int i = 0; i < size; i++) {
+                statementPk[i] = sqlString(ary.eltInternal(i));
+            }
+        } else {
+            statementPk = new String[] { sqlString(pk) };
+        }
+        return statementPk;
+    }
+
     /**
      * Executes an INSERT SQL statement
      * @param context
      * @param sql
+     * @param pk Rails PK
      * @return ActiveRecord::Result
      * @throws SQLException
      */
-    @JRubyMethod(name = "execute_insert", required = 1)
-    public IRubyObject execute_insert(final ThreadContext context, final IRubyObject sql) {
+    @JRubyMethod(name = "execute_insert_pk", required = 2)
+    public IRubyObject execute_insert_pk(final ThreadContext context, final IRubyObject sql, final IRubyObject pk) {
         return withConnection(context, new Callable<IRubyObject>() {
             public IRubyObject call(final Connection connection) throws SQLException {
                 Statement statement = null;
                 final String query = sqlString(sql);
                 try {
 
                     statement = createStatement(context, connection);
-                    statement.executeUpdate(query, Statement.RETURN_GENERATED_KEYS);
+
+                    if (pk == context.nil || pk == context.fals || !supportsGeneratedKeys(connection)) {
+                        statement.executeUpdate(query, Statement.RETURN_GENERATED_KEYS);
+                    } else {
+                        statement.executeUpdate(query, createStatementPk(pk));
+                    }
+
                     return mapGeneratedKeys(context, connection, statement);
 
                 } catch (final SQLException e) {
@@ -914,23 +939,35 @@ public IRubyObject call(final Connection connection) throws SQLException {
         });
     }
 
+    @Deprecated
+    @JRubyMethod(name = "execute_insert", required = 1)
+    public IRubyObject execute_insert(final ThreadContext context, final IRubyObject sql) {
+        return execute_insert_pk(context, sql, context.nil);
+    }
+
     /**
      * Executes an INSERT SQL statement using a prepared statement
      * @param context
      * @param sql
      * @param binds RubyArray of values to be bound to the query
+     * @param pk Rails PK
      * @return ActiveRecord::Result
      * @throws SQLException
      */
-    @JRubyMethod(name = "execute_insert", required = 2)
-    public IRubyObject execute_insert(final ThreadContext context, final IRubyObject sql, final IRubyObject binds) {
+    @JRubyMethod(name = "execute_insert_pk", required = 3)
+    public IRubyObject execute_insert_pk(final ThreadContext context, final IRubyObject sql, final IRubyObject binds,
+                                         final IRubyObject pk) {
         return withConnection(context, new Callable<IRubyObject>() {
             public IRubyObject call(final Connection connection) throws SQLException {
                 PreparedStatement statement = null;
                 final String query = sqlString(sql);
                 try {
+                    if (pk == context.nil || pk == context.fals || !supportsGeneratedKeys(connection)) {
+                        statement = connection.prepareStatement(query, Statement.RETURN_GENERATED_KEYS);
+                    } else {
+                        statement = connection.prepareStatement(query, createStatementPk(pk));
+                    }
 
-                    statement = connection.prepareStatement(query, Statement.RETURN_GENERATED_KEYS);
                     setStatementParameters(context, connection, statement, (RubyArray) binds);
                     statement.executeUpdate();
                     return mapGeneratedKeys(context, connection, statement);
@@ -945,6 +982,12 @@ public IRubyObject call(final Connection connection) throws SQLException {
         });
     }
 
+    @Deprecated
+    @JRubyMethod(name = "execute_insert", required = 2)
+    public IRubyObject execute_insert(final ThreadContext context, final IRubyObject binds, final IRubyObject sql) {
+        return execute_insert_pk(context, sql, binds, context.nil);
+    }
+
     /**
      * Executes an UPDATE (DELETE) SQL statement
      * @param context

test/db/mysql/unit_test.rb

@@ -96,7 +96,7 @@ def initialize; end
       connection_handler = connection_handler_stub
 
       config = { :jndi => 'jdbc/TestDS' }
-      connection_handler.expects(:jndi_connection)
+      connection_handler.expects(:jndi_connection).with() { |c| config = c }
       connection_handler.mysql_connection config
 
       # we do not complete username/database etc :
@@ -126,7 +126,7 @@ def initialize; end
       connection_handler = connection_handler_stub
 
       config = { database: 'MyDB' }
-      connection_handler.expects(:jdbc_connection)
+      connection_handler.expects(:jdbc_connection).with() { |c| config = c }
       ::Jdbc::MySQL.expects(:driver_name).returns('com.mysql.CustomDriver')
       connection_handler.mysql_connection config
       assert_equal 'com.mysql.CustomDriver', config[:driver]
@@ -138,7 +138,7 @@ def initialize; end
       connection_handler = connection_handler_stub
 
       config = { database: 'MyDB' }
-      connection_handler.expects(:jdbc_connection)
+      connection_handler.expects(:jdbc_connection).with() { |c| config = c }
       ::Jdbc::MySQL.expects(:driver_name).returns('com.mysql.cj.jdbc.Driver')
       connection_handler.mysql_connection config
       assert_equal 'com.mysql.cj.jdbc.Driver', config[:driver]
@@ -174,7 +174,7 @@ def teardown
       connection_handler = connection_handler_stub
 
       config = { host: '127.0.0.1', database: 'MyDB' }
-      connection_handler.expects(:jdbc_connection)
+      connection_handler.expects(:jdbc_connection).with() { |c| config = c }
       connection_handler.mysql_connection config
 
       # we do not complete username/database etc :
@@ -201,7 +201,7 @@ def teardown
       connection_handler = connection_handler_stub
 
       config = { database: 'MyDB', driver: false }
-      connection_handler.expects(:jdbc_connection)
+      connection_handler.expects(:jdbc_connection).with() { |c| config = c }
       connection_handler.expects(:require).never
       connection_handler.mysql_connection config
       assert_not config[:driver] # allow Java's service discovery mechanism (with connector/j 8.0)
@@ -211,7 +211,7 @@ def teardown
       connection_handler = connection_handler_stub
 
       config = { database: 'MyDB', driver: 'org.mariadb.jdbc.Driver' }
-      connection_handler.expects(:jdbc_connection)
+      connection_handler.expects(:jdbc_connection).with() { |c| config = c }
       connection_handler.mysql_connection config
 
       # we do not complete username/database etc :
@@ -224,4 +224,4 @@ def teardown
 
   end
 
-end if defined? JRUBY_VERSION
+end if defined? JRUBY_VERSION

test/db/postgresql/primary_key_is_not_the_first_column_test.rb

@@ -0,0 +1,37 @@
+require 'test_helper'
+require 'db/postgres'
+
+class PrimaryKeyIsNotTheFirstColumnTest < Test::Unit::TestCase
+
+  class CreateEmployees < ActiveRecord::Migration[4.2]
+    def self.up
+      create_table 'employees', :id => false do |t|
+        t.string :full_name, :null => false
+        t.primary_key :id, :serial
+      end
+    end
+    def self.down
+      drop_table 'employees'
+    end
+  end
+
+  def setup
+    CreateEmployees.up
+  end
+
+  def teardown
+    CreateEmployees.down
+  end
+
+  class Employee < ActiveRecord::Base
+  end
+
+  def test_returning_when_primary_key_is_not_the_first_column
+    e = Employee.new
+    e.full_name = 'Slartibartfast'
+    e.save!
+    e.reload
+    assert_equal 1, e.id
+  end
+
+end

test/db/postgresql/unit_test.rb

@@ -8,7 +8,7 @@ class PostgresUnitTest < Test::Unit::TestCase
       connection_handler = connection_handler_stub
 
       config = { :jndi => 'jdbc/TestDS' }
-      connection_handler.expects(:jndi_connection)
+      connection_handler.expects(:jndi_connection).with() { |c| config = c }
       connection_handler.postgresql_connection config
 
       # we do not complete username/database etc :