Skip to content

Commit ed4a155

Browse files
author
epriestley
committed
Rename "IDPaged" to "CursorPaged", "executeWithPager" to "executeWith[Cursor|Offset]Pager"
Summary: I'm trying to make progress on the policy/visibility stuff since it's a blocker for Wikimedia. First, I want to improve Projects so they can serve as policy groups (e.g., an object can have a visibility policy like "Visible to: members of project 'security'"). However, doing this without breaking anything or snowballing into a bigger change is a bit awkward because Projects are name-ordered and we have a Conduit API which does offset paging. Rather than breaking or rewriting this stuff, I want to just continue offset paging them for now. So I'm going to make PhabricatorPolicyQuery extend PhabricatorOffsetPagedQuery, but can't currently since the `executeWithPager` methods would clash. These methods do different things anyway and are probably better with different names. This also generally improves the names of these classes, since cursors are not necessarily IDs (in the feed case, they're "chronlogicalKeys", for example). I did leave some of the interals as "ID" since calling them "Cursor"s (e.g., `setAfterCursor()`) seemed a little wrong -- it should maybe be `setAfterCursorPosition()`. These APIs have very limited use and can easily be made more consistent later. Test Plan: Browsed around various affected tools; any issues here should throw/fail in a loud/obvious way. Reviewers: vrana, btrahan Reviewed By: vrana CC: aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D3177
1 parent 8ab1789 commit ed4a155

21 files changed

+35
-34
lines changed

src/__phutil_library_map__.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
'AphrontContextBarView' => 'view/layout/AphrontContextBarView.php',
2424
'AphrontController' => 'aphront/AphrontController.php',
2525
'AphrontCrumbsView' => 'view/layout/AphrontCrumbsView.php',
26+
'AphrontCursorPagerView' => 'view/control/AphrontCursorPagerView.php',
2627
'AphrontDefaultApplicationConfiguration' => 'aphront/configuration/AphrontDefaultApplicationConfiguration.php',
2728
'AphrontDialogResponse' => 'aphront/response/AphrontDialogResponse.php',
2829
'AphrontDialogView' => 'view/AphrontDialogView.php',
@@ -57,7 +58,6 @@
5758
'AphrontHeadsupActionListView' => 'view/layout/headsup/AphrontHeadsupActionListView.php',
5859
'AphrontHeadsupActionView' => 'view/layout/headsup/AphrontHeadsupActionView.php',
5960
'AphrontHeadsupView' => 'view/layout/headsup/AphrontHeadsupView.php',
60-
'AphrontIDPagerView' => 'view/control/AphrontIDPagerView.php',
6161
'AphrontIsolatedDatabaseConnectionTestCase' => 'infrastructure/storage/__tests__/AphrontIsolatedDatabaseConnectionTestCase.php',
6262
'AphrontIsolatedHTTPSink' => 'aphront/sink/AphrontIsolatedHTTPSink.php',
6363
'AphrontJSONResponse' => 'aphront/response/AphrontJSONResponse.php',
@@ -606,6 +606,7 @@
606606
'PhabricatorCountdownEditController' => 'applications/countdown/controller/PhabricatorCountdownEditController.php',
607607
'PhabricatorCountdownListController' => 'applications/countdown/controller/PhabricatorCountdownListController.php',
608608
'PhabricatorCountdownViewController' => 'applications/countdown/controller/PhabricatorCountdownViewController.php',
609+
'PhabricatorCursorPagedPolicyQuery' => 'infrastructure/query/policy/PhabricatorCursorPagedPolicyQuery.php',
609610
'PhabricatorDaemon' => 'infrastructure/daemon/PhabricatorDaemon.php',
610611
'PhabricatorDaemonCombinedLogController' => 'applications/daemon/controller/PhabricatorDaemonCombinedLogController.php',
611612
'PhabricatorDaemonConsoleController' => 'applications/daemon/controller/PhabricatorDaemonConsoleController.php',
@@ -734,7 +735,6 @@
734735
'PhabricatorHash' => 'infrastructure/util/PhabricatorHash.php',
735736
'PhabricatorHelpController' => 'applications/help/controller/PhabricatorHelpController.php',
736737
'PhabricatorHelpKeyboardShortcutController' => 'applications/help/controller/PhabricatorHelpKeyboardShortcutController.php',
737-
'PhabricatorIDPagedPolicyQuery' => 'infrastructure/query/policy/PhabricatorIDPagedPolicyQuery.php',
738738
'PhabricatorIRCBot' => 'infrastructure/daemon/irc/PhabricatorIRCBot.php',
739739
'PhabricatorIRCDifferentialNotificationHandler' => 'infrastructure/daemon/irc/handler/PhabricatorIRCDifferentialNotificationHandler.php',
740740
'PhabricatorIRCHandler' => 'infrastructure/daemon/irc/handler/PhabricatorIRCHandler.php',
@@ -1168,6 +1168,7 @@
11681168
'AphrontCalendarMonthView' => 'AphrontView',
11691169
'AphrontContextBarView' => 'AphrontView',
11701170
'AphrontCrumbsView' => 'AphrontView',
1171+
'AphrontCursorPagerView' => 'AphrontView',
11711172
'AphrontDefaultApplicationConfiguration' => 'AphrontApplicationConfiguration',
11721173
'AphrontDialogResponse' => 'AphrontResponse',
11731174
'AphrontDialogView' => 'AphrontView',
@@ -1201,7 +1202,6 @@
12011202
'AphrontHeadsupActionListView' => 'AphrontView',
12021203
'AphrontHeadsupActionView' => 'AphrontView',
12031204
'AphrontHeadsupView' => 'AphrontView',
1204-
'AphrontIDPagerView' => 'AphrontView',
12051205
'AphrontIsolatedDatabaseConnectionTestCase' => 'PhabricatorTestCase',
12061206
'AphrontIsolatedHTTPSink' => 'AphrontHTTPSink',
12071207
'AphrontJSONResponse' => 'AphrontResponse',
@@ -1667,7 +1667,7 @@
16671667
1 => 'PhabricatorPolicyInterface',
16681668
),
16691669
'PhabricatorChatLogEventType' => 'PhabricatorChatLogConstants',
1670-
'PhabricatorChatLogQuery' => 'PhabricatorIDPagedPolicyQuery',
1670+
'PhabricatorChatLogQuery' => 'PhabricatorCursorPagedPolicyQuery',
16711671
'PhabricatorConduitAPIController' => 'PhabricatorConduitController',
16721672
'PhabricatorConduitCertificateToken' => 'PhabricatorConduitDAO',
16731673
'PhabricatorConduitConnectionLog' => 'PhabricatorConduitDAO',
@@ -1686,6 +1686,7 @@
16861686
'PhabricatorCountdownEditController' => 'PhabricatorCountdownController',
16871687
'PhabricatorCountdownListController' => 'PhabricatorCountdownController',
16881688
'PhabricatorCountdownViewController' => 'PhabricatorCountdownController',
1689+
'PhabricatorCursorPagedPolicyQuery' => 'PhabricatorPolicyQuery',
16891690
'PhabricatorDaemon' => 'PhutilDaemon',
16901691
'PhabricatorDaemonCombinedLogController' => 'PhabricatorDaemonController',
16911692
'PhabricatorDaemonConsoleController' => 'PhabricatorDaemonController',
@@ -1741,7 +1742,7 @@
17411742
'PhabricatorFeedController' => 'PhabricatorController',
17421743
'PhabricatorFeedDAO' => 'PhabricatorLiskDAO',
17431744
'PhabricatorFeedPublicStreamController' => 'PhabricatorFeedController',
1744-
'PhabricatorFeedQuery' => 'PhabricatorIDPagedPolicyQuery',
1745+
'PhabricatorFeedQuery' => 'PhabricatorCursorPagedPolicyQuery',
17451746
'PhabricatorFeedStory' => 'PhabricatorPolicyInterface',
17461747
'PhabricatorFeedStoryAggregate' => 'PhabricatorFeedStory',
17471748
'PhabricatorFeedStoryAudit' => 'PhabricatorFeedStory',
@@ -1795,7 +1796,6 @@
17951796
'PhabricatorGoodForNothingWorker' => 'PhabricatorWorker',
17961797
'PhabricatorHelpController' => 'PhabricatorController',
17971798
'PhabricatorHelpKeyboardShortcutController' => 'PhabricatorHelpController',
1798-
'PhabricatorIDPagedPolicyQuery' => 'PhabricatorPolicyQuery',
17991799
'PhabricatorIRCBot' => 'PhabricatorDaemon',
18001800
'PhabricatorIRCDifferentialNotificationHandler' => 'PhabricatorIRCHandler',
18011801
'PhabricatorIRCLogHandler' => 'PhabricatorIRCHandler',
@@ -1911,7 +1911,7 @@
19111911
'PhabricatorPasteController' => 'PhabricatorController',
19121912
'PhabricatorPasteDAO' => 'PhabricatorLiskDAO',
19131913
'PhabricatorPasteListController' => 'PhabricatorPasteController',
1914-
'PhabricatorPasteQuery' => 'PhabricatorIDPagedPolicyQuery',
1914+
'PhabricatorPasteQuery' => 'PhabricatorCursorPagedPolicyQuery',
19151915
'PhabricatorPasteViewController' => 'PhabricatorPasteController',
19161916
'PhabricatorPeopleController' => 'PhabricatorController',
19171917
'PhabricatorPeopleEditController' => 'PhabricatorPeopleController',

