From 9c3afdc327132c7f1f4d05eebc0c05b715442e7d Mon Sep 17 00:00:00 2001
From: Jefferson Lai <jefflai2@gmail.com>
Date: Mon, 10 Feb 2014 03:00:05 -0800
Subject: Fixes Issue #13466.
Changed the call to a scope block to be evaluated with instance_eval.
The result is that ScopeRegistry can use the actual class instead of base_class when
caching scopes so queries made by classes with a common ancestor won't leak scopes.
---
activerecord/CHANGELOG.md | 11 +++++++++++
activerecord/lib/active_record/scoping.rb | 4 ++--
activerecord/lib/active_record/scoping/named.rb | 8 ++++++--
activerecord/test/cases/scoping/named_scoping_test.rb | 7 ++++++-
activerecord/test/fixtures/ratings.yml | 10 ++++++++++
activerecord/test/models/comment.rb | 1 +
activerecord/test/models/rating.rb | 4 ++++
activerecord/test/schema/schema.rb | 2 ++
8 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 6e77493de9..db754a02e6 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,14 @@
+* Changed scoped blocks to be executed with `instance_eval`
+
+ Named scopes (i.e. using STI) were previously cached according to
+ base class so scoped queries made by classes with a common ancestor would
+ leak. Changed the way scope blocks were called so inheritance rules are
+ followed during the call and scopes are cached correctly.
+
+ Fixes #13466.
+
+ *Jefferson Lai*
+
* Save `has_one` association even if the record doesn't changed.
Fixes #14407.
diff --git a/activerecord/lib/active_record/scoping.rb b/activerecord/lib/active_record/scoping.rb
index 3e43591672..fca4f1c9d3 100644
--- a/activerecord/lib/active_record/scoping.rb
+++ b/activerecord/lib/active_record/scoping.rb
@@ -11,11 +11,11 @@ module ActiveRecord
module ClassMethods
def current_scope #:nodoc:
- ScopeRegistry.value_for(:current_scope, base_class.to_s)
+ ScopeRegistry.value_for(:current_scope, self.to_s)
end
def current_scope=(scope) #:nodoc:
- ScopeRegistry.set_value_for(:current_scope, base_class.to_s, scope)
+ ScopeRegistry.set_value_for(:current_scope, self.to_s, scope)
end
end
diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb
index 49cadb66d0..826b710e92 100644
--- a/activerecord/lib/active_record/scoping/named.rb
+++ b/activerecord/lib/active_record/scoping/named.rb
@@ -148,9 +148,13 @@ module ActiveRecord
extension = Module.new(&block) if block
singleton_class.send(:define_method, name) do |*args|
- scope = all.scoping { body.call(*args) }
+ if body.respond_to?(:to_proc)
+ scope = all.scoping { instance_exec(*args, &body) }
+ else
+ # Body is given as an object instead of a block, so invoke call()
+ scope = all.scoping { body.call(*args) }
+ end
scope = scope.extending(extension) if extension
-
scope || all
end
end
diff --git a/activerecord/test/cases/scoping/named_scoping_test.rb b/activerecord/test/cases/scoping/named_scoping_test.rb
index f0ad9ebb8a..f66b76a7ad 100644
--- a/activerecord/test/cases/scoping/named_scoping_test.rb
+++ b/activerecord/test/cases/scoping/named_scoping_test.rb
@@ -2,12 +2,13 @@ require "cases/helper"
require 'models/post'
require 'models/topic'
require 'models/comment'
+require 'models/rating'
require 'models/reply'
require 'models/author'
require 'models/developer'
class NamedScopingTest < ActiveRecord::TestCase
- fixtures :posts, :authors, :topics, :comments, :author_addresses
+ fixtures :posts, :authors, :topics, :comments, :author_addresses, :ratings
def test_implements_enumerable
assert !Topic.all.empty?
@@ -115,6 +116,10 @@ class NamedScopingTest < ActiveRecord::TestCase
assert_equal 1,SpecialPost.containing_the_letter_a.count
end
+ def test_scope_subquery_with_STI
+ assert_nothing_raised { VerySpecialComment.special_parent(SpecialRating.first).count }
+ end
+
def test_has_many_through_associations_have_access_to_scopes
assert_not_equal Comment.containing_the_letter_e, authors(:david).comments
assert !Comment.containing_the_letter_e.empty?
diff --git a/activerecord/test/fixtures/ratings.yml b/activerecord/test/fixtures/ratings.yml
index 34e208efa3..2b45c5080e 100644
--- a/activerecord/test/fixtures/ratings.yml
+++ b/activerecord/test/fixtures/ratings.yml
@@ -2,13 +2,23 @@ normal_comment_rating:
id: 1
comment_id: 8
value: 1
+ type: Rating
special_comment_rating:
id: 2
comment_id: 6
value: 1
+ type: Rating
sub_special_comment_rating:
id: 3
comment_id: 12
value: 1
+ type: Rating
+
+special_rating:
+ id: 4
+ comment_id: 10
+ value: 1
+ type: SpecialRating
+ special_comment_id: 3
diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb
index ede5fbd0c6..6a9dfacf56 100644
--- a/activerecord/test/models/comment.rb
+++ b/activerecord/test/models/comment.rb
@@ -35,4 +35,5 @@ class SubSpecialComment < SpecialComment
end
class VerySpecialComment < Comment
+ scope :special_parent, -> (special_rating) { where parent_id: special_rating.special_comment.id }
end
diff --git a/activerecord/test/models/rating.rb b/activerecord/test/models/rating.rb
index 25a52c4ad7..5409230c2e 100644
--- a/activerecord/test/models/rating.rb
+++ b/activerecord/test/models/rating.rb
@@ -2,3 +2,7 @@ class Rating < ActiveRecord::Base
belongs_to :comment
has_many :taggings, :as => :taggable
end
+
+class SpecialRating < Rating
+ belongs_to :special_comment
+end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index b44e72a67c..cbcdf93673 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -579,7 +579,9 @@ ActiveRecord::Schema.define do
create_table :ratings, force: true do |t|
t.integer :comment_id
+ t.integer :special_comment_id
t.integer :value
+ t.string :type
end
create_table :readers, force: true do |t|
--
cgit v1.2.3