From a48128d36fc688a2982cec339c107e3d1b8688fe Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 3 Feb 2014 12:51:08 -0800 Subject: [PATCH] Partially use ApplicationSearch in main search Summary: Ref T4365. Primary search currently uses `PhabricatorSearchQuery` for storage, which is pretty much the same as `PhabricatorSavedQuery`, except that it's old and not used anywhere else anymore. Maniphest used to also use this table, but no longer does after Septmeber, 2013. We need to retain the class so the migration can work. This introduces `PhabricatorSearchApplicationSearchEngine` and `PhabricatorSearchDocumentQuery`, but they're both stubs that I just needed for technical reasons and/or to pass lint. The next couple patches will move logic into them and use ApplicationSearch properly. Test Plan: - Searched for stuff. - Searched for stuff with filters. - Searched for fulltext in Maniphest. - Grepped for `PhabricatorSearchQuery`. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4365 Differential Revision: https://secure.phabricator.com/D8120 --- src/__phutil_library_map__.php | 4 ++ .../PhabricatorHomeMainController.php | 7 +-- .../maniphest/query/ManiphestTaskQuery.php | 5 +- .../PhabricatorSearchController.php | 17 ++++--- .../PhabricatorSearchSelectController.php | 5 +- .../search/engine/PhabricatorSearchEngine.php | 6 +-- .../engine/PhabricatorSearchEngineElastic.php | 19 ++++--- .../engine/PhabricatorSearchEngineMySQL.php | 6 +-- ...abricatorSearchApplicationSearchEngine.php | 49 +++++++++++++++++++ .../query/PhabricatorSearchDocumentQuery.php | 14 ++++++ .../search/storage/PhabricatorSearchQuery.php | 13 ++++- .../view/PhabricatorSearchResultView.php | 7 +-- 12 files changed, 116 insertions(+), 36 deletions(-) create mode 100644 src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php create mode 100644 src/applications/search/query/PhabricatorSearchDocumentQuery.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d345617171..654ac6ee69 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1941,6 +1941,7 @@ phutil_register_library_map(array( 'PhabricatorSavedQueryQuery' => 'applications/search/query/PhabricatorSavedQueryQuery.php', 'PhabricatorScopedEnv' => 'infrastructure/env/PhabricatorScopedEnv.php', 'PhabricatorSearchAbstractDocument' => 'applications/search/index/PhabricatorSearchAbstractDocument.php', + 'PhabricatorSearchApplicationSearchEngine' => 'applications/search/query/PhabricatorSearchApplicationSearchEngine.php', 'PhabricatorSearchAttachController' => 'applications/search/controller/PhabricatorSearchAttachController.php', 'PhabricatorSearchBaseController' => 'applications/search/controller/PhabricatorSearchBaseController.php', 'PhabricatorSearchConfigOptions' => 'applications/search/config/PhabricatorSearchConfigOptions.php', @@ -1950,6 +1951,7 @@ phutil_register_library_map(array( 'PhabricatorSearchDocument' => 'applications/search/storage/document/PhabricatorSearchDocument.php', 'PhabricatorSearchDocumentField' => 'applications/search/storage/document/PhabricatorSearchDocumentField.php', 'PhabricatorSearchDocumentIndexer' => 'applications/search/index/PhabricatorSearchDocumentIndexer.php', + 'PhabricatorSearchDocumentQuery' => 'applications/search/query/PhabricatorSearchDocumentQuery.php', 'PhabricatorSearchDocumentRelationship' => 'applications/search/storage/document/PhabricatorSearchDocumentRelationship.php', 'PhabricatorSearchEditController' => 'applications/search/controller/PhabricatorSearchEditController.php', 'PhabricatorSearchEngine' => 'applications/search/engine/PhabricatorSearchEngine.php', @@ -4686,6 +4688,7 @@ phutil_register_library_map(array( 1 => 'PhabricatorPolicyInterface', ), 'PhabricatorSavedQueryQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorSearchApplicationSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorSearchAttachController' => 'PhabricatorSearchBaseController', 'PhabricatorSearchBaseController' => 'PhabricatorController', 'PhabricatorSearchConfigOptions' => 'PhabricatorApplicationConfigOptions', @@ -4694,6 +4697,7 @@ phutil_register_library_map(array( 'PhabricatorSearchDeleteController' => 'PhabricatorSearchBaseController', 'PhabricatorSearchDocument' => 'PhabricatorSearchDAO', 'PhabricatorSearchDocumentField' => 'PhabricatorSearchDAO', + 'PhabricatorSearchDocumentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorSearchDocumentRelationship' => 'PhabricatorSearchDAO', 'PhabricatorSearchEditController' => 'PhabricatorSearchBaseController', 'PhabricatorSearchEngineElastic' => 'PhabricatorSearchEngine', diff --git a/src/applications/home/controller/PhabricatorHomeMainController.php b/src/applications/home/controller/PhabricatorHomeMainController.php index 376f2014a1..f15dd6edec 100644 --- a/src/applications/home/controller/PhabricatorHomeMainController.php +++ b/src/applications/home/controller/PhabricatorHomeMainController.php @@ -98,9 +98,10 @@ final class PhabricatorHomeMainController return $response; } else if ($request->isFormPost()) { - $query = new PhabricatorSearchQuery(); - $query->setQuery($jump); - $query->save(); + $query = id(new PhabricatorSavedQuery()) + ->setEngineClassName('PhabricatorSearchApplicationSearchEngine') + ->setParameter('query', $jump) + ->save(); return id(new AphrontRedirectResponse()) ->setURI('/search/'.$query->getQueryKey().'/'); diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php index 82aebd05fb..4126dafe86 100644 --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -406,8 +406,9 @@ final class ManiphestTaskQuery // In doing a fulltext search, we first find all the PHIDs that match the // fulltext search, and then use that to limit the rest of the search - $fulltext_query = new PhabricatorSearchQuery(); - $fulltext_query->setQuery($this->fullTextSearch); + $fulltext_query = id(new PhabricatorSavedQuery()) + ->setEngineClassName('PhabricatorSearchApplicaionSearchEngine') + ->setParameter('query', $this->fullTextSearch); // NOTE: Setting this to something larger than 2^53 will raise errors in // ElasticSearch, and billions of results won't fit in memory anyway. diff --git a/src/applications/search/controller/PhabricatorSearchController.php b/src/applications/search/controller/PhabricatorSearchController.php index 02f615c380..e570f745bf 100644 --- a/src/applications/search/controller/PhabricatorSearchController.php +++ b/src/applications/search/controller/PhabricatorSearchController.php @@ -21,14 +21,15 @@ final class PhabricatorSearchController $user = $request->getUser(); if ($this->key) { - $query = id(new PhabricatorSearchQuery())->loadOneWhere( + $query = id(new PhabricatorSavedQuery())->loadOneWhere( 'queryKey = %s', $this->key); if (!$query) { return new Aphront404Response(); } } else { - $query = new PhabricatorSearchQuery(); + $query = id(new PhabricatorSavedQuery()) + ->setEngineClassName('PhabricatorSearchApplicationSearchEngine'); if ($request->isFormPost()) { $query_str = $request->getStr('query'); @@ -45,7 +46,7 @@ final class PhabricatorSearchController if ($response) { return $response; } else { - $query->setQuery($query_str); + $query->setParameter('query', $query_str); if ($request->getStr('scope')) { switch ($request->getStr('scope')) { @@ -101,7 +102,11 @@ final class PhabricatorSearchController } } - $query->save(); + try { + $query->save(); + } catch (AphrontQueryDuplicateKeyException $ex) { + // Someone has already executed this query. + } return id(new AphrontRedirectResponse()) ->setURI('/search/'.$query->getQueryKey().'/'); } @@ -157,7 +162,7 @@ final class PhabricatorSearchController id(new AphrontFormTextControl()) ->setLabel('Search') ->setName('query') - ->setValue($query->getQuery())) + ->setValue($query->getParameter('query'))) ->appendChild( id(new AphrontFormSelectControl()) ->setLabel('Document Type') @@ -227,7 +232,7 @@ final class PhabricatorSearchController if (!$request->getInt('page')) { $named = id(new PhabricatorObjectQuery()) ->setViewer($user) - ->withNames(array($query->getQuery())) + ->withNames(array($query->getParameter('queyr'))) ->execute(); if ($named) { $results = array_merge(array_keys($named), $results); diff --git a/src/applications/search/controller/PhabricatorSearchSelectController.php b/src/applications/search/controller/PhabricatorSearchSelectController.php index f0345d2b7d..69af860131 100644 --- a/src/applications/search/controller/PhabricatorSearchSelectController.php +++ b/src/applications/search/controller/PhabricatorSearchSelectController.php @@ -16,10 +16,11 @@ final class PhabricatorSearchSelectController $request = $this->getRequest(); $user = $request->getUser(); - $query = new PhabricatorSearchQuery(); + $query = new PhabricatorSavedQuery(); $query_str = $request->getStr('query'); - $query->setQuery($query_str); + $query->setEngineClassName('PhabricatorSearchApplicationSearchEngine'); + $query->setParameter('query', $query_str); $query->setParameter('type', $this->type); switch ($request->getStr('filter')) { diff --git a/src/applications/search/engine/PhabricatorSearchEngine.php b/src/applications/search/engine/PhabricatorSearchEngine.php index 48b765a9d3..772f4d6770 100644 --- a/src/applications/search/engine/PhabricatorSearchEngine.php +++ b/src/applications/search/engine/PhabricatorSearchEngine.php @@ -5,8 +5,6 @@ * Base class for Phabricator search engine providers. Each engine must offer * three capabilities: indexing, searching, and reconstruction (this can be * stubbed out if an engine can't reasonably do it, it is used for debugging). - * - * @group search */ abstract class PhabricatorSearchEngine { @@ -33,9 +31,9 @@ abstract class PhabricatorSearchEngine { /** * Execute a search query. * - * @param PhabricatorSearchQuery A query to execute. + * @param PhabricatorSavedQuery A query to execute. * @return list A list of matching PHIDs. */ - abstract public function executeSearch(PhabricatorSearchQuery $query); + abstract public function executeSearch(PhabricatorSavedQuery $query); } diff --git a/src/applications/search/engine/PhabricatorSearchEngineElastic.php b/src/applications/search/engine/PhabricatorSearchEngineElastic.php index 5b0f9aeaa0..ad4072e62b 100644 --- a/src/applications/search/engine/PhabricatorSearchEngineElastic.php +++ b/src/applications/search/engine/PhabricatorSearchEngineElastic.php @@ -1,8 +1,5 @@ getQuery() != '') { + if ($query->getParameter('query') != '') { $spec[] = array( 'field' => array( - 'field.corpus' => $query->getQuery(), + 'field.corpus' => $query->getParameter('query'), ), ); } @@ -167,7 +164,7 @@ final class PhabricatorSearchEngineElastic extends PhabricatorSearchEngine { ); } - if (!$query->getQuery()) { + if (!$query->getParameter('query')) { $spec['sort'] = array( array('dateCreated' => 'desc'), ); @@ -179,7 +176,7 @@ final class PhabricatorSearchEngineElastic extends PhabricatorSearchEngine { return $spec; } - public function executeSearch(PhabricatorSearchQuery $query) { + public function executeSearch(PhabricatorSavedQuery $query) { $type = $query->getParameter('type'); if ($type) { $uri = "/phabricator/{$type}/_search"; @@ -197,11 +194,13 @@ final class PhabricatorSearchEngineElastic extends PhabricatorSearchEngine { // elasticsearch probably uses Lucene query syntax: // http://lucene.apache.org/core/3_6_1/queryparsersyntax.html // Try literal search if operator search fails. - if (!$query->getQuery()) { + if (!$query->getParameter('query')) { throw $ex; } $query = clone $query; - $query->setQuery(addcslashes($query->getQuery(), '+-&|!(){}[]^"~*?:\\')); + $query->setQuery( + addcslashes( + $query->getParameter('query'), '+-&|!(){}[]^"~*?:\\')); $response = $this->executeRequest($uri, $this->buildSpec($query)); } diff --git a/src/applications/search/engine/PhabricatorSearchEngineMySQL.php b/src/applications/search/engine/PhabricatorSearchEngineMySQL.php index 65f3b6b4da..80aea6b94b 100644 --- a/src/applications/search/engine/PhabricatorSearchEngineMySQL.php +++ b/src/applications/search/engine/PhabricatorSearchEngineMySQL.php @@ -142,7 +142,7 @@ final class PhabricatorSearchEngineMySQL extends PhabricatorSearchEngine { return $adoc; } - public function executeSearch(PhabricatorSearchQuery $query) { + public function executeSearch(PhabricatorSavedQuery $query) { $where = array(); $join = array(); @@ -156,7 +156,7 @@ final class PhabricatorSearchEngineMySQL extends PhabricatorSearchEngine { $conn_r = $dao_doc->establishConnection('r'); - $q = $query->getQuery(); + $q = $query->getParameter('query'); if (strlen($q)) { $join[] = qsprintf( @@ -281,7 +281,7 @@ final class PhabricatorSearchEngineMySQL extends PhabricatorSearchEngine { protected function joinRelationship( AphrontDatabaseConnection $conn, - PhabricatorSearchQuery $query, + PhabricatorSavedQuery $query, $field, $type) { diff --git a/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php b/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php new file mode 100644 index 0000000000..dc08ed21ae --- /dev/null +++ b/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php @@ -0,0 +1,49 @@ + pht('All Documents'), + ); + + return $names; + } + + public function buildSavedQueryFromBuiltin($query_key) { + + $query = $this->newSavedQuery(); + $query->setQueryKey($query_key); + + switch ($query_key) { + case 'all': + return $query; + } + + return parent::buildSavedQueryFromBuiltin($query_key); + } + +} diff --git a/src/applications/search/query/PhabricatorSearchDocumentQuery.php b/src/applications/search/query/PhabricatorSearchDocumentQuery.php new file mode 100644 index 0000000000..b5a6e4cb8d --- /dev/null +++ b/src/applications/search/query/PhabricatorSearchDocumentQuery.php @@ -0,0 +1,14 @@ +query = $query; return $this; } @@ -84,7 +81,7 @@ final class PhabricatorSearchResultView extends AphrontView { return $str; } - $query = $this->query->getQuery(); + $query = $this->query->getParameter('query'); $quoted_regexp = '/"([^"]*)"/'; $matches = array(1 => array());