From b3a63a62a21b50033dd5cfdbb88ea9f2b98a1e30 Mon Sep 17 00:00:00 2001 From: vrana Date: Fri, 1 Mar 2013 11:28:02 -0800 Subject: [PATCH] Introduce PhabricatorEmptyQueryException Summary: It's dumb to execute a query which we know will return an empty result. Test Plan: Looked at comment preview with "11", didn't see "1 = 0" in DarkConsole. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D5177 --- src/__phutil_library_map__.php | 2 ++ .../chatlog/PhabricatorChatLogChannelQuery.php | 2 +- src/applications/chatlog/PhabricatorChatLogQuery.php | 2 +- .../conpherence/query/ConpherenceThreadQuery.php | 4 ++-- .../diffusion/query/DiffusionCommitQuery.php | 10 +++++----- src/applications/feed/PhabricatorFeedQuery.php | 2 +- src/applications/files/query/PhabricatorFileQuery.php | 2 +- .../notification/PhabricatorNotificationQuery.php | 2 +- src/applications/paste/query/PhabricatorPasteQuery.php | 2 +- src/applications/phame/query/PhameBlogQuery.php | 2 +- src/applications/pholio/query/PholioMockQuery.php | 2 +- .../__tests__/PhabricatorPolicyAwareTestQuery.php | 2 +- src/applications/ponder/query/PonderQuestionQuery.php | 2 +- .../project/query/PhabricatorProjectQuery.php | 2 +- .../repository/query/PhabricatorRepositoryQuery.php | 2 +- .../tokens/query/PhabricatorTokenGivenQuery.php | 2 +- .../tokens/query/PhabricatorTokenQuery.php | 2 +- .../PhabricatorApplicationTransactionCommentQuery.php | 2 +- .../query/PhabricatorApplicationTransactionQuery.php | 2 +- .../query/PhabricatorEmptyQueryException.php | 5 +++++ .../query/policy/PhabricatorPolicyAwareQuery.php | 6 +++++- 21 files changed, 35 insertions(+), 24 deletions(-) create mode 100644 src/infrastructure/query/PhabricatorEmptyQueryException.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 7b544147e1..cac6e83c72 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -861,6 +861,7 @@ phutil_register_library_map(array( 'PhabricatorEmailLoginController' => 'applications/auth/controller/PhabricatorEmailLoginController.php', 'PhabricatorEmailTokenController' => 'applications/auth/controller/PhabricatorEmailTokenController.php', 'PhabricatorEmailVerificationController' => 'applications/people/controller/PhabricatorEmailVerificationController.php', + 'PhabricatorEmptyQueryException' => 'infrastructure/query/PhabricatorEmptyQueryException.php', 'PhabricatorEnglishTranslation' => 'infrastructure/internationalization/PhabricatorEnglishTranslation.php', 'PhabricatorEnv' => 'infrastructure/env/PhabricatorEnv.php', 'PhabricatorEnvTestCase' => 'infrastructure/env/__tests__/PhabricatorEnvTestCase.php', @@ -2373,6 +2374,7 @@ phutil_register_library_map(array( 'PhabricatorEmailLoginController' => 'PhabricatorAuthController', 'PhabricatorEmailTokenController' => 'PhabricatorAuthController', 'PhabricatorEmailVerificationController' => 'PhabricatorPeopleController', + 'PhabricatorEmptyQueryException' => 'Exception', 'PhabricatorEnglishTranslation' => 'PhabricatorBaseEnglishTranslation', 'PhabricatorEnvTestCase' => 'PhabricatorTestCase', 'PhabricatorErrorExample' => 'PhabricatorUIExample', diff --git a/src/applications/chatlog/PhabricatorChatLogChannelQuery.php b/src/applications/chatlog/PhabricatorChatLogChannelQuery.php index 0930423fcb..98d23bd66d 100644 --- a/src/applications/chatlog/PhabricatorChatLogChannelQuery.php +++ b/src/applications/chatlog/PhabricatorChatLogChannelQuery.php @@ -16,7 +16,7 @@ final class PhabricatorChatLogChannelQuery return $this; } - public function loadPage() { + protected function loadPage() { $table = new PhabricatorChatLogChannel(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/chatlog/PhabricatorChatLogQuery.php b/src/applications/chatlog/PhabricatorChatLogQuery.php index 9a488f87a4..1c5a5ef43c 100644 --- a/src/applications/chatlog/PhabricatorChatLogQuery.php +++ b/src/applications/chatlog/PhabricatorChatLogQuery.php @@ -16,7 +16,7 @@ final class PhabricatorChatLogQuery return $this; } - public function loadPage() { + protected function loadPage() { $table = new PhabricatorChatLogEvent(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php index 269912f87a..2a0742f2e7 100644 --- a/src/applications/conpherence/query/ConpherenceThreadQuery.php +++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php @@ -37,7 +37,7 @@ final class ConpherenceThreadQuery return $this; } - public function loadPage() { + protected function loadPage() { $table = new ConpherenceThread(); $conn_r = $table->establishConnection('r'); @@ -113,7 +113,7 @@ final class ConpherenceThreadQuery ->setViewer($this->getViewer()) ->withObjectPHIDs(array_keys($conpherences)) ->needHandles(true) - ->loadPage(); + ->execute(); $transactions = mgroup($transactions, 'getObjectPHID'); foreach ($conpherences as $phid => $conpherence) { $current_transactions = $transactions[$phid]; diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php index 714981ef0f..68ed22f070 100644 --- a/src/applications/diffusion/query/DiffusionCommitQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitQuery.php @@ -22,7 +22,7 @@ final class DiffusionCommitQuery return $this; } - public function loadPage() { + protected function loadPage() { $table = new PhabricatorRepositoryCommit(); $conn_r = $table->establishConnection('r'); @@ -130,14 +130,14 @@ final class DiffusionCommitQuery } } - if ($sql) { - $where[] = '('.implode(' OR ', $sql).')'; - } else { + if (!$sql) { // If we discarded all possible identifiers (e.g., they all referenced // bogus repositories or were all too short), make sure the query finds // nothing. - $where[] = qsprintf($conn_r, '1 = 0'); + throw new PhabricatorEmptyQueryException('No commit identifiers.'); } + + $where[] = '('.implode(' OR ', $sql).')'; } if ($this->phids) { diff --git a/src/applications/feed/PhabricatorFeedQuery.php b/src/applications/feed/PhabricatorFeedQuery.php index ff32886223..4642307fc9 100644 --- a/src/applications/feed/PhabricatorFeedQuery.php +++ b/src/applications/feed/PhabricatorFeedQuery.php @@ -10,7 +10,7 @@ final class PhabricatorFeedQuery return $this; } - public function loadPage() { + protected function loadPage() { $story_table = new PhabricatorFeedStoryData(); $conn = $story_table->establishConnection('r'); diff --git a/src/applications/files/query/PhabricatorFileQuery.php b/src/applications/files/query/PhabricatorFileQuery.php index 26491d742f..f8a5d894d2 100644 --- a/src/applications/files/query/PhabricatorFileQuery.php +++ b/src/applications/files/query/PhabricatorFileQuery.php @@ -22,7 +22,7 @@ final class PhabricatorFileQuery return $this; } - public function loadPage() { + protected function loadPage() { $table = new PhabricatorFile(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/notification/PhabricatorNotificationQuery.php b/src/applications/notification/PhabricatorNotificationQuery.php index 2154ded0b2..ec5ba3603b 100644 --- a/src/applications/notification/PhabricatorNotificationQuery.php +++ b/src/applications/notification/PhabricatorNotificationQuery.php @@ -45,7 +45,7 @@ final class PhabricatorNotificationQuery /* -( Query Execution )---------------------------------------------------- */ - public function loadPage() { + protected function loadPage() { if (!$this->userPHID) { throw new Exception("Call setUser() before executing the query"); } diff --git a/src/applications/paste/query/PhabricatorPasteQuery.php b/src/applications/paste/query/PhabricatorPasteQuery.php index fe877a7da6..d22ab8154d 100644 --- a/src/applications/paste/query/PhabricatorPasteQuery.php +++ b/src/applications/paste/query/PhabricatorPasteQuery.php @@ -41,7 +41,7 @@ final class PhabricatorPasteQuery return $this; } - public function loadPage() { + protected function loadPage() { $table = new PhabricatorPaste(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/phame/query/PhameBlogQuery.php b/src/applications/phame/query/PhameBlogQuery.php index 89a9a3d86c..022e694275 100644 --- a/src/applications/phame/query/PhameBlogQuery.php +++ b/src/applications/phame/query/PhameBlogQuery.php @@ -25,7 +25,7 @@ final class PhameBlogQuery extends PhabricatorCursorPagedPolicyAwareQuery { return $this; } - public function loadPage() { + protected function loadPage() { $table = new PhameBlog(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/pholio/query/PholioMockQuery.php b/src/applications/pholio/query/PholioMockQuery.php index 8b7572702e..d222fed29b 100644 --- a/src/applications/pholio/query/PholioMockQuery.php +++ b/src/applications/pholio/query/PholioMockQuery.php @@ -50,7 +50,7 @@ final class PholioMockQuery return $this; } - public function loadPage() { + protected function loadPage() { $table = new PholioMock(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/policy/__tests__/PhabricatorPolicyAwareTestQuery.php b/src/applications/policy/__tests__/PhabricatorPolicyAwareTestQuery.php index f4e37a6ef8..9ff1e98ec6 100644 --- a/src/applications/policy/__tests__/PhabricatorPolicyAwareTestQuery.php +++ b/src/applications/policy/__tests__/PhabricatorPolicyAwareTestQuery.php @@ -18,7 +18,7 @@ final class PhabricatorPolicyAwareTestQuery $this->offset = 0; } - public function loadPage() { + protected function loadPage() { if ($this->getRawResultLimit()) { return array_slice( $this->results, diff --git a/src/applications/ponder/query/PonderQuestionQuery.php b/src/applications/ponder/query/PonderQuestionQuery.php index c942c1ad4c..b29a957e0f 100644 --- a/src/applications/ponder/query/PonderQuestionQuery.php +++ b/src/applications/ponder/query/PonderQuestionQuery.php @@ -84,7 +84,7 @@ final class PonderQuestionQuery } } - public function loadPage() { + protected function loadPage() { $question = new PonderQuestion(); $conn_r = $question->establishConnection('r'); diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php index 69d3e71635..8a25663548 100644 --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -53,7 +53,7 @@ final class PhabricatorProjectQuery return true; } - public function loadPage() { + protected function loadPage() { $table = new PhabricatorProject(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/repository/query/PhabricatorRepositoryQuery.php b/src/applications/repository/query/PhabricatorRepositoryQuery.php index 7fa0aeceb6..58401e6316 100644 --- a/src/applications/repository/query/PhabricatorRepositoryQuery.php +++ b/src/applications/repository/query/PhabricatorRepositoryQuery.php @@ -26,7 +26,7 @@ final class PhabricatorRepositoryQuery return true; } - public function loadPage() { + protected function loadPage() { $table = new PhabricatorRepository(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/tokens/query/PhabricatorTokenGivenQuery.php b/src/applications/tokens/query/PhabricatorTokenGivenQuery.php index 1974dd92a0..1c71589215 100644 --- a/src/applications/tokens/query/PhabricatorTokenGivenQuery.php +++ b/src/applications/tokens/query/PhabricatorTokenGivenQuery.php @@ -22,7 +22,7 @@ final class PhabricatorTokenGivenQuery return $this; } - public function loadPage() { + protected function loadPage() { $table = new PhabricatorTokenGiven(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/tokens/query/PhabricatorTokenQuery.php b/src/applications/tokens/query/PhabricatorTokenQuery.php index b5713b2f48..8fa36f8803 100644 --- a/src/applications/tokens/query/PhabricatorTokenQuery.php +++ b/src/applications/tokens/query/PhabricatorTokenQuery.php @@ -10,7 +10,7 @@ final class PhabricatorTokenQuery return $this; } - public function loadPage() { + protected function loadPage() { $tokens = $this->getBuiltinTokens(); if ($this->phids) { diff --git a/src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php b/src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php index 204976243e..6cb16c7e50 100644 --- a/src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php +++ b/src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php @@ -24,7 +24,7 @@ final class PhabricatorApplicationTransactionCommentQuery return $this; } - public function loadPage() { + protected function loadPage() { $table = $this->template; $conn_r = $table->establishConnection('r'); diff --git a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php b/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php index f49e37c8c1..8486235779 100644 --- a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php +++ b/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php @@ -45,7 +45,7 @@ abstract class PhabricatorApplicationTransactionQuery return $this; } - public function loadPage() { + protected function loadPage() { $table = $this->getTemplateApplicationTransaction(); $conn_r = $table->establishConnection('r'); diff --git a/src/infrastructure/query/PhabricatorEmptyQueryException.php b/src/infrastructure/query/PhabricatorEmptyQueryException.php new file mode 100644 index 0000000000..2e0aaa4a89 --- /dev/null +++ b/src/infrastructure/query/PhabricatorEmptyQueryException.php @@ -0,0 +1,5 @@ +rawResultLimit = 0; } - $page = $this->loadPage(); + try { + $page = $this->loadPage(); + } catch (PhabricatorEmptyQueryException $ex) { + $page = array(); + } $visible = $this->willFilterPage($page); $visible = $filter->apply($visible);