From a67898186175d9986b9e8cd8321afbc6fa84c8c6 Mon Sep 17 00:00:00 2001 From: George Goldberg Date: Wed, 10 May 2017 14:08:06 +0100 Subject: PLT-6488: Reduce database queries in user bulk import. (#6371) --- app/channel.go | 25 +++++++------- app/import.go | 104 ++++++++++++++++++++++++++++++++++++++++++--------------- app/team.go | 8 ++--- 3 files changed, 95 insertions(+), 42 deletions(-) (limited to 'app') diff --git a/app/channel.go b/app/channel.go index b4855bc9e..f39a57839 100644 --- a/app/channel.go +++ b/app/channel.go @@ -435,7 +435,7 @@ func DeleteChannel(channel *model.Channel, userId string) *model.AppError { return nil } -func addUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelMember, *model.AppError) { +func addUserToChannel(user *model.User, channel *model.Channel, teamMember *model.TeamMember) (*model.ChannelMember, *model.AppError) { if channel.DeleteAt > 0 { return nil, model.NewLocAppError("AddUserToChannel", "api.channel.add_user_to_channel.deleted.app_error", nil, "") } @@ -444,18 +444,8 @@ func addUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelM return nil, model.NewLocAppError("AddUserToChannel", "api.channel.add_user_to_channel.type.app_error", nil, "") } - tmchan := Srv.Store.Team().GetMember(channel.TeamId, user.Id) cmchan := Srv.Store.Channel().GetMember(channel.Id, user.Id) - if result := <-tmchan; result.Err != nil { - return nil, result.Err - } else { - teamMember := result.Data.(*model.TeamMember) - if teamMember.DeleteAt > 0 { - return nil, model.NewLocAppError("AddUserToChannel", "api.channel.add_user.to.channel.failed.deleted.app_error", nil, "") - } - } - if result := <-cmchan; result.Err != nil { if result.Err.Id != store.MISSING_CHANNEL_MEMBER_ERROR { return nil, result.Err @@ -485,8 +475,19 @@ func addUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelM } func AddUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelMember, *model.AppError) { + tmchan := Srv.Store.Team().GetMember(channel.TeamId, user.Id) + var teamMember *model.TeamMember + + if result := <-tmchan; result.Err != nil { + return nil, result.Err + } else { + teamMember = result.Data.(*model.TeamMember) + if teamMember.DeleteAt > 0 { + return nil, model.NewLocAppError("AddUserToChannel", "api.channel.add_user.to.channel.failed.deleted.app_error", nil, "") + } + } - newMember, err := addUserToChannel(user, channel) + newMember, err := addUserToChannel(user, channel, teamMember) if err != nil { return nil, err } diff --git a/app/import.go b/app/import.go index eb51fd34e..156591c0b 100644 --- a/app/import.go +++ b/app/import.go @@ -408,26 +408,44 @@ func ImportUser(data *UserImportData, dryRun bool) *model.AppError { return nil } + // We want to avoid database writes if nothing has changed. + hasUserChanged := false + hasUserRolesChanged := false + hasUserAuthDataChanged := false + hasUserEmailVerifiedChanged := false + var user *model.User if result := <-Srv.Store.User().GetByUsername(*data.Username); result.Err == nil { user = result.Data.(*model.User) } else { user = &model.User{} + hasUserChanged = true } user.Username = *data.Username - user.Email = *data.Email + + if user.Email != *data.Email { + hasUserChanged = true + hasUserEmailVerifiedChanged = true // Changing the email resets email verified to false by default. + user.Email = *data.Email + } var password string var authService string var authData *string if data.AuthService != nil { + if user.AuthService != *data.AuthService { + hasUserAuthDataChanged = true + } authService = *data.AuthService } // AuthData and Password are mutually exclusive. if data.AuthData != nil { + if user.AuthData == nil || *user.AuthData != *data.AuthData { + hasUserAuthDataChanged = true + } authData = data.AuthData password = "" } else if data.Password != nil { @@ -445,36 +463,63 @@ func ImportUser(data *UserImportData, dryRun bool) *model.AppError { // Automatically assume all emails are verified. emailVerified := true - user.EmailVerified = emailVerified + if user.EmailVerified != emailVerified { + user.EmailVerified = emailVerified + hasUserEmailVerifiedChanged = true + } if data.Nickname != nil { - user.Nickname = *data.Nickname + if user.Nickname != *data.Nickname { + user.Nickname = *data.Nickname + hasUserChanged = true + } } if data.FirstName != nil { - user.FirstName = *data.FirstName + if user.FirstName != *data.FirstName { + user.FirstName = *data.FirstName + hasUserChanged = true + } } if data.LastName != nil { - user.LastName = *data.LastName + if user.LastName != *data.LastName { + user.LastName = *data.LastName + hasUserChanged = true + } } if data.Position != nil { - user.Position = *data.Position + if user.Position != *data.Position { + user.Position = *data.Position + hasUserChanged = true + } } if data.Locale != nil { - user.Locale = *data.Locale + if user.Locale != *data.Locale { + user.Locale = *data.Locale + hasUserChanged = true + } } else { - user.Locale = *utils.Cfg.LocalizationSettings.DefaultClientLocale + if user.Locale != *utils.Cfg.LocalizationSettings.DefaultClientLocale { + user.Locale = *utils.Cfg.LocalizationSettings.DefaultClientLocale + hasUserChanged = true + } } var roles string if data.Roles != nil { - roles = *data.Roles + if user.Roles != *data.Roles { + roles = *data.Roles + hasUserRolesChanged = true + } } else if len(user.Roles) == 0 { // Set SYSTEM_USER roles on newly created users by default. - roles = model.ROLE_SYSTEM_USER.Id + if user.Roles != model.ROLE_SYSTEM_USER.Id { + roles = model.ROLE_SYSTEM_USER.Id + hasUserRolesChanged = true + } } user.Roles = roles @@ -483,24 +528,32 @@ func ImportUser(data *UserImportData, dryRun bool) *model.AppError { return err } } else { - if _, err := UpdateUser(user, false); err != nil { - return err + if hasUserChanged { + if _, err := UpdateUser(user, false); err != nil { + return err + } } - if _, err := UpdateUserRoles(user.Id, roles); err != nil { - return err + if hasUserRolesChanged { + if _, err := UpdateUserRoles(user.Id, roles); err != nil { + return err + } } if len(password) > 0 { if err := UpdatePassword(user, password); err != nil { return err } } else { - if res := <-Srv.Store.User().UpdateAuthData(user.Id, authService, authData, user.Email, false); res.Err != nil { - return res.Err + if hasUserAuthDataChanged { + if res := <-Srv.Store.User().UpdateAuthData(user.Id, authService, authData, user.Email, false); res.Err != nil { + return res.Err + } } } if emailVerified { - if err := VerifyUserEmail(user.Id); err != nil { - return err + if hasUserEmailVerifiedChanged { + if err := VerifyUserEmail(user.Id); err != nil { + return err + } } } } @@ -603,13 +656,12 @@ func ImportUserTeams(username string, data *[]UserTeamImportData) *model.AppErro roles = *tdata.Roles } - if _, err := GetTeamMember(team.Id, user.Id); err != nil { - if _, err := joinUserToTeam(team, user); err != nil { - return err - } + if _, err := joinUserToTeam(team, user); err != nil { + return err } - if member, err := GetTeamMember(team.Id, user.Id); err != nil { + var member *model.TeamMember + if member, err = GetTeamMember(team.Id, user.Id); err != nil { return err } else { if member.Roles != roles { @@ -619,7 +671,7 @@ func ImportUserTeams(username string, data *[]UserTeamImportData) *model.AppErro } } - if err := ImportUserChannels(user, team, tdata.Channels); err != nil { + if err := ImportUserChannels(user, team, member, tdata.Channels); err != nil { return err } } @@ -627,7 +679,7 @@ func ImportUserTeams(username string, data *[]UserTeamImportData) *model.AppErro return nil } -func ImportUserChannels(user *model.User, team *model.Team, data *[]UserChannelImportData) *model.AppError { +func ImportUserChannels(user *model.User, team *model.Team, teamMember *model.TeamMember, data *[]UserChannelImportData) *model.AppError { if data == nil { return nil } @@ -649,7 +701,7 @@ func ImportUserChannels(user *model.User, team *model.Team, data *[]UserChannelI var member *model.ChannelMember member, err = GetChannelMember(channel.Id, user.Id) if err != nil { - member, err = addUserToChannel(user, channel) + member, err = addUserToChannel(user, channel, teamMember) if err != nil { return err } diff --git a/app/team.go b/app/team.go index 41e24bf52..acc3966b9 100644 --- a/app/team.go +++ b/app/team.go @@ -286,10 +286,6 @@ func joinUserToTeam(team *model.Team, user *model.User) (bool, *model.AppError) } } - if uua := <-Srv.Store.User().UpdateUpdateAt(user.Id); uua.Err != nil { - return false, uua.Err - } - return false, nil } @@ -300,6 +296,10 @@ func JoinUserToTeam(team *model.Team, user *model.User, userRequestorId string) return nil } + if uua := <-Srv.Store.User().UpdateUpdateAt(user.Id); uua.Err != nil { + return uua.Err + } + channelRole := model.ROLE_CHANNEL_USER.Id if team.Email == user.Email { -- cgit v1.2.3-1-g7c22