aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorChen Kinnrot <kinnrot@gmail.com>2017-11-18 21:22:00 +0200
committerChen Kinnrot <kinnrot@gmail.com>2017-11-28 15:50:18 +0200
commit6552ce0361f30a46a58a8bcb659866f06c8b7430 (patch)
tree9dafcb449cade201f9979ba94d935d93501a37e2 /activerecord
parentf2b6406066888aaa10aba6c3cb95c2eb6dda760e (diff)
downloadrails-6552ce0361f30a46a58a8bcb659866f06c8b7430.tar.gz
rails-6552ce0361f30a46a58a8bcb659866f06c8b7430.tar.bz2
rails-6552ce0361f30a46a58a8bcb659866f06c8b7430.zip
Prevent scope named same as a ActiveRecord::Relation instance method.
Due to inconsistent behavior when chaining scopes and one scope named after a Relation method Validation code added in 2 places: - scope, to prevent problematic scope names. - enum, cause it tries to auto define scope.
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md6
-rw-r--r--activerecord/lib/active_record/enum.rb2
-rw-r--r--activerecord/lib/active_record/scoping/named.rb6
-rw-r--r--activerecord/test/cases/enum_test.rb18
-rw-r--r--activerecord/test/cases/scoping/named_scoping_test.rb16
5 files changed, 48 insertions, 0 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 58baad120b..35130996b1 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+* Don't allow scopes to be defined which conflict with instance methods on `Relation`.
+
+ Fixes #31120.
+
+ *kinnrot*
+
## Rails 5.2.0.beta1 (November 27, 2017) ##
* Add new error class `QueryCanceled` which will be raised
diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb
index f373b98035..1a3e6e4d09 100644
--- a/activerecord/lib/active_record/enum.rb
+++ b/activerecord/lib/active_record/enum.rb
@@ -221,6 +221,8 @@ module ActiveRecord
def detect_enum_conflict!(enum_name, method_name, klass_method = false)
if klass_method && dangerous_class_method?(method_name)
raise_conflict_error(enum_name, method_name, type: "class")
+ elsif klass_method && method_defined_within?(method_name, Relation)
+ raise_conflict_error(enum_name, method_name, type: "class", source: Relation.name)
elsif !klass_method && dangerous_attribute_method?(method_name)
raise_conflict_error(enum_name, method_name)
elsif !klass_method && method_defined_within?(method_name, _enum_methods_module, Module)
diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb
index 310af72c41..752655aa05 100644
--- a/activerecord/lib/active_record/scoping/named.rb
+++ b/activerecord/lib/active_record/scoping/named.rb
@@ -171,6 +171,12 @@ module ActiveRecord
"a class method with the same name."
end
+ if method_defined_within?(name, Relation)
+ raise ArgumentError, "You tried to define a scope named \"#{name}\" " \
+ "on the model \"#{self.name}\", but ActiveRecord::Relation already defined " \
+ "an instance method with the same name."
+ end
+
valid_scope_name?(name)
extension = Module.new(&block) if block
diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb
index 78cb89ccc5..7cda712112 100644
--- a/activerecord/test/cases/enum_test.rb
+++ b/activerecord/test/cases/enum_test.rb
@@ -308,6 +308,24 @@ class EnumTest < ActiveRecord::TestCase
end
end
+ test "reserved enum values for relation" do
+ relation_method_samples = [
+ :records,
+ :to_ary,
+ :scope_for_create
+ ]
+
+ relation_method_samples.each do |value|
+ e = assert_raises(ArgumentError, "enum value `#{value}` should not be allowed") do
+ Class.new(ActiveRecord::Base) do
+ self.table_name = "books"
+ enum category: [:other, value]
+ end
+ end
+ assert_match(/You tried to define an enum named .* on the model/, e.message)
+ end
+ end
+
test "overriding enum method should not raise" do
assert_nothing_raised do
Class.new(ActiveRecord::Base) do
diff --git a/activerecord/test/cases/scoping/named_scoping_test.rb b/activerecord/test/cases/scoping/named_scoping_test.rb
index b0431a4e34..17d3f27bb1 100644
--- a/activerecord/test/cases/scoping/named_scoping_test.rb
+++ b/activerecord/test/cases/scoping/named_scoping_test.rb
@@ -151,6 +151,22 @@ class NamedScopingTest < ActiveRecord::TestCase
assert_equal "The scope body needs to be callable.", e.message
end
+ def test_scopes_name_is_relation_method
+ conflicts = [
+ :records,
+ :to_ary,
+ :to_sql,
+ :explain
+ ]
+
+ conflicts.each do |name|
+ e = assert_raises ArgumentError do
+ Class.new(Post).class_eval { scope name, -> { where(approved: true) } }
+ end
+ assert_match(/You tried to define a scope named \"#{name}\" on the model/, e.message)
+ end
+ end
+
def test_active_records_have_scope_named__all__
assert !Topic.all.empty?