From 643f476e35aa3e5ea00a567fd189b710306a83b0 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 29 Oct 2024 15:43:47 +0100 Subject: [PATCH] Optimize branch protection rule loading (#32280) before if it was nonglob each load would try to glob it and the check that is not glob ... now we only do that once and no future loading will trigger it --- *Sponsored by Kithara Software GmbH* (cherry picked from commit 5d43801b72790ce5862aefdc4520edb06bb4cbba) --- models/git/protected_branch.go | 22 +++++++++----- ..._test.go => protected_branch_list_test.go} | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+), 8 deletions(-) rename models/git/{protected_banch_list_test.go => protected_branch_list_test.go} (79%) diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index a8b8c81bbe..4fc08020e0 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -79,14 +79,20 @@ func IsRuleNameSpecial(ruleName string) bool { } func (protectBranch *ProtectedBranch) loadGlob() { - if protectBranch.globRule == nil { - 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) + if protectBranch.isPlainName || protectBranch.globRule != nil { + return + } + // detect if it is not glob + if !IsRuleNameSpecial(protectBranch.RuleName) { + protectBranch.isPlainName = true + return + } + // now we load the glob + 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), '/') } } diff --git a/models/git/protected_banch_list_test.go b/models/git/protected_branch_list_test.go similarity index 79% rename from models/git/protected_banch_list_test.go rename to models/git/protected_branch_list_test.go index 09319d21a8..db7e54f685 100644 --- a/models/git/protected_banch_list_test.go +++ b/models/git/protected_branch_list_test.go @@ -75,3 +75,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) +}