Skip to content

Commit

Permalink
Merge branch 'dev' of https://github.com/jfrog/jfrog-client-go into dev
Browse files Browse the repository at this point in the history
  • Loading branch information
eyalbe4 committed Dec 25, 2023
2 parents 111d2a8 + cc521e0 commit 6561c3e
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 4 deletions.
10 changes: 10 additions & 0 deletions artifactory/services/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
17 changes: 15 additions & 2 deletions http/httpclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
Expand Down
44 changes: 44 additions & 0 deletions http/httpclient/client_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
9 changes: 9 additions & 0 deletions utils/io/httputils/httpclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
7 changes: 5 additions & 2 deletions xray/services/buildscan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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, "&")
}
Expand Down

0 comments on commit 6561c3e

Please sign in to comment.