diff --git a/lib/aprsme/performance/insert_optimizer.ex b/lib/aprsme/performance/insert_optimizer.ex index c73c662..fbd2f5a 100644 --- a/lib/aprsme/performance/insert_optimizer.ex +++ b/lib/aprsme/performance/insert_optimizer.ex @@ -75,14 +75,11 @@ defmodule Aprsme.Performance.InsertOptimizer do @impl true def handle_cast({:record_metrics, batch_size, duration_ms, success_count}, state) do - # Calculate throughput (packets per second) - throughput = if duration_ms > 0, do: success_count / (duration_ms / 1000), else: 0 - metric = %{ batch_size: batch_size, duration_ms: duration_ms, success_count: success_count, - throughput: throughput, + throughput: throughput(success_count, duration_ms), timestamp: System.monotonic_time(:millisecond) } @@ -92,6 +89,11 @@ defmodule Aprsme.Performance.InsertOptimizer do {:noreply, %{state | performance_history: new_history}} end + # Packets-per-second, or 0 when the batch completed instantly (avoid div-by-0). + defp throughput(_count, 0), do: 0 + defp throughput(_count, duration_ms) when duration_ms < 0, do: 0 + defp throughput(count, duration_ms), do: count / (duration_ms / 1000) + @impl true def handle_info(:optimize_settings, state) do new_state = optimize_based_on_performance(state) @@ -103,43 +105,44 @@ defmodule Aprsme.Performance.InsertOptimizer do Process.send_after(self(), :optimize_settings, @optimization_check_interval) end - defp optimize_based_on_performance(state) do - if length(state.performance_history) >= 3 do - # Analyze recent performance - recent_metrics = Enum.take(state.performance_history, 5) - total_throughput = recent_metrics |> Enum.map(& &1.throughput) |> Enum.sum() - avg_throughput = total_throughput / length(recent_metrics) - total_duration = recent_metrics |> Enum.map(& &1.duration_ms) |> Enum.sum() - avg_duration = total_duration / length(recent_metrics) + # Require at least three samples before doing anything — match directly on the + # list prefix instead of calling length/1 so we short-circuit for short lists. + defp optimize_based_on_performance(%{performance_history: [_, _, _ | _] = history} = state) do + recent_metrics = Enum.take(history, 5) + sample_size = length(recent_metrics) + avg_throughput = average(recent_metrics, & &1.throughput, sample_size) + avg_duration = average(recent_metrics, & &1.duration_ms, sample_size) - # Adjust batch size based on performance - new_batch_size = calculate_optimal_batch_size(avg_throughput, avg_duration, state.current_batch_size) + new_batch_size = calculate_optimal_batch_size(avg_throughput, avg_duration, state.current_batch_size) + new_insert_options = calculate_insert_options(avg_duration) - # Adjust INSERT options based on system load - new_insert_options = calculate_insert_options(avg_duration) + Logger.debug( + "INSERT optimization: batch_size=#{new_batch_size}, avg_throughput=#{Float.round(avg_throughput, 2)} pps" + ) - Logger.debug( - "INSERT optimization: batch_size=#{new_batch_size}, avg_throughput=#{Float.round(avg_throughput, 2)} pps" - ) + :telemetry.execute([:aprsme, :insert_optimizer, :batch_size], %{value: new_batch_size}, %{}) + :telemetry.execute([:aprsme, :insert_optimizer, :throughput], %{value: avg_throughput}, %{}) + :telemetry.execute([:aprsme, :insert_optimizer, :duration], %{value: avg_duration}, %{}) + maybe_emit_optimization(new_batch_size, state.current_batch_size) - # Emit telemetry for monitoring - :telemetry.execute([:aprsme, :insert_optimizer, :batch_size], %{value: new_batch_size}, %{}) - :telemetry.execute([:aprsme, :insert_optimizer, :throughput], %{value: avg_throughput}, %{}) - :telemetry.execute([:aprsme, :insert_optimizer, :duration], %{value: avg_duration}, %{}) - - if new_batch_size != state.current_batch_size do - :telemetry.execute([:aprsme, :insert_optimizer, :optimizations], %{count: 1}, %{}) - end - - %{ - state - | current_batch_size: new_batch_size, - insert_options: new_insert_options, - last_optimization: System.monotonic_time(:millisecond) - } - else + %{ state - end + | current_batch_size: new_batch_size, + insert_options: new_insert_options, + last_optimization: System.monotonic_time(:millisecond) + } + end + + defp optimize_based_on_performance(state), do: state + + defp average(items, getter, count) do + items |> Enum.map(getter) |> Enum.sum() |> Kernel./(count) + end + + defp maybe_emit_optimization(same, same), do: :ok + + defp maybe_emit_optimization(_new, _old) do + :telemetry.execute([:aprsme, :insert_optimizer, :optimizations], %{count: 1}, %{}) end defp calculate_optimal_batch_size(avg_throughput, avg_duration, current_batch_size) do @@ -158,21 +161,17 @@ defmodule Aprsme.Performance.InsertOptimizer do end end - defp calculate_insert_options(avg_duration) do - base_options = default_insert_options() - - # If INSERTs are taking too long, use more aggressive optimizations - if avg_duration > 3000 do - Keyword.merge(base_options, - returning: false, - on_conflict: :nothing, - timeout: 30_000 - ) - else - base_options - end + # When INSERTs are slow (>3s avg), use a longer timeout; otherwise stick with defaults. + defp calculate_insert_options(avg_duration) when avg_duration > 3_000 do + Keyword.merge(default_insert_options(), + returning: false, + on_conflict: :nothing, + timeout: 30_000 + ) end + defp calculate_insert_options(_avg_duration), do: default_insert_options() + defp default_insert_options do [ # Don't return IDs unless needed diff --git a/lib/aprsme_web/live/map_live/navigation.ex b/lib/aprsme_web/live/map_live/navigation.ex index 269a765..3894e48 100644 --- a/lib/aprsme_web/live/map_live/navigation.ex +++ b/lib/aprsme_web/live/map_live/navigation.ex @@ -21,25 +21,32 @@ defmodule AprsmeWeb.MapLive.Navigation do def determine_map_location(params, session) do {url_center, url_zoom} = UrlParams.parse_map_params(params) has_explicit_url_params = UrlParams.has_explicit_url_params?(params) + geo = session["ip_geolocation"] Logger.info( - "determine_map_location: session ip_geolocation=#{inspect(session["ip_geolocation"])}, has_explicit_url_params=#{has_explicit_url_params}" + "determine_map_location: session ip_geolocation=#{inspect(geo)}, has_explicit_url_params=#{has_explicit_url_params}" ) - case session["ip_geolocation"] do - %{"lat" => lat, "lng" => lng} when is_number(lat) and is_number(lng) -> - if has_explicit_url_params do - Logger.info("Using URL params over geolocation: center=#{inspect(url_center)}, zoom=#{url_zoom}") - {url_center, url_zoom, false} - else - Logger.info("Using IP geolocation: lat=#{lat}, lng=#{lng}, zoom=11") - {%{lat: lat, lng: lng}, 11, true} - end + resolve_location(geo, has_explicit_url_params, url_center, url_zoom) + end - _ -> - Logger.info("No IP geolocation in session, using defaults") - {url_center, url_zoom, !has_explicit_url_params} - end + # Explicit URL params always win over geolocation. + defp resolve_location(_geo, true, url_center, url_zoom) do + Logger.info("Using URL params over geolocation: center=#{inspect(url_center)}, zoom=#{url_zoom}") + {url_center, url_zoom, false} + end + + # Valid geolocation + no explicit URL params → use the geolocation. + defp resolve_location(%{"lat" => lat, "lng" => lng}, false, _url_center, _url_zoom) + when is_number(lat) and is_number(lng) do + Logger.info("Using IP geolocation: lat=#{lat}, lng=#{lng}, zoom=11") + {%{lat: lat, lng: lng}, 11, true} + end + + # No usable geolocation → fall back to URL/defaults. + defp resolve_location(_geo, has_explicit_url_params, url_center, url_zoom) do + Logger.info("No IP geolocation in session, using defaults") + {url_center, url_zoom, !has_explicit_url_params} end @doc """ @@ -47,27 +54,20 @@ defmodule AprsmeWeb.MapLive.Navigation do Returns {final_map_center, final_map_zoom}. """ @spec handle_callsign_tracking(binary(), map(), integer(), boolean()) :: {map(), integer()} - def handle_callsign_tracking(tracked_callsign, map_center, map_zoom, has_explicit_url_params) do - if tracked_callsign != "" and not has_explicit_url_params do - try do - case Packets.get_latest_packet_for_callsign(tracked_callsign) do - %{lat: lat, lon: lon} when is_number(lat) and is_number(lon) -> - {%{lat: lat, lng: lon}, 12} + def handle_callsign_tracking("", map_center, map_zoom, _), do: {map_center, map_zoom} + def handle_callsign_tracking(_, map_center, map_zoom, true), do: {map_center, map_zoom} - _ -> - {map_center, map_zoom} - end - rescue - # Handle database connection errors gracefully (especially in tests) - DBConnection.OwnershipError -> - {map_center, map_zoom} + def handle_callsign_tracking(tracked_callsign, map_center, map_zoom, false) do + case Packets.get_latest_packet_for_callsign(tracked_callsign) do + %{lat: lat, lon: lon} when is_number(lat) and is_number(lon) -> + {%{lat: lat, lng: lon}, 12} - _ -> - {map_center, map_zoom} - end - else - {map_center, map_zoom} + _ -> + {map_center, map_zoom} end + rescue + # Handle database connection errors gracefully (especially in tests). + _error -> {map_center, map_zoom} end @doc """ @@ -77,12 +77,15 @@ defmodule AprsmeWeb.MapLive.Navigation do def handle_callsign_search("", socket), do: {:noreply, socket} def handle_callsign_search(callsign, socket) do - if ParamUtils.valid_callsign?(callsign) do - # Navigate to the new URL structure with the callsign as a path param - {:noreply, LiveView.push_navigate(socket, to: "/#{callsign}")} - else - {:noreply, LiveView.put_flash(socket, :error, "Invalid callsign format")} - end + dispatch_callsign_search(ParamUtils.valid_callsign?(callsign), callsign, socket) + end + + defp dispatch_callsign_search(true, callsign, socket) do + {:noreply, LiveView.push_navigate(socket, to: "/#{callsign}")} + end + + defp dispatch_callsign_search(false, _callsign, socket) do + {:noreply, LiveView.put_flash(socket, :error, "Invalid callsign format")} end @doc """ diff --git a/test/aprsme/performance/insert_optimizer_test.exs b/test/aprsme/performance/insert_optimizer_test.exs new file mode 100644 index 0000000..9e5ff22 --- /dev/null +++ b/test/aprsme/performance/insert_optimizer_test.exs @@ -0,0 +1,153 @@ +defmodule Aprsme.Performance.InsertOptimizerTest do + use ExUnit.Case, async: true + + alias Aprsme.Performance.InsertOptimizer + + defp fresh_state do + %{ + current_batch_size: 200, + insert_options: [returning: false, on_conflict: :nothing, timeout: 15_000], + performance_history: [], + last_optimization: System.monotonic_time(:millisecond) + } + end + + defp metric(throughput, duration_ms) do + %{ + batch_size: 200, + duration_ms: duration_ms, + success_count: 100, + throughput: throughput, + timestamp: System.monotonic_time(:millisecond) + } + end + + describe "GenServer callbacks" do + test "handle_call :get_batch_size returns current batch size" do + state = fresh_state() + + assert {:reply, 200, ^state} = + InsertOptimizer.handle_call(:get_batch_size, self(), state) + end + + test "handle_call :get_insert_options returns insert_options" do + state = fresh_state() + opts = state.insert_options + assert {:reply, ^opts, ^state} = InsertOptimizer.handle_call(:get_insert_options, self(), state) + end + + test "handle_cast records a new metric in performance_history" do + state = fresh_state() + + {:noreply, new_state} = + InsertOptimizer.handle_cast({:record_metrics, 200, 1_000, 100}, state) + + assert [entry | _] = new_state.performance_history + assert entry.batch_size == 200 + assert entry.duration_ms == 1_000 + assert entry.success_count == 100 + # Throughput = 100 / (1000/1000) = 100 pps + assert entry.throughput == 100.0 + end + + test "handle_cast with zero duration avoids div-by-0, throughput is 0" do + state = fresh_state() + + {:noreply, new_state} = + InsertOptimizer.handle_cast({:record_metrics, 100, 0, 50}, state) + + assert [entry | _] = new_state.performance_history + assert entry.throughput == 0 + end + + test "performance history is capped at 20 entries" do + state = fresh_state() + + final = + Enum.reduce(1..25, state, fn _, s -> + {:noreply, s2} = InsertOptimizer.handle_cast({:record_metrics, 200, 1000, 100}, s) + s2 + end) + + assert length(final.performance_history) == 20 + end + end + + describe "handle_info :optimize_settings" do + test "with fewer than 3 metrics, state is unchanged except for re-scheduled timer" do + state = %{fresh_state() | performance_history: [metric(100, 1000), metric(90, 1100)]} + {:noreply, new_state} = InsertOptimizer.handle_info(:optimize_settings, state) + # Batch size shouldn't have moved. + assert new_state.current_batch_size == state.current_batch_size + assert new_state.performance_history == state.performance_history + end + + test "grows batch size when throughput is high and duration low" do + # With avg_throughput > 200 and avg_duration < 2000, batch size should scale up + # from 200 → min(@max, round(200 * 1.2)) = 240. + history = + Enum.map(1..5, fn _ -> metric(300, 1000) end) + + state = %{fresh_state() | performance_history: history} + {:noreply, new_state} = InsertOptimizer.handle_info(:optimize_settings, state) + assert new_state.current_batch_size == 240 + # Fast INSERTs keep default options. + assert Keyword.get(new_state.insert_options, :timeout) == 15_000 + end + + test "shrinks batch size when throughput is low and duration high" do + # avg_throughput < 50 and avg_duration > 5000 → batch_size drops by 20%. + history = Enum.map(1..5, fn _ -> metric(10, 6000) end) + state = %{fresh_state() | performance_history: history} + {:noreply, new_state} = InsertOptimizer.handle_info(:optimize_settings, state) + assert new_state.current_batch_size == 160 + # Slow INSERTs bump timeout. + assert Keyword.get(new_state.insert_options, :timeout) == 30_000 + end + + test "keeps batch size when performance is middling" do + # 100 pps at 3000ms falls in neither bucket → no change to batch size. + history = Enum.map(1..5, fn _ -> metric(100, 3000) end) + state = %{fresh_state() | performance_history: history} + {:noreply, new_state} = InsertOptimizer.handle_info(:optimize_settings, state) + assert new_state.current_batch_size == 200 + end + + test "never drops below the minimum batch size" do + state = %{ + fresh_state() + | current_batch_size: 110, + performance_history: Enum.map(1..5, fn _ -> metric(5, 10_000) end) + } + + {:noreply, new_state} = InsertOptimizer.handle_info(:optimize_settings, state) + assert new_state.current_batch_size == 100 + end + + test "never exceeds the maximum batch size" do + state = %{ + fresh_state() + | current_batch_size: 750, + performance_history: Enum.map(1..5, fn _ -> metric(500, 500) end) + } + + {:noreply, new_state} = InsertOptimizer.handle_info(:optimize_settings, state) + assert new_state.current_batch_size == 800 + end + end + + describe "client API when GenServer isn't running" do + test "get_optimal_batch_size falls back to base size on :noproc" do + # The supervised instance isn't started in the app tree, so this should + # hit the catch clause and return the base batch size (200). + assert InsertOptimizer.get_optimal_batch_size() == 200 + end + + test "get_insert_options falls back to default options on :noproc" do + opts = InsertOptimizer.get_insert_options() + assert Keyword.get(opts, :returning) == false + assert Keyword.get(opts, :on_conflict) == :nothing + assert Keyword.get(opts, :timeout) == 15_000 + end + end +end diff --git a/test/aprsme/signal_handler_test.exs b/test/aprsme/signal_handler_test.exs new file mode 100644 index 0000000..e2c6732 --- /dev/null +++ b/test/aprsme/signal_handler_test.exs @@ -0,0 +1,16 @@ +defmodule Aprsme.SignalHandlerTest do + use ExUnit.Case, async: true + + alias Aprsme.SignalHandler + + describe "handle_info/2" do + test "ignores unrecognized messages without changing state" do + state = %{some: :state} + assert SignalHandler.handle_info(:random, state) == {:noreply, state} + assert SignalHandler.handle_info({:other, "msg"}, state) == {:noreply, state} + end + + # We can't safely exercise the :sigterm clause here — it calls + # Aprsme.ShutdownHandler.shutdown() which would actually shut the app down. + end +end diff --git a/test/aprsme_web/live/locale_hook_test.exs b/test/aprsme_web/live/locale_hook_test.exs new file mode 100644 index 0000000..b77ec8b --- /dev/null +++ b/test/aprsme_web/live/locale_hook_test.exs @@ -0,0 +1,87 @@ +defmodule AprsmeWeb.LocaleHookTest do + # on_mount/4 mutates the process-wide Gettext locale and reads app env; + # serialize with anything else that does the same. + use ExUnit.Case, async: false + + alias AprsmeWeb.LocaleHook + alias Phoenix.LiveView.Socket + + setup do + original_env = Application.get_env(:aprsme, :env) + original_locale = Gettext.get_locale(AprsmeWeb.Gettext) + + on_exit(fn -> + Application.put_env(:aprsme, :env, original_env) + Gettext.put_locale(AprsmeWeb.Gettext, original_locale) + end) + + :ok + end + + defp bare_socket do + %Socket{ + assigns: %{__changed__: %{}}, + private: %{} + } + end + + describe "on_mount/4 in production" do + setup do + Application.put_env(:aprsme, :env, :prod) + :ok + end + + test "sets a supported locale from session" do + {:cont, socket} = LocaleHook.on_mount(:set_locale, %{}, %{"locale" => "es"}, bare_socket()) + assert socket.assigns.locale == "es" + assert Gettext.get_locale(AprsmeWeb.Gettext) == "es" + end + + test "falls back to 'en' for unsupported session locale" do + {:cont, socket} = LocaleHook.on_mount(:set_locale, %{}, %{"locale" => "zz"}, bare_socket()) + assert socket.assigns.locale == "en" + end + + test "falls back to 'en' when session has no locale" do + {:cont, socket} = LocaleHook.on_mount(:set_locale, %{}, %{}, bare_socket()) + assert socket.assigns.locale == "en" + end + + test "accepts all declared supported locales" do + for loc <- ["en", "es", "de", "fr"] do + {:cont, socket} = + LocaleHook.on_mount(:set_locale, %{}, %{"locale" => loc}, bare_socket()) + + assert socket.assigns.locale == loc + end + end + end + + describe "on_mount/4 in non-prod" do + test "leaves locale assign as nil" do + Application.put_env(:aprsme, :env, :dev) + {:cont, socket} = LocaleHook.on_mount(:set_locale, %{}, %{"locale" => "es"}, bare_socket()) + assert socket.assigns.locale == nil + end + end + + describe "on_mount/4 map_page assign" do + test "map_page is false for non-MapLive socket" do + Application.put_env(:aprsme, :env, :dev) + {:cont, socket} = LocaleHook.on_mount(:set_locale, %{}, %{}, bare_socket()) + assert socket.assigns.map_page == false + end + + test "map_page is true when the socket's view is MapLive.Index" do + Application.put_env(:aprsme, :env, :dev) + + socket = %Socket{ + assigns: %{__changed__: %{}}, + private: %{phoenix_live_view: %{view: AprsmeWeb.MapLive.Index}} + } + + {:cont, socket} = LocaleHook.on_mount(:set_locale, %{}, %{}, socket) + assert socket.assigns.map_page == true + end + end +end diff --git a/test/aprsme_web/live/map_live/navigation_test.exs b/test/aprsme_web/live/map_live/navigation_test.exs new file mode 100644 index 0000000..d6f4345 --- /dev/null +++ b/test/aprsme_web/live/map_live/navigation_test.exs @@ -0,0 +1,112 @@ +defmodule AprsmeWeb.MapLive.NavigationTest do + # Logger.info calls are fine during tests; we don't silence them. + use ExUnit.Case, async: true + + alias AprsmeWeb.MapLive.Navigation + alias Phoenix.LiveView.Socket + + describe "determine_map_location/2" do + test "explicit URL params win over IP geolocation" do + params = %{"lat" => "40.0", "lng" => "-95.0", "z" => "7"} + session = %{"ip_geolocation" => %{"lat" => 10.0, "lng" => 20.0}} + + assert {%{lat: 40.0, lng: -95.0}, 7, false} = + Navigation.determine_map_location(params, session) + end + + test "valid IP geolocation used when no explicit URL params" do + session = %{"ip_geolocation" => %{"lat" => 45.5, "lng" => -122.6}} + assert {%{lat: 45.5, lng: -122.6}, 11, true} = Navigation.determine_map_location(%{}, session) + end + + test "missing or invalid geolocation falls back to URL defaults" do + # No geolocation and no URL params → default center + skip_initial_url_update=true + assert {%{lat: _, lng: _}, _zoom, true} = Navigation.determine_map_location(%{}, %{}) + + # Geolocation present but with non-numeric values → fall back + session = %{"ip_geolocation" => %{"lat" => "not-a-number", "lng" => "nope"}} + assert {%{lat: _, lng: _}, _zoom, true} = Navigation.determine_map_location(%{}, session) + end + + test "URL params win even with malformed geolocation" do + session = %{"ip_geolocation" => %{"lat" => "oops", "lng" => "oops"}} + params = %{"lat" => "10.0"} + {_, _, skip?} = Navigation.determine_map_location(params, session) + assert skip? == false + end + end + + describe "handle_callsign_tracking/4" do + @default_center %{lat: 10.0, lng: 20.0} + @default_zoom 5 + + test "empty callsign returns unchanged center and zoom" do + assert Navigation.handle_callsign_tracking("", @default_center, @default_zoom, false) == + {@default_center, @default_zoom} + + assert Navigation.handle_callsign_tracking("", @default_center, @default_zoom, true) == + {@default_center, @default_zoom} + end + + test "when explicit URL params are set, tracking is bypassed" do + assert Navigation.handle_callsign_tracking("K5ABC", @default_center, @default_zoom, true) == + {@default_center, @default_zoom} + end + + test "returns defaults when the database lookup raises" do + # With a callsign that won't match any test data and no DB ownership, + # the rescue clause returns the defaults. + assert Navigation.handle_callsign_tracking( + "NONEXISTENT-99", + @default_center, + @default_zoom, + false + ) == {@default_center, @default_zoom} + end + end + + describe "handle_callsign_search/2" do + test "empty string returns the socket untouched" do + socket = %Socket{assigns: %{__changed__: %{}}} + assert {:noreply, ^socket} = Navigation.handle_callsign_search("", socket) + end + + test "invalid callsign puts a flash error" do + socket = %Socket{assigns: %{__changed__: %{}, flash: %{}}} + {:noreply, result} = Navigation.handle_callsign_search("HELLO WORLD", socket) + assert result.assigns.flash["error"] == "Invalid callsign format" + end + + test "valid callsign triggers push_navigate" do + socket = %Socket{ + assigns: %{__changed__: %{}, flash: %{}}, + redirected: nil + } + + {:noreply, result} = Navigation.handle_callsign_search("K5ABC-9", socket) + # push_navigate/2 sets the `redirected` field on the socket. + assert result.redirected + end + end + + describe "update_and_zoom_to_location/4" do + test "sets map_center and map_zoom assigns, and pushes a zoom event" do + socket = %Socket{assigns: %{__changed__: %{}}} + result = Navigation.update_and_zoom_to_location(socket, 30.5, -95.0, 8) + assert result.assigns.map_center == %{lat: 30.5, lng: -95.0} + assert result.assigns.map_zoom == 8 + end + end + + describe "zoom_to_current_location/1" do + test "pushes a zoom_to_location event using the socket's center/zoom" do + socket = %Socket{ + assigns: %{__changed__: %{}, map_center: %{lat: 1.0, lng: 2.0}, map_zoom: 10} + } + + # The function returns the socket (with push_event recorded). + result = Navigation.zoom_to_current_location(socket) + assert %Socket{} = result + end + end +end