now we can just manipulate the values hash in #only and #except

4 files changed, 26 insertions, 36 deletions

diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 7918d08cf0..8d6ed4c6d1 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -23,13 +23,13 @@ module ActiveRecord
     alias :loaded? :loaded
     alias :default_scoped? :default_scoped
 
-    def initialize(klass, table)
-      @klass, @table = klass, table
-
+    def initialize(klass, table, values = {})
+      @klass             = klass
+      @table             = table
+      @values            = values
       @implicit_readonly = nil
       @loaded            = false
       @default_scoped    = false
-      @values            = {}
     end
 
     def insert(values)
@@ -522,6 +522,10 @@ module ActiveRecord
       to_a.blank?
     end
 
+    def values
+      @values.dup
+    end
+
     private
 
     def references_eager_loaded_tables?
diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb
index 0a5e9ef171..1c2a06328f 100644
--- a/activerecord/lib/active_record/relation/merger.rb
+++ b/activerecord/lib/active_record/relation/merger.rb
@@ -17,16 +17,7 @@ module ActiveRecord
       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
+        HashMerger.new(relation, other.values).merge
       end
     end
 
diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb
index a365b5723b..7bf9c16959 100644
--- a/activerecord/lib/active_record/relation/spawn_methods.rb
+++ b/activerecord/lib/active_record/relation/spawn_methods.rb
@@ -1,4 +1,6 @@
 require 'active_support/core_ext/object/blank'
+require 'active_support/core_ext/hash/except'
+require 'active_support/core_ext/hash/slice'
 require 'active_record/relation/merger'
 
 module ActiveRecord
@@ -29,19 +31,9 @@ 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)
+      result = self.class.new(@klass, table, values.except(*skips))
       result.default_scoped = default_scoped
-
-      (Relation::MULTI_VALUE_METHODS - skips).each do |method|
-        result.send(:"#{method}_values=", send(:"#{method}_values"))
-      end
-
-      (Relation::SINGLE_VALUE_METHODS - skips).each do |method|
-        result.send(:"#{method}_value=", send(:"#{method}_value"))
-      end
-
       result.extend(*extending_values) if extending_values.any?
-
       result
     end
 
@@ -53,19 +45,9 @@ module ActiveRecord
     #   Post.order('id asc').only(:where, :order) # uses the specified order
     #
     def only(*onlies)
-      result = self.class.new(@klass, table)
+      result = self.class.new(@klass, table, values.slice(*onlies))
       result.default_scoped = default_scoped
-
-      (Relation::MULTI_VALUE_METHODS & onlies).each do |method|
-        result.send(:"#{method}_values=", send(:"#{method}_values"))
-      end
-
-      (Relation::SINGLE_VALUE_METHODS & onlies).each do |method|
-        result.send(:"#{method}_value=", send(:"#{method}_value"))
-      end
-
       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 c636284b8f..719b96fa0f 100644
--- a/activerecord/test/cases/relation_test.rb
+++ b/activerecord/test/cases/relation_test.rb
@@ -149,6 +149,19 @@ module ActiveRecord
     test 'merging a hash with unknown keys raises' do
       assert_raises(ArgumentError) { Relation::HashMerger.new(nil, omg: 'lol') }
     end
+
+    test '#values returns a dup of the values' do
+      relation = Relation.new(:a, :b).where! :foo
+      values   = relation.values
+
+      values[:where] = nil
+      assert_not_nil relation.where_values
+    end
+
+    test 'relations can be created with a values hash' do
+      relation = Relation.new(:a, :b, where: [:foo])
+      assert_equal [:foo], relation.where_values
+    end
   end
 
   class RelationMutationTest < ActiveSupport::TestCase