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 promises-aplus/cancellation-spec#7 - Throwing exceptions when you try to fulfill/reject FulfilledPromise and RejectedPromise.
guzzle · Mar 8, 2015 · aabcf7c · aabcf7c
1 parent e314897
commit aabcf7c
Show file tree

Hide file tree

Showing 8 changed files with 174 additions and 67 deletions.
diff --git a/src/CancellationException.php 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
@@ -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
@@ -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
@@ -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,18 +43,22 @@ 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);
 
     /**
      * 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
@@ -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
@@ -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
      */