feat: add caching helper stale?

similar to the same method in rails

.ameba.yml

@@ -1,2 +1,4 @@
 Metrics/CyclomaticComplexity:
   Enabled: false
+Lint/UselessAssign:
+  Enabled: false # TODO: Enable once https://github.com/crystal-ameba/ameba/issues/447 is resolved

shard.yml

@@ -1,17 +1,17 @@
 name: action-controller
-version: 7.3.1
+version: 7.4.0
 crystal: ">= 1.9.0"
 
 dependencies:
   habitat:
     github: luckyframework/habitat
-    version: ~> 0.4
+    version: ">= 0.4"
   lucky_router:
     github: luckyframework/lucky_router
-    version: ~> 0.5
+    version: ">= 0.5"
   exception_page:
     github: crystal-loot/exception_page
-    version: ~> 0.4
+    version: ">= 0.4"
 
   # Required for running specs in client apps
   hot_topic:

spec/cache_spec.cr

@@ -0,0 +1,92 @@
+require "./spec_helper"
+
+describe ActionController::Base do
+  it "should return 304 when using If-Modified-Since headers" do
+    client = AC::SpecHelper.client
+
+    last_modified = 5.minutes.ago
+    result = client.get("/caching?last_modified=#{last_modified.to_unix}", headers: HTTP::Headers{
+      "If-Modified-Since" => HTTP.format_time(last_modified),
+    })
+    result.status_code.should eq 304
+    result.body.should eq ""
+    result.headers["Last-Modified"]?.should eq HTTP.format_time(last_modified)
+
+    result = client.get("/caching?last_modified=#{last_modified.to_unix}", headers: HTTP::Headers{
+      "If-Modified-Since" => HTTP.format_time(4.minutes.ago),
+    })
+    result.status_code.should eq 304
+    result.body.should eq ""
+    result.headers["Last-Modified"]?.should eq HTTP.format_time(last_modified)
+
+    result = client.get("/caching?last_modified=#{last_modified.to_unix}", headers: HTTP::Headers{
+      "If-Modified-Since" => HTTP.format_time(6.minutes.ago),
+    })
+    result.status_code.should eq 200
+    result.body.should eq %("response-data")
+    result.headers["Last-Modified"]?.should eq HTTP.format_time(last_modified)
+  end
+
+  it "should return 304 when using If-None-Match headers" do
+    client = AC::SpecHelper.client
+    result = client.get("/caching", headers: HTTP::Headers{
+      "If-None-Match" => %("12345"),
+    })
+    result.status_code.should eq 304
+    result.body.should eq ""
+    result.headers["ETag"]?.should eq %("12345")
+
+    result = client.get("/caching", headers: HTTP::Headers{
+      "If-None-Match" => "*",
+    })
+    result.status_code.should eq 304
+    result.body.should eq ""
+    result.headers["ETag"]?.should eq %("12345")
+
+    result = client.get("/caching", headers: HTTP::Headers{
+      "If-None-Match" => %("11111", "12345"),
+    })
+    result.status_code.should eq 304
+    result.body.should eq ""
+    result.headers["ETag"]?.should eq %("12345")
+
+    result = client.get("/caching", headers: HTTP::Headers{
+      "If-None-Match" => %("11111"),
+    })
+    result.status_code.should eq 200
+    result.body.should eq %("response-data")
+    result.headers["ETag"]?.should eq %("12345")
+  end
+
+  it "should return 304 when using If-Modified-Since and If-None-Match headers" do
+    client = AC::SpecHelper.client
+
+    last_modified = 5.minutes.ago
+    result = client.get("/caching?last_modified=#{last_modified.to_unix}", headers: HTTP::Headers{
+      "If-Modified-Since" => HTTP.format_time(last_modified),
+      "If-None-Match"     => %("12345"),
+    })
+    result.status_code.should eq 304
+    result.body.should eq ""
+    result.headers["Last-Modified"]?.should eq HTTP.format_time(last_modified)
+    result.headers["ETag"]?.should eq %("12345")
+
+    result = client.get("/caching?last_modified=#{last_modified.to_unix}", headers: HTTP::Headers{
+      "If-Modified-Since" => HTTP.format_time(last_modified),
+      "If-None-Match"     => %("11111"),
+    })
+    result.status_code.should eq 200
+    result.body.should eq %("response-data")
+    result.headers["Last-Modified"]?.should eq HTTP.format_time(last_modified)
+    result.headers["ETag"]?.should eq %("12345")
+
+    result = client.get("/caching?last_modified=#{last_modified.to_unix}", headers: HTTP::Headers{
+      "If-Modified-Since" => HTTP.format_time(6.minutes.ago),
+      "If-None-Match"     => %("12345"),
+    })
+    result.status_code.should eq 200
+    result.body.should eq %("response-data")
+    result.headers["Last-Modified"]?.should eq HTTP.format_time(last_modified)
+    result.headers["ETag"]?.should eq %("12345")
+  end
+end

