Fix branch protection priority (#24045) (#24061)

Backport #24045 by @lunny

Fix #24044

Co-authored-by: Lunny Xiao <[email protected]>

Author: GiteaBot

models/git/protected_banch_list_test.go

@@ -0,0 +1,76 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package git
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestBranchRuleMatchPriority(t *testing.T) {
+	kases := []struct {
+		Rules            []string
+		BranchName       string
+		ExpectedMatchIdx int
+	}{
+		{
+			Rules:            []string{"release/*", "release/v1.17"},
+			BranchName:       "release/v1.17",
+			ExpectedMatchIdx: 1,
+		},
+		{
+			Rules:            []string{"release/v1.17", "release/*"},
+			BranchName:       "release/v1.17",
+			ExpectedMatchIdx: 0,
+		},
+		{
+			Rules:            []string{"release/**/v1.17", "release/test/v1.17"},
+			BranchName:       "release/test/v1.17",
+			ExpectedMatchIdx: 1,
+		},
+		{
+			Rules:            []string{"release/test/v1.17", "release/**/v1.17"},
+			BranchName:       "release/test/v1.17",
+			ExpectedMatchIdx: 0,
+		},
+		{
+			Rules:            []string{"release/**", "release/v1.0.0"},
+			BranchName:       "release/v1.0.0",
+			ExpectedMatchIdx: 1,
+		},
+		{
+			Rules:            []string{"release/v1.0.0", "release/**"},
+			BranchName:       "release/v1.0.0",
+			ExpectedMatchIdx: 0,
+		},
+		{
+			Rules:            []string{"release/**", "release/v1.0.0"},
+			BranchName:       "release/v2.0.0",
+			ExpectedMatchIdx: 0,
+		},
+		{
+			Rules:            []string{"release/*", "release/v1.0.0"},
+			BranchName:       "release/1/v2.0.0",
+			ExpectedMatchIdx: -1,
+		},
+	}
+
+	for _, kase := range kases {
+		var pbs ProtectedBranchRules
+		for _, rule := range kase.Rules {
+			pbs = append(pbs, &ProtectedBranch{RuleName: rule})
+		}
+		pbs.sort()
+		matchedPB := pbs.GetFirstMatched(kase.BranchName)
+		if matchedPB == nil {
+			if kase.ExpectedMatchIdx >= 0 {
+				assert.Error(t, fmt.Errorf("no matched rules but expected %s[%d]", kase.Rules[kase.ExpectedMatchIdx], kase.ExpectedMatchIdx))
+			}
+		} else {
+			assert.EqualValues(t, kase.Rules[kase.ExpectedMatchIdx], matchedPB.RuleName)
+		}
+	}
+}

models/git/protected_branch_list.go

@@ -28,12 +28,8 @@ func (rules ProtectedBranchRules) sort() {
 	sort.Slice(rules, func(i, j int) bool {
 		rules[i].loadGlob()
 		rules[j].loadGlob()
-		if rules[i].isPlainName {
-			if !rules[j].isPlainName {
-				return true
-			}
-		} else if rules[j].isPlainName {
-			return true
+		if rules[i].isPlainName != rules[j].isPlainName {
+			return rules[i].isPlainName // plain name comes first, so plain name means "less"
 		}
 		return rules[i].CreatedUnix < rules[j].CreatedUnix
 	})
@@ -46,7 +42,7 @@ func FindRepoProtectedBranchRules(ctx context.Context, repoID int64) (ProtectedB
 	if err != nil {
 		return nil, err
 	}
-	rules.sort()
+	rules.sort() // to make non-glob rules have higher priority, and for same glob/non-glob rules, first created rules have higher priority
 	return rules, nil
 }