Skip to content

Commit 4b08638

Browse files
ericproulxclaude
andcommitted
Make forward_match an internal kwarg instead of a route option
`forward_match` was threaded from the mount/route DSL through `Endpoint::Options` and the route options hash, only to be read once in `Route#initialize` to pick the match function. Since the value it derives from (`config.app`) already lives on the endpoint, compute it locally in `to_routes` and pass it to `Grape::Router::Route.new` as a keyword argument. This drops `forward_match` from `Endpoint::Options`, from the route options hash, and removes the now-dead `BaseRoute#forward_match` delegator (which had no callers). Adds a direct unit test for `Route#match?` under both forward/non-forward modes and an UPGRADING note for the removed introspection surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 927feec commit 4b08638

10 files changed

Lines changed: 115 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#### Features
44

55
* [#2768](https://github.com/ruby-grape/grape/pull/2768): Remove Guard - [@ericproulx](https://github.com/ericproulx).
6+
* [#2774](https://github.com/ruby-grape/grape/pull/2774): Make `forward_match` an internal kwarg instead of a route option - [@ericproulx](https://github.com/ericproulx).
67
* Your contribution here.
78

89
#### Fixes

UPGRADING.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
Upgrading Grape
22
===============
33

4+
### Upgrading to >= 4.0.0
5+
6+
#### `forward_match` is no longer exposed on routes
7+
8+
`forward_match` is an internal, construction-time detail that decides how a route matches incoming paths (a prefix match for mounted Rack apps, the compiled pattern otherwise). It is now passed to `Grape::Router::Route` as a keyword argument and derived from the mounted app instead of being carried in the route's options.
9+
10+
As a result it is no longer readable through `route.options[:forward_match]` or the `route.forward_match` reader — both previously returned the flag. Nothing in Grape consumed either, so this only affects code that introspected routes directly; there is no replacement, as the value is now purely internal.
11+
412
### Upgrading to >= 3.3
513

614
#### Minimum required Ruby is now 3.3

lib/grape/dsl/routing.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@ def mount(mounts, *opts)
160160
path:,
161161
app:,
162162
route_options: { anchor: false },
163-
forward_match: !app.respond_to?(:inheritable_setting),
164163
for: self
165164
)
166165
end

lib/grape/endpoint.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ def to_routes
276276
default_route_options = prepare_default_route_attributes(route_options)
277277
complete_route_options = route_options.merge(default_route_options)
278278
path_settings = prepare_default_path_settings
279+
forward_match = config.app && !config.app.respond_to?(:inheritable_setting)
279280

280281
config.http_methods.flat_map do |method|
281282
config.path.map do |path|
@@ -289,7 +290,7 @@ def to_routes
289290
version: default_route_options[:version],
290291
requirements: default_route_options[:requirements]
291292
)
292-
Grape::Router::Route.new(self, method, pattern, complete_route_options)
293+
Grape::Router::Route.new(self, method, pattern, complete_route_options, forward_match:)
293294
end
294295
end
295296
end
@@ -301,8 +302,7 @@ def prepare_default_route_attributes(route_options)
301302
requirements: prepare_routes_requirements(route_options[:requirements]),
302303
prefix: inheritable_setting.namespace_inheritable[:root_prefix],
303304
anchor: route_options.fetch(:anchor, true),
304-
settings: inheritable_setting.route.except(:declared_params, :saved_validations),
305-
forward_match: config.forward_match
305+
settings: inheritable_setting.route.except(:declared_params, :saved_validations)
306306
}
307307
end
308308

lib/grape/endpoint/options.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ class Endpoint
88
# stays a plain Hash for downstream gems (e.g. grape-swagger).
99
# +:method+ is renamed to +:http_methods+ on the value object to avoid
1010
# shadowing +Object#method+ via the generated Data accessor.
11-
Options = Data.define(:path, :http_methods, :for, :route_options, :app, :format, :forward_match) do
12-
def initialize(path:, method:, route_options: {}, app: nil, format: nil, forward_match: nil, **rest)
11+
Options = Data.define(:path, :http_methods, :for, :route_options, :app, :format) do
12+
def initialize(path:, method:, route_options: {}, app: nil, format: nil, **rest)
1313
path = Array(path)
1414
path << '/' if path.empty?
15-
super(path:, http_methods: Array(method), route_options:, app:, format:, forward_match:, **rest)
15+
super(path:, http_methods: Array(method), route_options:, app:, format:, **rest)
1616
end
1717
end
1818
end

lib/grape/router/base_route.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ class BaseRoute
1010
attr_reader :options, :pattern
1111

