Array with OR doesn't coerce

[madejejej]

Describe the bug

If you validate a nested schema inside an array and use an OR, type coercion doesn't work for the inner schema.

To Reproduce

it "returns success for valid input" do
  first_schema = Dry::Schema.Params do
    required(:event).filled(Types::String.enum("1"))
    required(:name).filled(:string)
    required(:timestamp).filled(:time)
  end

  second_schema = Dry::Schema.Params do
    required(:event).filled(Types::String.enum("2"))
    required(:name).filled(:string)
  end

  or_schema = Dry::Schema.Params do
    required(:events).array do
      hash(first_schema | second_schema)
    end
  end

  non_or_schema = Dry::Schema.Params do
    required(:events).array do
      hash(first_schema)
    end
  end

  input = { events: [{event: "1", name: "Hello", timestamp: "2021-11-30T16:22:58+00:00"}] }

  expect(non_or_schema.(input)).to be_success
  expect(or_schema.(input)).to be_success
end

Failures:

  1) Dry::Schema OR messages with complex multiple schemas with inner nestings returns success for valid input
     Failure/Error: expect(or_schema.(input)).to be_success
       expected `#<Dry::Schema::Result{:events=>[{:event=>"1", :name=>"Hello", :timestamp=>"2021-11-30T16:22:58+00:00"...={:events=>{0=>{:or=>[{:timestamp=>["must be a time"]}, {:event=>["must be one of: 2"]}]}}} path=[]>.success?` to be truthy, got false
     # ./spec/integration/schema/or_spec.rb:252:in `block (3 levels) in <top (required)>'

Expected behavior

Validation should be successful. events.0.timestamp should be coerced to Time.

My environment

• Affects my production application: YES
• Ruby version: 2.7.2
• OS: -

[madejejej]

The issue seems to go away when I swap the enum type to a string in both schemas 🤔

      first_schema = Dry::Schema.Params do
        required(:event).filled(:string)
        required(:name).filled(:string)
        required(:timestamp).filled(:time)
      end

      second_schema = Dry::Schema.Params do
        required(:event).filled(:string)
        required(:name).filled(:string)
      end

[madejejej]

Possible duplicate of #316 ?

[robisenberg]

Update: see my followup message for a correction, the workaround seems to not be doing any type-checking

I'm also experiencing the same issue, though I have a simpler test case and a bit more details on the issue (including what I believe is a workaround).

Workaround: use type instead of filled

It seems the type coercion for coercible types like :time, :date_time etc don't work properly in this scenario if you use the filled method, but do work if you use the type method instead (I am not familiar with the code, so don't know why that would be the case).

✅ Using .type (working)

# WORKING
Schema1 = Dry::Schema.Params do
  required(:date1).type(:date_time)
end

Schema2 = Dry::Schema.Params do
  required(:date2).type(:date_time)
end

Container = Dry::Schema.Params do
  optional(:foo).array { schema(Schema1 | Schema2) }
end

puts Container.(foo: [{ date1: "2022-08-15T15:21:45.000Z" }]).inspect
#<Dry::Schema::Result{:foo=>[{:date1=>"2022-08-15T15:21:45.000Z"}]} errors={} path=[]>

puts Container.(foo: [{ date2: "2022-08-15T15:21:45.000Z" }]).inspect
#<Dry::Schema::Result{:foo=>[{:date2=>"2022-08-15T15:21:45.000Z"}]} errors={} path=[]>

puts Container.(foo: [{}]).inspect
#<Dry::Schema::Result{:foo=>[{}]} errors={:foo=>{0=>{:or=>[{:date1=>["is missing"]}, {:date2=>["is missing"]}]}}} path=[]>

❌ Using .filled (not working)

# NOT WORKING
Schema1 = Dry::Schema.Params do
  required(:date1).filled(:date_time)
end

Schema2 = Dry::Schema.Params do
  required(:date2).filled(:date_time)
end

Container = Dry::Schema.Params do
  optional(:foo).array { schema(Schema1 | Schema2) }
end

# Should have no errors, but does...
puts Container.(foo: [{ date1: "2022-08-15T15:21:45.000Z" }]).inspect
#<Dry::Schema::Result{:foo=>[{:date1=>"2022-08-15T15:21:45.000Z"}]} errors={:foo=>{0=>{:or=>[{:date1=>["must be a date time"]}, {:date2=>["is missing"]}]}}} path=[]>

# Should have no errors, but does...
puts Container.(foo: [{ date2: "2022-08-15T15:21:45.000Z" }]).inspect
#<Dry::Schema::Result{:foo=>[{:date2=>"2022-08-15T15:21:45.000Z"}]} errors={:foo=>{0=>{:or=>[{:date1=>["is missing"]}, {:date2=>["must be a date time"]}]}}} path=[]>

