Page MenuHomePhabricator

D21442.id51041.diff
No OneTemporary

D21442.id51041.diff

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;

File Metadata

Mime Type
text/plain
Expires
Wed, Mar 5, 1:09 PM (2 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7225605
Default Alt Text
D21442.id51041.diff (6 KB)

Event Timeline