Address review comments

Author: harryzcy

modules/lfs/http_client.go

@@ -20,15 +20,12 @@ import (
 
 const httpBatchSize = 20
 
-var ErrUnexpectedEOF = errors.New("unexpected EOF in response")
-
 // HTTPClient is used to communicate with the LFS server
 // https://github.com/git-lfs/git-lfs/blob/main/docs/api/batch.md
 type HTTPClient struct {
-	client        *http.Client
-	endpoint      string
-	transfers     map[string]TransferAdapter
-	transferNames []string
+	client    *http.Client
+	endpoint  string
+	transfers map[string]TransferAdapter
 }
 
 // BatchSize returns the preferred size of batchs to process
@@ -54,26 +51,36 @@ func newHTTPClient(endpoint *url.URL, httpTransport *http.Transport) *HTTPClient
 		transfers: map[string]TransferAdapter{
 			basic.Name(): basic,
 		},
-		transferNames: []string{basic.Name()},
 	}
 
 	return client
 }
 
+func (c *HTTPClient) transferNames() []string {
+	keys := make([]string, len(c.transfers))
+	i := 0
+	for k := range c.transfers {
+		keys[i] = k
+		i++
+	}
+	return keys
+}
+
 func (c *HTTPClient) batch(ctx context.Context, operation string, objects []Pointer) (*BatchResponse, error) {
 	log.Trace("BATCH operation with objects: %v", objects)
 
 	url := fmt.Sprintf("%s/objects/batch", c.endpoint)
 
-	request := &BatchRequest{operation, c.transferNames, nil, objects}
+	request := &BatchRequest{operation, c.transferNames(), nil, objects}
 	payload := new(bytes.Buffer)
 	err := json.NewEncoder(payload).Encode(request)
 	if err != nil {
 		log.Error("Error encoding json: %v", err)
 		return nil, err
 	}
 
-	req, err := createRequest(ctx, http.MethodPost, url, MediaType, nil, payload)
+	req, err := createRequest(ctx, http.MethodPost, url, nil, payload)
+	req.Header.Set("Content-Type", MediaType)
 	if err != nil {
 		return nil, err
 	}
@@ -194,8 +201,8 @@ func (c *HTTPClient) performOperation(ctx context.Context, objects []Pointer, dc
 	return nil
 }
 
-// createRequest creates a new request, and sets the Content-Type and Accept headers.
-func createRequest(ctx context.Context, method, url, contentType string, headers map[string]string, body io.Reader) (*http.Request, error) {
+// createRequest creates a new request, and sets the headers.
+func createRequest(ctx context.Context, method, url string, headers map[string]string, body io.Reader) (*http.Request, error) {
 	log.Trace("createRequest: %s", url)
 	req, err := http.NewRequestWithContext(ctx, method, url, body)
 	if err != nil {
@@ -206,9 +213,6 @@ func createRequest(ctx context.Context, method, url, contentType string, headers
 	for key, value := range headers {
 		req.Header.Set(key, value)
 	}
-	if req.Header.Get("Content-Type") == "" {
-		req.Header.Set("Content-Type", contentType)
-	}
 	req.Header.Set("Accept", MediaType)
 
 	return req, nil
@@ -243,7 +247,7 @@ func handleErrorResponse(resp *http.Response) error {
 	err := json.NewDecoder(resp.Body).Decode(&er)
 	if err != nil {
 		if err == io.EOF {
-			return ErrUnexpectedEOF
+			return io.ErrUnexpectedEOF
 		}
 		log.Error("Error decoding json: %v", err)
 		return err

modules/lfs/http_client_test.go

@@ -177,7 +177,7 @@ func TestHTTPClientDownload(t *testing.T) {
 		// case 0
 		{
 			endpoint:      "https://status-not-ok.io",
-			expectederror: ErrUnexpectedEOF.Error(),
+			expectederror: io.ErrUnexpectedEOF.Error(),
 		},
 		// case 1
 		{
@@ -238,7 +238,6 @@ func TestHTTPClientDownload(t *testing.T) {
 			transfers: map[string]TransferAdapter{
 				"dummy": dummy,
 			},
-			transferNames: []string{"dummy"},
 		}
 
 		err := client.Download(context.Background(), []Pointer{p}, func(p Pointer, content io.ReadCloser, objectError error) error {
@@ -286,7 +285,7 @@ func TestHTTPClientUpload(t *testing.T) {
 		// case 0
 		{
 			endpoint:      "https://status-not-ok.io",
-			expectederror: ErrUnexpectedEOF.Error(),
+			expectederror: io.ErrUnexpectedEOF.Error(),
 		},
 		// case 1
 		{
@@ -347,7 +346,6 @@ func TestHTTPClientUpload(t *testing.T) {
 			transfers: map[string]TransferAdapter{
 				"dummy": dummy,
 			},
-			transferNames: []string{"dummy"},
 		}
 
 		err := client.Upload(context.Background(), []Pointer{p}, func(p Pointer, objectError error) (io.ReadCloser, error) {

modules/lfs/transferadapter.go

@@ -33,7 +33,7 @@ func (a *BasicTransferAdapter) Name() string {
 
 // Download reads the download location and downloads the data.
 func (a *BasicTransferAdapter) Download(ctx context.Context, l *Link) (io.ReadCloser, error) {
-	req, err := createRequest(ctx, http.MethodGet, l.Href, "", l.Header, nil)
+	req, err := createRequest(ctx, http.MethodGet, l.Href, l.Header, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -46,8 +46,8 @@ func (a *BasicTransferAdapter) Download(ctx context.Context, l *Link) (io.ReadCl
 
 // Upload sends the content to the LFS server.
 func (a *BasicTransferAdapter) Upload(ctx context.Context, l *Link, p Pointer, r io.Reader) error {
-	contentType := "application/octet-stream"
-	req, err := createRequest(ctx, http.MethodPut, l.Href, contentType, l.Header, r)
+	req, err := createRequest(ctx, http.MethodPut, l.Href, l.Header, r)
+	req.Header.Set("Content-Type", "application/octet-stream")
 	if err != nil {
 		return err
 	}
@@ -72,8 +72,8 @@ func (a *BasicTransferAdapter) Verify(ctx context.Context, l *Link, p Pointer) e
 		return err
 	}
 
-	contentType := MediaType
-	req, err := createRequest(ctx, http.MethodPost, l.Href, contentType, l.Header, bytes.NewReader(b))
+	req, err := createRequest(ctx, http.MethodPost, l.Href, l.Header, bytes.NewReader(b))
+	req.Header.Set("Content-Type", MediaType)
 	if err != nil {
 		return err
 	}