From 02f82c2af5c49e467369669a5b1bcdffdfa6fb03 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 21 Dec 2015 06:31:31 -0800 Subject: [PATCH] Modularize fulltext indexing of Projects, Subscriptions and Custom Fields Summary: Ref T9979. This is going to become `FulltextEngine`, but pave the way for that by pulling extensions out of it. Test Plan: {F1036624} - Used `bin/search index Txxx`, saw projects, subscribers and custom fields rebuild in the index. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9979 Differential Revision: https://secure.phabricator.com/D14835 --- src/__phutil_library_map__.php | 10 +++ ...ricatorProjectsFulltextEngineExtension.php | 37 +++++++++ .../PhabricatorFulltextEngineExtension.php | 28 +++++++ ...abricatorFulltextEngineExtensionModule.php | 44 +++++++++++ .../PhabricatorSearchDocumentIndexer.php | 78 ++----------------- ...orSubscriptionsFulltextEngineExtension.php | 41 ++++++++++ ...atorCustomFieldFulltextEngineExtension.php | 39 ++++++++++ 7 files changed, 206 insertions(+), 71 deletions(-) create mode 100644 src/applications/project/engineextension/PhabricatorProjectsFulltextEngineExtension.php create mode 100644 src/applications/search/index/PhabricatorFulltextEngineExtension.php create mode 100644 src/applications/search/index/PhabricatorFulltextEngineExtensionModule.php create mode 100644 src/applications/subscriptions/engineextension/PhabricatorSubscriptionsFulltextEngineExtension.php create mode 100644 src/infrastructure/customfield/engineextension/PhabricatorCustomFieldFulltextEngineExtension.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6a9b5b3fef..56fab4db41 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2017,6 +2017,7 @@ phutil_register_library_map(array( 'PhabricatorCustomFieldEditEngineExtension' => 'infrastructure/customfield/engineextension/PhabricatorCustomFieldEditEngineExtension.php', 'PhabricatorCustomFieldEditField' => 'infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php', 'PhabricatorCustomFieldEditType' => 'infrastructure/customfield/editor/PhabricatorCustomFieldEditType.php', + 'PhabricatorCustomFieldFulltextEngineExtension' => 'infrastructure/customfield/engineextension/PhabricatorCustomFieldFulltextEngineExtension.php', 'PhabricatorCustomFieldHeraldField' => 'infrastructure/customfield/herald/PhabricatorCustomFieldHeraldField.php', 'PhabricatorCustomFieldHeraldFieldGroup' => 'infrastructure/customfield/herald/PhabricatorCustomFieldHeraldFieldGroup.php', 'PhabricatorCustomFieldImplementationIncompleteException' => 'infrastructure/customfield/exception/PhabricatorCustomFieldImplementationIncompleteException.php', @@ -2326,6 +2327,8 @@ phutil_register_library_map(array( 'PhabricatorFlaggableInterface' => 'applications/flag/interface/PhabricatorFlaggableInterface.php', 'PhabricatorFlagsApplication' => 'applications/flag/application/PhabricatorFlagsApplication.php', 'PhabricatorFlagsUIEventListener' => 'applications/flag/events/PhabricatorFlagsUIEventListener.php', + 'PhabricatorFulltextEngineExtension' => 'applications/search/index/PhabricatorFulltextEngineExtension.php', + 'PhabricatorFulltextEngineExtensionModule' => 'applications/search/index/PhabricatorFulltextEngineExtensionModule.php', 'PhabricatorFundApplication' => 'applications/fund/application/PhabricatorFundApplication.php', 'PhabricatorGDSetupCheck' => 'applications/config/check/PhabricatorGDSetupCheck.php', 'PhabricatorGarbageCollector' => 'infrastructure/daemon/garbagecollector/PhabricatorGarbageCollector.php', @@ -2870,6 +2873,7 @@ phutil_register_library_map(array( 'PhabricatorProjectWatchController' => 'applications/project/controller/PhabricatorProjectWatchController.php', 'PhabricatorProjectsEditEngineExtension' => 'applications/project/engineextension/PhabricatorProjectsEditEngineExtension.php', 'PhabricatorProjectsEditField' => 'applications/transactions/editfield/PhabricatorProjectsEditField.php', + 'PhabricatorProjectsFulltextEngineExtension' => 'applications/project/engineextension/PhabricatorProjectsFulltextEngineExtension.php', 'PhabricatorProjectsPolicyRule' => 'applications/project/policyrule/PhabricatorProjectsPolicyRule.php', 'PhabricatorProjectsSearchEngineAttachment' => 'applications/project/engineextension/PhabricatorProjectsSearchEngineAttachment.php', 'PhabricatorProjectsSearchEngineExtension' => 'applications/project/engineextension/PhabricatorProjectsSearchEngineExtension.php', @@ -3168,6 +3172,7 @@ phutil_register_library_map(array( 'PhabricatorSubscriptionsEditController' => 'applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php', 'PhabricatorSubscriptionsEditEngineExtension' => 'applications/subscriptions/engineextension/PhabricatorSubscriptionsEditEngineExtension.php', 'PhabricatorSubscriptionsEditor' => 'applications/subscriptions/editor/PhabricatorSubscriptionsEditor.php', + 'PhabricatorSubscriptionsFulltextEngineExtension' => 'applications/subscriptions/engineextension/PhabricatorSubscriptionsFulltextEngineExtension.php', 'PhabricatorSubscriptionsHeraldAction' => 'applications/subscriptions/herald/PhabricatorSubscriptionsHeraldAction.php', 'PhabricatorSubscriptionsListController' => 'applications/subscriptions/controller/PhabricatorSubscriptionsListController.php', 'PhabricatorSubscriptionsRemoveSelfHeraldAction' => 'applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSelfHeraldAction.php', @@ -6177,6 +6182,7 @@ phutil_register_library_map(array( 'PhabricatorCustomFieldEditEngineExtension' => 'PhabricatorEditEngineExtension', 'PhabricatorCustomFieldEditField' => 'PhabricatorEditField', 'PhabricatorCustomFieldEditType' => 'PhabricatorEditType', + 'PhabricatorCustomFieldFulltextEngineExtension' => 'PhabricatorFulltextEngineExtension', 'PhabricatorCustomFieldHeraldField' => 'HeraldField', 'PhabricatorCustomFieldHeraldFieldGroup' => 'HeraldFieldGroup', 'PhabricatorCustomFieldImplementationIncompleteException' => 'Exception', @@ -6543,6 +6549,8 @@ phutil_register_library_map(array( 'PhabricatorFlaggableInterface' => 'PhabricatorPHIDInterface', 'PhabricatorFlagsApplication' => 'PhabricatorApplication', 'PhabricatorFlagsUIEventListener' => 'PhabricatorEventListener', + 'PhabricatorFulltextEngineExtension' => 'Phobject', + 'PhabricatorFulltextEngineExtensionModule' => 'PhabricatorConfigModule', 'PhabricatorFundApplication' => 'PhabricatorApplication', 'PhabricatorGDSetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorGarbageCollector' => 'Phobject', @@ -7176,6 +7184,7 @@ phutil_register_library_map(array( 'PhabricatorProjectWatchController' => 'PhabricatorProjectController', 'PhabricatorProjectsEditEngineExtension' => 'PhabricatorEditEngineExtension', 'PhabricatorProjectsEditField' => 'PhabricatorTokenizerEditField', + 'PhabricatorProjectsFulltextEngineExtension' => 'PhabricatorFulltextEngineExtension', 'PhabricatorProjectsPolicyRule' => 'PhabricatorPolicyRule', 'PhabricatorProjectsSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment', 'PhabricatorProjectsSearchEngineExtension' => 'PhabricatorSearchEngineExtension', @@ -7530,6 +7539,7 @@ phutil_register_library_map(array( 'PhabricatorSubscriptionsEditController' => 'PhabricatorController', 'PhabricatorSubscriptionsEditEngineExtension' => 'PhabricatorEditEngineExtension', 'PhabricatorSubscriptionsEditor' => 'PhabricatorEditor', + 'PhabricatorSubscriptionsFulltextEngineExtension' => 'PhabricatorFulltextEngineExtension', 'PhabricatorSubscriptionsHeraldAction' => 'HeraldAction', 'PhabricatorSubscriptionsListController' => 'PhabricatorController', 'PhabricatorSubscriptionsRemoveSelfHeraldAction' => 'PhabricatorSubscriptionsHeraldAction', diff --git a/src/applications/project/engineextension/PhabricatorProjectsFulltextEngineExtension.php b/src/applications/project/engineextension/PhabricatorProjectsFulltextEngineExtension.php new file mode 100644 index 0000000000..857783f811 --- /dev/null +++ b/src/applications/project/engineextension/PhabricatorProjectsFulltextEngineExtension.php @@ -0,0 +1,37 @@ +getPHID(), + PhabricatorProjectObjectHasProjectEdgeType::EDGECONST); + + if (!$project_phids) { + return; + } + + foreach ($project_phids as $project_phid) { + $document->addRelationship( + PhabricatorSearchRelationship::RELATIONSHIP_PROJECT, + $project_phid, + PhabricatorProjectProjectPHIDType::TYPECONST, + $document->getDocumentModified()); // Bogus timestamp. + } + } + +} diff --git a/src/applications/search/index/PhabricatorFulltextEngineExtension.php b/src/applications/search/index/PhabricatorFulltextEngineExtension.php new file mode 100644 index 0000000000..c52b35a58a --- /dev/null +++ b/src/applications/search/index/PhabricatorFulltextEngineExtension.php @@ -0,0 +1,28 @@ +getPhobjectClassConstant('EXTENSIONKEY'); + } + + final protected function getViewer() { + return PhabricatorUser::getOmnipotentUser(); + } + + abstract public function getExtensionName(); + + abstract public function shouldIndexFulltextObject($object); + + abstract public function indexFulltextObject( + $object, + PhabricatorSearchAbstractDocument $document); + + final public static function getAllExtensions() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getExtensionKey') + ->execute(); + } + +} diff --git a/src/applications/search/index/PhabricatorFulltextEngineExtensionModule.php b/src/applications/search/index/PhabricatorFulltextEngineExtensionModule.php new file mode 100644 index 0000000000..7e42d6e5f5 --- /dev/null +++ b/src/applications/search/index/PhabricatorFulltextEngineExtensionModule.php @@ -0,0 +1,44 @@ +getViewer(); + + $extensions = PhabricatorFulltextEngineExtension::getAllExtensions(); + + $rows = array(); + foreach ($extensions as $extension) { + $rows[] = array( + get_class($extension), + $extension->getExtensionName(), + ); + } + + $table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Class'), + pht('Name'), + )) + ->setColumnClasses( + array( + null, + 'wide pri', + )); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('FulltextEngine Extensions')) + ->setTable($table); + } + +} diff --git a/src/applications/search/index/PhabricatorSearchDocumentIndexer.php b/src/applications/search/index/PhabricatorSearchDocumentIndexer.php index a9ecc3e9c0..be2d38467a 100644 --- a/src/applications/search/index/PhabricatorSearchDocumentIndexer.php +++ b/src/applications/search/index/PhabricatorSearchDocumentIndexer.php @@ -52,20 +52,15 @@ abstract class PhabricatorSearchDocumentIndexer extends Phobject { $object = $this->loadDocumentByPHID($phid); - // Automatically rebuild CustomField indexes if the object uses custom - // fields. - if ($object instanceof PhabricatorCustomFieldInterface) { - $this->indexCustomFields($document, $object); + $extensions = PhabricatorFulltextEngineExtension::getAllExtensions(); + foreach ($extensions as $key => $extension) { + if (!$extension->shouldIndexFulltextObject($object)) { + unset($extensions[$key]); + } } - // Automatically rebuild subscriber indexes if the object is subscribable. - if ($object instanceof PhabricatorSubscribableInterface) { - $this->indexSubscribers($document); - } - - // Automatically build project relationships - if ($object instanceof PhabricatorProjectInterface) { - $this->indexProjects($document, $object); + foreach ($extensions as $extension) { + $extension->indexFulltextObject($object, $document); } $engine = PhabricatorSearchEngine::loadEngine(); @@ -82,43 +77,6 @@ abstract class PhabricatorSearchDocumentIndexer extends Phobject { ->setDocumentType(phid_get_type($phid)); } - protected function indexSubscribers( - PhabricatorSearchAbstractDocument $doc) { - - $subscribers = PhabricatorSubscribersQuery::loadSubscribersForPHID( - $doc->getPHID()); - $handles = id(new PhabricatorHandleQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs($subscribers) - ->execute(); - - foreach ($handles as $phid => $handle) { - $doc->addRelationship( - PhabricatorSearchRelationship::RELATIONSHIP_SUBSCRIBER, - $phid, - $handle->getType(), - $doc->getDocumentModified()); // Bogus timestamp. - } - } - - protected function indexProjects( - PhabricatorSearchAbstractDocument $doc, - PhabricatorProjectInterface $object) { - - $project_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( - $object->getPHID(), - PhabricatorProjectObjectHasProjectEdgeType::EDGECONST); - if ($project_phids) { - foreach ($project_phids as $project_phid) { - $doc->addRelationship( - PhabricatorSearchRelationship::RELATIONSHIP_PROJECT, - $project_phid, - PhabricatorProjectProjectPHIDType::TYPECONST, - $doc->getDocumentModified()); // Bogus timestamp. - } - } - } - protected function indexTransactions( PhabricatorSearchAbstractDocument $doc, PhabricatorApplicationTransactionQuery $query, @@ -141,28 +99,6 @@ abstract class PhabricatorSearchDocumentIndexer extends Phobject { } } - protected function indexCustomFields( - PhabricatorSearchAbstractDocument $document, - PhabricatorCustomFieldInterface $object) { - - // Rebuild the ApplicationSearch indexes. These are internal and not part of - // the fulltext search, but putting them in this workflow allows users to - // use the same tools to rebuild the indexes, which is easy to understand. - - $field_list = PhabricatorCustomField::getObjectFields( - $object, - PhabricatorCustomField::ROLE_DEFAULT); - - $field_list->setViewer($this->getViewer()); - $field_list->readFieldsFromStorage($object); - - // Rebuild ApplicationSearch indexes. - $field_list->rebuildIndexes($object); - - // Rebuild global search indexes. - $field_list->updateAbstractDocument($document); - } - private function dispatchDidUpdateIndexEvent( $phid, PhabricatorSearchAbstractDocument $document) { diff --git a/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsFulltextEngineExtension.php b/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsFulltextEngineExtension.php new file mode 100644 index 0000000000..3c25618ebe --- /dev/null +++ b/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsFulltextEngineExtension.php @@ -0,0 +1,41 @@ +getPHID()); + + if (!$subscriber_phids) { + return; + } + + $handles = id(new PhabricatorHandleQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($subscriber_phids) + ->execute(); + + foreach ($handles as $phid => $handle) { + $document->addRelationship( + PhabricatorSearchRelationship::RELATIONSHIP_SUBSCRIBER, + $phid, + $handle->getType(), + $document->getDocumentModified()); // Bogus timestamp. + } + } + +} diff --git a/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldFulltextEngineExtension.php b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldFulltextEngineExtension.php new file mode 100644 index 0000000000..8186ff5311 --- /dev/null +++ b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldFulltextEngineExtension.php @@ -0,0 +1,39 @@ +setViewer($this->getViewer()); + $field_list->readFieldsFromStorage($object); + + // Rebuild ApplicationSearch indexes. + $field_list->rebuildIndexes($object); + + // Rebuild global search indexes. + $field_list->updateAbstractDocument($document); + } + +}