From fdeb01c9a3ad74cf0541905e5f8b963ddb23f7ad Mon Sep 17 00:00:00 2001 From: Dan Healy Date: Tue, 14 Nov 2023 13:24:46 -0800 Subject: [PATCH 1/6] wip using contract dsl --- lib/hanami/action.rb | 4 ++ lib/hanami/action/params.rb | 43 +++++++++++- lib/hanami/action/validatable.rb | 9 +++ spec/unit/hanami/action/params_spec.rb | 13 ++-- spec/unit/hanami/action/rules_spec.rb | 93 ++++++++++++++++++++++++++ 5 files changed, 151 insertions(+), 11 deletions(-) create mode 100644 spec/unit/hanami/action/rules_spec.rb diff --git a/lib/hanami/action.rb b/lib/hanami/action.rb index 4e1015cb..38f481de 100644 --- a/lib/hanami/action.rb +++ b/lib/hanami/action.rb @@ -138,6 +138,10 @@ def self.params(_klass = nil) "To use `params`, please add 'hanami/validations' gem to your Gemfile" end + def self.contract(_klass = nil) + puts "Contract" + 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 diff --git a/lib/hanami/action/params.rb b/lib/hanami/action/params.rb index 2e1dfe83..4cfdef5c 100644 --- a/lib/hanami/action/params.rb +++ b/lib/hanami/action/params.rb @@ -148,6 +148,45 @@ def self.params(&blk) validations(&blk || -> {}) end + # Define full contract validations. This takes precendence over the older `params` DSL. + # + # @param blk [Proc] the validation definitions + # + # @since 2.1.0 + # + # @example + # class Event < Hanami::Action + # contract do + # params do + # required(:start_date).value(:date) + # required(:end_date).value(:date) + # end + # rule do + # if start_date > end_date + # base.failure('event cannot end before it starts') + # end + # end + # end + # + # def handle(req, *) + # halt 400 unless req.params.valid? + # # ... + # end + # end + def self.contract(&blk) + @_contract = Dry::Validation::Contract.build(&blk || -> {}) + end + + # Accessor for the validations contract + # + # @return [Dry::Validation::Contract, NilClass] + # + # @since 2.1.0 + # @api private + def self._contract + @_contract + end + # Initialize the params and freeze them. # # @param env [Hash] a Rack env or an hash of params. @@ -159,9 +198,9 @@ def self.params(&blk) def initialize(env) @env = env super(_extract_params) - validation = validate + validation = self.class._contract ? self.class._contract.call(@input) : validate @params = validation.to_h - @errors = Errors.new(validation.messages) + @errors = Errors.new(self.class._contract ? validation.errors.to_h : validation.messages) freeze end diff --git a/lib/hanami/action/validatable.rb b/lib/hanami/action/validatable.rb index 6eaceb64..d2e96753 100644 --- a/lib/hanami/action/validatable.rb +++ b/lib/hanami/action/validatable.rb @@ -105,6 +105,15 @@ def params(klass = nil, &blk) @params_class = klass end + + def contract(klass = nil, &blk) + if klass.nil? + klass = const_set(PARAMS_CLASS_NAME, Class.new(Params)) + klass.class_eval { contract(&blk) } + end + + @params_class = klass + end end end end diff --git a/spec/unit/hanami/action/params_spec.rb b/spec/unit/hanami/action/params_spec.rb index da356999..7842ecc9 100644 --- a/spec/unit/hanami/action/params_spec.rb +++ b/spec/unit/hanami/action/params_spec.rb @@ -3,15 +3,10 @@ require "rack" RSpec.describe Hanami::Action::Params do - xit "is frozen" - - # This is temporary suspended. - # We need to get the dependency Hanami::Validations, more stable before to enable this back. - # - # it 'is frozen' do - # params = Hanami::Action::Params.new({id: '23'}) - # params.must_be :frozen? - # end + xit 'is frozen' do + params = Hanami::Action::Params.new({id: '23'}) + params.must_be :frozen? + end describe "#raw" do let(:params) { Class.new(Hanami::Action::Params) } diff --git a/spec/unit/hanami/action/rules_spec.rb b/spec/unit/hanami/action/rules_spec.rb new file mode 100644 index 00000000..f055ed34 --- /dev/null +++ b/spec/unit/hanami/action/rules_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require "rack" + +RSpec.describe Hanami::Action do + # Deprecated behavior + describe ".params" do + let(:klass) do + Class.new(described_class) do + puts "Defining params" + params do + required(:book).schema do + required(:code).filled(:str?) + end + end + + def handle(request, response) + response[:params] = request.params + end + end + end + + let(:action) { klass.new() } + let(:response) { action.call(given_input) } + + context "given valid input" do + let(:given_input) { { book: { code: "abc" } } } + + it "is valid" do + expect(response[:params].valid?).to eq(true) + end + end + + context "given invalid input" do + let(:given_input) { { book: { code: nil } } } + + it "is not valid" do + expect(response[:params].valid?).to eq(false) + end + end + end + + describe ".contract" do + + context "when providing a block" do + let(:klass) do + Class.new(described_class) do + contract do + params do + required(:book).schema do + required(:code).filled(:str?) + end + end + rule("book.code") do + key.failure('must be "abc"') unless value == "abc" + end + end + + def handle(request, response) + response[:params] = request.params + end + end + end + + let(:action) { klass.new() } + let(:response) { action.call(given_input) } + + context "given valid input" do + let(:given_input) { { book: { code: "abc" } } } + + it "is valid" do + expect(response[:params].valid?).to eq(true) + end + end + + context "given invalid input" do + let(:given_input) { { book: { code: nil } } } + + it "is not valid" do + expect(response[:params].valid?).to eq(false) + end + end + + context "given input which does not pass rules" do + let(:given_input) { { book: { code: "xyz" } } } + + it "is not valid" do + expect(response[:params].valid?).to eq(false) + end + end + end + end +end From 8241383139d0a22b72bda54bfa6f66d648ac1400 Mon Sep 17 00:00:00 2001 From: Dan Healy Date: Wed, 15 Nov 2023 19:46:45 -0800 Subject: [PATCH 2/6] Validatable rule definitions pass to Contract --- lib/hanami/action.rb | 4 -- lib/hanami/action/params.rb | 47 ++++++++---------- lib/hanami/action/validatable.rb | 37 +++++++++++--- spec/unit/hanami/action/params_spec.rb | 13 +++-- spec/unit/hanami/action/rules_spec.rb | 67 ++++++++++++-------------- 5 files changed, 92 insertions(+), 76 deletions(-) diff --git a/lib/hanami/action.rb b/lib/hanami/action.rb index 38f481de..4e1015cb 100644 --- a/lib/hanami/action.rb +++ b/lib/hanami/action.rb @@ -138,10 +138,6 @@ def self.params(_klass = nil) "To use `params`, please add 'hanami/validations' gem to your Gemfile" end - def self.contract(_klass = nil) - puts "Contract" - 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 diff --git a/lib/hanami/action/params.rb b/lib/hanami/action/params.rb index 4cfdef5c..1ff4a9f1 100644 --- a/lib/hanami/action/params.rb +++ b/lib/hanami/action/params.rb @@ -148,23 +148,23 @@ def self.params(&blk) validations(&blk || -> {}) end - # Define full contract validations. This takes precendence over the older `params` DSL. + # Define a validation rule. # - # @param blk [Proc] the validation definitions + # @param blk [Proc] the rule definition # - # @since 2.1.0 + # @since 2.x.x # # @example # class Event < Hanami::Action - # contract do - # params do - # required(:start_date).value(:date) - # required(:end_date).value(:date) - # end - # rule do - # if start_date > end_date - # base.failure('event cannot end before it starts') - # end + # params do + # required(:start_date).value(:date) + # required(:end_date).value(:date) + # end + # + # # Rules must be defined after the params schema. + # rule(:start_date, :end_date) do + # if start_date > end_date + # base.failure('event cannot end before it starts') # end # end # @@ -173,18 +173,13 @@ def self.params(&blk) # # ... # end # end - def self.contract(&blk) - @_contract = Dry::Validation::Contract.build(&blk || -> {}) - end - - # Accessor for the validations contract - # - # @return [Dry::Validation::Contract, NilClass] - # - # @since 2.1.0 - # @api private - def self._contract - @_contract + def self.rule(*keys, &blk) + unless self._validator + raise ArgumentError.new( + "The params schema must be defined prior to adding rules." + ) + end + self._validator.class.rule(*keys, &blk) end # Initialize the params and freeze them. @@ -198,9 +193,9 @@ def self._contract def initialize(env) @env = env super(_extract_params) - validation = self.class._contract ? self.class._contract.call(@input) : validate + validation = validate @params = validation.to_h - @errors = Errors.new(self.class._contract ? validation.errors.to_h : validation.messages) + @errors = Errors.new(validation.messages) freeze end diff --git a/lib/hanami/action/validatable.rb b/lib/hanami/action/validatable.rb index d2e96753..415d5651 100644 --- a/lib/hanami/action/validatable.rb +++ b/lib/hanami/action/validatable.rb @@ -106,13 +106,38 @@ def params(klass = nil, &blk) @params_class = klass end - def contract(klass = nil, &blk) - if klass.nil? - klass = const_set(PARAMS_CLASS_NAME, Class.new(Params)) - klass.class_eval { contract(&blk) } + # Define a validation rule. + # + # @param blk [Proc] the rule definition + # + # @since 2.x.x + # + # @example + # class Event < Hanami::Action + # params do + # required(:start_date).value(:date) + # required(:end_date).value(:date) + # end + # + # # Rules must be defined after the params schema. + # rule(:start_date, :end_date) do + # if start_date > end_date + # base.failure('event cannot end before it starts') + # end + # end + # + # def handle(req, *) + # halt 400 unless req.params.valid? + # # ... + # end + # end + def rule(*keys, &blk) + unless @params_class + raise ArgumentError.new( + "The params schema must be defined prior to adding rules." + ) end - - @params_class = klass + @params_class.class_eval { rule(*keys, &blk) } end end end diff --git a/spec/unit/hanami/action/params_spec.rb b/spec/unit/hanami/action/params_spec.rb index 7842ecc9..da356999 100644 --- a/spec/unit/hanami/action/params_spec.rb +++ b/spec/unit/hanami/action/params_spec.rb @@ -3,10 +3,15 @@ require "rack" RSpec.describe Hanami::Action::Params do - xit 'is frozen' do - params = Hanami::Action::Params.new({id: '23'}) - params.must_be :frozen? - end + xit "is frozen" + + # This is temporary suspended. + # We need to get the dependency Hanami::Validations, more stable before to enable this back. + # + # it 'is frozen' do + # params = Hanami::Action::Params.new({id: '23'}) + # params.must_be :frozen? + # end describe "#raw" do let(:params) { Class.new(Hanami::Action::Params) } diff --git a/spec/unit/hanami/action/rules_spec.rb b/spec/unit/hanami/action/rules_spec.rb index f055ed34..86aa2885 100644 --- a/spec/unit/hanami/action/rules_spec.rb +++ b/spec/unit/hanami/action/rules_spec.rb @@ -7,7 +7,6 @@ describe ".params" do let(:klass) do Class.new(described_class) do - puts "Defining params" params do required(:book).schema do required(:code).filled(:str?) @@ -40,53 +39,49 @@ def handle(request, response) end end - describe ".contract" do - - context "when providing a block" do - let(:klass) do - Class.new(described_class) do - contract do - params do - required(:book).schema do - required(:code).filled(:str?) - end - end - rule("book.code") do - key.failure('must be "abc"') unless value == "abc" - end - end - - def handle(request, response) - response[:params] = request.params + describe ".rule" do + let(:klass) do + Class.new(described_class) do + params do + required(:book).schema do + required(:code).filled(:str?) end end + + rule("book.code") do + key.failure('must be "abc"') unless value == "abc" + end + + def handle(request, response) + response[:params] = request.params + end end + end - let(:action) { klass.new() } - let(:response) { action.call(given_input) } + let(:action) { klass.new() } + let(:response) { action.call(given_input) } - context "given valid input" do - let(:given_input) { { book: { code: "abc" } } } + context "given valid input" do + let(:given_input) { { book: { code: "abc" } } } - it "is valid" do - expect(response[:params].valid?).to eq(true) - end + it "is valid" do + expect(response[:params].valid?).to eq(true) end + end - context "given invalid input" do - let(:given_input) { { book: { code: nil } } } + context "given invalid input" do + let(:given_input) { { book: { code: nil } } } - it "is not valid" do - expect(response[:params].valid?).to eq(false) - end + it "is not valid" do + expect(response[:params].valid?).to eq(false) end + end - context "given input which does not pass rules" do - let(:given_input) { { book: { code: "xyz" } } } + context "given input which does not pass rules" do + let(:given_input) { { book: { code: "xyz" } } } - it "is not valid" do - expect(response[:params].valid?).to eq(false) - end + it "is not valid" do + expect(response[:params].valid?).to eq(false) end end end From eb1e909195acb8538753ec1b9e9240900c4b9a40 Mon Sep 17 00:00:00 2001 From: Dan Healy Date: Thu, 16 Nov 2023 16:08:07 -0800 Subject: [PATCH 3/6] Remove extra guard in params.rb Co-authored-by: Philip Arndt --- lib/hanami/action/params.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/hanami/action/params.rb b/lib/hanami/action/params.rb index 1ff4a9f1..0f97a318 100644 --- a/lib/hanami/action/params.rb +++ b/lib/hanami/action/params.rb @@ -174,11 +174,6 @@ def self.params(&blk) # end # end def self.rule(*keys, &blk) - unless self._validator - raise ArgumentError.new( - "The params schema must be defined prior to adding rules." - ) - end self._validator.class.rule(*keys, &blk) end From 04454e717dff5a6684ac731579af3b1dca9f29aa Mon Sep 17 00:00:00 2001 From: Philip Arndt Date: Fri, 17 Nov 2023 13:13:08 +1300 Subject: [PATCH 4/6] Fix rubocop offenses --- spec/unit/hanami/action/rules_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/unit/hanami/action/rules_spec.rb b/spec/unit/hanami/action/rules_spec.rb index 86aa2885..2128cce8 100644 --- a/spec/unit/hanami/action/rules_spec.rb +++ b/spec/unit/hanami/action/rules_spec.rb @@ -23,7 +23,7 @@ def handle(request, response) let(:response) { action.call(given_input) } context "given valid input" do - let(:given_input) { { book: { code: "abc" } } } + let(:given_input) { {book: {code: "abc"}} } it "is valid" do expect(response[:params].valid?).to eq(true) @@ -31,7 +31,7 @@ def handle(request, response) end context "given invalid input" do - let(:given_input) { { book: { code: nil } } } + let(:given_input) { {book: {code: nil}} } it "is not valid" do expect(response[:params].valid?).to eq(false) @@ -62,7 +62,7 @@ def handle(request, response) let(:response) { action.call(given_input) } context "given valid input" do - let(:given_input) { { book: { code: "abc" } } } + let(:given_input) { {book: {code: "abc"}} } it "is valid" do expect(response[:params].valid?).to eq(true) @@ -70,7 +70,7 @@ def handle(request, response) end context "given invalid input" do - let(:given_input) { { book: { code: nil } } } + let(:given_input) { {book: {code: nil}} } it "is not valid" do expect(response[:params].valid?).to eq(false) @@ -78,7 +78,7 @@ def handle(request, response) end context "given input which does not pass rules" do - let(:given_input) { { book: { code: "xyz" } } } + let(:given_input) { {book: {code: "xyz"}} } it "is not valid" do expect(response[:params].valid?).to eq(false) From 66dc22849e827114ec26c4d40463f93532981215 Mon Sep 17 00:00:00 2001 From: Dan Healy Date: Thu, 16 Nov 2023 16:17:36 -0800 Subject: [PATCH 5/6] Fix some more rubocop issues --- lib/hanami/action/params.rb | 4 ++-- lib/hanami/action/validatable.rb | 4 ++-- spec/unit/hanami/action/rules_spec.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/hanami/action/params.rb b/lib/hanami/action/params.rb index 0f97a318..2130b54f 100644 --- a/lib/hanami/action/params.rb +++ b/lib/hanami/action/params.rb @@ -173,8 +173,8 @@ def self.params(&blk) # # ... # end # end - def self.rule(*keys, &blk) - self._validator.class.rule(*keys, &blk) + def self.rule(...) + _validator.class.rule(...) end # Initialize the params and freeze them. diff --git a/lib/hanami/action/validatable.rb b/lib/hanami/action/validatable.rb index 415d5651..43436dc0 100644 --- a/lib/hanami/action/validatable.rb +++ b/lib/hanami/action/validatable.rb @@ -131,13 +131,13 @@ def params(klass = nil, &blk) # # ... # end # end - def rule(*keys, &blk) + def rule(...) unless @params_class raise ArgumentError.new( "The params schema must be defined prior to adding rules." ) end - @params_class.class_eval { rule(*keys, &blk) } + @params_class.class_eval { rule(...) } end end end diff --git a/spec/unit/hanami/action/rules_spec.rb b/spec/unit/hanami/action/rules_spec.rb index 2128cce8..d0dfe113 100644 --- a/spec/unit/hanami/action/rules_spec.rb +++ b/spec/unit/hanami/action/rules_spec.rb @@ -19,7 +19,7 @@ def handle(request, response) end end - let(:action) { klass.new() } + let(:action) { klass.new } let(:response) { action.call(given_input) } context "given valid input" do @@ -58,7 +58,7 @@ def handle(request, response) end end - let(:action) { klass.new() } + let(:action) { klass.new } let(:response) { action.call(given_input) } context "given valid input" do From c173f10d7bfd826ec31f66e2db9644a42c87527c Mon Sep 17 00:00:00 2001 From: Dan Healy Date: Fri, 17 Nov 2023 10:03:11 -0800 Subject: [PATCH 6/6] Remove erroneous comment --- spec/unit/hanami/action/rules_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/unit/hanami/action/rules_spec.rb b/spec/unit/hanami/action/rules_spec.rb index d0dfe113..49fcc7cd 100644 --- a/spec/unit/hanami/action/rules_spec.rb +++ b/spec/unit/hanami/action/rules_spec.rb @@ -3,7 +3,6 @@ require "rack" RSpec.describe Hanami::Action do - # Deprecated behavior describe ".params" do let(:klass) do Class.new(described_class) do