fix: make sure we respect caching being enabled/disabled (#27)

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)