src/applications/chatlog/PhabricatorChatLogQuery.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* limitations under the License.
1717
*/
1818

19-
final class PhabricatorChatLogQuery extends PhabricatorIDPagedPolicyQuery {
19+
final class PhabricatorChatLogQuery extends PhabricatorCursorPagedPolicyQuery {
2020

2121
private $channels;
2222
private $maximumEpoch;

src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public function processRequest() {
3232
$uri = clone $request->getRequestURI();
3333
$uri->setQueryParams(array());
3434

35-
$pager = new AphrontIDPagerView();
35+
$pager = new AphrontCursorPagerView();
3636
$pager->setURI($uri);
3737
$pager->setPageSize(250);
3838

@@ -46,7 +46,7 @@ public function processRequest() {
4646
$pager->setAfterID($after);
4747
$pager->setBeforeID($before);
4848

49-
$logs = $query->executeWithPager($pager);
49+
$logs = $query->executeWithCursorPager($pager);
5050

5151
// Show chat logs oldest-first.
5252
$logs = array_reverse($logs);

src/applications/chatlog/storage/PhabricatorChatLogEvent.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public function getCapabilities() {
3535

3636
public function getPolicy($capability) {
3737
// TODO: This is sort of silly and mostly just so that we can use
38-
// IDPagedPolicyQuery; once we implement Channel objects we should
38+
// CursorPagedPolicyQuery; once we implement Channel objects we should
3939
// just delegate policy to them.
4040
return PhabricatorPolicies::POLICY_PUBLIC;
4141
}

src/applications/directory/controller/PhabricatorDirectoryMainController.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,11 +402,11 @@ private function buildFeedView(array $phids) {
402402
$feed_query->setFilterPHIDs($phids);
403403
}
404404

405-
$pager = new AphrontIDPagerView();
405+
$pager = new AphrontCursorPagerView();
406406
$pager->readFromRequest($request);
407407
$pager->setPageSize(200);
408408

409-
$feed = $feed_query->executeWithPager($pager);
409+
$feed = $feed_query->executeWithCursorPager($pager);
410410

411411
$builder = new PhabricatorFeedBuilder($feed);
412412
$builder->setUser($user);

src/applications/drydock/controller/DrydockLogController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function processRequest() {
4141
$pager->setOffset($request->getInt('offset'));
4242
$pager->setURI($request->getRequestURI(), 'offset');
4343

44-
$logs = $query->executeWithPager($pager);
44+
$logs = $query->executeWithOffsetPager($pager);
4545

4646
$rows = array();
4747
foreach ($logs as $log) {

src/applications/feed/PhabricatorFeedQuery.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* limitations under the License.
1717
*/
1818

19-
final class PhabricatorFeedQuery extends PhabricatorIDPagedPolicyQuery {
19+
final class PhabricatorFeedQuery extends PhabricatorCursorPagedPolicyQuery {
2020

2121
private $filterPHIDs;
2222

src/applications/herald/controller/HeraldHomeController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public function processRequest() {
9393
$pager->setURI($request->getRequestURI(), 'offset');
9494
$pager->setOffset($request->getStr('offset'));
9595

96-
$rules = $query->executeWithPager($pager);
96+
$rules = $query->executeWithOffsetPager($pager);
9797

9898
$need_phids = mpull($rules, 'getAuthorPHID');
9999
$handles = id(new PhabricatorObjectHandleData($need_phids))

src/applications/herald/controller/HeraldRuleEditHistoryController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function processRequest() {
3636
$pager->setURI($request->getRequestURI(), 'offset');
3737
$pager->setOffset($request->getStr('offset'));
3838

39-
$edits = $edit_query->executeWithPager($pager);
39+
$edits = $edit_query->executeWithOffsetPager($pager);
4040

4141
$need_phids = mpull($edits, 'getEditorPHID');
4242
$handles = id(new PhabricatorObjectHandleData($need_phids))

src/applications/notification/controller/PhabricatorNotificationListController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public function processRequest() {
5454
break;
5555
}
5656

57-
$notifications = $query->executeWithPager($pager);
57+
$notifications = $query->executeWithOffsetPager($pager);
5858

5959
if ($notifications) {
6060
$builder = new PhabricatorNotificationBuilder($notifications);

src/applications/oauthserver/controller/client/PhabricatorOAuthClientListController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public function processRequest() {
4040

4141
$query = new PhabricatorOAuthServerClientQuery();
4242
$query->withCreatorPHIDs(array($current_user->getPHID()));
43-
$clients = $query->executeWithPager($pager);
43+
$clients = $query->executeWithOffsetPager($pager);
4444

4545
$rows = array();
4646
$rowc = array();

src/applications/oauthserver/controller/clientauthorization/PhabricatorOAuthClientAuthorizationListController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public function processRequest() {
4040

4141
$query = new PhabricatorOAuthClientAuthorizationQuery();
4242
$query->withUserPHIDs(array($current_user->getPHID()));
43-
$authorizations = $query->executeWithPager($pager);
43+
$authorizations = $query->executeWithOffsetPager($pager);
4444

4545
$client_authorizations = mpull($authorizations, null, 'getClientPHID');
4646
$client_phids = array_keys($client_authorizations);

src/applications/paste/controller/PhabricatorPasteListController.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public function processRequest() {
7373
$request = $this->getRequest();
7474
$user = $request->getUser();
7575

76-
$pager = new AphrontIDPagerView();
76+
$pager = new AphrontCursorPagerView();
7777
$pager->readFromRequest($request);
7878

7979
$query = new PhabricatorPasteQuery();
@@ -95,10 +95,10 @@ public function processRequest() {
9595
break;
9696
case 'my':
9797
$query->withAuthorPHIDs(array($user->getPHID()));
98-
$paste_list = $query->executeWithPager($pager);
98+
$paste_list = $query->executeWithCursorPager($pager);
9999
break;
100100
case 'all':
101-
$paste_list = $query->executeWithPager($pager);
101+
$paste_list = $query->executeWithCursorPager($pager);
102102
break;
103103
}
104104

src/applications/paste/query/PhabricatorPasteQuery.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* limitations under the License.
1717
*/
1818

19-
final class PhabricatorPasteQuery extends PhabricatorIDPagedPolicyQuery {
19+
final class PhabricatorPasteQuery extends PhabricatorCursorPagedPolicyQuery {
2020

2121
private $pasteIDs;
2222
private $authorPHIDs;

src/applications/people/controller/PhabricatorPeopleListController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public function processRequest() {
3939

4040
$users = id(new PhabricatorPeopleQuery())
4141
->needPrimaryEmail(true)
42-
->executeWithPager($pager);
42+
->executeWithOffsetPager($pager);
4343

4444
$rows = array();
4545
foreach ($users as $user) {

src/applications/phame/controller/blog/list/PhameAllBlogListController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function processRequest() {
3131

3232
$blogs = id(new PhameBlogQuery())
3333
->needBloggers(true)
34-
->executeWithPager($this->getPager());
34+
->executeWithOffsetPager($this->getPager());
3535
$this->setBlogs($blogs);
3636

3737
$page_title = 'All Blogs';

src/applications/phame/controller/blog/list/PhameUserBlogListController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public function processRequest() {
5151
$blogs = id(new PhameBlogQuery())
5252
->withPHIDs($blog_phids)
5353
->needBloggers(true)
54-
->executeWithPager($this->getPager());
54+
->executeWithOffsetPager($this->getPager());
5555

5656
$this->setBlogs($blogs);
5757

src/applications/phame/controller/post/list/PhamePostListBaseController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ private function loadBloggersFromPosts(array $posts) {
7373
protected function buildPostListPageResponse() {
7474
$pager = $this->getPager();
7575
$query = $this->getPhamePostQuery();
76-
$posts = $query->executeWithPager($pager);
76+
$posts = $query->executeWithOffsetPager($pager);
7777

7878
$bloggers = $this->loadBloggersFromPosts($posts);
7979

src/infrastructure/query/PhabricatorOffsetPagedQuery.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ final protected function buildLimitClause(AphrontDatabaseConnection $conn_r) {
4747
}
4848
}
4949

50-
final public function executeWithPager(AphrontPagerView $pager) {
50+
final public function executeWithOffsetPager(AphrontPagerView $pager) {
5151
$this->setLimit($pager->getPageSize() + 1);
5252
$this->setOffset($pager->getOffset());
5353

src/infrastructure/query/policy/PhabricatorIDPagedPolicyQuery.php renamed to src/infrastructure/query/policy/PhabricatorCursorPagedPolicyQuery.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@
1717
*/
1818

1919
/**
20-
* A query class which uses ID-based paging. This paging is much more performant
21-
* than offset-based paging in the presence of policy filtering.
20+
* A query class which uses cursor-based paging. This paging is much more
21+
* performant than offset-based paging in the presence of policy filtering.
2222
*/
23-
abstract class PhabricatorIDPagedPolicyQuery extends PhabricatorPolicyQuery {
23+
abstract class PhabricatorCursorPagedPolicyQuery
24+
extends PhabricatorPolicyQuery {
2425

2526
private $afterID;
2627
private $beforeID;
@@ -101,7 +102,7 @@ final protected function processResults(array $results) {
101102
}
102103

103104

104-
final public function executeWithPager(AphrontIDPagerView $pager) {
105+
final public function executeWithCursorPager(AphrontCursorPagerView $pager) {
105106
$this->setLimit($pager->getPageSize() + 1);
106107

107108
if ($pager->getAfterID()) {

src/view/control/AphrontIDPagerView.php renamed to src/view/control/AphrontCursorPagerView.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* limitations under the License.
1717
*/
1818

19-
final class AphrontIDPagerView extends AphrontView {
19+
final class AphrontCursorPagerView extends AphrontView {
2020

2121
private $afterID;
2222
private $beforeID;

0 commit comments

Comments
 (0)