Modularize transaction/comment indexing in the FulltextEngine

Summary: Ref T9979. This is currently hard-coded but can be done in a generic way. This has one minor behavioral changes: answer text is no longer included in the question text index in Ponder. I'm not planning to accommodate that for now since I don't want to dig this hole any deeper than I already have. This behavior should be different anyway (e.g., index the answer, then show the question in the results or something). Test Plan: - Put a unique word in a Maniphest comment. - Searched for the word. - Found the task. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9979 Differential Revision: https://secure.phabricator.com/D14837
2024-12-22 21:40:55 +01:00 · 2015-12-21 07:08:17 -08:00 · 2015-12-21 07:08:17 -08:00 · ecc3314a25
commit ecc3314a25
parent aab1574e33
15 changed files with 80 additions and 115 deletions
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -3242,6 +3242,7 @@ phutil_register_library_map(array(
    'PhabricatorTransactions' => 'applications/transactions/constants/PhabricatorTransactions.php',
    'PhabricatorTransactionsApplication' => 'applications/transactions/application/PhabricatorTransactionsApplication.php',
    'PhabricatorTransactionsDestructionEngineExtension' => 'applications/transactions/engineextension/PhabricatorTransactionsDestructionEngineExtension.php',
+    'PhabricatorTransactionsFulltextEngineExtension' => 'applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php',
    'PhabricatorTransformedFile' => 'applications/files/storage/PhabricatorTransformedFile.php',
    'PhabricatorTranslationsConfigOptions' => 'applications/config/option/PhabricatorTranslationsConfigOptions.php',
    'PhabricatorTriggerAction' => 'infrastructure/daemon/workers/action/PhabricatorTriggerAction.php',
@ -7614,6 +7615,7 @@ phutil_register_library_map(array(
    'PhabricatorTransactions' => 'Phobject',
    'PhabricatorTransactionsApplication' => 'PhabricatorApplication',
    'PhabricatorTransactionsDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension',
+    'PhabricatorTransactionsFulltextEngineExtension' => 'PhabricatorFulltextEngineExtension',
    'PhabricatorTransformedFile' => 'PhabricatorFileDAO',
    'PhabricatorTranslationsConfigOptions' => 'PhabricatorApplicationConfigOptions',
    'PhabricatorTriggerAction' => 'Phobject',
--- a/src/applications/calendar/search/PhabricatorCalendarEventSearchIndexer.php
+++ b/src/applications/calendar/search/PhabricatorCalendarEventSearchIndexer.php
@ -41,11 +41,6 @@ final class PhabricatorCalendarEventSearchIndexer
      PhabricatorCalendarEventPHIDType::TYPECONST,
      time());

-    $this->indexTransactions(
-      $doc,
-      new PhabricatorCalendarEventTransactionQuery(),
-      array($phid));
-
    return $doc;
  }

--- a/src/applications/differential/search/DifferentialSearchIndexer.php
+++ b/src/applications/differential/search/DifferentialSearchIndexer.php
@ -43,11 +43,6 @@ final class DifferentialSearchIndexer
      DifferentialRevisionPHIDType::TYPECONST,
      time());

-    $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) {
--- a/src/applications/diviner/search/DivinerBookSearchIndexer.php
+++ b/src/applications/diviner/search/DivinerBookSearchIndexer.php
@ -24,11 +24,6 @@ final class DivinerBookSearchIndexer extends PhabricatorSearchDocumentIndexer {
      PhabricatorRepositoryRepositoryPHIDType::TYPECONST,
      $book->getDateCreated());

-    $this->indexTransactions(
-      $doc,
-      new DivinerLiveBookTransactionQuery(),
-      array($phid));
-
    return $doc;
  }

--- a/src/applications/fund/search/FundInitiativeIndexer.php
+++ b/src/applications/fund/search/FundInitiativeIndexer.php
@ -51,11 +51,6 @@ final class FundInitiativeIndexer
      FundInitiativePHIDType::TYPECONST,
      time());

-    $this->indexTransactions(
-      $doc,
-      new FundInitiativeTransactionQuery(),
-      array($initiative->getPHID()));
-
    return $doc;
  }
 }
