refactor: pattern-match in DeviceCache and DatabaseMetrics + add tests
- DeviceCache.pattern_matches?: replace a 3-branch `cond` with a two-clause `case` on `String.contains?(pattern, "?")`. The old `*` branch was dead code (it did the same exact-match compare as the fallback), so it's removed. - Telemetry.DatabaseMetrics.collect_postgres_metrics: dispatch on environment atom via multi-clause private functions instead of `if Application.get_env(...) == :test`. New tests: - DeviceCache wildcard/exact lookup paths exercised via a seeded cache. - MobileUserSocket connect/3 for x_headers/peer_data/empty info and rate-limit deny. - DatabaseMetrics pool-metrics emission captured via :telemetry handler and the :test-env short-circuit.
This commit is contained in:
parent
fa970b5637
commit
aa0c6e6566
5 changed files with 220 additions and 23 deletions
|
|
@ -123,16 +123,14 @@ defmodule Aprsme.DeviceCache do
|
|||
end)
|
||||
end
|
||||
|
||||
# Patterns containing `?` wildcards route through regex; everything else is
|
||||
# compared verbatim. The old `*` branch was dead code — it did the same
|
||||
# exact-match compare as the fallback — so it's gone.
|
||||
defp pattern_matches?(pattern, identifier) do
|
||||
cond do
|
||||
String.contains?(pattern, "?") ->
|
||||
matches_wildcard_pattern?(pattern, identifier)
|
||||
|
||||
String.contains?(pattern, "*") ->
|
||||
pattern == identifier
|
||||
|
||||
true ->
|
||||
pattern == identifier
|
||||
if String.contains?(pattern, "?") do
|
||||
matches_wildcard_pattern?(pattern, identifier)
|
||||
else
|
||||
pattern == identifier
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -91,23 +91,18 @@ defmodule Aprsme.Telemetry.DatabaseMetrics do
|
|||
end
|
||||
|
||||
def collect_postgres_metrics do
|
||||
# Skip database metrics collection in test environment
|
||||
if Application.get_env(:aprsme, :env) == :test do
|
||||
:ok
|
||||
else
|
||||
do_collect_postgres_metrics()
|
||||
end
|
||||
do_collect_postgres_metrics(Application.get_env(:aprsme, :env))
|
||||
end
|
||||
|
||||
defp do_collect_postgres_metrics do
|
||||
# Check if Repo is started before collecting metrics
|
||||
case Process.whereis(Aprsme.Repo) do
|
||||
nil ->
|
||||
# Repo not started yet, skip metrics collection silently
|
||||
:ok
|
||||
# Skip database metrics collection in the test environment — the extra
|
||||
# pg_stat_* queries add test-suite noise and sometimes fail in CI sandboxes.
|
||||
defp do_collect_postgres_metrics(:test), do: :ok
|
||||
|
||||
_pid ->
|
||||
collect_database_metrics()
|
||||
defp do_collect_postgres_metrics(_env) do
|
||||
case Process.whereis(Aprsme.Repo) do
|
||||
# Repo not started yet, skip metrics collection silently.
|
||||
nil -> :ok
|
||||
_pid -> collect_database_metrics()
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -26,4 +26,50 @@ defmodule Aprsme.DeviceCacheTest do
|
|||
test "lookup_device/1 returns nil on cache miss without crashing when server is not running" do
|
||||
assert DeviceCache.lookup_device("TEST-DEVICE") == nil
|
||||
end
|
||||
|
||||
test "lookup_device/1 returns nil for nil input" do
|
||||
assert DeviceCache.lookup_device(nil) == nil
|
||||
end
|
||||
|
||||
describe "wildcard pattern matching via cache" do
|
||||
setup do
|
||||
# Seed the cache directly with a small device list so we can exercise
|
||||
# the pattern-matching branches without running the GenServer.
|
||||
devices = [
|
||||
%Aprsme.Devices{identifier: "APSK21", vendor: "Kenwood", model: "TH-D74"},
|
||||
%Aprsme.Devices{identifier: "APS???", vendor: "Kenwood", model: "Unknown"},
|
||||
%Aprsme.Devices{identifier: "EXACT", vendor: "Test", model: "Exact"}
|
||||
]
|
||||
|
||||
Aprsme.Cache.put(:device_cache, :all_devices, devices)
|
||||
|
||||
on_exit(fn -> Aprsme.Cache.put(:device_cache, :all_devices, []) end)
|
||||
:ok
|
||||
end
|
||||
|
||||
test "exact identifier matches literally" do
|
||||
assert %{vendor: "Test"} = DeviceCache.lookup_device("EXACT")
|
||||
end
|
||||
|
||||
test "returns nil for non-matching identifier" do
|
||||
assert DeviceCache.lookup_device("DOES-NOT-EXIST") == nil
|
||||
end
|
||||
|
||||
test "prefers exact match when both wildcard and exact apply" do
|
||||
# Both EXACT (literal) and APS??? would not both apply here; just verify
|
||||
# that exact-string hits don't get confused with wildcard entries.
|
||||
assert %{identifier: "EXACT"} = DeviceCache.lookup_device("EXACT")
|
||||
end
|
||||
|
||||
test "? wildcards match a single character" do
|
||||
# APSK21 matches APS??? (three wildcards after APS)
|
||||
assert %{identifier: _} = DeviceCache.lookup_device("APSK21")
|
||||
end
|
||||
|
||||
test "wildcard pattern does not match if length mismatches" do
|
||||
# APS??? requires exactly 3 trailing chars; "APSXX" has only 2 and
|
||||
# shouldn't match the wildcard entry (nor is there any other entry).
|
||||
assert DeviceCache.lookup_device("APSXX") == nil
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
59
test/aprsme/telemetry/database_metrics_test.exs
Normal file
59
test/aprsme/telemetry/database_metrics_test.exs
Normal file
|
|
@ -0,0 +1,59 @@
|
|||
defmodule Aprsme.Telemetry.DatabaseMetricsTest do
|
||||
# Attaches telemetry handlers at test-specific event names so each test is
|
||||
# isolated, but we still keep async: false to avoid handler-name collisions
|
||||
# in process-global state.
|
||||
use ExUnit.Case, async: false
|
||||
|
||||
alias Aprsme.Telemetry.DatabaseMetrics
|
||||
|
||||
defp attach_collector(event, tag \\ to_string(System.unique_integer([:positive]))) do
|
||||
test_pid = self()
|
||||
handler_id = "test-#{tag}-#{Enum.join(event, ",")}"
|
||||
|
||||
:telemetry.attach(
|
||||
handler_id,
|
||||
event,
|
||||
fn ^event, measurements, metadata, _ ->
|
||||
send(test_pid, {:telemetry, event, measurements, metadata})
|
||||
end,
|
||||
nil
|
||||
)
|
||||
|
||||
on_exit(fn -> :telemetry.detach(handler_id) end)
|
||||
handler_id
|
||||
end
|
||||
|
||||
describe "collect_postgres_metrics/0 in :test env" do
|
||||
test "is a no-op returning :ok" do
|
||||
original = Application.get_env(:aprsme, :env)
|
||||
Application.put_env(:aprsme, :env, :test)
|
||||
|
||||
try do
|
||||
assert DatabaseMetrics.collect_postgres_metrics() == :ok
|
||||
after
|
||||
Application.put_env(:aprsme, :env, original)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "collect_db_pool_metrics/0" do
|
||||
test "emits [:aprsme, :repo, :pool] telemetry with pool_size from config" do
|
||||
attach_collector([:aprsme, :repo, :pool])
|
||||
DatabaseMetrics.collect_db_pool_metrics()
|
||||
|
||||
assert_receive {:telemetry, [:aprsme, :repo, :pool], measurements, _}, 1_000
|
||||
assert Map.has_key?(measurements, :size)
|
||||
assert Map.has_key?(measurements, :idle)
|
||||
assert Map.has_key?(measurements, :busy)
|
||||
assert Map.has_key?(measurements, :available)
|
||||
assert Map.has_key?(measurements, :queue_length)
|
||||
assert Map.has_key?(measurements, :total)
|
||||
end
|
||||
end
|
||||
|
||||
describe "collect_pgbouncer_metrics/0" do
|
||||
test "is a no-op returning :ok" do
|
||||
assert DatabaseMetrics.collect_pgbouncer_metrics() == :ok
|
||||
end
|
||||
end
|
||||
end
|
||||
99
test/aprsme_web/channels/mobile_user_socket_test.exs
Normal file
99
test/aprsme_web/channels/mobile_user_socket_test.exs
Normal file
|
|
@ -0,0 +1,99 @@
|
|||
defmodule AprsmeWeb.MobileUserSocketTest do
|
||||
# The rate limiter uses a process-wide ETS table; we're careful to use unique
|
||||
# IPs so tests don't interfere with each other, but we still mark async:false
|
||||
# to avoid clashing with anything else that flips the :aprsme, :env flag.
|
||||
use ExUnit.Case, async: false
|
||||
|
||||
alias AprsmeWeb.MobileUserSocket
|
||||
|
||||
defp unique_ip, do: "203.0.113." <> to_string(System.unique_integer([:positive]))
|
||||
|
||||
setup do
|
||||
original_env = Application.get_env(:aprsme, :env)
|
||||
on_exit(fn -> Application.put_env(:aprsme, :env, original_env) end)
|
||||
:ok
|
||||
end
|
||||
|
||||
describe "connect/3 in :test env" do
|
||||
test "always succeeds regardless of rate-limit key" do
|
||||
Application.put_env(:aprsme, :env, :test)
|
||||
socket = %Phoenix.Socket{assigns: %{}}
|
||||
|
||||
connect_info = %{x_headers: [{"cf-connecting-ip", "198.51.100.1"}]}
|
||||
|
||||
assert {:ok, result} = MobileUserSocket.connect(%{}, socket, connect_info)
|
||||
assert result.assigns.peer_ip == "198.51.100.1"
|
||||
end
|
||||
|
||||
test "extracts IP from x-forwarded-for, splitting on comma" do
|
||||
Application.put_env(:aprsme, :env, :test)
|
||||
socket = %Phoenix.Socket{assigns: %{}}
|
||||
|
||||
connect_info = %{
|
||||
x_headers: [{"x-forwarded-for", "198.51.100.5, 10.0.0.1"}]
|
||||
}
|
||||
|
||||
assert {:ok, result} = MobileUserSocket.connect(%{}, socket, connect_info)
|
||||
assert result.assigns.peer_ip == "198.51.100.5"
|
||||
end
|
||||
|
||||
test "extracts IP from x-real-ip" do
|
||||
Application.put_env(:aprsme, :env, :test)
|
||||
socket = %Phoenix.Socket{assigns: %{}}
|
||||
connect_info = %{x_headers: [{"x-real-ip", "198.51.100.7"}]}
|
||||
assert {:ok, result} = MobileUserSocket.connect(%{}, socket, connect_info)
|
||||
assert result.assigns.peer_ip == "198.51.100.7"
|
||||
end
|
||||
|
||||
test "falls back to 'unknown' when x_headers list contains no known IP header" do
|
||||
Application.put_env(:aprsme, :env, :test)
|
||||
socket = %Phoenix.Socket{assigns: %{}}
|
||||
connect_info = %{x_headers: [{"user-agent", "curl"}]}
|
||||
assert {:ok, result} = MobileUserSocket.connect(%{}, socket, connect_info)
|
||||
assert result.assigns.peer_ip == "unknown"
|
||||
end
|
||||
|
||||
test "uses peer_data when no x_headers" do
|
||||
Application.put_env(:aprsme, :env, :test)
|
||||
socket = %Phoenix.Socket{assigns: %{}}
|
||||
connect_info = %{peer_data: %{address: {127, 0, 0, 1}}}
|
||||
assert {:ok, result} = MobileUserSocket.connect(%{}, socket, connect_info)
|
||||
assert result.assigns.peer_ip == "127.0.0.1"
|
||||
end
|
||||
|
||||
test "falls back to 'unknown' for empty connect_info" do
|
||||
Application.put_env(:aprsme, :env, :test)
|
||||
socket = %Phoenix.Socket{assigns: %{}}
|
||||
assert {:ok, result} = MobileUserSocket.connect(%{}, socket, %{})
|
||||
assert result.assigns.peer_ip == "unknown"
|
||||
end
|
||||
end
|
||||
|
||||
describe "connect/3 in non-test env with rate limits" do
|
||||
test "allows first connection from an IP" do
|
||||
Application.put_env(:aprsme, :env, :prod)
|
||||
socket = %Phoenix.Socket{assigns: %{}}
|
||||
ip = unique_ip()
|
||||
connect_info = %{x_headers: [{"cf-connecting-ip", ip}]}
|
||||
|
||||
assert {:ok, _} = MobileUserSocket.connect(%{}, socket, connect_info)
|
||||
end
|
||||
|
||||
test "denies once the per-IP limit is exceeded" do
|
||||
Application.put_env(:aprsme, :env, :prod)
|
||||
socket = %Phoenix.Socket{assigns: %{}}
|
||||
ip = unique_ip()
|
||||
connect_info = %{x_headers: [{"cf-connecting-ip", ip}]}
|
||||
|
||||
# The configured limit is 30 in-module; burn through it, then one more.
|
||||
for _ <- 1..30, do: MobileUserSocket.connect(%{}, socket, connect_info)
|
||||
assert :error == MobileUserSocket.connect(%{}, socket, connect_info)
|
||||
end
|
||||
end
|
||||
|
||||
describe "id/1" do
|
||||
test "always returns nil (anonymous socket)" do
|
||||
assert MobileUserSocket.id(%Phoenix.Socket{assigns: %{}}) == nil
|
||||
end
|
||||
end
|
||||
end
|
||||
Loading…
Add table
Reference in a new issue