From 050c9de0f017186824f2f3c89c3efadc6e4b2033 Mon Sep 17 00:00:00 2001 From: Hanzei <16541325+hanzei@users.noreply.github.com> Date: Wed, 17 Oct 2018 14:24:31 +0200 Subject: Migrate to idiomatic error handling in app/command.go (#9675) --- app/command.go | 250 ++++++++++++++++++++++++++++++--------------------------- 1 file changed, 130 insertions(+), 120 deletions(-) diff --git a/app/command.go b/app/command.go index 8f824982d..5c7ff6ecf 100644 --- a/app/command.go +++ b/app/command.go @@ -52,7 +52,9 @@ func (a *App) CreateCommandPost(post *model.Post, teamId string, response *model if response.ResponseType == model.COMMAND_RESPONSE_TYPE_IN_CHANNEL { return a.CreatePostMissingChannel(post, true) - } else if (response.ResponseType == "" || response.ResponseType == model.COMMAND_RESPONSE_TYPE_EPHEMERAL) && (response.Text != "" || response.Attachments != nil) { + } + + if (response.ResponseType == "" || response.ResponseType == model.COMMAND_RESPONSE_TYPE_EPHEMERAL) && (response.Text != "" || response.Attachments != nil) { post.ParentId = "" a.SendEphemeralPost(post.UserId, post) } @@ -83,16 +85,17 @@ func (a *App) ListAutocompleteCommands(teamId string, T goi18n.TranslateFunc) ([ } if *a.Config().ServiceSettings.EnableCommands { - if result := <-a.Srv.Store.Command().GetByTeam(teamId); result.Err != nil { + result := <-a.Srv.Store.Command().GetByTeam(teamId) + if result.Err != nil { return nil, result.Err - } else { - teamCmds := result.Data.([]*model.Command) - for _, cmd := range teamCmds { - if cmd.AutoComplete && !seen[cmd.Id] { - cmd.Sanitize() - seen[cmd.Trigger] = true - commands = append(commands, cmd) - } + } + + teamCmds := result.Data.([]*model.Command) + for _, cmd := range teamCmds { + if cmd.AutoComplete && !seen[cmd.Id] { + cmd.Sanitize() + seen[cmd.Trigger] = true + commands = append(commands, cmd) } } } @@ -105,11 +108,12 @@ func (a *App) ListTeamCommands(teamId string) ([]*model.Command, *model.AppError return nil, model.NewAppError("ListTeamCommands", "api.command.disabled.app_error", nil, "", http.StatusNotImplemented) } - if result := <-a.Srv.Store.Command().GetByTeam(teamId); result.Err != nil { + result := <-a.Srv.Store.Command().GetByTeam(teamId) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.([]*model.Command), nil } + + return result.Data.([]*model.Command), nil } func (a *App) ListAllCommands(teamId string, T goi18n.TranslateFunc) ([]*model.Command, *model.AppError) { @@ -134,16 +138,16 @@ func (a *App) ListAllCommands(teamId string, T goi18n.TranslateFunc) ([]*model.C } if *a.Config().ServiceSettings.EnableCommands { - if result := <-a.Srv.Store.Command().GetByTeam(teamId); result.Err != nil { + result := <-a.Srv.Store.Command().GetByTeam(teamId) + if result.Err != nil { return nil, result.Err - } else { - teamCmds := result.Data.([]*model.Command) - for _, cmd := range teamCmds { - if !seen[cmd.Trigger] { - cmd.Sanitize() - seen[cmd.Trigger] = true - commands = append(commands, cmd) - } + } + teamCmds := result.Data.([]*model.Command) + for _, cmd := range teamCmds { + if !seen[cmd.Trigger] { + cmd.Sanitize() + seen[cmd.Trigger] = true + commands = append(commands, cmd) } } } @@ -165,9 +169,11 @@ func (a *App) ExecuteCommand(args *model.CommandArgs) (*model.CommandResponse, * } } - if cmd, response, err := a.ExecutePluginCommand(args); err != nil { - return nil, err - } else if cmd != nil { + cmd, response, appErr := a.ExecutePluginCommand(args) + if appErr != nil { + return nil, appErr + } + if cmd != nil { return a.HandleCommandResponse(cmd, args, response, true) } @@ -179,93 +185,91 @@ func (a *App) ExecuteCommand(args *model.CommandArgs) (*model.CommandResponse, * teamChan := a.Srv.Store.Team().Get(args.TeamId) userChan := a.Srv.Store.User().Get(args.UserId) - if result := <-a.Srv.Store.Command().GetByTeam(args.TeamId); result.Err != nil { + result := <-a.Srv.Store.Command().GetByTeam(args.TeamId) + if result.Err != nil { return nil, result.Err - } else { + } - var team *model.Team - if tr := <-teamChan; tr.Err != nil { - return nil, tr.Err - } else { - team = tr.Data.(*model.Team) - } + tr := <-teamChan + if tr.Err != nil { + return nil, tr.Err + } + team := tr.Data.(*model.Team) - var user *model.User - if ur := <-userChan; ur.Err != nil { - return nil, ur.Err - } else { - user = ur.Data.(*model.User) - } + ur := <-userChan + if ur.Err != nil { + return nil, ur.Err + } + user := ur.Data.(*model.User) - var channel *model.Channel - if cr := <-chanChan; cr.Err != nil { - return nil, cr.Err - } else { - channel = cr.Data.(*model.Channel) - } + cr := <-chanChan + if cr.Err != nil { + return nil, cr.Err + } + channel := cr.Data.(*model.Channel) - teamCmds := result.Data.([]*model.Command) - for _, cmd := range teamCmds { - if trigger == cmd.Trigger { - mlog.Debug(fmt.Sprintf(utils.T("api.command.execute_command.debug"), trigger, args.UserId)) + teamCmds := result.Data.([]*model.Command) + for _, cmd := range teamCmds { + if trigger == cmd.Trigger { + mlog.Debug(fmt.Sprintf(utils.T("api.command.execute_command.debug"), trigger, args.UserId)) - p := url.Values{} - p.Set("token", cmd.Token) + p := url.Values{} + p.Set("token", cmd.Token) - p.Set("team_id", cmd.TeamId) - p.Set("team_domain", team.Name) + p.Set("team_id", cmd.TeamId) + p.Set("team_domain", team.Name) - p.Set("channel_id", args.ChannelId) - p.Set("channel_name", channel.Name) + p.Set("channel_id", args.ChannelId) + p.Set("channel_name", channel.Name) - p.Set("user_id", args.UserId) - p.Set("user_name", user.Username) + p.Set("user_id", args.UserId) + p.Set("user_name", user.Username) - p.Set("command", "/"+trigger) - p.Set("text", message) + p.Set("command", "/"+trigger) + p.Set("text", message) - if hook, err := a.CreateCommandWebhook(cmd.Id, args); err != nil { - return nil, model.NewAppError("command", "api.command.execute_command.failed.app_error", map[string]interface{}{"Trigger": trigger}, err.Error(), http.StatusInternalServerError) - } else { - p.Set("response_url", args.SiteURL+"/hooks/commands/"+hook.Id) - } + hook, appErr := a.CreateCommandWebhook(cmd.Id, args) + if appErr != nil { + return nil, model.NewAppError("command", "api.command.execute_command.failed.app_error", map[string]interface{}{"Trigger": trigger}, appErr.Error(), http.StatusInternalServerError) + } + p.Set("response_url", args.SiteURL+"/hooks/commands/"+hook.Id) - var req *http.Request - if cmd.Method == model.COMMAND_METHOD_GET { - req, _ = http.NewRequest(http.MethodGet, cmd.URL, nil) + var req *http.Request + if cmd.Method == model.COMMAND_METHOD_GET { + req, _ = http.NewRequest(http.MethodGet, cmd.URL, nil) - if req.URL.RawQuery != "" { - req.URL.RawQuery += "&" - } - req.URL.RawQuery += p.Encode() - } else { - req, _ = http.NewRequest(http.MethodPost, cmd.URL, strings.NewReader(p.Encode())) + if req.URL.RawQuery != "" { + req.URL.RawQuery += "&" } + req.URL.RawQuery += p.Encode() + } else { + req, _ = http.NewRequest(http.MethodPost, cmd.URL, strings.NewReader(p.Encode())) + } - req.Header.Set("Accept", "application/json") - req.Header.Set("Authorization", "Token "+cmd.Token) - if cmd.Method == model.COMMAND_METHOD_POST { - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - } + req.Header.Set("Accept", "application/json") + req.Header.Set("Authorization", "Token "+cmd.Token) + if cmd.Method == model.COMMAND_METHOD_POST { + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + } - if resp, err := a.HTTPService.MakeClient(false).Do(req); err != nil { - return nil, model.NewAppError("command", "api.command.execute_command.failed.app_error", map[string]interface{}{"Trigger": trigger}, err.Error(), http.StatusInternalServerError) - } else { - if resp.StatusCode == http.StatusOK { - if response, err := model.CommandResponseFromHTTPBody(resp.Header.Get("Content-Type"), resp.Body); err != nil { - return nil, model.NewAppError("command", "api.command.execute_command.failed.app_error", map[string]interface{}{"Trigger": trigger}, err.Error(), http.StatusInternalServerError) - } else if response == nil { - return nil, model.NewAppError("command", "api.command.execute_command.failed_empty.app_error", map[string]interface{}{"Trigger": trigger}, "", http.StatusInternalServerError) - } else { - return a.HandleCommandResponse(cmd, args, response, false) - } - } else { - defer resp.Body.Close() - body, _ := ioutil.ReadAll(resp.Body) - return nil, model.NewAppError("command", "api.command.execute_command.failed_resp.app_error", map[string]interface{}{"Trigger": trigger, "Status": resp.Status}, string(body), http.StatusInternalServerError) - } - } + resp, err := a.HTTPService.MakeClient(false).Do(req) + if err != nil { + return nil, model.NewAppError("command", "api.command.execute_command.failed.app_error", map[string]interface{}{"Trigger": trigger}, err.Error(), http.StatusInternalServerError) + } + if resp.StatusCode != http.StatusOK { + defer resp.Body.Close() + body, _ := ioutil.ReadAll(resp.Body) + return nil, model.NewAppError("command", "api.command.execute_command.failed_resp.app_error", map[string]interface{}{"Trigger": trigger, "Status": resp.Status}, string(body), http.StatusInternalServerError) + } + + response, err := model.CommandResponseFromHTTPBody(resp.Header.Get("Content-Type"), resp.Body) + if err != nil { + return nil, model.NewAppError("command", "api.command.execute_command.failed.app_error", map[string]interface{}{"Trigger": trigger}, err.Error(), http.StatusInternalServerError) + } + if response == nil { + return nil, model.NewAppError("command", "api.command.execute_command.failed_empty.app_error", map[string]interface{}{"Trigger": trigger}, "", http.StatusInternalServerError) } + return a.HandleCommandResponse(cmd, args, response, false) } } @@ -327,28 +331,31 @@ func (a *App) CreateCommand(cmd *model.Command) (*model.Command, *model.AppError cmd.Trigger = strings.ToLower(cmd.Trigger) - if result := <-a.Srv.Store.Command().GetByTeam(cmd.TeamId); result.Err != nil { + result := <-a.Srv.Store.Command().GetByTeam(cmd.TeamId) + if result.Err != nil { return nil, result.Err - } else { - teamCmds := result.Data.([]*model.Command) - for _, existingCommand := range teamCmds { - if cmd.Trigger == existingCommand.Trigger { - return nil, model.NewAppError("CreateCommand", "api.command.duplicate_trigger.app_error", nil, "", http.StatusBadRequest) - } + } + + teamCmds := result.Data.([]*model.Command) + for _, existingCommand := range teamCmds { + if cmd.Trigger == existingCommand.Trigger { + return nil, model.NewAppError("CreateCommand", "api.command.duplicate_trigger.app_error", nil, "", http.StatusBadRequest) } - for _, builtInProvider := range commandProviders { - builtInCommand := builtInProvider.GetCommand(a, utils.T) - if builtInCommand != nil && cmd.Trigger == builtInCommand.Trigger { - return nil, model.NewAppError("CreateCommand", "api.command.duplicate_trigger.app_error", nil, "", http.StatusBadRequest) - } + } + + for _, builtInProvider := range commandProviders { + builtInCommand := builtInProvider.GetCommand(a, utils.T) + if builtInCommand != nil && cmd.Trigger == builtInCommand.Trigger { + return nil, model.NewAppError("CreateCommand", "api.command.duplicate_trigger.app_error", nil, "", http.StatusBadRequest) } } - if result := <-a.Srv.Store.Command().Save(cmd); result.Err != nil { + result = <-a.Srv.Store.Command().Save(cmd) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.(*model.Command), nil } + + return result.Data.(*model.Command), nil } func (a *App) GetCommand(commandId string) (*model.Command, *model.AppError) { @@ -356,12 +363,13 @@ func (a *App) GetCommand(commandId string) (*model.Command, *model.AppError) { return nil, model.NewAppError("GetCommand", "api.command.disabled.app_error", nil, "", http.StatusNotImplemented) } - if result := <-a.Srv.Store.Command().Get(commandId); result.Err != nil { + result := <-a.Srv.Store.Command().Get(commandId) + if result.Err != nil { result.Err.StatusCode = http.StatusNotFound return nil, result.Err - } else { - return result.Data.(*model.Command), nil } + + return result.Data.(*model.Command), nil } func (a *App) UpdateCommand(oldCmd, updatedCmd *model.Command) (*model.Command, *model.AppError) { @@ -378,17 +386,18 @@ func (a *App) UpdateCommand(oldCmd, updatedCmd *model.Command) (*model.Command, updatedCmd.CreatorId = oldCmd.CreatorId updatedCmd.TeamId = oldCmd.TeamId - if result := <-a.Srv.Store.Command().Update(updatedCmd); result.Err != nil { + result := <-a.Srv.Store.Command().Update(updatedCmd) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.(*model.Command), nil } + return result.Data.(*model.Command), nil } func (a *App) MoveCommand(team *model.Team, command *model.Command) *model.AppError { command.TeamId = team.Id - if result := <-a.Srv.Store.Command().Update(command); result.Err != nil { + result := <-a.Srv.Store.Command().Update(command) + if result.Err != nil { return result.Err } @@ -402,11 +411,12 @@ func (a *App) RegenCommandToken(cmd *model.Command) (*model.Command, *model.AppE cmd.Token = model.NewId() - if result := <-a.Srv.Store.Command().Update(cmd); result.Err != nil { + result := <-a.Srv.Store.Command().Update(cmd) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.(*model.Command), nil } + + return result.Data.(*model.Command), nil } func (a *App) DeleteCommand(commandId string) *model.AppError { -- cgit v1.2.3-1-g7c22