[PLT-7764] Fix in:channel search when channel name/displayname includes - (#7603)

* fix in:channel search when channel name/displayname includes `-`

* fix failing test in PostgreSQL

* add lowercase indexes of channel's name and displayname to postgresql, and add unit tests

store/sqlstore/channel_store.go

@@ -85,6 +85,11 @@ func (s SqlChannelStore) CreateIndexesIfNotExists() {
 	s.CreateIndexIfNotExists("idx_channels_create_at", "Channels", "CreateAt")
 	s.CreateIndexIfNotExists("idx_channels_delete_at", "Channels", "DeleteAt")
 
+	if s.DriverName() == model.DATABASE_DRIVER_POSTGRES {
+		s.CreateIndexIfNotExists("idx_channels_name_lower", "Channels", "lower(Name)")
+		s.CreateIndexIfNotExists("idx_channels_displayname_lower", "Channels", "lower(DisplayName)")
+	}
+
 	s.CreateIndexIfNotExists("idx_channelmembers_channel_id", "ChannelMembers", "ChannelId")
 	s.CreateIndexIfNotExists("idx_channelmembers_user_id", "ChannelMembers", "UserId")
 
@@ -1246,43 +1251,25 @@ 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{}
 
-	// these chars have special meaning and can be treated as spaces
+	// These chars must be removed from the like query.
 	for _, c := range ignoreUserSearchChar {
-		term = strings.Replace(term, c, " ", -1)
+		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)
 	}
 
 	if term == "" {
 		searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "", 1)
-	} else if s.DriverName() == model.DATABASE_DRIVER_POSTGRES {
+	} else {
-		splitTerm := strings.Fields(term)
+		isPostgreSQL := s.DriverName() == model.DATABASE_DRIVER_POSTGRES
-		for i, t := range strings.Fields(term) {
+		searchQuery = generateSearchQuery(searchQuery, term, "Name, DisplayName", parameters, isPostgreSQL)
-			if i == len(splitTerm)-1 {
-				splitTerm[i] = t + ":*"
-			} else {
-				splitTerm[i] = t + ":* &"
-			}
-		}
-
-		term = strings.Join(splitTerm, " ")
-
-		searchClause := fmt.Sprintf("AND (%s) @@  to_tsquery('simple', :Term)", "Name || ' ' || DisplayName")
-		searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", searchClause, 1)
-	} else if s.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)", "Name, DisplayName")
-		searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", searchClause, 1)
 	}
 
 	var channels model.ChannelList
 
