aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2017-07-16 21:31:08 +0900
committerRyuta Kamizono <kamipo@gmail.com>2017-07-16 21:31:08 +0900
commitd476553d1cdeee0805585c2d4e2e6ee6be841288 (patch)
tree0f358d6cd1d751d676f8513a04e7b53777f080aa /activerecord
parent16f2b2044eaaa54b7bc205ef9af1689a152b2fdf (diff)
downloadrails-d476553d1cdeee0805585c2d4e2e6ee6be841288.tar.gz
rails-d476553d1cdeee0805585c2d4e2e6ee6be841288.tar.bz2
rails-d476553d1cdeee0805585c2d4e2e6ee6be841288.zip
Don't cache `scope_for_create`
I investigated where `scope_for_create` is reused in tests with the following code: ```diff --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -590,6 +590,10 @@ def where_values_hash(relation_table_name = table_name) end def scope_for_create + if defined?(@scope_for_create) && @scope_for_create + puts caller + puts "defined" + end @scope_for_create ||= where_values_hash.merge!(create_with_value.stringify_keys) end ``` It was hit only `test_scope_for_create_is_cached`. This means that `scope_for_create` will not be reused in normal use cases. So we can remove caching `scope_for_create` to respect changing `where_clause` and `create_with_value`.
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/lib/active_record/associations/association.rb2
-rw-r--r--activerecord/lib/active_record/associations/singular_association.rb2
-rw-r--r--activerecord/lib/active_record/relation.rb4
-rw-r--r--activerecord/test/cases/relation_test.rb12
4 files changed, 6 insertions, 14 deletions
diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb
index 6acf831759..c166cfd68f 100644
--- a/activerecord/lib/active_record/associations/association.rb
+++ b/activerecord/lib/active_record/associations/association.rb
@@ -171,7 +171,7 @@ module ActiveRecord
skip_assign = [reflection.foreign_key, reflection.type].compact
assigned_keys = record.changed_attribute_names_to_save
assigned_keys += except_from_scope_attributes.keys.map(&:to_s)
- attributes = scope_for_create.except(*(assigned_keys - skip_assign))
+ attributes = scope_for_create.except!(*(assigned_keys - skip_assign))
record.send(:_assign_attributes, attributes) if attributes.any?
set_inverse_instance(record)
end
diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb
index 4896afcca7..66993b9bf7 100644
--- a/activerecord/lib/active_record/associations/singular_association.rb
+++ b/activerecord/lib/active_record/associations/singular_association.rb
@@ -31,7 +31,7 @@ module ActiveRecord
private
def scope_for_create
- super.except(klass.primary_key)
+ super.except!(klass.primary_key)
end
def find_target
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 23cb1baaed..c5d98e970a 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -556,7 +556,7 @@ module ActiveRecord
end
def reset
- @to_sql = @scope_for_create = @arel = @loaded = @should_eager_load = nil
+ @to_sql = @arel = @loaded = @should_eager_load = nil
@records = [].freeze
@offsets = {}
self
@@ -590,7 +590,7 @@ module ActiveRecord
end
def scope_for_create
- @scope_for_create ||= where_values_hash.merge!(create_with_value.stringify_keys)
+ where_values_hash.merge!(create_with_value.stringify_keys)
end
# Returns true if relation needs eager loading.
diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb
index d2859cbafd..82b071fb98 100644
--- a/activerecord/test/cases/relation_test.rb
+++ b/activerecord/test/cases/relation_test.rb
@@ -89,21 +89,13 @@ module ActiveRecord
def test_create_with_value_with_wheres
relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
- relation.where!(id: 10)
- relation.create_with_value = { hello: "world" }
- assert_equal({ "hello" => "world", "id" => 10 }, relation.scope_for_create)
- end
-
- # FIXME: is this really wanted or expected behavior?
- def test_scope_for_create_is_cached
- relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
assert_equal({}, relation.scope_for_create)
relation.where!(id: 10)
- assert_equal({}, relation.scope_for_create)
+ assert_equal({ "id" => 10 }, relation.scope_for_create)
relation.create_with_value = { hello: "world" }
- assert_equal({}, relation.scope_for_create)
+ assert_equal({ "hello" => "world", "id" => 10 }, relation.scope_for_create)
end
def test_bad_constants_raise_errors