diff options
7 files changed, 210 insertions, 136 deletions
diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 2972b7e13e..5a44d3a156 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -15,19 +15,20 @@ module ActiveRecord def scope scope = klass.unscoped - scope = scope.extending(*Array(options[:extend])) + + scope.extending!(*Array(options[:extend])) # 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, :references, :order, :limit, :joins, :group, :having, :offset, :select)) + scope.merge!(options.slice( + :readonly, :references, :order, :limit, :joins, :group, :having, :offset, :select, :uniq)) - if options[:through] && !options[:include] - scope = scope.includes(source_options[:include]) + if options[:include] + scope.includes! options[:include] + elsif options[:through] + scope.includes! source_options[:include] end - scope = scope.uniq if options[:uniq] - add_constraints(scope) end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index b007b8c168..4ed78bcbef 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -6,14 +6,20 @@ module ActiveRecord # = Active Record Relation class Relation JoinOperation = Struct.new(:relation, :join_class, :on) - ASSOCIATION_METHODS = [:includes, :eager_load, :preload] - MULTI_VALUE_METHODS = [:select, :group, :order, :joins, :where, :having, :bind, :references] - SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :from, :reordering, :reverse_order, :uniq] + + MULTI_VALUE_METHODS = [:includes, :eager_load, :preload, :select, :group, + :order, :joins, :where, :having, :bind, :references, + :extending] + + SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :from, :reordering, + :reverse_order, :uniq, :create_with] + + VALUE_METHODS = MULTI_VALUE_METHODS + SINGLE_VALUE_METHODS include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches, Explain, Delegation attr_reader :table, :klass, :loaded - attr_accessor :extensions, :default_scoped + attr_accessor :default_scoped alias :loaded? :loaded alias :default_scoped? :default_scoped @@ -25,8 +31,8 @@ module ActiveRecord @default_scoped = false SINGLE_VALUE_METHODS.each {|v| instance_variable_set(:"@#{v}_value", nil)} - (ASSOCIATION_METHODS + MULTI_VALUE_METHODS).each {|v| instance_variable_set(:"@#{v}_values", [])} - @extensions = [] + MULTI_VALUE_METHODS.each {|v| instance_variable_set(:"@#{v}_values", [])} + @create_with_value = {} end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 74f8e30404..52061a2286 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -6,7 +6,8 @@ module ActiveRecord # Find operates with four different retrieval approaches: # # * Find by id - This can either be a specific id (1), a list of ids (1, 5, 6), or an array of ids ([5, 6, 10]). - # If no record can be found for all of the listed ids, then RecordNotFound will be raised. + # If no record can be found for all of the listed ids, then RecordNotFound will be raised. If the primary key + # is an integer, find by id coerces its arguments using +to_i+. # * Find first - This will return the first record matched by the options used. These options can either be specific # conditions or merely an order. If no record can be matched, +nil+ is returned. Use # <tt>Model.find(:first, *args)</tt> or its shortcut <tt>Model.first(*args)</tt>. @@ -51,6 +52,7 @@ module ActiveRecord # # # find by id # Person.find(1) # returns the object for ID = 1 + # Person.find("1") # returns the object for ID = 1 # Person.find(1, 2, 6) # returns an array for objects with IDs in (1, 2, 6) # Person.find([7, 17]) # returns an array for objects with IDs in (7, 17) # Person.find([1]) # returns an array for the object with ID = 1 diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb new file mode 100644 index 0000000000..0a5e9ef171 --- /dev/null +++ b/activerecord/lib/active_record/relation/merger.rb @@ -0,0 +1,121 @@ +require 'active_support/core_ext/object/blank' +require 'active_support/core_ext/hash/keys' + +module ActiveRecord + class Relation + class Merger + attr_reader :relation, :other + + def initialize(relation, other) + @relation = relation + + if other.default_scoped? && other.klass != relation.klass + @other = other.with_default_scope + else + @other = other + end + end + + def merge + HashMerger.new(relation, other_hash).merge + end + + private + + def other_hash + hash = {} + Relation::MULTI_VALUE_METHODS.map { |name| hash[name] = other.send("#{name}_values") } + Relation::SINGLE_VALUE_METHODS.map { |name| hash[name] = other.send("#{name}_value") } + hash + end + end + + class HashMerger + attr_reader :relation, :values + + def initialize(relation, values) + values.assert_valid_keys(*Relation::VALUE_METHODS) + + @relation = relation + @values = values + end + + def normal_values + Relation::SINGLE_VALUE_METHODS + + Relation::MULTI_VALUE_METHODS - + [:where, :order, :bind, :reverse_order, :lock, :create_with, :reordering] + end + + def merge + normal_values.each do |name| + value = values[name] + relation.send("#{name}!", value) unless value.blank? + end + + merge_multi_values + merge_single_values + + relation + end + + private + + def merge_multi_values + relation.where_values = merged_wheres + relation.bind_values = merged_binds + + if values[:reordering] + # override any order specified in the original relation + relation.reorder! values[:order] + elsif values[:order] + # merge in order_values from r + relation.order! values[:order] + end + + relation.extend(*values[:extending]) unless values[:extending].blank? + end + + def merge_single_values + relation.lock_value = values[:lock] unless relation.lock_value + relation.reverse_order_value = values[:reverse_order] + + unless values[:create_with].blank? + relation.create_with_value = (relation.create_with_value || {}).merge(values[:create_with]) + end + end + + def merged_binds + if values[:bind] + (relation.bind_values + values[:bind]).uniq(&:first) + else + relation.bind_values + end + end + + def merged_wheres + if values[:where] + merged_wheres = relation.where_values + values[:where] + + unless relation.where_values.empty? + # Remove duplicates, last one wins. + seen = Hash.new { |h,table| h[table] = {} } + merged_wheres = merged_wheres.reverse.reject { |w| + nuke = false + if w.respond_to?(:operator) && w.operator == :== + name = w.left.name + table = w.left.relation.name + nuke = seen[table][name] + seen[table][name] = true + end + nuke + }.reverse + end + + merged_wheres + else + relation.where_values + end + end + end + end +end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 10492165fb..6b7e03f9f7 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -10,7 +10,9 @@ module ActiveRecord :where_values, :having_values, :bind_values, :limit_value, :offset_value, :lock_value, :readonly_value, :create_with_value, :from_value, :reordering_value, :reverse_order_value, - :uniq_value, :references_values + :uniq_value, :references_values, :extending_values + + alias extensions extending_values def includes(*args) args.empty? ? self : clone.includes!(*args) @@ -97,14 +99,9 @@ module ActiveRecord end end - def select!(value = Proc.new) - if block_given? - # TODO: test - to_a.select! { |*block_args| value.call(*block_args) } - else - self.select_values += Array.wrap(value) - self - end + def select!(value) + self.select_values += Array.wrap(value) + self end def group(*args) @@ -353,7 +350,9 @@ module ActiveRecord def extending!(*modules, &block) modules << Module.new(&block) if block_given? - self.send(:apply_modules, modules.flatten) + self.extending_values = modules.flatten + extend(*extending_values) if extending_values.any? + self end @@ -494,13 +493,6 @@ module ActiveRecord end end - def apply_modules(modules) - unless modules.empty? - @extensions += modules - modules.each {|extension| extend(extension) } - end - end - def reverse_sql_order(order_query) order_query = ["#{quoted_table_name}.#{quoted_primary_key} ASC"] if order_query.empty? diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 19ec41e5ca..a365b5723b 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -1,81 +1,24 @@ require 'active_support/core_ext/object/blank' +require 'active_record/relation/merger' module ActiveRecord module SpawnMethods - def merge(r) - return self unless r - return to_a & r if r.is_a?(Array) - - merged_relation = clone - - r = r.with_default_scope if r.default_scoped? && r.klass != klass - - Relation::ASSOCIATION_METHODS.each do |method| - value = r.send(:"#{method}_values") - - unless value.empty? - if method == :includes - merged_relation = merged_relation.includes(value) - else - merged_relation.send(:"#{method}_values=", value) - end - end - end - - (Relation::MULTI_VALUE_METHODS - [:joins, :where, :order, :binds]).each do |method| - value = r.send(:"#{method}_values") - next if value.empty? - - value += merged_relation.send(:"#{method}_values") - merged_relation.send :"#{method}_values=", value - end - - merged_relation.joins_values += r.joins_values - - merged_wheres = @where_values + r.where_values - - merged_binds = (@bind_values + r.bind_values).uniq(&:first) - - unless @where_values.empty? - # Remove duplicates, last one wins. - seen = Hash.new { |h,table| h[table] = {} } - merged_wheres = merged_wheres.reverse.reject { |w| - nuke = false - if w.respond_to?(:operator) && w.operator == :== - name = w.left.name - table = w.left.relation.name - nuke = seen[table][name] - seen[table][name] = true - end - nuke - }.reverse - end - - merged_relation.where_values = merged_wheres - merged_relation.bind_values = merged_binds - - (Relation::SINGLE_VALUE_METHODS - [:lock, :create_with, :reordering]).each do |method| - value = r.send(:"#{method}_value") - merged_relation.send(:"#{method}_value=", value) unless value.nil? + def merge(other) + if other.is_a?(Array) + to_a & other + elsif other + clone.merge!(other) + else + self end + end - merged_relation.lock_value = r.lock_value unless merged_relation.lock_value - - merged_relation = merged_relation.create_with(r.create_with_value) unless r.create_with_value.empty? - - if (r.reordering_value) - # override any order specified in the original relation - merged_relation.reordering_value = true - merged_relation.order_values = r.order_values + def merge!(other) + if other.is_a?(Hash) + Relation::HashMerger.new(self, other).merge else - # merge in order_values from r - merged_relation.order_values += r.order_values + Relation::Merger.new(self, other).merge end - - # Apply scope extension modules - merged_relation.send :apply_modules, r.extensions - - merged_relation end # Removes from the query the condition(s) specified in +skips+. @@ -89,7 +32,7 @@ module ActiveRecord result = self.class.new(@klass, table) result.default_scoped = default_scoped - ((Relation::ASSOCIATION_METHODS + Relation::MULTI_VALUE_METHODS) - skips).each do |method| + (Relation::MULTI_VALUE_METHODS - skips).each do |method| result.send(:"#{method}_values=", send(:"#{method}_values")) end @@ -97,8 +40,7 @@ module ActiveRecord result.send(:"#{method}_value=", send(:"#{method}_value")) end - # Apply scope extension modules - result.send(:apply_modules, extensions) + result.extend(*extending_values) if extending_values.any? result end @@ -114,7 +56,7 @@ module ActiveRecord result = self.class.new(@klass, table) result.default_scoped = default_scoped - ((Relation::ASSOCIATION_METHODS + Relation::MULTI_VALUE_METHODS) & onlies).each do |method| + (Relation::MULTI_VALUE_METHODS & onlies).each do |method| result.send(:"#{method}_values=", send(:"#{method}_values")) end @@ -122,8 +64,7 @@ module ActiveRecord result.send(:"#{method}_value=", send(:"#{method}_value")) end - # Apply scope extension modules - result.send(:apply_modules, extensions) + result.extend(*extending_values) if extending_values.any? result end diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 4b18c37d27..633654a961 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -19,33 +19,12 @@ module ActiveRecord assert !relation.loaded, 'relation is not loaded' end - def test_single_values - assert_equal [:limit, :offset, :lock, :readonly, :from, :reordering, :reverse_order, :uniq].map(&:to_s).sort, - Relation::SINGLE_VALUE_METHODS.map(&:to_s).sort - end - def test_initialize_single_values relation = Relation.new :a, :b - Relation::SINGLE_VALUE_METHODS.each do |method| + (Relation::SINGLE_VALUE_METHODS - [:create_with]).each do |method| assert_nil relation.send("#{method}_value"), method.to_s end - end - - def test_association_methods - assert_equal [:includes, :eager_load, :preload].map(&:to_s).sort, - Relation::ASSOCIATION_METHODS.map(&:to_s).sort - end - - def test_initialize_association_methods - relation = Relation.new :a, :b - Relation::ASSOCIATION_METHODS.each do |method| - assert_equal [], relation.send("#{method}_values"), method.to_s - end - end - - def test_multi_value_methods - assert_equal [:select, :group, :order, :joins, :where, :having, :bind, :references].map(&:to_s).sort, - Relation::MULTI_VALUE_METHODS.map(&:to_s).sort + assert_equal({}, relation.create_with_value) end def test_multi_value_initialize @@ -154,6 +133,22 @@ module ActiveRecord relation = relation.apply_finder_options(:references => :foo) assert_equal ['foo'], relation.references_values end + + test 'merging a hash into a relation' do + relation = Relation.new :a, :b + relation = relation.merge where: ['lol'], readonly: true + + assert_equal ['lol'], relation.where_values + assert_equal true, relation.readonly_value + end + + test 'merging an empty hash into a relation' do + assert_equal [], Relation.new(:a, :b).merge({}).where_values + end + + test 'merging a hash with unknown keys raises' do + assert_raises(ArgumentError) { Relation::HashMerger.new(nil, omg: 'lol') } + end end class RelationMutationTest < ActiveSupport::TestCase @@ -161,7 +156,7 @@ module ActiveRecord @relation ||= Relation.new :a, :b end - (Relation::ASSOCIATION_METHODS + Relation::MULTI_VALUE_METHODS - [:references]).each do |method| + (Relation::MULTI_VALUE_METHODS - [:references, :extending]).each do |method| test "##{method}!" do assert relation.public_send("#{method}!", :foo).equal?(relation) assert_equal [:foo], relation.public_send("#{method}_values") @@ -173,7 +168,20 @@ module ActiveRecord assert relation.references_values.include?('foo') end - (Relation::SINGLE_VALUE_METHODS - [:lock, :reordering, :reverse_order]).each do |method| + test 'extending!' do + mod = Module.new + + assert relation.extending!(mod).equal?(relation) + assert [mod], relation.extending_values + assert relation.is_a?(mod) + end + + test 'extending! with empty args' do + relation.extending! + assert_equal [], relation.extending_values + end + + (Relation::SINGLE_VALUE_METHODS - [:lock, :reordering, :reverse_order, :create_with]).each do |method| test "##{method}!" do assert relation.public_send("#{method}!", :foo).equal?(relation) assert_equal :foo, relation.public_send("#{method}_value") @@ -200,11 +208,14 @@ module ActiveRecord assert !relation.reverse_order_value end - test 'extending!' do - mod = Module.new + test 'create_with!' do + assert relation.create_with!(foo: 'bar').equal?(relation) + assert_equal({foo: 'bar'}, relation.create_with_value) + end - assert relation.extending!(mod).equal?(relation) - assert relation.is_a?(mod) + test 'merge!' do + assert relation.merge!(where: ['foo']).equal?(relation) + assert_equal ['foo'], relation.where_values end end end |