Skip to content

Commit e489995

Browse files
namusyakadblock
authored andcommitted
Introduce Router#process_route for refactoring
1 parent 499f6c6 commit e489995

File tree

4 files changed

+34
-30
lines changed

4 files changed

+34
-30
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Next Release
1212
#### Fixes
1313

1414
* [#1505](https://github.com/ruby-grape/grape/pull/1505): Run `before` and `after` callbacks, but skip the rest when handling OPTIONS - [@jlfaber](https://github.com/jlfaber).
15-
* [#1517](https://github.com/ruby-grape/grape/pull/1517): Modify the priority of ANY routes, fixes #1089 - [@namusyaka](https://github.com/namusyaka), [@wagenet](https://github.com/wagenet).
15+
* [#1517](https://github.com/ruby-grape/grape/pull/1517), [#1089](https://github.com/ruby-grape/grape/pull/1089): Fix: priority of ANY routes - [@namusyaka](https://github.com/namusyaka), [@wagenet](https://github.com/wagenet).
1616

1717
0.18.0 (10/7/2016)
1818
==================

UPGRADING.md

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@ Upgrading Grape
33

44
### Upgrading to >= 0.18.1
55

6-
#### Modify the priority of :any routes
6+
#### Changes in priority of :any routes
77

8-
Conventional, the :any routes had been searched after matching first route and 405 routes.
9-
This version adds the :any routes processing before 405 processing, so the behavior of following code will be changed.
8+
Prior to this version, `:any` routes were searched after matching first route and 405 routes. This behavior has changed and `:any` routes are now searched before 405 processing. In the following example the `:any` route will match first when making a request with an unsupported verb.
109

1110
```ruby
1211
post :example do
@@ -16,7 +15,7 @@ route :any, '*path' do
1615
error! :not_found, 404
1716
end
1817

19-
get '/example' #=> before: 405, currently: 404
18+
get '/example' #=> before: 405, after: 404
2019
```
2120

2221
#### Removed param processing from built-in OPTIONS handler

lib/grape/router.rb

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -69,60 +69,65 @@ def recognize_path(input)
6969

7070
def identity(env)
7171
route = nil
72-
response = transaction(env) do |input, method, routing_args|
72+
response = transaction(env) do |input, method|
7373
route = match?(input, method)
74-
if route
75-
env[Grape::Env::GRAPE_ROUTING_ARGS] = make_routing_args(routing_args, route, input)
76-
route.exec(env)
77-
end
74+
process_route(route, env) if route
7875
end
7976
[response, route]
8077
end
8178

8279
def rotation(env, exact_route = nil)
8380
response = nil
84-
input, method, routing_args = *extract_required_args(env)
81+
input, method = *extract_input_and_method(env)
8582
map[method].each do |route|
8683
next if exact_route == route
8784
next unless route.match?(input)
88-
env[Grape::Env::GRAPE_ROUTING_ARGS] = make_routing_args(routing_args, route, input)
89-
response = route.exec(env)
85+
response = process_route(route, env)
9086
break unless cascade?(response)
9187
end
9288
response
9389
end
9490

9591
def transaction(env)
96-
input, method, routing_args = *extract_required_args(env)
97-
response = yield(input, method, routing_args)
92+
input, method = *extract_input_and_method(env)
93+
response = yield(input, method)
9894

9995
return response if response && !(cascade = cascade?(response))
100-
10196
neighbor = greedy_match?(input)
10297

103-
if neighbor && method == 'OPTIONS'
104-
return (!cascade && neighbor) ? call_with_allow_headers(env, neighbor.allow_header, neighbor.endpoint) : nil
105-
end
106-
107-
if route = match?(input, '*')
108-
env[Grape::Env::GRAPE_ROUTING_ARGS] = make_routing_args(routing_args, route, input)
109-
response = route.exec(env)
98+
# If neighbor exists and request method is OPTIONS,
99+
# return response by using #call_with_allow_headers.
100+
return call_with_allow_headers(
101+
env,
102+
neighbor.allow_header,
103+
neighbor.endpoint
104+
) if neighbor && method == 'OPTIONS' && !cascade
105+
106+
route = match?(input, '*')
107+
if route
108+
response = process_route(route, env)
110109
return response if response && !(cascade = cascade?(response))
111110
end
112111

113112
(!cascade && neighbor) ? call_with_allow_headers(env, neighbor.allow_header, neighbor.endpoint) : nil
114113
end
115114

115+
def process_route(route, env)
116+
input, = *extract_input_and_method(env)
117+
routing_args = env[Grape::Env::GRAPE_ROUTING_ARGS]
118+
env[Grape::Env::GRAPE_ROUTING_ARGS] = make_routing_args(routing_args, route, input)
119+
route.exec(env)
120+
end
121+
116122
def make_routing_args(default_args, route, input)
117123
args = default_args || { route_info: route }
118124
args.merge(route.params(input))
119125
end
120126

121-
def extract_required_args(env)
127+
def extract_input_and_method(env)
122128
input = string_for(env[Grape::Http::Headers::PATH_INFO])
123129
method = env[Grape::Http::Headers::REQUEST_METHOD]
124-
routing_args = env[Grape::Env::GRAPE_ROUTING_ARGS]
125-
[input, method, routing_args]
130+
[input, method]
126131
end
127132

128133
def with_optimization

lib/grape/router/pattern.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,12 @@ def pattern_options
3636

3737
def build_path(pattern, anchor: false, suffix: nil, **_options)
3838
unless anchor || pattern.end_with?('*path')
39-
pattern << ?/ unless pattern.end_with?(?/)
39+
pattern << '/' unless pattern.end_with?('/')
4040
pattern << '*path'
4141
end
42-
pattern = pattern.split(?/).tap { |parts|
43-
parts[parts.length - 1] = ?? + parts.last
44-
}.join(?/) if pattern.end_with?('*path')
42+
pattern = pattern.split('/').tap do |parts|
43+
parts[parts.length - 1] = '?' + parts.last
44+
end.join('/') if pattern.end_with?('*path')
4545
pattern + suffix.to_s
4646
end
4747

0 commit comments

Comments
 (0)