spec/open_api_spec.cr

@@ -9,7 +9,7 @@ describe ActionController::OpenAPI do
   it "generates openapi docs" do
     result = ActionController::OpenAPI.generate_open_api_docs("title", "version", description: "desc")
     result[:openapi].should eq "3.0.3"
-    result[:paths].size.should eq 22
+    result[:paths].size.should eq 23
     result[:info][:description].should eq "desc"
   end
 end

spec/spec_helper.cr

@@ -8,6 +8,20 @@ Spec.before_suite do
   ::Log.setup "*", :debug, Log::IOBackend.new(formatter: ActionController.default_formatter)
 end
 
+class Caching < ActionController::Base
+  base "/caching"
+
+  @[AC::Route::GET("/")]
+  def index(last_modified : Int64 = Time.utc.to_unix, public : Bool = false) : String?
+    last_modified = Time.unix last_modified
+    etag = %("12345")
+
+    if stale? last_modified, etag, public
+      "response-data"
+    end
+  end
+end
+
 abstract class FilterOrdering < ActionController::Base
   @trusted = false
   @in_around = false
@@ -205,7 +219,7 @@ class TemplateOne < ActionController::Base
   layout "layout_main.ecr"
 
   get "/", :index do
-    data = client_ip # ameba:disable Lint/UselessAssign
+    data = client_ip
     if params["inline"]?
       render html: template("inner.ecr")
     else
@@ -214,7 +228,7 @@ class TemplateOne < ActionController::Base
   end
 
   get "/:id", :show do
-    data = params["id"] # ameba:disable Lint/UselessAssign
+    data = params["id"]
     if params["inline"]?
       render html: partial("inner.ecr")
     else
@@ -232,7 +246,7 @@ class TemplateTwo < TemplateOne
   layout "layout_alt.ecr"
 
   get "/", :index do
-    data = 50 # ameba:disable Lint/UselessAssign
+    data = 50
     render template: "inner.ecr"
   end
 end

src/action-controller/base.cr

@@ -309,7 +309,7 @@ abstract class ActionController::Base
       {% for klass, details in RESCUE %}
         {% block = details[1] %}
         {% if block != nil %}
-          def {{details[0]}}({{*details[1].args}})
+          def {{details[0]}}({{details[1].args.splat}})
             {{details[1].body}}
           end
         {% end %}
@@ -668,7 +668,7 @@ abstract class ActionController::Base
         \{% end %}
       \{% end %}
       \{{ ("def " + function.id.stringify + "(").id }}
-        \{{*block.args}}
+        \{{block.args.splat}}
       \{{ ")".id }}
         \{{block.body}}
       \{{ "end".id }}
@@ -687,7 +687,7 @@ abstract class ActionController::Base
         {{ ann.id }}
       {% end %}
     {% end %}
-    def {{function.id}}({{*block.args}})
+    def {{function.id}}({{block.args.splat}})
       {{block.body}}
     end
   end
@@ -830,4 +830,49 @@ abstract class ActionController::Base
     @__client_ip__ = cip
     cip
   end
