diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b1cb64a4..ca46339de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ * [#2825](https://github.com/ruby-grape/grape/pull/2825): Stop `present` entity autodetection from picking up a top-level `::Entity` constant - [@ericproulx](https://github.com/ericproulx). * [#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). * [#2834](https://github.com/ruby-grape/grape/pull/2834): Restore the #2824 fix for cascaded routes leaking `route_info` and path captures, silently reverted by #2829 - [@ericproulx](https://github.com/ericproulx). * Your contribution here. diff --git a/lib/grape/middleware/versioner/path.rb b/lib/grape/middleware/versioner/path.rb index 2129a6b31..c310c2dfc 100644 --- a/lib/grape/middleware/versioner/path.rb +++ b/lib/grape/middleware/versioner/path.rb @@ -31,14 +31,35 @@ def before end slash_position = path_info.index('/', 1) # omit the first one - return unless slash_position + return version_from_first_segment(path_info, slash_position) if slash_position + version_from_only_segment(path_info) + end + + private + + def version_from_first_segment(path_info, slash_position) potential_version = path_info[1..(slash_position - 1)] return unless potential_version.match?(pattern) version_not_found! unless potential_version_match?(potential_version) env[Grape::Env::API_VERSION] = potential_version end + + # The path is a single segment (e.g. `GET /v1` — the root route of a + # path-versioned API). Nothing follows to disambiguate a version from a + # plain path or from a `.format` suffix (`/v1.json`), so the version is + # only recorded on an exact match against the declared versions, and an + # unmatched segment is left for the router to resolve — never a 404. + def version_from_only_segment(path_info) + candidate = path_info[1..] + candidate = candidate[0...candidate.rindex('.')] while candidate.include?('.') && !declared_version?(candidate) + env[Grape::Env::API_VERSION] = candidate if declared_version?(candidate) + end + + def declared_version?(candidate) + versions.present? && versions.include?(candidate) + end end end end diff --git a/spec/grape/middleware/versioner/path_spec.rb b/spec/grape/middleware/versioner/path_spec.rb index c91d316a8..2a3cc3415 100644 --- a/spec/grape/middleware/versioner/path_spec.rb +++ b/spec/grape/middleware/versioner/path_spec.rb @@ -59,4 +59,39 @@ expect(subject.call(Rack::PATH_INFO => '/mounted/v1/foo').last).to eq('v1') end end + + # Regression: the version was only extracted when a second slash followed it, + # so the root route of a path-versioned API (`GET /v1`) ran without + # env['api.version'] being set. + context 'when the version is the only path segment' do + let(:options) { { versions: %w[v1] } } + + it 'sets the version' do + expect(subject.call(Rack::PATH_INFO => '/v1').last).to eq('v1') + end + + it 'sets the version when a format suffix follows' do + expect(subject.call(Rack::PATH_INFO => '/v1.json').last).to eq('v1') + end + + context 'with a dotted version' do + let(:options) { { versions: %w[v1.2] } } + + it 'sets the version when a format suffix follows' do + expect(subject.call(Rack::PATH_INFO => '/v1.2.json').last).to eq('v1.2') + end + end + + context 'behind a prefix' do + let(:options) { { versions: %w[v1], prefix: '/api' } } + + it 'sets the version' do + expect(subject.call(Rack::PATH_INFO => '/api/v1').last).to eq('v1') + end + end + + it 'leaves an unknown segment for the router instead of failing with 404' do + expect(subject.call(Rack::PATH_INFO => '/awesome').last).to be_nil + end + end end diff --git a/spec/shared/versioning_examples.rb b/spec/shared/versioning_examples.rb index 8d794f4e2..ef7a8f227 100644 --- a/spec/shared/versioning_examples.rb +++ b/spec/shared/versioning_examples.rb @@ -11,6 +11,16 @@ expect(last_response.body).to eql 'Version: v1' end + it 'sets the API version for the root route' do + subject.format :txt + subject.version 'v1', **macro_options.except(:format) + subject.get do + "Version: #{request.env[Grape::Env::API_VERSION]}" + end + versioned_get '/', 'v1', macro_options + expect(last_response.body).to eql 'Version: v1' + end + it 'adds the prefix before the API version' do subject.format :txt subject.prefix 'api'