Skip to content

Commit 5d94a8a

Browse files
author
epriestley
committed
Use delegation to generalize application search controllers
Summary: Ref T2625. Lifts almost all of the search logic out of Paste controllers and into Search. This uses controller delegation for generalization. We use this in a few places, but don't use it very much yet. I think it's pretty reasonable as-is, but I might be able to make even more stuff free. There are some slightly rough edges around routes, still, but I want to hit Phame and Differential (which both have multiple application search engines) before trying to generalize that. Test Plan: Executed, browsed and managed Paste searches. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2625 Differential Revision: https://secure.phabricator.com/D6073
1 parent cf5009d commit 5d94a8a

10 files changed

+367
-237
lines changed

src/__phutil_library_map__.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,9 @@
743743
'PhabricatorApplicationReleephConfigOptions' => 'applications/releeph/config/PhabricatorApplicationReleephConfigOptions.php',
744744
'PhabricatorApplicationRepositories' => 'applications/repository/application/PhabricatorApplicationRepositories.php',
745745
'PhabricatorApplicationSearch' => 'applications/search/application/PhabricatorApplicationSearch.php',
746+
'PhabricatorApplicationSearchController' => 'applications/search/controller/PhabricatorApplicationSearchController.php',
746747
'PhabricatorApplicationSearchEngine' => 'applications/search/engine/PhabricatorApplicationSearchEngine.php',
748+
'PhabricatorApplicationSearchResultsControllerInterface' => 'applications/search/interface/PhabricatorApplicationSearchResultsControllerInterface.php',
747749
'PhabricatorApplicationSettings' => 'applications/settings/application/PhabricatorApplicationSettings.php',
748750
'PhabricatorApplicationSlowvote' => 'applications/slowvote/application/PhabricatorApplicationSlowvote.php',
749751
'PhabricatorApplicationStatusView' => 'applications/meta/view/PhabricatorApplicationStatusView.php',
@@ -1235,7 +1237,6 @@
12351237
'PhabricatorPasteDAO' => 'applications/paste/storage/PhabricatorPasteDAO.php',
12361238
'PhabricatorPasteEditController' => 'applications/paste/controller/PhabricatorPasteEditController.php',
12371239
'PhabricatorPasteListController' => 'applications/paste/controller/PhabricatorPasteListController.php',
1238-
'PhabricatorPasteQueriesController' => 'applications/paste/controller/PhabricatorPasteQueriesController.php',
12391240
'PhabricatorPasteQuery' => 'applications/paste/query/PhabricatorPasteQuery.php',
12401241
'PhabricatorPasteRemarkupRule' => 'applications/paste/remarkup/PhabricatorPasteRemarkupRule.php',
12411242
'PhabricatorPasteSearchEngine' => 'applications/paste/query/PhabricatorPasteSearchEngine.php',
@@ -2530,6 +2531,7 @@
25302531
'PhabricatorApplicationReleephConfigOptions' => 'PhabricatorApplicationConfigOptions',
25312532
'PhabricatorApplicationRepositories' => 'PhabricatorApplication',
25322533
'PhabricatorApplicationSearch' => 'PhabricatorApplication',
2534+
'PhabricatorApplicationSearchController' => 'PhabricatorSearchBaseController',
25332535
'PhabricatorApplicationSettings' => 'PhabricatorApplication',
25342536
'PhabricatorApplicationSlowvote' => 'PhabricatorApplication',
25352537
'PhabricatorApplicationStatusView' => 'AphrontView',
@@ -3016,8 +3018,11 @@
30163018
'PhabricatorPasteController' => 'PhabricatorController',
30173019
'PhabricatorPasteDAO' => 'PhabricatorLiskDAO',
30183020
'PhabricatorPasteEditController' => 'PhabricatorPasteController',
3019-
'PhabricatorPasteListController' => 'PhabricatorPasteController',
3020-
'PhabricatorPasteQueriesController' => 'PhabricatorPasteController',
3021+
'PhabricatorPasteListController' =>
3022+
array(
3023+
0 => 'PhabricatorPasteController',
3024+
1 => 'PhabricatorApplicationSearchResultsControllerInterface',
3025+
),
30213026
'PhabricatorPasteQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
30223027
'PhabricatorPasteRemarkupRule' => 'PhabricatorRemarkupRuleObject',
30233028
'PhabricatorPasteSearchEngine' => 'PhabricatorApplicationSearchEngine',

