From 0536712ee7e9592a503bf3057e7203173076872a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 12 Apr 2023 01:22:49 +0800 Subject: [PATCH] Fix branch protection priority (#24045) Fix #24044 --- models/git/protected_banch_list_test.go | 76 +++++++++++++++++++++++++ models/git/protected_branch_list.go | 10 +--- 2 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 models/git/protected_banch_list_test.go diff --git a/models/git/protected_banch_list_test.go b/models/git/protected_banch_list_test.go new file mode 100644 index 000000000000..4bb3136d580b --- /dev/null +++ b/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) + } + } +} diff --git a/models/git/protected_branch_list.go b/models/git/protected_branch_list.go index 99c433aa000f..17fe6d701fb4 100644 --- a/models/git/protected_branch_list.go +++ b/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 }