Merge pull request #465 from kemalcr/rework-param-parser

Rework param parser

CHANGELOG.md

@@ -1,10 +1,10 @@
 # Next
 
-- `env.params.files` is now an `Array(FileUpload)`. You can iterate over to access the images.
+- *[breaking change]* Removed `env.params.files`. You can use Crystal's built-in `HTTP::FormData.parse` instead
 
 ```ruby
-env.params.files.each do |file|
+post "/upload" do |env|
-
+  HTTP::FormData.parse(env.request) do |upload|
     filename = file.filename
 
     if !filename.is_a?(String)
@@ -15,9 +15,20 @@ env.params.files.each do |file|
       IO.copy(file.tmpfile, f)
     end
     "Upload OK"
+  end
 end
 ```
 
+- *[breaking change]* From now on to access dynamic url params in a WebSocket route you have to use
+
+```ruby
+ws "/:id" do |socket, context|
+  id = context.ws_route_lookup.params["id"]
+end
+```
+
+- *[breaking change]* Removed `_method` magic param
+
 # 0.23.0 (17-06-2018)
 
 - Crystal 0.25.0 support 🎉

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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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)
+    create_request_and_return_io_and_context(filter_middleware, request)
-    io_with_context = create_request_and_return_io(kemal, 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

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)
+    context = create_request_and_return_io_and_context(kemal, request)[1]
-    url_params = Kemal::ParamParser.new(request).url
+    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)
+    context = create_request_and_return_io_and_context(kemal, request)[1]
-    url_params = Kemal::ParamParser.new(request).url
+    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"

spec/route_handler_spec.cr

@@ -102,48 +102,6 @@ describe "Kemal::RouteHandler" do
     client_response.body.should eq("Skills ruby,crystal")
   end
 
-  it "checks for _method param in POST request to simulate PUT" do
-    put "/" do
-      "Hello World from PUT"
-    end
-    request = HTTP::Request.new(
-      "POST",
-      "/",
-      body: "_method=PUT",
-      headers: HTTP::Headers{"Content-Type" => "application/x-www-form-urlencoded"}
-    )
-    client_response = call_request_on_app(request)
-    client_response.body.should eq("Hello World from PUT")
-  end
-
-  it "checks for _method param in POST request to simulate PATCH" do
-    patch "/" do
-      "Hello World from PATCH"
-    end
-    request = HTTP::Request.new(
-      "POST",
-      "/",
-      body: "_method=PATCH",
-      headers: HTTP::Headers{"Content-Type" => "application/x-www-form-urlencoded; charset=UTF-8"}
-    )
-    client_response = call_request_on_app(request)
-    client_response.body.should eq("Hello World from PATCH")
-  end
-
-  it "checks for _method param in POST request to simulate DELETE" do
-    delete "/" do
-      "Hello World from DELETE"
-    end
-    request = HTTP::Request.new(
-      "POST",
-      "/",
-      body: "_method=DELETE",
-      headers: HTTP::Headers{"Content-Type" => "application/x-www-form-urlencoded; charset=UTF-8"}
-    )
-    client_response = call_request_on_app(request)
-    client_response.body.should eq("Hello World from DELETE")
-  end
-
   it "can process HTTP HEAD requests for defined GET routes" do
     get "/" do
       "Hello World from GET"

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)

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
 

src/kemal/ext/context.cr

@@ -14,12 +14,7 @@ class HTTP::Server
     end
 
     def params
-      @request.url_params ||= route_lookup.params
+      @params ||= Kemal::ParamParser.new(@request, route_lookup.params)
-      @params ||= if @request.param_parser
-                    @request.param_parser.not_nil!
-                  else
-                    Kemal::ParamParser.new(@request)
-                  end
     end
 
     def redirect(url : String, status_code : Int32 = 302)
@@ -36,7 +31,7 @@ class HTTP::Server
     end
 
     def route_lookup
-      Kemal::RouteHandler::INSTANCE.lookup_route(@request.override_method.as(String), @request.path)
+      Kemal::RouteHandler::INSTANCE.lookup_route(@request.method.as(String), @request.path)
     end
 
     def route_defined?

src/kemal/ext/request.cr

