Skip to content

Commit c79d261

Browse files
bobtrahanbobtrahan
committed
Diffusion - move DiffusionExistsQuery to work over conduit
Summary: le title. However, this also "is the first" conversion so sets the precedence for how this will all work. See comments in the code. I think this helps us keep the new code we're writing to a minimum. Wondering if the conduit end point could be more generic, and rather than have a switch statement on VCS type, one can just implement the "handleSubversion" version and have that called? Ref T2784 Test Plan: slapped an "or true" in the conditional protecting this code path. verified it worked on all 3 vcs systems, including typing in garbage and getting a 404 Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2784 Differential Revision: https://secure.phabricator.com/D5803
1 parent ea78f92 commit c79d261

10 files changed

+197
-78
lines changed

‎src/__phutil_library_map__.php‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,8 @@
144144
'ConduitAPI_differential_updaterevision_Method' => 'applications/differential/conduit/ConduitAPI_differential_updaterevision_Method.php',
145145
'ConduitAPI_differential_updateunitresults_Method' => 'applications/differential/conduit/ConduitAPI_differential_updateunitresults_Method.php',
146146
'ConduitAPI_diffusion_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_Method.php',
147+
'ConduitAPI_diffusion_abstractquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php',
148+
'ConduitAPI_diffusion_existsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_existsquery_Method.php',
147149
'ConduitAPI_diffusion_findsymbols_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_findsymbols_Method.php',
148150
'ConduitAPI_diffusion_getcommits_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php',
149151
'ConduitAPI_diffusion_getlintmessages_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_getlintmessages_Method.php',
@@ -418,7 +420,6 @@
418420
'DiffusionDiffController' => 'applications/diffusion/controller/DiffusionDiffController.php',
419421
'DiffusionDiffQuery' => 'applications/diffusion/query/diff/DiffusionDiffQuery.php',
420422
'DiffusionEmptyResultView' => 'applications/diffusion/view/DiffusionEmptyResultView.php',
421-
'DiffusionExistsQuery' => 'applications/diffusion/query/exists/DiffusionExistsQuery.php',
422423
'DiffusionExternalController' => 'applications/diffusion/controller/DiffusionExternalController.php',
423424
'DiffusionFileContent' => 'applications/diffusion/data/DiffusionFileContent.php',
424425
'DiffusionFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionFileContentQuery.php',
@@ -429,7 +430,6 @@
429430
'DiffusionGitCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionGitCommitTagsQuery.php',
430431
'DiffusionGitContainsQuery' => 'applications/diffusion/query/contains/DiffusionGitContainsQuery.php',
431432
'DiffusionGitDiffQuery' => 'applications/diffusion/query/diff/DiffusionGitDiffQuery.php',
432-
'DiffusionGitExistsQuery' => 'applications/diffusion/query/exists/DiffusionGitExistsQuery.php',
433433
'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php',
434434
'DiffusionGitHistoryQuery' => 'applications/diffusion/query/history/DiffusionGitHistoryQuery.php',
435435
'DiffusionGitLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionGitLastModifiedQuery.php',
@@ -455,7 +455,6 @@
455455
'DiffusionMercurialCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionMercurialCommitTagsQuery.php',
456456
'DiffusionMercurialContainsQuery' => 'applications/diffusion/query/contains/DiffusionMercurialContainsQuery.php',
457457
'DiffusionMercurialDiffQuery' => 'applications/diffusion/query/diff/DiffusionMercurialDiffQuery.php',
458-
'DiffusionMercurialExistsQuery' => 'applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php',
459458
'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php',
460459
'DiffusionMercurialHistoryQuery' => 'applications/diffusion/query/history/DiffusionMercurialHistoryQuery.php',
461460
'DiffusionMercurialLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionMercurialLastModifiedQuery.php',
@@ -486,7 +485,6 @@
486485
'DiffusionSvnCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionSvnCommitTagsQuery.php',
487486
'DiffusionSvnContainsQuery' => 'applications/diffusion/query/contains/DiffusionSvnContainsQuery.php',
488487
'DiffusionSvnDiffQuery' => 'applications/diffusion/query/diff/DiffusionSvnDiffQuery.php',
489-
'DiffusionSvnExistsQuery' => 'applications/diffusion/query/exists/DiffusionSvnExistsQuery.php',
490488
'DiffusionSvnFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php',
491489
'DiffusionSvnHistoryQuery' => 'applications/diffusion/query/history/DiffusionSvnHistoryQuery.php',
492490
'DiffusionSvnLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionSvnLastModifiedQuery.php',
@@ -1913,6 +1911,8 @@
19131911
'ConduitAPI_differential_updaterevision_Method' => 'ConduitAPIMethod',
19141912
'ConduitAPI_differential_updateunitresults_Method' => 'ConduitAPIMethod',
19151913
'ConduitAPI_diffusion_Method' => 'ConduitAPIMethod',
1914+
'ConduitAPI_diffusion_abstractquery_Method' => 'ConduitAPI_diffusion_Method',
1915+
'ConduitAPI_diffusion_existsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method',
19161916
'ConduitAPI_diffusion_findsymbols_Method' => 'ConduitAPI_diffusion_Method',
19171917
'ConduitAPI_diffusion_getcommits_Method' => 'ConduitAPI_diffusion_Method',
19181918
'ConduitAPI_diffusion_getlintmessages_Method' => 'ConduitAPI_diffusion_Method',
@@ -2176,7 +2176,6 @@
21762176
'DiffusionDiffController' => 'DiffusionController',
21772177
'DiffusionDiffQuery' => 'DiffusionQuery',
21782178
'DiffusionEmptyResultView' => 'DiffusionView',
2179-
'DiffusionExistsQuery' => 'DiffusionQuery',
21802179
'DiffusionExternalController' => 'DiffusionController',
21812180
'DiffusionFileContentQuery' => 'DiffusionQuery',
21822181
'DiffusionGitBranchQuery' => 'DiffusionBranchQuery',
@@ -2186,7 +2185,6 @@
21862185
'DiffusionGitCommitTagsQuery' => 'DiffusionCommitTagsQuery',
21872186
'DiffusionGitContainsQuery' => 'DiffusionContainsQuery',
21882187
'DiffusionGitDiffQuery' => 'DiffusionDiffQuery',
2189-
'DiffusionGitExistsQuery' => 'DiffusionExistsQuery',
21902188
'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery',
21912189
'DiffusionGitHistoryQuery' => 'DiffusionHistoryQuery',
21922190
'DiffusionGitLastModifiedQuery' => 'DiffusionLastModifiedQuery',
@@ -2211,7 +2209,6 @@
22112209
'DiffusionMercurialCommitTagsQuery' => 'DiffusionCommitTagsQuery',
22122210
'DiffusionMercurialContainsQuery' => 'DiffusionContainsQuery',
22132211
'DiffusionMercurialDiffQuery' => 'DiffusionDiffQuery',
2214-
'DiffusionMercurialExistsQuery' => 'DiffusionExistsQuery',
22152212
'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery',
22162213
'DiffusionMercurialHistoryQuery' => 'DiffusionHistoryQuery',
22172214
'DiffusionMercurialLastModifiedQuery' => 'DiffusionLastModifiedQuery',
@@ -2234,7 +2231,6 @@
22342231
'DiffusionSvnCommitTagsQuery' => 'DiffusionCommitTagsQuery',
22352232
'DiffusionSvnContainsQuery' => 'DiffusionContainsQuery',
22362233
'DiffusionSvnDiffQuery' => 'DiffusionDiffQuery',
2237-
'DiffusionSvnExistsQuery' => 'DiffusionExistsQuery',
22382234
'DiffusionSvnFileContentQuery' => 'DiffusionFileContentQuery',
22392235
'DiffusionSvnHistoryQuery' => 'DiffusionHistoryQuery',
22402236
'DiffusionSvnLastModifiedQuery' => 'DiffusionLastModifiedQuery',