--- a/src/applications/maniphest/search/ManiphestSearchIndexer.php
+++ b/src/applications/maniphest/search/ManiphestSearchIndexer.php
@ -34,11 +34,6 @@ final class ManiphestSearchIndexer extends PhabricatorSearchDocumentIndexer {
      ManiphestTaskPHIDType::TYPECONST,
      time());

-    $this->indexTransactions(
-      $doc,
-      new ManiphestTransactionQuery(),
-      array($phid));
-
    $owner = $task->getOwnerPHID();
    if ($owner) {
      $doc->addRelationship(
--- a/src/applications/passphrase/search/PassphraseSearchIndexer.php
+++ b/src/applications/passphrase/search/PassphraseSearchIndexer.php
@ -28,11 +28,6 @@ final class PassphraseSearchIndexer extends PhabricatorSearchDocumentIndexer {
      PassphraseCredentialPHIDType::TYPECONST,
      time());

-    $this->indexTransactions(
-      $doc,
-      new PassphraseCredentialTransactionQuery(),
-      array($phid));
-
    return $doc;
  }

--- a/src/applications/pholio/search/PholioSearchIndexer.php
+++ b/src/applications/pholio/search/PholioSearchIndexer.php
@ -24,11 +24,6 @@ final class PholioSearchIndexer extends PhabricatorSearchDocumentIndexer {
      PhabricatorPeopleUserPHIDType::TYPECONST,
      $mock->getDateCreated());

-    $this->indexTransactions(
-      $doc,
-      new PholioTransactionQuery(),
-      array($phid));
-
    return $doc;
  }

--- a/src/applications/ponder/search/PonderSearchIndexer.php
+++ b/src/applications/ponder/search/PonderSearchIndexer.php
@ -37,15 +37,6 @@ final class PonderSearchIndexer
      }
    }

-    $this->indexTransactions(
-      $doc,
-      new PonderQuestionTransactionQuery(),
-      array($phid));
-    $this->indexTransactions(
-      $doc,
-      new PonderAnswerTransactionQuery(),
-      mpull($answers, 'getPHID'));
-
    return $doc;
  }
 }
--- a/src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php
+++ b/src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php
@ -53,11 +53,6 @@ final class PhabricatorRepositoryCommitSearchIndexer
      PhabricatorRepositoryRepositoryPHIDType::TYPECONST,
      $date_created);

-    $this->indexTransactions(
-      $doc,
-      new PhabricatorAuditTransactionQuery(),
-      array($commit->getPHID()));
-
    return $doc;
  }
 }
--- a/src/applications/search/index/PhabricatorSearchDocumentIndexer.php
+++ b/src/applications/search/index/PhabricatorSearchDocumentIndexer.php
@ -75,26 +75,4 @@ abstract class PhabricatorSearchDocumentIndexer extends Phobject {
      ->setDocumentType(phid_get_type($phid));
  }

-  protected function indexTransactions(
-    PhabricatorSearchAbstractDocument $doc,
-    PhabricatorApplicationTransactionQuery $query,
-    array $phids) {
-
-    $xactions = id(clone $query)
-      ->setViewer($this->getViewer())
-      ->withObjectPHIDs($phids)
-      ->execute();
-
-    foreach ($xactions as $xaction) {
-      if (!$xaction->hasComment()) {
-        continue;
-      }
-
-      $comment = $xaction->getComment();
-      $doc->addField(
-        PhabricatorSearchDocumentFieldType::FIELD_COMMENT,
-        $comment->getContent());
-    }
-  }
-
 }
--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php
+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php
@ -17,30 +17,17 @@ final class PhabricatorApplicationTransactionShowOlderController
    if (!$object) {
      return new Aphront404Response();
    }
+
    if (!$object instanceof PhabricatorApplicationTransactionInterface) {
      return new Aphront404Response();
    }

