perf(pr,issue): load the interactive create form data concurrently

Signed-off-by: bircni <bircni@icloud.com>
This commit is contained in:
bircni committed 2026-10-18 18:59:59 +02:00
1 parent 517d164d8a
commit 9360f39cc8
7 files changed
+230 -29

No files matched your search

+4 -11
View File
@@ -326,24 +326,17 @@ func (a *application) createPullInteractively(cmd *cobra.Command, snapshot setti
if err != nil { if err != nil {
return err return err
} }
repo, err := (services.Repositories{Client: target.client}).View(cmd.Context(), target.Repo)
if err != nil {
return err
}
service := services.Pulls{Client: target.client, Repository: target.Repo} service := services.Pulls{Client: target.client, Repository: target.Repo}
selectables, err := service.CreateSelectables(cmd.Context()) loaded, err := service.CreateChoices(cmd.Context())
if err != nil { if err != nil {
return err return err
} }
repo, selectables := loaded.Repository, loaded.CreateSelectables
baseOwner, _, _ := strings.Cut(target.Repo, "/") baseOwner, _, _ := strings.Cut(target.Repo, "/")
headOwner, headBranch := a.defaultPullHead(cmd.Context()) headOwner, headBranch := a.defaultPullHead(cmd.Context())
template, err := service.Template(cmd.Context(), repo.DefaultBranch)
if err != nil {
return err
}
title, body, _ := a.singleCommitMessage(cmd.Context(), repo.DefaultBranch, headBranch) title, body, _ := a.singleCommitMessage(cmd.Context(), repo.DefaultBranch, headBranch)
if template != "" { if loaded.Template != "" {
body = template body = loaded.Template
} }
choices := terminal.PullFormChoices{ choices := terminal.PullFormChoices{
Repo: target.Repo, BaseOwner: baseOwner, Base: repo.DefaultBranch, HeadOwner: headOwner, HeadBranch: headBranch, Title: title, Body: body, Repo: target.Repo, BaseOwner: baseOwner, Base: repo.DefaultBranch, HeadOwner: headOwner, HeadBranch: headBranch, Title: title, Body: body,
+5
View File
@@ -33,6 +33,11 @@ func pageOf[T any](items []T, limit, page int) ([]T, error) {
return items[start:min(start+limit, len(items))], nil return items[start:min(start+limit, len(items))], nil
} }
// loadTogether runs the loaders concurrently and returns their errors joined.
func loadTogether(loaders ...func() error) error {
return concurrently(len(loaders), func(i int) error { return loaders[i]() })
}
// listAll collects pages until the server returns an empty one. // listAll collects pages until the server returns an empty one.
func listAll[T any](fetch func(page int) ([]T, error)) ([]T, error) { func listAll[T any](fetch func(page int) ([]T, error)) ([]T, error) {
result := []T{} result := []T{}
+13 -4
View File
@@ -41,12 +41,21 @@ type IssueCreateSelectables struct {
Templates []IssueTemplate Templates []IssueTemplate
} }
// CreateSelectables loads templates and, when the user can push, assignees/milestones/labels. // CreateSelectables concurrently loads templates and, when the user can push, assignees/milestones/labels.
func (s Issues) CreateSelectables(ctx context.Context) (IssueCreateSelectables, error) { func (s Issues) CreateSelectables(ctx context.Context) (IssueCreateSelectables, error) {
choices, err := loadCreateSelectables(ctx, s.Client, s.Repository) var result IssueCreateSelectables
err := loadTogether(
func() (err error) {
result.CreateSelectables, err = loadCreateSelectables(ctx, s.Client, s.Repository)
return err
},
func() (err error) {
result.Templates, err = s.IssueTemplates(ctx)
return err
},
)
if err != nil { if err != nil {
return IssueCreateSelectables{}, err return IssueCreateSelectables{}, err
} }
templates, err := s.IssueTemplates(ctx) return result, nil
return IssueCreateSelectables{CreateSelectables: choices, Templates: templates}, err
} }
@@ -8,6 +8,7 @@ import (
"fmt" "fmt"
"net/http" "net/http"
"strings" "strings"
"sync"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@@ -57,9 +58,12 @@ func TestCreateSelectablesLoadWriteFieldsOnlyWithPush(t *testing.T) {
} }
for _, push := range []bool{false, true} { for _, push := range []bool{false, true} {
t.Run(fmt.Sprintf("push=%t", push), func(t *testing.T) { t.Run(fmt.Sprintf("push=%t", push), func(t *testing.T) {
var mu sync.Mutex
calls := map[string]int{} calls := map[string]int{}
client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
mu.Lock()
calls[r.URL.Path]++ calls[r.URL.Path]++
mu.Unlock()
switch r.URL.Path { switch r.URL.Path {
case "/api/v1/repos/alice/project": case "/api/v1/repos/alice/project":
_, _ = fmt.Fprintf(w, `{"full_name":"alice/project","permissions":{"push":%t,"pull":true}}`, push) _, _ = fmt.Fprintf(w, `{"full_name":"alice/project","permissions":{"push":%t,"pull":true}}`, push)
+59 -12
View File
@@ -23,30 +23,77 @@ func (s Pulls) CreateSelectables(ctx context.Context) (CreateSelectables, error)
return loadCreateSelectables(ctx, s.Client, s.Repository) return loadCreateSelectables(ctx, s.Client, s.Repository)
} }
// PullCreateChoices seed the interactive create form: the repository, its pull request template
// on the default branch, and the metadata selectables.
type PullCreateChoices struct {
Repository Repository
Template string
CreateSelectables
}
// CreateChoices fetches the repository once for its default branch and permissions, then loads
// the pull request template and the selectables concurrently.
func (s Pulls) CreateChoices(ctx context.Context) (PullCreateChoices, error) {
item, err := (Repositories{Client: s.Client}).get(ctx, s.Repository)
if err != nil {
return PullCreateChoices{}, err
}
result := PullCreateChoices{Repository: repositoryFrom(item)}
err = loadTogether(
func() (err error) {
result.Template, err = s.Template(ctx, result.Repository.DefaultBranch)
return err
},
func() (err error) {
result.CreateSelectables, err = createSelectablesFor(ctx, s.Client, s.Repository, permissionsFrom(item).Push)
return err
},
)
if err != nil {
return PullCreateChoices{}, err
}
return result, nil
}
func loadCreateSelectables(ctx context.Context, client *transport.Client, repository string) (CreateSelectables, error) { func loadCreateSelectables(ctx context.Context, client *transport.Client, repository string) (CreateSelectables, error) {
result := CreateSelectables{Assignees: []string{}, Milestones: []Milestone{}, Labels: []Label{}}
permissions, err := (Repositories{Client: client}).viewPermissions(ctx, repository) permissions, err := (Repositories{Client: client}).viewPermissions(ctx, repository)
if err != nil || !permissions.Push { if err != nil {
return result, err return CreateSelectables{Assignees: []string{}, Milestones: []Milestone{}, Labels: []Label{}}, err
}
return createSelectablesFor(ctx, client, repository, permissions.Push)
}
// createSelectablesFor loads assignees, milestones, and labels concurrently when canPush.
func createSelectablesFor(ctx context.Context, client *transport.Client, repository string, canPush bool) (CreateSelectables, error) {
result := CreateSelectables{Assignees: []string{}, Milestones: []Milestone{}, Labels: []Label{}}
if !canPush {
return result, nil
} }
result.CanPush = true result.CanPush = true
if result.Assignees, err = (Repositories{Client: client}).assignees(ctx, repository); err != nil { err := loadTogether(
return result, err func() (err error) {
} result.Assignees, err = (Repositories{Client: client}).assignees(ctx, repository)
if result.Milestones, err = listAll(func(page int) ([]Milestone, error) { return err
},
func() (err error) {
result.Milestones, err = listAll(func(page int) ([]Milestone, error) {
return (Milestones{Client: client, Repository: repository}).List(ctx, "open", 100, page) return (Milestones{Client: client, Repository: repository}).List(ctx, "open", 100, page)
}); err != nil { })
return result, err return err
} },
func() (err error) {
if result.Labels, err = (Labels{Client: client, Repository: repository}).All(ctx); err != nil { if result.Labels, err = (Labels{Client: client, Repository: repository}).All(ctx); err != nil {
return result, err return err
} }
// Like --label, offer the labels shared by the organization that owns the repository. // Like --label, offer the labels shared by the organization that owns the repository.
owner, _, _ := strings.Cut(repository, "/") owner, _, _ := strings.Cut(repository, "/")
if _, isOrg, err := organization(ctx, client, owner); err != nil || !isOrg { if _, isOrg, err := organization(ctx, client, owner); err != nil || !isOrg {
return result, err return err
} }
orgLabels, err := (Labels{Client: client, Organization: owner}).All(ctx) orgLabels, err := (Labels{Client: client, Organization: owner}).All(ctx)
result.Labels = append(result.Labels, orgLabels...) result.Labels = append(result.Labels, orgLabels...)
return err
},
)
return result, err return result, err
} }
@@ -0,0 +1,136 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package services
import (
"context"
"fmt"
"net/http"
"strings"
"sync"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestPullCreateChoicesFetchRepositoryOnce(t *testing.T) {
for _, push := range []bool{false, true} {
t.Run(fmt.Sprintf("push=%t", push), func(t *testing.T) {
var mu sync.Mutex
calls := map[string]int{}
client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
mu.Lock()
calls[r.URL.Path]++
mu.Unlock()
switch {
case r.URL.Path == "/api/v1/orgs/alice":
w.WriteHeader(http.StatusNotFound)
case r.URL.Path == "/api/v1/repos/alice/project":
_, _ = fmt.Fprintf(w, `{"full_name":"alice/project","default_branch":"trunk","permissions":{"push":%t}}`, push)
case strings.HasPrefix(r.URL.Path, "/api/v1/repos/alice/project/raw/"):
assert.Equal(t, "trunk", r.URL.Query().Get("ref"))
if r.URL.Path != "/api/v1/repos/alice/project/raw/.gitea/PULL_REQUEST_TEMPLATE.md" {
w.WriteHeader(http.StatusNotFound)
return
}
_, _ = w.Write([]byte("## Checklist\n"))
case r.URL.Path == "/api/v1/repos/alice/project/assignees":
_, _ = w.Write([]byte(`[{"login":"alice"}]`))
case r.URL.Path == "/api/v1/repos/alice/project/milestones" && r.URL.Query().Get("page") == "1":
_, _ = w.Write([]byte(`[{"id":1,"title":"v1"}]`))
case r.URL.Path == "/api/v1/repos/alice/project/labels" && r.URL.Query().Get("page") == "1":
_, _ = w.Write([]byte(`[{"id":2,"name":"bug"}]`))
case r.URL.Path == "/api/v1/repos/alice/project/milestones", r.URL.Path == "/api/v1/repos/alice/project/labels":
_, _ = w.Write([]byte(`[]`))
default:
t.Errorf("unexpected path %s", r.URL.Path)
w.WriteHeader(http.StatusNotFound)
}
}))
choices, err := (Pulls{Client: client, Repository: "alice/project"}).CreateChoices(context.Background())
require.NoError(t, err)
require.Equal(t, "alice/project", choices.Repository.FullName)
require.Equal(t, "trunk", choices.Repository.DefaultBranch)
require.Equal(t, "## Checklist\n", choices.Template)
want := CreateSelectables{Assignees: []string{}, Milestones: []Milestone{}, Labels: []Label{}}
if push {
want = CreateSelectables{
CanPush: true,
Assignees: []string{"alice"},
Milestones: []Milestone{{ID: 1, Title: "v1"}},
Labels: []Label{{ID: 2, Name: "bug"}},
}
}
require.Equal(t, want, choices.CreateSelectables)
require.Equal(t, 1, calls["/api/v1/repos/alice/project"])
if !push {
require.Zero(t, calls["/api/v1/repos/alice/project/assignees"])
}
})
}
}
func TestPullCreateChoicesLoadConcurrently(t *testing.T) {
// The template, assignees, milestones, and labels requests only complete once all four are in flight.
paths := []string{"raw/PULL_REQUEST_TEMPLATE.md", "assignees", "milestones", "labels"}
arrived := map[string]chan struct{}{}
for _, path := range paths {
arrived[path] = make(chan struct{})
}
var once sync.Map
client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
path := strings.TrimPrefix(r.URL.Path, "/api/v1/repos/alice/project/")
if r.URL.Path == "/api/v1/orgs/alice" {
w.WriteHeader(http.StatusNotFound)
return
}
if r.URL.Path == "/api/v1/repos/alice/project" {
_, _ = w.Write([]byte(`{"full_name":"alice/project","default_branch":"main","permissions":{"push":true}}`))
return
}
if ch, ok := arrived[path]; ok {
if _, loaded := once.LoadOrStore(path, true); !loaded {
close(ch)
}
for _, other := range paths {
select {
case <-arrived[other]:
case <-time.After(5 * time.Second):
t.Errorf("%s did not run concurrently with %s", other, path)
w.WriteHeader(http.StatusInternalServerError)
return
}
}
}
if strings.HasPrefix(path, "raw/") {
_, _ = w.Write([]byte("template"))
return
}
_, _ = w.Write([]byte(`[]`))
}))
choices, err := (Pulls{Client: client, Repository: "alice/project"}).CreateChoices(context.Background())
require.NoError(t, err)
require.Equal(t, "template", choices.Template)
}
func TestPullCreateChoicesReportFailures(t *testing.T) {
for _, failing := range []string{"/api/v1/repos/alice/project", "/api/v1/repos/alice/project/labels", "/api/v1/repos/alice/project/raw/PULL_REQUEST_TEMPLATE.md"} {
t.Run(failing, func(t *testing.T) {
client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch {
case r.URL.Path == failing:
w.WriteHeader(http.StatusInternalServerError)
case r.URL.Path == "/api/v1/repos/alice/project":
_, _ = w.Write([]byte(`{"full_name":"alice/project","default_branch":"main","permissions":{"push":true}}`))
default:
_, _ = w.Write([]byte(`[]`))
}
}))
_, err := (Pulls{Client: client, Repository: "alice/project"}).CreateChoices(context.Background())
require.ErrorContains(t, err, "500")
})
}
}
+9 -2
View File
@@ -141,10 +141,17 @@ type RepositoryPermissions struct {
// viewPermissions reads repository access flags without exposing them on Repository results. // viewPermissions reads repository access flags without exposing them on Repository results.
func (s Repositories) viewPermissions(ctx context.Context, repo string) (RepositoryPermissions, error) { func (s Repositories) viewPermissions(ctx context.Context, repo string) (RepositoryPermissions, error) {
item, err := s.get(ctx, repo) item, err := s.get(ctx, repo)
if err != nil || item.Permissions == nil { if err != nil {
return RepositoryPermissions{}, err return RepositoryPermissions{}, err
} }
return RepositoryPermissions{Admin: item.Permissions.Admin, Push: item.Permissions.Push, Pull: item.Permissions.Pull}, nil return permissionsFrom(item), nil
}
func permissionsFrom(item *gitea.Repository) RepositoryPermissions {
if item.Permissions == nil {
return RepositoryPermissions{}
}
return RepositoryPermissions{Admin: item.Permissions.Admin, Push: item.Permissions.Push, Pull: item.Permissions.Pull}
} }
// Assignee is a user that can be assigned on repository issues. // Assignee is a user that can be assigned on repository issues.