+
+  # Sets the etag and/or last_modified on the response and checks it against the client request.
+  # If it’s fresh and we don’t need to generate anything, a reply of 304 Not Modified is sent.
+  def stale?(last_modified : Time? = nil, etag : String? = nil, public : Bool = false)
+    resp_headers = response.headers
+    req_headers = request.headers
+
+    # configure and extract headers
+    if etag
+      resp_headers["ETag"] = etag
+      if none_match = req_headers["If-None-Match"]?
+        req_etags = none_match.split(",").map(&.strip)
+      end
+    end
+
+    if last_modified
+      resp_headers["Last-Modified"] = HTTP.format_time(last_modified)
+      if req_modified = req_headers["If-Modified-Since"]?
+        modified_since = HTTP.parse_time(req_modified)
+      end
+    end
+
+    if public
+      cache_control = (resp_headers["Cache-Control"]? || "").split(",").map(&.strip)
+      cache_control.delete("private")
+      cache_control.delete("no-cache")
+      cache_control << "public"
+      resp_headers["Cache-Control"] = cache_control.join(", ")
+    end
+
+    # check values
+    fresh = if req_etags && modified_since
+              etag_matches = req_etags.includes?(etag) || req_etags.includes?("*")
+              etag_matches && (modified_since >= last_modified)
+            elsif modified_since
+              modified_since >= last_modified
+            elsif req_etags
+              req_etags.includes?(etag) || req_etags.includes?("*")
+            else
+              false
+            end
+
+    head(:not_modified) if fresh
+    !fresh
+  end
 end

src/action-controller/responders.cr

@@ -108,7 +108,7 @@ module ActionController::Responders
     %ret_val = {{ json || yaml || xml || html || text || binary || template || partial }}
 
     {% if status.is_a?(SymbolLiteral) %}
-      %response.status_code = {{STATUS_CODES[status]}}
+      %response.status_code = {{STATUS_CODES[status] || REDIRECTION_CODES[status]}}
     {% else %}
       %response.status_code = ({{status}}).to_i
     {% end %}

src/action-controller/router/builder.cr

@@ -168,7 +168,7 @@ module ActionController::Route::Builder
   macro __build_transformer_functions__
       {% for type, block in RESPONDERS %}
         # :nodoc:
-        def self.transform_{{type.gsub(/\W/, "_").id}}(%instance, {{*block.args}}, *_args)
+        def self.transform_{{type.gsub(/\W/, "_").id}}(%instance, {{block.args.splat}}, *_args)
           __yield__(%instance) do
             {{block.body}}
           end
@@ -197,7 +197,7 @@ module ActionController::Route::Builder
 
       {% for type, block in PARSERS %}
         # :nodoc:
-        def self.parse_{{type.gsub(/\W/, "_").id}}({{*block.args}}, **_ignore)
+        def self.parse_{{type.gsub(/\W/, "_").id}}({{block.args.splat}}, **_ignore)
           {{block.body}}
         end
       {% end %}

src/action-controller/session/message_verifier.cr

@@ -23,9 +23,9 @@ class ActionController::MessageVerifier
     if digest && valid_message?(data, digest)
       String.new(decode(data))
     end
-  rescue argument_error : ArgumentError
-    return if argument_error.message =~ %r{invalid base64}
-    raise argument_error
+  rescue error : ArgumentError
+    return if error.message =~ %r{invalid base64}
+    raise error
   end
 
   def verify(signed_message) : String

src/action-controller/spec_helper.cr

@@ -102,10 +102,10 @@ class HotTopic::Client(T) < HTTP::Client
       unless handshake_response.headers["Sec-WebSocket-Accept"]? == challenge_response
         raise Socket::Error.new("Handshake got denied. Server did not verify WebSocket challenge.")
       end
-    rescue exc
+    rescue error
       local_io.close
       remote_io.close
-      raise exc
+      raise error
     end
 
     HTTP::WebSocket.new HTTP::WebSocket::Protocol.new(local_io, masked: true)