refactor: reuse the domain settings predicates and tighten the moved rules after review
Use IssuesEnabled, ShowsIssueSubmissionReceipt, AcceptsAnonymousIssues and EffectiveSettings instead of hand-written copies; rename CanMarkRead to HeadUnread; add RevisionCandidate.ResolvedRef; name the compare and push-summary limits; restore real-git workflow-guard scenarios and split the history tests.
35 files · +230 -129
M docs/merge-requests.md +13 -0
| @@ -91,3 +91,16 @@ A root note can carry a verdict: `--approve` or `--request-changes`. The text ma | |||
| 91 | 91 | - `mr show` lists the latest verdict of each author (`verdicts` in `--json`). | |
| 92 | 92 | - An approval is stale once the branch head differs from the head it was given on. Changes requested stay current until their author gives a new verdict, whatever is pushed. | |
| 93 | 93 | - Verdicts are immutable: editing or deleting the note keeps its verdict. Only a new verdict of the same author replaces it. | |
| 94 | + | ||
| 95 | + | ## Shared read position and history queries | |
| 96 | + | ||
| 97 | + | The read position is shared, not per principal. `MergeRequestDetail.HeadUnread` | |
| 98 | + | is a fact: the nonzero head differs from that position. The web presentation | |
| 99 | + | still decides who sees the mark-read button. | |
| 100 | + | ||
| 101 | + | Repository comparisons return at most `ports.MaxCompareCommits` (500) commits. | |
| 102 | + | Push summaries count at most `ports.MaxPushSummaryCommits` (100) commits and | |
| 103 | + | report whether more exist; ancestry, rather than the bounded list, detects a | |
| 104 | + | rewrite. Merge-request details include at most `ports.MaxMergeRequestCommits` | |
| 105 | + | (100) commits. These history queries remain on the merge-request service | |
| 106 | + | because it owns the merge-git port. | |
| @@ -91,3 +91,16 @@ A root note can carry a verdict: `--approve` or `--request-changes`. The text ma | |||||
| 91 | - `mr show` lists the latest verdict of each author (`verdicts` in `--json`). | 91 | - `mr show` lists the latest verdict of each author (`verdicts` in `--json`). | ||
| 92 | - An approval is stale once the branch head differs from the head it was given on. Changes requested stay current until their author gives a new verdict, whatever is pushed. | 92 | - An approval is stale once the branch head differs from the head it was given on. Changes requested stay current until their author gives a new verdict, whatever is pushed. | ||
| 93 | - Verdicts are immutable: editing or deleting the note keeps its verdict. Only a new verdict of the same author replaces it. | 93 | - Verdicts are immutable: editing or deleting the note keeps its verdict. Only a new verdict of the same author replaces it. | ||
| 94 | + | ||||
| 95 | + | ## Shared read position and history queries | |||
| 96 | + | ||||
| 97 | + | The read position is shared, not per principal. `MergeRequestDetail.HeadUnread` | |||
| 98 | + | is a fact: the nonzero head differs from that position. The web presentation | |||
| 99 | + | still decides who sees the mark-read button. | |||
| 100 | + | ||||
| 101 | + | Repository comparisons return at most `ports.MaxCompareCommits` (500) commits. | |||
| 102 | + | Push summaries count at most `ports.MaxPushSummaryCommits` (100) commits and | |||
| 103 | + | report whether more exist; ancestry, rather than the bounded list, detects a | |||
| 104 | + | rewrite. Merge-request details include at most `ports.MaxMergeRequestCommits` | |||
| 105 | + | (100) commits. These history queries remain on the merge-request service | |||
| 106 | + | because it owns the merge-git port. | |||
M internal/adapters/ermitegit/convert.go +1 -9
| @@ -128,20 +128,12 @@ func resolveRevision(r *git.Repo, rev domain.Revision) (resolved, error) { | |||
| 128 | 128 | return resolved{}, err | |
| 129 | 129 | } | |
| 130 | 130 | if ok { | |
| 131 | - | return resolved{commit: c, ref: candidateRef(candidate)}, nil | |
| 131 | + | return resolved{commit: c, ref: candidate.ResolvedRef()}, nil | |
| 132 | 132 | } | |
| 133 | 133 | } | |
| 134 | 134 | return resolved{}, notFound(rev) | |
| 135 | 135 | } | |
| 136 | 136 | ||
| 137 | - | // candidateRef translates a ref lookup into the resolved ref field. | |
| 138 | - | func candidateRef(candidate domain.RevisionCandidate) string { | |
| 139 | - | if candidate.Kind == domain.RevisionCandidateRef { | |
| 140 | - | return candidate.Name | |
| 141 | - | } | |
| 142 | - | return "" | |
| 143 | - | } | |
| 144 | - | ||
| 145 | 137 | // lookupRevisionCandidate translates one domain candidate to the library API. | |
| 146 | 138 | func lookupRevisionCandidate(r *git.Repo, candidate domain.RevisionCandidate) (git.ObjectID, bool, error) { | |
| 147 | 139 | switch candidate.Kind { | |
| @@ -128,20 +128,12 @@ func resolveRevision(r *git.Repo, rev domain.Revision) (resolved, error) { | |||||
| 128 | return resolved{}, err | 128 | return resolved{}, err | ||
| 129 | } | 129 | } | ||
| 130 | if ok { | 130 | if ok { | ||
| 131 | - | return resolved{commit: c, ref: candidateRef(candidate)}, nil | 131 | + | return resolved{commit: c, ref: candidate.ResolvedRef()}, nil |
| 132 | } | 132 | } | ||
| 133 | } | 133 | } | ||
| 134 | return resolved{}, notFound(rev) | 134 | return resolved{}, notFound(rev) | ||
| 135 | } | 135 | } | ||
| 136 | 136 | ||||
| 137 | - | // candidateRef translates a ref lookup into the resolved ref field. | |||
| 138 | - | func candidateRef(candidate domain.RevisionCandidate) string { | |||
| 139 | - | if candidate.Kind == domain.RevisionCandidateRef { | |||
| 140 | - | return candidate.Name | |||
| 141 | - | } | |||
| 142 | - | return "" | |||
| 143 | - | } | |||
| 144 | - | ||||
| 145 | // lookupRevisionCandidate translates one domain candidate to the library API. | 137 | // lookupRevisionCandidate translates one domain candidate to the library API. | ||
| 146 | func lookupRevisionCandidate(r *git.Repo, candidate domain.RevisionCandidate) (git.ObjectID, bool, error) { | 138 | func lookupRevisionCandidate(r *git.Repo, candidate domain.RevisionCandidate) (git.ObjectID, bool, error) { | ||
| 147 | switch candidate.Kind { | 139 | switch candidate.Kind { | ||
M internal/adapters/ermitegit/ermitegit.go +1 -1
| @@ -291,7 +291,7 @@ func (a *Adapter) ListRefs(ctx context.Context, repo domain.RepoName) ([]domain. | |||
| 291 | 291 | ||
| 292 | 292 | // ResolveRevision turns a revision into a commit. | |
| 293 | 293 | // | |
| 294 | - | //hexcheck:ignore suspicious-business-logic-in-adapter selects the configured CLI or library backend, with a separate route for abbreviated IDs; resolution order comes from domain.Revision.Candidates | |
| 294 | + | //hexcheck:ignore suspicious-business-logic-in-adapter selects the configured CLI or library backend, with a separate route for abbreviated IDs; resolveRevision calls domain.Revision.Candidates for resolution order | |
| 295 | 295 | func (a *Adapter) ResolveRevision(ctx context.Context, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) { | |
| 296 | 296 | cli := func() (domain.ResolvedRevision, error) { return a.cli.ResolveRevision(ctx, repo, rev) } | |
| 297 | 297 | route := a.routes.ResolveRevision | |
| @@ -291,7 +291,7 @@ func (a *Adapter) ListRefs(ctx context.Context, repo domain.RepoName) ([]domain. | |||||
| 291 | 291 | ||||
| 292 | // ResolveRevision turns a revision into a commit. | 292 | // ResolveRevision turns a revision into a commit. | ||
| 293 | // | 293 | // | ||
| 294 | - | //hexcheck:ignore suspicious-business-logic-in-adapter selects the configured CLI or library backend, with a separate route for abbreviated IDs; resolution order comes from domain.Revision.Candidates | 294 | + | //hexcheck:ignore suspicious-business-logic-in-adapter selects the configured CLI or library backend, with a separate route for abbreviated IDs; resolveRevision calls domain.Revision.Candidates for resolution order |
| 295 | func (a *Adapter) ResolveRevision(ctx context.Context, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) { | 295 | func (a *Adapter) ResolveRevision(ctx context.Context, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) { | ||
| 296 | cli := func() (domain.ResolvedRevision, error) { return a.cli.ResolveRevision(ctx, repo, rev) } | 296 | cli := func() (domain.ResolvedRevision, error) { return a.cli.ResolveRevision(ctx, repo, rev) } | ||
| 297 | route := a.routes.ResolveRevision | 297 | route := a.routes.ResolveRevision | ||
M internal/adapters/gitcli/push_trees.go +6 -6
| @@ -25,7 +25,7 @@ const ( | |||
| 25 | 25 | ||
| 26 | 26 | // PushSubtrees peels push endpoints, walks new commits, and reads entries at path. | |
| 27 | 27 | func (g *Git) PushSubtrees(ctx context.Context, repo domain.RepoName, updates []domain.RefUpdate, objects ports.PushObjects, path string, bounds domain.PushWalkBounds) (domain.PushSubtrees, error) { | |
| 28 | - | out := domain.PushSubtrees{Ends: map[string]domain.PushedEnd{}, Subtrees: map[string]domain.Subtree{}} | |
| 28 | + | out := domain.PushSubtrees{Ends: map[string]domain.PushObjectType{}, Subtrees: map[string]domain.Subtree{}} | |
| 29 | 29 | dir, err := g.repoDir(repo) | |
| 30 | 30 | if err != nil { | |
| 31 | 31 | return out, err | |
| @@ -79,13 +79,13 @@ func (g *Git) readPushSubtrees(ctx context.Context, dir string, extra []string, | |||
| 79 | 79 | } | |
| 80 | 80 | ||
| 81 | 81 | // pushCommitTips translates peeled endpoint answers and deduplicates rev-list tips. | |
| 82 | - | func pushCommitTips(kept []domain.RefUpdate, types []string, ends map[string]domain.PushedEnd) []string { | |
| 82 | + | func pushCommitTips(kept []domain.RefUpdate, types []string, ends map[string]domain.PushObjectType) []string { | |
| 83 | 83 | var tips []string | |
| 84 | 84 | seen := map[string]bool{} | |
| 85 | 85 | for i, up := range kept { | |
| 86 | 86 | for j, id := range []string{up.Old, up.New} { | |
| 87 | 87 | if id != domain.ZeroOID { | |
| 88 | - | ends[id] = domain.PushedEnd{Type: domain.ObjectType(types[2*i+j])} | |
| 88 | + | ends[id] = domain.PushObjectType(types[2*i+j]) | |
| 89 | 89 | } | |
| 90 | 90 | } | |
| 91 | 91 | if types[2*i+1] == "commit" && !seen[up.New] { | |
| @@ -113,18 +113,18 @@ func commitAndParentIDs(commits []domain.PushedCommit) []string { | |||
| 113 | 113 | } | |
| 114 | 114 | ||
| 115 | 115 | // pushEndSubtreeIDs omits blobs (not tree-ish) and includes HEAD only for creates. | |
| 116 | - | func pushEndSubtreeIDs(kept []domain.RefUpdate, ends map[string]domain.PushedEnd) []string { | |
| 116 | + | func pushEndSubtreeIDs(kept []domain.RefUpdate, ends map[string]domain.PushObjectType) []string { | |
| 117 | 117 | var ids []string | |
| 118 | 118 | for _, up := range kept { | |
| 119 | 119 | for _, id := range []string{up.Old, up.New} { | |
| 120 | - | if id != domain.ZeroOID && ends[id].Type != domain.ObjectBlob { | |
| 120 | + | if id != domain.ZeroOID && ends[id] != domain.PushObjectBlob { | |
| 121 | 121 | ids = append(ids, id) | |
| 122 | 122 | } | |
| 123 | 123 | } | |
| 124 | 124 | } | |
| 125 | 125 | // HEAD is needed only for a new tree-ish ref. Keep it in the end batch. | |
| 126 | 126 | for _, up := range kept { | |
| 127 | - | if up.Old == domain.ZeroOID && ends[up.New].Type != domain.ObjectBlob { | |
| 127 | + | if up.Old == domain.ZeroOID && ends[up.New] != domain.PushObjectBlob { | |
| 128 | 128 | ids = append(ids, domain.HeadKey) | |
| 129 | 129 | break | |
| 130 | 130 | } | |
| @@ -25,7 +25,7 @@ const ( | |||||
| 25 | 25 | ||||
| 26 | // PushSubtrees peels push endpoints, walks new commits, and reads entries at path. | 26 | // PushSubtrees peels push endpoints, walks new commits, and reads entries at path. | ||
| 27 | func (g *Git) PushSubtrees(ctx context.Context, repo domain.RepoName, updates []domain.RefUpdate, objects ports.PushObjects, path string, bounds domain.PushWalkBounds) (domain.PushSubtrees, error) { | 27 | func (g *Git) PushSubtrees(ctx context.Context, repo domain.RepoName, updates []domain.RefUpdate, objects ports.PushObjects, path string, bounds domain.PushWalkBounds) (domain.PushSubtrees, error) { | ||
| 28 | - | out := domain.PushSubtrees{Ends: map[string]domain.PushedEnd{}, Subtrees: map[string]domain.Subtree{}} | 28 | + | out := domain.PushSubtrees{Ends: map[string]domain.PushObjectType{}, Subtrees: map[string]domain.Subtree{}} |
| 29 | dir, err := g.repoDir(repo) | 29 | dir, err := g.repoDir(repo) | ||
| 30 | if err != nil { | 30 | if err != nil { | ||
| 31 | return out, err | 31 | return out, err | ||
| @@ -79,13 +79,13 @@ func (g *Git) readPushSubtrees(ctx context.Context, dir string, extra []string, | |||||
| 79 | } | 79 | } | ||
| 80 | 80 | ||||
| 81 | // pushCommitTips translates peeled endpoint answers and deduplicates rev-list tips. | 81 | // pushCommitTips translates peeled endpoint answers and deduplicates rev-list tips. | ||
| 82 | - | func pushCommitTips(kept []domain.RefUpdate, types []string, ends map[string]domain.PushedEnd) []string { | 82 | + | func pushCommitTips(kept []domain.RefUpdate, types []string, ends map[string]domain.PushObjectType) []string { |
| 83 | var tips []string | 83 | var tips []string | ||
| 84 | seen := map[string]bool{} | 84 | seen := map[string]bool{} | ||
| 85 | for i, up := range kept { | 85 | for i, up := range kept { | ||
| 86 | for j, id := range []string{up.Old, up.New} { | 86 | for j, id := range []string{up.Old, up.New} { | ||
| 87 | if id != domain.ZeroOID { | 87 | if id != domain.ZeroOID { | ||
| 88 | - | ends[id] = domain.PushedEnd{Type: domain.ObjectType(types[2*i+j])} | 88 | + | ends[id] = domain.PushObjectType(types[2*i+j]) |
| 89 | } | 89 | } | ||
| 90 | } | 90 | } | ||
| 91 | if types[2*i+1] == "commit" && !seen[up.New] { | 91 | if types[2*i+1] == "commit" && !seen[up.New] { | ||
| @@ -113,18 +113,18 @@ func commitAndParentIDs(commits []domain.PushedCommit) []string { | |||||
| 113 | } | 113 | } | ||
| 114 | 114 | ||||
| 115 | // pushEndSubtreeIDs omits blobs (not tree-ish) and includes HEAD only for creates. | 115 | // pushEndSubtreeIDs omits blobs (not tree-ish) and includes HEAD only for creates. | ||
| 116 | - | func pushEndSubtreeIDs(kept []domain.RefUpdate, ends map[string]domain.PushedEnd) []string { | 116 | + | func pushEndSubtreeIDs(kept []domain.RefUpdate, ends map[string]domain.PushObjectType) []string { |
| 117 | var ids []string | 117 | var ids []string | ||
| 118 | for _, up := range kept { | 118 | for _, up := range kept { | ||
| 119 | for _, id := range []string{up.Old, up.New} { | 119 | for _, id := range []string{up.Old, up.New} { | ||
| 120 | - | if id != domain.ZeroOID && ends[id].Type != domain.ObjectBlob { | 120 | + | if id != domain.ZeroOID && ends[id] != domain.PushObjectBlob { |
| 121 | ids = append(ids, id) | 121 | ids = append(ids, id) | ||
| 122 | } | 122 | } | ||
| 123 | } | 123 | } | ||
| 124 | } | 124 | } | ||
| 125 | // HEAD is needed only for a new tree-ish ref. Keep it in the end batch. | 125 | // HEAD is needed only for a new tree-ish ref. Keep it in the end batch. | ||
| 126 | for _, up := range kept { | 126 | for _, up := range kept { | ||
| 127 | - | if up.Old == domain.ZeroOID && ends[up.New].Type != domain.ObjectBlob { | 127 | + | if up.Old == domain.ZeroOID && ends[up.New] != domain.PushObjectBlob { |
| 128 | ids = append(ids, domain.HeadKey) | 128 | ids = append(ids, domain.HeadKey) | ||
| 129 | break | 129 | break | ||
| 130 | } | 130 | } | ||
M internal/adapters/gitcli/push_trees_test.go +31 -4
| @@ -242,12 +242,12 @@ func TestPushSubtreesPeelsAndClassifiesBothEnds(t *testing.T) { | |||
| 242 | 242 | tree := f.Git("rev-parse", plain+"^{tree}") | |
| 243 | 243 | for _, r := range []struct { | |
| 244 | 244 | id string | |
| 245 | - | kind domain.ObjectType | |
| 246 | - | }{{plain, domain.ObjectCommit}, {tag(plain, "commit"), domain.ObjectCommit}, {blob, domain.ObjectBlob}, {tag(blob, "blob"), domain.ObjectBlob}, {tree, domain.ObjectTree}, {tag(tag(plain, "commit"), "tag"), domain.ObjectCommit}} { | |
| 245 | + | kind domain.PushObjectType | |
| 246 | + | }{{plain, domain.PushObjectCommit}, {tag(plain, "commit"), domain.PushObjectCommit}, {blob, domain.PushObjectBlob}, {tag(blob, "blob"), domain.PushObjectBlob}, {tree, domain.PushObjectTree}, {tag(tag(plain, "commit"), "tag"), domain.PushObjectCommit}} { | |
| 247 | 247 | got, err := g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: r.id}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds()) | |
| 248 | 248 | require.NoError(t, err) | |
| 249 | - | assert.Equal(t, r.kind, got.Ends[r.id].Type) | |
| 250 | - | if r.kind == domain.ObjectBlob { | |
| 249 | + | assert.Equal(t, r.kind, got.Ends[r.id]) | |
| 250 | + | if r.kind == domain.PushObjectBlob { | |
| 251 | 251 | assert.NotContains(t, got.Subtrees, r.id) | |
| 252 | 252 | } | |
| 253 | 253 | } | |
| @@ -504,3 +504,30 @@ func TestFirstDifferentEntry(t *testing.T) { | |||
| 504 | 504 | _, err = g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{Dir: t.TempDir()}, domain.ObjectID{}, tree) | |
| 505 | 505 | assert.ErrorIs(t, err, domain.ErrInvalid) | |
| 506 | 506 | } | |
| 507 | + | ||
| 508 | + | func TestChangedWorkflowPushHistories(t *testing.T) { | |
| 509 | + | g, data := newGit(t) | |
| 510 | + | ctx := context.Background() | |
| 511 | + | h := newGuardHistory(t, g, data) | |
| 512 | + | t.Cleanup(func() { _ = g.Close(ctx) }) | |
| 513 | + | run := func(old, tip, want string) { | |
| 514 | + | t.Helper() | |
| 515 | + | got, err := changedWorkflow(g, ctx, h.f.Name, []domain.RefUpdate{{Old: old, New: tip, Ref: "refs/heads/pushed"}}, ports.PushObjects{}) | |
| 516 | + | require.NoError(t, err) | |
| 517 | + | assert.Equal(t, want, got) | |
| 518 | + | } | |
| 519 | + | run(domain.ZeroOID, h.commit("A", "", h.m), "") | |
| 520 | + | change := h.commit("C", "", h.m) | |
| 521 | + | revert := h.commit("A", "", change) | |
| 522 | + | run(h.m, revert, ".github/workflows/ci.yml") | |
| 523 | + | run(domain.ZeroOID, h.commit("D", "", h.m, h.o), ".github/workflows/ci.yml") | |
| 524 | + | h.f.Git("symbolic-ref", "HEAD", "refs/heads/nothing") | |
| 525 | + | run(domain.ZeroOID, h.m, ".github/workflows/ci.yml") | |
| 526 | + | ups := []domain.RefUpdate{{Old: domain.ZeroOID, New: h.m, Ref: "refs/tags/t"}} | |
| 527 | + | trees, err := g.PushSubtrees(ctx, h.f.Name, ups, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds()) | |
| 528 | + | require.NoError(t, err) | |
| 529 | + | assert.NotContains(t, trees.Subtrees, domain.HeadKey) | |
| 530 | + | plain := h.commit("", "") | |
| 531 | + | h.f.Git("update-ref", "refs/heads/plain", plain) | |
| 532 | + | run(domain.ZeroOID, plain, "") | |
| 533 | + | } | |
| @@ -242,12 +242,12 @@ func TestPushSubtreesPeelsAndClassifiesBothEnds(t *testing.T) { | |||||
| 242 | tree := f.Git("rev-parse", plain+"^{tree}") | 242 | tree := f.Git("rev-parse", plain+"^{tree}") | ||
| 243 | for _, r := range []struct { | 243 | for _, r := range []struct { | ||
| 244 | id string | 244 | id string | ||
| 245 | - | kind domain.ObjectType | 245 | + | kind domain.PushObjectType |
| 246 | - | }{{plain, domain.ObjectCommit}, {tag(plain, "commit"), domain.ObjectCommit}, {blob, domain.ObjectBlob}, {tag(blob, "blob"), domain.ObjectBlob}, {tree, domain.ObjectTree}, {tag(tag(plain, "commit"), "tag"), domain.ObjectCommit}} { | 246 | + | }{{plain, domain.PushObjectCommit}, {tag(plain, "commit"), domain.PushObjectCommit}, {blob, domain.PushObjectBlob}, {tag(blob, "blob"), domain.PushObjectBlob}, {tree, domain.PushObjectTree}, {tag(tag(plain, "commit"), "tag"), domain.PushObjectCommit}} { |
| 247 | got, err := g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: r.id}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds()) | 247 | got, err := g.PushSubtrees(ctx, f.Name, []domain.RefUpdate{{Old: domain.ZeroOID, New: r.id}}, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds()) | ||
| 248 | require.NoError(t, err) | 248 | require.NoError(t, err) | ||
| 249 | - | assert.Equal(t, r.kind, got.Ends[r.id].Type) | 249 | + | assert.Equal(t, r.kind, got.Ends[r.id]) |
| 250 | - | if r.kind == domain.ObjectBlob { | 250 | + | if r.kind == domain.PushObjectBlob { |
| 251 | assert.NotContains(t, got.Subtrees, r.id) | 251 | assert.NotContains(t, got.Subtrees, r.id) | ||
| 252 | } | 252 | } | ||
| 253 | } | 253 | } | ||
| @@ -504,3 +504,30 @@ func TestFirstDifferentEntry(t *testing.T) { | |||||
| 504 | _, err = g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{Dir: t.TempDir()}, domain.ObjectID{}, tree) | 504 | _, err = g.FirstDifferentEntry(ctx, f.Name, ports.PushObjects{Dir: t.TempDir()}, domain.ObjectID{}, tree) | ||
| 505 | assert.ErrorIs(t, err, domain.ErrInvalid) | 505 | assert.ErrorIs(t, err, domain.ErrInvalid) | ||
| 506 | } | 506 | } | ||
| 507 | + | ||||
| 508 | + | func TestChangedWorkflowPushHistories(t *testing.T) { | |||
| 509 | + | g, data := newGit(t) | |||
| 510 | + | ctx := context.Background() | |||
| 511 | + | h := newGuardHistory(t, g, data) | |||
| 512 | + | t.Cleanup(func() { _ = g.Close(ctx) }) | |||
| 513 | + | run := func(old, tip, want string) { | |||
| 514 | + | t.Helper() | |||
| 515 | + | got, err := changedWorkflow(g, ctx, h.f.Name, []domain.RefUpdate{{Old: old, New: tip, Ref: "refs/heads/pushed"}}, ports.PushObjects{}) | |||
| 516 | + | require.NoError(t, err) | |||
| 517 | + | assert.Equal(t, want, got) | |||
| 518 | + | } | |||
| 519 | + | run(domain.ZeroOID, h.commit("A", "", h.m), "") | |||
| 520 | + | change := h.commit("C", "", h.m) | |||
| 521 | + | revert := h.commit("A", "", change) | |||
| 522 | + | run(h.m, revert, ".github/workflows/ci.yml") | |||
| 523 | + | run(domain.ZeroOID, h.commit("D", "", h.m, h.o), ".github/workflows/ci.yml") | |||
| 524 | + | h.f.Git("symbolic-ref", "HEAD", "refs/heads/nothing") | |||
| 525 | + | run(domain.ZeroOID, h.m, ".github/workflows/ci.yml") | |||
| 526 | + | ups := []domain.RefUpdate{{Old: domain.ZeroOID, New: h.m, Ref: "refs/tags/t"}} | |||
| 527 | + | trees, err := g.PushSubtrees(ctx, h.f.Name, ups, ports.PushObjects{}, domain.WorkflowDir, domain.DefaultPushWalkBounds()) | |||
| 528 | + | require.NoError(t, err) | |||
| 529 | + | assert.NotContains(t, trees.Subtrees, domain.HeadKey) | |||
| 530 | + | plain := h.commit("", "") | |||
| 531 | + | h.f.Git("update-ref", "refs/heads/plain", plain) | |||
| 532 | + | run(domain.ZeroOID, plain, "") | |||
| 533 | + | } | |||
M internal/adapters/gitcli/read.go +1 -9
| @@ -163,7 +163,7 @@ func resolve(c *catConn, rev domain.Revision) (domain.ResolvedRevision, error) { | |||
| 163 | 163 | if !ok { | |
| 164 | 164 | continue | |
| 165 | 165 | } | |
| 166 | - | return domain.ResolvedRevision{CommitID: id, Ref: candidateRef(cand)}, nil | |
| 166 | + | return domain.ResolvedRevision{CommitID: id, Ref: cand.ResolvedRef()}, nil | |
| 167 | 167 | } | |
| 168 | 168 | return domain.ResolvedRevision{}, revisionNotFound(rev) | |
| 169 | 169 | } | |
| @@ -191,14 +191,6 @@ func lookupRevisionCandidate(c *catConn, candidate domain.RevisionCandidate) (do | |||
| 191 | 191 | return id, true, nil | |
| 192 | 192 | } | |
| 193 | 193 | ||
| 194 | - | // candidateRef translates a ref lookup into the resolved ref field. | |
| 195 | - | func candidateRef(candidate domain.RevisionCandidate) string { | |
| 196 | - | if candidate.Kind == domain.RevisionCandidateRef { | |
| 197 | - | return candidate.Name | |
| 198 | - | } | |
| 199 | - | return "" | |
| 200 | - | } | |
| 201 | - | ||
| 202 | 194 | // ResolveRevision resolves a revision with `cat-file`. | |
| 203 | 195 | func (g *Git) ResolveRevision(ctx context.Context, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) { | |
| 204 | 196 | dir, err := g.repoDir(repo) | |
| @@ -163,7 +163,7 @@ func resolve(c *catConn, rev domain.Revision) (domain.ResolvedRevision, error) { | |||||
| 163 | if !ok { | 163 | if !ok { | ||
| 164 | continue | 164 | continue | ||
| 165 | } | 165 | } | ||
| 166 | - | return domain.ResolvedRevision{CommitID: id, Ref: candidateRef(cand)}, nil | 166 | + | return domain.ResolvedRevision{CommitID: id, Ref: cand.ResolvedRef()}, nil |
| 167 | } | 167 | } | ||
| 168 | return domain.ResolvedRevision{}, revisionNotFound(rev) | 168 | return domain.ResolvedRevision{}, revisionNotFound(rev) | ||
| 169 | } | 169 | } | ||
| @@ -191,14 +191,6 @@ func lookupRevisionCandidate(c *catConn, candidate domain.RevisionCandidate) (do | |||||
| 191 | return id, true, nil | 191 | return id, true, nil | ||
| 192 | } | 192 | } | ||
| 193 | 193 | ||||
| 194 | - | // candidateRef translates a ref lookup into the resolved ref field. | |||
| 195 | - | func candidateRef(candidate domain.RevisionCandidate) string { | |||
| 196 | - | if candidate.Kind == domain.RevisionCandidateRef { | |||
| 197 | - | return candidate.Name | |||
| 198 | - | } | |||
| 199 | - | return "" | |||
| 200 | - | } | |||
| 201 | - | ||||
| 202 | // ResolveRevision resolves a revision with `cat-file`. | 194 | // ResolveRevision resolves a revision with `cat-file`. | ||
| 203 | func (g *Git) ResolveRevision(ctx context.Context, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) { | 195 | func (g *Git) ResolveRevision(ctx context.Context, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) { | ||
| 204 | dir, err := g.repoDir(repo) | 196 | dir, err := g.repoDir(repo) | ||
M internal/adapters/githttp/handler.go +1 -1
| @@ -329,7 +329,7 @@ func credentials(r *http.Request) (secret string, present bool) { | |||
| 329 | 329 | // credentials is asked for them wherever the repository does not show itself, | |
| 330 | 330 | // which keeps a private repository and a missing one indistinguishable. | |
| 331 | 331 | // | |
| 332 | - | //hexcheck:ignore suspicious-business-logic-in-adapter maps authorization errors to HTTP responses, challenging anonymous callers on hidden or missing repositories | |
| 332 | + | //hexcheck:ignore suspicious-business-logic-in-adapter implements the smart-HTTP protocol visibility/auth challenge policy (401 vs 404) for hidden or missing repositories | |
| 333 | 333 | func (h *Handler) denied(ctx context.Context, w http.ResponseWriter, p domain.Principal, err error) { | |
| 334 | 334 | switch { | |
| 335 | 335 | case errors.Is(err, domain.ErrUnauthenticated): | |
| @@ -329,7 +329,7 @@ func credentials(r *http.Request) (secret string, present bool) { | |||||
| 329 | // credentials is asked for them wherever the repository does not show itself, | 329 | // credentials is asked for them wherever the repository does not show itself, | ||
| 330 | // which keeps a private repository and a missing one indistinguishable. | 330 | // which keeps a private repository and a missing one indistinguishable. | ||
| 331 | // | 331 | // | ||
| 332 | - | //hexcheck:ignore suspicious-business-logic-in-adapter maps authorization errors to HTTP responses, challenging anonymous callers on hidden or missing repositories | 332 | + | //hexcheck:ignore suspicious-business-logic-in-adapter implements the smart-HTTP protocol visibility/auth challenge policy (401 vs 404) for hidden or missing repositories |
| 333 | func (h *Handler) denied(ctx context.Context, w http.ResponseWriter, p domain.Principal, err error) { | 333 | func (h *Handler) denied(ctx context.Context, w http.ResponseWriter, p domain.Principal, err error) { | ||
| 334 | switch { | 334 | switch { | ||
| 335 | case errors.Is(err, domain.ErrUnauthenticated): | 335 | case errors.Is(err, domain.ErrUnauthenticated): | ||
M internal/adapters/kvstore/repos.go +2 -6
| @@ -207,12 +207,8 @@ func encodeRepo(r domain.Repo) ([]byte, error) { | |||
| 207 | 207 | if err != nil { | |
| 208 | 208 | return nil, err | |
| 209 | 209 | } | |
| 210 | - | settings := r.Settings | |
| 211 | - | if settings.IsZero() { | |
| 212 | - | // Settings are always stored explicitly, so that a later change of | |
| 213 | - | // the defaults never alters a stored repository. | |
| 214 | - | settings = domain.DefaultRepoSettings() | |
| 215 | - | } | |
| 210 | + | // Store defaults explicitly so future default changes do not alter stored repositories. | |
| 211 | + | settings := r.EffectiveSettings() | |
| 216 | 212 | pbSettings, err := encodeSettings(settings) | |
| 217 | 213 | if err != nil { | |
| 218 | 214 | return nil, err | |
| @@ -207,12 +207,8 @@ func encodeRepo(r domain.Repo) ([]byte, error) { | |||||
| 207 | if err != nil { | 207 | if err != nil { | ||
| 208 | return nil, err | 208 | return nil, err | ||
| 209 | } | 209 | } | ||
| 210 | - | settings := r.Settings | 210 | + | // Store defaults explicitly so future default changes do not alter stored repositories. |
| 211 | - | if settings.IsZero() { | 211 | + | settings := r.EffectiveSettings() |
| 212 | - | // Settings are always stored explicitly, so that a later change of | |||
| 213 | - | // the defaults never alters a stored repository. | |||
| 214 | - | settings = domain.DefaultRepoSettings() | |||
| 215 | - | } | |||
| 216 | pbSettings, err := encodeSettings(settings) | 212 | pbSettings, err := encodeSettings(settings) | ||
| 217 | if err != nil { | 213 | if err != nil { | ||
| 218 | return nil, err | 214 | return nil, err | ||
M internal/adapters/rpc/convert.go +1 -4
| @@ -58,10 +58,7 @@ func visibilityFromPB(v pb.Visibility) (domain.Visibility, error) { | |||
| 58 | 58 | } | |
| 59 | 59 | ||
| 60 | 60 | func repoToPB(v ports.RepoView) *pb.Repo { | |
| 61 | - | settings := v.Settings | |
| 62 | - | if settings.IsZero() { | |
| 63 | - | settings = domain.DefaultRepoSettings() | |
| 64 | - | } | |
| 61 | + | settings := v.EffectiveSettings() | |
| 65 | 62 | return &pb.Repo{ | |
| 66 | 63 | Name: v.Name.String(), | |
| 67 | 64 | Description: v.Description, | |
| @@ -58,10 +58,7 @@ func visibilityFromPB(v pb.Visibility) (domain.Visibility, error) { | |||||
| 58 | } | 58 | } | ||
| 59 | 59 | ||||
| 60 | func repoToPB(v ports.RepoView) *pb.Repo { | 60 | func repoToPB(v ports.RepoView) *pb.Repo { | ||
| 61 | - | settings := v.Settings | 61 | + | settings := v.EffectiveSettings() |
| 62 | - | if settings.IsZero() { | |||
| 63 | - | settings = domain.DefaultRepoSettings() | |||
| 64 | - | } | |||
| 65 | return &pb.Repo{ | 62 | return &pb.Repo{ | ||
| 66 | Name: v.Name.String(), | 63 | Name: v.Name.String(), | ||
| 67 | Description: v.Description, | 64 | Description: v.Description, | ||
M internal/adapters/web/events.go +3 -2
| @@ -342,6 +342,8 @@ func (s *Server) canSee(ctx context.Context, rawName string) error { | |||
| 342 | 342 | ||
| 343 | 343 | // issuesOn is domain.ErrNotFound unless the repository has issues on and | |
| 344 | 344 | // s.cfg.Issues is set: the same answer as for a repository the visitor cannot see. | |
| 345 | + | // | |
| 346 | + | //hexcheck:ignore suspicious-business-logic-in-adapter maps domain RepoSettings.IssuesEnabled to a not-found stream; decides nothing itself | |
| 345 | 347 | func (s *Server) issuesOn(ctx context.Context, rawName string) error { | |
| 346 | 348 | if s.cfg.Issues == nil { | |
| 347 | 349 | return domain.ErrNotFound | |
| @@ -354,8 +356,7 @@ func (s *Server) issuesOn(ctx context.Context, rawName string) error { | |||
| 354 | 356 | if err != nil { | |
| 355 | 357 | return err | |
| 356 | 358 | } | |
| 357 | - | enabled := v.EffectiveSettings().IssuesEnabled() | |
| 358 | - | if !enabled { | |
| 359 | + | if !v.EffectiveSettings().IssuesEnabled() { | |
| 359 | 360 | return domain.ErrNotFound | |
| 360 | 361 | } | |
| 361 | 362 | return nil | |
| @@ -342,6 +342,8 @@ func (s *Server) canSee(ctx context.Context, rawName string) error { | |||||
| 342 | 342 | ||||
| 343 | // issuesOn is domain.ErrNotFound unless the repository has issues on and | 343 | // issuesOn is domain.ErrNotFound unless the repository has issues on and | ||
| 344 | // s.cfg.Issues is set: the same answer as for a repository the visitor cannot see. | 344 | // s.cfg.Issues is set: the same answer as for a repository the visitor cannot see. | ||
| 345 | + | // | |||
| 346 | + | //hexcheck:ignore suspicious-business-logic-in-adapter maps domain RepoSettings.IssuesEnabled to a not-found stream; decides nothing itself | |||
| 345 | func (s *Server) issuesOn(ctx context.Context, rawName string) error { | 347 | func (s *Server) issuesOn(ctx context.Context, rawName string) error { | ||
| 346 | if s.cfg.Issues == nil { | 348 | if s.cfg.Issues == nil { | ||
| 347 | return domain.ErrNotFound | 349 | return domain.ErrNotFound | ||
| @@ -354,8 +356,7 @@ func (s *Server) issuesOn(ctx context.Context, rawName string) error { | |||||
| 354 | if err != nil { | 356 | if err != nil { | ||
| 355 | return err | 357 | return err | ||
| 356 | } | 358 | } | ||
| 357 | - | enabled := v.EffectiveSettings().IssuesEnabled() | 359 | + | if !v.EffectiveSettings().IssuesEnabled() { |
| 358 | - | if !enabled { | |||
| 359 | return domain.ErrNotFound | 360 | return domain.ErrNotFound | ||
| 360 | } | 361 | } | ||
| 361 | return nil | 362 | return nil | ||
M internal/adapters/web/fragments.go +3 -1
| @@ -355,6 +355,8 @@ func (f *fanout) lead(key fanKey, e *fanEntry, build func() ([]fragment, error)) | |||
| 355 | 355 | ||
| 356 | 356 | // banner tells the commits page how many commits its branch got since the | |
| 357 | 357 | // head it lists. | |
| 358 | + | // | |
| 359 | + | //hexcheck:ignore suspicious-business-logic-in-adapter picks the update of the listed branch from the push event; the count and rewrite come from MergeRequests.PushSummary | |
| 358 | 360 | func (l *liveStream) banner(ctx context.Context, p domain.PushEvent) ([]fragment, error) { | |
| 359 | 361 | if l.ref == "" || !isFullID(l.head) { | |
| 360 | 362 | return nil, nil | |
| @@ -363,7 +365,7 @@ func (l *liveStream) banner(ctx context.Context, p domain.PushEvent) ([]fragment | |||
| 363 | 365 | for _, u := range p.Updates { | |
| 364 | 366 | if u.Ref == "refs/heads/"+l.ref { | |
| 365 | 367 | tip, touched = u.New, true | |
| 366 | - | if u.New == domain.ZeroOID { | |
| 368 | + | if u.IsDelete() { | |
| 367 | 369 | tip = "" | |
| 368 | 370 | } | |
| 369 | 371 | } | |
| @@ -355,6 +355,8 @@ func (f *fanout) lead(key fanKey, e *fanEntry, build func() ([]fragment, error)) | |||||
| 355 | 355 | ||||
| 356 | // banner tells the commits page how many commits its branch got since the | 356 | // banner tells the commits page how many commits its branch got since the | ||
| 357 | // head it lists. | 357 | // head it lists. | ||
| 358 | + | // | |||
| 359 | + | //hexcheck:ignore suspicious-business-logic-in-adapter picks the update of the listed branch from the push event; the count and rewrite come from MergeRequests.PushSummary | |||
| 358 | func (l *liveStream) banner(ctx context.Context, p domain.PushEvent) ([]fragment, error) { | 360 | func (l *liveStream) banner(ctx context.Context, p domain.PushEvent) ([]fragment, error) { | ||
| 359 | if l.ref == "" || !isFullID(l.head) { | 361 | if l.ref == "" || !isFullID(l.head) { | ||
| 360 | return nil, nil | 362 | return nil, nil | ||
| @@ -363,7 +365,7 @@ func (l *liveStream) banner(ctx context.Context, p domain.PushEvent) ([]fragment | |||||
| 363 | for _, u := range p.Updates { | 365 | for _, u := range p.Updates { | ||
| 364 | if u.Ref == "refs/heads/"+l.ref { | 366 | if u.Ref == "refs/heads/"+l.ref { | ||
| 365 | tip, touched = u.New, true | 367 | tip, touched = u.New, true | ||
| 366 | - | if u.New == domain.ZeroOID { | 368 | + | if u.IsDelete() { |
| 367 | tip = "" | 369 | tip = "" | ||
| 368 | } | 370 | } | ||
| 369 | } | 371 | } | ||
M internal/adapters/web/issues.go +3 -3
| @@ -97,7 +97,7 @@ func (s *Server) issueRepo(ctx context.Context, rawName string) (*repoData, erro | |||
| 97 | 97 | if err != nil { | |
| 98 | 98 | return nil, err | |
| 99 | 99 | } | |
| 100 | - | if d.repo.Settings.Issues.OrDefault() == domain.IssuesOff { | |
| 100 | + | if !d.repo.Settings.IssuesEnabled() { | |
| 101 | 101 | return nil, errNotFound | |
| 102 | 102 | } | |
| 103 | 103 | return d, nil | |
| @@ -129,7 +129,7 @@ func (s *Server) issueRow(repo string, i domain.Issue) IssueRow { | |||
| 129 | 129 | ||
| 130 | 130 | // issueList lists the issues of one state, a page at a time. | |
| 131 | 131 | // | |
| 132 | - | //hexcheck:ignore suspicious-business-logic-in-adapter builds the list page from the use case results; whether the New issue button shows only reads the mode | |
| 132 | + | //hexcheck:ignore suspicious-business-logic-in-adapter renders issue-list navigation; signed-in users see New issue, while anonymous users need a configured form and the domain AcceptsAnonymousIssues predicate | |
| 133 | 133 | func (s *Server) issueList(ctx context.Context, r *http.Request) (templ.Component, int, error) { | |
| 134 | 134 | d, err := s.issueRepo(ctx, r.PathValue("repo")) | |
| 135 | 135 | if err != nil { | |
| @@ -179,7 +179,7 @@ func (s *Server) issueList(ctx context.Context, r *http.Request) (templ.Componen | |||
| 179 | 179 | if token != "" { | |
| 180 | 180 | p.Newest = link("") | |
| 181 | 181 | } | |
| 182 | - | if SignedIn(ctx) || (s.cfg.IssueForm != nil && d.repo.Settings.Issues.OrDefault() == domain.IssuesPublic) { | |
| 182 | + | if SignedIn(ctx) || (s.cfg.IssueForm != nil && d.repo.Settings.AcceptsAnonymousIssues()) { | |
| 183 | 183 | p.NewHref = issuesBase(d.name()) + "/new" | |
| 184 | 184 | } | |
| 185 | 185 | ||
| @@ -97,7 +97,7 @@ func (s *Server) issueRepo(ctx context.Context, rawName string) (*repoData, erro | |||||
| 97 | if err != nil { | 97 | if err != nil { | ||
| 98 | return nil, err | 98 | return nil, err | ||
| 99 | } | 99 | } | ||
| 100 | - | if d.repo.Settings.Issues.OrDefault() == domain.IssuesOff { | 100 | + | if !d.repo.Settings.IssuesEnabled() { |
| 101 | return nil, errNotFound | 101 | return nil, errNotFound | ||
| 102 | } | 102 | } | ||
| 103 | return d, nil | 103 | return d, nil | ||
| @@ -129,7 +129,7 @@ func (s *Server) issueRow(repo string, i domain.Issue) IssueRow { | |||||
| 129 | 129 | ||||
| 130 | // issueList lists the issues of one state, a page at a time. | 130 | // issueList lists the issues of one state, a page at a time. | ||
| 131 | // | 131 | // | ||
| 132 | - | //hexcheck:ignore suspicious-business-logic-in-adapter builds the list page from the use case results; whether the New issue button shows only reads the mode | 132 | + | //hexcheck:ignore suspicious-business-logic-in-adapter renders issue-list navigation; signed-in users see New issue, while anonymous users need a configured form and the domain AcceptsAnonymousIssues predicate |
| 133 | func (s *Server) issueList(ctx context.Context, r *http.Request) (templ.Component, int, error) { | 133 | func (s *Server) issueList(ctx context.Context, r *http.Request) (templ.Component, int, error) { | ||
| 134 | d, err := s.issueRepo(ctx, r.PathValue("repo")) | 134 | d, err := s.issueRepo(ctx, r.PathValue("repo")) | ||
| 135 | if err != nil { | 135 | if err != nil { | ||
| @@ -179,7 +179,7 @@ func (s *Server) issueList(ctx context.Context, r *http.Request) (templ.Componen | |||||
| 179 | if token != "" { | 179 | if token != "" { | ||
| 180 | p.Newest = link("") | 180 | p.Newest = link("") | ||
| 181 | } | 181 | } | ||
| 182 | - | if SignedIn(ctx) || (s.cfg.IssueForm != nil && d.repo.Settings.Issues.OrDefault() == domain.IssuesPublic) { | 182 | + | if SignedIn(ctx) || (s.cfg.IssueForm != nil && d.repo.Settings.AcceptsAnonymousIssues()) { |
| 183 | p.NewHref = issuesBase(d.name()) + "/new" | 183 | p.NewHref = issuesBase(d.name()) + "/new" | ||
| 184 | } | 184 | } | ||
| 185 | 185 | ||||
M internal/adapters/web/issues_form.go +3 -2
| @@ -245,6 +245,8 @@ func (s *Server) formAgain(ctx context.Context, w http.ResponseWriter, r *http.R | |||
| 245 | 245 | } | |
| 246 | 246 | ||
| 247 | 247 | // issueSubmittedPage is GET /<repo>/issues/submitted. | |
| 248 | + | // | |
| 249 | + | //hexcheck:ignore suspicious-business-logic-in-adapter maps domain RepoSettings.ShowsIssueSubmissionReceipt to a 404; decides nothing itself | |
| 248 | 250 | func (s *Server) issueSubmittedPage(w http.ResponseWriter, r *http.Request) { | |
| 249 | 251 | ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) | |
| 250 | 252 | defer cancel() | |
| @@ -253,8 +255,7 @@ func (s *Server) issueSubmittedPage(w http.ResponseWriter, r *http.Request) { | |||
| 253 | 255 | s.fail(ctx, w, r, err) | |
| 254 | 256 | return | |
| 255 | 257 | } | |
| 256 | - | showReceipt := d.repo.Settings.ShowsIssueSubmissionReceipt() | |
| 257 | - | if !showReceipt { | |
| 258 | + | if !d.repo.Settings.ShowsIssueSubmissionReceipt() { | |
| 258 | 259 | s.fail(ctx, w, r, errNotFound) | |
| 259 | 260 | return | |
| 260 | 261 | } | |
| @@ -245,6 +245,8 @@ func (s *Server) formAgain(ctx context.Context, w http.ResponseWriter, r *http.R | |||||
| 245 | } | 245 | } | ||
| 246 | 246 | ||||
| 247 | // issueSubmittedPage is GET /<repo>/issues/submitted. | 247 | // issueSubmittedPage is GET /<repo>/issues/submitted. | ||
| 248 | + | // | |||
| 249 | + | //hexcheck:ignore suspicious-business-logic-in-adapter maps domain RepoSettings.ShowsIssueSubmissionReceipt to a 404; decides nothing itself | |||
| 248 | func (s *Server) issueSubmittedPage(w http.ResponseWriter, r *http.Request) { | 250 | func (s *Server) issueSubmittedPage(w http.ResponseWriter, r *http.Request) { | ||
| 249 | ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) | 251 | ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) | ||
| 250 | defer cancel() | 252 | defer cancel() | ||
| @@ -253,8 +255,7 @@ func (s *Server) issueSubmittedPage(w http.ResponseWriter, r *http.Request) { | |||||
| 253 | s.fail(ctx, w, r, err) | 255 | s.fail(ctx, w, r, err) | ||
| 254 | return | 256 | return | ||
| 255 | } | 257 | } | ||
| 256 | - | showReceipt := d.repo.Settings.ShowsIssueSubmissionReceipt() | 258 | + | if !d.repo.Settings.ShowsIssueSubmissionReceipt() { |
| 257 | - | if !showReceipt { | |||
| 258 | s.fail(ctx, w, r, errNotFound) | 259 | s.fail(ctx, w, r, errNotFound) | ||
| 259 | return | 260 | return | ||
| 260 | } | 261 | } | ||
M internal/adapters/web/mr.go +1 -1
| @@ -654,7 +654,7 @@ func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ | |||
| 654 | 654 | ||
| 655 | 655 | func (s *Server) mrBox(ctx context.Context, d *repoData, res ports.MergeRequestDetail) *MergeBox { | |
| 656 | 656 | m := res.MergeRequest | |
| 657 | - | box := s.mergeBox(d, m.Number, m.Source, m.HeadID.String(), res.CanMarkRead, mergeabilityView(res.Mergeability, m.Target)) | |
| 657 | + | box := s.mergeBox(d, m.Number, m.Source, m.HeadID.String(), res.HeadUnread, mergeabilityView(res.Mergeability, m.Target)) | |
| 658 | 658 | box.Error = mergeErrorOf(ctx) | |
| 659 | 659 | if m.IsDraft() { | |
| 660 | 660 | box.ReadyAction = mrHref(d.name(), m.Number, "ready") | |
| @@ -654,7 +654,7 @@ func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ | |||||
| 654 | 654 | ||||
| 655 | func (s *Server) mrBox(ctx context.Context, d *repoData, res ports.MergeRequestDetail) *MergeBox { | 655 | func (s *Server) mrBox(ctx context.Context, d *repoData, res ports.MergeRequestDetail) *MergeBox { | ||
| 656 | m := res.MergeRequest | 656 | m := res.MergeRequest | ||
| 657 | - | box := s.mergeBox(d, m.Number, m.Source, m.HeadID.String(), res.CanMarkRead, mergeabilityView(res.Mergeability, m.Target)) | 657 | + | box := s.mergeBox(d, m.Number, m.Source, m.HeadID.String(), res.HeadUnread, mergeabilityView(res.Mergeability, m.Target)) |
| 658 | box.Error = mergeErrorOf(ctx) | 658 | box.Error = mergeErrorOf(ctx) | ||
| 659 | if m.IsDraft() { | 659 | if m.IsDraft() { | ||
| 660 | box.ReadyAction = mrHref(d.name(), m.Number, "ready") | 660 | box.ReadyAction = mrHref(d.name(), m.Number, "ready") | ||
M internal/adapters/web/repo.go +1 -1
| @@ -128,7 +128,7 @@ func (s *Server) openMRCount(ctx context.Context, name domain.RepoName) string { | |||
| 128 | 128 | // switcher on the head ref and the ":" targets. The caller sets the title, | |
| 129 | 129 | // the crumbs, the status and the rail. | |
| 130 | 130 | // | |
| 131 | - | //hexcheck:ignore suspicious-business-logic-in-adapter lists the tabs of the repository; the issues tab follows the setting, no rule is decided here | |
| 131 | + | //hexcheck:ignore suspicious-business-logic-in-adapter renders repository navigation; the issues tab uses the domain IssuesEnabled predicate | |
| 132 | 132 | func (s *Server) repoLayout(ctx context.Context, d *repoData, section string) Layout { | |
| 133 | 133 | l := s.base() | |
| 134 | 134 | l.Repo = d.name() | |
| @@ -128,7 +128,7 @@ func (s *Server) openMRCount(ctx context.Context, name domain.RepoName) string { | |||||
| 128 | // switcher on the head ref and the ":" targets. The caller sets the title, | 128 | // switcher on the head ref and the ":" targets. The caller sets the title, | ||
| 129 | // the crumbs, the status and the rail. | 129 | // the crumbs, the status and the rail. | ||
| 130 | // | 130 | // | ||
| 131 | - | //hexcheck:ignore suspicious-business-logic-in-adapter lists the tabs of the repository; the issues tab follows the setting, no rule is decided here | 131 | + | //hexcheck:ignore suspicious-business-logic-in-adapter renders repository navigation; the issues tab uses the domain IssuesEnabled predicate |
| 132 | func (s *Server) repoLayout(ctx context.Context, d *repoData, section string) Layout { | 132 | func (s *Server) repoLayout(ctx context.Context, d *repoData, section string) Layout { | ||
| 133 | l := s.base() | 133 | l := s.base() | ||
| 134 | l.Repo = d.name() | 134 | l.Repo = d.name() | ||
M internal/domain/ci_workflow_guard.go +8 -11
| @@ -4,14 +4,14 @@ package domain | |||
| 4 | 4 | ||
| 5 | 5 | import "fmt" | |
| 6 | 6 | ||
| 7 | - | // ObjectType is the type of a peeled Git object. | |
| 8 | - | type ObjectType string | |
| 7 | + | // PushObjectType is the type of a peeled Git object. | |
| 8 | + | type PushObjectType string | |
| 9 | 9 | ||
| 10 | 10 | // Peeled object types accepted by the push tree reader. | |
| 11 | 11 | const ( | |
| 12 | - | ObjectCommit ObjectType = "commit" | |
| 13 | - | ObjectTree ObjectType = "tree" | |
| 14 | - | ObjectBlob ObjectType = "blob" | |
| 12 | + | PushObjectCommit PushObjectType = "commit" | |
| 13 | + | PushObjectTree PushObjectType = "tree" | |
| 14 | + | PushObjectBlob PushObjectType = "blob" | |
| 15 | 15 | ) | |
| 16 | 16 | ||
| 17 | 17 | // PushWalkBounds bounds the walk over the commits of a push. | |
| @@ -26,9 +26,6 @@ const ( | |||
| 26 | 26 | // DefaultPushWalkBounds returns the job-token push walk limits. | |
| 27 | 27 | func DefaultPushWalkBounds() PushWalkBounds { return PushWalkBounds{MaxPushedCommits, MaxPushedIDs} } | |
| 28 | 28 | ||
| 29 | - | // PushedEnd describes a peeled ref endpoint. | |
| 30 | - | type PushedEnd struct{ Type ObjectType } | |
| 31 | - | ||
| 32 | 29 | // Subtree is the kind and identity of an entry at a tree path. | |
| 33 | 30 | type Subtree struct { | |
| 34 | 31 | Kind EntryKind | |
| @@ -48,7 +45,7 @@ const HeadKey = "HEAD" | |||
| 48 | 45 | // (or HeadKey), and newly reachable commits in rev-list order. Absent entries | |
| 49 | 46 | // mean there is no subtree at the requested path. | |
| 50 | 47 | type PushSubtrees struct { | |
| 51 | - | Ends map[string]PushedEnd | |
| 48 | + | Ends map[string]PushObjectType | |
| 52 | 49 | Commits []PushedCommit | |
| 53 | 50 | Subtrees map[string]Subtree | |
| 54 | 51 | } | |
| @@ -105,7 +102,7 @@ func WorkflowChangeOf(updates []RefUpdate, t PushSubtrees) (WorkflowTreeChange, | |||
| 105 | 102 | } | |
| 106 | 103 | } | |
| 107 | 104 | for _, up := range updates { | |
| 108 | - | if up.IsDelete() || t.Ends[up.New].Type == ObjectBlob { | |
| 105 | + | if up.IsDelete() || t.Ends[up.New] == PushObjectBlob { | |
| 109 | 106 | continue | |
| 110 | 107 | } | |
| 111 | 108 | cur := dirOf(up.New) | |
| @@ -115,7 +112,7 @@ func WorkflowChangeOf(updates []RefUpdate, t PushSubtrees) (WorkflowTreeChange, | |||
| 115 | 112 | var base ObjectID | |
| 116 | 113 | if up.IsCreate() { | |
| 117 | 114 | base = dirOf(HeadKey) | |
| 118 | - | } else if t.Ends[up.Old].Type != ObjectBlob { | |
| 115 | + | } else if t.Ends[up.Old] != PushObjectBlob { | |
| 119 | 116 | base = dirOf(up.Old) | |
| 120 | 117 | } | |
| 121 | 118 | if base != cur { | |
| @@ -4,14 +4,14 @@ package domain | |||||
| 4 | 4 | ||||
| 5 | import "fmt" | 5 | import "fmt" | ||
| 6 | 6 | ||||
| 7 | - | // ObjectType is the type of a peeled Git object. | 7 | + | // PushObjectType is the type of a peeled Git object. |
| 8 | - | type ObjectType string | 8 | + | type PushObjectType string |
| 9 | 9 | ||||
| 10 | // Peeled object types accepted by the push tree reader. | 10 | // Peeled object types accepted by the push tree reader. | ||
| 11 | const ( | 11 | const ( | ||
| 12 | - | ObjectCommit ObjectType = "commit" | 12 | + | PushObjectCommit PushObjectType = "commit" |
| 13 | - | ObjectTree ObjectType = "tree" | 13 | + | PushObjectTree PushObjectType = "tree" |
| 14 | - | ObjectBlob ObjectType = "blob" | 14 | + | PushObjectBlob PushObjectType = "blob" |
| 15 | ) | 15 | ) | ||
| 16 | 16 | ||||
| 17 | // PushWalkBounds bounds the walk over the commits of a push. | 17 | // PushWalkBounds bounds the walk over the commits of a push. | ||
| @@ -26,9 +26,6 @@ const ( | |||||
| 26 | // DefaultPushWalkBounds returns the job-token push walk limits. | 26 | // DefaultPushWalkBounds returns the job-token push walk limits. | ||
| 27 | func DefaultPushWalkBounds() PushWalkBounds { return PushWalkBounds{MaxPushedCommits, MaxPushedIDs} } | 27 | func DefaultPushWalkBounds() PushWalkBounds { return PushWalkBounds{MaxPushedCommits, MaxPushedIDs} } | ||
| 28 | 28 | ||||
| 29 | - | // PushedEnd describes a peeled ref endpoint. | |||
| 30 | - | type PushedEnd struct{ Type ObjectType } | |||
| 31 | - | ||||
| 32 | // Subtree is the kind and identity of an entry at a tree path. | 29 | // Subtree is the kind and identity of an entry at a tree path. | ||
| 33 | type Subtree struct { | 30 | type Subtree struct { | ||
| 34 | Kind EntryKind | 31 | Kind EntryKind | ||
| @@ -48,7 +45,7 @@ const HeadKey = "HEAD" | |||||
| 48 | // (or HeadKey), and newly reachable commits in rev-list order. Absent entries | 45 | // (or HeadKey), and newly reachable commits in rev-list order. Absent entries | ||
| 49 | // mean there is no subtree at the requested path. | 46 | // mean there is no subtree at the requested path. | ||
| 50 | type PushSubtrees struct { | 47 | type PushSubtrees struct { | ||
| 51 | - | Ends map[string]PushedEnd | 48 | + | Ends map[string]PushObjectType |
| 52 | Commits []PushedCommit | 49 | Commits []PushedCommit | ||
| 53 | Subtrees map[string]Subtree | 50 | Subtrees map[string]Subtree | ||
| 54 | } | 51 | } | ||
| @@ -105,7 +102,7 @@ func WorkflowChangeOf(updates []RefUpdate, t PushSubtrees) (WorkflowTreeChange, | |||||
| 105 | } | 102 | } | ||
| 106 | } | 103 | } | ||
| 107 | for _, up := range updates { | 104 | for _, up := range updates { | ||
| 108 | - | if up.IsDelete() || t.Ends[up.New].Type == ObjectBlob { | 105 | + | if up.IsDelete() || t.Ends[up.New] == PushObjectBlob { |
| 109 | continue | 106 | continue | ||
| 110 | } | 107 | } | ||
| 111 | cur := dirOf(up.New) | 108 | cur := dirOf(up.New) | ||
| @@ -115,7 +112,7 @@ func WorkflowChangeOf(updates []RefUpdate, t PushSubtrees) (WorkflowTreeChange, | |||||
| 115 | var base ObjectID | 112 | var base ObjectID | ||
| 116 | if up.IsCreate() { | 113 | if up.IsCreate() { | ||
| 117 | base = dirOf(HeadKey) | 114 | base = dirOf(HeadKey) | ||
| 118 | - | } else if t.Ends[up.Old].Type != ObjectBlob { | 115 | + | } else if t.Ends[up.Old] != PushObjectBlob { |
| 119 | base = dirOf(up.Old) | 116 | base = dirOf(up.Old) | ||
| 120 | } | 117 | } | ||
| 121 | if base != cur { | 118 | if base != cur { | ||
M internal/domain/ci_workflow_guard_test.go +16 -15
| @@ -35,14 +35,15 @@ func TestWorkflowChangeOf(t *testing.T) { | |||
| 35 | 35 | return domain.PushedCommit{ID: v, Parents: parents} | |
| 36 | 36 | } | |
| 37 | 37 | type row struct { | |
| 38 | - | name string | |
| 39 | - | ups []domain.RefUpdate | |
| 40 | - | commits []domain.PushedCommit | |
| 41 | - | trees map[string]domain.Subtree | |
| 42 | - | blobs []domain.ObjectID | |
| 43 | - | want domain.WorkflowTreeChange | |
| 44 | - | found bool | |
| 45 | - | missing bool | |
| 38 | + | name string | |
| 39 | + | ups []domain.RefUpdate | |
| 40 | + | commits []domain.PushedCommit | |
| 41 | + | trees map[string]domain.Subtree | |
| 42 | + | treeEnds []domain.ObjectID | |
| 43 | + | blobs []domain.ObjectID | |
| 44 | + | want domain.WorkflowTreeChange | |
| 45 | + | found bool | |
| 46 | + | missing bool | |
| 46 | 47 | } | |
| 47 | 48 | change := func(old, new domain.ObjectID) domain.WorkflowTreeChange { | |
| 48 | 49 | return domain.WorkflowTreeChange{Old: old, New: new} | |
| @@ -77,8 +78,8 @@ func TestWorkflowChangeOf(t *testing.T) { | |||
| 77 | 78 | {name: "missing ends fail closed", ups: []domain.RefUpdate{up(a, b)}, missing: true}, | |
| 78 | 79 | } | |
| 79 | 80 | rows = append(rows, | |
| 80 | - | row{name: "new tree end with workflows", ups: []domain.RefUpdate{up(zero, a)}, trees: map[string]domain.Subtree{a.String(): dir(x)}, want: change(zero, x), found: true}, | |
| 81 | - | row{name: "new tree end without workflows", ups: []domain.RefUpdate{up(zero, a)}}, | |
| 81 | + | row{name: "new tree end with workflows", treeEnds: []domain.ObjectID{a}, ups: []domain.RefUpdate{up(zero, a)}, trees: map[string]domain.Subtree{a.String(): dir(x)}, want: change(zero, x), found: true}, | |
| 82 | + | row{name: "new tree end without workflows", treeEnds: []domain.ObjectID{a}, ups: []domain.RefUpdate{up(zero, a)}}, | |
| 82 | 83 | row{name: "new blob ref", ups: []domain.RefUpdate{up(zero, a)}, blobs: []domain.ObjectID{a}}, | |
| 83 | 84 | ) | |
| 84 | 85 | ||
| @@ -92,19 +93,19 @@ func TestWorkflowChangeOf(t *testing.T) { | |||
| 92 | 93 | rows = append(rows, row{name: "gitlink to symlink", ups: []domain.RefUpdate{up(a, b)}, trees: map[string]domain.Subtree{a.String(): {Kind: domain.EntrySubmodule, ID: x}, b.String(): {Kind: domain.EntrySymlink, ID: y}}}) | |
| 93 | 94 | for _, r := range rows { | |
| 94 | 95 | t.Run(r.name, func(t *testing.T) { | |
| 95 | - | data := domain.PushSubtrees{Ends: map[string]domain.PushedEnd{}, Commits: r.commits, Subtrees: r.trees} | |
| 96 | + | data := domain.PushSubtrees{Ends: map[string]domain.PushObjectType{}, Commits: r.commits, Subtrees: r.trees} | |
| 96 | 97 | if !r.missing { | |
| 97 | 98 | for _, u := range r.ups { | |
| 98 | 99 | for _, oid := range []string{u.Old, u.New} { | |
| 99 | - | data.Ends[oid] = domain.PushedEnd{Type: domain.ObjectCommit} | |
| 100 | + | data.Ends[oid] = domain.PushObjectCommit | |
| 100 | 101 | } | |
| 101 | 102 | } | |
| 102 | 103 | } | |
| 103 | - | if r.name == "new tree end with workflows" || r.name == "new tree end without workflows" { | |
| 104 | - | data.Ends[a.String()] = domain.PushedEnd{Type: domain.ObjectTree} | |
| 104 | + | for _, tree := range r.treeEnds { | |
| 105 | + | data.Ends[tree.String()] = domain.PushObjectTree | |
| 105 | 106 | } | |
| 106 | 107 | for _, b := range r.blobs { | |
| 107 | - | data.Ends[b.String()] = domain.PushedEnd{Type: domain.ObjectBlob} | |
| 108 | + | data.Ends[b.String()] = domain.PushObjectBlob | |
| 108 | 109 | } | |
| 109 | 110 | got, found, err := domain.WorkflowChangeOf(r.ups, data) | |
| 110 | 111 | if r.missing { | |
| @@ -35,14 +35,15 @@ func TestWorkflowChangeOf(t *testing.T) { | |||||
| 35 | return domain.PushedCommit{ID: v, Parents: parents} | 35 | return domain.PushedCommit{ID: v, Parents: parents} | ||
| 36 | } | 36 | } | ||
| 37 | type row struct { | 37 | type row struct { | ||
| 38 | - | name string | 38 | + | name string |
| 39 | - | ups []domain.RefUpdate | 39 | + | ups []domain.RefUpdate |
| 40 | - | commits []domain.PushedCommit | 40 | + | commits []domain.PushedCommit |
| 41 | - | trees map[string]domain.Subtree | 41 | + | trees map[string]domain.Subtree |
| 42 | - | blobs []domain.ObjectID | 42 | + | treeEnds []domain.ObjectID |
| 43 | - | want domain.WorkflowTreeChange | 43 | + | blobs []domain.ObjectID |
| 44 | - | found bool | 44 | + | want domain.WorkflowTreeChange |
| 45 | - | missing bool | 45 | + | found bool |
| 46 | + | missing bool | |||
| 46 | } | 47 | } | ||
| 47 | change := func(old, new domain.ObjectID) domain.WorkflowTreeChange { | 48 | change := func(old, new domain.ObjectID) domain.WorkflowTreeChange { | ||
| 48 | return domain.WorkflowTreeChange{Old: old, New: new} | 49 | return domain.WorkflowTreeChange{Old: old, New: new} | ||
| @@ -77,8 +78,8 @@ func TestWorkflowChangeOf(t *testing.T) { | |||||
| 77 | {name: "missing ends fail closed", ups: []domain.RefUpdate{up(a, b)}, missing: true}, | 78 | {name: "missing ends fail closed", ups: []domain.RefUpdate{up(a, b)}, missing: true}, | ||
| 78 | } | 79 | } | ||
| 79 | rows = append(rows, | 80 | rows = append(rows, | ||
| 80 | - | row{name: "new tree end with workflows", ups: []domain.RefUpdate{up(zero, a)}, trees: map[string]domain.Subtree{a.String(): dir(x)}, want: change(zero, x), found: true}, | 81 | + | row{name: "new tree end with workflows", treeEnds: []domain.ObjectID{a}, ups: []domain.RefUpdate{up(zero, a)}, trees: map[string]domain.Subtree{a.String(): dir(x)}, want: change(zero, x), found: true}, |
| 81 | - | row{name: "new tree end without workflows", ups: []domain.RefUpdate{up(zero, a)}}, | 82 | + | row{name: "new tree end without workflows", treeEnds: []domain.ObjectID{a}, ups: []domain.RefUpdate{up(zero, a)}}, |
| 82 | row{name: "new blob ref", ups: []domain.RefUpdate{up(zero, a)}, blobs: []domain.ObjectID{a}}, | 83 | row{name: "new blob ref", ups: []domain.RefUpdate{up(zero, a)}, blobs: []domain.ObjectID{a}}, | ||
| 83 | ) | 84 | ) | ||
| 84 | 85 | ||||
| @@ -92,19 +93,19 @@ func TestWorkflowChangeOf(t *testing.T) { | |||||
| 92 | rows = append(rows, row{name: "gitlink to symlink", ups: []domain.RefUpdate{up(a, b)}, trees: map[string]domain.Subtree{a.String(): {Kind: domain.EntrySubmodule, ID: x}, b.String(): {Kind: domain.EntrySymlink, ID: y}}}) | 93 | rows = append(rows, row{name: "gitlink to symlink", ups: []domain.RefUpdate{up(a, b)}, trees: map[string]domain.Subtree{a.String(): {Kind: domain.EntrySubmodule, ID: x}, b.String(): {Kind: domain.EntrySymlink, ID: y}}}) | ||
| 93 | for _, r := range rows { | 94 | for _, r := range rows { | ||
| 94 | t.Run(r.name, func(t *testing.T) { | 95 | t.Run(r.name, func(t *testing.T) { | ||
| 95 | - | data := domain.PushSubtrees{Ends: map[string]domain.PushedEnd{}, Commits: r.commits, Subtrees: r.trees} | 96 | + | data := domain.PushSubtrees{Ends: map[string]domain.PushObjectType{}, Commits: r.commits, Subtrees: r.trees} |
| 96 | if !r.missing { | 97 | if !r.missing { | ||
| 97 | for _, u := range r.ups { | 98 | for _, u := range r.ups { | ||
| 98 | for _, oid := range []string{u.Old, u.New} { | 99 | for _, oid := range []string{u.Old, u.New} { | ||
| 99 | - | data.Ends[oid] = domain.PushedEnd{Type: domain.ObjectCommit} | 100 | + | data.Ends[oid] = domain.PushObjectCommit |
| 100 | } | 101 | } | ||
| 101 | } | 102 | } | ||
| 102 | } | 103 | } | ||
| 103 | - | if r.name == "new tree end with workflows" || r.name == "new tree end without workflows" { | 104 | + | for _, tree := range r.treeEnds { |
| 104 | - | data.Ends[a.String()] = domain.PushedEnd{Type: domain.ObjectTree} | 105 | + | data.Ends[tree.String()] = domain.PushObjectTree |
| 105 | } | 106 | } | ||
| 106 | for _, b := range r.blobs { | 107 | for _, b := range r.blobs { | ||
| 107 | - | data.Ends[b.String()] = domain.PushedEnd{Type: domain.ObjectBlob} | 108 | + | data.Ends[b.String()] = domain.PushObjectBlob |
| 108 | } | 109 | } | ||
| 109 | got, found, err := domain.WorkflowChangeOf(r.ups, data) | 110 | got, found, err := domain.WorkflowChangeOf(r.ups, data) | ||
| 110 | if r.missing { | 111 | if r.missing { | ||
M internal/domain/mergerequest.go +3 -0
| @@ -575,3 +575,6 @@ func (s RepoSettings) IssuesEnabled() bool { return s.Issues.OrDefault() != Issu | |||
| 575 | 575 | ||
| 576 | 576 | // ShowsIssueSubmissionReceipt reports whether submissions await public moderation. | |
| 577 | 577 | func (s RepoSettings) ShowsIssueSubmissionReceipt() bool { return s.Issues.OrDefault() == IssuesPublic } | |
| 578 | + | ||
| 579 | + | // AcceptsAnonymousIssues reports whether anyone may submit an issue. | |
| 580 | + | func (s RepoSettings) AcceptsAnonymousIssues() bool { return s.Issues.OrDefault() == IssuesPublic } | |
| @@ -575,3 +575,6 @@ func (s RepoSettings) IssuesEnabled() bool { return s.Issues.OrDefault() != Issu | |||||
| 575 | 575 | ||||
| 576 | // ShowsIssueSubmissionReceipt reports whether submissions await public moderation. | 576 | // ShowsIssueSubmissionReceipt reports whether submissions await public moderation. | ||
| 577 | func (s RepoSettings) ShowsIssueSubmissionReceipt() bool { return s.Issues.OrDefault() == IssuesPublic } | 577 | func (s RepoSettings) ShowsIssueSubmissionReceipt() bool { return s.Issues.OrDefault() == IssuesPublic } | ||
| 578 | + | ||||
| 579 | + | // AcceptsAnonymousIssues reports whether anyone may submit an issue. | |||
| 580 | + | func (s RepoSettings) AcceptsAnonymousIssues() bool { return s.Issues.OrDefault() == IssuesPublic } | |||
M internal/domain/repo_test.go +1 -0
| @@ -90,6 +90,7 @@ func TestRepoIssuePresentationRules(t *testing.T) { | |||
| 90 | 90 | s := domain.RepoSettings{Issues: tc.mode} | |
| 91 | 91 | assert.Equal(t, tc.enabled, s.IssuesEnabled()) | |
| 92 | 92 | assert.Equal(t, tc.receipt, s.ShowsIssueSubmissionReceipt()) | |
| 93 | + | assert.Equal(t, tc.receipt, s.AcceptsAnonymousIssues()) | |
| 93 | 94 | }) | |
| 94 | 95 | } | |
| 95 | 96 | } | |
| @@ -90,6 +90,7 @@ func TestRepoIssuePresentationRules(t *testing.T) { | |||||
| 90 | s := domain.RepoSettings{Issues: tc.mode} | 90 | s := domain.RepoSettings{Issues: tc.mode} | ||
| 91 | assert.Equal(t, tc.enabled, s.IssuesEnabled()) | 91 | assert.Equal(t, tc.enabled, s.IssuesEnabled()) | ||
| 92 | assert.Equal(t, tc.receipt, s.ShowsIssueSubmissionReceipt()) | 92 | assert.Equal(t, tc.receipt, s.ShowsIssueSubmissionReceipt()) | ||
| 93 | + | assert.Equal(t, tc.receipt, s.AcceptsAnonymousIssues()) | |||
| 93 | }) | 94 | }) | ||
| 94 | } | 95 | } | ||
| 95 | } | 96 | } | ||
M internal/domain/revision.go +8 -0
| @@ -44,3 +44,11 @@ func (r Revision) Candidates() []RevisionCandidate { | |||
| 44 | 44 | } | |
| 45 | 45 | return refs | |
| 46 | 46 | } | |
| 47 | + | ||
| 48 | + | // ResolvedRef returns the full ref name for a ref candidate, or empty otherwise. | |
| 49 | + | func (c RevisionCandidate) ResolvedRef() string { | |
| 50 | + | if c.Kind == RevisionCandidateRef { | |
| 51 | + | return c.Name | |
| 52 | + | } | |
| 53 | + | return "" | |
| 54 | + | } | |
| @@ -44,3 +44,11 @@ func (r Revision) Candidates() []RevisionCandidate { | |||||
| 44 | } | 44 | } | ||
| 45 | return refs | 45 | return refs | ||
| 46 | } | 46 | } | ||
| 47 | + | ||||
| 48 | + | // ResolvedRef returns the full ref name for a ref candidate, or empty otherwise. | |||
| 49 | + | func (c RevisionCandidate) ResolvedRef() string { | |||
| 50 | + | if c.Kind == RevisionCandidateRef { | |||
| 51 | + | return c.Name | |||
| 52 | + | } | |||
| 53 | + | return "" | |||
| 54 | + | } | |||
M internal/domain/revision_test.go +12 -0
| @@ -44,3 +44,15 @@ func TestRevisionCandidates(t *testing.T) { | |||
| 44 | 44 | }) | |
| 45 | 45 | } | |
| 46 | 46 | } | |
| 47 | + | ||
| 48 | + | func TestRevisionCandidateResolvedRef(t *testing.T) { | |
| 49 | + | for _, tc := range []struct { | |
| 50 | + | kind domain.RevisionCandidateKind | |
| 51 | + | want string | |
| 52 | + | }{ | |
| 53 | + | {domain.RevisionCandidateHead, ""}, {domain.RevisionCandidateObject, ""}, | |
| 54 | + | {domain.RevisionCandidateRef, "refs/heads/main"}, {domain.RevisionCandidateAbbrevID, ""}, | |
| 55 | + | } { | |
| 56 | + | assert.Equal(t, tc.want, (domain.RevisionCandidate{Kind: tc.kind, Name: "refs/heads/main"}).ResolvedRef()) | |
| 57 | + | } | |
| 58 | + | } | |
| @@ -44,3 +44,15 @@ func TestRevisionCandidates(t *testing.T) { | |||||
| 44 | }) | 44 | }) | ||
| 45 | } | 45 | } | ||
| 46 | } | 46 | } | ||
| 47 | + | ||||
| 48 | + | func TestRevisionCandidateResolvedRef(t *testing.T) { | |||
| 49 | + | for _, tc := range []struct { | |||
| 50 | + | kind domain.RevisionCandidateKind | |||
| 51 | + | want string | |||
| 52 | + | }{ | |||
| 53 | + | {domain.RevisionCandidateHead, ""}, {domain.RevisionCandidateObject, ""}, | |||
| 54 | + | {domain.RevisionCandidateRef, "refs/heads/main"}, {domain.RevisionCandidateAbbrevID, ""}, | |||
| 55 | + | } { | |||
| 56 | + | assert.Equal(t, tc.want, (domain.RevisionCandidate{Kind: tc.kind, Name: "refs/heads/main"}).ResolvedRef()) | |||
| 57 | + | } | |||
| 58 | + | } | |||
M internal/ports/dto.go +15 -4
| @@ -249,7 +249,6 @@ type IssueDetail struct { | |||
| 249 | 249 | Comments []domain.IssueComment | |
| 250 | 250 | } | |
| 251 | 251 | ||
| 252 | - | // MergeRequestDetail is a merge request with its review data. | |
| 253 | 252 | // CompareResult describes source commits absent from target, newest first. | |
| 254 | 253 | type CompareResult struct { | |
| 255 | 254 | Ahead []domain.Commit | |
| @@ -264,11 +263,13 @@ type PushSummary struct { | |||
| 264 | 263 | Rewritten bool | |
| 265 | 264 | } | |
| 266 | 265 | ||
| 266 | + | // MergeRequestDetail is a merge request with its review data. | |
| 267 | 267 | type MergeRequestDetail struct { | |
| 268 | - | // NewCommits is the unread prefix of Commits; zero if the read head is absent. | |
| 269 | - | NewCommits int | |
| 270 | - | CanMarkRead bool | |
| 271 | 268 | MergeRequest domain.MergeRequest | |
| 269 | + | // NewCommits is the unread prefix of Commits; zero if the read head is absent. | |
| 270 | + | NewCommits int | |
| 271 | + | // HeadUnread reports whether the nonzero head differs from the shared read position. | |
| 272 | + | HeadUnread bool | |
| 272 | 273 | // Commits are the commits of the head that the target lacks, newest | |
| 273 | 274 | // first, at most MaxMergeRequestCommits. | |
| 274 | 275 | Commits []domain.Commit | |
| @@ -374,3 +375,13 @@ type InboxOverview struct { | |||
| 374 | 375 | // whatever the filter: they feed the filter itself. | |
| 375 | 376 | Repos []domain.RepoName | |
| 376 | 377 | } | |
| 378 | + | ||
| 379 | + | // History query limits. | |
| 380 | + | const ( | |
| 381 | + | // MaxMergeRequestCommits bounds the commits returned with a merge request. | |
| 382 | + | MaxMergeRequestCommits = 100 | |
| 383 | + | // MaxCompareCommits bounds a repository comparison. | |
| 384 | + | MaxCompareCommits = 500 | |
| 385 | + | // MaxPushSummaryCommits bounds the commits counted in a push summary. | |
| 386 | + | MaxPushSummaryCommits = 100 | |
| 387 | + | ) | |
| @@ -249,7 +249,6 @@ type IssueDetail struct { | |||||
| 249 | Comments []domain.IssueComment | 249 | Comments []domain.IssueComment | ||
| 250 | } | 250 | } | ||
| 251 | 251 | ||||
| 252 | - | // MergeRequestDetail is a merge request with its review data. | |||
| 253 | // CompareResult describes source commits absent from target, newest first. | 252 | // CompareResult describes source commits absent from target, newest first. | ||
| 254 | type CompareResult struct { | 253 | type CompareResult struct { | ||
| 255 | Ahead []domain.Commit | 254 | Ahead []domain.Commit | ||
| @@ -264,11 +263,13 @@ type PushSummary struct { | |||||
| 264 | Rewritten bool | 263 | Rewritten bool | ||
| 265 | } | 264 | } | ||
| 266 | 265 | ||||
| 266 | + | // MergeRequestDetail is a merge request with its review data. | |||
| 267 | type MergeRequestDetail struct { | 267 | type MergeRequestDetail struct { | ||
| 268 | - | // NewCommits is the unread prefix of Commits; zero if the read head is absent. | |||
| 269 | - | NewCommits int | |||
| 270 | - | CanMarkRead bool | |||
| 271 | MergeRequest domain.MergeRequest | 268 | MergeRequest domain.MergeRequest | ||
| 269 | + | // NewCommits is the unread prefix of Commits; zero if the read head is absent. | |||
| 270 | + | NewCommits int | |||
| 271 | + | // HeadUnread reports whether the nonzero head differs from the shared read position. | |||
| 272 | + | HeadUnread bool | |||
| 272 | // Commits are the commits of the head that the target lacks, newest | 273 | // Commits are the commits of the head that the target lacks, newest | ||
| 273 | // first, at most MaxMergeRequestCommits. | 274 | // first, at most MaxMergeRequestCommits. | ||
| 274 | Commits []domain.Commit | 275 | Commits []domain.Commit | ||
| @@ -374,3 +375,13 @@ type InboxOverview struct { | |||||
| 374 | // whatever the filter: they feed the filter itself. | 375 | // whatever the filter: they feed the filter itself. | ||
| 375 | Repos []domain.RepoName | 376 | Repos []domain.RepoName | ||
| 376 | } | 377 | } | ||
| 378 | + | ||||
| 379 | + | // History query limits. | |||
| 380 | + | const ( | |||
| 381 | + | // MaxMergeRequestCommits bounds the commits returned with a merge request. | |||
| 382 | + | MaxMergeRequestCommits = 100 | |||
| 383 | + | // MaxCompareCommits bounds a repository comparison. | |||
| 384 | + | MaxCompareCommits = 500 | |||
| 385 | + | // MaxPushSummaryCommits bounds the commits counted in a push summary. | |||
| 386 | + | MaxPushSummaryCommits = 100 | |||
| 387 | + | ) | |||
M internal/ports/services.go +2 -0
| @@ -109,7 +109,9 @@ type EventService interface { | |||
| 109 | 109 | // MergeRequestService is the merge request use cases. *usecase.MergeRequests | |
| 110 | 110 | // implements it. | |
| 111 | 111 | type MergeRequestService interface { | |
| 112 | + | // Compare queries repository history; it stays here because this service owns the merge-git port. | |
| 112 | 113 | Compare(ctx context.Context, p domain.Principal, repo domain.RepoName, target, source domain.ObjectID) (CompareResult, error) | |
| 114 | + | // PushSummary queries repository history; it stays here because this service owns the merge-git port. | |
| 113 | 115 | PushSummary(ctx context.Context, p domain.Principal, repo domain.RepoName, oldHead, newTip domain.ObjectID) (PushSummary, error) | |
| 114 | 116 | Create(ctx context.Context, p domain.Principal, name domain.RepoName, params CreateMergeRequestParams) (domain.MergeRequest, error) | |
| 115 | 117 | List(ctx context.Context, p domain.Principal, name domain.RepoName, params ListMergeRequestsParams) (MergeRequestPage, error) | |
| @@ -109,7 +109,9 @@ type EventService interface { | |||||
| 109 | // MergeRequestService is the merge request use cases. *usecase.MergeRequests | 109 | // MergeRequestService is the merge request use cases. *usecase.MergeRequests | ||
| 110 | // implements it. | 110 | // implements it. | ||
| 111 | type MergeRequestService interface { | 111 | type MergeRequestService interface { | ||
| 112 | + | // Compare queries repository history; it stays here because this service owns the merge-git port. | |||
| 112 | Compare(ctx context.Context, p domain.Principal, repo domain.RepoName, target, source domain.ObjectID) (CompareResult, error) | 113 | Compare(ctx context.Context, p domain.Principal, repo domain.RepoName, target, source domain.ObjectID) (CompareResult, error) | ||
| 114 | + | // PushSummary queries repository history; it stays here because this service owns the merge-git port. | |||
| 113 | PushSummary(ctx context.Context, p domain.Principal, repo domain.RepoName, oldHead, newTip domain.ObjectID) (PushSummary, error) | 115 | PushSummary(ctx context.Context, p domain.Principal, repo domain.RepoName, oldHead, newTip domain.ObjectID) (PushSummary, error) | ||
| 114 | Create(ctx context.Context, p domain.Principal, name domain.RepoName, params CreateMergeRequestParams) (domain.MergeRequest, error) | 116 | Create(ctx context.Context, p domain.Principal, name domain.RepoName, params CreateMergeRequestParams) (domain.MergeRequest, error) | ||
| 115 | List(ctx context.Context, p domain.Principal, name domain.RepoName, params ListMergeRequestsParams) (MergeRequestPage, error) | 117 | List(ctx context.Context, p domain.Principal, name domain.RepoName, params ListMergeRequestsParams) (MergeRequestPage, error) | ||
M internal/usecase/ci.go +1 -1
| @@ -106,7 +106,7 @@ type CIConfig struct { | |||
| 106 | 106 | // read when an action of another repository is resolved for the run. Nil | |
| 107 | 107 | // reads nothing for a token (fail closed). | |
| 108 | 108 | Tokens ports.TokenStore | |
| 109 | - | // OIDC signs the ID tokens of the jobs (CI-4); nil issues none: jobs get no | |
| 109 | + | // OIDC signs the ID tokens of the jobs (CI-4); nil or disabled issues none: jobs get no | |
| 110 | 110 | // token endpoint, and the provider is not served. | |
| 111 | 111 | OIDC *OIDC | |
| 112 | 112 | // Settings resolves the variables and secrets of a job (CI-3). Nil gives | |
| @@ -106,7 +106,7 @@ type CIConfig struct { | |||||
| 106 | // read when an action of another repository is resolved for the run. Nil | 106 | // read when an action of another repository is resolved for the run. Nil | ||
| 107 | // reads nothing for a token (fail closed). | 107 | // reads nothing for a token (fail closed). | ||
| 108 | Tokens ports.TokenStore | 108 | Tokens ports.TokenStore | ||
| 109 | - | // OIDC signs the ID tokens of the jobs (CI-4); nil issues none: jobs get no | 109 | + | // OIDC signs the ID tokens of the jobs (CI-4); nil or disabled issues none: jobs get no |
| 110 | // token endpoint, and the provider is not served. | 110 | // token endpoint, and the provider is not served. | ||
| 111 | OIDC *OIDC | 111 | OIDC *OIDC | ||
| 112 | // Settings resolves the variables and secrets of a job (CI-3). Nil gives | 112 | // Settings resolves the variables and secrets of a job (CI-3). Nil gives | ||
M internal/usecase/ci_oidc.go +0 -6
| @@ -52,17 +52,11 @@ func (c *CI) OIDCEnabled() bool { return c.cfg.OIDC.Enabled() } | |||
| 52 | 52 | ||
| 53 | 53 | // OIDCDiscovery returns the OpenID Provider configuration. | |
| 54 | 54 | func (c *CI) OIDCDiscovery() ([]byte, error) { | |
| 55 | - | if !c.cfg.OIDC.Enabled() { | |
| 56 | - | return nil, domain.ErrNotFound | |
| 57 | - | } | |
| 58 | 55 | return c.cfg.OIDC.Discovery() | |
| 59 | 56 | } | |
| 60 | 57 | ||
| 61 | 58 | // OIDCKeys returns the JWKS. | |
| 62 | 59 | func (c *CI) OIDCKeys() ([]byte, error) { | |
| 63 | - | if !c.cfg.OIDC.Enabled() { | |
| 64 | - | return nil, domain.ErrNotFound | |
| 65 | - | } | |
| 66 | 60 | return c.cfg.OIDC.JWKS() | |
| 67 | 61 | } | |
| 68 | 62 | ||
| @@ -52,17 +52,11 @@ func (c *CI) OIDCEnabled() bool { return c.cfg.OIDC.Enabled() } | |||||
| 52 | 52 | ||||
| 53 | // OIDCDiscovery returns the OpenID Provider configuration. | 53 | // OIDCDiscovery returns the OpenID Provider configuration. | ||
| 54 | func (c *CI) OIDCDiscovery() ([]byte, error) { | 54 | func (c *CI) OIDCDiscovery() ([]byte, error) { | ||
| 55 | - | if !c.cfg.OIDC.Enabled() { | |||
| 56 | - | return nil, domain.ErrNotFound | |||
| 57 | - | } | |||
| 58 | return c.cfg.OIDC.Discovery() | 55 | return c.cfg.OIDC.Discovery() | ||
| 59 | } | 56 | } | ||
| 60 | 57 | ||||
| 61 | // OIDCKeys returns the JWKS. | 58 | // OIDCKeys returns the JWKS. | ||
| 62 | func (c *CI) OIDCKeys() ([]byte, error) { | 59 | func (c *CI) OIDCKeys() ([]byte, error) { | ||
| 63 | - | if !c.cfg.OIDC.Enabled() { | |||
| 64 | - | return nil, domain.ErrNotFound | |||
| 65 | - | } | |||
| 66 | return c.cfg.OIDC.JWKS() | 60 | return c.cfg.OIDC.JWKS() | ||
| 67 | } | 61 | } | ||
| 68 | 62 | ||||
M internal/usecase/events.go +2 -2
| @@ -573,7 +573,7 @@ func (e *Events) deliverOwnerOnly(ctx context.Context, w *watch, name domain.Rep | |||
| 573 | 573 | case err != nil: | |
| 574 | 574 | return fmt.Errorf("check access to %q: %w", name, err) | |
| 575 | 575 | } | |
| 576 | - | if repo.Settings.Issues.OrDefault() == domain.IssuesOff { | |
| 576 | + | if !repo.Settings.IssuesEnabled() { | |
| 577 | 577 | return nil | |
| 578 | 578 | } | |
| 579 | 579 | return send(out) | |
| @@ -613,7 +613,7 @@ func (e *Events) deliverRepo(ctx context.Context, w *watch, name domain.RepoName | |||
| 613 | 613 | if !canRead(w.p, repo) { | |
| 614 | 614 | return nil | |
| 615 | 615 | } | |
| 616 | - | if isIssueEvent(be.ev) && repo.Settings.Issues.OrDefault() == domain.IssuesOff { | |
| 616 | + | if isIssueEvent(be.ev) && !repo.Settings.IssuesEnabled() { | |
| 617 | 617 | return nil // issues were turned off since: a replay must not show them | |
| 618 | 618 | } | |
| 619 | 619 | if withRepo { | |
| @@ -573,7 +573,7 @@ func (e *Events) deliverOwnerOnly(ctx context.Context, w *watch, name domain.Rep | |||||
| 573 | case err != nil: | 573 | case err != nil: | ||
| 574 | return fmt.Errorf("check access to %q: %w", name, err) | 574 | return fmt.Errorf("check access to %q: %w", name, err) | ||
| 575 | } | 575 | } | ||
| 576 | - | if repo.Settings.Issues.OrDefault() == domain.IssuesOff { | 576 | + | if !repo.Settings.IssuesEnabled() { |
| 577 | return nil | 577 | return nil | ||
| 578 | } | 578 | } | ||
| 579 | return send(out) | 579 | return send(out) | ||
| @@ -613,7 +613,7 @@ func (e *Events) deliverRepo(ctx context.Context, w *watch, name domain.RepoName | |||||
| 613 | if !canRead(w.p, repo) { | 613 | if !canRead(w.p, repo) { | ||
| 614 | return nil | 614 | return nil | ||
| 615 | } | 615 | } | ||
| 616 | - | if isIssueEvent(be.ev) && repo.Settings.Issues.OrDefault() == domain.IssuesOff { | 616 | + | if isIssueEvent(be.ev) && !repo.Settings.IssuesEnabled() { |
| 617 | return nil // issues were turned off since: a replay must not show them | 617 | return nil // issues were turned off since: a replay must not show them | ||
| 618 | } | 618 | } | ||
| 619 | if withRepo { | 619 | if withRepo { | ||
M internal/usecase/inbox.go +1 -1
| @@ -74,7 +74,7 @@ func (in *Inbox) Overview(ctx context.Context, p domain.Principal, filter domain | |||
| 74 | 74 | return fmt.Errorf("read repository %q: %w", name, err) | |
| 75 | 75 | default: | |
| 76 | 76 | exists[name] = true | |
| 77 | - | issuesOn[name] = repo.Settings.Issues.OrDefault() != domain.IssuesOff | |
| 77 | + | issuesOn[name] = repo.Settings.IssuesEnabled() | |
| 78 | 78 | } | |
| 79 | 79 | return nil | |
| 80 | 80 | } | |
| @@ -74,7 +74,7 @@ func (in *Inbox) Overview(ctx context.Context, p domain.Principal, filter domain | |||||
| 74 | return fmt.Errorf("read repository %q: %w", name, err) | 74 | return fmt.Errorf("read repository %q: %w", name, err) | ||
| 75 | default: | 75 | default: | ||
| 76 | exists[name] = true | 76 | exists[name] = true | ||
| 77 | - | issuesOn[name] = repo.Settings.Issues.OrDefault() != domain.IssuesOff | 77 | + | issuesOn[name] = repo.Settings.IssuesEnabled() |
| 78 | } | 78 | } | ||
| 79 | return nil | 79 | return nil | ||
| 80 | } | 80 | } | ||
M internal/usecase/issue_moderation.go +1 -1
| @@ -71,7 +71,7 @@ func (m *IssueModeration) Approve(ctx context.Context, p domain.Principal, repo | |||
| 71 | 71 | if err != nil { | |
| 72 | 72 | return domain.Issue{}, err | |
| 73 | 73 | } | |
| 74 | - | if r.Settings.Issues.OrDefault() == domain.IssuesOff { | |
| 74 | + | if !r.Settings.IssuesEnabled() { | |
| 75 | 75 | return domain.Issue{}, domain.ErrNotFound | |
| 76 | 76 | } | |
| 77 | 77 | now := m.clock.Now().UTC() | |
| @@ -71,7 +71,7 @@ func (m *IssueModeration) Approve(ctx context.Context, p domain.Principal, repo | |||||
| 71 | if err != nil { | 71 | if err != nil { | ||
| 72 | return domain.Issue{}, err | 72 | return domain.Issue{}, err | ||
| 73 | } | 73 | } | ||
| 74 | - | if r.Settings.Issues.OrDefault() == domain.IssuesOff { | 74 | + | if !r.Settings.IssuesEnabled() { |
| 75 | return domain.Issue{}, domain.ErrNotFound | 75 | return domain.Issue{}, domain.ErrNotFound | ||
| 76 | } | 76 | } | ||
| 77 | now := m.clock.Now().UTC() | 77 | now := m.clock.Now().UTC() | ||
M internal/usecase/issue_submissions.go +1 -1
| @@ -162,7 +162,7 @@ func (s *IssueSubmissions) openRepo(ctx context.Context, repo domain.RepoName) e | |||
| 162 | 162 | } | |
| 163 | 163 | return err | |
| 164 | 164 | } | |
| 165 | - | if !canRead(domain.Anonymous(), r) || r.Settings.Issues.OrDefault() != domain.IssuesPublic { | |
| 165 | + | if !canRead(domain.Anonymous(), r) || !r.Settings.AcceptsAnonymousIssues() { | |
| 166 | 166 | return domain.ErrNotFound | |
| 167 | 167 | } | |
| 168 | 168 | return nil | |
| @@ -162,7 +162,7 @@ func (s *IssueSubmissions) openRepo(ctx context.Context, repo domain.RepoName) e | |||||
| 162 | } | 162 | } | ||
| 163 | return err | 163 | return err | ||
| 164 | } | 164 | } | ||
| 165 | - | if !canRead(domain.Anonymous(), r) || r.Settings.Issues.OrDefault() != domain.IssuesPublic { | 165 | + | if !canRead(domain.Anonymous(), r) || !r.Settings.AcceptsAnonymousIssues() { |
| 166 | return domain.ErrNotFound | 166 | return domain.ErrNotFound | ||
| 167 | } | 167 | } | ||
| 168 | return nil | 168 | return nil | ||
M internal/usecase/issues.go +1 -1
| @@ -59,7 +59,7 @@ func (i *Issues) enabled(ctx context.Context, name domain.RepoName) (domain.Repo | |||
| 59 | 59 | if err != nil { | |
| 60 | 60 | return domain.Repo{}, err | |
| 61 | 61 | } | |
| 62 | - | if repo.Settings.Issues.OrDefault() == domain.IssuesOff { | |
| 62 | + | if !repo.Settings.IssuesEnabled() { | |
| 63 | 63 | return domain.Repo{}, domain.ErrNotFound | |
| 64 | 64 | } | |
| 65 | 65 | return repo, nil | |
| @@ -59,7 +59,7 @@ func (i *Issues) enabled(ctx context.Context, name domain.RepoName) (domain.Repo | |||||
| 59 | if err != nil { | 59 | if err != nil { | ||
| 60 | return domain.Repo{}, err | 60 | return domain.Repo{}, err | ||
| 61 | } | 61 | } | ||
| 62 | - | if repo.Settings.Issues.OrDefault() == domain.IssuesOff { | 62 | + | if !repo.Settings.IssuesEnabled() { |
| 63 | return domain.Repo{}, domain.ErrNotFound | 63 | return domain.Repo{}, domain.ErrNotFound | ||
| 64 | } | 64 | } | ||
| 65 | return repo, nil | 65 | return repo, nil | ||
M internal/usecase/mergerequests.go +7 -10
| @@ -21,7 +21,7 @@ import ( | |||
| 21 | 21 | ) | |
| 22 | 22 | ||
| 23 | 23 | // MaxMergeRequestCommits is the most commits GetMergeRequest returns. | |
| 24 | - | const MaxMergeRequestCommits = 100 | |
| 24 | + | const MaxMergeRequestCommits = ports.MaxMergeRequestCommits | |
| 25 | 25 | ||
| 26 | 26 | // maxWriteRetries bounds the retries of a write that loses a race with another | |
| 27 | 27 | // write of the same merge request. Each lost race means another writer won, so | |
| @@ -546,7 +546,7 @@ func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain | |||
| 546 | 546 | if d.LastReadID, err = m.store.GetRead(ctx, name, number); err != nil { | |
| 547 | 547 | return ports.MergeRequestDetail{}, err | |
| 548 | 548 | } | |
| 549 | - | d.CanMarkRead = !mr.HeadID.IsZero() && mr.HeadID != d.LastReadID | |
| 549 | + | d.HeadUnread = !mr.HeadID.IsZero() && mr.HeadID != d.LastReadID | |
| 550 | 550 | ||
| 551 | 551 | exclude, base, err := m.rangeOf(ctx, mr) | |
| 552 | 552 | if err != nil { | |
| @@ -1055,10 +1055,7 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma | |||
| 1055 | 1055 | } | |
| 1056 | 1056 | return domain.MergeRequest{}, fmt.Errorf("get repository %q: %w", name, err) | |
| 1057 | 1057 | } | |
| 1058 | - | settings := repo.Settings | |
| 1059 | - | if settings.IsZero() { | |
| 1060 | - | settings = domain.DefaultRepoSettings() | |
| 1061 | - | } | |
| 1058 | + | settings := repo.EffectiveSettings() | |
| 1062 | 1059 | if !settings.Allows(mode) { | |
| 1063 | 1060 | return domain.MergeRequest{}, fmt.Errorf("%w: the %s merge mode is not allowed in %s", domain.ErrPreconditionFailed, mode, name) | |
| 1064 | 1061 | } | |
| @@ -1448,7 +1445,7 @@ func (m *MergeRequests) replyRoot(ctx context.Context, name domain.RepoName, num | |||
| 1448 | 1445 | return root, nil | |
| 1449 | 1446 | } | |
| 1450 | 1447 | ||
| 1451 | - | // Compare lists at most 500 source commits absent from target. Unlike bounded | |
| 1448 | + | // Compare lists at most ports.MaxCompareCommits source commits absent from target. Unlike bounded | |
| 1452 | 1449 | // history intersection, ancestry remains accurate even for deep histories. | |
| 1453 | 1450 | func (m *MergeRequests) Compare(ctx context.Context, p domain.Principal, repo domain.RepoName, target, source domain.ObjectID) (ports.CompareResult, error) { | |
| 1454 | 1451 | if _, err := m.readable(ctx, p, repo); err != nil { | |
| @@ -1461,7 +1458,7 @@ func (m *MergeRequests) Compare(ctx context.Context, p domain.Principal, repo do | |||
| 1461 | 1458 | if err != nil { | |
| 1462 | 1459 | return ports.CompareResult{}, err | |
| 1463 | 1460 | } | |
| 1464 | - | commits, more, err := m.git.ListRange(ctx, repo, target, source, 500) | |
| 1461 | + | commits, more, err := m.git.ListRange(ctx, repo, target, source, ports.MaxCompareCommits) | |
| 1465 | 1462 | if err != nil { | |
| 1466 | 1463 | return ports.CompareResult{}, err | |
| 1467 | 1464 | } | |
| @@ -1470,7 +1467,7 @@ func (m *MergeRequests) Compare(ctx context.Context, p domain.Principal, repo do | |||
| 1470 | 1467 | } | |
| 1471 | 1468 | ||
| 1472 | 1469 | // PushSummary detects rewrites by ancestry, not log ordering, and counts at | |
| 1473 | - | // most 100 new commits. More distinguishes a longer range from a rewrite. | |
| 1470 | + | // most ports.MaxPushSummaryCommits new commits. More distinguishes a longer range from a rewrite. | |
| 1474 | 1471 | func (m *MergeRequests) PushSummary(ctx context.Context, p domain.Principal, repo domain.RepoName, oldHead, newTip domain.ObjectID) (ports.PushSummary, error) { | |
| 1475 | 1472 | if _, err := m.readable(ctx, p, repo); err != nil { | |
| 1476 | 1473 | return ports.PushSummary{}, err | |
| @@ -1485,7 +1482,7 @@ func (m *MergeRequests) PushSummary(ctx context.Context, p domain.Principal, rep | |||
| 1485 | 1482 | if !ancestor { | |
| 1486 | 1483 | return ports.PushSummary{Rewritten: true}, nil | |
| 1487 | 1484 | } | |
| 1488 | - | commits, more, err := m.git.ListRange(ctx, repo, oldHead, newTip, 100) | |
| 1485 | + | commits, more, err := m.git.ListRange(ctx, repo, oldHead, newTip, ports.MaxPushSummaryCommits) | |
| 1489 | 1486 | if err != nil { | |
| 1490 | 1487 | return ports.PushSummary{}, err | |
| 1491 | 1488 | } | |
| @@ -21,7 +21,7 @@ import ( | |||||
| 21 | ) | 21 | ) | ||
| 22 | 22 | ||||
| 23 | // MaxMergeRequestCommits is the most commits GetMergeRequest returns. | 23 | // MaxMergeRequestCommits is the most commits GetMergeRequest returns. | ||
| 24 | - | const MaxMergeRequestCommits = 100 | 24 | + | const MaxMergeRequestCommits = ports.MaxMergeRequestCommits |
| 25 | 25 | ||||
| 26 | // maxWriteRetries bounds the retries of a write that loses a race with another | 26 | // maxWriteRetries bounds the retries of a write that loses a race with another | ||
| 27 | // write of the same merge request. Each lost race means another writer won, so | 27 | // write of the same merge request. Each lost race means another writer won, so | ||
| @@ -546,7 +546,7 @@ func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain | |||||
| 546 | if d.LastReadID, err = m.store.GetRead(ctx, name, number); err != nil { | 546 | if d.LastReadID, err = m.store.GetRead(ctx, name, number); err != nil { | ||
| 547 | return ports.MergeRequestDetail{}, err | 547 | return ports.MergeRequestDetail{}, err | ||
| 548 | } | 548 | } | ||
| 549 | - | d.CanMarkRead = !mr.HeadID.IsZero() && mr.HeadID != d.LastReadID | 549 | + | d.HeadUnread = !mr.HeadID.IsZero() && mr.HeadID != d.LastReadID |
| 550 | 550 | ||||
| 551 | exclude, base, err := m.rangeOf(ctx, mr) | 551 | exclude, base, err := m.rangeOf(ctx, mr) | ||
| 552 | if err != nil { | 552 | if err != nil { | ||
| @@ -1055,10 +1055,7 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma | |||||
| 1055 | } | 1055 | } | ||
| 1056 | return domain.MergeRequest{}, fmt.Errorf("get repository %q: %w", name, err) | 1056 | return domain.MergeRequest{}, fmt.Errorf("get repository %q: %w", name, err) | ||
| 1057 | } | 1057 | } | ||
| 1058 | - | settings := repo.Settings | 1058 | + | settings := repo.EffectiveSettings() |
| 1059 | - | if settings.IsZero() { | |||
| 1060 | - | settings = domain.DefaultRepoSettings() | |||
| 1061 | - | } | |||
| 1062 | if !settings.Allows(mode) { | 1059 | if !settings.Allows(mode) { | ||
| 1063 | return domain.MergeRequest{}, fmt.Errorf("%w: the %s merge mode is not allowed in %s", domain.ErrPreconditionFailed, mode, name) | 1060 | return domain.MergeRequest{}, fmt.Errorf("%w: the %s merge mode is not allowed in %s", domain.ErrPreconditionFailed, mode, name) | ||
| 1064 | } | 1061 | } | ||
| @@ -1448,7 +1445,7 @@ func (m *MergeRequests) replyRoot(ctx context.Context, name domain.RepoName, num | |||||
| 1448 | return root, nil | 1445 | return root, nil | ||
| 1449 | } | 1446 | } | ||
| 1450 | 1447 | ||||
| 1451 | - | // Compare lists at most 500 source commits absent from target. Unlike bounded | 1448 | + | // Compare lists at most ports.MaxCompareCommits source commits absent from target. Unlike bounded |
| 1452 | // history intersection, ancestry remains accurate even for deep histories. | 1449 | // history intersection, ancestry remains accurate even for deep histories. | ||
| 1453 | func (m *MergeRequests) Compare(ctx context.Context, p domain.Principal, repo domain.RepoName, target, source domain.ObjectID) (ports.CompareResult, error) { | 1450 | func (m *MergeRequests) Compare(ctx context.Context, p domain.Principal, repo domain.RepoName, target, source domain.ObjectID) (ports.CompareResult, error) { | ||
| 1454 | if _, err := m.readable(ctx, p, repo); err != nil { | 1451 | if _, err := m.readable(ctx, p, repo); err != nil { | ||
| @@ -1461,7 +1458,7 @@ func (m *MergeRequests) Compare(ctx context.Context, p domain.Principal, repo do | |||||
| 1461 | if err != nil { | 1458 | if err != nil { | ||
| 1462 | return ports.CompareResult{}, err | 1459 | return ports.CompareResult{}, err | ||
| 1463 | } | 1460 | } | ||
| 1464 | - | commits, more, err := m.git.ListRange(ctx, repo, target, source, 500) | 1461 | + | commits, more, err := m.git.ListRange(ctx, repo, target, source, ports.MaxCompareCommits) |
| 1465 | if err != nil { | 1462 | if err != nil { | ||
| 1466 | return ports.CompareResult{}, err | 1463 | return ports.CompareResult{}, err | ||
| 1467 | } | 1464 | } | ||
| @@ -1470,7 +1467,7 @@ func (m *MergeRequests) Compare(ctx context.Context, p domain.Principal, repo do | |||||
| 1470 | } | 1467 | } | ||
| 1471 | 1468 | ||||
| 1472 | // PushSummary detects rewrites by ancestry, not log ordering, and counts at | 1469 | // PushSummary detects rewrites by ancestry, not log ordering, and counts at | ||
| 1473 | - | // most 100 new commits. More distinguishes a longer range from a rewrite. | 1470 | + | // most ports.MaxPushSummaryCommits new commits. More distinguishes a longer range from a rewrite. |
| 1474 | func (m *MergeRequests) PushSummary(ctx context.Context, p domain.Principal, repo domain.RepoName, oldHead, newTip domain.ObjectID) (ports.PushSummary, error) { | 1471 | func (m *MergeRequests) PushSummary(ctx context.Context, p domain.Principal, repo domain.RepoName, oldHead, newTip domain.ObjectID) (ports.PushSummary, error) { | ||
| 1475 | if _, err := m.readable(ctx, p, repo); err != nil { | 1472 | if _, err := m.readable(ctx, p, repo); err != nil { | ||
| 1476 | return ports.PushSummary{}, err | 1473 | return ports.PushSummary{}, err | ||
| @@ -1485,7 +1482,7 @@ func (m *MergeRequests) PushSummary(ctx context.Context, p domain.Principal, rep | |||||
| 1485 | if !ancestor { | 1482 | if !ancestor { | ||
| 1486 | return ports.PushSummary{Rewritten: true}, nil | 1483 | return ports.PushSummary{Rewritten: true}, nil | ||
| 1487 | } | 1484 | } | ||
| 1488 | - | commits, more, err := m.git.ListRange(ctx, repo, oldHead, newTip, 100) | 1485 | + | commits, more, err := m.git.ListRange(ctx, repo, oldHead, newTip, ports.MaxPushSummaryCommits) |
| 1489 | if err != nil { | 1486 | if err != nil { | ||
| 1490 | return ports.PushSummary{}, err | 1487 | return ports.PushSummary{}, err | ||
| 1491 | } | 1488 | } | ||
M internal/usecase/mergerequests_test.go +61 -20
| @@ -234,36 +234,77 @@ func TestAddNoteValidation(t *testing.T) { | |||
| 234 | 234 | } | |
| 235 | 235 | } | |
| 236 | 236 | ||
| 237 | - | func TestHistorySummaries(t *testing.T) { | |
| 237 | + | func TestCompare(t *testing.T) { | |
| 238 | 238 | for _, tc := range []struct { | |
| 239 | - | name string | |
| 240 | - | ancestor, more bool | |
| 239 | + | name string | |
| 240 | + | ancestor, more, zero bool | |
| 241 | + | ancestorErr, rangeErr error | |
| 241 | 242 | }{ | |
| 242 | - | {"linear", true, false}, {"bounded", true, true}, {"rewritten", false, false}, | |
| 243 | + | {name: "linear", ancestor: true}, {name: "bounded", ancestor: true, more: true}, | |
| 244 | + | {name: "unrelated histories"}, {name: "zero target forwards ancestry error", zero: true, ancestorErr: domain.ErrInvalid}, | |
| 245 | + | {name: "ancestor error", ancestorErr: domain.ErrInvalid}, | |
| 246 | + | {name: "range error", ancestor: true, rangeErr: domain.ErrInvalid}, | |
| 243 | 247 | } { | |
| 244 | 248 | t.Run(tc.name, func(t *testing.T) { | |
| 245 | 249 | u := newMRUnit(t) | |
| 246 | - | old := someID(t) | |
| 247 | - | tip := oidFrom(t, "2222222222222222222222222222222222222222") | |
| 248 | - | u.meta.EXPECT().Get(mock.Anything, u.repo.Name).Return(u.repo, nil).Twice() | |
| 249 | - | u.git.EXPECT().IsAncestor(mock.Anything, u.repo.Name, old, tip).Return(tc.ancestor, nil).Twice() | |
| 250 | + | old, tip := someID(t), oidFrom(t, "2222222222222222222222222222222222222222") | |
| 251 | + | if tc.zero { | |
| 252 | + | old = domain.ObjectID{} | |
| 253 | + | } | |
| 254 | + | u.meta.EXPECT().Get(mock.Anything, u.repo.Name).Return(u.repo, nil) | |
| 255 | + | u.git.EXPECT().IsAncestor(mock.Anything, u.repo.Name, old, tip).Return(tc.ancestor, tc.ancestorErr) | |
| 250 | 256 | commits := []domain.Commit{{ID: tip}} | |
| 251 | - | u.git.EXPECT().ListRange(mock.Anything, u.repo.Name, old, tip, 500).Return(commits, tc.more, nil) | |
| 252 | - | comparison, err := u.mrs.Compare(context.Background(), domain.Root(), u.repo.Name, old, tip) | |
| 257 | + | if tc.ancestorErr == nil { | |
| 258 | + | u.git.EXPECT().ListRange(mock.Anything, u.repo.Name, old, tip, ports.MaxCompareCommits).Return(commits, tc.more, tc.rangeErr) | |
| 259 | + | } | |
| 260 | + | got, err := u.mrs.Compare(context.Background(), domain.Root(), u.repo.Name, old, tip) | |
| 261 | + | if tc.ancestorErr != nil { | |
| 262 | + | require.ErrorIs(t, err, tc.ancestorErr) | |
| 263 | + | return | |
| 264 | + | } | |
| 265 | + | if tc.rangeErr != nil { | |
| 266 | + | require.ErrorIs(t, err, tc.rangeErr) | |
| 267 | + | return | |
| 268 | + | } | |
| 253 | 269 | require.NoError(t, err) | |
| 254 | - | assert.Equal(t, !tc.ancestor, comparison.Behind) | |
| 255 | - | assert.Equal(t, !tc.more, comparison.Exact) | |
| 256 | - | assert.Equal(t, commits, comparison.Ahead) | |
| 257 | - | if tc.ancestor { | |
| 258 | - | u.git.EXPECT().ListRange(mock.Anything, u.repo.Name, old, tip, 100).Return(commits, tc.more, nil) | |
| 270 | + | assert.Equal(t, ports.CompareResult{Ahead: commits, Behind: !tc.ancestor, Exact: !tc.more}, got) | |
| 271 | + | }) | |
| 272 | + | } | |
| 273 | + | } | |
| 274 | + | ||
| 275 | + | func TestPushSummary(t *testing.T) { | |
| 276 | + | for _, tc := range []struct { | |
| 277 | + | name string | |
| 278 | + | ancestor, more bool | |
| 279 | + | ancestorErr, rangeErr error | |
| 280 | + | }{ | |
| 281 | + | {name: "linear", ancestor: true}, {name: "bounded", ancestor: true, more: true}, | |
| 282 | + | {name: "unrelated histories"}, {name: "ancestor error", ancestorErr: domain.ErrInvalid}, | |
| 283 | + | {name: "range error", ancestor: true, rangeErr: domain.ErrInvalid}, | |
| 284 | + | } { | |
| 285 | + | t.Run(tc.name, func(t *testing.T) { | |
| 286 | + | u := newMRUnit(t) | |
| 287 | + | old, tip := someID(t), oidFrom(t, "2222222222222222222222222222222222222222") | |
| 288 | + | u.meta.EXPECT().Get(mock.Anything, u.repo.Name).Return(u.repo, nil) | |
| 289 | + | u.git.EXPECT().IsAncestor(mock.Anything, u.repo.Name, old, tip).Return(tc.ancestor, tc.ancestorErr) | |
| 290 | + | if tc.ancestor && tc.ancestorErr == nil { | |
| 291 | + | u.git.EXPECT().ListRange(mock.Anything, u.repo.Name, old, tip, ports.MaxPushSummaryCommits).Return([]domain.Commit{{ID: tip}}, tc.more, tc.rangeErr) | |
| 292 | + | } | |
| 293 | + | got, err := u.mrs.PushSummary(context.Background(), domain.Root(), u.repo.Name, old, tip) | |
| 294 | + | if tc.ancestorErr != nil { | |
| 295 | + | require.ErrorIs(t, err, tc.ancestorErr) | |
| 296 | + | return | |
| 297 | + | } | |
| 298 | + | if tc.rangeErr != nil { | |
| 299 | + | require.ErrorIs(t, err, tc.rangeErr) | |
| 300 | + | return | |
| 259 | 301 | } | |
| 260 | - | summary, err := u.mrs.PushSummary(context.Background(), domain.Root(), u.repo.Name, old, tip) | |
| 261 | 302 | require.NoError(t, err) | |
| 262 | - | assert.Equal(t, !tc.ancestor, summary.Rewritten) | |
| 303 | + | want := ports.PushSummary{Rewritten: !tc.ancestor} | |
| 263 | 304 | if tc.ancestor { | |
| 264 | - | assert.Equal(t, 1, summary.Count) | |
| 265 | - | assert.Equal(t, tc.more, summary.More) | |
| 305 | + | want.Count, want.More = 1, tc.more | |
| 266 | 306 | } | |
| 307 | + | assert.Equal(t, want, got) | |
| 267 | 308 | }) | |
| 268 | 309 | } | |
| 269 | 310 | } | |
| @@ -301,7 +342,7 @@ func TestReadTrackingSharedPosition(t *testing.T) { | |||
| 301 | 342 | require.NoError(t, err) | |
| 302 | 343 | expected := 0 | |
| 303 | 344 | assert.Equal(t, read, detail.LastReadID) | |
| 304 | - | assert.Equal(t, read != head, detail.CanMarkRead) | |
| 345 | + | assert.Equal(t, read != head, detail.HeadUnread) | |
| 305 | 346 | if tc.readIndex > 0 { | |
| 306 | 347 | expected = tc.readIndex | |
| 307 | 348 | } | |
| @@ -234,36 +234,77 @@ func TestAddNoteValidation(t *testing.T) { | |||||
| 234 | } | 234 | } | ||
| 235 | } | 235 | } | ||
| 236 | 236 | ||||
| 237 | - | func TestHistorySummaries(t *testing.T) { | 237 | + | func TestCompare(t *testing.T) { |
| 238 | for _, tc := range []struct { | 238 | for _, tc := range []struct { | ||
| 239 | - | name string | 239 | + | name string |
| 240 | - | ancestor, more bool | 240 | + | ancestor, more, zero bool |
| 241 | + | ancestorErr, rangeErr error | |||
| 241 | }{ | 242 | }{ | ||
| 242 | - | {"linear", true, false}, {"bounded", true, true}, {"rewritten", false, false}, | 243 | + | {name: "linear", ancestor: true}, {name: "bounded", ancestor: true, more: true}, |
| 244 | + | {name: "unrelated histories"}, {name: "zero target forwards ancestry error", zero: true, ancestorErr: domain.ErrInvalid}, | |||
| 245 | + | {name: "ancestor error", ancestorErr: domain.ErrInvalid}, | |||
| 246 | + | {name: "range error", ancestor: true, rangeErr: domain.ErrInvalid}, | |||
| 243 | } { | 247 | } { | ||
| 244 | t.Run(tc.name, func(t *testing.T) { | 248 | t.Run(tc.name, func(t *testing.T) { | ||
| 245 | u := newMRUnit(t) | 249 | u := newMRUnit(t) | ||
| 246 | - | old := someID(t) | 250 | + | old, tip := someID(t), oidFrom(t, "2222222222222222222222222222222222222222") |
| 247 | - | tip := oidFrom(t, "2222222222222222222222222222222222222222") | 251 | + | if tc.zero { |
| 248 | - | u.meta.EXPECT().Get(mock.Anything, u.repo.Name).Return(u.repo, nil).Twice() | 252 | + | old = domain.ObjectID{} |
| 249 | - | u.git.EXPECT().IsAncestor(mock.Anything, u.repo.Name, old, tip).Return(tc.ancestor, nil).Twice() | 253 | + | } |
| 254 | + | u.meta.EXPECT().Get(mock.Anything, u.repo.Name).Return(u.repo, nil) | |||
| 255 | + | u.git.EXPECT().IsAncestor(mock.Anything, u.repo.Name, old, tip).Return(tc.ancestor, tc.ancestorErr) | |||
| 250 | commits := []domain.Commit{{ID: tip}} | 256 | commits := []domain.Commit{{ID: tip}} | ||
| 251 | - | u.git.EXPECT().ListRange(mock.Anything, u.repo.Name, old, tip, 500).Return(commits, tc.more, nil) | 257 | + | if tc.ancestorErr == nil { |
| 252 | - | comparison, err := u.mrs.Compare(context.Background(), domain.Root(), u.repo.Name, old, tip) | 258 | + | u.git.EXPECT().ListRange(mock.Anything, u.repo.Name, old, tip, ports.MaxCompareCommits).Return(commits, tc.more, tc.rangeErr) |
| 259 | + | } | |||
| 260 | + | got, err := u.mrs.Compare(context.Background(), domain.Root(), u.repo.Name, old, tip) | |||
| 261 | + | if tc.ancestorErr != nil { | |||
| 262 | + | require.ErrorIs(t, err, tc.ancestorErr) | |||
| 263 | + | return | |||
| 264 | + | } | |||
| 265 | + | if tc.rangeErr != nil { | |||
| 266 | + | require.ErrorIs(t, err, tc.rangeErr) | |||
| 267 | + | return | |||
| 268 | + | } | |||
| 253 | require.NoError(t, err) | 269 | require.NoError(t, err) | ||
| 254 | - | assert.Equal(t, !tc.ancestor, comparison.Behind) | 270 | + | assert.Equal(t, ports.CompareResult{Ahead: commits, Behind: !tc.ancestor, Exact: !tc.more}, got) |
| 255 | - | assert.Equal(t, !tc.more, comparison.Exact) | 271 | + | }) |
| 256 | - | assert.Equal(t, commits, comparison.Ahead) | 272 | + | } |
| 257 | - | if tc.ancestor { | 273 | + | } |
| 258 | - | u.git.EXPECT().ListRange(mock.Anything, u.repo.Name, old, tip, 100).Return(commits, tc.more, nil) | 274 | + | |
| 275 | + | func TestPushSummary(t *testing.T) { | |||
| 276 | + | for _, tc := range []struct { | |||
| 277 | + | name string | |||
| 278 | + | ancestor, more bool | |||
| 279 | + | ancestorErr, rangeErr error | |||
| 280 | + | }{ | |||
| 281 | + | {name: "linear", ancestor: true}, {name: "bounded", ancestor: true, more: true}, | |||
| 282 | + | {name: "unrelated histories"}, {name: "ancestor error", ancestorErr: domain.ErrInvalid}, | |||
| 283 | + | {name: "range error", ancestor: true, rangeErr: domain.ErrInvalid}, | |||
| 284 | + | } { | |||
| 285 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 286 | + | u := newMRUnit(t) | |||
| 287 | + | old, tip := someID(t), oidFrom(t, "2222222222222222222222222222222222222222") | |||
| 288 | + | u.meta.EXPECT().Get(mock.Anything, u.repo.Name).Return(u.repo, nil) | |||
| 289 | + | u.git.EXPECT().IsAncestor(mock.Anything, u.repo.Name, old, tip).Return(tc.ancestor, tc.ancestorErr) | |||
| 290 | + | if tc.ancestor && tc.ancestorErr == nil { | |||
| 291 | + | u.git.EXPECT().ListRange(mock.Anything, u.repo.Name, old, tip, ports.MaxPushSummaryCommits).Return([]domain.Commit{{ID: tip}}, tc.more, tc.rangeErr) | |||
| 292 | + | } | |||
| 293 | + | got, err := u.mrs.PushSummary(context.Background(), domain.Root(), u.repo.Name, old, tip) | |||
| 294 | + | if tc.ancestorErr != nil { | |||
| 295 | + | require.ErrorIs(t, err, tc.ancestorErr) | |||
| 296 | + | return | |||
| 297 | + | } | |||
| 298 | + | if tc.rangeErr != nil { | |||
| 299 | + | require.ErrorIs(t, err, tc.rangeErr) | |||
| 300 | + | return | |||
| 259 | } | 301 | } | ||
| 260 | - | summary, err := u.mrs.PushSummary(context.Background(), domain.Root(), u.repo.Name, old, tip) | |||
| 261 | require.NoError(t, err) | 302 | require.NoError(t, err) | ||
| 262 | - | assert.Equal(t, !tc.ancestor, summary.Rewritten) | 303 | + | want := ports.PushSummary{Rewritten: !tc.ancestor} |
| 263 | if tc.ancestor { | 304 | if tc.ancestor { | ||
| 264 | - | assert.Equal(t, 1, summary.Count) | 305 | + | want.Count, want.More = 1, tc.more |
| 265 | - | assert.Equal(t, tc.more, summary.More) | |||
| 266 | } | 306 | } | ||
| 307 | + | assert.Equal(t, want, got) | |||
| 267 | }) | 308 | }) | ||
| 268 | } | 309 | } | ||
| 269 | } | 310 | } | ||
| @@ -301,7 +342,7 @@ func TestReadTrackingSharedPosition(t *testing.T) { | |||||
| 301 | require.NoError(t, err) | 342 | require.NoError(t, err) | ||
| 302 | expected := 0 | 343 | expected := 0 | ||
| 303 | assert.Equal(t, read, detail.LastReadID) | 344 | assert.Equal(t, read, detail.LastReadID) | ||
| 304 | - | assert.Equal(t, read != head, detail.CanMarkRead) | 345 | + | assert.Equal(t, read != head, detail.HeadUnread) |
| 305 | if tc.readIndex > 0 { | 346 | if tc.readIndex > 0 { | ||
| 306 | expected = tc.readIndex | 347 | expected = tc.readIndex | ||
| 307 | } | 348 | } | ||
M internal/usecase/oidc_test.go +15 -0
| @@ -366,3 +366,18 @@ func TestDisabledOIDCRotationAuthorization(t *testing.T) { | |||
| 366 | 366 | _, err = o.JWKS() | |
| 367 | 367 | assert.ErrorIs(t, err, domain.ErrNotFound) | |
| 368 | 368 | } | |
| 369 | + | ||
| 370 | + | func TestDisabledOIDCPublicEndpoints(t *testing.T) { | |
| 371 | + | for _, provider := range []*usecase.OIDC{nil, {}} { | |
| 372 | + | _, err := provider.Discovery() | |
| 373 | + | require.ErrorIs(t, err, domain.ErrNotFound) | |
| 374 | + | _, err = provider.JWKS() | |
| 375 | + | require.ErrorIs(t, err, domain.ErrNotFound) | |
| 376 | + | ci := usecase.NewCI(context.Background(), usecase.CIConfig{Owner: "owner", Clock: fixedClock{}, OIDC: provider}) | |
| 377 | + | _, err = ci.OIDCDiscovery() | |
| 378 | + | require.ErrorIs(t, err, domain.ErrNotFound) | |
| 379 | + | _, err = ci.OIDCKeys() | |
| 380 | + | require.ErrorIs(t, err, domain.ErrNotFound) | |
| 381 | + | ci.Close(context.Background()) | |
| 382 | + | } | |
| 383 | + | } | |
| @@ -366,3 +366,18 @@ func TestDisabledOIDCRotationAuthorization(t *testing.T) { | |||||
| 366 | _, err = o.JWKS() | 366 | _, err = o.JWKS() | ||
| 367 | assert.ErrorIs(t, err, domain.ErrNotFound) | 367 | assert.ErrorIs(t, err, domain.ErrNotFound) | ||
| 368 | } | 368 | } | ||
| 369 | + | ||||
| 370 | + | func TestDisabledOIDCPublicEndpoints(t *testing.T) { | |||
| 371 | + | for _, provider := range []*usecase.OIDC{nil, {}} { | |||
| 372 | + | _, err := provider.Discovery() | |||
| 373 | + | require.ErrorIs(t, err, domain.ErrNotFound) | |||
| 374 | + | _, err = provider.JWKS() | |||
| 375 | + | require.ErrorIs(t, err, domain.ErrNotFound) | |||
| 376 | + | ci := usecase.NewCI(context.Background(), usecase.CIConfig{Owner: "owner", Clock: fixedClock{}, OIDC: provider}) | |||
| 377 | + | _, err = ci.OIDCDiscovery() | |||
| 378 | + | require.ErrorIs(t, err, domain.ErrNotFound) | |||
| 379 | + | _, err = ci.OIDCKeys() | |||
| 380 | + | require.ErrorIs(t, err, domain.ErrNotFound) | |||
| 381 | + | ci.Close(context.Background()) | |||
| 382 | + | } | |||
| 383 | + | } | |||
M internal/usecase/pushes_test.go +3 -3
| @@ -460,10 +460,10 @@ func (g *fakePushTrees) PushSubtrees(_ context.Context, _ domain.RepoName, ups [ | |||
| 460 | 460 | if g.data != nil { | |
| 461 | 461 | return *g.data, g.err | |
| 462 | 462 | } | |
| 463 | - | t := domain.PushSubtrees{Ends: map[string]domain.PushedEnd{}, Subtrees: map[string]domain.Subtree{}} | |
| 463 | + | t := domain.PushSubtrees{Ends: map[string]domain.PushObjectType{}, Subtrees: map[string]domain.Subtree{}} | |
| 464 | 464 | for _, up := range ups { | |
| 465 | - | t.Ends[up.Old] = domain.PushedEnd{Type: domain.ObjectCommit} | |
| 466 | - | t.Ends[up.New] = domain.PushedEnd{Type: domain.ObjectCommit} | |
| 465 | + | t.Ends[up.Old] = domain.PushObjectCommit | |
| 466 | + | t.Ends[up.New] = domain.PushObjectCommit | |
| 467 | 467 | if g.path != "" || g.change { | |
| 468 | 468 | id, _ := domain.ParseObjectID(up.New) | |
| 469 | 469 | t.Subtrees[up.New] = domain.Subtree{Kind: domain.EntryDirectory, ID: id} | |
| @@ -460,10 +460,10 @@ func (g *fakePushTrees) PushSubtrees(_ context.Context, _ domain.RepoName, ups [ | |||||
| 460 | if g.data != nil { | 460 | if g.data != nil { | ||
| 461 | return *g.data, g.err | 461 | return *g.data, g.err | ||
| 462 | } | 462 | } | ||
| 463 | - | t := domain.PushSubtrees{Ends: map[string]domain.PushedEnd{}, Subtrees: map[string]domain.Subtree{}} | 463 | + | t := domain.PushSubtrees{Ends: map[string]domain.PushObjectType{}, Subtrees: map[string]domain.Subtree{}} |
| 464 | for _, up := range ups { | 464 | for _, up := range ups { | ||
| 465 | - | t.Ends[up.Old] = domain.PushedEnd{Type: domain.ObjectCommit} | 465 | + | t.Ends[up.Old] = domain.PushObjectCommit |
| 466 | - | t.Ends[up.New] = domain.PushedEnd{Type: domain.ObjectCommit} | 466 | + | t.Ends[up.New] = domain.PushObjectCommit |
| 467 | if g.path != "" || g.change { | 467 | if g.path != "" || g.change { | ||
| 468 | id, _ := domain.ParseObjectID(up.New) | 468 | id, _ := domain.ParseObjectID(up.New) | ||
| 469 | t.Subtrees[up.New] = domain.Subtree{Kind: domain.EntryDirectory, ID: id} | 469 | t.Subtrees[up.New] = domain.Subtree{Kind: domain.EntryDirectory, ID: id} | ||
M internal/usecase/repos_write.go +1 -3
| @@ -165,9 +165,7 @@ func (r *Repos) updateMeta(ctx context.Context, name domain.RepoName, params por | |||
| 165 | 165 | if params.Visibility != nil { | |
| 166 | 166 | repo.Visibility = *params.Visibility | |
| 167 | 167 | } | |
| 168 | - | if repo.Settings.IsZero() { | |
| 169 | - | repo.Settings = domain.DefaultRepoSettings() | |
| 170 | - | } | |
| 168 | + | repo.Settings = repo.EffectiveSettings() | |
| 171 | 169 | if params.AllowedMergeModes != nil { | |
| 172 | 170 | repo.Settings.AllowedMergeModes = *params.AllowedMergeModes | |
| 173 | 171 | } | |
| @@ -165,9 +165,7 @@ func (r *Repos) updateMeta(ctx context.Context, name domain.RepoName, params por | |||||
| 165 | if params.Visibility != nil { | 165 | if params.Visibility != nil { | ||
| 166 | repo.Visibility = *params.Visibility | 166 | repo.Visibility = *params.Visibility | ||
| 167 | } | 167 | } | ||
| 168 | - | if repo.Settings.IsZero() { | 168 | + | repo.Settings = repo.EffectiveSettings() |
| 169 | - | repo.Settings = domain.DefaultRepoSettings() | |||
| 170 | - | } | |||
| 171 | if params.AllowedMergeModes != nil { | 169 | if params.AllowedMergeModes != nil { | ||
| 172 | repo.Settings.AllowedMergeModes = *params.AllowedMergeModes | 170 | repo.Settings.AllowedMergeModes = *params.AllowedMergeModes | ||
| 173 | } | 171 | } | ||