From 0b4a36953f831da71ba91dc6d8829370705efb95 Mon Sep 17 00:00:00 2001 From: Sdogruyol Date: Tue, 12 Jan 2016 21:37:12 +0200 Subject: [PATCH 1/4] Started reimplementing router --- spec/route_spec.cr | 36 ++++++++++++++++++------------------ src/kemal/context.cr | 8 ++++++-- src/kemal/handler.cr | 9 ++++----- src/kemal/param_parser.cr | 18 +++++------------- src/kemal/request.cr | 1 + src/kemal/route.cr | 35 ++++++----------------------------- 6 files changed, 40 insertions(+), 67 deletions(-) diff --git a/spec/route_spec.cr b/spec/route_spec.cr index f22220e..96101ab 100644 --- a/spec/route_spec.cr +++ b/spec/route_spec.cr @@ -2,24 +2,24 @@ require "./spec_helper" describe "Route" do describe "match?" do - it "doesn't match because of route" do - route = Route.new("GET", "/foo/bar") { "" } - request = HTTP::Request.new("GET", "/world?message=coco") - route.match?(request).should be_nil - end - - it "doesn't match because of method" do - route = Route.new("GET", "/foo/bar") { "" } - request = HTTP::Request.new("POST", "/foo/bar") - route.match?(request).should be_nil - end - - it "matches" do - route = Route.new("GET", "/foo/:one/path/:two") { "" } - request = HTTP::Request.new("GET", "/foo/uno/path/dos") - match = route.match?(request) - match.should eq true - end + # it "doesn't match because of route" do + # route = Route.new("GET", "/foo/bar") { "" } + # request = HTTP::Request.new("GET", "/world?message=coco") + # route.match?(request).should be_nil + # end + # + # it "doesn't match because of method" do + # route = Route.new("GET", "/foo/bar") { "" } + # request = HTTP::Request.new("POST", "/foo/bar") + # route.match?(request).should be_nil + # end + # + # it "matches" do + # route = Route.new("GET", "/foo/:one/path/:two") { "" } + # request = HTTP::Request.new("GET", "/foo/uno/path/dos") + # match = route.match?(request) + # match.should eq true + # end it "matches the correct route" do kemal = Kemal::Handler.new diff --git a/src/kemal/context.cr b/src/kemal/context.cr index 37192d2..f73c738 100644 --- a/src/kemal/context.cr +++ b/src/kemal/context.cr @@ -3,10 +3,10 @@ class Kemal::Context getter request getter response - getter params + getter route getter content_type - def initialize(@request, @params) + def initialize(@request, @route) @response = Kemal::Response.new end @@ -22,6 +22,10 @@ class Kemal::Context @response.content_type end + def params + Kemal::ParamParser.new(@route, @request).parse + end + delegate headers, @request delegate status_code, :"status_code=", :"content_type=", @response end diff --git a/src/kemal/handler.cr b/src/kemal/handler.cr index 0ac9f90..4b4e813 100644 --- a/src/kemal/handler.cr +++ b/src/kemal/handler.cr @@ -1,5 +1,4 @@ require "http/server" -require "uri" # Kemal::Handler is the main handler which handles all the HTTP requests. Routing, parsing, rendering e.g # are done in this handler. @@ -24,11 +23,11 @@ class Kemal::Handler < HTTP::Handler end def process_request(request) + url = request.path.not_nil! @routes.each do |route| - match = route.match?(request) - if match - params = Kemal::ParamParser.new(route, request).parse - context = Context.new(request, params) + url.match(route.pattern as Regex) do |url_params| + request.url_params = url_params + context = Context.new(request, route) begin body = route.handler.call(context).to_s return HTTP::Response.new(context.status_code, body, context.response_headers) diff --git a/src/kemal/param_parser.cr b/src/kemal/param_parser.cr index 26a013b..69fe879 100644 --- a/src/kemal/param_parser.cr +++ b/src/kemal/param_parser.cr @@ -11,17 +11,15 @@ class Kemal::ParamParser APPLICATION_JSON = "application/json" def initialize(@route, @request) - @route_components = route.components - @request_components = request.path.not_nil!.split "/" @params = {} of String => AllParamTypes end def parse - parse_components parse_request end def parse_request + parse_url_params parse_query parse_body parse_json @@ -37,6 +35,10 @@ class Kemal::ParamParser parse_part(@request.query) end + def parse_url_params + parse_part(@request.url_params.to_s) + end + # Parses JSON request body if Content-Type is `application/json`. # If request body is a JSON Hash then all the params are parsed and added into `params`. # If request body is a JSON Array it's added into `params` as `_json` and can be accessed @@ -45,7 +47,6 @@ class Kemal::ParamParser return unless @request.body && @request.headers["Content-Type"]? == APPLICATION_JSON body = @request.body as String - case json = JSON.parse(body).raw when Hash json.each do |k, v| @@ -63,13 +64,4 @@ class Kemal::ParamParser end end - def parse_components - @route_components.zip(@request_components) do |route_component, req_component| - if route_component.starts_with? ':' - @params[route_component[1..-1]] = req_component - else - return nil unless route_component == req_component - end - end - end end diff --git a/src/kemal/request.cr b/src/kemal/request.cr index 9dda339..16ff382 100644 --- a/src/kemal/request.cr +++ b/src/kemal/request.cr @@ -1,4 +1,5 @@ # Opening HTTP::Request to add override_method property class HTTP::Request property override_method + property url_params end diff --git a/src/kemal/route.cr b/src/kemal/route.cr index 7b84584..4a0437d 100644 --- a/src/kemal/route.cr +++ b/src/kemal/route.cr @@ -3,40 +3,17 @@ # what action to be done if the route is matched. class Kemal::Route getter handler - getter components + getter pattern def initialize(@method, path, &@handler : Kemal::Context -> _) - @components = path.split "/" + @pattern = pattern_to_regex path end - def match?(request) - check_for_method_override!(request) - return nil unless request.override_method == @method - components = request.path.not_nil!.split "/" - return nil unless components.size == @components.size - @components.zip(components) do |route_component, req_component| - unless route_component.starts_with? ':' - return nil unless route_component == req_component - end + private def pattern_to_regex(pattern) + pattern = pattern.gsub(/\:(?\w+)/) do |_, match| + "(?<#{match["param"]}>.*)" end - true + Regex.new "^#{pattern}/?$" end - # Checks if request params contain _method param to override request incoming method - def check_for_method_override!(request) - request.override_method = request.method - if request.method == "POST" - params = Kemal::ParamParser.new(self, request).parse_request - if params.has_key?("_method") && self.override_method_valid?(params["_method"]) - request.override_method = params["_method"] - end - end - end - - # Checks if method contained in _method param is valid one - def override_method_valid?(override_method) - return false unless override_method.is_a?(String) - override_method = override_method.upcase - return (override_method == "PUT" || override_method == "PATCH" || override_method == "DELETE") - end end From 3e8b2ae51c1c5dd3d2d074e0b09aff407c0e8d95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fatih=20Kadir=20Ak=C4=B1n?= Date: Wed, 13 Jan 2016 00:06:19 +0200 Subject: [PATCH 2/4] Implement parse_url_params --- src/kemal/param_parser.cr | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/kemal/param_parser.cr b/src/kemal/param_parser.cr index 69fe879..43ae3cc 100644 --- a/src/kemal/param_parser.cr +++ b/src/kemal/param_parser.cr @@ -36,7 +36,18 @@ class Kemal::ParamParser end def parse_url_params - parse_part(@request.url_params.to_s) + url_params = @request.url_params + begin + url_params = url_params.not_nil! + name_table = url_params.regex.name_table + size = url_params.size + size.times do |i| + name = name_table.fetch(i + 1) { i + 1 } + value = url_params[i + 1] + @params[name as String] = value + end + rescue + end end # Parses JSON request body if Content-Type is `application/json`. From 4c423e967f5a53e70c1e3c46a69c861a7b870ca2 Mon Sep 17 00:00:00 2001 From: sdogruyol Date: Wed, 13 Jan 2016 10:07:37 +0200 Subject: [PATCH 3/4] Refactor parse_url_params --- src/kemal/param_parser.cr | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/kemal/param_parser.cr b/src/kemal/param_parser.cr index 43ae3cc..c74ea70 100644 --- a/src/kemal/param_parser.cr +++ b/src/kemal/param_parser.cr @@ -35,18 +35,15 @@ class Kemal::ParamParser parse_part(@request.query) end + # Ditto: This needs memoization without the huge AllParamTypes union :| def parse_url_params - url_params = @request.url_params - begin - url_params = url_params.not_nil! + if @request.url_params + url_params = @request.url_params.not_nil! name_table = url_params.regex.name_table - size = url_params.size - size.times do |i| - name = name_table.fetch(i + 1) { i + 1 } - value = url_params[i + 1] - @params[name as String] = value + url_params.size.times do |i| + name = (name_table.fetch(i + 1) { i + 1 }) as String + @params[name] = url_params[i + 1] end - rescue end end From efcf5915819f639443eb753c40ca23230481cc5d Mon Sep 17 00:00:00 2001 From: sdogruyol Date: Wed, 13 Jan 2016 10:09:12 +0200 Subject: [PATCH 4/4] Remove some unnecessary specs --- spec/route_spec.cr | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/spec/route_spec.cr b/spec/route_spec.cr index 96101ab..818eee1 100644 --- a/spec/route_spec.cr +++ b/spec/route_spec.cr @@ -2,25 +2,6 @@ require "./spec_helper" describe "Route" do describe "match?" do - # it "doesn't match because of route" do - # route = Route.new("GET", "/foo/bar") { "" } - # request = HTTP::Request.new("GET", "/world?message=coco") - # route.match?(request).should be_nil - # end - # - # it "doesn't match because of method" do - # route = Route.new("GET", "/foo/bar") { "" } - # request = HTTP::Request.new("POST", "/foo/bar") - # route.match?(request).should be_nil - # end - # - # it "matches" do - # route = Route.new("GET", "/foo/:one/path/:two") { "" } - # request = HTTP::Request.new("GET", "/foo/uno/path/dos") - # match = route.match?(request) - # match.should eq true - # end - it "matches the correct route" do kemal = Kemal::Handler.new kemal.add_route "GET", "/route1" do |env|