diff --git a/internal/deputil/url.go b/internal/deputil/url.go index fa0ac4ed..3d096365 100644 --- a/internal/deputil/url.go +++ b/internal/deputil/url.go @@ -14,11 +14,15 @@ package deputil -import "net/http" +import ( + "net/http" -// URLChecker returns url if it's status code is 200, otherwise returns empty string -func URLChecker(url string) string { - resp, err := http.Get(url) + "github.com/slackapi/slack-cli/internal/slackhttp" +) + +// URLChecker returns url if its status code is 200, otherwise returns empty string +func URLChecker(httpClient slackhttp.HTTPClient, url string) string { + resp, err := httpClient.Get(url) if err != nil { return "" } diff --git a/internal/deputil/url_test.go b/internal/deputil/url_test.go index 52137ea1..f9a30334 100644 --- a/internal/deputil/url_test.go +++ b/internal/deputil/url_test.go @@ -15,15 +15,56 @@ package deputil import ( + "fmt" + "net/http" "testing" + "github.com/slackapi/slack-cli/internal/slackhttp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) func Test_URLChecker(t *testing.T) { - url := URLChecker("https://github.com/slack-samples/deno-starter-template") - assert.Equal(t, "https://github.com/slack-samples/deno-starter-template", url, "should return url when url is valid") + tests := map[string]struct { + url string + expectedURL string + setupHTTPClientMock func(*slackhttp.HTTPClientMock) + }{ + "Returns the URL when the HTTP status code is http.StatusOK": { + url: "https://github.com/slack-samples/deno-starter-template", + expectedURL: "https://github.com/slack-samples/deno-starter-template", + setupHTTPClientMock: func(httpClientMock *slackhttp.HTTPClientMock) { + resOK := slackhttp.MockHTTPResponse(http.StatusOK, "OK") + httpClientMock.On("Get", mock.Anything).Return(resOK, nil) + }, + }, + "Returns an empty string when the HTTP status code is not 200": { + url: "https://github.com/slack-samples/template-not-found", + expectedURL: "", + setupHTTPClientMock: func(httpClientMock *slackhttp.HTTPClientMock) { + resNotFound := slackhttp.MockHTTPResponse(http.StatusNotFound, "Not Found") + httpClientMock.On("Get", mock.Anything).Return(resNotFound, nil) + }, + }, + "Returns an empty string when the HTTPClient has an error": { + url: "invalid_url", + expectedURL: "", + setupHTTPClientMock: func(httpClientMock *slackhttp.HTTPClientMock) { + httpClientMock.On("Get", mock.Anything).Return(nil, fmt.Errorf("HTTPClient error")) + }, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + // Create mocks + httpClientMock := &slackhttp.HTTPClientMock{} + tt.setupHTTPClientMock(httpClientMock) - url = URLChecker("fake_url") - assert.Equal(t, "", url, "should return empty string when url is invalid") + // Execute + url := URLChecker(httpClientMock, tt.url) + + // Assertions + assert.Equal(t, tt.expectedURL, url) + }) + } } diff --git a/internal/pkg/create/create.go b/internal/pkg/create/create.go index ac23b9dc..0c0a38a2 100644 --- a/internal/pkg/create/create.go +++ b/internal/pkg/create/create.go @@ -39,6 +39,7 @@ import ( "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/slackapi/slack-cli/internal/slackhttp" "github.com/slackapi/slack-cli/internal/slacktrace" "github.com/slackapi/slack-cli/internal/style" "github.com/spf13/afero" @@ -225,7 +226,8 @@ func createApp(ctx context.Context, dirPath string, template Template, gitBranch if template.isGit { doctorSection, err := doctor.CheckGit(ctx) if doctorSection.HasError() || err != nil { - zipFileURL := generateGitZipFileURL(template.path, gitBranch) + httpClient := slackhttp.NewHTTPClient(slackhttp.HTTPClientOptions{}) + zipFileURL := generateGitZipFileURL(httpClient, template.path, gitBranch) if zipFileURL == "" { return slackerror.New(slackerror.ErrGitZipDownload) } @@ -520,18 +522,17 @@ func InstallProjectDependencies( return outputs } -// generateGitZipFileURL will return template's GitHub zip file download link -// In the future, this function can be extended to support other Git hosts, such as GitLab. -// TODO, @cchensh, we should get prepared for other non-Git hosts and refactor the create pkg -func generateGitZipFileURL(templateURL string, gitBranch string) string { +// generateGitZipFileURL will return the GitHub zip URL for a templateURL. +func generateGitZipFileURL(httpClient slackhttp.HTTPClient, templateURL string, gitBranch string) string { zipURL := strings.ReplaceAll(templateURL, ".git", "") + "/archive/refs/heads/" if gitBranch == "" { mainURL := zipURL + "main.zip" masterURL := zipURL + "master.zip" - zipURL = deputil.URLChecker(mainURL) + + zipURL = deputil.URLChecker(httpClient, mainURL) if zipURL == "" { - zipURL = deputil.URLChecker(masterURL) + zipURL = deputil.URLChecker(httpClient, masterURL) } } else { zipURL = zipURL + gitBranch + ".zip" diff --git a/internal/pkg/create/create_test.go b/internal/pkg/create/create_test.go index 1daf8b69..df8082f3 100644 --- a/internal/pkg/create/create_test.go +++ b/internal/pkg/create/create_test.go @@ -16,6 +16,7 @@ package create import ( "fmt" + "net/http" "path/filepath" "testing" @@ -23,6 +24,7 @@ import ( "github.com/slackapi/slack-cli/internal/experiment" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/slackcontext" + "github.com/slackapi/slack-cli/internal/slackhttp" "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -75,18 +77,71 @@ func TestGetAvailableDirectory(t *testing.T) { } func Test_generateGitZipFileURL(t *testing.T) { - url := generateGitZipFileURL("https://github.com/slack-samples/deno-starter-template", "pre-release-0316") - assert.Equal(t, "https://github.com/slack-samples/deno-starter-template/archive/refs/heads/pre-release-0316.zip", url, "should return zip download link with branch") - - url = generateGitZipFileURL("https://github.com/slack-samples/deno-starter-template", "") - assert.Equal(t, "https://github.com/slack-samples/deno-starter-template/archive/refs/heads/main.zip", url, "should return zip download link with main") + tests := map[string]struct { + templateURL string + gitBranch string + expectedURL string + setupHTTPClientMock func(*slackhttp.HTTPClientMock) + }{ + "Returns the zip URL using the main branch when no branch is provided": { + templateURL: "https://github.com/slack-samples/deno-starter-template", + gitBranch: "", + expectedURL: "https://github.com/slack-samples/deno-starter-template/archive/refs/heads/main.zip", + setupHTTPClientMock: func(httpClientMock *slackhttp.HTTPClientMock) { + res := slackhttp.MockHTTPResponse(http.StatusOK, "OK") + httpClientMock.On("Get", mock.Anything).Return(res, nil) + }, + }, + "Returns the zip URL using the master branch when no branch is provided and main branch doesn't exist": { + templateURL: "https://github.com/slack-samples/deno-starter-template", + gitBranch: "", + expectedURL: "https://github.com/slack-samples/deno-starter-template/archive/refs/heads/master.zip", + setupHTTPClientMock: func(httpClientMock *slackhttp.HTTPClientMock) { + res := slackhttp.MockHTTPResponse(http.StatusOK, "OK") + httpClientMock.On("Get", "https://github.com/slack-samples/deno-starter-template/archive/refs/heads/main.zip").Return(nil, fmt.Errorf("HttpClient error")) + httpClientMock.On("Get", "https://github.com/slack-samples/deno-starter-template/archive/refs/heads/master.zip").Return(res, nil) + }, + }, + "Returns the zip URL using the specified branch when a branch is provided": { + templateURL: "https://github.com/slack-samples/deno-starter-template", + gitBranch: "pre-release-0316", + expectedURL: "https://github.com/slack-samples/deno-starter-template/archive/refs/heads/pre-release-0316.zip", + setupHTTPClientMock: func(httpClientMock *slackhttp.HTTPClientMock) { + res := slackhttp.MockHTTPResponse(http.StatusOK, "OK") + httpClientMock.On("Get", mock.Anything).Return(res, nil) + }, + }, + "Returns an empty string when the HTTP status code is not 200": { + templateURL: "https://github.com/slack-samples/deno-starter-template", + gitBranch: "", + expectedURL: "", + setupHTTPClientMock: func(httpClientMock *slackhttp.HTTPClientMock) { + res := slackhttp.MockHTTPResponse(http.StatusNotFound, "Not Found") + httpClientMock.On("Get", mock.Anything).Return(res, nil) + }, + }, + "Returns an empty string when the HTTPClient has an error": { + templateURL: "https://github.com/slack-samples/deno-starter-template", + gitBranch: "", + expectedURL: "", + setupHTTPClientMock: func(httpClientMock *slackhttp.HTTPClientMock) { + httpClientMock.On("Get", mock.Anything).Return(nil, fmt.Errorf("HTTPClient error")) + }, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + // Create mocks + httpClientMock := &slackhttp.HTTPClientMock{} + tt.setupHTTPClientMock(httpClientMock) - // TODO - We should mock the `deputil.URLChecker` HTTP request so that the unit test is not dependent on the network activity and repo configuration - url = generateGitZipFileURL("https://github.com/google/uuid", "") - assert.Equal(t, "https://github.com/google/uuid/archive/refs/heads/master.zip", url, "should return zip download link with 'master' when 'main' branch doesn't exist") + // Execute + url := generateGitZipFileURL(httpClientMock, tt.templateURL, tt.gitBranch) - url = generateGitZipFileURL("fake_url", "") - assert.Equal(t, "", url, "should return empty string when url is invalid") + // Assertions + assert.Equal(t, tt.expectedURL, url) + }) + } } func TestCreateGitArgs(t *testing.T) { diff --git a/internal/slackhttp/http.go b/internal/slackhttp/http.go new file mode 100644 index 00000000..3cf38b3b --- /dev/null +++ b/internal/slackhttp/http.go @@ -0,0 +1,71 @@ +// Copyright 2022-2025 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package slackhttp + +import ( + "crypto/tls" + "net/http" + "time" +) + +const ( + // defaultTotalTimeout is the default timeout for the life of the request. + defaultTotalTimeout = 30 * time.Second +) + +// HTTPClient interface for http.Client. +type HTTPClient interface { + Do(req *http.Request) (*http.Response, error) + Get(url string) (*http.Response, error) +} + +// HTTPClientOptions allows for the customization of a http.Client. +type HTTPClientOptions struct { + AttemptTimeout *time.Duration // AttemptTimeout is how long each request waits before cancelled. + Backoff *time.Duration // Backoff is a constant duration to wait between requests. + Retries int // Retries for a request, if 0 then no retry logic is added. + RetryErrorCodes []string // RetryErrorCode is list of error codes to retry on. + RetryStatusCodes []int // RetryStatusCode is list of http status codes to retry on. + SkipTLSVerify bool // SkipTLSVerify will skip verifying the host's certificate. + TotalTimeOut time.Duration // TotalTimeOut for the life of the request (default: defaultTotalTimeout). +} + +// NewHTTPClient returns an http.Client configured with HTTPClientOptions. +func NewHTTPClient(opts HTTPClientOptions) *http.Client { + client := &http.Client{} + + if opts.TotalTimeOut == 0 { + client.Timeout = defaultTotalTimeout + } else { + client.Timeout = opts.TotalTimeOut + } + + var transport = http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: opts.SkipTLSVerify, + }, + Proxy: http.ProxyFromEnvironment, + } + + // If retries aren't specified return a non-retrying transport. + if opts.Retries == 0 { + client.Transport = &transport + return client + } + + // TODO: Implement retry logic + client.Transport = &transport + return client +} diff --git a/internal/slackhttp/http_mock.go b/internal/slackhttp/http_mock.go new file mode 100644 index 00000000..f6842f81 --- /dev/null +++ b/internal/slackhttp/http_mock.go @@ -0,0 +1,65 @@ +// Copyright 2022-2025 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package slackhttp + +import ( + "io" + "net/http" + "net/http/httptest" + + "github.com/stretchr/testify/mock" +) + +// HTTPClientMock implements a mock for the HTTPClient interface. +type HTTPClientMock struct { + mock.Mock +} + +// Do is a mock that tracks the calls to Do and returns the mocked http.Response and error. +func (m *HTTPClientMock) Do(req *http.Request) (*http.Response, error) { + args := m.Called(req) + + // http.Response can be nil when an error is provided. + var httpResp *http.Response + if _httpResp, ok := args.Get(0).(*http.Response); ok { + httpResp = _httpResp + } + + return httpResp, args.Error(1) +} + +// Get is a mock that tracks the calls to Get and returns the mocked http.Response and error. +func (m *HTTPClientMock) Get(url string) (*http.Response, error) { + args := m.Called(url) + + // http.Response can be nil when an error is provided. + var httpResp *http.Response + if _httpResp, ok := args.Get(0).(*http.Response); ok { + httpResp = _httpResp + } + + return httpResp, args.Error(1) +} + +// MockHTTPResponse is a helper that returns a mocked http.Response with the provided httpStatus and body. +func MockHTTPResponse(httpStatus int, body string) *http.Response { + resWriter := httptest.NewRecorder() + + resWriter.WriteHeader(httpStatus) + _, _ = io.WriteString(resWriter, body) + res := resWriter.Result() + + return res +} diff --git a/internal/slackhttp/http_test.go b/internal/slackhttp/http_test.go new file mode 100644 index 00000000..89fc269e --- /dev/null +++ b/internal/slackhttp/http_test.go @@ -0,0 +1,75 @@ +// Copyright 2022-2025 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package slackhttp + +import ( + "net/http" + "net/url" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func Test_NewHTTPClient(t *testing.T) { + tests := map[string]struct { + proxy func(*http.Request) (*url.URL, error) + expectedProxy func(*http.Request) (*url.URL, error) + retries int + expectedRetries int + skipTLSVerify bool + expectedSkipTLSVerify bool + timeout time.Duration + expectedTimeout time.Duration + }{ + "Returns a httpClient with default configuration": { + expectedTimeout: defaultTotalTimeout, + expectedRetries: 0, + expectedSkipTLSVerify: false, + expectedProxy: nil, + }, + "Returns a httpClient with custom configuration": { + proxy: http.ProxyFromEnvironment, + expectedProxy: http.ProxyFromEnvironment, + + retries: 3, + expectedRetries: 3, + + skipTLSVerify: true, + expectedSkipTLSVerify: true, + + timeout: 120 * time.Second, + expectedTimeout: 120 * time.Second, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + // Setup + opts := HTTPClientOptions{ + Retries: tt.expectedRetries, + SkipTLSVerify: tt.expectedSkipTLSVerify, + TotalTimeOut: tt.timeout, + } + + httpClient := NewHTTPClient(opts) + + // Assertions + assert.Equal(t, tt.expectedTimeout, httpClient.Timeout) + assert.Equal(t, tt.expectedSkipTLSVerify, httpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify) + assert.ObjectsAreEqual(tt.expectedProxy, httpClient.Transport.(*http.Transport).Proxy) + // TODO: add assertion for retries + }) + } +}