diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 84c76de72b..add8704e97 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -144,6 +144,8 @@ phutil_register_library_map(array( 'ConduitAPI_differential_updaterevision_Method' => 'applications/differential/conduit/ConduitAPI_differential_updaterevision_Method.php', 'ConduitAPI_differential_updateunitresults_Method' => 'applications/differential/conduit/ConduitAPI_differential_updateunitresults_Method.php', 'ConduitAPI_diffusion_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_Method.php', + 'ConduitAPI_diffusion_abstractquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php', + 'ConduitAPI_diffusion_existsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_existsquery_Method.php', 'ConduitAPI_diffusion_findsymbols_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_findsymbols_Method.php', 'ConduitAPI_diffusion_getcommits_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php', 'ConduitAPI_diffusion_getlintmessages_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_getlintmessages_Method.php', @@ -418,7 +420,6 @@ phutil_register_library_map(array( 'DiffusionDiffController' => 'applications/diffusion/controller/DiffusionDiffController.php', 'DiffusionDiffQuery' => 'applications/diffusion/query/diff/DiffusionDiffQuery.php', 'DiffusionEmptyResultView' => 'applications/diffusion/view/DiffusionEmptyResultView.php', - 'DiffusionExistsQuery' => 'applications/diffusion/query/exists/DiffusionExistsQuery.php', 'DiffusionExternalController' => 'applications/diffusion/controller/DiffusionExternalController.php', 'DiffusionFileContent' => 'applications/diffusion/data/DiffusionFileContent.php', 'DiffusionFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionFileContentQuery.php', @@ -429,7 +430,6 @@ phutil_register_library_map(array( 'DiffusionGitCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionGitCommitTagsQuery.php', 'DiffusionGitContainsQuery' => 'applications/diffusion/query/contains/DiffusionGitContainsQuery.php', 'DiffusionGitDiffQuery' => 'applications/diffusion/query/diff/DiffusionGitDiffQuery.php', - 'DiffusionGitExistsQuery' => 'applications/diffusion/query/exists/DiffusionGitExistsQuery.php', 'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php', 'DiffusionGitHistoryQuery' => 'applications/diffusion/query/history/DiffusionGitHistoryQuery.php', 'DiffusionGitLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionGitLastModifiedQuery.php', @@ -455,7 +455,6 @@ phutil_register_library_map(array( 'DiffusionMercurialCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionMercurialCommitTagsQuery.php', 'DiffusionMercurialContainsQuery' => 'applications/diffusion/query/contains/DiffusionMercurialContainsQuery.php', 'DiffusionMercurialDiffQuery' => 'applications/diffusion/query/diff/DiffusionMercurialDiffQuery.php', - 'DiffusionMercurialExistsQuery' => 'applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php', 'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php', 'DiffusionMercurialHistoryQuery' => 'applications/diffusion/query/history/DiffusionMercurialHistoryQuery.php', 'DiffusionMercurialLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionMercurialLastModifiedQuery.php', @@ -486,7 +485,6 @@ phutil_register_library_map(array( 'DiffusionSvnCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionSvnCommitTagsQuery.php', 'DiffusionSvnContainsQuery' => 'applications/diffusion/query/contains/DiffusionSvnContainsQuery.php', 'DiffusionSvnDiffQuery' => 'applications/diffusion/query/diff/DiffusionSvnDiffQuery.php', - 'DiffusionSvnExistsQuery' => 'applications/diffusion/query/exists/DiffusionSvnExistsQuery.php', 'DiffusionSvnFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php', 'DiffusionSvnHistoryQuery' => 'applications/diffusion/query/history/DiffusionSvnHistoryQuery.php', 'DiffusionSvnLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionSvnLastModifiedQuery.php', @@ -1913,6 +1911,8 @@ phutil_register_library_map(array( 'ConduitAPI_differential_updaterevision_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_updateunitresults_Method' => 'ConduitAPIMethod', 'ConduitAPI_diffusion_Method' => 'ConduitAPIMethod', + 'ConduitAPI_diffusion_abstractquery_Method' => 'ConduitAPI_diffusion_Method', + 'ConduitAPI_diffusion_existsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', 'ConduitAPI_diffusion_findsymbols_Method' => 'ConduitAPI_diffusion_Method', 'ConduitAPI_diffusion_getcommits_Method' => 'ConduitAPI_diffusion_Method', 'ConduitAPI_diffusion_getlintmessages_Method' => 'ConduitAPI_diffusion_Method', @@ -2176,7 +2176,6 @@ phutil_register_library_map(array( 'DiffusionDiffController' => 'DiffusionController', 'DiffusionDiffQuery' => 'DiffusionQuery', 'DiffusionEmptyResultView' => 'DiffusionView', - 'DiffusionExistsQuery' => 'DiffusionQuery', 'DiffusionExternalController' => 'DiffusionController', 'DiffusionFileContentQuery' => 'DiffusionQuery', 'DiffusionGitBranchQuery' => 'DiffusionBranchQuery', @@ -2186,7 +2185,6 @@ phutil_register_library_map(array( 'DiffusionGitCommitTagsQuery' => 'DiffusionCommitTagsQuery', 'DiffusionGitContainsQuery' => 'DiffusionContainsQuery', 'DiffusionGitDiffQuery' => 'DiffusionDiffQuery', - 'DiffusionGitExistsQuery' => 'DiffusionExistsQuery', 'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionGitHistoryQuery' => 'DiffusionHistoryQuery', 'DiffusionGitLastModifiedQuery' => 'DiffusionLastModifiedQuery', @@ -2211,7 +2209,6 @@ phutil_register_library_map(array( 'DiffusionMercurialCommitTagsQuery' => 'DiffusionCommitTagsQuery', 'DiffusionMercurialContainsQuery' => 'DiffusionContainsQuery', 'DiffusionMercurialDiffQuery' => 'DiffusionDiffQuery', - 'DiffusionMercurialExistsQuery' => 'DiffusionExistsQuery', 'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionMercurialHistoryQuery' => 'DiffusionHistoryQuery', 'DiffusionMercurialLastModifiedQuery' => 'DiffusionLastModifiedQuery', @@ -2234,7 +2231,6 @@ phutil_register_library_map(array( 'DiffusionSvnCommitTagsQuery' => 'DiffusionCommitTagsQuery', 'DiffusionSvnContainsQuery' => 'DiffusionContainsQuery', 'DiffusionSvnDiffQuery' => 'DiffusionDiffQuery', - 'DiffusionSvnExistsQuery' => 'DiffusionExistsQuery', 'DiffusionSvnFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionSvnHistoryQuery' => 'DiffusionHistoryQuery', 'DiffusionSvnLastModifiedQuery' => 'DiffusionLastModifiedQuery', diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php new file mode 100644 index 0000000000..4d31d8a348 --- /dev/null +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php @@ -0,0 +1,101 @@ +diffusionRequest = $request; + return $this; + } + protected function getDiffusionRequest() { + return $this->diffusionRequest; + } + + final public function defineErrorTypes() { + return $this->defineCustomErrorTypes() + + array( + 'ERR-UNKNOWN-REPOSITORY-VCS' => + pht('Unknown repository VCS type.'), + 'ERR-UNSUPPORTED-VCS' => + pht('VCS is not supported for this method.')); + } + /** + * Subclasses should override this to specify custom error types. + */ + protected function defineCustomErrorTypes() { + return array(); + } + + final public function defineParamTypes() { + return $this->defineCustomParamTypes() + + array( + 'callsign' => 'required string'); + } + /** + * Subclasses should override this to specify custom param types. + */ + protected function defineCustomParamTypes() { + return array(); + } + + /** + * Subclasses should override these methods with the proper result for the + * pertinent version control system, e.g. getGitResult for Git. + * + * If the result is not supported for that VCS, do not implement it. e.g. + * Subversion (SVN) does not support branches. + */ + protected function getGitResult(ConduitAPIRequest $request) { + throw new ConduitException('ERR-UNSUPPORTED-VCS'); + } + protected function getSVNResult(ConduitAPIRequest $request) { + throw new ConduitException('ERR-UNSUPPORTED-VCS'); + } + protected function getMercurialResult(ConduitAPIRequest $request) { + throw new ConduitException('ERR-UNSUPPORTED-VCS'); + } + + /** + * This method is final because each query 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. + */ + final protected function execute(ConduitAPIRequest $request) { + $drequest = DiffusionRequest::newFromDictionary( + array( + 'callsign' => $request->getValue('callsign'), + )); + $this->setDiffusionRequest($drequest); + + return $this->getResult($request); + } + + protected function getResult(ConduitAPIRequest $request) { + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + $result = null; + switch ($repository->getVersionControlSystem()) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + $result = $this->getGitResult($request); + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + $result = $this->getMercurialResult($request); + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + $result = $this->getSVNResult($request); + break; + default: + throw new ConduitException('ERR-UNKNOWN-REPOSITORY-VCS'); + break; + } + return $result; + } +} diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_existsquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_existsquery_Method.php new file mode 100644 index 0000000000..1948960a46 --- /dev/null +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_existsquery_Method.php @@ -0,0 +1,55 @@ + 'required string' + ); + } + + protected function getGitResult(ConduitAPIRequest $request) { + $repository = $this->getDiffusionRequest()->getRepository(); + $commit = $request->getValue('commit'); + list($err, $merge_base) = $repository->execLocalCommand( + 'cat-file -t %s', + $commit); + return !$err; + } + + protected function getSVNResult(ConduitAPIRequest $request) { + $repository = $this->getDiffusionRequest()->getRepository(); + $commit = $request->getValue('commit'); + list($info) = $repository->execxRemoteCommand( + 'info %s', + $repository->getRemoteURI()); + $exists = false; + $matches = null; + if (preg_match('/^Revision: (\d+)$/m', $info, $matches)) { + $base_revision = $matches[1]; + $exists = $base_revision >= $commit; + } + return $exists; + } + + protected function getMercurialResult(ConduitAPIRequest $request) { + $repository = $this->getDiffusionRequest()->getRepository(); + $commit = $request->getValue('commit'); + list($err, $stdout) = $repository->execLocalCommand( + 'id --rev %s', + $commit); + return !$err; + } +} diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 16fe0d999a..c448c56a27 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -30,8 +30,9 @@ final class DiffusionCommitController extends DiffusionController { $commit = $drequest->loadCommit(); if (!$commit) { - $query = DiffusionExistsQuery::newFromDiffusionRequest($drequest); - $exists = $query->loadExistentialData(); + $exists = $this->callConduitWithDiffusionRequest( + 'diffusion.existsquery', + array('commit' => $drequest->getCommit())); if (!$exists) { return new Aphront404Response(); } diff --git a/src/applications/diffusion/controller/DiffusionController.php b/src/applications/diffusion/controller/DiffusionController.php index 4eb056e451..36f3c4f87a 100644 --- a/src/applications/diffusion/controller/DiffusionController.php +++ b/src/applications/diffusion/controller/DiffusionController.php @@ -321,4 +321,17 @@ abstract class DiffusionController extends PhabricatorController { return $crumb_list; } + protected function callConduitWithDiffusionRequest( + $method, + array $params = array()) { + + $user = $this->getRequest()->getUser(); + $drequest = $this->getDiffusionRequest(); + + return DiffusionQuery::callConduitWithDiffusionRequest( + $user, + $drequest, + $method, + $params); + } } diff --git a/src/applications/diffusion/query/DiffusionQuery.php b/src/applications/diffusion/query/DiffusionQuery.php index 24120ce0f0..7846730ef3 100644 --- a/src/applications/diffusion/query/DiffusionQuery.php +++ b/src/applications/diffusion/query/DiffusionQuery.php @@ -36,6 +36,27 @@ abstract class DiffusionQuery extends PhabricatorQuery { return $this->request; } + final public static function callConduitWithDiffusionRequest( + PhabricatorUser $user, + DiffusionRequest $drequest, + $method, + array $params = array()) { + + $repository = $drequest->getRepository(); + + $core_params = array( + 'callsign' => $repository->getCallsign() + ); + $params = $params + $core_params; + + return id(new ConduitCall( + $method, + $params + )) + ->setUser($user) + ->execute(); + } + public function execute() { return $this->executeQuery(); } diff --git a/src/applications/diffusion/query/exists/DiffusionExistsQuery.php b/src/applications/diffusion/query/exists/DiffusionExistsQuery.php deleted file mode 100644 index 17a22970c4..0000000000 --- a/src/applications/diffusion/query/exists/DiffusionExistsQuery.php +++ /dev/null @@ -1,13 +0,0 @@ -executeQuery(); - } -} diff --git a/src/applications/diffusion/query/exists/DiffusionGitExistsQuery.php b/src/applications/diffusion/query/exists/DiffusionGitExistsQuery.php deleted file mode 100644 index 58dd71430e..0000000000 --- a/src/applications/diffusion/query/exists/DiffusionGitExistsQuery.php +++ /dev/null @@ -1,16 +0,0 @@ -getRequest(); - $repository = $request->getRepository(); - - list($err, $merge_base) = $repository->execLocalCommand( - 'cat-file -t %s', - $request->getCommit()); - - return !$err; - } - -} diff --git a/src/applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php b/src/applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php deleted file mode 100644 index 6f911e482e..0000000000 --- a/src/applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php +++ /dev/null @@ -1,16 +0,0 @@ -getRequest(); - $repository = $request->getRepository(); - - list($err, $stdout) = $repository->execLocalCommand( - 'id --rev %s', - $request->getCommit()); - - return !$err; - - } -} diff --git a/src/applications/diffusion/query/exists/DiffusionSvnExistsQuery.php b/src/applications/diffusion/query/exists/DiffusionSvnExistsQuery.php deleted file mode 100644 index 7d43fb081f..0000000000 --- a/src/applications/diffusion/query/exists/DiffusionSvnExistsQuery.php +++ /dev/null @@ -1,23 +0,0 @@ -getRequest(); - $repository = $request->getRepository(); - - list($info) = $repository->execxRemoteCommand( - 'info %s', - $repository->getRemoteURI()); - - $matches = null; - $exists = false; - if (preg_match('/^Revision: (\d+)$/m', $info, $matches)) { - $base_revision = $matches[1]; - $exists = $base_revision >= $request->getCommit(); - } - - return $exists; - } - -}