Skip to content

Commit

Permalink
Removes Redirect Exception
Browse files Browse the repository at this point in the history
Since it is now using hash table lookup for redirects there is no need for
raising errors, if the controller action combination does not exists it throws
KeyError exception.

Signed-off-by: Elias Perez <eliasjpr@gmail.com>
  • Loading branch information
eliasjpr committed Nov 7, 2017
1 parent ecdd809 commit ee61ce1
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 13 deletions.
15 changes: 8 additions & 7 deletions spec/amber/controller/helpers/redirect_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,13 @@ module Amber::Controller::Helpers
describe ".from_controller_action" do
Amber::Server.router.draw :web do
get "/redirect/:id", RedirectController, :show
get "/redirect/:id/edit", RedirectController, :edit
get "/redirect", RedirectController, :index
end

it "raises an error for invalid controller/action" do
expect_raises Exceptions::Controller::Redirect do
redirector = Redirector.from_controller_action("bad", :bad)
expect_raises KeyError do
redirector = Redirector.from_controller_action(:bad, :bad)
end
end

Expand All @@ -79,18 +80,18 @@ module Amber::Controller::Helpers
assert_expected_response?(controller, "/redirect/5", 302)
end

it "redirects to correct location for given controller action" do
it "redirects to :show" do
controller = build_controller
redirector = Redirector.from_controller_action("redirect", :show, params: {"id" => "11"})
redirector = Redirector.from_controller_action(:redirect, :show, params: {"id" => "11"})
redirector.redirect(controller)
assert_expected_response?(controller, "/redirect/11", 302)
end

it "redirects to correct location for given controller action" do
it "redirects to edit action" do
controller = build_controller
redirector = Redirector.from_controller_action("redirect", :index)
redirector = Redirector.from_controller_action(:redirect, :edit, params: {"id" => "123"})
redirector.redirect(controller)
assert_expected_response?(controller, "/redirect", 302)
assert_expected_response?(controller, "/redirect/123/edit", 302)
end
end

Expand Down
6 changes: 2 additions & 4 deletions src/amber/controller/helpers/redirect.cr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module Amber::Controller::Helpers
Redirector.from_controller_action(controller_name, action, **args).redirect(self)
end

def redirect_to(controller : String | Symbol | Class, action : Symbol, **args)
def redirect_to(controller : Symbol | Class, action : Symbol, **args)
Redirector.from_controller_action(controller, action, **args).redirect(self)
end

Expand All @@ -28,10 +28,8 @@ module Amber::Controller::Helpers
@params : Hash(String, String)? = nil
@flash : Hash(String, String)? = nil

def self.from_controller_action(controller : String | Symbol | Class, action : Symbol, **options)
controller = controller.to_s.sub(/Controller$/, "").downcase
def self.from_controller_action(controller : Symbol | Class, action : Symbol, **options)
route = Amber::Server.router.match_by_controller_action(controller, action)
raise Exceptions::Controller::Redirect.new("#{controller}##{action} not found!") unless route
params = options[:params]?
location, params = route.not_nil!.substitute_keys_in_path(params)
status = options[:status]? || DEFAULT_STATUS_CODE
Expand Down
6 changes: 5 additions & 1 deletion src/amber/router/route.cr
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ module Amber
{result, params}
end

def match?(controller, action)
def match?(controller : Class, action : Symbol)
self.controller.downcase == controller.to_s.downcase && self.action == action
end

def match?(controller : Symbol, action : Symbol)
self.controller.downcase == "#{controller}controller" && self.action == action
end
end
Expand Down
2 changes: 1 addition & 1 deletion src/amber/router/router.cr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module Amber
# This is the main application handler all routers should finally hit this
# handler.
class Router
property :routes, :socket_routes
property :routes, :routes_hash, :socket_routes

def initialize
@routes = Radix::Tree(Route).new
Expand Down

0 comments on commit ee61ce1

Please sign in to comment.