diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e54cdff8..6cb4abcb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ * [#2853](https://github.com/ruby-grape/grape/pull/2853): Restore, behind a deprecation warning, the trailing positional options Hash of `requires`, `optional` and `use`, which #2618 turned into a parameter name - [@ericproulx](https://github.com/ericproulx). * [#2856](https://github.com/ruby-grape/grape/pull/2856): Update simplecov - [@ericproulx](https://github.com/ericproulx). * [#2841](https://github.com/ruby-grape/grape/pull/2841): Stop `use`, `helpers`, `rescue_from` and other registrations declared below a route from reaching it when an earlier registration had seeded the same key (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). +* [#2846](https://github.com/ruby-grape/grape/pull/2846): Keep a `:version` path capture in `params` when the API declares no version, instead of always dropping it as Grape's own - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/lib/grape/request.rb b/lib/grape/request.rb index c56237e1d..e016169ed 100644 --- a/lib/grape/request.rb +++ b/lib/grape/request.rb @@ -168,8 +168,7 @@ def cookies? def make_params params = @params_builder.call(rack_params) - routing_args = env[Grape::Env::GRAPE_ROUTING_ARGS] - filtered = routing_args&.except(:version, :route_info) + filtered = routing_args_as_params(env[Grape::Env::GRAPE_ROUTING_ARGS]) return params if filtered.blank? params.deep_merge!(filtered) @@ -177,6 +176,31 @@ def make_params raise Grape::Exceptions::RequestError end + # The routing args carry two things that are not request params: + # +:route_info+, which is always Grape's own, and +:version+, which is only + # Grape's own when the API declared a version — that is captured as a path + # segment and exposed through +env['api.version']+ instead. + # + # An API that declares no version can legitimately name a param +:version+ + # (`route_param :version`, `get '/:version'`), and that capture belongs to + # the application. Dropping it unconditionally left `params[:version]` nil + # on a route that had matched, losing the segment silently. + def routing_args_as_params(routing_args) + return if routing_args.nil? + return routing_args.except(:version, :route_info) if grape_owns_version?(routing_args) + + routing_args.except(:route_info) + end + + # A route reports a +version+ only when the API declared one, which is the + # case where the captured segment is Grape's rather than the application's. + def grape_owns_version?(routing_args) + return false unless routing_args.key?(:version) + + route = routing_args[:route_info] + route.respond_to?(:version) && !route.version.nil? + end + # Uses a plain `each_header` block instead of `each_header.with_object`: # `with_object` can only pass the block one value plus the memo, so the # `k, v` pair would be boxed into a throwaway Array on every header. A diff --git a/spec/grape/request_spec.rb b/spec/grape/request_spec.rb index 19675178f..f1566960d 100644 --- a/spec/grape/request_spec.rb +++ b/spec/grape/request_spec.rb @@ -50,13 +50,29 @@ let(:routing_args) do { version: '123', - route_info: '456', + route_info: instance_double(Grape::Router::Route, version: route_version), c: 'ccc' } end - it 'cuts version and route_info' do - expect(request.params).to eq(ActiveSupport::HashWithIndifferentAccess.new(a: '123', b: 'xyz', c: 'ccc')) + context 'when the route carries a version of its own' do + let(:route_version) { 'v1' } + + it 'cuts version and route_info' do + expect(request.params).to eq(ActiveSupport::HashWithIndifferentAccess.new(a: '123', b: 'xyz', c: 'ccc')) + end + end + + # Without a declared version the captured segment is the application's: + # `route_param :version` on an unversioned API has to reach the endpoint. + context 'when the route carries no version' do + let(:route_version) { nil } + + it 'cuts only route_info' do + expect(request.params).to eq( + ActiveSupport::HashWithIndifferentAccess.new(a: '123', b: 'xyz', c: 'ccc', version: '123') + ) + end end end diff --git a/spec/integration/hashie/hashie_spec.rb b/spec/integration/hashie/hashie_spec.rb index c4d373e73..2a839b8e8 100644 --- a/spec/integration/hashie/hashie_spec.rb +++ b/spec/integration/hashie/hashie_spec.rb @@ -152,7 +152,7 @@ let(:routing_args) do { version: '123', - route_info: '456', + route_info: instance_double(Grape::Router::Route, version: 'v1'), c: 'ccc' } end