‎src/applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
<?php
2+
3+
/**
4+
* @group conduit
5+
*/
6+
abstract class ConduitAPI_diffusion_abstractquery_Method
7+
extends ConduitAPI_diffusion_Method {
8+
9+
private $diffusionRequest;
10+
protected function setDiffusionRequest(DiffusionRequest $request) {
11+
$this->diffusionRequest = $request;
12+
return $this;
13+
}
14+
protected function getDiffusionRequest() {
15+
return $this->diffusionRequest;
16+
}
17+
18+
final public function defineErrorTypes() {
19+
return $this->defineCustomErrorTypes() +
20+
array(
21+
'ERR-UNKNOWN-REPOSITORY-VCS' =>
22+
pht('Unknown repository VCS type.'),
23+
'ERR-UNSUPPORTED-VCS' =>
24+
pht('VCS is not supported for this method.'));
25+
}
26+
/**
27+
* Subclasses should override this to specify custom error types.
28+
*/
29+
protected function defineCustomErrorTypes() {
30+
return array();
31+
}
32+
33+
final public function defineParamTypes() {
34+
return $this->defineCustomParamTypes() +
35+
array(
36+
'callsign' => 'required string');
37+
}
38+
/**
39+
* Subclasses should override this to specify custom param types.
40+
*/
41+
protected function defineCustomParamTypes() {
42+
return array();
43+
}
44+
45+
/**
46+
* Subclasses should override these methods with the proper result for the
47+
* pertinent version control system, e.g. getGitResult for Git.
48+
*
49+
* If the result is not supported for that VCS, do not implement it. e.g.
50+
* Subversion (SVN) does not support branches.
51+
*/
52+
protected function getGitResult(ConduitAPIRequest $request) {
53+
throw new ConduitException('ERR-UNSUPPORTED-VCS');
54+
}
55+
protected function getSVNResult(ConduitAPIRequest $request) {
56+
throw new ConduitException('ERR-UNSUPPORTED-VCS');
57+
}
58+
protected function getMercurialResult(ConduitAPIRequest $request) {
59+
throw new ConduitException('ERR-UNSUPPORTED-VCS');
60+
}
61+
62+
/**
63+
* This method is final because each query will need to construct a
64+
* @{class:DiffusionRequest} and use it. Consolidating this codepath and
65+
* enforcing @{method:getDiffusionRequest} works when we need it is good.
66+
*
67+
* @{method:getResult} should be overridden by subclasses as necessary, e.g.
68+
* there is a common operation across all version control systems that
69+
* should occur after @{method:getResult}, like formatting a timestamp.
70+
*/
71+
final protected function execute(ConduitAPIRequest $request) {
72+
$drequest = DiffusionRequest::newFromDictionary(
73+
array(
74+
'callsign' => $request->getValue('callsign'),
75+
));
76+
$this->setDiffusionRequest($drequest);
77+
78+
return $this->getResult($request);
79+
}
80+
81+
protected function getResult(ConduitAPIRequest $request) {
82+
$drequest = $this->getDiffusionRequest();
83+
$repository = $drequest->getRepository();
84+
$result = null;
85+
switch ($repository->getVersionControlSystem()) {
86+
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
87+
$result = $this->getGitResult($request);
88+
break;
89+
case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
90+
$result = $this->getMercurialResult($request);
91+
break;
92+
case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
93+
$result = $this->getSVNResult($request);
94+
break;
95+
default:
96+
throw new ConduitException('ERR-UNKNOWN-REPOSITORY-VCS');
97+
break;
98+
}
99+
return $result;
100+
}
101+
}

