diff options
Diffstat (limited to 'activerecord/lib/active_record/associations')
5 files changed, 64 insertions, 43 deletions
diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 6cc401e6cc..6f8b76abda 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -20,31 +20,19 @@ module ActiveRecord # It's okay to just apply all these like this. The options will only be present if the # association supports that option; this is enforced by the association builder. scope = scope.apply_finder_options(options.slice( - :readonly, :include, :order, :limit, :joins, :group, :having, :offset)) + :readonly, :include, :order, :limit, :joins, :group, :having, :offset, :select)) if options[:through] && !options[:include] scope = scope.includes(source_options[:include]) end - if select = select_value - scope = scope.select(select) - end + scope = scope.uniq if options[:uniq] add_constraints(scope) end private - def select_value - select_value = options[:select] - - if reflection.collection? - select_value ||= options[:uniq] && "DISTINCT #{reflection.quoted_table_name}.*" - end - - select_value - end - def add_constraints(scope) tables = construct_tables diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index cec876149c..362f1053cd 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -344,8 +344,12 @@ module ActiveRecord if options[:counter_sql] interpolate(options[:counter_sql]) else - # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */ - interpolate(options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" } + # replace the SELECT clause with COUNT(SELECTS), preserving any hints within /* ... */ + interpolate(options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) do + count_with = $2.to_s + count_with = '*' if count_with.blank? || count_with =~ /,/ + "SELECT #{$1}COUNT(#{count_with}) FROM" + end end end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 50ee60284c..3353cdf1ef 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -89,8 +89,12 @@ module ActiveRecord records.each { |r| r.destroy } update_counter(-records.length) unless inverse_updates_counter_cache? else - keys = records.map { |r| r[reflection.association_primary_key] } - scope = scoped.where(reflection.association_primary_key => keys) + scope = scoped + + unless records == load_target + keys = records.map { |r| r[reflection.association_primary_key] } + scope = scoped.where(reflection.association_primary_key => keys) + end if method == :delete_all update_counter(-scope.delete_all) 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 2e818dca5d..7e6e3be382 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -6,6 +6,13 @@ module ActiveRecord class HasManyThroughAssociation < HasManyAssociation #:nodoc: include ThroughAssociation + def initialize(owner, reflection) + super + + @through_records = {} + @through_association = nil + end + # Returns the size of the collection by executing a SELECT COUNT(*) query if the collection hasn't been # loaded and calling collection.size if it has. If it's more likely than not that the collection does # have a size larger than zero, and you need to fetch that collection afterwards, it'll take one fewer @@ -42,27 +49,36 @@ module ActiveRecord end end - through_record(record).save! + save_through_record(record) update_counter(1) record end private - def through_record(record) - through_association = owner.association(through_reflection.name) - attributes = construct_join_attributes(record) - - through_record = Array.wrap(through_association.target).find { |candidate| - candidate.attributes.slice(*attributes.keys) == attributes - } + def through_association + @through_association ||= owner.association(through_reflection.name) + end - unless through_record - through_record = through_association.build(attributes) + # We temporarily cache through record that has been build, because if we build a + # through record in build_record and then subsequently call insert_record, then we + # want to use the exact same object. + # + # However, after insert_record has been called, we clear the cache entry because + # we want it to be possible to have multiple instances of the same record in an + # association + def build_through_record(record) + @through_records[record.object_id] ||= begin + through_record = through_association.build(construct_join_attributes(record)) through_record.send("#{source_reflection.name}=", record) + through_record end + end - through_record + def save_through_record(record) + build_through_record(record).save! + ensure + @through_records.delete(record.object_id) end def build_record(attributes, options = {}) @@ -73,9 +89,9 @@ module ActiveRecord inverse = source_reflection.inverse_of if inverse if inverse.macro == :has_many - record.send(inverse.name) << through_record(record) + record.send(inverse.name) << build_through_record(record) elsif inverse.macro == :has_one - record.send("#{inverse.name}=", through_record(record)) + record.send("#{inverse.name}=", build_through_record(record)) end end @@ -104,8 +120,7 @@ module ActiveRecord def delete_records(records, method) ensure_not_nested - through = owner.association(through_reflection.name) - scope = through.scoped.where(construct_join_attributes(*records)) + scope = through_association.scoped.where(construct_join_attributes(*records)) case method when :destroy @@ -116,7 +131,7 @@ module ActiveRecord count = scope.delete_all end - delete_through_records(through, records) + delete_through_records(records) if through_reflection.macro == :has_many && update_through_counter?(method) update_counter(-count, through_reflection) @@ -125,15 +140,25 @@ module ActiveRecord update_counter(-count) end - def delete_through_records(through, records) - if through_reflection.macro == :has_many - records.each do |record| - through.target.delete(through_record(record)) - end - else - records.each do |record| - through.target = nil if through.target == through_record(record) + def through_records_for(record) + attributes = construct_join_attributes(record) + candidates = Array.wrap(through_association.target) + candidates.find_all { |c| c.attributes.slice(*attributes.keys) == attributes } + end + + def delete_through_records(records) + records.each do |record| + through_records = through_records_for(record) + + if through_reflection.macro == :has_many + through_records.each { |r| through_association.target.delete(r) } + else + if through_records.include?(through_association.target) + through_association.target = nil + end end + + @through_records.delete(record.object_id) end end diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index b347a94978..f95e5337c2 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -44,7 +44,7 @@ module ActiveRecord join_attributes = { source_reflection.foreign_key => records.map { |record| - record.send(source_reflection.association_primary_key) + record.send(source_reflection.association_primary_key(reflection.klass)) } } |