Merge branch 'master' into set_binds

* master: (2794 commits)
  doc, API example on how to use `Model#exists?` with multiple IDs. [ci skip]
  Restore DATABASE_URL even if it's nil in connection_handler test
  [ci skip] - error_messages_for has been deprecated since 2.3.8 - lets reduce any confusion for users
  Ensure Active Record connection consistency
  Revert "ask the fixture set for the sql statements"
  Check `respond_to` before delegation due to: https://github.com/ruby/ruby/commit/d781caaf313b8649948c107bba277e5ad7307314
  Adding Hash#compact and Hash#compact! methods
  MySQL version 4.1 was EOL on December 31, 2009 We should at least recommend modern versions of MySQL to users.
  clear cache on body close so that cache remains during rendering
  add a more restricted codepath for templates fixes #13390
  refactor generator tests to use block form of Tempfile
  Fix typo [ci skip]
  Move finish_template as the last public method in the generator
  Minor typos fix [ci skip]
  make `change_column_null` reversible. Closes #13576.
  create/drop test and development databases only if RAILS_ENV is nil
  Revert "Speedup String#to"
  typo fix in test name. [ci skip].
  `core_ext/string/access.rb` test what we are documenting.
  Fix typo in image_tag documentation
  ...

Conflicts:
	activerecord/lib/active_record/associations/join_dependency/join_association.rb
	activerecord/lib/active_record/relation/query_methods.rb

1 files changed, 69 insertions, 100 deletions

diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index efd7ecb97c..52531a3520 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -4,10 +4,9 @@ module ActiveRecord
     #
     # CollectionAssociation is an abstract class that provides common stuff to
     # ease the implementation of association proxies that represent
-    # collections. See the class hierarchy in AssociationProxy.
+    # collections. See the class hierarchy in Association.
     #
     #   CollectionAssociation:
-    #     HasAndBelongsToManyAssociation => has_and_belongs_to_many
     #     HasManyAssociation => has_many
     #       HasManyThroughAssociation + ThroughAssociation => has_many :through
     #
@@ -34,7 +33,7 @@ module ActiveRecord
           reload
         end
 
-        @proxy ||= CollectionProxy.new(klass, self)
+        @proxy ||= CollectionProxy.create(klass, self)
       end
 
       # Implements the writer method, e.g. foo.items= for Foo.has_many :items
@@ -44,7 +43,7 @@ module ActiveRecord
 
       # Implements the ids reader method, e.g. foo.item_ids for Foo.has_many :items
       def ids_reader
-        if loaded? || options[:finder_sql]
+        if loaded?
           load_target.map do |record|
             record.send(reflection.association_primary_key)
           end
@@ -67,11 +66,11 @@ module ActiveRecord
         @target = []
       end
 
-      def select(select = nil)
+      def select(*fields)
         if block_given?
           load_target.select.each { |e| yield e }
         else
-          scope.select(select)
+          scope.select(*fields)
         end
       end
 
@@ -79,17 +78,14 @@ module ActiveRecord
         if block_given?
           load_target.find(*args) { |*block_args| yield(*block_args) }
         else
-          if options[:finder_sql]
-            find_by_scan(*args)
-          elsif options[:inverse_of]
-            args = args.flatten
-            raise RecordNotFound, "Couldn't find #{scope.klass.name} without an ID" if args.blank?
-
+          if options[:inverse_of] && loaded?
+            args_flatten = args.flatten
+            raise RecordNotFound, "Couldn't find #{scope.klass.name} without an ID" if args_flatten.blank?
             result = find_by_scan(*args)
 
             result_size = Array(result).size
-            if !result || result_size != args.size
-              scope.raise_record_not_found_exception!(args, result_size, args.size)
+            if !result || result_size != args_flatten.size
+              scope.raise_record_not_found_exception!(args_flatten, result_size, args_flatten.size)
             else
               result
             end
@@ -153,11 +149,33 @@ module ActiveRecord
         end
       end
 
