discourse/spec/lib/service/base/each_step_spec.rb
Loïc Guitaut f92a035936
DEV: Fix isolation bugs in service framework's each step (#40311)
Three independent issues in `each`'s isolation, each surfacing as soon
as the step is used outside the simplest shapes:

1. Nested `each` blocks crashed inside `with_isolation`'s `ensure`. The
isolation kept its snapshot in a single slot that the inner call would
overwrite then null, so the outer's cleanup ran against `nil`. The
snapshot now lives on a stack, making isolation re-entrant.

2. ActiveRecord models in the context lost their primary key inside an
`each` block. The snapshot used `deep_dup`, which recurses into AR
objects via `dup` and AR's `dup` returns an unpersisted copy with `id ==
nil`. Basic shapes like `model :user; each :things do ... end` silently
swapped the real user for a useless ghost. The deep copy was originally
an attempt at mutation isolation for collections, but that contract was
never documented, its supporting spec was removed before the original PR
merged, and the rest of the framework already lets steps mutate what
they receive. A shallow dup matches the documented "variables set inside
the loop don't leak" guarantee and stops corrupting models.

3. Non-persisted state leaked between iterations. The whole loop ran
inside one isolation, so iteration N could read scratch values iteration
N-1 had left around. Each iteration now gets its own isolation. Steps
inside the same iteration still share state freely (step 2 can act on
what step 1 produced); only cross-iteration carry-over now requires
`persist:`, which makes that dependency visible at the `each`
declaration.
2026-05-27 09:47:34 +02:00

314 lines
7.7 KiB
Ruby
Vendored

# frozen_string_literal: true
RSpec.describe Service::Base::EachStep do
describe "#call" do
subject(:result) { service.call(dependencies) }
let(:dependencies) { {} }
context "when iterating over a collection" do
let(:service) do
Class.new do
include Service::Base
model :users
each :users, persist: { processed: -> { [] } } do
step :process_user
end
step :finalize
private
def fetch_users
context[:input_users]
end
def process_user(user:, index:, processed:)
processed << { user:, index: }
end
def finalize
context[:finalized] = true
end
end
end
let(:dependencies) { { input_users: %i[alice bob charlie] } }
it { is_expected.to run_successfully }
it "provides each item under the singularized name and its index" do
expect(result[:processed]).to contain_exactly(
{ user: :alice, index: 0 },
{ user: :bob, index: 1 },
{ user: :charlie, index: 2 },
)
end
it "continues to subsequent steps after iteration" do
expect(result[:finalized]).to be true
end
end
context "when using the as: option" do
let(:service) do
Class.new do
include Service::Base
model :users
each :users, as: :member, persist: { processed: -> { [] } } do
step :process_member
end
private
def fetch_users
context[:input_users]
end
def process_member(member:, processed:)
processed << member
end
end
end
let(:dependencies) { { input_users: %i[alice bob] } }
it "provides items under the custom name" do
expect(result[:processed]).to contain_exactly(:alice, :bob)
end
end
context "with variable isolation" do
let(:service) do
Class.new do
include Service::Base
step :setup
each :users do
model :profile
step :track
end
step :check_after
private
def setup
context[:users] = %i[alice bob]
context[:existing_value] = "original"
end
def fetch_profile(user:)
"#{user}_profile"
end
def track(user:)
context[:set_inside_loop] = user
end
def check_after
context[:profile_after] = context[:profile]
context[:user_after] = context[:user]
context[:index_after] = context[:index]
context[:set_inside_after] = context[:set_inside_loop]
context[:existing_after] = context[:existing_value]
end
end
end
it "discards non-persisted variables set inside the loop" do
expect(result).to have_attributes(profile_after: nil, set_inside_after: nil)
end
it "keeps the last item and index after the loop" do
expect(result).to have_attributes(user_after: :bob, index_after: 1)
end
it "restores pre-existing values after the loop" do
expect(result).to have_attributes(existing_after: "original")
end
end
context "with isolation between iterations" do
let(:service) do
Class.new do
include Service::Base
step :setup
each :users, persist: { observed_values: -> { [] } } do
step :track
end
private
def setup
context[:users] = %i[alice bob]
end
def track(user:, observed_values:)
observed_values << context[:transient]
context[:transient] = user
end
end
end
it "does not leak non-persisted variables between iterations" do
expect(result).to have_attributes(observed_values: [nil, nil])
end
end
context "when the outer context contains an ActiveRecord model" do
fab!(:user)
let(:service) do
Class.new do
include Service::Base
model :user
step :setup_usernames
each :usernames,
persist: {
seen_user_ids: -> { [] },
user_persisted_states: -> { [] },
} do
step :track_user
end
private
def fetch_user(user_id:)
User.find(user_id)
end
def setup_usernames
context[:usernames] = %i[alice bob]
end
def track_user(user:, seen_user_ids:, user_persisted_states:)
seen_user_ids << user.id
user_persisted_states << user.persisted?
end
end
end
let(:dependencies) { { user_id: user.id } }
it "keeps the original model available inside each iterations" do
expect(result).to have_attributes(
seen_user_ids: [user.id, user.id],
user_persisted_states: [true, true],
)
end
end
context "with persist option" do
context "with a lambda initializer" do
let(:service) do
Class.new do
include Service::Base
model :users
each :users, persist: { results: -> { { created: [], failed: [] } } } do
step :process_user
end
private
def fetch_users
context[:input_users]
end
def process_user(user:, results:)
user == :invalid ? results[:failed] << user : results[:created] << user
end
end
end
let(:dependencies) { { input_users: %i[alice invalid bob] } }
it "initializes and accumulates across iterations" do
expect(result[:results]).to eq(created: %i[alice bob], failed: [:invalid])
end
end
context "with a method symbol initializer" do
let(:service) do
Class.new do
include Service::Base
model :users
each :users, persist: { results: :initial_results } do
step :process_user
end
private
def fetch_users
context[:input_users]
end
def initial_results
{ processed: [], count: 0 }
end
def process_user(user:, results:)
results[:processed] << user
results[:count] += 1
end
end
end
let(:dependencies) { { input_users: %i[alice bob] } }
it "initializes by calling the method" do
expect(result[:results]).to eq(processed: %i[alice bob], count: 2)
end
end
end
context "when nesting each steps" do
let(:service) do
Class.new do
include Service::Base
model :groups
each :groups, persist: { processed: -> { [] } } do
each :group, as: :item, persist: { processed: -> { context[:processed] } } do
step :collect_item
end
end
private
def fetch_groups
context[:input_groups]
end
def collect_item(group:, item:, processed:)
processed << [group, item]
end
end
end
let(:dependencies) { { input_groups: [%i[alice bob], %i[charlie dave]] } }
it { is_expected.to run_successfully }
it "accumulates results across both levels of iteration" do
expect(result[:processed]).to eq(
[
[%i[alice bob], :alice],
[%i[alice bob], :bob],
[%i[charlie dave], :charlie],
[%i[charlie dave], :dave],
],
)
end
end
end
end