diff --git a/lib/aprsme/accounts.ex b/lib/aprsme/accounts.ex index 419b5ec..20947f4 100644 --- a/lib/aprsme/accounts.ex +++ b/lib/aprsme/accounts.ex @@ -41,19 +41,11 @@ defmodule Aprsme.Accounts do """ def get_user_by_email_and_password(email, password) when is_binary(email) and is_binary(password) do - user = - try do - Repo.get_by(User, email: email) - rescue - _ -> nil - end - - validate_user_password(user, password) - end - - defp validate_user_password(user, password) do - if User.valid_password?(user, password) do + with %User{} = user <- Repo.get_by(User, email: email), + true <- User.valid_password?(user, password) do user + else + _ -> nil end end @@ -229,10 +221,7 @@ defmodule Aprsme.Accounts do Ecto.Multi.new() |> Ecto.Multi.update(:user, changeset) |> Repo.transaction() - |> case do - {:ok, %{user: user}} -> {:ok, user} - {:error, :user, changeset, _} -> {:error, changeset} - end + |> transaction_unwrap() end @doc """ @@ -273,10 +262,7 @@ defmodule Aprsme.Accounts do |> Ecto.Multi.update(:user, changeset) |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, :all)) |> Repo.transaction() - |> case do - {:ok, %{user: user}} -> {:ok, user} - {:error, :user, changeset, _} -> {:error, changeset} - end + |> transaction_unwrap() end ## Session @@ -415,7 +401,16 @@ defmodule Aprsme.Accounts do |> Ecto.Multi.update(:user, User.password_changeset(user, attrs)) |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, :all)) |> Repo.transaction() - |> case do + |> transaction_unwrap() + end + + # --- Private helpers --- + + # Unwraps an Ecto.Multi transaction result, converting the verbose + # {:ok, %{key: value}} / {:error, key, changeset, _} tuple into the + # standard {:ok, value} / {:error, changeset} shape used by callers. + defp transaction_unwrap(multi_result) do + case multi_result do {:ok, %{user: user}} -> {:ok, user} {:error, :user, changeset, _} -> {:error, changeset} end diff --git a/lib/aprsme/packets.ex b/lib/aprsme/packets.ex index 7427f87..69bf04a 100644 --- a/lib/aprsme/packets.ex +++ b/lib/aprsme/packets.ex @@ -162,23 +162,37 @@ defmodule Aprsme.Packets do find_coordinate_value(map, coord_type) || find_coordinate_value(map, coord_string) end - # Pattern matching for direct coordinate access - defp find_coordinate_value(%{latitude: lat}, :latitude) when not is_nil(lat), do: lat - defp find_coordinate_value(%{longitude: lon}, :longitude) when not is_nil(lon), do: lon - defp find_coordinate_value(%{"latitude" => lat}, "latitude") when not is_nil(lat), do: lat - defp find_coordinate_value(%{"longitude" => lon}, "longitude") when not is_nil(lon), do: lon + # Pattern matching for coordinate access — normalizes atom/string keys and + # nested/flat maps into a uniform lookup before extracting the value. + defp find_coordinate_value(map, coord_type) do + key = coord_key(coord_type) + direct_get(map, key) || deep_get(map, [:position, key]) + end - # Pattern matching for nested position access - defp find_coordinate_value(%{position: %{latitude: lat}}, :latitude) when not is_nil(lat), do: lat - defp find_coordinate_value(%{position: %{longitude: lon}}, :longitude) when not is_nil(lon), do: lon - defp find_coordinate_value(%{position: %{"latitude" => lat}}, :latitude) when not is_nil(lat), do: lat - defp find_coordinate_value(%{position: %{"longitude" => lon}}, :longitude) when not is_nil(lon), do: lon - defp find_coordinate_value(%{"position" => %{latitude: lat}}, "latitude") when not is_nil(lat), do: lat - defp find_coordinate_value(%{"position" => %{longitude: lon}}, "longitude") when not is_nil(lon), do: lon - defp find_coordinate_value(%{"position" => %{"latitude" => lat}}, "latitude") when not is_nil(lat), do: lat - defp find_coordinate_value(%{"position" => %{"longitude" => lon}}, "longitude") when not is_nil(lon), do: lon + defp coord_key(k) when is_atom(k), do: Atom.to_string(k) + defp coord_key(k), do: k - defp find_coordinate_value(_, _), do: nil + defp direct_get(map, key) when is_map(map) do + Map.get(map, key) || Map.get(map, String.to_existing_atom(key)) + rescue + ArgumentError -> nil + end + + defp direct_get(_, _), do: nil + + defp deep_get(map, []) when is_map(map), do: map + + defp deep_get(map, [head | tail]) when is_map(map) do + case Map.get(map, head) || Map.get(map, String.to_existing_atom(head)) do + nil -> nil + inner when is_map(inner) -> deep_get(inner, tail) + _ -> nil + end + rescue + ArgumentError -> nil + end + + defp deep_get(_, _), do: nil defp set_lat_lon(attrs, lat, lon) do # Optimize: avoid creating anonymous function on each call @@ -247,55 +261,29 @@ defmodule Aprsme.Packets do """ @spec store_bad_packet(map() | String.t(), any()) :: {:ok, struct()} | {:error, Ecto.Changeset.t()} - def store_bad_packet(packet_data, error) when is_binary(packet_data) do - error_type = - case error do - %{type: type} -> type - %{__struct__: struct} -> struct |> to_string() |> String.replace("Elixir.", "") - _ -> "UnknownError" - end - - error_message = - case error do - %{message: message} -> message - %{__struct__: _} -> Exception.message(error) - _ -> inspect(error) - end - + def store_bad_packet(packet_data, error) do %BadPacket{} |> BadPacket.changeset(%{ - raw_packet: Aprsme.EncodingUtils.sanitize_string(packet_data), - error_message: error_message, - error_type: error_type, + raw_packet: raw_packet_string(packet_data), + error_message: extract_error_message(error), + error_type: extract_error_type(error), attempted_at: DateTime.utc_now() }) |> Repo.insert() end - def store_bad_packet(packet_data, error) when is_map(packet_data) do - error_type = - case error do - %{type: type} -> type - %{__struct__: struct} -> struct |> to_string() |> String.replace("Elixir.", "") - _ -> "UnknownError" - end + defp raw_packet_string(data) when is_binary(data), do: Aprsme.EncodingUtils.sanitize_string(data) + defp raw_packet_string(%{raw_packet: rp}), do: rp + defp raw_packet_string(%{"raw_packet" => rp}), do: rp + defp raw_packet_string(data), do: inspect(data) - error_message = - case error do - %{message: message} -> message - %{__struct__: _} -> Exception.message(error) - _ -> inspect(error) - end + defp extract_error_type(%{type: type}), do: type + defp extract_error_type(%{__struct__: struct}), do: struct |> to_string() |> String.replace("Elixir.", "") + defp extract_error_type(_), do: "UnknownError" - %BadPacket{} - |> BadPacket.changeset(%{ - raw_packet: packet_data[:raw_packet] || packet_data["raw_packet"] || inspect(packet_data), - error_message: error_message, - error_type: error_type, - attempted_at: DateTime.utc_now() - }) - |> Repo.insert() - end + defp extract_error_message(%{message: message}), do: message + defp extract_error_message(%{__struct__: _} = error), do: Exception.message(error) + defp extract_error_message(error), do: inspect(error) # Extracts position data from packet, checking various possible locations defp extract_position(packet_data) do diff --git a/lib/aprsme_web/live/map_live/data_builder.ex b/lib/aprsme_web/live/map_live/data_builder.ex index 998fce1..43f340f 100644 --- a/lib/aprsme_web/live/map_live/data_builder.ex +++ b/lib/aprsme_web/live/map_live/data_builder.ex @@ -215,23 +215,22 @@ defmodule AprsmeWeb.MapLive.DataBuilder do """ @spec build_simple_popup(map(), boolean()) :: String.t() def build_simple_popup(packet, has_weather) do - # Build popup HTML directly without database queries - callsign = map_label(packet) - timestamp_dt = get_packet_received_at(packet) - cache_buster = System.system_time(:millisecond) + packet + |> build_simple_popup_data() + |> Map.put(:weather_link, has_weather || weather_packet?(packet)) + |> render_popup() + end - # Check if this packet itself is a weather packet + defp build_simple_popup_data(packet) do is_weather = weather_packet?(packet) if is_weather do - # Build weather popup %{ - callsign: callsign, + callsign: map_label(packet), comment: nil, - timestamp_dt: timestamp_dt, - cache_buster: cache_buster, + timestamp_dt: get_packet_received_at(packet), + cache_buster: System.system_time(:millisecond), weather: true, - weather_link: true, temperature: get_weather_field(packet, :temperature), temp_unit: "°F", humidity: get_weather_field(packet, :humidity), @@ -248,29 +247,27 @@ defmodule AprsmeWeb.MapLive.DataBuilder do rain_24h_unit: "in", rain_since_midnight_unit: "in" } - |> PopupComponent.popup() - |> Safe.to_iodata() - |> IO.iodata_to_binary() else - # Build standard popup raw_comment = get_packet_field(packet, :comment, "") clean_comment = Aprsme.EncodingUtils.sanitize_comment(raw_comment) %{ - callsign: callsign, + callsign: map_label(packet), comment: clean_comment, - timestamp_dt: timestamp_dt, - cache_buster: cache_buster, - weather: false, - # Use pre-fetched weather info - weather_link: has_weather + timestamp_dt: get_packet_received_at(packet), + cache_buster: System.system_time(:millisecond), + weather: false } - |> PopupComponent.popup() - |> Safe.to_iodata() - |> IO.iodata_to_binary() end end + defp render_popup(data) do + data + |> PopupComponent.popup() + |> Safe.to_iodata() + |> IO.iodata_to_binary() + end + @doc """ Select the best packet to display for a callsign. Moved from historical_loader.ex. diff --git a/lib/aprsme_web/live/shared/param_utils.ex b/lib/aprsme_web/live/shared/param_utils.ex index 45ac3ea..d832a75 100644 --- a/lib/aprsme_web/live/shared/param_utils.ex +++ b/lib/aprsme_web/live/shared/param_utils.ex @@ -9,20 +9,7 @@ defmodule AprsmeWeb.Live.Shared.ParamUtils do """ @spec parse_float_in_range(binary() | any(), float(), float(), float()) :: float() def parse_float_in_range(str, default, min, max) when is_binary(str) do - # Sanitize input first - sanitized = sanitize_numeric_string(str) - - case Float.parse(sanitized) do - {val, ""} when val >= min and val <= max -> - if finite?(val), do: val, else: default - - {val, _remainder} when val >= min and val <= max -> - # Accept even with trailing characters, but validate the number - if finite?(val), do: val, else: default - - _ -> - default - end + parse_in_range(str, default, min, max, &Float.parse/1, fn v -> finite?(v) && v end) end def parse_float_in_range(_, default, _, _), do: default @@ -32,24 +19,23 @@ defmodule AprsmeWeb.Live.Shared.ParamUtils do """ @spec parse_int_in_range(binary() | any(), integer(), integer(), integer()) :: integer() def parse_int_in_range(str, default, min, max) when is_binary(str) do - # Sanitize input first + parse_in_range(str, default, min, max, &Integer.parse/1) + end + + def parse_int_in_range(_, default, _, _), do: default + + defp parse_in_range(str, default, min, max, parser, validator \\ fn v -> v end) do sanitized = sanitize_numeric_string(str) - case Integer.parse(sanitized) do - {val, ""} when val >= min and val <= max -> - val - - {val, _remainder} when val >= min and val <= max -> - # Accept even with trailing characters - val + case parser.(sanitized) do + {val, _} when val >= min and val <= max -> + validator.(val) || default _ -> default end end - def parse_int_in_range(_, default, _, _), do: default - @doc """ Sanitize numeric strings to prevent injection attacks. """