-      # Remove all records from this association.
+      # Removes all records from the association without calling callbacks
+      # on the associated records. It honors the `:dependent` option. However
+      # if the `:dependent` value is `:destroy` then in that case the `:delete_all`
+      # deletion strategy for the association is applied.
+      #
+      # You can force a particular deletion strategy by passing a parameter.
+      #
+      # Example:
+      #
+      # @author.books.delete_all(:nullify)
+      # @author.books.delete_all(:delete_all)
       #
       # See delete for more info.
-      def delete_all
-        delete(:all).tap do
+      def delete_all(dependent = nil)
+        if dependent.present? && ![:nullify, :delete_all].include?(dependent)
+          raise ArgumentError, "Valid values are :nullify or :delete_all"
+        end
+
+        dependent = if dependent.present?
+                      dependent
+                    elsif options[:dependent] == :destroy
+                      :delete_all
+                    else
+                      options[:dependent]
+                    end
+
+        delete(:all, dependent: dependent).tap do
           reset
           loaded!
         end
@@ -173,36 +191,29 @@ module ActiveRecord
         end
       end
 
-      # Count all records using SQL. If the +:counter_sql+ or +:finder_sql+ option is set for the
-      # association, it will be used for the query. Otherwise, construct options and pass them with
+      # Count all records using SQL.  Construct options and pass them with
       # scope to the target class's +count+.
       def count(column_name = nil, count_options = {})
+        # TODO: Remove count_options argument as soon we remove support to
+        # activerecord-deprecated_finders.
         column_name, count_options = nil, column_name if column_name.is_a?(Hash)
 
-        if options[:counter_sql] || options[:finder_sql]
-          unless count_options.blank?
-            raise ArgumentError, "If finder_sql/counter_sql is used then options cannot be passed"
-          end
-
-          reflection.klass.count_by_sql(custom_counter_sql)
-        else
-          relation = scope
-          if association_scope.distinct_value
-            # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL.
-            column_name ||= reflection.klass.primary_key
-            relation = relation.distinct
-          end
+        relation = scope
+        if association_scope.distinct_value
+          # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL.
+          column_name ||= reflection.klass.primary_key
+          relation = relation.distinct
+        end
 
-          value = relation.count(column_name)
+        value = relation.count(column_name)
 
-          limit  = options[:limit]
-          offset = options[:offset]
+        limit  = options[:limit]
+        offset = options[:offset]
 
-          if limit || offset
-            [ [value - offset.to_i, 0].max, limit.to_i ].min
-          else
-            value
-          end
+        if limit || offset
+          [ [value - offset.to_i, 0].max, limit.to_i ].min
+        else
+          value
         end
       end
 
@@ -214,18 +225,10 @@ module ActiveRecord
       # are actually removed from the database, that depends precisely on
       # +delete_records+. They are in any case removed from the collection.
       def delete(*records)
-        dependent = options[:dependent]
+        _options = records.extract_options!
+        dependent = _options[:dependent] || options[:dependent]
 
         if records.first == :all
-
-          if dependent && dependent == :destroy
-            message = 'In Rails 4.1 delete_all on associations would not fire callbacks. ' \
-                      'It means if the :dependent option is :destroy then the associated ' \
-                      'records would be deleted without loading and invoking callbacks.'
-
-            ActiveRecord::Base.logger ? ActiveRecord::Base.logger.warn(message) : $stderr.puts(message)
-          end
-
           if loaded? || dependent == :destroy
             delete_or_destroy(load_target, dependent)
           else
@@ -285,14 +288,14 @@ module ActiveRecord
 
       # Returns true if the collection is empty.
       #
-      # If the collection has been loaded or the <tt>:counter_sql</tt> option
-      # is provided, it is equivalent to <tt>collection.size.zero?</tt>. If the
+      # If the collection has been loaded
+      # it is equivalent to <tt>collection.size.zero?</tt>. If the
       # collection has not been loaded, it is equivalent to
       # <tt>collection.exists?</tt>. If the collection has not already been
       # loaded and you are going to fetch the records anyway it is better to
       # check <tt>collection.length.zero?</tt>.
       def empty?
-        if loaded? || options[:counter_sql]
+        if loaded?
           size.zero?
         else
           @target.blank? && !scope.exists?
@@ -345,7 +348,6 @@ module ActiveRecord
           if record.new_record?
             include_in_memory?(record)
           else
