Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added new FileUploadV2 function to avoid server side file timeouts #1130

Merged
merged 4 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 155 additions & 0 deletions files.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package slack

import (
"context"
"encoding/json"
"fmt"
"io"
"net/url"
Expand Down Expand Up @@ -145,6 +146,44 @@ type ListFilesParameters struct {
Cursor string
}

type FileUploadV2Parameters struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[IMO] Prefer UploadFileV2Parameters. It would be easier to use if aligned with the method name.

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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uploadToURLParameters ?

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"`
Expand Down Expand Up @@ -416,3 +455,119 @@ 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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about defining struct since there are many parameters?

values := url.Values{
"token": {api.token},
"filename": {fileName},
"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()
}

// 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 != "" {
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)
}
return err
}

// 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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is preferable to define struct instead of using FileUploadV2Prameters.

request := []FileSummary{{ID: fileID, Title: params.Title}}
requestBytes, err := json.Marshal(request)
if err != nil {
return nil, err
}
values := url.Values{
"token": {api.token},
"files": {string(requestBytes)},
"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()
}
return response, nil
}

// UploadFileV2 uploads file to a given slack channel using 3 steps -
// 1. Get an upload URL using files.getUploadURLExternal API
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// 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) {
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")
}
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
}

c, err := api.completeUploadExternal(ctx, u.FileID, params)
if err != nil {
return nil, err
}
if len(c.Files) != 1 {
return nil, fmt.Errorf("file.upload.v2: something went wrong; received %d files instead of 1", len(c.Files))
}

return &c.Files[0], nil
}
62 changes: 62 additions & 0 deletions files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
3 changes: 3 additions & 0 deletions misc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down