Inconsistent behavior in nested schemas via passed in blocks

[stevenhan2]

Describe the bug

When schemas are passed blocks, required field behavior does not always work, and there is inconsistent behavior for reaching validation rules -- although it seems to correlate with whether the required is evaluated.

To Reproduce

class TireSchema < Dry::Schema::JSON
    define do
        optional(:tread).value(:string, included_in?: %w(offroad allterrain sport road))
        optional(:size).value(:integer)
    end
end

class SportsCarSchema < Dry::Schema::JSON
    define do
        optional(:recommended_tire).hash.schema(TireSchema.new) do
            optional(:tread).value(:string, included_in?: %w(sport road))
        end
    end
end

class RaceDriverContract < Dry::Validation::Contract
    rule('car.recommended_tire.size') do
        puts "The runtime made it to the rule!"
    end

    json do
        required(:car).hash.schema(SportsCarSchema.new) do
            required(:recommended_tire).hash.schema(TireSchema.new) do
                required(:size).value(:integer)
            end
        end
    end
end

contract = RaceDriverContract.new

contract.call({ car: { recommended_tire: { tread: "invalid value" } } })
# The runtime made it to the rule!
# => #<Dry::Validation::Result{:car=>{:recommended_tire=>{:tread=>"invalid value"}}} errors={:car=>{:recommended_tire=>{:tread=>["must be one of: offroad, allterrain, sport, road"]}}}>

contract.call({ car: { recommended_tire: { tread: "sport" } } })
# => #<Dry::Validation::Result{:car=>{:recommended_tire=>{:tread=>"sport"}}} errors={:car=>{:recommended_tire=>{:size=>["is missing"]}}}>

contract.call({ car: { recommended_tire: { tread: "allterrain" } } })
# The runtime made it to the rule!
# => #<Dry::Validation::Result{:car=>{:recommended_tire=>{:tread=>"allterrain"}}} errors={:car=>{:recommended_tire=>{:tread=>["must be one of: sport, road"]}}}>

Expected behavior

I would expect every one of the above results to contain {:recommended_tire=>{:size=>["is missing"]}, and I would either expect all of them to make it to the rule or none of them to make it to the rule.

My environment

• Affects my production application: YES
• Ruby version: 2.7.2
• OS: macOS 12.0.1 21A559

[solnic]

Does it work when you do .hash instead of .hash.schema? Why do you do .hash.schema btw? I understand that it's possible but maybe it shouldn't be if it causes issues.

[stevenhan2]

Does it work when you do .hash instead of .hash.schema? Why do you do .hash.schema btw? I understand that it's possible but maybe it shouldn't be if it causes issues.

No 😞 -- replacing all .hash.schema with .hash in the example (three locations) did not change anything.

[solnic]

OK thanks, clearly a bug. Thanks for reporting.

[marcocarvalho]

👍

[marcocarvalho]

Same issue, I tried some iterations over the same error: https://github.com/marcocarvalho/dry-validation-playground

[marcocarvalho]

Schema = Dry::Schema.JSON do
  optional(:external_id).value(:string, max_size?: 64)
  optional(:first_name).value(:string, max_size?: 255)
  optional(:last_name).value(:string, max_size?: 255)
  optional(:type).value(:string, max_size?: 32)
  optional(:birthdate).value(:string, format?: /\A(-?(?:[1-9][0-9]*)?[0-9]{4})-(1[0-2]|0[1-9])-(3[0-1]|0[1-9]|[1-2][0-9])\Z/)
end

OnlyCommonFieldsSchema = Dry::Schema.JSON do
  optional(:last_name).value(:string, max_size?: 255)
  optional(:type).value(:string, max_size?: 32)
  optional(:birthdate).value(:string, format?: /\A(-?(?:[1-9][0-9]*)?[0-9]{4})-(1[0-2]|0[1-9])-(3[0-1]|0[1-9]|[1-2][0-9])\Z/)
end

class Contract < Dry::Validation::Contract
  json do
    required(:person).hash do
      required(:external_id).value(:string, max_size?: 64)
      required(:first_name).value(:string, max_size?: 255)
      optional(:last_name).value(:string, max_size?: 255)
      optional(:type).value(:string, max_size?: 32)
      optional(:birthdate).value(:string, format?: /\A(-?(?:[1-9][0-9]*)?[0-9]{4})-(1[0-2]|0[1-9])-(3[0-1]|0[1-9]|[1-2][0-9])\Z/)
    end
  end
end

class CommonFieldsContract < Dry::Validation::Contract
  json do
    required(:person).hash(OnlyCommonFieldsSchema) do
      required(:external_id).value(:string, max_size?: 64)
      required(:first_name).value(:string, max_size?: 255)
    end
  end
end

class InheritSchemaContract < Dry::Validation::Contract
  json do
    required(:person).hash(Schema) do
      required(:external_id).value(:string, max_size?: 64)
      required(:first_name).value(:string, max_size?: 255)
    end
  end
end

params = {
   person: {
      last_name: "x"*256,
      type: "u"*33,
      birthdate: "invalid"
   }
}

puts InheritSchemaContract.new.call(params).errors.inspect
puts CommonFieldsContract.new.call(params).errors.inspect
puts Contract.new.call(params).inspect

InheritSchemaContract and CommonFieldsContract will not return external_id and first_name missing key error, but Contract will. At least InheritSchemaContract and CommonFieldsContract have consistent errors :) .

[marcocarvalho]

@solnic, this breaks the reusability of schemas. Any chance this could be worked on?

[danperes97]

:up: