Skip to content

Commit

Permalink
Makes #declared unavailable to before filters
Browse files Browse the repository at this point in the history
In response to issue ruby-grape#1074

This adds a mechanism by which Grape::DSL::InsideRoute methods may be
overriden after certain filters are run.

The first problem case of a filter being utilized before it should that
necessitated this enhancement is the `#declared` method, which was
returning un-coerced params in `before` filters.

There may be other problem cases, which may be rectified using this same
mechanism.
  • Loading branch information
jrforrest committed Aug 28, 2015
1 parent 6c70bbd commit 31b071a
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 31 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,10 @@ The returned hash is a `Hashie::Mash` instance, allowing you to access parameter
declared(params).user == declared(params)["user"]
```


`#declared` is not available to `before` filters, as those are evaluated prior
to parameter coercion.

### Include missing

By default `declared(params)` includes parameters that have `nil` values. If you want to return only the parameters that are not `nil`, you can use the `include_missing` option. By default, `include_missing` is set to `true`. Consider the following API:
Expand Down
85 changes: 56 additions & 29 deletions lib/grape/dsl/inside_route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,46 +6,73 @@ module InsideRoute
extend ActiveSupport::Concern
include Grape::DSL::Settings

# A filtering method that will return a hash
# consisting only of keys that have been declared by a
# `params` statement against the current/target endpoint or parent
# namespaces.
#
# @param params [Hash] The initial hash to filter. Usually this will just be `params`
# @param options [Hash] Can pass `:include_missing`, `:stringify` and `:include_parent_namespaces`
# options. `:include_parent_namespaces` defaults to true, hence must be set to false if
# you want only to return params declared against the current/target endpoint.
def declared(params, options = {}, declared_params = nil)
options = options.reverse_merge(include_missing: true, include_parent_namespaces: true)
# Denotes a situation where a DSL method has been invoked in a
# filter which it should not yet be available in
class MethodNotYetAvailable < StandardError; end

declared_params ||= (!options[:include_parent_namespaces] ? route_setting(:declared_params) : (route_setting(:saved_declared_params) || [])).flatten(1) || []
# @param type [Symbol] The type of filter for which evaluation has been
# completed
# @return [Module] A module containing method overrides suitable for the
# position in the filter evaluation sequence denoted by +type+. This
# defaults to an empty module if no overrides are defined for the given
# filter +type+.
def self.post_filter_methods(type)
@post_filter_modules ||= { before: PostBeforeFilter }
@post_filter_modules[type] || Module.new
end

fail ArgumentError, 'Tried to filter for declared parameters but none exist.' unless declared_params
# Methods which should not be available in filters until the before filter
# has completed
module PostBeforeFilter
def declared(params, options = {}, declared_params = nil)
options = options.reverse_merge(include_missing: true, include_parent_namespaces: true)

if params.is_a? Array
params.map do |param|
declared(param || {}, options, declared_params)
end
else
declared_params.each_with_object(Hashie::Mash.new) do |key, hash|
key = { key => nil } unless key.is_a? Hash
declared_params ||= (!options[:include_parent_namespaces] ? route_setting(:declared_params) : (route_setting(:saved_declared_params) || [])).flatten(1) || []

key.each_pair do |parent, children|
output_key = options[:stringify] ? parent.to_s : parent.to_sym
fail ArgumentError, 'Tried to filter for declared parameters but none exist.' unless declared_params

if params.is_a? Array
params.map do |param|
declared(param || {}, options, declared_params)
end
else
declared_params.each_with_object(Hashie::Mash.new) do |key, hash|
key = { key => nil } unless key.is_a? Hash

next unless options[:include_missing] || params.key?(parent)
key.each_pair do |parent, children|
output_key = options[:stringify] ? parent.to_s : parent.to_sym

hash[output_key] = if children
children_params = params[parent] || (children.is_a?(Array) ? [] : {})
declared(children_params, options, Array(children))
else
params[parent]
end
next unless options[:include_missing] || params.key?(parent)

hash[output_key] = if children
children_params = params[parent] || (children.is_a?(Array) ? [] : {})
declared(children_params, options, Array(children))
else
params[parent]
end
end
end
end
end
end

# A filtering method that will return a hash
# consisting only of keys that have been declared by a
# `params` statement against the current/target endpoint or parent
# namespaces.
#
# @see +PostBeforeFilter#declared+
#
# @param params [Hash] The initial hash to filter. Usually this will just be `params`
# @param options [Hash] Can pass `:include_missing`, `:stringify` and `:include_parent_namespaces`
# options. `:include_parent_namespaces` defaults to true, hence must be set to false if
# you want only to return params declared against the current/target endpoint.
def declared(*params)
raise MethodNotYetAvailable,
"#declared is not available prior to parameter validation."
end


# The API version as specified in the URL.
def version
env[Grape::Env::API_VERSION]
Expand Down
1 change: 1 addition & 0 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ def run_filters(filters, type = :other)
instance_eval(&filter)
end
end
extend DSL::InsideRoute.post_filter_methods(type)
end

def befores
Expand Down
5 changes: 3 additions & 2 deletions spec/grape/dsl/inside_route_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,9 @@ def initialize
describe '#declared' do
# see endpoint_spec.rb#declared for spec coverage

it 'returns an empty hash' do
expect(subject.declared({})).to eq({})
it 'is not available by default' do
expect { subject.declared({}) }.to raise_error(
Grape::DSL::InsideRoute::MethodNotYetAvailable)
end
end
end
Expand Down
10 changes: 10 additions & 0 deletions spec/grape/endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,16 @@ def app
end
end

it 'does not work in a before filter' do
subject.before do
declared(params)
end
subject.get('/declared'){ declared(params) }

expect { get('/declared') }.to raise_error(
Grape::DSL::InsideRoute::MethodNotYetAvailable)
end

it 'has as many keys as there are declared params' do
inner_params = nil
subject.get '/declared' do
Expand Down

0 comments on commit 31b071a

Please sign in to comment.