Internalize all InheritableSetting state behind semantic accessors (integration of #2797–#2808)#2809
Open
ericproulx wants to merge 26 commits into
Open
Internalize all InheritableSetting state behind semantic accessors (integration of #2797–#2808)#2809ericproulx wants to merge 26 commits into
ericproulx wants to merge 26 commits into
Conversation
…sors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ccessors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng accessors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…Setting accessors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tting accessors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nheritableSetting accessors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…::InheritableSetting accessors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…accessors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-integration # Conflicts: # lib/grape/util/inheritable_setting.rb
…tting-integration # Conflicts: # lib/grape/util/inheritable_setting.rb
…le-setting-integration # Conflicts: # lib/grape/endpoint.rb # lib/grape/util/inheritable_setting.rb
…etting-integration # Conflicts: # lib/grape/endpoint.rb # lib/grape/util/inheritable_setting.rb
…e-setting-integration # Conflicts: # lib/grape/util/inheritable_setting.rb
…tting-integration # Conflicts: # lib/grape/util/inheritable_setting.rb
…setting-integration # Conflicts: # lib/grape/util/inheritable_setting.rb # lib/grape/validations/contract_scope.rb
…heritableSetting accessors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ccessors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…g accessors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eSetting accessors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…etting-integration # Conflicts: # lib/grape/endpoint.rb # lib/grape/util/inheritable_setting.rb
…g-integration # Conflicts: # lib/grape/endpoint.rb # lib/grape/util/inheritable_setting.rb
…ting-integration # Conflicts: # lib/grape/util/inheritable_setting.rb
…le-setting-integration # Conflicts: # lib/grape/endpoint.rb # lib/grape/util/inheritable_setting.rb
…r catalog Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Danger ReportNo issues found. |
Contributor
Author
|
@dblock this one includes a 2797 to 2808. |
…espace_inheritable hash copy collect_route_config_per_pattern only ever read do_not_route_head and do_not_route_options from the hash, so the to_hash copy, the ROOT_PREFIX_VERSIONING_KEYS stripping and its concurrency comment were all guarding reads that no longer exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integration of the full
Grape::Util::InheritableSettingencapsulation series. Every semantic key held in thenamespace_stackableandnamespace_inheritablestores is now written and read through an intention-revealing accessor onInheritableSetting— no caller outside the class indexes into either store per key anymore. Merging this PR closes the twelve standalone PRs it integrates:Each sub-PR was developed standalone against
masterwith a full green suite; this branch merges them with conflicts resolved (details below). If you prefer to merge the PRs individually, close this one instead — it adds nothing beyond the merge resolutions and one.rubocop.ymlbump.What each piece contributes
validations/add_validation,declared_params/add_declared_params,params_documentation/add_params_documentation,named_params/add_named_params,reset_validations!,point_in_time_copy_for_endpointapi_class/point_in_time_copiesreaders; renamesroute_setting(:saved_validations)→route_setting(:validations)callbacks/add_callbackcallbacks[:before]); singular→pluralized storage mapping centralized inCALLBACK_STORE_KEYS;Endpoint#compile!reads withfetchso key drift fails at bootrescue_options/add_rescue_options,add_all_rescue_handler,add_grape_exceptions_rescue_handler,add_internal_grape_exceptions_rescue_handler,rescue_all?,rescue_grape_exceptions?, handler readersrescue_optionsabsorbs the nearest-wins.last;rescue_fromdrops itsnamespace_inheritable/argpreludecontent_types,formatters,parsers,error_formatters+add_*representations/add_representationmiddleware/add_middleware,helpers/add_helper|| []fallback in themiddlewareDSL reader (same root cause)namespaces/add_namespace,namespace_path,namespace_requirements,mount_path/add_mount_pathjoined_space_pathandfilter_map(&:requirements);mount_pathabsorbs the outermost-wins.first; fixes the wrong@paramdoc onNamespace.joined_space_pathcontract_key_maps/add_contract_key_mapformat,default_format,default_error_formatter,default_error_status(+=writers)namespace_inheritablebatch;error_middleware_optionsbecomes fully store-freeversion,version_options,root_prefix,cascade(+=writers),cascade_defined?cascadeis presence-sensitive (explicit nil ≠ never-set); bothkey?(:cascade)call sites now read throughcascade_defined?do_not_route_head,do_not_route_options,do_not_document,lint(!writers /?readers)namespace_inheritable.to_hash.except(*ROOT_PREFIX_VERSIONING_KEYS)copy inInstance— its consumer only ever read the two routing flags, so the versioning-key stripping guarded reads that no longer existbuild_params_with+authbuild_params_with/=,auth/=build_params_with's two DSL writers (API-level and params-blockbuild_with) funnel through one accessorCross-cutting design
add_*writers mean stacking registrations (stackable store), plain=writers mean nearest-wins scalar overrides (inheritable store),!/?mean flags. Readers absorb the read conventions previously spelled out at call sites (.last,.first,flattenstays caller-side where it's a caller concern, deep-merge,filter_map); writers absorb the storage shapes (single-entry hashes, pluralized keys).to_hashoutput, route pattern settings and any external readers (grape-swagger) see identical values. The raw keys are declared internal in UPGRADING; actually narrowing the visibility ofnamespace_stackable/namespace_stackable_with_hash/namespace_inheritableis a deliberate follow-up, not part of this PR.StackableValues#[]never returnsnil— discovered while writing the accessors: therepresentations ||= []seeding, themiddleware || []fallback, and a now-unnecessary&.before.lastwere all removed with empirical verification.?readers returnfalserather thannilwhen unset (all consumers are boolean-context); the emptymiddlewareread returns the shared frozen array instead of a fresh mutable one.Integration-only changes (beyond the twelve PRs)
Endpoint#error_middleware_options,#compile!,#build_stack,#initialize(where Internalize params state behind Grape::Util::InheritableSetting accessors #2797'spoint_in_time_copy_for_endpointsupersedes Internalize represent registrations behind Grape::Util::InheritableSetting accessors #2801's and Internalize format and error-response defaults behind Grape::Util::InheritableSetting accessors #2805's line-level edits, withcopy.default_error_status ||= 500harmonized onto the accessor) andContractScope(Internalize params state behind Grape::Util::InheritableSetting accessors #2797 + Internalize contract key maps behind Grape::Util::InheritableSetting accessors #2804).ns_inhlocal inerror_middleware_options(every entry reads through accessors after Internalize rescue configuration behind Grape::Util::InheritableSetting accessors #2799 + Internalize format and error-response defaults behind Grape::Util::InheritableSetting accessors #2805 combine)..rubocop.yml:Metrics/ClassLength305 → 320 — the accessor catalog putsInheritableSettingat 316 lines; each PR passed individually, the union crosses the old limit.What deliberately remains raw (the follow-up visibility PR)
One bulk
to_hashconsumer, which needs a dedicated accessor before the store readers can go protected:Endpoint#prepare_default_path_settings(merged stackable+inheritable hash for route pattern settings). The other former bulk consumer —Instance's route-config hash copy — turned out to be vestigial and was removed in #2807.Verification
Full
rubocop(330 files, no offenses) and the full spec suite (2363 examples, 0 failures) pass on this branch, as they did on each of the twelve standalone branches.🤖 Generated with Claude Code