From 6e57582aff82d295ca558eec23c401b33d9a6dcd Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 31 Jul 2016 08:19:17 -0700 Subject: [PATCH] Allow `*.search` Conduit API methods to have data bulk-loaded by extensions Summary: Ref T11404. Currently, SearchEngineAttachments can bulk-load data but SearchEngineExtensions can not. This leads to poor performance of custom fields. See T11404 for discussion. This changes the API to support a bulk load + format pattern like the one Attachments use. The next change will use it to bulk-load custom field data. Test Plan: - Ran `differential.query`, `differential.revision.search` as a sanity check. - No behavioral changes are expected - See next revision. Reviewers: yelirekim, chad Reviewed By: chad Maniphest Tasks: T11404 Differential Revision: https://secure.phabricator.com/D16350 --- .../query/ConduitResultSearchEngineExtension.php | 2 +- .../PhabricatorPolicySearchEngineExtension.php | 2 +- .../PhabricatorApplicationSearchEngine.php | 16 ++++++++++++---- .../PhabricatorLiskSearchEngineExtension.php | 2 +- .../PhabricatorSearchEngineExtension.php | 6 +++++- .../PhabricatorSpacesSearchEngineExtension.php | 2 +- ...abricatorCustomFieldSearchEngineExtension.php | 2 +- 7 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/applications/conduit/query/ConduitResultSearchEngineExtension.php b/src/applications/conduit/query/ConduitResultSearchEngineExtension.php index 9bf4b1a346..f3ca974fde 100644 --- a/src/applications/conduit/query/ConduitResultSearchEngineExtension.php +++ b/src/applications/conduit/query/ConduitResultSearchEngineExtension.php @@ -25,7 +25,7 @@ final class ConduitResultSearchEngineExtension return $object->getFieldSpecificationsForConduit(); } - public function getFieldValuesForConduit($object) { + public function getFieldValuesForConduit($object, $data) { return $object->getFieldValuesForConduit(); } diff --git a/src/applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php b/src/applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php index a2bb2271f4..2b603bc95f 100644 --- a/src/applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php +++ b/src/applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php @@ -30,7 +30,7 @@ final class PhabricatorPolicySearchEngineExtension ); } - public function getFieldValuesForConduit($object) { + public function getFieldValuesForConduit($object, $data) { $capabilities = $object->getCapabilities(); $map = array(); diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index 3d6b4d9cff..d2ce3ad946 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -1138,6 +1138,11 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { if ($objects) { $field_extensions = $this->getConduitFieldExtensions(); + $extension_data = array(); + foreach ($field_extensions as $key => $extension) { + $extension_data[$key] = $extension->loadExtensionConduitData($objects); + } + $attachment_data = array(); foreach ($attachments as $key => $attachment) { $attachment_data[$key] = $attachment->loadAttachmentData( @@ -1148,7 +1153,8 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { foreach ($objects as $object) { $field_map = $this->getObjectWireFieldsForConduit( $object, - $field_extensions); + $field_extensions, + $extension_data); $attachment_map = array(); foreach ($attachments as $key => $attachment) { @@ -1312,11 +1318,13 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { protected function getObjectWireFieldsForConduit( $object, - array $field_extensions) { + array $field_extensions, + array $extension_data) { $fields = array(); - foreach ($field_extensions as $extension) { - $fields += $extension->getFieldValuesForConduit($object); + foreach ($field_extensions as $key => $extension) { + $data = idx($extension_data, $key, array()); + $fields += $extension->getFieldValuesForConduit($object, $data); } return $fields; diff --git a/src/applications/search/engineextension/PhabricatorLiskSearchEngineExtension.php b/src/applications/search/engineextension/PhabricatorLiskSearchEngineExtension.php index c502ad4cc6..c4d338bc15 100644 --- a/src/applications/search/engineextension/PhabricatorLiskSearchEngineExtension.php +++ b/src/applications/search/engineextension/PhabricatorLiskSearchEngineExtension.php @@ -44,7 +44,7 @@ final class PhabricatorLiskSearchEngineExtension ); } - public function getFieldValuesForConduit($object) { + public function getFieldValuesForConduit($object, $data) { return array( 'dateCreated' => (int)$object->getDateCreated(), 'dateModified' => (int)$object->getDateModified(), diff --git a/src/applications/search/engineextension/PhabricatorSearchEngineExtension.php b/src/applications/search/engineextension/PhabricatorSearchEngineExtension.php index 39af0a3fdd..f93c09cf8f 100644 --- a/src/applications/search/engineextension/PhabricatorSearchEngineExtension.php +++ b/src/applications/search/engineextension/PhabricatorSearchEngineExtension.php @@ -56,7 +56,11 @@ abstract class PhabricatorSearchEngineExtension extends Phobject { return array(); } - public function getFieldValuesForConduit($object) { + public function loadExtensionConduitData(array $objects) { + return null; + } + + public function getFieldValuesForConduit($object, $data) { return array(); } diff --git a/src/applications/spaces/engineextension/PhabricatorSpacesSearchEngineExtension.php b/src/applications/spaces/engineextension/PhabricatorSpacesSearchEngineExtension.php index 7bf92370e4..829b7b2b93 100644 --- a/src/applications/spaces/engineextension/PhabricatorSpacesSearchEngineExtension.php +++ b/src/applications/spaces/engineextension/PhabricatorSpacesSearchEngineExtension.php @@ -63,7 +63,7 @@ final class PhabricatorSpacesSearchEngineExtension ); } - public function getFieldValuesForConduit($object) { + public function getFieldValuesForConduit($object, $data) { return array( 'spacePHID' => $object->getSpacePHID(), ); diff --git a/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php index 246ac713fc..b159b8f89d 100644 --- a/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php +++ b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php @@ -80,7 +80,7 @@ final class PhabricatorCustomFieldSearchEngineExtension return $map; } - public function getFieldValuesForConduit($object) { + public function getFieldValuesForConduit($object, $data) { // TODO: This is currently very inefficient. We should bulk-load these // field values instead.