Skip to content

Commit 63d84e5

Browse files
harryzcyharryzcy
committed
Simplify adapter logic since only one is active
1 parent c4c51fc commit 63d84e5

File tree

2 files changed

+47
-67
lines changed

2 files changed

+47
-67
lines changed

‎modules/lfs/http_client.go

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,17 @@ import (
2020

2121
const httpBatchSize = 20
2222

23+
var (
24+
ErrUnexpectedEOF = errors.New("unexpected EOF in response")
25+
)
26+
2327
// HTTPClient is used to communicate with the LFS server
2428
// https://github.com/git-lfs/git-lfs/blob/main/docs/api/batch.md
2529
type HTTPClient struct {
26-
client *http.Client
27-
endpoint string
28-
transfers map[string]TransferAdapter
30+
client *http.Client
31+
endpoint string
32+
transfers []string
33+
activeAdapter TransferAdapter
2934
}
3035

3136
// BatchSize returns the preferred size of batchs to process
@@ -44,36 +49,23 @@ func newHTTPClient(endpoint *url.URL, httpTransport *http.Transport) *HTTPClient
4449
Transport: httpTransport,
4550
}
4651

52+
basic := &BasicTransferAdapter{hc}
4753
client := &HTTPClient{
48-
client: hc,
49-
endpoint: strings.TrimSuffix(endpoint.String(), "/"),
50-
transfers: make(map[string]TransferAdapter),
54+
client: hc,
55+
endpoint: strings.TrimSuffix(endpoint.String(), "/"),
56+
transfers: []string{basic.Name()},
57+
activeAdapter: basic,
5158
}
5259

53-
basic := &BasicTransferAdapter{hc}
54-
client.transfers[basic.Name()] = basic
55-
5660
return client
5761
}
5862

