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

Unexpected error when hash validation fails #325

Open
bitberry-dev opened this issue Oct 30, 2020 · 6 comments
Open

Unexpected error when hash validation fails #325

bitberry-dev opened this issue Oct 30, 2020 · 6 comments

Comments

@bitberry-dev
Copy link

bitberry-dev commented Oct 30, 2020

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

  • Affects my production application: YES
  • Ruby version: 2.7.2
  • OS: OS X 10.15.7

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

@bitberry-dev
Copy link
Author

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
Copy link
Member

solnic commented Oct 30, 2020

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

@bitberry-dev
Copy link
Author

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
Copy link
Author

After a little research I found interesting place

if block && type_spec.equal?(:hash)

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 solnic added this to the 1.5.7 milestone Nov 12, 2020
@solnic
Copy link
Member

solnic commented Nov 12, 2020

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

@solnic solnic modified the milestones: 1.5.7, 1.6.2 Feb 2, 2021
@patodevilla
Copy link

@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"]}>

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

3 participants