From 423b482c0bf33cf1f150dcb62482177841bba5e0 Mon Sep 17 00:00:00 2001 From: Florent Pillet Date: Tue, 3 Jan 2017 08:39:30 +0100 Subject: [PATCH 1/4] Rewrote `executing` observable To guarantee consistent delivery of true / false values --- Sources/Action/Action.swift | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/Sources/Action/Action.swift b/Sources/Action/Action.swift index 82511e28..78ed9aac 100644 --- a/Sources/Action/Action.swift +++ b/Sources/Action/Action.swift @@ -98,19 +98,14 @@ public final class Action { .of(notEnabledError, underlyingError) .merge() - let executionStart = executionObservables - let executionEnd = executionObservables - .flatMap { observable -> Observable in - return observable - .flatMap { _ in Observable.empty() } - .concat(Observable.just()) - .catchErrorJustReturn() + executing = executionObservables.flatMap { + Observable + .just(true) + .concat($0.ignoreElements().map { _ in true }.catchError { _ in Observable.empty() }) + .concat(Observable.just(false)) } - - executing = Observable - .of(executionStart.map { _ in true }, executionEnd.map { _ in false }) - .merge() .startWith(false) + .shareReplay(1) Observable .combineLatest(executing, enabledIf) { !$0 && $1 } From ee0ebfcfb0ed11f9fad030525e4fad9a5984c481 Mon Sep 17 00:00:00 2001 From: Florent Pillet Date: Tue, 3 Jan 2017 08:41:31 +0100 Subject: [PATCH 2/4] Remove incorrect information about delivery the `executing` events are not guaranteed to be delivered on MainScheduler --- Sources/Action/Action.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/Action/Action.swift b/Sources/Action/Action.swift index 78ed9aac..3c812658 100644 --- a/Sources/Action/Action.swift +++ b/Sources/Action/Action.swift @@ -38,7 +38,6 @@ public final class Action { public let elements: Observable /// Whether or not we're currently executing. - /// Always observed on MainScheduler. public let executing: Observable /// Observables returned by the workFactory. From 96850ff517562308188467e22f97f317bf9af808 Mon Sep 17 00:00:00 2001 From: Florent Pillet Date: Tue, 3 Jan 2017 09:30:49 +0100 Subject: [PATCH 3/4] An even better and clearer way to write the `executing` concatenation --- Sources/Action/Action.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Action/Action.swift b/Sources/Action/Action.swift index 3c812658..33420071 100644 --- a/Sources/Action/Action.swift +++ b/Sources/Action/Action.swift @@ -98,10 +98,10 @@ public final class Action { .merge() executing = executionObservables.flatMap { - Observable - .just(true) - .concat($0.ignoreElements().map { _ in true }.catchError { _ in Observable.empty() }) - .concat(Observable.just(false)) + Observable.concat([ + Observable.just(true), + $0.ignoreElements().map { _ in true }.catchError { _ in Observable.empty() }, + Observable.just(false)]) } .startWith(false) .shareReplay(1) From 5a17cfeb9dc2452cec4b0404ca4af0d42d1a7d24 Mon Sep 17 00:00:00 2001 From: Florent Pillet Date: Tue, 3 Jan 2017 09:57:37 +0100 Subject: [PATCH 4/4] Tweaked `executing` observable code as per @ishkawa's suggestion --- Sources/Action/Action.swift | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Sources/Action/Action.swift b/Sources/Action/Action.swift index 33420071..8d0b50fe 100644 --- a/Sources/Action/Action.swift +++ b/Sources/Action/Action.swift @@ -98,10 +98,14 @@ public final class Action { .merge() executing = executionObservables.flatMap { - Observable.concat([ - Observable.just(true), - $0.ignoreElements().map { _ in true }.catchError { _ in Observable.empty() }, - Observable.just(false)]) + execution -> Observable in + let execution = execution + .flatMap { _ in Observable.empty() } + .catchError { _ in Observable.empty()} + + return Observable.concat([Observable.just(true), + execution, + Observable.just(false)]) } .startWith(false) .shareReplay(1) @@ -117,7 +121,7 @@ public final class Action { defer { inputs.onNext(value) } - + let execution = executionObservables .take(1) .flatMap { $0 }