Refactor default Req client (#25)

This commit refactors the existing default Req HTTP client to provide
full support for all options documented in https://hexdocs.pm/req/Req.html#new/1.

Key changes:
- the client now accepts all options defined in Req.new/1
- global configuration can be set in `config.exs`
- per-function call overrides are possible using the `req` argument
- the Req client configuration has been deprecated and moved from the
  Client module to the Connection module

Author: kianmeng

config/dev.exs

@@ -4,7 +4,11 @@ config :open_api_typesense,
   api_key: "xyz",
   host: "localhost",
   port: 8108,
-  scheme: "http"
+  scheme: "http",
+  # see https://hexdocs.pm/req/Req.html#new/1
+  options: [
+    retry: false
+  ]
 
 config :oapi_generator,
   default: [

config/test.exs

@@ -5,5 +5,7 @@ config :open_api_typesense,
   host: "localhost",
   port: 8108,
   scheme: "http",
-  max_retries: 0,
-  retry: false
+  # see https://hexdocs.pm/req/Req.html#new/1
+  options: [
+    retry: false
+  ]

lib/open_api_typesense/client.ex

@@ -23,23 +23,24 @@ defmodule OpenApiTypesense.Client do
 
   @doc since: "0.2.0"
   @spec get_host :: String.t() | nil
+  @deprecated "Use OpenApiTypesense.Connection.config(:host) instead"
   def get_host, do: Application.get_env(:open_api_typesense, :host)
 
   @doc since: "0.2.0"
   @spec get_scheme :: String.t() | nil
+  @deprecated "Use OpenApiTypesense.Connection.config(:scheme) instead"
   def get_scheme, do: Application.get_env(:open_api_typesense, :scheme)
 
   @doc since: "0.2.0"
   @spec get_port :: non_neg_integer() | nil
+  @deprecated "Use OpenApiTypesense.Connection.config(:port) instead"
   def get_port, do: Application.get_env(:open_api_typesense, :port)
 
   @doc since: "0.5.0"
   @spec get_client :: keyword() | nil
+  @deprecated "Use OpenApiTypesense.Connection.config(:client) instead"
   def get_client, do: Application.get_env(:open_api_typesense, :client)
 
-  defp test_max_retries, do: Application.get_env(:open_api_typesense, :max_retries)
-  defp test_retry, do: Application.get_env(:open_api_typesense, :retry)
-
   @doc """
   Returns the Typesense's API key
 
@@ -90,40 +91,42 @@ defmodule OpenApiTypesense.Client do
     if client do
       client.request(conn, opts)
     else
-      default_client(conn, opts)
+      req_client = build_req_client(conn, opts)
+      req_request(req_client, opts)
     end
   end
 
-  defp default_client(conn, opts) do
-    # Req.Request.append_error_steps and its retry option are used here.
-    # options like retry, max_retries, etc. can be found in:
-    # https://hexdocs.pm/req/Req.Steps.html#retry/1
-    # NOTE: look at source code in Github
+  def build_req_client(conn, opts) do
+    # Default request options. These can be overridden in this hierarchy:
+    # 1. Via the `:options` key in `config.exs`.
+    # 2. By passing `:req` key to `opts` arg to request/2.
+    req_options =
+      conn
+      |> Map.get(:options, [])
+      |> Keyword.merge(Access.get(opts, :req, []))
+
     url =
       %URI{
         scheme: conn.scheme,
         host: conn.host,
         port: conn.port,
-        path: opts[:url],
-        query: URI.encode_query(opts[:query] || [])
+        path: Access.get(opts, :url),
+        query: URI.encode_query(Access.get(opts, :query, []))
       }
 
-    {_req, resp} =
-      [
-        method: opts[:method] || :get,
-        body: encode_body(opts),
-        url: url,
-        max_retries: test_max_retries() || opts[:req][:max_retries] || 3,
-        retry: test_retry() || opts[:req][:retry] || :safe_transient,
-        compress_body: opts[:req][:compress] || false,
-        cache: opts[:req][:cache] || false,
-        decode_json: [keys: :atoms]
-      ]
-      |> Req.new()
-      |> Req.Request.merge_options(Map.to_list(Map.get(conn, :options, %{})))
-      |> Req.Request.put_header("x-typesense-api-key", Map.get(conn, :api_key))
-      |> Req.Request.run_request()
+    [
+      method: Access.get(opts, :method, :get),
+      body: encode_body(opts),
+      url: url,
+      decode_json: [keys: :atoms]
+    ]
+    |> Req.new()
+    |> Req.Request.merge_options(req_options)
+    |> Req.Request.put_header("x-typesense-api-key", Map.get(conn, :api_key))
+  end
 
+  defp req_request(req_client, opts) do
+    {_req, resp} = Req.Request.run_request(req_client)
     parse_resp(resp, opts[:response])
   end
 

lib/open_api_typesense/connection.ex

@@ -5,18 +5,25 @@ defmodule OpenApiTypesense.Connection do
   Fetches credentials either from application env or map.
   """
 
-  alias OpenApiTypesense.Client
-
   @derive {Inspect, except: [:api_key]}
-  defstruct [:host, :api_key, :port, :scheme, :client]
+  defstruct [
+    :host,
+    :api_key,
+    :port,
+    :scheme,
+    :client,
+    # see https://hexdocs.pm/req/Req.html#new/1
+    options: []
+  ]
 
   @typedoc since: "0.2.0"
   @type t() :: %{
           host: binary() | nil,
           api_key: binary() | nil,
           port: non_neg_integer() | nil,
           scheme: binary() | nil,
-          client: list() | nil
+          client: list() | nil,
+          options: list()
         }
 
   @doc """
@@ -49,15 +56,16 @@ defmodule OpenApiTypesense.Connection do
 
   @doc since: "0.2.0"
   @spec new(t() | map()) :: %__MODULE__{}
-  def new(%__MODULE__{} = connection) when is_struct(connection) do
-    connection
+  def new(%__MODULE__{} = conn) when is_struct(conn, __MODULE__) do
+    conn
   end
 
-  def new(connection) when is_map(connection) do
-    missing_fields = Enum.sort(required_fields() -- Map.keys(connection))
+  def new(conn) when is_map(conn) do
+    missing_fields = Enum.sort(required_fields() -- Map.keys(conn))
 
     if missing_fields == [] do
-      struct(__MODULE__, connection)
+      fields = Map.merge(defaults(), conn)
+      struct(__MODULE__, fields)
     else
       raise ArgumentError, "Missing required fields: #{inspect(missing_fields)}"
     end
@@ -75,19 +83,29 @@ defmodule OpenApiTypesense.Connection do
     # another HTTP client. See README.
     __MODULE__
     |> struct(%{})
-    |> Map.drop([:__struct__])
-    |> Map.drop([:client])
+    |> Map.drop([:__struct__, :client, :options])
     |> Map.keys()
   end
 
   @spec defaults :: map()
   defp defaults do
     %{
-      host: Client.get_host(),
-      api_key: Client.api_key(),
-      port: Client.get_port(),
-      scheme: Client.get_scheme(),
-      client: Client.get_client()
+      host: config(:host),
+      api_key: config(:api_key),
+      port: config(:port),
+      scheme: config(:scheme),
+      client: config(:client),
+      options: config(:options, [])
     }
   end
+
+  @spec config(atom()) :: any()
+  def config(key, default \\ nil) do
+    Access.get(config(), key, default)
+  end
+
+  @spec config :: list()
+  def config do
+    Application.get_all_env(:open_api_typesense)
+  end
 end

lib/open_api_typesense/operations/health.ex

@@ -14,7 +14,9 @@ defmodule OpenApiTypesense.Health do
   """
 
   @spec health :: {:ok, OpenApiTypesense.HealthStatus.t()} | :error
-  def health, do: health(Connection.new())
+  def health do
+    health(Connection.new(), [])
+  end
 
   @doc """
   Either one of:
@@ -24,12 +26,12 @@ defmodule OpenApiTypesense.Health do
   """
   @spec health(Connection.t() | map() | keyword()) ::
           {:ok, OpenApiTypesense.HealthStatus.t()} | :error
-  def health(opts) when is_list(opts) do
-    health(Connection.new(), opts)
+  def health(conn) when is_struct(conn, Connection) do
+    health(conn, [])
   end
 
-  def health(conn) do
-    health(conn, [])
+  def health(conn) when is_map(conn) do
+    health(Connection.new(conn), [])
   end
 
   @doc """

test/connection_test.exs

@@ -17,7 +17,8 @@ defmodule ConnectionTest do
              api_key: "xyz",
              host: "localhost",
              port: 8108,
-             scheme: "http"
+             scheme: "http",
+             options: [retry: false]
            }
   end
 
@@ -35,7 +36,8 @@ defmodule ConnectionTest do
              api_key: "myapikey",
              host: "otherhost",
              port: 9200,
-             scheme: "https"
+             scheme: "https",
+             options: [retry: false]
            }
   end
 

test/client_test.exs test/custom_client_test.exstest/client_test.exs renamed to test/custom_client_test.exs

@@ -1,4 +1,4 @@
-defmodule ClientTest do
+defmodule CustomClientTest do
   use ExUnit.Case, async: false
   require Logger
 
@@ -110,7 +110,6 @@ defmodule ClientTest do
     assert CustomClient === Application.get_env(:open_api_typesense, :client)
 
     assert {:ok, %{"ok" => true}} = OpenApiTypesense.Health.health()
-    assert {:ok, %{"ok" => true}} = OpenApiTypesense.Health.health([])
     assert {:ok, %{"ok" => true}} = OpenApiTypesense.Health.health(conn)
     assert {:ok, %{"ok" => true}} = OpenApiTypesense.Health.health(map_conn)
     assert {:ok, %{"ok" => true}} = OpenApiTypesense.Health.health(conn, [])

test/default_client_test.exs

@@ -0,0 +1,39 @@
+defmodule DefaultClientTest do
+  use ExUnit.Case, async: false
+
+  alias OpenApiTypesense.Client
+  alias OpenApiTypesense.Connection
+
+  require Logger
+
+  describe "request/2" do
+    @tag ["27.1": true, "27.0": true, "26.0": true]
+    test "default to req http client if no custom client set" do
+      conn = Connection.new()
+
+      opts = %{
+        args: [],
+        call: {OpenApiTypesense.Health, :health},
+        url: "/health",
+        method: :get,
+        response: [{200, {OpenApiTypesense.HealthStatus, :t}}],
+        opts: []
+      }
+
+      assert {:ok, %OpenApiTypesense.HealthStatus{ok: true}} = Client.request(conn, opts)
+    end
+  end
+
+  describe "build_req_client/2" do
+    test "default req options" do
+      req = Client.build_req_client(Connection.new(), [])
+      assert req.headers == %{"x-typesense-api-key" => ["xyz"]}
+      assert req.options == %{decode_json: [keys: :atoms], retry: false}
+    end
+
+    test "override req options through req field" do
+      req = Client.build_req_client(Connection.new(), req: [retry: true, cache: false])
+      assert req.options == %{decode_json: [keys: :atoms], cache: false, retry: true}
+    end
+  end
+end

test/operations/health_test.exs

@@ -15,7 +15,6 @@ defmodule HealthTest do
   @tag ["27.1": true, "27.0": true, "26.0": true]
   test "success: health check", %{conn: conn, map_conn: map_conn} do
     assert {:ok, %HealthStatus{ok: true}} = Health.health()
-    assert {:ok, %HealthStatus{ok: true}} = Health.health([])
     assert {:ok, %HealthStatus{ok: true}} = Health.health(conn)
     assert {:ok, %HealthStatus{ok: true}} = Health.health(map_conn)
     assert {:ok, %HealthStatus{ok: true}} = Health.health(conn, [])