datamapper / dm-types

DataMapper plugin providing extra data types
http://datamapper.org/
MIT License
55 stars 80 forks source link

Enums are nil when an already created object is fetched #61

Open HoneyryderChuck opened 12 years ago

HoneyryderChuck commented 12 years ago

Greetings

I'm having some issues with the use of Enum as a type. I declare it in the model like this:


class Bong
property :label,  Enum[:a, :b, :c]
end

After that I create an instance:


a = Bong.create(:label => :a)
a.label #=> :a
a.label = :b
a.label #=> :b
a.id #=> 1

But later, as I fetch it, I get the strangest behaviour:


a = Bong.get(1)
a.label #=> nil

can you tell me why this happens?

postmodern commented 12 years ago

I could not reproduce this using the latest version of dm-types in dm-bug-report. What version of dm-types are you using?

HoneyryderChuck commented 12 years ago

I'm using dm-types 1.2.1.

HoneyryderChuck commented 12 years ago

Hi,

I tried your example in the bug-report and... it worked! But finally I figured out why it was not working in the first place. This has to do how the Enums from DataMapper are stored in the DB. They are being stored as Integers and not DB enums.

Let me explain a bit my point. I'm currently developing 2 applications that communicate with the same database (MySQL, by the way, which supports enums). One of the applications is in Ruby on Rails, therefore using Active Record. Since AR doesn't support DB enums natively, we had to add the gem "enum_column3" to make it support it.

The other app is in Sinatra. Since Sinatra's support for AR is a bit shaky, and as I always wanted to jump to DataMapper, I took the decision and went with it. Thing is, the physical model is dependent of the first app (that is where the migration and written/run). Therefore, DataMapper should not migrate anything, only interact with the data it already exists. So, the second app is expecting to fetch Integers, only to find Enums there.

Would it be possible to support MySQL enums for such a case?

postmodern commented 12 years ago

@TiagoCardoso1983 I believe DataMapper's Enum should work just fine with integers. I wonder if AR's enum_column3 is using different Integer values than what DataMapper's Enum is expecting (thus it returning nil)?

As a workaround, you could define that column as a plain Integer. Then override the reader/writer methods, to map values through a Hash.

ENUM = {1 => ..., 2 => ...}

def foo
  ENUM[super]
end

def foo=(new_foo)
  super(ENUM.inverse[new_foo])
end

This reminds me, I really need to write a custom Property which is like Enum, but for mapping known values to Symbols, and vice versa.

HoneyryderChuck commented 12 years ago

http://dev.mysql.com/doc/refman/5.0/en/enum.html

enum_column3 is using the values directly, since MySQL enum allows strings, and recommends them.

For this reason I cannot use your recommended solution, since I cannot change the column type from enum to integer.

I think the Enum type is fine as it is, since it covers the lowest common denominator (not all DBs support Enum types). I just think that the Enum should extend that property for the cases in which the DB is MySQL and the enum is actually a DB enum (either through a DM inner extension or through a separate gem, as AR did).

A new type could also be a good idea. I just can't think of a better naming than Enum, cuz that is (also) what it is :)

Tell me what you think. For me, the best short-term solution would be to switch the type of the column from Enum to String (or Symbol) and then add a validation to check if the added value is of an accepted value).

HoneyryderChuck commented 12 years ago

Hi again,

I managed some kind of work-around. Basically, I wrote some custom type. Since I'm still not overtly familiar with the DM structure, I'll post the code here so you can take a further look:


require 'dm-core'
require 'dm-types/support/flags'

module DataMapper
  class Property
    class MySqlEnum < String

      include Flags

      def initialize(model, name, options = {})
        @flags = options.fetch(:flags, self.class.flags).map(&:to_s)
        super
      end

      def valid?(value, negated=false)
        super && @flags.include?(value)
      end

    end
  end
end

Basically the idea was to handle the value as a string, validate it according to what was passed as possible labels (both of which I did, in my own possibly faulty way) and handle the truncation of the enum column values case some odd value is read from the DB (without that validation, that's what happens, we get a DB exception).

As a further improvement on the whole EnumAsInteger/EnumAsEnum debate, I think DM should allow both, in which the "real" enum type should be defined in a config file, some like DataMapper::Enum.as_integer = false or something of that kind. My problem with my current implementation is that it may raise maintenance issues on the long run since people will get confused with the two implementations of enum and why one should need them.

Regards