Merge pull request #26182 from bogdan/remove-relation-metaprogramming

Remove over meta programming in AR::Relation

4 files changed, 59 insertions, 94 deletions

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 9a1ac207f2..bd41653df0 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 103075d09c..32b8781a6b 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?