From 6964ae61dc2f2c6a9b6c515c1a3b204d2c8e2e5f Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Tue, 8 Aug 2017 11:47:17 -0400 Subject: Added unit tests for getFile headers (#7045) * Added unit tests for getFile headers * Fixed exe type test to run correctly on multiple platforms * Make sure we close the body on all Client4 calls * Changed Response.Response field to Response.Header * Clarified type of Response.Header --- model/client4.go | 202 +++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 153 insertions(+), 49 deletions(-) (limited to 'model') diff --git a/model/client4.go b/model/client4.go index 2daca4dc9..0f7578539 100644 --- a/model/client4.go +++ b/model/client4.go @@ -21,6 +21,7 @@ type Response struct { RequestId string Etag string ServerVersion string + Header http.Header } type Client4 struct { @@ -36,10 +37,20 @@ func NewAPIv4Client(url string) *Client4 { } func BuildErrorResponse(r *http.Response, err *AppError) *Response { - if r == nil { - return &Response{StatusCode: 0, Error: err} + var statusCode int + var header http.Header + if r != nil { + statusCode = r.StatusCode + header = r.Header } else { - return &Response{StatusCode: r.StatusCode, Error: err} + statusCode = 0 + header = make(http.Header, 0) + } + + return &Response{ + StatusCode: statusCode, + Error: err, + Header: header, } } @@ -49,6 +60,7 @@ func BuildResponse(r *http.Response) *Response { RequestId: r.Header.Get(HEADER_REQUEST_ID), Etag: r.Header.Get(HEADER_ETAG_SERVER), ServerVersion: r.Header.Get(HEADER_VERSION_ID), + Header: r.Header, } } @@ -328,12 +340,15 @@ func (c *Client4) DoUploadFile(url string, data []byte, contentType string) (*Fi } if rp, err := c.HttpClient.Do(rq); err != nil || rp == nil { - return nil, &Response{Error: NewAppError(url, "model.client.connecting.app_error", nil, err.Error(), 0)} - } else if rp.StatusCode >= 300 { - return nil, &Response{StatusCode: rp.StatusCode, Error: AppErrorFromJson(rp.Body)} + return nil, BuildErrorResponse(rp, NewAppError(url, "model.client.connecting.app_error", nil, err.Error(), 0)) } else { defer closeBody(rp) - return FileUploadResponseFromJson(rp.Body), BuildResponse(rp) + + if rp.StatusCode >= 300 { + return nil, BuildErrorResponse(rp, AppErrorFromJson(rp.Body)) + } else { + return FileUploadResponseFromJson(rp.Body), BuildResponse(rp) + } } } @@ -347,12 +362,15 @@ func (c *Client4) DoEmojiUploadFile(url string, data []byte, contentType string) } if rp, err := c.HttpClient.Do(rq); err != nil || rp == nil { - return nil, &Response{Error: NewAppError(url, "model.client.connecting.app_error", nil, err.Error(), 0)} - } else if rp.StatusCode >= 300 { - return nil, &Response{StatusCode: rp.StatusCode, Error: AppErrorFromJson(rp.Body)} + return nil, BuildErrorResponse(rp, NewAppError(url, "model.client.connecting.app_error", nil, err.Error(), 0)) } else { defer closeBody(rp) - return EmojiFromJson(rp.Body), BuildResponse(rp) + + if rp.StatusCode >= 300 { + return nil, BuildErrorResponse(rp, AppErrorFromJson(rp.Body)) + } else { + return EmojiFromJson(rp.Body), BuildResponse(rp) + } } } @@ -366,12 +384,15 @@ func (c *Client4) DoUploadImportTeam(url string, data []byte, contentType string } if rp, err := c.HttpClient.Do(rq); err != nil || rp == nil { - return nil, &Response{Error: NewAppError(url, "model.client.connecting.app_error", nil, err.Error(), 0)} - } else if rp.StatusCode >= 300 { - return nil, &Response{StatusCode: rp.StatusCode, Error: AppErrorFromJson(rp.Body)} + return nil, BuildErrorResponse(rp, NewAppError(url, "model.client.connecting.app_error", nil, err.Error(), 0)) } else { defer closeBody(rp) - return MapFromJson(rp.Body), BuildResponse(rp) + + if rp.StatusCode >= 300 { + return nil, BuildErrorResponse(rp, AppErrorFromJson(rp.Body)) + } else { + return MapFromJson(rp.Body), BuildResponse(rp) + } } } @@ -584,10 +605,14 @@ func (c *Client4) AutocompleteUsers(username string, etag string) (*UserAutocomp func (c *Client4) GetProfileImage(userId, etag string) ([]byte, *Response) { if r, err := c.DoApiGet(c.GetUserRoute(userId)+"/image", etag); err != nil { return nil, BuildErrorResponse(r, err) - } else if data, err := ioutil.ReadAll(r.Body); err != nil { - return nil, &Response{StatusCode: r.StatusCode, Error: NewAppError("GetProfileImage", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)} } else { - return data, BuildResponse(r) + defer closeBody(r) + + if data, err := ioutil.ReadAll(r.Body); err != nil { + return nil, BuildErrorResponse(r, NewAppError("GetProfileImage", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)) + } else { + return data, BuildResponse(r) + } } } @@ -953,11 +978,14 @@ func (c *Client4) SetProfileImage(userId string, data []byte) (bool, *Response) if rp, err := c.HttpClient.Do(rq); err != nil || rp == nil { // set to http.StatusForbidden(403) return false, &Response{StatusCode: http.StatusForbidden, Error: NewAppError(c.GetUserRoute(userId)+"/image", "model.client.connecting.app_error", nil, err.Error(), 403)} - } else if rp.StatusCode >= 300 { - return false, &Response{StatusCode: rp.StatusCode, Error: AppErrorFromJson(rp.Body)} } else { defer closeBody(rp) - return CheckStatusOK(rp), BuildResponse(rp) + + if rp.StatusCode >= 300 { + return false, BuildErrorResponse(rp, AppErrorFromJson(rp.Body)) + } else { + return CheckStatusOK(rp), BuildResponse(rp) + } } } @@ -1805,10 +1833,29 @@ func (c *Client4) UploadFile(data []byte, channelId string, filename string) (*F func (c *Client4) GetFile(fileId string) ([]byte, *Response) { if r, err := c.DoApiGet(c.GetFileRoute(fileId), ""); err != nil { return nil, BuildErrorResponse(r, err) - } else if data, err := ioutil.ReadAll(r.Body); err != nil { - return nil, &Response{StatusCode: r.StatusCode, Error: NewAppError("GetFile", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)} } else { - return data, BuildResponse(r) + defer closeBody(r) + + if data, err := ioutil.ReadAll(r.Body); err != nil { + return nil, BuildErrorResponse(r, NewAppError("GetFile", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)) + } else { + return data, BuildResponse(r) + } + } +} + +// DownloadFile gets the bytes for a file by id, optionally adding headers to force the browser to download it +func (c *Client4) DownloadFile(fileId string, download bool) ([]byte, *Response) { + if r, err := c.DoApiGet(c.GetFileRoute(fileId)+fmt.Sprintf("?download=%v", download), ""); err != nil { + return nil, BuildErrorResponse(r, err) + } else { + defer closeBody(r) + + if data, err := ioutil.ReadAll(r.Body); err != nil { + return nil, BuildErrorResponse(r, NewAppError("DownloadFile", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)) + } else { + return data, BuildResponse(r) + } } } @@ -1816,10 +1863,29 @@ func (c *Client4) GetFile(fileId string) ([]byte, *Response) { func (c *Client4) GetFileThumbnail(fileId string) ([]byte, *Response) { if r, err := c.DoApiGet(c.GetFileRoute(fileId)+"/thumbnail", ""); err != nil { return nil, BuildErrorResponse(r, err) - } else if data, err := ioutil.ReadAll(r.Body); err != nil { - return nil, &Response{StatusCode: r.StatusCode, Error: NewAppError("GetFileThumbnail", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)} } else { - return data, BuildResponse(r) + defer closeBody(r) + + if data, err := ioutil.ReadAll(r.Body); err != nil { + return nil, BuildErrorResponse(r, NewAppError("GetFileThumbnail", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)) + } else { + return data, BuildResponse(r) + } + } +} + +// DownloadFileThumbnail gets the bytes for a file by id, optionally adding headers to force the browser to download it. +func (c *Client4) DownloadFileThumbnail(fileId string, download bool) ([]byte, *Response) { + if r, err := c.DoApiGet(c.GetFileRoute(fileId)+fmt.Sprintf("/thumbnail?download=%v", download), ""); err != nil { + return nil, BuildErrorResponse(r, err) + } else { + defer closeBody(r) + + if data, err := ioutil.ReadAll(r.Body); err != nil { + return nil, BuildErrorResponse(r, NewAppError("DownloadFileThumbnail", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)) + } else { + return data, BuildResponse(r) + } } } @@ -1828,6 +1894,8 @@ func (c *Client4) GetFileLink(fileId string) (string, *Response) { if r, err := c.DoApiGet(c.GetFileRoute(fileId)+"/link", ""); err != nil { return "", BuildErrorResponse(r, err) } else { + defer closeBody(r) + return MapFromJson(r.Body)["link"], BuildResponse(r) } } @@ -1836,10 +1904,29 @@ func (c *Client4) GetFileLink(fileId string) (string, *Response) { func (c *Client4) GetFilePreview(fileId string) ([]byte, *Response) { if r, err := c.DoApiGet(c.GetFileRoute(fileId)+"/preview", ""); err != nil { return nil, BuildErrorResponse(r, err) - } else if data, err := ioutil.ReadAll(r.Body); err != nil { - return nil, &Response{StatusCode: r.StatusCode, Error: NewAppError("GetFilePreview", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)} } else { - return data, BuildResponse(r) + defer closeBody(r) + + if data, err := ioutil.ReadAll(r.Body); err != nil { + return nil, BuildErrorResponse(r, NewAppError("GetFilePreview", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)) + } else { + return data, BuildResponse(r) + } + } +} + +// DownloadFilePreview gets the bytes for a file by id. +func (c *Client4) DownloadFilePreview(fileId string, download bool) ([]byte, *Response) { + if r, err := c.DoApiGet(c.GetFileRoute(fileId)+fmt.Sprintf("/preview?download=%v", download), ""); err != nil { + return nil, BuildErrorResponse(r, err) + } else { + defer closeBody(r) + + if data, err := ioutil.ReadAll(r.Body); err != nil { + return nil, BuildErrorResponse(r, NewAppError("DownloadFilePreview", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)) + } else { + return data, BuildResponse(r) + } } } @@ -1985,11 +2072,14 @@ func (c *Client4) UploadLicenseFile(data []byte) (bool, *Response) { if rp, err := c.HttpClient.Do(rq); err != nil || rp == nil { return false, &Response{StatusCode: http.StatusForbidden, Error: NewAppError(c.GetLicenseRoute(), "model.client.connecting.app_error", nil, err.Error(), http.StatusForbidden)} - } else if rp.StatusCode >= 300 { - return false, &Response{StatusCode: rp.StatusCode, Error: AppErrorFromJson(rp.Body)} } else { defer closeBody(rp) - return CheckStatusOK(rp), BuildResponse(rp) + + if rp.StatusCode >= 300 { + return false, BuildErrorResponse(rp, AppErrorFromJson(rp.Body)) + } else { + return CheckStatusOK(rp), BuildResponse(rp) + } } } @@ -2370,15 +2460,16 @@ func (c *Client4) DownloadComplianceReport(reportId string) ([]byte, *Response) if rp, err := c.HttpClient.Do(rq); err != nil || rp == nil { return nil, &Response{Error: NewAppError("DownloadComplianceReport", "model.client.connecting.app_error", nil, err.Error(), http.StatusBadRequest)} - } else if rp.StatusCode >= 300 { - defer rp.Body.Close() - return nil, &Response{StatusCode: rp.StatusCode, Error: AppErrorFromJson(rp.Body)} - } else if data, err := ioutil.ReadAll(rp.Body); err != nil { - defer closeBody(rp) - return nil, &Response{StatusCode: rp.StatusCode, Error: NewAppError("DownloadComplianceReport", "model.client.read_file.app_error", nil, err.Error(), rp.StatusCode)} } else { defer closeBody(rp) - return data, BuildResponse(rp) + + if rp.StatusCode >= 300 { + return nil, BuildErrorResponse(rp, AppErrorFromJson(rp.Body)) + } else if data, err := ioutil.ReadAll(rp.Body); err != nil { + return nil, BuildErrorResponse(rp, NewAppError("DownloadComplianceReport", "model.client.read_file.app_error", nil, err.Error(), rp.StatusCode)) + } else { + return data, BuildResponse(rp) + } } } @@ -2436,10 +2527,16 @@ func (c *Client4) GetAudits(page int, perPage int, etag string) (Audits, *Respon func (c *Client4) GetBrandImage() ([]byte, *Response) { if r, err := c.DoApiGet(c.GetBrandRoute()+"/image", ""); err != nil { return nil, BuildErrorResponse(r, err) - } else if data, err := ioutil.ReadAll(r.Body); err != nil { - return nil, &Response{StatusCode: r.StatusCode, Error: NewAppError("GetBrandImage", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)} } else { - return data, BuildResponse(r) + defer closeBody(r) + + if r.StatusCode >= 300 { + return nil, BuildErrorResponse(r, AppErrorFromJson(r.Body)) + } else if data, err := ioutil.ReadAll(r.Body); err != nil { + return nil, BuildErrorResponse(r, NewAppError("GetBrandImage", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)) + } else { + return data, BuildResponse(r) + } } } @@ -2468,11 +2565,14 @@ func (c *Client4) UploadBrandImage(data []byte) (bool, *Response) { if rp, err := c.HttpClient.Do(rq); err != nil || rp == nil { return false, &Response{StatusCode: http.StatusForbidden, Error: NewAppError(c.GetBrandRoute()+"/image", "model.client.connecting.app_error", nil, err.Error(), http.StatusForbidden)} - } else if rp.StatusCode >= 300 { - return false, &Response{StatusCode: rp.StatusCode, Error: AppErrorFromJson(rp.Body)} } else { defer closeBody(rp) - return CheckStatusOK(rp), BuildResponse(rp) + + if rp.StatusCode >= 300 { + return false, BuildErrorResponse(rp, AppErrorFromJson(rp.Body)) + } else { + return CheckStatusOK(rp), BuildResponse(rp) + } } } @@ -2800,10 +2900,14 @@ func (c *Client4) GetEmoji(emojiId string) (*Emoji, *Response) { func (c *Client4) GetEmojiImage(emojiId string) ([]byte, *Response) { if r, err := c.DoApiGet(c.GetEmojiRoute(emojiId)+"/image", ""); err != nil { return nil, BuildErrorResponse(r, err) - } else if data, err := ioutil.ReadAll(r.Body); err != nil { - return nil, &Response{StatusCode: r.StatusCode, Error: NewAppError("GetEmojiImage", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)} } else { - return data, BuildResponse(r) + defer closeBody(r) + + if data, err := ioutil.ReadAll(r.Body); err != nil { + return nil, BuildErrorResponse(r, NewAppError("GetEmojiImage", "model.client.read_file.app_error", nil, err.Error(), r.StatusCode)) + } else { + return data, BuildResponse(r) + } } } -- cgit v1.2.3-1-g7c22