-            load_target if options[:finder_sql]
             loaded? ? target.include?(record) : scope.exists?(record)
           end
         else
@@ -362,8 +364,8 @@ module ActiveRecord
         target
       end
 
-      def add_to_target(record)
-        callback(:before_add, record)
+      def add_to_target(record, skip_callbacks = false)
+        callback(:before_add, record) unless skip_callbacks
         yield(record) if block_given?
 
         if association_scope.distinct_value && index = @target.index(record)
@@ -372,7 +374,7 @@ module ActiveRecord
           @target << record
         end
 
-        callback(:after_add, record)
+        callback(:after_add, record) unless skip_callbacks
         set_inverse_instance(record)
 
         record
@@ -390,31 +392,8 @@ module ActiveRecord
 
       private
 
-        def custom_counter_sql
-          if options[:counter_sql]
-            interpolate(options[:counter_sql])
-          else
-            # replace the SELECT clause with COUNT(SELECTS), preserving any hints within /* ... */
-            interpolate(options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) do
-              count_with = $2.to_s
-              count_with = '*' if count_with.blank? || count_with =~ /,/ || count_with =~ /\.\*/
-              "SELECT #{$1}COUNT(#{count_with}) FROM"
-            end
-          end
-        end
-
-        def custom_finder_sql
-          interpolate(options[:finder_sql])
-        end
-
         def find_target
-          records =
-            if options[:finder_sql]
-              reflection.klass.find_by_sql(custom_finder_sql)
-            else
-              scope.to_a
-            end
-
+          records = scope.to_a
           records.each { |record| set_inverse_instance(record) }
           records
         end
@@ -529,20 +508,13 @@ module ActiveRecord
 
         def callback(method, record)
           callbacks_for(method).each do |callback|
-            case callback
-            when Symbol
-              owner.send(callback, record)
-            when Proc
-              callback.call(owner, record)
-            else
-              callback.send(method, owner, record)
-            end
+            callback.call(method, owner, record)
           end
         end
 
         def callbacks_for(callback_name)
           full_callback_name = "#{callback_name}_for_#{reflection.name}"
-          owner.class.send(full_callback_name.to_sym) || []
+          owner.class.send(full_callback_name)
         end
 
         # Should we deal with assoc.first or assoc.last by issuing an independent query to
@@ -553,24 +525,21 @@ module ActiveRecord
         # Otherwise, go to the database only if none of the following are true:
         #   * target already loaded
         #   * owner is new record
-        #   * custom :finder_sql exists
         #   * target contains new or changed record(s)
-        #   * the first arg is an integer (which indicates the number of records to be returned)
         def fetch_first_or_last_using_find?(args)
           if args.first.is_a?(Hash)
             true
           else
             !(loaded? ||
               owner.new_record? ||
-              options[:finder_sql] ||
-              target.any? { |record| record.new_record? || record.changed? } ||
-              args.first.kind_of?(Integer))
+              target.any? { |record| record.new_record? || record.changed? })
           end
         end
 
         def include_in_memory?(record)
           if reflection.is_a?(ActiveRecord::Reflection::ThroughReflection)
-            owner.send(reflection.through_reflection.name).any? { |source|
+            assoc = owner.association(reflection.through_reflection.name)
+            assoc.reader.any? { |source|
               target = source.send(reflection.source_reflection.name)
               target.respond_to?(:include?) ? target.include?(record) : target == record
             } || target.include?(record)
@@ -579,18 +548,18 @@ module ActiveRecord
           end
         end
 
-        # If using a custom finder_sql or if the :inverse_of option has been
+        # If the :inverse_of option has been
         # specified, then #find scans the entire collection.
         def find_by_scan(*args)
           expects_array = args.first.kind_of?(Array)
-          ids           = args.flatten.compact.map{ |arg| arg.to_i }.uniq
+          ids           = args.flatten.compact.map{ |arg| arg.to_s }.uniq
 
           if ids.size == 1
             id = ids.first
-            record = load_target.detect { |r| id == r.id }
+            record = load_target.detect { |r| id == r.id.to_s }
             expects_array ? [ record ] : record
           else
-            load_target.select { |r| ids.include?(r.id) }
+            load_target.select { |r| ids.include?(r.id.to_s) }
           end
         end