From db68e598a10d36013b7ff0994eca86e0464355e1 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Tue, 25 Apr 2017 11:00:41 -0400 Subject: PLT-4457 Added API to get multiple users by their usernames (#6218) * Allow getting profiles by username without a team * Changed UserStore.GetProfilesByUsernames to return an array * PLT-4457 Added API to get multiple users by their usernames * Changed users/names route to users/usernames --- api4/user.go | 19 +++++++++++++++ api4/user_test.go | 32 +++++++++++++++++++++++++ app/notification.go | 8 +++---- app/user.go | 56 +++++++++++++++++--------------------------- model/client4.go | 10 ++++++++ store/sql_user_store.go | 21 ++++++++--------- store/sql_user_store_test.go | 52 ++++++++++++++++++++++++++++++++++++---- 7 files changed, 144 insertions(+), 54 deletions(-) diff --git a/api4/user.go b/api4/user.go index 70182c1ab..1c870f1c1 100644 --- a/api4/user.go +++ b/api4/user.go @@ -22,6 +22,7 @@ func InitUser() { BaseRoutes.Users.Handle("", ApiHandler(createUser)).Methods("POST") BaseRoutes.Users.Handle("", ApiSessionRequired(getUsers)).Methods("GET") BaseRoutes.Users.Handle("/ids", ApiSessionRequired(getUsersByIds)).Methods("POST") + BaseRoutes.Users.Handle("/usernames", ApiSessionRequired(getUsersByNames)).Methods("POST") BaseRoutes.Users.Handle("/search", ApiSessionRequired(searchUsers)).Methods("POST") BaseRoutes.Users.Handle("/autocomplete", ApiSessionRequired(autocompleteUsers)).Methods("GET") @@ -367,6 +368,24 @@ func getUsersByIds(c *Context, w http.ResponseWriter, r *http.Request) { } } +func getUsersByNames(c *Context, w http.ResponseWriter, r *http.Request) { + usernames := model.ArrayFromJson(r.Body) + + if len(usernames) == 0 { + c.SetInvalidParam("usernames") + return + } + + // No permission check required + + if users, err := app.GetUsersByUsernames(usernames, c.IsSystemAdmin()); err != nil { + c.Err = err + return + } else { + w.Write([]byte(model.UserListToJson(users))) + } +} + func searchUsers(c *Context, w http.ResponseWriter, r *http.Request) { props := model.UserSearchFromJson(r.Body) if props == nil { diff --git a/api4/user_test.go b/api4/user_test.go index 9a360c7e4..20def9a17 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -666,6 +666,38 @@ func TestGetUsersByIds(t *testing.T) { CheckUnauthorizedStatus(t, resp) } +func TestGetUsersByUsernames(t *testing.T) { + th := Setup().InitBasic() + Client := th.Client + + users, resp := Client.GetUsersByUsernames([]string{th.BasicUser.Username}) + CheckNoError(t, resp) + + if users[0].Id != th.BasicUser.Id { + t.Fatal("returned wrong user") + } + CheckUserSanitization(t, users[0]) + + _, resp = Client.GetUsersByIds([]string{}) + CheckBadRequestStatus(t, resp) + + users, resp = Client.GetUsersByUsernames([]string{"junk"}) + CheckNoError(t, resp) + if len(users) > 0 { + t.Fatal("no users should be returned") + } + + users, resp = Client.GetUsersByUsernames([]string{"junk", th.BasicUser.Username}) + CheckNoError(t, resp) + if len(users) != 1 { + t.Fatal("1 user should be returned") + } + + Client.Logout() + _, resp = Client.GetUsersByUsernames([]string{th.BasicUser.Username}) + CheckUnauthorizedStatus(t, resp) +} + func TestUpdateUser(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() defer TearDown() diff --git a/app/notification.go b/app/notification.go index e983f5e8c..195f808c7 100644 --- a/app/notification.go +++ b/app/notification.go @@ -95,7 +95,7 @@ func SendNotifications(post *model.Post, team *model.Team, channel *model.Channe if len(potentialOtherMentions) > 0 { if result := <-Srv.Store.User().GetProfilesByUsernames(potentialOtherMentions, team.Id); result.Err == nil { - outOfChannelMentions := result.Data.(map[string]*model.User) + outOfChannelMentions := result.Data.([]*model.User) go sendOutOfChannelMentions(sender, post, team.Id, outOfChannelMentions) } } @@ -592,13 +592,13 @@ func getMobileAppSessions(userId string) ([]*model.Session, *model.AppError) { } } -func sendOutOfChannelMentions(sender *model.User, post *model.Post, teamId string, profiles map[string]*model.User) *model.AppError { - if len(profiles) == 0 { +func sendOutOfChannelMentions(sender *model.User, post *model.Post, teamId string, users []*model.User) *model.AppError { + if len(users) == 0 { return nil } var usernames []string - for _, user := range profiles { + for _, user := range users { usernames = append(usernames, user.Username) } sort.Strings(usernames) diff --git a/app/user.go b/app/user.go index 827ad18f9..86e7cf0b0 100644 --- a/app/user.go +++ b/app/user.go @@ -430,11 +430,7 @@ func GetUsersPage(page int, perPage int, asAdmin bool) ([]*model.User, *model.Ap return nil, err } - for _, user := range users { - SanitizeProfile(user, asAdmin) - } - - return users, nil + return sanitizeProfiles(users, asAdmin), nil } func GetUsersEtag() string { @@ -479,11 +475,7 @@ func GetUsersInTeamPage(teamId string, page int, perPage int, asAdmin bool) ([]* return nil, err } - for _, user := range users { - SanitizeProfile(user, asAdmin) - } - - return users, nil + return sanitizeProfiles(users, asAdmin), nil } func GetUsersNotInTeamPage(teamId string, page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { @@ -492,11 +484,7 @@ func GetUsersNotInTeamPage(teamId string, page int, perPage int, asAdmin bool) ( return nil, err } - for _, user := range users { - SanitizeProfile(user, asAdmin) - } - - return users, nil + return sanitizeProfiles(users, asAdmin), nil } func GetUsersInTeamEtag(teamId string) string { @@ -537,11 +525,7 @@ func GetUsersInChannelPage(channelId string, page int, perPage int, asAdmin bool return nil, err } - for _, user := range users { - SanitizeProfile(user, asAdmin) - } - - return users, nil + return sanitizeProfiles(users, asAdmin), nil } func GetUsersNotInChannel(teamId string, channelId string, offset int, limit int) ([]*model.User, *model.AppError) { @@ -574,11 +558,7 @@ func GetUsersNotInChannelPage(teamId string, channelId string, page int, perPage return nil, err } - for _, user := range users { - SanitizeProfile(user, asAdmin) - } - - return users, nil + return sanitizeProfiles(users, asAdmin), nil } func GetUsersWithoutTeamPage(page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { @@ -587,11 +567,7 @@ func GetUsersWithoutTeamPage(page int, perPage int, asAdmin bool) ([]*model.User return nil, err } - for _, user := range users { - SanitizeProfile(user, asAdmin) - } - - return users, nil + return sanitizeProfiles(users, asAdmin), nil } func GetUsersWithoutTeam(offset int, limit int) ([]*model.User, *model.AppError) { @@ -607,13 +583,25 @@ func GetUsersByIds(userIds []string, asAdmin bool) ([]*model.User, *model.AppErr return nil, result.Err } else { users := result.Data.([]*model.User) + return sanitizeProfiles(users, asAdmin), nil + } +} - for _, u := range users { - SanitizeProfile(u, asAdmin) - } +func GetUsersByUsernames(usernames []string, asAdmin bool) ([]*model.User, *model.AppError) { + if result := <-Srv.Store.User().GetProfilesByUsernames(usernames, ""); result.Err != nil { + return nil, result.Err + } else { + users := result.Data.([]*model.User) + return sanitizeProfiles(users, asAdmin), nil + } +} - return users, nil +func sanitizeProfiles(users []*model.User, asAdmin bool) []*model.User { + for _, u := range users { + SanitizeProfile(u, asAdmin) } + + return users } func GenerateMfaSecret(userId string) (*model.MfaSecret, *model.AppError) { diff --git a/model/client4.go b/model/client4.go index 1a2ce523b..2a7338809 100644 --- a/model/client4.go +++ b/model/client4.go @@ -607,6 +607,16 @@ func (c *Client4) GetUsersByIds(userIds []string) ([]*User, *Response) { } } +// GetUsersByUsernames returns a list of users based on the provided usernames. +func (c *Client4) GetUsersByUsernames(usernames []string) ([]*User, *Response) { + if r, err := c.DoApiPost(c.GetUsersRoute()+"/usernames", ArrayToJson(usernames)); err != nil { + return nil, &Response{StatusCode: r.StatusCode, Error: err} + } else { + defer closeBody(r) + return UserListFromJson(r.Body), BuildResponse(r) + } +} + // SearchUsers returns a list of users based on some search criteria. func (c *Client4) SearchUsers(search *UserSearch) ([]*User, *Response) { if r, err := c.DoApiPost(c.GetUsersRoute()+"/search", search.ToJson()); err != nil { diff --git a/store/sql_user_store.go b/store/sql_user_store.go index 8bd16f696..b15f349d1 100644 --- a/store/sql_user_store.go +++ b/store/sql_user_store.go @@ -729,20 +729,19 @@ func (us SqlUserStore) GetProfilesByUsernames(usernames []string, teamId string) idQuery += ":username" + strconv.Itoa(index) } - props["TeamId"] = teamId + var query string + if teamId == "" { + query = `SELECT * FROM Users WHERE Username IN (` + idQuery + `)` + } else { + query = `SELECT Users.* FROM Users INNER JOIN TeamMembers ON + Users.Id = TeamMembers.UserId AND Users.Username IN (` + idQuery + `) AND TeamMembers.TeamId = :TeamId ` + props["TeamId"] = teamId + } - if _, err := us.GetReplica().Select(&users, `SELECT Users.* FROM Users INNER JOIN TeamMembers ON - Users.Id = TeamMembers.UserId AND Users.Username IN (`+idQuery+`) AND TeamMembers.TeamId = :TeamId `, props); err != nil { + if _, err := us.GetReplica().Select(&users, query, props); err != nil { result.Err = model.NewLocAppError("SqlUserStore.GetProfilesByUsernames", "store.sql_user.get_profiles.app_error", nil, err.Error()) } else { - userMap := make(map[string]*model.User) - - for _, u := range users { - u.Sanitize(map[string]bool{}) - userMap[u.Id] = u - } - - result.Data = userMap + result.Data = users } storeChannel <- result diff --git a/store/sql_user_store_test.go b/store/sql_user_store_test.go index 7d7379668..367dc46dc 100644 --- a/store/sql_user_store_test.go +++ b/store/sql_user_store_test.go @@ -836,25 +836,67 @@ func TestUserStoreGetProfilesByUsernames(t *testing.T) { if r1 := <-store.User().GetProfilesByUsernames([]string{u1.Username, u2.Username}, teamId); r1.Err != nil { t.Fatal(r1.Err) } else { - users := r1.Data.(map[string]*model.User) + users := r1.Data.([]*model.User) if len(users) != 2 { t.Fatal("invalid returned users") } - if users[u1.Id].Id != u1.Id { - t.Fatal("invalid returned user") + if users[0].Id != u1.Id && users[1].Id != u1.Id { + t.Fatal("invalid returned user 1") + } + + if users[0].Id != u2.Id && users[1].Id != u2.Id { + t.Fatal("invalid returned user 2") } } if r1 := <-store.User().GetProfilesByUsernames([]string{u1.Username}, teamId); r1.Err != nil { t.Fatal(r1.Err) } else { - users := r1.Data.(map[string]*model.User) + users := r1.Data.([]*model.User) if len(users) != 1 { t.Fatal("invalid returned users") } - if users[u1.Id].Id != u1.Id { + if users[0].Id != u1.Id { + t.Fatal("invalid returned user") + } + } + + team2Id := model.NewId() + + u3 := &model.User{} + u3.Email = model.NewId() + u3.Username = "username3" + model.NewId() + Must(store.User().Save(u3)) + Must(store.Team().SaveMember(&model.TeamMember{TeamId: team2Id, UserId: u3.Id})) + + if r1 := <-store.User().GetProfilesByUsernames([]string{u1.Username, u3.Username}, ""); r1.Err != nil { + t.Fatal(r1.Err) + } else { + users := r1.Data.([]*model.User) + if len(users) != 2 { + t.Fatal("invalid returned users") + } + + if users[0].Id != u1.Id && users[1].Id != u1.Id { + t.Fatal("invalid returned user 1") + } + + if users[0].Id != u3.Id && users[1].Id != u3.Id { + t.Fatal("invalid returned user 3") + } + } + + if r1 := <-store.User().GetProfilesByUsernames([]string{u1.Username, u3.Username}, teamId); r1.Err != nil { + t.Fatal(r1.Err) + } else { + users := r1.Data.([]*model.User) + if len(users) != 1 { + t.Fatal("invalid returned users") + } + + if users[0].Id != u1.Id { t.Fatal("invalid returned user") } } -- cgit v1.2.3-1-g7c22