Don't invoke onValue for a canceled operation (#177)
Fixes #172
The `isCompleted` boolean does not necessarily indicate that
`valueOrCancellation` will complete with a value. If the completer was
completed with a `Future` and then canceled before that argument
invokes the listener callback, the `valueOrCancellation` future will
complete with `null` but `isCompleted` will be true. We only have a
value to pass to `onValue` when the completer was completed and not
canceled.
Add tests for the behavior of `then` when `cancel` is called after
`completer.complete`, both when it was completed with a Future and a
non-Future value.
Move the initialization for `originalCompleter` to the `setup` for the
tests so that it can be used before calling `then`.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1560001..4680638 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
-## 2.6.2
+## 2.6.2-dev
-* Internal changes only.
+* Fix a bug where `CancelableOperation.then` may invoke the `onValue` callback,
+ even if it had been canceled before `CancelableOperation.value` completes.
## 2.6.1
diff --git a/lib/src/cancelable_operation.dart b/lib/src/cancelable_operation.dart
index c321078..6e6ef17 100644
--- a/lib/src/cancelable_operation.dart
+++ b/lib/src/cancelable_operation.dart
@@ -101,7 +101,7 @@
valueOrCancellation().then((T? result) {
if (!completer.isCanceled) {
- if (isCompleted) {
+ if (isCompleted && !isCanceled) {
assert(result is T);
completer.complete(Future.sync(() => onValue(result as T)));
} else if (onCancel != null) {
diff --git a/test/cancelable_operation_test.dart b/test/cancelable_operation_test.dart
index 0e8d8d4..47a1e91 100644
--- a/test/cancelable_operation_test.dart
+++ b/test/cancelable_operation_test.dart
@@ -267,10 +267,10 @@
onError = expectAsync2((e, s) => 'Fake', count: 0, id: 'onError');
onCancel = expectAsync0(() => 'Fake', count: 0, id: 'onCancel');
propagateCancel = false;
+ originalCompleter = CancelableCompleter();
});
CancelableOperation<String> runThen() {
- originalCompleter = CancelableCompleter();
return originalCompleter.operation.then(onValue!,
onError: onError,
onCancel: onCancel,
@@ -387,6 +387,29 @@
expect(runThen().value, throwsA('error'));
originalCompleter.operation.cancel();
});
+
+ test('after completing with a future does not invoke `onValue`',
+ () async {
+ onValue = expectAsync1((_) => '', count: 0);
+ onCancel = null;
+ var operation = runThen();
+ var workCompleter = Completer<int>();
+ originalCompleter.complete(workCompleter.future);
+ originalCompleter.operation.cancel();
+ workCompleter.complete(0);
+ expect(operation.isCanceled, true);
+ await workCompleter.future;
+ });
+
+ test('after the value is completed invokes `onValue`', () {
+ onValue = expectAsync1((_) => 'foo', count: 1);
+ onCancel = expectAsync1((_) => '', count: 0);
+ originalCompleter.complete(0);
+ originalCompleter.operation.cancel();
+ var operation = runThen();
+ expect(operation.value, completion('foo'));
+ expect(operation.isCanceled, false);
+ });
});
group('returned operation canceled', () {
