Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Create new branch from branch selection dropdown #2130

Merged
merged 4 commits into from
Oct 15, 2017
Merged

Create new branch from branch selection dropdown #2130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
571cba5
Create new branch from branch selection dropdown and rewrite it to VueJS
Jul 9, 2017
b4cca45
Make updateLocalCopyToCommit as not exported
lafriks Oct 15, 2017
caccfb4
Move branch name validation to model
lafriks Oct 15, 2017
3d0a597
Fix possible race condition
lafriks Oct 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 132 additions & 0 deletions integrations/repo_branch_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
// Copyright 2017 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package integrations

import (
"net/http"
"path"
"strings"
"testing"

"github.com/Unknwon/i18n"
"github.com/stretchr/testify/assert"
)

func testCreateBranch(t *testing.T, session *TestSession, user, repo, oldRefName, newBranchName string, expectedStatus int) string {
var csrf string
if expectedStatus == http.StatusNotFound {
csrf = GetCSRF(t, session, path.Join(user, repo, "src/master"))
} else {
csrf = GetCSRF(t, session, path.Join(user, repo, "src", oldRefName))
}
req := NewRequestWithValues(t, "POST", path.Join(user, repo, "branches/_new", oldRefName), map[string]string{
"_csrf": csrf,
"new_branch_name": newBranchName,
})
resp := session.MakeRequest(t, req, expectedStatus)
if expectedStatus != http.StatusFound {
return ""
}
return RedirectURL(t, resp)
}

func TestCreateBranch(t *testing.T) {
tests := []struct {
OldBranchOrCommit string
NewBranch string
CreateRelease string
FlashMessage string
ExpectedStatus int
}{
{
OldBranchOrCommit: "master",
NewBranch: "feature/test1",
ExpectedStatus: http.StatusFound,
FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test1"),
},
{
OldBranchOrCommit: "master",
NewBranch: "",
ExpectedStatus: http.StatusFound,
FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.require_error"),
},
{
OldBranchOrCommit: "master",
NewBranch: "feature=test1",
ExpectedStatus: http.StatusFound,
FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.git_ref_name_error"),
},
{
OldBranchOrCommit: "master",
NewBranch: strings.Repeat("b", 101),
ExpectedStatus: http.StatusFound,
FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.max_size_error", "100"),
},
{
OldBranchOrCommit: "master",
NewBranch: "master",
ExpectedStatus: http.StatusFound,
FlashMessage: i18n.Tr("en", "repo.branch.branch_already_exists", "master"),
},
{
OldBranchOrCommit: "master",
NewBranch: "master/test",
ExpectedStatus: http.StatusFound,
FlashMessage: i18n.Tr("en", "repo.branch.branch_name_conflict", "master/test", "master"),
},
{
OldBranchOrCommit: "acd1d892867872cb47f3993468605b8aa59aa2e0",
NewBranch: "feature/test2",
ExpectedStatus: http.StatusNotFound,
},
{
OldBranchOrCommit: "65f1bf27bc3bf70f64657658635e66094edbcb4d",
NewBranch: "feature/test3",
ExpectedStatus: http.StatusFound,
FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test3"),
},
{
OldBranchOrCommit: "master",
NewBranch: "v1.0.0",
CreateRelease: "v1.0.0",
ExpectedStatus: http.StatusFound,
FlashMessage: i18n.Tr("en", "repo.branch.tag_collision", "v1.0.0"),
},
{
OldBranchOrCommit: "v1.0.0",
NewBranch: "feature/test4",
CreateRelease: "v1.0.0",
ExpectedStatus: http.StatusFound,
FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test4"),
},
}
for _, test := range tests {
prepareTestEnv(t)
session := loginUser(t, "user2")
if test.CreateRelease != "" {
createNewRelease(t, session, "/user2/repo1", test.CreateRelease, test.CreateRelease, false, false)
}
redirectURL := testCreateBranch(t, session, "user2", "repo1", test.OldBranchOrCommit, test.NewBranch, test.ExpectedStatus)
if test.ExpectedStatus == http.StatusFound {
req := NewRequest(t, "GET", redirectURL)
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
assert.Equal(t,
test.FlashMessage,
strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
)
}
}
}

func TestCreateBranchInvalidCSRF(t *testing.T) {
prepareTestEnv(t)
session := loginUser(t, "user2")
req := NewRequestWithValues(t, "POST", "user2/repo1/branches/_new/master", map[string]string{
"_csrf": "fake_csrf",
"new_branch_name": "test",
})
session.MakeRequest(t, req, http.StatusBadRequest)
}
45 changes: 45 additions & 0 deletions models/error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,51 @@ func (err ErrBranchNotExist) Error() string {
return fmt.Sprintf("branch does not exist [name: %s]", err.Name)
}

// ErrBranchAlreadyExists represents an error that branch with such name already exists
type ErrBranchAlreadyExists struct {
BranchName string
}

