From 9bc75fd007d56d818b8620569410a20aa92c9fc5 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Sun, 6 Apr 2008 02:18:42 +0000 Subject: Remove duplicate code from associations. [Pratik] git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9231 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 2 + .../associations/association_collection.rb | 68 +++++++++++++++++---- .../has_and_belongs_to_many_association.rb | 53 ++-------------- .../associations/has_many_association.rb | 41 ------------- .../associations/has_many_through_association.rb | 70 ++++------------------ .../associations/has_one_through_association.rb | 2 +- 6 files changed, 72 insertions(+), 164 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 474b1499d3..90173a4bc1 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Remove duplicate code from associations. [Pratik] + * Refactor HasManyThroughAssociation to inherit from HasManyAssociation. Association callbacks and _ids= now work with hm:t. #11516 [rubyruy] * Ensure HABTM#create and HABTM#build do not load entire association. [Pratik] diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 73f22cb0fa..9c724d2117 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -3,6 +3,51 @@ require 'set' module ActiveRecord module Associations class AssociationCollection < AssociationProxy #:nodoc: + def initialize(owner, reflection) + super + construct_sql + end + + def find(*args) + options = args.extract_options! + + # If using a custom finder_sql, scan the entire collection. + if @reflection.options[:finder_sql] + expects_array = args.first.kind_of?(Array) + ids = args.flatten.compact.uniq.map(&:to_i) + + if ids.size == 1 + id = ids.first + record = load_target.detect { |r| id == r.id } + expects_array ? [ record ] : record + else + load_target.select { |r| ids.include?(r.id) } + end + else + conditions = "#{@finder_sql}" + if sanitized_conditions = sanitize_sql(options[:conditions]) + conditions << " AND (#{sanitized_conditions})" + end + + options[:conditions] = conditions + + if options[:order] && @reflection.options[:order] + options[:order] = "#{options[:order]}, #{@reflection.options[:order]}" + elsif @reflection.options[:order] + options[:order] = @reflection.options[:order] + end + + # Build options specific to association + construct_find_options!(options) + + merge_options_from_reflection!(options) + + # Pass through args exactly as we received them. + args << options + @reflection.klass.find(*args) + end + end + def to_ary load_target @target.to_ary @@ -30,10 +75,9 @@ module ActiveRecord @owner.transaction do flatten_deeper(records).each do |record| raise_on_type_mismatch(record) - callback(:before_add, record) - result &&= insert_record(record) unless @owner.new_record? - @target << record - callback(:after_add, record) + add_record_to_target_with_callbacks(record) do |r| + result &&= insert_record(record) unless @owner.new_record? + end end end @@ -63,18 +107,13 @@ module ActiveRecord def delete(*records) records = flatten_deeper(records) records.each { |record| raise_on_type_mismatch(record) } - records.reject! do |record| - if record.new_record? - callback(:before_remove, record) - @target.delete(record) - callback(:after_remove, record) - end - end - return if records.empty? @owner.transaction do records.each { |record| callback(:before_remove, record) } - delete_records(records) + + old_records = records.reject {|r| r.new_record? } + delete_records(old_records) if old_records.any? + records.each do |record| @target.delete(record) callback(:after_remove, record) @@ -180,6 +219,9 @@ module ActiveRecord end protected + def construct_find_options!(options) + end + def load_target if !@owner.new_record? || foreign_key_present begin 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 8ce5b83831..d4143e0645 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 @@ -1,11 +1,6 @@ module ActiveRecord module Associations class HasAndBelongsToManyAssociation < AssociationCollection #:nodoc: - def initialize(owner, reflection) - super - construct_sql - end - def create(attributes = {}) create_record(attributes) { |record| insert_record(record) } end @@ -14,53 +9,13 @@ module ActiveRecord create_record(attributes) { |record| insert_record(record, true) } end - def find_first - load_target.first - end - - def find(*args) - options = args.extract_options! - - # If using a custom finder_sql, scan the entire collection. - if @reflection.options[:finder_sql] - expects_array = args.first.kind_of?(Array) - ids = args.flatten.compact.uniq - - if ids.size == 1 - id = ids.first.to_i - record = load_target.detect { |r| id == r.id } - expects_array ? [record] : record - else - load_target.select { |r| ids.include?(r.id) } - end - else - conditions = "#{@finder_sql}" - - if sanitized_conditions = sanitize_sql(options[:conditions]) - conditions << " AND (#{sanitized_conditions})" - end - - options[:conditions] = conditions + protected + def construct_find_options!(options) options[:joins] = @join_sql options[:readonly] = finding_with_ambiguous_select?(options[:select] || @reflection.options[:select]) - - if options[:order] && @reflection.options[:order] - options[:order] = "#{options[:order]}, #{@reflection.options[:order]}" - elsif @reflection.options[:order] - options[:order] = @reflection.options[:order] - end - - merge_options_from_reflection!(options) - - options[:select] ||= (@reflection.options[:select] || '*') - - # Pass through args exactly as we received them. - args << options - @reflection.klass.find(*args) + options[:select] ||= (@reflection.options[:select] || '*') end - end - - protected + def count_records load_target.size end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 6cf10c2192..963a938485 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -1,11 +1,6 @@ module ActiveRecord module Associations class HasManyAssociation < AssociationCollection #:nodoc: - def initialize(owner, reflection) - super - construct_sql - end - # Count the number of associated records. All arguments are optional. def count(*args) if @reflection.options[:counter_sql] @@ -23,42 +18,6 @@ module ActiveRecord end end - def find(*args) - options = args.extract_options! - - # If using a custom finder_sql, scan the entire collection. - if @reflection.options[:finder_sql] - expects_array = args.first.kind_of?(Array) - ids = args.flatten.compact.uniq.map(&:to_i) - - if ids.size == 1 - id = ids.first - record = load_target.detect { |r| id == r.id } - expects_array ? [ record ] : record - else - load_target.select { |r| ids.include?(r.id) } - end - else - conditions = "#{@finder_sql}" - if sanitized_conditions = sanitize_sql(options[:conditions]) - conditions << " AND (#{sanitized_conditions})" - end - options[:conditions] = conditions - - if options[:order] && @reflection.options[:order] - options[:order] = "#{options[:order]}, #{@reflection.options[:order]}" - elsif @reflection.options[:order] - options[:order] = @reflection.options[:order] - end - - merge_options_from_reflection!(options) - - # Pass through args exactly as we received them. - args << options - @reflection.klass.find(*args) - end - end - protected def count_records count = if has_cached_counter? 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 23cead3ca6..ebea313c18 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -2,37 +2,8 @@ module ActiveRecord module Associations class HasManyThroughAssociation < HasManyAssociation #:nodoc: def initialize(owner, reflection) - super reflection.check_validity! - @finder_sql = construct_conditions - end - - - def find(*args) - options = args.extract_options! - - conditions = "#{@finder_sql}" - if sanitized_conditions = sanitize_sql(options[:conditions]) - conditions << " AND (#{sanitized_conditions})" - end - options[:conditions] = conditions - - if options[:order] && @reflection.options[:order] - options[:order] = "#{options[:order]}, #{@reflection.options[:order]}" - elsif @reflection.options[:order] - options[:order] = @reflection.options[:order] - end - - options[:select] = construct_select(options[:select]) - options[:from] ||= construct_from - options[:joins] = construct_joins(options[:joins]) - options[:include] = @reflection.source_reflection.options[:include] if options[:include].nil? - - merge_options_from_reflection!(options) - - # Pass through args exactly as we received them. - args << options - @reflection.klass.find(*args) + super end alias_method :new, :build @@ -59,15 +30,6 @@ module ActiveRecord return @target.size if loaded? return count end - - # Calculate sum using SQL, not Enumerable - def sum(*args) - if block_given? - calculate(:sum, *args) { |*block_args| yield(*block_args) } - else - calculate(:sum, *args) - end - end def count(*args) column_name, options = @reflection.klass.send(:construct_count_options_from_args, *args) @@ -79,8 +41,14 @@ module ActiveRecord @reflection.klass.send(:with_scope, construct_scope) { @reflection.klass.count(column_name, options) } end - protected + def construct_find_options!(options) + options[:select] = construct_select(options[:select]) + options[:from] ||= construct_from + options[:joins] = construct_joins(options[:joins]) + options[:include] = @reflection.source_reflection.options[:include] if options[:include].nil? + end + def insert_record(record, force=true) if record.new_record? if force @@ -101,26 +69,6 @@ module ActiveRecord end end - def method_missing(method, *args) - if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method)) - if block_given? - super { |*block_args| yield(*block_args) } - else - super - end - elsif @reflection.klass.scopes.include?(method) - @reflection.klass.scopes[method].call(self, *args) - else - with_scope construct_scope do - if block_given? - @reflection.klass.send(method, *args) { |*block_args| yield(*block_args) } - else - @reflection.klass.send(method, *args) - end - end - end - end - def find_target @reflection.klass.find(:all, :select => construct_select, @@ -237,6 +185,8 @@ module ActiveRecord @finder_sql = "#{@reflection.quoted_table_name}.#{@reflection.primary_key_name} = #{@owner.quoted_id}" @finder_sql << " AND (#{conditions})" if conditions + else + @finder_sql = construct_conditions end if @reflection.options[:counter_sql] diff --git a/activerecord/lib/active_record/associations/has_one_through_association.rb b/activerecord/lib/active_record/associations/has_one_through_association.rb index ecf9d8208d..c846956e1f 100644 --- a/activerecord/lib/active_record/associations/has_one_through_association.rb +++ b/activerecord/lib/active_record/associations/has_one_through_association.rb @@ -1,6 +1,6 @@ module ActiveRecord module Associations - class HasOneThroughAssociation < ActiveRecord::Associations::HasManyThroughAssociation + class HasOneThroughAssociation < HasManyThroughAssociation def create_through_record(new_value) #nodoc: klass = @reflection.through_reflection.klass -- cgit v1.2.3