From aabcf7cc7bbcef9db8e70a41bb700c3fdcfa1d1d Mon Sep 17 00:00:00 2001
From: Michael Dowling <mtdowling@gmail.com>
Date: Sun, 8 Mar 2015 15:00:05 -0700
Subject: [PATCH] Fixing up cancellation and adding constants

- This commit adds constants for each promise state.
- Removing the "cancelled" state in favor of rejecting promises with a
  CancellationException. Trying to follow
  https://github.com/promises-aplus/cancellation-spec/issues/7
- Throwing exceptions when you try to fulfill/reject FulfilledPromise and
  RejectedPromise.
---
 src/CancellationException.php  |  6 +++
 src/FulfilledPromise.php       |  6 +--
 src/Promise.php                | 79 +++++++++++++++-------------------
 src/PromiseInterface.php       | 21 +++++++--
 src/RejectedPromise.php        |  6 +--
 tests/FulfilledPromiseTest.php | 33 ++++++++++++--
 tests/PromiseTest.php          | 58 ++++++++++++++++++++++---
 tests/RejectedPromiseTest.php  | 32 ++++++++++++--
 8 files changed, 174 insertions(+), 67 deletions(-)
 create mode 100644 src/CancellationException.php

diff --git a/src/CancellationException.php b/src/CancellationException.php
new file mode 100644
index 0000000..d359ace
--- /dev/null
+++ b/src/CancellationException.php
@@ -0,0 +1,6 @@
+<?php
+namespace GuzzleHttp\Promise;
+
+class CancellationException extends RejectionException
+{
+}
diff --git a/src/FulfilledPromise.php b/src/FulfilledPromise.php
index 29e7ba4..474d5e3 100644
--- a/src/FulfilledPromise.php
+++ b/src/FulfilledPromise.php
@@ -46,17 +46,17 @@ public function wait($unwrap = true, $defaultDelivery = null)
 
     public function getState()
     {
-        return 'fulfilled';
+        return self::FULFILLED;
     }
 
     public function resolve($value)
     {
-        // pass
+        throw new \RuntimeException("Cannot resolve a fulfilled promise");
     }
 
     public function reject($reason)
     {
-        // pass
+        throw new \RuntimeException("Cannot reject a fulfilled promise");
     }
 
     public function cancel()
