Middleware ordering (#236)

Improve and correct request middleware

 Request -> Middleware -> Filter -> Route

spec/handler_spec.cr

@@ -0,0 +1,30 @@
+require "./spec_helper"
+
+class CustomTestHandler < HTTP::Handler
+  def call(env)
+    env.response << "Kemal"
+    call_next env
+  end
+end
+
+describe "Handler" do
+  it "adds custom handler before before_*" do
+    filter_middleware = Kemal::Middleware::Filter.new
+    filter_middleware._add_route_filter("GET", "/", :before) do |env|
+      env.response << " is"
+    end
+
+    filter_middleware._add_route_filter("GET", "/", :before) do |env|
+      env.response << " so"
+    end
+    add_handler CustomTestHandler.new
+
+    get "/" do |env|
+      " Great"
+    end
+    request = HTTP::Request.new("GET", "/")
+    client_response = call_request_on_app(request)
+    client_response.status_code.should eq(200)
+    client_response.body.should eq("Kemal is so Great")
+  end
+end

spec/helpers_spec.cr

@@ -30,7 +30,7 @@ describe "Macros" do
     it "sets a custom logger" do
       config = Kemal::Config::INSTANCE
       logger CustomLogHandler.new
-      config.handlers.last.should be_a(CustomLogHandler)
+      config.handlers[4].should be_a(CustomLogHandler)
       config.logger.should be_a(CustomLogHandler)
     end
   end
@@ -77,7 +77,6 @@ describe "Macros" do
           "Content-Type"                => "text/plain",
         }
       end
-
       request = HTTP::Request.new("GET", "/headers")
       response = call_request_on_app(request)
       response.headers["Access-Control-Allow-Origin"].should eq("*")
@@ -126,11 +125,7 @@ describe "Macros" do
   describe "#gzip" do
     it "adds HTTP::DeflateHandler to handlers" do
       gzip true
-      Kemal.config.handlers.last.is_a?(HTTP::DeflateHandler).should eq true
-    end
-
-    it "doesn't add HTTP::DeflateHandler to handlers by default" do
-      Kemal.config.handlers.last.is_a?(HTTP::DeflateHandler).should eq false
+      Kemal.config.handlers[4].should be_a(HTTP::DeflateHandler)
     end
   end
 

spec/spec_helper.cr

@@ -3,14 +3,9 @@ require "../src/*"
 
 include Kemal
 
-class CustomTestHandler < HTTP::Handler
-  def call(request)
-    call_next request
-  end
-end
-
 class CustomLogHandler < Kemal::BaseLogHandler
-  def call(context)
+  def call(env)
+    call_next env
   end
 
   def write(message)
@@ -44,20 +39,30 @@ def call_request_on_app(request)
   io = MemoryIO.new
   response = HTTP::Server::Response.new(io)
   context = HTTP::Server::Context.new(request, response)
-  Kemal::RouteHandler::INSTANCE.call(context)
+  main_handler = build_main_handler
+  main_handler.call context
   response.close
   io.rewind
   HTTP::Client::Response.from_io(io, decompress: false)
 end
 
+def build_main_handler
+  main_handler = Kemal.config.handlers.first
+  current_handler = main_handler
+  Kemal.config.handlers.each_with_index do |handler, index|
+    current_handler.next = handler
+    current_handler = handler
+  end
+  main_handler
+end
+
 Spec.before_each do
   config = Kemal.config
   config.env = "development"
   config.setup
-  config.add_handler Kemal::RouteHandler::INSTANCE
 end
 
 Spec.after_each do
-  Kemal.config.handlers.clear
+  Kemal.config.clear
   Kemal::RouteHandler::INSTANCE.tree = Radix::Tree(Route).new
 end

src/kemal/config.cr

@@ -29,6 +29,9 @@ module Kemal
       @error_handler = nil
       @always_rescue = true
       @server = uninitialized HTTP::Server
+      @router_included = false
+      @custom_handler_position = 4
+      @default_handlers_setup = false
     end
 
     def logger
@@ -43,15 +46,30 @@ module Kemal
       ssl ? "https" : "http"
     end
 
+    def clear
+      @router_included = false
+      @custom_handler_position = 4
+      @default_handlers_setup = false
+      HANDLERS.clear
+    end
+
     def handlers
       HANDLERS
     end
 
     def add_handler(handler : HTTP::Handler)
-      HANDLERS << handler
+      setup
+      HANDLERS.insert @custom_handler_position, handler
+      @custom_handler_position = @custom_handler_position + 1
+    end
+
+    def add_filter_handler(handler : HTTP::Handler)
+      setup
+      HANDLERS.insert HANDLERS.size - 1, handler
     end
 
     def add_ws_handler(handler : HTTP::WebSocketHandler)
+      setup
       HANDLERS << handler
     end
 
@@ -67,10 +85,15 @@ module Kemal
     end
 
     def setup
+      unless @default_handlers_setup && @router_included
         setup_init_handler
         setup_log_handler
         setup_error_handler
         setup_static_file_handler
+        @default_handlers_setup = true
+        @router_included = true
+        HANDLERS.insert(HANDLERS.size, Kemal::RouteHandler::INSTANCE)
+      end
     end
 
     private def setup_init_handler

src/kemal/middleware/filters.cr

@@ -7,7 +7,7 @@ module Kemal::Middleware
     # This middleware is lazily instantiated and added to the handlers as soon as a call to `after_X` or `before_X` is made.
     def initialize
       @tree = Radix::Tree(Array(Kemal::Middleware::Block)).new
-      Kemal.config.add_handler(self)
+      Kemal.config.add_filter_handler(self)
     end
 
     # The call order of the filters is before_all -> before_x -> X -> after_x -> after_all

src/kemal/route.cr

@@ -8,7 +8,9 @@ module Kemal
     @method : String
 
     def initialize(@method, @path : String, &handler : HTTP::Server::Context -> _)
-      @handler = ->(context : HTTP::Server::Context) { handler.call(context).to_s }
+      @handler = ->(context : HTTP::Server::Context) do
+        handler.call(context).to_s
+      end
     end
   end
 end