From 2d0a2a7a8ac71c42dc2ba06bf0b5796865d22461 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Glauco=20Cust=C3=B3dio?= Date: Tue, 18 Dec 2018 09:53:36 +0000 Subject: [PATCH 1/3] add failing test --- spec/grape/validations/params_scope_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/grape/validations/params_scope_spec.rb b/spec/grape/validations/params_scope_spec.rb index 10b21db61..12865b8bc 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/spec/grape/validations/params_scope_spec.rb @@ -155,6 +155,16 @@ def initialize(value) expect(last_response.status).to eq(400) expect(last_response.body).to eq('foo is empty') end + + it do + subject.params do + requires :foo, as: :bar, allow_blank: false + end + subject.get('/alias-not-blank-with-value') {} + get '/alias-not-blank-with-value', foo: 'any' + + expect(last_response.status).to eq(200) + end end context 'array without coerce type explicitly given' do From 2dcd425de5db7eeb4b203cb2edd8e75a94692d2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Glauco=20Cust=C3=B3dio?= Date: Tue, 18 Dec 2018 19:51:44 +0000 Subject: [PATCH 2/3] fix: validators checking aliased param instead of original one --- CHANGELOG.md | 4 ++-- lib/grape/validations/params_scope.rb | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffbdac6ed..deed5cd74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,11 +5,11 @@ * Your contribution here. * [#1850](https://github.com/ruby-grape/grape/pull/1850): Adds `same_as` validator - [@glaucocustodio](https://github.com/glaucocustodio). * [#1833](https://github.com/ruby-grape/grape/pull/1833): Allows to set the `ParamBuilder` globally - [@myxoh](https://github.com/myxoh). -* [#1844](https://github.com/ruby-grape/grape/pull/1844): Fix: enforce `:tempfile` to be a `Tempfile` object in `File` validator - [@Nyangawa](https://github.com/Nyangawa). #### Fixes -* Your contribution here. +* [#1852](https://github.com/ruby-grape/grape/pull/1852): Validators checking aliased param instead of original one - [@glaucocustodio](https://github.com/glaucocustodio). +* [#1844](https://github.com/ruby-grape/grape/pull/1844): Enforce `:tempfile` to be a `Tempfile` object in `File` validator - [@Nyangawa](https://github.com/Nyangawa). ### 1.2.2 (2018/12/07) diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index 05c0c8c0a..2e68ea8a6 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -293,6 +293,8 @@ def validates(attrs, validations) # type casted values coerce_type validations, attrs, doc_attrs, opts + move_alias_to_end(validations) + validations.each do |type, options| validate(type, options, attrs, doc_attrs, opts) end @@ -431,6 +433,11 @@ def validate_value_coercion(coerce_type, *values_list) end end + def move_alias_to_end(validations) + alias_validation = validations.delete(:as) + validations[:as] = alias_validation if alias_validation + end + def extract_message_option(attrs) return nil unless attrs.is_a?(Array) opts = attrs.last.is_a?(Hash) ? attrs.pop : {} From 41de94ea10f14a5dc68fc59a7c891760e6f7a14e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Glauco=20Cust=C3=B3dio?= Date: Wed, 19 Dec 2018 19:36:13 +0000 Subject: [PATCH 3/3] change fix to not rely on the order of validators --- .rubocop_todo.yml | 8 ++++---- CHANGELOG.md | 2 +- lib/grape/endpoint.rb | 10 +++++++++- lib/grape/validations/params_scope.rb | 7 ------- lib/grape/validations/validators/as.rb | 1 - 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 4c81012cd..627a5a51a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2018-12-06 21:06:59 -0500 using RuboCop version 0.51.0. +# on 2018-12-20 21:38:24 +0000 using RuboCop version 0.51.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -38,13 +38,13 @@ Metrics/BlockLength: # Offense count: 9 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 288 + Max: 295 # Offense count: 31 Metrics/CyclomaticComplexity: Max: 14 -# Offense count: 1227 +# Offense count: 1234 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https Metrics/LineLength: @@ -55,7 +55,7 @@ Metrics/LineLength: Metrics/MethodLength: Max: 33 -# Offense count: 11 +# Offense count: 12 # Configuration parameters: CountComments. Metrics/ModuleLength: Max: 220 diff --git a/CHANGELOG.md b/CHANGELOG.md index deed5cd74..6fcf57a14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ #### Fixes -* [#1852](https://github.com/ruby-grape/grape/pull/1852): Validators checking aliased param instead of original one - [@glaucocustodio](https://github.com/glaucocustodio). +* [#1852](https://github.com/ruby-grape/grape/pull/1852): `allow_blank` called after `as` when the original param is not blank - [@glaucocustodio](https://github.com/glaucocustodio). * [#1844](https://github.com/ruby-grape/grape/pull/1844): Enforce `:tempfile` to be a `Tempfile` object in `File` validator - [@Nyangawa](https://github.com/Nyangawa). ### 1.2.2 (2018/12/07) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 5100f9654..650c53203 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -258,6 +258,7 @@ def run else run_filters before_validations, :before_validation run_validators validations, request + remove_aliased_params run_filters after_validations, :after_validation response_object = @block ? @block.call(self) : nil end @@ -319,7 +320,14 @@ def build_helpers Module.new { helpers.each { |mod_to_include| include mod_to_include } } end - private :build_stack, :build_helpers + def remove_aliased_params + return unless route_setting(:aliased_params) + route_setting(:aliased_params).flat_map(&:keys).each do |aliased_param| + @params.delete(aliased_param) + end + end + + private :build_stack, :build_helpers, :remove_aliased_params def helpers lazy_initialize! && @helpers diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index 2e68ea8a6..05c0c8c0a 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -293,8 +293,6 @@ def validates(attrs, validations) # type casted values coerce_type validations, attrs, doc_attrs, opts - move_alias_to_end(validations) - validations.each do |type, options| validate(type, options, attrs, doc_attrs, opts) end @@ -433,11 +431,6 @@ def validate_value_coercion(coerce_type, *values_list) end end - def move_alias_to_end(validations) - alias_validation = validations.delete(:as) - validations[:as] = alias_validation if alias_validation - end - def extract_message_option(attrs) return nil unless attrs.is_a?(Array) opts = attrs.last.is_a?(Hash) ? attrs.pop : {} diff --git a/lib/grape/validations/validators/as.rb b/lib/grape/validations/validators/as.rb index d1840539b..7547c8edd 100644 --- a/lib/grape/validations/validators/as.rb +++ b/lib/grape/validations/validators/as.rb @@ -8,7 +8,6 @@ def initialize(attrs, options, required, scope, opts = {}) def validate_param!(attr_name, params) params[@alias] = params[attr_name] - params.delete(attr_name) end end end