Skip to content

Commit

Permalink
Fix unknown validator exception when using requires/optional with Ent…
Browse files Browse the repository at this point in the history
…ity (ruby-grape#2338)

* Fix crash when using requires/optional with Entity

when that entity specifies keywords that grape interprets as custom validators, ie: is_array, param_type, etc.

This PR changes it so that it skips looking for a custom validator for those special documentation keywords.

This allows you to do something like:

```rb
desc 'Create some entity',
     entity: SomeEntity,
     params: SomeEntity.documentation
params do
  requires :none,
           except: %i[field1 field2],
           using:
             SomeEntity.documentation
end
post do
 ...
end
```

and SomeEntity can specify documentation like:

```rb
      class SomeEntity < Grape::Entity
        expose :id, documentation: { type: Integer, format: 'int64', desc: 'ID' }
        expose :name, documentation: { type: String, desc: 'Name', require: true, param_type: 'body' }
        expose :array_field,
               documentation: {
                 type: String,
                 desc: 'Array Field',
                 require: true,
                 param_type: 'bold'
               }
      end
```

and it won't crash on startup and give you correct documentation and param validation to boot.

Open questions:
1. Is this an exhaustive list of keywords?
2. Is there a better way to maintain/get the list of keywords?
3. Is there a better approach altogeher?

* Cleanup code and add to existing tests that fail without these changes.

* Update after merge with master

* PR feedback
  • Loading branch information
mscrivo authored Jul 4, 2023
1 parent 96ac079 commit 8e1488d
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 8 deletions.
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config --auto-gen-only-exclude --exclude-limit 5000`
# on 2023-07-01 15:43:53 UTC using RuboCop version 1.50.2.
# on 2023-07-04 00:22:04 UTC using RuboCop version 1.50.2.
# 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
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* [#2333](https://github.com/ruby-grape/grape/pull/2333): Use custom messages in parameter validation with arity 1 - [@thedevjoao](https://github.com/TheDevJoao).
* [#2341](https://github.com/ruby-grape/grape/pull/2341): Stop yielding skip value - [@ericproulx](https://github.com/ericproulx).
* [#2342](https://github.com/ruby-grape/grape/pull/2342): Allow specifying a handler for grape_exceptions - [@mscrivo](https://github.com/mscrivo).
* [#2338](https://github.com/ruby-grape/grape/pull/2338): Fix unknown validator when using requires/optional with entity - [@mscrivo](https://github.com/mscrivo).
* Your contribution here.

#### Fixes
Expand Down
8 changes: 7 additions & 1 deletion lib/grape/validations/params_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ class ParamsScope

include Grape::DSL::Parameters

# There are a number of documentation options on entities that don't have
# corresponding validators. Since there is nowhere that enumerates them all,
# we maintain a list of them here and skip looking up validators for them.
RESERVED_DOCUMENTATION_KEYWORDS = %i[as required param_type is_array format example].freeze

class Attr
attr_accessor :key, :scope

Expand Down Expand Up @@ -359,7 +364,8 @@ def validates(attrs, validations)
coerce_type validations, attrs, doc, opts

validations.each do |type, options|
next if type == :as
# Don't try to look up validators for documentation params that don't have one.
next if RESERVED_DOCUMENTATION_KEYWORDS.include?(type)

validate(type, options, attrs, doc, opts)
end
Expand Down
13 changes: 7 additions & 6 deletions spec/grape/validations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,12 @@ def define_optional_using
context 'requires :all using Grape::Entity documentation' do
def define_requires_all
documentation = {
required_field: { type: String },
optional_field: { type: String }
required_field: { type: String, required: true, param_type: 'query' },
optional_field: { type: String },
optional_array_field: { type: Array[String], is_array: true }
}
subject.params do
requires :all, except: :optional_field, using: documentation
requires :all, except: %i[optional_field optional_array_field], using: documentation
end
end
before do
Expand All @@ -195,7 +196,7 @@ def define_requires_all

it 'adds entity documentation to declared params' do
define_requires_all
expect(Grape::Validations::ParamsScope::Attr.attrs_keys(declared_params)).to eq(%i[required_field optional_field])
expect(Grape::Validations::ParamsScope::Attr.attrs_keys(declared_params)).to eq(%i[required_field optional_field optional_array_field])
end

it 'errors when required_field is not present' do
Expand All @@ -214,8 +215,8 @@ def define_requires_all
context 'requires :none using Grape::Entity documentation' do
def define_requires_none
documentation = {
required_field: { type: String },
optional_field: { type: String }
required_field: { type: String, example: 'Foo' },
optional_field: { type: Integer, format: 'int64' }
}
subject.params do
requires :none, except: :required_field, using: documentation
Expand Down

0 comments on commit 8e1488d

Please sign in to comment.