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

1 files changed, 153 insertions, 49 deletions

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)
+		}
 	}
 }