Skip to content

Commit

Permalink
Implemented 405 response instead of 404, resolves #25.
Browse files Browse the repository at this point in the history
  • Loading branch information
WorstOfAny committed Sep 5, 2017
1 parent 15dd686 commit faa803e
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 76 deletions.
8 changes: 7 additions & 1 deletion lib/flame/dispatcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ def config
@app_class.config
end

## Available routes endpoint
def available_endpoint
@available_endpoint ||=
@app_class.router.routes.endpoint(*request.path.parts)
end

## Build a path to the given controller and action, with any expected params
##
## @param ctrl [Flame::Controller] class of controller
Expand Down Expand Up @@ -155,7 +161,7 @@ def cached_tilts
## Return response if HTTP-method is OPTIONS
def try_options
return unless request.http_method == :OPTIONS
allow = @app_class.router.routes.dig(request.path)&.allow
allow = available_endpoint&.allow
halt 404 unless allow
response.headers['Allow'] = allow
end
Expand Down
9 changes: 6 additions & 3 deletions lib/flame/dispatcher/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ module Routes

## Find route and try execute it
def try_route
route = @app_class.router.find_route(request.path, request.http_method)
return nil unless route
http_method = request.http_method
http_method = :GET if http_method == :HEAD
return nil unless available_endpoint
route = available_endpoint[http_method]
halt(405, nil, 'Allow' => available_endpoint.allow) unless route
status 200
execute_route(route)
execute_route route
true
end

Expand Down
2 changes: 1 addition & 1 deletion lib/flame/path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class Path
## @param parts [Array<String, Flame::Path>] parts of expected path
## @return [Flame::Path] path from parts
def self.merge(*parts)
parts.join('/').gsub(%r{\/{2,}}, '/')
parts.join('/').gsub(%r|/{2,}|, '/')
end

def initialize(*paths)
Expand Down
22 changes: 3 additions & 19 deletions lib/flame/router.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,30 +36,14 @@ def add_controller(ctrl, path = nil, &block)

using GorillaPatch::DigEmpty

## Find route by any attributes
## @param path [Flame::Path, String] path for route search
## @param http_method [Symbol, nil] HTTP-method
## @return [Flame::Route, nil] return the found route, otherwise `nil`
def find_route(path, http_method = nil)
path = Flame::Path.new(path) unless path.is_a?(Flame::Path)
endpoint = routes.dig(*path.parts)&.dig_through_opt_args
return unless endpoint
http_method = :GET if http_method == :HEAD
## For `find_nearest_route` with any method
route = http_method ? endpoint[http_method] : endpoint.values.first
route if route.is_a? Flame::Router::Route
end

## Find the nearest route by path
## @param path [Flame::Path] path for route finding
## @return [Flame::Route, nil] return the found nearest route or `nil`
def find_nearest_route(path)
path = Flame::Path.new(path) if path.is_a? String
path_parts = path.parts.dup
path_parts = Flame::Path.new(path).parts.dup
while path_parts.size >= 0
route = find_route Flame::Path.new(*path_parts)
break if route || path_parts.empty?
path_parts.pop
route = routes.endpoint(*path_parts)&.values&.first
break if route&.is_a?(Flame::Router::Route) || path_parts.pop.nil?
end
route.is_a?(Flame::Router::Routes) ? route[nil] : route
end
Expand Down
6 changes: 6 additions & 0 deletions lib/flame/router/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ def allow
methods.push(:OPTIONS).join(', ')
end

## Move like '#dig' with '#dig_through_opt_args'
## @param path_parts [Array<String, Flame::Path, Flame::Path::Part>]
def endpoint(*path_parts)
dig(*path_parts)&.dig_through_opt_args
end

private

%i[req opt].each do |type|
Expand Down
16 changes: 16 additions & 0 deletions spec/unit/dispatcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ def hello(name)
"Hello, #{name}!"
end

def baz(var = nil)
"Hello, #{var}!"
end

def test
'Route content'
end
Expand Down Expand Up @@ -128,6 +132,12 @@ class DispatcherApplication < Flame::Application
respond.body.should.equal []
end

it 'should return 405 for not allowed HTTP-method with Allow header' do
respond = @init.call(method: 'POST').run!.last
respond.headers['Allow'].should.equal 'GET, OPTIONS'
respond.status.should.equal 405
end

describe 'OPTIONS HTTP-method' do
before do
@respond = @init.call(method: 'OPTIONS').run!.last
Expand Down Expand Up @@ -158,6 +168,12 @@ class DispatcherApplication < Flame::Application
respond = dispatcher.run!.last
respond.headers.key?('Allow').should.be.false
end

should 'return `Allow` header for route with optional parameters' do
dispatcher = @init.call(method: 'OPTIONS', path: '/baz')
respond = dispatcher.run!.last
respond.headers.key?('Allow').should.be.true
end
end
end

Expand Down
14 changes: 14 additions & 0 deletions spec/unit/router/routes_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,18 @@
@routes['foo']['bar'].allow.should.be.nil
end
end

describe '#endpoint' do
should 'return nested routes from path' do
routes = @init.call(['/foo', '/:?var', '/bar'])
routes.endpoint('/foo').should.equal routes['foo'][':?var']
routes.endpoint('/foo/:?').should.equal routes['foo'][':?var']
routes.endpoint('/foo/some/bar')
.should.equal routes['foo'][':?var']['bar']
end

should 'return nil for not-existing path' do
@routes.endpoint('/foo/baz').should.be.nil
end
end
end
52 changes: 0 additions & 52 deletions spec/unit/router_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -266,58 +266,6 @@ class RouterApplication < Flame::Application
end
end

describe '#find_route' do
it 'should receive path as String' do
@router.add_controller RouterRESTController
-> { @router.find_route('/router_rest/42', :PUT) }
.should.not.raise(NoMethodError)
end

it 'should return route by path and HTTP-method' do
@router.add_controller RouterRESTController
@router.find_route('/router_rest/42', :PUT)
.should.equal Flame::Router::Route.new(
RouterRESTController, :update
)
end

it 'should return route by path without optional argument' do
@router.add_controller RouterController
@router.find_route('/router/foo/bar/baz', :GET)
.should.equal Flame::Router::Route.new(
RouterController, :foo
)
end

it 'should return route by path without multiple optional arguments' do
@router.add_controller RouterController
@router.find_route('/router/foo/bar/baz', :GET)
.should.equal Flame::Router::Route.new(
RouterController, :foo
)
end

it 'should return route by HEAD HTTP-request instead of GET' do
@router.add_controller RouterRESTController
@router.find_route('/router_rest', :HEAD)
.should.equal Flame::Router::Route.new(
RouterRESTController, :index
)
end

it 'should return nil for not existing route' do
@router.add_controller RouterRESTController
@router.find_route(action: :foo)
.should.equal nil
end

it 'should return nil by path without required argument' do
@router.add_controller RouterController
@router.find_route(path: '/router/foo/bar')
.should.equal nil
end
end

describe '#find_nearest_route' do
it 'should return route by path' do
@router.add_controller RouterController
Expand Down

0 comments on commit faa803e

Please sign in to comment.