From 63e39cef328bfc92c9fef23d557b368dcde798c3 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Fri, 17 May 2013 14:11:20 -0700 Subject: [PATCH] Diffusion - move search (grep) system commands to happen over Conduit Summary: Ref T2784. Test Plan: loaded up my git and mercurial copies of Phabricator. Searched for "diff". Observed many results and pagination working correctly. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2784 Differential Revision: https://secure.phabricator.com/D5955 --- src/__phutil_library_map__.php | 2 + ...onduitAPI_diffusion_searchquery_Method.php | 117 ++++++++++++++++++ .../controller/DiffusionBrowseController.php | 70 ++--------- 3 files changed, 131 insertions(+), 58 deletions(-) create mode 100644 src/applications/diffusion/conduit/ConduitAPI_diffusion_searchquery_Method.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 846e54efc7..1a18c0cba6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -157,6 +157,7 @@ phutil_register_library_map(array( '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_searchquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_searchquery_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', @@ -1955,6 +1956,7 @@ phutil_register_library_map(array( '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_searchquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', 'ConduitAPI_diffusion_tagsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', 'ConduitAPI_feed_Method' => 'ConduitAPIMethod', 'ConduitAPI_feed_publish_Method' => 'ConduitAPI_feed_Method', diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_searchquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_searchquery_Method.php new file mode 100644 index 0000000000..0c4a1016b5 --- /dev/null +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_searchquery_Method.php @@ -0,0 +1,117 @@ + 'required string', + 'stableCommitName' => 'required string', + 'grep' => 'required string', + 'limit' => 'optional int', + 'offset' => 'optional int', + ); + } + + protected function defineCustomErrorTypes() { + return array( + 'ERR-GREP-COMMAND' => 'Grep command failed.'); + } + + protected function getResult(ConduitAPIRequest $request) { + try { + $results = parent::getResult($request); + } catch (CommandException $ex) { + throw id(new ConduitException('ERR-GREP-COMMAND')) + ->setErrorDescription($ex->getStderr()); + } + + $offset = $request->getValue('offset'); + $results = array_slice($results, $offset); + + return $results; + } + + protected function getGitResult(ConduitAPIRequest $request) { + $drequest = $this->getDiffusionRequest(); + $path = $drequest->getPath(); + $stable_commit_name = $request->getValue('stableCommitName'); + $grep = $request->getValue('grep'); + $repository = $drequest->getRepository(); + $limit = $request->getValue('limit'); + $offset = $request->getValue('offset'); + + $results = array(); + $future = $repository->getLocalCommandFuture( + // NOTE: --perl-regexp is available only with libpcre compiled in. + 'grep --extended-regexp --null -n --no-color -e %s %s -- %s', + $grep, + $stable_commit_name, + $path); + + $binary_pattern = '/Binary file [^:]*:(.+) matches/'; + $lines = new LinesOfALargeExecFuture($future); + foreach ($lines as $line) { + $result = null; + if (preg_match('/[^:]*:(.+)\0(.+)\0(.*)/', $line, $result)) { + $results[] = array_slice($result, 1); + } else if (preg_match($binary_pattern, $line, $result)) { + list(, $path) = $result; + $results[] = array($path, null, pht('Binary file')); + } else { + $results[] = array(null, null, $line); + } + if (count($results) >= $offset + $limit) { + break; + } + } + unset($lines); + + return $results; + } + + protected function getMercurialResult(ConduitAPIRequest $request) { + $drequest = $this->getDiffusionRequest(); + $path = $drequest->getPath(); + $stable_commit_name = $request->getValue('stableCommitName'); + $grep = $request->getValue('grep'); + $repository = $drequest->getRepository(); + $limit = $request->getValue('limit'); + $offset = $request->getValue('offset'); + + $results = array(); + $future = $repository->getLocalCommandFuture( + 'grep --rev %s --print0 --line-number %s %s', + hgsprintf('ancestors(%s)', $stable_commit_name), + $grep, + $path); + + $lines = id(new LinesOfALargeExecFuture($future))->setDelimiter("\0"); + $parts = array(); + foreach ($lines as $line) { + $parts[] = $line; + if (count($parts) == 4) { + list($path, $char_offset, $line, $string) = $parts; + $results[] = array($path, $line, $string); + if (count($results) >= $offset + $limit) { + break; + } + $parts = array(); + } + } + unset($lines); + + return $results; + } +} diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index afb2b9035e..91d0cb7563 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -171,70 +171,24 @@ final class DiffusionBrowseController extends DiffusionController { try { - switch ($repository->getVersionControlSystem()) { - case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: - $future = $repository->getLocalCommandFuture( - // NOTE: --perl-regexp is available only with libpcre compiled in. - 'grep --extended-regexp --null -n --no-color -e %s %s -- %s', - $this->getRequest()->getStr('grep'), - $drequest->getStableCommitName(), - $drequest->getPath()); + $results = $this->callConduitWithDiffusionRequest( + 'diffusion.searchquery', + array( + 'grep' => $this->getRequest()->getStr('grep'), + 'stableCommitName' => $drequest->getStableCommitName(), + 'path' => $drequest->getPath(), + 'limit' => $limit + 1, + 'offset' => $page)); - $binary_pattern = '/Binary file [^:]*:(.+) matches/'; - $lines = new LinesOfALargeExecFuture($future); - foreach ($lines as $line) { - $result = null; - if (preg_match('/[^:]*:(.+)\0(.+)\0(.*)/', $line, $result)) { - $results[] = array_slice($result, 1); - } else if (preg_match($binary_pattern, $line, $result)) { - list(, $path) = $result; - $results[] = array($path, null, pht('Binary file')); - } else { - $results[] = array(null, null, $line); - } - if (count($results) > $page + $limit) { - break; - } - } - unset($lines); - - break; - - case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: - $future = $repository->getLocalCommandFuture( - 'grep --rev %s --print0 --line-number %s %s', - hgsprintf('ancestors(%s)', $drequest->getStableCommitName()), - $this->getRequest()->getStr('grep'), - $drequest->getPath()); - - $lines = id(new LinesOfALargeExecFuture($future))->setDelimiter("\0"); - $parts = array(); - foreach ($lines as $line) { - $parts[] = $line; - if (count($parts) == 4) { - list($path, $offset, $line, $string) = $parts; - $results[] = array($path, $line, $string); - if (count($results) > $page + $limit) { - break; - } - $parts = array(); - } - } - unset($lines); - - break; - } - - } catch (CommandException $ex) { - $stderr = $ex->getStderr(); - if ($stderr != '') { + } catch (ConduitException $ex) { + $err = $ex->getErrorDescription(); + if ($err != '') { return id(new AphrontErrorView()) ->setTitle(pht('Search Error')) - ->appendChild($stderr); + ->appendChild($err); } } - $results = array_slice($results, $page); $results = $pager->sliceResults($results); require_celerity_resource('syntax-highlighting-css');