From df943fbf91ae8c1d5912d13629e3af3cd994b872 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Tue, 30 Jun 2020 15:38:58 +0200 Subject: [PATCH] [MM-26397] Take query size and order into account for Bleve (#14882) * [MM-26397] Take query size and order into account for Bleve * Add a test to check post search pagination * Add tests for checking limit when searching users * Make pagination an independent test to discriminate DB engines --- services/searchengine/bleveengine/search.go | 14 ++++++--- store/searchtest/post_layer.go | 32 +++++++++++++++++++++ store/searchtest/user_layer.go | 26 +++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/services/searchengine/bleveengine/search.go b/services/searchengine/bleveengine/search.go index bafacc4f9c..4512b628fc 100644 --- a/services/searchengine/bleveengine/search.go +++ b/services/searchengine/bleveengine/search.go @@ -196,7 +196,8 @@ func (b *BleveEngine) SearchPosts(channels *model.ChannelList, searchParams []*m query.AddMustNot(notFilters...) } - search := bleve.NewSearchRequest(query) + search := bleve.NewSearchRequestOptions(query, perPage, page*perPage, false) + search.SortBy([]string{"-CreateAt"}) results, err := b.PostIndex.Search(search) if err != nil { return nil, nil, model.NewAppError("Bleveengine.SearchPosts", "bleveengine.search_posts.error", nil, err.Error(), http.StatusInternalServerError) @@ -311,6 +312,7 @@ func (b *BleveEngine) SearchChannels(teamId, term string) ([]string, *model.AppE } query := bleve.NewSearchRequest(bleve.NewConjunctionQuery(queries...)) + query.Size = model.CHANNEL_SEARCH_DEFAULT_LIMIT results, err := b.ChannelIndex.Search(query) if err != nil { return nil, model.NewAppError("Bleveengine.SearchChannels", "bleveengine.search_channels.error", nil, err.Error(), http.StatusInternalServerError) @@ -368,7 +370,9 @@ func (b *BleveEngine) SearchUsersInChannel(teamId, channelId string, restrictedT query := bleve.NewConjunctionQuery(queries...) - uchan, err := b.UserIndex.Search(bleve.NewSearchRequest(query)) + uchanSearch := bleve.NewSearchRequest(query) + uchanSearch.Size = options.Limit + uchan, err := b.UserIndex.Search(uchanSearch) if err != nil { return nil, nil, model.NewAppError("Bleveengine.SearchUsersInChannel", "bleveengine.search_users_in_channel.uchan.error", nil, err.Error(), http.StatusInternalServerError) } @@ -403,7 +407,9 @@ func (b *BleveEngine) SearchUsersInChannel(teamId, channelId string, restrictedT boolQ.AddMust(restrictedChannelsQ) } - nuchan, err := b.UserIndex.Search(bleve.NewSearchRequest(boolQ)) + nuchanSearch := bleve.NewSearchRequest(boolQ) + nuchanSearch.Size = options.Limit + nuchan, err := b.UserIndex.Search(nuchanSearch) if err != nil { return nil, nil, model.NewAppError("Bleveengine.SearchUsersInChannel", "bleveengine.search_users_in_channel.nuchan.error", nil, err.Error(), http.StatusInternalServerError) } @@ -465,7 +471,7 @@ func (b *BleveEngine) SearchUsersInTeam(teamId string, restrictedToChannels []st } search := bleve.NewSearchRequest(rootQ) - + search.Size = options.Limit results, err := b.UserIndex.Search(search) if err != nil { return nil, model.NewAppError("Bleveengine.SearchUsersInTeam", "bleveengine.search_users_in_team.error", nil, err.Error(), http.StatusInternalServerError) diff --git a/store/searchtest/post_layer.go b/store/searchtest/post_layer.go index 9124f1e5b1..20de0a87c5 100644 --- a/store/searchtest/post_layer.go +++ b/store/searchtest/post_layer.go @@ -18,6 +18,11 @@ var searchPostStoreTests = []searchTest{ Fn: testSearchPostsIncludingDMs, Tags: []string{ENGINE_ALL}, }, + { + Name: "Should be able to search posts using pagination", + Fn: testSearchPostsWithPagination, + Tags: []string{ENGINE_ELASTICSEARCH, ENGINE_BLEVE}, + }, { Name: "Should return pinned and unpinned posts", Fn: testSearchReturnPinnedAndUnpinned, @@ -293,6 +298,33 @@ func testSearchPostsIncludingDMs(t *testing.T, th *SearchTestHelper) { th.checkPostInSearchResults(t, p2.Id, results.Posts) } +func testSearchPostsWithPagination(t *testing.T, th *SearchTestHelper) { + direct, err := th.createDirectChannel(th.Team.Id, "direct", "direct", []*model.User{th.User, th.User2}) + require.Nil(t, err) + defer th.deleteChannel(direct) + + p1, err := th.createPost(th.User.Id, direct.Id, "dm test", "", model.POST_DEFAULT, 10000, false) + require.Nil(t, err) + _, err = th.createPost(th.User.Id, direct.Id, "dm other", "", model.POST_DEFAULT, 20000, false) + require.Nil(t, err) + p2, err := th.createPost(th.User.Id, th.ChannelBasic.Id, "channel test", "", model.POST_DEFAULT, 0, false) + require.Nil(t, err) + defer th.deleteUserPosts(th.User.Id) + + params := &model.SearchParams{Terms: "test"} + results, err := th.Store.Post().SearchPostsInTeamForUser([]*model.SearchParams{params}, th.User.Id, th.Team.Id, false, false, 0, 1) + require.Nil(t, err) + + require.Len(t, results.Posts, 1) + th.checkPostInSearchResults(t, p2.Id, results.Posts) + + results, err = th.Store.Post().SearchPostsInTeamForUser([]*model.SearchParams{params}, th.User.Id, th.Team.Id, false, false, 1, 1) + require.Nil(t, err) + + require.Len(t, results.Posts, 1) + th.checkPostInSearchResults(t, p1.Id, results.Posts) +} + func testSearchReturnPinnedAndUnpinned(t *testing.T, th *SearchTestHelper) { p1, err := th.createPost(th.User.Id, th.ChannelBasic.Id, "channel test unpinned", "", model.POST_DEFAULT, 0, false) require.Nil(t, err) diff --git a/store/searchtest/user_layer.go b/store/searchtest/user_layer.go index 93bb74e4dd..fdc1737d67 100644 --- a/store/searchtest/user_layer.go +++ b/store/searchtest/user_layer.go @@ -164,6 +164,22 @@ func TestSearchUserStore(t *testing.T, s store.Store, testEngine *SearchTestEngi } func testGetAllUsersInChannelWithEmptyTerm(t *testing.T, th *SearchTestHelper) { + options := &model.UserSearchOptions{ + AllowFullNames: true, + Limit: model.USER_SEARCH_DEFAULT_LIMIT, + } + users, err := th.Store.User().AutocompleteUsersInChannel(th.Team.Id, th.ChannelBasic.Id, "", options) + require.Nil(t, err) + th.assertUsersMatchInAnyOrder(t, []*model.User{th.User}, users.InChannel) + th.assertUsersMatchInAnyOrder(t, []*model.User{th.User2}, users.OutOfChannel) + + t.Run("Should be able to correctly honor limit when autocompleting", func(t *testing.T) { + result, err := th.Store.User().AutocompleteUsersInChannel(th.Team.Id, th.ChannelBasic.Id, "", options) + require.Nil(t, err) + require.Len(t, result.InChannel, 1) + require.Len(t, result.OutOfChannel, 1) + }) + t.Run("Return all users in team", func(t *testing.T) { options := createDefaultOptions(true, false, false) users, err := th.Store.User().AutocompleteUsersInChannel(th.Team.Id, th.ChannelBasic.Id, "", options) @@ -171,6 +187,7 @@ func testGetAllUsersInChannelWithEmptyTerm(t *testing.T, th *SearchTestHelper) { th.assertUsersMatchInAnyOrder(t, []*model.User{th.User}, users.InChannel) th.assertUsersMatchInAnyOrder(t, []*model.User{th.User2}, users.OutOfChannel) }) + t.Run("Return all users in teams even though some of them don't have a team associated", func(t *testing.T) { options := createDefaultOptions(true, false, false) userAlternate, err := th.createUser("user-alternate", "user-alternate", "user", "alternate") @@ -773,6 +790,15 @@ func testSearchUsersInTeam(t *testing.T, th *SearchTestHelper) { require.Nil(t, apperr) th.assertUsersMatchInAnyOrder(t, []*model.User{}, users) }) + t.Run("Should honor the limit when searching users in team", func(t *testing.T) { + optionsWithLimit := &model.UserSearchOptions{ + Limit: 1, + } + + users, apperr := th.Store.User().Search(th.Team.Id, "", optionsWithLimit) + require.Nil(t, apperr) + require.Len(t, users, 1) + }) } func testSearchUsersInTeamUsernameWithDot(t *testing.T, th *SearchTestHelper) {