diff --git a/lib/aprsme/encoding_utils.ex b/lib/aprsme/encoding_utils.ex index 36e131e..4e3d128 100644 --- a/lib/aprsme/encoding_utils.ex +++ b/lib/aprsme/encoding_utils.ex @@ -293,35 +293,44 @@ defmodule Aprsme.EncodingUtils do defp sanitize_string_fields(packet) do Enum.reduce(@string_fields, packet, fn field, acc -> - string_key = to_string(field) - # Use sanitize_comment for comment field to strip Mic-E telemetry - sanitizer = if field == :comment, do: &sanitize_comment/1, else: &sanitize_string/1 - - cond do - Map.has_key?(acc, field) -> - Map.update(acc, field, nil, sanitizer) - - Map.has_key?(acc, string_key) -> - Map.update(acc, string_key, nil, sanitizer) - - true -> - acc - end + # sanitize_comment strips Mic-E telemetry; everything else uses the + # generic sanitizer. + sanitizer = field_sanitizer(field) + update_existing_key(acc, field, sanitizer) end) end + defp field_sanitizer(:comment), do: &sanitize_comment/1 + defp field_sanitizer(_other), do: &sanitize_string/1 + defp sanitize_nested_map(packet, key, sanitizer) do - string_key = to_string(key) + update_existing_key(packet, key, nil_aware(sanitizer)) + end + # Apply `sanitizer` to whichever key (atom or string) actually exists in the + # map. If neither is present, return the map unchanged. + defp update_existing_key(map, atom_key, sanitizer) do + string_key = to_string(atom_key) + + case first_present_key(map, atom_key, string_key) do + nil -> map + key -> Map.update(map, key, nil, sanitizer) + end + end + + defp first_present_key(map, atom_key, string_key) do cond do - Map.has_key?(packet, key) -> - Map.update(packet, key, nil, &if(is_nil(&1), do: nil, else: sanitizer.(&1))) + Map.has_key?(map, atom_key) -> atom_key + Map.has_key?(map, string_key) -> string_key + true -> nil + end + end - Map.has_key?(packet, string_key) -> - Map.update(packet, string_key, nil, &if(is_nil(&1), do: nil, else: sanitizer.(&1))) - - true -> - packet + # Wraps a sanitizer so it passes `nil` values through unchanged. + defp nil_aware(sanitizer) do + fn + nil -> nil + value -> sanitizer.(value) end end diff --git a/lib/aprsme_web/live/status_live/index.ex b/lib/aprsme_web/live/status_live/index.ex index 8aefbbf..a5d9495 100644 --- a/lib/aprsme_web/live/status_live/index.ex +++ b/lib/aprsme_web/live/status_live/index.ex @@ -459,28 +459,20 @@ defmodule AprsmeWeb.StatusLive.Index 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 calculate_health_score(aprs_status) do - cond do - not aprs_status.connected -> 1 - # Less than 5 minutes - aprs_status.uptime_seconds < 300 -> 2 - # Less than 1 hour - aprs_status.uptime_seconds < 3600 -> 3 - # Less than 1 day - aprs_status.uptime_seconds < 86_400 -> 4 - # More than 1 day - true -> 5 - end - 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" + + # Health score buckets: 1 = disconnected, 2–5 = connected by stability ramp. + defp calculate_health_score(%{connected: false}), do: 1 + defp calculate_health_score(%{uptime_seconds: s}) when s < 300, do: 2 + defp calculate_health_score(%{uptime_seconds: s}) when s < 3_600, do: 3 + defp calculate_health_score(%{uptime_seconds: s}) when s < 86_400, do: 4 + defp calculate_health_score(_), do: 5 def get_health_description(score, connected) do case {score, connected} do @@ -496,16 +488,14 @@ defmodule AprsmeWeb.StatusLive.Index do def format_time_ago(nil), do: gettext("Never") def format_time_ago(datetime) do - diff_seconds = DateTime.diff(DateTime.utc_now(), datetime) - - cond do - diff_seconds < 60 -> gettext("%{count} seconds ago", count: diff_seconds) - diff_seconds < 3600 -> gettext("%{count} minutes ago", count: div(diff_seconds, 60)) - diff_seconds < 86_400 -> gettext("%{count} hours ago", count: div(diff_seconds, 3600)) - true -> gettext("%{count} days ago", count: div(diff_seconds, 86_400)) - end + DateTime.utc_now() |> DateTime.diff(datetime) |> format_seconds_ago() end + defp format_seconds_ago(s) when s < 60, do: gettext("%{count} seconds ago", count: s) + defp format_seconds_ago(s) when s < 3_600, do: gettext("%{count} minutes ago", count: div(s, 60)) + defp format_seconds_ago(s) when s < 86_400, do: gettext("%{count} hours ago", count: div(s, 3_600)) + defp format_seconds_ago(s), do: gettext("%{count} days ago", count: div(s, 86_400)) + def format_number(number) when is_integer(number) do number |> Integer.to_string() diff --git a/lib/aprsme_web/plugs/health_check.ex b/lib/aprsme_web/plugs/health_check.ex index afa5a5b..23d8059 100644 --- a/lib/aprsme_web/plugs/health_check.ex +++ b/lib/aprsme_web/plugs/health_check.ex @@ -42,21 +42,19 @@ defmodule AprsmeWeb.Plugs.HealthCheck do end defp check_health(:readiness) do - # Readiness probe - fails when shutting down to stop new traffic + # Readiness probe - fails when shutting down to stop new traffic. health_status = Application.get_env(:aprsme, :health_status, :healthy) + readiness_status(health_status, shutting_down?()) + end - cond do - health_status == :draining -> - {:error, "Application is draining connections"} + defp readiness_status(:draining, _shutting_down?), do: {:error, "Application is draining connections"} - shutting_down?() -> - {:error, "Application is shutting down"} + defp readiness_status(_status, true), do: {:error, "Application is shutting down"} - true -> - case full_health_checks() do - :ok -> {:ok, "OK"} - {:error, reason} -> {:error, "Readiness check failed: #{reason}"} - end + defp readiness_status(_status, false) do + case full_health_checks() do + :ok -> {:ok, "OK"} + {:error, reason} -> {:error, "Readiness check failed: #{reason}"} end end @@ -94,23 +92,21 @@ defmodule AprsmeWeb.Plugs.HealthCheck do defp shutting_down? do # Check if ShutdownHandler process exists and is shutting down - case Process.whereis(Aprsme.ShutdownHandler) do - nil -> - # Process doesn't exist, not shutting down - false + Aprsme.ShutdownHandler |> Process.whereis() |> ask_shutting_down?() + end - pid when is_pid(pid) -> - # Process exists, check if alive and call it - if Process.alive?(pid) do - try do - GenServer.call(pid, :shutting_down?, 5000) - catch - :exit, _ -> false - _, _ -> false - end - else - false - end - end + # No ShutdownHandler process → definitely not shutting down. + defp ask_shutting_down?(nil), do: false + + defp ask_shutting_down?(pid) when is_pid(pid) do + ask_if_alive(Process.alive?(pid), pid) + end + + defp ask_if_alive(false, _pid), do: false + + defp ask_if_alive(true, pid) do + GenServer.call(pid, :shutting_down?, 5000) + catch + _kind, _reason -> false end end diff --git a/test/aprsme_web/live/status_live/index_helpers_test.exs b/test/aprsme_web/live/status_live/index_helpers_test.exs new file mode 100644 index 0000000..a257b97 --- /dev/null +++ b/test/aprsme_web/live/status_live/index_helpers_test.exs @@ -0,0 +1,100 @@ +defmodule AprsmeWeb.StatusLive.IndexHelpersTest do + use ExUnit.Case, async: true + + alias AprsmeWeb.StatusLive.Index + + describe "format_uptime/1" do + test "returns 'Not connected' for zero and negative" do + assert Index.format_uptime(0) =~ "Not connected" + assert Index.format_uptime(-10) =~ "Not connected" + end + + test "seconds-only for values under a minute" do + assert Index.format_uptime(5) == "5s" + end + + test "minutes and seconds for values under an hour" do + assert Index.format_uptime(125) == "2m 5s" + end + + test "hours/minutes/seconds for values under a day" do + assert Index.format_uptime(3 * 3600 + 4 * 60 + 5) == "3h 4m 5s" + end + + test "days/hours/minutes/seconds for longer values" do + assert Index.format_uptime(2 * 86_400 + 1 * 3600 + 30 * 60 + 15) == "2d 1h 30m 15s" + end + end + + describe "format_time_ago/1" do + test "returns 'Never' for nil" do + assert Index.format_time_ago(nil) =~ "Never" + end + + test "formats seconds" do + dt = DateTime.add(DateTime.utc_now(), -10, :second) + result = Index.format_time_ago(dt) + assert result =~ "seconds ago" + end + + test "formats minutes" do + dt = DateTime.add(DateTime.utc_now(), -120, :second) + result = Index.format_time_ago(dt) + assert result =~ "minutes ago" + end + + test "formats hours" do + dt = DateTime.add(DateTime.utc_now(), -3 * 3600, :second) + result = Index.format_time_ago(dt) + assert result =~ "hours ago" + end + + test "formats days" do + dt = DateTime.add(DateTime.utc_now(), -3 * 86_400, :second) + result = Index.format_time_ago(dt) + assert result =~ "days ago" + end + end + + describe "get_health_description/2" do + test "returns connection-issues text for score 1 with disconnect" do + assert Index.get_health_description(1, false) =~ "Disconnected" + end + + test "returns stability text for scores 2-5 when connected" do + assert Index.get_health_description(2, true) =~ "Recently" + assert Index.get_health_description(3, true) =~ "Good" + assert Index.get_health_description(4, true) =~ "Very good" + assert Index.get_health_description(5, true) =~ "Excellent" + end + + test "returns unknown text for mismatched (score, connected) combos" do + assert Index.get_health_description(5, false) =~ "Unknown" + assert Index.get_health_description(99, true) =~ "Unknown" + end + end + + describe "format_number/1" do + test "adds comma thousand separators to integers" do + assert Index.format_number(1_000) == "1,000" + assert Index.format_number(1_234_567) == "1,234,567" + end + + test "leaves small integers untouched" do + assert Index.format_number(42) == "42" + assert Index.format_number(999) == "999" + end + + test "handles zero and negative integers" do + assert Index.format_number(0) == "0" + # Negative number formatting: preserve the minus sign in front. + assert Index.format_number(-1_234) =~ "1,234" + end + + test "converts non-integer input to string" do + assert Index.format_number(1.5) == "1.5" + assert Index.format_number(nil) == "" + assert Index.format_number(:atom) == "atom" + end + end +end