diff --git a/CHANGELOG.md b/CHANGELOG.md index 0049465b9..ee970b475 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ * [#2827](https://github.com/ruby-grape/grape/pull/2827): Make the `cascade` DSL getter return the configured value (`cascade false` read back as `true`) - [@ericproulx](https://github.com/ericproulx). * [#2829](https://github.com/ruby-grape/grape/pull/2829): Fix a cascading route handing over only to the last route registered for the path, making a middle version (3+ mounted versions with a catch-all) answer 406 - [@ericproulx](https://github.com/ericproulx). * [#2826](https://github.com/ruby-grape/grape/pull/2826): Fix `api.version` not being set for the root route of a path-versioned API (`GET /v1`) - [@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