Skip to content

Commit 1738d6e

Browse files
peterguyampcode-com
peterguy
and
ampcode-com
committed
refactor: use parsed *url.URL types instead of string endpoints
- config struct: unexport fields, replace Endpoint/Proxy strings with endpointURL/proxyURL (*url.URL) and proxyPath, parse once in readConfig - api.ClientOpts: Endpoint string → EndpointURL *url.URL - login: accept *url.URL, move endpoint conflict check to handler - code_intel_upload: simplify makeCodeIntelUploadURL with stack copy, remove error return - api.go: use JoinPath for URL construction instead of string concat - Update all call sites and tests Amp-Thread-ID: https://ampcode.com/threads/T-019cc5da-fb05-71f8-962b-689cfc5b494d Co-authored-by: Amp <amp@ampcode.com>
1 parent fcfad36 commit 1738d6e

21 files changed

Lines changed: 300 additions & 233 deletions

‎cmd/src/batch_common.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) (err error
537537
if err != nil {
538538
return execUI.CreatingBatchSpecError(lr.MaxUnlicensedChangesets, err)
539539
}
540-
previewURL := cfg.Endpoint + url
540+
previewURL := cfg.endpointURL.JoinPath(url).String()
541541
execUI.CreatingBatchSpecSuccess(previewURL)
542542

543543
hasWorkspaceFiles := false
@@ -567,7 +567,7 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) (err error
567567
if err != nil {
568568
return err
569569
}
570-
execUI.ApplyingBatchSpecSuccess(cfg.Endpoint + batch.URL)
570+
execUI.ApplyingBatchSpecSuccess(cfg.endpointURL.JoinPath(batch.URL).String())
571571

572572
return nil
573573
}

‎cmd/src/batch_remote.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ Examples:
157157

158158
executionURL := fmt.Sprintf(
159159
"%s/%s/batch-changes/%s/executions/%s",
160-
strings.TrimSuffix(cfg.Endpoint, "/"),
160+
cfg.endpointURL,
161161
strings.TrimPrefix(namespace.URL, "/"),
162162
batchChangeName,
163163
batchSpecID,

‎cmd/src/batch_repositories.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ Examples:
131131
Max: max,
132132
RepoCount: len(repos),
133133
Repos: repos,
134-
SourcegraphEndpoint: cfg.Endpoint,
134+
SourcegraphEndpoint: cfg.endpointURL.String(),
135135
}); err != nil {
136136
return err
137137
}

‎cmd/src/code_intel_upload.go‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"flag"
88
"fmt"
99
"io"
10-
"net/url"
1110
"os"
1211
"strings"
1312
"time"
@@ -87,10 +86,7 @@ func handleCodeIntelUpload(args []string) error {
8786
return handleUploadError(uploadOptions.SourcegraphInstanceOptions.AccessToken, err)
8887
}
8988

90-
uploadURL, err := makeCodeIntelUploadURL(uploadID)
91-
if err != nil {
92-
return err
93-
}
89+
uploadURL := makeCodeIntelUploadURL(uploadID)
9490

