fix: unwrap Repo.transaction return values for ecto_sql 3.14, fix sandbox deadlock in rover_planning tests

- accounts.ex: Repo.transaction in ecto_sql 3.14 wraps all return values in {:ok, ...},
  causing callers to receive {:ok, {:error, ...}} and {:ok, {:ok, ...}} instead of
  direct tuples. Unwrap in update_user_email and update_user_and_delete_all_tokens.
- rover_planning_test.exs: switch to async: false + Oban.Testing manual mode so
  RoverPathProfileWorker jobs don't deadlock on sandbox connections inside
  Repo.transaction. Introduced create_and_complete_mission!/2 and run_backfill!/1
  helpers that separate transaction lifecycle from worker execution.
This commit is contained in:
Graham McInitre 2026-07-21 10:24:06 -05:00
parent d30b6d3022
commit 255c99cb36
2 changed files with 86 additions and 45 deletions

View file

@ -231,17 +231,20 @@ defmodule Microwaveprop.Accounts do
def update_user_email(user, token) do def update_user_email(user, token) do
context = "change:#{user.email}" context = "change:#{user.email}"
Repo.transaction(fn -> {:ok, result} =
with {:ok, query} <- UserToken.verify_change_email_token_query(token, context), Repo.transaction(fn ->
%UserToken{sent_to: email} <- Repo.one(query), with {:ok, query} <- UserToken.verify_change_email_token_query(token, context),
{:ok, user} <- Repo.update(User.email_changeset(user, %{email: email})), %UserToken{sent_to: email} <- Repo.one(query),
{_count, _result} <- {:ok, user} <- Repo.update(User.email_changeset(user, %{email: email})),
Repo.delete_all(from(UserToken, where: [user_id: ^user.id, context: ^context])) do {_count, _result} <-
{:ok, user} Repo.delete_all(from(UserToken, where: [user_id: ^user.id, context: ^context])) do
else {:ok, user}
_ -> {:error, :transaction_aborted} else
end _ -> {:error, :transaction_aborted}
end) end
end)
result
end end
@doc """ @doc """
@ -525,12 +528,14 @@ defmodule Microwaveprop.Accounts do
## Token helper ## Token helper
defp update_user_and_delete_all_tokens(changeset) do defp update_user_and_delete_all_tokens(changeset) do
Repo.transaction(fn -> {:ok, result} =
with {:ok, user} <- Repo.update(changeset) do Repo.transaction(fn ->
Repo.delete_all(from(t in UserToken, where: t.user_id == ^user.id)) with {:ok, user} <- Repo.update(changeset) do
Repo.delete_all(from(t in UserToken, where: t.user_id == ^user.id))
{:ok, {user, []}}
end
end)
{:ok, {user, []}} result
end
end)
end end
end end

View file

