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

Composable contracts #593

Open
3 of 7 tasks
ianwhite opened this issue Oct 6, 2019 · 22 comments
Open
3 of 7 tasks

Composable contracts #593

ianwhite opened this issue Oct 6, 2019 · 22 comments
Labels
⚠️ experimental ⚠️ Experimental features that may change before major version bump feature wip

Comments

@ianwhite
Copy link
Collaborator

ianwhite commented Oct 6, 2019

This feature will allow contracts to be composed of other contracts, optionally mounted at input paths.

Example

# this contract defines its own params and rules, but also uses the
# schema/rules of CustomerContract, and AddressContract on the input at :address
# via the #contract and #path methods
class OrderContract < ApplicationContract
  params do
    required(:accept_terms).value(:bool)
  end

  rule(:accept_terms).validate(:acceptance)

  contract CustomerContract

  path :address do
    contract AddressContract
  end
end

Implementation

  • Initial on branch composable-contracts

    • Create a :composable extension
    • implement new #contract feature in the :composable extension
    • Add #path block DSL
    • Rewrite ResultSet as extension to Result
    • Rewrite the contract/call_spec integration test using the desired new syntax
    • Add an example
    • Document the changes in docsite
  • Future work

    • Decide on exact semantics for merging result values/errors if its unclear
    • Ensure it works wrt: concurrency
    • Figure out if we should integrate with other extensions in the short term
    • Decide on a syntax so contracts can be applied to nested arrays of input
    • Decide on a syntax so contracts can be conditionally be applied depending on the input
    • If consensus that the API is good, remove the :composable extension and rewrite contract and result to include the new functionality by default

Resources

@ianwhite ianwhite added feature wip ⚠️ experimental ⚠️ Experimental features that may change before major version bump labels Oct 6, 2019
ianwhite added a commit that referenced this issue Oct 7, 2019
ianwhite added a commit that referenced this issue Oct 7, 2019
@solnic
Copy link
Member

solnic commented Oct 8, 2019

Feel free to open a draft PR early if you'd like to get some feedback sooner.

@ianwhite
Copy link
Collaborator Author

ianwhite commented Oct 8, 2019

Will do, when the next item (simplify ResultSet) is checked off.

@ianwhite
Copy link
Collaborator Author

Closing in favour of #595

@cgeorgii
Copy link

I think this issue should stay open until it's resolved, because I believe other people would also like to get notified whenever it gets solved and if it's open it won't be forgotten (hopefully).

@ianwhite
Copy link
Collaborator Author

Hi, I'm happy to do that. I'm hoping to get some time to work on the new plan outlined in #595 over the Xmas break.

@jameskbride
Copy link

Hi @ianwhite and @solnic! First I just wanted to take a moment and thank you and the rest of the devs involved in this project for all of your efforts. It's really appreciated, and I know how hard it is to maintain projects like these.

That said I was hoping to find out what the state of this feature request is? I see from the threads that some of the work has shifted to dry-struct and dry-schema, and that work appears to have been completed, at least in part with the changes from dry-rb/dry-struct#139 and dry-rb/dry-schema#256. I also see that #595 has been closed, presumably with the intention of extracting some of the logic from dry-schema and dry-struct. Is that still the plan, and is this feature still viable?

I'd be willing to take a look and see what needs to be be changed, but I might need some direction. Let me know when you get a chance and thanks again for your work here!

@solnic
Copy link
Member

solnic commented Mar 26, 2020

@jameskbride hey 😄 this is on hold but I think @ianwhite is still interested in working on this, so please try to sync up with him first.

@ianwhite
Copy link
Collaborator Author

ianwhite commented Mar 26, 2020 via email

@bilby91
Copy link

bilby91 commented Apr 16, 2020

@ianwhite @jameskbride I'm really interested in this feature as well and probably will have some bandwidth to help on this!

@anmacagno
Copy link

I am composing contracts in this way. What do you think?

class EmployeeContract < Dry::Validation::Contract
  json do
    required(:concepts).array(:hash)
  end

  rule(:concepts).each do
    result = ConceptContract.new.call(value)
    unless result.success?
      meta_hash = { text: 'invalid concept' }.merge(result.errors.to_h)
      key.failure(meta_hash)
    end
  end
end

@solnic
Copy link
Member

solnic commented May 21, 2020

@anmacagno I think it's perfectly fine to do it like that for the time being, but eventually we definitely want a dedicated feature that would make it easier

@ianwhite
Copy link
Collaborator Author

ianwhite commented May 21, 2020 via email

tadeusz-niemiec pushed a commit to tadeusz-niemiec/dry-schema that referenced this issue May 28, 2020
@tak1n
Copy link

tak1n commented Jan 28, 2021

