From 1fa763f51472075d26a67f0fc89e2696bd867280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Sat, 27 Feb 2021 14:21:26 +0100 Subject: [PATCH] Avoid having to first query for username in `repo list` Dynamically construct the GraphQL query by using the `viewer` connection if the owner isn't set and the `repositoryOwner(login:"...")` connection if the owner was set. --- pkg/cmd/repo/list/fixtures/repoList.json | 1 + pkg/cmd/repo/list/http.go | 58 ++++++++++++++++-------- pkg/cmd/repo/list/list.go | 19 ++------ pkg/cmd/repo/list/list_test.go | 12 ----- 4 files changed, 42 insertions(+), 48 deletions(-) diff --git a/pkg/cmd/repo/list/fixtures/repoList.json b/pkg/cmd/repo/list/fixtures/repoList.json index 8ee348f57..18bb5eff8 100644 --- a/pkg/cmd/repo/list/fixtures/repoList.json +++ b/pkg/cmd/repo/list/fixtures/repoList.json @@ -1,6 +1,7 @@ { "data": { "repositoryOwner": { + "login": "octocat", "repositories": { "totalCount": 3, "nodes": [ diff --git a/pkg/cmd/repo/list/http.go b/pkg/cmd/repo/list/http.go index 8413b479f..97bf584fb 100644 --- a/pkg/cmd/repo/list/http.go +++ b/pkg/cmd/repo/list/http.go @@ -3,6 +3,7 @@ package list import ( "context" "net/http" + "reflect" "strings" "time" @@ -39,31 +40,18 @@ func (r Repository) Info() string { } type RepositoryList struct { + Owner string Repositories []Repository TotalCount int } func listRepos(client *http.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { - type query struct { - RepositoryOwner struct { - Repositories struct { - Nodes []Repository - TotalCount int - PageInfo struct { - HasNextPage bool - EndCursor string - } - } `graphql:"repositories(first: $perPage, after: $endCursor, privacy: $privacy, isFork: $fork, ownerAffiliations: OWNER, orderBy: { field: PUSHED_AT, direction: DESC })"` - } `graphql:"repositoryOwner(login: $owner)"` - } - perPage := limit if perPage > 100 { perPage = 100 } variables := map[string]interface{}{ - "owner": githubv4.String(owner), "perPage": githubv4.Int(perPage), "endCursor": (*githubv4.String)(nil), } @@ -82,28 +70,58 @@ func listRepos(client *http.Client, hostname string, limit int, owner string, fi variables["fork"] = (*githubv4.Boolean)(nil) } + var ownerConnection string + if owner == "" { + ownerConnection = `graphql:"repositoryOwner: viewer"` + } else { + ownerConnection = `graphql:"repositoryOwner(login: $owner)"` + variables["owner"] = githubv4.String(owner) + } + + type repositoryOwner struct { + Login string + Repositories struct { + Nodes []Repository + TotalCount int + PageInfo struct { + HasNextPage bool + EndCursor string + } + } `graphql:"repositories(first: $perPage, after: $endCursor, privacy: $privacy, isFork: $fork, ownerAffiliations: OWNER, orderBy: { field: PUSHED_AT, direction: DESC })"` + } + query := reflect.StructOf([]reflect.StructField{ + { + Name: "RepositoryOwner", + Type: reflect.TypeOf(repositoryOwner{}), + Tag: reflect.StructTag(ownerConnection), + }, + }) + listResult := RepositoryList{} pagination: for { - var result query + result := reflect.New(query) gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) - err := gql.QueryNamed(context.Background(), "RepositoryList", &result, variables) + err := gql.QueryNamed(context.Background(), "RepositoryList", result.Interface(), variables) if err != nil { return nil, err } - listResult.TotalCount = result.RepositoryOwner.Repositories.TotalCount - for _, repo := range result.RepositoryOwner.Repositories.Nodes { + owner := result.Elem().FieldByName("RepositoryOwner").Interface().(repositoryOwner) + listResult.TotalCount = owner.Repositories.TotalCount + listResult.Owner = owner.Login + + for _, repo := range owner.Repositories.Nodes { listResult.Repositories = append(listResult.Repositories, repo) if len(listResult.Repositories) >= limit { break pagination } } - if !result.RepositoryOwner.Repositories.PageInfo.HasNextPage { + if !owner.Repositories.PageInfo.HasNextPage { break } - variables["endCursor"] = githubv4.String(result.RepositoryOwner.Repositories.PageInfo.EndCursor) + variables["endCursor"] = githubv4.String(owner.Repositories.PageInfo.EndCursor) } return &listResult, nil diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index 4fd74f827..62e16a596 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -5,7 +5,6 @@ import ( "net/http" "time" - "github.com/cli/cli/api" "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" @@ -94,25 +93,13 @@ func listRun(opts *ListOptions) error { return err } - apiClient := api.NewClientFromHTTP(httpClient) - - isTerminal := opts.IO.IsStdoutTTY() - - owner := opts.Owner - if owner == "" { - owner, err = api.CurrentLoginName(apiClient, ghinstance.OverridableDefault()) - if err != nil { - return err - } - } - filter := FilterOptions{ Visibility: opts.Visibility, Fork: opts.Fork, Source: opts.Source, } - listResult, err := listRepos(httpClient, ghinstance.OverridableDefault(), opts.Limit, owner, filter) + listResult, err := listRepos(httpClient, ghinstance.OverridableDefault(), opts.Limit, opts.Owner, filter) if err != nil { return err } @@ -139,9 +126,9 @@ func listRun(opts *ListOptions) error { tp.EndRow() } - if isTerminal { + if opts.IO.IsStdoutTTY() { hasFilters := filter.Visibility != "" || filter.Fork || filter.Source - title := listHeader(owner, len(listResult.Repositories), listResult.TotalCount, hasFilters) + title := listHeader(listResult.Owner, len(listResult.Repositories), listResult.TotalCount, hasFilters) fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } diff --git a/pkg/cmd/repo/list/list_test.go b/pkg/cmd/repo/list/list_test.go index d6bc37afe..fb55cb11c 100644 --- a/pkg/cmd/repo/list/list_test.go +++ b/pkg/cmd/repo/list/list_test.go @@ -57,10 +57,6 @@ func TestRepoList_nontty(t *testing.T) { httpReg := &httpmock.Registry{} defer httpReg.Verify(t) - httpReg.Register( - httpmock.GraphQL(`query UserCurrent\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`), - ) httpReg.Register( httpmock.GraphQL(`query RepositoryList\b`), httpmock.FileResponse("./fixtures/repoList.json"), @@ -99,10 +95,6 @@ func TestRepoList_tty(t *testing.T) { httpReg := &httpmock.Registry{} defer httpReg.Verify(t) - httpReg.Register( - httpmock.GraphQL(`query UserCurrent\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`), - ) httpReg.Register( httpmock.GraphQL(`query RepositoryList\b`), httpmock.FileResponse("./fixtures/repoList.json"), @@ -139,10 +131,6 @@ func TestRepoList_filtering(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) - http.Register( - httpmock.GraphQL(`query UserCurrent\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`), - ) http.Register( httpmock.GraphQL(`query RepositoryList\b`), httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) {