dry-schema icon indicating copy to clipboard operation
dry-schema copied to clipboard

Options with default values

Open HoneyryderChuck opened this issue 6 years ago • 15 comments

I'm trying to use this lib for schema validation, and there is this use case where a key needs to be filled with a default, when nothing is passed.

I know that this wasn't in the scope of dry-validation, but that this is dry-schema, maybe the use case now fits.

Something like:

optional(:op_type).default("read")

HoneyryderChuck avatar Feb 23 '19 16:02 HoneyryderChuck

Actually, I may now add it to dry-validation. I need to think about it some more though. If this gets added here, it'll be a plugin.

@timriley @flash-gordon what's your opinion on this feature? My thinking is that it's a domain concern, so should live in objects that are part of the domain layer, and dry-v contracts will be something like this, so...

solnic avatar Feb 23 '19 20:02 solnic

I've been starting to using this lib for json schema validation. The spec allows the definition of a default for a field, see: https://stackoverflow.com/questions/7011425/how-do-you-use-the-json-schema-default-attribute-in-json-net. If this is the goal of the lib, to implement some json schema validation, it'd be sweet to have it 😊

HoneyryderChuck avatar Feb 24 '19 11:02 HoneyryderChuck

@solnic It can be useful, especially there's no established way of providing defaults for deeply nested structures. I would use default types for that under the hood, as in, required(:op_type).value(Types::Integer.default(0)). The issue is the same as https://github.com/dry-rb/dry-schema/issues/44 and I suggested using default types there as well, not sure what you meant by coercion empty strings to Undefined there, though.

flash-gordon avatar Feb 24 '19 17:02 flash-gordon

yeah this should be handled by types, even if we decide to add a default("foo") shortcut

not sure what you meant by coercion empty strings to Undefined there, though.

Previously, nil was used by Types::Default, now we need Undefined, so to trigger default values, we'd have to have a special coercion type that doesn't cast empty strings to nil (current Params::Nil behavior), and instead we need to get "" => Undefined.

solnic avatar Feb 24 '19 21:02 solnic

but Params::Nil is already a type so if you want to make work like "" => Undefined you'll need to do it explicitly, without default types at all. My suggestion is only about absent keys

flash-gordon avatar Feb 25 '19 12:02 flash-gordon

I think we want a new type for this. With absent keys we can fill them in for each key that has a default value.

solnic avatar Feb 25 '19 14:02 solnic

An alternative to using types is to use a callback:

require 'dry/schema'

class Dry::Schema::Macros::DSL
  def default(value)
    schema_dsl.before(:rule_applier) do |result|
      result.update(name => value) unless result[name]
    end
  end
end

schema = Dry::Schema.Params do
  optional(:op_type).filled(:string).default('read')
end

puts schema.({}).inspect
# #<Dry::Schema::Result{:op_type=>"read"} errors={}>

This could be properly implemented because all of the ingridients are available. Schemas track full paths to individual keys (including nested schemas), the DSL could gather all the defaults that you want to set and then turn that into either a callback or a 1st-class processor step that would be applied before rules are applied.

I think the actual trick is to properly determine (or specify) when defaults should be applied. ie when a key is not present? when a value is empty? when a value is nil? etc.

solnic avatar Mar 12 '20 10:03 solnic

@solnic, did you mean result.update(name => value) unless result[name] 🙂?

irastypain avatar Apr 30 '20 09:04 irastypain

@irastypain oops, yes I did. Just updated this example. Thanks for spotting it :)

solnic avatar May 16 '20 09:05 solnic

@solnic I think you'll want to update that DSL extension recommendation to

result.update(name => value) if result.output && !result[name]

Otherwise it'll break nested paths because output will be nil and result#[] doesn't nil check here. Example:

# with the `result.update(name => value) unless result[name]` in the DSL `default` ext
class Contract < Dry::Validation::Contract
  params do
    required(:data).hash do
      required(:foo).maybe(:bool).default(false)
    end
  end
end

Contract.new.call({ 'bad_param' => true })
#=> NoMethodError Exception: undefined method `[]' for nil:NilClass

EDIT

Actually, this extension might just break nesting in general... Even with that change ☝️ I, might avoid a single nesting error, but it still breaks on the reduce in result. 😕

class Dry::Schema::Macros::DSL
  def default(value)
    schema_dsl.before(:rule_applier) do |result|
      result.update(name => value) if result.output && !result[name]
    end
  end
end

class Contract < Dry::Validation::Contract
  params do
    required(:data).hash do
      required(:foo).maybe(:bool).default(false)
    end
  end
end

class Contract2 < Dry::Validation::Contract
  params do
    required(:data).hash do
      required(:object).hash do # doubly nested data
        required(:foo).maybe(:bool).default(false)
      end
    end
  end
end

Contract.new.call({ 'bad_param' => true })
#=> #<Dry::Validation::Result{:data=>{}} errors={:data=>{:foo=>["is missing"]}}>

Contract2.new.call({ 'bad_param' => true })
# NoMethodError: undefined method `[]' for nil:NilClass
# path/to/gems/dry-schema-1.5.0/lib/dry/schema/result.rb:52:in `block in at'

wspurgin avatar Jun 05 '20 14:06 wspurgin

@solnic This last example provided by @wspurgin is working on dry-schema (1.13.0).

Can we considerate push this to next version?

jmillandev avatar Jul 05 '23 23:07 jmillandev

We could add it as an experimental extension for now and see what happens 🙂

solnic avatar Jul 07 '23 10:07 solnic

The method is useful, but it does not work if optional nested blocks are not passed, and when calling default, you need to build them and set the default value. The second point is that I would like to pass the value from the contract constructor to the method, its parameters during initialization. But DSL just doesn't see them, unfortunately

XenosRbel avatar Dec 11 '23 11:12 XenosRbel