fix: make sure we respect caching being enabled/disabled

coveralls.json

@@ -0,0 +1,7 @@
+{
+  "skip_files": ["lib/application.ex"],
+  "custom_stop_words": ["defdelegate"],
+  "terminal_options": {"file_column_width": 80},
+  "coverage_options": {"treat_no_relevant_lines_as_covered": true, "minimum_coverage": 85}
+}
+

lib/request_cache.ex

@@ -16,20 +16,13 @@ defmodule RequestCache do
     end
   end
 
-  if RequestCache.Application.dependency_found?(:absinthe) do
-    @spec store(
-      result :: any,
-      opts_or_ttl :: opts | pos_integer
-    ) :: {:middleware, module, RequestCache.ResolverMiddleware.opts}
-    def store(result, ttl) when is_integer(ttl) do
-      store(result, [ttl: ttl])
-    end
-
-    def store(result, opts) when is_list(opts) do
+  if RequestCache.Application.dependency_found?(:absinthe) and
+     RequestCache.Application.dependency_found?(:absinthe_plug) do
+    def store(result, opts_or_ttl) do
       if RequestCache.Config.enabled?() do
-        {:middleware, RequestCache.ResolverMiddleware, Keyword.put(opts, :value, result)}
+        RequestCache.ResolverMiddleware.store_result(result, opts_or_ttl)
       else
-        {:ok, result}
+        result
       end
     end
 

lib/request_cache/middleware.ex

@@ -27,7 +27,7 @@ if absinthe_loaded? do
           context: Map.update!(
             resolution.context,
             RequestCache.Config.conn_private_key(),
-            &Keyword.put(&1, :request, opts)
+            &Keyword.merge(&1, [request: opts, cache_request?: true])
           )
         }
       else

lib/request_cache/plug.ex

@@ -19,6 +19,8 @@ defmodule RequestCache.Plug do
   @json_regex ~r/^(\[|\{)(.*|\n)*(\]|\})$/
   @html_regex ~r/<!DOCTYPE\s+html>/i
 
+  def request_cache_header, do: @request_cache_header
+
   @impl Plug
   def init(opts), do: opts
 
@@ -161,9 +163,21 @@ defmodule RequestCache.Plug do
   end
 
   defp enabled_for_request?(%Plug.Conn{private: private}) do
-    get_in(private, [conn_private_key(), :enabled?])
-    || get_in(private, [:absinthe, :context, conn_private_key(), :enabled?])
-    || []
+    plug_present? = get_in(private, [conn_private_key(), :enabled?]) ||
+                    get_in(private, [:absinthe, :context, conn_private_key(), :enabled?])
+
+    marked_for_cache? = get_in(private, [conn_private_key(), :cache_request?]) ||
+                        get_in(private, [:absinthe, :context, conn_private_key(), :cache_request?])
+
+    if plug_present? do
+      Util.verbose_log("[RequestCache.Plug] Plug enabled for request")
+    end
+
+    if marked_for_cache? do
+      Util.verbose_log("[RequestCache.Plug] Plug has been marked for cache")
+    end
+
+    plug_present? && marked_for_cache?
   end
 
   defp conn_request(%Plug.Conn{private: private}) do
@@ -191,7 +205,7 @@ defmodule RequestCache.Plug do
       Util.verbose_log("[RequestCache.Plug] Storing REST request in #{conn_private_key()}")
 
       Plug.Conn.put_private(conn, conn_private_key(),
-        enabled?: true,
+        cache_request?: true,
         request: opts
       )
     else
@@ -202,16 +216,7 @@ defmodule RequestCache.Plug do
   end
 
   def store_request(conn, ttl) when is_integer(ttl) do
-    if conn.private[conn_private_key()][:enabled?] do
-      Plug.Conn.put_private(conn, conn_private_key(),
-        enabled?: true,
-        request: [ttl: ttl]
-      )
-    else
-      Util.log_cache_disabled_message()
-
-      conn
-    end
+    store_request(conn, [ttl: ttl])
   end
 
   defp conn_private_key do

lib/request_cache/resolver_middleware.ex

@@ -18,7 +18,7 @@ if absinthe_loaded? do
 
     defp enable_cache_for_resolution(resolution, opts) do
       if resolution.context[RequestCache.Config.conn_private_key()][:enabled?] do
-        config = [request: Keyword.delete(opts, :value)]
+        config = [request: Keyword.delete(opts, :value), cache_request?: true]
 
         resolution = %{resolution |
           state: :resolved,
@@ -43,5 +43,17 @@ if absinthe_loaded? do
         }
       end
     end
+
+    @spec store_result(
+      result :: any,
+      opts_or_ttl :: opts | pos_integer
+    ) :: {:middleware, module, RequestCache.ResolverMiddleware.opts}
+    def store_result(result, ttl) when is_integer(ttl) do
+      store_result(result, [ttl: ttl])
+    end
+
+    def store_result(result, opts) when is_list(opts) do
+      {:middleware, RequestCache.ResolverMiddleware, Keyword.put(opts, :value, result)}
+    end
   end
 end

test/request_cache_absinthe_test.exs

