From f1c75a63823c5cead8aefd0479a6faa8a7f4dba5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 29 Aug 2013 11:52:29 -0700 Subject: [PATCH] Allow construction of ApplicationSearch queries with GET Summary: Ref T3775 (discussion here). Ref T2625. T3775 presents two problems: # Existing tools which linked to `/differential/active/epriestley/` (that is, put a username in the URL) can't generate search links now. # Humans can't edit the URL anymore, either. I think (1) is an actual issue, and this fixes it. I think (2) is pretty fluff, and this doesn't really try to fix it, although it probably improves it. The fix for (1) is: - Provide a helper to read a parameter containing either a list of user PHIDs or a list of usernames, so `/?users[]=PHID-USER-xyz` (from a tokenizer) and `/?users=alincoln,htaft` (from an external program) are equivalent inputs. - Rename all the form parameters to be more digestable (`authorPHIDs` -> `authors`). Almost all of them were in this form already anyway. This just gives us `?users=alincoln` instead of `userPHIDs=alincoln`. - Inside ApplicationSearch, if a request has no query associated with it but does have query parameters, build a query from the request instead of issuing the user's default query. Basically, this means that `/differential/` runs the default query, while `/differential/?users=x` runs a custom query. Test Plan: {F56612} Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2625, T3775 Differential Revision: https://secure.phabricator.com/D6840 --- src/aphront/AphrontRequest.php | 6 +- .../PhabricatorCountdownSearchEngine.php | 2 +- .../DifferentialRevisionSearchEngine.php | 16 +++--- .../query/PhabricatorFeedSearchEngine.php | 4 +- .../query/PhabricatorFileSearchEngine.php | 2 +- .../herald/query/HeraldRuleSearchEngine.php | 2 +- .../query/LegalpadDocumentSearchEngine.php | 4 +- .../query/PhabricatorMacroSearchEngine.php | 2 +- .../query/PhabricatorPasteSearchEngine.php | 2 +- .../pholio/query/PholioMockSearchEngine.php | 2 +- .../query/PonderQuestionSearchEngine.php | 4 +- .../query/PhabricatorProjectSearchEngine.php | 6 +- .../query/ReleephRequestSearchEngine.php | 6 +- ...PhabricatorApplicationSearchController.php | 10 +++- .../PhabricatorApplicationSearchEngine.php | 55 +++++++++++++++++++ .../query/PhabricatorSlowvoteSearchEngine.php | 2 +- 16 files changed, 98 insertions(+), 27 deletions(-) diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php index 6f3b127d66..df58762baf 100644 --- a/src/aphront/AphrontRequest.php +++ b/src/aphront/AphrontRequest.php @@ -168,6 +168,10 @@ final class AphrontRequest { (idx($_FILES[$name], 'error') !== UPLOAD_ERR_NO_FILE); } + final public function isHTTPGet() { + return ($_SERVER['REQUEST_METHOD'] == 'GET'); + } + final public function isHTTPPost() { return ($_SERVER['REQUEST_METHOD'] == 'POST'); } @@ -416,7 +420,7 @@ final class AphrontRequest { // Remove magic parameters like __dialog__ and __ajax__. foreach ($data as $key => $value) { - if (strncmp($key, '__', 2)) { + if (!strncmp($key, '__', 2)) { unset($data[$key]); } } diff --git a/src/applications/countdown/query/PhabricatorCountdownSearchEngine.php b/src/applications/countdown/query/PhabricatorCountdownSearchEngine.php index 753eb6f5d6..1c25df5343 100644 --- a/src/applications/countdown/query/PhabricatorCountdownSearchEngine.php +++ b/src/applications/countdown/query/PhabricatorCountdownSearchEngine.php @@ -7,7 +7,7 @@ final class PhabricatorCountdownSearchEngine $saved = new PhabricatorSavedQuery(); $saved->setParameter( 'authorPHIDs', - array_values($request->getArr('authors'))); + $this->readUsersFromRequest($request, 'authors')); $saved->setParameter('upcoming', $request->getBool('upcoming')); diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php index 7d221aa756..a91887cc06 100644 --- a/src/applications/differential/query/DifferentialRevisionSearchEngine.php +++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php @@ -15,19 +15,19 @@ final class DifferentialRevisionSearchEngine $saved->setParameter( 'responsiblePHIDs', - $request->getArr('responsiblePHIDs')); + $this->readUsersFromRequest($request, 'responsibles')); $saved->setParameter( 'authorPHIDs', - $request->getArr('authorPHIDs')); + $this->readUsersFromRequest($request, 'authors')); $saved->setParameter( 'reviewerPHIDs', - $request->getArr('reviewerPHIDs')); + $this->readUsersFromRequest($request, 'reviewers')); $saved->setParameter( 'subscriberPHIDs', - $request->getArr('subscriberPHIDs')); + $this->readUsersFromRequest($request, 'subscribers')); $saved->setParameter( 'draft', @@ -115,25 +115,25 @@ final class DifferentialRevisionSearchEngine ->appendChild( id(new AphrontFormTokenizerControl()) ->setLabel(pht('Responsible Users')) - ->setName('responsiblePHIDs') + ->setName('responsibles') ->setDatasource('/typeahead/common/accounts/') ->setValue(array_select_keys($tokens, $responsible_phids))) ->appendChild( id(new AphrontFormTokenizerControl()) ->setLabel(pht('Authors')) - ->setName('authorPHIDs') + ->setName('authors') ->setDatasource('/typeahead/common/accounts/') ->setValue(array_select_keys($tokens, $author_phids))) ->appendChild( id(new AphrontFormTokenizerControl()) ->setLabel(pht('Reviewers')) - ->setName('reviewerPHIDs') + ->setName('reviewers') ->setDatasource('/typeahead/common/accounts/') ->setValue(array_select_keys($tokens, $reviewer_phids))) ->appendChild( id(new AphrontFormTokenizerControl()) ->setLabel(pht('Subscribers')) - ->setName('subscriberPHIDs') + ->setName('subscribers') ->setDatasource('/typeahead/common/allmailable/') ->setValue(array_select_keys($tokens, $subscriber_phids))) ->appendChild( diff --git a/src/applications/feed/query/PhabricatorFeedSearchEngine.php b/src/applications/feed/query/PhabricatorFeedSearchEngine.php index e012fc0e01..dfd626eb7c 100644 --- a/src/applications/feed/query/PhabricatorFeedSearchEngine.php +++ b/src/applications/feed/query/PhabricatorFeedSearchEngine.php @@ -8,7 +8,7 @@ final class PhabricatorFeedSearchEngine $saved->setParameter( 'userPHIDs', - array_values($request->getArr('userPHIDs'))); + $this->readUsersFromRequest($request, 'users')); $saved->setParameter( 'projectPHIDs', @@ -76,7 +76,7 @@ final class PhabricatorFeedSearchEngine ->appendChild( id(new AphrontFormTokenizerControl()) ->setDatasource('/typeahead/common/users/') - ->setName('userPHIDs') + ->setName('users') ->setLabel(pht('Include Users')) ->setValue($user_tokens)) ->appendChild( diff --git a/src/applications/files/query/PhabricatorFileSearchEngine.php b/src/applications/files/query/PhabricatorFileSearchEngine.php index cef9c566b2..c1129dcdf4 100644 --- a/src/applications/files/query/PhabricatorFileSearchEngine.php +++ b/src/applications/files/query/PhabricatorFileSearchEngine.php @@ -7,7 +7,7 @@ final class PhabricatorFileSearchEngine $saved = new PhabricatorSavedQuery(); $saved->setParameter( 'authorPHIDs', - array_values($request->getArr('authors'))); + $this->readUsersFromRequest($request, 'authors')); $saved->setParameter('explicit', $request->getBool('explicit')); $saved->setParameter('createdStart', $request->getStr('createdStart')); diff --git a/src/applications/herald/query/HeraldRuleSearchEngine.php b/src/applications/herald/query/HeraldRuleSearchEngine.php index 3e787fa3b1..55f28caca9 100644 --- a/src/applications/herald/query/HeraldRuleSearchEngine.php +++ b/src/applications/herald/query/HeraldRuleSearchEngine.php @@ -8,7 +8,7 @@ final class HeraldRuleSearchEngine $saved->setParameter( 'authorPHIDs', - array_values($request->getArr('authors'))); + $this->readUsersFromRequest($request, 'authors')); $saved->setParameter('contentType', $request->getStr('contentType')); $saved->setParameter('ruleType', $request->getStr('ruleType')); diff --git a/src/applications/legalpad/query/LegalpadDocumentSearchEngine.php b/src/applications/legalpad/query/LegalpadDocumentSearchEngine.php index b743a72f3f..39711676a9 100644 --- a/src/applications/legalpad/query/LegalpadDocumentSearchEngine.php +++ b/src/applications/legalpad/query/LegalpadDocumentSearchEngine.php @@ -10,11 +10,11 @@ final class LegalpadDocumentSearchEngine $saved = new PhabricatorSavedQuery(); $saved->setParameter( 'creatorPHIDs', - array_values($request->getArr('creators'))); + $this->readUsersFromRequest($request, 'creators')); $saved->setParameter( 'contributorPHIDs', - array_values($request->getArr('contributors'))); + $this->readUsersFromRequest($request, 'contributors')); $saved->setParameter('createdStart', $request->getStr('createdStart')); $saved->setParameter('createdEnd', $request->getStr('createdEnd')); diff --git a/src/applications/macro/query/PhabricatorMacroSearchEngine.php b/src/applications/macro/query/PhabricatorMacroSearchEngine.php index d30c0383b0..e5df4ab1b5 100644 --- a/src/applications/macro/query/PhabricatorMacroSearchEngine.php +++ b/src/applications/macro/query/PhabricatorMacroSearchEngine.php @@ -7,7 +7,7 @@ final class PhabricatorMacroSearchEngine $saved = new PhabricatorSavedQuery(); $saved->setParameter( 'authorPHIDs', - array_values($request->getArr('authors'))); + $this->readUsersFromRequest($request, 'authors')); $saved->setParameter('status', $request->getStr('status')); $saved->setParameter('names', $request->getStrList('names')); diff --git a/src/applications/paste/query/PhabricatorPasteSearchEngine.php b/src/applications/paste/query/PhabricatorPasteSearchEngine.php index 3cdcb73fa6..c6c9da3540 100644 --- a/src/applications/paste/query/PhabricatorPasteSearchEngine.php +++ b/src/applications/paste/query/PhabricatorPasteSearchEngine.php @@ -10,7 +10,7 @@ final class PhabricatorPasteSearchEngine $saved = new PhabricatorSavedQuery(); $saved->setParameter( 'authorPHIDs', - array_values($request->getArr('authors'))); + $this->readUsersFromRequest($request, 'authors')); $languages = $request->getStrList('languages'); if ($request->getBool('noLanguage')) { diff --git a/src/applications/pholio/query/PholioMockSearchEngine.php b/src/applications/pholio/query/PholioMockSearchEngine.php index 57ffa2127c..9f7cc1ffe2 100644 --- a/src/applications/pholio/query/PholioMockSearchEngine.php +++ b/src/applications/pholio/query/PholioMockSearchEngine.php @@ -7,7 +7,7 @@ final class PholioMockSearchEngine $saved = new PhabricatorSavedQuery(); $saved->setParameter( 'authorPHIDs', - array_values($request->getArr('authors'))); + $this->readUsersFromRequest($request, 'authors')); return $saved; } diff --git a/src/applications/ponder/query/PonderQuestionSearchEngine.php b/src/applications/ponder/query/PonderQuestionSearchEngine.php index 6de6fc220c..b6d2c6562e 100644 --- a/src/applications/ponder/query/PonderQuestionSearchEngine.php +++ b/src/applications/ponder/query/PonderQuestionSearchEngine.php @@ -8,11 +8,11 @@ final class PonderQuestionSearchEngine $saved->setParameter( 'authorPHIDs', - array_values($request->getArr('authors'))); + $this->readUsersFromRequest($request, 'authors')); $saved->setParameter( 'answererPHIDs', - array_values($request->getArr('answerers'))); + $this->readUsersFromRequest($request, 'answerers')); $saved->setParameter('status', $request->getStr('status')); diff --git a/src/applications/project/query/PhabricatorProjectSearchEngine.php b/src/applications/project/query/PhabricatorProjectSearchEngine.php index 72bf5990ed..7e479a37fd 100644 --- a/src/applications/project/query/PhabricatorProjectSearchEngine.php +++ b/src/applications/project/query/PhabricatorProjectSearchEngine.php @@ -6,7 +6,9 @@ final class PhabricatorProjectSearchEngine public function buildSavedQueryFromRequest(AphrontRequest $request) { $saved = new PhabricatorSavedQuery(); - $saved->setParameter('memberPHIDs', $request->getArr('memberPHIDs')); + $saved->setParameter( + 'memberPHIDs', + $this->readUsersFromRequest($request, 'members')); $saved->setParameter('status', $request->getStr('status')); return $saved; @@ -45,7 +47,7 @@ final class PhabricatorProjectSearchEngine ->appendChild( id(new AphrontFormTokenizerControl()) ->setDatasource('/typeahead/common/users/') - ->setName('memberPHIDs') + ->setName('members') ->setLabel(pht('Members')) ->setValue($member_tokens)) ->appendChild( diff --git a/src/applications/releeph/query/ReleephRequestSearchEngine.php b/src/applications/releeph/query/ReleephRequestSearchEngine.php index 0042fec9d6..a2378e51fc 100644 --- a/src/applications/releeph/query/ReleephRequestSearchEngine.php +++ b/src/applications/releeph/query/ReleephRequestSearchEngine.php @@ -25,7 +25,9 @@ final class ReleephRequestSearchEngine $saved->setParameter('status', $request->getStr('status')); $saved->setParameter('severity', $request->getStr('severity')); - $saved->setParameter('requestorPHIDs', $request->getArr('requestorPHIDs')); + $saved->setParameter( + 'requestorPHIDs', + $this->readUsersFromRequest($request, 'requestors')); return $saved; } @@ -79,7 +81,7 @@ final class ReleephRequestSearchEngine ->appendChild( id(new AphrontFormTokenizerControl()) ->setDatasource('/typeahead/common/users/') - ->setName('requestorPHIDs') + ->setName('requestors') ->setLabel(pht('Requestors')) ->setValue($requestor_tokens)); } diff --git a/src/applications/search/controller/PhabricatorApplicationSearchController.php b/src/applications/search/controller/PhabricatorApplicationSearchController.php index dfc97ec9c8..49360b30a7 100644 --- a/src/applications/search/controller/PhabricatorApplicationSearchController.php +++ b/src/applications/search/controller/PhabricatorApplicationSearchController.php @@ -107,7 +107,15 @@ final class PhabricatorApplicationSearchController $run_query = false; $query_key = $request->getStr('query'); } else if (!strlen($this->queryKey)) { - $query_key = head_key($engine->loadEnabledNamedQueries()); + if ($request->isHTTPGet() && $request->getPassthroughRequestData()) { + // If this is a GET request and it has some query data, don't + // do anything. We'll build and execute a query from it below. + // This allows external tools to build URIs like "/query/?users=a,b". + } else { + // Otherwise, there's no query data so just run the user's default + // query for this application. + $query_key = head_key($engine->loadEnabledNamedQueries()); + } } if ($engine->isBuiltinQuery($query_key)) { diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index e02eaa7470..63f297a727 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -7,6 +7,7 @@ * @task builtin Builtin Queries * @task uri Query URIs * @task dates Date Filters + * @task read Reading Utilities * * @group search */ @@ -234,6 +235,60 @@ abstract class PhabricatorApplicationSearchEngine { } +/* -( Reading Utilities )--------------------------------------------------- */ + + + /** + * Read a list of user PHIDs from a request in a flexible way. This method + * supports either of these forms: + * + * users[]=alincoln&users[]=htaft + * users=alincoln,htaft + * + * Additionally, users can be specified either by PHID or by name. + * + * The main goal of this flexibility is to allow external programs to generate + * links to pages (like "alincoln's open revisions") without needing to make + * API calls. + * + * @param AphrontRequest Request to read user PHIDs from. + * @param string Key to read in the request. + * @return list List of user PHIDs. + * + * @task read + */ + protected function readUsersFromRequest(AphrontRequest $request, $key) { + $list = $request->getArr($key, null); + if ($list === null) { + $list = $request->getStrList($key); + } + + $phids = array(); + $names = array(); + $user_type = PhabricatorPHIDConstants::PHID_TYPE_USER; + foreach ($list as $item) { + if (phid_get_type($item) == $user_type) { + $phids[] = $item; + } else { + $names[] = $item; + } + } + + if ($names) { + $users = id(new PhabricatorPeopleQuery()) + ->setViewer($this->requireViewer()) + ->withUsernames($names) + ->execute(); + foreach ($users as $user) { + $phids[] = $user->getPHID(); + } + $phids = array_unique($phids); + } + + return $phids; + } + + /* -( Dates )-------------------------------------------------------------- */ diff --git a/src/applications/slowvote/query/PhabricatorSlowvoteSearchEngine.php b/src/applications/slowvote/query/PhabricatorSlowvoteSearchEngine.php index f704b12b3b..00abbf5fff 100644 --- a/src/applications/slowvote/query/PhabricatorSlowvoteSearchEngine.php +++ b/src/applications/slowvote/query/PhabricatorSlowvoteSearchEngine.php @@ -7,7 +7,7 @@ final class PhabricatorSlowvoteSearchEngine $saved = new PhabricatorSavedQuery(); $saved->setParameter( 'authorPHIDs', - array_values($request->getArr('authors'))); + $this->readUsersFromRequest($request, 'authors')); $saved->setParameter('voted', $request->getBool('voted'));