puts Container.(foo: [{}]).inspect
#<Dry::Schema::Result{:foo=>[{}]} errors={:foo=>{0=>{:or=>[{:date1=>["is missing"]}, {:date2=>["is missing"]}]}}} path=[]>

---

A few other points @madejejej:

1. In your example, changing:

   required(:timestamp).filled(:time)
   

   to:

   required(:timestamp).type(:time)
   

   works as expected for me.

2. I don't believe this issue is a duplicate of #316 because the issue there is that it doesn't work when calling the form with string keys (e.g. Container.("foo" => ...) but it works when using symbols. In our case we're calling with symbols rather than strings, so shouldn't be connected.

3. As you noted, this works fine if we change the type declaration to use :string, but I think that's to be expected since that isn't a type that needs coercion (unlike the problem types of :time or :date_time).

[robisenberg]

So, in fact, my suggested workaround of using .type instead of .filled actually doesn't work because it is not validating that what you pass in the field is actually a valid :date_time string; instead it will accept anything as valid.

Schema1 = Dry::Schema.Params do
  required(:date1).type(:date_time)
end

Schema2 = Dry::Schema.Params do
  required(:date2).type(:date_time)
end

Container = Dry::Schema.Params do
  optional(:foo).array { schema(Schema1 | Schema2) }
end

# should give an error, but doesn't...
puts Container.(foo: [{ date1: "NOT A DATE" }]).inspect
#<Dry::Schema::Result{:foo=>[{:date1=>"NOT A DATE"}]} errors={} path=[]>

So, here's the simplest failure case:

# NOT WORKING
Schema1 = Dry::Schema.Params do
  required(:date1).filled(:date_time)
end

Schema2 = Dry::Schema.Params do
  required(:date2).filled(:date_time)
end

Container = Dry::Schema.Params do
  optional(:foo).array { schema(Schema1 | Schema2) }
end

# Should have no errors, but does...
puts Container.(foo: [{ date1: "2022-08-15T15:21:45.000Z" }]).inspect
#<Dry::Schema::Result{:foo=>[{:date1=>"2022-08-15T15:21:45.000Z"}]} errors={:foo=>{0=>{:or=>[{:date1=>["must be a date time"]}, {:date2=>["is missing"]}]}}} path=[]>

# Should have no errors, but does...
puts Container.(foo: [{ date2: "2022-08-15T15:21:45.000Z" }]).inspect
#<Dry::Schema::Result{:foo=>[{:date2=>"2022-08-15T15:21:45.000Z"}]} errors={:foo=>{0=>{:or=>[{:date1=>["is missing"]}, {:date2=>["must be a date time"]}]}}} path=[]>

# Correctly gives an error
puts Container.(foo: [{}]).inspect
#<Dry::Schema::Result{:foo=>[{}]} errors={:foo=>{0=>{:or=>[{:date1=>["is missing"]}, {:date2=>["is missing"]}]}}} path=[]>

# When working, this should also give an error
puts Container.(foo: [{ date1: "NOT A DATE" }]).inspect

[solnic]

This works in 1.10.1:

require "dry/schema"

Schema1 = Dry::Schema.Params do
  required(:date1).value(:date_time)
end

Schema2 = Dry::Schema.Params do
  required(:date2).value(:date_time)
end

Container = Dry::Schema.Params do
  optional(:foo).array(:hash, Schema1 | Schema2)
end


puts Container.(foo: [{ date1: "2022-08-15T15:21:45.000Z" }]).inspect
#<Dry::Schema::Result{:foo=>[{:date1=>#<DateTime: 2022-08-15T15:21:45+00:00 ((2459807j,55305s,0n),+0s,2299161j)>}]} errors={} path=[]>

puts Container.(foo: [{ date2: "2022-08-15T15:21:45.000Z" }]).inspect
#<Dry::Schema::Result{:foo=>[{:date2=>#<DateTime: 2022-08-15T15:21:45+00:00 ((2459807j,55305s,0n),+0s,2299161j)>}]} errors={} path=[]>

puts Container.(foo: [{}]).inspect
#<Dry::Schema::Result{:foo=>[{}]} errors={:foo=>{0=>{:or=>[{:date1=>["is missing"]}, {:date2=>["is missing"]}]}}} path=[]>

puts Container.(foo: [{ date1: "NOT A DATE" }]).inspect
#<Dry::Schema::Result{:foo=>[{:date1=>"NOT A DATE"}]} errors={:foo=>{0=>{:or=>[{:date1=>["must be a date time"]}, {:date2=>["is missing"]}]}}} path=[]>

I reckon this is expected behavior. If you skip array(:hash) part, then there's no type spec, and w/o it, coercion is not enabled. This is by design and it will be mandatory to provide type specs in 2.0.0. This is reason number 95132512 why type specs should've been mandatory right from the start 😢