Test HTTPClient (#2185)

* Sort dependencies

* Add :bypass dependency

* Test HTTPClient

* Rename test module

lib/plausible/http_client.ex

@@ -25,13 +25,13 @@ defmodule Plausible.HTTPClient do
   @doc """
   Make a GET request
   """
-  @spec get(url(), headers(), params()) :: response()
-  def get(url, headers \\ [], params \\ nil) do
-    call(:get, url, headers, params)
+  @spec get(url(), headers()) :: response()
+  def get(url, headers \\ []) do
+    call(:get, url, headers, nil)
   end
 
   defp call(method, url, headers, params) do
-    params = maybe_encode_params(params, headers)
+    {params, headers} = maybe_encode_params(params, headers)
 
     method
     |> build_request(url, headers, params)
@@ -46,8 +46,8 @@ defmodule Plausible.HTTPClient do
     Finch.request(request, Plausible.Finch)
   end
 
-  defp maybe_encode_params(params, _headers) when is_binary(params) or is_nil(params) do
-    params
+  defp maybe_encode_params(params, headers) when is_binary(params) or is_nil(params) do
+    {params, headers}
   end
 
   defp maybe_encode_params(params, headers) when is_map(params) do
@@ -60,10 +60,13 @@ defmodule Plausible.HTTPClient do
 
     case String.downcase(content_type) do
       "application/x-www-form-urlencoded" ->
-        URI.encode_query(params)
+        {URI.encode_query(params), headers}
+
+      "application/json" ->
+        {Jason.encode!(params), headers}
 
       _ ->
-        Jason.encode!(params)
+        {Jason.encode!(params), [{"content-type", "application/json"} | headers]}
     end
   end
 end

mix.exs

@@ -51,66 +51,67 @@ defmodule Plausible.MixProject do
   # Type `mix help deps` for examples and options.
   defp deps do
     [
-      {:finch, "~> 0.12.0", override: true},
+      {:bamboo, "~> 2.2"},
+      {:bamboo_phoenix, "~> 1.0.0"},
+      {:bamboo_postmark, git: "https://github.com/pablo-co/bamboo_postmark.git", tag: "master"},
+      {:bamboo_smtp, "~> 4.1"},
       {:bcrypt_elixir, "~> 2.0"},
+      {:bypass, "~> 2.1", only: [:dev, :test]},
+      {:cachex, "~> 3.4"},
+      {:clickhouse_ecto, git: "https://github.com/plausible/clickhouse_ecto.git"},
       {:combination, "~> 0.0.3"},
-      {:cors_plug, "~> 3.0"},
-      {:plug, "~> 1.13", override: true},
       {:connection, "~> 1.1", override: true},
+      {:cors_plug, "~> 3.0"},
+      {:credo, "~> 1.5", only: [:dev, :test], runtime: false},
+      {:csv, "~> 2.3"},
+      {:dialyxir, "~> 1.0", only: [:dev, :test], runtime: false},
+      {:double, "~> 0.8.0", only: :test},
       {:ecto_sql, "~> 3.0"},
       {:elixir_uuid, "~> 1.2", only: :test},
+      {:envy, "~> 1.1.1"},
+      {:ex_machina, "~> 2.3", only: :test},
+      {:excoveralls, "~> 0.10", only: :test},
+      {:exvcr, "~> 0.11", only: :test},
+      {:finch, "~> 0.12.0", override: true},
+      {:floki, "~> 0.32.0", only: :test},
+      {:fun_with_flags, "~> 1.8.1"},
+      {:fun_with_flags_ui, "~> 0.8"},
+      {:geolix, "~> 2.0"},
+      {:geolix_adapter_mmdb2, "~> 0.6.0"},
+      {:hackney, "~> 1.8"},
+      {:hammer, "~> 6.0"},
+      {:httpoison, "~> 1.4"},
       {:jason, "~> 1.3"},
+      {:kaffy, "~> 0.9.0"},
+      {:location, git: "https://github.com/plausible/location.git"},
+      {:mimic, "~> 1.7", only: :test},
+      {:nanoid, "~> 2.0.2"},
+      {:oauther, "~> 1.3"},
+      {:oban, "~> 2.11"},
+      {:observer_cli, "~> 1.7"},
+      {:opentelemetry, "~> 1.0"},
+      {:opentelemetry_api, "~> 1.0"},
+      {:opentelemetry_ecto, "~> 1.0.0"},
+      {:opentelemetry_exporter, "~> 1.0"},
+      {:opentelemetry_phoenix, "~> 1.0"},
       {:phoenix, "~> 1.6.0"},
       {:phoenix_ecto, "~> 4.0"},
       {:phoenix_html, "~> 2.12"},
       {:phoenix_live_reload, "~> 1.2", only: :dev},
-      {:phoenix_pubsub, "~> 2.0"},
-      {:plug_cowboy, "~> 2.3"},
-      {:ref_inspector, "~> 1.3"},
-      {:timex, "~> 3.6"},
-      {:ua_inspector, "~> 3.0"},
-      {:bamboo, "~> 2.2"},
-      {:hackney, "~> 1.8"},
-      {:bamboo_phoenix, "~> 1.0.0"},
-      {:bamboo_postmark, git: "https://github.com/pablo-co/bamboo_postmark.git", tag: "master"},
-      {:bamboo_smtp, "~> 4.1"},
-      {:sentry, "~> 8.0"},
-      {:httpoison, "~> 1.4"},
-      {:ex_machina, "~> 2.3", only: :test},
-      {:excoveralls, "~> 0.10", only: :test},
-      {:double, "~> 0.8.0", only: :test},
-      {:php_serializer, "~> 2.0"},
-      {:csv, "~> 2.3"},
-      {:oauther, "~> 1.3"},
-      {:nanoid, "~> 2.0.2"},
-      {:siphash, "~> 3.2"},
-      {:oban, "~> 2.11"},
-      {:geolix, "~> 2.0"},
-      {:clickhouse_ecto, git: "https://github.com/plausible/clickhouse_ecto.git"},
-      {:location, git: "https://github.com/plausible/location.git"},
-      {:geolix_adapter_mmdb2, "~> 0.6.0"},
-      {:cachex, "~> 3.4"},
-      {:dialyxir, "~> 1.0", only: [:dev, :test], runtime: false},
-      {:credo, "~> 1.5", only: [:dev, :test], runtime: false},
-      {:kaffy, "~> 0.9.0"},
-      {:envy, "~> 1.1.1"},
       {:phoenix_pagination, "~> 0.7.0"},
-      {:hammer, "~> 6.0"},
-      {:public_suffix, git: "https://github.com/axelson/publicsuffix-elixir"},
-      {:floki, "~> 0.32.0", only: :test},
-      {:referrer_blocklist, git: "https://github.com/plausible/referrer-blocklist.git"},
-      {:fun_with_flags, "~> 1.8.1"},
-      {:fun_with_flags_ui, "~> 0.8"},
-      {:opentelemetry_api, "~> 1.0"},
-      {:opentelemetry, "~> 1.0"},
-      {:opentelemetry_exporter, "~> 1.0"},
-      {:opentelemetry_phoenix, "~> 1.0"},
-      {:telemetry, "~> 1.0", override: true},
-      {:opentelemetry_ecto, "~> 1.0.0"},
-      {:observer_cli, "~> 1.7"},
-      {:mimic, "~> 1.7", only: :test},
+      {:phoenix_pubsub, "~> 2.0"},
+      {:php_serializer, "~> 2.0"},
+      {:plug, "~> 1.13", override: true},
+      {:plug_cowboy, "~> 2.3"},
       {:prom_ex, "~> 1.7.1"},
-      {:exvcr, "~> 0.11", only: :test}
+      {:public_suffix, git: "https://github.com/axelson/publicsuffix-elixir"},
+      {:ref_inspector, "~> 1.3"},
+      {:referrer_blocklist, git: "https://github.com/plausible/referrer-blocklist.git"},
+      {:sentry, "~> 8.0"},
+      {:siphash, "~> 3.2"},
+      {:telemetry, "~> 1.0", override: true},
+      {:timex, "~> 3.6"},
+      {:ua_inspector, "~> 3.0"}
     ]
   end
 

mix.lock

@@ -6,6 +6,7 @@
   "bamboo_smtp": {:hex, :bamboo_smtp, "4.1.0", "ba547be4146ae592f63af05c6c7b7b5195b2b6ca57eeea9d80070b38eacd528b", [:mix], [{:bamboo, "~> 2.2.0", [hex: :bamboo, repo: "hexpm", optional: false]}, {:gen_smtp, "~> 1.1.1", [hex: :gen_smtp, repo: "hexpm", optional: false]}], "hexpm", "cb1a2856ab0507d10df609428314aa5e18231e8b1801a5bc6e42f319eeb50ad9"},
   "bcrypt_elixir": {:hex, :bcrypt_elixir, "2.3.1", "5114d780459a04f2b4aeef52307de23de961b69e13a5cd98a911e39fda13f420", [:make, :mix], [{:comeonin, "~> 5.3", [hex: :comeonin, repo: "hexpm", optional: false]}, {:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "42182d5f46764def15bf9af83739e3bf4ad22661b1c34fc3e88558efced07279"},
   "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm", "7af5c7e09fe1d40f76c8e4f9dd2be7cebd83909f31fee7cd0e9eadc567da8353"},
+  "bypass": {:hex, :bypass, "2.1.0", "909782781bf8e20ee86a9cabde36b259d44af8b9f38756173e8f5e2e1fabb9b1", [:mix], [{:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.0", [hex: :plug_cowboy, repo: "hexpm", optional: false]}, {:ranch, "~> 1.3", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "d9b5df8fa5b7a6efa08384e9bbecfe4ce61c77d28a4282f79e02f1ef78d96b80"},
   "cachex": {:hex, :cachex, "3.4.0", "868b2959ea4aeb328c6b60ff66c8d5123c083466ad3c33d3d8b5f142e13101fb", [:mix], [{:eternal, "~> 1.2", [hex: :eternal, repo: "hexpm", optional: false]}, {:jumper, "~> 1.0", [hex: :jumper, repo: "hexpm", optional: false]}, {:sleeplocks, "~> 1.1", [hex: :sleeplocks, repo: "hexpm", optional: false]}, {:unsafe, "~> 1.0", [hex: :unsafe, repo: "hexpm", optional: false]}], "hexpm", "370123b1ab4fba4d2965fb18f87fd758325709787c8c5fce35b3fe80645ccbe5"},
   "castore": {:hex, :castore, "0.1.17", "ba672681de4e51ed8ec1f74ed624d104c0db72742ea1a5e74edbc770c815182f", [:mix], [], "hexpm", "d9844227ed52d26e7519224525cb6868650c272d4a3d327ce3ca5570c12163f9"},
   "certifi": {:hex, :certifi, "2.9.0", "6f2a475689dd47f19fb74334859d460a2dc4e3252a3324bd2111b8f0429e7e21", [:rebar3], [], "hexpm", "266da46bdb06d6c6d35fde799bcb28d36d985d424ad7c08b5bb48f5b5cdd4641"},

test/plausible/http_client_test.exs

@@ -0,0 +1,109 @@
+defmodule Plausible.HTTPClientTest do
+  use ExUnit.Case, async: true
+
+  alias Plausible.HTTPClient
+  alias Plug.Conn
+
+  setup do
+    bypass = Bypass.open()
+    {:ok, bypass: bypass}
+  end
+
+  test "get/2 works", %{bypass: bypass} do
+    Bypass.expect_once(bypass, "GET", "/", fn conn ->
+      Conn.resp(conn, 200, "ok")
+    end)
+
+    assert {:ok, %Finch.Response{status: 200, body: "ok"}} = HTTPClient.get(bypass_url(bypass))
+  end
+
+  test "post/3 works", %{bypass: bypass} do
+    Bypass.expect_once(bypass, "POST", "/", fn conn ->
+      Conn.resp(conn, 200, "ok")
+    end)
+
+    assert {:ok, %Finch.Response{status: 200, body: "ok"}} = HTTPClient.post(bypass_url(bypass))
+  end
+
+  test "post/3 doesn't alter params if binary passed",
+       %{
+         bypass: bypass
+       } do
+    body = "raw binary"
+    headers = []
+
+    Bypass.expect_once(bypass, "POST", "/", fn conn ->
+      opts = Plug.Parsers.init(parsers: [:urlencoded, {:json, json_decoder: Jason}])
+
+      conn
+      |> Plug.Parsers.call(opts)
+      |> Conn.resp(200, body)
+    end)
+
+    assert {:ok, %Finch.Response{status: 200, body: ^body}} =
+             HTTPClient.post(bypass_url(bypass), headers, body)
+  end
+
+  test "post/3 URL-encodes params if request content-type is set to application/x-www-form-urlencoded and a map is supplied",
+       %{
+         bypass: bypass
+       } do
+    body = %{hello: :world, alice: :bob}
+    headers = [{"Content-Type", "application/x-www-form-urlencoded"}]
+
+    Bypass.expect_once(bypass, "POST", "/", fn conn ->
+      opts = Plug.Parsers.init(parsers: [:urlencoded])
+      conn = Plug.Parsers.call(conn, opts)
+
+      assert hd(Conn.get_req_header(conn, "content-type")) == "application/x-www-form-urlencoded"
+      assert conn.body_params["hello"] == "world"
+      assert conn.body_params["alice"] == "bob"
+      Conn.resp(conn, 200, "ok")
+    end)
+
+    assert {:ok, %Finch.Response{status: 200, body: "ok"}} =
+             HTTPClient.post(bypass_url(bypass), headers, body)
+  end
+
+  test "post/3 JSON-encodes params if request content-type is other than application/x-www-form-urlencoded and a map is supplied",
+       %{
+         bypass: bypass
+       } do
+    params = %{hello: :world, alice: :bob}
+    headers_no_content_type = [{"foo", "moo"}]
+    headers_json = [{"Content-Type", "application/json"}]
+
+    Bypass.expect_once(bypass, "POST", "/any", fn conn ->
+      opts = Plug.Parsers.init(parsers: [:urlencoded, {:json, json_decoder: Jason}])
+      conn = Plug.Parsers.call(conn, opts)
+
+      assert Conn.get_req_header(conn, "content-type") == ["application/json"]
+      assert conn.body_params["hello"] == "world"
+      assert conn.body_params["alice"] == "bob"
+      Conn.resp(conn, 200, "ok")
+    end)
+
+    Bypass.expect_once(bypass, "POST", "/json", fn conn ->
+      opts = Plug.Parsers.init(parsers: [{:json, json_decoder: Jason}])
+      conn = Plug.Parsers.call(conn, opts)
+
+      assert Conn.get_req_header(conn, "content-type") == ["application/json"]
+      assert conn.body_params["hello"] == "world"
+      assert conn.body_params["alice"] == "bob"
+      Conn.resp(conn, 200, "ok")
+    end)
+
+    assert {:ok, %Finch.Response{status: 200, body: "ok"}} =
+             HTTPClient.post(bypass_url(bypass, path: "/json"), headers_json, params)
+
+    assert {:ok, %Finch.Response{status: 200, body: "ok"}} =
+             HTTPClient.post(bypass_url(bypass, path: "/any"), headers_no_content_type, params)
+  end
+
+  defp bypass_url(bypass, opts \\ []) do
+    port = bypass.port
+    path = Keyword.get(opts, :path, "/")
+
+    "http://localhost:#{port}#{path}"
+  end
+end