From d634c0d82b8ebd7b019baed535d89541bc8e4f6a Mon Sep 17 00:00:00 2001
From: Nikita Sokolov <faucct@gmail.com>
Date: Fri, 21 Sep 2018 10:53:00 +0300
Subject: ActiveRecord::Associations::Preloader should not fail to preload
 through missing records

---
 .../lib/active_record/associations/preloader.rb    | 52 ++++++++++------------
 .../associations/cascaded_eager_loading_test.rb    |  5 +++
 2 files changed, 29 insertions(+), 28 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb
index a8f94b574d..9253f4ccb4 100644
--- a/activerecord/lib/active_record/associations/preloader.rb
+++ b/activerecord/lib/active_record/associations/preloader.rb
@@ -98,12 +98,14 @@ module ActiveRecord
 
         # Loads all the given data into +records+ for the +association+.
         def preloaders_on(association, records, scope, polymorphic_parent = false)
-          if association.respond_to?(:to_hash)
+          case association
+          when Hash
             preloaders_for_hash(association, records, scope, polymorphic_parent)
-          elsif association.is_a?(Symbol)
-            preloaders_for_one(association, records, scope, polymorphic_parent)
-          elsif association.respond_to?(:to_str)
-            preloaders_for_one(association.to_sym, records, scope, polymorphic_parent)
+          when Symbol, String
+            grouped_records(association.to_sym, records, polymorphic_parent)
+              .flat_map do |reflection, reflection_records|
+                preloaders_for_reflection reflection, reflection_records, scope
+              end
           else
             raise ArgumentError, "#{association.inspect} was not recognized for preload"
           end
@@ -111,17 +113,15 @@ module ActiveRecord
 
         def preloaders_for_hash(association, records, scope, polymorphic_parent)
           association.flat_map { |parent, child|
-            loaders = preloaders_for_one parent, records, scope, polymorphic_parent
-
-            recs = loaders.flat_map(&:preloaded_records).uniq
-
-            reflection = records.first.class._reflect_on_association(parent)
-            polymorphic_parent = reflection && reflection.options[:polymorphic]
-
-            loaders.concat Array.wrap(child).flat_map { |assoc|
-              preloaders_on assoc, recs, scope, polymorphic_parent
-            }
-            loaders
+            grouped_records(parent, records, polymorphic_parent).flat_map do |reflection, reflection_records|
+              loaders = preloaders_for_reflection(reflection, reflection_records, scope)
+              recs = loaders.flat_map(&:preloaded_records)
+              child_polymorphic_parent = reflection && reflection.options[:polymorphic]
+              loaders.concat Array.wrap(child).flat_map { |assoc|
+                preloaders_on assoc, recs, scope, child_polymorphic_parent
+              }
+              loaders
+            end
           }
         end
 
@@ -137,13 +137,11 @@ module ActiveRecord
         # Additionally, polymorphic belongs_to associations can have multiple associated
         # classes, depending on the polymorphic_type field. So we group by the classes as
         # well.
-        def preloaders_for_one(association, records, scope, polymorphic_parent)
-          grouped_records(association, records, polymorphic_parent).flat_map do |reflection, klasses|
-            klasses.map do |rhs_klass, rs|
-              loader = preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope)
-              loader.run self
-              loader
-            end
+        def preloaders_for_reflection(reflection, records, scope)
+          records.group_by { |record| record.association(reflection.name).klass }.map do |rhs_klass, rs|
+            loader = preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope)
+            loader.run self
+            loader
           end
         end
 
@@ -151,11 +149,9 @@ module ActiveRecord
           h = {}
           records.each do |record|
             next unless record
-            next if polymorphic_parent && !record.class._reflect_on_association(association)
-            assoc = record.association(association)
-            next unless assoc.klass
-            klasses = h[assoc.reflection] ||= {}
-            (klasses[assoc.klass] ||= []) << record
+            reflection = record.class._reflect_on_association(association)
+            next if polymorphic_parent && !reflection || !record.association(association).klass
+            (h[reflection] ||= []) << record
           end
           h
         end
diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
index ba2104eb26..4cbc65b6d8 100644
--- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
+++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
@@ -160,6 +160,11 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_preload_through_missing_records
+    post = Post.where.not(author_id: Author.select(:id)).preload(author: { comments: :post }).first!
+    assert_no_queries { assert_nil post.author }
+  end
+
   def test_eager_association_loading_with_recursive_cascading_four_levels_has_many_through
     source = Vertex.all.merge!(includes: { sinks: { sinks: { sinks: :sinks } } }, order: "vertices.id").first
     assert_equal vertices(:vertex_4), assert_no_queries { source.sinks.first.sinks.first.sinks.first }
-- 
cgit v1.2.3


From c401c43850e79a4d994e22dd8d82a69612bb947f Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Tue, 2 Oct 2018 14:39:44 -0400
Subject: Fix call sites

---
 .../associations/builder/collection_association.rb             |  2 +-
 activerecord/lib/active_record/model_schema.rb                 | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb
index 35a72c3850..ff57c40121 100644
--- a/activerecord/lib/active_record/associations/builder/collection_association.rb
+++ b/activerecord/lib/active_record/associations/builder/collection_association.rb
@@ -24,7 +24,7 @@ module ActiveRecord::Associations::Builder # :nodoc:
       if block_given?
         extension_module_name = "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension"
         extension = Module.new(&Proc.new)
-        model.parent.const_set(extension_module_name, extension)
+        model.module_parent.const_set(extension_module_name, extension)
       end
     end
 
diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb
index 9b985e049b..5a4a1fb969 100644
--- a/activerecord/lib/active_record/model_schema.rb
+++ b/activerecord/lib/active_record/model_schema.rb
@@ -218,11 +218,11 @@ module ActiveRecord
       end
 
       def full_table_name_prefix #:nodoc:
-        (parents.detect { |p| p.respond_to?(:table_name_prefix) } || self).table_name_prefix
+        (module_parents.detect { |p| p.respond_to?(:table_name_prefix) } || self).table_name_prefix
       end
 
       def full_table_name_suffix #:nodoc:
-        (parents.detect { |p| p.respond_to?(:table_name_suffix) } || self).table_name_suffix
+        (module_parents.detect { |p| p.respond_to?(:table_name_suffix) } || self).table_name_suffix
       end
 
       # The array of names of environments where destructive actions should be prohibited. By default,
@@ -503,9 +503,9 @@ module ActiveRecord
         def compute_table_name
           if base_class?
             # Nested classes are prefixed with singular parent table name.
-            if parent < Base && !parent.abstract_class?
-              contained = parent.table_name
-              contained = contained.singularize if parent.pluralize_table_names
+            if module_parent < Base && !module_parent.abstract_class?
+              contained = module_parent.table_name
+              contained = contained.singularize if module_parent.pluralize_table_names
               contained += "_"
             end
 
-- 
cgit v1.2.3


From bfea0af4ba7d717d6a065b4370e3ccfd8869dde6 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Wed, 3 Oct 2018 05:29:27 +0900
Subject: Move UPDATE/DELETE with JOIN handling to the Arel side

---
 .../abstract/database_statements.rb                | 17 ------
 .../connection_adapters/abstract_mysql_adapter.rb  | 26 --------
 activerecord/lib/active_record/relation.rb         | 38 ++++--------
 activerecord/lib/arel/visitors/mysql.rb            | 48 +++++++++------
 activerecord/lib/arel/visitors/to_sql.rb           | 69 +++++++++++++++-------
 5 files changed, 92 insertions(+), 106 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
index feacdf6931..0059f0b773 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
@@ -410,16 +410,6 @@ module ActiveRecord
         end
       end
 
-      # The default strategy for an UPDATE with joins is to use a subquery. This doesn't work
-      # on MySQL (even when aliasing the tables), but MySQL allows using JOIN directly in
-      # an UPDATE statement, so in the MySQL adapters we redefine this to do that.
-      def join_to_update(update, select, key) # :nodoc:
-        subselect = subquery_for(key, select)
-
-        update.where key.in(subselect)
-      end
-      alias join_to_delete join_to_update
-
       private
         def default_insert_value(column)
           Arel.sql("DEFAULT")
@@ -460,13 +450,6 @@ module ActiveRecord
           total_sql.join(";\n")
         end
 
-        # Returns a subquery for the given key using the join information.
-        def subquery_for(key, select)
-          subselect = select.clone
-          subselect.projections = [key]
-          subselect
-        end
-
         # Returns an ActiveRecord::Result instance.
         def select(sql, name = nil, binds = [])
           exec_query(sql, name, binds, prepare: false)
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 09242a0f14..d40f38fb77 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -223,18 +223,6 @@ module ActiveRecord
         execute "ROLLBACK"
       end
 
-      # In the simple case, MySQL allows us to place JOINs directly into the UPDATE
-      # query. However, this does not allow for LIMIT, OFFSET and ORDER. To support
-      # these, we must use a subquery.
-      def join_to_update(update, select, key) # :nodoc:
-        if select.limit || select.offset || select.orders.any?
-          super
-        else
-          update.table select.source
-          update.wheres = select.constraints
-        end
-      end
-
       def empty_insert_statement_value(primary_key = nil)
         "VALUES ()"
       end
@@ -733,20 +721,6 @@ module ActiveRecord
           [remove_column_for_alter(table_name, :updated_at), remove_column_for_alter(table_name, :created_at)]
         end
 
-        # MySQL is too stupid to create a temporary table for use subquery, so we have
-        # to give it some prompting in the form of a subsubquery. Ugh!
-        def subquery_for(key, select)
-          subselect = select.clone
-          subselect.projections = [key]
-
-          # Materialize subquery by adding distinct
-          # to work with MySQL 5.7.6 which sets optimizer_switch='derived_merge=on'
-          subselect.distinct unless select.limit || select.offset || select.orders.any?
-
-          key_name = quote_column_name(key.name)
-          Arel::SelectManager.new(subselect.as("__active_record_temp")).project(Arel.sql(key_name))
-        end
-
         def supports_rename_index?
           mariadb? ? false : version >= "5.7.6"
         end
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index d8cff30b88..d5b6082d13 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -348,7 +348,12 @@ module ActiveRecord
       end
 
       stmt = Arel::UpdateManager.new
-      stmt.table(table)
+      stmt.table(arel.join_sources.empty? ? table : arel.source)
+      stmt.key = arel_attribute(primary_key)
+      stmt.take(arel.limit)
+      stmt.offset(arel.offset)
+      stmt.order(*arel.orders)
+      stmt.wheres = arel.constraints
 
       if updates.is_a?(Hash)
         stmt.set _substitute_values(updates)
@@ -356,16 +361,6 @@ module ActiveRecord
         stmt.set Arel.sql(klass.sanitize_sql_for_assignment(updates, table.name))
       end
 
-      if has_join_values?
-        @klass.connection.join_to_update(stmt, arel, arel_attribute(primary_key))
-      else
-        stmt.key = arel_attribute(primary_key)
-        stmt.take(arel.limit)
-        stmt.offset(arel.offset)
-        stmt.order(*arel.orders)
-        stmt.wheres = arel.constraints
-      end
-
       @klass.connection.update stmt, "#{@klass} Update All"
     end
 
@@ -483,17 +478,12 @@ module ActiveRecord
       end
 
       stmt = Arel::DeleteManager.new
-      stmt.from(table)
-
-      if has_join_values?
-        @klass.connection.join_to_delete(stmt, arel, arel_attribute(primary_key))
-      else
-        stmt.key = arel_attribute(primary_key)
-        stmt.take(arel.limit)
-        stmt.offset(arel.offset)
-        stmt.order(*arel.orders)
-        stmt.wheres = arel.constraints
-      end
+      stmt.from(arel.join_sources.empty? ? table : arel.source)
+      stmt.key = arel_attribute(primary_key)
+      stmt.take(arel.limit)
+      stmt.offset(arel.offset)
+      stmt.order(*arel.orders)
+      stmt.wheres = arel.constraints
 
       affected = @klass.connection.delete(stmt, "#{@klass} Destroy")
 
@@ -648,10 +638,6 @@ module ActiveRecord
         end
       end
 
-      def has_join_values?
-        joins_values.any? || left_outer_joins_values.any?
-      end
-
       def exec_queries(&block)
         skip_query_cache_if_necessary do
           @records =
diff --git a/activerecord/lib/arel/visitors/mysql.rb b/activerecord/lib/arel/visitors/mysql.rb
index eb8a449079..081452caeb 100644
--- a/activerecord/lib/arel/visitors/mysql.rb
+++ b/activerecord/lib/arel/visitors/mysql.rb
@@ -65,12 +65,42 @@ module Arel # :nodoc: all
           collector
         end
 
+        # In the simple case, MySQL allows us to place JOINs directly into the UPDATE
+        # query. However, this does not allow for LIMIT, OFFSET and ORDER. To support
+        # these, we must use a subquery.
+        def prepare_update_statement(o)
+          if has_join_sources?(o)
+            if has_limit_or_offset_or_orders?(o)
+              super
+            else
+              o
+            end
+          elsif o.offset
+            super
+          else
+            o
+          end
+        end
+
+        def prepare_delete_statement(o)
+          if has_join_sources?(o) || o.offset
+            super
+          else
+            o
+          end
+        end
+
+        # MySQL is too stupid to create a temporary table for use subquery, so we have
+        # to give it some prompting in the form of a subsubquery.
         def build_subselect(key, o)
           subselect = super
 
           # Materialize subquery by adding distinct
           # to work with MySQL 5.7.6 which sets optimizer_switch='derived_merge=on'
-          subselect.distinct unless subselect.limit || subselect.offset || subselect.orders.any?
+          unless has_limit_or_offset_or_orders?(subselect)
+            core = subselect.cores.last
+            core.set_quantifier = Arel::Nodes::Distinct.new
+          end
 
           Nodes::SelectStatement.new.tap do |stmt|
             core = stmt.cores.last
@@ -78,22 +108,6 @@ module Arel # :nodoc: all
             core.projections = [Arel.sql(quote_column_name(key.name))]
           end
         end
-
-        def collect_where_for(o, collector)
-          return super if o.offset
-
-          unless o.wheres.empty?
-            collector << " WHERE "
-            collector = inject_join o.wheres, collector, " AND "
-          end
-
-          unless o.orders.empty?
-            collector << " ORDER BY "
-            collector = inject_join o.orders, collector, ", "
-          end
-
-          maybe_visit o.limit, collector
-        end
     end
   end
 end
diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb
index 0172204fc8..7c0f6c2e97 100644
--- a/activerecord/lib/arel/visitors/to_sql.rb
+++ b/activerecord/lib/arel/visitors/to_sql.rb
@@ -74,26 +74,17 @@ module Arel # :nodoc: all
       private
 
         def visit_Arel_Nodes_DeleteStatement(o, collector)
+          o = prepare_delete_statement(o)
+
           collector << "DELETE FROM "
           collector = visit o.relation, collector
 
           collect_where_for(o, collector)
         end
 
-        # FIXME: we should probably have a 2-pass visitor for this
-        def build_subselect(key, o)
-          stmt             = Nodes::SelectStatement.new
-          core             = stmt.cores.first
-          core.froms       = o.relation
-          core.wheres      = o.wheres
-          core.projections = [key]
-          stmt.limit       = o.limit
-          stmt.offset      = o.offset
-          stmt.orders      = o.orders
-          stmt
-        end
-
         def visit_Arel_Nodes_UpdateStatement(o, collector)
+          o = prepare_update_statement(o)
+
           collector << "UPDATE "
           collector = visit o.relation, collector
           unless o.values.empty?
@@ -800,19 +791,57 @@ module Arel # :nodoc: all
           }
         end
 
-        def collect_where_for(o, collector)
-          if o.orders.empty? && o.limit.nil? && o.offset.nil?
-            wheres = o.wheres
+        def has_join_sources?(o)
+          o.relation.is_a?(Nodes::JoinSource) && !o.relation.right.empty?
+        end
+
+        def has_limit_or_offset_or_orders?(o)
+          o.limit || o.offset || !o.orders.empty?
+        end
+
+        # The default strategy for an UPDATE with joins is to use a subquery. This doesn't work
+        # on MySQL (even when aliasing the tables), but MySQL allows using JOIN directly in
+        # an UPDATE statement, so in the MySQL visitor we redefine this to do that.
+        def prepare_update_statement(o)
+          if o.key && (has_limit_or_offset_or_orders?(o) || has_join_sources?(o))
+            stmt = o.clone
+            stmt.limit = nil
+            stmt.offset = nil
+            stmt.orders = []
+            stmt.wheres = [Nodes::In.new(o.key, [build_subselect(o.key, o)])]
+            stmt.relation = o.relation.left if has_join_sources?(o)
+            stmt
           else
-            wheres = [Nodes::In.new(o.key, [build_subselect(o.key, o)])]
+            o
           end
+        end
+        alias :prepare_delete_statement :prepare_update_statement
+
+        # FIXME: we should probably have a 2-pass visitor for this
+        def build_subselect(key, o)
+          stmt             = Nodes::SelectStatement.new
+          core             = stmt.cores.first
+          core.froms       = o.relation
+          core.wheres      = o.wheres
+          core.projections = [key]
+          stmt.limit       = o.limit
+          stmt.offset      = o.offset
+          stmt.orders      = o.orders
+          stmt
+        end
 
-          unless wheres.empty?
+        def collect_where_for(o, collector)
+          unless o.wheres.empty?
             collector << " WHERE "
-            collector = inject_join wheres, collector, " AND "
+            collector = inject_join o.wheres, collector, " AND "
           end
 
-          collector
+          unless o.orders.empty?
+            collector << " ORDER BY "
+            collector = inject_join o.orders, collector, ", "
+          end
+
+          maybe_visit o.limit, collector
         end
 
         def infix_value(o, collector, value)
-- 
cgit v1.2.3


From 516e0d61bbe939268501f3d29c1078030776e041 Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Wed, 3 Oct 2018 13:50:19 -0400
Subject: Move test_fixtures and render_context to separate files

---
 .../active_record/fixture_set/render_context.rb    |  17 ++
 activerecord/lib/active_record/fixtures.rb         | 216 +--------------------
 activerecord/lib/active_record/test_fixtures.rb    | 201 +++++++++++++++++++
 3 files changed, 220 insertions(+), 214 deletions(-)
 create mode 100644 activerecord/lib/active_record/fixture_set/render_context.rb
 create mode 100644 activerecord/lib/active_record/test_fixtures.rb

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/fixture_set/render_context.rb b/activerecord/lib/active_record/fixture_set/render_context.rb
new file mode 100644
index 0000000000..c90b5343dc
--- /dev/null
+++ b/activerecord/lib/active_record/fixture_set/render_context.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+# NOTE: This class has to be defined in compact style in
+# order for rendering context subclassing to work correctly.
+class ActiveRecord::FixtureSet::RenderContext # :nodoc:
+  def self.create_subclass
+    Class.new(ActiveRecord::FixtureSet.context_class) do
+      def get_binding
+        binding()
+      end
+
+      def binary(path)
+        %(!!binary "#{Base64.strict_encode64(File.read(path))}")
+      end
+    end
+  end
+end
diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index 0d1fdcfb28..e697c30bb6 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -7,6 +7,8 @@ require "set"
 require "active_support/dependencies"
 require "active_support/core_ext/digest/uuid"
 require "active_record/fixture_set/file"
+require "active_record/fixture_set/render_context"
+require "active_record/test_fixtures"
 require "active_record/errors"
 
 module ActiveRecord
@@ -851,217 +853,3 @@ module ActiveRecord
     end
   end
 end