9591
if codeintelUploadFlags.json {
9692
serialized, err := json.Marshal(map[string]any{
@@ -132,7 +128,7 @@ func codeintelUploadOptions(out *output.Output) upload.UploadOptions {
132128
associatedIndexID = &codeintelUploadFlags.associatedIndexID
133129
}
134130

135-
cfg.AdditionalHeaders["Content-Type"] = "application/x-protobuf+scip"
131+
cfg.additionalHeaders["Content-Type"] = "application/x-protobuf+scip"
136132

137133
logger := upload.NewRequestLogger(
138134
os.Stdout,
@@ -153,9 +149,9 @@ func codeintelUploadOptions(out *output.Output) upload.UploadOptions {
153149
AssociatedIndexID: associatedIndexID,
154150
},
155151
SourcegraphInstanceOptions: upload.SourcegraphInstanceOptions{
156-
SourcegraphURL: cfg.Endpoint,
157-
AccessToken: cfg.AccessToken,
158-
AdditionalHeaders: cfg.AdditionalHeaders,
152+
SourcegraphURL: cfg.endpointURL.String(),
153+
AccessToken: cfg.accessToken,
154+
AdditionalHeaders: cfg.additionalHeaders,
159155
MaxRetries: 5,
160156
RetryInterval: time.Second,
161157
Path: codeintelUploadFlags.uploadRoute,
@@ -191,16 +187,12 @@ func printInferredArguments(out *output.Output) {
191187

192188
// makeCodeIntelUploadURL constructs a URL to the upload with the given internal identifier.
193189
// The base of the URL is constructed from the configured Sourcegraph instance.
194-
func makeCodeIntelUploadURL(uploadID int) (string, error) {
195-
url, err := url.Parse(cfg.Endpoint)
196-
if err != nil {
197-
return "", err
198-
}
199-
190+
func makeCodeIntelUploadURL(uploadID int) string {
191+
u := *cfg.endpointURL
200192
graphqlID := base64.URLEncoding.EncodeToString(fmt.Appendf(nil, `SCIPUpload:%d`, uploadID))
201-
url.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID
202-
url.User = nil
203-
return url.String(), nil
193+
u.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID
194+
u.User = nil
195+
return u.String()
204196
}
205197

206198
type errorWithHint struct {

‎cmd/src/debug_compose.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ Examples:
7575
return errors.Wrap(err, "failed to get containers for subcommand with err")
7676
}
7777
// Safety check user knows what they are targeting with this debug command
78-
log.Printf("This command will archive docker-cli data for %d containers\n SRC_ENDPOINT: %v\n Output filename: %v", len(containers), cfg.Endpoint, base)
78+
log.Printf("This command will archive docker-cli data for %d containers\n SRC_ENDPOINT: %v\n Output filename: %v", len(containers), cfg.endpointURL, base)
7979
if verified, _ := verify("Do you want to start writing to an archive?"); !verified {
8080
return nil
8181
}

‎cmd/src/debug_kube.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ Examples:
8484
return errors.Wrapf(err, "failed to get current-context")
8585
}
8686
// Safety check user knows what they've targeted with this command
87-
log.Printf("Archiving kubectl data for %d pods\n SRC_ENDPOINT: %v\n Context: %s Namespace: %v\n Output filename: %v", len(pods.Items), cfg.Endpoint, kubectx, namespace, base)
87+
log.Printf("Archiving kubectl data for %d pods\n SRC_ENDPOINT: %v\n Context: %s Namespace: %v\n Output filename: %v", len(pods.Items), cfg.endpointURL, kubectx, namespace, base)
8888
if verified, _ := verify("Do you want to start writing to an archive?"); !verified {
8989
return nil
9090
}

‎cmd/src/debug_server.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ Examples:
7272
defer zw.Close()
7373

7474
// Safety check user knows what they are targeting with this debug command
75-
log.Printf("This command will archive docker-cli data for container: %s\n SRC_ENDPOINT: %s\n Output filename: %s", container, cfg.Endpoint, base)
75+
log.Printf("This command will archive docker-cli data for container: %s\n SRC_ENDPOINT: %s\n Output filename: %s", container, cfg.endpointURL, base)
7676
if verified, _ := verify("Do you want to start writing to an archive?"); !verified {
7777
return nil
7878
}

‎cmd/src/login.go‎

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,21 @@ Examples:
4444
if err := flagSet.Parse(args); err != nil {
4545
return err
4646
}
47-
endpoint := cfg.Endpoint
47+
4848
if flagSet.NArg() >= 1 {
49-
endpoint = flagSet.Arg(0)
50-
}
51-
if endpoint == "" {
52-
return cmderrors.Usage("expected exactly one argument: the Sourcegraph URL, or SRC_ENDPOINT to be set")
49+
arg := flagSet.Arg(0)
50+
parsed, err := parseEndpoint(arg)
51+
if err != nil {
52+
return cmderrors.Usage(fmt.Sprintf("invalid endpoint URL: %s", arg))
53+
}
54+
if parsed.String() != cfg.endpointURL.String() {
55+
return cmderrors.Usage(fmt.Sprintf("endpoint argument %s conflicts with configured endpoint %s", parsed, cfg.endpointURL))
56+
}
5357
}
5458

5559
client := cfg.apiClient(apiFlags, io.Discard)
5660

57-
return loginCmd(context.Background(), cfg, client, endpoint, os.Stdout)
61+
return loginCmd(context.Background(), cfg, client, os.Stdout)
5862
}
5963

6064
commands = append(commands, &command{
@@ -64,9 +68,7 @@ Examples:
6468
})
6569
}
6670

67-
func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg string, out io.Writer) error {
68-
endpointArg = cleanEndpoint(endpointArg)
69-
71+
func loginCmd(ctx context.Context, cfg *config, client api.Client, out io.Writer) error {
7072
printProblem := func(problem string) {
7173
fmt.Fprintf(out, "❌ Problem: %s\n", problem)
7274
}
@@ -77,23 +79,16 @@ func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg s
7779
export SRC_ACCESS_TOKEN=(your access token)
7880
7981
To verify that it's working, run the login command again.
80-
`, endpointArg, endpointArg)
82+
`, cfg.endpointURL, cfg.endpointURL)
8183

82-
if cfg.ConfigFilePath != "" {
84+
if cfg.configFilePath != "" {
8385
fmt.Fprintln(out)
84-
fmt.Fprintf(out, "⚠️ Warning: Configuring src with a JSON file is deprecated. Please migrate to using the env vars SRC_ENDPOINT, SRC_ACCESS_TOKEN, and SRC_PROXY instead, and then remove %s. See https://github.com/sourcegraph/src-cli#readme for more information.\n", cfg.ConfigFilePath)
86+
fmt.Fprintf(out, "⚠️ Warning: Configuring src with a JSON file is deprecated. Please migrate to using the env vars SRC_ENDPOINT, SRC_ACCESS_TOKEN, and SRC_PROXY instead, and then remove %s. See https://github.com/sourcegraph/src-cli#readme for more information.\n", cfg.configFilePath)
8587
}
8688

87-
noToken := cfg.AccessToken == ""
88-
endpointConflict := endpointArg != cfg.Endpoint
89-
if noToken || endpointConflict {
89+
if cfg.accessToken == "" {
9090
fmt.Fprintln(out)
91-
switch {
92-
case noToken:
93-
printProblem("No access token is configured.")
94-
case endpointConflict:
95-
printProblem(fmt.Sprintf("The configured endpoint is %s, not %s.", cfg.Endpoint, endpointArg))
96-
}
91+
printProblem("No access token is configured.")
9792
fmt.Fprintln(out, createAccessTokenMessage)
9893
return cmderrors.ExitCode1
9994
}
@@ -107,7 +102,7 @@ func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg s
107102
if strings.HasPrefix(err.Error(), "error: 401 Unauthorized") || strings.HasPrefix(err.Error(), "error: 403 Forbidden") {
108103
printProblem("Invalid access token.")
109104
} else {
110-
printProblem(fmt.Sprintf("Error communicating with %s: %s", endpointArg, err))
105+
printProblem(fmt.Sprintf("Error communicating with %s: %s", cfg.endpointURL, err))
111106
}
112107
fmt.Fprintln(out, createAccessTokenMessage)
113108
fmt.Fprintln(out, " (If you need to supply custom HTTP request headers, see information about SRC_HEADER_* and SRC_HEADERS env vars at https://github.com/sourcegraph/src-cli/blob/main/AUTH_PROXY.md)")
@@ -117,11 +112,11 @@ func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg s
117112
if result.CurrentUser == nil {
118113
// This should never happen; we verified there is an access token, so there should always be
119114
// a user.
120-
printProblem(fmt.Sprintf("Unable to determine user on %s.", endpointArg))
115+
printProblem(fmt.Sprintf("Unable to determine user on %s.", cfg.endpointURL))
121116
return cmderrors.ExitCode1
122117
}
123118
fmt.Fprintln(out)
124-
fmt.Fprintf(out, "✔️ Authenticated as %s on %s\n", result.CurrentUser.Username, endpointArg)
119+
fmt.Fprintf(out, "✔️ Authenticated as %s on %s\n", result.CurrentUser.Username, cfg.endpointURL)
125120
fmt.Fprintln(out)
126121
return nil
127122
}

‎cmd/src/login_test.go‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,34 +7,25 @@ import (
77
"io"
88
"net/http"
99
"net/http/httptest"
10+
"net/url"
1011
"strings"
1112
"testing"
1213

1314
"github.com/sourcegraph/src-cli/internal/cmderrors"
1415
)
1516

1617
func TestLogin(t *testing.T) {
17-
check := func(t *testing.T, cfg *config, endpointArg string) (output string, err error) {
18+
check := func(t *testing.T, cfg *config) (output string, err error) {
1819
t.Helper()
1920

2021
var out bytes.Buffer
21-
err = loginCmd(context.Background(), cfg, cfg.apiClient(nil, io.Discard), endpointArg, &out)
22+
err = loginCmd(context.Background(), cfg, cfg.apiClient(nil, io.Discard), &out)
2223
return strings.TrimSpace(out.String()), err
2324
}
2425

25-
t.Run("different endpoint in config vs. arg", func(t *testing.T) {
26-
out, err := check(t, &config{Endpoint: "https://example.com"}, "https://sourcegraph.example.com")
27-
if err != cmderrors.ExitCode1 {
28-
t.Fatal(err)
29-
}
30-
wantOut := "❌ Problem: No access token is configured.\n\n🛠 To fix: Create an access token by going to https://sourcegraph.example.com/user/settings/tokens, then set the following environment variables in your terminal:\n\n export SRC_ENDPOINT=https://sourcegraph.example.com\n export SRC_ACCESS_TOKEN=(your access token)\n\n To verify that it's working, run the login command again."
31-
if out != wantOut {
32-
t.Errorf("got output %q, want %q", out, wantOut)
33-
}
34-
})
35-
3626
t.Run("no access token", func(t *testing.T) {
37-
out, err := check(t, &config{Endpoint: "https://example.com"}, "https://sourcegraph.example.com")
27+
endpoint := &url.URL{Scheme: "https", Host: "sourcegraph.example.com"}
28+
out, err := check(t, &config{endpointURL: endpoint})
3829
if err != cmderrors.ExitCode1 {
3930
t.Fatal(err)
4031
}
@@ -45,7 +36,8 @@ func TestLogin(t *testing.T) {
4536
})
4637

4738
t.Run("warning when using config file", func(t *testing.T) {
48-
out, err := check(t, &config{Endpoint: "https://example.com", ConfigFilePath: "f"}, "https://example.com")
39+
endpoint := &url.URL{Scheme: "https", Host: "example.com"}
40+
out, err := check(t, &config{endpointURL: endpoint, configFilePath: "f"})
4941
if err != cmderrors.ExitCode1 {
5042
t.Fatal(err)
5143
}
@@ -62,13 +54,13 @@ func TestLogin(t *testing.T) {
6254
}))
6355
defer s.Close()
6456

65-
endpoint := s.URL
66-
out, err := check(t, &config{Endpoint: endpoint, AccessToken: "x"}, endpoint)
57+
u, _ := url.ParseRequestURI(s.URL)
58+
out, err := check(t, &config{endpointURL: u, accessToken: "x"})
6759
if err != cmderrors.ExitCode1 {
6860
t.Fatal(err)
6961
}
7062
wantOut := "❌ Problem: Invalid access token.\n\n🛠 To fix: Create an access token by going to $ENDPOINT/user/settings/tokens, then set the following environment variables in your terminal:\n\n export SRC_ENDPOINT=$ENDPOINT\n export SRC_ACCESS_TOKEN=(your access token)\n\n To verify that it's working, run the login command again.\n\n (If you need to supply custom HTTP request headers, see information about SRC_HEADER_* and SRC_HEADERS env vars at https://github.com/sourcegraph/src-cli/blob/main/AUTH_PROXY.md)"
71-
wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", endpoint)
63+
wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", s.URL)
7264
if out != wantOut {
7365
t.Errorf("got output %q, want %q", out, wantOut)
7466
}
@@ -81,13 +73,13 @@ func TestLogin(t *testing.T) {
8173
}))
8274
defer s.Close()
8375

84-
endpoint := s.URL
85-
out, err := check(t, &config{Endpoint: endpoint, AccessToken: "x"}, endpoint)
76+
u, _ := url.ParseRequestURI(s.URL)
77+
out, err := check(t, &config{endpointURL: u, accessToken: "x"})
8678
if err != nil {
8779
t.Fatal(err)
8880
}
8981
wantOut := "✔️ Authenticated as alice on $ENDPOINT"
90-
wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", endpoint)
82+
wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", s.URL)
9183
if out != wantOut {
9284
t.Errorf("got output %q, want %q", out, wantOut)
9385
}

0 commit comments

Comments
 (0)