Merge branch 'master' of github.com:rails/rails

actionpack/lib/action_dispatch/middleware/callbacks.rb

+require 'active_support/core_ext/module/delegation'
+
 module ActionDispatch
   # Provide callbacks to be executed before and after the request dispatch.
   class Callbacks
@@ -5,10 +7,8 @@ class Callbacks
 
     define_callbacks :call, :rescuable => true
 
-    def self.to_prepare(*args, &block)
-      ActiveSupport::Deprecation.warn "ActionDispatch::Callbacks.to_prepare is deprecated. " <<
-        "Please use ActionDispatch::Reloader.to_prepare instead."
-      ActionDispatch::Reloader.to_prepare(*args, &block)
+    class << self
+      delegate :to_prepare, :to_cleanup, :to => "ActionDispatch::Reloader"
     end
 
     def self.before(*args, &block)

actionpack/lib/action_dispatch/middleware/reloader.rb

@@ -68,6 +68,9 @@ def call(env)
       response = @app.call(env)
       response[2].extend(CleanupOnClose)
       response
+    rescue Exception
+      _run_cleanup_callbacks
+      raise
     end
   end
 end

actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb

@@ -59,7 +59,10 @@ module ClassMethods
           #     <script type="text/javascript" src="/javascripts/body.js"></script>
           #     <script type="text/javascript" src="/javascripts/tail.js"></script>
           def register_javascript_expansion(expansions)
-            JavascriptIncludeTag.expansions.merge!(expansions)
+            js_expansions = JavascriptIncludeTag.expansions
+            expansions.each do |key, values|
+              js_expansions[key] = (js_expansions[key] || []) | Array(values) if values
+            end
           end
         end
 
@@ -170,4 +173,4 @@ def javascript_include_tag(*sources)
 
     end
   end
-end
\ No newline at end of file
+end

actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb

@@ -44,7 +44,10 @@ module ClassMethods
           #     <link href="/stylesheets/body.css"  media="screen" rel="stylesheet" type="text/css" />
           #     <link href="/stylesheets/tail.css"  media="screen" rel="stylesheet" type="text/css" />
           def register_stylesheet_expansion(expansions)
-            StylesheetIncludeTag.expansions.merge!(expansions)
+            style_expansions = StylesheetIncludeTag.expansions
+            expansions.each do |key, values|
+              style_expansions[key] = (style_expansions[key] || []) | Array(values) if values
+            end
           end
         end
 
@@ -141,4 +144,4 @@ def stylesheet_link_tag(*sources)
 
     end
   end
-end
\ No newline at end of file
+end

actionpack/test/dispatch/callbacks_test.rb

@@ -29,14 +29,16 @@ def test_before_and_after_callbacks
     assert_equal 4, Foo.b
   end
 
-  def test_to_prepare_deprecation
-    prepared = false
-    assert_deprecated do
-      ActionDispatch::Callbacks.to_prepare { prepared = true }
-    end
+  def test_to_prepare_and_cleanup_delegation
+    prepared = cleaned = false
+    ActionDispatch::Callbacks.to_prepare { prepared = true }
+    ActionDispatch::Callbacks.to_prepare { cleaned = true }
 
     ActionDispatch::Reloader.prepare!
     assert prepared
+
+    ActionDispatch::Reloader.cleanup!
+    assert cleaned
   end
 
   private

actionpack/test/dispatch/reloader_test.rb

@@ -116,6 +116,20 @@ def test_manual_reloading
     assert cleaned
   end
 
+  def test_cleanup_callbacks_are_called_on_exceptions
+    cleaned = false
+    Reloader.to_cleanup { cleaned  = true }
+
+    begin
+      call_and_return_body do
+        raise "error"
+      end
+    rescue
+    end
+
+    assert cleaned
+  end
+
   private
     def call_and_return_body(&block)
       @reloader ||= Reloader.new(block || proc {[200, {}, 'response']})

actionpack/test/template/asset_tag_helper_test.rb

@@ -54,6 +54,9 @@ def url_for(*args)
   def teardown
     config.perform_caching = false
     ENV.delete('RAILS_ASSET_ID')
+
+    JavascriptIncludeTag.expansions.clear
+    StylesheetIncludeTag.expansions.clear
   end
 
   AutoDiscoveryToTag = {
@@ -268,6 +271,14 @@ def test_custom_javascript_expansions_and_defaults_puts_application_js_at_the_en
     assert_dom_equal  %(<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), javascript_include_tag('controls',:defaults, :robbery, 'effects')
   end
 
+  def test_registering_javascript_expansions_merges_with_existing_expansions
+    ENV["RAILS_ASSET_ID"] = ""
+    ActionView::Helpers::AssetTagHelper::register_javascript_expansion :can_merge => ['bank']
+    ActionView::Helpers::AssetTagHelper::register_javascript_expansion :can_merge => ['robber']
+    ActionView::Helpers::AssetTagHelper::register_javascript_expansion :can_merge => ['bank']
+    assert_dom_equal  %(<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>), javascript_include_tag(:can_merge)
+  end
+
   def test_custom_javascript_expansions_with_undefined_symbol
     ActionView::Helpers::AssetTagHelper::register_javascript_expansion :monkey => nil
     assert_raise(ArgumentError) { javascript_include_tag('first', :monkey, 'last') }
@@ -327,6 +338,14 @@ def test_custom_stylesheet_expansions_with_undefined_symbol
     assert_raise(ArgumentError) { stylesheet_link_tag('first', :monkey, 'last') }
   end
 
+  def test_registering_stylesheet_expansions_merges_with_existing_expansions
+    ENV["RAILS_ASSET_ID"] = ""
+    ActionView::Helpers::AssetTagHelper::register_stylesheet_expansion :can_merge => ['bank']
+    ActionView::Helpers::AssetTagHelper::register_stylesheet_expansion :can_merge => ['robber']
+    ActionView::Helpers::AssetTagHelper::register_stylesheet_expansion :can_merge => ['bank']
+    assert_dom_equal  %(<link href="/stylesheets/bank.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/robber.css" media="screen" rel="stylesheet" type="text/css" />), stylesheet_link_tag(:can_merge)
+  end
+
   def test_image_path
     ImagePathToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) }
   end

