Make the "result" property on Future private

Summary: Depends on D21033. Ref T11968. This is just an incremental step in modernizing Future and making it more robust. Currently, subclasses are expected to write directly to `$this->result`, but this isn't consistent with how modern classes generally work.

Test Plan: Ran unit tests, created this revision.

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21034

src/future/Future.php

@@ -7,7 +7,9 @@
  */
 abstract class Future extends Phobject {
 
-  protected $result;
+  private $hasResult = false;
+  private $result;
+
   protected $exception;
 
   /**
@@ -79,22 +81,6 @@ abstract class Future extends Phobject {
   }
 
 
-  /**
-   * Retrieve the final result of the future. This method will be called after
-   * the future is ready (as per @{method:isReady}) but before results are
-   * passed back to the caller. The major use of this function is that you can
-   * override it in subclasses to do postprocessing or error checking, which is
-   * particularly useful if building application-specific futures on top of
-   * primitive transport futures (like @{class:CurlFuture} and
-   * @{class:ExecFuture}) which can make it tricky to hook this logic into the
-   * main pipeline.
-   *
-   * @return mixed   Final resolution of this future.
-   */
-  protected function getResult() {
-    return $this->result;
-  }
-
   /**
    * Default amount of time to wait on stream select for this future. Normally
    * 1 second is fine, but if the future has a timeout sooner than that it
@@ -109,4 +95,38 @@ abstract class Future extends Phobject {
     return $this;
   }
 
+  /**
+   * Retrieve the final result of the future.
+   *
+   * @return wild Final resolution of this future.
+   */
+  final protected function getResult() {
+    if (!$this->hasResult()) {
+      throw new Exception(
+        pht(
+          'Future has not yet resolved. Resolve futures before retrieving '.
+          'results.'));
+    }
+
+    return $this->result;
+  }
+
+  final protected function setResult($result) {
+    if ($this->hasResult()) {
+      throw new Exception(
+        pht(
+          'Future has already resolved. Futures may not resolve more than '.
+          'once.'));
+    }
+
+    $this->hasResult = true;
+    $this->result = $result;
+
+    return $this;
+  }
+
+  final protected function hasResult() {
+    return $this->hasResult;
+  }
+
 }

src/future/FutureProxy.php

@@ -31,7 +31,9 @@ abstract class FutureProxy extends Future {
   }
 
   public function resolve() {
-    $this->getProxiedFuture()->resolve();
+    $result = $this->getProxiedFuture()->resolve();
+    $result = $this->didReceiveResult($result);
+    $this->setResult($result);
     return $this->getResult();
   }
 
@@ -52,15 +54,6 @@ abstract class FutureProxy extends Future {
     return $this->getProxiedFuture()->getWriteSockets();
   }
 
