diff --git a/.gitignore b/.gitignore index baf5cbc..588507e 100644 --- a/.gitignore +++ b/.gitignore @@ -33,6 +33,7 @@ birdy_chat-*.tar # Ignore messages folder /priv/messages/ +!priv/messages/README.md # In case you use Node.js/npm, you want to ignore these. npm-debug.log diff --git a/config/runtime.exs b/config/runtime.exs index b6444aa..4c7f23e 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -21,7 +21,7 @@ if System.get_env("PHX_SERVER") do end config :birdy_chat, BirdyChatWeb.Endpoint, - http: [port: String.to_integer(System.get_env("PORT", "4000"))] + http: [port: String.to_integer(System.get_env("BIRDY_CHAT_PORT", "4000"))] if config_env() == :prod do # The secret key base is used to sign/encrypt cookies and other secrets. diff --git a/lib/birdy_chat/dispatcher.ex b/lib/birdy_chat/dispatcher.ex index b47c823..e0a3b7d 100644 --- a/lib/birdy_chat/dispatcher.ex +++ b/lib/birdy_chat/dispatcher.ex @@ -1,4 +1,18 @@ defmodule BirdyChat.Dispatcher do + @moduledoc """ + Main dispatcher of messages - decides to either write them to local file system or send them via + HTTP to peers. + + It originally started as a websocket connection between servers, but then I decided to rip + it out and replace with simple HTTP request-response for the following reasons: + + 1. HTTP guarantees immediate feedback (request succeeds or not), making the addition of caching + or retries easy. + 2. HTTP requests have well-know semantic so I can i.e use HTTP statuses for error signals + instead of inventing my own error language. + """ + + @spec dispatch(Ecto.Changeset.t()) :: :ok | {:error, String.t()} def dispatch(%Ecto.Changeset{changes: changes} = changeset) do case changes do %{routing: :local} -> BirdyChat.MessageWriter.write(changeset.changes) @@ -6,13 +20,13 @@ defmodule BirdyChat.Dispatcher do end end - def send_to_remote(%{server: server, to: to} = message) do - {_name, base_url} = + defp send_to_remote(%{server: server, from: from} = message) do + {name, base_url} = BirdyChat.Identity.peers() |> Enum.find(fn {name, _url} -> name == server end) api_url = base_url <> "/api/internal" - token = Phoenix.Token.sign(BirdyChatWeb.Endpoint, "serverAuth", to) + token = Phoenix.Token.sign(BirdyChatWeb.Endpoint, "serverAuth", from) {_request, result} = Req.new(url: api_url, retry: false, method: :post) @@ -24,6 +38,12 @@ defmodule BirdyChat.Dispatcher do # Handle more when you encounter errors case result do %Req.Response{status: 201} -> :ok + # This should never happen under normal circumstances so I am commenting this out but maybe + # needs a second look. + # %Req.Response{status: 403} -> {:error, "Unauthorised"} + + # Peer is down. + %Req.TransportError{reason: :econnrefused} -> {:error, "peer #{name} is unreachable"} end end diff --git a/lib/birdy_chat/identity.ex b/lib/birdy_chat/identity.ex index ae46c1d..cbb502f 100644 --- a/lib/birdy_chat/identity.ex +++ b/lib/birdy_chat/identity.ex @@ -55,6 +55,12 @@ defmodule BirdyChat.Identity do peers: %{"test2" => "http://localhost:4001"}, mode: :test } + + {identity, peers} -> + peers = parse_peers(peers) + identity = parse_identity(identity) + + %__MODULE__{identity: identity, peers: peers, mode: :connected} end end end diff --git a/lib/birdy_chat/message_writer.ex b/lib/birdy_chat/message_writer.ex index b6f53aa..c9d565a 100644 --- a/lib/birdy_chat/message_writer.ex +++ b/lib/birdy_chat/message_writer.ex @@ -1,5 +1,7 @@ defmodule BirdyChat.MessageWriter do - @moduledoc false + @moduledoc """ + Simple file writer that stores messages in priv folder of Elixir application/release. + """ @spec write(%{to: String.t(), from: String.t(), message: String.t()}) :: :ok def write(message) do diff --git a/lib/birdy_chat_web/api/messages/controller.ex b/lib/birdy_chat_web/api/messages/controller.ex index 66161bd..ea86b9b 100644 --- a/lib/birdy_chat_web/api/messages/controller.ex +++ b/lib/birdy_chat_web/api/messages/controller.ex @@ -10,10 +10,10 @@ defmodule BirdyChatWeb.Api.Messages.Controller do |> put_status(:created) |> render(:create, message: changeset.changes) - :error -> + {:error, error} -> conn |> put_status(:unprocessable_entity) - |> render(:error, changeset: changeset) + |> render(:error, message: error) end {:error, changeset} -> diff --git a/lib/birdy_chat_web/api/messages/json.ex b/lib/birdy_chat_web/api/messages/json.ex index a238764..74613fa 100644 --- a/lib/birdy_chat_web/api/messages/json.ex +++ b/lib/birdy_chat_web/api/messages/json.ex @@ -3,6 +3,10 @@ defmodule BirdyChatWeb.Api.Messages.JSON do message end + def render("error.json", %{message: message}) do + %{errors: %{"general" => Gettext.dgettext(BirdyChatWeb.Gettext, "errors", message, [])}} + end + def render("error.json", %{changeset: changeset}) do errors = Ecto.Changeset.traverse_errors(changeset, &get_error/1) %{errors: errors} diff --git a/mix.exs b/mix.exs index 9d6d261..fbb8c69 100644 --- a/mix.exs +++ b/mix.exs @@ -63,7 +63,10 @@ defmodule BirdyChat.MixProject do {:jason, "~> 1.2"}, {:dns_cluster, "~> 0.2.0"}, {:bandit, "~> 1.5"}, - {:credo, "~> 1.0", only: [:dev, :test]}, + + # Static analysis tools + {:credo, "~> 1.0", only: [:dev, :test], runtime: false}, + {:dialyxir, "~> 1.0", only: [:dev, :test], runtime: false}, # Telemetry {:opentelemetry, "~> 1.0"}, @@ -90,6 +93,7 @@ defmodule BirdyChat.MixProject do precommit: [ "compile --warnings-as-errors", "credo --strict", + "dialyzer", "deps.unlock --unused", "format", "test" diff --git a/mix.lock b/mix.lock index f6329e1..7b2c04f 100644 --- a/mix.lock +++ b/mix.lock @@ -7,9 +7,11 @@ "credo": {:hex, :credo, "1.7.16", "a9f1389d13d19c631cb123c77a813dbf16449a2aebf602f590defa08953309d4", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "d0562af33756b21f248f066a9119e3890722031b6d199f22e3cf95550e4f1579"}, "ctx": {:hex, :ctx, "0.6.0", "8ff88b70e6400c4df90142e7f130625b82086077a45364a78d208ed3ed53c7fe", [:rebar3], [], "hexpm", "a14ed2d1b67723dbebbe423b28d7615eb0bdcba6ff28f2d1f1b0a7e1d4aa5fc2"}, "decimal": {:hex, :decimal, "2.3.0", "3ad6255aa77b4a3c4f818171b12d237500e63525c2fd056699967a3e7ea20f62", [:mix], [], "hexpm", "a4d66355cb29cb47c3cf30e71329e58361cfcb37c34235ef3bf1d7bf3773aeac"}, + "dialyxir": {:hex, :dialyxir, "1.4.7", "dda948fcee52962e4b6c5b4b16b2d8fa7d50d8645bbae8b8685c3f9ecb7f5f4d", [:mix], [{:erlex, ">= 0.2.8", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "b34527202e6eb8cee198efec110996c25c5898f43a4094df157f8d28f27d9efe"}, "dns_cluster": {:hex, :dns_cluster, "0.2.0", "aa8eb46e3bd0326bd67b84790c561733b25c5ba2fe3c7e36f28e88f384ebcb33", [:mix], [], "hexpm", "ba6f1893411c69c01b9e8e8f772062535a4cf70f3f35bcc964a324078d8c8240"}, "ecto": {:hex, :ecto, "3.13.5", "9d4a69700183f33bf97208294768e561f5c7f1ecf417e0fa1006e4a91713a834", [:mix], [{:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "df9efebf70cf94142739ba357499661ef5dbb559ef902b68ea1f3c1fabce36de"}, "elixir_make": {:hex, :elixir_make, "0.9.0", "6484b3cd8c0cee58f09f05ecaf1a140a8c97670671a6a0e7ab4dc326c3109726", [:mix], [], "hexpm", "db23d4fd8b757462ad02f8aa73431a426fe6671c80b200d9710caf3d1dd0ffdb"}, + "erlex": {:hex, :erlex, "0.2.8", "cd8116f20f3c0afe376d1e8d1f0ae2452337729f68be016ea544a72f767d9c12", [:mix], [], "hexpm", "9d66ff9fedf69e49dc3fd12831e12a8a37b76f8651dd21cd45fcf5561a8a7590"}, "esbuild": {:hex, :esbuild, "0.10.0", "b0aa3388a1c23e727c5a3e7427c932d89ee791746b0081bbe56103e9ef3d291f", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "468489cda427b974a7cc9f03ace55368a83e1a7be12fba7e30969af78e5f8c70"}, "expo": {:hex, :expo, "1.1.1", "4202e1d2ca6e2b3b63e02f69cfe0a404f77702b041d02b58597c00992b601db5", [:mix], [], "hexpm", "5fb308b9cb359ae200b7e23d37c76978673aa1b06e2b3075d814ce12c5811640"}, "file_system": {:hex, :file_system, "1.1.1", "31864f4685b0148f25bd3fbef2b1228457c0c89024ad67f7a81a3ffbc0bbad3a", [:mix], [], "hexpm", "7a15ff97dfe526aeefb090a7a9d3d03aa907e100e262a0f8f7746b78f8f87a5d"}, diff --git a/priv/messages/README.md b/priv/messages/README.md new file mode 100644 index 0000000..d677767 --- /dev/null +++ b/priv/messages/README.md @@ -0,0 +1 @@ +This folder is used for storing messages local to this server. The contents are otherwise ignored. diff --git a/test/birdy_chat_web/api/messages_test.exs b/test/birdy_chat_web/api/messages_test.exs index 4ecdd2c..e1c7cd1 100644 --- a/test/birdy_chat_web/api/messages_test.exs +++ b/test/birdy_chat_web/api/messages_test.exs @@ -15,6 +15,16 @@ defmodule BirdyChatWeb.Api.MessagesTest do end describe "POST /api/messages to other server" do + test "returns error when the peer is down", %{conn: conn, url: url} do + message = %{from: "test1-user", to: "test2-user", message: "123"} + Req.Test.expect(BirdyChat.Dispatcher, &Req.Test.transport_error(&1, :econnrefused)) + + payload = Jason.encode!(message) + conn = post(conn, url, payload) + assert result = json_response(conn, :unprocessable_entity) + assert result == %{"errors" => %{"general" => "peer test2 is unreachable"}} + end + test "returns error when a peer is unknown", %{conn: conn, url: url} do message = %{from: "test1-user", to: "fake-user", message: "123"} @@ -50,7 +60,7 @@ defmodule BirdyChatWeb.Api.MessagesTest do {"authorization", token} = Enum.find(conn.req_headers, fn {key, _v} -> key == "authorization" end) - {:ok, "test2-user"} = + {:ok, "test1-user"} = Phoenix.Token.verify(BirdyChatWeb.Endpoint, "serverAuth", token, max_age: 1200) assert conn.body_params == expected_body_params