diff options
5 files changed, 156 insertions, 56 deletions
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 1548e68cea..832b963052 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -34,7 +34,7 @@ module ActiveRecord reload end - CollectionProxy.new(self) + CollectionProxy.new(klass, 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 e444b0ed83..6e05af894e 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -30,9 +30,9 @@ module ActiveRecord class CollectionProxy < Relation delegate(*(ActiveRecord::Calculations.public_instance_methods - [:count]), to: :scope) - def initialize(association) #:nodoc: + def initialize(klass, association) #:nodoc: @association = association - super association.klass, association.klass.arel_table + super klass, klass.arel_table merge! association.scope(nullify: false) end diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index dbfa92bbbd..2184625e22 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -1,27 +1,113 @@ -require 'thread' +require 'active_support/concern' +require 'mutex_m' module ActiveRecord module Delegation # :nodoc: - # Set up common delegations for performance (avoids method_missing) + extend ActiveSupport::Concern + + # This module creates compiled delegation methods dynamically at runtime, which makes + # subsequent calls to that method faster by avoiding method_missing. The delegations + # may vary depending on the klass of a relation, so we create a subclass of Relation + # for each different klass, and the delegations are compiled into that subclass only. + delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, :to => :to_a delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :connection, :columns_hash, :auto_explain_threshold_in_seconds, :to => :klass - @@delegation_mutex = Mutex.new + module ClassSpecificRelation + extend ActiveSupport::Concern - def self.delegate_to_scoped_klass(method) - if method.to_s =~ /\A[a-zA-Z_]\w*[!?]?\z/ - module_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{method}(*args, &block) - scoping { @klass.#{method}(*args, &block) } + included do + @delegation_mutex = Mutex.new + end + + module ClassMethods + def name + superclass.name + end + + def delegate_to_scoped_klass(method) + @delegation_mutex.synchronize do + return if method_defined?(method) + + if method.to_s =~ /\A[a-zA-Z_]\w*[!?]?\z/ + module_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{method}(*args, &block) + scoping { @klass.#{method}(*args, &block) } + end + RUBY + else + module_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{method}(*args, &block) + scoping { @klass.send(#{method.inspect}, *args, &block) } + end + RUBY + end end - RUBY - else - module_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{method}(*args, &block) - scoping { @klass.send(#{method.inspect}, *args, &block) } + end + + def delegate(method, opts = {}) + @delegation_mutex.synchronize do + return if method_defined?(method) + super end - RUBY + end + end + + protected + + def method_missing(method, *args, &block) + if @klass.respond_to?(method) + self.class.delegate_to_scoped_klass(method) + scoping { @klass.send(method, *args, &block) } + elsif Array.method_defined?(method) + self.class.delegate method, :to => :to_a + to_a.send(method, *args, &block) + elsif arel.respond_to?(method) + self.class.delegate method, :to => :arel + arel.send(method, *args, &block) + else + super + end + end + end + + module ClassMethods + # This hash is keyed by klass.name to avoid memory leaks in development mode + @@subclasses = Hash.new { |h, k| h[k] = {} }.extend(Mutex_m) + + def new(klass, *args) + relation = relation_class_for(klass).allocate + relation.__send__(:initialize, klass, *args) + relation + end + + # Cache the constants in @@subclasses because looking them up via const_get + # make instantiation significantly slower. + def relation_class_for(klass) + if klass && klass.name + if subclass = @@subclasses.synchronize { @@subclasses[self][klass.name] } + subclass + else + subclass = const_get("#{name.gsub('::', '_')}_#{klass.name.gsub('::', '_')}", false) + @@subclasses.synchronize { @@subclasses[self][klass.name] = subclass } + subclass + end + else + ActiveRecord::Relation + end + end + + # Check const_defined? in case another thread has already defined the constant. + # I am not sure whether this is strictly necessary. + def const_missing(name) + @@subclasses.synchronize { + if const_defined?(name) + const_get(name) + else + const_set(name, Class.new(self) { include ClassSpecificRelation }) + end + } end end @@ -35,28 +121,10 @@ module ActiveRecord def method_missing(method, *args, &block) if @klass.respond_to?(method) - @@delegation_mutex.synchronize do - unless ::ActiveRecord::Delegation.method_defined?(method) - ::ActiveRecord::Delegation.delegate_to_scoped_klass(method) - end - end - scoping { @klass.send(method, *args, &block) } elsif Array.method_defined?(method) - @@delegation_mutex.synchronize do - unless ::ActiveRecord::Delegation.method_defined?(method) - ::ActiveRecord::Delegation.delegate method, :to => :to_a - end - end - to_a.send(method, *args, &block) elsif arel.respond_to?(method) - @@delegation_mutex.synchronize do - unless ::ActiveRecord::Delegation.method_defined?(method) - ::ActiveRecord::Delegation.delegate method, :to => :arel - end - end - arel.send(method, *args, &block) else super diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 98e278df82..92dc575d37 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -6,26 +6,26 @@ module ActiveRecord class RelationTest < ActiveRecord::TestCase fixtures :posts, :comments - class FakeKlass < Struct.new(:table_name) + class FakeKlass < Struct.new(:table_name, :name) end def test_construction relation = nil assert_nothing_raised do - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b end - assert_equal :a, relation.klass + assert_equal FakeKlass, relation.klass assert_equal :b, relation.table assert !relation.loaded, 'relation is not loaded' end def test_responds_to_model_and_returns_klass - relation = Relation.new :a, :b - assert_equal :a, relation.model + relation = Relation.new FakeKlass, :b + assert_equal FakeKlass, relation.model end def test_initialize_single_values - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b (Relation::SINGLE_VALUE_METHODS - [:create_with]).each do |method| assert_nil relation.send("#{method}_value"), method.to_s end @@ -33,19 +33,19 @@ module ActiveRecord end def test_multi_value_initialize - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b Relation::MULTI_VALUE_METHODS.each do |method| assert_equal [], relation.send("#{method}_values"), method.to_s end end def test_extensions - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert_equal [], relation.extensions end def test_empty_where_values_hash - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert_equal({}, relation.where_values_hash) relation.where! :hello @@ -79,7 +79,7 @@ module ActiveRecord end def test_scope_for_create - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert_equal({}, relation.scope_for_create) end @@ -110,31 +110,31 @@ module ActiveRecord end def test_empty_eager_loading? - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert !relation.eager_loading? end def test_eager_load_values - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b relation.eager_load! :b assert relation.eager_loading? end def test_references_values - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert_equal [], relation.references_values relation = relation.references(:foo).references(:omg, :lol) assert_equal ['foo', 'omg', 'lol'], relation.references_values end def test_references_values_dont_duplicate - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b relation = relation.references(:foo).references(:foo) assert_equal ['foo'], relation.references_values end test 'merging a hash into a relation' do - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b relation = relation.merge where: :lol, readonly: true assert_equal [:lol], relation.where_values @@ -142,7 +142,7 @@ module ActiveRecord end test 'merging an empty hash into a relation' do - assert_equal [], Relation.new(:a, :b).merge({}).where_values + assert_equal [], Relation.new(FakeKlass, :b).merge({}).where_values end test 'merging a hash with unknown keys raises' do @@ -150,7 +150,7 @@ module ActiveRecord end test '#values returns a dup of the values' do - relation = Relation.new(:a, :b).where! :foo + relation = Relation.new(FakeKlass, :b).where! :foo values = relation.values values[:where] = nil @@ -158,18 +158,18 @@ module ActiveRecord end test 'relations can be created with a values hash' do - relation = Relation.new(:a, :b, where: [:foo]) + relation = Relation.new(FakeKlass, :b, where: [:foo]) assert_equal [:foo], relation.where_values end test 'merging a single where value' do - relation = Relation.new(:a, :b) + relation = Relation.new(FakeKlass, :b) relation.merge!(where: :foo) assert_equal [:foo], relation.where_values end test 'merging a hash interpolates conditions' do - klass = stub + klass = stub_everything klass.stubs(:sanitize_sql).with(['foo = ?', 'bar']).returns('foo = bar') relation = Relation.new(klass, :b) @@ -179,8 +179,11 @@ module ActiveRecord end class RelationMutationTest < ActiveSupport::TestCase + class FakeKlass < Struct.new(:table_name, :name) + end + def relation - @relation ||= Relation.new :a, :b + @relation ||= Relation.new FakeKlass, :b end (Relation::MULTI_VALUE_METHODS - [:references, :extending]).each do |method| diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index c34aeaf925..0cd838c0b0 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1452,4 +1452,33 @@ class RelationTest < ActiveRecord::TestCase assert_equal expected, result end end + + test "delegations do not leak to other classes" do + Topic.all.by_lifo + assert Topic.all.class.method_defined?(:by_lifo) + assert !Post.all.respond_to?(:by_lifo) + end + + class OMGTopic < ActiveRecord::Base + self.table_name = 'topics' + + def self.__omg__ + "omgtopic" + end + end + + test "delegations do not clash across classes" do + begin + class ::Array + def __omg__ + "array" + end + end + + assert_equal "array", Topic.all.__omg__ + assert_equal "omgtopic", OMGTopic.all.__omg__ + ensure + Array.send(:remove_method, :__omg__) + end + end end |