diff --git a/artifactory/services/delete.go b/artifactory/services/delete.go index 887eb264e..c71357fa2 100644 --- a/artifactory/services/delete.go +++ b/artifactory/services/delete.go @@ -13,6 +13,7 @@ import ( urlutil "github.com/jfrog/jfrog-client-go/utils" "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-client-go/utils/io/content" + "github.com/jfrog/jfrog-client-go/utils/io/httputils" "github.com/jfrog/jfrog-client-go/utils/log" ) @@ -108,6 +109,7 @@ func (ds *DeleteService) createFileHandlerFunc(result *utils.Result) fileDeleteH return nil } httpClientsDetails := ds.GetArtifactoryDetails().CreateHttpClientDetails() + httpClientsDetails.AddPreRetryInterceptor(ds.createPreRetryInterceptor(deletePath, logMsgPrefix+"Checking existence of "+resultItem.GetItemRelativePath())) resp, body, err := ds.client.SendDelete(deletePath, nil, &httpClientsDetails) if err != nil { log.Error(err) @@ -123,6 +125,14 @@ func (ds *DeleteService) createFileHandlerFunc(result *utils.Result) fileDeleteH } } +func (ds *DeleteService) createPreRetryInterceptor(deletePath, retryLog string) httputils.PreRetryInterceptor { + return func() (shouldRetry bool) { + retryHttpClientsDetails := ds.GetArtifactoryDetails().CreateHttpClientDetails() + _, _, nonExistErr := ds.client.GetHttpClient().SendHead(deletePath, retryHttpClientsDetails, retryLog) + return nonExistErr == nil + } +} + func (ds *DeleteService) DeleteFiles(deleteItems *content.ContentReader) (int, error) { producerConsumer := parallel.NewBounedRunner(ds.GetThreads(), false) errorsQueue := clientutils.NewErrorsQueue(1) diff --git a/http/httpclient/client.go b/http/httpclient/client.go index 38d152a9a..999a9ee95 100644 --- a/http/httpclient/client.go +++ b/http/httpclient/client.go @@ -131,8 +131,7 @@ func (jc *HttpClient) Send(method, url string, content []byte, followRedirect, c if resp == nil { return false, errorutils.CheckErrorf("%sReceived empty response from server", logMsgPrefix) } - // If response-code < 500 and it is not 429, should not retry - if resp.StatusCode < 500 && resp.StatusCode != http.StatusTooManyRequests { + if !jc.shouldRetry(resp, &httpClientsDetails) { return false, nil } // Perform retry @@ -145,6 +144,20 @@ func (jc *HttpClient) Send(method, url string, content []byte, followRedirect, c return } +func (jc *HttpClient) shouldRetry(resp *http.Response, httpClientsDetails *httputils.HttpClientDetails) bool { + // If response-code < 500 and it is not 429, should not retry + if resp.StatusCode < 500 && resp.StatusCode != http.StatusTooManyRequests { + return false + } + // If any of the preretry interceptors is false - return false + for _, shouldRetry := range httpClientsDetails.PreRetryInterceptors { + if !shouldRetry() { + return false + } + } + return true +} + func (jc *HttpClient) createReq(method, url string, content []byte) (req *http.Request, err error) { if content != nil { return jc.newRequest(method, url, bytes.NewBuffer(content)) diff --git a/http/httpclient/client_test.go b/http/httpclient/client_test.go new file mode 100644 index 000000000..2ca64fccb --- /dev/null +++ b/http/httpclient/client_test.go @@ -0,0 +1,44 @@ +package httpclient + +import ( + "net/http" + "testing" + + "github.com/jfrog/jfrog-client-go/utils/io/httputils" + "github.com/stretchr/testify/assert" +) + +var shouldRetryCases = []struct { + name string + status int + expectedRetry bool + preRetryInterceptor httputils.PreRetryInterceptor +}{ + // Status 200 + {"200", http.StatusOK, false, nil}, + {"200 with interceptor returning false", http.StatusOK, false, func() bool { return false }}, + {"200 with interceptor returning true", http.StatusOK, false, func() bool { return true }}, + + // Status 502 + {"502", http.StatusBadGateway, true, nil}, + {"429", http.StatusTooManyRequests, true, nil}, + {"502 with interceptor returning false", http.StatusBadGateway, false, func() bool { return false }}, + {"502 with interceptor returning true", http.StatusBadGateway, true, func() bool { return true }}, +} + +func TestShouldRetry(t *testing.T) { + httpClient, err := ClientBuilder().Build() + assert.NoError(t, err) + + for _, testCase := range shouldRetryCases { + t.Run(testCase.name, func(t *testing.T) { + httpClientsDetails := &httputils.HttpClientDetails{} + if testCase.preRetryInterceptor != nil { + httpClientsDetails.AddPreRetryInterceptor(testCase.preRetryInterceptor) + } + shouldRetry := httpClient.shouldRetry(&http.Response{StatusCode: testCase.status}, httpClientsDetails) + assert.NoError(t, err) + assert.Equal(t, testCase.expectedRetry, shouldRetry) + }) + } +} diff --git a/utils/io/httputils/httpclient.go b/utils/io/httputils/httpclient.go index d44f0fc31..9d86a7eb1 100644 --- a/utils/io/httputils/httpclient.go +++ b/utils/io/httputils/httpclient.go @@ -16,8 +16,12 @@ type HttpClientDetails struct { Transport *http.Transport DialTimeout time.Duration OverallRequestTimeout time.Duration + // Prior to each retry attempt, the list of PreRetryInterceptors is invoked sequentially. If any of these interceptors yields a 'false' response, the retry process stops instantly. + PreRetryInterceptors []PreRetryInterceptor } +type PreRetryInterceptor func() (shouldRetry bool) + func (httpClientDetails HttpClientDetails) Clone() *HttpClientDetails { headers := make(map[string]string) utils.MergeMaps(httpClientDetails.Headers, headers) @@ -34,5 +38,10 @@ func (httpClientDetails HttpClientDetails) Clone() *HttpClientDetails { Transport: transport, DialTimeout: httpClientDetails.DialTimeout, OverallRequestTimeout: httpClientDetails.OverallRequestTimeout, + PreRetryInterceptors: httpClientDetails.PreRetryInterceptors, } } + +func (httpClientDetails *HttpClientDetails) AddPreRetryInterceptor(preRetryInterceptors PreRetryInterceptor) { + httpClientDetails.PreRetryInterceptors = append(httpClientDetails.PreRetryInterceptors, preRetryInterceptors) +} diff --git a/xray/services/buildscan.go b/xray/services/buildscan.go index 3861803df..734738ed3 100644 --- a/xray/services/buildscan.go +++ b/xray/services/buildscan.go @@ -97,7 +97,7 @@ func (bs *BuildScanService) prepareGetResultsRequest(params XrayBuildParams, par httpClientsDetails := bs.XrayDetails.CreateHttpClientDetails() utils.SetContentType("application/json", &httpClientsDetails.Headers) if version.NewVersion(xrayVer).AtLeast(buildScanResultsPostApiMinXrayVersion) { - getResultsReqFunc = bs.getResultsPostRequestFunc(paramsBytes, &httpClientsDetails, queryParams) + getResultsReqFunc = bs.getResultsPostRequestFunc(params, paramsBytes, &httpClientsDetails, queryParams) return } getResultsReqFunc = bs.getResultsGetRequestFunc(params, &httpClientsDetails, queryParams) @@ -155,8 +155,11 @@ func (bs *BuildScanService) getResultsGetRequestFunc(params XrayBuildParams, htt } } -func (bs *BuildScanService) getResultsPostRequestFunc(paramsBytes []byte, httpClientsDetails *httputils.HttpClientDetails, queryParams []string) func() (*http.Response, []byte, error) { +func (bs *BuildScanService) getResultsPostRequestFunc(params XrayBuildParams, paramsBytes []byte, httpClientsDetails *httputils.HttpClientDetails, queryParams []string) func() (*http.Response, []byte, error) { endPoint := fmt.Sprintf("%s%s/%s", bs.XrayDetails.GetUrl(), BuildScanAPI, buildScanResultsPostApi) + if params.Project != "" { + queryParams = append(queryParams, projectKeyQueryParam+params.Project) + } if len(queryParams) > 0 { endPoint += "?" + strings.Join(queryParams, "&") }