From dd319d47bb59fa3578c8703c3dcee1fe1ad63cd9 Mon Sep 17 00:00:00 2001 From: mrimp Date: Mon, 17 Jun 2024 16:25:59 +0200 Subject: [PATCH 01/12] First basic version - prototype --- Gemfile | 1 + lib/hanami/action.rb | 34 +++++- lib/hanami/action/contract.rb | 107 ++++++++++++++++++ lib/hanami/action/request.rb | 14 ++- lib/hanami/action/validatable.rb | 15 +++ .../without_hanami_validations_spec.rb | 22 ++++ spec/support/fixtures.rb | 32 ++++++ spec/unit/hanami/action/contract_spec.rb | 26 +++++ 8 files changed, 249 insertions(+), 2 deletions(-) create mode 100644 lib/hanami/action/contract.rb create mode 100644 spec/unit/hanami/action/contract_spec.rb diff --git a/Gemfile b/Gemfile index e4ff253f..24408730 100644 --- a/Gemfile +++ b/Gemfile @@ -31,4 +31,5 @@ group :benchmarks do gem "memory_profiler" end +gem 'byebug' gem "hanami-devtools", github: "hanami/devtools", branch: "main" diff --git a/lib/hanami/action.rb b/lib/hanami/action.rb index 4e1015cb..4266ec54 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,params,contract,validatable}.rb" ) loader.inflector.inflect("csrf_protection" => "CSRFProtection") end @@ -109,6 +109,10 @@ def self.inherited(subclass) if instance_variable_defined?(:@params_class) subclass.instance_variable_set(:@params_class, @params_class) end + + if instance_variable_defined?(:@contract_class) + subclass.instance_variable_set(:@contract_class, @contract_class) + end end # Returns the class which defines the params @@ -125,6 +129,19 @@ def self.params_class @params_class || BaseParams end + # Returns the class which defines the contract + # + # Returns the class which has been provided to define the + # contract. By default this will be Hanami::Action::Contract. + # + # @return [Class] A contract class or Hanami::Action::Contract + # + # @api private + # @since 2.2.0 + def self.contract_class + @contract_class || Contract + end + # Placeholder implementation for params class method # # Raises a developer friendly error to include `hanami/validations`. @@ -138,6 +155,19 @@ def self.params(_klass = nil) "To use `params`, please add 'hanami/validations' gem to your Gemfile" end + # Placeholder implementation for contract class method + # + # Raises a developer friendly error to include `hanami/validations`. + # + # @raise [NoMethodError] + # + # @api public + # @since 2.2.0 + def self.contract + raise NoMethodError, + "To use `contract`, please add 'hanami/validations' gem to your Gemfile" + end + # @overload self.append_before(*callbacks, &block) # Define a callback for an Action. # The callback will be executed **before** the action is called, in the @@ -306,9 +336,11 @@ def call(env) halted = catch :halt do params = self.class.params_class.new(env) + contract = self.class.contract_class.new(env) if self.class.instance_variable_defined?(:@contract_class) request = build_request( env: env, params: params, + contract: contract, session_enabled: session_enabled? ) response = build_response( diff --git a/lib/hanami/action/contract.rb b/lib/hanami/action/contract.rb new file mode 100644 index 00000000..cb522ec6 --- /dev/null +++ b/lib/hanami/action/contract.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +module Hanami + class Action + # A wrapper for Params class that allows for defining validation rules using Dry::Validation + # + # Accessible via the `contract` method in an action class. + # Although more complex domain-specific validations, or validations concerned with things such as uniqueness + # are usually better performed at layers deeper than your HTTP actions, Contract still provides helpful features + # that you can use without contravening the advice form above. + # + # @since 2.2.0 + class Contract + # @attr_reader env [Hash] the Rack env + # + # @since 2.2.0 + # @api private + attr_reader :env + + # Define a contract for the given action + # + # @param blk [Proc] the block to define the contract, including [Params] as a contract schema and connected rules + # + # @since 2.2.0 + # @api private + # + # @example + # class Create < Hanami::Action + # contract do + # params do + # required(:birth_date).value(:date) + # end + # rule(:birth_date) do + # key.failure('you must be 18 years or older to register') if value > Date.today - 18.years + # end + # + # def handle(req, *) + # halt 400 unless req.contract.errors + # # ... + # end + # end + def self.contract(&blk) + @_validator = Dry::Validation::Contract.build(&blk) + end + + # @since 2.2.0 + # @api private + class << self + attr_reader :_validator + end + + # Initialize the contract and freeze it. + # + # @param env [Hash] a Rack env or an hash of params. + # + # @return [nie wiem jeszcze] + # + # @since 2.2.0 + # @api public + def initialize(env) + @env = env + @input = extract_params + end + + # Validates the object, running the Dry::Validation contract and returning it + # @since 2.2.0 + # @api public + def call + @result = validate + result + end + + attr_reader :result + + private + + # TODO: shared with params.rb + def validate + self.class._validator.call(@input) + end + + def extract_params + result = {} + + if env.key?(Action::RACK_INPUT) + result.merge! ::Rack::Request.new(env).params + result.merge! _router_params + else + result.merge! _router_params(env) + env[Action::REQUEST_METHOD] ||= Action::DEFAULT_REQUEST_METHOD + end + + result + end + + def _router_params(fallback = {}) + env.fetch(ROUTER_PARAMS) do + if session = fallback.delete(Action::RACK_SESSION) + fallback[Action::RACK_SESSION] = Utils::Hash.deep_symbolize(session) + end + + fallback + end + end + end + end +end diff --git a/lib/hanami/action/request.rb b/lib/hanami/action/request.rb index 4c123366..de248006 100644 --- a/lib/hanami/action/request.rb +++ b/lib/hanami/action/request.rb @@ -27,12 +27,24 @@ class Request < ::Rack::Request # @api public attr_reader :params + + # Returns the contract that the request params can be validated against. + # + # For an action with {Validatable} included, this will be a {Contract} instance, otherwise it is not available + # + # @return [Contract] + # + # @since 2.2.0 + # @api public + attr_reader :contract + # @since 2.0.0 # @api private - def initialize(env:, params:, session_enabled: false) + def initialize(env:, params:, contract: nil, session_enabled: false) super(env) @params = params + @contract = contract @session_enabled = session_enabled end diff --git a/lib/hanami/action/validatable.rb b/lib/hanami/action/validatable.rb index 6eaceb64..3369fe02 100644 --- a/lib/hanami/action/validatable.rb +++ b/lib/hanami/action/validatable.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative "params" +require_relative "contract" module Hanami class Action @@ -17,6 +18,12 @@ module Validatable # @since 0.3.0 PARAMS_CLASS_NAME = "Params" + # Defines the base class name for contract + # + # @api private + # @since 2.2.0 + PARAMS_CLASS_NAME = "Contract" + # @api private # @since 0.1.0 def self.included(base) @@ -105,6 +112,14 @@ def params(klass = nil, &blk) @params_class = klass end + + # @since 2.2.0 + # @api public + def contract(&blk) + klass = const_set("Contract", Class.new(Contract)) + klass.class_eval { contract(&blk) } + @contract_class = klass + end end end end diff --git a/spec/isolation/without_hanami_validations_spec.rb b/spec/isolation/without_hanami_validations_spec.rb index d0568d3f..936118be 100644 --- a/spec/isolation/without_hanami_validations_spec.rb +++ b/spec/isolation/without_hanami_validations_spec.rb @@ -15,6 +15,10 @@ expect(defined?(Hanami::Action::Params)).to be(nil) end + it "doesn't load Hanami::Action::Contract" do + expect(defined?(Hanami::Action::Contract)).to be(nil) + end + it "doesn't have params DSL" do expect do Class.new(Hanami::Action) do @@ -28,6 +32,24 @@ ) end + it "doesn't have contract DSL" do + expect do + Class.new(Hanami::Action) do + 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 + end + end.to raise_error( + NoMethodError, + /To use `contract`, please add 'hanami\/validations' gem to your Gemfile/ + ) + end + it "has params that don't respond to .valid?" do action = Class.new(Hanami::Action) do def handle(req, res) diff --git a/spec/support/fixtures.rb b/spec/support/fixtures.rb index ac803e03..bfdfdf4c 100644 --- a/spec/support/fixtures.rb +++ b/spec/support/fixtures.rb @@ -1902,3 +1902,35 @@ def call(env) end end end + +class ContractAction < Hanami::Action + contract do + schema do + required(:birth_date).filled(:date) + end + + rule(:birth_date) do + key.failure("you must be 18 years or older") if value < Date.today << (12 * 18) + end + end + + def handle(request, response) + contract = request.contract.call + unless contract.errors.empty? + response.body = { errors: contract.errors.to_h } + response.status = 302 + end + end +end + +class BaseContract < Hanami::Action::Contract + 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 +end diff --git a/spec/unit/hanami/action/contract_spec.rb b/spec/unit/hanami/action/contract_spec.rb new file mode 100644 index 00000000..d0d3718c --- /dev/null +++ b/spec/unit/hanami/action/contract_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require "rack" +require 'byebug' + +RSpec.describe Hanami::Action::Contract do + describe "when defined as block in action" do + let(:action) { ContractAction.new } + it "is accessible as a Contract" do + response = action.call("birth_date" => "2000-01-01") + + expect(response.status).to eq 302 + expect(response.body).to eq ["{:errors=>{:birth_date=>[\"is missing\"]}}"] + end + end + + describe "works as a standalone contract class" do + it "validates the input" do + contract = BaseContract.new(start_date: "2000-01-01") + + result = contract.call + + expect(result.errors.to_h).to eq(start_date: ["must be in the future"]) + end + end +end From 47b719a5527dfbcf30cfc15b8845e40b744fadda Mon Sep 17 00:00:00 2001 From: mrimp Date: Tue, 18 Jun 2024 12:10:30 +0200 Subject: [PATCH 02/12] More details in documentation and slight refactors --- lib/hanami/action.rb | 2 + lib/hanami/action/base_params.rb | 32 +------------ lib/hanami/action/contract.rb | 58 ++++++++---------------- lib/hanami/action/params_extraction.rb | 45 ++++++++++++++++++ lib/hanami/action/validatable.rb | 2 +- spec/support/fixtures.rb | 20 ++++++++ spec/unit/hanami/action/contract_spec.rb | 13 +++++- 7 files changed, 100 insertions(+), 72 deletions(-) create mode 100644 lib/hanami/action/params_extraction.rb diff --git a/lib/hanami/action.rb b/lib/hanami/action.rb index 4266ec54..5c0d080d 100644 --- a/lib/hanami/action.rb +++ b/lib/hanami/action.rb @@ -151,6 +151,7 @@ def self.contract_class # @api public # @since 2.0.0 def self.params(_klass = nil) + raise NoMethodError, "To use `params`, please add 'hanami/validations' gem to your Gemfile" end @@ -164,6 +165,7 @@ def self.params(_klass = nil) # @api public # @since 2.2.0 def self.contract + raise NoMethodError, "To use `contract`, please add 'hanami/validations' gem to your Gemfile" end diff --git a/lib/hanami/action/base_params.rb b/lib/hanami/action/base_params.rb index 203d7173..4169b8e1 100644 --- a/lib/hanami/action/base_params.rb +++ b/lib/hanami/action/base_params.rb @@ -39,7 +39,7 @@ class BaseParams # @api private def initialize(env) @env = env - @raw = _extract_params + @raw = Hanami::Action::ParamsExtraction.new(env).call @params = Utils::Hash.deep_symbolize(@raw) freeze end @@ -135,36 +135,6 @@ def to_h def each(&blk) to_h.each(&blk) end - - private - - # @since 0.7.0 - # @api private - def _extract_params - result = {} - - if env.key?(Action::RACK_INPUT) - result.merge! ::Rack::Request.new(env).params - result.merge! _router_params - else - result.merge! _router_params(env) - env[Action::REQUEST_METHOD] ||= Action::DEFAULT_REQUEST_METHOD - end - - result - end - - # @since 0.7.0 - # @api private - def _router_params(fallback = {}) - env.fetch(ROUTER_PARAMS) do - if session = fallback.delete(Action::RACK_SESSION) - fallback[Action::RACK_SESSION] = Utils::Hash.deep_symbolize(session) - end - - fallback - end - end end end end diff --git a/lib/hanami/action/contract.rb b/lib/hanami/action/contract.rb index cb522ec6..20ace860 100644 --- a/lib/hanami/action/contract.rb +++ b/lib/hanami/action/contract.rb @@ -1,8 +1,10 @@ # frozen_string_literal: true +require 'byebug' module Hanami class Action - # A wrapper for Params class that allows for defining validation rules using Dry::Validation + # A wrapper for defining validation rules using Dry::Validation. This class essentially + # wraps a Dry::Validation::Contract and acts as a proxy to actually use Dry gem # # Accessible via the `contract` method in an action class. # Although more complex domain-specific validations, or validations concerned with things such as uniqueness @@ -26,18 +28,19 @@ class Contract # # @example # class Create < Hanami::Action - # contract do - # params do - # required(:birth_date).value(:date) - # end - # rule(:birth_date) do - # key.failure('you must be 18 years or older to register') if value > Date.today - 18.years - # end + # contract do + # params do + # required(:birth_date).value(:date) + # end + # rule(:birth_date) do + # key.failure('you must be 18 years or older to register') if value > Date.today - 18.years + # end + # end # - # def handle(req, *) - # halt 400 unless req.contract.errors - # # ... - # end + # def handle(req, *) + # halt 400 unless req.contract.call.errors.empty? + # # ... + # end # end def self.contract(&blk) @_validator = Dry::Validation::Contract.build(&blk) @@ -53,16 +56,17 @@ class << self # # @param env [Hash] a Rack env or an hash of params. # - # @return [nie wiem jeszcze] + # @return [Hash] # # @since 2.2.0 # @api public def initialize(env) @env = env - @input = extract_params + @input = Hanami::Action::ParamsExtraction.new(env).call end # Validates the object, running the Dry::Validation contract and returning it + # Contract needs to be called explicitly and handled the same way, by itself it does not invalidate the request. # @since 2.2.0 # @api public def call @@ -74,34 +78,10 @@ def call private - # TODO: shared with params.rb + # @since 2.2.0 def validate self.class._validator.call(@input) end - - def extract_params - result = {} - - if env.key?(Action::RACK_INPUT) - result.merge! ::Rack::Request.new(env).params - result.merge! _router_params - else - result.merge! _router_params(env) - env[Action::REQUEST_METHOD] ||= Action::DEFAULT_REQUEST_METHOD - end - - result - end - - def _router_params(fallback = {}) - env.fetch(ROUTER_PARAMS) do - if session = fallback.delete(Action::RACK_SESSION) - fallback[Action::RACK_SESSION] = Utils::Hash.deep_symbolize(session) - end - - fallback - end - end end end end diff --git a/lib/hanami/action/params_extraction.rb b/lib/hanami/action/params_extraction.rb new file mode 100644 index 00000000..3988ff41 --- /dev/null +++ b/lib/hanami/action/params_extraction.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require "rack/request" + +module Hanami + class Action + class ParamsExtraction + def initialize(env) + @env = env + end + + def call + _extract_params + end + + private + + attr_reader :env + + def _extract_params + result = {} + + if env.key?(Action::RACK_INPUT) + result.merge! ::Rack::Request.new(env).params + result.merge! _router_params + else + result.merge! _router_params(env) + env[Action::REQUEST_METHOD] ||= Action::DEFAULT_REQUEST_METHOD + end + + result + end + + def _router_params(fallback = {}) + env.fetch(ROUTER_PARAMS) do + if (session = fallback.delete(Action::RACK_SESSION)) + fallback[Action::RACK_SESSION] = Utils::Hash.deep_symbolize(session) + end + + fallback + end + end + end + end +end diff --git a/lib/hanami/action/validatable.rb b/lib/hanami/action/validatable.rb index 3369fe02..23ae600c 100644 --- a/lib/hanami/action/validatable.rb +++ b/lib/hanami/action/validatable.rb @@ -18,7 +18,7 @@ module Validatable # @since 0.3.0 PARAMS_CLASS_NAME = "Params" - # Defines the base class name for contract + # Defines the contract base class # # @api private # @since 2.2.0 diff --git a/spec/support/fixtures.rb b/spec/support/fixtures.rb index bfdfdf4c..88be628e 100644 --- a/spec/support/fixtures.rb +++ b/spec/support/fixtures.rb @@ -1934,3 +1934,23 @@ class BaseContract < Hanami::Action::Contract end end end + +AddressSchema = Dry::Schema.Params do + required(:country).value(:string) + required(:zipcode).value(:string) + required(:street).value(:string) +end + +ContactSchema = Dry::Schema.Params do + required(:email).value(:string) + required(:mobile).value(:string) +end + +class OutsideSchemasContract < Hanami::Action::Contract + contract do + params(AddressSchema, ContactSchema) do + required(:name).value(:string) + required(:age).value(:integer) + end + end +end diff --git a/spec/unit/hanami/action/contract_spec.rb b/spec/unit/hanami/action/contract_spec.rb index d0d3718c..93316a02 100644 --- a/spec/unit/hanami/action/contract_spec.rb +++ b/spec/unit/hanami/action/contract_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "rack" -require 'byebug' RSpec.describe Hanami::Action::Contract do describe "when defined as block in action" do @@ -22,5 +21,17 @@ expect(result.errors.to_h).to eq(start_date: ["must be in the future"]) end + + it "allows for usage of outside classes as schemas" do + contract = OutsideSchemasContract.new(country: "PL", zipcode: "11-123", mobile: "123092902", name: "myguy") + + result = contract.call + + expect(result.errors.to_h).to eq( + street: ["is missing"], + email: ["is missing"], + age: ["is missing"] + ) + end end end From bb742e1d874f16b30c1d874e0483216d049d7b53 Mon Sep 17 00:00:00 2001 From: mrimp Date: Tue, 18 Jun 2024 12:12:05 +0200 Subject: [PATCH 03/12] fix constant name --- lib/hanami/action/validatable.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/hanami/action/validatable.rb b/lib/hanami/action/validatable.rb index 23ae600c..6b63cb8d 100644 --- a/lib/hanami/action/validatable.rb +++ b/lib/hanami/action/validatable.rb @@ -22,7 +22,7 @@ module Validatable # # @api private # @since 2.2.0 - PARAMS_CLASS_NAME = "Contract" + CONTRACT_CLASS_NAME = "Contract" # @api private # @since 0.1.0 @@ -116,7 +116,7 @@ def params(klass = nil, &blk) # @since 2.2.0 # @api public def contract(&blk) - klass = const_set("Contract", Class.new(Contract)) + klass = const_set(CONTRACT_CLASS_NAME, Class.new(Contract)) klass.class_eval { contract(&blk) } @contract_class = klass end From 4e052b8d2763afcffa83890dd2672a7a76696543 Mon Sep 17 00:00:00 2001 From: mrimp Date: Wed, 10 Jul 2024 15:01:11 +0200 Subject: [PATCH 04/12] Second version, that aligns contract more with params --- lib/hanami/action.rb | 17 +++--- lib/hanami/action/base_params.rb | 32 +++++++++- lib/hanami/action/contract.rb | 75 ++++++++++++++++++++---- lib/hanami/action/request.rb | 14 +---- lib/hanami/action/validatable.rb | 11 ++-- spec/support/fixtures.rb | 16 +++-- spec/unit/hanami/action/contract_spec.rb | 27 +++++---- 7 files changed, 139 insertions(+), 53 deletions(-) diff --git a/lib/hanami/action.rb b/lib/hanami/action.rb index 5c0d080d..4fe1cf49 100644 --- a/lib/hanami/action.rb +++ b/lib/hanami/action.rb @@ -8,6 +8,7 @@ require "rack" require "rack/utils" require "zeitwerk" +require 'byebug' require_relative "action/constants" require_relative "action/errors" @@ -110,9 +111,6 @@ def self.inherited(subclass) subclass.instance_variable_set(:@params_class, @params_class) end - if instance_variable_defined?(:@contract_class) - subclass.instance_variable_set(:@contract_class, @contract_class) - end end # Returns the class which defines the params @@ -129,6 +127,7 @@ def self.params_class @params_class || BaseParams end + # Returns the class which defines the contract # # Returns the class which has been provided to define the @@ -138,9 +137,9 @@ def self.params_class # # @api private # @since 2.2.0 - def self.contract_class - @contract_class || Contract - end + # def self.contract_class + # @contract_class || Contract + # end # Placeholder implementation for params class method # @@ -337,12 +336,10 @@ def call(env) response = nil halted = catch :halt do - params = self.class.params_class.new(env) - contract = self.class.contract_class.new(env) if self.class.instance_variable_defined?(:@contract_class) - request = build_request( + params = self.class.params_class.new(env) + request = build_request( env: env, params: params, - contract: contract, session_enabled: session_enabled? ) response = build_response( diff --git a/lib/hanami/action/base_params.rb b/lib/hanami/action/base_params.rb index 4169b8e1..203d7173 100644 --- a/lib/hanami/action/base_params.rb +++ b/lib/hanami/action/base_params.rb @@ -39,7 +39,7 @@ class BaseParams # @api private def initialize(env) @env = env - @raw = Hanami::Action::ParamsExtraction.new(env).call + @raw = _extract_params @params = Utils::Hash.deep_symbolize(@raw) freeze end @@ -135,6 +135,36 @@ def to_h def each(&blk) to_h.each(&blk) end + + private + + # @since 0.7.0 + # @api private + def _extract_params + result = {} + + if env.key?(Action::RACK_INPUT) + result.merge! ::Rack::Request.new(env).params + result.merge! _router_params + else + result.merge! _router_params(env) + env[Action::REQUEST_METHOD] ||= Action::DEFAULT_REQUEST_METHOD + end + + result + end + + # @since 0.7.0 + # @api private + def _router_params(fallback = {}) + env.fetch(ROUTER_PARAMS) do + if session = fallback.delete(Action::RACK_SESSION) + fallback[Action::RACK_SESSION] = Utils::Hash.deep_symbolize(session) + end + + fallback + end + end end end end diff --git a/lib/hanami/action/contract.rb b/lib/hanami/action/contract.rb index 20ace860..09cb1461 100644 --- a/lib/hanami/action/contract.rb +++ b/lib/hanami/action/contract.rb @@ -1,18 +1,36 @@ # frozen_string_literal: true -require 'byebug' + +require "byebug" module Hanami class Action # A wrapper for defining validation rules using Dry::Validation. This class essentially # wraps a Dry::Validation::Contract and acts as a proxy to actually use Dry gem # - # Accessible via the `contract` method in an action class. + # Defined via the `contract` block in an action class. # Although more complex domain-specific validations, or validations concerned with things such as uniqueness # are usually better performed at layers deeper than your HTTP actions, Contract still provides helpful features # that you can use without contravening the advice form above. # # @since 2.2.0 class Contract + # A wrapper for the result of a contract validation + # @since 2.2.0 + # @api private + class Result < SimpleDelegator + # @since 2.0.0 + # @api private + def output + __getobj__.to_h + end + + # @since 2.0.0 + # @api private + def messages + __getobj__.errors.to_h + end + end + # @attr_reader env [Hash] the Rack env # # @since 2.2.0 @@ -43,12 +61,16 @@ class Contract # end # end def self.contract(&blk) - @_validator = Dry::Validation::Contract.build(&blk) + validations(&blk || -> {}) end # @since 2.2.0 # @api private class << self + def validations(&blk) + @_validator = Dry::Validation::Contract.build(&blk) + end + attr_reader :_validator end @@ -63,24 +85,57 @@ class << self def initialize(env) @env = env @input = Hanami::Action::ParamsExtraction.new(env).call + validation = validate + @params = validation.to_h + @errors = Hanami::Action::Params::Errors.new(validation.messages) + freeze end - # Validates the object, running the Dry::Validation contract and returning it - # Contract needs to be called explicitly and handled the same way, by itself it does not invalidate the request. + attr_reader :errors + + # Returns true if no validation errors are found, + # false otherwise. + # + # @return [TrueClass, FalseClass] + # # @since 2.2.0 - # @api public - def call - @result = validate - result + # + def valid? + errors.empty? end + # Serialize validated params to Hash + # + # @return [::Hash] + # + # @since 2.2.0 + def to_h + validate.output + end + alias_method :to_hash, :to_h + attr_reader :result + # Returns the value for the given params key. + # + # @param key [Symbol] the key + # + # @return [Object,nil] the associated value, if found + # + # @since 2.2.0 + # @api public + def [](key) + @params[key] + end + private # @since 2.2.0 def validate - self.class._validator.call(@input) + # Result.new need to take this in, and provide messages and output + Result.new( + self.class._validator.call(@input) + ) end end end diff --git a/lib/hanami/action/request.rb b/lib/hanami/action/request.rb index de248006..4c123366 100644 --- a/lib/hanami/action/request.rb +++ b/lib/hanami/action/request.rb @@ -27,24 +27,12 @@ class Request < ::Rack::Request # @api public attr_reader :params - - # Returns the contract that the request params can be validated against. - # - # For an action with {Validatable} included, this will be a {Contract} instance, otherwise it is not available - # - # @return [Contract] - # - # @since 2.2.0 - # @api public - attr_reader :contract - # @since 2.0.0 # @api private - def initialize(env:, params:, contract: nil, session_enabled: false) + def initialize(env:, params:, session_enabled: false) super(env) @params = params - @contract = contract @session_enabled = session_enabled end diff --git a/lib/hanami/action/validatable.rb b/lib/hanami/action/validatable.rb index 6b63cb8d..a07df3bd 100644 --- a/lib/hanami/action/validatable.rb +++ b/lib/hanami/action/validatable.rb @@ -109,16 +109,17 @@ def params(klass = nil, &blk) klass = const_set(PARAMS_CLASS_NAME, Class.new(Params)) klass.class_eval { params(&blk) } end - @params_class = klass end # @since 2.2.0 # @api public - def contract(&blk) - klass = const_set(CONTRACT_CLASS_NAME, Class.new(Contract)) - klass.class_eval { contract(&blk) } - @contract_class = klass + def contract(klass = nil, &blk) + if klass.nil? + klass = const_set(CONTRACT_CLASS_NAME, Class.new(Contract)) + klass.class_eval { contract(&blk) } + end + @params_class = klass end end end diff --git a/spec/support/fixtures.rb b/spec/support/fixtures.rb index 88be628e..f7125e77 100644 --- a/spec/support/fixtures.rb +++ b/spec/support/fixtures.rb @@ -1314,6 +1314,7 @@ class Update < Hanami::Action end def handle(req, res) + byebug valid = req.params.valid? res.status = 201 @@ -1905,8 +1906,11 @@ def call(env) class ContractAction < Hanami::Action contract do - schema do + params do required(:birth_date).filled(:date) + required(:book).schema do + required(:title).filled(:str?) + end end rule(:birth_date) do @@ -1915,9 +1919,13 @@ class ContractAction < Hanami::Action end def handle(request, response) - contract = request.contract.call - unless contract.errors.empty? - response.body = { errors: contract.errors.to_h } + if request.params.valid? + response.status = 201 + response.body = JSON.generate( + new_name: request.params[:book][:title].upcase + ) + else + response.body = { errors: request.params.errors.to_h } response.status = 302 end end diff --git a/spec/unit/hanami/action/contract_spec.rb b/spec/unit/hanami/action/contract_spec.rb index 93316a02..18ca3131 100644 --- a/spec/unit/hanami/action/contract_spec.rb +++ b/spec/unit/hanami/action/contract_spec.rb @@ -5,11 +5,22 @@ RSpec.describe Hanami::Action::Contract do describe "when defined as block in action" do let(:action) { ContractAction.new } - it "is accessible as a Contract" do - response = action.call("birth_date" => "2000-01-01") + context "when it has errors" do + it "returns them" do + response = action.call("birth_date" => "2000-01-01") - expect(response.status).to eq 302 - expect(response.body).to eq ["{:errors=>{:birth_date=>[\"is missing\"]}}"] + expect(response.status).to eq 302 + expect(response.body).to eq ["{:errors=>{:book=>[\"is missing\"], :birth_date=>[\"you must be 18 years or older\"]}}"] + end + end + + context "when it is valid" do + it "works" do + response = action.call("birth_date" => Date.today - (365 * 15), "book" => {"title" => "Hanami"}) + + expect(response.status).to eq 201 + expect(response.body).to eq ["{\"new_name\":\"HANAMI\"}"] + end end end @@ -17,17 +28,13 @@ it "validates the input" do contract = BaseContract.new(start_date: "2000-01-01") - result = contract.call - - expect(result.errors.to_h).to eq(start_date: ["must be in the future"]) + expect(contract.errors.to_h).to eq(start_date: ["must be in the future"]) end it "allows for usage of outside classes as schemas" do contract = OutsideSchemasContract.new(country: "PL", zipcode: "11-123", mobile: "123092902", name: "myguy") - result = contract.call - - expect(result.errors.to_h).to eq( + expect(contract.errors.to_h).to eq( street: ["is missing"], email: ["is missing"], age: ["is missing"] From c64c741d9fe49e2ca25f905485fa291c38a18197 Mon Sep 17 00:00:00 2001 From: mrimp Date: Wed, 10 Jul 2024 15:27:20 +0200 Subject: [PATCH 05/12] remove debug statements --- Gemfile | 1 - lib/hanami/action.rb | 1 - lib/hanami/action/contract.rb | 2 -- spec/support/fixtures.rb | 1 - 4 files changed, 5 deletions(-) diff --git a/Gemfile b/Gemfile index 24408730..e4ff253f 100644 --- a/Gemfile +++ b/Gemfile @@ -31,5 +31,4 @@ group :benchmarks do gem "memory_profiler" end -gem 'byebug' gem "hanami-devtools", github: "hanami/devtools", branch: "main" diff --git a/lib/hanami/action.rb b/lib/hanami/action.rb index 4fe1cf49..20aa4321 100644 --- a/lib/hanami/action.rb +++ b/lib/hanami/action.rb @@ -8,7 +8,6 @@ require "rack" require "rack/utils" require "zeitwerk" -require 'byebug' require_relative "action/constants" require_relative "action/errors" diff --git a/lib/hanami/action/contract.rb b/lib/hanami/action/contract.rb index 09cb1461..fb59b77a 100644 --- a/lib/hanami/action/contract.rb +++ b/lib/hanami/action/contract.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "byebug" - module Hanami class Action # A wrapper for defining validation rules using Dry::Validation. This class essentially diff --git a/spec/support/fixtures.rb b/spec/support/fixtures.rb index f7125e77..09167070 100644 --- a/spec/support/fixtures.rb +++ b/spec/support/fixtures.rb @@ -1314,7 +1314,6 @@ class Update < Hanami::Action end def handle(req, res) - byebug valid = req.params.valid? res.status = 201 From ef832c6849ac4e8c0385c6083c3624c4e07206a8 Mon Sep 17 00:00:00 2001 From: mrimp Date: Thu, 18 Jul 2024 14:12:02 +0200 Subject: [PATCH 06/12] Some missing doc statements, remove useless code --- lib/hanami/action.rb | 14 -------------- lib/hanami/action/contract.rb | 1 - lib/hanami/action/params_extraction.rb | 2 ++ lib/hanami/action/validatable.rb | 8 +++----- 4 files changed, 5 insertions(+), 20 deletions(-) diff --git a/lib/hanami/action.rb b/lib/hanami/action.rb index 20aa4321..7d341c4b 100644 --- a/lib/hanami/action.rb +++ b/lib/hanami/action.rb @@ -126,20 +126,6 @@ def self.params_class @params_class || BaseParams end - - # Returns the class which defines the contract - # - # Returns the class which has been provided to define the - # contract. By default this will be Hanami::Action::Contract. - # - # @return [Class] A contract class or Hanami::Action::Contract - # - # @api private - # @since 2.2.0 - # def self.contract_class - # @contract_class || Contract - # end - # Placeholder implementation for params class method # # Raises a developer friendly error to include `hanami/validations`. diff --git a/lib/hanami/action/contract.rb b/lib/hanami/action/contract.rb index fb59b77a..576d2fd0 100644 --- a/lib/hanami/action/contract.rb +++ b/lib/hanami/action/contract.rb @@ -130,7 +130,6 @@ def [](key) # @since 2.2.0 def validate - # Result.new need to take this in, and provide messages and output Result.new( self.class._validator.call(@input) ) diff --git a/lib/hanami/action/params_extraction.rb b/lib/hanami/action/params_extraction.rb index 3988ff41..550fbc7d 100644 --- a/lib/hanami/action/params_extraction.rb +++ b/lib/hanami/action/params_extraction.rb @@ -4,6 +4,8 @@ module Hanami class Action + # since 2.2.0 + # @api private class ParamsExtraction def initialize(env) @env = env diff --git a/lib/hanami/action/validatable.rb b/lib/hanami/action/validatable.rb index a07df3bd..e3953235 100644 --- a/lib/hanami/action/validatable.rb +++ b/lib/hanami/action/validatable.rb @@ -114,11 +114,9 @@ def params(klass = nil, &blk) # @since 2.2.0 # @api public - def contract(klass = nil, &blk) - if klass.nil? - klass = const_set(CONTRACT_CLASS_NAME, Class.new(Contract)) - klass.class_eval { contract(&blk) } - end + def contract(&blk) + klass = const_set(CONTRACT_CLASS_NAME, Class.new(Contract)) + klass.class_eval { contract(&blk) } @params_class = klass end end From 59f1216edebd122293ed6c31fdc398fd59284a0c Mon Sep 17 00:00:00 2001 From: mrimp Date: Fri, 2 Aug 2024 10:46:45 +0200 Subject: [PATCH 07/12] Fix rubocop offenses --- lib/hanami/action.rb | 3 --- spec/isolation/without_hanami_validations_spec.rb | 2 +- spec/support/fixtures.rb | 4 ++-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/hanami/action.rb b/lib/hanami/action.rb index 7d341c4b..ef4929a9 100644 --- a/lib/hanami/action.rb +++ b/lib/hanami/action.rb @@ -109,7 +109,6 @@ def self.inherited(subclass) if instance_variable_defined?(:@params_class) subclass.instance_variable_set(:@params_class, @params_class) end - end # Returns the class which defines the params @@ -135,7 +134,6 @@ def self.params_class # @api public # @since 2.0.0 def self.params(_klass = nil) - raise NoMethodError, "To use `params`, please add 'hanami/validations' gem to your Gemfile" end @@ -149,7 +147,6 @@ def self.params(_klass = nil) # @api public # @since 2.2.0 def self.contract - raise NoMethodError, "To use `contract`, please add 'hanami/validations' gem to your Gemfile" end diff --git a/spec/isolation/without_hanami_validations_spec.rb b/spec/isolation/without_hanami_validations_spec.rb index 936118be..e5e544d1 100644 --- a/spec/isolation/without_hanami_validations_spec.rb +++ b/spec/isolation/without_hanami_validations_spec.rb @@ -40,7 +40,7 @@ required(:start_date).value(:date) end rule(:start_date) do - key.failure('must be in the future') if value <= Date.today + key.failure("must be in the future") if value <= Date.today end end end diff --git a/spec/support/fixtures.rb b/spec/support/fixtures.rb index 09167070..3dd59e5e 100644 --- a/spec/support/fixtures.rb +++ b/spec/support/fixtures.rb @@ -1924,7 +1924,7 @@ def handle(request, response) new_name: request.params[:book][:title].upcase ) else - response.body = { errors: request.params.errors.to_h } + response.body = {errors: request.params.errors.to_h} response.status = 302 end end @@ -1937,7 +1937,7 @@ class BaseContract < Hanami::Action::Contract end rule(:start_date) do - key.failure('must be in the future') if value <= Date.today + key.failure("must be in the future") if value <= Date.today end end end From 9debfdb77cebd1a42387dfa007a811dc20f160d2 Mon Sep 17 00:00:00 2001 From: mrimp Date: Mon, 5 Aug 2024 09:02:28 +0200 Subject: [PATCH 08/12] Better names for methods, remove obsolete extraction --- lib/hanami/action/contract.rb | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/hanami/action/contract.rb b/lib/hanami/action/contract.rb index 576d2fd0..64e250c7 100644 --- a/lib/hanami/action/contract.rb +++ b/lib/hanami/action/contract.rb @@ -18,13 +18,13 @@ class Contract class Result < SimpleDelegator # @since 2.0.0 # @api private - def output + def to_h __getobj__.to_h end # @since 2.0.0 # @api private - def messages + def errors __getobj__.errors.to_h end end @@ -59,16 +59,12 @@ def messages # end # end def self.contract(&blk) - validations(&blk || -> {}) + @_validator = Dry::Validation::Contract.build(&blk) end # @since 2.2.0 # @api private class << self - def validations(&blk) - @_validator = Dry::Validation::Contract.build(&blk) - end - attr_reader :_validator end @@ -85,7 +81,7 @@ def initialize(env) @input = Hanami::Action::ParamsExtraction.new(env).call validation = validate @params = validation.to_h - @errors = Hanami::Action::Params::Errors.new(validation.messages) + @errors = Hanami::Action::Params::Errors.new(validation.errors) freeze end @@ -108,9 +104,8 @@ def valid? # # @since 2.2.0 def to_h - validate.output + validate.to_h end - alias_method :to_hash, :to_h attr_reader :result From 56081d2adfc3ed3ec95e1aea9afc75928fcd2b86 Mon Sep 17 00:00:00 2001 From: mrimp Date: Wed, 7 Aug 2024 11:04:39 +0200 Subject: [PATCH 09/12] Use ivar in to_h implementaion --- lib/hanami/action/contract.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/hanami/action/contract.rb b/lib/hanami/action/contract.rb index 64e250c7..2e226a6e 100644 --- a/lib/hanami/action/contract.rb +++ b/lib/hanami/action/contract.rb @@ -104,7 +104,7 @@ def valid? # # @since 2.2.0 def to_h - validate.to_h + @params end attr_reader :result From 9a264e14be965567bb25b44afc269de7d6c144ed Mon Sep 17 00:00:00 2001 From: mrimp Date: Mon, 12 Aug 2024 15:50:31 +0200 Subject: [PATCH 10/12] Extract common interfrace of Params and Contract --- lib/hanami/action/base_params.rb | 68 +--- lib/hanami/action/contract.rb | 67 +--- lib/hanami/action/params.rb | 197 +---------- lib/hanami/action/request.rb | 7 +- .../request_params/action_validations.rb | 219 ++++++++++++ lib/hanami/action/request_params/base.rb | 69 ++++ lib/hanami/action/validatable.rb | 8 +- spec/support/fixtures.rb | 59 ++++ spec/unit/hanami/action/contract_spec.rb | 313 ++++++++++++++++++ spec/unit/hanami/action/params_spec.rb | 2 +- 10 files changed, 680 insertions(+), 329 deletions(-) create mode 100644 lib/hanami/action/request_params/action_validations.rb create mode 100644 lib/hanami/action/request_params/base.rb diff --git a/lib/hanami/action/base_params.rb b/lib/hanami/action/base_params.rb index 203d7173..e85b44dc 100644 --- a/lib/hanami/action/base_params.rb +++ b/lib/hanami/action/base_params.rb @@ -19,6 +19,7 @@ class Action # @api private # @since 0.7.0 class BaseParams + include Hanami::Action::RequestParams::Base # @attr_reader env [Hash] the Rack env # # @since 0.7.0 @@ -39,23 +40,11 @@ class BaseParams # @api private def initialize(env) @env = env - @raw = _extract_params + @raw = Hanami::Action::ParamsExtraction.new(env).call @params = Utils::Hash.deep_symbolize(@raw) freeze end - # Returns the value for the given params key. - # - # @param key [Symbol] the key - # - # @return [Object,nil] the associated value, if found - # - # @since 0.7.0 - # @api public - def [](key) - @params[key] - end - # Returns an value associated with the given params key. # # You can access nested attributes by listing all the keys in the path. This uses the same key @@ -86,17 +75,6 @@ def [](key) # end # end # - # @since 0.7.0 - # @api public - def get(*keys) - @params.dig(*keys) - end - - # This is for compatibility with Hanami::Helpers::FormHelper::Values - # - # @api private - # @since 0.8.0 - alias_method :dig, :get # Returns true at all times, providing a common interface with {Params}. # @@ -110,50 +88,8 @@ def valid? true end - # Returns a hash of the parsed request params. - # - # @return [Hash] - # - # @since 0.7.0 - # @api public - def to_h - @params - end - alias_method :to_hash, :to_h - - # Iterates over the params. - # - # Calls the given block with each param key-value pair; returns the full hash of params. - # - # @yieldparam key [Symbol] - # @yieldparam value [Object] - # - # @return [to_h] - # - # @since 0.7.1 - # @api public - def each(&blk) - to_h.each(&blk) - end - private - # @since 0.7.0 - # @api private - def _extract_params - result = {} - - if env.key?(Action::RACK_INPUT) - result.merge! ::Rack::Request.new(env).params - result.merge! _router_params - else - result.merge! _router_params(env) - env[Action::REQUEST_METHOD] ||= Action::DEFAULT_REQUEST_METHOD - end - - result - end - # @since 0.7.0 # @api private def _router_params(fallback = {}) diff --git a/lib/hanami/action/contract.rb b/lib/hanami/action/contract.rb index 2e226a6e..144119eb 100644 --- a/lib/hanami/action/contract.rb +++ b/lib/hanami/action/contract.rb @@ -12,6 +12,8 @@ class Action # # @since 2.2.0 class Contract + include Hanami::Action::RequestParams::ActionValidations + include Hanami::Action::RequestParams::Base # A wrapper for the result of a contract validation # @since 2.2.0 # @api private @@ -22,19 +24,17 @@ def to_h __getobj__.to_h end + # This method is called messages not errors to be consistent with the Hanami::Validations::Result + # + # @return [Hash] the error messages + # # @since 2.0.0 # @api private - def errors + def messages __getobj__.errors.to_h end end - # @attr_reader env [Hash] the Rack env - # - # @since 2.2.0 - # @api private - attr_reader :env - # Define a contract for the given action # # @param blk [Proc] the block to define the contract, including [Params] as a contract schema and connected rules @@ -68,59 +68,6 @@ class << self attr_reader :_validator end - # Initialize the contract and freeze it. - # - # @param env [Hash] a Rack env or an hash of params. - # - # @return [Hash] - # - # @since 2.2.0 - # @api public - def initialize(env) - @env = env - @input = Hanami::Action::ParamsExtraction.new(env).call - validation = validate - @params = validation.to_h - @errors = Hanami::Action::Params::Errors.new(validation.errors) - freeze - end - - attr_reader :errors - - # Returns true if no validation errors are found, - # false otherwise. - # - # @return [TrueClass, FalseClass] - # - # @since 2.2.0 - # - def valid? - errors.empty? - end - - # Serialize validated params to Hash - # - # @return [::Hash] - # - # @since 2.2.0 - def to_h - @params - end - - attr_reader :result - - # Returns the value for the given params key. - # - # @param key [Symbol] the key - # - # @return [Object,nil] the associated value, if found - # - # @since 2.2.0 - # @api public - def [](key) - @params[key] - end - private # @since 2.2.0 diff --git a/lib/hanami/action/params.rb b/lib/hanami/action/params.rb index 2e1dfe83..360acb30 100644 --- a/lib/hanami/action/params.rb +++ b/lib/hanami/action/params.rb @@ -16,97 +16,7 @@ class Action # @since 0.1.0 class Params < BaseParams include Hanami::Validations::Form - - # Params errors - # - # @since 1.1.0 - class Errors < SimpleDelegator - # @since 1.1.0 - # @api private - def initialize(errors = {}) - super(errors.dup) - end - - # Add an error to the param validations - # - # This has a semantic similar to `Hash#dig` where you use a set of keys - # to get a nested value, here you use a set of keys to set a nested - # value. - # - # @param args [Array] an array of arguments: the last - # one is the message to add (String), while the beginning of the array - # is made of keys to reach the attribute. - # - # @raise [ArgumentError] when try to add a message for a key that is - # already filled with incompatible message type. - # This usually happens with nested attributes: if you have a `:book` - # schema and the input doesn't include data for `:book`, the messages - # will be `["is missing"]`. In that case you can't add an error for a - # key nested under `:book`. - # - # @since 1.1.0 - # - # @example Basic usage - # require "hanami/controller" - # - # class MyAction < Hanami::Action - # params do - # required(:book).schema do - # required(:isbn).filled(:str?) - # end - # end - # - # def handle(req, res) - # # 1. Don't try to save the record if the params aren't valid - # return unless req.params.valid? - # - # BookRepository.new.create(req.params[:book]) - # rescue Hanami::Model::UniqueConstraintViolationError - # # 2. Add an error in case the record wasn't unique - # req.params.errors.add(:book, :isbn, "is not unique") - # end - # end - # - # @example Invalid argument - # require "hanami/controller" - # - # class MyAction < Hanami::Action - # params do - # required(:book).schema do - # required(:title).filled(:str?) - # end - # end - # - # def handle(req, *) - # puts req.params.to_h # => {} - # puts req.params.valid? # => false - # puts req.params.error_messages # => ["Book is missing"] - # puts req.params.errors # => {:book=>["is missing"]} - # - # req.params.errors.add(:book, :isbn, "is not unique") # => ArgumentError - # end - # end - def add(*args) - *keys, key, error = args - _nested_attribute(keys, key) << error - rescue TypeError - raise ArgumentError.new("Can't add #{args.map(&:inspect).join(', ')} to #{inspect}") - end - - private - - # @since 1.1.0 - # @api private - def _nested_attribute(keys, key) - if keys.empty? - self - else - keys.inject(self) { |result, k| result[k] ||= {} } - dig(*keys) - end[key] ||= [] - end - end - + include Hanami::Action::RequestParams::ActionValidations # This is a Hanami::Validations extension point # # @since 0.7.0 @@ -147,111 +57,6 @@ def self._base_rules def self.params(&blk) validations(&blk || -> {}) end - - # Initialize the params and freeze them. - # - # @param env [Hash] a Rack env or an hash of params. - # - # @return [Params] - # - # @since 0.1.0 - # @api private - def initialize(env) - @env = env - super(_extract_params) - validation = validate - @params = validation.to_h - @errors = Errors.new(validation.messages) - freeze - end - - # Returns raw params from Rack env - # - # @return [Hash] - # - # @since 0.3.2 - def raw - @input - end - - # Returns structured error messages - # - # @return [Hash] - # - # @since 0.7.0 - # - # @example - # params.errors - # # => { - # :email=>["is missing", "is in invalid format"], - # :name=>["is missing"], - # :tos=>["is missing"], - # :age=>["is missing"], - # :address=>["is missing"] - # } - attr_reader :errors - - # Returns flat collection of full error messages - # - # @return [Array] - # - # @since 0.7.0 - # - # @example - # params.error_messages - # # => [ - # "Email is missing", - # "Email is in invalid format", - # "Name is missing", - # "Tos is missing", - # "Age is missing", - # "Address is missing" - # ] - def error_messages(error_set = errors) - error_set.each_with_object([]) do |(key, messages), result| - k = Utils::String.titleize(key) - - msgs = if messages.is_a?(::Hash) - error_messages(messages) - else - messages.map { |message| "#{k} #{message}" } - end - - result.concat(msgs) - end - end - - # Returns true if no validation errors are found, - # false otherwise. - # - # @return [TrueClass, FalseClass] - # - # @since 0.7.0 - # - # @example - # params.valid? # => true - def valid? - errors.empty? - end - - # Serialize validated params to Hash - # - # @return [::Hash] - # - # @since 0.3.0 - def to_h - @params - end - alias_method :to_hash, :to_h - - # Pattern-matching support - # - # @return [::Hash] - # - # @since 2.0.2 - def deconstruct_keys(*) - to_hash - end end end end diff --git a/lib/hanami/action/request.rb b/lib/hanami/action/request.rb index 4c123366..c3318c7b 100644 --- a/lib/hanami/action/request.rb +++ b/lib/hanami/action/request.rb @@ -18,10 +18,11 @@ class Action class Request < ::Rack::Request # Returns the request's params. # - # For an action with {Validatable} included, this will be a {Params} instance, otherwise a - # {BaseParams}. + # For an action with {Validatable} included, this will be a {Params} or {Contract} instance + # (if using contract block), + # otherwise a {BaseParams} when using params block. # - # @return [BaseParams,Params] + # @return [BaseParams,Params, Contract] # # @since 2.0.0 # @api public diff --git a/lib/hanami/action/request_params/action_validations.rb b/lib/hanami/action/request_params/action_validations.rb new file mode 100644 index 00000000..a5ad8ac9 --- /dev/null +++ b/lib/hanami/action/request_params/action_validations.rb @@ -0,0 +1,219 @@ +# frozen_string_literal: true + +module Hanami + class Action + module RequestParams + # Handles different ways to validate request params in an action, that come in from Hanami::Action::Validatable + # It is supposed to handle all approaches through a common interface to access params/contract in the same way + # + # If you use params block or contract block, you can access the params/contract in the same way through + # the request object + # They will have the same methods available and all. Just keep in mind the differences between schema and contract + # since 2.2.0 + # @api private + module ActionValidations + # Common errors for both params and contract validations + # + # @since 2.2.0 + class Errors < SimpleDelegator + # @since 1.1.0 + # @api private + def initialize(errors = {}) + super(errors.dup) + end + + # Add an error to the param validations + # + # This has a semantic similar to `Hash#dig` where you use a set of keys + # to get a nested value, here you use a set of keys to set a nested + # value. + # + # @param args [Array] an array of arguments: the last + # one is the message to add (String), while the beginning of the array + # is made of keys to reach the attribute. + # + # @raise [ArgumentError] when try to add a message for a key that is + # already filled with incompatible message type. + # This usually happens with nested attributes: if you have a `:book` + # schema and the input doesn't include data for `:book`, the messages + # will be `["is missing"]`. In that case you can't add an error for a + # key nested under `:book`. + # + # @since 1.1.0 + # + # @example Basic usage + # require "hanami/controller" + # + # class MyAction < Hanami::Action + # params do + # required(:book).schema do + # required(:isbn).filled(:str?) + # end + # end + # + # def handle(req, res) + # # 1. Don't try to save the record if the params aren't valid + # return unless req.params.valid? + # + # BookRepository.new.create(req.params[:book]) + # rescue Hanami::Model::UniqueConstraintViolationError + # # 2. Add an error in case the record wasn't unique + # req.params.errors.add(:book, :isbn, "is not unique") + # end + # end + # + # @example Invalid argument + # require "hanami/controller" + # + # class MyAction < Hanami::Action + # params do + # required(:book).schema do + # required(:title).filled(:str?) + # end + # end + # + # def handle(req, *) + # puts req.params.to_h # => {} + # puts req.params.valid? # => false + # puts req.params.error_messages # => ["Book is missing"] + # puts req.params.errors # => {:book=>["is missing"]} + # + # req.params.errors.add(:book, :isbn, "is not unique") # => ArgumentError + # end + # end + def add(*args) + *keys, key, error = args + _nested_attribute(keys, key) << error + rescue TypeError + raise ArgumentError.new("Can't add #{args.map(&:inspect).join(', ')} to #{inspect}") + end + + private + + # @since 1.1.0 + # @api private + def _nested_attribute(keys, key) + if keys.empty? + self + else + keys.inject(self) { |result, k| result[k] ||= {} } + dig(*keys) + end[key] ||= [] + end + end + + # Initialize the params/contract and freeze. + # + # @param env [Hash] a Rack env or an hash of params. + # + # @return [Params] + # + # @since 2.2.0 + # @api private + def initialize(env) + @env = env + @input = Hanami::Action::ParamsExtraction.new(env).call + validation = validate + @params = validation.to_h + @errors = Errors.new(validation.messages) + freeze + end + + # Provide the same initialize method for both params and contract + # since 2.2.0 + # @api private + def self.included(klass) + klass.send :prepend, ActionValidations + end + + # Returns raw params from Rack env + # + # @return [Hash] + # + # @since 2.2.0 + def raw + @input + end + + # Returns structured error messages + # + # @return [Hash] + # + # @since 2.2.0 + # + # @example + # params.errors + # # => { + # :email=>["is missing", "is in invalid format"], + # :name=>["is missing"], + # :tos=>["is missing"], + # :age=>["is missing"], + # :address=>["is missing"] + # } + attr_reader :errors + + # Returns flat collection of full error messages + # + # @return [Array] + # + # @since 2.2.0 + # + # @example + # params.error_messages + # # => [ + # "Email is missing", + # "Email is in invalid format", + # "Name is missing", + # "Tos is missing", + # "Age is missing", + # "Address is missing" + # ] + def error_messages(error_set = errors) + error_set.each_with_object([]) do |(key, messages), result| + k = Utils::String.titleize(key) + + msgs = if messages.is_a?(::Hash) + error_messages(messages) + else + messages.map { |message| "#{k} #{message}" } + end + + result.concat(msgs) + end + end + + # Returns true if no validation errors are found, + # false otherwise. + # + # @return [TrueClass, FalseClass] + # + # @since 2.2.0 + # + # @example + # params.valid? # => true + def valid? + errors.empty? + end + + # Serialize validated params to Hash + # + # @return [::Hash] + # + # @since 2.2.0 + def to_h + @params + end + alias_method :to_hash, :to_h + + # Pattern-matching support + # + # @return [::Hash] + # + # @since 2.2.0 + def deconstruct_keys(*) + to_hash + end + end + end + end +end diff --git a/lib/hanami/action/request_params/base.rb b/lib/hanami/action/request_params/base.rb new file mode 100644 index 00000000..a71e5919 --- /dev/null +++ b/lib/hanami/action/request_params/base.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +module Hanami + class Action + module RequestParams + # Where ActionValidations provides common interface for handling validations, this Base modules handles the most + # common and basic methods for accessing params in an action. It is included in both BaseParams and Contract + # classes since Params inherits from BaseParams and Contract is more independent cause it delegates to + # Dry::Validation quickly + # + # Params is also coupled with Hanami::Validations::Form and this Base module is a way to breach the gap and make + # them more compatible. + # + # since 2.2.0 + # @api private + module Base + # @since 0.7.0 + # @api public + def get(*keys) + @params.dig(*keys) + end + + # This is for compatibility with Hanami::Helpers::FormHelper::Values + # + # @api private + # @since 0.8.0 + alias_method :dig, :get + + # Returns a hash of the parsed request params. + # + # @return [Hash] + # + # @since 0.7.0 + # @api public + def to_h + @params + end + alias_method :to_hash, :to_h + + # Iterates over the params. + # + # Calls the given block with each param key-value pair; returns the full hash of params. + # + # @yieldparam key [Symbol] + # @yieldparam value [Object] + # + # @return [to_h] + # + # @since 0.7.1 + # @api public + def each(&blk) + to_h.each(&blk) + end + + # Returns the value for the given params key. + # + # @param key [Symbol] the key + # + # @return [Object,nil] the associated value, if found + # + # @since 0.7.0 + # @api public + def [](key) + @params[key] + end + end + end + end +end diff --git a/lib/hanami/action/validatable.rb b/lib/hanami/action/validatable.rb index e3953235..a07df3bd 100644 --- a/lib/hanami/action/validatable.rb +++ b/lib/hanami/action/validatable.rb @@ -114,9 +114,11 @@ def params(klass = nil, &blk) # @since 2.2.0 # @api public - def contract(&blk) - klass = const_set(CONTRACT_CLASS_NAME, Class.new(Contract)) - klass.class_eval { contract(&blk) } + def contract(klass = nil, &blk) + if klass.nil? + klass = const_set(CONTRACT_CLASS_NAME, Class.new(Contract)) + klass.class_eval { contract(&blk) } + end @params_class = klass end end diff --git a/spec/support/fixtures.rb b/spec/support/fixtures.rb index 3dd59e5e..55087772 100644 --- a/spec/support/fixtures.rb +++ b/spec/support/fixtures.rb @@ -1942,6 +1942,17 @@ class BaseContract < Hanami::Action::Contract end end +class NestedContractParams < Hanami::Action::Contract + contract do + params do + required(:signup).schema do + required(:name).filled(:str?) + required(:age).filled(:int?, gteq?: 18) + end + end + end +end + AddressSchema = Dry::Schema.Params do required(:country).value(:string) required(:zipcode).value(:string) @@ -1961,3 +1972,51 @@ class OutsideSchemasContract < Hanami::Action::Contract end end end + +class TestContract < Hanami::Action::Contract + contract do + params do + required(:name).value(:string) + required(:email).filled(format?: /\A.+@.+\z/) + required(:tos).filled(:bool?) + required(:age).filled(:int?) + required(:address).schema do + required(:line_one).filled + required(:deep).schema do + required(:deep_attr).filled(:str?) + end + end + + optional(:array).maybe do + each do + schema do + required(:name).filled(:str?) + end + end + end + end + + rule(:name) do + key.failure("must be Luca") if value != "Luca" + end + end +end + +class WhitelistedUploadDslContractAction < Hanami::Action + contract do + params do + required(:id).maybe(:integer) + required(:upload).filled + end + end + + def handle(req, res) + res.body = req.params.to_h.inspect + end +end + +class RawContractAction < Hanami::Action + def handle(req, res) + res.body = req.params.to_h.inspect + end +end diff --git a/spec/unit/hanami/action/contract_spec.rb b/spec/unit/hanami/action/contract_spec.rb index 18ca3131..bcb46344 100644 --- a/spec/unit/hanami/action/contract_spec.rb +++ b/spec/unit/hanami/action/contract_spec.rb @@ -41,4 +41,317 @@ ) end end + + describe "#raw" do + let(:params) { Class.new(Hanami::Action::Contract) } + + context "when this feature isn't enabled" do + let(:action) { RawContractAction.new } + + it "raw gets all params" do + File.open("spec/support/fixtures/multipart-upload.png", "rb") do |upload| + response = action.call("id" => "1", "unknown" => "2", "upload" => upload) + + expect(response[:params][:id]).to eq("1") + expect(response[:params][:unknown]).to eq("2") + expect(FileUtils.cmp(response[:params][:upload], upload)).to be(true) + + expect(response[:params].raw.fetch("id")).to eq("1") + expect(response[:params].raw.fetch("unknown")).to eq("2") + expect(response[:params].raw.fetch("upload")).to eq(upload) + end + end + end + + context "when this feature is enabled" do + let(:action) { WhitelistedUploadDslContractAction.new } + + it "raw gets all params" do + Tempfile.create("multipart-upload") do |upload| + response = action.call("id" => "1", "unknown" => "2", "upload" => upload, "_csrf_token" => "3") + + expect(response[:params][:id]).to eq(1) + expect(response[:params][:unknown]).to be(nil) + expect(response[:params][:upload]).to eq(upload) + + expect(response[:params].raw.fetch("id")).to eq("1") + expect(response[:params].raw.fetch("unknown")).to eq("2") + expect(response[:params].raw.fetch("upload")).to eq(upload) + end + end + end + end + + describe "validations" do + it "isn't valid with empty params" do + params = TestContract.new({}) + + expect(params.valid?).to be(false) + + expect(params.errors.fetch(:email)).to eq(["is missing"]) + expect(params.errors.fetch(:name)).to eq(["is missing"]) + expect(params.errors.fetch(:tos)).to eq(["is missing"]) + expect(params.errors.fetch(:address)).to eq(["is missing"]) + + expect(params.error_messages).to eq(["Name is missing", "Email is missing", "Tos is missing", "Age is missing", "Address is missing"]) + end + + it "isn't valid with empty nested params" do + params = NestedContractParams.new(signup: {}) + + expect(params.valid?).to be(false) + + expect(params.errors.fetch(:signup).fetch(:name)).to eq(["is missing"]) + + with_hanami_validations(1) do + expect(params.error_messages).to eq(["Name is missing", "Age is missing", "Age must be greater than or equal to 18"]) + end + + with_hanami_validations(2) do + expect(params.error_messages).to eq(["Name is missing", "Age is missing"]) + end + end + + it "is it valid when all the validation criteria are met" do + params = TestContract.new(email: "test@hanamirb.org", + password: "123456", + password_confirmation: "123456", + name: "Luca", + tos: true, + age: 34, + address: { + line_one: "10 High Street", + deep: { + deep_attr: "blue" + } + }) + + expect(params.valid?).to be(true) + expect(params.errors).to be_empty + expect(params.error_messages).to be_empty + end + + it "has input available through the hash accessor" do + params = TestContract.new(name: "John", age: "1", address: {line_one: "10 High Street"}) + + expect(params[:name]).to eq("John") + expect(params[:age]).to be("1") + expect(params[:address][:line_one]).to eq("10 High Street") + end + + it "allows nested hash access via symbols" do + params = TestContract.new(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) + end + end + + describe "#get" do + context "with data" do + let(:params) do + TestContract.new( + name: "Luca", + address: {line_one: "10 High Street", deep: {deep_attr: 1}}, + array: [{name: "Lennon"}, {name: "Wayne"}] + ) + end + + it "returns nil for nil argument" do + expect(params.get(nil)).to be(nil) + end + + it "returns nil for unknown param" do + expect(params.get(:unknown)).to be(nil) + end + + it "allows to read top level param" do + expect(params.get(:name)).to eq("Luca") + end + + it "allows to read nested param" do + expect(params.get(:address, :line_one)).to eq("10 High Street") + end + + it "returns nil for unknown nested param" do + expect(params.get(:address, :unknown)).to be(nil) + end + + it "allows to read data under arrays" do + expect(params.get(:array, 0, :name)).to eq("Lennon") + expect(params.get(:array, 1, :name)).to eq("Wayne") + end + end + + context "without data" do + let(:params) { TestContract.new({}) } + + it "returns nil for nil argument" do + expect(params.get(nil)).to be(nil) + end + + it "returns nil for unknown param" do + expect(params.get(:unknown)).to be(nil) + end + + it "returns nil for top level param" do + expect(params.get(:name)).to be(nil) + end + + it "returns nil for nested param" do + expect(params.get(:address, :line_one)).to be(nil) + end + + it "returns nil for unknown nested param" do + expect(params.get(:address, :unknown)).to be(nil) + end + end + end + + context "without data" do + let(:params) { TestContract.new({}) } + + it "returns nil for nil argument" do + expect(params.get(nil)).to be(nil) + end + + it "returns nil for unknown param" do + expect(params.get(:unknown)).to be(nil) + end + + it "returns nil for top level param" do + expect(params.get(:name)).to be(nil) + end + + it "returns nil for nested param" do + expect(params.get(:address, :line_one)).to be(nil) + end + + it "returns nil for unknown nested param" do + expect(params.get(:address, :unknown)).to be(nil) + end + end + + describe "#deconstruct_keys" do + it "supports pattern-matching" do + contract = TestContract.new(name: "Luca") + contract => { name: } + expect(name).to eq("Luca") + end + end + + describe "#to_h" do + let(:params) { TestContract.new(name: "Luca") } + + it "returns a ::Hash" do + expect(params.to_hash).to be_kind_of(::Hash) + end + + it "returns unfrozen Hash" do + expect(params.to_hash).to_not be_frozen + end + + it "handles nested params" do + input = { + "address" => { + "deep" => { + "deep_attr" => "foo" + } + } + } + + expected = { + address: { + deep: { + deep_attr: "foo" + } + } + } + + actual = TestContract.new(input).to_hash + expect(actual).to eq(expected) + + expect(actual).to be_kind_of(::Hash) + expect(actual[:address]).to be_kind_of(::Hash) + expect(actual[:address][:deep]).to be_kind_of(::Hash) + end + end + + describe "#to_hash" do + let(:params) { TestContract.new(name: "Luca") } + + it "returns a ::Hash" do + expect(params.to_hash).to be_kind_of(::Hash) + end + + it "returns unfrozen Hash" do + expect(params.to_hash).to_not be_frozen + end + + it "handles nested params" do + input = { + "address" => { + "deep" => { + "deep_attr" => "foo" + } + } + } + + expected = { + address: { + deep: { + deep_attr: "foo" + } + } + } + + actual = TestContract.new(input).to_hash + expect(actual).to eq(expected) + + expect(actual).to be_kind_of(::Hash) + expect(actual[:address]).to be_kind_of(::Hash) + expect(actual[:address][:deep]).to be_kind_of(::Hash) + end + end + + describe "#errors" do + let(:klass) do + Class.new(described_class) do + contract do + params do + required(:birth_date).filled(:date) + end + + rule(:birth_date) do + key.failure("you must be 18 years or older") if value < Date.today << (12 * 18) + end + end + end + end + + let(:params) { klass.new(birth_date: Date.today) } + + it "is of type Hanami::Action::Params::Errors" do + expect(params.errors).to be_kind_of(Hanami::Action::Params::Errors) + end + + it "affects #valid?" do + expect(params).to be_valid + + params.errors.add(:birth_date, "is not unique") + expect(params).to_not be_valid + end + + it "appends message to already existing messages" do + params = klass.new(birth_date: "") + params.errors.add(:birth_date, "is invalid") + + expect(params.error_messages).to eq(["Birth Date must be filled", "Birth Date is invalid"]) + end + + it "is included in #error_messages" do + params.errors.add(:birth_date, "is not unique") + expect(params.error_messages).to eq(["Birth Date is not unique"]) + end + end end diff --git a/spec/unit/hanami/action/params_spec.rb b/spec/unit/hanami/action/params_spec.rb index da356999..170a4985 100644 --- a/spec/unit/hanami/action/params_spec.rb +++ b/spec/unit/hanami/action/params_spec.rb @@ -495,7 +495,7 @@ expect(params).to_not be_valid end - it "appens message to already existing messages" do + it "appends message to already existing messages" do params = klass.new(book: {}) params.errors.add(:book, :code, "is invalid") From 7733380c10a896e1285794f747fbac5b5e5688eb Mon Sep 17 00:00:00 2001 From: mrimp Date: Mon, 12 Aug 2024 15:52:38 +0200 Subject: [PATCH 11/12] Final small changes --- .../request_params/action_validations.rb | 19 ------------------- lib/hanami/action/request_params/base.rb | 19 ++++++++++++++----- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/lib/hanami/action/request_params/action_validations.rb b/lib/hanami/action/request_params/action_validations.rb index a5ad8ac9..c0aaea22 100644 --- a/lib/hanami/action/request_params/action_validations.rb +++ b/lib/hanami/action/request_params/action_validations.rb @@ -126,15 +126,6 @@ def self.included(klass) klass.send :prepend, ActionValidations end - # Returns raw params from Rack env - # - # @return [Hash] - # - # @since 2.2.0 - def raw - @input - end - # Returns structured error messages # # @return [Hash] @@ -195,16 +186,6 @@ def valid? errors.empty? end - # Serialize validated params to Hash - # - # @return [::Hash] - # - # @since 2.2.0 - def to_h - @params - end - alias_method :to_hash, :to_h - # Pattern-matching support # # @return [::Hash] diff --git a/lib/hanami/action/request_params/base.rb b/lib/hanami/action/request_params/base.rb index a71e5919..978b2ad6 100644 --- a/lib/hanami/action/request_params/base.rb +++ b/lib/hanami/action/request_params/base.rb @@ -14,7 +14,7 @@ module RequestParams # since 2.2.0 # @api private module Base - # @since 0.7.0 + # @since 2.2.0 # @api public def get(*keys) @params.dig(*keys) @@ -23,14 +23,23 @@ def get(*keys) # This is for compatibility with Hanami::Helpers::FormHelper::Values # # @api private - # @since 0.8.0 + # @since 2.2.0 alias_method :dig, :get + # Returns raw params from Rack env + # + # @return [Hash] + # + # @since 2.2.0 + def raw + @input + end + # Returns a hash of the parsed request params. # # @return [Hash] # - # @since 0.7.0 + # @since 2.2.0 # @api public def to_h @params @@ -46,7 +55,7 @@ def to_h # # @return [to_h] # - # @since 0.7.1 + # @since 2.2.0 # @api public def each(&blk) to_h.each(&blk) @@ -58,7 +67,7 @@ def each(&blk) # # @return [Object,nil] the associated value, if found # - # @since 0.7.0 + # @since 2.2.0 # @api public def [](key) @params[key] From 3f26e7c1f3fc5a5e1c99b8f7ab50528d11430ff3 Mon Sep 17 00:00:00 2001 From: mrimp Date: Mon, 26 Aug 2024 19:36:19 +0200 Subject: [PATCH 12/12] Fix confusion with #raw --- lib/hanami/action/base_params.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/hanami/action/base_params.rb b/lib/hanami/action/base_params.rb index e85b44dc..3b6999c4 100644 --- a/lib/hanami/action/base_params.rb +++ b/lib/hanami/action/base_params.rb @@ -26,12 +26,6 @@ class BaseParams # @api private attr_reader :env - # @attr_reader raw [Hash] the raw params from the request - # - # @since 0.7.0 - # @api private - attr_reader :raw - # Returns a new frozen params object for the Rack env. # # @param env [Hash] a Rack env or an hash of params. @@ -40,8 +34,8 @@ class BaseParams # @api private def initialize(env) @env = env - @raw = Hanami::Action::ParamsExtraction.new(env).call - @params = Utils::Hash.deep_symbolize(@raw) + @input = Hanami::Action::ParamsExtraction.new(env).call + @params = Utils::Hash.deep_symbolize(@input) freeze end