refactor: pattern-match in TimeHelpers.to_datetime and PageController

- TimeHelpers.to_datetime: `cond` with match?/2 guards becomes five
  function heads dispatched by input type — DateTime passes through,
  NaiveDateTime anchors to UTC, integer is a millisecond Unix epoch,
  binary tries ISO 8601, everything else is nil.
- PageController.health: the three-branch `cond` + inner `if db_healthy`
  becomes a chain of small respond_* helpers, each pattern-matched on a
  single axis (health_status, shutting_down?, db_healthy?). db_healthy?/0
  isolates the rescue.
- format_uptime splits the cond into four format_uptime_parts/4 clauses.

Adds tests for the PageController actions (health 200 / 503-draining,
ready, sitemap.xml, .well-known/api-catalog, status.json). Coverage
66.81 → 67.29%.
This commit is contained in:
Graham McIntire 2026-04-23 13:58:36 -05:00
parent 73342df543
commit 564d21bb26
No known key found for this signature in database
GPG key ID: F4ABF488E6029E59
3 changed files with 148 additions and 73 deletions

View file

@ -5,59 +5,62 @@ defmodule AprsmeWeb.PageController do
alias Aprsme.Cluster.LeaderElection
def health(conn, _params) do
# Use our health check plug logic
health_status = Application.get_env(:aprsme, :health_status, :healthy)
version = :aprsme |> Application.spec(:vsn) |> List.to_string()
respond_health(conn, version, Application.get_env(:aprsme, :health_status, :healthy))
end
cond do
health_status == :draining ->
conn
|> put_status(503)
|> json(%{
status: "draining",
version: version,
message: "Application is draining connections",
timestamp: DateTime.utc_now()
})
defp respond_health(conn, version, :draining) do
health_error(conn, version, "draining", "Application is draining connections")
end
Aprsme.ShutdownHandler.shutting_down?() ->
conn
|> put_status(503)
|> json(%{
status: "shutting_down",
version: version,
message: "Application is shutting down",
timestamp: DateTime.utc_now()
})
defp respond_health(conn, version, _status) do
respond_shutdown_or_db(conn, version, Aprsme.ShutdownHandler.shutting_down?())
end
true ->
# Normal health check
db_healthy =
try do
_ = Aprsme.Repo.query!("SELECT 1")
true
rescue
_ -> false
end
defp respond_shutdown_or_db(conn, version, true) do
health_error(conn, version, "shutting_down", "Application is shutting down")
end
if db_healthy do
json(conn, %{
status: "ok",
version: version,
database: "connected",
timestamp: DateTime.utc_now()
})
else
conn
|> put_status(503)
|> json(%{
status: "error",
version: version,
database: "disconnected",
timestamp: DateTime.utc_now()
})
end
end
defp respond_shutdown_or_db(conn, version, false) do
respond_db_check(conn, version, db_healthy?())
end
defp respond_db_check(conn, version, true) do
json(conn, %{
status: "ok",
version: version,
database: "connected",
timestamp: DateTime.utc_now()
})
end
defp respond_db_check(conn, version, false) do
conn
|> put_status(503)
|> json(%{
status: "error",
version: version,
database: "disconnected",
timestamp: DateTime.utc_now()
})
end
defp health_error(conn, version, status, message) do
conn
|> put_status(503)
|> json(%{
status: status,
version: version,
message: message,
timestamp: DateTime.utc_now()
})
end
defp db_healthy? do
_ = Aprsme.Repo.query!("SELECT 1")
true
rescue
_ -> false
end
@sitemap_paths ~w(/ /about /api /status /packets)
@ -158,12 +161,11 @@ defmodule AprsmeWeb.PageController do
hours = div(rem(seconds, 86_400), 3600)
minutes = div(rem(seconds, 3600), 60)
secs = rem(seconds, 60)
cond do
days > 0 -> "#{days}d #{hours}h #{minutes}m #{secs}s"
hours > 0 -> "#{hours}h #{minutes}m #{secs}s"
minutes > 0 -> "#{minutes}m #{secs}s"
true -> "#{secs}s"
end
format_uptime_parts(days, hours, minutes, secs)
end
defp format_uptime_parts(d, h, m, s) when d > 0, do: "#{d}d #{h}h #{m}m #{s}s"
defp format_uptime_parts(_d, h, m, s) when h > 0, do: "#{h}h #{m}m #{s}s"
defp format_uptime_parts(_d, _h, m, s) when m > 0, do: "#{m}m #{s}s"
defp format_uptime_parts(_d, _h, _m, s), do: "#{s}s"
end

