mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
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`.
This commit is contained in:
parent
16f2b2044e
commit
d476553d1c
4 changed files with 8 additions and 16 deletions
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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.
|
||||
|
|
|
@ -89,23 +89,15 @@ module ActiveRecord
|
|||
|
||||
def test_create_with_value_with_wheres
|
||||
relation = Relation.new(Post, Post.arel_table, Post.predicate_builder)
|
||||
assert_equal({}, relation.scope_for_create)
|
||||
|
||||
relation.where!(id: 10)
|
||||
assert_equal({ "id" => 10 }, relation.scope_for_create)
|
||||
|
||||
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)
|
||||
|
||||
relation.create_with_value = { hello: "world" }
|
||||
assert_equal({}, relation.scope_for_create)
|
||||
end
|
||||
|
||||
def test_bad_constants_raise_errors
|
||||
assert_raises(NameError) do
|
||||
ActiveRecord::Relation::HelloWorld
|
||||
|
|
Loading…
Reference in a new issue