aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorAaron Patterson <aaron.patterson@gmail.com>2011-01-01 11:43:53 -0800
committerAaron Patterson <aaron.patterson@gmail.com>2011-01-01 11:43:53 -0800
commit5c7fd8766586dba42fba6fc738a733175dd9d46b (patch)
tree5f1653b6ca1a307828b10c1013b84e1e4e41a97e /activerecord
parent171172f324444a9ea9c98e4f3663c6e401fb3ec7 (diff)
parent12675988813e82ac30f7c0e0008c12c4cf5d8cdc (diff)
downloadrails-5c7fd8766586dba42fba6fc738a733175dd9d46b.tar.gz
rails-5c7fd8766586dba42fba6fc738a733175dd9d46b.tar.bz2
rails-5c7fd8766586dba42fba6fc738a733175dd9d46b.zip
Merge remote branch 'jonleighton/association_fixes' into fuuu
* jonleighton/association_fixes: Rename AssociationReflection#primary_key_name to foreign_key, since the options key which it relates to is :foreign_key Support for :counter_cache on polymorphic belongs_to Refactor BelongsToAssociation to allow BelongsToPolymorphicAssociation to inherit from it Specify the STI type condition using SQL IN rather than a whole load of ORs. Required a fix to ActiveRecord::Relation#merge for properly merging create_with_value. This also fixes a situation where the type condition was appearing twice in the resultant SQL query. Verify that when has_many associated objects are destroyed via :dependent => :destroy, when the parent is destroyed, the callbacks are run Get rid of extra_conditions param from configure_dependency_for_has_many. I can't see a particularly plausible argument for this being used by plugins, and if they really want they can just redefine the callback or whatever. Note also that before my recent commit the extra_conditions param was completely ignored for :dependent => :destroy. And owner_quoted_id can go too Now we can drop-kick AssociationReflection#dependent_conditions into oblivion. Refactor configure_dependency_for_has_many to use AssociationCollection#delete_all. It was necessary to change test_before_destroy in lifecycle_test.rb so that it checks topic.replies.size *before* doing the destroy, as afterwards it will now (correctly) be 0.
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/lib/active_record/association_preload.rb22
-rw-r--r--activerecord/lib/active_record/associations.rb68
-rw-r--r--activerecord/lib/active_record/associations/association_collection.rb6
-rw-r--r--activerecord/lib/active_record/associations/association_proxy.rb36
-rw-r--r--activerecord/lib/active_record/associations/belongs_to_association.rb106
-rw-r--r--activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb66
-rw-r--r--activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb8
-rw-r--r--activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb4
-rw-r--r--activerecord/lib/active_record/associations/has_association.rb4
-rw-r--r--activerecord/lib/active_record/associations/has_many_association.rb10
-rw-r--r--activerecord/lib/active_record/associations/has_many_through_association.rb2
-rw-r--r--activerecord/lib/active_record/associations/has_one_association.rb15
-rw-r--r--activerecord/lib/active_record/associations/through_association.rb12
-rw-r--r--activerecord/lib/active_record/autosave_association.rb6
-rw-r--r--activerecord/lib/active_record/base.rb12
-rw-r--r--activerecord/lib/active_record/fixtures.rb2
-rw-r--r--activerecord/lib/active_record/reflection.rb25
-rw-r--r--activerecord/lib/active_record/relation/calculations.rb2
-rw-r--r--activerecord/lib/active_record/relation/spawn_methods.rb6
-rw-r--r--activerecord/test/cases/associations/belongs_to_associations_test.rb54
-rw-r--r--activerecord/test/cases/associations/callbacks_test.rb9
-rw-r--r--activerecord/test/cases/lifecycle_test.rb7
-rw-r--r--activerecord/test/cases/reflection_test.rb8
-rw-r--r--activerecord/test/cases/relation_scoping_test.rb7
-rw-r--r--activerecord/test/models/company.rb17
-rw-r--r--activerecord/test/schema/schema.rb1
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|