default create_with_value to a hash so we can eliminate conditionals,…

… add test surrounding create_with(nil) behavior
rails · Jun 27, 2011 · 997aed2 · 997aed2
1 parent 506b2da
commit 997aed2
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 5 deletions.
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
@@ -6,7 +6,7 @@ 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]
-    SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :create_with, :from, :reorder, :reverse_order]
+    SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :from, :reorder, :reverse_order]
 
     include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches
 
@@ -29,6 +29,7 @@ def initialize(klass, table)
       SINGLE_VALUE_METHODS.each {|v| instance_variable_set(:"@#{v}_value", nil)}
       (ASSOCIATION_METHODS + MULTI_VALUE_METHODS).each {|v| instance_variable_set(:"@#{v}_values", [])}
       @extensions = []
+      @create_with_value = {}
     end
 
     def insert(values)
@@ -403,7 +404,7 @@ def where_values_hash
     end
 
     def scope_for_create
-      @scope_for_create ||= where_values_hash.merge(@create_with_value || {})
+      @scope_for_create ||= where_values_hash.merge(create_with_value)
     end
 
     def eager_loading?

diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
@@ -137,7 +137,7 @@ def readonly(value = true)
 
     def create_with(value)
       relation = clone
-      relation.create_with_value = value && (@create_with_value || {}).merge(value)
+      relation.create_with_value = value ? create_with_value.merge(value) : {}
       relation
     end
 

diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb
@@ -55,7 +55,7 @@ def merge(r)
 
       merged_relation.lock_value = r.lock_value unless merged_relation.lock_value
 
-      merged_relation = merged_relation.create_with(r.create_with_value) if r.create_with_value
+      merged_relation = merged_relation.create_with(r.create_with_value) unless r.create_with_value.empty?
 
       # Apply scope extension modules
       merged_relation.send :apply_modules, r.extensions

diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb
@@ -472,6 +472,13 @@ def test_create_with_reset
     assert_equal 'Jamis', jamis.name
   end
 
+  # FIXME: I don't know if this is *desired* behavior, but it is *today's*
+  # behavior.
+  def test_create_with_empty_hash_will_not_reset
+    jamis = PoorDeveloperCalledJamis.create_with(:name => 'Aaron').create_with({}).new
+    assert_equal 'Aaron', jamis.name
+  end
+
   def test_unscoped_with_named_scope_should_not_have_default_scope
     assert_equal [DeveloperCalledJamis.find(developers(:poor_jamis).id)], DeveloperCalledJamis.poor
 

diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb
@@ -20,7 +20,7 @@ def test_construction
     end
 
     def test_single_values
-      assert_equal [:limit, :offset, :lock, :readonly, :create_with, :from, :reorder, :reverse_order].map(&:to_s).sort,
+      assert_equal [:limit, :offset, :lock, :readonly, :from, :reorder, :reverse_order].map(&:to_s).sort,
         Relation::SINGLE_VALUE_METHODS.map(&:to_s).sort
     end
 
@@ -103,6 +103,7 @@ def test_create_with_value
       relation = Relation.new Post, Post.arel_table
       hash = { :hello => 'world' }
       relation.create_with_value = hash
+      p relation.method(:create_with_value).source_location
       assert_equal hash, relation.scope_for_create
     end