From 5b78fbcb0583568392762b15a262b3106cfb5185 Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Thu, 8 Sep 2022 11:47:28 -0400 Subject: [PATCH] Allow re-entrant actions to be processed (#1352) * wip * wip * clean up * wip * wip * wip * wip * fix Co-authored-by: Brandon Williams --- .../Tests/LoginCoreTests/LoginCoreTests.swift | 1 + Sources/ComposableArchitecture/Store.swift | 15 +++- .../ComposableArchitecture/TestStore.swift | 14 ++- .../CompatibilityTests.swift | 85 +++++++++++++++++++ 4 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 Tests/ComposableArchitectureTests/CompatibilityTests.swift diff --git a/Examples/TicTacToe/tic-tac-toe/Tests/LoginCoreTests/LoginCoreTests.swift b/Examples/TicTacToe/tic-tac-toe/Tests/LoginCoreTests/LoginCoreTests.swift index 3c93630eb4e2..b7a5e0817e3d 100644 --- a/Examples/TicTacToe/tic-tac-toe/Tests/LoginCoreTests/LoginCoreTests.swift +++ b/Examples/TicTacToe/tic-tac-toe/Tests/LoginCoreTests/LoginCoreTests.swift @@ -105,5 +105,6 @@ final class LoginCoreTests: XCTestCase { await store.send(.twoFactorDismissed) { $0.twoFactor = nil } + await store.finish() } } diff --git a/Sources/ComposableArchitecture/Store.swift b/Sources/ComposableArchitecture/Store.swift index 51f3a5b47d17..468b5d8ab458 100644 --- a/Sources/ComposableArchitecture/Store.swift +++ b/Sources/ComposableArchitecture/Store.swift @@ -327,15 +327,24 @@ public final class Store { self.isSending = true var currentState = self.state.value + let tasks = Box<[Task]>(wrappedValue: []) defer { + withExtendedLifetime(self.bufferedActions) { + self.bufferedActions.removeAll() + } self.isSending = false self.state.value = currentState + // NB: Handle any re-entrant actions + if !self.bufferedActions.isEmpty { + if let task = self.send( + self.bufferedActions.removeLast(), originatingFrom: originatingAction + ) { + tasks.wrappedValue.append(task) + } + } } - let tasks = Box<[Task]>(wrappedValue: []) - var index = self.bufferedActions.startIndex - defer { self.bufferedActions = [] } while index < self.bufferedActions.endIndex { defer { index += 1 } let action = self.bufferedActions[index] diff --git a/Sources/ComposableArchitecture/TestStore.swift b/Sources/ComposableArchitecture/TestStore.swift index f25ea8814d3d..e2a9a0c2469c 100644 --- a/Sources/ComposableArchitecture/TestStore.swift +++ b/Sources/ComposableArchitecture/TestStore.swift @@ -238,7 +238,19 @@ self.store = Store( initialState: initialState, - reducer: Reducer { [unowned self] state, action, _ in + reducer: Reducer { [weak self] state, action, _ in + guard let self = self + else { + XCTFail( + """ + An effect sent an action to the store after the store was deallocated. + """, + file: file, + line: line + ) + return .none + } + let effects: Effect switch action.origin { case let .send(scopedAction): diff --git a/Tests/ComposableArchitectureTests/CompatibilityTests.swift b/Tests/ComposableArchitectureTests/CompatibilityTests.swift new file mode 100644 index 000000000000..f56ad1bd01a3 --- /dev/null +++ b/Tests/ComposableArchitectureTests/CompatibilityTests.swift @@ -0,0 +1,85 @@ +import Combine +import CombineSchedulers +import ComposableArchitecture +import XCTest + +@MainActor +final class CompatibilityTests: XCTestCase { + func testCaseStudy_ReentrantEffect() { + let cancelID = UUID() + + struct State: Equatable {} + enum Action: Equatable { + case start + case kickOffAction + case actionSender(OnDeinit) + case stop + + var description: String { + switch self { + case .start: + return "start" + case .kickOffAction: + return "kickOffAction" + case .actionSender: + return "actionSender" + case .stop: + return "stop" + } + } + } + let passThroughSubject = PassthroughSubject() + + var handledActions: [String] = [] + + let reducer = Reducer { state, action, env in + handledActions.append(action.description) + + switch action { + case .start: + return passThroughSubject + .eraseToEffect() + .cancellable(id: cancelID) + + case .kickOffAction: + return Effect(value: .actionSender(OnDeinit { passThroughSubject.send(.stop) })) + + case .actionSender: + return .none + + case .stop: + return .cancel(id: cancelID) + } + } + + let store = Store( + initialState: .init(), + reducer: reducer, + environment: () + ) + + let viewStore = ViewStore(store) + + viewStore.send(.start) + viewStore.send(.kickOffAction) + + XCTAssertNoDifference( + handledActions, + [ + "start", + "kickOffAction", + "actionSender", + "stop", + ] + ) + } +} + +private final class OnDeinit: Equatable { + private let onDeinit: () -> () + init(onDeinit: @escaping () -> ()) { + self.onDeinit = onDeinit + } + deinit { self.onDeinit() } + static func == (lhs: OnDeinit, rhs: OnDeinit) -> Bool { true } +}