-
-module ActiveRecord
-  module TestFixtures
-    extend ActiveSupport::Concern
-
-    def before_setup # :nodoc:
-      setup_fixtures
-      super
-    end
-
-    def after_teardown # :nodoc:
-      super
-      teardown_fixtures
-    end
-
-    included do
-      class_attribute :fixture_path, instance_writer: false
-      class_attribute :fixture_table_names, default: []
-      class_attribute :fixture_class_names, default: {}
-      class_attribute :use_transactional_tests, default: true
-      class_attribute :use_instantiated_fixtures, default: false # true, false, or :no_instances
-      class_attribute :pre_loaded_fixtures, default: false
-      class_attribute :config, default: ActiveRecord::Base
-      class_attribute :lock_threads, default: true
-    end
-
-    module ClassMethods
-      # Sets the model class for a fixture when the class name cannot be inferred from the fixture name.
-      #
-      # Examples:
-      #
-      #   set_fixture_class some_fixture:        SomeModel,
-      #                     'namespaced/fixture' => Another::Model
-      #
-      # The keys must be the fixture names, that coincide with the short paths to the fixture files.
-      def set_fixture_class(class_names = {})
-        self.fixture_class_names = fixture_class_names.merge(class_names.stringify_keys)
-      end
-
-      def fixtures(*fixture_set_names)
-        if fixture_set_names.first == :all
-          raise StandardError, "No fixture path found. Please set `#{self}.fixture_path`." if fixture_path.blank?
-          fixture_set_names = Dir["#{fixture_path}/{**,*}/*.{yml}"].uniq
-          fixture_set_names.map! { |f| f[(fixture_path.to_s.size + 1)..-5] }
-        else
-          fixture_set_names = fixture_set_names.flatten.map(&:to_s)
-        end
-
-        self.fixture_table_names |= fixture_set_names
-        setup_fixture_accessors(fixture_set_names)
-      end
-
-      def setup_fixture_accessors(fixture_set_names = nil)
-        fixture_set_names = Array(fixture_set_names || fixture_table_names)
-        methods = Module.new do
-          fixture_set_names.each do |fs_name|
-            fs_name = fs_name.to_s
-            accessor_name = fs_name.tr("/", "_").to_sym
-
-            define_method(accessor_name) do |*fixture_names|
-              force_reload = fixture_names.pop if fixture_names.last == true || fixture_names.last == :reload
-              return_single_record = fixture_names.size == 1
-              fixture_names = @loaded_fixtures[fs_name].fixtures.keys if fixture_names.empty?
-
-              @fixture_cache[fs_name] ||= {}
-
-              instances = fixture_names.map do |f_name|
-                f_name = f_name.to_s if f_name.is_a?(Symbol)
-                @fixture_cache[fs_name].delete(f_name) if force_reload
-
-                if @loaded_fixtures[fs_name][f_name]
-                  @fixture_cache[fs_name][f_name] ||= @loaded_fixtures[fs_name][f_name].find
-                else
-                  raise StandardError, "No fixture named '#{f_name}' found for fixture set '#{fs_name}'"
-                end
-              end
-
-              return_single_record ? instances.first : instances
-            end
-            private accessor_name
-          end
-        end
-        include methods
-      end
-
-      def uses_transaction(*methods)
-        @uses_transaction = [] unless defined?(@uses_transaction)
-        @uses_transaction.concat methods.map(&:to_s)
-      end
-
-      def uses_transaction?(method)
-        @uses_transaction = [] unless defined?(@uses_transaction)
-        @uses_transaction.include?(method.to_s)
-      end
-    end
-
-    def run_in_transaction?
-      use_transactional_tests &&
-        !self.class.uses_transaction?(method_name)
-    end
-
-    def setup_fixtures(config = ActiveRecord::Base)
-      if pre_loaded_fixtures && !use_transactional_tests
-        raise RuntimeError, "pre_loaded_fixtures requires use_transactional_tests"
-      end
-
-      @fixture_cache = {}
-      @fixture_connections = []
-      @@already_loaded_fixtures ||= {}
-      @connection_subscriber = nil
-
-      # Load fixtures once and begin transaction.
-      if run_in_transaction?
-        if @@already_loaded_fixtures[self.class]
-          @loaded_fixtures = @@already_loaded_fixtures[self.class]
-        else
-          @loaded_fixtures = load_fixtures(config)
-          @@already_loaded_fixtures[self.class] = @loaded_fixtures
-        end
-
-        # Begin transactions for connections already established
-        @fixture_connections = enlist_fixture_connections
-        @fixture_connections.each do |connection|
-          connection.begin_transaction joinable: false
-          connection.pool.lock_thread = true if lock_threads
-        end
-
-        # When connections are established in the future, begin a transaction too
-        @connection_subscriber = ActiveSupport::Notifications.subscribe("!connection.active_record") do |_, _, _, _, payload|
-          spec_name = payload[:spec_name] if payload.key?(:spec_name)
-
-          if spec_name
-            begin
-              connection = ActiveRecord::Base.connection_handler.retrieve_connection(spec_name)
-            rescue ConnectionNotEstablished
-              connection = nil
-            end
-
-            if connection && !@fixture_connections.include?(connection)
-              connection.begin_transaction joinable: false
-              connection.pool.lock_thread = true if lock_threads
-              @fixture_connections << connection
-            end
-          end
-        end
-
-      # Load fixtures for every test.
-      else
-        ActiveRecord::FixtureSet.reset_cache
-        @@already_loaded_fixtures[self.class] = nil
-        @loaded_fixtures = load_fixtures(config)
-      end
-
-      # Instantiate fixtures for every test if requested.
-      instantiate_fixtures if use_instantiated_fixtures
-    end
-
-    def teardown_fixtures
-      # Rollback changes if a transaction is active.
-      if run_in_transaction?
-        ActiveSupport::Notifications.unsubscribe(@connection_subscriber) if @connection_subscriber
-        @fixture_connections.each do |connection|
-          connection.rollback_transaction if connection.transaction_open?
-          connection.pool.lock_thread = false
-        end
-        @fixture_connections.clear
-      else
-        ActiveRecord::FixtureSet.reset_cache
-      end
-
-      ActiveRecord::Base.clear_active_connections!
-    end
-
-    def enlist_fixture_connections
-      ActiveRecord::Base.connection_handler.connection_pool_list.map(&:connection)
-    end
-
-    private
-      def load_fixtures(config)
-        fixtures = ActiveRecord::FixtureSet.create_fixtures(fixture_path, fixture_table_names, fixture_class_names, config)
-        Hash[fixtures.map { |f| [f.name, f] }]
-      end
-
-      def instantiate_fixtures
-        if pre_loaded_fixtures
-          raise RuntimeError, "Load fixtures before instantiating them." if ActiveRecord::FixtureSet.all_loaded_fixtures.empty?
-          ActiveRecord::FixtureSet.instantiate_all_loaded_fixtures(self, load_instances?)
-        else
-          raise RuntimeError, "Load fixtures before instantiating them." if @loaded_fixtures.nil?
-          @loaded_fixtures.each_value do |fixture_set|
-            ActiveRecord::FixtureSet.instantiate_fixtures(self, fixture_set, load_instances?)
-          end
-        end
-      end
-
-      def load_instances?
-        use_instantiated_fixtures != :no_instances
-      end
-  end
-end
-
-class ActiveRecord::FixtureSet::RenderContext # :nodoc:
-  def self.create_subclass
-    Class.new ActiveRecord::FixtureSet.context_class do
-      def get_binding
-        binding()
-      end
-
-      def binary(path)
-        %(!!binary "#{Base64.strict_encode64(File.read(path))}")
-      end
-    end
-  end
-end
diff --git a/activerecord/lib/active_record/test_fixtures.rb b/activerecord/lib/active_record/test_fixtures.rb
new file mode 100644
index 0000000000..7b7b3f7112
--- /dev/null
+++ b/activerecord/lib/active_record/test_fixtures.rb
@@ -0,0 +1,201 @@
+# frozen_string_literal: true
+
+module ActiveRecord
+  module TestFixtures
+    extend ActiveSupport::Concern
+
+    def before_setup # :nodoc:
+      setup_fixtures
+      super
+    end
+
+    def after_teardown # :nodoc:
+      super
+      teardown_fixtures
+    end
+
+    included do
+      class_attribute :fixture_path, instance_writer: false
+      class_attribute :fixture_table_names, default: []
+      class_attribute :fixture_class_names, default: {}
+      class_attribute :use_transactional_tests, default: true
+      class_attribute :use_instantiated_fixtures, default: false # true, false, or :no_instances
+      class_attribute :pre_loaded_fixtures, default: false
+      class_attribute :config, default: ActiveRecord::Base
+      class_attribute :lock_threads, default: true
+    end
+
+    module ClassMethods
+      # Sets the model class for a fixture when the class name cannot be inferred from the fixture name.
+      #
+      # Examples:
+      #
+      #   set_fixture_class some_fixture:        SomeModel,
+      #                     'namespaced/fixture' => Another::Model
+      #
+      # The keys must be the fixture names, that coincide with the short paths to the fixture files.
+      def set_fixture_class(class_names = {})
+        self.fixture_class_names = fixture_class_names.merge(class_names.stringify_keys)
+      end
+
+      def fixtures(*fixture_set_names)
+        if fixture_set_names.first == :all
+          raise StandardError, "No fixture path found. Please set `#{self}.fixture_path`." if fixture_path.blank?
+          fixture_set_names = Dir["#{fixture_path}/{**,*}/*.{yml}"].uniq
+          fixture_set_names.map! { |f| f[(fixture_path.to_s.size + 1)..-5] }
+        else
+          fixture_set_names = fixture_set_names.flatten.map(&:to_s)
+        end
+
+        self.fixture_table_names |= fixture_set_names
+        setup_fixture_accessors(fixture_set_names)
+      end
+
+      def setup_fixture_accessors(fixture_set_names = nil)
+        fixture_set_names = Array(fixture_set_names || fixture_table_names)
+        methods = Module.new do
+          fixture_set_names.each do |fs_name|
+            fs_name = fs_name.to_s
+            accessor_name = fs_name.tr("/", "_").to_sym
+
+            define_method(accessor_name) do |*fixture_names|
+              force_reload = fixture_names.pop if fixture_names.last == true || fixture_names.last == :reload
+              return_single_record = fixture_names.size == 1
+              fixture_names = @loaded_fixtures[fs_name].fixtures.keys if fixture_names.empty?
+
+              @fixture_cache[fs_name] ||= {}
+
+              instances = fixture_names.map do |f_name|
+                f_name = f_name.to_s if f_name.is_a?(Symbol)
+                @fixture_cache[fs_name].delete(f_name) if force_reload
+
+                if @loaded_fixtures[fs_name][f_name]
+                  @fixture_cache[fs_name][f_name] ||= @loaded_fixtures[fs_name][f_name].find
+                else
+                  raise StandardError, "No fixture named '#{f_name}' found for fixture set '#{fs_name}'"
+                end
+              end
+
+              return_single_record ? instances.first : instances
+            end
+            private accessor_name
+          end
+        end
+        include methods
+      end
+
+      def uses_transaction(*methods)
+        @uses_transaction = [] unless defined?(@uses_transaction)
+        @uses_transaction.concat methods.map(&:to_s)
+      end
+
+      def uses_transaction?(method)
+        @uses_transaction = [] unless defined?(@uses_transaction)
+        @uses_transaction.include?(method.to_s)
+      end
+    end
+
+    def run_in_transaction?
+      use_transactional_tests &&
+        !self.class.uses_transaction?(method_name)
+    end
+
+    def setup_fixtures(config = ActiveRecord::Base)
+      if pre_loaded_fixtures && !use_transactional_tests
+        raise RuntimeError, "pre_loaded_fixtures requires use_transactional_tests"
+      end
+
+      @fixture_cache = {}
+      @fixture_connections = []
+      @@already_loaded_fixtures ||= {}
+      @connection_subscriber = nil
+
+      # Load fixtures once and begin transaction.
+      if run_in_transaction?
+        if @@already_loaded_fixtures[self.class]
+          @loaded_fixtures = @@already_loaded_fixtures[self.class]
+        else
+          @loaded_fixtures = load_fixtures(config)
+          @@already_loaded_fixtures[self.class] = @loaded_fixtures
+        end
+
+        # Begin transactions for connections already established
+        @fixture_connections = enlist_fixture_connections
+        @fixture_connections.each do |connection|
+          connection.begin_transaction joinable: false
+          connection.pool.lock_thread = true if lock_threads
+        end
+
+        # When connections are established in the future, begin a transaction too
+        @connection_subscriber = ActiveSupport::Notifications.subscribe("!connection.active_record") do |_, _, _, _, payload|
+          spec_name = payload[:spec_name] if payload.key?(:spec_name)
+
+          if spec_name
+            begin
+              connection = ActiveRecord::Base.connection_handler.retrieve_connection(spec_name)
+            rescue ConnectionNotEstablished
+              connection = nil
+            end
+
+            if connection && !@fixture_connections.include?(connection)
+              connection.begin_transaction joinable: false
+              connection.pool.lock_thread = true if lock_threads
+              @fixture_connections << connection
+            end
+          end
+        end
+
+      # Load fixtures for every test.
+      else
+        ActiveRecord::FixtureSet.reset_cache
+        @@already_loaded_fixtures[self.class] = nil
+        @loaded_fixtures = load_fixtures(config)
+      end
+
+      # Instantiate fixtures for every test if requested.
+      instantiate_fixtures if use_instantiated_fixtures
+    end
+
+    def teardown_fixtures
+      # Rollback changes if a transaction is active.
+      if run_in_transaction?
+        ActiveSupport::Notifications.unsubscribe(@connection_subscriber) if @connection_subscriber
+        @fixture_connections.each do |connection|
+          connection.rollback_transaction if connection.transaction_open?
+          connection.pool.lock_thread = false
+        end
+        @fixture_connections.clear
+      else
+        ActiveRecord::FixtureSet.reset_cache
+      end
+
+      ActiveRecord::Base.clear_active_connections!
+    end
+
+    def enlist_fixture_connections
+      ActiveRecord::Base.connection_handler.connection_pool_list.map(&:connection)
+    end
+
+    private
+      def load_fixtures(config)
+        fixtures = ActiveRecord::FixtureSet.create_fixtures(fixture_path, fixture_table_names, fixture_class_names, config)
+        Hash[fixtures.map { |f| [f.name, f] }]
+      end
+
+      def instantiate_fixtures
+        if pre_loaded_fixtures
+          raise RuntimeError, "Load fixtures before instantiating them." if ActiveRecord::FixtureSet.all_loaded_fixtures.empty?
+          ActiveRecord::FixtureSet.instantiate_all_loaded_fixtures(self, load_instances?)
+        else
+          raise RuntimeError, "Load fixtures before instantiating them." if @loaded_fixtures.nil?
+          @loaded_fixtures.each_value do |fixture_set|
+            ActiveRecord::FixtureSet.instantiate_fixtures(self, fixture_set, load_instances?)
+          end
+        end
+      end
+
+      def load_instances?
+        use_instantiated_fixtures != :no_instances
+      end
+  end
+end
-- 
cgit v1.2.3


From 25839860711987b48b5e5a6d919b4ea294ac10a8 Mon Sep 17 00:00:00 2001
From: Eugene Kenny <elkenny@gmail.com>
Date: Thu, 4 Oct 2018 01:19:48 +0100
Subject: Remove unreachable database warning

`establish_connection` will never raise `ActiveRecord::NoDatabaseError`,
because it doesn't connect to a database; it sets up a connection pool.
---
 activerecord/lib/active_record/railtie.rb | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb
index 81ad9ef3a2..538659d6bd 100644
--- a/activerecord/lib/active_record/railtie.rb
+++ b/activerecord/lib/active_record/railtie.rb
@@ -168,21 +168,7 @@ end_error
     initializer "active_record.initialize_database" do
       ActiveSupport.on_load(:active_record) do
         self.configurations = Rails.application.config.database_configuration
-
-        begin
-          establish_connection
-        rescue ActiveRecord::NoDatabaseError
-          warn <<-end_warning
-Oops - You have a database configured, but it doesn't exist yet!
-
-Here's how to get started:
-
-  1. Configure your database in config/database.yml.
-  2. Run `rails db:create` to create the database.
-  3. Run `rails db:setup` to load your database schema.
-end_warning
-          raise
-        end
+        establish_connection
       end
     end
 
-- 
cgit v1.2.3


From 50684d15c6546d3050d93896612edd9c5a0fca62 Mon Sep 17 00:00:00 2001
From: Aidan Haran <aidanharan@yahoo.com>
Date: Thu, 4 Oct 2018 11:56:00 +0100
Subject: Escape table name so that it can be used in regular expression

---
 activerecord/test/cases/relation_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb
index fbeb617b29..68161f6a84 100644
--- a/activerecord/test/cases/relation_test.rb
+++ b/activerecord/test/cases/relation_test.rb
@@ -232,7 +232,7 @@ module ActiveRecord
       assert_equal 3, nb_inner_join, "Wrong amount of INNER JOIN in query"
 
       # using `\W` as the column separator
