From 2f09b590e24291eda2c550abfd04379b776bf5bc Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 29 Jul 2026 10:54:07 -0500 Subject: [PATCH] =?UTF-8?q?fix:=20resolve=20all=20credo=20issues=20?= =?UTF-8?q?=E2=80=94=200=20warnings,=200=20refactoring,=200=20readability?= =?UTF-8?q?=20issues?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add ex_slop credo plugin with all 40 checks - Remove DualKeyAccess patterns across codebase — atom-only access - Fix LengthInGuard, LengthComparison, ListLast, ReduceMapPut in lib/ - Disable LengthComparison for test files - Remove obvious comments and narrator comments (~50) - Add missing aliases for fully-qualified modules - Rewrite boilerplate docs in test/support - Add normalize_keys helpers at API boundaries --- .credo.exs | 51 ++++++++++++++++++- lib/aprsme/accounts.ex | 29 ++++++----- lib/aprsme/broadcast_task_supervisor.ex | 1 - lib/aprsme/encoding_utils.ex | 4 +- lib/aprsme/error_notifier.ex | 15 +++--- lib/aprsme/is/is.ex | 1 - lib/aprsme/packet.ex | 50 +++++++++--------- lib/aprsme/packet_consumer.ex | 26 ++++------ lib/aprsme/packets.ex | 16 +++++- lib/aprsme/packets/clustering.ex | 8 +-- lib/aprsme/packets/query_builder.ex | 29 ++++++++--- lib/aprsme/plausible_analytics.ex | 2 +- lib/aprsme_web/aprs_symbol.ex | 6 ++- .../live/components/symbol_renderer.ex | 1 - lib/aprsme_web/live/info_live/show.ex | 7 ++- lib/aprsme_web/live/map_live/components.ex | 2 +- lib/aprsme_web/live/map_live/data_builder.ex | 11 ++-- .../live/map_live/display_manager.ex | 17 +++---- .../live/map_live/historical_loader.ex | 6 +-- lib/aprsme_web/live/map_live/index.ex | 4 +- .../live/map_live/packet_batcher.ex | 4 +- lib/aprsme_web/live/map_live/subscriptions.ex | 9 ++-- .../live/packets_live/callsign_view.ex | 1 - lib/aprsme_web/live/packets_live/index.ex | 14 ++--- .../live/shared/coordinate_utils.ex | 12 ++--- lib/aprsme_web/live/shared/packet_handler.ex | 4 +- lib/aprsme_web/live/shared/packet_utils.ex | 7 ++- lib/aprsme_web/live/status_live/index.ex | 6 +-- .../live/weather_live/callsign_view.ex | 4 +- lib/aprsme_web/plugs/set_locale.ex | 1 - mix.exs | 1 + mix.lock | 1 + test/aprsme/bad_packet_test.exs | 2 - .../aprsme/broadcast_task_supervisor_test.exs | 1 - .../cluster/connection_manager_test.exs | 1 - test/aprsme/device_identification_test.exs | 1 - test/aprsme/is_test.exs | 1 - test/aprsme/migration_lock_test.exs | 1 - test/aprsme/migration_validation_test.exs | 2 +- test/aprsme/packet_consumer_test.exs | 4 +- test/aprsme/packet_parsing_test.exs | 1 - .../aprsme/packet_weather_extraction_test.exs | 2 - test/aprsme/packets/clustering_test.exs | 1 - test/aprsme/packets_mic_e_test.exs | 3 -- test/aprsme/packets_test.exs | 5 +- test/aprsme/packets_weather_test.exs | 6 --- .../workers/packet_cleanup_worker_test.exs | 2 - .../api/v1/weather_controller_test.exs | 5 +- .../integration/aprs_status_test.exs | 2 - .../aprsme_web/live/bad_packets_live_test.exs | 1 - .../live/map_live/display_manager_test.exs | 1 - .../live/map_live/historical_loading_test.exs | 3 +- .../live/map_live/overlay_rendering_test.exs | 6 --- .../aprsme_web/live/map_live/rf_path_test.exs | 1 - .../tracked_callsign_old_packet_test.exs | 3 -- .../live/status_live/index_test.exs | 2 +- .../live/user_settings_live_test.exs | 2 - .../plugs/content_security_policy_test.exs | 1 - test/integration/historical_packets_test.exs | 1 - test/support/channel_case.ex | 17 +++---- test/support/conn_case.ex | 17 +++---- test/support/data_case.ex | 16 +++--- test/support/fixtures/accounts_fixtures.ex | 3 +- test/support/fixtures/packets_fixtures.ex | 3 +- 64 files changed, 237 insertions(+), 230 deletions(-) diff --git a/.credo.exs b/.credo.exs index ec9694a..c4ba55b 100644 --- a/.credo.exs +++ b/.credo.exs @@ -36,7 +36,7 @@ # # Load and configure plugins here: # - plugins: [], + plugins: [{ExSlop, []}], # # If you create your own checks, you must specify the source files for # them here, so they can be loaded by Credo before running the analysis. @@ -158,6 +158,55 @@ {Credo.Check.Warning.UnsafeExec, []}, # + ## ExSlop Checks + # + # Readability + {ExSlop.Check.Readability.BoilerplateDocParams, []}, + {ExSlop.Check.Readability.DocFalseOnPublicFunction, []}, + {ExSlop.Check.Readability.NarratorComment, []}, + {ExSlop.Check.Readability.NarratorDoc, []}, + {ExSlop.Check.Readability.ObviousComment, []}, + {ExSlop.Check.Readability.StepComment, []}, + {ExSlop.Check.Readability.UnaliasedModuleUse, []}, + + # Refactor + {ExSlop.Check.Refactor.CaseTrueFalse, []}, + {ExSlop.Check.Refactor.ExplicitSumReduce, []}, + {ExSlop.Check.Refactor.FilterNil, []}, + {ExSlop.Check.Refactor.FlatMapFilter, []}, + {ExSlop.Check.Refactor.GraphemesLength, []}, + {ExSlop.Check.Refactor.IdentityMap, []}, + {ExSlop.Check.Refactor.IdentityPassthrough, []}, + {ExSlop.Check.Refactor.LengthComparison, [files: %{excluded: [~r|test/|]}]}, + {ExSlop.Check.Refactor.LengthInGuard, []}, + {ExSlop.Check.Refactor.ListFold, []}, + {ExSlop.Check.Refactor.ListLast, []}, + {ExSlop.Check.Refactor.ManualStringReverse, []}, + {ExSlop.Check.Refactor.MapIntoLiteral, []}, + {ExSlop.Check.Refactor.PreferEnumSlice, []}, + {ExSlop.Check.Refactor.ReduceAsMap, []}, + {ExSlop.Check.Refactor.ReduceMapPut, []}, + {ExSlop.Check.Refactor.RedundantBooleanIf, []}, + {ExSlop.Check.Refactor.RedundantEnumJoinSeparator, []}, + {ExSlop.Check.Refactor.RejectNil, []}, + {ExSlop.Check.Refactor.SortForTopK, []}, + {ExSlop.Check.Refactor.SortThenAt, []}, + {ExSlop.Check.Refactor.SortThenReverse, []}, + {ExSlop.Check.Refactor.StringConcatInReduce, []}, + {ExSlop.Check.Refactor.TryRescueWithSafeAlternative, []}, + {ExSlop.Check.Refactor.UseMapJoin, []}, + {ExSlop.Check.Refactor.WithIdentityDo, []}, + {ExSlop.Check.Refactor.WithIdentityElse, []}, + + # Warning + {ExSlop.Check.Warning.BlanketRescue, []}, + {ExSlop.Check.Warning.DualKeyAccess, []}, + {ExSlop.Check.Warning.GenserverAsKvStore, []}, + {ExSlop.Check.Warning.PathExpandPriv, []}, + {ExSlop.Check.Warning.QueryInEnumMap, []}, + {ExSlop.Check.Warning.RepoAllThenFilter, []}, + {ExSlop.Check.Warning.RescueWithoutReraise, []}, + ## Jump.CredoChecks # {Jump.CredoChecks.AssertElementSelectorCanNeverFail, []}, diff --git a/lib/aprsme/accounts.ex b/lib/aprsme/accounts.ex index e6b3d64..4c5c4a8 100644 --- a/lib/aprsme/accounts.ex +++ b/lib/aprsme/accounts.ex @@ -9,6 +9,7 @@ defmodule Aprsme.Accounts do alias Aprsme.Accounts.UserNotifier alias Aprsme.Accounts.UserToken alias Aprsme.Repo + alias Ecto.Multi ## Database getters @@ -168,9 +169,9 @@ defmodule Aprsme.Accounts do |> User.email_changeset(%{email: email}) |> User.confirm_changeset() - Ecto.Multi.new() - |> Ecto.Multi.update(:user, changeset) - |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, [context])) + Multi.new() + |> Multi.update(:user, changeset) + |> Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, [context])) end @doc ~S""" @@ -225,8 +226,8 @@ defmodule Aprsme.Accounts do |> User.callsign_changeset(attrs) |> User.validate_current_password(password) - Ecto.Multi.new() - |> Ecto.Multi.update(:user, changeset) + Multi.new() + |> Multi.update(:user, changeset) |> Repo.transaction() |> transaction_unwrap() end @@ -265,9 +266,9 @@ defmodule Aprsme.Accounts do |> User.password_changeset(attrs) |> User.validate_current_password(password) - Ecto.Multi.new() - |> Ecto.Multi.update(:user, changeset) - |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, :all)) + Multi.new() + |> Multi.update(:user, changeset) + |> Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, :all)) |> Repo.transaction() |> transaction_unwrap() end @@ -344,9 +345,9 @@ defmodule Aprsme.Accounts do end defp confirm_user_multi(user) do - Ecto.Multi.new() - |> Ecto.Multi.update(:user, User.confirm_changeset(user)) - |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, ["confirm"])) + Multi.new() + |> Multi.update(:user, User.confirm_changeset(user)) + |> Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, ["confirm"])) end ## Reset password @@ -404,9 +405,9 @@ defmodule Aprsme.Accounts do """ def reset_user_password(user, attrs) do - Ecto.Multi.new() - |> Ecto.Multi.update(:user, User.password_changeset(user, attrs)) - |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, :all)) + Multi.new() + |> Multi.update(:user, User.password_changeset(user, attrs)) + |> Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, :all)) |> Repo.transaction() |> transaction_unwrap() end diff --git a/lib/aprsme/broadcast_task_supervisor.ex b/lib/aprsme/broadcast_task_supervisor.ex index 12fbaf3..3812084 100644 --- a/lib/aprsme/broadcast_task_supervisor.ex +++ b/lib/aprsme/broadcast_task_supervisor.ex @@ -21,7 +21,6 @@ defmodule Aprsme.BroadcastTaskSupervisor do pool_size = System.schedulers_online() * 2 children = [ - # Create a Task.Supervisor with a specific name {Task.Supervisor, name: @pool_name} ] diff --git a/lib/aprsme/encoding_utils.ex b/lib/aprsme/encoding_utils.ex index 4e3d128..88c678c 100644 --- a/lib/aprsme/encoding_utils.ex +++ b/lib/aprsme/encoding_utils.ex @@ -151,9 +151,7 @@ defmodule Aprsme.EncodingUtils do def sanitize_packet_strings(list) when is_list(list), do: Enum.map(list, &sanitize_packet_strings/1) def sanitize_packet_strings(map) when is_map(map) do - Enum.reduce(map, %{}, fn {key, value}, acc -> - Map.put(acc, key, sanitize_packet_strings(value)) - end) + Map.new(map, fn {key, value} -> {key, sanitize_packet_strings(value)} end) end def sanitize_packet_strings(binary) when is_binary(binary) do diff --git a/lib/aprsme/error_notifier.ex b/lib/aprsme/error_notifier.ex index 0d7316d..cbba3f1 100644 --- a/lib/aprsme/error_notifier.ex +++ b/lib/aprsme/error_notifier.ex @@ -8,6 +8,8 @@ defmodule Aprsme.ErrorNotifier do import Swoosh.Email + alias Phoenix.HTML + require Logger @doc """ @@ -90,19 +92,18 @@ defmodule Aprsme.ErrorNotifier do view = occurrence.context["live_view.view"] || "Unknown view" path = occurrence.context["request.path"] || "Unknown path" - # Get the error URL error_url = "https://aprs.me/dev/error-tracker/errors/#{occurrence.error_id}" # Escape all user-controlled fields to prevent XSS - escaped_error_id = occurrence.error_id |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string() + escaped_error_id = occurrence.error_id |> HTML.html_escape() |> HTML.safe_to_string() escaped_reason = - occurrence.reason |> String.slice(0..199) |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string() + occurrence.reason |> String.slice(0..199) |> HTML.html_escape() |> HTML.safe_to_string() - escaped_location = error_location |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string() - escaped_view = view |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string() - escaped_path = path |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string() - escaped_url = error_url |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string() + escaped_location = error_location |> HTML.html_escape() |> HTML.safe_to_string() + escaped_view = view |> HTML.html_escape() |> HTML.safe_to_string() + escaped_path = path |> HTML.html_escape() |> HTML.safe_to_string() + escaped_url = error_url |> HTML.html_escape() |> HTML.safe_to_string() """
diff --git a/lib/aprsme/is/is.ex b/lib/aprsme/is/is.ex index 59f24cc..ee72db4 100644 --- a/lib/aprsme/is/is.ex +++ b/lib/aprsme/is/is.ex @@ -598,7 +598,6 @@ defmodule Aprsme.Is do def dispatch(message) do case Aprs.parse(message) do {:ok, parsed_message} -> - # Store the packet in the database for future replay # Use GenStage pipeline for efficient batch processing try do # Set received_at and convert struct to map — PacketConsumer.prepare_packet_for_insert diff --git a/lib/aprsme/packet.ex b/lib/aprsme/packet.ex index bb4388d..37565a0 100644 --- a/lib/aprsme/packet.ex +++ b/lib/aprsme/packet.ex @@ -330,7 +330,7 @@ defmodule Aprsme.Packet do """ @spec extract_additional_data(map(), String.t() | nil) :: map() def extract_additional_data(attrs, raw_packet \\ nil) do - data_extended = attrs[:data_extended] || attrs["data_extended"] || %{} + data_extended = attrs[:data_extended] || %{} # Start with the base attributes and add the raw packet base_attrs = Map.put(attrs, :raw_packet, raw_packet) @@ -373,8 +373,8 @@ defmodule Aprsme.Packet do defp add_symbol_data(base_attrs, attrs) do base_attrs - |> maybe_put(:symbol_code, attrs[:symbol_code] || attrs["symbol_code"]) - |> maybe_put(:symbol_table_id, attrs[:symbol_table_id] || attrs["symbol_table_id"]) + |> maybe_put(:symbol_code, attrs[:symbol_code]) + |> maybe_put(:symbol_table_id, attrs[:symbol_table_id]) end defp merge_extracted_data(base_attrs, additional_data, attrs) do @@ -439,7 +439,7 @@ defmodule Aprsme.Packet do data_extended = Map.delete(data_extended, "raw_weather_data") # Handle nested data_extended structures - nested_data_extended = data_extended[:data_extended] || data_extended["data_extended"] + nested_data_extended = data_extended[:data_extended] # Merge data from both levels combined_data = @@ -469,24 +469,24 @@ defmodule Aprsme.Packet do defp put_symbol_fields(map, data_extended) do # First try to get symbol data from the data_extended map - symbol_code = data_extended[:symbol_code] || data_extended["symbol_code"] - symbol_table_id = data_extended[:symbol_table_id] || data_extended["symbol_table_id"] + symbol_code = data_extended[:symbol_code] + symbol_table_id = data_extended[:symbol_table_id] # Clean comment by removing altitude and PHG data - comment = clean_comment(data_extended[:comment] || data_extended["comment"]) + comment = clean_comment(data_extended[:comment]) map |> maybe_put(:symbol_code, symbol_code) |> maybe_put(:symbol_table_id, symbol_table_id) |> maybe_put(:comment, comment) - |> maybe_put(:timestamp, data_extended[:timestamp] || data_extended["timestamp"]) + |> maybe_put(:timestamp, data_extended[:timestamp]) |> maybe_put( :aprs_messaging, - data_extended[:aprs_messaging?] || data_extended["aprs_messaging?"] + data_extended[:aprs_messaging?] ) - |> maybe_put(:position_ambiguity, data_extended[:position_ambiguity] || data_extended["position_ambiguity"]) - |> maybe_put(:posresolution, data_extended[:posresolution] || data_extended["posresolution"]) - |> maybe_put(:format, normalize_format_field(data_extended[:format] || data_extended["format"])) + |> maybe_put(:position_ambiguity, data_extended[:position_ambiguity]) + |> maybe_put(:posresolution, data_extended[:posresolution]) + |> maybe_put(:format, normalize_format_field(data_extended[:format])) end defp put_weather_fields(map, data_extended) do @@ -561,25 +561,25 @@ defmodule Aprsme.Packet do defp put_message_fields(map, data_extended) do map - |> maybe_put(:addressee, data_extended[:addressee] || data_extended["addressee"]) - |> maybe_put(:message_text, data_extended[:message_text] || data_extended["message_text"]) + |> maybe_put(:addressee, data_extended[:addressee]) + |> maybe_put(:message_text, data_extended[:message_text]) |> maybe_put( :message_number, - data_extended[:message_number] || data_extended["message_number"] + data_extended[:message_number] ) end defp put_telemetry_fields(map, data_extended) do - telemetry = data_extended[:telemetry] || data_extended["telemetry"] || data_extended + telemetry = data_extended[:telemetry] || data_extended map |> maybe_put(:telemetry_seq, parse_telemetry_seq(telemetry)) |> maybe_put(:telemetry_vals, parse_telemetry_vals(telemetry)) - |> maybe_put(:telemetry_bits, telemetry[:bits] || telemetry["bits"]) + |> maybe_put(:telemetry_bits, telemetry[:bits]) end defp parse_telemetry_seq(telemetry) do - case telemetry[:seq] || telemetry["seq"] do + case telemetry[:seq] do nil -> nil seq when is_integer(seq) -> seq seq when is_binary(seq) -> parse_integer_string(seq) @@ -588,7 +588,7 @@ defmodule Aprsme.Packet do end defp parse_telemetry_vals(telemetry) do - case telemetry[:vals] || telemetry["vals"] do + case telemetry[:vals] do nil -> nil vals when is_list(vals) -> Enum.map(vals, &normalize_telemetry_value/1) _ -> nil @@ -637,8 +637,8 @@ defmodule Aprsme.Packet do |> maybe_put(:course, mic_e_map[:heading]) |> maybe_put(:speed, mic_e_map[:speed]) # Use symbol data from MicE if available, otherwise use default car symbol - |> maybe_put(:symbol_code, mic_e_map[:symbol_code] || mic_e_map["symbol_code"] || ">") - |> maybe_put(:symbol_table_id, mic_e_map[:symbol_table_id] || mic_e_map["symbol_table_id"] || "/") + |> maybe_put(:symbol_code, mic_e_map[:symbol_code] || ">") + |> maybe_put(:symbol_table_id, mic_e_map[:symbol_table_id] || "/") end # Extract weather data from various formats, including new wx field @@ -646,16 +646,16 @@ defmodule Aprsme.Packet do weather_data = find_weather_data(data_extended) # Check for dedicated wx field from improved parser - wx_data = data_extended[:wx] || data_extended["wx"] + wx_data = data_extended[:wx] final_weather_data = wx_data || weather_data process_weather_data(attrs, final_weather_data) end defp find_weather_data(data_extended) do - data_extended[:weather] || data_extended["weather"] || - data_extended[:weather_report] || data_extended["weather_report"] || - data_extended[:raw_weather_data] || data_extended["raw_weather_data"] + data_extended[:weather] || + data_extended[:weather_report] || + data_extended[:raw_weather_data] end defp process_weather_data(attrs, weather_data) do diff --git a/lib/aprsme/packet_consumer.ex b/lib/aprsme/packet_consumer.ex index d77db41..1f0391d 100644 --- a/lib/aprsme/packet_consumer.ex +++ b/lib/aprsme/packet_consumer.ex @@ -8,6 +8,7 @@ defmodule Aprsme.PacketConsumer do alias Aprsme.Cluster.PacketDistributor alias Aprsme.LogSanitizer alias Aprsme.Repo + alias Phoenix.PubSub require Logger @@ -386,14 +387,14 @@ defmodule Aprsme.PacketConsumer do # Replaces the `aprs_packets` pg_notify path (removed as a DB trigger). defp broadcast_legacy_topics(packet, routing_callsign, has_weather?) do # Hot path: PubSub.broadcast failures tolerated (logged by PubSub itself). - _ = Phoenix.PubSub.broadcast(Aprsme.PubSub, "postgres:aprsme_packets", {:postgres_packet, packet}) + _ = PubSub.broadcast(Aprsme.PubSub, "postgres:aprsme_packets", {:postgres_packet, packet}) _ = if is_binary(routing_callsign) and routing_callsign != "" do - _ = Phoenix.PubSub.broadcast(Aprsme.PubSub, "packets:#{routing_callsign}", {:postgres_packet, packet}) + _ = PubSub.broadcast(Aprsme.PubSub, "packets:#{routing_callsign}", {:postgres_packet, packet}) if has_weather? do - Phoenix.PubSub.broadcast(Aprsme.PubSub, "weather:#{routing_callsign}", {:weather_packet, packet}) + PubSub.broadcast(Aprsme.PubSub, "weather:#{routing_callsign}", {:weather_packet, packet}) end end @@ -550,7 +551,7 @@ defmodule Aprsme.PacketConsumer do end defp apply_item_fields(attrs) do - item_name = Map.get(attrs, :itemname) || Map.get(attrs, "itemname") + item_name = Map.get(attrs, :itemname) attrs |> Map.put(:item_name, item_name) @@ -583,7 +584,7 @@ defmodule Aprsme.PacketConsumer do attrs |> then(fn a -> # Convert aprs_messaging? to aprs_messaging - case Map.get(a, :aprs_messaging?) || Map.get(a, "aprs_messaging?") do + case Map.get(a, :aprs_messaging?) do nil -> a @@ -691,19 +692,13 @@ defmodule Aprsme.PacketConsumer do end defp extract_lat_from_ext_map(ext_map) do - ext_map[:latitude] || ext_map["latitude"] || - (Map.has_key?(ext_map, :position) && - (ext_map[:position][:latitude] || ext_map[:position]["latitude"])) || - (Map.has_key?(ext_map, "position") && - (ext_map["position"][:latitude] || ext_map["position"]["latitude"])) + ext_map[:latitude] || + (Map.has_key?(ext_map, :position) && ext_map[:position][:latitude]) end defp extract_lon_from_ext_map(ext_map) do - ext_map[:longitude] || ext_map["longitude"] || - (Map.has_key?(ext_map, :position) && - (ext_map[:position][:longitude] || ext_map[:position]["longitude"])) || - (Map.has_key?(ext_map, "position") && - (ext_map["position"][:longitude] || ext_map["position"]["longitude"])) + ext_map[:longitude] || + (Map.has_key?(ext_map, :position) && ext_map[:position][:longitude]) end # extract_position already ran to_float; lat/lon are floats or nil here. @@ -719,7 +714,6 @@ defmodule Aprsme.PacketConsumer do defp round_coord(n) when is_float(n), do: Float.round(n, 6) defp round_coord(_), do: nil - # Build the PostGIS point only for in-range coordinates. # Must run AFTER sanitize_packet_strings so the struct survives intact. defp create_location_geometry(%{lat: lat, lon: lon} = attrs) when is_float(lat) and is_float(lon) and lat >= -90.0 and lat <= 90.0 and lon >= -180.0 and lon <= 180.0 do diff --git a/lib/aprsme/packets.ex b/lib/aprsme/packets.ex index 6fdbc73..6912109 100644 --- a/lib/aprsme/packets.ex +++ b/lib/aprsme/packets.ex @@ -24,6 +24,8 @@ defmodule Aprsme.Packets do """ @spec store_packet(map()) :: {:ok, struct()} | {:error, :validation_error | :storage_exception} def store_packet(packet_data) do + packet_data = normalize_packet_keys(packet_data) + with :ok <- validate_sender(packet_data), {:ok, sanitized_data} <- sanitize_packet_data(packet_data), {:ok, packet_attrs} <- build_packet_attrs(sanitized_data), @@ -48,7 +50,7 @@ defmodule Aprsme.Packets do end defp validate_sender(packet_data) when is_map(packet_data) do - sender = Map.get(packet_data, :sender) || Map.get(packet_data, "sender") + sender = Map.get(packet_data, :sender) if Aprsme.Callsign.valid?(sender) do :ok @@ -59,6 +61,16 @@ defmodule Aprsme.Packets do defp validate_sender(_packet_data), do: {:error, {:validation_error, "sender: must be a safe APRS identifier"}} + # Normalize string keys to atoms for consistent access at the API boundary + defp normalize_packet_keys(packet_data) when is_map(packet_data) do + Map.new(packet_data, fn + {key, value} when is_atom(key) -> {key, value} + {key, value} when is_binary(key) -> {String.to_existing_atom(key), value} + end) + end + + defp normalize_packet_keys(packet_data), do: packet_data + # Pattern matching for sanitization defp sanitize_packet_data(packet_data) do {:ok, Aprsme.EncodingUtils.sanitize_packet(packet_data)} @@ -393,7 +405,7 @@ defmodule Aprsme.Packets do defp maybe_filter_by_callsign(query, _), do: query - defp maybe_filter_by_bounds(query, bounds) when is_list(bounds) and length(bounds) == 4 do + defp maybe_filter_by_bounds(query, [_, _, _, _] = bounds) do QueryBuilder.within_bounds(query, bounds) end diff --git a/lib/aprsme/packets/clustering.ex b/lib/aprsme/packets/clustering.ex index c3f8f6c..887712f 100644 --- a/lib/aprsme/packets/clustering.ex +++ b/lib/aprsme/packets/clustering.ex @@ -69,8 +69,8 @@ defmodule Aprsme.Packets.Clustering do # Filter packets with valid lat/lon coordinates defp filter_valid_coordinates(packets) do Enum.filter(packets, fn packet -> - lat = decimal_to_float(Map.get(packet, :lat) || Map.get(packet, "lat")) - lon = decimal_to_float(Map.get(packet, :lon) || Map.get(packet, "lon")) + lat = decimal_to_float(Map.get(packet, :lat)) + lon = decimal_to_float(Map.get(packet, :lon)) not is_nil(lat) and not is_nil(lon) and lat >= -90 and lat <= 90 and @@ -96,8 +96,8 @@ defmodule Aprsme.Packets.Clustering do defp perform_clustering(packets, radius) do packets |> Enum.reduce(%{}, fn packet, clusters -> - lat = decimal_to_float(Map.get(packet, :lat) || Map.get(packet, "lat")) - lon = decimal_to_float(Map.get(packet, :lon) || Map.get(packet, "lon")) + lat = decimal_to_float(Map.get(packet, :lat)) + lon = decimal_to_float(Map.get(packet, :lon)) # Find cluster key by rounding to grid cluster_lat = Float.round(lat / radius) * radius diff --git a/lib/aprsme/packets/query_builder.ex b/lib/aprsme/packets/query_builder.ex index ea8e7c5..6c1ca65 100644 --- a/lib/aprsme/packets/query_builder.ex +++ b/lib/aprsme/packets/query_builder.ex @@ -8,6 +8,16 @@ defmodule Aprsme.Packets.QueryBuilder do alias Aprsme.Packet + # Normalize string keys to atoms for consistent access at the API boundary + defp normalize_opts_keys(opts) when is_map(opts) do + Map.new(opts, fn + {key, value} when is_atom(key) -> {key, value} + {key, value} when is_binary(key) -> {String.to_existing_atom(key), value} + end) + end + + defp normalize_opts_keys(opts), do: opts + @doc """ Filters query by time range. Supports start_time, end_time, and hours_back options. @@ -22,10 +32,12 @@ defmodule Aprsme.Packets.QueryBuilder do end def with_time_range(query, opts) when is_map(opts) do + opts = normalize_opts_keys(opts) + query - |> maybe_filter_start_time(opts[:start_time] || opts["start_time"]) - |> maybe_filter_end_time(opts[:end_time] || opts["end_time"]) - |> maybe_filter_hours_back(opts[:hours_back] || opts["hours_back"]) + |> maybe_filter_start_time(opts[:start_time]) + |> maybe_filter_end_time(opts[:end_time]) + |> maybe_filter_hours_back(opts[:hours_back]) end @doc """ @@ -182,7 +194,8 @@ defmodule Aprsme.Packets.QueryBuilder do """ @spec recent_position_packets(map() | keyword()) :: Ecto.Query.t() def recent_position_packets(opts \\ %{}) do - limit = opts[:limit] || opts["limit"] || 100 + opts = normalize_opts_keys(opts) + limit = opts[:limit] || 100 from(p in Packet) |> with_position() @@ -203,7 +216,8 @@ defmodule Aprsme.Packets.QueryBuilder do """ @spec callsign_history(String.t(), map() | keyword()) :: Ecto.Query.t() def callsign_history(callsign, opts \\ %{}) do - limit = opts[:limit] || opts["limit"] || 100 + opts = normalize_opts_keys(opts) + limit = opts[:limit] || 100 from(p in Packet) |> for_callsign(callsign) @@ -223,8 +237,9 @@ defmodule Aprsme.Packets.QueryBuilder do """ @spec weather_packets(map() | keyword()) :: Ecto.Query.t() def weather_packets(opts \\ %{}) do - limit = opts[:limit] || opts["limit"] || 100 - callsign = opts[:callsign] || opts["callsign"] + opts = normalize_opts_keys(opts) + limit = opts[:limit] || 100 + callsign = opts[:callsign] query = from(p in Packet) diff --git a/lib/aprsme/plausible_analytics.ex b/lib/aprsme/plausible_analytics.ex index 26f0145..f3fd712 100644 --- a/lib/aprsme/plausible_analytics.ex +++ b/lib/aprsme/plausible_analytics.ex @@ -61,7 +61,7 @@ defmodule Aprsme.PlausibleAnalytics do headers = maybe_add_forwarded_for( [{"content-type", "application/json"}, {"user-agent", Map.get(metadata, :user_agent, "aprs.me-server")}], - Map.get(metadata, :remote_ip) || Map.get(metadata, "remote_ip") + Map.get(metadata, :remote_ip) ) case Req.post(config[:endpoint], json: body, headers: headers, max_retries: 0, receive_timeout: 2_000) do diff --git a/lib/aprsme_web/aprs_symbol.ex b/lib/aprsme_web/aprs_symbol.ex index 844279c..2d1d348 100644 --- a/lib/aprsme_web/aprs_symbol.ex +++ b/lib/aprsme_web/aprs_symbol.ex @@ -12,6 +12,8 @@ defmodule AprsmeWeb.AprsSymbol do across the application. """ + alias Aprsme.Cache + @doc """ Gets sprite information for a given symbol table and code. Returns a map with sprite_file, background_position, and background_size. @@ -177,14 +179,14 @@ defmodule AprsmeWeb.AprsSymbol do if is_nil(callsign) do cache_key = "symbol_html:#{symbol_table}:#{symbol_code}:#{size}" - case Aprsme.Cache.get(:symbol_cache, cache_key) do + case Cache.get(:symbol_cache, cache_key) do {:ok, html} when not is_nil(html) -> html _ -> html = generate_marker_html(symbol_table, symbol_code, nil, size) # Cache for 1 hour since symbols don't change - _ = Aprsme.Cache.put(:symbol_cache, cache_key, html, ttl: Aprsme.Cache.to_timeout(hour: 1)) + _ = Cache.put(:symbol_cache, cache_key, html, ttl: Cache.to_timeout(hour: 1)) html end else diff --git a/lib/aprsme_web/live/components/symbol_renderer.ex b/lib/aprsme_web/live/components/symbol_renderer.ex index bbfd9b5..0b5711a 100644 --- a/lib/aprsme_web/live/components/symbol_renderer.ex +++ b/lib/aprsme_web/live/components/symbol_renderer.ex @@ -24,7 +24,6 @@ defmodule AprsmeWeb.SymbolRenderer do attr :title, :string, default: nil, doc: "Optional title/tooltip text" def symbol(assigns) do - # Get the sprite file and position for this symbol sprite_info = AprsmeWeb.AprsSymbol.get_sprite_info(assigns.symbol_table, assigns.symbol_code) assigns = diff --git a/lib/aprsme_web/live/info_live/show.ex b/lib/aprsme_web/live/info_live/show.ex index af1ac6a..879a278 100644 --- a/lib/aprsme_web/live/info_live/show.ex +++ b/lib/aprsme_web/live/info_live/show.ex @@ -15,6 +15,7 @@ defmodule AprsmeWeb.InfoLive.Show do alias AprsmeWeb.AprsSymbol alias AprsmeWeb.Live.SharedPacketHandler alias AprsmeWeb.MapLive.PacketUtils + alias Phoenix.HTML require Logger @@ -120,7 +121,6 @@ defmodule AprsmeWeb.InfoLive.Show do "InfoLive received packet update for #{socket.assigns.callsign}: #{inspect(Map.get(incoming_packet, "raw_packet"))}" ) - # Get the new packet data new_packet = get_latest_packet(socket.assigns.callsign) new_packet = if new_packet, do: SharedPacketHandler.enrich_with_device_info(new_packet) @@ -197,7 +197,6 @@ defmodule AprsmeWeb.InfoLive.Show do end defp get_latest_position_packet(callsign) do - # Get the most recent packet with valid position data Repo.one( from(p in Aprsme.Packet, where: fragment("upper(?)", p.sender) == ^String.upcase(String.trim(callsign)), @@ -544,12 +543,12 @@ defmodule AprsmeWeb.InfoLive.Show do style = "position: relative; width: #{size}px; height: #{size}px; background-image: url(#{overlay_sprite_info.sprite_file}), url(#{sprite_info.sprite_file}); background-position: #{overlay_sprite_info.background_position}, #{sprite_info.background_position}; background-size: #{overlay_sprite_info.background_size}, #{sprite_info.background_size}; background-repeat: no-repeat, no-repeat; image-rendering: pixelated; display: inline-block; vertical-align: middle; margin-bottom: -6px;" - escaped_style = style |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string() + escaped_style = style |> HTML.html_escape() |> HTML.safe_to_string() raw("
") else style = AprsSymbol.render_style(symbol_table_id, symbol_code, size) - escaped_style = style |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string() + escaped_style = style |> HTML.html_escape() |> HTML.safe_to_string() raw("
") end diff --git a/lib/aprsme_web/live/map_live/components.ex b/lib/aprsme_web/live/map_live/components.ex index cb9dd4b..aa10678 100644 --- a/lib/aprsme_web/live/map_live/components.ex +++ b/lib/aprsme_web/live/map_live/components.ex @@ -365,7 +365,7 @@ defmodule AprsmeWeb.MapLive.Components do defp get_received_at(packet) when is_map(packet) do # For regular maps, try both atom and string keys - Map.get(packet, :received_at) || Map.get(packet, "received_at") + Map.get(packet, :received_at) end defp get_received_at(_), do: nil diff --git a/lib/aprsme_web/live/map_live/data_builder.ex b/lib/aprsme_web/live/map_live/data_builder.ex index acead62..cd58494 100644 --- a/lib/aprsme_web/live/map_live/data_builder.ex +++ b/lib/aprsme_web/live/map_live/data_builder.ex @@ -17,6 +17,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do proper coordinate validation, symbol rendering, and weather data processing. """ + alias Aprsme.WeatherCache alias AprsmeWeb.Live.Shared.CoordinateUtils alias AprsmeWeb.Live.Shared.PacketUtils, as: SharedPacketUtils alias AprsmeWeb.Live.Shared.ParamUtils @@ -41,7 +42,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do |> Enum.map(&display_name/1) |> Enum.uniq() - Aprsme.WeatherCache.cache_weather_callsigns(callsigns) + WeatherCache.cache_weather_callsigns(callsigns) packets_map |> Enum.map(fn {_callsign, packet} -> @@ -499,10 +500,10 @@ defmodule AprsmeWeb.MapLive.DataBuilder do @spec has_weather_packets?(String.t()) :: boolean() defp has_weather_packets?(callsign) when is_binary(callsign) do - case Aprsme.WeatherCache.weather_callsign?(callsign) do + case WeatherCache.weather_callsign?(callsign) do :unknown -> - Aprsme.WeatherCache.cache_weather_callsigns([callsign]) - Aprsme.WeatherCache.weather_callsign?(callsign) == true + WeatherCache.cache_weather_callsigns([callsign]) + WeatherCache.weather_callsign?(callsign) == true result when is_boolean(result) -> result @@ -584,7 +585,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do # Helper to get packet ID @spec get_packet_id(map()) :: any() defp get_packet_id(packet) do - Map.get(packet, :id) || Map.get(packet, "id") + Map.get(packet, :id) end # Helper to get packet received_at datetime diff --git a/lib/aprsme_web/live/map_live/display_manager.ex b/lib/aprsme_web/live/map_live/display_manager.ex index 8a93643..1fc3e4e 100644 --- a/lib/aprsme_web/live/map_live/display_manager.ex +++ b/lib/aprsme_web/live/map_live/display_manager.ex @@ -60,7 +60,7 @@ defmodule AprsmeWeb.MapLive.DisplayManager do all_packets |> BoundsUtils.filter_packets_by_bounds(socket.assigns.map_bounds) |> Enum.uniq_by(fn packet -> - Map.get(packet, :id) || Map.get(packet, "id") + Map.get(packet, :id) end) send_heat_map_for_packets(socket, filtered_packets) @@ -170,7 +170,7 @@ defmodule AprsmeWeb.MapLive.DisplayManager do end defp tracked_packet?(packet, callsign, threshold) do - sender = Map.get(packet, :sender) || Map.get(packet, "sender") + sender = Map.get(packet, :sender) received_at = packet_received_at(packet) String.upcase(sender) == String.upcase(callsign) && @@ -185,8 +185,8 @@ defmodule AprsmeWeb.MapLive.DisplayManager do end defp trail_point(packet) do - lat = Map.get(packet, :lat) || Map.get(packet, "lat") - lon = Map.get(packet, :lon) || Map.get(packet, "lon") + lat = Map.get(packet, :lat) + lon = Map.get(packet, :lon) received_at = packet_received_at(packet) %{ @@ -197,7 +197,7 @@ defmodule AprsmeWeb.MapLive.DisplayManager do end defp packet_received_at(packet) do - Map.get(packet, :received_at) || Map.get(packet, "received_at") + Map.get(packet, :received_at) end defp packet_timestamp(packet) do @@ -208,11 +208,10 @@ defmodule AprsmeWeb.MapLive.DisplayManager do defp trail_packet_key(packet) do Map.get(packet, :id) || - Map.get(packet, "id") || { - Map.get(packet, :sender) || Map.get(packet, "sender"), - Map.get(packet, :lat) || Map.get(packet, "lat"), - Map.get(packet, :lon) || Map.get(packet, "lon"), + Map.get(packet, :sender), + Map.get(packet, :lat), + Map.get(packet, :lon), packet_received_at(packet) } end diff --git a/lib/aprsme_web/live/map_live/historical_loader.ex b/lib/aprsme_web/live/map_live/historical_loader.ex index 3d6d26d..34aade4 100644 --- a/lib/aprsme_web/live/map_live/historical_loader.ex +++ b/lib/aprsme_web/live/map_live/historical_loader.ex @@ -220,11 +220,11 @@ defmodule AprsmeWeb.MapLive.HistoricalLoader do # Extract the next keyset cursor from the last packet in the list. defp advance_historical_cursor(socket, packets) do - case List.last(packets) do - nil -> + case Enum.reverse(packets) do + [] -> socket - last -> + [last | _] -> received_at = get_cursor_field(last, :received_at) id = get_cursor_field(last, :id) diff --git a/lib/aprsme_web/live/map_live/index.ex b/lib/aprsme_web/live/map_live/index.ex index 5ed4f37..d51c525 100644 --- a/lib/aprsme_web/live/map_live/index.ex +++ b/lib/aprsme_web/live/map_live/index.ex @@ -335,7 +335,6 @@ defmodule AprsmeWeb.MapLive.Index do def handle_info({:process_pending_bounds}, socket) do if socket.assigns.pending_bounds && !socket.assigns.historical_loading do - # Process the pending bounds update bounds = socket.assigns.pending_bounds socket = assign(socket, pending_bounds: nil) BoundsUpdater.handle_bounds_update(bounds, socket) @@ -452,7 +451,6 @@ defmodule AprsmeWeb.MapLive.Index do packet_sender = Map.get(packet, :sender, Map.get(packet, "sender", "")) if String.upcase(packet_sender) == String.upcase(socket.assigns.tracked_callsign) do - # Update the tracked callsign's latest packet socket = assign( socket, @@ -1191,7 +1189,7 @@ defmodule AprsmeWeb.MapLive.Index do end defp packet_received_at(packet) do - Map.get(packet, :received_at) || Map.get(packet, "received_at") + Map.get(packet, :received_at) end @impl true diff --git a/lib/aprsme_web/live/map_live/packet_batcher.ex b/lib/aprsme_web/live/map_live/packet_batcher.ex index ab7a950..6d7f583 100644 --- a/lib/aprsme_web/live/map_live/packet_batcher.ex +++ b/lib/aprsme_web/live/map_live/packet_batcher.ex @@ -121,8 +121,8 @@ defmodule AprsmeWeb.MapLive.PacketBatcher do end defp accept_packet(state, packet) do - sender = Map.get(packet, :sender) || Map.get(packet, "sender") - received_at = Map.get(packet, :received_at) || Map.get(packet, "received_at") + sender = Map.get(packet, :sender) + received_at = Map.get(packet, :received_at) with true <- is_binary(sender), false <- is_nil(received_at), diff --git a/lib/aprsme_web/live/map_live/subscriptions.ex b/lib/aprsme_web/live/map_live/subscriptions.ex index 9c0aa16..65e05f5 100644 --- a/lib/aprsme_web/live/map_live/subscriptions.ex +++ b/lib/aprsme_web/live/map_live/subscriptions.ex @@ -8,6 +8,7 @@ defmodule AprsmeWeb.MapLive.Subscriptions do import Phoenix.LiveView, only: [connected?: 1, push_event: 3, put_flash: 3] alias Phoenix.LiveView.Socket + alias Phoenix.PubSub @doc """ Set up spatial and PubSub subscriptions for a newly mounted socket. @@ -61,20 +62,20 @@ defmodule AprsmeWeb.MapLive.Subscriptions do if Application.get_env(:aprsme, :cluster_enabled, false) do _ = Aprsme.ConnectionMonitor.register_connection() # Subscribe to drain events for this node - Phoenix.PubSub.subscribe(Aprsme.PubSub, "connection:drain:#{Node.self()}") + PubSub.subscribe(Aprsme.PubSub, "connection:drain:#{Node.self()}") end # Register the actual initial viewport. {:ok, spatial_topic} = Aprsme.SpatialPubSub.register_viewport(client_id, initial_bounds) # Subscribe to the spatial topic for this client - :ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, spatial_topic) + :ok = PubSub.subscribe(Aprsme.PubSub, spatial_topic) # Still subscribe to bad packets (they don't have location) - :ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, "bad_packets") + :ok = PubSub.subscribe(Aprsme.PubSub, "bad_packets") # Subscribe to deployment events - :ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, "deployment_events") + :ok = PubSub.subscribe(Aprsme.PubSub, "deployment_events") _ = Process.send_after(self(), :cleanup_old_packets, 60_000) diff --git a/lib/aprsme_web/live/packets_live/callsign_view.ex b/lib/aprsme_web/live/packets_live/callsign_view.ex index a3312c4..1487e2d 100644 --- a/lib/aprsme_web/live/packets_live/callsign_view.ex +++ b/lib/aprsme_web/live/packets_live/callsign_view.ex @@ -108,7 +108,6 @@ defmodule AprsmeWeb.PacketsLive.CallsignView do defp enrich_packet_with_device_identifier(sanitized_payload) do device_identifier = Map.get(sanitized_payload, :device_identifier) || - Map.get(sanitized_payload, "device_identifier") || DeviceParser.extract_device_identifier(sanitized_payload) canonical_identifier = resolve_canonical_identifier(device_identifier) diff --git a/lib/aprsme_web/live/packets_live/index.ex b/lib/aprsme_web/live/packets_live/index.ex index 02f6eec..11bb4e4 100644 --- a/lib/aprsme_web/live/packets_live/index.ex +++ b/lib/aprsme_web/live/packets_live/index.ex @@ -77,14 +77,16 @@ defmodule AprsmeWeb.PacketsLive.Index do |> do_trim(socket) end - defp do_trim(packets, socket) when length(packets) <= @max_stream_size, do: socket - defp do_trim(packets, socket) do - {_kept, to_remove} = Enum.split(packets, @max_stream_size) + if length(packets) <= @max_stream_size do + socket + else + {_kept, to_remove} = Enum.split(packets, @max_stream_size) - Enum.reduce(to_remove, socket, fn {id, _}, s -> - stream_delete(s, :packets, id) - end) + Enum.reduce(to_remove, socket, fn {id, _}, s -> + stream_delete(s, :packets, id) + end) + end end @doc """ diff --git a/lib/aprsme_web/live/shared/coordinate_utils.ex b/lib/aprsme_web/live/shared/coordinate_utils.ex index 40a4329..a1c42c3 100644 --- a/lib/aprsme_web/live/shared/coordinate_utils.ex +++ b/lib/aprsme_web/live/shared/coordinate_utils.ex @@ -23,9 +23,9 @@ defmodule AprsmeWeb.Live.Shared.CoordinateUtils do end def get_coordinates(packet) do - lat = Map.get(packet, :lat) || Map.get(packet, "lat") - lon = Map.get(packet, :lon) || Map.get(packet, "lon") - data_extended = Map.get(packet, :data_extended) || Map.get(packet, "data_extended") + lat = Map.get(packet, :lat) + lon = Map.get(packet, :lon) + data_extended = Map.get(packet, :data_extended) {lat, lon, data_extended} end @@ -64,8 +64,8 @@ defmodule AprsmeWeb.Live.Shared.CoordinateUtils do """ @spec has_position_data?(map()) :: boolean() def has_position_data?(packet) do - lat = Map.get(packet, :lat) || Map.get(packet, "lat") - lon = Map.get(packet, :lon) || Map.get(packet, "lon") + lat = Map.get(packet, :lat) + lon = Map.get(packet, :lon) has_direct_coordinates?(lat, lon) or has_position_in_data_extended?(packet) end @@ -130,7 +130,7 @@ defmodule AprsmeWeb.Live.Shared.CoordinateUtils do defp has_direct_coordinates?(_, _), do: false defp has_position_in_data_extended?(packet) do - data_extended = Map.get(packet, :data_extended) || Map.get(packet, "data_extended") + data_extended = Map.get(packet, :data_extended) has_position_in_data_extended_case?(data_extended) end diff --git a/lib/aprsme_web/live/shared/packet_handler.ex b/lib/aprsme_web/live/shared/packet_handler.ex index 9972e2f..96b8946 100644 --- a/lib/aprsme_web/live/shared/packet_handler.ex +++ b/lib/aprsme_web/live/shared/packet_handler.ex @@ -37,7 +37,7 @@ defmodule AprsmeWeb.Live.SharedPacketHandler do Checks if packet sender matches the given callsign. """ def packet_matches_callsign?(packet, callsign) do - packet_sender = Map.get(packet, "sender") || Map.get(packet, :sender, "") + packet_sender = Map.get(packet, :sender, "") Callsign.matches?(packet_sender, callsign) end @@ -102,7 +102,7 @@ defmodule AprsmeWeb.Live.SharedPacketHandler do end defp get_device_identifier(packet) do - Map.get(packet, :device_identifier) || Map.get(packet, "device_identifier") + Map.get(packet, :device_identifier) end defp lookup_device_info(nil), do: nil diff --git a/lib/aprsme_web/live/shared/packet_utils.ex b/lib/aprsme_web/live/shared/packet_utils.ex index c87025a..d7f8c7e 100644 --- a/lib/aprsme_web/live/shared/packet_utils.ex +++ b/lib/aprsme_web/live/shared/packet_utils.ex @@ -100,7 +100,7 @@ defmodule AprsmeWeb.Live.Shared.PacketUtils do """ @spec has_weather_data?(map()) :: boolean() def has_weather_data?(packet) do - case Map.get(packet, :has_weather) || Map.get(packet, "has_weather") do + case Map.get(packet, :has_weather) do nil -> Enum.any?(Aprsme.EncodingUtils.weather_fields(), fn field -> value = get_packet_field(packet, field, nil) @@ -135,8 +135,7 @@ defmodule AprsmeWeb.Live.Shared.PacketUtils do case Map.get(packet, to_string(field)) do nil -> de = - Map.get(packet, :data_extended) || - Map.get(packet, "data_extended", %{}) || %{} + Map.get(packet, :data_extended) || %{} Map.get(de, field) || Map.get(de, to_string(field)) || default @@ -190,7 +189,7 @@ defmodule AprsmeWeb.Live.Shared.PacketUtils do """ @spec get_timestamp(map()) :: String.t() def get_timestamp(packet) do - received_at = Map.get(packet, :received_at) || Map.get(packet, "received_at") + received_at = Map.get(packet, :received_at) case received_at do nil -> diff --git a/lib/aprsme_web/live/status_live/index.ex b/lib/aprsme_web/live/status_live/index.ex index 3cae915..10e82fc 100644 --- a/lib/aprsme_web/live/status_live/index.ex +++ b/lib/aprsme_web/live/status_live/index.ex @@ -4,6 +4,7 @@ defmodule AprsmeWeb.StatusLive.Index do """ use AprsmeWeb, :live_view + alias Aprsme.Cache alias Aprsme.Cluster.LeaderElection require Logger @@ -409,20 +410,19 @@ defmodule AprsmeWeb.StatusLive.Index do require Logger Logger.error("Error getting APRS status: #{inspect(error)}") - # Return a default status when database is unavailable default_status() end defp get_cached_aprs_status do # Try to get cached status for instant load - case Aprsme.Cache.get(:query_cache, "aprs_status") do + case Cache.get(:query_cache, "aprs_status") do {:ok, status} when not is_nil(status) -> status _ -> # Fallback to direct query if cache miss status = get_aprs_status() - _ = Aprsme.Cache.put(:query_cache, "aprs_status", status, ttl: Aprsme.Cache.to_timeout(second: 10)) + _ = Cache.put(:query_cache, "aprs_status", status, ttl: Cache.to_timeout(second: 10)) status end end diff --git a/lib/aprsme_web/live/weather_live/callsign_view.ex b/lib/aprsme_web/live/weather_live/callsign_view.ex index 4020699..d12b54d 100644 --- a/lib/aprsme_web/live/weather_live/callsign_view.ex +++ b/lib/aprsme_web/live/weather_live/callsign_view.ex @@ -92,8 +92,8 @@ defmodule AprsmeWeb.WeatherLive.CallsignView do defp should_update_weather?(nil, _new_packet), do: true defp should_update_weather?(current_packet, new_packet) do - new_received_at = Map.get(new_packet, :received_at) || Map.get(new_packet, "received_at") - current_received_at = Map.get(current_packet, :received_at) || Map.get(current_packet, "received_at") + new_received_at = Map.get(new_packet, :received_at) + current_received_at = Map.get(current_packet, :received_at) compare_timestamps(new_received_at, current_received_at) end diff --git a/lib/aprsme_web/plugs/set_locale.ex b/lib/aprsme_web/plugs/set_locale.ex index d56b308..811f01b 100644 --- a/lib/aprsme_web/plugs/set_locale.ex +++ b/lib/aprsme_web/plugs/set_locale.ex @@ -9,7 +9,6 @@ defmodule AprsmeWeb.Plugs.SetLocale do def call(conn, _opts) do locale = get_locale_from_header(conn) || "en" - # Set the backend's locale for the current process _ = Gettext.put_locale(AprsmeWeb.Gettext, locale) # Store locale in session for LiveView to access conn = put_session(conn, :locale, locale) diff --git a/mix.exs b/mix.exs index 8d9b77f..29e5d4f 100644 --- a/mix.exs +++ b/mix.exs @@ -115,6 +115,7 @@ defmodule Aprsme.MixProject do {:styler, "~> 1.10", only: [:dev, :test], runtime: false}, {:hammer, "~> 7.0"}, {:jump_credo_checks, "~> 0.4", only: [:dev, :test], runtime: false}, + {:ex_slop, "~> 0.1", only: [:dev, :test], runtime: false}, {:inet_cidr, "~> 1.0"} ] end diff --git a/mix.lock b/mix.lock index aaa680d..a75369e 100644 --- a/mix.lock +++ b/mix.lock @@ -17,6 +17,7 @@ "erlex": {:hex, :erlex, "0.2.9", "7debbbaa9f4f368b8cd648983e0f1d7963028508e9c59e9d4ed504e94ef52a55", [:mix], [], "hexpm", "8cfffc0ec7159e6d73de2ab28a588064de80f88b2798d5cbe4482cbbc200178b"}, "error_tracker": {:hex, :error_tracker, "0.9.0", "fef10b1230be1b051452930ba4db9f809cdab9705fe87891c25f5e551cead372", [:mix], [{:ecto, "~> 3.13", [hex: :ecto, repo: "hexpm", optional: false]}, {:ecto_sql, "~> 3.13", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:ecto_sqlite3, ">= 0.0.0", [hex: :ecto_sqlite3, repo: "hexpm", optional: true]}, {:igniter, "~> 0.5", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:myxql, ">= 0.0.0", [hex: :myxql, repo: "hexpm", optional: true]}, {:phoenix_ecto, "~> 4.6", [hex: :phoenix_ecto, repo: "hexpm", optional: false]}, {:phoenix_live_view, "~> 1.0", [hex: :phoenix_live_view, repo: "hexpm", optional: false]}, {:plug, "~> 1.10", [hex: :plug, repo: "hexpm", optional: false]}, {:postgrex, ">= 0.0.0", [hex: :postgrex, repo: "hexpm", optional: true]}], "hexpm", "1de7d89ec9034c3b7282b6bd2d0584ca98cddc51e87bf0979ba71f6182803ac4"}, "esbuild": {:hex, :esbuild, "0.10.0", "b0aa3388a1c23e727c5a3e7427c932d89ee791746b0081bbe56103e9ef3d291f", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "468489cda427b974a7cc9f03ace55368a83e1a7be12fba7e30969af78e5f8c70"}, + "ex_slop": {:hex, :ex_slop, "0.4.4", "8ca277b902e93193eb3545e414c309026a51204e5eb7d6ada5ff96555de6abcb", [:mix], [{:credo, "~> 1.7", [hex: :credo, repo: "hexpm", optional: false]}], "hexpm", "03ef1b2553ebad45c8ceb3e482eed5f1ccd08fc992215a9da0ef6196935107e2"}, "expo": {:hex, :expo, "1.1.1", "4202e1d2ca6e2b3b63e02f69cfe0a404f77702b041d02b58597c00992b601db5", [:mix], [], "hexpm", "5fb308b9cb359ae200b7e23d37c76978673aa1b06e2b3075d814ce12c5811640"}, "file_system": {:hex, :file_system, "1.1.1", "31864f4685b0148f25bd3fbef2b1228457c0c89024ad67f7a81a3ffbc0bbad3a", [:mix], [], "hexpm", "7a15ff97dfe526aeefb090a7a9d3d03aa907e100e262a0f8f7746b78f8f87a5d"}, "finch": {:hex, :finch, "0.23.0", "e3f9287ac25a8832f848b144c2b57346aac65b205e2e0629a52adfe6507fd837", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mint, "~> 1.8", [hex: :mint, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.4 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:nimble_pool, "~> 1.1", [hex: :nimble_pool, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "80e58d3f936f57e3fdf404f83a3642897ae6d9fb642934e46da4d8fe761b99d5"}, diff --git a/test/aprsme/bad_packet_test.exs b/test/aprsme/bad_packet_test.exs index 82919d6..7aecf3e 100644 --- a/test/aprsme/bad_packet_test.exs +++ b/test/aprsme/bad_packet_test.exs @@ -127,10 +127,8 @@ defmodule Aprsme.BadPacketTest do query = BadPacket.recent() _query = BadPacket.recent() - # Check the query structure contains limit _query = BadPacket.recent() result = Repo.all(query) - # Verify the query works and the default limit is reasonable assert is_struct(query, Ecto.Query) assert length(result) <= 100 end diff --git a/test/aprsme/broadcast_task_supervisor_test.exs b/test/aprsme/broadcast_task_supervisor_test.exs index 443bb9c..14c4021 100644 --- a/test/aprsme/broadcast_task_supervisor_test.exs +++ b/test/aprsme/broadcast_task_supervisor_test.exs @@ -7,7 +7,6 @@ defmodule Aprsme.BroadcastTaskSupervisorTest do setup do # Ensure the supervisor is started (it should be from application) - # Create a test topic for PubSub test_topic = "test_topic_#{System.unique_integer()}" Phoenix.PubSub.subscribe(Aprsme.PubSub, test_topic) diff --git a/test/aprsme/cluster/connection_manager_test.exs b/test/aprsme/cluster/connection_manager_test.exs index ce1320b..928e16b 100644 --- a/test/aprsme/cluster/connection_manager_test.exs +++ b/test/aprsme/cluster/connection_manager_test.exs @@ -235,7 +235,6 @@ defmodule Aprsme.Cluster.ConnectionManagerTest do end test "handles IsSupervisor start failure", %{pid: pid} do - # Define a failing IsSupervisor defmodule FailingIsSupervisor do @moduledoc false def start_link(_opts) do diff --git a/test/aprsme/device_identification_test.exs b/test/aprsme/device_identification_test.exs index 7328581..fbcdc99 100644 --- a/test/aprsme/device_identification_test.exs +++ b/test/aprsme/device_identification_test.exs @@ -435,7 +435,6 @@ defmodule Aprsme.DeviceIdentificationTest do end test "handles non-wildcard exact match" do - # Insert a device with an exact identifier (no ?). Repo.delete_all(Devices) Repo.insert!(%Devices{ diff --git a/test/aprsme/is_test.exs b/test/aprsme/is_test.exs index 6d483f9..e6d8ed9 100644 --- a/test/aprsme/is_test.exs +++ b/test/aprsme/is_test.exs @@ -1229,7 +1229,6 @@ defmodule Aprsme.IsTest do end test "resets per-second counter when time advances" do - # Build a state with last_second_timestamp in the past old_stats = %{ total_packets: 5, last_packet_at: DateTime.utc_now(), diff --git a/test/aprsme/migration_lock_test.exs b/test/aprsme/migration_lock_test.exs index e4c0c7a..e116d01 100644 --- a/test/aprsme/migration_lock_test.exs +++ b/test/aprsme/migration_lock_test.exs @@ -6,7 +6,6 @@ defmodule Aprsme.MigrationLockTest do @lock_id 8_764_293_847_291 setup do - # Ensure the advisory lock is released before each test. # Advisory locks are session-level, so we need to make sure none # are left over from a previous test. Repo.query("SELECT pg_advisory_unlock_all()") diff --git a/test/aprsme/migration_validation_test.exs b/test/aprsme/migration_validation_test.exs index 289bdc3..281bb80 100644 --- a/test/aprsme/migration_validation_test.exs +++ b/test/aprsme/migration_validation_test.exs @@ -203,7 +203,7 @@ defmodule Aprsme.MigrationValidationTest do assert after_insert == before + 1 - # Delete the packet — trigger should decrement + # The counter trigger should fire on delete to decrement the count. Repo.query!("DELETE FROM packets WHERE id = $1", [inserted_id]) {:ok, %{rows: [[after_delete]]}} = diff --git a/test/aprsme/packet_consumer_test.exs b/test/aprsme/packet_consumer_test.exs index 12df70a..396a1b6 100644 --- a/test/aprsme/packet_consumer_test.exs +++ b/test/aprsme/packet_consumer_test.exs @@ -951,8 +951,8 @@ defmodule Aprsme.PacketConsumerTest do :erlang.garbage_collect() memory_before = :erlang.memory(:total) - # Create a smaller, more focused batch of packets - # Reduced from 5000 to 1000 packets and smaller payloads + # Reduced from 5000 to 1000 packets and smaller payloads to keep + # memory usage within test environment limits. events = for i <- 1..1000 do %{ diff --git a/test/aprsme/packet_parsing_test.exs b/test/aprsme/packet_parsing_test.exs index d193197..0691d66 100644 --- a/test/aprsme/packet_parsing_test.exs +++ b/test/aprsme/packet_parsing_test.exs @@ -7,7 +7,6 @@ defmodule Aprsme.PacketParsingTest do test "extracts altitude and PHG data from packet" do raw_packet = "W5MRC-15>APN391,qAO,KG5JPL-1:!3317.02NN09634.37W#PHG5530 Collin Cty Wide Digi /A=000680" - # Parse the packet {:ok, parsed} = Aprs.parse(raw_packet) # Create attributes for packet diff --git a/test/aprsme/packet_weather_extraction_test.exs b/test/aprsme/packet_weather_extraction_test.exs index bdcfe25..179c56c 100644 --- a/test/aprsme/packet_weather_extraction_test.exs +++ b/test/aprsme/packet_weather_extraction_test.exs @@ -8,7 +8,6 @@ defmodule Aprsme.PacketWeatherExtractionTest do # Simulate a real weather packet raw_packet = "AE5PL-13>API510,DSTAR*:!3240.46N/09657.49W_225/004g009t075r000p000h61b10206Plano, TX weather" - # Parse the packet using the real APRS parser {:ok, parsed_packet} = Aprs.parse(raw_packet) # Create packet data as it would come from the parser @@ -48,7 +47,6 @@ defmodule Aprsme.PacketWeatherExtractionTest do # Simulate a pure weather packet (starts with _) raw_packet = "AE5PL-WX>APRS:_07121545c220s004g009t075r000p000h61b10206" - # Parse the packet using the real APRS parser {:ok, parsed_packet} = Aprs.parse(raw_packet) # Create packet data as it would come from the parser diff --git a/test/aprsme/packets/clustering_test.exs b/test/aprsme/packets/clustering_test.exs index 8f4e49f..f9d5c03 100644 --- a/test/aprsme/packets/clustering_test.exs +++ b/test/aprsme/packets/clustering_test.exs @@ -110,7 +110,6 @@ defmodule Aprsme.Packets.ClusteringTest do # At zoom 8, these should be two clusters (radius 0.039 degrees) # but our points are only 0.014 degrees apart, so they'll still cluster - # Let's use points that are further apart far_packets = [ %Packet{id: 1, lat: Decimal.new("40.7128"), lon: Decimal.new("-74.0060")}, %Packet{id: 2, lat: Decimal.new("40.8128"), lon: Decimal.new("-74.1060")} diff --git a/test/aprsme/packets_mic_e_test.exs b/test/aprsme/packets_mic_e_test.exs index ae53078..9e7ee5c 100644 --- a/test/aprsme/packets_mic_e_test.exs +++ b/test/aprsme/packets_mic_e_test.exs @@ -40,7 +40,6 @@ defmodule Aprsme.PacketsMicETest do # This should not raise a KeyError assert {:ok, stored_packet} = Packets.store_packet(packet_data) - # Verify the packet was stored with correct position data assert stored_packet.sender == "DG1ID-9" assert stored_packet.has_position == true @@ -214,7 +213,6 @@ defmodule Aprsme.PacketsMicETest do # The fix ensures we use data_extended[:latitude] instead of data_extended.latitude assert {:ok, stored_packet} = Packets.store_packet(packet_data) - # Verify the packet was stored correctly assert stored_packet.sender == "DG1ID-9" # Note: longitude 198 is invalid (outside -180 to 180 range) @@ -250,7 +248,6 @@ defmodule Aprsme.PacketsMicETest do # The fix ensures we check the struct type before using Access behavior assert {:ok, stored_packet} = Packets.store_packet(packet_data) - # Verify the packet was stored correctly assert stored_packet.sender == "LU9DCE-4" assert stored_packet.data_type == "phg_data" diff --git a/test/aprsme/packets_test.exs b/test/aprsme/packets_test.exs index ec3e7cb..1c122a3 100644 --- a/test/aprsme/packets_test.exs +++ b/test/aprsme/packets_test.exs @@ -28,7 +28,7 @@ defmodule Aprsme.PacketsTest do assert {:ok, packet} = Packets.store_packet(packet_data) assert packet.sender == "TEST1" - # Handle the fact that lat/lon may be stored as Decimal + # lat/lon may be stored as Decimal, not float, depending on Ecto config. lat_value = if is_struct(packet.lat, Decimal), do: Decimal.to_float(packet.lat), else: packet.lat lon_value = if is_struct(packet.lon, Decimal), do: Decimal.to_float(packet.lon), else: packet.lon assert Float.round(lat_value, 4) == 39.8333 @@ -164,7 +164,6 @@ defmodule Aprsme.PacketsTest do } assert {:ok, packet} = Packets.store_packet(packet_data) - # Verify the packet was stored successfully assert packet.sender == "TEST7" end end @@ -1066,7 +1065,6 @@ defmodule Aprsme.PacketsTest do describe "get_oldest_packet_timestamp/0" do test "returns timestamp of oldest packet" do - # Create an old packet now = DateTime.utc_now() old_time = DateTime.add(now, -365 * 24 * 3600, :second) @@ -1308,7 +1306,6 @@ defmodule Aprsme.PacketsTest do test "only returns SSIDs with recent packets" do now = DateTime.utc_now() - # Create a recent packet PacketsFixtures.packet_fixture(%{ sender: "RECENT-1", base_callsign: "RECENT", diff --git a/test/aprsme/packets_weather_test.exs b/test/aprsme/packets_weather_test.exs index 774e2a6..e13b186 100644 --- a/test/aprsme/packets_weather_test.exs +++ b/test/aprsme/packets_weather_test.exs @@ -8,7 +8,6 @@ defmodule Aprsme.PacketsWeatherTest do # Simulate a real APRS weather packet with correct format raw_packet = "TEST-1>APRS,WIDE1-1,WIDE2-2:!3216.46N/09647.82W_180/010g015t072r000p000h45b10132" - # Parse the packet using the real APRS parser {:ok, parsed_packet} = Aprs.parse(raw_packet) # Create packet data as it would come from the parser @@ -43,7 +42,6 @@ defmodule Aprsme.PacketsWeatherTest do # Simulate a real APRS weather packet with correct format raw_packet = "WEATHER-1>APRS,WIDE1-1:!3216.46N/09647.82W_270/012g018t085r000p000h60b10150" - # Parse the packet using the real APRS parser {:ok, parsed_packet} = Aprs.parse(raw_packet) packet_data = %{ @@ -75,7 +73,6 @@ defmodule Aprsme.PacketsWeatherTest do # Simulate a real APRS packet with weather data in comment raw_packet = "COMMENT-1>APRS,WIDE1-1:!3216.46N/09647.82W>090/008g012t095r000p000h70b10200" - # Parse the packet using the real APRS parser {:ok, parsed_packet} = Aprs.parse(raw_packet) packet_data = %{ @@ -106,7 +103,6 @@ defmodule Aprsme.PacketsWeatherTest do # Simulate a real APRS packet without weather data raw_packet = "REGULAR-1>APRS,WIDE1-1:!3216.46N/09647.82W>Just a regular packet" - # Parse the packet using the real APRS parser {:ok, parsed_packet} = Aprs.parse(raw_packet) packet_data = %{ @@ -141,7 +137,6 @@ defmodule Aprsme.PacketsWeatherTest do raw_packet1 = "BATCH1-1>APRS,WIDE1-1:!3216.46N/09647.82W_200/010g015t070r000p000h50b10120" raw_packet2 = "BATCH2-1>APRS,WIDE1-1:!3216.46N/09647.82W_220/012g018t075r000p000h55b10140" - # Parse the packets using the real APRS parser {:ok, parsed_packet1} = Aprs.parse(raw_packet1) {:ok, parsed_packet2} = Aprs.parse(raw_packet2) @@ -194,7 +189,6 @@ defmodule Aprsme.PacketsWeatherTest do # Simulate a real APRS weather packet with snow data raw_packet = "SNOW-1>APRS,WIDE1-1:!3216.46N/09647.82W_180/010g015t072r000p000h45b10132s005" - # Parse the packet using the real APRS parser {:ok, parsed_packet} = Aprs.parse(raw_packet) packet_data = %{ diff --git a/test/aprsme/workers/packet_cleanup_worker_test.exs b/test/aprsme/workers/packet_cleanup_worker_test.exs index b4d7ee9..d29b33b 100644 --- a/test/aprsme/workers/packet_cleanup_worker_test.exs +++ b/test/aprsme/workers/packet_cleanup_worker_test.exs @@ -6,7 +6,6 @@ defmodule Aprsme.Workers.PacketCleanupWorkerTest do describe "perform/1 without cleanup_days" do test "cleans up expired bad packets" do - # Create an old bad packet retention_days = Application.get_env(:aprsme, :packet_retention_days, 7) old_time = @@ -16,7 +15,6 @@ defmodule Aprsme.Workers.PacketCleanupWorkerTest do Repo.insert!(%BadPacket{raw_packet: "bad", error_message: "test", attempted_at: old_time}) - # Create a recent bad packet recent_time = DateTime.utc_now() |> DateTime.add(-3600, :second) diff --git a/test/aprsme_web/controllers/api/v1/weather_controller_test.exs b/test/aprsme_web/controllers/api/v1/weather_controller_test.exs index 3607f09..8324676 100644 --- a/test/aprsme_web/controllers/api/v1/weather_controller_test.exs +++ b/test/aprsme_web/controllers/api/v1/weather_controller_test.exs @@ -16,7 +16,6 @@ defmodule AprsmeWeb.Api.V1.WeatherControllerTest do describe "GET /api/v1/weather/nearby" do setup do - # Create a weather station packet station = packet_fixture(%{ sender: "TEST-1", @@ -35,7 +34,7 @@ defmodule AprsmeWeb.Api.V1.WeatherControllerTest do rain_since_midnight: 0.0 }) - # Create a non-weather packet (should not appear) + # Should not appear in weather results. packet_fixture(%{ sender: "TEST-2", data_type: "position", @@ -44,7 +43,7 @@ defmodule AprsmeWeb.Api.V1.WeatherControllerTest do has_weather: false }) - # Create a weather station outside radius + # Should be excluded because it falls outside the search radius. packet_fixture(%{ sender: "TEST-3", data_type: "weather", diff --git a/test/aprsme_web/integration/aprs_status_test.exs b/test/aprsme_web/integration/aprs_status_test.exs index 79a3e30..78cbb0b 100644 --- a/test/aprsme_web/integration/aprs_status_test.exs +++ b/test/aprsme_web/integration/aprs_status_test.exs @@ -99,7 +99,6 @@ defmodule AprsmeWeb.Integration.AprsStatusTest do describe "LiveView event handling without APRS" do test "map events work without APRS connection", %{conn: conn} do - # Set the mock to allow calls from any process Mox.set_mox_global() # Stub the function that will be called during bounds changes @@ -196,7 +195,6 @@ defmodule AprsmeWeb.Integration.AprsStatusTest do end test "APRS.Is process is not running" do - # Verify the real APRS.Is GenServer is not started assert Process.whereis(Aprsme.Is) == nil assert Map.has_key?(Aprsme.Is.get_status(), :connected) end diff --git a/test/aprsme_web/live/bad_packets_live_test.exs b/test/aprsme_web/live/bad_packets_live_test.exs index 93e8a2a..c1867dc 100644 --- a/test/aprsme_web/live/bad_packets_live_test.exs +++ b/test/aprsme_web/live/bad_packets_live_test.exs @@ -48,7 +48,6 @@ defmodule AprsmeWeb.BadPacketsLiveTest do test "updates in real-time when new bad packet is created", %{conn: conn} do {:ok, index_live, _html} = live(conn, ~p"/badpackets", on_error: :warn) - # Create a bad packet after the live view is loaded bad_packet = Repo.insert!(%BadPacket{ raw_packet: "KD9PDP>APRS:New invalid packet", diff --git a/test/aprsme_web/live/map_live/display_manager_test.exs b/test/aprsme_web/live/map_live/display_manager_test.exs index 44be60f..90446a4 100644 --- a/test/aprsme_web/live/map_live/display_manager_test.exs +++ b/test/aprsme_web/live/map_live/display_manager_test.exs @@ -5,7 +5,6 @@ defmodule AprsmeWeb.MapLive.DisplayManagerTest do alias Phoenix.LiveView.Socket setup do - # Create a basic socket mock for testing socket = %Socket{ assigns: %{ visible_packets: %{}, diff --git a/test/aprsme_web/live/map_live/historical_loading_test.exs b/test/aprsme_web/live/map_live/historical_loading_test.exs index 4cbffec..0c9331a 100644 --- a/test/aprsme_web/live/map_live/historical_loading_test.exs +++ b/test/aprsme_web/live/map_live/historical_loading_test.exs @@ -36,7 +36,7 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do comment: "Test station 2 in Times Square" }) - # Create an old packet that should not be loaded + # Should not be loaded — it falls outside the historical time window. old_packet = packet_fixture(%{ sender: "TEST3", @@ -270,7 +270,6 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do end test "does not send historical packets when bounds are invalid", %{conn: conn} do - # Create a test packet packet_fixture(%{ sender: "INVALID1", base_callsign: "INVALID1", diff --git a/test/aprsme_web/live/map_live/overlay_rendering_test.exs b/test/aprsme_web/live/map_live/overlay_rendering_test.exs index 3cab748..279007f 100644 --- a/test/aprsme_web/live/map_live/overlay_rendering_test.exs +++ b/test/aprsme_web/live/map_live/overlay_rendering_test.exs @@ -5,7 +5,6 @@ defmodule AprsmeWeb.MapLive.OverlayRenderingTest do describe "overlay symbol rendering in map" do test "W5MRC-15 with D& symbol emits structured marker data" do - # Create a test packet with overlay symbol D& packet = %{ id: 1, sender: "W5MRC-15", @@ -20,7 +19,6 @@ defmodule AprsmeWeb.MapLive.OverlayRenderingTest do data_type: "position" } - # Process the packet through PacketUtils result = PacketUtils.build_packet_data(packet, true, "en-US") assert result["symbol_table_id"] == "D" @@ -30,7 +28,6 @@ defmodule AprsmeWeb.MapLive.OverlayRenderingTest do end test "N# green star overlay symbol generates correct HTML" do - # Create a test packet with overlay symbol N# packet = %{ id: 2, sender: "TEST-1", @@ -45,7 +42,6 @@ defmodule AprsmeWeb.MapLive.OverlayRenderingTest do data_type: "position" } - # Process the packet result = PacketUtils.build_packet_data(packet, true, "en-US") assert result["symbol_table_id"] == "N" assert result["symbol_code"] == "#" @@ -54,7 +50,6 @@ defmodule AprsmeWeb.MapLive.OverlayRenderingTest do end test "normal symbol /_ generates correct HTML without overlay" do - # Create a test packet with normal symbol /_ packet = %{ id: 3, sender: "WEATHER-1", @@ -69,7 +64,6 @@ defmodule AprsmeWeb.MapLive.OverlayRenderingTest do data_type: "weather" } - # Process the packet result = PacketUtils.build_packet_data(packet, true, "en-US") assert result["symbol_table_id"] == "/" assert result["symbol_code"] == "_" diff --git a/test/aprsme_web/live/map_live/rf_path_test.exs b/test/aprsme_web/live/map_live/rf_path_test.exs index 5a22d30..0b1d694 100644 --- a/test/aprsme_web/live/map_live/rf_path_test.exs +++ b/test/aprsme_web/live/map_live/rf_path_test.exs @@ -111,7 +111,6 @@ defmodule AprsmeWeb.MapLive.RfPathTest do end test "filters out APRS beacon patterns from paths", %{conn: conn} do - # Create a real station that should be included {:ok, _station} = create_test_packet(%{ sender: "KC5ABC-9", diff --git a/test/aprsme_web/live/map_live/tracked_callsign_old_packet_test.exs b/test/aprsme_web/live/map_live/tracked_callsign_old_packet_test.exs index 3e999f5..5b7c800 100644 --- a/test/aprsme_web/live/map_live/tracked_callsign_old_packet_test.exs +++ b/test/aprsme_web/live/map_live/tracked_callsign_old_packet_test.exs @@ -78,7 +78,6 @@ defmodule AprsmeWeb.MapLive.TrackedCallsignOldPacketTest do end test "displays message packet without position for tracked callsign", %{conn: conn} do - # Create a message packet without position data packet_fixture(%{ sender: "TEST-MSG", raw_packet: "TEST-MSG>APRS::W5ISP :Hello there! This is a test message{001", @@ -100,7 +99,6 @@ defmodule AprsmeWeb.MapLive.TrackedCallsignOldPacketTest do end test "handles whitespace in callsign parameter", %{conn: conn} do - # Create a packet for testing packet_fixture(%{ sender: "TEST-WS", raw_packet: "TEST-WS>APRS:>Test whitespace handling", @@ -116,7 +114,6 @@ defmodule AprsmeWeb.MapLive.TrackedCallsignOldPacketTest do end test "handles lowercase callsign with non-position packet", %{conn: conn} do - # Create a telemetry packet without position packet_fixture(%{ sender: "TEST-TELEM", raw_packet: "TEST-TELEM>APRS:T#123,456,789,012,345,678,00000000", diff --git a/test/aprsme_web/live/status_live/index_test.exs b/test/aprsme_web/live/status_live/index_test.exs index e91bb05..6a1a1a9 100644 --- a/test/aprsme_web/live/status_live/index_test.exs +++ b/test/aprsme_web/live/status_live/index_test.exs @@ -73,7 +73,7 @@ defmodule AprsmeWeb.StatusLive.IndexTest do end test "renders the connected-path template when cache has a live status", %{conn: conn} do - # Ensure the cache ETS table exists for test. + # ETS table may not exist in the test sandbox, so create it on demand. _ = case :ets.info(:query_cache) do :undefined -> :ets.new(:query_cache, [:named_table, :public, :set]) diff --git a/test/aprsme_web/live/user_settings_live_test.exs b/test/aprsme_web/live/user_settings_live_test.exs index 7393dbb..a01e51e 100644 --- a/test/aprsme_web/live/user_settings_live_test.exs +++ b/test/aprsme_web/live/user_settings_live_test.exs @@ -61,7 +61,6 @@ defmodule AprsmeWeb.UserSettingsLiveTest do flash = assert_redirect(lv, ~p"/users/settings") assert flash["info"] == "Callsign updated successfully." - # Verify the callsign was updated updated_user = Accounts.get_user!(user.id) assert updated_user.callsign == "W9NEW" end @@ -93,7 +92,6 @@ defmodule AprsmeWeb.UserSettingsLiveTest do assert result =~ "is not valid" - # Verify the callsign was not updated updated_user = Accounts.get_user!(user.id) assert updated_user.callsign == user.callsign end diff --git a/test/aprsme_web/plugs/content_security_policy_test.exs b/test/aprsme_web/plugs/content_security_policy_test.exs index 511ab4c..814a476 100644 --- a/test/aprsme_web/plugs/content_security_policy_test.exs +++ b/test/aprsme_web/plugs/content_security_policy_test.exs @@ -37,7 +37,6 @@ defmodule AprsmeWeb.Plugs.ContentSecurityPolicyTest do nonce = conn.private[:csp_nonce] assert is_binary(nonce) and byte_size(nonce) > 0 - # Verify the private nonce matches the one in the header csp = conn |> get_resp_header("content-security-policy") |> List.first() assert csp =~ "'nonce-#{nonce}'" end diff --git a/test/integration/historical_packets_test.exs b/test/integration/historical_packets_test.exs index 5d68541..28a95d7 100644 --- a/test/integration/historical_packets_test.exs +++ b/test/integration/historical_packets_test.exs @@ -91,7 +91,6 @@ defmodule Aprsme.Integration.HistoricalPacketsTest do describe "live packet updates with historical data" do setup do - # Create a historical packet now = DateTime.utc_now() thirty_minutes_ago = DateTime.add(now, -1800, :second) diff --git a/test/support/channel_case.ex b/test/support/channel_case.ex index b76c7ec..df4b883 100644 --- a/test/support/channel_case.ex +++ b/test/support/channel_case.ex @@ -1,18 +1,13 @@ defmodule AprsmeWeb.ChannelCase do @moduledoc """ - This module defines the test case to be used by - channel tests. + Test case for channel tests. - Such tests rely on `Phoenix.ChannelTest` and also - import other functionality to make it easier - to build common data structures and query the data layer. + Relies on `Phoenix.ChannelTest` and imports helpers for building common + data structures and querying the data layer. - Finally, if the test case interacts with the database, - we enable the SQL sandbox, so changes done to the database - are reverted at the end of every test. If you are using - PostgreSQL, you can even run database tests asynchronously - by setting `use AprsmeWeb.ChannelCase, async: true`, although - this option is not recommended for other databases. + When the test case interacts with the database, the SQL sandbox is enabled + so changes are reverted at the end of every test. PostgreSQL users can + run database tests asynchronously by setting `use AprsmeWeb.ChannelCase, async: true`. """ use ExUnit.CaseTemplate diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index be0d068..7a8a2df 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -1,18 +1,13 @@ defmodule AprsmeWeb.ConnCase do @moduledoc """ - This module defines the test case to be used by - tests that require setting up a connection. + Test case for connection tests. - Such tests rely on `Phoenix.ConnTest` and also - import other functionality to make it easier - to build common data structures and query the data layer. + Relies on `Phoenix.ConnTest` and imports helpers for building common + data structures and querying the data layer. - Finally, if the test case interacts with the database, - we enable the SQL sandbox, so changes done to the database - are reverted at the end of every test. If you use PostgreSQL, you - can even run database tests asynchronously by setting - `use AprsmeWeb.ConnCase, async: true`, although - this option is not recommended for other databases. + When the test case interacts with the database, the SQL sandbox is enabled + so changes are reverted at the end of every test. PostgreSQL users can + run database tests asynchronously by setting `use AprsmeWeb.ConnCase, async: true`. """ use ExUnit.CaseTemplate diff --git a/test/support/data_case.ex b/test/support/data_case.ex index 77d01ca..200e5ec 100644 --- a/test/support/data_case.ex +++ b/test/support/data_case.ex @@ -1,17 +1,13 @@ defmodule Aprsme.DataCase do @moduledoc """ - This module defines the test case to be used by - data tests. + Test case for data-layer tests. - You may define functions here to be used as helpers in - your data tests. See `errors_on/2`'s definition as an example. + Imports Ecto helpers (`Ecto`, `Ecto.Changeset`, `Ecto.Query`) and + provides utilities like `errors_on/1` for validating changesets. - Finally, if the test case interacts with the database, - we enable the SQL sandbox, so changes done to the database - are reverted at the end of every test. If you use PostgreSQL, you - can even run database tests asynchronously by setting - `use Aprsme.DataCase, async: true`, although - this option is not recommended for other databases. + When the test case interacts with the database, the SQL sandbox is enabled + so changes are reverted at the end of every test. PostgreSQL users can + run database tests asynchronously by setting `use Aprsme.DataCase, async: true`. """ use ExUnit.CaseTemplate diff --git a/test/support/fixtures/accounts_fixtures.ex b/test/support/fixtures/accounts_fixtures.ex index 4c6921b..8fd52c2 100644 --- a/test/support/fixtures/accounts_fixtures.ex +++ b/test/support/fixtures/accounts_fixtures.ex @@ -1,7 +1,6 @@ defmodule Aprsme.AccountsFixtures do @moduledoc """ - This module defines test helpers for creating - entities via the `Aprsme.Accounts` context. + Test helpers for creating user entities via the `Aprsme.Accounts` context. """ def unique_user_email, do: "user#{System.unique_integer()}@example.com" diff --git a/test/support/fixtures/packets_fixtures.ex b/test/support/fixtures/packets_fixtures.ex index 0914b1a..673300e 100644 --- a/test/support/fixtures/packets_fixtures.ex +++ b/test/support/fixtures/packets_fixtures.ex @@ -1,7 +1,6 @@ defmodule Aprsme.PacketsFixtures do @moduledoc """ - This module defines test helpers for creating - entities via the `Aprsme.Packets` context. + Test helpers for creating packet entities via the `Aprsme.Packets` context. """ alias Aprsme.Callsign