From 67228acfbbfe8eaf6f4413eed3d9c4136c6498fc Mon Sep 17 00:00:00 2001 From: Paolo Capriotti Date: Thu, 16 Sep 2021 11:00:23 +0200 Subject: [PATCH 1/3] Work around imprecisions in ES result ordering This sorts the result list after the fact by putting exact handle and name searches at the beginning, so we don't have to rely on ES being precise with the ordering of exact matches. Hopefully this fixes the flakyness of the `order-name` integration test, and makes #1774 unnecessary. --- changelog.d/3-bug-fixes/search-order-fix | 1 + libs/brig-types/src/Brig/Types/Search.hs | 2 + services/brig/src/Brig/User/API/Search.hs | 50 +++++++------------ .../brig/src/Brig/User/Search/SearchIndex.hs | 7 ++- 4 files changed, 23 insertions(+), 37 deletions(-) create mode 100644 changelog.d/3-bug-fixes/search-order-fix diff --git a/changelog.d/3-bug-fixes/search-order-fix b/changelog.d/3-bug-fixes/search-order-fix new file mode 100644 index 0000000000..9525ebc091 --- /dev/null +++ b/changelog.d/3-bug-fixes/search-order-fix @@ -0,0 +1 @@ +Make searches more consistenty return exact name matches first diff --git a/libs/brig-types/src/Brig/Types/Search.hs b/libs/brig-types/src/Brig/Types/Search.hs index e660ae504f..4c4a8144b6 100644 --- a/libs/brig-types/src/Brig/Types/Search.hs +++ b/libs/brig-types/src/Brig/Types/Search.hs @@ -28,6 +28,7 @@ module Brig.Types.Search where import Data.Id (TeamId) +import Imports import Wire.API.User.Search data TeamSearchInfo @@ -38,3 +39,4 @@ data TeamSearchInfo TeamOnly TeamId | -- | When searching user is part of a team and 'Brig.Options.setSearchSameTeamOnly' is False TeamAndNonMembers TeamId + deriving (Show) diff --git a/services/brig/src/Brig/User/API/Search.hs b/services/brig/src/Brig/User/API/Search.hs index ecaccf457b..f7936fd153 100644 --- a/services/brig/src/Brig/User/API/Search.hs +++ b/services/brig/src/Brig/User/API/Search.hs @@ -32,14 +32,12 @@ import qualified Brig.Options as Opts import Brig.Team.Util (ensurePermissions) import Brig.Types.Search as Search import Brig.User.API.Handle (contactFromProfile) -import qualified Brig.User.API.Handle as HandleAPI import Brig.User.Search.Index import qualified Brig.User.Search.SearchIndex as Q import Brig.User.Search.TeamUserSearch (RoleFilter (..), TeamUserSearchSortBy (..), TeamUserSearchSortOrder (..)) import qualified Brig.User.Search.TeamUserSearch as Q import Control.Lens (view) import Data.Domain (Domain) -import Data.Handle (parseHandle) import Data.Id import Data.Predicate import Data.Range @@ -150,23 +148,10 @@ searchLocally searcherId searchTerm maybeMaxResults = do let maxResults = maybe 15 (fromIntegral . fromRange) maybeMaxResults teamSearchInfo <- mkTeamSearchInfo - maybeExactHandleMatch <- exactHandleSearch teamSearchInfo + esResult <- Q.searchIndex searcherId teamSearchInfo searchTerm maxResults - let exactHandleMatchCount = length maybeExactHandleMatch - esMaxResults = maxResults - exactHandleMatchCount - - esResult <- - if esMaxResults > 0 - then Q.searchIndex searcherId teamSearchInfo searchTerm esMaxResults - else pure $ SearchResult 0 0 0 [] - - -- Prepend results matching exact handle and results from ES. - pure $ - esResult - { searchResults = maybeToList maybeExactHandleMatch <> searchResults esResult, - searchFound = exactHandleMatchCount + searchFound esResult, - searchReturned = exactHandleMatchCount + searchReturned esResult - } + -- Move exact matches to the beginning of the list + pure $ esResult {searchResults = sortResults teamSearchInfo (searchResults esResult)} where handleTeamVisibility :: TeamId -> TeamSearchVisibility -> Search.TeamSearchInfo handleTeamVisibility t Team.SearchVisibilityStandard = Search.TeamAndNonMembers t @@ -186,21 +171,20 @@ searchLocally searcherId searchTerm maybeMaxResults = do -- For team users, we need to check the visibility flag handleTeamVisibility t <$> Intra.getTeamSearchVisibility t - exactHandleSearch :: TeamSearchInfo -> Handler (Maybe Contact) - exactHandleSearch teamSearchInfo = do - let searchedHandleMaybe = parseHandle searchTerm - exactHandleResult <- - case searchedHandleMaybe of - Nothing -> pure Nothing - Just searchedHandle -> - contactFromProfile - <$$> HandleAPI.getLocalHandleInfo searcherId searchedHandle - pure $ case teamSearchInfo of - Search.TeamOnly t -> - if Just t == (contactTeam =<< exactHandleResult) - then exactHandleResult - else Nothing - _ -> exactHandleResult + sortResults :: TeamSearchInfo -> [Public.Contact] -> [Public.Contact] + sortResults tinfo = uncurry (<>) . partition (isExactMatch tinfo) + + -- A match is considered "exact" if the name or handle are the same as the + -- search term. + -- However, for team searches, exact name matches for non-team members are + -- not considered exact + isExactMatch tinfo c = + isExactHandleMatch c || (isExactNameMatch c && includeExact tinfo c) + includeExact (Search.TeamOnly t) c = contactTeam c == Just t + includeExact (Search.TeamAndNonMembers t) c = contactTeam c == Just t + includeExact _ _ = True + isExactHandleMatch c = contactHandle c == Just searchTerm + isExactNameMatch c = contactName c == searchTerm teamUserSearchH :: ( JSON diff --git a/services/brig/src/Brig/User/Search/SearchIndex.hs b/services/brig/src/Brig/User/Search/SearchIndex.hs index 5ec9005a51..b3489dcd82 100644 --- a/services/brig/src/Brig/User/Search/SearchIndex.hs +++ b/services/brig/src/Brig/User/Search/SearchIndex.hs @@ -97,7 +97,8 @@ defaultUserQuery u teamSearchInfo (normalized -> term') = ( ES.mkMultiMatchQuery [ ES.FieldName "handle.prefix^2", ES.FieldName "normalized.prefix", - ES.FieldName "normalized^3" + ES.FieldName "normalized^3", + ES.FieldName "handle^4" ] (ES.QueryString term') ) @@ -122,9 +123,7 @@ defaultUserQuery u teamSearchInfo (normalized -> term') = { ES.boolQueryMustMatch = [ ES.QueryBoolQuery boolQuery - { ES.boolQueryShouldMatch = [matchPhraseOrPrefix, legacyPrefixMatch], - -- This removes exact handle matches, as they are fetched from cassandra - ES.boolQueryMustNotMatch = [termQ "handle" term'] + { ES.boolQueryShouldMatch = [matchPhraseOrPrefix, legacyPrefixMatch] } ], ES.boolQueryShouldMatch = [ES.QueryExistsQuery (ES.FieldName "handle")] From 4daa80d4befcda9f71c75d74f158776c7ce892dd Mon Sep 17 00:00:00 2001 From: Paolo Capriotti Date: Thu, 16 Sep 2021 11:47:52 +0200 Subject: [PATCH 2/3] fixup! Work around imprecisions in ES result ordering --- services/brig/src/Brig/User/Search/SearchIndex.hs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/brig/src/Brig/User/Search/SearchIndex.hs b/services/brig/src/Brig/User/Search/SearchIndex.hs index b3489dcd82..76af31f7ce 100644 --- a/services/brig/src/Brig/User/Search/SearchIndex.hs +++ b/services/brig/src/Brig/User/Search/SearchIndex.hs @@ -97,8 +97,7 @@ defaultUserQuery u teamSearchInfo (normalized -> term') = ( ES.mkMultiMatchQuery [ ES.FieldName "handle.prefix^2", ES.FieldName "normalized.prefix", - ES.FieldName "normalized^3", - ES.FieldName "handle^4" + ES.FieldName "normalized^3" ] (ES.QueryString term') ) From 4d7ec3aa0df21333704941304c8d64c7ed04c670 Mon Sep 17 00:00:00 2001 From: jschaul Date: Thu, 16 Sep 2021 20:08:33 +0200 Subject: [PATCH 3/3] remove redundant import --- services/brig/src/Brig/User/API/Search.hs | 1 - 1 file changed, 1 deletion(-) diff --git a/services/brig/src/Brig/User/API/Search.hs b/services/brig/src/Brig/User/API/Search.hs index f7936fd153..67aa61e669 100644 --- a/services/brig/src/Brig/User/API/Search.hs +++ b/services/brig/src/Brig/User/API/Search.hs @@ -31,7 +31,6 @@ import qualified Brig.IO.Intra as Intra import qualified Brig.Options as Opts import Brig.Team.Util (ensurePermissions) import Brig.Types.Search as Search -import Brig.User.API.Handle (contactFromProfile) import Brig.User.Search.Index import qualified Brig.User.Search.SearchIndex as Q import Brig.User.Search.TeamUserSearch (RoleFilter (..), TeamUserSearchSortBy (..), TeamUserSearchSortOrder (..))