From 56feeb94cb34bd6d31eb528d9a708c6097aea7ea Mon Sep 17 00:00:00 2001 From: Junya Hayashi Date: Fri, 1 Jan 2021 20:56:25 +0900 Subject: [PATCH] remotefiles: add tests --- remotefiles.go | 36 +++++--- remotefiles_test.go | 200 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 223 insertions(+), 13 deletions(-) create mode 100644 remotefiles_test.go diff --git a/remotefiles.go b/remotefiles.go index 06e33315d..5ff6f46cf 100644 --- a/remotefiles.go +++ b/remotefiles.go @@ -2,6 +2,7 @@ package slack import ( "context" + "fmt" "io" "net/url" "strconv" @@ -90,11 +91,8 @@ func (api *Client) AddRemoteFile(params RemoteFileParameters) (remotefile *Remot // AddRemoteFileContext adds a remote file and setting a custom context func (api *Client) AddRemoteFileContext(ctx context.Context, params RemoteFileParameters) (remotefile *RemoteFile, err error) { - // Test if user token is valid. This helps because client.Do doesn't like this for some reason. XXX: More - // investigation needed, but for now this will do. - _, err = api.AuthTest() - if err != nil { - return nil, err + if params.ExternalID == "" || params.ExternalURL == "" || params.Title == "" { + return nil, ErrParametersMissing } response := &remoteFileResponseFull{} values := url.Values{ @@ -167,6 +165,12 @@ func (api *Client) GetRemoteFileInfo(externalID, fileID string) (remotefile *Rem // GetRemoteFileInfoContext retrieves the complete remote file information given with a custom context. func (api *Client) GetRemoteFileInfoContext(ctx context.Context, externalID, fileID string) (remotefile *RemoteFile, err error) { + if fileID == "" && externalID == "" { + return nil, fmt.Errorf("either externalID or fileID is required") + } + if fileID != "" && externalID != "" { + return nil, fmt.Errorf("don't provide both externalID and fileID") + } values := url.Values{ "token": {api.token}, } @@ -190,6 +194,12 @@ func (api *Client) ShareRemoteFile(channels []string, externalID, fileID string) // ShareRemoteFileContext shares a remote file to channels with a custom context func (api *Client) ShareRemoteFileContext(ctx context.Context, channels []string, externalID, fileID string) (file *RemoteFile, err error) { + if channels == nil || len(channels) == 0 { + return nil, ErrParametersMissing + } + if fileID == "" && externalID == "" { + return nil, fmt.Errorf("either externalID or fileID is required") + } values := url.Values{ "token": {api.token}, "channels": {strings.Join(channels, ",")}, @@ -214,12 +224,6 @@ func (api *Client) UpdateRemoteFile(fileID string, params RemoteFileParameters) // UpdateRemoteFileContext updates a remote file with a custom context func (api *Client) UpdateRemoteFileContext(ctx context.Context, fileID string, params RemoteFileParameters) (remotefile *RemoteFile, err error) { - // Test if user token is valid. This helps because client.Do doesn't like this for some reason. XXX: More - // investigation needed, but for now this will do. - _, err = api.AuthTest() - if err != nil { - return nil, err - } response := &remoteFileResponseFull{} values := url.Values{ "token": {api.token}, @@ -243,9 +247,9 @@ func (api *Client) UpdateRemoteFileContext(ctx context.Context, fileID string, p values.Add("indexable_file_contents", params.IndexableFileContents) } if params.PreviewImageReader != nil { - err = postWithMultipartResponse(ctx, api.httpclient, api.endpoint+"files.remote.add", "preview.png", "preview_image", values, params.PreviewImageReader, response, api) + err = postWithMultipartResponse(ctx, api.httpclient, api.endpoint+"files.remote.update", "preview.png", "preview_image", values, params.PreviewImageReader, response, api) } else { - response, err = api.remoteFileRequest(ctx, "files.remote.add", values) + response, err = api.remoteFileRequest(ctx, "files.remote.update", values) } if err != nil { @@ -262,6 +266,12 @@ func (api *Client) RemoveRemoteFile(externalID, fileID string) (err error) { // RemoveRemoteFileContext removes a remote file with a custom context func (api *Client) RemoveRemoteFileContext(ctx context.Context, externalID, fileID string) (err error) { + if fileID == "" && externalID == "" { + return fmt.Errorf("either externalID or fileID is required") + } + if fileID != "" && externalID != "" { + return fmt.Errorf("don't provide both externalID and fileID") + } values := url.Values{ "token": {api.token}, } diff --git a/remotefiles_test.go b/remotefiles_test.go new file mode 100644 index 000000000..397bdf6b8 --- /dev/null +++ b/remotefiles_test.go @@ -0,0 +1,200 @@ +package slack + +import ( + "encoding/json" + "net/http" + "strings" + "testing" +) + +func addRemoteFileHandler(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Content-Type", "application/json") + response, _ := json.Marshal(remoteFileResponseFull{ + SlackResponse: SlackResponse{Ok: true}}) + rw.Write(response) +} + +func TestAddRemoteFile(t *testing.T) { + http.HandleFunc("/files.remote.add", addRemoteFileHandler) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + params := RemoteFileParameters{ + ExternalID: "externalID", + ExternalURL: "http://example.com/", + Title: "example", + } + if _, err := api.AddRemoteFile(params); err != nil { + t.Errorf("Unexpected error: %s", err) + } +} + +func TestAddRemoteFileWithoutTitle(t *testing.T) { + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + params := RemoteFileParameters{ + ExternalID: "externalID", + ExternalURL: "http://example.com/", + } + if _, err := api.AddRemoteFile(params); err != ErrParametersMissing { + t.Errorf("Expected ErrParametersMissing. got %s", err) + } +} + +func listRemoteFileHandler(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Content-Type", "application/json") + response, _ := json.Marshal(remoteFileResponseFull{ + SlackResponse: SlackResponse{Ok: true}}) + rw.Write(response) +} + +func TestListRemoteFile(t *testing.T) { + http.HandleFunc("/files.remote.list", listRemoteFileHandler) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + params := ListRemoteFilesParameters{} + if _, _, err := api.ListRemoteFiles(params); err != nil { + t.Errorf("Unexpected error: %s", err) + } +} + +func getRemoteFileInfoHandler(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Content-Type", "application/json") + response, _ := json.Marshal(remoteFileResponseFull{ + SlackResponse: SlackResponse{Ok: true}}) + rw.Write(response) +} + +func TestGetRemoteFileInfo(t *testing.T) { + http.HandleFunc("/files.remote.info", getRemoteFileInfoHandler) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + if _, err := api.GetRemoteFileInfo("ExternalID", ""); err != nil { + t.Errorf("Unexpected error: %s", err) + } +} + +func TestGetRemoteFileInfoWithoutID(t *testing.T) { + http.HandleFunc("/files.remote.info", getRemoteFileInfoHandler) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + _, err := api.GetRemoteFileInfo("", "") + if err == nil { + t.Fatal("Expected error when both externalID and fileID is not provided, instead got nil") + } + if !strings.Contains(err.Error(), "either externalID or fileID is required") { + t.Errorf("Error message should mention a required field") + } +} + +func TestGetRemoteFileInfoWithFileIDAndExternalID(t *testing.T) { + http.HandleFunc("/files.remote.info", getRemoteFileInfoHandler) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + _, err := api.GetRemoteFileInfo("ExternalID", "FileID") + if err == nil { + t.Fatal("Expected error when both externalID and fileID are both provided, instead got nil") + } + if !strings.Contains(err.Error(), "don't provide both externalID and fileID") { + t.Errorf("Error message should mention don't providing both externalID and fileID") + } +} + +func shareRemoteFileHandler(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Content-Type", "application/json") + response, _ := json.Marshal(remoteFileResponseFull{ + SlackResponse: SlackResponse{Ok: true}}) + rw.Write(response) +} + +func TestShareRemoteFile(t *testing.T) { + http.HandleFunc("/files.remote.share", shareRemoteFileHandler) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + if _, err := api.ShareRemoteFile([]string{"channel"}, "ExternalID", ""); err != nil { + t.Errorf("Unexpected error: %s", err) + } +} + +func TestShareRemoteFileWithoutChannels(t *testing.T) { + http.HandleFunc("/files.remote.share", shareRemoteFileHandler) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + if _, err := api.ShareRemoteFile([]string{}, "ExternalID", ""); err != ErrParametersMissing { + t.Errorf("Expected ErrParametersMissing. got %s", err) + } +} + +func TestShareRemoteFileWithoutID(t *testing.T) { + http.HandleFunc("/files.remote.info", getRemoteFileInfoHandler) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + _, err := api.ShareRemoteFile([]string{"channel"}, "", "") + if err == nil { + t.Fatal("Expected error when both externalID and fileID is not provided, instead got nil") + } + if !strings.Contains(err.Error(), "either externalID or fileID is required") { + t.Errorf("Error message should mention a required field") + } +} + +func updateRemoteFileHandler(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Content-Type", "application/json") + response, _ := json.Marshal(remoteFileResponseFull{ + SlackResponse: SlackResponse{Ok: true}}) + rw.Write(response) +} + +func TestUpdateRemoteFile(t *testing.T) { + http.HandleFunc("/files.remote.update", updateRemoteFileHandler) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + params := RemoteFileParameters{ + ExternalURL: "http://example.com/", + Title: "example", + } + if _, err := api.UpdateRemoteFile("fileID", params); err != nil { + t.Errorf("Unexpected error: %s", err) + } +} + +func removeRemoteFileHandler(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Content-Type", "application/json") + response, _ := json.Marshal(remoteFileResponseFull{ + SlackResponse: SlackResponse{Ok: true}}) + rw.Write(response) +} + +func TestRemoveRemoteFile(t *testing.T) { + http.HandleFunc("/files.remote.remove", removeRemoteFileHandler) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + if err := api.RemoveRemoteFile("ExternalID", ""); err != nil { + t.Errorf("Unexpected error: %s", err) + } +} + +func TestRemoveRemoteFileWithoutID(t *testing.T) { + http.HandleFunc("/files.remote.remove", removeRemoteFileHandler) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + err := api.RemoveRemoteFile("", "") + if err == nil { + t.Fatal("Expected error when both externalID and fileID is not provided, instead got nil") + } + if !strings.Contains(err.Error(), "either externalID or fileID is required") { + t.Errorf("Error message should mention a required field") + } +} + +func TestRemoveRemoteFileWithFileIDAndExternalID(t *testing.T) { + http.HandleFunc("/files.remote.remove", removeRemoteFileHandler) + once.Do(startServer) + api := New("testing-token", OptionAPIURL("http://"+serverAddr+"/")) + err := api.RemoveRemoteFile("ExternalID", "FileID") + if err == nil { + t.Fatal("Expected error when both externalID and fileID are both provided, instead got nil") + } + if !strings.Contains(err.Error(), "don't provide both externalID and fileID") { + t.Errorf("Error message should mention don't providing both externalID and fileID") + } +}