thetron / mongoid-enum

Sweet enum sugar for your Mongoid documents
MIT License
117 stars 167 forks source link

make sure getters always return Symbols #30

Open tomasc opened 8 years ago

tomasc commented 8 years ago

We have ran into several issues when relying on enum to return Symbol type.

This PR replaces read_attribute by send, since the read_attribute returns unprocessed (raw) value before typecast (demongoize) and therefore can potentially return something else than Symbol.

Also, this PR makes sure all Array values (in multiple option) are Symbols as well.

For your reference, here you can see how Mongoid uses read_attribute to get raw value which is then processed to a proper type using demongoize: https://github.com/mongodb/mongoid/blob/master/lib/mongoid/fields.rb#L411-L424

This is extermely important especially now, when Symbols are being deprecated and an app using Mongoid can potentially start returning Strings instead of Symbols. See here: https://jira.mongodb.org/browse/RUBY-1075?jql=project%20%3D%20RUBY%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20priority%20DESC%2C%20key%20DESC

onomated commented 8 years ago

+1.
Currently running into this issue as I had to manually rename all my Mongo collections to remove underscores. This changed the field types to strings and breaks the <enum_val>? methods. I got into this situation due to commit 2288ee6 which stopped using Mongoid's aliasing features, so I would've had to reference enum fields with underscore prefixes. Instead of changing to underscore prefixes, I changed the mongoid enum prefix configuration to empty string. As a result I had to rename the fields in the DB.

Monkey-patched in my project with this module. The solution in this PR resulted in stack overflow:

module CoreExtensions
  module Mongoid
    module Enum
      module EnsureSymbol
        extend ActiveSupport::Concern

        included do |base|
          base.instance_eval do
            def define_array_field_accessor(name, field_name)
              class_eval <<-EORUBY, __FILE__, __LINE__ + 1
                def #{name}=(vals)
                  self.write_attribute(:#{field_name}, Array(vals).compact.map(&:to_sym))
                end
                def #{name}()
                  self.read_attribute(:#{field_name}).map{ |i| i.try(:to_sym) }
                end
              EORUBY
            end

            def define_string_field_accessor(name, field_name)
              class_eval <<-EORUBY, __FILE__, __LINE__ + 1
                def #{name}=(val)
                  self.write_attribute(:#{field_name}, val.try(:to_sym))
                end
                def #{name}()
                  self.read_attribute(:#{field_name}).try(:to_sym)
                end
              EORUBY
            end
          end
        end

      end
    end
  end
end

Applied by:

Mongoid::Enum.include CoreExtensions::Mongoid::Enum::EnsureSymbol
tomasc commented 8 years ago

any chance to have this merged?

tomasc commented 8 years ago

can we have this merged please?

jeremyhaile commented 8 years ago

I'd question this change vs a change that converts everything to strings. The reason is that the "symbol" type is deprecated by BSON and is deprecated in Mongo 3.2.

I opened an issue specifically for this to discuss: https://github.com/thetron/mongoid-enum/issues/41

tomasc commented 8 years ago

@jeremyhaile I am not quite sure whether changes on the db level should have impact on how the enum abstraction works in Ruby (unless of course there is deprecation of Symbols in Ruby itself). Perhaps a way forward might be to add config option (Symbol/String)?