From 847b45a4318f6ae14f5f6686a3f8d0817cf421e3 Mon Sep 17 00:00:00 2001 From: Dmitry Popov Date: Fri, 14 Nov 2025 19:35:45 +0100 Subject: [PATCH 01/32] fix(core): added gracefull map poll recovery from saved state. added map slug unique checks --- lib/wanderer_app/api/changes/slugify_name.ex | 66 ++- lib/wanderer_app/map/map_pool.ex | 188 +++++- .../map/map_pool_dynamic_supervisor.ex | 8 +- lib/wanderer_app/map/map_pool_state.ex | 190 ++++++ lib/wanderer_app/map/map_pool_supervisor.ex | 6 + .../map/server/map_server_impl.ex | 7 + .../event_handlers/map_core_event_handler.ex | 112 ++-- lib/wanderer_app_web/live/map/map_live.ex | 7 + lib/wanderer_app_web/live/maps/maps_live.ex | 299 +++++++--- ...14184420_ensure_no_duplicate_map_slugs.exs | 212 +++++++ .../map/map_pool_crash_integration_test.exs | 463 +++++++++++++++ .../unit/map/map_pool_crash_recovery_test.exs | 561 ++++++++++++++++++ test/unit/map/slug_uniqueness_test.exs | 320 ++++++++++ 13 files changed, 2300 insertions(+), 139 deletions(-) create mode 100644 lib/wanderer_app/map/map_pool_state.ex create mode 100644 priv/repo/migrations/20251114184420_ensure_no_duplicate_map_slugs.exs create mode 100644 test/integration/map/map_pool_crash_integration_test.exs create mode 100644 test/unit/map/map_pool_crash_recovery_test.exs create mode 100644 test/unit/map/slug_uniqueness_test.exs diff --git a/lib/wanderer_app/api/changes/slugify_name.ex b/lib/wanderer_app/api/changes/slugify_name.ex index 2c6b0fad..24c3dab2 100644 --- a/lib/wanderer_app/api/changes/slugify_name.ex +++ b/lib/wanderer_app/api/changes/slugify_name.ex @@ -1,8 +1,25 @@ defmodule WandererApp.Api.Changes.SlugifyName do + @moduledoc """ + Ensures map slugs are unique by: + 1. Slugifying the provided slug/name + 2. Checking for existing slugs (optimization) + 3. Finding next available slug with numeric suffix if needed + 4. Relying on database unique constraint as final arbiter + + Race Condition Mitigation: + - Optimistic check reduces DB roundtrips for most cases + - Database unique index ensures no duplicates slip through + - Proper error messages for constraint violations + - Telemetry events for monitoring conflicts + """ use Ash.Resource.Change alias Ash.Changeset require Ash.Query + require Logger + + # Maximum number of attempts to find a unique slug + @max_attempts 100 @impl true @spec change(Changeset.t(), keyword, Change.context()) :: Changeset.t() @@ -26,7 +43,7 @@ defmodule WandererApp.Api.Changes.SlugifyName do # Get the current record ID if this is an update operation current_id = Changeset.get_attribute(changeset, :id) - # Check if the base slug is available + # Check if the base slug is available (optimization to avoid numeric suffixes when possible) if slug_available?(base_slug, current_id) do base_slug else @@ -35,16 +52,44 @@ defmodule WandererApp.Api.Changes.SlugifyName do end end - defp find_available_slug(base_slug, current_id, n) do + defp find_available_slug(base_slug, current_id, n) when n <= @max_attempts do candidate_slug = "#{base_slug}-#{n}" if slug_available?(candidate_slug, current_id) do + # Emit telemetry when we had to use a suffix (indicates potential conflict) + :telemetry.execute( + [:wanderer_app, :map, :slug_suffix_used], + %{suffix_number: n}, + %{base_slug: base_slug, final_slug: candidate_slug} + ) + candidate_slug else find_available_slug(base_slug, current_id, n + 1) end end + defp find_available_slug(base_slug, _current_id, n) when n > @max_attempts do + # Fallback: use timestamp suffix if we've tried too many numeric suffixes + # This handles edge cases where many maps have similar names + timestamp = System.system_time(:millisecond) + fallback_slug = "#{base_slug}-#{timestamp}" + + Logger.warning( + "Slug generation exceeded #{@max_attempts} attempts for '#{base_slug}', using timestamp fallback", + base_slug: base_slug, + fallback_slug: fallback_slug + ) + + :telemetry.execute( + [:wanderer_app, :map, :slug_fallback_used], + %{attempts: n}, + %{base_slug: base_slug, fallback_slug: fallback_slug} + ) + + fallback_slug + end + defp slug_available?(slug, current_id) do query = WandererApp.Api.Map @@ -60,9 +105,20 @@ defmodule WandererApp.Api.Changes.SlugifyName do |> Ash.Query.limit(1) case Ash.read(query) do - {:ok, []} -> true - {:ok, _} -> false - {:error, _} -> false + {:ok, []} -> + true + + {:ok, _existing} -> + false + + {:error, error} -> + # Log error but be conservative - assume slug is not available + Logger.warning("Error checking slug availability", + slug: slug, + error: inspect(error) + ) + + false end end end diff --git a/lib/wanderer_app/map/map_pool.ex b/lib/wanderer_app/map/map_pool.ex index 07dfe880..bc1581e0 100644 --- a/lib/wanderer_app/map/map_pool.ex +++ b/lib/wanderer_app/map/map_pool.ex @@ -4,7 +4,7 @@ defmodule WandererApp.Map.MapPool do require Logger - alias WandererApp.Map.Server + alias WandererApp.Map.{MapPoolState, Server} defstruct [ :map_ids, @@ -26,7 +26,17 @@ defmodule WandererApp.Map.MapPool do def new(), do: __struct__() def new(args), do: __struct__(args) - def start_link(map_ids) do + # Accept both {uuid, map_ids} tuple (from supervisor restart) and just map_ids (legacy) + def start_link({uuid, map_ids}) when is_binary(uuid) and is_list(map_ids) do + GenServer.start_link( + @name, + {uuid, map_ids}, + name: Module.concat(__MODULE__, uuid) + ) + end + + # For backward compatibility - generate UUID if only map_ids provided + def start_link(map_ids) when is_list(map_ids) do uuid = UUID.uuid1() GenServer.start_link( @@ -38,13 +48,42 @@ defmodule WandererApp.Map.MapPool do @impl true def init({uuid, map_ids}) do - {:ok, _} = Registry.register(@unique_registry, Module.concat(__MODULE__, uuid), map_ids) + # Check for crash recovery - if we have previous state in ETS, merge it with new map_ids + {final_map_ids, recovery_info} = + case MapPoolState.get_pool_state(uuid) do + {:ok, recovered_map_ids} -> + # Merge and deduplicate map IDs + merged = Enum.uniq(recovered_map_ids ++ map_ids) + recovery_count = length(recovered_map_ids) + + Logger.info( + "[Map Pool #{uuid}] Crash recovery detected: recovering #{recovery_count} maps", + pool_uuid: uuid, + recovered_maps: recovered_map_ids, + new_maps: map_ids, + total_maps: length(merged) + ) + + # Emit telemetry for crash recovery + :telemetry.execute( + [:wanderer_app, :map_pool, :recovery, :start], + %{recovered_map_count: recovery_count, total_map_count: length(merged)}, + %{pool_uuid: uuid} + ) + + {merged, %{recovered: true, count: recovery_count}} + + {:error, :not_found} -> + # Normal startup, no previous state to recover + {map_ids, %{recovered: false}} + end + + # Register with empty list - maps will be added as they're started in handle_continue + {:ok, _} = Registry.register(@unique_registry, Module.concat(__MODULE__, uuid), []) {:ok, _} = Registry.register(@registry, __MODULE__, uuid) - map_ids - |> Enum.each(fn id -> - Cachex.put(@cache, id, uuid) - end) + # Don't pre-populate cache - will be populated as maps start in handle_continue + # This prevents duplicates when recovering state = %{ @@ -53,32 +92,96 @@ defmodule WandererApp.Map.MapPool do } |> new() - {:ok, state, {:continue, {:start, map_ids}}} + {:ok, state, {:continue, {:start, {final_map_ids, recovery_info}}}} end @impl true - def terminate(_reason, _state) do + def terminate(reason, %{uuid: uuid} = _state) do + # On graceful shutdown, clean up ETS state + # On crash, keep ETS state for recovery + case reason do + :normal -> + Logger.debug("[Map Pool #{uuid}] Graceful shutdown, cleaning up ETS state") + MapPoolState.delete_pool_state(uuid) + + :shutdown -> + Logger.debug("[Map Pool #{uuid}] Graceful shutdown, cleaning up ETS state") + MapPoolState.delete_pool_state(uuid) + + {:shutdown, _} -> + Logger.debug("[Map Pool #{uuid}] Graceful shutdown, cleaning up ETS state") + MapPoolState.delete_pool_state(uuid) + + _ -> + Logger.warning("[Map Pool #{uuid}] Abnormal termination (#{inspect(reason)}), keeping ETS state for recovery") + # Keep ETS state for crash recovery + :ok + end + :ok end @impl true - def handle_continue({:start, map_ids}, state) do + def handle_continue({:start, {map_ids, recovery_info}}, state) do Logger.info("#{@name} started") + # Track recovery statistics + start_time = System.monotonic_time(:millisecond) + initial_count = length(map_ids) + # Start maps synchronously and accumulate state changes - new_state = + {new_state, failed_maps} = map_ids - |> Enum.reduce(state, fn map_id, current_state -> + |> Enum.reduce({state, []}, fn map_id, {current_state, failed} -> case do_start_map(map_id, current_state) do {:ok, updated_state} -> - updated_state + {updated_state, failed} {:error, reason} -> Logger.error("[Map Pool] Failed to start map #{map_id}: #{reason}") - current_state + + # Emit telemetry for individual map recovery failure + if recovery_info.recovered do + :telemetry.execute( + [:wanderer_app, :map_pool, :recovery, :map_failed], + %{map_id: map_id}, + %{pool_uuid: state.uuid, reason: reason} + ) + end + + {current_state, [map_id | failed]} end end) + # Calculate final statistics + end_time = System.monotonic_time(:millisecond) + duration_ms = end_time - start_time + successful_count = length(new_state.map_ids) + failed_count = length(failed_maps) + + # Log and emit telemetry for recovery completion + if recovery_info.recovered do + Logger.info( + "[Map Pool #{state.uuid}] Crash recovery completed: #{successful_count}/#{initial_count} maps recovered in #{duration_ms}ms", + pool_uuid: state.uuid, + recovered_count: successful_count, + failed_count: failed_count, + total_count: initial_count, + duration_ms: duration_ms, + failed_maps: failed_maps + ) + + :telemetry.execute( + [:wanderer_app, :map_pool, :recovery, :complete], + %{ + recovered_count: successful_count, + failed_count: failed_count, + duration_ms: duration_ms + }, + %{pool_uuid: state.uuid} + ) + end + # Schedule periodic tasks Process.send_after(self(), :backup_state, @backup_state_timeout) Process.send_after(self(), :cleanup_systems, 15_000) @@ -181,6 +284,9 @@ defmodule WandererApp.Map.MapPool do # Step 4: Update GenServer state (last, as this is in-memory and fast) new_state = %{state | map_ids: [map_id | map_ids]} + # Step 5: Persist state to ETS for crash recovery + MapPoolState.save_pool_state(uuid, new_state.map_ids) + Logger.debug("[Map Pool] Successfully started map #{map_id} in pool #{uuid}") {:ok, new_state} rescue @@ -278,6 +384,9 @@ defmodule WandererApp.Map.MapPool do # Step 4: Update GenServer state (last, as this is in-memory and fast) new_state = %{state | map_ids: map_ids |> Enum.reject(fn id -> id == map_id end)} + # Step 5: Persist state to ETS for crash recovery + MapPoolState.save_pool_state(uuid, new_state.map_ids) + Logger.debug("[Map Pool] Successfully stopped map #{map_id} from pool #{uuid}") {:ok, new_state} rescue @@ -335,10 +444,14 @@ defmodule WandererApp.Map.MapPool do def handle_call(:error, _, state), do: {:stop, :error, :ok, state} @impl true - def handle_info(:backup_state, %{map_ids: map_ids} = state) do + def handle_info(:backup_state, %{map_ids: map_ids, uuid: uuid} = state) do Process.send_after(self(), :backup_state, @backup_state_timeout) try do + # Persist pool state to ETS + MapPoolState.save_pool_state(uuid, map_ids) + + # Backup individual map states to database map_ids |> Task.async_stream( fn map_id -> @@ -534,6 +647,51 @@ defmodule WandererApp.Map.MapPool do {:noreply, state} end + def handle_info(:map_deleted, %{map_ids: map_ids} = state) do + # When a map is deleted, stop all maps in this pool that are deleted + # This is a graceful shutdown triggered by user action + Logger.info("[Map Pool #{state.uuid}] Received map_deleted event, stopping affected maps") + + # Check which of our maps were deleted and stop them + new_state = + map_ids + |> Enum.reduce(state, fn map_id, current_state -> + # Check if the map still exists in the database + case WandererApp.MapRepo.get(map_id) do + {:ok, %{deleted: true}} -> + Logger.info("[Map Pool #{state.uuid}] Map #{map_id} was deleted, stopping it") + + case do_stop_map(map_id, current_state) do + {:ok, updated_state} -> + updated_state + + {:error, reason} -> + Logger.error("[Map Pool #{state.uuid}] Failed to stop deleted map #{map_id}: #{reason}") + current_state + end + + {:ok, _map} -> + # Map still exists and is not deleted + current_state + + {:error, _} -> + # Map doesn't exist, should stop it + Logger.info("[Map Pool #{state.uuid}] Map #{map_id} not found, stopping it") + + case do_stop_map(map_id, current_state) do + {:ok, updated_state} -> + updated_state + + {:error, reason} -> + Logger.error("[Map Pool #{state.uuid}] Failed to stop missing map #{map_id}: #{reason}") + current_state + end + end + end) + + {:noreply, new_state} + end + def handle_info(event, state) do try do Server.Impl.handle_event(event) diff --git a/lib/wanderer_app/map/map_pool_dynamic_supervisor.ex b/lib/wanderer_app/map/map_pool_dynamic_supervisor.ex index 232fb025..7a5b3697 100644 --- a/lib/wanderer_app/map/map_pool_dynamic_supervisor.ex +++ b/lib/wanderer_app/map/map_pool_dynamic_supervisor.ex @@ -140,9 +140,13 @@ defmodule WandererApp.Map.MapPoolDynamicSupervisor do end defp start_child(map_ids, pools_count) do - case DynamicSupervisor.start_child(@name, {WandererApp.Map.MapPool, map_ids}) do + # Generate UUID for the new pool - this will be used for crash recovery + uuid = UUID.uuid1() + + # Pass both UUID and map_ids to the pool for crash recovery support + case DynamicSupervisor.start_child(@name, {WandererApp.Map.MapPool, {uuid, map_ids}}) do {:ok, pid} -> - Logger.info("Starting map pool, total map_pools: #{pools_count + 1}") + Logger.info("Starting map pool #{uuid}, total map_pools: #{pools_count + 1}") {:ok, pid} {:error, {:already_started, pid}} -> diff --git a/lib/wanderer_app/map/map_pool_state.ex b/lib/wanderer_app/map/map_pool_state.ex new file mode 100644 index 00000000..81198918 --- /dev/null +++ b/lib/wanderer_app/map/map_pool_state.ex @@ -0,0 +1,190 @@ +defmodule WandererApp.Map.MapPoolState do + @moduledoc """ + Helper module for persisting MapPool state to ETS for crash recovery. + + This module provides functions to save and retrieve MapPool state from an ETS table. + The state survives GenServer crashes but is lost on node restart, which ensures + automatic recovery from crashes while avoiding stale state on system restart. + + ## ETS Table Ownership + + The ETS table `:map_pool_state_table` is owned by the MapPoolSupervisor, + ensuring it survives individual MapPool process crashes. + + ## State Format + + State is stored as tuples: `{pool_uuid, map_ids, last_updated_timestamp}` + where: + - `pool_uuid` is the unique identifier for the pool (key) + - `map_ids` is a list of map IDs managed by this pool + - `last_updated_timestamp` is the Unix timestamp of the last update + """ + + require Logger + + @table_name :map_pool_state_table + @stale_threshold_hours 24 + + @doc """ + Initializes the ETS table for storing MapPool state. + + This should be called by the MapPoolSupervisor during initialization. + The table is created as: + - `:set` - Each pool UUID has exactly one entry + - `:public` - Any process can read/write + - `:named_table` - Can be accessed by name + + Returns the table reference or raises if table already exists. + """ + @spec init_table() :: :ets.table() + def init_table do + :ets.new(@table_name, [:set, :public, :named_table]) + end + + @doc """ + Saves the current state of a MapPool to ETS. + + ## Parameters + - `uuid` - The unique identifier for the pool + - `map_ids` - List of map IDs currently managed by this pool + + ## Examples + + iex> MapPoolState.save_pool_state("pool-123", [1, 2, 3]) + :ok + """ + @spec save_pool_state(String.t(), [integer()]) :: :ok + def save_pool_state(uuid, map_ids) when is_binary(uuid) and is_list(map_ids) do + timestamp = System.system_time(:second) + true = :ets.insert(@table_name, {uuid, map_ids, timestamp}) + + Logger.debug("Saved MapPool state for #{uuid}: #{length(map_ids)} maps", + pool_uuid: uuid, + map_count: length(map_ids) + ) + + :ok + end + + @doc """ + Retrieves the saved state for a MapPool from ETS. + + ## Parameters + - `uuid` - The unique identifier for the pool + + ## Returns + - `{:ok, map_ids}` if state exists + - `{:error, :not_found}` if no state exists for this UUID + + ## Examples + + iex> MapPoolState.get_pool_state("pool-123") + {:ok, [1, 2, 3]} + + iex> MapPoolState.get_pool_state("non-existent") + {:error, :not_found} + """ + @spec get_pool_state(String.t()) :: {:ok, [integer()]} | {:error, :not_found} + def get_pool_state(uuid) when is_binary(uuid) do + case :ets.lookup(@table_name, uuid) do + [{^uuid, map_ids, _timestamp}] -> + {:ok, map_ids} + + [] -> + {:error, :not_found} + end + end + + @doc """ + Deletes the state for a MapPool from ETS. + + This should be called when a pool is gracefully shut down. + + ## Parameters + - `uuid` - The unique identifier for the pool + + ## Examples + + iex> MapPoolState.delete_pool_state("pool-123") + :ok + """ + @spec delete_pool_state(String.t()) :: :ok + def delete_pool_state(uuid) when is_binary(uuid) do + true = :ets.delete(@table_name, uuid) + + Logger.debug("Deleted MapPool state for #{uuid}", pool_uuid: uuid) + + :ok + end + + @doc """ + Removes stale entries from the ETS table. + + Entries are considered stale if they haven't been updated in the last + #{@stale_threshold_hours} hours. This helps prevent the table from growing + unbounded due to pool UUIDs that are no longer in use. + + Returns the number of entries deleted. + + ## Examples + + iex> MapPoolState.cleanup_stale_entries() + {:ok, 3} + """ + @spec cleanup_stale_entries() :: {:ok, non_neg_integer()} + def cleanup_stale_entries do + stale_threshold = System.system_time(:second) - @stale_threshold_hours * 3600 + + match_spec = [ + { + {:"$1", :"$2", :"$3"}, + [{:<, :"$3", stale_threshold}], + [:"$1"] + } + ] + + stale_uuids = :ets.select(@table_name, match_spec) + + Enum.each(stale_uuids, fn uuid -> + :ets.delete(@table_name, uuid) + + Logger.info("Cleaned up stale MapPool state for #{uuid}", + pool_uuid: uuid, + reason: :stale + ) + end) + + {:ok, length(stale_uuids)} + end + + @doc """ + Returns all pool states currently stored in ETS. + + Useful for debugging and monitoring. + + ## Examples + + iex> MapPoolState.list_all_states() + [ + {"pool-123", [1, 2, 3], 1699564800}, + {"pool-456", [4, 5], 1699564900} + ] + """ + @spec list_all_states() :: [{String.t(), [integer()], integer()}] + def list_all_states do + :ets.tab2list(@table_name) + end + + @doc """ + Returns the count of pool states currently stored in ETS. + + ## Examples + + iex> MapPoolState.count_states() + 5 + """ + @spec count_states() :: non_neg_integer() + def count_states do + :ets.info(@table_name, :size) + end +end diff --git a/lib/wanderer_app/map/map_pool_supervisor.ex b/lib/wanderer_app/map/map_pool_supervisor.ex index cfa284dd..d7efee18 100644 --- a/lib/wanderer_app/map/map_pool_supervisor.ex +++ b/lib/wanderer_app/map/map_pool_supervisor.ex @@ -2,6 +2,8 @@ defmodule WandererApp.Map.MapPoolSupervisor do @moduledoc false use Supervisor + alias WandererApp.Map.MapPoolState + @name __MODULE__ @registry :map_pool_registry @unique_registry :unique_map_pool_registry @@ -11,6 +13,10 @@ defmodule WandererApp.Map.MapPoolSupervisor do end def init(_args) do + # Initialize ETS table for MapPool state persistence + # This table survives individual MapPool crashes but is lost on node restart + MapPoolState.init_table() + children = [ {Registry, [keys: :unique, name: @unique_registry]}, {Registry, [keys: :duplicate, name: @registry]}, diff --git a/lib/wanderer_app/map/server/map_server_impl.ex b/lib/wanderer_app/map/server/map_server_impl.ex index 503d072f..af245ce6 100644 --- a/lib/wanderer_app/map/server/map_server_impl.ex +++ b/lib/wanderer_app/map/server/map_server_impl.ex @@ -358,6 +358,13 @@ defmodule WandererApp.Map.Server.Impl do update_options(map_id, options) end + def handle_event(:map_deleted) do + # Map has been deleted - this event is handled by MapPool to stop the server + # and by MapLive to redirect users. Nothing to do here. + Logger.debug("Map deletion event received, will be handled by MapPool") + :ok + end + def handle_event({ref, _result}) when is_reference(ref) do Process.demonitor(ref, [:flush]) end diff --git a/lib/wanderer_app_web/live/map/event_handlers/map_core_event_handler.ex b/lib/wanderer_app_web/live/map/event_handlers/map_core_event_handler.ex index ea134552..008919f4 100644 --- a/lib/wanderer_app_web/live/map/event_handlers/map_core_event_handler.ex +++ b/lib/wanderer_app_web/live/map/event_handlers/map_core_event_handler.ex @@ -21,59 +21,85 @@ defmodule WandererAppWeb.MapCoreEventHandler do :refresh_permissions, %{assigns: %{current_user: current_user, map_slug: map_slug}} = socket ) do - {:ok, %{id: map_id, user_permissions: user_permissions, owner_id: owner_id}} = - map_slug - |> WandererApp.Api.Map.get_map_by_slug!() - |> Ash.load(:user_permissions, actor: current_user) + try do + {:ok, %{id: map_id, user_permissions: user_permissions, owner_id: owner_id}} = + map_slug + |> WandererApp.Api.Map.get_map_by_slug!() + |> Ash.load(:user_permissions, actor: current_user) - user_permissions = - WandererApp.Permissions.get_map_permissions( - user_permissions, - owner_id, - current_user.characters |> Enum.map(& &1.id) - ) + user_permissions = + WandererApp.Permissions.get_map_permissions( + user_permissions, + owner_id, + current_user.characters |> Enum.map(& &1.id) + ) - case user_permissions do - %{view_system: false} -> - socket - |> Phoenix.LiveView.put_flash(:error, "Your access to the map have been revoked.") - |> Phoenix.LiveView.push_navigate(to: ~p"/maps") + case user_permissions do + %{view_system: false} -> + socket + |> Phoenix.LiveView.put_flash(:error, "Your access to the map have been revoked.") + |> Phoenix.LiveView.push_navigate(to: ~p"/maps") - %{track_character: track_character} -> - {:ok, map_characters} = - case WandererApp.MapCharacterSettingsRepo.get_tracked_by_map_filtered( - map_id, - current_user.characters |> Enum.map(& &1.id) - ) do - {:ok, settings} -> - {:ok, - settings - |> Enum.map(fn s -> s |> Ash.load!(:character) |> Map.get(:character) end)} + %{track_character: track_character} -> + {:ok, map_characters} = + case WandererApp.MapCharacterSettingsRepo.get_tracked_by_map_filtered( + map_id, + current_user.characters |> Enum.map(& &1.id) + ) do + {:ok, settings} -> + {:ok, + settings + |> Enum.map(fn s -> s |> Ash.load!(:character) |> Map.get(:character) end)} + + _ -> + {:ok, []} + end + + case track_character do + false -> + :ok = WandererApp.Character.TrackingUtils.untrack(map_characters, map_id, self()) _ -> - {:ok, []} + :ok = + WandererApp.Character.TrackingUtils.track( + map_characters, + map_id, + true, + self() + ) end - case track_character do - false -> - :ok = WandererApp.Character.TrackingUtils.untrack(map_characters, map_id, self()) + socket + |> assign(user_permissions: user_permissions) + |> MapEventHandler.push_map_event( + "user_permissions", + user_permissions + ) + end + rescue + error in Ash.Error.Invalid.MultipleResults -> + Logger.error("Multiple maps found with slug '#{map_slug}' during refresh_permissions", + slug: map_slug, + error: inspect(error) + ) - _ -> - :ok = - WandererApp.Character.TrackingUtils.track( - map_characters, - map_id, - true, - self() - ) - end + # Emit telemetry for monitoring + :telemetry.execute( + [:wanderer_app, :map, :duplicate_slug_detected], + %{count: 1}, + %{slug: map_slug, operation: :refresh_permissions} + ) + + # Return socket unchanged - permissions won't refresh but won't crash + socket + + error -> + Logger.error("Error refreshing permissions for map slug '#{map_slug}'", + slug: map_slug, + error: inspect(error) + ) socket - |> assign(user_permissions: user_permissions) - |> MapEventHandler.push_map_event( - "user_permissions", - user_permissions - ) end end diff --git a/lib/wanderer_app_web/live/map/map_live.ex b/lib/wanderer_app_web/live/map/map_live.ex index a7c91580..1e7d9639 100644 --- a/lib/wanderer_app_web/live/map/map_live.ex +++ b/lib/wanderer_app_web/live/map/map_live.ex @@ -74,6 +74,13 @@ defmodule WandererAppWeb.MapLive do "You don't have main character set, please update it in tracking settings (top right icon)." )} + def handle_info(:map_deleted, socket), + do: + {:noreply, + socket + |> put_flash(:info, "This map has been deleted.") + |> push_navigate(to: ~p"/maps")} + def handle_info(:no_access, socket), do: {:noreply, diff --git a/lib/wanderer_app_web/live/maps/maps_live.ex b/lib/wanderer_app_web/live/maps/maps_live.ex index f63dea61..04f60ff5 100644 --- a/lib/wanderer_app_web/live/maps/maps_live.ex +++ b/lib/wanderer_app_web/live/maps/maps_live.ex @@ -1,6 +1,8 @@ defmodule WandererAppWeb.MapsLive do use WandererAppWeb, :live_view + alias Phoenix.LiveView.AsyncResult + require Logger @pubsub_client Application.compile_env(:wanderer_app, :pubsub_client) @@ -275,17 +277,57 @@ defmodule WandererAppWeb.MapsLive do :telemetry.execute([:wanderer_app, :map, :created], %{count: 1}) maybe_create_default_acl(form, new_map) + # Reload maps synchronously to avoid timing issues with flash messages + {:ok, %{maps: maps}} = load_maps(current_user) + {:noreply, socket - |> assign_async(:maps, fn -> - load_maps(current_user) - end) + |> put_flash( + :info, + "Map '#{new_map.name}' created successfully with slug '#{new_map.slug}'" + ) + |> assign(:maps, AsyncResult.ok(maps)) |> push_patch(to: ~p"/maps")} + {:error, %Ash.Error.Invalid{errors: errors}} -> + # Check for slug uniqueness constraint violation + slug_error = + Enum.find(errors, fn error -> + case error do + %{field: :slug} -> true + %{message: message} when is_binary(message) -> String.contains?(message, "unique") + _ -> false + end + end) + + error_message = + if slug_error do + "A map with this name already exists. The system will automatically adjust the name if needed. Please try again." + else + errors + |> Enum.map(fn error -> + field = Map.get(error, :field, "field") + message = Map.get(error, :message, "validation error") + "#{field}: #{message}" + end) + |> Enum.join(", ") + end + + Logger.warning("Map creation failed", + form: form, + errors: inspect(errors), + slug_error: slug_error != nil + ) + + {:noreply, + socket + |> put_flash(:error, "Failed to create map: #{error_message}") + |> assign(error: error_message)} + {:error, %{errors: errors}} -> error_message = errors - |> Enum.map(fn %{field: _field} = error -> + |> Enum.map(fn error -> "#{Map.get(error, :message, "Field validation error")}" end) |> Enum.join(", ") @@ -296,9 +338,14 @@ defmodule WandererAppWeb.MapsLive do |> assign(error: error_message)} {:error, error} -> + Logger.error("Unexpected error creating map", + form: form, + error: inspect(error) + ) + {:noreply, socket - |> put_flash(:error, "Failed to create map") + |> put_flash(:error, "Failed to create map. Please try again.") |> assign(error: error)} end end @@ -342,97 +389,156 @@ defmodule WandererAppWeb.MapsLive do %{"form" => form} = _params, %{assigns: %{map_slug: map_slug, current_user: current_user}} = socket ) do - {:ok, map} = - map_slug - |> WandererApp.Api.Map.get_map_by_slug!() - |> Ash.load(:acls) + case get_map_by_slug_safely(map_slug) do + {:ok, map} -> + # Successfully found the map, proceed with loading and updating + {:ok, map_with_acls} = Ash.load(map, :acls) - scope = - form - |> Map.get("scope") - |> case do - "" -> "wormholes" - scope -> scope - end + scope = + form + |> Map.get("scope") + |> case do + "" -> "wormholes" + scope -> scope + end - form = - form - |> Map.put("acls", form["acls"] || []) - |> Map.put("scope", scope) - |> Map.put( - "only_tracked_characters", - (form["only_tracked_characters"] || "false") |> String.to_existing_atom() - ) + form = + form + |> Map.put("acls", form["acls"] || []) + |> Map.put("scope", scope) + |> Map.put( + "only_tracked_characters", + (form["only_tracked_characters"] || "false") |> String.to_existing_atom() + ) - map - |> WandererApp.Api.Map.update(form) - |> case do - {:ok, _updated_map} -> - {added_acls, removed_acls} = map.acls |> Enum.map(& &1.id) |> _get_acls_diff(form["acls"]) + map_with_acls + |> WandererApp.Api.Map.update(form) + |> case do + {:ok, _updated_map} -> + {added_acls, removed_acls} = + map_with_acls.acls |> Enum.map(& &1.id) |> _get_acls_diff(form["acls"]) - Phoenix.PubSub.broadcast( - WandererApp.PubSub, - "maps:#{map.id}", - {:map_acl_updated, map.id, added_acls, removed_acls} - ) + Phoenix.PubSub.broadcast( + WandererApp.PubSub, + "maps:#{map_with_acls.id}", + {:map_acl_updated, map_with_acls.id, added_acls, removed_acls} + ) - {:ok, tracked_characters} = - WandererApp.Maps.get_tracked_map_characters(map.id, current_user) + {:ok, tracked_characters} = + WandererApp.Maps.get_tracked_map_characters(map_with_acls.id, current_user) - first_tracked_character_id = Enum.map(tracked_characters, & &1.id) |> List.first() + first_tracked_character_id = Enum.map(tracked_characters, & &1.id) |> List.first() - added_acls - |> Enum.each(fn acl_id -> - WandererApp.User.ActivityTracker.track_map_event(:map_acl_added, %{ - character_id: first_tracked_character_id, - user_id: current_user.id, - map_id: map.id, - acl_id: acl_id - }) - end) + added_acls + |> Enum.each(fn acl_id -> + WandererApp.User.ActivityTracker.track_map_event(:map_acl_added, %{ + character_id: first_tracked_character_id, + user_id: current_user.id, + map_id: map_with_acls.id, + acl_id: acl_id + }) + end) - removed_acls - |> Enum.each(fn acl_id -> - WandererApp.User.ActivityTracker.track_map_event(:map_acl_removed, %{ - character_id: first_tracked_character_id, - user_id: current_user.id, - map_id: map.id, - acl_id: acl_id - }) - end) + removed_acls + |> Enum.each(fn acl_id -> + WandererApp.User.ActivityTracker.track_map_event(:map_acl_removed, %{ + character_id: first_tracked_character_id, + user_id: current_user.id, + map_id: map_with_acls.id, + acl_id: acl_id + }) + end) + {:noreply, + socket + |> push_navigate(to: ~p"/maps")} + + {:error, error} -> + {:noreply, + socket + |> put_flash(:error, "Failed to update map") + |> assign(error: error)} + end + + {:error, :multiple_results} -> {:noreply, socket + |> put_flash( + :error, + "Multiple maps found with this identifier. Please contact support to resolve this issue." + ) |> push_navigate(to: ~p"/maps")} - {:error, error} -> + {:error, :not_found} -> {:noreply, socket - |> put_flash(:error, "Failed to update map") - |> assign(error: error)} + |> put_flash(:error, "Map not found") + |> push_navigate(to: ~p"/maps")} + + {:error, _reason} -> + {:noreply, + socket + |> put_flash(:error, "Failed to load map. Please try again.") + |> push_navigate(to: ~p"/maps")} end end def handle_event("delete", %{"data" => map_slug} = _params, socket) do - map = - map_slug - |> WandererApp.Api.Map.get_map_by_slug!() - |> WandererApp.Api.Map.mark_as_deleted!() + case get_map_by_slug_safely(map_slug) do + {:ok, map} -> + # Successfully found the map, proceed with deletion + deleted_map = WandererApp.Api.Map.mark_as_deleted!(map) - Phoenix.PubSub.broadcast( - WandererApp.PubSub, - "maps:#{map.id}", - :map_deleted - ) + Phoenix.PubSub.broadcast( + WandererApp.PubSub, + "maps:#{deleted_map.id}", + :map_deleted + ) - current_user = socket.assigns.current_user + current_user = socket.assigns.current_user - {:noreply, - socket - |> assign_async(:maps, fn -> - load_maps(current_user) - end) - |> push_patch(to: ~p"/maps")} + # Reload maps synchronously to avoid timing issues with flash messages + {:ok, %{maps: maps}} = load_maps(current_user) + + {:noreply, + socket + |> assign(:maps, AsyncResult.ok(maps)) + |> push_patch(to: ~p"/maps")} + + {:error, :multiple_results} -> + # Multiple maps found with this slug - data integrity issue + # Reload maps synchronously + {:ok, %{maps: maps}} = load_maps(socket.assigns.current_user) + + {:noreply, + socket + |> put_flash( + :error, + "Multiple maps found with this identifier. Please contact support to resolve this issue." + ) + |> assign(:maps, AsyncResult.ok(maps))} + + {:error, :not_found} -> + # Map not found + # Reload maps synchronously + {:ok, %{maps: maps}} = load_maps(socket.assigns.current_user) + + {:noreply, + socket + |> put_flash(:error, "Map not found or already deleted") + |> assign(:maps, AsyncResult.ok(maps)) + |> push_patch(to: ~p"/maps")} + + {:error, _reason} -> + # Other error + # Reload maps synchronously + {:ok, %{maps: maps}} = load_maps(socket.assigns.current_user) + + {:noreply, + socket + |> put_flash(:error, "Failed to delete map. Please try again.") + |> assign(:maps, AsyncResult.ok(maps))} + end end def handle_event( @@ -681,4 +787,49 @@ defmodule WandererAppWeb.MapsLive do map |> Map.put(:acls, acls |> Enum.map(&map_acl/1)) end + + @doc """ + Safely retrieves a map by slug, handling the case where multiple maps + with the same slug exist (database integrity issue). + + Returns: + - `{:ok, map}` - Single map found + - `{:error, :multiple_results}` - Multiple maps found (logs error) + - `{:error, :not_found}` - No map found + - `{:error, reason}` - Other error + """ + defp get_map_by_slug_safely(slug) do + try do + map = WandererApp.Api.Map.get_map_by_slug!(slug) + {:ok, map} + rescue + error in Ash.Error.Invalid.MultipleResults -> + Logger.error("Multiple maps found with slug '#{slug}' - database integrity issue", + slug: slug, + error: inspect(error) + ) + + # Emit telemetry for monitoring + :telemetry.execute( + [:wanderer_app, :map, :duplicate_slug_detected], + %{count: 1}, + %{slug: slug, operation: :get_by_slug} + ) + + # Return error - caller should handle this appropriately + {:error, :multiple_results} + + error in Ash.Error.Query.NotFound -> + Logger.debug("Map not found with slug: #{slug}") + {:error, :not_found} + + error -> + Logger.error("Error retrieving map by slug", + slug: slug, + error: inspect(error) + ) + + {:error, :unknown_error} + end + end end diff --git a/priv/repo/migrations/20251114184420_ensure_no_duplicate_map_slugs.exs b/priv/repo/migrations/20251114184420_ensure_no_duplicate_map_slugs.exs new file mode 100644 index 00000000..161dead0 --- /dev/null +++ b/priv/repo/migrations/20251114184420_ensure_no_duplicate_map_slugs.exs @@ -0,0 +1,212 @@ +defmodule WandererApp.Repo.Migrations.EnsureNoDuplicateMapSlugs do + @moduledoc """ + Final migration to ensure all duplicate map slugs are removed and unique index exists. + + This migration: + 1. Checks for any remaining duplicate slugs + 2. Fixes duplicates by renaming them (keeps oldest, renames newer ones) + 3. Ensures unique index exists on maps_v1.slug + 4. Verifies no duplicates remain after migration + + Safe to run multiple times (idempotent). + """ + use Ecto.Migration + import Ecto.Query + require Logger + + def up do + IO.puts("\n=== Starting Map Slug Deduplication Migration ===\n") + + # Step 1: Check for duplicates + duplicate_count = count_duplicates() + + if duplicate_count > 0 do + IO.puts("Found #{duplicate_count} duplicate slug(s) - proceeding with cleanup...") + + # Step 2: Drop index temporarily if it exists (to allow updates) + drop_index_if_exists() + + # Step 3: Fix all duplicates + fix_duplicate_slugs() + + # Step 4: Recreate unique index + ensure_unique_index() + else + IO.puts("No duplicate slugs found - ensuring unique index exists...") + ensure_unique_index() + end + + # Step 5: Final verification + verify_no_duplicates() + + IO.puts("\n=== Migration completed successfully! ===\n") + end + + def down do + # This migration is idempotent and only fixes data integrity issues + # No need to revert as it doesn't change schema in a harmful way + IO.puts("This migration does not need to be reverted") + :ok + end + + defp count_duplicates do + duplicates_query = """ + SELECT COUNT(*) as duplicate_count + FROM ( + SELECT slug + FROM maps_v1 + WHERE deleted = false + GROUP BY slug + HAVING COUNT(*) > 1 + ) duplicates + """ + + case repo().query(duplicates_query, []) do + {:ok, %{rows: [[count]]}} -> + count + {:error, error} -> + IO.puts("Error counting duplicates: #{inspect(error)}") + 0 + end + end + + defp drop_index_if_exists do + index_exists_query = """ + SELECT EXISTS ( + SELECT 1 + FROM pg_indexes + WHERE tablename = 'maps_v1' + AND indexname = 'maps_v1_unique_slug_index' + ) + """ + + case repo().query(index_exists_query, []) do + {:ok, %{rows: [[true]]}} -> + IO.puts("Temporarily dropping unique index to allow updates...") + execute("DROP INDEX IF EXISTS maps_v1_unique_slug_index") + IO.puts("✓ Index dropped") + + {:ok, %{rows: [[false]]}} -> + IO.puts("No existing index to drop") + + {:error, error} -> + IO.puts("Error checking index: #{inspect(error)}") + end + end + + defp fix_duplicate_slugs do + # Get all duplicate slugs with their IDs and timestamps + # Order by inserted_at to keep the oldest one unchanged + duplicates_query = """ + SELECT + slug, + array_agg(id::text ORDER BY inserted_at ASC, id ASC) as ids, + array_agg(name ORDER BY inserted_at ASC, id ASC) as names + FROM maps_v1 + WHERE deleted = false + GROUP BY slug + HAVING COUNT(*) > 1 + ORDER BY slug + """ + + case repo().query(duplicates_query, []) do + {:ok, %{rows: rows}} when length(rows) > 0 -> + IO.puts("\nFixing #{length(rows)} duplicate slug(s)...") + + Enum.each(rows, fn [slug, ids, names] -> + IO.puts("\n Processing: '#{slug}' (#{length(ids)} duplicates)") + + # Keep the first one (oldest by inserted_at), rename the rest + [keep_id | rename_ids] = ids + [keep_name | rename_names] = names + + IO.puts(" ✓ Keeping: #{keep_id} - '#{keep_name}'") + + # Rename duplicates + rename_ids + |> Enum.zip(rename_names) + |> Enum.with_index(2) + |> Enum.each(fn {{id_string, name}, n} -> + new_slug = generate_unique_slug(slug, n) + + # Use parameterized query for safety + update_query = "UPDATE maps_v1 SET slug = $1 WHERE id::text = $2" + repo().query!(update_query, [new_slug, id_string]) + + IO.puts(" → Renamed: #{id_string} - '#{name}' to slug '#{new_slug}'") + end) + end) + + IO.puts("\n✓ All duplicate slugs fixed!") + + {:ok, %{rows: []}} -> + IO.puts("No duplicate slugs to fix") + + {:error, error} -> + IO.puts("Error finding duplicates: #{inspect(error)}") + raise "Failed to query duplicate slugs: #{inspect(error)}" + end + end + + defp generate_unique_slug(base_slug, n) when n >= 2 do + candidate = "#{base_slug}-#{n}" + + # Check if this slug already exists + check_query = "SELECT COUNT(*) FROM maps_v1 WHERE slug = $1 AND deleted = false" + + case repo().query!(check_query, [candidate]) do + %{rows: [[0]]} -> + candidate + %{rows: [[_count]]} -> + # Try next number + generate_unique_slug(base_slug, n + 1) + end + end + + defp ensure_unique_index do + # Check if index exists + index_exists_query = """ + SELECT EXISTS ( + SELECT 1 + FROM pg_indexes + WHERE tablename = 'maps_v1' + AND indexname = 'maps_v1_unique_slug_index' + ) + """ + + case repo().query(index_exists_query, []) do + {:ok, %{rows: [[true]]}} -> + IO.puts("✓ Unique index on slug already exists") + + {:ok, %{rows: [[false]]}} -> + IO.puts("Creating unique index on slug column...") + + create_if_not_exists( + index(:maps_v1, [:slug], + unique: true, + name: :maps_v1_unique_slug_index, + where: "deleted = false" + ) + ) + + IO.puts("✓ Unique index created successfully!") + + {:error, error} -> + IO.puts("Error checking index: #{inspect(error)}") + raise "Failed to check index existence: #{inspect(error)}" + end + end + + defp verify_no_duplicates do + IO.puts("\nVerifying no duplicates remain...") + + remaining_duplicates = count_duplicates() + + if remaining_duplicates > 0 do + IO.puts("❌ ERROR: #{remaining_duplicates} duplicate(s) still exist!") + raise "Migration failed: duplicates still exist after cleanup" + else + IO.puts("✓ Verification passed: No duplicates found") + end + end +end diff --git a/test/integration/map/map_pool_crash_integration_test.exs b/test/integration/map/map_pool_crash_integration_test.exs new file mode 100644 index 00000000..e175fa99 --- /dev/null +++ b/test/integration/map/map_pool_crash_integration_test.exs @@ -0,0 +1,463 @@ +defmodule WandererApp.Map.MapPoolCrashIntegrationTest do + @moduledoc """ + Integration tests for MapPool crash recovery. + + These tests verify end-to-end crash recovery behavior including: + - MapPool GenServer crashes and restarts + - State recovery from ETS + - Registry and cache consistency after recovery + - Telemetry events during recovery + - Multi-pool scenarios + + Note: Many tests are skipped as they require full map infrastructure + (database, Server.Impl, map data, etc.) to be set up. + """ + + use ExUnit.Case, async: false + + alias WandererApp.Map.{MapPool, MapPoolDynamicSupervisor, MapPoolState} + + @cache :map_pool_cache + @registry :map_pool_registry + @unique_registry :unique_map_pool_registry + @ets_table :map_pool_state_table + + setup do + # Clean up any existing test data + cleanup_test_data() + + # Check if required infrastructure is running + supervisor_running? = Process.whereis(MapPoolDynamicSupervisor) != nil + + ets_exists? = + try do + :ets.info(@ets_table) != :undefined + rescue + _ -> false + end + + on_exit(fn -> + cleanup_test_data() + end) + + {:ok, supervisor_running: supervisor_running?, ets_exists: ets_exists?} + end + + defp cleanup_test_data do + # Clean up test caches + WandererApp.Cache.delete("started_maps") + Cachex.clear(@cache) + + # Clean up ETS entries + if :ets.whereis(@ets_table) != :undefined do + :ets.match_delete(@ets_table, {:"$1", :"$2", :"$3"}) + end + end + + defp find_pool_pid(uuid) do + pool_name = Module.concat(MapPool, uuid) + + case Registry.lookup(@unique_registry, pool_name) do + [{pid, _value}] -> {:ok, pid} + [] -> {:error, :not_found} + end + end + + describe "End-to-end crash recovery" do + @tag :skip + @tag :integration + test "MapPool recovers all maps after abnormal crash" do + # This test would: + # 1. Start a MapPool with test maps via MapPoolDynamicSupervisor + # 2. Verify maps are running and state is in ETS + # 3. Simulate crash using GenServer.call(pool_pid, :error) + # 4. Wait for supervisor to restart the pool + # 5. Verify all maps are recovered + # 6. Verify Registry, Cache, and ETS are consistent + + # Requires: + # - Test map data in database + # - Server.Impl.start_map to work with test data + # - Full supervision tree running + + :ok + end + + @tag :skip + @tag :integration + test "MapPool preserves ETS state on abnormal termination" do + # This test would: + # 1. Start a MapPool with maps + # 2. Force crash + # 3. Verify ETS state is preserved (not deleted) + # 4. Verify new pool instance recovers from ETS + + :ok + end + + @tag :skip + @tag :integration + test "MapPool cleans ETS state on graceful shutdown" do + # This test would: + # 1. Start a MapPool with maps + # 2. Gracefully stop the pool (GenServer.cast(pool_pid, :stop)) + # 3. Verify ETS state is deleted + # 4. Verify new pool starts with empty state + + :ok + end + end + + describe "Multi-pool crash scenarios" do + @tag :skip + @tag :integration + test "multiple pools crash and recover independently" do + # This test would: + # 1. Start multiple MapPool instances with different maps + # 2. Crash one pool + # 3. Verify only that pool recovers, others unaffected + # 4. Verify no cross-pool state corruption + + :ok + end + + @tag :skip + @tag :integration + test "concurrent pool crashes don't corrupt recovery state" do + # This test would: + # 1. Start multiple pools + # 2. Crash multiple pools simultaneously + # 3. Verify all pools recover correctly + # 4. Verify no ETS corruption or race conditions + + :ok + end + end + + describe "State consistency after recovery" do + @tag :skip + @tag :integration + test "Registry state matches recovered state" do + # This test would verify that after recovery: + # - unique_registry has correct map_ids for pool UUID + # - map_pool_registry has correct pool UUID entry + # - All map_ids in Registry match ETS state + + :ok + end + + @tag :skip + @tag :integration + test "Cache state matches recovered state" do + # This test would verify that after recovery: + # - map_pool_cache has correct map_id -> uuid mappings + # - started_maps cache includes all recovered maps + # - No orphaned cache entries + + :ok + end + + @tag :skip + @tag :integration + test "Map servers are actually running after recovery" do + # This test would: + # 1. Recover maps from crash + # 2. Verify each map's GenServer is actually running + # 3. Verify maps respond to requests + # 4. Verify map state is correct + + :ok + end + end + + describe "Recovery failure handling" do + @tag :skip + @tag :integration + test "recovery continues when individual map fails to start" do + # This test would: + # 1. Save state with maps [1, 2, 3] to ETS + # 2. Delete map 2 from database + # 3. Trigger recovery + # 4. Verify maps 1 and 3 recover successfully + # 5. Verify map 2 failure is logged and telemetry emitted + # 6. Verify pool continues with maps [1, 3] + + :ok + end + + @tag :skip + @tag :integration + test "recovery handles maps already running in different pool" do + # This test would simulate a race condition where: + # 1. Pool A crashes with map X + # 2. Before recovery, map X is started in Pool B + # 3. Pool A tries to recover map X + # 4. Verify conflict is detected and handled gracefully + + :ok + end + + @tag :skip + @tag :integration + test "recovery handles corrupted ETS state" do + # This test would: + # 1. Manually corrupt ETS state (invalid map IDs, wrong types, etc.) + # 2. Trigger recovery + # 3. Verify pool handles corruption gracefully + # 4. Verify telemetry emitted for failures + # 5. Verify pool continues with valid maps only + + :ok + end + end + + describe "Telemetry during recovery" do + test "telemetry events emitted in correct order", %{ets_exists: ets_exists?} do + if ets_exists? do + test_pid = self() + events = [] + + # Attach handlers for all recovery events + :telemetry.attach_many( + "test-recovery-events", + [ + [:wanderer_app, :map_pool, :recovery, :start], + [:wanderer_app, :map_pool, :recovery, :complete], + [:wanderer_app, :map_pool, :recovery, :map_failed] + ], + fn event, measurements, metadata, _config -> + send(test_pid, {:telemetry_event, event, measurements, metadata}) + end, + nil + ) + + uuid = "test-pool-#{:rand.uniform(1_000_000)}" + + # Simulate recovery sequence + # 1. Start event + :telemetry.execute( + [:wanderer_app, :map_pool, :recovery, :start], + %{recovered_map_count: 3, total_map_count: 3}, + %{pool_uuid: uuid} + ) + + # 2. Complete event (in real recovery, this comes after all maps start) + :telemetry.execute( + [:wanderer_app, :map_pool, :recovery, :complete], + %{recovered_count: 3, failed_count: 0, duration_ms: 100}, + %{pool_uuid: uuid} + ) + + # Verify we received both events + assert_receive {:telemetry_event, [:wanderer_app, :map_pool, :recovery, :start], _, _}, + 500 + + assert_receive {:telemetry_event, [:wanderer_app, :map_pool, :recovery, :complete], _, _}, + 500 + + :telemetry.detach("test-recovery-events") + else + :ok + end + end + + @tag :skip + @tag :integration + test "telemetry includes accurate recovery statistics" do + # This test would verify that: + # - recovered_map_count matches actual recovered maps + # - failed_count matches actual failed maps + # - duration_ms is accurate + # - All metadata is correct + + :ok + end + end + + describe "Interaction with Reconciler" do + @tag :skip + @tag :integration + test "Reconciler doesn't interfere with crash recovery" do + # This test would: + # 1. Crash a pool with maps + # 2. Trigger both recovery and reconciliation + # 3. Verify they don't conflict + # 4. Verify final state is consistent + + :ok + end + + @tag :skip + @tag :integration + test "Reconciler detects failed recovery" do + # This test would: + # 1. Crash a pool with map X + # 2. Make recovery fail for map X + # 3. Run reconciler + # 4. Verify reconciler detects and potentially fixes the issue + + :ok + end + end + + describe "Edge cases" do + @tag :skip + @tag :integration + test "recovery during pool at capacity" do + # This test would: + # 1. Create pool with 19 maps + # 2. Crash pool while adding 20th map + # 3. Verify recovery handles capacity limit + # 4. Verify all maps start or overflow is handled + + :ok + end + + @tag :skip + @tag :integration + test "recovery with empty map list" do + # This test would: + # 1. Crash pool with empty map_ids + # 2. Verify recovery completes successfully + # 3. Verify pool starts with no maps + + :ok + end + + @tag :skip + @tag :integration + test "multiple crashes in quick succession" do + # This test would: + # 1. Crash pool + # 2. Immediately crash again during recovery + # 3. Verify supervisor's max_restarts is respected + # 4. Verify state remains consistent + + :ok + end + end + + describe "Performance under load" do + @tag :slow + @tag :skip + @tag :integration + test "recovery completes within 2 seconds for 20 maps" do + # This test would: + # 1. Create pool with 20 maps (pool limit) + # 2. Crash pool + # 3. Measure time to full recovery + # 4. Assert recovery < 2 seconds + + :ok + end + + @tag :slow + @tag :skip + @tag :integration + test "recovery doesn't block other pools" do + # This test would: + # 1. Start multiple pools + # 2. Crash one pool with many maps + # 3. Verify other pools continue to operate normally during recovery + # 4. Measure performance impact on healthy pools + + :ok + end + end + + describe "Supervisor interaction" do + test "ETS table survives individual pool crash", %{ets_exists: ets_exists?} do + if ets_exists? do + # Verify ETS table is owned by supervisor, not individual pools + table_info = :ets.info(@ets_table) + owner_pid = Keyword.get(table_info, :owner) + + # Owner should be alive and be the supervisor or a system process + assert Process.alive?(owner_pid) + + # Verify we can still access the table + uuid = "test-pool-#{:rand.uniform(1_000_000)}" + MapPoolState.save_pool_state(uuid, [1, 2, 3]) + assert {:ok, [1, 2, 3]} = MapPoolState.get_pool_state(uuid) + else + :ok + end + end + + @tag :skip + @tag :integration + test "supervisor restarts pool after crash" do + # This test would: + # 1. Start a pool via DynamicSupervisor + # 2. Crash the pool + # 3. Verify supervisor restarts it + # 4. Verify new PID is different from old PID + # 5. Verify pool is functional after restart + + :ok + end + end + + describe "Database consistency" do + @tag :skip + @tag :integration + test "recovered maps load latest state from database" do + # This test would: + # 1. Start maps with initial state + # 2. Modify map state in database + # 3. Crash pool + # 4. Verify recovered maps have latest database state + + :ok + end + + @tag :skip + @tag :integration + test "recovery uses MapState for map configuration" do + # This test would: + # 1. Verify recovery calls WandererApp.Map.get_map_state!/1 + # 2. Verify state comes from database MapState table + # 3. Verify maps start with correct configuration + + :ok + end + end + + describe "Real-world scenarios" do + @tag :skip + @tag :integration + test "recovery after OOM crash" do + # This test would simulate recovery after out-of-memory crash: + # 1. Start pool with maps + # 2. Simulate OOM condition + # 3. Verify recovery completes successfully + # 4. Verify no memory leaks after recovery + + :ok + end + + @tag :skip + @tag :integration + test "recovery after network partition" do + # This test would simulate recovery after network issues: + # 1. Start maps with external dependencies + # 2. Simulate network partition + # 3. Crash pool + # 4. Verify recovery handles network errors gracefully + + :ok + end + + @tag :skip + @tag :integration + test "recovery preserves user sessions" do + # This test would: + # 1. Start maps with active user sessions + # 2. Crash pool + # 3. Verify users can continue after recovery + # 4. Verify presence tracking works after recovery + + :ok + end + end +end diff --git a/test/unit/map/map_pool_crash_recovery_test.exs b/test/unit/map/map_pool_crash_recovery_test.exs new file mode 100644 index 00000000..8c55e54c --- /dev/null +++ b/test/unit/map/map_pool_crash_recovery_test.exs @@ -0,0 +1,561 @@ +defmodule WandererApp.Map.MapPoolCrashRecoveryTest do + use ExUnit.Case, async: false + + alias WandererApp.Map.{MapPool, MapPoolState} + + @cache :map_pool_cache + @registry :map_pool_registry + @unique_registry :unique_map_pool_registry + @ets_table :map_pool_state_table + + setup do + # Clean up any existing test data + cleanup_test_data() + + # Check if ETS table exists + ets_exists? = + try do + :ets.info(@ets_table) != :undefined + rescue + _ -> false + end + + on_exit(fn -> + cleanup_test_data() + end) + + {:ok, ets_exists: ets_exists?} + end + + defp cleanup_test_data do + # Clean up test caches + WandererApp.Cache.delete("started_maps") + Cachex.clear(@cache) + + # Clean up ETS entries for test pools + if :ets.whereis(@ets_table) != :undefined do + :ets.match_delete(@ets_table, {:"$1", :"$2", :"$3"}) + end + end + + defp create_test_pool_with_uuid(uuid, map_ids) do + # Manually register in unique_registry + {:ok, _} = Registry.register(@unique_registry, Module.concat(MapPool, uuid), map_ids) + {:ok, _} = Registry.register(@registry, MapPool, uuid) + + # Add to cache + Enum.each(map_ids, fn map_id -> + Cachex.put(@cache, map_id, uuid) + end) + + # Save to ETS + MapPoolState.save_pool_state(uuid, map_ids) + + uuid + end + + defp get_pool_map_ids(uuid) do + case Registry.lookup(@unique_registry, Module.concat(MapPool, uuid)) do + [{_pid, map_ids}] -> map_ids + [] -> [] + end + end + + describe "MapPoolState - ETS operations" do + test "save_pool_state stores state in ETS", %{ets_exists: ets_exists?} do + if ets_exists? do + uuid = "test-pool-#{:rand.uniform(1_000_000)}" + map_ids = [1, 2, 3] + + assert :ok = MapPoolState.save_pool_state(uuid, map_ids) + + # Verify it's in ETS + assert {:ok, ^map_ids} = MapPoolState.get_pool_state(uuid) + else + :ok + end + end + + test "get_pool_state returns not_found for non-existent pool", %{ets_exists: ets_exists?} do + if ets_exists? do + uuid = "non-existent-#{:rand.uniform(1_000_000)}" + + assert {:error, :not_found} = MapPoolState.get_pool_state(uuid) + else + :ok + end + end + + test "delete_pool_state removes state from ETS", %{ets_exists: ets_exists?} do + if ets_exists? do + uuid = "test-pool-#{:rand.uniform(1_000_000)}" + map_ids = [1, 2, 3] + + MapPoolState.save_pool_state(uuid, map_ids) + assert {:ok, ^map_ids} = MapPoolState.get_pool_state(uuid) + + assert :ok = MapPoolState.delete_pool_state(uuid) + assert {:error, :not_found} = MapPoolState.get_pool_state(uuid) + else + :ok + end + end + + test "save_pool_state updates existing state", %{ets_exists: ets_exists?} do + if ets_exists? do + uuid = "test-pool-#{:rand.uniform(1_000_000)}" + + # Save initial state + MapPoolState.save_pool_state(uuid, [1, 2]) + assert {:ok, [1, 2]} = MapPoolState.get_pool_state(uuid) + + # Update state + MapPoolState.save_pool_state(uuid, [1, 2, 3, 4]) + assert {:ok, [1, 2, 3, 4]} = MapPoolState.get_pool_state(uuid) + else + :ok + end + end + + test "list_all_states returns all pool states", %{ets_exists: ets_exists?} do + if ets_exists? do + # Clean first + :ets.delete_all_objects(@ets_table) + + uuid1 = "test-pool-1-#{:rand.uniform(1_000_000)}" + uuid2 = "test-pool-2-#{:rand.uniform(1_000_000)}" + + MapPoolState.save_pool_state(uuid1, [1, 2]) + MapPoolState.save_pool_state(uuid2, [3, 4]) + + states = MapPoolState.list_all_states() + assert length(states) >= 2 + + # Verify our pools are in there + uuids = Enum.map(states, fn {uuid, _map_ids, _timestamp} -> uuid end) + assert uuid1 in uuids + assert uuid2 in uuids + else + :ok + end + end + + test "count_states returns correct count", %{ets_exists: ets_exists?} do + if ets_exists? do + # Clean first + :ets.delete_all_objects(@ets_table) + + uuid1 = "test-pool-1-#{:rand.uniform(1_000_000)}" + uuid2 = "test-pool-2-#{:rand.uniform(1_000_000)}" + + MapPoolState.save_pool_state(uuid1, [1, 2]) + MapPoolState.save_pool_state(uuid2, [3, 4]) + + count = MapPoolState.count_states() + assert count >= 2 + else + :ok + end + end + end + + describe "MapPoolState - stale entry cleanup" do + test "cleanup_stale_entries removes old entries", %{ets_exists: ets_exists?} do + if ets_exists? do + uuid = "stale-pool-#{:rand.uniform(1_000_000)}" + + # Manually insert a stale entry (24+ hours old) + stale_timestamp = System.system_time(:second) - 25 * 3600 + :ets.insert(@ets_table, {uuid, [1, 2], stale_timestamp}) + + assert {:ok, [1, 2]} = MapPoolState.get_pool_state(uuid) + + # Clean up stale entries + {:ok, deleted_count} = MapPoolState.cleanup_stale_entries() + assert deleted_count >= 1 + + # Verify stale entry was removed + assert {:error, :not_found} = MapPoolState.get_pool_state(uuid) + else + :ok + end + end + + test "cleanup_stale_entries preserves recent entries", %{ets_exists: ets_exists?} do + if ets_exists? do + uuid = "recent-pool-#{:rand.uniform(1_000_000)}" + map_ids = [1, 2, 3] + + # Save recent entry + MapPoolState.save_pool_state(uuid, map_ids) + + # Clean up + MapPoolState.cleanup_stale_entries() + + # Recent entry should still exist + assert {:ok, ^map_ids} = MapPoolState.get_pool_state(uuid) + else + :ok + end + end + end + + describe "Crash recovery - basic scenarios" do + @tag :skip + test "MapPool recovers single map after crash" do + # This test requires a full MapPool GenServer with actual map data + # Skipping as it needs integration with Server.Impl.start_map + :ok + end + + @tag :skip + test "MapPool recovers multiple maps after crash" do + # Similar to above - requires full integration + :ok + end + + @tag :skip + test "MapPool merges new and recovered map_ids" do + # Tests that if pool crashes while starting a new map, + # both the new map and recovered maps are started + :ok + end + end + + describe "Crash recovery - telemetry" do + test "recovery emits start telemetry event", %{ets_exists: ets_exists?} do + if ets_exists? do + test_pid = self() + + # Attach telemetry handler + :telemetry.attach( + "test-recovery-start", + [:wanderer_app, :map_pool, :recovery, :start], + fn _event, measurements, metadata, _config -> + send(test_pid, {:telemetry_start, measurements, metadata}) + end, + nil + ) + + uuid = "test-pool-#{:rand.uniform(1_000_000)}" + recovered_maps = [1, 2, 3] + + # Save state to ETS (simulating previous run) + MapPoolState.save_pool_state(uuid, recovered_maps) + + # Simulate init with recovery + # Note: Can't actually start a MapPool here without full integration, + # but we can verify the telemetry handler is set up correctly + + # Manually emit the event to test handler + :telemetry.execute( + [:wanderer_app, :map_pool, :recovery, :start], + %{recovered_map_count: 3, total_map_count: 3}, + %{pool_uuid: uuid} + ) + + assert_receive {:telemetry_start, measurements, metadata}, 500 + + assert measurements.recovered_map_count == 3 + assert measurements.total_map_count == 3 + assert metadata.pool_uuid == uuid + + # Cleanup + :telemetry.detach("test-recovery-start") + else + :ok + end + end + + test "recovery emits complete telemetry event", %{ets_exists: ets_exists?} do + if ets_exists? do + test_pid = self() + + :telemetry.attach( + "test-recovery-complete", + [:wanderer_app, :map_pool, :recovery, :complete], + fn _event, measurements, metadata, _config -> + send(test_pid, {:telemetry_complete, measurements, metadata}) + end, + nil + ) + + uuid = "test-pool-#{:rand.uniform(1_000_000)}" + + # Manually emit the event + :telemetry.execute( + [:wanderer_app, :map_pool, :recovery, :complete], + %{recovered_count: 3, failed_count: 0, duration_ms: 100}, + %{pool_uuid: uuid} + ) + + assert_receive {:telemetry_complete, measurements, metadata}, 500 + + assert measurements.recovered_count == 3 + assert measurements.failed_count == 0 + assert measurements.duration_ms == 100 + assert metadata.pool_uuid == uuid + + :telemetry.detach("test-recovery-complete") + else + :ok + end + end + + test "recovery emits map_failed telemetry event", %{ets_exists: ets_exists?} do + if ets_exists? do + test_pid = self() + + :telemetry.attach( + "test-recovery-map-failed", + [:wanderer_app, :map_pool, :recovery, :map_failed], + fn _event, measurements, metadata, _config -> + send(test_pid, {:telemetry_map_failed, measurements, metadata}) + end, + nil + ) + + uuid = "test-pool-#{:rand.uniform(1_000_000)}" + failed_map_id = 123 + + # Manually emit the event + :telemetry.execute( + [:wanderer_app, :map_pool, :recovery, :map_failed], + %{map_id: failed_map_id}, + %{pool_uuid: uuid, reason: "Map not found"} + ) + + assert_receive {:telemetry_map_failed, measurements, metadata}, 500 + + assert measurements.map_id == failed_map_id + assert metadata.pool_uuid == uuid + assert metadata.reason == "Map not found" + + :telemetry.detach("test-recovery-map-failed") + else + :ok + end + end + end + + describe "Crash recovery - state persistence" do + @tag :skip + test "state persisted after successful map start" do + # Would need to start actual MapPool and trigger start_map + :ok + end + + @tag :skip + test "state persisted after successful map stop" do + # Would need to start actual MapPool and trigger stop_map + :ok + end + + @tag :skip + test "state persisted during backup_state" do + # Would need to trigger backup_state handler + :ok + end + end + + describe "Graceful shutdown cleanup" do + test "ETS state cleaned on normal termination", %{ets_exists: ets_exists?} do + if ets_exists? do + uuid = "test-pool-#{:rand.uniform(1_000_000)}" + map_ids = [1, 2, 3] + + # Save state + MapPoolState.save_pool_state(uuid, map_ids) + assert {:ok, ^map_ids} = MapPoolState.get_pool_state(uuid) + + # Simulate graceful shutdown by calling delete + MapPoolState.delete_pool_state(uuid) + + # State should be gone + assert {:error, :not_found} = MapPoolState.get_pool_state(uuid) + else + :ok + end + end + + @tag :skip + test "ETS state preserved on abnormal termination" do + # Would need to actually crash a MapPool to test this + # The terminate callback would not call delete_pool_state + :ok + end + end + + describe "Edge cases" do + test "recovery with empty map_ids list", %{ets_exists: ets_exists?} do + if ets_exists? do + uuid = "test-pool-#{:rand.uniform(1_000_000)}" + + # Save empty state + MapPoolState.save_pool_state(uuid, []) + assert {:ok, []} = MapPoolState.get_pool_state(uuid) + else + :ok + end + end + + test "recovery with duplicate map_ids gets deduplicated", %{ets_exists: ets_exists?} do + if ets_exists? do + # This tests the deduplication logic in init + # If we have [1, 2] in ETS and [2, 3] in new map_ids, + # result should be [1, 2, 3] after Enum.uniq + + recovered_maps = [1, 2] + new_maps = [2, 3] + expected = Enum.uniq(recovered_maps ++ new_maps) + + # Should be [1, 2, 3] or [2, 3, 1] depending on order + assert 1 in expected + assert 2 in expected + assert 3 in expected + assert length(expected) == 3 + else + :ok + end + end + + test "large number of maps in recovery", %{ets_exists: ets_exists?} do + if ets_exists? do + uuid = "test-pool-#{:rand.uniform(1_000_000)}" + # Test with 20 maps (the pool limit) + map_ids = Enum.to_list(1..20) + + MapPoolState.save_pool_state(uuid, map_ids) + assert {:ok, recovered} = MapPoolState.get_pool_state(uuid) + assert length(recovered) == 20 + assert recovered == map_ids + else + :ok + end + end + end + + describe "Concurrent operations" do + test "multiple pools can save state concurrently", %{ets_exists: ets_exists?} do + if ets_exists? do + # Create 10 pools concurrently + tasks = + 1..10 + |> Enum.map(fn i -> + Task.async(fn -> + uuid = "concurrent-pool-#{i}-#{:rand.uniform(1_000_000)}" + map_ids = [i * 10, i * 10 + 1] + MapPoolState.save_pool_state(uuid, map_ids) + {uuid, map_ids} + end) + end) + + results = Task.await_many(tasks, 5000) + + # Verify all pools saved successfully + Enum.each(results, fn {uuid, expected_map_ids} -> + assert {:ok, ^expected_map_ids} = MapPoolState.get_pool_state(uuid) + end) + else + :ok + end + end + + test "concurrent reads and writes don't corrupt state", %{ets_exists: ets_exists?} do + if ets_exists? do + uuid = "test-pool-#{:rand.uniform(1_000_000)}" + MapPoolState.save_pool_state(uuid, [1, 2, 3]) + + # Spawn multiple readers and writers + readers = + 1..5 + |> Enum.map(fn _ -> + Task.async(fn -> + MapPoolState.get_pool_state(uuid) + end) + end) + + writers = + 1..5 + |> Enum.map(fn i -> + Task.async(fn -> + MapPoolState.save_pool_state(uuid, [i, i + 1]) + end) + end) + + # All operations should complete without error + reader_results = Task.await_many(readers, 5000) + writer_results = Task.await_many(writers, 5000) + + assert Enum.all?(reader_results, fn + {:ok, _} -> true + _ -> false + end) + + assert Enum.all?(writer_results, fn :ok -> true end) + + # Final state should be valid (one of the writer's values) + assert {:ok, final_state} = MapPoolState.get_pool_state(uuid) + assert is_list(final_state) + assert length(final_state) == 2 + else + :ok + end + end + end + + describe "Performance" do + @tag :slow + test "recovery completes within acceptable time", %{ets_exists: ets_exists?} do + if ets_exists? do + uuid = "perf-pool-#{:rand.uniform(1_000_000)}" + # Test with pool at limit (20 maps) + map_ids = Enum.to_list(1..20) + + # Measure save time + {save_time_us, :ok} = :timer.tc(fn -> + MapPoolState.save_pool_state(uuid, map_ids) + end) + + # Measure retrieval time + {get_time_us, {:ok, _}} = :timer.tc(fn -> + MapPoolState.get_pool_state(uuid) + end) + + # Both operations should be very fast (< 1ms) + assert save_time_us < 1000, "Save took #{save_time_us}µs, expected < 1000µs" + assert get_time_us < 1000, "Get took #{get_time_us}µs, expected < 1000µs" + else + :ok + end + end + + @tag :slow + test "cleanup performance with many stale entries", %{ets_exists: ets_exists?} do + if ets_exists? do + # Insert 100 stale entries + stale_timestamp = System.system_time(:second) - 25 * 3600 + + 1..100 + |> Enum.each(fn i -> + uuid = "stale-pool-#{i}" + :ets.insert(@ets_table, {uuid, [i], stale_timestamp}) + end) + + # Measure cleanup time + {cleanup_time_us, {:ok, deleted_count}} = :timer.tc(fn -> + MapPoolState.cleanup_stale_entries() + end) + + # Should have deleted at least 100 entries + assert deleted_count >= 100 + + # Cleanup should be reasonably fast (< 100ms for 100 entries) + assert cleanup_time_us < 100_000, + "Cleanup took #{cleanup_time_us}µs, expected < 100,000µs" + else + :ok + end + end + end +end diff --git a/test/unit/map/slug_uniqueness_test.exs b/test/unit/map/slug_uniqueness_test.exs new file mode 100644 index 00000000..f32c1202 --- /dev/null +++ b/test/unit/map/slug_uniqueness_test.exs @@ -0,0 +1,320 @@ +defmodule WandererApp.Map.SlugUniquenessTest do + @moduledoc """ + Tests for map slug uniqueness constraints and handling. + + These tests verify that: + 1. Database unique constraint is enforced + 2. Application-level slug generation handles uniqueness + 3. Concurrent map creation doesn't create duplicates + 4. Error handling works correctly for slug conflicts + """ + use WandererApp.DataCase, async: false + + alias WandererApp.Api.Map + + require Logger + + describe "slug uniqueness constraint" do + setup do + # Create a test user + user = create_test_user() + %{user: user} + end + + test "prevents duplicate slugs via database constraint", %{user: user} do + # Create first map with a specific slug + {:ok, _map1} = + Map.new(%{ + name: "Test Map", + slug: "test-map", + owner_id: user.id, + description: "First map", + scope: "wormholes" + }) + + # Attempt to create second map with same slug by bypassing Ash slug generation + # This simulates a race condition where slug generation passes but DB insert fails + result = + Map.new(%{ + name: "Different Name", + slug: "test-map", + owner_id: user.id, + description: "Second map", + scope: "wormholes" + }) + + # Should get a unique constraint error from database + assert {:error, _error} = result + end + + test "automatically increments slug when duplicate detected", %{user: user} do + # Create first map + {:ok, map1} = + Map.new(%{ + name: "Test Map", + slug: "test-map", + owner_id: user.id, + description: "First map", + scope: "wormholes" + }) + + assert map1.slug == "test-map" + + # Create second map with same name (should auto-increment slug) + {:ok, map2} = + Map.new(%{ + name: "Test Map", + slug: "test-map", + owner_id: user.id, + description: "Second map", + scope: "wormholes" + }) + + # Slug should be automatically incremented + assert map2.slug == "test-map-2" + + # Create third map with same name + {:ok, map3} = + Map.new(%{ + name: "Test Map", + slug: "test-map", + owner_id: user.id, + description: "Third map", + scope: "wormholes" + }) + + assert map3.slug == "test-map-3" + end + + test "handles many maps with similar names", %{user: user} do + # Create 10 maps with the same base slug + maps = + for i <- 1..10 do + {:ok, map} = + Map.new(%{ + name: "Popular Name", + slug: "popular-name", + owner_id: user.id, + description: "Map #{i}", + scope: "wormholes" + }) + + map + end + + # Verify all slugs are unique + slugs = Enum.map(maps, & &1.slug) + assert length(Enum.uniq(slugs)) == 10 + + # First should keep the base slug + assert List.first(maps).slug == "popular-name" + + # Others should be numbered + assert "popular-name-2" in slugs + assert "popular-name-10" in slugs + end + end + + describe "concurrent slug creation (race condition)" do + setup do + user = create_test_user() + %{user: user} + end + + @tag :slow + test "handles concurrent map creation with identical slugs", %{user: user} do + # Create 5 concurrent map creation requests with the same slug + tasks = + for i <- 1..5 do + Task.async(fn -> + Map.new(%{ + name: "Concurrent Test", + slug: "concurrent-test", + owner_id: user.id, + description: "Concurrent map #{i}", + scope: "wormholes" + }) + end) + end + + # Wait for all tasks to complete + results = Task.await_many(tasks, 10_000) + + # All should either succeed or fail gracefully (no crashes) + assert length(results) == 5 + + # Get successful results + successful = Enum.filter(results, &match?({:ok, _}, &1)) + failed = Enum.filter(results, &match?({:error, _}, &1)) + + # At least some should succeed + assert length(successful) > 0 + + # Extract maps from successful results + maps = Enum.map(successful, fn {:ok, map} -> map end) + + # Verify all successful maps have unique slugs + slugs = Enum.map(maps, & &1.slug) + assert length(Enum.uniq(slugs)) == length(slugs), "All successful maps should have unique slugs" + + # Log results for visibility + Logger.info("Concurrent test: #{length(successful)} succeeded, #{length(failed)} failed") + Logger.info("Unique slugs created: #{inspect(slugs)}") + end + + @tag :slow + test "concurrent creation with different names creates different base slugs", %{user: user} do + # Create concurrent requests with different names (should all succeed) + tasks = + for i <- 1..5 do + Task.async(fn -> + Map.new(%{ + name: "Concurrent Map #{i}", + slug: "concurrent-map-#{i}", + owner_id: user.id, + description: "Map #{i}", + scope: "wormholes" + }) + end) + end + + results = Task.await_many(tasks, 10_000) + + # All should succeed + assert Enum.all?(results, &match?({:ok, _}, &1)) + + # All should have different slugs + slugs = Enum.map(results, fn {:ok, map} -> map.slug end) + assert length(Enum.uniq(slugs)) == 5 + end + end + + describe "slug generation edge cases" do + setup do + user = create_test_user() + %{user: user} + end + + test "handles very long slugs", %{user: user} do + # Create map with name that would generate very long slug + long_name = String.duplicate("a", 100) + + {:ok, map} = + Map.new(%{ + name: long_name, + slug: long_name, + owner_id: user.id, + description: "Long name test", + scope: "wormholes" + }) + + # Slug should be truncated to max length (40 chars based on map.ex constraints) + assert String.length(map.slug) <= 40 + end + + test "handles special characters in slugs", %{user: user} do + # Test that special characters are properly slugified + {:ok, map} = + Map.new(%{ + name: "Test: Map & Name!", + slug: "test-map-name", + owner_id: user.id, + description: "Special chars test", + scope: "wormholes" + }) + + # Slug should only contain allowed characters + assert map.slug =~ ~r/^[a-z0-9-]+$/ + end + end + + describe "slug update operations" do + setup do + user = create_test_user() + + {:ok, map} = + Map.new(%{ + name: "Original Map", + slug: "original-map", + owner_id: user.id, + description: "Original", + scope: "wormholes" + }) + + %{user: user, map: map} + end + + test "updating map with same slug succeeds", %{map: map} do + # Update other fields, keep same slug + result = + Map.update(map, %{ + description: "Updated description", + slug: "original-map" + }) + + assert {:ok, updated_map} = result + assert updated_map.slug == "original-map" + assert updated_map.description == "Updated description" + end + + test "updating to conflicting slug is handled", %{user: user, map: map} do + # Create another map + {:ok, _other_map} = + Map.new(%{ + name: "Other Map", + slug: "other-map", + owner_id: user.id, + description: "Other", + scope: "wormholes" + }) + + # Try to update first map to use other map's slug + result = + Map.update(map, %{ + slug: "other-map" + }) + + # Should either fail or auto-increment + case result do + {:ok, updated_map} -> + # If successful, slug should be different + assert updated_map.slug != "other-map" + assert updated_map.slug =~ ~r/^other-map-\d+$/ + + {:error, _} -> + # Or it can fail with validation error + :ok + end + end + end + + describe "get_map_by_slug with duplicates" do + setup do + user = create_test_user() + %{user: user} + end + + test "get_map_by_slug! raises on duplicates if they exist" do + # Note: This test documents the behavior when duplicates somehow exist + # In production, this should be prevented by our fixes + # If duplicates exist (data integrity issue), the query should fail + + # This is a documentation test - we can't easily create duplicates + # due to the database constraint, but we document expected behavior + assert true + end + end + + # Helper functions + + defp create_test_user do + # Create a test user with necessary attributes + {:ok, user} = + WandererApp.Api.User.new(%{ + name: "Test User #{:rand.uniform(10_000)}", + eve_id: :rand.uniform(100_000_000) + }) + + user + end +end From 6f4240d931c093b7a7511f37bd3d8aea440ed100 Mon Sep 17 00:00:00 2001 From: CI Date: Fri, 14 Nov 2025 18:36:20 +0000 Subject: [PATCH 02/32] chore: release version v1.84.18 --- CHANGELOG.md | 9 +++++++++ mix.exs | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 342766f9..ee3b7143 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ +## [v1.84.18](https://github.com/wanderer-industries/wanderer/compare/v1.84.17...v1.84.18) (2025-11-14) + + + + +### Bug Fixes: + +* core: added gracefull map poll recovery from saved state. added map slug unique checks + ## [v1.84.17](https://github.com/wanderer-industries/wanderer/compare/v1.84.16...v1.84.17) (2025-11-14) diff --git a/mix.exs b/mix.exs index 0cf8dce4..9f29fdf3 100644 --- a/mix.exs +++ b/mix.exs @@ -3,7 +3,7 @@ defmodule WandererApp.MixProject do @source_url "https://github.com/wanderer-industries/wanderer" - @version "1.84.17" + @version "1.84.18" def project do [ From 308e81a464f7d200e7e4330c8ff651aeda7a41ac Mon Sep 17 00:00:00 2001 From: CI Date: Fri, 14 Nov 2025 18:36:20 +0000 Subject: [PATCH 03/32] chore: [skip ci] From ac4dd4c28b90b514e4c2cc361d31100beee2911f Mon Sep 17 00:00:00 2001 From: Dmitry Popov Date: Fri, 14 Nov 2025 20:28:12 +0100 Subject: [PATCH 04/32] fix(core): fixed map start issues --- lib/wanderer_app/map/map_pool.ex | 99 +++++++++++++++++-- .../map/map_pool_dynamic_supervisor.ex | 26 ++++- .../map/server/map_server_impl.ex | 79 ++++++++++++--- test/unit/map/cache_rtree_test.exs | 2 +- 4 files changed, 183 insertions(+), 23 deletions(-) diff --git a/lib/wanderer_app/map/map_pool.ex b/lib/wanderer_app/map/map_pool.ex index bc1581e0..c136a6c3 100644 --- a/lib/wanderer_app/map/map_pool.ex +++ b/lib/wanderer_app/map/map_pool.ex @@ -194,6 +194,68 @@ defmodule WandererApp.Map.MapPool do {:noreply, new_state} end + @impl true + def handle_continue({:init_map, map_id}, %{uuid: uuid} = state) do + # Perform the actual map initialization asynchronously + # This runs after the GenServer.call has already returned + start_time = System.monotonic_time(:millisecond) + + try do + # Initialize the map state and start the map server + map_id + |> WandererApp.Map.get_map_state!() + |> Server.Impl.start_map() + + duration = System.monotonic_time(:millisecond) - start_time + + Logger.info("[Map Pool #{uuid}] Map #{map_id} initialized successfully in #{duration}ms") + + # Emit telemetry for slow initializations + if duration > 5_000 do + Logger.warning("[Map Pool #{uuid}] Slow map initialization: #{map_id} took #{duration}ms") + + :telemetry.execute( + [:wanderer_app, :map_pool, :slow_init], + %{duration_ms: duration}, + %{map_id: map_id, pool_uuid: uuid} + ) + end + + {:noreply, state} + rescue + e -> + duration = System.monotonic_time(:millisecond) - start_time + + Logger.error(""" + [Map Pool #{uuid}] Failed to initialize map #{map_id} after #{duration}ms: #{Exception.message(e)} + #{Exception.format_stacktrace(__STACKTRACE__)} + """) + + # Rollback: Remove from state, registry, and cache + new_state = %{state | map_ids: state.map_ids |> Enum.reject(fn id -> id == map_id end)} + + # Update registry + Registry.update_value(@unique_registry, Module.concat(__MODULE__, uuid), fn r_map_ids -> + r_map_ids |> Enum.reject(fn id -> id == map_id end) + end) + + # Remove from cache + Cachex.del(@cache, map_id) + + # Update ETS state + MapPoolState.save_pool_state(uuid, new_state.map_ids) + + # Emit telemetry for failed initialization + :telemetry.execute( + [:wanderer_app, :map_pool, :init_failed], + %{duration_ms: duration}, + %{map_id: map_id, pool_uuid: uuid, reason: Exception.message(e)} + ) + + {:noreply, new_state} + end + end + @impl true def handle_cast(:stop, state), do: {:stop, :normal, state} @@ -214,13 +276,38 @@ defmodule WandererApp.Map.MapPool do {:reply, :ok, state} else - case do_start_map(map_id, state) do - {:ok, new_state} -> - {:reply, :ok, new_state} + # Check if map is already started or being initialized + if map_id in map_ids do + Logger.debug("[Map Pool #{uuid}] Map #{map_id} already in pool") + {:reply, {:ok, :already_started}, state} + else + # Pre-register the map in registry and cache to claim ownership + # This prevents race conditions where multiple pools try to start the same map + registry_result = + Registry.update_value(@unique_registry, Module.concat(__MODULE__, uuid), fn r_map_ids -> + [map_id | r_map_ids] + end) - {:error, _reason} -> - # Error already logged in do_start_map - {:reply, :ok, state} + case registry_result do + {_new_value, _old_value} -> + # Add to cache + Cachex.put(@cache, map_id, uuid) + + # Add to state + new_state = %{state | map_ids: [map_id | map_ids]} + + # Persist state to ETS + MapPoolState.save_pool_state(uuid, new_state.map_ids) + + Logger.debug("[Map Pool #{uuid}] Map #{map_id} queued for async initialization") + + # Return immediately and initialize asynchronously + {:reply, {:ok, :initializing}, new_state, {:continue, {:init_map, map_id}}} + + :error -> + Logger.error("[Map Pool #{uuid}] Failed to register map #{map_id} in registry") + {:reply, {:error, :registration_failed}, state} + end end end end diff --git a/lib/wanderer_app/map/map_pool_dynamic_supervisor.ex b/lib/wanderer_app/map/map_pool_dynamic_supervisor.ex index 7a5b3697..2d6d325c 100644 --- a/lib/wanderer_app/map/map_pool_dynamic_supervisor.ex +++ b/lib/wanderer_app/map/map_pool_dynamic_supervisor.ex @@ -8,6 +8,7 @@ defmodule WandererApp.Map.MapPoolDynamicSupervisor do @registry :map_pool_registry @unique_registry :unique_map_pool_registry @map_pool_limit 20 + @genserver_call_timeout :timer.seconds(30) @name __MODULE__ @@ -30,7 +31,26 @@ defmodule WandererApp.Map.MapPoolDynamicSupervisor do start_child([map_id], pools |> Enum.count()) pid -> - GenServer.call(pid, {:start_map, map_id}) + result = GenServer.call(pid, {:start_map, map_id}, @genserver_call_timeout) + + case result do + {:ok, :initializing} -> + Logger.debug("[Map Pool Supervisor] Map #{map_id} queued for async initialization") + result + + {:ok, :already_started} -> + Logger.debug("[Map Pool Supervisor] Map #{map_id} already started") + result + + :ok -> + # Legacy synchronous response (from crash recovery path) + Logger.debug("[Map Pool Supervisor] Map #{map_id} started synchronously") + result + + other -> + Logger.warning("[Map Pool Supervisor] Unexpected response for map #{map_id}: #{inspect(other)}") + other + end end end end @@ -59,7 +79,7 @@ defmodule WandererApp.Map.MapPoolDynamicSupervisor do find_pool_by_scanning_registry(map_id) [{pool_pid, _}] -> - GenServer.call(pool_pid, {:stop_map, map_id}) + GenServer.call(pool_pid, {:stop_map, map_id}, @genserver_call_timeout) end {:error, reason} -> @@ -102,7 +122,7 @@ defmodule WandererApp.Map.MapPoolDynamicSupervisor do # Update the cache to fix the inconsistency Cachex.put(@cache, map_id, pool_uuid) - GenServer.call(pool_pid, {:stop_map, map_id}) + GenServer.call(pool_pid, {:stop_map, map_id}, @genserver_call_timeout) nil -> Logger.debug("Map #{map_id} not found in any pool registry") diff --git a/lib/wanderer_app/map/server/map_server_impl.ex b/lib/wanderer_app/map/server/map_server_impl.ex index af245ce6..15436735 100644 --- a/lib/wanderer_app/map/server/map_server_impl.ex +++ b/lib/wanderer_app/map/server/map_server_impl.ex @@ -45,19 +45,72 @@ defmodule WandererApp.Map.Server.Impl do } |> new() - with {:ok, map} <- - WandererApp.MapRepo.get(map_id, [ - :owner, - :characters, - acls: [ - :owner_id, - members: [:role, :eve_character_id, :eve_corporation_id, :eve_alliance_id] - ] - ]), - {:ok, systems} <- WandererApp.MapSystemRepo.get_visible_by_map(map_id), - {:ok, connections} <- WandererApp.MapConnectionRepo.get_by_map(map_id), - {:ok, subscription_settings} <- - WandererApp.Map.SubscriptionManager.get_active_map_subscription(map_id) do + # Parallelize database queries for faster initialization + start_time = System.monotonic_time(:millisecond) + + tasks = [ + Task.async(fn -> + {:map, WandererApp.MapRepo.get(map_id, [ + :owner, + :characters, + acls: [ + :owner_id, + members: [:role, :eve_character_id, :eve_corporation_id, :eve_alliance_id] + ] + ])} + end), + Task.async(fn -> + {:systems, WandererApp.MapSystemRepo.get_visible_by_map(map_id)} + end), + Task.async(fn -> + {:connections, WandererApp.MapConnectionRepo.get_by_map(map_id)} + end), + Task.async(fn -> + {:subscription, WandererApp.Map.SubscriptionManager.get_active_map_subscription(map_id)} + end) + ] + + results = Task.await_many(tasks, :timer.seconds(15)) + + duration = System.monotonic_time(:millisecond) - start_time + + # Emit telemetry for slow initializations + if duration > 5_000 do + Logger.warning("[Map Server] Slow map state initialization: #{map_id} took #{duration}ms") + + :telemetry.execute( + [:wanderer_app, :map, :slow_init], + %{duration_ms: duration}, + %{map_id: map_id} + ) + end + + # Extract results + map_result = Enum.find_value(results, fn + {:map, result} -> result + _ -> nil + end) + + systems_result = Enum.find_value(results, fn + {:systems, result} -> result + _ -> nil + end) + + connections_result = Enum.find_value(results, fn + {:connections, result} -> result + _ -> nil + end) + + subscription_result = Enum.find_value(results, fn + {:subscription, result} -> result + _ -> nil + end) + + # Process results + with {:ok, map} <- map_result, + {:ok, systems} <- systems_result, + {:ok, connections} <- connections_result, + {:ok, subscription_settings} <- subscription_result do initial_state |> init_map( map, diff --git a/test/unit/map/cache_rtree_test.exs b/test/unit/map/cache_rtree_test.exs index f9820084..aaad9ce0 100644 --- a/test/unit/map/cache_rtree_test.exs +++ b/test/unit/map/cache_rtree_test.exs @@ -410,7 +410,7 @@ defmodule WandererApp.Map.CacheRTreeTest do # Check many positions for availability (simulating auto-positioning) test_positions = for x <- 0..20, y <- 0..20, do: {x * 100, y * 50} - for {x, y} do + for {x, y} <- test_positions do box = [{x, x + 130}, {y, y + 34}] {:ok, _ids} = CacheRTree.query(box, name) # Not asserting anything, just verifying queries work From 4198e4b07a8489fa531451a44c5e5df39c24f0c9 Mon Sep 17 00:00:00 2001 From: CI Date: Fri, 14 Nov 2025 19:28:51 +0000 Subject: [PATCH 05/32] chore: release version v1.84.19 --- CHANGELOG.md | 9 +++++++++ mix.exs | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee3b7143..d37a7bad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ +## [v1.84.19](https://github.com/wanderer-industries/wanderer/compare/v1.84.18...v1.84.19) (2025-11-14) + + + + +### Bug Fixes: + +* core: fixed map start issues + ## [v1.84.18](https://github.com/wanderer-industries/wanderer/compare/v1.84.17...v1.84.18) (2025-11-14) diff --git a/mix.exs b/mix.exs index 9f29fdf3..b794daec 100644 --- a/mix.exs +++ b/mix.exs @@ -3,7 +3,7 @@ defmodule WandererApp.MixProject do @source_url "https://github.com/wanderer-industries/wanderer" - @version "1.84.18" + @version "1.84.19" def project do [ From e15bfa426a9f50c308310d44b1ae7a9e60a3d83b Mon Sep 17 00:00:00 2001 From: CI Date: Fri, 14 Nov 2025 19:28:51 +0000 Subject: [PATCH 06/32] chore: [skip ci] From 0be7a5f9d0ea65369cc01c5d0ae87032558235cb Mon Sep 17 00:00:00 2001 From: Dmitry Popov Date: Sat, 15 Nov 2025 08:25:55 +0100 Subject: [PATCH 07/32] fix(core): fixed map start issues --- lib/wanderer_app/api/map.ex | 2 +- lib/wanderer_app/map/map_pool.ex | 17 +++++++++++++---- .../map/map_pool_dynamic_supervisor.ex | 14 ++++++++++---- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/lib/wanderer_app/api/map.ex b/lib/wanderer_app/api/map.ex index 2735aee3..374cbe63 100644 --- a/lib/wanderer_app/api/map.ex +++ b/lib/wanderer_app/api/map.ex @@ -31,7 +31,7 @@ defmodule WandererApp.Api.Map do routes do base("/maps") get(:by_slug, route: "/:slug") - index :read + # index :read post(:new) patch(:update) delete(:destroy) diff --git a/lib/wanderer_app/map/map_pool.ex b/lib/wanderer_app/map/map_pool.ex index c136a6c3..fe359db5 100644 --- a/lib/wanderer_app/map/map_pool.ex +++ b/lib/wanderer_app/map/map_pool.ex @@ -15,7 +15,7 @@ defmodule WandererApp.Map.MapPool do @cache :map_pool_cache @registry :map_pool_registry @unique_registry :unique_map_pool_registry - @map_pool_limit 20 + @map_pool_limit 10 @garbage_collection_interval :timer.hours(4) @systems_cleanup_timeout :timer.minutes(30) @@ -113,7 +113,10 @@ defmodule WandererApp.Map.MapPool do MapPoolState.delete_pool_state(uuid) _ -> - Logger.warning("[Map Pool #{uuid}] Abnormal termination (#{inspect(reason)}), keeping ETS state for recovery") + Logger.warning( + "[Map Pool #{uuid}] Abnormal termination (#{inspect(reason)}), keeping ETS state for recovery" + ) + # Keep ETS state for crash recovery :ok end @@ -753,7 +756,10 @@ defmodule WandererApp.Map.MapPool do updated_state {:error, reason} -> - Logger.error("[Map Pool #{state.uuid}] Failed to stop deleted map #{map_id}: #{reason}") + Logger.error( + "[Map Pool #{state.uuid}] Failed to stop deleted map #{map_id}: #{reason}" + ) + current_state end @@ -770,7 +776,10 @@ defmodule WandererApp.Map.MapPool do updated_state {:error, reason} -> - Logger.error("[Map Pool #{state.uuid}] Failed to stop missing map #{map_id}: #{reason}") + Logger.error( + "[Map Pool #{state.uuid}] Failed to stop missing map #{map_id}: #{reason}" + ) + current_state end end diff --git a/lib/wanderer_app/map/map_pool_dynamic_supervisor.ex b/lib/wanderer_app/map/map_pool_dynamic_supervisor.ex index 2d6d325c..8e96bdb3 100644 --- a/lib/wanderer_app/map/map_pool_dynamic_supervisor.ex +++ b/lib/wanderer_app/map/map_pool_dynamic_supervisor.ex @@ -7,8 +7,8 @@ defmodule WandererApp.Map.MapPoolDynamicSupervisor do @cache :map_pool_cache @registry :map_pool_registry @unique_registry :unique_map_pool_registry - @map_pool_limit 20 - @genserver_call_timeout :timer.seconds(30) + @map_pool_limit 10 + @genserver_call_timeout :timer.minutes(2) @name __MODULE__ @@ -35,7 +35,10 @@ defmodule WandererApp.Map.MapPoolDynamicSupervisor do case result do {:ok, :initializing} -> - Logger.debug("[Map Pool Supervisor] Map #{map_id} queued for async initialization") + Logger.debug( + "[Map Pool Supervisor] Map #{map_id} queued for async initialization" + ) + result {:ok, :already_started} -> @@ -48,7 +51,10 @@ defmodule WandererApp.Map.MapPoolDynamicSupervisor do result other -> - Logger.warning("[Map Pool Supervisor] Unexpected response for map #{map_id}: #{inspect(other)}") + Logger.warning( + "[Map Pool Supervisor] Unexpected response for map #{map_id}: #{inspect(other)}" + ) + other end end From bd865b9f64d0d1043f7d31c099adcba930b156aa Mon Sep 17 00:00:00 2001 From: CI Date: Sat, 15 Nov 2025 07:29:25 +0000 Subject: [PATCH 08/32] chore: release version v1.84.20 --- CHANGELOG.md | 9 +++++++++ mix.exs | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d37a7bad..6b08b4e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ +## [v1.84.20](https://github.com/wanderer-industries/wanderer/compare/v1.84.19...v1.84.20) (2025-11-15) + + + + +### Bug Fixes: + +* core: fixed map start issues + ## [v1.84.19](https://github.com/wanderer-industries/wanderer/compare/v1.84.18...v1.84.19) (2025-11-14) diff --git a/mix.exs b/mix.exs index b794daec..72e66b26 100644 --- a/mix.exs +++ b/mix.exs @@ -3,7 +3,7 @@ defmodule WandererApp.MixProject do @source_url "https://github.com/wanderer-industries/wanderer" - @version "1.84.19" + @version "1.84.20" def project do [ From 843b3b86b21184d63bde09c6ae61ef2680fccba5 Mon Sep 17 00:00:00 2001 From: CI Date: Sat, 15 Nov 2025 07:29:25 +0000 Subject: [PATCH 09/32] chore: [skip ci] From e6dbba7283b6e94978dcdcb4f2328907e9a98dee Mon Sep 17 00:00:00 2001 From: Dmitry Popov Date: Sat, 15 Nov 2025 09:47:48 +0100 Subject: [PATCH 10/32] fix(core): fixed map characters adding --- lib/wanderer_app/map.ex | 89 ++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 59 deletions(-) diff --git a/lib/wanderer_app/map.ex b/lib/wanderer_app/map.ex index 69833e3b..70b0694d 100644 --- a/lib/wanderer_app/map.ex +++ b/lib/wanderer_app/map.ex @@ -53,8 +53,8 @@ defmodule WandererApp.Map do {:ok, map} -> map - _ -> - Logger.error(fn -> "Failed to get map #{map_id}" end) + error -> + Logger.error("Failed to get map #{map_id}: #{inspect(error)}") %{} end end @@ -183,9 +183,31 @@ defmodule WandererApp.Map do def add_characters!(map, []), do: map - def add_characters!(%{map_id: map_id} = map, [character | rest]) do - add_character(map_id, character) - add_characters!(map, rest) + def add_characters!(%{map_id: map_id} = map, characters) when is_list(characters) do + # Get current characters list once + current_characters = Map.get(map, :characters, []) + + characters_ids = + characters + |> Enum.map(fn %{id: char_id} -> char_id end) + + # Filter out characters that already exist + new_character_ids = + characters_ids + |> Enum.reject(fn char_id -> char_id in current_characters end) + + # If all characters already exist, return early + if new_character_ids == [] do + map + else + case update_map(map_id, %{characters: new_character_ids ++ current_characters}) do + {:commit, map} -> + map + + _ -> + map + end + end end def add_character( @@ -198,61 +220,10 @@ defmodule WandererApp.Map do case not (characters |> Enum.member?(character_id)) do true -> - WandererApp.Character.get_map_character(map_id, character_id) - |> case do - {:ok, - %{ - alliance_id: alliance_id, - corporation_id: corporation_id, - solar_system_id: solar_system_id, - structure_id: structure_id, - station_id: station_id, - ship: ship_type_id, - ship_name: ship_name - }} -> - map_id - |> update_map(%{characters: [character_id | characters]}) + map_id + |> update_map(%{characters: [character_id | characters]}) - # WandererApp.Cache.insert( - # "map:#{map_id}:character:#{character_id}:alliance_id", - # alliance_id - # ) - - # WandererApp.Cache.insert( - # "map:#{map_id}:character:#{character_id}:corporation_id", - # corporation_id - # ) - - # WandererApp.Cache.insert( - # "map:#{map_id}:character:#{character_id}:solar_system_id", - # solar_system_id - # ) - - # WandererApp.Cache.insert( - # "map:#{map_id}:character:#{character_id}:structure_id", - # structure_id - # ) - - # WandererApp.Cache.insert( - # "map:#{map_id}:character:#{character_id}:station_id", - # station_id - # ) - - # WandererApp.Cache.insert( - # "map:#{map_id}:character:#{character_id}:ship_type_id", - # ship_type_id - # ) - - # WandererApp.Cache.insert( - # "map:#{map_id}:character:#{character_id}:ship_name", - # ship_name - # ) - - :ok - - error -> - error - end + :ok _ -> {:error, :already_exists} From b22970fef36f27af594d70c33a87b0dc4a39355d Mon Sep 17 00:00:00 2001 From: CI Date: Sat, 15 Nov 2025 08:48:30 +0000 Subject: [PATCH 11/32] chore: release version v1.84.21 --- CHANGELOG.md | 9 +++++++++ mix.exs | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b08b4e7..74e3e7a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ +## [v1.84.21](https://github.com/wanderer-industries/wanderer/compare/v1.84.20...v1.84.21) (2025-11-15) + + + + +### Bug Fixes: + +* core: fixed map characters adding + ## [v1.84.20](https://github.com/wanderer-industries/wanderer/compare/v1.84.19...v1.84.20) (2025-11-15) diff --git a/mix.exs b/mix.exs index 72e66b26..300585b6 100644 --- a/mix.exs +++ b/mix.exs @@ -3,7 +3,7 @@ defmodule WandererApp.MixProject do @source_url "https://github.com/wanderer-industries/wanderer" - @version "1.84.20" + @version "1.84.21" def project do [ From 20c8a5371229f8b82e91406abb08955eb349db1c Mon Sep 17 00:00:00 2001 From: CI Date: Sat, 15 Nov 2025 08:48:30 +0000 Subject: [PATCH 12/32] chore: [skip ci] From 8934935e10b64422f4f7e9f12fbadeb83f667323 Mon Sep 17 00:00:00 2001 From: Dmitry Popov Date: Sat, 15 Nov 2025 12:37:27 +0100 Subject: [PATCH 13/32] fix(core): fixed map initialization --- lib/wanderer_app/map/map_pool.ex | 64 ++++++++++++------- .../map/server/map_server_impl.ex | 57 +++++++++-------- 2 files changed, 73 insertions(+), 48 deletions(-) diff --git a/lib/wanderer_app/map/map_pool.ex b/lib/wanderer_app/map/map_pool.ex index fe359db5..97b401df 100644 --- a/lib/wanderer_app/map/map_pool.ex +++ b/lib/wanderer_app/map/map_pool.ex @@ -204,10 +204,8 @@ defmodule WandererApp.Map.MapPool do start_time = System.monotonic_time(:millisecond) try do - # Initialize the map state and start the map server - map_id - |> WandererApp.Map.get_map_state!() - |> Server.Impl.start_map() + # Initialize the map state and start the map server using extracted helper + do_initialize_map_server(map_id) duration = System.monotonic_time(:millisecond) - start_time @@ -234,19 +232,8 @@ defmodule WandererApp.Map.MapPool do #{Exception.format_stacktrace(__STACKTRACE__)} """) - # Rollback: Remove from state, registry, and cache - new_state = %{state | map_ids: state.map_ids |> Enum.reject(fn id -> id == map_id end)} - - # Update registry - Registry.update_value(@unique_registry, Module.concat(__MODULE__, uuid), fn r_map_ids -> - r_map_ids |> Enum.reject(fn id -> id == map_id end) - end) - - # Remove from cache - Cachex.del(@cache, map_id) - - # Update ETS state - MapPoolState.save_pool_state(uuid, new_state.map_ids) + # Rollback: Remove from state, registry, cache, and ETS using extracted helper + new_state = do_unregister_map(map_id, uuid, state) # Emit telemetry for failed initialization :telemetry.execute( @@ -358,16 +345,16 @@ defmodule WandererApp.Map.MapPool do # Step 2: Add to cache case Cachex.put(@cache, map_id, uuid) do {:ok, _} -> - completed_operations = [:cache | completed_operations] + :ok {:error, reason} -> raise "Failed to add to cache: #{inspect(reason)}" end - # Step 3: Start the map server - map_id - |> WandererApp.Map.get_map_state!() - |> Server.Impl.start_map() + completed_operations = [:cache | completed_operations] + + # Step 3: Start the map server using extracted helper + do_initialize_map_server(map_id) completed_operations = [:map_server | completed_operations] @@ -459,12 +446,14 @@ defmodule WandererApp.Map.MapPool do # Step 2: Delete from cache case Cachex.del(@cache, map_id) do {:ok, _} -> - completed_operations = [:cache | completed_operations] + :ok {:error, reason} -> raise "Failed to delete from cache: #{inspect(reason)}" end + completed_operations = [:cache | completed_operations] + # Step 3: Stop the map server (clean up all map resources) map_id |> Server.Impl.stop_map() @@ -493,6 +482,35 @@ defmodule WandererApp.Map.MapPool do end end + # Helper function to initialize the map server (no state management) + # This extracts the common map initialization logic used in both + # synchronous (do_start_map) and asynchronous ({:init_map, map_id}) paths + defp do_initialize_map_server(map_id) do + map_id + |> WandererApp.Map.get_map_state!() + |> Server.Impl.start_map() + end + + # Helper function to unregister a map from all tracking + # Used for rollback when map initialization fails in the async path + defp do_unregister_map(map_id, uuid, state) do + # Remove from registry + Registry.update_value(@unique_registry, Module.concat(__MODULE__, uuid), fn r_map_ids -> + Enum.reject(r_map_ids, &(&1 == map_id)) + end) + + # Remove from cache + Cachex.del(@cache, map_id) + + # Update state + new_state = %{state | map_ids: Enum.reject(state.map_ids, &(&1 == map_id))} + + # Update ETS + MapPoolState.save_pool_state(uuid, new_state.map_ids) + + new_state + end + defp rollback_stop_map_operations(map_id, uuid, completed_operations) do Logger.warning("[Map Pool] Attempting to rollback stop_map operations for #{map_id}") diff --git a/lib/wanderer_app/map/server/map_server_impl.ex b/lib/wanderer_app/map/server/map_server_impl.ex index 15436735..8110ef53 100644 --- a/lib/wanderer_app/map/server/map_server_impl.ex +++ b/lib/wanderer_app/map/server/map_server_impl.ex @@ -25,6 +25,7 @@ defmodule WandererApp.Map.Server.Impl do ] @pubsub_client Application.compile_env(:wanderer_app, :pubsub_client) + @ddrt Application.compile_env(:wanderer_app, :ddrt) @connections_cleanup_timeout :timer.minutes(1) @@ -50,14 +51,15 @@ defmodule WandererApp.Map.Server.Impl do tasks = [ Task.async(fn -> - {:map, WandererApp.MapRepo.get(map_id, [ - :owner, - :characters, - acls: [ - :owner_id, - members: [:role, :eve_character_id, :eve_corporation_id, :eve_alliance_id] - ] - ])} + {:map, + WandererApp.MapRepo.get(map_id, [ + :owner, + :characters, + acls: [ + :owner_id, + members: [:role, :eve_character_id, :eve_corporation_id, :eve_alliance_id] + ] + ])} end), Task.async(fn -> {:systems, WandererApp.MapSystemRepo.get_visible_by_map(map_id)} @@ -86,25 +88,29 @@ defmodule WandererApp.Map.Server.Impl do end # Extract results - map_result = Enum.find_value(results, fn - {:map, result} -> result - _ -> nil - end) + map_result = + Enum.find_value(results, fn + {:map, result} -> result + _ -> nil + end) - systems_result = Enum.find_value(results, fn - {:systems, result} -> result - _ -> nil - end) + systems_result = + Enum.find_value(results, fn + {:systems, result} -> result + _ -> nil + end) - connections_result = Enum.find_value(results, fn - {:connections, result} -> result - _ -> nil - end) + connections_result = + Enum.find_value(results, fn + {:connections, result} -> result + _ -> nil + end) - subscription_result = Enum.find_value(results, fn - {:subscription, result} -> result - _ -> nil - end) + subscription_result = + Enum.find_value(results, fn + {:subscription, result} -> result + _ -> nil + end) # Process results with {:ok, map} <- map_result, @@ -141,7 +147,6 @@ defmodule WandererApp.Map.Server.Impl do "maps:#{map_id}" ) - WandererApp.Map.CacheRTree.init_tree("rtree_#{map_id}", %{width: 150, verbose: false}) Process.send_after(self(), {:update_characters, map_id}, @update_characters_timeout) Process.send_after( @@ -512,6 +517,8 @@ defmodule WandererApp.Map.Server.Impl do ) do {:ok, options} = WandererApp.MapRepo.options_to_form_data(initial_map) + @ddrt.init_tree("rtree_#{map_id}", %{width: 150, verbose: false}) + map = initial_map |> WandererApp.Map.new() From f3077c0bf1d77101cddf69e198c47bbd124fbd67 Mon Sep 17 00:00:00 2001 From: CI Date: Sat, 15 Nov 2025 11:38:00 +0000 Subject: [PATCH 14/32] chore: release version v1.84.22 --- CHANGELOG.md | 9 +++++++++ mix.exs | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74e3e7a6..d88d4ffd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ +## [v1.84.22](https://github.com/wanderer-industries/wanderer/compare/v1.84.21...v1.84.22) (2025-11-15) + + + + +### Bug Fixes: + +* core: fixed map initialization + ## [v1.84.21](https://github.com/wanderer-industries/wanderer/compare/v1.84.20...v1.84.21) (2025-11-15) diff --git a/mix.exs b/mix.exs index 300585b6..f9f04b6d 100644 --- a/mix.exs +++ b/mix.exs @@ -3,7 +3,7 @@ defmodule WandererApp.MixProject do @source_url "https://github.com/wanderer-industries/wanderer" - @version "1.84.21" + @version "1.84.22" def project do [ From 812582d955225614788632d799a5258fa076b7f3 Mon Sep 17 00:00:00 2001 From: CI Date: Sat, 15 Nov 2025 11:38:00 +0000 Subject: [PATCH 15/32] chore: [skip ci] From 06626f910b320c1f958ed813ddb1f07948167a65 Mon Sep 17 00:00:00 2001 From: DanSylvest Date: Sat, 15 Nov 2025 21:30:45 +0300 Subject: [PATCH 16/32] fix(Map): Fixed problem related with error if settings was removed and mapper crashed. Fixed settings reset. --- assets/css/app.css | 4 +- .../SolarSystemNodeDefault.module.scss | 20 +- .../TopSearch/TopSearch.module.scss | 12 +- assets/lib/phoenix/index.js | 4 +- .../components/layouts/blog.html.heex | 6 +- .../controllers/blog_html/changelog.html.heex | 2 +- .../controllers/blog_html/contacts.heex | 2 +- .../controllers/blog_html/index.html.heex | 64 +----- .../controllers/blog_html/license.html.heex | 32 +-- .../controllers/blog_html/list.html.heex | 12 +- .../controllers/blog_html/show.html.heex | 208 +++++++++--------- lib/wanderer_app_web/router.ex | 1 + m | 3 + priv/static/cache_manifest.json | 4 +- priv/static/favicon.svg.gz | Bin 337 -> 339 bytes 15 files changed, 171 insertions(+), 203 deletions(-) diff --git a/assets/css/app.css b/assets/css/app.css index 47ae02cd..ec916592 100644 --- a/assets/css/app.css +++ b/assets/css/app.css @@ -73,7 +73,9 @@ body > div:first-of-type { } .maps_bg { - background-image: url('../images/maps_bg.webp'); + /* OLD image */ + /* background-image: url('../images/maps_bg.webp'); */ + background-image: url('https://raw.githubusercontent.com/wanderer-industries/wanderer-assets/d65f3a87ba36c26a75f501f34804f1454af96165/images/eve-screen-catalyst-expansion-bg.jpg'); background-size: cover; background-position: center; width: 100%; diff --git a/assets/js/hooks/Mapper/components/map/components/SolarSystemNode/SolarSystemNodeDefault.module.scss b/assets/js/hooks/Mapper/components/map/components/SolarSystemNode/SolarSystemNodeDefault.module.scss index 1ebb049e..a525cb55 100644 --- a/assets/js/hooks/Mapper/components/map/components/SolarSystemNode/SolarSystemNodeDefault.module.scss +++ b/assets/js/hooks/Mapper/components/map/components/SolarSystemNode/SolarSystemNodeDefault.module.scss @@ -1,6 +1,6 @@ @use "sass:color"; @use '@/hooks/Mapper/components/map/styles/eve-common-variables'; -@import '@/hooks/Mapper/components/map/styles/solar-system-node'; +@use '@/hooks/Mapper/components/map/styles/solar-system-node' as v; @keyframes move-stripes { from { @@ -26,8 +26,8 @@ background-color: var(--rf-node-bg-color, #202020) !important; color: var(--rf-text-color, #ffffff); - box-shadow: 0 0 5px rgba($dark-bg, 0.5); - border: 1px solid color.adjust($pastel-blue, $lightness: -10%); + box-shadow: 0 0 5px rgba(v.$dark-bg, 0.5); + border: 1px solid color.adjust(v.$pastel-blue, $lightness: -10%); border-radius: 5px; position: relative; z-index: 3; @@ -99,7 +99,7 @@ } &.selected { - border-color: $pastel-pink; + border-color: v.$pastel-pink; box-shadow: 0 0 10px #9a1af1c2; } @@ -113,11 +113,11 @@ bottom: 0; z-index: -1; - border-color: $neon-color-1; + border-color: v.$neon-color-1; background: repeating-linear-gradient( 45deg, - $neon-color-3 0px, - $neon-color-3 8px, + v.$neon-color-3 0px, + v.$neon-color-3 8px, transparent 8px, transparent 21px ); @@ -146,7 +146,7 @@ border: 1px solid var(--eve-solar-system-status-color-lookingFor-dark15); background-image: linear-gradient(275deg, #45ff8f2f, #457fff2f); &.selected { - border-color: $pastel-pink; + border-color: v.$pastel-pink; } } @@ -347,13 +347,13 @@ .Handle { min-width: initial; min-height: initial; - border: 1px solid $pastel-blue; + border: 1px solid v.$pastel-blue; width: 5px; height: 5px; pointer-events: auto; &.selected { - border-color: $pastel-pink; + border-color: v.$pastel-pink; } &.HandleTop { diff --git a/assets/js/hooks/Mapper/components/mapRootContent/components/TopSearch/TopSearch.module.scss b/assets/js/hooks/Mapper/components/mapRootContent/components/TopSearch/TopSearch.module.scss index 20cb4943..459eae9a 100644 --- a/assets/js/hooks/Mapper/components/mapRootContent/components/TopSearch/TopSearch.module.scss +++ b/assets/js/hooks/Mapper/components/mapRootContent/components/TopSearch/TopSearch.module.scss @@ -1,6 +1,6 @@ @use "sass:color"; @use '@/hooks/Mapper/components/map/styles/eve-common-variables'; -@import '@/hooks/Mapper/components/map/styles/solar-system-node'; +@use '@/hooks/Mapper/components/map/styles/solar-system-node' as v; :root { --rf-has-user-characters: #ffc75d; @@ -108,7 +108,7 @@ } &.selected { - border-color: $pastel-pink; + border-color: v.$pastel-pink; box-shadow: 0 0 10px #9a1af1c2; } @@ -122,11 +122,11 @@ bottom: 0; z-index: -1; - border-color: $neon-color-1; + border-color: v.$neon-color-1; background: repeating-linear-gradient( 45deg, - $neon-color-3 0px, - $neon-color-3 8px, + v.$neon-color-3 0px, + v.$neon-color-3 8px, transparent 8px, transparent 21px ); @@ -152,7 +152,7 @@ &.eve-system-status-lookingFor { background-image: linear-gradient(275deg, #45ff8f2f, #457fff2f); &.selected { - border-color: $pastel-pink; + border-color: v.$pastel-pink; } } diff --git a/assets/lib/phoenix/index.js b/assets/lib/phoenix/index.js index 2a0f4b63..0924049f 100644 --- a/assets/lib/phoenix/index.js +++ b/assets/lib/phoenix/index.js @@ -12,11 +12,11 @@ const animateBg = function (bgCanvas) { */ const randomInRange = (max, min) => Math.floor(Math.random() * (max - min + 1)) + min; const BASE_SIZE = 1; - const VELOCITY_INC = 1.01; + const VELOCITY_INC = 1.002; const VELOCITY_INIT_INC = 0.525; const JUMP_VELOCITY_INC = 0.55; const JUMP_SIZE_INC = 1.15; - const SIZE_INC = 1.01; + const SIZE_INC = 1.002; const RAD = Math.PI / 180; const WARP_COLORS = [ [197, 239, 247], diff --git a/lib/wanderer_app_web/components/layouts/blog.html.heex b/lib/wanderer_app_web/components/layouts/blog.html.heex index 80749d6f..4057229e 100644 --- a/lib/wanderer_app_web/components/layouts/blog.html.heex +++ b/lib/wanderer_app_web/components/layouts/blog.html.heex @@ -5,7 +5,7 @@
-
+
- Wanderer + Wanderer
@@ -45,7 +45,7 @@