added api to delete, edit or get a specific branch protection

Author: Antonello Tartamo

models/git/protected_branch.go

@@ -30,10 +30,10 @@ var ErrBranchIsProtected = errors.New("branch is protected")
 // ProtectedBranch struct
 type ProtectedBranch struct {
 	ID                            int64                  `xorm:"pk autoincr"`
-	RepoID                        int64                  `xorm:"UNIQUE(s) DEFAULT 0"`
-	OwnerID                       int64                  `xorm:"UNIQUE(s) DEFAULT 0"`
+	RepoID                        int64                  `xorm:"INDEX DEFAULT 0"`
+	OwnerID                       int64                  `xorm:"INDEX DEFAULT 0"`
 	Repo                          *repo_model.Repository `xorm:"-"`
-	RuleName                      string                 `xorm:"'branch_name' UNIQUE(s)"` // a branch name or a glob match to branch name
+	RuleName                      string                 `xorm:"'branch_name'"` // a branch name or a glob match to branch name
 	Priority                      int64                  `xorm:"NOT NULL DEFAULT 0"`
 	globRule                      glob.Glob              `xorm:"-"`
 	isPlainName                   bool                   `xorm:"-"`
@@ -327,25 +327,51 @@ func GetOrgProtectedBranchRuleByName(ctx context.Context, ownerID int64, ruleNam
 
 // GetProtectedBranchRuleByName getting protected branch rule by name
 func GetProtectedBranchRuleByName(ctx context.Context, repoID int64, ruleName string) (*ProtectedBranch, error) {
+	repo, err := repo_model.GetRepositoryByID(ctx, repoID)
+	if err != nil {
+		return nil, fmt.Errorf("GetRepositoryByID: %w", err)
+	}
+	if err := repo.LoadOwner(ctx); err != nil {
+		return nil, fmt.Errorf("LoadOwner: %w", err)
+	}
+	if repo.Owner.IsOrganization() {
+		if rel, exist, err := db.Get[ProtectedBranch](ctx, builder.Eq{"owner_id": repo.OwnerID, "branch_name": ruleName}); err != nil {
+			return nil, err
+		} else if exist {
+			return rel, nil
+		}
+	}
 	// branch_name is legacy name, it actually is rule name
 	rel, exist, err := db.Get[ProtectedBranch](ctx, builder.Eq{"repo_id": repoID, "branch_name": ruleName})
 	if err != nil {
 		return nil, err
-	} else if !exist {
-		return nil, nil
 	}
-	return rel, nil
+	return util.Iif(exist, rel, nil), nil
 }
 
 // GetProtectedBranchRuleByID getting protected branch rule by rule ID
 func GetProtectedBranchRuleByID(ctx context.Context, repoID, ruleID int64) (*ProtectedBranch, error) {
+	repo, err := repo_model.GetRepositoryByID(ctx, repoID)
+	if err != nil {
+		return nil, fmt.Errorf("GetRepositoryByID: %w", err)
+	}
+	if err := repo.LoadOwner(ctx); err != nil {
+		return nil, fmt.Errorf("LoadOwner: %w", err)
+	}
+	if repo.Owner.IsOrganization() {
+		if rel, exist, err := db.Get[ProtectedBranch](ctx, builder.Eq{"owner_id": repo.OwnerID, "id": ruleID}); err != nil {
+			return nil, err
+		} else if exist {
+			return rel, nil
+		}
+	}
+
 	rel, exist, err := db.Get[ProtectedBranch](ctx, builder.Eq{"repo_id": repoID, "id": ruleID})
 	if err != nil {
 		return nil, err
-	} else if !exist {
-		return nil, nil
 	}
-	return rel, nil
+
+	return util.Iif(exist, rel, nil), nil
 }
 
 // WhitelistOptions represent all sorts of whitelists used for protected branches
@@ -673,6 +699,22 @@ func updateOrgTeamWhitelist(ctx context.Context, org *organization.Organization,
 	return whitelist, nil
 }
 
+// DeleteProtectedBranch removes ProtectedBranch relation between the user and repository.
+func DeleteOrgProtectedBranch(ctx context.Context, org *organization.Organization, id int64) (err error) {
+	protectedBranch := &ProtectedBranch{
+		OwnerID: org.ID,
+		ID:      id,
+	}
+
+	if affected, err := db.GetEngine(ctx).Delete(protectedBranch); err != nil {
+		return err
+	} else if affected != 1 {
+		return fmt.Errorf("delete protected branch ID(%v) failed", id)
+	}
+
+	return nil
+}
+
 // DeleteProtectedBranch removes ProtectedBranch relation between the user and repository.
 func DeleteProtectedBranch(ctx context.Context, repo *repo_model.Repository, id int64) (err error) {
 	err = repo.MustNotBeArchived()

models/migrations/migrations.go

@@ -395,6 +395,7 @@ func prepareMigrationTasks() []*migration {
 		newMigration(321, "Use LONGTEXT for some columns and fix review_state.updated_files column", v1_25.UseLongTextInSomeColumnsAndFixBugs),
 		newMigration(322, "Extend comment tree_path length limit", v1_25.ExtendCommentTreePathLength),
 		newMigration(323, "Add support for actions concurrency", v1_25.AddActionsConcurrency),
+		newMigration(324, "Add owner_id to protected_branch", v1_25.AddOwnerIDToProtectedBranch),
 	}
 	return preparedMigrations
 }

models/migrations/v1_25/v324.go

@@ -0,0 +1,48 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_25
+
+import (
+	"code.gitea.io/gitea/modules/setting"
+
+	"xorm.io/xorm"
+)
+
+func AddOwnerIDToProtectedBranch(x *xorm.Engine) error {
+	type ProtectedBranch struct {
+		ID      int64 `xorm:"pk autoincr"`
+		RepoID  int64 `xorm:"INDEX DEFAULT 0"`
+		OwnerID int64 `xorm:"INDEX DEFAULT 0"`
+	}
+
+	if err := x.Sync(new(ProtectedBranch)); err != nil {
+		return err
+	}
+
+	db := x.NewSession()
+	defer db.Close()
+
+	if err := db.Begin(); err != nil {
+		return err
+	}
+
+	// Drop old unique index if it exists, ignoring errors if it doesn't.
+	if _, err := db.Exec("DROP INDEX `UQE_protected_branch_s`"); err != nil {
+		return err
+		//log.Warn("Could not drop index UQE_protected_branch_s: %v", err)
+	}
+
+	// These partial indexes might not be supported on all database versions, but they are the correct approach.
+	// We will assume modern database versions. The WHERE clause might need adjustment for MSSQL.
+	if !setting.Database.Type.IsMSSQL() {
+		if _, err := db.Exec("CREATE UNIQUE INDEX `UQE_protected_branch_repo_id_branch_name` ON `protected_branch` (`repo_id`, `branch_name`) WHERE `repo_id` != 0"); err != nil {
+			return err
+		}
+		if _, err := db.Exec("CREATE UNIQUE INDEX `UQE_protected_branch_owner_id_branch_name` ON `protected_branch` (`owner_id`, `branch_name`) WHERE `owner_id` != 0"); err != nil {
+			return err
+		}
+	}
+
+	return db.Commit()
+}

models/migrations/v1_25/v324_test.go

@@ -0,0 +1,106 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_25
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/migrations/base"
+	"code.gitea.io/gitea/modules/setting"
+
+	"github.com/stretchr/testify/assert"
+	"xorm.io/xorm"
+	"xorm.io/xorm/schemas"
+)
+
+func prepareOldProtectedBranch(t *testing.T) (*xorm.Engine, func()) {
+	type ProtectedBranch struct {
+		ID       int64  `xorm:"pk autoincr"`
+		RepoID   int64  `xorm:"UNIQUE(s)"`
+		RuleName string `xorm:"'branch_name' UNIQUE(s)"`
+	}
+
+	return base.PrepareTestEnv(t, 0, new(ProtectedBranch))
+}
+
+func Test_AddOwnerIDToProtectedBranch(t *testing.T) {
+	x, deferable := prepareOldProtectedBranch(t)
+	defer deferable()
+	if x == nil {
+		return
+	}
+
+	// The test fixtures will have already created the UQE_protected_branch_s index.
+	// We can run the migration.
+	assert.NoError(t, AddOwnerIDToProtectedBranch(x))
+
+	// Verify that the new column exists.
+	type ProtectedBranch struct {
+		ID       int64 `xorm:"pk autoincr"`
+		RepoID   int64 `xorm:"INDEX DEFAULT 0"`
+		OwnerID  int64 `xorm:"INDEX DEFAULT 0"`
+		RuleName string
+	}
+
+	has, err := x.Dialect().IsColumnExist(x.DB(), t.Context(), "protected_branch", "owner_id")
+	assert.NoError(t, err)
+	assert.True(t, has, "owner_id column should exist")
+
+	// Skip index check for unsupported DBs
+	if setting.Database.Type.IsMSSQL() || setting.Database.Type.IsSQLite3() {
+		t.Log("Skipping index check for unsupported database")
+		return
+	}
+
+	table, err := x.TableInfo("protected_branch")
+	assert.NoError(t, err)
+
+	// Check if the old index is gone
+	_, exist := table.Indexes["UQE_protected_branch_s"]
+	assert.False(t, exist, "old index UQE_protected_branch_s should not exist")
+
+	// Check if new indexes are created
+	idx, exist := table.Indexes["UQE_protected_branch_repo_id_branch_name"]
+	assert.True(t, exist, "new index UQE_protected_branch_repo_id_branch_name should exist")
+	if exist {
+		assert.Equal(t, schemas.UniqueType, idx.Type)
+		assert.ElementsMatch(t, []string{"repo_id", "branch_name"}, idx.Cols)
+	}
+
+	idx, exist = table.Indexes["UQE_protected_branch_owner_id_branch_name"]
+	assert.True(t, exist, "new index UQE_protected_branch_owner_id_branch_name should exist")
+	if exist {
+		assert.Equal(t, schemas.UniqueType, idx.Type)
+		assert.ElementsMatch(t, []string{"owner_id", "branch_name"}, idx.Cols)
+	}
+
+	// Test repo-level unique constraint
+	_, err = x.Insert(&ProtectedBranch{RepoID: 1, RuleName: "main"})
+	assert.NoError(t, err)
+	_, err = x.Insert(&ProtectedBranch{RepoID: 1, RuleName: "main"})
+	assert.Error(t, err, "should fail to insert duplicate repo-level rule")
+
+	// Test org-level unique constraint
+	_, err = x.Insert(&ProtectedBranch{OwnerID: 1, RuleName: "main"})
+	assert.NoError(t, err)
+	_, err = x.Insert(&ProtectedBranch{OwnerID: 1, RuleName: "main"})
+	assert.Error(t, err, "should fail to insert duplicate org-level rule")
+
+	// Test that repo-level and org-level rules with the same name don't conflict
+	_, err = x.Insert(&ProtectedBranch{RepoID: 2, RuleName: "develop"})
+	assert.NoError(t, err)
+	_, err = x.Insert(&ProtectedBranch{OwnerID: 2, RuleName: "develop"})
+	assert.NoError(t, err)
+
+	// Test that rules with repo_id=0 or owner_id=0 don't conflict with partial indexes
+	_, err = x.Insert(&ProtectedBranch{RepoID: 3, OwnerID: 0, RuleName: "feature-a"})
+	assert.NoError(t, err)
+	_, err = x.Insert(&ProtectedBranch{RepoID: 4, OwnerID: 0, RuleName: "feature-a"})
+	assert.NoError(t, err)
+
+	_, err = x.Insert(&ProtectedBranch{RepoID: 0, OwnerID: 3, RuleName: "feature-b"})
+	assert.NoError(t, err)
+	_, err = x.Insert(&ProtectedBranch{RepoID: 0, OwnerID: 4, RuleName: "feature-b"})
+	assert.NoError(t, err)
+}

routers/api/v1/api.go

@@ -1187,6 +1187,11 @@ func Routes() *web.Router {
 				m.Group("/branch_protections", func() {
 					m.Get("", repo.ListOrgBranchProtections)
 					m.Post("", bind(api.CreateBranchProtectionOption{}), repo.CreateOrgBranchProtection)
+					m.Group("/{name}", func() {
+						m.Get("", repo.GetOrgBranchProtection)
+						m.Patch("", bind(api.EditBranchProtectionOption{}), repo.EditOrgBranchProtection)
+						m.Delete("", repo.DeleteOrgBranchProtection)
+					})
 				}, reqToken(), reqOrgOwnership())
 			}, orgAssignment(true), tokenRequiresScopes(auth_model.AccessTokenScopeCategoryRepository))