Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13218866
D14032.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
11 KB
Referenced Files
None
Subscribers
None
D14032.diff
View Options
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
Details
Attached
Mime Type
text/plain
Expires
Sun, May 19, 1:20 PM (3 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6289664
Default Alt Text
D14032.diff (11 KB)
Attached To
Mode
D14032: Allow Controllers to return a wider range of "response-like" objects
Attached
Detach File
Event Timeline
Log In to Comment