dry-rb / dry-validation

Validation library with type-safe schemas and rules
https://dry-rb.org/gems/dry-validation
MIT License
1.34k stars 188 forks source link

NoMethodError is raised when validating non-hash objects with a Dry::Validation::Contract that has config.validate_keys set to true #716

Closed tanyongkee closed 1 year ago

tanyongkee commented 1 year ago

Describe the bug

NoMethodError is raised when validating non-hash objects with a Dry::Validation::Contract that has config.validate_keys set to true.

Example

To Reproduce

class TestContract < Dry::Validation::Contract
  config.validate_keys = true
  json do
    required(:name).value(:hash) do
      optional(:title).filled(:string)
    end
  end
end

TestContract.new.call(1)

NoMethodError: undefined method `flat_map' for 1:Integer

        hash.flat_map { |key, value|
            ^^^^^^^^^

Expected behavior

=> #<Dry::Validation::Result{} errors={:name=>["is missing"]}>

The expected behavior is observed when config.validate_keys = false

My environment

solnic commented 1 year ago

I couldn't reproduce it with dry-validation 1.9.0 and dry-schema 1.11.2

class TestContract < Dry::Validation::Contract
  config.validate_keys = true
  json do
    required(:name).value(:hash) do
      optional(:title).filled(:string)
    end
  end
end

TestContract.new.call(name: nil)
# => #<Dry::Validation::Result{:name=>nil} errors={:name=>["must be a hash"]}>

TestContract.new.call(name: 1)
# => #<Dry::Validation::Result{:name=>1} errors={:name=>["must be a hash"]}>
timriley commented 1 year ago

@solnic I think the problem raised initially is if the contract is called with a non-hash value:

TestContract.new.call(1)

Of course, that's nonsensical usage for the structure of the contract we've been discussing here.

I wonder if we should raise a more meaningful error earlier in the call stack rather than waiting for a NoMethodError about flat_map?

solnic commented 1 year ago

Oh right. Thanks Tim. I'm pretty sure it happens regardless of the validate_keys setting though. dry-schema doesn't check the input and assumes you passed in a hash. We could change it to only expect a hash and raise a meaningful error in case of non-hash values.

bryszard commented 1 year ago

Can the issue be closed?