@@ -1,32 +0,0 @@
-class HTTP::Request
-  property override_method
-  property url_params : Hash(String, String)?
-  getter param_parser : Kemal::ParamParser?
-
-  def override_method
-    @override_method ||= check_for_method_override!
-  end
-
-  def content_type
-    @headers["Content-Type"]?
-  end
-
-  # Checks if method contained in _method param is valid one
-  def self.override_method_valid?(override_method : String)
-    override_method = override_method.upcase
-    override_method == "PUT" || override_method == "PATCH" || override_method == "DELETE"
-  end
-
-  # Checks if request params contain _method param to override request incoming method
-  private def check_for_method_override!
-    @override_method = @method
-    if @method == "POST"
-      @param_parser = Kemal::ParamParser.new(self)
-      params = @param_parser.not_nil!.body
-      if params.has_key?("_method") && HTTP::Request.override_method_valid?(params["_method"])
-        @override_method = params["_method"]
-      end
-    end
-    @override_method
-  end
-end

src/kemal/file_upload.cr

@@ -1,25 +0,0 @@
-module Kemal
-  # :nodoc:
-  struct FileUpload
-    getter tmpfile : Tempfile
-    getter filename : String?
-    getter headers : HTTP::Headers
-    getter creation_time : Time?
-    getter modification_time : Time?
-    getter read_time : Time?
-    getter size : UInt64?
-
-    def initialize(upload)
-      @tmpfile = Tempfile.new(filename)
-      ::File.open(@tmpfile.path, "w") do |file|
-        IO.copy(upload.body, file)
-      end
-      @filename = upload.filename
-      @headers = upload.headers
-      @creation_time = upload.creation_time
-      @modification_time = upload.modification_time
-      @read_time = upload.read_time
-      @size = upload.size
-    end
-  end
-end

src/kemal/filter_handler.cr

@@ -14,12 +14,12 @@ module Kemal
     def call(context : HTTP::Server::Context)
       return call_next(context) unless context.route_defined?
       call_block_for_path_type("ALL", context.request.path, :before, context)
-      call_block_for_path_type(context.request.override_method, context.request.path, :before, context)
+      call_block_for_path_type(context.request.method, context.request.path, :before, context)
       if Kemal.config.error_handlers.has_key?(context.response.status_code)
         raise Kemal::Exceptions::CustomException.new(context)
       end
       call_next(context)
-      call_block_for_path_type(context.request.override_method, context.request.path, :after, context)
+      call_block_for_path_type(context.request.method, context.request.path, :after, context)
       call_block_for_path_type("ALL", context.request.path, :after, context)
       context
     end

src/kemal/helpers/exceptions.cr

@@ -8,7 +8,7 @@ module Kemal::Exceptions
 
   class RouteNotFound < Exception
     def initialize(context : HTTP::Server::Context)
-      super "Requested path: '#{context.request.override_method}:#{context.request.path}' was not found."
+      super "Requested path: '#{context.request.method}:#{context.request.path}' was not found."
     end
   end
 

src/kemal/param_parser.cr

@@ -9,14 +9,11 @@ module Kemal
     PARTS            = %w(url query body json)
     # :nodoc:
     alias AllParamTypes = Nil | String | Int64 | Float64 | Bool | Hash(String, JSON::Any) | Array(JSON::Any)
-    getter files
 
-    def initialize(@request : HTTP::Request)
+    def initialize(@request : HTTP::Request, @url : Hash(String, String) = {} of String => String)
-      @url = {} of String => String
       @query = HTTP::Params.new({} of String => Array(String))
       @body = HTTP::Params.new({} of String => Array(String))
       @json = {} of String => AllParamTypes
-      @files = [] of FileUpload
       @url_parsed = false
       @query_parsed = false
       @body_parsed = false
@@ -42,16 +39,12 @@ 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)
         return
       end
-      if content_type.try(&.starts_with?(MULTIPART_FORM))
-        parse_file_upload
-        return
-      end
     end
 
     private def parse_query
@@ -59,23 +52,7 @@ module Kemal
     end
 
     private def parse_url
-      if params = @request.url_params
+      @url.each { |key, value| @url[key] = unescape_url_param(value) }
-        params.each do |key, value|
-          @url[key] = unescape_url_param(value)
-        end
-      end
-    end
-
-    private def parse_file_upload
-      HTTP::FormData.parse(@request) do |upload|
-        next unless upload
-        filename = upload.filename
-        if !filename.nil?
-          @files << FileUpload.new(upload: upload)
-        else
-          @body.add(upload.name, upload.body.gets_to_end)
-        end
-      end
     end
 
     # Parses JSON request body if Content-Type is `application/json`.

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