diff --git a/lib/wanderer_app/esi/api_client.ex b/lib/wanderer_app/esi/api_client.ex index 58ab7c4c..6201757c 100644 --- a/lib/wanderer_app/esi/api_client.ex +++ b/lib/wanderer_app/esi/api_client.ex @@ -13,6 +13,7 @@ defmodule WandererApp.Esi.ApiClient do @retry_opts [retry: false, retry_log_level: :warning] @timeout_opts [pool_timeout: 15_000, receive_timeout: :timer.minutes(1)] @api_retry_count 1 + @validators_ttl :timer.hours(1) @logger Application.compile_env(:wanderer_app, :logger) @@ -24,7 +25,8 @@ defmodule WandererApp.Esi.ApiClient do # Helper function to get Req options with appropriate Finch pool defp req_options_for_pool(pool) do - [base_url: "https://esi.evetech.net", finch: pool] + [base_url: "https://esi.evetech.net", finch: pool] ++ + Application.get_env(:wanderer_app, :esi_req_options, []) end def get_server_status, do: do_get("/status", [], @cache_opts) @@ -310,18 +312,17 @@ defmodule WandererApp.Esi.ApiClient do (opts |> with_refresh_token()) ++ @cache_opts ) - defp with_user_agent_opts(opts), + defp with_user_agent_opts(opts, extra_headers \\ []), do: opts |> Keyword.merge( - headers: [{:user_agent, "Wanderer/#{WandererApp.Env.vsn()} #{@wanderrer_user_agent}"}] + headers: + [{:user_agent, "Wanderer/#{WandererApp.Env.vsn()} #{@wanderrer_user_agent}"}] ++ + extra_headers ) defp with_refresh_token(opts), do: opts |> Keyword.merge(refresh_token?: true) - defp with_cache_opts(opts), - do: opts |> Keyword.merge(@cache_opts) |> Keyword.merge(cache_dir: System.tmp_dir!()) - defp do_get(path, api_opts, opts, pool \\ @general_pool) do case Cachex.get(:api_cache, path) do {:ok, cached_data} when not is_nil(cached_data) -> @@ -339,17 +340,20 @@ defmodule WandererApp.Esi.ApiClient do |> Req.get( api_opts |> Keyword.merge(url: path) - |> with_user_agent_opts() - |> with_cache_opts() + |> with_user_agent_opts(conditional_headers(path, opts)) |> Keyword.merge(@retry_opts) |> Keyword.merge(@timeout_opts) ) |> case do {:ok, %{status: 200, body: body, headers: headers}} -> + maybe_store_validators(path, body, headers, opts) maybe_cache_response(path, body, headers, opts) {:ok, body} + {:ok, %{status: 304, headers: headers}} -> + handle_not_modified(path, headers, api_opts, opts, pool) + {:ok, %{status: 504}} -> {:error, :timeout} @@ -476,10 +480,10 @@ defmodule WandererApp.Esi.ApiClient do defp maybe_cache_response(path, body, %{"expires" => [expires]} = _headers, opts) when is_binary(path) and not is_nil(expires) do try do - if opts |> Keyword.get(:cache, false) do - cached_ttl = - DateTime.diff(Timex.parse!(expires, "{RFC1123}"), DateTime.utc_now(), :millisecond) + cached_ttl = + DateTime.diff(Timex.parse!(expires, "{RFC1123}"), DateTime.utc_now(), :millisecond) + if opts |> Keyword.get(:cache, false) and cached_ttl > 0 do Cachex.put( :api_cache, path, @@ -497,6 +501,78 @@ defmodule WandererApp.Esi.ApiClient do defp maybe_cache_response(_path, _body, _headers, _opts), do: :ok + # Conditional request support (ETag / Last-Modified). + # + # On 304 ESI returns fresh cache headers (expires, etag, ...) while the body is the one we + # already have. The body is kept from the stored 200 response, but headers must be taken from + # the 304 response, otherwise a stale `expires` makes the response uncacheable and we re-fetch + # it on every tick. + defp validators_key(path), do: {:esi_validators, path} + + defp conditional_headers(path, opts) do + with true <- Keyword.get(opts, :cache, false), + false <- Keyword.get(opts, :skip_conditional, false), + {:ok, %{} = validators} <- Cachex.get(:api_cache, validators_key(path)) do + [ + {"if-none-match", validators.etag}, + {"if-modified-since", validators.last_modified} + ] + |> Enum.reject(fn {_name, value} -> is_nil(value) end) + else + _ -> [] + end + end + + defp maybe_store_validators(path, body, headers, opts) do + etag = get_header(headers, "etag") + last_modified = get_header(headers, "last-modified") + + if Keyword.get(opts, :cache, false) and not (is_nil(etag) and is_nil(last_modified)) do + Cachex.put( + :api_cache, + validators_key(path), + %{body: body, etag: etag, last_modified: last_modified}, + ttl: @validators_ttl + ) + end + + :ok + end + + defp handle_not_modified(path, headers, api_opts, opts, pool) do + case Cachex.get(:api_cache, validators_key(path)) do + {:ok, %{body: body} = validators} -> + Cachex.put( + :api_cache, + validators_key(path), + %{ + validators + | etag: get_header(headers, "etag") || validators.etag, + last_modified: get_header(headers, "last-modified") || validators.last_modified + }, + ttl: @validators_ttl + ) + + maybe_cache_response(path, body, headers, opts) + + {:ok, body} + + _ -> + if Keyword.get(opts, :skip_conditional, false) do + {:error, "Unexpected status: 304"} + else + do_get_request(path, api_opts, Keyword.put(opts, :skip_conditional, true), pool) + end + end + end + + defp get_header(headers, name) do + case Map.get(headers, name) do + [value | _] -> value + _ -> nil + end + end + defp do_post(url, opts) do try do case Req.post("#{url}", opts |> with_user_agent_opts()) do diff --git a/test/unit/esi/api_client_conditional_test.exs b/test/unit/esi/api_client_conditional_test.exs new file mode 100644 index 00000000..71fbfce5 --- /dev/null +++ b/test/unit/esi/api_client_conditional_test.exs @@ -0,0 +1,110 @@ +defmodule WandererApp.Esi.ApiClientConditionalTest do + # Uses the shared :api_cache Cachex instance and global app env + use ExUnit.Case, async: false + + alias WandererApp.Esi.ApiClient + + @path "/status" + @validators_key {:esi_validators, @path} + + setup do + previous = Application.get_env(:wanderer_app, :esi_req_options) + Application.put_env(:wanderer_app, :esi_req_options, plug: {Req.Test, ApiClient}) + + clear_cache() + + on_exit(fn -> + clear_cache() + + if previous, + do: Application.put_env(:wanderer_app, :esi_req_options, previous), + else: Application.delete_env(:wanderer_app, :esi_req_options) + end) + + :ok + end + + test "caches 200 response until expires" do + stub(fn conn -> ok_response(conn, %{"players" => 1}, "\"v1\"", future(30)) end) + + assert {:ok, %{"players" => 1}} = ApiClient.get_server_status() + assert {:ok, %{"players" => 1}} = ApiClient.get_server_status() + + assert_received {:request, _} + refute_received {:request, _} + end + + test "304 reuses stored body and extends cache with headers from the 304 response" do + stub(fn conn -> ok_response(conn, %{"players" => 1}, "\"v1\"", future(30)) end) + assert {:ok, %{"players" => 1}} = ApiClient.get_server_status() + assert_received {:request, _} + + # Simulate the fresh cache window passing + Cachex.del(:api_cache, @path) + + stub(fn conn -> + conn + |> Plug.Conn.put_resp_header("etag", "\"v1\"") + |> Plug.Conn.put_resp_header("expires", future(60)) + |> Plug.Conn.send_resp(304, "") + end) + + assert {:ok, %{"players" => 1}} = ApiClient.get_server_status() + assert_received {:request, headers} + assert {"if-none-match", "\"v1\""} in headers + + assert {:ok, ttl} = Cachex.ttl(:api_cache, @path) + assert is_integer(ttl) and ttl > 30_000 + + # Served from the extended cache, no new request + assert {:ok, %{"players" => 1}} = ApiClient.get_server_status() + refute_received {:request, _} + end + + test "304 without stored body retries unconditionally" do + stub(fn conn -> + if Plug.Conn.get_req_header(conn, "if-none-match") == [] do + ok_response(conn, %{"players" => 2}, "\"v2\"", future(30)) + else + Plug.Conn.send_resp(conn, 304, "") + end + end) + + Cachex.put(:api_cache, @validators_key, %{etag: "\"v1\"", last_modified: nil}) + + assert {:ok, %{"players" => 2}} = ApiClient.get_server_status() + end + + test "does not cache responses with expires in the past" do + stub(fn conn -> ok_response(conn, %{"players" => 1}, "\"v1\"", past(10)) end) + + assert {:ok, %{"players" => 1}} = ApiClient.get_server_status() + assert {:ok, nil} = Cachex.get(:api_cache, @path) + end + + defp stub(fun) do + test_pid = self() + + Req.Test.stub(ApiClient, fn conn -> + send(test_pid, {:request, conn.req_headers}) + fun.(conn) + end) + end + + defp ok_response(conn, body, etag, expires) do + conn + |> Plug.Conn.put_resp_header("etag", etag) + |> Plug.Conn.put_resp_header("expires", expires) + |> Req.Test.json(body) + end + + defp future(seconds), do: http_date(DateTime.add(DateTime.utc_now(), seconds, :second)) + defp past(seconds), do: http_date(DateTime.add(DateTime.utc_now(), -seconds, :second)) + + defp http_date(datetime), do: Calendar.strftime(datetime, "%a, %d %b %Y %H:%M:%S GMT") + + defp clear_cache do + Cachex.del(:api_cache, @path) + Cachex.del(:api_cache, @validators_key) + end +end