PLT-7207: Change from fulltext to LIKE search for user filtering (#7343)

* PLT-7202: Switch user search to LIKE queries to avoid fulltext pitfalls.

* Add 2 char name unit test.

* Escape underscores properly.

* Add more tests and fix * handling.

* Make search/indexes case insensitive for postgres.
This commit is contained in:
George Goldberg
2017-09-27 18:44:22 +01:00
committed by GitHub
parent 8c80cdde38
commit 8d662105d3
3 changed files with 119 additions and 62 deletions

View File

@@ -1583,7 +1583,7 @@ func (s SqlChannelStore) performSearch(searchQuery string, term string, paramete
result := store.StoreResult{}
// these chars have special meaning and can be treated as spaces
for _, c := range specialUserSearchChar {
for _, c := range ignoreUserSearchChar {
term = strings.Replace(term, c, " ", -1)
}

View File

@@ -78,6 +78,14 @@ func (us SqlUserStore) CreateIndexesIfNotExists() {
us.CreateIndexIfNotExists("idx_users_create_at", "Users", "CreateAt")
us.CreateIndexIfNotExists("idx_users_delete_at", "Users", "DeleteAt")
if *utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
us.CreateIndexIfNotExists("idx_users_email_lower", "Users", "lower(Email)")
us.CreateIndexIfNotExists("idx_users_username_lower", "Users", "lower(Username)")
us.CreateIndexIfNotExists("idx_users_nickname_lower", "Users", "lower(Nickname)")
us.CreateIndexIfNotExists("idx_users_firstname_lower", "Users", "lower(FirstName)")
us.CreateIndexIfNotExists("idx_users_lastname_lower", "Users", "lower(LastName)")
}
us.CreateFullTextIndexIfNotExists("idx_users_all_txt", "Users", USER_SEARCH_TYPE_ALL)
us.CreateFullTextIndexIfNotExists("idx_users_all_no_full_name_txt", "Users", USER_SEARCH_TYPE_ALL_NO_FULL_NAME)
us.CreateFullTextIndexIfNotExists("idx_users_names_txt", "Users", USER_SEARCH_TYPE_NAMES)
@@ -1398,46 +1406,26 @@ func (us SqlUserStore) SearchInChannel(channelId string, term string, options ma
return storeChannel
}
var specialUserSearchChar = []string{
"<",
">",
"+",
"-",
"(",
")",
"~",
":",
"*",
"\"",
"!",
"@",
var escapeUserSearchChar = []string{
"%",
"_",
}
var postgresSearchChar = []string{
"(",
")",
":",
"!",
var ignoreUserSearchChar = []string{
"*",
}
func (us SqlUserStore) performSearch(searchQuery string, term string, options map[string]bool, parameters map[string]interface{}) store.StoreResult {
result := store.StoreResult{}
// Special handling for emails
originalTerm := term
postgresUseOriginalTerm := false
if strings.Contains(term, "@") && strings.Contains(term, ".") {
if *utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
postgresUseOriginalTerm = true
} else if *utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_MYSQL {
lastIndex := strings.LastIndex(term, ".")
term = term[0:lastIndex]
}
// These chars must be removed from the like query.
for _, c := range ignoreUserSearchChar {
term = strings.Replace(term, c, "", -1)
}
// these chars have special meaning and can be treated as spaces
for _, c := range specialUserSearchChar {
term = strings.Replace(term, c, " ", -1)
// These chars must be escaped in the like query.
for _, c := range escapeUserSearchChar {
term = strings.Replace(term, c, "*"+c, -1)
}
searchType := USER_SEARCH_TYPE_ALL
@@ -1457,45 +1445,30 @@ func (us SqlUserStore) performSearch(searchQuery string, term string, options ma
if term == "" {
searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "", 1)
} else if *utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
if postgresUseOriginalTerm {
term = originalTerm
// these chars will break the query and must be removed
for _, c := range postgresSearchChar {
term = strings.Replace(term, c, "", -1)
}
} else {
splitTerm := strings.Fields(term)
for i, t := range strings.Fields(term) {
if i == len(splitTerm)-1 {
splitTerm[i] = t + ":*"
} else {
splitTerms := strings.Fields(term)
splitFields := strings.Split(searchType, ", ")
terms := []string{}
for i, term := range splitTerms {
fields := []string{}
for _, field := range splitFields {
if *utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
fields = append(fields, fmt.Sprintf("lower(%s) LIKE lower(%s) escape '*' ", field, fmt.Sprintf(":Term%d", i)))
} else {
splitTerm[i] = t + ":* &"
fields = append(fields, fmt.Sprintf("%s LIKE %s escape '*' ", field, fmt.Sprintf(":Term%d", i)))
}
}
term = strings.Join(splitTerm, " ")
terms = append(terms, fmt.Sprintf("(%s)", strings.Join(fields, " OR ")))
parameters[fmt.Sprintf("Term%d", i)] = fmt.Sprintf("%s%%", term)
}
searchType = convertMySQLFullTextColumnsToPostgres(searchType)
searchClause := fmt.Sprintf("AND (%s) @@ to_tsquery('simple', :Term)", searchType)
searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", searchClause, 1)
} else if *utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_MYSQL {
splitTerm := strings.Fields(term)
for i, t := range strings.Fields(term) {
splitTerm[i] = "+" + t + "*"
}
term = strings.Join(splitTerm, " ")
searchClause := fmt.Sprintf("AND MATCH(%s) AGAINST (:Term IN BOOLEAN MODE)", searchType)
searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", searchClause, 1)
term := strings.Join(terms, " AND ")
searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", fmt.Sprintf(" AND %s ", term), 1)
}
var users []*model.User
parameters["Term"] = term
if _, err := us.GetReplica().Select(&users, searchQuery, parameters); err != nil {
result.Err = model.NewAppError("SqlUserStore.Search", "store.sql_user.search.app_error", nil, "term="+term+", "+"search_type="+searchType+", "+err.Error(), http.StatusInternalServerError)
} else {

View File

@@ -1334,10 +1334,28 @@ func TestUserStoreSearch(t *testing.T) {
u3.DeleteAt = 1
store.Must(ss.User().Save(u3))
u5 := &model.User{}
u5.Username = "yu" + model.NewId()
u5.FirstName = "En"
u5.LastName = "Yu"
u5.Nickname = "enyu"
u5.Email = model.NewId() + "@simulator.amazonses.com"
store.Must(ss.User().Save(u5))
u6 := &model.User{}
u6.Username = "underscore" + model.NewId()
u6.FirstName = "Du_"
u6.LastName = "_DE"
u6.Nickname = "lodash"
u6.Email = model.NewId() + "@simulator.amazonses.com"
store.Must(ss.User().Save(u6))
tid := model.NewId()
store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u1.Id}))
store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u2.Id}))
store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u3.Id}))
store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u5.Id}))
store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u6.Id}))
searchOptions := map[string]bool{}
searchOptions[store.USER_SEARCH_OPTION_NAMES_ONLY] = true
@@ -1367,6 +1385,22 @@ func TestUserStoreSearch(t *testing.T) {
}
}
if r1 := <-ss.User().Search(tid, "en", searchOptions); r1.Err != nil {
t.Fatal(r1.Err)
} else {
profiles := r1.Data.([]*model.User)
found1 := false
for _, profile := range profiles {
if profile.Id == u5.Id {
found1 = true
}
}
if !found1 {
t.Fatal("should have found user")
}
}
searchOptions[store.USER_SEARCH_OPTION_NAMES_ONLY] = false
if r1 := <-ss.User().Search(tid, u1.Email, searchOptions); r1.Err != nil {
@@ -1429,6 +1463,56 @@ func TestUserStoreSearch(t *testing.T) {
}
}
// % should be escaped and searched for.
if r1 := <-ss.User().Search(tid, "h%", searchOptions); r1.Err != nil {
t.Fatal(r1.Err)
} else {
profiles := r1.Data.([]*model.User)
if len(profiles) != 0 {
t.Fatal("shouldn't have found anything")
}
}
// "_" should be properly escaped and searched for.
if r1 := <-ss.User().Search(tid, "h_", searchOptions); r1.Err != nil {
t.Fatal(r1.Err)
} else {
profiles := r1.Data.([]*model.User)
if len(profiles) != 0 {
t.Fatal("shouldn't have found anything")
}
}
if r1 := <-ss.User().Search(tid, "Du_", searchOptions); r1.Err != nil {
t.Fatal(r1.Err)
} else {
profiles := r1.Data.([]*model.User)
found6 := false
for _, profile := range profiles {
if profile.Id == u6.Id {
found6 = true
}
}
if !found6 {
t.Fatal("should have found user")
}
}
if r1 := <-ss.User().Search(tid, "_dE", searchOptions); r1.Err != nil {
t.Fatal(r1.Err)
} else {
profiles := r1.Data.([]*model.User)
found6 := false
for _, profile := range profiles {
if profile.Id == u6.Id {
found6 = true
}
}
if !found6 {
t.Fatal("should have found user")
}
}
searchOptions[store.USER_SEARCH_OPTION_ALLOW_INACTIVE] = true
if r1 := <-ss.User().Search(tid, "jimb", searchOptions); r1.Err != nil {