-      assert queries.any? { |sql| %r[INNER\s+JOIN\s+#{Author.quoted_table_name}\s+\Wauthors_categorizations\W]i.match?(sql) }, "Should be aliasing the child INNER JOINs in query"
+      assert queries.any? { |sql| %r[INNER\s+JOIN\s+#{Regexp.escape(Author.quoted_table_name)}\s+\Wauthors_categorizations\W]i.match?(sql) }, "Should be aliasing the child INNER JOINs in query"
     end
 
     def test_relation_with_merged_joins_aliased_works
-- 
cgit v1.2.3


From 4429540995b061cf120b6468ce76cbb44fcaba9c Mon Sep 17 00:00:00 2001
From: Matt Jones <al2o3cr@gmail.com>
Date: Thu, 30 Aug 2018 16:53:16 -0400
Subject: failing test for eager loading

cherry-picked from https://github.com/rails/rails/pull/33763
---
 activerecord/test/cases/associations/cascaded_eager_loading_test.rb | 5 +++++
 1 file changed, 5 insertions(+)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
index 4cbc65b6d8..a9e22c7643 100644
--- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
+++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
@@ -165,6 +165,11 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
     assert_no_queries { assert_nil post.author }
   end
 
+  def test_eager_association_loading_with_missing_first_record
+    posts = Post.where(id: 3).preload(author: { comments: :post }).to_a
+    assert_equal posts.size, 1
+  end
+
   def test_eager_association_loading_with_recursive_cascading_four_levels_has_many_through
     source = Vertex.all.merge!(includes: { sinks: { sinks: { sinks: :sinks } } }, order: "vertices.id").first
     assert_equal vertices(:vertex_4), assert_no_queries { source.sinks.first.sinks.first.sinks.first }
-- 
cgit v1.2.3


From 47adad38166fc6cf6bd306ff0c2f20ea922dbae6 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 5 Oct 2018 00:58:57 +0900
Subject: Restore `preloaders_for_one` method

Since the above comment is for the `preloaders_for_one`.
---
 activerecord/lib/active_record/associations/preloader.rb | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb
index 9253f4ccb4..8997579527 100644
--- a/activerecord/lib/active_record/associations/preloader.rb
+++ b/activerecord/lib/active_record/associations/preloader.rb
@@ -102,10 +102,7 @@ module ActiveRecord
           when Hash
             preloaders_for_hash(association, records, scope, polymorphic_parent)
           when Symbol, String
-            grouped_records(association.to_sym, records, polymorphic_parent)
-              .flat_map do |reflection, reflection_records|
-                preloaders_for_reflection reflection, reflection_records, scope
-              end
+            preloaders_for_one(association, records, scope, polymorphic_parent)
           else
             raise ArgumentError, "#{association.inspect} was not recognized for preload"
           end
@@ -127,7 +124,7 @@ module ActiveRecord
 
         # Loads all the given data into +records+ for a singular +association+.
         #
-        # Functions by instantiating a preloader class such as Preloader::HasManyThrough and
+        # Functions by instantiating a preloader class such as Preloader::Association and
         # call the +run+ method for each passed in class in the +records+ argument.
         #
         # Not all records have the same class, so group then preload group on the reflection
@@ -137,6 +134,13 @@ module ActiveRecord
         # Additionally, polymorphic belongs_to associations can have multiple associated
         # classes, depending on the polymorphic_type field. So we group by the classes as
         # well.
+        def preloaders_for_one(association, records, scope, polymorphic_parent)
+          grouped_records(association, records, polymorphic_parent)
+            .flat_map do |reflection, reflection_records|
+              preloaders_for_reflection reflection, reflection_records, scope
+            end
+        end
+
         def preloaders_for_reflection(reflection, records, scope)
           records.group_by { |record| record.association(reflection.name).klass }.map do |rhs_klass, rs|
             loader = preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope)
-- 
cgit v1.2.3


From 5c7c3fe0d7059ba62e14bf6ac4ddfbdcabd905a6 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 5 Oct 2018 02:16:58 +0900
Subject: Simplify the condition in `prepare_update_statement`

---
 activerecord/lib/arel/visitors/mysql.rb | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/arel/visitors/mysql.rb b/activerecord/lib/arel/visitors/mysql.rb
index 081452caeb..9d9294fecc 100644
--- a/activerecord/lib/arel/visitors/mysql.rb
+++ b/activerecord/lib/arel/visitors/mysql.rb
@@ -69,13 +69,7 @@ module Arel # :nodoc: all
         # query. However, this does not allow for LIMIT, OFFSET and ORDER. To support
         # these, we must use a subquery.
         def prepare_update_statement(o)
-          if has_join_sources?(o)
-            if has_limit_or_offset_or_orders?(o)
-              super
-            else
-              o
-            end
-          elsif o.offset
+          if o.offset || has_join_sources?(o) && has_limit_or_offset_or_orders?(o)
             super
           else
             o
@@ -83,7 +77,7 @@ module Arel # :nodoc: all
         end
 
         def prepare_delete_statement(o)
-          if has_join_sources?(o) || o.offset
+          if o.offset || has_join_sources?(o)
             super
           else
             o
-- 
cgit v1.2.3


From 811be477786455d144819a5e9fbb7f9f54b8da69 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 5 Oct 2018 03:07:12 +0900
Subject: Use `assert_no_queries` not to ignore BEGIN/COMMIT queries

`test_update_does_not_run_sql_if_record_has_not_changed` would pass
without #18501 since `assert_queries` ignores BEGIN/COMMIT unless
`ignore_none: true` is given.

Since #32647, empty BEGIN/COMMIT is ommited. So we no longer need to use
`assert_queries(0)` to ignore BEGIN/COMMIT in the queries.
---
 .../test/cases/associations/belongs_to_associations_test.rb  |  4 ++--
 .../cases/associations/eager_load_nested_include_test.rb     |  2 +-
 activerecord/test/cases/associations/eager_test.rb           |  2 +-
 .../has_and_belongs_to_many_associations_test.rb             |  2 +-
 .../test/cases/associations/has_many_associations_test.rb    | 12 ++++++------
 .../cases/associations/has_many_through_associations_test.rb | 12 ++++++------
 .../test/cases/associations/has_one_associations_test.rb     |  6 +++---
 activerecord/test/cases/autosave_association_test.rb         |  6 +++---
 activerecord/test/cases/collection_cache_key_test.rb         |  4 ++--
 activerecord/test/cases/dirty_test.rb                        |  4 ++--
 activerecord/test/cases/persistence_test.rb                  | 12 +++++-------
 activerecord/test/cases/query_cache_test.rb                  |  7 ++++---
 activerecord/test/cases/touch_later_test.rb                  |  2 +-
 13 files changed, 37 insertions(+), 38 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index 1fca1be181..d520919332 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -694,7 +694,7 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
     line_item = LineItem.create!
     Invoice.create!(line_items: [line_item])
 
-    assert_queries(0) { line_item.save }
+    assert_no_queries { line_item.save }
   end
 
   def test_belongs_to_with_touch_option_on_destroy
@@ -789,7 +789,7 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
 
   def test_dont_find_target_when_foreign_key_is_null
     tagging = taggings(:thinking_general)
-    assert_queries(0) { tagging.super_tag }
+    assert_no_queries { tagging.super_tag }
   end
 
   def test_dont_find_target_when_saving_foreign_key_after_stale_association_loaded
diff --git a/activerecord/test/cases/associations/eager_load_nested_include_test.rb b/activerecord/test/cases/associations/eager_load_nested_include_test.rb
index c5b2b77bd4..525ad3197a 100644
--- a/activerecord/test/cases/associations/eager_load_nested_include_test.rb
+++ b/activerecord/test/cases/associations/eager_load_nested_include_test.rb
@@ -92,7 +92,7 @@ class EagerLoadPolyAssocsTest < ActiveRecord::TestCase
   def test_include_query
     res = ShapeExpression.all.merge!(includes: [ :shape, { paint: :non_poly } ]).to_a
     assert_equal NUM_SHAPE_EXPRESSIONS, res.size
-    assert_queries(0) do
+    assert_no_queries do
       res.each do |se|
         assert_not_nil se.paint.non_poly, "this is the association that was loading incorrectly before the change"
         assert_not_nil se.shape, "just making sure other associations still work"
diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index 79b3b4a6ad..39034746c9 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -1346,7 +1346,7 @@ class EagerAssociationTest < ActiveRecord::TestCase
   def test_joins_with_includes_should_preload_via_joins
     post = assert_queries(1) { Post.includes(:comments).joins(:comments).order("posts.id desc").to_a.first }
 
-    assert_queries(0) do
+    assert_no_queries do
       assert_not_equal 0, post.comments.to_a.count
     end
   end
diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index 482302055d..1061a495fc 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -741,7 +741,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
   def test_get_ids_for_loaded_associations
     developer = developers(:david)
     developer.projects.reload
-    assert_queries(0) do
+    assert_no_queries do
       developer.project_ids
       developer.project_ids
     end
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 0b44515e00..bc280811a6 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -1266,7 +1266,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_calling_empty_with_counter_cache
     post = posts(:welcome)
-    assert_queries(0) do
+    assert_no_queries do
       assert_not_empty post.comments
     end
   end
@@ -1800,7 +1800,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     firm.clients = []
     firm.save
 
-    assert_queries(0, ignore_none: true) do
+    assert_no_queries do
       firm.clients = []
     end
 
@@ -1835,7 +1835,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_get_ids_for_loaded_associations
     company = companies(:first_firm)
     company.clients.reload
-    assert_queries(0) do
+    assert_no_queries do
       company.client_ids
       company.client_ids
     end
@@ -1862,11 +1862,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_get_ids_for_association_on_new_record_does_not_try_to_find_records
-    Company.columns  # Load schema information so we don't query below
-    Contract.columns # if running just this test.
+    # Load schema information so we don't query below if running just this test.
+    companies(:first_client).contract_ids
 
     company = Company.new
-    assert_queries(0) do
+    assert_no_queries do
       company.contract_ids
     end
 
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 442f4a93d4..40631e3272 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -274,7 +274,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     assert_queries(1) { posts(:thinking) }
     new_person = nil # so block binding catches it
 
-    assert_queries(0) do
+    assert_no_queries do
       new_person = Person.new first_name: "bob"
     end
 
@@ -294,7 +294,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   def test_associate_new_by_building
     assert_queries(1) { posts(:thinking) }
 
-    assert_queries(0) do
+    assert_no_queries do
       posts(:thinking).people.build(first_name: "Bob")
       posts(:thinking).people.new(first_name: "Ted")
     end
@@ -571,10 +571,10 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
       posts(:welcome).people = [people(:david)]
     end
 
-    assert_queries(0) {
+    assert_no_queries do
       assert_includes posts(:welcome).people, people(:david)
       assert_not_includes posts(:welcome).people, people(:michael)
-    }
+    end
 
     assert_includes posts(:welcome).reload.people.reload, people(:david)
     assert_not_includes posts(:welcome).reload.people.reload, people(:michael)
@@ -698,7 +698,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
       posts(:welcome).people.clear
     end
 
-    assert_queries(0) do
+    assert_no_queries do
       assert_empty posts(:welcome).people
     end
 
@@ -788,7 +788,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   def test_get_ids_for_loaded_associations
     person = people(:michael)
     person.posts.reload
-    assert_queries(0) do
+    assert_no_queries do
       person.post_ids
       person.post_ids
     end
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index 9eea34d2b9..634f0ca736 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -37,10 +37,10 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
   def test_has_one_cache_nils
     firm = companies(:another_firm)
     assert_queries(1) { assert_nil firm.account }
-    assert_queries(0) { assert_nil firm.account }
+    assert_no_queries { assert_nil firm.account }
 
-    firms = Firm.all.merge!(includes: :account).to_a
-    assert_queries(0) { firms.each(&:account) }
+    firms = Firm.includes(:account).to_a
+    assert_no_queries { firms.each(&:account) }
   end
 
   def test_with_select
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index db3a58eba9..c3f09f249c 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -1100,7 +1100,7 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
     assert @pirate.save
 
     Pirate.transaction do
-      assert_queries(0) do
+      assert_no_queries do
         assert @pirate.save
       end
     end
@@ -1181,12 +1181,12 @@ class TestAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase
 
   def test_changed_for_autosave_should_handle_cycles
     @ship.pirate = @pirate
-    assert_queries(0) { @ship.save! }
+    assert_no_queries { @ship.save! }
 
     @parrot = @pirate.parrots.create(name: "some_name")
     @parrot.name = "changed_name"
     assert_queries(1) { @ship.save! }
-    assert_queries(0) { @ship.save! }
+    assert_no_queries { @ship.save! }
   end
 
   def test_should_automatically_save_bang_the_associated_model
diff --git a/activerecord/test/cases/collection_cache_key_test.rb b/activerecord/test/cases/collection_cache_key_test.rb
index a5d908344a..844b2b2162 100644
--- a/activerecord/test/cases/collection_cache_key_test.rb
+++ b/activerecord/test/cases/collection_cache_key_test.rb
@@ -91,12 +91,12 @@ module ActiveRecord
       developers = Developer.where(name: "David")
 
       assert_queries(1) { developers.cache_key }
-      assert_queries(0) { developers.cache_key }
+      assert_no_queries { developers.cache_key }
     end
 
     test "it doesn't trigger any query if the relation is already loaded" do
       developers = Developer.where(name: "David").load
-      assert_queries(0) { developers.cache_key }
+      assert_no_queries { developers.cache_key }
     end
 
     test "relation cache_key changes when the sql query changes" do
diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb
index b1ebd20d6b..dfd74bfcb4 100644
--- a/activerecord/test/cases/dirty_test.rb
+++ b/activerecord/test/cases/dirty_test.rb
@@ -336,7 +336,7 @@ class DirtyTest < ActiveRecord::TestCase
     end
 
     with_partial_writes Pirate, true do
-      assert_queries(0) { 2.times { pirate.save! } }
+      assert_no_queries { 2.times { pirate.save! } }
       assert_equal old_updated_on, pirate.reload.updated_on
 
       assert_queries(1) { pirate.catchphrase = "bar"; pirate.save! }
@@ -355,7 +355,7 @@ class DirtyTest < ActiveRecord::TestCase
     old_lock_version = person.lock_version
 
     with_partial_writes Person, true do
-      assert_queries(0) { 2.times { person.save! } }
+      assert_no_queries { 2.times { person.save! } }
       assert_equal old_lock_version, person.reload.lock_version
 
       assert_queries(1) { person.first_name = "bar"; person.save! }
diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index 8073cabae6..4830ff2b5f 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -446,19 +446,17 @@ class PersistenceTest < ActiveRecord::TestCase
   end
 
   def test_update_attribute_does_not_run_sql_if_attribute_is_not_changed
-    klass = Class.new(Topic) do
-      def self.name; "Topic"; end
-    end
-    topic = klass.create(title: "Another New Topic")
-    assert_queries(0) do
+    topic = Topic.create(title: "Another New Topic")
+    assert_no_queries do
       assert topic.update_attribute(:title, "Another New Topic")
     end
   end
 
   def test_update_does_not_run_sql_if_record_has_not_changed
     topic = Topic.create(title: "Another New Topic")
-    assert_queries(0) { assert topic.update(title: "Another New Topic") }
-    assert_queries(0) { assert topic.update(title: "Another New Topic") }
+    assert_no_queries do
+      assert topic.update(title: "Another New Topic")
+    end
   end
 
   def test_delete
diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb
index 3eb4e04cb7..565190c476 100644
--- a/activerecord/test/cases/query_cache_test.rb
+++ b/activerecord/test/cases/query_cache_test.rb
@@ -190,7 +190,7 @@ class QueryCacheTest < ActiveRecord::TestCase
       Task.cache do
         assert_queries(2) { Task.find(1); Task.find(2) }
       end
-      assert_queries(0) { Task.find(1); Task.find(1); Task.find(2) }
+      assert_no_queries { Task.find(1); Task.find(1); Task.find(2) }
     end
   end
 
@@ -372,7 +372,7 @@ class QueryCacheTest < ActiveRecord::TestCase
     end
 
     # Check that if the same query is run again, no queries are executed
-    assert_queries(0) do
+    assert_no_queries do
       assert_equal 0, Post.where(title: "test").to_a.count
     end
 
@@ -427,8 +427,9 @@ class QueryCacheTest < ActiveRecord::TestCase
       # Clear places where type information is cached
       Task.reset_column_information
       Task.initialize_find_by_cache
+      Task.define_attribute_methods
 
-      assert_queries(0) do
+      assert_no_queries do
         Task.find(1)
       end
     end
diff --git a/activerecord/test/cases/touch_later_test.rb b/activerecord/test/cases/touch_later_test.rb
index 925a4609a2..cd3d5ed7d1 100644
--- a/activerecord/test/cases/touch_later_test.rb
+++ b/activerecord/test/cases/touch_later_test.rb
@@ -100,7 +100,7 @@ class TouchLaterTest < ActiveRecord::TestCase
 
   def test_touch_later_dont_hit_the_db
     invoice = Invoice.create!
-    assert_queries(0) do
+    assert_no_queries do
       invoice.touch_later
     end
   end
-- 
cgit v1.2.3


From 45be690f8e6db019aac6198ba49d608a2e14824b Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 5 Oct 2018 04:30:01 +0900
Subject: Remove `ignore_none: false` to assert no queries more strictly

Follow up 811be477786455d144819a5e9fbb7f9f54b8da69.
---
 .../has_and_belongs_to_many_associations_test.rb   |  8 ++++----
 .../associations/has_many_associations_test.rb     | 24 +++++++++++-----------
 .../has_many_through_associations_test.rb          |  2 +-
 .../associations/has_one_associations_test.rb      |  2 +-
 .../nested_through_associations_test.rb            |  4 ++--
 .../test/cases/autosave_association_test.rb        |  8 ++++----
 activerecord/test/cases/null_relation_test.rb      | 14 ++++++-------
 7 files changed, 31 insertions(+), 31 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index 1061a495fc..a90dcc0576 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -310,7 +310,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build
     devel = Developer.find(1)
-    proj = assert_no_queries(ignore_none: false) { devel.projects.build("name" => "Projekt") }
+    proj = assert_no_queries { devel.projects.build("name" => "Projekt") }
     assert_not_predicate devel.projects, :loaded?
 
     assert_equal devel.projects.last, proj
@@ -325,7 +325,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
 
   def test_new_aliased_to_build
     devel = Developer.find(1)
-    proj = assert_no_queries(ignore_none: false) { devel.projects.new("name" => "Projekt") }
+    proj = assert_no_queries { devel.projects.new("name" => "Projekt") }
     assert_not_predicate devel.projects, :loaded?
 
     assert_equal devel.projects.last, proj
@@ -546,7 +546,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
 
     developer = project.developers.first
 
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       assert_predicate project.developers, :loaded?
       assert_includes project.developers, developer
     end
@@ -859,7 +859,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
 
   def test_has_and_belongs_to_many_associations_on_new_records_use_null_relations
     projects = Developer.new.projects
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       assert_equal [], projects
       assert_equal [], projects.where(title: "omg")
       assert_equal [], projects.pluck(:title)
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index bc280811a6..036df1c31e 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -458,7 +458,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_finder_method_with_dirty_target
     company = companies(:first_firm)
     new_clients = []
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       new_clients << company.clients_of_firm.build(name: "Another Client")
       new_clients << company.clients_of_firm.build(name: "Another Client II")
       new_clients << company.clients_of_firm.build(name: "Another Client III")
@@ -478,7 +478,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_finder_bang_method_with_dirty_target
     company = companies(:first_firm)
     new_clients = []
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       new_clients << company.clients_of_firm.build(name: "Another Client")
       new_clients << company.clients_of_firm.build(name: "Another Client II")
       new_clients << company.clients_of_firm.build(name: "Another Client III")
@@ -955,7 +955,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_transactions_when_adding_to_new_record
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       firm = Firm.new
       firm.clients_of_firm.concat(Client.new("name" => "Natural Company"))
     end
@@ -970,7 +970,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_new_aliased_to_build
     company = companies(:first_firm)
-    new_client = assert_no_queries(ignore_none: false) { company.clients_of_firm.new("name" => "Another Client") }
+    new_client = assert_no_queries { company.clients_of_firm.new("name" => "Another Client") }
     assert_not_predicate company.clients_of_firm, :loaded?
 
     assert_equal "Another Client", new_client.name
@@ -980,7 +980,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build
     company = companies(:first_firm)
-    new_client = assert_no_queries(ignore_none: false) { company.clients_of_firm.build("name" => "Another Client") }
+    new_client = assert_no_queries { company.clients_of_firm.build("name" => "Another Client") }
     assert_not_predicate company.clients_of_firm, :loaded?
 
     assert_equal "Another Client", new_client.name
@@ -1037,7 +1037,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build_many
     company = companies(:first_firm)
-    new_clients = assert_no_queries(ignore_none: false) { company.clients_of_firm.build([{ "name" => "Another Client" }, { "name" => "Another Client II" }]) }
+    new_clients = assert_no_queries { company.clients_of_firm.build([{ "name" => "Another Client" }, { "name" => "Another Client II" }]) }
     assert_equal 2, new_clients.size
   end
 
@@ -1063,7 +1063,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build_via_block
     company = companies(:first_firm)
-    new_client = assert_no_queries(ignore_none: false) { company.clients_of_firm.build { |client| client.name = "Another Client" } }
+    new_client = assert_no_queries { company.clients_of_firm.build { |client| client.name = "Another Client" } }
     assert_not_predicate company.clients_of_firm, :loaded?
 
     assert_equal "Another Client", new_client.name
@@ -1073,7 +1073,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build_many_via_block
     company = companies(:first_firm)
-    new_clients = assert_no_queries(ignore_none: false) do
+    new_clients = assert_no_queries do
       company.clients_of_firm.build([{ "name" => "Another Client" }, { "name" => "Another Client II" }]) do |client|
         client.name = "changed"
       end
@@ -1364,7 +1364,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_transaction_when_deleting_new_record
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       firm = Firm.new
       client = Client.new("name" => "New Client")
       firm.clients_of_firm << client
@@ -1822,7 +1822,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_transactions_when_replacing_on_new_record
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       firm = Firm.new
       firm.clients_of_firm = [Client.new("name" => "New Client")]
     end
@@ -1972,7 +1972,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     firm.clients.load_target
     assert_predicate firm.clients, :loaded?
 
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       firm.clients.first
       assert_equal 2, firm.clients.first(2).size
       firm.clients.last
@@ -2385,7 +2385,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   test "has many associations on new records use null relations" do
     post = Post.new
 
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       assert_equal [], post.comments
       assert_equal [], post.comments.where(body: "omg")
       assert_equal [], post.comments.pluck(:body)
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 40631e3272..94ad6e2d4d 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -1198,7 +1198,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   def test_has_many_through_associations_on_new_records_use_null_relations
     person = Person.new
 
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       assert_equal [], person.posts
       assert_equal [], person.posts.where(body: "omg")
       assert_equal [], person.posts.pluck(:body)
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index 634f0ca736..885d9d7c2c 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -231,7 +231,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_build_association_dont_create_transaction
-    assert_no_queries(ignore_none: false) {
+    assert_no_queries {
       Firm.new.build_account
     }
   end
diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb
index 03ed1c1d47..5821744530 100644
--- a/activerecord/test/cases/associations/nested_through_associations_test.rb
+++ b/activerecord/test/cases/associations/nested_through_associations_test.rb
@@ -137,7 +137,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase
   def test_has_many_through_has_one_through_with_has_one_source_reflection_preload
     members = assert_queries(4) { Member.includes(:nested_sponsors).to_a }
     mustache = sponsors(:moustache_club_sponsor_for_groucho)
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       assert_equal [mustache], members.first.nested_sponsors
     end
   end
@@ -196,7 +196,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase
 
     # postgresql test if randomly executed then executes "SHOW max_identifier_length". Hence
     # the need to ignore certain predefined sqls that deal with system calls.
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       assert_equal [groucho_details, other_details], members.first.organization_member_details_2.sort_by(&:id)
     end
   end
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index c3f09f249c..42d47527b2 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -642,7 +642,7 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa
 
   def test_build_before_save
     company = companies(:first_firm)
-    new_client = assert_no_queries(ignore_none: false) { company.clients_of_firm.build("name" => "Another Client") }
+    new_client = assert_no_queries { company.clients_of_firm.build("name" => "Another Client") }
     assert_not_predicate company.clients_of_firm, :loaded?
 
     company.name += "-changed"
@@ -653,7 +653,7 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa
 
   def test_build_many_before_save
     company = companies(:first_firm)
-    assert_no_queries(ignore_none: false) { company.clients_of_firm.build([{ "name" => "Another Client" }, { "name" => "Another Client II" }]) }
+    assert_no_queries { company.clients_of_firm.build([{ "name" => "Another Client" }, { "name" => "Another Client II" }]) }
 
     company.name += "-changed"
     assert_queries(3) { assert company.save }
@@ -662,7 +662,7 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa
 
   def test_build_via_block_before_save
     company = companies(:first_firm)
-    new_client = assert_no_queries(ignore_none: false) { company.clients_of_firm.build { |client| client.name = "Another Client" } }
+    new_client = assert_no_queries { company.clients_of_firm.build { |client| client.name = "Another Client" } }
     assert_not_predicate company.clients_of_firm, :loaded?
 
     company.name += "-changed"
@@ -673,7 +673,7 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa
 
   def test_build_many_via_block_before_save
     company = companies(:first_firm)
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       company.clients_of_firm.build([{ "name" => "Another Client" }, { "name" => "Another Client II" }]) do |client|
         client.name = "changed"
       end
diff --git a/activerecord/test/cases/null_relation_test.rb b/activerecord/test/cases/null_relation_test.rb
index 17527568f8..c5d5ded5ab 100644
--- a/activerecord/test/cases/null_relation_test.rb
+++ b/activerecord/test/cases/null_relation_test.rb
@@ -10,26 +10,26 @@ class NullRelationTest < ActiveRecord::TestCase
   fixtures :posts, :comments
 
   def test_none
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       assert_equal [], Developer.none
       assert_equal [], Developer.all.none
     end
   end
 
   def test_none_chainable
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       assert_equal [], Developer.none.where(name: "David")
     end
   end
 
   def test_none_chainable_to_existing_scope_extension_method
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       assert_equal 1, Topic.anonymous_extension.none.one
     end
   end
 
   def test_none_chained_to_methods_firing_queries_straight_to_db
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       assert_equal [],    Developer.none.pluck(:id, :name)
       assert_equal 0,     Developer.none.delete_all
       assert_equal 0,     Developer.none.update_all(name: "David")
@@ -39,7 +39,7 @@ class NullRelationTest < ActiveRecord::TestCase
   end
 
   def test_null_relation_content_size_methods
-    assert_no_queries(ignore_none: false) do
+    assert_no_queries do
       assert_equal 0,     Developer.none.size
       assert_equal 0,     Developer.none.count
       assert_equal true,  Developer.none.empty?
@@ -61,7 +61,7 @@ class NullRelationTest < ActiveRecord::TestCase
 
   [:count, :sum].each do |method|
     define_method "test_null_relation_#{method}" do
-      assert_no_queries(ignore_none: false) do
+      assert_no_queries do
         assert_equal 0, Comment.none.public_send(method, :id)
         assert_equal Hash.new, Comment.none.group(:post_id).public_send(method, :id)
       end
@@ -70,7 +70,7 @@ class NullRelationTest < ActiveRecord::TestCase
 
   [:average, :minimum, :maximum].each do |method|
     define_method "test_null_relation_#{method}" do
-      assert_no_queries(ignore_none: false) do
+      assert_no_queries do
         assert_nil Comment.none.public_send(method, :id)
         assert_equal Hash.new, Comment.none.group(:post_id).public_send(method, :id)
       end
-- 
cgit v1.2.3


From 7e7a6a351c4845d8d92238147450b22fe87627f9 Mon Sep 17 00:00:00 2001
From: bogdanvlviv <bogdanvlviv@gmail.com>
Date: Fri, 5 Oct 2018 14:36:13 +0300
Subject: Exercise stringify of database configurations

Since #33968 we stringify keys of database configuration
This commit adds more assertions in order to ensure that and prevent any
regression in the future.
Currently, if remove `to_s` added in #33968 from `env_name.to_s` on the
line
(activerecord/lib/active_record/database_configurations.rb:107), there is
no test that would fail. One of the added assertions should emphasize why we need
this `to_s`.

Follow up #33968
---
 .../test/cases/connection_adapters/connection_handler_test.rb        | 5 +++++
 1 file changed, 5 insertions(+)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb
index 5e3447efde..51d0cc3d12 100644
--- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb
+++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb
@@ -170,6 +170,11 @@ module ActiveRecord
         ActiveRecord::Base.configurations = config
         ActiveRecord::Base.configurations.configs_for.each do |db_config|
           assert_instance_of ActiveRecord::DatabaseConfigurations::HashConfig, db_config
+          assert_instance_of String, db_config.env_name
+          assert_instance_of String, db_config.spec_name
+          db_config.config.keys.each do |key|
+            assert_instance_of String, key
+          end
         end
       ensure
         ActiveRecord::Base.configurations = @prev_configs
-- 
cgit v1.2.3


From 2622f290c014f02daffb9ac72a4bf611776028a0 Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Fri, 5 Oct 2018 14:09:46 -0400
Subject: Organize FixtureSet class methods

---
 activerecord/lib/active_record/fixtures.rb | 206 +++++++++++++++--------------
 1 file changed, 104 insertions(+), 102 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index e697c30bb6..85c8297b02 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -442,60 +442,6 @@ module ActiveRecord
 
     @@all_cached_fixtures = Hash.new { |h, k| h[k] = {} }
 
-    def self.default_fixture_model_name(fixture_set_name, config = ActiveRecord::Base) # :nodoc:
-      config.pluralize_table_names ?
-        fixture_set_name.singularize.camelize :
-        fixture_set_name.camelize
-    end
-
-    def self.default_fixture_table_name(fixture_set_name, config = ActiveRecord::Base) # :nodoc:
-      "#{ config.table_name_prefix }"\
-      "#{ fixture_set_name.tr('/', '_') }"\
-      "#{ config.table_name_suffix }".to_sym
-    end
-
-    def self.reset_cache
-      @@all_cached_fixtures.clear
-    end
-
-    def self.cache_for_connection(connection)
-      @@all_cached_fixtures[connection]
-    end
-
-    def self.fixture_is_cached?(connection, table_name)
-      cache_for_connection(connection)[table_name]
-    end
-
-    def self.cached_fixtures(connection, keys_to_fetch = nil)
-      if keys_to_fetch
-        cache_for_connection(connection).values_at(*keys_to_fetch)
-      else
-        cache_for_connection(connection).values
-      end
-    end
-
-    def self.cache_fixtures(connection, fixtures_map)
-      cache_for_connection(connection).update(fixtures_map)
-    end
-
-    def self.instantiate_fixtures(object, fixture_set, load_instances = true)
-      if load_instances
-        fixture_set.each do |fixture_name, fixture|
-          begin
-            object.instance_variable_set "@#{fixture_name}", fixture.find
-          rescue FixtureClassNotFound
-            nil
-          end
-        end
-      end
-    end
-
-    def self.instantiate_all_loaded_fixtures(object, load_instances = true)
-      all_loaded_fixtures.each_value do |fixture_set|
-        instantiate_fixtures(object, fixture_set, load_instances)
-      end
-    end
-
     cattr_accessor :all_loaded_fixtures, default: {}
 
     class ClassCache
@@ -530,71 +476,127 @@ module ActiveRecord
         end
     end
 
-    def self.create_fixtures(fixtures_directory, fixture_set_names, class_names = {}, config = ActiveRecord::Base)
-      fixture_set_names = Array(fixture_set_names).map(&:to_s)
-      class_names = ClassCache.new class_names, config
+    class << self
+      def default_fixture_model_name(fixture_set_name, config = ActiveRecord::Base) # :nodoc:
+        config.pluralize_table_names ?
+          fixture_set_name.singularize.camelize :
+          fixture_set_name.camelize
+      end
 
-      # FIXME: Apparently JK uses this.
-      connection = block_given? ? yield : ActiveRecord::Base.connection
+      def default_fixture_table_name(fixture_set_name, config = ActiveRecord::Base) # :nodoc:
+        "#{ config.table_name_prefix }"\
+        "#{ fixture_set_name.tr('/', '_') }"\
+        "#{ config.table_name_suffix }".to_sym
+      end
 
-      files_to_read = fixture_set_names.reject { |fs_name|
-        fixture_is_cached?(connection, fs_name)
-      }
+      def reset_cache
+        @@all_cached_fixtures.clear
+      end
 
-      unless files_to_read.empty?
-        fixtures_map = {}
+      def cache_for_connection(connection)
+        @@all_cached_fixtures[connection]
+      end
 
-        fixture_sets = files_to_read.map do |fs_name|
-          klass = class_names[fs_name]
-          conn = klass ? klass.connection : connection
-          fixtures_map[fs_name] = new( # ActiveRecord::FixtureSet.new
-            conn,
-            fs_name,
-            klass,
-            ::File.join(fixtures_directory, fs_name))
-        end
+      def fixture_is_cached?(connection, table_name)
+        cache_for_connection(connection)[table_name]
+      end
 
-        update_all_loaded_fixtures fixtures_map
-        fixture_sets_by_connection = fixture_sets.group_by { |fs| fs.model_class ? fs.model_class.connection : connection }
+      def cached_fixtures(connection, keys_to_fetch = nil)
+        if keys_to_fetch
+          cache_for_connection(connection).values_at(*keys_to_fetch)
+        else
+          cache_for_connection(connection).values
+        end
+      end
 
-        fixture_sets_by_connection.each do |conn, set|
-          table_rows_for_connection = Hash.new { |h, k| h[k] = [] }
+      def cache_fixtures(connection, fixtures_map)
+        cache_for_connection(connection).update(fixtures_map)
+      end
 
-          set.each do |fs|
-            fs.table_rows.each do |table, rows|
-              table_rows_for_connection[table].unshift(*rows)
+      def instantiate_fixtures(object, fixture_set, load_instances = true)
+        if load_instances
+          fixture_set.each do |fixture_name, fixture|
+            begin
+              object.instance_variable_set "@#{fixture_name}", fixture.find
+            rescue FixtureClassNotFound
+              nil
             end
           end
-          conn.insert_fixtures_set(table_rows_for_connection, table_rows_for_connection.keys)
+        end
+      end
 
-          # Cap primary key sequences to max(pk).
-          if conn.respond_to?(:reset_pk_sequence!)
-            set.each { |fs| conn.reset_pk_sequence!(fs.table_name) }
-          end
+      def instantiate_all_loaded_fixtures(object, load_instances = true)
+        all_loaded_fixtures.each_value do |fixture_set|
+          instantiate_fixtures(object, fixture_set, load_instances)
         end
+      end
+
+      def create_fixtures(fixtures_directory, fixture_set_names, class_names = {}, config = ActiveRecord::Base)
+        fixture_set_names = Array(fixture_set_names).map(&:to_s)
+        class_names = ClassCache.new class_names, config
+
+        # FIXME: Apparently JK uses this.
+        connection = block_given? ? yield : ActiveRecord::Base.connection
+
+        files_to_read = fixture_set_names.reject { |fs_name|
+          fixture_is_cached?(connection, fs_name)
+        }
+
+        unless files_to_read.empty?
+          fixtures_map = {}
+
+          fixture_sets = files_to_read.map do |fs_name|
+            klass = class_names[fs_name]
+            conn = klass ? klass.connection : connection
+            fixtures_map[fs_name] = new( # ActiveRecord::FixtureSet.new
+              conn,
+              fs_name,
+              klass,
+              ::File.join(fixtures_directory, fs_name))
+          end
+
+          update_all_loaded_fixtures fixtures_map
+          fixture_sets_by_connection = fixture_sets.group_by { |fs| fs.model_class ? fs.model_class.connection : connection }
+
+          fixture_sets_by_connection.each do |conn, set|
+            table_rows_for_connection = Hash.new { |h, k| h[k] = [] }
 
-        cache_fixtures(connection, fixtures_map)
+            set.each do |fs|
+              fs.table_rows.each do |table, rows|
+                table_rows_for_connection[table].unshift(*rows)
+              end
+            end
+            conn.insert_fixtures_set(table_rows_for_connection, table_rows_for_connection.keys)
+
+            # Cap primary key sequences to max(pk).
+            if conn.respond_to?(:reset_pk_sequence!)
+              set.each { |fs| conn.reset_pk_sequence!(fs.table_name) }
+            end
+          end
+
+          cache_fixtures(connection, fixtures_map)
+        end
+        cached_fixtures(connection, fixture_set_names)
       end
-      cached_fixtures(connection, fixture_set_names)
-    end
 
-    # Returns a consistent, platform-independent identifier for +label+.
-    # Integer identifiers are values less than 2^30. UUIDs are RFC 4122 version 5 SHA-1 hashes.
-    def self.identify(label, column_type = :integer)
-      if column_type == :uuid
-        Digest::UUID.uuid_v5(Digest::UUID::OID_NAMESPACE, label.to_s)
-      else
-        Zlib.crc32(label.to_s) % MAX_ID
+      # Returns a consistent, platform-independent identifier for +label+.
+      # Integer identifiers are values less than 2^30. UUIDs are RFC 4122 version 5 SHA-1 hashes.
+      def identify(label, column_type = :integer)
+        if column_type == :uuid
+          Digest::UUID.uuid_v5(Digest::UUID::OID_NAMESPACE, label.to_s)
+        else
+          Zlib.crc32(label.to_s) % MAX_ID
+        end
       end
-    end
 
-    # Superclass for the evaluation contexts used by ERB fixtures.
-    def self.context_class
-      @context_class ||= Class.new
-    end
+      # Superclass for the evaluation contexts used by ERB fixtures.
+      def context_class
+        @context_class ||= Class.new
+      end
 
-    def self.update_all_loaded_fixtures(fixtures_map) # :nodoc:
-      all_loaded_fixtures.update(fixtures_map)
+      def update_all_loaded_fixtures(fixtures_map) # :nodoc:
+        all_loaded_fixtures.update(fixtures_map)
+      end
     end
 
     attr_reader :table_name, :name, :fixtures, :model_class, :config
-- 
cgit v1.2.3


From bf88250c3c24e26c1bac757c8967cf8e17f909f0 Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Fri, 5 Oct 2018 14:13:31 -0400
Subject: Move FixtureSet.create_fixtures reading and inserting sections into
 private methods

---
 activerecord/lib/active_record/fixtures.rb | 99 +++++++++++++++++-------------
 1 file changed, 58 insertions(+), 41 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index 85c8297b02..ffdf6ebba0 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -514,13 +514,12 @@ module ActiveRecord
       end
 
       def instantiate_fixtures(object, fixture_set, load_instances = true)
-        if load_instances
-          fixture_set.each do |fixture_name, fixture|
-            begin
-              object.instance_variable_set "@#{fixture_name}", fixture.find
-            rescue FixtureClassNotFound
-              nil
-            end
+        return unless load_instances
+        fixture_set.each do |fixture_name, fixture|
+          begin
+            object.instance_variable_set "@#{fixture_name}", fixture.find
+          rescue FixtureClassNotFound
+            nil
           end
         end
       end
@@ -538,42 +537,17 @@ module ActiveRecord
         # FIXME: Apparently JK uses this.
         connection = block_given? ? yield : ActiveRecord::Base.connection
 
-        files_to_read = fixture_set_names.reject { |fs_name|
+        fixture_files_to_read = fixture_set_names.reject do |fs_name|
           fixture_is_cached?(connection, fs_name)
-        }
-
-        unless files_to_read.empty?
-          fixtures_map = {}
-
-          fixture_sets = files_to_read.map do |fs_name|
-            klass = class_names[fs_name]
-            conn = klass ? klass.connection : connection
-            fixtures_map[fs_name] = new( # ActiveRecord::FixtureSet.new
-              conn,
-              fs_name,
-              klass,
-              ::File.join(fixtures_directory, fs_name))
-          end
-
-          update_all_loaded_fixtures fixtures_map
-          fixture_sets_by_connection = fixture_sets.group_by { |fs| fs.model_class ? fs.model_class.connection : connection }
-
-          fixture_sets_by_connection.each do |conn, set|
-            table_rows_for_connection = Hash.new { |h, k| h[k] = [] }
-
-            set.each do |fs|
-              fs.table_rows.each do |table, rows|
-                table_rows_for_connection[table].unshift(*rows)
-              end
-            end
-            conn.insert_fixtures_set(table_rows_for_connection, table_rows_for_connection.keys)
-
-            # Cap primary key sequences to max(pk).
-            if conn.respond_to?(:reset_pk_sequence!)
-              set.each { |fs| conn.reset_pk_sequence!(fs.table_name) }
-            end
-          end
+        end
 
+        if fixture_files_to_read.any?
+          fixtures_map = read_and_insert(
+            fixtures_directory,
+            fixture_files_to_read,
+            class_names,
+            connection,
+          )
           cache_fixtures(connection, fixtures_map)
         end
         cached_fixtures(connection, fixture_set_names)
@@ -594,6 +568,49 @@ module ActiveRecord
         @context_class ||= Class.new
       end
 
+      private
+
+      def read_and_insert(fixtures_directory, fixture_files, class_names, connection) # :nodoc:
+        fixtures_map = {}
+        fixture_sets = fixture_files.map do |fixture_set_name|
+          klass = class_names[fixture_set_name]
+          conn = klass&.connection || connection
+          fixtures_map[fixture_set_name] = new( # ActiveRecord::FixtureSet.new
+            conn,
+            fixture_set_name,
+            klass,
+            ::File.join(fixtures_directory, fixture_set_name)
+          )
+        end
+        update_all_loaded_fixtures(fixtures_map)
+
+        insert(fixture_sets, connection)
+
+        fixtures_map
+      end
+
+      def insert(fixture_sets, connection) # :nodoc:
+        fixture_sets_by_connection = fixture_sets.group_by do |fixture_set|
+          fixture_set.model_class&.connection || connection
+        end
+
+        fixture_sets_by_connection.each do |conn, set|
+          table_rows_for_connection = Hash.new { |h, k| h[k] = [] }
+
+          set.each do |fixture_set|
+            fixture_set.table_rows.each do |table, rows|
+              table_rows_for_connection[table].unshift(*rows)
+            end
+          end
+          conn.insert_fixtures_set(table_rows_for_connection, table_rows_for_connection.keys)
+
+          # Cap primary key sequences to max(pk).
+          if conn.respond_to?(:reset_pk_sequence!)
+            set.each { |fs| conn.reset_pk_sequence!(fs.table_name) }
+          end
+        end
+      end
+
       def update_all_loaded_fixtures(fixtures_map) # :nodoc:
         all_loaded_fixtures.update(fixtures_map)
       end
-- 
cgit v1.2.3


From ea4b52ab3cd375d314b0d9d74724cdb49b4fcdd4 Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Fri, 5 Oct 2018 14:20:02 -0400
Subject: Small refactors to FixtureSet::ClassCache and Fixture

---
 activerecord/lib/active_record/fixtures.rb | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index ffdf6ebba0..d3300426cb 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -450,14 +450,16 @@ module ActiveRecord
         @config      = config
 
         # Remove string values that aren't constants or subclasses of AR
-        @class_names.delete_if { |klass_name, klass| !insert_class(@class_names, klass_name, klass) }
+        @class_names.delete_if do |klass_name, klass|
+          !insert_class(@class_names, klass_name, klass)
+        end
       end
 
       def [](fs_name)
-        @class_names.fetch(fs_name) {
+        @class_names.fetch(fs_name) do
           klass = default_fixture_model(fs_name, @config).safe_constantize
           insert_class(@class_names, fs_name, klass)
-        }
+        end
       end
 
       private
@@ -862,12 +864,9 @@ module ActiveRecord
     alias :to_hash :fixture
 
     def find
-      if model_class
-        model_class.unscoped do
-          model_class.find(fixture[model_class.primary_key])
-        end
-      else
-        raise FixtureClassNotFound, "No class attached to find."
+      raise FixtureClassNotFound, "No class attached to find." unless model_class
+      model_class.unscoped do
+        model_class.find(fixture[model_class.primary_key])
       end
     end
   end
-- 
cgit v1.2.3


From ee5ab2228be88caa9a1dcdd8841488847cb43c07 Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Fri, 5 Oct 2018 14:28:15 -0400
Subject: Introduce FixtureSet::ModelMetadata

---
 .../active_record/fixture_set/model_metadata.rb    | 40 ++++++++++++++++++++++
 activerecord/lib/active_record/fixtures.rb         | 38 +++++---------------
 2 files changed, 49 insertions(+), 29 deletions(-)
 create mode 100644 activerecord/lib/active_record/fixture_set/model_metadata.rb

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/fixture_set/model_metadata.rb b/activerecord/lib/active_record/fixture_set/model_metadata.rb
new file mode 100644
index 0000000000..edc03939c8
--- /dev/null
+++ b/activerecord/lib/active_record/fixture_set/model_metadata.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module ActiveRecord
+  class FixtureSet
+    class ModelMetadata # :nodoc:
+      def initialize(model_class, table_name)
+        @model_class = model_class
+        @table_name  = table_name
+      end
+
+      def primary_key_name
+        @primary_key_name ||= @model_class && @model_class.primary_key
+      end
+
+      def primary_key_type
+        @primary_key_type ||= @model_class && @model_class.type_for_attribute(@model_class.primary_key).type
+      end
+
+      def has_primary_key_column?
+        @has_primary_key_column ||= primary_key_name &&
+          @model_class.columns.any? { |col| col.name == primary_key_name }
+      end
+
+      def timestamp_column_names
+        @timestamp_column_names ||=
+          %w(created_at created_on updated_at updated_on) & column_names
+      end
+
+      def inheritance_column_name
+        @inheritance_column_name ||= @model_class && @model_class.inheritance_column
+      end
+
+      private
+
+        def column_names
+          @column_names ||= @model_class.connection.columns(@table_name).collect(&:name)
+        end
+    end
+  end
+end
diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index d3300426cb..30fbc2a84a 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -8,6 +8,7 @@ require "active_support/dependencies"
 require "active_support/core_ext/digest/uuid"
 require "active_record/fixture_set/file"
 require "active_record/fixture_set/render_context"
+require "active_record/fixture_set/model_metadata"
 require "active_record/test_fixtures"
 require "active_record/errors"
 
@@ -669,7 +670,7 @@ module ActiveRecord
         if model_class
           # fill in timestamp columns if they aren't specified and the model is set to record_timestamps
           if model_class.record_timestamps
-            timestamp_column_names.each do |c_name|
+            model_metadata.timestamp_column_names.each do |c_name|
               row[c_name] = now unless row.key?(c_name)
             end
           end
@@ -680,8 +681,8 @@ module ActiveRecord
           end
 
           # generate a primary key if necessary
-          if has_primary_key_column? && !row.include?(primary_key_name)
-            row[primary_key_name] = ActiveRecord::FixtureSet.identify(label, primary_key_type)
+          if model_metadata.has_primary_key_column? && !row.include?(model_metadata.primary_key_name)
+            row[model_metadata.primary_key_name] = ActiveRecord::FixtureSet.identify(label, model_metadata.primary_key_type)
           end
 
           # Resolve enums
@@ -693,8 +694,8 @@ module ActiveRecord
 
           # If STI is used, find the correct subclass for association reflection
           reflection_class =
-            if row.include?(inheritance_column_name)
-              row[inheritance_column_name].constantize rescue model_class
+            if row.include?(model_metadata.inheritance_column_name)
+              row[model_metadata.inheritance_column_name].constantize rescue model_class
             else
               model_class
             end
@@ -760,13 +761,6 @@ module ActiveRecord
     end
 
     private
-      def primary_key_name
-        @primary_key_name ||= model_class && model_class.primary_key
-      end
-
-      def primary_key_type
-        @primary_key_type ||= model_class && model_class.type_for_attribute(model_class.primary_key).type
-      end
 
       def add_join_records(rows, row, association)
         # This is the case when the join table has no fixtures file
@@ -778,28 +772,14 @@ module ActiveRecord
 
           targets = targets.is_a?(Array) ? targets : targets.split(/\s*,\s*/)
           rows[table_name].concat targets.map { |target|
-            { lhs_key => row[primary_key_name],
+            { lhs_key => row[model_metadata.primary_key_name],
               rhs_key => ActiveRecord::FixtureSet.identify(target, column_type) }
           }
         end
       end
 
-      def has_primary_key_column?
-        @has_primary_key_column ||= primary_key_name &&
-          model_class.columns.any? { |c| c.name == primary_key_name }
-      end
-
-      def timestamp_column_names
-        @timestamp_column_names ||=
-          %w(created_at created_on updated_at updated_on) & column_names
-      end
-
-      def inheritance_column_name
-        @inheritance_column_name ||= model_class && model_class.inheritance_column
-      end
-
-      def column_names
-        @column_names ||= @connection.columns(@table_name).collect(&:name)
+      def model_metadata
+        @model_metadata ||= ModelMetadata.new(model_class, table_name)
       end
 
       def model_class=(class_name)
-- 
cgit v1.2.3


From 06121631ce5d4b9575d9e8e8e0d2e70e637f0ac6 Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Fri, 5 Oct 2018 14:33:10 -0400
Subject: Introduce FixtureSet::TableRows and FixtureSet::TableRow

---
 .../lib/active_record/fixture_set/table_row.rb     | 79 ++++++++++++++++++
 .../lib/active_record/fixture_set/table_rows.rb    | 97 ++++++++++++++++++++++
 activerecord/lib/active_record/fixtures.rb         | 95 ++-------------------
 3 files changed, 183 insertions(+), 88 deletions(-)
 create mode 100644 activerecord/lib/active_record/fixture_set/table_row.rb
 create mode 100644 activerecord/lib/active_record/fixture_set/table_rows.rb

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/fixture_set/table_row.rb b/activerecord/lib/active_record/fixture_set/table_row.rb
new file mode 100644
index 0000000000..5f72c1df38
--- /dev/null
+++ b/activerecord/lib/active_record/fixture_set/table_row.rb
@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+module ActiveRecord
+  class FixtureSet
+    class TableRow # :nodoc:
+      def initialize(fixture, table_rows:, label:, now:)
+        @table_rows = table_rows
+        @label = label
+        @now = now
+        @row = fixture.to_hash
+        fill_row_model_attributes
+      end
+
+      def to_hash
+        @row
+      end
+
+      private
+
+        def model_metadata
+          @table_rows.model_metadata
+        end
+
+        def model_class
+          @table_rows.model_class
+        end
+
+        def fill_row_model_attributes
+          return unless model_class
+          fill_timestamps
+          interpolate_label
+          generate_primary_key
+          resolve_enums
+          @table_rows.resolve_sti_reflections(@row)
+        end
+
+        def reflection_class
+          @reflection_class ||= if @row.include?(model_metadata.inheritance_column_name)
+            @row[model_metadata.inheritance_column_name].constantize rescue model_class
+          else
+            model_class
+          end
+        end
+
+        def fill_timestamps
+          # fill in timestamp columns if they aren't specified and the model is set to record_timestamps
+          if model_class.record_timestamps
+            model_metadata.timestamp_column_names.each do |c_name|
+              @row[c_name] = @now unless @row.key?(c_name)
+            end
+          end
+        end
+
+        def interpolate_label
+          # interpolate the fixture label
+          @row.each do |key, value|
+            @row[key] = value.gsub("$LABEL", @label.to_s) if value.is_a?(String)
+          end
+        end
+
+        def generate_primary_key
+          # generate a primary key if necessary
+          if model_metadata.has_primary_key_column? && !@row.include?(model_metadata.primary_key_name)
+            @row[model_metadata.primary_key_name] = ActiveRecord::FixtureSet.identify(
+              @label, model_metadata.primary_key_type
+            )
+          end
+        end
+
+        def resolve_enums
+          model_class.defined_enums.each do |name, values|
+            if @row.include?(name)
+              @row[name] = values.fetch(@row[name], @row[name])
+            end
+          end
+        end
+    end
+  end
+end
diff --git a/activerecord/lib/active_record/fixture_set/table_rows.rb b/activerecord/lib/active_record/fixture_set/table_rows.rb
new file mode 100644
index 0000000000..c7227ef133
--- /dev/null
+++ b/activerecord/lib/active_record/fixture_set/table_rows.rb
@@ -0,0 +1,97 @@
+# frozen_string_literal: true
+
+require "active_record/fixture_set/table_row"
+require "active_record/fixture_set/model_metadata"
+
+module ActiveRecord
+  class FixtureSet
+    class TableRows # :nodoc:
+      def initialize(table_name, model_class:, fixtures:, config:)
+        @table_name  = table_name
+        @model_class = model_class
+
+        # track any join tables we need to insert later
+        @tables = Hash.new { |h, table| h[table] = [] }
+
+        build_table_rows_from(fixtures, config)
+      end
+
+      attr_reader :table_name, :model_class
+
+      def to_hash
+        @tables.transform_values { |rows| rows.map(&:to_hash) }
+      end
+
+      def model_metadata
+        @model_metadata ||= ModelMetadata.new(model_class, table_name)
+      end
+
+      def resolve_sti_reflections(row)
+        # If STI is used, find the correct subclass for association reflection
+        reflection_class = reflection_class_for(row)
+
+        reflection_class._reflections.each_value do |association|
+          case association.macro
+          when :belongs_to
+            # Do not replace association name with association foreign key if they are named the same
+            fk_name = (association.options[:foreign_key] || "#{association.name}_id").to_s
+
+            if association.name.to_s != fk_name && value = row.delete(association.name.to_s)
+              if association.polymorphic? && value.sub!(/\s*\(([^\)]*)\)\s*$/, "")
+                # support polymorphic belongs_to as "label (Type)"
+                row[association.foreign_type] = $1
+              end
+
+              fk_type = reflection_class.type_for_attribute(fk_name).type
+              row[fk_name] = ActiveRecord::FixtureSet.identify(value, fk_type)
+            end
+          when :has_many
+            if association.options[:through]
+              add_join_records(row, HasManyThroughProxy.new(association))
+            end
+          end
+        end
+      end
+
+      private
+
+        def build_table_rows_from(fixtures, config)
+          now = config.default_timezone == :utc ? Time.now.utc : Time.now
+
+          @tables[table_name] = fixtures.map do |label, fixture|
+            TableRow.new(
+              fixture,
+              table_rows: self,
+              label: label,
+              now: now,
+            )
+          end
+        end
+
+        def reflection_class_for(row)
+          if row.include?(model_metadata.inheritance_column_name)
+            row[model_metadata.inheritance_column_name].constantize rescue model_class
+          else
+            model_class
+          end
+        end
+
+        def add_join_records(row, association)
+          # This is the case when the join table has no fixtures file
+          if (targets = row.delete(association.name.to_s))
+            table_name  = association.join_table
+            column_type = association.primary_key_type
+            lhs_key     = association.lhs_key
+            rhs_key     = association.rhs_key
+
+            targets = targets.is_a?(Array) ? targets : targets.split(/\s*,\s*/)
+            joins   = targets.map do |target|
+              { lhs_key => row[model_metadata.primary_key_name],
+                rhs_key => ActiveRecord::FixtureSet.identify(target, column_type) }
+            end
+            @tables[table_name].concat(joins)
+          end
+        end
+    end
+  end
+end
diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index 30fbc2a84a..9c05843999 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -8,7 +8,7 @@ require "active_support/dependencies"
 require "active_support/core_ext/digest/uuid"
 require "active_record/fixture_set/file"
 require "active_record/fixture_set/render_context"
-require "active_record/fixture_set/model_metadata"
+require "active_record/fixture_set/table_rows"
 require "active_record/test_fixtures"
 require "active_record/errors"
 
@@ -656,76 +656,15 @@ module ActiveRecord
     # Returns a hash of rows to be inserted. The key is the table, the value is
     # a list of rows to insert to that table.
     def table_rows
-      now = config.default_timezone == :utc ? Time.now.utc : Time.now
-
       # allow a standard key to be used for doing defaults in YAML
       fixtures.delete("DEFAULTS")
 
-      # track any join tables we need to insert later
-      rows = Hash.new { |h, table| h[table] = [] }
-
-      rows[table_name] = fixtures.map do |label, fixture|
-        row = fixture.to_hash
-
-        if model_class
-          # fill in timestamp columns if they aren't specified and the model is set to record_timestamps
-          if model_class.record_timestamps
-            model_metadata.timestamp_column_names.each do |c_name|
-              row[c_name] = now unless row.key?(c_name)
-            end
-          end
-
-          # interpolate the fixture label
-          row.each do |key, value|
-            row[key] = value.gsub("$LABEL", label.to_s) if value.is_a?(String)
-          end
-
-          # generate a primary key if necessary
-          if model_metadata.has_primary_key_column? && !row.include?(model_metadata.primary_key_name)
-            row[model_metadata.primary_key_name] = ActiveRecord::FixtureSet.identify(label, model_metadata.primary_key_type)
-          end
-
-          # Resolve enums
-          model_class.defined_enums.each do |name, values|
-            if row.include?(name)
-              row[name] = values.fetch(row[name], row[name])
-            end
-          end
-
-          # If STI is used, find the correct subclass for association reflection
-          reflection_class =
-            if row.include?(model_metadata.inheritance_column_name)
-              row[model_metadata.inheritance_column_name].constantize rescue model_class
-            else
-              model_class
-            end
-
-          reflection_class._reflections.each_value do |association|
-            case association.macro
-            when :belongs_to
-              # Do not replace association name with association foreign key if they are named the same
-              fk_name = (association.options[:foreign_key] || "#{association.name}_id").to_s
-
-              if association.name.to_s != fk_name && value = row.delete(association.name.to_s)
-                if association.polymorphic? && value.sub!(/\s*\(([^\)]*)\)\s*$/, "")
-                  # support polymorphic belongs_to as "label (Type)"
-                  row[association.foreign_type] = $1
-                end
-
-                fk_type = reflection_class.type_for_attribute(fk_name).type
-                row[fk_name] = ActiveRecord::FixtureSet.identify(value, fk_type)
-              end
-            when :has_many
-              if association.options[:through]
-                add_join_records(rows, row, HasManyThroughProxy.new(association))
-              end
-            end
-          end
-        end
-
-        row
-      end
-      rows
+      TableRows.new(
+        table_name,
+        model_class: model_class,
+        fixtures: fixtures,
+        config: config,
+      ).to_hash
     end
 
     class ReflectionProxy # :nodoc:
@@ -762,26 +701,6 @@ module ActiveRecord
 
     private
 
-      def add_join_records(rows, row, association)
-        # This is the case when the join table has no fixtures file
-        if (targets = row.delete(association.name.to_s))
-          table_name  = association.join_table
-          column_type = association.primary_key_type
-          lhs_key     = association.lhs_key
-          rhs_key     = association.rhs_key
-
-          targets = targets.is_a?(Array) ? targets : targets.split(/\s*,\s*/)
-          rows[table_name].concat targets.map { |target|
-            { lhs_key => row[model_metadata.primary_key_name],
-              rhs_key => ActiveRecord::FixtureSet.identify(target, column_type) }
-          }
-        end
-      end
-
-      def model_metadata
-        @model_metadata ||= ModelMetadata.new(model_class, table_name)
-      end
-
       def model_class=(class_name)
         if class_name.is_a?(Class) # TODO: Should be an AR::Base type class, or any?
           @model_class = class_name
-- 
cgit v1.2.3


From cfeabbfdcbc27e78efd9e183ba74a2fc9a7a69a0 Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Fri, 5 Oct 2018 14:35:01 -0400
Subject: Move FixtureSet::ReflectionProxy and FixtureSet::HasManyThroughProxy
 to FixtureSet::TableRows

---
 .../lib/active_record/fixture_set/table_rows.rb    | 32 ++++++++++++++++++++++
 activerecord/lib/active_record/fixtures.rb         | 32 ----------------------
 2 files changed, 32 insertions(+), 32 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/fixture_set/table_rows.rb b/activerecord/lib/active_record/fixture_set/table_rows.rb
index c7227ef133..e8335a2e10 100644
--- a/activerecord/lib/active_record/fixture_set/table_rows.rb
+++ b/activerecord/lib/active_record/fixture_set/table_rows.rb
@@ -6,6 +6,38 @@ require "active_record/fixture_set/model_metadata"
 module ActiveRecord
   class FixtureSet
     class TableRows # :nodoc:
+      class ReflectionProxy # :nodoc:
+        def initialize(association)
+          @association = association
+        end
+
+        def join_table
+          @association.join_table
+        end
+
+        def name
+          @association.name
+        end
+
+        def primary_key_type
+          @association.klass.type_for_attribute(@association.klass.primary_key).type
+        end
+      end
+
+      class HasManyThroughProxy < ReflectionProxy # :nodoc:
+        def rhs_key
+          @association.foreign_key
+        end
+
+        def lhs_key
+          @association.through_reflection.foreign_key
+        end
+
+        def join_table
+          @association.through_reflection.table_name
+        end
+      end
+
       def initialize(table_name, model_class:, fixtures:, config:)
         @table_name  = table_name
         @model_class = model_class
diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index 9c05843999..b090c76a38 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -667,38 +667,6 @@ module ActiveRecord
       ).to_hash
     end
 
-    class ReflectionProxy # :nodoc:
-      def initialize(association)
-        @association = association
-      end
-
-      def join_table
-        @association.join_table
-      end
-
-      def name
-        @association.name
-      end
-
-      def primary_key_type
-        @association.klass.type_for_attribute(@association.klass.primary_key).type
-      end
-    end
-
-    class HasManyThroughProxy < ReflectionProxy # :nodoc:
-      def rhs_key
-        @association.foreign_key
-      end
-
-      def lhs_key
-        @association.through_reflection.foreign_key
-      end
-
-      def join_table
-        @association.through_reflection.table_name
-      end
-    end
-
     private
 
       def model_class=(class_name)
-- 
cgit v1.2.3


From 8ca6d95509b89495131bf4b36a68a55815658845 Mon Sep 17 00:00:00 2001
From: Lachlan Sylvester <lachlan.sylvester@hypothetical.com.au>
Date: Fri, 5 Oct 2018 16:51:15 +1000
Subject: add test for cache_version precision

---
 activerecord/test/cases/integration_test.rb | 86 ++++++++++++++++++-----------
 1 file changed, 55 insertions(+), 31 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/integration_test.rb b/activerecord/test/cases/integration_test.rb
index 36cd63c4d4..5687afbc71 100644
--- a/activerecord/test/cases/integration_test.rb
+++ b/activerecord/test/cases/integration_test.rb
@@ -157,18 +157,40 @@ class IntegrationTest < ActiveRecord::TestCase
     skip("Subsecond precision is not supported") unless subsecond_precision_supported?
     dev = Developer.first
     key = dev.cache_key
-    dev.touch
+    travel_to dev.updated_at + 0.000001 do
+      dev.touch
+    end
     assert_not_equal key, dev.cache_key
   end
 
   def test_cache_key_format_is_not_too_precise
-    skip("Subsecond precision is not supported") unless subsecond_precision_supported?
     dev = Developer.first
     dev.touch
     key = dev.cache_key
     assert_equal key, dev.reload.cache_key
   end
 
+  def test_cache_version_format_is_precise_enough
+    skip("Subsecond precision is not supported") unless subsecond_precision_supported?
+    with_cache_versioning do
+      dev = Developer.first
+      version = dev.cache_version.to_param
+      travel_to Developer.first.updated_at + 0.000001 do
+        dev.touch
+      end
+      assert_not_equal version, dev.cache_version.to_param
+    end
+  end
+
+  def test_cache_version_format_is_not_too_precise
+    with_cache_versioning do
+      dev = Developer.first
+      dev.touch
+      key = dev.cache_version.to_param
+      assert_equal key, dev.reload.cache_version.to_param
+    end
+  end
+
   def test_named_timestamps_for_cache_key
     assert_deprecated do
       owner = owners(:blackbeard)
@@ -185,50 +207,52 @@ class IntegrationTest < ActiveRecord::TestCase
   end
 
   def test_cache_key_is_stable_with_versioning_on
-    Developer.cache_versioning = true
-
-    developer = Developer.first
-    first_key = developer.cache_key
+    with_cache_versioning do
+      developer = Developer.first
+      first_key = developer.cache_key
 
-    developer.touch
-    second_key = developer.cache_key
+      developer.touch
+      second_key = developer.cache_key
 
-    assert_equal first_key, second_key
-  ensure
-    Developer.cache_versioning = false
+      assert_equal first_key, second_key
+    end
   end
 
   def test_cache_version_changes_with_versioning_on
-    Developer.cache_versioning = true
-
-    developer     = Developer.first
-    first_version = developer.cache_version
+    with_cache_versioning do
+      developer     = Developer.first
+      first_version = developer.cache_version
 
-    travel 10.seconds do
-      developer.touch
-    end
+      travel 10.seconds do
+        developer.touch
+      end
 
-    second_version = developer.cache_version
+      second_version = developer.cache_version
 
-    assert_not_equal first_version, second_version
-  ensure
-    Developer.cache_versioning = false
+      assert_not_equal first_version, second_version
+    end
   end
 
   def test_cache_key_retains_version_when_custom_timestamp_is_used
-    Developer.cache_versioning = true
+    with_cache_versioning do
+      developer = Developer.first
+      first_key = developer.cache_key_with_version
 
-    developer = Developer.first
-    first_key = developer.cache_key_with_version
+      travel 10.seconds do
+        developer.touch
+      end
 
-    travel 10.seconds do
-      developer.touch
-    end
+      second_key = developer.cache_key_with_version
 
-    second_key = developer.cache_key_with_version
+      assert_not_equal first_key, second_key
+    end
+  end
 
-    assert_not_equal first_key, second_key
+  def with_cache_versioning(value = true)
+    @old_cache_versioning = ActiveRecord::Base.cache_versioning
+    ActiveRecord::Base.cache_versioning = value
+    yield
   ensure
-    Developer.cache_versioning = false
+    ActiveRecord::Base.cache_versioning = @old_cache_versioning
   end
 end
-- 
cgit v1.2.3


From 1f8534ca85c32ee26f0e179ac38acf7e61fb0e69 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sun, 7 Oct 2018 15:44:05 +0900
Subject: Fix `AssociationRelation` not to set inverse instance key just like
 before

Since #31575, `set_inverse_instance` replaces the foreign key by the
current owner immediately to make it happen when a record is added to
collection association.

But `set_inverse_instance` is not only called when a record is added,
but also when a record is loaded from queries. And also, that loaded
records are not always associated records for some reason (using `or`,
`unscope`, `rewhere`, etc).

It is hard to distinguish whether or not we should invoke
`set_inverse_instance`, but at least we should avoid the undesired
side-effect which was brought from #31575.

Fixes #34108.
---
 activerecord/lib/active_record/association_relation.rb       |  6 +++---
 activerecord/lib/active_record/associations/association.rb   |  8 ++++++++
 .../test/cases/associations/has_many_associations_test.rb    | 12 ++++++++++++
 3 files changed, 23 insertions(+), 3 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/association_relation.rb b/activerecord/lib/active_record/association_relation.rb
index 403667fb70..4c538ef2bd 100644
--- a/activerecord/lib/active_record/association_relation.rb
+++ b/activerecord/lib/active_record/association_relation.rb
@@ -31,9 +31,9 @@ module ActiveRecord
     private
 
       def exec_queries
-        super do |r|
-          @association.set_inverse_instance r
-          yield r if block_given?
+        super do |record|
+          @association.set_inverse_instance_from_queries(record)
+          yield record if block_given?
         end
       end
   end
diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb
index 44596f4424..e4433356e2 100644
--- a/activerecord/lib/active_record/associations/association.rb
+++ b/activerecord/lib/active_record/associations/association.rb
@@ -103,6 +103,13 @@ module ActiveRecord
         record
       end
 
+      def set_inverse_instance_from_queries(record)
+        if inverse = inverse_association_for(record)
+          inverse.inversed_from_queries(owner)
+        end
+        record
+      end
+
       # Remove the inverse association, if possible
       def remove_inverse_instance(record)
         if inverse = inverse_association_for(record)
@@ -114,6 +121,7 @@ module ActiveRecord
         self.target = record
         @inversed = !!record
       end
+      alias :inversed_from_queries :inversed_from
 
       # Returns the class of the target. belongs_to polymorphic overrides this to look at the
       # polymorphic_type field on the owner.
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 036df1c31e..45e2aadaf4 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -2356,6 +2356,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal [accounts(:signals37)], firm.accounts.open
   end
 
+  def test_association_with_or_doesnt_set_inverse_instance_key
+    firm = companies(:first_firm)
+    accounts = firm.accounts.or(Account.where(firm_id: nil)).order(:id)
+    assert_equal [firm.id, nil], accounts.map(&:firm_id)
+  end
+
+  def test_association_with_rewhere_doesnt_inverse_instance_key
+    firm = companies(:first_firm)
+    accounts = firm.accounts.rewhere(firm_id: [firm.id, nil]).order(:id)
+    assert_equal [firm.id, nil], accounts.map(&:firm_id)
+  end
+
   test "first_or_initialize adds the record to the association" do
     firm = Firm.create! name: "omg"
     client = firm.clients_of_firm.first_or_initialize
-- 
cgit v1.2.3


From 2b5b08b884853b9ec87252d079c7bc81a46931ec Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Mon, 8 Oct 2018 00:00:34 +0900
Subject: Fix test name to add missing "set"

---
 activerecord/test/cases/associations/has_many_associations_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 45e2aadaf4..39b3747fd7 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -2362,7 +2362,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal [firm.id, nil], accounts.map(&:firm_id)
   end
 
-  def test_association_with_rewhere_doesnt_inverse_instance_key
+  def test_association_with_rewhere_doesnt_set_inverse_instance_key
     firm = companies(:first_firm)
     accounts = firm.accounts.rewhere(firm_id: [firm.id, nil]).order(:id)
     assert_equal [firm.id, nil], accounts.map(&:firm_id)
-- 
cgit v1.2.3


From 0b4cfa2ba3e978386240e0e56a409616cc32fd02 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Wed, 19 Sep 2018 06:20:07 +0900
Subject: Don't expose internal methods in the associations

---
 .../lib/active_record/associations/association.rb  | 36 +++++++++++-----------
 .../associations/has_many_through_association.rb   | 28 ++++++++---------
 2 files changed, 32 insertions(+), 32 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb
index e4433356e2..4686b9f517 100644
--- a/activerecord/lib/active_record/associations/association.rb
+++ b/activerecord/lib/active_record/associations/association.rb
@@ -79,18 +79,6 @@ module ActiveRecord
         target_scope.merge!(association_scope)
       end
 
-      # The scope for this association.
-      #
-      # Note that the association_scope is merged into the target_scope only when the
-      # scope method is called. This is because at that point the call may be surrounded
-      # by scope.scoping { ... } or with_scope { ... } etc, which affects the scope which
-      # actually gets built.
-      def association_scope
-        if klass
-          @association_scope ||= AssociationScope.scope(self)
-        end
-      end
-
       def reset_scope
         @association_scope = nil
       end
@@ -129,12 +117,6 @@ module ActiveRecord
         reflection.klass
       end
 
-      # Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the
-      # through association's scope)
-      def target_scope
-        AssociationRelation.create(klass, self).merge!(klass.all)
-      end
-
       def extensions
         extensions = klass.default_extensions | reflection.extensions
 
@@ -195,6 +177,24 @@ module ActiveRecord
       end
 
       private
+        # The scope for this association.
+        #
+        # Note that the association_scope is merged into the target_scope only when the
+        # scope method is called. This is because at that point the call may be surrounded
+        # by scope.scoping { ... } or unscoped { ... } etc, which affects the scope which
+        # actually gets built.
+        def association_scope
+          if klass
+            @association_scope ||= AssociationScope.scope(self)
+          end
+        end
+
+        # Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the
+        # through association's scope)
+        def target_scope
+          AssociationRelation.create(klass, self).merge!(klass.all)
+        end
+
         def scope_for_create
           scope.scope_for_create
         end
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index 617956c768..f84ac65fa2 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -21,20 +21,6 @@ module ActiveRecord
         super
       end
 
-      def concat_records(records)
-        ensure_not_nested
-
-        records = super(records, true)
-
-        if owner.new_record? && records
-          records.flatten.each do |record|
-            build_through_record(record)
-          end
-        end
-
-        records
-      end
-
       def insert_record(record, validate = true, raise = false)
         ensure_not_nested
 
@@ -48,6 +34,20 @@ module ActiveRecord
       end
 
       private
+        def concat_records(records)
+          ensure_not_nested
+
+          records = super(records, true)
+
+          if owner.new_record? && records
+            records.flatten.each do |record|
+              build_through_record(record)
+            end
+          end
+
+          records
+        end
+
         # The through record (built with build_record) is temporarily cached
         # so that it may be reused if insert_record is subsequently called.
         #
-- 
cgit v1.2.3


From 3969de3e876e003e305b58bd82c82d2079b52e28 Mon Sep 17 00:00:00 2001
From: Chris Fung <aergonaut@gmail.com>
Date: Sun, 7 Oct 2018 14:12:06 -0700
Subject: Add docs to ActiveRecord::Persistence#belongs_to_touch_method

[ci skip]
---
 activerecord/lib/active_record/persistence.rb | 2 ++
 1 file changed, 2 insertions(+)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 7ceb7d1a55..fd7466dfaf 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -593,6 +593,8 @@ module ActiveRecord
       @_association_destroy_exception = nil
     end
 
+    # The name of the method used to touch a +belongs_to+ association when the
+    # +:touch+ option is used.
     def belongs_to_touch_method
       :touch
     end
-- 
cgit v1.2.3


From 673e33eee533e104f47b5e22db2c1a879e3bbc49 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Francesco=20Rodr=C3=ADguez?= <frodsan@me.com>
Date: Mon, 8 Oct 2018 08:32:56 +0200
Subject: [ci skip] Fix typo

---
 activerecord/lib/active_record/integration.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb
index 6cf26a9792..456689ec6d 100644
--- a/activerecord/lib/active_record/integration.rb
+++ b/activerecord/lib/active_record/integration.rb
@@ -60,7 +60,7 @@ module ActiveRecord
     # the cache key will also include a version.
     #
     #   Product.cache_versioning = false
-    #   Person.find(5).cache_key  # => "people/5-20071224150000" (updated_at available)
+    #   Product.find(5).cache_key  # => "products/5-20071224150000" (updated_at available)
     def cache_key(*timestamp_names)
       if new_record?
         "#{model_name.cache_key}/new"
-- 
cgit v1.2.3


From 0d435c17b743ae4ae10cff08acd7e7589e60ce4d Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Thu, 4 Oct 2018 16:34:43 -0400
Subject: Move db:migrate:status to DatabaseTasks method

---
 .../lib/active_record/railties/databases.rake      | 13 +-----------
 .../lib/active_record/tasks/database_tasks.rb      | 15 ++++++++++++++
 .../test/cases/tasks/database_tasks_test.rb        | 24 +++++++++++++++++++++-
 3 files changed, 39 insertions(+), 13 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake
index be5ef350a9..748fd65aa2 100644
--- a/activerecord/lib/active_record/railties/databases.rake
+++ b/activerecord/lib/active_record/railties/databases.rake
@@ -149,18 +149,7 @@ db_namespace = namespace :db do
 
     desc "Display status of migrations"
     task status: :load_config do
-      unless ActiveRecord::SchemaMigration.table_exists?
-        abort "Schema migrations table does not exist yet."
-      end
-
-      # output
-      puts "\ndatabase: #{ActiveRecord::Base.connection_config[:database]}\n\n"
-      puts "#{'Status'.center(8)}  #{'Migration ID'.ljust(14)}  Migration Name"
-      puts "-" * 50
-      ActiveRecord::Base.connection.migration_context.migrations_status.each do |status, version, name|
-        puts "#{status.center(8)}  #{version.ljust(14)}  #{name}"
-      end
-      puts
+      ActiveRecord::Tasks::DatabaseTasks.migrate_status
     end
   end
 
diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb
index 5e29085aff..974d7a1c0a 100644
--- a/activerecord/lib/active_record/tasks/database_tasks.rb
+++ b/activerecord/lib/active_record/tasks/database_tasks.rb
@@ -197,6 +197,21 @@ module ActiveRecord
         Migration.verbose = verbose_was
       end
 
+      def migrate_status
+        unless ActiveRecord::SchemaMigration.table_exists?
+          Kernel.abort "Schema migrations table does not exist yet."
+        end
+
+        # output
+        puts "\ndatabase: #{ActiveRecord::Base.connection_config[:database]}\n\n"
+        puts "#{'Status'.center(8)}  #{'Migration ID'.ljust(14)}  Migration Name"
+        puts "-" * 50
+        ActiveRecord::Base.connection.migration_context.migrations_status.each do |status, version, name|
+          puts "#{status.center(8)}  #{version.ljust(14)}  #{name}"
+        end
+        puts
+      end
+
       def check_target_version
         if target_version && !(Migration::MigrationFilenameRegexp.match?(ENV["VERSION"]) || /\A\d+\z/.match?(ENV["VERSION"]))
           raise "Invalid format of target version: `VERSION=#{ENV['VERSION']}`"
diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb
index d674bd562f..3fd1813d64 100644
--- a/activerecord/test/cases/tasks/database_tasks_test.rb
+++ b/activerecord/test/cases/tasks/database_tasks_test.rb
@@ -731,7 +731,7 @@ module ActiveRecord
   end
 
   if current_adapter?(:SQLite3Adapter) && !in_memory_db?
-    class DatabaseTasksMigrateTest < ActiveRecord::TestCase
+    class DatabaseTasksMigrationTestCase < ActiveRecord::TestCase
       self.use_transactional_tests = false
 
       # Use a memory db here to avoid having to rollback at the end
@@ -751,7 +751,9 @@ module ActiveRecord
         @conn.release_connection if @conn
         ActiveRecord::Base.establish_connection :arunit
       end
+    end
 
+    class DatabaseTasksMigrateTest < DatabaseTasksMigrationTestCase
       def test_migrate_set_and_unset_verbose_and_version_env_vars
         verbose, version = ENV["VERBOSE"], ENV["VERSION"]
         ENV["VERSION"] = "2"
@@ -812,6 +814,26 @@ module ActiveRecord
           end
         end
     end
+
+    class DatabaseTasksMigrateStatusTest < DatabaseTasksMigrationTestCase
+      def test_migrate_status_table
+        ActiveRecord::SchemaMigration.create_table
+        output = capture_migration_status
+        assert_match(/database: :memory:/, output)
+        assert_match(/down    001             Valid people have last names/, output)
+        assert_match(/down    002             We need reminders/, output)
+        assert_match(/down    003             Innocent jointable/, output)
+        ActiveRecord::SchemaMigration.drop_table
+      end
+
+      private
+
+        def capture_migration_status
+          capture(:stdout) do
+            ActiveRecord::Tasks::DatabaseTasks.migrate_status
+          end
+        end
+    end
   end
 
   class DatabaseTasksMigrateErrorTest < ActiveRecord::TestCase
-- 
cgit v1.2.3


From a1ee4a9ff9d4a3cb255365310ead0dc7b739c6be Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Tue, 9 Oct 2018 03:31:34 +0900
Subject: Call `define_attribute_methods` before `assert_no_queries` to address
 CI flakiness

Follow up 45be690f8e6db019aac6198ba49d608a2e14824b.

Somehow calling `define_attribute_methods` in `build`/`new` sometimes
causes the `table_exists?` query.

To address CI flakiness due to `assert_no_queries` failure, ensure
`define_attribute_methods` before `assert_no_queries`.
---
 .../has_and_belongs_to_many_associations_test.rb   |  8 ++++
 .../associations/has_many_associations_test.rb     | 49 +++++++++++++++++++---
 .../has_many_through_associations_test.rb          |  6 +++
 .../associations/has_one_associations_test.rb      | 10 +++--
 .../test/cases/autosave_association_test.rb        | 16 +++++++
 5 files changed, 80 insertions(+), 9 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index a90dcc0576..38b121d37b 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -310,6 +310,10 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build
     devel = Developer.find(1)
+
+    # Load schema information so we don't query below if running just this test.
+    Project.define_attribute_methods
+
     proj = assert_no_queries { devel.projects.build("name" => "Projekt") }
     assert_not_predicate devel.projects, :loaded?
 
@@ -325,6 +329,10 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
 
   def test_new_aliased_to_build
     devel = Developer.find(1)
+
+    # Load schema information so we don't query below if running just this test.
+    Project.define_attribute_methods
+
     proj = assert_no_queries { devel.projects.new("name" => "Projekt") }
     assert_not_predicate devel.projects, :loaded?
 
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 39b3747fd7..eb65f9e74f 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -458,6 +458,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_finder_method_with_dirty_target
     company = companies(:first_firm)
     new_clients = []
+
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
     assert_no_queries do
       new_clients << company.clients_of_firm.build(name: "Another Client")
       new_clients << company.clients_of_firm.build(name: "Another Client II")
@@ -478,6 +482,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_finder_bang_method_with_dirty_target
     company = companies(:first_firm)
     new_clients = []
+
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
     assert_no_queries do
       new_clients << company.clients_of_firm.build(name: "Another Client")
       new_clients << company.clients_of_firm.build(name: "Another Client II")
@@ -955,8 +963,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_transactions_when_adding_to_new_record
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
+    firm = Firm.new
     assert_no_queries do
-      firm = Firm.new
       firm.clients_of_firm.concat(Client.new("name" => "Natural Company"))
     end
   end
@@ -970,6 +981,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_new_aliased_to_build
     company = companies(:first_firm)
+
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
     new_client = assert_no_queries { company.clients_of_firm.new("name" => "Another Client") }
     assert_not_predicate company.clients_of_firm, :loaded?
 
@@ -980,6 +995,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build
     company = companies(:first_firm)
+
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
     new_client = assert_no_queries { company.clients_of_firm.build("name" => "Another Client") }
     assert_not_predicate company.clients_of_firm, :loaded?
 
@@ -1037,6 +1056,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build_many
     company = companies(:first_firm)
+
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
     new_clients = assert_no_queries { company.clients_of_firm.build([{ "name" => "Another Client" }, { "name" => "Another Client II" }]) }
     assert_equal 2, new_clients.size
   end
@@ -1049,10 +1072,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build_without_loading_association
     first_topic = topics(:first)
-    Reply.column_names
 
     assert_equal 1, first_topic.replies.length
 
+    # Load schema information so we don't query below if running just this test.
+    Reply.define_attribute_methods
+
     assert_no_queries do
       first_topic.replies.build(title: "Not saved", content: "Superstars")
       assert_equal 2, first_topic.replies.size
@@ -1063,6 +1088,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build_via_block
     company = companies(:first_firm)
+
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
     new_client = assert_no_queries { company.clients_of_firm.build { |client| client.name = "Another Client" } }
     assert_not_predicate company.clients_of_firm, :loaded?
 
@@ -1073,6 +1102,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build_many_via_block
     company = companies(:first_firm)
+
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
     new_clients = assert_no_queries do
       company.clients_of_firm.build([{ "name" => "Another Client" }, { "name" => "Another Client II" }]) do |client|
         client.name = "changed"
@@ -1086,8 +1119,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_create_without_loading_association
     first_firm = companies(:first_firm)
-    Firm.column_names
-    Client.column_names
 
     assert_equal 2, first_firm.clients_of_firm.size
     first_firm.clients_of_firm.reset
@@ -1364,8 +1395,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_transaction_when_deleting_new_record
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
+    firm = Firm.new
     assert_no_queries do
-      firm = Firm.new
       client = Client.new("name" => "New Client")
       firm.clients_of_firm << client
       firm.clients_of_firm.destroy(client)
@@ -1822,8 +1856,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_transactions_when_replacing_on_new_record
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
+    firm = Firm.new
     assert_no_queries do
-      firm = Firm.new
       firm.clients_of_firm = [Client.new("name" => "New Client")]
     end
   end
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 94ad6e2d4d..7b405c74c4 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -274,6 +274,9 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     assert_queries(1) { posts(:thinking) }
     new_person = nil # so block binding catches it
 
+    # Load schema information so we don't query below if running just this test.
+    Person.define_attribute_methods
+
     assert_no_queries do
       new_person = Person.new first_name: "bob"
     end
@@ -294,6 +297,9 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   def test_associate_new_by_building
     assert_queries(1) { posts(:thinking) }
 
+    # Load schema information so we don't query below if running just this test.
+    Person.define_attribute_methods
+
     assert_no_queries do
       posts(:thinking).people.build(first_name: "Bob")
       posts(:thinking).people.new(first_name: "Ted")
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index 885d9d7c2c..801720b214 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -231,9 +231,13 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_build_association_dont_create_transaction
-    assert_no_queries {
-      Firm.new.build_account
-    }
+    # Load schema information so we don't query below if running just this test.
+    Account.define_attribute_methods
+
+    firm = Firm.new
+    assert_no_queries do
+      firm.build_account
+    end
   end
 
   def test_building_the_associated_object_with_implicit_sti_base_class
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index 42d47527b2..88df0eed55 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -642,6 +642,10 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa
 
   def test_build_before_save
     company = companies(:first_firm)
+
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
     new_client = assert_no_queries { company.clients_of_firm.build("name" => "Another Client") }
     assert_not_predicate company.clients_of_firm, :loaded?
 
@@ -653,6 +657,10 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa
 
   def test_build_many_before_save
     company = companies(:first_firm)
+
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
     assert_no_queries { company.clients_of_firm.build([{ "name" => "Another Client" }, { "name" => "Another Client II" }]) }
 
     company.name += "-changed"
@@ -662,6 +670,10 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa
 
   def test_build_via_block_before_save
     company = companies(:first_firm)
+
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
     new_client = assert_no_queries { company.clients_of_firm.build { |client| client.name = "Another Client" } }
     assert_not_predicate company.clients_of_firm, :loaded?
 
@@ -673,6 +685,10 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa
 
   def test_build_many_via_block_before_save
     company = companies(:first_firm)
+
+    # Load schema information so we don't query below if running just this test.
+    Client.define_attribute_methods
+
     assert_no_queries do
       company.clients_of_firm.build([{ "name" => "Another Client" }, { "name" => "Another Client II" }]) do |client|
         client.name = "changed"
-- 
cgit v1.2.3


From 136b738cd261a7f54f478f9fb160afb9f5e50a02 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Mon, 8 Oct 2018 16:45:05 +0900
Subject: Generate delegation methods to named scope in the definition time

The delegation methods to named scope are defined when `method_missing`
is invoked on the relation.

Since #29301, the receiver in the named scope is changed to the relation
like others (e.g. `default_scope`, etc) for consistency.

Most named scopes would be delegated from relation by `method_missing`,
since we don't allow scopes to be defined which conflict with instance
methods on `Relation` (#31179). But if a named scope is defined with the
same name as any method on the `superclass` (e.g. `Kernel.open`), the
`method_missing` on the relation is not invoked.

To address the issue, make the delegation methods to named scope is
generated in the definition time.

Fixes #34098.
---
 .../lib/active_record/relation/delegation.rb       | 30 ++++++++++++++++++++++
 activerecord/lib/active_record/scoping/named.rb    |  2 ++
 .../associations/has_many_associations_test.rb     |  1 +
 activerecord/test/cases/relations_test.rb          | 10 ++++++++
 activerecord/test/models/account.rb                | 13 +++++++---
 activerecord/test/models/post.rb                   |  8 ++++--
 6 files changed, 59 insertions(+), 5 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb
index 8f657840f5..383dc1bf4b 100644
--- a/activerecord/lib/active_record/relation/delegation.rb
+++ b/activerecord/lib/active_record/relation/delegation.rb
@@ -17,6 +17,7 @@ module ActiveRecord
           delegate = Class.new(klass) {
             include ClassSpecificRelation
           }
+          include_relation_methods(delegate)
           mangled_name = klass.name.gsub("::", "_")
           const_set mangled_name, delegate
           private_constant mangled_name
@@ -29,6 +30,35 @@ module ActiveRecord
         child_class.initialize_relation_delegate_cache
         super
       end
+
+      protected
+        def include_relation_methods(delegate)
+          superclass.include_relation_methods(delegate) unless base_class?
+          delegate.include generated_relation_methods
+        end
+
+      private
+        def generated_relation_methods
+          @generated_relation_methods ||= Module.new.tap do |mod|
+            mod_name = "GeneratedRelationMethods"
+            const_set mod_name, mod
+            private_constant mod_name
+          end
+        end
+
+        def generate_relation_method(method)
+          if /\A[a-zA-Z_]\w*[!?]?\z/.match?(method)
+            generated_relation_methods.module_eval <<-RUBY, __FILE__, __LINE__ + 1
+              def #{method}(*args, &block)
+                scoping { klass.#{method}(*args, &block) }
+              end
+            RUBY
+          else
+            generated_relation_methods.send(:define_method, method) do |*args, &block|
+              scoping { klass.public_send(method, *args, &block) }
+            end
+          end
+        end
     end
 
     extend ActiveSupport::Concern
diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb
index 573d97b819..d5cc5db97e 100644
--- a/activerecord/lib/active_record/scoping/named.rb
+++ b/activerecord/lib/active_record/scoping/named.rb
@@ -191,6 +191,8 @@ module ActiveRecord
               scope
             end
           end
+
+          generate_relation_method(name)
         end
 
         private
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index eb65f9e74f..2d16815da8 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -2391,6 +2391,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_collection_association_with_private_kernel_method
     firm = companies(:first_firm)
     assert_equal [accounts(:signals37)], firm.accounts.open
+    assert_equal [accounts(:signals37)], firm.accounts.available
   end
 
   def test_association_with_or_doesnt_set_inverse_instance_key
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 9914a61033..e471ee8039 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -1812,6 +1812,16 @@ class RelationTest < ActiveRecord::TestCase
     assert_equal "Thank you for the welcome,Thank you again for the welcome", Post.first.comments.join(",")
   end
 
+  def test_relation_with_private_kernel_method
+    accounts = Account.all
+    assert_equal [accounts(:signals37)], accounts.open
+    assert_equal [accounts(:signals37)], accounts.available
+
+    sub_accounts = SubAccount.all
+    assert_equal [accounts(:signals37)], sub_accounts.open
+    assert_equal [accounts(:signals37)], sub_accounts.available
+  end
+
   test "#skip_query_cache!" do
     Post.cache do
       assert_queries(1) do
diff --git a/activerecord/test/models/account.rb b/activerecord/test/models/account.rb
index 0c3cd45a81..639e395743 100644
--- a/activerecord/test/models/account.rb
+++ b/activerecord/test/models/account.rb
@@ -11,9 +11,8 @@ class Account < ActiveRecord::Base
   end
 
   # Test private kernel method through collection proxy using has_many.
-  def self.open
-    where("firm_name = ?", "37signals")
-  end
+  scope :open, -> { where("firm_name = ?", "37signals") }
+  scope :available, -> { open }
 
   before_destroy do |account|
     if account.firm
@@ -32,3 +31,11 @@ class Account < ActiveRecord::Base
       "Sir, yes sir!"
     end
 end
+
+class SubAccount < Account
+  def self.instantiate_instance_of(klass, attributes, column_types = {}, &block)
+    klass = superclass
+    super
+  end
+  private_class_method :instantiate_instance_of
+end
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index 528585fb75..710a75ad30 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -297,8 +297,6 @@ end
 class FakeKlass
   extend ActiveRecord::Delegation::DelegateCache
 
-  inherited self
-
   class << self
     def connection
       Post.connection
@@ -335,5 +333,11 @@ class FakeKlass
     def predicate_builder
       Post.predicate_builder
     end
+
+    def base_class?
+      true
+    end
   end
+
+  inherited self
 end
-- 
cgit v1.2.3


From 92ccb7c75fd0d07479ea3b458b405b8e6667e05f Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Wed, 10 Oct 2018 05:31:20 +0900
Subject: Improve DELETE with JOIN handling to avoid subqueries if possible

Before:

```
  Pet Destroy (0.8ms)  DELETE FROM `pets` WHERE `pets`.`pet_id` IN (SELECT `pet_id` FROM (SELECT DISTINCT `pets`.`pet_id` FROM `pets` LEFT OUTER JOIN `toys` ON `toys`.`pet_id` = `pets`.`pet_id` WHERE `toys`.`name` = ?) AS __active_record_temp)  [["name", "Bone"]]
```

After:

```
  Pet Destroy (1.0ms)  DELETE `pets` FROM `pets` LEFT OUTER JOIN `toys` ON `toys`.`pet_id` = `pets`.`pet_id` WHERE `toys`.`name` = ?  [["name", "Bone"]]
```
---
 activerecord/lib/arel/visitors/mysql.rb  | 9 +--------
 activerecord/lib/arel/visitors/to_sql.rb | 8 +++++++-
 2 files changed, 8 insertions(+), 9 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/arel/visitors/mysql.rb b/activerecord/lib/arel/visitors/mysql.rb
index 9d9294fecc..5d10088427 100644
--- a/activerecord/lib/arel/visitors/mysql.rb
+++ b/activerecord/lib/arel/visitors/mysql.rb
@@ -75,14 +75,7 @@ module Arel # :nodoc: all
             o
           end
         end
-
-        def prepare_delete_statement(o)
-          if o.offset || has_join_sources?(o)
-            super
-          else
-            o
-          end
-        end
+        alias :prepare_delete_statement :prepare_update_statement
 
         # MySQL is too stupid to create a temporary table for use subquery, so we have
         # to give it some prompting in the form of a subsubquery.
diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb
index 7c0f6c2e97..a55450956b 100644
--- a/activerecord/lib/arel/visitors/to_sql.rb
+++ b/activerecord/lib/arel/visitors/to_sql.rb
@@ -76,7 +76,13 @@ module Arel # :nodoc: all
         def visit_Arel_Nodes_DeleteStatement(o, collector)
           o = prepare_delete_statement(o)
 
-          collector << "DELETE FROM "
+          if has_join_sources?(o)
+            collector << "DELETE "
+            visit o.relation.left, collector
+            collector << " FROM "
+          else
+            collector << "DELETE FROM "
+          end
           collector = visit o.relation, collector
 
           collect_where_for(o, collector)
-- 
cgit v1.2.3


From 287c0de8a10d070098a59f4be2772291c7b67576 Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Thu, 4 Oct 2018 15:30:50 -0400
Subject: Add multi-db support to rails db:migrate:status

---
 activerecord/lib/active_record/railties/databases.rake | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake
index 748fd65aa2..1c7ceb4981 100644
--- a/activerecord/lib/active_record/railties/databases.rake
+++ b/activerecord/lib/active_record/railties/databases.rake
@@ -149,7 +149,21 @@ db_namespace = namespace :db do
 
     desc "Display status of migrations"
     task status: :load_config do
-      ActiveRecord::Tasks::DatabaseTasks.migrate_status
+      ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
+        ActiveRecord::Base.establish_connection(db_config.config)
+        ActiveRecord::Tasks::DatabaseTasks.migrate_status
+      end
+    end
+
+    namespace :status do
+      ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
+        desc "Display status of migrations for #{spec_name} database"
+        task spec_name => :load_config do
+          db_config = ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, spec_name: spec_name)
+          ActiveRecord::Base.establish_connection(db_config.config)
+          ActiveRecord::Tasks::DatabaseTasks.migrate_status
+        end
+      end
     end
   end
 
-- 
cgit v1.2.3


From f569955e061dd8a587f1052fe30cb9684eeb848a Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Wed, 10 Oct 2018 06:48:20 +0900
Subject: Refactor Arel visitor to use `collect_nodes_for` as much as possible

---
 activerecord/lib/arel/visitors/to_sql.rb | 43 ++++++++------------------------
 1 file changed, 10 insertions(+), 33 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb
index a55450956b..7ce26884a5 100644
--- a/activerecord/lib/arel/visitors/to_sql.rb
+++ b/activerecord/lib/arel/visitors/to_sql.rb
@@ -85,7 +85,9 @@ module Arel # :nodoc: all
           end
           collector = visit o.relation, collector
 
-          collect_where_for(o, collector)
+          collect_nodes_for o.wheres, collector, " WHERE ", " AND "
+          collect_nodes_for o.orders, collector, " ORDER BY "
+          maybe_visit o.limit, collector
         end
 
         def visit_Arel_Nodes_UpdateStatement(o, collector)
@@ -93,12 +95,11 @@ module Arel # :nodoc: all
 
           collector << "UPDATE "
           collector = visit o.relation, collector
-          unless o.values.empty?
-            collector << " SET "
-            collector = inject_join o.values, collector, ", "
-          end
+          collect_nodes_for o.values, collector, " SET "
 
-          collect_where_for(o, collector)
+          collect_nodes_for o.wheres, collector, " WHERE ", " AND "
+          collect_nodes_for o.orders, collector, " ORDER BY "
+          maybe_visit o.limit, collector
         end
 
         def visit_Arel_Nodes_InsertStatement(o, collector)
@@ -231,10 +232,7 @@ module Arel # :nodoc: all
 
           collect_nodes_for o.wheres, collector, WHERE, AND
           collect_nodes_for o.groups, collector, GROUP_BY
-          unless o.havings.empty?
-            collector << " HAVING "
-            inject_join o.havings, collector, AND
-          end
+          collect_nodes_for o.havings, collector, " HAVING ", AND
           collect_nodes_for o.windows, collector, WINDOW
 
           collector
@@ -243,11 +241,7 @@ module Arel # :nodoc: all
         def collect_nodes_for(nodes, collector, spacer, connector = COMMA)
           unless nodes.empty?
             collector << spacer
-            len = nodes.length - 1
-            nodes.each_with_index do |x, i|
-              collector = visit(x, collector)
-              collector << connector unless len == i
-            end
+            inject_join nodes, collector, connector
           end
         end
 
@@ -302,10 +296,7 @@ module Arel # :nodoc: all
         def visit_Arel_Nodes_Window(o, collector)
           collector << "("
 
-          if o.partitions.any?
-            collector << "PARTITION BY "
-            collector = inject_join o.partitions, collector, ", "
-          end
+          collect_nodes_for o.partitions, collector, "PARTITION BY "
 
           if o.orders.any?
             collector << SPACE if o.partitions.any?
@@ -836,20 +827,6 @@ module Arel # :nodoc: all
           stmt
         end
 
-        def collect_where_for(o, collector)
-          unless o.wheres.empty?
-            collector << " WHERE "
-            collector = inject_join o.wheres, collector, " AND "
-          end
-
-          unless o.orders.empty?
-            collector << " ORDER BY "
-            collector = inject_join o.orders, collector, ", "
-          end
-
-          maybe_visit o.limit, collector
-        end
-
         def infix_value(o, collector, value)
           collector = visit o.left, collector
           collector << value
-- 
cgit v1.2.3


From e0d147fa905eb5cd71797777c69b0aaff0802eb5 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Wed, 10 Oct 2018 08:03:06 +0900
Subject: Fix odd indentation

---
 activerecord/lib/arel/visitors/mysql.rb | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/arel/visitors/mysql.rb b/activerecord/lib/arel/visitors/mysql.rb
index 5d10088427..32f6705d04 100644
--- a/activerecord/lib/arel/visitors/mysql.rb
+++ b/activerecord/lib/arel/visitors/mysql.rb
@@ -9,20 +9,20 @@ module Arel # :nodoc: all
             collector << "( "
           end
 
-          collector =   case o.left
-                        when Arel::Nodes::Union
-                          visit_Arel_Nodes_Union o.left, collector, true
-                        else
-                          visit o.left, collector
+          case o.left
+          when Arel::Nodes::Union
+            visit_Arel_Nodes_Union o.left, collector, true
+          else
+            visit o.left, collector
           end
 
           collector << " UNION "
 
-          collector =    case o.right
-                         when Arel::Nodes::Union
-                           visit_Arel_Nodes_Union o.right, collector, true
-                         else
-                           visit o.right, collector
+          case o.right
+          when Arel::Nodes::Union
+            visit_Arel_Nodes_Union o.right, collector, true
+          else
+            visit o.right, collector
           end
 
           if suppress_parens
-- 
cgit v1.2.3


From 961776832d13b4cea7f67aface2c2882416ac975 Mon Sep 17 00:00:00 2001
From: Christophe Maximin <christophe.maximin@gmail.com>
Date: Fri, 5 Oct 2018 13:00:45 +0100
Subject: Clear QueryCache when reloading associations

---
 activerecord/CHANGELOG.md                          | 17 +++++++++
 .../lib/active_record/associations/association.rb  |  4 ++-
 .../active_record/associations/collection_proxy.rb |  2 +-
 .../associations/singular_association.rb           |  2 +-
 .../associations/belongs_to_associations_test.rb   | 24 +++++++++++++
 .../associations/has_many_associations_test.rb     | 42 ++++++++++++++++++++++
 .../associations/has_one_associations_test.rb      | 23 ++++++++++++
 7 files changed, 111 insertions(+), 3 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 96fd6a62c6..19558a9e0c 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,20 @@
+*   Reloading associations now clears the Query Cache like `Persistence#reload` does.
+
+    ```
+    class Post < ActiveRecord::Base
+      has_one :category
+      belongs_to :author
+      has_many :comments
+    end
+
+    # Each of the following will now clear the query cache.
+    post.reload_category
+    post.reload_author
+    post.comments.reload
+    ```
+
+    *Christophe Maximin*
+
 *   Added `index` option for `change_table` migration helpers.
     With this change you can create indexes while adding new
     columns into the existing tables.
diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb
index 44596f4424..ab2da75c37 100644
--- a/activerecord/lib/active_record/associations/association.rb
+++ b/activerecord/lib/active_record/associations/association.rb
@@ -40,7 +40,9 @@ module ActiveRecord
       end
 
       # Reloads the \target and returns +self+ on success.
-      def reload
+      # The QueryCache is cleared if +force+ is true.
+      def reload(force = false)
+        klass.connection.clear_query_cache if force && klass
         reset
         reset_scope
         load_target
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index 9a30198b95..08b7c9274d 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -1088,7 +1088,7 @@ module ActiveRecord
       #   person.pets.reload # fetches pets from the database
       #   # => [#<Pet id: 1, name: "Snoop", group: "dogs", person_id: 1>]
       def reload
-        proxy_association.reload
+        proxy_association.reload(true)
         reset_scope
       end
 
diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb
index cfab16a745..8e50cce102 100644
--- a/activerecord/lib/active_record/associations/singular_association.rb
+++ b/activerecord/lib/active_record/associations/singular_association.rb
@@ -26,7 +26,7 @@ module ActiveRecord
       # Implements the reload reader method, e.g. foo.reload_bar for
       # Foo.has_one :bar
       def force_reload_reader
-        klass.uncached { reload }
+        reload(true)
         target
       end
 
diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index d520919332..93dd427951 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -367,6 +367,30 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
     assert_equal "ODEGY", odegy_account.reload_firm.name
   end
 
+  def test_reload_the_belonging_object_with_query_cache
+    odegy_account_id = accounts(:odegy_account).id
+
+    connection = ActiveRecord::Base.connection
+    connection.enable_query_cache!
+    connection.clear_query_cache
+
+    # Populate the cache with a query
+    odegy_account = Account.find(odegy_account_id)
+
+    # Populate the cache with a second query
+    odegy_account.firm
+
+    assert_equal 2, connection.query_cache.size
+
+    # Clear the cache and fetch the firm again, populating the cache with a query
+    assert_queries(1) { odegy_account.reload_firm }
+
+    # This query is not cached anymore, so it should make a real SQL query
+    assert_queries(1) { Account.find(odegy_account_id) }
+  ensure
+    ActiveRecord::Base.connection.disable_query_cache!
+  end
+
   def test_natural_assignment_to_nil
     client = Client.find(3)
     client.firm = nil
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 036df1c31e..32da272814 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -821,6 +821,48 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_not_same original_object, collection.first, "Expected #first after #reload to return a new object"
   end
 
+  def test_reload_with_query_cache
+    connection = ActiveRecord::Base.connection
+    connection.enable_query_cache!
+    connection.clear_query_cache
+
+    # Populate the cache with a query
+    firm = Firm.first
+    # Populate the cache with a second query
+    firm.clients.load
+
+    assert_equal 2, connection.query_cache.size
+
+    # Clear the cache and fetch the clients again, populating the cache with a query
+    assert_queries(1) { firm.clients.reload }
+    # This query is cached, so it shouldn't make a real SQL query
+    assert_queries(0) { firm.clients.load }
+
+    assert_equal 1, connection.query_cache.size
+  ensure
+    ActiveRecord::Base.connection.disable_query_cache!
+  end
+
+  def test_reloading_unloaded_associations_with_query_cache
+    connection = ActiveRecord::Base.connection
+    connection.enable_query_cache!
+    connection.clear_query_cache
+
+    firm = Firm.create!(name: "firm name")
+    client = firm.clients.create!(name: "client name")
+    firm.clients.to_a # add request to cache
+
+    connection.uncached do
+      client.update!(name: "new client name")
+    end
+
+    firm = Firm.find(firm.id)
+
+    assert_equal [client.name], firm.clients.reload.map(&:name)
+  ensure
+    ActiveRecord::Base.connection.disable_query_cache!
+  end
+
   def test_find_all_with_include_and_conditions
     assert_nothing_raised do
       Developer.all.merge!(joins: :audit_logs, where: { "audit_logs.message" => nil, :name => "Smith" }).to_a
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index 885d9d7c2c..1990877dbc 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -329,6 +329,29 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
     assert_equal 80, odegy.reload_account.credit_limit
   end
 
+  def test_reload_association_with_query_cache
+    odegy_id = companies(:odegy).id
+
+    connection = ActiveRecord::Base.connection
+    connection.enable_query_cache!
+    connection.clear_query_cache
+
+    # Populate the cache with a query
+    odegy = Company.find(odegy_id)
+    # Populate the cache with a second query
+    odegy.account
+
+    assert_equal 2, connection.query_cache.size
+
+    # Clear the cache and fetch the account again, populating the cache with a query
+    assert_queries(1) { odegy.reload_account }
+
+    # This query is not cached anymore, so it should make a real SQL query
+    assert_queries(1) { Company.find(odegy_id) }
+  ensure
+    ActiveRecord::Base.connection.disable_query_cache!
+  end
+
   def test_build
     firm = Firm.new("name" => "GlobalMegaCorp")
     firm.save
-- 
cgit v1.2.3


From 301409a98f2bef4f431a10cae74a3430bbaca9c2 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Wed, 10 Oct 2018 09:47:55 +0900
Subject: Call `load_schema` before `assert_no_queries`

Follow up 45be690f8e6db019aac6198ba49d608a2e14824b.

`predicate_builder.build` in `where` requires `load_schema` for
`type_for_attribute`.
---
 activerecord/test/cases/null_relation_test.rb | 1 +
 1 file changed, 1 insertion(+)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/null_relation_test.rb b/activerecord/test/cases/null_relation_test.rb
index c5d5ded5ab..ee96ea1af6 100644
--- a/activerecord/test/cases/null_relation_test.rb
+++ b/activerecord/test/cases/null_relation_test.rb
@@ -17,6 +17,7 @@ class NullRelationTest < ActiveRecord::TestCase
   end
 
   def test_none_chainable
+    Developer.send(:load_schema)
     assert_no_queries do
       assert_equal [], Developer.none.where(name: "David")
     end
-- 
cgit v1.2.3


From 9b9640112e197e94e13940c0c0f990cc80ca5498 Mon Sep 17 00:00:00 2001
From: Alberto Almagro <albertoalmagro@gmail.com>
Date: Sat, 6 Oct 2018 18:08:45 +0200
Subject: Raise on invalid definition values

When defining a Hash enum it can be easy to use [] instead of {}. This
commit checks that only valid definition values are provided, those can
be a Hash, an array of Symbols or an array of Strings. Otherwise it
raises an ArgumentError.

Fixes #33961
---
 activerecord/CHANGELOG.md              | 11 +++++++++++
 activerecord/lib/active_record/enum.rb | 10 ++++++++++
 activerecord/test/cases/enum_test.rb   | 11 +++++++++++
 3 files changed, 32 insertions(+)

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 19558a9e0c..6397bc361a 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,14 @@
+*   Enum raises on invalid definition values
+
+    When defining a Hash enum it can be easy to use [] instead of {}. This
+    commit checks that only valid definition values are provided, those can
+    be a Hash, an array of Symbols or an array of Strings. Otherwise it
+    raises an ArgumentError.
+
+    Fixes #33961
+
+    *Alberto Almagro*
+
 *   Reloading associations now clears the Query Cache like `Persistence#reload` does.
 
     ```
diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb
index 23ecb24542..49fedc232c 100644
--- a/activerecord/lib/active_record/enum.rb
+++ b/activerecord/lib/active_record/enum.rb
@@ -150,6 +150,7 @@ module ActiveRecord
       enum_prefix = definitions.delete(:_prefix)
       enum_suffix = definitions.delete(:_suffix)
       definitions.each do |name, values|
+        assert_valid_enum_definition_values(values)
         # statuses = { }
         enum_values = ActiveSupport::HashWithIndifferentAccess.new
         name = name.to_s
@@ -210,6 +211,15 @@ module ActiveRecord
         end
       end
 
+      def assert_valid_enum_definition_values(values)
+        unless values.is_a?(Hash) || values.all? { |v| v.is_a?(Symbol) } || values.all? { |v| v.is_a?(String) }
+          error_message = <<~MSG
+            Enum values #{values} must be either a hash, an array of symbols, or an array of strings.
+          MSG
+          raise ArgumentError, error_message
+        end
+      end
+
       ENUM_CONFLICT_MESSAGE = \
         "You tried to define an enum named \"%{enum}\" on the model \"%{klass}\", but " \
         "this will generate a %{type} method \"%{method}\", which is already defined " \
diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb
index d5a1d11e12..b4593ccdf2 100644
--- a/activerecord/test/cases/enum_test.rb
+++ b/activerecord/test/cases/enum_test.rb
@@ -265,6 +265,17 @@ class EnumTest < ActiveRecord::TestCase
     assert_equal "published", @book.status
   end
 
+  test "invalid definition values raise an ArgumentError" do
+    e = assert_raises(ArgumentError) do
+      Class.new(ActiveRecord::Base) do
+        self.table_name = "books"
+        enum status: [proposed: 1, written: 2, published: 3]
+      end
+    end
+
+    assert_match(/must be either a hash, an array of symbols, or an array of strings./, e.message)
+  end
+
   test "reserved enum names" do
     klass = Class.new(ActiveRecord::Base) do
       self.table_name = "books"
-- 
cgit v1.2.3


From df11558942f9eb97f9ed0293d1d4508f7685bb51 Mon Sep 17 00:00:00 2001
From: Alberto Almagro <albertoalmagro@gmail.com>
Date: Sat, 6 Oct 2018 18:15:14 +0200
Subject: Privatize ENUM_CONFLICT_MESSAGE constant

---
 activerecord/lib/active_record/enum.rb | 1 +
 1 file changed, 1 insertion(+)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb
index 49fedc232c..3d97e4e513 100644
--- a/activerecord/lib/active_record/enum.rb
+++ b/activerecord/lib/active_record/enum.rb
@@ -224,6 +224,7 @@ module ActiveRecord
         "You tried to define an enum named \"%{enum}\" on the model \"%{klass}\", but " \
         "this will generate a %{type} method \"%{method}\", which is already defined " \
         "by %{source}."
+      private_constant :ENUM_CONFLICT_MESSAGE
 
       def detect_enum_conflict!(enum_name, method_name, klass_method = false)
         if klass_method && dangerous_class_method?(method_name)
-- 
cgit v1.2.3


From 31021a8c85bb80300deb01db96876858ff417f4a Mon Sep 17 00:00:00 2001
From: Eileen Uchitelle <eileencodes@gmail.com>
Date: Fri, 28 Sep 2018 16:52:54 -0400
Subject: Basic API for connection switching

This PR adds the ability to 1) connect to multiple databases in a model,
and 2) switch between those connections using a block.

To connect a model to a set of databases for writing and reading use
the following API. This API supercedes `establish_connection`. The
`writing` and `reading` keys represent handler / role names and
`animals` and `animals_replica` represents the database key to look up
the configuration hash from.

```
class AnimalsBase < ApplicationRecord
  connects_to database: { writing: :animals, reading: :animals_replica }
end
```

Inside the application - outside the model declaration - we can switch
connections with a block call to `connected_to`.

If we want to connect to a db that isn't default (ie readonly_slow) we
can connect like this:

Outside the model we may want to connect to a new database (one that is
not in the default writing/reading set) - for example a slow replica for
making slow queries. To do this we have the `connected_to` method that
takes a `database` hash that matches the signature of `connects_to`. The
`connected_to` method also takes a block.

```
AcitveRecord::Base.connected_to(database: { slow_readonly: :primary_replica_slow }) do
  ModelInPrimary.do_something_thats_slow
end
```

For models that are already loaded and connections that are already
connected, `connected_to` doesn't need to pass in a `database` because
you may want to run queries against multiple databases using a specific
role/handler.

In this case `connected_to` can take a `role` and use that to swap on
the connection passed. This simplies queries - and matches how we do it
in GitHub. Once you're connected to the database you don't need to
re-connect, we assume the connection is in the pool and simply pass the
handler we'd like to swap on.

```
ActiveRecord::Base.connected_to(role: :reading) do
  Dog.read_something_from_dog
  ModelInPrimary.do_something_from_model_in_primary
end
```
---
 activerecord/CHANGELOG.md                          |  36 ++++
 .../lib/active_record/connection_handling.rb       |  97 ++++++++++-
 activerecord/lib/active_record/core.rb             |   3 +
 .../connection_handlers_multi_db_test.rb           | 193 +++++++++++++++++++++
 4 files changed, 328 insertions(+), 1 deletion(-)
 create mode 100644 activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 6397bc361a..00f4ee1aaa 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,39 @@
+*   Add basic API for connection switching to support multiple databases.
+
+    1) Adds a `connects_to` method for models to connect to multiple databases. Example:
+
+    ```
+    class AnimalsModel < ApplicationRecord
+      self.abstract_class = true
+
+      connects_to database: { writing: :animals_primary, reading: :animals_replica }
+    end
+
+    class Dog < AnimalsModel
+      # connected to both the animals_primary db for writing and the animals_replica for reading
+    end
+    ```
+
+    2) Adds a `connected_to` block method for switching connection roles or connecting to
+    a database that the model didn't connect to. Connecting to the database in this block is
+    useful when you have another defined connection, for example `slow_replica` that you don't
+    want to connect to by default but need in the console, or a specific code block.
+
+    ```
+    ActiveRecord::Base.connected_to(role: :reading) do
+      Dog.first # finds dog from replica connected to AnimalsBase
+      Book.first # doesn't have a reading connection, will raise an error
+    end
+    ```
+
+    ```
+    ActiveRecord::Base.connected_to(database: :slow_replica) do
+      SlowReplicaModel.first # if the db config has a slow_replica configuration this will be used to do the lookup, otherwise this will throw an exception
+    end
+    ```
+
+    *Eileen M. Uchitelle*
+
 *   Enum raises on invalid definition values
 
     When defining a Hash enum it can be easy to use [] instead of {}. This
diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb
index 18114f9e1c..5141271db9 100644
--- a/activerecord/lib/active_record/connection_handling.rb
+++ b/activerecord/lib/active_record/connection_handling.rb
@@ -47,6 +47,92 @@ module ActiveRecord
     # The exceptions AdapterNotSpecified, AdapterNotFound and +ArgumentError+
     # may be returned on an error.
     def establish_connection(config_or_env = nil)
+      config_hash = resolve_config_for_connection(config_or_env)
+      connection_handler.establish_connection(config_hash)
+    end
+
+    # Connects a model to the databases specified. The +database+ keyword
+    # takes a hash consisting of a +role+ and a +database_key+.
+    #
+    # This will create a connection handler for switching between connections,
+    # look up the config hash using the +database_key+ and finally
+    # establishes a connection to that config.
+    #
+    #   class AnimalsModel < ApplicationRecord
+    #     self.abstract_class = true
+    #
+    #     connects_to database: { writing: :primary, reading: :primary_replica }
+    #   end
+    #
+    # Returns an array of established connections.
+    def connects_to(database: {})
+      connections = []
+
+      database.each do |role, database_key|
+        config_hash = resolve_config_for_connection(database_key)
+        handler = lookup_connection_handler(role.to_sym)
+
+        connections << handler.establish_connection(config_hash)
+      end
+
+      connections
+    end
+
+    # Connects to a database or role (ex writing, reading, or another
+    # custom role) for the duration of the block.
+    #
+    # If a role is passed, Active Record will look up the connection
+    # based on the requested role:
+    #
+    #   ActiveRecord::Base.connected_to(role: :writing) do
+    #     Dog.create! # creates dog using dog connection
+    #   end
+    #
+    #   ActiveRecord::Base.connected_to(role: :reading) do
+    #     Dog.create! # throws exception because we're on a replica
+    #   end
+    #
+    #   ActiveRecord::Base.connected_to(role: :unknown_ode) do
+    #     # raises exception due to non-existent role
+    #   end
+    #
+    # For cases where you may want to connect to a database outside of the model,
+    # you can use +connected_to+ with a +database+ argument. The +database+ argument
+    # expects a symbol that corresponds to the database key in your config.
+    #
+    # This will connect to a new database for the queries inside the block.
+    #
+    #   ActiveRecord::Base.connected_to(database: :animals_slow_replica) do
+    #     Dog.run_a_long_query # runs a long query while connected to the +animals_slow_replica+
+    #   end
+    def connected_to(database: nil, role: nil, &blk)
+      if database && role
+        raise ArgumentError, "connected_to can only accept a database or role argument, but not both arguments."
+      elsif database
+        config_hash = resolve_config_for_connection(database)
+        handler = lookup_connection_handler(database.to_sym)
+
+        with_handler(database.to_sym) do
+          handler.establish_connection(config_hash)
+          return yield
+        end
+      elsif role
+        with_handler(role.to_sym, &blk)
+      else
+        raise ArgumentError, "must provide a `database` or a `role`."
+      end
+    end
+
+    def lookup_connection_handler(handler_key) # :nodoc:
+      connection_handlers[handler_key] ||= ActiveRecord::ConnectionAdapters::ConnectionHandler.new
+    end
+
+    def with_handler(handler_key, &blk) # :nodoc:
+      handler = lookup_connection_handler(handler_key)
+      swap_connection_handler(handler, &blk)
+    end
+
+    def resolve_config_for_connection(config_or_env) # :nodoc:
       raise "Anonymous class is not allowed." unless name
 
       config_or_env ||= DEFAULT_ENV.call.to_sym
@@ -57,7 +143,7 @@ module ActiveRecord
       config_hash = resolver.resolve(config_or_env, pool_name).symbolize_keys
       config_hash[:name] = pool_name
 
-      connection_handler.establish_connection(config_hash)
+      config_hash
     end
 
     # Returns the connection currently associated with the class. This can
@@ -118,5 +204,14 @@ module ActiveRecord
 
     delegate :clear_active_connections!, :clear_reloadable_connections!,
       :clear_all_connections!, :flush_idle_connections!, to: :connection_handler
+
+    private
+
+      def swap_connection_handler(handler, &blk) # :nodoc:
+        old_handler, ActiveRecord::Base.connection_handler = ActiveRecord::Base.connection_handler, handler
+        yield
+      ensure
+        ActiveRecord::Base.connection_handler = old_handler
+      end
   end
 end
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index 392602bc0f..b53681ee20 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -124,6 +124,8 @@ module ActiveRecord
 
       mattr_accessor :belongs_to_required_by_default, instance_accessor: false
 
+      mattr_accessor :connection_handlers, instance_accessor: false, default: {}
+
       class_attribute :default_connection_handler, instance_writer: false
 
       self.filter_attributes = []
@@ -137,6 +139,7 @@ module ActiveRecord
       end
 
       self.default_connection_handler = ConnectionAdapters::ConnectionHandler.new
+      self.connection_handlers = { writing: ActiveRecord::Base.default_connection_handler }
     end
 
     module ClassMethods
diff --git a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb
new file mode 100644
index 0000000000..d4e8cbee81
--- /dev/null
+++ b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb
@@ -0,0 +1,193 @@
+# frozen_string_literal: true
+
+require "cases/helper"
+require "models/person"
+
+module ActiveRecord
+  module ConnectionAdapters
+    class ConnectionHandlersMultiDbTest < ActiveRecord::TestCase
+      self.use_transactional_tests = false
+
+      fixtures :people
+
+      def setup
+        @handlers = { writing: ConnectionHandler.new, reading: ConnectionHandler.new }
+        @rw_handler = @handlers[:writing]
+        @ro_handler = @handlers[:reading]
+        @spec_name = "primary"
+        @rw_pool = @handlers[:writing].establish_connection(ActiveRecord::Base.configurations["arunit"])
+        @ro_pool = @handlers[:reading].establish_connection(ActiveRecord::Base.configurations["arunit"])
+      end
+
+      def teardown
+        ActiveRecord::Base.connection_handlers = { writing: ActiveRecord::Base.default_connection_handler }
+      end
+
+      class MultiConnectionTestModel < ActiveRecord::Base
+      end
+
+      def test_multiple_connection_handlers_works_in_a_threaded_environment
+        tf_writing = Tempfile.open "test_writing"
+        tf_reading = Tempfile.open "test_reading"
+
+        MultiConnectionTestModel.connects_to database: { writing: { database: tf_writing.path, adapter: "sqlite3" }, reading: { database: tf_reading.path, adapter: "sqlite3" } }
+
+        MultiConnectionTestModel.connection.execute("CREATE TABLE `test_1` (connection_role VARCHAR (255))")
+        MultiConnectionTestModel.connection.execute("INSERT INTO test_1 VALUES ('writing')")
+
+        ActiveRecord::Base.connected_to(role: :reading) do
+          MultiConnectionTestModel.connection.execute("CREATE TABLE `test_1` (connection_role VARCHAR (255))")
+          MultiConnectionTestModel.connection.execute("INSERT INTO test_1 VALUES ('reading')")
+        end
+
+        read_latch = Concurrent::CountDownLatch.new
+        write_latch = Concurrent::CountDownLatch.new
+
+        MultiConnectionTestModel.connection
+
+        thread = Thread.new do
+          MultiConnectionTestModel.connection
+
+          write_latch.wait
+          assert_equal "writing", MultiConnectionTestModel.connection.select_value("SELECT connection_role from test_1")
+          read_latch.count_down
+        end
+
+        ActiveRecord::Base.connected_to(role: :reading) do
+          write_latch.count_down
+          assert_equal "reading", MultiConnectionTestModel.connection.select_value("SELECT connection_role from test_1")
+          read_latch.wait
+        end
+
+        thread.join
+      ensure
+        tf_reading.close
+        tf_reading.unlink
+        tf_writing.close
+        tf_writing.unlink
+      end
+
+      unless in_memory_db?
+        def test_establish_connection_using_3_levels_config
+          previous_env, ENV["RAILS_ENV"] = ENV["RAILS_ENV"], "default_env"
+
+          config = {
+            "default_env" => {
+              "readonly" => { "adapter" => "sqlite3", "database" => "db/readonly.sqlite3" },
+              "primary"  => { "adapter" => "sqlite3", "database" => "db/primary.sqlite3" }
+            }
+          }
+          @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config
+
+          ActiveRecord::Base.connects_to(database: { writing: :primary, reading: :readonly })
+
+          assert_not_nil pool = ActiveRecord::Base.connection_handlers[:writing].retrieve_connection_pool("primary")
+          assert_equal "db/primary.sqlite3", pool.spec.config[:database]
+
+          assert_not_nil pool = ActiveRecord::Base.connection_handlers[:reading].retrieve_connection_pool("primary")
+          assert_equal "db/readonly.sqlite3", pool.spec.config[:database]
+        ensure
+          ActiveRecord::Base.configurations = @prev_configs
+          ActiveRecord::Base.establish_connection(:arunit)
+          ENV["RAILS_ENV"] = previous_env
+        end
+
+        def test_switching_connections_via_handler
+          previous_env, ENV["RAILS_ENV"] = ENV["RAILS_ENV"], "default_env"
+
+          config = {
+            "default_env" => {
+              "readonly" => { "adapter" => "sqlite3", "database" => "db/readonly.sqlite3" },
+              "primary"  => { "adapter" => "sqlite3", "database" => "db/primary.sqlite3" }
+            }
+          }
+          @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config
+
+          ActiveRecord::Base.connects_to(database: { writing: :primary, reading: :readonly })
+
+          ActiveRecord::Base.connected_to(role: :reading) do
+            @ro_handler = ActiveRecord::Base.connection_handler
+            assert_equal ActiveRecord::Base.connection_handler, ActiveRecord::Base.connection_handlers[:reading]
+          end
+
+          ActiveRecord::Base.connected_to(role: :writing) do
+            assert_equal ActiveRecord::Base.connection_handler, ActiveRecord::Base.connection_handlers[:writing]
+            assert_not_equal @ro_handler, ActiveRecord::Base.connection_handler
+          end
+        ensure
+          ActiveRecord::Base.configurations = @prev_configs
+          ActiveRecord::Base.establish_connection(:arunit)
+          ENV["RAILS_ENV"] = previous_env
+        end
+
+        def test_connects_to_with_single_configuration
+          config = {
+            "development" => { "adapter" => "sqlite3", "database" => "db/primary.sqlite3" },
+          }
+          @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config
+
+          ActiveRecord::Base.connects_to database: { writing: :development }
+
+          assert_equal 1, ActiveRecord::Base.connection_handlers.size
+          assert_equal ActiveRecord::Base.connection_handler, ActiveRecord::Base.connection_handlers[:writing]
+        ensure
+          ActiveRecord::Base.configurations = @prev_configs
+          ActiveRecord::Base.establish_connection(:arunit)
+        end
+
+        def test_connects_to_using_top_level_key_in_two_level_config
+          config = {
+            "development" => { "adapter" => "sqlite3", "database" => "db/primary.sqlite3" },
+            "development_readonly" => { "adapter" => "sqlite3", "database" => "db/readonly.sqlite3" }
+          }
+          @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config
+
+          ActiveRecord::Base.connects_to database: { writing: :development, reading: :development_readonly }
+
+          assert_not_nil pool = ActiveRecord::Base.connection_handlers[:reading].retrieve_connection_pool("primary")
+          assert_equal "db/readonly.sqlite3", pool.spec.config[:database]
+        ensure
+          ActiveRecord::Base.configurations = @prev_configs
+          ActiveRecord::Base.establish_connection(:arunit)
+        end
+      end
+
+      def test_connection_pools
+        assert_equal([@rw_pool], @handlers[:writing].connection_pools)
+        assert_equal([@ro_pool], @handlers[:reading].connection_pools)
+      end
+
+      def test_retrieve_connection
+        assert @rw_handler.retrieve_connection(@spec_name)
+        assert @ro_handler.retrieve_connection(@spec_name)
+      end
+
+      def test_active_connections?
+        assert_not_predicate @rw_handler, :active_connections?
+        assert_not_predicate @ro_handler, :active_connections?
+
+        assert @rw_handler.retrieve_connection(@spec_name)
+        assert @ro_handler.retrieve_connection(@spec_name)
+
+        assert_predicate @rw_handler, :active_connections?
+        assert_predicate @ro_handler, :active_connections?
+
+        @rw_handler.clear_active_connections!
+        assert_not_predicate @rw_handler, :active_connections?
+
+        @ro_handler.clear_active_connections!
+        assert_not_predicate @ro_handler, :active_connections?
+      end
+
+      def test_retrieve_connection_pool
+        assert_not_nil @rw_handler.retrieve_connection_pool(@spec_name)
+        assert_not_nil @ro_handler.retrieve_connection_pool(@spec_name)
+      end
+
+      def test_retrieve_connection_pool_with_invalid_id
+        assert_nil @rw_handler.retrieve_connection_pool("foo")
+        assert_nil @ro_handler.retrieve_connection_pool("foo")
+      end
+    end
+  end
+end
-- 
cgit v1.2.3