refactor: more pattern matching + drop unreachable get_field_value clause

- Aprsme.Packet.put_phg_fields: dispatch on the shape of the phg value
  (map / 4-char binary / anything else) via put_phg_fields_for/2.
- Aprsme.Packet.get_field_value: remove the catch-all nil clause. After
  the put_phg_fields refactor, dialyzer can prove every call site passes
  a map and flagged the fallback as unreachable — so it's gone, and
  a future mistyped caller now fails loudly instead of silently.
- LogSanitizer.sanitize_map: extract redact_field/2 for readability.
- DeviceIdentification.pattern_matches?: match_pattern/3 dispatched on
  String.contains?/2 keeps the rescue tight around the regex path.
- InfoLive.Show: get_received_at pattern-matches on key shape;
  format_distance splits locale × range via four format_*_distance
  helpers; calculate_course pipes through normalize_bearing/1.

Adds 15 tests exercising the InfoLive.Show helpers (imperial vs. metric
formatting, cardinal bearings, haversine symmetry, Decimal inputs).
Coverage 67.43 → 67.72%.
This commit is contained in:
Graham McIntire 2026-04-23 14:09:40 -05:00
parent bb080fd1dd
commit 35bd098b30
No known key found for this signature in database
GPG key ID: F4ABF488E6029E59
5 changed files with 166 additions and 69 deletions

View file

@ -232,18 +232,18 @@ defmodule Aprsme.DeviceIdentification do
end
defp pattern_matches?(pattern, identifier) do
if String.contains?(pattern, "?") do
try do
regex = wildcard_pattern_to_regex(pattern)
Regex.match?(regex, identifier)
rescue
_e in Regex.CompileError -> false
end
else
pattern == identifier
end
match_pattern(String.contains?(pattern, "?"), pattern, identifier)
end
defp match_pattern(true, pattern, identifier) do
regex = wildcard_pattern_to_regex(pattern)
Regex.match?(regex, identifier)
rescue
_e in Regex.CompileError -> false
end
defp match_pattern(false, pattern, identifier), do: pattern == identifier
# Converts a pattern with ? wildcards to a regex
defp wildcard_pattern_to_regex(pattern) when is_binary(pattern) do
# Replace ? with a placeholder, escape all regex metacharacters except the placeholder,

View file

