From 2036f4bb44eb9ce0accf68c48cf3539ccef28273 Mon Sep 17 00:00:00 2001 From: Terence Le Huu Phuong Date: Sun, 24 May 2020 17:07:46 +0200 Subject: [PATCH 1/7] [FEATURE] [API] Add Endpoint for Branch Creation Issue: https://github.com/go-gitea/gitea/issues/11376 This commit introduces an API endpoint for branch creation. The added route is POST /repos/{owner}/{repo}/branches. A JSON with the name of the new branch and the name of the old branch is required as parameters. Signed-off-by: Terence Le Huu Phuong --- modules/structs/repo.go | 16 ++++++ routers/api/v1/api.go | 1 + routers/api/v1/repo/branch.go | 86 +++++++++++++++++++++++++++++++ routers/api/v1/swagger/options.go | 3 ++ templates/swagger/v1_json.tmpl | 66 ++++++++++++++++++++++++ 5 files changed, 172 insertions(+) diff --git a/modules/structs/repo.go b/modules/structs/repo.go index 70de9b74694f7..832d330e749a2 100644 --- a/modules/structs/repo.go +++ b/modules/structs/repo.go @@ -160,6 +160,22 @@ type EditRepoOption struct { Archived *bool `json:"archived,omitempty"` } +// CreateBranchRepoOption options when creating a branch in a repository +// swagger:model +type CreateBranchRepoOption struct { + + // Name of the branch to create + // + // required: true + // unique: true + BranchName string `json:"new_branch_name" binding:"Required;GitRefName;MaxSize(100)"` + + // Name of the old branch to create from + // + // unique: true + OldBranchName string `json:"old_branch_name" binding:"GitRefName;MaxSize(100)"` +} + // TransferRepoOption options when transfer a repository's ownership // swagger:model type TransferRepoOption struct { diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 0d62b751cc898..2eb39f607008e 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -665,6 +665,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Get("", repo.ListBranches) m.Get("/*", context.RepoRefByType(context.RepoRefBranch), repo.GetBranch) m.Delete("/*", reqRepoWriter(models.UnitTypeCode), context.RepoRefByType(context.RepoRefBranch), repo.DeleteBranch) + m.Post("", reqRepoWriter(models.UnitTypeCode), bind(api.CreateBranchRepoOption{}), repo.CreateBranch) }, reqRepoReader(models.UnitTypeCode)) m.Group("/branch_protections", func() { m.Get("", repo.ListBranchProtections) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 57c74d7dab86a..8c27bf73074c9 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -182,6 +182,92 @@ func DeleteBranch(ctx *context.APIContext) { ctx.Status(http.StatusNoContent) } +// CreateRepoBranch creates a branch for a user's repository +func CreateRepoBranch(ctx *context.APIContext, opt api.CreateBranchRepoOption) { + + if len(opt.OldBranchName) == 0 { + opt.OldBranchName = ctx.Repo.Repository.DefaultBranch + } + + err := repo_module.CreateNewBranch(ctx.User, ctx.Repo.Repository, opt.OldBranchName, opt.BranchName) + + if err != nil { + if models.IsErrTagAlreadyExists(err) { + ctx.Error(http.StatusConflict, "", "The branch with the same tag already exists.") + + } else if models.IsErrBranchAlreadyExists(err) || git.IsErrPushOutOfDate(err) { + ctx.Error(http.StatusConflict, "", "The branch already exists.") + + } else if models.IsErrBranchNameConflict(err) { + ctx.Error(http.StatusConflict, "", "The branch with the same name already exists.") + + } else { + ctx.Error(http.StatusInternalServerError, "CreateRepoBranch", err) + + } + return + } + + branch, err := repo_module.GetBranch(ctx.Repo.Repository, opt.BranchName) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetBranch", err) + return + } + + commit, err := branch.GetCommit() + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetCommit", err) + return + } + + branchProtection, err := ctx.Repo.Repository.GetBranchProtection(branch.Name) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetBranchProtection", err) + return + } + + br, err := convert.ToBranch(ctx.Repo.Repository, branch, commit, branchProtection, ctx.User, ctx.Repo.IsAdmin()) + if err != nil { + ctx.Error(http.StatusInternalServerError, "convert.ToBranch", err) + return + } + + ctx.JSON(http.StatusCreated, br) +} + +// CreateBranch create a branch of a repository +func CreateBranch(ctx *context.APIContext, opt api.CreateBranchRepoOption) { + // swagger:operation POST /repos/{owner}/{repo}/branches repository repoCreateBranch + // --- + // summary: Create a branch + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: body + // in: body + // schema: + // "$ref": "#/definitions/CreateBranchRepoOption" + // responses: + // "201": + // "$ref": "#/responses/Branch" + // "409": + // description: The branch with the same name already exists. + + CreateRepoBranch(ctx, opt) +} + // ListBranches list all the branches of a repository func ListBranches(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/branches repository repoListBranches diff --git a/routers/api/v1/swagger/options.go b/routers/api/v1/swagger/options.go index f13dc63864366..d9ef05c335991 100644 --- a/routers/api/v1/swagger/options.go +++ b/routers/api/v1/swagger/options.go @@ -129,6 +129,9 @@ type swaggerParameterBodies struct { // in:body EditReactionOption api.EditReactionOption + // in:body + CreateBranchRepoOption api.CreateBranchRepoOption + // in:body CreateBranchProtectionOption api.CreateBranchProtectionOption diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 0cbe33bd247ba..7590f19d5df08 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -2241,6 +2241,50 @@ "$ref": "#/responses/BranchList" } } + }, + "post": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Create a branch", + "operationId": "repoCreateBranch", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/CreateBranchRepoOption" + } + } + ], + "responses": { + "201": { + "$ref": "#/responses/Branch" + }, + "409": { + "description": "The branch with the same name already exists." + } + } } }, "/repos/{owner}/{repo}/branches/{branch}": { @@ -10886,6 +10930,28 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "CreateBranchRepoOption": { + "description": "CreateBranchRepoOption options when creating a branch in a repository", + "type": "object", + "required": [ + "new_branch_name" + ], + "properties": { + "new_branch_name": { + "description": "Name of the branch to create", + "type": "string", + "uniqueItems": true, + "x-go-name": "BranchName" + }, + "old_branch_name": { + "description": "Name of the old branch to create from", + "type": "string", + "uniqueItems": true, + "x-go-name": "OldBranchName" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "CreateEmailOption": { "description": "CreateEmailOption options when creating email addresses", "type": "object", From 19483e19c82ca44ea42805f049ad8412b22dcde2 Mon Sep 17 00:00:00 2001 From: Terence Le Huu Phuong Date: Thu, 28 May 2020 16:30:58 +0200 Subject: [PATCH 2/7] Put all the logic into CreateBranch and removed CreateRepoBranch --- routers/api/v1/repo/branch.go | 62 ++++++++++++++++------------------- 1 file changed, 28 insertions(+), 34 deletions(-) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 8c27bf73074c9..554340e1aac07 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -183,7 +183,34 @@ func DeleteBranch(ctx *context.APIContext) { } // CreateRepoBranch creates a branch for a user's repository -func CreateRepoBranch(ctx *context.APIContext, opt api.CreateBranchRepoOption) { +func CreateBranch(ctx *context.APIContext, opt api.CreateBranchRepoOption) { + // swagger:operation POST /repos/{owner}/{repo}/branches repository repoCreateBranch + // --- + // summary: Create a branch + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: body + // in: body + // schema: + // "$ref": "#/definitions/CreateBranchRepoOption" + // responses: + // "201": + // "$ref": "#/responses/Branch" + // "409": + // description: The branch with the same name already exists. if len(opt.OldBranchName) == 0 { opt.OldBranchName = ctx.Repo.Repository.DefaultBranch @@ -235,39 +262,6 @@ func CreateRepoBranch(ctx *context.APIContext, opt api.CreateBranchRepoOption) { ctx.JSON(http.StatusCreated, br) } -// CreateBranch create a branch of a repository -func CreateBranch(ctx *context.APIContext, opt api.CreateBranchRepoOption) { - // swagger:operation POST /repos/{owner}/{repo}/branches repository repoCreateBranch - // --- - // summary: Create a branch - // consumes: - // - application/json - // produces: - // - application/json - // parameters: - // - name: owner - // in: path - // description: owner of the repo - // type: string - // required: true - // - name: repo - // in: path - // description: name of the repo - // type: string - // required: true - // - name: body - // in: body - // schema: - // "$ref": "#/definitions/CreateBranchRepoOption" - // responses: - // "201": - // "$ref": "#/responses/Branch" - // "409": - // description: The branch with the same name already exists. - - CreateRepoBranch(ctx, opt) -} - // ListBranches list all the branches of a repository func ListBranches(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/branches repository repoListBranches From fc682dc1dab18cf48ff16d272dab6e4ff8ab3939 Mon Sep 17 00:00:00 2001 From: Terence Le Huu Phuong Date: Thu, 28 May 2020 16:33:54 +0200 Subject: [PATCH 3/7] - Added the error ErrBranchDoesNotExist in error.go - Made the CreateNewBranch function return an errBranchDoesNotExist error when the OldBranch does not exist - Made the CreateBranch API function checks that the repository is not empty and that branch exists. --- models/error.go | 15 +++++++++++++++ modules/repository/branch.go | 4 +++- routers/api/v1/repo/branch.go | 12 +++++++++++- templates/swagger/v1_json.tmpl | 3 +++ 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/models/error.go b/models/error.go index 3b05a7152cba4..e9343cbe7c680 100644 --- a/models/error.go +++ b/models/error.go @@ -995,6 +995,21 @@ func IsErrWontSign(err error) bool { // |______ / |__| (____ /___| /\___ >___| / // \/ \/ \/ \/ \/ +// ErrBranchDoesNotExist represents an error that branch with such name does not exist. +type ErrBranchDoesNotExist struct { + BranchName string +} + +// IsErrBranchDoesNotExist checks if an error is an ErrBranchDoesNotExist. +func IsErrBranchDoesNotExist(err error) bool { + _, ok := err.(ErrBranchDoesNotExist) + return ok +} + +func (err ErrBranchDoesNotExist) Error() string { + return fmt.Sprintf("branch does not exist [name: %s]", err.BranchName) +} + // ErrBranchAlreadyExists represents an error that branch with such name already exists. type ErrBranchAlreadyExists struct { BranchName string diff --git a/modules/repository/branch.go b/modules/repository/branch.go index 418ba25c89d54..94be6f0f5a8e0 100644 --- a/modules/repository/branch.go +++ b/modules/repository/branch.go @@ -71,7 +71,9 @@ func CreateNewBranch(doer *models.User, repo *models.Repository, oldBranchName, } if !git.IsBranchExist(repo.RepoPath(), oldBranchName) { - return fmt.Errorf("OldBranch: %s does not exist. Cannot create new branch from this", oldBranchName) + return models.ErrBranchDoesNotExist{ + BranchName: oldBranchName, + } } basePath, err := models.CreateTemporaryPath("branch-maker") diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 554340e1aac07..90db597ef7cb7 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -182,7 +182,7 @@ func DeleteBranch(ctx *context.APIContext) { ctx.Status(http.StatusNoContent) } -// CreateRepoBranch creates a branch for a user's repository +// CreateBranch creates a branch for a user's repository func CreateBranch(ctx *context.APIContext, opt api.CreateBranchRepoOption) { // swagger:operation POST /repos/{owner}/{repo}/branches repository repoCreateBranch // --- @@ -209,9 +209,16 @@ func CreateBranch(ctx *context.APIContext, opt api.CreateBranchRepoOption) { // responses: // "201": // "$ref": "#/responses/Branch" + // "404": + // description: The old branch does not exist. // "409": // description: The branch with the same name already exists. + if ctx.Repo.Repository.IsEmpty { + ctx.Error(http.StatusNotFound, "", "Git Repository is empty.") + return + } + if len(opt.OldBranchName) == 0 { opt.OldBranchName = ctx.Repo.Repository.DefaultBranch } @@ -219,6 +226,9 @@ func CreateBranch(ctx *context.APIContext, opt api.CreateBranchRepoOption) { err := repo_module.CreateNewBranch(ctx.User, ctx.Repo.Repository, opt.OldBranchName, opt.BranchName) if err != nil { + if models.IsErrBranchDoesNotExist(err) { + ctx.Error(http.StatusNotFound, "", "The old branch does not exist") + } if models.IsErrTagAlreadyExists(err) { ctx.Error(http.StatusConflict, "", "The branch with the same tag already exists.") diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 7590f19d5df08..70f12b083f5b7 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -2281,6 +2281,9 @@ "201": { "$ref": "#/responses/Branch" }, + "404": { + "description": "The old branch does not exist." + }, "409": { "description": "The branch with the same name already exists." } From 32e3057ce357d5aff355c879754e0845788bbb22 Mon Sep 17 00:00:00 2001 From: Terence Le Huu Phuong Date: Thu, 28 May 2020 17:39:38 +0200 Subject: [PATCH 4/7] - Added a resetFixtures helper function in integration_test.go to fine-tune test env resetting - Added api test for CreateBranch - Used resetFixture instead of the more general prepareTestEnv in the repo_branch_test CreateBranch tests --- integrations/api_branch_test.go | 70 +++++++++++++++++++++++++++++++- integrations/integration_test.go | 12 ++++++ integrations/repo_branch_test.go | 2 +- 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/integrations/api_branch_test.go b/integrations/api_branch_test.go index acf7525f80052..5b0fdc0192307 100644 --- a/integrations/api_branch_test.go +++ b/integrations/api_branch_test.go @@ -5,11 +5,11 @@ package integrations import ( + api "code.gitea.io/gitea/modules/structs" "net/http" + "net/url" "testing" - api "code.gitea.io/gitea/modules/structs" - "github.com/stretchr/testify/assert" ) @@ -100,6 +100,72 @@ func TestAPIGetBranch(t *testing.T) { } } +func TestAPICreateBranch(t *testing.T) { + onGiteaRun(t, testAPICreateBranches) +} + +func testAPICreateBranches(t *testing.T, giteaURL *url.URL) { + defer resetFixtures(t) + + username := "user2" + ctx := NewAPITestContext(t, username, "my-noo-repo") + giteaURL.Path = ctx.GitPath() + + t.Run("CreateRepo", doAPICreateRepository(ctx, false)) + tests := []struct { + OldBranch string + NewBranch string + ExpectedHTTPStatus int + }{ + // Creating branch from default branch + { + OldBranch: "", + NewBranch: "new_branch_from_default_branch", + ExpectedHTTPStatus: http.StatusCreated, + }, + // Creating branch from master + { + OldBranch: "master", + NewBranch: "new_branch_from_master_1", + ExpectedHTTPStatus: http.StatusCreated, + }, + // Trying to create from master but already exists + { + OldBranch: "master", + NewBranch: "new_branch_from_master_1", + ExpectedHTTPStatus: http.StatusConflict, + }, + // Trying to create from other branch (not default branch) + { + OldBranch: "new_branch_from_master_1", + NewBranch: "branch_2", + ExpectedHTTPStatus: http.StatusCreated, + }, + // Trying to create from a branch which does not exist + { + OldBranch: "does_not_exist", + NewBranch: "new_branch_from_non_existent", + ExpectedHTTPStatus: http.StatusNotFound, + }, + } + for _, test := range tests { + session := ctx.Session + token := getTokenForLoggedInUser(t, session) + req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/my-noo-repo/branches?token="+token, &api.CreateBranchRepoOption{ + BranchName: test.NewBranch, + OldBranchName: test.OldBranch, + }) + resp := session.MakeRequest(t, req, test.ExpectedHTTPStatus) + + var branch api.Branch + DecodeJSON(t, resp, &branch) + + if test.ExpectedHTTPStatus == http.StatusCreated { + assert.EqualValues(t, test.NewBranch, branch.Name) + } + } +} + func TestAPIBranchProtection(t *testing.T) { defer prepareTestEnv(t)() diff --git a/integrations/integration_test.go b/integrations/integration_test.go index c6a4169751b5c..3c0125af6c712 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -26,6 +26,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/graceful" + "code.gitea.io/gitea/modules/queue" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/routers" "code.gitea.io/gitea/routers/routes" @@ -459,3 +460,14 @@ func GetCSRF(t testing.TB, session *TestSession, urlStr string) string { doc := NewHTMLParser(t, resp.Body) return doc.GetCSRF() } + +// resetFixtures flushes queues, reloads fixtures and resets test repositories within a single test. +// Most tests should call defer prepareTestEnv(t)() (or have onGiteaRun do that for them) but sometimes +// within a single test this is required +func resetFixtures(t *testing.T) { + assert.NoError(t, queue.GetManager().FlushAll(context.Background(), -1)) + assert.NoError(t, models.LoadFixtures()) + assert.NoError(t, os.RemoveAll(setting.RepoRootPath)) + assert.NoError(t, com.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), + setting.RepoRootPath)) +} diff --git a/integrations/repo_branch_test.go b/integrations/repo_branch_test.go index 68806f347bd3c..1392d518ca4cd 100644 --- a/integrations/repo_branch_test.go +++ b/integrations/repo_branch_test.go @@ -110,7 +110,7 @@ func testCreateBranches(t *testing.T, giteaURL *url.URL) { }, } for _, test := range tests { - defer prepareTestEnv(t)() + defer resetFixtures(t) session := loginUser(t, "user2") if test.CreateRelease != "" { createNewRelease(t, session, "/user2/repo1", test.CreateRelease, test.CreateRelease, false, false) From f6100a1a1bc1119fd33055d8939984f3c9d5fb23 Mon Sep 17 00:00:00 2001 From: Terence Le Huu Phuong Date: Fri, 29 May 2020 00:37:36 +0200 Subject: [PATCH 5/7] Moved the resetFixtures call inside the loop for APICreateBranch function --- integrations/api_branch_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/api_branch_test.go b/integrations/api_branch_test.go index 5b0fdc0192307..2e5a7b02f97f7 100644 --- a/integrations/api_branch_test.go +++ b/integrations/api_branch_test.go @@ -105,7 +105,6 @@ func TestAPICreateBranch(t *testing.T) { } func testAPICreateBranches(t *testing.T, giteaURL *url.URL) { - defer resetFixtures(t) username := "user2" ctx := NewAPITestContext(t, username, "my-noo-repo") @@ -149,6 +148,7 @@ func testAPICreateBranches(t *testing.T, giteaURL *url.URL) { }, } for _, test := range tests { + defer resetFixtures(t) session := ctx.Session token := getTokenForLoggedInUser(t, session) req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/my-noo-repo/branches?token="+token, &api.CreateBranchRepoOption{ From c87a2b4e83dc2b0a6d88170b0d1e60d0d09adb5b Mon Sep 17 00:00:00 2001 From: Terence Le Huu Phuong Date: Fri, 29 May 2020 01:20:13 +0200 Subject: [PATCH 6/7] Put the prepareTestEnv back in repo_branch_test --- integrations/repo_branch_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/repo_branch_test.go b/integrations/repo_branch_test.go index 1392d518ca4cd..68806f347bd3c 100644 --- a/integrations/repo_branch_test.go +++ b/integrations/repo_branch_test.go @@ -110,7 +110,7 @@ func testCreateBranches(t *testing.T, giteaURL *url.URL) { }, } for _, test := range tests { - defer resetFixtures(t) + defer prepareTestEnv(t)() session := loginUser(t, "user2") if test.CreateRelease != "" { createNewRelease(t, session, "/user2/repo1", test.CreateRelease, test.CreateRelease, false, false) From 96513051b04b07bf558fff65b9c827d7355fd517 Mon Sep 17 00:00:00 2001 From: Terence Le Huu Phuong Date: Fri, 29 May 2020 11:53:45 +0200 Subject: [PATCH 7/7] fix import order/sort api branch test --- integrations/api_branch_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/integrations/api_branch_test.go b/integrations/api_branch_test.go index 2e5a7b02f97f7..26d8fb4b451f9 100644 --- a/integrations/api_branch_test.go +++ b/integrations/api_branch_test.go @@ -5,11 +5,12 @@ package integrations import ( - api "code.gitea.io/gitea/modules/structs" "net/http" "net/url" "testing" + api "code.gitea.io/gitea/modules/structs" + "github.com/stretchr/testify/assert" )