[Redirect] Allows for Class and Symbol Controller name (#363)

* [REDIRECT] Allows for Class and Symbol Full Controller name

Issue: #353

> As a developer I would like to pass and full controller name to a
redirect as a class or symbol and allow for the redirection to happen

Example

`redirect_to controller: :HomeController, action: :index`

- Would be nice to accept HelloController::Index or
Admin::UserController:Index

* fixup! [REDIRECT] Allows for Class and Symbol Full Controller name

* Adds Hash Table Lookup for Routes


Looking up routes base on controller and action in a radix tree causes On2
many cases because of the radix tree.

To avoid that pain this adds hash table to perform lookups by conntroller
name and action while this might consume more memory it performs better and
makes the code cleaner and lookups based on controller#action are now more reliable

Signed-off-by: Elias Perez <eliasjpr@gmail.com>

* Removes Redirect Exception

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>

* Cleanup and fixes specs

Signed-off-by: Elias Perez <eliasjpr@gmail.com>

* Cleanup

spec/amber/controller/helpers/redirect_spec.cr

@@ -1,146 +1,111 @@
 require "../../../../spec_helper"
 
+def assert_expected_response?(controller, location, status_code)
+  controller.response.headers["location"].should eq location
+  controller.response.status_code.should eq status_code
+end
+
 module Amber::Controller::Helpers
   describe Redirector do
     describe "#redirect" do
       it "redirects to location" do
         controller = build_controller
         redirector = Redirector.new("/some_path")
-
         redirector.redirect(controller)
-
-        controller.response.headers["location"].should eq "/some_path"
-        controller.response.status_code.should eq 302
-        controller.context.content.nil?.should eq false
+        assert_expected_response?(controller, "/some_path", 302)
       end
 
       it "raises Exception::Controller::Redirect on invalid location" do
         controller = build_controller
-
         expect_raises Exceptions::Controller::Redirect do
           redirector = Redirector.new("", params: {"user_id" => "123"})
+          redirector.redirect(controller)
         end
       end
 
       context "with params" do
         it "redirect to location and adds params to url" do
           controller = build_controller
           redirector = Redirector.new("/some_path", params: {"user_id" => "123"})
-
           redirector.redirect(controller)
-
-          controller.response.headers["location"].should eq "/some_path?user_id=123"
-          controller.response.status_code.should eq 302
-          controller.context.content.nil?.should eq false
+          assert_expected_response?(controller, "/some_path?user_id=123", 302)
+          controller.response.headers["location"] = ""
         end
       end
 
       context "with flash" do
         it "redirects to location and adds flash" do
           controller = build_controller
           redirector = Redirector.new("/some_path", flash: {"success" => "Record saved!"})
-
-          flash = controller.flash
           redirector.redirect(controller)
-
-          controller.response.headers["location"].should eq "/some_path"
-          flash["success"].should eq "Record saved!"
-          controller.response.status_code.should eq 302
-          controller.context.content.nil?.should eq false
+          controller.flash["success"].should eq "Record saved!"
+          assert_expected_response?(controller, "/some_path", 302)
         end
       end
 
       context "with a different status code" do
         it "redirects to location and sets a status code" do
           controller = build_controller
           redirector = Redirector.new("/some_path", status = 301)
-
-          flash = controller.flash
           redirector.redirect(controller)
-
-          controller.response.headers["location"].should eq "/some_path"
-          controller.response.status_code.should eq 301
-          controller.context.content.nil?.should eq false
+          assert_expected_response?(controller, "/some_path", 301)
         end
       end
     end
 
     describe ".from_controller_action" do
-      it "raises an error for invalid controller/action" do
-        router = Amber::Server.router
-        router.draw :web { put "/invalid/:id", HelloController, :edit }
-        controller = build_controller
-
-        expect_raises Exceptions::Controller::Redirect do
-          redirector = Redirector.from_controller_action("bad", :bad)
-        end
+      Amber::Server.router.draw :web do
+        get "/redirect/:id", RedirectController, :show
+        get "/redirect/:id/edit", RedirectController, :edit
+        get "/redirect", RedirectController, :index
       end
 
-      context "when scope is present" do
-        it "redirects to the correct scoped location" do
-          router = Amber::Server.router
-          router.draw :web, "/scoped" { delete "/hello/:id", HelloController, :destroy }
-          controller = build_controller
-          redirector = Redirector.from_controller_action("hello", :destroy, params: {"id" => "5"})
-
-          redirector.redirect(controller)
-
-          controller.response.headers["location"].should eq "/scoped/hello/5"
-          controller.response.status_code.should eq 302
-          controller.context.content.nil?.should eq false
+      it "raises an error for invalid controller/action" do
+        expect_raises KeyError do
+          redirector = Redirector.from_controller_action(:bad, :bad)
         end
       end
 
-      context "with params" do
-        it "redirects to correct location for given controller action" do
-          router = Amber::Server.router
-          router.draw :web { get "/fake/:id", HelloController, :show }
-          controller = build_controller
-          redirector = Redirector.from_controller_action("hello", :show, params: {"id" => "11"})
-
-          redirector.redirect(controller)
-
-          controller.response.headers["location"].should eq "/fake/11"
-          controller.response.status_code.should eq 302
-          controller.context.content.nil?.should eq false
-        end
+      it "redirects to full controller name as symbol" do
+        controller = build_controller
+        redirector = Redirector.from_controller_action(RedirectController, :show, params: {"id" => "5"})
+        redirector.redirect(controller)
+        assert_expected_response?(controller, "/redirect/5", 302)
       end
 
-      context "without params" do
-        it "redirects to correct location for given controller action" do
-          router = Amber::Server.router
-          router.draw :web { get "/fake", HelloController, :index }
-          controller = build_controller
-          redirector = Redirector.from_controller_action("hello", :index)
+      it "redirects to full controller name as class" do
+        controller = build_controller
+        redirector = Redirector.from_controller_action(RedirectController, :show, params: {"id" => "5"})
+        redirector.redirect(controller)
+        assert_expected_response?(controller, "/redirect/5", 302)
+      end
 
-          redirector.redirect(controller)
+      it "redirects to :show" do
+        controller = build_controller
+        redirector = Redirector.from_controller_action(:redirect, :show, params: {"id" => "11"})
+        redirector.redirect(controller)
+        assert_expected_response?(controller, "/redirect/11", 302)
+      end
 
-          controller.response.headers["location"].should eq "/fake"
-          controller.response.status_code.should eq 302
-          controller.context.content.nil?.should eq false
-        end
+      it "redirects to edit action" do
+        controller = build_controller
+        redirector = Redirector.from_controller_action(:redirect, :edit, params: {"id" => "123"})
+        redirector.redirect(controller)
+        assert_expected_response?(controller, "/redirect/123/edit", 302)
       end
     end
 
     describe "#redirect_back" do
-      context "and has a valid referrer" do
-        it "sets the correct response headers" do
-          hello_controller = build_controller("/world")
-
-          hello_controller.redirect_back
-          response = hello_controller.response
-
-          response.headers["Location"].should eq "/world"
-        end
+      it "sets the correct response headers" do
+        controller = build_controller("/redirect")
+        controller.redirect_back
+        controller.response.headers["Location"].should eq "/redirect"
       end
 
-      context "and does not have a referrer" do
-        it "raises an error" do
-          hello_controller = build_controller
-
-          expect_raises Exceptions::Controller::Redirect do
-            hello_controller.redirect_back
-          end
+      it "raises an error" do
+        controller = build_controller("")
+        expect_raises Exceptions::Controller::Redirect do
+          controller.redirect_back
         end
       end
     end

spec/amber/router/pipe/pipeline_spec.cr

@@ -14,98 +14,61 @@ module Amber
         pipeline.pipeline[:api].size.should eq 2
       end
 
-      describe "#call" do
+      describe "with given server config" do
+        pipeline = Pipeline.new
+
+        Amber::Server.router.draw :web do
+          get "/valid/route", HelloController, :world
+          get "/index/:name", HelloController, :world
+          resources "/hello", HelloController
+        end
+
+        pipeline.build :web { }
+        pipeline.prepare_pipelines
+
         it "raises exception when route not found" do
-          pipeline = Pipeline.new
           request = HTTP::Request.new("GET", "/bad/route")
-          pipeline.build :web { plug Amber::Pipe::Logger.new }
-          Amber::Server.router.draw :web { get "/valid/route", HelloController, :world }
           create_request_and_return_io(pipeline, request).status_code.should eq 404
         end
 
         it "routes" do
-          pipeline = Pipeline.new
           request = HTTP::Request.new("GET", "/index/elias")
-          pipeline.build :web { plug Amber::Pipe::Logger.new }
-          Amber::Server.router.draw :web { get "/index/:name", HelloController, :world }
-
-          pipeline.prepare_pipelines
           response = create_request_and_return_io(pipeline, request)
-
           response.body.should eq "Hello World!"
         end
-      end
 
-      describe "http requests" do
         it "perform GET request" do
           request = HTTP::Request.new("GET", "/hello")
-          pipeline = Pipeline.new
-          pipeline.build :web { plug Amber::Pipe::Logger.new }
-          Amber::Server.router.draw :web { get "/hello", HelloController, :index }
-
-          pipeline.prepare_pipelines
           response = create_request_and_return_io(pipeline, request)
-
           response.body.should eq "Index"
         end
 
         it "perform PUT request" do
           request = HTTP::Request.new("PUT", "/hello/1")
-          pipeline = Pipeline.new
-          pipeline.build :web { plug Amber::Pipe::Logger.new }
-          Amber::Server.router.draw :web { put "/hello/:id", HelloController, :update }
-
-          pipeline.prepare_pipelines
           response = create_request_and_return_io(pipeline, request)
-
           response.body.should eq "Update"
         end
 
         it "perform PATCH request" do
           request = HTTP::Request.new("PATCH", "/hello/1")
-          pipeline = Pipeline.new
-          pipeline.build :web { plug Amber::Pipe::Logger.new }
-          Amber::Server.router.draw :web { patch "/hello/:id", HelloController, :update }
-
-          pipeline.prepare_pipelines
           response = create_request_and_return_io(pipeline, request)
-
           response.body.should eq "Update"
         end
 
         it "perform POST request" do
           request = HTTP::Request.new("POST", "/hello")
-          pipeline = Pipeline.new
-          pipeline.build :web { plug Amber::Pipe::Logger.new }
-          Amber::Server.router.draw :web { post "/hello", HelloController, :create }
-
-          pipeline.prepare_pipelines
           response = create_request_and_return_io(pipeline, request)
-
           response.body.should eq "Create"
         end
 
         it "perform DELETE request" do
           request = HTTP::Request.new("DELETE", "/hello/1")
-          pipeline = Pipeline.new
-          pipeline.build :web { plug Amber::Pipe::Logger.new }
-          Amber::Server.router.draw :web { delete "/hello/:id", HelloController, :destroy }
-
-          pipeline.prepare_pipelines
           response = create_request_and_return_io(pipeline, request)
-
           response.body.should eq "Destroy"
         end
-      end
 
-      describe "X-Powered-By Header" do
         it "initializes context with X-Powered-By: Amber" do
-          pipeline = Pipeline.new
           request = HTTP::Request.new("GET", "/index/faustino")
-          pipeline.build :web { plug Amber::Pipe::Logger.new }
-          Amber::Server.router.draw :web { get "/index/:name", HelloController, :world }
-
-          pipeline.prepare_pipelines
           response = create_request_and_return_io(pipeline, request)
           response.headers["X-Powered-By"].should eq "Amber"
         end