-	parameters["Term"] = term
-
 	if _, err := s.GetReplica().Select(&channels, searchQuery, parameters); err != nil {
 		result.Err = model.NewAppError("SqlChannelStore.Search", "store.sql_channel.search.app_error", nil, "term="+term+", "+", "+err.Error(), http.StatusInternalServerError)
 	} else {

store/sqlstore/user_store.go

@@ -1028,6 +1028,28 @@ var ignoreUserSearchChar = []string{
 	"*",
 }
 
+func generateSearchQuery(searchQuery, term, searchField string, parameters map[string]interface{}, isPostgreSQL bool) string {
+	splitTerms := strings.Fields(term)
+	splitFields := strings.Split(searchField, ", ")
+
+	terms := []string{}
+	for i, term := range splitTerms {
+		fields := []string{}
+		for _, field := range splitFields {
+			if isPostgreSQL {
+				fields = append(fields, fmt.Sprintf("lower(%s) LIKE lower(%s) escape '*' ", field, fmt.Sprintf(":Term%d", i)))
+			} else {
+				fields = append(fields, fmt.Sprintf("%s LIKE %s escape '*' ", field, fmt.Sprintf(":Term%d", i)))
+			}
+		}
+		terms = append(terms, fmt.Sprintf("(%s)", strings.Join(fields, " OR ")))
+		parameters[fmt.Sprintf("Term%d", i)] = fmt.Sprintf("%s%%", term)
+	}
+
+	searchClause := strings.Join(terms, " AND ")
+	return strings.Replace(searchQuery, "SEARCH_CLAUSE", fmt.Sprintf(" AND %s ", searchClause), 1)
+}
+
 func (us SqlUserStore) performSearch(searchQuery string, term string, options map[string]bool, parameters map[string]interface{}) store.StoreResult {
 	result := store.StoreResult{}
 
@@ -1059,25 +1081,8 @@ func (us SqlUserStore) performSearch(searchQuery string, term string, options ma
 	if term == "" {
 		searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "", 1)
 	} else {
-		splitTerms := strings.Fields(term)
+		isPostgreSQL := us.DriverName() == model.DATABASE_DRIVER_POSTGRES
-		splitFields := strings.Split(searchType, ", ")
+		searchQuery = generateSearchQuery(searchQuery, term, searchType, parameters, isPostgreSQL)
-
-		terms := []string{}
-		for i, term := range splitTerms {
-			fields := []string{}
-			for _, field := range splitFields {
-				if us.DriverName() == model.DATABASE_DRIVER_POSTGRES {
-					fields = append(fields, fmt.Sprintf("lower(%s) LIKE lower(%s) escape '*' ", field, fmt.Sprintf(":Term%d", i)))
-				} else {
-					fields = append(fields, fmt.Sprintf("%s LIKE %s escape '*' ", field, fmt.Sprintf(":Term%d", i)))
-				}
-			}
-			terms = append(terms, fmt.Sprintf("(%s)", strings.Join(fields, " OR ")))
-			parameters[fmt.Sprintf("Term%d", i)] = fmt.Sprintf("%s%%", term)
-		}
-
-		term := strings.Join(terms, " AND ")
-		searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", fmt.Sprintf(" AND %s ", term), 1)
 	}
 
 	var users []*model.User

store/storetest/channel_store.go

@@ -1673,6 +1673,27 @@ func testChannelStoreSearchMore(t *testing.T, ss store.Store) {
 	o5.Type = model.CHANNEL_PRIVATE
 	store.Must(ss.Channel().Save(&o5))
 
+	o6 := model.Channel{}
+	o6.TeamId = o1.TeamId
+	o6.DisplayName = "Off-Topic"
+	o6.Name = "off-topic"
+	o6.Type = model.CHANNEL_OPEN
+	store.Must(ss.Channel().Save(&o6))
+
+	o7 := model.Channel{}
+	o7.TeamId = o1.TeamId
+	o7.DisplayName = "Off-Set"
+	o7.Name = "off-set"
+	o7.Type = model.CHANNEL_OPEN
+	store.Must(ss.Channel().Save(&o7))
+
+	o8 := model.Channel{}
+	o8.TeamId = o1.TeamId
+	o8.DisplayName = "Off-Limit"
+	o8.Name = "off-limit"
+	o8.Type = model.CHANNEL_PRIVATE
+	store.Must(ss.Channel().Save(&o8))
+
 	if result := <-ss.Channel().SearchMore(m1.UserId, o1.TeamId, "ChannelA"); result.Err != nil {
 		t.Fatal(result.Err)
 	} else {
@@ -1708,6 +1729,45 @@ func testChannelStoreSearchMore(t *testing.T, ss store.Store) {
 		}
 	}
 
+	if result := <-ss.Channel().SearchMore(m1.UserId, o1.TeamId, "off-"); result.Err != nil {
+		t.Fatal(result.Err)
+	} else {
+		channels := result.Data.(*model.ChannelList)
+		if len(*channels) != 2 {
+			t.Fatal("should return 2 channels, not including private channel")
+		}
+
+		if (*channels)[0].Name != o7.Name {
+			t.Fatal("wrong channel returned")
+		}
+
+		if (*channels)[1].Name != o6.Name {
+			t.Fatal("wrong channel returned")
+		}
+	}
+
+	if result := <-ss.Channel().SearchMore(m1.UserId, o1.TeamId, "off-topic"); result.Err != nil {
+		t.Fatal(result.Err)
+	} else {
+		channels := result.Data.(*model.ChannelList)
+		if len(*channels) != 1 {
+			t.Fatal("should return 1 channel")
+		}
+
+		if (*channels)[0].Name != o6.Name {
+			t.Fatal("wrong channel returned")
+		}
+	}
+
+	if result := <-ss.Channel().SearchMore(m1.UserId, o1.TeamId, "off-topics"); result.Err != nil {
+		t.Fatal(result.Err)
+	} else {
+		channels := result.Data.(*model.ChannelList)
+		if len(*channels) != 0 {
+			t.Logf("%v\n", *channels)
+			t.Fatal("should be empty")
+		}
+	}
 }
 
 func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
@@ -1764,6 +1824,27 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
 	o5.Type = model.CHANNEL_PRIVATE
 	store.Must(ss.Channel().Save(&o5))
 
+	o6 := model.Channel{}
+	o6.TeamId = o1.TeamId
+	o6.DisplayName = "Off-Topic"
+	o6.Name = "off-topic"
+	o6.Type = model.CHANNEL_OPEN
+	store.Must(ss.Channel().Save(&o6))
+
+	o7 := model.Channel{}
+	o7.TeamId = o1.TeamId
+	o7.DisplayName = "Off-Set"
+	o7.Name = "off-set"
+	o7.Type = model.CHANNEL_OPEN
+	store.Must(ss.Channel().Save(&o7))
+
+	o8 := model.Channel{}
+	o8.TeamId = o1.TeamId
+	o8.DisplayName = "Off-Limit"
+	o8.Name = "off-limit"
+	o8.Type = model.CHANNEL_PRIVATE
+	store.Must(ss.Channel().Save(&o8))
+
 	if result := <-ss.Channel().SearchInTeam(o1.TeamId, "ChannelA"); result.Err != nil {
 		t.Fatal(result.Err)
 	} else {
@@ -1790,6 +1871,45 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) {
 			t.Fatal("should be empty")
 		}
 	}
+
+	if result := <-ss.Channel().SearchInTeam(o1.TeamId, "off-"); result.Err != nil {
+		t.Fatal(result.Err)
+	} else {
+		channels := result.Data.(*model.ChannelList)
+		if len(*channels) != 2 {
+			t.Fatal("should return 2 channels, not including private channel")
+		}
+
+		if (*channels)[0].Name != o7.Name {
+			t.Fatal("wrong channel returned")
+		}
+
+		if (*channels)[1].Name != o6.Name {
+			t.Fatal("wrong channel returned")
+		}
+	}
+
+	if result := <-ss.Channel().SearchInTeam(o1.TeamId, "off-topic"); result.Err != nil {
+		t.Fatal(result.Err)
+	} else {
+		channels := result.Data.(*model.ChannelList)
+		if len(*channels) != 1 {
+			t.Fatal("should return 1 channel")
+		}
+
+		if (*channels)[0].Name != o6.Name {
+			t.Fatal("wrong channel returned")
+		}
+	}
+
+	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")
+		}
+	}
 }
 
 func testChannelStoreGetMembersByIds(t *testing.T, ss store.Store) {