From 5861feca2772e90ce0033ea54cada1566d2167fc Mon Sep 17 00:00:00 2001 From: Subeom Choi Date: Wed, 17 Apr 2024 17:09:40 +0900 Subject: [PATCH] change: do not cancel return of then from upstream --- .../Contract/Chain/ContractFilter.swift | 22 +++++---- .../Contract/Chain/ContractRecover.swift | 22 +++++---- .../Contract/Chain/ContractThen.swift | 22 +++++---- .../Promise/Chain/PromiseRecover.swift | 18 +++----- .../Promise/Chain/PromiseThen.swift | 18 +++----- Source/Concurrency/Promise/Promise.swift | 45 ++++--------------- .../Contract/Chain/ContractFilterTest.swift | 4 +- .../Contract/Chain/ContractRecoverTest.swift | 2 +- .../Contract/Chain/ContractThenTest.swift | 2 +- .../Promise/Attribute/PromiseDelayTest.swift | 2 +- .../Promise/Chain/PromiseCatchTest.swift | 2 +- .../Promise/Chain/PromiseFinallyTest.swift | 2 +- .../Promise/Chain/PromiseRecoverTest.swift | 4 +- .../Promise/Chain/PromiseThenTest.swift | 4 +- 14 files changed, 62 insertions(+), 107 deletions(-) diff --git a/Source/Concurrency/Contract/Chain/ContractFilter.swift b/Source/Concurrency/Contract/Chain/ContractFilter.swift index 436b1c9..16e06d0 100644 --- a/Source/Concurrency/Contract/Chain/ContractFilter.swift +++ b/Source/Concurrency/Contract/Chain/ContractFilter.swift @@ -67,7 +67,7 @@ extension Contract { subscribe( queue: queue, - onResolved: schedule { [weak self] value, finish in + onResolved: schedule { value, finish in let promise = block(value) promise.subscribe( queue: queue, @@ -77,11 +77,10 @@ extension Contract { contract.resolve(result) }, onRejected: { _ in }, - onCanceled: { [weak contract] in contract?.cancel() } - ) - self?.subscribe( - queue: queue, - onCanceled: { [weak promise] in promise?.cancel() } + onCanceled: { [weak contract] in + defer { finish() } + contract?.cancel() + } ) }, onRejected: { error in contract.reject(error) }, @@ -109,7 +108,7 @@ extension Contract { subscribe( queue: queue, - onResolved: schedule { [weak self] value, finish in + onResolved: schedule { value, finish in guard let promise = block(value) else { return } promise.subscribe( queue: queue, @@ -118,11 +117,10 @@ extension Contract { contract.resolve($0) }, onRejected: { _ in }, - onCanceled: { [weak contract] in contract?.cancel() } - ) - self?.subscribe( - queue: queue, - onCanceled: { [weak promise] in promise?.cancel() } + onCanceled: { [weak contract] in + defer { finish() } + contract?.cancel() + } ) }, onRejected: { error in contract.reject(error) }, diff --git a/Source/Concurrency/Contract/Chain/ContractRecover.swift b/Source/Concurrency/Contract/Chain/ContractRecover.swift index fd3fa5e..d1b4b2f 100644 --- a/Source/Concurrency/Contract/Chain/ContractRecover.swift +++ b/Source/Concurrency/Contract/Chain/ContractRecover.swift @@ -56,7 +56,7 @@ extension Contract { subscribe( queue: queue, onResolved: { value in contract.resolve(value) }, - onRejected: schedule { [weak self] error, finish in + onRejected: schedule { error, finish in do { let promise = try block(error) promise.subscribe( @@ -69,11 +69,10 @@ extension Contract { defer { finish() } contract.reject($0) }, - onCanceled: { [weak contract] in contract?.cancel() } - ) - self?.subscribe( - queue: queue, - onCanceled: { [weak promise] in promise?.cancel() } + onCanceled: { [weak contract] in + defer { finish() } + contract?.cancel() + } ) } catch let error { @@ -132,7 +131,7 @@ extension Contract { subscribe( queue: queue, onResolved: { value in contract.resolve(value) }, - onRejected: schedule { [weak self] error, finish in + onRejected: schedule { error, finish in let promise = block(error) promise.subscribe( queue: queue, @@ -144,11 +143,10 @@ extension Contract { defer { finish() } contract.reject($0) }, - onCanceled: { [weak contract] in contract?.cancel() } - ) - self?.subscribe( - queue: queue, - onCanceled: { [weak promise] in promise?.cancel() } + onCanceled: { [weak contract] in + defer { finish() } + contract?.cancel() + } ) }, onCanceled: { [weak contract] in contract?.cancel() } diff --git a/Source/Concurrency/Contract/Chain/ContractThen.swift b/Source/Concurrency/Contract/Chain/ContractThen.swift index f30f6bc..eff6a04 100644 --- a/Source/Concurrency/Contract/Chain/ContractThen.swift +++ b/Source/Concurrency/Contract/Chain/ContractThen.swift @@ -55,7 +55,7 @@ extension Contract { subscribe( queue: queue, - onResolved: schedule { [weak self] value, finish in + onResolved: schedule { value, finish in do { let promise = try block(value) promise.subscribe( @@ -68,11 +68,10 @@ extension Contract { defer { finish() } contract.reject($0) }, - onCanceled: { [weak contract] in contract?.cancel() } - ) - self?.subscribe( - queue: queue, - onCanceled: { [weak promise] in promise?.cancel() } + onCanceled: { [weak contract] in + defer { finish() } + contract?.cancel() + } ) } catch let error { @@ -131,7 +130,7 @@ extension Contract where Failure == Never { subscribe( queue: queue, - onResolved: schedule { [weak self] value, finish in + onResolved: schedule { value, finish in let promise = block(value) promise.subscribe( queue: queue, @@ -143,11 +142,10 @@ extension Contract where Failure == Never { defer { finish() } contract.reject($0) }, - onCanceled: { [weak contract] in contract?.cancel() } - ) - self?.subscribe( - queue: queue, - onCanceled: { [weak promise] in promise?.cancel() } + onCanceled: { [weak contract] in + defer { finish() } + contract?.cancel() + } ) }, onRejected: { _ in }, diff --git a/Source/Concurrency/Promise/Chain/PromiseRecover.swift b/Source/Concurrency/Promise/Chain/PromiseRecover.swift index 737920b..374435f 100644 --- a/Source/Concurrency/Promise/Chain/PromiseRecover.swift +++ b/Source/Concurrency/Promise/Chain/PromiseRecover.swift @@ -24,7 +24,8 @@ extension Promise { do { let value = try block($0) promiseReturn.resolve(value) - } catch let error { + } + catch let error { promiseReturn.reject(error) } }, @@ -46,7 +47,7 @@ extension Promise { subscribe( queue: queue, onResolved: { promiseReturn.resolve($0) }, - onRejected: { [weak self] in + onRejected: { do { let promise = try block($0) promise.subscribe( @@ -55,11 +56,8 @@ extension Promise { onRejected: { promiseReturn.reject($0) }, onCanceled: { [weak promiseReturn] in promiseReturn?.cancel() } ) - self?.subscribe( - queue: queue, - onCanceled: { [weak promise] in promise?.cancel() } - ) - } catch let error { + } + catch let error { promiseReturn.reject(error) } }, @@ -105,7 +103,7 @@ extension Promise { subscribe( queue: queue, onResolved: { promiseReturn.resolve($0) }, - onRejected: { [weak self] in + onRejected: { let promise = block($0) promise.subscribe( queue: queue, @@ -113,10 +111,6 @@ extension Promise { onRejected: { promiseReturn.reject($0) }, onCanceled: { [weak promiseReturn] in promiseReturn?.cancel() } ) - self?.subscribe( - queue: queue, - onCanceled: { [weak promise] in promise?.cancel() } - ) }, onCanceled: { [weak promiseReturn] in promiseReturn?.cancel() } ) diff --git a/Source/Concurrency/Promise/Chain/PromiseThen.swift b/Source/Concurrency/Promise/Chain/PromiseThen.swift index ee34d36..b1ea4fa 100644 --- a/Source/Concurrency/Promise/Chain/PromiseThen.swift +++ b/Source/Concurrency/Promise/Chain/PromiseThen.swift @@ -23,7 +23,8 @@ extension Promise { do { let value = try block($0) promiseReturn.resolve(value) - } catch let error { + } + catch let error { promiseReturn.reject(error) } }, @@ -45,7 +46,7 @@ extension Promise { subscribe( queue: queue, - onResolved: { [weak self] in + onResolved: { do { let promise = try block($0) promise.subscribe( @@ -54,11 +55,8 @@ extension Promise { onRejected: { promiseReturn.reject($0) }, onCanceled: { [weak promiseReturn] in promiseReturn?.cancel() } ) - self?.subscribe( - queue: queue, - onCanceled: { [weak promise] in promise?.cancel() } - ) - } catch let error { + } + catch let error { promiseReturn.reject(error) } }, @@ -104,7 +102,7 @@ extension Promise where Failure == Never { subscribe( queue: queue, - onResolved: { [weak self] in + onResolved: { let promise = block($0) promise.subscribe( queue: queue, @@ -112,10 +110,6 @@ extension Promise where Failure == Never { onRejected: { promiseReturn.reject($0) }, onCanceled: { [weak promiseReturn] in promiseReturn?.cancel() } ) - self?.subscribe( - queue: queue, - onCanceled: { [weak promise] in promise?.cancel() } - ) }, onRejected: { _ in }, onCanceled: { [weak promiseReturn] in promiseReturn?.cancel() } diff --git a/Source/Concurrency/Promise/Promise.swift b/Source/Concurrency/Promise/Promise.swift index 94c08f4..fe20d73 100644 --- a/Source/Concurrency/Promise/Promise.swift +++ b/Source/Concurrency/Promise/Promise.swift @@ -13,16 +13,12 @@ public final class Promise { let queue: DispatchQueue let pendingGroup: DispatchGroup - var cancelSubscribers: [PromiseCancelSubscriber] - init(queue: DispatchQueue = .global()) { self.state = Atomic(.pending) self.queue = queue self.pendingGroup = DispatchGroup() - self.cancelSubscribers = [] - pendingGroup.enter() } @@ -241,16 +237,8 @@ extension Promise { func cancel() { state.mutate { state in - if case .canceled = state {} else { - if case .pending = state { - pendingGroup.leave() - } - for cancelSubscriber in self.cancelSubscribers { - pendingGroup.notify(queue: queue) { - cancelSubscriber.onCanceled() - } - } - cancelSubscribers = [] + if case .pending = state { + pendingGroup.leave() return .canceled } @@ -274,18 +262,8 @@ extension Promise { else if case .rejected(let error) = state { onRejected(error) } - } - state.capture { state in - if case .canceled = state { - pendingGroup.notify(queue: queue) { - onCanceled() - } - } - else { - cancelSubscribers.append(PromiseCancelSubscriber( - queue: queue, - onCanceled: onCanceled - )) + else if case .canceled = state { + onCanceled() } } } @@ -294,17 +272,12 @@ extension Promise { queue: DispatchQueue, onCanceled: @escaping () -> Void ) { - state.capture { state in + let state = self.state + pendingGroup.notify(queue: queue) { + let state = state.capture { $0 } + if case .canceled = state { - pendingGroup.notify(queue: queue) { - onCanceled() - } - } - else { - cancelSubscribers.append(PromiseCancelSubscriber( - queue: queue, - onCanceled: onCanceled - )) + onCanceled() } } } diff --git a/Test/Concurrency/Contract/Chain/ContractFilterTest.swift b/Test/Concurrency/Contract/Chain/ContractFilterTest.swift index 97ddb23..e87ab45 100644 --- a/Test/Concurrency/Contract/Chain/ContractFilterTest.swift +++ b/Test/Concurrency/Contract/Chain/ContractFilterTest.swift @@ -74,7 +74,7 @@ final class ContractFilterTest: XCTestCase { contract0.resolve("10") } PromiseTest.expect(semaphore: end, timeout: .seconds(1)) - PromiseTest.expect(promise: filterPromise, state: .canceled, timeout: .seconds(1)) + PromiseTest.expect(promise: filterPromise, state: .pending, timeout: .seconds(1)) } func test__never_filter_bool() { @@ -142,7 +142,7 @@ final class ContractFilterTest: XCTestCase { contract0.resolve("10") } PromiseTest.expect(semaphore: end, timeout: .seconds(1)) - PromiseTest.expect(promise: filterPromise, state: .canceled, timeout: .seconds(1)) + PromiseTest.expect(promise: filterPromise, state: .pending, timeout: .seconds(1)) } func test__filter_schedule_sync() throws { diff --git a/Test/Concurrency/Contract/Chain/ContractRecoverTest.swift b/Test/Concurrency/Contract/Chain/ContractRecoverTest.swift index cbcf3a7..bfa0c10 100644 --- a/Test/Concurrency/Contract/Chain/ContractRecoverTest.swift +++ b/Test/Concurrency/Contract/Chain/ContractRecoverTest.swift @@ -77,7 +77,7 @@ final class ContractRecoverTest: XCTestCase { contract0.reject(ContractTest.SampleError.one) } PromiseTest.expect(semaphore: end, timeout: .seconds(1)) - PromiseTest.expect(promise: recoverPromise, state: .canceled, timeout: .seconds(1)) + PromiseTest.expect(promise: recoverPromise, state: .pending, timeout: .seconds(1)) } func test__recover_schedule_sync() throws { diff --git a/Test/Concurrency/Contract/Chain/ContractThenTest.swift b/Test/Concurrency/Contract/Chain/ContractThenTest.swift index ab0de5c..24c6bdb 100644 --- a/Test/Concurrency/Contract/Chain/ContractThenTest.swift +++ b/Test/Concurrency/Contract/Chain/ContractThenTest.swift @@ -245,7 +245,7 @@ final class ContractThenTest: XCTestCase { contract0.resolve(10) } PromiseTest.expect(semaphore: end, timeout: .seconds(1)) - PromiseTest.expect(promise: thenPromise, state: .canceled, timeout: .seconds(1)) + PromiseTest.expect(promise: thenPromise, state: .pending, timeout: .seconds(1)) } func test__never_then_return_value() { diff --git a/Test/Concurrency/Promise/Attribute/PromiseDelayTest.swift b/Test/Concurrency/Promise/Attribute/PromiseDelayTest.swift index 60787dc..806d360 100644 --- a/Test/Concurrency/Promise/Attribute/PromiseDelayTest.swift +++ b/Test/Concurrency/Promise/Attribute/PromiseDelayTest.swift @@ -72,7 +72,7 @@ final class PromiseDelayTest: XCTestCase { } PromiseTest.expect(semaphore: end, timeout: .seconds(1)) - PromiseTest.expect(promise: promise2, state: .canceled, timeout: .seconds(1)) + PromiseTest.expect(promise: promise2, state: .resolved({ $0 == () }), timeout: .seconds(1)) } func test__never_delay_create() { diff --git a/Test/Concurrency/Promise/Chain/PromiseCatchTest.swift b/Test/Concurrency/Promise/Chain/PromiseCatchTest.swift index b95d5ab..44eadc6 100644 --- a/Test/Concurrency/Promise/Chain/PromiseCatchTest.swift +++ b/Test/Concurrency/Promise/Chain/PromiseCatchTest.swift @@ -52,6 +52,6 @@ final class PromiseCatchTest: XCTestCase { } PromiseTest.expect(semaphore: end, timeout: .seconds(1)) - PromiseTest.expect(promise: promise1, state: .canceled, timeout: .seconds(1)) + PromiseTest.expect(promise: promise1, state: .rejected(PromiseTest.SampleError.one), timeout: .seconds(1)) } } diff --git a/Test/Concurrency/Promise/Chain/PromiseFinallyTest.swift b/Test/Concurrency/Promise/Chain/PromiseFinallyTest.swift index a5e9c99..60ece08 100644 --- a/Test/Concurrency/Promise/Chain/PromiseFinallyTest.swift +++ b/Test/Concurrency/Promise/Chain/PromiseFinallyTest.swift @@ -51,6 +51,6 @@ final class PromiseFinallyTest: XCTestCase { } PromiseTest.expect(semaphore: end, timeout: .seconds(1)) - PromiseTest.expect(promise: promise1, state: .canceled, timeout: .seconds(1)) + PromiseTest.expect(promise: promise1, state: .rejected(PromiseTest.SampleError.one), timeout: .seconds(1)) } } diff --git a/Test/Concurrency/Promise/Chain/PromiseRecoverTest.swift b/Test/Concurrency/Promise/Chain/PromiseRecoverTest.swift index efa9974..d4e53fd 100644 --- a/Test/Concurrency/Promise/Chain/PromiseRecoverTest.swift +++ b/Test/Concurrency/Promise/Chain/PromiseRecoverTest.swift @@ -140,7 +140,7 @@ final class PromiseRecoverTest: XCTestCase { } PromiseTest.expect(semaphore: end, timeout: .seconds(1)) - PromiseTest.expect(promise: promise1, state: .canceled, timeout: .seconds(1)) - PromiseTest.expect(promise: recoverPromise, state: .canceled, timeout: .seconds(1)) + PromiseTest.expect(promise: promise1, state: .pending, timeout: .seconds(1)) + PromiseTest.expect(promise: recoverPromise, state: .pending, timeout: .seconds(1)) } } diff --git a/Test/Concurrency/Promise/Chain/PromiseThenTest.swift b/Test/Concurrency/Promise/Chain/PromiseThenTest.swift index ef98bfc..c0b51ea 100644 --- a/Test/Concurrency/Promise/Chain/PromiseThenTest.swift +++ b/Test/Concurrency/Promise/Chain/PromiseThenTest.swift @@ -170,8 +170,8 @@ final class PromiseThenTest: XCTestCase { } PromiseTest.expect(semaphore: end, timeout: .seconds(1)) - PromiseTest.expect(promise: promise1, state: .canceled, timeout: .seconds(1)) - PromiseTest.expect(promise: thenPromise, state: .canceled, timeout: .seconds(1)) + PromiseTest.expect(promise: promise1, state: .pending, timeout: .seconds(1)) + PromiseTest.expect(promise: thenPromise, state: .pending, timeout: .seconds(1)) } func test__then_return_promise_from_reject() {