Type exception on save? when using uniqueness validation #117

Closed
mbrock opened this Issue Jan 13, 2015 · 7 comments

Comments

Projects
None yet
2 participants
@mbrock
Contributor

mbrock commented Jan 13, 2015

Hello!

Here is a small program that causes an InvalidType exception when using `save?'. The reason is that there is a uniqueness validation on the relevant field, which assumes that the value is castable.

require 'nobrainer'

NoBrainer.configure { |c| c.app_name = 'nobrainer_test_uniqueness_validation' }
NoBrainer.drop!

class X
  include NoBrainer::Document
  field :i, type: Integer
  validates_uniqueness_of :i
end

x = X.new i: "foo"
x.save?

I am not very familiar with validations and I'm not sure what I would expect. But it seems wrong that save? throws an exception.

@nviennot

This comment has been minimized.

Show comment
Hide comment
@nviennot

nviennot Jan 13, 2015

Owner

It will, because "foo" is not an integer.

You can also use save instead of save? and you will see that the exception is NoBrainer::Error::InvalidType: I should be a integer.
So I don't think this has anything to do with the uniqueness validator.

Owner

nviennot commented Jan 13, 2015

It will, because "foo" is not an integer.

You can also use save instead of save? and you will see that the exception is NoBrainer::Error::InvalidType: I should be a integer.
So I don't think this has anything to do with the uniqueness validator.

@mbrock

This comment has been minimized.

Show comment
Hide comment
@mbrock

mbrock Jan 13, 2015

Contributor

Sorry, yes, to clarify: without the uniqueness validator, save? does not throw, but instead leaves a "should be an integer" message in x.errors.

Contributor

mbrock commented Jan 13, 2015

Sorry, yes, to clarify: without the uniqueness validator, save? does not throw, but instead leaves a "should be an integer" message in x.errors.

@mbrock

This comment has been minimized.

Show comment
Hide comment
@mbrock

mbrock Jan 13, 2015

Contributor

For context, my application has an explicit numericality validator to make sure that the field in question is a number, and also a scoped uniqueness validation. The CRUD interface wants to use save? and then show the errors to the user. But now, if the user accidentally submits foo as the number, she will get a stack trace instead of a validation error.

Contributor

mbrock commented Jan 13, 2015

For context, my application has an explicit numericality validator to make sure that the field in question is a number, and also a scoped uniqueness validation. The CRUD interface wants to use save? and then show the errors to the user. But now, if the user accidentally submits foo as the number, she will get a stack trace instead of a validation error.

@nviennot

This comment has been minimized.

Show comment
Hide comment
@nviennot

nviennot Jan 13, 2015

Owner

The type checking in nobrainer works like validations, so if the CRUD interface uses save?, then you can use x.errors.full_messages to get the type error. A validation error on i will be set.

Using the Integer type implies some numericality validator defined as the following:

Integers are accepted.
Strings are converted to integers only when the resulting integer can be converted back to the original stripped string. For example, " -4 " and "+3" are valid, but "4f" or "" are not.
Floats are accepted when their values matches exactly an integer.
Any other value is ignored, and a validation error is added.

see http://nobrainer.io/docs/types/

The validator that you add on top of nobrainer's internal type checking is extra. (Maybe useful if you want to use a positive constraint or whatnot).

FYI, the same rules apply on where() queries when it comes to type safetly. e.g. User.where(i: 'foo') will raise an InvalidType.

What was your expectation when you specified :type => Integer?

Owner

nviennot commented Jan 13, 2015

The type checking in nobrainer works like validations, so if the CRUD interface uses save?, then you can use x.errors.full_messages to get the type error. A validation error on i will be set.

Using the Integer type implies some numericality validator defined as the following:

Integers are accepted.
Strings are converted to integers only when the resulting integer can be converted back to the original stripped string. For example, " -4 " and "+3" are valid, but "4f" or "" are not.
Floats are accepted when their values matches exactly an integer.
Any other value is ignored, and a validation error is added.

see http://nobrainer.io/docs/types/

The validator that you add on top of nobrainer's internal type checking is extra. (Maybe useful if you want to use a positive constraint or whatnot).

FYI, the same rules apply on where() queries when it comes to type safetly. e.g. User.where(i: 'foo') will raise an InvalidType.

What was your expectation when you specified :type => Integer?

@mbrock

This comment has been minimized.

Show comment
Hide comment
@mbrock

mbrock Jan 13, 2015

Contributor

My expectation is that save? will never raise an exception for field type errors. I do expect save without the question mark to raise, of course. If I remove the uniqueness validation from my example, then everything works as I expect.

Contributor

mbrock commented Jan 13, 2015

My expectation is that save? will never raise an exception for field type errors. I do expect save without the question mark to raise, of course. If I remove the uniqueness validation from my example, then everything works as I expect.

@nviennot nviennot closed this in 208ec54 Jan 13, 2015

@nviennot

This comment has been minimized.

Show comment
Hide comment
@nviennot

nviennot Jan 13, 2015

Owner

Sorry, I didn't understand your issue sooner. Fixed!

Owner

nviennot commented Jan 13, 2015

Sorry, I didn't understand your issue sooner. Fixed!

@mbrock

This comment has been minimized.

Show comment
Hide comment
@mbrock

mbrock Jan 13, 2015

Contributor

Excellent, thank you! 👍

Contributor

mbrock commented Jan 13, 2015

Excellent, thank you! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment