@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Lift Diffusion Conduit call proxying to the root level of Conduit

Summary:
Ref T13552. Some Diffusion conduit calls may only be served by a node which hosts a working copy on disk, so they're proxied if received by a different node.

This capability is currently bound tightly to "DiffusionRequest", which is a bundle of context parameters used by some Diffusion calls. However, call proxying is not fundamentally a Diffusion behavior.

I want to perform proxying on a "*.search" call which does not use the "DiffusionRequest" parameter bundle. Lift proxying to the root level of Conduit.

Test Plan: Browsed diffusion in a clusterized repsository.

Maniphest Tasks: T13552

Differential Revision: https://secure.phabricator.com/D21442

+69 -37
+12
src/applications/conduit/method/ConduitAPIMethod.php
··· 120 120 public function executeMethod(ConduitAPIRequest $request) { 121 121 $this->setViewer($request->getUser()); 122 122 123 + $client = $this->newConduitCallProxyClient($request); 124 + if ($client) { 125 + // We're proxying, so just make an intracluster call. 126 + return $client->callMethodSynchronous( 127 + $this->getAPIMethodName(), 128 + $request->getAllParameters()); 129 + } 130 + 123 131 return $this->execute($request); 132 + } 133 + 134 + protected function newConduitCallProxyClient(ConduitAPIRequest $request) { 135 + return null; 124 136 } 125 137 126 138 abstract public function getAPIMethodName();
+4
src/applications/conduit/protocol/ConduitAPIRequest.php
··· 51 51 return $this->user; 52 52 } 53 53 54 + public function getViewer() { 55 + return $this->getUser(); 56 + } 57 + 54 58 public function setOAuthToken( 55 59 PhabricatorOAuthServerAccessToken $oauth_token) { 56 60 $this->oauthToken = $oauth_token;
+36 -37
src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php
··· 85 85 * should occur after @{method:getResult}, like formatting a timestamp. 86 86 */ 87 87 final protected function execute(ConduitAPIRequest $request) { 88 + $drequest = $this->getDiffusionRequest(); 89 + 90 + // We pass this flag on to prevent proxying of any other Conduit calls 91 + // which we need to make in order to respond to this one. Although we 92 + // could safely proxy them, we take a big performance hit in the common 93 + // case, and doing more proxying wouldn't exercise any additional code so 94 + // we wouldn't gain a testability/predictability benefit. 95 + $is_cluster_request = $request->getIsClusterRequest(); 96 + $drequest->setIsClusterRequest($is_cluster_request); 97 + 98 + $viewer = $request->getViewer(); 99 + $repository = $drequest->getRepository(); 100 + 101 + // TODO: Allow web UI queries opt out of this if they don't care about 102 + // fetching the most up-to-date data? Synchronization can be slow, and a 103 + // lot of web reads are probably fine if they're a few seconds out of 104 + // date. 105 + id(new DiffusionRepositoryClusterEngine()) 106 + ->setViewer($viewer) 107 + ->setRepository($repository) 108 + ->synchronizeWorkingCopyBeforeRead(); 109 + 110 + return $this->getResult($request); 111 + } 112 + 113 + 114 + protected function newConduitCallProxyClient(ConduitAPIRequest $request) { 115 + $viewer = $request->getViewer(); 116 + 88 117 $identifier = $request->getValue('repository'); 89 118 if ($identifier === null) { 90 119 $identifier = $request->getValue('callsign'); ··· 92 121 93 122 $drequest = DiffusionRequest::newFromDictionary( 94 123 array( 95 - 'user' => $request->getUser(), 124 + 'user' => $viewer, 96 125 'repository' => $identifier, 97 126 'branch' => $request->getValue('branch'), 98 127 'path' => $request->getValue('path'), ··· 106 135 $identifier)); 107 136 } 108 137 109 - // Figure out whether we're going to handle this request on this device, 110 - // or proxy it to another node in the cluster. 138 + $repository = $drequest->getRepository(); 111 139 112 - // If this is a cluster request and we need to proxy, we'll explode here 113 - // to prevent infinite recursion. 114 - 115 - $is_cluster_request = $request->getIsClusterRequest(); 116 - $viewer = $request->getUser(); 117 - 118 - $repository = $drequest->getRepository(); 119 - $client = $repository->newConduitClient( 120 - $viewer, 121 - $is_cluster_request); 140 + $client = $repository->newConduitClientForRequest($request); 122 141 if ($client) { 123 - // We're proxying, so just make an intracluster call. 124 - return $client->callMethodSynchronous( 125 - $this->getAPIMethodName(), 126 - $request->getAllParameters()); 127 - } else { 128 - 129 - // We pass this flag on to prevent proxying of any other Conduit calls 130 - // which we need to make in order to respond to this one. Although we 131 - // could safely proxy them, we take a big performance hit in the common 132 - // case, and doing more proxying wouldn't exercise any additional code so 133 - // we wouldn't gain a testability/predictability benefit. 134 - $drequest->setIsClusterRequest($is_cluster_request); 142 + return $client; 143 + } 135 144 136 - $this->setDiffusionRequest($drequest); 137 - 138 - // TODO: Allow web UI queries opt out of this if they don't care about 139 - // fetching the most up-to-date data? Synchronization can be slow, and a 140 - // lot of web reads are probably fine if they're a few seconds out of 141 - // date. 142 - id(new DiffusionRepositoryClusterEngine()) 143 - ->setViewer($viewer) 144 - ->setRepository($repository) 145 - ->synchronizeWorkingCopyBeforeRead(); 145 + $this->setDiffusionRequest($drequest); 146 146 147 - return $this->getResult($request); 148 - } 147 + return null; 149 148 } 150 149 151 150 protected function getResult(ConduitAPIRequest $request) {
+17
src/applications/repository/storage/PhabricatorRepository.php
··· 2241 2241 return $client; 2242 2242 } 2243 2243 2244 + public function newConduitClientForRequest(ConduitAPIRequest $request) { 2245 + // Figure out whether we're going to handle this request on this device, 2246 + // or proxy it to another node in the cluster. 2247 + 2248 + // If this is a cluster request and we need to proxy, we'll explode here 2249 + // to prevent infinite recursion. 2250 + 2251 + $viewer = $request->getViewer(); 2252 + $is_cluster_request = $request->getIsClusterRequest(); 2253 + 2254 + $client = $this->newConduitClient( 2255 + $viewer, 2256 + $is_cluster_request); 2257 + 2258 + return $client; 2259 + } 2260 + 2244 2261 public function getPassthroughEnvironmentalVariables() { 2245 2262 $env = $_ENV; 2246 2263