Skip to content

Populate the env route information when calling the automatically added endpoints #2076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* [#2067](https://github.com/ruby-grape/grape/pull/2067): Coerce empty string to nil for all primitive types except String - [@petekinnecom](https://github.com/petekinnecom).
* [#2064](https://github.com/ruby-grape/grape/pull/2064): Fix Ruby 2.7 deprecation warning in `Grape::Middleware::Base#initialize` - [@skarger](https://github.com/skarger).
* [#2072](https://github.com/ruby-grape/grape/pull/2072): Fix `Grape.eager_load!` and `compile!` - [@stanhu](https://github.com/stanhu).
* [#2076](https://github.com/ruby-grape/grape/pull/2076): Make route information available for hooks when the automatically generated endpoints are invoked - [@anakinj](https://github.com/anakinj).
* Your contribution here.

### 1.3.3 (2020/05/23)
Expand Down
48 changes: 22 additions & 26 deletions lib/grape/api/instance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -192,37 +192,15 @@ def cascade?
# will return an HTTP 405 response for any HTTP method that the resource
# cannot handle.
def add_head_not_allowed_methods_and_options_methods
routes_map = {}

self.class.endpoints.each do |endpoint|
routes = endpoint.routes
routes.each do |route|
# using the :any shorthand produces [nil] for route methods, substitute all manually
route_key = route.pattern.to_regexp
routes_map[route_key] ||= {}
route_settings = routes_map[route_key]
route_settings[:pattern] = route.pattern
route_settings[:requirements] = route.requirements
route_settings[:path] = route.origin
route_settings[:methods] ||= []
if route.request_method == '*' || route_settings[:methods].include?('*')
route_settings[:methods] = Grape::Http::Headers::SUPPORTED_METHODS
else
route_settings[:methods] << route.request_method
end
route_settings[:endpoint] = route.app
end
end

versioned_route_configs = collect_route_config_per_pattern
# The paths we collected are prepared (cf. Path#prepare), so they
# contain already versioning information when using path versioning.
# Disable versioning so adding a route won't prepend versioning
# informations again.
without_root_prefix do
without_versioning do
routes_map.each_value do |config|
methods = config[:methods]
allowed_methods = methods.dup
versioned_route_configs.each do |config|
allowed_methods = config[:methods].dup

unless self.class.namespace_inheritable(:do_not_route_head)
allowed_methods |= [Grape::Http::Headers::HEAD] if allowed_methods.include?(Grape::Http::Headers::GET)
Expand All @@ -241,6 +219,25 @@ def add_head_not_allowed_methods_and_options_methods
end
end

def collect_route_config_per_pattern
all_routes = self.class.endpoints.map(&:routes).flatten
routes_by_regexp = all_routes.group_by { |route| route.pattern.to_regexp }

# Build the configuration based on the first endpoint and the collection of methods supported.
routes_by_regexp.values.map do |routes|
last_route = routes.last # Most of the configuration is taken from the last endpoint
matching_wildchar = routes.any? { |route| route.request_method == '*' }
{
options: {},
pattern: last_route.pattern,
requirements: last_route.requirements,
path: last_route.origin,
endpoint: last_route.app,
methods: matching_wildchar ? Grape::Http::Headers::SUPPORTED_METHODS : routes.map(&:request_method)
}
end
end

# Generate a route that returns an HTTP 405 response for a user defined
# path on methods not specified
def generate_not_allowed_method(pattern, allowed_methods: [], **attributes)
Expand All @@ -252,7 +249,6 @@ def generate_not_allowed_method(pattern, allowed_methods: [], **attributes)
end
not_allowed_methods = supported_methods - allowed_methods
return if not_allowed_methods.empty?

@router.associate_routes(pattern, not_allowed_methods: not_allowed_methods, **attributes)
end

Expand Down
35 changes: 19 additions & 16 deletions lib/grape/router.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,37 +99,34 @@ def transaction(env)
response = yield(input, method)

return response if response && !(cascade = cascade?(response))
neighbor = greedy_match?(input)
last_neighbor_route = greedy_match?(input)

# If neighbor exists and request method is OPTIONS,
# If last_neighbor_route exists and request method is OPTIONS,
# return response by using #call_with_allow_headers.
return call_with_allow_headers(
env,
neighbor.allow_header,
neighbor.endpoint
) if neighbor && method == Grape::Http::Headers::OPTIONS && !cascade
return call_with_allow_headers(env, last_neighbor_route) if last_neighbor_route && method == Grape::Http::Headers::OPTIONS && !cascade

route = match?(input, '*')
return neighbor.endpoint.call(env) if neighbor && cascade && route

return last_neighbor_route.endpoint.call(env) if last_neighbor_route && cascade && route

if route
response = process_route(route, env)
return response if response && !(cascade = cascade?(response))
end

!cascade && neighbor ? call_with_allow_headers(env, neighbor.allow_header, neighbor.endpoint) : nil
return call_with_allow_headers(env, last_neighbor_route) if !cascade && last_neighbor_route

nil
end

def process_route(route, env)
input, = *extract_input_and_method(env)
routing_args = env[Grape::Env::GRAPE_ROUTING_ARGS]
env[Grape::Env::GRAPE_ROUTING_ARGS] = make_routing_args(routing_args, route, input)
prepare_env_from_route(env, route)
route.exec(env)
end

def make_routing_args(default_args, route, input)
args = default_args || { route_info: route }
args.merge(route.params(input))
args.merge(route.params(input) || {})
end

def extract_input_and_method(env)
Expand Down Expand Up @@ -160,9 +157,15 @@ def greedy_match?(input)
@neutral_map.detect { |route| last_match["_#{route.index}"] }
end

def call_with_allow_headers(env, methods, endpoint)
env[Grape::Env::GRAPE_ALLOWED_METHODS] = methods.join(', ').freeze
endpoint.call(env)
def call_with_allow_headers(env, route)
prepare_env_from_route(env, route)
env[Grape::Env::GRAPE_ALLOWED_METHODS] = route.allow_header.join(', ').freeze
route.endpoint.call(env)
end

def prepare_env_from_route(env, route)
input, = *extract_input_and_method(env)
env[Grape::Env::GRAPE_ROUTING_ARGS] = make_routing_args(env[Grape::Env::GRAPE_ROUTING_ARGS], route, input)
end

def cascade?(response)
Expand Down
65 changes: 65 additions & 0 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,71 @@ class DummyFormatClass
end
end

describe 'when hook behaviour is controlled by attributes on the route ' do
before do
subject.before do
error!('Access Denied', 401) unless route.options[:secret] == params[:secret]
end

subject.namespace 'example' do
before do
error!('Access Denied', 401) unless route.options[:namespace_secret] == params[:namespace_secret]
end

desc 'it gets with secret', secret: 'password'
get { status(params[:id] == '504' ? 200 : 404) }

desc 'it post with secret', secret: 'password', namespace_secret: 'namespace_password'
post {}
end
end

context 'when HTTP method is not defined' do
let(:response) { delete('/example') }

it 'responds with a 405 status' do
expect(response.status).to eql 405
end
end

context 'when HTTP method is defined with attribute' do
let(:response) { post('/example?secret=incorrect_password') }
it 'responds with the defined error in the before hook' do
expect(response.status).to eql 401
end
end

context 'when HTTP method is defined and the underlying before hook expectation is not met' do
let(:response) { post('/example?secret=password&namespace_secret=wrong_namespace_password') }
it 'ends up in the endpoint' do
expect(response.status).to eql 401
end
end

context 'when HTTP method is defined and everything is like the before hooks expect' do
let(:response) { post('/example?secret=password&namespace_secret=namespace_password') }
it 'ends up in the endpoint' do
expect(response.status).to eql 201
end
end

context 'when HEAD is called for the defined GET' do
let(:response) { head('/example?id=504') }

it 'responds with 401 because before expectations in before hooks are not met' do
expect(response.status).to eql 401
end
end

context 'when HEAD is called for the defined GET' do
let(:response) { head('/example?id=504&secret=password') }

it 'responds with 200 because before hooks are not called' do
expect(response.status).to eql 200
end
end
end

context 'allows HEAD on a GET request that' do
before do
subject.get 'example' do
Expand Down