From fc5e91479ba5eea9934cdc6d03dc2d765f681f45 Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Thu, 23 Apr 2026 14:18:26 -0500
Subject: [PATCH] refactor: pattern-match CoreComponents.safe_row_id +
 CallsignController

- CoreComponents.safe_row_id: four function heads dispatch on the row
  shape (atom-key map, string-key map, other map fallback, non-map)
  instead of a cond with Map.has_key? checks.
- Api.V1.CallsignController.validate_callsign: check_callsign_format/2
  splits the validation success/failure branches into two clauses; the
  binary-vs-everything-else dispatch stays in validate_callsign/1.

Adds 5 integration tests for the CallsignController covering invalid,
not-found, success, case-normalization, and length-limit paths. The
test file is async:false because it shares the rate-limiter ETS with
other API tests and could otherwise trip the 100/min per-IP cap.
---
 lib/aprsme_web/components/core_components.ex  | 17 +++----
 .../controllers/api/v1/callsign_controller.ex | 12 ++---
 .../api/v1/callsign_controller_test.exs       | 51 +++++++++++++++++++
 3 files changed, 63 insertions(+), 17 deletions(-)
 create mode 100644 test/aprsme_web/controllers/api/v1/callsign_controller_test.exs

diff --git a/lib/aprsme_web/components/core_components.ex b/lib/aprsme_web/components/core_components.ex
index 6d9094b..dd8856c 100644
--- a/lib/aprsme_web/components/core_components.ex
+++ b/lib/aprsme_web/components/core_components.ex
@@ -553,18 +553,17 @@ defmodule AprsmeWeb.CoreComponents do
     """
   end
 
-  # Helper function to safely generate row IDs
+  # Helper function to safely generate row IDs.
+  # Prefer the :id / "id" field on maps; hash the inspected row as a
+  # last-resort fallback. Non-map rows go through Phoenix.Param.
+  defp safe_row_id(%{id: id}), do: id
+  defp safe_row_id(%{"id" => id}), do: id
+
   defp safe_row_id(row) when is_map(row) do
-    cond do
-      Map.has_key?(row, :id) -> row.id
-      Map.has_key?(row, "id") -> row["id"]
-      true -> :md5 |> :crypto.hash(inspect(row)) |> Base.encode16() |> String.slice(0, 8)
-    end
+    :md5 |> :crypto.hash(inspect(row)) |> Base.encode16() |> String.slice(0, 8)
   end
 
-  defp safe_row_id(row) do
-    Phoenix.Param.to_param(row)
-  end
+  defp safe_row_id(row), do: Phoenix.Param.to_param(row)
 
   @doc ~S"""
   Renders a table with generic styling.
diff --git a/lib/aprsme_web/controllers/api/v1/callsign_controller.ex b/lib/aprsme_web/controllers/api/v1/callsign_controller.ex
index 701093c..bbc0a79 100644
--- a/lib/aprsme_web/controllers/api/v1/callsign_controller.ex
+++ b/lib/aprsme_web/controllers/api/v1/callsign_controller.ex
@@ -44,19 +44,15 @@ defmodule AprsmeWeb.Api.V1.CallsignController do
   end
 
   defp validate_callsign(callsign) when is_binary(callsign) do
-    # Remove any whitespace and convert to uppercase
     normalized = callsign |> String.trim() |> String.upcase()
-
-    if validate_callsign_format(normalized) do
-      {:ok, normalized}
-    else
-      {:error, :invalid_callsign}
-    end
+    check_callsign_format(validate_callsign_format(normalized), normalized)
   end
 
-  defp validate_callsign(nil), do: {:error, :invalid_callsign}
   defp validate_callsign(_), do: {:error, :invalid_callsign}
 
+  defp check_callsign_format(true, normalized), do: {:ok, normalized}
+  defp check_callsign_format(false, _normalized), do: {:error, :invalid_callsign}
+
   defp validate_callsign_format(callsign) do
     # Basic callsign validation
     # Matches patterns like: N0CALL, N0CALL-9, W1ABC-15, etc.
diff --git a/test/aprsme_web/controllers/api/v1/callsign_controller_test.exs b/test/aprsme_web/controllers/api/v1/callsign_controller_test.exs
new file mode 100644
index 0000000..02e6dca
--- /dev/null
+++ b/test/aprsme_web/controllers/api/v1/callsign_controller_test.exs
@@ -0,0 +1,51 @@
+defmodule AprsmeWeb.Api.V1.CallsignControllerTest do
+  # async: false — shares the per-IP rate-limiter ETS with other API tests.
+  # Running concurrently trips the 100/min cap and causes 429s under load.
+  use AprsmeWeb.ConnCase, async: false
+
+  import Aprsme.PacketsFixtures
+
+  describe "GET /api/v1/callsign/:callsign" do
+    test "returns 400 for an invalid callsign format", %{conn: conn} do
+      conn = get(conn, ~p"/api/v1/callsign/invalid@callsign")
+      assert conn.status == 400
+      body = json_response(conn, 400)
+      assert body["error"]["message"] =~ "Invalid callsign"
+    end
+
+    test "returns empty/not-found response for an unknown callsign", %{conn: conn} do
+      conn = get(conn, ~p"/api/v1/callsign/Z9ZZZ-99")
+      assert conn.status == 200
+      body = json_response(conn, 200)
+      # The not_found template renders {data: nil, message: "No packets..."}.
+      assert body["data"] == nil
+      assert body["message"] =~ "No packets found"
+    end
+
+    test "returns the most recent packet when one exists", %{conn: conn} do
+      packet_fixture(%{sender: "W1ABC-9"})
+
+      conn = get(conn, ~p"/api/v1/callsign/W1ABC-9")
+      assert conn.status == 200
+      body = json_response(conn, 200)
+      assert body["data"]["callsign"] == "W1ABC-9"
+      assert is_map(body["data"]["position"])
+    end
+
+    test "normalizes the callsign to uppercase", %{conn: conn} do
+      packet_fixture(%{sender: "K5ABC-1"})
+
+      conn = get(conn, ~p"/api/v1/callsign/k5abc-1")
+      assert conn.status == 200
+      body = json_response(conn, 200)
+      assert body["data"]["callsign"] == "K5ABC-1"
+    end
+
+    test "rejects a callsign that's too long (over 12 chars)", %{conn: conn} do
+      # 13 chars
+      long_callsign = "N0CALLN0CALLN"
+      conn = get(conn, ~p"/api/v1/callsign/#{long_callsign}")
+      assert conn.status == 400
+    end
+  end
+end