Skip to content

Commit a6548bc

Browse files
williammartinCopilot
williammartin
and
Copilot
committed
Route autolink requests through api.Client
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 598c3576-22dc-4d7f-a72d-4f84922174a1
1 parent d387926 commit a6548bc

8 files changed

Lines changed: 138 additions & 117 deletions

File tree

‎pkg/cmd/repo/autolink/create/http.go‎

Lines changed: 10 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"net/http"
88

99
"github.com/cli/cli/v2/api"
10-
"github.com/cli/cli/v2/internal/ghinstance"
1110
"github.com/cli/cli/v2/internal/ghrepo"
1211
"github.com/cli/cli/v2/internal/safeurl"
1312
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
@@ -24,7 +23,7 @@ type AutolinkCreateRequest struct {
2423
}
2524

2625
func (a *AutolinkCreator) Create(repo ghrepo.Interface, request AutolinkCreateRequest) (*shared.Autolink, error) {
27-
url, err := safeurl.JoinPathWithHostPrefix(ghinstance.RESTPrefix(repo.RepoHost()), "repos", repo.RepoOwner(), repo.RepoName(), "autolinks")
26+
path, err := safeurl.JoinPath("repos", repo.RepoOwner(), repo.RepoName(), "autolinks")
2827
if err != nil {
2928
return nil, err
3029
}
@@ -35,47 +34,19 @@ func (a *AutolinkCreator) Create(repo ghrepo.Interface, request AutolinkCreateRe
3534
}
3635
requestBody := bytes.NewReader(requestByte)
3736

38-
req, err := http.NewRequest(http.MethodPost, url.String(), requestBody)
39-
if err != nil {
40-
return nil, err
41-
}
42-
43-
resp, err := a.HTTPClient.Do(req)
44-
if err != nil {
45-
return nil, err
46-
}
47-
48-
defer resp.Body.Close()
49-
50-
err = handleAutolinkCreateError(resp)
51-
52-
if err != nil {
53-
return nil, err
54-
}
55-
5637
var autolink shared.Autolink
57-
58-
err = json.NewDecoder(resp.Body).Decode(&autolink)
38+
// TODO(api-client-rollout)
39+
// This line of code is part of a mechanical roll out of the api client.
40+
// As a follow up, consider whether the api client can be injected to this call site, rather than constructed
41+
err = api.NewClientFromHTTP(a.HTTPClient).REST(repo.RepoHost(), http.MethodPost, path.String(), requestBody, &autolink)
5942
if err != nil {
60-
return nil, err
61-
}
62-
63-
return &autolink, nil
64-
}
65-
66-
func handleAutolinkCreateError(resp *http.Response) error {
67-
switch resp.StatusCode {
68-
case http.StatusCreated:
69-
return nil
70-
case http.StatusNotFound:
71-
err := api.HandleHTTPError(resp)
7243
var httpErr api.HTTPError
73-
if errors.As(err, &httpErr) {
44+
if errors.As(err, &httpErr) && httpErr.StatusCode == http.StatusNotFound {
7445
httpErr.Message = "Must have admin rights to Repository."
75-
return httpErr
46+
return nil, httpErr
7647
}
77-
return err
78-
default:
79-
return api.HandleHTTPError(resp)
48+
return nil, err
8049
}
50+
51+
return &autolink, nil
8152
}

‎pkg/cmd/repo/autolink/create/http_test.go‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"testing"
77

88
"github.com/MakeNowJust/heredoc"
9+
"github.com/cli/cli/v2/api"
910
"github.com/cli/cli/v2/internal/ghrepo"
1011
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
1112
"github.com/cli/cli/v2/pkg/httpmock"
@@ -25,6 +26,7 @@ func TestAutolinkCreator_Create(t *testing.T) {
2526
expectedAutolink *shared.Autolink
2627
expectErr bool
2728
expectedErrMsg string
29+
expectedStatus int
2830
}{
2931
{
3032
name: "201 successful creation",
@@ -68,7 +70,8 @@ func TestAutolinkCreator_Create(t *testing.T) {
6870
"documentation_url": "https://docs.github.com/rest/repos/autolinks#create-an-autolink-reference-for-a-repository",
6971
"status": "422"
7072
}`,
71-
expectErr: true,
73+
expectErr: true,
74+
expectedStatus: http.StatusUnprocessableEntity,
7275
expectedErrMsg: heredoc.Doc(`
7376
HTTP 422: Validation Failed (https://api.github.com/repos/OWNER/REPO/autolinks)
7477
url_template must be an absolute URL`),
@@ -88,6 +91,7 @@ func TestAutolinkCreator_Create(t *testing.T) {
8891
}`,
8992
expectErr: true,
9093
expectedErrMsg: "HTTP 404: Must have admin rights to Repository. (https://api.github.com/repos/OWNER/REPO/autolinks)",
94+
expectedStatus: http.StatusNotFound,
9195
},
9296
{
9397
name: "422 URL template missing <num>",
@@ -96,9 +100,10 @@ func TestAutolinkCreator_Create(t *testing.T) {
96100
KeyPrefix: "TICKET-",
97101
URLTemplate: "https://example.com/TICKET",
98102
},
99-
stubStatus: http.StatusUnprocessableEntity,
100-
stubRespJSON: `{"message":"Validation Failed","errors":[{"resource":"KeyLink","code":"custom","field":"url_template","message":"url_template is missing a <num> token"}],"documentation_url":"https://docs.github.com/rest/repos/autolinks#create-an-autolink-reference-for-a-repository","status":"422"}`,
101-
expectErr: true,
103+
stubStatus: http.StatusUnprocessableEntity,
104+
stubRespJSON: `{"message":"Validation Failed","errors":[{"resource":"KeyLink","code":"custom","field":"url_template","message":"url_template is missing a <num> token"}],"documentation_url":"https://docs.github.com/rest/repos/autolinks#create-an-autolink-reference-for-a-repository","status":"422"}`,
105+
expectErr: true,
106+
expectedStatus: http.StatusUnprocessableEntity,
102107
expectedErrMsg: heredoc.Doc(`
103108
HTTP 422: Validation Failed (https://api.github.com/repos/OWNER/REPO/autolinks)
104109
url_template is missing a <num> token`),
@@ -110,9 +115,10 @@ func TestAutolinkCreator_Create(t *testing.T) {
110115
KeyPrefix: "TICKET-",
111116
URLTemplate: "https://example.com/TICKET?query=<num>",
112117
},
113-
stubStatus: http.StatusUnprocessableEntity,
114-
stubRespJSON: `{"message":"Validation Failed","errors":[{"resource":"KeyLink","code":"already_exists","field":"key_prefix"}],"documentation_url":"https://docs.github.com/rest/repos/autolinks#create-an-autolink-reference-for-a-repository","status":"422"}`,
115-
expectErr: true,
118+
stubStatus: http.StatusUnprocessableEntity,
119+
stubRespJSON: `{"message":"Validation Failed","errors":[{"resource":"KeyLink","code":"already_exists","field":"key_prefix"}],"documentation_url":"https://docs.github.com/rest/repos/autolinks#create-an-autolink-reference-for-a-repository","status":"422"}`,
120+
expectErr: true,
121+
expectedStatus: http.StatusUnprocessableEntity,
116122
expectedErrMsg: heredoc.Doc(`
117123
HTTP 422: Validation Failed (https://api.github.com/repos/OWNER/REPO/autolinks)
118124
KeyLink.key_prefix already exists`),
@@ -146,6 +152,9 @@ func TestAutolinkCreator_Create(t *testing.T) {
146152

147153
if tt.expectErr {
148154
require.EqualError(t, err, tt.expectedErrMsg)
155+
var httpErr api.HTTPError
156+
require.ErrorAs(t, err, &httpErr)
157+
assert.Equal(t, tt.expectedStatus, httpErr.StatusCode)
149158
} else {
150159
require.NoError(t, err)
151160
assert.Equal(t, tt.expectedAutolink, autolink)

‎pkg/cmd/repo/autolink/delete/http.go‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
package delete
22

33
import (
4+
"errors"
45
"fmt"
56
"net/http"
67

78
"github.com/cli/cli/v2/api"
8-
"github.com/cli/cli/v2/internal/ghinstance"
99
"github.com/cli/cli/v2/internal/ghrepo"
1010
"github.com/cli/cli/v2/internal/safeurl"
1111
)
@@ -15,26 +15,22 @@ type AutolinkDeleter struct {
1515
}
1616

1717
func (a *AutolinkDeleter) Delete(repo ghrepo.Interface, id string) error {
18-
url, err := safeurl.JoinPathWithHostPrefix(ghinstance.RESTPrefix(repo.RepoHost()), "repos", repo.RepoOwner(), repo.RepoName(), "autolinks", id)
19-
if err != nil {
20-
return err
21-
}
22-
req, err := http.NewRequest(http.MethodDelete, url.String(), nil)
18+
path, err := safeurl.JoinPath("repos", repo.RepoOwner(), repo.RepoName(), "autolinks", id)
2319
if err != nil {
2420
return err
2521
}
2622

27-
resp, err := a.HTTPClient.Do(req)
23+
// TODO(api-client-rollout)
24+
// This line of code is part of a mechanical roll out of the api client.
25+
// As a follow up, consider whether the api client can be injected to this call site, rather than constructed
26+
err = api.NewClientFromHTTP(a.HTTPClient).REST(repo.RepoHost(), http.MethodDelete, path.String(), nil, nil)
2827
if err != nil {
28+
var httpErr api.HTTPError
29+
if errors.As(err, &httpErr) && httpErr.StatusCode == http.StatusNotFound {
30+
return fmt.Errorf("error deleting autolink: HTTP 404: Perhaps you are missing admin rights to the repository? (%s)", httpErr.RequestURL)
31+
}
2932
return err
3033
}
31-
defer resp.Body.Close()
32-
33-
if resp.StatusCode == http.StatusNotFound {
34-
return fmt.Errorf("error deleting autolink: HTTP 404: Perhaps you are missing admin rights to the repository? (%s)", url)
35-
} else if resp.StatusCode > 299 {
36-
return api.HandleHTTPError(resp)
37-
}
3834

3935
return nil
4036
}

‎pkg/cmd/repo/autolink/delete/http_test.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ import (
55
"net/http"
66
"testing"
77

8+
cliapi "github.com/cli/cli/v2/api"
89
"github.com/cli/cli/v2/internal/ghrepo"
910
"github.com/cli/cli/v2/pkg/httpmock"
10-
"github.com/cli/go-gh/v2/pkg/api"
11+
ghapi "github.com/cli/go-gh/v2/pkg/api"
12+
"github.com/stretchr/testify/assert"
1113
"github.com/stretchr/testify/require"
1214
)
1315

@@ -22,6 +24,7 @@ func TestAutolinkDeleter_Delete(t *testing.T) {
2224

2325
expectErr bool
2426
expectedErrMsg string
27+
expectedStatus int
2528
}{
2629
{
2730
name: "204 successful delete",
@@ -38,12 +41,13 @@ func TestAutolinkDeleter_Delete(t *testing.T) {
3841
{
3942
name: "500 unexpected error",
4043
id: "123",
41-
stubResp: api.HTTPError{
44+
stubResp: ghapi.HTTPError{
4245
Message: "arbitrary error",
4346
},
4447
stubStatus: http.StatusInternalServerError,
4548
expectErr: true,
4649
expectedErrMsg: "HTTP 500: arbitrary error (https://api.github.com/repos/OWNER/REPO/autolinks/123)",
50+
expectedStatus: http.StatusInternalServerError,
4751
},
4852
}
4953

@@ -67,6 +71,12 @@ func TestAutolinkDeleter_Delete(t *testing.T) {
6771

6872
if tt.expectErr {
6973
require.EqualError(t, err, tt.expectedErrMsg)
74+
if tt.expectedStatus != 0 {
75+
var httpErr cliapi.HTTPError
76+
require.ErrorAs(t, err, &httpErr)
77+
assert.Equal(t, tt.expectedStatus, httpErr.StatusCode)
78+
assert.Contains(t, err.Error(), "HTTP 500")
79+
}
7080
} else {
7181
require.NoError(t, err)
7282
}

‎pkg/cmd/repo/autolink/list/http.go‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
package list
22

33
import (
4-
"encoding/json"
4+
"errors"
55
"fmt"
66
"net/http"
77

88
"github.com/cli/cli/v2/api"
9-
"github.com/cli/cli/v2/internal/ghinstance"
109
"github.com/cli/cli/v2/internal/ghrepo"
1110
"github.com/cli/cli/v2/internal/safeurl"
1211
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
@@ -17,29 +16,21 @@ type AutolinkLister struct {
1716
}
1817

1918
func (a *AutolinkLister) List(repo ghrepo.Interface) ([]shared.Autolink, error) {
20-
url, err := safeurl.JoinPathWithHostPrefix(ghinstance.RESTPrefix(repo.RepoHost()), "repos", repo.RepoOwner(), repo.RepoName(), "autolinks")
19+
path, err := safeurl.JoinPath("repos", repo.RepoOwner(), repo.RepoName(), "autolinks")
2120
if err != nil {
2221
return nil, err
2322
}
24-
req, err := http.NewRequest(http.MethodGet, url.String(), nil)
25-
if err != nil {
26-
return nil, err
27-
}
28-
29-
resp, err := a.HTTPClient.Do(req)
30-
if err != nil {
31-
return nil, err
32-
}
33-
defer resp.Body.Close()
3423

35-
if resp.StatusCode == http.StatusNotFound {
36-
return nil, fmt.Errorf("error getting autolinks: HTTP 404: Perhaps you are missing admin rights to the repository? (%s)", url)
37-
} else if resp.StatusCode > 299 {
38-
return nil, api.HandleHTTPError(resp)
39-
}
4024
var autolinks []shared.Autolink
41-
err = json.NewDecoder(resp.Body).Decode(&autolinks)
25+
// TODO(api-client-rollout)
26+
// This line of code is part of a mechanical roll out of the api client.
27+
// As a follow up, consider whether the api client can be injected to this call site, rather than constructed
28+
err = api.NewClientFromHTTP(a.HTTPClient).REST(repo.RepoHost(), http.MethodGet, path.String(), nil, &autolinks)
4229
if err != nil {
30+
var httpErr api.HTTPError
31+
if errors.As(err, &httpErr) && httpErr.StatusCode == http.StatusNotFound {
32+
return nil, fmt.Errorf("error getting autolinks: HTTP 404: Perhaps you are missing admin rights to the repository? (%s)", httpErr.RequestURL)
33+
}
4334
return nil, err
4435
}
4536

‎pkg/cmd/repo/autolink/list/http_test.go‎

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"net/http"
66
"testing"
77

8+
"github.com/cli/cli/v2/api"
89
"github.com/cli/cli/v2/internal/ghrepo"
910
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
1011
"github.com/cli/cli/v2/pkg/httpmock"
@@ -14,16 +15,20 @@ import (
1415

1516
func TestAutolinkLister_List(t *testing.T) {
1617
tests := []struct {
17-
name string
18-
repo ghrepo.Interface
19-
resp []shared.Autolink
20-
status int
18+
name string
19+
repo ghrepo.Interface
20+
resp any
21+
status int
22+
expectedAutolinks []shared.Autolink
23+
expectedErrMsg string
24+
expectedStatus int
2125
}{
2226
{
23-
name: "no autolinks",
24-
repo: ghrepo.New("OWNER", "REPO"),
25-
resp: []shared.Autolink{},
26-
status: http.StatusOK,
27+
name: "no autolinks",
28+
repo: ghrepo.New("OWNER", "REPO"),
29+
resp: []shared.Autolink{},
30+
status: http.StatusOK,
31+
expectedAutolinks: []shared.Autolink{},
2732
},
2833
{
2934
name: "two autolinks",
@@ -43,11 +48,39 @@ func TestAutolinkLister_List(t *testing.T) {
4348
},
4449
},
4550
status: http.StatusOK,
51+
expectedAutolinks: []shared.Autolink{
52+
{
53+
ID: 1,
54+
IsAlphanumeric: true,
55+
KeyPrefix: "key",
56+
URLTemplate: "https://example.com",
57+
},
58+
{
59+
ID: 2,
60+
IsAlphanumeric: false,
61+
KeyPrefix: "key2",
62+
URLTemplate: "https://example2.com",
63+
},
64+
},
65+
},
66+
{
67+
name: "404 repo not found",
68+
repo: ghrepo.New("OWNER", "REPO"),
69+
resp: map[string]any{
70+
"message": "Not Found",
71+
},
72+
status: http.StatusNotFound,
73+
expectedErrMsg: "error getting autolinks: HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/repos/OWNER/REPO/autolinks)",
4674
},
4775
{
48-
name: "http error",
49-
repo: ghrepo.New("OWNER", "REPO"),
50-
status: http.StatusNotFound,
76+
name: "500 unexpected error",
77+
repo: ghrepo.New("OWNER", "REPO"),
78+
resp: map[string]any{
79+
"message": "arbitrary error",
80+
},
81+
status: http.StatusInternalServerError,
82+
expectedErrMsg: "HTTP 500: arbitrary error (https://api.github.com/repos/OWNER/REPO/autolinks)",
83+
expectedStatus: http.StatusInternalServerError,
5184
},
5285
}
5386

@@ -64,12 +97,17 @@ func TestAutolinkLister_List(t *testing.T) {
6497
HTTPClient: &http.Client{Transport: reg},
6598
}
6699
autolinks, err := autolinkLister.List(tt.repo)
67-
if tt.status == http.StatusNotFound {
68-
require.Error(t, err)
69-
assert.Equal(t, "error getting autolinks: HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/repos/OWNER/REPO/autolinks)", err.Error())
100+
if tt.expectedErrMsg != "" {
101+
require.EqualError(t, err, tt.expectedErrMsg)
102+
if tt.expectedStatus != 0 {
103+
var httpErr api.HTTPError
104+
require.ErrorAs(t, err, &httpErr)
105+
assert.Equal(t, tt.expectedStatus, httpErr.StatusCode)
106+
assert.Contains(t, err.Error(), "HTTP 500")
107+
}
70108
} else {
71109
require.NoError(t, err)
72-
assert.Equal(t, tt.resp, autolinks)
110+
assert.Equal(t, tt.expectedAutolinks, autolinks)
73111
}
74112
})
75113
}

0 commit comments

Comments
 (0)