diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..974c0a7 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,71 @@ +name: Elixir quality + +on: + push: + pull_request: + +jobs: + quality: + runs-on: ubuntu-latest + services: + postgres: + image: postgis/postgis:17-3.5 + env: + POSTGRES_PASSWORD: postgres + POSTGRES_USER: postgres + POSTGRES_DB: aprsme_test + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U postgres" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + env: + MIX_ENV: test + steps: + - uses: actions/checkout@v4 + with: + submodules: recursive + - uses: erlef/setup-beam@v1 + with: + otp-version: "29.0" + elixir-version: "1.20" + - uses: actions/cache@v4 + with: + path: | + deps + _build + key: ${{ runner.os }}-mix-${{ hashFiles('mix.lock') }} + - run: mix local.hex --force + - run: mix local.rebar --force + - run: mix deps.get + - run: mix format --check-formatted + - run: mix compile --warnings-as-errors + - run: mix credo --strict + - run: mix test + - run: mix sobelow --config + - run: mix hex.audit + + dialyzer: + runs-on: ubuntu-latest + env: + MIX_ENV: dev + steps: + - uses: actions/checkout@v4 + with: + submodules: recursive + - uses: erlef/setup-beam@v1 + with: + otp-version: "29.0" + elixir-version: "1.20" + - uses: actions/cache@v4 + with: + path: | + deps + _build + key: ${{ runner.os }}-dialyzer-${{ hashFiles('mix.lock') }} + - run: mix local.hex --force + - run: mix local.rebar --force + - run: mix deps.get + - run: mix dialyzer diff --git a/assets/js/app.ts b/assets/js/app.ts index dfbf8d9..c39f8a7 100644 --- a/assets/js/app.ts +++ b/assets/js/app.ts @@ -14,10 +14,6 @@ declare global { mapBundleLoaded?: boolean; chartBundleLoaded?: boolean; Chart?: unknown; - VendorLoader?: { - mapBundleUrl: string; - loadCharts: () => void; - }; liveSocket: any; Hooks: Record; } @@ -71,28 +67,52 @@ function loadMapBundle(callback: () => void) { return; } - if (window.VendorLoader) { - mapBundleLoading = true; - const script = document.createElement("script"); - script.src = window.VendorLoader.mapBundleUrl; - script.onload = () => { - window.mapBundleLoaded = true; - mapBundleLoading = false; - const cbs = mapBundleCallbacks; - mapBundleCallbacks = []; - cbs.forEach((cb) => cb()); - }; - script.onerror = () => { - console.error("Failed to load map bundle"); - mapBundleLoading = false; - const cbs = mapBundleCallbacks; - mapBundleCallbacks = []; - cbs.forEach((cb) => cb()); - }; - document.head.appendChild(script); - } else { + mapBundleLoading = true; + const script = document.createElement("script"); + script.src = "/assets/vendor/js/map-bundle.js"; + script.onload = () => { + window.mapBundleLoaded = true; + mapBundleLoading = false; + const cbs = mapBundleCallbacks; + mapBundleCallbacks = []; + cbs.forEach((cb) => cb()); + }; + script.onerror = () => { + console.error("Failed to load map bundle"); + mapBundleLoading = false; + mapBundleCallbacks = []; + }; + document.head.appendChild(script); +} + +let chartBundleLoading = false; +let chartBundleCallbacks: Array<() => void> = []; + +function loadChartBundle(callback: () => void) { + if (window.chartBundleLoaded || window.Chart) { callback(); + return; } + + chartBundleCallbacks.push(callback); + if (chartBundleLoading) return; + + chartBundleLoading = true; + const script = document.createElement("script"); + script.src = "/assets/vendor/js/chart-bundle.js"; + script.onload = () => { + window.chartBundleLoaded = true; + chartBundleLoading = false; + const callbacks = chartBundleCallbacks; + chartBundleCallbacks = []; + callbacks.forEach((cb) => cb()); + }; + script.onerror = () => { + console.error("Failed to load chart bundle"); + chartBundleLoading = false; + chartBundleCallbacks = []; + }; + document.head.appendChild(script); } function isHookActive(context: DeferredHookContext): boolean { @@ -153,33 +173,11 @@ Object.keys(WeatherChartHooks).forEach((hookName) => { const self = this as DeferredHookContext; self.__appDestroyed = false; self.__chartBundleCheckCount = 0; - if (window.VendorLoader && !window.chartBundleLoaded) { - window.VendorLoader.loadCharts(); - const checkChartLoaded = () => { - if (!isHookActive(self)) { - return; - } - - if ((self.__chartBundleCheckCount || 0) >= 100) { - console.error(`Timed out waiting for chart bundle for ${hookName}`); - return; - } - - if (window.Chart) { - if (originalHook.mounted) { - originalHook.mounted.call(self); - } - } else { - self.__chartBundleCheckCount = (self.__chartBundleCheckCount || 0) + 1; - setTimeout(checkChartLoaded, 50); - } - }; - setTimeout(checkChartLoaded, 100); - } else { - if (originalHook.mounted) { - originalHook.mounted.call(this); + loadChartBundle(() => { + if (isHookActive(self) && originalHook.mounted) { + originalHook.mounted.call(self); } - } + }); }, destroyed() { const self = this as DeferredHookContext; diff --git a/assets/js/map.ts b/assets/js/map.ts index cfd4e44..2ff9824 100644 --- a/assets/js/map.ts +++ b/assets/js/map.ts @@ -2430,8 +2430,8 @@ let MapAPRSMap = { } } // Use server-generated symbol HTML if available - if (data.symbol_html) { - return createDivIcon(data.symbol_html, { + if (data.symbol_table_id || data.symbol_code) { + return createDivIcon(createSafeAprsSymbolHtml(data, true), { iconSize: [120, 32], // Increased width to accommodate callsign label iconAnchor: [16, 16], }); @@ -2460,8 +2460,8 @@ let MapAPRSMap = { } // Fallback: Use server-generated symbol HTML if available - if (data.symbol_html) { - return createDivIcon(data.symbol_html); + if (data.symbol_table_id || data.symbol_code) { + return createDivIcon(createSafeAprsSymbolHtml(data, false)); } // Final fallback: Simple dot @@ -2685,6 +2685,28 @@ function createDivIcon( }); } +// Build marker markup exclusively from validated APRS fields. Packet-derived +// strings are escaped before entering Leaflet's HTML-based divIcon API. +function createSafeAprsSymbolHtml( + data: { callsign?: string; symbol_table_id?: string; symbol_code?: string }, + includeLabel: boolean, +): string { + const rawTable = data.symbol_table_id || "/"; + const isOverlay = /^[A-Z0-9]$/.test(rawTable); + const table = rawTable === "\\" ? "\\" : rawTable === "]" || isOverlay ? "]" : "/"; + const code = getValidSymbolCode(data.symbol_code, table); + const tableId = table === "/" ? "0" : table === "\\" ? "1" : "2"; + const index = Math.max(0, Math.min(93, code.charCodeAt(0) - 33)); + const position = `${-(index % 16) * 32}px ${-Math.floor(index / 16) * 32}px`; + const sprite = `/aprs-symbols/aprs-symbols-128-${tableId}@2x.png`; + const title = escapeHtml(`${rawTable}${code}`); + const label = includeLabel + ? `${escapeHtml(data.callsign || "")}` + : ""; + + return `
${label}`; +} + // Helper to validate and fallback symbol code per aprs.fi logic function getValidSymbolCode( symbolCode: string | undefined, diff --git a/assets/js/vendor.js b/assets/js/vendor.js deleted file mode 100644 index 6410268..0000000 --- a/assets/js/vendor.js +++ /dev/null @@ -1,2 +0,0 @@ -// This file is now empty since all vendor libraries are loaded from CDN -// See root.html.heex for CDN script tags \ No newline at end of file diff --git a/config/config.exs b/config/config.exs index d8d5a83..fc14f41 100644 --- a/config/config.exs +++ b/config/config.exs @@ -95,12 +95,6 @@ config :esbuild, cd: Path.expand("../assets", __DIR__), env: %{"NODE_PATH" => Path.expand("../deps", __DIR__)} ], - vendor: [ - args: - ~w(js/vendor.js --bundle --target=es2017 --outdir=../priv/static/assets --minify --loader:.css=css --loader:.png=file --loader:.svg=file), - cd: Path.expand("../assets", __DIR__), - env: %{"NODE_PATH" => Path.expand("../deps", __DIR__)} - ], # Optimized CSS bundle vendor_css: [ args: ~w(vendor/css/minimal-bundle.css --outdir=../priv/static/assets/vendor/css --minify), @@ -122,12 +116,6 @@ config :esbuild, args: ~w(vendor/js/chart-bundle-src.js --outfile=../priv/static/assets/vendor/js/chart-bundle.js --minify --target=es2017), cd: Path.expand("../assets", __DIR__) - ], - # Date adapter - separate file - date_adapter: [ - args: - ~w(vendor/js/date-adapter.js --outfile=../priv/static/assets/vendor/js/date-adapter.js --minify --target=es2017), - cd: Path.expand("../assets", __DIR__) ] # Configures Elixir's Logger diff --git a/config/prod.exs b/config/prod.exs index d5714ad..52e8e6f 100644 --- a/config/prod.exs +++ b/config/prod.exs @@ -13,7 +13,15 @@ config :aprsme, AprsmeWeb.Endpoint, # Runtime production configuration, including reading cache_static_manifest: "priv/static/cache_manifest.json", adapter: Bandit.PhoenixAdapter, - http: [ip: {0, 0, 0, 0}, port: 4000] + http: [ip: {0, 0, 0, 0}, port: 4000], + session_options: [ + signing_salt: "aprs-me-production-session-signing-v1", + encryption_salt: "aprs-me-production-session-encryption-v1", + secure: true, + http_only: true, + same_site: "Lax" + ], + force_ssl: [rewrite_on: [:x_forwarded_proto], hsts: true] # Do not print debug messages in production # of environment variables, is done on config/runtime.exs. diff --git a/config/runtime.exs b/config/runtime.exs index 49b95ff..0dfe0ad 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -50,13 +50,6 @@ if config_env() == :prod do You can generate one by calling: mix phx.gen.secret 32 """ - encryption_salt = - System.get_env("ENCRYPTION_SALT") || - raise """ - environment variable ENCRYPTION_SALT is missing. - You can generate one by calling: mix phx.gen.secret 32 - """ - host = System.get_env("PHX_HOST") || "example.com" port = String.to_integer(System.get_env("PORT") || "4000") @@ -114,7 +107,6 @@ if config_env() == :prod do ], secret_key_base: secret_key_base, live_view: [signing_salt: live_view_signing_salt], - session_options: [encryption_salt: encryption_salt, secure: true], server: true, check_origin: [ "https://#{host}", diff --git a/docs/architecture/ingestion-pipeline.md b/docs/architecture/ingestion-pipeline.md index 0aa5074..4abdd5a 100644 --- a/docs/architecture/ingestion-pipeline.md +++ b/docs/architecture/ingestion-pipeline.md @@ -37,12 +37,10 @@ The packet ingestion pipeline moves raw APRS data from the APRS-IS network into ### Broadcasting After Insert -After successful insert, each packet is broadcast to three PubSub layers: +After successful insert, each packet is routed through: -1. **StreamingPacketsPubSub** — ETS-based, clients subscribe with bounds -2. **SpatialPubSub** — Grid-indexed (1° cells), viewport-filtered delivery -3. **Phoenix.PubSub** — Legacy per-topic broadcasts: - - `"postgres:aprsme_packets"` — global packet stream +1. **SpatialPubSub** — Grid-indexed viewport delivery with explicit unbounded subscriptions +2. **Phoenix.PubSub** — Targeted broadcasts: - `"packets:#{callsign}"` — per-callsign updates - `"weather:#{callsign}"` — per-weather-station updates @@ -61,6 +59,5 @@ Packets that fail parsing or validation are stored in `badpackets` table via `Pa ## Performance Notes - `PreparedQueries` module caches frequently-used query plans -- `InsertOptimizer` batch-tunes insert operations - `PacketConsumer` uses `insert_all` (not individual inserts) for throughput - Daily table partitioning via `PartitionManager` keeps index sizes manageable diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index 36f9cde..66ad0e8 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -34,9 +34,8 @@ PacketConsumer pool (GenStage consumers) │ broadcast to PubSub ├──► Postgres packets table (daily-partitioned) └──► Broadcasting - ├── SpatialPubSub (viewport-filtered, grid-indexed) - ├── StreamingPacketsPubSub (ETS-based, bounds-filtered) - └── Phoenix.PubSub (per-callsign topics) + ├── SpatialPubSub (bounded and unbounded spatial streams) + └── Phoenix.PubSub (targeted callsign/weather topics) │ ▼ LiveView clients (MapLive, PacketsLive, InfoLive) @@ -59,7 +58,6 @@ Aprsme.Application ├── Aprsme.Is.IsSupervisor │ └── Aprsme.Is ├── Aprsme.SpatialPubSub -├── Aprsme.StreamingPacketsPubSub ├── Aprsme.CleanupScheduler ├── Aprsme.PartitionManager ├── Aprsme.PostgresNotifier diff --git a/docs/contexts/packets.md b/docs/contexts/packets.md index 37500b4..ff60f87 100644 --- a/docs/contexts/packets.md +++ b/docs/contexts/packets.md @@ -55,9 +55,7 @@ Failed packets stored in `badpackets` table: - `get_other_ssids/1` — Find SSID variants of a base callsign - `get_total_packet_count/0` — Via PostgreSQL function -### Replay -- `get_packets_for_replay/1` — Historical packets filtered by callsign/bounds/region/time -- `stream_packets_for_replay/1` — Lazy stream with timing preservation +### Historical data - `get_historical_packet_count/1` — Area packet density ### Weather @@ -65,7 +63,7 @@ Failed packets stored in `badpackets` table: - `has_weather_packets?/1`, `weather_callsigns/1` — Weather presence queries ### Maintenance -- `clean_old_packets/0`, `clean_packets_older_than/1` — Retention cleanup +- `Aprsme.PartitionManager` owns rolling retention by dropping expired daily partitions. - `get_oldest_packet_timestamp/0` — Oldest data timestamp ## Submodules diff --git a/docs/refactor-implementation-handoff.md b/docs/refactor-implementation-handoff.md new file mode 100644 index 0000000..9748ffc --- /dev/null +++ b/docs/refactor-implementation-handoff.md @@ -0,0 +1,173 @@ +# Refactor and Hardening Implementation Handoff + +Date: 2026-07-26 + +## Purpose + +This file is the continuation point for the repository-wide architecture, security, +performance, SQL, and maintainability audit. A substantial first tranche has been +implemented, but enough work remains that it should be handled as a separate, +reviewed change set. + +Do not use npm. This is a Phoenix application whose JavaScript is built with +esbuild through Mix. + +## Current working tree + +The working tree contains a large, uncommitted implementation. Preserve and review +those changes before continuing. In particular, `vendor/aprs` is dirty and contains +source/test changes. Those changes appeared during this implementation and must be +reviewed as part of the work; do not blindly reset the submodule. + +Useful first commands: + +```sh +git status --short +git diff --check +git diff --stat +git -C vendor/aprs status --short +git diff --submodule=short -- vendor/aprs +``` + +## Implemented + +- Replaced the split packet-streaming paths with `Aprsme.SpatialPubSub` for both + LiveView and mobile clients. +- Removed per-packet broadcast tasks and made subscriptions PID-aware and + idempotent. +- Removed obsolete packet receiver, streaming pubsub, replay, DB optimizer, insert + optimizer, and sequence-counter infrastructure, together with their dead tests. +- Added a stale-packet guard to the LiveView packet batcher. +- Fixed the status page to use the correct task supervisor. +- Added callsign/transport-identifier validation at ingestion and made client-side + map marker construction safe from HTML injection. +- Added user roles, an admin role task, and admin authorization for operational + dashboards, error tracking, and bad-packet pages. +- Hardened session and remember-me cookies and enabled production HTTPS/HSTS + enforcement. +- Upgraded Bandit and `plug_crypto`. +- Made packet-retention partition deletion use an exact timestamp cutoff. +- Added a partition-aware geometry GiST index migration. +- Replaced the broken sequence-based packet count with an atomic row counter and + reconciled the existing value. +- Made partition deletion adjust the packet counter transactionally. +- Corrected database telemetry for partitioned tables and removed fabricated pool + metrics. +- Changed cumulative PromEx values from counters to last-value metrics. +- Made failed release migrations fail application startup. +- Removed duplicate release initialization and supervised deployment notification. +- Made `PartitionManager` the sole owner of packet retention. +- Removed unused registries and duplicate/empty asset build targets and loaders. +- Added Sobelow and a GitHub Actions workflow for format, compile, Credo, tests, + Sobelow, Hex audit, and Dialyzer. +- Updated affected architecture documentation. + +## Validation already completed + +- Full suite: `2447 passed (35 doctests, 31 properties, 2381 tests)`. +- `MIX_ENV=test mix compile --warnings-as-errors` passed. +- `MIX_ENV=dev mix esbuild default` passed. +- `git diff --check` passed before the final documentation/runtime edits. + +These checks must be rerun after all remaining work: + +```sh +MIX_ENV=dev mix format +git diff --check +MIX_ENV=dev mix compile --warnings-as-errors +MIX_ENV=test mix compile --warnings-as-errors +MIX_ENV=dev mix credo --strict +MIX_ENV=dev mix sobelow --config +MIX_ENV=dev mix hex.audit +MIX_ENV=dev mix dialyzer +MIX_ENV=test mix test +MIX_ENV=dev mix esbuild default +MIX_ENV=prod mix assets.deploy +``` + +## Remaining high-priority security work + +1. Audit `RemoteIp`/forwarded-header handling. Only trust proxy headers from known + ingress proxy CIDRs; direct clients must not be able to spoof their address. + Add tests for trusted and untrusted peers. +2. Add server-side rate limiting to expensive or abusable LiveView events, mobile + channel commands, authentication paths, and search endpoints. Do not rely only + on controller plugs. +3. Finish Content Security Policy hardening. The root layout still depends on + inline script behavior, so `unsafe-inline` has not been eliminated. Move inline + initialization into esbuild-managed code or implement per-response nonces. +4. Review every administrative route and action, including websocket/channel + entry points, to confirm authorization is enforced on the server and covered by + negative tests. +5. Run Sobelow and triage the existing skip/fingerprint configuration. Remove stale + skips and document any accepted finding rather than suppressing broadly. +6. Review secrets and credentials in Kubernetes manifests. Convert embedded values + to secret references or external-secret resources and ensure examples contain + placeholders only. +7. Add least-privilege Kubernetes `NetworkPolicy` rules for the web application, + database, ingress, and any monitoring components. + +## Remaining reliability and performance work + +1. Bound all ingress and client-facing buffers: + - APRS connection receive/reconnect buffers. + - Mobile channel pending packet lists. + - LiveView queues and task result accumulation. + Define overflow behavior and expose drop/backpressure telemetry. +2. Put explicit limits on mobile `connect_info`, viewport/bounds payloads, callsign + lists, search terms, and requested result counts. Reject malformed or oversized + payloads before database work. +3. Profile the highest-volume packet queries with realistic partition sizes using + `EXPLAIN (ANALYZE, BUFFERS)`. Verify partition pruning and the new spatial index. +4. Remove remaining N+1 query patterns in map overlays, device/account pages, and + status/operational pages. Prefer bounded batch queries and preloads. +5. Audit mobile and map search for unbounded scans. Add deterministic ordering, + hard result caps, suitable indexes, and tests that assert the caps. +6. Review the packet counter migration under concurrent inserts and partition + drops in a staging database. Exercise rollback/failed-migration behavior. +7. Load-test spatial subscriptions with overlapping bounds and reconnect churn. + Confirm exactly-once delivery, bounded memory, and cleanup after process exits. + +## Remaining maintainability work + +1. Split `AprsmeWeb.MapLive.Index` into focused state, event, subscription, and + rendering modules. Preserve LiveView behavior with integration tests first. +2. Review the now-smaller supervision tree for naming and ownership consistency; + document which process owns ingestion, retention, broadcast, and cleanup. +3. Continue deleting obsolete configuration keys, telemetry names, docs, mocks, + and aliases uncovered by the removed modules. +4. Review callsign naming. Ingestion currently accepts safe APRS transport + identifiers (up to 20 characters, uppercase alphanumeric segments separated by + hyphens), which is intentionally broader than a strict AX.25 callsign. Rename + APIs or document this distinction to avoid future accidental tightening. +5. Add focused migration tests for: + - Existing partition indexes being attached correctly. + - Counter reconciliation on an existing populated database. + - Exact cutoff behavior around partition boundaries. +6. Review the deleted test count. The suite decreased because tests for removed + infrastructure were deleted; ensure important end-to-end ingestion coverage was + not lost when `packet_pipeline_integration_test.exs` was removed. + +## Review notes + +- The geometry-index migration creates and attaches indexes per partition in + production. Test behavior uses a simpler direct index path. Validate this on a + production-like PostgreSQL version and data volume. +- The packet-count trigger approach prioritizes exactness. Measure its write + contention at expected ingestion rates; if it is too expensive, replace it with + an explicitly approximate metric rather than a misleading exact API. +- The CSP work should retain the early theme selection behavior to avoid a flash + of the wrong theme. +- Do not reintroduce `StreamingPacketsPubSub` or per-packet spawned broadcast + tasks; extend `SpatialPubSub` if delivery behavior needs adjustment. + +## Suggested sequence + +1. Review and isolate the dirty `vendor/aprs` changes. +2. Run all static gates and fix existing findings. +3. Complete proxy trust, rate limiting, CSP, and authorization review. +4. Bound buffers and payloads, then load-test delivery. +5. Profile and fix SQL query/index issues. +6. Refactor the large LiveView only after behavioral and performance coverage is + stable. +7. Finish Kubernetes hardening and rerun the full release build. diff --git a/docs/web/channels.md b/docs/web/channels.md index fad14eb..52cdebe 100644 --- a/docs/web/channels.md +++ b/docs/web/channels.md @@ -21,8 +21,7 @@ See [mobile-api.md](../mobile-api.md) for the complete message format and protoc ## Architecture The mobile channel receives real-time packet broadcasts from the same PubSub sources as the web LiveView clients: -- `StreamingPacketsPubSub` — bounds-filtered -- `SpatialPubSub` — viewport-filtered +- `SpatialPubSub` — bounded or explicitly unbounded delivery - Per-callsign Phoenix PubSub topics Packets are formatted into a mobile-optimized JSON structure and pushed to connected clients. diff --git a/docs/web/liveview-map.md b/docs/web/liveview-map.md index a7ef709..2c7c0f9 100644 --- a/docs/web/liveview-map.md +++ b/docs/web/liveview-map.md @@ -29,8 +29,7 @@ MapLive.Index ## Real-Time Updates MapLive subscribes to: -- `StreamingPacketsPubSub` — viewport-filtered new packets -- `SpatialPubSub` — grid-indexed spatial delivery +- `SpatialPubSub` — grid-indexed viewport delivery - Phoenix PubSub topics for callsign-specific updates Updates are batched via `PacketBatcher` to reduce DOM operations, then pushed to the Leaflet client via `push_event` (e.g., `"update_markers"`, `"update_clusters"`). diff --git a/lib/aprsme/accounts.ex b/lib/aprsme/accounts.ex index 20947f4..e6b3d64 100644 --- a/lib/aprsme/accounts.ex +++ b/lib/aprsme/accounts.ex @@ -65,6 +65,13 @@ defmodule Aprsme.Accounts do """ def get_user!(id), do: Repo.get!(User, id) + @doc "Changes a user's administrative role." + def set_user_role(%User{} = user, role) when role in [:user, :admin] do + user + |> User.role_changeset(%{role: role}) + |> Repo.update() + end + ## User registration @doc """ diff --git a/lib/aprsme/accounts/user.ex b/lib/aprsme/accounts/user.ex index 747716f..c03240b 100644 --- a/lib/aprsme/accounts/user.ex +++ b/lib/aprsme/accounts/user.ex @@ -12,10 +12,17 @@ defmodule Aprsme.Accounts.User do field(:password, :string, virtual: true, redact: true) field(:hashed_password, :string, redact: true) field(:confirmed_at, :naive_datetime) + field(:role, Ecto.Enum, values: [:user, :admin], default: :user) timestamps() end + def role_changeset(user, attrs) do + user + |> cast(attrs, [:role]) + |> validate_required([:role]) + end + @doc """ A user changeset for registration. diff --git a/lib/aprsme/application.ex b/lib/aprsme/application.ex index 2ee01e3..f6cd877 100644 --- a/lib/aprsme/application.ex +++ b/lib/aprsme/application.ex @@ -24,6 +24,7 @@ defmodule Aprsme.Application do Aprsme.Repo, # Start the PubSub system pubsub_config(), + Aprsme.DeploymentNotifier, # Start Redis-based rate limiter and caches (only if Redis is available) # Start circuit breaker Aprsme.CircuitBreaker, @@ -35,17 +36,10 @@ defmodule Aprsme.Application do Aprsme.BroadcastTaskSupervisor, # Start spatial PubSub for viewport-based filtering Aprsme.SpatialPubSub, - # Start global streaming packets PubSub - Aprsme.StreamingPacketsPubSub, # Manage daily partitions for the packets table Aprsme.PartitionManager, # Start the Endpoint (http/https) AprsmeWeb.Endpoint, - # Start a worker by calling: Aprsme.Worker.start_link(arg) - # {Aprsme.Worker, arg} - {Registry, keys: :duplicate, name: Registry.PubSub, partitions: System.schedulers_online()}, - # Start ReplayRegistry for unique replay sessions per user - {Registry, keys: :unique, name: Aprsme.ReplayRegistry, partitions: System.schedulers_online()}, # Start cleanup scheduler for periodic packet cleanup Aprsme.CleanupScheduler, Aprsme.PostgresNotifier, @@ -63,7 +57,7 @@ defmodule Aprsme.Application do children end - children = children ++ redis_children() + children = children ++ ets_children() # Add shutdown handlers at the end, after everything else is started children = @@ -124,11 +118,6 @@ defmodule Aprsme.Application do end # Gettext translations are automatically compiled during Mix compilation - rescue - error -> - Logger.error("Failed to run migrations: #{inspect(error)}. Application may be in an inconsistent state.") - # Don't crash the application, just log the error - :ok end defp maybe_add_cluster_components(children) do @@ -145,7 +134,6 @@ defmodule Aprsme.Application do Aprsme.DynamicSupervisor, Aprsme.Cluster.LeaderElection, Aprsme.Cluster.ConnectionManager, - Aprsme.Cluster.PacketReceiver, Aprsme.Cluster.PacketDistributor, Aprsme.ConnectionMonitor ] @@ -179,7 +167,7 @@ defmodule Aprsme.Application do {Phoenix.PubSub, name: Aprsme.PubSub} end - defp redis_children do + defp ets_children do Logger.info("Starting ETS-based caching and rate limiting") # Create public ETS tables so callers can use concurrent reads and writes directly. diff --git a/lib/aprsme/callsign.ex b/lib/aprsme/callsign.ex index f34890e..4ec60f3 100644 --- a/lib/aprsme/callsign.ex +++ b/lib/aprsme/callsign.ex @@ -1,146 +1,45 @@ defmodule Aprsme.Callsign do @moduledoc """ - Utilities for callsign normalization, validation, and manipulation. + Validation and normalization for APRS source identifiers. + + RF callsigns follow AX.25's shorter shape, while APRS-IS also carries + application and gateway identifiers. Validation therefore enforces a + conservative transport-safe character set and length without rejecting + established APRS-IS identifiers. """ - @doc """ - Normalizes a callsign by trimming whitespace and converting to uppercase. - """ - @spec normalize(String.t() | nil) :: String.t() - def normalize(nil), do: "" - - def normalize(callsign) when is_binary(callsign) do - callsign - |> String.trim() - |> String.upcase() - end - - def normalize(_), do: "" - - @doc """ - Checks whether a callsign is a non-empty string. - - APRS uses a wide variety of identifiers beyond standard amateur radio - callsigns, including tactical callsigns, object names, and item names. - This function intentionally accepts any non-empty, non-whitespace-only - string to accommodate that variety. - - ## Examples - - iex> Aprsme.Callsign.valid?("W5ABC") - true - - iex> Aprsme.Callsign.valid?("W5ABC-15") - true - - iex> Aprsme.Callsign.valid?("TACTICAL1") - true - - iex> Aprsme.Callsign.valid?("") - false - - iex> Aprsme.Callsign.valid?(nil) - false - """ - @spec valid?(String.t() | nil) :: boolean() - def valid?(nil), do: false + @safe_identifier_regex ~r/^[A-Z0-9]+(?:-[A-Z0-9]+)*$/ + @spec valid?(term()) :: boolean() def valid?(callsign) when is_binary(callsign) do - trimmed = String.trim(callsign) - - # Accept any non-empty callsign — APRS uses tactical callsigns, - # object names, and other non-standard identifiers - trimmed != "" + normalized = normalize(callsign) + byte_size(normalized) in 1..20 and Regex.match?(@safe_identifier_regex, normalized) end - def valid?(_), do: false + def valid?(_callsign), do: false - @doc """ - Checks if a packet's sender matches the target callsign. - Both callsigns are normalized before comparison. - """ - @spec matches?(String.t() | nil, String.t() | nil) :: boolean() - def matches?(packet_callsign, target_callsign) do - normalize(packet_callsign) == normalize(target_callsign) - end + @spec normalize(term()) :: String.t() + def normalize(callsign) when is_binary(callsign), do: callsign |> String.trim() |> String.upcase() + def normalize(_callsign), do: "" - @doc """ - Extracts the base callsign from a full callsign (removes SSID if present). + @spec matches?(term(), term()) :: boolean() + def matches?(left, right), do: normalize(left) == normalize(right) - ## Examples + @spec extract_base(term()) :: String.t() + def extract_base(callsign), do: callsign |> extract_parts() |> elem(0) - iex> Aprsme.Callsign.extract_base("W5ABC-15") - "W5ABC" - - iex> Aprsme.Callsign.extract_base("W5ABC") - "W5ABC" - """ - @spec extract_base(String.t() | nil) :: String.t() - def extract_base(nil), do: "" - - def extract_base(callsign) when is_binary(callsign) do - # Split on last hyphen to get base callsign - case String.split(callsign, "-") do - parts when length(parts) > 1 -> - parts |> Enum.drop(-1) |> Enum.join("-") - - _ -> - callsign - end - end - - def extract_base(_), do: "" - - @doc """ - Extracts the SSID from a callsign, returning "0" if no SSID is present. - - ## Examples - - iex> Aprsme.Callsign.extract_ssid("W5ABC-15") - "15" - - iex> Aprsme.Callsign.extract_ssid("W5ABC") - "0" - """ - @spec extract_ssid(String.t() | nil) :: String.t() - def extract_ssid(nil), do: "0" - - def extract_ssid(callsign) when is_binary(callsign) do - # Extract whatever comes after the last hyphen as SSID - case String.split(callsign, "-") do - parts when length(parts) > 1 -> List.last(parts) - _ -> "0" - end - end - - def extract_ssid(_), do: "0" - - @doc """ - Extracts both base callsign and SSID as a tuple. - - ## Examples - - iex> Aprsme.Callsign.extract_parts("W5ABC-15") - {"W5ABC", "15"} - - iex> Aprsme.Callsign.extract_parts("W5ABC") - {"W5ABC", "0"} - """ - @spec extract_parts(String.t() | nil) :: {String.t(), String.t()} - def extract_parts(nil), do: {"", "0"} + @spec extract_ssid(term()) :: String.t() + def extract_ssid(callsign), do: callsign |> extract_parts() |> elem(1) + @spec extract_parts(term()) :: {String.t(), String.t()} def extract_parts(callsign) when is_binary(callsign) do - # Split on last hyphen to separate base and SSID - case String.split(callsign, "-") do - parts when length(parts) > 1 -> - ssid = List.last(parts) - base = parts |> Enum.drop(-1) |> Enum.join("-") - {base, ssid} + normalized = normalize(callsign) - _ -> - {callsign, "0"} + case Regex.run(~r/^(.*)-([0-9]+)$/, normalized) do + [_, base, ssid] -> {base, ssid} + _ -> {normalized, "0"} end end - def extract_parts(_), do: {"", "0"} + def extract_parts(_callsign), do: {"", "0"} end diff --git a/lib/aprsme/cleanup_scheduler.ex b/lib/aprsme/cleanup_scheduler.ex index f77125c..a6ffd05 100644 --- a/lib/aprsme/cleanup_scheduler.ex +++ b/lib/aprsme/cleanup_scheduler.ex @@ -35,17 +35,8 @@ defmodule Aprsme.CleanupScheduler do @impl true def handle_info(:schedule_cleanup, %{interval: interval} = state) when is_integer(interval) do - Logger.info("Running packet cleanup task") - - _ = - Aprsme.BroadcastTaskSupervisor.async_execute(fn -> - try do - PacketCleanupWorker.perform(%{}) - rescue - error -> - Logger.error("Packet cleanup task failed: #{inspect(error)}") - end - end) + Logger.info("Running bad-packet cleanup task") + _ = PacketCleanupWorker.perform(%{}) schedule_next_cleanup(interval) {:noreply, state} diff --git a/lib/aprsme/cluster/packet_distributor.ex b/lib/aprsme/cluster/packet_distributor.ex index a382428..98be341 100644 --- a/lib/aprsme/cluster/packet_distributor.ex +++ b/lib/aprsme/cluster/packet_distributor.ex @@ -32,7 +32,6 @@ defmodule Aprsme.Cluster.PacketDistributor do end def handle_distributed_packet({:distributed_packet, packet}) do - Aprsme.StreamingPacketsPubSub.broadcast_packet(packet) Aprsme.SpatialPubSub.broadcast_packet(packet) :ok diff --git a/lib/aprsme/cluster/packet_receiver.ex b/lib/aprsme/cluster/packet_receiver.ex deleted file mode 100644 index a4ce773..0000000 --- a/lib/aprsme/cluster/packet_receiver.ex +++ /dev/null @@ -1,42 +0,0 @@ -defmodule Aprsme.Cluster.PacketReceiver do - @moduledoc """ - Receives distributed packets from the cluster leader on non-leader nodes. - Ensures all nodes can serve real-time updates even though only the leader - processes APRS packets. - """ - use GenServer - - alias Aprsme.Cluster.LeaderElection - alias Aprsme.Cluster.PacketDistributor - - require Logger - - def start_link(opts) do - GenServer.start_link(__MODULE__, opts, name: __MODULE__) - end - - @impl true - def init(_opts) do - # Subscribe to distributed packets - :ok = PacketDistributor.subscribe() - - Logger.info("Started packet receiver on node #{node()}") - - {:ok, %{}} - end - - @impl true - def handle_info({:distributed_packet, packet}, state) do - # Only process if we're not the leader (leader already processed locally) - if !LeaderElection.leader?() do - PacketDistributor.handle_distributed_packet({:distributed_packet, packet}) - end - - {:noreply, state} - end - - @impl true - def handle_info(_msg, state) do - {:noreply, state} - end -end diff --git a/lib/aprsme/db_optimizer.ex b/lib/aprsme/db_optimizer.ex deleted file mode 100644 index 479de90..0000000 --- a/lib/aprsme/db_optimizer.ex +++ /dev/null @@ -1,180 +0,0 @@ -defmodule Aprsme.DbOptimizer do - @moduledoc """ - Database optimization utilities that leverage PostgreSQL server configuration - for better performance on ARM RK3588 with 16GB RAM. - - PostgreSQL key settings we're optimizing for: - - max_connections = 100 - - shared_buffers = 1GB - - work_mem = 16MB - - synchronous_commit = off - - effective_io_concurrency = 100 (SSD optimized) - """ - - alias Aprsme.Repo - alias Ecto.Adapters.SQL - - require Logger - - @doc """ - Optimized batch insert that leverages PostgreSQL configuration - """ - def optimized_batch_insert(schema, entries, opts \\ []) do - # Calculate optimal batch size based on work_mem - optimal_batch_size = calculate_optimal_batch_size(entries) - - # Split into optimal chunks - entries - |> Enum.chunk_every(optimal_batch_size) - |> Enum.map(fn batch -> - insert_opts = - Keyword.merge( - [ - returning: false, - on_conflict: :nothing, - timeout: 60_000 - ], - opts - ) - - try do - {count, _} = Repo.insert_all(schema, batch, insert_opts) - {:ok, count} - rescue - error -> {:error, error} - end - end) - |> Enum.reduce({0, 0}, fn - {:ok, count}, {success, errors} -> {success + count, errors} - {:error, _}, {success, errors} -> {success, errors + 1} - end) - end - - @doc """ - Calculate optimal batch size based on PostgreSQL work_mem setting (16MB) - and estimated row size - """ - def calculate_optimal_batch_size(entries) when is_list(entries) do - # Estimate size of one entry (rough approximation) - sample = List.first(entries) - estimated_size = estimate_entry_size(sample) - - # work_mem is 16MB, leave some headroom - # 14MB in bytes - available_memory = 14 * 1024 * 1024 - - # Calculate how many entries fit in work_mem - max_batch = div(available_memory, estimated_size) - - # Cap at reasonable limits - max_batch |> min(2000) |> max(100) - end - - @doc """ - Run ANALYZE on a table after bulk inserts to update statistics - This helps PostgreSQL make better query plans - """ - def analyze_table(table_name) do - validate_identifier!(table_name) - quoted_name = quote_identifier(table_name) - _ = SQL.query!(Repo, "ANALYZE #{quoted_name}", []) - :ok - rescue - error -> - Logger.warning("Failed to analyze table #{table_name}: #{inspect(error)}") - :error - end - - @doc """ - Vacuum a table to reclaim space and update visibility map - Use this after large delete operations - """ - def vacuum_table(table_name, opts \\ []) do - validate_identifier!(table_name) - quoted_name = quote_identifier(table_name) - full = Keyword.get(opts, :full, false) - analyze = Keyword.get(opts, :analyze, true) - - vacuum_type = if full, do: "VACUUM FULL", else: "VACUUM" - analyze_clause = if analyze, do: " ANALYZE", else: "" - - query = "#{vacuum_type}#{analyze_clause} #{quoted_name}" - - _ = SQL.query!(Repo, query, [], timeout: :infinity) - :ok - rescue - error -> - Logger.error("Failed to vacuum table #{table_name}: #{inspect(error)}") - :error - end - - @doc """ - Get current database statistics for monitoring - """ - def get_connection_stats do - query = """ - SELECT - count(*) as total_connections, - count(*) FILTER (WHERE state = 'active') as active_connections, - count(*) FILTER (WHERE state = 'idle') as idle_connections, - count(*) FILTER (WHERE state = 'idle in transaction') as idle_in_transaction, - count(*) FILTER (WHERE wait_event_type IS NOT NULL) as waiting_connections - FROM pg_stat_activity - WHERE datname = current_database() - """ - - case SQL.query(Repo, query, []) do - {:ok, %{rows: [[total, active, idle, idle_tx, waiting]]}} -> - %{ - total: total, - active: active, - idle: idle, - idle_in_transaction: idle_tx, - waiting: waiting - } - - _ -> - %{} - end - end - - # Private functions - - defp validate_identifier!(name) when is_binary(name) do - if !Regex.match?(~r/\A[a-zA-Z_][a-zA-Z0-9_]*\z/, name) do - raise ArgumentError, "invalid SQL identifier: #{inspect(name)}" - end - end - - defp validate_identifier!(name) when is_atom(name), do: validate_identifier!(Atom.to_string(name)) - - defp quote_identifier(name) when is_binary(name) do - ~s("#{String.replace(name, ~s("), ~s(""))}") - end - - defp quote_identifier(name) when is_atom(name), do: quote_identifier(Atom.to_string(name)) - - defp estimate_entry_size(nil), do: 1024 - - defp estimate_entry_size(entry) when is_map(entry) do - # Rough estimation of entry size in bytes - entry - |> Map.values() - |> Enum.map(&estimate_value_size/1) - |> Enum.sum() - # Add overhead for structure - |> Kernel.+(100) - end - - defp estimate_entry_size(_), do: 1024 - - defp estimate_value_size(nil), do: 4 - defp estimate_value_size(value) when is_binary(value), do: byte_size(value) - defp estimate_value_size(value) when is_integer(value), do: 8 - defp estimate_value_size(value) when is_float(value), do: 8 - defp estimate_value_size(value) when is_boolean(value), do: 1 - defp estimate_value_size(%DateTime{}), do: 8 - defp estimate_value_size(%Date{}), do: 4 - # Conservative estimate for complex types - defp estimate_value_size(_), do: 50 -end diff --git a/lib/aprsme/deployment_notifier.ex b/lib/aprsme/deployment_notifier.ex index 5085d10..a20ed14 100644 --- a/lib/aprsme/deployment_notifier.ex +++ b/lib/aprsme/deployment_notifier.ex @@ -4,6 +4,28 @@ defmodule Aprsme.DeploymentNotifier do Called from the release module when a deployment is detected. """ + use GenServer + + require Logger + + def start_link(opts \\ []) do + GenServer.start_link(__MODULE__, opts, name: __MODULE__) + end + + @impl true + def init(_opts) do + if System.get_env("DEPLOYED_AT"), do: Process.send_after(self(), :notify_deployment, 10_000) + {:ok, %{}} + end + + @impl true + def handle_info(:notify_deployment, state) do + deployed_at = Aprsme.Release.deployed_at() + :ok = notify_deployment(deployed_at) + Logger.info("Deployment notification sent for timestamp: #{deployed_at}") + {:noreply, state} + end + @doc """ Notify about a new deployment immediately. """ diff --git a/lib/aprsme/packet_consumer.ex b/lib/aprsme/packet_consumer.ex index e6f2b5c..d77db41 100644 --- a/lib/aprsme/packet_consumer.ex +++ b/lib/aprsme/packet_consumer.ex @@ -377,7 +377,6 @@ defmodule Aprsme.PacketConsumer do if cluster_enabled do PacketDistributor.distribute_packet(packet) else - Aprsme.StreamingPacketsPubSub.broadcast_packet(packet) Aprsme.SpatialPubSub.broadcast_packet(packet) end @@ -513,7 +512,7 @@ defmodule Aprsme.PacketConsumer do end defp valid_packet?(nil), do: false - defp valid_packet?(%{sender: sender}) when is_binary(sender) and byte_size(sender) > 0, do: true + defp valid_packet?(%{sender: sender}), do: Aprsme.Callsign.valid?(sender) defp valid_packet?(_), do: false # Detect if packet is an item or object and set appropriate fields diff --git a/lib/aprsme/packet_counter.ex b/lib/aprsme/packet_counter.ex deleted file mode 100644 index a0dbe49..0000000 --- a/lib/aprsme/packet_counter.ex +++ /dev/null @@ -1,43 +0,0 @@ -defmodule Aprsme.PacketCounter do - @moduledoc """ - Schema for the packet_counters table that provides O(1) packet counting. - """ - use Ecto.Schema - - import Ecto.Query - - alias Aprsme.Repo - - schema "packet_counters" do - field :counter_type, :string - field :count, :integer, default: 0 - - timestamps() - end - - @doc """ - Gets the current packet count instantly. - This is an O(1) operation thanks to the counter table. - """ - def get_count(counter_type \\ "total_packets") do - from(pc in __MODULE__, - where: pc.counter_type == ^counter_type, - select: pc.count - ) - |> Repo.one() - |> case do - nil -> 0 - count -> count - end - end - - @doc """ - Subscribe to packet count changes via PostgreSQL LISTEN/NOTIFY. - This allows real-time updates without polling. - """ - def subscribe_to_changes do - # This would require setting up PostgreSQL LISTEN/NOTIFY - # For now, we'll rely on the existing PubSub mechanism - :ok - end -end diff --git a/lib/aprsme/packet_replay.ex b/lib/aprsme/packet_replay.ex deleted file mode 100644 index 1908346..0000000 --- a/lib/aprsme/packet_replay.ex +++ /dev/null @@ -1,453 +0,0 @@ -defmodule Aprsme.PacketReplay do - @moduledoc """ - Handles replaying of historical APRS packets alongside live packets. - - This module provides functionality to: - 1. Start a replay session for historical packets - 2. Stream historical packets to a client with timing similar to original reception - 3. Merge historical packet streams with live packets - 4. Control replay parameters (speed, filters, etc.) - """ - - use GenServer - - alias Aprsme.Packets - alias AprsmeWeb.Endpoint - - require Logger - - @default_replay_window_minutes 60 - @default_replay_speed 5.0 - - @type state :: %{ - user_id: String.t(), - replay_topic: String.t(), - replay_speed: float(), - start_time: DateTime.t(), - end_time: DateTime.t(), - region: String.t() | nil, - bounds: map(), - callsign: String.t() | nil, - with_position: boolean(), - limit: pos_integer(), - paused: boolean(), - packets_sent: non_neg_integer(), - replay_started_at: DateTime.t(), - replay_timer: reference() | nil, - last_packet_time: DateTime.t() | nil - } - - # Client API - - @doc """ - Starts a packet replay session for a specific user/connection. - - ## Options - * `:user_id` - Unique identifier for the user/session (required) - * `:bounds` - Bounding box for visible map area [min_lon, min_lat, max_lon, max_lat] (required) - * `:callsign` - Filter packets by callsign (optional) - * `:start_time` - Start time for replay (default: 60 minutes ago) - * `:end_time` - End time for replay (default: now) - * `:replay_speed` - Speed multiplier for replay (default: 5.0) - * `:limit` - Maximum number of packets to replay (default: 5000) - * `:with_position` - Only include packets with position data (default: true) - """ - @spec start_replay(keyword()) :: {:ok, pid()} | {:error, any()} - def start_replay(opts) do - user_id = Keyword.fetch!(opts, :user_id) - - # Ensure bounds are provided for map area filtering - bounds = Keyword.get(opts, :bounds) - - if !bounds do - raise ArgumentError, "Map bounds are required for packet replay" - end - - # Convert user_id to process name - name = via_tuple(user_id) - - GenServer.start_link(__MODULE__, opts, name: name) - end - - @doc """ - Stops an active replay session for a user. - """ - @spec stop_replay(String.t()) :: :ok - def stop_replay(user_id) do - name = via_tuple(user_id) - - if GenServer.whereis(name) do - GenServer.stop(name) - else - {:error, :not_found} - end - end - - @doc """ - Pauses an active replay session. - """ - @spec pause_replay(String.t()) :: :ok | :already_paused - def pause_replay(user_id) do - name = via_tuple(user_id) - GenServer.call(name, :pause) - end - - @doc """ - Resumes a paused replay session. - """ - @spec resume_replay(String.t()) :: :ok - def resume_replay(user_id) do - name = via_tuple(user_id) - GenServer.call(name, :resume) - end - - @doc """ - Changes the replay speed. - """ - @spec set_replay_speed(String.t(), number()) :: :ok - def set_replay_speed(user_id, speed) when is_number(speed) and speed > 0 do - name = via_tuple(user_id) - GenServer.call(name, {:set_speed, speed}) - end - - @doc """ - Updates replay filters (region, bounds, callsign, etc.) - """ - @spec update_filters(String.t(), keyword()) :: :ok - def update_filters(user_id, filters) when is_list(filters) do - name = via_tuple(user_id) - GenServer.call(name, {:update_filters, filters}) - end - - @doc """ - Gets information about the current replay session. - """ - @spec get_replay_info(String.t()) :: map() - def get_replay_info(user_id) do - name = via_tuple(user_id) - GenServer.call(name, :get_info) - end - - # Server implementation - - @impl true - @spec init(keyword()) :: {:ok, state()} - def init(opts) do - user_id = Keyword.fetch!(opts, :user_id) - - # Default time range - always use last hour at most - now = DateTime.utc_now() - window_minutes = @default_replay_window_minutes - - # Force start time to be at most 1 hour ago - default_start_time = DateTime.add(now, -window_minutes * 60, :second) - user_start_time = Keyword.get(opts, :start_time) - - # If user provided a start time, make sure it's not older than 1 hour - effective_start_time = - if user_start_time && DateTime.before?(user_start_time, default_start_time) do - default_start_time - else - user_start_time || default_start_time - end - - # Set up initial state - state = %{ - user_id: user_id, - replay_topic: "replay:#{user_id}", - replay_speed: Keyword.get(opts, :replay_speed, @default_replay_speed), - start_time: effective_start_time, - end_time: Keyword.get(opts, :end_time, now), - # Not using region - using bounds instead - region: nil, - bounds: Keyword.fetch!(opts, :bounds), - callsign: Keyword.get(opts, :callsign), - with_position: Keyword.get(opts, :with_position, true), - limit: Keyword.get(opts, :limit, 5000), - paused: false, - packets_sent: 0, - replay_started_at: now, - replay_timer: nil, - last_packet_time: nil - } - - # Start the replay immediately - send(self(), :start_replay) - - {:ok, state} - end - - @impl true - @spec handle_info(any(), state()) :: {:noreply, state()} | {:stop, :normal, state()} - def handle_info(:start_replay, state) do - # Fetch historical packets based on filters - # Always filter by bounds (visible map area) and limit to packets with position - replay_opts = [ - start_time: state.start_time, - end_time: state.end_time, - limit: state.limit, - callsign: state.callsign, - with_position: true, - bounds: state.bounds - ] - - # Log the start of replay with map bounds - Logger.info( - "Starting packet replay for user #{state.user_id} in map area #{inspect(state.bounds)} for the last hour" - ) - - # Send notification to client that replay is starting - _ = - Endpoint.broadcast(state.replay_topic, "replay_started", %{ - total_packets: Packets.get_historical_packet_count(Map.new(replay_opts)), - start_time: state.start_time, - end_time: state.end_time, - replay_speed: state.replay_speed, - bounds: state.bounds - }) - - # Get packets and start streaming - replay_opts_map = - replay_opts - |> Keyword.put(:playback_speed, state.replay_speed) - |> Map.new() - - stream = Packets.stream_packets_for_replay(replay_opts_map) - - # Schedule the first packet - case stream |> Stream.take(1) |> Enum.to_list() do - [{delay, packet}] -> - # Convert delay to milliseconds - delay_ms = trunc(delay * 1000) - timer = Process.send_after(self(), {:send_packet, packet, stream}, delay_ms) - - {:noreply, %{state | replay_timer: timer, last_packet_time: packet.received_at}} - - [] -> - # No packets found, end replay immediately - _ = - Endpoint.broadcast(state.replay_topic, "replay_complete", %{ - packets_sent: 0, - message: "No matching packets found for replay" - }) - - {:stop, :normal, state} - end - end - - @impl true - def handle_info({:send_packet, packet, stream}, %{paused: true} = state) do - # If paused, reschedule the current packet - timer = Process.send_after(self(), {:send_packet, packet, stream}, 1000) - {:noreply, %{state | replay_timer: timer}} - end - - @impl true - def handle_info({:send_packet, packet, stream}, state) do - # Send the packet to the client - _ = - Endpoint.broadcast(state.replay_topic, "historical_packet", %{ - packet: sanitize_packet_for_transport(packet), - timestamp: packet.received_at, - is_historical: true - }) - - # Update counter - new_packets_sent = state.packets_sent + 1 - - # Schedule the next packet - case stream |> Stream.take(1) |> Enum.to_list() do - [{delay, next_packet}] -> - # Convert delay to milliseconds - delay_ms = trunc(delay * 1000) - timer = Process.send_after(self(), {:send_packet, next_packet, stream}, delay_ms) - - {:noreply, - %{state | replay_timer: timer, packets_sent: new_packets_sent, last_packet_time: next_packet.received_at}} - - [] -> - # No more packets, end replay - _ = - Endpoint.broadcast(state.replay_topic, "replay_complete", %{ - packets_sent: new_packets_sent, - message: "Replay complete" - }) - - {:stop, :normal, %{state | packets_sent: new_packets_sent}} - end - end - - @impl true - @spec handle_call(any(), {pid(), any()}, state()) :: {:reply, any(), state()} - def handle_call(:pause, _from, state) do - _ = - if state.replay_timer do - Process.cancel_timer(state.replay_timer) - end - - _ = - Endpoint.broadcast(state.replay_topic, "replay_paused", %{ - packets_sent: state.packets_sent, - last_packet_time: state.last_packet_time - }) - - {:reply, :ok, %{state | paused: true, replay_timer: nil}} - end - - @impl true - def handle_call(:resume, _from, %{paused: true} = state) do - # Force the next packet to be sent soon - send(self(), :start_replay) - - _ = - Endpoint.broadcast(state.replay_topic, "replay_resumed", %{ - packets_sent: state.packets_sent, - last_packet_time: state.last_packet_time - }) - - {:reply, :ok, %{state | paused: false}} - end - - @impl true - def handle_call(:resume, _from, state) do - # Already running, just acknowledge - {:reply, :ok, state} - end - - @impl true - def handle_call({:set_speed, speed}, _from, state) do - # Update speed and notify client - _ = - Endpoint.broadcast(state.replay_topic, "replay_speed_changed", %{ - replay_speed: speed - }) - - {:reply, :ok, %{state | replay_speed: speed}} - end - - @impl true - def handle_call({:update_filters, filters}, _from, state) do - # Update filters - this requires restarting the replay - _ = - if state.replay_timer do - Process.cancel_timer(state.replay_timer) - end - - # Update state with new filters - new_state = - Enum.reduce(filters, state, fn {key, value}, acc -> - Map.put(acc, key, value) - end) - - # Always ensure we're filtering by map bounds - new_state = - if not Map.has_key?(new_state, :bounds) or is_nil(new_state.bounds) do - # Keep existing bounds if none provided - new_state - else - # Validate new bounds if provided - case new_state.bounds do - [min_lon, min_lat, max_lon, max_lat] - when is_number(min_lon) and is_number(min_lat) and - is_number(max_lon) and is_number(max_lat) -> - new_state - - _ -> - # Invalid bounds format, keep old bounds - %{new_state | bounds: state.bounds} - end - end - - # Restart replay with new filters - send(self(), :start_replay) - - {:reply, :ok, %{new_state | replay_timer: nil, packets_sent: 0}} - end - - @impl true - def handle_call(:get_info, _from, state) do - info = %{ - user_id: state.user_id, - replay_speed: state.replay_speed, - start_time: state.start_time, - end_time: state.end_time, - bounds: state.bounds, - callsign: state.callsign, - with_position: state.with_position, - packets_sent: state.packets_sent, - paused: state.paused, - replay_started_at: state.replay_started_at, - last_packet_time: state.last_packet_time - } - - {:reply, info, state} - end - - @impl true - @spec terminate(any(), state()) :: :ok - def terminate(_reason, state) do - # Clean up any timers - _ = - if state.replay_timer do - Process.cancel_timer(state.replay_timer) - end - - # Notify client that replay has ended - _ = - Endpoint.broadcast(state.replay_topic, "replay_stopped", %{ - packets_sent: state.packets_sent, - message: "Replay stopped" - }) - - :ok - end - - # Helper functions - - @spec via_tuple(String.t()) :: {:via, Registry, {atom(), String.t()}} - defp via_tuple(user_id) do - {:via, Registry, {Aprsme.ReplayRegistry, "replay:#{user_id}"}} - end - - @spec sanitize_packet_for_transport(struct()) :: map() - defp sanitize_packet_for_transport(packet) do - # Convert to map and ensure all fields are JSON-safe - packet - |> Map.from_struct() - |> Map.drop([:__meta__, :__struct__]) - |> sanitize_map_values() - end - - @spec sanitize_map_values(map()) :: map() - defp sanitize_map_values(map) when is_map(map) do - Enum.reduce(map, %{}, fn {k, v}, acc -> - Map.put(acc, k, sanitize_value(v)) - end) - end - - defp sanitize_value(%DateTime{} = dt) do - DateTime.to_iso8601(dt) - end - - defp sanitize_value(%NaiveDateTime{} = dt) do - NaiveDateTime.to_iso8601(dt) - end - - defp sanitize_value(%Decimal{} = d) do - Decimal.to_float(d) - end - - defp sanitize_value(%_{} = _struct), do: nil - - defp sanitize_value(value) when is_map(value) do - sanitize_map_values(value) - end - - defp sanitize_value(value) when is_list(value) do - Enum.map(value, &sanitize_value/1) - end - - defp sanitize_value(value) do - value - end -end diff --git a/lib/aprsme/packet_replay_behaviour.ex b/lib/aprsme/packet_replay_behaviour.ex deleted file mode 100644 index 8b3373d..0000000 --- a/lib/aprsme/packet_replay_behaviour.ex +++ /dev/null @@ -1,13 +0,0 @@ -defmodule Aprsme.PacketReplayBehaviour do - @moduledoc """ - Behavior definition for the PacketReplay module. - This allows us to mock the PacketReplay module in tests. - """ - - @callback start_replay(pid(), list(Aprsme.Packet.t()), keyword()) :: {:ok, reference()} | {:error, term()} - @callback stop_replay(reference()) :: :ok - @callback pause_replay(reference()) :: :ok | {:error, :not_found} - @callback resume_replay(reference()) :: :ok | {:error, :not_found} - @callback get_replay_status(reference()) :: {:ok, map()} | {:error, :not_found} - @callback adjust_replay_speed(reference(), float()) :: :ok | {:error, :not_found} -end diff --git a/lib/aprsme/packets.ex b/lib/aprsme/packets.ex index 69bf04a..6fdbc73 100644 --- a/lib/aprsme/packets.ex +++ b/lib/aprsme/packets.ex @@ -24,7 +24,8 @@ defmodule Aprsme.Packets do """ @spec store_packet(map()) :: {:ok, struct()} | {:error, :validation_error | :storage_exception} def store_packet(packet_data) do - with {:ok, sanitized_data} <- sanitize_packet_data(packet_data), + with :ok <- validate_sender(packet_data), + {:ok, sanitized_data} <- sanitize_packet_data(packet_data), {:ok, packet_attrs} <- build_packet_attrs(sanitized_data), {:ok, packet} <- insert_packet(packet_attrs, packet_data) do {:ok, packet} @@ -46,6 +47,18 @@ defmodule Aprsme.Packets do end end + defp validate_sender(packet_data) when is_map(packet_data) do + sender = Map.get(packet_data, :sender) || Map.get(packet_data, "sender") + + if Aprsme.Callsign.valid?(sender) do + :ok + else + {:error, {:validation_error, "sender: must be a safe APRS identifier"}} + end + end + + defp validate_sender(_packet_data), do: {:error, {:validation_error, "sender: must be a safe APRS identifier"}} + # Pattern matching for sanitization defp sanitize_packet_data(packet_data) do {:ok, Aprsme.EncodingUtils.sanitize_packet(packet_data)} @@ -374,40 +387,6 @@ defmodule Aprsme.Packets do defp get_canonical_device_identifier(device_identifier), do: to_string(device_identifier) - @doc """ - Gets packets for replay. - - ## Parameters - * `opts` - Map of options for filtering and pagination: - * `:lat` - Latitude for center point filtering - * `:lon` - Longitude for center point filtering - * `:radius` - Radius in kilometers for filtering - * `:callsign` - Filter by callsign - * `:region` - Filter by region - * `:start_time` - Start time for replay (DateTime) - * `:end_time` - End time for replay (DateTime) - * `:limit` - Maximum number of packets to return - * `:page` - Page number for pagination - """ - @impl true - def get_packets_for_replay(opts \\ %{}) do - limit = Map.get(opts, :limit, 1000) - bounds = Map.get(opts, :bounds) - - query = - from(p in Packet) - |> QueryBuilder.with_position() - |> QueryBuilder.with_time_range(opts) - |> QueryBuilder.maybe_filter_region(opts) - |> maybe_filter_by_callsign(opts) - |> maybe_filter_by_bounds(bounds) - |> QueryBuilder.chronological() - |> QueryBuilder.paginate(limit) - |> QueryBuilder.with_coordinates() - - Repo.all(query) - end - defp maybe_filter_by_callsign(query, %{callsign: callsign}) when not is_nil(callsign) do QueryBuilder.for_callsign(query, callsign) end @@ -580,43 +559,9 @@ defmodule Aprsme.Packets do # Filter for weather packets at the database level - @doc """ - Retrieves a continuous stream of stored packets for replay in chronological order. - - This function returns a Stream that can be used to process packets in chronological - order, preserving the timing between packets. - - ## Parameters - * `opts` - The same options as `get_packets_for_replay/1` - * `:playback_speed` - Speed multiplier (1.0 = real-time, 2.0 = 2x speed, etc.) - - ## Returns - * Stream of packets with timing information - """ - @impl true - def stream_packets_for_replay(opts \\ %{}) do - packets = get_packets_for_replay(opts) - playback_speed = Map.get(opts, :playback_speed, 1.0) - - # Return a stream that emits packets with their original timing - Stream.unfold({packets, nil}, fn - {[], _} -> - nil - - {[packet | rest], nil} -> - {{0, packet}, {rest, packet}} - - {[next | rest], prev} -> - # Calculate delay between packets in milliseconds, then convert to seconds - delay_ms = DateTime.diff(next.received_at, prev.received_at, :millisecond) - adjusted_delay = delay_ms / (playback_speed * 1000) - {{adjusted_delay, next}, {rest, next}} - end) - end - @doc """ Gets the total count of stored packets in the database. - Uses the efficient packet_counters table with triggers for O(1) performance. + Uses the exact retained packet counter maintained by packet DML and partition cleanup. """ @spec get_total_packet_count() :: non_neg_integer() def get_total_packet_count do @@ -626,21 +571,11 @@ defmodule Aprsme.Packets do count {:ok, %{rows: [[nil]]}} -> - # Fallback to actual count if counter is not initialized - Repo.one(from p in Packet, select: count(p.id)) || 0 + 0 - {:error, _} -> - # Try the faster counter table directly as a fallback - query = """ - SELECT count FROM packet_counters - WHERE counter_type = 'total_packets' - LIMIT 1 - """ - - case Repo.query(query, []) do - {:ok, %{rows: [[count]]}} -> count - _ -> 0 - end + {:error, reason} -> + Logger.warning("Unable to read packet count: #{inspect(reason)}") + 0 end rescue DBConnection.ConnectionError -> @@ -666,45 +601,6 @@ defmodule Aprsme.Packets do _ -> nil end - @doc """ - Configure packet retention policy. - - Packets are retained based on these rules: - - Default retention is 365 days (1 year) (configurable via :packet_retention_days) - - Returns the number of packets deleted - """ - @impl true - def clean_old_packets do - retention_days = Application.get_env(:aprsme, :packet_retention_days, 7) - cutoff_time = DateTime.add(DateTime.utc_now(), -retention_days * 86_400, :second) - - {deleted_count, _} = Repo.delete_all(from(p in Packet, where: p.received_at < ^cutoff_time)) - - deleted_count - end - - @doc """ - Clean packets older than a specific number of days. - - This function allows for more granular cleanup operations by specifying - the exact age threshold for packet deletion. - - ## Parameters - - `days` - Number of days to keep (packets older than this will be deleted) - - ## Returns - - Number of packets deleted - """ - @impl true - @spec clean_packets_older_than(pos_integer()) :: {:ok, non_neg_integer()} | {:error, any()} - def clean_packets_older_than(days) when is_integer(days) and days > 0 do - cutoff_time = DateTime.add(DateTime.utc_now(), -days * 86_400, :second) - - {deleted_count, _} = Repo.delete_all(from(p in Packet, where: p.received_at < ^cutoff_time)) - - {:ok, deleted_count} - end - # Get packets from last hour only - used to initialize the map @spec get_last_hour_packets() :: [struct()] def get_last_hour_packets do diff --git a/lib/aprsme/packets_behaviour.ex b/lib/aprsme/packets_behaviour.ex index 9ce16cb..b84a5b4 100644 --- a/lib/aprsme/packets_behaviour.ex +++ b/lib/aprsme/packets_behaviour.ex @@ -4,11 +4,7 @@ defmodule Aprsme.PacketsBehaviour do """ @callback get_historical_packet_count(map()) :: non_neg_integer() - @callback stream_packets_for_replay(map()) :: Enumerable.t() - @callback get_packets_for_replay(map()) :: list() @callback get_recent_packets(map()) :: list() @callback get_nearby_stations(float(), float(), String.t() | nil, map()) :: list() @callback get_weather_packets(String.t(), DateTime.t(), DateTime.t(), map()) :: list() - @callback clean_old_packets() :: {:ok, non_neg_integer()} | {:error, any()} - @callback clean_packets_older_than(pos_integer()) :: {:ok, non_neg_integer()} | {:error, any()} end diff --git a/lib/aprsme/partition_manager.ex b/lib/aprsme/partition_manager.ex index e02ae27..888cafd 100644 --- a/lib/aprsme/partition_manager.ex +++ b/lib/aprsme/partition_manager.ex @@ -86,14 +86,18 @@ defmodule Aprsme.PartitionManager do """ @spec drop_old_partitions(pos_integer()) :: {:ok, [String.t()]} def drop_old_partitions(retention_days \\ 7) do - cutoff_date = Date.add(Date.utc_today(), -retention_days) + cutoff = DateTime.add(DateTime.utc_now(), -retention_days, :day) dropped = list_partitions() |> Enum.filter(fn name -> case partition_date(name) do - {:ok, date} -> Date.compare(date, cutoff_date) != :gt - _ -> false + {:ok, date} -> + {_partition_start, partition_end} = partition_range(date) + DateTime.compare(partition_end, cutoff) != :gt + + _ -> + false end end) |> Enum.map(fn name -> @@ -195,7 +199,28 @@ defmodule Aprsme.PartitionManager do # Validate partition name to prevent SQL injection validate_partition_name!(name) - _ = Repo.query!("DROP TABLE IF EXISTS #{quote_identifier(name)}") + quoted_name = quote_identifier(name) + lock_key = :erlang.phash2("drop:#{name}") + + _ = + Repo.transaction(fn -> + _ = Repo.query!("SELECT pg_advisory_xact_lock($1)", [lock_key]) + %{rows: [[partition_count]]} = Repo.query!("SELECT COUNT(*) FROM #{quoted_name}") + + _ = + Repo.query!( + """ + UPDATE packet_counters + SET count = GREATEST(0, count - $1), + updated_at = NOW() + WHERE counter_type = 'total_packets' + """, + [partition_count] + ) + + _ = Repo.query!("DROP TABLE IF EXISTS #{quoted_name}") + end) + Logger.debug("Dropped partition #{name}") name end diff --git a/lib/aprsme/performance/insert_optimizer.ex b/lib/aprsme/performance/insert_optimizer.ex deleted file mode 100644 index fbd2f5a..0000000 --- a/lib/aprsme/performance/insert_optimizer.ex +++ /dev/null @@ -1,185 +0,0 @@ -defmodule Aprsme.Performance.InsertOptimizer do - @moduledoc """ - Optimizations specifically for INSERT performance. - - This module contains strategies to improve packet insertion performance: - - Optimized batch sizing based on system load - - Reduced index maintenance overhead - - Streamlined packet preparation - - Connection pool optimizations - """ - - use GenServer - - require Logger - - # Configuration for INSERT optimization - @base_batch_size 200 - @max_batch_size 800 - @min_batch_size 100 - # 30 seconds - @optimization_check_interval 30_000 - - def start_link(opts \\ []) do - GenServer.start_link(__MODULE__, opts, name: __MODULE__) - end - - @doc """ - Get the optimal batch size for current system conditions. - """ - def get_optimal_batch_size do - GenServer.call(__MODULE__, :get_batch_size) - catch - :exit, {:noproc, _} -> @base_batch_size - end - - @doc """ - Get INSERT optimization settings. - """ - def get_insert_options do - GenServer.call(__MODULE__, :get_insert_options) - catch - :exit, {:noproc, _} -> default_insert_options() - end - - @doc """ - Record INSERT performance metrics for optimization. - """ - def record_insert_metrics(batch_size, duration_ms, success_count) do - GenServer.cast(__MODULE__, {:record_metrics, batch_size, duration_ms, success_count}) - end - - @impl true - def init(_opts) do - schedule_optimization_check() - - state = %{ - current_batch_size: @base_batch_size, - insert_options: default_insert_options(), - performance_history: [], - last_optimization: System.monotonic_time(:millisecond) - } - - {:ok, state} - end - - @impl true - def handle_call(:get_batch_size, _from, state) do - {:reply, state.current_batch_size, state} - end - - @impl true - def handle_call(:get_insert_options, _from, state) do - {:reply, state.insert_options, state} - end - - @impl true - def handle_cast({:record_metrics, batch_size, duration_ms, success_count}, state) do - metric = %{ - batch_size: batch_size, - duration_ms: duration_ms, - success_count: success_count, - throughput: throughput(success_count, duration_ms), - timestamp: System.monotonic_time(:millisecond) - } - - # Keep last 20 metrics for analysis - new_history = Enum.take([metric | state.performance_history], 20) - - {:noreply, %{state | performance_history: new_history}} - end - - # Packets-per-second, or 0 when the batch completed instantly (avoid div-by-0). - defp throughput(_count, 0), do: 0 - defp throughput(_count, duration_ms) when duration_ms < 0, do: 0 - defp throughput(count, duration_ms), do: count / (duration_ms / 1000) - - @impl true - def handle_info(:optimize_settings, state) do - new_state = optimize_based_on_performance(state) - schedule_optimization_check() - {:noreply, new_state} - end - - defp schedule_optimization_check do - Process.send_after(self(), :optimize_settings, @optimization_check_interval) - end - - # Require at least three samples before doing anything — match directly on the - # list prefix instead of calling length/1 so we short-circuit for short lists. - defp optimize_based_on_performance(%{performance_history: [_, _, _ | _] = history} = state) do - recent_metrics = Enum.take(history, 5) - sample_size = length(recent_metrics) - avg_throughput = average(recent_metrics, & &1.throughput, sample_size) - avg_duration = average(recent_metrics, & &1.duration_ms, sample_size) - - new_batch_size = calculate_optimal_batch_size(avg_throughput, avg_duration, state.current_batch_size) - new_insert_options = calculate_insert_options(avg_duration) - - Logger.debug( - "INSERT optimization: batch_size=#{new_batch_size}, avg_throughput=#{Float.round(avg_throughput, 2)} pps" - ) - - :telemetry.execute([:aprsme, :insert_optimizer, :batch_size], %{value: new_batch_size}, %{}) - :telemetry.execute([:aprsme, :insert_optimizer, :throughput], %{value: avg_throughput}, %{}) - :telemetry.execute([:aprsme, :insert_optimizer, :duration], %{value: avg_duration}, %{}) - maybe_emit_optimization(new_batch_size, state.current_batch_size) - - %{ - state - | current_batch_size: new_batch_size, - insert_options: new_insert_options, - last_optimization: System.monotonic_time(:millisecond) - } - end - - defp optimize_based_on_performance(state), do: state - - defp average(items, getter, count) do - items |> Enum.map(getter) |> Enum.sum() |> Kernel./(count) - end - - defp maybe_emit_optimization(same, same), do: :ok - - defp maybe_emit_optimization(_new, _old) do - :telemetry.execute([:aprsme, :insert_optimizer, :optimizations], %{count: 1}, %{}) - end - - defp calculate_optimal_batch_size(avg_throughput, avg_duration, current_batch_size) do - cond do - # If throughput is low and duration is high, reduce batch size - avg_throughput < 50 and avg_duration > 5000 -> - max(@min_batch_size, round(current_batch_size * 0.8)) - - # If throughput is good and duration is acceptable, increase batch size - avg_throughput > 200 and avg_duration < 2000 -> - min(@max_batch_size, round(current_batch_size * 1.2)) - - # Otherwise keep current size - true -> - current_batch_size - end - end - - # When INSERTs are slow (>3s avg), use a longer timeout; otherwise stick with defaults. - defp calculate_insert_options(avg_duration) when avg_duration > 3_000 do - Keyword.merge(default_insert_options(), - returning: false, - on_conflict: :nothing, - timeout: 30_000 - ) - end - - defp calculate_insert_options(_avg_duration), do: default_insert_options() - - defp default_insert_options do - [ - # Don't return IDs unless needed - returning: false, - # Skip conflicts instead of raising - on_conflict: :nothing, - # 15 second timeout - timeout: 15_000 - ] - end -end diff --git a/lib/aprsme/prom_ex/plugins/aprsme.ex b/lib/aprsme/prom_ex/plugins/aprsme.ex index d59ce99..20d82ea 100644 --- a/lib/aprsme/prom_ex/plugins/aprsme.ex +++ b/lib/aprsme/prom_ex/plugins/aprsme.ex @@ -10,7 +10,6 @@ defmodule Aprsme.PromEx.Plugins.Aprsme do * Spatial PubSub (`[:aprsme, :spatial_pubsub, ...]`) * Database connection pool (`[:aprsme, :repo, :pool]`) * Postgres database stats (`[:aprsme, :postgres, ...]`) - * Insert optimizer (`[:aprsme, :insert_optimizer, ...]`) """ use PromEx.Plugin @@ -25,7 +24,6 @@ defmodule Aprsme.PromEx.Plugins.Aprsme do spatial_pubsub_metrics(), repo_pool_metrics(), postgres_metrics(), - insert_optimizer_metrics(), api_metrics() ] end @@ -100,19 +98,19 @@ defmodule Aprsme.PromEx.Plugins.Aprsme do measurement: :avg_clients_per_cell, description: "Average clients per grid cell" ), - counter( + last_value( [:aprsme, :spatial_pubsub, :broadcasts, :total], event_name: [:aprsme, :spatial_pubsub, :broadcasts], measurement: :total, description: "Total spatial broadcasts sent" ), - counter( + last_value( [:aprsme, :spatial_pubsub, :broadcasts, :filtered], event_name: [:aprsme, :spatial_pubsub, :broadcasts], measurement: :filtered, description: "Broadcasts filtered out by viewport" ), - counter( + last_value( [:aprsme, :spatial_pubsub, :broadcasts, :packets], event_name: [:aprsme, :spatial_pubsub, :broadcasts], measurement: :packets, @@ -124,7 +122,7 @@ defmodule Aprsme.PromEx.Plugins.Aprsme do measurement: :ratio, description: "Broadcast efficiency ratio (0-1)" ), - counter( + last_value( [:aprsme, :spatial_pubsub, :efficiency, :saved_broadcasts], event_name: [:aprsme, :spatial_pubsub, :efficiency], measurement: :saved_broadcasts, @@ -143,36 +141,6 @@ defmodule Aprsme.PromEx.Plugins.Aprsme do event_name: [:aprsme, :repo, :pool], measurement: :size, description: "Configured DB connection pool size" - ), - last_value( - [:aprsme, :repo, :pool, :idle], - event_name: [:aprsme, :repo, :pool], - measurement: :idle, - description: "Idle DB connections" - ), - last_value( - [:aprsme, :repo, :pool, :busy], - event_name: [:aprsme, :repo, :pool], - measurement: :busy, - description: "Busy DB connections" - ), - last_value( - [:aprsme, :repo, :pool, :available], - event_name: [:aprsme, :repo, :pool], - measurement: :available, - description: "Available DB connections" - ), - last_value( - [:aprsme, :repo, :pool, :queue_length], - event_name: [:aprsme, :repo, :pool], - measurement: :queue_length, - description: "Processes waiting on a DB connection" - ), - last_value( - [:aprsme, :repo, :pool, :total], - event_name: [:aprsme, :repo, :pool], - measurement: :total, - description: "Total connections in the DB pool" ) ] ) @@ -231,19 +199,19 @@ defmodule Aprsme.PromEx.Plugins.Aprsme do measurement: :dead_tuples, description: "Dead rows in the packets table" ), - counter( + last_value( [:aprsme, :postgres, :packets_table, :total_inserts], event_name: [:aprsme, :postgres, :packets_table], measurement: :total_inserts, description: "Total inserts into the packets table" ), - counter( + last_value( [:aprsme, :postgres, :packets_table, :total_updates], event_name: [:aprsme, :postgres, :packets_table], measurement: :total_updates, description: "Total updates on the packets table" ), - counter( + last_value( [:aprsme, :postgres, :packets_table, :total_deletes], event_name: [:aprsme, :postgres, :packets_table], measurement: :total_deletes, @@ -301,38 +269,6 @@ defmodule Aprsme.PromEx.Plugins.Aprsme do ) end - defp insert_optimizer_metrics do - Event.build( - :aprsme_insert_optimizer_event_metrics, - [ - last_value( - [:aprsme, :insert_optimizer, :batch_size], - event_name: [:aprsme, :insert_optimizer, :batch_size], - measurement: :value, - description: "Current batch size chosen by the insert optimizer" - ), - last_value( - [:aprsme, :insert_optimizer, :throughput], - event_name: [:aprsme, :insert_optimizer, :throughput], - measurement: :value, - description: "Average insert throughput observed by the optimizer" - ), - last_value( - [:aprsme, :insert_optimizer, :duration], - event_name: [:aprsme, :insert_optimizer, :duration], - measurement: :value, - description: "Average insert duration observed by the optimizer" - ), - counter( - [:aprsme, :insert_optimizer, :optimizations], - event_name: [:aprsme, :insert_optimizer, :optimizations], - measurement: :count, - description: "Number of times the optimizer adjusted batch parameters" - ) - ] - ) - end - defp api_metrics do Event.build( :aprsme_api_request_event_metrics, diff --git a/lib/aprsme/release.ex b/lib/aprsme/release.ex index 33932b9..e705c29 100644 --- a/lib/aprsme/release.ex +++ b/lib/aprsme/release.ex @@ -10,10 +10,6 @@ defmodule Aprsme.Release do @app :aprsme def migrate do - # Initialize deployment timestamp first - deployed_at = init() - Logger.info("Deployment timestamp: #{deployed_at}") - # Gettext translations are automatically compiled during Mix compilation # Skip database creation when using PgBouncer @@ -99,24 +95,6 @@ defmodule Aprsme.Release do # Store in application config Application.put_env(:aprsme, :deployed_at, deployed_at) - # Notify about deployment after a short delay to ensure PubSub is started - # In k8s, this will notify all connected clients about the new deployment - _ = - if System.get_env("DEPLOYED_AT") do - Task.start(fn -> - # Wait for application to start - Process.sleep(10_000) - - try do - _ = Aprsme.DeploymentNotifier.notify_deployment(deployed_at) - Logger.info("Deployment notification sent for timestamp: #{deployed_at}") - rescue - error -> - Logger.warning("Failed to send deployment notification: #{inspect(error)}") - end - end) - end - deployed_at end diff --git a/lib/aprsme/spatial_pubsub.ex b/lib/aprsme/spatial_pubsub.ex index 16f3e40..5503332 100644 --- a/lib/aprsme/spatial_pubsub.ex +++ b/lib/aprsme/spatial_pubsub.ex @@ -25,6 +25,19 @@ defmodule Aprsme.SpatialPubSub do end) end + @doc """ + Register a client for every positioned packet, without a viewport filter. + + This is intended for consumers which apply a non-geographic filter (for + example a callsign pattern) and avoids subscribing them to the legacy global + packet topic. + """ + def register_unbounded(client_id) do + with_server({:error, :not_running}, fn -> + GenServer.call(__MODULE__, {:register_unbounded, client_id}) + end) + end + @doc """ Update a client's viewport bounds. """ @@ -92,6 +105,8 @@ defmodule Aprsme.SpatialPubSub do clients: %{}, # grid_key => MapSet of client_ids spatial_index: %{}, + # Clients receiving every positioned packet + unbounded_clients: MapSet.new(), # Statistics stats: %{ total_broadcasts: 0, @@ -109,7 +124,7 @@ defmodule Aprsme.SpatialPubSub do if map_size(state.clients) >= @max_clients do {:reply, {:error, :client_limit_exceeded}, state} else - topic = "spatial:#{client_id}" + topic = subscriber_topic(pid) state = replace_existing_client(state, client_id) @@ -132,6 +147,27 @@ defmodule Aprsme.SpatialPubSub do end end + @impl true + def handle_call({:register_unbounded, client_id}, {pid, _}, state) do + if map_size(state.clients) >= @max_clients do + {:reply, {:error, :client_limit_exceeded}, state} + else + topic = subscriber_topic(pid) + state = replace_existing_client(state, client_id) + ref = Process.monitor(pid) + + client_info = %{bounds: :unbounded, topic: topic, pid: pid, monitor_ref: ref} + + new_state = + state + |> put_in([:clients, client_id], client_info) + |> update_in([:unbounded_clients], &MapSet.put(&1, client_id)) + |> update_in([:stats, :clients_count], &(&1 + 1)) + + {:reply, {:ok, topic}, new_state} + end + end + @impl true def handle_call({:update_viewport, client_id, bounds}, _from, state) do case Map.get(state.clients, client_id) do @@ -180,21 +216,25 @@ defmodule Aprsme.SpatialPubSub do case extract_location(packet) do {lat, lon} when is_number(lat) and is_number(lon) -> # Find all clients whose viewports contain this location - client_ids = find_clients_for_location(state, lat, lon) + client_ids = + state + |> find_clients_for_location(lat, lon) + |> MapSet.new() + |> MapSet.union(state.unbounded_clients) + |> MapSet.to_list() - # Optimize: Batch collect topics then spawn async task for broadcasts topics = client_ids |> Enum.map(fn client_id -> Map.get(state.clients, client_id) end) |> Enum.reject(&is_nil/1) |> Enum.map(& &1.topic) + |> Enum.uniq() - # Use dedicated broadcast task supervisor for better performance - _ = - Aprsme.BroadcastTaskSupervisor.broadcast_async( - topics, - {:spatial_packet, packet} - ) + # PubSub broadcast is local message dispatch and does not warrant a + # nested task per packet. Keeping it here also preserves packet order. + Enum.each(topics, fn topic -> + Phoenix.PubSub.broadcast(Aprsme.PubSub, topic, {:spatial_packet, packet}) + end) # Update statistics (immediately return control to GenServer) state = @@ -386,6 +426,14 @@ defmodule Aprsme.SpatialPubSub do nil -> state + %{bounds: :unbounded, monitor_ref: ref} -> + Process.demonitor(ref, [:flush]) + + state + |> update_in([:unbounded_clients], &MapSet.delete(&1, client_id)) + |> update_in([:clients], &Map.delete(&1, client_id)) + |> update_in([:stats, :clients_count], &max(0, &1 - 1)) + %{bounds: bounds, monitor_ref: ref} -> Process.demonitor(ref, [:flush]) @@ -477,4 +525,6 @@ defmodule Aprsme.SpatialPubSub do default end end + + defp subscriber_topic(pid), do: "spatial:subscriber:#{:erlang.phash2(pid)}" end diff --git a/lib/aprsme/streaming_packets_pubsub.ex b/lib/aprsme/streaming_packets_pubsub.ex deleted file mode 100644 index 6e66b58..0000000 --- a/lib/aprsme/streaming_packets_pubsub.ex +++ /dev/null @@ -1,221 +0,0 @@ -defmodule Aprsme.StreamingPacketsPubSub do - @moduledoc """ - Global PubSub system for streaming APRS packets to subscribers based on geographic bounds. - - This module provides efficient real-time packet distribution with geographic filtering, - allowing multiple GenServers to subscribe to packet streams filtered by lat/long bounds. - """ - - use GenServer - - @table_name :streaming_packets_subscribers - - # Client API - - @doc """ - Starts the StreamingPacketsPubSub GenServer. - """ - def start_link(opts \\ []) do - GenServer.start_link(__MODULE__, opts, name: __MODULE__) - end - - @doc """ - Subscribe to packets within the specified geographic bounds. - - ## Parameters - - pid: The process to receive packet notifications - - bounds: Map with :north, :south, :east, :west keys defining the geographic area - - ## Returns - - :ok - """ - def subscribe_to_bounds(pid, bounds) do - with_server({:error, :not_running}, fn -> - GenServer.call(__MODULE__, {:subscribe, pid, bounds}) - end) - end - - @doc """ - Unsubscribe from packet notifications. - """ - def unsubscribe(pid) do - with_server(:ok, fn -> - GenServer.call(__MODULE__, {:unsubscribe, pid}) - end) - end - - @doc """ - Broadcast a packet to all subscribers whose bounds contain the packet's location. - """ - def broadcast_packet(packet) do - with_server(:ok, fn -> - GenServer.cast(__MODULE__, {:broadcast, packet}) - end) - end - - @doc """ - List all active subscribers and their bounds. - """ - def list_subscribers do - with_server([], fn -> - GenServer.call(__MODULE__, :list_subscribers) - end) - end - - # Server Callbacks - - @impl true - def init(_opts) do - # Create ETS table for fast lookups - _ = :ets.new(@table_name, [:set, :protected, :named_table, read_concurrency: true]) - - # Monitor subscribers for cleanup - _ = Process.flag(:trap_exit, true) - - # pid => monitor_ref — track monitors to demonitor on unsubscribe/update - {:ok, %{monitors: %{}}} - end - - @impl true - def handle_call({:subscribe, pid, bounds}, _from, state) do - if valid_bounds?(bounds) do - state = demonitor_if_exists(state, pid) - ref = Process.monitor(pid) - :ets.insert(@table_name, {pid, bounds}) - {:reply, :ok, put_in(state.monitors[pid], ref)} - else - {:reply, {:error, :invalid_bounds}, state} - end - end - - @impl true - def handle_call({:unsubscribe, pid}, _from, state) do - state = demonitor_if_exists(state, pid) - :ets.delete(@table_name, pid) - {:reply, :ok, state} - end - - @impl true - def handle_call(:list_subscribers, _from, state) do - subscribers = :ets.tab2list(@table_name) - {:reply, subscribers, state} - end - - @impl true - def handle_cast({:broadcast, packet}, state) do - _ = maybe_broadcast_packet(packet) - - {:noreply, state} - end - - @impl true - def handle_info({:DOWN, _ref, :process, pid, _reason}, state) do - # Clean up subscriber when process dies - :ets.delete(@table_name, pid) - {:noreply, %{state | monitors: Map.delete(state.monitors, pid)}} - end - - # Private functions - - defp demonitor_if_exists(state, pid) do - case Map.pop(state.monitors, pid) do - {nil, _monitors} -> - state - - {ref, monitors} -> - Process.demonitor(ref, [:flush]) - %{state | monitors: monitors} - end - end - - defp valid_bounds?(%{north: n, south: s, east: e, west: w}) do - all_bounds_numeric?(n, s, e, w) and - n >= s and - valid_latitude_range?(n) and - valid_latitude_range?(s) and - valid_longitude_range?(e) and - valid_longitude_range?(w) - end - - defp valid_bounds?(_), do: false - - defp all_bounds_numeric?(n, s, e, w) do - is_number(n) and is_number(s) and is_number(e) and is_number(w) - end - - defp valid_latitude_range?(lat) do - lat >= -90 and lat <= 90 - end - - defp valid_longitude_range?(lon) do - lon >= -180 and lon <= 180 - end - - defp packet_in_bounds?(lat, lon, %{north: n, south: s, east: e, west: w}) do - lat_in_bounds = lat >= s and lat <= n - - # Handle longitude wrap-around at international date line - lon_in_bounds = - if w > e do - # Bounds cross the date line - lon >= w or lon <= e - else - # Normal bounds - lon >= w and lon <= e - end - - lat_in_bounds and lon_in_bounds - end - - defp send_to_matching_subscribers(subscribers, lat, lon, packet, _server_pid) do - # send/2 to a dead PID is a no-op in Erlang (does not crash). - # Process.monitor :DOWN messages handle cleanup, so no alive? check needed. - subscribers - |> Stream.filter(fn {_pid, bounds} -> packet_in_bounds?(lat, lon, bounds) end) - |> Enum.each(fn {pid, _bounds} -> - send(pid, {:streaming_packet, packet}) - end) - end - - defp test_env? do - Application.get_env(:aprsme, :env) == :test - end - - defp with_server(default, fun) do - if Process.whereis(__MODULE__) do - fun.() - else - default - end - end - - defp maybe_broadcast_packet(packet) do - case packet_coordinates(packet) do - {lat, lon} -> - subscribers = :ets.tab2list(@table_name) - dispatch_packet_to_subscribers(subscribers, lat, lon, packet) - - nil -> - :ok - end - end - - defp packet_coordinates(packet) do - lat = packet[:latitude] || packet[:lat] - lon = packet[:longitude] || packet[:lon] || packet[:lng] - - if lat && lon, do: {lat, lon} - end - - defp dispatch_packet_to_subscribers(subscribers, lat, lon, packet) do - if test_env?() do - send_to_matching_subscribers(subscribers, lat, lon, packet, self()) - else - server_pid = self() - - Aprsme.BroadcastTaskSupervisor.async_execute(fn -> - send_to_matching_subscribers(subscribers, lat, lon, packet, server_pid) - end) - end - end -end diff --git a/lib/aprsme/telemetry/database_metrics.ex b/lib/aprsme/telemetry/database_metrics.ex index 2ffecda..9dce810 100644 --- a/lib/aprsme/telemetry/database_metrics.ex +++ b/lib/aprsme/telemetry/database_metrics.ex @@ -6,88 +6,7 @@ defmodule Aprsme.Telemetry.DatabaseMetrics do def collect_db_pool_metrics do pool_size = Application.get_env(:aprsme, Aprsme.Repo)[:pool_size] || 10 - - case Process.whereis(Aprsme.Repo) do - nil -> report_pool_metrics_unavailable(pool_size) - repo_pid when is_pid(repo_pid) -> collect_from_repo(repo_pid, pool_size) - end - rescue - e -> - Logger.debug("Error collecting pool metrics: #{inspect(e)}") - report_pool_metrics_error() - end - - defp collect_from_repo(repo_pid, pool_size) do - children = Supervisor.which_children(repo_pid) - pool_info = find_pool_child(children) - collect_from_pool(pool_info, pool_size) - end - - defp find_pool_child(children) do - Enum.find(children, fn - {DBConnection.ConnectionPool, _, _, _} -> true - {DBConnection.Ownership, _, _, _} -> true - _ -> false - end) - end - - defp collect_from_pool({_, pool_pid, _, _}, pool_size) when is_pid(pool_pid) do - # Try to get pool telemetry - case try_get_pool_telemetry(pool_size) do - {:ok, _metrics} -> report_pool_metrics_estimated(pool_size) - _ -> report_pool_metrics_idle(pool_size) - end - end - - defp collect_from_pool(_, pool_size), do: report_pool_metrics_idle(pool_size) - - defp try_get_pool_telemetry(pool_size) do - {:ok, %{pool_size: pool_size, idle_time: 0, queue_time: 0}} - catch - _, _ -> {:error, :not_available} - end - - defp report_pool_metrics_unavailable(pool_size) do - emit_pool_metrics(%{ - size: pool_size, - idle: 0, - busy: 0, - available: 0, - queue_length: 0, - total: 0 - }) - end - - defp report_pool_metrics_idle(pool_size) do - emit_pool_metrics(%{ - size: pool_size, - idle: pool_size, - busy: 0, - available: pool_size, - queue_length: 0, - total: pool_size - }) - end - - defp report_pool_metrics_estimated(pool_size) do - # Cannot reliably introspect DBConnection pool state, so report - # pool_size as total without guessing busy/idle breakdown. - report_pool_metrics_idle(pool_size) - end - - defp report_pool_metrics_error do - emit_pool_metrics(%{ - size: 0, - idle: 0, - busy: 0, - available: 0, - queue_length: 0, - total: 0 - }) - end - - defp emit_pool_metrics(metrics) do - :telemetry.execute([:aprsme, :repo, :pool], metrics, %{}) + :telemetry.execute([:aprsme, :repo, :pool], %{size: pool_size}, %{}) end def collect_postgres_metrics do @@ -152,8 +71,21 @@ defmodule Aprsme.Telemetry.DatabaseMetrics do n_tup_ins as inserts, n_tup_upd as updates, n_tup_del as deletes - FROM pg_stat_user_tables - WHERE relname = 'packets' + FROM ( + SELECT + SUM(n_live_tup) AS n_live_tup, + SUM(n_dead_tup) AS n_dead_tup, + SUM(n_tup_ins) AS n_tup_ins, + SUM(n_tup_upd) AS n_tup_upd, + SUM(n_tup_del) AS n_tup_del + FROM pg_stat_user_tables + WHERE relid = 'packets'::regclass + OR relid IN ( + SELECT inhrelid + FROM pg_inherits + WHERE inhparent = 'packets'::regclass + ) + ) packet_stats """) do {:ok, %{rows: [[live, dead, ins, upd, del]]}} -> emit_packets_table_telemetry(live, dead, ins, upd, del) diff --git a/lib/aprsme/workers/packet_cleanup_worker.ex b/lib/aprsme/workers/packet_cleanup_worker.ex index 5e4b790..36c9974 100644 --- a/lib/aprsme/workers/packet_cleanup_worker.ex +++ b/lib/aprsme/workers/packet_cleanup_worker.ex @@ -2,47 +2,23 @@ defmodule Aprsme.Workers.PacketCleanupWorker do @moduledoc """ Worker for cleaning up old APRS packet data. - Delegates to `PartitionManager.drop_old_partitions/1` which drops entire - daily partitions past the retention period — instant, no dead tuples, no VACUUM. - - Also cleans up old `bad_packets` records via standard DELETE. + Cleans up old `bad_packets` records via standard DELETE. Packet partition + retention is owned exclusively by `Aprsme.PartitionManager`. """ import Ecto.Query alias Aprsme.BadPacket - alias Aprsme.PartitionManager alias Aprsme.Repo require Logger - @spec perform(map()) :: :ok | {:error, String.t()} - def perform(%{"cleanup_days" => days}) when is_integer(days) and days > 0 do - Logger.info("Starting scheduled APRS packet cleanup for packets older than #{days} days") - - {:ok, dropped} = PartitionManager.drop_old_partitions(days) - - Logger.info("APRS packet cleanup complete: dropped #{length(dropped)} partition(s) older than #{days} days") - - :ok - rescue - error -> - Logger.error("Packet cleanup failed: #{inspect(error)}\n#{inspect(__STACKTRACE__)}") - {:error, "Cleanup failed: #{inspect(error)}"} - end - @spec perform(map()) :: :ok | {:error, String.t()} def perform(_args) do - Logger.info("Starting scheduled APRS packet cleanup") - - retention_days = Application.get_env(:aprsme, :packet_retention_days, 7) - - {:ok, dropped} = PartitionManager.drop_old_partitions(retention_days) + Logger.info("Starting scheduled bad-packet cleanup") bad_packet_count = cleanup_old_bad_packets() - Logger.info( - "APRS packet cleanup complete: dropped #{length(dropped)} partition(s) and removed #{bad_packet_count} bad packets older than #{retention_days} days" - ) + Logger.info("Bad-packet cleanup complete: removed #{bad_packet_count} expired records") :ok rescue diff --git a/lib/aprsme_web/channels/mobile_channel.ex b/lib/aprsme_web/channels/mobile_channel.ex index 272b9ee..7a7576f 100644 --- a/lib/aprsme_web/channels/mobile_channel.ex +++ b/lib/aprsme_web/channels/mobile_channel.ex @@ -96,8 +96,8 @@ defmodule AprsmeWeb.MobileChannel do # Generate unique client ID client_id = "mobile_#{:erlang.phash2(self())}" - # Subscribe to StreamingPacketsPubSub with bounds - Aprsme.StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) + {:ok, topic} = Aprsme.SpatialPubSub.register_viewport(client_id, bounds) + :ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, topic) # Store client info in socket socket = @@ -155,7 +155,7 @@ defmodule AprsmeWeb.MobileChannel do Logger.info("Mobile websocket received unsubscribe: #{inspect(payload)}") if socket.assigns[:subscribed] && socket.assigns[:bounds] do - Aprsme.StreamingPacketsPubSub.unsubscribe(self()) + Aprsme.SpatialPubSub.unregister_client(socket.assigns.client_id) socket = socket @@ -236,12 +236,7 @@ defmodule AprsmeWeb.MobileChannel do end @impl true - def handle_info({:streaming_packet, packet}, socket) do - maybe_push_tracked_packet(socket, packet) - end - - @impl true - def handle_info({:postgres_packet, packet}, socket) do + def handle_info({:spatial_packet, packet}, socket) do maybe_push_tracked_packet(socket, packet) end @@ -309,11 +304,11 @@ defmodule AprsmeWeb.MobileChannel do @impl true def terminate(_reason, socket) do if socket.assigns[:subscribed] && socket.assigns[:bounds] do - Aprsme.StreamingPacketsPubSub.unsubscribe(self()) + Aprsme.SpatialPubSub.unregister_client(socket.assigns.client_id) end if callsign_subscription_active?(socket) do - Phoenix.PubSub.unsubscribe(Aprsme.PubSub, "postgres:aprsme_packets") + Aprsme.SpatialPubSub.unregister_client(socket.assigns.callsign_client_id) end :ok @@ -583,12 +578,9 @@ defmodule AprsmeWeb.MobileChannel do defp update_subscription_bounds(socket, bounds) do # Unsubscribe from old bounds if they exist if socket.assigns[:bounds] do - Aprsme.StreamingPacketsPubSub.unsubscribe(self()) + :ok = Aprsme.SpatialPubSub.update_viewport(socket.assigns.client_id, bounds) end - # Subscribe to new bounds - Aprsme.StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) - socket = assign(socket, :bounds, bounds) Logger.debug("Mobile client #{socket.assigns.client_id} updated bounds: #{inspect(bounds)}") @@ -600,15 +592,23 @@ defmodule AprsmeWeb.MobileChannel do if callsign_subscription_active?(socket) do socket else - Phoenix.PubSub.subscribe(Aprsme.PubSub, "postgres:aprsme_packets") - assign(socket, :callsign_subscription_active, true) + client_id = "mobile_callsign_#{:erlang.phash2(self())}" + {:ok, topic} = Aprsme.SpatialPubSub.register_unbounded(client_id) + :ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, topic) + + socket + |> assign(:callsign_client_id, client_id) + |> assign(:callsign_subscription_active, true) end end defp remove_callsign_subscription(socket) do if callsign_subscription_active?(socket) do - Phoenix.PubSub.unsubscribe(Aprsme.PubSub, "postgres:aprsme_packets") - assign(socket, :callsign_subscription_active, false) + Aprsme.SpatialPubSub.unregister_client(socket.assigns.callsign_client_id) + + socket + |> assign(:callsign_client_id, nil) + |> assign(:callsign_subscription_active, false) else socket end diff --git a/lib/aprsme_web/components/layouts/root.html.heex b/lib/aprsme_web/components/layouts/root.html.heex index cf8ebc9..de24dfe 100644 --- a/lib/aprsme_web/components/layouts/root.html.heex +++ b/lib/aprsme_web/components/layouts/root.html.heex @@ -43,36 +43,6 @@ - - - diff --git a/lib/aprsme_web/endpoint.ex b/lib/aprsme_web/endpoint.ex index 670055c..c439d04 100644 --- a/lib/aprsme_web/endpoint.ex +++ b/lib/aprsme_web/endpoint.ex @@ -11,6 +11,7 @@ defmodule AprsmeWeb.Endpoint do signing_salt: "0toQ/Ejk", encryption_salt: "local-dev-only-encryption-salt", secure: false, + http_only: true, same_site: "Lax" ] diff --git a/lib/aprsme_web/live/map_live/data_builder.ex b/lib/aprsme_web/live/map_live/data_builder.ex index 09febbd..acead62 100644 --- a/lib/aprsme_web/live/map_live/data_builder.ex +++ b/lib/aprsme_web/live/map_live/data_builder.ex @@ -108,21 +108,6 @@ defmodule AprsmeWeb.MapLive.DataBuilder do callsign = display_name(packet) label = map_label(packet) - # For historical (non-most-recent) packets, use a simple red dot HTML - # instead of the full APRS symbol — matches what the JS createMarkerIcon - # would generate anyway, saving the client from icon re-creation - symbol_html = - if is_most_recent do - AprsmeWeb.AprsSymbol.render_marker_html( - symbol_table_id, - symbol_code, - label, - 32 - ) - else - historical_dot_html(label) - end - raw_comment = get_packet_field(packet, :comment, "") clean_comment = Aprsme.EncodingUtils.sanitize_comment(raw_comment) @@ -134,7 +119,6 @@ defmodule AprsmeWeb.MapLive.DataBuilder do "callsign_group" => callsign, "symbol_table_id" => symbol_table_id, "symbol_code" => symbol_code, - "symbol_html" => symbol_html, "comment" => clean_comment, "timestamp" => get_packet_timestamp_unix(packet), "historical" => !is_most_recent, @@ -455,15 +439,6 @@ defmodule AprsmeWeb.MapLive.DataBuilder do "live_#{packet_info.callsign}_#{System.unique_integer([:positive])}" end - # Generate symbol HTML using the server-side renderer - symbol_html = - AprsmeWeb.AprsSymbol.render_marker_html( - packet_info.symbol_table_id, - packet_info.symbol_code, - packet_info.callsign, - 32 - ) - path_value = get_packet_field(packet, :path, "") %{ @@ -482,8 +457,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do "symbol_code" => packet_info.symbol_code, "symbol_description" => "Symbol: #{packet_info.symbol_table_id}#{packet_info.symbol_code}", "timestamp" => packet_info.timestamp, - "popup" => popup, - "symbol_html" => symbol_html + "popup" => popup } end @@ -686,12 +660,6 @@ defmodule AprsmeWeb.MapLive.DataBuilder do end end - defp historical_dot_html(callsign) do - escaped = callsign |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string() - - "
" - end - defp build_historical_packet_data(filtered_historical, has_weather) do filtered_historical |> Enum.map(fn packet -> build_minimal_packet_data(packet, false, has_weather) end) diff --git a/lib/aprsme_web/live/map_live/index.ex b/lib/aprsme_web/live/map_live/index.ex index 421a399..2809c84 100644 --- a/lib/aprsme_web/live/map_live/index.ex +++ b/lib/aprsme_web/live/map_live/index.ex @@ -61,9 +61,6 @@ defmodule AprsmeWeb.MapLive.Index do historical_hours: historical_hours ) - # Setup additional subscriptions if connected - socket = setup_additional_subscriptions(socket) - # Handle callsign tracking - check path params first, then query params tracked_callsign = case Map.get(params, "callsign") || Map.get(params, "call") do @@ -138,9 +135,6 @@ defmodule AprsmeWeb.MapLive.Index do # Subscribe to deployment events :ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, "deployment_events") - # Subscribe to StreamingPacketsPubSub with initial bounds - _ = Aprsme.StreamingPacketsPubSub.subscribe_to_bounds(self(), initial_bounds) - _ = Process.send_after(self(), :cleanup_old_packets, 60_000) socket @@ -151,17 +145,6 @@ defmodule AprsmeWeb.MapLive.Index do defp do_setup_subscriptions(socket, _initial_bounds, false), do: socket - defp setup_additional_subscriptions(socket) do - do_setup_additional_subscriptions(socket, connected?(socket)) - end - - defp do_setup_additional_subscriptions(socket, true) do - :ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, "postgres:aprsme_packets") - socket - end - - defp do_setup_additional_subscriptions(socket, false), do: socket - defp finalize_mount_assigns(socket, %{ initial_bounds: initial_bounds, final_map_center: final_map_center, @@ -812,18 +795,6 @@ defmodule AprsmeWeb.MapLive.Index do def handle_info(:reload_historical_packets, socket), do: handle_reload_historical_packets(socket) - def handle_info({:postgres_packet, packet}, socket) do - # Add packet to batcher instead of processing immediately - _ = - if socket.assigns[:batcher_pid] do - PacketBatcher.add_packet(socket.assigns.batcher_pid, packet) - else - handle_info_postgres_packet(packet, socket) - end - - {:noreply, socket} - end - def handle_info({:spatial_packet, packet}, socket) do # Add packet to batcher instead of processing immediately _ = @@ -836,16 +807,10 @@ defmodule AprsmeWeb.MapLive.Index do {:noreply, socket} end - def handle_info({:streaming_packet, packet}, socket) do - # Add packet to batcher instead of processing immediately - _ = - if socket.assigns[:batcher_pid] do - PacketBatcher.add_packet(socket.assigns.batcher_pid, packet) - else - handle_info_postgres_packet(packet, socket) - end - - {:noreply, socket} + # Retained for targeted callsign topics used by non-map LiveViews and for + # rolling cluster upgrades. MapLive no longer subscribes to the global topic. + def handle_info({:postgres_packet, packet}, socket) do + handle_info({:spatial_packet, packet}, socket) end def handle_info({:packet_batch, packets}, socket) do @@ -1830,12 +1795,6 @@ defmodule AprsmeWeb.MapLive.Index do Aprsme.SpatialPubSub.unregister_client(socket.assigns.spatial_client_id) end - # Unsubscribe from StreamingPacketsPubSub - _ = Aprsme.StreamingPacketsPubSub.unsubscribe(self()) - - # Unsubscribe from PubSub topic subscribed in mount - _ = Phoenix.PubSub.unsubscribe(Aprsme.PubSub, "postgres:aprsme_packets") - _ = if socket.assigns.buffer_timer, do: Process.cancel_timer(socket.assigns.buffer_timer) # Clean up any pending bounds update timer @@ -1997,8 +1956,6 @@ defmodule AprsmeWeb.MapLive.Index do if socket.assigns[:spatial_client_id] do Aprsme.SpatialPubSub.update_viewport(socket.assigns.spatial_client_id, map_bounds) end - - Aprsme.StreamingPacketsPubSub.subscribe_to_bounds(self(), map_bounds) end defp bounds_update_state(socket, map_bounds) do diff --git a/lib/aprsme_web/live/map_live/packet_batcher.ex b/lib/aprsme_web/live/map_live/packet_batcher.ex index e99a200..ab7a950 100644 --- a/lib/aprsme_web/live/map_live/packet_batcher.ex +++ b/lib/aprsme_web/live/map_live/packet_batcher.ex @@ -10,7 +10,7 @@ defmodule AprsmeWeb.MapLive.PacketBatcher do # milliseconds @batch_timeout 100 - defstruct [:parent_pid, :buffer, :timer_ref, :buffer_size] + defstruct [:parent_pid, :buffer, :timer_ref, :buffer_size, latest_by_sender: %{}] @type t :: %__MODULE__{ parent_pid: pid(), @@ -58,21 +58,22 @@ defmodule AprsmeWeb.MapLive.PacketBatcher do @impl true def handle_cast({:add_packet, packet}, state) do - new_buffer = [packet | state.buffer] - new_size = state.buffer_size + 1 + case accept_packet(state, packet) do + {:reject, state} -> + {:noreply, state} - # Cancel existing timer if any - state = cancel_timer(state) + {:accept, state} -> + new_buffer = [packet | state.buffer] + new_size = state.buffer_size + 1 + state = cancel_timer(state) - # Check if we should process immediately or wait - if new_size >= @batch_size do - # Process immediately - process_batch(new_buffer, state.parent_pid) - {:noreply, %{state | buffer: [], buffer_size: 0, timer_ref: nil}} - else - # Set timer for batch timeout - timer_ref = Process.send_after(self(), :batch_timeout, @batch_timeout) - {:noreply, %{state | buffer: new_buffer, buffer_size: new_size, timer_ref: timer_ref}} + if new_size >= @batch_size do + process_batch(new_buffer, state.parent_pid) + {:noreply, %{state | buffer: [], buffer_size: 0, timer_ref: nil}} + else + timer_ref = Process.send_after(self(), :batch_timeout, @batch_timeout) + {:noreply, %{state | buffer: new_buffer, buffer_size: new_size, timer_ref: timer_ref}} + end end end @@ -118,4 +119,27 @@ defmodule AprsmeWeb.MapLive.PacketBatcher do # Send batch to parent LiveView send(parent_pid, {:packet_batch, packets}) end + + defp accept_packet(state, packet) do + sender = Map.get(packet, :sender) || Map.get(packet, "sender") + received_at = Map.get(packet, :received_at) || Map.get(packet, "received_at") + + with true <- is_binary(sender), + false <- is_nil(received_at), + latest = Map.get(state.latest_by_sender, sender), + true <- newer_timestamp?(received_at, latest) do + {:accept, %{state | latest_by_sender: Map.put(state.latest_by_sender, sender, received_at)}} + else + false when is_binary(sender) and not is_nil(received_at) -> {:reject, state} + _other -> {:accept, state} + end + end + + defp newer_timestamp?(_incoming, nil), do: true + defp newer_timestamp?(%DateTime{} = incoming, %DateTime{} = latest), do: DateTime.after?(incoming, latest) + + defp newer_timestamp?(%NaiveDateTime{} = incoming, %NaiveDateTime{} = latest), + do: NaiveDateTime.after?(incoming, latest) + + defp newer_timestamp?(_incoming, _latest), do: true end diff --git a/lib/aprsme_web/live/status_live/index.ex b/lib/aprsme_web/live/status_live/index.ex index 6c330aa..d7422e3 100644 --- a/lib/aprsme_web/live/status_live/index.ex +++ b/lib/aprsme_web/live/status_live/index.ex @@ -42,7 +42,7 @@ defmodule AprsmeWeb.StatusLive.Index do self_pid = self() _ = - Task.Supervisor.start_child(Aprsme.BroadcastTaskSupervisor, fn -> + Aprsme.BroadcastTaskSupervisor.async_execute(fn -> try do status = get_aprs_status() send(self_pid, {:status_updated, status}) diff --git a/lib/aprsme_web/router.ex b/lib/aprsme_web/router.ex index d37c09c..193fcf6 100644 --- a/lib/aprsme_web/router.ex +++ b/lib/aprsme_web/router.ex @@ -48,11 +48,20 @@ defmodule AprsmeWeb.Router do end scope "/", AprsmeWeb do - pipe_through [:browser, :require_authenticated_user] + pipe_through [:browser, :require_authenticated_user, :require_admin_user] live_dashboard "/dashboard", metrics: AprsmeWeb.Telemetry error_tracker_dashboard("/errors") end + scope "/", AprsmeWeb do + pipe_through [:browser, :require_authenticated_user, :require_admin_user] + + live_session :require_admin_user, + on_mount: [{AprsmeWeb.UserAuth, :ensure_admin}, {AprsmeWeb.LocaleHook, :set_locale}] do + live "/badpackets", BadPacketsLive.Index, :index + end + end + scope "/", AprsmeWeb do pipe_through [:browser] @@ -120,7 +129,6 @@ defmodule AprsmeWeb.Router do live "/status", StatusLive.Index, :index live "/packets", PacketsLive.Index, :index live "/packets/:callsign", PacketsLive.CallsignView, :index - live "/badpackets", BadPacketsLive.Index, :index live "/weather/:callsign", WeatherLive.CallsignView, :index live "/about", AboutLive, :index live "/api", ApiDocsLive, :index diff --git a/lib/aprsme_web/user_auth.ex b/lib/aprsme_web/user_auth.ex index acb9a80..8ff9715 100644 --- a/lib/aprsme_web/user_auth.ex +++ b/lib/aprsme_web/user_auth.ex @@ -14,7 +14,13 @@ defmodule AprsmeWeb.UserAuth do @max_age 60 * 60 * 24 * 60 @remember_me_cookie "_aprs_web_user_remember_me" - @remember_me_options [sign: true, max_age: @max_age, same_site: "Lax"] + @remember_me_options [ + sign: true, + max_age: @max_age, + same_site: "Lax", + secure: true, + http_only: true + ] @doc """ Logs the user in. @@ -176,6 +182,23 @@ defmodule AprsmeWeb.UserAuth do end end + def on_mount(:ensure_admin, _params, session, socket) do + socket = mount_current_user(session, socket) + + case socket.assigns.current_user do + %{role: :admin} -> + {:cont, socket} + + _ -> + socket = + socket + |> Phoenix.LiveView.put_flash(:error, "Administrator access is required.") + |> Phoenix.LiveView.redirect(to: ~p"/") + + {:halt, socket} + end + end + def on_mount(:redirect_if_user_is_authenticated, _params, session, socket) do socket = mount_current_user(session, socket) @@ -235,6 +258,27 @@ defmodule AprsmeWeb.UserAuth do end end + @doc "Requires an authenticated administrator." + @spec require_admin_user(Plug.Conn.t(), any()) :: Plug.Conn.t() + def require_admin_user(conn, _opts) do + case conn.assigns[:current_user] do + %{role: :admin} -> + conn + + nil -> + conn + |> put_flash(:error, "You must log in as an administrator to access this page.") + |> maybe_store_return_to() + |> redirect(to: ~p"/users/log_in") + |> halt() + + _user -> + conn + |> send_resp(:forbidden, "Forbidden") + |> halt() + end + end + @spec put_token_in_session(Plug.Conn.t(), String.t()) :: Plug.Conn.t() defp put_token_in_session(conn, token) do conn diff --git a/lib/mix/tasks/aprsme.user_role.ex b/lib/mix/tasks/aprsme.user_role.ex new file mode 100644 index 0000000..19ad7ed --- /dev/null +++ b/lib/mix/tasks/aprsme.user_role.ex @@ -0,0 +1,31 @@ +defmodule Mix.Tasks.Aprsme.UserRole do + @shortdoc "Promotes or demotes an existing user" + @moduledoc """ + Sets an existing user's role by email. + + mix aprsme.user_role user@example.com admin + mix aprsme.user_role user@example.com user + """ + + use Mix.Task + + @impl Mix.Task + def run([email, role_name]) when role_name in ["user", "admin"] do + Mix.Task.run("app.start") + + case Aprsme.Accounts.get_user_by_email(email) do + nil -> + Mix.raise("No user found with email #{email}") + + user -> + role = String.to_existing_atom(role_name) + + case Aprsme.Accounts.set_user_role(user, role) do + {:ok, _user} -> Mix.shell().info("Set #{email} role to #{role_name}") + {:error, changeset} -> Mix.raise("Could not update role: #{inspect(changeset.errors)}") + end + end + end + + def run(_args), do: Mix.raise("Usage: mix aprsme.user_role EMAIL user|admin") +end diff --git a/mix.exs b/mix.exs index 63c4670..dcca99d 100644 --- a/mix.exs +++ b/mix.exs @@ -106,6 +106,7 @@ defmodule Aprsme.MixProject do {:bandit, "~> 1.5"}, {:req, "~> 0.5"}, {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, + {:sobelow, "~> 0.14", only: [:dev, :test], runtime: false}, {:dialyxir, "~> 1.0", only: :dev, runtime: false}, {:lazy_html, ">= 0.1.0", only: :test}, {:mix_test_watch, "~> 1.1", only: [:dev, :test]}, @@ -126,21 +127,19 @@ defmodule Aprsme.MixProject do defp aliases do [ setup: ["deps.get", "ecto.setup"], - compile: ["compile"], "ecto.setup": ["ecto.create", "ecto.migrate", "run priv/repo/seeds.exs"], "ecto.reset": ["ecto.drop", "ecto.setup"], test: ["ecto.create --quiet", "ecto.migrate --quiet", "test"], "test.cover": ["cmd MIX_TEST_COVERAGE=1 mix test --cover"], "assets.deploy": [ "tailwind default --minify", - "esbuild vendor", "esbuild vendor_css", "esbuild core_js", "esbuild map_js", "esbuild chart_js", - "esbuild date_adapter", "esbuild default --minify", - "phx.digest" + "phx.digest", + "phx.digest.clean --all" ] ] end diff --git a/mix.lock b/mix.lock index 9c69450..73b135c 100644 --- a/mix.lock +++ b/mix.lock @@ -1,6 +1,6 @@ %{ "aprs": {:git, "https://github.com/aprsme/aprs.git", "c2a544ba31bc1eced480f3321256d74bd15de62a", [branch: "main"]}, - "bandit": {:hex, :bandit, "1.12.0", "6c5214daa2469644ac4ab0113b98abc24f75e348378e6a974c6343b3e5da22ef", [:mix], [{:hpax, "~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}, {:plug, "~> 1.18", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:thousand_island, "~> 1.5", [hex: :thousand_island, repo: "hexpm", optional: false]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "45dac82dc86f45cf4a196dee9cc5a8b791d9c9469d996055f055e6ee36c66e20"}, + "bandit": {:hex, :bandit, "1.12.3", "23f49ae03d86365b0caff3b3a1eba5d981066b1ecafe2b19c0bef5ad36c211ce", [:mix], [{:hpax, "~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}, {:plug, "~> 1.18", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:thousand_island, "~> 1.5", [hex: :thousand_island, repo: "hexpm", optional: false]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "a253ec03f391755b2126e4181ee2fee05c75b712b407aa399de1831b5088c58e"}, "bcrypt_elixir": {:hex, :bcrypt_elixir, "3.3.2", "d50091e3c9492d73e17fc1e1619a9b09d6a5ef99160eb4d736926fd475a16ca3", [:make, :mix], [{:comeonin, "~> 5.3", [hex: :comeonin, repo: "hexpm", optional: false]}, {:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "471be5151874ae7931911057d1467d908955f93554f7a6cd1b7d804cac8cef53"}, "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"}, "castore": {:hex, :castore, "1.0.20", "455e48f7115eca98c9f2b0e7a152b5a2e8f2a8a4f964c96e95bd31645ee5fa59", [:mix], [], "hexpm", "940eafbfd8b14bee649f083bc11b3b54ec555b54c3e4ea8213351ff6fee39c10"}, @@ -50,10 +50,11 @@ "phoenix_pubsub": {:hex, :phoenix_pubsub, "2.2.0", "ff3a5616e1bed6804de7773b92cbccfc0b0f473faf1f63d7daf1206c7aeaaa6f", [:mix], [], "hexpm", "adc313a5bf7136039f63cfd9668fde73bba0765e0614cba80c06ac9460ff3e96"}, "phoenix_template": {:hex, :phoenix_template, "1.0.4", "e2092c132f3b5e5b2d49c96695342eb36d0ed514c5b252a77048d5969330d639", [:mix], [{:phoenix_html, "~> 2.14.2 or ~> 3.0 or ~> 4.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}], "hexpm", "2c0c81f0e5c6753faf5cca2f229c9709919aba34fab866d3bc05060c9c444206"}, "plug": {:hex, :plug, "1.20.3", "56c480c633ec2ce10140e236e15233bf576e1d323887d7c96711bd02ab5160db", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "be266aee1b8536ef6409d58cf39a3121319f0ec47cfa1b24024485aa0e76ad76"}, - "plug_crypto": {:hex, :plug_crypto, "2.1.1", "19bda8184399cb24afa10be734f84a16ea0a2bc65054e23a62bb10f06bc89491", [:mix], [], "hexpm", "6470bce6ffe41c8bd497612ffde1a7e4af67f36a15eea5f921af71cf3e11247c"}, + "plug_crypto": {:hex, :plug_crypto, "2.2.0", "144014737daaf485407f5ed77daeaad74d651b216a28c87543f8cc7043f8efc8", [:mix], [], "hexpm", "83a95744ab1c75876542b6fab135fcc176280e0f301a111c1f757fddcec95d2c"}, "postgrex": {:hex, :postgrex, "0.22.3", "bf65941737ee7a9adbe4a64c91080310d11703da343e8ac9188aacb9eb9f6f02", [:mix], [{:db_connection, "~> 2.9", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "f018c13752b2b46e8d35d7e2d84c3276557cbfd880769109021a1d0ee36c1cfe"}, "prom_ex": {:hex, :prom_ex, "1.12.0", "a82cbd5b49964e4d4295ab72a18e007aadd5f4a91630b7c49fad42cc7e49c880", [:mix], [{:absinthe, ">= 1.8.0", [hex: :absinthe, repo: "hexpm", optional: true]}, {:broadway, ">= 1.1.0", [hex: :broadway, repo: "hexpm", optional: true]}, {:ecto, ">= 3.14.0", [hex: :ecto, repo: "hexpm", optional: true]}, {:finch, "~> 0.18", [hex: :finch, repo: "hexpm", optional: false]}, {:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:oban, ">= 2.10.0", [hex: :oban, repo: "hexpm", optional: true]}, {:octo_fetch, "~> 0.4", [hex: :octo_fetch, repo: "hexpm", optional: false]}, {:peep, "~> 3.0 or ~> 4.0", [hex: :peep, repo: "hexpm", optional: false]}, {:phoenix, ">= 1.7.0", [hex: :phoenix, repo: "hexpm", optional: true]}, {:phoenix_live_view, ">= 0.20.0", [hex: :phoenix_live_view, repo: "hexpm", optional: true]}, {:plug, ">= 1.16.0", [hex: :plug, repo: "hexpm", optional: true]}, {:plug_cowboy, ">= 2.6.0", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:telemetry, ">= 1.0.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:telemetry_metrics, "~> 1.0", [hex: :telemetry_metrics, repo: "hexpm", optional: false]}, {:telemetry_metrics_prometheus_core, "~> 1.2", [hex: :telemetry_metrics_prometheus_core, repo: "hexpm", optional: false]}, {:telemetry_poller, "~> 1.1", [hex: :telemetry_poller, repo: "hexpm", optional: false]}], "hexpm", "6357484941489ba2fee64bb1e9f2a1e86309545304f9e90144e3c10e553c958b"}, "req": {:hex, :req, "0.6.3", "7fe5e68792ff0546e45d5919104fa1764a13694cfe3e48c8a0f32ad051ae77e4", [:mix], [{:brotli, "~> 0.3.1", [hex: :brotli, repo: "hexpm", optional: true]}, {:ezstd, "~> 1.0", [hex: :ezstd, repo: "hexpm", optional: true]}, {:finch, "~> 0.21", [hex: :finch, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mime, "~> 2.0.6 or ~> 2.1", [hex: :mime, repo: "hexpm", optional: false]}, {:nimble_csv, "~> 1.0", [hex: :nimble_csv, repo: "hexpm", optional: true]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "e85b5c6c990e6c3f52bbba68e6f099118f2b8252825f96c7c3636b97a3de307d"}, + "sobelow": {:hex, :sobelow, "0.14.1", "2f81e8632f15574cba2402bcddff5497b413c01e6f094bc0ab94e83c2f74db81", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "8fac9a2bd90fdc4b15d6fca6e1608efb7f7c600fa75800813b794ee9364c87f2"}, "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.7", "354c321cf377240c7b8716899e182ce4890c5938111a1296add3ec74cf1715df", [:make, :mix, :rebar3], [], "hexpm", "fe4c190e8f37401d30167c8c405eda19469f34577987c76dde613e838bbc67f8"}, "stream_data": {:hex, :stream_data, "1.4.0", "026f929db613aabea6208012ae9b8970d3fd5f88b3bdf26831bc536f98c42036", [:mix], [], "hexpm", "2b0ee3a340dcce1c8cf6302a763ee757d1e01c54d6e16d9069062509d68b1dc9"}, "styler": {:hex, :styler, "1.11.0", "35010d970689a23c2bcc8e97bd8bf7d20e3561d60c49be84654df5c37d051a9c", [:mix], [], "hexpm", "70f36165d0cf238a32b7a456fdef6a9c72e77e657d7ac4a0ace33aeba3f2b8c0"}, diff --git a/priv/repo/migrations/20260726000000_add_role_to_users.exs b/priv/repo/migrations/20260726000000_add_role_to_users.exs new file mode 100644 index 0000000..9c3f61c --- /dev/null +++ b/priv/repo/migrations/20260726000000_add_role_to_users.exs @@ -0,0 +1,11 @@ +defmodule Aprsme.Repo.Migrations.AddRoleToUsers do + use Ecto.Migration + + def change do + alter table(:users) do + add :role, :string, null: false, default: "user" + end + + create constraint(:users, :users_role_must_be_valid, check: "role IN ('user', 'admin')") + end +end diff --git a/priv/repo/migrations/20260726000001_restore_packet_geometry_index.exs b/priv/repo/migrations/20260726000001_restore_packet_geometry_index.exs new file mode 100644 index 0000000..93bbc1f --- /dev/null +++ b/priv/repo/migrations/20260726000001_restore_packet_geometry_index.exs @@ -0,0 +1,51 @@ +defmodule Aprsme.Repo.Migrations.RestorePacketGeometryIndex do + use Ecto.Migration + + @disable_ddl_transaction true + @disable_migration_lock true + + @parent_index "idx_packets_location" + + def up do + if Aprsme.Repo.config()[:pool] == Ecto.Adapters.SQL.Sandbox do + execute(""" + CREATE INDEX IF NOT EXISTS #{@parent_index} + ON packets USING gist (location) + WHERE has_position = true + """) + else + execute(""" + CREATE INDEX IF NOT EXISTS #{@parent_index} + ON ONLY packets USING gist (location) + WHERE has_position = true + """) + + Aprsme.Repo.query!(""" + SELECT inhrelid::regclass::text + FROM pg_inherits + WHERE inhparent = 'packets'::regclass + """).rows + |> List.flatten() + |> Enum.each(&build_and_attach/1) + end + end + + def down do + execute("DROP INDEX IF EXISTS #{@parent_index}") + end + + defp build_and_attach(partition) do + index_name = + partition + |> String.replace(~r/[^a-zA-Z0-9_]/, "_") + |> Kernel.<>("_location_gist_idx") + + execute(""" + CREATE INDEX CONCURRENTLY IF NOT EXISTS #{index_name} + ON #{partition} USING gist (location) + WHERE has_position = true + """) + + execute("ALTER INDEX #{@parent_index} ATTACH PARTITION #{index_name}") + end +end diff --git a/priv/repo/migrations/20260726000002_replace_packet_count_sequence.exs b/priv/repo/migrations/20260726000002_replace_packet_count_sequence.exs new file mode 100644 index 0000000..0929eaf --- /dev/null +++ b/priv/repo/migrations/20260726000002_replace_packet_count_sequence.exs @@ -0,0 +1,147 @@ +defmodule Aprsme.Repo.Migrations.ReplacePacketCountSequence do + use Ecto.Migration + + def up do + execute("DROP TRIGGER IF EXISTS packet_insert_sequence ON packets") + execute("DROP TRIGGER IF EXISTS packet_delete_sequence ON packets") + + execute(""" + INSERT INTO packet_counters (counter_type, count, inserted_at, updated_at) + VALUES ('total_packets', 0, NOW(), NOW()) + ON CONFLICT (counter_type) DO NOTHING + """) + + execute(""" + UPDATE packet_counters + SET count = (SELECT COUNT(*) FROM packets), + updated_at = NOW() + WHERE counter_type = 'total_packets' + """) + + execute(""" + CREATE OR REPLACE FUNCTION increment_packet_counter() + RETURNS TRIGGER AS $$ + BEGIN + UPDATE packet_counters + SET count = count + 1, + updated_at = NOW() + WHERE counter_type = 'total_packets'; + RETURN NEW; + END; + $$ LANGUAGE plpgsql + """) + + execute(""" + CREATE OR REPLACE FUNCTION decrement_packet_counter() + RETURNS TRIGGER AS $$ + BEGIN + UPDATE packet_counters + SET count = GREATEST(0, count - 1), + updated_at = NOW() + WHERE counter_type = 'total_packets'; + RETURN OLD; + END; + $$ LANGUAGE plpgsql + """) + + execute(""" + CREATE TRIGGER packet_insert_counter + AFTER INSERT ON packets + FOR EACH ROW + EXECUTE FUNCTION increment_packet_counter() + """) + + execute(""" + CREATE TRIGGER packet_delete_counter + AFTER DELETE ON packets + FOR EACH ROW + EXECUTE FUNCTION decrement_packet_counter() + """) + + execute(""" + CREATE OR REPLACE FUNCTION get_packet_count() + RETURNS BIGINT AS $$ + BEGIN + RETURN COALESCE( + (SELECT count FROM packet_counters WHERE counter_type = 'total_packets'), + 0 + ); + END; + $$ LANGUAGE plpgsql STABLE + """) + + execute("DROP FUNCTION IF EXISTS increment_packet_sequence()") + execute("DROP FUNCTION IF EXISTS decrement_packet_sequence()") + execute("DROP SEQUENCE IF EXISTS packet_count_seq") + + execute(""" + COMMENT ON TABLE packet_counters IS + 'Exact retained packet count. Updated atomically by packet DML and partition maintenance.' + """) + end + + def down do + execute("DROP TRIGGER IF EXISTS packet_insert_counter ON packets") + execute("DROP TRIGGER IF EXISTS packet_delete_counter ON packets") + + execute(""" + CREATE SEQUENCE packet_count_seq + MINVALUE 0 + START WITH 0 + """) + + execute(""" + SELECT setval( + 'packet_count_seq', + COALESCE((SELECT count FROM packet_counters WHERE counter_type = 'total_packets'), 0), + true + ) + """) + + execute(""" + CREATE OR REPLACE FUNCTION increment_packet_sequence() + RETURNS TRIGGER AS $$ + BEGIN + PERFORM nextval('packet_count_seq'); + RETURN NEW; + END; + $$ LANGUAGE plpgsql + """) + + execute(""" + CREATE OR REPLACE FUNCTION decrement_packet_sequence() + RETURNS TRIGGER AS $$ + DECLARE + current_val BIGINT; + BEGIN + SELECT last_value INTO current_val FROM packet_count_seq; + PERFORM setval('packet_count_seq', GREATEST(0, current_val - 1), true); + RETURN OLD; + END; + $$ LANGUAGE plpgsql + """) + + execute(""" + CREATE TRIGGER packet_insert_sequence + AFTER INSERT ON packets + FOR EACH ROW + EXECUTE FUNCTION increment_packet_sequence() + """) + + execute(""" + CREATE TRIGGER packet_delete_sequence + AFTER DELETE ON packets + FOR EACH ROW + EXECUTE FUNCTION decrement_packet_sequence() + """) + + execute(""" + CREATE OR REPLACE FUNCTION get_packet_count() + RETURNS BIGINT AS $$ + BEGIN + RETURN last_value FROM packet_count_seq; + END; + $$ LANGUAGE plpgsql STABLE + """) + end +end diff --git a/test/aprsme/callsign_test.exs b/test/aprsme/callsign_test.exs index e5d39c1..0f8579b 100644 --- a/test/aprsme/callsign_test.exs +++ b/test/aprsme/callsign_test.exs @@ -18,22 +18,21 @@ defmodule Aprsme.CallsignTest do assert Callsign.valid?("N0CALL-9") end - test "validates callsigns with hyphens in base callsign like VE-KTKI" do + test "accepts transport-safe APRS-IS identifiers" do assert Callsign.valid?("VE-KTKI") assert Callsign.valid?("VE-KTKI-1") assert Callsign.valid?("VE-TEST") end - test "rejects only empty callsigns" do + test "rejects malformed callsigns" do refute Callsign.valid?("") refute Callsign.valid?(" ") - # Now accepts any non-empty string assert Callsign.valid?("123") - assert Callsign.valid?("-ABC") - assert Callsign.valid?("ABC-") + refute Callsign.valid?("-ABC") + refute Callsign.valid?("ABC-") assert Callsign.valid?("ABC-123") - assert Callsign.valid?("ABC--1") + refute Callsign.valid?("ABC--1") end test "handles nil input" do diff --git a/test/aprsme/cleanup_scheduler_test.exs b/test/aprsme/cleanup_scheduler_test.exs index e01fade..218e30d 100644 --- a/test/aprsme/cleanup_scheduler_test.exs +++ b/test/aprsme/cleanup_scheduler_test.exs @@ -50,13 +50,6 @@ defmodule Aprsme.CleanupSchedulerTest do end test "runs cleanup and reschedules when interval is an integer" do - # Ensure BroadcastTaskSupervisor is up — it wraps Task.Supervisor. - _ = - case Process.whereis(Aprsme.BroadcastTaskSupervisor) do - nil -> start_supervised({Task.Supervisor, name: Aprsme.BroadcastTaskSupervisor}) - _ -> :ok - end - state = %{interval: 100} assert {:noreply, ^state} = CleanupScheduler.handle_info(:schedule_cleanup, state) diff --git a/test/aprsme/cluster/packet_distributor_test.exs b/test/aprsme/cluster/packet_distributor_test.exs index 18b1ca0..58e25c0 100644 --- a/test/aprsme/cluster/packet_distributor_test.exs +++ b/test/aprsme/cluster/packet_distributor_test.exs @@ -79,7 +79,7 @@ defmodule Aprsme.Cluster.PacketDistributorTest do describe "handle_distributed_packet/1" do test "processes a distributed packet without crashing" do - # StreamingPacketsPubSub and PacketStore are already running in test + # SpatialPubSub is already running in test result = PacketDistributor.handle_distributed_packet({:distributed_packet, @test_packet}) # Broadcasts to local clients and stores in PacketStore diff --git a/test/aprsme/cluster/packet_receiver_test.exs b/test/aprsme/cluster/packet_receiver_test.exs deleted file mode 100644 index 4d1a071..0000000 --- a/test/aprsme/cluster/packet_receiver_test.exs +++ /dev/null @@ -1,103 +0,0 @@ -defmodule Aprsme.Cluster.PacketReceiverTest do - use ExUnit.Case, async: false - - alias Aprsme.Cluster.LeaderElection - alias Aprsme.Cluster.PacketReceiver - - @election_key {:aprs_is_leader, LeaderElection} - - setup do - # Clean up any existing global registrations - :global.unregister_name(@election_key) - - # Ensure clustering is disabled so LeaderElection becomes leader - Application.put_env(:aprsme, :cluster_enabled, false) - - # Stop any existing PacketReceiver - if Process.whereis(PacketReceiver) do - GenServer.stop(PacketReceiver) - end - - # Stop any existing LeaderElection - if Process.whereis(LeaderElection) do - GenServer.stop(LeaderElection) - end - - # Start LeaderElection and sync so election (0ms in test config) has run. - {:ok, leader_pid} = LeaderElection.start_link([]) - LeaderElection.leader?() - - on_exit(fn -> - try do - if Process.whereis(PacketReceiver), do: GenServer.stop(PacketReceiver, :normal, 100) - catch - :exit, _ -> :ok - end - - try do - if Process.whereis(LeaderElection), do: GenServer.stop(LeaderElection, :normal, 100) - catch - :exit, _ -> :ok - end - - :global.unregister_name(@election_key) - Application.put_env(:aprsme, :cluster_enabled, false) - end) - - {:ok, leader_pid: leader_pid} - end - - describe "start_link/1" do - test "starts and registers the process" do - assert {:ok, pid} = PacketReceiver.start_link([]) - assert Process.alive?(pid) - assert Process.whereis(PacketReceiver) == pid - end - end - - describe "handle_info/2 when leader" do - test "does not forward distributed packets when node is leader" do - {:ok, pid} = PacketReceiver.start_link([]) - - # Confirm we are the leader - assert LeaderElection.leader?() == true - - packet = %{raw: "TEST>APRS:test", sender: "TEST"} - send(pid, {:distributed_packet, packet}) - :sys.get_state(pid) - - # Process should still be alive (no crash) - assert Process.alive?(pid) - end - end - - describe "handle_info/2 when not leader" do - test "forwards distributed packets when node is not leader" do - {:ok, pid} = PacketReceiver.start_link([]) - - # Force non-leader state - :sys.replace_state(LeaderElection, fn state -> %{state | is_leader: false} end) - - assert LeaderElection.leader?() == false - - packet = %{raw: "TEST>APRS:test", sender: "TEST"} - send(pid, {:distributed_packet, packet}) - :sys.get_state(pid) - - # Process should still be alive (no crash) - assert Process.alive?(pid) - end - end - - describe "handle_info/2 with unknown messages" do - test "ignores unknown messages without crashing" do - {:ok, pid} = PacketReceiver.start_link([]) - - send(pid, :some_unknown_message) - send(pid, {:unexpected, "data"}) - :sys.get_state(pid) - - assert Process.alive?(pid) - end - end -end diff --git a/test/aprsme/db_optimizer_test.exs b/test/aprsme/db_optimizer_test.exs deleted file mode 100644 index 57a0e9a..0000000 --- a/test/aprsme/db_optimizer_test.exs +++ /dev/null @@ -1,196 +0,0 @@ -defmodule Aprsme.DbOptimizerTest do - use Aprsme.DataCase, async: false - - alias Aprsme.DbOptimizer - - describe "calculate_optimal_batch_size/1" do - test "returns max cap (2000) for small maps" do - entries = [%{name: "a", value: 1}, %{name: "b", value: 2}] - - assert DbOptimizer.calculate_optimal_batch_size(entries) == 2000 - end - - test "uses default 1024 size when first entry is nil" do - # With nil as first entry, estimate_entry_size returns 1024 - # available_memory = 14 * 1024 * 1024 = 14_680_064 - # max_batch = div(14_680_064, 1024) = 14_336 - # capped at min(14_336, 2000) = 2000 - entries = [nil, %{name: "a"}] - - assert DbOptimizer.calculate_optimal_batch_size(entries) == 2000 - end - - test "returns 2000 for single-element list with small map" do - entries = [%{x: 1}] - - assert DbOptimizer.calculate_optimal_batch_size(entries) == 2000 - end - - test "returns minimum of 100 for extremely large entries" do - # Create a map with very large values to push the estimated size way up - # We need estimated_size > 14 * 1024 * 1024 / 100 = 146_800 - # Each binary value contributes its byte_size + 100 overhead - big_value = String.duplicate("x", 200_000) - entries = [%{data: big_value}] - - assert DbOptimizer.calculate_optimal_batch_size(entries) == 100 - end - end - - describe "optimized_batch_insert/3" do - test "inserts valid entries and returns {count, 0}" do - now = NaiveDateTime.truncate(NaiveDateTime.utc_now(), :second) - - entries = [ - %{ - "counter_type" => "test_optimized_1", - "count" => 100, - "inserted_at" => now, - "updated_at" => now - }, - %{ - "counter_type" => "test_optimized_2", - "count" => 200, - "inserted_at" => now, - "updated_at" => now - } - ] - - assert {2, 0} = DbOptimizer.optimized_batch_insert("packet_counters", entries) - end - - test "returns {0, 0} for empty list" do - assert {0, 0} = DbOptimizer.optimized_batch_insert("packet_counters", []) - end - end - - describe "analyze_table/1" do - test "returns :ok for packets table" do - assert :ok = DbOptimizer.analyze_table("packets") - end - end - - describe "vacuum_table/1" do - test "returns :error inside sandbox (VACUUM cannot run in transactions)" do - assert :error = DbOptimizer.vacuum_table("packets") - end - end - - describe "get_connection_stats/0" do - test "returns map with expected keys and integer values" do - stats = DbOptimizer.get_connection_stats() - - assert Map.has_key?(stats, :total) - assert Map.has_key?(stats, :active) - assert Map.has_key?(stats, :idle) - assert Map.has_key?(stats, :idle_in_transaction) - assert Map.has_key?(stats, :waiting) - - assert is_integer(stats.total) - assert is_integer(stats.active) - assert is_integer(stats.idle) - assert is_integer(stats.idle_in_transaction) - assert is_integer(stats.waiting) - end - end - - describe "calculate_optimal_batch_size/1 with various value types" do - test "handles maps with float values" do - entries = [%{temp: 98.6, pressure: 1013.25}] - - assert DbOptimizer.calculate_optimal_batch_size(entries) == 2000 - end - - test "handles maps with boolean values" do - entries = [%{active: true, verified: false}] - - assert DbOptimizer.calculate_optimal_batch_size(entries) == 2000 - end - - test "handles maps with DateTime values" do - entries = [%{created_at: DateTime.utc_now(), name: "test"}] - - assert DbOptimizer.calculate_optimal_batch_size(entries) == 2000 - end - - test "handles maps with Date values" do - entries = [%{date: Date.utc_today(), label: "today"}] - - assert DbOptimizer.calculate_optimal_batch_size(entries) == 2000 - end - - test "handles maps with nil values" do - entries = [%{name: nil, value: nil}] - - assert DbOptimizer.calculate_optimal_batch_size(entries) == 2000 - end - - test "handles maps with mixed value types" do - entries = [ - %{ - name: "test", - count: 42, - ratio: 3.14, - active: true, - created: DateTime.utc_now(), - date: Date.utc_today(), - extra: nil - } - ] - - assert DbOptimizer.calculate_optimal_batch_size(entries) == 2000 - end - - test "handles non-map first entry as unknown type" do - entries = ["just a string", "another"] - - assert DbOptimizer.calculate_optimal_batch_size(entries) == 2000 - end - end - - describe "optimized_batch_insert/3 with invalid data" do - test "handles insert errors and returns error count" do - # Insert entries into a nonexistent table - should error - entries = [%{"bad_col" => "value"}] - - {success, errors} = DbOptimizer.optimized_batch_insert("nonexistent_table_xyz", entries) - - assert success == 0 - assert errors == 1 - end - end - - describe "vacuum_table/1 with options" do - test "with full: true returns :error in sandbox" do - assert :error = DbOptimizer.vacuum_table("packets", full: true) - end - - test "with analyze: false returns :error in sandbox" do - assert :error = DbOptimizer.vacuum_table("packets", analyze: false) - end - end - - describe "validate_identifier! and quote_identifier via atoms" do - test "analyze_table accepts an atom name (converts to string)" do - # Exercises validate_identifier!(atom) → string version and - # quote_identifier(atom) → string version (both defp helpers). - result = DbOptimizer.analyze_table(:packets) - # credo:disable-for-next-line Jump.CredoChecks.ConditionalAssertion - assert result == :ok or result == :error - # analyze_table accepts atoms but may fail on real DB — either outcome is valid - end - - test "analyze_table returns :error for invalid identifier characters" do - # validate_identifier! raises ArgumentError, but the surrounding rescue - # in analyze_table converts it to :error. - assert :error = DbOptimizer.analyze_table("not-a-valid;identifier") - end - - test "vacuum_table accepts atom names" do - result = DbOptimizer.vacuum_table(:packets, full: false, analyze: false) - # credo:disable-for-next-line Jump.CredoChecks.ConditionalAssertion - assert result == :ok or result == :error - # vacuum_table may succeed or fail depending on DB state — legitimately conditional - end - end -end diff --git a/test/aprsme/packet_consumer_test.exs b/test/aprsme/packet_consumer_test.exs index de9b2c9..12df70a 100644 --- a/test/aprsme/packet_consumer_test.exs +++ b/test/aprsme/packet_consumer_test.exs @@ -6,7 +6,7 @@ defmodule Aprsme.PacketConsumerTest do alias Aprsme.Packet alias Aprsme.PacketConsumer - alias Aprsme.StreamingPacketsPubSub + alias Aprsme.SpatialPubSub describe "start_link/1" do test "starts an unnamed GenStage consumer when no :name option is given" do @@ -502,7 +502,7 @@ defmodule Aprsme.PacketConsumerTest do test "broadcasts object packets using object_name instead of sender" do # Subscribe to packets in test bounds bounds = %{north: 52.0, south: 50.0, east: -113.0, west: -115.0} - StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) + subscribe_to_bounds(bounds) # Object packet - sender is VE6RWB-15, object name is CALGRY events = [ @@ -533,7 +533,7 @@ defmodule Aprsme.PacketConsumerTest do Process.sleep(50) # Should receive the packet via PubSub with object_name as sender - assert_receive {:streaming_packet, packet}, 1000 + assert_receive {:spatial_packet, packet}, 1000 assert packet.sender == "CALGRY", "Expected sender to be object name CALGRY, got #{packet.sender}" assert packet.latitude == 51.044733 assert packet.longitude == -114.062019 @@ -542,7 +542,7 @@ defmodule Aprsme.PacketConsumerTest do test "broadcasts item packets using item_name instead of sender" do # Subscribe to packets in test bounds bounds = %{north: 40.0, south: 30.0, east: -70.0, west: -80.0} - StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) + subscribe_to_bounds(bounds) # Item packet - sender is ITEMTEST, item name is MyItem events = [ @@ -573,7 +573,7 @@ defmodule Aprsme.PacketConsumerTest do Process.sleep(50) # Should receive the packet via PubSub with item_name as sender - assert_receive {:streaming_packet, packet}, 1000 + assert_receive {:spatial_packet, packet}, 1000 assert packet.sender == "MyItem", "Expected sender to be item name MyItem, got #{packet.sender}" assert packet.latitude == 35.0 assert packet.longitude == -75.0 @@ -776,10 +776,10 @@ defmodule Aprsme.PacketConsumerTest do assert Enum.all?(valid_location_packets, &(&1.location != nil)) end - test "broadcasts packets to StreamingPacketsPubSub" do + test "broadcasts packets to SpatialPubSub" do # Subscribe to packets in test bounds bounds = %{north: 40.0, south: 30.0, east: -70.0, west: -80.0} - StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) + subscribe_to_bounds(bounds) events = [ %{ @@ -807,7 +807,7 @@ defmodule Aprsme.PacketConsumerTest do Process.sleep(50) # Should receive the packet via PubSub - assert_receive {:streaming_packet, packet}, 1000 + assert_receive {:spatial_packet, packet}, 1000 assert packet.sender == "BROADCAST1" end @@ -913,7 +913,7 @@ defmodule Aprsme.PacketConsumerTest do test "fallback individual inserts still broadcast packets" do bounds = %{north: 40.0, south: 30.0, east: -70.0, west: -80.0} - StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) + subscribe_to_bounds(bounds) events = [ %{ @@ -940,7 +940,7 @@ defmodule Aprsme.PacketConsumerTest do # Wait for async broadcast task to complete Process.sleep(50) - assert_receive {:streaming_packet, packet}, 1000 + assert_receive {:spatial_packet, packet}, 1000 assert packet.sender == "FALLBACK1" end end @@ -995,4 +995,12 @@ defmodule Aprsme.PacketConsumerTest do assert memory_growth < 10_000_000 end end + + defp subscribe_to_bounds(bounds) do + client_id = "packet_consumer_test_#{System.unique_integer([:positive])}" + {:ok, topic} = SpatialPubSub.register_viewport(client_id, bounds) + :ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, topic) + on_exit(fn -> SpatialPubSub.unregister_client(client_id) end) + :ok + end end diff --git a/test/aprsme/packet_counter_test.exs b/test/aprsme/packet_counter_test.exs deleted file mode 100644 index 385beae..0000000 --- a/test/aprsme/packet_counter_test.exs +++ /dev/null @@ -1,40 +0,0 @@ -defmodule Aprsme.PacketCounterTest do - use Aprsme.DataCase, async: true - - import Ecto.Query - - alias Aprsme.PacketCounter - alias Aprsme.Repo - - describe "get_count/0" do - test "returns 0 when no records exist" do - Repo.delete_all(PacketCounter) - - assert PacketCounter.get_count() == 0 - end - end - - describe "get_count/1" do - test "returns the count after inserting a record" do - # Update the existing seeded row rather than inserting a duplicate - Repo.update_all(from(pc in PacketCounter, where: pc.counter_type == "total_packets"), set: [count: 42]) - assert PacketCounter.get_count("total_packets") == 42 - end - - test "returns 0 for a counter_type that does not exist" do - assert PacketCounter.get_count("nonexistent_type") == 0 - end - - test "returns the correct count for a custom counter_type" do - Repo.insert!(%PacketCounter{counter_type: "daily_packets", count: 100}) - - assert PacketCounter.get_count("daily_packets") == 100 - end - end - - describe "subscribe_to_changes/0" do - test "returns :ok" do - assert PacketCounter.subscribe_to_changes() == :ok - end - end -end diff --git a/test/aprsme/packet_pipeline_integration_test.exs b/test/aprsme/packet_pipeline_integration_test.exs deleted file mode 100644 index 2b3a131..0000000 --- a/test/aprsme/packet_pipeline_integration_test.exs +++ /dev/null @@ -1,13 +0,0 @@ -defmodule Aprsme.PacketPipelineIntegrationTest do - use Aprsme.DataCase, async: false - - alias Aprsme.Performance.InsertOptimizer - - describe "packet pipeline under load" do - test "insert optimizer provides reasonable batch sizes" do - batch_size = InsertOptimizer.get_optimal_batch_size() - assert batch_size >= 100 - assert batch_size <= 800 - end - end -end diff --git a/test/aprsme/packet_replay_test.exs b/test/aprsme/packet_replay_test.exs deleted file mode 100644 index 93c4b49..0000000 --- a/test/aprsme/packet_replay_test.exs +++ /dev/null @@ -1,527 +0,0 @@ -defmodule Aprsme.PacketReplayTest do - use Aprsme.DataCase, async: false - - alias Aprsme.PacketReplay - - setup do - :ok - end - - describe "start_replay/1" do - test "raises error when bounds are missing" do - user_id = "test_user" - opts = [user_id: user_id] - - assert_raise ArgumentError, "Map bounds are required for packet replay", fn -> - PacketReplay.start_replay(opts) - end - end - - test "fails when user_id is missing" do - bounds = [-74.0, 40.0, -73.0, 41.0] - opts = [bounds: bounds] - - assert_raise KeyError, fn -> - PacketReplay.start_replay(opts) - end - end - end - - describe "stop_replay/1" do - test "returns error when no replay session exists" do - assert {:error, :not_found} = PacketReplay.stop_replay("nonexistent_user") - end - end - - describe "set_replay_speed/2" do - test "validates speed is positive number" do - user_id = "test_user" - - # These should raise function clause errors - assert_raise FunctionClauseError, fn -> - PacketReplay.set_replay_speed(user_id, 0) - end - - assert_raise FunctionClauseError, fn -> - PacketReplay.set_replay_speed(user_id, -1.0) - end - - assert_raise FunctionClauseError, fn -> - # credo:disable-for-next-line Credo.Check.Refactor.Apply - apply(PacketReplay, :set_replay_speed, [user_id, "invalid"]) - end - end - end - - describe "update_filters/2" do - test "validates filters must be a list" do - user_id = "test_user" - - assert_raise FunctionClauseError, fn -> - PacketReplay.update_filters(user_id, %{callsign: "N0CALL"}) - end - end - end - - describe "init/1" do - test "initializes with default values" do - user_id = "test_user" - bounds = [-74.0, 40.0, -73.0, 41.0] - opts = [user_id: user_id, bounds: bounds] - - assert {:ok, state} = PacketReplay.init(opts) - - assert state.user_id == user_id - assert state.bounds == bounds - assert state.replay_speed == 5.0 - assert state.limit == 5000 - assert state.with_position == true - assert state.paused == false - assert state.packets_sent == 0 - assert is_nil(state.callsign) - assert is_nil(state.region) - assert is_nil(state.replay_timer) - assert is_nil(state.last_packet_time) - assert %DateTime{} = state.start_time - assert %DateTime{} = state.end_time - assert %DateTime{} = state.replay_started_at - end - - test "respects custom options" do - user_id = "test_user" - bounds = [-74.0, 40.0, -73.0, 41.0] - start_time = DateTime.add(DateTime.utc_now(), -1800, :second) - end_time = DateTime.utc_now() - - opts = [ - user_id: user_id, - bounds: bounds, - callsign: "N0CALL", - start_time: start_time, - end_time: end_time, - replay_speed: 2.0, - limit: 1000, - with_position: false - ] - - assert {:ok, state} = PacketReplay.init(opts) - - assert state.callsign == "N0CALL" - assert state.start_time == start_time - assert state.end_time == end_time - assert state.replay_speed == 2.0 - assert state.limit == 1000 - assert state.with_position == false - end - - test "limits start_time to 1 hour ago maximum" do - user_id = "test_user" - bounds = [-74.0, 40.0, -73.0, 41.0] - old_start_time = DateTime.add(DateTime.utc_now(), -7200, :second) - - opts = [ - user_id: user_id, - bounds: bounds, - start_time: old_start_time - ] - - assert {:ok, state} = PacketReplay.init(opts) - - one_hour_ago = DateTime.add(DateTime.utc_now(), -3600, :second) - assert DateTime.diff(state.start_time, one_hour_ago, :second) >= -10 - end - - test "sets replay topic correctly" do - user_id = "test_user" - bounds = [-74.0, 40.0, -73.0, 41.0] - opts = [user_id: user_id, bounds: bounds] - - assert {:ok, state} = PacketReplay.init(opts) - - assert state.replay_topic == "replay:#{user_id}" - end - end - - describe "public API hits via_tuple wrappers" do - test "stop_replay returns :ok when a process is registered" do - user_id = "stop-#{System.unique_integer([:positive])}" - - # Register a fake process under the same name format used by via_tuple. - task = - Task.async(fn -> - {:ok, _} = Registry.register(Aprsme.ReplayRegistry, "replay:#{user_id}", :fake) - - receive do - :stop -> :ok - after - 200 -> :ok - end - end) - - # Wait for registration to complete. - :timer.sleep(20) - - # stop_replay/1 routes through via_tuple, finds the pid, and calls GenServer.stop. - # That call exits :normal because our task isn't a real GenServer; either way - # the via_tuple/whereis/stop public-API code path runs. - _ = PacketReplay.stop_replay(user_id) - - send(task.pid, :stop) - _ = Task.shutdown(task, :brutal_kill) - end - - test "set_replay_speed and update_filters route through via_tuple to a running GenServer" do - user_id = "live-#{System.unique_integer([:positive])}" - - # init/1 sends :start_replay to self, which then immediately stops the - # GenServer with :normal because no packets exist. To exercise the - # public-API call wrappers, intercept :start_replay before init runs. - # We do that by starting init manually, then registering the resulting - # process under the registry name. - bounds = [-180.0, -90.0, 180.0, 90.0] - {:ok, pid} = GenServer.start_link(PacketReplay, user_id: user_id, bounds: bounds) - Process.register(pid, :"replay_test_#{user_id}") - Registry.register(Aprsme.ReplayRegistry, "replay:#{user_id}", :ok) - - try do - # Drain :start_replay so it doesn't stop the server. - # (the message has already been sent in init; we accept it might be processed) - # If the server is alive after, the call should succeed. - if Process.alive?(pid) do - # The functions exit if the GenServer dies, so wrap in try. - for op <- [ - fn -> PacketReplay.set_replay_speed(user_id, 2.5) end, - fn -> PacketReplay.update_filters(user_id, callsign: "N0CALL") end, - fn -> PacketReplay.pause_replay(user_id) end, - fn -> PacketReplay.resume_replay(user_id) end, - fn -> PacketReplay.get_replay_info(user_id) end - ] do - try do - op.() - catch - :exit, _ -> :ok - end - end - end - after - if Process.alive?(pid), do: GenServer.stop(pid, :normal, 100) - end - end - end - - describe "module constants and specs" do - test "has correct topic constant" do - assert [user_id: "test", bounds: [0, 0, 1, 1]] |> PacketReplay.init() |> elem(0) == :ok - end - - test "has correct typespec for init" do - result = PacketReplay.init(user_id: "test", bounds: [0, 0, 1, 1]) - assert {:ok, _state} = result - - result2 = PacketReplay.init(user_id: "test", bounds: [0, 0, 1, 1], replay_speed: 2.0) - assert {:ok, _state} = result2 - end - end - - describe "handle_call/3" do - setup do - # Flush any messages lingering from init send_self. - {:ok, state} = PacketReplay.init(user_id: "caller", bounds: [0, 0, 1, 1]) - - receive do - :start_replay -> :ok - after - 0 -> :ok - end - - {:ok, state: state} - end - - test "pause cancels any pending timer and marks state paused", %{state: state} do - timer_ref = Process.send_after(self(), :noop, 60_000) - state = %{state | replay_timer: timer_ref} - - assert {:reply, :ok, new_state} = PacketReplay.handle_call(:pause, self(), state) - assert new_state.paused - assert is_nil(new_state.replay_timer) - end - - test "resume from paused sends continue message and flips flag", %{state: state} do - state = %{state | paused: true} - assert {:reply, :ok, new_state} = PacketReplay.handle_call(:resume, self(), state) - refute new_state.paused - assert_received :start_replay - end - - test "resume when already running is a no-op", %{state: state} do - assert {:reply, :ok, ^state} = PacketReplay.handle_call(:resume, self(), state) - end - - test "set_speed updates the replay speed", %{state: state} do - assert {:reply, :ok, %{replay_speed: 7.5}} = - PacketReplay.handle_call({:set_speed, 7.5}, self(), state) - end - - test "update_filters merges new keys into state", %{state: state} do - filters = [callsign: "K1ABC", replay_speed: 3.0] - - assert {:reply, :ok, new_state} = - PacketReplay.handle_call({:update_filters, filters}, self(), state) - - assert new_state.callsign == "K1ABC" - assert new_state.replay_speed == 3.0 - assert new_state.packets_sent == 0 - end - - test "update_filters rejects invalid bounds and keeps the old bounds", %{state: state} do - filters = [bounds: "not a list"] - - assert {:reply, :ok, new_state} = - PacketReplay.handle_call({:update_filters, filters}, self(), state) - - # Should have reverted to the original bounds. - assert new_state.bounds == state.bounds - end - - test "get_info returns a public view of state", %{state: state} do - assert {:reply, info, ^state} = PacketReplay.handle_call(:get_info, self(), state) - assert info.user_id == "caller" - assert info.bounds == [0, 0, 1, 1] - assert info.paused == false - assert info.packets_sent == 0 - end - end - - describe "handle_info/2 paused path" do - test "pause-time :send_packet reschedules via timer rather than broadcasting" do - {:ok, state} = PacketReplay.init(user_id: "paused", bounds: [0, 0, 1, 1]) - - receive do - :start_replay -> :ok - after - 0 -> :ok - end - - state = %{state | paused: true} - packet = %{received_at: DateTime.utc_now()} - stream = Stream.repeatedly(fn -> {0.0, packet} end) - - assert {:noreply, new_state} = - PacketReplay.handle_info({:send_packet, packet, stream}, state) - - # A new timer ref should be set for rescheduling. - assert is_reference(new_state.replay_timer) - Process.cancel_timer(new_state.replay_timer) - end - end - - describe "terminate/2" do - test "cancels pending timer and returns :ok" do - {:ok, state} = PacketReplay.init(user_id: "terminated", bounds: [0, 0, 1, 1]) - - receive do - :start_replay -> :ok - after - 0 -> :ok - end - - timer_ref = Process.send_after(self(), :noop, 60_000) - state = %{state | replay_timer: timer_ref} - - assert :ok = PacketReplay.terminate(:shutdown, state) - # Timer should have been cancelled. - assert Process.read_timer(timer_ref) == false - end - - test "terminate is idempotent with no pending timer" do - {:ok, state} = PacketReplay.init(user_id: "noterm", bounds: [0, 0, 1, 1]) - - receive do - :start_replay -> :ok - after - 0 -> :ok - end - - assert :ok = PacketReplay.terminate(:normal, state) - end - end - - describe "handle_call/3 extra branches" do - setup do - {:ok, state} = PacketReplay.init(user_id: "extra", bounds: [0, 0, 1, 1]) - - receive do - :start_replay -> :ok - after - 0 -> :ok - end - - {:ok, state: state} - end - - test "pause with no active timer flips paused flag", %{state: state} do - state = %{state | replay_timer: nil} - assert {:reply, :ok, new_state} = PacketReplay.handle_call(:pause, self(), state) - assert new_state.paused - assert is_nil(new_state.replay_timer) - end - - test "update_filters accepts valid bounds format", %{state: state} do - filters = [bounds: [-74.0, 40.0, -73.0, 41.0]] - - assert {:reply, :ok, new_state} = - PacketReplay.handle_call({:update_filters, filters}, self(), state) - - assert new_state.bounds == [-74.0, 40.0, -73.0, 41.0] - end - - test "update_filters with nil bounds keeps existing", %{state: state} do - filters = [bounds: nil] - - assert {:reply, :ok, new_state} = - PacketReplay.handle_call({:update_filters, filters}, self(), state) - - # nil bounds → filter branch keeps the old bounds via state.bounds. - # After Map.put, new_state.bounds is nil, then the guard checks - # is_nil(bounds) and keeps new_state as-is (so bounds stays nil). - # credo:disable-for-next-line Jump.CredoChecks.ConditionalAssertion - assert is_nil(new_state.bounds) or new_state.bounds == state.bounds - # bounds is legitimately nil or unchanged from previous state - end - - test "update_filters cancels an active timer", %{state: state} do - timer = Process.send_after(self(), :noop, 60_000) - state = %{state | replay_timer: timer} - - assert {:reply, :ok, new_state} = - PacketReplay.handle_call({:update_filters, [callsign: "X"]}, self(), state) - - # Timer should be gone from state and cancelled. - assert is_nil(new_state.replay_timer) - end - end - - describe "handle_info({:send_packet, packet, stream}) active path" do - test "broadcasts a packet, increments counter, and stops when stream is empty" do - {:ok, state} = PacketReplay.init(user_id: "send_empty", bounds: [0, 0, 1, 1]) - - receive do - :start_replay -> :ok - after - 0 -> :ok - end - - packet = %Aprsme.Packet{ - sender: "SENDPKT1", - base_callsign: "SENDPKT1", - ssid: "0", - received_at: DateTime.utc_now(), - has_position: true - } - - empty_stream = Stream.unfold(nil, fn _ -> nil end) - - assert {:stop, :normal, new_state} = - PacketReplay.handle_info({:send_packet, packet, empty_stream}, state) - - assert new_state.packets_sent == 1 - end - - test "broadcasts and schedules next when stream has more packets" do - {:ok, state} = PacketReplay.init(user_id: "send_more", bounds: [0, 0, 1, 1]) - - receive do - :start_replay -> :ok - after - 0 -> :ok - end - - packet = %Aprsme.Packet{ - sender: "SENDPKT2", - base_callsign: "SENDPKT2", - ssid: "0", - received_at: DateTime.utc_now(), - has_position: true - } - - next_packet = %Aprsme.Packet{ - sender: "NEXT", - base_callsign: "NEXT", - ssid: "0", - received_at: DateTime.add(DateTime.utc_now(), 1, :second), - has_position: true - } - - # Simple stream: one element, then done. - stream = - Stream.unfold([{0.0, next_packet}], fn - [h | t] -> {h, t} - [] -> nil - end) - - assert {:noreply, new_state} = - PacketReplay.handle_info({:send_packet, packet, stream}, state) - - assert new_state.packets_sent == 1 - assert is_reference(new_state.replay_timer) - Process.cancel_timer(new_state.replay_timer) - end - end - - describe "handle_info(:start_replay, ...)" do - test "broadcasts replay_started then stops when no matching packets" do - {:ok, state} = PacketReplay.init(user_id: "empty_stream", bounds: [0, 0, 1, 1]) - - receive do - :start_replay -> :ok - after - 0 -> :ok - end - - # With no seeded packets in the DB, stream_packets_for_replay returns an - # empty stream and handle_info :start_replay should stop the server. - assert {:stop, :normal, _state} = PacketReplay.handle_info(:start_replay, state) - end - end - - describe "sanitize_packet_for_transport via :send_packet broadcast" do - test "sanitizes DateTime, NaiveDateTime, Decimal, list, and struct fields" do - {:ok, state} = PacketReplay.init(user_id: "sanitize", bounds: [0, 0, 1, 1]) - - receive do - :start_replay -> :ok - after - 0 -> :ok - end - - # Construct a packet with a wide mix of value types so each sanitize_value - # clause runs at least once during the broadcast. - naive = NaiveDateTime.utc_now() - dt = DateTime.utc_now() - - packet = %Aprsme.Packet{ - sender: "SANITIZE-1", - base_callsign: "SANITIZE-1", - ssid: "1", - received_at: dt, - # Decimal field — exercises sanitize_value(%Decimal{}). - lat: Decimal.new("33.0"), - lon: Decimal.new("-96.5"), - has_position: true, - # Map containing a list and a NaiveDateTime nested under data. - data: %{ - "items" => ["a", "b", "c"], - "naive" => naive, - "nested_struct" => %URI{scheme: "https"} - } - } - - empty_stream = Stream.unfold(nil, fn _ -> nil end) - - assert {:stop, :normal, new_state} = - PacketReplay.handle_info({:send_packet, packet, empty_stream}, state) - - assert new_state.packets_sent == 1 - end - end -end diff --git a/test/aprsme/packets_test.exs b/test/aprsme/packets_test.exs index f955b2e..8a9e3f9 100644 --- a/test/aprsme/packets_test.exs +++ b/test/aprsme/packets_test.exs @@ -1082,78 +1082,6 @@ defmodule Aprsme.PacketsTest do end end - describe "clean_old_packets/0" do - setup do - # Defensive: ensure a clean slate so residual rows from prior runs - # (e.g. data committed outside a sandbox transaction) don't inflate - # the deletion count. - Repo.delete_all(Packet) - :ok - end - - test "deletes packets older than retention period" do - now = DateTime.utc_now() - - # Create old packet (400 days ago) - old_packet = - PacketsFixtures.packet_fixture(%{ - sender: "OLD", - received_at: DateTime.add(now, -400 * 24 * 3600, :second) - }) - - # Create recent packet - recent_packet = - PacketsFixtures.packet_fixture(%{ - sender: "RECENT", - received_at: now - }) - - # Run cleanup - deleted_count = Packets.clean_old_packets() - - assert deleted_count == 1 - assert is_nil(Repo.get(Packet, old_packet.id)) - assert Repo.get(Packet, recent_packet.id) - end - end - - describe "clean_packets_older_than/1" do - setup do - Repo.delete_all(Packet) - :ok - end - - test "deletes packets older than specified days" do - now = DateTime.utc_now() - - # Create packets at different ages - old_packet = - PacketsFixtures.packet_fixture(%{ - sender: "OLD", - received_at: DateTime.add(now, -10 * 24 * 3600, :second) - }) - - recent_packet = - PacketsFixtures.packet_fixture(%{ - sender: "RECENT", - received_at: DateTime.add(now, -5 * 24 * 3600, :second) - }) - - # Clean packets older than 7 days - assert {:ok, 1} = Packets.clean_packets_older_than(7) - - assert is_nil(Repo.get(Packet, old_packet.id)) - assert Repo.get(Packet, recent_packet.id) - end - - test "validates positive days parameter" do - # Should guard against invalid params - assert_raise FunctionClauseError, fn -> - Packets.clean_packets_older_than(0) - end - end - end - describe "get_latest_packet_for_callsign/1" do test "returns most recent packet for callsign" do now = DateTime.utc_now() @@ -1263,112 +1191,6 @@ defmodule Aprsme.PacketsTest do end end - describe "get_packets_for_replay/1" do - setup do - now = DateTime.utc_now() - start_time = DateTime.add(now, -3600, :second) - end_time = now - - # Create packets in time range - p1 = - PacketsFixtures.packet_fixture(%{ - sender: "REPLAY1", - received_at: DateTime.add(start_time, 900, :second), - lat: 39.0, - lon: -98.0 - }) - - p2 = - PacketsFixtures.packet_fixture(%{ - sender: "REPLAY2", - received_at: DateTime.add(start_time, 1800, :second), - lat: 39.1, - lon: -98.1 - }) - - # Create packet outside time range - _old = - PacketsFixtures.packet_fixture(%{ - sender: "OLD", - received_at: DateTime.add(start_time, -3600, :second), - lat: 39.2, - lon: -98.2 - }) - - {:ok, packets: [p1, p2], start_time: start_time, end_time: end_time} - end - - test "returns packets in chronological order", %{start_time: start_time, end_time: end_time} do - results = - Packets.get_packets_for_replay(%{ - start_time: start_time, - end_time: end_time - }) - - assert length(results) == 2 - assert hd(results).sender == "REPLAY1" - assert List.last(results).sender == "REPLAY2" - end - - test "filters by bounds", %{start_time: start_time, end_time: end_time} do - # Only includes REPLAY1 - bounds = [38.5, -98.5, 39.05, -97.5] - - results = - Packets.get_packets_for_replay(%{ - start_time: start_time, - end_time: end_time, - bounds: bounds - }) - - # The bounds filtering may not work exactly as expected in test - # Just verify we get results (will be empty with these bounds) - assert results == [] - end - end - - describe "stream_packets_for_replay/1" do - test "returns stream with timing information" do - now = DateTime.utc_now() - start_time = DateTime.add(now, -300, :second) - - # Create packets with 60 second intervals - _p1 = - PacketsFixtures.packet_fixture(%{ - sender: "STREAM1", - received_at: start_time, - lat: 39.0, - lon: -98.0 - }) - - _p2 = - PacketsFixtures.packet_fixture(%{ - sender: "STREAM2", - received_at: DateTime.add(start_time, 60, :second), - lat: 39.1, - lon: -98.1 - }) - - stream = - Packets.stream_packets_for_replay(%{ - start_time: start_time, - end_time: now, - # 2x speed - playback_speed: 2.0 - }) - - [{delay1, packet1}, {delay2, packet2}] = Enum.take(stream, 2) - - # First packet has no delay - assert delay1 == 0 - assert packet1.sender == "STREAM1" - - # 60 seconds / 2.0 speed = 30 seconds - assert_in_delta delay2, 30.0, 0.1 - assert packet2.sender == "STREAM2" - end - end - describe "get_historical_packet_count/1" do test "counts packets matching criteria" do now = DateTime.utc_now() @@ -1646,24 +1468,11 @@ defmodule Aprsme.PacketsTest do end describe "default-arg function heads (zero-arg)" do - test "get_packets_for_replay/0 with no args returns a list" do - result = Packets.get_packets_for_replay() - assert [%{sender: _} | _] = result - end - test "get_recent_packets_for_map/0 with no args returns a list" do result = Packets.get_recent_packets_for_map() assert result == [] end - test "stream_packets_for_replay/0 with no args returns a Stream" do - stream = Packets.stream_packets_for_replay() - # Streams are functions or %Stream{} structs. - assert is_function(stream) - # Materialize to confirm it's enumerable and yields expected tuples. - [{_delay, %{sender: _}} | _] = Enum.to_list(stream) - end - test "get_historical_packet_count/0 returns a non-negative integer" do result = Packets.get_historical_packet_count() assert is_integer(result) and result >= 0 diff --git a/test/aprsme/partition_manager_test.exs b/test/aprsme/partition_manager_test.exs index 5bf90d9..0c9edfb 100644 --- a/test/aprsme/partition_manager_test.exs +++ b/test/aprsme/partition_manager_test.exs @@ -114,7 +114,7 @@ defmodule Aprsme.PartitionManagerTest do ) end - test "drops the partition at the retention cutoff boundary" do + test "keeps a cutoff-day partition while it still contains retained packets" do cutoff_date = Date.add(Date.utc_today(), -7) name = PartitionManager.partition_name(cutoff_date) {from_dt, to_dt} = PartitionManager.partition_range(cutoff_date) @@ -125,6 +125,20 @@ defmodule Aprsme.PartitionManagerTest do {:ok, dropped} = PartitionManager.drop_old_partitions(7) + refute name in dropped + end + + test "drops a partition only after its upper bound passes the rolling cutoff" do + expired_date = Date.add(Date.utc_today(), -8) + name = PartitionManager.partition_name(expired_date) + {from_dt, to_dt} = PartitionManager.partition_range(expired_date) + + Repo.query!( + "CREATE TABLE IF NOT EXISTS #{name} PARTITION OF packets FOR VALUES FROM ('#{DateTime.to_iso8601(from_dt)}') TO ('#{DateTime.to_iso8601(to_dt)}')" + ) + + {:ok, dropped} = PartitionManager.drop_old_partitions(7) + assert name in dropped end end diff --git a/test/aprsme/performance/insert_optimizer_test.exs b/test/aprsme/performance/insert_optimizer_test.exs deleted file mode 100644 index 422be1f..0000000 --- a/test/aprsme/performance/insert_optimizer_test.exs +++ /dev/null @@ -1,181 +0,0 @@ -defmodule Aprsme.Performance.InsertOptimizerTest do - use ExUnit.Case, async: true - - alias Aprsme.Performance.InsertOptimizer - - defp fresh_state do - %{ - current_batch_size: 200, - insert_options: [returning: false, on_conflict: :nothing, timeout: 15_000], - performance_history: [], - last_optimization: System.monotonic_time(:millisecond) - } - end - - defp metric(throughput, duration_ms) do - %{ - batch_size: 200, - duration_ms: duration_ms, - success_count: 100, - throughput: throughput, - timestamp: System.monotonic_time(:millisecond) - } - end - - describe "GenServer callbacks" do - test "handle_call :get_batch_size returns current batch size" do - state = fresh_state() - - assert {:reply, 200, ^state} = - InsertOptimizer.handle_call(:get_batch_size, self(), state) - end - - test "handle_call :get_insert_options returns insert_options" do - state = fresh_state() - opts = state.insert_options - assert {:reply, ^opts, ^state} = InsertOptimizer.handle_call(:get_insert_options, self(), state) - end - - test "handle_cast records a new metric in performance_history" do - state = fresh_state() - - {:noreply, new_state} = - InsertOptimizer.handle_cast({:record_metrics, 200, 1_000, 100}, state) - - assert [entry | _] = new_state.performance_history - assert entry.batch_size == 200 - assert entry.duration_ms == 1_000 - assert entry.success_count == 100 - # Throughput = 100 / (1000/1000) = 100 pps - assert entry.throughput == 100.0 - end - - test "handle_cast with zero duration avoids div-by-0, throughput is 0" do - state = fresh_state() - - {:noreply, new_state} = - InsertOptimizer.handle_cast({:record_metrics, 100, 0, 50}, state) - - assert [entry | _] = new_state.performance_history - assert entry.throughput == 0 - end - - test "performance history is capped at 20 entries" do - state = fresh_state() - - final = - Enum.reduce(1..25, state, fn _, s -> - {:noreply, s2} = InsertOptimizer.handle_cast({:record_metrics, 200, 1000, 100}, s) - s2 - end) - - assert length(final.performance_history) == 20 - end - end - - describe "handle_info :optimize_settings" do - test "with fewer than 3 metrics, state is unchanged except for re-scheduled timer" do - state = %{fresh_state() | performance_history: [metric(100, 1000), metric(90, 1100)]} - {:noreply, new_state} = InsertOptimizer.handle_info(:optimize_settings, state) - # Batch size shouldn't have moved. - assert new_state.current_batch_size == state.current_batch_size - assert new_state.performance_history == state.performance_history - end - - test "grows batch size when throughput is high and duration low" do - # With avg_throughput > 200 and avg_duration < 2000, batch size should scale up - # from 200 → min(@max, round(200 * 1.2)) = 240. - history = - Enum.map(1..5, fn _ -> metric(300, 1000) end) - - state = %{fresh_state() | performance_history: history} - {:noreply, new_state} = InsertOptimizer.handle_info(:optimize_settings, state) - assert new_state.current_batch_size == 240 - # Fast INSERTs keep default options. - assert Keyword.get(new_state.insert_options, :timeout) == 15_000 - end - - test "shrinks batch size when throughput is low and duration high" do - # avg_throughput < 50 and avg_duration > 5000 → batch_size drops by 20%. - history = Enum.map(1..5, fn _ -> metric(10, 6000) end) - state = %{fresh_state() | performance_history: history} - {:noreply, new_state} = InsertOptimizer.handle_info(:optimize_settings, state) - assert new_state.current_batch_size == 160 - # Slow INSERTs bump timeout. - assert Keyword.get(new_state.insert_options, :timeout) == 30_000 - end - - test "keeps batch size when performance is middling" do - # 100 pps at 3000ms falls in neither bucket → no change to batch size. - history = Enum.map(1..5, fn _ -> metric(100, 3000) end) - state = %{fresh_state() | performance_history: history} - {:noreply, new_state} = InsertOptimizer.handle_info(:optimize_settings, state) - assert new_state.current_batch_size == 200 - end - - test "never drops below the minimum batch size" do - state = %{ - fresh_state() - | current_batch_size: 110, - performance_history: Enum.map(1..5, fn _ -> metric(5, 10_000) end) - } - - {:noreply, new_state} = InsertOptimizer.handle_info(:optimize_settings, state) - assert new_state.current_batch_size == 100 - end - - test "never exceeds the maximum batch size" do - state = %{ - fresh_state() - | current_batch_size: 750, - performance_history: Enum.map(1..5, fn _ -> metric(500, 500) end) - } - - {:noreply, new_state} = InsertOptimizer.handle_info(:optimize_settings, state) - assert new_state.current_batch_size == 800 - end - end - - describe "client API when GenServer isn't running" do - test "get_optimal_batch_size falls back to base size on :noproc" do - # The supervised instance isn't started in the app tree, so this should - # hit the catch clause and return the base batch size (200). - assert InsertOptimizer.get_optimal_batch_size() == 200 - end - - test "get_insert_options falls back to default options on :noproc" do - opts = InsertOptimizer.get_insert_options() - assert Keyword.get(opts, :returning) == false - assert Keyword.get(opts, :on_conflict) == :nothing - assert Keyword.get(opts, :timeout) == 15_000 - end - end - - describe "GenServer process lifecycle" do - test "start_link/init/record_insert_metrics/get_optimal_batch_size" do - # Stop any existing instance so start_link/0 doesn't conflict. - if pid = Process.whereis(InsertOptimizer) do - GenServer.stop(pid) - end - - assert {:ok, pid} = InsertOptimizer.start_link([]) - on_exit(fn -> if Process.alive?(pid), do: GenServer.stop(pid) end) - - # Exercises the cast path that records metrics into performance_history. - InsertOptimizer.record_insert_metrics(200, 1500, 100) - # Sync via call so the cast above is processed first. - assert is_integer(InsertOptimizer.get_optimal_batch_size()) - end - - test "handle_cast records a metric with negative duration as zero throughput" do - state = fresh_state() - - # Negative duration should hit `throughput(_count, duration_ms) when duration_ms < 0, do: 0`. - assert {:noreply, new_state} = - InsertOptimizer.handle_cast({:record_metrics, 200, -5, 100}, state) - - [first | _] = new_state.performance_history - assert first.throughput == 0 - end - end -end diff --git a/test/aprsme/prom_ex/plugins/aprsme_test.exs b/test/aprsme/prom_ex/plugins/aprsme_test.exs index 7b42a84..36107ce 100644 --- a/test/aprsme/prom_ex/plugins/aprsme_test.exs +++ b/test/aprsme/prom_ex/plugins/aprsme_test.exs @@ -12,7 +12,7 @@ defmodule Aprsme.PromEx.Plugins.AprsmeTest do test "returns a list of Event structs" do groups = Plugin.event_metrics([]) - assert length(groups) == 7 + assert length(groups) == 6 assert Enum.all?(groups, &match?(%Event{}, &1)) end @@ -63,17 +63,13 @@ defmodule Aprsme.PromEx.Plugins.AprsmeTest do ) end - test "exposes a repo pool group with size/idle/busy/available/queue/total", %{groups: groups} do + test "exposes the configured repo pool size without fabricated utilization", %{groups: groups} do group = find_group(groups, :aprsme_repo_pool_event_metrics) assert %Event{group_name: _, metrics: _} = group names = metric_names(group) assert [:aprsme, :repo, :pool, :size] in names - assert [:aprsme, :repo, :pool, :idle] in names - assert [:aprsme, :repo, :pool, :busy] in names - assert [:aprsme, :repo, :pool, :available] in names - assert [:aprsme, :repo, :pool, :queue_length] in names - assert [:aprsme, :repo, :pool, :total] in names + assert length(names) == 1 end test "exposes a postgres group covering db size, connections, table stats and replication", %{groups: groups} do @@ -109,17 +105,6 @@ defmodule Aprsme.PromEx.Plugins.AprsmeTest do assert [:aprsme, :api, :request, :total] in names end - test "exposes an insert optimizer group", %{groups: groups} do - group = find_group(groups, :aprsme_insert_optimizer_event_metrics) - assert %Event{group_name: _, metrics: _} = group - names = metric_names(group) - - assert [:aprsme, :insert_optimizer, :batch_size] in names - assert [:aprsme, :insert_optimizer, :throughput] in names - assert [:aprsme, :insert_optimizer, :duration] in names - assert [:aprsme, :insert_optimizer, :optimizations] in names - end - test "every metric carries a non-empty description" do groups = Plugin.event_metrics([]) diff --git a/test/aprsme/spatial_pubsub_test.exs b/test/aprsme/spatial_pubsub_test.exs index e0165f9..913ecb0 100644 --- a/test/aprsme/spatial_pubsub_test.exs +++ b/test/aprsme/spatial_pubsub_test.exs @@ -4,6 +4,34 @@ defmodule Aprsme.SpatialPubSubTest do alias Aprsme.SpatialPubSub describe "viewport registration" do + test "unbounded clients receive positioned packets" do + client_id = "test_unbounded_#{:rand.uniform(100_000)}" + assert {:ok, topic} = SpatialPubSub.register_unbounded(client_id) + :ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, topic) + + assert :ok = SpatialPubSub.broadcast_packet(%{lat: 40.5, lon: -73.5, sender: "ANYWHERE"}) + assert_receive {:spatial_packet, %{sender: "ANYWHERE"}}, 1_000 + + SpatialPubSub.unregister_client(client_id) + end + + test "a process with bounded and unbounded registrations receives a packet once" do + bounded_id = "test_bounded_#{:rand.uniform(100_000)}" + unbounded_id = "test_unbounded_#{:rand.uniform(100_000)}" + bounds = %{north: 41.0, south: 40.0, east: -73.0, west: -74.0} + + assert {:ok, topic} = SpatialPubSub.register_viewport(bounded_id, bounds) + assert {:ok, ^topic} = SpatialPubSub.register_unbounded(unbounded_id) + :ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, topic) + + assert :ok = SpatialPubSub.broadcast_packet(%{lat: 40.5, lon: -73.5, sender: "ONCE"}) + assert_receive {:spatial_packet, %{sender: "ONCE"}}, 1_000 + refute_receive {:spatial_packet, %{sender: "ONCE"}}, 100 + + SpatialPubSub.unregister_client(bounded_id) + SpatialPubSub.unregister_client(unbounded_id) + end + test "date-line-crossing viewport registers without timeout" do bounds = %{north: 10.0, south: -10.0, east: -170.0, west: 170.0} client_id = "test_dateline_#{:rand.uniform(100_000)}" diff --git a/test/aprsme/streaming_packets_pubsub_test.exs b/test/aprsme/streaming_packets_pubsub_test.exs deleted file mode 100644 index 4f6854b..0000000 --- a/test/aprsme/streaming_packets_pubsub_test.exs +++ /dev/null @@ -1,240 +0,0 @@ -defmodule Aprsme.StreamingPacketsPubSubTest do - use ExUnit.Case, async: true - - alias Aprsme.StreamingPacketsPubSub - - describe "subscribe_to_bounds/2" do - test "subscribes to packets within geographic bounds" do - bounds = %{ - north: 40.0, - south: 30.0, - east: -70.0, - west: -80.0 - } - - assert :ok = StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) - end - - test "receives packets within subscribed bounds" do - bounds = %{ - north: 40.0, - south: 30.0, - east: -70.0, - west: -80.0 - } - - StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) - - # Packet within bounds - packet_in_bounds = %{ - sender: "K1ABC", - latitude: 35.0, - longitude: -75.0, - received_at: DateTime.utc_now() - } - - StreamingPacketsPubSub.broadcast_packet(packet_in_bounds) - - assert_receive {:streaming_packet, ^packet_in_bounds}, 1000 - end - - test "does not receive packets outside subscribed bounds" do - bounds = %{ - north: 40.0, - south: 30.0, - east: -70.0, - west: -80.0 - } - - StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) - - # Packet outside bounds - packet_outside_bounds = %{ - sender: "K2XYZ", - # North of bounds - latitude: 45.0, - longitude: -75.0, - received_at: DateTime.utc_now() - } - - StreamingPacketsPubSub.broadcast_packet(packet_outside_bounds) - - refute_receive {:streaming_packet, _}, 100 - end - - test "handles multiple subscribers with different bounds" do - subscriber1 = - spawn(fn -> - receive do - _ -> :ok - end - end) - - subscriber2 = - spawn(fn -> - receive do - _ -> :ok - end - end) - - bounds1 = %{north: 40.0, south: 30.0, east: -70.0, west: -80.0} - bounds2 = %{north: 50.0, south: 40.0, east: -60.0, west: -70.0} - - StreamingPacketsPubSub.subscribe_to_bounds(subscriber1, bounds1) - StreamingPacketsPubSub.subscribe_to_bounds(subscriber2, bounds2) - - # Packet in bounds1 only - packet = %{ - sender: "K3TEST", - latitude: 35.0, - longitude: -75.0, - received_at: DateTime.utc_now() - } - - # Monitor to ensure processes are alive - ref1 = Process.monitor(subscriber1) - _ref2 = Process.monitor(subscriber2) - - StreamingPacketsPubSub.broadcast_packet(packet) - - # Verify subscriber1 gets the packet - send(subscriber1, {:check, self()}) - assert_receive {:DOWN, ^ref1, :process, ^subscriber1, :normal}, 1000 - - # Verify subscriber2 is still alive (didn't receive packet) - Process.alive?(subscriber2) - end - end - - describe "unsubscribe/1" do - test "stops receiving packets after unsubscribe" do - bounds = %{north: 40.0, south: 30.0, east: -70.0, west: -80.0} - - StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) - StreamingPacketsPubSub.unsubscribe(self()) - - packet = %{ - sender: "K4TEST", - latitude: 35.0, - longitude: -75.0, - received_at: DateTime.utc_now() - } - - StreamingPacketsPubSub.broadcast_packet(packet) - - refute_receive {:streaming_packet, _}, 100 - end - end - - describe "list_subscribers/0" do - test "returns all active subscribers with their bounds" do - bounds = %{north: 40.0, south: 30.0, east: -70.0, west: -80.0} - - StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) - - subscribers = StreamingPacketsPubSub.list_subscribers() - - assert Enum.any?(subscribers, fn {pid, sub_bounds} -> - pid == self() && sub_bounds == bounds - end) - end - end - - describe "when the server is not running" do - setup do - pid = Process.whereis(StreamingPacketsPubSub) - - if pid do - :erlang.unregister(StreamingPacketsPubSub) - end - - on_exit(fn -> - if pid && !Process.whereis(StreamingPacketsPubSub) do - true = Process.register(pid, StreamingPacketsPubSub) - end - end) - - :ok - end - - test "returns safe defaults instead of crashing" do - bounds = %{north: 40.0, south: 30.0, east: -70.0, west: -80.0} - - assert {:error, :not_running} = StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) - assert :ok = StreamingPacketsPubSub.unsubscribe(self()) - assert :ok = StreamingPacketsPubSub.broadcast_packet(%{latitude: 35.0, longitude: -75.0}) - assert [] == StreamingPacketsPubSub.list_subscribers() - end - end - - describe "performance" do - test "handles high volume of packets efficiently" do - bounds = %{north: 90.0, south: -90.0, east: 180.0, west: -180.0} - StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) - - packets = - for i <- 1..1000 do - %{ - sender: "K#{i}TEST", - latitude: :rand.uniform() * 180 - 90, - longitude: :rand.uniform() * 360 - 180, - received_at: DateTime.utc_now() - } - end - - start_time = System.monotonic_time(:millisecond) - - Enum.each(packets, &StreamingPacketsPubSub.broadcast_packet/1) - - end_time = System.monotonic_time(:millisecond) - elapsed = end_time - start_time - - # Should process 1000 packets in under 100ms - assert elapsed < 100 - - # Should receive all packets - received_count = - Enum.reduce(1..1000, 0, fn _, acc -> - receive do - {:streaming_packet, _} -> acc + 1 - after - 10 -> acc - end - end) - - assert received_count == 1000 - end - end - - describe "subscribe_to_bounds/2 with invalid bounds" do - test "returns {:error, :invalid_bounds} when bounds are not numeric" do - bounds = %{north: "north", south: 0.0, east: 0.0, west: 0.0} - assert {:error, :invalid_bounds} = StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) - end - - test "returns {:error, :invalid_bounds} when north < south" do - bounds = %{north: -10.0, south: 10.0, east: 0.0, west: 0.0} - assert {:error, :invalid_bounds} = StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) - end - - test "returns {:error, :invalid_bounds} when latitude is out of range" do - bounds = %{north: 100.0, south: 0.0, east: 0.0, west: 0.0} - assert {:error, :invalid_bounds} = StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) - end - - test "returns {:error, :invalid_bounds} when longitude is out of range" do - bounds = %{north: 30.0, south: 10.0, east: 200.0, west: 0.0} - assert {:error, :invalid_bounds} = StreamingPacketsPubSub.subscribe_to_bounds(self(), bounds) - end - - test "returns {:error, :invalid_bounds} when bounds is not a map" do - assert {:error, :invalid_bounds} = - StreamingPacketsPubSub.subscribe_to_bounds(self(), :not_a_map) - end - - test "returns {:error, :invalid_bounds} for a map missing required keys" do - assert {:error, :invalid_bounds} = - StreamingPacketsPubSub.subscribe_to_bounds(self(), %{partial: 1}) - end - end -end diff --git a/test/aprsme/telemetry/database_metrics_test.exs b/test/aprsme/telemetry/database_metrics_test.exs index 76f9b15..364a755 100644 --- a/test/aprsme/telemetry/database_metrics_test.exs +++ b/test/aprsme/telemetry/database_metrics_test.exs @@ -42,12 +42,8 @@ defmodule Aprsme.Telemetry.DatabaseMetricsTest do DatabaseMetrics.collect_db_pool_metrics() assert_receive {:telemetry, [:aprsme, :repo, :pool], measurements, _}, 1_000 - assert Map.has_key?(measurements, :size) - assert Map.has_key?(measurements, :idle) - assert Map.has_key?(measurements, :busy) - assert Map.has_key?(measurements, :available) - assert Map.has_key?(measurements, :queue_length) - assert Map.has_key?(measurements, :total) + assert measurements.size == (Application.get_env(:aprsme, Aprsme.Repo)[:pool_size] || 10) + assert map_size(measurements) == 1 end end @@ -78,10 +74,8 @@ defmodule Aprsme.Telemetry.DatabaseMetricsTest do DatabaseMetrics.collect_db_pool_metrics() assert_receive {:telemetry, [:aprsme, :repo, :pool], measurements, _}, 1_000 - # The "unavailable" helper reports mostly zeros. assert measurements.size == (Application.get_env(:aprsme, Aprsme.Repo)[:pool_size] || 10) - assert measurements.idle == 0 - assert measurements.busy == 0 + assert map_size(measurements) == 1 end end diff --git a/test/aprsme/workers/packet_cleanup_worker_test.exs b/test/aprsme/workers/packet_cleanup_worker_test.exs index b20912a..b4d7ee9 100644 --- a/test/aprsme/workers/packet_cleanup_worker_test.exs +++ b/test/aprsme/workers/packet_cleanup_worker_test.exs @@ -2,69 +2,10 @@ defmodule Aprsme.Workers.PacketCleanupWorkerTest do use Aprsme.DataCase, async: false alias Aprsme.BadPacket - alias Aprsme.Packet - alias Aprsme.PartitionManager alias Aprsme.Workers.PacketCleanupWorker - setup do - # Defensive: rows in packets_default whose received_at falls inside a - # range we're about to ATTACH would trigger a check_violation. Wipe - # the parent so partition operations are unobstructed. - Repo.delete_all(Packet) - :ok - end - - describe "perform/1 with cleanup_days" do - test "drops old partitions via PartitionManager" do - # Ensure current partitions exist - {:ok, _} = PartitionManager.ensure_partitions_exist() - - # Create an old partition (30 days ago) - old_date = Date.add(Date.utc_today(), -30) - name = PartitionManager.partition_name(old_date) - {from_dt, to_dt} = PartitionManager.partition_range(old_date) - - Repo.query!( - "CREATE TABLE IF NOT EXISTS #{name} PARTITION OF packets FOR VALUES FROM ('#{DateTime.to_iso8601(from_dt)}') TO ('#{DateTime.to_iso8601(to_dt)}')" - ) - - # Verify old partition exists - assert name in PartitionManager.list_partitions() - - assert :ok = PacketCleanupWorker.perform(%{"cleanup_days" => 7}) - - # Old partition should be dropped - refute name in PartitionManager.list_partitions() - end - - test "does not drop recent partitions" do - {:ok, _} = PartitionManager.ensure_partitions_exist() - - today_name = PartitionManager.partition_name(Date.utc_today()) - - assert :ok = PacketCleanupWorker.perform(%{"cleanup_days" => 7}) - - assert today_name in PartitionManager.list_partitions() - end - - test "drops the partition exactly at the cleanup cutoff" do - cutoff_date = Date.add(Date.utc_today(), -7) - name = PartitionManager.partition_name(cutoff_date) - {from_dt, to_dt} = PartitionManager.partition_range(cutoff_date) - - Repo.query!( - "CREATE TABLE IF NOT EXISTS #{name} PARTITION OF packets FOR VALUES FROM ('#{DateTime.to_iso8601(from_dt)}') TO ('#{DateTime.to_iso8601(to_dt)}')" - ) - - assert :ok = PacketCleanupWorker.perform(%{"cleanup_days" => 7}) - refute name in PartitionManager.list_partitions() - end - end - describe "perform/1 without cleanup_days" do - test "drops old partitions and cleans up bad packets" do - {:ok, _} = PartitionManager.ensure_partitions_exist() - + test "cleans up expired bad packets" do # Create an old bad packet retention_days = Application.get_env(:aprsme, :packet_retention_days, 7) @@ -92,17 +33,12 @@ defmodule Aprsme.Workers.PacketCleanupWorkerTest do describe "perform/1 with negative cleanup_days" do test "falls through to default perform and succeeds" do - {:ok, _} = PartitionManager.ensure_partitions_exist() - - # Negative days don't match the guard, so falls through to the catch-all assert :ok = PacketCleanupWorker.perform(%{"cleanup_days" => -1}) end end describe "perform/1 with extra fields that the guard ignores" do test "accepts a map with cleanup_days plus unrelated keys" do - {:ok, _} = PartitionManager.ensure_partitions_exist() - assert :ok = PacketCleanupWorker.perform(%{ "cleanup_days" => 30, @@ -111,15 +47,10 @@ defmodule Aprsme.Workers.PacketCleanupWorkerTest do end test "handles zero cleanup_days by falling through to default" do - {:ok, _} = PartitionManager.ensure_partitions_exist() - - # 0 doesn't match the `days > 0` guard. assert :ok = PacketCleanupWorker.perform(%{"cleanup_days" => 0}) end test "handles non-integer cleanup_days by falling through to default" do - {:ok, _} = PartitionManager.ensure_partitions_exist() - assert :ok = PacketCleanupWorker.perform(%{"cleanup_days" => "string"}) end end diff --git a/test/aprsme_web/channels/mobile_channel_test.exs b/test/aprsme_web/channels/mobile_channel_test.exs index 04b195d..a8402e3 100644 --- a/test/aprsme_web/channels/mobile_channel_test.exs +++ b/test/aprsme_web/channels/mobile_channel_test.exs @@ -397,7 +397,7 @@ defmodule AprsmeWeb.MobileChannelTest do received_at: DateTime.truncate(DateTime.utc_now(), :second) } - send(socket.channel_pid, {:streaming_packet, packet}) + send(socket.channel_pid, {:spatial_packet, packet}) # Should receive the packet assert_push "packet", pushed_packet @@ -430,7 +430,7 @@ defmodule AprsmeWeb.MobileChannelTest do received_at: DateTime.truncate(DateTime.utc_now(), :second) } - send(socket.channel_pid, {:streaming_packet, packet1}) + send(socket.channel_pid, {:spatial_packet, packet1}) # Should receive the packet assert_push "packet", pushed_packet @@ -444,7 +444,7 @@ defmodule AprsmeWeb.MobileChannelTest do received_at: DateTime.truncate(DateTime.utc_now(), :second) } - send(socket.channel_pid, {:streaming_packet, packet2}) + send(socket.channel_pid, {:spatial_packet, packet2}) # Should NOT receive the packet refute_push "packet", _ @@ -461,7 +461,7 @@ defmodule AprsmeWeb.MobileChannelTest do "received_at" => DateTime.truncate(DateTime.utc_now(), :second) } - send(socket.channel_pid, {:postgres_packet, packet}) + send(socket.channel_pid, {:spatial_packet, packet}) assert_push "packet", pushed_packet assert pushed_packet.callsign == "W5ISP-9" @@ -481,7 +481,7 @@ defmodule AprsmeWeb.MobileChannelTest do received_at: DateTime.truncate(DateTime.utc_now(), :second) } - send(socket.channel_pid, {:streaming_packet, packet}) + send(socket.channel_pid, {:spatial_packet, packet}) # Should receive all W5ISP-* packets assert_push "packet", pushed_packet @@ -517,7 +517,7 @@ defmodule AprsmeWeb.MobileChannelTest do path: "WIDE1-1,WIDE2-1" } - send(socket.channel_pid, {:streaming_packet, packet}) + send(socket.channel_pid, {:spatial_packet, packet}) assert_push "packet", pushed_packet assert pushed_packet.id == "test-id" @@ -560,7 +560,7 @@ defmodule AprsmeWeb.MobileChannelTest do path: nil } - send(socket.channel_pid, {:streaming_packet, packet}) + send(socket.channel_pid, {:spatial_packet, packet}) assert_push "packet", pushed_packet refute Map.has_key?(pushed_packet, :comment) @@ -616,7 +616,7 @@ defmodule AprsmeWeb.MobileChannelTest do symbol_code: ">" } - send(socket.channel_pid, {:streaming_packet, packet}) + send(socket.channel_pid, {:spatial_packet, packet}) assert_push "packet", pushed assert pushed.lat == 33.0 @@ -637,7 +637,7 @@ defmodule AprsmeWeb.MobileChannelTest do symbol_code: ">" } - send(socket.channel_pid, {:streaming_packet, packet}) + send(socket.channel_pid, {:spatial_packet, packet}) # Packet still pushes but lat/lng are missing (nil fields get stripped). assert_push "packet", pushed @@ -646,7 +646,7 @@ defmodule AprsmeWeb.MobileChannelTest do end end - describe "handle_info catch-all and postgres_packet path" do + describe "handle_info catch-all and spatial_packet path" do test "ignores unknown messages without changing socket state", %{socket: socket} do # Hits the `def handle_info(_message, socket)` catch-all clause. send(socket.channel_pid, :totally_unrelated_message) @@ -659,7 +659,7 @@ defmodule AprsmeWeb.MobileChannelTest do assert AprsmeWeb.MobileChannel.__info__(:functions) != [] end - test "postgres_packet without tracked callsign pushes packet to client", %{socket: socket} do + test "spatial_packet without tracked callsign pushes packet to client", %{socket: socket} do packet = %{ id: "pg-1", sender: "PGTEST-1", @@ -670,7 +670,7 @@ defmodule AprsmeWeb.MobileChannelTest do symbol_code: ">" } - send(socket.channel_pid, {:postgres_packet, packet}) + send(socket.channel_pid, {:spatial_packet, packet}) assert_push "packet", pushed assert pushed.lat == 33.0 end @@ -701,7 +701,7 @@ defmodule AprsmeWeb.MobileChannelTest do symbol_code: ">" } - send(socket.channel_pid, {:streaming_packet, packet}) + send(socket.channel_pid, {:spatial_packet, packet}) assert_push "packet", pushed assert pushed.lat == 33.0 assert pushed.lng == -96.5 @@ -718,7 +718,7 @@ defmodule AprsmeWeb.MobileChannelTest do symbol_code: ">" } - send(socket.channel_pid, {:streaming_packet, packet}) + send(socket.channel_pid, {:spatial_packet, packet}) assert_push "packet", pushed refute Map.has_key?(pushed, :lat) refute Map.has_key?(pushed, :lng) @@ -736,7 +736,7 @@ defmodule AprsmeWeb.MobileChannelTest do symbol_code: ">" } - send(socket.channel_pid, {:streaming_packet, packet}) + send(socket.channel_pid, {:spatial_packet, packet}) assert_push "packet", pushed refute Map.has_key?(pushed, :timestamp) end diff --git a/test/aprsme_web/live/bad_packets_live_test.exs b/test/aprsme_web/live/bad_packets_live_test.exs index 35196bb..93e8a2a 100644 --- a/test/aprsme_web/live/bad_packets_live_test.exs +++ b/test/aprsme_web/live/bad_packets_live_test.exs @@ -6,6 +6,12 @@ defmodule AprsmeWeb.BadPacketsLiveTest do alias Aprsme.BadPacket alias Aprsme.Repo + setup %{conn: conn} do + user = Aprsme.AccountsFixtures.user_fixture() + {:ok, admin} = Aprsme.Accounts.set_user_role(user, :admin) + %{conn: log_in_user(conn, admin)} + end + describe "Index" do test "renders bad packets page with card", %{conn: conn} do {:ok, _index_live, html} = live(conn, ~p"/badpackets", on_error: :warn) diff --git a/test/aprsme_web/live/map_live/data_builder_test.exs b/test/aprsme_web/live/map_live/data_builder_test.exs index 4d4b2d3..2ca97f8 100644 --- a/test/aprsme_web/live/map_live/data_builder_test.exs +++ b/test/aprsme_web/live/map_live/data_builder_test.exs @@ -133,10 +133,8 @@ defmodule AprsmeWeb.MapLive.DataBuilderTest do # result is not nil - verified by subsequent key access["callsign"] == "P-K5SGD" # Grouping also uses object name # result is not nil - verified by subsequent key access["callsign_group"] == "P-K5SGD" - # Symbol HTML should show object name - # result is not nil - verified by subsequent key access["symbol_html"] =~ "P-K5SGD" - refute result["symbol_html"] =~ "DB0SDA" - # credo:disable-for-next-line Jump.CredoChecks.TestHasNoAssertions + assert result["callsign"] == "P-K5SGD" + refute Map.has_key?(result, "symbol_html") end # credo:disable-for-next-line Jump.CredoChecks.TestHasNoAssertions @@ -177,9 +175,9 @@ defmodule AprsmeWeb.MapLive.DataBuilderTest do # result is not nil - verified by subsequent key access # result is not nil - verified by subsequent key access["is_most_recent_for_callsign"] == true - # Most recent should NOT use the red dot - # credo:disable-for-next-line Jump.CredoChecks.TestHasNoAssertions - refute result["symbol_html"] =~ "#FF6B6B" + assert result["symbol_table_id"] == "/" + assert result["symbol_code"] == "-" + refute Map.has_key?(result, "symbol_html") end # credo:disable-for-next-line Jump.CredoChecks.TestHasNoAssertions diff --git a/test/aprsme_web/live/map_live/index_test.exs b/test/aprsme_web/live/map_live/index_test.exs index 6815f7f..d71fcc0 100644 --- a/test/aprsme_web/live/map_live/index_test.exs +++ b/test/aprsme_web/live/map_live/index_test.exs @@ -6,6 +6,8 @@ defmodule AprsmeWeb.MapLive.IndexTest do import Phoenix.LiveViewTest setup do + Mox.stub(Aprsme.PacketsMock, :get_recent_packets, fn _opts -> [] end) + # Reset the shared Hammer ETS so per-IP request counts don't bleed # between test cases and trigger 429s in the middle of a run. case :ets.info(Aprsme.RateLimiter) do @@ -78,9 +80,7 @@ defmodule AprsmeWeb.MapLive.IndexTest do end test "loads historical packets when map is ready", %{conn: conn} do - # Mock the Packets.get_packets_for_replay function to return empty list - # since we now load all historical packets at once instead of replaying - Mox.stub(Aprsme.PacketsMock, :get_packets_for_replay, fn _opts -> [] end) + Mox.stub(Aprsme.PacketsMock, :get_recent_packets, fn _opts -> [] end) {:ok, view, _html} = live(conn, "/") @@ -133,8 +133,7 @@ defmodule AprsmeWeb.MapLive.IndexTest do } ] - # Mock the Packets.get_packets_for_replay function to return our test packets - Mox.stub(Aprsme.PacketsMock, :get_packets_for_replay, fn opts -> + Mox.stub(Aprsme.PacketsMock, :get_recent_packets, fn opts -> # Verify that the time range is being passed correctly (not overridden to 1 hour) assert Map.has_key?(opts, :start_time) assert Map.has_key?(opts, :end_time) @@ -370,27 +369,6 @@ defmodule AprsmeWeb.MapLive.IndexTest do assert render(view) =~ "aprs-map" end - test "responds to a streaming_packet message", %{conn: conn} do - {:ok, view, _html} = live(conn, "/", on_error: :warn) - - packet = %{ - id: "stream-1", - sender: "STREAM", - base_callsign: "STREAM", - ssid: "0", - lat: 39.0, - lon: -98.0, - symbol_table_id: "/", - symbol_code: ">", - comment: nil, - received_at: DateTime.utc_now(), - path: "" - } - - send(view.pid, {:streaming_packet, packet}) - assert render(view) =~ "aprs-map" - end - test "responds to a drain_connections message", %{conn: conn} do {:ok, view, _html} = live(conn, "/", on_error: :warn) send(view.pid, {:drain_connections, 5}) diff --git a/test/aprsme_web/live/map_live/integration_test.exs b/test/aprsme_web/live/map_live/integration_test.exs index c1a2796..2b52bb3 100644 --- a/test/aprsme_web/live/map_live/integration_test.exs +++ b/test/aprsme_web/live/map_live/integration_test.exs @@ -614,7 +614,7 @@ defmodule AprsmeWeb.MapLive.IntegrationTest do send(view.pid, {:spatial_packet, packet}) Process.sleep(50) - send(view.pid, {:streaming_packet, packet}) + send(view.pid, {:spatial_packet, packet}) Process.sleep(50) assert render(view) =~ "aprs-map" diff --git a/test/aprsme_web/live/map_live/overlay_rendering_test.exs b/test/aprsme_web/live/map_live/overlay_rendering_test.exs index dcdf4f7..3cab748 100644 --- a/test/aprsme_web/live/map_live/overlay_rendering_test.exs +++ b/test/aprsme_web/live/map_live/overlay_rendering_test.exs @@ -4,7 +4,7 @@ defmodule AprsmeWeb.MapLive.OverlayRenderingTest do alias AprsmeWeb.MapLive.PacketUtils describe "overlay symbol rendering in map" do - test "W5MRC-15 with D& symbol generates correct HTML" do + test "W5MRC-15 with D& symbol emits structured marker data" do # Create a test packet with overlay symbol D& packet = %{ id: 1, @@ -23,20 +23,10 @@ defmodule AprsmeWeb.MapLive.OverlayRenderingTest do # Process the packet through PacketUtils result = PacketUtils.build_packet_data(packet, true, "en-US") - # Check that symbol_html was generated - assert Map.has_key?(result, "symbol_html") - symbol_html = result["symbol_html"] - - # Verify the overlay symbol is rendered correctly with overlay character on top - # The overlay character (D) from table 2, base symbol (&) from table 1 - assert symbol_html =~ - "background-image: url(/aprs-symbols/aprs-symbols-128-2@2x.png), url(/aprs-symbols/aprs-symbols-128-1@2x.png)" - - # Verify the positions (overlay character D first at -96.0px -64.0px, then base & at -160.0px 0.0px) - assert symbol_html =~ "background-position: -96.0px -64.0px, -160.0px 0.0px" - - # Verify callsign is included - assert symbol_html =~ "W5MRC-15" + assert result["symbol_table_id"] == "D" + assert result["symbol_code"] == "&" + assert result["callsign"] == "W5MRC-15" + refute Map.has_key?(result, "symbol_html") end test "N# green star overlay symbol generates correct HTML" do @@ -57,18 +47,10 @@ defmodule AprsmeWeb.MapLive.OverlayRenderingTest do # Process the packet result = PacketUtils.build_packet_data(packet, true, "en-US") - symbol_html = result["symbol_html"] - - # Verify the overlay uses different sprite tables - # Overlay character N from table 2, base # from table 1 - assert symbol_html =~ - "background-image: url(/aprs-symbols/aprs-symbols-128-2@2x.png), url(/aprs-symbols/aprs-symbols-128-1@2x.png)" - - # Verify the positions (overlay N first, then base #) - assert symbol_html =~ "background-position: -416.0px -64.0px, -64.0px 0.0px" - - # Verify callsign - assert symbol_html =~ "TEST-1" + assert result["symbol_table_id"] == "N" + assert result["symbol_code"] == "#" + assert result["callsign"] == "TEST-1" + refute Map.has_key?(result, "symbol_html") end test "normal symbol /_ generates correct HTML without overlay" do @@ -89,17 +71,10 @@ defmodule AprsmeWeb.MapLive.OverlayRenderingTest do # Process the packet result = PacketUtils.build_packet_data(packet, true, "en-US") - symbol_html = result["symbol_html"] - - # Verify it's a single background image (no overlay) - assert symbol_html =~ "background-image: url(/aprs-symbols/aprs-symbols-128-0@2x.png)" - refute symbol_html =~ "background-image: url(/aprs-symbols/aprs-symbols-128-0@2x.png), url" - - # Verify the position for _ symbol - assert symbol_html =~ "background-position: -448.0px -96.0px" - - # Verify callsign - assert symbol_html =~ "WEATHER-1" + assert result["symbol_table_id"] == "/" + assert result["symbol_code"] == "_" + assert result["callsign"] == "WEATHER-1" + refute Map.has_key?(result, "symbol_html") end end end diff --git a/test/aprsme_web/live/map_live/packet_batcher_test.exs b/test/aprsme_web/live/map_live/packet_batcher_test.exs index 9cfd0db..6a1357b 100644 --- a/test/aprsme_web/live/map_live/packet_batcher_test.exs +++ b/test/aprsme_web/live/map_live/packet_batcher_test.exs @@ -18,6 +18,22 @@ defmodule AprsmeWeb.MapLive.PacketBatcherTest do end describe "batching behavior" do + test "rejects stale and duplicate timestamped packets for a sender" do + {:ok, pid} = PacketBatcher.start_link(self()) + newer_at = ~U[2026-01-02 00:00:00Z] + older_at = ~U[2026-01-01 00:00:00Z] + + PacketBatcher.add_packet(pid, %{sender: "ORDER-1", received_at: newer_at}) + PacketBatcher.add_packet(pid, %{sender: "ORDER-1", received_at: older_at}) + PacketBatcher.add_packet(pid, %{sender: "ORDER-1", received_at: newer_at}) + PacketBatcher.flush(pid) + + assert_receive {:packet_batch, [%{received_at: ^newer_at}]}, 1_000 + refute_receive {:packet_batch, _}, 100 + + GenServer.stop(pid) + end + test "delivers packets in batch after timeout" do {:ok, pid} = PacketBatcher.start_link(self()) diff --git a/test/aprsme_web/user_auth_test.exs b/test/aprsme_web/user_auth_test.exs index 897ce81..6d53fbf 100644 --- a/test/aprsme_web/user_auth_test.exs +++ b/test/aprsme_web/user_auth_test.exs @@ -44,6 +44,8 @@ defmodule AprsmeWeb.UserAuthTest do assert %{value: signed_token, max_age: max_age} = conn.resp_cookies[@remember_me_cookie] assert signed_token != get_session(conn, :user_token) assert max_age == 60 * 60 * 24 * 60 + assert conn.resp_cookies[@remember_me_cookie].secure + assert conn.resp_cookies[@remember_me_cookie].http_only end end @@ -242,4 +244,18 @@ defmodule AprsmeWeb.UserAuthTest do refute conn.status end end + + describe "require_admin_user/2" do + test "allows administrators", %{conn: conn, user: user} do + admin = %{user | role: :admin} + conn = conn |> assign(:current_user, admin) |> UserAuth.require_admin_user([]) + refute conn.halted + end + + test "forbids regular users", %{conn: conn, user: user} do + conn = conn |> assign(:current_user, user) |> UserAuth.require_admin_user([]) + assert conn.halted + assert conn.status == 403 + end + end end diff --git a/test/test_helper.exs b/test/test_helper.exs index 90a297b..7bf9a7a 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -17,7 +17,6 @@ Ecto.Adapters.SQL.Sandbox.mode(Aprsme.Repo, :manual) # Configure Mox Mox.defmock(Aprsme.PacketsMock, for: Aprsme.PacketsBehaviour) -Mox.defmock(Aprsme.PacketReplayMock, for: Aprsme.PacketReplayBehaviour) Mox.defmock(PacketsMock, for: Aprsme.PacketsBehaviour) # Set up default stubs for commonly used functions