Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

validates_numericality_of doesn't work as expected #2237

Closed
achappellAtUniqueltd opened this Issue · 9 comments

7 participants

Andrew Chappell Durran Jordan Tobias Schlottke Kareem Kouddous Milovan Zogovic Ronan O'Driscoll Gerad Suyderhoud
Andrew Chappell

Using mongoid (3.0.1):

I have run into problems with the validator. Specifically, it will accept floats and even non-numbers for integer fields and the validation passes.

class MyTest
  include Mongoid::Document
  include Mongoid::Timestamps

  field :filesize, :type => Integer

  validates_numericality_of :filesize, :greater_than_or_equal_to => 0, :only_integer => true
end
mt = MyTest.new
mt.filesize = 5.9
mt.valid? # true
mt.filesize == 5 #true

That shouldn't be valid, but it says it is and truncates it to 5.

There's also this issue:

mt = MyTest.new
mt.filesize = 'horse'
mt.valid? # true
mt.filesize == 0 #true

This seems directly related to issue #2137, which was closed. However, this is a relevant issue for us as it breaks the ActiveRecord validation API.

Durran Jordan
Owner

Yeah the difference is in the underlying design of AR vs. Mongoid as I explained in #2137. Mongoid is casting everything to avoid storing two values before/after typecast for each attribute. I'm still looking into a way to solve this without the potential memory bloat for large documents. This won't be able to be changed as well until 4.0 at the earliest since this is a backwards incompatible change from Mongoid's perspective even though it does not match ActiveRecord.

Andrew Chappell

I was thinking about this problem, and I can understand not wanting to store everything twice. So, what if at the time of casting, you hook in the validation to validate the raw data. If it checks out, then cast as it's doing now, otherwise, process the validation error?

Tobias Schlottke

+1

Kareem Kouddous

+1 This is causing a huge headache for us as we rely on numericality validation extensively across our app. Would a valid approach be to move the type conversion to only when a document is about to be persisted?

Ronan O'Driscoll

+1

Gerad Suyderhoud

It seems like you'd only have to store two values when you're a) writing to an attribute and b) performing a type cast. It seems fairly unlikely that this would lead to memory bloat. Have people using ActiveRecord complained?

In any case, those of us who want to use mongoid to store valid numeric data from user input are going to have to validate the numericality somehow, so we're going to see the memory bloat no matter what.

Perhaps it make sense to make the ActiveRecord style behavior the default and then allowing people who care about the memory overhead disable it.

Gerad Suyderhoud

Alternatively, you could trade space for time and just have the getter do the typecast from the attribute_before_type_cast (if the attribute_before_type_cast is set)

Gerad Suyderhoud gerad referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
Gerad Suyderhoud gerad referenced this issue from a commit in fortnightlabs/mongoid
Gerad Suyderhoud gerad add attribute_before_type_cast
Among other things, this fixes numericality validation. If this looks good, and
it's deemed necessary, I'm happy to add a configuration option which disables
the functionality for people with memory overhead concerns.

See #2137, #2237, #2436, #2429, #2450.
1bbf22d
Durran Jordan
Owner

Closing as we have #2465 to address.

Durran Jordan durran closed this
Durran Jordan durran referenced this issue from a commit
Gerad Suyderhoud gerad add attribute_before_type_cast
Among other things, this fixes numericality validation. If this looks good, and
it's deemed necessary, I'm happy to add a configuration option which disables
the functionality for people with memory overhead concerns.

See #2137, #2237, #2436, #2429, #2450.
b72d350
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.