From e2aa519c31cc77cb4dcf183d3b0f095043c77888 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Wed, 7 Sep 2022 14:17:57 +0200 Subject: [PATCH] Test HTTPClient (#2185) * Sort dependencies * Add :bypass dependency * Test HTTPClient * Rename test module --- lib/plausible/http_client.ex | 19 +++-- mix.exs | 97 +++++++++++++------------ mix.lock | 1 + test/plausible/http_client_test.exs | 109 ++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+), 56 deletions(-) create mode 100644 test/plausible/http_client_test.exs diff --git a/lib/plausible/http_client.ex b/lib/plausible/http_client.ex index a2c490d013..dcc8a9bb18 100644 --- a/lib/plausible/http_client.ex +++ b/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 diff --git a/mix.exs b/mix.exs index 005a2d73ee..939c7d6d8e 100644 --- a/mix.exs +++ b/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 diff --git a/mix.lock b/mix.lock index bbaa8264a0..1f47fac3b8 100644 --- a/mix.lock +++ b/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"}, diff --git a/test/plausible/http_client_test.exs b/test/plausible/http_client_test.exs new file mode 100644 index 0000000000..7c2f4a3c51 --- /dev/null +++ b/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