diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -285,7 +285,7 @@ 'ConpherenceSettings' => 'applications/conpherence/constants/ConpherenceSettings.php', 'ConpherenceTestCase' => 'applications/conpherence/__tests__/ConpherenceTestCase.php', 'ConpherenceThread' => 'applications/conpherence/storage/ConpherenceThread.php', - 'ConpherenceThreadIndexer' => 'applications/conpherence/search/ConpherenceThreadIndexer.php', + 'ConpherenceThreadIndexEngineExtension' => 'applications/conpherence/engineextension/ConpherenceThreadIndexEngineExtension.php', 'ConpherenceThreadListView' => 'applications/conpherence/view/ConpherenceThreadListView.php', 'ConpherenceThreadMailReceiver' => 'applications/conpherence/mail/ConpherenceThreadMailReceiver.php', 'ConpherenceThreadMembersPolicyRule' => 'applications/conpherence/policyrule/ConpherenceThreadMembersPolicyRule.php', @@ -2378,6 +2378,8 @@ 'PhabricatorImageTransformer' => 'applications/files/PhabricatorImageTransformer.php', 'PhabricatorImagemagickSetupCheck' => 'applications/config/check/PhabricatorImagemagickSetupCheck.php', 'PhabricatorIndexEngine' => 'applications/search/index/PhabricatorIndexEngine.php', + 'PhabricatorIndexEngineExtension' => 'applications/search/index/PhabricatorIndexEngineExtension.php', + 'PhabricatorIndexEngineExtensionModule' => 'applications/search/index/PhabricatorIndexEngineExtensionModule.php', 'PhabricatorInfrastructureTestCase' => '__tests__/PhabricatorInfrastructureTestCase.php', 'PhabricatorInlineCommentController' => 'infrastructure/diff/PhabricatorInlineCommentController.php', 'PhabricatorInlineCommentInterface' => 'infrastructure/diff/interface/PhabricatorInlineCommentInterface.php', @@ -4181,7 +4183,7 @@ 'PhabricatorMentionableInterface', 'PhabricatorDestructibleInterface', ), - 'ConpherenceThreadIndexer' => 'PhabricatorSearchDocumentIndexer', + 'ConpherenceThreadIndexEngineExtension' => 'PhabricatorIndexEngineExtension', 'ConpherenceThreadListView' => 'AphrontView', 'ConpherenceThreadMailReceiver' => 'PhabricatorObjectMailReceiver', 'ConpherenceThreadMembersPolicyRule' => 'PhabricatorPolicyRule', @@ -6606,6 +6608,8 @@ 'PhabricatorImageTransformer' => 'Phobject', 'PhabricatorImagemagickSetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorIndexEngine' => 'Phobject', + 'PhabricatorIndexEngineExtension' => 'Phobject', + 'PhabricatorIndexEngineExtensionModule' => 'PhabricatorConfigModule', 'PhabricatorInfrastructureTestCase' => 'PhabricatorTestCase', 'PhabricatorInlineCommentController' => 'PhabricatorController', 'PhabricatorInlineCommentInterface' => 'PhabricatorMarkupInterface', diff --git a/src/applications/conpherence/editor/ConpherenceEditor.php b/src/applications/conpherence/editor/ConpherenceEditor.php --- a/src/applications/conpherence/editor/ConpherenceEditor.php +++ b/src/applications/conpherence/editor/ConpherenceEditor.php @@ -601,22 +601,6 @@ return true; } - protected function getSearchContextParameter( - PhabricatorLiskDAO $object, - array $xactions) { - - $comment_phids = array(); - foreach ($xactions as $xaction) { - if ($xaction->hasComment()) { - $comment_phids[] = $xaction->getPHID(); - } - } - - return array( - 'commentPHIDs' => $comment_phids, - ); - } - protected function extractFilePHIDsFromCustomTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { diff --git a/src/applications/conpherence/search/ConpherenceThreadIndexer.php b/src/applications/conpherence/engineextension/ConpherenceThreadIndexEngineExtension.php rename from src/applications/conpherence/search/ConpherenceThreadIndexer.php rename to src/applications/conpherence/engineextension/ConpherenceThreadIndexEngineExtension.php --- a/src/applications/conpherence/search/ConpherenceThreadIndexer.php +++ b/src/applications/conpherence/engineextension/ConpherenceThreadIndexEngineExtension.php @@ -1,58 +1,50 @@ setViewer($this->getViewer()) - ->withPHIDs(array($phid)) - ->executeOne(); - - if (!$object) { - throw new Exception(pht('No thread "%s" exists!', $phid)); - } + const EXTENSIONKEY = 'conpherence.thread'; - return $object; + public function getExtensionName() { + return pht('Conpherence Threads'); } - protected function buildAbstractDocumentByPHID($phid) { - $thread = $this->loadDocumentByPHID($phid); + public function shouldIndexObject($object) { + return ($object instanceof ConpherenceThread); + } - // NOTE: We're explicitly not building a document here, only rebuilding - // the Conpherence search index. + public function indexObject( + PhabricatorIndexEngine $engine, + $object) { - $context = nonempty($this->getContext(), array()); - $comment_phids = idx($context, 'commentPHIDs'); + $force = $this->shouldForceFullReindex(); - if (is_array($comment_phids) && !$comment_phids) { - // If this property is set, but empty, the transaction did not - // include any chat text. For example, a user might have left the - // conversation. - return null; + if (!$force) { + $xaction_phids = $this->getParameter('transactionPHIDs'); + if (!$xaction_phids) { + return; + } } $query = id(new ConpherenceTransactionQuery()) ->setViewer($this->getViewer()) - ->withObjectPHIDs(array($thread->getPHID())) + ->withObjectPHIDs(array($object->getPHID())) ->withTransactionTypes(array(PhabricatorTransactions::TYPE_COMMENT)) ->needComments(true); - if ($comment_phids !== null) { - $query->withPHIDs($comment_phids); + if (!$force) { + $query->withPHIDs($xaction_phids); } $xactions = $query->execute(); - foreach ($xactions as $xaction) { - $this->indexComment($thread, $xaction); + if (!$xactions) { + return; } - return null; + foreach ($xactions as $xaction) { + $this->indexComment($object, $xaction); + } } private function indexComment( diff --git a/src/applications/search/index/PhabricatorIndexEngine.php b/src/applications/search/index/PhabricatorIndexEngine.php --- a/src/applications/search/index/PhabricatorIndexEngine.php +++ b/src/applications/search/index/PhabricatorIndexEngine.php @@ -2,14 +2,108 @@ final class PhabricatorIndexEngine extends Phobject { - public function indexDocumentByPHID($phid, $context) { + private $object; + private $extensions; + private $versions; + private $parameters; + + public function setParameters(array $parameters) { + $this->parameters = $parameters; + return $this; + } + + public function getParameters() { + return $this->parameters; + } + + public function setObject($object) { + $this->object = $object; + return $this; + } + + public function getObject() { + return $this->object; + } + + public function shouldIndexObject() { + $extensions = $this->newExtensions(); + + $parameters = $this->getParameters(); + foreach ($extensions as $extension) { + $extension->setParameters($parameters); + } + + $object = $this->getObject(); + $versions = array(); + foreach ($extensions as $key => $extension) { + $version = $extension->getIndexVersion($object); + if ($version !== null) { + $versions[$key] = (string)$version; + } + } + + if (idx($parameters, 'force')) { + $current_versions = array(); + } else { + // TODO: Load current indexed versions. + $current_versions = array(); + } + + foreach ($versions as $key => $version) { + $current_version = idx($current_versions, $key); + + if ($current_version === null) { + continue; + } + + // If nothing has changed since we built the current index, we do not + // need to rebuild the index. + if ($current_version === $version) { + unset($extensions[$key]); + } + } + + $this->extensions = $extensions; + $this->versions = $versions; + + // We should index the object only if there is any work to be done. + return (bool)$this->extensions; + } + + public function indexObject() { + $extensions = $this->extensions; + $object = $this->getObject(); + + foreach ($extensions as $key => $extension) { + $extension->indexObject($this, $object); + } + + // TODO: Save new index versions. + + return $this; + } + + private function newExtensions() { + $object = $this->getObject(); + + $extensions = PhabricatorIndexEngineExtension::getAllExtensions(); + foreach ($extensions as $key => $extension) { + if (!$extension->shouldIndexObject($object)) { + unset($extensions[$key]); + } + } + + return $extensions; + } + + public function indexDocumentByPHID($phid) { $indexers = id(new PhutilClassMapQuery()) ->setAncestorClass('PhabricatorSearchDocumentIndexer') ->execute(); foreach ($indexers as $indexer) { if ($indexer->shouldIndexDocumentByPHID($phid)) { - $indexer->indexDocumentByPHID($phid, $context); + $indexer->indexDocumentByPHID($phid); break; } } diff --git a/src/applications/search/index/PhabricatorIndexEngineExtension.php b/src/applications/search/index/PhabricatorIndexEngineExtension.php new file mode 100644 --- /dev/null +++ b/src/applications/search/index/PhabricatorIndexEngineExtension.php @@ -0,0 +1,48 @@ +parameters = $parameters; + return $this; + } + + public function getParameter($key, $default = null) { + return idx($this->parameters, $key, $default); + } + + final public function getExtensionKey() { + return $this->getPhobjectClassConstant('EXTENSIONKEY'); + } + + final protected function getViewer() { + return PhabricatorUser::getOmnipotentUser(); + } + + abstract public function getExtensionName(); + + abstract public function shouldIndexObject($object); + + abstract public function indexObject( + PhabricatorIndexEngine $engine, + $object); + + public function getIndexVersion($object) { + return null; + } + + final public static function getAllExtensions() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getExtensionKey') + ->execute(); + } + + final public function shouldForceFullReindex() { + return $this->getParameter('force'); + } + +} diff --git a/src/applications/search/index/PhabricatorIndexEngineExtensionModule.php b/src/applications/search/index/PhabricatorIndexEngineExtensionModule.php new file mode 100644 --- /dev/null +++ b/src/applications/search/index/PhabricatorIndexEngineExtensionModule.php @@ -0,0 +1,44 @@ +getViewer(); + + $extensions = PhabricatorIndexEngineExtension::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('IndexEngine Extensions')) + ->setTable($table); + } + +} diff --git a/src/applications/search/index/PhabricatorSearchDocumentIndexer.php b/src/applications/search/index/PhabricatorSearchDocumentIndexer.php --- a/src/applications/search/index/PhabricatorSearchDocumentIndexer.php +++ b/src/applications/search/index/PhabricatorSearchDocumentIndexer.php @@ -2,17 +2,6 @@ abstract class PhabricatorSearchDocumentIndexer extends Phobject { - private $context; - - protected function setContext($context) { - $this->context = $context; - return $this; - } - - protected function getContext() { - return $this->context; - } - abstract public function getIndexableObject(); abstract protected function buildAbstractDocumentByPHID($phid); @@ -41,9 +30,7 @@ return $object; } - public function indexDocumentByPHID($phid, $context) { - $this->setContext($context); - + public function indexDocumentByPHID($phid) { $document = $this->buildAbstractDocumentByPHID($phid); if ($document === null) { // This indexer doesn't build a document index, so we're done. diff --git a/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php b/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php --- a/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php +++ b/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php @@ -30,6 +30,13 @@ 'it more difficult to debug search indexing.'), ), array( + 'name' => 'force', + 'short' => 'f', + 'help' => pht( + 'Force a complete rebuild of the entire index instead of an '. + 'incremental update.'), + ), + array( 'name' => 'objects', 'wildcard' => true, ), @@ -41,6 +48,7 @@ $is_all = $args->getArg('all'); $is_type = $args->getArg('type'); + $is_force = $args->getArg('force'); $obj_names = $args->getArg('objects'); @@ -93,10 +101,14 @@ $bar = id(new PhutilConsoleProgressBar()) ->setTotal(count($phids)); + $parameters = array( + 'force' => $is_force, + ); + $any_success = false; foreach ($phids as $phid) { try { - PhabricatorSearchWorker::queueDocumentForIndexing($phid); + PhabricatorSearchWorker::queueDocumentForIndexing($phid, $parameters); $any_success = true; } catch (Exception $ex) { phlog($ex); diff --git a/src/applications/search/worker/PhabricatorSearchWorker.php b/src/applications/search/worker/PhabricatorSearchWorker.php --- a/src/applications/search/worker/PhabricatorSearchWorker.php +++ b/src/applications/search/worker/PhabricatorSearchWorker.php @@ -2,12 +2,16 @@ final class PhabricatorSearchWorker extends PhabricatorWorker { - public static function queueDocumentForIndexing($phid, $context = null) { + public static function queueDocumentForIndexing($phid, $parameters = null) { + if ($parameters === null) { + $parameters = array(); + } + parent::scheduleTask( __CLASS__, array( 'documentPHID' => $phid, - 'context' => $context, + 'parameters' => $parameters, ), array( 'priority' => parent::PRIORITY_IMPORT, @@ -17,9 +21,18 @@ protected function doWork() { $data = $this->getTaskData(); $object_phid = idx($data, 'documentPHID'); - $context = idx($data, 'context'); - $engine = new PhabricatorIndexEngine(); + $object = $this->loadObjectForIndexing($object_phid); + + $engine = id(new PhabricatorIndexEngine()) + ->setObject($object); + + $parameters = idx($data, 'parameters', array()); + $engine->setParameters($parameters); + + if (!$engine->shouldIndexObject()) { + return; + } $key = "index.{$object_phid}"; $lock = PhabricatorGlobalLock::newLock($key); @@ -27,10 +40,15 @@ $lock->lock(1); try { - $object = $this->loadObjectForIndexing($object_phid); + // Reload the object now that we have a lock, to make sure we have the + // most current version. + $object = $this->loadObjectForIndexing($object->getPHID()); + + $engine->setObject($object); - $engine->indexDocumentByPHID($object->getPHID(), $context); + $engine->indexObject(); + $engine->indexDocumentByPHID($object->getPHID()); } catch (Exception $ex) { $lock->unlock(); diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -1095,7 +1095,9 @@ if ($this->supportsSearch()) { PhabricatorSearchWorker::queueDocumentForIndexing( $object->getPHID(), - $this->getSearchContextParameter($object, $xactions)); + array( + 'transactionPHIDs' => mpull($xactions, 'getPHID'), + )); } if ($this->shouldPublishFeedStory($object, $xactions)) { @@ -2862,15 +2864,6 @@ return false; } - /** - * @task search - */ - protected function getSearchContextParameter( - PhabricatorLiskDAO $object, - array $xactions) { - return null; - } - /* -( Herald Integration )-------------------------------------------------- */