mirror of
https://gh.wpcy.net/https://github.com/discourse/discourse.git
synced 2026-05-27 23:52:59 +08:00
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.
314 lines
7.7 KiB
Ruby
Vendored
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
|