View file

@ -45,25 +45,16 @@ defmodule AprsmeWeb.TimeHelpers do
DateTime or NaiveDateTime structs.
"""
@spec to_datetime(String.t() | integer() | DateTime.t() | NaiveDateTime.t() | any()) :: DateTime.t() | nil
def to_datetime(ts) do
cond do
is_binary(ts) ->
case DateTime.from_iso8601(ts) do
{:ok, dt, _} -> dt
_ -> nil
end
def to_datetime(%DateTime{} = dt), do: dt
def to_datetime(%NaiveDateTime{} = ndt), do: DateTime.from_naive!(ndt, "Etc/UTC")
def to_datetime(ts) when is_integer(ts), do: DateTime.from_unix!(ts, :millisecond)
is_integer(ts) ->
DateTime.from_unix!(ts, :millisecond)
match?(%DateTime{}, ts) ->
ts
match?(%NaiveDateTime{}, ts) ->
DateTime.from_naive!(ts, "Etc/UTC")
true ->
nil
def to_datetime(ts) when is_binary(ts) do
case DateTime.from_iso8601(ts) do
{:ok, dt, _} -> dt
_ -> nil
end
end
def to_datetime(_), do: nil
end

View file

@ -0,0 +1,82 @@
defmodule AprsmeWeb.PageControllerTest do
# Mutates :aprsme, :health_status; must not run in parallel with other tests
# that read it (the HealthCheck plug, for example).
use AprsmeWeb.ConnCase, async: false
setup do
original = Application.get_env(:aprsme, :health_status)
on_exit(fn -> Application.put_env(:aprsme, :health_status, original || :healthy) end)
:ok
end
describe "health/2 controller action" do
# Call the action directly; the /health endpoint is otherwise served by
# the HealthCheck plug as text/plain.
test "returns 200 with ok status when app is healthy and DB is up" do
Application.put_env(:aprsme, :health_status, :healthy)
base = Phoenix.Controller.put_format(Phoenix.ConnTest.build_conn(), "json")
result = AprsmeWeb.PageController.health(base, %{})
assert result.status == 200
body = Jason.decode!(result.resp_body)
assert body["status"] == "ok"
assert body["database"] == "connected"
assert is_binary(body["version"])
end
test "returns 503 draining when app is draining" do
Application.put_env(:aprsme, :health_status, :draining)
base = Phoenix.Controller.put_format(Phoenix.ConnTest.build_conn(), "json")
result = AprsmeWeb.PageController.health(base, %{})
assert result.status == 503
body = Jason.decode!(result.resp_body)
assert body["status"] == "draining"
assert body["message"] =~ "draining"
end
end
describe "GET /ready" do
test "returns 200 with ready status and version", %{conn: conn} do
conn = get(conn, "/ready")
assert conn.status == 200
body = json_response(conn, 200)
assert body["status"] == "ready"
assert is_binary(body["version"])
end
end
describe "GET /sitemap.xml" do
test "returns an XML urlset with known paths", %{conn: conn} do
conn = get(conn, "/sitemap.xml")
assert conn.status == 200
xml = conn.resp_body
assert xml =~ ~s(<?xml version="1.0" encoding="UTF-8"?>)
assert xml =~ "<urlset"
assert xml =~ "/about"
assert xml =~ "/status"
end
end
describe "GET /.well-known/api-catalog" do
test "returns an RFC 9264-style linkset", %{conn: conn} do
conn = get(conn, "/.well-known/api-catalog")
assert conn.status == 200
body = Jason.decode!(conn.resp_body)
assert is_list(body["linkset"])
anchors = Enum.map(body["linkset"], & &1["anchor"])
assert Enum.any?(anchors, &String.contains?(&1, "/api/v1"))
assert Enum.any?(anchors, &String.contains?(&1, "/mobile/websocket"))
end
end
describe "GET /status.json (status_json/2)" do
test "returns application + aprs_is sections", %{conn: conn} do
conn = get(conn, "/status.json")
assert conn.status == 200
body = json_response(conn, 200)
assert Map.has_key?(body, "aprs_is")
assert Map.has_key?(body, "application")
assert is_binary(body["application"]["version"])
end
end
end