diff --git a/src/Promise.php b/src/Promise.php
index d629bfb..4a7de90 100644
--- a/src/Promise.php
+++ b/src/Promise.php
@@ -12,19 +12,10 @@
  */
 class Promise implements PromiseInterface
 {
-    /** @var string Promise state: pending, fulfilled, rejected, cancelled */
-    private $state = 'pending';
-
-    /** @var array[] Array of [promise, fulfilled, rejected] */
+    private $state = self::PENDING;
     private $handlers = [];
-
-    /** @var callable Wait function */
     private $waitFn;
-
-    /** @var callable */
     private $cancelFn;
-
-    /** @var mixed Delivered result */
     private $result;
 
     /**
@@ -60,7 +51,7 @@ public function then(
         callable $onFulfilled = null,
         callable $onRejected = null
     ) {
-        if ($this->state === 'pending') {
+        if ($this->state === self::PENDING) {
             $p = new Promise([$this, 'wait'], [$this, 'cancel']);
             // Keep track of this dependent promise so that we resolve it
             // later when a value has been delivered.
@@ -69,7 +60,7 @@ public function then(
         }
 
         // Return a fulfilled promise and immediately invoke any callbacks.
-        if ($this->state === 'fulfilled') {
+        if ($this->state === self::FULFILLED) {
             return $onFulfilled
                 ? self::promiseFor($this->result)->then($onFulfilled)
                 : self::promiseFor($this->result);
@@ -86,7 +77,7 @@ public function then(
 
     public function wait($unwrap = true, $defaultDelivery = null)
     {
-        if ($this->state === 'pending') {
+        if ($this->state === self::PENDING) {
             if (!$this->waitFn) {
                 // If there's not wait function, then resolve the promise with
                 // the provided $defaultDelivery value.
@@ -97,7 +88,7 @@ public function wait($unwrap = true, $defaultDelivery = null)
                     $wfn = $this->waitFn;
                     $this->waitFn = null;
                     $wfn();
-                    if ($this->state === 'pending') {
+                    if ($this->state === self::PENDING) {
                         throw new \LogicException('Invoking the wait callback did not resolve the promise');
                     }
                 } catch (\Exception $e) {
@@ -118,7 +109,7 @@ public function wait($unwrap = true, $defaultDelivery = null)
             $result = $result->wait();
         }
 
-        if ($this->state === 'fulfilled') {
+        if ($this->state === self::FULFILLED) {
             return $result;
         }
 
@@ -135,12 +126,11 @@ public function getState()
 
     public function cancel()
     {
-        if ($this->state !== 'pending') {
+        if ($this->state !== self::PENDING) {
             return;
         }
 
         $this->waitFn = null;
-
         if ($this->cancelFn) {
             $fn = $this->cancelFn;
             $this->cancelFn = null;
@@ -152,17 +142,19 @@ public function cancel()
             }
         }
 
-        $this->state = 'cancelled';
-        $this->result = new \LogicException('Promise has been cancelled');
+        // Reject the promise only if it wasn't rejected in a then callback.
+        if ($this->state === self::PENDING) {
+            $this->reject(new CancellationException('Promise has been cancelled'));
+        }
     }
 
     public function resolve($value)
     {
-        if ($this->state !== 'pending') {
+        if ($this->state !== self::PENDING) {
             throw new \RuntimeException("Cannot resolve a {$this->state} promise");
         }
 
-        $this->state = 'fulfilled';
+        $this->state = self::FULFILLED;
         $this->result = $value;
         $this->cancelFn = $this->waitFn = null;
 
@@ -173,11 +165,11 @@ public function resolve($value)
 
     public function reject($reason)
     {
-        if ($this->state !== 'pending') {
+        if ($this->state !== self::PENDING) {
             throw new \RuntimeException("Cannot reject a {$this->state} promise");
         }
 
-        $this->state = 'rejected';
+        $this->state = self::REJECTED;
         $this->result = $reason;
         $this->cancelFn = $this->waitFn = null;
 
@@ -196,7 +188,7 @@ private function deliver($value)
         $pending = [
             [
                 'value'    => $value,
-                'index'    => $this->state === 'fulfilled' ? 1 : 2,
+                'index'    => $this->state === self::FULFILLED ? 1 : 2,
                 'handlers' => $this->handlers
             ]
         ];
@@ -319,28 +311,25 @@ private function resolveForwardPromise(array $group)
         /** @var Promise $promise */
         $promise = $group['value'];
         $handlers = $group['handlers'];
-
-        switch ($promise->getState()) {
-            case 'pending':
-                // The promise is an instance of Promise, so merge in the
-                // dependent handlers into the promise.
-                $promise->handlers = array_merge($promise->handlers, $handlers);
-                return null;
-            case 'fulfilled':
-                return [
-                    'value'    => $promise->result,
-                    'handlers' => $handlers,
-                    'index'    => 1
-                ];
-            case 'rejected':
-                return [
-                    'value'    => $promise->result,
-                    'handlers' => $handlers,
-                    'index'    => 2
-                ];
+        $state = $promise->getState();
+        if ($state === self::PENDING) {
+            // The promise is an instance of Promise, so merge in the
+            // dependent handlers into the promise.
+            $promise->handlers = array_merge($promise->handlers, $handlers);
+            return null;
+        } elseif ($state === self::FULFILLED) {
+            return [
+                'value'    => $promise->result,
+                'handlers' => $handlers,
+                'index'    => 1
+            ];
+        } else { // rejected
+            return [
+                'value'    => $promise->result,
+                'handlers' => $handlers,
+                'index'    => 2
+            ];
         }
-
-        return null;
     }
 
     /**
diff --git a/src/PromiseInterface.php b/src/PromiseInterface.php
index 23513f6..a8d2e6c 100644
--- a/src/PromiseInterface.php
+++ b/src/PromiseInterface.php
@@ -2,10 +2,20 @@
 namespace GuzzleHttp\Promise;
 
 /**
- * Represents the eventual outcome of a deferred.
+ * A promise represents the eventual result of an asynchronous operation.
+ *
+ * The primary way of interacting with a promise is through its then method,
+ * which registers callbacks to receive either a promise’s eventual value or
+ * the reason why the promise cannot be fulfilled.
+ *
+ * @link https://promisesaplus.com/
  */
 interface PromiseInterface
 {
+    const PENDING = 'pending';
+    const FULFILLED = 'fulfilled';
+    const REJECTED = 'rejected';
+
     /**
      * Create a new promise that chains off of the current promise.
      *
@@ -20,9 +30,10 @@ public function then(
     );
 
     /**
-     * Get the state of the promise.
+     * Get the state of the promise ("pending", "rejected", or "fulfilled").
      *
-     * State can be one of: pending, fulfilled, rejected, or cancelled.
+     * The three states can be checked against the constants defined on
+     * PromiseInterface: PENDING, FULFILLED, and REJECTED.
      *
      * @return string
      */
@@ -32,6 +43,7 @@ public function getState();
      * Resolve the promise with the given value.
      *
      * @param mixed $value
+     * @throws \RuntimeException if the promise is already resolved.
      */
     public function resolve($value);
 
@@ -39,11 +51,14 @@ public function resolve($value);
      * Reject the promise with the given reason.
      *
      * @param mixed $reason
+     * @throws \RuntimeException if the promise is already resolved.
      */
     public function reject($reason);
 
     /**
      * Cancels the promise if possible.
+     *
+     * @link https://github.com/promises-aplus/cancellation-spec/issues/7
      */
     public function cancel();
 
diff --git a/src/RejectedPromise.php b/src/RejectedPromise.php
index 6c30eb2..621a554 100644
--- a/src/RejectedPromise.php
+++ b/src/RejectedPromise.php
@@ -50,17 +50,17 @@ public function wait($unwrap = true, $defaultDelivery = null)
 
     public function getState()
     {
-        return 'rejected';
+        return self::REJECTED;
     }
 
     public function resolve($value)
     {
-        // pass
+        throw new \RuntimeException("Cannot resolve a rejected promise");
     }
 
     public function reject($reason)
     {
-        // pass
+        throw new \RuntimeException("Cannot reject a rejected promise");
     }
 
     public function cancel()
diff --git a/tests/FulfilledPromiseTest.php b/tests/FulfilledPromiseTest.php
index 3633de1..185e166 100644
--- a/tests/FulfilledPromiseTest.php
+++ b/tests/FulfilledPromiseTest.php
@@ -9,16 +9,41 @@
  */
 class FulfilledPromiseTest extends \PHPUnit_Framework_TestCase
 {
-    public function testCannotModifyState()
+    public function testReturnsValueWhenWaitedUpon()
     {
         $p = new FulfilledPromise('foo');
         $this->assertEquals('fulfilled', $p->getState());
-        $p->resolve('bar');
-        $p->cancel();
-        $p->reject('baz');
         $this->assertEquals('foo', $p->wait(true));
     }
 
+    public function testCannotCancel()
+    {
+        $p = new FulfilledPromise('foo');
+        $this->assertEquals('fulfilled', $p->getState());
+        $p->cancel();
+        $this->assertEquals('foo', $p->wait());
+    }
+
+    /**
+     * @expectedException \RuntimeException
+     * @exepctedExceptionMessage Cannot resolve a fulfilled promise
+     */
+    public function testCannotResolve()
+    {
+        $p = new FulfilledPromise('foo');
+        $p->resolve('bar');
+    }
+
+    /**
+     * @expectedException \RuntimeException
+     * @exepctedExceptionMessage Cannot reject a fulfilled promise
+     */
+    public function testCannotReject()
+    {
+        $p = new FulfilledPromise('foo');
+        $p->reject('bar');
+    }
+
     /**
      * @expectedException \InvalidArgumentException
      */
diff --git a/tests/PromiseTest.php b/tests/PromiseTest.php
index a829b50..c74befc 100644
--- a/tests/PromiseTest.php
+++ b/tests/PromiseTest.php
@@ -1,6 +1,7 @@
 <?php
 namespace GuzzleHttp\Tests;
 
+use GuzzleHttp\Promise\CancellationException;
 use GuzzleHttp\Promise\Promise;
 use GuzzleHttp\Promise\RejectedPromise;
 
@@ -129,24 +130,71 @@ public function testCannotCancelNonPending()
         $this->assertEquals('fulfilled', $p->getState());
     }
 
+    /**
+     * @expectedException \GuzzleHttp\Promise\CancellationException
+     */
     public function testCancelsPromiseWhenNoCancelFunction()
     {
         $p = new Promise();
         $p->cancel();
-        $this->assertEquals('cancelled', $p->getState());
+        $this->assertEquals('rejected', $p->getState());
+        $p->wait();
     }
 
     public function testCancelsPromiseWithCancelFunction()
     {
         $called = false;
-        $p = new Promise(null, function () use (&$called) {
-            $called = true;
-        });
+        $p = new Promise(null, function () use (&$called) { $called = true; });
         $p->cancel();
-        $this->assertEquals('cancelled', $p->getState());
+        $this->assertEquals('rejected', $p->getState());
         $this->assertTrue($called);
     }
 
+    public function testCancelsUppermostPendingPromise()
+    {
+        $called = false;
+        $p1 = new Promise(null, function () use (&$called) { $called = true; });
+        $p2 = $p1->then(function () {});
+        $p3 = $p2->then(function () {});
+        $p4 = $p3->then(function () {});
+        $p3->cancel();
+        $this->assertEquals('rejected', $p1->getState());
+        $this->assertEquals('rejected', $p2->getState());
+        $this->assertEquals('rejected', $p2->getState());
+        $this->assertEquals('rejected', $p4->getState());
+        $this->assertTrue($called);
+        try {
+            $p3->wait();
+            $this->fail();
+        } catch (CancellationException $e) {
+            $this->assertContains('cancelled', $e->getMessage());
+        }
+        try {
+            $p4->wait();
+            $this->fail();
+        } catch (CancellationException $e) {
+            $this->assertContains('cancelled', $e->getMessage());
+        }
+    }
+
+    public function testCancelsChildPromises()
+    {
+        $called1 = $called2 = $called3 = false;
+        $p1 = new Promise(null, function () use (&$called1) { $called1 = true; });
+        $p2 = new Promise(null, function () use (&$called2) { $called2 = true; });
+        $p3 = new Promise(null, function () use (&$called3) { $called3 = true; });
+        $p4 = $p2->then(function () use ($p3) { return $p3; });
+        $p5 = $p4->then(function () { $this->fail(); });
+        $p4->cancel();
+        $this->assertEquals('pending', $p1->getState());
+        $this->assertEquals('rejected', $p2->getState());
+        $this->assertEquals('rejected', $p4->getState());
+        $this->assertEquals('rejected', $p5->getState());
+        $this->assertFalse($called1);
+        $this->assertTrue($called2);
+        $this->assertFalse($called3);
+    }
+
     public function testRejectsPromiseWhenCancelFails()
     {
         $called = false;
diff --git a/tests/RejectedPromiseTest.php b/tests/RejectedPromiseTest.php
index eab4ad5..fcfae57 100644
--- a/tests/RejectedPromiseTest.php
+++ b/tests/RejectedPromiseTest.php
@@ -9,13 +9,10 @@
  */
 class RejectedPromiseTest extends \PHPUnit_Framework_TestCase
 {
-    public function testCannotModifyState()
+    public function testThrowsReasonWhenWaitedUpon()
     {
         $p = new RejectedPromise('foo');
         $this->assertEquals('rejected', $p->getState());
-        $p->resolve('bar');
-        $p->cancel();
-        $p->reject('baz');
         try {
             $p->wait(true);
             $this->fail();
@@ -25,6 +22,33 @@ public function testCannotModifyState()
         }
     }
 
+    public function testCannotCancel()
+    {
+        $p = new RejectedPromise('foo');
+        $p->cancel();
+        $this->assertEquals('rejected', $p->getState());
+    }
+
+    /**
+     * @expectedException \RuntimeException
+     * @exepctedExceptionMessage Cannot resolve a rejected promise
+     */
+    public function testCannotResolve()
+    {
+        $p = new RejectedPromise('foo');
+        $p->resolve('bar');
+    }
+
+    /**
+     * @expectedException \RuntimeException
+     * @exepctedExceptionMessage Cannot reject a rejected promise
+     */
+    public function testCannotReject()
+    {
+        $p = new RejectedPromise('foo');
+        $p->reject('bar');
+    }
+
     public function testThrowsSpecificException()
     {
         $e = new \Exception();