diff --git a/src/applications/conduit/method/ConduitAPIMethod.php b/src/applications/conduit/method/ConduitAPIMethod.php --- a/src/applications/conduit/method/ConduitAPIMethod.php +++ b/src/applications/conduit/method/ConduitAPIMethod.php @@ -120,9 +120,21 @@ public function executeMethod(ConduitAPIRequest $request) { $this->setViewer($request->getUser()); + $client = $this->newConduitCallProxyClient($request); + if ($client) { + // We're proxying, so just make an intracluster call. + return $client->callMethodSynchronous( + $this->getAPIMethodName(), + $request->getAllParameters()); + } + return $this->execute($request); } + protected function newConduitCallProxyClient(ConduitAPIRequest $request) { + return null; + } + abstract public function getAPIMethodName(); /** diff --git a/src/applications/conduit/protocol/ConduitAPIRequest.php b/src/applications/conduit/protocol/ConduitAPIRequest.php --- a/src/applications/conduit/protocol/ConduitAPIRequest.php +++ b/src/applications/conduit/protocol/ConduitAPIRequest.php @@ -51,6 +51,10 @@ return $this->user; } + public function getViewer() { + return $this->getUser(); + } + public function setOAuthToken( PhabricatorOAuthServerAccessToken $oauth_token) { $this->oauthToken = $oauth_token; diff --git a/src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php --- a/src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php @@ -85,6 +85,35 @@ * should occur after @{method:getResult}, like formatting a timestamp. */ final protected function execute(ConduitAPIRequest $request) { + $drequest = $this->getDiffusionRequest(); + + // We pass this flag on to prevent proxying of any other Conduit calls + // which we need to make in order to respond to this one. Although we + // could safely proxy them, we take a big performance hit in the common + // case, and doing more proxying wouldn't exercise any additional code so + // we wouldn't gain a testability/predictability benefit. + $is_cluster_request = $request->getIsClusterRequest(); + $drequest->setIsClusterRequest($is_cluster_request); + + $viewer = $request->getViewer(); + $repository = $drequest->getRepository(); + + // TODO: Allow web UI queries opt out of this if they don't care about + // fetching the most up-to-date data? Synchronization can be slow, and a + // lot of web reads are probably fine if they're a few seconds out of + // date. + id(new DiffusionRepositoryClusterEngine()) + ->setViewer($viewer) + ->setRepository($repository) + ->synchronizeWorkingCopyBeforeRead(); + + return $this->getResult($request); + } + + + protected function newConduitCallProxyClient(ConduitAPIRequest $request) { + $viewer = $request->getViewer(); + $identifier = $request->getValue('repository'); if ($identifier === null) { $identifier = $request->getValue('callsign'); @@ -92,7 +121,7 @@ $drequest = DiffusionRequest::newFromDictionary( array( - 'user' => $request->getUser(), + 'user' => $viewer, 'repository' => $identifier, 'branch' => $request->getValue('branch'), 'path' => $request->getValue('path'), @@ -106,46 +135,16 @@ $identifier)); } - // Figure out whether we're going to handle this request on this device, - // or proxy it to another node in the cluster. - - // If this is a cluster request and we need to proxy, we'll explode here - // to prevent infinite recursion. - - $is_cluster_request = $request->getIsClusterRequest(); - $viewer = $request->getUser(); - $repository = $drequest->getRepository(); - $client = $repository->newConduitClient( - $viewer, - $is_cluster_request); + + $client = $repository->newConduitClientForRequest($request); if ($client) { - // We're proxying, so just make an intracluster call. - return $client->callMethodSynchronous( - $this->getAPIMethodName(), - $request->getAllParameters()); - } else { - - // We pass this flag on to prevent proxying of any other Conduit calls - // which we need to make in order to respond to this one. Although we - // could safely proxy them, we take a big performance hit in the common - // case, and doing more proxying wouldn't exercise any additional code so - // we wouldn't gain a testability/predictability benefit. - $drequest->setIsClusterRequest($is_cluster_request); - - $this->setDiffusionRequest($drequest); - - // TODO: Allow web UI queries opt out of this if they don't care about - // fetching the most up-to-date data? Synchronization can be slow, and a - // lot of web reads are probably fine if they're a few seconds out of - // date. - id(new DiffusionRepositoryClusterEngine()) - ->setViewer($viewer) - ->setRepository($repository) - ->synchronizeWorkingCopyBeforeRead(); - - return $this->getResult($request); + return $client; } + + $this->setDiffusionRequest($drequest); + + return null; } protected function getResult(ConduitAPIRequest $request) { diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -2241,6 +2241,23 @@ return $client; } + public function newConduitClientForRequest(ConduitAPIRequest $request) { + // Figure out whether we're going to handle this request on this device, + // or proxy it to another node in the cluster. + + // If this is a cluster request and we need to proxy, we'll explode here + // to prevent infinite recursion. + + $viewer = $request->getViewer(); + $is_cluster_request = $request->getIsClusterRequest(); + + $client = $this->newConduitClient( + $viewer, + $is_cluster_request); + + return $client; + } + public function getPassthroughEnvironmentalVariables() { $env = $_ENV;