diff --git a/.rubocop.yml b/.rubocop.yml index 114b2cab7..cf3c032e4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -52,7 +52,7 @@ Metrics/BlockLength: - spec/**/*_spec.rb Metrics/ClassLength: - Max: 320 + Max: 340 Metrics/CyclomaticComplexity: Max: 15 diff --git a/CHANGELOG.md b/CHANGELOG.md index 8677c4562..53cf6bbc5 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). +* [#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 but is deprecated via `Grape.deprecator`; `Router::Pattern::Path` reads a dedicated `path_settings` snapshot, and `mount_paths` exposes the full mount-path stack - [@ericproulx](https://github.com/ericproulx). * [#2797](https://github.com/ruby-grape/grape/pull/2797): Internalize params state (validations, declared params, param documentation, named params) behind `Grape::Util::InheritableSetting` accessors, and remove the unused `api_class` / `point_in_time_copies` readers - [@ericproulx](https://github.com/ericproulx). * [#2798](https://github.com/ruby-grape/grape/pull/2798): Internalize callback filters behind `Grape::Util::InheritableSetting` accessors (`callbacks` / `add_callback`) - [@ericproulx](https://github.com/ericproulx). * [#2799](https://github.com/ruby-grape/grape/pull/2799): Internalize the remaining rescue configuration (`rescue_options`, meta-selector handlers and flags) behind `Grape::Util::InheritableSetting` accessors - [@ericproulx](https://github.com/ericproulx). diff --git a/UPGRADING.md b/UPGRADING.md index 926d3e657..f9d1cddd4 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -141,6 +141,17 @@ The flags flipped by `do_not_route_head!`, `do_not_route_options!`, `do_not_docu The params-builder strategy written by `build_with` (both the API-level and the params-block DSL) and the authentication configuration written by the `auth` DSL are now recorded and read through dedicated accessors on `Grape::Util::InheritableSetting` (`build_params_with` / `build_params_with=`, `auth` / `auth=`) instead of raw `namespace_inheritable` keys, completing the per-key `namespace_inheritable` cleanup. The keys' storage is unchanged for now, so `namespace_inheritable[:build_params_with]` and `namespace_inheritable[:auth]` still return the same values, but they should be considered internal. +#### `InheritableSetting`'s raw stores are internal, except `namespace_stackable` + +The per-key encapsulation documented in the sections above is now enforced where the ecosystem allows: `Grape::Util::InheritableSetting#namespace_inheritable` is protected and `#namespace_stackable_with_hash` is private — every `namespace_inheritable` key is reachable only through its dedicated accessor. + +`#namespace_stackable` deliberately **remains public but is deprecated** — reading it warns through `Grape.deprecator` (Grape itself reads the underlying store directly and never warns): grape-swagger reads it directly (`swagger_documentation_adder.rb` reads `[:namespace]` and `[:mount_path]`, `token_owner_resolver.rb` reads `[:helpers]`, and `request_param_parsers/route.rb` walks the `StackableValues#inherited_values` chain to collect per-scope params). Migrate to the dedicated accessors (`namespaces`, `mount_paths`, `helpers`, …) for everything they cover; the reader will be removed or narrowed in a future major once grape-swagger has migrated. The `route`, `namespace` and `global` scratch stores (backing `route_setting` / `namespace_setting` / `global_setting`) also remain public and unchanged. + +Two supporting changes: + +* `Router::Pattern::Path` no longer receives a merged dump of both stores; `Endpoint#to_routes` passes the new `InheritableSetting#path_settings` snapshot — a `Grape::Util::InheritableSetting::PathSettings` value object (`Data`) — which carries exactly the six values `Path` reads (full mount-path stack, root prefix, format, raw content-types stack, version, version options) with always-present keys. `Path`'s `key?`-presence guards became nil-tolerant truthiness checks — equivalent under the snapshot. `Endpoint#prepare_default_path_settings` is removed. +* `InheritableSetting#mount_paths` is added, exposing the full mount-path stack (one entry per mount level, outermost first), complementing `#mount_path`, which returns only the outermost entry. + ### Upgrading to >= 3.3 #### Minimum required Ruby is now 3.3 diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index d6388910a..754044515 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -282,7 +282,7 @@ def compile! def to_routes route_options = config.route_options params = config.params - path_settings = prepare_default_path_settings + path_settings = inheritable_setting.path_settings forward_match = bare_rack_app? version = prepare_version(inheritable_setting.version) prefix = inheritable_setting.root_prefix @@ -313,12 +313,6 @@ def bare_rack_app? config.app && !config.app.is_a?(Grape::Mountable) end - def prepare_default_path_settings - namespace_stackable_hash = inheritable_setting.namespace_stackable.to_hash - namespace_inheritable_hash = inheritable_setting.namespace_inheritable.to_hash - namespace_stackable_hash.merge!(namespace_inheritable_hash) - end - def prepare_routes_requirements(route_options_requirements) namespace_requirements = inheritable_setting.namespace_requirements namespace_requirements << route_options_requirements if route_options_requirements.present? diff --git a/lib/grape/router/pattern/path.rb b/lib/grape/router/pattern/path.rb index ff2e97083..e34ef2f7c 100644 --- a/lib/grape/router/pattern/path.rb +++ b/lib/grape/router/pattern/path.rb @@ -4,10 +4,11 @@ module Grape class Router class Pattern # Assembles the path template a {Pattern} compiles into a matcher. It turns - # a raw path plus the API's inheritable settings (mount path, root prefix, - # path-versioning and format) into an +origin+ (the route prefix) and a - # +suffix+ (the format segment). {Pattern.build} is the entry point that - # wires this into pattern construction. + # a raw path plus the path-settings snapshot taken by + # {Grape::Util::InheritableSetting#path_settings} (mount paths, root + # prefix, path-versioning and format) into an +origin+ (the route prefix) + # and a +suffix+ (the format segment). {Pattern.build} is the entry point + # that wires this into pattern construction. class Path DEFAULT_FORMAT_SEGMENT = '(/.:format)' NO_VERSIONING_WITH_VALID_PATH_FORMAT_SEGMENT = '(.:format)' @@ -27,7 +28,7 @@ def to_s private def build_suffix(raw_path, raw_namespace, settings) - return "(.#{settings[:format]})" if uses_specific_format?(settings) + return "(.#{settings.format})" if uses_specific_format?(settings) return NO_VERSIONING_WITH_VALID_PATH_FORMAT_SEGMENT if !uses_path_versioning?(settings) || valid_part?(raw_namespace) || valid_part?(raw_path) DEFAULT_FORMAT_SEGMENT @@ -35,8 +36,8 @@ def build_suffix(raw_path, raw_namespace, settings) def build_parts(raw_path, raw_namespace, settings) parts = [] - add_part(parts, settings[:mount_path]) - add_part(parts, settings[:root_prefix]) + add_part(parts, settings.mount_path) + add_part(parts, settings.root_prefix) parts << VERSION_SEGMENT if uses_path_versioning?(settings) add_part(parts, raw_namespace) add_part(parts, raw_path) @@ -52,15 +53,11 @@ def not_slash?(value) end def uses_specific_format?(settings) - return false unless settings.key?(:format) && settings.key?(:content_types) - - settings[:format] && Array(settings[:content_types]).size == 1 + settings.format && Array(settings.content_types).size == 1 end def uses_path_versioning?(settings) - return false unless settings.key?(:version) && settings[:version_options] - - settings[:version] && settings[:version_options].using == :path + settings.version && settings.version_options&.using == :path end def valid_part?(part) diff --git a/lib/grape/util/inheritable_setting.rb b/lib/grape/util/inheritable_setting.rb index ba1bc7b05..74ed6d723 100644 --- a/lib/grape/util/inheritable_setting.rb +++ b/lib/grape/util/inheritable_setting.rb @@ -2,8 +2,18 @@ module Grape module Util - # A branchable, inheritable settings object which can store both stackable - # and inheritable values (see InheritableValues and StackableValues). + # The per-scope settings registry behind the Grape DSL. The semantic + # accessors below — grouped by concern — are the supported API: +add_*+ + # writers stack one registration per call (read back outermost scope + # first), plain +=+ writers are nearest-wins scalar overrides, and + # +!+/+?+ pairs are scope flags. Deep-merged readers return nil when + # nothing is registered; plain stack readers return a frozen empty + # Array. The backing stores (InheritableValues, StackableValues and the + # per-scope Hashes) and their keys are internal. + # + # Settings instances form a chain: a scope inherits its parent's values + # (see #inherit_from), and endpoints snapshot the chain with + # #point_in_time_copy_for_endpoint. class InheritableSetting # Maps the callbacks DSL method names to their pluralized # namespace-stackable storage keys (see #callbacks / #add_callback). @@ -15,7 +25,16 @@ class InheritableSetting finally: :finallies }.freeze - attr_reader :route, :namespace, :namespace_inheritable, :namespace_stackable, :parent + attr_reader :route, :namespace, :parent + + # @deprecated Kept public for ecosystem compatibility — grape-swagger + # reads it directly (and walks its inherited_values chain) — but every + # semantic key has a dedicated accessor below; use those instead. + # Internal code reads the ivar and never trips this warning. + def namespace_stackable + Grape.deprecator.warn('Grape::Util::InheritableSetting#namespace_stackable is deprecated; use the dedicated semantic accessors (#namespaces, #mount_paths, #helpers, ...) instead.') + @namespace_stackable + end # Retrieve global settings. def self.global @@ -56,8 +75,8 @@ def inherit_from(parent) @parent = parent - namespace_inheritable.inherited_values = parent.namespace_inheritable - namespace_stackable.inherited_values = parent.namespace_stackable + @namespace_inheritable.inherited_values = parent.namespace_inheritable + @namespace_stackable.inherited_values = parent.raw_namespace_stackable @route = parent.route.merge(route) @point_in_time_copies&.each { |cloned_one| cloned_one.inherit_from parent } @@ -100,8 +119,8 @@ def to_hash global: global.clone, route: route.clone, namespace: namespace.to_hash, - namespace_inheritable: namespace_inheritable.to_hash, - namespace_stackable: namespace_stackable.to_hash, + namespace_inheritable: @namespace_inheritable.to_hash, + namespace_stackable: @namespace_stackable.to_hash, rescue_handlers:, base_only_rescue_handlers: } @@ -112,33 +131,26 @@ def ==(other) end alias eql? == - def namespace_stackable_with_hash(key) - data = namespace_stackable[key] - return if data.blank? - - data.each_with_object({}) { |value, result| result.deep_merge!(value) } - end - # Validator instances registered by +params+ and +contract+ blocks, # outermost scope first. Record them with #add_validation; the backing # store is an internal detail. def validations - namespace_stackable[:validations] + @namespace_stackable[:validations] end def add_validation(validator) - namespace_stackable[:validations] = validator + @namespace_stackable[:validations] = validator end # Declared-params entries registered by +params+ blocks, one Array per # scope, outermost scope first. Record them with #add_declared_params; # the backing store is an internal detail. def declared_params - namespace_stackable[:declared_params] + @namespace_stackable[:declared_params] end def add_declared_params(params) - namespace_stackable[:declared_params] = params + @namespace_stackable[:declared_params] = params end # Param documentation recorded by +params+ blocks (see @@ -151,14 +163,14 @@ def params_documentation end def add_params_documentation(documented_attrs) - namespace_stackable[:params] = documented_attrs + @namespace_stackable[:params] = documented_attrs end # Drops this scope's own validations, declared params and params # documentation once an endpoint has consumed them (see # +reset_validations!+ in DSL::Validations). Inherited entries are kept. def reset_validations! - namespace_stackable.delete(:declared_params, :params, :validations) + @namespace_stackable.delete(:declared_params, :params, :validations) end # Reusable +params :name do ... end+ blocks defined in helpers, as one @@ -170,7 +182,7 @@ def named_params end def add_named_params(named_params) - namespace_stackable[:named_params] = named_params + @namespace_stackable[:named_params] = named_params end # Filter blocks registered by the callbacks DSL (see DSL::Callbacks), @@ -179,11 +191,11 @@ def add_named_params(named_params) # +:finally+), outermost scope first. Record them with #add_callback; # the backing store is an internal detail. def callbacks - CALLBACK_STORE_KEYS.transform_values { |store_key| namespace_stackable[store_key] } + CALLBACK_STORE_KEYS.transform_values { |store_key| @namespace_stackable[store_key] } end def add_callback(callback_name, block) - namespace_stackable[CALLBACK_STORE_KEYS.fetch(callback_name)] = block + @namespace_stackable[CALLBACK_STORE_KEYS.fetch(callback_name)] = block end # Response-shaping options recorded by +rescue_from+ (see @@ -192,11 +204,11 @@ def add_callback(callback_name, block) # +rescue_from+ was never called. Record them with #add_rescue_options; # the backing store is an internal detail. def rescue_options - namespace_stackable[:rescue_options].last + @namespace_stackable[:rescue_options].last end def add_rescue_options(options) - namespace_stackable[:rescue_options] = options + @namespace_stackable[:rescue_options] = options end # Meta-selector registrations from +rescue_from :all+, @@ -206,38 +218,57 @@ def add_rescue_options(options) # through #rescue_all? / #rescue_grape_exceptions?; the backing store # is an internal detail. def add_all_rescue_handler(handler) - namespace_inheritable[:rescue_all] = true - namespace_inheritable[:all_rescue_handler] = handler + @namespace_inheritable[:rescue_all] = true + @namespace_inheritable[:all_rescue_handler] = handler end def add_grape_exceptions_rescue_handler(handler) - namespace_inheritable[:rescue_all] = true - namespace_inheritable[:rescue_grape_exceptions] = true - namespace_inheritable[:grape_exceptions_rescue_handler] = handler + @namespace_inheritable[:rescue_all] = true + @namespace_inheritable[:rescue_grape_exceptions] = true + @namespace_inheritable[:grape_exceptions_rescue_handler] = handler end def add_internal_grape_exceptions_rescue_handler(handler) - namespace_inheritable[:internal_grape_exceptions_rescue_handler] = handler + @namespace_inheritable[:internal_grape_exceptions_rescue_handler] = handler end def rescue_all? - namespace_inheritable[:rescue_all] == true + @namespace_inheritable[:rescue_all] == true end def rescue_grape_exceptions? - namespace_inheritable[:rescue_grape_exceptions] == true + @namespace_inheritable[:rescue_grape_exceptions] == true end def all_rescue_handler - namespace_inheritable[:all_rescue_handler] + @namespace_inheritable[:all_rescue_handler] end def grape_exceptions_rescue_handler - namespace_inheritable[:grape_exceptions_rescue_handler] + @namespace_inheritable[:grape_exceptions_rescue_handler] end def internal_grape_exceptions_rescue_handler - namespace_inheritable[:internal_grape_exceptions_rescue_handler] + @namespace_inheritable[:internal_grape_exceptions_rescue_handler] + 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. + def rescue_handlers + merged_rescue_handlers(:rescue_handlers) + end + + def base_only_rescue_handlers + merged_rescue_handlers(:base_only_rescue_handlers) + end + + # An exception class registered twice in the same scope keeps its first + # handler, and keeps the position it was first registered at. + def add_rescue_handlers(mapping, subclasses:) + @rescue_handler_maps ||= {} + own = (@rescue_handler_maps[subclasses ? :rescue_handlers : :base_only_rescue_handlers] ||= {}) + own.merge!(mapping) { |_klass, registered, _new| registered } end # Content negotiation registries recorded by the request/response DSL @@ -252,7 +283,7 @@ def content_types end def add_content_type(format, content_type) - namespace_stackable[:content_types] = { format => content_type } + @namespace_stackable[:content_types] = { format => content_type } end def formatters @@ -260,7 +291,7 @@ def formatters end def add_formatter(content_type, formatter) - namespace_stackable[:formatters] = { content_type => formatter } + @namespace_stackable[:formatters] = { content_type => formatter } end def parsers @@ -268,7 +299,7 @@ def parsers end def add_parser(content_type, parser) - namespace_stackable[:parsers] = { content_type => parser } + @namespace_stackable[:parsers] = { content_type => parser } end def error_formatters @@ -276,7 +307,7 @@ def error_formatters end def add_error_formatter(format, formatter) - namespace_stackable[:error_formatters] = { format => formatter } + @namespace_stackable[:error_formatters] = { format => formatter } end # Model-class => entity-class registrations from +represent+ (see @@ -289,7 +320,7 @@ def representations end def add_representation(model_class, entity_class) - namespace_stackable[:representations] = { model_class => entity_class } + @namespace_stackable[:representations] = { model_class => entity_class } end # Middleware specs recorded by the middleware DSL (+use+, +insert+, @@ -298,22 +329,22 @@ def add_representation(model_class, entity_class) # first. Record them with #add_middleware; the backing store is an # internal detail. def middleware - namespace_stackable[:middleware] + @namespace_stackable[:middleware] end def add_middleware(operation_with_arguments) - namespace_stackable[:middleware] = operation_with_arguments + @namespace_stackable[:middleware] = operation_with_arguments end # Helper modules registered by +helpers+ blocks and modules (see # DSL::Helpers), outermost scope first. Record them with #add_helper; # the backing store is an internal detail. def helpers - namespace_stackable[:helpers] + @namespace_stackable[:helpers] end def add_helper(mod) - namespace_stackable[:helpers] = mod + @namespace_stackable[:helpers] = mod end # Grape::Namespace objects registered by the +namespace+ DSL and its @@ -322,11 +353,11 @@ def add_helper(mod) # store. Record them with #add_namespace; the backing store is an # internal detail. def namespaces - namespace_stackable[:namespace] + @namespace_stackable[:namespace] end def add_namespace(namespace) - namespace_stackable[:namespace] = namespace + @namespace_stackable[:namespace] = namespace end # The normalized path prefix formed by joining every registered @@ -346,11 +377,18 @@ def namespace_requirements # outermost mount path — nil when the API is not mounted; the backing # store is an internal detail. def mount_path - namespace_stackable[:mount_path].first + @namespace_stackable[:mount_path].first end def add_mount_path(mount_path) - namespace_stackable[:mount_path] = mount_path + @namespace_stackable[:mount_path] = mount_path + end + + # The full mount-path stack — one entry per mount level, outermost + # first; what Router::Pattern::Path joins into a route's origin (see + # #path_settings). + def mount_paths + @namespace_stackable[:mount_path] end # Dry::Schema key maps registered by +contract+ blocks (see @@ -359,11 +397,11 @@ def add_mount_path(mount_path) # declared keys. Record them with #add_contract_key_map; the backing # store is an internal detail. def contract_key_maps - namespace_stackable[:contract_key_map] + @namespace_stackable[:contract_key_map] end def add_contract_key_map(key_map) - namespace_stackable[:contract_key_map] = key_map + @namespace_stackable[:contract_key_map] = key_map end # Serialization and error-response defaults recorded by the @@ -377,35 +415,35 @@ def add_contract_key_map(key_map) # when never set (Endpoint applies the request-serving fallbacks); the # backing store is an internal detail. def format - namespace_inheritable[:format] + @namespace_inheritable[:format] end def format=(format) - namespace_inheritable[:format] = format + @namespace_inheritable[:format] = format end def default_format - namespace_inheritable[:default_format] + @namespace_inheritable[:default_format] end def default_format=(default_format) - namespace_inheritable[:default_format] = default_format + @namespace_inheritable[:default_format] = default_format end def default_error_formatter - namespace_inheritable[:default_error_formatter] + @namespace_inheritable[:default_error_formatter] end def default_error_formatter=(formatter) - namespace_inheritable[:default_error_formatter] = formatter + @namespace_inheritable[:default_error_formatter] = formatter end def default_error_status - namespace_inheritable[:default_error_status] + @namespace_inheritable[:default_error_status] end def default_error_status=(status) - namespace_inheritable[:default_error_status] = status + @namespace_inheritable[:default_error_status] = status end # Versioning state recorded by the routing DSL (see DSL::Routing): @@ -415,27 +453,27 @@ def default_error_status=(status) # Nearest-wins scalars with plain += writers; readers return nil when # never set; the backing store is an internal detail. def version - namespace_inheritable[:version] + @namespace_inheritable[:version] end def version=(versions) - namespace_inheritable[:version] = versions + @namespace_inheritable[:version] = versions end def version_options - namespace_inheritable[:version_options] + @namespace_inheritable[:version_options] end def version_options=(options) - namespace_inheritable[:version_options] = options + @namespace_inheritable[:version_options] = options end def root_prefix - namespace_inheritable[:root_prefix] + @namespace_inheritable[:root_prefix] end def root_prefix=(prefix) - namespace_inheritable[:root_prefix] = prefix + @namespace_inheritable[:root_prefix] = prefix end # Cascade flag assigned by the +cascade+ DSL. An explicit nil is @@ -444,15 +482,15 @@ def root_prefix=(prefix) # assigned it — Grape::API::Instance#cascade? falls back to the # version options' cascade, then to true, when it was never assigned. def cascade - namespace_inheritable[:cascade] + @namespace_inheritable[:cascade] end def cascade=(value) - namespace_inheritable[:cascade] = value + @namespace_inheritable[:cascade] = value end def cascade_defined? - namespace_inheritable.key?(:cascade) + @namespace_inheritable.key?(:cascade) end # Scope flags flipped by the routing DSL's bang methods (see @@ -461,35 +499,35 @@ def cascade_defined? # 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 + @namespace_inheritable[:do_not_route_head] = true end def do_not_route_head? - namespace_inheritable[:do_not_route_head] == true + @namespace_inheritable[:do_not_route_head] == true end def do_not_route_options! - namespace_inheritable[:do_not_route_options] = true + @namespace_inheritable[:do_not_route_options] = true end def do_not_route_options? - namespace_inheritable[:do_not_route_options] == true + @namespace_inheritable[:do_not_route_options] == true end def do_not_document! - namespace_inheritable[:do_not_document] = true + @namespace_inheritable[:do_not_document] = true end def do_not_document? - namespace_inheritable[:do_not_document] == true + @namespace_inheritable[:do_not_document] == true end def lint! - namespace_inheritable[:lint] = true + @namespace_inheritable[:lint] = true end def lint? - namespace_inheritable[:lint] == true + @namespace_inheritable[:lint] == true end # The params-builder strategy set by +build_with+ (both the @@ -498,11 +536,11 @@ def lint? # builds its Grape::Request. Nearest-wins scalar; nil when never set; # the backing store is an internal detail. def build_params_with - namespace_inheritable[:build_params_with] + @namespace_inheritable[:build_params_with] end def build_params_with=(strategy) - namespace_inheritable[:build_params_with] = strategy + @namespace_inheritable[:build_params_with] = strategy end # The authentication configuration Hash recorded by the +auth+ DSL @@ -511,34 +549,42 @@ def build_params_with=(strategy) # to warn about unauthenticated bare Rack mounts; the backing store is # an internal detail. def auth - namespace_inheritable[:auth] + @namespace_inheritable[:auth] end def auth=(auth_options) - namespace_inheritable[:auth] = auth_options + @namespace_inheritable[:auth] = auth_options + end + + # The PathSettings snapshot Router::Pattern::Path reads to assemble a + # route's origin and suffix (see Endpoint#to_routes). +mount_path+ is the full stack — one entry per + # mount level, outermost first — unlike #mount_path, which returns + # only the outermost entry; +content_types+ is the raw registration + # stack, because Path counts registrations rather than distinct + # formats. Unset members are nil. + def path_settings + PathSettings.new( + mount_path: mount_paths.presence, + root_prefix:, + format:, + content_types: @namespace_stackable[:content_types].presence, + version:, + version_options: + ) 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. - def rescue_handlers - merged_rescue_handlers(:rescue_handlers) - end + protected - def base_only_rescue_handlers - merged_rescue_handlers(:base_only_rescue_handlers) - end + # Peer access to the inheritable store for #inherit_from and + # #copy_state_from; internal code reads the ivar directly. + attr_reader :namespace_inheritable - # An exception class registered twice in the same scope keeps its first - # handler, and keeps the position it was first registered at. - def add_rescue_handlers(mapping, subclasses:) - @rescue_handler_maps ||= {} - own = (@rescue_handler_maps[subclasses ? :rescue_handlers : :base_only_rescue_handlers] ||= {}) - own.merge!(mapping) { |_klass, registered, _new| registered } + # Peer access to the stackable store for #inherit_from and + # #copy_state_from, bypassing the deprecated public reader. + def raw_namespace_stackable + @namespace_stackable end - protected - # This scope's own +rescue_from+ registrations, before inheritance: # {rescue_handlers: {klass => handler}, base_only_rescue_handlers: {...}}. attr_reader :rescue_handler_maps @@ -559,10 +605,21 @@ def merged_rescue_handlers(key) def copy_state_from(source) @namespace = source.namespace.clone @namespace_inheritable = source.namespace_inheritable.clone - @namespace_stackable = source.namespace_stackable.clone + @namespace_stackable = source.raw_namespace_stackable.clone @rescue_handler_maps = source.rescue_handler_maps&.dup @route = source.route.clone end + + private + + # Deep-merges a stackable key's registrations into one Hash, nearest + # scope winning; nil when nothing is registered. + def namespace_stackable_with_hash(key) + data = @namespace_stackable[key] + return if data.blank? + + data.each_with_object({}) { |value, result| result.deep_merge!(value) } + end end end end diff --git a/lib/grape/util/inheritable_setting/path_settings.rb b/lib/grape/util/inheritable_setting/path_settings.rb new file mode 100644 index 000000000..350776a2c --- /dev/null +++ b/lib/grape/util/inheritable_setting/path_settings.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Grape + module Util + class InheritableSetting + # Immutable snapshot of the settings Router::Pattern::Path reads to + # assemble a route's origin and suffix, built by + # InheritableSetting#path_settings. +mount_path+ carries the full + # mount-path stack (one entry per mount level, outermost first) and + # +content_types+ the raw registration stack — Path counts + # registrations, not distinct formats. Unset members are nil. + PathSettings = Data.define(:mount_path, :root_prefix, :format, :content_types, :version, :version_options) do + def initialize(mount_path: nil, root_prefix: nil, format: nil, content_types: nil, version: nil, version_options: nil) + super + end + end + end + end +end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index b2cfb1ef4..b990ca83e 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -18,13 +18,12 @@ subject.get('/x') { 'x' } subject.compile! - shared = subject.base_instance.inheritable_setting.namespace_inheritable - expect(shared).not_to receive(:delete) + shared = subject.base_instance.inheritable_setting subject.change! subject.compile! - expect(shared[:version_options]).to be_present + expect(shared.version_options).to be_present end end diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index 48e0bef77..acfe417f7 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -8,12 +8,11 @@ include Grape::DSL::InsideRoute include Grape::DSL::Settings - attr_reader :env, :request, :new_settings + attr_reader :env, :request def initialize @env = {} @header = {} - @new_settings = { namespace_inheritable: inheritable_setting.namespace_inheritable, namespace_stackable: inheritable_setting.namespace_stackable } end def header(key = nil, val = nil) diff --git a/spec/grape/dsl/settings_spec.rb b/spec/grape/dsl/settings_spec.rb index fbe881ea2..7311d466f 100644 --- a/spec/grape/dsl/settings_spec.rb +++ b/spec/grape/dsl/settings_spec.rb @@ -59,31 +59,31 @@ def reset_validations!; end end end - describe '#namespace_inheritable' do + describe 'inheritable scalar accessors' do it 'inherits values from surrounding namespace' do subject.with_namespace do - subject.inheritable_setting.namespace_inheritable[:some_thing] = :foo_bar - expect(subject.inheritable_setting.namespace_inheritable[:some_thing]).to eq :foo_bar + subject.inheritable_setting.format = :foo_bar + expect(subject.inheritable_setting.format).to eq :foo_bar subject.with_namespace do - expect(subject.inheritable_setting.namespace_inheritable[:some_thing]).to eq :foo_bar - subject.inheritable_setting.namespace_inheritable[:some_thing] = :foo_bar_2 - expect(subject.inheritable_setting.namespace_inheritable[:some_thing]).to eq :foo_bar_2 + expect(subject.inheritable_setting.format).to eq :foo_bar + subject.inheritable_setting.format = :foo_bar_2 + expect(subject.inheritable_setting.format).to eq :foo_bar_2 end - expect(subject.inheritable_setting.namespace_inheritable[:some_thing]).to eq :foo_bar + expect(subject.inheritable_setting.format).to eq :foo_bar end end end - describe '#namespace_stackable' do + describe 'stackable accessors' do it 'stacks values from surrounding namespace' do subject.with_namespace do - subject.inheritable_setting.namespace_stackable[:some_thing] = :foo_bar - expect(subject.inheritable_setting.namespace_stackable[:some_thing]).to eq [:foo_bar] + subject.inheritable_setting.add_helper(:foo_bar) + expect(subject.inheritable_setting.helpers).to eq [:foo_bar] subject.with_namespace do - subject.inheritable_setting.namespace_stackable[:some_thing] = :foo_bar_2 - expect(subject.inheritable_setting.namespace_stackable[:some_thing]).to eq %i[foo_bar foo_bar_2] + subject.inheritable_setting.add_helper(:foo_bar_2) + expect(subject.inheritable_setting.helpers).to eq %i[foo_bar foo_bar_2] end - expect(subject.inheritable_setting.namespace_stackable[:some_thing]).to eq [:foo_bar] + expect(subject.inheritable_setting.helpers).to eq [:foo_bar] end end end @@ -99,36 +99,36 @@ def reset_validations!; end obj3_copy = nil obj1.with_namespace do - obj1.inheritable_setting.namespace_stackable[:some_thing] = :obj1 - expect(obj1.inheritable_setting.namespace_stackable[:some_thing]).to eq [:obj1] + obj1.inheritable_setting.add_helper(:obj1) + expect(obj1.inheritable_setting.helpers).to eq [:obj1] obj1_copy = obj1.inheritable_setting.point_in_time_copy end - expect(obj1.inheritable_setting.namespace_stackable[:some_thing]).to eq [] - expect(obj1_copy.namespace_stackable[:some_thing]).to eq [:obj1] + expect(obj1.inheritable_setting.helpers).to eq [] + expect(obj1_copy.helpers).to eq [:obj1] obj2.with_namespace do - obj2.inheritable_setting.namespace_stackable[:some_thing] = :obj2 - expect(obj2.inheritable_setting.namespace_stackable[:some_thing]).to eq [:obj2] + obj2.inheritable_setting.add_helper(:obj2) + expect(obj2.inheritable_setting.helpers).to eq [:obj2] obj2_copy = obj2.inheritable_setting.point_in_time_copy end - expect(obj2.inheritable_setting.namespace_stackable[:some_thing]).to eq [] - expect(obj2_copy.namespace_stackable[:some_thing]).to eq [:obj2] + expect(obj2.inheritable_setting.helpers).to eq [] + expect(obj2_copy.helpers).to eq [:obj2] obj3.with_namespace do - obj3.inheritable_setting.namespace_stackable[:some_thing] = :obj3 - expect(obj3.inheritable_setting.namespace_stackable[:some_thing]).to eq [:obj3] + obj3.inheritable_setting.add_helper(:obj3) + expect(obj3.inheritable_setting.helpers).to eq [:obj3] obj3_copy = obj3.inheritable_setting.point_in_time_copy end - expect(obj3.inheritable_setting.namespace_stackable[:some_thing]).to eq [] - expect(obj3_copy.namespace_stackable[:some_thing]).to eq [:obj3] + expect(obj3.inheritable_setting.helpers).to eq [] + expect(obj3_copy.helpers).to eq [:obj3] # obj1.top_level_setting.inherit_from obj2_copy.point_in_time_copy # obj2.top_level_setting.inherit_from obj3_copy.point_in_time_copy - # expect(obj1_copy.namespace_stackable[:some_thing]).to eq %i[obj3 obj2 obj1] + # expect(obj1_copy.helpers).to eq %i[obj3 obj2 obj1] end end end diff --git a/spec/grape/router/pattern/path_spec.rb b/spec/grape/router/pattern/path_spec.rb index 5ab9e9239..91d52f6bf 100644 --- a/spec/grape/router/pattern/path_spec.rb +++ b/spec/grape/router/pattern/path_spec.rb @@ -1,22 +1,26 @@ # frozen_string_literal: true RSpec.describe Grape::Router::Pattern::Path do + def path_settings(**attrs) + Grape::Util::InheritableSetting::PathSettings.new(**attrs) + end + describe '#origin' do context 'mount_path' do it 'is not included when it is nil' do - path = described_class.new(nil, nil, mount_path: '/foo/bar') + path = described_class.new(nil, nil, path_settings(mount_path: '/foo/bar')) expect(path.origin).to eql '/foo/bar' end it 'is included when it is not nil' do - path = described_class.new(nil, nil, {}) + path = described_class.new(nil, nil, path_settings) expect(path.origin).to eql('/') end end context 'root_prefix' do it 'is not included when it is nil' do - path = described_class.new(nil, nil, {}) + path = described_class.new(nil, nil, path_settings) expect(path.origin).to eql('/') end @@ -24,8 +28,7 @@ path = described_class.new( nil, nil, - mount_path: '/foo', - root_prefix: '/hello' + path_settings(mount_path: '/foo', root_prefix: '/hello') ) expect(path.origin).to eql('/foo/hello') @@ -36,8 +39,7 @@ path = described_class.new( nil, 'namespace', - mount_path: '/foo', - root_prefix: '/hello' + path_settings(mount_path: '/foo', root_prefix: '/hello') ) expect(path.origin).to eql('/foo/hello/namespace') @@ -47,8 +49,7 @@ path = described_class.new( 'raw_path', 'namespace', - mount_path: '/foo', - root_prefix: '/hello' + path_settings(mount_path: '/foo', root_prefix: '/hello') ) expect(path.origin).to eql('/foo/hello/namespace/raw_path') @@ -58,31 +59,31 @@ describe '#suffix' do context 'when using a specific format' do it 'accepts specified format' do - path = described_class.new(nil, nil, format: 'json', content_types: 'application/json') + path = described_class.new(nil, nil, path_settings(format: 'json', content_types: 'application/json')) expect(path.suffix).to eql('(.json)') end end context 'when path versioning is used' do it "includes a '/'" do - path = described_class.new(nil, nil, version: :v1, version_options: Grape::DSL::VersionOptions.new) + path = described_class.new(nil, nil, path_settings(version: :v1, version_options: Grape::DSL::VersionOptions.new)) expect(path.suffix).to eql('(/.:format)') end end context 'when path versioning is not used' do it "does not include a '/' when the path has a namespace" do - path = described_class.new(nil, 'namespace', {}) + path = described_class.new(nil, 'namespace', path_settings) expect(path.suffix).to eql('(.:format)') end it "does not include a '/' when the path has a path" do - path = described_class.new('/path', nil, version: :v1, version_options: Grape::DSL::VersionOptions.new) + path = described_class.new('/path', nil, path_settings(version: :v1, version_options: Grape::DSL::VersionOptions.new)) expect(path.suffix).to eql('(.:format)') end it "includes a '/' otherwise" do - path = described_class.new(nil, nil, version: :v1, version_options: Grape::DSL::VersionOptions.new) + path = described_class.new(nil, nil, path_settings(version: :v1, version_options: Grape::DSL::VersionOptions.new)) expect(path.suffix).to eql('(/.:format)') end end diff --git a/spec/grape/router/pattern_spec.rb b/spec/grape/router/pattern_spec.rb index b11914fcc..87ccde017 100644 --- a/spec/grape/router/pattern_spec.rb +++ b/spec/grape/router/pattern_spec.rb @@ -15,7 +15,7 @@ describe '.build' do subject(:pattern) do - described_class.build(path: '/foo', namespace: 'ns', settings: { root_prefix: '/api' }, anchor: true, params: {}, version: nil, requirements: {}) + described_class.build(path: '/foo', namespace: 'ns', settings: Grape::Util::InheritableSetting::PathSettings.new(root_prefix: '/api'), anchor: true, params: {}, version: nil, requirements: {}) end it 'assembles origin/suffix from the path, namespace and settings via Path' do diff --git a/spec/grape/util/inheritable_setting_spec.rb b/spec/grape/util/inheritable_setting_spec.rb index 02f0f041b..e5496b446 100644 --- a/spec/grape/util/inheritable_setting_spec.rb +++ b/spec/grape/util/inheritable_setting_spec.rb @@ -10,8 +10,8 @@ described_class.new.tap do |settings| settings.global[:global_thing] = :global_foo_bar settings.namespace[:namespace_thing] = :namespace_foo_bar - settings.namespace_inheritable[:namespace_inheritable_thing] = :namespace_inheritable_foo_bar - settings.namespace_stackable[:namespace_stackable_thing] = :namespace_stackable_foo_bar + settings.root_prefix = :namespace_inheritable_foo_bar + settings.add_helper(:namespace_stackable_foo_bar) settings.route[:route_thing] = :route_foo_bar end end @@ -19,8 +19,8 @@ let(:other_parent) do described_class.new.tap do |settings| settings.namespace[:namespace_thing] = :namespace_foo_bar_other - settings.namespace_inheritable[:namespace_inheritable_thing] = :namespace_inheritable_foo_bar_other - settings.namespace_stackable[:namespace_stackable_thing] = :namespace_stackable_foo_bar_other + settings.root_prefix = :namespace_inheritable_foo_bar_other + settings.add_helper(:namespace_stackable_foo_bar_other) settings.route[:route_thing] = :route_foo_bar_other end end @@ -68,39 +68,39 @@ describe '#namespace_inheritable' do it 'works with inheritable values' do - expect(subject.namespace_inheritable[:namespace_inheritable_thing]).to eq :namespace_inheritable_foo_bar + expect(subject.root_prefix).to eq :namespace_inheritable_foo_bar end it 'handles different parents' do - expect(subject.namespace_inheritable[:namespace_inheritable_thing]).to eq :namespace_inheritable_foo_bar + expect(subject.root_prefix).to eq :namespace_inheritable_foo_bar subject.inherit_from other_parent - expect(subject.namespace_inheritable[:namespace_inheritable_thing]).to eq :namespace_inheritable_foo_bar_other + expect(subject.root_prefix).to eq :namespace_inheritable_foo_bar_other subject.inherit_from parent - expect(subject.namespace_inheritable[:namespace_inheritable_thing]).to eq :namespace_inheritable_foo_bar + expect(subject.root_prefix).to eq :namespace_inheritable_foo_bar subject.inherit_from other_parent - subject.namespace_inheritable[:namespace_inheritable_thing] = :my_thing + subject.root_prefix = :my_thing - expect(subject.namespace_inheritable[:namespace_inheritable_thing]).to eq :my_thing + expect(subject.root_prefix).to eq :my_thing subject.inherit_from parent - expect(subject.namespace_inheritable[:namespace_inheritable_thing]).to eq :my_thing + expect(subject.root_prefix).to eq :my_thing end end describe '#namespace_stackable' do it 'works with stackable values' do - expect(subject.namespace_stackable[:namespace_stackable_thing]).to eq [:namespace_stackable_foo_bar] + expect(subject.helpers).to eq [:namespace_stackable_foo_bar] subject.inherit_from other_parent - expect(subject.namespace_stackable[:namespace_stackable_thing]).to eq [:namespace_stackable_foo_bar_other] + expect(subject.helpers).to eq [:namespace_stackable_foo_bar_other] end end @@ -164,24 +164,24 @@ end it 'decouples namespace inheritable values' do - expect(cloned_obj.namespace_inheritable[:namespace_inheritable_thing]).to eq :namespace_inheritable_foo_bar + expect(cloned_obj.root_prefix).to eq :namespace_inheritable_foo_bar - subject.namespace_inheritable[:namespace_inheritable_thing] = :my_thing - expect(subject.namespace_inheritable[:namespace_inheritable_thing]).to eq :my_thing + subject.root_prefix = :my_thing + expect(subject.root_prefix).to eq :my_thing - expect(cloned_obj.namespace_inheritable[:namespace_inheritable_thing]).to eq :namespace_inheritable_foo_bar + expect(cloned_obj.root_prefix).to eq :namespace_inheritable_foo_bar - cloned_obj.namespace_inheritable[:namespace_inheritable_thing] = :my_cloned_thing - expect(cloned_obj.namespace_inheritable[:namespace_inheritable_thing]).to eq :my_cloned_thing - expect(subject.namespace_inheritable[:namespace_inheritable_thing]).to eq :my_thing + cloned_obj.root_prefix = :my_cloned_thing + expect(cloned_obj.root_prefix).to eq :my_cloned_thing + expect(subject.root_prefix).to eq :my_thing end it 'decouples namespace stackable values' do - expect(cloned_obj.namespace_stackable[:namespace_stackable_thing]).to eq [:namespace_stackable_foo_bar] + expect(cloned_obj.helpers).to eq [:namespace_stackable_foo_bar] - subject.namespace_stackable[:namespace_stackable_thing] = :other_thing - expect(subject.namespace_stackable[:namespace_stackable_thing]).to eq %i[namespace_stackable_foo_bar other_thing] - expect(cloned_obj.namespace_stackable[:namespace_stackable_thing]).to eq [:namespace_stackable_foo_bar] + subject.add_helper(:other_thing) + expect(subject.helpers).to eq %i[namespace_stackable_foo_bar other_thing] + expect(cloned_obj.helpers).to eq [:namespace_stackable_foo_bar] end it 'decouples route values' do @@ -196,21 +196,29 @@ end end + describe '#namespace_stackable' do + it 'is deprecated in favor of the semantic accessors' do + expect { subject.namespace_stackable }.to raise_error( + ActiveSupport::DeprecationException, /namespace_stackable is deprecated/ + ) + end + end + describe '#to_hash' do it 'return all settings as a hash' do subject.global[:global_thing] = :global_foo_bar subject.namespace[:namespace_thing] = :namespace_foo_bar - subject.namespace_inheritable[:namespace_inheritable_thing] = :namespace_inheritable_foo_bar - subject.namespace_stackable[:namespace_stackable_thing] = [:namespace_stackable_foo_bar] + subject.root_prefix = :namespace_inheritable_foo_bar + subject.add_helper([:namespace_stackable_foo_bar]) subject.add_rescue_handlers({ StandardError => :handler }, subclasses: true) subject.route[:route_thing] = :route_foo_bar expect(subject.to_hash).to match( global: { global_thing: :global_foo_bar }, namespace: { namespace_thing: :namespace_foo_bar }, namespace_inheritable: { - namespace_inheritable_thing: :namespace_inheritable_foo_bar + root_prefix: :namespace_inheritable_foo_bar }, - namespace_stackable: { namespace_stackable_thing: [:namespace_stackable_foo_bar, [:namespace_stackable_foo_bar]] }, + namespace_stackable: { helpers: [:namespace_stackable_foo_bar, [:namespace_stackable_foo_bar]] }, rescue_handlers: { StandardError => :handler }, base_only_rescue_handlers: nil, route: { route_thing: :route_foo_bar }