dry-rb / dry-schema

Coercion and validation for data structures
https://dry-rb.org/gems/dry-schema
MIT License
426 stars 109 forks source link

Unexpected error when hash validation fails #325

Open bitberry-dev opened 4 years ago

bitberry-dev commented 4 years ago

Hello, Piotr! I found a bug

Describe the bug

In Params processor when validation for a key with hash value fails then we get another error for a key with integer value (string containing integer).

To Reproduce

require "dry/schema"

SCHEMA = Dry::Schema.Params do
  optional(:integer).value(:integer)
  optional(:string).value(:string)
  optional(:hash).value(:hash)
end

# valid format
sample_data = {
  integer: "1", # valid
  string: "string", # valid
  hash: {} # valid
}

result = SCHEMA.(sample_data)
result.success? # => true (as expected)
result.errors.messages.size # => 0 (as expected)

# string is invalid
sample_data_with_invalid_string = {
  integer: "1", # valid
  string: true, # invalid
  hash: {} # valid
}

result = SCHEMA.(sample_data_with_invalid_string)
result.success? # => false (as expected)
result.errors.messages.size # => 1 (as expected)

# hash is invalid
sample_data_with_invalid_hash = {
  integer: "1", # valid
  string: "string", # valid
  hash: "another_string" # invalid
}

result = SCHEMA.(sample_data_with_invalid_hash)
result.success? # => false (as expected)
result.errors.messages.size # => 2 (???)
# errors={:integer=>["must be an integer"], :hash=>["must be a hash"]}

Expected behavior

The last test should return only one error for hash

Your environment

P.S. Great gem btw, thank you for your work

bitberry-dev commented 4 years ago

An interesting thing this bug occurs only when .value(:hash) is used. When .hash used all is okay.

require "dry/schema"

SCHEMA1 = Dry::Schema.Params do
  optional(:integer).value(:integer)
  optional(:hash).value(:hash)
end
# => #<Dry::Schema::Params keys=["integer", "hash"] rules={:integer=>"key?(:integer) THEN key[integer](int?)", :hash=>"key?(:hash) THEN key[hash](hash?)"}>

SCHEMA2 = Dry::Schema.Params do
  optional(:integer).value(:integer)
  optional(:hash).hash
end
# => #<Dry::Schema::Params keys=["integer", "hash"] rules={:integer=>"key?(:integer) THEN key[integer](int?)", :hash=>"key?(:hash) THEN key[hash](hash?)"}>

data = {
  integer: '1',
  hash: nil
}

SCHEMA1.(data)
# => #<Dry::Schema::Result{:integer=>"1", :hash=>nil} errors={:integer=>["must be an integer"], :hash=>["must be a hash"]}>

SCHEMA2.(data)
# => #<Dry::Schema::Result{:integer=>1, :hash=>nil} errors={:hash=>["must be a hash"]}>

.hash works as expected

solnic commented 4 years ago

That's an interesting bug, thanks for the report! What if you use different key names?

bitberry-dev commented 4 years ago

It does not depend on the name of the keys, I got it with completely different keys, in the example I used them for clarity :)

bitberry-dev commented 4 years ago

After a little research I found interesting place https://github.com/dry-rb/dry-schema/blob/f67d54ec0c884bd6261c508018bbf1def756635c/lib/dry/schema/macros/value.rb#L46

At this point call optional(:hash).value(:hash) ignored Only calls with block will processed optional(:hash).value(:hash) {}

So if I write optional(:hash).value(:hash) {} then schemas works as expected

Hope this helps!

solnic commented 4 years ago

@bitberry-dev hmm ok, thanks! I'll take a look

patodevilla commented 3 years ago

@solnic is this bug already reported? It appears empty hashes are not detected by validate_keys

schema = Dry::Schema.JSON do
  config.validate_keys = true
end
schema.({hello: {}})
 => #<Dry::Schema::Result{} errors={}> 
schema.({hello: 1})
 => #<Dry::Schema::Result{} errors={:hello=>["is not allowed"]}>