‎src/applications/diffusion/conduit/ConduitAPI_diffusion_existsquery_Method.php‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php
2+
3+
/**
4+
* @group conduit
5+
*/
6+
final class ConduitAPI_diffusion_existsquery_Method
7+
extends ConduitAPI_diffusion_abstractquery_Method {
8+
9+
public function getMethodDescription() {
10+
return "Determine if code exists in a version control system.";
11+
}
12+
13+
public function defineReturnType() {
14+
return 'bool';
15+
}
16+
17+
protected function defineCustomParamTypes() {
18+
return array(
19+
'commit' => 'required string'
20+
);
21+
}
22+
23+
protected function getGitResult(ConduitAPIRequest $request) {
24+
$repository = $this->getDiffusionRequest()->getRepository();
25+
$commit = $request->getValue('commit');
26+
list($err, $merge_base) = $repository->execLocalCommand(
27+
'cat-file -t %s',
28+
$commit);
29+
return !$err;
30+
}
31+
32+
protected function getSVNResult(ConduitAPIRequest $request) {
33+
$repository = $this->getDiffusionRequest()->getRepository();
34+
$commit = $request->getValue('commit');
35+
list($info) = $repository->execxRemoteCommand(
36+
'info %s',
37+
$repository->getRemoteURI());
38+
$exists = false;
39+
$matches = null;
40+
if (preg_match('/^Revision: (\d+)$/m', $info, $matches)) {
41+
$base_revision = $matches[1];
42+
$exists = $base_revision >= $commit;
43+
}
44+
return $exists;
45+
}
46+
47+
protected function getMercurialResult(ConduitAPIRequest $request) {
48+
$repository = $this->getDiffusionRequest()->getRepository();
49+
$commit = $request->getValue('commit');
50+
list($err, $stdout) = $repository->execLocalCommand(
51+
'id --rev %s',
52+
$commit);
53+
return !$err;
54+
}
55+
}