-    $template = $object->getApplicationTransactionTemplate();
-    $queries = id(new PhutilClassMapQuery())
-      ->setAncestorClass('PhabricatorApplicationTransactionQuery')
-      ->execute();
-
-    $object_query = null;
-    foreach ($queries as $query) {
-      if ($query->getTemplateApplicationTransaction() == $template) {
-        $object_query = $query;
-        break;
-      }
-    }
-
-    if (!$object_query) {
+    $query = PhabricatorApplicationTransactionQuery::newQueryForObject($object);
+    if (!$query) {
      return new Aphront404Response();
    }

-    $timeline = $this->buildTransactionTimeline(
-      $object,
-      $query);
+    $timeline = $this->buildTransactionTimeline($object, $query);

    $phui_timeline = $timeline->buildPHUITimelineView($with_hiding = false);
    $phui_timeline->setShouldAddSpacers(false);
--- a/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php
+++ b/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php
@ -0,0 +1,44 @@
+<?php
+
+final class PhabricatorTransactionsFulltextEngineExtension
+  extends PhabricatorFulltextEngineExtension {
+
+  const EXTENSIONKEY = 'transactions';
+
+  public function getExtensionName() {
+    return pht('Comments');
+  }
+
+  public function shouldIndexFulltextObject($object) {
+    return ($object instanceof PhabricatorApplicationTransactionInterface);
+  }
+
+  public function indexFulltextObject(
+    $object,
+    PhabricatorSearchAbstractDocument $document) {
+
+    $query = PhabricatorApplicationTransactionQuery::newQueryForObject($object);
+    if (!$query) {
+      return;
+    }
+
+    $xactions = $query
+      ->setViewer($this->getViewer())
+      ->withObjectPHIDs(array($object->getPHID()))
+      ->needComments(true)
+      ->execute();
+
+    foreach ($xactions as $xaction) {
+      if (!$xaction->hasComment()) {
+        continue;
+      }
+
+      $comment = $xaction->getComment();
+
+      $document->addField(
+        PhabricatorSearchDocumentFieldType::FIELD_COMMENT,
+        $comment->getContent());
+    }
+  }
+
+}
--- a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php
+++ b/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php
@ -11,6 +11,27 @@ abstract class PhabricatorApplicationTransactionQuery
  private $needComments = true;
  private $needHandles  = true;

+  final public static function newQueryForObject(
+    PhabricatorApplicationTransactionInterface $object) {
+
+    $xaction = $object->getApplicationTransactionTemplate();
+    $target_class = get_class($xaction);
+
+    $queries = id(new PhutilClassMapQuery())
+      ->setAncestorClass(__CLASS__)
+      ->execute();
+    foreach ($queries as $query) {
+      $query_xaction = $query->getTemplateApplicationTransaction();
+      $query_class = get_class($query_xaction);
+
+      if ($query_class === $target_class) {
+        return id(clone $query);
+      }
+    }
+
+    return null;
+  }
+
  abstract public function getTemplateApplicationTransaction();

  protected function buildMoreWhereClauses(AphrontDatabaseConnection $conn_r) {
--- a/src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php
+++ b/src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php
@ -92,8 +92,15 @@ final class PhabricatorApplicationTransactionPublishWorker

    $viewer = PhabricatorUser::getOmnipotentUser();

-    $type = phid_get_subtype(head($xaction_phids));
-    $xactions = $this->buildTransactionQuery($type)
+    $query = PhabricatorApplicationTransactionQuery::newQueryForObject($object);
+    if (!$query) {
+      throw new PhabricatorWorkerPermanentFailureException(
+        pht(
+          'Unable to load query for transaction object "%s"!',
+          $object->getPHID()));
+    }
+
+    $xactions = $query
      ->setViewer($viewer)
      ->withPHIDs($xaction_phids)
      ->needComments(true)
@ -111,29 +118,4 @@ final class PhabricatorApplicationTransactionPublishWorker
    return array_select_keys($xactions, $xaction_phids);
  }

-
-  /**
-   * Build a new transaction query of the appropriate class so we can load
-   * the transactions.
-   */
-  private function buildTransactionQuery($type) {
-    $queries = id(new PhutilClassMapQuery())
-      ->setAncestorClass('PhabricatorApplicationTransactionQuery')
-      ->execute();
-
-    foreach ($queries as $query) {
-      $query_type = $query
-        ->getTemplateApplicationTransaction()
-        ->getApplicationTransactionType();
-      if ($query_type == $type) {
-        return $query;
-      }
-    }
-
-    throw new PhabricatorWorkerPermanentFailureException(
-      pht(
-        'Unable to load query for transaction type "%s"!',
-        $type));
-  }
-
 }