src/aphront/AphrontController.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,18 @@ abstract class AphrontController extends Phobject {
77

88
private $request;
99
private $currentApplication;
10+
private $delegatingController;
11+
12+
13+
public function setDelegatingController(
14+
AphrontController $delegating_controller) {
15+
$this->delegatingController = $delegating_controller;
16+
return $this;
17+
}
18+
19+
public function getDelegatingController() {
20+
return $this->delegatingController;
21+
}
1022

1123
public function willBeginExecution() {
1224
return;
@@ -31,6 +43,13 @@ final public function getRequest() {
3143
}
3244

3345
final public function delegateToController(AphrontController $controller) {
46+
$controller->setDelegatingController($this);
47+
48+
$application = $this->getCurrentApplication();
49+
if ($application) {
50+
$controller->setCurrentApplication($application);
51+
}
52+
3453
return $controller->processRequest();
3554
}
3655

src/applications/paste/application/PhabricatorApplicationPaste.php

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,9 @@ public function getRoutes() {
3232
return array(
3333
'/P(?P<id>[1-9]\d*)' => 'PhabricatorPasteViewController',
3434
'/paste/' => array(
35-
'' => 'PhabricatorPasteListController',
36-
'create/' => 'PhabricatorPasteEditController',
37-
'edit/(?P<id>[1-9]\d*)/' => 'PhabricatorPasteEditController',
38-
'filter/(?P<filter>\w+)/' => 'PhabricatorPasteListController',
39-
'query/(?P<queryKey>[^/]+)/'=> 'PhabricatorPasteListController',
40-
'savedqueries/' => 'PhabricatorPasteQueriesController',
35+
'(query/(?P<queryKey>[^/]+)/)?' => 'PhabricatorPasteListController',
36+
'create/' => 'PhabricatorPasteEditController',
37+
'edit/(?P<id>[1-9]\d*)/' => 'PhabricatorPasteEditController',
4138
),
4239
);
4340
}

src/applications/paste/controller/PhabricatorPasteController.php

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,27 +12,9 @@ public function buildSideNavView($for_app = false) {
1212
$nav->addFilter('create', pht('Create Paste'));
1313
}
1414

15-
$nav->addLabel(pht('Queries'));
16-
17-
$engine = id(new PhabricatorPasteSearchEngine())
18-
->setViewer($user);
19-
20-
$named_queries = id(new PhabricatorNamedQueryQuery())
15+
id(new PhabricatorPasteSearchEngine())
2116
->setViewer($user)
22-
->withUserPHIDs(array($user->getPHID()))
23-
->withEngineClassNames(array(get_class($engine)))
24-
->execute();
25-
26-
$named_queries = $named_queries + $engine->getBuiltinQueries($user);
27-
28-
foreach ($named_queries as $query) {
29-
$nav->addFilter('query/'.$query->getQueryKey(), $query->getQueryName());
30-
}
31-
32-
$nav->addFilter('savedqueries', pht('Edit Queries...'));
33-
34-
$nav->addLabel(pht('Search'));
35-
$nav->addFilter('query/advanced', pht('Advanced Search'));
17+
->addNavigationItems($nav);
3618

3719
$nav->selectFilter(null);
3820

src/applications/paste/controller/PhabricatorPasteListController.php

Lines changed: 8 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?php
22

3-
final class PhabricatorPasteListController extends PhabricatorPasteController {
3+
final class PhabricatorPasteListController extends PhabricatorPasteController
4+
implements PhabricatorApplicationSearchResultsControllerInterface {
45

56
private $queryKey;
67

@@ -14,128 +15,15 @@ public function willProcessRequest(array $data) {
1415

1516
public function processRequest() {
1617
$request = $this->getRequest();
17-
$user = $request->getUser();
18+
$controller = id(new PhabricatorApplicationSearchController($request))
19+
->setQueryKey($this->queryKey)
20+
->setSearchEngine(new PhabricatorPasteSearchEngine())
21+
->setNavigation($this->buildSideNavView());
1822

19-
$engine = id(new PhabricatorPasteSearchEngine())
20-
->setViewer($user);
21-
22-
if ($request->isFormPost()) {
23-
return id(new AphrontRedirectResponse())->setURI(
24-
$engine->getQueryResultsPageURI(
25-
$engine->buildSavedQueryFromRequest($request)->getQueryKey()));
26-
}
27-
28-
$nav = $this->buildSideNavView();
29-
30-
$named_query = null;
31-
$run_query = true;
32-
$query_key = $this->queryKey;
33-
if ($this->queryKey == 'advanced') {
34-
$run_query = false;
35-
$query_key = $request->getStr('query');
36-
}
37-
38-
if ($engine->isBuiltinQuery($query_key)) {
39-
$saved_query = $engine->buildSavedQueryFromBuiltin($query_key);
40-
$named_query = $engine->getBuiltinQuery($query_key);
41-
} else if ($query_key) {
42-
$saved_query = id(new PhabricatorSavedQueryQuery())
43-
->setViewer($user)
44-
->withQueryKeys(array($query_key))
45-
->executeOne();
46-
47-
if (!$saved_query) {
48-
return new Aphront404Response();
49-
}
50-
51-
$named_query = id(new PhabricatorNamedQueryQuery())
52-
->setViewer($user)
53-
->withQueryKeys(array($saved_query->getQueryKey()))
54-
->withEngineClassNames(array(get_class($engine)))
55-
->withUserPHIDs(array($user->getPHID()))
56-
->executeOne();
57-
} else {
58-
$saved_query = $engine->buildSavedQueryFromRequest($request);
59-
}
60-
61-
$filter = $nav->selectFilter(
62-
'query/'.$saved_query->getQueryKey(),
63-
'query/advanced');
64-
65-
$form = id(new AphrontFormView())
66-
->setNoShading(true)
67-
->setUser($user);
68-
69-
$engine->buildSearchForm($form, $saved_query);
70-
71-
$submit = id(new AphrontFormSubmitControl())
72-
->setValue(pht('Execute Query'));
73-
74-
if ($run_query && !$named_query) {
75-
$submit->addCancelButton(
76-
'/search/edit/'.$saved_query->getQueryKey().'/',
77-
pht('Save Custom Query...'));
78-
}
79-
80-
$form->appendChild($submit);
81-
$filter_view = id(new AphrontListFilterView())->appendChild($form);
82-
83-
if ($run_query && $named_query) {
84-
if ($named_query->getIsBuiltin()) {
85-
$description = pht(
86-
'Showing results for query "%s".',
87-
$named_query->getQueryName());
88-
} else {
89-
$description = pht(
90-
'Showing results for saved query "%s".',
91-
$named_query->getQueryName());
92-
}
93-
94-
$filter_view->setCollapsed(
95-
pht('Edit Query...'),
96-
pht('Hide Query'),
97-
$description,
98-
$this->getApplicationURI('query/advanced/?query='.$query_key));
99-
}
100-
101-
$nav->appendChild($filter_view);
102-
103-
if ($run_query) {
104-
$query = id(new PhabricatorPasteSearchEngine())
105-
->buildQueryFromSavedQuery($saved_query);
106-
107-
$pager = new AphrontCursorPagerView();
108-
$pager->readFromRequest($request);
109-
$pastes = $query->setViewer($request->getUser())
110-
->needContent(true)
111-
->executeWithCursorPager($pager);
112-
113-
$list = $this->buildPasteList($pastes);
114-
$list->setPager($pager);
115-
$list->setNoDataString(pht("No results found for this query."));
116-
117-
$nav->appendChild($list);
118-
}
119-
120-
$crumbs = $this
121-
->buildApplicationCrumbs($nav)
122-
->addCrumb(
123-
id(new PhabricatorCrumbView())
124-
->setName(pht("Pastes"))
125-
->setHref($this->getApplicationURI('filter/'.$filter.'/')));
126-
127-
$nav->setCrumbs($crumbs);
128-
129-
return $this->buildApplicationPage(
130-
$nav,
131-
array(
132-
'title' => pht("Pastes"),
133-
'device' => true,
134-
'dust' => true,
135-
));
23+
return $this->delegateToController($controller);
13624
}
13725

138-
private function buildPasteList(array $pastes) {
26+
public function renderResultsList(array $pastes) {
13927
assert_instances_of($pastes, 'PhabricatorPaste');
14028

14129
$user = $this->getRequest()->getUser();

src/applications/paste/controller/PhabricatorPasteQueriesController.php

Lines changed: 0 additions & 80 deletions
This file was deleted.

src/applications/paste/query/PhabricatorPasteSearchEngine.php

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,8 @@ public function buildSearchForm(
7070
->setValue($author_tokens));
7171
}
7272

73-
public function getQueryResultsPageURI($query_key) {
74-
return '/paste/query/'.$query_key.'/';
75-
}
76-
77-
public function getQueryManagementURI() {
78-
return '/paste/savedqueries/';
73+
protected function getURI($path) {
74+
return '/paste/'.$path;
7975
}
8076

8177
public function getBuiltinQueryNames() {

0 commit comments

Comments
 (0)