Skip to content

Commit f35ad42

Browse files
harryzcyharryzcy
committed
Refactor to avoid callback logic
1 parent 080bf89 commit f35ad42

File tree

2 files changed

+90
-99
lines changed

2 files changed

+90
-99
lines changed

‎modules/lfs/http_client.go

Lines changed: 64 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"context"
99
"errors"
1010
"fmt"
11+
"io"
1112
"net/http"
1213
"net/url"
1314
"strings"
@@ -17,7 +18,7 @@ import (
1718
"code.gitea.io/gitea/modules/proxy"
1819
)
1920

20-
const batchSize = 20
21+
const httpBatchSize = 20
2122

2223
// HTTPClient is used to communicate with the LFS server
2324
// https://github.com/git-lfs/git-lfs/blob/main/docs/api/batch.md
@@ -29,7 +30,7 @@ type HTTPClient struct {
2930

3031
// BatchSize returns the preferred size of batchs to process
3132
func (c *HTTPClient) BatchSize() int {
32-
return batchSize
33+
return httpBatchSize
3334
}
3435

3536
func newHTTPClient(endpoint *url.URL, httpTransport *http.Transport) *HTTPClient {
@@ -50,7 +51,6 @@ func newHTTPClient(endpoint *url.URL, httpTransport *http.Transport) *HTTPClient
5051
}
5152

5253
basic := &BasicTransferAdapter{hc}
53-
5454
client.transfers[basic.Name()] = basic
5555

5656
return client
@@ -82,32 +82,17 @@ func (c *HTTPClient) batch(ctx context.Context, operation string, objects []Poin
8282
return nil, err
8383
}
8484

85-
log.Trace("Calling: %s", url)
86-
87-
req, err := http.NewRequestWithContext(ctx, "POST", url, payload)
85+
req, err := createRequest(ctx, http.MethodPost, url, MediaType, nil, payload)
8886
if err != nil {
89-
log.Error("Error creating request: %v", err)
9087
return nil, err
9188
}
92-
req.Header.Set("Content-type", MediaType)
93-
req.Header.Set("Accept", MediaType)
9489

95-
res, err := c.client.Do(req)
90+
res, err := performRequest(ctx, c.client, req)
9691
if err != nil {
97-
select {
98-
case <-ctx.Done():
99-
return nil, ctx.Err()
100-
default:
101-
}
102-
log.Error("Error while processing request: %v", err)
10392
return nil, err
10493
}
10594
defer res.Body.Close()
10695

107-
if res.StatusCode != http.StatusOK {
108-
return nil, fmt.Errorf("unexpected server response: %s", res.Status)
109-
}
110-
11196
var response BatchResponse
11297
err = json.NewDecoder(res.Body).Decode(&response)
11398
if err != nil {
@@ -219,3 +204,62 @@ func (c *HTTPClient) performOperation(ctx context.Context, objects []Pointer, dc
219204

220205
return nil
221206
}
207+
208+
// createRequest creates a new request, and sets the Content-Type and Accept headers.
209+
func createRequest(ctx context.Context, method, url, contentType string, headers map[string]string, body io.Reader) (*http.Request, error) {
210+
log.Trace("createRequest: %s", url)
211+
req, err := http.NewRequestWithContext(ctx, method, url, body)
212+
if err != nil {
213+
log.Error("Error creating request: %v", err)
214+
return nil, err
215+
}
216+
217+
for key, value := range headers {
218+
req.Header.Set(key, value)
219+
}
220+
if req.Header.Get("Content-Type") == "" {
221+
req.Header.Set("Content-Type", contentType)
222+
}
223+
req.Header.Set("Accept", MediaType)
224+
225+
return req, nil
226+
}
227+
228+
// performRequest sends a request, optionally performs a callback on the request and returns the response.
229+
// If the status code is 200, the response is returned, and it will contain a non-nil Body.
230+
// Otherwise, it will return an error, and the Body will be nil or closed.
231+
func performRequest(ctx context.Context, client *http.Client, req *http.Request) (*http.Response, error) {
232+
log.Trace("performRequest: %s", req.URL)
233+
res, err := client.Do(req)
234+
if err != nil {
235+
select {
236+
case <-ctx.Done():
237+
return res, ctx.Err()
238+
default:
239+
}
240+
log.Error("Error while processing request: %v", err)
241+
return res, err
242+
}
243+
244+
if res.StatusCode != http.StatusOK {
245+
defer res.Body.Close()
246+
return res, handleErrorResponse(res)
247+
}
248+
249+
return res, nil
250+
}
251+
252+
func handleErrorResponse(resp *http.Response) error {
253+
var er ErrorResponse
254+
err := json.NewDecoder(resp.Body).Decode(&er)
255+
if err != nil {
256+
log.Error("Error decoding json: %v", err)
257+
return err
258+
}
259+
260+
if err != nil {
261+
return fmt.Errorf("request failed with status %s", resp.Status)
262+
}
263+
log.Trace("ErrorResponse: %v", er)
264+
return errors.New(er.Message)
265+
}

‎modules/lfs/transferadapter.go

Lines changed: 26 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -6,55 +6,57 @@ package lfs
66
import (
77
"bytes"
88
"context"
9-
"errors"
10-
"fmt"
119
"io"
1210
"net/http"
1311

1412
"code.gitea.io/gitea/modules/json"
1513
"code.gitea.io/gitea/modules/log"
1614
)
1715

18-
// TransferAdapter represents an adapter for downloading/uploading LFS objects
16+
// TransferAdapter represents an adapter for downloading/uploading LFS objects.
1917
type TransferAdapter interface {
2018
Name() string
2119
Download(ctx context.Context, l *Link) (io.ReadCloser, error)
2220
Upload(ctx context.Context, l *Link, p Pointer, r io.Reader) error
2321
Verify(ctx context.Context, l *Link, p Pointer) error
2422
}
2523

26-
// BasicTransferAdapter implements the "basic" adapter
24+
// BasicTransferAdapter implements the "basic" adapter.
2725
type BasicTransferAdapter struct {
2826
client *http.Client
2927
}
3028

31-
// Name returns the name of the adapter
29+
// Name returns the name of the adapter.
3230
func (a *BasicTransferAdapter) Name() string {
3331
return "basic"
3432
}
3533

36-
// Download reads the download location and downloads the data
34+
// Download reads the download location and downloads the data.
3735
func (a *BasicTransferAdapter) Download(ctx context.Context, l *Link) (io.ReadCloser, error) {
38-
resp, err := a.performRequest(ctx, "GET", l, nil, nil)
36+
req, err := createRequest(ctx, http.MethodGet, l.Href, "", nil, nil)
37+
if err != nil {
38+
return nil, err
39+
}
40+
resp, err := performRequest(ctx, a.client, req)
3941
if err != nil {
4042
return nil, err
4143
}
4244
return resp.Body, nil
4345
}
4446

45-
// Upload sends the content to the LFS server
47+
// Upload sends the content to the LFS server.
4648
func (a *BasicTransferAdapter) Upload(ctx context.Context, l *Link, p Pointer, r io.Reader) error {
47-
res, err := a.performRequest(ctx, "PUT", l, r, func(req *http.Request) {
48-
if len(req.Header.Get("Content-Type")) == 0 {
49-
req.Header.Set("Content-Type", "application/octet-stream")
50-
}
51-
52-
if req.Header.Get("Transfer-Encoding") == "chunked" {
53-
req.TransferEncoding = []string{"chunked"}
54-
}
49+
contentType := "application/octet-stream"
50+
req, err := createRequest(ctx, http.MethodPut, l.Href, contentType, nil, r)
51+
if err != nil {
52+
return err
53+
}
54+
if req.Header.Get("Transfer-Encoding") == "chunked" {
55+
req.TransferEncoding = []string{"chunked"}
56+
}
57+
req.ContentLength = p.Size
5558

56-
req.ContentLength = p.Size
57-
})
59+
res, err := performRequest(ctx, a.client, req)
5860
if err != nil {
5961
return err
6062
}
@@ -70,70 +72,15 @@ func (a *BasicTransferAdapter) Verify(ctx context.Context, l *Link, p Pointer) e
7072
return err
7173
}
7274

73-
res, err := a.performRequest(ctx, "POST", l, bytes.NewReader(b), func(req *http.Request) {
74-
req.Header.Set("Content-Type", MediaType)
75-
})
75+
contentType := MediaType
76+
req, err := createRequest(ctx, http.MethodPost, l.Href, contentType, nil, bytes.NewReader(b))
7677
if err != nil {
7778
return err
7879
}
79-
defer res.Body.Close()
80-
return nil
81-
}
82-
83-
// performRequest sends a request, optionally performs a callback on the request and returns the response.
84-
// If the status code is 200, the response is returned, and it will contain a non-nil Body.
85-
// Otherwise, it will return an error, and the Body will be nil or closed.
86-
func (a *BasicTransferAdapter) performRequest(ctx context.Context, method string, l *Link, body io.Reader, callback func(*http.Request)) (*http.Response, error) {
87-
log.Trace("Calling: %s %s", method, l.Href)
88-
89-
req, err := http.NewRequestWithContext(ctx, method, l.Href, body)
80+
res, err := performRequest(ctx, a.client, req)
9081
if err != nil {
91-
log.Error("Error creating request: %v", err)
92-
return nil, err
93-
}
94-
for key, value := range l.Header {
95-
req.Header.Set(key, value)
96-
}
97-
req.Header.Set("Accept", MediaType)
98-
99-
if callback != nil {
100-
callback(req)
101-
}
102-
103-
res, err := a.client.Do(req)
104-
if err != nil {
105-
select {
106-
case <-ctx.Done():
107-
return res, ctx.Err()
108-
default:
109-
}
110-
log.Error("Error while processing request: %v", err)
111-
return res, err
112-
}
113-
114-
if res.StatusCode != http.StatusOK {
115-
return res, handleErrorResponse(res)
116-
}
117-
118-
return res, nil
119-
}
120-
121-
func handleErrorResponse(resp *http.Response) error {
122-
defer resp.Body.Close()
123-
124-
er, err := decodeResponseError(resp.Body)
125-
if err != nil {
126-
return fmt.Errorf("request failed with status %s", resp.Status)
127-
}
128-
log.Trace("ErrorRespone: %v", er)
129-
return errors.New(er.Message)
130-
}
131-
132-
func decodeResponseError(r io.Reader) (ErrorResponse, error) {
133-
var er ErrorResponse
134-
err := json.NewDecoder(r).Decode(&er)
135-
if err != nil {
136-
log.Error("Error decoding json: %v", err)
82+
return err
13783
}
138-
return er, err
84+
defer res.Body.Close()
85+
return nil
13986
}

0 commit comments

Comments
 (0)