‎src/applications/diffusion/controller/DiffusionCommitController.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@ public function processRequest() {
3030
$commit = $drequest->loadCommit();
3131

3232
if (!$commit) {
33-
$query = DiffusionExistsQuery::newFromDiffusionRequest($drequest);
34-
$exists = $query->loadExistentialData();
33+
$exists = $this->callConduitWithDiffusionRequest(
34+
'diffusion.existsquery',
35+
array('commit' => $drequest->getCommit()));
3536
if (!$exists) {
3637
return new Aphront404Response();
3738
}

‎src/applications/diffusion/controller/DiffusionController.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,4 +321,17 @@ private function buildCrumbList(array $spec = array()) {
321321
return $crumb_list;
322322
}
323323

324+
protected function callConduitWithDiffusionRequest(
325+
$method,
326+
array $params = array()) {
327+
328+
$user = $this->getRequest()->getUser();
329+
$drequest = $this->getDiffusionRequest();
330+
331+
return DiffusionQuery::callConduitWithDiffusionRequest(
332+
$user,
333+
$drequest,
334+
$method,
335+
$params);
336+
}
324337
}

‎src/applications/diffusion/query/DiffusionQuery.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,27 @@ final protected function getRequest() {
3636
return $this->request;
3737
}
3838

39+
final public static function callConduitWithDiffusionRequest(
40+
PhabricatorUser $user,
41+
DiffusionRequest $drequest,
42+
$method,
43+
array $params = array()) {
44+
45+
$repository = $drequest->getRepository();
46+
47+
$core_params = array(
48+
'callsign' => $repository->getCallsign()
49+
);
50+
$params = $params + $core_params;
51+
52+
return id(new ConduitCall(
53+
$method,
54+
$params
55+
))
56+
->setUser($user)
57+
->execute();
58+
}
59+
3960
public function execute() {
4061
return $this->executeQuery();
4162
}

‎src/applications/diffusion/query/exists/DiffusionExistsQuery.php‎

Lines changed: 0 additions & 13 deletions
This file was deleted.

‎src/applications/diffusion/query/exists/DiffusionGitExistsQuery.php‎

Lines changed: 0 additions & 16 deletions
This file was deleted.

‎src/applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php‎

Lines changed: 0 additions & 16 deletions
This file was deleted.

‎src/applications/diffusion/query/exists/DiffusionSvnExistsQuery.php‎

Lines changed: 0 additions & 23 deletions
This file was deleted.

0 commit comments

Comments
 (0)