From 7c0861f21669eb8cb44cdd614306c37b0741cac1 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Sun, 22 Mar 2026 17:12:11 -0500 Subject: [PATCH] fix: replace duplicate spatial registrations --- lib/aprsme/spatial_pubsub.ex | 22 ++++++++++++++++++++-- test/aprsme/spatial_pubsub_test.exs | 22 ++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/lib/aprsme/spatial_pubsub.ex b/lib/aprsme/spatial_pubsub.ex index fe59d24..97ead72 100644 --- a/lib/aprsme/spatial_pubsub.ex +++ b/lib/aprsme/spatial_pubsub.ex @@ -85,14 +85,17 @@ defmodule Aprsme.SpatialPubSub do # Create a unique topic for this client topic = "spatial:#{client_id}" + state = replace_existing_client(state, client_id) + # Monitor the client process - Process.monitor(pid) + ref = Process.monitor(pid) # Update client info client_info = %{ bounds: normalize_bounds(bounds), topic: topic, - pid: pid + pid: pid, + monitor_ref: ref } # Update spatial index @@ -353,6 +356,14 @@ defmodule Aprsme.SpatialPubSub do nil -> state + %{bounds: bounds, monitor_ref: ref} -> + Process.demonitor(ref, [:flush]) + + state + |> remove_from_spatial_index(client_id, bounds) + |> update_in([:clients], &Map.delete(&1, client_id)) + |> update_in([:stats, :clients_count], &max(0, &1 - 1)) + %{bounds: bounds} -> state |> remove_from_spatial_index(client_id, bounds) @@ -361,6 +372,13 @@ defmodule Aprsme.SpatialPubSub do end end + defp replace_existing_client(state, client_id) do + case Map.get(state.clients, client_id) do + nil -> state + _client_info -> remove_client(state, client_id) + end + end + defp calculate_avg_clients_per_cell(spatial_index) do non_nil_cells = spatial_index diff --git a/test/aprsme/spatial_pubsub_test.exs b/test/aprsme/spatial_pubsub_test.exs index b6633f2..b7a4636 100644 --- a/test/aprsme/spatial_pubsub_test.exs +++ b/test/aprsme/spatial_pubsub_test.exs @@ -34,6 +34,28 @@ defmodule Aprsme.SpatialPubSubTest do SpatialPubSub.unregister_client(client_id) end + + test "re-registering an existing client replaces the old viewport without inflating counts" do + client_id = "test_reregister_#{:rand.uniform(100_000)}" + bounds_1 = %{north: 41.0, south: 40.0, east: -73.0, west: -74.0} + bounds_2 = %{north: 36.0, south: 35.0, east: -79.0, west: -80.0} + + before_stats = SpatialPubSub.get_stats() + + assert {:ok, _topic} = SpatialPubSub.register_viewport(client_id, bounds_1) + mid_stats = SpatialPubSub.get_stats() + assert mid_stats.clients_count == before_stats.clients_count + 1 + + assert {:ok, _topic} = SpatialPubSub.register_viewport(client_id, bounds_2) + after_stats = SpatialPubSub.get_stats() + + assert after_stats.clients_count == before_stats.clients_count + 1 + + state = :sys.get_state(SpatialPubSub) + assert state.clients[client_id].bounds == bounds_2 + + SpatialPubSub.unregister_client(client_id) + end end describe "duplicate packet prevention" do