-  protected function getResult() {
-    if ($this->result === null) {
-      $result = $this->getProxiedFuture()->resolve();
-      $result = $this->didReceiveResult($result);
-      $this->result = $result;
-    }
-    return $this->result;
-  }
-
   public function start() {
     $this->getProxiedFuture()->start();
     return $this;

src/future/ImmediateFuture.php

@@ -7,7 +7,7 @@
 final class ImmediateFuture extends Future {
 
   public function __construct($result) {
-    $this->result = $result;
+    $this->setResult($result);
   }
 
   public function isReady() {

src/future/exec/ExecFuture.php

@@ -381,22 +381,25 @@ final class ExecFuture extends PhutilExecutableFuture {
    * @task resolve
    */
   public function resolveKill() {
-    if (!$this->result) {
+    if (!$this->hasResult()) {
       $signal = 9;
 
       if ($this->proc) {
         proc_terminate($this->proc, $signal);
       }
 
-      $this->result = array(
+      $result = array(
         128 + $signal,
         $this->stdout,
         $this->stderr,
       );
+
+      $this->setResult($result);
+
       $this->closeProcess();
     }
 
-    return $this->result;
+    return $this->getResult();
   }
 
 
@@ -764,11 +767,14 @@ final class ExecFuture extends PhutilExecutableFuture {
         }
       }
 
-      $this->result = array(
+      $result = array(
         $err,
         $this->stdout,
         $this->stderr,
       );
+
+      $this->setResult($result);
+
       $this->closeProcess();
       return true;
     }
@@ -839,11 +845,18 @@ final class ExecFuture extends PhutilExecutableFuture {
     unset($this->windowsStderrTempFile);
 
     if ($this->profilerCallID !== null) {
+      if ($this->hasResult()) {
+        $result = $this->getResult();
+        $err = idx($result, 0);
+      } else {
+        $err = null;
+      }
+
       $profiler = PhutilServiceProfiler::getInstance();
       $profiler->endServiceCall(
         $this->profilerCallID,
         array(
-          'err' => $this->result ? idx($this->result, 0) : null,
+          'err' => $err,
         ));
       $this->profilerCallID = null;
     }

src/future/exec/PhutilExecPassthru.php

@@ -20,8 +20,6 @@
  */
 final class PhutilExecPassthru extends PhutilExecutableFuture {
 
-  private $passthruResult;
-
 
 /* -(  Executing Passthru Commands  )---------------------------------------- */
 
@@ -105,15 +103,12 @@ final class PhutilExecPassthru extends PhutilExecutableFuture {
     // full control of the console. We're just implementing the interfaces to
     // make it easier to share code with ExecFuture.
 
-    if ($this->passthruResult === null) {
+    if (!$this->hasResult()) {
-      $this->passthruResult = $this->execute();
+      $result = $this->execute();
+      $this->setResult($result);
     }
 
     return true;
   }
 
-  protected function getResult() {
-    return $this->passthruResult;
-  }
-
 }

src/future/http/HTTPFuture.php

@@ -180,8 +180,9 @@ final class HTTPFuture extends BaseHTTPFuture {
 
     if (!$socket) {
       $this->stateReady = true;
-      $this->result = $this->buildErrorResult(
+      $this->setResult(
-        HTTPFutureTransportResponseStatus::ERROR_CONNECTION_FAILED);
+        $this->buildErrorResult(
+          HTTPFutureTransportResponseStatus::ERROR_CONNECTION_FAILED));
       return null;
     }
 
@@ -209,16 +210,19 @@ final class HTTPFuture extends BaseHTTPFuture {
     $this->stateReady = true;
 
     if ($timeout) {
-      $this->result = $this->buildErrorResult(
+      $this->setResult(
-        HTTPFutureTransportResponseStatus::ERROR_TIMEOUT);
+        $this->buildErrorResult(
+          HTTPFutureTransportResponseStatus::ERROR_TIMEOUT));
     } else if (!$this->stateConnected) {
-      $this->result = $this->buildErrorResult(
+      $this->setResult(
-        HTTPFutureTransportResponseStatus::ERROR_CONNECTION_REFUSED);
+        $this->buildErrorResult(
+          HTTPFutureTransportResponseStatus::ERROR_CONNECTION_REFUSED));
     } else if (!$this->stateWriteComplete) {
-      $this->result = $this->buildErrorResult(
+      $this->setResult(
-        HTTPFutureTransportResponseStatus::ERROR_CONNECTION_FAILED);
+        $this->buildErrorResult(
+          HTTPFutureTransportResponseStatus::ERROR_CONNECTION_FAILED));
     } else {
-      $this->result = $this->parseRawHTTPResponse($this->response);
+      $this->setResult($this->parseRawHTTPResponse($this->response));
     }
 
     $profiler = PhutilServiceProfiler::getInstance();

src/future/http/HTTPSFuture.php

@@ -194,7 +194,7 @@ final class HTTPSFuture extends BaseHTTPFuture {
   }
 
   public function isReady() {
-    if (isset($this->result)) {
+    if ($this->hasResult()) {
       return true;
     }
 
@@ -476,7 +476,7 @@ final class HTTPSFuture extends BaseHTTPFuture {
 
       $body = null;
       $headers = array();
-      $this->result = array($status, $body, $headers);
+      $this->setResult(array($status, $body, $headers));
     } else if ($this->parser) {
       $streaming_parser = $this->parser;
       try {
@@ -491,12 +491,12 @@ final class HTTPSFuture extends BaseHTTPFuture {
         $result = array($ex, null, array());
       }
 
-      $this->result = $result;
+      $this->setResult($result);
     } else {
       // cURL returns headers of all redirects, we strip all but the final one.
       $redirects = curl_getinfo($curl, CURLINFO_REDIRECT_COUNT);
       $result = preg_replace('/^(.*\r\n\r\n){'.$redirects.'}/sU', '', $result);
-      $this->result = $this->parseRawHTTPResponse($result);
+      $this->setResult($this->parseRawHTTPResponse($result));
     }
 
     curl_multi_remove_handle(self::$multi, $curl);
@@ -518,7 +518,7 @@ final class HTTPSFuture extends BaseHTTPFuture {
 
     $sink = $this->getProgressSink();
     if ($sink) {
-      $status = head($this->result);
+      $status = head($this->getResult());
       if ($status->isError()) {
         $sink->didFailWork();
       } else {