aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2019-02-14 20:46:51 +0900
committerRyuta Kamizono <kamipo@gmail.com>2019-02-15 12:06:45 +0900
commitcdb8697b4a654aad2e65590d58c5f581a53d6b33 (patch)
tree8ac091bcf825ff2bc8dbdca92a0e612fb930a847
parent79b8b626216b4f8f24b5606f3edd43d5b14cd2a6 (diff)
downloadrails-cdb8697b4a654aad2e65590d58c5f581a53d6b33.tar.gz
rails-cdb8697b4a654aad2e65590d58c5f581a53d6b33.tar.bz2
rails-cdb8697b4a654aad2e65590d58c5f581a53d6b33.zip
Revert "Merge pull request #35186 from kamipo/fix_leaking_scope_on_relation_create"
This reverts commit b67d5c6dedbf033515a96a95d24d085bf99a0d07, reversing changes made to 2e018361c7c51e36d1d98bf770b7456d78dee68b. Reason: #35186 may cause that silently leaking information when people upgrade the app. We need deprecation first before making this.
-rw-r--r--activerecord/CHANGELOG.md6
-rw-r--r--activerecord/lib/active_record/relation.rb13
-rw-r--r--activerecord/lib/active_record/scoping.rb8
-rw-r--r--activerecord/test/cases/base_test.rb2
-rw-r--r--activerecord/test/cases/relations_test.rb42
-rw-r--r--activerecord/test/models/bird.rb4
6 files changed, 38 insertions, 37 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 47ae71f2a0..63205368c7 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,9 +1,3 @@
-* Fix `relation.create` to avoid leaking scope to initialization block and callbacks.
-
- Fixes #9894, #17577.
-
- *Ryuta Kamizono*
-
* Allow applications to automatically switch connections.
Adds a middleware and configuration options that can be used in your
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index f98d9bb2c0..ecca833847 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -67,7 +67,6 @@ module ActiveRecord
# user = users.new { |user| user.name = 'Oscar' }
# user.name # => Oscar
def new(attributes = nil, &block)
- block = klass.current_scope_restoring_block(&block)
scoping { klass.new(attributes, &block) }
end
@@ -93,11 +92,7 @@ module ActiveRecord
# users.create(name: nil) # validation on name
# # => #<User id: nil, name: nil, ...>
def create(attributes = nil, &block)
- if attributes.is_a?(Array)
- attributes.collect { |attr| create(attr, &block) }
- else
- new(attributes, &block).tap(&:save)
- end
+ scoping { klass.create(attributes, &block) }
end
# Similar to #create, but calls
@@ -107,11 +102,7 @@ module ActiveRecord
# Expects arguments in the same format as
# {ActiveRecord::Base.create!}[rdoc-ref:Persistence::ClassMethods#create!].
def create!(attributes = nil, &block)
- if attributes.is_a?(Array)
- attributes.collect { |attr| create!(attr, &block) }
- else
- new(attributes, &block).tap(&:save!)
- end
+ scoping { klass.create!(attributes, &block) }
end
def first_or_create(attributes = nil, &block) # :nodoc:
diff --git a/activerecord/lib/active_record/scoping.rb b/activerecord/lib/active_record/scoping.rb
index 1142a87d25..35e9dcbffc 100644
--- a/activerecord/lib/active_record/scoping.rb
+++ b/activerecord/lib/active_record/scoping.rb
@@ -30,14 +30,6 @@ module ActiveRecord
def current_scope=(scope)
ScopeRegistry.set_value_for(:current_scope, self, scope)
end
-
- def current_scope_restoring_block(&block)
- current_scope = self.current_scope(true)
- -> *args do
- self.current_scope = current_scope
- yield(*args) if block_given?
- end
- end
end
def populate_with_current_scope_attributes # :nodoc:
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 1b8f748bad..d560b4e519 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -1536,7 +1536,7 @@ class BasicsTest < ActiveRecord::TestCase
Bird.create!(name: "Bluejay")
ActiveRecord::Base.connection.while_preventing_writes do
- assert_nothing_raised { Bird.where(name: "Bluejay").explain }
+ assert_queries(2) { Bird.where(name: "Bluejay").explain }
end
end
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 857d743605..98ba47f1b8 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -1167,13 +1167,21 @@ class RelationTest < ActiveRecord::TestCase
assert_equal "green", parrot.color
end
+ def test_first_or_create_with_after_initialize
+ Bird.create!(color: "yellow", name: "canary")
+ parrot = Bird.where(color: "green").first_or_create do |bird|
+ bird.name = "parrot"
+ bird.enable_count = true
+ end
+ assert_equal 0, parrot.total_count
+ end
+
def test_first_or_create_with_block
- canary = Bird.create!(color: "yellow", name: "canary")
+ Bird.create!(color: "yellow", name: "canary")
parrot = Bird.where(color: "green").first_or_create do |bird|
bird.name = "parrot"
- assert_equal canary, Bird.find_by!(name: "canary")
+ assert_equal 0, Bird.count
end
- assert_equal 1, parrot.total_count
assert_kind_of Bird, parrot
assert_predicate parrot, :persisted?
assert_equal "green", parrot.color
@@ -1214,13 +1222,21 @@ class RelationTest < ActiveRecord::TestCase
assert_raises(ActiveRecord::RecordInvalid) { Bird.where(color: "green").first_or_create! }
end
+ def test_first_or_create_bang_with_after_initialize
+ Bird.create!(color: "yellow", name: "canary")
+ parrot = Bird.where(color: "green").first_or_create! do |bird|
+ bird.name = "parrot"
+ bird.enable_count = true
+ end
+ assert_equal 0, parrot.total_count
+ end
+
def test_first_or_create_bang_with_valid_block
- canary = Bird.create!(color: "yellow", name: "canary")
+ Bird.create!(color: "yellow", name: "canary")
parrot = Bird.where(color: "green").first_or_create! do |bird|
bird.name = "parrot"
- assert_equal canary, Bird.find_by!(name: "canary")
+ assert_equal 0, Bird.count
end
- assert_equal 1, parrot.total_count
assert_kind_of Bird, parrot
assert_predicate parrot, :persisted?
assert_equal "green", parrot.color
@@ -1269,13 +1285,21 @@ class RelationTest < ActiveRecord::TestCase
assert_equal "green", parrot.color
end
+ def test_first_or_initialize_with_after_initialize
+ Bird.create!(color: "yellow", name: "canary")
+ parrot = Bird.where(color: "green").first_or_initialize do |bird|
+ bird.name = "parrot"
+ bird.enable_count = true
+ end
+ assert_equal 0, parrot.total_count
+ end
+
def test_first_or_initialize_with_block
- canary = Bird.create!(color: "yellow", name: "canary")
+ Bird.create!(color: "yellow", name: "canary")
parrot = Bird.where(color: "green").first_or_initialize do |bird|
bird.name = "parrot"
- assert_equal canary, Bird.find_by!(name: "canary")
+ assert_equal 0, Bird.count
end
- assert_equal 1, parrot.total_count
assert_kind_of Bird, parrot
assert_not_predicate parrot, :persisted?
assert_predicate parrot, :valid?
diff --git a/activerecord/test/models/bird.rb b/activerecord/test/models/bird.rb
index c9f6759c7d..20af7c6122 100644
--- a/activerecord/test/models/bird.rb
+++ b/activerecord/test/models/bird.rb
@@ -17,8 +17,8 @@ class Bird < ActiveRecord::Base
throw(:abort)
end
- attr_accessor :total_count
+ attr_accessor :total_count, :enable_count
after_initialize do
- self.total_count = Bird.count
+ self.total_count = Bird.count if enable_count
end
end