59-
func (c *HTTPClient) transferNames() []string {
60-
keys := make([]string, len(c.transfers))
61-
62-
i := 0
63-
for k := range c.transfers {
64-
keys[i] = k
65-
i++
66-
}
67-
68-
return keys
69-
}
70-
7163
func (c *HTTPClient) batch(ctx context.Context, operation string, objects []Pointer) (*BatchResponse, error) {
7264
log.Trace("BATCH operation with objects: %v", objects)
7365

7466
url := fmt.Sprintf("%s/objects/batch", c.endpoint)
7567

76-
request := &BatchRequest{operation, c.transferNames(), nil, objects}
68+
request := &BatchRequest{operation, c.transfers, nil, objects}
7769
payload := new(bytes.Buffer)
7870
err := json.NewEncoder(payload).Encode(request)
7971
if err != nil {
@@ -131,11 +123,6 @@ func (c *HTTPClient) performOperation(ctx context.Context, objects []Pointer, dc
131123
return err
132124
}
133125

134-
transferAdapter, ok := c.transfers[result.Transfer]
135-
if !ok {
136-
return fmt.Errorf("TransferAdapter not found: %s", result.Transfer)
137-
}
138-
139126
for _, object := range result.Objects {
140127
if object.Error != nil {
141128
objectError := errors.New(object.Error.Message)
@@ -169,15 +156,15 @@ func (c *HTTPClient) performOperation(ctx context.Context, objects []Pointer, dc
169156
return err
170157
}
171158

172-
err = transferAdapter.Upload(ctx, link, object.Pointer, content)
159+
err = c.activeAdapter.Upload(ctx, link, object.Pointer, content)
173160

174161
if err != nil {
175162
return err
176163
}
177164

178165
link, ok = object.Actions["verify"]
179166
if ok {
180-
if err := transferAdapter.Verify(ctx, link, object.Pointer); err != nil {
167+
if err := c.activeAdapter.Verify(ctx, link, object.Pointer); err != nil {
181168
return err
182169
}
183170
}
@@ -188,7 +175,7 @@ func (c *HTTPClient) performOperation(ctx context.Context, objects []Pointer, dc
188175
return errors.New("missing action 'download'")
189176
}
190177

191-
content, err := transferAdapter.Download(ctx, link)
178+
content, err := c.activeAdapter.Download(ctx, link)
192179
if err != nil {
193180
return err
194181
}
@@ -250,6 +237,9 @@ func handleErrorResponse(resp *http.Response) error {
250237
var er ErrorResponse
251238
err := json.NewDecoder(resp.Body).Decode(&er)
252239
if err != nil {
240+
if err == io.EOF {
241+
return ErrUnexpectedEOF
242+
}
253243
log.Error("Error decoding json: %v", err)
254244
return err
255245
}

‎modules/lfs/http_client_test.go

Lines changed: 27 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func TestHTTPClientDownload(t *testing.T) {
177177
// case 0
178178
{
179179
endpoint: "https://status-not-ok.io",
180-
expectederror: "Unexpected server response: ",
180+
expectederror: ErrUnexpectedEOF.Error(),
181181
},
182182
// case 1
183183
{
@@ -195,49 +195,44 @@ func TestHTTPClientDownload(t *testing.T) {
195195
expectederror: "",
196196
},
197197
// case 4
198-
{
199-
endpoint: "https://unknown-transfer-adapter.io",
200-
expectederror: "TransferAdapter not found: ",
201-
},
202-
// case 5
203198
{
204199
endpoint: "https://error-in-response-objects.io",
205200
expectederror: "Object not found",
206201
},
207-
// case 6
202+
// case 5
208203
{
209204
endpoint: "https://empty-actions-map.io",
210-
expectederror: "Missing action 'download'",
205+
expectederror: "missing action 'download'",
211206
},
212-
// case 7
207+
// case 6
213208
{
214209
endpoint: "https://download-actions-map.io",
215210
expectederror: "",
216211
},
217-
// case 8
212+
// case 7
218213
{
219214
endpoint: "https://upload-actions-map.io",
220-
expectederror: "Missing action 'download'",
215+
expectederror: "missing action 'download'",
221216
},
222-
// case 9
217+
// case 8
223218
{
224219
endpoint: "https://verify-actions-map.io",
225-
expectederror: "Missing action 'download'",
220+
expectederror: "missing action 'download'",
226221
},
227-
// case 10
222+
// case 9
228223
{
229224
endpoint: "https://unknown-actions-map.io",
230-
expectederror: "Missing action 'download'",
225+
expectederror: "missing action 'download'",
231226
},
232227
}
233228

234229
for n, c := range cases {
235230
client := &HTTPClient{
236-
client: hc,
237-
endpoint: c.endpoint,
238-
transfers: make(map[string]TransferAdapter),
231+
client: hc,
232+
endpoint: c.endpoint,
233+
transfers: []string{"dummy"},
234+
activeAdapter: dummy,
239235
}
240-
client.transfers["dummy"] = dummy
241236

242237
err := client.Download(context.Background(), []Pointer{p}, func(p Pointer, content io.ReadCloser, objectError error) error {
243238
if objectError != nil {
@@ -284,7 +279,7 @@ func TestHTTPClientUpload(t *testing.T) {
284279
// case 0
285280
{
286281
endpoint: "https://status-not-ok.io",
287-
expectederror: "Unexpected server response: ",
282+
expectederror: ErrUnexpectedEOF.Error(),
288283
},
289284
// case 1
290285
{
@@ -302,49 +297,44 @@ func TestHTTPClientUpload(t *testing.T) {
302297
expectederror: "",
303298
},
304299
// case 4
305-
{
306-
endpoint: "https://unknown-transfer-adapter.io",
307-
expectederror: "TransferAdapter not found: ",
308-
},
309-
// case 5
310300
{
311301
endpoint: "https://error-in-response-objects.io",
312302
expectederror: "Object not found",
313303
},
314-
// case 6
304+
// case 5
315305
{
316306
endpoint: "https://empty-actions-map.io",
317307
expectederror: "",
318308
},
319-
// case 7
309+
// case 6
320310
{
321311
endpoint: "https://download-actions-map.io",
322-
expectederror: "Missing action 'upload'",
312+
expectederror: "missing action 'upload'",
323313
},
324-
// case 8
314+
// case 7
325315
{
326316
endpoint: "https://upload-actions-map.io",
327317
expectederror: "",
328318
},
329-
// case 9
319+
// case 8
330320
{
331321
endpoint: "https://verify-actions-map.io",
332-
expectederror: "Missing action 'upload'",
322+
expectederror: "missing action 'upload'",
333323
},
334-
// case 10
324+
// case 9
335325
{
336326
endpoint: "https://unknown-actions-map.io",
337-
expectederror: "Missing action 'upload'",
327+
expectederror: "missing action 'upload'",
338328
},
339329
}
340330

341331
for n, c := range cases {
342332
client := &HTTPClient{
343-
client: hc,
344-
endpoint: c.endpoint,
345-
transfers: make(map[string]TransferAdapter),
333+
client: hc,
334+
endpoint: c.endpoint,
335+
transfers: []string{"dummy"},
336+
activeAdapter: dummy,
346337
}
347-
client.transfers["dummy"] = dummy
348338

349339
err := client.Upload(context.Background(), []Pointer{p}, func(p Pointer, objectError error) (io.ReadCloser, error) {
350340
return io.NopCloser(new(bytes.Buffer)), objectError

0 commit comments

Comments
 (0)