From f6b196474008e3de305db08424be059d946de489 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 21 Dec 2012 14:21:31 -0800 Subject: [PATCH] Improve Search architecture Summary: The search indexing API has several problems right now: - Always runs in-process. - It would be nice to push this into the task queue for performance. However, the API currently passses an object all the way through (and some indexers depend on preloaded object attributes), so it can't be dumped into the task queue at any stage since we can't serialize it. - Being able to use the task queue will also make rebuilding indexes faster. - Instead, make the API phid-oriented. - No uniform indexing API. - Each "Editor" currently calls SomeCustomIndexer::indexThing(). This won't work with AbstractTransactions. The API is also just weird. - Instead, provide a uniform API. - No uniform CLI. - We have `scripts/search/reindex_everything.php`, but it doesn't actually index everything. Each new document type needs to be separately added to it, leading to stuff like D3839. Third-party applications can't provide indexers. - Instead, let indexers expose documents for indexing. - Not application-oriented. - All the indexers live in search/ right now, which isn't the right organization in an application-orietned view of the world. - Instead, move indexers to applications and load them with SymbolLoader. Test Plan: - `bin/search index` - Indexed one revision, one task. - Indexed `--type TASK`, `--type DREV`, etc., for all types. - Indexed `--all`. - Added the word "saboteur" to a revision, task, wiki page, and question and then searched for it. - Creating users is a pain; searched for a user after indexing. - Creating commits is a pain; searched for a commit after indexing. - Mocks aren't currently loadable in the result view, so their indexing is moot. Reviewers: btrahan, vrana Reviewed By: btrahan CC: 20after4, aran Maniphest Tasks: T1991, T2104 Differential Revision: https://secure.phabricator.com/D4261 --- bin/search | 1 + scripts/search/index_one_commit.php | 35 +---- scripts/search/manage_search.php | 22 +++ scripts/search/reindex_all_users.php | 14 +- scripts/search/reindex_everything.php | 33 +---- src/__phutil_library_map__.php | 35 +++-- .../editor/PhabricatorAuditCommentEditor.php | 5 +- .../editor/DifferentialCommentEditor.php | 4 +- .../editor/DifferentialRevisionEditor.php | 4 +- .../search/DifferentialSearchIndexer.php} | 14 +- .../DiffusionCommitEditController.php | 3 +- .../editor/ManiphestTransactionEditor.php | 5 +- .../search/ManiphestSearchIndexer.php} | 14 +- .../search/PhabricatorUserSearchIndexer.php} | 15 +- .../people/storage/PhabricatorUser.php | 4 +- ...lioIndexer.php => PholioSearchIndexer.php} | 12 +- .../editor/PhrictionDocumentEditor.php | 4 +- .../search/PhrictionSearchIndexer.php} | 17 ++- .../ponder/editor/PonderAnswerEditor.php | 3 +- .../ponder/editor/PonderCommentEditor.php | 4 +- .../ponder/editor/PonderQuestionEditor.php | 4 +- ...derIndexer.php => PonderSearchIndexer.php} | 14 +- ...bricatorRepositoryCommitSearchIndexer.php} | 17 ++- ...atorRepositoryCommitChangeParserWorker.php | 4 +- .../PhabricatorSearchDocumentIndexer.php | 54 +++++++ .../search/index/PhabricatorSearchIndexer.php | 30 ++++ .../PhabricatorSearchDocumentIndexer.php | 21 --- ...abricatorSearchManagementIndexWorkflow.php | 137 ++++++++++++++++++ .../PhabricatorSearchManagementWorkflow.php | 13 ++ .../PhabricatorBaseEnglishTranslation.php | 5 + 30 files changed, 386 insertions(+), 161 deletions(-) create mode 120000 bin/search create mode 100755 scripts/search/manage_search.php rename src/applications/{search/index/indexer/PhabricatorSearchDifferentialIndexer.php => differential/search/DifferentialSearchIndexer.php} (92%) rename src/applications/{search/index/indexer/PhabricatorSearchManiphestIndexer.php => maniphest/search/ManiphestSearchIndexer.php} (93%) rename src/applications/{search/index/indexer/PhabricatorSearchUserIndexer.php => people/search/PhabricatorUserSearchIndexer.php} (75%) rename src/applications/pholio/indexer/{PholioIndexer.php => PholioSearchIndexer.php} (70%) rename src/applications/{search/index/indexer/PhabricatorSearchPhrictionIndexer.php => phriction/search/PhrictionSearchIndexer.php} (68%) rename src/applications/ponder/search/{PhabricatorSearchPonderIndexer.php => PonderSearchIndexer.php} (87%) rename src/applications/{search/index/indexer/PhabricatorSearchCommitIndexer.php => repository/search/PhabricatorRepositoryCommitSearchIndexer.php} (87%) create mode 100644 src/applications/search/index/PhabricatorSearchDocumentIndexer.php create mode 100644 src/applications/search/index/PhabricatorSearchIndexer.php delete mode 100644 src/applications/search/index/indexer/PhabricatorSearchDocumentIndexer.php create mode 100644 src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php create mode 100644 src/applications/search/management/PhabricatorSearchManagementWorkflow.php diff --git a/bin/search b/bin/search new file mode 120000 index 0000000000..774c1e1412 --- /dev/null +++ b/bin/search @@ -0,0 +1 @@ +../scripts/search/manage_search.php \ No newline at end of file diff --git a/scripts/search/index_one_commit.php b/scripts/search/index_one_commit.php index 3429f675b3..7803caccef 100755 --- a/scripts/search/index_one_commit.php +++ b/scripts/search/index_one_commit.php @@ -1,36 +1,5 @@ #!/usr/bin/env php \n"; - die(1); -} - -$commit = isset($argv[1]) ? $argv[1] : null; -if (!$commit) { - throw new Exception("Provide a commit to index!"); -} -$matches = null; -if (!preg_match('/r([A-Z]+)([a-z0-9]+)/', $commit, $matches)) { - throw new Exception("Can't parse commit identifier!"); -} -$repo = id(new PhabricatorRepository())->loadOneWhere( - 'callsign = %s', - $matches[1]); -if (!$repo) { - throw new Exception("Unknown repository!"); -} - -$commit = id(new PhabricatorRepositoryCommit())->loadOneWhere( - 'repositoryID = %d AND commitIdentifier = %s', - $repo->getID(), - $matches[2]); -if (!$commit) { - throw new Exception('Unknown commit.'); -} - -PhabricatorSearchCommitIndexer::indexCommit($commit); -echo "Done.\n"; +echo "Use 'bin/search index rXnnnnnn' instead of this script.\n"; +exit(1); diff --git a/scripts/search/manage_search.php b/scripts/search/manage_search.php new file mode 100755 index 0000000000..25c391b6c3 --- /dev/null +++ b/scripts/search/manage_search.php @@ -0,0 +1,22 @@ +#!/usr/bin/env php +setTagline('manage search'); +$args->setSynopsis(<<parseStandardArguments(); + +$workflows = array( + new PhabricatorSearchManagementIndexWorkflow(), + new PhutilHelpArgumentWorkflow(), +); + +$args->parseWorkflows($workflows); diff --git a/scripts/search/reindex_all_users.php b/scripts/search/reindex_all_users.php index 330102780f..68aee018d3 100755 --- a/scripts/search/reindex_all_users.php +++ b/scripts/search/reindex_all_users.php @@ -1,15 +1,5 @@ #!/usr/bin/env php loadAll(); -echo "Indexing ".count($users)." users"; -foreach ($users as $user) { - PhabricatorSearchUserIndexer::indexUser($user); - echo '.'; -} -echo "\n"; -echo "Done.\n"; - +echo "Use 'bin/search index --type USER' instead of this script.\n"; +die(1); diff --git a/scripts/search/reindex_everything.php b/scripts/search/reindex_everything.php index 4639e28714..bbf02b43c7 100755 --- a/scripts/search/reindex_everything.php +++ b/scripts/search/reindex_everything.php @@ -1,34 +1,5 @@ #!/usr/bin/env php 'applications/differential/field/specification/DifferentialRevisionStatusFieldSpecification.php', 'DifferentialRevisionUpdateHistoryView' => 'applications/differential/view/DifferentialRevisionUpdateHistoryView.php', 'DifferentialRevisionViewController' => 'applications/differential/controller/DifferentialRevisionViewController.php', + 'DifferentialSearchIndexer' => 'applications/differential/search/DifferentialSearchIndexer.php', 'DifferentialSubscribeController' => 'applications/differential/controller/DifferentialSubscribeController.php', 'DifferentialSummaryFieldSpecification' => 'applications/differential/field/specification/DifferentialSummaryFieldSpecification.php', 'DifferentialTasksAttacher' => 'applications/differential/DifferentialTasksAttacher.php', @@ -533,6 +534,7 @@ phutil_register_library_map(array( 'ManiphestSavedQueryDeleteController' => 'applications/maniphest/controller/ManiphestSavedQueryDeleteController.php', 'ManiphestSavedQueryEditController' => 'applications/maniphest/controller/ManiphestSavedQueryEditController.php', 'ManiphestSavedQueryListController' => 'applications/maniphest/controller/ManiphestSavedQueryListController.php', + 'ManiphestSearchIndexer' => 'applications/maniphest/search/ManiphestSearchIndexer.php', 'ManiphestSubpriorityController' => 'applications/maniphest/controller/ManiphestSubpriorityController.php', 'ManiphestTask' => 'applications/maniphest/storage/ManiphestTask.php', 'ManiphestTaskAuxiliaryStorage' => 'applications/maniphest/storage/ManiphestTaskAuxiliaryStorage.php', @@ -1052,6 +1054,7 @@ phutil_register_library_map(array( 'PhabricatorRepositoryCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php', 'PhabricatorRepositoryCommitOwnersWorker' => 'applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php', 'PhabricatorRepositoryCommitParserWorker' => 'applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php', + 'PhabricatorRepositoryCommitSearchIndexer' => 'applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php', 'PhabricatorRepositoryController' => 'applications/repository/controller/PhabricatorRepositoryController.php', 'PhabricatorRepositoryCreateController' => 'applications/repository/controller/PhabricatorRepositoryCreateController.php', 'PhabricatorRepositoryDAO' => 'applications/repository/storage/PhabricatorRepositoryDAO.php', @@ -1084,28 +1087,25 @@ phutil_register_library_map(array( 'PhabricatorSearchAbstractDocument' => 'applications/search/index/PhabricatorSearchAbstractDocument.php', 'PhabricatorSearchAttachController' => 'applications/search/controller/PhabricatorSearchAttachController.php', 'PhabricatorSearchBaseController' => 'applications/search/controller/PhabricatorSearchBaseController.php', - 'PhabricatorSearchCommitIndexer' => 'applications/search/index/indexer/PhabricatorSearchCommitIndexer.php', 'PhabricatorSearchController' => 'applications/search/controller/PhabricatorSearchController.php', 'PhabricatorSearchDAO' => 'applications/search/storage/PhabricatorSearchDAO.php', - 'PhabricatorSearchDifferentialIndexer' => 'applications/search/index/indexer/PhabricatorSearchDifferentialIndexer.php', 'PhabricatorSearchDocument' => 'applications/search/storage/document/PhabricatorSearchDocument.php', 'PhabricatorSearchDocumentField' => 'applications/search/storage/document/PhabricatorSearchDocumentField.php', - 'PhabricatorSearchDocumentIndexer' => 'applications/search/index/indexer/PhabricatorSearchDocumentIndexer.php', + 'PhabricatorSearchDocumentIndexer' => 'applications/search/index/PhabricatorSearchDocumentIndexer.php', 'PhabricatorSearchDocumentRelationship' => 'applications/search/storage/document/PhabricatorSearchDocumentRelationship.php', 'PhabricatorSearchEngine' => 'applications/search/engine/PhabricatorSearchEngine.php', 'PhabricatorSearchEngineElastic' => 'applications/search/engine/PhabricatorSearchEngineElastic.php', 'PhabricatorSearchEngineMySQL' => 'applications/search/engine/PhabricatorSearchEngineMySQL.php', 'PhabricatorSearchEngineSelector' => 'applications/search/selector/PhabricatorSearchEngineSelector.php', 'PhabricatorSearchField' => 'applications/search/constants/PhabricatorSearchField.php', - 'PhabricatorSearchManiphestIndexer' => 'applications/search/index/indexer/PhabricatorSearchManiphestIndexer.php', - 'PhabricatorSearchPhrictionIndexer' => 'applications/search/index/indexer/PhabricatorSearchPhrictionIndexer.php', - 'PhabricatorSearchPonderIndexer' => 'applications/ponder/search/PhabricatorSearchPonderIndexer.php', + 'PhabricatorSearchIndexer' => 'applications/search/index/PhabricatorSearchIndexer.php', + 'PhabricatorSearchManagementIndexWorkflow' => 'applications/search/management/PhabricatorSearchManagementIndexWorkflow.php', + 'PhabricatorSearchManagementWorkflow' => 'applications/search/management/PhabricatorSearchManagementWorkflow.php', 'PhabricatorSearchQuery' => 'applications/search/storage/PhabricatorSearchQuery.php', 'PhabricatorSearchRelationship' => 'applications/search/constants/PhabricatorSearchRelationship.php', 'PhabricatorSearchResultView' => 'applications/search/view/PhabricatorSearchResultView.php', 'PhabricatorSearchScope' => 'applications/search/constants/PhabricatorSearchScope.php', 'PhabricatorSearchSelectController' => 'applications/search/controller/PhabricatorSearchSelectController.php', - 'PhabricatorSearchUserIndexer' => 'applications/search/index/indexer/PhabricatorSearchUserIndexer.php', 'PhabricatorSettingsAdjustController' => 'applications/settings/controller/PhabricatorSettingsAdjustController.php', 'PhabricatorSettingsMainController' => 'applications/settings/controller/PhabricatorSettingsMainController.php', 'PhabricatorSettingsPanel' => 'applications/settings/panel/PhabricatorSettingsPanel.php', @@ -1191,6 +1191,7 @@ phutil_register_library_map(array( 'PhabricatorUserPreferences' => 'applications/settings/storage/PhabricatorUserPreferences.php', 'PhabricatorUserProfile' => 'applications/people/storage/PhabricatorUserProfile.php', 'PhabricatorUserSSHKey' => 'applications/settings/storage/PhabricatorUserSSHKey.php', + 'PhabricatorUserSearchIndexer' => 'applications/people/search/PhabricatorUserSearchIndexer.php', 'PhabricatorUserStatus' => 'applications/people/storage/PhabricatorUserStatus.php', 'PhabricatorUserStatusInvalidEpochException' => 'applications/people/exception/PhabricatorUserStatusInvalidEpochException.php', 'PhabricatorUserStatusOverlapException' => 'applications/people/exception/PhabricatorUserStatusOverlapException.php', @@ -1256,7 +1257,6 @@ phutil_register_library_map(array( 'PholioController' => 'applications/pholio/controller/PholioController.php', 'PholioDAO' => 'applications/pholio/storage/PholioDAO.php', 'PholioImage' => 'applications/pholio/storage/PholioImage.php', - 'PholioIndexer' => 'applications/pholio/indexer/PholioIndexer.php', 'PholioMock' => 'applications/pholio/storage/PholioMock.php', 'PholioMockCommentController' => 'applications/pholio/controller/PholioMockCommentController.php', 'PholioMockEditController' => 'applications/pholio/controller/PholioMockEditController.php', @@ -1265,6 +1265,7 @@ phutil_register_library_map(array( 'PholioMockQuery' => 'applications/pholio/query/PholioMockQuery.php', 'PholioMockViewController' => 'applications/pholio/controller/PholioMockViewController.php', 'PholioReplyHandler' => 'applications/pholio/mail/PholioReplyHandler.php', + 'PholioSearchIndexer' => 'applications/pholio/indexer/PholioSearchIndexer.php', 'PholioTransaction' => 'applications/pholio/storage/PholioTransaction.php', 'PholioTransactionComment' => 'applications/pholio/storage/PholioTransactionComment.php', 'PholioTransactionQuery' => 'applications/pholio/query/PholioTransactionQuery.php', @@ -1291,6 +1292,7 @@ phutil_register_library_map(array( 'PhrictionHistoryController' => 'applications/phriction/controller/PhrictionHistoryController.php', 'PhrictionListController' => 'applications/phriction/controller/PhrictionListController.php', 'PhrictionNewController' => 'applications/phriction/controller/PhrictionNewController.php', + 'PhrictionSearchIndexer' => 'applications/phriction/search/PhrictionSearchIndexer.php', 'PonderAddAnswerView' => 'applications/ponder/view/PonderAddAnswerView.php', 'PonderAddCommentView' => 'applications/ponder/view/PonderAddCommentView.php', 'PonderAnswer' => 'applications/ponder/storage/PonderAnswer.php', @@ -1324,6 +1326,7 @@ phutil_register_library_map(array( 'PonderQuestionViewController' => 'applications/ponder/controller/PonderQuestionViewController.php', 'PonderReplyHandler' => 'applications/ponder/PonderReplyHandler.php', 'PonderRuleQuestion' => 'infrastructure/markup/rule/PonderRuleQuestion.php', + 'PonderSearchIndexer' => 'applications/ponder/search/PonderSearchIndexer.php', 'PonderUserProfileView' => 'applications/ponder/view/PonderUserProfileView.php', 'PonderVotableInterface' => 'applications/ponder/storage/PonderVotableInterface.php', 'PonderVotableView' => 'applications/ponder/view/PonderVotableView.php', @@ -1631,6 +1634,7 @@ phutil_register_library_map(array( 'DifferentialRevisionStatusFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialRevisionUpdateHistoryView' => 'AphrontView', 'DifferentialRevisionViewController' => 'DifferentialController', + 'DifferentialSearchIndexer' => 'PhabricatorSearchDocumentIndexer', 'DifferentialSubscribeController' => 'DifferentialController', 'DifferentialSummaryFieldSpecification' => 'DifferentialFreeformFieldSpecification', 'DifferentialTestPlanFieldSpecification' => 'DifferentialFieldSpecification', @@ -1815,6 +1819,7 @@ phutil_register_library_map(array( 'ManiphestSavedQueryDeleteController' => 'ManiphestController', 'ManiphestSavedQueryEditController' => 'ManiphestController', 'ManiphestSavedQueryListController' => 'ManiphestController', + 'ManiphestSearchIndexer' => 'PhabricatorSearchDocumentIndexer', 'ManiphestSubpriorityController' => 'ManiphestController', 'ManiphestTask' => array( @@ -2324,6 +2329,7 @@ phutil_register_library_map(array( 'PhabricatorRepositoryCommitMessageParserWorker' => 'PhabricatorRepositoryCommitParserWorker', 'PhabricatorRepositoryCommitOwnersWorker' => 'PhabricatorRepositoryCommitParserWorker', 'PhabricatorRepositoryCommitParserWorker' => 'PhabricatorWorker', + 'PhabricatorRepositoryCommitSearchIndexer' => 'PhabricatorSearchDocumentIndexer', 'PhabricatorRepositoryController' => 'PhabricatorController', 'PhabricatorRepositoryCreateController' => 'PhabricatorRepositoryController', 'PhabricatorRepositoryDAO' => 'PhabricatorLiskDAO', @@ -2351,22 +2357,18 @@ phutil_register_library_map(array( 'PhabricatorSSHWorkflow' => 'PhutilArgumentWorkflow', 'PhabricatorSearchAttachController' => 'PhabricatorSearchBaseController', 'PhabricatorSearchBaseController' => 'PhabricatorController', - 'PhabricatorSearchCommitIndexer' => 'PhabricatorSearchDocumentIndexer', 'PhabricatorSearchController' => 'PhabricatorSearchBaseController', 'PhabricatorSearchDAO' => 'PhabricatorLiskDAO', - 'PhabricatorSearchDifferentialIndexer' => 'PhabricatorSearchDocumentIndexer', 'PhabricatorSearchDocument' => 'PhabricatorSearchDAO', 'PhabricatorSearchDocumentField' => 'PhabricatorSearchDAO', 'PhabricatorSearchDocumentRelationship' => 'PhabricatorSearchDAO', 'PhabricatorSearchEngineElastic' => 'PhabricatorSearchEngine', 'PhabricatorSearchEngineMySQL' => 'PhabricatorSearchEngine', - 'PhabricatorSearchManiphestIndexer' => 'PhabricatorSearchDocumentIndexer', - 'PhabricatorSearchPhrictionIndexer' => 'PhabricatorSearchDocumentIndexer', - 'PhabricatorSearchPonderIndexer' => 'PhabricatorSearchDocumentIndexer', + 'PhabricatorSearchManagementIndexWorkflow' => 'PhabricatorSearchManagementWorkflow', + 'PhabricatorSearchManagementWorkflow' => 'PhutilArgumentWorkflow', 'PhabricatorSearchQuery' => 'PhabricatorSearchDAO', 'PhabricatorSearchResultView' => 'AphrontView', 'PhabricatorSearchSelectController' => 'PhabricatorSearchBaseController', - 'PhabricatorSearchUserIndexer' => 'PhabricatorSearchDocumentIndexer', 'PhabricatorSettingsAdjustController' => 'PhabricatorController', 'PhabricatorSettingsMainController' => 'PhabricatorController', 'PhabricatorSettingsPanelAccount' => 'PhabricatorSettingsPanel', @@ -2444,6 +2446,7 @@ phutil_register_library_map(array( 'PhabricatorUserPreferences' => 'PhabricatorUserDAO', 'PhabricatorUserProfile' => 'PhabricatorUserDAO', 'PhabricatorUserSSHKey' => 'PhabricatorUserDAO', + 'PhabricatorUserSearchIndexer' => 'PhabricatorSearchDocumentIndexer', 'PhabricatorUserStatus' => 'PhabricatorUserDAO', 'PhabricatorUserStatusInvalidEpochException' => 'Exception', 'PhabricatorUserStatusOverlapException' => 'Exception', @@ -2520,7 +2523,6 @@ phutil_register_library_map(array( 0 => 'PholioDAO', 1 => 'PhabricatorMarkupInterface', ), - 'PholioIndexer' => 'PhabricatorSearchDocumentIndexer', 'PholioMock' => array( 0 => 'PholioDAO', @@ -2535,6 +2537,7 @@ phutil_register_library_map(array( 'PholioMockQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PholioMockViewController' => 'PholioController', 'PholioReplyHandler' => 'PhabricatorMailReplyHandler', + 'PholioSearchIndexer' => 'PhabricatorSearchDocumentIndexer', 'PholioTransaction' => 'PhabricatorApplicationTransaction', 'PholioTransactionComment' => 'PhabricatorApplicationTransactionComment', 'PholioTransactionQuery' => 'PhabricatorApplicationTransactionQuery', @@ -2564,6 +2567,7 @@ phutil_register_library_map(array( 'PhrictionHistoryController' => 'PhrictionController', 'PhrictionListController' => 'PhrictionController', 'PhrictionNewController' => 'PhrictionController', + 'PhrictionSearchIndexer' => 'PhabricatorSearchDocumentIndexer', 'PonderAddAnswerView' => 'AphrontView', 'PonderAddCommentView' => 'AphrontView', 'PonderAnswer' => @@ -2611,6 +2615,7 @@ phutil_register_library_map(array( 'PonderQuestionViewController' => 'PonderController', 'PonderReplyHandler' => 'PhabricatorMailReplyHandler', 'PonderRuleQuestion' => 'PhabricatorRemarkupRuleObjectName', + 'PonderSearchIndexer' => 'PhabricatorSearchDocumentIndexer', 'PonderUserProfileView' => 'AphrontView', 'PonderVotableView' => 'AphrontView', 'PonderVoteEditor' => 'PhabricatorEditor', diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php index 75c58e2f88..9595fc6316 100644 --- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php @@ -289,7 +289,10 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor { $feed_phids = array_diff($requests_phids, $feed_dont_publish_phids); $this->publishFeedStory($comment, $feed_phids); - PhabricatorSearchCommitIndexer::indexCommit($commit); + + id(new PhabricatorSearchIndexer()) + ->indexDocumentByPHID($commit->getPHID()); + $this->sendMail($comment, $other_comments, $inline_comments, $requests); } diff --git a/src/applications/differential/editor/DifferentialCommentEditor.php b/src/applications/differential/editor/DifferentialCommentEditor.php index 3826c29397..cbeff32a12 100644 --- a/src/applications/differential/editor/DifferentialCommentEditor.php +++ b/src/applications/differential/editor/DifferentialCommentEditor.php @@ -598,8 +598,8 @@ final class DifferentialCommentEditor extends PhabricatorEditor { ->setMailRecipientPHIDs($mailed_phids) ->publish(); - // TODO: Move to workers - PhabricatorSearchDifferentialIndexer::indexRevision($revision); + id(new PhabricatorSearchIndexer()) + ->indexDocumentByPHID($revision->getPHID()); return $comment; } diff --git a/src/applications/differential/editor/DifferentialRevisionEditor.php b/src/applications/differential/editor/DifferentialRevisionEditor.php index e4dc8db969..f347cd3b1c 100644 --- a/src/applications/differential/editor/DifferentialRevisionEditor.php +++ b/src/applications/differential/editor/DifferentialRevisionEditor.php @@ -489,8 +489,8 @@ final class DifferentialRevisionEditor extends PhabricatorEditor { ->setMailRecipientPHIDs($mailed_phids) ->publish(); - // TODO: Move this into a worker task thing. - PhabricatorSearchDifferentialIndexer::indexRevision($revision); + id(new PhabricatorSearchIndexer()) + ->indexDocumentByPHID($revision->getPHID()); } public static function addCCAndUpdateRevision( diff --git a/src/applications/search/index/indexer/PhabricatorSearchDifferentialIndexer.php b/src/applications/differential/search/DifferentialSearchIndexer.php similarity index 92% rename from src/applications/search/index/indexer/PhabricatorSearchDifferentialIndexer.php rename to src/applications/differential/search/DifferentialSearchIndexer.php index 90c0c86d1e..32dcadda95 100644 --- a/src/applications/search/index/indexer/PhabricatorSearchDifferentialIndexer.php +++ b/src/applications/differential/search/DifferentialSearchIndexer.php @@ -1,12 +1,18 @@ loadDocumentByPHID($phid); + $doc = new PhabricatorSearchAbstractDocument(); $doc->setPHID($rev->getPHID()); $doc->setDocumentType(PhabricatorPHIDConstants::PHID_TYPE_DREV); @@ -108,6 +114,6 @@ final class PhabricatorSearchDifferentialIndexer $rev->getDateModified()); // Bogus timestamp. } - self::reindexAbstractDocument($doc); + return $doc; } } diff --git a/src/applications/diffusion/controller/DiffusionCommitEditController.php b/src/applications/diffusion/controller/DiffusionCommitEditController.php index 49aeaf9acc..fcfb0cd42e 100644 --- a/src/applications/diffusion/controller/DiffusionCommitEditController.php +++ b/src/applications/diffusion/controller/DiffusionCommitEditController.php @@ -44,7 +44,8 @@ final class DiffusionCommitEditController extends DiffusionController { } $editor->save(); - PhabricatorSearchCommitIndexer::indexCommit($commit); + id(new PhabricatorSearchIndexer()) + ->indexDocumentByPHID($commit->getPHID()); return id(new AphrontRedirectResponse()) ->setURI('/r'.$callsign.$commit->getCommitIdentifier()); diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php index 8fa7970dc4..fc908726d7 100644 --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -199,9 +199,8 @@ final class ManiphestTransactionEditor extends PhabricatorEditor { $transactions, $mail->buildRecipientList()); - // TODO: Do this offline via workers - PhabricatorSearchManiphestIndexer::indexTask($task); - + id(new PhabricatorSearchIndexer()) + ->indexDocumentByPHID($task->getPHID()); } protected function getSubjectPrefix() { diff --git a/src/applications/search/index/indexer/PhabricatorSearchManiphestIndexer.php b/src/applications/maniphest/search/ManiphestSearchIndexer.php similarity index 93% rename from src/applications/search/index/indexer/PhabricatorSearchManiphestIndexer.php rename to src/applications/maniphest/search/ManiphestSearchIndexer.php index 5d02f56223..93a5ff88f3 100644 --- a/src/applications/search/index/indexer/PhabricatorSearchManiphestIndexer.php +++ b/src/applications/maniphest/search/ManiphestSearchIndexer.php @@ -1,12 +1,18 @@ loadDocumentByPHID($phid); + $doc = new PhabricatorSearchAbstractDocument(); $doc->setPHID($task->getPHID()); $doc->setDocumentType(PhabricatorPHIDConstants::PHID_TYPE_TASK); @@ -114,6 +120,6 @@ final class PhabricatorSearchManiphestIndexer $time); } - self::reindexAbstractDocument($doc); + return $doc; } } diff --git a/src/applications/search/index/indexer/PhabricatorSearchUserIndexer.php b/src/applications/people/search/PhabricatorUserSearchIndexer.php similarity index 75% rename from src/applications/search/index/indexer/PhabricatorSearchUserIndexer.php rename to src/applications/people/search/PhabricatorUserSearchIndexer.php index 18c787d4d0..64a119823b 100644 --- a/src/applications/search/index/indexer/PhabricatorSearchUserIndexer.php +++ b/src/applications/people/search/PhabricatorUserSearchIndexer.php @@ -1,12 +1,15 @@ loadDocumentByPHID($phid); + $doc = new PhabricatorSearchAbstractDocument(); $doc->setPHID($user->getPHID()); $doc->setDocumentType(PhabricatorPHIDConstants::PHID_TYPE_USER); @@ -25,6 +28,6 @@ final class PhabricatorSearchUserIndexer time()); } - self::reindexAbstractDocument($doc); + return $doc; } } diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index a0d621ca8c..07bf892360 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -104,7 +104,9 @@ final class PhabricatorUser extends PhabricatorUserDAO implements PhutilPerson { $result = parent::save(); $this->updateNameTokens(); - PhabricatorSearchUserIndexer::indexUser($this); + + id(new PhabricatorSearchIndexer()) + ->indexDocumentByPHID($this->getPHID()); return $result; } diff --git a/src/applications/pholio/indexer/PholioIndexer.php b/src/applications/pholio/indexer/PholioSearchIndexer.php similarity index 70% rename from src/applications/pholio/indexer/PholioIndexer.php rename to src/applications/pholio/indexer/PholioSearchIndexer.php index 2a87de40ca..d73a174c45 100644 --- a/src/applications/pholio/indexer/PholioIndexer.php +++ b/src/applications/pholio/indexer/PholioSearchIndexer.php @@ -3,9 +3,15 @@ /** * @group pholio */ -final class PholioIndexer extends PhabricatorSearchDocumentIndexer { +final class PholioSearchIndexer extends PhabricatorSearchDocumentIndexer { + + public function getIndexableObject() { + return new PholioMock(); + } + + protected function buildAbstractDocumentByPHID($phid) { + $mock = $this->loadDocumentByPHID($phid); - public static function indexMock(PholioMock $mock) { $doc = new PhabricatorSearchAbstractDocument(); $doc->setPHID($mock->getPHID()); $doc->setDocumentType(phid_get_type($mock->getPHID())); @@ -23,6 +29,6 @@ final class PholioIndexer extends PhabricatorSearchDocumentIndexer { PhabricatorPHIDConstants::PHID_TYPE_USER, $mock->getDateCreated()); - self::reindexAbstractDocument($doc); + return $doc; } } diff --git a/src/applications/phriction/editor/PhrictionDocumentEditor.php b/src/applications/phriction/editor/PhrictionDocumentEditor.php index 290a54745c..fb35dbe73a 100644 --- a/src/applications/phriction/editor/PhrictionDocumentEditor.php +++ b/src/applications/phriction/editor/PhrictionDocumentEditor.php @@ -172,7 +172,9 @@ final class PhrictionDocumentEditor extends PhabricatorEditor { $document->save(); $document->attachContent($new_content); - PhabricatorSearchPhrictionIndexer::indexDocument($document); + + id(new PhabricatorSearchIndexer()) + ->indexDocumentByPHID($document->getPHID()); $project_phid = null; $slug = $document->getSlug(); diff --git a/src/applications/search/index/indexer/PhabricatorSearchPhrictionIndexer.php b/src/applications/phriction/search/PhrictionSearchIndexer.php similarity index 68% rename from src/applications/search/index/indexer/PhabricatorSearchPhrictionIndexer.php rename to src/applications/phriction/search/PhrictionSearchIndexer.php index 808756091d..f2565b2495 100644 --- a/src/applications/search/index/indexer/PhabricatorSearchPhrictionIndexer.php +++ b/src/applications/phriction/search/PhrictionSearchIndexer.php @@ -1,12 +1,21 @@ loadDocumentByPHID($phid); + + $content = id(new PhrictionContent())->load($document->getContentID()); + $document->attachContent($content); + $content = $document->getContent(); $doc = new PhabricatorSearchAbstractDocument(); @@ -28,6 +37,6 @@ final class PhabricatorSearchPhrictionIndexer PhabricatorPHIDConstants::PHID_TYPE_USER, $content->getDateCreated()); - self::reindexAbstractDocument($doc); + return $doc; } } diff --git a/src/applications/ponder/editor/PonderAnswerEditor.php b/src/applications/ponder/editor/PonderAnswerEditor.php index 89d3bb90a8..2a41fcebe1 100644 --- a/src/applications/ponder/editor/PonderAnswerEditor.php +++ b/src/applications/ponder/editor/PonderAnswerEditor.php @@ -47,7 +47,8 @@ final class PonderAnswerEditor extends PhabricatorEditor { $trans->saveTransaction(); $question->attachRelated(); - PhabricatorSearchPonderIndexer::indexQuestion($question); + id(new PhabricatorSearchIndexer()) + ->indexDocumentByPHID($question->getPHID()); // subscribe author and @mentions $subeditor = id(new PhabricatorSubscriptionsEditor()) diff --git a/src/applications/ponder/editor/PonderCommentEditor.php b/src/applications/ponder/editor/PonderCommentEditor.php index 14c0a0cebc..1b7ab918ff 100644 --- a/src/applications/ponder/editor/PonderCommentEditor.php +++ b/src/applications/ponder/editor/PonderCommentEditor.php @@ -39,8 +39,8 @@ final class PonderCommentEditor extends PhabricatorEditor { $target = $this->targetPHID; $comment->save(); - $question->attachRelated(); - PhabricatorSearchPonderIndexer::indexQuestion($question); + id(new PhabricatorSearchIndexer()) + ->indexDocumentByPHID($question->getPHID()); // subscribe author and @mentions $subeditor = id(new PhabricatorSubscriptionsEditor()) diff --git a/src/applications/ponder/editor/PonderQuestionEditor.php b/src/applications/ponder/editor/PonderQuestionEditor.php index e17bce0ee7..9a7491dfcb 100644 --- a/src/applications/ponder/editor/PonderQuestionEditor.php +++ b/src/applications/ponder/editor/PonderQuestionEditor.php @@ -24,9 +24,9 @@ final class PonderQuestionEditor extends PhabricatorEditor { $question = $this->question; $question->save(); - // search index $question->attachRelated(); - PhabricatorSearchPonderIndexer::indexQuestion($question); + id(new PhabricatorSearchIndexer()) + ->indexDocumentByPHID($question->getPHID()); // subscribe author and @mentions $subeditor = id(new PhabricatorSubscriptionsEditor()) diff --git a/src/applications/ponder/search/PhabricatorSearchPonderIndexer.php b/src/applications/ponder/search/PonderSearchIndexer.php similarity index 87% rename from src/applications/ponder/search/PhabricatorSearchPonderIndexer.php rename to src/applications/ponder/search/PonderSearchIndexer.php index 755b4f4b35..ebdcffcef9 100644 --- a/src/applications/ponder/search/PhabricatorSearchPonderIndexer.php +++ b/src/applications/ponder/search/PonderSearchIndexer.php @@ -1,10 +1,16 @@ loadDocumentByPHID($phid); + + $question->attachRelated(); $doc = new PhabricatorSearchAbstractDocument(); $doc->setPHID($question->getPHID()); @@ -61,6 +67,6 @@ final class PhabricatorSearchPonderIndexer $question->getDateModified()); // Bogus timestamp. } - self::reindexAbstractDocument($doc); + return $doc; } } diff --git a/src/applications/search/index/indexer/PhabricatorSearchCommitIndexer.php b/src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php similarity index 87% rename from src/applications/search/index/indexer/PhabricatorSearchCommitIndexer.php rename to src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php index f4f50093d1..9097185c51 100644 --- a/src/applications/search/index/indexer/PhabricatorSearchCommitIndexer.php +++ b/src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php @@ -1,12 +1,15 @@ loadDocumentByPHID($phid); + $commit_data = id(new PhabricatorRepositoryCommitData())->loadOneWhere( 'commitID = %d', $commit->getID()); @@ -19,7 +22,7 @@ final class PhabricatorSearchCommitIndexer $commit->getRepositoryID()); if (!$repository) { - return; + throw new Exception("No such repository!"); } $title = 'r'.$repository->getCallsign().$commit->getCommitIdentifier(). @@ -75,7 +78,7 @@ final class PhabricatorSearchCommitIndexer } } - self::reindexAbstractDocument($doc); + return $doc; } } diff --git a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php index 1e53098435..10b99993cc 100644 --- a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php +++ b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php @@ -57,7 +57,9 @@ abstract class PhabricatorRepositoryCommitChangeParserWorker protected function finishParse() { $commit = $this->commit; - PhabricatorSearchCommitIndexer::indexCommit($commit); + + id(new PhabricatorSearchIndexer()) + ->indexDocumentByPHID($commit->getPHID()); PhabricatorOwnersPackagePathValidator::updateOwnersPackagePaths($commit); if ($this->shouldQueueFollowupTasks()) { diff --git a/src/applications/search/index/PhabricatorSearchDocumentIndexer.php b/src/applications/search/index/PhabricatorSearchDocumentIndexer.php new file mode 100644 index 0000000000..e0fa8afe6d --- /dev/null +++ b/src/applications/search/index/PhabricatorSearchDocumentIndexer.php @@ -0,0 +1,54 @@ +getIndexableObject(); + return (phid_get_type($phid) == phid_get_type($object->generatePHID())); + } + + public function getIndexIterator() { + $object = $this->getIndexableObject(); + return new LiskMigrationIterator($object); + } + + protected function loadDocumentByPHID($phid) { + $object = $this->getIndexableObject(); + $document = $object->loadOneWhere('phid = %s', $phid); + if (!$document) { + throw new Exception("Unable to load document by phid '{$phid}'!"); + } + return $document; + } + + public function indexDocumentByPHID($phid) { + try { + $document = $this->buildAbstractDocumentByPHID($phid); + + $engine = PhabricatorSearchEngineSelector::newSelector()->newEngine(); + try { + $engine->reindexAbstractDocument($document); + } catch (Exception $ex) { + $phid = $document->getPHID(); + $class = get_class($engine); + + phlog("Unable to index document {$phid} by engine {$class}."); + phlog($ex); + } + + } catch (Exception $ex) { + $class = get_class($this); + phlog("Unable to build document {$phid} by indexer {$class}."); + phlog($ex); + } + + return $this; + } + +} diff --git a/src/applications/search/index/PhabricatorSearchIndexer.php b/src/applications/search/index/PhabricatorSearchIndexer.php new file mode 100644 index 0000000000..b6c2b13efb --- /dev/null +++ b/src/applications/search/index/PhabricatorSearchIndexer.php @@ -0,0 +1,30 @@ +setAncestorClass('PhabricatorSearchDocumentIndexer') + ->setConcreteOnly(true) + ->setType('class') + ->selectAndLoadSymbols(); + + $indexers = array(); + foreach ($doc_indexer_symbols as $symbol) { + $indexers[] = newv($symbol['name'], array()); + } + + foreach ($indexers as $indexer) { + if ($indexer->shouldIndexDocumentByPHID($phid)) { + $indexer->indexDocumentByPHID($phid); + break; + } + } + + return $this; + } + +} diff --git a/src/applications/search/index/indexer/PhabricatorSearchDocumentIndexer.php b/src/applications/search/index/indexer/PhabricatorSearchDocumentIndexer.php deleted file mode 100644 index b7ea31552c..0000000000 --- a/src/applications/search/index/indexer/PhabricatorSearchDocumentIndexer.php +++ /dev/null @@ -1,21 +0,0 @@ -newEngine(); - try { - $engine->reindexAbstractDocument($document); - } catch (Exception $ex) { - $phid = $document->getPHID(); - $class = get_class($engine); - phlog("Unable to index document {$phid} by engine {$class}."); - } - } - -} diff --git a/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php b/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php new file mode 100644 index 0000000000..aa2135128a --- /dev/null +++ b/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php @@ -0,0 +1,137 @@ +setName('index') + ->setSynopsis('Build or rebuild search indexes.') + ->setExamples( + "**index** D123\n". + "**index** --type DREV\n". + "**index** --all") + ->setArguments( + array( + array( + 'name' => 'all', + 'help' => 'Reindex all documents.', + ), + array( + 'name' => 'type', + 'param' => 'TYPE', + 'help' => 'PHID type to reindex, like "TASK" or "DREV".', + ), + array( + 'name' => 'background', + 'help' => 'Instead of indexing in this process, queue tasks for '. + 'the daemons. This is better if you are indexing a lot '. + 'of stuff, but less helpful for debugging.', + ), + array( + 'name' => 'foreground', + 'help' => 'Index in this process, even if there are many objects '. + 'to index. This is helpful for debugging.', + ), + array( + 'name' => 'objects', + 'wildcard' => true, + ), + ) + ); + } + + public function execute(PhutilArgumentParser $args) { + $console = PhutilConsole::getConsole(); + + $is_all = $args->getArg('all'); + $is_type = $args->getArg('type'); + + $obj_names = $args->getArg('objects'); + + + if ($obj_names && ($is_all || $is_type)) { + throw new PhutilArgumentUsageException( + "You can not name objects to index alongside the '--all' or '--type' ". + "flags."); + } else if (!$obj_names && !($is_all || $is_type)) { + throw new PhutilArgumentUsageException( + "Provide one of '--all', '--type' or a list of object names."); + } + + if ($obj_names) { + $phids = $this->loadPHIDsByNames($obj_names); + } else { + $phids = $this->loadPHIDsByTypes($is_type); + } + + if (!$phids) { + throw new PhutilArgumentUsageException( + "Nothing to index!"); + } + + $groups = phid_group_by_type($phids); + foreach ($groups as $group_type => $group) { + $console->writeOut( + pht( + "Indexing %d object(s) of type %s.", + count($group), + $group_type)."\n"); + } + + $indexer = new PhabricatorSearchIndexer(); + foreach ($phids as $phid) { + $indexer->indexDocumentByPHID($phid); + $console->writeOut(pht("Indexing '%s'...\n", $phid)); + } + + $console->writeOut("Done.\n"); + } + + private function loadPHIDsByNames(array $names) { + $phids = array(); + foreach ($names as $name) { + $phid = PhabricatorPHID::fromObjectName($name); + if (!$phid) { + throw new PhutilArgumentUsageException( + "'{$name}' is not the name of a known object."); + } + $phids[] = $phid; + } + return $phids; + } + + private function loadPHIDsByTypes($type) { + $indexer_symbols = id(new PhutilSymbolLoader()) + ->setAncestorClass('PhabricatorSearchDocumentIndexer') + ->setConcreteOnly(true) + ->setType('class') + ->selectAndLoadSymbols(); + + $indexers = array(); + foreach ($indexer_symbols as $symbol) { + $indexers[] = newv($symbol['name'], array()); + } + + $phids = array(); + foreach ($indexers as $indexer) { + $indexer_phid = $indexer->getIndexableObject()->generatePHID(); + $indexer_type = phid_get_type($indexer_phid); + + if ($type && ($indexer_type != $type)) { + continue; + } + + $iterator = $indexer->getIndexIterator(); + foreach ($iterator as $object) { + $phids[] = $object->getPHID(); + } + } + + return $phids; + } + +} diff --git a/src/applications/search/management/PhabricatorSearchManagementWorkflow.php b/src/applications/search/management/PhabricatorSearchManagementWorkflow.php new file mode 100644 index 0000000000..4f7fad9ef3 --- /dev/null +++ b/src/applications/search/management/PhabricatorSearchManagementWorkflow.php @@ -0,0 +1,13 @@ + array( + "Indexing %d object of type %s.", + "Indexing %d object of type %s.", + ), + ); }