@ -1,17 +1,16 @@
defmodule Microwaveprop.RoverPlanningTest do defmodule Microwaveprop.RoverPlanningTest do
use Microwaveprop.DataCase, async: true use Microwaveprop.DataCase, async: false
use Oban.Testing, repo: Microwaveprop.Repo
alias Microwaveprop.AccountsFixtures alias Microwaveprop.AccountsFixtures
alias Microwaveprop.Repo alias Microwaveprop.Repo
alias Microwaveprop.Rover alias Microwaveprop.Rover
alias Microwaveprop.RoverPlanning alias Microwaveprop.RoverPlanning
alias Microwaveprop.RoverPlanning.Mission alias Microwaveprop.RoverPlanning.Station
alias Microwaveprop.Terrain.ElevationClient alias Microwaveprop.Terrain.ElevationClient
alias Microwaveprop.Workers.RoverPathProfileWorker
setup do setup do
# Oban runs inline in test, so RoverPathProfileWorker fires immediately
# when the context enqueues jobs. Stub elevation lookups so the worker
# can complete without hitting the real API.
Req.Test.stub(ElevationClient, fn conn -> Req.Test.stub(ElevationClient, fn conn ->
params = Plug.Conn.fetch_query_params(conn).query_params params = Plug.Conn.fetch_query_params(conn).query_params
lat_count = params["latitude"] |> String.split(",") |> length() lat_count = params["latitude"] |> String.split(",") |> length()
@ -21,6 +20,57 @@ defmodule Microwaveprop.RoverPlanningTest do
:ok :ok
end end
# Creates a mission and runs the full reconcile + compute pipeline under
# manual Oban mode so that:
# 1. The transaction inside reconcile_mission_paths/1 completes before any
# profile worker executes (avoiding a deadlock where the transaction
# holds the single sandbox connection while Task.async_stream tasks
# spawned by PathCompute.compute also need the DB).
# 2. Each RoverPathProfileWorker runs individually via perform_job, which
# executes in the test process (the sandbox owner when shared: true).
defp create_and_complete_mission!(user, attrs) do
Oban.Testing.with_testing_mode(:manual, fn ->
{:ok, mission} = RoverPlanning.create_mission(user, attrs)
mission
|> Repo.preload(stations: from(s in Station, order_by: s.position))
|> RoverPlanning.reconcile_mission_paths()
mission
|> RoverPlanning.list_paths()
|> Enum.each(fn path ->
perform_job(RoverPathProfileWorker, %{
"mission_id" => mission.id,
"rover_location_id" => path.rover_location_id,
"station_id" => path.station_id,
"band_mhz" => path.band_mhz
})
end)
mission
end)
end
# Runs backfill_paths/0 under manual Oban mode, then performs each
# enqueued RoverPathProfileWorker job for the given mission so paths
# land at :complete.
defp run_backfill!(mission) do
Oban.Testing.with_testing_mode(:manual, fn ->
assert :ok = RoverPlanning.backfill_paths()
mission
|> RoverPlanning.list_paths()
|> Enum.each(fn path ->
perform_job(RoverPathProfileWorker, %{
"mission_id" => mission.id,
"rover_location_id" => path.rover_location_id,
"station_id" => path.station_id,
"band_mhz" => path.band_mhz
})
end)
end)
end
defp create_rover_location(attrs) do defp create_rover_location(attrs) do
user = AccountsFixtures.user_fixture() user = AccountsFixtures.user_fixture()
@ -55,7 +105,7 @@ defmodule Microwaveprop.RoverPlanningTest do
_ideal_loc = create_rover_location(%{lat: 32.0, lon: -97.0, status: :good}) _ideal_loc = create_rover_location(%{lat: 32.0, lon: -97.0, status: :good})
_off_limits = create_rover_location(%{lat: 33.0, lon: -98.0, status: :bad}) _off_limits = create_rover_location(%{lat: 33.0, lon: -98.0, status: :bad})
assert {:ok, mission} = RoverPlanning.create_mission(user, valid_attrs()) mission = create_and_complete_mission!(user, valid_attrs())
assert mission.user_id == user.id assert mission.user_id == user.id
assert [station] = mission.stations assert [station] = mission.stations
assert station.grid == "EM12KP" assert station.grid == "EM12KP"
@ -63,31 +113,24 @@ defmodule Microwaveprop.RoverPlanningTest do
assert is_float(station.lon) assert is_float(station.lon)
paths = RoverPlanning.list_paths(mission) paths = RoverPlanning.list_paths(mission)
# only_known_good=true -> only the :good location pairs with the station.
# Oban runs inline in test so the worker has already completed.
assert length(paths) == 1 assert length(paths) == 1
assert hd(paths).status in [:complete, :failed] assert hd(paths).status == :complete
end end
test "completed path caches the link-budget loss components" do test "completed path caches the link-budget loss components" do
user = AccountsFixtures.user_fixture() user = AccountsFixtures.user_fixture()
_loc = create_rover_location(%{lat: 32.0, lon: -97.0, status: :good}) _loc = create_rover_location(%{lat: 32.0, lon: -97.0, status: :good})
assert {:ok, mission} = RoverPlanning.create_mission(user, valid_attrs()) mission = create_and_complete_mission!(user, valid_attrs())
[path] = RoverPlanning.list_paths(mission) [path] = RoverPlanning.list_paths(mission)
assert path.status == :complete assert path.status == :complete
# The result map MUST carry the cached loss-budget so the show
# page (and any future caller) can render link math without
# re-running the computation. Values come from the band's
# absorption coefficients x distance + free-space path loss + the
# already-cached terrain diffraction. Stored as floats.
assert is_float(path.result["free_space_loss_db"]) assert is_float(path.result["free_space_loss_db"])
assert is_float(path.result["oxygen_loss_db"]) assert is_float(path.result["oxygen_loss_db"])
assert is_float(path.result["humidity_loss_db_baseline"]) assert is_float(path.result["humidity_loss_db_baseline"])
assert is_float(path.result["total_baseline_loss_db"]) assert is_float(path.result["total_baseline_loss_db"])
# Free-space loss for a ~140 km, 10 GHz path is ~155 dB; sanity # Free-space loss for a ~140 km, 10 GHz path is ~155 dB; sanity
# check it's in the right order of magnitude (not zero, not 1000). # check it's in the right order of magnitude.
assert path.result["free_space_loss_db"] > 100 assert path.result["free_space_loss_db"] > 100
assert path.result["free_space_loss_db"] < 200 assert path.result["free_space_loss_db"] < 200
end end
@ -96,13 +139,8 @@ defmodule Microwaveprop.RoverPlanningTest do
user = AccountsFixtures.user_fixture() user = AccountsFixtures.user_fixture()
_loc = create_rover_location(%{lat: 32.0, lon: -97.0, status: :good}) _loc = create_rover_location(%{lat: 32.0, lon: -97.0, status: :good})
assert {:ok, mission} = RoverPlanning.create_mission(user, valid_attrs()) mission = create_and_complete_mission!(user, valid_attrs())
[path] = RoverPlanning.list_paths(mission) [path] = RoverPlanning.list_paths(mission)
# The worker is the gate: status only flips to :complete after the
# path-calculator pipeline (terrain + propagation lookup) has run.
# The score itself is nil in the test env (no .prop file on disk),
# but the key MUST be present so the show-page can render "---"
# without a key-existence check at every cell.
assert path.status == :complete assert path.status == :complete
assert Map.has_key?(path.result, "propagation_score") assert Map.has_key?(path.result, "propagation_score")
end end
@ -111,7 +149,7 @@ defmodule Microwaveprop.RoverPlanningTest do
user = AccountsFixtures.user_fixture() user = AccountsFixtures.user_fixture()
_loc = create_rover_location(%{lat: 32.0, lon: -97.0, status: :good}) _loc = create_rover_location(%{lat: 32.0, lon: -97.0, status: :good})
assert {:ok, mission} = RoverPlanning.create_mission(user, valid_attrs()) mission = create_and_complete_mission!(user, valid_attrs())
[path] = RoverPlanning.list_paths(mission) [path] = RoverPlanning.list_paths(mission)
assert path.status == :complete assert path.status == :complete
@ -122,10 +160,8 @@ defmodule Microwaveprop.RoverPlanningTest do
|> Ecto.Changeset.change(%{status: :failed, error: "transient", result: nil}) |> Ecto.Changeset.change(%{status: :failed, error: "transient", result: nil})
|> Repo.update() |> Repo.update()
assert :ok = RoverPlanning.backfill_paths() run_backfill!(mission)
# Oban inline -> backfill enqueues a job, the worker reruns
# immediately, and the path flips back to :complete.
[reloaded] = RoverPlanning.list_paths(mission) [reloaded] = RoverPlanning.list_paths(mission)
assert reloaded.status == :complete assert reloaded.status == :complete
assert map_size(reloaded.result) > 0 assert map_size(reloaded.result) > 0
@ -135,14 +171,14 @@ defmodule Microwaveprop.RoverPlanningTest do
user = AccountsFixtures.user_fixture() user = AccountsFixtures.user_fixture()
_loc = create_rover_location(%{lat: 32.0, lon: -97.0, status: :good}) _loc = create_rover_location(%{lat: 32.0, lon: -97.0, status: :good})
assert {:ok, mission} = RoverPlanning.create_mission(user, valid_attrs()) mission = create_and_complete_mission!(user, valid_attrs())
# Simulate a half-broken mission: paths were never persisted (e.g. # Simulate a half-broken mission: paths were never persisted (e.g.
# the original create_mission Multi succeeded but the Oban insert # the original create_mission Multi succeeded but the Oban insert
# was lost due to a crash). Backfill must repopulate the matrix. # was lost due to a crash). Backfill must repopulate the matrix.
Repo.delete_all(Ecto.Query.from(p in RoverPlanning.Path, where: p.mission_id == ^mission.id)) Repo.delete_all(Ecto.Query.from(p in RoverPlanning.Path, where: p.mission_id == ^mission.id))
assert RoverPlanning.list_paths(mission) == [] assert RoverPlanning.list_paths(mission) == []
assert :ok = RoverPlanning.backfill_paths() run_backfill!(mission)
assert [path] = RoverPlanning.list_paths(mission) assert [path] = RoverPlanning.list_paths(mission)
assert path.status == :complete assert path.status == :complete