@anmacagno thx for ur snippet. Just a short extension, when using array(:hash) any key coming inside concepts json will go through. Therefore we use the schema of the child contract in the parent contract schema.

class EmployeeContract < Dry::Validation::Contract
  json do
    required(:concepts).array(ConceptContract.schema)
  end

  rule(:concepts).each do
    result = ConceptContract.new.call(value)
    unless result.success?
      meta_hash = { text: 'invalid concept' }.merge(result.errors.to_h)
      key.failure(meta_hash)
    end
  end
end

@tbsvttr
Copy link

tbsvttr commented Feb 17, 2021

@solnic This feature would much improve this already great library!

@solnic
Copy link
Member

solnic commented Feb 18, 2021

@tbsvttr ah thanks, I know it's long-time coming but we'll get to implement it eventually 🙂

@tbsvttr
Copy link

tbsvttr commented Feb 22, 2021

@tak1n While your solution works very well it has the downside that now each error hash has this text key value pair added which neither adds very useful additional information and also could lead to the confusion that the validated field in a hash has a field named text. Is there a way to get rid of it?

@jacobtani
Copy link

Hi- are we getting closer to allowing contracts to be reused within another contract?

@solnic
Copy link
Member

solnic commented Apr 19, 2021

@jacobtani not really :( currently it doesn't seem like there's anybody who'd have time to work on this feature. Like I mentioned, we'll get to this eventually. It's on my radar and it's going to be my priority after we're done with Hanami 2.0 (FWIW).

@johanlunds
Copy link

johanlunds commented Oct 25, 2021

This is my improvement to @tak1n's and @anmacagno's solutions:

Dry::Validation.register_macro(:contract) do |macro:|
  contract_instance = macro.args[0]
  contract_result = contract_instance.(value)
  unless contract_result.success?
    # Generate a message string instead of adding a
    # dummy 'text' value.  I don't love this, and I
    # wonder if it's possible to improve this.
    msg = flattened_error_messages(
      contract_result.errors.to_h
    ).join(', ')
    key.failure(msg)
  end
end

def flattened_error_messages(errors, path = [])
  errors.each_with_object([]) do |(key, value), msgs|
    case value
    when Array
      value.each { |v| msgs << "#{(path + [key]).join('.')} #{v}" }
    when Hash
      msgs.concat(flattened_error_messages(value, path + [key]))
    end
  end
end

# ...

class MyContract < Dry::Validation::Contract
  params do
    # hash:
    required(:a).hash(A.schema)
    # array of hashes:
    required(:b).array(:hash, B.schema)
  end

  rule(:a).validate(contract: A)
  rule(:b).each(contract: B)
end

@johanlunds
Copy link

johanlunds commented Oct 25, 2021

Updated variant, now with proper error message nesting. Full solution:

# Use it like this:
#
#     MyContract = Dry::Validation.Contract do
#       params do
#         # for a hash:
#         required(:a).hash(OtherContract.schema)
#
#         # for an array of hashes:
#         required(:b).array(:hash, OtherContract.schema)
#       end
#
#       rule(:a).validate(contract: OtherContract)
#       rule(:b).each(contract: OtherContract)
#     end
#
Dry::Validation.register_macro(:contract) do |macro:|
  contract_instance = macro.args[0]
  contract_result = contract_instance.(value)
  unless contract_result.success?
    errors = contract_result.errors
    errors.each do |error|
      key(key.path.to_a + error.path).failure(error.text)
    end
  end
end

@zavan
Copy link

zavan commented Nov 16, 2022

If you're looking to just merge schemas at the top level like myself, looks like you can do this:

class MyContract < Dry::Validation::Contract
  params(MyOtherContract.schema, MyThirdContract.new(args).schema) do
    required(:a).value(:string)
  end
end

This is in the docs, but I had missed it.

If you need the rules you can copy them over too:

my_contract.rules = my_other_contract.rules + my_third_contract.rules

@ssoulless
Copy link

Updated variant, now with proper error message nesting. Full solution:

# Use it like this:
#
#     MyContract = Dry::Validation.Contract do
#       params do
#         # for a hash:
#         required(:a).hash(OtherContract.schema)
#
#         # for an array of hashes:
#         required(:b).array(:hash, OtherContract.schema)
#       end
#
#       rule(:a).validate(contract: OtherContract)
#       rule(:b).each(contract: OtherContract)
#     end
#
Dry::Validation.register_macro(:contract) do |macro:|
  contract_instance = macro.args[0]
  contract_result = contract_instance.(value)
  unless contract_result.success?
    errors = contract_result.errors
    errors.each do |error|
      key(key.path.to_a + error.path).failure(error.text)
    end
  end
end

This should be a built-in feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ experimental ⚠️ Experimental features that may change before major version bump feature wip
Projects
None yet
Development

No branches or pull requests