@@ -16,6 +16,14 @@ defmodule RequestCacheAbsintheTest do
         end
       end
 
+      field :uncached_hello, :string do
+        resolve fn _, %{context: %{call_pid: pid}} ->
+          EnsureCalledOnlyOnce.call(pid)
+
+          {:ok, "HelloUncached"}
+        end
+      end
+
       field :hello_world, :string do
         middleware RequestCache.Middleware, ttl: :timer.seconds(100)
 
@@ -66,13 +74,34 @@ defmodule RequestCacheAbsintheTest do
   @query "query Hello { hello }"
   @query_2 "query Hello2 { helloWorld }"
   @query_error "query HelloError { helloError }"
+  @uncached_query "query HelloUncached { uncachedHello }"
 
   setup do
     {:ok, pid} = EnsureCalledOnlyOnce.start_link()
 
     %{call_pid: pid}
   end
 
+  @tag capture_log: true
+  test "does not cache queries that don't ask for caching", %{call_pid: pid} do
+    assert %Plug.Conn{} = :get
+      |> conn(graphql_url(@uncached_query))
+      |> RequestCache.Support.Utils.ensure_default_opts()
+      |> Absinthe.Plug.put_options(context: %{call_pid: pid})
+      |> Router.call([])
+
+    assert_raise Plug.Conn.WrapperError, fn ->
+      conn = :get
+      |> conn(graphql_url(@uncached_query))
+      |> RequestCache.Support.Utils.ensure_default_opts()
+      |> Absinthe.Plug.put_options(context: %{call_pid: pid})
+      |> Router.call([])
+
+      assert [] === get_resp_header(conn, RequestCache.Plug.request_cache_header())
+    end
+  end
+
+  @tag capture_log: true
   test "allows you to use middleware before a resolver to cache the results of the request", %{call_pid: pid} do
     conn = :get
       |> conn(graphql_url(@query_2))
@@ -88,6 +117,7 @@ defmodule RequestCacheAbsintheTest do
       |> Map.get(:resp_body)
   end
 
+  @tag capture_log: true
   test "allows you to use &store/2 in a resolver to cache the results of the request", %{call_pid: pid} do
     conn = :get
       |> conn(graphql_url(@query))
@@ -103,6 +133,7 @@ defmodule RequestCacheAbsintheTest do
       |> Map.get(:resp_body)
   end
 
+  @tag capture_log: true
   test "throws an error when called twice without cache", %{call_pid: pid} do
     conn = :get
       |> conn(graphql_url(@query_error))

test/request_cache_plug_test.exs

@@ -14,6 +14,12 @@ defmodule RequestCachePlugTest do
     plug :match
     plug :dispatch
 
+    match "/my_uncached_route" do
+      EnsureCalledOnlyOnce.call(conn.private[:call_pid])
+
+      send_resp(conn, 200, Jason.encode!(%{test: Enum.random(1..100_000_000)}))
+    end
+
     match "/my_route" do
       EnsureCalledOnlyOnce.call(conn.private[:call_pid])
 
@@ -96,6 +102,26 @@ defmodule RequestCachePlugTest do
     %{caller_pid: pid}
   end
 
+  @tag capture_log: true
+  test "does not cache routes that don't ask for caching", %{caller_pid: pid} do
+    assert %Plug.Conn{} = :get
+      |> conn("/my_uncached_route")
+      |> RequestCache.Support.Utils.ensure_default_opts()
+      |> put_private(:call_pid, pid)
+      |> Router.call([])
+
+    assert_raise Plug.Conn.WrapperError, fn ->
+      conn = %Plug.Conn{} = :get
+        |> conn("/my_uncached_route")
+        |> RequestCache.Support.Utils.ensure_default_opts()
+        |> put_private(:call_pid, pid)
+        |> Router.call([])
+
+      assert [] === get_resp_header(conn, RequestCache.Plug.request_cache_header())
+    end
+  end
+
+  @tag capture_log: true
   test "stops any plug from running if cache is found", %{caller_pid: pid} do
     assert %Plug.Conn{} = :get
       |> conn("/my_route")
@@ -110,6 +136,7 @@ defmodule RequestCachePlugTest do
       |> RouterWithBreakingPlug.call([])
   end
 
+  @tag capture_log: true
   test "stops any plug from running if cache using default ttl is found", %{caller_pid: pid} do
     assert %Plug.Conn{} = :get
       |> conn("/my_route_default_ttl")
@@ -133,6 +160,7 @@ defmodule RequestCachePlugTest do
     end) =~ "RequestCache requested"
   end
 
+  @tag capture_log: true
   test "includes proper headers with when served from the cache", %{
     caller_pid: pid
   } do
@@ -162,10 +190,11 @@ defmodule RequestCachePlugTest do
            ]
   end
 
+  @tag capture_log: true
   test "allows for for custom content-type header and returns it when served from the cache", %{
     caller_pid: pid
   } do
-    route = "/my_route/:param"
+    route = "/my_route/cache"
     assert %Plug.Conn{resp_headers: uncached_headers} =
              :get
              |> conn(route)