Refactor Branch struct in package modules/git

models/fixtures/branch.yml

@@ -93,3 +93,111 @@
  is_deleted: false
  deleted_by_id: 0
  deleted_unix: 0
+
+-
+ id: 16
+ repo_id: 16
+ name: 'master'
+ commit_id: '69554a64c1e6030f051e5c3f94bfbd773cd6a324'
+ commit_message: 'not signed commit'
+ commit_time: 1502042309
+ pusher_id: 2
+ is_deleted: false
+ deleted_by_id: 0
+ deleted_unix: 0
+
+-
+ id: 17
+ repo_id: 16
+ name: 'not-signed'
+ commit_id: '69554a64c1e6030f051e5c3f94bfbd773cd6a324'
+ commit_message: 'not signed commit'
+ commit_time: 1502042309
+ pusher_id: 2
+ is_deleted: false
+ deleted_by_id: 0
+ deleted_unix: 0
+
+-
+ id: 18
+ repo_id: 16
+ name: 'good-sign-not-yet-validated'
+ commit_id: '27566bd5738fc8b4e3fef3c5e72cce608537bd95'
+ commit_message: 'good signed commit (with not yet validated email)'
+ commit_time: 1502042234
+ pusher_id: 2
+ is_deleted: false
+ deleted_by_id: 0
+ deleted_unix: 0
+
+-
+ id: 19
+ repo_id: 16
+ name: 'good-sign'
+ commit_id: 'f27c2b2b03dcab38beaf89b0ab4ff61f6de63441'
+ commit_message: 'good signed commit'
+ commit_time: 1502042101
+ pusher_id: 2
+ is_deleted: false
+ deleted_by_id: 0
+ deleted_unix: 0
+
+-
+ id: 20
+ repo_id: 1
+ name: 'feature/1'
+ commit_id: '65f1bf27bc3bf70f64657658635e66094edbcb4d'
+ commit_message: 'Initial commit'
+ commit_time: 1489950479
+ pusher_id: 2
+ is_deleted: false
+ deleted_by_id: 0
+ deleted_unix: 0
+
+-
+ id: 21
+ repo_id: 49
+ name: 'master'
+ commit_id: 'aacbdfe9e1c4b47f60abe81849045fa4e96f1d75'
+ commit_message: "Add 'test/test.txt'"
+ commit_time: 1572535577
+ pusher_id: 2
+ is_deleted: false
+ deleted_by_id: 0
+ deleted_unix: 0
+
+-
+ id: 22
+ repo_id: 1
+ name: 'develop'
+ commit_id: '65f1bf27bc3bf70f64657658635e66094edbcb4d'
+ commit_message: "Initial commit"
+ commit_time: 1489927679
+ pusher_id: 1
+ is_deleted: false
+ deleted_by_id: 0
+ deleted_unix: 0
+
+-
+ id: 23
+ repo_id: 3
+ name: 'master'
+ commit_id: '2a47ca4b614a9f5a43abbd5ad851a54a616ffee6'
+ commit_message: "init project"
+ commit_time: 1497448461
+ pusher_id: 1
+ is_deleted: false
+ deleted_by_id: 0
+ deleted_unix: 0
+
+-
+ id: 24
+ repo_id: 3
+ name: 'test_branch'
+ commit_id: 'd22b4d4daa5be07329fcef6ed458f00cf3392da0'
+ commit_message: "test commit"
+ commit_time: 1602935385
+ pusher_id: 1
+ is_deleted: false
+ deleted_by_id: 0
+ deleted_unix: 0

modules/git/repo_branch.go

@@ -7,7 +7,6 @@ package git
 import (
 "context"
 "errors"
-"fmt"
 "strings"
 )
 
@@ -25,36 +24,6 @@ func IsBranchExist(ctx context.Context, repoPath, name string) bool {
 return IsReferenceExist(ctx, repoPath, BranchPrefix+name)
 }
 
