Page MenuHomePhabricator

Show matching context from the document body in ferret search results
Needs ReviewPublic

Authored by mmodell on Feb 28 2020, 4:08 AM.

Details

Maniphest Tasks
T230787: Make search context highlights work with the ferret search engine
Reviewers
brennen
Patch without arc
git checkout -b D1179 && curl -L https://phabricator.wikimedia.org/D1179?download=true | git apply
Summary

This is working fairly well, surprisingly.

Note: I've cheated, to get the context I re-build the search document using the indexing code
instead of querying mysql to get it out of the {object-type}_ffields table.
I _think_ this is more efficient than the alternative, however, I may be entirely mistaken.

Bug: T230787
Maniphest-Task: T230787

Test Plan

run a global-search query and look for snippets of the body text, complete with bolded text for any matching terms that appear within the body.

Diff Detail

Repository
rPHAB Phabricator
Branch
search-context (branched from wmf/stable)
Lint
Lint ErrorsExcuse: unrelated
SeverityLocationCodeMessage
Error/srv/phab/libext/misc/scripts/init/init-script.php:3PHL3One Class Per File
Error/srv/phab/libext/misc/src/oauth/PhabricatorMediaWikiAuthProvider.php:273PHL1Unknown Symbol
Error/srv/phab/libext/misc/src/other/CustomLoginHandler.php:6PHL1Unknown Symbol
Errorsrc/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerAdapter.php:4PHL1Unknown Symbol
Unit
Unit Test Errors
Build Status
Buildable 3281
Build 5442: arc lint + arc unit

Unit TestsFailed

Excuse: unrelated
TimeTest
177 msPhabricatorLibraryTestCase::testEverythingImplemented
EXCEPTION (PhutilMissingSymbolException): Failed to load class/interface "PhabricatorMailImplementationPHPMailerAdapter". The symbol map for library "phabricator" (at "/srv/phab/phabricator/src") claims this symbol (of type "class/interface") is defined in "applications/metamta/adapter/PhabricatorMailImplementationPHPMailerAdapter.php", but loading that source file did not cause the symbol to become defined.
363 msPhabricatorLibraryTestCase::testMethodVisibility
EXCEPTION (PhutilMissingSymbolException): Failed to load class or interface "PhabricatorMailImplementationAdapter". The class or interface "PhabricatorMailImplementationAdapter" is not defined in the library map of any loaded library.
154 msPhabricatorCelerityTestCase::testCelerityMaps
1 assertion passed.
11 msPhabricatorConduitTestCase::testConduitMethods
1 assertion passed.
0 msPhabricatorInfrastructureTestCase::testApplicationsInstalled
1 assertion passed.
View Full Test Results (2 Failed · 5 Passed)

Event Timeline

mmodell requested review of this revision.Feb 28 2020, 4:08 AM
mmodell created this revision.

A couple of nits commented inline. Still working on getting my head around the larger mechanics. What I understand seems pretty reasonable.

src/applications/search/fulltextstorage/PhabricatorFerretFulltextStorageEngine.php
89

is_null($fulltext_engine)?

src/applications/search/view/PhabricatorSearchResultView.php
56

Any case this would ever hit something that consists entirely of 0? i.e., should this check something besides truthiness of the $body value itself?

mmodell updated this revision to Diff 3044.EditedMar 4 2020, 1:17 AM
  • Improved context display behavior
  • Improved some comments
  • Now searches the whole body for context instead of trimming to 1200 bytes.
mmodell edited the summary of this revision. (Show Details)Mar 4 2020, 1:19 AM
mmodell retitled this revision from WIP: show matching context from the document body in ferret search results to Show matching context from the document body in ferret search results.Apr 8 2020, 5:35 PM
mmodell added a project: Phabricator.