@ -32,17 +32,23 @@ defmodule Aprsme.LogSanitizer do
Sanitize a map by removing or redacting sensitive fields
"""
def sanitize_map(data) when is_map(data) do
Enum.reduce(@sensitive_fields, data, fn field, acc ->
cond do
Map.has_key?(acc, field) -> Map.put(acc, field, "[REDACTED]")
Map.has_key?(acc, to_string(field)) -> Map.put(acc, to_string(field), "[REDACTED]")
true -> acc
end
end)
Enum.reduce(@sensitive_fields, data, &redact_field/2)
end
def sanitize_map(data), do: data
# Redact a sensitive field under either its atom key or its string form,
# whichever actually exists in the map. Missing = untouched.
defp redact_field(field, acc) do
string_key = to_string(field)
cond do
Map.has_key?(acc, field) -> Map.put(acc, field, "[REDACTED]")
Map.has_key?(acc, string_key) -> Map.put(acc, string_key, "[REDACTED]")
true -> acc
end
end
# Helper to redact a matched sensitive pattern
defp redact_match(match) do
case String.split(match, ["=", ":"]) do

View file

@ -521,25 +521,25 @@ defmodule Aprsme.Packet do
end
defp put_phg_fields(map, data_extended) do
phg = get_field_value(data_extended, :phg)
cond do
phg && is_map(phg) ->
map
|> maybe_put(:phg_power, get_field_value(phg, :power))
|> maybe_put(:phg_height, get_field_value(phg, :height))
|> maybe_put(:phg_gain, get_field_value(phg, :gain))
|> maybe_put(:phg_directivity, get_field_value(phg, :directivity))
phg && is_binary(phg) && String.length(phg) == 4 ->
# Handle new string format from improved parser (e.g., "1060")
parse_phg_string(map, phg)
true ->
map
end
put_phg_fields_for(map, get_field_value(data_extended, :phg))
end
# PHG comes in two shapes: a parsed map, or a 4-char string (e.g. "1060").
# Match on the shape directly instead of `cond`-ing on is_map/is_binary.
defp put_phg_fields_for(map, phg) when is_map(phg) do
map
|> maybe_put(:phg_power, get_field_value(phg, :power))
|> maybe_put(:phg_height, get_field_value(phg, :height))
|> maybe_put(:phg_gain, get_field_value(phg, :gain))
|> maybe_put(:phg_directivity, get_field_value(phg, :directivity))
end
defp put_phg_fields_for(map, phg) when is_binary(phg) and byte_size(phg) == 4 do
parse_phg_string(map, phg)
end
defp put_phg_fields_for(map, _phg), do: map
# Parse PHG string format (e.g., "1060" -> power=1, height=0, gain=6, dir=0)
defp parse_phg_string(
map,
@ -867,10 +867,9 @@ defmodule Aprsme.Packet do
maybe_put(map, :radiorange, get_field_value(data, :radiorange))
end
# Helper to get field value from either atom or string key
# Helper to get field value from either atom or string key. All call sites
# pass a map (dialyzer proves this); no fallback clause needed.
defp get_field_value(data, field) when is_map(data) do
data[field] || data[to_string(field)]
end
defp get_field_value(_, _), do: nil
end

View file

@ -305,58 +305,48 @@ defmodule AprsmeWeb.InfoLive.Show do
end
end
defp get_received_at(packet) do
cond do
Map.has_key?(packet, :received_at) -> packet.received_at
Map.has_key?(packet, "received_at") -> packet["received_at"]
true -> nil
end
defp get_received_at(%{received_at: value}), do: value
defp get_received_at(%{"received_at" => value}), do: value
defp get_received_at(_), do: nil
# English locale uses imperial (ft/mi), everything else uses metric (m/km).
def format_distance(km, locale \\ "en")
def format_distance(km, "en") do
miles = Aprsme.Convert.kph_to_mph(km)
format_imperial_distance(miles)
end
def format_distance(km, locale \\ "en") do
case locale do
"en" ->
# Use imperial units for English locale
miles = Aprsme.Convert.kph_to_mph(km)
def format_distance(km, _locale), do: format_metric_distance(km)
if miles < 1.0 do
feet = miles * 5280
"#{Float.round(feet, 0)} ft"
else
"#{Float.round(miles, 2)} mi"
end
defp format_imperial_distance(miles) when miles < 1.0, do: "#{Float.round(miles * 5280, 0)} ft"
_ ->
# Use metric units for other locales
if km < 1.0 do
"#{Float.round(km * 1000, 0)} m"
else
"#{Float.round(km, 2)} km"
end
end
end
defp format_imperial_distance(miles), do: "#{Float.round(miles, 2)} mi"
defp format_metric_distance(km) when km < 1.0, do: "#{Float.round(km * 1000, 0)} m"
defp format_metric_distance(km), do: "#{Float.round(km, 2)} km"
def calculate_course(lat1, lon1, lat2, lon2) do
# Calculate bearing from point 1 to point 2
# Convert Decimal to float if needed
# Convert Decimal to float if needed.
lat1 = to_float(lat1)
lon1 = to_float(lon1)
lat2 = to_float(lat2)
lon2 = to_float(lon2)
dlon = :math.pi() / 180 * (lon2 - lon1)
lat1_rad = :math.pi() / 180 * lat1
lat2_rad = :math.pi() / 180 * lat2
y = :math.sin(dlon) * :math.cos(lat2_rad)
x = :math.cos(lat1_rad) * :math.sin(lat2_rad) - :math.sin(lat1_rad) * :math.cos(lat2_rad) * :math.cos(dlon)
bearing = :math.atan2(y, x) * 180 / :math.pi()
# Convert to 0-360 range
if bearing < 0, do: bearing + 360, else: bearing
normalize_bearing(:math.atan2(y, x) * 180 / :math.pi())
end
# Convert atan2 output from ±180 to 0-360.
defp normalize_bearing(b) when b < 0, do: b + 360
defp normalize_bearing(b), do: b
defp get_heard_by_stations(callsign, locale) do
alias Aprsme.Repo

View file

@ -0,0 +1,102 @@
defmodule AprsmeWeb.InfoLive.ShowHelpersTest do
use ExUnit.Case, async: true
alias AprsmeWeb.InfoLive.Show
describe "format_distance/2 with English locale (imperial)" do
test "uses feet for distances under a mile" do
# 0.5 km → ~0.31 miles → ~1641 ft
result = Show.format_distance(0.5, "en")
assert result =~ "ft"
refute result =~ "mi"
end
test "uses miles for distances >= 1 mile" do
# 10 km → ~6.21 miles
result = Show.format_distance(10.0, "en")
assert result =~ "mi"
refute result =~ "ft"
end
test "default locale is English" do
assert Show.format_distance(10.0) =~ "mi"
end
end
describe "format_distance/2 with non-English locale (metric)" do
test "uses meters for distances under 1 km" do
result = Show.format_distance(0.5, "de")
assert result =~ "m"
refute result =~ "km"
end
test "uses km for longer distances" do
result = Show.format_distance(5.0, "fr")
assert result =~ "km"
end
test "handles multiple metric locales" do
for loc <- ["es", "de", "fr", "ja"] do
assert Show.format_distance(10.0, loc) =~ "km"
end
end
end
describe "calculate_course/4" do
test "returns 0 (or 360) for due north" do
# From (0,0) to (1,0) — moving north.
bearing = Show.calculate_course(0.0, 0.0, 1.0, 0.0)
assert bearing == 0.0 or bearing == 360.0
end
test "returns ~90 for due east" do
# From (0,0) to (0,1) — moving east.
bearing = Show.calculate_course(0.0, 0.0, 0.0, 1.0)
assert_in_delta bearing, 90.0, 0.5
end
test "returns ~180 for due south" do
bearing = Show.calculate_course(1.0, 0.0, 0.0, 0.0)
assert_in_delta bearing, 180.0, 0.5
end
test "returns ~270 for due west" do
bearing = Show.calculate_course(0.0, 1.0, 0.0, 0.0)
assert_in_delta bearing, 270.0, 0.5
end
test "always returns a value in 0..360" do
for {lat1, lon1, lat2, lon2} <- [
{0.0, 0.0, 45.0, 45.0},
{10.0, 20.0, -10.0, -20.0},
{0.0, 0.0, 0.0, -1.0}
] do
b = Show.calculate_course(lat1, lon1, lat2, lon2)
assert b >= 0
assert b <= 360
end
end
test "accepts Decimal inputs" do
result = Show.calculate_course(Decimal.new("0.0"), Decimal.new("0.0"), Decimal.new("1.0"), Decimal.new("0.0"))
assert is_float(result)
end
end
describe "haversine/4" do
test "returns 0 for identical points" do
assert Show.haversine(40.0, -100.0, 40.0, -100.0) == 0.0
end
test "is symmetric" do
a = Show.haversine(40.0, -100.0, 41.0, -99.0)
b = Show.haversine(41.0, -99.0, 40.0, -100.0)
assert_in_delta a, b, 0.001
end
test "returns positive distance for distinct points" do
d = Show.haversine(40.0, -100.0, 41.0, -99.0)
assert d > 0
end
end
end