diff --git a/lib/aprs/accounts.ex b/lib/aprs/accounts.ex index f192352..a7a8523 100644 --- a/lib/aprs/accounts.ex +++ b/lib/aprs/accounts.ex @@ -42,7 +42,13 @@ defmodule Aprs.Accounts do """ def get_user_by_email_and_password(email, password) when is_binary(email) and is_binary(password) do user = Repo.get_by(User, email: email) - if User.valid_password?(user, password), do: user + validate_user_password(user, password) + end + + defp validate_user_password(user, password) do + if User.valid_password?(user, password) do + user + end end @doc """ @@ -257,15 +263,16 @@ defmodule Aprs.Accounts do {:error, :already_confirmed} """ - def deliver_user_confirmation_instructions(%User{} = user, confirmation_url_fun) + def deliver_user_confirmation_instructions(%User{confirmed_at: nil} = user, confirmation_url_fun) when is_function(confirmation_url_fun, 1) do - if user.confirmed_at do - {:error, :already_confirmed} - else - {encoded_token, user_token} = UserToken.build_email_token(user, "confirm") - Repo.insert!(user_token) - UserNotifier.deliver_confirmation_instructions(user, confirmation_url_fun.(encoded_token)) - end + {encoded_token, user_token} = UserToken.build_email_token(user, "confirm") + Repo.insert!(user_token) + UserNotifier.deliver_confirmation_instructions(user, confirmation_url_fun.(encoded_token)) + end + + def deliver_user_confirmation_instructions(%User{confirmed_at: confirmed_at} = _user, _confirmation_url_fun) + when not is_nil(confirmed_at) do + {:error, :already_confirmed} end @doc """ diff --git a/lib/aprs/accounts/user.ex b/lib/aprs/accounts/user.ex index f392740..d81e560 100644 --- a/lib/aprs/accounts/user.ex +++ b/lib/aprs/accounts/user.ex @@ -65,7 +65,11 @@ defmodule Aprs.Accounts.User do hash_password? = Keyword.get(opts, :hash_password, true) password = get_change(changeset, :password) - if hash_password? && password && changeset.valid? do + do_hash_password(changeset, hash_password?, password) + end + + defp do_hash_password(changeset, true, password) when is_binary(password) do + if changeset.valid? do changeset # If using Bcrypt, then further validate it is at most 72 bytes long |> validate_length(:password, max: 72, count: :bytes) @@ -76,16 +80,21 @@ defmodule Aprs.Accounts.User do end end + defp do_hash_password(changeset, _, _), do: changeset + defp maybe_validate_unique_email(changeset, opts) do - if Keyword.get(opts, :validate_email, true) do - changeset - |> unsafe_validate_unique(:email, Aprs.Repo) - |> unique_constraint(:email) - else - changeset - end + validate_email? = Keyword.get(opts, :validate_email, true) + do_validate_unique_email(changeset, validate_email?) end + defp do_validate_unique_email(changeset, true) do + changeset + |> unsafe_validate_unique(:email, Aprs.Repo) + |> unique_constraint(:email) + end + + defp do_validate_unique_email(changeset, false), do: changeset + @doc """ A user changeset for changing the email. diff --git a/lib/aprs/application.ex b/lib/aprs/application.ex index f0bfbc1..5b3f227 100644 --- a/lib/aprs/application.ex +++ b/lib/aprs/application.ex @@ -36,12 +36,7 @@ defmodule Aprs.Application do Aprs.PostgresNotifier ] - children = - if Application.get_env(:aprs, :env) in [:prod, :dev] do - children ++ [Aprs.Is.IsSupervisor] - else - children - end + children = maybe_add_is_supervisor(children, Application.get_env(:aprs, :env)) # See https://hexdocs.pm/elixir/Supervisor.html # for other strategies and supported options @@ -58,17 +53,8 @@ defmodule Aprs.Application do end defp migrate do - if Application.get_env(:aprs, :auto_migrate, true) do - require Logger - - Logger.info("Running database migrations...") - Aprs.Release.migrate() - Logger.info("Database migrations completed") - else - require Logger - - Logger.info("Automatic migrations disabled") - end + auto_migrate = Application.get_env(:aprs, :auto_migrate, true) + do_migrate(auto_migrate) rescue error -> require Logger @@ -77,4 +63,24 @@ defmodule Aprs.Application do # Don't crash the application, just log the error :ok end + + defp maybe_add_is_supervisor(children, env) when env in [:prod, :dev] do + children ++ [Aprs.Is.IsSupervisor] + end + + defp maybe_add_is_supervisor(children, _env), do: children + + defp do_migrate(true) do + require Logger + + Logger.info("Running database migrations...") + Aprs.Release.migrate() + Logger.info("Database migrations completed") + end + + defp do_migrate(false) do + require Logger + + Logger.info("Automatic migrations disabled") + end end diff --git a/lib/aprs/device_identification.ex b/lib/aprs/device_identification.ex index d483374..561ec94 100644 --- a/lib/aprs/device_identification.ex +++ b/lib/aprs/device_identification.ex @@ -37,7 +37,9 @@ defmodule Aprs.DeviceIdentification do @spec identify_device(String.t()) :: String.t() def identify_device(symbols) do Enum.find_value(@device_patterns, "Unknown", fn {regex, name} -> - if Regex.match?(regex, symbols), do: name + if Regex.match?(regex, symbols) do + name + end end) end @@ -66,13 +68,11 @@ defmodule Aprs.DeviceIdentification do Returns a list of all known device models for a given manufacturer. """ @spec known_models(String.t()) :: [String.t()] - def known_models(manufacturer) do - case manufacturer do - "Kenwood" -> ["TH-D74", "TH-D74A", "DM-710", "DM-700"] - "Yaesu" -> ["VX-8", "FTM-350", "VX-8G", "FT1D", "FTM-400DR", "FTM-100D", "FT2D"] - "Byonics" -> ["TinyTrack3", "TinyTrack4"] - "SCS GmbH & Co." -> ["P4dragon DR-7400 modems", "P4dragon DR-7800 modems"] - _ -> [] - end - end + def known_models("Kenwood"), do: ["TH-D74", "TH-D74A", "DM-710", "DM-700"] + + def known_models("Yaesu"), do: ["VX-8", "FTM-350", "VX-8G", "FT1D", "FTM-400DR", "FTM-100D", "FT2D"] + + def known_models("Byonics"), do: ["TinyTrack3", "TinyTrack4"] + def known_models("SCS GmbH & Co."), do: ["P4dragon DR-7400 modems", "P4dragon DR-7800 modems"] + def known_models(_), do: [] end diff --git a/lib/aprs/encoding_utils.ex b/lib/aprs/encoding_utils.ex index 9471781..290a7d3 100644 --- a/lib/aprs/encoding_utils.ex +++ b/lib/aprs/encoding_utils.ex @@ -69,43 +69,43 @@ defmodule Aprs.EncodingUtils do def sanitize_data_extended(data_extended) when is_map(data_extended) do # Handle generic maps by sanitizing all string values Enum.reduce(data_extended, %{}, fn {key, value}, acc -> - sanitized_value = - case value do - val when is_binary(val) -> sanitize_string(val) - val -> val - end - + sanitized_value = sanitize_map_value(value) Map.put(acc, key, sanitized_value) end) end def sanitize_data_extended(data_extended), do: data_extended + defp sanitize_map_value(val) when is_binary(val), do: sanitize_string(val) + defp sanitize_map_value(val), do: val + # Private helper functions @spec scrub_problematic_bytes(binary()) :: String.t() defp scrub_problematic_bytes(binary) do # Handle both invalid UTF-8 sequences and problematic control characters - cleaned = - if String.valid?(binary) do - # String is valid UTF-8, just remove control characters - binary - # Remove null bytes - |> String.replace(<<0>>, "") - # Remove other control chars - |> String.replace(~r/[\x01-\x08\x0B\x0C\x0E-\x1F\x7F]/, "") - else - # String has invalid UTF-8, filter byte by byte - binary - |> :binary.bin_to_list() - |> Enum.filter(&valid_utf8_byte?/1) - |> :binary.list_to_bin() - |> ensure_valid_utf8() - end - + cleaned = clean_binary_by_validity(binary, String.valid?(binary)) String.trim(cleaned) end + defp clean_binary_by_validity(binary, true) do + # String is valid UTF-8, just remove control characters + binary + # Remove null bytes + |> String.replace(<<0>>, "") + # Remove other control chars + |> String.replace(~r/[\x01-\x08\x0B\x0C\x0E-\x1F\x7F]/, "") + end + + defp clean_binary_by_validity(binary, false) do + # String has invalid UTF-8, filter byte by byte + binary + |> :binary.bin_to_list() + |> Enum.filter(&valid_utf8_byte?/1) + |> :binary.list_to_bin() + |> ensure_valid_utf8() + end + # Check if byte should be kept (ASCII printable + safe whitespace) @spec valid_utf8_byte?(integer()) :: boolean() defp valid_utf8_byte?(byte) when byte >= 32 and byte <= 126, do: true @@ -115,7 +115,11 @@ defmodule Aprs.EncodingUtils do # Ensure the final result is valid UTF-8 @spec ensure_valid_utf8(binary()) :: binary() defp ensure_valid_utf8(binary) do - if String.valid?(binary), do: binary, else: try_convert_utf8(binary) + if String.valid?(binary) do + binary + else + try_convert_utf8(binary) + end end defp try_convert_utf8(binary) do @@ -125,13 +129,17 @@ defmodule Aprs.EncodingUtils do _ -> # Last resort: keep only ASCII - binary - |> :binary.bin_to_list() - |> Enum.filter(fn byte -> byte >= 32 and byte <= 126 end) - |> :binary.list_to_bin() + fallback_to_ascii(binary) end end + defp fallback_to_ascii(binary) do + binary + |> :binary.bin_to_list() + |> Enum.filter(fn byte -> byte >= 32 and byte <= 126 end) + |> :binary.list_to_bin() + end + @doc """ Converts a binary to a hex string representation for debugging. @@ -169,29 +177,29 @@ defmodule Aprs.EncodingUtils do byte_count: byte_count } - if valid do - Map.put(base_info, :char_count, String.length(binary)) - else - # Try to find where the invalid sequence starts - invalid_at = find_invalid_byte_position(binary, 0) - Map.put(base_info, :invalid_at, invalid_at) - end + add_encoding_details(base_info, binary, valid) + end + + defp add_encoding_details(base_info, binary, true) do + Map.put(base_info, :char_count, String.length(binary)) + end + + defp add_encoding_details(base_info, binary, false) do + # Try to find where the invalid sequence starts + invalid_at = find_invalid_byte_position(binary, 0) + Map.put(base_info, :invalid_at, invalid_at) end @spec find_invalid_byte_position(binary(), non_neg_integer()) :: non_neg_integer() | nil defp find_invalid_byte_position(<<>>, _pos), do: nil - defp find_invalid_byte_position(binary, pos) do - case binary do - <> -> - if String.valid?(head) do - find_invalid_byte_position(tail, pos + 1) - else - pos - end - - _ -> - pos + defp find_invalid_byte_position(<>, pos) do + if String.valid?(head) do + find_invalid_byte_position(tail, pos + 1) + else + pos end end + + defp find_invalid_byte_position(_, pos), do: pos end diff --git a/lib/aprs/is/is.ex b/lib/aprs/is/is.ex index bd4cde3..28aa510 100644 --- a/lib/aprs/is/is.ex +++ b/lib/aprs/is/is.ex @@ -29,63 +29,73 @@ defmodule Aprs.Is do @impl true def init(_opts) do - # Prevent APRS-IS connections in test environment - if Application.get_env(:aprs, :env) == :test or - Application.get_env(:aprs, :disable_aprs_connection, false) do - Logger.warning("APRS-IS connection disabled in test environment") - {:stop, :test_environment_disabled} + env = Application.get_env(:aprs, :env) + disable_connection = Application.get_env(:aprs, :disable_aprs_connection, false) + + do_init(env, disable_connection) + end + + defp do_init(:test, _) do + Logger.warning("APRS-IS connection disabled in test environment") + {:stop, :test_environment_disabled} + end + + defp do_init(_, true) do + Logger.warning("APRS-IS connection disabled in test environment") + {:stop, :test_environment_disabled} + end + + defp do_init(_env, false) do + # Trap exits so we can gracefully shut down + Process.flag(:trap_exit, true) + + # Add a small delay to prevent rapid reconnection attempts + Process.sleep(2000) + + # Get startup parameters + server = Application.get_env(:aprs, :aprs_is_server, ~c"rotate.aprs2.net") + port = Application.get_env(:aprs, :aprs_is_port, 14_580) + default_filter = Application.get_env(:aprs, :aprs_is_default_filter, "r/33/-96/100") + aprs_user_id = Application.get_env(:aprs, :aprs_is_login_id, "W5ISP") + aprs_passcode = Application.get_env(:aprs, :aprs_is_password, "-1") + + # Record connection start time + connected_at = DateTime.utc_now() + + # Initialize packet statistics + packet_stats = %{ + total_packets: 0, + last_packet_at: nil, + packets_per_second: 0, + last_second_count: 0, + last_second_timestamp: System.system_time(:second) + } + + with {:ok, socket} <- connect_to_aprs_is(server, port), + :ok <- send_login_string(socket, aprs_user_id, aprs_passcode, default_filter) do + timer = create_timer(@aprs_timeout) + keepalive_timer = create_keepalive_timer(@keepalive_interval) + + {:ok, + %{ + server: server, + port: port, + socket: socket, + timer: timer, + keepalive_timer: keepalive_timer, + connected_at: connected_at, + packet_stats: packet_stats, + buffer: "", + login_params: %{ + user_id: aprs_user_id, + passcode: aprs_passcode, + filter: default_filter + } + }} else - # Trap exits so we can gracefully shut down - Process.flag(:trap_exit, true) - - # Add a small delay to prevent rapid reconnection attempts - Process.sleep(2000) - - # Get startup parameters - server = Application.get_env(:aprs, :aprs_is_server, ~c"rotate.aprs2.net") - port = Application.get_env(:aprs, :aprs_is_port, 14_580) - default_filter = Application.get_env(:aprs, :aprs_is_default_filter, "r/33/-96/100") - aprs_user_id = Application.get_env(:aprs, :aprs_is_login_id, "W5ISP") - aprs_passcode = Application.get_env(:aprs, :aprs_is_password, "-1") - - # Record connection start time - connected_at = DateTime.utc_now() - - # Initialize packet statistics - packet_stats = %{ - total_packets: 0, - last_packet_at: nil, - packets_per_second: 0, - last_second_count: 0, - last_second_timestamp: System.system_time(:second) - } - - with {:ok, socket} <- connect_to_aprs_is(server, port), - :ok <- send_login_string(socket, aprs_user_id, aprs_passcode, default_filter) do - timer = create_timer(@aprs_timeout) - keepalive_timer = create_keepalive_timer(@keepalive_interval) - - {:ok, - %{ - server: server, - port: port, - socket: socket, - timer: timer, - keepalive_timer: keepalive_timer, - connected_at: connected_at, - packet_stats: packet_stats, - buffer: "", - login_params: %{ - user_id: aprs_user_id, - passcode: aprs_passcode, - filter: default_filter - } - }} - else - _ -> - Logger.error("Unable to establish connection or log in to APRS-IS") - {:stop, :aprs_connection_failed} - end + _ -> + Logger.error("Unable to establish connection or log in to APRS-IS") + {:stop, :aprs_connection_failed} end end @@ -157,15 +167,26 @@ defmodule Aprs.Is do {:ok, :ssl.sslsocket()} | {:error, any()} defp connect_to_aprs_is(server, port) do # Additional safeguard: prevent connections in test environment - if Application.get_env(:aprs, :env) == :test or - Application.get_env(:aprs, :disable_aprs_connection, false) do - Logger.warning("Attempted APRS-IS connection blocked in test environment") - {:error, :test_environment_blocked} - else - Logger.debug("Connecting to: #{server}:#{port}") - opts = [:binary, active: true] - :gen_tcp.connect(String.to_charlist(server), port, opts) - end + env = Application.get_env(:aprs, :env) + disable_connection = Application.get_env(:aprs, :disable_aprs_connection, false) + + do_connect_to_aprs_is(server, port, env, disable_connection) + end + + defp do_connect_to_aprs_is(_server, _port, :test, _) do + Logger.warning("Attempted APRS-IS connection blocked in test environment") + {:error, :test_environment_blocked} + end + + defp do_connect_to_aprs_is(_server, _port, _, true) do + Logger.warning("Attempted APRS-IS connection blocked in test environment") + {:error, :test_environment_blocked} + end + + defp do_connect_to_aprs_is(server, port, _env, false) do + Logger.debug("Connecting to: #{server}:#{port}") + opts = [:binary, active: true] + :gen_tcp.connect(String.to_charlist(server), port, opts) end @spec send_login_string(:ssl.sslsocket(), String.t(), String.t(), String.t()) :: @@ -352,8 +373,9 @@ defmodule Aprs.Is do Logger.info("Terminating APRS-IS connection: #{inspect(reason)}") # Log any remaining buffered data - if Map.has_key?(state, :buffer) and state.buffer != "" do - Logger.warning("Terminating with incomplete packet in buffer: #{inspect(state.buffer)}") + case Map.get(state, :buffer, "") do + "" -> :ok + buffer -> Logger.warning("Terminating with incomplete packet in buffer: #{inspect(buffer)}") end # Cancel timers @@ -361,9 +383,13 @@ defmodule Aprs.Is do if Map.has_key?(state, :keepalive_timer), do: Process.cancel_timer(state.keepalive_timer) # Close socket - if Map.has_key?(state, :socket) do - Logger.info("Closing socket") - :gen_tcp.close(state.socket) + case Map.get(state, :socket) do + nil -> + :ok + + socket -> + Logger.info("Closing socket") + :gen_tcp.close(socket) end :normal @@ -470,11 +496,11 @@ defmodule Aprs.Is do new_second_count = stats.last_second_count + 1 %{ - stats - | total_packets: new_total, - last_packet_at: DateTime.utc_now(), - packets_per_second: new_second_count, - last_second_count: new_second_count + total_packets: new_total, + last_packet_at: DateTime.utc_now(), + packets_per_second: new_second_count, + last_second_count: new_second_count, + last_second_timestamp: stats.last_second_timestamp } end end diff --git a/lib/aprs/packet.ex b/lib/aprs/packet.ex index b6a7f8e..406d799 100644 --- a/lib/aprs/packet.ex +++ b/lib/aprs/packet.ex @@ -130,69 +130,79 @@ defmodule Aprs.Packet do lon = get_field(changeset, :lon) || get_change(changeset, :lon) # Also check data_extended for coordinates - {lat, lon} = - case {lat, lon} do - {nil, nil} -> - data_extended = get_change(changeset, :data_extended) + {lat, lon} = extract_coordinates_from_changeset(changeset, {lat, lon}) - if data_extended do - {data_extended[:latitude], data_extended[:longitude]} - else - {nil, nil} - end + create_geometry_from_coordinates(changeset, lat, lon) + end - coords -> - coords - end + defp extract_coordinates_from_changeset(changeset, {nil, nil}) do + data_extended = get_change(changeset, :data_extended) + extract_coordinates_from_data_extended(data_extended) + end + defp extract_coordinates_from_changeset(_changeset, coords), do: coords + + defp extract_coordinates_from_data_extended(nil), do: {nil, nil} + + defp extract_coordinates_from_data_extended(data_extended) when is_map(data_extended) do + {data_extended[:latitude], data_extended[:longitude]} + end + + defp extract_coordinates_from_data_extended(_), do: {nil, nil} + + defp create_geometry_from_coordinates(changeset, lat, lon) do if valid_coordinates?(lat, lon) do - try do - location = create_point(lat, lon) - - if location do - put_change(changeset, :location, location) - else - changeset - end - rescue - error -> - require Logger - - Logger.error("Failed to create geometry for lat=#{lat}, lon=#{lon}: #{inspect(error)}") - changeset - end + create_and_set_location(changeset, lat, lon) else changeset end end + defp create_and_set_location(changeset, lat, lon) do + case create_point(lat, lon) do + nil -> changeset + location -> put_change(changeset, :location, location) + end + rescue + error -> + require Logger + + Logger.error("Failed to create geometry for lat=#{lat}, lon=#{lon}: #{inspect(error)}") + changeset + end + defp maybe_set_has_position(changeset) do location = get_field(changeset, :location) || get_change(changeset, :location) - if location do + case location do + nil -> check_legacy_coordinates(changeset) + _location -> put_change(changeset, :has_position, true) + end + end + + defp check_legacy_coordinates(changeset) do + lat = get_field(changeset, :lat) || get_change(changeset, :lat) + lon = get_field(changeset, :lon) || get_change(changeset, :lon) + + if valid_coordinates?(lat, lon) do put_change(changeset, :has_position, true) else - # Check legacy lat/lon fields - lat = get_field(changeset, :lat) || get_change(changeset, :lat) - lon = get_field(changeset, :lon) || get_change(changeset, :lon) - - if valid_coordinates?(lat, lon) do - put_change(changeset, :has_position, true) - else - changeset - end + changeset end end defp valid_coordinates?(lat, lon) do - lat = if is_struct(lat, Decimal), do: Decimal.to_float(lat), else: lat - lon = if is_struct(lon, Decimal), do: Decimal.to_float(lon), else: lon + lat = normalize_coordinate(lat) + lon = normalize_coordinate(lon) is_number(lat) && is_number(lon) && lat >= -90 && lat <= 90 && lon >= -180 && lon <= 180 end + defp normalize_coordinate(%Decimal{} = decimal), do: Decimal.to_float(decimal) + defp normalize_coordinate(coord), do: coord + # Convert atom data_type to string for storage defp normalize_data_type(%{data_type: data_type} = attrs) when is_atom(data_type) do %{attrs | data_type: to_string(data_type)} @@ -202,12 +212,13 @@ defmodule Aprs.Packet do %{attrs | "data_type" => to_string(data_type)} end - # Handle :data_type key access format defp normalize_data_type(attrs) when is_map(attrs) do - if Map.has_key?(attrs, :data_type) and is_atom(attrs.data_type) do - %{attrs | data_type: to_string(attrs.data_type)} - else - attrs + case {Map.has_key?(attrs, :data_type), Map.get(attrs, :data_type)} do + {true, data_type} when is_atom(data_type) -> + %{attrs | data_type: to_string(data_type)} + + _ -> + attrs end end @@ -485,8 +496,8 @@ defmodule Aprs.Packet do @spec create_point(number() | nil, number() | nil) :: Geo.Point.t() | nil def create_point(lat, lon) when (is_number(lat) or is_struct(lat, Decimal)) and (is_number(lon) or is_struct(lon, Decimal)) do - lat = if is_struct(lat, Decimal), do: Decimal.to_float(lat), else: lat - lon = if is_struct(lon, Decimal), do: Decimal.to_float(lon), else: lon + lat = normalize_coordinate(lat) + lon = normalize_coordinate(lon) if valid_coordinates?(lat, lon) do %Geo.Point{coordinates: {lon, lat}, srid: 4326} diff --git a/lib/aprs_web/controllers/error_html.ex b/lib/aprs_web/controllers/error_html.ex index 611a0e1..30d4323 100644 --- a/lib/aprs_web/controllers/error_html.ex +++ b/lib/aprs_web/controllers/error_html.ex @@ -1,4 +1,5 @@ defmodule AprsWeb.ErrorHTML do + @moduledoc false use AprsWeb, :html # If you want to customize your error pages, diff --git a/lib/aprs_web/controllers/error_json.ex b/lib/aprs_web/controllers/error_json.ex index af69a0f..fce5114 100644 --- a/lib/aprs_web/controllers/error_json.ex +++ b/lib/aprs_web/controllers/error_json.ex @@ -1,4 +1,5 @@ defmodule AprsWeb.ErrorJSON do + @moduledoc false # If you want to customize a particular status code, # you may add your own clauses, such as: # diff --git a/lib/aprs_web/controllers/page_controller.ex b/lib/aprs_web/controllers/page_controller.ex index eeb5220..ae79aa6 100644 --- a/lib/aprs_web/controllers/page_controller.ex +++ b/lib/aprs_web/controllers/page_controller.ex @@ -1,4 +1,5 @@ defmodule AprsWeb.PageController do + @moduledoc false use AprsWeb, :controller def home(conn, _params) do diff --git a/lib/aprs_web/controllers/page_html.ex b/lib/aprs_web/controllers/page_html.ex index 7841d6a..78a01af 100644 --- a/lib/aprs_web/controllers/page_html.ex +++ b/lib/aprs_web/controllers/page_html.ex @@ -1,4 +1,5 @@ defmodule AprsWeb.PageHTML do + @moduledoc false use AprsWeb, :html embed_templates "page_html/*" diff --git a/lib/aprs_web/controllers/user_session_controller.ex b/lib/aprs_web/controllers/user_session_controller.ex index 85fdd5c..31161de 100644 --- a/lib/aprs_web/controllers/user_session_controller.ex +++ b/lib/aprs_web/controllers/user_session_controller.ex @@ -1,4 +1,5 @@ defmodule AprsWeb.UserSessionController do + @moduledoc false use AprsWeb, :controller alias Aprs.Accounts diff --git a/lib/aprs_web/endpoint.ex b/lib/aprs_web/endpoint.ex index 7179843..868aeb0 100644 --- a/lib/aprs_web/endpoint.ex +++ b/lib/aprs_web/endpoint.ex @@ -1,4 +1,6 @@ defmodule AprsWeb.Endpoint do + @moduledoc false + use Phoenix.Endpoint, otp_app: :aprs # The session will be stored in the cookie and signed, diff --git a/lib/aprs_web/live/map_live/callsign_view.ex b/lib/aprs_web/live/map_live/callsign_view.ex index f188249..2954ed8 100644 --- a/lib/aprs_web/live/map_live/callsign_view.ex +++ b/lib/aprs_web/live/map_live/callsign_view.ex @@ -1,4 +1,5 @@ defmodule AprsWeb.MapLive.CallsignView do + @moduledoc false use AprsWeb, :live_view alias Aprs.EncodingUtils @@ -817,28 +818,27 @@ defmodule AprsWeb.MapLive.CallsignView do packets |> Enum.reduce([], fn packet, acc -> {lat, lng, _} = MapHelpers.get_coordinates(packet) - - if lat && lng do - # Check if this position is different from the last position - case acc do - [] -> - # First packet, always include - [packet | acc] - - [last_packet | _] -> - if position_changed?(packet, last_packet) do - [packet | acc] - else - acc - end - end - else - acc - end + process_packet_position(packet, acc, lat, lng) end) |> Enum.reverse() end + defp process_packet_position(packet, acc, lat, lng) when not is_nil(lat) and not is_nil(lng) do + check_position_uniqueness(packet, acc) + end + + defp process_packet_position(_packet, acc, _lat, _lng), do: acc + + defp check_position_uniqueness(packet, []), do: [packet] + + defp check_position_uniqueness(packet, [last_packet | _] = acc) do + if position_changed?(packet, last_packet) do + [packet | acc] + else + acc + end + end + # Check if position changed significantly between two packets (more than ~1 meter) @spec position_changed?(struct(), struct()) :: boolean() defp position_changed?(packet1, packet2) do diff --git a/lib/aprs_web/live/map_live/index.ex b/lib/aprs_web/live/map_live/index.ex index 58a83f0..0aaf6d9 100644 --- a/lib/aprs_web/live/map_live/index.ex +++ b/lib/aprs_web/live/map_live/index.ex @@ -9,8 +9,6 @@ defmodule AprsWeb.MapLive.Index do alias AprsWeb.MapLive.PacketUtils alias Phoenix.LiveView.Socket - require Logger - @default_center %{lat: 39.8283, lng: -98.5795} @default_zoom 5 @finch_name Aprs.Finch @@ -143,15 +141,11 @@ defmodule AprsWeb.MapLive.Index do @impl true def handle_event("bounds_changed", %{"bounds" => bounds}, socket) do - Logger.debug("handle_event bounds_changed: #{inspect(bounds)} vs current #{inspect(socket.assigns.map_bounds)}") - handle_bounds_update(bounds, socket) end @impl true def handle_event("update_bounds", %{"bounds" => bounds}, socket) do - Logger.debug("handle_event update_bounds: #{inspect(bounds)} vs current #{inspect(socket.assigns.map_bounds)}") - handle_bounds_update(bounds, socket) end @@ -304,6 +298,12 @@ defmodule AprsWeb.MapLive.Index do {:noreply, socket} end + @impl true + def handle_event("get_assigns", _params, socket) do + send(self(), {:test_assigns, socket.assigns}) + {:noreply, socket} + end + @spec handle_bounds_update(map(), Socket.t()) :: {:noreply, Socket.t()} defp handle_bounds_update(bounds, socket) do # Update the map bounds from the client @@ -314,8 +314,6 @@ defmodule AprsWeb.MapLive.Index do west: bounds["west"] } - Logger.debug("handle_bounds_update: new #{inspect(map_bounds)} vs current #{inspect(socket.assigns.map_bounds)}") - # Validate bounds to prevent invalid coordinates if map_bounds.north > 90 or map_bounds.south < -90 or map_bounds.north <= map_bounds.south do @@ -340,8 +338,6 @@ defmodule AprsWeb.MapLive.Index do @spec process_bounds_update(map(), Socket.t()) :: Socket.t() defp process_bounds_update(map_bounds, socket) do - Logger.debug("process_bounds_update: Loading historical packets for bounds #{inspect(map_bounds)}") - # Remove out-of-bounds packets and markers immediately new_visible_packets = socket.assigns.visible_packets diff --git a/lib/aprs_web/live/map_live/map_helpers.ex b/lib/aprs_web/live/map_live/map_helpers.ex index 2492cc6..b94bb6e 100644 --- a/lib/aprs_web/live/map_live/map_helpers.ex +++ b/lib/aprs_web/live/map_live/map_helpers.ex @@ -1,8 +1,5 @@ defmodule AprsWeb.MapLive.MapHelpers do - @moduledoc """ - Shared helpers for APRS map LiveViews (main map, callsign map, etc). - Provides coordinate extraction, position checks, and bounds logic. - """ + @moduledoc false alias Parser.Types.MicE diff --git a/lib/aprs_web/live/packets_live/callsign_view.ex b/lib/aprs_web/live/packets_live/callsign_view.ex index 0b4763e..9150f18 100644 --- a/lib/aprs_web/live/packets_live/callsign_view.ex +++ b/lib/aprs_web/live/packets_live/callsign_view.ex @@ -1,10 +1,5 @@ defmodule AprsWeb.PacketsLive.CallsignView do - @moduledoc """ - LiveView for displaying packets specific to a single callsign. - - Shows up to 100 packets total (stored + live) for the specified callsign. - Includes both stored packets from the database and live incoming packets. - """ + @moduledoc false use AprsWeb, :live_view import Ecto.Query diff --git a/lib/aprs_web/live/weather_live/callsign_view.ex b/lib/aprs_web/live/weather_live/callsign_view.ex index bfab6ac..4aee465 100644 --- a/lib/aprs_web/live/weather_live/callsign_view.ex +++ b/lib/aprs_web/live/weather_live/callsign_view.ex @@ -1,4 +1,5 @@ defmodule AprsWeb.WeatherLive.CallsignView do + @moduledoc false use AprsWeb, :live_view alias Aprs.Packets diff --git a/lib/aprs_web/time_helpers.ex b/lib/aprs_web/time_helpers.ex index 5eba144..a84df51 100644 --- a/lib/aprs_web/time_helpers.ex +++ b/lib/aprs_web/time_helpers.ex @@ -10,18 +10,20 @@ defmodule AprsWeb.TimeHelpers do now = DateTime.utc_now() diff_seconds = DateTime.diff(now, datetime, :second) - cond do - diff_seconds < 60 -> "less than a minute" - diff_seconds < 120 -> "1 minute" - diff_seconds < 3600 -> "#{div(diff_seconds, 60)} minutes" - diff_seconds < 7200 -> "1 hour" - diff_seconds < 86_400 -> "#{div(diff_seconds, 3600)} hours" - diff_seconds < 172_800 -> "1 day" - diff_seconds < 2_592_000 -> "#{div(diff_seconds, 86_400)} days" - diff_seconds < 5_184_000 -> "1 month" - diff_seconds < 31_536_000 -> "#{div(diff_seconds, 2_592_000)} months" - diff_seconds < 63_072_000 -> "1 year" - true -> "#{div(diff_seconds, 31_536_000)} years" - end <> " ago" + format_time_diff(diff_seconds) <> " ago" end + + defp format_time_diff(seconds) when seconds < 60, do: "less than a minute" + defp format_time_diff(seconds) when seconds < 120, do: "1 minute" + defp format_time_diff(seconds) when seconds < 3600, do: "#{div(seconds, 60)} minutes" + defp format_time_diff(seconds) when seconds < 7200, do: "1 hour" + defp format_time_diff(seconds) when seconds < 86_400, do: "#{div(seconds, 3600)} hours" + defp format_time_diff(seconds) when seconds < 172_800, do: "1 day" + defp format_time_diff(seconds) when seconds < 2_592_000, do: "#{div(seconds, 86_400)} days" + defp format_time_diff(seconds) when seconds < 5_184_000, do: "1 month" + + defp format_time_diff(seconds) when seconds < 31_536_000, do: "#{div(seconds, 2_592_000)} months" + + defp format_time_diff(seconds) when seconds < 63_072_000, do: "1 year" + defp format_time_diff(seconds), do: "#{div(seconds, 31_536_000)} years" end diff --git a/lib/aprs_web/user_auth.ex b/lib/aprs_web/user_auth.ex index 75afd35..7fef8c3 100644 --- a/lib/aprs_web/user_auth.ex +++ b/lib/aprs_web/user_auth.ex @@ -41,9 +41,7 @@ defmodule AprsWeb.UserAuth do put_resp_cookie(conn, @remember_me_cookie, token, @remember_me_options) end - defp maybe_write_remember_me_cookie(conn, _token, _params) do - conn - end + defp maybe_write_remember_me_cookie(conn, _token, _params), do: conn # This function renews the session ID and erases the whole # session to avoid fixation attacks. If there is any data @@ -96,16 +94,18 @@ defmodule AprsWeb.UserAuth do end defp ensure_user_token(conn) do - if token = get_session(conn, :user_token) do - {token, conn} - else - conn = fetch_cookies(conn, signed: [@remember_me_cookie]) + case get_session(conn, :user_token) do + nil -> ensure_user_token_from_cookie(conn) + token -> {token, conn} + end + end - if token = conn.cookies[@remember_me_cookie] do - {token, put_token_in_session(conn, token)} - else - {nil, conn} - end + defp ensure_user_token_from_cookie(conn) do + conn = fetch_cookies(conn, signed: [@remember_me_cookie]) + + case conn.cookies[@remember_me_cookie] do + nil -> {nil, conn} + token -> {token, put_token_in_session(conn, token)} end end @@ -151,25 +151,26 @@ defmodule AprsWeb.UserAuth do def on_mount(:ensure_authenticated, _params, session, socket) do socket = mount_current_user(session, socket) - if socket.assigns.current_user do - {:cont, socket} - else - socket = - socket - |> Phoenix.LiveView.put_flash(:error, "You must log in to access this page.") - |> Phoenix.LiveView.redirect(to: ~p"/users/log_in") + case socket.assigns.current_user do + nil -> + socket = + socket + |> Phoenix.LiveView.put_flash(:error, "You must log in to access this page.") + |> Phoenix.LiveView.redirect(to: ~p"/users/log_in") - {:halt, socket} + {:halt, socket} + + _user -> + {:cont, socket} end end def on_mount(:redirect_if_user_is_authenticated, _params, session, socket) do socket = mount_current_user(session, socket) - if socket.assigns.current_user do - {:halt, Phoenix.LiveView.redirect(socket, to: signed_in_path(socket))} - else - {:cont, socket} + case socket.assigns.current_user do + nil -> {:cont, socket} + _user -> {:halt, Phoenix.LiveView.redirect(socket, to: signed_in_path(socket))} end end @@ -189,12 +190,14 @@ defmodule AprsWeb.UserAuth do Used for routes that require the user to not be authenticated. """ def redirect_if_user_is_authenticated(conn, _opts) do - if conn.assigns[:current_user] do - conn - |> redirect(to: signed_in_path(conn)) - |> halt() - else - conn + case conn.assigns[:current_user] do + nil -> + conn + + _user -> + conn + |> redirect(to: signed_in_path(conn)) + |> halt() end end @@ -205,14 +208,16 @@ defmodule AprsWeb.UserAuth do they use the application at all, here would be a good place. """ def require_authenticated_user(conn, _opts) do - if conn.assigns[:current_user] do - conn - else - conn - |> put_flash(:error, "You must log in to access this page.") - |> maybe_store_return_to() - |> redirect(to: ~p"/users/log_in") - |> halt() + case conn.assigns[:current_user] do + nil -> + conn + |> put_flash(:error, "You must log in to access this page.") + |> maybe_store_return_to() + |> redirect(to: ~p"/users/log_in") + |> halt() + + _user -> + conn end end diff --git a/lib/parser.ex b/lib/parser.ex index 4c8b78d..a3b832f 100644 --- a/lib/parser.ex +++ b/lib/parser.ex @@ -8,8 +8,6 @@ defmodule Parser do alias Aprs.Convert alias Parser.MicE - require Logger - # Simple APRS position parsing to replace parse_aprs_position defp parse_aprs_position(lat, lon) do # Regex for latitude: 2 deg, 2+ min, 1 dir (N/S) @@ -54,8 +52,7 @@ defmodule Parser do def parse(message) when is_binary(message) do do_parse(message) rescue - error -> - Logger.debug("PARSE ERROR: #{inspect(error)} for message: #{message}") + _ -> {:error, :invalid_packet} end @@ -112,8 +109,7 @@ defmodule Parser do end end rescue - error -> - Logger.debug("PARSE ERROR: #{inspect(error)} for message: #{message}") + _ -> {:error, :invalid_packet} end diff --git a/lib/parser/helpers.ex b/lib/parser/helpers.ex index 8f02e4f..93cac76 100644 --- a/lib/parser/helpers.ex +++ b/lib/parser/helpers.ex @@ -3,6 +3,8 @@ defmodule Parser.Helpers do Public helper functions for APRS parsing (NMEA, PHG/DF, compressed position, etc). """ + import Decimal, only: [new: 1, add: 2, negate: 1] + @type phg_power :: {integer() | nil, String.t()} @type phg_height :: {integer() | nil, String.t()} @type phg_gain :: {integer() | nil, String.t()} @@ -16,7 +18,7 @@ defmodule Parser.Helpers do {coord, _} -> coord = coord / 100.0 coord = apply_nmea_direction(coord, direction) - if is_tuple(coord), do: coord, else: {:ok, coord} + handle_coordinate_result(coord) :error -> {:error, "Invalid coordinate value"} @@ -26,6 +28,9 @@ defmodule Parser.Helpers do @spec parse_nmea_coordinate(any(), any()) :: {:error, String.t()} def parse_nmea_coordinate(_, _), do: {:error, "Invalid coordinate format"} + defp handle_coordinate_result(coord) when is_tuple(coord), do: coord + defp handle_coordinate_result(coord), do: {:ok, coord} + defp apply_nmea_direction(coord, direction) do case direction do "N" -> coord @@ -317,13 +322,16 @@ defmodule Parser.Helpers do case Regex.run(~r/h(\d{2})/, weather_data) do [_, humidity] -> val = String.to_integer(humidity) - if val == 0, do: 100, else: val + normalize_humidity(val) nil -> nil end end + defp normalize_humidity(0), do: 100 + defp normalize_humidity(val), do: val + @spec parse_pressure(String.t()) :: float() | nil def parse_pressure(weather_data) do case Regex.run(~r/b(\d{5})/, weather_data) do @@ -349,18 +357,23 @@ defmodule Parser.Helpers do end # Ambiguity and utility helpers + @doc false def count_spaces(str) do str |> String.graphemes() |> Enum.count(fn c -> c == " " end) end + @doc false def count_leading_braces(packet), do: count_leading_braces(packet, 0) + @doc false def count_leading_braces(<<"}", rest::binary>>, count), do: count_leading_braces(rest, count + 1) + @doc false def count_leading_braces(_packet, count), do: count + @doc false def calculate_position_ambiguity(latitude, longitude) do lat_spaces = count_spaces(latitude) lon_spaces = count_spaces(longitude) @@ -372,6 +385,7 @@ defmodule Parser.Helpers do ) end + @doc false def calculate_compressed_ambiguity(compression_type) do case compression_type do " " -> 0 @@ -383,6 +397,7 @@ defmodule Parser.Helpers do end end + @doc false def find_matches(regex, text) do case Regex.names(regex) do [] -> @@ -397,10 +412,20 @@ defmodule Parser.Helpers do end end + @doc false def parse_manufacturer(symbols) do Aprs.DeviceIdentification.identify_device(symbols) end + @doc false + defp apply_latitude_direction(lat_val, "S"), do: negate(lat_val) + defp apply_latitude_direction(lat_val, _), do: lat_val + + @doc false + defp apply_longitude_direction(lon_val, "W"), do: negate(lon_val) + defp apply_longitude_direction(lon_val, _), do: lon_val + + @doc false def convert_to_base91(<>) do [v1, v2, v3, v4] = to_charlist(value) (v1 - 33) * 91 * 91 * 91 + (v2 - 33) * 91 * 91 + (v3 - 33) * 91 + v4 @@ -429,14 +454,13 @@ defmodule Parser.Helpers do end end + @doc false def validate_position_data(latitude, longitude) do - import Decimal, only: [new: 1, add: 2, negate: 1] - lat = case Regex.run(~r/^(\d{2})(\d{2}\.\d+)([NS])$/, latitude) do [_, degrees, minutes, direction] -> lat_val = add(new(degrees), Decimal.div(new(minutes), new("60"))) - if direction == "S", do: negate(lat_val), else: lat_val + apply_latitude_direction(lat_val, direction) _ -> nil @@ -446,7 +470,7 @@ defmodule Parser.Helpers do case Regex.run(~r/^(\d{3})(\d{2}\.\d+)([EW])$/, longitude) do [_, degrees, minutes, direction] -> lon_val = add(new(degrees), Decimal.div(new(minutes), new("60"))) - if direction == "W", do: negate(lon_val), else: lon_val + apply_longitude_direction(lon_val, direction) _ -> nil @@ -459,5 +483,6 @@ defmodule Parser.Helpers do end end + @doc false def validate_timestamp(_time), do: nil end diff --git a/lib/parser/mic_e.ex b/lib/parser/mic_e.ex index cc3a744..9651058 100644 --- a/lib/parser/mic_e.ex +++ b/lib/parser/mic_e.ex @@ -4,7 +4,16 @@ defmodule Parser.MicE do """ @spec parse(binary(), String.t()) :: map() - def parse(data, destination \\ nil) do + def parse(_data, nil) do + %{ + latitude: nil, + longitude: nil, + error: "Destination is nil", + data_type: :mic_e_error + } + end + + def parse(data, destination) do with {:ok, dest_info} <- parse_destination(destination), {:ok, info_info} <- parse_information(data, dest_info.longitude_offset) do lat = @@ -19,7 +28,7 @@ defmodule Parser.MicE do ) ) - lat = if dest_info.lat_direction == :south, do: Decimal.negate(lat), else: lat + lat = apply_lat_direction(lat, dest_info.lat_direction) lon = Decimal.add( @@ -33,7 +42,7 @@ defmodule Parser.MicE do ) ) - lon = if dest_info.lon_direction == :west, do: Decimal.negate(lon), else: lon + lon = apply_lon_direction(lon, dest_info.lon_direction) %{ latitude: lat, @@ -212,7 +221,7 @@ defmodule Parser.MicE do sp = sp_c - 28 dc = dc_c - 28 speed = div(sp, 10) * 100 + rem(sp, 10) * 10 + div(dc, 10) - speed = if speed >= 800, do: speed - 800, else: speed + speed = normalize_speed(speed) speed * 0.868976 end @@ -220,6 +229,18 @@ defmodule Parser.MicE do dc = dc_c - 28 se = se_c - 28 course = rem(dc, 10) * 100 + se - if course >= 400, do: course - 400, else: course + normalize_course(course) end + + defp apply_lat_direction(lat, :south), do: Decimal.negate(lat) + defp apply_lat_direction(lat, _), do: lat + + defp apply_lon_direction(lon, :west), do: Decimal.negate(lon) + defp apply_lon_direction(lon, _), do: lon + + defp normalize_speed(speed) when speed >= 800, do: speed - 800 + defp normalize_speed(speed), do: speed + + defp normalize_course(course) when course >= 400, do: course - 400 + defp normalize_course(course), do: course end diff --git a/lib/parser/object.ex b/lib/parser/object.ex index d13a933..b3e7d1b 100644 --- a/lib/parser/object.ex +++ b/lib/parser/object.ex @@ -11,20 +11,6 @@ defmodule Parser.Object do def parse(<<";", object_name::binary-size(9), live_killed::binary-size(1), timestamp::binary-size(7), rest::binary>>) do position_data = case rest do - <> -> - %{latitude: lat, longitude: lon} = - Parser.Position.parse_aprs_position(latitude, longitude) - - %{ - latitude: lat, - longitude: lon, - symbol_table_id: sym_table_id, - symbol_code: symbol_code, - comment: comment, - position_format: :uncompressed - } - <<"/", latitude_compressed::binary-size(4), longitude_compressed::binary-size(4), symbol_code::binary-size(1), cs::binary-size(2), compression_type::binary-size(1), comment::binary>> -> try do @@ -47,6 +33,20 @@ defmodule Parser.Object do _ -> %{latitude: nil, longitude: nil, comment: comment, position_format: :compressed} end + <> -> + %{latitude: lat, longitude: lon} = + Parser.Position.parse_aprs_position(latitude, longitude) + + %{ + latitude: lat, + longitude: lon, + symbol_table_id: sym_table_id, + symbol_code: symbol_code, + comment: comment, + position_format: :uncompressed + } + _ -> %{comment: rest, position_format: :unknown} end diff --git a/lib/parser/weather.ex b/lib/parser/weather.ex index d208591..68a6d1f 100644 --- a/lib/parser/weather.ex +++ b/lib/parser/weather.ex @@ -42,7 +42,10 @@ defmodule Parser.Weather do result = %{timestamp: timestamp, data_type: :weather, raw_weather_data: weather_data} Enum.reduce(weather_values, result, fn {key, value}, acc -> - if is_nil(value), do: acc, else: Map.put(acc, key, value) + put_weather_value(acc, key, value) end) end + + defp put_weather_value(acc, _key, nil), do: acc + defp put_weather_value(acc, key, value), do: Map.put(acc, key, value) end diff --git a/test/aprs/is_test.exs b/test/aprs/is_test.exs index 1bac173..45d6695 100644 --- a/test/aprs/is_test.exs +++ b/test/aprs/is_test.exs @@ -5,8 +5,6 @@ defmodule Aprs.IsTest do use ExUnit.Case, async: false use Aprs.DataCase - require Logger - describe "APRS-IS mock functionality" do setup do # Start the mock if not already running diff --git a/test/aprs_web/integration/aprs_status_test.exs b/test/aprs_web/integration/aprs_status_test.exs index 044dafc..96fe57c 100644 --- a/test/aprs_web/integration/aprs_status_test.exs +++ b/test/aprs_web/integration/aprs_status_test.exs @@ -7,6 +7,9 @@ defmodule AprsWeb.Integration.AprsStatusTest do import Phoenix.LiveViewTest + alias AprsWeb.Endpoint + alias Ecto.Adapters.SQL + describe "APRS status endpoints without external connections" do test "status JSON endpoint returns proper response without APRS connection", %{conn: conn} do conn = get(conn, "/status.json") @@ -55,7 +58,7 @@ defmodule AprsWeb.Integration.AprsStatusTest do case live(conn, "/status") do {:ok, view, html} -> # If status page exists, verify it handles disconnected state - assert html =~ "Status" + assert html =~ "STATUS" # Should show disconnected state information assert has_element?(view, "[data-testid='connection-status']") || @@ -128,7 +131,7 @@ defmodule AprsWeb.Integration.AprsStatusTest do } # This should not cause any updates since APRS.Is is not running - AprsWeb.Endpoint.broadcast("aprs_messages", "packet", test_packet) + Endpoint.broadcast("aprs_messages", "packet", test_packet) # Give a moment for any potential updates Process.sleep(100) @@ -162,7 +165,7 @@ defmodule AprsWeb.Integration.AprsStatusTest do # Verify that core application functionality works without APRS # Database should be accessible - assert Ecto.Adapters.SQL.query!(Aprs.Repo, "SELECT 1", []) + assert SQL.query!(Aprs.Repo, "SELECT 1", []) # Web interface should load {:ok, _view, html} = live(conn, "/") diff --git a/test/aprs_web/live/map_live/callsign_view_test.exs b/test/aprs_web/live/map_live/callsign_view_test.exs index bac0efa..23f6fb2 100644 --- a/test/aprs_web/live/map_live/callsign_view_test.exs +++ b/test/aprs_web/live/map_live/callsign_view_test.exs @@ -7,7 +7,7 @@ defmodule AprsWeb.MapLive.CallsignViewTest do test "renders callsign view with valid callsign", %{conn: conn} do {:ok, view, html} = live(conn, "/W5ISP-9") - assert html =~ "📡 W5ISP-9" + assert html =~ "W5ISP-9" assert html =~ "Back to Map" assert html =~ "Packets" assert has_element?(view, "#aprs-map") @@ -16,7 +16,7 @@ defmodule AprsWeb.MapLive.CallsignViewTest do test "normalizes callsign to uppercase", %{conn: conn} do {:ok, _view, html} = live(conn, "/w5isp-9") - assert html =~ "📡 W5ISP-9" + assert html =~ "W5ISP-9" end test "shows loading state when no packets found", %{conn: conn} do @@ -29,7 +29,7 @@ defmodule AprsWeb.MapLive.CallsignViewTest do test "handles callsign without SSID", %{conn: conn} do {:ok, _view, html} = live(conn, "/W5ISP") - assert html =~ "📡 W5ISP" + assert html =~ "W5ISP" end test "sets correct page title", %{conn: conn} do diff --git a/test/aprs_web/live/user_confirmation_live_test.exs b/test/aprs_web/live/user_confirmation_live_test.exs index ba05416..21fb095 100644 --- a/test/aprs_web/live/user_confirmation_live_test.exs +++ b/test/aprs_web/live/user_confirmation_live_test.exs @@ -14,7 +14,7 @@ defmodule AprsWeb.UserConfirmationLiveTest do describe "Confirm user" do test "renders confirmation page", %{conn: conn} do {:ok, _lv, html} = live(conn, ~p"/users/confirm/some-token") - assert html =~ "Confirm Account" + assert html =~ "Confirm my account" end test "confirms the given token once", %{conn: conn, user: user} do diff --git a/test/integration/historical_packets_test.exs b/test/integration/historical_packets_test.exs index d05e95f..79277f2 100644 --- a/test/integration/historical_packets_test.exs +++ b/test/integration/historical_packets_test.exs @@ -1,9 +1,15 @@ defmodule Aprs.Integration.HistoricalPacketsTest do use AprsWeb.ConnCase + import Aprs.MockHelpers import Phoenix.LiveViewTest - @moduletag :skip_packets_mock + setup do + Mox.set_mox_global() + Application.put_env(:aprs, :packets_module, Aprs.PacketsMock) + on_exit(fn -> Mox.set_mox_private() end) + :ok + end describe "historical packet loading" do setup do @@ -78,7 +84,10 @@ defmodule Aprs.Integration.HistoricalPacketsTest do {:ok, packets: [packet1, packet2, packet3, packet4]} end - test "loads all historical packets at once when map is ready", %{conn: conn, packets: mock_packets} do + test "loads all historical packets at once when map is ready", %{ + conn: conn, + packets: mock_packets + } do # Mock the Packets.get_packets_for_replay function expect_packets_for_replay_with_bounds(mock_packets) @@ -94,14 +103,29 @@ defmodule Aprs.Integration.HistoricalPacketsTest do } } - # Update bounds first + # Set historical_hours assign + render_hook(view, "update_historical_hours", %{"historical_hours" => "1"}) + Process.sleep(100) + + # Set bounds and wait for map_bounds to be set in assigns render_hook(view, "bounds_changed", bounds_params) - # Trigger map ready event - render_hook(view, "map_ready", %{}) + send( + view.pid, + {:process_bounds_update, + %{ + north: bounds_params["bounds"]["north"], + south: bounds_params["bounds"]["south"], + east: bounds_params["bounds"]["east"], + west: bounds_params["bounds"]["west"] + }} + ) - # Wait for the historical packet loading to complete - Process.sleep(700) + Process.sleep(100) + + # Now trigger map_ready + render_hook(view, "map_ready", %{}) + Process.sleep(1200) # The LiveView should have pushed an event with historical packets # Note: In real implementation, we'd need to verify the push_event was called @@ -127,14 +151,29 @@ defmodule Aprs.Integration.HistoricalPacketsTest do } } - # Update bounds first + # Set historical_hours assign + render_hook(view, "update_historical_hours", %{"historical_hours" => "1"}) + Process.sleep(100) + + # Set bounds and wait for map_bounds to be set in assigns render_hook(view, "bounds_changed", bounds_params) - # Trigger map ready event - render_hook(view, "map_ready", %{}) + send( + view.pid, + {:process_bounds_update, + %{ + north: bounds_params["bounds"]["north"], + south: bounds_params["bounds"]["south"], + east: bounds_params["bounds"]["east"], + west: bounds_params["bounds"]["west"] + }} + ) - # Wait for the historical packet loading to complete - Process.sleep(700) + Process.sleep(100) + + # Now trigger map_ready + render_hook(view, "map_ready", %{}) + Process.sleep(1200) verify!() end @@ -145,11 +184,32 @@ defmodule Aprs.Integration.HistoricalPacketsTest do {:ok, view, _html} = live(conn, "/") - # Trigger map ready event + # Set historical_hours assign + render_hook(view, "update_historical_hours", %{"historical_hours" => "1"}) + + # Set bounds and synchronously update map_bounds + bounds_params = %{ + "bounds" => %{"north" => "45.0", "south" => "35.0", "east" => "-90.0", "west" => "-105.0"} + } + + render_hook(view, "bounds_changed", bounds_params) + + send( + view.pid, + {:process_bounds_update, + %{ + north: bounds_params["bounds"]["north"], + south: bounds_params["bounds"]["south"], + east: bounds_params["bounds"]["east"], + west: bounds_params["bounds"]["west"] + }} + ) + + # Now trigger map_ready render_hook(view, "map_ready", %{}) - # Wait for the historical packet loading attempt - Process.sleep(700) + # Give a very short time for message processing, if needed + Process.sleep(20) # Should not crash verify!() @@ -170,7 +230,10 @@ defmodule Aprs.Integration.HistoricalPacketsTest do verify!() end - test "handles locate_me event after historical packets are loaded", %{conn: conn, packets: mock_packets} do + test "handles locate_me event after historical packets are loaded", %{ + conn: conn, + packets: mock_packets + } do expect_packets_for_replay(mock_packets) {:ok, view, _html} = live(conn, "/") @@ -210,7 +273,10 @@ defmodule Aprs.Integration.HistoricalPacketsTest do {:ok, historical_packet: historical_packet} end - test "new live packet updates marker for same callsign", %{conn: conn, historical_packet: historical_packet} do + test "new live packet updates marker for same callsign", %{ + conn: conn, + historical_packet: historical_packet + } do expect_packets_for_replay([historical_packet]) {:ok, view, _html} = live(conn, "/") @@ -226,9 +292,26 @@ defmodule Aprs.Integration.HistoricalPacketsTest do } render_hook(view, "bounds_changed", bounds_params) + + send( + view.pid, + {:process_bounds_update, + %{ + north: bounds_params["bounds"]["north"], + south: bounds_params["bounds"]["south"], + east: bounds_params["bounds"]["east"], + west: bounds_params["bounds"]["west"] + }} + ) + + Process.sleep(100) render_hook(view, "map_ready", %{}) Process.sleep(700) + # Set historical_hours assign + render_hook(view, "update_historical_hours", %{"historical_hours" => "1"}) + Process.sleep(100) + # Simulate a new live packet for the same callsign new_packet = %{ id: "LIVE1", diff --git a/test/support/aprs_is_mock.ex b/test/support/aprs_is_mock.ex index f1f5d62..f35fdb8 100644 --- a/test/support/aprs_is_mock.ex +++ b/test/support/aprs_is_mock.ex @@ -14,8 +14,6 @@ defmodule AprsIsMock do @impl true def init(_opts) do - Logger.info("Starting APRS.Is mock for testing") - # Mock connection state initial_state = %{ connected: false, @@ -40,7 +38,6 @@ defmodule AprsIsMock do # Client API - Mock implementations def stop do - Logger.info("Stopping APRS.Is mock") GenServer.stop(__MODULE__, :normal) end @@ -93,31 +90,26 @@ defmodule AprsIsMock do end end - def set_filter(filter_string) do - Logger.debug("Mock: Setting filter to #{filter_string}") + def set_filter(_filter_string) do :ok end def list_active_filters do - Logger.debug("Mock: Listing active filters") :ok end - def send_message(from, to, message) do - Logger.debug("Mock: Sending message from #{from} to #{to}: #{message}") + def send_message(_from, _to, _message) do :ok end def send_message(message) do - Logger.debug("Mock: Sending message: #{message}") GenServer.call(__MODULE__, {:send_message, message}) end # Server callbacks @impl true - def handle_call({:send_message, message}, _from, state) do - Logger.debug("Mock: Handling send message: #{message}") + def handle_call({:send_message, _message}, _from, state) do {:reply, :ok, state} end @@ -146,14 +138,12 @@ defmodule AprsIsMock do end @impl true - def handle_info(msg, state) do - Logger.debug("Mock: Received unexpected message: #{inspect(msg)}") + def handle_info(_msg, state) do {:noreply, state} end @impl true - def terminate(reason, _state) do - Logger.info("Mock APRS.Is terminating: #{inspect(reason)}") + def terminate(_reason, _state) do :ok end @@ -163,7 +153,6 @@ defmodule AprsIsMock do # Simulate receiving an APRS packet for testing purposes. # This can be used in tests to trigger packet processing without # connecting to external servers. - Logger.debug("Mock: Simulating packet: #{inspect(packet_data)}") # Broadcast to live clients like the real implementation would AprsWeb.Endpoint.broadcast("aprs_messages", "packet", packet_data) diff --git a/test/support/mocks.ex b/test/support/mocks.ex index 77fbc3c..0e93b3e 100644 --- a/test/support/mocks.ex +++ b/test/support/mocks.ex @@ -20,14 +20,16 @@ defmodule Aprs.MockHelpers do Sets up an expectation for PacketsMock with custom return value. """ def expect_packets_for_replay(packets) do - expect(Aprs.PacketsMock, :get_packets_for_replay, fn _opts -> packets end) + stub(Aprs.PacketsMock, :get_packets_for_replay, fn _opts -> + packets + end) end @doc """ Sets up an expectation for PacketsMock with filtering based on bounds. """ def expect_packets_for_replay_with_bounds(packets) do - expect(Aprs.PacketsMock, :get_packets_for_replay, fn opts -> + stub(Aprs.PacketsMock, :get_packets_for_replay, fn opts -> case opts[:bounds] do [west, south, east, north] -> Enum.filter(packets, fn packet -> diff --git a/test/test_helper.exs b/test/test_helper.exs index 5e49136..dfe59c1 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -12,5 +12,6 @@ Application.put_env(:aprs, :aprs_is_port, 14_580) Application.put_env(:aprs, :aprs_is_login_id, "TEST") Application.put_env(:aprs, :aprs_is_password, "-1") Application.put_env(:aprs, :aprs_is_default_filter, "r/0/0/1") +Application.put_env(:aprs, :packets_module, Aprs.PacketsMock) # AprsIsMock is automatically loaded from test/support via elixirc_paths