Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rule.each produces error when input is nil #708

Closed
bautrey37 opened this issue May 4, 2022 · 2 comments
Closed

rule.each produces error when input is nil #708

bautrey37 opened this issue May 4, 2022 · 2 comments

Comments

@bautrey37
Copy link
Contributor

Describe the bug

Error is returned back on rule.each when passed in the input value is nil. nil input should be accepted when a maybe is used on the key.

Error: undefined method `each_with_index' for nil:NilClass (NoMethodError)

To Reproduce

require 'dry-validation'

input = {
  webhooks: nil
}

Schema = Dry::Schema.Params do
  required(:id).value(:string)
end

Contract = Dry::Validation.Contract do
  params do
    optional(:webhooks).maybe { type?(Array) & max_size?(10) }
  end

  register_macro(:schema) do |macro:|
    key.failure('something')
  end

  rule(:webhooks).each(schema: Schema)
end

result = Contract.call(input)
puts result.to_h

Stack trace:

9: from /.../.rvm/gems/ruby-2.6.6/gems/dry-validation-1.7.0/lib/dry/validation/contract.rb:105:in `call'
8: from /.../.rvm/gems/ruby-2.6.6/gems/dry-validation-1.7.0/lib/dry/validation/result.rb:25:in `new'
7: from /.../.rvm/gems/ruby-2.6.6/gems/dry-validation-1.7.0/lib/dry/validation/contract.rb:106:in `block in call'
6: from /.../.rvm/gems/ruby-2.6.6/gems/dry-validation-1.7.0/lib/dry/validation/contract.rb:106:in `each'
5: from /.../.rvm/gems/ruby-2.6.6/gems/dry-validation-1.7.0/lib/dry/validation/contract.rb:109:in `block (2 levels) in call'
4: from /.../.rvm/gems/ruby-2.6.6/gems/dry-validation-1.7.0/lib/dry/validation/rule.rb:38:in `call'
3: from /.../.rvm/gems/ruby-2.6.6/gems/dry-validation-1.7.0/lib/dry/validation/rule.rb:38:in `new'
2: from /.../.rvm/gems/ruby-2.6.6/gems/dry-validation-1.7.0/lib/dry/validation/evaluator.rb:77:in `initialize'
1: from /.../.rvm/gems/ruby-2.6.6/gems/dry-validation-1.7.0/lib/dry/validation/evaluator.rb:77:in `instance_exec'
/.../.rvm/gems/ruby-2.6.6/gems/dry-validation-1.7.0/lib/dry/validation/rule.rb:86:in `block in each': undefined method `each_with_index' for nil:NilClass (NoMethodError)

Location where error occurs: https://github.com/dry-rb/dry-validation/blob/main/lib/dry/validation/rule.rb#L86

Expected behavior

The contract should pass validation since the maybe specifies that nil values are accepted.

Potential solution

This could be fixed by adding a || values[root].nil? to the conditional check on the unless block. Is this solution OK?

My environment

  • Affects my production application: YES
  • Ruby version: 2.6
  • OS: MacOS
  • dry-validation: 1.7
@solnic
Copy link
Member

solnic commented May 8, 2022

Thanks for reporting this. I'm not sure if we want to implicitly skip applying rules just because something is nil without actually making sure that nil is an acceptable value. IIRC it does happen already in some cases but I've always had mixed feelings about this. Having said that, for consistency's sake we should fix it like you suggested.

@bautrey37
Copy link
Contributor Author

Opened a PR for this. #709

I agree it's not best to implicitly skip applying rules when a value is nil. I don't know how to implement the solution any better.

@solnic solnic closed this as completed in 10a9d68 May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants