refactor: apply functional programming patterns across codebase
- accounts.ex: replace validate_user_password (if-based) with with-chain; drop unnecessary try/rescue around Repo.get_by. Extract transaction_unwrap helper to eliminate 3 copies of the Multi result-unwrap case block. - packets.ex: deduplicate store_bad_packet by extracting raw_packet_string, extract_error_type, and extract_error_message into composable helpers. Normalize find_coordinate_value — collapse 12 clauses into a generic deep_get/direct_get that handles atom/string keys and nested maps uniformly. - data_builder.ex: split build_simple_popup into separate data-construction (build_simple_popup_data) and rendering (render_popup) phases, eliminating duplicated PopupComponent/Safe/IO pipelines. - param_utils.ex: unify parse_float_in_range and parse_int_in_range via a shared parse_in_range that accepts Float.parse/Integer.parse as a function parameter — functions-as-values pattern.
This commit is contained in:
parent
bc60fec98c
commit
01ecf3d6bc
4 changed files with 89 additions and 123 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
"""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue