diff --git a/src/applications/differential/customfield/DifferentialSummaryField.php b/src/applications/differential/customfield/DifferentialSummaryField.php index 9db2651f6f..4d95a56eb3 100644 --- a/src/applications/differential/customfield/DifferentialSummaryField.php +++ b/src/applications/differential/customfield/DifferentialSummaryField.php @@ -78,4 +78,15 @@ final class DifferentialSummaryField $xaction->getNewValue()); } + public function shouldAppearInGlobalSearch() { + return true; + } + + public function updateAbstractDocument( + PhabricatorSearchAbstractDocument $document) { + if (strlen($this->getValue())) { + $document->addField('body', $this->getValue()); + } + } + } diff --git a/src/applications/differential/customfield/DifferentialTestPlanField.php b/src/applications/differential/customfield/DifferentialTestPlanField.php index ba22675c7c..3c4dbdadc5 100644 --- a/src/applications/differential/customfield/DifferentialTestPlanField.php +++ b/src/applications/differential/customfield/DifferentialTestPlanField.php @@ -92,4 +92,16 @@ final class DifferentialTestPlanField $xaction->getNewValue()); } + + public function shouldAppearInGlobalSearch() { + return true; + } + + public function updateAbstractDocument( + PhabricatorSearchAbstractDocument $document) { + if (strlen($this->getValue())) { + $document->addField('plan', $this->getValue()); + } + } + } diff --git a/src/applications/differential/search/DifferentialSearchIndexer.php b/src/applications/differential/search/DifferentialSearchIndexer.php index 2dee73de54..9bc27402fe 100644 --- a/src/applications/differential/search/DifferentialSearchIndexer.php +++ b/src/applications/differential/search/DifferentialSearchIndexer.php @@ -1,8 +1,5 @@ setViewer($this->getViewer()) + ->withPHIDs(array($phid)) + ->needReviewerStatus(true) + ->executeOne(); + if (!$object) { + throw new Exception("Unable to load object by phid '{$phid}'!"); + } + return $object; + } + protected function buildAbstractDocumentByPHID($phid) { $rev = $this->loadDocumentByPHID($phid); @@ -20,24 +29,6 @@ final class DifferentialSearchIndexer $doc->setDocumentCreated($rev->getDateCreated()); $doc->setDocumentModified($rev->getDateModified()); - $aux_fields = DifferentialFieldSelector::newSelector() - ->getFieldSpecifications(); - foreach ($aux_fields as $key => $aux_field) { - $aux_field->setUser(PhabricatorUser::getOmnipotentUser()); - if (!$aux_field->shouldAddToSearchIndex()) { - unset($aux_fields[$key]); - } - } - - $aux_fields = DifferentialAuxiliaryField::loadFromStorage( - $rev, - $aux_fields); - foreach ($aux_fields as $aux_field) { - $doc->addField( - $aux_field->getKeyForSearchIndex(), - $aux_field->getValueForSearchIndex()); - } - $doc->addRelationship( PhabricatorSearchRelationship::RELATIONSHIP_AUTHOR, $rev->getAuthorPHID(), @@ -52,29 +43,16 @@ final class DifferentialSearchIndexer DifferentialPHIDTypeRevision::TYPECONST, time()); - $comments = id(new DifferentialCommentQuery()) - ->withRevisionPHIDs(array($rev->getPHID())) - ->execute(); - - $inlines = id(new DifferentialInlineCommentQuery()) - ->withRevisionIDs(array($rev->getID())) - ->withNotDraft(true) - ->execute(); - - foreach (array_merge($comments, $inlines) as $comment) { - if (strlen($comment->getContent())) { - $doc->addField( - PhabricatorSearchField::FIELD_COMMENT, - $comment->getContent()); - } - } - - $rev->loadRelationships(); + $this->indexTransactions( + $doc, + new DifferentialTransactionQuery(), + array($rev->getPHID())); // If a revision needs review, the owners are the reviewers. Otherwise, the // owner is the author (e.g., accepted, rejected, closed). if ($rev->getStatus() == ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { - $reviewers = $rev->getReviewers(); + $reviewers = $rev->getReviewerStatus(); + $reviewers = mpull($reviewers, 'getReviewerPHID', 'getReviewerPHID'); if ($reviewers) { foreach ($reviewers as $phid) { $doc->addRelationship( @@ -98,20 +76,6 @@ final class DifferentialSearchIndexer $rev->getDateCreated()); } - $ccphids = $rev->getCCPHIDs(); - $handles = id(new PhabricatorHandleQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->withPHIDs($ccphids) - ->execute(); - - foreach ($handles as $phid => $handle) { - $doc->addRelationship( - PhabricatorSearchRelationship::RELATIONSHIP_SUBSCRIBER, - $phid, - $handle->getType(), - $rev->getDateModified()); // Bogus timestamp. - } - return $doc; } } diff --git a/src/applications/maniphest/search/ManiphestSearchIndexer.php b/src/applications/maniphest/search/ManiphestSearchIndexer.php index 971488f64c..41e699d762 100644 --- a/src/applications/maniphest/search/ManiphestSearchIndexer.php +++ b/src/applications/maniphest/search/ManiphestSearchIndexer.php @@ -81,8 +81,6 @@ final class ManiphestSearchIndexer time()); } - $this->indexCustomFields($doc, $task); - return $doc; } } diff --git a/src/applications/people/search/PhabricatorUserSearchIndexer.php b/src/applications/people/search/PhabricatorUserSearchIndexer.php index 8d74334ee9..bb0a2a337c 100644 --- a/src/applications/people/search/PhabricatorUserSearchIndexer.php +++ b/src/applications/people/search/PhabricatorUserSearchIndexer.php @@ -25,8 +25,6 @@ final class PhabricatorUserSearchIndexer PhabricatorPeoplePHIDTypeUser::TYPECONST, time()); - $this->indexCustomFields($doc, $user); - return $doc; } } diff --git a/src/applications/ponder/search/PonderSearchIndexer.php b/src/applications/ponder/search/PonderSearchIndexer.php index 854d4fd9d7..82cc13dc9a 100644 --- a/src/applications/ponder/search/PonderSearchIndexer.php +++ b/src/applications/ponder/search/PonderSearchIndexer.php @@ -46,8 +46,6 @@ final class PonderSearchIndexer new PonderAnswerTransactionQuery(), mpull($answers, 'getPHID')); - $this->indexSubscribers($doc); - return $doc; } } diff --git a/src/applications/project/search/PhabricatorProjectSearchIndexer.php b/src/applications/project/search/PhabricatorProjectSearchIndexer.php index a5c8d5dca3..17a0ac7be4 100644 --- a/src/applications/project/search/PhabricatorProjectSearchIndexer.php +++ b/src/applications/project/search/PhabricatorProjectSearchIndexer.php @@ -17,9 +17,6 @@ final class PhabricatorProjectSearchIndexer $doc->setDocumentCreated($project->getDateCreated()); $doc->setDocumentModified($project->getDateModified()); - $this->indexSubscribers($doc); - $this->indexCustomFields($doc, $project); - $doc->addRelationship( $project->isArchived() ? PhabricatorSearchRelationship::RELATIONSHIP_CLOSED diff --git a/src/applications/search/index/PhabricatorSearchDocumentIndexer.php b/src/applications/search/index/PhabricatorSearchDocumentIndexer.php index 1965c55809..4781a8fdd2 100644 --- a/src/applications/search/index/PhabricatorSearchDocumentIndexer.php +++ b/src/applications/search/index/PhabricatorSearchDocumentIndexer.php @@ -37,6 +37,19 @@ abstract class PhabricatorSearchDocumentIndexer { try { $document = $this->buildAbstractDocumentByPHID($phid); + $object = $this->loadDocumentByPHID($phid); + + // Automatically rebuild CustomField indexes if the object uses custom + // fields. + if ($object instanceof PhabricatorCustomFieldInterface) { + $this->indexCustomFields($document, $object); + } + + // Automatically rebuild subscriber indexes if the object is subscribable. + if ($object instanceof PhabricatorSubscribableInterface) { + $this->indexSubscribers($document); + } + $engine = PhabricatorSearchEngineSelector::newSelector()->newEngine(); try { $engine->reindexAbstractDocument($document); @@ -107,7 +120,7 @@ abstract class PhabricatorSearchDocumentIndexer { } protected function indexCustomFields( - PhabricatorSearchAbstractDocument $doc, + PhabricatorSearchAbstractDocument $document, PhabricatorCustomFieldInterface $object) { // Rebuild the ApplicationSearch indexes. These are internal and not part of @@ -116,17 +129,16 @@ abstract class PhabricatorSearchDocumentIndexer { $field_list = PhabricatorCustomField::getObjectFields( $object, - PhabricatorCustomField::ROLE_APPLICATIONSEARCH); + PhabricatorCustomField::ROLE_DEFAULT); $field_list->setViewer($this->getViewer()); $field_list->readFieldsFromStorage($object); + + // Rebuild ApplicationSearch indexes. $field_list->rebuildIndexes($object); - // We could also allow fields to provide fulltext content, and index it - // here on the document. No one has asked for this yet, though, and the - // existing "search" key isn't a good fit to interpret to mean we should - // index stuff here, since it can be set on a lot of fields which don't - // contain anything resembling fulltext. + // Rebuild global search indexes. + $field_list->updateAbstractDocument($document); } private function dispatchDidUpdateIndexEvent( diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php index 133bf382b8..3cd46b0690 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomField.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomField.php @@ -1,16 +1,17 @@ shouldAppearInPropertyView(); case self::ROLE_LIST: return $this->shouldAppearInListView(); + case self::ROLE_GLOBALSEARCH: + return $this->shouldAppearInGlobalSearch(); case self::ROLE_DEFAULT: return true; default: @@ -1091,4 +1095,30 @@ abstract class PhabricatorCustomField { } +/* -( Global Search )------------------------------------------------------ */ + + + /** + * @task globalsearch + */ + public function shouldAppearInGlobalSearch() { + if ($this->proxy) { + return $this->proxy->shouldAppearInGlobalSearch(); + } + return false; + } + + + /** + * @task globalsearch + */ + public function updateAbstractDocument( + PhabricatorSearchAbstractDocument $document) { + if ($this->proxy) { + return $this->proxy->updateAbstractDocument($document); + } + return $document; + } + + } diff --git a/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php b/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php index ffe96bc285..dae4bd507a 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php @@ -52,7 +52,7 @@ final class PhabricatorCustomFieldList extends Phobject { } if (!$keys) { - return; + return $this; } // NOTE: We assume all fields share the same storage. This isn't guaranteed @@ -79,6 +79,8 @@ final class PhabricatorCustomFieldList extends Phobject { $field->setValueFromStorage(null); } } + + return $this; } public function appendFieldsToForm(AphrontFormView $form) { @@ -304,4 +306,19 @@ final class PhabricatorCustomFieldList extends Phobject { $any_index->saveTransaction(); } + public function updateAbstractDocument( + PhabricatorSearchAbstractDocument $document) { + + $role = PhabricatorCustomField::ROLE_GLOBALSEARCH; + foreach ($this->getFields() as $field) { + if (!$field->shouldEnableForRole($role)) { + continue; + } + $field->updateAbstractDocument($document); + } + + return $document; + } + + }