Skip to content

Commit

Permalink
fix(Switch): Don't duplicate outside steps inside switch matches. (#133)
Browse files Browse the repository at this point in the history
We use a temporary reactor struct for composition to ensure that each switch branch can be validated.  Once that's done we copy all the steps from that reactor into the new switch step's options.  Unfortunately we were using the parent reactor, which already contained steps which wound up being duplicated into all switch branches.

Closes #132.

Thanks @mitel!
  • Loading branch information
jimsynz authored Nov 6, 2024
1 parent c0b4deb commit 9bc4a2e
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 10 deletions.
2 changes: 2 additions & 0 deletions .formatter.exs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ spark_locals_without_parens = [
default: 1,
flunk: 2,
flunk: 3,
fun: 1,
group: 1,
group: 2,
impl: 1,
input: 1,
input: 2,
level: 1,
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/elixir.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ jobs:
with:
spark-formatter: true
postgres: false
igniter-upgrade: false
6 changes: 4 additions & 2 deletions lib/reactor/dsl/switch.ex
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,10 @@ defmodule Reactor.Dsl.Switch do
alias Spark.{Dsl.Verifier, Error.DslError}

def build(switch, reactor) do
with {:ok, matches} <- build_matches(switch, reactor),
{:ok, default} <- build_default(switch, reactor) do
sub_reactor = Builder.new(reactor.id)

with {:ok, matches} <- build_matches(switch, sub_reactor),
{:ok, default} <- build_default(switch, sub_reactor) do
Builder.add_step(
reactor,
switch.name,
Expand Down
8 changes: 0 additions & 8 deletions mix.lock
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
%{
"bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"},
"castore": {:hex, :castore, "1.0.8", "dedcf20ea746694647f883590b82d9e96014057aff1d44d03ec90f36a5c0dc6e", [:mix], [], "hexpm", "0b2b66d2ee742cb1d9cb8c8be3b43c3a70ee8651f37b75a8b982e036752983f1"},
"credo": {:hex, :credo, "1.7.7", "771445037228f763f9b2afd612b6aa2fd8e28432a95dbbc60d8e03ce71ba4446", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "8bc87496c9aaacdc3f90f01b7b0582467b69b4bd2441fe8aae3109d843cc2f2e"},
"decimal": {:hex, :decimal, "2.1.1", "5611dca5d4b2c3dd497dec8f68751f1f1a54755e8ed2a966c2633cf885973ad6", [:mix], [], "hexpm", "53cfe5f497ed0e7771ae1a475575603d77425099ba5faef9394932b35020ffcc"},
"dialyxir": {:hex, :dialyxir, "1.4.3", "edd0124f358f0b9e95bfe53a9fcf806d615d8f838e2202a9f430d59566b6b53b", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "bf2cfb75cd5c5006bec30141b131663299c661a864ec7fbbc72dfa557487a986"},
Expand All @@ -10,27 +9,20 @@
"ex_check": {:hex, :ex_check, "0.16.0", "07615bef493c5b8d12d5119de3914274277299c6483989e52b0f6b8358a26b5f", [:mix], [], "hexpm", "4d809b72a18d405514dda4809257d8e665ae7cf37a7aee3be6b74a34dec310f5"},
"ex_doc": {:git, "https://github.com/elixir-lang/ex_doc.git", "453199337dc605a3eafc7a41e3909fd6ec0656e9", []},
"file_system": {:hex, :file_system, "1.0.0", "b689cc7dcee665f774de94b5a832e578bd7963c8e637ef940cd44327db7de2cd", [:mix], [], "hexpm", "6752092d66aec5a10e662aefeed8ddb9531d79db0bc145bb8c40325ca1d8536d"},
"finch": {:hex, :finch, "0.18.0", "944ac7d34d0bd2ac8998f79f7a811b21d87d911e77a786bc5810adb75632ada4", [:mix], [{:castore, "~> 0.1 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: false]}, {:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mint, "~> 1.3", [hex: :mint, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.4 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:nimble_pool, "~> 0.2.6 or ~> 1.0", [hex: :nimble_pool, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "69f5045b042e531e53edc2574f15e25e735b522c37e2ddb766e15b979e03aa65"},
"git_cli": {:hex, :git_cli, "0.3.0", "a5422f9b95c99483385b976f5d43f7e8233283a47cda13533d7c16131cb14df5", [:mix], [], "hexpm", "78cb952f4c86a41f4d3511f1d3ecb28edb268e3a7df278de2faa1bd4672eaf9b"},
"git_ops": {:hex, :git_ops, "2.6.1", "cc7799a68c26cf814d6d1a5121415b4f5bf813de200908f930b27a2f1fe9dad5", [:mix], [{:git_cli, "~> 0.2", [hex: :git_cli, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.0", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "ce62d07e41fe993ec22c35d5edb11cf333a21ddaead6f5d9868fcb607d42039e"},
"glob_ex": {:hex, :glob_ex, "0.1.10", "d819a368637495a5c1962ef34f48fe4e9a09032410b96ade5758f2cd1cc5fcde", [:mix], [], "hexpm", "c75357e57d71c85ef8ef7269b6e787dce3f0ff71e585f79a90e4d5477c532b90"},
"hpax": {:hex, :hpax, "1.0.0", "28dcf54509fe2152a3d040e4e3df5b265dcb6cb532029ecbacf4ce52caea3fd2", [:mix], [], "hexpm", "7f1314731d711e2ca5fdc7fd361296593fc2542570b3105595bb0bc6d0fad601"},
"igniter": {:hex, :igniter, "0.3.76", "ff283416402f4d1ef3f79ab57d38aac08389b3768fc81da03795ce5347f1167f", [:mix], [{:glob_ex, "~> 0.1.7", [hex: :glob_ex, repo: "hexpm", optional: false]}, {:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:rewrite, "~> 0.9", [hex: :rewrite, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.4", [hex: :sourceror, repo: "hexpm", optional: false]}, {:spitfire, ">= 0.1.3 and < 1.0.0-0", [hex: :spitfire, repo: "hexpm", optional: false]}], "hexpm", "4692f874f969dc4856167469a3415a5a57a362314f37e1cdb14431316a74e896"},
"iterex": {:hex, :iterex, "0.1.1", "90378a9561ad87da46737dceaf02e68a0b3023746216a4de34a0c509f5f505d4", [:mix], [], "hexpm", "c4f5916a6dbb03aa4c3d5c480069e13075ca6a57bd0c28d643da3891962440ad"},
"jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"},
"libgraph": {:hex, :libgraph, "0.16.0", "3936f3eca6ef826e08880230f806bfea13193e49bf153f93edcf0239d4fd1d07", [:mix], [], "hexpm", "41ca92240e8a4138c30a7e06466acc709b0cbb795c643e9e17174a178982d6bf"},
"makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"},
"makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"},
"makeup_erlang": {:hex, :makeup_erlang, "0.1.5", "e0ff5a7c708dda34311f7522a8758e23bfcd7d8d8068dc312b5eb41c6fd76eba", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "94d2e986428585a21516d7d7149781480013c56e30c6a233534bedf38867a59a"},
"mime": {:hex, :mime, "2.0.6", "8f18486773d9b15f95f4f4f1e39b710045fa1de891fada4516559967276e4dc2", [:mix], [], "hexpm", "c9945363a6b26d747389aac3643f8e0e09d30499a138ad64fe8fd1d13d9b153e"},
"mimic": {:hex, :mimic, "1.8.2", "f4cf6ad13a305c5ee1a6c304ee36fc7afb3ad748e2af8cd5fbf122f44a283534", [:mix], [], "hexpm", "abc982d5fdcc4cb5292980cb698cd47c0c5d9541b401e540fb695b69f1d46485"},
"mint": {:hex, :mint, "1.6.2", "af6d97a4051eee4f05b5500671d47c3a67dac7386045d87a904126fd4bbcea2e", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:hpax, "~> 0.1.1 or ~> 0.2.0 or ~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}], "hexpm", "5ee441dffc1892f1ae59127f74afe8fd82fda6587794278d924e4d90ea3d63f9"},
"mix_audit": {:hex, :mix_audit, "2.1.3", "c70983d5cab5dca923f9a6efe559abfb4ec3f8e87762f02bab00fa4106d17eda", [:make, :mix], [{:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}, {:yaml_elixir, "~> 2.9", [hex: :yaml_elixir, repo: "hexpm", optional: false]}], "hexpm", "8c3987100b23099aea2f2df0af4d296701efd031affb08d0746b2be9e35988ec"},
"mix_test_watch": {:hex, :mix_test_watch, "1.2.0", "1f9acd9e1104f62f280e30fc2243ae5e6d8ddc2f7f4dc9bceb454b9a41c82b42", [:mix], [{:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}], "hexpm", "278dc955c20b3fb9a3168b5c2493c2e5cffad133548d307e0a50c7f2cfbf34f6"},
"nimble_options": {:hex, :nimble_options, "1.1.1", "e3a492d54d85fc3fd7c5baf411d9d2852922f66e69476317787a7b2bb000a61b", [:mix], [], "hexpm", "821b2470ca9442c4b6984882fe9bb0389371b8ddec4d45a9504f00a66f650b44"},
"nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},
"nimble_pool": {:hex, :nimble_pool, "1.1.0", "bf9c29fbdcba3564a8b800d1eeb5a3c58f36e1e11d7b7fb2e084a643f645f06b", [:mix], [], "hexpm", "af2e4e6b34197db81f7aad230c1118eac993acc0dae6bc83bac0126d4ae0813a"},
"req": {:hex, :req, "0.5.2", "70b4976e5fbefe84e5a57fd3eea49d4e9aa0ac015301275490eafeaec380f97f", [:mix], [{:brotli, "~> 0.3.1", [hex: :brotli, repo: "hexpm", optional: true]}, {:ezstd, "~> 1.0", [hex: :ezstd, repo: "hexpm", optional: true]}, {:finch, "~> 0.17", [hex: :finch, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mime, "~> 2.0.6 or ~> 2.1", [hex: :mime, repo: "hexpm", optional: false]}, {:nimble_csv, "~> 1.0", [hex: :nimble_csv, repo: "hexpm", optional: true]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "0c63539ab4c2d6ced6114d2684276cef18ac185ee00674ee9af4b1febba1f986"},
"rewrite": {:hex, :rewrite, "0.10.5", "6afadeae0b9d843b27ac6225e88e165884875e0aed333ef4ad3bf36f9c101bed", [:mix], [{:glob_ex, "~> 0.1", [hex: :glob_ex, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.0", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "51cc347a4269ad3a1e7a2c4122dbac9198302b082f5615964358b4635ebf3d4f"},
"sobelow": {:hex, :sobelow, "0.13.0", "218afe9075904793f5c64b8837cc356e493d88fddde126a463839351870b8d1e", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "cd6e9026b85fc35d7529da14f95e85a078d9dd1907a9097b3ba6ac7ebbe34a0d"},
"sourceror": {:hex, :sourceror, "1.6.0", "9907884e1449a4bd7dbaabe95088ed4d9a09c3c791fb0103964e6316bc9448a7", [:mix], [], "hexpm", "e90aef8c82dacf32c89c8ef83d1416fc343cd3e5556773eeffd2c1e3f991f699"},
Expand Down
89 changes: 89 additions & 0 deletions test/reactor/bugs/switch_preceding_steps_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
defmodule Reactor.Bugs.SwitchPrecedingStepsTest do
@moduledoc false
use ExUnit.Case, async: false
import ExUnit.CaptureLog

defmodule MooStep do
@moduledoc false
use Reactor.Step
require Logger

def run(_args, _context, _opts) do
Logger.warning("MOO")
{:ok, :moo}
end
end

defmodule BooStep do
@moduledoc false
use Reactor.Step
require Logger

def run(args, _context, _opts) do
Logger.warning("BOO")
{:ok, !args.value}
end
end

defmodule TruthyStep do
@moduledoc false
use Reactor.Step
require Logger

def run(_args, _context, _opts) do
Logger.warning("TRUTHY")
{:ok, :truthy}
end
end

defmodule FalsyStep do
@moduledoc false
use Reactor.Step
require Logger

def run(_args, _context, _opts) do
Logger.warning("FALSY")
{:ok, :falsy}
end
end

defmodule BugReactor do
@moduledoc false
use Reactor

input :value

step :moo, MooStep

step :boo, BooStep do
argument :value, input(:value)
end

switch :is_truthy? do
on result(:boo)

matches? &(&1 in [nil, false]) do
step :falsy, FalsyStep
end

default do
step :truthy, TruthyStep
end
end
end

test "steps preceding a switch only get executed once" do
logs =
[level: :warning, format: "$message\n", colors: [enabled: false]]
|> capture_log(fn ->
Reactor.run(BugReactor, %{value: true})
end)
|> String.split(~r/\n+/)
|> Enum.reject(&(&1 == ""))
|> Enum.frequencies()

assert logs["MOO"] == 1
assert logs["BOO"] == 1
assert logs["FALSY"] == 1
end
end

0 comments on commit 9bc4a2e

Please sign in to comment.