From 9b9640112e197e94e13940c0c0f990cc80ca5498 Mon Sep 17 00:00:00 2001 From: Alberto Almagro Date: Sat, 6 Oct 2018 18:08:45 +0200 Subject: Raise on invalid definition values When defining a Hash enum it can be easy to use [] instead of {}. This commit checks that only valid definition values are provided, those can be a Hash, an array of Symbols or an array of Strings. Otherwise it raises an ArgumentError. Fixes #33961 --- activerecord/CHANGELOG.md | 11 +++++++++++ activerecord/lib/active_record/enum.rb | 10 ++++++++++ activerecord/test/cases/enum_test.rb | 11 +++++++++++ 3 files changed, 32 insertions(+) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 19558a9e0c..6397bc361a 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,14 @@ +* Enum raises on invalid definition values + + When defining a Hash enum it can be easy to use [] instead of {}. This + commit checks that only valid definition values are provided, those can + be a Hash, an array of Symbols or an array of Strings. Otherwise it + raises an ArgumentError. + + Fixes #33961 + + *Alberto Almagro* + * Reloading associations now clears the Query Cache like `Persistence#reload` does. ``` diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index 23ecb24542..49fedc232c 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -150,6 +150,7 @@ module ActiveRecord enum_prefix = definitions.delete(:_prefix) enum_suffix = definitions.delete(:_suffix) definitions.each do |name, values| + assert_valid_enum_definition_values(values) # statuses = { } enum_values = ActiveSupport::HashWithIndifferentAccess.new name = name.to_s @@ -210,6 +211,15 @@ module ActiveRecord end end + def assert_valid_enum_definition_values(values) + unless values.is_a?(Hash) || values.all? { |v| v.is_a?(Symbol) } || values.all? { |v| v.is_a?(String) } + error_message = <<~MSG + Enum values #{values} must be either a hash, an array of symbols, or an array of strings. + MSG + raise ArgumentError, error_message + end + end + ENUM_CONFLICT_MESSAGE = \ "You tried to define an enum named \"%{enum}\" on the model \"%{klass}\", but " \ "this will generate a %{type} method \"%{method}\", which is already defined " \ diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb index d5a1d11e12..b4593ccdf2 100644 --- a/activerecord/test/cases/enum_test.rb +++ b/activerecord/test/cases/enum_test.rb @@ -265,6 +265,17 @@ class EnumTest < ActiveRecord::TestCase assert_equal "published", @book.status end + test "invalid definition values raise an ArgumentError" do + e = assert_raises(ArgumentError) do + Class.new(ActiveRecord::Base) do + self.table_name = "books" + enum status: [proposed: 1, written: 2, published: 3] + end + end + + assert_match(/must be either a hash, an array of symbols, or an array of strings./, e.message) + end + test "reserved enum names" do klass = Class.new(ActiveRecord::Base) do self.table_name = "books" -- cgit v1.2.3 From df11558942f9eb97f9ed0293d1d4508f7685bb51 Mon Sep 17 00:00:00 2001 From: Alberto Almagro Date: Sat, 6 Oct 2018 18:15:14 +0200 Subject: Privatize ENUM_CONFLICT_MESSAGE constant --- activerecord/lib/active_record/enum.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index 49fedc232c..3d97e4e513 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -224,6 +224,7 @@ module ActiveRecord "You tried to define an enum named \"%{enum}\" on the model \"%{klass}\", but " \ "this will generate a %{type} method \"%{method}\", which is already defined " \ "by %{source}." + private_constant :ENUM_CONFLICT_MESSAGE def detect_enum_conflict!(enum_name, method_name, klass_method = false) if klass_method && dangerous_class_method?(method_name) -- cgit v1.2.3