From c1d771d86ca51681e3481633830fed0af018976d Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 14 May 2013 13:53:32 -0700 Subject: [PATCH] Diffusion - move DiffQuery to Conduit Summary: title does not say it all; also added conduit wrappers for rawdiffquery and lastmodifiedquery. These get the wrapper treatment since they are used in daemons. Ref T2784. Test Plan: for each of the 3 VCS, did the following: 1) loaded up CALLSIGN and verified 'Modified' column data showed up correctly in Browse Repository box. 2) loaded up the "change" view for a specific file and verified content showed up correctly. 3) loaded up a specific commit and noted the changes ajax loaded A-OK Reviewers: epriestley Reviewed By: epriestley CC: chad, aran, Korvin Maniphest Tasks: T2784 Differential Revision: https://secure.phabricator.com/D5896 --- src/__phutil_library_map__.php | 22 +-- ...ConduitAPI_diffusion_diffquery_Method.php} | 148 +++++++++++++++--- ...API_diffusion_lastmodifiedquery_Method.php | 108 +++++++++++++ ...nduitAPI_diffusion_rawdiffquery_Method.php | 47 ++++++ .../DiffusionBrowseFileController.php | 42 +++-- .../controller/DiffusionChangeController.php | 14 +- .../controller/DiffusionCommitController.php | 7 +- .../controller/DiffusionDiffController.php | 18 ++- .../DiffusionLastModifiedController.php | 20 ++- .../query/diff/DiffusionDiffQuery.php | 33 ---- .../query/diff/DiffusionGitDiffQuery.php | 45 ------ .../diff/DiffusionMercurialDiffQuery.php | 41 ----- .../DiffusionGitLastModifiedQuery.php | 29 ---- .../DiffusionLastModifiedQuery.php | 14 -- .../DiffusionMercurialLastModifiedQuery.php | 32 ---- .../DiffusionSvnLastModifiedQuery.php | 27 ---- .../herald/adapter/HeraldCommitAdapter.php | 12 +- 17 files changed, 357 insertions(+), 302 deletions(-) rename src/applications/diffusion/{query/diff/DiffusionSvnDiffQuery.php => conduit/ConduitAPI_diffusion_diffquery_Method.php} (50%) create mode 100644 src/applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php create mode 100644 src/applications/diffusion/conduit/ConduitAPI_diffusion_rawdiffquery_Method.php delete mode 100644 src/applications/diffusion/query/diff/DiffusionDiffQuery.php delete mode 100644 src/applications/diffusion/query/diff/DiffusionGitDiffQuery.php delete mode 100644 src/applications/diffusion/query/diff/DiffusionMercurialDiffQuery.php delete mode 100644 src/applications/diffusion/query/lastmodified/DiffusionGitLastModifiedQuery.php delete mode 100644 src/applications/diffusion/query/lastmodified/DiffusionLastModifiedQuery.php delete mode 100644 src/applications/diffusion/query/lastmodified/DiffusionMercurialLastModifiedQuery.php delete mode 100644 src/applications/diffusion/query/lastmodified/DiffusionSvnLastModifiedQuery.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 39cdd72ccc..057ff9eb05 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -147,12 +147,15 @@ phutil_register_library_map(array( 'ConduitAPI_diffusion_abstractquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_abstractquery_Method.php', 'ConduitAPI_diffusion_branchquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php', 'ConduitAPI_diffusion_browsequery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_browsequery_Method.php', + 'ConduitAPI_diffusion_diffquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_diffquery_Method.php', 'ConduitAPI_diffusion_existsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_existsquery_Method.php', 'ConduitAPI_diffusion_filecontentquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_filecontentquery_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', 'ConduitAPI_diffusion_getrecentcommitsbypath_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_getrecentcommitsbypath_Method.php', + 'ConduitAPI_diffusion_lastmodifiedquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php', + 'ConduitAPI_diffusion_rawdiffquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_rawdiffquery_Method.php', 'ConduitAPI_diffusion_tagsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php', 'ConduitAPI_feed_Method' => 'applications/feed/conduit/ConduitAPI_feed_Method.php', 'ConduitAPI_feed_publish_Method' => 'applications/feed/conduit/ConduitAPI_feed_publish_Method.php', @@ -425,7 +428,6 @@ phutil_register_library_map(array( 'DiffusionContainsQuery' => 'applications/diffusion/query/contains/DiffusionContainsQuery.php', 'DiffusionController' => 'applications/diffusion/controller/DiffusionController.php', 'DiffusionDiffController' => 'applications/diffusion/controller/DiffusionDiffController.php', - 'DiffusionDiffQuery' => 'applications/diffusion/query/diff/DiffusionDiffQuery.php', 'DiffusionEmptyResultView' => 'applications/diffusion/view/DiffusionEmptyResultView.php', 'DiffusionExternalController' => 'applications/diffusion/controller/DiffusionExternalController.php', 'DiffusionFileContent' => 'applications/diffusion/data/DiffusionFileContent.php', @@ -434,10 +436,8 @@ phutil_register_library_map(array( 'DiffusionGitBranchTestCase' => 'applications/diffusion/data/__tests__/DiffusionGitBranchTestCase.php', 'DiffusionGitCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionGitCommitParentsQuery.php', 'DiffusionGitContainsQuery' => 'applications/diffusion/query/contains/DiffusionGitContainsQuery.php', - 'DiffusionGitDiffQuery' => 'applications/diffusion/query/diff/DiffusionGitDiffQuery.php', 'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php', 'DiffusionGitHistoryQuery' => 'applications/diffusion/query/history/DiffusionGitHistoryQuery.php', - 'DiffusionGitLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionGitLastModifiedQuery.php', 'DiffusionGitMergedCommitsQuery' => 'applications/diffusion/query/mergedcommits/DiffusionGitMergedCommitsQuery.php', 'DiffusionGitRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php', 'DiffusionGitRequest' => 'applications/diffusion/request/DiffusionGitRequest.php', @@ -449,16 +449,13 @@ phutil_register_library_map(array( 'DiffusionInlineCommentController' => 'applications/diffusion/controller/DiffusionInlineCommentController.php', 'DiffusionInlineCommentPreviewController' => 'applications/diffusion/controller/DiffusionInlineCommentPreviewController.php', 'DiffusionLastModifiedController' => 'applications/diffusion/controller/DiffusionLastModifiedController.php', - 'DiffusionLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionLastModifiedQuery.php', 'DiffusionLintController' => 'applications/diffusion/controller/DiffusionLintController.php', 'DiffusionLintDetailsController' => 'applications/diffusion/controller/DiffusionLintDetailsController.php', 'DiffusionLintSaveRunner' => 'applications/diffusion/DiffusionLintSaveRunner.php', 'DiffusionMercurialCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php', 'DiffusionMercurialContainsQuery' => 'applications/diffusion/query/contains/DiffusionMercurialContainsQuery.php', - 'DiffusionMercurialDiffQuery' => 'applications/diffusion/query/diff/DiffusionMercurialDiffQuery.php', 'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php', 'DiffusionMercurialHistoryQuery' => 'applications/diffusion/query/history/DiffusionMercurialHistoryQuery.php', - 'DiffusionMercurialLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionMercurialLastModifiedQuery.php', 'DiffusionMercurialMergedCommitsQuery' => 'applications/diffusion/query/mergedcommits/DiffusionMercurialMergedCommitsQuery.php', 'DiffusionMercurialRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php', 'DiffusionMercurialRequest' => 'applications/diffusion/request/DiffusionMercurialRequest.php', @@ -482,10 +479,8 @@ phutil_register_library_map(array( 'DiffusionSetupException' => 'applications/diffusion/exception/DiffusionSetupException.php', 'DiffusionSvnCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionSvnCommitParentsQuery.php', 'DiffusionSvnContainsQuery' => 'applications/diffusion/query/contains/DiffusionSvnContainsQuery.php', - 'DiffusionSvnDiffQuery' => 'applications/diffusion/query/diff/DiffusionSvnDiffQuery.php', 'DiffusionSvnFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php', 'DiffusionSvnHistoryQuery' => 'applications/diffusion/query/history/DiffusionSvnHistoryQuery.php', - 'DiffusionSvnLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionSvnLastModifiedQuery.php', 'DiffusionSvnMergedCommitsQuery' => 'applications/diffusion/query/mergedcommits/DiffusionSvnMergedCommitsQuery.php', 'DiffusionSvnRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php', 'DiffusionSvnRequest' => 'applications/diffusion/request/DiffusionSvnRequest.php', @@ -1942,12 +1937,15 @@ phutil_register_library_map(array( 'ConduitAPI_diffusion_abstractquery_Method' => 'ConduitAPI_diffusion_Method', 'ConduitAPI_diffusion_branchquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', 'ConduitAPI_diffusion_browsequery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', + 'ConduitAPI_diffusion_diffquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', 'ConduitAPI_diffusion_existsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', 'ConduitAPI_diffusion_filecontentquery_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', 'ConduitAPI_diffusion_getrecentcommitsbypath_Method' => 'ConduitAPI_diffusion_Method', + 'ConduitAPI_diffusion_lastmodifiedquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', + 'ConduitAPI_diffusion_rawdiffquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', 'ConduitAPI_diffusion_tagsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', 'ConduitAPI_feed_Method' => 'ConduitAPIMethod', 'ConduitAPI_feed_publish_Method' => 'ConduitAPI_feed_Method', @@ -2210,17 +2208,14 @@ phutil_register_library_map(array( 'DiffusionContainsQuery' => 'DiffusionQuery', 'DiffusionController' => 'PhabricatorController', 'DiffusionDiffController' => 'DiffusionController', - 'DiffusionDiffQuery' => 'DiffusionQuery', 'DiffusionEmptyResultView' => 'DiffusionView', 'DiffusionExternalController' => 'DiffusionController', 'DiffusionFileContentQuery' => 'DiffusionQuery', 'DiffusionGitBranchTestCase' => 'PhabricatorTestCase', 'DiffusionGitCommitParentsQuery' => 'DiffusionCommitParentsQuery', 'DiffusionGitContainsQuery' => 'DiffusionContainsQuery', - 'DiffusionGitDiffQuery' => 'DiffusionDiffQuery', 'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionGitHistoryQuery' => 'DiffusionHistoryQuery', - 'DiffusionGitLastModifiedQuery' => 'DiffusionLastModifiedQuery', 'DiffusionGitMergedCommitsQuery' => 'DiffusionMergedCommitsQuery', 'DiffusionGitRawDiffQuery' => 'DiffusionRawDiffQuery', 'DiffusionGitRequest' => 'DiffusionRequest', @@ -2232,15 +2227,12 @@ phutil_register_library_map(array( 'DiffusionInlineCommentController' => 'PhabricatorInlineCommentController', 'DiffusionInlineCommentPreviewController' => 'PhabricatorInlineCommentPreviewController', 'DiffusionLastModifiedController' => 'DiffusionController', - 'DiffusionLastModifiedQuery' => 'DiffusionQuery', 'DiffusionLintController' => 'DiffusionController', 'DiffusionLintDetailsController' => 'DiffusionController', 'DiffusionMercurialCommitParentsQuery' => 'DiffusionCommitParentsQuery', 'DiffusionMercurialContainsQuery' => 'DiffusionContainsQuery', - 'DiffusionMercurialDiffQuery' => 'DiffusionDiffQuery', 'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionMercurialHistoryQuery' => 'DiffusionHistoryQuery', - 'DiffusionMercurialLastModifiedQuery' => 'DiffusionLastModifiedQuery', 'DiffusionMercurialMergedCommitsQuery' => 'DiffusionMergedCommitsQuery', 'DiffusionMercurialRawDiffQuery' => 'DiffusionRawDiffQuery', 'DiffusionMercurialRequest' => 'DiffusionRequest', @@ -2256,10 +2248,8 @@ phutil_register_library_map(array( 'DiffusionSetupException' => 'AphrontUsageException', 'DiffusionSvnCommitParentsQuery' => 'DiffusionCommitParentsQuery', 'DiffusionSvnContainsQuery' => 'DiffusionContainsQuery', - 'DiffusionSvnDiffQuery' => 'DiffusionDiffQuery', 'DiffusionSvnFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionSvnHistoryQuery' => 'DiffusionHistoryQuery', - 'DiffusionSvnLastModifiedQuery' => 'DiffusionLastModifiedQuery', 'DiffusionSvnMergedCommitsQuery' => 'DiffusionMergedCommitsQuery', 'DiffusionSvnRawDiffQuery' => 'DiffusionRawDiffQuery', 'DiffusionSvnRequest' => 'DiffusionRequest', diff --git a/src/applications/diffusion/query/diff/DiffusionSvnDiffQuery.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_diffquery_Method.php similarity index 50% rename from src/applications/diffusion/query/diff/DiffusionSvnDiffQuery.php rename to src/applications/diffusion/conduit/ConduitAPI_diffusion_diffquery_Method.php index a974344c67..055e2adf75 100644 --- a/src/applications/diffusion/query/diff/DiffusionSvnDiffQuery.php +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_diffquery_Method.php @@ -1,14 +1,57 @@ getRequest(); + private $effectiveCommit; + + public function getMethodDescription() { + return + 'Get diff information from a repository for a specific path at an '. + '(optional) commit.'; + } + + public function defineReturnType() { + return 'array'; + } + + protected function defineCustomParamTypes() { + return array( + 'path' => 'required string', + 'commit' => 'optional string', + ); + } + + protected function getResult(ConduitAPIRequest $request) { + $result = parent::getResult($request); + + return array( + 'changes' => mpull($result, 'toDictionary'), + 'effectiveCommit' => $this->getEffectiveCommit($request)); + } + + protected function getGitResult(ConduitAPIRequest $request) { + return $this->getGitOrMercurialResult($request); + } + + protected function getMercurialResult(ConduitAPIRequest $request) { + return $this->getGitOrMercurialResult($request); + } + + /** + * NOTE: We have to work particularly hard for SVN as compared to other VCS. + * That's okay but means this shares little code with the other VCS. + */ + protected function getSVNResult(ConduitAPIRequest $request) { + $drequest = $this->getDiffusionRequest(); $repository = $drequest->getRepository(); - $effective_commit = $this->getEffectiveCommit(); + $effective_commit = $this->getEffectiveCommit($request); if (!$effective_commit) { - return null; + return $this->getEmptyResult(); } $drequest = clone $drequest; @@ -26,7 +69,7 @@ final class DiffusionSvnDiffQuery extends DiffusionDiffQuery { } if (!$path) { - return null; + return $this->getEmptyResult(); } $change_type = $path->getChangeType(); @@ -76,8 +119,8 @@ final class DiffusionSvnDiffQuery extends DiffusionDiffQuery { } $futures = array( - 'old' => $this->buildContentFuture($old), - 'new' => $this->buildContentFuture($new), + 'old' => $this->buildSVNContentFuture($old), + 'new' => $this->buildSVNContentFuture($new), ); $futures = array_filter($futures); @@ -101,39 +144,51 @@ final class DiffusionSvnDiffQuery extends DiffusionDiffQuery { $engine->setNewName($new_name); $raw_diff = $engine->generateRawDiffFromFileContent($old_data, $new_data); - $parser = new ArcanistDiffParser(); - - $try_encoding = $repository->getDetail('encoding'); - if ($try_encoding) { - $parser->setTryEncoding($try_encoding); - } - - $parser->setDetectBinaryFiles(true); - $arcanist_changes = DiffusionPathChange::convertToArcanistChanges( $path_changes); + $parser = $this->getDefaultParser(); $parser->setChanges($arcanist_changes); $parser->forcePath($path->getPath()); $changes = $parser->parseDiff($raw_diff); $change = $changes[$path->getPath()]; - $diff = DifferentialDiff::newFromRawChanges(array($change)); - $changesets = $diff->getChangesets(); - $changeset = reset($changesets); - - $this->renderingReference = $drequest->getPath().';'.$drequest->getCommit(); - - return $changeset; + return array($change); } - private function buildContentFuture($spec) { + private function getEffectiveCommit(ConduitAPIRequest $request) { + if ($this->effectiveCommit === null) { + $drequest = $this->getDiffusionRequest(); + $user = $request->getUser(); + $commit = null; + + $conduit_result = DiffusionQuery::callConduitWithDiffusionRequest( + $user, + $drequest, + 'diffusion.lastmodifiedquery', + array( + 'commit' => $drequest->getCommit(), + 'path' => $drequest->getPath())); + $c_dict = $conduit_result['commit']; + if ($c_dict) { + $commit = PhabricatorRepositoryCommit::newFromDictionary($c_dict); + } + if (!$commit) { + // TODO: Improve error messages here. + return null; + } + $this->effectiveCommit = $commit->getCommitIdentifier(); + } + return $this->effectiveCommit; + } + + private function buildSVNContentFuture($spec) { if (!$spec) { return null; } - $drequest = $this->getRequest(); + $drequest = $this->getDiffusionRequest(); $repository = $drequest->getRepository(); list($ref, $rev) = $spec; @@ -144,4 +199,45 @@ final class DiffusionSvnDiffQuery extends DiffusionDiffQuery { $rev); } + private function getGitOrMercurialResult(ConduitAPIRequest $request) { + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + + $effective_commit = $this->getEffectiveCommit($request); + if (!$effective_commit) { + return $this->getEmptyResult(1); + } + // TODO: This side effect is kind of sketchy. + $drequest->setCommit($effective_commit); + + $raw_query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest); + $raw_diff = $raw_query->loadRawDiff(); + if (!$raw_diff) { + return $this->getEmptyResult(2); + } + + $parser = $this->getDefaultParser(); + $changes = $parser->parseDiff($raw_diff); + + return $changes; + } + + private function getDefaultParser() { + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + + $parser = new ArcanistDiffParser(); + $try_encoding = $repository->getDetail('encoding'); + if ($try_encoding) { + $parser->setTryEncoding($try_encoding); + } + $parser->setDetectBinaryFiles(true); + + return $parser; + } + + private function getEmptyResult() { + return array(); + } + } diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php new file mode 100644 index 0000000000..34f16b4183 --- /dev/null +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php @@ -0,0 +1,108 @@ + 'required string', + 'path' => 'required string', + ); + } + + protected function getResult(ConduitAPIRequest $request) { + list($commit, $commit_data) = parent::getResult($request); + if ($commit) { + $commit = $commit->toDictionary(); + } + if ($commit_data) { + $commit_data = $commit_data->toDictionary(); + } + return array( + 'commit' => $commit, + 'commitData' => $commit_data); + } + + protected function getGitResult(ConduitAPIRequest $request) { + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + + list($hash) = $repository->execxLocalCommand( + 'log -n1 --format=%%H %s -- %s', + $drequest->getCommit(), + $drequest->getPath()); + $hash = trim($hash); + + return $this->loadDataFromHash($hash); + } + + protected function getSVNResult(ConduitAPIRequest $request) { + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + + $path = $drequest->getPath(); + + $history_query = DiffusionHistoryQuery::newFromDiffusionRequest( + $drequest); + $history_query->setLimit(1); + $history_query->needChildChanges(true); + $history_query->needDirectChanges(true); + $history_array = $history_query->loadHistory(); + + if (!$history_array) { + return array(array(), array()); + } + + $history = reset($history_array); + + return array($history->getCommit(), $history->getCommitData()); + } + + protected function getMercurialResult(ConduitAPIRequest $request) { + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + + $path = $drequest->getPath(); + + list($hash) = $repository->execxLocalCommand( + 'log --template %s --limit 1 --removed --rev %s -- %s', + '{node}', + hgsprintf('reverse(%s::%s)', '0', $drequest->getCommit()), + nonempty(ltrim($path, '/'), '.')); + + return $this->loadDataFromHash($hash); + } + + private function loadDataFromHash($hash) { + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + + $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere( + 'repositoryID = %d AND commitIdentifier = %s', + $repository->getID(), + $hash); + + if ($commit) { + $commit_data = $commit->loadCommitData(); + } else { + $commit = array(); + $commit_data = array(); + } + + return array($commit, $commit_data); + } + +} diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_rawdiffquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_rawdiffquery_Method.php new file mode 100644 index 0000000000..63701277be --- /dev/null +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_rawdiffquery_Method.php @@ -0,0 +1,47 @@ + 'required string', + 'path' => 'optional string', + 'timeout' => 'optional int', + 'linesOfContext' => 'optional int', + 'againstCommit' => 'optional string', + ); + } + + protected function getResult(ConduitAPIRequest $request) { + $drequest = $this->getDiffusionRequest(); + $timeout = $request->getValue('timeout'); + $lines_of_context = $request->getValue('linesOfContext'); + $against_commit = $request->getValue('againstCommit'); + + $raw_query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest); + if ($timeout !== null) { + $raw_query->setTimeout($timeout); + } + if ($lines_of_context !== null) { + $raw_query->setLinesOfContext($lines_of_context); + } + if ($against_commit !== null) { + $raw_query->setAgainstCommit($against_commit); + } + return $raw_query->loadRawDiff(); + } +} diff --git a/src/applications/diffusion/controller/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/DiffusionBrowseFileController.php index 9615f9461b..f43cf6a76c 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseFileController.php @@ -927,31 +927,29 @@ final class DiffusionBrowseFileController extends DiffusionController { return null; } - $diff_query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest); - $diff_query->setAgainstCommit($target_commit); - try { - $raw_diff = $diff_query->loadRawDiff(); - $old_line = 0; - $new_line = 0; + $raw_diff = $this->callConduitWithDiffusionRequest( + 'diffusion.rawdiffquery', + array( + 'commit' => $drequest->getCommit(), + 'path' => $drequest->getPath(), + 'againstCommit' => $target_commit)); + $old_line = 0; + $new_line = 0; - foreach (explode("\n", $raw_diff) as $text) { - if ($text[0] == '-' || $text[0] == ' ') { - $old_line++; - } - if ($text[0] == '+' || $text[0] == ' ') { - $new_line++; - } - if ($new_line == $line) { - return $old_line; - } + foreach (explode("\n", $raw_diff) as $text) { + if ($text[0] == '-' || $text[0] == ' ') { + $old_line++; + } + if ($text[0] == '+' || $text[0] == ' ') { + $new_line++; + } + if ($new_line == $line) { + return $old_line; } - - // We didn't find the target line. - return $line; - - } catch (Exception $ex) { - return $line; } + + // We didn't find the target line. + return $line; } private function loadParentRevisionOf($commit) { diff --git a/src/applications/diffusion/controller/DiffusionChangeController.php b/src/applications/diffusion/controller/DiffusionChangeController.php index e0c940cc0a..0e3aa40ab4 100644 --- a/src/applications/diffusion/controller/DiffusionChangeController.php +++ b/src/applications/diffusion/controller/DiffusionChangeController.php @@ -7,8 +7,16 @@ final class DiffusionChangeController extends DiffusionController { $content = array(); - $diff_query = DiffusionDiffQuery::newFromDiffusionRequest($drequest); - $changeset = $diff_query->loadChangeset(); + $data = $this->callConduitWithDiffusionRequest( + 'diffusion.diffquery', + array( + 'commit' => $drequest->getCommit(), + 'path' => $drequest->getPath())); + $drequest->setCommit($data['effectiveCommit']); + $raw_changes = ArcanistDiffChange::newFromConduit($data['changes']); + $diff = DifferentialDiff::newFromRawChanges($raw_changes); + $changesets = $diff->getChangesets(); + $changeset = reset($changesets); if (!$changeset) { // TODO: Refine this. @@ -28,7 +36,7 @@ final class DiffusionChangeController extends DiffusionController { $changeset_view->setVisibleChangesets($changesets); $changeset_view->setRenderingReferences( array( - 0 => $diff_query->getRenderingReference(), + 0 => $drequest->generateURI(array('action' => 'rendering-ref')) )); $raw_params = array( diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index eef27a36a1..641690b627 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -943,8 +943,11 @@ final class DiffusionCommitController extends DiffusionController { } private function buildRawDiffResponse(DiffusionRequest $drequest) { - $raw_query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest); - $raw_diff = $raw_query->loadRawDiff(); + $raw_diff = $this->callConduitWithDiffusionRequest( + 'diffusion.rawdiffquery', + array( + 'commit' => $drequest->getCommit(), + 'path' => $drequest->getPath())); $file = PhabricatorFile::buildFromFileDataOrHash( $raw_diff, diff --git a/src/applications/diffusion/controller/DiffusionDiffController.php b/src/applications/diffusion/controller/DiffusionDiffController.php index 5d129cd274..2e98ef2175 100644 --- a/src/applications/diffusion/controller/DiffusionDiffController.php +++ b/src/applications/diffusion/controller/DiffusionDiffController.php @@ -42,19 +42,27 @@ final class DiffusionDiffController extends DiffusionController { return id(new AphrontRedirectResponse())->setURI($uri); } - - $diff_query = DiffusionDiffQuery::newFromDiffusionRequest($drequest); - $changeset = $diff_query->loadChangeset(); + $data = $this->callConduitWithDiffusionRequest( + 'diffusion.diffquery', + array( + 'commit' => $drequest->getCommit(), + 'path' => $drequest->getPath())); + $drequest->setCommit($data['effectiveCommit']); + $raw_changes = ArcanistDiffChange::newFromConduit($data['changes']); + $diff = DifferentialDiff::newFromRawChanges($raw_changes); + $changesets = $diff->getChangesets(); + $changeset = reset($changesets); if (!$changeset) { return new Aphront404Response(); } - $parser = new DifferentialChangesetParser(); $parser->setUser($user); $parser->setChangeset($changeset); - $parser->setRenderingReference($diff_query->getRenderingReference()); + $parser->setRenderingReference($drequest->generateURI( + array( + 'action' => 'rendering-ref'))); $pquery = new DiffusionPathIDQuery(array($changeset->getFilename())); $ids = $pquery->loadPathIDs(); diff --git a/src/applications/diffusion/controller/DiffusionLastModifiedController.php b/src/applications/diffusion/controller/DiffusionLastModifiedController.php index eece88418a..d8f96a6758 100644 --- a/src/applications/diffusion/controller/DiffusionLastModifiedController.php +++ b/src/applications/diffusion/controller/DiffusionLastModifiedController.php @@ -5,10 +5,24 @@ final class DiffusionLastModifiedController extends DiffusionController { public function processRequest() { $drequest = $this->getDiffusionRequest(); $request = $this->getRequest(); + $commit = null; + $commit_data = null; - $modified_query = DiffusionLastModifiedQuery::newFromDiffusionRequest( - $drequest); - list($commit, $commit_data) = $modified_query->loadLastModification(); + $conduit_result = $this->callConduitWithDiffusionRequest( + 'diffusion.lastmodifiedquery', + array( + 'commit' => $drequest->getCommit(), + 'path' => $drequest->getPath() + )); + $c_dict = $conduit_result['commit']; + if ($c_dict) { + $commit = PhabricatorRepositoryCommit::newFromDictionary($c_dict); + } + $c_d_dict = $conduit_result['commitData']; + if ($c_d_dict) { + $commit_data = + PhabricatorRepositoryCommitData::newFromDictionary($c_d_dict); + } $phids = array(); if ($commit_data) { diff --git a/src/applications/diffusion/query/diff/DiffusionDiffQuery.php b/src/applications/diffusion/query/diff/DiffusionDiffQuery.php deleted file mode 100644 index 804d8d3e09..0000000000 --- a/src/applications/diffusion/query/diff/DiffusionDiffQuery.php +++ /dev/null @@ -1,33 +0,0 @@ -renderingReference; - } - - final public function loadChangeset() { - return $this->executeQuery(); - } - - protected function getEffectiveCommit() { - $drequest = $this->getRequest(); - - $modified_query = DiffusionLastModifiedQuery::newFromDiffusionRequest( - $drequest); - list($commit) = $modified_query->loadLastModification(); - if (!$commit) { - // TODO: Improve error messages here. - return null; - } - return $commit->getCommitIdentifier(); - } - -} diff --git a/src/applications/diffusion/query/diff/DiffusionGitDiffQuery.php b/src/applications/diffusion/query/diff/DiffusionGitDiffQuery.php deleted file mode 100644 index 51c5d19bc1..0000000000 --- a/src/applications/diffusion/query/diff/DiffusionGitDiffQuery.php +++ /dev/null @@ -1,45 +0,0 @@ -getRequest(); - $repository = $drequest->getRepository(); - - $effective_commit = $this->getEffectiveCommit(); - if (!$effective_commit) { - return null; - } - // TODO: This side effect is kind of sketchy. - $drequest->setCommit($effective_commit); - - $raw_query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest); - $raw_diff = $raw_query->loadRawDiff(); - - if (!$raw_diff) { - return null; - } - - $parser = new ArcanistDiffParser(); - - $try_encoding = $repository->getDetail('encoding'); - if ($try_encoding) { - $parser->setTryEncoding($try_encoding); - } - - $parser->setDetectBinaryFiles(true); - $changes = $parser->parseDiff($raw_diff); - - $diff = DifferentialDiff::newFromRawChanges($changes); - $changesets = $diff->getChangesets(); - $changeset = reset($changesets); - - $this->renderingReference = $drequest->generateURI( - array( - 'action' => 'rendering-ref', - )); - - return $changeset; - } - -} diff --git a/src/applications/diffusion/query/diff/DiffusionMercurialDiffQuery.php b/src/applications/diffusion/query/diff/DiffusionMercurialDiffQuery.php deleted file mode 100644 index ba1e32863d..0000000000 --- a/src/applications/diffusion/query/diff/DiffusionMercurialDiffQuery.php +++ /dev/null @@ -1,41 +0,0 @@ -getRequest(); - $repository = $drequest->getRepository(); - - $effective_commit = $this->getEffectiveCommit(); - if (!$effective_commit) { - return null; - } - // TODO: This side effect is kind of skethcy. - $drequest->setCommit($effective_commit); - - $query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest); - $raw_diff = $query->loadRawDiff(); - - $parser = new ArcanistDiffParser(); - - $try_encoding = $repository->getDetail('encoding'); - if ($try_encoding) { - $parser->setTryEncoding($try_encoding); - } - - $parser->setDetectBinaryFiles(true); - $changes = $parser->parseDiff($raw_diff); - - $diff = DifferentialDiff::newFromRawChanges($changes); - $changesets = $diff->getChangesets(); - $changeset = reset($changesets); - - $this->renderingReference = $drequest->generateURI( - array( - 'action' => 'rendering-ref', - )); - - return $changeset; - } - -} diff --git a/src/applications/diffusion/query/lastmodified/DiffusionGitLastModifiedQuery.php b/src/applications/diffusion/query/lastmodified/DiffusionGitLastModifiedQuery.php deleted file mode 100644 index 70a87a8cfb..0000000000 --- a/src/applications/diffusion/query/lastmodified/DiffusionGitLastModifiedQuery.php +++ /dev/null @@ -1,29 +0,0 @@ -getRequest(); - $repository = $drequest->getRepository(); - - list($hash) = $repository->execxLocalCommand( - 'log -n1 --format=%%H %s -- %s', - $drequest->getCommit(), - $drequest->getPath()); - $hash = trim($hash); - - $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere( - 'repositoryID = %d AND commitIdentifier = %s', - $repository->getID(), - $hash); - - if ($commit) { - $commit_data = $commit->loadCommitData(); - } else { - $commit_data = null; - } - - return array($commit, $commit_data); - } - -} diff --git a/src/applications/diffusion/query/lastmodified/DiffusionLastModifiedQuery.php b/src/applications/diffusion/query/lastmodified/DiffusionLastModifiedQuery.php deleted file mode 100644 index dc649cdc58..0000000000 --- a/src/applications/diffusion/query/lastmodified/DiffusionLastModifiedQuery.php +++ /dev/null @@ -1,14 +0,0 @@ -executeQuery(); - } - -} diff --git a/src/applications/diffusion/query/lastmodified/DiffusionMercurialLastModifiedQuery.php b/src/applications/diffusion/query/lastmodified/DiffusionMercurialLastModifiedQuery.php deleted file mode 100644 index 4a7c89f223..0000000000 --- a/src/applications/diffusion/query/lastmodified/DiffusionMercurialLastModifiedQuery.php +++ /dev/null @@ -1,32 +0,0 @@ -getRequest(); - $repository = $drequest->getRepository(); - - $path = $drequest->getPath(); - - list($hash) = $repository->execxLocalCommand( - 'log --template %s --limit 1 --removed --rev %s -- %s', - '{node}', - hgsprintf('reverse(%s::%s)', '0', $drequest->getCommit()), - nonempty(ltrim($path, '/'), '.')); - - $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere( - 'repositoryID = %d AND commitIdentifier = %s', - $repository->getID(), - $hash); - - if ($commit) { - $commit_data = $commit->loadCommitData(); - } else { - $commit_data = null; - } - - return array($commit, $commit_data); - } - -} diff --git a/src/applications/diffusion/query/lastmodified/DiffusionSvnLastModifiedQuery.php b/src/applications/diffusion/query/lastmodified/DiffusionSvnLastModifiedQuery.php deleted file mode 100644 index 5140eb50f2..0000000000 --- a/src/applications/diffusion/query/lastmodified/DiffusionSvnLastModifiedQuery.php +++ /dev/null @@ -1,27 +0,0 @@ -getRequest(); - $repository = $drequest->getRepository(); - - $path = $drequest->getPath(); - - $history_query = DiffusionHistoryQuery::newFromDiffusionRequest( - $drequest); - $history_query->setLimit(1); - $history_query->needChildChanges(true); - $history_query->needDirectChanges(true); - $history_array = $history_query->loadHistory(); - - if (!$history_array) { - return array(null, null); - } - - $history = reset($history_array); - - return array($history->getCommit(), $history->getCommitData()); - } - -} diff --git a/src/applications/herald/adapter/HeraldCommitAdapter.php b/src/applications/herald/adapter/HeraldCommitAdapter.php index 5dee9ec0f6..543221eb63 100644 --- a/src/applications/herald/adapter/HeraldCommitAdapter.php +++ b/src/applications/herald/adapter/HeraldCommitAdapter.php @@ -110,10 +110,14 @@ final class HeraldCommitAdapter extends HeraldObjectAdapter { 'commit' => $this->commit->getCommitIdentifier(), )); - $raw = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest) - ->setTimeout(60 * 60 * 15) - ->setLinesOfContext(0) - ->loadRawDiff(); + $raw = DiffusionQuery::callConduitWithDiffusionRequest( + $drequest, + PhabricatorUser::getOmnipotentUser(), + 'diffusion.rawdiffquery', + array( + 'commit' => $this->commit->getCommitIdentifier(), + 'timeout' => 60 * 60 * 15, + 'linesOfContext' => 0)); $parser = new ArcanistDiffParser(); $changes = $parser->parseDiff($raw);