solnic / virtus

[DISCONTINUED ] Attributes on Steroids for Plain Old Ruby Objects
MIT License
3.77k stars 229 forks source link

custom coercion with arrays #284

Open doughsay opened 10 years ago

doughsay commented 10 years ago

Using virtus 1.0.3 I get this behavior when attempting custom coercion with an array:

class MyInt < Virtus::Attribute
  def coerce(value)
    value.to_i
  end
end

class Thing
  include Virtus.model

  attribute :int, MyInt
  attribute :ints, Array[MyInt]
end

Thing.new(:int => '1', :ints => ['1', '2'])
#<Thing:0x007f9b309a74a0 @int=1, @ints=["1", "2"]>

According to the readme this seems like it should work... As you can see, the "int" field was coerced but the "ints" array was not. Is this a bug or am I doing something wrong?

solnic commented 10 years ago

You must specify primitive in custom attributes. Virtus looks up for the attribute based on the primitive which is the actual type you want to coerce to. Attribute subclasses are not types. I know this is a little bit confusing, it has its historical reasons and I'll be changing (read: improving and simplifying) that completely in virtus 2.0.

For now to make this work you can do this:

class MyInt < Virtus::Attribute
  primitive Integer # otherwise it defaults to BasicObject

  def coerce(value)
    value.to_i
  end
end

class Thing
  include Virtus.model

  attribute :int, MyInt
  attribute :ints, Array[MyInt]
end

EDIT: fix typo /type/attribute/ (see even I confuse it ;))

doughsay commented 10 years ago

Thanks for the info. I didn't realize primitive was needed because in both examples in the readme it's not present. This solves my issue though, thanks!

doughsay commented 10 years ago

Sorry, I spoke too soon. For the simple case above, it actually does work, but I was obviously not trying to write a custom Int type... Below is something closer to what I was attempting, and it still isn't working even with specifying primitive.

class ObjectId < Virtus::Attribute
  primitive Moped::BSON::ObjectId

  def coerce(value)
    Moped::BSON::ObjectId.from_string(value)
  end
end

class Thing
  include Virtus.model(:strict => true)

  attribute :id, ObjectId
  attribute :other_ids, Array[ObjectId]
end

t = Thing.new(:id => '53e51e803f30e1037e0008ac', :other_ids => ['53e51eee3f30e1037e002c01', '53e51eee3f30e1037e002c02'])

t.id.class
#=> Moped::BSON::ObjectId < Object

t.other_ids[0].class
#=> String < Object
doughsay commented 10 years ago

I should point out that the coercer never seems to run at all in this case...

solnic commented 10 years ago

OK I will look into it

justfalter commented 10 years ago

I added specs that demonstrate the failure in #294, hopefully this helps out.

I also poked around on this a bit, and it seems come down to the fact that, unless there is an Axiom type defined for the primitive that you're using, virtus will never think that the attribute needs to be coerced. This is because Axiom::Types.infer returns the type for BasicObject when it doesn't have anything associated with the provided class:

[3] pry(main)> Axiom::Types.infer(Moped::BSON::ObjectId)
=> Axiom::Types::Object (BasicObject)

Since all ruby objects are derived from BasicObject, I guess virtus doesn't feel that the values need to be coerced.

If you define an axiom type for the primitive class, everything starts working:

Axiom::Types::Object.new { primitive: Moped::BSON::ObjectId }

solnic commented 10 years ago

Yes this makes sense. Thanks for the hints. I'll see what I can do to quickly fix it.

justfalter commented 10 years ago

FYI, there seems to be a bit of a gotcha w.r.t Axiom::Types caching inferences. Things work well if you create the type prior to inferring:

class Foo; end
Axiom::Types::Object.new { primitive Foo }
=> Axiom::Types::Object (Foo)
Axiom::Types.infer Foo
=> Axiom::Types::Object (Foo)

If you, however, try to infer the type AND THEN create the type, the Axiom::Types.infer cache is never updated.

class Bar; end
Axiom::Types.infer Bar
=> Axiom::Types::Object (BasicObject)
Axiom::Types::Object.new { primitive Bar }                                                                                                                                   
=> Axiom::Types::Object (Bar)
Axiom::Types.infer Bar
=> Axiom::Types::Object (BasicObject)

Ideally, the old cache entry would be invalidated when a new axiom type is defined.

backus commented 9 years ago

Is this still an issue or is registering your type with Axiom the solution? I'm having trouble with custom attributes in hashes and arrays but I'm not sure if I'm using Virtus wrong or not.