graham/aprs.me
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor: remove dead code, duplication, and over-abstractions

Browse source 
- Delete dead config keys and test file (typo'd aprsme_is_*, vacuous disable test)
- Remove duplicate function clauses and identical function pairs
- Rename misleading one_hour_ago variable to one_day_ago
- Strip stale Oban comment
- Remove TestHelpers time function duplicates
- Inline Aprsme.Schema module (only 1 caller)
- Deduplicate prod esbuild config
- Thin SymbolRenderer delegations, inline TimeUtils wrappers
- Remove redundant DeploymentNotifier GenServer polling
- Replace random fallback in get_callsign_key with sentinel
This commit is contained in:
Graham McInitre 2026-07-21 10:36:49 -05:00
parent 243a98df77
commit 1006fdaefc
20 changed files with 45 additions and 268 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
config/prod.exs
View file

@ -15,15 +15,6 @@ config :aprsme, AprsmeWeb.Endpoint,
adapter: Bandit.PhoenixAdapter,
http: [ip: {0, 0, 0, 0}, port: 4000]
config :esbuild,
version: "0.25.4",
default: [
args:
~w(js/app.ts --bundle --target=es2020 --outdir=../priv/static/assets --loader:.css=css --loader:.png=file --loader:.svg=file --external:/fonts/* --external:/images/*),
cd: Path.expand("../assets", __DIR__),
env: %{"NODE_PATH" => Path.expand("../deps", __DIR__)}
]
# Do not print debug messages in production
# of environment variables, is done on config/runtime.exs.
config :logger, level: :info

4
lib/aprsme/application.ex
View file

@ -7,8 +7,6 @@ defmodule Aprsme.Application do
require Logger
# Configure Oban for background jobs
@impl true
def start(_type, _args) do
# Initialize deployment timestamp
@ -53,8 +51,6 @@ defmodule Aprsme.Application do
# Start cleanup scheduler for periodic packet cleanup
Aprsme.CleanupScheduler,
Aprsme.PostgresNotifier,
# Start deployment notifier
Aprsme.DeploymentNotifier,
# Start the packet processing pipeline
Aprsme.PacketPipelineSupervisor
]

50
lib/aprsme/deployment_notifier.ex
View file

@ -1,57 +1,11 @@
defmodule Aprsme.DeploymentNotifier do
@moduledoc """
Monitors for deployment changes and notifies connected clients.
In k8s, this detects when the DEPLOYED_AT environment variable changes.
Notifies connected clients about deployment changes.
Called from the release module when a deployment is detected.
"""
use GenServer
require Logger
@check_interval 30_000
def start_link(opts \\ []) do
GenServer.start_link(__MODULE__, opts, name: __MODULE__)
end
def init(_opts) do
# Schedule first check
Process.send_after(self(), :check_deployment, @check_interval)
# Get initial deployment timestamp
deployed_at = Aprsme.Release.deployed_at()
{:ok, %{deployed_at: deployed_at}}
end
def handle_info(:check_deployment, state) do
# Schedule next check
Process.send_after(self(), :check_deployment, @check_interval)
# Get current deployment timestamp
current_deployed_at = Aprsme.Release.deployed_at()
# Check if deployment timestamp changed (shouldn't happen in same process, but useful for monitoring)
if current_deployed_at == state.deployed_at do
{:noreply, state}
# Broadcast the new deployment
else
Logger.info("Deployment timestamp changed from #{state.deployed_at} to #{current_deployed_at}")
_ =
Phoenix.PubSub.broadcast(
Aprsme.PubSub,
"deployment_events",
{:new_deployment, %{deployed_at: current_deployed_at}}
)
{:noreply, %{state | deployed_at: current_deployed_at}}
end
end
@doc """
Notify about a new deployment immediately.
This can be called from the release module when deployment is detected.
"""
def notify_deployment(deployed_at) do
Phoenix.PubSub.broadcast(

5
lib/aprsme/packet.ex
View file

@ -1,6 +1,6 @@
defmodule Aprsme.Packet do
@moduledoc false
use Aprsme.Schema
use Ecto.Schema
import Ecto.Changeset
@ -8,6 +8,9 @@ defmodule Aprsme.Packet do
alias Aprsme.DataExtended
alias AprsmeWeb.Live.Shared.CoordinateUtils
@primary_key {:id, :binary_id, autogenerate: true}
@foreign_key_type :binary_id
schema "packets" do
field(:base_callsign, :string)
field(:data_type, :string)

11
lib/aprsme/schema.ex
View file

@ -1,11 +0,0 @@
defmodule Aprsme.Schema do
@moduledoc false
defmacro __using__(_) do
quote do
use Ecto.Schema
@primary_key {:id, :binary_id, autogenerate: true}
@foreign_key_type :binary_id
end
end
end

18
lib/aprsme_web/live/components/symbol_renderer.ex
View file

@ -25,7 +25,7 @@ defmodule AprsmeWeb.SymbolRenderer do
def symbol(assigns) do
# Get the sprite file and position for this symbol
sprite_info = get_sprite_info(assigns.symbol_table, assigns.symbol_code)
sprite_info = AprsmeWeb.AprsSymbol.get_sprite_info(assigns.symbol_table, assigns.symbol_code)
assigns =
assign(assigns,
@ -82,20 +82,4 @@ defmodule AprsmeWeb.SymbolRenderer do
</div>
"""
end
@doc """
Gets sprite information for a given symbol table and code.
Returns a map with sprite_file, background_position, and background_size.
"""
def get_sprite_info(symbol_table, symbol_code) do
AprsmeWeb.AprsSymbol.get_sprite_info(symbol_table, symbol_code)
end
@doc """
Renders an APRS symbol for use in Leaflet markers.
Returns HTML string that can be used as marker content.
"""
def render_marker_symbol(symbol_table, symbol_code, callsign \\ nil, size \\ 32) do
AprsmeWeb.AprsSymbol.render_marker_html(symbol_table, symbol_code, callsign, size)
end
end

19
lib/aprsme_web/live/map_live/data_builder.ex
View file

@ -113,7 +113,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do
# would generate anyway, saving the client from icon re-creation
symbol_html =
if is_most_recent do
AprsmeWeb.SymbolRenderer.render_marker_symbol(
AprsmeWeb.AprsSymbol.render_marker_html(
symbol_table_id,
symbol_code,
label,
@ -217,12 +217,12 @@ defmodule AprsmeWeb.MapLive.DataBuilder do
def build_simple_popup(packet, has_weather) do
packet
|> build_simple_popup_data()
|> Map.put(:weather_link, has_weather || weather_packet?(packet))
|> Map.put(:weather_link, has_weather || SharedPacketUtils.has_weather_data?(packet))
|> render_popup()
end
defp build_simple_popup_data(packet) do
is_weather = weather_packet?(packet)
is_weather = SharedPacketUtils.has_weather_data?(packet)
if is_weather do
%{
@ -278,7 +278,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do
{position_packets, weather_packets} =
Enum.split_with(packets, fn packet ->
# A packet is a position packet if it's NOT a weather packet
not weather_packet?(packet)
not SharedPacketUtils.has_weather_data?(packet)
end)
# Prefer the most recent position packet, fall back to most recent weather packet
@ -304,7 +304,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do
@spec build_weather_callsign_set(list()) :: MapSet.t()
def build_weather_callsign_set(packets) do
packets
|> Enum.filter(&weather_packet?/1)
|> Enum.filter(&SharedPacketUtils.has_weather_data?/1)
|> MapSet.new(fn packet -> String.upcase(display_name(packet)) end)
end
@ -332,7 +332,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do
timestamp: get_timestamp(packet),
comment: clean_comment,
safe_data_extended: convert_tuples_to_strings(data_extended),
is_weather_packet: weather_packet?(packet)
is_weather_packet: SharedPacketUtils.has_weather_data?(packet)
}
end
@ -457,7 +457,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do
# Generate symbol HTML using the server-side renderer
symbol_html =
AprsmeWeb.SymbolRenderer.render_marker_symbol(
AprsmeWeb.AprsSymbol.render_marker_html(
packet_info.symbol_table_id,
packet_info.symbol_code,
packet_info.callsign,
@ -523,11 +523,6 @@ defmodule AprsmeWeb.MapLive.DataBuilder do
SharedPacketUtils.map_label(packet)
end
@spec weather_packet?(map()) :: boolean()
defp weather_packet?(packet) do
SharedPacketUtils.weather_packet?(packet)
end
@spec has_weather_packets?(String.t()) :: boolean()
defp has_weather_packets?(callsign) when is_binary(callsign) do
case Aprsme.WeatherCache.weather_callsign?(callsign) do

12
lib/aprsme_web/live/map_live/index.ex
View file

@ -40,7 +40,7 @@ defmodule AprsmeWeb.MapLive.Index do
# Basic setup
deployed_at = Aprsme.Release.deployed_at()
one_hour_ago = TimeUtils.one_day_ago()
one_day_ago = TimeUtils.hours_ago(24)
# Parse and determine map location
{map_center, map_zoom, should_skip_initial_url_update} = Navigation.determine_map_location(params, session)
@ -96,7 +96,7 @@ defmodule AprsmeWeb.MapLive.Index do
should_skip_initial_url_update: should_skip_initial_url_update,
tracked_callsign: tracked_callsign,
deployed_at: deployed_at,
one_hour_ago: one_hour_ago
one_day_ago: one_day_ago
})}
end
@ -170,12 +170,12 @@ defmodule AprsmeWeb.MapLive.Index do
should_skip_initial_url_update: should_skip_initial_url_update,
tracked_callsign: tracked_callsign,
deployed_at: deployed_at,
one_hour_ago: one_hour_ago
one_day_ago: one_day_ago
}) do
# Don't override trail_duration and historical_hours if they're already set
trail_duration = Map.get(socket.assigns, :trail_duration, "1")
historical_hours = Map.get(socket.assigns, :historical_hours, "1")
packet_age_threshold = Map.get(socket.assigns, :packet_age_threshold, one_hour_ago)
packet_age_threshold = Map.get(socket.assigns, :packet_age_threshold, one_day_ago)
# If tracking a specific callsign, fetch their latest packet and other SSIDs
{tracked_callsign_latest_packet, other_ssids} =
@ -251,7 +251,7 @@ defmodule AprsmeWeb.MapLive.Index do
end
@spec assign_defaults(Socket.t(), DateTime.t()) :: Socket.t()
defp assign_defaults(socket, one_hour_ago) do
defp assign_defaults(socket, one_day_ago) do
assign(socket,
packets: [],
visible_packets: %{},
@ -266,7 +266,7 @@ defmodule AprsmeWeb.MapLive.Index do
},
map_center: UrlParams.default_center(),
map_zoom: UrlParams.default_zoom(),
packet_age_threshold: one_hour_ago,
packet_age_threshold: one_day_ago,
map_ready: false,
historical_loaded: false,
bounds_update_timer: nil,

13
lib/aprsme_web/live/shared/packet_utils.ex
View file

@ -8,16 +8,17 @@ defmodule AprsmeWeb.Live.Shared.PacketUtils do
@doc """
Get unique callsign key from packet.
Returns the display name, or "__unknown__" if none is available.
"""
@spec get_callsign_key(map()) :: binary()
def get_callsign_key(packet) when is_map(packet) do
case display_name(packet) do
"" -> [:positive] |> System.unique_integer() |> to_string()
"" -> "__unknown__"
callsign -> callsign
end
end
def get_callsign_key(_packet), do: [:positive] |> System.unique_integer() |> to_string()
def get_callsign_key(_packet), do: "__unknown__"
@doc """
Prune oldest packets from a map to enforce memory limits.
@ -111,14 +112,6 @@ defmodule AprsmeWeb.Live.Shared.PacketUtils do
end
end
@doc """
Check if packet is a weather packet based on its data.
"""
@spec weather_packet?(map()) :: boolean()
def weather_packet?(packet) do
has_weather_data?(packet)
end
@doc """
Get weather field value from packet with fallback.
"""

32
lib/aprsme_web/time_utils.ex
View file

@ -11,30 +11,6 @@ defmodule AprsmeWeb.TimeUtils do
DateTime.add(DateTime.utc_now(), -hours * 3600, :second)
end
@doc """
Returns a DateTime that is one hour before now.
"""
@spec one_hour_ago() :: DateTime.t()
def one_hour_ago, do: hours_ago(1)
@doc """
Returns a DateTime that is 24 hours (one day) before now.
"""
@spec one_day_ago() :: DateTime.t()
def one_day_ago, do: hours_ago(24)
@doc """
Returns a DateTime that is 48 hours (two days) before now.
"""
@spec two_days_ago() :: DateTime.t()
def two_days_ago, do: hours_ago(48)
@doc """
Returns a DateTime that is 7 days (one week) before now.
"""
@spec one_week_ago() :: DateTime.t()
def one_week_ago, do: hours_ago(24 * 7)
@doc """
Returns a DateTime that is the specified number of days before now.
"""
@ -49,10 +25,10 @@ defmodule AprsmeWeb.TimeUtils do
Returns a tuple of {start_time, end_time} for common time ranges.
"""
@spec time_range(time_range_atom()) :: {DateTime.t(), DateTime.t()}
def time_range(:last_hour), do: {one_hour_ago(), DateTime.utc_now()}
def time_range(:last_day), do: {one_day_ago(), DateTime.utc_now()}
def time_range(:last_two_days), do: {two_days_ago(), DateTime.utc_now()}
def time_range(:last_week), do: {one_week_ago(), DateTime.utc_now()}
def time_range(:last_hour), do: {hours_ago(1), DateTime.utc_now()}
def time_range(:last_day), do: {hours_ago(24), DateTime.utc_now()}
def time_range(:last_two_days), do: {hours_ago(48), DateTime.utc_now()}
def time_range(:last_week), do: {hours_ago(24 * 7), DateTime.utc_now()}
@doc """
Returns a default time range of 48 hours.

1
lib/aprsme_web/weather_units.ex
View file

@ -53,7 +53,6 @@ defmodule AprsmeWeb.WeatherUnits do
Formats pressure (keeps hPa for all locales as it's standard).
"""
@spec format_pressure(number() | any(), any()) :: {number() | any(), String.t()}
def format_pressure(pressure, _locale) when is_number(pressure), do: {pressure, "hPa"}
def format_pressure(pressure, _locale), do: {pressure, "hPa"}
@doc """

34
test/aprsme/deployment_notifier_test.exs
View file

@ -13,38 +13,4 @@ defmodule Aprsme.DeploymentNotifierTest do
assert_receive {:new_deployment, %{deployed_at: ^now}}, 1000
end
end
describe "handle_info :check_deployment" do
test "no-op when deployed_at is unchanged" do
deployed = DateTime.utc_now()
Application.put_env(:aprsme, :deployed_at, deployed)
state = %{deployed_at: deployed}
assert {:noreply, ^state} = DeploymentNotifier.handle_info(:check_deployment, state)
end
test "broadcasts and updates state when deployed_at has changed" do
:ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, "deployment_events")
new_deploy = DateTime.utc_now()
old_deploy = DateTime.add(new_deploy, -60, :second)
# Simulate an updated deploy timestamp in app env.
Application.put_env(:aprsme, :deployed_at, new_deploy)
state = %{deployed_at: old_deploy}
assert {:noreply, %{deployed_at: ^new_deploy}} =
DeploymentNotifier.handle_info(:check_deployment, state)
assert_receive {:new_deployment, %{deployed_at: ^new_deploy}}, 1000
end
end
describe "init/1" do
test "schedules a check and stores the current deployed_at in state" do
assert {:ok, %{deployed_at: %DateTime{}}} = DeploymentNotifier.init([])
# The 30s check is scheduled — we don't wait for it.
end
end
end

21
test/aprsme_web/live/components/symbol_renderer_test.exs
View file

@ -5,27 +5,6 @@ defmodule AprsmeWeb.SymbolRendererTest do
alias AprsmeWeb.SymbolRenderer
describe "get_sprite_info/2" do
test "delegates to AprsmeWeb.AprsSymbol" do
info = SymbolRenderer.get_sprite_info("/", "_")
assert Map.has_key?(info, :sprite_file)
assert Map.has_key?(info, :background_position)
assert Map.has_key?(info, :background_size)
end
end
describe "render_marker_symbol/4" do
test "returns HTML string for a symbol" do
html = SymbolRenderer.render_marker_symbol("/", ">", "K5ABC", 32)
assert html =~ "K5ABC"
end
test "works without a callsign" do
html = SymbolRenderer.render_marker_symbol("/", ">")
assert String.length(html) > 0
end
end
describe "symbol/1 component" do
test "renders a container with the given size" do
html = render_component(&SymbolRenderer.symbol/1, symbol_table: "/", symbol_code: ">", size: 48)

6
test/aprsme_web/live/map_live/packet_utils_test.exs
View file

@ -283,7 +283,7 @@ defmodule AprsmeWeb.MapLive.PacketUtilsTest do
temperature: 75
}
assert SharedPacketUtils.weather_packet?(weather_packet)
assert SharedPacketUtils.has_weather_data?(weather_packet)
end
test "identifies weather packets with humidity" do
@ -294,7 +294,7 @@ defmodule AprsmeWeb.MapLive.PacketUtilsTest do
humidity: 80
}
assert SharedPacketUtils.weather_packet?(weather_packet)
assert SharedPacketUtils.has_weather_data?(weather_packet)
end
test "does not identify non-weather packets" do
@ -304,7 +304,7 @@ defmodule AprsmeWeb.MapLive.PacketUtilsTest do
symbol_code: ">"
}
refute SharedPacketUtils.weather_packet?(regular_packet)
refute SharedPacketUtils.has_weather_data?(regular_packet)
end
test "has_weather_packets? returns false for non-existent callsign" do

15
test/aprsme_web/live/shared/packet_utils_test.exs
View file

@ -23,13 +23,13 @@ defmodule AprsmeWeb.Live.Shared.PacketUtilsTest do
assert PacketUtils.get_callsign_key(packet) == "OBJ"
end
test "returns a unique fallback key for non-maps" do
assert PacketUtils.get_callsign_key(nil) =~ ~r/^\d+$/
test "returns __unknown__ for non-maps" do
assert PacketUtils.get_callsign_key(nil) == "__unknown__"
end
test "returns a unique fallback key when every field is empty" do
test "returns __unknown__ when every field is empty" do
packet = %{sender: "", object_name: "", item_name: ""}
assert PacketUtils.get_callsign_key(packet) =~ ~r/^\d+$/
assert PacketUtils.get_callsign_key(packet) == "__unknown__"
end
test "strips whitespace from object/item names" do
@ -184,13 +184,6 @@ defmodule AprsmeWeb.Live.Shared.PacketUtilsTest do
end
end
describe "weather_packet?/1" do
test "mirrors has_weather_data?/1" do
assert PacketUtils.weather_packet?(%{temperature: 72})
refute PacketUtils.weather_packet?(%{sender: "K5ABC"})
end
end
describe "packet_within_time_threshold?/2" do
test "returns true when packet is after threshold" do
now = DateTime.utc_now()

9
test/aprsme_web/test_helpers_test.exs
View file

@ -44,17 +44,12 @@ defmodule AprsmeWeb.TestHelpersTest do
describe "time helpers" do
test "hours_ago returns a DateTime earlier than now" do
then_dt = TestHelpers.hours_ago(2)
assert DateTime.before?(then_dt, DateTime.utc_now())
end
test "minutes_ago returns a DateTime earlier than now" do
then_dt = TestHelpers.minutes_ago(10)
then_dt = AprsmeWeb.TimeUtils.hours_ago(2)
assert DateTime.before?(then_dt, DateTime.utc_now())
end
test "days_ago returns a DateTime earlier than now" do
then_dt = TestHelpers.days_ago(3)
then_dt = AprsmeWeb.TimeUtils.days_ago(3)
assert DateTime.before?(then_dt, DateTime.utc_now())
end
end

18
test/aprsme_web/time_utils_test.exs
View file

@ -24,24 +24,24 @@ defmodule AprsmeWeb.TimeUtilsTest do
end
end
describe "named helpers" do
test "one_hour_ago is about 3600 seconds before now" do
dt = TimeUtils.one_hour_ago()
describe "hours_ago/1 with common values" do
test "hours_ago(1) is about 3600 seconds before now" do
dt = TimeUtils.hours_ago(1)
assert_in_delta DateTime.diff(DateTime.utc_now(), dt, :second), 3600, @slack_seconds
end
test "one_day_ago is about 86400 seconds before now" do
dt = TimeUtils.one_day_ago()
test "hours_ago(24) is about 86400 seconds before now" do
dt = TimeUtils.hours_ago(24)
assert_in_delta DateTime.diff(DateTime.utc_now(), dt, :second), 86_400, @slack_seconds
end
test "two_days_ago is about 172800 seconds before now" do
dt = TimeUtils.two_days_ago()
test "hours_ago(48) is about 172800 seconds before now" do
dt = TimeUtils.hours_ago(48)
assert_in_delta DateTime.diff(DateTime.utc_now(), dt, :second), 172_800, @slack_seconds
end
test "one_week_ago is about 604800 seconds before now" do
dt = TimeUtils.one_week_ago()
test "hours_ago(168) is about 604800 seconds before now" do
dt = TimeUtils.hours_ago(168)
assert_in_delta DateTime.diff(DateTime.utc_now(), dt, :second), 604_800, @slack_seconds
end
end

15
test/support/test_helpers.ex
View file

@ -63,19 +63,4 @@ defmodule AprsmeWeb.TestHelpers do
"west" => "-96.0"
}
end
@doc """
Common time calculations used across tests.
"""
def hours_ago(hours) when is_number(hours) do
DateTime.add(DateTime.utc_now(), -hours * 3600, :second)
end
def minutes_ago(minutes) when is_number(minutes) do
DateTime.add(DateTime.utc_now(), -minutes * 60, :second)
end
def days_ago(days) when is_number(days) do
DateTime.add(DateTime.utc_now(), -days * 86_400, :second)
end
end

5
test/test_helper.exs
View file

@ -25,12 +25,7 @@ Mox.stub(Aprsme.PacketsMock, :get_recent_packets, fn _opts -> [] end)
Mox.stub(Aprsme.PacketsMock, :get_nearby_stations, fn _lat, _lon, _exclude, _opts -> [] end)
# Ensure no external APRS connections during tests
Application.put_env(:aprsme, :disable_aprs_connection, true)
Application.put_env(:aprsme, :aprs_is_server, "mock.aprs.test")
Application.put_env(:aprsme, :aprsme_is_port, 14_580)
Application.put_env(:aprsme, :aprsme_is_login_id, "TEST")
Application.put_env(:aprsme, :aprsme_is_password, "-1")
Application.put_env(:aprsme, :aprsme_is_default_filter, "r/0/0/1")
Application.put_env(:aprsme, :packets_module, Aprsme.PacketsMock)
# AprsIsMock is automatically loaded from test/support via elixirc_paths

16
test/test_vacuous_disable_test.exs
View file

@ -1,16 +0,0 @@
defmodule TestVacuousDisable do
@moduledoc false
use ExUnit.Case, async: true
# credo:disable-for-next-line Jump.CredoChecks.VacuousTest
test "this should be disabled" do
result = %{a: 1}
assert result.a == 1
end
# credo:disable-for-next-line Jump.CredoChecks.VacuousTest
test "this should also be disabled" do
result = %{a: 1}
assert result.a == 1
end
end