Skip to content

Commit

Permalink
Add Promise.makeChild()
Browse files Browse the repository at this point in the history
Fixes #56.
  • Loading branch information
lilyball committed Jul 27, 2020
1 parent 5a58c73 commit 7078029
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 9 deletions.
10 changes: 7 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@ an asynchronous resource (such as a network load). In this scenario you may wish
resource, without preventing cancellation of the resource load if all clients cancel their requests. This can be accomplished by holding onto the result of calling
`.propagatingCancellation(on:cancelRequested:)`. The promise returned from this method will propagate cancellation to its parent as soon as all children
have requested cancellation even if the promise is still in scope. When cancellation is requested, the `cancelRequested` handler will be invoked immediately prior to
propagating cancellation upwards; this enables you to release your reference to the promise (so a new request by a client will create a brand new resource load). An
example of this might look like:
propagating cancellation upwards; this enables you to release your reference to the promise (so a new request by a client will create a brand new resource load).
Returning a new child to each client can be done using `makeChild()`. An example of this might look like:

```swift
func loadResource(at url: URL) {
Expand All @@ -334,7 +334,7 @@ func loadResource(at url: URL) {
resourceLoads[url] = promise
}
// Return a new child for each request so all clients have to cancel, not just one.
return promise.then(on: .immediate, { _ in })
return promise.makeChild()
}
```

