Skip to content

Commit

Permalink
Concatenate parent declared params with current one (#1422)
Browse files Browse the repository at this point in the history
  • Loading branch information
namusyaka authored and dblock committed Jun 11, 2016
1 parent 7805dd0 commit 864568d
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* [#1414](https://github.com/ruby-grape/grape/pull/1414): Fix multiple version definitions for path versioning - [@304](https://github.com/304).
* [#1415](https://github.com/ruby-grape/grape/pull/1415): Fix `declared(params, include_parent_namespaces: false)` - [@304](https://github.com/304).
* [#1421](https://github.com/ruby-grape/grape/pull/1421): Avoid polluting `Grape::Middleware::Error` - [@namusyaka](https://github.com/namusyaka).
* [#1422](https://github.com/ruby-grape/grape/pull/1422): Concat parent declared params with current one - [@plukevdh](https://github.com/plukevdh), [@rnubel](https://github.com/rnubel), [@namusyaka](https://github.com/namusyaka).

0.16.2 (4/12/2016)
==================
Expand Down
7 changes: 7 additions & 0 deletions lib/grape/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ def inherited(subclass)
def inherit_settings(other_settings)
top_level_setting.inherit_from other_settings.point_in_time_copy

# Propagate any inherited params down to our endpoints, and reset any
# compiled routes.
endpoints.each do |e|
e.inherit_settings(top_level_setting.namespace_stackable)
e.reset_routes!
end

reset_routes!
end
end
Expand Down
44 changes: 30 additions & 14 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,7 @@ class Endpoint

class << self
def new(*args, &block)
if self == Endpoint
Class.new(Endpoint).new(*args, &block)
else
super
end
self == Endpoint ? Class.new(Endpoint).new(*args, &block) : super
end

def before_each(new_setup = false, &block)
Expand All @@ -33,9 +29,7 @@ def before_each(new_setup = false, &block)

def run_before_each(endpoint)
superclass.run_before_each(endpoint) unless self == Endpoint
before_each.each do |blk|
blk.call(endpoint) if blk.respond_to? :call
end
before_each.each { |blk| blk.call(endpoint) if blk.respond_to?(:call) }
end

# @api private
Expand Down Expand Up @@ -68,6 +62,18 @@ def generate_api_method(method_name, &block)
end
end

# Create a new endpoint.
# @param new_settings [InheritableSetting] settings to determine the params,
# validations, and other properties from.
# @param options [Hash] attributes of this endpoint
# @option options path [String or Array] the path to this endpoint, within
# the current scope.
# @option options method [String or Array] which HTTP method(s) can be used
# to reach this endpoint.
# @option options route_options [Hash]
# @note This happens at the time of API definition, so in this context the
# endpoint does not know if it will be mounted under a different endpoint.
# @yield a block defining what your API should do when this endpoint is hit
def initialize(new_settings, options = {}, &block)
require_option(options, :path)
require_option(options, :method)
Expand Down Expand Up @@ -96,6 +102,20 @@ def initialize(new_settings, options = {}, &block)
@block = self.class.generate_api_method(method_name, &block)
end

# Update our settings from a given set of stackable parameters. Used when
# the endpoint's API is mounted under another one.
def inherit_settings(namespace_stackable)
inheritable_setting.route[:saved_validations] += namespace_stackable[:validations]
parent_declared_params = namespace_stackable[:declared_params]

if parent_declared_params
inheritable_setting.route[:declared_params] ||= []
inheritable_setting.route[:declared_params].concat(parent_declared_params.flatten)
end

endpoints && endpoints.each { |e| e.inherit_settings(namespace_stackable) }
end

def require_option(options, key)
raise Grape::Exceptions::MissingOption.new(key) unless options.key?(key)
end
Expand Down Expand Up @@ -284,9 +304,7 @@ def build_stack(helpers)

def build_helpers
helpers = namespace_stackable(:helpers) || []
Module.new do
helpers.each { |mod_to_include| include mod_to_include }
end
Module.new { helpers.each { |mod_to_include| include mod_to_include } }
end

private :build_stack, :build_helpers
Expand All @@ -301,9 +319,7 @@ def lazy_initialize!
@lazy_initialize_lock.synchronize do
return true if @lazy_initialized

@helpers = build_helpers.tap do |mod|
self.class.send(:include, mod)
end
@helpers = build_helpers.tap { |mod| self.class.send(:include, mod) }
@app = options[:app] || build_stack(@helpers)

@lazy_initialized = true
Expand Down
50 changes: 50 additions & 0 deletions spec/grape/endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,56 @@ def app
end
end

describe '#declared; from a nested mounted endpoint' do
before do
doubly_mounted = Class.new(Grape::API)
doubly_mounted.namespace :more do
params do
requires :y, type: Integer
end
route_param :y do
get do
{
params: params,
declared_params: declared(params)
}
end
end
end

mounted = Class.new(Grape::API)
mounted.namespace :another do
params do
requires :mount_space, type: Integer
end
route_param :mount_space do
mount doubly_mounted
end
end

subject.format :json
subject.namespace :something do
params do
requires :id, type: Integer
end
resource ':id' do
mount mounted
end
end
end

it 'can access parent attributes' do
get '/something/123/another/456/more/789'
expect(last_response.status).to eq 200
json = JSON.parse(last_response.body, symbolize_names: true)

# test all three levels of params
expect(json[:declared_params][:y]).to eq 789
expect(json[:declared_params][:mount_space]).to eq 456
expect(json[:declared_params][:id]).to eq 123
end
end

describe '#params' do
it 'is available to the caller' do
subject.get('/hey') do
Expand Down

0 comments on commit 864568d

Please sign in to comment.