From 5b42628e79e85b8697a42f5f566f7cbfd1160847 Mon Sep 17 00:00:00 2001 From: Bogdan Gusiev Date: Tue, 16 Aug 2016 14:47:11 +0300 Subject: Remove over meta programming in AR::Relation Introduced low level methods #set_value and #get_value for setting query attributes: relation.set_value(:where, {id: 1}) relation.get_value(:includes) Used those internally when working with relation's attributes at the abstract level --- activerecord/lib/active_record/relation.rb | 13 +-- activerecord/lib/active_record/relation/merger.rb | 12 +- .../lib/active_record/relation/query_methods.rb | 125 ++++++++------------- activerecord/test/cases/relation_test.rb | 3 +- 4 files changed, 59 insertions(+), 94 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 3983065d7a..85b1ddf8db 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -504,15 +504,10 @@ module ActiveRecord # Post.limit(100).delete_all # # => ActiveRecord::ActiveRecordError: delete_all doesn't support limit def delete_all(conditions = nil) - invalid_methods = INVALID_METHODS_FOR_DELETE_ALL.select { |method| - if MULTI_VALUE_METHODS.include?(method) - send("#{method}_values").any? - elsif SINGLE_VALUE_METHODS.include?(method) - send("#{method}_value") - elsif CLAUSE_METHODS.include?(method) - send("#{method}_clause").any? - end - } + invalid_methods = INVALID_METHODS_FOR_DELETE_ALL.select do |method| + value = get_value(method) + SINGLE_VALUE_METHODS.include?(method) ? value : value.any? + end if invalid_methods.any? raise ActiveRecordError.new("delete_all doesn't support #{invalid_methods.join(', ')}") end diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index df43b5c64b..5dac00724a 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -151,15 +151,11 @@ module ActiveRecord end end - CLAUSE_METHOD_NAMES = CLAUSE_METHODS.map do |name| - ["#{name}_clause", "#{name}_clause="] - end - def merge_clauses - CLAUSE_METHOD_NAMES.each do |(reader, writer)| - clause = relation.send(reader) - other_clause = other.send(reader) - relation.send(writer, clause.merge(other_clause)) + CLAUSE_METHODS.each do |method| + clause = relation.get_value(method) + other_clause = other.get_value(method) + relation.set_value(method, clause.merge(other_clause)) end end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 1a59b3a146..4ee490274b 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -56,49 +56,25 @@ module ActiveRecord end FROZEN_EMPTY_ARRAY = [].freeze - Relation::MULTI_VALUE_METHODS.each do |name| - class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{name}_values - @values[:#{name}] || FROZEN_EMPTY_ARRAY - end - - def #{name}_values=(values) - assert_mutability! - @values[:#{name}] = values - end - CODE - end + FROZEN_EMPTY_HASH = {}.freeze - (Relation::SINGLE_VALUE_METHODS - [:create_with]).each do |name| + Relation::VALUE_METHODS.each do |name| + method_name = case name + when *Relation::MULTI_VALUE_METHODS then "#{name}_values" + when *Relation::SINGLE_VALUE_METHODS then "#{name}_value" + when *Relation::CLAUSE_METHODS then "#{name}_clause" + end class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{name}_value # def readonly_value - @values[:#{name}] # @values[:readonly] + def #{method_name} # def includes_values + get_value(#{name.inspect}) # get_value(:includes) end # end - CODE - end - Relation::SINGLE_VALUE_METHODS.each do |name| - class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{name}_value=(value) # def readonly_value=(value) - assert_mutability! # assert_mutability! - @values[:#{name}] = value # @values[:readonly] = value + def #{method_name}=(value) # def includes_values=(value) + set_value(#{name.inspect}, value) # set_value(:includes, value) end # end CODE end - Relation::CLAUSE_METHODS.each do |name| - class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{name}_clause # def where_clause - @values[:#{name}] || new_#{name}_clause # @values[:where] || new_where_clause - end # end - # - def #{name}_clause=(value) # def where_clause=(value) - assert_mutability! # assert_mutability! - @values[:#{name}] = value # @values[:where] = value - end # end - CODE - end - def bound_attributes if limit_value && !string_containing_comma?(limit_value) limit_bind = Attribute.with_cast_value( @@ -124,11 +100,6 @@ module ActiveRecord ) end - FROZEN_EMPTY_HASH = {}.freeze - def create_with_value # :nodoc: - @values[:create_with] || FROZEN_EMPTY_HASH - end - alias extensions extending_values # Specify relationships to be included in the result set. For @@ -418,7 +389,10 @@ module ActiveRecord args.each do |scope| case scope when Symbol - symbol_unscoping(scope) + if !VALID_UNSCOPING_VALUES.include?(scope) + raise ArgumentError, "Called unscope() with invalid unscoping argument ':#{scope}'. Valid arguments are :#{VALID_UNSCOPING_VALUES.to_a.join(", :")}." + end + set_value(scope, nil) when Hash scope.each do |key, target_value| if key != :where @@ -950,6 +924,17 @@ module ActiveRecord @arel ||= build_arel end + # Returns a relation value with a given name + def get_value(name) # :nodoc: + @values[name] || default_value_for(name) + end + + # Sets the relation value with the given name + def set_value(name, value) # :nodoc: + assert_mutability! + @values[name] = value + end + private def assert_mutability! @@ -986,29 +971,6 @@ module ActiveRecord arel end - def symbol_unscoping(scope) - if !VALID_UNSCOPING_VALUES.include?(scope) - raise ArgumentError, "Called unscope() with invalid unscoping argument ':#{scope}'. Valid arguments are :#{VALID_UNSCOPING_VALUES.to_a.join(", :")}." - end - - clause_method = Relation::CLAUSE_METHODS.include?(scope) - multi_val_method = Relation::MULTI_VALUE_METHODS.include?(scope) - if clause_method - unscope_code = "#{scope}_clause=" - else - unscope_code = "#{scope}_value#{'s' if multi_val_method}=" - end - - case scope - when :order - result = [] - else - result = [] if multi_val_method - end - - send(unscope_code, result) - end - def build_from opts = from_clause.value name = from_clause.name @@ -1210,28 +1172,39 @@ module ActiveRecord end end + STRUCTURAL_OR_METHODS = Relation::VALUE_METHODS - [:extending, :where, :having] def structurally_incompatible_values_for_or(other) - Relation::SINGLE_VALUE_METHODS.reject { |m| send("#{m}_value") == other.send("#{m}_value") } + - (Relation::MULTI_VALUE_METHODS - [:extending]).reject { |m| send("#{m}_values") == other.send("#{m}_values") } + - (Relation::CLAUSE_METHODS - [:having, :where]).reject { |m| send("#{m}_clause") == other.send("#{m}_clause") } - end - - def new_where_clause - Relation::WhereClause.empty + STRUCTURAL_OR_METHODS.reject do |method| + get_value(method) == other.get_value(method) + end end - alias new_having_clause new_where_clause def where_clause_factory @where_clause_factory ||= Relation::WhereClauseFactory.new(klass, predicate_builder) end alias having_clause_factory where_clause_factory - def new_from_clause - Relation::FromClause.empty - end - def string_containing_comma?(value) ::String === value && value.include?(",") end + + def default_value_for(name) + case name + when :create_with + FROZEN_EMPTY_HASH + when :readonly + false + when :where, :having + Relation::WhereClause.empty + when :from + Relation::FromClause.empty + when *Relation::MULTI_VALUE_METHODS + FROZEN_EMPTY_ARRAY + when *Relation::SINGLE_VALUE_METHODS + nil + else + raise ArgumentError, "unknown relation value #{name.inspect}" + end + end end end diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 951b83e87b..0bd4ee3e36 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -40,9 +40,10 @@ module ActiveRecord def test_initialize_single_values relation = Relation.new(FakeKlass, :b, nil) - (Relation::SINGLE_VALUE_METHODS - [:create_with]).each do |method| + (Relation::SINGLE_VALUE_METHODS - [:create_with, :readonly]).each do |method| assert_nil relation.send("#{method}_value"), method.to_s end + assert_equal false, relation.readonly_value value = relation.create_with_value assert_equal({}, value) assert_predicate value, :frozen? -- cgit v1.2.3