Expand Down Expand Up @@ -450,8 +450,12 @@ Unless you explicitly state otherwise, any contribution intentionally submitted
other children request cancellation. Unlike `tap`, requesting cancellation of `onCancel` when there are no other children will propagate cancellation to the parent. The
motivation here is attaching an `onCancel` observer shouldn't prevent cancellation that would otherwise occur, but when it's the only child it should behave like the
other standard observers ([#57][]).
- Add method `Promise.makeChild()`. This returns a new child of the receiver that adopts the receiver's value and propagates cancellation like any other observer.
The purpose here is to be used when handing back multiple children of one parent to callers, as handing back the parent means any one caller can cancel it without
the other callers' participation. This is particularly useful in conjunction with `propagatingCancellation(on:cancelRequested:)` ([#56][]).

[#54]: https://github.com/lilyball/Tomorrowland/issues/54 "Resolver.resolve(with: Promise) handles cancellation incorrectly"
[#56]: https://github.com/lilyball/Tomorrowland/issues/56 "Add Promise.makeChild()"
[#57]: https://github.com/lilyball/Tomorrowland/issues/57 "onCancel() handler prevents automatic cancellation propagation"

### v1.3.0
Expand Down
10 changes: 10 additions & 0 deletions Sources/ObjC/TWLPromise.h
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,16 @@ NS_SWIFT_NAME(ObjCPromise)
/// \returns A new promise that will resolve to the same value as the receiver.
- (TWLPromise<ValueType,ErrorType> *)propagatingCancellationOnContext:(TWLContext *)context cancelRequestedHandler:(void (^)(TWLPromise<ValueType,ErrorType> *promise))cancelRequested NS_SWIFT_NAME(propagatingCancellation(on:cancelRequested:));

/// Returns a promise that adopts the same value as the receiver.
///
/// This method is used in order to hand back child promises to callers so that they cannot directly
/// request cancellation of a shared parent promise. This is most useful in conjunction with
/// \c -propagatingCancellationOnContext:cancelRequestedHandler: but could also be used any time a
/// shared promise is given to multiple callers.
///
/// \returns A new promise that will resolve to the same value as the receiver.
- (TWLPromise<ValueType,ErrorType> *)makeChild;

/// Returns the promise's value if it's already been resolved.
///
/// If the return value is \c YES and both \a *outValue and \a *outError are \c nil this means the
Expand Down
7 changes: 7 additions & 0 deletions Sources/ObjC/TWLPromise.mm
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,13 @@ - (TWLPromise *)propagatingCancellationOnContext:(TWLContext *)context cancelReq
return promise;
}

- (TWLPromise *)makeChild {
TWLResolver *resolver;
auto promise = [[TWLPromise alloc] initWithResolver:&resolver];
[self pipeToResolver:resolver];
return promise;
}

- (BOOL)getValue:(id _Nullable __strong *)outValue error:(id _Nullable __strong *)outError {
return [_box getValue:outValue error:outError];
}
Expand Down
18 changes: 18 additions & 0 deletions Sources/Promise.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1157,6 +1157,10 @@ public struct Promise<Value,Error> {
/// (such as a network load) without preventing cancellation of the load in the event that no
/// children care about it anymore.
///
/// - Important: Do not give the returned promise directly to callers. Instead always hand back
/// a child, such as returned from `makeChild`. Otherwise automatic cancellation propagation
/// won't work as expected.
///
/// - Parameter context: The context to invoke the callback on.
/// - Parameter cancelRequested: The callback that is invoked when the promise is requested to
/// cancel, either because `.requestCancel()` was invoked on it directly or because all
Expand Down Expand Up @@ -1185,6 +1189,20 @@ public struct Promise<Value,Error> {
return promise
}

/// Returns a promise that adopts the same value as the receiver.
///
/// This method is used in order to hand back child promises to callers so that they cannot
/// directly request cancellation of a shared parent promise. This is most useful in conjunction
/// with `propagatingCancellation(on:cancelRequested:)` but could also be used any time a shared
/// promise is given to multiple callers.
///
/// - Returns: A new promise that will resolve to the same value as the receiver.
public func makeChild() -> Promise<Value,Error> {
let (promise, resolver) = Promise<Value,Error>.makeWithResolver()
pipe(to: resolver)
return promise
}

// MARK: -

/// Passes the `Promise` to a block and then returns the `Promise` for further chaining.
Expand Down
15 changes: 15 additions & 0 deletions Tests/CancelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,21 @@ final class CancelTests: XCTestCase {
}
}

func testPropagateCancelMakeChild() {
let expectations: [XCTestExpectation]
let sema: DispatchSemaphore
do {
let promise: Promise<Int,String>
(promise, sema) = Promise<Int,String>.makeCancellablePromise(error: "foo")
let promise2 = promise.makeChild()
expectations = [promise, promise2].map({ XCTestExpectation(on: .immediate, onCancel: $0) })
promise2.requestCancel()
}
sema.signal()
wait(for: expectations, timeout: 0)

}

func testPropagateCancelTryFlatMapResultThrowing() {
struct DummyError: Swift.Error {}
let expectations: [XCTestExpectation]
Expand Down
14 changes: 14 additions & 0 deletions Tests/ObjC/TWLCancelTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,20 @@ - (void)testPropagateCancelWhenCancelled {
}];
}

- (void)testPropagateCancelMakeChild {
NSArray<XCTestExpectation*> *expectations;
dispatch_semaphore_t sema = dispatch_semaphore_create(0);
@autoreleasepool {
TWLPromise *promise = makeCancellablePromiseWithValue(@2, &sema);
TWLPromise *promise2 = [promise makeChild];
expectations = @[TWLExpectationCancelOnContext(TWLContext.immediate, promise),
TWLExpectationCancelOnContext(TWLContext.immediate, promise2)];
[promise2 requestCancel];
}
dispatch_semaphore_signal(sema);
[self waitForExpectations:expectations timeout:0];
}

- (void)testPropagateCancelDelayedPromise {
NSArray<XCTestExpectation*> *expectations;
dispatch_semaphore_t sema = dispatch_semaphore_create(0);
Expand Down
11 changes: 11 additions & 0 deletions Tests/ObjC/TWLPromiseTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,17 @@ - (void)testAlwaysReturningPromise {
[self waitForExpectations:@[expectation] timeout:1];
}

- (void)testMakeChild {
TWLResolver<NSNumber*,NSString*> *resolver;
__auto_type promise = [[TWLPromise<NSNumber*,NSString*> alloc] initWithResolver:&resolver];
__auto_type promise2 = [promise makeChild];
XCTAssertNotEqualObjects(promise, promise2);
TWLAssertPromiseNotResolved(promise2);
[resolver fulfillWithValue:@42];
TWLAssertPromiseFulfilledWithValue(promise, @42);
TWLAssertPromiseFulfilledWithValue(promise2, @42);
}

- (void)testExtremeChaining {
// Piping one promise to another should support extreme recursion without blowing the stack.

Expand Down
22 changes: 16 additions & 6 deletions Tests/PromiseTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,16 @@ final class PromiseTests: XCTestCase {
wait(for: [innerExpectation, outerExpectation], timeout: 1)
}

func testMakeChild() {
let (promise, resolver) = Promise<Int,String>.makeWithResolver()
let promise2 = promise.makeChild()
XCTAssertNotEqual(promise, promise2);
XCTAssertNil(promise2.result);
resolver.fulfill(with: 42)
XCTAssertEqual(promise.result, .value(42))
XCTAssertEqual(promise2.result, .value(42))
}

func testUpcast() {
struct DummyError: Error {}
let promise = Promise<Int,DummyError>(rejected: DummyError())
Expand Down Expand Up @@ -565,8 +575,8 @@ final class PromiseTests: XCTestCase {
})
blockChildPromise = childPromise
}
let promise1 = childPromise.then(on: .immediate, { _ in })
let promise2 = childPromise.then(on: .immediate, { _ in })
let promise1 = childPromise.makeChild()
let promise2 = childPromise.makeChild()
promise1.requestCancel()
guard XCTWaiter(delegate: self).wait(for: [notYetExpectation], timeout: 0) == .completed else {
XCTFail("Cancel requested on child too early")
Expand All @@ -578,7 +588,7 @@ final class PromiseTests: XCTestCase {
wait(for: [childExpectation, requestExpectation] + cancelExpectations, timeout: 0, enforceOrder: true)

// Adding new children at this point causes no problems, they're just insta-cancelled.
let promise3 = childPromise.then(on: .immediate, { _ in })
let promise3 = childPromise.makeChild()
XCTAssertEqual(promise3.result, .cancelled)

withExtendedLifetime(childPromise) {} // Ensure childPromise lives to this point
Expand Down Expand Up @@ -662,7 +672,7 @@ final class PromiseTests: XCTestCase {
childExpectation.fulfill()
})
}
childPromise.then(on: .immediate, { _ in }).requestCancel()
childPromise.makeChild().requestCancel()
// cancel should already be enqueued on the queue at this point. No need for a timeout
queue.waitUntilAllOperationsAreFinished()
wait(for: [childExpectation], timeout: 0)
Expand Down Expand Up @@ -710,13 +720,13 @@ final class PromiseTests: XCTestCase {
childPromise = rootPromise.propagatingCancellation(on: .immediate, cancelRequested: { _ in
childExpectation.fulfill()
})
let child2Promise = rootPromise.then(on: .immediate, { _ in })
let child2Promise = rootPromise.makeChild()
child2Expectation = XCTestExpectation(on: .immediate, onSuccess: child2Promise, expectedValue: 42)
child2Promise.tap().always(on: .immediate, { _ in
notYetExpectation.fulfill()
})
}
childPromise.then(on: .immediate, { _ in }).requestCancel()
childPromise.makeChild().requestCancel()
wait(for: [notYetExpectation, childExpectation], timeout: 0)
sema.signal()
wait(for: [rootPromiseCancelExpectation, child2Expectation], timeout: 1)
Expand Down

0 comments on commit 7078029

Please sign in to comment.