diff options
11 files changed, 79 insertions, 60 deletions
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 14aa557b6c..00321ec860 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -16,12 +16,6 @@ module ActiveRecord # If you need to work on all current children, new and existing records, # +load_target+ and the +loaded+ flag are your friends. class CollectionAssociation < Association #:nodoc: - attr_reader :proxy - - def initialize(owner, reflection) - super - @proxy = CollectionProxy.new(self) - end # Implements the reader method, e.g. foo.items for Foo.has_many :items def reader(force_reload = false) @@ -31,7 +25,7 @@ module ActiveRecord reload end - proxy + CollectionProxy.new(self) end # Implements the writer method, e.g. foo.items= for Foo.has_many :items diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 261a829281..47ed9f58d8 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -33,14 +33,7 @@ module ActiveRecord # # is computed directly through SQL and does not trigger by itself the # instantiation of the actual post records. - class CollectionProxy # :nodoc: - alias :proxy_extend :extend - - instance_methods.each { |m| undef_method m unless m.to_s =~ /^(?:nil\?|send|object_id|to_a)$|^__|^respond_to|proxy_/ } - - delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :includes, :from, - :lock, :readonly, :having, :pluck, :to => :scoped - + class CollectionProxy < Relation # :nodoc: delegate :target, :load_target, :loaded?, :to => :@association delegate :select, :find, :first, :last, @@ -52,7 +45,8 @@ module ActiveRecord def initialize(association) @association = association - Array(association.options[:extend]).each { |ext| proxy_extend(ext) } + super association.klass, association.klass.arel_table + merge! association.scoped end alias_method :new, :build @@ -61,15 +55,24 @@ module ActiveRecord @association end + # We don't want this object to be put on the scoping stack, because + # that could create an infinite loop where we call an @association + # method, which gets the current scope, which is this object, which + # delegates to @association, and so on. + def scoping + @association.scoped.scoping { yield } + end + + def spawn + scoped + end + def scoped(options = nil) association = @association - scope = association.scoped - scope.extending! do + super.extending! do define_method(:proxy_association) { association } end - scope.merge!(options) if options - scope end def respond_to?(name, include_private = false) @@ -81,7 +84,7 @@ module ActiveRecord def method_missing(method, *args, &block) match = DynamicMatchers::Method.match(self, method) if match && match.is_a?(DynamicMatchers::Instantiator) - scoped.send(method, *args) do |r| + super do |r| proxy_association.send :set_owner_attributes, r proxy_association.send :add_to_target, r yield(r) if block_given? @@ -101,10 +104,14 @@ module ActiveRecord end else - scoped.readonly(nil).public_send(method, *args, &block) + super end end + def ==(other) + load_target == other + end + # Forwards <tt>===</tt> explicitly to the \target because the instance method # removal above doesn't catch it. Loads the \target if needed. def ===(other) diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 3005bef092..d545e7799d 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -350,7 +350,7 @@ module ActiveRecord end records_to_destroy.each do |record| - association.proxy.destroy(record) + association.destroy(record) end end diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index b40bf2b3cf..6a0cdd5917 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -34,9 +34,6 @@ module ActiveRecord private def self.build(attribute, value) case value - when ActiveRecord::Relation - value = value.select(value.klass.arel_table[value.klass.primary_key]) if value.select_values.empty? - attribute.in(value.arel.ast) when Array, ActiveRecord::Associations::CollectionProxy values = value.to_a.map {|x| x.is_a?(ActiveRecord::Model) ? x.id : x} ranges, values = values.partition {|v| v.is_a?(Range)} @@ -59,6 +56,9 @@ module ActiveRecord array_predicates = ranges.map { |range| attribute.in(range) } array_predicates << values_predicate array_predicates.inject { |composite, predicate| composite.or(predicate) } + when ActiveRecord::Relation + value = value.select(value.klass.arel_table[value.klass.primary_key]) if value.select_values.empty? + attribute.in(value.arel.ast) when Range attribute.in(value) when ActiveRecord::Model diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 855477eaed..29536b16c4 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -40,7 +40,7 @@ module ActiveRecord alias extensions extending_values def includes(*args) - args.empty? ? self : clone.includes!(*args) + args.empty? ? self : spawn.includes!(*args) end def includes!(*args) @@ -51,7 +51,7 @@ module ActiveRecord end def eager_load(*args) - args.blank? ? self : clone.eager_load!(*args) + args.blank? ? self : spawn.eager_load!(*args) end def eager_load!(*args) @@ -60,7 +60,7 @@ module ActiveRecord end def preload(*args) - args.blank? ? self : clone.preload!(*args) + args.blank? ? self : spawn.preload!(*args) end def preload!(*args) @@ -79,7 +79,7 @@ module ActiveRecord # User.includes(:posts).where("posts.name = 'foo'").references(:posts) # # => Query now knows the string references posts, so adds a JOIN def references(*args) - args.blank? ? self : clone.references!(*args) + args.blank? ? self : spawn.references!(*args) end def references!(*args) @@ -120,7 +120,7 @@ module ActiveRecord if block_given? to_a.select { |*block_args| value.call(*block_args) } else - clone.select!(value) + spawn.select!(value) end end @@ -130,7 +130,7 @@ module ActiveRecord end def group(*args) - args.blank? ? self : clone.group!(*args) + args.blank? ? self : spawn.group!(*args) end def group!(*args) @@ -139,7 +139,7 @@ module ActiveRecord end def order(*args) - args.blank? ? self : clone.order!(*args) + args.blank? ? self : spawn.order!(*args) end def order!(*args) @@ -165,7 +165,7 @@ module ActiveRecord # generates a query with 'ORDER BY id ASC, name ASC'. # def reorder(*args) - args.blank? ? self : clone.reorder!(*args) + args.blank? ? self : spawn.reorder!(*args) end def reorder!(*args) @@ -175,7 +175,7 @@ module ActiveRecord end def joins(*args) - args.compact.blank? ? self : clone.joins!(*args) + args.compact.blank? ? self : spawn.joins!(*args) end def joins!(*args) @@ -186,7 +186,7 @@ module ActiveRecord end def bind(value) - clone.bind!(value) + spawn.bind!(value) end def bind!(value) @@ -195,7 +195,7 @@ module ActiveRecord end def where(opts, *rest) - opts.blank? ? self : clone.where!(opts, *rest) + opts.blank? ? self : spawn.where!(opts, *rest) end def where!(opts, *rest) @@ -206,7 +206,7 @@ module ActiveRecord end def having(opts, *rest) - opts.blank? ? self : clone.having!(opts, *rest) + opts.blank? ? self : spawn.having!(opts, *rest) end def having!(opts, *rest) @@ -217,7 +217,7 @@ module ActiveRecord end def limit(value) - clone.limit!(value) + spawn.limit!(value) end def limit!(value) @@ -226,7 +226,7 @@ module ActiveRecord end def offset(value) - clone.offset!(value) + spawn.offset!(value) end def offset!(value) @@ -235,7 +235,7 @@ module ActiveRecord end def lock(locks = true) - clone.lock!(locks) + spawn.lock!(locks) end def lock!(locks = true) @@ -283,7 +283,7 @@ module ActiveRecord end def readonly(value = true) - clone.readonly!(value) + spawn.readonly!(value) end def readonly!(value = true) @@ -292,7 +292,7 @@ module ActiveRecord end def create_with(value) - clone.create_with!(value) + spawn.create_with!(value) end def create_with!(value) @@ -301,7 +301,7 @@ module ActiveRecord end def from(value) - clone.from!(value) + spawn.from!(value) end def from!(value) @@ -320,7 +320,7 @@ module ActiveRecord # User.select(:name).uniq.uniq(false) # # => You can also remove the uniqueness def uniq(value = true) - clone.uniq!(value) + spawn.uniq!(value) end def uniq!(value = true) @@ -366,7 +366,7 @@ module ActiveRecord # end def extending(*modules, &block) if modules.any? || block - clone.extending!(*modules, &block) + spawn.extending!(*modules, &block) else self end @@ -382,7 +382,7 @@ module ActiveRecord end def reverse_order - clone.reverse_order! + spawn.reverse_order! end def reverse_order! diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index f6d178db7a..80d087a9ea 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -5,7 +5,12 @@ require 'active_record/relation/merger' module ActiveRecord module SpawnMethods - + + # This is overridden by Associations::CollectionProxy + def spawn #:nodoc: + clone + end + # Merges in the conditions from <tt>other</tt>, if <tt>other</tt> is an <tt>ActiveRecord::Relation</tt>. # Returns an array representing the intersection of the resulting records with <tt>other</tt>, if <tt>other</tt> is an array. # @@ -23,7 +28,7 @@ module ActiveRecord if other.is_a?(Array) to_a & other elsif other - clone.merge!(other) + spawn.merge!(other) else self end @@ -42,7 +47,7 @@ module ActiveRecord # Post.where('id > 10').order('id asc').except(:where) # discards the where condition but keeps the order # def except(*skips) - result = self.class.new(@klass, table, values.except(*skips)) + result = Relation.new(klass, table, values.except(*skips)) result.default_scoped = default_scoped result.extend(*extending_values) if extending_values.any? result @@ -56,7 +61,7 @@ module ActiveRecord # Post.order('id asc').only(:where, :order) # uses the specified order # def only(*onlies) - result = self.class.new(@klass, table, values.slice(*onlies)) + result = Relation.new(klass, table, values.slice(*onlies)) result.default_scoped = default_scoped result.extend(*extending_values) if extending_values.any? result diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 2e44005847..08467900f9 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -189,7 +189,7 @@ class EagerAssociationTest < ActiveRecord::TestCase author = assert_queries(3) { Author.scoped(:includes => {:posts_with_comments => :comments}).find(author_id) } # find the author, then find the posts, then find the comments author.posts_with_comments.each do |post_with_comments| assert_equal post_with_comments.comments.length, post_with_comments.comments.count - assert_nil post_with_comments.comments.uniq! + assert_nil post_with_comments.comments.to_a.uniq! end end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 22fd80df28..cd581485d5 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -497,7 +497,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase def test_include_uses_array_include_after_loaded project = projects(:active_record) - project.developers.class # force load target + project.developers.load_target developer = project.developers.first diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index f74fe42dc2..47ad6a1ba7 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -892,12 +892,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase client_id = firm.clients_of_firm.first.id assert_equal 1, firm.clients_of_firm.size - cleared = firm.clients_of_firm.clear + firm.clients_of_firm.clear assert_equal 0, firm.clients_of_firm.size assert_equal 0, firm.clients_of_firm(true).size assert_equal [], Client.destroyed_client_ids[firm.id] - assert_equal firm.clients_of_firm.object_id, cleared.object_id # Should not be destroyed since the association is not dependent. assert_nothing_raised do @@ -1359,7 +1358,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_include_uses_array_include_after_loaded firm = companies(:first_firm) - firm.clients.class # force load target + firm.clients.load_target client = firm.clients.first @@ -1409,7 +1408,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_calling_first_or_last_on_loaded_association_should_not_fetch_with_query firm = companies(:first_firm) - firm.clients.class # force load target + firm.clients.load_target assert firm.clients.loaded? assert_no_queries do @@ -1708,4 +1707,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase firm = companies(:first_firm) assert_equal [accounts(:signals37)], firm.accounts.open end + + test "first_or_initialize adds the record to the association" do + firm = Firm.create! name: 'omg' + client = firm.clients_of_firm.first_or_initialize + assert_equal [client], firm.clients_of_firm + end + + test "first_or_create adds the record to the association" do + firm = Firm.create! name: 'omg' + firm.clients_of_firm.load_target + client = firm.clients_of_firm.first_or_create name: 'lol' + assert_equal [client], firm.clients_of_firm + assert_equal [client], firm.reload.clients_of_firm + end end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 3606ce691c..20b0eeb5ea 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -676,7 +676,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_has_many_through_include_uses_array_include_after_loaded david = authors(:david) - david.categories.class # force load target + david.categories.load_target category = david.categories.first diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index c93c91803e..8ef3bfef15 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -872,7 +872,7 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase 2.times { |i| @pirate.parrots.create!(:name => "parrots_#{i}") } before = @pirate.parrots.map { |c| c.mark_for_destruction ; c } - class << @pirate.parrots + class << @pirate.association(:parrots) def destroy(*args) super raise 'Oh noes!' @@ -1277,7 +1277,7 @@ module AutosaveAssociationOnACollectionAssociationTests def test_should_not_load_the_associated_models_if_they_were_not_loaded_yet assert_queries(1) { @pirate.catchphrase = 'Arr'; @pirate.save! } - @pirate.send(@association_name).class # hack to load the target + @pirate.send(@association_name).load_target assert_queries(3) do @pirate.catchphrase = 'Yarr' |