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.
This commit is contained in:
Graham McIntire 2026-04-23 14:18:26 -05:00
parent ed993aa4de
commit fc5e91479b
No known key found for this signature in database
GPG key ID: F4ABF488E6029E59
3 changed files with 63 additions and 17 deletions

View file

@ -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.

View file

@ -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.

View file

@ -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