Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate the EffectHandler protocol #210

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Deprecate the EffectHandler protocol #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 43 additions & 29 deletions MobiusCore/Source/EffectHandlers/EffectExecutor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,29 @@
import Foundation

final class EffectExecutor<Effect, Event>: Connectable {
private let handleEffect: (Effect, EffectCallback<Event>) -> Disposable
enum Operation {
case eventEmitting((Effect, EffectCallback<Event>) -> Disposable)
case eventReturning((Effect) -> Event?)
case sideEffecting((Effect) -> Void)
}

private let operation: Operation
private var output: Consumer<Event>?

private let lock = Lock()

// Keep track of each received effect's state.
// When an effect has completed, it should be removed from this dictionary.
// When disposing this effect handler, all entries must be removed.
private var handlingEffects: [Int64: EffectHandlingState<Event>] = [:]
private var ongoingEffects: [Int64: EffectHandlingState<Event>] = [:]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 this rename

private var nextID = Int64(0)

init(handleInput: @escaping (Effect, EffectCallback<Event>) -> Disposable) {
self.handleEffect = handleInput
init(operation: Operation) {
self.operation = operation
}

deinit {
dispose()
}

func connect(_ consumer: @escaping Consumer<Event>) -> Connection<Effect> {
Expand All @@ -49,7 +59,31 @@ final class EffectExecutor<Effect, Event>: Connectable {
}
}

func handle(_ effect: Effect) {
private func handle(_ effect: Effect) {
switch operation {
case .eventEmitting(let handler): handleOngoing(effect, handler: handler)
case .eventReturning(let handler): handler(effect).map { event in output?(event) }
case .sideEffecting(let handler): handler(effect)
}
}

private func dispose() {
lock.synchronized {
// Dispose any effects currently being handled. We also need to `end` their callbacks to remove the
// references we are keeping to them.
ongoingEffects.values
.forEach {
$0.disposable.dispose()
$0.callback.end()
}

// Restore the state of this `Connectable` to its pre-connected state.
ongoingEffects = [:]
output = nil
}
}

private func handleOngoing(_ effect: Effect, handler: @escaping (Effect, EffectCallback<Event>) -> Disposable) {
let id: Int64 = lock.synchronized {
nextID += 1
return nextID
Expand All @@ -63,47 +97,27 @@ final class EffectExecutor<Effect, Event>: Connectable {
onEnd: { [weak self] in self?.delete(id: id) }
)

let disposable = handleEffect(effect, callback)

let disposable = handler(effect, callback)
store(id: id, callback: callback, disposable: disposable)

// We cannot know if `callback.end()` was called before `self.store(..)`. This check ensures that if
// the callback was ended early, the reference to it will be deleted.
if callback.ended {
delete(id: id)
}
}

func dispose() {
lock.synchronized {
// Dispose any effects currently being handled. We also need to `end` their callbacks to remove the
// references we are keeping to them.
handlingEffects.values
.forEach {
$0.disposable.dispose()
$0.callback.end()
}

// Restore the state of this `Connectable` to its pre-connected state.
handlingEffects = [:]
output = nil
}
}

private func store(id: Int64, callback: EffectCallback<Event>, disposable: Disposable) {
lock.synchronized {
handlingEffects[id] = EffectHandlingState(callback: callback, disposable: disposable)
ongoingEffects[id] = EffectHandlingState(callback: callback, disposable: disposable)
}
}

private func delete(id: Int64) {
lock.synchronized {
handlingEffects[id] = nil
ongoingEffects[id] = nil
}
}

deinit {
dispose()
}
}

private struct EffectHandlingState<Event> {
Expand Down
2 changes: 2 additions & 0 deletions MobiusCore/Source/EffectHandlers/EffectHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
/// call `callback.end()`.
///
/// Note: `EffectHandler` should be used in conjunction with an `EffectRouter`.
@available(*, deprecated)
public protocol EffectHandler {
associatedtype EffectParameters
associatedtype Event
Expand All @@ -44,6 +45,7 @@ public protocol EffectHandler {
}

/// A type-erased wrapper of the `EffectHandler` protocol.
@available(*, deprecated)
public struct AnyEffectHandler<EffectParameters, Event>: EffectHandler {
private let handleClosure: (EffectParameters, EffectCallback<Event>) -> Disposable

Expand Down
16 changes: 10 additions & 6 deletions MobiusCore/Source/EffectHandlers/EffectRouter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,21 @@ public struct _PartialEffectRouter<Effect, EffectParameters, Event> {
fileprivate let path: (Effect) -> EffectParameters?
fileprivate let queue: DispatchQueue?

func routed<C: Connectable>(
_ connectable: C
) -> EffectRouter<Effect, Event> where C.Input == EffectParameters, C.Output == Event {
let route = Route(extractParameters: path, connectable: connectable, queue: queue)
return EffectRouter(routes: routes + [route])
}

/// Route to an `EffectHandler`.
///
/// - Parameter effectHandler: the `EffectHandler` for the route in question.
@available(*, deprecated, message: "prefer routing directly to the handling closure, eg: .to(myEffectHandler.handle)")
public func to<Handler: EffectHandler>(
_ effectHandler: Handler
) -> EffectRouter<Effect, Event> where Handler.EffectParameters == EffectParameters, Handler.Event == Event {
let connectable = EffectExecutor(handleInput: effectHandler.handle)
let route = Route<Effect, Event>(extractParameters: path, connectable: connectable, queue: queue)
return EffectRouter(routes: routes + [route])
return routed(EffectExecutor(operation: .eventEmitting(effectHandler.handle)))
}

/// Route to a Connectable.
Expand All @@ -95,9 +101,7 @@ public struct _PartialEffectRouter<Effect, EffectParameters, Event> {
public func to<C: Connectable>(
_ connectable: C
) -> EffectRouter<Effect, Event> where C.Input == EffectParameters, C.Output == Event {
let connectable = ThreadSafeConnectable(connectable: connectable)
let route = Route(extractParameters: path, connectable: connectable, queue: queue)
return EffectRouter(routes: routes + [route])
return routed(ThreadSafeConnectable(connectable: connectable))
}

/// Handle an the current `Effect` asynchronously on the provided `DispatchQueue`
Expand Down
16 changes: 3 additions & 13 deletions MobiusCore/Source/EffectHandlers/EffectRouterDSL.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public extension _PartialEffectRouter {
func to(
_ handle: @escaping (EffectParameters, EffectCallback<Event>) -> Disposable
) -> EffectRouter<Effect, Event> {
return to(AnyEffectHandler(handle: handle))
return routed(EffectExecutor(operation: .eventEmitting(handle)))
}

/// Route to a side-effecting closure.
Expand All @@ -39,11 +39,7 @@ public extension _PartialEffectRouter {
func to(
_ fireAndForget: @escaping (EffectParameters) -> Void
) -> EffectRouter<Effect, Event> {
return to { parameters, callback in
fireAndForget(parameters)
callback.end()
return AnonymousDisposable {}
}
return routed(EffectExecutor(operation: .sideEffecting(fireAndForget)))
}

/// Route to a closure which returns an optional event when given the parameters as input.
Expand All @@ -53,12 +49,6 @@ public extension _PartialEffectRouter {
func toEvent(
_ eventClosure: @escaping (EffectParameters) -> Event?
) -> EffectRouter<Effect, Event> {
return to { parameters, callback in
if let event = eventClosure(parameters) {
callback.send(event)
}
callback.end()
return AnonymousDisposable {}
}
return routed(EffectExecutor(operation: .eventReturning(eventClosure)))
}
}
8 changes: 5 additions & 3 deletions MobiusCore/Test/EffectHandlers/AnyEffectHandlerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import Quick
private typealias Effect = String
private typealias Event = String

@available(*, deprecated)
class AnyEffectHandlerTests: QuickSpec {
// swiftlint:disable:next function_body_length
override func spec() {
Expand Down Expand Up @@ -59,7 +60,7 @@ class AnyEffectHandlerTests: QuickSpec {

context("when initialized with wrapped effect handler") {
beforeEach {
let wrapped = TestEffectHandler()
let wrapped = WrappedEffectHandler()
effectHandler = AnyEffectHandler(handler: wrapped)
}

Expand All @@ -68,7 +69,7 @@ class AnyEffectHandlerTests: QuickSpec {

context("when initialized with doubly wrapped effect handler") {
beforeEach {
let wrapped = TestEffectHandler()
let wrapped = WrappedEffectHandler()
let inner = AnyEffectHandler(handler: wrapped)
effectHandler = AnyEffectHandler(handler: inner)
}
Expand All @@ -79,7 +80,8 @@ class AnyEffectHandlerTests: QuickSpec {
}
}

private struct TestEffectHandler: EffectHandler {
@available(*, deprecated)
private struct WrappedEffectHandler: EffectHandler {
func handle(_ effect: Effect, _ callback: EffectCallback<Event>) -> Disposable {
callback.send(effect)
return AnonymousDisposable {
Expand Down
10 changes: 5 additions & 5 deletions MobiusCore/Test/EffectHandlers/EffectHandlerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ private enum Event {
class EffectHandlerTests: QuickSpec {
override func spec() {
describe("Handling effects with EffectHandler") {
var effectHandler: AnyEffectHandler<Effect, Event>!
var effectHandler: TestEffectHandler<Effect, Event>!
var executeEffect: ((Effect) -> Void)!
var receivedEvents: [Event]!

beforeEach {
effectHandler = AnyEffectHandler(handle: handleEffect)
effectHandler = handleEffect
receivedEvents = []
let callback = EffectCallback(
onSend: { event in
Expand All @@ -44,7 +44,7 @@ class EffectHandlerTests: QuickSpec {
onEnd: {}
)
executeEffect = { effect in
_ = effectHandler.handle(effect, callback)
_ = effectHandler(effect, callback)
}
}

Expand All @@ -64,13 +64,13 @@ class EffectHandlerTests: QuickSpec {
describe("Disposing EffectHandler") {
it("calls the returned disposable when disposing") {
var disposed = false
let effectHandler = AnyEffectHandler<Effect, Event> { _, _ in
let effectHandler: TestEffectHandler<Effect, Event> = { _, _ in
AnonymousDisposable {
disposed = true
}
}
let callback = EffectCallback<Event>(onSend: { _ in }, onEnd: {})
effectHandler.handle(.effect1, callback).dispose()
effectHandler(.effect1, callback).dispose()

expect(disposed).to(beTrue())
}
Expand Down
6 changes: 3 additions & 3 deletions MobiusCore/Test/EffectHandlers/EffectRouterTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ class EffectRouterTests: QuickSpec {
receivedEvents = []
disposed1 = false
disposed2 = false
let effectHandler1 = AnyEffectHandler<Effect, Event> { _, callback in
let effectHandler1: TestEffectHandler<Effect, Event> = { _, callback in
callback.send(.eventForEffect1)
return AnonymousDisposable {
disposed1 = true
}
}
let effectHandler2 = AnyEffectHandler<Effect, Event> { _, callback in
let effectHandler2: TestEffectHandler<Effect, Event> = { _, callback in
callback.send(.eventForEffect2)
return AnonymousDisposable {
disposed2 = true
Expand Down Expand Up @@ -118,7 +118,7 @@ class EffectRouterTests: QuickSpec {
var dispose: (() -> Void)!

beforeEach {
let handler = AnyEffectHandler<Effect, Event> { _, _ in
let handler: TestEffectHandler<Effect, Event> = { _, _ in
AnonymousDisposable {}
}
let invalidRouter = EffectRouter<Effect, Event>()
Expand Down
4 changes: 2 additions & 2 deletions MobiusCore/Test/EventRouterDisposalLogicalRaceRegressionTest.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ private class EffectCollaborator {
}

extension EffectCollaborator {
func makeEffectHandler<Effect, Event>(replyEvent: Event) -> AnyEffectHandler<Effect, Event> {
return AnyEffectHandler<Effect, Event> { _, callback in
func makeEffectHandler<Effect, Event>(replyEvent: Event) -> TestEffectHandler<Effect, Event> {
return { _, callback in
let cancellationToken = self.asyncDoStuff {
callback.send(replyEvent)
callback.end()
Expand Down
2 changes: 1 addition & 1 deletion MobiusCore/Test/MobiusLoopTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ class MobiusLoopTests: QuickSpec {
beforeEach {
disposed.value = false
didReceiveEffect.value = false
let effectHandler = AnyEffectHandler<Int, Int> { _, _ in
let effectHandler: TestEffectHandler<Int, Int> = { _, _ in
didReceiveEffect.value = true
return AnonymousDisposable {
disposed.value = true
Expand Down
2 changes: 1 addition & 1 deletion MobiusCore/Test/NonReentrancyTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class NonReentrancyTests: QuickSpec {
}
}

let testEffectHandler = AnyEffectHandler<Effect, Event> {
let testEffectHandler: TestEffectHandler<Effect, Event> = {
handleEffect($0, $1)
return AnonymousDisposable {}
}
Expand Down
2 changes: 2 additions & 0 deletions MobiusCore/Test/TestingUtil.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import Foundation
@testable import MobiusCore
import Nimble

typealias TestEffectHandler<EffectParameters, Event> = (EffectParameters, EffectCallback<Event>) -> Disposable

class SimpleTestConnectable: Connectable {
var disposed = false

Expand Down
Loading