From aad29dc1a0cd375e1316ae61f8a86001b8d32047 Mon Sep 17 00:00:00 2001 From: Red Davis Date: Fri, 10 Jun 2022 17:14:43 +0100 Subject: [PATCH] Refactor effect management (#32) --- RedUx.xcodeproj/project.pbxproj | 4 ++ RedUx/Source/Effect.swift | 3 + RedUx/Source/EffectManager.swift | 29 +++++--- RedUx/Source/Reducer.swift | 18 ++--- RedUx/Source/Store.swift | 9 ++- RedUxTests/Tests/EffectManagerTests.swift | 81 +++++++++++++++++++++++ RedUxTests/Tests/StoreTests.swift | 4 +- RedUxTests/Tests/ViewModelTests.swift | 2 +- 8 files changed, 129 insertions(+), 21 deletions(-) create mode 100644 RedUxTests/Tests/EffectManagerTests.swift diff --git a/RedUx.xcodeproj/project.pbxproj b/RedUx.xcodeproj/project.pbxproj index 49cc54c..f5020f1 100644 --- a/RedUx.xcodeproj/project.pbxproj +++ b/RedUx.xcodeproj/project.pbxproj @@ -38,6 +38,7 @@ A4B9E5A727A269590000ED07 /* RedUxable.swift in Sources */ = {isa = PBXBuildFile; fileRef = A4B9E5A627A269590000ED07 /* RedUxable.swift */; }; A4C3619A276A086A00511525 /* AppStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = A4C36199276A086A00511525 /* AppStore.swift */; }; A4D2CFA026C0FC34008D25DE /* View+Extension.swift in Sources */ = {isa = PBXBuildFile; fileRef = A4D2CF9F26C0FC34008D25DE /* View+Extension.swift */; }; + A4EA10AC28536E720037A053 /* EffectManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A4EA10AB28536E720037A053 /* EffectManagerTests.swift */; }; A4EE8D352763B4ED00BE7F55 /* Models.swift in Sources */ = {isa = PBXBuildFile; fileRef = A4EE8D342763B4ED00BE7F55 /* Models.swift */; }; /* End PBXBuildFile section */ @@ -113,6 +114,7 @@ A4C36199276A086A00511525 /* AppStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppStore.swift; sourceTree = ""; }; A4D2CF9F26C0FC34008D25DE /* View+Extension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "View+Extension.swift"; sourceTree = ""; }; A4D2CFA526C16A69008D25DE /* Package.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Package.swift; sourceTree = ""; }; + A4EA10AB28536E720037A053 /* EffectManagerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EffectManagerTests.swift; sourceTree = ""; }; A4EE8D342763B4ED00BE7F55 /* Models.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Models.swift; sourceTree = ""; }; /* End PBXFileReference section */ @@ -310,6 +312,7 @@ children = ( A4773C482827C44600828A14 /* ActionStatusTests.swift */, A4B1E28128313E3100B6FCB8 /* EffectTests.swift */, + A4EA10AB28536E720037A053 /* EffectManagerTests.swift */, A425FB6F275F9769002AFD72 /* ReducerTests.swift */, A47E64C4267A10F3005E265C /* StoreTests.swift */, A40EE2A427A044FE00663E6C /* ViewModelTests.swift */, @@ -536,6 +539,7 @@ A47E64C5267A10F3005E265C /* StoreTests.swift in Sources */, A4773C4B2827C86A00828A14 /* ValueStatusTests.swift in Sources */, A47BE5FB27BC467A0011ECE6 /* Assertions.swift in Sources */, + A4EA10AC28536E720037A053 /* EffectManagerTests.swift in Sources */, A4EE8D352763B4ED00BE7F55 /* Models.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; diff --git a/RedUx/Source/Effect.swift b/RedUx/Source/Effect.swift index 7775532..204479a 100644 --- a/RedUx/Source/Effect.swift +++ b/RedUx/Source/Effect.swift @@ -5,6 +5,9 @@ public struct Effect { /// The ID of the effect. let id: String + /// The unique ID of the effect. + let uuid: String = UUID().uuidString + /// Indicate whether this effect is a cancellation effect. let isCancellation: Bool diff --git a/RedUx/Source/EffectManager.swift b/RedUx/Source/EffectManager.swift index 62e23ba..fa21c33 100644 --- a/RedUx/Source/EffectManager.swift +++ b/RedUx/Source/EffectManager.swift @@ -1,18 +1,31 @@ import Foundation actor EffectManager { - var tasks: [String: Task] = [:] + var tasks: [String: TaskWrapper] = [:] // MARK: Task management - func addTask(_ task: Task, with id: String) { - self.removeTask(id) // Cancel previous task with the same ID. - self.tasks[id] = task + func addTask(_ task: Task, id: String, uuid: String) { + self.cancelTask(id) // Cancel previous task with the same ID. + self.tasks[id] = .init(id: uuid, task: task) } - func removeTask(_ id: String) { - guard let task = self.tasks[id] else { return } - task.cancel() - self.tasks[id] = nil + func cancelTask(_ id: String) { + guard let wrapper = self.tasks[id] else { return } + wrapper.task.cancel() + self.tasks.removeValue(forKey: id) } + + func taskComplete(id: String, uuid: String) { + guard let wrapper = self.tasks[id], + wrapper.id == uuid else { return } + self.tasks.removeValue(forKey: id) + } +} + +// MARK: Task wrapper + +struct TaskWrapper { + let id: String + let task: Task } diff --git a/RedUx/Source/Reducer.swift b/RedUx/Source/Reducer.swift index b8085db..24719f7 100644 --- a/RedUx/Source/Reducer.swift +++ b/RedUx/Source/Reducer.swift @@ -71,15 +71,16 @@ extension Reducer { ) -> Reducer { .init { appState, appEvent, appEnvironment in guard let event = getEvent(appEvent) else { return .none } - let eventStream = self.reduce( + guard let effect = self.reduce( &appState[keyPath: state], event, environment(appEnvironment) - ) + ) else { return .none } - return eventStream?.compactMap { event in - setAppEvent(event) - }.eraseToEffect() + return effect.compactMap { + setAppEvent($0) + } + .eraseToEffect(id: effect.id) } } @@ -106,16 +107,17 @@ extension Reducer { guard let event = getEvent(appEvent) else { return .none } var state = getState(appState) - let eventStream = self.reduce( + let effect = self.reduce( &state, event, environment(appEnvironment) ) setAppState(&appState, state) - return eventStream?.compactMap { event in + guard let effect = effect else { return .none } + return effect.compactMap { event in setAppEvent(event) - }.eraseToEffect() + }.eraseToEffect(id: effect.id) } } } diff --git a/RedUx/Source/Store.swift b/RedUx/Source/Store.swift index 631058b..4ab76fc 100644 --- a/RedUx/Source/Store.swift +++ b/RedUx/Source/Store.swift @@ -104,17 +104,20 @@ public final class Store { Task(priority: .high) { guard !effect.isCancellation else { - await self.effectManager.removeTask(effect.id) + await self.effectManager.cancelTask(effect.id) return } let task = effect.sink( receiveValue: { [weak self] in self?.send($0) }, receiveCompletion: { [weak self] _ in - await self?.effectManager.removeTask(effect.id) + await self?.effectManager.taskComplete( + id: effect.id, + uuid: effect.uuid + ) } ) - await self.effectManager.addTask(task, with: effect.id) + await self.effectManager.addTask(task, id: effect.id, uuid: effect.uuid) } } while !self.eventBacklog.isEmpty } diff --git a/RedUxTests/Tests/EffectManagerTests.swift b/RedUxTests/Tests/EffectManagerTests.swift new file mode 100644 index 0000000..a80234a --- /dev/null +++ b/RedUxTests/Tests/EffectManagerTests.swift @@ -0,0 +1,81 @@ +import Asynchrone +import XCTest +@testable import RedUx + +final class EffectManagerTests: XCTestCase { + private var manager: EffectManager! + + // MARK: Setup + + override func setUpWithError() throws { + self.manager = .init() + } + + func testAddingTask() async { + let task = Task {} + let id = "1" + let internalID = "2" + await self.manager.addTask(task, id: id, uuid: internalID) + + await XCTAsyncAssertEqual( + { 1 }, + { await self.manager.tasks.count } + ) + } + + func testCancellingTask() async { + let id = "1" + let internalID = "2" + let completionExpectation = self.expectation(description: "Completion called") + + let task = Empty(completeImmediately: false) + .sink( + receiveValue: {}, + receiveCompletion: { result in + switch result { + case .failure(let error) where error is CancellationError:() + case .failure(let error): + XCTFail("Invalid failure error. CancellationError expected: \(error)") + case .finished: + XCTFail(".finished is an invalid result") + } + completionExpectation.fulfill() + } + ) + + await self.manager.addTask(task, id: id, uuid: internalID) + await self.manager.cancelTask(id) + + await self.waitForExpectations(timeout: 5.0, handler: nil) + await XCTAsyncAssertEqual( + { 0 }, + { await self.manager.tasks.count } + ) + } + + func testNotifyingOfTaskCompletion() async { + let task = Task {} + let id = "1" + let internalID = "2" + await self.manager.addTask(task, id: id, uuid: internalID) + await self.manager.taskComplete(id: id, uuid: internalID) + + await XCTAsyncAssertEqual( + { 0 }, + { await self.manager.tasks.count } + ) + } + + func testNotifyingOfTaskCompletionWithDifferentInternalID() async { + let task = Task {} + let id = "1" + let internalID = "2" + await self.manager.addTask(task, id: id, uuid: internalID) + await self.manager.taskComplete(id: id, uuid: "different") + + await XCTAsyncAssertEqual( + { 1 }, + { await self.manager.tasks.count } + ) + } +} diff --git a/RedUxTests/Tests/StoreTests.swift b/RedUxTests/Tests/StoreTests.swift index 8c770d4..1d51d87 100644 --- a/RedUxTests/Tests/StoreTests.swift +++ b/RedUxTests/Tests/StoreTests.swift @@ -145,6 +145,7 @@ final class StoreTests: XCTestCase { func testCancellingEffect() async { let id = "1" + let internalID = "2" let task = Empty(completeImmediately: false) .sink( receiveValue: {}, @@ -153,7 +154,8 @@ final class StoreTests: XCTestCase { await self.manager.addTask( task, - with: id + id: id, + uuid: internalID ) await XCTAsyncAssertEqual( diff --git a/RedUxTests/Tests/ViewModelTests.swift b/RedUxTests/Tests/ViewModelTests.swift index 0c7c925..b3eeb19 100644 --- a/RedUxTests/Tests/ViewModelTests.swift +++ b/RedUxTests/Tests/ViewModelTests.swift @@ -49,7 +49,7 @@ final class ViewModelTests: XCTestCase { func testStateChangesFromEventViaEffectPropagateToViewModel() async { XCTAssertNil(self.store.state.value) - await self.store.send(.setValueViaEffect(self.value)) + self.store.send(.setValueViaEffect(self.value)) await XCTAssertEventuallyEqual( self.viewModel.state,