From 8135fdbd993270690702e06a233440038c087d22 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 6 Oct 2021 08:50:42 -0400 Subject: [PATCH 1/4] Switch GetCodespace to new API endpoint - Drop GetCodespaceToken as it is no longer necessary - Introduces new behavior with the API to optionally include connection details in the GET request. Ommitting to do so results in a faster response --- internal/codespaces/api/api.go | 89 ++++++++---------------------- internal/codespaces/codespaces.go | 7 +-- internal/codespaces/states.go | 9 +-- pkg/cmd/codespace/common.go | 28 +++------- pkg/cmd/codespace/create.go | 3 +- pkg/cmd/codespace/delete.go | 14 +---- pkg/cmd/codespace/delete_test.go | 17 +----- pkg/cmd/codespace/logs.go | 4 +- pkg/cmd/codespace/mock_api.go | 91 ++++--------------------------- pkg/cmd/codespace/ports.go | 27 ++------- pkg/cmd/codespace/ssh.go | 4 +- 11 files changed, 62 insertions(+), 231 deletions(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 86214a668..d336665cf 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -151,6 +151,7 @@ type Codespace struct { Name string `json:"name"` CreatedAt string `json:"created_at"` LastUsedAt string `json:"last_used_at"` + State string `json:"state"` Branch string `json:"branch"` RepositoryName string `json:"repository_name"` RepositoryNWO string `json:"repository_nwo"` @@ -158,6 +159,8 @@ type Codespace struct { Environment CodespaceEnvironment `json:"environment"` } +const CodespaceStateProvisioned = "provisioned" + type CodespaceEnvironment struct { State string `json:"state"` Connection CodespaceEnvironmentConnection `json:"connection"` @@ -229,68 +232,28 @@ type getCodespaceTokenResponse struct { RepositoryToken string `json:"repository_token"` } -// ErrNotProvisioned is returned by GetCodespacesToken to indicate that the -// creation of a codespace is not yet complete and that the caller should try again. -var ErrNotProvisioned = errors.New("codespace not provisioned") - -// GetCodespaceToken returns a codespace token for the user. -func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName string) (string, error) { - reqBody, err := json.Marshal(getCodespaceTokenRequest{true}) - if err != nil { - return "", fmt.Errorf("error preparing request body: %w", err) - } - - req, err := http.NewRequest( - http.MethodPost, - a.githubAPI+"/vscs_internal/user/"+ownerLogin+"/codespaces/"+codespaceName+"/token", - bytes.NewBuffer(reqBody), - ) - if err != nil { - return "", fmt.Errorf("error creating request: %w", err) - } - - a.setHeaders(req) - resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*/token") - if err != nil { - return "", fmt.Errorf("error making request: %w", err) - } - defer resp.Body.Close() - - b, err := ioutil.ReadAll(resp.Body) - if err != nil { - return "", fmt.Errorf("error reading response body: %w", err) - } - - if resp.StatusCode != http.StatusOK { - if resp.StatusCode == http.StatusUnprocessableEntity { - return "", ErrNotProvisioned - } - - return "", jsonErrorResponse(b) - } - - var response getCodespaceTokenResponse - if err := json.Unmarshal(b, &response); err != nil { - return "", fmt.Errorf("error unmarshaling response: %w", err) - } - - return response.RepositoryToken, nil -} - -// GetCodespace returns a codespace for the user. -func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) (*Codespace, error) { +// GetCodespace returns the user codespace based on the provided name. +// If the codespace is not found, an error is returned. +// If includeConnection is true, it will return the connection information for the codespace. +func (a *API) GetCodespace(ctx context.Context, codespaceName string, includeConnection bool) (*Codespace, error) { req, err := http.NewRequest( http.MethodGet, - a.githubAPI+"/vscs_internal/user/"+owner+"/codespaces/"+codespace, + a.githubAPI+"/user/codespaces/"+codespaceName, nil, ) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) } - // TODO: use a.setHeaders() - req.Header.Set("Authorization", "Bearer "+token) - resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*") + if includeConnection { + q := req.URL.Query() + q.Add("internal", "true") + q.Add("refresh", "true") + req.URL.RawQuery = q.Encode() + } + + a.setHeaders(req) + resp, err := a.do(ctx, req, "/user/codespaces/*") if err != nil { return nil, fmt.Errorf("error making request: %w", err) } @@ -437,7 +400,6 @@ func (a *API) GetCodespacesMachines(ctx context.Context, repoID int, branch, loc // CreateCodespaceParams are the required parameters for provisioning a Codespace. type CreateCodespaceParams struct { - User string RepositoryID int Branch, Machine, Location string } @@ -464,21 +426,16 @@ func (a *API) CreateCodespace(ctx context.Context, params *CreateCodespaceParams case <-ctx.Done(): return nil, ctx.Err() case <-ticker.C: - token, err := a.GetCodespaceToken(ctx, params.User, codespace.Name) - if err != nil { - if err == ErrNotProvisioned { - // Do nothing. We expect this to fail until the codespace is provisioned - continue - } - - return nil, fmt.Errorf("failed to get codespace token: %w", err) - } - - codespace, err = a.GetCodespace(ctx, token, params.User, codespace.Name) + codespace, err = a.GetCodespace(ctx, codespace.Name, false) if err != nil { return nil, fmt.Errorf("failed to get codespace: %w", err) } + // we continue to poll until the codespace shows as provisioned + if codespace.State != CodespaceStateProvisioned { + continue + } + return codespace, nil } } diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 968217280..ab013409b 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -24,14 +24,13 @@ func connectionReady(codespace *api.Codespace) bool { } type apiClient interface { - GetCodespace(ctx context.Context, token, user, name string) (*api.Codespace, error) - GetCodespaceToken(ctx context.Context, user, codespace string) (string, error) + GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) StartCodespace(ctx context.Context, name string) error } // ConnectToLiveshare waits for a Codespace to become running, // and connects to it using a Live Share session. -func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, userLogin, token string, codespace *api.Codespace) (*liveshare.Session, error) { +func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, codespace *api.Codespace) (*liveshare.Session, error) { var startedCodespace bool if codespace.Environment.State != api.CodespaceEnvironmentStateAvailable { startedCodespace = true @@ -55,7 +54,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, us } var err error - codespace, err = apiClient.GetCodespace(ctx, token, userLogin, codespace.Name) + codespace, err = apiClient.GetCodespace(ctx, codespace.Name, true) if err != nil { return nil, fmt.Errorf("error getting codespace: %w", err) } diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index b9d12a796..14fdd0b10 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -36,13 +36,8 @@ type PostCreateState struct { // PollPostCreateStates watches for state changes in a codespace, // and calls the supplied poller for each batch of state changes. // It runs until it encounters an error, including cancellation of the context. -func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, user *api.User, codespace *api.Codespace, poller func([]PostCreateState)) (err error) { - token, err := apiClient.GetCodespaceToken(ctx, user.Login, codespace.Name) - if err != nil { - return fmt.Errorf("getting codespace token: %w", err) - } - - session, err := ConnectToLiveshare(ctx, log, apiClient, user.Login, token, codespace) +func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, codespace *api.Codespace, poller func([]PostCreateState)) (err error) { + session, err := ConnectToLiveshare(ctx, log, apiClient, codespace) if err != nil { return fmt.Errorf("connect to Live Share: %w", err) } diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index 831db7366..4dae92a90 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -33,8 +33,7 @@ func NewApp(logger *output.Logger, apiClient apiClient) *App { //go:generate moq -fmt goimports -rm -skip-ensure -out mock_api.go . apiClient type apiClient interface { GetUser(ctx context.Context) (*api.User, error) - GetCodespaceToken(ctx context.Context, user, name string) (string, error) - GetCodespace(ctx context.Context, token, user, name string) (*api.Codespace, error) + GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) ListCodespaces(ctx context.Context) ([]*api.Codespace, error) DeleteCodespace(ctx context.Context, name string) error StartCodespace(ctx context.Context, name string) error @@ -96,35 +95,24 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace) ( } // getOrChooseCodespace prompts the user to choose a codespace if the codespaceName is empty. -// It then fetches the codespace token and the codespace record. -func getOrChooseCodespace(ctx context.Context, apiClient apiClient, user *api.User, codespaceName string) (codespace *api.Codespace, token string, err error) { +// It then fetches the codespace record with full connection details. +func getOrChooseCodespace(ctx context.Context, apiClient apiClient, codespaceName string) (codespace *api.Codespace, err error) { if codespaceName == "" { codespace, err = chooseCodespace(ctx, apiClient) if err != nil { if err == errNoCodespaces { - return nil, "", err + return nil, err } - return nil, "", fmt.Errorf("choosing codespace: %w", err) - } - codespaceName = codespace.Name - - token, err = apiClient.GetCodespaceToken(ctx, user.Login, codespaceName) - if err != nil { - return nil, "", fmt.Errorf("getting codespace token: %w", err) + return nil, fmt.Errorf("choosing codespace: %w", err) } } else { - token, err = apiClient.GetCodespaceToken(ctx, user.Login, codespaceName) + codespace, err = apiClient.GetCodespace(ctx, codespaceName, true) if err != nil { - return nil, "", fmt.Errorf("getting codespace token for given codespace: %w", err) - } - - codespace, err = apiClient.GetCodespace(ctx, token, user.Login, codespaceName) - if err != nil { - return nil, "", fmt.Errorf("getting full codespace details: %w", err) + return nil, fmt.Errorf("getting full codespace details: %w", err) } } - return codespace, token, nil + return codespace, nil } func safeClose(closer io.Closer, err *error) { diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index 96342f27d..a558f4c67 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -81,7 +81,6 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { a.logger.Print("Creating your codespace...") codespace, err := a.apiClient.CreateCodespace(ctx, &api.CreateCodespaceParams{ - User: userResult.User.Login, RepositoryID: repository.ID, Branch: branch, Machine: machine, @@ -157,7 +156,7 @@ func showStatus(ctx context.Context, log *output.Logger, apiClient apiClient, us } } - err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, codespace, poller) + err := codespaces.PollPostCreateStates(ctx, log, apiClient, codespace, poller) if err != nil { if errors.Is(err, context.Canceled) && breakNextState { return nil // we cancelled the context to stop polling, we can ignore the error diff --git a/pkg/cmd/codespace/delete.go b/pkg/cmd/codespace/delete.go index 52c917698..36bee15b7 100644 --- a/pkg/cmd/codespace/delete.go +++ b/pkg/cmd/codespace/delete.go @@ -58,12 +58,7 @@ func newDeleteCmd(app *App) *cobra.Command { return deleteCmd } -func (a *App) Delete(ctx context.Context, opts deleteOptions) error { - user, err := a.apiClient.GetUser(ctx) - if err != nil { - return fmt.Errorf("error getting user: %w", err) - } - +func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) { var codespaces []*api.Codespace nameFilter := opts.codespaceName if nameFilter == "" { @@ -80,12 +75,7 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) error { nameFilter = c.Name } } else { - token, err := a.apiClient.GetCodespaceToken(ctx, user.Login, nameFilter) - if err != nil { - return fmt.Errorf("error getting codespace token: %w", err) - } - - codespace, err := a.apiClient.GetCodespace(ctx, token, user.Login, nameFilter) + codespace, err := a.apiClient.GetCodespace(ctx, nameFilter, false) if err != nil { return fmt.Errorf("error fetching codespace information: %w", err) } diff --git a/pkg/cmd/codespace/delete_test.go b/pkg/cmd/codespace/delete_test.go index 5a793614c..ba616b22e 100644 --- a/pkg/cmd/codespace/delete_test.go +++ b/pkg/cmd/codespace/delete_test.go @@ -168,19 +168,7 @@ func TestDelete(t *testing.T) { return tt.codespaces, nil } } else { - apiMock.GetCodespaceTokenFunc = func(_ context.Context, userLogin, name string) (string, error) { - if userLogin != user.Login { - return "", fmt.Errorf("unexpected user %q", userLogin) - } - return "CS_TOKEN", nil - } - apiMock.GetCodespaceFunc = func(_ context.Context, token, userLogin, name string) (*api.Codespace, error) { - if userLogin != user.Login { - return nil, fmt.Errorf("unexpected user %q", userLogin) - } - if token != "CS_TOKEN" { - return nil, fmt.Errorf("unexpected token %q", token) - } + apiMock.GetCodespaceFunc = func(_ context.Context, name string) (*api.Codespace, error) { return tt.codespaces[0], nil } } @@ -206,9 +194,6 @@ func TestDelete(t *testing.T) { if (err != nil) != tt.wantErr { t.Errorf("delete() error = %v, wantErr %v", err, tt.wantErr) } - if n := len(apiMock.GetUserCalls()); n != 1 { - t.Errorf("GetUser invoked %d times, expected %d", n, 1) - } var gotDeleted []string for _, delArgs := range apiMock.DeleteCodespaceCalls() { gotDeleted = append(gotDeleted, delArgs.Name) diff --git a/pkg/cmd/codespace/logs.go b/pkg/cmd/codespace/logs.go index 0e52b488a..9bfc4a967 100644 --- a/pkg/cmd/codespace/logs.go +++ b/pkg/cmd/codespace/logs.go @@ -46,12 +46,12 @@ func (a *App) Logs(ctx context.Context, codespaceName string, follow bool) (err authkeys <- checkAuthorizedKeys(ctx, a.apiClient, user.Login) }() - codespace, token, err := getOrChooseCodespace(ctx, a.apiClient, user, codespaceName) + codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName) if err != nil { return fmt.Errorf("get or choose codespace: %w", err) } - session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, user.Login, token, codespace) + session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, codespace) if err != nil { return fmt.Errorf("connecting to Live Share: %w", err) } diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index 05ddd7612..11daffd74 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -25,7 +25,7 @@ import ( // DeleteCodespaceFunc: func(ctx context.Context, name string) error { // panic("mock out the DeleteCodespace method") // }, -// GetCodespaceFunc: func(ctx context.Context, token string, user string, name string) (*api.Codespace, error) { +// GetCodespaceFunc: func(ctx context.Context, name string) (*api.Codespace, error) { // panic("mock out the GetCodespace method") // }, // GetCodespaceRegionLocationFunc: func(ctx context.Context) (string, error) { @@ -34,9 +34,6 @@ import ( // GetCodespaceRepositoryContentsFunc: func(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) { // panic("mock out the GetCodespaceRepositoryContents method") // }, -// GetCodespaceTokenFunc: func(ctx context.Context, user string, name string) (string, error) { -// panic("mock out the GetCodespaceToken method") -// }, // GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch string, location string) ([]*api.Machine, error) { // panic("mock out the GetCodespacesMachines method") // }, @@ -69,7 +66,7 @@ type apiClientMock struct { DeleteCodespaceFunc func(ctx context.Context, name string) error // GetCodespaceFunc mocks the GetCodespace method. - GetCodespaceFunc func(ctx context.Context, token string, user string, name string) (*api.Codespace, error) + GetCodespaceFunc func(ctx context.Context, name string) (*api.Codespace, error) // GetCodespaceRegionLocationFunc mocks the GetCodespaceRegionLocation method. GetCodespaceRegionLocationFunc func(ctx context.Context) (string, error) @@ -77,9 +74,6 @@ type apiClientMock struct { // GetCodespaceRepositoryContentsFunc mocks the GetCodespaceRepositoryContents method. GetCodespaceRepositoryContentsFunc func(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) - // GetCodespaceTokenFunc mocks the GetCodespaceToken method. - GetCodespaceTokenFunc func(ctx context.Context, user string, name string) (string, error) - // GetCodespacesMachinesFunc mocks the GetCodespacesMachines method. GetCodespacesMachinesFunc func(ctx context.Context, repoID int, branch string, location string) ([]*api.Machine, error) @@ -122,10 +116,6 @@ type apiClientMock struct { GetCodespace []struct { // Ctx is the ctx argument value. Ctx context.Context - // Token is the token argument value. - Token string - // User is the user argument value. - User string // Name is the name argument value. Name string } @@ -143,15 +133,6 @@ type apiClientMock struct { // Path is the path argument value. Path string } - // GetCodespaceToken holds details about calls to the GetCodespaceToken method. - GetCodespaceToken []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // User is the user argument value. - User string - // Name is the name argument value. - Name string - } // GetCodespacesMachines holds details about calls to the GetCodespacesMachines method. GetCodespacesMachines []struct { // Ctx is the ctx argument value. @@ -194,7 +175,6 @@ type apiClientMock struct { lockGetCodespace sync.RWMutex lockGetCodespaceRegionLocation sync.RWMutex lockGetCodespaceRepositoryContents sync.RWMutex - lockGetCodespaceToken sync.RWMutex lockGetCodespacesMachines sync.RWMutex lockGetRepository sync.RWMutex lockGetUser sync.RWMutex @@ -308,41 +288,33 @@ func (mock *apiClientMock) DeleteCodespaceCalls() []struct { } // GetCodespace calls GetCodespaceFunc. -func (mock *apiClientMock) GetCodespace(ctx context.Context, token string, user string, name string) (*api.Codespace, error) { +func (mock *apiClientMock) GetCodespace(ctx context.Context, name string) (*api.Codespace, error) { if mock.GetCodespaceFunc == nil { panic("apiClientMock.GetCodespaceFunc: method is nil but apiClient.GetCodespace was just called") } callInfo := struct { - Ctx context.Context - Token string - User string - Name string + Ctx context.Context + Name string }{ - Ctx: ctx, - Token: token, - User: user, - Name: name, + Ctx: ctx, + Name: name, } mock.lockGetCodespace.Lock() mock.calls.GetCodespace = append(mock.calls.GetCodespace, callInfo) mock.lockGetCodespace.Unlock() - return mock.GetCodespaceFunc(ctx, token, user, name) + return mock.GetCodespaceFunc(ctx, name) } // GetCodespaceCalls gets all the calls that were made to GetCodespace. // Check the length with: // len(mockedapiClient.GetCodespaceCalls()) func (mock *apiClientMock) GetCodespaceCalls() []struct { - Ctx context.Context - Token string - User string - Name string + Ctx context.Context + Name string } { var calls []struct { - Ctx context.Context - Token string - User string - Name string + Ctx context.Context + Name string } mock.lockGetCodespace.RLock() calls = mock.calls.GetCodespace @@ -420,45 +392,6 @@ func (mock *apiClientMock) GetCodespaceRepositoryContentsCalls() []struct { return calls } -// GetCodespaceToken calls GetCodespaceTokenFunc. -func (mock *apiClientMock) GetCodespaceToken(ctx context.Context, user string, name string) (string, error) { - if mock.GetCodespaceTokenFunc == nil { - panic("apiClientMock.GetCodespaceTokenFunc: method is nil but apiClient.GetCodespaceToken was just called") - } - callInfo := struct { - Ctx context.Context - User string - Name string - }{ - Ctx: ctx, - User: user, - Name: name, - } - mock.lockGetCodespaceToken.Lock() - mock.calls.GetCodespaceToken = append(mock.calls.GetCodespaceToken, callInfo) - mock.lockGetCodespaceToken.Unlock() - return mock.GetCodespaceTokenFunc(ctx, user, name) -} - -// GetCodespaceTokenCalls gets all the calls that were made to GetCodespaceToken. -// Check the length with: -// len(mockedapiClient.GetCodespaceTokenCalls()) -func (mock *apiClientMock) GetCodespaceTokenCalls() []struct { - Ctx context.Context - User string - Name string -} { - var calls []struct { - Ctx context.Context - User string - Name string - } - mock.lockGetCodespaceToken.RLock() - calls = mock.calls.GetCodespaceToken - mock.lockGetCodespaceToken.RUnlock() - return calls -} - // GetCodespacesMachines calls GetCodespacesMachinesFunc. func (mock *apiClientMock) GetCodespacesMachines(ctx context.Context, repoID int, branch string, location string) ([]*api.Machine, error) { if mock.GetCodespacesMachinesFunc == nil { diff --git a/pkg/cmd/codespace/ports.go b/pkg/cmd/codespace/ports.go index d2c14a388..c36f46078 100644 --- a/pkg/cmd/codespace/ports.go +++ b/pkg/cmd/codespace/ports.go @@ -49,12 +49,7 @@ func newPortsCmd(app *App) *cobra.Command { // ListPorts lists known ports in a codespace. func (a *App) ListPorts(ctx context.Context, codespaceName string, asJSON bool) (err error) { - user, err := a.apiClient.GetUser(ctx) - if err != nil { - return fmt.Errorf("error getting user: %w", err) - } - - codespace, token, err := getOrChooseCodespace(ctx, a.apiClient, user, codespaceName) + codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName) if err != nil { // TODO(josebalius): remove special handling of this error here and it other places if err == errNoCodespaces { @@ -65,7 +60,7 @@ func (a *App) ListPorts(ctx context.Context, codespaceName string, asJSON bool) devContainerCh := getDevContainer(ctx, a.apiClient, codespace) - session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, user.Login, token, codespace) + session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, codespace) if err != nil { return fmt.Errorf("error connecting to Live Share: %w", err) } @@ -191,12 +186,7 @@ func newPortsPrivateCmd(app *App) *cobra.Command { } func (a *App) UpdatePortVisibility(ctx context.Context, codespaceName, sourcePort string, public bool) (err error) { - user, err := a.apiClient.GetUser(ctx) - if err != nil { - return fmt.Errorf("error getting user: %w", err) - } - - codespace, token, err := getOrChooseCodespace(ctx, a.apiClient, user, codespaceName) + codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName) if err != nil { if err == errNoCodespaces { return err @@ -204,7 +194,7 @@ func (a *App) UpdatePortVisibility(ctx context.Context, codespaceName, sourcePor return fmt.Errorf("error getting codespace: %w", err) } - session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, user.Login, token, codespace) + session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, codespace) if err != nil { return fmt.Errorf("error connecting to Live Share: %w", err) } @@ -255,12 +245,7 @@ func (a *App) ForwardPorts(ctx context.Context, codespaceName string, ports []st return fmt.Errorf("get port pairs: %w", err) } - user, err := a.apiClient.GetUser(ctx) - if err != nil { - return fmt.Errorf("error getting user: %w", err) - } - - codespace, token, err := getOrChooseCodespace(ctx, a.apiClient, user, codespaceName) + codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName) if err != nil { if err == errNoCodespaces { return err @@ -268,7 +253,7 @@ func (a *App) ForwardPorts(ctx context.Context, codespaceName string, ports []st return fmt.Errorf("error getting codespace: %w", err) } - session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, user.Login, token, codespace) + session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, codespace) if err != nil { return fmt.Errorf("error connecting to Live Share: %w", err) } diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index a7bb682ba..c90b542cb 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -45,12 +45,12 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, sshProfile, codespaceNa authkeys <- checkAuthorizedKeys(ctx, a.apiClient, user.Login) }() - codespace, token, err := getOrChooseCodespace(ctx, a.apiClient, user, codespaceName) + codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName) if err != nil { return fmt.Errorf("get or choose codespace: %w", err) } - session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, user.Login, token, codespace) + session, err := codespaces.ConnectToLiveshare(ctx, a.logger, a.apiClient, codespace) if err != nil { return fmt.Errorf("error connecting to Live Share: %w", err) } From 8bb55359b1f9b7a9a97f0d8d162d3d9b3717d7a1 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 6 Oct 2021 09:10:00 -0400 Subject: [PATCH 2/4] Update mock API --- pkg/cmd/codespace/mock_api.go | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index 11daffd74..df9b6f3b2 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -25,7 +25,7 @@ import ( // DeleteCodespaceFunc: func(ctx context.Context, name string) error { // panic("mock out the DeleteCodespace method") // }, -// GetCodespaceFunc: func(ctx context.Context, name string) (*api.Codespace, error) { +// GetCodespaceFunc: func(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) { // panic("mock out the GetCodespace method") // }, // GetCodespaceRegionLocationFunc: func(ctx context.Context) (string, error) { @@ -66,7 +66,7 @@ type apiClientMock struct { DeleteCodespaceFunc func(ctx context.Context, name string) error // GetCodespaceFunc mocks the GetCodespace method. - GetCodespaceFunc func(ctx context.Context, name string) (*api.Codespace, error) + GetCodespaceFunc func(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) // GetCodespaceRegionLocationFunc mocks the GetCodespaceRegionLocation method. GetCodespaceRegionLocationFunc func(ctx context.Context) (string, error) @@ -118,6 +118,8 @@ type apiClientMock struct { Ctx context.Context // Name is the name argument value. Name string + // IncludeConnection is the includeConnection argument value. + IncludeConnection bool } // GetCodespaceRegionLocation holds details about calls to the GetCodespaceRegionLocation method. GetCodespaceRegionLocation []struct { @@ -288,33 +290,37 @@ func (mock *apiClientMock) DeleteCodespaceCalls() []struct { } // GetCodespace calls GetCodespaceFunc. -func (mock *apiClientMock) GetCodespace(ctx context.Context, name string) (*api.Codespace, error) { +func (mock *apiClientMock) GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) { if mock.GetCodespaceFunc == nil { panic("apiClientMock.GetCodespaceFunc: method is nil but apiClient.GetCodespace was just called") } callInfo := struct { - Ctx context.Context - Name string + Ctx context.Context + Name string + IncludeConnection bool }{ - Ctx: ctx, - Name: name, + Ctx: ctx, + Name: name, + IncludeConnection: includeConnection, } mock.lockGetCodespace.Lock() mock.calls.GetCodespace = append(mock.calls.GetCodespace, callInfo) mock.lockGetCodespace.Unlock() - return mock.GetCodespaceFunc(ctx, name) + return mock.GetCodespaceFunc(ctx, name, includeConnection) } // GetCodespaceCalls gets all the calls that were made to GetCodespace. // Check the length with: // len(mockedapiClient.GetCodespaceCalls()) func (mock *apiClientMock) GetCodespaceCalls() []struct { - Ctx context.Context - Name string + Ctx context.Context + Name string + IncludeConnection bool } { var calls []struct { - Ctx context.Context - Name string + Ctx context.Context + Name string + IncludeConnection bool } mock.lockGetCodespace.RLock() calls = mock.calls.GetCodespace From 7c497f283b06d9e2e1bff145d6a0b7edf4b3616a Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 6 Oct 2021 09:15:07 -0400 Subject: [PATCH 3/4] Update test signature --- pkg/cmd/codespace/delete_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/codespace/delete_test.go b/pkg/cmd/codespace/delete_test.go index ba616b22e..14896098a 100644 --- a/pkg/cmd/codespace/delete_test.go +++ b/pkg/cmd/codespace/delete_test.go @@ -168,7 +168,7 @@ func TestDelete(t *testing.T) { return tt.codespaces, nil } } else { - apiMock.GetCodespaceFunc = func(_ context.Context, name string) (*api.Codespace, error) { + apiMock.GetCodespaceFunc = func(_ context.Context, name string, includeConnection bool) (*api.Codespace, error) { return tt.codespaces[0], nil } } From b2ff4c321a942539ad5dd4931c1e64738c05db34 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 6 Oct 2021 09:25:39 -0400 Subject: [PATCH 4/4] Remove unused structs --- internal/codespaces/api/api.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index d336665cf..0bc8615fc 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -223,15 +223,6 @@ func (a *API) ListCodespaces(ctx context.Context) ([]*Codespace, error) { return response.Codespaces, nil } -// getCodespaceTokenRequest is the request body for the get codespace token endpoint. -type getCodespaceTokenRequest struct { - MintRepositoryToken bool `json:"mint_repository_token"` -} - -type getCodespaceTokenResponse struct { - RepositoryToken string `json:"repository_token"` -} - // GetCodespace returns the user codespace based on the provided name. // If the codespace is not found, an error is returned. // If includeConnection is true, it will return the connection information for the codespace.