fix & rename & more logging

Author: 6543

integrations/branches_test.go

@@ -57,7 +57,9 @@ func branchAction(t *testing.T, button string) (*HTMLDoc, string) {
 
 	htmlDoc := NewHTMLParser(t, resp.Body)
 	link, exists := htmlDoc.doc.Find(button).Attr("data-url")
-	assert.True(t, exists, "The template has changed")
+	if !assert.True(t, exists, "The template has changed") {
+		t.Skip()
+	}
 
 	req = NewRequestWithValues(t, "POST", link, map[string]string{
 		"_csrf": getCsrf(t, htmlDoc.doc),
@@ -69,7 +71,7 @@ func branchAction(t *testing.T, button string) (*HTMLDoc, string) {
 	req = NewRequest(t, "GET", "/user2/repo1/branches")
 	resp = session.MakeRequest(t, req, http.StatusOK)
 
-	return NewHTMLParser(t, resp.Body), url.Query()["name"][0]
+	return NewHTMLParser(t, resp.Body), url.Query().Get("name")
 }
 
 func getCsrf(t *testing.T, doc *goquery.Document) string {

modules/git/repo_branch_gogit.go

@@ -42,9 +42,10 @@ func (repo *Repository) GetBranches(skip, limit int) ([]string, int, error) {
 		if i < skip {
 			i++
 			return nil
-		} else if limit != 0 && i < skip+limit {
+		} else if limit != 0 && count > skip+limit {
 			return nil
 		}
+
 		branchNames = append(branchNames, strings.TrimPrefix(branch.Name().String(), BranchPrefix))
 		return nil
 	})

modules/repository/branch.go

@@ -13,6 +13,9 @@ import (
 
 // GetBranch returns a branch by its name
 func GetBranch(repo *models.Repository, branch string) (*git.Branch, error) {
+	if len(branch) == 0 {
+		return nil, fmt.Errorf("GetBranch: empty string for branch")
+	}
 	gitRepo, err := git.OpenRepository(repo.RepoPath())
 	if err != nil {
 		return nil, err

routers/repo/branch.go

@@ -58,12 +58,14 @@ func Branches(ctx *context.Context) {
 		page = 1
 	}
 
-	pageSize := ctx.QueryInt("limit")
-	if pageSize <= 0 || pageSize > git.BranchesRangeSize {
-		pageSize = git.BranchesRangeSize
+	limit := ctx.QueryInt("limit")
+	if limit <= 0 || limit > git.BranchesRangeSize {
+		limit = git.BranchesRangeSize
 	}
 
-	branches, branchesCount := loadBranches(ctx, page, pageSize)
+	skip := (page - 1) * limit
+	log.Info("Branches: skip: %d limit: %d", skip, limit)
+	branches, branchesCount := loadBranches(ctx, skip, limit)
 	if ctx.Written() {
 		return
 	}
@@ -80,6 +82,7 @@ func DeleteBranchPost(ctx *context.Context) {
 	defer redirect(ctx)
 	branchName := ctx.Query("name")
 	if branchName == ctx.Repo.Repository.DefaultBranch {
+		log.Warn("DeleteBranch: Can't delete default branch '%s'", branchName)
 		ctx.Flash.Error(ctx.Tr("repo.branch.default_deletion_failed", branchName))
 		return
 	}
@@ -92,16 +95,19 @@ func DeleteBranchPost(ctx *context.Context) {
 	}
 
 	if isProtected {
+		log.Warn("DeleteBranch: Can't delete protected branch '%s'", branchName)
 		ctx.Flash.Error(ctx.Tr("repo.branch.protected_deletion_failed", branchName))
 		return
 	}
 
-	if !ctx.Repo.GitRepo.IsBranchExist(branchName) || branchName == ctx.Repo.Repository.DefaultBranch {
+	if !ctx.Repo.GitRepo.IsBranchExist(branchName) {
+		log.Warn("DeleteBranch: Can't delete non existing branch '%s'", branchName)
 		ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName))
 		return
 	}
 
 	if err := deleteBranch(ctx, branchName); err != nil {
+		log.Error("DeleteBranch: %v", err)
 		ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName))
 		return
 	}
@@ -129,10 +135,11 @@ func RestoreBranchPost(ctx *context.Context) {
 		Env:    models.PushingEnvironment(ctx.User, ctx.Repo.Repository),
 	}); err != nil {
 		if strings.Contains(err.Error(), "already exists") {
+			log.Warn("RestoreBranch: Can't restore branch '%s', since one with same name already exist", deletedBranch.Name)
 			ctx.Flash.Error(ctx.Tr("repo.branch.already_exists", deletedBranch.Name))
 			return
 		}
-		log.Error("CreateBranch: %v", err)
+		log.Error("RestoreBranch: CreateBranch: %v", err)
 		ctx.Flash.Error(ctx.Tr("repo.branch.restore_failed", deletedBranch.Name))
 		return
 	}
@@ -148,7 +155,7 @@ func RestoreBranchPost(ctx *context.Context) {
 			RepoUserName: ctx.Repo.Owner.Name,
 			RepoName:     ctx.Repo.Repository.Name,
 		}); err != nil {
-		log.Error("Update: %v", err)
+		log.Error("RestoreBranch: Update: %v", err)
 	}
 
 	ctx.Flash.Success(ctx.Tr("repo.branch.restore_success", deletedBranch.Name))
@@ -197,17 +204,17 @@ func deleteBranch(ctx *context.Context, branchName string) error {
 
 // loadBranches loads branches from the repository limited by page & pageSize.
 // NOTE: May write to context on error.
-func loadBranches(ctx *context.Context, page, pageSize int) ([]*Branch, int) {
+func loadBranches(ctx *context.Context, skip, limit int) ([]*Branch, int) {
 	defaultBranch, err := repo_module.GetBranch(ctx.Repo.Repository, ctx.Repo.Repository.DefaultBranch)
 	if err != nil {
+		log.Error("loadBranches: get default branch: %v", err)
 		ctx.ServerError("GetDefaultBranch", err)
 		return nil, 0
 	}
 
-	skip, _ := models.ListOptions{Page: page, PageSize: pageSize}.GetStartEnd()
-
-	rawBranches, totalNumOfBranches, err := repo_module.GetBranches(ctx.Repo.Repository, skip, pageSize)
+	rawBranches, totalNumOfBranches, err := repo_module.GetBranches(ctx.Repo.Repository, skip, limit)
 	if err != nil {
+		log.Error("GetBranches: %v", err)
 		ctx.ServerError("GetBranches", err)
 		return nil, 0
 	}
@@ -226,7 +233,7 @@ func loadBranches(ctx *context.Context, page, pageSize int) ([]*Branch, int) {
 
 	var branches []*Branch
 	for i := range rawBranches {
-		if strings.EqualFold(rawBranches[i].Name, ctx.Repo.Repository.DefaultBranch) {
+		if rawBranches[i].Name == defaultBranch.Name {
 			// Skip default branch
 			continue
 		}
@@ -240,6 +247,7 @@ func loadBranches(ctx *context.Context, page, pageSize int) ([]*Branch, int) {
 	}
 
 	// Always add the default branch
+	log.Info("loadOneBranch: load default: '%s'", defaultBranch.Name)
 	branches = append(branches, loadOneBranch(ctx, defaultBranch, protectedBranches, repoIDToRepo, repoIDToGitRepo))
 
 	if ctx.Repo.CanWrite(models.UnitTypeCode) {
@@ -257,6 +265,7 @@ func loadBranches(ctx *context.Context, page, pageSize int) ([]*Branch, int) {
 func loadOneBranch(ctx *context.Context, rawBranch *git.Branch, protectedBranches []*models.ProtectedBranch,
 	repoIDToRepo map[int64]*models.Repository,
 	repoIDToGitRepo map[int64]*git.Repository) *Branch {
+	log.Info("loadOneBranch: '%s'", rawBranch.Name)
 
 	commit, err := rawBranch.GetCommit()
 	if err != nil {