// IsErrBranchAlreadyExists checks if an error is an ErrBranchAlreadyExists.
func IsErrBranchAlreadyExists(err error) bool {
_, ok := err.(ErrBranchAlreadyExists)
return ok
}

func (err ErrBranchAlreadyExists) Error() string {
return fmt.Sprintf("branch already exists [name: %s]", err.BranchName)
}

// ErrBranchNameConflict represents an error that branch name conflicts with other branch
type ErrBranchNameConflict struct {
BranchName string
}

// IsErrBranchNameConflict checks if an error is an ErrBranchNameConflict.
func IsErrBranchNameConflict(err error) bool {
_, ok := err.(ErrBranchNameConflict)
return ok
}

func (err ErrBranchNameConflict) Error() string {
return fmt.Sprintf("branch conflicts with existing branch [name: %s]", err.BranchName)
}

// ErrTagAlreadyExists represents an error that tag with such name already exists
type ErrTagAlreadyExists struct {
TagName string
}

// IsErrTagAlreadyExists checks if an error is an ErrTagAlreadyExists.
func IsErrTagAlreadyExists(err error) bool {
_, ok := err.(ErrTagAlreadyExists)
return ok
}

func (err ErrTagAlreadyExists) Error() string {
return fmt.Sprintf("tag already exists [name: %s]", err.TagName)
}

// __ __ ___. .__ __
// / \ / \ ____\_ |__ | |__ ____ ____ | | __
// \ \/\/ // __ \| __ \| | \ / _ \ / _ \| |/ /
Expand Down
35 changes: 0 additions & 35 deletions models/repo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2426,38 +2426,3 @@ func (repo *Repository) GetUserFork(userID int64) (*Repository, error) {
}
return &forkedRepo, nil
}

// __________ .__
// \______ \____________ ____ ____ | |__
// | | _/\_ __ \__ \ / \_/ ___\| | \
// | | \ | | \// __ \| | \ \___| Y \
// |______ / |__| (____ /___| /\___ >___| /
// \/ \/ \/ \/ \/
//

// CreateNewBranch creates a new repository branch
func (repo *Repository) CreateNewBranch(doer *User, oldBranchName, branchName string) (err error) {
repoWorkingPool.CheckIn(com.ToStr(repo.ID))
defer repoWorkingPool.CheckOut(com.ToStr(repo.ID))

localPath := repo.LocalCopyPath()

if err = discardLocalRepoBranchChanges(localPath, oldBranchName); err != nil {
return fmt.Errorf("discardLocalRepoChanges: %v", err)
} else if err = repo.UpdateLocalCopyBranch(oldBranchName); err != nil {
return fmt.Errorf("UpdateLocalCopyBranch: %v", err)
}

if err = repo.CheckoutNewBranch(oldBranchName, branchName); err != nil {
return fmt.Errorf("CreateNewBranch: %v", err)
}

if err = git.Push(localPath, git.PushOptions{
Remote: "origin",
Branch: branchName,
}); err != nil {
return fmt.Errorf("Push: %v", err)
}

return nil
}
133 changes: 133 additions & 0 deletions models/repo_branch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@
package models

import (
"fmt"
"time"

"code.gitea.io/git"
"code.gitea.io/gitea/modules/setting"

"github.com/Unknwon/com"
)

// Branch holds the branch information
Expand Down Expand Up @@ -36,6 +42,11 @@ func GetBranchesByPath(path string) ([]*Branch, error) {
return branches, nil
}

// CanCreateBranch returns true if repository meets the requirements for creating new branches.
func (repo *Repository) CanCreateBranch() bool {
return !repo.IsMirror
}

