diff --git a/lib/wanderer_app/api/map.ex b/lib/wanderer_app/api/map.ex index 982b8f6a..e72d5c9e 100644 --- a/lib/wanderer_app/api/map.ex +++ b/lib/wanderer_app/api/map.ex @@ -8,6 +8,8 @@ defmodule WandererApp.Api.Map do alias Ash.Resource.Change.Builtins + require Logger + postgres do repo(WandererApp.Repo) table("maps_v1") @@ -55,6 +57,7 @@ defmodule WandererApp.Api.Map do define(:mark_as_deleted, action: :mark_as_deleted) define(:update_api_key, action: :update_api_key) define(:toggle_webhooks, action: :toggle_webhooks) + define(:toggle_sse, action: :toggle_sse) define(:by_id, get_by: [:id], @@ -103,7 +106,16 @@ defmodule WandererApp.Api.Map do end create :new do - accept [:name, :slug, :description, :scope, :only_tracked_characters, :owner_id, :sse_enabled] + accept [ + :name, + :slug, + :description, + :scope, + :only_tracked_characters, + :owner_id, + :sse_enabled + ] + primary?(true) argument :create_default_acl, :boolean, allow_nil?: true argument :acls, {:array, :uuid}, allow_nil?: true @@ -188,6 +200,14 @@ defmodule WandererApp.Api.Map do require_atomic? false end + update :toggle_sse do + require_atomic? false + accept [:sse_enabled] + + # Validate subscription when enabling SSE + validate &validate_sse_subscription/2 + end + create :duplicate do accept [:name, :description, :scope, :only_tracked_characters] argument :source_map_id, :uuid, allow_nil?: false @@ -373,19 +393,13 @@ defmodule WandererApp.Api.Map do end end - # Private validation functions - - @doc false - # Validates that SSE can be enabled based on subscription status. + # SSE Subscription Validation # - # Validation rules: - # 1. Skip if SSE not being enabled (no validation needed) - # 2. Skip during map creation (map_id is nil, subscription doesn't exist yet) - # 3. Skip in Community Edition mode (subscriptions disabled globally) - # 4. Require active subscription in Enterprise mode - # - # This ensures users cannot enable SSE without a valid subscription in Enterprise mode, - # while allowing SSE in Community Edition and during map creation. + # This validation ensures that SSE can only be enabled when: + # 1. SSE is being disabled (always allowed) + # 2. Map is being created (skip validation, will be checked on first update) + # 3. Community Edition mode (always allowed) + # 4. Enterprise mode with active subscription defp validate_sse_subscription(changeset, _context) do sse_enabled = Ash.Changeset.get_attribute(changeset, :sse_enabled) map_id = changeset.data.id @@ -397,7 +411,6 @@ defmodule WandererApp.Api.Map do :ok # Map creation (no ID yet) - skip validation - # Subscription check will happen on first update if they try to enable SSE is_nil(map_id) -> :ok @@ -411,7 +424,6 @@ defmodule WandererApp.Api.Map do end end - # Helper to check if map has an active subscription defp validate_active_subscription(map_id) do case WandererApp.Map.is_subscription_active?(map_id) do {:ok, true} -> @@ -421,11 +433,8 @@ defmodule WandererApp.Api.Map do {:error, field: :sse_enabled, message: "Active subscription required to enable SSE"} {:error, reason} -> - require Logger - Logger.warning("Failed to check subscription for map #{map_id}: #{inspect(reason)}") - # Fail open - allow the operation but log the error - # This prevents database errors from blocking legitimate operations - :ok + Logger.error("Error checking subscription status: #{inspect(reason)}") + {:error, field: :sse_enabled, message: "Unable to verify subscription status"} end end end diff --git a/lib/wanderer_app/api/map_webhook_subscription.ex b/lib/wanderer_app/api/map_webhook_subscription.ex index e669ffb7..594cee61 100644 --- a/lib/wanderer_app/api/map_webhook_subscription.ex +++ b/lib/wanderer_app/api/map_webhook_subscription.ex @@ -58,6 +58,7 @@ defmodule WandererApp.Api.MapWebhookSubscription do :consecutive_failures, :secret ] + require_atomic? false end diff --git a/lib/wanderer_app/esi/api_client.ex b/lib/wanderer_app/esi/api_client.ex index a895aa6c..6013efac 100644 --- a/lib/wanderer_app/esi/api_client.ex +++ b/lib/wanderer_app/esi/api_client.ex @@ -463,7 +463,8 @@ defmodule WandererApp.Esi.ApiClient do {:error, reason} -> # Check if this is a Finch pool error - if is_exception(reason) and Exception.message(reason) =~ "unable to provide a connection" do + if is_exception(reason) and + Exception.message(reason) =~ "unable to provide a connection" do :telemetry.execute( [:wanderer_app, :finch, :pool_exhausted], %{count: 1}, @@ -677,7 +678,8 @@ defmodule WandererApp.Esi.ApiClient do {:error, reason} -> # Check if this is a Finch pool error - if is_exception(reason) and Exception.message(reason) =~ "unable to provide a connection" do + if is_exception(reason) and + Exception.message(reason) =~ "unable to provide a connection" do :telemetry.execute( [:wanderer_app, :finch, :pool_exhausted], %{count: 1}, diff --git a/lib/wanderer_app/kills/message_handler.ex b/lib/wanderer_app/kills/message_handler.ex index f9fd734b..b2c2fedd 100644 --- a/lib/wanderer_app/kills/message_handler.ex +++ b/lib/wanderer_app/kills/message_handler.ex @@ -403,10 +403,24 @@ defmodule WandererApp.Kills.MessageHandler do defp extract_field(_data, _field_names), do: nil - # Specific field extractors using the generic function + # Generic nested field extraction - tries flat keys first, then nested object + @spec extract_nested_field(map(), list(String.t()), String.t(), String.t()) :: String.t() | nil + defp extract_nested_field(data, flat_keys, nested_key, field) when is_map(data) do + case extract_field(data, flat_keys) do + nil -> + case data[nested_key] do + %{^field => value} when is_binary(value) and value != "" -> value + _ -> nil + end + + value -> + value + end + end + + # Specific field extractors using the generic functions @spec get_character_name(map() | any()) :: String.t() | nil defp get_character_name(data) when is_map(data) do - # Try multiple possible field names field_names = ["attacker_name", "victim_name", "character_name", "name"] extract_field(data, field_names) || @@ -419,30 +433,26 @@ defmodule WandererApp.Kills.MessageHandler do defp get_character_name(_), do: nil @spec get_corp_ticker(map() | any()) :: String.t() | nil - defp get_corp_ticker(data) when is_map(data) do - extract_field(data, ["corporation_ticker", "corp_ticker"]) - end + defp get_corp_ticker(data) when is_map(data), + do: extract_nested_field(data, ["corporation_ticker", "corp_ticker"], "corporation", "ticker") defp get_corp_ticker(_), do: nil @spec get_corp_name(map() | any()) :: String.t() | nil - defp get_corp_name(data) when is_map(data) do - extract_field(data, ["corporation_name", "corp_name"]) - end + defp get_corp_name(data) when is_map(data), + do: extract_nested_field(data, ["corporation_name", "corp_name"], "corporation", "name") defp get_corp_name(_), do: nil @spec get_alliance_ticker(map() | any()) :: String.t() | nil - defp get_alliance_ticker(data) when is_map(data) do - extract_field(data, ["alliance_ticker"]) - end + defp get_alliance_ticker(data) when is_map(data), + do: extract_nested_field(data, ["alliance_ticker"], "alliance", "ticker") defp get_alliance_ticker(_), do: nil @spec get_alliance_name(map() | any()) :: String.t() | nil - defp get_alliance_name(data) when is_map(data) do - extract_field(data, ["alliance_name"]) - end + defp get_alliance_name(data) when is_map(data), + do: extract_nested_field(data, ["alliance_name"], "alliance", "name") defp get_alliance_name(_), do: nil diff --git a/lib/wanderer_app_web/live/admin/admin_live.html.heex b/lib/wanderer_app_web/live/admin/admin_live.html.heex index 4fd76ff4..9dbbe3a3 100644 --- a/lib/wanderer_app_web/live/admin/admin_live.html.heex +++ b/lib/wanderer_app_web/live/admin/admin_live.html.heex @@ -336,8 +336,8 @@ label="Valid" options={Enum.map(@valid_types, fn valid_type -> {valid_type.label, valid_type.id} end)} /> - - + + + +
+

Server-Sent Events (SSE)

+
+ +
+

+ When enabled, external clients can subscribe to real-time map events via SSE. +

+
<.live_component diff --git a/priv/repo/migrations/20251118202140_add_public_api_key_unique_index.exs b/priv/repo/migrations/20251118202140_add_public_api_key_unique_index.exs index 5d20a4f7..9fac4820 100644 --- a/priv/repo/migrations/20251118202140_add_public_api_key_unique_index.exs +++ b/priv/repo/migrations/20251118202140_add_public_api_key_unique_index.exs @@ -52,25 +52,24 @@ defmodule WandererApp.Repo.Migrations.AddPublicApiKeyUniqueIndex do end defp create_backup_table do - repo().query!(""" - CREATE TABLE IF NOT EXISTS maps_v1_api_key_backup ( - id UUID PRIMARY KEY, - map_id UUID NOT NULL, - old_public_api_key TEXT NOT NULL, - reason TEXT NOT NULL, - backed_up_at TIMESTAMP NOT NULL DEFAULT NOW() + repo().query!( + """ + CREATE TABLE IF NOT EXISTS maps_v1_api_key_backup ( + id UUID PRIMARY KEY, + map_id UUID NOT NULL, + old_public_api_key TEXT NOT NULL, + reason TEXT NOT NULL, + backed_up_at TIMESTAMP NOT NULL DEFAULT NOW() + ) + """, + [] ) - """, []) IO.puts("Created backup table maps_v1_api_key_backup") end def down do - drop_if_exists( - index(:maps_v1, [:public_api_key], - name: :maps_v1_unique_public_api_key_index - ) - ) + drop_if_exists(index(:maps_v1, [:public_api_key], name: :maps_v1_unique_public_api_key_index)) IO.puts("Dropped unique index on maps_v1.public_api_key") @@ -119,6 +118,7 @@ defmodule WandererApp.Repo.Migrations.AddPublicApiKeyUniqueIndex do INSERT INTO maps_v1_api_key_backup (id, map_id, old_public_api_key, reason) VALUES (gen_random_uuid(), $1::uuid, $2, 'duplicate_api_key_cleared_for_unique_index') """ + repo().query!(backup_query, [id, api_key]) # Clear the duplicate diff --git a/test/integration/api/v1/map_system_api_v1_test.exs b/test/integration/api/v1/map_system_api_v1_test.exs index ef0576df..3732191b 100644 --- a/test/integration/api/v1/map_system_api_v1_test.exs +++ b/test/integration/api/v1/map_system_api_v1_test.exs @@ -247,9 +247,10 @@ defmodule WandererAppWeb.Api.V1.MapSystemApiV1Test do payload = %{ "data" => %{ "type" => "map_systems", - "attributes" => %{ - # Missing solar_system_id - JSON:API returns 400 for schema validation - } + "attributes" => + %{ + # Missing solar_system_id - JSON:API returns 400 for schema validation + } } } diff --git a/test/integration/map/character_location_tracking_test.exs b/test/integration/map/character_location_tracking_test.exs index 30e3a927..70b326d1 100644 --- a/test/integration/map/character_location_tracking_test.exs +++ b/test/integration/map/character_location_tracking_test.exs @@ -47,24 +47,26 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do user = create_user(%{name: "Test User", hash: "test_hash_#{:rand.uniform(1_000_000)}"}) # Create test character with location tracking scopes - character = create_character(%{ - eve_id: "#{@test_character_eve_id}", - name: "Test Character", - user_id: user.id, - scopes: "esi-location.read_location.v1 esi-location.read_ship_type.v1", - tracking_pool: "default" - }) + character = + create_character(%{ + eve_id: "#{@test_character_eve_id}", + name: "Test Character", + user_id: user.id, + scopes: "esi-location.read_location.v1 esi-location.read_ship_type.v1", + tracking_pool: "default" + }) # Create test map # Note: scope: :all is used because :none prevents system addition # (is_connection_valid returns false for :none scope) - map = create_map(%{ - name: "Test Char Track", - slug: "test-char-tracking-#{:rand.uniform(1_000_000)}", - owner_id: character.id, - scope: :all, - only_tracked_characters: false - }) + map = + create_map(%{ + name: "Test Char Track", + slug: "test-char-tracking-#{:rand.uniform(1_000_000)}", + owner_id: character.id, + scope: :all, + only_tracked_characters: false + }) on_exit(fn -> cleanup_test_data(map.id) @@ -150,6 +152,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Setup: Character starts at Jita set_character_location(character.id, @system_jita) + WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", @system_jita @@ -157,6 +160,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # First update - start system is intentionally NOT added yet CharactersImpl.update_characters(map.id) + refute system_on_map?(map.id, @system_jita), "Start system should not be added until character moves" @@ -167,8 +171,11 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do CharactersImpl.update_characters(map.id) # Verify: Both systems should be on map after character moves - assert wait_for_system_on_map(map.id, @system_jita), "Jita should be added after character moves" - assert wait_for_system_on_map(map.id, @system_amarr), "Amarr should be added as the new location" + assert wait_for_system_on_map(map.id, @system_jita), + "Jita should be added after character moves" + + assert wait_for_system_on_map(map.id, @system_amarr), + "Amarr should be added as the new location" end end @@ -185,6 +192,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Character starts at Jita set_character_location(character.id, @system_jita) + WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", @system_jita @@ -192,6 +200,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # First update - start system is intentionally NOT added yet CharactersImpl.update_characters(map.id) + refute system_on_map?(map.id, @system_jita), "Start system should not be added until character moves" @@ -202,7 +211,9 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do CharactersImpl.update_characters(map.id) # Verify both Jita and Amarr are now on map - assert wait_for_system_on_map(map.id, @system_jita), "Jita (start) should be on map after movement" + assert wait_for_system_on_map(map.id, @system_jita), + "Jita (start) should be on map after movement" + assert wait_for_system_on_map(map.id, @system_amarr), "Amarr should be on map" # Rapid jump to Dodixie before next update cycle @@ -213,7 +224,10 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Verify: All three systems should be on map assert wait_for_system_on_map(map.id, @system_jita), "Jita (start) should still be on map" - assert wait_for_system_on_map(map.id, @system_amarr), "Amarr (intermediate) should still be on map - this is the critical test" + + assert wait_for_system_on_map(map.id, @system_amarr), + "Amarr (intermediate) should still be on map - this is the critical test" + assert wait_for_system_on_map(map.id, @system_dodixie), "Dodixie (end) should be on map" end @@ -230,6 +244,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Start at Jita set_character_location(character.id, @system_jita) + WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", @system_jita @@ -284,9 +299,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Verify start_solar_system_id still exists after first update {:ok, start_system} = - WandererApp.Cache.lookup( - "map:#{map.id}:character:#{character.id}:start_solar_system_id" - ) + WandererApp.Cache.lookup("map:#{map.id}:character:#{character.id}:start_solar_system_id") assert start_system == @system_jita, "start_solar_system_id should persist after first update (not be taken/removed)" @@ -369,6 +382,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Set character at Jita and set start location set_character_location(character.id, @system_jita) + WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", @system_jita @@ -401,6 +415,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Set up character location set_character_location(character.id, @system_jita) + WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", @system_jita @@ -424,19 +439,22 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Create a second character user2 = create_user(%{name: "Test User 2", hash: "test_hash_#{:rand.uniform(1_000_000)}"}) - character2 = create_character(%{ - eve_id: "#{@test_character_eve_id + 1}", - name: "Test Character 2", - user_id: user2.id, - scopes: "esi-location.read_location.v1 esi-location.read_ship_type.v1", - tracking_pool: "default" - }) + + character2 = + create_character(%{ + eve_id: "#{@test_character_eve_id + 1}", + name: "Test Character 2", + user_id: user2.id, + scopes: "esi-location.read_location.v1 esi-location.read_ship_type.v1", + tracking_pool: "default" + }) # Add both characters to map presence add_character_to_map_presence(map.id, character2.id) # Set locations for both characters set_character_location(character2.id, @system_amarr) + WandererApp.Cache.insert( "map:#{map.id}:character:#{character2.id}:start_solar_system_id", @system_amarr @@ -464,6 +482,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Set up character with location set_character_location(character.id, @system_jita) + WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", @system_jita @@ -491,32 +510,38 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do ensure_map_started(map.id) # Create multiple characters for concurrent processing - characters = for i <- 1..5 do - user = create_user(%{ - name: "Test User #{i}", - hash: "test_hash_#{:rand.uniform(1_000_000)}" - }) + characters = + for i <- 1..5 do + user = + create_user(%{ + name: "Test User #{i}", + hash: "test_hash_#{:rand.uniform(1_000_000)}" + }) - character = create_character(%{ - eve_id: "#{@test_character_eve_id + i}", - name: "Test Character #{i}", - user_id: user.id, - scopes: "esi-location.read_location.v1 esi-location.read_ship_type.v1", - tracking_pool: "default" - }) + character = + create_character(%{ + eve_id: "#{@test_character_eve_id + i}", + name: "Test Character #{i}", + user_id: user.id, + scopes: "esi-location.read_location.v1 esi-location.read_ship_type.v1", + tracking_pool: "default" + }) - # Add character to presence and set location - add_character_to_map_presence(map.id, character.id) + # Add character to presence and set location + add_character_to_map_presence(map.id, character.id) - solar_system_id = Enum.at([@system_jita, @system_amarr, @system_dodixie, @system_rens], rem(i, 4)) - set_character_location(character.id, solar_system_id) - WandererApp.Cache.insert( - "map:#{map.id}:character:#{character.id}:start_solar_system_id", - solar_system_id - ) + solar_system_id = + Enum.at([@system_jita, @system_amarr, @system_dodixie, @system_rens], rem(i, 4)) - character - end + set_character_location(character.id, solar_system_id) + + WandererApp.Cache.insert( + "map:#{map.id}:character:#{character.id}:start_solar_system_id", + solar_system_id + ) + + character + end # Run update_characters - should handle all characters concurrently result = CharactersImpl.update_characters(map.id) @@ -563,7 +588,8 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do CharactersImpl.update_characters(map.id) # Should receive start and complete events (or error event if something failed) - assert_receive {:telemetry_event, [:wanderer_app, :map, :update_characters, :start], _, _}, 1000 + assert_receive {:telemetry_event, [:wanderer_app, :map, :update_characters, :start], _, _}, + 1000 # Should receive either complete or error event receive do @@ -593,6 +619,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Set location in character cache set_character_location(character.id, @system_jita) + WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", @system_jita @@ -613,10 +640,12 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Verify both caches updated {:ok, character_data} = Cachex.get(:character_cache, character.id) + {:ok, map_cached_location} = WandererApp.Cache.lookup("map:#{map.id}:character:#{character.id}:solar_system_id") assert character_data.solar_system_id == @system_amarr + assert map_cached_location == @system_amarr, "Both caches should be consistent after update" end diff --git a/test/support/map_test_helpers.ex b/test/support/map_test_helpers.ex index 0b6c2419..dc35c081 100644 --- a/test/support/map_test_helpers.ex +++ b/test/support/map_test_helpers.ex @@ -356,7 +356,8 @@ defmodule WandererApp.MapTestHelpers do def set_character_location(character_id, solar_system_id, opts \\ []) do structure_id = opts[:structure_id] station_id = opts[:station_id] - ship = opts[:ship] || 670 # Capsule + # Capsule + ship = opts[:ship] || 670 # First get the existing character from cache or database to maintain all fields {:ok, existing_character} = WandererApp.Character.get_character(character_id)