From d2ef57a12808d01dd8407d84fe88b1a7b665b899 Mon Sep 17 00:00:00 2001 From: Serdar Dogruyol Date: Sat, 30 Jun 2018 16:49:04 +0300 Subject: [PATCH] Improve param parsing and remove url_params from HTTP::Request --- spec/middleware/filters_spec.cr | 52 ++++++++++++++++----------------- spec/param_parser_spec.cr | 8 ++--- spec/spec_helper.cr | 7 +++-- spec/websocket_handler_spec.cr | 6 ++-- src/kemal/ext/context.cr | 4 +-- src/kemal/ext/request.cr | 7 ----- src/kemal/param_parser.cr | 11 ++----- src/kemal/websocket_handler.cr | 1 - 8 files changed, 41 insertions(+), 55 deletions(-) delete mode 100644 src/kemal/ext/request.cr diff --git a/spec/middleware/filters_spec.cr b/spec/middleware/filters_spec.cr index 41853be..9bc2564 100644 --- a/spec/middleware/filters_spec.cr +++ b/spec/middleware/filters_spec.cr @@ -13,8 +13,8 @@ describe "Kemal::FilterHandler" do test_filter.modified.should eq("false") request = HTTP::Request.new("GET", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("true") end @@ -33,14 +33,14 @@ describe "Kemal::FilterHandler" do test_filter.modified.should eq("false") request = HTTP::Request.new("GET", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("true") request = HTTP::Request.new("POST", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("true") end @@ -61,14 +61,14 @@ describe "Kemal::FilterHandler" do test_filter.modified.should eq("false") request = HTTP::Request.new("GET", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("false") request = HTTP::Request.new("POST", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("false") end @@ -85,8 +85,8 @@ describe "Kemal::FilterHandler" do test_filter.modified.should eq("false") request = HTTP::Request.new("GET", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("true") end @@ -105,14 +105,14 @@ describe "Kemal::FilterHandler" do test_filter.modified.should eq("false") request = HTTP::Request.new("GET", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("true") request = HTTP::Request.new("POST", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("true") end @@ -132,14 +132,14 @@ describe "Kemal::FilterHandler" do test_filter.modified.should eq("false") request = HTTP::Request.new("GET", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("false") request = HTTP::Request.new("POST", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("false") end @@ -166,20 +166,20 @@ describe "Kemal::FilterHandler" do test_filter_second.modified.should eq("false") test_filter_third.modified.should eq("false") request = HTTP::Request.new("GET", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("true") request = HTTP::Request.new("POST", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("false") request = HTTP::Request.new("PUT", "/greetings") - create_request_and_return_io(filter_middleware, request) - io_with_context = create_request_and_return_io(kemal, request) + create_request_and_return_io_and_context(filter_middleware, request) + io_with_context = create_request_and_return_io_and_context(kemal, request)[0] client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) client_response.body.should eq("true") end diff --git a/spec/param_parser_spec.cr b/spec/param_parser_spec.cr index 305a0a3..d63a229 100644 --- a/spec/param_parser_spec.cr +++ b/spec/param_parser_spec.cr @@ -28,8 +28,8 @@ describe "ParamParser" do end request = HTTP::Request.new("POST", "/hello/cemal") # Radix tree MUST be run to parse url params. - create_request_and_return_io(kemal, request) - url_params = Kemal::ParamParser.new(request).url + context = create_request_and_return_io_and_context(kemal, request)[1] + url_params = Kemal::ParamParser.new(request, context.route_lookup.params).url url_params["hasan"].should eq "cemal" end @@ -43,8 +43,8 @@ describe "ParamParser" do end request = HTTP::Request.new("POST", "/hello/sam%2Bspec%40gmail.com/%2419.99/a%C3%B1o") # Radix tree MUST be run to parse url params. - create_request_and_return_io(kemal, request) - url_params = Kemal::ParamParser.new(request).url + context = create_request_and_return_io_and_context(kemal, request)[1] + url_params = Kemal::ParamParser.new(request, context.route_lookup.params).url url_params["email"].should eq "sam+spec@gmail.com" url_params["money"].should eq "$19.99" url_params["spanish"].should eq "año" diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index a4876c2..4d27003 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -29,17 +29,17 @@ end add_context_storage_type(TestContextStorageType) add_context_storage_type(AnotherContextStorageType) -def create_request_and_return_io(handler, request) +def create_request_and_return_io_and_context(handler, request) io = IO::Memory.new response = HTTP::Server::Response.new(io) context = HTTP::Server::Context.new(request, response) handler.call(context) response.close io.rewind - io + {io, context} end -def create_ws_request_and_return_io(handler, request) +def create_ws_request_and_return_io_and_context(handler, request) io = IO::Memory.new response = HTTP::Server::Response.new(io) context = HTTP::Server::Context.new(request, response) @@ -49,6 +49,7 @@ def create_ws_request_and_return_io(handler, request) # Raises because the IO::Memory is empty end io.rewind + {io, context} end def call_request_on_app(request) diff --git a/spec/websocket_handler_spec.cr b/spec/websocket_handler_spec.cr index bb867b2..bc02d3c 100644 --- a/spec/websocket_handler_spec.cr +++ b/spec/websocket_handler_spec.cr @@ -32,13 +32,13 @@ describe "Kemal::WebSocketHandler" do } request = HTTP::Request.new("GET", "/", headers) - io_with_context = create_ws_request_and_return_io(handler, request) + io_with_context = create_ws_request_and_return_io_and_context(handler, request)[0] io_with_context.to_s.should eq("HTTP/1.1 101 Switching Protocols\r\nUpgrade: websocket\r\nConnection: Upgrade\r\nSec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n\r\n\x81\u0005Match") end it "fetches named url parameters" do handler = Kemal::WebSocketHandler::INSTANCE - ws "/:id" { |_, c| c.params.url["id"] } + ws "/:id" { |_, c| c.ws_route_lookup.params["id"] } headers = HTTP::Headers{ "Upgrade" => "websocket", "Connection" => "Upgrade", @@ -46,7 +46,7 @@ describe "Kemal::WebSocketHandler" do "Sec-WebSocket-Version" => "13", } request = HTTP::Request.new("GET", "/1234", headers) - io_with_context = create_ws_request_and_return_io(handler, request) + io_with_context = create_ws_request_and_return_io_and_context(handler, request)[0] io_with_context.to_s.should eq("HTTP/1.1 101 Switching Protocols\r\nUpgrade: websocket\r\nConnection: Upgrade\r\nSec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n\r\n") end diff --git a/src/kemal/ext/context.cr b/src/kemal/ext/context.cr index 5bad0cd..9f2d3c0 100644 --- a/src/kemal/ext/context.cr +++ b/src/kemal/ext/context.cr @@ -14,9 +14,7 @@ class HTTP::Server end def params - @request.url_params ||= route_lookup.params - @params ||= Kemal::ParamParser.new(@request) - + @params ||= Kemal::ParamParser.new(@request, route_lookup.params) end def redirect(url : String, status_code : Int32 = 302) diff --git a/src/kemal/ext/request.cr b/src/kemal/ext/request.cr deleted file mode 100644 index a5791d9..0000000 --- a/src/kemal/ext/request.cr +++ /dev/null @@ -1,7 +0,0 @@ -class HTTP::Request - property url_params : Hash(String, String)? - - def content_type - @headers["Content-Type"]? - end -end diff --git a/src/kemal/param_parser.cr b/src/kemal/param_parser.cr index 65834e4..18bf281 100644 --- a/src/kemal/param_parser.cr +++ b/src/kemal/param_parser.cr @@ -11,8 +11,7 @@ module Kemal alias AllParamTypes = Nil | String | Int64 | Float64 | Bool | Hash(String, JSON::Any) | Array(JSON::Any) getter files - def initialize(@request : HTTP::Request) - @url = {} of String => String + def initialize(@request : HTTP::Request, @url : Hash(String, String) = {} of String => String) @query = HTTP::Params.new({} of String => Array(String)) @body = HTTP::Params.new({} of String => Array(String)) @json = {} of String => AllParamTypes @@ -42,7 +41,7 @@ module Kemal {% end %} private def parse_body - content_type = @request.content_type + content_type = @request.headers["Content-Type"]? return unless content_type if content_type.try(&.starts_with?(URL_ENCODED_FORM)) @body = parse_part(@request.body) @@ -59,11 +58,7 @@ module Kemal end private def parse_url - if params = @request.url_params - params.each do |key, value| - @url[key] = unescape_url_param(value) - end - end + @url.each { |key, value| @url[key] = unescape_url_param(value) } end private def parse_file_upload diff --git a/src/kemal/websocket_handler.cr b/src/kemal/websocket_handler.cr index dd31bf8..9550bef 100644 --- a/src/kemal/websocket_handler.cr +++ b/src/kemal/websocket_handler.cr @@ -11,7 +11,6 @@ module Kemal def call(context : HTTP::Server::Context) return call_next(context) unless context.ws_route_defined? && websocket_upgrade_request?(context) - context.request.url_params ||= context.ws_route_lookup.params content = context.websocket.call(context) context.response.print(content) context