madeintandem / hstore_accessor

Adds typed hstore-backed field support to ActiveRecord models.
MIT License
242 stars 47 forks source link

Typecasting causes unexpected behavior #18

Closed drock closed 10 years ago

drock commented 10 years ago

Typecasting of values as they are set was added with #9 . This is causing my models to behave in a manner different than a normal ActiveRecord model attributes. Particularly, when passing in a string to an hstore attribute with the :integer type, a non-numerical string gets cast to 0. I would expect the model to accept the value and then my numericality validator to report the error when calling valid?

Example:

class MyModel < ActiveRecord::Base
    hstore_accesor :data, :number => :integer

    validates :number, numericality: { only_integer: true }
end

m = MyModel.new :number => 'lkj'
m.valid? #returns true
m.number #returns 0

In my opinion, this is causing the model to make an assumption about the input it should not be. This is particularly true when passing data in from a params hash from a form.

m.update_attributes params[:my_model]

The model should not be assuming that the user who submitted the form meant to put zero instead of lkj. This forces me as a developer to put this validation logic in the controller before being passed to the model but that is not the best place for such logic.

devmyndstatus commented 10 years ago

Derek,

Thanks for raising this, I agree it's probably better for this not to result in a zero. That said, I do think it's reasonable for the hstore_accessor to handle the type casting. I think there are two fixes we should do here:

1) Try to find a way to run the type casting on a setter after the validation stage runs, this gives users a chance to catch issues with normal AR validation.

2) Instead of casting to defaults, as in the case of a non-numeric string casting to zero, we should probably just raise an error when a bad cast would occur.

Thanks, // JC Grubbs | DevMynd http://devmynd.com

On Mar 28, 2014, at 9:30 AM, Derek Gallo notifications@github.com wrote:

Typecasting of values as they are set was added with #9 . This is causing my models to behave in a manner different than a normal ActiveRecord model attributes. Particularly, when passing in a string to an hstore attribute with the :integer type, a non-numerical string gets cast to 0. I would expect the model to accept the value and then my numericality validator to report the error when calling valid?

Example:

class MyModel < ActiveRecord::Base hstore_accesor :data, :number => :integer

validates :number, numericality: { only_integer: true }

end

m = MyModel.new :number => 'lkj' m.valid? #returns true m.number #returns 0 In my opinion, this is causing the model to make an assumption about the input it should not be. This is particularly true when passing data in from a params hash from a form.

m.update_attributes params[:my_model] The model should not be assuming that the user who submitted the form meant to put zero instead of lkj. This forces me as a developer to put this validation logic in the controller before being passed to the model but that is not the best place for such logic.

— Reply to this email directly or view it on GitHub.

drock commented 10 years ago

After messing around with a standard ActiveRecord model some more, I retract this issue. This is in fact exhibiting the same behavior as a standard ActiveRecord model. If you have a model with an attribute that maps to an integer column in the database and you try to set it to a string, it will in fact be set as a zero.