Merge branch 'main' into lunny/fix_delete_branch_perm

Author: lunny

models/git/protected_branch.go

@@ -34,6 +34,7 @@ type ProtectedBranch struct {
 	RepoID                        int64                  `xorm:"UNIQUE(s)"`
 	Repo                          *repo_model.Repository `xorm:"-"`
 	RuleName                      string                 `xorm:"'branch_name' UNIQUE(s)"` // a branch name or a glob match to branch name
+	Priority                      int64                  `xorm:"NOT NULL DEFAULT 0"`
 	globRule                      glob.Glob              `xorm:"-"`
 	isPlainName                   bool                   `xorm:"-"`
 	CanPush                       bool                   `xorm:"NOT NULL DEFAULT false"`
@@ -413,21 +414,52 @@ func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, prote
 	}
 	protectBranch.ApprovalsWhitelistTeamIDs = whitelist
 
-	// Make sure protectBranch.ID is not 0 for whitelists
+	// Looks like it's a new rule
 	if protectBranch.ID == 0 {
+		// as it's a new rule and if priority was not set, we need to calc it.
+		if protectBranch.Priority == 0 {
+			var lowestPrio int64
+			// because of mssql we can not use builder or save xorm syntax, so raw sql it is
+			if _, err := db.GetEngine(ctx).SQL(`SELECT MAX(priority) FROM protected_branch WHERE repo_id = ?`, protectBranch.RepoID).
+				Get(&lowestPrio); err != nil {
+				return err
+			}
+			log.Trace("Create new ProtectedBranch at repo[%d] and detect current lowest priority '%d'", protectBranch.RepoID, lowestPrio)
+			protectBranch.Priority = lowestPrio + 1
+		}
+
 		if _, err = db.GetEngine(ctx).Insert(protectBranch); err != nil {
 			return fmt.Errorf("Insert: %v", err)
 		}
 		return nil
 	}
 
+	// update the rule
 	if _, err = db.GetEngine(ctx).ID(protectBranch.ID).AllCols().Update(protectBranch); err != nil {
 		return fmt.Errorf("Update: %v", err)
 	}
 
 	return nil
 }
 
