Skip to content

Commit

Permalink
Follow up to bbe7fe4 to fix enum leakage across classes.
Browse files Browse the repository at this point in the history
The original attempt didn't really fix the problem and wasn't testing the
problematic area. This commit corrected those issues in the original commit.

Also removed the private `enum_mapping_for` method. As `defined_enums` is now a
method, this method doesn't provide much value anymore.
  • Loading branch information
chancancode committed Apr 8, 2014
1 parent 3f5eb59 commit a5664fe
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 11 deletions.
9 changes: 6 additions & 3 deletions activerecord/lib/active_record/enum.rb
@@ -1,3 +1,5 @@
require 'active_support/core_ext/object/deep_dup'

module ActiveRecord module ActiveRecord
# Declare an enum attribute where the values map to integers in the database, # Declare an enum attribute where the values map to integers in the database,
# but can be queried by name. Example: # but can be queried by name. Example:
Expand Down Expand Up @@ -70,8 +72,9 @@ def self.extended(base)
base.defined_enums = {} base.defined_enums = {}
end end


def enum_mapping_for(attr_name) # :nodoc: def inherited(base)
defined_enums[attr_name.to_s] base.defined_enums = defined_enums.deep_dup

This comment has been minimized.

Copy link
@rafaelfranca

rafaelfranca Apr 8, 2014

Member

Why do we have an inherited and an extended callbacks?

This comment has been minimized.

Copy link
@chancancode

chancancode via email Apr 8, 2014

Author Member
super
end end


def enum(definitions) def enum(definitions)
Expand Down Expand Up @@ -136,7 +139,7 @@ def _enum_methods_module
mod = Module.new do mod = Module.new do
private private
def save_changed_attribute(attr_name, value) def save_changed_attribute(attr_name, value)
if (mapping = self.class.enum_mapping_for(attr_name)) if (mapping = self.class.defined_enums[attr_name])
if attribute_changed?(attr_name) if attribute_changed?(attr_name)
old = changed_attributes[attr_name] old = changed_attributes[attr_name]


Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/validations/uniqueness.rb
Expand Up @@ -93,7 +93,7 @@ def deserialize_attribute(record, attribute, value)
end end


def map_enum_attribute(klass, attribute, value) def map_enum_attribute(klass, attribute, value)
mapping = klass.enum_mapping_for(attribute.to_s) mapping = klass.defined_enums[attribute.to_s]
value = mapping[value] if value && mapping value = mapping[value] if value && mapping
value value
end end
Expand Down
35 changes: 28 additions & 7 deletions activerecord/test/cases/enum_test.rb
Expand Up @@ -252,17 +252,38 @@ def self.name; 'Book'; end
end end


test "enums are distinct per class" do test "enums are distinct per class" do
Plane = Class.new(ActiveRecord::Base) do klass1 = Class.new(ActiveRecord::Base) do
enum status: [:grounded, :operational] self.table_name = "books"
enum status: [:proposed, :written]
end

klass2 = Class.new(ActiveRecord::Base) do
self.table_name = "books"
enum status: [:drafted, :uploaded]
end end
assert_equal({ "proposed" => 0, "written" => 1, "published" => 2 }, Book.statuses)
assert_equal({ "grounded" => 0, "operational" => 1 }, Plane.statuses) book1 = klass1.proposed.create!
book1.status = :written
assert_equal ['proposed', 'written'], book1.status_change

book2 = klass2.drafted.create!
book2.status = :uploaded
assert_equal ['drafted', 'uploaded'], book2.status_change
end end


test "enums are inheritable" do test "enums are inheritable" do
Encyclopedia = Class.new(Book) do subklass1 = Class.new(Book)
enum status: [:published, :reprinted]
subklass2 = Class.new(Book) do
enum status: [:drafted, :uploaded]
end end
assert_equal({ "published" => 0, "reprinted" => 1 }, Encyclopedia.statuses)
book1 = subklass1.proposed.create!
book1.status = :written
assert_equal ['proposed', 'written'], book1.status_change

book2 = subklass2.drafted.create!
book2.status = :uploaded
assert_equal ['drafted', 'uploaded'], book2.status_change
end end
end end

1 comment on commit a5664fe

@chancancode
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.