Merge branch 'master' into active-storage-import

Gemfile

@@ -108,8 +108,7 @@ local_gemfile = File.expand_path(".Gemfile", __dir__)
 instance_eval File.read local_gemfile if File.exist? local_gemfile
 
 group :test do
-  # FIX: Our test suite isn't ready to run in random order yet.
-  gem "minitest", "< 5.3.4"
+  gem "minitest-bisect"
 
   platforms :mri do
     gem "stackprof"

Gemfile.lock

@@ -301,7 +301,12 @@ GEM
     mime-types-data (3.2016.0521)
     mini_magick (4.8.0)
     mini_portile2 (2.2.0)
-    minitest (5.3.3)
+    minitest (5.10.2)
+    minitest-bisect (1.4.0)
+      minitest-server (~> 1.0)
+      path_expander (~> 1.0)
+    minitest-server (1.0.4)
+      minitest (~> 5.0)
     mocha (0.14.0)
       metaclass (~> 0.0.1)
     mono_logger (1.1.0)
@@ -326,6 +331,7 @@ GEM
     parallel (1.11.2)
     parser (2.4.0.0)
       ast (~> 2.2)
+    path_expander (1.0.1)
     pg (0.19.0)
     pg (0.19.0-x64-mingw32)
     pg (0.19.0-x86-mingw32)
@@ -507,6 +513,7 @@ DEPENDENCIES
   listen (>= 3.0.5, < 3.2)
   mini_magick
   minitest (< 5.3.4)
+  minitest-bisect
   mocha (~> 0.14)
   mysql2 (>= 0.4.4)
   nokogiri (>= 1.6.8)

actionmailer/test/base_test.rb

@@ -981,8 +981,7 @@ def welcome
   test "has access to params" do
     params = { name: "World" }
 
-    assert_called_with(BaseMailer, :welcome, [params]) do
-      BaseMailerPreview.call(:welcome, params)
-    end
+    message = BaseMailerPreview.call(:welcome, params)
+    assert_equal "World", message["name"].decoded
   end
 end

actionpack/lib/action_dispatch/http/response.rb

@@ -259,11 +259,10 @@ def sending_file=(v)
     #   response.charset = 'utf-16' # => 'utf-16'
     #   response.charset = nil      # => 'utf-8'
     def charset=(charset)
-      header_info = parsed_content_type_header
+      content_type = parsed_content_type_header.mime_type
       if false == charset
-        set_header CONTENT_TYPE, header_info.mime_type
+        set_content_type content_type, nil
       else
-        content_type = header_info.mime_type
         set_content_type content_type, charset || self.class.default_charset
       end
     end

actionpack/test/journey/gtg/transition_table_test.rb

@@ -21,7 +21,7 @@ def test_to_json
           assert json["accepting"]
         end
 
