From cbde69f6ee008db9c36dbf30f4da6344deebbb79 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 16 Oct 2024 23:00:26 +0200 Subject: [PATCH 1/9] Add test for ProtectedBranchRules.sort --- models/git/protected_banch_list_test.go | 29 +++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/models/git/protected_banch_list_test.go b/models/git/protected_banch_list_test.go index 4bb3136d580bf..94a48f37e6a5b 100644 --- a/models/git/protected_banch_list_test.go +++ b/models/git/protected_banch_list_test.go @@ -74,3 +74,32 @@ func TestBranchRuleMatchPriority(t *testing.T) { } } } + +func TestBranchRuleSort(t *testing.T) { + in := []*ProtectedBranch{{ + RuleName: "b", + CreatedUnix: 1, + }, { + RuleName: "b/*", + CreatedUnix: 3, + }, { + RuleName: "a/*", + CreatedUnix: 2, + }, { + RuleName: "c", + CreatedUnix: 0, + }, { + RuleName: "a", + CreatedUnix: 4, + }} + expect := []string{"c", "b", "a", "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) +} From ccb2e12189e2d234e5de5184164dc42360c1186c Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 16 Oct 2024 23:07:58 +0200 Subject: [PATCH 2/9] optimize loadGlob for protectedBranch --- models/git/protected_branch.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index bde6057375e55..70033476247c4 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -83,14 +83,18 @@ func IsRuleNameSpecial(ruleName string) bool { } func (protectBranch *ProtectedBranch) loadGlob() { - if protectBranch.globRule == nil { + if protectBranch.globRule == nil && !protectBranch.isPlainName { + // detect if it is not glob + if !IsRuleNameSpecial(protectBranch.RuleName) { + protectBranch.isPlainName = true + return + } var err error protectBranch.globRule, err = glob.Compile(protectBranch.RuleName, '/') if err != nil { log.Warn("Invalid glob rule for ProtectedBranch[%d]: %s %v", protectBranch.ID, protectBranch.RuleName, err) protectBranch.globRule = glob.MustCompile(glob.QuoteMeta(protectBranch.RuleName), '/') } - protectBranch.isPlainName = !IsRuleNameSpecial(protectBranch.RuleName) } } From e6b983410f9ec393b13a38f3789ba1376f693868 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 16 Oct 2024 23:08:35 +0200 Subject: [PATCH 3/9] sort non glob protected branch roules by name --- models/git/protected_banch_list_test.go | 2 +- models/git/protected_branch_list.go | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/models/git/protected_banch_list_test.go b/models/git/protected_banch_list_test.go index 94a48f37e6a5b..8852e898d8e3b 100644 --- a/models/git/protected_banch_list_test.go +++ b/models/git/protected_banch_list_test.go @@ -92,7 +92,7 @@ func TestBranchRuleSort(t *testing.T) { RuleName: "a", CreatedUnix: 4, }} - expect := []string{"c", "b", "a", "a/*", "b/*"} + expect := []string{"a", "b", "c", "a/*", "b/*"} pbr := ProtectedBranchRules(in) pbr.sort() diff --git a/models/git/protected_branch_list.go b/models/git/protected_branch_list.go index 613333a5a2950..9fe7d3f1a4cd5 100644 --- a/models/git/protected_branch_list.go +++ b/models/git/protected_branch_list.go @@ -6,6 +6,7 @@ package git import ( "context" "sort" + "strings" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/optional" @@ -29,7 +30,12 @@ func (rules ProtectedBranchRules) sort() { rules[i].loadGlob() rules[j].loadGlob() if rules[i].isPlainName != rules[j].isPlainName { - return rules[i].isPlainName // plain name comes first, so plain name means "less" + // plain name comes first, so plain name means "less" + return rules[i].isPlainName + } + if rules[i].isPlainName { + // both are plain names so sort alphabetically + return strings.Compare(rules[i].RuleName, rules[j].RuleName) < 0 } return rules[i].CreatedUnix < rules[j].CreatedUnix }) From 74760d522333bb155ca6513e2b7e0c4098a26f75 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 16 Oct 2024 23:21:10 +0200 Subject: [PATCH 4/9] update comment --- models/git/protected_branch_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/git/protected_branch_list.go b/models/git/protected_branch_list.go index 9fe7d3f1a4cd5..d028a4941411c 100644 --- a/models/git/protected_branch_list.go +++ b/models/git/protected_branch_list.go @@ -48,7 +48,7 @@ func FindRepoProtectedBranchRules(ctx context.Context, repoID int64) (ProtectedB if err != nil { return nil, err } - rules.sort() // to make non-glob rules have higher priority, and for same glob/non-glob rules, first created rules have higher priority + rules.sort() // make sure first match is detected in right order return rules, nil } From 3103499b2a9b75a33ef25c12591bd899ac908363 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 17 Oct 2024 10:32:18 +0200 Subject: [PATCH 5/9] clean unrelated --- models/git/protected_banch_list_test.go | 2 +- models/git/protected_branch_list.go | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/models/git/protected_banch_list_test.go b/models/git/protected_banch_list_test.go index 8852e898d8e3b..94a48f37e6a5b 100644 --- a/models/git/protected_banch_list_test.go +++ b/models/git/protected_banch_list_test.go @@ -92,7 +92,7 @@ func TestBranchRuleSort(t *testing.T) { RuleName: "a", CreatedUnix: 4, }} - expect := []string{"a", "b", "c", "a/*", "b/*"} + expect := []string{"c", "b", "a", "a/*", "b/*"} pbr := ProtectedBranchRules(in) pbr.sort() diff --git a/models/git/protected_branch_list.go b/models/git/protected_branch_list.go index d028a4941411c..613333a5a2950 100644 --- a/models/git/protected_branch_list.go +++ b/models/git/protected_branch_list.go @@ -6,7 +6,6 @@ package git import ( "context" "sort" - "strings" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/optional" @@ -30,12 +29,7 @@ func (rules ProtectedBranchRules) sort() { rules[i].loadGlob() rules[j].loadGlob() if rules[i].isPlainName != rules[j].isPlainName { - // plain name comes first, so plain name means "less" - return rules[i].isPlainName - } - if rules[i].isPlainName { - // both are plain names so sort alphabetically - return strings.Compare(rules[i].RuleName, rules[j].RuleName) < 0 + return rules[i].isPlainName // plain name comes first, so plain name means "less" } return rules[i].CreatedUnix < rules[j].CreatedUnix }) @@ -48,7 +42,7 @@ func FindRepoProtectedBranchRules(ctx context.Context, repoID int64) (ProtectedB if err != nil { return nil, err } - rules.sort() // make sure first match is detected in right order + 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 } From c8b35fda0155e083061d759d99a142649a1379c9 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 17 Oct 2024 11:09:48 +0200 Subject: [PATCH 6/9] fix misspell --- ...protected_banch_list_test.go => protected_branch_list_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename models/git/{protected_banch_list_test.go => protected_branch_list_test.go} (100%) diff --git a/models/git/protected_banch_list_test.go b/models/git/protected_branch_list_test.go similarity index 100% rename from models/git/protected_banch_list_test.go rename to models/git/protected_branch_list_test.go From 3c17638bc099ec06c20444380515d3c0f135492e Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 17 Oct 2024 11:11:09 +0200 Subject: [PATCH 7/9] Update models/git/protected_branch.go Co-authored-by: Lunny Xiao --- models/git/protected_branch.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index 70033476247c4..f07eb1b91c2b7 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -83,7 +83,8 @@ func IsRuleNameSpecial(ruleName string) bool { } func (protectBranch *ProtectedBranch) loadGlob() { - if protectBranch.globRule == nil && !protectBranch.isPlainName { + if protectBranch.isPlainName || protectBranch.globRule != nil { + return // detect if it is not glob if !IsRuleNameSpecial(protectBranch.RuleName) { protectBranch.isPlainName = true From d52b8a89332d1673a047b57d1c6da45c316cbf2c Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 17 Oct 2024 11:12:28 +0200 Subject: [PATCH 8/9] as per @lunny ... finished --- models/git/protected_branch.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index f07eb1b91c2b7..163522a2f3786 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -84,18 +84,18 @@ func IsRuleNameSpecial(ruleName string) bool { func (protectBranch *ProtectedBranch) loadGlob() { if protectBranch.isPlainName || protectBranch.globRule != nil { - return - // detect if it is not glob - if !IsRuleNameSpecial(protectBranch.RuleName) { - protectBranch.isPlainName = true - return - } - var err error - protectBranch.globRule, err = glob.Compile(protectBranch.RuleName, '/') - if err != nil { - log.Warn("Invalid glob rule for ProtectedBranch[%d]: %s %v", protectBranch.ID, protectBranch.RuleName, err) - protectBranch.globRule = glob.MustCompile(glob.QuoteMeta(protectBranch.RuleName), '/') - } + return + } + // detect if it is not glob + if !IsRuleNameSpecial(protectBranch.RuleName) { + protectBranch.isPlainName = true + return + } + var err error + protectBranch.globRule, err = glob.Compile(protectBranch.RuleName, '/') + if err != nil { + log.Warn("Invalid glob rule for ProtectedBranch[%d]: %s %v", protectBranch.ID, protectBranch.RuleName, err) + protectBranch.globRule = glob.MustCompile(glob.QuoteMeta(protectBranch.RuleName), '/') } } From df7939e61bb2e20393d3986556f2a77e4bc30608 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 17 Oct 2024 11:13:17 +0200 Subject: [PATCH 9/9] code comment --- models/git/protected_branch.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index 163522a2f3786..a4cf9e4df11cb 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -91,6 +91,7 @@ func (protectBranch *ProtectedBranch) loadGlob() { protectBranch.isPlainName = true return } + // now we load the glob var err error protectBranch.globRule, err = glob.Compile(protectBranch.RuleName, '/') if err != nil {