1212
def_delegators :@pattern, :path, :origin
13-
def_delegators :@options, :description, :version, :requirements, :prefix, :anchor, :settings, :forward_match, *Grape::Util::ApiDescription::DSL_METHODS
13+
def_delegators :@options, :description, :version, :requirements, :prefix, :anchor, :settings, *Grape::Util::ApiDescription::DSL_METHODS
1414

1515
def initialize(pattern, options = {})
1616
@pattern = pattern

lib/grape/router/route.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ class Route < BaseRoute
1212

1313
def_delegators :@app, :call
1414

15-
def initialize(endpoint, method, pattern, options)
15+
def initialize(endpoint, method, pattern, options, forward_match:)
1616
super(pattern, options)
1717
@app = endpoint
1818
@request_method = upcase_method(method)
19-
@match_function = options[:forward_match] ? FORWARD_MATCH_METHOD : NON_FORWARD_MATCH_METHOD
19+
@match_function = forward_match ? FORWARD_MATCH_METHOD : NON_FORWARD_MATCH_METHOD
2020
end
2121

2222
def convert_to_head_request!

spec/grape/api_spec.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3708,6 +3708,21 @@ def self.call(object, _env)
37083708
expect(last_response.body).to eq('sauce')
37093709
end
37103710

3711+
it 'does not forward unknown subpaths to a mounted API' do
3712+
subject.namespace :cool do
3713+
app = Class.new(Grape::API) # rubocop:disable RSpec/DescribedClass
3714+
app.get('/awesome') { 'sauce' }
3715+
mount app
3716+
end
3717+
3718+
get '/cool/awesome'
3719+
expect(last_response).to be_successful
3720+
get '/cool/awesome/subpath'
3721+
expect(last_response).to be_not_found
3722+
get '/cool/unknown'
3723+
expect(last_response).to be_not_found
3724+
end
3725+
37113726
it 'mounts on a nested path' do
37123727
app1 = Class.new(described_class)
37133728
app2 = Class.new(described_class)

spec/grape/endpoint_spec.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1034,7 +1034,6 @@ def memoized
10341034
path: '/path',
10351035
app: {},
10361036
route_options: { anchor: false },
1037-
forward_match: true,
10381037
for: Class.new
10391038
}
10401039
end

spec/grape/router/route_spec.rb

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe Grape::Router::Route do
4+
let(:instance) { described_class.new(endpoint, :get, pattern, options, forward_match:) }
5+
let(:endpoint) { instance_double(Grape::Endpoint) }
6+
let(:options) { {} }
7+
let(:pattern) do
8+
Grape::Router::Pattern.new(
9+
origin: '/mounty',
10+
suffix: '',
11+
anchor: true,
12+
params: {},
13+
format: nil,
14+
version: nil,
15+
requirements: {}
16+
)
17+
end
18+
19+
describe 'inheritance' do
20+
subject { described_class.new(endpoint, :get, pattern, options, forward_match: false) }
21+
22+
it { is_expected.to be_a(Grape::Router::BaseRoute) }
23+
end
24+
25+
describe '#match?' do
26+
subject { instance.match?(input) }
27+
28+
context 'when forward_match is true' do
29+
let(:forward_match) { true }
30+
31+
context 'with the exact origin' do
32+
let(:input) { '/mounty' }
33+
34+
it { is_expected.to be_truthy }
35+
end
36+
37+
context 'with a subpath under the origin' do
38+
let(:input) { '/mounty/awesome/deep' }
39+
40+
it 'matches on the origin prefix' do
41+
expect(subject).to be_truthy
42+
end
43+
end
44+
45+
context 'with a path outside the origin' do
46+
let(:input) { '/other' }
47+
48+
it { is_expected.to be_falsey }
49+
end
50+
51+
context 'with a blank input' do
52+
let(:input) { '' }
53+
54+
it { is_expected.to be(false) }
55+
end
56+
end
57+
58+
context 'when forward_match is false' do
59+
let(:forward_match) { false }
60+
61+
context 'with the exact origin' do
62+
let(:input) { '/mounty' }
63+
64+
it { is_expected.to be_truthy }
65+
end
66+
67+
context 'with a subpath under the origin' do
68+
let(:input) { '/mounty/awesome/deep' }
69+
70+
it 'does not match beyond the anchored pattern' do
71+
expect(subject).to be_falsey
72+
end
73+
end
74+
75+
context 'with a blank input' do
76+
let(:input) { '' }
77+
78+
it { is_expected.to be(false) }
79+
end
80+
end
81+
end
82+
end

0 commit comments

Comments
 (0)