maintainability: obsolete config cleanup, focused migration tests
Some checks failed
Build and Push / Build and Push Docker Image (push) Failing after 2s
Some checks failed
Build and Push / Build and Push Docker Image (push) Failing after 2s
- Removed stale .dialyzer_ignore.exs entry for deleted packet_replay_test.exs - Removed orphaned :initialize_replay_delay config key from test.exs - Updated CLAUDE.md: StreamingPacketsPubSub -> SpatialPubSub - Added migration_validation_test.exs: 12 tests covering geometry GiST indexes, counter reconciliation, and partition boundary cutoff behavior - Updated handoff document: marked maintainability #3 and #5 as done
This commit is contained in:
parent
9f45937a82
commit
9ba4dac53a
5 changed files with 338 additions and 10 deletions
|
|
@ -56,6 +56,5 @@
|
||||||
# type warnings from the Elixir compiler. All are pre-existing and benign.
|
# type warnings from the Elixir compiler. All are pre-existing and benign.
|
||||||
{"test/aprsme/packets_mic_e_test.exs"},
|
{"test/aprsme/packets_mic_e_test.exs"},
|
||||||
{"test/aprsme/packets_test.exs"},
|
{"test/aprsme/packets_test.exs"},
|
||||||
{"test/aprsme/is_test.exs"},
|
{"test/aprsme/is_test.exs"}
|
||||||
{"test/aprsme/packet_replay_test.exs"}
|
|
||||||
]
|
]
|
||||||
|
|
|
||||||
|
|
@ -13,7 +13,7 @@ The application provides a real-time WebSocket API for mobile clients (iOS/Andro
|
||||||
- Real-time streaming of APRS packets
|
- Real-time streaming of APRS packets
|
||||||
- Geographic bounds filtering (only receive packets in viewport)
|
- Geographic bounds filtering (only receive packets in viewport)
|
||||||
- Dynamic bounds updates (follow user as they pan/zoom)
|
- Dynamic bounds updates (follow user as they pan/zoom)
|
||||||
- Efficient - leverages existing StreamingPacketsPubSub infrastructure
|
- Efficient - leverages existing SpatialPubSub infrastructure
|
||||||
|
|
||||||
## Elixir guidelines
|
## Elixir guidelines
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -50,9 +50,6 @@ config :aprsme, :cleanup_scheduler, enabled: false
|
||||||
# Speed up ConnectionManager init in test environment
|
# Speed up ConnectionManager init in test environment
|
||||||
config :aprsme, :connection_manager_init_delay, 0
|
config :aprsme, :connection_manager_init_delay, 0
|
||||||
|
|
||||||
# Disable initialize replay delay in test environment
|
|
||||||
config :aprsme, :initialize_replay_delay, 0
|
|
||||||
|
|
||||||
# Configure the packets module to use the mock in tests
|
# Configure the packets module to use the mock in tests
|
||||||
config :aprsme, :packets_module, Aprsme.PacketsMock
|
config :aprsme, :packets_module, Aprsme.PacketsMock
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -134,16 +134,16 @@ MIX_ENV=prod mix assets.deploy
|
||||||
rendering modules. Preserve LiveView behavior with integration tests first.
|
rendering modules. Preserve LiveView behavior with integration tests first.
|
||||||
2. Review the now-smaller supervision tree for naming and ownership consistency;
|
2. Review the now-smaller supervision tree for naming and ownership consistency;
|
||||||
document which process owns ingestion, retention, broadcast, and cleanup.
|
document which process owns ingestion, retention, broadcast, and cleanup.
|
||||||
3. Continue deleting obsolete configuration keys, telemetry names, docs, mocks,
|
3. ~~Continue deleting obsolete configuration keys, telemetry names, docs, mocks,
|
||||||
and aliases uncovered by the removed modules.
|
and aliases uncovered by the removed modules.~~ **DONE** — removed stale `.dialyzer_ignore.exs` entry, orphaned `:initialize_replay_delay` config key, updated `CLAUDE.md` StreamingPacketsPubSub → SpatialPubSub reference. No stale telemetry/metric or mock references remain.
|
||||||
4. Review callsign naming. Ingestion currently accepts safe APRS transport
|
4. Review callsign naming. Ingestion currently accepts safe APRS transport
|
||||||
identifiers (up to 20 characters, uppercase alphanumeric segments separated by
|
identifiers (up to 20 characters, uppercase alphanumeric segments separated by
|
||||||
hyphens), which is intentionally broader than a strict AX.25 callsign. Rename
|
hyphens), which is intentionally broader than a strict AX.25 callsign. Rename
|
||||||
APIs or document this distinction to avoid future accidental tightening.
|
APIs or document this distinction to avoid future accidental tightening.
|
||||||
5. Add focused migration tests for:
|
5. ~~Add focused migration tests for:
|
||||||
- Existing partition indexes being attached correctly.
|
- Existing partition indexes being attached correctly.
|
||||||
- Counter reconciliation on an existing populated database.
|
- Counter reconciliation on an existing populated database.
|
||||||
- Exact cutoff behavior around partition boundaries.
|
- Exact cutoff behavior around partition boundaries.~~ **DONE** — 12 tests in `test/aprsme/migration_validation_test.exs`: GiST index on parent + child partitions, counter reconciliation with INSERT/deletes, strict `<` upper bound cutoff with partition boundary straddling.
|
||||||
6. Review the deleted test count. The suite decreased because tests for removed
|
6. Review the deleted test count. The suite decreased because tests for removed
|
||||||
infrastructure were deleted; ensure important end-to-end ingestion coverage was
|
infrastructure were deleted; ensure important end-to-end ingestion coverage was
|
||||||
not lost when `packet_pipeline_integration_test.exs` was removed.
|
not lost when `packet_pipeline_integration_test.exs` was removed.
|
||||||
|
|
|
||||||
332
test/aprsme/migration_validation_test.exs
Normal file
332
test/aprsme/migration_validation_test.exs
Normal file
|
|
@ -0,0 +1,332 @@
|
||||||
|
defmodule Aprsme.MigrationValidationTest do
|
||||||
|
use Aprsme.DataCase, async: false
|
||||||
|
|
||||||
|
alias Aprsme.PartitionManager
|
||||||
|
|
||||||
|
@moduletag :migration
|
||||||
|
|
||||||
|
setup do
|
||||||
|
# Ensure counters row exists
|
||||||
|
Repo.query!("""
|
||||||
|
INSERT INTO packet_counters (counter_type, count, inserted_at, updated_at)
|
||||||
|
VALUES ('total_packets', 0, NOW(), NOW())
|
||||||
|
ON CONFLICT (counter_type) DO NOTHING
|
||||||
|
""")
|
||||||
|
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
|
||||||
|
# ── 1. Geometry index (idx_packets_location) on partitions ──────────────
|
||||||
|
|
||||||
|
describe "geometry index (idx_packets_location)" do
|
||||||
|
test "parent table has the GiST index" do
|
||||||
|
{:ok, %{rows: rows}} =
|
||||||
|
Repo.query("""
|
||||||
|
SELECT indexname
|
||||||
|
FROM pg_indexes
|
||||||
|
WHERE tablename = 'packets'
|
||||||
|
AND schemaname = 'public'
|
||||||
|
AND indexname = 'idx_packets_location'
|
||||||
|
""")
|
||||||
|
|
||||||
|
assert length(rows) == 1
|
||||||
|
assert [["idx_packets_location"]] = rows
|
||||||
|
end
|
||||||
|
|
||||||
|
test "index is valid (indisvalid = true) on parent" do
|
||||||
|
{:ok, %{rows: rows}} =
|
||||||
|
Repo.query("""
|
||||||
|
SELECT indisvalid
|
||||||
|
FROM pg_index
|
||||||
|
WHERE indexrelid = 'idx_packets_location'::regclass
|
||||||
|
""")
|
||||||
|
|
||||||
|
assert [[true]] = rows
|
||||||
|
end
|
||||||
|
|
||||||
|
test "parent index has child partition indexes attached via pg_inherits" do
|
||||||
|
# The parent GiST index should have child entries in pg_inherits
|
||||||
|
# linking it to per-partition index relations.
|
||||||
|
{:ok, %{rows: inherited}} =
|
||||||
|
Repo.query("""
|
||||||
|
SELECT inhrelid::regclass::text
|
||||||
|
FROM pg_inherits
|
||||||
|
WHERE inhparent = 'idx_packets_location'::regclass
|
||||||
|
""")
|
||||||
|
|
||||||
|
# At least one child partition index must be attached
|
||||||
|
assert inherited != []
|
||||||
|
|
||||||
|
# Each inherited index name ends with _location_idx (Postgres auto-named)
|
||||||
|
# or _location_gist_idx (from migration build_and_attach).
|
||||||
|
child_names = List.flatten(inherited)
|
||||||
|
|
||||||
|
assert Enum.any?(child_names, fn name ->
|
||||||
|
String.ends_with?(name, "_location_idx") or String.ends_with?(name, "_location_gist_idx")
|
||||||
|
end)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "idx_packets_location is a GiST index on location column" do
|
||||||
|
# Verify the index uses GiST access method on the location column
|
||||||
|
{:ok, %{rows: [[am_name]]}} =
|
||||||
|
Repo.query("""
|
||||||
|
SELECT am.amname
|
||||||
|
FROM pg_index i
|
||||||
|
JOIN pg_class c ON c.oid = i.indexrelid
|
||||||
|
JOIN pg_am am ON am.oid = c.relam
|
||||||
|
WHERE i.indexrelid = 'idx_packets_location'::regclass
|
||||||
|
""")
|
||||||
|
|
||||||
|
assert am_name == "gist"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# ── 2. Counter reconciliation ───────────────────────────────────────────
|
||||||
|
|
||||||
|
describe "packet counter reconciliation" do
|
||||||
|
test "reconciliation sets counter to exact row count after INSERT" do
|
||||||
|
now = DateTime.truncate(DateTime.utc_now(), :second)
|
||||||
|
|
||||||
|
# Insert packets via raw SQL (gen_random_uuid() for binary UUID)
|
||||||
|
for i <- 1..5 do
|
||||||
|
Repo.query!(
|
||||||
|
"""
|
||||||
|
INSERT INTO packets (id, sender, base_callsign, ssid, destination, raw_packet,
|
||||||
|
data_type, lat, lon, received_at, inserted_at, updated_at)
|
||||||
|
VALUES (gen_random_uuid(), 'RECON' || $1, 'RECON', $1::text, 'APRS',
|
||||||
|
'RECON' || $1 || '>APRS:test', 'position', 39.0, -98.0,
|
||||||
|
$2, $2, $2)
|
||||||
|
ON CONFLICT DO NOTHING
|
||||||
|
""",
|
||||||
|
["#{i}", DateTime.add(now, -i * 60, :second)]
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
{:ok, %{rows: [[total]]}} = Repo.query("SELECT COUNT(*) FROM packets")
|
||||||
|
assert total >= 5
|
||||||
|
|
||||||
|
# Set counter to wrong value (simulate drift)
|
||||||
|
Repo.query!("""
|
||||||
|
UPDATE packet_counters
|
||||||
|
SET count = 999, updated_at = NOW()
|
||||||
|
WHERE counter_type = 'total_packets'
|
||||||
|
""")
|
||||||
|
|
||||||
|
# Run reconciliation (from migration 20260726000002)
|
||||||
|
Repo.query!("""
|
||||||
|
UPDATE packet_counters
|
||||||
|
SET count = (SELECT COUNT(*) FROM packets),
|
||||||
|
updated_at = NOW()
|
||||||
|
WHERE counter_type = 'total_packets'
|
||||||
|
""")
|
||||||
|
|
||||||
|
{:ok, %{rows: [[counter_val]]}} =
|
||||||
|
Repo.query("SELECT count FROM packet_counters WHERE counter_type = 'total_packets'")
|
||||||
|
|
||||||
|
{:ok, %{rows: [[actual_count]]}} =
|
||||||
|
Repo.query("SELECT COUNT(*) FROM packets")
|
||||||
|
|
||||||
|
assert counter_val == actual_count
|
||||||
|
end
|
||||||
|
|
||||||
|
test "reconciliation resets to zero on empty packets table" do
|
||||||
|
# Set counter to wrong value
|
||||||
|
Repo.query!("""
|
||||||
|
UPDATE packet_counters
|
||||||
|
SET count = 123, updated_at = NOW()
|
||||||
|
WHERE counter_type = 'total_packets'
|
||||||
|
""")
|
||||||
|
|
||||||
|
# Run reconciliation
|
||||||
|
Repo.query!("""
|
||||||
|
UPDATE packet_counters
|
||||||
|
SET count = (SELECT COUNT(*) FROM packets),
|
||||||
|
updated_at = NOW()
|
||||||
|
WHERE counter_type = 'total_packets'
|
||||||
|
""")
|
||||||
|
|
||||||
|
{:ok, %{rows: [[counter_val]]}} =
|
||||||
|
Repo.query("SELECT count FROM packet_counters WHERE counter_type = 'total_packets'")
|
||||||
|
|
||||||
|
{:ok, %{rows: [[real_count]]}} =
|
||||||
|
Repo.query("SELECT COUNT(*) FROM packets")
|
||||||
|
|
||||||
|
assert counter_val == real_count
|
||||||
|
end
|
||||||
|
|
||||||
|
test "get_packet_count() function returns the reconciled counter value" do
|
||||||
|
Repo.query!("""
|
||||||
|
UPDATE packet_counters
|
||||||
|
SET count = (SELECT COUNT(*) FROM packets),
|
||||||
|
updated_at = NOW()
|
||||||
|
WHERE counter_type = 'total_packets'
|
||||||
|
""")
|
||||||
|
|
||||||
|
{:ok, %{rows: [[func_count]]}} =
|
||||||
|
Repo.query("SELECT get_packet_count()")
|
||||||
|
|
||||||
|
{:ok, %{rows: [[real_count]]}} =
|
||||||
|
Repo.query("SELECT COUNT(*) FROM packets")
|
||||||
|
|
||||||
|
assert func_count == real_count
|
||||||
|
end
|
||||||
|
|
||||||
|
test "triggers keep counter in sync on insert and delete" do
|
||||||
|
Repo.query!("""
|
||||||
|
UPDATE packet_counters
|
||||||
|
SET count = (SELECT COUNT(*) FROM packets),
|
||||||
|
updated_at = NOW()
|
||||||
|
WHERE counter_type = 'total_packets'
|
||||||
|
""")
|
||||||
|
|
||||||
|
{:ok, %{rows: [[before]]}} =
|
||||||
|
Repo.query("SELECT count FROM packet_counters WHERE counter_type = 'total_packets'")
|
||||||
|
|
||||||
|
now = DateTime.truncate(DateTime.utc_now(), :second)
|
||||||
|
|
||||||
|
# Insert via raw SQL — triggers should fire
|
||||||
|
{:ok, %{rows: [[inserted_id]]}} =
|
||||||
|
Repo.query(
|
||||||
|
"""
|
||||||
|
INSERT INTO packets (id, sender, base_callsign, ssid, destination, raw_packet,
|
||||||
|
data_type, lat, lon, location, has_position, received_at, inserted_at, updated_at)
|
||||||
|
VALUES (gen_random_uuid(), 'TRIGTST', 'TRIGTST', '0', 'APRS', 'TRIGTST>APRS:test',
|
||||||
|
'position', 39.0, -98.0, ST_SetSRID(ST_MakePoint(-98.0, 39.0), 4326), true,
|
||||||
|
$1, $1, $1)
|
||||||
|
RETURNING id
|
||||||
|
""",
|
||||||
|
[now]
|
||||||
|
)
|
||||||
|
|
||||||
|
{:ok, %{rows: [[after_insert]]}} =
|
||||||
|
Repo.query("SELECT count FROM packet_counters WHERE counter_type = 'total_packets'")
|
||||||
|
|
||||||
|
assert after_insert == before + 1
|
||||||
|
|
||||||
|
# Delete the packet — trigger should decrement
|
||||||
|
Repo.query!("DELETE FROM packets WHERE id = $1", [inserted_id])
|
||||||
|
|
||||||
|
{:ok, %{rows: [[after_delete]]}} =
|
||||||
|
Repo.query("SELECT count FROM packet_counters WHERE counter_type = 'total_packets'")
|
||||||
|
|
||||||
|
assert after_delete == before
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# ── 3. Partition boundary cutoff ────────────────────────────────────────
|
||||||
|
|
||||||
|
describe "partition boundary cutoff" do
|
||||||
|
setup do
|
||||||
|
# Ensure today's partition exists so raw INSERTs route correctly
|
||||||
|
{:ok, _} = PartitionManager.ensure_partitions_exist()
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
|
||||||
|
test "FOR VALUES FROM ... TO ... uses strict < upper bound" do
|
||||||
|
date = Date.utc_today()
|
||||||
|
partition_name = PartitionManager.partition_name(date)
|
||||||
|
{from_dt, to_dt} = PartitionManager.partition_range(date)
|
||||||
|
|
||||||
|
# Insert packet at exact partition start (>= FROM bound)
|
||||||
|
{:ok, %{rows: [[start_id]]}} =
|
||||||
|
Repo.query(
|
||||||
|
"""
|
||||||
|
INSERT INTO packets (id, sender, base_callsign, ssid, destination, raw_packet,
|
||||||
|
data_type, received_at, inserted_at, updated_at)
|
||||||
|
VALUES (gen_random_uuid(), 'BDSTART', 'BDSTART', '0', 'APRS', 'BDSTART>APRS:test',
|
||||||
|
'status', $1, $1, $1)
|
||||||
|
RETURNING id
|
||||||
|
""",
|
||||||
|
[from_dt]
|
||||||
|
)
|
||||||
|
|
||||||
|
# Insert packet at exact partition end (< TO bound)
|
||||||
|
{:ok, %{rows: [[end_id]]}} =
|
||||||
|
Repo.query(
|
||||||
|
"""
|
||||||
|
INSERT INTO packets (id, sender, base_callsign, ssid, destination, raw_packet,
|
||||||
|
data_type, received_at, inserted_at, updated_at)
|
||||||
|
VALUES (gen_random_uuid(), 'BDEND', 'BDEND', '0', 'APRS', 'BDEND>APRS:test',
|
||||||
|
'status', $1, $1, $1)
|
||||||
|
RETURNING id
|
||||||
|
""",
|
||||||
|
[to_dt]
|
||||||
|
)
|
||||||
|
|
||||||
|
# Packet at FROM should be in this partition (>= FROM)
|
||||||
|
{:ok, %{rows: [[start_count]]}} =
|
||||||
|
Repo.query("SELECT COUNT(*) FROM #{partition_name} WHERE id = $1::uuid", [start_id])
|
||||||
|
|
||||||
|
assert start_count == 1
|
||||||
|
|
||||||
|
# Packet at TO should NOT be in this partition (< TO, strict upper bound)
|
||||||
|
{:ok, %{rows: [[end_count]]}} =
|
||||||
|
Repo.query("SELECT COUNT(*) FROM #{partition_name} WHERE id = $1::uuid", [end_id])
|
||||||
|
|
||||||
|
assert end_count == 0
|
||||||
|
end
|
||||||
|
|
||||||
|
test "packet at boundary timestamp is findable via parent table" do
|
||||||
|
date = Date.utc_today()
|
||||||
|
{_from_dt, to_dt} = PartitionManager.partition_range(date)
|
||||||
|
|
||||||
|
{:ok, %{rows: [[end_id]]}} =
|
||||||
|
Repo.query(
|
||||||
|
"""
|
||||||
|
INSERT INTO packets (id, sender, base_callsign, ssid, destination, raw_packet,
|
||||||
|
data_type, received_at, inserted_at, updated_at)
|
||||||
|
VALUES (gen_random_uuid(), 'ROUTABLE', 'ROUTABLE', '0', 'APRS', 'ROUTABLE>APRS:test',
|
||||||
|
'status', $1, $1, $1)
|
||||||
|
RETURNING id
|
||||||
|
""",
|
||||||
|
[to_dt]
|
||||||
|
)
|
||||||
|
|
||||||
|
# Packet should be findable via parent table query
|
||||||
|
{:ok, %{rows: [[_]]}} =
|
||||||
|
Repo.query("SELECT sender FROM packets WHERE id = $1::uuid", [end_id])
|
||||||
|
|
||||||
|
# Verify it's NOT in today's partition (boundary exclusive)
|
||||||
|
today_name = PartitionManager.partition_name(date)
|
||||||
|
|
||||||
|
{:ok, %{rows: [[in_today]]}} =
|
||||||
|
Repo.query("SELECT COUNT(*) FROM #{today_name} WHERE id = $1::uuid", [end_id])
|
||||||
|
|
||||||
|
assert in_today == 0
|
||||||
|
end
|
||||||
|
|
||||||
|
test "drop_old_partitions drops partitions whose upper bound <= cutoff" do
|
||||||
|
old_date = Date.add(Date.utc_today(), -30)
|
||||||
|
old_name = PartitionManager.partition_name(old_date)
|
||||||
|
{from_dt, to_dt} = PartitionManager.partition_range(old_date)
|
||||||
|
|
||||||
|
Repo.query!("""
|
||||||
|
CREATE TABLE IF NOT EXISTS #{old_name}
|
||||||
|
PARTITION OF packets
|
||||||
|
FOR VALUES FROM ('#{DateTime.to_iso8601(from_dt)}') TO ('#{DateTime.to_iso8601(to_dt)}')
|
||||||
|
""")
|
||||||
|
|
||||||
|
assert old_name in PartitionManager.list_partitions()
|
||||||
|
|
||||||
|
{:ok, dropped} = PartitionManager.drop_old_partitions(8)
|
||||||
|
|
||||||
|
assert old_name in dropped
|
||||||
|
end
|
||||||
|
|
||||||
|
test "boundary-day partition is retained when its upper bound > 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)}')
|
||||||
|
""")
|
||||||
|
|
||||||
|
{:ok, dropped} = PartitionManager.drop_old_partitions(7)
|
||||||
|
|
||||||
|
refute name in dropped
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
Loading…
Add table
Reference in a new issue