-// Branch represents a Git branch.
-type Branch struct {
-Name string
-Path string
-
-gitRepo *Repository
-}
-
-// GetHEADBranch returns corresponding branch of HEAD.
-func (repo *Repository) GetHEADBranch() (*Branch, error) {
-if repo == nil {
-return nil, errors.New("nil repo")
-}
-stdout, _, err := NewCommand("symbolic-ref", "HEAD").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path})
-if err != nil {
-return nil, err
-}
-stdout = strings.TrimSpace(stdout)
-
-if !strings.HasPrefix(stdout, BranchPrefix) {
-return nil, fmt.Errorf("invalid HEAD branch: %v", stdout)
-}
-
-return &Branch{
-Name: stdout[len(BranchPrefix):],
-Path: stdout,
-gitRepo: repo,
-}, nil
-}
-
 func GetDefaultBranch(ctx context.Context, repoPath string) (string, error) {
 stdout, _, err := NewCommand("symbolic-ref", "HEAD").RunStdString(ctx, &RunOpts{Dir: repoPath})
 if err != nil {
@@ -67,37 +36,6 @@ func GetDefaultBranch(ctx context.Context, repoPath string) (string, error) {
 return strings.TrimPrefix(stdout, BranchPrefix), nil
 }
 
-// GetBranch returns a branch by it's name
-func (repo *Repository) GetBranch(branch string) (*Branch, error) {
-if !repo.IsBranchExist(branch) {
-return nil, ErrBranchNotExist{branch}
-}
-return &Branch{
-Path: repo.Path,
-Name: branch,
-gitRepo: repo,
-}, nil
-}
-
-// GetBranches returns a slice of *git.Branch
-func (repo *Repository) GetBranches(skip, limit int) ([]*Branch, int, error) {
-brs, countAll, err := repo.GetBranchNames(skip, limit)
-if err != nil {
-return nil, 0, err
-}
-
-branches := make([]*Branch, len(brs))
-for i := range brs {
-branches[i] = &Branch{
-Path: repo.Path,
-Name: brs[i],
-gitRepo: repo,
-}
-}
-
-return branches, countAll, nil
-}
-
 // DeleteBranchOptions Option(s) for delete branch
 type DeleteBranchOptions struct {
 Force bool
@@ -147,11 +85,6 @@ func (repo *Repository) RemoveRemote(name string) error {
 return err
 }
 
-// GetCommit returns the head commit of a branch
-func (branch *Branch) GetCommit() (*Commit, error) {
-return branch.gitRepo.GetBranchCommit(branch.Name)
-}
-
 // RenameBranch rename a branch
 func (repo *Repository) RenameBranch(from, to string) error {
 _, _, err := NewCommand("branch", "-m").AddDynamicArguments(from, to).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path})

modules/gitrepo/branch.go

@@ -11,14 +11,14 @@ import (
 
 // GetBranchesByPath returns a branch by its path
 // if limit = 0 it will not limit
-func GetBranchesByPath(ctx context.Context, repo Repository, skip, limit int) ([]*git.Branch, int, error) {
+func GetBranchesByPath(ctx context.Context, repo Repository, skip, limit int) ([]string, int, error) {
 gitRepo, err := OpenRepository(ctx, repo)
 if err != nil {
 return nil, 0, err
 }
 defer gitRepo.Close()
 
-return gitRepo.GetBranches(skip, limit)
+return gitRepo.GetBranchNames(skip, limit)
 }
 
 func GetBranchCommitID(ctx context.Context, repo Repository, branch string) (string, error) {

routers/api/v1/repo/branch.go

@@ -59,17 +59,16 @@ func GetBranch(ctx *context.APIContext) {
 
 branchName := ctx.PathParam("*")
 
-branch, err := ctx.Repo.GitRepo.GetBranch(branchName)
+exist, err := git_model.IsBranchExist(ctx, ctx.Repo.Repository.ID, branchName)
 if err != nil {
-if git.IsErrBranchNotExist(err) {
-ctx.APIErrorNotFound(err)
-} else {
-ctx.APIErrorInternal(err)
-}
+ctx.APIErrorInternal(err)
+return
+} else if !exist {
+ctx.APIErrorNotFound(err)
 return
 }
 
-c, err := branch.GetCommit()
+c, err := ctx.Repo.GitRepo.GetBranchCommit(branchName)
 if err != nil {
 ctx.APIErrorInternal(err)
 return
@@ -81,7 +80,7 @@ func GetBranch(ctx *context.APIContext) {
 return
 }
 
-br, err := convert.ToBranch(ctx, ctx.Repo.Repository, branch.Name, c, branchProtection, ctx.Doer, ctx.Repo.IsAdmin())
+br, err := convert.ToBranch(ctx, ctx.Repo.Repository, branchName, c, branchProtection, ctx.Doer, ctx.Repo.IsAdmin())
 if err != nil {
 ctx.APIErrorInternal(err)
 return
@@ -260,25 +259,19 @@ func CreateBranch(ctx *context.APIContext) {
 return
 }
 
-branch, err := ctx.Repo.GitRepo.GetBranch(opt.BranchName)
-if err != nil {
-ctx.APIErrorInternal(err)
-return
-}
-
-commit, err := branch.GetCommit()
+commit, err := ctx.Repo.GitRepo.GetBranchCommit(opt.BranchName)
 if err != nil {
 ctx.APIErrorInternal(err)
 return
 }
 
-branchProtection, err := git_model.GetFirstMatchProtectedBranchRule(ctx, ctx.Repo.Repository.ID, branch.Name)
+branchProtection, err := git_model.GetFirstMatchProtectedBranchRule(ctx, ctx.Repo.Repository.ID, opt.BranchName)
 if err != nil {
 ctx.APIErrorInternal(err)
 return
 }
 
-br, err := convert.ToBranch(ctx, ctx.Repo.Repository, branch.Name, commit, branchProtection, ctx.Doer, ctx.Repo.IsAdmin())
+br, err := convert.ToBranch(ctx, ctx.Repo.Repository, opt.BranchName, commit, branchProtection, ctx.Doer, ctx.Repo.IsAdmin())
 if err != nil {
 ctx.APIErrorInternal(err)
 return

routers/api/v1/repo/commits.go

@@ -179,13 +179,7 @@ func GetAllCommits(ctx *context.APIContext) {
 var baseCommit *git.Commit
 if len(sha) == 0 {
 // no sha supplied - use default branch
-head, err := ctx.Repo.GitRepo.GetHEADBranch()
-if err != nil {
-ctx.APIErrorInternal(err)
-return
-}
-
-baseCommit, err = ctx.Repo.GitRepo.GetBranchCommit(head.Name)
+baseCommit, err = ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultBranch)
 if err != nil {
 ctx.APIErrorInternal(err)
 return

routers/web/repo/editor.go

@@ -668,7 +668,7 @@ func UploadFilePost(ctx *context.Context) {
 }
 
 if oldBranchName != branchName {
-if _, err := ctx.Repo.GitRepo.GetBranch(branchName); err == nil {
+if exist, err := git_model.IsBranchExist(ctx, ctx.Repo.Repository.ID, branchName); err == nil && exist {
 ctx.Data["Err_NewBranchName"] = true
 ctx.RenderWithErr(ctx.Tr("repo.editor.branch_already_exists", branchName), tplUploadFile, &form)
 return
@@ -875,12 +875,11 @@ func GetUniquePatchBranchName(ctx *context.Context) string {
 prefix := ctx.Doer.LowerName + "-patch-"
 for i := 1; i <= 1000; i++ {
 branchName := fmt.Sprintf("%s%d", prefix, i)
-if _, err := ctx.Repo.GitRepo.GetBranch(branchName); err != nil {
-if git.IsErrBranchNotExist(err) {
-return branchName
-}
+if exist, err := git_model.IsBranchExist(ctx, ctx.Repo.Repository.ID, branchName); err != nil {
 log.Error("GetUniquePatchBranchName: %v", err)
 return ""
+} else if !exist {
+return branchName
 }
 }
 return ""

routers/web/repo/view_home.go

@@ -269,7 +269,7 @@ func handleRepoEmptyOrBroken(ctx *context.Context) {
 } else if reallyEmpty {
 showEmpty = true // the repo is really empty
 updateContextRepoEmptyAndStatus(ctx, true, repo_model.RepositoryReady)
-} else if branches, _, _ := ctx.Repo.GitRepo.GetBranches(0, 1); len(branches) == 0 {
+} else if branches, _, _ := ctx.Repo.GitRepo.GetBranchNames(0, 1); len(branches) == 0 {
 showEmpty = true // it is not really empty, but there is no branch
 // at the moment, other repo units like "actions" are not able to handle such case,
 // so we just mark the repo as empty to prevent from displaying these units.

services/context/repo.go

@@ -817,9 +817,9 @@ func RepoRefByType(detectRefType git.RefType) func(*Context) {
 if reqPath == "" {
 refShortName = ctx.Repo.Repository.DefaultBranch
 if !gitrepo.IsBranchExist(ctx, ctx.Repo.Repository, refShortName) {
-brs, _, err := ctx.Repo.GitRepo.GetBranches(0, 1)
+brs, _, err := ctx.Repo.GitRepo.GetBranchNames(0, 1)
 if err == nil && len(brs) != 0 {
-refShortName = brs[0].Name
+refShortName = brs[0]
 } else if len(brs) == 0 {
 log.Error("No branches in non-empty repository %s", ctx.Repo.GitRepo.Path)
 } else {