Skip to content

Commit 6f7c937

Browse files
authored
fix: implement nested_steps/1 for Step.Switch to fix dependency resolution (#288)
Steps inside a `switch` block couldn't reference external steps via `result(:step_name)` when running with async execution. This was because `Step.Switch` didn't implement the `nested_steps/1` callback, so the planner couldn't see the nested steps' dependencies and create proper edges in the dependency graph. Without proper dependency edges, the Switch step could run before its nested steps' dependencies completed, causing a "step cannot be found" error when the emitted steps were planned at runtime. Closes #273
1 parent 6f160ca commit 6f7c937

File tree

3 files changed

+123
-0
lines changed

3 files changed

+123
-0
lines changed

lib/reactor/step/switch.ex

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ defmodule Reactor.Step.Switch do
7070
@type on_option :: {:on, atom}
7171

7272
@doc false
73+
@impl true
7374
@spec run(Reactor.inputs(), Reactor.context(), options) :: {:ok, any} | {:error, any}
7475
def run(arguments, _context, options) do
7576
allow_async? = Keyword.get(options, :allow_async?, true)
@@ -90,6 +91,19 @@ defmodule Reactor.Step.Switch do
9091
def to_mermaid(step, options),
9192
do: __MODULE__.Mermaid.to_mermaid(step, options)
9293

94+
@doc false
95+
@impl true
96+
def nested_steps(options) do
97+
matches = Keyword.get(options, :matches, [])
98+
default = Keyword.get(options, :default, [])
99+
100+
match_steps =
101+
matches
102+
|> Enum.flat_map(fn {_predicate, steps} -> steps end)
103+
104+
match_steps ++ default
105+
end
106+
93107
defp find_match(matches, value) do
94108
Enum.reduce_while(matches, :no_match, fn {predicate, steps}, :no_match ->
95109
if predicate.(value) do

test/reactor/dsl/switch_test.exs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,4 +123,76 @@ defmodule Reactor.Dsl.SwitchTest do
123123
end) =~ ~r/amscray/
124124
end
125125
end
126+
127+
describe "issue #273 - nested steps can refer to external step results with async execution" do
128+
defmodule SwitchExternalResultAsyncReactor do
129+
@moduledoc false
130+
use Reactor
131+
132+
input :params
133+
134+
step :create_personal_organization do
135+
run fn _, _ ->
136+
Process.sleep(10)
137+
{:ok, %{org_id: "org_123"}}
138+
end
139+
end
140+
141+
step :create_session_token do
142+
run fn _, _ ->
143+
{:ok, "token_abc"}
144+
end
145+
end
146+
147+
switch :populate_scope do
148+
on input(:params)
149+
150+
matches? &(Map.get(&1, "invite_token") != nil) do
151+
step :populate_scope do
152+
argument :user_token, result(:create_session_token)
153+
wait_for :create_personal_organization
154+
155+
run fn %{user_token: token}, _ ->
156+
{:ok, %{token: token, source: :invite}}
157+
end
158+
end
159+
end
160+
161+
default do
162+
step :populate_scope do
163+
argument :user_token, result(:create_session_token)
164+
argument :scope, result(:create_personal_organization)
165+
166+
run fn %{scope: scope, user_token: token}, _ ->
167+
{:ok, %{token: token, org_id: scope.org_id, source: :default}}
168+
end
169+
end
170+
end
171+
end
172+
173+
return :populate_scope
174+
end
175+
176+
test "when switch nested steps depend on external results, they are resolved correctly with async execution" do
177+
assert {:ok, result} =
178+
Reactor.run(SwitchExternalResultAsyncReactor, %{params: %{}}, %{}, async?: true)
179+
180+
assert result.source == :default
181+
assert result.org_id == "org_123"
182+
assert result.token == "token_abc"
183+
end
184+
185+
test "when switch nested steps depend on external results via wait_for, they are resolved correctly" do
186+
assert {:ok, result} =
187+
Reactor.run(
188+
SwitchExternalResultAsyncReactor,
189+
%{params: %{"invite_token" => "inv_123"}},
190+
%{},
191+
async?: true
192+
)
193+
194+
assert result.source == :invite
195+
assert result.token == "token_abc"
196+
end
197+
end
126198
end

test/reactor/step/switch_test.exs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,4 +117,41 @@ defmodule Reactor.Step.SwitchTest do
117117
assert error =~ ~r/no default branch/i
118118
end
119119
end
120+
121+
describe "nested_steps/1" do
122+
test "returns all steps from matches and default", %{matches: matches, default: default} do
123+
options = [matches: matches, default: default, on: :value]
124+
nested = Switch.nested_steps(options)
125+
126+
step_names = Enum.map(nested, & &1.name)
127+
assert :is_nil in step_names
128+
assert :is_false in step_names
129+
assert :is_other in step_names
130+
assert length(nested) == 3
131+
end
132+
133+
test "returns empty list when no matches or default", %{} do
134+
assert [] == Switch.nested_steps(on: :value)
135+
end
136+
137+
test "returns only match steps when no default", %{matches: matches} do
138+
options = [matches: matches, on: :value]
139+
nested = Switch.nested_steps(options)
140+
141+
step_names = Enum.map(nested, & &1.name)
142+
assert :is_nil in step_names
143+
assert :is_false in step_names
144+
refute :is_other in step_names
145+
assert length(nested) == 2
146+
end
147+
148+
test "returns only default steps when no matches", %{default: default} do
149+
options = [default: default, on: :value]
150+
nested = Switch.nested_steps(options)
151+
152+
step_names = Enum.map(nested, & &1.name)
153+
assert :is_other in step_names
154+
assert length(nested) == 1
155+
end
156+
end
120157
end

0 commit comments

Comments
 (0)