From 6232ff3738a2e5e52469dc23840bb96385b5d8ea Mon Sep 17 00:00:00 2001 From: Sergey Shpak Date: Tue, 23 Oct 2018 12:36:24 +0200 Subject: Migrate to idiomatic error handling in app/slackimport.go (#9694) * MM-12610 Migrate to idiomatic error handling in the file in the mattermost-server repo (#9686) * MM-12610 Migrate to idiomatic error handling in the file in the mattermost-server repo (mattermost#9686) * MM-12610 Migrate to idiomatic error handling in the file in the mattermost-server repo (#9686) --- app/slackimport.go | 154 ++++++++++++++++++++++++++++------------------------- 1 file changed, 80 insertions(+), 74 deletions(-) diff --git a/app/slackimport.go b/app/slackimport.go index d3a91815d..ba467d357 100644 --- a/app/slackimport.go +++ b/app/slackimport.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "mime/multipart" + "net/http" "path/filepath" "regexp" "strconv" @@ -17,8 +18,6 @@ import ( "time" "unicode/utf8" - "net/http" - "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" @@ -97,9 +96,8 @@ func SlackConvertChannelName(channelName string, channelId string) string { if isValidChannelNameCharacters(newName) { return newName - } else { - return strings.ToLower(channelId) } + return strings.ToLower(channelId) } func SlackParseChannels(data io.Reader) ([]SlackChannel, error) { @@ -143,13 +141,12 @@ func (a *App) SlackAddUsers(teamId string, slackusers []SlackUser, importerLog * addedUsers := make(map[string]*model.User) // Need the team - var team *model.Team - if result := <-a.Srv.Store.Team().Get(teamId); result.Err != nil { + result := <-a.Srv.Store.Team().Get(teamId) + if result.Err != nil { importerLog.WriteString(utils.T("api.slackimport.slack_import.team_fail")) return addedUsers - } else { - team = result.Data.(*model.Team) } + team := result.Data.(*model.Team) for _, sUser := range slackusers { firstName := sUser.Profile.FirstName @@ -183,25 +180,25 @@ func (a *App) SlackAddUsers(teamId string, slackusers []SlackUser, importerLog * Password: password, } - if mUser := a.OldImportUser(team, &newUser); mUser != nil { - addedUsers[sUser.Id] = mUser - importerLog.WriteString(utils.T("api.slackimport.slack_add_users.email_pwd", map[string]interface{}{"Email": newUser.Email, "Password": password})) - } else { + mUser := a.OldImportUser(team, &newUser) + if mUser == nil { importerLog.WriteString(utils.T("api.slackimport.slack_add_users.unable_import", map[string]interface{}{"Username": sUser.Username})) + continue } + addedUsers[sUser.Id] = mUser + importerLog.WriteString(utils.T("api.slackimport.slack_add_users.email_pwd", map[string]interface{}{"Email": newUser.Email, "Password": password})) } return addedUsers } func (a *App) SlackAddBotUser(teamId string, log *bytes.Buffer) *model.User { - var team *model.Team - if result := <-a.Srv.Store.Team().Get(teamId); result.Err != nil { + result := <-a.Srv.Store.Team().Get(teamId) + if result.Err != nil { log.WriteString(utils.T("api.slackimport.slack_import.team_fail")) return nil - } else { - team = result.Data.(*model.Team) } + team := result.Data.(*model.Team) password := model.NewId() username := "slackimportuser_" + model.NewId() @@ -215,13 +212,14 @@ func (a *App) SlackAddBotUser(teamId string, log *bytes.Buffer) *model.User { Password: password, } - if mUser := a.OldImportUser(team, &botUser); mUser != nil { - log.WriteString(utils.T("api.slackimport.slack_add_bot_user.email_pwd", map[string]interface{}{"Email": botUser.Email, "Password": password})) - return mUser - } else { + mUser := a.OldImportUser(team, &botUser) + if mUser == nil { log.WriteString(utils.T("api.slackimport.slack_add_bot_user.unable_import", map[string]interface{}{"Username": username})) return nil } + + log.WriteString(utils.T("api.slackimport.slack_add_bot_user.email_pwd", map[string]interface{}{"Email": botUser.Email, "Password": password})) + return mUser } func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []SlackPost, users map[string]*model.User, uploads map[string]*zip.File, botUser *model.User) { @@ -231,7 +229,8 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack if sPost.User == "" { mlog.Debug("Slack Import: Unable to import the message as the user field is missing.") continue - } else if users[sPost.User] == nil { + } + if users[sPost.User] == nil { mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User)) continue } @@ -258,10 +257,12 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack if sPost.Comment == nil { mlog.Debug("Slack Import: Unable to import the message as it has no comments.") continue - } else if sPost.Comment.User == "" { + } + if sPost.Comment.User == "" { mlog.Debug("Slack Import: Unable to import the message as the user field is missing.") continue - } else if users[sPost.Comment.User] == nil { + } + if users[sPost.Comment.User] == nil { mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User)) continue } @@ -276,7 +277,8 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack if botUser == nil { mlog.Warn("Slack Import: Unable to import the bot message as the bot user does not exist.") continue - } else if sPost.BotId == "" { + } + if sPost.BotId == "" { mlog.Warn("Slack Import: Unable to import bot message as the BotId field is missing.") continue } @@ -300,7 +302,8 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack if sPost.User == "" { mlog.Debug("Slack Import: Unable to import the message as the user field is missing.") continue - } else if users[sPost.User] == nil { + } + if users[sPost.User] == nil { mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User)) continue } @@ -327,7 +330,8 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack if sPost.User == "" { mlog.Debug("Slack Import: Unable to import the message as the user field is missing.") continue - } else if users[sPost.User] == nil { + } + if users[sPost.User] == nil { mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User)) continue } @@ -342,7 +346,8 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack if sPost.User == "" { mlog.Debug("Slack Import: Unable to import the message as the user field is missing.") continue - } else if users[sPost.User] == nil { + } + if users[sPost.User] == nil { mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User)) continue } @@ -358,7 +363,8 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack if sPost.User == "" { mlog.Debug("Slack Import: Unable to import the message as the user field is missing.") continue - } else if users[sPost.User] == nil { + } + if users[sPost.User] == nil { mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User)) continue } @@ -374,7 +380,8 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack if sPost.User == "" { mlog.Debug("Slack Import: Unable to import the message as the user field is missing.") continue - } else if users[sPost.User] == nil { + } + if users[sPost.User] == nil { mlog.Debug(fmt.Sprintf("Slack Import: Unable to add the message as the Slack user %v does not exist in Mattermost.", sPost.User)) continue } @@ -393,48 +400,47 @@ func (a *App) SlackAddPosts(teamId string, channel *model.Channel, posts []Slack } func (a *App) SlackUploadFile(sPost SlackPost, uploads map[string]*zip.File, teamId string, channelId string, userId string) (*model.FileInfo, bool) { - if sPost.File != nil { - if file, ok := uploads[sPost.File.Id]; ok { - openFile, err := file.Open() - if err != nil { - mlog.Warn(fmt.Sprintf("Slack Import: Unable to open the file %v from the Slack export: %v.", sPost.File.Id, err.Error())) - return nil, false - } - defer openFile.Close() - - timestamp := utils.TimeFromMillis(SlackConvertTimeStamp(sPost.TimeStamp)) - uploadedFile, err := a.OldImportFile(timestamp, openFile, teamId, channelId, userId, filepath.Base(file.Name)) - if err != nil { - mlog.Warn(fmt.Sprintf("Slack Import: An error occurred when uploading file %v: %v.", sPost.File.Id, err.Error())) - return nil, false - } - - return uploadedFile, true - } else { - mlog.Warn(fmt.Sprintf("Slack Import: Unable to import file %v as the file is missing from the Slack export zip file.", sPost.File.Id)) - return nil, false - } - } else { + if sPost.File == nil { mlog.Warn("Slack Import: Unable to attach the file to the post as the latter has no file section present in Slack export.") return nil, false } + file, ok := uploads[sPost.File.Id] + if !ok { + mlog.Warn(fmt.Sprintf("Slack Import: Unable to import file %v as the file is missing from the Slack export zip file.", sPost.File.Id)) + return nil, false + } + openFile, err := file.Open() + if err != nil { + mlog.Warn(fmt.Sprintf("Slack Import: Unable to open the file %v from the Slack export: %v.", sPost.File.Id, err.Error())) + return nil, false + } + defer openFile.Close() + + timestamp := utils.TimeFromMillis(SlackConvertTimeStamp(sPost.TimeStamp)) + uploadedFile, err := a.OldImportFile(timestamp, openFile, teamId, channelId, userId, filepath.Base(file.Name)) + if err != nil { + mlog.Warn(fmt.Sprintf("Slack Import: An error occurred when uploading file %v: %v.", sPost.File.Id, err.Error())) + return nil, false + } + + return uploadedFile, true } func (a *App) deactivateSlackBotUser(user *model.User) { - _, err := a.UpdateActive(user, false) - if err != nil { + if _, err := a.UpdateActive(user, false); err != nil { mlog.Warn("Slack Import: Unable to deactivate the user account used for the bot.") } } func (a *App) addSlackUsersToChannel(members []string, users map[string]*model.User, channel *model.Channel, log *bytes.Buffer) { for _, member := range members { - if user, ok := users[member]; !ok { + user, ok := users[member] + if !ok { log.WriteString(utils.T("api.slackimport.slack_add_channels.failed_to_add_user", map[string]interface{}{"Username": "?"})) - } else { - if _, err := a.AddUserToChannel(user, channel); err != nil { - log.WriteString(utils.T("api.slackimport.slack_add_channels.failed_to_add_user", map[string]interface{}{"Username": user.Username})) - } + continue + } + if _, err := a.AddUserToChannel(user, channel); err != nil { + log.WriteString(utils.T("api.slackimport.slack_add_channels.failed_to_add_user", map[string]interface{}{"Username": user.Username})) } } } @@ -733,32 +739,32 @@ func (a *App) OldImportUser(team *model.Team, user *model.User) *model.User { user.Roles = model.SYSTEM_USER_ROLE_ID - if result := <-a.Srv.Store.User().Save(user); result.Err != nil { + result := <-a.Srv.Store.User().Save(user) + if result.Err != nil { mlog.Error(fmt.Sprintf("Error saving user. err=%v", result.Err)) return nil - } else { - ruser := result.Data.(*model.User) - - if cresult := <-a.Srv.Store.User().VerifyEmail(ruser.Id); cresult.Err != nil { - mlog.Error(fmt.Sprintf("Failed to set email verified err=%v", cresult.Err)) - } + } + ruser := result.Data.(*model.User) - if err := a.JoinUserToTeam(team, user, ""); err != nil { - mlog.Error(fmt.Sprintf("Failed to join team when importing err=%v", err)) - } + if cresult := <-a.Srv.Store.User().VerifyEmail(ruser.Id); cresult.Err != nil { + mlog.Error(fmt.Sprintf("Failed to set email verified err=%v", cresult.Err)) + } - return ruser + if err := a.JoinUserToTeam(team, user, ""); err != nil { + mlog.Error(fmt.Sprintf("Failed to join team when importing err=%v", err)) } + + return ruser } func (a *App) OldImportChannel(channel *model.Channel) *model.Channel { - if result := <-a.Srv.Store.Channel().Save(channel, *a.Config().TeamSettings.MaxChannelsPerTeam); result.Err != nil { + result := <-a.Srv.Store.Channel().Save(channel, *a.Config().TeamSettings.MaxChannelsPerTeam) + if result.Err != nil { return nil - } else { - sc := result.Data.(*model.Channel) - - return sc } + sc := result.Data.(*model.Channel) + + return sc } func (a *App) OldImportFile(timestamp time.Time, file io.Reader, teamId string, channelId string, userId string, fileName string) (*model.FileInfo, error) { -- cgit v1.2.3-1-g7c22