activerecord/CHANGELOG

 *Rails 3.1.0 (unreleased)*
 
+* Removed support for interpolated SQL conditions.  Please use scoping
+along with attribute conditionals as a replacement.
+
 * Added ActiveRecord::Base#has_secure_password (via ActiveModel::SecurePassword) to encapsulate dead-simple password usage with BCrypt encryption and salting [DHH]. Example:
 
     # Schema: User(name:string, password_digest:string, password_salt:string)

activerecord/lib/active_record/association_preload.rb

@@ -185,6 +185,9 @@ def construct_id_map(records, primary_key=nil)
       end
 
       def preload_has_and_belongs_to_many_association(records, reflection, preload_options={})
+
+        left = reflection.klass.arel_table
+
         table_name = reflection.klass.quoted_table_name
         id_to_record_map, ids = construct_id_map(records)
         records.each {|record| record.send(reflection.name).loaded}
@@ -193,14 +196,28 @@ def preload_has_and_belongs_to_many_association(records, reflection, preload_opt
         conditions = "t0.#{reflection.primary_key_name} #{in_or_equals_for_ids(ids)}"
         conditions << append_conditions(reflection, preload_options)
 
+        right = Arel::Table.new(options[:join_table]).alias('t0')
+        condition = left[reflection.klass.primary_key].eq(
+          right[reflection.association_foreign_key])
+
+        join = left.create_join(right, left.create_on(condition))
+        select = [
+          # FIXME: options[:select] is always nil in the tests.  Do we really
+          # need it?
+          options[:select] || left[Arel.star],
+          right[reflection.primary_key_name].as(
+            Arel.sql('the_parent_record_id'))
+        ]
+
         associated_records_proxy = reflection.klass.unscoped.
             includes(options[:include]).
-            joins("INNER JOIN #{connection.quote_table_name options[:join_table]} t0 ON #{reflection.klass.quoted_table_name}.#{reflection.klass.primary_key} = t0.#{reflection.association_foreign_key}").
-            select("#{options[:select] || table_name+'.*'}, t0.#{reflection.primary_key_name} as the_parent_record_id").
             order(options[:order])
 
+        associated_records_proxy.joins_values = [join]
+        associated_records_proxy.select_values = select
+
         all_associated_records = associated_records(ids) do |some_ids|
-          associated_records_proxy.where([conditions, ids]).to_a
+          associated_records_proxy.where([conditions, some_ids]).to_a
         end
 
         set_association_collection_records(id_to_record_map, reflection.name, all_associated_records, 'the_parent_record_id')
@@ -373,14 +390,9 @@ def find_associated_records(ids, reflection, preload_options)
         end
       end
 
-
-      def interpolate_sql_for_preload(sql)
-        instance_eval("%@#{sql.gsub('@', '\@')}@", __FILE__, __LINE__)
-      end
-
       def append_conditions(reflection, preload_options)
         sql = ""
-        sql << " AND (#{interpolate_sql_for_preload(reflection.sanitized_conditions)})" if reflection.sanitized_conditions
+        sql << " AND (#{reflection.sanitized_conditions})" if reflection.sanitized_conditions
         sql << " AND (#{sanitize_sql preload_options[:conditions]})" if preload_options[:conditions]
         sql
       end

activerecord/lib/active_record/associations.rb

@@ -20,18 +20,30 @@ def initialize(owner_class_name, reflection)
     end
   end
 
-  class HasManyThroughAssociationPolymorphicError < ActiveRecordError #:nodoc:
+  class HasManyThroughAssociationPolymorphicSourceError < ActiveRecordError #:nodoc:
     def initialize(owner_class_name, reflection, source_reflection)
       super("Cannot have a has_many :through association '#{owner_class_name}##{reflection.name}' on the polymorphic object '#{source_reflection.class_name}##{source_reflection.name}'.")
     end
   end
 
+  class HasManyThroughAssociationPolymorphicThroughError < ActiveRecordError #:nodoc:
+    def initialize(owner_class_name, reflection)
+      super("Cannot have a has_many :through association '#{owner_class_name}##{reflection.name}' which goes through the polymorphic association '#{owner_class_name}##{reflection.through_reflection.name}'.")
+    end
+  end
+
   class HasManyThroughAssociationPointlessSourceTypeError < ActiveRecordError #:nodoc:
     def initialize(owner_class_name, reflection, source_reflection)
       super("Cannot have a has_many :through association '#{owner_class_name}##{reflection.name}' with a :source_type option if the '#{reflection.through_reflection.class_name}##{source_reflection.name}' is not polymorphic.  Try removing :source_type on your association.")
     end
   end
 
+  class HasOneThroughCantAssociateThroughCollection < ActiveRecordError #:nodoc:
+    def initialize(owner_class_name, reflection, through_reflection)
+      super("Cannot have a has_one :through association '#{owner_class_name}##{reflection.name}' where the :through association '#{owner_class_name}##{through_reflection.name}' is a collection. Specify a has_one or belongs_to association in the :through option instead.")
+    end
+  end
+
   class HasManyThroughSourceAssociationNotFoundError < ActiveRecordError #:nodoc:
     def initialize(reflection)
       through_reflection      = reflection.through_reflection
@@ -1223,14 +1235,12 @@ def belongs_to(association_id, options = {})
 
         if reflection.options[:polymorphic]
           association_accessor_methods(reflection, BelongsToPolymorphicAssociation)
-          association_foreign_type_setter_method(reflection)
         else
           association_accessor_methods(reflection, BelongsToAssociation)
           association_constructor_method(:build,  reflection, BelongsToAssociation)
           association_constructor_method(:create, reflection, BelongsToAssociation)
         end
 
-        association_foreign_key_setter_method(reflection)
         add_counter_cache_callbacks(reflection)          if options[:counter_cache]
         add_touch_callbacks(reflection, options[:touch]) if options[:touch]
 
@@ -1450,7 +1460,7 @@ def association_accessor_methods(reflection, association_proxy_class)
             force_reload = params.first unless params.empty?
             association = association_instance_get(reflection.name)
 
-            if association.nil? || force_reload
+            if association.nil? || force_reload || association.stale_target?
               association = association_proxy_class.new(self, reflection)
               retval = force_reload ? reflection.klass.uncached { association.reload } : association.reload
               if retval.nil? and association_proxy_class == BelongsToAssociation
@@ -1497,7 +1507,11 @@ def collection_reader_method(reflection, association_proxy_class)
               association_instance_set(reflection.name, association)
             end
 
-            reflection.klass.uncached { association.reload } if force_reload
+            if force_reload
+              reflection.klass.uncached { association.reload }
+            elsif association.stale_target?
+              association.reload
+            end
 
             association
           end
@@ -1506,16 +1520,9 @@ def collection_reader_method(reflection, association_proxy_class)
             if send(reflection.name).loaded? || reflection.options[:finder_sql]
               send(reflection.name).map { |r| r.id }
             else
-              if reflection.through_reflection && reflection.source_reflection.belongs_to?
-                through = reflection.through_reflection
-                primary_key = reflection.source_reflection.primary_key_name
-                send(through.name).select("DISTINCT #{through.quoted_table_name}.#{primary_key}").map! { |r| r.send(primary_key) }
-              else
-                send(reflection.name).select("#{reflection.quoted_table_name}.#{reflection.klass.primary_key}").except(:includes).map! { |r| r.id }
-              end
+              send(reflection.name).select("#{reflection.quoted_table_name}.#{reflection.klass.primary_key}").except(:includes).map! { |r| r.id }
             end
           end
-
         end
 
         def collection_accessor_methods(reflection, association_proxy_class, writer = true)
@@ -1557,45 +1564,6 @@ def association_constructor_method(constructor, reflection, association_proxy_cl
           end
         end
 
-        def association_foreign_key_setter_method(reflection)
-          setters = reflect_on_all_associations(:belongs_to).map do |belongs_to_reflection|
-            if belongs_to_reflection.primary_key_name == reflection.primary_key_name
-              "association_instance_set(:#{belongs_to_reflection.name}, nil);"
-            end
-          end.compact.join
-
-          if method_defined?(:"#{reflection.primary_key_name}=")
-            undef_method :"#{reflection.primary_key_name}="
-          end
-
-          class_eval <<-FILE, __FILE__, __LINE__ + 1
-            def #{reflection.primary_key_name}=(new_id)
-              write_attribute :#{reflection.primary_key_name}, new_id
-              if #{reflection.primary_key_name}_changed?
-                #{ setters }
-              end
-            end
-          FILE
-        end
-
-        def association_foreign_type_setter_method(reflection)
-          setters = reflect_on_all_associations(:belongs_to).map do |belongs_to_reflection|
-            if belongs_to_reflection.options[:foreign_type] == reflection.options[:foreign_type]
-              "association_instance_set(:#{belongs_to_reflection.name}, nil);"
-            end
-          end.compact.join
-
-          field = reflection.options[:foreign_type]
-          class_eval <<-FILE, __FILE__, __LINE__ + 1
-            def #{field}=(new_id)
-              write_attribute :#{field}, new_id
-              if #{field}_changed?
-                #{ setters }
-              end
-            end
-          FILE
-        end
-
         def add_counter_cache_callbacks(reflection)
           cache_column = reflection.counter_cache_column
 

activerecord/lib/active_record/associations/association_collection.rb

@@ -462,10 +462,10 @@ def add_record_to_target_with_callbacks(record)
           callback(:before_add, record)
           yield(record) if block_given?
           @target ||= [] unless loaded?
-          if index = @target.index(record)
+          if @reflection.options[:uniq] && index = @target.index(record)
             @target[index] = record
           else
-             @target << record
+            @target << record
           end
           callback(:after_add, record)
           set_inverse_instance(record, @owner)

activerecord/lib/active_record/associations/association_proxy.rb

@@ -130,6 +130,16 @@ def loaded
         @loaded = true
       end
 
+      # The target is stale if the target no longer points to the record(s) that the
+      # relevant foreign_key(s) refers to. If stale, the association accessor method
+      # on the owner will reload the target. It's up to subclasses to implement this
+      # method if relevant.
+      #
+      # Note that if the target has not been loaded, it is not considered stale.
+      def stale_target?
+        false
+      end
+
       # Returns the target of this proxy, same as +proxy_target+.
       def target
         @target

activerecord/lib/active_record/associations/belongs_to_association.rb

@@ -42,6 +42,17 @@ def updated?
         @updated
       end
 
+      def stale_target?
+        if @target && @target.persisted?
+          target_id   = @target.send(@reflection.association_primary_key).to_s
+          foreign_key = @owner.send(@reflection.primary_key_name).to_s
+
+          target_id != foreign_key
+        else
+          false
+        end
+      end
+
       private
         def find_target
           find_method = if @reflection.options[:primary_key]

activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb

@@ -23,6 +23,19 @@ def updated?
         @updated
       end
 
+      def stale_target?
+        if @target && @target.persisted?
+          target_id    = @target.send(@reflection.association_primary_key).to_s
+          foreign_key  = @owner.send(@reflection.primary_key_name).to_s
+          target_type  = @target.class.base_class.name
+          foreign_type = @owner.send(@reflection.options[:foreign_type]).to_s
+
+          target_id != foreign_key || target_type != foreign_type
+        else
+          false
+        end
+      end
+
       private
 
         # NOTE - for now, we're only supporting inverse setting from belongs_to back onto

activerecord/lib/active_record/associations/has_many_through_association.rb

@@ -59,6 +59,7 @@ def delete_records(records)
 
         def find_target
           return [] unless target_reflection_has_associated_record?
+          update_stale_state
           scoped.all
         end
 

activerecord/lib/active_record/associations/has_one_through_association.rb

@@ -33,6 +33,7 @@ def create_through_record(new_value) #nodoc:
 
     private
       def find_target
+        update_stale_state
         scoped.first
       end
     end

activerecord/lib/active_record/associations/through_association_scope.rb

@@ -10,6 +10,17 @@ def scoped
         end
       end
 
+      def stale_target?
+        if @target && @reflection.through_reflection.macro == :belongs_to && defined?(@through_foreign_key)
+          previous_key = @through_foreign_key.to_s
+          current_key  = @owner.send(@reflection.through_reflection.primary_key_name).to_s
+
+          previous_key != current_key
+        else
+          false
+        end
+      end
+
       protected
 
       def construct_find_scope
@@ -63,8 +74,8 @@ def construct_from
       end
 
       def construct_select(custom_select = nil)
-        distinct = "DISTINCT " if @reflection.options[:uniq]
-        custom_select || @reflection.options[:select] || "#{distinct}#{@reflection.quoted_table_name}.*"
+        distinct = "DISTINCT #{@reflection.quoted_table_name}.*" if @reflection.options[:uniq]
+        custom_select || @reflection.options[:select] || distinct
       end
 
       def construct_joins
@@ -106,7 +117,12 @@ def construct_join_attributes(associate)
         # TODO: revisit this to allow it for deletion, supposing dependent option is supported
         raise ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(@owner, @reflection) if [:has_one, :has_many].include?(@reflection.source_reflection.macro)
 
-        join_attributes = construct_owner_attributes(@reflection.through_reflection).merge(@reflection.source_reflection.primary_key_name => associate.id)
+        join_attributes = construct_owner_attributes(@reflection.through_reflection)
+
+        join_attributes.merge!(
+          @reflection.source_reflection.primary_key_name =>
+            associate.send(@reflection.source_reflection.association_primary_key)
+        )
 
         if @reflection.options[:source_type]
           join_attributes.merge!(@reflection.source_reflection.options[:foreign_type] => associate.class.base_class.name)
@@ -160,6 +176,14 @@ def build_sti_condition
       end
 
       alias_method :sql_conditions, :conditions
+
+      def update_stale_state
+        construct_scope if stale_target?
+
+        if @reflection.through_reflection.macro == :belongs_to
+          @through_foreign_key = @owner.send(@reflection.through_reflection.primary_key_name)
+        end
+      end
     end
   end
 end

activerecord/lib/active_record/base.rb

@@ -913,7 +913,7 @@ def construct_finder_arel(options = {}, scope = nil)
         end
 
         def type_condition
-          sti_column = arel_table[inheritance_column]
+          sti_column = arel_table[inheritance_column.to_sym]
           condition = sti_column.eq(sti_name)
           descendants.each { |subclass| condition = condition.or(sti_column.eq(subclass.sti_name)) }
 

activerecord/lib/active_record/nested_attributes.rb

@@ -405,7 +405,18 @@ def assign_nested_attributes_for_collection_association(association_name, attrib
           end
 
         elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes['id'].to_s }
-          association.send(:add_record_to_target_with_callbacks, existing_record) if !association.loaded? && !call_reject_if(association_name, attributes)
+          unless association.loaded? || call_reject_if(association_name, attributes)
+            # Make sure we are operating on the actual object which is in the association's
+            # proxy_target array (either by finding it, or adding it if not found)
+            target_record = association.proxy_target.detect { |record| record == existing_record }
+
+            if target_record
+              existing_record = target_record
+            else
+              association.send(:add_record_to_target_with_callbacks, existing_record)
+            end
+          end
+
           assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
 
         else

activerecord/lib/active_record/reflection.rb

@@ -209,7 +209,10 @@ def association_foreign_key
       end
 
       def association_primary_key
-        @association_primary_key ||= options[:primary_key] || klass.primary_key
+        @association_primary_key ||=
+          options[:primary_key] ||
+          !options[:polymorphic] && klass.primary_key ||
+          'id'
       end
 
       def active_record_primary_key
@@ -372,6 +375,10 @@ def check_validity!
           raise HasManyThroughAssociationNotFoundError.new(active_record.name, self)
         end
 
+        if through_reflection.options[:polymorphic]
+          raise HasManyThroughAssociationPolymorphicThroughError.new(active_record.name, self)
+        end
+
         if source_reflection.nil?
           raise HasManyThroughSourceAssociationNotFoundError.new(self)
         end
@@ -381,13 +388,17 @@ def check_validity!
         end
 
         if source_reflection.options[:polymorphic] && options[:source_type].nil?
-          raise HasManyThroughAssociationPolymorphicError.new(active_record.name, self, source_reflection)
+          raise HasManyThroughAssociationPolymorphicSourceError.new(active_record.name, self, source_reflection)
         end
 
         unless [:belongs_to, :has_many, :has_one].include?(source_reflection.macro) && source_reflection.options[:through].nil?
           raise HasManyThroughSourceAssociationMacroError.new(self)
         end
 
+        if macro == :has_one && through_reflection.collection?
+          raise HasOneThroughCantAssociateThroughCollection.new(active_record.name, self, through_reflection)
+        end
+
         check_validity_of_inverse!
       end
 

activerecord/lib/active_record/relation.rb

@@ -31,7 +31,7 @@ def insert(values)
       im = arel.compile_insert values
       im.into @table
       primary_key_name = @klass.primary_key
-      primary_key_value = Hash === values ? values[primary_key_name] : nil
+      primary_key_value = primary_key_name && Hash === values ? values[primary_key] : nil
 
       @klass.connection.insert(
         im.to_sql,

activerecord/lib/active_record/relation/predicate_builder.rb

@@ -15,7 +15,7 @@ def self.build_from_hash(engine, attributes, default_table)
             table = Arel::Table.new(table_name, :engine => engine)
           end
 
-          attribute = table[column] || Arel::Attribute.new(table, column)
+          attribute = table[column.to_sym]
 
           case value
           when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::Relation
@@ -26,7 +26,7 @@ def self.build_from_hash(engine, attributes, default_table)
           when Range, Arel::Relation
             attribute.in(value)
           when ActiveRecord::Base
-            attribute.eq(Arel.sql(value.quoted_id))
+            attribute.eq(value.id)
           when Class
             # FIXME: I think we need to deprecate this behavior
             attribute.eq(value.name)

activerecord/test/cases/associations/belongs_to_associations_test.rb

@@ -17,7 +17,7 @@
 class BelongsToAssociationsTest < ActiveRecord::TestCase
   fixtures :accounts, :companies, :developers, :projects, :topics,
            :developers_projects, :computers, :authors, :author_addresses,
-           :posts, :tags, :taggings, :comments
+           :posts, :tags, :taggings, :comments, :sponsors, :members
 
   def test_belongs_to
     Client.find(3).firm.name
@@ -488,39 +488,53 @@ def test_attributes_are_being_set_when_initialized_from_belongs_to_association_w
   end
 
   def test_reassigning_the_parent_id_updates_the_object
-    original_parent = Firm.create! :name => "original"
-    updated_parent = Firm.create! :name => "updated"
+    client = companies(:second_client)
 
-    client = Client.new("client_of" => original_parent.id)
-    assert_equal original_parent, client.firm
-    assert_equal original_parent, client.firm_with_condition
-    assert_equal original_parent, client.firm_with_other_name
+    client.firm
+    client.firm_with_condition
+    firm_proxy                = client.send(:association_instance_get, :firm)
+    firm_with_condition_proxy = client.send(:association_instance_get, :firm_with_condition)
 
-    client.client_of = updated_parent.id
-    assert_equal updated_parent, client.firm
-    assert_equal updated_parent, client.firm_with_condition
-    assert_equal updated_parent, client.firm_with_other_name
+    assert !firm_proxy.stale_target?
+    assert !firm_with_condition_proxy.stale_target?
+    assert_equal companies(:first_firm), client.firm
+    assert_equal companies(:first_firm), client.firm_with_condition
+
+    client.client_of = companies(:another_firm).id
+
+    assert firm_proxy.stale_target?
+    assert firm_with_condition_proxy.stale_target?
+    assert_equal companies(:another_firm), client.firm
+    assert_equal companies(:another_firm), client.firm_with_condition
   end
 
   def test_polymorphic_reassignment_of_associated_id_updates_the_object
-    member1 = Member.create!
-    member2 = Member.create!
+    sponsor = sponsors(:moustache_club_sponsor_for_groucho)
+
+    sponsor.sponsorable
+    proxy = sponsor.send(:association_instance_get, :sponsorable)
+
+    assert !proxy.stale_target?
+    assert_equal members(:groucho), sponsor.sponsorable
 
-    sponsor = Sponsor.new("sponsorable_type" => "Member", "sponsorable_id" => member1.id)
-    assert_equal member1, sponsor.sponsorable
+    sponsor.sponsorable_id = members(:some_other_guy).id
 
-    sponsor.sponsorable_id = member2.id
-    assert_equal member2, sponsor.sponsorable
+    assert proxy.stale_target?
+    assert_equal members(:some_other_guy), sponsor.sponsorable
   end
 
   def test_polymorphic_reassignment_of_associated_type_updates_the_object
-    member1 = Member.create!
+    sponsor = sponsors(:moustache_club_sponsor_for_groucho)
 
-    sponsor = Sponsor.new("sponsorable_type" => "Member", "sponsorable_id" => member1.id)
-    assert_equal member1, sponsor.sponsorable
+    sponsor.sponsorable
+    proxy = sponsor.send(:association_instance_get, :sponsorable)
 
-    sponsor.sponsorable_type = "Firm"
-    assert_not_equal member1, sponsor.sponsorable
-  end
+    assert !proxy.stale_target?
+    assert_equal members(:groucho), sponsor.sponsorable
+
+    sponsor.sponsorable_type = 'Firm'
 
+    assert proxy.stale_target?
+    assert_equal companies(:first_firm), sponsor.sponsorable
+  end
 end

activerecord/test/cases/associations/eager_test.rb

@@ -658,10 +658,6 @@ def test_limited_eager_with_numeric_in_association
     assert_equal people(:david, :susan), Person.find(:all, :include => [:readers, :primary_contact, :number1_fan], :conditions => "number1_fans_people.first_name like 'M%'", :order => 'people.id', :limit => 2, :offset => 0)
   end
 
-  def test_preload_with_interpolation
-    assert_equal [comments(:greetings)], Post.find(posts(:welcome).id, :include => :comments_with_interpolated_conditions).comments_with_interpolated_conditions
-  end
-
   def test_polymorphic_type_condition
     post = Post.find(posts(:thinking).id, :include => :taggings)
     assert post.taggings.include?(taggings(:thinking_general))

activerecord/test/cases/associations/has_many_through_associations_test.rb

@@ -19,9 +19,10 @@
 require 'models/subscription'
 require 'models/categorization'
 require 'models/category'
+require 'models/essay'
 
 class HasManyThroughAssociationsTest < ActiveRecord::TestCase
-  fixtures :posts, :readers, :people, :comments, :authors,
+  fixtures :posts, :readers, :people, :comments, :authors, :categories,
            :owners, :pets, :toys, :jobs, :references, :companies,
            :subscribers, :books, :subscriptions, :developers, :categorizations
 
@@ -45,6 +46,16 @@ def test_associate_existing
     assert posts(:thinking).reload.people(true).include?(people(:david))
   end
 
+  def test_associate_existing_record_twice_should_add_to_target_twice
+    post   = posts(:thinking)
+    person = people(:david)
+
+    assert_difference 'post.people.to_a.count', 2 do
+      post.people << person
+      post.people << person
+    end
+  end
+
   def test_associating_new
     assert_queries(1) { posts(:thinking) }
     new_person = nil # so block binding catches it
@@ -324,12 +335,8 @@ def test_inner_join_with_quoted_table_name
     assert_equal 2, people(:michael).jobs.size
   end
 
-  def test_get_ids_for_belongs_to_source
-    assert_sql(/DISTINCT/) { assert_equal [posts(:welcome).id, posts(:authorless).id].sort, people(:michael).post_ids.sort }
-  end
-
-  def test_get_ids_for_has_many_source
-    assert_equal [comments(:eager_other_comment1).id], authors(:mary).comment_ids
+  def test_get_ids
+    assert_equal [posts(:welcome).id, posts(:authorless).id].sort, people(:michael).post_ids.sort
   end
 
   def test_get_ids_for_loaded_associations
@@ -397,6 +404,34 @@ def test_has_many_association_through_a_has_many_association_to_self
     assert_equal people(:susan).agents.map(&:agents).flatten, people(:susan).agents_of_agents
   end
 
+  def test_associate_existing_with_nonstandard_primary_key_on_belongs_to
+    Categorization.create(:author => authors(:mary), :named_category_name => categories(:general).name)
+    assert_equal categories(:general), authors(:mary).named_categories.first
+  end
+
+  def test_collection_build_with_nonstandard_primary_key_on_belongs_to
+    author   = authors(:mary)
+    category = author.named_categories.build(:name => "Primary")
+    author.save
+    assert Categorization.exists?(:author_id => author.id, :named_category_name => category.name)
+    assert author.named_categories(true).include?(category)
+  end
+
+  def test_collection_create_with_nonstandard_primary_key_on_belongs_to
+    author   = authors(:mary)
+    category = author.named_categories.create(:name => "Primary")
+    assert Categorization.exists?(:author_id => author.id, :named_category_name => category.name)
+    assert author.named_categories(true).include?(category)
+  end
+
+  def test_collection_delete_with_nonstandard_primary_key_on_belongs_to
+    author   = authors(:mary)
+    category = author.named_categories.create(:name => "Primary")
+    author.named_categories.delete(category)
+    assert !Categorization.exists?(:author_id => author.id, :named_category_name => category.name)
+    assert author.named_categories(true).empty?
+  end
+
   def test_collection_singular_ids_getter_with_string_primary_keys
     book = books(:awdr)
     assert_equal 2, book.subscriber_ids.size
@@ -466,7 +501,7 @@ def test_size_of_through_association_should_increase_correctly_when_has_many_ass
   end
 
   def test_has_many_through_with_default_scope_on_join_model
-    assert_equal posts(:welcome).comments, authors(:david).comments_on_first_posts
+    assert_equal posts(:welcome).comments.order('id').all, authors(:david).comments_on_first_posts
   end
 
   def test_create_has_many_through_with_default_scope_on_join_model
@@ -486,4 +521,33 @@ def test_joining_has_many_through_belongs_to
 
     assert_equal [posts(:eager_other)], posts
   end
+
+  def test_select_chosen_fields_only
+    author = authors(:david)
+    assert_equal ['body'], author.comments.select('comments.body').first.attributes.keys
+  end
+
+  def test_get_has_many_through_belongs_to_ids_with_conditions
+    assert_equal [categories(:general).id], authors(:mary).categories_like_general_ids
+  end
+
+  def test_count_has_many_through_with_named_scope
+    assert_equal 2, authors(:mary).categories.count
+    assert_equal 1, authors(:mary).categories.general.count
+  end
+
+  def test_has_many_through_belongs_to_should_update_when_the_through_foreign_key_changes
+    post = posts(:eager_other)
+
+    post.author_categorizations
+    proxy = post.send(:association_instance_get, :author_categorizations)
+
+    assert !proxy.stale_target?
+    assert_equal authors(:mary).categorizations.sort_by(&:id), post.author_categorizations.sort_by(&:id)
+
+    post.author_id = authors(:david).id
+
+    assert proxy.stale_target?
+    assert_equal authors(:david).categorizations.sort_by(&:id), post.author_categorizations.sort_by(&:id)
+  end
 end

activerecord/test/cases/associations/has_one_through_associations_test.rb

@@ -25,10 +25,6 @@ def test_has_one_through_with_has_one
     assert_equal clubs(:boring_club), @member.club
   end
 
-  def test_has_one_through_with_has_many
-    assert_equal clubs(:moustache_club), @member.favourite_club
-  end
-
   def test_creating_association_creates_through_record
     new_member = Member.create(:name => "Chris")
     new_member.club = Club.create(:name => "LRUG")
@@ -88,6 +84,18 @@ def test_has_one_through_eager_loading_through_polymorphic
     assert_not_nil assert_no_queries {members[0].sponsor_club}
   end
 
+  def test_has_one_through_with_conditions_eager_loading
+    # conditions on the through table
+    assert_equal clubs(:moustache_club), Member.find(@member.id, :include => :favourite_club).favourite_club
+    memberships(:membership_of_favourite_club).update_attribute(:favourite, false)
+    assert_equal nil,                    Member.find(@member.id, :include => :favourite_club).favourite_club
+
+    # conditions on the source table
+    assert_equal clubs(:moustache_club), Member.find(@member.id, :include => :hairy_club).hairy_club
+    clubs(:moustache_club).update_attribute(:name, "Association of Clean-Shaven Persons")
+    assert_equal nil,                    Member.find(@member.id, :include => :hairy_club).hairy_club
+  end
+
   def test_has_one_through_polymorphic_with_source_type
     assert_equal members(:groucho), clubs(:moustache_club).sponsored_member
   end
@@ -235,6 +243,27 @@ def test_value_is_properly_quoted
   end
 
   def test_has_one_through_with_default_scope_on_join_model
-    assert_equal posts(:welcome).comments.first, authors(:david).comment_on_first_posts
+    assert_equal posts(:welcome).comments.order('id').first, authors(:david).comment_on_first_post
+  end
+
+  def test_has_one_through_many_raises_exception
+    assert_raise(ActiveRecord::HasOneThroughCantAssociateThroughCollection) do
+      members(:groucho).club_through_many
+    end
+  end
+
+  def test_has_one_through_belongs_to_should_update_when_the_through_foreign_key_changes
+    minivan = minivans(:cool_first)
+
+    minivan.dashboard
+    proxy = minivan.send(:association_instance_get, :dashboard)
+
+    assert !proxy.stale_target?
+    assert_equal dashboards(:cool_first), minivan.dashboard
+
+    minivan.speedometer_id = speedometers(:second).id
+
+    assert proxy.stale_target?
+    assert_equal dashboards(:second), minivan.dashboard
   end
 end

activerecord/test/cases/associations/join_model_test.rb

@@ -340,11 +340,16 @@ def test_has_many_through_join_model_with_conditions
   end
 
   def test_has_many_polymorphic
-    assert_raise ActiveRecord::HasManyThroughAssociationPolymorphicError do
-      assert_equal posts(:welcome, :thinking), tags(:general).taggables
+    assert_raise ActiveRecord::HasManyThroughAssociationPolymorphicSourceError do
+      tags(:general).taggables
     end
+
+    assert_raise ActiveRecord::HasManyThroughAssociationPolymorphicThroughError do
+      taggings(:welcome_general).things
+    end
+
     assert_raise ActiveRecord::EagerLoadPolymorphicError do
-      assert_equal posts(:welcome, :thinking), tags(:general).taggings.find(:all, :include => :taggable, :conditions => 'bogus_table.column = 1')
+      tags(:general).taggings.find(:all, :include => :taggable, :conditions => 'bogus_table.column = 1')
     end
   end
 

activerecord/test/cases/nested_attributes_test.rb

@@ -298,7 +298,7 @@ def test_should_accept_update_only_option
 
   def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true
     @ship.delete
-    
+
     @pirate.reload.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower' })
 
     assert_not_nil @pirate.ship
@@ -411,6 +411,7 @@ def test_should_take_a_hash_with_string_keys_and_update_the_associated_model
 
   def test_should_modify_an_existing_record_if_there_is_a_matching_composite_id
     @pirate.stubs(:id).returns('ABC1X')
+    @ship.stubs(:pirate_id).returns(@pirate.id) # prevents pirate from being reloaded due to non-matching foreign key
     @ship.pirate_attributes = { :id => @pirate.id, :catchphrase => 'Arr' }
 
     assert_equal 'Arr', @ship.pirate.catchphrase
@@ -451,7 +452,7 @@ def test_should_work_with_update_attributes_as_well
 
   def test_should_not_destroy_the_associated_model_until_the_parent_is_saved
     pirate = @ship.pirate
-    
+
     @ship.attributes = { :pirate_attributes => { :id => pirate.id, '_destroy' => true } }
     assert_nothing_raised(ActiveRecord::RecordNotFound) { Pirate.find(pirate.id) }
     @ship.save

activerecord/test/cases/readonly_test.rb

@@ -12,7 +12,7 @@ def Project.foo() find :first end
 
 
 class ReadOnlyTest < ActiveRecord::TestCase
-  fixtures :posts, :comments, :developers, :projects, :developers_projects
+  fixtures :posts, :comments, :developers, :projects, :developers_projects, :people, :readers
 
   def test_cant_save_readonly_record
     dev = Developer.find(1)
@@ -71,6 +71,18 @@ def test_has_many_with_through_is_not_implicitly_marked_readonly
     assert !people.any?(&:readonly?)
   end
 
+  def test_has_many_with_through_is_not_implicitly_marked_readonly_while_finding_by_id
+    assert !posts(:welcome).people.find(1).readonly?
+  end
+
+  def test_has_many_with_through_is_not_implicitly_marked_readonly_while_finding_first
+    assert !posts(:welcome).people.first.readonly?
+  end
+
+  def test_has_many_with_through_is_not_implicitly_marked_readonly_while_finding_last
+    assert !posts(:welcome).people.last.readonly?
+  end
+
   def test_readonly_scoping
     Post.send(:with_scope, :find => { :conditions => '1=1' }) do
       assert !Post.find(1).readonly?

activerecord/test/cases/reflection_test.rb

@@ -7,6 +7,7 @@
 require 'models/ship'
 require 'models/pirate'
 require 'models/price_estimate'
+require 'models/tagging'
 
 class ReflectionTest < ActiveRecord::TestCase
   include ActiveRecord::Reflection
@@ -194,6 +195,7 @@ def test_has_many_through_reflection
   def test_association_primary_key
     assert_equal "id", Author.reflect_on_association(:posts).association_primary_key.to_s
     assert_equal "name", Author.reflect_on_association(:essay).association_primary_key.to_s
+    assert_equal "id", Tagging.reflect_on_association(:taggable).association_primary_key.to_s
   end
 
   def test_active_record_primary_key

activerecord/test/fixtures/dashboards.yml

 cool_first:
   dashboard_id: d1
-  name: my_dashboard
\ No newline at end of file
+  name: my_dashboard
+second:
+  dashboard_id: d2
+  name: second

activerecord/test/fixtures/members.yml

 groucho:
+  id: 1
   name: Groucho Marx
   member_type_id: 1
 some_other_guy:
+  id: 2
   name: Englebert Humperdink
   member_type_id: 2

activerecord/test/fixtures/memberships.yml

 membership_of_boring_club:
   joined_on: <%= 3.weeks.ago.to_s(:db) %>
   club: boring_club
-  member: groucho
+  member_id: 1
   favourite: false
   type: CurrentMembership
 
 membership_of_favourite_club:
   joined_on: <%= 3.weeks.ago.to_s(:db) %>
   club: moustache_club
-  member: groucho
+  member_id: 1
   favourite: true
   type: Membership
 
 other_guys_membership:
   joined_on: <%= 4.weeks.ago.to_s(:db) %>
   club: boring_club
-  member: some_other_guy
+  member_id: 2
   favourite: false
   type: CurrentMembership

activerecord/test/fixtures/speedometers.yml

 cool_first:
   speedometer_id: s1
   name: my_speedometer
-  dashboard_id: d1
\ No newline at end of file
+  dashboard_id: d1
+second:
+  speedometer_id: s2
+  name: second
+  dashboard_id: d2

activerecord/test/fixtures/sponsors.yml

 moustache_club_sponsor_for_groucho:
   sponsor_club: moustache_club
-  sponsorable: groucho (Member)
+  sponsorable_id: 1
+  sponsorable_type: Member
 boring_club_sponsor_for_groucho:
   sponsor_club: boring_club
-  sponsorable: some_other_guy (Member)
+  sponsorable_id: 2
+  sponsorable_type: Member
 crazy_club_sponsor_for_groucho:
   sponsor_club: crazy_club
-  sponsorable: some_other_guy (Member)
\ No newline at end of file
+  sponsorable_id: 2
+  sponsorable_type: Member

activerecord/test/models/author.rb

@@ -28,7 +28,9 @@ def testing_proxy_target
 
   has_many :first_posts
   has_many :comments_on_first_posts, :through => :first_posts, :source => :comments, :order => 'posts.id desc, comments.id asc'
-  has_one  :comment_on_first_posts,  :through => :first_posts, :source => :comments, :order => 'posts.id desc, comments.id asc'
+
+  has_one :first_post
+  has_one :comment_on_first_post,  :through => :first_post, :source => :comments, :order => 'posts.id desc, comments.id asc'
 
   has_many :thinking_posts, :class_name => 'Post', :conditions => { :title => 'So I was thinking' }, :dependent => :delete_all
   has_many :welcome_posts,  :class_name => 'Post', :conditions => { :title => 'Welcome to the weblog' }
@@ -76,6 +78,7 @@ def testing_proxy_target
 
   has_many :categorizations
   has_many :categories, :through => :categorizations
+  has_many :named_categories, :through => :categorizations
 
   has_many :special_categorizations
   has_many :special_categories, :through => :special_categorizations, :source => :category

activerecord/test/models/categorization.rb

 class Categorization < ActiveRecord::Base
   belongs_to :post
   belongs_to :category
+  belongs_to :named_category, :class_name => 'Category', :foreign_key => :named_category_name, :primary_key => :name
   belongs_to :author
 
   belongs_to :author_using_custom_pk,  :class_name => 'Author', :foreign_key => :author_id, :primary_key => :author_address_extra_id

activerecord/test/models/category.rb

@@ -23,6 +23,8 @@ def self.what_are_you
 
   has_many :categorizations
   has_many :authors, :through => :categorizations, :select => 'authors.*, categorizations.post_id'
+
+  scope :general, :conditions => { :name => 'General' }
 end
 
 class SpecialCategory < Category

activerecord/test/models/member.rb

 class Member < ActiveRecord::Base
   has_one :current_membership
-  has_many :memberships
+  has_one :membership
   has_many :fellow_members, :through => :club, :source => :members
   has_one :club, :through => :current_membership
-  has_one :favourite_club, :through => :memberships, :conditions => ["memberships.favourite = ?", true], :source => :club
+  has_one :favourite_club, :through => :membership, :conditions => ["memberships.favourite = ?", true], :source => :club
+  has_one :hairy_club, :through => :membership, :conditions => {:clubs => {:name => "Moustache and Eyebrow Fancier Club"}}, :source => :club
   has_one :sponsor, :as => :sponsorable
   has_one :sponsor_club, :through => :sponsor
   has_one :member_detail
   has_one :organization, :through => :member_detail
   belongs_to :member_type
-end
\ No newline at end of file
+
+  has_many :current_memberships
+  has_one :club_through_many, :through => :current_memberships, :source => :club
+end

activerecord/test/models/post.rb

@@ -40,9 +40,6 @@ def find_most_recent
   has_many :author_favorites, :through => :author
   has_many :author_categorizations, :through => :author, :source => :categorizations
 
-  has_many :comments_with_interpolated_conditions, :class_name => 'Comment',
-      :conditions => ['#{"#{aliased_table_name}." rescue ""}body = ?', 'Thank you for the welcome']
-
   has_one  :very_special_comment
   has_one  :very_special_comment_with_post, :class_name => "VerySpecialComment", :include => :post
   has_many :special_comments

activerecord/test/models/tagging.rb

@@ -7,4 +7,5 @@ class Tagging < ActiveRecord::Base
   belongs_to :super_tag,   :class_name => 'Tag', :foreign_key => 'super_tag_id'
   belongs_to :invalid_tag, :class_name => 'Tag', :foreign_key => 'tag_id'
   belongs_to :taggable, :polymorphic => true, :counter_cache => true
-end
\ No newline at end of file
+  has_many :things, :through => :taggable
+end

activerecord/test/schema/schema.rb

@@ -110,6 +110,7 @@ def create_table(*args, &block)
 
   create_table :categorizations, :force => true do |t|
     t.column :category_id, :integer
+    t.string :named_category_name
     t.column :post_id, :integer
     t.column :author_id, :integer
     t.column :special, :boolean