Page MenuHomePhabricator
Files F14023826

D14032.id33926.diff
No OneTemporary
Actions

D14032.id33926.diff
View Options

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -8,7 +8,7 @@
return array(
'names' => array(
'core.pkg.css' => '928faf7e',
- 'core.pkg.js' => 'a590b451',
+ 'core.pkg.js' => '47dc9ebb',
'darkconsole.pkg.js' => 'e7393ebb',
'differential.pkg.css' => '2de124c9',
'differential.pkg.js' => '813c1633',
@@ -210,7 +210,7 @@
'rsrc/externals/javelin/ext/view/__tests__/ViewInterpreter.js' => '7a94d6a5',
'rsrc/externals/javelin/ext/view/__tests__/ViewRenderer.js' => '6ea96ac9',
'rsrc/externals/javelin/lib/Cookie.js' => '62dfea03',
- 'rsrc/externals/javelin/lib/DOM.js' => '147805fa',
+ 'rsrc/externals/javelin/lib/DOM.js' => '805b806a',
'rsrc/externals/javelin/lib/History.js' => 'd4505101',
'rsrc/externals/javelin/lib/JSON.js' => '69adf288',
'rsrc/externals/javelin/lib/Leader.js' => '331b1611',
@@ -659,7 +659,7 @@
'javelin-color' => '7e41274a',
'javelin-cookie' => '62dfea03',
'javelin-diffusion-locate-file-source' => 'b42eddc7',
- 'javelin-dom' => '147805fa',
+ 'javelin-dom' => '805b806a',
'javelin-dynval' => 'f6555212',
'javelin-event' => '85ea0626',
'javelin-fx' => '54b612ba',
@@ -918,13 +918,6 @@
'javelin-uri',
'phabricator-textareautils',
),
- '147805fa' => array(
- 'javelin-magical-init',
- 'javelin-install',
- 'javelin-util',
- 'javelin-vector',
- 'javelin-stratcom',
- ),
'1499a8cb' => array(
'javelin-behavior',
'javelin-stratcom',
@@ -1451,6 +1444,13 @@
'javelin-behavior',
'javelin-history',
),
+ '805b806a' => array(
+ 'javelin-magical-init',
+ 'javelin-install',
+ 'javelin-util',
+ 'javelin-vector',
+ 'javelin-stratcom',
+ ),
82439934 => array(
'javelin-behavior',
'javelin-dom',
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -158,6 +158,7 @@
'AphrontRequest' => 'aphront/AphrontRequest.php',
'AphrontRequestTestCase' => 'aphront/__tests__/AphrontRequestTestCase.php',
'AphrontResponse' => 'aphront/response/AphrontResponse.php',
+ 'AphrontResponseProducerInterface' => 'aphront/interface/AphrontResponseProducerInterface.php',
'AphrontRoutingMap' => 'aphront/site/AphrontRoutingMap.php',
'AphrontRoutingResult' => 'aphront/site/AphrontRoutingResult.php',
'AphrontSideNavFilterView' => 'view/layout/AphrontSideNavFilterView.php',
@@ -3732,7 +3733,10 @@
'AphrontCursorPagerView' => 'AphrontView',
'AphrontDefaultApplicationConfiguration' => 'AphrontApplicationConfiguration',
'AphrontDialogResponse' => 'AphrontResponse',
- 'AphrontDialogView' => 'AphrontView',
+ 'AphrontDialogView' => array(
+ 'AphrontView',
+ 'AphrontResponseProducerInterface',
+ ),
'AphrontException' => 'Exception',
'AphrontFileResponse' => 'AphrontResponse',
'AphrontFormCheckboxControl' => 'AphrontFormControl',
@@ -3775,7 +3779,10 @@
'AphrontPageView' => 'AphrontView',
'AphrontPlainTextResponse' => 'AphrontResponse',
'AphrontProgressBarView' => 'AphrontBarView',
- 'AphrontProxyResponse' => 'AphrontResponse',
+ 'AphrontProxyResponse' => array(
+ 'AphrontResponse',
+ 'AphrontResponseProducerInterface',
+ ),
'AphrontRedirectResponse' => 'AphrontResponse',
'AphrontRedirectResponseTestCase' => 'PhabricatorTestCase',
'AphrontReloadResponse' => 'AphrontRedirectResponse',
diff --git a/src/aphront/AphrontController.php b/src/aphront/AphrontController.php
--- a/src/aphront/AphrontController.php
+++ b/src/aphront/AphrontController.php
@@ -24,10 +24,6 @@
return;
}
- public function didProcessRequest($response) {
- return $response;
- }
-
public function handleRequest(AphrontRequest $request) {
if (method_exists($this, 'processRequest')) {
return $this->processRequest();
diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php
--- a/src/aphront/configuration/AphrontApplicationConfiguration.php
+++ b/src/aphront/configuration/AphrontApplicationConfiguration.php
@@ -1,7 +1,8 @@
<?php
/**
- * @task routing URI Routing
+ * @task routing URI Routing
+ * @task response Response Handling
*/
abstract class AphrontApplicationConfiguration extends Phobject {
@@ -234,6 +235,7 @@
if (!$response) {
$controller->willProcessRequest($uri_data);
$response = $controller->handleRequest($request);
+ $this->validateControllerResponse($controller, $response);
}
} catch (Exception $ex) {
$original_exception = $ex;
@@ -241,8 +243,8 @@
}
try {
- $response = $controller->didProcessRequest($response);
- $response = $this->willSendResponse($response, $controller);
+ $response = $this->produceResponse($request, $response);
+ $response = $controller->willSendResponse($response);
$response->setRequest($request);
$unexpected_output = PhabricatorStartup::endOutputCapture();
@@ -424,4 +426,148 @@
return $site;
}
+
+/* -( Response Handling )-------------------------------------------------- */
+
+
+ /**
+ * Tests if a response is of a valid type.
+ *
+ * @param wild Supposedly valid response.
+ * @return bool True if the object is of a valid type.
+ * @task response
+ */
+ private function isValidResponseObject($response) {
+ if ($response instanceof AphrontResponse) {
+ return true;
+ }
+
+ if ($response instanceof AphrontResponseProducerInterface) {
+ return true;
+ }
+
+ return false;
+ }
+
+
+ /**
+ * Verifies that the return value from an @{class:AphrontController} is
+ * of an allowed type.
+ *
+ * @param AphrontController Controller which returned the response.
+ * @param wild Supposedly valid response.
+ * @return void
+ * @task response
+ */
+ private function validateControllerResponse(
+ AphrontController $controller,
+ $response) {
+
+ if ($this->isValidResponseObject($response)) {
+ return;
+ }
+
+ throw new Exception(
+ pht(
+ 'Controller "%s" returned an invalid response from call to "%s". '.
+ 'This method must return an object of class "%s", or an object '.
+ 'which implements the "%s" interface.',
+ get_class($controller),
+ 'handleRequest()',
+ 'AphrontResponse',
+ 'AphrontResponseProducerInterface'));
+ }
+
+
+ /**
+ * Verifies that the erturn value from an
+ * @{class:AphrontResponseProducerInterface} is of an allowed type.
+ *
+ * @param AphrontResponseProducerInterface Object which produced
+ * this response.
+ * @param wild Supposedly valid response.
+ * @return void
+ * @task response
+ */
+ private function validateProducerResponse(
+ AphrontResponseProducerInterface $producer,
+ $response) {
+
+ if ($this->isValidResponseObject($response)) {
+ return;
+ }
+
+ throw new Exception(
+ pht(
+ 'Producer "%s" returned an invalid response from call to "%s". '.
+ 'This method must return an object of class "%s", or an object '.
+ 'which implements the "%s" interface.',
+ get_class($producer),
+ 'produceAphrontResponse()',
+ 'AphrontResponse',
+ 'AphrontResponseProducerInterface'));
+ }
+
+
+ /**
+ * Resolves a response object into an @{class:AphrontResponse}.
+ *
+ * Controllers are permitted to return actual responses of class
+ * @{class:AphrontResponse}, or other objects which implement
+ * @{interface:AphrontResponseProducerInterface} and can produce a response.
+ *
+ * If a controller returns a response producer, invoke it now and produce
+ * the real response.
+ *
+ * @param AphrontRequest Request being handled.
+ * @param AphrontResponse|AphrontResponseProducerInterface Response, or
+ * response producer.
+ * @return AphrontResponse Response after any required production.
+ * @task response
+ */
+ private function produceResponse(AphrontRequest $request, $response) {
+ $original = $response;
+
+ // Detect cycles on the exact same objects. It's still possible to produce
+ // infinite responses as long as they're all unique, but we can only
+ // reasonably detect cycles, not guarantee that response production halts.
+
+ $seen = array();
+ while (true) {
+ // NOTE: It is permissible for an object to be both a response and a
+ // response producer. If so, being a producer is "stronger". This is
+ // used by AphrontProxyResponse.
+
+ // If this response is a valid response, hand over the request first.
+ if ($response instanceof AphrontResponse) {
+ $response->setRequest($request);
+ }
+
+ // If this isn't a producer, we're all done.
+ if (!($response instanceof AphrontResponseProducerInterface)) {
+ break;
+ }
+
+ $hash = spl_object_hash($response);
+ if (isset($seen[$hash])) {
+ throw new Exception(
+ pht(
+ 'Failure while producing response for object of class "%s": '.
+ 'encountered production cycle (identical object, of class "%s", '.
+ 'was produced twice).',
+ get_class($original),
+ get_class($response)));
+ }
+
+ $seen[$hash] = true;
+
+ $new_response = $response->produceAphrontResponse();
+ $this->validateProducerResponse($response, $new_response);
+ $response = $new_response;
+ }
+
+ return $response;
+ }
+
+
}
diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php
--- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php
+++ b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php
@@ -273,10 +273,6 @@
return $response;
}
- public function willSendResponse(AphrontResponse $response) {
- return $response;
- }
-
public function build404Controller() {
return array(new Phabricator404Controller(), array());
}
diff --git a/src/aphront/interface/AphrontResponseProducerInterface.php b/src/aphront/interface/AphrontResponseProducerInterface.php
new file mode 100644
--- /dev/null
+++ b/src/aphront/interface/AphrontResponseProducerInterface.php
@@ -0,0 +1,24 @@
+<?php
+
+/**
+ * An object can implement this interface to allow it to be returned directly
+ * from an @{class:AphrontController}.
+ *
+ * Normally, controllers must return an @{class:AphrontResponse}. Sometimes,
+ * this is not convenient or requires an awkward API. If it's preferable to
+ * return some other type of object which is equivalent to or describes a
+ * valid response, that object can implement this interface and produce a
+ * response later.
+ */
+interface AphrontResponseProducerInterface {
+
+
+ /**
+ * Produce the equivalent @{class:AphrontResponse} for this object.
+ *
+ * @return AphrontResponse Equivalent response.
+ */
+ public function produceAphrontResponse();
+
+
+}
diff --git a/src/aphront/response/AphrontProxyResponse.php b/src/aphront/response/AphrontProxyResponse.php
--- a/src/aphront/response/AphrontProxyResponse.php
+++ b/src/aphront/response/AphrontProxyResponse.php
@@ -8,7 +8,9 @@
* then constructing a real @{class:AphrontAjaxResponse} in
* @{method:reduceProxyResponse}.
*/
-abstract class AphrontProxyResponse extends AphrontResponse {
+abstract class AphrontProxyResponse
+ extends AphrontResponse
+ implements AphrontResponseProducerInterface {
private $proxy;
@@ -71,4 +73,12 @@
'reduceProxyResponse()'));
}
+
+/* -( AphrontResponseProducerInterface )----------------------------------- */
+
+
+ public function produceAphrontResponse() {
+ return $this->reduceProxyResponse();
+ }
+
}
diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -370,28 +370,8 @@
return $this->buildPageResponse($page);
}
- public function didProcessRequest($response) {
- // If a bare DialogView is returned, wrap it in a DialogResponse.
- if ($response instanceof AphrontDialogView) {
- $response = id(new AphrontDialogResponse())->setDialog($response);
- }
-
+ public function willSendResponse(AphrontResponse $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(
- pht('Cycle while reducing proxy responses: %s',
- implode(' -> ', $seen)));
- }
- $seen[$hash] = get_class($response);
-
- $response = $response->reduceProxyResponse();
- }
if ($response instanceof AphrontDialogResponse) {
if (!$request->isAjax() && !$request->isQuicksand()) {
diff --git a/src/view/AphrontDialogView.php b/src/view/AphrontDialogView.php
--- a/src/view/AphrontDialogView.php
+++ b/src/view/AphrontDialogView.php
@@ -1,6 +1,8 @@
<?php
-final class AphrontDialogView extends AphrontView {
+final class AphrontDialogView
+ extends AphrontView
+ implements AphrontResponseProducerInterface {
private $title;
private $shortTitle;
@@ -371,4 +373,13 @@
}
}
+
+/* -( AphrontResponseProducerInterface )----------------------------------- */
+
+
+ public function produceAphrontResponse() {
+ return id(new AphrontDialogResponse())
+ ->setDialog($this);
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Fri, Nov 8, 3:49 AM (1 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6762090
Default Alt Text
D14032.id33926.diff (13 KB)

Event Timeline

Phabricator · Built by Phacility