// GetBranch returns a branch by it's name
func (repo *Repository) GetBranch(branch string) (*Branch, error) {
if !git.IsBranchExist(repo.RepoPath(), branch) {
Expand All @@ -52,6 +63,128 @@ func (repo *Repository) GetBranches() ([]*Branch, error) {
return GetBranchesByPath(repo.RepoPath())
}

// CheckBranchName validates branch name with existing repository branches
func (repo *Repository) CheckBranchName(name string) error {
gitRepo, err := git.OpenRepository(repo.RepoPath())
if err != nil {
return err
}

if _, err := gitRepo.GetTag(name); err == nil {
return ErrTagAlreadyExists{name}
}

branches, err := repo.GetBranches()
if err != nil {
return err
}

for _, branch := range branches {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing this, how about something like this that scales to thousands of branches?

if _, err := gitRepo.GetBranch(name); err != nil {
  return ErrConflict(name)
}
if strings.Contains(name, "/") {
  for index := strings.LastIndex(name, "/"); index > 0; index := strings.LastIndex(name, "/") {
    if conflict, err := gitRepo.GetBranch(name[:index]); err != nil {
      return ErrConflict(name[:index])
    }
  }
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not the same (if name is feat and there already exists branch feat/test) this code won't return error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One option is to just try the git branch ... command, and see if it succeeds or not. I assume that internally git has an efficient way to check for naming conflicts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that gitRepo.CreateBranch will throw a collision error no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then CheckBranchName will only provide part of validation (as I understand that that was an initial request to create it at first place to reuse it in other places later). Also currently GetBranches is called on every repo page load (in ctx RepoAssignment) so it's usage here anyway won't be that big deal

if branch.Name == name {
return ErrBranchAlreadyExists{branch.Name}
} else if (len(branch.Name) < len(name) && branch.Name+"/" == name[0:len(branch.Name)+1]) ||
(len(branch.Name) > len(name) && name+"/" == branch.Name[0:len(name)+1]) {
return ErrBranchNameConflict{branch.Name}
}
}
return nil
}

// CreateNewBranch creates a new repository branch
func (repo *Repository) CreateNewBranch(doer *User, oldBranchName, branchName string) (err error) {
repoWorkingPool.CheckIn(com.ToStr(repo.ID))
defer repoWorkingPool.CheckOut(com.ToStr(repo.ID))

// Check if branch name can be used
if err := repo.CheckBranchName(branchName); err != nil {
return err
}

localPath := repo.LocalCopyPath()

if err = discardLocalRepoBranchChanges(localPath, oldBranchName); err != nil {
return fmt.Errorf("discardLocalRepoChanges: %v", err)
} else if err = repo.UpdateLocalCopyBranch(oldBranchName); err != nil {
return fmt.Errorf("UpdateLocalCopyBranch: %v", err)
}

if err = repo.CheckoutNewBranch(oldBranchName, branchName); err != nil {
return fmt.Errorf("CreateNewBranch: %v", err)
}

if err = git.Push(localPath, git.PushOptions{
Remote: "origin",
Branch: branchName,
}); err != nil {
return fmt.Errorf("Push: %v", err)
}

return nil
}

// updateLocalCopyToCommit pulls latest changes of given commit from repoPath to localPath.
// It creates a new clone if local copy does not exist.
// This function checks out target commit by default, it is safe to assume subsequent
// operations are operating against target commit when caller has confidence for no race condition.
func updateLocalCopyToCommit(repoPath, localPath, commit string) error {
if !com.IsExist(localPath) {
if err := git.Clone(repoPath, localPath, git.CloneRepoOptions{
Timeout: time.Duration(setting.Git.Timeout.Clone) * time.Second,
}); err != nil {
return fmt.Errorf("git clone: %v", err)
}
} else {
_, err := git.NewCommand("fetch", "origin").RunInDir(localPath)
if err != nil {
return fmt.Errorf("git fetch origin: %v", err)
}
if err := git.ResetHEAD(localPath, true, "HEAD"); err != nil {
return fmt.Errorf("git reset --hard HEAD: %v", err)
}
}
if err := git.Checkout(localPath, git.CheckoutOptions{
Branch: commit,
}); err != nil {
return fmt.Errorf("git checkout %s: %v", commit, err)
}
return nil
}

// updateLocalCopyToCommit makes sure local copy of repository is at given commit.
func (repo *Repository) updateLocalCopyToCommit(commit string) error {
return updateLocalCopyToCommit(repo.RepoPath(), repo.LocalCopyPath(), commit)
}

// CreateNewBranchFromCommit creates a new repository branch
func (repo *Repository) CreateNewBranchFromCommit(doer *User, commit, branchName string) (err error) {
repoWorkingPool.CheckIn(com.ToStr(repo.ID))
defer repoWorkingPool.CheckOut(com.ToStr(repo.ID))

// Check if branch name can be used
if err := repo.CheckBranchName(branchName); err != nil {
return err
}

localPath := repo.LocalCopyPath()

if err = repo.updateLocalCopyToCommit(commit); err != nil {
return fmt.Errorf("UpdateLocalCopyBranch: %v", err)
}

if err = repo.CheckoutNewBranch(commit, branchName); err != nil {
return fmt.Errorf("CheckoutNewBranch: %v", err)
}

if err = git.Push(localPath, git.PushOptions{
Remote: "origin",
Branch: branchName,
}); err != nil {
return fmt.Errorf("Push: %v", err)
}

return nil
}

// GetCommit returns all the commits of a branch
func (branch *Branch) GetCommit() (*git.Commit, error) {
gitRepo, err := git.OpenRepository(branch.Path)
Expand Down
20 changes: 20 additions & 0 deletions modules/auth/repo_branch_form.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2017 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package auth

import (
"github.com/go-macaron/binding"
macaron "gopkg.in/macaron.v1"
)

// NewBranchForm form for creating a new branch
type NewBranchForm struct {
NewBranchName string `binding:"Required;MaxSize(100);GitRefName"`
}

// Validate validates the fields
func (f *NewBranchForm) Validate(ctx *macaron.Context, errs binding.Errors) binding.Errors {
return validate(errs, ctx.Data, f, ctx.Locale)
}
Loading