diff options
26 files changed, 260 insertions, 255 deletions
diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index ecf7b6c210..638897a86b 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -205,7 +205,7 @@ module ActiveRecord # 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( + right[reflection.foreign_key].as( Arel.sql('the_parent_record_id')) ] @@ -220,7 +220,7 @@ module ActiveRecord all_associated_records = associated_records(ids) do |some_ids| method = in_or_equal(some_ids) - conditions = right[reflection.primary_key_name].send(*method) + conditions = right[reflection.foreign_key].send(*method) conditions = custom_conditions.inject(conditions) do |ast, cond| ast.and cond end @@ -241,7 +241,7 @@ module ActiveRecord unless through_records.empty? through_reflection = reflections[options[:through]] - through_primary_key = through_reflection.primary_key_name + through_primary_key = through_reflection.foreign_key source = reflection.source_reflection.name through_records.first.class.preload_associations(through_records, source) if through_reflection.macro == :belongs_to @@ -255,7 +255,7 @@ module ActiveRecord end end else - set_association_single_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options), reflection.primary_key_name) + set_association_single_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options), reflection.foreign_key) end end @@ -263,8 +263,8 @@ module ActiveRecord return if records.first.send(reflection.name).loaded? options = reflection.options - primary_key_name = reflection.through_reflection_primary_key_name - id_to_record_map, ids = construct_id_map(records, primary_key_name || reflection.options[:primary_key]) + foreign_key = reflection.through_reflection_foreign_key + id_to_record_map, ids = construct_id_map(records, foreign_key || reflection.options[:primary_key]) records.each {|record| record.send(reflection.name).loaded} if options[:through] @@ -281,7 +281,7 @@ module ActiveRecord else set_association_collection_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options), - reflection.primary_key_name) + reflection.foreign_key) end end @@ -319,7 +319,7 @@ module ActiveRecord def preload_belongs_to_association(records, reflection, preload_options={}) return if records.first.send("loaded_#{reflection.name}?") options = reflection.options - primary_key_name = reflection.primary_key_name + foreign_key = reflection.foreign_key klasses_and_ids = {} @@ -330,7 +330,7 @@ module ActiveRecord # to their parent_records records.each do |record| if klass = record.send(polymorph_type) - klass_id = record.send(primary_key_name) + klass_id = record.send(foreign_key) if klass_id id_map = klasses_and_ids[klass.constantize] ||= {} (id_map[klass_id.to_s] ||= []) << record @@ -339,7 +339,7 @@ module ActiveRecord end else id_map = records.group_by do |record| - key = record.send(primary_key_name) + key = record.send(foreign_key) key && key.to_s end id_map.delete nil @@ -369,7 +369,7 @@ module ActiveRecord conditions = [] - key = reflection.primary_key_name + key = reflection.foreign_key if interface = reflection.options[:as] key = "#{interface}_id" diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index b49cf8de95..9a4f6d4dfd 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1612,16 +1612,14 @@ module ActiveRecord # - set the foreign key to NULL if the option is set to :nullify # - do not delete the parent record if there is any child record if the # option is set to :restrict - # - # The +extra_conditions+ parameter, which is not used within the main - # Active Record codebase, is meant to allow plugins to define extra - # finder conditions. - def configure_dependency_for_has_many(reflection, extra_conditions = nil) - if reflection.options.include?(:dependent) + def configure_dependency_for_has_many(reflection) + if reflection.options[:dependent] + method_name = "has_many_dependent_for_#{reflection.name}" + case reflection.options[:dependent] - when :destroy - method_name = "has_many_dependent_destroy_for_#{reflection.name}".to_sym - define_method(method_name) do + when :destroy, :delete_all, :nullify + define_method(method_name) do + if reflection.options[:dependent] == :destroy send(reflection.name).each do |o| # No point in executing the counter update since we're going to destroy the parent anyway counter_method = ('belongs_to_counter_cache_before_destroy_for_' + self.class.name.downcase).to_sym @@ -1633,35 +1631,21 @@ module ActiveRecord o.destroy end end - before_destroy method_name - when :delete_all - before_destroy do |record| - self.class.send(:delete_all_has_many_dependencies, - record, - reflection.name, - reflection.klass, - reflection.dependent_conditions(record, self.class, extra_conditions)) - end - when :nullify - before_destroy do |record| - self.class.send(:nullify_has_many_dependencies, - record, - reflection.name, - reflection.klass, - reflection.primary_key_name, - reflection.dependent_conditions(record, self.class, extra_conditions)) - end - when :restrict - method_name = "has_many_dependent_restrict_for_#{reflection.name}".to_sym - define_method(method_name) do - unless send(reflection.name).empty? - raise DeleteRestrictionError.new(reflection) - end + + # AssociationProxy#delete_all looks at the :dependent option and acts accordingly + send(reflection.name).delete_all + end + when :restrict + define_method(method_name) do + unless send(reflection.name).empty? + raise DeleteRestrictionError.new(reflection) end - before_destroy method_name - else - raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, :nullify or :restrict (#{reflection.options[:dependent].inspect})" + end + else + raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, :nullify or :restrict (#{reflection.options[:dependent].inspect})" end + + before_destroy method_name end end @@ -1686,7 +1670,7 @@ module ActiveRecord class_eval <<-eoruby, __FILE__, __LINE__ + 1 def #{method_name} association = #{reflection.name} - association.update_attribute(#{reflection.primary_key_name.inspect}, nil) if association + association.update_attribute(#{reflection.foreign_key.inspect}, nil) if association end eoruby when :restrict @@ -1724,14 +1708,6 @@ module ActiveRecord end end - def delete_all_has_many_dependencies(record, reflection_name, association_class, dependent_conditions) - association_class.delete_all(dependent_conditions) - end - - def nullify_has_many_dependencies(record, reflection_name, association_class, primary_key_name, dependent_conditions) - association_class.update_all("#{primary_key_name} = NULL", dependent_conditions) - end - mattr_accessor :valid_keys_for_has_many_association @@valid_keys_for_has_many_association = [ :class_name, :table_name, :foreign_key, :primary_key, @@ -1806,7 +1782,7 @@ module ActiveRecord reflection = create_reflection(:has_and_belongs_to_many, association_id, options, self) - if reflection.association_foreign_key == reflection.primary_key_name + if reflection.association_foreign_key == reflection.foreign_key raise HasAndBelongsToManyAssociationForeignKeyNeeded.new(reflection) end diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 108e316672..0959ea2c5a 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -31,10 +31,6 @@ module ActiveRecord end end - def scoped - with_scope(@scope) { @reflection.klass.scoped } - end - def find(*args) options = args.extract_options! @@ -488,7 +484,7 @@ module ActiveRecord ensure_owner_is_persisted! transaction do - with_scope(:create => @scope[:create].merge(scoped.where_values_hash)) do + with_scope(:create => @scope[:create].merge(scoped.scope_for_create)) do build_record(attrs, &block) end end diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 6720d83199..7e68241a2c 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -8,7 +8,7 @@ module ActiveRecord # # AssociationProxy # BelongsToAssociation - # BelongsToPolymorphicAssociation + # BelongsToPolymorphicAssociation # AssociationCollection + HasAssociation # HasAndBelongsToManyAssociation # HasManyAssociation @@ -116,6 +116,7 @@ module ActiveRecord # Reloads the \target and returns +self+ on success. def reload reset + construct_scope load_target self unless @target.nil? end @@ -166,6 +167,10 @@ module ActiveRecord end end + def scoped + with_scope(@scope) { target_klass.scoped } + end + protected def interpolate_sql(sql, record = nil) @owner.send(:interpolate_sql, sql, record) @@ -192,15 +197,19 @@ module ActiveRecord # Forwards +with_scope+ to the reflection. def with_scope(*args, &block) - @reflection.klass.send :with_scope, *args, &block + target_klass.send :with_scope, *args, &block end # Construct the scope used for find/create queries on the target def construct_scope - @scope = { - :find => construct_find_scope, - :create => construct_create_scope - } + if target_klass + @scope = { + :find => construct_find_scope, + :create => construct_create_scope + } + else + @scope = nil + end end # Implemented by subclasses @@ -214,7 +223,7 @@ module ActiveRecord end def aliased_table - @reflection.klass.arel_table + target_klass.arel_table end # Set the inverse association, if possible @@ -224,6 +233,12 @@ module ActiveRecord end end + # This class of the target. belongs_to polymorphic overrides this to look at the + # polymorphic_type field on the owner. + def target_klass + @reflection.klass + end + private # Forwards any missing method call to the \target. def method_missing(method, *args) @@ -254,7 +269,7 @@ module ActiveRecord def load_target return nil unless defined?(@loaded) - if !loaded? && (!@owner.new_record? || foreign_key_present) + if !loaded? && (!@owner.new_record? || foreign_key_present) && @scope @target = find_target end @@ -282,11 +297,6 @@ module ActiveRecord end end - # Returns the ID of the owner, quoted if needed. - def owner_quoted_id - @owner.quoted_id - end - # Can be redefined by subclasses, notably polymorphic belongs_to # The record parameter is necessary to support polymorphic inverses as we must check for # the association in the specific class of the record. diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index 98c1c13524..63eb38ab34 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -11,29 +11,16 @@ module ActiveRecord end def replace(record) - counter_cache_name = @reflection.counter_cache_column - - if record.nil? - if counter_cache_name && @owner.persisted? - @reflection.klass.decrement_counter(counter_cache_name, previous_record_id) if @owner[@reflection.primary_key_name] - end - - @target = @owner[@reflection.primary_key_name] = nil - else - raise_on_type_mismatch(record) - - if counter_cache_name && @owner.persisted? && record.id != @owner[@reflection.primary_key_name] - @reflection.klass.increment_counter(counter_cache_name, record.id) - @reflection.klass.decrement_counter(counter_cache_name, @owner[@reflection.primary_key_name]) if @owner[@reflection.primary_key_name] - end - - @target = (AssociationProxy === record ? record.target : record) - @owner[@reflection.primary_key_name] = record_id(record) if record.persisted? - @updated = true - end + record = record.target if AssociationProxy === record + raise_on_type_mismatch(record) unless record.nil? + update_counters(record) + replace_keys(record) set_inverse_instance(record) + @target = record + @updated = true if record + loaded record end @@ -44,8 +31,8 @@ module ActiveRecord 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 = @target[@reflection.association_primary_key].to_s + foreign_key = @owner[@reflection.foreign_key].to_s target_id != foreign_key else @@ -54,31 +41,55 @@ module ActiveRecord end private + def update_counters(record) + counter_cache_name = @reflection.counter_cache_column + + if counter_cache_name && @owner.persisted? && different_target?(record) + if record + record.class.increment_counter(counter_cache_name, record.id) + end + + if foreign_key_present + target_klass.decrement_counter(counter_cache_name, target_id) + end + end + end + + # Checks whether record is different to the current target, without loading it + def different_target?(record) + record.nil? && @owner[@reflection.foreign_key] || + record.id != @owner[@reflection.foreign_key] + end + + def replace_keys(record) + @owner[@reflection.foreign_key] = record && record[@reflection.association_primary_key] + end + def find_target - find_method = if @reflection.options[:primary_key] - "find_by_#{@reflection.options[:primary_key]}" - else - "find" - end - - options = @reflection.options.dup.slice(:select, :include, :readonly) - - the_target = with_scope(:find => @scope[:find]) do - @reflection.klass.send(find_method, - @owner[@reflection.primary_key_name], - options - ) if @owner[@reflection.primary_key_name] + if foreign_key_present + scoped.first.tap { |record| set_inverse_instance(record) } end - set_inverse_instance(the_target) - the_target end def construct_find_scope - { :conditions => conditions } + { + :conditions => construct_conditions, + :select => @reflection.options[:select], + :include => @reflection.options[:include], + :readonly => @reflection.options[:readonly] + } + end + + def construct_conditions + conditions = aliased_table[@reflection.association_primary_key]. + eq(@owner[@reflection.foreign_key]) + + conditions = conditions.and(Arel.sql(sql_conditions)) if sql_conditions + conditions end def foreign_key_present - !@owner[@reflection.primary_key_name].nil? + !@owner[@reflection.foreign_key].nil? end # NOTE - for now, we're only supporting inverse setting from belongs_to back onto @@ -88,17 +99,12 @@ module ActiveRecord inverse && inverse.macro == :has_one end - def record_id(record) - record.send(@reflection.options[:primary_key] || :id) - end - - def previous_record_id - @previous_record_id ||= if @reflection.options[:primary_key] - previous_record = @owner.send(@reflection.name) - previous_record.nil? ? nil : previous_record.id - else - @owner[@reflection.primary_key_name] - end + def target_id + if @reflection.options[:primary_key] + @owner.send(@reflection.name).try(:id) + else + @owner[@reflection.foreign_key] + end end end end diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index 90eff7399c..46adc048b8 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -1,32 +1,11 @@ module ActiveRecord # = Active Record Belongs To Polymorphic Association module Associations - class BelongsToPolymorphicAssociation < AssociationProxy #:nodoc: - def replace(record) - if record.nil? - @target = @owner[@reflection.primary_key_name] = @owner[@reflection.options[:foreign_type]] = nil - else - @target = (AssociationProxy === record ? record.target : record) - - @owner[@reflection.primary_key_name] = record_id(record) - @owner[@reflection.options[:foreign_type]] = record.class.base_class.name.to_s - - @updated = true - end - - set_inverse_instance(record) - loaded - record - end - - def updated? - @updated - end - + class BelongsToPolymorphicAssociation < BelongsToAssociation #:nodoc: 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 + foreign_key = @owner.send(@reflection.foreign_key).to_s target_type = @target.class.base_class.name foreign_type = @owner.send(@reflection.options[:foreign_type]).to_s @@ -38,43 +17,26 @@ module ActiveRecord private - def inverse_reflection_for(record) - @reflection.polymorphic_inverse_of(record.class) + def replace_keys(record) + super + @owner[@reflection.options[:foreign_type]] = record && record.class.base_class.name end - def invertible_for?(record) - inverse = inverse_reflection_for(record) - inverse && inverse.macro == :has_one + def different_target?(record) + super || record.class != target_klass end - def construct_find_scope - { :conditions => conditions } - end - - def find_target - return nil if association_class.nil? - - target = association_class.send(:with_scope, :find => @scope[:find]) do - association_class.find( - @owner[@reflection.primary_key_name], - :select => @reflection.options[:select], - :include => @reflection.options[:include] - ) - end - set_inverse_instance(target) - target - end - - def foreign_key_present - !@owner[@reflection.primary_key_name].nil? + def inverse_reflection_for(record) + @reflection.polymorphic_inverse_of(record.class) end - def record_id(record) - record.send(@reflection.options[:primary_key] || :id) + def target_klass + type = @owner[@reflection.options[:foreign_type]] + type && type.constantize end - def association_class - @owner[@reflection.options[:foreign_type]] ? @owner[@reflection.options[:foreign_type]].constantize : nil + def raise_on_type_mismatch(record) + # A polymorphic association cannot have a type mismatch, by definition end end end diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb index 02707dfae1..1e5149d80f 100644 --- a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb @@ -193,11 +193,11 @@ module ActiveRecord first_key = second_key = nil if through_reflection.macro == :belongs_to - jt_primary_key = through_reflection.primary_key_name + jt_primary_key = through_reflection.foreign_key jt_foreign_key = through_reflection.association_primary_key else jt_primary_key = through_reflection.active_record_primary_key - jt_foreign_key = through_reflection.primary_key_name + jt_foreign_key = through_reflection.foreign_key if through_reflection.options[:as] # has_many :through against a polymorphic join jt_conditions << @@ -231,7 +231,7 @@ module ActiveRecord join_table[reflection.source_reflection.options[:foreign_type]]. eq(reflection.options[:source_type]) else - second_key = source_reflection.primary_key_name + second_key = source_reflection.foreign_key end end @@ -262,7 +262,7 @@ module ActiveRecord end def join_belongs_to_to(relation) - foreign_key = options[:foreign_key] || reflection.primary_key_name + foreign_key = options[:foreign_key] || reflection.foreign_key primary_key = options[:primary_key] || reflection.klass.primary_key join_target_table( diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index b1d454545f..5336b6cc28 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -40,7 +40,7 @@ module ActiveRecord attributes = columns.map do |column| name = column.name value = case name.to_s - when @reflection.primary_key_name.to_s + when @reflection.foreign_key.to_s @owner.id when @reflection.association_foreign_key.to_s record.id @@ -64,7 +64,7 @@ module ActiveRecord records.each { |record| @owner.connection.delete(interpolate_sql(sql, record)) } else relation = Arel::Table.new(@reflection.options[:join_table]) - stmt = relation.where(relation[@reflection.primary_key_name].eq(@owner.id). + stmt = relation.where(relation[@reflection.foreign_key].eq(@owner.id). and(relation[@reflection.association_foreign_key].in(records.map { |x| x.id }.compact)) ).compile_delete @owner.connection.delete stmt.to_sql diff --git a/activerecord/lib/active_record/associations/has_association.rb b/activerecord/lib/active_record/associations/has_association.rb index 0ecdb696ea..190fed77c2 100644 --- a/activerecord/lib/active_record/associations/has_association.rb +++ b/activerecord/lib/active_record/associations/has_association.rb @@ -14,9 +14,9 @@ module ActiveRecord def construct_owner_attributes(reflection = @reflection) attributes = {} if reflection.macro == :belongs_to - attributes[reflection.association_primary_key] = @owner.send(reflection.primary_key_name) + attributes[reflection.association_primary_key] = @owner.send(reflection.foreign_key) else - attributes[reflection.primary_key_name] = @owner.send(reflection.active_record_primary_key) + attributes[reflection.foreign_key] = @owner.send(reflection.active_record_primary_key) if reflection.options[:as] attributes["#{reflection.options[:as]}_type"] = @owner.class.base_class.name diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index c3360463cd..0d044b28e4 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -7,14 +7,6 @@ module ActiveRecord # is provided by its child HasManyThroughAssociation. class HasManyAssociation < AssociationCollection #:nodoc: protected - def owner_quoted_id - if @reflection.options[:primary_key] - @owner.class.quote_value(@owner.send(@reflection.options[:primary_key])) - else - @owner.quoted_id - end - end - # Returns the number of records in this collection. # # If the association has a counter cache it gets that value. Otherwise @@ -66,7 +58,7 @@ module ActiveRecord when :delete_all @reflection.klass.delete(records.map { |r| r.id }) else - updates = { @reflection.primary_key_name => nil } + updates = { @reflection.foreign_key => nil } conditions = { @reflection.association_primary_key => records.map { |r| r.id } } with_scope(@scope) do 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 e2b008034e..2348ee099c 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -31,7 +31,7 @@ module ActiveRecord protected def target_reflection_has_associated_record? - if @reflection.through_reflection.macro == :belongs_to && @owner[@reflection.through_reflection.primary_key_name].blank? + if @reflection.through_reflection.macro == :belongs_to && @owner[@reflection.through_reflection.foreign_key].blank? false else true diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index c32aaf986e..12ccb3af8a 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -38,11 +38,11 @@ module ActiveRecord @target.destroy if @target.persisted? @owner.clear_association_cache when :nullify - @target[@reflection.primary_key_name] = nil + @target[@reflection.foreign_key] = nil @target.save if @owner.persisted? && @target.persisted? end else - @target[@reflection.primary_key_name] = nil + @target[@reflection.foreign_key] = nil @target.save if @owner.persisted? && @target.persisted? end end @@ -65,15 +65,6 @@ module ActiveRecord end end - protected - def owner_quoted_id - if @reflection.options[:primary_key] - @owner.class.quote_value(@owner.send(@reflection.options[:primary_key])) - else - @owner.quoted_id - end - end - private def find_target options = @reflection.options.dup.slice(:select, :order, :include, :readonly) @@ -105,7 +96,7 @@ module ActiveRecord if replace_existing replace(record, true) else - record[@reflection.primary_key_name] = @owner.id if @owner.persisted? + record[@reflection.foreign_key] = @owner.id if @owner.persisted? self.target = record set_inverse_instance(record) end diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index 57718285f8..c78f5d969f 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -13,7 +13,7 @@ module ActiveRecord 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 + current_key = @owner.send(@reflection.through_reflection.foreign_key).to_s previous_key != current_key else @@ -69,14 +69,14 @@ module ActiveRecord if @reflection.source_reflection.macro == :belongs_to reflection_primary_key = @reflection.source_reflection.options[:primary_key] || @reflection.klass.primary_key - source_primary_key = @reflection.source_reflection.primary_key_name + source_primary_key = @reflection.source_reflection.foreign_key if @reflection.options[:source_type] column = @reflection.source_reflection.options[:foreign_type] conditions << right[column].eq(@reflection.options[:source_type]) end else - reflection_primary_key = @reflection.source_reflection.primary_key_name + reflection_primary_key = @reflection.source_reflection.foreign_key source_primary_key = @reflection.source_reflection.options[:primary_key] || @reflection.through_reflection.klass.primary_key if @reflection.source_reflection.options[:as] @@ -100,7 +100,7 @@ module ActiveRecord raise ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(@owner, @reflection) if [:has_one, :has_many].include?(@reflection.source_reflection.macro) join_attributes = { - @reflection.source_reflection.primary_key_name => + @reflection.source_reflection.foreign_key => associate.send(@reflection.source_reflection.association_primary_key) } @@ -158,10 +158,8 @@ module ActiveRecord 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) + @through_foreign_key = @owner.send(@reflection.through_reflection.foreign_key) end end end diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 4a18719551..c6c1dd8b87 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -343,8 +343,8 @@ module ActiveRecord association.destroy else key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id - if autosave != false && (new_record? || association.new_record? || association[reflection.primary_key_name] != key || autosave) - association[reflection.primary_key_name] = key + if autosave != false && (new_record? || association.new_record? || association[reflection.foreign_key] != key || autosave) + association[reflection.foreign_key] = key saved = association.save(:validate => !autosave) raise ActiveRecord::Rollback if !saved && autosave saved @@ -367,7 +367,7 @@ module ActiveRecord if association.updated? association_id = association.send(reflection.options[:primary_key] || :id) - self[reflection.primary_key_name] = association_id + self[reflection.foreign_key] = association_id end saved if autosave diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 396ab226a9..0a2e436ecc 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -874,7 +874,12 @@ module ActiveRecord #:nodoc: def relation #:nodoc: @relation ||= Relation.new(self, arel_table) - finder_needs_type_condition? ? @relation.where(type_condition) : @relation + + if finder_needs_type_condition? + @relation.where(type_condition).create_with(inheritance_column.to_sym => sti_name) + else + @relation + end end # Finder methods must instantiate through this method to work with the @@ -914,10 +919,9 @@ module ActiveRecord #:nodoc: def type_condition 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)) } + sti_names = ([self] + descendants).map { |model| model.sti_name } - condition + sti_column.in(sti_names) end # Guesses the table name, but does not decorate it with prefix and suffix information. diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 6fb723f2f5..3ddd6687b9 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -634,7 +634,7 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) targets.each do |target| join_fixtures["#{label}_#{target}"] = Fixture.new( - { association.primary_key_name => row[primary_key_name], + { association.foreign_key => row[primary_key_name], association.association_foreign_key => Fixtures.identify(target) }, nil, @connection) end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 0310e7050d..81a95d4971 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -196,8 +196,8 @@ module ActiveRecord @quoted_table_name ||= klass.quoted_table_name end - def primary_key_name - @primary_key_name ||= options[:foreign_key] || derive_primary_key_name + def foreign_key + @foreign_key ||= options[:foreign_key] || derive_foreign_key end def primary_key_column @@ -251,7 +251,7 @@ module ActiveRecord false end - def through_reflection_primary_key_name + def through_reflection_foreign_key end def source_reflection @@ -298,17 +298,6 @@ module ActiveRecord !options[:validate].nil? ? options[:validate] : (options[:autosave] == true || macro == :has_many) end - def dependent_conditions(record, base_class, extra_conditions) - dependent_conditions = [] - dependent_conditions << "#{primary_key_name} = #{record.send(name).send(:owner_quoted_id)}" - dependent_conditions << "#{options[:as]}_type = '#{base_class.name}'" if options[:as] - dependent_conditions << klass.send(:sanitize_sql, options[:conditions]) if options[:conditions] - dependent_conditions << extra_conditions if extra_conditions - dependent_conditions = dependent_conditions.collect {|where| "(#{where})" }.join(" AND ") - dependent_conditions = dependent_conditions.gsub('@', '\@') - dependent_conditions - end - # Returns +true+ if +self+ is a +belongs_to+ reflection. def belongs_to? macro == :belongs_to @@ -321,7 +310,7 @@ module ActiveRecord class_name end - def derive_primary_key_name + def derive_foreign_key if belongs_to? "#{name}_id" elsif options[:as] @@ -403,11 +392,11 @@ module ActiveRecord end def through_reflection_primary_key - through_reflection.belongs_to? ? through_reflection.klass.primary_key : through_reflection.primary_key_name + through_reflection.belongs_to? ? through_reflection.klass.primary_key : through_reflection.foreign_key end - def through_reflection_primary_key_name - through_reflection.primary_key_name if through_reflection.belongs_to? + def through_reflection_foreign_key + through_reflection.foreign_key if through_reflection.belongs_to? end private diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index fd45bb24dd..139752b190 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -211,7 +211,7 @@ module ActiveRecord group_attr = @group_values association = @klass.reflect_on_association(group_attr.first.to_sym) associated = group_attr.size == 1 && association && association.macro == :belongs_to # only count belongs_to associations - group_fields = Array(associated ? association.primary_key_name : group_attr) + group_fields = Array(associated ? association.foreign_key : group_attr) group_aliases = group_fields.map { |field| column_alias_for(field) } group_columns = group_aliases.zip(group_fields).map { |aliaz,field| [aliaz, column_for(field)] diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 5acf3ec83a..e1f7fa2949 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -46,13 +46,17 @@ module ActiveRecord merged_relation.where_values = merged_wheres - (Relation::SINGLE_VALUE_METHODS - [:lock]).each do |method| + (Relation::SINGLE_VALUE_METHODS - [:lock, :create_with]).each do |method| value = r.send(:"#{method}_value") merged_relation.send(:"#{method}_value=", value) unless value.nil? end merged_relation.lock_value = r.lock_value unless merged_relation.lock_value + if r.create_with_value + merged_relation.create_with_value = (merged_relation.create_with_value || {}).merge(r.create_with_value) + end + # Apply scope extension modules merged_relation.send :apply_modules, r.extensions diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index cdde9a80d3..f97f89b6fe 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -198,16 +198,23 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal 1, Post.find(p.id).comments.size end - def test_belongs_to_with_primary_key_counter_with_assigning_nil - debate = Topic.create("title" => "debate") - reply = Reply.create("title" => "blah!", "content" => "world around!", "parent_title" => "debate") + def test_belongs_to_with_primary_key_counter + debate = Topic.create("title" => "debate") + debate2 = Topic.create("title" => "debate2") + reply = Reply.create("title" => "blah!", "content" => "world around!", "parent_title" => "debate") + + assert_equal 1, debate.reload.replies_count + assert_equal 0, debate2.reload.replies_count - assert_equal debate.title, reply.parent_title - assert_equal 1, Topic.find(debate.id).send(:read_attribute, "replies_count") + reply.topic_with_primary_key = debate2 + + assert_equal 0, debate.reload.replies_count + assert_equal 1, debate2.reload.replies_count reply.topic_with_primary_key = nil - assert_equal 0, Topic.find(debate.id).send(:read_attribute, "replies_count") + assert_equal 0, debate.reload.replies_count + assert_equal 0, debate2.reload.replies_count end def test_belongs_to_counter_with_reassigning @@ -419,6 +426,18 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_nil sponsor.sponsorable_id end + def test_assignment_updates_foreign_id_field_for_new_and_saved_records + client = Client.new + saved_firm = Firm.create :name => "Saved" + new_firm = Firm.new + + client.firm = saved_firm + assert_equal saved_firm.id, client.client_of + + client.firm = new_firm + assert_nil client.client_of + end + def test_polymorphic_assignment_with_primary_key_updates_foreign_id_field_for_new_and_saved_records essay = Essay.new saved_writer = Author.create(:name => "David") @@ -537,4 +556,27 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert proxy.stale_target? assert_equal companies(:first_firm), sponsor.sponsorable end + + def test_reloading_association_with_key_change + client = companies(:second_client) + firm = client.firm # note this is a proxy object + + client.firm = companies(:another_firm) + assert_equal companies(:another_firm), firm.reload + + client.client_of = companies(:first_firm).id + assert_equal companies(:first_firm), firm.reload + end + + def test_polymorphic_counter_cache + tagging = taggings(:welcome_general) + post = posts(:welcome) + comment = comments(:greetings) + + assert_difference 'post.reload.taggings_count', -1 do + assert_difference 'comment.reload.taggings_count', +1 do + tagging.taggable = comment + end + end + end end diff --git a/activerecord/test/cases/associations/callbacks_test.rb b/activerecord/test/cases/associations/callbacks_test.rb index 6a30e2905b..2d0d4541b4 100644 --- a/activerecord/test/cases/associations/callbacks_test.rb +++ b/activerecord/test/cases/associations/callbacks_test.rb @@ -3,6 +3,7 @@ require 'models/post' require 'models/author' require 'models/project' require 'models/developer' +require 'models/company' class AssociationCallbacksTest < ActiveRecord::TestCase fixtures :posts, :authors, :projects, :developers @@ -81,6 +82,14 @@ class AssociationCallbacksTest < ActiveRecord::TestCase assert_equal callback_log, jack.post_log end + def test_has_many_callbacks_for_destroy_on_parent + firm = Firm.create! :name => "Firm" + client = firm.clients.create! :name => "Client" + firm.destroy + + assert_equal ["before_remove#{client.id}", "after_remove#{client.id}"], firm.log + end + def test_has_and_belongs_to_many_add_callback david = developers(:david) ar = projects(:active_record) diff --git a/activerecord/test/cases/lifecycle_test.rb b/activerecord/test/cases/lifecycle_test.rb index b8c3ffb9cb..0558deb71b 100644 --- a/activerecord/test/cases/lifecycle_test.rb +++ b/activerecord/test/cases/lifecycle_test.rb @@ -101,9 +101,10 @@ class LifecycleTest < ActiveRecord::TestCase fixtures :topics, :developers, :minimalistics def test_before_destroy - original_count = Topic.count - (topic_to_be_destroyed = Topic.find(1)).destroy - assert_equal original_count - (1 + topic_to_be_destroyed.replies.size), Topic.count + topic = Topic.find(1) + assert_difference 'Topic.count', -(1 + topic.replies.size) do + topic.destroy + end end def test_auto_observer diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 901c12b26c..e3db34520e 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -8,6 +8,8 @@ require 'models/ship' require 'models/pirate' require 'models/price_estimate' require 'models/tagging' +require 'models/author' +require 'models/post' class ReflectionTest < ActiveRecord::TestCase include ActiveRecord::Reflection @@ -127,11 +129,11 @@ class ReflectionTest < ActiveRecord::TestCase def test_belongs_to_inferred_foreign_key_from_assoc_name Company.belongs_to :foo - assert_equal "foo_id", Company.reflect_on_association(:foo).primary_key_name + assert_equal "foo_id", Company.reflect_on_association(:foo).foreign_key Company.belongs_to :bar, :class_name => "Xyzzy" - assert_equal "bar_id", Company.reflect_on_association(:bar).primary_key_name + assert_equal "bar_id", Company.reflect_on_association(:bar).foreign_key Company.belongs_to :baz, :class_name => "Xyzzy", :foreign_key => "xyzzy_id" - assert_equal "xyzzy_id", Company.reflect_on_association(:baz).primary_key_name + assert_equal "xyzzy_id", Company.reflect_on_association(:baz).foreign_key end def test_association_reflection_in_modules diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb index e8672515fc..7c6899d438 100644 --- a/activerecord/test/cases/relation_scoping_test.rb +++ b/activerecord/test/cases/relation_scoping_test.rb @@ -485,4 +485,11 @@ class DefaultScopingTest < ActiveRecord::TestCase posts_offset_limit = Post.offset(2).limit(3) assert_equal posts_limit_offset, posts_offset_limit end + + def test_create_with_merge + aaron = (PoorDeveloperCalledJamis.create_with(:name => 'foo', :salary => 20) & + PoorDeveloperCalledJamis.create_with(:name => 'Aaron')).new + assert_equal 20, aaron.salary + assert_equal 'Aaron', aaron.name + end end diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index ee5f77b613..7af4dfe2c9 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -38,7 +38,9 @@ end class Firm < Company has_many :clients, :order => "id", :dependent => :destroy, :counter_sql => "SELECT COUNT(*) FROM companies WHERE firm_id = 1 " + - "AND (#{QUOTED_TYPE} = 'Client' OR #{QUOTED_TYPE} = 'SpecialClient' OR #{QUOTED_TYPE} = 'VerySpecialClient' )" + "AND (#{QUOTED_TYPE} = 'Client' OR #{QUOTED_TYPE} = 'SpecialClient' OR #{QUOTED_TYPE} = 'VerySpecialClient' )", + :before_remove => :log_before_remove, + :after_remove => :log_after_remove has_many :unsorted_clients, :class_name => "Client" has_many :clients_sorted_desc, :class_name => "Client", :order => "id DESC" has_many :clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id" @@ -88,6 +90,19 @@ class Firm < Company has_one :unautosaved_account, :foreign_key => "firm_id", :class_name => 'Account', :autosave => false has_many :accounts has_many :unautosaved_accounts, :foreign_key => "firm_id", :class_name => 'Account', :autosave => false + + def log + @log ||= [] + end + + private + def log_before_remove(record) + log << "before_remove#{record.id}" + end + + def log_after_remove(record) + log << "after_remove#{record.id}" + end end class DependentFirm < Company diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 3dea7e1492..7f366b2c91 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -143,6 +143,7 @@ ActiveRecord::Schema.define do t.text :body, :null => false end t.string :type + t.integer :taggings_count, :default => 0 end create_table :companies, :force => true do |t| |