From 9f6ab88efd04f940f6dcd2729c565658cfc641c3 Mon Sep 17 00:00:00 2001 From: Tim Riley Date: Tue, 3 Sep 2024 00:04:08 +1000 Subject: [PATCH] Support receiving dry-validation contracts directly --- lib/hanami/action.rb | 7 +- lib/hanami/action/params.rb | 37 ++++++---- lib/hanami/action/validatable.rb | 25 ++++--- .../without_hanami_validations_spec.rb | 15 ---- spec/support/fixtures.rb | 20 +++-- spec/unit/hanami/action/contract_spec.rb | 22 +----- spec/unit/hanami/action/params_spec.rb | 74 ++++++++++--------- 7 files changed, 91 insertions(+), 109 deletions(-) diff --git a/lib/hanami/action.rb b/lib/hanami/action.rb index 4708c2f7..74839e41 100644 --- a/lib/hanami/action.rb +++ b/lib/hanami/action.rb @@ -39,7 +39,7 @@ def self.gem_loader loader.ignore( "#{root}/hanami-controller.rb", "#{root}/hanami/controller/version.rb", - "#{root}/hanami/action/{constants,errors,params,validatable}.rb" + "#{root}/hanami/action/{constants,errors,validatable}.rb" ) loader.inflector.inflect("csrf_protection" => "CSRFProtection") end @@ -72,6 +72,7 @@ def self.gem_loader setting :public_directory, default: Config::DEFAULT_PUBLIC_DIRECTORY setting :before_callbacks, default: Utils::Callbacks::Chain.new, mutable: true setting :after_callbacks, default: Utils::Callbacks::Chain.new, mutable: true + setting :contract_class # @!scope class @@ -316,7 +317,9 @@ def call(env) response = nil halted = catch :halt do - params = self.class.params_class.new(env) + # TODO: call contract_class.new at #initialize and capture it as an ivar (better perf for + # long-lived actions) + params = Params.new(env: env, validator: config.contract_class&.new) request = build_request( env: env, params: params, diff --git a/lib/hanami/action/params.rb b/lib/hanami/action/params.rb index a02003ab..9308e41b 100644 --- a/lib/hanami/action/params.rb +++ b/lib/hanami/action/params.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "hanami/utils/hash" + module Hanami class Action # A set of params requested by the client @@ -103,6 +105,11 @@ def _nested_attribute(keys, key) end end + # @api private + # @since 2.2.0 + DEFAULT_VALIDATOR = Utils::Hash.method(:deep_symbolize) + private_constant :DEFAULT_VALIDATOR + # Defines validations for the params, using the `params` schema of a dry-validation contract. # # @param block [Proc] the schema definition @@ -112,19 +119,14 @@ def _nested_attribute(keys, key) # @api public # @since 0.7.0 def self.params(&block) - @_validator = Class.new(Dry::Validation::Contract) { params(&block || -> {}) }.new - end + # TODO: add tests for this case + unless defined?(Dry::Validation::Contract) + message = %(To use `.params`, please add the "hanami-validations" gem to your Gemfile) + raise NoMethodError, message + end - # Defines validations for the params, using a dry-validation contract. - # - # @param block [Proc] the contract definition - # - # @see https://dry-rb.org/gems/dry-validation/ - # - # @api public - # @since 2.2.0 - def self.contract(&block) - @_validator = Class.new(Dry::Validation::Contract, &block).new + # TODO: deprecation warning + @_validator = Class.new(Dry::Validation::Contract) { params(&block || -> {}) }.new end class << self @@ -143,13 +145,18 @@ class << self # # @since 0.1.0 # @api private - def initialize(env) + def initialize(env:, validator: nil) @env = env @raw = _extract_params - validation = self.class._validator.call(raw) + # Fall back to the default validator here rather than in `._validator` itself. This allows + # `._validator` to return nil when there is no user-defined validator, which is important + # for the backwards compatibility behavior in `Validatable::ClassMethods#params`. + validator ||= self.class._validator || DEFAULT_VALIDATOR + validation = validator.call(raw) + @params = validation.to_h - @errors = Errors.new(validation.errors.to_h) + @errors = Errors.new(validation.respond_to?(:errors) ? validation.errors.to_h : {}) freeze end diff --git a/lib/hanami/action/validatable.rb b/lib/hanami/action/validatable.rb index ffd4cda7..5a6a4f10 100644 --- a/lib/hanami/action/validatable.rb +++ b/lib/hanami/action/validatable.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require_relative "params" - module Hanami class Action # Support for validating params when calling actions. @@ -98,12 +96,18 @@ module ClassMethods # @api public # @since 0.3.0 def params(klass = nil, &block) - if klass.nil? - klass = const_set(PARAMS_CLASS_NAME, Class.new(Params)) - klass.params(&block) - end + contract_class = + if klass.nil? + Class.new(Dry::Validation::Contract) { params(&block) } + elsif klass < Params + # Handle deprecated behavior of providing custom Hanami::Action::Params subclasses. + # TODO: deprecation warning here + klass._validator.class + else + klass + end - @params_class = klass + config.contract_class = contract_class end # Defines a validation contract for the params passed to {Hanami::Action#call}. @@ -187,12 +191,9 @@ def params(klass = nil, &block) # @api public # @since 2.2.0 def contract(klass = nil, &block) - if klass.nil? - klass = const_set(PARAMS_CLASS_NAME, Class.new(Params)) - klass.contract(&block) - end + contract_class = klass || Class.new(Dry::Validation::Contract, &block) - @params_class = klass + config.contract_class = contract_class end end end diff --git a/spec/isolation/without_hanami_validations_spec.rb b/spec/isolation/without_hanami_validations_spec.rb index d88e3ec9..a2ac42e9 100644 --- a/spec/isolation/without_hanami_validations_spec.rb +++ b/spec/isolation/without_hanami_validations_spec.rb @@ -11,10 +11,6 @@ expect(defined?(Hanami::Action::Validatable)).to be(nil) end - it "doesn't load Hanami::Action::Params" do - expect(defined?(Hanami::Action::Params)).to be(nil) - end - it "doesn't have params DSL" do expect do Class.new(Hanami::Action) do @@ -53,17 +49,6 @@ def handle(req, res) response = action.new.call({}) expect(response.body).to eq(["[true, true]"]) end - - it "has params that don't respond to .errors" do - action = Class.new(Hanami::Action) do - def handle(req, res) - res.body = req.params.respond_to?(:errors) - end - end - - response = action.new.call({}) - expect(response.body).to eq(["false"]) - end end RSpec::Support::Runner.run diff --git a/spec/support/fixtures.rb b/spec/support/fixtures.rb index 5a4f8a32..9441795c 100644 --- a/spec/support/fixtures.rb +++ b/spec/support/fixtures.rb @@ -1930,23 +1930,21 @@ def handle(request, response) end end -class ExternalContractParams < Hanami::Action::Params - contract do - params do - required(:birth_date).filled(:date) - required(:book).schema do - required(:title).filled(:str?) - end +class ExternalContract < Dry::Validation::Contract + params do + required(:birth_date).filled(:date) + required(:book).schema do + required(:title).filled(:str?) end + end - rule(:birth_date) do - key.failure("you must be 18 years or older") if value < Date.today << (12 * 18) - end + rule(:birth_date) do + key.failure("you must be 18 years or older") if value < Date.today << (12 * 18) end end class ExternalContractAction < ContractAction - contract ExternalContractParams + contract ExternalContract end class WhitelistedUploadDslContractAction < Hanami::Action diff --git a/spec/unit/hanami/action/contract_spec.rb b/spec/unit/hanami/action/contract_spec.rb index 692903e5..bc72dd62 100644 --- a/spec/unit/hanami/action/contract_spec.rb +++ b/spec/unit/hanami/action/contract_spec.rb @@ -25,7 +25,7 @@ end end - describe "provided by a standlone params class using a contract" do + describe "provided by a standlone contract class" do let(:action) { ExternalContractAction.new } context "when it has errors" do @@ -47,26 +47,6 @@ end end - describe "standalone class" do - it "validates the input" do - params_class = Class.new(Hanami::Action::Params) { - contract do - params do - required(:start_date).value(:date) - end - - rule(:start_date) do - key.failure("must be in the future") if value <= Date.today - end - end - } - - params = params_class.new(start_date: "2000-01-01") - - expect(params.errors.to_h).to eq(start_date: ["must be in the future"]) - end - end - describe "#raw" do context "without a contract" do let(:action) { RawContractAction.new } diff --git a/spec/unit/hanami/action/params_spec.rb b/spec/unit/hanami/action/params_spec.rb index 4a3b299b..f6973ac5 100644 --- a/spec/unit/hanami/action/params_spec.rb +++ b/spec/unit/hanami/action/params_spec.rb @@ -156,7 +156,7 @@ describe "validations" do it "isn't valid with empty params" do - params = TestParams.new({}) + params = TestParams.new(env: {}) expect(params.valid?).to be(false) @@ -169,7 +169,7 @@ end it "isn't valid with empty nested params" do - params = NestedParams.new(signup: {}) + params = NestedParams.new(env: {signup: {}}) expect(params.valid?).to be(false) @@ -185,18 +185,22 @@ end it "is it valid when all the validation criteria are met" do - params = TestParams.new(email: "test@hanamirb.org", - password: "123456", - password_confirmation: "123456", - name: "Luca", - tos: "1", - age: "34", - address: { - line_one: "10 High Street", - deep: { - deep_attr: "blue" - } - }) + params = TestParams.new( + env: { + email: "test@hanamirb.org", + password: "123456", + password_confirmation: "123456", + name: "Luca", + tos: "1", + age: "34", + address: { + line_one: "10 High Street", + deep: { + deep_attr: "blue" + } + } + } + ) expect(params.valid?).to be(true) expect(params.errors).to be_empty @@ -204,7 +208,7 @@ end it "has input available through the hash accessor" do - params = TestParams.new(name: "John", age: "1", address: {line_one: "10 High Street"}) + params = TestParams.new(env: {name: "John", age: "1", address: {line_one: "10 High Street"}}) expect(params[:name]).to eq("John") expect(params[:age]).to be(1) @@ -212,7 +216,7 @@ end it "allows nested hash access via symbols" do - params = TestParams.new(name: "John", address: {line_one: "10 High Street", deep: {deep_attr: 1}}) + params = TestParams.new(env: {name: "John", address: {line_one: "10 High Street", deep: {deep_attr: 1}}}) expect(params[:name]).to eq("John") expect(params[:address][:line_one]).to eq("10 High Street") expect(params[:address][:deep][:deep_attr]).to be(1) @@ -223,9 +227,11 @@ context "with data" do let(:params) do TestParams.new( - name: "John", - address: {line_one: "10 High Street", deep: {deep_attr: 1}}, - array: [{name: "Lennon"}, {name: "Wayne"}] + env: { + name: "John", + address: {line_one: "10 High Street", deep: {deep_attr: 1}}, + array: [{name: "Lennon"}, {name: "Wayne"}] + } ) end @@ -256,7 +262,7 @@ end context "without data" do - let(:params) { TestParams.new({}) } + let(:params) { TestParams.new(env: {}) } it "returns nil for nil argument" do expect(params.get(nil)).to be(nil) @@ -281,7 +287,7 @@ end describe "#to_h" do - let(:params) { TestParams.new(name: "Jane") } + let(:params) { TestParams.new(env: {name: "Jane"}) } it "returns a ::Hash" do expect(params.to_h).to be_kind_of(::Hash) @@ -316,7 +322,7 @@ } } - actual = TestParams.new(input).to_h + actual = TestParams.new(env: input).to_h expect(actual).to eq(expected) expect(actual).to be_kind_of(::Hash) @@ -349,7 +355,7 @@ } } - actual = TestParams.new(input).to_h + actual = TestParams.new(env: input).to_h expect(actual).to eq(expected) expect(actual).to be_kind_of(::Hash) @@ -360,7 +366,7 @@ end describe "#to_hash" do - let(:params) { TestParams.new(name: "Jane") } + let(:params) { TestParams.new(env: {name: "Jane"}) } it "returns a ::Hash" do expect(params.to_hash).to be_kind_of(::Hash) @@ -395,7 +401,7 @@ } } - actual = TestParams.new(input).to_hash + actual = TestParams.new(env: input).to_hash expect(actual).to eq(expected) expect(actual).to be_kind_of(::Hash) @@ -428,7 +434,7 @@ } } - actual = TestParams.new(input).to_hash + actual = TestParams.new(env: input).to_hash expect(actual).to eq(expected) expect(actual).to be_kind_of(::Hash) @@ -438,7 +444,7 @@ it "does not stringify values" do input = {"name" => 123} - params = TestParams.new(input) + params = TestParams.new(env: input) expect(params[:name]).to be(123) end @@ -448,9 +454,11 @@ describe "#deconstruct_keys" do let(:params) do TestParams.new( - name: "John", - address: {line_one: "10 High Street", deep: {deep_attr: 1}}, - array: [{name: "Lennon"}, {name: "Wayne"}] + env: { + name: "John", + address: {line_one: "10 High Street", deep: {deep_attr: 1}}, + array: [{name: "Lennon"}, {name: "Wayne"}] + } ) end @@ -471,7 +479,7 @@ end end - let(:params) { klass.new(book: {code: "abc"}) } + let(:params) { klass.new(env: {book: {code: "abc"}}) } it "returns Hanami::Action::Params::Errors" do expect(params.errors).to be_kind_of(Hanami::Action::Params::Errors) @@ -485,7 +493,7 @@ end it "appends message to already existing messages" do - params = klass.new(book: {}) + params = klass.new(env: {book: {}}) params.errors.add(:book, :code, "is invalid") expect(params.error_messages).to eq(["Code is missing", "Code is invalid"]) @@ -497,7 +505,7 @@ end it "raises error when try to add an error " do - params = klass.new({}) + params = klass.new(env: {}) expect { params.errors.add(:book, :code, "is invalid") }.to raise_error(ArgumentError, %(Can't add :book, :code, "is invalid" to {:book=>["is missing"]})) end