From 7ff2aef7facdeb025a1651ef411fceb3d81932c1 Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Fri, 3 Feb 2017 15:17:34 -0500 Subject: Implement GET /users endpoint for APIv4 (#5277) --- api/command_loadtest.go | 2 +- api/user.go | 29 ++------ api4/apitestlib.go | 2 + api4/channel_test.go | 12 +-- api4/params.go | 23 ++++++ api4/user.go | 62 ++++++++++++++++ api4/user_test.go | 172 +++++++++++++++++++++++++++++++++++++++++++ app/channel.go | 10 +-- app/notification.go | 2 +- app/user.go | 134 ++++++++++++++++++++++++++++++--- model/authorization.go | 7 ++ model/client4.go | 44 +++++++++++ store/sql_user_store.go | 60 +++++++++------ store/sql_user_store_test.go | 128 +++++++++++++++++++++++++++----- store/store.go | 3 +- 15 files changed, 603 insertions(+), 87 deletions(-) diff --git a/api/command_loadtest.go b/api/command_loadtest.go index 5ad2736a0..3b9ebfe47 100644 --- a/api/command_loadtest.go +++ b/api/command_loadtest.go @@ -291,7 +291,7 @@ func (me *LoadTestProvider) PostsCommand(c *Context, channelId string, message s var usernames []string if result := <-app.Srv.Store.User().GetProfiles(c.TeamId, 0, 1000); result.Err == nil { - profileUsers := result.Data.(map[string]*model.User) + profileUsers := result.Data.([]*model.User) usernames = make([]string, len(profileUsers)) i := 0 for _, userprof := range profileUsers { diff --git a/api/user.go b/api/user.go index 6f40388b2..c6d9e5c25 100644 --- a/api/user.go +++ b/api/user.go @@ -442,14 +442,10 @@ func getProfiles(c *Context, w http.ResponseWriter, r *http.Request) { return } - if profiles, err := app.GetUsers(offset, limit); err != nil { + if profiles, err := app.GetUsersMap(offset, limit, c.IsSystemAdmin()); err != nil { c.Err = err return } else { - for k, p := range profiles { - profiles[k] = sanitizeProfile(c, p) - } - w.Header().Set(model.HEADER_ETAG_SERVER, etag) w.Write([]byte(model.UserMapToJson(profiles))) } @@ -482,14 +478,10 @@ func getProfilesInTeam(c *Context, w http.ResponseWriter, r *http.Request) { return } - if profiles, err := app.GetUsersInTeam(teamId, offset, limit); err != nil { + if profiles, err := app.GetUsersInTeamMap(teamId, offset, limit, c.IsSystemAdmin()); err != nil { c.Err = err return } else { - for k, p := range profiles { - profiles[k] = sanitizeProfile(c, p) - } - w.Header().Set(model.HEADER_ETAG_SERVER, etag) w.Write([]byte(model.UserMapToJson(profiles))) } @@ -523,17 +515,10 @@ func getProfilesInChannel(c *Context, w http.ResponseWriter, r *http.Request) { return } - var profiles map[string]*model.User - var profileErr *model.AppError - - if profiles, err = app.GetUsersInChannel(channelId, offset, limit); profileErr != nil { - c.Err = profileErr + if profiles, err := app.GetUsersInChannelMap(channelId, offset, limit, c.IsSystemAdmin()); err != nil { + c.Err = err return } else { - for k, p := range profiles { - profiles[k] = sanitizeProfile(c, p) - } - w.Write([]byte(model.UserMapToJson(profiles))) } } @@ -566,14 +551,10 @@ func getProfilesNotInChannel(c *Context, w http.ResponseWriter, r *http.Request) return } - if profiles, err := app.GetUsersNotInChannel(c.TeamId, channelId, offset, limit); err != nil { + if profiles, err := app.GetUsersNotInChannelMap(c.TeamId, channelId, offset, limit, c.IsSystemAdmin()); err != nil { c.Err = err return } else { - for k, p := range profiles { - profiles[k] = sanitizeProfile(c, p) - } - w.Write([]byte(model.UserMapToJson(profiles))) } } diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 6229c8a08..f647ffa9c 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -97,6 +97,8 @@ func (me *TestHelper) InitBasic() *TestHelper { LinkUserToTeam(me.BasicUser, me.BasicTeam) me.BasicUser2 = me.CreateUser() LinkUserToTeam(me.BasicUser2, me.BasicTeam) + app.AddUserToChannel(me.BasicUser, me.BasicChannel) + app.AddUserToChannel(me.BasicUser2, me.BasicChannel) app.UpdateUserRoles(me.BasicUser.Id, model.ROLE_SYSTEM_USER.Id) me.LoginBasic() diff --git a/api4/channel_test.go b/api4/channel_test.go index 5123d7730..237d57f01 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -77,17 +77,17 @@ func TestCreateChannel(t *testing.T) { // Check permissions with policy config changes isLicensed := utils.IsLicensed license := utils.License - restrictPublicChannel := *utils.Cfg.TeamSettings.RestrictPublicChannelManagement - restrictPrivateChannel := *utils.Cfg.TeamSettings.RestrictPrivateChannelManagement + restrictPublicChannel := *utils.Cfg.TeamSettings.RestrictPublicChannelCreation + restrictPrivateChannel := *utils.Cfg.TeamSettings.RestrictPrivateChannelCreation defer func() { - *utils.Cfg.TeamSettings.RestrictPublicChannelManagement = restrictPublicChannel - *utils.Cfg.TeamSettings.RestrictPrivateChannelManagement = restrictPrivateChannel + *utils.Cfg.TeamSettings.RestrictPublicChannelCreation = restrictPublicChannel + *utils.Cfg.TeamSettings.RestrictPrivateChannelCreation = restrictPrivateChannel utils.IsLicensed = isLicensed utils.License = license utils.SetDefaultRolesBasedOnConfig() }() - *utils.Cfg.TeamSettings.RestrictPublicChannelManagement = model.PERMISSIONS_ALL - *utils.Cfg.TeamSettings.RestrictPrivateChannelManagement = model.PERMISSIONS_ALL + *utils.Cfg.TeamSettings.RestrictPublicChannelCreation = model.PERMISSIONS_ALL + *utils.Cfg.TeamSettings.RestrictPrivateChannelCreation = model.PERMISSIONS_ALL utils.SetDefaultRolesBasedOnConfig() utils.IsLicensed = true utils.License = &model.License{Features: &model.Features{}} diff --git a/api4/params.go b/api4/params.go index 452b9ba21..1c0c153ea 100644 --- a/api4/params.go +++ b/api4/params.go @@ -5,10 +5,17 @@ package api4 import ( "net/http" + "strconv" "github.com/gorilla/mux" ) +const ( + PAGE_DEFAULT = 0 + PER_PAGE_DEFAULT = 60 + PER_PAGE_MAXIMUM = 200 +) + type ApiParams struct { UserId string TeamId string @@ -18,6 +25,8 @@ type ApiParams struct { CommandId string HookId string EmojiId string + Page int + PerPage int } func ApiParamsFromRequest(r *http.Request) *ApiParams { @@ -57,5 +66,19 @@ func ApiParamsFromRequest(r *http.Request) *ApiParams { params.EmojiId = val } + if val, err := strconv.Atoi(r.URL.Query().Get("page")); err != nil { + params.Page = PAGE_DEFAULT + } else { + params.Page = val + } + + if val, err := strconv.Atoi(r.URL.Query().Get("per_page")); err != nil { + params.PerPage = PER_PAGE_DEFAULT + } else if val > PER_PAGE_MAXIMUM { + params.PerPage = PER_PAGE_MAXIMUM + } else { + params.PerPage = val + } + return params } diff --git a/api4/user.go b/api4/user.go index 19d3446fb..74983aa54 100644 --- a/api4/user.go +++ b/api4/user.go @@ -16,6 +16,7 @@ func InitUser() { l4g.Debug(utils.T("api.user.init.debug")) BaseRoutes.Users.Handle("", ApiHandler(createUser)).Methods("POST") + BaseRoutes.Users.Handle("", ApiSessionRequired(getUsers)).Methods("GET") BaseRoutes.Users.Handle("/ids", ApiSessionRequired(getUsersByIds)).Methods("POST") BaseRoutes.User.Handle("", ApiSessionRequired(getUser)).Methods("GET") @@ -86,6 +87,67 @@ func getUser(c *Context, w http.ResponseWriter, r *http.Request) { } } +func getUsers(c *Context, w http.ResponseWriter, r *http.Request) { + inTeamId := r.URL.Query().Get("in_team") + inChannelId := r.URL.Query().Get("in_channel") + notInChannelId := r.URL.Query().Get("not_in_channel") + + if len(notInChannelId) > 0 && len(inTeamId) == 0 { + c.SetInvalidParam("team_id") + return + } + + var profiles []*model.User + var err *model.AppError + etag := "" + + if len(notInChannelId) > 0 { + if !app.SessionHasPermissionToChannel(c.Session, notInChannelId, model.PERMISSION_READ_CHANNEL) { + c.SetPermissionError(model.PERMISSION_READ_CHANNEL) + return + } + + profiles, err = app.GetUsersNotInChannelPage(inTeamId, notInChannelId, c.Params.Page, c.Params.PerPage, c.IsSystemAdmin()) + } else if len(inTeamId) > 0 { + if !app.SessionHasPermissionToTeam(c.Session, inTeamId, model.PERMISSION_VIEW_TEAM) { + c.SetPermissionError(model.PERMISSION_VIEW_TEAM) + return + } + + etag = app.GetUsersInTeamEtag(inTeamId) + if HandleEtag(etag, "Get Users in Team", w, r) { + return + } + + profiles, err = app.GetUsersInTeamPage(inTeamId, c.Params.Page, c.Params.PerPage, c.IsSystemAdmin()) + } else if len(inChannelId) > 0 { + if !app.SessionHasPermissionToChannel(c.Session, inChannelId, model.PERMISSION_READ_CHANNEL) { + c.SetPermissionError(model.PERMISSION_READ_CHANNEL) + return + } + + profiles, err = app.GetUsersInChannelPage(inChannelId, c.Params.Page, c.Params.PerPage, c.IsSystemAdmin()) + } else { + // No permission check required + + etag = app.GetUsersEtag() + if HandleEtag(etag, "Get Users", w, r) { + return + } + profiles, err = app.GetUsersPage(c.Params.Page, c.Params.PerPage, c.IsSystemAdmin()) + } + + if err != nil { + c.Err = err + return + } else { + if len(etag) > 0 { + w.Header().Set(model.HEADER_ETAG_SERVER, etag) + } + w.Write([]byte(model.UserListToJson(profiles))) + } +} + func getUsersByIds(c *Context, w http.ResponseWriter, r *http.Request) { userIds := model.ArrayFromJson(r.Body) diff --git a/api4/user_test.go b/api4/user_test.go index 54aae4e49..082b48dd6 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -242,3 +242,175 @@ func TestUpdateUserRoles(t *testing.T) { _, resp = SystemAdminClient.UpdateUserRoles(model.NewId(), model.ROLE_SYSTEM_USER.Id) CheckBadRequestStatus(t, resp) } + +func TestGetUsers(t *testing.T) { + th := Setup().InitBasic() + defer TearDown() + Client := th.Client + + rusers, resp := Client.GetUsers(0, 60, "") + CheckNoError(t, resp) + for _, u := range rusers { + CheckUserSanitization(t, u) + } + + rusers, resp = Client.GetUsers(0, 60, resp.Etag) + CheckEtag(t, rusers, resp) + + rusers, resp = Client.GetUsers(0, 1, "") + CheckNoError(t, resp) + if len(rusers) != 1 { + t.Fatal("should be 1 per page") + } + + rusers, resp = Client.GetUsers(1, 1, "") + CheckNoError(t, resp) + if len(rusers) != 1 { + t.Fatal("should be 1 per page") + } + + rusers, resp = Client.GetUsers(10000, 100, "") + CheckNoError(t, resp) + if len(rusers) != 0 { + t.Fatal("should be no users") + } + + // Check default params for page and per_page + if _, err := Client.DoApiGet("/users", ""); err != nil { + t.Fatal("should not have errored") + } + + Client.Logout() + _, resp = Client.GetUsers(0, 60, "") + CheckUnauthorizedStatus(t, resp) +} + +func TestGetUsersInTeam(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer TearDown() + Client := th.Client + teamId := th.BasicTeam.Id + + rusers, resp := Client.GetUsersInTeam(teamId, 0, 60, "") + CheckNoError(t, resp) + for _, u := range rusers { + CheckUserSanitization(t, u) + } + + rusers, resp = Client.GetUsersInTeam(teamId, 0, 60, resp.Etag) + CheckEtag(t, rusers, resp) + + rusers, resp = Client.GetUsersInTeam(teamId, 0, 1, "") + CheckNoError(t, resp) + if len(rusers) != 1 { + t.Fatal("should be 1 per page") + } + + rusers, resp = Client.GetUsersInTeam(teamId, 1, 1, "") + CheckNoError(t, resp) + if len(rusers) != 1 { + t.Fatal("should be 1 per page") + } + + rusers, resp = Client.GetUsersInTeam(teamId, 10000, 100, "") + CheckNoError(t, resp) + if len(rusers) != 0 { + t.Fatal("should be no users") + } + + Client.Logout() + _, resp = Client.GetUsersInTeam(teamId, 0, 60, "") + CheckUnauthorizedStatus(t, resp) + + user := th.CreateUser() + Client.Login(user.Email, user.Password) + _, resp = Client.GetUsersInTeam(teamId, 0, 60, "") + CheckForbiddenStatus(t, resp) + + _, resp = th.SystemAdminClient.GetUsersInTeam(teamId, 0, 60, "") + CheckNoError(t, resp) +} + +func TestGetUsersInChannel(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer TearDown() + Client := th.Client + channelId := th.BasicChannel.Id + + rusers, resp := Client.GetUsersInChannel(channelId, 0, 60, "") + CheckNoError(t, resp) + for _, u := range rusers { + CheckUserSanitization(t, u) + } + + rusers, resp = Client.GetUsersInChannel(channelId, 0, 1, "") + CheckNoError(t, resp) + if len(rusers) != 1 { + t.Fatal("should be 1 per page") + } + + rusers, resp = Client.GetUsersInChannel(channelId, 1, 1, "") + CheckNoError(t, resp) + if len(rusers) != 1 { + t.Fatal("should be 1 per page") + } + + rusers, resp = Client.GetUsersInChannel(channelId, 10000, 100, "") + CheckNoError(t, resp) + if len(rusers) != 0 { + t.Fatal("should be no users") + } + + Client.Logout() + _, resp = Client.GetUsersInChannel(channelId, 0, 60, "") + CheckUnauthorizedStatus(t, resp) + + user := th.CreateUser() + Client.Login(user.Email, user.Password) + _, resp = Client.GetUsersInChannel(channelId, 0, 60, "") + CheckForbiddenStatus(t, resp) + + _, resp = th.SystemAdminClient.GetUsersInChannel(channelId, 0, 60, "") + CheckNoError(t, resp) +} + +func TestGetUsersNotInChannel(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer TearDown() + Client := th.Client + teamId := th.BasicTeam.Id + channelId := th.BasicChannel.Id + + user := th.CreateUser() + LinkUserToTeam(user, th.BasicTeam) + + rusers, resp := Client.GetUsersNotInChannel(teamId, channelId, 0, 60, "") + CheckNoError(t, resp) + for _, u := range rusers { + CheckUserSanitization(t, u) + } + + rusers, resp = Client.GetUsersNotInChannel(teamId, channelId, 0, 1, "") + CheckNoError(t, resp) + if len(rusers) != 1 { + t.Log(len(rusers)) + t.Fatal("should be 1 per page") + } + + rusers, resp = Client.GetUsersNotInChannel(teamId, channelId, 10000, 100, "") + CheckNoError(t, resp) + if len(rusers) != 0 { + t.Fatal("should be no users") + } + + Client.Logout() + _, resp = Client.GetUsersNotInChannel(teamId, channelId, 0, 60, "") + CheckUnauthorizedStatus(t, resp) + + Client.Login(user.Email, user.Password) + _, resp = Client.GetUsersNotInChannel(teamId, channelId, 0, 60, "") + CheckForbiddenStatus(t, resp) + + _, resp = th.SystemAdminClient.GetUsersNotInChannel(teamId, channelId, 0, 60, "") + CheckNoError(t, resp) +} diff --git a/app/channel.go b/app/channel.go index 02124f3c8..3609a36b7 100644 --- a/app/channel.go +++ b/app/channel.go @@ -404,22 +404,20 @@ func AddUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelM } func AddDirectChannels(teamId string, user *model.User) *model.AppError { - var profiles map[string]*model.User + var profiles []*model.User if result := <-Srv.Store.User().GetProfiles(teamId, 0, 100); result.Err != nil { return model.NewLocAppError("AddDirectChannels", "api.user.add_direct_channels_and_forget.failed.error", map[string]interface{}{"UserId": user.Id, "TeamId": teamId, "Error": result.Err.Error()}, "") } else { - profiles = result.Data.(map[string]*model.User) + profiles = result.Data.([]*model.User) } var preferences model.Preferences - for id := range profiles { - if id == user.Id { + for _, profile := range profiles { + if profile.Id == user.Id { continue } - profile := profiles[id] - preference := model.Preference{ UserId: user.Id, Category: model.PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW, diff --git a/app/notification.go b/app/notification.go index 9ad0b346b..cc4e13ab1 100644 --- a/app/notification.go +++ b/app/notification.go @@ -25,7 +25,7 @@ import ( ) func SendNotifications(post *model.Post, team *model.Team, channel *model.Channel, sender *model.User) ([]string, *model.AppError) { - pchan := Srv.Store.User().GetProfilesInChannel(channel.Id, -1, -1, true) + pchan := Srv.Store.User().GetAllProfilesInChannel(channel.Id, true) fchan := Srv.Store.FileInfo().GetForPost(post.Id) var profileMap map[string]*model.User diff --git a/app/user.go b/app/user.go index f9137b1e9..bc809cfaa 100644 --- a/app/user.go +++ b/app/user.go @@ -377,44 +377,160 @@ func GetUserForLogin(loginId string, onlyLdap bool) (*model.User, *model.AppErro } } -func GetUsers(offset int, limit int) (map[string]*model.User, *model.AppError) { +func GetUsers(offset int, limit int) ([]*model.User, *model.AppError) { if result := <-Srv.Store.User().GetAllProfiles(offset, limit); result.Err != nil { return nil, result.Err } else { - return result.Data.(map[string]*model.User), nil + return result.Data.([]*model.User), nil + } +} + +func GetUsersMap(page int, perPage int, asAdmin bool) (map[string]*model.User, *model.AppError) { + users, err := GetUsers(page*perPage, perPage) + if err != nil { + return nil, err + } + + userMap := make(map[string]*model.User, len(users)) + + for _, user := range users { + SanitizeProfile(user, asAdmin) + userMap[user.Id] = user } + + return userMap, nil +} + +func GetUsersPage(page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { + users, err := GetUsers(page*perPage, perPage) + if err != nil { + return nil, err + } + + for _, user := range users { + SanitizeProfile(user, asAdmin) + } + + return users, nil } func GetUsersEtag() string { return (<-Srv.Store.User().GetEtagForAllProfiles()).Data.(string) } -func GetUsersInTeam(teamId string, offset int, limit int) (map[string]*model.User, *model.AppError) { +func GetUsersInTeam(teamId string, offset int, limit int) ([]*model.User, *model.AppError) { if result := <-Srv.Store.User().GetProfiles(teamId, offset, limit); result.Err != nil { return nil, result.Err } else { - return result.Data.(map[string]*model.User), nil + return result.Data.([]*model.User), nil + } +} + +func GetUsersInTeamMap(teamId string, offset int, limit int, asAdmin bool) (map[string]*model.User, *model.AppError) { + users, err := GetUsersInTeam(teamId, offset, limit) + if err != nil { + return nil, err + } + + userMap := make(map[string]*model.User, len(users)) + + for _, user := range users { + SanitizeProfile(user, asAdmin) + userMap[user.Id] = user } + + return userMap, nil +} + +func GetUsersInTeamPage(teamId string, page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { + users, err := GetUsersInTeam(teamId, page*perPage, perPage) + if err != nil { + return nil, err + } + + for _, user := range users { + SanitizeProfile(user, asAdmin) + } + + return users, nil } func GetUsersInTeamEtag(teamId string) string { return (<-Srv.Store.User().GetEtagForProfiles(teamId)).Data.(string) } -func GetUsersInChannel(channelId string, offset int, limit int) (map[string]*model.User, *model.AppError) { - if result := <-Srv.Store.User().GetProfilesInChannel(channelId, offset, limit, false); result.Err != nil { +func GetUsersInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { + if result := <-Srv.Store.User().GetProfilesInChannel(channelId, offset, limit); result.Err != nil { return nil, result.Err } else { - return result.Data.(map[string]*model.User), nil + return result.Data.([]*model.User), nil + } +} + +func GetUsersInChannelMap(channelId string, offset int, limit int, asAdmin bool) (map[string]*model.User, *model.AppError) { + users, err := GetUsersInChannel(channelId, offset, limit) + if err != nil { + return nil, err + } + + userMap := make(map[string]*model.User, len(users)) + + for _, user := range users { + SanitizeProfile(user, asAdmin) + userMap[user.Id] = user } + + return userMap, nil +} + +func GetUsersInChannelPage(channelId string, page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { + users, err := GetUsersInChannel(channelId, page*perPage, perPage) + if err != nil { + return nil, err + } + + for _, user := range users { + SanitizeProfile(user, asAdmin) + } + + return users, nil } -func GetUsersNotInChannel(teamId string, channelId string, offset int, limit int) (map[string]*model.User, *model.AppError) { +func GetUsersNotInChannel(teamId string, channelId string, offset int, limit int) ([]*model.User, *model.AppError) { if result := <-Srv.Store.User().GetProfilesNotInChannel(teamId, channelId, offset, limit); result.Err != nil { return nil, result.Err } else { - return result.Data.(map[string]*model.User), nil + return result.Data.([]*model.User), nil + } +} + +func GetUsersNotInChannelMap(teamId string, channelId string, offset int, limit int, asAdmin bool) (map[string]*model.User, *model.AppError) { + users, err := GetUsersNotInChannel(teamId, channelId, offset, limit) + if err != nil { + return nil, err + } + + userMap := make(map[string]*model.User, len(users)) + + for _, user := range users { + SanitizeProfile(user, asAdmin) + userMap[user.Id] = user } + + return userMap, nil +} + +func GetUsersNotInChannelPage(teamId string, channelId string, page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { + users, err := GetUsersNotInChannel(teamId, channelId, page*perPage, perPage) + if err != nil { + return nil, err + } + + for _, user := range users { + SanitizeProfile(user, asAdmin) + } + + return users, nil } func GetUsersByIds(userIds []string, asAdmin bool) ([]*model.User, *model.AppError) { diff --git a/model/authorization.go b/model/authorization.go index 56bb58913..ed38bf9b7 100644 --- a/model/authorization.go +++ b/model/authorization.go @@ -54,6 +54,7 @@ var PERMISSION_REMOVE_USER_FROM_TEAM *Permission var PERMISSION_CREATE_TEAM *Permission var PERMISSION_MANAGE_TEAM *Permission var PERMISSION_IMPORT_TEAM *Permission +var PERMISSION_VIEW_TEAM *Permission // General permission that encompases all system admin functions // in the future this could be broken up to allow access to some @@ -268,6 +269,11 @@ func InitalizePermissions() { "authentication.permissions.import_team.name", "authentication.permissions.import_team.description", } + PERMISSION_VIEW_TEAM = &Permission{ + "view_team", + "authentication.permissions.view_team.name", + "authentication.permissions.view_team.description", + } } func InitalizeRoles() { @@ -314,6 +320,7 @@ func InitalizeRoles() { []string{ PERMISSION_LIST_TEAM_CHANNELS.Id, PERMISSION_JOIN_PUBLIC_CHANNELS.Id, + PERMISSION_VIEW_TEAM.Id, }, } BuiltInRoles[ROLE_TEAM_USER.Id] = ROLE_TEAM_USER diff --git a/model/client4.go b/model/client4.go index 6ea0398d0..e189257d2 100644 --- a/model/client4.go +++ b/model/client4.go @@ -210,6 +210,50 @@ func (c *Client4) GetUser(userId, etag string) (*User, *Response) { } } +// GetUsers returns a page of users on the system. Page counting starts at 0. +func (c *Client4) GetUsers(page int, perPage int, etag string) ([]*User, *Response) { + query := fmt.Sprintf("?page=%v&per_page=%v", page, perPage) + if r, err := c.DoApiGet(c.GetUsersRoute()+query, etag); err != nil { + return nil, &Response{StatusCode: r.StatusCode, Error: err} + } else { + defer closeBody(r) + return UserListFromJson(r.Body), BuildResponse(r) + } +} + +// GetUsersInTeam returns a page of users on a team. Page counting starts at 0. +func (c *Client4) GetUsersInTeam(teamId string, page int, perPage int, etag string) ([]*User, *Response) { + query := fmt.Sprintf("?in_team=%v&page=%v&per_page=%v", teamId, page, perPage) + if r, err := c.DoApiGet(c.GetUsersRoute()+query, etag); err != nil { + return nil, &Response{StatusCode: r.StatusCode, Error: err} + } else { + defer closeBody(r) + return UserListFromJson(r.Body), BuildResponse(r) + } +} + +// GetUsersInChannel returns a page of users on a team. Page counting starts at 0. +func (c *Client4) GetUsersInChannel(channelId string, page int, perPage int, etag string) ([]*User, *Response) { + query := fmt.Sprintf("?in_channel=%v&page=%v&per_page=%v", channelId, page, perPage) + if r, err := c.DoApiGet(c.GetUsersRoute()+query, etag); err != nil { + return nil, &Response{StatusCode: r.StatusCode, Error: err} + } else { + defer closeBody(r) + return UserListFromJson(r.Body), BuildResponse(r) + } +} + +// GetUsersNotInChannel returns a page of users on a team. Page counting starts at 0. +func (c *Client4) GetUsersNotInChannel(teamId, channelId string, page int, perPage int, etag string) ([]*User, *Response) { + query := fmt.Sprintf("?in_team=%v¬_in_channel=%v&page=%v&per_page=%v", teamId, channelId, page, perPage) + if r, err := c.DoApiGet(c.GetUsersRoute()+query, etag); err != nil { + return nil, &Response{StatusCode: r.StatusCode, Error: err} + } else { + defer closeBody(r) + return UserListFromJson(r.Body), BuildResponse(r) + } +} + // GetUsersByIds returns a list of users based on the provided user ids. func (c *Client4) GetUsersByIds(userIds []string) ([]*User, *Response) { if r, err := c.DoApiPost(c.GetUsersRoute()+"/ids", ArrayToJson(userIds)); err != nil { diff --git a/store/sql_user_store.go b/store/sql_user_store.go index 827c5a064..7ca33fd78 100644 --- a/store/sql_user_store.go +++ b/store/sql_user_store.go @@ -510,16 +510,13 @@ func (us SqlUserStore) GetAllProfiles(offset int, limit int) StoreChannel { result.Err = model.NewLocAppError("SqlUserStore.GetAllProfiles", "store.sql_user.get_profiles.app_error", nil, err.Error()) } else { - userMap := make(map[string]*model.User) - for _, u := range users { u.Password = "" u.AuthData = new(string) *u.AuthData = "" - userMap[u.Id] = u } - result.Data = userMap + result.Data = users } storeChannel <- result @@ -562,16 +559,13 @@ func (us SqlUserStore) GetProfiles(teamId string, offset int, limit int) StoreCh result.Err = model.NewLocAppError("SqlUserStore.GetProfiles", "store.sql_user.get_profiles.app_error", nil, err.Error()) } else { - userMap := make(map[string]*model.User) - for _, u := range users { u.Password = "" u.AuthData = new(string) *u.AuthData = "" - userMap[u.Id] = u } - result.Data = userMap + result.Data = users } storeChannel <- result @@ -598,7 +592,38 @@ func (us SqlUserStore) InvalidateProfilesInChannelCache(channelId string) { profilesInChannelCache.Remove(channelId) } -func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit int, allowFromCache bool) StoreChannel { +func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit int) StoreChannel { + + storeChannel := make(StoreChannel) + + go func() { + result := StoreResult{} + + var users []*model.User + + query := "SELECT Users.* FROM Users, ChannelMembers WHERE ChannelMembers.ChannelId = :ChannelId AND Users.Id = ChannelMembers.UserId AND Users.DeleteAt = 0 ORDER BY Users.Username ASC LIMIT :Limit OFFSET :Offset" + + if _, err := us.GetReplica().Select(&users, query, map[string]interface{}{"ChannelId": channelId, "Offset": offset, "Limit": limit}); err != nil { + result.Err = model.NewLocAppError("SqlUserStore.GetProfilesInChannel", "store.sql_user.get_profiles.app_error", nil, err.Error()) + } else { + + for _, u := range users { + u.Password = "" + u.AuthData = new(string) + *u.AuthData = "" + } + + result.Data = users + } + + storeChannel <- result + close(storeChannel) + }() + + return storeChannel +} + +func (us SqlUserStore) GetAllProfilesInChannel(channelId string, allowFromCache bool) StoreChannel { storeChannel := make(StoreChannel) @@ -606,7 +631,7 @@ func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit result := StoreResult{} metrics := einterfaces.GetMetricsInterface() - if allowFromCache && offset == -1 && limit == -1 { + if allowFromCache { if cacheItem, ok := profilesInChannelCache.Get(channelId); ok { if metrics != nil { metrics.IncrementMemCacheHitCounter("Profiles in Channel") @@ -630,12 +655,8 @@ func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit query := "SELECT Users.* FROM Users, ChannelMembers WHERE ChannelMembers.ChannelId = :ChannelId AND Users.Id = ChannelMembers.UserId AND Users.DeleteAt = 0" - if limit >= 0 && offset >= 0 { - query += " ORDER BY Users.Username ASC LIMIT :Limit OFFSET :Offset" - } - - if _, err := us.GetReplica().Select(&users, query, map[string]interface{}{"ChannelId": channelId, "Offset": offset, "Limit": limit}); err != nil { - result.Err = model.NewLocAppError("SqlUserStore.GetProfilesInChannel", "store.sql_user.get_profiles.app_error", nil, err.Error()) + if _, err := us.GetReplica().Select(&users, query, map[string]interface{}{"ChannelId": channelId}); err != nil { + result.Err = model.NewLocAppError("SqlUserStore.GetAllProfilesInChannel", "store.sql_user.get_profiles.app_error", nil, err.Error()) } else { userMap := make(map[string]*model.User) @@ -649,7 +670,7 @@ func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit result.Data = userMap - if allowFromCache && offset == -1 && limit == -1 { + if allowFromCache { profilesInChannelCache.AddWithExpiresInSecs(channelId, userMap, PROFILES_IN_CHANNEL_CACHE_SEC) } } @@ -688,16 +709,13 @@ func (us SqlUserStore) GetProfilesNotInChannel(teamId string, channelId string, result.Err = model.NewLocAppError("SqlUserStore.GetProfilesNotInChannel", "store.sql_user.get_profiles.app_error", nil, err.Error()) } else { - userMap := make(map[string]*model.User) - for _, u := range users { u.Password = "" u.AuthData = new(string) *u.AuthData = "" - 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 449c6aa52..95be3e258 100644 --- a/store/sql_user_store_test.go +++ b/store/sql_user_store_test.go @@ -208,7 +208,7 @@ func TestUserStoreGetAllProfiles(t *testing.T) { if r1 := <-store.User().GetAllProfiles(0, 100); 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") } @@ -217,7 +217,7 @@ func TestUserStoreGetAllProfiles(t *testing.T) { if r2 := <-store.User().GetAllProfiles(0, 1); r2.Err != nil { t.Fatal(r2.Err) } else { - users := r2.Data.(map[string]*model.User) + users := r2.Data.([]*model.User) if len(users) != 1 { t.Fatal("invalid returned users, limit did not work") } @@ -242,20 +242,27 @@ func TestUserStoreGetProfiles(t *testing.T) { if r1 := <-store.User().GetProfiles(teamId, 0, 100); 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") + found := false + for _, u := range users { + if u.Id == u1.Id { + found = true + } + } + + if !found { + t.Fatal("missing user") } } if r2 := <-store.User().GetProfiles("123", 0, 100); r2.Err != nil { t.Fatal(r2.Err) } else { - if len(r2.Data.(map[string]*model.User)) != 0 { + if len(r2.Data.([]*model.User)) != 0 { t.Fatal("should have returned empty map") } } @@ -310,7 +317,85 @@ func TestUserStoreGetProfilesInChannel(t *testing.T) { Must(store.Channel().SaveMember(&m2)) Must(store.Channel().SaveMember(&m3)) - if r1 := <-store.User().GetProfilesInChannel(c1.Id, -1, -1, false); r1.Err != nil { + if r1 := <-store.User().GetProfilesInChannel(c1.Id, 0, 100); r1.Err != nil { + t.Fatal(r1.Err) + } else { + users := r1.Data.([]*model.User) + if len(users) != 2 { + t.Fatal("invalid returned users") + } + + found := false + for _, u := range users { + if u.Id == u1.Id { + found = true + } + } + + if !found { + t.Fatal("missing user") + } + } + + if r2 := <-store.User().GetProfilesInChannel(c2.Id, 0, 1); r2.Err != nil { + t.Fatal(r2.Err) + } else { + if len(r2.Data.([]*model.User)) != 1 { + t.Fatal("should have returned only 1 user") + } + } +} + +func TestUserStoreGetAllProfilesInChannel(t *testing.T) { + Setup() + + teamId := model.NewId() + + u1 := &model.User{} + u1.Email = model.NewId() + Must(store.User().Save(u1)) + Must(store.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u1.Id})) + + u2 := &model.User{} + u2.Email = model.NewId() + Must(store.User().Save(u2)) + Must(store.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u2.Id})) + + c1 := model.Channel{} + c1.TeamId = teamId + c1.DisplayName = "Profiles in channel" + c1.Name = "profiles-" + model.NewId() + c1.Type = model.CHANNEL_OPEN + + c2 := model.Channel{} + c2.TeamId = teamId + c2.DisplayName = "Profiles in private" + c2.Name = "profiles-" + model.NewId() + c2.Type = model.CHANNEL_PRIVATE + + Must(store.Channel().Save(&c1)) + Must(store.Channel().Save(&c2)) + + m1 := model.ChannelMember{} + m1.ChannelId = c1.Id + m1.UserId = u1.Id + m1.NotifyProps = model.GetDefaultChannelNotifyProps() + + m2 := model.ChannelMember{} + m2.ChannelId = c1.Id + m2.UserId = u2.Id + m2.NotifyProps = model.GetDefaultChannelNotifyProps() + + m3 := model.ChannelMember{} + m3.ChannelId = c2.Id + m3.UserId = u1.Id + m3.NotifyProps = model.GetDefaultChannelNotifyProps() + + Must(store.Channel().SaveMember(&m1)) + Must(store.Channel().SaveMember(&m2)) + Must(store.Channel().SaveMember(&m3)) + + if r1 := <-store.User().GetAllProfilesInChannel(c1.Id, false); r1.Err != nil { t.Fatal(r1.Err) } else { users := r1.Data.(map[string]*model.User) @@ -323,7 +408,7 @@ func TestUserStoreGetProfilesInChannel(t *testing.T) { } } - if r2 := <-store.User().GetProfilesInChannel(c2.Id, -1, -1, false); r2.Err != nil { + if r2 := <-store.User().GetAllProfilesInChannel(c2.Id, false); r2.Err != nil { t.Fatal(r2.Err) } else { if len(r2.Data.(map[string]*model.User)) != 1 { @@ -331,7 +416,7 @@ func TestUserStoreGetProfilesInChannel(t *testing.T) { } } - if r2 := <-store.User().GetProfilesInChannel(c2.Id, -1, -1, true); r2.Err != nil { + if r2 := <-store.User().GetAllProfilesInChannel(c2.Id, true); r2.Err != nil { t.Fatal(r2.Err) } else { if len(r2.Data.(map[string]*model.User)) != 1 { @@ -339,7 +424,7 @@ func TestUserStoreGetProfilesInChannel(t *testing.T) { } } - if r2 := <-store.User().GetProfilesInChannel(c2.Id, -1, -1, true); r2.Err != nil { + if r2 := <-store.User().GetAllProfilesInChannel(c2.Id, true); r2.Err != nil { t.Fatal(r2.Err) } else { if len(r2.Data.(map[string]*model.User)) != 1 { @@ -383,20 +468,27 @@ func TestUserStoreGetProfilesNotInChannel(t *testing.T) { if r1 := <-store.User().GetProfilesNotInChannel(teamId, c1.Id, 0, 100); 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") + found := false + for _, u := range users { + if u.Id == u1.Id { + found = true + } + } + + if !found { + t.Fatal("missing user") } } if r2 := <-store.User().GetProfilesNotInChannel(teamId, c2.Id, 0, 100); r2.Err != nil { t.Fatal(r2.Err) } else { - if len(r2.Data.(map[string]*model.User)) != 2 { + if len(r2.Data.([]*model.User)) != 2 { t.Fatal("invalid returned users") } } @@ -423,7 +515,7 @@ func TestUserStoreGetProfilesNotInChannel(t *testing.T) { if r1 := <-store.User().GetProfilesNotInChannel(teamId, c1.Id, 0, 100); r1.Err != nil { t.Fatal(r1.Err) } else { - users := r1.Data.(map[string]*model.User) + users := r1.Data.([]*model.User) if len(users) != 0 { t.Fatal("invalid returned users") } @@ -432,7 +524,7 @@ func TestUserStoreGetProfilesNotInChannel(t *testing.T) { if r2 := <-store.User().GetProfilesNotInChannel(teamId, c2.Id, 0, 100); r2.Err != nil { t.Fatal(r2.Err) } else { - if len(r2.Data.(map[string]*model.User)) != 1 { + if len(r2.Data.([]*model.User)) != 1 { t.Fatal("should have had 1 user not in channel") } } @@ -576,8 +668,8 @@ func TestUserStoreGetProfilesByIds(t *testing.T) { if r2 := <-store.User().GetProfiles("123", 0, 100); r2.Err != nil { t.Fatal(r2.Err) } else { - if len(r2.Data.(map[string]*model.User)) != 0 { - t.Fatal("should have returned empty map") + if len(r2.Data.([]*model.User)) != 0 { + t.Fatal("should have returned empty array") } } } diff --git a/store/store.go b/store/store.go index c75ec4554..96d9509b8 100644 --- a/store/store.go +++ b/store/store.go @@ -162,7 +162,8 @@ type UserStore interface { GetAll() StoreChannel InvalidateProfilesInChannelCacheByUser(userId string) InvalidateProfilesInChannelCache(channelId string) - GetProfilesInChannel(channelId string, offset int, limit int, allowFromCache bool) StoreChannel + GetProfilesInChannel(channelId string, offset int, limit int) StoreChannel + GetAllProfilesInChannel(channelId string, allowFromCache bool) StoreChannel GetProfilesNotInChannel(teamId string, channelId string, offset int, limit int) StoreChannel GetProfilesByUsernames(usernames []string, teamId string) StoreChannel GetAllProfiles(offset int, limit int) StoreChannel -- cgit v1.2.3-1-g7c22