grape icon indicating copy to clipboard operation
grape copied to clipboard

Values Validator => Proc's arity < 2 ?

Open ericproulx opened this issue 2 years ago • 3 comments

While running specs with --warnings I found an interesting API endpoint under spec/grape/validations/validators/values_spec.rb with a successful test but maybe not the way we wanted.

The api is defined like this

params do
  requires :input_one, :input_two, type: Integer, values: { value: ->(v1, v2) { v1 + v2 > 10 } }
end
get '/proc/arity2'

And the test goes like this

context 'when arity is > 1' do
  it 'returns an error status code' do
    get '/proc/arity2', input_one: 2, input_two: 3
    expect(last_response.status).to eq 400
  end
end

It will pass but it also emits 2 warnings

Error 'wrong number of arguments (given 1, expected 2)' raised while validating attribute 'input_one'
Error 'wrong number of arguments (given 1, expected 2)' raised while validating attribute 'input_two'

If we take a closer look a the code

  • Proc's arity is expected to be < 2
  • each params a process individually (SingleAttributeIterator) so there's no relation between :input_one and :input_two even if it seems to be.

I was wondering if its really a bug. Also, I think we should raise more awareness with proper error instead of a warning.

ericproulx avatar Jan 03 '24 21:01 ericproulx

Change the code to be v1 + v2 == 5, does it pass? If not it's a bug ;)

dblock avatar Jan 04 '24 17:01 dblock

Change the code to be v1 + v2 == 5, does it pass? If not it's a bug ;)

It just doesn't work since arity is set to 1 and its not a MultipleAttributeIterator

ericproulx avatar Jan 05 '24 21:01 ericproulx

Change the code to be v1 + v2 == 5, does it pass? If not it's a bug ;)

It just doesn't work since arity is set to 1 and its not a MultipleAttributeIterator

Obvious bug then :)

dblock avatar Jan 05 '24 22:01 dblock