dry-rb / dry-types

Flexible type system for Ruby with coercions and constraints
https://dry-rb.org/gems/dry-types
MIT License
859 stars 134 forks source link

Params coercion for Array works in an unintuitive way #425

Closed Quintasan closed 3 years ago

Quintasan commented 3 years ago

Describe the bug

When using a params schema, if you specify a key that has to be an Array then it doesn't get coerced unless it's a Ruby array.

pry(#<AppsController>)> search_params
=> {"search"=>"", "users"=>"[10001]", "page"=>"1", "current_user_id"=>10000}
[9] pry(#<AppsController>)> AppSearchContract.new.call(search_params)
=> #<Dry::Validation::Result{:search=>"", :current_user_id=>10000, :page=>1, :users=>"[10001]", :per_page=>24} errors={:users=>["must be an array"]}>
[10] pry(#<AppsController>)> search_params["users"] = "10001"
=> "10001"
[11] pry(#<AppsController>)> search_params
=> {"search"=>"", "users"=>"10001", "page"=>"1", "current_user_id"=>10000}
[12] pry(#<AppsController>)> AppSearchContract.new.call(search_params)
=> #<Dry::Validation::Result{:search=>"", :current_user_id=>10000, :page=>1, :users=>"10001", :per_page=>24} errors={:users=>["must be an array"]}>
[13] pry(#<AppsController>)> search_params["users"] = "10001,10002"
=> "10001,10002"
[14] pry(#<AppsController>)> AppSearchContract.new.call(search_params)
=> #<Dry::Validation::Result{:search=>"", :current_user_id=>10000, :page=>1, :users=>"10001,10002", :per_page=>24} errors={:users=>["must be an array"]}>
[15] pry(#<AppsController>)> search_params["users"] = "[10001,10002]"
=> "[10001,10002]"
[16] pry(#<AppsController>)> AppSearchContract.new.call(search_params)
=> #<Dry::Validation::Result{:search=>"", :current_user_id=>10000, :page=>1, :users=>"[10001,10002]", :per_page=>24} errors={:users=>["must be an array"]}>
[17] pry(#<AppsController>)> search_params["users"] = 10001
=> 10001
[18] pry(#<AppsController>)> AppSearchContract.new.call(search_params)
=> #<Dry::Validation::Result{:search=>"", :current_user_id=>10000, :page=>1, :users=>10001, :per_page=>24} errors={:users=>["must be an array"]}>
[19] pry(#<AppsController>)> search_params["users"] = [10001]
=> [10001]
[20] pry(#<AppsController>)> AppSearchContract.new.call(search_params)
=> #<Dry::Validation::Result{:search=>"", :current_user_id=>10000, :page=>1, :users=>[10001], :per_page=>24} errors={}>

To Reproduce

require 'bundler/inline'

gemfile do
  source "https://rubygems.org"
  gem 'dry-schema'
  gem 'dry-validation'
end

my_schema = Dry::Schema.Params do
  required(:users).value(:array)
end

params = {
  users: "1,2,3,4"
}

puts "params[:users] is #{params[:users].inspect}:#{params[:users].class}"
result = my_schema.call(params)
puts result.errors.to_h

params[:users] = [1,2,3,4]
puts "params[:users] is #{params[:users].inspect}:#{params[:users].class}"
result = my_schema.call(params)
puts result.errors.to_h

Expected behavior

I would expect strings like "1,2,3,4" or "[1,2,3,4]" to be coerced into an Array instead, the actual format supported is to be discussed but the current behavior effectively performs no coercion.

My environment

flash-gordon commented 3 years ago

It is expected. "Params" types intend to work with rack params, they can encode ruby arrays when names are passed as user_id[]=1&user_id[]=2. In your case, I'd build a custom constructor type that coerces data to the expected output. Changing params type to support custom encoding doesn't look justified FWIW.