From 46e39b767938bc3ba1e833dd2bdbb34ca10e3706 Mon Sep 17 00:00:00 2001
From: Jeremy Woertink <jeremywoertink@gmail.com>
Date: Fri, 28 May 2021 14:28:13 -0700
Subject: [PATCH] Moving params in to HTTP::Server::Context so it's available
 anywhere context is, and ensures we only parse once per request. Fixes #1473

---
 spec/lucky/params_spec.cr                 | 13 +++++++++++++
 src/lucky/action.cr                       |  1 +
 src/lucky/context_extensions.cr           |  6 ++++++
 src/lucky/http_method_override_handler.cr |  4 ++--
 src/lucky/param_helpers.cr                |  6 ++----
 src/lucky/params.cr                       | 10 ++++------
 6 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/spec/lucky/params_spec.cr b/spec/lucky/params_spec.cr
index 24c8c0b51..f19780f91 100644
--- a/spec/lucky/params_spec.cr
+++ b/spec/lucky/params_spec.cr
@@ -683,4 +683,17 @@ describe Lucky::Params do
       params.should eq({"filter" => {"name" => "euphonium"}, "page" => "1", "per" => "50"})
     end
   end
+
+  describe "Setting route_params later" do
+    it "returns the correct values for get?" do
+      request = build_request body: "", content_type: ""
+      route_params = {"id" => "from_route"}
+      params = Lucky::Params.new(request)
+
+      params.get?(:id).should eq nil
+
+      params.route_params = route_params
+      params.get?(:id).should eq "from_route"
+    end
+  end
 end
diff --git a/src/lucky/action.cr b/src/lucky/action.cr
index 8de6ce5a5..659641d2c 100644
--- a/src/lucky/action.cr
+++ b/src/lucky/action.cr
@@ -4,6 +4,7 @@ abstract class Lucky::Action
   getter :context, :route_params
 
   def initialize(@context : HTTP::Server::Context, @route_params : Hash(String, String))
+    context.params.route_params = @route_params
   end
 
   abstract def call
diff --git a/src/lucky/context_extensions.cr b/src/lucky/context_extensions.cr
index 5835dc322..73a2a68cf 100644
--- a/src/lucky/context_extensions.cr
+++ b/src/lucky/context_extensions.cr
@@ -24,4 +24,10 @@ class HTTP::Server::Context
   def flash : Lucky::FlashStore
     @_flash ||= Lucky::FlashStore.from_session(session)
   end
+
+  @_params : Lucky::Params?
+
+  def params : Lucky::Params
+    @_params ||= Lucky::Params.new(request)
+  end
 end
diff --git a/src/lucky/http_method_override_handler.cr b/src/lucky/http_method_override_handler.cr
index c9377215a..d3bd46c88 100644
--- a/src/lucky/http_method_override_handler.cr
+++ b/src/lucky/http_method_override_handler.cr
@@ -12,11 +12,11 @@ class Lucky::HttpMethodOverrideHandler
   end
 
   private def override_allowed?(context, http_method) : Bool
-    ["POST"].includes?(context.request.method) && ["PATCH", "PUT", "DELETE"].includes?(http_method)
+    (context.request.method == "POST") && ["PATCH", "PUT", "DELETE"].includes?(http_method)
   end
 
   private def overridden_http_method(context) : String?
-    Lucky::Params.new(context.request).get?(:_method).try(&.upcase)
+    context.params.get?(:_method).try(&.upcase)
   rescue Lucky::ParamParsingError
     nil
   end
diff --git a/src/lucky/param_helpers.cr b/src/lucky/param_helpers.cr
index 63d6988a2..52a2bb866 100644
--- a/src/lucky/param_helpers.cr
+++ b/src/lucky/param_helpers.cr
@@ -1,7 +1,5 @@
 module Lucky::ParamHelpers
-  @_params : Lucky::Params?
-
-  def params : Lucky::Params
-    @_params ||= Lucky::Params.new(context.request, @route_params)
+  memoize def params : Lucky::Params
+    context.params
   end
 end
diff --git a/src/lucky/params.cr b/src/lucky/params.cr
index 263af081c..cb2336a76 100644
--- a/src/lucky/params.cr
+++ b/src/lucky/params.cr
@@ -1,11 +1,9 @@
 class Lucky::Params
-  @request : HTTP::Request
-  @route_params : Hash(String, String) = {} of String => String
-
   # :nodoc:
-  private getter :request
+  private getter request : HTTP::Request
   # :nodoc:
-  private getter :route_params
+  private getter route_params : Hash(String, String)
+  setter :route_params
 
   # Create a new params object
   #
@@ -20,7 +18,7 @@ class Lucky::Params
   #
   # Lucky::Params.new(request, route_params)
   # ```
-  def initialize(@request, @route_params = {} of String => String)
+  def initialize(@request : HTTP::Request, @route_params : Hash(String, String) = empty_params)
   end
 
   # Parses the request body as `JSON::Any` or raises `Lucky::ParamParsingError` if JSON is invalid.