From d937f41233f735b8d4f3b73a0fe87668d66ea7f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Thu, 27 Sep 2018 18:55:07 +0200 Subject: Idiomatic error handling for app/import*.go (#9473) --- app/import.go | 97 ++++++++++++++++------------------- app/import_functions.go | 131 ++++++++++++++++++++++------------------------- app/import_validators.go | 4 +- 3 files changed, 107 insertions(+), 125 deletions(-) diff --git a/app/import.go b/app/import.go index 496c6b7fc..468f9e505 100644 --- a/app/import.go +++ b/app/import.go @@ -42,47 +42,48 @@ func (a *App) BulkImport(fileReader io.Reader, dryRun bool, workers int) (*model var line LineImportData if err := decoder.Decode(&line); err != nil { return model.NewAppError("BulkImport", "app.import.bulk_import.json_decode.error", nil, err.Error(), http.StatusBadRequest), lineNumber - } else { - if lineNumber == 1 { - importDataFileVersion, apperr := processImportDataFileVersionLine(line) - if apperr != nil { - return apperr, lineNumber - } + } - if importDataFileVersion != 1 { - return model.NewAppError("BulkImport", "app.import.bulk_import.unsupported_version.error", nil, "", http.StatusBadRequest), lineNumber - } - } else { - if line.Type != lastLineType { - if lastLineType != "" { - // Changing type. Clear out the worker queue before continuing. - close(linesChan) - wg.Wait() - - // Check no errors occurred while waiting for the queue to empty. - if len(errorsChan) != 0 { - err := <-errorsChan - return err.Error, err.LineNumber - } - } - - // Set up the workers and channel for this type. - lastLineType = line.Type - linesChan = make(chan LineImportWorkerData, workers) - for i := 0; i < workers; i++ { - wg.Add(1) - go a.bulkImportWorker(dryRun, &wg, linesChan, errorsChan) - } - } + if lineNumber == 1 { + importDataFileVersion, apperr := processImportDataFileVersionLine(line) + if apperr != nil { + return apperr, lineNumber + } - select { - case linesChan <- LineImportWorkerData{line, lineNumber}: - case err := <-errorsChan: - close(linesChan) - wg.Wait() + if importDataFileVersion != 1 { + return model.NewAppError("BulkImport", "app.import.bulk_import.unsupported_version.error", nil, "", http.StatusBadRequest), lineNumber + } + continue + } + + if line.Type != lastLineType { + if lastLineType != "" { + // Changing type. Clear out the worker queue before continuing. + close(linesChan) + wg.Wait() + + // Check no errors occurred while waiting for the queue to empty. + if len(errorsChan) != 0 { + err := <-errorsChan return err.Error, err.LineNumber } } + + // Set up the workers and channel for this type. + lastLineType = line.Type + linesChan = make(chan LineImportWorkerData, workers) + for i := 0; i < workers; i++ { + wg.Add(1) + go a.bulkImportWorker(dryRun, &wg, linesChan, errorsChan) + } + } + + select { + case linesChan <- LineImportWorkerData{line, lineNumber}: + case err := <-errorsChan: + close(linesChan) + wg.Wait() + return err.Error, err.LineNumber } } @@ -120,51 +121,43 @@ func (a *App) ImportLine(line LineImportData, dryRun bool) *model.AppError { case line.Type == "scheme": if line.Scheme == nil { return model.NewAppError("BulkImport", "app.import.import_line.null_scheme.error", nil, "", http.StatusBadRequest) - } else { - return a.ImportScheme(line.Scheme, dryRun) } + return a.ImportScheme(line.Scheme, dryRun) case line.Type == "team": if line.Team == nil { return model.NewAppError("BulkImport", "app.import.import_line.null_team.error", nil, "", http.StatusBadRequest) - } else { - return a.ImportTeam(line.Team, dryRun) } + return a.ImportTeam(line.Team, dryRun) case line.Type == "channel": if line.Channel == nil { return model.NewAppError("BulkImport", "app.import.import_line.null_channel.error", nil, "", http.StatusBadRequest) - } else { - return a.ImportChannel(line.Channel, dryRun) } + return a.ImportChannel(line.Channel, dryRun) case line.Type == "user": if line.User == nil { return model.NewAppError("BulkImport", "app.import.import_line.null_user.error", nil, "", http.StatusBadRequest) - } else { - return a.ImportUser(line.User, dryRun) } + return a.ImportUser(line.User, dryRun) case line.Type == "post": if line.Post == nil { return model.NewAppError("BulkImport", "app.import.import_line.null_post.error", nil, "", http.StatusBadRequest) - } else { - return a.ImportPost(line.Post, dryRun) } + return a.ImportPost(line.Post, dryRun) case line.Type == "direct_channel": if line.DirectChannel == nil { return model.NewAppError("BulkImport", "app.import.import_line.null_direct_channel.error", nil, "", http.StatusBadRequest) - } else { - return a.ImportDirectChannel(line.DirectChannel, dryRun) } + return a.ImportDirectChannel(line.DirectChannel, dryRun) case line.Type == "direct_post": if line.DirectPost == nil { return model.NewAppError("BulkImport", "app.import.import_line.null_direct_post.error", nil, "", http.StatusBadRequest) - } else { - return a.ImportDirectPost(line.DirectPost, dryRun) } + return a.ImportDirectPost(line.DirectPost, dryRun) case line.Type == "emoji": if line.Emoji == nil { return model.NewAppError("BulkImport", "app.import.import_line.null_emoji.error", nil, "", http.StatusBadRequest) - } else { - return a.ImportEmoji(line.Emoji, dryRun) } + return a.ImportEmoji(line.Emoji, dryRun) default: return model.NewAppError("BulkImport", "app.import.import_line.unknown_line_type.error", map[string]interface{}{"Type": line.Type}, "", http.StatusBadRequest) } diff --git a/app/import_functions.go b/app/import_functions.go index cd047ba71..bec215eec 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -200,12 +200,11 @@ func (a *App) ImportChannel(data *ChannelImportData, dryRun bool) *model.AppErro return nil } - var team *model.Team - if result := <-a.Srv.Store.Team().GetByName(*data.Team); result.Err != nil { + result := <-a.Srv.Store.Team().GetByName(*data.Team) + if result.Err != nil { return model.NewAppError("BulkImport", "app.import.import_channel.team_not_found.error", map[string]interface{}{"TeamName": *data.Team}, result.Err.Error(), http.StatusBadRequest) - } else { - team = result.Data.(*model.Team) } + team := result.Data.(*model.Team) var channel *model.Channel if result := <-a.Srv.Store.Channel().GetByNameIncludeDeleted(team.Id, *data.Name, true); result.Err == nil { @@ -649,9 +648,12 @@ func (a *App) ImportUserTeams(user *model.User, data *[]UserTeamImportData) *mod a.UpdateTeamMemberSchemeRoles(team.Id, user.Id, isSchemeUser, isSchemeAdmin) } - if defaultChannel, err := a.GetChannelByName(model.DEFAULT_CHANNEL, team.Id, true); err != nil { + defaultChannel, err := a.GetChannelByName(model.DEFAULT_CHANNEL, team.Id, true) + if err != nil { return err - } else if _, err = a.addUserToChannel(user, defaultChannel, member); err != nil { + } + + if _, err = a.addUserToChannel(user, defaultChannel, member); err != nil { return err } @@ -767,12 +769,12 @@ func (a *App) ImportReaction(data *ReactionImportData, post *model.Post, dryRun return err } - var user *model.User - if result := <-a.Srv.Store.User().GetByUsername(*data.User); result.Err != nil { + result := <-a.Srv.Store.User().GetByUsername(*data.User) + if result.Err != nil { return model.NewAppError("BulkImport", "app.import.import_post.user_not_found.error", map[string]interface{}{"Username": data.User}, result.Err.Error(), http.StatusBadRequest) - } else { - user = result.Data.(*model.User) } + user := result.Data.(*model.User) + reaction := &model.Reaction{ UserId: user.Id, PostId: post.Id, @@ -790,20 +792,18 @@ func (a *App) ImportReply(data *ReplyImportData, post *model.Post, teamId string return err } - var user *model.User - if result := <-a.Srv.Store.User().GetByUsername(*data.User); result.Err != nil { + result := <-a.Srv.Store.User().GetByUsername(*data.User) + if result.Err != nil { return model.NewAppError("BulkImport", "app.import.import_post.user_not_found.error", map[string]interface{}{"Username": data.User}, result.Err.Error(), http.StatusBadRequest) - } else { - user = result.Data.(*model.User) } + user := result.Data.(*model.User) // Check if this post already exists. - var replies []*model.Post - if result := <-a.Srv.Store.Post().GetPostsCreatedAt(post.ChannelId, *data.CreateAt); result.Err != nil { + result = <-a.Srv.Store.Post().GetPostsCreatedAt(post.ChannelId, *data.CreateAt) + if result.Err != nil { return result.Err - } else { - replies = result.Data.([]*model.Post) } + replies := result.Data.([]*model.Post) var reply *model.Post for _, r := range replies { @@ -882,34 +882,30 @@ func (a *App) ImportPost(data *PostImportData, dryRun bool) *model.AppError { return nil } - var team *model.Team - if result := <-a.Srv.Store.Team().GetByName(*data.Team); result.Err != nil { + result := <-a.Srv.Store.Team().GetByName(*data.Team) + if result.Err != nil { return model.NewAppError("BulkImport", "app.import.import_post.team_not_found.error", map[string]interface{}{"TeamName": *data.Team}, result.Err.Error(), http.StatusBadRequest) - } else { - team = result.Data.(*model.Team) } + team := result.Data.(*model.Team) - var channel *model.Channel - if result := <-a.Srv.Store.Channel().GetByName(team.Id, *data.Channel, false); result.Err != nil { + result = <-a.Srv.Store.Channel().GetByName(team.Id, *data.Channel, false) + if result.Err != nil { return model.NewAppError("BulkImport", "app.import.import_post.channel_not_found.error", map[string]interface{}{"ChannelName": *data.Channel}, result.Err.Error(), http.StatusBadRequest) - } else { - channel = result.Data.(*model.Channel) } + channel := result.Data.(*model.Channel) - var user *model.User - if result := <-a.Srv.Store.User().GetByUsername(*data.User); result.Err != nil { + result = <-a.Srv.Store.User().GetByUsername(*data.User) + if result.Err != nil { return model.NewAppError("BulkImport", "app.import.import_post.user_not_found.error", map[string]interface{}{"Username": *data.User}, result.Err.Error(), http.StatusBadRequest) - } else { - user = result.Data.(*model.User) } + user := result.Data.(*model.User) // Check if this post already exists. - var posts []*model.Post - if result := <-a.Srv.Store.Post().GetPostsCreatedAt(channel.Id, *data.CreateAt); result.Err != nil { + result = <-a.Srv.Store.Post().GetPostsCreatedAt(channel.Id, *data.CreateAt) + if result.Err != nil { return result.Err - } else { - posts = result.Data.([]*model.Post) } + posts := result.Data.([]*model.Post) var post *model.Post for _, p := range posts { @@ -952,13 +948,11 @@ func (a *App) ImportPost(data *PostImportData, dryRun bool) *model.AppError { var preferences model.Preferences for _, username := range *data.FlaggedBy { - var user *model.User - - if result := <-a.Srv.Store.User().GetByUsername(username); result.Err != nil { + result := <-a.Srv.Store.User().GetByUsername(username) + if result.Err != nil { return model.NewAppError("BulkImport", "app.import.import_post.user_not_found.error", map[string]interface{}{"Username": username}, result.Err.Error(), http.StatusBadRequest) - } else { - user = result.Data.(*model.User) } + user := result.Data.(*model.User) preferences = append(preferences, model.Preference{ UserId: user.Id, @@ -1027,13 +1021,13 @@ func (a *App) ImportDirectChannel(data *DirectChannelImportData, dryRun bool) *m var userIds []string userMap := make(map[string]string) for _, username := range *data.Members { - if result := <-a.Srv.Store.User().GetByUsername(username); result.Err == nil { - user := result.Data.(*model.User) - userIds = append(userIds, user.Id) - userMap[username] = user.Id - } else { + result := <-a.Srv.Store.User().GetByUsername(username) + if result.Err != nil { return model.NewAppError("BulkImport", "app.import.import_direct_channel.member_not_found.error", nil, result.Err.Error(), http.StatusBadRequest) } + user := result.Data.(*model.User) + userIds = append(userIds, user.Id) + userMap[username] = user.Id } var channel *model.Channel @@ -1042,16 +1036,14 @@ func (a *App) ImportDirectChannel(data *DirectChannelImportData, dryRun bool) *m ch, err := a.createDirectChannel(userIds[0], userIds[1]) if err != nil && err.Id != store.CHANNEL_EXISTS_ERROR { return model.NewAppError("BulkImport", "app.import.import_direct_channel.create_direct_channel.error", nil, err.Error(), http.StatusBadRequest) - } else { - channel = ch } + channel = ch } else { ch, err := a.createGroupChannel(userIds, userIds[0]) if err != nil && err.Id != store.CHANNEL_EXISTS_ERROR { return model.NewAppError("BulkImport", "app.import.import_direct_channel.create_group_channel.error", nil, err.Error(), http.StatusBadRequest) - } else { - channel = ch } + channel = ch } var preferences model.Preferences @@ -1103,12 +1095,12 @@ func (a *App) ImportDirectPost(data *DirectPostImportData, dryRun bool) *model.A var userIds []string for _, username := range *data.ChannelMembers { - if result := <-a.Srv.Store.User().GetByUsername(username); result.Err == nil { - user := result.Data.(*model.User) - userIds = append(userIds, user.Id) - } else { + result := <-a.Srv.Store.User().GetByUsername(username) + if result.Err != nil { return model.NewAppError("BulkImport", "app.import.import_direct_post.channel_member_not_found.error", nil, result.Err.Error(), http.StatusBadRequest) } + user := result.Data.(*model.User) + userIds = append(userIds, user.Id) } var channel *model.Channel @@ -1116,32 +1108,28 @@ func (a *App) ImportDirectPost(data *DirectPostImportData, dryRun bool) *model.A ch, err := a.createDirectChannel(userIds[0], userIds[1]) if err != nil && err.Id != store.CHANNEL_EXISTS_ERROR { return model.NewAppError("BulkImport", "app.import.import_direct_post.create_direct_channel.error", nil, err.Error(), http.StatusBadRequest) - } else { - channel = ch } + channel = ch } else { ch, err := a.createGroupChannel(userIds, userIds[0]) if err != nil && err.Id != store.CHANNEL_EXISTS_ERROR { return model.NewAppError("BulkImport", "app.import.import_direct_post.create_group_channel.error", nil, err.Error(), http.StatusBadRequest) - } else { - channel = ch } + channel = ch } - var user *model.User - if result := <-a.Srv.Store.User().GetByUsername(*data.User); result.Err != nil { + result := <-a.Srv.Store.User().GetByUsername(*data.User) + if result.Err != nil { return model.NewAppError("BulkImport", "app.import.import_direct_post.user_not_found.error", map[string]interface{}{"Username": *data.User}, "", http.StatusBadRequest) - } else { - user = result.Data.(*model.User) } + user := result.Data.(*model.User) // Check if this post already exists. - var posts []*model.Post - if result := <-a.Srv.Store.Post().GetPostsCreatedAt(channel.Id, *data.CreateAt); result.Err != nil { + result = <-a.Srv.Store.Post().GetPostsCreatedAt(channel.Id, *data.CreateAt) + if result.Err != nil { return result.Err - } else { - posts = result.Data.([]*model.Post) } + posts := result.Data.([]*model.Post) var post *model.Post for _, p := range posts { @@ -1184,13 +1172,11 @@ func (a *App) ImportDirectPost(data *DirectPostImportData, dryRun bool) *model.A var preferences model.Preferences for _, username := range *data.FlaggedBy { - var user *model.User - - if result := <-a.Srv.Store.User().GetByUsername(username); result.Err != nil { + result := <-a.Srv.Store.User().GetByUsername(username) + if result.Err != nil { return model.NewAppError("BulkImport", "app.import.import_direct_post.user_not_found.error", map[string]interface{}{"Username": username}, "", http.StatusBadRequest) - } else { - user = result.Data.(*model.User) } + user := result.Data.(*model.User) preferences = append(preferences, model.Preference{ UserId: user.Id, @@ -1239,9 +1225,12 @@ func (a *App) ImportEmoji(data *EmojiImportData, dryRun bool) *model.AppError { var emoji *model.Emoji - if result := <-a.Srv.Store.Emoji().GetByName(*data.Name); result.Err != nil && result.Err.StatusCode != http.StatusNotFound { + result := <-a.Srv.Store.Emoji().GetByName(*data.Name) + if result.Err != nil && result.Err.StatusCode != http.StatusNotFound { return result.Err - } else if result.Data != nil { + } + + if result.Data != nil { emoji = result.Data.(*model.Emoji) } diff --git a/app/import_validators.go b/app/import_validators.go index b23ecd8f9..683ebb769 100644 --- a/app/import_validators.go +++ b/app/import_validators.go @@ -283,9 +283,9 @@ func validateUserImportData(data *UserImportData) *model.AppError { if data.Teams != nil { return validateUserTeamsImportData(data.Teams) - } else { - return nil } + + return nil } func validateUserTeamsImportData(data *[]UserTeamImportData) *model.AppError { -- cgit v1.2.3-1-g7c22