-        if system("dot -V 2>/dev/null")
+        if system("dot -V", 2 => File::NULL)
           def test_to_svg
             table = tt %w{
               /articles(.:format)

actionview/Rakefile

@@ -37,7 +37,7 @@ namespace :test do
       start_time = Time.now
 
       loop do
-        break if system("lsof -i :4567 >/dev/null")
+        break if system("lsof -i :4567", 1 => File::NULL)
 
         if Time.now - start_time > 5
           puts "Timed out after 5 seconds"

activejob/test/support/integration/adapters/sidekiq.rb

@@ -38,7 +38,7 @@ def start_workers
       # Sidekiq is not warning-clean :(
       $VERBOSE = false
 
-      $stdin.reopen("/dev/null")
+      $stdin.reopen(File::NULL)
       $stdout.sync = true
       $stderr.sync = true
 

activerecord/lib/active_record/attribute_methods/read.rb

@@ -31,9 +31,11 @@ def define_method_attribute(name)
             temp_method = "__temp__#{safe_name}"
 
             ActiveRecord::AttributeMethods::AttrNames.set_name_cache safe_name, name
+            sync_with_transaction_state = "sync_with_transaction_state" if name == primary_key
 
             generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
               def #{temp_method}
+                #{sync_with_transaction_state}
                 name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{safe_name}
                 _read_attribute(name) { |n| missing_attribute(n, caller) }
               end
@@ -57,6 +59,7 @@ def read_attribute(attr_name, &block)
         end
 
         name = self.class.primary_key if name == "id".freeze && self.class.primary_key
+        sync_with_transaction_state if name == self.class.primary_key
         _read_attribute(name, &block)
       end
 

activerecord/lib/active_record/attribute_methods/write.rb

@@ -15,10 +15,12 @@ module ClassMethods
           def define_method_attribute=(name)
             safe_name = name.unpack("h*".freeze).first
             ActiveRecord::AttributeMethods::AttrNames.set_name_cache safe_name, name
+            sync_with_transaction_state = "sync_with_transaction_state" if name == primary_key
 
             generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
               def __temp__#{safe_name}=(value)
                 name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{safe_name}
+                #{sync_with_transaction_state}
                 _write_attribute(name, value)
               end
               alias_method #{(name + '=').inspect}, :__temp__#{safe_name}=
@@ -38,6 +40,7 @@ def write_attribute(attr_name, value)
         end
 
         name = self.class.primary_key if name == "id".freeze && self.class.primary_key
+        sync_with_transaction_state if name == self.class.primary_key
         _write_attribute(name, value)
       end
 

activerecord/lib/active_record/core.rb

@@ -168,8 +168,7 @@ def find(*ids) # :nodoc:
 
         id = ids.first
 
-        return super if id.kind_of?(Array) ||
-                         id.is_a?(ActiveRecord::Base)
+        return super if StatementCache.unsupported_value?(id)
 
         key = primary_key
 
@@ -194,7 +193,7 @@ def find_by(*args) # :nodoc:
         hash = args.first
 
         return super if !(Hash === hash) || hash.values.any? { |v|
-          v.nil? || Array === v || Hash === v || Relation === v || Base === v
+          StatementCache.unsupported_value?(v)
         }
 
         # We can't cache Post.find_by(author: david) ...yet

activerecord/lib/active_record/relation/merger.rb

@@ -135,31 +135,33 @@ def merge_multi_values
           if other.reordering_value
             # override any order specified in the original relation
             relation.reorder! other.order_values
-          elsif other.order_values
+          elsif other.order_values.any?
             # merge in order_values from relation
             relation.order! other.order_values
           end
 
-          relation.extend(*other.extending_values) unless other.extending_values.blank?
+          extensions = other.extensions - relation.extensions
+          relation.extending!(*extensions) if extensions.any?
         end
 
         def merge_single_values
-          if relation.from_clause.empty?
-            relation.from_clause = other.from_clause
-          end
-          relation.lock_value ||= other.lock_value
+          relation.lock_value ||= other.lock_value if other.lock_value
 
           unless other.create_with_value.blank?
             relation.create_with_value = (relation.create_with_value || {}).merge(other.create_with_value)
           end
         end
 
         def merge_clauses
-          CLAUSE_METHODS.each do |method|
-            clause = relation.get_value(method)
-            other_clause = other.get_value(method)
-            relation.set_value(method, clause.merge(other_clause))
+          if relation.from_clause.empty? && !other.from_clause.empty?
+            relation.from_clause = other.from_clause
           end
+
+          where_clause = relation.where_clause.merge(other.where_clause)
+          relation.where_clause = where_clause unless where_clause.empty?
+
+          having_clause = relation.having_clause.merge(other.having_clause)
+          relation.having_clause = having_clause unless having_clause.empty?
         end
     end
   end

activerecord/lib/active_record/relation/query_methods.rb

@@ -902,16 +902,17 @@ def arel # :nodoc:
       @arel ||= build_arel
     end
 
-    # Returns a relation value with a given name
-    def get_value(name) # :nodoc:
-      @values[name] || default_value_for(name)
-    end
+    protected
+      # Returns a relation value with a given name
+      def get_value(name) # :nodoc:
+        @values[name] || default_value_for(name)
+      end
 
-    # Sets the relation value with the given name
-    def set_value(name, value) # :nodoc:
-      assert_mutability!
-      @values[name] = value
-    end
+      # Sets the relation value with the given name
+      def set_value(name, value) # :nodoc:
+        assert_mutability!
+        @values[name] = value
+      end
 
     private
 

activerecord/lib/active_record/statement_cache.rb

@@ -108,6 +108,11 @@ def execute(params, klass, connection, &block)
 
       klass.find_by_sql(sql, bind_values, preparable: true, &block)
     end
-    alias :call :execute
+
+    def self.unsupported_value?(value)
+      case value
+      when NilClass, Array, Range, Hash, Relation, Base then true
+      end
+    end
   end
 end

activerecord/lib/active_record/transactions.rb

@@ -432,8 +432,8 @@ def restore_transaction_record_state(force = false)
             @new_record = restore_state[:new_record]
             @destroyed  = restore_state[:destroyed]
             pk = self.class.primary_key
-            if pk && read_attribute(pk) != restore_state[:id]
-              write_attribute(pk, restore_state[:id])
+            if pk && _read_attribute(pk) != restore_state[:id]
+              _write_attribute(pk, restore_state[:id])
             end
             freeze if restore_state[:frozen?]
           end

activerecord/test/cases/adapter_test.rb

@@ -158,26 +158,6 @@ class << @connection
       end
     end
 
-    # test resetting sequences in odd tables in PostgreSQL
-    if ActiveRecord::Base.connection.respond_to?(:reset_pk_sequence!)
-      require "models/movie"
-      require "models/subscriber"
-
-      def test_reset_empty_table_with_custom_pk
-        Movie.delete_all
-        Movie.connection.reset_pk_sequence! "movies"
-        assert_equal 1, Movie.create(name: "fight club").id
-      end
-
-      def test_reset_table_with_non_integer_pk
-        Subscriber.delete_all
-        Subscriber.connection.reset_pk_sequence! "subscribers"
-        sub = Subscriber.new(name: "robert drake")
-        sub.id = "bob drake"
-        assert_nothing_raised { sub.save! }
-      end
-    end
-
     def test_uniqueness_violations_are_translated_to_specific_exception
       @connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
       error = assert_raises(ActiveRecord::RecordNotUnique) do
@@ -368,5 +348,25 @@ def setup
         assert !@connection.transaction_open?
       end
     end
+
+    # test resetting sequences in odd tables in PostgreSQL
+    if ActiveRecord::Base.connection.respond_to?(:reset_pk_sequence!)
+      require "models/movie"
+      require "models/subscriber"
+
+      def test_reset_empty_table_with_custom_pk
+        Movie.delete_all
+        Movie.connection.reset_pk_sequence! "movies"
+        assert_equal 1, Movie.create(name: "fight club").id
+      end
+
+      def test_reset_table_with_non_integer_pk
+        Subscriber.delete_all
+        Subscriber.connection.reset_pk_sequence! "subscribers"
+        sub = Subscriber.new(name: "robert drake")
+        sub.id = "bob drake"
+        assert_nothing_raised { sub.save! }
+      end
+    end
   end
 end

activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb

@@ -3,6 +3,8 @@
 require "cases/helper"
 
 class SchemaMigrationsTest < ActiveRecord::Mysql2TestCase
+  self.use_transactional_tests = false
+
   def test_renaming_index_on_foreign_key
     connection.add_index "engines", "car_id"
     connection.add_foreign_key :engines, :cars, name: "fk_engines_cars"
@@ -33,6 +35,8 @@ def test_initializes_internal_metadata_for_encoding_utf8mb4
 
       assert connection.column_exists?(table_name, :key, :string)
     end
+  ensure
+    ActiveRecord::InternalMetadata[:environment] = ActiveRecord::Migrator.current_environment
   end
 
   private

activerecord/test/cases/adapters/postgresql/hstore_test.rb

@@ -21,12 +21,7 @@ def to_unsafe_h
     def setup
       @connection = ActiveRecord::Base.connection
 
-      unless @connection.extension_enabled?("hstore")
-        @connection.enable_extension "hstore"
-        @connection.commit_db_transaction
-      end
-
-      @connection.reconnect!
+      enable_extension!("hstore", @connection)
 
       @connection.transaction do
         @connection.create_table("hstores") do |t|
@@ -42,6 +37,7 @@ def setup
 
     teardown do
       @connection.drop_table "hstores", if_exists: true
+      disable_extension!("hstore", @connection)
     end
 
     def test_hstore_included_in_extensions

activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb

@@ -327,26 +327,32 @@ def test_reload_type_map_for_newly_defined_types
       end
 
       def test_only_reload_type_map_once_for_every_unrecognized_type
+        reset_connection
+        connection = ActiveRecord::Base.connection
+
         silence_warnings do
           assert_queries 2, ignore_none: true do
-            @connection.select_all "select 'pg_catalog.pg_class'::regclass"
+            connection.select_all "select 'pg_catalog.pg_class'::regclass"
           end
           assert_queries 1, ignore_none: true do
-            @connection.select_all "select 'pg_catalog.pg_class'::regclass"
+            connection.select_all "select 'pg_catalog.pg_class'::regclass"
           end
           assert_queries 2, ignore_none: true do
-            @connection.select_all "SELECT NULL::anyarray"
+            connection.select_all "SELECT NULL::anyarray"
           end
         end
       ensure
         reset_connection
       end
 
       def test_only_warn_on_first_encounter_of_unrecognized_oid
+        reset_connection
+        connection = ActiveRecord::Base.connection
+
         warning = capture(:stderr) {
-          @connection.select_all "select 'pg_catalog.pg_class'::regclass"
-          @connection.select_all "select 'pg_catalog.pg_class'::regclass"
-          @connection.select_all "select 'pg_catalog.pg_class'::regclass"
+          connection.select_all "select 'pg_catalog.pg_class'::regclass"
+          connection.select_all "select 'pg_catalog.pg_class'::regclass"
+          connection.select_all "select 'pg_catalog.pg_class'::regclass"
         }
         assert_match(/\Aunknown OID \d+: failed to recognize type of 'regclass'\. It will be treated as String\.\n\z/, warning)
       ensure

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -465,7 +465,10 @@ def test_finder_bang_method_with_dirty_target
   end
 
   def test_create_resets_cached_counters
+    Reader.delete_all
+
     person = Person.create!(first_name: "tenderlove")
+
     post   = Post.first
 
     assert_equal [], person.readers