diff options
author | Chen Kinnrot <kinnrot@gmail.com> | 2017-11-18 21:22:00 +0200 |
---|---|---|
committer | Chen Kinnrot <kinnrot@gmail.com> | 2017-11-28 15:50:18 +0200 |
commit | 6552ce0361f30a46a58a8bcb659866f06c8b7430 (patch) | |
tree | 9dafcb449cade201f9979ba94d935d93501a37e2 /activerecord | |
parent | f2b6406066888aaa10aba6c3cb95c2eb6dda760e (diff) | |
download | rails-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.md | 6 | ||||
-rw-r--r-- | activerecord/lib/active_record/enum.rb | 2 | ||||
-rw-r--r-- | activerecord/lib/active_record/scoping/named.rb | 6 | ||||
-rw-r--r-- | activerecord/test/cases/enum_test.rb | 18 | ||||
-rw-r--r-- | activerecord/test/cases/scoping/named_scoping_test.rb | 16 |
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? |