diff --git a/lib/wanderer_app/repositories/map_repo.ex b/lib/wanderer_app/repositories/map_repo.ex index 9a06da84..a99d441a 100644 --- a/lib/wanderer_app/repositories/map_repo.ex +++ b/lib/wanderer_app/repositories/map_repo.ex @@ -21,11 +21,33 @@ defmodule WandererApp.MapRepo do {:ok, map} -> map |> load_relationships(relationships) - _ -> + # Ash wraps NotFound inside Ash.Error.Invalid, so match the wrapped shape + # as well as a bare NotFound. Getting this wrong turns a genuine + # "map does not exist" into a generic error for every caller. + {:error, %Ash.Error.Query.NotFound{}} -> {:error, :not_found} + + {:error, %Ash.Error.Invalid{errors: errors}} = error -> + if Enum.any?(errors, &match?(%Ash.Error.Query.NotFound{}, &1)) do + {:error, :not_found} + else + log_get_failure(map_id, error) + error + end + + {:error, _reason} = error -> + log_get_failure(map_id, error) + error end end + # Previously every error was flattened into {:error, :not_found}, which masked + # infrastructure faults (e.g. DBConnection ownership errors) as "map does not + # exist" and made them very hard to diagnose. + defp log_get_failure(map_id, {:error, reason}) do + Logger.error("MapRepo.get failed for map #{inspect(map_id)}: #{inspect(reason)}") + end + def get_by_slug_with_permissions(map_slug, current_user) do map_slug |> WandererApp.Api.Map.get_map_by_slug!() diff --git a/test/integration/map/character_location_tracking_test.exs b/test/integration/map/character_location_tracking_test.exs index 70b326d1..4420fc6a 100644 --- a/test/integration/map/character_location_tracking_test.exs +++ b/test/integration/map/character_location_tracking_test.exs @@ -19,6 +19,11 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do use WandererApp.IntegrationCase, async: false + # These tests start real map servers. The MapPool GenServer loads map state + # from the database during init, so it needs shared sandbox mode -- see + # WandererApp.IntegrationCase. + @moduletag :shared_sandbox + import Mox setup :verify_on_exit! @@ -26,15 +31,29 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do import WandererApp.MapTestHelpers alias WandererApp.Map.Server.CharactersImpl - alias WandererApp.Map.Server.SystemsImpl @test_character_eve_id 2_123_456_789 - # EVE Online solar system IDs for testing - @system_jita 30_000_142 + # EVE Online solar system IDs for testing. + # + # NOTE: Jita (30_000_142) is deliberately NOT used here. It is hardcoded as + # permanently un-addable in production: + # + # # map_server_connections_impl.ex + # @jita 30_000_142 + # @prohibited_systems [@jita] + # + # These tests previously used Jita as the "start system" and asserted it was + # added to the map, which asserts behaviour the application forbids by design + # and can never pass. Hek is a plain hi-sec system with no such restriction, so + # the intent of each test (a system the character moved away from gets added) + # is preserved while testing behaviour that is actually reachable. + # The prohibition itself is pinned by its own regression test below. + @system_hek 30_002_053 @system_amarr 30_002_187 @system_dodixie 30_002_659 @system_rens 30_002_510 + @system_jita_prohibited 30_000_142 setup do # Setup system static info cache for test systems @@ -65,6 +84,11 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do slug: "test-char-tracking-#{:rand.uniform(1_000_000)}", owner_id: character.id, scope: :all, + # `scopes` (array) takes precedence over `scope` in + # CharactersImpl.get_effective_scopes/1 and defaults to [:wormholes]. + # Without setting it, hi-sec movement is rejected as an invalid + # connection and no system is ever added. + scopes: [:hi, :low, :null, :pochven, :wormholes], only_tracked_characters: false }) @@ -93,7 +117,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do @tag :integration test "character location update adds system to map", %{map: map, character: character} do # This test verifies the basic flow: - # 1. Character starts tracking on a map at Jita + # 1. Character starts tracking on a map at Hek # 2. Character moves to Amarr # 3. update_characters() is called # 4. Both systems are added to the map @@ -102,23 +126,23 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do ensure_map_started(map.id) # Setup: Add character to presence - add_character_to_map_presence(map.id, character.id) + track_character_on_map(map.id, character.id) - # Setup: Character starts at Jita - set_character_location(character.id, @system_jita) + # Setup: Character starts at Hek + set_character_location(character.id, @system_hek) # Setup: Set start_solar_system_id (this happens when tracking starts) # Note: The start system is NOT added until the character moves WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", - @system_jita + @system_hek ) # Execute: First update - start system is intentionally NOT added yet CharactersImpl.update_characters(map.id) - # Verify: Jita should NOT be on map yet (design: start position not added) - refute system_on_map?(map.id, @system_jita), + # Verify: Hek should NOT be on map yet (design: start position not added) + refute system_on_map?(map.id, @system_hek), "Start system should not be added until character moves" # Character moves to Amarr @@ -128,8 +152,8 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do CharactersImpl.update_characters(map.id) # Verify: Both systems should now be on map - 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_hek), + "Hek should be added after character moves" assert wait_for_system_on_map(map.id, @system_amarr), "Amarr should be added as the new location" @@ -148,20 +172,20 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do ensure_map_started(map.id) # Setup: Add character to presence - add_character_to_map_presence(map.id, character.id) + track_character_on_map(map.id, character.id) - # Setup: Character starts at Jita - set_character_location(character.id, @system_jita) + # Setup: Character starts at Hek + set_character_location(character.id, @system_hek) WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", - @system_jita + @system_hek ) # First update - start system is intentionally NOT added yet CharactersImpl.update_characters(map.id) - refute system_on_map?(map.id, @system_jita), + refute system_on_map?(map.id, @system_hek), "Start system should not be added until character moves" # Character moves to Amarr @@ -171,8 +195,8 @@ 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_hek), + "Hek should be added after character moves" assert wait_for_system_on_map(map.id, @system_amarr), "Amarr should be added as the new location" @@ -188,31 +212,31 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Note: Start system is NOT added until character moves (design decision) ensure_map_started(map.id) - add_character_to_map_presence(map.id, character.id) + track_character_on_map(map.id, character.id) - # Character starts at Jita - set_character_location(character.id, @system_jita) + # Character starts at Hek + set_character_location(character.id, @system_hek) WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", - @system_jita + @system_hek ) # First update - start system is intentionally NOT added yet CharactersImpl.update_characters(map.id) - refute system_on_map?(map.id, @system_jita), + refute system_on_map?(map.id, @system_hek), "Start system should not be added until character moves" # Rapid jump to Amarr (intermediate system) set_character_location(character.id, @system_amarr) - # Second update - should add both Jita (start) and Amarr (current) + # Second update - should add both Hek (start) and Amarr (current) 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" + # Verify both Hek and Amarr are now on map + assert wait_for_system_on_map(map.id, @system_hek), + "Hek (start) should be on map after movement" assert wait_for_system_on_map(map.id, @system_amarr), "Amarr should be on map" @@ -223,7 +247,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do CharactersImpl.update_characters(map.id) # 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_hek), "Hek (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" @@ -240,21 +264,21 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # don't cause intermediate systems to be lost due to cache races. ensure_map_started(map.id) - add_character_to_map_presence(map.id, character.id) + track_character_on_map(map.id, character.id) - # Start at Jita - set_character_location(character.id, @system_jita) + # Start at Hek + set_character_location(character.id, @system_hek) WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", - @system_jita + @system_hek ) CharactersImpl.update_characters(map.id) # Simulate rapid updates happening faster than update_characters cycle (1 second) # Jump through 4 systems in quick succession - systems = [@system_amarr, @system_dodixie, @system_rens, @system_jita] + systems = [@system_amarr, @system_dodixie, @system_rens, @system_hek] for system <- systems do set_character_location(character.id, system) @@ -266,7 +290,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Verify: All systems should eventually be on the map # Even if some updates happened concurrently - for system <- [@system_jita | systems] do + for system <- [@system_hek | systems] do assert wait_for_system_on_map(map.id, system), "System #{system} should be on map despite rapid movements" end @@ -283,15 +307,15 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # start_solar_system_id should not be lost after first use ensure_map_started(map.id) - add_character_to_map_presence(map.id, character.id) + track_character_on_map(map.id, character.id) - # Set character at Jita - set_character_location(character.id, @system_jita) + # Set character at Hek + set_character_location(character.id, @system_hek) # Set start_solar_system_id WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", - @system_jita + @system_hek ) # First update @@ -301,7 +325,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do {:ok, start_system} = WandererApp.Cache.lookup("map:#{map.id}:character:#{character.id}:start_solar_system_id") - assert start_system == @system_jita, + assert start_system == @system_hek, "start_solar_system_id should persist after first update (not be taken/removed)" # Character moves to Amarr @@ -311,7 +335,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do CharactersImpl.update_characters(map.id) # Verify both systems are on map - assert wait_for_system_on_map(map.id, @system_jita) + assert wait_for_system_on_map(map.id, @system_hek) assert wait_for_system_on_map(map.id, @system_amarr) end @@ -325,22 +349,22 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Design: Start system is NOT added until character moves ensure_map_started(map.id) - add_character_to_map_presence(map.id, character.id) + track_character_on_map(map.id, character.id) - # Character is at Jita, no previous location - set_character_location(character.id, @system_jita) + # Character is at Hek, no previous location + set_character_location(character.id, @system_hek) # Set start_solar_system_id WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", - @system_jita + @system_hek ) # First update - character still at start position CharactersImpl.update_characters(map.id) - # Verify Jita is NOT added yet (design: start position not added until movement) - refute system_on_map?(map.id, @system_jita), + # Verify Hek is NOT added yet (design: start position not added until movement) + refute system_on_map?(map.id, @system_hek), "Start system should not be added until character moves" # Character moves to Amarr @@ -350,8 +374,8 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do CharactersImpl.update_characters(map.id) # Verify both systems are added after movement - assert wait_for_system_on_map(map.id, @system_jita), - "Jita should be added after character moves away" + assert wait_for_system_on_map(map.id, @system_hek), + "Hek should be added after character moves away" assert wait_for_system_on_map(map.id, @system_amarr), "Amarr should be added as the new location" @@ -366,7 +390,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # (Retry logic not yet implemented) ensure_map_started(map.id) - add_character_to_map_presence(map.id, character.id) + track_character_on_map(map.id, character.id) test_pid = self() @@ -380,12 +404,12 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do nil ) - # Set character at Jita and set start location - set_character_location(character.id, @system_jita) + # Set character at Hek and set start location + set_character_location(character.id, @system_hek) WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", - @system_jita + @system_hek ) # Trigger update which may encounter database issues @@ -411,14 +435,14 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # and logged without crashing the entire update_characters cycle ensure_map_started(map.id) - add_character_to_map_presence(map.id, character.id) + track_character_on_map(map.id, character.id) # Set up character location - set_character_location(character.id, @system_jita) + set_character_location(character.id, @system_hek) WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", - @system_jita + @system_hek ) # Run update_characters - should complete even if individual character updates fail @@ -478,14 +502,14 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # Note: Recovery ETS table not yet implemented ensure_map_started(map.id) - add_character_to_map_presence(map.id, character.id) + track_character_on_map(map.id, character.id) # Set up character with location - set_character_location(character.id, @system_jita) + set_character_location(character.id, @system_hek) WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", - @system_jita + @system_hek ) # Run multiple update cycles to verify stability @@ -528,10 +552,10 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do }) # Add character to presence and set location - add_character_to_map_presence(map.id, character.id) + track_character_on_map(map.id, character.id) solar_system_id = - Enum.at([@system_jita, @system_amarr, @system_dodixie, @system_rens], rem(i, 4)) + Enum.at([@system_hek, @system_amarr, @system_dodixie, @system_rens], rem(i, 4)) set_character_location(character.id, solar_system_id) @@ -563,7 +587,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # emit proper telemetry events for monitoring ensure_map_started(map.id) - add_character_to_map_presence(map.id, character.id) + track_character_on_map(map.id, character.id) test_pid = self() @@ -582,7 +606,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do ) # Set up character location - set_character_location(character.id, @system_jita) + set_character_location(character.id, @system_hek) # Trigger update_characters CharactersImpl.update_characters(map.id) @@ -615,14 +639,14 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do # 3. Cachex.get(:character_state_cache, character_id) - character state ensure_map_started(map.id) - add_character_to_map_presence(map.id, character.id) + track_character_on_map(map.id, character.id) # Set location in character cache - set_character_location(character.id, @system_jita) + set_character_location(character.id, @system_hek) WandererApp.Cache.insert( "map:#{map.id}:character:#{character.id}:start_solar_system_id", - @system_jita + @system_hek ) CharactersImpl.update_characters(map.id) @@ -631,7 +655,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do {:ok, map_cached_location} = WandererApp.Cache.lookup("map:#{map.id}:character:#{character.id}:solar_system_id") - assert map_cached_location == @system_jita, + assert map_cached_location == @system_hek, "Map-specific cache should match character cache" # Move character @@ -685,7 +709,7 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do %{ character_id: character.id, map_id: map.id, - from_system: @system_jita, + from_system: @system_hek, to_system: @system_amarr } ) @@ -702,4 +726,43 @@ defmodule WandererApp.Map.CharacterLocationTrackingTest do :telemetry.detach("test-character-location-events") end end + + describe "Prohibited systems" do + @tag :integration + test "Jita is never added to a map, even when a character moves through it", + %{map: map, character: character} do + # Pins the reason every other test in this module uses Hek rather than + # Jita as its start system. Jita is hardcoded as permanently un-addable + # (`@prohibited_systems [@jita]` in map_server_connections_impl.ex). + # + # If this test starts failing, the prohibition was changed or removed -- + # revisit the constants at the top of this file rather than deleting this. + ensure_map_started(map.id) + track_character_on_map(map.id, character.id) + + set_character_location(character.id, @system_jita_prohibited) + + WandererApp.Cache.insert( + "map:#{map.id}:character:#{character.id}:start_solar_system_id", + @system_jita_prohibited + ) + + CharactersImpl.update_characters(map.id) + + # Move away from Jita. For any non-prohibited system this is exactly the + # sequence that causes the start system to be added to the map. + set_character_location(character.id, @system_amarr) + CharactersImpl.update_characters(map.id) + + # The prohibition is evaluated per-connection, not per-system: Jita being + # prohibited invalidates the whole Jita->Amarr movement, so neither + # endpoint is added. Assert both, so the test pins the actual semantics + # rather than a convenient half of them. + refute system_on_map?(map.id, @system_jita_prohibited), + "Jita is in @prohibited_systems and must never be added to a map" + + refute system_on_map?(map.id, @system_amarr), + "A connection touching a prohibited system is rejected entirely" + end + end end diff --git a/test/support/integration_case.ex b/test/support/integration_case.ex index bebfa7b3..047124f8 100644 --- a/test/support/integration_case.ex +++ b/test/support/integration_case.ex @@ -2,17 +2,19 @@ defmodule WandererApp.IntegrationCase do @moduledoc """ This module defines the test case for integration tests. - Integration tests use shared sandbox mode (`shared: true`) when running async - to avoid timing issues with dynamically spawned processes like MapPool GenServers - that need database access immediately upon spawn. + Integration tests default to a private sandbox owner. Suites that start real + map servers must opt into shared sandbox mode: - For async integration tests, shared mode allows: - - MapPool GenServers to access the database without explicit allowance - - Tests to run in parallel without complex permission granting - - Reliable test execution without race conditions + use WandererApp.IntegrationCase, async: false + @moduletag :shared_sandbox - For synchronous integration tests, shared mode is disabled (shared: false) - for better isolation. + Shared mode is required when a dynamically spawned process queries the + database immediately upon spawn (MapPool GenServers load map state during + `init`), because there is no window in which the test process can allow it + onto the connection first. + + Shared mode is node-global, so it is opt-in and reverted on exit. It is only + valid with `async: false`. Use this case for: - API controller integration tests that spawn map servers @@ -62,15 +64,18 @@ defmodule WandererApp.IntegrationCase do end @doc """ - Sets up the sandbox with shared mode for async integration tests. + Sets up the test sandbox, per the `:shared_sandbox` moduletag. - For async tests (async: true): - - Uses shared: true to allow dynamically spawned processes database access - - Trades some isolation for reliability and simplicity + With `@moduletag :shared_sandbox` (only valid with `async: false`): + - Uses shared: true so dynamically spawned processes (e.g. MapPool + GenServers that query the DB during `init`) get database access + - Trades some isolation for reliability with background processes - For sync tests (async: false): - - Uses shared: false for better isolation + Without the tag (the default): + - Starts a dedicated, private sandbox owner (shared: false) - Child processes require explicit allowance + + Raises `ArgumentError` if `:shared_sandbox` is set on an `async: true` suite. """ def setup_sandbox(tags) do # Ensure the repo is started before setting up sandbox @@ -78,20 +83,39 @@ defmodule WandererApp.IntegrationCase do {:ok, _} = WandererApp.Repo.start_link() end - # For integration tests: - # - Use shared: true for async tests to avoid MapPool timing issues - # - Use shared: false for sync tests for better isolation - shared_mode = tags[:async] == true + # Shared mode is opt-in per suite via `@moduletag :shared_sandbox`. + # + # Suites that start real map servers need it: MapPool GenServers are spawned + # dynamically and load map state from the DB *during init*, so there is no + # point at which the test process can allow them onto the connection first + # (polling loses the race). Shared mode is the only mechanism that covers a + # process that queries immediately upon spawn. + # + # It is opt-in rather than global because Sandbox shared mode applies to the + # whole node: enabling it for every sync integration suite regresses suites + # that rely on owner-private connections. Ecto only permits shared mode when + # the test is not async, so the tag is rejected on async suites. + shared_mode = tags[:shared_sandbox] == true + + if shared_mode and tags[:async] == true do + raise ArgumentError, + "#{inspect(tags[:module])} sets @moduletag :shared_sandbox but is `async: true`. " <> + "Ecto sandbox shared mode is node-global and is only safe with `async: false`." + end # Set up sandbox mode based on test type pid = if shared_mode do - # For async tests with shared mode: - # Checkout the sandbox connection instead of starting an owner - # This allows multiple async tests to use the same connection pool :ok = Ecto.Adapters.SQL.Sandbox.checkout(WandererApp.Repo) - # Put the connection in shared mode Ecto.Adapters.SQL.Sandbox.mode(WandererApp.Repo, {:shared, self()}) + + # Shared mode is node-global, so it MUST be reverted to :manual when the + # test ends. Leaving it set leaks into every later suite on the node and + # is why an earlier global-flip attempt broke unrelated controller tests. + on_exit(fn -> + Ecto.Adapters.SQL.Sandbox.mode(WandererApp.Repo, :manual) + end) + self() else # For sync tests, start a dedicated owner @@ -117,10 +141,15 @@ defmodule WandererApp.IntegrationCase do Allows a process to access the database by granting it sandbox access. This is necessary for background processes that need database access in non-shared mode. """ - def allow_database_access(pid) when is_pid(pid) do - owner_pid = Process.get(:sandbox_owner_pid) + def allow_database_access(pid, owner_pid \\ nil) when is_pid(pid) do + owner_pid = owner_pid || Process.get(:sandbox_owner_pid) if owner_pid do + # Returns `:ok | {:already, :owner | :allowed}` on every poll tick; + # re-allowing an already-allowed process is a normal, non-error case, so + # both are treated as success here. It raises only for infrastructure + # faults (repo not started, or `pid`/`owner_pid` not resolving to a live + # process) -- those should surface rather than be swallowed. Ecto.Adapters.SQL.Sandbox.allow(WandererApp.Repo, owner_pid, pid) end end @@ -192,6 +221,23 @@ defmodule WandererApp.IntegrationCase do |> Enum.filter(&is_pid/1) |> Enum.filter(&Process.alive?/1) |> Enum.each(fn child_pid -> + # Grant BOTH mock ownership and Ecto sandbox access. + # + # MapPool GenServers are spawned dynamically *after* the one-shot + # grant_supervision_tree_access/2 call above has already run, so + # they are never on the sandbox connection. Their map-state + # loaders (Task.async in Map.Server.Impl.do_init_state/1) then die + # with DBConnection.OwnershipError, the error is reported as + # "map not loaded", and the test surfaces only a misleading + # "Timeout waiting for map ... Check Map.Manager is running". + # + # Sandbox.allow/3 also propagates to the pool's Task.async + # children via $callers, which is what the loaders rely on. + # + # owner_pid is passed explicitly: this runs inside the spawned + # monitor process, where the :sandbox_owner_pid process dict entry + # set by setup_sandbox/1 is not visible. + allow_database_access(child_pid, owner_pid) WandererApp.Test.MockOwnership.allow_mocks_for_process(child_pid, owner_pid) end) diff --git a/test/support/map_test_helpers.ex b/test/support/map_test_helpers.ex index 31c3453d..ba31fdb2 100644 --- a/test/support/map_test_helpers.ex +++ b/test/support/map_test_helpers.ex @@ -322,6 +322,29 @@ defmodule WandererApp.MapTestHelpers do wandering: [], triglavian_invasion_status: nil, sun_type_id: 45041 + }, + # Hek. Used as the default "start system" in location-tracking tests + # because, unlike Jita, it is not in @prohibited_systems and so can + # actually be added to a map. + 30_002_053 => %{ + solar_system_id: 30_002_053, + region_id: 10_000_042, + constellation_id: 20_000_302, + solar_system_name: "Hek", + solar_system_name_lc: "hek", + constellation_name: "Hedgehog", + region_name: "Metropolis", + system_class: 0, + security: "0.5", + type_description: "High Security", + class_title: "High Sec", + is_shattered: false, + effect_name: nil, + effect_power: nil, + statics: [], + wandering: [], + triglavian_invasion_status: nil, + sun_type_id: 45041 } } end @@ -333,7 +356,7 @@ defmodule WandererApp.MapTestHelpers do ## Parameters - character_id: The character ID to update - solar_system_id: The solar system ID where the character is located - - opts: Optional parameters (structure_id, station_id, ship) + - opts: Optional parameters (structure_id, station_id, ship, online) ## Examples iex> set_character_location(character.id, 30_000_142, ship: 670) @@ -349,12 +372,18 @@ defmodule WandererApp.MapTestHelpers do {:ok, existing_character} = WandererApp.Character.get_character(character_id) # Update character cache (mimics Character.update_character/2) + # `online: true` is required, not cosmetic. When a character is offline the + # `{:character_online, ...}` handler in CharactersImpl DELETES the + # `map::character::solar_system_id` cache key that + # `check_location_update` just wrote, so no location change is ever observed + # and no system is added. Characters reporting a location are online. character_data = Map.merge(existing_character, %{ solar_system_id: solar_system_id, structure_id: structure_id, station_id: station_id, ship: ship, + online: Keyword.get(opts, :online, true), updated_at: DateTime.utc_now() }) @@ -379,6 +408,45 @@ defmodule WandererApp.MapTestHelpers do WandererApp.Cache.insert("map_#{map_id}:presence_character_ids", updated_chars) end + @doc """ + Makes a character actually tracked for `update_characters/1`. + + Presence alone is NOT enough. `WandererApp.Map.get_tracked_character_ids/1` + requires both: + + * the character to be registered on the map (`map.characters`), and + * a `character::map::tracking_start_time` cache key + + Production sets these via `CharactersImpl.track_character/2`, which needs real + `MapCharacterSettings` rows and ESI access tokens. Tests short-circuit to the + same end state, mirroring `Character.TrackingUtils.track_character/3`. + + Without this, `update_characters/1` iterates an empty list and silently does + nothing, which reads as "the feature is broken" rather than "the character was + never tracked". + """ + def track_character_on_map(map_id, character_id) do + add_character_to_map_presence(map_id, character_id) + + {:ok, character} = WandererApp.Character.get_character(character_id) + :ok = WandererApp.Map.add_character(map_id, character) + + WandererApp.Cache.insert( + "character:#{character_id}:map:#{map_id}:tracking_start_time", + DateTime.utc_now() + ) + + # Production clears stale per-map location caches when tracking starts + # (TrackingUtils.track_character/4). Mirror it, so a character tracked after + # a previous location was cached does not appear to have "already been" at + # that system. + WandererApp.Cache.delete("map:#{map_id}:character:#{character_id}:solar_system_id") + WandererApp.Cache.delete("map:#{map_id}:character:#{character_id}:station_id") + WandererApp.Cache.delete("map:#{map_id}:character:#{character_id}:structure_id") + + :ok + end + @doc """ Helper to get all systems currently on the map. Uses :map_cache instead of :map_state_cache because add_system/2 updates :map_cache.