diff --git a/.rubocop.yml b/.rubocop.yml index cf3c032e4..fd41df233 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -52,7 +52,7 @@ Metrics/BlockLength: - spec/**/*_spec.rb Metrics/ClassLength: - Max: 340 + Max: 370 Metrics/CyclomaticComplexity: Max: 15 diff --git a/CHANGELOG.md b/CHANGELOG.md index c2aa94494..602bf71ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ * [#2808](https://github.com/ruby-grape/grape/pull/2808): Internalize `build_params_with` and `auth` behind `Grape::Util::InheritableSetting` accessors - [@ericproulx](https://github.com/ericproulx). * [#2807](https://github.com/ruby-grape/grape/pull/2807): Internalize routing scope flags behind `Grape::Util::InheritableSetting` accessors (`do_not_route_head`, `do_not_route_options`, `do_not_document`, `lint` — `!` writers / `?` readers), and drop the vestigial `namespace_inheritable` hash copy in `Grape::API::Instance`'s route-config collection - [@ericproulx](https://github.com/ericproulx). * [#2810](https://github.com/ruby-grape/grape/pull/2810): Make `Grape::Util::InheritableSetting`'s raw stores internal where the ecosystem allows: `namespace_inheritable` is now protected and `namespace_stackable_with_hash` private, while `namespace_stackable` stays public for grape-swagger; `Router::Pattern::Path` reads a dedicated `path_settings` snapshot, and `mount_paths` exposes the full mount-path stack - [@ericproulx](https://github.com/ericproulx). +* [#2811](https://github.com/ruby-grape/grape/pull/2811): Internalize the route scope behind `Grape::Util::InheritableSetting` accessors (`route_validations`, `route_declared_params`, `route_renamed_params` / `add_route_renamed_param`, `route_description`, `route_settings`, `route_setting`, `inherit_route_params`), so no external code touches the raw `route` store - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/lib/grape/dsl/declared.rb b/lib/grape/dsl/declared.rb index 05096bd6b..ddbb7ab78 100644 --- a/lib/grape/dsl/declared.rb +++ b/lib/grape/dsl/declared.rb @@ -24,8 +24,8 @@ def declared(passed_params, include_parent_namespaces: true, include_missing: tr contract_key_map = inheritable_setting.contract_key_maps handler = DeclaredParamsHandler.new(include_missing:, evaluate_given:, stringify:, contract_key_map:) - declared_params = include_parent_namespaces ? inheritable_setting.route[:declared_params] : (inheritable_setting.declared_params.last || []) - renamed_params = inheritable_setting.route[:renamed_params] || {} + declared_params = include_parent_namespaces ? inheritable_setting.route_declared_params : (inheritable_setting.declared_params.last || []) + renamed_params = inheritable_setting.route_renamed_params route_params = config.params handler.call(passed_params, declared_params, route_params, renamed_params) diff --git a/lib/grape/dsl/desc.rb b/lib/grape/dsl/desc.rb index 3d358bfd4..27a027e6e 100644 --- a/lib/grape/dsl/desc.rb +++ b/lib/grape/dsl/desc.rb @@ -66,7 +66,7 @@ def desc(description, *legacy_options, **options, &config_block) # Only the route scope is consumed downstream (by +route+ and the # route's readers, e.g. +http_codes+); the namespace scope was # write-only, so it is no longer populated. - inheritable_setting.route[:description] = settings + inheritable_setting.route_description = settings end end end diff --git a/lib/grape/dsl/routing.rb b/lib/grape/dsl/routing.rb index c202959b3..cd478c369 100644 --- a/lib/grape/dsl/routing.rb +++ b/lib/grape/dsl/routing.rb @@ -184,7 +184,7 @@ def mount(mounts, *opts) # end def route(methods, paths = ['/'], requirements: nil, anchor: true, **route_options, &) http_methods = methods == :any ? '*' : methods - endpoint_description = inheritable_setting.route[:description] || {} + endpoint_description = inheritable_setting.route_description # +params+, +requirements+ and +anchor+ each travel as their own endpoint # input; the route-options bag keeps the description's other keys diff --git a/lib/grape/dsl/settings.rb b/lib/grape/dsl/settings.rb index bc62dbbd1..a5baeea2a 100644 --- a/lib/grape/dsl/settings.rb +++ b/lib/grape/dsl/settings.rb @@ -37,7 +37,7 @@ def global_setting(key, value = nil) end def route_setting(key, value = nil) - get_or_set(inheritable_setting.route, key, value) + inheritable_setting.route_setting(key, value) end def namespace_setting(key, value = nil) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 754044515..636d52b99 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -88,11 +88,7 @@ def initialize(new_settings, http_methods:, path:, api:, app: nil, params: {}, r # the endpoint's API is mounted under another one. # @param settings [Grape::Util::InheritableSetting] def inherit_settings(settings) - parent_validations = settings.validations - inheritable_setting.route[:validations].concat(parent_validations) if parent_validations.any? - parent_declared_params = settings.declared_params - inheritable_setting.route[:declared_params].concat(parent_declared_params.flatten) if parent_declared_params.any? - + inheritable_setting.inherit_route_params(settings) endpoints&.each { |e| e.inherit_settings(settings) } end @@ -201,7 +197,7 @@ def execute end def run_validators(request:) - validators = inheritable_setting.route[:validations] + validators = inheritable_setting.route_validations return if validators.blank? validation_exceptions = nil @@ -288,7 +284,7 @@ def to_routes prefix = inheritable_setting.root_prefix requirements = prepare_routes_requirements(config.requirements) anchor = config.anchor - settings = inheritable_setting.route.except(:declared_params, :validations) + settings = inheritable_setting.route_settings config.http_methods.flat_map do |method| config.path.map do |path| diff --git a/lib/grape/util/inheritable_setting.rb b/lib/grape/util/inheritable_setting.rb index 342e24d4b..cf0435a7f 100644 --- a/lib/grape/util/inheritable_setting.rb +++ b/lib/grape/util/inheritable_setting.rb @@ -98,8 +98,8 @@ def point_in_time_copy # and request-serving defaults are applied. def point_in_time_copy_for_endpoint copy = point_in_time_copy - copy.route[:declared_params] = copy.declared_params.flatten - copy.route[:validations] = copy.validations.dup + copy.route_declared_params = copy.declared_params.flatten + copy.route_validations = copy.validations.dup copy.default_error_status ||= 500 copy end @@ -110,6 +110,77 @@ def route_end @route = {} end + # Validator instances and declared-params entries for the route currently + # being built. Unlike the same-named namespace stacks (#validations / + # #declared_params), these are flat per-route snapshots: seeded when an + # endpoint copy is forked (see #point_in_time_copy_for_endpoint), topped + # up from mounting parents (see Endpoint#inherit_settings), and read back + # by run_validators / #declared. + def route_validations + @route[:validations] + end + + def route_validations=(validations) + @route[:validations] = validations + end + + def route_declared_params + @route[:declared_params] + end + + def route_declared_params=(declared_params) + @route[:declared_params] = declared_params + end + + # Path => renamed-name map recorded by +as:+ (see ParamsScope), consumed + # by #declared. Record entries with #add_route_renamed_param; an empty + # Hash when nothing was renamed. + def route_renamed_params + @route[:renamed_params] || {} + end + + def add_route_renamed_param(path, new_name) + (@route[:renamed_params] ||= {})[path] = new_name + end + + # Endpoint description recorded by +desc+ (see DSL::Desc), consumed by + # +route+. An empty Hash when +desc+ was never called. + def route_description + @route[:description] || {} + end + + def route_description=(description) + @route[:description] = description + end + + # The route-scope settings handed to each Grape::Router::Route: every + # +route_setting+ registration plus the description, minus the internal + # param snapshots (#route_validations / #route_declared_params). + def route_settings + route.except(:declared_params, :validations) + end + + # Read (when +value+ is nil) or write an arbitrary route-scoped setting. + # This is the open store behind the +route_setting+ DSL; the known keys + # have the dedicated accessors above. + def route_setting(key, value = nil) + return @route[key] if value.nil? + + @route[key] = value + end + + # Fold a mounting parent scope's accumulated validations and declared + # params into this endpoint copy's per-route snapshots (see + # Endpoint#inherit_settings). Both are appended, so the parent's entries + # follow the ones already seeded from the surrounding scopes. + def inherit_route_params(parent) + parent_validations = parent.validations + route_validations.concat(parent_validations) if parent_validations.any? + + parent_declared_params = parent.declared_params + route_declared_params.concat(parent_declared_params.flatten) if parent_declared_params.any? + end + # Return a serializable hash of our values. def to_hash { diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index 02438c080..dcfd44095 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -211,10 +211,7 @@ def build_full_path # @param new_name [String, Symbol] the new name of the parameter (the # renamed name, with the +as: ...+ semantic) def push_renamed_param(path, new_name) - api_route_setting = @api.inheritable_setting.route - base = api_route_setting[:renamed_params] || {} - base[Array(path).map(&:to_s)] = new_name.to_s - api_route_setting[:renamed_params] = base + @api.inheritable_setting.add_route_renamed_param(Array(path).map(&:to_s), new_name.to_s) end def require_required_and_optional_fields(context, using:, except: nil) diff --git a/spec/grape/util/inheritable_setting_spec.rb b/spec/grape/util/inheritable_setting_spec.rb index 0b19b6ad1..1ec27ed9e 100644 --- a/spec/grape/util/inheritable_setting_spec.rb +++ b/spec/grape/util/inheritable_setting_spec.rb @@ -140,6 +140,50 @@ end end + describe 'route-scope accessors' do + it 'reads and writes the per-route validation snapshot' do + subject.route_validations = [:validator] + expect(subject.route_validations).to eq [:validator] + end + + it 'reads and writes the per-route declared-params snapshot' do + subject.route_declared_params = [:id] + expect(subject.route_declared_params).to eq [:id] + end + + it 'defaults renamed params to an empty hash and accumulates additions' do + expect(subject.route_renamed_params).to eq({}) + + subject.add_route_renamed_param(['a'], 'b') + subject.add_route_renamed_param(['c'], 'd') + expect(subject.route_renamed_params).to eq({ ['a'] => 'b', ['c'] => 'd' }) + end + + it 'defaults the description to an empty hash and round-trips writes' do + expect(subject.route_description).to eq({}) + + subject.route_description = { description: 'x' } + expect(subject.route_description).to eq({ description: 'x' }) + end + + it 'exposes route settings without the internal param snapshots' do + subject.route_end + subject.route_validations = [:validator] + subject.route_declared_params = [:id] + subject.route_description = { description: 'x' } + subject.route[:custom] = :value + + expect(subject.route_settings).to eq(description: { description: 'x' }, custom: :value) + end + + it 'reads and writes arbitrary route settings' do + expect(subject.route_setting(:custom)).to be_nil + + subject.route_setting(:custom, :value) + expect(subject.route_setting(:custom)).to eq :value + end + end + describe '#inherit_from' do it 'notifies clones' do new_settings = subject.point_in_time_copy