From 251a67a8b2145381b4c0552d25727e10cacc8628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Tue, 7 Apr 2026 15:09:35 +0200 Subject: [PATCH 1/4] graphql: add tree-->lastcommit, further optimize history querying (#1547) --- api/graphql/graph/git.generated.go | 119 ++++++++++++++++-- api/graphql/graph/repository.generated.go | 9 +- api/graphql/graph/root_.generated.go | 23 +++- api/graphql/graphql_test.go | 21 ++++ api/graphql/models/models.go | 12 ++ api/graphql/resolvers/git.go | 26 +++- api/graphql/resolvers/repo.go | 10 +- api/graphql/resolvers/root.go | 8 +- api/graphql/schema/git.graphql | 5 +- repository/gogit.go | 147 ++++++++++++---------- repository/repo_testing.go | 33 ++++- 11 files changed, 317 insertions(+), 96 deletions(-) diff --git a/api/graphql/graph/git.generated.go b/api/graphql/graph/git.generated.go index d493c8ae7d9288e8aa6e26d6a274ff3cb5e1dab1..1c1e59ce605b3d71003f7f0fe6060ffc270a7a13 100644 --- a/api/graphql/graph/git.generated.go +++ b/api/graphql/graph/git.generated.go @@ -28,6 +28,9 @@ type GitCommitResolver interface { Files(ctx context.Context, obj *models.GitCommitMeta, after *string, before *string, first *int, last *int) (*models.GitChangedFileConnection, error) Diff(ctx context.Context, obj *models.GitCommitMeta, path string) (*repository.FileDiff, error) } +type GitTreeEntryResolver interface { + LastCommit(ctx context.Context, obj *models.GitTreeEntry) (*models.GitCommitMeta, error) +} // endregion ************************** generated!.gotpl ************************** @@ -2514,7 +2517,7 @@ func (ec *executionContext) fieldContext_GitRefConnection_totalCount(_ context.C return fc, nil } -func (ec *executionContext) _GitTreeEntry_name(ctx context.Context, field graphql.CollectedField, obj *repository.TreeEntry) (ret graphql.Marshaler) { +func (ec *executionContext) _GitTreeEntry_name(ctx context.Context, field graphql.CollectedField, obj *models.GitTreeEntry) (ret graphql.Marshaler) { fc, err := ec.fieldContext_GitTreeEntry_name(ctx, field) if err != nil { return graphql.Null @@ -2558,7 +2561,7 @@ func (ec *executionContext) fieldContext_GitTreeEntry_name(_ context.Context, fi return fc, nil } -func (ec *executionContext) _GitTreeEntry_type(ctx context.Context, field graphql.CollectedField, obj *repository.TreeEntry) (ret graphql.Marshaler) { +func (ec *executionContext) _GitTreeEntry_type(ctx context.Context, field graphql.CollectedField, obj *models.GitTreeEntry) (ret graphql.Marshaler) { fc, err := ec.fieldContext_GitTreeEntry_type(ctx, field) if err != nil { return graphql.Null @@ -2602,7 +2605,7 @@ func (ec *executionContext) fieldContext_GitTreeEntry_type(_ context.Context, fi return fc, nil } -func (ec *executionContext) _GitTreeEntry_hash(ctx context.Context, field graphql.CollectedField, obj *repository.TreeEntry) (ret graphql.Marshaler) { +func (ec *executionContext) _GitTreeEntry_hash(ctx context.Context, field graphql.CollectedField, obj *models.GitTreeEntry) (ret graphql.Marshaler) { fc, err := ec.fieldContext_GitTreeEntry_hash(ctx, field) if err != nil { return graphql.Null @@ -2646,6 +2649,69 @@ func (ec *executionContext) fieldContext_GitTreeEntry_hash(_ context.Context, fi return fc, nil } +func (ec *executionContext) _GitTreeEntry_lastCommit(ctx context.Context, field graphql.CollectedField, obj *models.GitTreeEntry) (ret graphql.Marshaler) { + fc, err := ec.fieldContext_GitTreeEntry_lastCommit(ctx, field) + if err != nil { + return graphql.Null + } + ctx = graphql.WithFieldContext(ctx, fc) + defer func() { + if r := recover(); r != nil { + ec.Error(ctx, ec.Recover(ctx, r)) + ret = graphql.Null + } + }() + resTmp, err := ec.ResolverMiddleware(ctx, func(rctx context.Context) (any, error) { + ctx = rctx // use context from middleware stack in children + return ec.resolvers.GitTreeEntry().LastCommit(rctx, obj) + }) + if err != nil { + ec.Error(ctx, err) + return graphql.Null + } + if resTmp == nil { + return graphql.Null + } + res := resTmp.(*models.GitCommitMeta) + fc.Result = res + return ec.marshalOGitCommit2ᚖgithubᚗcomᚋgitᚑbugᚋgitᚑbugᚋapiᚋgraphqlᚋmodelsᚐGitCommitMeta(ctx, field.Selections, res) +} + +func (ec *executionContext) fieldContext_GitTreeEntry_lastCommit(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { + fc = &graphql.FieldContext{ + Object: "GitTreeEntry", + Field: field, + IsMethod: true, + IsResolver: true, + Child: func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { + switch field.Name { + case "hash": + return ec.fieldContext_GitCommit_hash(ctx, field) + case "shortHash": + return ec.fieldContext_GitCommit_shortHash(ctx, field) + case "message": + return ec.fieldContext_GitCommit_message(ctx, field) + case "fullMessage": + return ec.fieldContext_GitCommit_fullMessage(ctx, field) + case "authorName": + return ec.fieldContext_GitCommit_authorName(ctx, field) + case "authorEmail": + return ec.fieldContext_GitCommit_authorEmail(ctx, field) + case "date": + return ec.fieldContext_GitCommit_date(ctx, field) + case "parents": + return ec.fieldContext_GitCommit_parents(ctx, field) + case "files": + return ec.fieldContext_GitCommit_files(ctx, field) + case "diff": + return ec.fieldContext_GitCommit_diff(ctx, field) + } + return nil, fmt.Errorf("no field named %q was found under type GitCommit", field.Name) + }, + } + return fc, nil +} + // endregion **************************** field.gotpl ***************************** // region **************************** input.gotpl ***************************** @@ -3427,7 +3493,7 @@ func (ec *executionContext) _GitRefConnection(ctx context.Context, sel ast.Selec var gitTreeEntryImplementors = []string{"GitTreeEntry"} -func (ec *executionContext) _GitTreeEntry(ctx context.Context, sel ast.SelectionSet, obj *repository.TreeEntry) graphql.Marshaler { +func (ec *executionContext) _GitTreeEntry(ctx context.Context, sel ast.SelectionSet, obj *models.GitTreeEntry) graphql.Marshaler { fields := graphql.CollectFields(ec.OperationContext, sel, gitTreeEntryImplementors) out := graphql.NewFieldSet(fields) @@ -3439,18 +3505,51 @@ func (ec *executionContext) _GitTreeEntry(ctx context.Context, sel ast.Selection case "name": out.Values[i] = ec._GitTreeEntry_name(ctx, field, obj) if out.Values[i] == graphql.Null { - out.Invalids++ + atomic.AddUint32(&out.Invalids, 1) } case "type": out.Values[i] = ec._GitTreeEntry_type(ctx, field, obj) if out.Values[i] == graphql.Null { - out.Invalids++ + atomic.AddUint32(&out.Invalids, 1) } case "hash": out.Values[i] = ec._GitTreeEntry_hash(ctx, field, obj) if out.Values[i] == graphql.Null { - out.Invalids++ + atomic.AddUint32(&out.Invalids, 1) + } + case "lastCommit": + field := field + + innerFunc := func(ctx context.Context, _ *graphql.FieldSet) (res graphql.Marshaler) { + defer func() { + if r := recover(); r != nil { + ec.Error(ctx, ec.Recover(ctx, r)) + } + }() + res = ec._GitTreeEntry_lastCommit(ctx, field, obj) + return res + } + + if field.Deferrable != nil { + dfs, ok := deferred[field.Deferrable.Label] + di := 0 + if ok { + dfs.AddField(field) + di = len(dfs.Values) - 1 + } else { + dfs = graphql.NewFieldSet([]graphql.CollectedField{field}) + deferred[field.Deferrable.Label] = dfs + } + dfs.Concurrently(di, func(ctx context.Context) graphql.Marshaler { + return innerFunc(ctx, dfs) + }) + + // don't run the out.Concurrently() call below + out.Values[i] = graphql.Null + continue } + + out.Concurrently(i, func(ctx context.Context) graphql.Marshaler { return innerFunc(ctx, out) }) default: panic("unknown field " + strconv.Quote(field.Name)) } @@ -3890,7 +3989,7 @@ var ( } ) -func (ec *executionContext) marshalNGitTreeEntry2ᚕᚖgithubᚗcomᚋgitᚑbugᚋgitᚑbugᚋrepositoryᚐTreeEntryᚄ(ctx context.Context, sel ast.SelectionSet, v []*repository.TreeEntry) graphql.Marshaler { +func (ec *executionContext) marshalNGitTreeEntry2ᚕᚖgithubᚗcomᚋgitᚑbugᚋgitᚑbugᚋapiᚋgraphqlᚋmodelsᚐGitTreeEntryᚄ(ctx context.Context, sel ast.SelectionSet, v []*models.GitTreeEntry) graphql.Marshaler { ret := make(graphql.Array, len(v)) var wg sync.WaitGroup isLen1 := len(v) == 1 @@ -3914,7 +4013,7 @@ func (ec *executionContext) marshalNGitTreeEntry2ᚕᚖgithubᚗcomᚋgitᚑbug if !isLen1 { defer wg.Done() } - ret[i] = ec.marshalNGitTreeEntry2ᚖgithubᚗcomᚋgitᚑbugᚋgitᚑbugᚋrepositoryᚐTreeEntry(ctx, sel, v[i]) + ret[i] = ec.marshalNGitTreeEntry2ᚖgithubᚗcomᚋgitᚑbugᚋgitᚑbugᚋapiᚋgraphqlᚋmodelsᚐGitTreeEntry(ctx, sel, v[i]) } if isLen1 { f(i) @@ -3934,7 +4033,7 @@ func (ec *executionContext) marshalNGitTreeEntry2ᚕᚖgithubᚗcomᚋgitᚑbug return ret } -func (ec *executionContext) marshalNGitTreeEntry2ᚖgithubᚗcomᚋgitᚑbugᚋgitᚑbugᚋrepositoryᚐTreeEntry(ctx context.Context, sel ast.SelectionSet, v *repository.TreeEntry) graphql.Marshaler { +func (ec *executionContext) marshalNGitTreeEntry2ᚖgithubᚗcomᚋgitᚑbugᚋgitᚑbugᚋapiᚋgraphqlᚋmodelsᚐGitTreeEntry(ctx context.Context, sel ast.SelectionSet, v *models.GitTreeEntry) graphql.Marshaler { if v == nil { if !graphql.HasFieldError(ctx, graphql.GetFieldContext(ctx)) { ec.Errorf(ctx, "the requested element is null which the schema does not allow") diff --git a/api/graphql/graph/repository.generated.go b/api/graphql/graph/repository.generated.go index 989bc4bdf3b5e66d899e4ddc8a8e4dff511220f4..ca746413d1f33f4bc382f3e92fd6d71f8cd7b543 100644 --- a/api/graphql/graph/repository.generated.go +++ b/api/graphql/graph/repository.generated.go @@ -13,7 +13,6 @@ import ( "github.com/99designs/gqlgen/graphql" "github.com/git-bug/git-bug/api/graphql/models" - "github.com/git-bug/git-bug/repository" "github.com/vektah/gqlparser/v2/ast" ) @@ -27,7 +26,7 @@ type RepositoryResolver interface { Identity(ctx context.Context, obj *models.Repository, prefix string) (models.IdentityWrapper, error) UserIdentity(ctx context.Context, obj *models.Repository) (models.IdentityWrapper, error) Refs(ctx context.Context, obj *models.Repository, after *string, before *string, first *int, last *int, typeArg *models.GitRefType) (*models.GitRefConnection, error) - Tree(ctx context.Context, obj *models.Repository, ref string, path *string) ([]*repository.TreeEntry, error) + Tree(ctx context.Context, obj *models.Repository, ref string, path *string) ([]*models.GitTreeEntry, error) Blob(ctx context.Context, obj *models.Repository, ref string, path string) (*models.GitBlob, error) Commits(ctx context.Context, obj *models.Repository, after *string, first *int, ref string, path *string, since *time.Time, until *time.Time) (*models.GitCommitConnection, error) Commit(ctx context.Context, obj *models.Repository, hash string) (*models.GitCommitMeta, error) @@ -1353,9 +1352,9 @@ func (ec *executionContext) _Repository_tree(ctx context.Context, field graphql. } return graphql.Null } - res := resTmp.([]*repository.TreeEntry) + res := resTmp.([]*models.GitTreeEntry) fc.Result = res - return ec.marshalNGitTreeEntry2ᚕᚖgithubᚗcomᚋgitᚑbugᚋgitᚑbugᚋrepositoryᚐTreeEntryᚄ(ctx, field.Selections, res) + return ec.marshalNGitTreeEntry2ᚕᚖgithubᚗcomᚋgitᚑbugᚋgitᚑbugᚋapiᚋgraphqlᚋmodelsᚐGitTreeEntryᚄ(ctx, field.Selections, res) } func (ec *executionContext) fieldContext_Repository_tree(ctx context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { @@ -1372,6 +1371,8 @@ func (ec *executionContext) fieldContext_Repository_tree(ctx context.Context, fi return ec.fieldContext_GitTreeEntry_type(ctx, field) case "hash": return ec.fieldContext_GitTreeEntry_hash(ctx, field) + case "lastCommit": + return ec.fieldContext_GitTreeEntry_lastCommit(ctx, field) } return nil, fmt.Errorf("no field named %q was found under type GitTreeEntry", field.Name) }, diff --git a/api/graphql/graph/root_.generated.go b/api/graphql/graph/root_.generated.go index 202c4391fc1f2715bbd23ab030f93f2c784851df..8d2e2aa1a930023ea1cbb89a694cf09af04ce634 100644 --- a/api/graphql/graph/root_.generated.go +++ b/api/graphql/graph/root_.generated.go @@ -50,6 +50,7 @@ type ResolverRoot interface { BugSetTitleTimelineItem() BugSetTitleTimelineItemResolver Color() ColorResolver GitCommit() GitCommitResolver + GitTreeEntry() GitTreeEntryResolver Identity() IdentityResolver Label() LabelResolver Mutation() MutationResolver @@ -380,6 +381,7 @@ type ComplexityRoot struct { GitTreeEntry struct { Hash func(childComplexity int) int + LastCommit func(childComplexity int) int Name func(childComplexity int) int ObjectType func(childComplexity int) int } @@ -1875,6 +1877,13 @@ func (e *executableSchema) Complexity(ctx context.Context, typeName, field strin return e.complexity.GitTreeEntry.Hash(childComplexity), true + case "GitTreeEntry.lastCommit": + if e.complexity.GitTreeEntry.LastCommit == nil { + break + } + + return e.complexity.GitTreeEntry.LastCommit(childComplexity), true + case "GitTreeEntry.name": if e.complexity.GitTreeEntry.Name == nil { break @@ -3172,13 +3181,16 @@ type GitRef { """An entry in a git tree (directory listing).""" type GitTreeEntry -@goModel(model: "github.com/git-bug/git-bug/repository.TreeEntry") { +@goModel(model: "github.com/git-bug/git-bug/api/graphql/models.GitTreeEntry") { """File or directory name within the parent tree.""" name: String! """Whether this entry is a file, directory, symlink, or submodule.""" type: GitObjectType! @goField(name: "ObjectType") """Git object hash.""" hash: String! + """The last git commit that touched this tree entry. Null when the entry + cannot be resolved within the history depth limit.""" + lastCommit: GitCommit } """The content of a git blob (file).""" @@ -3467,7 +3479,7 @@ type OperationEdge { } `, BuiltIn: false}, {Name: "../schema/repository.graphql", Input: `type Repository { - """The name of the repository""" + """The name of the repository. Null for the default (unnamed) repository in a single-repo setup.""" name: String """All the bugs""" @@ -3484,6 +3496,7 @@ type OperationEdge { query: String ): BugConnection! + """Look up a bug by id prefix. Returns null if no bug matches the prefix.""" bug(prefix: String!): Bug """All the identities""" @@ -3498,6 +3511,7 @@ type OperationEdge { last: Int ): IdentityConnection! + """Look up an identity by id prefix. Returns null if no identity matches the prefix.""" identity(prefix: String!): Identity """The identity created or selected by the user as its own""" @@ -3542,7 +3556,7 @@ type OperationEdge { until: Time ): GitCommitConnection! - """A single commit by hash.""" + """A single commit by hash. Returns null if the hash does not exist in the repository.""" commit(hash: String!): GitCommit """The most recent commit that touched each of the named entries in the @@ -3576,7 +3590,8 @@ type RepositoryEdge { } `, BuiltIn: false}, {Name: "../schema/root.graphql", Input: `type Query { - """Access a repository by reference/name. If no ref is given, the default repository is returned if any.""" + """Access a repository by reference/name. If no ref is given, the default repository is returned if any. + Returns null if the referenced repository does not exist.""" repository(ref: String): Repository """List all registered repositories.""" diff --git a/api/graphql/graphql_test.go b/api/graphql/graphql_test.go index 9ffe7e88562d39c5a43c569ae453ad9b9282bc44..716febc6b10db20e924e6c57f0affa3ae1cdaa36 100644 --- a/api/graphql/graphql_test.go +++ b/api/graphql/graphql_test.go @@ -393,6 +393,27 @@ func TestGitBrowseQueries(t *testing.T) { require.Equal(t, "TREE", byName["src"]) }) + t.Run("tree_lastCommit", func(t *testing.T) { + var resp struct { + Repository struct { + Tree []struct { + Name string + LastCommit struct{ Hash string } + } + } + } + require.NoError(t, c.Post(`query { + repository { tree(ref: "main", path: "") { name lastCommit { hash } } } + }`, &resp)) + byName := make(map[string]string) + for _, e := range resp.Repository.Tree { + byName[e.Name] = e.LastCommit.Hash + } + require.Equal(t, string(c3), byName["README.md"]) // changed in c3 + require.Equal(t, string(c2), byName["main.go"]) // changed in c2 + require.Equal(t, string(c2), byName["src"]) // util.go added in c2 + }) + // ── commits ─────────────────────────────────────────────────────────────── t.Run("commits", func(t *testing.T) { diff --git a/api/graphql/models/models.go b/api/graphql/models/models.go index f130962ee58534187932c79be1b7208f5d825d9d..9678689df9c17e207a28ac046aec47d1cef2f74c 100644 --- a/api/graphql/models/models.go +++ b/api/graphql/models/models.go @@ -23,3 +23,15 @@ type GitCommitMeta struct { Repo *cache.RepoCache repository.CommitMeta } + +// GitTreeEntry wraps a TreeEntry with the repository context (Repo, Ref, Path) +// of the resolution to that tree. SiblingNames lists all entries in the same +// directory so that the first lastCommit resolver call walks history for the whole +// directory at once; subsequent sibling calls hit the cache. +type GitTreeEntry struct { + Repo *cache.RepoCache + Ref string + Path string + SiblingNames []string + repository.TreeEntry +} diff --git a/api/graphql/resolvers/git.go b/api/graphql/resolvers/git.go index 6c7cfeb40f875acc6c2648901770a238be099adc..bddd03c64adb656e01a7a5ef569f5489fd4670dc 100644 --- a/api/graphql/resolvers/git.go +++ b/api/graphql/resolvers/git.go @@ -6,7 +6,6 @@ import ( "github.com/git-bug/git-bug/api/graphql/connections" "github.com/git-bug/git-bug/api/graphql/graph" "github.com/git-bug/git-bug/api/graphql/models" - "github.com/git-bug/git-bug/cache" "github.com/git-bug/git-bug/repository" ) @@ -14,9 +13,7 @@ const blobTruncateSize = 1 << 20 // 1 MiB var _ graph.GitCommitResolver = &gitCommitResolver{} -type gitCommitResolver struct { - cache *cache.MultiRepoCache -} +type gitCommitResolver struct{} func (r gitCommitResolver) ShortHash(_ context.Context, obj *models.GitCommitMeta) (string, error) { s := string(obj.Hash) @@ -72,3 +69,24 @@ func (r gitCommitResolver) Diff(_ context.Context, obj *models.GitCommitMeta, pa } return &fd, nil } + +var _ graph.GitTreeEntryResolver = &gitTreeEntryResolver{} + +type gitTreeEntryResolver struct{} + +func (r gitTreeEntryResolver) LastCommit(_ context.Context, obj *models.GitTreeEntry) (*models.GitCommitMeta, error) { + repo := obj.Repo.BrowseRepo() + // Pass all sibling names so the history walk covers the whole directory, + // which is nearly the same cost as walking for a single entry. + // Concurrent calls for the same directory are deduplicated by a singleflight + // inside LastCommitForEntries; subsequent calls hit the LRU cache. + commits, err := repo.LastCommitForEntries(obj.Ref, obj.Path, obj.SiblingNames) + if err != nil { + return nil, err + } + meta, ok := commits[obj.Name] + if !ok { + return nil, nil + } + return &models.GitCommitMeta{Repo: obj.Repo, CommitMeta: meta}, nil +} diff --git a/api/graphql/resolvers/repo.go b/api/graphql/resolvers/repo.go index cfd816210279a95551fd6203a3c04f8c37b82460..63104480f4a9e3dab8a742babaf87e292dbab132 100644 --- a/api/graphql/resolvers/repo.go +++ b/api/graphql/resolvers/repo.go @@ -260,7 +260,7 @@ func (repoResolver) Refs(_ context.Context, obj *models.Repository, after *strin return connections.Connection(refs, edger, conMaker, input) } -func (repoResolver) Tree(_ context.Context, obj *models.Repository, ref string, path *string) ([]*repository.TreeEntry, error) { +func (repoResolver) Tree(_ context.Context, obj *models.Repository, ref string, path *string) ([]*models.GitTreeEntry, error) { repo := obj.Repo.BrowseRepo() p := "" if path != nil { @@ -270,9 +270,13 @@ func (repoResolver) Tree(_ context.Context, obj *models.Repository, ref string, if err != nil { return nil, err } - ptrs := make([]*repository.TreeEntry, len(entries)) + names := make([]string, len(entries)) + for i, e := range entries { + names[i] = e.Name + } + ptrs := make([]*models.GitTreeEntry, len(entries)) for i := range entries { - ptrs[i] = &entries[i] + ptrs[i] = &models.GitTreeEntry{Repo: obj.Repo, Ref: ref, Path: p, SiblingNames: names, TreeEntry: entries[i]} } return ptrs, nil } diff --git a/api/graphql/resolvers/root.go b/api/graphql/resolvers/root.go index 295b8e161c6486a1a75015c821c3b1773833fd1b..fb9948d129bccb4331fb968597ac93ba880d4b8b 100644 --- a/api/graphql/resolvers/root.go +++ b/api/graphql/resolvers/root.go @@ -58,7 +58,9 @@ func (RootResolver) Bug() graph.BugResolver { } func (r RootResolver) GitCommit() graph.GitCommitResolver { - return &gitCommitResolver{ - cache: r.MultiRepoCache, - } + return &gitCommitResolver{} +} + +func (r RootResolver) GitTreeEntry() graph.GitTreeEntryResolver { + return &gitTreeEntryResolver{} } diff --git a/api/graphql/schema/git.graphql b/api/graphql/schema/git.graphql index d600af5228018ac98ea4416956eaae6631dcb9d7..3e28db7474acfe15d9d11986b8bac1b85dbea3d6 100644 --- a/api/graphql/schema/git.graphql +++ b/api/graphql/schema/git.graphql @@ -14,13 +14,16 @@ type GitRef { """An entry in a git tree (directory listing).""" type GitTreeEntry -@goModel(model: "github.com/git-bug/git-bug/repository.TreeEntry") { +@goModel(model: "github.com/git-bug/git-bug/api/graphql/models.GitTreeEntry") { """File or directory name within the parent tree.""" name: String! """Whether this entry is a file, directory, symlink, or submodule.""" type: GitObjectType! @goField(name: "ObjectType") """Git object hash.""" hash: String! + """The last git commit that touched this tree entry. Null when the entry + cannot be resolved within the history depth limit.""" + lastCommit: GitCommit } """The content of a git blob (file).""" diff --git a/repository/gogit.go b/repository/gogit.go index 397cadf3dfad6ba7e28293a4c45f06da8179c829..000f1ccbce1d27b84709bb4a138dd8bb657f3936 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -23,6 +23,7 @@ import ( "github.com/go-git/go-git/v5/plumbing/object" lru "github.com/hashicorp/golang-lru/v2" "golang.org/x/sync/errgroup" + "golang.org/x/sync/singleflight" "golang.org/x/sys/execabs" "github.com/git-bug/git-bug/util/lamport" @@ -66,6 +67,9 @@ type GoGitRepo struct { // across refs that point to the same directory tree. The LRU bounds // memory to lastCommitCacheSize unique (treeHash, directory) pairs. lastCommitCache *lru.Cache[string, map[string]CommitMeta] + // lastCommitSF deduplicates concurrent walks for the same cache key so + // that a cold cache under parallel requests triggers only one history walk. + lastCommitSF singleflight.Group keyring Keyring localStorage LocalStorage @@ -1330,93 +1334,106 @@ func (repo *GoGitRepo) LastCommitForEntries(ref, path string, names []string) (m return result, nil } - // Cache miss: walk history for ALL entries in this directory so the - // cached result is complete and valid for any future name subset. - remaining := make(map[string]bool, len(startEntries)) - for name := range startEntries { - remaining[name] = true - } - result := make(map[string]CommitMeta, len(remaining)) - - repo.rMutex.Lock() + // Cache miss: use singleflight so that concurrent calls for the same + // directory share one history walk instead of each doing their own. + val, err, _ := repo.lastCommitSF.Do(cacheKey, func() (any, error) { + // Re-check inside Do: another goroutine may have populated the cache + // between our initial Get and acquiring the singleflight key. + if cached, ok := repo.lastCommitCache.Get(cacheKey); ok { + return cached, nil + } - iter, err := repo.r.Log(&gogit.LogOptions{ - From: startHash, - Order: gogit.LogOrderCommitterTime, - }) - if err != nil { - repo.rMutex.Unlock() - return nil, err - } + remaining := make(map[string]bool, len(startEntries)) + for name := range startEntries { + remaining[name] = true + } + result := make(map[string]CommitMeta, len(remaining)) - // Seed the parent-reuse cache with the entries we already fetched above - // so the first iteration's current-tree read is skipped for free. - // In a linear history this halves tree reads for every subsequent step: - // the parent fetched at depth D is the current commit at depth D+1. - cachedParentHash := startHash - cachedParentEntries := startEntries + repo.rMutex.Lock() - for depth := 0; len(remaining) > 0 && depth < lastCommitDepthLimit; depth++ { - c, err := iter.Next() - if err == io.EOF { - break - } + iter, err := repo.r.Log(&gogit.LogOptions{ + From: startHash, + Order: gogit.LogOrderCommitterTime, + }) if err != nil { - iter.Close() repo.rMutex.Unlock() return nil, err } - var currentEntries map[string]plumbing.Hash - if c.Hash == cachedParentHash && cachedParentEntries != nil { - currentEntries = cachedParentEntries - } else { - _, currentEntries, err = treeEntriesAtPath(c, path) + // Seed the parent-reuse cache with the entries we already fetched above + // so the first iteration's current-tree read is skipped for free. + // In a linear history this halves tree reads for every subsequent step: + // the parent fetched at depth D is the current commit at depth D+1. + cachedParentHash := startHash + cachedParentEntries := startEntries + + for depth := 0; len(remaining) > 0 && depth < lastCommitDepthLimit; depth++ { + c, err := iter.Next() + if err == io.EOF { + break + } if err != nil { - // path may not exist in this commit; treat as empty - currentEntries = map[string]plumbing.Hash{} + iter.Close() + repo.rMutex.Unlock() + return nil, err } - } - var parentEntries map[string]plumbing.Hash - cachedParentHash = plumbing.ZeroHash - cachedParentEntries = nil - if len(c.ParentHashes) > 0 { - if parent, err := c.Parents().Next(); err == nil { - _, parentEntries, _ = treeEntriesAtPath(parent, path) - cachedParentHash = c.ParentHashes[0] - cachedParentEntries = parentEntries + var currentEntries map[string]plumbing.Hash + if c.Hash == cachedParentHash && cachedParentEntries != nil { + currentEntries = cachedParentEntries + } else { + _, currentEntries, err = treeEntriesAtPath(c, path) + if err != nil { + // path may not exist in this commit; treat as empty + currentEntries = map[string]plumbing.Hash{} + } + } + + var parentEntries map[string]plumbing.Hash + cachedParentHash = plumbing.ZeroHash + cachedParentEntries = nil + if len(c.ParentHashes) > 0 { + if parent, err := c.Parents().Next(); err == nil { + _, parentEntries, _ = treeEntriesAtPath(parent, path) + cachedParentHash = c.ParentHashes[0] + cachedParentEntries = parentEntries + } } - } - meta := commitToMeta(c) - for name := range remaining { - curHash, inCurrent := currentEntries[name] - parentHash, inParent := parentEntries[name] - if inCurrent != inParent || (inCurrent && curHash != parentHash) { - result[name] = meta - delete(remaining, name) + meta := commitToMeta(c) + for name := range remaining { + curHash, inCurrent := currentEntries[name] + parentHash, inParent := parentEntries[name] + if inCurrent != inParent || (inCurrent && curHash != parentHash) { + result[name] = meta + delete(remaining, name) + } } } - } - iter.Close() - repo.rMutex.Unlock() + iter.Close() + repo.rMutex.Unlock() - // Store a defensive copy so that callers cannot mutate cached entries. - // The cached map contains all directory entries, not just the requested - // names, so future calls for the same directory are fully served from - // cache regardless of which names they request. - cached := make(map[string]CommitMeta, len(result)) - for k, v := range result { - cached[k] = v + // Store a defensive copy so that callers cannot mutate cached entries. + // The cached map contains all directory entries, not just the requested + // names, so future calls for the same directory are fully served from + // cache regardless of which names they request. + cached := make(map[string]CommitMeta, len(result)) + for k, v := range result { + cached[k] = v + } + repo.lastCommitCache.Add(cacheKey, cached) + return cached, nil + }) + if err != nil { + return nil, err } - repo.lastCommitCache.Add(cacheKey, cached) // Return only the entries that were requested. + full := val.(map[string]CommitMeta) filtered := make(map[string]CommitMeta, len(names)) for _, n := range names { - if m, ok := result[n]; ok { + if m, ok := full[n]; ok { filtered[n] = m } } diff --git a/repository/repo_testing.go b/repository/repo_testing.go index ff78a2d949d0ae40c22c2375fe414003e0744045..58ee39b286f8e2deeab71bf649e18aa6e095f61e 100644 --- a/repository/repo_testing.go +++ b/repository/repo_testing.go @@ -4,6 +4,7 @@ import ( "io" "math/rand" "os" + "sync" "testing" "time" @@ -601,7 +602,7 @@ func RepoBrowseTest(t *testing.T, repo browsable) { require.Equal(t, c1, readmeLog[1].Hash) }) - t.Run("CommitLog/since-until", func(t *testing.T) { + t.Run("CommitLog_since-until", func(t *testing.T) { // since = far future → no commits future := time.Now().Add(24 * time.Hour) none, err := repo.CommitLog("main", "", 10, "", &future, nil) @@ -654,7 +655,7 @@ func RepoBrowseTest(t *testing.T, repo browsable) { require.NotContains(t, partial, "ghost.txt") }) - t.Run("LastCommitForEntries/cache-subset", func(t *testing.T) { + t.Run("LastCommitForEntries_cache-subset", func(t *testing.T) { // First call with one name — seeds (or hits) the cache for this directory. r1, err := repo.LastCommitForEntries("main", "", []string{"README.md"}) require.NoError(t, err) @@ -676,6 +677,34 @@ func RepoBrowseTest(t *testing.T, repo browsable) { require.Equal(t, c2, r3["main.go"].Hash) }) + t.Run("LastCommitForEntries_concurrent", func(t *testing.T) { + // Use the "feature" ref so the cache is cold for this key. + // This exercises the singleflight path. + // The race detector will catch any data races in the cache or walk logic. + const workers = 20 + type result struct { + m map[string]CommitMeta + err error + } + results := make([]result, workers) + var wg sync.WaitGroup + wg.Add(workers) + for i := range workers { + go func() { + defer wg.Done() + m, err := repo.LastCommitForEntries("feature", "", []string{"README.md", "main.go", "src"}) + results[i] = result{m, err} + }() + } + wg.Wait() + for _, r := range results { + require.NoError(t, r.err) + require.Equal(t, c1, r.m["README.md"].Hash) // feature is at c2, README unchanged since c1 + require.Equal(t, c2, r.m["main.go"].Hash) + require.Equal(t, c2, r.m["src"].Hash) + } + }) + // ── CommitDetail ────────────────────────────────────────────────────────── t.Run("CommitDetail", func(t *testing.T) { From e77b465e4161db0c8ef5a877889cc83399d7cd96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Wed, 8 Apr 2026 15:31:07 +0200 Subject: [PATCH 2/4] repo: ReadData return a reader, improve the http handler (#1549) --- api/http/git_file_handler.go | 46 +++++++++++++++++++++++++----- api/http/git_file_handlers_test.go | 2 ++ cache/repo_cache_common.go | 3 +- entities/identity/identity.go | 3 +- entity/dag/operation_pack.go | 8 +++++- repository/gogit.go | 12 ++------ repository/mock_repo.go | 4 +-- repository/repo.go | 5 ++-- repository/repo_testing.go | 5 +++- 9 files changed, 64 insertions(+), 24 deletions(-) diff --git a/api/http/git_file_handler.go b/api/http/git_file_handler.go index 5db54a5228c8f1604a4815733e03b0364d583be9..a8aeec78eda328f9adb449d9df92bab7a1f931ff 100644 --- a/api/http/git_file_handler.go +++ b/api/http/git_file_handler.go @@ -2,8 +2,9 @@ package http import ( "bytes" + "errors" + "io" "net/http" - "time" "github.com/gorilla/mux" @@ -47,15 +48,46 @@ func (gfh *gitFileHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) { return } - // TODO: this mean that the whole file will be buffered in memory - // This can be a problem for big files. There might be a way around - // that by implementing a io.ReadSeeker that would read and discard - // data when a seek is called. - data, err := repo.ReadData(hash) + reader, err := repo.ReadData(hash) + if errors.Is(err, repository.ErrNotFound) { + http.Error(rw, "blob not found", http.StatusNotFound) + return + } if err != nil { http.Error(rw, err.Error(), http.StatusInternalServerError) return } + defer func() { + _ = reader.Close() + }() + + serveContent(rw, r, reader) +} + +// serveContent is a somewhat equivalent of http.serveContent, without support for range request. +// This is necessary as the repo (and go-git)'s data reader doesn't support Seek(). +func serveContent(w http.ResponseWriter, r *http.Request, content io.Reader) { + if w.Header().Get("Content-Type") == "" { + // Sniff the type from the first up to 512 bytes. + var buf [512]byte + n, err := io.ReadFull(content, buf[:]) + switch err { + case nil: + w.Header().Set("Content-Type", http.DetectContentType(buf[:n])) + content = io.MultiReader(bytes.NewReader(buf[:n]), content) + case io.ErrUnexpectedEOF, io.EOF: + w.Header().Set("Content-Type", http.DetectContentType(buf[:n])) + content = bytes.NewReader(buf[:n]) + default: + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + } + + w.WriteHeader(http.StatusOK) + if r.Method == http.MethodHead { + return + } - http.ServeContent(rw, r, "", time.Now(), bytes.NewReader(data)) + _, _ = io.Copy(w, content) } diff --git a/api/http/git_file_handlers_test.go b/api/http/git_file_handlers_test.go index 830e035e402379aa788e2c7727ad6f8c72616d8b..d7a215322b8d06eb9dd952d67c0eae8d83c5e3e8 100644 --- a/api/http/git_file_handlers_test.go +++ b/api/http/git_file_handlers_test.go @@ -87,6 +87,8 @@ func TestGitFileHandlers(t *testing.T) { downloadHandler.ServeHTTP(w, r) assert.Equal(t, http.StatusOK, w.Code) + // check if content type detection is working + assert.Equal(t, "image/png", w.Header().Get("Content-Type")) assert.Equal(t, data.Bytes(), w.Body.Bytes()) } diff --git a/cache/repo_cache_common.go b/cache/repo_cache_common.go index 838f056d2ecbf53912a7fc334e4cd86d07fb6f43..cd8b13ec43c285d669f7688a363bfe38866961ce 100644 --- a/cache/repo_cache_common.go +++ b/cache/repo_cache_common.go @@ -1,6 +1,7 @@ package cache import ( + "io" "sync" "github.com/pkg/errors" @@ -71,7 +72,7 @@ func (c *RepoCache) LocalStorage() repository.LocalStorage { } // ReadData will attempt to read arbitrary data from the given hash -func (c *RepoCache) ReadData(hash repository.Hash) ([]byte, error) { +func (c *RepoCache) ReadData(hash repository.Hash) (io.ReadCloser, error) { return c.repo.ReadData(hash) } diff --git a/entities/identity/identity.go b/entities/identity/identity.go index 9a10098fa21dd798506e9dcd1a364d62277d3313..1436259afccb74a489903e599bdbb354aaae5cf2 100644 --- a/entities/identity/identity.go +++ b/entities/identity/identity.go @@ -136,7 +136,8 @@ func read(repo repository.Repo, ref string) (*Identity, error) { } var version version - err = json.Unmarshal(data, &version) + err = json.NewDecoder(data).Decode(&version) + _ = data.Close() if err != nil { return nil, errors.Wrapf(err, "failed to decode Identity version json %s", hash) } diff --git a/entity/dag/operation_pack.go b/entity/dag/operation_pack.go index 1fd4daf7d7c9ef7af17e381b07c95732bd246cfa..eab42bc57d0d79d5f0d2e351f650f21cd37b874f 100644 --- a/entity/dag/operation_pack.go +++ b/entity/dag/operation_pack.go @@ -3,6 +3,7 @@ package dag import ( "encoding/json" "fmt" + "io" "strconv" "strings" @@ -242,7 +243,12 @@ func readOperationPack(def Definition, repo repository.RepoData, resolvers entit for _, entry := range entries { switch { case entry.Name == opsEntryName: - data, err := repo.ReadData(entry.Hash) + r, err := repo.ReadData(entry.Hash) + if err != nil { + return nil, errors.Wrap(err, "failed to read git blob data") + } + data, err := io.ReadAll(r) + _ = r.Close() if err != nil { return nil, errors.Wrap(err, "failed to read git blob data") } diff --git a/repository/gogit.go b/repository/gogit.go index 000f1ccbce1d27b84709bb4a138dd8bb657f3936..797837b1491eb072422cef60e8a2c05c05fe2c28 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -550,25 +550,19 @@ func (repo *GoGitRepo) StoreData(data []byte) (Hash, error) { } // ReadData will attempt to read arbitrary data from the given hash -func (repo *GoGitRepo) ReadData(hash Hash) ([]byte, error) { +func (repo *GoGitRepo) ReadData(hash Hash) (io.ReadCloser, error) { repo.rMutex.Lock() defer repo.rMutex.Unlock() obj, err := repo.r.BlobObject(plumbing.NewHash(hash.String())) - if err == plumbing.ErrObjectNotFound { + if errors.Is(err, plumbing.ErrObjectNotFound) { return nil, ErrNotFound } if err != nil { return nil, err } - r, err := obj.Reader() - if err != nil { - return nil, err - } - - // TODO: return a io.Reader instead - return io.ReadAll(r) + return obj.Reader() } // StoreTree will store a mapping key-->Hash as a Git tree diff --git a/repository/mock_repo.go b/repository/mock_repo.go index d2ed2e3e05c7aafb7be74aac67031e35551d3e14..01d48f52b2fadceee976d6e8fcf0ce0270ec7ca9 100644 --- a/repository/mock_repo.go +++ b/repository/mock_repo.go @@ -265,13 +265,13 @@ func (r *mockRepoDataBrowse) StoreData(data []byte) (Hash, error) { return hash, nil } -func (r *mockRepoDataBrowse) ReadData(hash Hash) ([]byte, error) { +func (r *mockRepoDataBrowse) ReadData(hash Hash) (io.ReadCloser, error) { data, ok := r.blobs[hash] if !ok { return nil, ErrNotFound } - return data, nil + return io.NopCloser(bytes.NewReader(data)), nil } func (r *mockRepoDataBrowse) StoreTree(entries []TreeEntry) (Hash, error) { diff --git a/repository/repo.go b/repository/repo.go index 469ff24aaf563412ab459255d13d144e4992e2a3..72bcfe3fea388dfe2a963deb33a730d359710f3e 100644 --- a/repository/repo.go +++ b/repository/repo.go @@ -147,9 +147,10 @@ type RepoData interface { // StoreData will store arbitrary data and return the corresponding hash StoreData(data []byte) (Hash, error) - // ReadData will attempt to read arbitrary data from the given hash + // ReadData returns a reader for arbitrary data associated with the given hash. // Returns ErrNotFound if not found. - ReadData(hash Hash) ([]byte, error) + // The caller must close the reader. + ReadData(hash Hash) (io.ReadCloser, error) // StoreTree will store a mapping key-->Hash as a Git tree StoreTree(mapping []TreeEntry) (Hash, error) diff --git a/repository/repo_testing.go b/repository/repo_testing.go index 58ee39b286f8e2deeab71bf649e18aa6e095f61e..2ba28a1856759f557047d833c95c09fc0cad7706 100644 --- a/repository/repo_testing.go +++ b/repository/repo_testing.go @@ -107,7 +107,10 @@ func RepoDataTest(t *testing.T, repo RepoData) { require.NoError(t, err) require.True(t, blobHash1.IsValid()) - blob1Read, err := repo.ReadData(blobHash1) + blob1Reader, err := repo.ReadData(blobHash1) + require.NoError(t, err) + defer func() { require.NoError(t, blob1Reader.Close()) }() + blob1Read, err := io.ReadAll(blob1Reader) require.NoError(t, err) require.Equal(t, data, blob1Read) From 66f1bc37d66f25a24a185f09f30f390eb436e13e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Wed, 8 Apr 2026 16:54:45 +0200 Subject: [PATCH 3/4] graphql: replace GitRef.isDefault by Repository.head (#1551) isDefault was barely working --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- api/graphql/graph/git.generated.go | 51 ----------- api/graphql/graph/repository.generated.go | 101 ++++++++++++++++++++++ api/graphql/graph/root.generated.go | 2 + api/graphql/graph/root_.generated.go | 22 ++--- api/graphql/models/gen_models.go | 2 - api/graphql/resolvers/repo.go | 12 ++- api/graphql/schema/git.graphql | 2 - api/graphql/schema/repository.graphql | 5 ++ repository/browse.go | 5 +- repository/gogit.go | 62 ++++++------- repository/mock_repo.go | 35 +++++--- repository/repo.go | 6 +- repository/repo_testing.go | 13 ++- 13 files changed, 197 insertions(+), 121 deletions(-) diff --git a/api/graphql/graph/git.generated.go b/api/graphql/graph/git.generated.go index 1c1e59ce605b3d71003f7f0fe6060ffc270a7a13..4f719b7dd1cdb6c4bb3c6930b0b3bce6210001dd 100644 --- a/api/graphql/graph/git.generated.go +++ b/api/graphql/graph/git.generated.go @@ -2319,50 +2319,6 @@ func (ec *executionContext) fieldContext_GitRef_hash(_ context.Context, field gr return fc, nil } -func (ec *executionContext) _GitRef_isDefault(ctx context.Context, field graphql.CollectedField, obj *models.GitRef) (ret graphql.Marshaler) { - fc, err := ec.fieldContext_GitRef_isDefault(ctx, field) - if err != nil { - return graphql.Null - } - ctx = graphql.WithFieldContext(ctx, fc) - defer func() { - if r := recover(); r != nil { - ec.Error(ctx, ec.Recover(ctx, r)) - ret = graphql.Null - } - }() - resTmp, err := ec.ResolverMiddleware(ctx, func(rctx context.Context) (any, error) { - ctx = rctx // use context from middleware stack in children - return obj.IsDefault, nil - }) - if err != nil { - ec.Error(ctx, err) - return graphql.Null - } - if resTmp == nil { - if !graphql.HasFieldError(ctx, fc) { - ec.Errorf(ctx, "must not be null") - } - return graphql.Null - } - res := resTmp.(bool) - fc.Result = res - return ec.marshalNBoolean2bool(ctx, field.Selections, res) -} - -func (ec *executionContext) fieldContext_GitRef_isDefault(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { - fc = &graphql.FieldContext{ - Object: "GitRef", - Field: field, - IsMethod: false, - IsResolver: false, - Child: func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { - return nil, errors.New("field of type Boolean does not have child fields") - }, - } - return fc, nil -} - func (ec *executionContext) _GitRefConnection_nodes(ctx context.Context, field graphql.CollectedField, obj *models.GitRefConnection) (ret graphql.Marshaler) { fc, err := ec.fieldContext_GitRefConnection_nodes(ctx, field) if err != nil { @@ -2410,8 +2366,6 @@ func (ec *executionContext) fieldContext_GitRefConnection_nodes(_ context.Contex return ec.fieldContext_GitRef_type(ctx, field) case "hash": return ec.fieldContext_GitRef_hash(ctx, field) - case "isDefault": - return ec.fieldContext_GitRef_isDefault(ctx, field) } return nil, fmt.Errorf("no field named %q was found under type GitRef", field.Name) }, @@ -3414,11 +3368,6 @@ func (ec *executionContext) _GitRef(ctx context.Context, sel ast.SelectionSet, o if out.Values[i] == graphql.Null { out.Invalids++ } - case "isDefault": - out.Values[i] = ec._GitRef_isDefault(ctx, field, obj) - if out.Values[i] == graphql.Null { - out.Invalids++ - } default: panic("unknown field " + strconv.Quote(field.Name)) } diff --git a/api/graphql/graph/repository.generated.go b/api/graphql/graph/repository.generated.go index ca746413d1f33f4bc382f3e92fd6d71f8cd7b543..1ed350b21067bf6bca5e80f932c0871337352724 100644 --- a/api/graphql/graph/repository.generated.go +++ b/api/graphql/graph/repository.generated.go @@ -31,6 +31,7 @@ type RepositoryResolver interface { Commits(ctx context.Context, obj *models.Repository, after *string, first *int, ref string, path *string, since *time.Time, until *time.Time) (*models.GitCommitConnection, error) Commit(ctx context.Context, obj *models.Repository, hash string) (*models.GitCommitMeta, error) LastCommits(ctx context.Context, obj *models.Repository, ref string, path *string, names []string) ([]*models.GitLastCommit, error) + Head(ctx context.Context, obj *models.Repository) (*models.GitCommitMeta, error) ValidLabels(ctx context.Context, obj *models.Repository, after *string, before *string, first *int, last *int) (*models.LabelConnection, error) } @@ -1655,6 +1656,69 @@ func (ec *executionContext) fieldContext_Repository_lastCommits(ctx context.Cont return fc, nil } +func (ec *executionContext) _Repository_head(ctx context.Context, field graphql.CollectedField, obj *models.Repository) (ret graphql.Marshaler) { + fc, err := ec.fieldContext_Repository_head(ctx, field) + if err != nil { + return graphql.Null + } + ctx = graphql.WithFieldContext(ctx, fc) + defer func() { + if r := recover(); r != nil { + ec.Error(ctx, ec.Recover(ctx, r)) + ret = graphql.Null + } + }() + resTmp, err := ec.ResolverMiddleware(ctx, func(rctx context.Context) (any, error) { + ctx = rctx // use context from middleware stack in children + return ec.resolvers.Repository().Head(rctx, obj) + }) + if err != nil { + ec.Error(ctx, err) + return graphql.Null + } + if resTmp == nil { + return graphql.Null + } + res := resTmp.(*models.GitCommitMeta) + fc.Result = res + return ec.marshalOGitCommit2ᚖgithubᚗcomᚋgitᚑbugᚋgitᚑbugᚋapiᚋgraphqlᚋmodelsᚐGitCommitMeta(ctx, field.Selections, res) +} + +func (ec *executionContext) fieldContext_Repository_head(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { + fc = &graphql.FieldContext{ + Object: "Repository", + Field: field, + IsMethod: true, + IsResolver: true, + Child: func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { + switch field.Name { + case "hash": + return ec.fieldContext_GitCommit_hash(ctx, field) + case "shortHash": + return ec.fieldContext_GitCommit_shortHash(ctx, field) + case "message": + return ec.fieldContext_GitCommit_message(ctx, field) + case "fullMessage": + return ec.fieldContext_GitCommit_fullMessage(ctx, field) + case "authorName": + return ec.fieldContext_GitCommit_authorName(ctx, field) + case "authorEmail": + return ec.fieldContext_GitCommit_authorEmail(ctx, field) + case "date": + return ec.fieldContext_GitCommit_date(ctx, field) + case "parents": + return ec.fieldContext_GitCommit_parents(ctx, field) + case "files": + return ec.fieldContext_GitCommit_files(ctx, field) + case "diff": + return ec.fieldContext_GitCommit_diff(ctx, field) + } + return nil, fmt.Errorf("no field named %q was found under type GitCommit", field.Name) + }, + } + return fc, nil +} + func (ec *executionContext) _Repository_validLabels(ctx context.Context, field graphql.CollectedField, obj *models.Repository) (ret graphql.Marshaler) { fc, err := ec.fieldContext_Repository_validLabels(ctx, field) if err != nil { @@ -1833,6 +1897,8 @@ func (ec *executionContext) fieldContext_RepositoryConnection_nodes(_ context.Co return ec.fieldContext_Repository_commit(ctx, field) case "lastCommits": return ec.fieldContext_Repository_lastCommits(ctx, field) + case "head": + return ec.fieldContext_Repository_head(ctx, field) case "validLabels": return ec.fieldContext_Repository_validLabels(ctx, field) } @@ -2047,6 +2113,8 @@ func (ec *executionContext) fieldContext_RepositoryEdge_node(_ context.Context, return ec.fieldContext_Repository_commit(ctx, field) case "lastCommits": return ec.fieldContext_Repository_lastCommits(ctx, field) + case "head": + return ec.fieldContext_Repository_head(ctx, field) case "validLabels": return ec.fieldContext_Repository_validLabels(ctx, field) } @@ -2492,6 +2560,39 @@ func (ec *executionContext) _Repository(ctx context.Context, sel ast.SelectionSe continue } + out.Concurrently(i, func(ctx context.Context) graphql.Marshaler { return innerFunc(ctx, out) }) + case "head": + field := field + + innerFunc := func(ctx context.Context, _ *graphql.FieldSet) (res graphql.Marshaler) { + defer func() { + if r := recover(); r != nil { + ec.Error(ctx, ec.Recover(ctx, r)) + } + }() + res = ec._Repository_head(ctx, field, obj) + return res + } + + if field.Deferrable != nil { + dfs, ok := deferred[field.Deferrable.Label] + di := 0 + if ok { + dfs.AddField(field) + di = len(dfs.Values) - 1 + } else { + dfs = graphql.NewFieldSet([]graphql.CollectedField{field}) + deferred[field.Deferrable.Label] = dfs + } + dfs.Concurrently(di, func(ctx context.Context) graphql.Marshaler { + return innerFunc(ctx, dfs) + }) + + // don't run the out.Concurrently() call below + out.Values[i] = graphql.Null + continue + } + out.Concurrently(i, func(ctx context.Context) graphql.Marshaler { return innerFunc(ctx, out) }) case "validLabels": field := field diff --git a/api/graphql/graph/root.generated.go b/api/graphql/graph/root.generated.go index 3a487d9862408b03e1365a06f962c7d82d49d507..f98f85e51aac8785a0ff75ace48f40a9395fb04b 100644 --- a/api/graphql/graph/root.generated.go +++ b/api/graphql/graph/root.generated.go @@ -1082,6 +1082,8 @@ func (ec *executionContext) fieldContext_Query_repository(ctx context.Context, f return ec.fieldContext_Repository_commit(ctx, field) case "lastCommits": return ec.fieldContext_Repository_lastCommits(ctx, field) + case "head": + return ec.fieldContext_Repository_head(ctx, field) case "validLabels": return ec.fieldContext_Repository_validLabels(ctx, field) } diff --git a/api/graphql/graph/root_.generated.go b/api/graphql/graph/root_.generated.go index 8d2e2aa1a930023ea1cbb89a694cf09af04ce634..dfccd87bf2f78a2ce5af36870588330de8159713 100644 --- a/api/graphql/graph/root_.generated.go +++ b/api/graphql/graph/root_.generated.go @@ -367,7 +367,6 @@ type ComplexityRoot struct { GitRef struct { Hash func(childComplexity int) int - IsDefault func(childComplexity int) int Name func(childComplexity int) int ShortName func(childComplexity int) int Type func(childComplexity int) int @@ -479,6 +478,7 @@ type ComplexityRoot struct { Bug func(childComplexity int, prefix string) int Commit func(childComplexity int, hash string) int Commits func(childComplexity int, after *string, first *int, ref string, path *string, since *time.Time, until *time.Time) int + Head func(childComplexity int) int Identity func(childComplexity int, prefix string) int LastCommits func(childComplexity int, ref string, path *string, names []string) int Name func(childComplexity int) int @@ -1821,13 +1821,6 @@ func (e *executableSchema) Complexity(ctx context.Context, typeName, field strin return e.complexity.GitRef.Hash(childComplexity), true - case "GitRef.isDefault": - if e.complexity.GitRef.IsDefault == nil { - break - } - - return e.complexity.GitRef.IsDefault(childComplexity), true - case "GitRef.name": if e.complexity.GitRef.Name == nil { break @@ -2354,6 +2347,13 @@ func (e *executableSchema) Complexity(ctx context.Context, typeName, field strin return e.complexity.Repository.Commits(childComplexity, args["after"].(*string), args["first"].(*int), args["ref"].(string), args["path"].(*string), args["since"].(*time.Time), args["until"].(*time.Time)), true + case "Repository.head": + if e.complexity.Repository.Head == nil { + break + } + + return e.complexity.Repository.Head(childComplexity), true + case "Repository.identity": if e.complexity.Repository.Identity == nil { break @@ -3175,8 +3175,6 @@ type GitRef { type: GitRefType! """Commit hash the reference points to.""" hash: String! - """True for the branch HEAD currently points to.""" - isDefault: Boolean! } """An entry in a git tree (directory listing).""" @@ -3564,6 +3562,10 @@ type OperationEdge { tree listing without blocking the initial tree fetch.""" lastCommits(ref: String!, path: String, names: [String!]!): [GitLastCommit!]! + """The currently checked-out commit (branch, tag, hash ...) in the git repository. + Null if there is none (bare repo).""" + head: GitCommit + """List of valid labels.""" validLabels( """Returns the elements in the list that come after the specified cursor.""" diff --git a/api/graphql/models/gen_models.go b/api/graphql/models/gen_models.go index 33b9b4417e326399d36d647478421e4465561982..51e6452862411cc4ef8658e683f5768783f56595 100644 --- a/api/graphql/models/gen_models.go +++ b/api/graphql/models/gen_models.go @@ -320,8 +320,6 @@ type GitRef struct { Type GitRefType `json:"type"` // Commit hash the reference points to. Hash string `json:"hash"` - // True for the branch HEAD currently points to. - IsDefault bool `json:"isDefault"` } type GitRefConnection struct { diff --git a/api/graphql/resolvers/repo.go b/api/graphql/resolvers/repo.go index 63104480f4a9e3dab8a742babaf87e292dbab132..bc19cf3572d0616a4281dbbfa2c8ef0178c9f31b 100644 --- a/api/graphql/resolvers/repo.go +++ b/api/graphql/resolvers/repo.go @@ -222,7 +222,6 @@ func (repoResolver) Refs(_ context.Context, obj *models.Repository, after *strin ShortName: b.Name, Type: models.GitRefTypeBranch, Hash: string(b.Hash), - IsDefault: b.IsDefault, }) } } @@ -422,3 +421,14 @@ func (repoResolver) LastCommits(_ context.Context, obj *models.Repository, ref s } return result, nil } + +func (repoResolver) Head(_ context.Context, obj *models.Repository) (*models.GitCommitMeta, error) { + meta, err := obj.Repo.BrowseRepo().Head() + if errors.Is(err, repository.ErrNotFound) { + return nil, nil + } + if err != nil { + return nil, err + } + return &models.GitCommitMeta{Repo: obj.Repo, CommitMeta: meta}, nil +} diff --git a/api/graphql/schema/git.graphql b/api/graphql/schema/git.graphql index 3e28db7474acfe15d9d11986b8bac1b85dbea3d6..abae2a7d07ad34936cbbd8092976ad46f06b3867 100644 --- a/api/graphql/schema/git.graphql +++ b/api/graphql/schema/git.graphql @@ -8,8 +8,6 @@ type GitRef { type: GitRefType! """Commit hash the reference points to.""" hash: String! - """True for the branch HEAD currently points to.""" - isDefault: Boolean! } """An entry in a git tree (directory listing).""" diff --git a/api/graphql/schema/repository.graphql b/api/graphql/schema/repository.graphql index 3705d7e715fcf2fb6724f99af8edbb129c8d02d5..3e986a560f317e546e8a8f14e60fd36d50b029d1 100644 --- a/api/graphql/schema/repository.graphql +++ b/api/graphql/schema/repository.graphql @@ -84,6 +84,11 @@ type Repository { tree listing without blocking the initial tree fetch.""" lastCommits(ref: String!, path: String, names: [String!]!): [GitLastCommit!]! + """The commit pointed to by HEAD in the git repository. + Null if HEAD cannot be resolved to a commit, for example in an empty or unborn + repository, or if HEAD is missing or invalid.""" + head: GitCommit + """List of valid labels.""" validLabels( """Returns the elements in the list that come after the specified cursor.""" diff --git a/repository/browse.go b/repository/browse.go index 4fa994c386b0eaa31e7dfddb849c14fa5a6be59d..f0159b5f7d1a5e0d7a6971ac4b98cc0bebecaaa3 100644 --- a/repository/browse.go +++ b/repository/browse.go @@ -146,9 +146,8 @@ type FileDiff struct { // BranchInfo describes a local branch returned by RepoBrowse.Branches. type BranchInfo struct { - Name string - Hash Hash // commit hash - IsDefault bool // true for the branch HEAD points to + Name string + Hash Hash // commit hash } // TagInfo describes a tag returned by RepoBrowse.Tags. diff --git a/repository/gogit.go b/repository/gogit.go index 797837b1491eb072422cef60e8a2c05c05fe2c28..1403eb8c196fddfe4be7e6aa979481246bb91a45 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -999,39 +999,8 @@ func (repo *GoGitRepo) resolveRefToHash(ref string) (plumbing.Hash, error) { return plumbing.ZeroHash, ErrNotFound } -// defaultBranchName returns the short name of the default branch. -func (repo *GoGitRepo) defaultBranchName() string { - repo.rMutex.Lock() - defer repo.rMutex.Unlock() - - // refs/remotes/origin/HEAD is a symbolic ref set by git clone that points - // to the remote's default branch (e.g. refs/remotes/origin/main). It is - // the most reliable signal for "what does the upstream consider default". - ref, err := repo.r.Reference("refs/remotes/origin/HEAD", false) - if err == nil && ref.Type() == plumbing.SymbolicReference { - const prefix = "refs/remotes/origin/" - if target := ref.Target().String(); strings.HasPrefix(target, prefix) { - return strings.TrimPrefix(target, prefix) - } - } - // Fall back to well-known names for repos without a configured remote. - for _, name := range []string{"main", "master", "trunk", "develop"} { - _, err := repo.r.Reference(plumbing.NewBranchReferenceName(name), false) - if err == nil { - return name - } - } - return "" -} - -// Branches returns all local branches. IsDefault marks the upstream's default -// branch, determined in order: -// 1. refs/remotes/origin/HEAD (set by git clone, reflects the server default) -// 2. First match among: main, master, trunk, develop -// 3. No branch marked if none of the above resolve +// Branches returns all local branches (refs/heads/*). func (repo *GoGitRepo) Branches() ([]BranchInfo, error) { - defaultBranch := repo.defaultBranchName() - repo.rMutex.Lock() defer repo.rMutex.Unlock() @@ -1046,9 +1015,8 @@ func (repo *GoGitRepo) Branches() ([]BranchInfo, error) { return nil } branches = append(branches, BranchInfo{ - Name: r.Name().Short(), - Hash: Hash(r.Hash().String()), - IsDefault: r.Name().Short() == defaultBranch, + Name: r.Name().Short(), + Hash: Hash(r.Hash().String()), }) return nil }) @@ -1582,6 +1550,30 @@ func (repo *GoGitRepo) CommitFileDiff(hash Hash, filePath string) (FileDiff, err return FileDiff{}, ErrNotFound } +// Head returns the commit that HEAD currently points to. +func (repo *GoGitRepo) Head() (CommitMeta, error) { + repo.rMutex.Lock() + defer repo.rMutex.Unlock() + + ref, err := repo.r.Head() + if err == plumbing.ErrReferenceNotFound { + return CommitMeta{}, ErrNotFound + } + if err != nil { + return CommitMeta{}, err + } + + c, err := repo.r.CommitObject(ref.Hash()) + if err == plumbing.ErrObjectNotFound { + return CommitMeta{}, ErrNotFound + } + if err != nil { + return CommitMeta{}, err + } + + return commitToMeta(c), nil +} + // buildDiffHunks converts a go-git FilePatch into DiffHunks with line numbers // and context grouping. func buildDiffHunks(fp fdiff.FilePatch) []DiffHunk { diff --git a/repository/mock_repo.go b/repository/mock_repo.go index 01d48f52b2fadceee976d6e8fcf0ce0270ec7ca9..e13e18cfdfc2b314885c4513c60f5900fd3cc8dd 100644 --- a/repository/mock_repo.go +++ b/repository/mock_repo.go @@ -232,20 +232,18 @@ type commit struct { } type mockRepoDataBrowse struct { - blobs map[Hash][]byte - trees map[Hash]string - commits map[Hash]commit - refs map[string]Hash - defaultBranch string + blobs map[Hash][]byte + trees map[Hash]string + commits map[Hash]commit + refs map[string]Hash } func newMockRepoDataBrowse() *mockRepoDataBrowse { return &mockRepoDataBrowse{ - blobs: make(map[Hash][]byte), - trees: make(map[Hash]string), - commits: make(map[Hash]commit), - refs: make(map[string]Hash), - defaultBranch: "main", + blobs: make(map[Hash][]byte), + trees: make(map[Hash]string), + commits: make(map[Hash]commit), + refs: make(map[string]Hash), } } @@ -545,9 +543,8 @@ func (r *mockRepoDataBrowse) Branches() ([]BranchInfo, error) { continue } branches = append(branches, BranchInfo{ - Name: name, - Hash: hash, - IsDefault: name == r.defaultBranch, + Name: name, + Hash: hash, }) } return branches, nil @@ -797,6 +794,18 @@ func (r *mockRepoDataBrowse) CommitFileDiff(hash Hash, filePath string) (FileDif return fd, nil } +func (r *mockRepoDataBrowse) Head() (CommitMeta, error) { + hash, ok := r.refs["HEAD"] + if !ok { + return CommitMeta{}, ErrNotFound + } + c, ok := r.commits[hash] + if !ok { + return CommitMeta{}, ErrNotFound + } + return mockCommitMeta(hash, c), nil +} + // mockDiffHunks produces a single DiffHunk using a prefix/suffix scan. func mockDiffHunks(old, new []byte) []DiffHunk { oldLines := splitBlobLines(old) diff --git a/repository/repo.go b/repository/repo.go index 72bcfe3fea388dfe2a963deb33a730d359710f3e..e081038a0d285304ba0c095e6c9a201aa6abdf6c 100644 --- a/repository/repo.go +++ b/repository/repo.go @@ -219,7 +219,6 @@ type RepoClock interface { // refs/heads/, refs/tags/, full ref name, raw commit hash. type RepoBrowse interface { // Branches returns all local branches (refs/heads/*). - // IsDefault marks the branch HEAD points to. // All other ref namespaces — including git-bug's internal refs // (refs/bugs/, refs/identities/, …) — are excluded. Branches() ([]BranchInfo, error) @@ -262,6 +261,11 @@ type RepoBrowse interface { // identified by its hash. Diffs against the first parent only; the // initial commit is diffed against the empty tree. CommitFileDiff(hash Hash, filePath string) (FileDiff, error) + + // Head returns the commit that HEAD currently points to. + // Returns ErrNotFound if HEAD cannot be resolved to a commit, including + // for an empty (unborn) repository. + Head() (CommitMeta, error) } // ClockLoader hold which logical clock need to exist for an entity and diff --git a/repository/repo_testing.go b/repository/repo_testing.go index 2ba28a1856759f557047d833c95c09fc0cad7706..53780c145e291c3ec0af1b8ecdc72e4cae24b008 100644 --- a/repository/repo_testing.go +++ b/repository/repo_testing.go @@ -466,10 +466,7 @@ func RepoBrowseTest(t *testing.T, repo browsable) { } require.Equal(t, c3, byName["main"].Hash) - require.True(t, byName["main"].IsDefault) - require.Equal(t, c2, byName["feature"].Hash) - require.False(t, byName["feature"].IsDefault) }) // ── Tags ────────────────────────────────────────────────────────────────── @@ -771,4 +768,14 @@ func RepoBrowseTest(t *testing.T, repo browsable) { _, err = repo.CommitFileDiff(randomHash(), "main.go") require.ErrorIs(t, err, ErrNotFound) }) + + // ── Head ────────────────────────────────────────────────────────────────── + + t.Run("Head", func(t *testing.T) { + require.NoError(t, repo.UpdateRef("HEAD", c3)) + + meta, err := repo.Head() + require.NoError(t, err) + require.Equal(t, c3, meta.Hash) + }) } From c4375a319da4f58c5d38c90449ce8d6112f0015a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Wed, 8 Apr 2026 21:20:27 +0200 Subject: [PATCH 4/4] api/http: add support for ref+path to get a blob content (#1550) --- api/http/git_file_handler.go | 104 ++++++++++++--- api/http/git_file_handlers_test.go | 199 +++++++++++++++++++++++------ commands/webui.go | 6 +- 3 files changed, 247 insertions(+), 62 deletions(-) diff --git a/api/http/git_file_handler.go b/api/http/git_file_handler.go index a8aeec78eda328f9adb449d9df92bab7a1f931ff..60c175c95cfd94cd6172b3000d4a069e1b3fd69e 100644 --- a/api/http/git_file_handler.go +++ b/api/http/git_file_handler.go @@ -5,6 +5,8 @@ import ( "errors" "io" "net/http" + "strconv" + "strings" "github.com/gorilla/mux" @@ -12,11 +14,17 @@ import ( "github.com/git-bug/git-bug/repository" ) -// implement a http.Handler that will read and server git blob. +// gitFileHandler implements an http.Handler that reads and serves a git blob. +// +// The route is /gitfile/{repo}/{rest:.+} where rest is resolved as follows: +// - If rest contains no slash and is a valid git hash: serve the blob by hash. +// - Otherwise: try each slash as a split point between ref and path, longest +// ref first (right to left). This handles refs with slashes (e.g. +// "feature/foo") and matches the ref that is most specific. // // Expected gorilla/mux parameters: -// - "repo" : the ref of the repo or "" for the default one -// - "hash" : the git hash of the file to retrieve +// - "repo": the repo ref or "" for the default one +// - "rest": the hash, or the ref+path combined type gitFileHandler struct { mrc *cache.MultiRepoCache } @@ -26,47 +34,101 @@ func NewGitFileHandler(mrc *cache.MultiRepoCache) http.Handler { } func (gfh *gitFileHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) { + vars := mux.Vars(r) + var repo *cache.RepoCache var err error - repoVar := mux.Vars(r)["repo"] - switch repoVar { + switch repoVar := vars["repo"]; repoVar { case "": repo, err = gfh.mrc.DefaultRepo() default: repo, err = gfh.mrc.ResolveRepo(repoVar) } - if err != nil { http.Error(rw, "invalid repo reference", http.StatusBadRequest) return } - hash := repository.Hash(mux.Vars(r)["hash"]) - if !hash.IsValid() { - http.Error(rw, "invalid git hash", http.StatusBadRequest) + rest := vars["rest"] + if rest == "" { + http.Error(rw, "missing path", http.StatusBadRequest) return } - reader, err := repo.ReadData(hash) - if errors.Is(err, repository.ErrNotFound) { - http.Error(rw, "blob not found", http.StatusNotFound) + // If rest is a single segment that is a valid git hash, serve by hash. + if !strings.Contains(rest, "/") { + if hash := repository.Hash(rest); hash.IsValid() { + reader, err := repo.ReadData(hash) + if errors.Is(err, repository.ErrNotFound) { + http.Error(rw, "not found", http.StatusNotFound) + return + } + if err != nil { + http.Error(rw, "internal server error", http.StatusInternalServerError) + return + } + defer reader.Close() + serveContent(rw, r, reader, -1, hash) + return + } + // Single segment that is not a hash: malformed request. + http.Error(rw, "expected or /", http.StatusBadRequest) return } - if err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) + + // Greedy ref+path resolution: try split points longest ref first (right to + // left) so that refs with slashes (e.g. "feature/foo") take precedence over + // shorter prefixes. + segments := strings.Split(rest, "/") + for i := len(segments) - 1; i >= 1; i-- { + ref := strings.Join(segments[:i], "/") + path := strings.Join(segments[i:], "/") + rc, size, hash, err := repo.BrowseRepo().BlobAtPath(ref, path) + if errors.Is(err, repository.ErrNotFound) { + continue + } + if err != nil { + http.Error(rw, "internal server error", http.StatusInternalServerError) + return + } + defer rc.Close() + serveContent(rw, r, rc, size, hash) return } - defer func() { - _ = reader.Close() - }() - serveContent(rw, r, reader) + http.Error(rw, "not found", http.StatusNotFound) +} + +// ifNoneMatchContains reports whether the If-None-Match header value matches +// etag per RFC 9110 weak comparison: handles "*", comma-separated lists, and +// weak validators (W/"..."). +func ifNoneMatchContains(header, etag string) bool { + if header == "*" { + return true + } + for _, token := range strings.Split(header, ",") { + token = strings.TrimSpace(token) + token = strings.TrimPrefix(token, "W/") + if token == etag { + return true + } + } + return false } // serveContent is a somewhat equivalent of http.serveContent, without support for range request. // This is necessary as the repo (and go-git)'s data reader doesn't support Seek(). -func serveContent(w http.ResponseWriter, r *http.Request, content io.Reader) { +func serveContent(w http.ResponseWriter, r *http.Request, content io.Reader, size int64, hash repository.Hash) { + if hash.IsValid() { + etag := `"` + string(hash) + `"` + w.Header().Set("ETag", etag) + if ifNoneMatchContains(r.Header.Get("If-None-Match"), etag) { + w.WriteHeader(http.StatusNotModified) + return + } + } + if w.Header().Get("Content-Type") == "" { // Sniff the type from the first up to 512 bytes. var buf [512]byte @@ -84,6 +146,10 @@ func serveContent(w http.ResponseWriter, r *http.Request, content io.Reader) { } } + if size >= 0 { + w.Header().Set("Content-Length", strconv.FormatInt(size, 10)) + } + w.WriteHeader(http.StatusOK) if r.Method == http.MethodHead { return diff --git a/api/http/git_file_handlers_test.go b/api/http/git_file_handlers_test.go index d7a215322b8d06eb9dd952d67c0eae8d83c5e3e8..50cc7b5ddb66a36d3a05f0000152c5202a09b5f1 100644 --- a/api/http/git_file_handlers_test.go +++ b/api/http/git_file_handlers_test.go @@ -2,6 +2,7 @@ package http import ( "bytes" + "context" "image" "image/png" "mime/multipart" @@ -29,66 +30,186 @@ func TestGitFileHandlers(t *testing.T) { author, err := repoCache.Identities().New("test identity", "test@test.org") require.NoError(t, err) - err = repoCache.SetUserIdentity(author) require.NoError(t, err) - // UPLOAD - - uploadHandler := NewGitUploadFileHandler(mrc) - + // Build a PNG image to use as test content. img := image.NewNRGBA(image.Rect(0, 0, 50, 50)) data := &bytes.Buffer{} err = png.Encode(data, img) require.NoError(t, err) + imgBytes := data.Bytes() + + // ── Upload ──────────────────────────────────────────────────────────────── + + t.Run("Upload", func(t *testing.T) { + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + part, err := writer.CreateFormFile("uploadfile", "noname") + require.NoError(t, err) + _, err = part.Write(imgBytes) + require.NoError(t, err) + require.NoError(t, writer.Close()) + + w := httptest.NewRecorder() + r, _ := http.NewRequest("POST", "/", body) + r.Header.Add("Content-Type", writer.FormDataContentType()) + r = r.WithContext(auth.CtxWithUser(r.Context(), author.Id())) + r = mux.SetURLVars(r, map[string]string{"repo": ""}) + + NewGitUploadFileHandler(mrc).ServeHTTP(w, r) + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, `{"hash":"3426a1488292d8f3f3c59ca679681336542b986f"}`, w.Body.String()) + }) + + // ── Download by hash ────────────────────────────────────────────────────── + + t.Run("DownloadByHash", func(t *testing.T) { + w := httptest.NewRecorder() + r, _ := http.NewRequest("GET", "/", nil) + r = r.WithContext(auth.CtxWithUser(r.Context(), author.Id())) + r = mux.SetURLVars(r, map[string]string{ + "repo": "", + "rest": "3426a1488292d8f3f3c59ca679681336542b986f", + }) + + NewGitFileHandler(mrc).ServeHTTP(w, r) + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "image/png", w.Header().Get("Content-Type")) + assert.Equal(t, imgBytes, w.Body.Bytes()) + }) + + // Set up commits to test ref+path resolution. + // + // Ambiguity test: git's filesystem prevents refs/heads/feature and + // refs/heads/feature/foo from coexisting (file vs directory). Instead, use + // a branch "feature" and a tag "feature/foo" — different namespaces, no + // conflict. resolveRefToHash checks both heads and tags, so the tag is a + // valid ref. With longest-ref-first resolution, "feature/foo/image.png" + // must resolve via the tag (imgBytes), not via the branch (which has no + // foo/image.png and would 404). + imgHash, err := repo.StoreData(imgBytes) + require.NoError(t, err) - body := &bytes.Buffer{} - writer := multipart.NewWriter(body) - part, err := writer.CreateFormFile("uploadfile", "noname") - assert.NoError(t, err) + otherBytes := []byte("other content") + otherHash, err := repo.StoreData(otherBytes) + require.NoError(t, err) - _, err = part.Write(data.Bytes()) - assert.NoError(t, err) + imgTreeHash, err := repo.StoreTree([]repository.TreeEntry{ + {ObjectType: repository.Blob, Hash: imgHash, Name: "image.png"}, + }) + require.NoError(t, err) - err = writer.Close() - assert.NoError(t, err) + otherTreeHash, err := repo.StoreTree([]repository.TreeEntry{ + {ObjectType: repository.Blob, Hash: otherHash, Name: "image.png"}, + }) + require.NoError(t, err) - w := httptest.NewRecorder() - r, _ := http.NewRequest("GET", "/", body) - r.Header.Add("Content-Type", writer.FormDataContentType()) + mainCommit, err := repo.StoreCommit(imgTreeHash) + require.NoError(t, err) + featureCommit, err := repo.StoreCommit(otherTreeHash) + require.NoError(t, err) - // Simulate auth - r = r.WithContext(auth.CtxWithUser(r.Context(), author.Id())) + require.NoError(t, repo.UpdateRef("refs/heads/main", mainCommit)) + // "feature" branch has otherBytes; "feature/foo" tag has imgBytes. + require.NoError(t, repo.UpdateRef("refs/heads/feature", featureCommit)) + require.NoError(t, repo.UpdateRef("refs/tags/feature/foo", mainCommit)) + + handler := NewGitFileHandler(mrc) + authCtx := auth.CtxWithUser(context.Background(), author.Id()) + + serve := func(rest string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + r, _ := http.NewRequest("GET", "/", nil) + r = r.WithContext(authCtx) + r = mux.SetURLVars(r, map[string]string{"repo": "", "rest": rest}) + handler.ServeHTTP(w, r) + return w + } - // Handler's params - r = mux.SetURLVars(r, map[string]string{ - "repo": "", + serveWithHeader := func(rest, headerName, headerVal string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + r, _ := http.NewRequest("GET", "/", nil) + r = r.WithContext(authCtx) + r.Header.Set(headerName, headerVal) + r = mux.SetURLVars(r, map[string]string{"repo": "", "rest": rest}) + handler.ServeHTTP(w, r) + return w + } + + // ── Download by ref+path (simple ref) ───────────────────────────────────── + + t.Run("DownloadByRefPath", func(t *testing.T) { + w := serve("main/image.png") + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "image/png", w.Header().Get("Content-Type")) + assert.Equal(t, imgBytes, w.Body.Bytes()) }) - uploadHandler.ServeHTTP(w, r) + // ── Download by ref+path (ref with slash) ───────────────────────────────── + + t.Run("DownloadByRefWithSlash", func(t *testing.T) { + // "feature/foo" is a tag; verify multi-segment refs resolve correctly. + w := serve("feature/foo/image.png") + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "image/png", w.Header().Get("Content-Type")) + assert.Equal(t, imgBytes, w.Body.Bytes()) + }) - assert.Equal(t, http.StatusOK, w.Code) - assert.Equal(t, `{"hash":"3426a1488292d8f3f3c59ca679681336542b986f"}`, w.Body.String()) - // DOWNLOAD + // ── Ambiguous ref: longest ref wins ─────────────────────────────────────── + // Both "feature" (branch, otherBytes) and "feature/foo" (tag, imgBytes) + // exist. "feature/foo/image.png" must resolve via the longer ref + // "feature/foo" → imgBytes, not via "feature" → foo/image.png (404). - downloadHandler := NewGitFileHandler(mrc) + t.Run("AmbiguousRefLongestWins", func(t *testing.T) { + w := serve("feature/foo/image.png") + assert.Equal(t, http.StatusOK, w.Code) + // Must be imgBytes (from tag feature/foo), not otherBytes (from branch feature). + assert.Equal(t, imgBytes, w.Body.Bytes()) + }) - w = httptest.NewRecorder() - r, _ = http.NewRequest("GET", "/", nil) + // ── Conditional GET: 304 with matching ETag ─────────────────────────────── - // Simulate auth - r = r.WithContext(auth.CtxWithUser(r.Context(), author.Id())) + t.Run("ConditionalGet304", func(t *testing.T) { + // First request to get the ETag. + w := serve("main/image.png") + require.Equal(t, http.StatusOK, w.Code) + etag := w.Header().Get("ETag") + require.NotEmpty(t, etag) - // Handler's params - r = mux.SetURLVars(r, map[string]string{ - "repo": "", - "hash": "3426a1488292d8f3f3c59ca679681336542b986f", + // Second request with If-None-Match should get 304. + w = serveWithHeader("main/image.png", "If-None-Match", etag) + assert.Equal(t, http.StatusNotModified, w.Code) + assert.Equal(t, etag, w.Header().Get("ETag")) + assert.Empty(t, w.Body.Bytes()) }) - downloadHandler.ServeHTTP(w, r) - assert.Equal(t, http.StatusOK, w.Code) - // check if content type detection is working - assert.Equal(t, "image/png", w.Header().Get("Content-Type")) + t.Run("ConditionalGetWeakETag", func(t *testing.T) { + w := serve("main/image.png") + require.Equal(t, http.StatusOK, w.Code) + etag := w.Header().Get("ETag") + + // Weak form of the same ETag should also match. + w = serveWithHeader("main/image.png", "If-None-Match", "W/"+etag) + assert.Equal(t, http.StatusNotModified, w.Code) + }) - assert.Equal(t, data.Bytes(), w.Body.Bytes()) + t.Run("ConditionalGetWildcard", func(t *testing.T) { + w := serveWithHeader("main/image.png", "If-None-Match", "*") + assert.Equal(t, http.StatusNotModified, w.Code) + }) + + // ── Not found ───────────────────────────────────────────────────────────── + + t.Run("NotFound", func(t *testing.T) { + w := serve("main/nonexistent.png") + assert.Equal(t, http.StatusNotFound, w.Code) + }) + + // ── Malformed: single segment that is not a hash ─────────────────────────── + + t.Run("MalformedSingleSegment", func(t *testing.T) { + w := serve("main") + assert.Equal(t, http.StatusBadRequest, w.Code) + }) } diff --git a/commands/webui.go b/commands/webui.go index 7f8db1b00e9660abf90279adf0d9942b5e1ccae7..55dc002380b768841d6314c1e4a6cfdc883ea536 100644 --- a/commands/webui.go +++ b/commands/webui.go @@ -96,11 +96,9 @@ func setupRoutes(env *execenv.Env, opts webUIOptions) (*mux.Router, func() error errOut = env.Err } - graphqlHandler := graphql.NewHandler(mrc, errOut) - router.Path("/playground").Handler(playground.Handler("git-bug", "/graphql")) - router.Path("/graphql").Handler(graphqlHandler) - router.Path("/gitfile/{repo}/{hash}").Handler(httpapi.NewGitFileHandler(mrc)) + router.Path("/graphql").Handler(graphql.NewHandler(mrc, errOut)) + router.Path("/gitfile/{repo}/{rest:.+}").Handler(httpapi.NewGitFileHandler(mrc)) router.Path("/upload/{repo}").Methods("POST").Handler(httpapi.NewGitUploadFileHandler(mrc)) router.PathPrefix("/").Handler(webui.NewHandler())