diff --git a/CHANGELOG.md b/CHANGELOG.md index 95778516bd..4d4563faee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) ================== diff --git a/lib/grape/api.rb b/lib/grape/api.rb index d72aea15cd..78dfda1580 100644 --- a/lib/grape/api.rb +++ b/lib/grape/api.rb @@ -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 diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 18d7888c17..75ed75c48e 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -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) @@ -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 @@ -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) @@ -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 @@ -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 @@ -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 diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 05daa1ee26..63510594b5 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -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