Make isComplete reflect result availability (#178)

Fixes #176

Change the `CancelableOperation.isComplete` to forward to the `_inner`
completer, which is not completed until the result is available unlike
`CancelableOperation.isComplete` which may lead the result when
`complete` is called with a `Future` argument. Update the docs to
reflect the new behavior. Keep the detail about this property indicating
whether the operation can still be canceled. This detail was incorrectly
stated before, but matches the new implementation.

Remove details pointing to `CancelableCompleter` from the
`CancelableOperation` docs. Flesh out the docs on the completer so that
the distinction between the two `isComplete` getters is more clear.

Remove a detail about not returning `null` from the `cancel()` doc since
the non-nullable return type already makes this clear.

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 * Fix a bug where `CancelableOperation.then` may invoke the `onValue` callback,
   even if it had been canceled before `CancelableOperation.value` completes.
+* Fix a bug in `CancelableOperation.isComplete` where it may appear to be
+  complete and no longer be cancelable when it in fact could still be canceled.
 
 ## 2.6.1
 

lib/src/cancelable_operation.dart

@@ -31,9 +31,7 @@ class CancelableOperation<T> {
   /// It's guaranteed to only be called once.
   ///
   /// Calling this constructor is equivalent to creating a [CancelableCompleter]
-  /// and completing it with [inner]. As such, [isCompleted] is true from the
-  /// moment this [CancelableOperation] is created, regardless of whether
-  /// [inner] has completed yet or not.
+  /// and completing it with [inner].
   factory CancelableOperation.fromFuture(Future<T> inner,
       {FutureOr Function()? onCancel}) {
     var completer = CancelableCompleter<T>(onCancel: onCancel);
@@ -124,20 +122,19 @@ class CancelableOperation<T> {
 
   /// Cancels this operation.
   ///
-  /// This returns the [Future] returned by the [CancelableCompleter]'s
-  /// `onCancel` callback. Unlike [Stream.cancel], it never returns `null`.
+  /// If this operation [isComplete] or [isCanceled] this call is ignored.
+  /// Returns the result of the `onCancel` callback, if one exists.
   Future cancel() => _completer._cancel();
 
   /// Whether this operation has been canceled before it completed.
   bool get isCanceled => _completer.isCanceled;
 
-  /// Whether the [CancelableCompleter] backing this operation has been
-  /// completed.
+  /// Whether the result of this operation is ready.
   ///
-  /// This value being true does not imply that the [value] future has
-  /// completed, but merely that it is no longer possible to [cancel] the
-  /// operation.
-  bool get isCompleted => _completer.isCompleted;
+  /// When ready, the [value] future is completed with the result value
+  /// or error, and this operation can no longer be cancelled.
+  /// An operation may be complete before the listeners on [value] are invoked.
+  bool get isCompleted => _completer._inner.isCompleted;
 }
 
 /// A completer for a [CancelableOperation].
@@ -161,11 +158,18 @@ class CancelableCompleter<T> {
   /// The operation controlled by this completer.
   late final operation = CancelableOperation<T>._(this);
 
-  /// Whether the completer has completed.
+  /// Whether the [complete] or [completeError] have been called.
+  ///
+  /// Once this completer has been completed with either a result or error,
+  /// neither method may be called again.
+  ///
+  /// If [complete] was called with a [Future] argument, this completer may be
+  /// completed before it's [operation] is completed. In that case the
+  /// [operation] may still be canceled before the result is available.
   bool get isCompleted => _isCompleted;
   bool _isCompleted = false;
 
-  /// Whether the completer was canceled before being completed.
+  /// Whether the completer was canceled before the result was ready.
   bool get isCanceled => _isCanceled;
   bool _isCanceled = false;
 
@@ -174,8 +178,15 @@ class CancelableCompleter<T> {
 
   /// Completes [operation] to [value].
   ///
-  /// If [value] is a [Future], this will complete to the result of that
-  /// [Future] once it completes.
+  /// If [value] is a [Future] the [operation] will complete with the result of
+  /// that `Future` once it is available.
+  /// In that case [isComplete] will be true before the [operation] is complete.
+  ///
+  /// If the type [T] is not nullable [value] may be not be omitted or `null`.
+  ///
+  /// This method may not be called after either [complete] or [completeError]
+  /// has been called once.
+  /// The [isCompleted] is true when either of these methods have been called.
   void complete([FutureOr<T>? value]) {
     if (_isCompleted) throw StateError('Operation already completed');
     _isCompleted = true;
@@ -202,7 +213,11 @@ class CancelableCompleter<T> {
     });
   }
 
-  /// Completes [operation] to [error].
+  /// Completes [operation] with [error] and [stackTrace].
+  ///
+  /// This method may not be called after either [complete] or [completeError]
+  /// has been called once.
+  /// The [isCompleted] is true when either of these methods have been called.
   void completeError(Object error, [StackTrace? stackTrace]) {
     if (_isCompleted) throw StateError('Operation already completed');
     _isCompleted = true;
@@ -211,7 +226,17 @@ class CancelableCompleter<T> {
     _inner.completeError(error, stackTrace);
   }
 
-  /// Cancel the completer.
+  /// Cancel the operation.
+  ///
+  /// This call is be ignored if the result of the operation is already
+  /// available.
+  /// The result of the operation may only be available some time after
+  /// the completer has been completed (using [complete] or [completeError],
+  /// which sets [isCompleted] to true) if completed with a [Future].
+  /// The completer can be cancelled until the result becomes available,
+  /// even if [isCompleted] is true.
+  ///
+  /// This call is ignored if this completer has already been canceled.
   Future _cancel() {
     if (_inner.isCompleted) return Future.value();
 

test/cancelable_operation_test.dart

@@ -44,12 +44,13 @@ void main() {
       expect(completer.isCompleted, isTrue);
     });
 
-    test('sends errors in a future to the future', () {
+    test('sends errors in a future to the future', () async {
       expect(completer.operation.value, throwsA('error'));
       expect(completer.isCompleted, isFalse);
       expect(completer.operation.isCompleted, isFalse);
       completer.complete(Future.error('error'));
       expect(completer.isCompleted, isTrue);
+      await flushMicrotasks();
       expect(completer.operation.isCompleted, isTrue);
     });
 
@@ -68,6 +69,15 @@ void main() {
       expect(await operation.then((_) {}).value, null);
     });
 
+    test('is not complete until the result is available', () async {
+      var backingWork = Completer();
+      var operation = CancelableOperation.fromFuture(backingWork.future);
+      expect(operation.isCompleted, isFalse);
+      backingWork.complete();
+      await backingWork.future;
+      expect(operation.isCompleted, isTrue);
+    });
+
     group('throws a StateError if completed', () {
       test('successfully twice', () {
         completer.complete(1);