From dd669c6d4eca9661dae94a4fca33d53e771bbea1 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 11 Dec 2012 17:27:25 -0800
Subject: [PATCH] Make AphrontProxyResponse reduce to a real response instead
 of building a string

Summary: Currently, AphrontProxyResponse is expected to build a string. This prevents some response types (like Dialog) from being proxied, because they have special rules. Instead, make proxy responses reduce into a non-proxied response so it's possible to proxy any type of response and hit all the normal rules for it.

Test Plan: Built a proxied DialogResponse on top of this.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2104, T912

Differential Revision: https://secure.phabricator.com/D4159
---
 src/aphront/response/AphrontProxyResponse.php    | 11 +++++++++--
 .../base/controller/PhabricatorController.php    | 16 ++++++++++++++++
 ...PhabricatorApplicationTransactionResponse.php |  7 ++-----
 .../diff/PhabricatorChangesetResponse.php        |  4 ++--
 4 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/src/aphront/response/AphrontProxyResponse.php b/src/aphront/response/AphrontProxyResponse.php
index a644c45a9b..8e031a977c 100644
--- a/src/aphront/response/AphrontProxyResponse.php
+++ b/src/aphront/response/AphrontProxyResponse.php
@@ -5,8 +5,8 @@
  * a response might be substantially an Ajax response, but add structure to the
  * response content. It can do this by extending @{class:AphrontProxyResponse},
  * instantiating an @{class:AphrontAjaxResponse} in @{method:buildProxy}, and
- * then using the proxy to construct the response string in
- * @{method:buildResponseString}.
+ * then constructing a real @{class:AphrontAjaxResponse} in
+ * @{method:reduceProxyResponse}.
  *
  * @group aphront
  */
@@ -63,5 +63,12 @@ abstract class AphrontProxyResponse extends AphrontResponse {
   }
 
   abstract protected function buildProxy();
+  abstract public function reduceProxyResponse();
+
+  final public function buildResponseString() {
+    throw new Exception(
+      "AphrontProxyResponse must implement reduceProxyResponse().");
+  }
+
 
 }
diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php
index 39ca6fdfde..297ea6502e 100644
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -181,6 +181,22 @@ abstract class PhabricatorController extends AphrontController {
   public function didProcessRequest($response) {
     $request = $this->getRequest();
     $response->setRequest($request);
+
+    $seen = array();
+    while ($response instanceof AphrontProxyResponse) {
+
+      $hash = spl_object_hash($response);
+      if (isset($seen[$hash])) {
+        $seen[] = get_class($response);
+        throw new Exception(
+          "Cycle while reducing proxy responses: ".
+          implode(' -> ', $seen));
+      }
+      $seen[$hash] = get_class($response);
+
+      $response = $response->reduceProxyResponse();
+    }
+
     if ($response instanceof AphrontDialogResponse) {
       if (!$request->isAjax()) {
         $view = new PhabricatorStandardPageView();
diff --git a/src/applications/transactions/response/PhabricatorApplicationTransactionResponse.php b/src/applications/transactions/response/PhabricatorApplicationTransactionResponse.php
index a0ff24345f..cc5eede13d 100644
--- a/src/applications/transactions/response/PhabricatorApplicationTransactionResponse.php
+++ b/src/applications/transactions/response/PhabricatorApplicationTransactionResponse.php
@@ -40,7 +40,7 @@ final class PhabricatorApplicationTransactionResponse
     return $this->viewer;
   }
 
-  public function buildResponseString() {
+  public function reduceProxyResponse() {
     $view = id(new PhabricatorApplicationTransactionView())
       ->setViewer($this->getViewer())
       ->setTransactions($this->getTransactions());
@@ -56,10 +56,7 @@ final class PhabricatorApplicationTransactionResponse
       'spacer'   => PhabricatorTimelineView::renderSpacer(),
     );
 
-    return $this
-      ->getProxy()
-      ->setContent($content)
-      ->buildResponseString();
+    return $this->getProxy()->setContent($content);
   }
 
 }
diff --git a/src/infrastructure/diff/PhabricatorChangesetResponse.php b/src/infrastructure/diff/PhabricatorChangesetResponse.php
index 86f4f3dccf..b4897eb18e 100644
--- a/src/infrastructure/diff/PhabricatorChangesetResponse.php
+++ b/src/infrastructure/diff/PhabricatorChangesetResponse.php
@@ -19,7 +19,7 @@ final class PhabricatorChangesetResponse extends AphrontProxyResponse {
     return new AphrontAjaxResponse();
   }
 
-  public function buildResponseString() {
+  public function reduceProxyResponse() {
     $content = array(
       'changeset' => $this->renderedChangeset,
     );
@@ -28,7 +28,7 @@ final class PhabricatorChangesetResponse extends AphrontProxyResponse {
       $content['coverage'] = $this->coverage;
     }
 
-    return $this->getProxy()->setContent($content)->buildResponseString();
+    return $this->getProxy()->setContent($content);
   }
 
 }