ABC-79: Optimize channel autocomplete query (#8163)

* optimize channel autocomplete query * move to new autocomplete endpoint
2018-01-31 08:26:40 -06:00
--- a/api4/channel.go
+++ b/api4/channel.go
@@ -20,6 +20,7 @@ func (api *API) InitChannel() {
 	api.BaseRoutes.ChannelsForTeam.Handle("/deleted", api.ApiSessionRequired(getDeletedChannelsForTeam)).Methods("GET")
 	api.BaseRoutes.ChannelsForTeam.Handle("/ids", api.ApiSessionRequired(getPublicChannelsByIdsForTeam)).Methods("POST")
 	api.BaseRoutes.ChannelsForTeam.Handle("/search", api.ApiSessionRequired(searchChannelsForTeam)).Methods("POST")
+	api.BaseRoutes.ChannelsForTeam.Handle("/autocomplete", api.ApiSessionRequired(autocompleteChannelsForTeam)).Methods("GET")
 	api.BaseRoutes.User.Handle("/teams/{team_id:[A-Za-z0-9]+}/channels", api.ApiSessionRequired(getChannelsForTeamForUser)).Methods("GET")

 	api.BaseRoutes.Channel.Handle("", api.ApiSessionRequired(getChannel)).Methods("GET")
@@ -489,6 +490,27 @@ func getChannelsForTeamForUser(c *Context, w http.ResponseWriter, r *http.Reques
 	}
 }

+func autocompleteChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) {
+	c.RequireTeamId()
+	if c.Err != nil {
+		return
+	}
+
+	if !c.App.SessionHasPermissionToTeam(c.Session, c.Params.TeamId, model.PERMISSION_LIST_TEAM_CHANNELS) {
+		c.SetPermissionError(model.PERMISSION_LIST_TEAM_CHANNELS)
+		return
+	}
+
+	name := r.URL.Query().Get("name")
+
+	if channels, err := c.App.AutocompleteChannels(c.Params.TeamId, name); err != nil {
+		c.Err = err
+		return
+	} else {
+		w.Write([]byte(channels.ToJson()))
+	}
+}
+
 func searchChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) {
 	c.RequireTeamId()
 	if c.Err != nil {
--- a/app/channel.go
+++ b/app/channel.go
@@ -1255,6 +1255,14 @@ func (a *App) UpdateChannelLastViewedAt(channelIds []string, userId string) *mod
 	return nil
 }

+func (a *App) AutocompleteChannels(teamId string, term string) (*model.ChannelList, *model.AppError) {
+	if result := <-a.Srv.Store.Channel().AutocompleteInTeam(teamId, term); result.Err != nil {
+		return nil, result.Err
+	} else {
+		return result.Data.(*model.ChannelList), nil
+	}
+}
+
 func (a *App) SearchChannels(teamId string, term string) (*model.ChannelList, *model.AppError) {
 	if result := <-a.Srv.Store.Channel().SearchInTeam(teamId, term); result.Err != nil {
 		return nil, result.Err
--- a/store/sqlstore/channel_store.go
+++ b/store/sqlstore/channel_store.go
@@ -7,6 +7,7 @@ import (
 	"database/sql"
 	"fmt"
 	"net/http"
+	"sort"
 	"strconv"
 	"strings"

@@ -81,6 +82,7 @@ func NewSqlChannelStore(sqlStore SqlStore, metrics einterfaces.MetricsInterface)
 func (s SqlChannelStore) CreateIndexesIfNotExists() {
 	s.CreateIndexIfNotExists("idx_channels_team_id", "Channels", "TeamId")
 	s.CreateIndexIfNotExists("idx_channels_name", "Channels", "Name")
+	s.CreateIndexIfNotExists("idx_channels_displayname", "Channels", "DisplayName")
 	s.CreateIndexIfNotExists("idx_channels_update_at", "Channels", "UpdateAt")
 	s.CreateIndexIfNotExists("idx_channels_create_at", "Channels", "CreateAt")
 	s.CreateIndexIfNotExists("idx_channels_delete_at", "Channels", "DeleteAt")
@@ -1257,6 +1259,46 @@ func (s SqlChannelStore) GetMembersForUser(teamId string, userId string) store.S
 	})
 }

+func (s SqlChannelStore) AutocompleteInTeam(teamId string, term string) store.StoreChannel {
+	return store.Do(func(result *store.StoreResult) {
+		queryFormat := `
+			SELECT
+				*
+			FROM
+				Channels
+			WHERE
+				TeamId = :TeamId
+				AND Type = 'O'
+				AND DeleteAt = 0
+				%v
+			LIMIT 50`
+
+		var channels model.ChannelList
+
+		if likeClause, likeTerm := s.buildLIKEClause(term); likeClause == "" {
+			if _, err := s.GetReplica().Select(&channels, fmt.Sprintf(queryFormat, ""), map[string]interface{}{"TeamId": teamId}); err != nil {
+				result.Err = model.NewAppError("SqlChannelStore.AutocompleteInTeam", "store.sql_channel.search.app_error", nil, "term="+term+", "+", "+err.Error(), http.StatusInternalServerError)
+			}
+		} else {
+			// Using a UNION results in index_merge and fulltext queries and is much faster than the ref
+			// query you would get using an OR of the LIKE and full-text clauses.
+			fulltextClause, fulltextTerm := s.buildFulltextClause(term)
+			likeQuery := fmt.Sprintf(queryFormat, "AND "+likeClause)
+			fulltextQuery := fmt.Sprintf(queryFormat, "AND "+fulltextClause)
+			query := fmt.Sprintf("(%v) UNION (%v) LIMIT 50", likeQuery, fulltextQuery)
+
+			if _, err := s.GetReplica().Select(&channels, query, map[string]interface{}{"TeamId": teamId, "LikeTerm": likeTerm, "FulltextTerm": fulltextTerm}); err != nil {
+				result.Err = model.NewAppError("SqlChannelStore.AutocompleteInTeam", "store.sql_channel.search.app_error", nil, "term="+term+", "+", "+err.Error(), http.StatusInternalServerError)
+			}
+		}
+
+		sort.Slice(channels, func(a, b int) bool {
+			return channels[a].DisplayName < channels[b].DisplayName
+		})
+		result.Data = &channels
+	})
+}
+
 func (s SqlChannelStore) SearchInTeam(teamId string, term string) store.StoreChannel {
 	return store.Do(func(result *store.StoreResult) {
 		searchQuery := `
@@ -1305,13 +1347,8 @@ func (s SqlChannelStore) SearchMore(userId string, teamId string, term string) s
 	})
 }

-func (s SqlChannelStore) performSearch(searchQuery string, term string, parameters map[string]interface{}) store.StoreResult {
-	result := store.StoreResult{}
-
-	// Copy the terms as we will need to prepare them differently for each search type.
-	likeTerm := term
-	fulltextTerm := term
-
+func (s SqlChannelStore) buildLIKEClause(term string) (likeClause, likeTerm string) {
+	likeTerm = term
 	searchColumns := "Name, DisplayName"

 	// These chars must be removed from the like query.
@@ -1324,15 +1361,10 @@ func (s SqlChannelStore) performSearch(searchQuery string, term string, paramete
 		likeTerm = strings.Replace(likeTerm, c, "*"+c, -1)
 	}

-	// These chars must be treated as spaces in the fulltext query.
-	for _, c := range spaceFulltextSearchChar {
-		fulltextTerm = strings.Replace(fulltextTerm, c, " ", -1)
+	if likeTerm == "" {
+		return
 	}

-	if likeTerm == "" {
-		// If the likeTerm is empty after preparing, then don't bother searching.
-		searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "", 1)
-	} else {
 	// Prepare the LIKE portion of the query.
 	var searchFields []string
 	for _, field := range strings.Split(searchColumns, ", ") {
@@ -1342,8 +1374,22 @@ func (s SqlChannelStore) performSearch(searchQuery string, term string, paramete
 			searchFields = append(searchFields, fmt.Sprintf("%s LIKE %s escape '*'", field, ":LikeTerm"))
 		}
 	}
-		likeSearchClause := fmt.Sprintf("(%s)", strings.Join(searchFields, " OR "))
-		parameters["LikeTerm"] = fmt.Sprintf("%s%%", likeTerm)
+
+	likeClause = fmt.Sprintf("(%s)", strings.Join(searchFields, " OR "))
+	likeTerm += "%"
+	return
+}
+
+func (s SqlChannelStore) buildFulltextClause(term string) (fulltextClause, fulltextTerm string) {
+	// Copy the terms as we will need to prepare them differently for each search type.
+	fulltextTerm = term
+
+	searchColumns := "Name, DisplayName"
+
+	// These chars must be treated as spaces in the fulltext query.
+	for _, c := range spaceFulltextSearchChar {
+		fulltextTerm = strings.Replace(fulltextTerm, c, " ", -1)
+	}

 	// Prepare the FULLTEXT portion of the query.
 	if s.DriverName() == model.DATABASE_DRIVER_POSTGRES {
@@ -1358,8 +1404,7 @@ func (s SqlChannelStore) performSearch(searchQuery string, term string, paramete

 		fulltextTerm = strings.Join(splitTerm, " ")

-			fulltextSearchClause := fmt.Sprintf("((%s) @@ to_tsquery(:FulltextTerm))", convertMySQLFullTextColumnsToPostgres(searchColumns))
-			searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "AND ("+likeSearchClause+" OR "+fulltextSearchClause+")", 1)
+		fulltextClause = fmt.Sprintf("((%s) @@ to_tsquery(:FulltextTerm))", convertMySQLFullTextColumnsToPostgres(searchColumns))
 	} else if s.DriverName() == model.DATABASE_DRIVER_MYSQL {
 		splitTerm := strings.Fields(fulltextTerm)
 		for i, t := range strings.Fields(fulltextTerm) {
@@ -1368,12 +1413,25 @@ func (s SqlChannelStore) performSearch(searchQuery string, term string, paramete

 		fulltextTerm = strings.Join(splitTerm, " ")

-			fulltextSearchClause := fmt.Sprintf("MATCH(%s) AGAINST (:FulltextTerm IN BOOLEAN MODE)", searchColumns)
-			searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", fmt.Sprintf("AND (%s OR %s)", likeSearchClause, fulltextSearchClause), 1)
-		}
+		fulltextClause = fmt.Sprintf("MATCH(%s) AGAINST (:FulltextTerm IN BOOLEAN MODE)", searchColumns)
 	}

+	return
+}
+
+func (s SqlChannelStore) performSearch(searchQuery string, term string, parameters map[string]interface{}) store.StoreResult {
+	result := store.StoreResult{}
+
+	likeClause, likeTerm := s.buildLIKEClause(term)
+	if likeTerm == "" {
+		// If the likeTerm is empty after preparing, then don't bother searching.
+		searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "", 1)
+	} else {
+		parameters["LikeTerm"] = likeTerm
+		fulltextClause, fulltextTerm := s.buildFulltextClause(term)
 		parameters["FulltextTerm"] = fulltextTerm
+		searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "AND ("+likeClause+" OR "+fulltextClause+")", 1)
+	}

 	var channels model.ChannelList

--- a/store/store.go
+++ b/store/store.go
@@ -154,6 +154,7 @@ type ChannelStore interface {
 	AnalyticsTypeCount(teamId string, channelType string) StoreChannel
 	ExtraUpdateByUser(userId string, time int64) StoreChannel
 	GetMembersForUser(teamId string, userId string) StoreChannel
+	AutocompleteInTeam(teamId string, term string) StoreChannel
 	SearchInTeam(teamId string, term string) StoreChannel
 	SearchMore(userId string, teamId string, term string) StoreChannel
 	GetMembersByIds(channelId string, userIds []string) StoreChannel
--- a/store/storetest/channel_store.go
+++ b/store/storetest/channel_store.go
@@ -1940,7 +1940,12 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
 	o11.Type = model.CHANNEL_OPEN
 	store.Must(ss.Channel().Save(&o11, -1))

-	if result := <-ss.Channel().SearchInTeam(o1.TeamId, "ChannelA"); result.Err != nil {
+	for name, search := range map[string]func(teamId string, term string) store.StoreChannel{
+		"AutocompleteInTeam": ss.Channel().AutocompleteInTeam,
+		"SearchInTeam":       ss.Channel().SearchInTeam,
+	} {
+		t.Run(name, func(t *testing.T) {
+			if result := <-search(o1.TeamId, "ChannelA"); result.Err != nil {
 				t.Fatal(result.Err)
 			} else {
 				channels := result.Data.(*model.ChannelList)
@@ -1949,7 +1954,7 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
 				}
 			}

-	if result := <-ss.Channel().SearchInTeam(o1.TeamId, ""); result.Err != nil {
+			if result := <-search(o1.TeamId, ""); result.Err != nil {
 				t.Fatal(result.Err)
 			} else {
 				channels := result.Data.(*model.ChannelList)
@@ -1958,7 +1963,7 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
 				}
 			}

-	if result := <-ss.Channel().SearchInTeam(o1.TeamId, "blargh"); result.Err != nil {
+			if result := <-search(o1.TeamId, "blargh"); result.Err != nil {
 				t.Fatal(result.Err)
 			} else {
 				channels := result.Data.(*model.ChannelList)
@@ -1967,7 +1972,7 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
 				}
 			}

-	if result := <-ss.Channel().SearchInTeam(o1.TeamId, "off-"); result.Err != nil {
+			if result := <-search(o1.TeamId, "off-"); result.Err != nil {
 				t.Fatal(result.Err)
 			} else {
 				channels := result.Data.(*model.ChannelList)
@@ -1984,7 +1989,7 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
 				}
 			}

-	if result := <-ss.Channel().SearchInTeam(o1.TeamId, "off-topic"); result.Err != nil {
+			if result := <-search(o1.TeamId, "off-topic"); result.Err != nil {
 				t.Fatal(result.Err)
 			} else {
 				channels := result.Data.(*model.ChannelList)
@@ -1997,21 +2002,7 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
 				}
 			}

-	/*
-		// Disabling this check as it will fail on PostgreSQL as we have "liberalised" channel matching to deal with
-		// Full-Text Stemming Limitations.
-		if result := <-ss.Channel().SearchMore(m1.
-		if result := <-ss.Channel().SearchInTeam(o1.TeamId, "off-topics"); result.Err != nil {
-			t.Fatal(result.Err)
-		} else {
-			channels := result.Data.(*model.ChannelList)
-			if len(*channels) != 0 {
-				t.Fatal("should be empty")
-			}
-		}
-	*/
-
-	if result := <-ss.Channel().SearchInTeam(o1.TeamId, "town square"); result.Err != nil {
+			if result := <-search(o1.TeamId, "town square"); result.Err != nil {
 				t.Fatal(result.Err)
 			} else {
 				channels := result.Data.(*model.ChannelList)
@@ -2024,7 +2015,7 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
 				}
 			}

-	if result := <-ss.Channel().SearchInTeam(o1.TeamId, "the"); result.Err != nil {
+			if result := <-search(o1.TeamId, "the"); result.Err != nil {
 				t.Fatal(result.Err)
 			} else {
 				channels := result.Data.(*model.ChannelList)
@@ -2038,7 +2029,7 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
 				}
 			}

-	if result := <-ss.Channel().SearchInTeam(o1.TeamId, "Mobile"); result.Err != nil {
+			if result := <-search(o1.TeamId, "Mobile"); result.Err != nil {
 				t.Fatal(result.Err)
 			} else {
 				channels := result.Data.(*model.ChannelList)
@@ -2051,6 +2042,8 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
 					t.Fatal("wrong channel returned")
 				}
 			}
+		})
+	}
 }

 func testChannelStoreGetMembersByIds(t *testing.T, ss store.Store) {
--- a/store/storetest/mocks/ChannelStore.go
+++ b/store/storetest/mocks/ChannelStore.go
@@ -45,6 +45,22 @@ func (_m *ChannelStore) AnalyticsTypeCount(teamId string, channelType string) st
 	return r0
 }

+// AutocompleteInTeam provides a mock function with given fields: teamId, term
+func (_m *ChannelStore) AutocompleteInTeam(teamId string, term string) store.StoreChannel {
+	ret := _m.Called(teamId, term)
+
+	var r0 store.StoreChannel
+	if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok {
+		r0 = rf(teamId, term)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(store.StoreChannel)
+		}
+	}
+
+	return r0
+}
+
 // CreateDirectChannel provides a mock function with given fields: userId, otherUserId
 func (_m *ChannelStore) CreateDirectChannel(userId string, otherUserId string) store.StoreChannel {
 	ret := _m.Called(userId, otherUserId)
--- a/store/storetest/mocks/UserAccessTokenStore.go
+++ b/store/storetest/mocks/UserAccessTokenStore.go
@@ -61,13 +61,13 @@ func (_m *UserAccessTokenStore) Get(tokenId string) store.StoreChannel {
 	return r0
 }

-// GetAll provides a mock function with given fields:
-func (_m *UserAccessTokenStore) GetAll(page int, perPage int) store.StoreChannel {
-	ret := _m.Called(page, perPage)
+// GetAll provides a mock function with given fields: offset, limit
+func (_m *UserAccessTokenStore) GetAll(offset int, limit int) store.StoreChannel {
+	ret := _m.Called(offset, limit)

 	var r0 store.StoreChannel
 	if rf, ok := ret.Get(0).(func(int, int) store.StoreChannel); ok {
-		r0 = rf(page, perPage)
+		r0 = rf(offset, limit)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(store.StoreChannel)
@@ -109,13 +109,13 @@ func (_m *UserAccessTokenStore) GetByUser(userId string, page int, perPage int)
 	return r0
 }

-// Search provides a mock function with given fields:
-func (_m *UserAccessTokenStore) Search(term string) store.StoreChannel {
-	ret := _m.Called(term)
+// Save provides a mock function with given fields: token
+func (_m *UserAccessTokenStore) Save(token *model.UserAccessToken) store.StoreChannel {
+	ret := _m.Called(token)

 	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
-		r0 = rf(term)
+	if rf, ok := ret.Get(0).(func(*model.UserAccessToken) store.StoreChannel); ok {
+		r0 = rf(token)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(store.StoreChannel)
@@ -125,13 +125,13 @@ func (_m *UserAccessTokenStore) Search(term string) store.StoreChannel {
 	return r0
 }

-// Save provides a mock function with given fields: token
-func (_m *UserAccessTokenStore) Save(token *model.UserAccessToken) store.StoreChannel {
-	ret := _m.Called(token)
+// Search provides a mock function with given fields: term
+func (_m *UserAccessTokenStore) Search(term string) store.StoreChannel {
+	ret := _m.Called(term)

 	var r0 store.StoreChannel
-	if rf, ok := ret.Get(0).(func(*model.UserAccessToken) store.StoreChannel); ok {
-		r0 = rf(token)
+	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+		r0 = rf(term)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(store.StoreChannel)