Monorepo for Tangled

knotserver/git: fix pagination in git.Branches

Signed-off-by: oppiliappan <me@oppi.li>

authored by

oppiliappan and committed by tangled.org a24c79e5 ef019e79

+403 -25
+33 -3
knotserver/git/branch.go
··· 12 12 "tangled.org/core/types" 13 13 ) 14 14 15 - func (g *GitRepo) Branches() ([]types.Branch, error) { 15 + type BranchesOptions struct { 16 + Limit int 17 + Offset int 18 + } 19 + 20 + func (g *GitRepo) Branches(opts *BranchesOptions) ([]types.Branch, error) { 21 + if opts == nil { 22 + opts = &BranchesOptions{} 23 + } 24 + 16 25 fields := []string{ 17 26 "refname:short", 18 27 "objectname", ··· 33 42 if i != 0 { 34 43 outFormat.WriteString(fieldSeparator) 35 44 } 36 - outFormat.WriteString(fmt.Sprintf("%%(%s)", f)) 45 + fmt.Fprintf(&outFormat, "%%(%s)", f) 37 46 } 38 47 outFormat.WriteString("") 39 48 outFormat.WriteString(recordSeparator) 40 49 41 - output, err := g.forEachRef(outFormat.String(), "refs/heads") 50 + args := []string{outFormat.String(), "--sort=-creatordate"} 51 + 52 + // only add the count if the limit is a non-zero value, 53 + // if it is zero, get as many tags as we can 54 + if opts.Limit > 0 { 55 + args = append(args, fmt.Sprintf("--count=%d", opts.Offset+opts.Limit)) 56 + } 57 + 58 + args = append(args, "refs/heads") 59 + 60 + output, err := g.forEachRef(args...) 42 61 if err != nil { 43 62 return nil, fmt.Errorf("failed to get branches: %w", err) 44 63 } ··· 48 67 return nil, nil 49 68 } 50 69 70 + startIdx := opts.Offset 71 + if startIdx >= len(records) { 72 + return nil, nil 73 + } 74 + 75 + endIdx := len(records) 76 + if opts.Limit > 0 { 77 + endIdx = min(startIdx+opts.Limit, len(records)) 78 + } 79 + 80 + records = records[startIdx:endIdx] 51 81 branches := make([]types.Branch, 0, len(records)) 52 82 53 83 // ignore errors here
+355
knotserver/git/branch_test.go
··· 1 + package git 2 + 3 + import ( 4 + "path/filepath" 5 + "slices" 6 + "testing" 7 + 8 + gogit "github.com/go-git/go-git/v5" 9 + "github.com/go-git/go-git/v5/plumbing" 10 + "github.com/stretchr/testify/assert" 11 + "github.com/stretchr/testify/require" 12 + "github.com/stretchr/testify/suite" 13 + 14 + "tangled.org/core/sets" 15 + ) 16 + 17 + type BranchSuite struct { 18 + suite.Suite 19 + *RepoSuite 20 + } 21 + 22 + func TestBranchSuite(t *testing.T) { 23 + t.Parallel() 24 + suite.Run(t, new(BranchSuite)) 25 + } 26 + 27 + func (s *BranchSuite) SetupTest() { 28 + s.RepoSuite = NewRepoSuite(s.T()) 29 + } 30 + 31 + func (s *BranchSuite) TearDownTest() { 32 + s.RepoSuite.cleanup() 33 + } 34 + 35 + func (s *BranchSuite) setupRepoWithBranches() { 36 + s.init() 37 + 38 + // get the initial commit on master 39 + head, err := s.repo.r.Head() 40 + require.NoError(s.T(), err) 41 + initialCommit := head.Hash() 42 + 43 + // create multiple branches with commits 44 + // branch-1 45 + s.createBranch("branch-1", initialCommit) 46 + s.checkoutBranch("branch-1") 47 + _ = s.commitFile("file1.txt", "content 1", "Add file1 on branch-1") 48 + 49 + // branch-2 50 + s.createBranch("branch-2", initialCommit) 51 + s.checkoutBranch("branch-2") 52 + _ = s.commitFile("file2.txt", "content 2", "Add file2 on branch-2") 53 + 54 + // branch-3 55 + s.createBranch("branch-3", initialCommit) 56 + s.checkoutBranch("branch-3") 57 + _ = s.commitFile("file3.txt", "content 3", "Add file3 on branch-3") 58 + 59 + // branch-4 60 + s.createBranch("branch-4", initialCommit) 61 + s.checkoutBranch("branch-4") 62 + s.commitFile("file4.txt", "content 4", "Add file4 on branch-4") 63 + 64 + // back to master and make a commit 65 + s.checkoutBranch("master") 66 + s.commitFile("master-file.txt", "master content", "Add file on master") 67 + 68 + // verify we have multiple branches 69 + refs, err := s.repo.r.References() 70 + require.NoError(s.T(), err) 71 + 72 + branchCount := 0 73 + err = refs.ForEach(func(ref *plumbing.Reference) error { 74 + if ref.Name().IsBranch() { 75 + branchCount++ 76 + } 77 + return nil 78 + }) 79 + require.NoError(s.T(), err) 80 + 81 + // we should have 5 branches: master, branch-1, branch-2, branch-3, branch-4 82 + assert.Equal(s.T(), 5, branchCount, "expected 5 branches") 83 + } 84 + 85 + func (s *BranchSuite) TestBranches_All() { 86 + s.setupRepoWithBranches() 87 + 88 + branches, err := s.repo.Branches(&BranchesOptions{}) 89 + require.NoError(s.T(), err) 90 + 91 + assert.Len(s.T(), branches, 5, "expected 5 branches") 92 + 93 + expectedBranches := sets.Collect(slices.Values([]string{ 94 + "master", 95 + "branch-1", 96 + "branch-2", 97 + "branch-3", 98 + "branch-4", 99 + })) 100 + 101 + for _, branch := range branches { 102 + assert.True(s.T(), expectedBranches.Contains(branch.Reference.Name), 103 + "unexpected branch: %s", branch.Reference.Name) 104 + assert.NotEmpty(s.T(), branch.Reference.Hash, "branch hash should not be empty") 105 + assert.NotNil(s.T(), branch.Commit, "branch commit should not be nil") 106 + } 107 + } 108 + 109 + func (s *BranchSuite) TestBranches_WithLimit() { 110 + s.setupRepoWithBranches() 111 + 112 + tests := []struct { 113 + name string 114 + limit int 115 + expectedCount int 116 + }{ 117 + { 118 + name: "limit 1", 119 + limit: 1, 120 + expectedCount: 1, 121 + }, 122 + { 123 + name: "limit 2", 124 + limit: 2, 125 + expectedCount: 2, 126 + }, 127 + { 128 + name: "limit 3", 129 + limit: 3, 130 + expectedCount: 3, 131 + }, 132 + { 133 + name: "limit 10 (more than available)", 134 + limit: 10, 135 + expectedCount: 5, 136 + }, 137 + } 138 + 139 + for _, tt := range tests { 140 + s.Run(tt.name, func() { 141 + branches, err := s.repo.Branches(&BranchesOptions{ 142 + Limit: tt.limit, 143 + }) 144 + require.NoError(s.T(), err) 145 + assert.Len(s.T(), branches, tt.expectedCount, "expected %d branches", tt.expectedCount) 146 + }) 147 + } 148 + } 149 + 150 + func (s *BranchSuite) TestBranches_WithOffset() { 151 + s.setupRepoWithBranches() 152 + 153 + tests := []struct { 154 + name string 155 + offset int 156 + expectedCount int 157 + }{ 158 + { 159 + name: "offset 0", 160 + offset: 0, 161 + expectedCount: 5, 162 + }, 163 + { 164 + name: "offset 1", 165 + offset: 1, 166 + expectedCount: 4, 167 + }, 168 + { 169 + name: "offset 2", 170 + offset: 2, 171 + expectedCount: 3, 172 + }, 173 + { 174 + name: "offset 4", 175 + offset: 4, 176 + expectedCount: 1, 177 + }, 178 + { 179 + name: "offset 5 (all skipped)", 180 + offset: 5, 181 + expectedCount: 0, 182 + }, 183 + { 184 + name: "offset 10 (more than available)", 185 + offset: 10, 186 + expectedCount: 0, 187 + }, 188 + } 189 + 190 + for _, tt := range tests { 191 + s.Run(tt.name, func() { 192 + branches, err := s.repo.Branches(&BranchesOptions{ 193 + Offset: tt.offset, 194 + }) 195 + require.NoError(s.T(), err) 196 + assert.Len(s.T(), branches, tt.expectedCount, "expected %d branches", tt.expectedCount) 197 + }) 198 + } 199 + } 200 + 201 + func (s *BranchSuite) TestBranches_WithLimitAndOffset() { 202 + s.setupRepoWithBranches() 203 + 204 + tests := []struct { 205 + name string 206 + limit int 207 + offset int 208 + expectedCount int 209 + }{ 210 + { 211 + name: "limit 2, offset 0", 212 + limit: 2, 213 + offset: 0, 214 + expectedCount: 2, 215 + }, 216 + { 217 + name: "limit 2, offset 1", 218 + limit: 2, 219 + offset: 1, 220 + expectedCount: 2, 221 + }, 222 + { 223 + name: "limit 2, offset 3", 224 + limit: 2, 225 + offset: 3, 226 + expectedCount: 2, 227 + }, 228 + { 229 + name: "limit 2, offset 4", 230 + limit: 2, 231 + offset: 4, 232 + expectedCount: 1, 233 + }, 234 + { 235 + name: "limit 3, offset 2", 236 + limit: 3, 237 + offset: 2, 238 + expectedCount: 3, 239 + }, 240 + { 241 + name: "limit 10, offset 3", 242 + limit: 10, 243 + offset: 3, 244 + expectedCount: 2, 245 + }, 246 + } 247 + 248 + for _, tt := range tests { 249 + s.Run(tt.name, func() { 250 + branches, err := s.repo.Branches(&BranchesOptions{ 251 + Limit: tt.limit, 252 + Offset: tt.offset, 253 + }) 254 + require.NoError(s.T(), err) 255 + assert.Len(s.T(), branches, tt.expectedCount, "expected %d branches", tt.expectedCount) 256 + }) 257 + } 258 + } 259 + 260 + func (s *BranchSuite) TestBranches_EmptyRepo() { 261 + repoPath := filepath.Join(s.tempDir, "empty-repo") 262 + 263 + _, err := gogit.PlainInit(repoPath, false) 264 + require.NoError(s.T(), err) 265 + 266 + gitRepo, err := PlainOpen(repoPath) 267 + require.NoError(s.T(), err) 268 + 269 + branches, err := gitRepo.Branches(&BranchesOptions{}) 270 + require.NoError(s.T(), err) 271 + 272 + if branches != nil { 273 + assert.Empty(s.T(), branches, "expected no branches in empty repo") 274 + } 275 + } 276 + 277 + func (s *BranchSuite) TestBranches_Pagination() { 278 + s.setupRepoWithBranches() 279 + 280 + allBranches, err := s.repo.Branches(&BranchesOptions{}) 281 + require.NoError(s.T(), err) 282 + assert.Len(s.T(), allBranches, 5, "expected 5 branches") 283 + 284 + pageSize := 2 285 + var paginatedBranches []string 286 + 287 + for offset := 0; offset < len(allBranches); offset += pageSize { 288 + branches, err := s.repo.Branches(&BranchesOptions{ 289 + Limit: pageSize, 290 + Offset: offset, 291 + }) 292 + require.NoError(s.T(), err) 293 + for _, branch := range branches { 294 + paginatedBranches = append(paginatedBranches, branch.Reference.Name) 295 + } 296 + } 297 + 298 + assert.Len(s.T(), paginatedBranches, len(allBranches), "pagination should return all branches") 299 + 300 + // create sets to verify all branches are present 301 + allBranchNames := sets.New[string]() 302 + for _, branch := range allBranches { 303 + allBranchNames.Insert(branch.Reference.Name) 304 + } 305 + 306 + paginatedBranchNames := sets.New[string]() 307 + for _, name := range paginatedBranches { 308 + paginatedBranchNames.Insert(name) 309 + } 310 + 311 + assert.EqualValues(s.T(), allBranchNames, paginatedBranchNames, 312 + "pagination should return the same set of branches") 313 + } 314 + 315 + func (s *BranchSuite) TestBranches_VerifyBranchFields() { 316 + s.setupRepoWithBranches() 317 + 318 + branches, err := s.repo.Branches(&BranchesOptions{}) 319 + require.NoError(s.T(), err) 320 + 321 + found := false 322 + for i := range branches { 323 + if branches[i].Reference.Name == "master" { 324 + found = true 325 + assert.Equal(s.T(), "master", branches[i].Reference.Name) 326 + assert.NotEmpty(s.T(), branches[i].Reference.Hash) 327 + assert.NotNil(s.T(), branches[i].Commit) 328 + assert.NotEmpty(s.T(), branches[i].Commit.Author.Name) 329 + assert.NotEmpty(s.T(), branches[i].Commit.Author.Email) 330 + assert.False(s.T(), branches[i].Commit.Hash.IsZero()) 331 + break 332 + } 333 + } 334 + 335 + assert.True(s.T(), found, "master branch not found") 336 + } 337 + 338 + func (s *BranchSuite) TestBranches_NilOptions() { 339 + s.setupRepoWithBranches() 340 + 341 + branches, err := s.repo.Branches(nil) 342 + require.NoError(s.T(), err) 343 + assert.Len(s.T(), branches, 5, "nil options should return all branches") 344 + } 345 + 346 + func (s *BranchSuite) TestBranches_ZeroLimitAndOffset() { 347 + s.setupRepoWithBranches() 348 + 349 + branches, err := s.repo.Branches(&BranchesOptions{ 350 + Limit: 0, 351 + Offset: 0, 352 + }) 353 + require.NoError(s.T(), err) 354 + assert.Len(s.T(), branches, 5, "zero limit should return all branches") 355 + }
+1 -1
knotserver/git/post_receive.go
··· 95 95 // git rev-list <newsha> ^other-branches --not ^this-branch 96 96 args = append(args, line.NewSha.String()) 97 97 98 - branches, _ := g.Branches() 98 + branches, _ := g.Branches(nil) 99 99 for _, b := range branches { 100 100 if !strings.Contains(line.Ref, b.Name) { 101 101 args = append(args, fmt.Sprintf("^%s", b.Name))
+14 -21
knotserver/xrpc/repo_branches.go
··· 17 17 return 18 18 } 19 19 20 - cursor := r.URL.Query().Get("cursor") 20 + // default 21 + limit := 50 22 + offset := 0 21 23 22 - // limit := 50 // default 23 - // if limitStr := r.URL.Query().Get("limit"); limitStr != "" { 24 - // if l, err := strconv.Atoi(limitStr); err == nil && l > 0 && l <= 100 { 25 - // limit = l 26 - // } 27 - // } 24 + if l, err := strconv.Atoi(r.URL.Query().Get("limit")); err == nil && l > 0 && l <= 100 { 25 + limit = l 26 + } 28 27 29 - limit := 500 28 + if o, err := strconv.Atoi(r.URL.Query().Get("cursor")); err == nil && o > 0 { 29 + offset = o 30 + } 30 31 31 32 gr, err := git.PlainOpen(repoPath) 32 33 if err != nil { ··· 34 35 return 35 36 } 36 37 37 - branches, _ := gr.Branches() 38 - 39 - offset := 0 40 - if cursor != "" { 41 - if o, err := strconv.Atoi(cursor); err == nil && o >= 0 && o < len(branches) { 42 - offset = o 43 - } 44 - } 45 - 46 - end := min(offset+limit, len(branches)) 47 - 48 - paginatedBranches := branches[offset:end] 38 + branches, _ := gr.Branches(&git.BranchesOptions{ 39 + Limit: limit, 40 + Offset: offset, 41 + }) 49 42 50 43 // Create response using existing types.RepoBranchesResponse 51 44 response := types.RepoBranchesResponse{ 52 - Branches: paginatedBranches, 45 + Branches: branches, 53 46 } 54 47 55 48 writeJson(w, response)