Migrate "User.GetProfilesByUsernames" to Sync by default (#11523)

* changed GetProfilesByUsernames signature

* modified the UserStore interface to reflect changed made to the GetProfilesByUsernames function signature

* modify usages of GetProfilesByUsernames

* fix gofmt

* fixed failing userstore tests

app/notification.go

@@ -141,8 +141,8 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 		}
 
 		if len(m.OtherPotentialMentions) > 0 && !post.IsSystemMessage() {
-			if profilesResult := <-a.Srv.Store.User().GetProfilesByUsernames(m.OtherPotentialMentions, &model.ViewUsersRestrictions{Teams: []string{team.Id}}); profilesResult.Err == nil {
+			if users, err := a.Srv.Store.User().GetProfilesByUsernames(m.OtherPotentialMentions, &model.ViewUsersRestrictions{Teams: []string{team.Id}}); err == nil {
-				channelMentions := model.UserSlice(profilesResult.Data.([]*model.User)).FilterByActive(true)
+				channelMentions := model.UserSlice(users).FilterByActive(true)
 
 				var outOfChannelMentions model.UserSlice
 				var outOfGroupsMentions model.UserSlice

app/user.go

@@ -625,11 +625,11 @@ func (a *App) GetUsersByGroupChannelIds(channelIds []string, asAdmin bool) (map[
 }
 
 func (a *App) GetUsersByUsernames(usernames []string, asAdmin bool, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) {
-	result := <-a.Srv.Store.User().GetProfilesByUsernames(usernames, viewRestrictions)
+	users, err := a.Srv.Store.User().GetProfilesByUsernames(usernames, viewRestrictions)
-	if result.Err != nil {
+	if err != nil {
-		return nil, result.Err
+		return nil, err
 	}
-	return a.sanitizeProfiles(result.Data.([]*model.User), asAdmin), nil
+	return a.sanitizeProfiles(users, asAdmin), nil
 }
 
 func (a *App) sanitizeProfiles(users []*model.User, asAdmin bool) []*model.User {

store/sqlstore/user_store.go

@@ -719,32 +719,28 @@ func (us SqlUserStore) GetProfilesWithoutTeam(offset int, limit int, viewRestric
 	})
 }
 
-func (us SqlUserStore) GetProfilesByUsernames(usernames []string, viewRestrictions *model.ViewUsersRestrictions) store.StoreChannel {
+func (us SqlUserStore) GetProfilesByUsernames(usernames []string, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) {
-	return store.Do(func(result *store.StoreResult) {
+	query := us.usersQuery
-		query := us.usersQuery
 
-		query = applyViewRestrictionsFilter(query, viewRestrictions, true)
+	query = applyViewRestrictionsFilter(query, viewRestrictions, true)
 
-		query = query.
+	query = query.
-			Where(map[string]interface{}{
+		Where(map[string]interface{}{
-				"Username": usernames,
+			"Username": usernames,
-			}).
+		}).
-			OrderBy("u.Username ASC")
+		OrderBy("u.Username ASC")
 
-		queryString, args, err := query.ToSql()
+	queryString, args, err := query.ToSql()
-		if err != nil {
+	if err != nil {
-			result.Err = model.NewAppError("SqlUserStore.GetProfilesByUsernames", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlUserStore.GetProfilesByUsernames", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError)
-			return
+	}
-		}
 
-		var users []*model.User
+	var users []*model.User
-		if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil {
+	if _, err := us.GetReplica().Select(&users, queryString, args...); err != nil {
-			result.Err = model.NewAppError("SqlUserStore.GetProfilesByUsernames", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, model.NewAppError("SqlUserStore.GetProfilesByUsernames", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError)
-			return
+	}
-		}
 
-		result.Data = users
+	return users, nil
-	})
 }
 
 type UserWithLastActivityAt struct {

store/store.go

@@ -268,7 +268,7 @@ type UserStore interface {
 	GetAllProfilesInChannel(channelId string, allowFromCache bool) (map[string]*model.User, *model.AppError)
 	GetProfilesNotInChannel(teamId string, channelId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError)
 	GetProfilesWithoutTeam(offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) StoreChannel
-	GetProfilesByUsernames(usernames []string, viewRestrictions *model.ViewUsersRestrictions) StoreChannel
+	GetProfilesByUsernames(usernames []string, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError)
 	GetAllProfiles(options *model.UserGetOptions) ([]*model.User, *model.AppError)
 	GetProfiles(options *model.UserGetOptions) ([]*model.User, *model.AppError)
 	GetProfileByIds(userIds []string, options *UserGetByIdsOpts, allowFromCache bool) ([]*model.User, *model.AppError)

store/storetest/mocks/UserStore.go

@@ -560,19 +560,28 @@ func (_m *UserStore) GetProfiles(options *model.UserGetOptions) ([]*model.User,
 }
 
 // GetProfilesByUsernames provides a mock function with given fields: usernames, viewRestrictions
-func (_m *UserStore) GetProfilesByUsernames(usernames []string, viewRestrictions *model.ViewUsersRestrictions) store.StoreChannel {
+func (_m *UserStore) GetProfilesByUsernames(usernames []string, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) {
 	ret := _m.Called(usernames, viewRestrictions)
 
-	var r0 store.StoreChannel
+	var r0 []*model.User
-	if rf, ok := ret.Get(0).(func([]string, *model.ViewUsersRestrictions) store.StoreChannel); ok {
+	if rf, ok := ret.Get(0).(func([]string, *model.ViewUsersRestrictions) []*model.User); ok {
 		r0 = rf(usernames, viewRestrictions)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(store.StoreChannel)
+			r0 = ret.Get(0).([]*model.User)
 		}
 	}
 
-	return r0
+	var r1 *model.AppError
+	if rf, ok := ret.Get(1).(func([]string, *model.ViewUsersRestrictions) *model.AppError); ok {
+		r1 = rf(usernames, viewRestrictions)
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(*model.AppError)
+		}
+	}
+
+	return r0, r1
 }
 
 // GetProfilesInChannel provides a mock function with given fields: channelId, offset, limit

store/storetest/user_store.go

@@ -1372,33 +1372,33 @@ func testUserStoreGetProfilesByUsernames(t *testing.T, ss store.Store) {
 	defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }()
 
 	t.Run("get by u1 and u2 usernames, team id 1", func(t *testing.T) {
-		result := <-ss.User().GetProfilesByUsernames([]string{u1.Username, u2.Username}, &model.ViewUsersRestrictions{Teams: []string{teamId}})
+		users, err := ss.User().GetProfilesByUsernames([]string{u1.Username, u2.Username}, &model.ViewUsersRestrictions{Teams: []string{teamId}})
-		require.Nil(t, result.Err)
+		require.Nil(t, err)
-		assert.Equal(t, []*model.User{u1, u2}, result.Data.([]*model.User))
+		assert.Equal(t, []*model.User{u1, u2}, users)
 	})
 
 	t.Run("get by u1 username, team id 1", func(t *testing.T) {
-		result := <-ss.User().GetProfilesByUsernames([]string{u1.Username}, &model.ViewUsersRestrictions{Teams: []string{teamId}})
+		users, err := ss.User().GetProfilesByUsernames([]string{u1.Username}, &model.ViewUsersRestrictions{Teams: []string{teamId}})
-		require.Nil(t, result.Err)
+		require.Nil(t, err)
-		assert.Equal(t, []*model.User{u1}, result.Data.([]*model.User))
+		assert.Equal(t, []*model.User{u1}, users)
 	})
 
 	t.Run("get by u1 and u3 usernames, no team id", func(t *testing.T) {
-		result := <-ss.User().GetProfilesByUsernames([]string{u1.Username, u3.Username}, nil)
+		users, err := ss.User().GetProfilesByUsernames([]string{u1.Username, u3.Username}, nil)
-		require.Nil(t, result.Err)
+		require.Nil(t, err)
-		assert.Equal(t, []*model.User{u1, u3}, result.Data.([]*model.User))
+		assert.Equal(t, []*model.User{u1, u3}, users)
 	})
 
 	t.Run("get by u1 and u3 usernames, team id 1", func(t *testing.T) {
-		result := <-ss.User().GetProfilesByUsernames([]string{u1.Username, u3.Username}, &model.ViewUsersRestrictions{Teams: []string{teamId}})
+		users, err := ss.User().GetProfilesByUsernames([]string{u1.Username, u3.Username}, &model.ViewUsersRestrictions{Teams: []string{teamId}})
-		require.Nil(t, result.Err)
+		require.Nil(t, err)
-		assert.Equal(t, []*model.User{u1}, result.Data.([]*model.User))
+		assert.Equal(t, []*model.User{u1}, users)
 	})
 
 	t.Run("get by u1 and u3 usernames, team id 2", func(t *testing.T) {
-		result := <-ss.User().GetProfilesByUsernames([]string{u1.Username, u3.Username}, &model.ViewUsersRestrictions{Teams: []string{team2Id}})
+		users, err := ss.User().GetProfilesByUsernames([]string{u1.Username, u3.Username}, &model.ViewUsersRestrictions{Teams: []string{team2Id}})
-		require.Nil(t, result.Err)
+		require.Nil(t, err)
-		assert.Equal(t, []*model.User{u3}, result.Data.([]*model.User))
+		assert.Equal(t, []*model.User{u3}, users)
 	})
 }