Diffusion - move some DiffusionRequest queries to occur over Conduit
Summary: Ref T2784. This one was a wee bit complicated. Had to add PhabricatorUser and concept of initFromConduit (or not) to DiffusionRequest. Test Plan: foreach repo, visited CALLSIGN and clicked a commit and verified they laoded correctly. Hacked code to hit NOT via Conduit and repeated tests to great success. Reviewers: epriestley Reviewed By: epriestley CC: chad, aran, Korvin Maniphest Tasks: T2784 Differential Revision: https://secure.phabricator.com/D5928
This commit is contained in:
@@ -17,6 +17,9 @@ abstract class ConduitAPI_diffusion_abstractquery_Method
|
||||
}
|
||||
|
||||
private $diffusionRequest;
|
||||
private $repository;
|
||||
private $shouldCreateDiffusionRequest = true;
|
||||
|
||||
protected function setDiffusionRequest(DiffusionRequest $request) {
|
||||
$this->diffusionRequest = $request;
|
||||
return $this;
|
||||
@@ -25,10 +28,54 @@ abstract class ConduitAPI_diffusion_abstractquery_Method
|
||||
return $this->diffusionRequest;
|
||||
}
|
||||
|
||||
/**
|
||||
* A wee bit of magic here. If @{method:shouldCreateDiffusionRequest}
|
||||
* returns false, this function grabs a repository object based on the
|
||||
* callsign directly. Otherwise, the repository was loaded when we created a
|
||||
* @{class:DiffusionRequest}, so this function just pulls it out of the
|
||||
* @{class:DiffusionRequest}.
|
||||
*
|
||||
* @return @{class:PhabricatorRepository} $repository
|
||||
*/
|
||||
protected function getRepository(ConduitAPIRequest $request) {
|
||||
if (!$this->repository) {
|
||||
if ($this->shouldCreateDiffusionRequest()) {
|
||||
$this->repository = $this->getDiffusionRequest()->getRepository();
|
||||
} else {
|
||||
$callsign = $request->getValue('callsign');
|
||||
$repository = id(new PhabricatorRepository())->loadOneWhere(
|
||||
'callsign = %s',
|
||||
$callsign);
|
||||
if (!$repository) {
|
||||
throw new ConduitException('ERR-UNKNOWN-REPOSITORY');
|
||||
}
|
||||
$this->repository = $repository;
|
||||
}
|
||||
}
|
||||
return $this->repository;
|
||||
}
|
||||
|
||||
/**
|
||||
* You should probably not mess with this unless your conduit method is
|
||||
* involved with the creation / validation / etc. of
|
||||
* @{class:DiffusionRequest}s. If you are dealing with
|
||||
* @{class:DiffusionRequest}, setting this to false should help avoid
|
||||
* infinite loops.
|
||||
*/
|
||||
protected function setShouldCreateDiffusionRequest($should) {
|
||||
$this->shouldCreateDiffusionRequest = $should;
|
||||
return $this;
|
||||
}
|
||||
private function shouldCreateDiffusionRequest() {
|
||||
return $this->shouldCreateDiffusionRequest;
|
||||
}
|
||||
|
||||
final public function defineErrorTypes() {
|
||||
return $this->defineCustomErrorTypes() +
|
||||
array(
|
||||
'ERR-UNKNOWN-REPOSITORY-VCS' =>
|
||||
'ERR-UNKNOWN-REPOSITORY' =>
|
||||
pht('There is no repository with that callsign.'),
|
||||
'ERR-UNKNOWN-VCS-TYPE' =>
|
||||
pht('Unknown repository VCS type.'),
|
||||
'ERR-UNSUPPORTED-VCS' =>
|
||||
pht('VCS is not supported for this method.'));
|
||||
@@ -70,29 +117,39 @@ abstract class ConduitAPI_diffusion_abstractquery_Method
|
||||
}
|
||||
|
||||
/**
|
||||
* This method is final because each query will need to construct a
|
||||
* This method is final because most queries will need to construct a
|
||||
* @{class:DiffusionRequest} and use it. Consolidating this codepath and
|
||||
* enforcing @{method:getDiffusionRequest} works when we need it is good.
|
||||
*
|
||||
* @{method:getResult} should be overridden by subclasses as necessary, e.g.
|
||||
* there is a common operation across all version control systems that
|
||||
* should occur after @{method:getResult}, like formatting a timestamp.
|
||||
*
|
||||
* In the rare cases where one does not want to create a
|
||||
* @{class:DiffusionRequest} - suppose to avoid infinite loops in the
|
||||
* creation of a @{class:DiffusionRequest} - make sure to call
|
||||
*
|
||||
* $this->setShouldCreateDiffusionRequest(false);
|
||||
*
|
||||
* in the constructor of the pertinent Conduit method.
|
||||
*/
|
||||
final protected function execute(ConduitAPIRequest $request) {
|
||||
$drequest = DiffusionRequest::newFromDictionary(
|
||||
array(
|
||||
'callsign' => $request->getValue('callsign'),
|
||||
'path' => $request->getValue('path'),
|
||||
'commit' => $request->getValue('commit'),
|
||||
));
|
||||
$this->setDiffusionRequest($drequest);
|
||||
if ($this->shouldCreateDiffusionRequest()) {
|
||||
$drequest = DiffusionRequest::newFromDictionary(
|
||||
array(
|
||||
'user' => $request->getUser(),
|
||||
'callsign' => $request->getValue('callsign'),
|
||||
'path' => $request->getValue('path'),
|
||||
'commit' => $request->getValue('commit'),
|
||||
));
|
||||
$this->setDiffusionRequest($drequest);
|
||||
}
|
||||
|
||||
return $this->getResult($request);
|
||||
}
|
||||
|
||||
protected function getResult(ConduitAPIRequest $request) {
|
||||
$drequest = $this->getDiffusionRequest();
|
||||
$repository = $drequest->getRepository();
|
||||
$repository = $this->getRepository($request);
|
||||
$result = null;
|
||||
switch ($repository->getVersionControlSystem()) {
|
||||
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
|
||||
@@ -105,7 +162,7 @@ abstract class ConduitAPI_diffusion_abstractquery_Method
|
||||
$result = $this->getSVNResult($request);
|
||||
break;
|
||||
default:
|
||||
throw new ConduitException('ERR-UNKNOWN-REPOSITORY-VCS');
|
||||
throw new ConduitException('ERR-UNKNOWN-VCS-TYPE');
|
||||
break;
|
||||
}
|
||||
return $result;
|
||||
|
||||
Reference in New Issue
Block a user