+func UpdateProtectBranchPriorities(ctx context.Context, repo *repo_model.Repository, ids []int64) error {
+	prio := int64(1)
+	return db.WithTx(ctx, func(ctx context.Context) error {
+		for _, id := range ids {
+			if _, err := db.GetEngine(ctx).
+				ID(id).Where("repo_id = ?", repo.ID).
+				Cols("priority").
+				Update(&ProtectedBranch{
+					Priority: prio,
+				}); err != nil {
+				return err
+			}
+			prio++
+		}
+		return nil
+	})
+}
+
 // updateApprovalWhitelist checks whether the user whitelist changed and returns a whitelist with
 // the users from newWhitelist which have explicit read or write access to the repo.
 func updateApprovalWhitelist(ctx context.Context, repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {

models/git/protected_branch_list.go

@@ -28,6 +28,13 @@ func (rules ProtectedBranchRules) sort() {
 	sort.Slice(rules, func(i, j int) bool {
 		rules[i].loadGlob()
 		rules[j].loadGlob()
+
+		// if priority differ, use that to sort
+		if rules[i].Priority != rules[j].Priority {
+			return rules[i].Priority < rules[j].Priority
+		}
+
+		// now we sort the old way
 		if rules[i].isPlainName != rules[j].isPlainName {
 			return rules[i].isPlainName // plain name comes first, so plain name means "less"
 		}

models/git/protected_branch_list_test.go

@@ -75,7 +75,7 @@ func TestBranchRuleMatchPriority(t *testing.T) {
 	}
 }
 
-func TestBranchRuleSort(t *testing.T) {
+func TestBranchRuleSortLegacy(t *testing.T) {
 	in := []*ProtectedBranch{{
 		RuleName:    "b",
 		CreatedUnix: 1,
@@ -103,3 +103,37 @@ func TestBranchRuleSort(t *testing.T) {
 	}
 	assert.Equal(t, expect, got)
 }
+
+func TestBranchRuleSortPriority(t *testing.T) {
+	in := []*ProtectedBranch{{
+		RuleName:    "b",
+		CreatedUnix: 1,
+		Priority:    4,
+	}, {
+		RuleName:    "b/*",
+		CreatedUnix: 3,
+		Priority:    2,
+	}, {
+		RuleName:    "a/*",
+		CreatedUnix: 2,
+		Priority:    1,
+	}, {
+		RuleName:    "c",
+		CreatedUnix: 0,
+		Priority:    0,
+	}, {
+		RuleName:    "a",
+		CreatedUnix: 4,
+		Priority:    3,
+	}}
+	expect := []string{"c", "a/*", "b/*", "a", "b"}
+
+	pbr := ProtectedBranchRules(in)
+	pbr.sort()
+
+	var got []string
+	for i := range pbr {
+		got = append(got, pbr[i].RuleName)
+	}
+	assert.Equal(t, expect, got)
+}

models/git/protected_branch_test.go

@@ -7,6 +7,10 @@ import (
 	"fmt"
 	"testing"
 
+	"code.gitea.io/gitea/models/db"
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unittest"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -76,3 +80,77 @@ func TestBranchRuleMatch(t *testing.T) {
 		)
 	}
 }
+
+func TestUpdateProtectBranchPriorities(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+
+	// Create some test protected branches with initial priorities
+	protectedBranches := []*ProtectedBranch{
+		{
+			RepoID:   repo.ID,
+			RuleName: "master",
+			Priority: 1,
+		},
+		{
+			RepoID:   repo.ID,
+			RuleName: "develop",
+			Priority: 2,
+		},
+		{
+			RepoID:   repo.ID,
+			RuleName: "feature/*",
+			Priority: 3,
+		},
+	}
+
+	for _, pb := range protectedBranches {
+		_, err := db.GetEngine(db.DefaultContext).Insert(pb)
+		assert.NoError(t, err)
+	}
+
+	// Test updating priorities
+	newPriorities := []int64{protectedBranches[2].ID, protectedBranches[0].ID, protectedBranches[1].ID}
+	err := UpdateProtectBranchPriorities(db.DefaultContext, repo, newPriorities)
+	assert.NoError(t, err)
+
+	// Verify new priorities
+	pbs, err := FindRepoProtectedBranchRules(db.DefaultContext, repo.ID)
+	assert.NoError(t, err)
+
+	expectedPriorities := map[string]int64{
+		"feature/*": 1,
+		"master":    2,
+		"develop":   3,
+	}
+
+	for _, pb := range pbs {
+		assert.Equal(t, expectedPriorities[pb.RuleName], pb.Priority)
+	}
+}
+
+func TestNewProtectBranchPriority(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+
+	err := UpdateProtectBranch(db.DefaultContext, repo, &ProtectedBranch{
+		RepoID:   repo.ID,
+		RuleName: "branch-1",
+		Priority: 1,
+	}, WhitelistOptions{})
+	assert.NoError(t, err)
+
+	newPB := &ProtectedBranch{
+		RepoID:   repo.ID,
+		RuleName: "branch-2",
+		// Priority intentionally omitted
+	}
+
+	err = UpdateProtectBranch(db.DefaultContext, repo, newPB, WhitelistOptions{})
+	assert.NoError(t, err)
+
+	savedPB2, err := GetFirstMatchProtectedBranchRule(db.DefaultContext, repo.ID, "branch-2")
+	assert.NoError(t, err)
+	assert.Equal(t, int64(2), savedPB2.Priority)
+}

models/migrations/migrations.go

@@ -367,6 +367,7 @@ func prepareMigrationTasks() []*migration {
 		newMigration(307, "Fix milestone deadline_unix when there is no due date", v1_23.FixMilestoneNoDueDate),
 		newMigration(308, "Add index(user_id, is_deleted) for action table", v1_23.AddNewIndexForUserDashboard),
 		newMigration(309, "Improve Notification table indices", v1_23.ImproveNotificationTableIndices),
+		newMigration(310, "Add Priority to ProtectedBranch", v1_23.AddPriorityToProtectedBranch),
 	}
 	return preparedMigrations
 }

models/migrations/v1_23/v310.go

@@ -0,0 +1,16 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_23 //nolint
+
+import (
+	"xorm.io/xorm"
+)
+
+func AddPriorityToProtectedBranch(x *xorm.Engine) error {
+	type ProtectedBranch struct {
+		Priority int64 `xorm:"NOT NULL DEFAULT 0"`
+	}
+
+	return x.Sync(new(ProtectedBranch))
+}

models/organization/team_test.go

@@ -197,3 +197,8 @@ func TestUsersInTeamsCount(t *testing.T) {
 	test([]int64{1, 2, 3, 4, 5}, []int64{2, 5}, 2)    // userid 2,4
 	test([]int64{1, 2, 3, 4, 5}, []int64{2, 3, 5}, 3) // userid 2,4,5
 }
+
+func TestIsUsableTeamName(t *testing.T) {
+	assert.NoError(t, organization.IsUsableTeamName("usable"))
+	assert.True(t, db.IsErrNameReserved(organization.IsUsableTeamName("new")))
+}

modules/structs/repo_branch.go

@@ -25,6 +25,7 @@ type BranchProtection struct {
 	// Deprecated: true
 	BranchName                    string   `json:"branch_name"`
 	RuleName                      string   `json:"rule_name"`
+	Priority                      int64    `json:"priority"`
 	EnablePush                    bool     `json:"enable_push"`
 	EnablePushWhitelist           bool     `json:"enable_push_whitelist"`
 	PushWhitelistUsernames        []string `json:"push_whitelist_usernames"`
@@ -64,6 +65,7 @@ type CreateBranchProtectionOption struct {
 	// Deprecated: true
 	BranchName                    string   `json:"branch_name"`
 	RuleName                      string   `json:"rule_name"`
+	Priority                      int64    `json:"priority"`
 	EnablePush                    bool     `json:"enable_push"`
 	EnablePushWhitelist           bool     `json:"enable_push_whitelist"`
 	PushWhitelistUsernames        []string `json:"push_whitelist_usernames"`
@@ -96,6 +98,7 @@ type CreateBranchProtectionOption struct {
 
 // EditBranchProtectionOption options for editing a branch protection
 type EditBranchProtectionOption struct {
+	Priority                      *int64   `json:"priority"`
 	EnablePush                    *bool    `json:"enable_push"`
 	EnablePushWhitelist           *bool    `json:"enable_push_whitelist"`
 	PushWhitelistUsernames        []string `json:"push_whitelist_usernames"`
@@ -125,3 +128,8 @@ type EditBranchProtectionOption struct {
 	UnprotectedFilePatterns       *string  `json:"unprotected_file_patterns"`
 	BlockAdminMergeOverride       *bool    `json:"block_admin_merge_override"`
 }
+
+// UpdateBranchProtectionPriories a list to update the branch protection rule priorities
+type UpdateBranchProtectionPriories struct {
+	IDs []int64 `json:"ids"`
+}

modules/util/truncate.go

@@ -41,6 +41,8 @@ func SplitStringAtByteN(input string, n int) (left, right string) {
 
 // SplitTrimSpace splits the string at given separator and trims leading and trailing space
 func SplitTrimSpace(input, sep string) []string {
+	// Trim initial leading & trailing space
+	input = strings.TrimSpace(input)
 	// replace CRLF with LF
 	input = strings.ReplaceAll(input, "\r\n", "\n")
 

modules/validation/binding.go

@@ -10,6 +10,7 @@ import (
 
 	"code.gitea.io/gitea/modules/auth"
 	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/util"
 
 	"gitea.com/go-chi/binding"
 	"github.com/gobwas/glob"
@@ -31,6 +32,7 @@ const (
 // AddBindingRules adds additional binding rules
 func AddBindingRules() {
 	addGitRefNameBindingRule()
+	addValidURLListBindingRule()
 	addValidURLBindingRule()
 	addValidSiteURLBindingRule()
 	addGlobPatternRule()
@@ -44,7 +46,7 @@ func addGitRefNameBindingRule() {
 	// Git refname validation rule
 	binding.AddRule(&binding.Rule{
 		IsMatch: func(rule string) bool {
-			return strings.HasPrefix(rule, "GitRefName")
+			return rule == "GitRefName"
 		},
 		IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) {
 			str := fmt.Sprintf("%v", val)
@@ -58,11 +60,38 @@ func addGitRefNameBindingRule() {
 	})
 }
 
+func addValidURLListBindingRule() {
+	// URL validation rule
+	binding.AddRule(&binding.Rule{
+		IsMatch: func(rule string) bool {
+			return rule == "ValidUrlList"
+		},
+		IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) {
+			str := fmt.Sprintf("%v", val)
+			if len(str) == 0 {
+				errs.Add([]string{name}, binding.ERR_URL, "Url")
+				return false, errs
+			}
+
+			ok := true
+			urls := util.SplitTrimSpace(str, "\n")
+			for _, u := range urls {
+				if !IsValidURL(u) {
+					ok = false
+					errs.Add([]string{name}, binding.ERR_URL, u)
+				}
+			}
+
+			return ok, errs
+		},
+	})
+}
+
 func addValidURLBindingRule() {
 	// URL validation rule
 	binding.AddRule(&binding.Rule{
 		IsMatch: func(rule string) bool {
-			return strings.HasPrefix(rule, "ValidUrl")
+			return rule == "ValidUrl"
 		},
 		IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) {
 			str := fmt.Sprintf("%v", val)
@@ -80,7 +109,7 @@ func addValidSiteURLBindingRule() {
 	// URL validation rule
 	binding.AddRule(&binding.Rule{
 		IsMatch: func(rule string) bool {
-			return strings.HasPrefix(rule, "ValidSiteUrl")
+			return rule == "ValidSiteUrl"
 		},
 		IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) {
 			str := fmt.Sprintf("%v", val)
@@ -171,7 +200,7 @@ func addUsernamePatternRule() {
 func addValidGroupTeamMapRule() {
 	binding.AddRule(&binding.Rule{
 		IsMatch: func(rule string) bool {
-			return strings.HasPrefix(rule, "ValidGroupTeamMap")
+			return rule == "ValidGroupTeamMap"
 		},
 		IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) {
 			_, err := auth.UnmarshalGroupTeamMapping(fmt.Sprintf("%v", val))