From f4f73e0a7e466146cf6613d59e6e12c3edd24076 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 1 Sep 2017 09:19:01 -0700 Subject: [PATCH] Separate fulltext engine extensions into "enrich" and "index" phases Summary: Ref T12819. Some of the extensions "enrich" the document (adding more fields or relationships), while others "index" it (insert it into some kind of index for later searching). Currently, these are all muddled under a single "index" phase. However, the Ferret extension cares about fields and relationships which other extensions may add. Split this into two phases: "enrich" adds fields and relationships so other extensions can read them later if they want. "Index" happens after the document is built and has all the fields and relationships. The specific problem this solves is that comments may not have been added to the document when the Ferret extension runs. By moving them to the "enrich" phase, the Ferret engine will be able to see and index comments. Test Plan: Ran `bin/search index ...`, grepped for `indexFulltextDocument`. Reviewers: chad Reviewed By: chad Maniphest Tasks: T12819 Differential Revision: https://secure.phabricator.com/D18513 --- ...ricatorProjectsFulltextEngineExtension.php | 4 ++-- ...PhabricatorLiskFulltextEngineExtension.php | 4 ++-- .../index/PhabricatorFulltextEngine.php | 17 +++++++++++++--- .../PhabricatorFulltextEngineExtension.php | 20 ++++++++++++++++--- ...orSubscriptionsFulltextEngineExtension.php | 4 ++-- ...torTransactionsFulltextEngineExtension.php | 4 ++-- ...atorCustomFieldFulltextEngineExtension.php | 4 ++-- 7 files changed, 41 insertions(+), 16 deletions(-) diff --git a/src/applications/project/engineextension/PhabricatorProjectsFulltextEngineExtension.php b/src/applications/project/engineextension/PhabricatorProjectsFulltextEngineExtension.php index 857783f811..79f395ebaa 100644 --- a/src/applications/project/engineextension/PhabricatorProjectsFulltextEngineExtension.php +++ b/src/applications/project/engineextension/PhabricatorProjectsFulltextEngineExtension.php @@ -9,11 +9,11 @@ final class PhabricatorProjectsFulltextEngineExtension return pht('Projects'); } - public function shouldIndexFulltextObject($object) { + public function shouldEnrichFulltextObject($object) { return ($object instanceof PhabricatorProjectInterface); } - public function indexFulltextObject( + public function enrichFulltextObject( $object, PhabricatorSearchAbstractDocument $document) { diff --git a/src/applications/search/engineextension/PhabricatorLiskFulltextEngineExtension.php b/src/applications/search/engineextension/PhabricatorLiskFulltextEngineExtension.php index 9cbe384a10..1626d248da 100644 --- a/src/applications/search/engineextension/PhabricatorLiskFulltextEngineExtension.php +++ b/src/applications/search/engineextension/PhabricatorLiskFulltextEngineExtension.php @@ -9,7 +9,7 @@ final class PhabricatorLiskFulltextEngineExtension return pht('Lisk Builtin Properties'); } - public function shouldIndexFulltextObject($object) { + public function shouldEnrichFulltextObject($object) { if (!($object instanceof PhabricatorLiskDAO)) { return false; } @@ -21,7 +21,7 @@ final class PhabricatorLiskFulltextEngineExtension return true; } - public function indexFulltextObject( + public function enrichFulltextObject( $object, PhabricatorSearchAbstractDocument $document) { diff --git a/src/applications/search/index/PhabricatorFulltextEngine.php b/src/applications/search/index/PhabricatorFulltextEngine.php index 9f20917b3f..e1b2f4471d 100644 --- a/src/applications/search/index/PhabricatorFulltextEngine.php +++ b/src/applications/search/index/PhabricatorFulltextEngine.php @@ -26,9 +26,16 @@ abstract class PhabricatorFulltextEngine $object = $this->getObject(); $extensions = PhabricatorFulltextEngineExtension::getAllExtensions(); + + $enrich_extensions = array(); + $index_extensions = array(); foreach ($extensions as $key => $extension) { - if (!$extension->shouldIndexFulltextObject($object)) { - unset($extensions[$key]); + if ($extension->shouldEnrichFulltextObject($object)) { + $enrich_extensions[] = $extension; + } + + if ($extension->shouldIndexFulltextObject($object)) { + $index_extensions[] = $extension; } } @@ -36,7 +43,11 @@ abstract class PhabricatorFulltextEngine $this->buildAbstractDocument($document, $object); - foreach ($extensions as $extension) { + foreach ($enrich_extensions as $extension) { + $extension->enrichFulltextObject($object, $document); + } + + foreach ($index_extensions as $extension) { $extension->indexFulltextObject($object, $document); } diff --git a/src/applications/search/index/PhabricatorFulltextEngineExtension.php b/src/applications/search/index/PhabricatorFulltextEngineExtension.php index c52b35a58a..52aabe7c8b 100644 --- a/src/applications/search/index/PhabricatorFulltextEngineExtension.php +++ b/src/applications/search/index/PhabricatorFulltextEngineExtension.php @@ -12,11 +12,25 @@ abstract class PhabricatorFulltextEngineExtension extends Phobject { abstract public function getExtensionName(); - abstract public function shouldIndexFulltextObject($object); + public function shouldEnrichFulltextObject($object) { + return false; + } - abstract public function indexFulltextObject( + public function enrichFulltextObject( $object, - PhabricatorSearchAbstractDocument $document); + PhabricatorSearchAbstractDocument $document) { + return; + } + + public function shouldIndexFulltextObject($object) { + return false; + } + + public function indexFulltextObject( + $object, + PhabricatorSearchAbstractDocument $document) { + return; + } final public static function getAllExtensions() { return id(new PhutilClassMapQuery()) diff --git a/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsFulltextEngineExtension.php b/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsFulltextEngineExtension.php index 3c25618ebe..e7876136a1 100644 --- a/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsFulltextEngineExtension.php +++ b/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsFulltextEngineExtension.php @@ -9,11 +9,11 @@ final class PhabricatorSubscriptionsFulltextEngineExtension return pht('Subscribers'); } - public function shouldIndexFulltextObject($object) { + public function shouldEnrichFulltextObject($object) { return ($object instanceof PhabricatorSubscribableInterface); } - public function indexFulltextObject( + public function enrichFulltextObject( $object, PhabricatorSearchAbstractDocument $document) { diff --git a/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php b/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php index b95fc0167a..701ad34ca7 100644 --- a/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php +++ b/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php @@ -9,11 +9,11 @@ final class PhabricatorTransactionsFulltextEngineExtension return pht('Comments'); } - public function shouldIndexFulltextObject($object) { + public function shouldEnrichFulltextObject($object) { return ($object instanceof PhabricatorApplicationTransactionInterface); } - public function indexFulltextObject( + public function enrichFulltextObject( $object, PhabricatorSearchAbstractDocument $document) { diff --git a/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldFulltextEngineExtension.php b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldFulltextEngineExtension.php index 8186ff5311..9e0684cacc 100644 --- a/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldFulltextEngineExtension.php +++ b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldFulltextEngineExtension.php @@ -9,11 +9,11 @@ final class PhabricatorCustomFieldFulltextEngineExtension return pht('Custom Fields'); } - public function shouldIndexFulltextObject($object) { + public function shouldEnrichFulltextObject($object) { return ($object instanceof PhabricatorCustomFieldInterface); } - public function indexFulltextObject( + public function enrichFulltextObject( $object, PhabricatorSearchAbstractDocument $document) { -- 2.51.2