refactor(domain): own commit-object and diff read budgets
CommitObjectBudget, ValidateCommitObjects and DiffBudget decide eligibility, deduplication, windows and byte/line limits; gitcli and ermitegit only stream.
- commit
- 0424a8adfd6f2e58982ddc6f693210fd60dae845
- author
- brice · · 2026-10-19 19:28 UTC
- committer
- brice · · 2026-10-19 20:01 UTC
- parent
- ac21956
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +264 -61
M internal/adapters/ermitegit/commitobjects.go +12 -11
| @@ -22,10 +22,10 @@ var _ ports.CommitObjectReader = (*Adapter)(nil) | |||
| 22 | 22 | // always serves it, and the wrapped reader only the repositories the library | |
| 23 | 23 | // cannot read. | |
| 24 | 24 | // | |
| 25 | - | //hexcheck:ignore suspicious-business-logic-in-adapter business rule for commit eligibility and byte budgets; pending move to domain/usecase | |
| 25 | + | //hexcheck:ignore suspicious-business-logic-in-adapter streams object headers and bytes; eligibility, deduplication and byte limits are decided by domain.CommitObjectBudget, request validation by domain.ValidateCommitObjects | |
| 26 | 26 | func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids []domain.ObjectID) (map[domain.ObjectID][]byte, error) { | |
| 27 | - | if len(ids) > ports.MaxCommitLimit { | |
| 28 | - | return nil, fmt.Errorf("%w: %d commit objects asked, at most %d", domain.ErrInvalid, len(ids), ports.MaxCommitLimit) | |
| 27 | + | if err := domain.ValidateCommitObjects(ids, ports.MaxCommitLimit); err != nil { | |
| 28 | + | return nil, err | |
| 29 | 29 | } | |
| 30 | 30 | cli := func() (map[domain.ObjectID][]byte, error) { | |
| 31 | 31 | r, ok := a.cli.(ports.CommitObjectReader) | |
| @@ -36,12 +36,12 @@ func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, i | |||
| 36 | 36 | } | |
| 37 | 37 | return serve(ctx, a, repo, func(r *git.Repo) (map[domain.ObjectID][]byte, error) { | |
| 38 | 38 | out := make(map[domain.ObjectID][]byte, len(ids)) | |
| 39 | - | var total int64 | |
| 39 | + | var budget domain.CommitObjectBudget | |
| 40 | 40 | for _, id := range ids { | |
| 41 | 41 | if err := ctx.Err(); err != nil { | |
| 42 | 42 | return nil, err | |
| 43 | 43 | } | |
| 44 | - | if _, done := out[id]; done { | |
| 44 | + | if budget.Seen(id) { | |
| 45 | 45 | continue | |
| 46 | 46 | } | |
| 47 | 47 | gid, err := git.ParseID(id.String()) | |
| @@ -55,12 +55,13 @@ func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, i | |||
| 55 | 55 | if err != nil { | |
| 56 | 56 | return nil, err | |
| 57 | 57 | } | |
| 58 | - | if typ != git.TypeCommit || size > domain.MaxCommitObjectBytes { | |
| 59 | - | continue | |
| 60 | - | } | |
| 61 | - | if total+size > domain.MaxCommitObjectsBytes { | |
| 58 | + | take, stop := budget.Admit(id, typ == git.TypeCommit, size) | |
| 59 | + | if stop { | |
| 62 | 60 | break | |
| 63 | 61 | } | |
| 62 | + | if !take { | |
| 63 | + | continue | |
| 64 | + | } | |
| 64 | 65 | typ, data, err := r.ReadObject(gid, nil) | |
| 65 | 66 | if errors.Is(err, git.ErrNotFound) { | |
| 66 | 67 | continue // removed meanwhile | |
| @@ -68,9 +69,9 @@ func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, i | |||
| 68 | 69 | if err != nil { | |
| 69 | 70 | return nil, err | |
| 70 | 71 | } | |
| 71 | - | if typ == git.TypeCommit && len(data) <= domain.MaxCommitObjectBytes && total+int64(len(data)) <= domain.MaxCommitObjectsBytes { | |
| 72 | + | if take, _ := budget.Admit(id, typ == git.TypeCommit, int64(len(data))); take { | |
| 72 | 73 | out[id] = data | |
| 73 | - | total += int64(len(data)) | |
| 74 | + | budget.Charge(id, int64(len(data))) | |
| 74 | 75 | } | |
| 75 | 76 | } | |
| 76 | 77 | return out, nil | |
| @@ -22,10 +22,10 @@ var _ ports.CommitObjectReader = (*Adapter)(nil) | |||||
| 22 | // always serves it, and the wrapped reader only the repositories the library | 22 | // always serves it, and the wrapped reader only the repositories the library | ||
| 23 | // cannot read. | 23 | // cannot read. | ||
| 24 | // | 24 | // | ||
| 25 | - | //hexcheck:ignore suspicious-business-logic-in-adapter business rule for commit eligibility and byte budgets; pending move to domain/usecase | 25 | + | //hexcheck:ignore suspicious-business-logic-in-adapter streams object headers and bytes; eligibility, deduplication and byte limits are decided by domain.CommitObjectBudget, request validation by domain.ValidateCommitObjects |
| 26 | func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids []domain.ObjectID) (map[domain.ObjectID][]byte, error) { | 26 | func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids []domain.ObjectID) (map[domain.ObjectID][]byte, error) { | ||
| 27 | - | if len(ids) > ports.MaxCommitLimit { | 27 | + | if err := domain.ValidateCommitObjects(ids, ports.MaxCommitLimit); err != nil { |
| 28 | - | return nil, fmt.Errorf("%w: %d commit objects asked, at most %d", domain.ErrInvalid, len(ids), ports.MaxCommitLimit) | 28 | + | return nil, err |
| 29 | } | 29 | } | ||
| 30 | cli := func() (map[domain.ObjectID][]byte, error) { | 30 | cli := func() (map[domain.ObjectID][]byte, error) { | ||
| 31 | r, ok := a.cli.(ports.CommitObjectReader) | 31 | r, ok := a.cli.(ports.CommitObjectReader) | ||
| @@ -36,12 +36,12 @@ func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, i | |||||
| 36 | } | 36 | } | ||
| 37 | return serve(ctx, a, repo, func(r *git.Repo) (map[domain.ObjectID][]byte, error) { | 37 | return serve(ctx, a, repo, func(r *git.Repo) (map[domain.ObjectID][]byte, error) { | ||
| 38 | out := make(map[domain.ObjectID][]byte, len(ids)) | 38 | out := make(map[domain.ObjectID][]byte, len(ids)) | ||
| 39 | - | var total int64 | 39 | + | var budget domain.CommitObjectBudget |
| 40 | for _, id := range ids { | 40 | for _, id := range ids { | ||
| 41 | if err := ctx.Err(); err != nil { | 41 | if err := ctx.Err(); err != nil { | ||
| 42 | return nil, err | 42 | return nil, err | ||
| 43 | } | 43 | } | ||
| 44 | - | if _, done := out[id]; done { | 44 | + | if budget.Seen(id) { |
| 45 | continue | 45 | continue | ||
| 46 | } | 46 | } | ||
| 47 | gid, err := git.ParseID(id.String()) | 47 | gid, err := git.ParseID(id.String()) | ||
| @@ -55,12 +55,13 @@ func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, i | |||||
| 55 | if err != nil { | 55 | if err != nil { | ||
| 56 | return nil, err | 56 | return nil, err | ||
| 57 | } | 57 | } | ||
| 58 | - | if typ != git.TypeCommit || size > domain.MaxCommitObjectBytes { | 58 | + | take, stop := budget.Admit(id, typ == git.TypeCommit, size) |
| 59 | - | continue | 59 | + | if stop { |
| 60 | - | } | |||
| 61 | - | if total+size > domain.MaxCommitObjectsBytes { | |||
| 62 | break | 60 | break | ||
| 63 | } | 61 | } | ||
| 62 | + | if !take { | |||
| 63 | + | continue | |||
| 64 | + | } | |||
| 64 | typ, data, err := r.ReadObject(gid, nil) | 65 | typ, data, err := r.ReadObject(gid, nil) | ||
| 65 | if errors.Is(err, git.ErrNotFound) { | 66 | if errors.Is(err, git.ErrNotFound) { | ||
| 66 | continue // removed meanwhile | 67 | continue // removed meanwhile | ||
| @@ -68,9 +69,9 @@ func (a *Adapter) ReadCommitObjects(ctx context.Context, repo domain.RepoName, i | |||||
| 68 | if err != nil { | 69 | if err != nil { | ||
| 69 | return nil, err | 70 | return nil, err | ||
| 70 | } | 71 | } | ||
| 71 | - | if typ == git.TypeCommit && len(data) <= domain.MaxCommitObjectBytes && total+int64(len(data)) <= domain.MaxCommitObjectsBytes { | 72 | + | if take, _ := budget.Admit(id, typ == git.TypeCommit, int64(len(data))); take { |
| 72 | out[id] = data | 73 | out[id] = data | ||
| 73 | - | total += int64(len(data)) | 74 | + | budget.Charge(id, int64(len(data))) |
| 74 | } | 75 | } | ||
| 75 | } | 76 | } | ||
| 76 | return out, nil | 77 | return out, nil | ||
M internal/adapters/gitcli/commitobjects.go +11 -15
| @@ -19,15 +19,10 @@ var _ ports.CommitObjectReader = (*Git)(nil) | |||
| 19 | 19 | // that would take the total over domain.MaxCommitObjectsBytes (known from | |
| 20 | 20 | // `info`, before any content is read, so the stream stays consistent). | |
| 21 | 21 | // | |
| 22 | - | //hexcheck:ignore suspicious-business-logic-in-adapter business rule for commit eligibility and byte budgets; pending move to domain/usecase | |
| 22 | + | //hexcheck:ignore suspicious-business-logic-in-adapter streams object headers and bytes; eligibility, deduplication and byte limits are decided by domain.CommitObjectBudget, request validation by domain.ValidateCommitObjects | |
| 23 | 23 | func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids []domain.ObjectID) (map[domain.ObjectID][]byte, error) { | |
| 24 | - | if len(ids) > ports.MaxCommitLimit { | |
| 25 | - | return nil, fmt.Errorf("%w: %d commit objects asked, at most %d", domain.ErrInvalid, len(ids), ports.MaxCommitLimit) | |
| 26 | - | } | |
| 27 | - | for _, id := range ids { | |
| 28 | - | if id.IsZero() { | |
| 29 | - | return nil, fmt.Errorf("%w: empty commit id", domain.ErrInvalidObjectID) | |
| 30 | - | } | |
| 24 | + | if err := domain.ValidateCommitObjects(ids, ports.MaxCommitLimit); err != nil { | |
| 25 | + | return nil, err | |
| 31 | 26 | } | |
| 32 | 27 | dir, err := g.repoDir(repo) | |
| 33 | 28 | if err != nil { | |
| @@ -36,12 +31,12 @@ func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids [ | |||
| 36 | 31 | out := make(map[domain.ObjectID][]byte, len(ids)) | |
| 37 | 32 | err = g.cat.do(ctx, dir, func(c *catConn) error { | |
| 38 | 33 | clear(out) // a retry on a fresh process starts over | |
| 39 | - | var total int64 | |
| 34 | + | var budget domain.CommitObjectBudget | |
| 40 | 35 | for _, id := range ids { | |
| 41 | 36 | if err := ctx.Err(); err != nil { | |
| 42 | 37 | return err | |
| 43 | 38 | } | |
| 44 | - | if _, done := out[id]; done { | |
| 39 | + | if budget.Seen(id) { | |
| 45 | 40 | continue | |
| 46 | 41 | } | |
| 47 | 42 | h, err := c.info(id.String()) | |
| @@ -51,18 +46,19 @@ func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids [ | |||
| 51 | 46 | if err != nil { | |
| 52 | 47 | return err | |
| 53 | 48 | } | |
| 54 | - | if h.Type != "commit" || h.Size > domain.MaxCommitObjectBytes { | |
| 55 | - | continue | |
| 56 | - | } | |
| 57 | - | if total+h.Size > domain.MaxCommitObjectsBytes { | |
| 49 | + | take, stop := budget.Admit(id, h.Type == "commit", h.Size) | |
| 50 | + | if stop { | |
| 58 | 51 | break | |
| 59 | 52 | } | |
| 53 | + | if !take { | |
| 54 | + | continue | |
| 55 | + | } | |
| 60 | 56 | _, data, err := c.contents(id.String(), domain.MaxCommitObjectBytes) | |
| 61 | 57 | if err != nil { | |
| 62 | 58 | return err | |
| 63 | 59 | } | |
| 64 | 60 | out[id] = data | |
| 65 | - | total += int64(len(data)) | |
| 61 | + | budget.Charge(id, int64(len(data))) | |
| 66 | 62 | } | |
| 67 | 63 | return nil | |
| 68 | 64 | }) | |
| @@ -19,15 +19,10 @@ var _ ports.CommitObjectReader = (*Git)(nil) | |||||
| 19 | // that would take the total over domain.MaxCommitObjectsBytes (known from | 19 | // that would take the total over domain.MaxCommitObjectsBytes (known from | ||
| 20 | // `info`, before any content is read, so the stream stays consistent). | 20 | // `info`, before any content is read, so the stream stays consistent). | ||
| 21 | // | 21 | // | ||
| 22 | - | //hexcheck:ignore suspicious-business-logic-in-adapter business rule for commit eligibility and byte budgets; pending move to domain/usecase | 22 | + | //hexcheck:ignore suspicious-business-logic-in-adapter streams object headers and bytes; eligibility, deduplication and byte limits are decided by domain.CommitObjectBudget, request validation by domain.ValidateCommitObjects |
| 23 | func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids []domain.ObjectID) (map[domain.ObjectID][]byte, error) { | 23 | func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids []domain.ObjectID) (map[domain.ObjectID][]byte, error) { | ||
| 24 | - | if len(ids) > ports.MaxCommitLimit { | 24 | + | if err := domain.ValidateCommitObjects(ids, ports.MaxCommitLimit); err != nil { |
| 25 | - | return nil, fmt.Errorf("%w: %d commit objects asked, at most %d", domain.ErrInvalid, len(ids), ports.MaxCommitLimit) | 25 | + | return nil, err |
| 26 | - | } | |||
| 27 | - | for _, id := range ids { | |||
| 28 | - | if id.IsZero() { | |||
| 29 | - | return nil, fmt.Errorf("%w: empty commit id", domain.ErrInvalidObjectID) | |||
| 30 | - | } | |||
| 31 | } | 26 | } | ||
| 32 | dir, err := g.repoDir(repo) | 27 | dir, err := g.repoDir(repo) | ||
| 33 | if err != nil { | 28 | if err != nil { | ||
| @@ -36,12 +31,12 @@ func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids [ | |||||
| 36 | out := make(map[domain.ObjectID][]byte, len(ids)) | 31 | out := make(map[domain.ObjectID][]byte, len(ids)) | ||
| 37 | err = g.cat.do(ctx, dir, func(c *catConn) error { | 32 | err = g.cat.do(ctx, dir, func(c *catConn) error { | ||
| 38 | clear(out) // a retry on a fresh process starts over | 33 | clear(out) // a retry on a fresh process starts over | ||
| 39 | - | var total int64 | 34 | + | var budget domain.CommitObjectBudget |
| 40 | for _, id := range ids { | 35 | for _, id := range ids { | ||
| 41 | if err := ctx.Err(); err != nil { | 36 | if err := ctx.Err(); err != nil { | ||
| 42 | return err | 37 | return err | ||
| 43 | } | 38 | } | ||
| 44 | - | if _, done := out[id]; done { | 39 | + | if budget.Seen(id) { |
| 45 | continue | 40 | continue | ||
| 46 | } | 41 | } | ||
| 47 | h, err := c.info(id.String()) | 42 | h, err := c.info(id.String()) | ||
| @@ -51,18 +46,19 @@ func (g *Git) ReadCommitObjects(ctx context.Context, repo domain.RepoName, ids [ | |||||
| 51 | if err != nil { | 46 | if err != nil { | ||
| 52 | return err | 47 | return err | ||
| 53 | } | 48 | } | ||
| 54 | - | if h.Type != "commit" || h.Size > domain.MaxCommitObjectBytes { | 49 | + | take, stop := budget.Admit(id, h.Type == "commit", h.Size) |
| 55 | - | continue | 50 | + | if stop { |
| 56 | - | } | |||
| 57 | - | if total+h.Size > domain.MaxCommitObjectsBytes { | |||
| 58 | break | 51 | break | ||
| 59 | } | 52 | } | ||
| 53 | + | if !take { | |||
| 54 | + | continue | |||
| 55 | + | } | |||
| 60 | _, data, err := c.contents(id.String(), domain.MaxCommitObjectBytes) | 56 | _, data, err := c.contents(id.String(), domain.MaxCommitObjectBytes) | ||
| 61 | if err != nil { | 57 | if err != nil { | ||
| 62 | return err | 58 | return err | ||
| 63 | } | 59 | } | ||
| 64 | out[id] = data | 60 | out[id] = data | ||
| 65 | - | total += int64(len(data)) | 61 | + | budget.Charge(id, int64(len(data))) |
| 66 | } | 62 | } | ||
| 67 | return nil | 63 | return nil | ||
| 68 | }) | 64 | }) | ||
M internal/adapters/gitcli/diff.go +15 -35
| @@ -89,7 +89,7 @@ type rawEntry struct { | |||
| 89 | 89 | ||
| 90 | 90 | // readDiff parses the output of diff-tree --raw --numstat -p. | |
| 91 | 91 | // | |
| 92 | - | //hexcheck:ignore suspicious-business-logic-in-adapter business rule for diff windowing and truncation budgets; pending move to domain/usecase | |
| 92 | + | //hexcheck:ignore suspicious-business-logic-in-adapter streams raw, numstat and patch sections; domain.DiffBudget decides window and hunk eligibility and accounts retained lines and bytes; adapter only skips sections and stops the process | |
| 93 | 93 | func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.DiffStats, error) { | |
| 94 | 94 | entries, err := readRaw(r) | |
| 95 | 95 | if err != nil { | |
| @@ -105,23 +105,17 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||
| 105 | 105 | stats.Deletions += entries[i].del | |
| 106 | 106 | } | |
| 107 | 107 | var diff domain.Diff | |
| 108 | - | if limits.MaxFiles > 0 && len(entries) > limits.MaxFiles { | |
| 109 | - | entries = entries[:limits.MaxFiles] | |
| 110 | - | diff.Truncated = true | |
| 111 | - | } | |
| 112 | - | // Only the files of the window [from, to) get their hunks; the others are | |
| 113 | - | // listed as they are, neither truncated nor counted in the budgets. | |
| 114 | - | from, to := max(limits.From, 0), len(entries) | |
| 115 | - | if limits.To > 0 { | |
| 116 | - | to = min(limits.To, len(entries)) | |
| 117 | - | } | |
| 108 | + | budget := domain.NewDiffBudget(limits) | |
| 109 | + | kept, from, to, truncated := budget.Window(len(entries)) | |
| 110 | + | entries = entries[:kept] | |
| 111 | + | diff.Truncated = truncated | |
| 118 | 112 | ||
| 119 | 113 | // The patch holds one section per file, two for a type change, in the | |
| 120 | 114 | // order of the entries. Entries without lines to show (binary, pure | |
| 121 | 115 | // rename, mode change) have a section without hunks. | |
| 122 | 116 | lastRead := -1 | |
| 123 | 117 | for i := from; i < to; i++ { | |
| 124 | - | if wantsHunks(&entries[i], limits, limits.MaxTotalLines, limits.MaxTotalBytes) { | |
| 118 | + | if budget.WantsHunks(entries[i].file.Binary, entries[i].added+entries[i].del) { | |
| 125 | 119 | lastRead = i | |
| 126 | 120 | } | |
| 127 | 121 | } | |
| @@ -131,8 +125,7 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||
| 131 | 125 | _, _ = r.Discard(1) | |
| 132 | 126 | } | |
| 133 | 127 | } | |
| 134 | - | pr := &patchReader{lr: newLineReader(r, maxDiffLine), bytesLeft: limits.MaxTotalBytes} | |
| 135 | - | budget := limits.MaxTotalLines | |
| 128 | + | pr := &patchReader{lr: newLineReader(r, maxDiffLine), budget: budget} | |
| 136 | 129 | diff.Files = make([]domain.FileDiff, 0, len(entries)) | |
| 137 | 130 | for i := range entries { | |
| 138 | 131 | e := &entries[i] | |
| @@ -142,21 +135,19 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||
| 142 | 135 | } | |
| 143 | 136 | inWindow := i >= from && i < to | |
| 144 | 137 | switch { | |
| 145 | - | case !inWindow, !changedLines(e): | |
| 138 | + | case !inWindow, !pr.budget.ChangedLines(e.file.Binary, e.added+e.del): | |
| 146 | 139 | // Nothing to show. | |
| 147 | - | case !wantsHunks(e, limits, budget, pr.bytesLeft): | |
| 140 | + | case !pr.budget.WantsHunks(e.file.Binary, e.added+e.del): | |
| 148 | 141 | e.file.Truncated = true | |
| 149 | 142 | default: | |
| 150 | - | bytesBefore := pr.bytesLeft | |
| 151 | - | hunks, lines, truncated, err := pr.file(sections, min(limits.MaxFileLines, budget)) | |
| 143 | + | before := pr.budget | |
| 144 | + | hunks, _, truncated, err := pr.file(sections, pr.budget.FileLines()) | |
| 152 | 145 | if err != nil { | |
| 153 | 146 | return domain.Diff{}, domain.DiffStats{}, err | |
| 154 | 147 | } | |
| 155 | 148 | e.file.Hunks, e.file.Truncated = hunks, truncated | |
| 156 | 149 | if truncated { | |
| 157 | - | pr.bytesLeft = bytesBefore // nothing of this file is kept | |
| 158 | - | } else { | |
| 159 | - | budget -= lines | |
| 150 | + | pr.budget = before // nothing of this file is kept | |
| 160 | 151 | } | |
| 161 | 152 | sections = 0 | |
| 162 | 153 | } | |
| @@ -175,17 +166,6 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||
| 175 | 166 | return diff, stats, nil | |
| 176 | 167 | } | |
| 177 | 168 | ||
| 178 | - | // changedLines reports whether the file has text lines in its patch. | |
| 179 | - | func changedLines(e *rawEntry) bool { return !e.file.Binary && e.added+e.del > 0 } | |
| 180 | - | ||
| 181 | - | // wantsHunks reports whether the lines of e can still be shown: the changed | |
| 182 | - | // lines alone, a lower bound of the hunk lines, must fit the per-file limit | |
| 183 | - | // and what remains of the total, and some text bytes must remain. | |
| 184 | - | func wantsHunks(e *rawEntry, limits domain.DiffLimits, linesLeft, bytesLeft int) bool { | |
| 185 | - | n := e.added + e.del | |
| 186 | - | return changedLines(e) && n <= limits.MaxFileLines && n <= linesLeft && bytesLeft > 0 | |
| 187 | - | } | |
| 188 | - | ||
| 189 | 169 | // readRaw reads the `--raw -z` records, which start with ':'. | |
| 190 | 170 | func readRaw(r *bufio.Reader) ([]rawEntry, error) { | |
| 191 | 171 | var entries []rawEntry | |
| @@ -333,7 +313,7 @@ type patchReader struct { | |||
| 333 | 313 | eof bool | |
| 334 | 314 | cut bool // the last line read was cut | |
| 335 | 315 | ||
| 336 | - | bytesLeft int // text bytes still allowed in the diff | |
| 316 | + | budget domain.DiffBudget | |
| 337 | 317 | } | |
| 338 | 318 | ||
| 339 | 319 | const sectionMarker = "diff --git " | |
| @@ -480,8 +460,8 @@ func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncate | |||
| 480 | 460 | continue | |
| 481 | 461 | } | |
| 482 | 462 | lines++ | |
| 483 | - | p.bytesLeft -= len(text) | |
| 484 | - | if lines > max || p.bytesLeft < 0 { | |
| 463 | + | fits := p.budget.ChargeLine(len(text)) | |
| 464 | + | if lines > max || !fits { | |
| 485 | 465 | return nil, lines, true, nil | |
| 486 | 466 | } | |
| 487 | 467 | cur.Lines = append(cur.Lines, l) | |
| @@ -89,7 +89,7 @@ type rawEntry struct { | |||||
| 89 | 89 | ||||
| 90 | // readDiff parses the output of diff-tree --raw --numstat -p. | 90 | // readDiff parses the output of diff-tree --raw --numstat -p. | ||
| 91 | // | 91 | // | ||
| 92 | - | //hexcheck:ignore suspicious-business-logic-in-adapter business rule for diff windowing and truncation budgets; pending move to domain/usecase | 92 | + | //hexcheck:ignore suspicious-business-logic-in-adapter streams raw, numstat and patch sections; domain.DiffBudget decides window and hunk eligibility and accounts retained lines and bytes; adapter only skips sections and stops the process |
| 93 | func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.DiffStats, error) { | 93 | func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.DiffStats, error) { | ||
| 94 | entries, err := readRaw(r) | 94 | entries, err := readRaw(r) | ||
| 95 | if err != nil { | 95 | if err != nil { | ||
| @@ -105,23 +105,17 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||||
| 105 | stats.Deletions += entries[i].del | 105 | stats.Deletions += entries[i].del | ||
| 106 | } | 106 | } | ||
| 107 | var diff domain.Diff | 107 | var diff domain.Diff | ||
| 108 | - | if limits.MaxFiles > 0 && len(entries) > limits.MaxFiles { | 108 | + | budget := domain.NewDiffBudget(limits) |
| 109 | - | entries = entries[:limits.MaxFiles] | 109 | + | kept, from, to, truncated := budget.Window(len(entries)) |
| 110 | - | diff.Truncated = true | 110 | + | entries = entries[:kept] |
| 111 | - | } | 111 | + | diff.Truncated = truncated |
| 112 | - | // Only the files of the window [from, to) get their hunks; the others are | |||
| 113 | - | // listed as they are, neither truncated nor counted in the budgets. | |||
| 114 | - | from, to := max(limits.From, 0), len(entries) | |||
| 115 | - | if limits.To > 0 { | |||
| 116 | - | to = min(limits.To, len(entries)) | |||
| 117 | - | } | |||
| 118 | 112 | ||||
| 119 | // The patch holds one section per file, two for a type change, in the | 113 | // The patch holds one section per file, two for a type change, in the | ||
| 120 | // order of the entries. Entries without lines to show (binary, pure | 114 | // order of the entries. Entries without lines to show (binary, pure | ||
| 121 | // rename, mode change) have a section without hunks. | 115 | // rename, mode change) have a section without hunks. | ||
| 122 | lastRead := -1 | 116 | lastRead := -1 | ||
| 123 | for i := from; i < to; i++ { | 117 | for i := from; i < to; i++ { | ||
| 124 | - | if wantsHunks(&entries[i], limits, limits.MaxTotalLines, limits.MaxTotalBytes) { | 118 | + | if budget.WantsHunks(entries[i].file.Binary, entries[i].added+entries[i].del) { |
| 125 | lastRead = i | 119 | lastRead = i | ||
| 126 | } | 120 | } | ||
| 127 | } | 121 | } | ||
| @@ -131,8 +125,7 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||||
| 131 | _, _ = r.Discard(1) | 125 | _, _ = r.Discard(1) | ||
| 132 | } | 126 | } | ||
| 133 | } | 127 | } | ||
| 134 | - | pr := &patchReader{lr: newLineReader(r, maxDiffLine), bytesLeft: limits.MaxTotalBytes} | 128 | + | pr := &patchReader{lr: newLineReader(r, maxDiffLine), budget: budget} |
| 135 | - | budget := limits.MaxTotalLines | |||
| 136 | diff.Files = make([]domain.FileDiff, 0, len(entries)) | 129 | diff.Files = make([]domain.FileDiff, 0, len(entries)) | ||
| 137 | for i := range entries { | 130 | for i := range entries { | ||
| 138 | e := &entries[i] | 131 | e := &entries[i] | ||
| @@ -142,21 +135,19 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||||
| 142 | } | 135 | } | ||
| 143 | inWindow := i >= from && i < to | 136 | inWindow := i >= from && i < to | ||
| 144 | switch { | 137 | switch { | ||
| 145 | - | case !inWindow, !changedLines(e): | 138 | + | case !inWindow, !pr.budget.ChangedLines(e.file.Binary, e.added+e.del): |
| 146 | // Nothing to show. | 139 | // Nothing to show. | ||
| 147 | - | case !wantsHunks(e, limits, budget, pr.bytesLeft): | 140 | + | case !pr.budget.WantsHunks(e.file.Binary, e.added+e.del): |
| 148 | e.file.Truncated = true | 141 | e.file.Truncated = true | ||
| 149 | default: | 142 | default: | ||
| 150 | - | bytesBefore := pr.bytesLeft | 143 | + | before := pr.budget |
| 151 | - | hunks, lines, truncated, err := pr.file(sections, min(limits.MaxFileLines, budget)) | 144 | + | hunks, _, truncated, err := pr.file(sections, pr.budget.FileLines()) |
| 152 | if err != nil { | 145 | if err != nil { | ||
| 153 | return domain.Diff{}, domain.DiffStats{}, err | 146 | return domain.Diff{}, domain.DiffStats{}, err | ||
| 154 | } | 147 | } | ||
| 155 | e.file.Hunks, e.file.Truncated = hunks, truncated | 148 | e.file.Hunks, e.file.Truncated = hunks, truncated | ||
| 156 | if truncated { | 149 | if truncated { | ||
| 157 | - | pr.bytesLeft = bytesBefore // nothing of this file is kept | 150 | + | pr.budget = before // nothing of this file is kept |
| 158 | - | } else { | |||
| 159 | - | budget -= lines | |||
| 160 | } | 151 | } | ||
| 161 | sections = 0 | 152 | sections = 0 | ||
| 162 | } | 153 | } | ||
| @@ -175,17 +166,6 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di | |||||
| 175 | return diff, stats, nil | 166 | return diff, stats, nil | ||
| 176 | } | 167 | } | ||
| 177 | 168 | ||||
| 178 | - | // changedLines reports whether the file has text lines in its patch. | |||
| 179 | - | func changedLines(e *rawEntry) bool { return !e.file.Binary && e.added+e.del > 0 } | |||
| 180 | - | ||||
| 181 | - | // wantsHunks reports whether the lines of e can still be shown: the changed | |||
| 182 | - | // lines alone, a lower bound of the hunk lines, must fit the per-file limit | |||
| 183 | - | // and what remains of the total, and some text bytes must remain. | |||
| 184 | - | func wantsHunks(e *rawEntry, limits domain.DiffLimits, linesLeft, bytesLeft int) bool { | |||
| 185 | - | n := e.added + e.del | |||
| 186 | - | return changedLines(e) && n <= limits.MaxFileLines && n <= linesLeft && bytesLeft > 0 | |||
| 187 | - | } | |||
| 188 | - | ||||
| 189 | // readRaw reads the `--raw -z` records, which start with ':'. | 169 | // readRaw reads the `--raw -z` records, which start with ':'. | ||
| 190 | func readRaw(r *bufio.Reader) ([]rawEntry, error) { | 170 | func readRaw(r *bufio.Reader) ([]rawEntry, error) { | ||
| 191 | var entries []rawEntry | 171 | var entries []rawEntry | ||
| @@ -333,7 +313,7 @@ type patchReader struct { | |||||
| 333 | eof bool | 313 | eof bool | ||
| 334 | cut bool // the last line read was cut | 314 | cut bool // the last line read was cut | ||
| 335 | 315 | ||||
| 336 | - | bytesLeft int // text bytes still allowed in the diff | 316 | + | budget domain.DiffBudget |
| 337 | } | 317 | } | ||
| 338 | 318 | ||||
| 339 | const sectionMarker = "diff --git " | 319 | const sectionMarker = "diff --git " | ||
| @@ -480,8 +460,8 @@ func (p *patchReader) section(max int) (hunks []domain.Hunk, lines int, truncate | |||||
| 480 | continue | 460 | continue | ||
| 481 | } | 461 | } | ||
| 482 | lines++ | 462 | lines++ | ||
| 483 | - | p.bytesLeft -= len(text) | 463 | + | fits := p.budget.ChargeLine(len(text)) |
| 484 | - | if lines > max || p.bytesLeft < 0 { | 464 | + | if lines > max || !fits { |
| 485 | return nil, lines, true, nil | 465 | return nil, lines, true, nil | ||
| 486 | } | 466 | } | ||
| 487 | cur.Lines = append(cur.Lines, l) | 467 | cur.Lines = append(cur.Lines, l) | ||
A internal/domain/budget.go +85 -0
| @@ -0,0 +1,85 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | package domain | |
| 4 | + | ||
| 5 | + | import "fmt" | |
| 6 | + | ||
| 7 | + | // ValidateCommitObjects checks a request before any repository I/O. max is the | |
| 8 | + | // caller's commit-count limit. | |
| 9 | + | func ValidateCommitObjects(ids []ObjectID, max int) error { | |
| 10 | + | if len(ids) > max { | |
| 11 | + | return fmt.Errorf("%w: %d commit objects asked, at most %d", ErrInvalid, len(ids), max) | |
| 12 | + | } | |
| 13 | + | for _, id := range ids { | |
| 14 | + | if id.IsZero() { | |
| 15 | + | return fmt.Errorf("%w: empty commit id", ErrInvalidObjectID) | |
| 16 | + | } | |
| 17 | + | } | |
| 18 | + | return nil | |
| 19 | + | } | |
| 20 | + | ||
| 21 | + | // CommitObjectBudget bounds a batch of raw commits. Its zero value is ready to | |
| 22 | + | // use. Admission is a preview; Charge records only successfully read content. | |
| 23 | + | type CommitObjectBudget struct { | |
| 24 | + | total int64 | |
| 25 | + | seen map[ObjectID]bool | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | func (b *CommitObjectBudget) Seen(id ObjectID) bool { return b.seen[id] } | |
| 29 | + | ||
| 30 | + | func (b *CommitObjectBudget) Admit(id ObjectID, commit bool, size int64) (take, stop bool) { | |
| 31 | + | if b.Seen(id) || !commit || size < 0 || size > MaxCommitObjectBytes { | |
| 32 | + | return false, false | |
| 33 | + | } | |
| 34 | + | if size > MaxCommitObjectsBytes-b.total { | |
| 35 | + | return false, true | |
| 36 | + | } | |
| 37 | + | return true, false | |
| 38 | + | } | |
| 39 | + | ||
| 40 | + | func (b *CommitObjectBudget) Charge(id ObjectID, size int64) { | |
| 41 | + | if b.seen == nil { | |
| 42 | + | b.seen = make(map[ObjectID]bool) | |
| 43 | + | } | |
| 44 | + | b.seen[id] = true | |
| 45 | + | b.total += size | |
| 46 | + | } | |
| 47 | + | ||
| 48 | + | // DiffBudget owns file selection and retained hunk accounting. Truncated files | |
| 49 | + | // consume no budget; streaming readers may use a copy while attempting a file. | |
| 50 | + | type DiffBudget struct { | |
| 51 | + | limits DiffLimits | |
| 52 | + | linesLeft, bytesLeft int | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | func NewDiffBudget(limits DiffLimits) DiffBudget { | |
| 56 | + | return DiffBudget{limits: limits, linesLeft: limits.MaxTotalLines, bytesLeft: limits.MaxTotalBytes} | |
| 57 | + | } | |
| 58 | + | ||
| 59 | + | func (b DiffBudget) Window(files int) (kept, from, to int, truncated bool) { | |
| 60 | + | kept = files | |
| 61 | + | if b.limits.MaxFiles > 0 && kept > b.limits.MaxFiles { | |
| 62 | + | kept = b.limits.MaxFiles | |
| 63 | + | truncated = true | |
| 64 | + | } | |
| 65 | + | from, to = max(b.limits.From, 0), kept | |
| 66 | + | if b.limits.To > 0 { | |
| 67 | + | to = min(b.limits.To, kept) | |
| 68 | + | } | |
| 69 | + | return | |
| 70 | + | } | |
| 71 | + | ||
| 72 | + | func (b DiffBudget) ChangedLines(binary bool, n int) bool { return !binary && n > 0 } | |
| 73 | + | ||
| 74 | + | func (b DiffBudget) WantsHunks(binary bool, n int) bool { | |
| 75 | + | return b.ChangedLines(binary, n) && n <= b.limits.MaxFileLines && n <= b.linesLeft && b.bytesLeft > 0 | |
| 76 | + | } | |
| 77 | + | ||
| 78 | + | func (b DiffBudget) FileLines() int { return min(b.limits.MaxFileLines, b.linesLeft) } | |
| 79 | + | ||
| 80 | + | // ChargeLine accounts for one decoded hunk line and reports whether it fits. | |
| 81 | + | func (b *DiffBudget) ChargeLine(bytes int) bool { | |
| 82 | + | b.linesLeft-- | |
| 83 | + | b.bytesLeft -= bytes | |
| 84 | + | return b.linesLeft >= 0 && b.bytesLeft >= 0 | |
| 85 | + | } | |
| @@ -0,0 +1,85 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | package domain | |||
| 4 | + | ||||
| 5 | + | import "fmt" | |||
| 6 | + | ||||
| 7 | + | // ValidateCommitObjects checks a request before any repository I/O. max is the | |||
| 8 | + | // caller's commit-count limit. | |||
| 9 | + | func ValidateCommitObjects(ids []ObjectID, max int) error { | |||
| 10 | + | if len(ids) > max { | |||
| 11 | + | return fmt.Errorf("%w: %d commit objects asked, at most %d", ErrInvalid, len(ids), max) | |||
| 12 | + | } | |||
| 13 | + | for _, id := range ids { | |||
| 14 | + | if id.IsZero() { | |||
| 15 | + | return fmt.Errorf("%w: empty commit id", ErrInvalidObjectID) | |||
| 16 | + | } | |||
| 17 | + | } | |||
| 18 | + | return nil | |||
| 19 | + | } | |||
| 20 | + | ||||
| 21 | + | // CommitObjectBudget bounds a batch of raw commits. Its zero value is ready to | |||
| 22 | + | // use. Admission is a preview; Charge records only successfully read content. | |||
| 23 | + | type CommitObjectBudget struct { | |||
| 24 | + | total int64 | |||
| 25 | + | seen map[ObjectID]bool | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | func (b *CommitObjectBudget) Seen(id ObjectID) bool { return b.seen[id] } | |||
| 29 | + | ||||
| 30 | + | func (b *CommitObjectBudget) Admit(id ObjectID, commit bool, size int64) (take, stop bool) { | |||
| 31 | + | if b.Seen(id) || !commit || size < 0 || size > MaxCommitObjectBytes { | |||
| 32 | + | return false, false | |||
| 33 | + | } | |||
| 34 | + | if size > MaxCommitObjectsBytes-b.total { | |||
| 35 | + | return false, true | |||
| 36 | + | } | |||
| 37 | + | return true, false | |||
| 38 | + | } | |||
| 39 | + | ||||
| 40 | + | func (b *CommitObjectBudget) Charge(id ObjectID, size int64) { | |||
| 41 | + | if b.seen == nil { | |||
| 42 | + | b.seen = make(map[ObjectID]bool) | |||
| 43 | + | } | |||
| 44 | + | b.seen[id] = true | |||
| 45 | + | b.total += size | |||
| 46 | + | } | |||
| 47 | + | ||||
| 48 | + | // DiffBudget owns file selection and retained hunk accounting. Truncated files | |||
| 49 | + | // consume no budget; streaming readers may use a copy while attempting a file. | |||
| 50 | + | type DiffBudget struct { | |||
| 51 | + | limits DiffLimits | |||
| 52 | + | linesLeft, bytesLeft int | |||
| 53 | + | } | |||
| 54 | + | ||||
| 55 | + | func NewDiffBudget(limits DiffLimits) DiffBudget { | |||
| 56 | + | return DiffBudget{limits: limits, linesLeft: limits.MaxTotalLines, bytesLeft: limits.MaxTotalBytes} | |||
| 57 | + | } | |||
| 58 | + | ||||
| 59 | + | func (b DiffBudget) Window(files int) (kept, from, to int, truncated bool) { | |||
| 60 | + | kept = files | |||
| 61 | + | if b.limits.MaxFiles > 0 && kept > b.limits.MaxFiles { | |||
| 62 | + | kept = b.limits.MaxFiles | |||
| 63 | + | truncated = true | |||
| 64 | + | } | |||
| 65 | + | from, to = max(b.limits.From, 0), kept | |||
| 66 | + | if b.limits.To > 0 { | |||
| 67 | + | to = min(b.limits.To, kept) | |||
| 68 | + | } | |||
| 69 | + | return | |||
| 70 | + | } | |||
| 71 | + | ||||
| 72 | + | func (b DiffBudget) ChangedLines(binary bool, n int) bool { return !binary && n > 0 } | |||
| 73 | + | ||||
| 74 | + | func (b DiffBudget) WantsHunks(binary bool, n int) bool { | |||
| 75 | + | return b.ChangedLines(binary, n) && n <= b.limits.MaxFileLines && n <= b.linesLeft && b.bytesLeft > 0 | |||
| 76 | + | } | |||
| 77 | + | ||||
| 78 | + | func (b DiffBudget) FileLines() int { return min(b.limits.MaxFileLines, b.linesLeft) } | |||
| 79 | + | ||||
| 80 | + | // ChargeLine accounts for one decoded hunk line and reports whether it fits. | |||
| 81 | + | func (b *DiffBudget) ChargeLine(bytes int) bool { | |||
| 82 | + | b.linesLeft-- | |||
| 83 | + | b.bytesLeft -= bytes | |||
| 84 | + | return b.linesLeft >= 0 && b.bytesLeft >= 0 | |||
| 85 | + | } | |||
A internal/domain/budget_test.go +141 -0
| @@ -0,0 +1,141 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | package domain | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "errors" | |
| 7 | + | "testing" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | func budgetID(t *testing.T, s string) ObjectID { | |
| 11 | + | t.Helper() | |
| 12 | + | id, err := ParseObjectID(s) | |
| 13 | + | if err != nil { | |
| 14 | + | t.Fatal(err) | |
| 15 | + | } | |
| 16 | + | return id | |
| 17 | + | } | |
| 18 | + | ||
| 19 | + | func TestValidateCommitObjects(t *testing.T) { | |
| 20 | + | id := budgetID(t, "1111111111111111111111111111111111111111") | |
| 21 | + | for _, tt := range []struct { | |
| 22 | + | name string | |
| 23 | + | ids []ObjectID | |
| 24 | + | max int | |
| 25 | + | want error | |
| 26 | + | }{ | |
| 27 | + | {"empty", nil, 1, nil}, {"boundary", []ObjectID{id}, 1, nil}, | |
| 28 | + | {"too many", []ObjectID{id, id}, 1, ErrInvalid}, {"zero", []ObjectID{{}}, 1, ErrInvalidObjectID}, | |
| 29 | + | } { | |
| 30 | + | t.Run(tt.name, func(t *testing.T) { | |
| 31 | + | if err := ValidateCommitObjects(tt.ids, tt.max); !errors.Is(err, tt.want) { | |
| 32 | + | t.Fatalf("got %v, want %v", err, tt.want) | |
| 33 | + | } | |
| 34 | + | }) | |
| 35 | + | } | |
| 36 | + | } | |
| 37 | + | ||
| 38 | + | func TestCommitObjectBudget(t *testing.T) { | |
| 39 | + | id := budgetID(t, "1111111111111111111111111111111111111111") | |
| 40 | + | for _, tt := range []struct { | |
| 41 | + | name string | |
| 42 | + | commit bool | |
| 43 | + | size, total int64 | |
| 44 | + | seen, take, stop bool | |
| 45 | + | }{ | |
| 46 | + | {"non commit", false, 1, 0, false, false, false}, | |
| 47 | + | {"oversize", true, MaxCommitObjectBytes + 1, 0, false, false, false}, | |
| 48 | + | {"exact object", true, MaxCommitObjectBytes, 0, false, true, false}, | |
| 49 | + | {"exact total", true, 1, MaxCommitObjectsBytes - 1, false, true, false}, | |
| 50 | + | {"over total", true, 2, MaxCommitObjectsBytes - 1, false, false, true}, | |
| 51 | + | {"duplicate", true, 1, MaxCommitObjectsBytes, true, false, false}, | |
| 52 | + | {"negative", true, -1, 0, false, false, false}, | |
| 53 | + | } { | |
| 54 | + | t.Run(tt.name, func(t *testing.T) { | |
| 55 | + | b := CommitObjectBudget{total: tt.total} | |
| 56 | + | if tt.seen { | |
| 57 | + | b.seen = map[ObjectID]bool{id: true} | |
| 58 | + | } | |
| 59 | + | take, stop := b.Admit(id, tt.commit, tt.size) | |
| 60 | + | if take != tt.take || stop != tt.stop { | |
| 61 | + | t.Fatalf("got %v,%v", take, stop) | |
| 62 | + | } | |
| 63 | + | if take { | |
| 64 | + | b.Charge(id, tt.size) | |
| 65 | + | if !b.Seen(id) || b.total != tt.total+tt.size { | |
| 66 | + | t.Fatal("charge not recorded") | |
| 67 | + | } | |
| 68 | + | } | |
| 69 | + | }) | |
| 70 | + | } | |
| 71 | + | } | |
| 72 | + | ||
| 73 | + | func TestDiffBudgetWindow(t *testing.T) { | |
| 74 | + | for _, tt := range []struct { | |
| 75 | + | name string | |
| 76 | + | limits DiffLimits | |
| 77 | + | kept, from, to int | |
| 78 | + | truncated bool | |
| 79 | + | }{ | |
| 80 | + | {"unlimited", DiffLimits{}, 5, 0, 5, false}, | |
| 81 | + | {"cap", DiffLimits{MaxFiles: 3}, 3, 0, 3, true}, | |
| 82 | + | {"window", DiffLimits{From: 1, To: 4}, 5, 1, 4, false}, | |
| 83 | + | {"clamped", DiffLimits{From: -1, To: 9, MaxFiles: 3}, 3, 0, 3, true}, | |
| 84 | + | {"past end", DiffLimits{From: 8}, 5, 8, 5, false}, | |
| 85 | + | {"reversed", DiffLimits{From: 4, To: 2}, 5, 4, 2, false}, | |
| 86 | + | } { | |
| 87 | + | t.Run(tt.name, func(t *testing.T) { | |
| 88 | + | b := NewDiffBudget(tt.limits) | |
| 89 | + | k, f, to, tr := b.Window(5) | |
| 90 | + | if k != tt.kept || f != tt.from || to != tt.to || tr != tt.truncated { | |
| 91 | + | t.Fatalf("got %d,%d,%d,%v", k, f, to, tr) | |
| 92 | + | } | |
| 93 | + | }) | |
| 94 | + | } | |
| 95 | + | } | |
| 96 | + | ||
| 97 | + | func TestDiffBudgetHunks(t *testing.T) { | |
| 98 | + | for _, tt := range []struct { | |
| 99 | + | name string | |
| 100 | + | binary bool | |
| 101 | + | n, lines, bytes int | |
| 102 | + | want bool | |
| 103 | + | }{ | |
| 104 | + | {"fits", false, 3, 3, 1, true}, {"binary", true, 3, 3, 1, false}, | |
| 105 | + | {"no changes", false, 0, 3, 1, false}, {"file limit", false, 4, 5, 1, false}, | |
| 106 | + | {"total lines", false, 3, 2, 1, false}, {"no bytes", false, 1, 3, 0, false}, | |
| 107 | + | } { | |
| 108 | + | t.Run(tt.name, func(t *testing.T) { | |
| 109 | + | b := NewDiffBudget(DiffLimits{MaxFileLines: 3, MaxTotalLines: tt.lines, MaxTotalBytes: tt.bytes}) | |
| 110 | + | if got := b.WantsHunks(tt.binary, tt.n); got != tt.want { | |
| 111 | + | t.Fatalf("got %v", got) | |
| 112 | + | } | |
| 113 | + | }) | |
| 114 | + | } | |
| 115 | + | } | |
| 116 | + | ||
| 117 | + | func TestDiffBudgetCharge(t *testing.T) { | |
| 118 | + | for _, tt := range []struct { | |
| 119 | + | name string | |
| 120 | + | bytes int | |
| 121 | + | fits bool | |
| 122 | + | }{{"exact", 3, true}, {"over", 4, false}} { | |
| 123 | + | t.Run(tt.name, func(t *testing.T) { | |
| 124 | + | b := NewDiffBudget(DiffLimits{MaxFileLines: 2, MaxTotalLines: 1, MaxTotalBytes: 3}) | |
| 125 | + | before := b | |
| 126 | + | if b.FileLines() != 1 { | |
| 127 | + | t.Fatal("wrong file limit") | |
| 128 | + | } | |
| 129 | + | if b.ChargeLine(tt.bytes) != tt.fits { | |
| 130 | + | t.Fatal("wrong charge result") | |
| 131 | + | } | |
| 132 | + | if b.ChargeLine(0) { | |
| 133 | + | t.Fatal("line budget exceeded") | |
| 134 | + | } | |
| 135 | + | b = before | |
| 136 | + | if !b.WantsHunks(false, 1) { | |
| 137 | + | t.Fatal("discarded file consumed budget") | |
| 138 | + | } | |
| 139 | + | }) | |
| 140 | + | } | |
| 141 | + | } | |
| @@ -0,0 +1,141 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | package domain | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "errors" | |||
| 7 | + | "testing" | |||
| 8 | + | ) | |||
| 9 | + | ||||
| 10 | + | func budgetID(t *testing.T, s string) ObjectID { | |||
| 11 | + | t.Helper() | |||
| 12 | + | id, err := ParseObjectID(s) | |||
| 13 | + | if err != nil { | |||
| 14 | + | t.Fatal(err) | |||
| 15 | + | } | |||
| 16 | + | return id | |||
| 17 | + | } | |||
| 18 | + | ||||
| 19 | + | func TestValidateCommitObjects(t *testing.T) { | |||
| 20 | + | id := budgetID(t, "1111111111111111111111111111111111111111") | |||
| 21 | + | for _, tt := range []struct { | |||
| 22 | + | name string | |||
| 23 | + | ids []ObjectID | |||
| 24 | + | max int | |||
| 25 | + | want error | |||
| 26 | + | }{ | |||
| 27 | + | {"empty", nil, 1, nil}, {"boundary", []ObjectID{id}, 1, nil}, | |||
| 28 | + | {"too many", []ObjectID{id, id}, 1, ErrInvalid}, {"zero", []ObjectID{{}}, 1, ErrInvalidObjectID}, | |||
| 29 | + | } { | |||
| 30 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 31 | + | if err := ValidateCommitObjects(tt.ids, tt.max); !errors.Is(err, tt.want) { | |||
| 32 | + | t.Fatalf("got %v, want %v", err, tt.want) | |||
| 33 | + | } | |||
| 34 | + | }) | |||
| 35 | + | } | |||
| 36 | + | } | |||
| 37 | + | ||||
| 38 | + | func TestCommitObjectBudget(t *testing.T) { | |||
| 39 | + | id := budgetID(t, "1111111111111111111111111111111111111111") | |||
| 40 | + | for _, tt := range []struct { | |||
| 41 | + | name string | |||
| 42 | + | commit bool | |||
| 43 | + | size, total int64 | |||
| 44 | + | seen, take, stop bool | |||
| 45 | + | }{ | |||
| 46 | + | {"non commit", false, 1, 0, false, false, false}, | |||
| 47 | + | {"oversize", true, MaxCommitObjectBytes + 1, 0, false, false, false}, | |||
| 48 | + | {"exact object", true, MaxCommitObjectBytes, 0, false, true, false}, | |||
| 49 | + | {"exact total", true, 1, MaxCommitObjectsBytes - 1, false, true, false}, | |||
| 50 | + | {"over total", true, 2, MaxCommitObjectsBytes - 1, false, false, true}, | |||
| 51 | + | {"duplicate", true, 1, MaxCommitObjectsBytes, true, false, false}, | |||
| 52 | + | {"negative", true, -1, 0, false, false, false}, | |||
| 53 | + | } { | |||
| 54 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 55 | + | b := CommitObjectBudget{total: tt.total} | |||
| 56 | + | if tt.seen { | |||
| 57 | + | b.seen = map[ObjectID]bool{id: true} | |||
| 58 | + | } | |||
| 59 | + | take, stop := b.Admit(id, tt.commit, tt.size) | |||
| 60 | + | if take != tt.take || stop != tt.stop { | |||
| 61 | + | t.Fatalf("got %v,%v", take, stop) | |||
| 62 | + | } | |||
| 63 | + | if take { | |||
| 64 | + | b.Charge(id, tt.size) | |||
| 65 | + | if !b.Seen(id) || b.total != tt.total+tt.size { | |||
| 66 | + | t.Fatal("charge not recorded") | |||
| 67 | + | } | |||
| 68 | + | } | |||
| 69 | + | }) | |||
| 70 | + | } | |||
| 71 | + | } | |||
| 72 | + | ||||
| 73 | + | func TestDiffBudgetWindow(t *testing.T) { | |||
| 74 | + | for _, tt := range []struct { | |||
| 75 | + | name string | |||
| 76 | + | limits DiffLimits | |||
| 77 | + | kept, from, to int | |||
| 78 | + | truncated bool | |||
| 79 | + | }{ | |||
| 80 | + | {"unlimited", DiffLimits{}, 5, 0, 5, false}, | |||
| 81 | + | {"cap", DiffLimits{MaxFiles: 3}, 3, 0, 3, true}, | |||
| 82 | + | {"window", DiffLimits{From: 1, To: 4}, 5, 1, 4, false}, | |||
| 83 | + | {"clamped", DiffLimits{From: -1, To: 9, MaxFiles: 3}, 3, 0, 3, true}, | |||
| 84 | + | {"past end", DiffLimits{From: 8}, 5, 8, 5, false}, | |||
| 85 | + | {"reversed", DiffLimits{From: 4, To: 2}, 5, 4, 2, false}, | |||
| 86 | + | } { | |||
| 87 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 88 | + | b := NewDiffBudget(tt.limits) | |||
| 89 | + | k, f, to, tr := b.Window(5) | |||
| 90 | + | if k != tt.kept || f != tt.from || to != tt.to || tr != tt.truncated { | |||
| 91 | + | t.Fatalf("got %d,%d,%d,%v", k, f, to, tr) | |||
| 92 | + | } | |||
| 93 | + | }) | |||
| 94 | + | } | |||
| 95 | + | } | |||
| 96 | + | ||||
| 97 | + | func TestDiffBudgetHunks(t *testing.T) { | |||
| 98 | + | for _, tt := range []struct { | |||
| 99 | + | name string | |||
| 100 | + | binary bool | |||
| 101 | + | n, lines, bytes int | |||
| 102 | + | want bool | |||
| 103 | + | }{ | |||
| 104 | + | {"fits", false, 3, 3, 1, true}, {"binary", true, 3, 3, 1, false}, | |||
| 105 | + | {"no changes", false, 0, 3, 1, false}, {"file limit", false, 4, 5, 1, false}, | |||
| 106 | + | {"total lines", false, 3, 2, 1, false}, {"no bytes", false, 1, 3, 0, false}, | |||
| 107 | + | } { | |||
| 108 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 109 | + | b := NewDiffBudget(DiffLimits{MaxFileLines: 3, MaxTotalLines: tt.lines, MaxTotalBytes: tt.bytes}) | |||
| 110 | + | if got := b.WantsHunks(tt.binary, tt.n); got != tt.want { | |||
| 111 | + | t.Fatalf("got %v", got) | |||
| 112 | + | } | |||
| 113 | + | }) | |||
| 114 | + | } | |||
| 115 | + | } | |||
| 116 | + | ||||
| 117 | + | func TestDiffBudgetCharge(t *testing.T) { | |||
| 118 | + | for _, tt := range []struct { | |||
| 119 | + | name string | |||
| 120 | + | bytes int | |||
| 121 | + | fits bool | |||
| 122 | + | }{{"exact", 3, true}, {"over", 4, false}} { | |||
| 123 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 124 | + | b := NewDiffBudget(DiffLimits{MaxFileLines: 2, MaxTotalLines: 1, MaxTotalBytes: 3}) | |||
| 125 | + | before := b | |||
| 126 | + | if b.FileLines() != 1 { | |||
| 127 | + | t.Fatal("wrong file limit") | |||
| 128 | + | } | |||
| 129 | + | if b.ChargeLine(tt.bytes) != tt.fits { | |||
| 130 | + | t.Fatal("wrong charge result") | |||
| 131 | + | } | |||
| 132 | + | if b.ChargeLine(0) { | |||
| 133 | + | t.Fatal("line budget exceeded") | |||
| 134 | + | } | |||
| 135 | + | b = before | |||
| 136 | + | if !b.WantsHunks(false, 1) { | |||
| 137 | + | t.Fatal("discarded file consumed budget") | |||
| 138 | + | } | |||
| 139 | + | }) | |||
| 140 | + | } | |||
| 141 | + | } | |||