diff --git a/CHANGELOG.md b/CHANGELOG.md index ea1fe04c1..59160a53c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ * [#2786](https://github.com/ruby-grape/grape/pull/2786): Make route `requirements` and `anchor` explicit keyword arguments and first-class endpoint inputs instead of opaque `route_options` keys - [@ericproulx](https://github.com/ericproulx). * [#2788](https://github.com/ruby-grape/grape/pull/2788): Compare the base API instead of `to_s` when refreshing a mounted app - [@ericproulx](https://github.com/ericproulx). * [#2796](https://github.com/ruby-grape/grape/pull/2796): Remove `Grape::Util::ReverseStackableValues`, storing rescue handlers in plain per-scope hashes merged over `InheritableSetting#parent` - [@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). * Your contribution here. #### Fixes diff --git a/UPGRADING.md b/UPGRADING.md index 9bd07d5df..2b21d69d0 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -82,6 +82,10 @@ A route's declared params were previously carried inside the `route_options` bag Like `params` above, a route's `requirements` and `anchor` were previously carried inside the `route_options` bag. They are now composed into their own endpoint inputs (`Grape::Endpoint::Options` gains `:requirements` and `:anchor` members, and `Grape::Endpoint.new` gains `requirements:` and `anchor:` keywords) and exposed only through the `route.requirements` and `route.anchor` readers. `route.options[:requirements]` and `route.options[:anchor]` now return `nil` — including for a mount's `anchor: false`. Nothing in Grape or grape-swagger read them that way, so this only affects code that reached into the options Hash for these keys directly. +#### Routing scope flags are recorded through `InheritableSetting` accessors + +The flags flipped by `do_not_route_head!`, `do_not_route_options!`, `do_not_document!` and `lint!` are now recorded and read through dedicated accessors on `Grape::Util::InheritableSetting` (`!` writers, `?` readers) instead of raw `namespace_inheritable` keys. The `?` readers return `false` (rather than `nil`) when never set; every consumer only used them in boolean context, so behavior is unchanged. The keys' storage is unchanged for now, so `namespace_inheritable[:do_not_route_head]` and friends still return the same values, but they should be considered internal. + ### Upgrading to >= 3.3 #### Minimum required Ruby is now 3.3 diff --git a/lib/grape/api/instance.rb b/lib/grape/api/instance.rb index c9e39a3ff..458023541 100644 --- a/lib/grape/api/instance.rb +++ b/lib/grape/api/instance.rb @@ -154,18 +154,11 @@ def add_head_not_allowed_methods_and_options_methods # contain already versioning information when using path versioning. all_routes = self.class.endpoints.flat_map(&:routes) - # Read the settings the helper routes need from a *copy* with the - # root-prefix/versioning keys stripped, so adding these routes won't - # prepend versioning information again. This used to delete those keys - # from the shared class-level settings and restore them in an ensure; - # a request served concurrently on another instance during a runtime - # recompile could observe the missing keys (e.g. via #cascade?). A - # local copy keeps that mutation off the shared object. - namespace_inheritable = self.class.inheritable_setting.namespace_inheritable.to_hash.except(*ROOT_PREFIX_VERSIONING_KEYS) - collect_route_config_per_pattern(all_routes, namespace_inheritable) + collect_route_config_per_pattern(all_routes) end - def collect_route_config_per_pattern(all_routes, namespace_inheritable) + def collect_route_config_per_pattern(all_routes) + setting = self.class.inheritable_setting routes_by_regexp = all_routes.group_by(&:pattern_regexp) # Build the configuration based on the first endpoint and the collection of methods supported. @@ -174,18 +167,15 @@ def collect_route_config_per_pattern(all_routes, namespace_inheritable) last_route = routes.last # Most of the configuration is taken from the last endpoint allowed_methods = routes.map(&:request_method) - allowed_methods |= [Rack::HEAD] if !namespace_inheritable[:do_not_route_head] && allowed_methods.include?(Rack::GET) + allowed_methods |= [Rack::HEAD] if !setting.do_not_route_head? && allowed_methods.include?(Rack::GET) - allow_header = namespace_inheritable[:do_not_route_options] ? allowed_methods : [Rack::OPTIONS] | allowed_methods - last_route.app.options_route_enabled = true unless namespace_inheritable[:do_not_route_options] || allowed_methods.include?(Rack::OPTIONS) + allow_header = setting.do_not_route_options? ? allowed_methods : [Rack::OPTIONS] | allowed_methods + last_route.app.options_route_enabled = true unless setting.do_not_route_options? || allowed_methods.include?(Rack::OPTIONS) greedy_route = Grape::Router::GreedyRoute.new(last_route.pattern, endpoint: last_route.app, allow_header:) @router.associate_routes(greedy_route) end end - - ROOT_PREFIX_VERSIONING_KEYS = %i[version version_options root_prefix].freeze - private_constant :ROOT_PREFIX_VERSIONING_KEYS end end end diff --git a/lib/grape/dsl/routing.rb b/lib/grape/dsl/routing.rb index 20bbc7d07..2e16d1e4c 100644 --- a/lib/grape/dsl/routing.rb +++ b/lib/grape/dsl/routing.rb @@ -107,20 +107,20 @@ def build_with(build_with) # Do not route HEAD requests to GET requests automatically. def do_not_route_head! - inheritable_setting.namespace_inheritable[:do_not_route_head] = true + inheritable_setting.do_not_route_head! end # Do not automatically route OPTIONS. def do_not_route_options! - inheritable_setting.namespace_inheritable[:do_not_route_options] = true + inheritable_setting.do_not_route_options! end def lint! - inheritable_setting.namespace_inheritable[:lint] = true + inheritable_setting.lint! end def do_not_document! - inheritable_setting.namespace_inheritable[:do_not_document] = true + inheritable_setting.do_not_document! end def mount(mounts, *opts) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 9502cdc2c..8e74bbd4a 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -124,7 +124,7 @@ def mount_in(router) compile! routes.each do |route| router.append(route.apply(self)) - next if inheritable_setting.namespace_inheritable[:do_not_route_head] || route.request_method != Rack::GET + next if inheritable_setting.do_not_route_head? || route.request_method != Rack::GET router.append(route.to_head.apply(self)) end @@ -422,7 +422,7 @@ def build_response_cookies end def lint? - inheritable_setting.namespace_inheritable[:lint] || Grape.config.lint + inheritable_setting.lint? || Grape.config.lint end end end diff --git a/lib/grape/util/inheritable_setting.rb b/lib/grape/util/inheritable_setting.rb index 912b1f68d..2f85d9919 100644 --- a/lib/grape/util/inheritable_setting.rb +++ b/lib/grape/util/inheritable_setting.rb @@ -106,6 +106,43 @@ def namespace_stackable_with_hash(key) data.each_with_object({}) { |value, result| result.deep_merge!(value) } end + # Scope flags flipped by the routing DSL's bang methods (see + # DSL::Routing#do_not_route_head! and friends; Validations::OneofCollector + # also flips +do_not_document!+): once set in a scope they apply to it + # and everything nested under it. Readers return false when never set; + # the backing store is an internal detail. + def do_not_route_head! + namespace_inheritable[:do_not_route_head] = true + end + + def do_not_route_head? + namespace_inheritable[:do_not_route_head] == true + end + + def do_not_route_options! + namespace_inheritable[:do_not_route_options] = true + end + + def do_not_route_options? + namespace_inheritable[:do_not_route_options] == true + end + + def do_not_document! + namespace_inheritable[:do_not_document] = true + end + + def do_not_document? + namespace_inheritable[:do_not_document] == true + end + + def lint! + namespace_inheritable[:lint] = true + end + + def lint? + namespace_inheritable[:lint] == true + end + # Rescue-handler maps registered by +rescue_from+, keyed by exception # class and merged so a nested scope's handler wins. Record them with # #add_rescue_handlers; the backing store is an internal detail. diff --git a/lib/grape/validations/oneof_collector.rb b/lib/grape/validations/oneof_collector.rb index 84fa32d08..2023f07f1 100644 --- a/lib/grape/validations/oneof_collector.rb +++ b/lib/grape/validations/oneof_collector.rb @@ -12,7 +12,7 @@ class OneofCollector def initialize @inheritable_setting = Grape::Util::InheritableSetting.new - @inheritable_setting.namespace_inheritable[:do_not_document] = true + @inheritable_setting.do_not_document! end def configuration diff --git a/lib/grape/validations/params_documentation.rb b/lib/grape/validations/params_documentation.rb index a17e80b0d..72d3b0111 100644 --- a/lib/grape/validations/params_documentation.rb +++ b/lib/grape/validations/params_documentation.rb @@ -6,7 +6,7 @@ module Validations # +ValidationsSpec+; never mutates the user's validations hash. module ParamsDocumentation def document_params(attrs, spec) - return if @api.inheritable_setting.namespace_inheritable[:do_not_document] + return if @api.inheritable_setting.do_not_document? documented_attrs = attrs.to_h do |name| [full_name(name), extract_details(spec)] diff --git a/spec/grape/dsl/routing_spec.rb b/spec/grape/dsl/routing_spec.rb index 57afe0c47..81cf482e2 100644 --- a/spec/grape/dsl/routing_spec.rb +++ b/spec/grape/dsl/routing_spec.rb @@ -58,14 +58,14 @@ class << self describe '.do_not_route_head!' do it 'sets do not route head option' do subject.do_not_route_head! - expect(subject.inheritable_setting.namespace_inheritable[:do_not_route_head]).to be(true) + expect(subject.inheritable_setting.do_not_route_head?).to be(true) end end describe '.do_not_route_options!' do it 'sets do not route options option' do subject.do_not_route_options! - expect(subject.inheritable_setting.namespace_inheritable[:do_not_route_options]).to be(true) + expect(subject.inheritable_setting.do_not_route_options?).to be(true) end end diff --git a/spec/grape/validations/params_documentation_spec.rb b/spec/grape/validations/params_documentation_spec.rb index b6e1fdcab..a0b1174a5 100644 --- a/spec/grape/validations/params_documentation_spec.rb +++ b/spec/grape/validations/params_documentation_spec.rb @@ -64,7 +64,7 @@ def spec_for(validations) end before do - api_double.inheritable_setting.namespace_inheritable[:do_not_document] = true + api_double.inheritable_setting.do_not_document! end it 'does not store any documented params' do