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 @@ 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,149 @@ 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. + * @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) { + phlog(get_class($response)); + + // 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 @@ +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 @@ setDialog($this); + } + }