diff --git a/lib/dry/operation/extensions/validation.rb b/lib/dry/operation/extensions/validation.rb index 1c55123..c98e7c0 100644 --- a/lib/dry/operation/extensions/validation.rb +++ b/lib/dry/operation/extensions/validation.rb @@ -13,9 +13,45 @@ module Extensions # # When this extension is included, define your contract on your operation class using # `params`, `schema`, or `contract`, or make a `#contract` dependency available from your - # operation instance. + # operation instance. The operation's input is validated before its method body runs, and the + # method receives the contract's output in place of the input it was called with. # - # @see https://dry-rb.org/gems/dry-validation/ + # The input is the last positional argument (when positional args are present), or the keyword + # arguments in full. Both of these validate `{name: "Alice"}`: + # + # ``` + # operation.call(name: "Alice") + # operation.call({name: "Alice"}) + # ``` + # + # When the input is given as a positional argument, any earlier positional arguments and any + # keyword arguments are considered the operation's own. They never pass through the validation + # contract, and are never set by the contract's output. Use these for arguments that control + # the operation's own behavior alongside its validated input. + # + # ``` + # class UpdateUser < Dry::Operation + # include Dry::Operation::Extensions::Validation + # + # params do + # required(:name).filled(:string) + # end + # + # def call(id, attrs, notify: false) + # step persist(id, attrs) + # end + # end + # + # update_user = UpdateUser.new + # update_user.call(123, {name: "Alice", admin: true}, notify: true) + # # id is 123 - not passed through the contract + # # attrs is {name: "Alice"} - admin filtered out by the contract + # # notify is true - not passed through the contract + # ``` + # + # When there is no contract defined, all arguments are forwarded untouched. + # + # @see https://hanakai.org/learn/dry/dry-validation # # @api public # @since 1.2.0 @@ -168,60 +204,22 @@ def name private - # rubocop:disable Metrics/PerceivedComplexity def define_validation_method - # Cache named kwargs outside the method closure so we only search for them once. - named_kwargs = nil - find_named_kwargs = method(:find_named_kwargs) - - define_method(@method_name) do |input = {}, *rest, **kwargs, &block| - use_kwargs = !kwargs.empty? && input.empty? && rest.empty? - actual_input = use_kwargs ? kwargs : input - - validation_result = validate(actual_input) - - case validation_result - when Dry::Monads::Success - validated_input = validation_result.value! - - if use_kwargs - # Ensure named kwargs from the wrapped method are still passed through even if - # they are not in the validation output. This is important for kwargs that exist - # to serve the method's own logic, separate to the scope of validatable input. - named_kwargs ||= find_named_kwargs.call(method(__method__).super_method) - passthrough_keys = actual_input - .slice(*named_kwargs) - .reject { |k, _| validated_input.key?(k) } - validated_input = passthrough_keys.merge(validated_input) - - super(**validated_input, &block) - else - super(validated_input, *rest, **kwargs, &block) - end - when Dry::Monads::Failure - throw_failure(validation_result) + define_method(@method_name) do |*args, **kwargs, &block| + # Without a contract there's nothing to validate, so the arguments the method was + # called with are forwarded untouched. + return super(*args, **kwargs, &block) unless contract + + if args.empty? + # The keyword arguments are the input. + super(**step(validate(kwargs)), &block) + else + # The last positional argument is the input. Pass through all other args. + *rest, input = args + super(*rest, step(validate(input)), **kwargs, &block) end end end - # rubocop:enable Metrics/PerceivedComplexity - - NAMED_KWARG_TYPES = %i[key keyreq].freeze - - def find_named_kwargs(method) - # Walk up the method chain to find the first method with named kwargs. - while method - named_kwargs = method - .parameters - .select { |type, _| NAMED_KWARG_TYPES.include?(type) } - .map(&:last) - - return named_kwargs if named_kwargs.any? - - method = method.super_method - end - - [] - end end end end diff --git a/spec/integration/extensions/validation_spec.rb b/spec/integration/extensions/validation_spec.rb index 772a7d3..0ac33d7 100644 --- a/spec/integration/extensions/validation_spec.rb +++ b/spec/integration/extensions/validation_spec.rb @@ -375,18 +375,18 @@ def call(input) end end - describe "selective validation with unvalidated keys preserved" do - it "preserves unvalidated named kwargs and coerces validated values" do + describe "arguments passed through alongside validated input" do + it "passes through keyword arguments when input given as a positional argument" do calculate = Class.new(Dry::Operation) do include Dry::Operation::Extensions::Validation params do - # Validate and coerce x and y, but not operation + # Validate and coerce x and y. `operation` is not input, so it isn't here. required(:x).value(:integer) required(:y).value(:integer) end - def call(operation:, **values) + def call(values, operation:) # operation is passed through (not validated) # x and y are coerced from strings to integers result = case operation @@ -398,14 +398,14 @@ def call(operation:, **values) end end - # x and y are coerced to integers, operation is preserved as symbol - result = calculate.new.call(operation: :add, x: "10", y: "20") + # x and y are coerced to integers, operation is preserved as a symbol + result = calculate.new.call({x: "10", y: "20"}, operation: :add) expect(result).to be_success expect(result.value!).to eq(operation: :add, x: 10, y: 20, result: 30) end - it "filters out invalid keys in splat args while preserving named kwargs" do + it "passes through leading positional arguments when input given as a final positional argument" do update_user = Class.new(Dry::Operation) do include Dry::Operation::Extensions::Validation @@ -415,20 +415,149 @@ def call(operation:, **values) # Deliberately doesn't allow :admin - should be filtered end - def call(id:, **attrs) - # id should be preserved (named kwarg) - # admin should be filtered (not in contract, and not a named kwarg) + def call(id, attrs) + # id is a parameter, so it's untouched by the contract + # admin should be filtered (not allowed by the contract) attrs.merge(id: id) end end - # User tries to sneak in admin: true - result = update_user.new.call(id: 123, name: "Alice", admin: true) + # User tries to sneak in `id: 456` and `admin: true` + result = update_user.new.call(123, {name: "Alice", id: 456, admin: true}) expect(result).to be_success - # admin should NOT be present - contract filtered it + # id and admin from the input are filtered out by the contract expect(result.value!).to eq(id: 123, name: "Alice") expect(result.value!).not_to have_key(:admin) end + + it "passes through both leading positional arguments as well keyword arguments" do + operation = Class.new(Dry::Operation) do + include Dry::Operation::Extensions::Validation + + params do + required(:name).filled(:string) + end + + def call(suffix, attrs, upcase: false) + name = "#{attrs[:name]}#{suffix}" + + upcase ? name.upcase : name + end + end + + result = operation.new.call("!", {name: "Alice"}, upcase: true) + + expect(result).to eq(Success("ALICE!")) + end + end + + describe "operations taking no input" do + it "calls a method taking no arguments when no contract is defined" do + operation = Class.new(Dry::Operation) do + include Dry::Operation::Extensions::Validation + + def call + step build_thing + end + + private + + def build_thing + Success(:thing) + end + end + + expect(operation.new.call).to eq(Success(:thing)) + end + + it "forwards arguments untouched when no contract is defined" do + operation = Class.new(Dry::Operation) do + include Dry::Operation::Extensions::Validation + + def call(input, extra: nil) + [input, extra] + end + end + + expect(operation.new.call(:input, extra: :extra)).to eq(Success([:input, :extra])) + end + + it "calls a method taking no arguments when the contract validates an empty input" do + operation = Class.new(Dry::Operation) do + include Dry::Operation::Extensions::Validation + + params do + optional(:name).filled(:string) + end + + def call + step build_thing + end + + private + + def build_thing + Success(:thing) + end + end + + expect(operation.new.call).to eq(Success(:thing)) + end + + it "fails validation when a method taking no arguments has a contract requiring input" do + operation = Class.new(Dry::Operation) do + include Dry::Operation::Extensions::Validation + + params do + required(:name).filled(:string) + end + + def call + Success(:thing) + end + end + + result = operation.new.call + + expect(result).to be_failure + failure_type, validation_result = result.failure + expect(failure_type).to eq(:invalid) + expect(validation_result.errors.to_h).to eq(name: ["is missing"]) + end + + it "raises when input is given to a method that takes no arguments" do + operation = Class.new(Dry::Operation) do + include Dry::Operation::Extensions::Validation + + params do + optional(:name).filled(:string) + end + + def call + Success(:thing) + end + end + + expect { operation.new.call(name: "Alice") } + .to raise_error(ArgumentError, /wrong number of arguments/) + end + + it "validates an empty input for a method taking only keyword arguments" do + operation = Class.new(Dry::Operation) do + include Dry::Operation::Extensions::Validation + + params do + optional(:name).filled(:string) + end + + def call(name: "anonymous") + name + end + end + + expect(operation.new.call).to eq(Success("anonymous")) + expect(operation.new.call(name: "Alice")).to eq(Success("Alice")) + end end end