From 41c148650da7f1b8db4b580399599138a38d4e7a Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 30 Jul 2025 13:17:56 -0500 Subject: [PATCH] refactor: Remove CachedQueries and rename optimized functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove all CachedQueries usage throughout the codebase - Replace with direct Packets module calls - Delete CachedQueries module entirely - Rename get_recent_packets_optimized to get_recent_packets - Add has_weather_packets? function to Packets module - Fix duplicate function definitions - Update all test references to use new function names This simplifies the codebase by removing the caching layer and eliminates the database ownership errors in tests. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- lib/aprsme/cached_queries.ex | 229 ------------------ lib/aprsme/packets.ex | 101 ++++---- lib/aprsme/packets_behaviour.ex | 1 - .../controllers/api/v1/callsign_controller.ex | 4 +- lib/aprsme_web/live/map_live/data_builder.ex | 3 +- .../live/map_live/historical_loader.ex | 10 +- lib/aprsme_web/live/map_live/index.ex | 6 +- lib/aprsme_web/live/map_live/navigation.ex | 4 +- lib/aprsme_web/live/map_live/rf_path.ex | 4 +- .../live/weather_live/callsign_view.ex | 4 +- .../integration/aprs_status_test.exs | 2 +- .../live/map_live/movement_test.exs | 2 +- test/support/mock_helpers.ex | 2 +- test/test_helper.exs | 2 +- 14 files changed, 64 insertions(+), 310 deletions(-) delete mode 100644 lib/aprsme/cached_queries.ex diff --git a/lib/aprsme/cached_queries.ex b/lib/aprsme/cached_queries.ex deleted file mode 100644 index 9d53b8c..0000000 --- a/lib/aprsme/cached_queries.ex +++ /dev/null @@ -1,229 +0,0 @@ -defmodule Aprsme.CachedQueries do - @moduledoc """ - Caching layer for database queries to improve performance - """ - - alias Aprsme.Cache - alias Aprsme.Packet - alias Aprsme.Packets - alias Aprsme.Repo - alias Ecto.Adapters.SQL - - # 1 minute for frequently changing data - @cache_ttl_short Cache.to_timeout(minute: 1) - # 1 minute for moderately changing data - @cache_ttl_medium Cache.to_timeout(minute: 1) - - @doc """ - Get recent packets with caching - """ - def get_recent_packets_cached(opts) do - cache_key = generate_cache_key("recent_packets", opts) - - case Cache.get(:query_cache, cache_key) do - {:ok, result} when not is_nil(result) -> - result - - _ -> - result = Packets.get_recent_packets_optimized(opts) - Cache.put(:query_cache, cache_key, result, ttl: @cache_ttl_short) - result - end - end - - @doc """ - Get weather packets with caching - """ - def get_weather_packets_cached(callsign, start_time, end_time, opts) do - cache_key = generate_cache_key("weather", {callsign, start_time, end_time, opts}) - - case Cache.get(:query_cache, cache_key) do - {:ok, result} when not is_nil(result) -> - result - - _ -> - result = Packets.get_weather_packets(callsign, start_time, end_time, opts) - Cache.put(:query_cache, cache_key, result, ttl: @cache_ttl_medium) - result - end - end - - @doc """ - Get packet count with caching. - Now uses the efficient packet_counters table for O(1) performance. - """ - def get_total_packet_count_cached do - cache_key = "total_packet_count" - - case Cache.get(:query_cache, cache_key) do - {:ok, result} when not is_nil(result) -> - result - - _ -> - # This is now extremely fast due to the counter table - result = Packets.get_total_packet_count() - # Cache for only 5 seconds since the query is now instant - Cache.put(:query_cache, cache_key, result, ttl: Cache.to_timeout(second: 5)) - result - end - end - - @doc """ - Get latest packet for callsign with caching - """ - def get_latest_packet_for_callsign_cached(callsign) do - cache_key = generate_cache_key("latest_packet", callsign) - - case Cache.get(:query_cache, cache_key) do - {:ok, result} when not is_nil(result) -> - result - - _ -> - result = Packets.get_latest_packet_for_callsign(callsign) - # Shorter TTL for latest packets as they change frequently - Cache.put(:query_cache, cache_key, result, ttl: @cache_ttl_short) - result - end - end - - @doc """ - Get latest weather packet for callsign with caching. - Uses the optimized query that checks recent data first. - """ - def get_latest_weather_packet_cached(callsign) do - cache_key = generate_cache_key("latest_weather_packet", callsign) - - case Cache.get(:query_cache, cache_key) do - {:ok, result} when not is_nil(result) -> - result - - _ -> - result = Packets.get_latest_weather_packet(callsign) - # Cache for 5 minutes since weather updates are less frequent - Cache.put(:query_cache, cache_key, result, ttl: @cache_ttl_short) - result - end - end - - @doc """ - Check if a callsign has weather packets with caching. - Uses exact match for performance. - """ - def has_weather_packets_cached?(callsign) do - cache_key = generate_cache_key("has_weather_packets", callsign) - - case Cache.get(:query_cache, cache_key) do - {:ok, result} when not is_nil(result) -> - result - - _ -> - # Use exact match with proper index instead of ilike - import Ecto.Query - - query = - from p in Packet, - where: p.sender == ^callsign, - where: - not is_nil(p.temperature) or not is_nil(p.humidity) or not is_nil(p.pressure) or - not is_nil(p.wind_speed) or not is_nil(p.wind_direction) or not is_nil(p.rain_1h), - limit: 1, - select: true - - result = Repo.exists?(query) - # Cache for 15 minutes - Cache.put(:query_cache, cache_key, result, ttl: @cache_ttl_medium) - result - end - end - - @doc """ - Invalidate cache entries for a specific callsign - """ - def invalidate_callsign_cache(callsign) do - # Pattern-based cache invalidation - patterns = [ - "latest_packet:#{callsign}", - "latest_weather_packet:#{callsign}", - "has_weather_packets:#{callsign}", - "weather:#{callsign}:*" - ] - - Enum.each(patterns, fn pattern -> - Cache.del(:query_cache, pattern) - end) - end - - @doc """ - Invalidate all cached queries - """ - def invalidate_all_cache do - Cache.clear(:query_cache) - end - - @doc """ - Get cache statistics - """ - def get_cache_stats do - {:ok, stats} = Cache.stats(:query_cache) - stats - end - - @doc """ - Get path station positions with caching - """ - def get_path_station_positions_cached(callsigns) when is_list(callsigns) do - cache_key = generate_cache_key("path_stations", Enum.sort(callsigns)) - - case Cache.get(:query_cache, cache_key) do - {:ok, result} when not is_nil(result) -> - result - - _ -> - query = """ - WITH latest_positions AS ( - SELECT DISTINCT ON (sender) - sender, - lat, - lon, - received_at - FROM packets - WHERE - sender = ANY($1::text[]) - AND lat IS NOT NULL - AND lon IS NOT NULL - AND received_at > NOW() - INTERVAL '7 days' - ORDER BY sender, received_at DESC - ) - SELECT sender, lat, lon - FROM latest_positions - ORDER BY array_position($1::text[], sender) - """ - - result = - case SQL.query(Repo, query, [callsigns]) do - {:ok, %{rows: rows}} -> - Enum.map(rows, fn [callsign, lat, lon] -> - %{ - callsign: callsign, - lat: Decimal.to_float(lat), - lng: Decimal.to_float(lon) - } - end) - - {:error, _} -> - [] - end - - # Cache for 5 minutes - Cache.put(:query_cache, cache_key, result, ttl: @cache_ttl_medium) - result - end - end - - # Private helper functions - - defp generate_cache_key(prefix, data) do - hash = :erlang.phash2(data) - "#{prefix}:#{hash}" - end -end diff --git a/lib/aprsme/packets.ex b/lib/aprsme/packets.ex index 280ad06..098bc97 100644 --- a/lib/aprsme/packets.ex +++ b/lib/aprsme/packets.ex @@ -204,7 +204,7 @@ defmodule Aprsme.Packets do {:ok, packet} -> # Invalidate cache for this packet's callsign if Map.has_key?(attrs, :sender) do - Aprsme.CachedQueries.invalidate_callsign_cache(attrs.sender) + # Cache invalidation removed - no longer using CachedQueries end {:ok, packet} @@ -436,32 +436,12 @@ defmodule Aprsme.Packets do end @doc """ - Gets recent packets for the map view. - This is used for initial map loading to show only recent packets. + Gets recent packets for initial map load. + This uses an efficient query pattern for the most common use case. """ @impl true @spec get_recent_packets(map()) :: [struct()] def get_recent_packets(opts \\ %{}) do - # Use provided hours_back or default to 24 hours - opts_with_time = - if Map.has_key?(opts, :hours_back) do - opts - else - Map.put(opts, :hours_back, 24) - end - - opts_with_time - |> QueryBuilder.recent_position_packets() - |> Repo.all() - end - - @doc """ - Gets recent packets optimized for initial map load. - This uses a more efficient query pattern for the most common use case. - """ - @impl true - @spec get_recent_packets_optimized(map()) :: [struct()] - def get_recent_packets_optimized(opts \\ %{}) do # Use hours_back from opts if provided, otherwise default to 24 hours hours_back = Map.get(opts, :hours_back, 24) time_ago = DateTime.add(DateTime.utc_now(), -hours_back * 3600, :second) @@ -812,41 +792,46 @@ defmodule Aprsme.Packets do """ @spec get_latest_weather_packet(String.t()) :: struct() | nil def get_latest_weather_packet(callsign) when is_binary(callsign) do - # First try last 24 hours - one_day_ago = TimeUtils.one_day_ago() - - recent_packet = - Repo.one( - from(p in Packet, - where: p.sender == ^callsign, - where: - not is_nil(p.temperature) or not is_nil(p.humidity) or not is_nil(p.pressure) or - not is_nil(p.wind_speed) or not is_nil(p.wind_direction) or not is_nil(p.rain_1h), - where: p.received_at >= ^one_day_ago, - order_by: [desc: p.received_at], - limit: 1 - ) - ) - - case recent_packet do - nil -> - # If no recent packet, expand to 7 days - one_week_ago = TimeUtils.one_week_ago() - - Repo.one( - from(p in Packet, - where: p.sender == ^callsign, - where: - not is_nil(p.temperature) or not is_nil(p.humidity) or not is_nil(p.pressure) or - not is_nil(p.wind_speed) or not is_nil(p.wind_direction) or not is_nil(p.rain_1h), - where: p.received_at >= ^one_week_ago, - order_by: [desc: p.received_at], - limit: 1 - ) - ) - - packet -> - packet + # Use proper index with short time window first + case get_latest_weather_in_window(callsign, 24) do + nil -> get_latest_weather_in_window(callsign, 168) + packet -> packet end end + + @doc """ + Check if a callsign has any weather packets. + """ + def has_weather_packets?(callsign) when is_binary(callsign) do + import Ecto.Query + + query = + from p in Packet, + where: p.sender == ^callsign, + where: + not is_nil(p.temperature) or not is_nil(p.humidity) or not is_nil(p.pressure) or + not is_nil(p.wind_speed) or not is_nil(p.wind_direction) or not is_nil(p.rain_1h), + limit: 1, + select: true + + Repo.exists?(query) + end + + defp get_latest_weather_in_window(callsign, hours) do + import Ecto.Query + + since = DateTime.add(DateTime.utc_now(), -hours * 3600, :second) + + query = + from p in Packet, + where: p.sender == ^callsign, + where: p.received_at > ^since, + where: + not is_nil(p.temperature) or not is_nil(p.humidity) or not is_nil(p.pressure) or + not is_nil(p.wind_speed) or not is_nil(p.wind_direction) or not is_nil(p.rain_1h), + order_by: [desc: p.received_at], + limit: 1 + + Repo.one(query) + end end diff --git a/lib/aprsme/packets_behaviour.ex b/lib/aprsme/packets_behaviour.ex index ea4e18f..9ce16cb 100644 --- a/lib/aprsme/packets_behaviour.ex +++ b/lib/aprsme/packets_behaviour.ex @@ -7,7 +7,6 @@ defmodule Aprsme.PacketsBehaviour do @callback stream_packets_for_replay(map()) :: Enumerable.t() @callback get_packets_for_replay(map()) :: list() @callback get_recent_packets(map()) :: list() - @callback get_recent_packets_optimized(map()) :: list() @callback get_nearby_stations(float(), float(), String.t() | nil, map()) :: list() @callback get_weather_packets(String.t(), DateTime.t(), DateTime.t(), map()) :: list() @callback clean_old_packets() :: {:ok, non_neg_integer()} | {:error, any()} diff --git a/lib/aprsme_web/controllers/api/v1/callsign_controller.ex b/lib/aprsme_web/controllers/api/v1/callsign_controller.ex index 3efcc70..701093c 100644 --- a/lib/aprsme_web/controllers/api/v1/callsign_controller.ex +++ b/lib/aprsme_web/controllers/api/v1/callsign_controller.ex @@ -4,8 +4,8 @@ defmodule AprsmeWeb.Api.V1.CallsignController do """ use AprsmeWeb, :controller - alias Aprsme.CachedQueries alias Aprsme.ErrorHandler + alias Aprsme.Packets alias AprsmeWeb.Api.V1.CallsignJSON action_fallback AprsmeWeb.Api.V1.FallbackController @@ -70,7 +70,7 @@ defmodule AprsmeWeb.Api.V1.CallsignController do # Get the most recent packet for this callsign regardless of age or type # Use cached version for better performance with error handling fn -> - case CachedQueries.get_latest_packet_for_callsign_cached(callsign) do + case Packets.get_latest_packet_for_callsign(callsign) do nil -> {:error, :not_found} packet -> {:ok, packet} end diff --git a/lib/aprsme_web/live/map_live/data_builder.ex b/lib/aprsme_web/live/map_live/data_builder.ex index 620dbda..8061742 100644 --- a/lib/aprsme_web/live/map_live/data_builder.ex +++ b/lib/aprsme_web/live/map_live/data_builder.ex @@ -464,8 +464,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do @spec has_weather_packets?(String.t()) :: boolean() defp has_weather_packets?(callsign) when is_binary(callsign) do - # Use cached query for better performance - Aprsme.CachedQueries.has_weather_packets_cached?(callsign) + Aprsme.Packets.has_weather_packets?(callsign) rescue _ -> false end diff --git a/lib/aprsme_web/live/map_live/historical_loader.ex b/lib/aprsme_web/live/map_live/historical_loader.ex index 527c5de..ef130b0 100644 --- a/lib/aprsme_web/live/map_live/historical_loader.ex +++ b/lib/aprsme_web/live/map_live/historical_loader.ex @@ -5,7 +5,7 @@ defmodule AprsmeWeb.MapLive.HistoricalLoader do import Phoenix.Component, only: [assign: 3] - alias Aprsme.CachedQueries + alias Aprsme.Packets alias AprsmeWeb.Live.Shared.PacketUtils, as: SharedPacketUtils alias AprsmeWeb.MapLive.DataBuilder alias Phoenix.LiveView @@ -147,11 +147,11 @@ defmodule AprsmeWeb.MapLive.HistoricalLoader do # Adjust batch size if it would exceed the limit adjusted_batch_size = min(batch_size, max_packets_for_zoom - offset) - packets_module = Application.get_env(:aprsme, :packets_module, Aprsme.Packets) + packets_module = Application.get_env(:aprsme, :packets_module, Packets) historical_packets = try do - if packets_module == Aprsme.Packets do + if packets_module == Packets do # Use cached queries for better performance # Include zoom level in cache key for better cache efficiency params = %{ @@ -168,10 +168,10 @@ defmodule AprsmeWeb.MapLive.HistoricalLoader do historical_hours = SharedPacketUtils.parse_historical_hours(socket.assigns.historical_hours || "1") params = Map.put(params, :hours_back, historical_hours) - CachedQueries.get_recent_packets_cached(params) + Packets.get_recent_packets(params) else # Fallback for testing - packets_module.get_recent_packets_optimized(%{ + packets_module.get_recent_packets(%{ bounds: bounds, limit: batch_size, offset: offset diff --git a/lib/aprsme_web/live/map_live/index.ex b/lib/aprsme_web/live/map_live/index.ex index f9fb069..2341127 100644 --- a/lib/aprsme_web/live/map_live/index.ex +++ b/lib/aprsme_web/live/map_live/index.ex @@ -9,7 +9,7 @@ defmodule AprsmeWeb.MapLive.Index do import AprsmeWeb.TimeHelpers, only: [time_ago_in_words: 1] import Phoenix.LiveView, only: [connected?: 1, push_event: 3, push_patch: 2, put_flash: 3] - alias Aprsme.CachedQueries + alias Aprsme.Packets alias Aprsme.Packets.Clustering alias AprsmeWeb.Endpoint alias AprsmeWeb.Live.Shared.BoundsUtils @@ -179,7 +179,7 @@ defmodule AprsmeWeb.MapLive.Index do if tracked_callsign == "" do nil else - CachedQueries.get_latest_packet_for_callsign_cached(tracked_callsign) + Packets.get_latest_packet_for_callsign(tracked_callsign) end assign(socket, @@ -757,7 +757,7 @@ defmodule AprsmeWeb.MapLive.Index do station_packets = stations |> Enum.map(fn callsign -> - CachedQueries.get_latest_packet_for_callsign_cached(callsign) + Packets.get_latest_packet_for_callsign(callsign) end) |> Enum.filter(& &1) diff --git a/lib/aprsme_web/live/map_live/navigation.ex b/lib/aprsme_web/live/map_live/navigation.ex index 71bbb9e..fea93e4 100644 --- a/lib/aprsme_web/live/map_live/navigation.ex +++ b/lib/aprsme_web/live/map_live/navigation.ex @@ -5,7 +5,7 @@ defmodule AprsmeWeb.MapLive.Navigation do import Phoenix.Component, only: [assign: 3] - alias Aprsme.CachedQueries + alias Aprsme.Packets alias AprsmeWeb.MapLive.UrlParams alias AprsmeWeb.MapLive.Utils alias Phoenix.LiveView @@ -40,7 +40,7 @@ defmodule AprsmeWeb.MapLive.Navigation do @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 - case CachedQueries.get_latest_packet_for_callsign_cached(tracked_callsign) 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} diff --git a/lib/aprsme_web/live/map_live/rf_path.ex b/lib/aprsme_web/live/map_live/rf_path.ex index 4b78755..cba1ad7 100644 --- a/lib/aprsme_web/live/map_live/rf_path.ex +++ b/lib/aprsme_web/live/map_live/rf_path.ex @@ -3,7 +3,7 @@ defmodule AprsmeWeb.MapLive.RfPath do Handles RF path parsing and visualization for APRS packets. """ - alias Aprsme.CachedQueries + alias Aprsme.Packets alias AprsmeWeb.MapLive.Utils @doc """ @@ -52,7 +52,7 @@ defmodule AprsmeWeb.MapLive.RfPath do end defp get_station_position(callsign) do - case CachedQueries.get_latest_packet_for_callsign_cached(callsign) do + case Packets.get_latest_packet_for_callsign(callsign) do %{lat: lat, lon: lon} when is_number(lat) and is_number(lon) -> %{ callsign: callsign, diff --git a/lib/aprsme_web/live/weather_live/callsign_view.ex b/lib/aprsme_web/live/weather_live/callsign_view.ex index f3c86bf..db99024 100644 --- a/lib/aprsme_web/live/weather_live/callsign_view.ex +++ b/lib/aprsme_web/live/weather_live/callsign_view.ex @@ -121,12 +121,12 @@ defmodule AprsmeWeb.WeatherLive.CallsignView do defp get_latest_weather_packet(callsign) do # Use optimized cached query that checks recent data first - Aprsme.CachedQueries.get_latest_weather_packet_cached(callsign) + Aprsme.Packets.get_latest_weather_packet(callsign) end defp get_weather_history(callsign, start_time, end_time) do # Use cached queries to avoid repeated database hits - Aprsme.CachedQueries.get_weather_packets_cached(callsign, start_time, end_time, %{limit: 500}) + Aprsme.Packets.get_weather_packets(callsign, start_time, end_time, %{limit: 500}) end defp default_time_range do diff --git a/test/aprsme_web/integration/aprs_status_test.exs b/test/aprsme_web/integration/aprs_status_test.exs index dc44eec..7ae3df4 100644 --- a/test/aprsme_web/integration/aprs_status_test.exs +++ b/test/aprsme_web/integration/aprs_status_test.exs @@ -100,7 +100,7 @@ defmodule AprsmeWeb.Integration.AprsStatusTest do Mox.set_mox_global() # Stub the function that will be called during bounds changes - Mox.stub(Aprsme.PacketsMock, :get_recent_packets_optimized, fn _opts -> [] end) + Mox.stub(Aprsme.PacketsMock, :get_recent_packets, fn _opts -> [] end) {:ok, view, _html} = live(conn, "/") diff --git a/test/aprsme_web/live/map_live/movement_test.exs b/test/aprsme_web/live/map_live/movement_test.exs index 9e13475..ecd4586 100644 --- a/test/aprsme_web/live/map_live/movement_test.exs +++ b/test/aprsme_web/live/map_live/movement_test.exs @@ -11,7 +11,7 @@ defmodule AprsmeWeb.MapLive.MovementTest do describe "GPS drift filtering" do setup do # Mock the Packets module to return empty results for historical queries - stub(Aprsme.PacketsMock, :get_recent_packets_optimized, fn _args -> [] end) + stub(Aprsme.PacketsMock, :get_recent_packets, fn _args -> [] end) :ok end diff --git a/test/support/mock_helpers.ex b/test/support/mock_helpers.ex index a70ac6c..58b3363 100644 --- a/test/support/mock_helpers.ex +++ b/test/support/mock_helpers.ex @@ -17,7 +17,7 @@ defmodule Aprsme.MockHelpers do {:ok, []} end) - Mox.stub(Aprsme.PacketsMock, :get_recent_packets_optimized, fn _opts -> + Mox.stub(Aprsme.PacketsMock, :get_recent_packets, fn _opts -> [] end) diff --git a/test/test_helper.exs b/test/test_helper.exs index 70b2937..e01b556 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -7,7 +7,7 @@ Mox.defmock(Aprsme.PacketReplayMock, for: Aprsme.PacketReplayBehaviour) Mox.defmock(PacketsMock, for: Aprsme.PacketsBehaviour) # Set up default stubs for commonly used functions -Mox.stub(Aprsme.PacketsMock, :get_recent_packets_optimized, fn _opts -> [] end) +Mox.stub(Aprsme.PacketsMock, :get_recent_packets, fn _opts -> [] end) Mox.stub(Aprsme.PacketsMock, :get_nearby_stations, fn _lat, _lon, _exclude, _opts -> [] end) # Ensure no external APRS connections during tests