From 8563f13262fed664b29d3fda8f6a26e2e7d70085 Mon Sep 17 00:00:00 2001 From: Sarthak Kothari Date: Mon, 21 Nov 2022 00:13:27 -0800 Subject: [PATCH 1/5] Added new FileUploadV2 function to avoid server side file timeouts --- files.go | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++ files_test.go | 62 +++++++++++++++++++++ misc.go | 3 ++ 3 files changed, 210 insertions(+) diff --git a/files.go b/files.go index e7cfe1fee..5c2957f53 100644 --- a/files.go +++ b/files.go @@ -2,6 +2,7 @@ package slack import ( "context" + "encoding/json" "fmt" "io" "net/url" @@ -145,6 +146,44 @@ type ListFilesParameters struct { Cursor string } +type FileUploadV2Parameters struct { + File string + FileSize int + Content string + Reader io.Reader + Filename string + Title string + InitialComment string + Channel string + ThreadTimestamp string + AltTxt string + SnippetText string +} + +type getUploadURLExternalResponse struct { + UploadURL string `json:"upload_url"` + FileID string `json:"file_id"` + SlackResponse +} + +type uploadToExternalParams struct { + UploadURL string + Reader io.Reader + File string + Content string + Filename string +} + +type FileSummary struct { + ID string `json:"id"` + Title string `json:"title"` +} + +type completeUploadExternalResponse struct { + SlackResponse + Files []FileSummary `json:"files"` +} + type fileResponseFull struct { File `json:"file"` Paging `json:"paging"` @@ -416,3 +455,109 @@ func (api *Client) ShareFilePublicURLContext(ctx context.Context, fileID string) } return &response.File, response.Comments, &response.Paging, nil } + +func (api *Client) getUploadURLExternal(ctx context.Context, fileSize int, fileName, altText, snippetText string) (*getUploadURLExternalResponse, error) { + values := url.Values{ + "token": {api.token}, + } + values.Add("filename", fileName) + values.Add("length", strconv.Itoa(fileSize)) + if altText != "" { + values.Add("initial_comment", altText) + } + if snippetText != "" { + values.Add("thread_ts", snippetText) + } + response := &getUploadURLExternalResponse{} + err := api.postMethod(ctx, "files.getUploadURLExternal", values, response) + if err != nil { + return nil, err + } + + return response, response.Err() +} + +func (api *Client) uploadToURL(ctx context.Context, params uploadToExternalParams) (err error) { + values := url.Values{} + if params.Content != "" { + values.Add("content", params.Content) + values.Add("token", api.token) + err = postForm(ctx, api.httpclient, params.UploadURL, values, nil, api) + } else if params.File != "" { + err = postLocalWithMultipartResponse(ctx, api.httpclient, params.UploadURL, params.File, "file", api.token, values, nil, api) + } else if params.Reader != nil { + err = postWithMultipartResponse(ctx, api.httpclient, params.UploadURL, params.Filename, "file", api.token, values, params.Reader, nil, api) + } + _ = 1 + return err +} + +func (api *Client) completeUploadExternal(ctx context.Context, fileID string, params FileUploadV2Parameters) (file *FileSummary, err error) { + values := url.Values{ + "token": {api.token}, + } + + request := []FileSummary{{ID: fileID, Title: params.Title}} + requestBytes, err := json.Marshal(request) + if err != nil { + return nil, err + } + values.Add("files", string(requestBytes)) + values.Add("channel_id", params.Channel) + + if params.InitialComment != "" { + values.Add("initial_comment", params.InitialComment) + } + if params.ThreadTimestamp != "" { + values.Add("thread_ts", params.ThreadTimestamp) + } + response := &completeUploadExternalResponse{} + err = api.postMethod(ctx, "files.completeUploadExternal", values, response) + if err != nil { + return nil, err + } + if response.Err() != nil { + return nil, response.Err() + } + if len(response.Files) != 1 { + return nil, fmt.Errorf("files.completeUploadExternal: something went wrong; recieved %d files instead of 1", len(response.Files)) + } + return &response.Files[0], nil +} + +func (api *Client) UploadFileV2(params FileUploadV2Parameters) (*FileSummary, error) { + return api.UploadFileV2Context(context.Background(), params) +} + +func (api *Client) UploadFileV2Context(ctx context.Context, params FileUploadV2Parameters) (file *FileSummary, err error) { + if params.Filename == "" { + return nil, fmt.Errorf("file.upload.v2: filename cannot be empty") + } + if params.FileSize == 0 { + return nil, fmt.Errorf("file.upload.v2: file size cannot be 0") + } + if params.Channel == "" { + return nil, fmt.Errorf("file.upload.v2: channel cannot be empty") + } + u, err := api.getUploadURLExternal(ctx, params.FileSize, params.Filename, params.AltTxt, params.SnippetText) + if err != nil { + return nil, err + } + + err = api.uploadToURL(ctx, uploadToExternalParams{ + UploadURL: u.UploadURL, + Reader: params.Reader, + File: params.File, + Content: params.Content, + Filename: params.Filename, + }) + if err != nil { + return nil, err + } + + file, err = api.completeUploadExternal(ctx, u.FileID, params) + if err != nil { + return nil, err + } + return file, nil +} diff --git a/files_test.go b/files_test.go index 5361c06d3..3a962fe39 100644 --- a/files_test.go +++ b/files_test.go @@ -211,3 +211,65 @@ func TestUploadFileWithoutFilename(t *testing.T) { t.Errorf("Error message should mention empty FileUploadParameters.Filename") } } + +func uploadURLHandler(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Content-Type", "application/json") + response, _ := json.Marshal(getUploadURLExternalResponse{ + FileID: "RandomID", + UploadURL: "http://" + serverAddr + "/abc", + SlackResponse: SlackResponse{Ok: true}}) + rw.Write(response) +} + +func urlFileUploadHandler(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Content-Type", "text") + rw.Write([]byte("Ok: 200, file uploaded")) +} + +func completeURLUpload(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Content-Type", "application/json") + response, _ := json.Marshal(completeUploadExternalResponse{ + Files: []FileSummary{ + { + ID: "RandomID", + Title: "", + }, + }, + SlackResponse: SlackResponse{Ok: true}}) + rw.Write(response) +} + +func TestUploadFileV2(t *testing.T) { + http.HandleFunc("/files.getUploadURLExternal", uploadURLHandler) + http.HandleFunc("/abc", urlFileUploadHandler) + http.HandleFunc("/files.completeUploadExternal", completeURLUpload) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + + params := FileUploadV2Parameters{ + Filename: "test.txt", Content: "test content", FileSize: 10, + Channel: "CXXXXXXXX", + } + if _, err := api.UploadFileV2(params); err != nil { + t.Errorf("Unexpected error: %s", err) + } + + reader := bytes.NewBufferString("test reader") + params = FileUploadV2Parameters{ + Filename: "test.txt", + Reader: reader, + FileSize: 10, + Channel: "CXXXXXXXX"} + if _, err := api.UploadFileV2(params); err != nil { + t.Errorf("Unexpected error: %s", err) + } + + largeByt := make([]byte, 107374200) + reader = bytes.NewBuffer(largeByt) + params = FileUploadV2Parameters{ + Filename: "test.txt", Reader: reader, FileSize: len(largeByt), + Channel: "CXXXXXXXX"} + if _, err := api.UploadFileV2(params); err != nil { + t.Errorf("Unexpected error: %s", err) + } +} diff --git a/misc.go b/misc.go index 9180116af..7e5a8d54a 100644 --- a/misc.go +++ b/misc.go @@ -307,6 +307,9 @@ type responseParser func(*http.Response) error func newJSONParser(dst interface{}) responseParser { return func(resp *http.Response) error { + if dst == nil { + return nil + } return json.NewDecoder(resp.Body).Decode(dst) } } From d85f3251b69119b075da29d2c863ea0452ddd136 Mon Sep 17 00:00:00 2001 From: Sarthak Kothari Date: Mon, 21 Nov 2022 08:30:00 -0800 Subject: [PATCH 2/5] Adding comments and cleanup --- files.go | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/files.go b/files.go index 5c2957f53..5c750d14e 100644 --- a/files.go +++ b/files.go @@ -456,6 +456,7 @@ func (api *Client) ShareFilePublicURLContext(ctx context.Context, fileID string) return &response.File, response.Comments, &response.Paging, nil } +// getUploadURLExternal gets a URL and fileID from slack which can later be used to upload a file func (api *Client) getUploadURLExternal(ctx context.Context, fileSize int, fileName, altText, snippetText string) (*getUploadURLExternalResponse, error) { values := url.Values{ "token": {api.token}, @@ -477,6 +478,7 @@ func (api *Client) getUploadURLExternal(ctx context.Context, fileSize int, fileN return response, response.Err() } +// uploadToURL uploads the file to the provided URL using post method func (api *Client) uploadToURL(ctx context.Context, params uploadToExternalParams) (err error) { values := url.Values{} if params.Content != "" { @@ -488,11 +490,11 @@ func (api *Client) uploadToURL(ctx context.Context, params uploadToExternalParam } else if params.Reader != nil { err = postWithMultipartResponse(ctx, api.httpclient, params.UploadURL, params.Filename, "file", api.token, values, params.Reader, nil, api) } - _ = 1 return err } -func (api *Client) completeUploadExternal(ctx context.Context, fileID string, params FileUploadV2Parameters) (file *FileSummary, err error) { +// completeUploadExternal once files are uploaded, this completes the upload and shares it to the specified channel +func (api *Client) completeUploadExternal(ctx context.Context, fileID string, params FileUploadV2Parameters) (file *completeUploadExternalResponse, err error) { values := url.Values{ "token": {api.token}, } @@ -519,16 +521,21 @@ func (api *Client) completeUploadExternal(ctx context.Context, fileID string, pa if response.Err() != nil { return nil, response.Err() } - if len(response.Files) != 1 { - return nil, fmt.Errorf("files.completeUploadExternal: something went wrong; recieved %d files instead of 1", len(response.Files)) - } - return &response.Files[0], nil + return response, nil } +// UploadFileV2 uploads file to a given slack channel using 3 steps - +// 1. Get an upload URL using files.getUploadURLExternal API +// 2. Send the file as a post to the URL provided by slack +// 3. Complete the upload and share it to the specified channel using files.completeUploadExternal func (api *Client) UploadFileV2(params FileUploadV2Parameters) (*FileSummary, error) { return api.UploadFileV2Context(context.Background(), params) } +// UploadFileV2 uploads file to a given slack channel using 3 steps with a custom context - +// 1. Get an upload URL using files.getUploadURLExternal API +// 2. Send the file as a post to the URL provided by slack +// 3. Complete the upload and share it to the specified channel using files.completeUploadExternal func (api *Client) UploadFileV2Context(ctx context.Context, params FileUploadV2Parameters) (file *FileSummary, err error) { if params.Filename == "" { return nil, fmt.Errorf("file.upload.v2: filename cannot be empty") @@ -555,9 +562,13 @@ func (api *Client) UploadFileV2Context(ctx context.Context, params FileUploadV2P return nil, err } - file, err = api.completeUploadExternal(ctx, u.FileID, params) + c, err := api.completeUploadExternal(ctx, u.FileID, params) if err != nil { return nil, err } - return file, nil + if len(c.Files) != 1 { + return nil, fmt.Errorf("file.upload.v2: something went wrong; recieved %d files instead of 1", len(c.Files)) + } + + return &c.Files[0], nil } From 5b488b4a52c6295c9e128894ed48d149ca5164b2 Mon Sep 17 00:00:00 2001 From: Sarthak Kothari Date: Fri, 2 Dec 2022 17:56:25 -0800 Subject: [PATCH 3/5] fix lint --- files.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/files.go b/files.go index 5c750d14e..50bf04e6f 100644 --- a/files.go +++ b/files.go @@ -524,7 +524,7 @@ func (api *Client) completeUploadExternal(ctx context.Context, fileID string, pa return response, nil } -// UploadFileV2 uploads file to a given slack channel using 3 steps - +// UploadFileV2 uploads file to a given slack channel using 3 steps - // 1. Get an upload URL using files.getUploadURLExternal API // 2. Send the file as a post to the URL provided by slack // 3. Complete the upload and share it to the specified channel using files.completeUploadExternal @@ -532,7 +532,7 @@ func (api *Client) UploadFileV2(params FileUploadV2Parameters) (*FileSummary, er return api.UploadFileV2Context(context.Background(), params) } -// UploadFileV2 uploads file to a given slack channel using 3 steps with a custom context - +// UploadFileV2 uploads file to a given slack channel using 3 steps with a custom context - // 1. Get an upload URL using files.getUploadURLExternal API // 2. Send the file as a post to the URL provided by slack // 3. Complete the upload and share it to the specified channel using files.completeUploadExternal @@ -567,7 +567,7 @@ func (api *Client) UploadFileV2Context(ctx context.Context, params FileUploadV2P return nil, err } if len(c.Files) != 1 { - return nil, fmt.Errorf("file.upload.v2: something went wrong; recieved %d files instead of 1", len(c.Files)) + return nil, fmt.Errorf("file.upload.v2: something went wrong; received %d files instead of 1", len(c.Files)) } return &c.Files[0], nil From 7c39b554599a5cac924386bed1355b5c1ff730d9 Mon Sep 17 00:00:00 2001 From: Sarthak Kothari Date: Fri, 2 Dec 2022 18:48:55 -0800 Subject: [PATCH 4/5] More improvements --- files.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/files.go b/files.go index 50bf04e6f..d32001047 100644 --- a/files.go +++ b/files.go @@ -459,10 +459,10 @@ func (api *Client) ShareFilePublicURLContext(ctx context.Context, fileID string) // getUploadURLExternal gets a URL and fileID from slack which can later be used to upload a file func (api *Client) getUploadURLExternal(ctx context.Context, fileSize int, fileName, altText, snippetText string) (*getUploadURLExternalResponse, error) { values := url.Values{ - "token": {api.token}, + "token": {api.token}, + "filename": {fileName}, + "length": {strconv.Itoa(fileSize)}, } - values.Add("filename", fileName) - values.Add("length", strconv.Itoa(fileSize)) if altText != "" { values.Add("initial_comment", altText) } @@ -495,17 +495,16 @@ func (api *Client) uploadToURL(ctx context.Context, params uploadToExternalParam // completeUploadExternal once files are uploaded, this completes the upload and shares it to the specified channel func (api *Client) completeUploadExternal(ctx context.Context, fileID string, params FileUploadV2Parameters) (file *completeUploadExternalResponse, err error) { - values := url.Values{ - "token": {api.token}, - } - request := []FileSummary{{ID: fileID, Title: params.Title}} requestBytes, err := json.Marshal(request) if err != nil { return nil, err } - values.Add("files", string(requestBytes)) - values.Add("channel_id", params.Channel) + values := url.Values{ + "token": {api.token}, + "files": {string(requestBytes)}, + "channel_id": {params.Channel}, + } if params.InitialComment != "" { values.Add("initial_comment", params.InitialComment) From bddec0ac2cd0142ba49f994453522141e885a9d2 Mon Sep 17 00:00:00 2001 From: Naoki Kanatani Date: Tue, 20 Dec 2022 23:42:47 +0900 Subject: [PATCH 5/5] refactor FileUploadV2 implementation --- files.go | 72 ++++++++++++++++++++++++++++++++++----------------- files_test.go | 6 ++--- 2 files changed, 51 insertions(+), 27 deletions(-) diff --git a/files.go b/files.go index d32001047..356284420 100644 --- a/files.go +++ b/files.go @@ -146,7 +146,7 @@ type ListFilesParameters struct { Cursor string } -type FileUploadV2Parameters struct { +type UploadFileV2Parameters struct { File string FileSize int Content string @@ -160,13 +160,20 @@ type FileUploadV2Parameters struct { SnippetText string } +type getUploadURLExternalParameters struct { + altText string + fileSize int + fileName string + snippetText string +} + type getUploadURLExternalResponse struct { UploadURL string `json:"upload_url"` FileID string `json:"file_id"` SlackResponse } -type uploadToExternalParams struct { +type uploadToURLParameters struct { UploadURL string Reader io.Reader File string @@ -179,6 +186,13 @@ type FileSummary struct { Title string `json:"title"` } +type completeUploadExternalParameters struct { + title string + channel string + initialComment string + threadTimestamp string +} + type completeUploadExternalResponse struct { SlackResponse Files []FileSummary `json:"files"` @@ -457,17 +471,17 @@ func (api *Client) ShareFilePublicURLContext(ctx context.Context, fileID string) } // getUploadURLExternal gets a URL and fileID from slack which can later be used to upload a file -func (api *Client) getUploadURLExternal(ctx context.Context, fileSize int, fileName, altText, snippetText string) (*getUploadURLExternalResponse, error) { +func (api *Client) getUploadURLExternal(ctx context.Context, params getUploadURLExternalParameters) (*getUploadURLExternalResponse, error) { values := url.Values{ "token": {api.token}, - "filename": {fileName}, - "length": {strconv.Itoa(fileSize)}, + "filename": {params.fileName}, + "length": {strconv.Itoa(params.fileSize)}, } - if altText != "" { - values.Add("initial_comment", altText) + if params.altText != "" { + values.Add("initial_comment", params.altText) } - if snippetText != "" { - values.Add("thread_ts", snippetText) + if params.snippetText != "" { + values.Add("thread_ts", params.snippetText) } response := &getUploadURLExternalResponse{} err := api.postMethod(ctx, "files.getUploadURLExternal", values, response) @@ -479,7 +493,7 @@ func (api *Client) getUploadURLExternal(ctx context.Context, fileSize int, fileN } // uploadToURL uploads the file to the provided URL using post method -func (api *Client) uploadToURL(ctx context.Context, params uploadToExternalParams) (err error) { +func (api *Client) uploadToURL(ctx context.Context, params uploadToURLParameters) (err error) { values := url.Values{} if params.Content != "" { values.Add("content", params.Content) @@ -494,8 +508,8 @@ func (api *Client) uploadToURL(ctx context.Context, params uploadToExternalParam } // completeUploadExternal once files are uploaded, this completes the upload and shares it to the specified channel -func (api *Client) completeUploadExternal(ctx context.Context, fileID string, params FileUploadV2Parameters) (file *completeUploadExternalResponse, err error) { - request := []FileSummary{{ID: fileID, Title: params.Title}} +func (api *Client) completeUploadExternal(ctx context.Context, fileID string, params completeUploadExternalParameters) (file *completeUploadExternalResponse, err error) { + request := []FileSummary{{ID: fileID, Title: params.title}} requestBytes, err := json.Marshal(request) if err != nil { return nil, err @@ -503,14 +517,14 @@ func (api *Client) completeUploadExternal(ctx context.Context, fileID string, pa values := url.Values{ "token": {api.token}, "files": {string(requestBytes)}, - "channel_id": {params.Channel}, + "channel_id": {params.channel}, } - if params.InitialComment != "" { - values.Add("initial_comment", params.InitialComment) + if params.initialComment != "" { + values.Add("initial_comment", params.initialComment) } - if params.ThreadTimestamp != "" { - values.Add("thread_ts", params.ThreadTimestamp) + if params.threadTimestamp != "" { + values.Add("thread_ts", params.threadTimestamp) } response := &completeUploadExternalResponse{} err = api.postMethod(ctx, "files.completeUploadExternal", values, response) @@ -524,18 +538,18 @@ func (api *Client) completeUploadExternal(ctx context.Context, fileID string, pa } // UploadFileV2 uploads file to a given slack channel using 3 steps - -// 1. Get an upload URL using files.getUploadURLExternal API +// 1. Get an upload URL using files.getUploadURLExternal API // 2. Send the file as a post to the URL provided by slack // 3. Complete the upload and share it to the specified channel using files.completeUploadExternal -func (api *Client) UploadFileV2(params FileUploadV2Parameters) (*FileSummary, error) { +func (api *Client) UploadFileV2(params UploadFileV2Parameters) (*FileSummary, error) { return api.UploadFileV2Context(context.Background(), params) } // UploadFileV2 uploads file to a given slack channel using 3 steps with a custom context - -// 1. Get an upload URL using files.getUploadURLExternal API +// 1. Get an upload URL using files.getUploadURLExternal API // 2. Send the file as a post to the URL provided by slack // 3. Complete the upload and share it to the specified channel using files.completeUploadExternal -func (api *Client) UploadFileV2Context(ctx context.Context, params FileUploadV2Parameters) (file *FileSummary, err error) { +func (api *Client) UploadFileV2Context(ctx context.Context, params UploadFileV2Parameters) (file *FileSummary, err error) { if params.Filename == "" { return nil, fmt.Errorf("file.upload.v2: filename cannot be empty") } @@ -545,12 +559,17 @@ func (api *Client) UploadFileV2Context(ctx context.Context, params FileUploadV2P if params.Channel == "" { return nil, fmt.Errorf("file.upload.v2: channel cannot be empty") } - u, err := api.getUploadURLExternal(ctx, params.FileSize, params.Filename, params.AltTxt, params.SnippetText) + u, err := api.getUploadURLExternal(ctx, getUploadURLExternalParameters{ + altText: params.AltTxt, + fileName: params.Filename, + fileSize: params.FileSize, + snippetText: params.SnippetText, + }) if err != nil { return nil, err } - err = api.uploadToURL(ctx, uploadToExternalParams{ + err = api.uploadToURL(ctx, uploadToURLParameters{ UploadURL: u.UploadURL, Reader: params.Reader, File: params.File, @@ -561,7 +580,12 @@ func (api *Client) UploadFileV2Context(ctx context.Context, params FileUploadV2P return nil, err } - c, err := api.completeUploadExternal(ctx, u.FileID, params) + c, err := api.completeUploadExternal(ctx, u.FileID, completeUploadExternalParameters{ + title: params.Title, + channel: params.Channel, + initialComment: params.InitialComment, + threadTimestamp: params.ThreadTimestamp, + }) if err != nil { return nil, err } diff --git a/files_test.go b/files_test.go index 3a962fe39..f304a4e95 100644 --- a/files_test.go +++ b/files_test.go @@ -246,7 +246,7 @@ func TestUploadFileV2(t *testing.T) { once.Do(startServer) api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) - params := FileUploadV2Parameters{ + params := UploadFileV2Parameters{ Filename: "test.txt", Content: "test content", FileSize: 10, Channel: "CXXXXXXXX", } @@ -255,7 +255,7 @@ func TestUploadFileV2(t *testing.T) { } reader := bytes.NewBufferString("test reader") - params = FileUploadV2Parameters{ + params = UploadFileV2Parameters{ Filename: "test.txt", Reader: reader, FileSize: 10, @@ -266,7 +266,7 @@ func TestUploadFileV2(t *testing.T) { largeByt := make([]byte, 107374200) reader = bytes.NewBuffer(largeByt) - params = FileUploadV2Parameters{ + params = UploadFileV2Parameters{ Filename: "test.txt", Reader: reader, FileSize: len(largeByt), Channel: "CXXXXXXXX"} if _, err := api.UploadFileV2(params); err != nil {