Fix data-race during testing (#30999)

Fix #30992

(cherry picked from commit 47accfebbd69e5f47d1b97a3e39cf181fab7e597)

Conflicts:
	models/unit/unit.go
	trivial context conflict because of
	e07b0e75ff Add a direct link from repo header to unit settings
This commit is contained in:
wxiaoguang 2024-05-20 12:35:38 +08:00 committed by Earl Warren
parent a649610d61
commit f6e50abd65
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
3 changed files with 36 additions and 20 deletions

View file

@ -7,6 +7,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"strings" "strings"
"sync/atomic"
"code.gitea.io/gitea/models/perm" "code.gitea.io/gitea/models/perm"
"code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/container"
@ -106,14 +107,27 @@ var (
TypeExternalTracker, TypeExternalTracker,
} }
// DisabledRepoUnits contains the units that have been globally disabled disabledRepoUnitsAtomic atomic.Pointer[[]Type] // the units that have been globally disabled
DisabledRepoUnits = []Type{}
// AllowedRepoUnitGroups contains the units that have been globally enabled, // AllowedRepoUnitGroups contains the units that have been globally enabled,
// with mutually exclusive units grouped together. // with mutually exclusive units grouped together.
AllowedRepoUnitGroups = [][]Type{} AllowedRepoUnitGroups = [][]Type{}
) )
// DisabledRepoUnitsGet returns the globally disabled units, it is a quick patch to fix data-race during testing.
// Because the queue worker might read when a test is mocking the value. FIXME: refactor to a clear solution later.
func DisabledRepoUnitsGet() []Type {
v := disabledRepoUnitsAtomic.Load()
if v == nil {
return nil
}
return *v
}
func DisabledRepoUnitsSet(v []Type) {
disabledRepoUnitsAtomic.Store(&v)
}
// Get valid set of default repository units from settings // Get valid set of default repository units from settings
func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type { func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
units := defaultUnits units := defaultUnits
@ -131,7 +145,7 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
} }
// Remove disabled units // Remove disabled units
for _, disabledUnit := range DisabledRepoUnits { for _, disabledUnit := range DisabledRepoUnitsGet() {
for i, unit := range units { for i, unit := range units {
if unit == disabledUnit { if unit == disabledUnit {
units = append(units[:i], units[i+1:]...) units = append(units[:i], units[i+1:]...)
@ -144,11 +158,11 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
// LoadUnitConfig load units from settings // LoadUnitConfig load units from settings
func LoadUnitConfig() error { func LoadUnitConfig() error {
var invalidKeys []string disabledRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DisabledRepoUnits...)
DisabledRepoUnits, invalidKeys = FindUnitTypes(setting.Repository.DisabledRepoUnits...)
if len(invalidKeys) > 0 { if len(invalidKeys) > 0 {
log.Warn("Invalid keys in disabled repo units: %s", strings.Join(invalidKeys, ", ")) log.Warn("Invalid keys in disabled repo units: %s", strings.Join(invalidKeys, ", "))
} }
DisabledRepoUnitsSet(disabledRepoUnits)
setDefaultRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DefaultRepoUnits...) setDefaultRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DefaultRepoUnits...)
if len(invalidKeys) > 0 { if len(invalidKeys) > 0 {
@ -210,7 +224,7 @@ func LoadUnitConfig() error {
// UnitGlobalDisabled checks if unit type is global disabled // UnitGlobalDisabled checks if unit type is global disabled
func (u Type) UnitGlobalDisabled() bool { func (u Type) UnitGlobalDisabled() bool {
for _, ud := range DisabledRepoUnits { for _, ud := range DisabledRepoUnitsGet() {
if u == ud { if u == ud {
return true return true
} }

View file

@ -14,10 +14,10 @@ import (
func TestLoadUnitConfig(t *testing.T) { func TestLoadUnitConfig(t *testing.T) {
t.Run("regular", func(t *testing.T) { t.Run("regular", func(t *testing.T) {
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) { defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
DisabledRepoUnits = disabledRepoUnits DisabledRepoUnitsSet(disabledRepoUnits)
DefaultRepoUnits = defaultRepoUnits DefaultRepoUnits = defaultRepoUnits
DefaultForkRepoUnits = defaultForkRepoUnits DefaultForkRepoUnits = defaultForkRepoUnits
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits) }(DisabledRepoUnitsGet(), DefaultRepoUnits, DefaultForkRepoUnits)
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) { defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
setting.Repository.DisabledRepoUnits = disabledRepoUnits setting.Repository.DisabledRepoUnits = disabledRepoUnits
setting.Repository.DefaultRepoUnits = defaultRepoUnits setting.Repository.DefaultRepoUnits = defaultRepoUnits
@ -28,16 +28,16 @@ func TestLoadUnitConfig(t *testing.T) {
setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls"} setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls"}
setting.Repository.DefaultForkRepoUnits = []string{"repo.releases"} setting.Repository.DefaultForkRepoUnits = []string{"repo.releases"}
assert.NoError(t, LoadUnitConfig()) assert.NoError(t, LoadUnitConfig())
assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits) assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnitsGet())
assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits) assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits) assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
}) })
t.Run("invalid", func(t *testing.T) { t.Run("invalid", func(t *testing.T) {
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) { defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
DisabledRepoUnits = disabledRepoUnits DisabledRepoUnitsSet(disabledRepoUnits)
DefaultRepoUnits = defaultRepoUnits DefaultRepoUnits = defaultRepoUnits
DefaultForkRepoUnits = defaultForkRepoUnits DefaultForkRepoUnits = defaultForkRepoUnits
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits) }(DisabledRepoUnitsGet(), DefaultRepoUnits, DefaultForkRepoUnits)
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) { defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
setting.Repository.DisabledRepoUnits = disabledRepoUnits setting.Repository.DisabledRepoUnits = disabledRepoUnits
setting.Repository.DefaultRepoUnits = defaultRepoUnits setting.Repository.DefaultRepoUnits = defaultRepoUnits
@ -48,16 +48,16 @@ func TestLoadUnitConfig(t *testing.T) {
setting.Repository.DefaultRepoUnits = []string{"repo.code", "invalid.2", "repo.releases", "repo.issues", "repo.pulls"} setting.Repository.DefaultRepoUnits = []string{"repo.code", "invalid.2", "repo.releases", "repo.issues", "repo.pulls"}
setting.Repository.DefaultForkRepoUnits = []string{"invalid.3", "repo.releases"} setting.Repository.DefaultForkRepoUnits = []string{"invalid.3", "repo.releases"}
assert.NoError(t, LoadUnitConfig()) assert.NoError(t, LoadUnitConfig())
assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits) assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnitsGet())
assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits) assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits) assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
}) })
t.Run("duplicate", func(t *testing.T) { t.Run("duplicate", func(t *testing.T) {
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) { defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
DisabledRepoUnits = disabledRepoUnits DisabledRepoUnitsSet(disabledRepoUnits)
DefaultRepoUnits = defaultRepoUnits DefaultRepoUnits = defaultRepoUnits
DefaultForkRepoUnits = defaultForkRepoUnits DefaultForkRepoUnits = defaultForkRepoUnits
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits) }(DisabledRepoUnitsGet(), DefaultRepoUnits, DefaultForkRepoUnits)
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) { defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
setting.Repository.DisabledRepoUnits = disabledRepoUnits setting.Repository.DisabledRepoUnits = disabledRepoUnits
setting.Repository.DefaultRepoUnits = defaultRepoUnits setting.Repository.DefaultRepoUnits = defaultRepoUnits
@ -68,16 +68,16 @@ func TestLoadUnitConfig(t *testing.T) {
setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls", "repo.code"} setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls", "repo.code"}
setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"} setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"}
assert.NoError(t, LoadUnitConfig()) assert.NoError(t, LoadUnitConfig())
assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits) assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnitsGet())
assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits) assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits) assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
}) })
t.Run("empty_default", func(t *testing.T) { t.Run("empty_default", func(t *testing.T) {
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) { defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
DisabledRepoUnits = disabledRepoUnits DisabledRepoUnitsSet(disabledRepoUnits)
DefaultRepoUnits = defaultRepoUnits DefaultRepoUnits = defaultRepoUnits
DefaultForkRepoUnits = defaultForkRepoUnits DefaultForkRepoUnits = defaultForkRepoUnits
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits) }(DisabledRepoUnitsGet(), DefaultRepoUnits, DefaultForkRepoUnits)
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) { defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
setting.Repository.DisabledRepoUnits = disabledRepoUnits setting.Repository.DisabledRepoUnits = disabledRepoUnits
setting.Repository.DefaultRepoUnits = defaultRepoUnits setting.Repository.DefaultRepoUnits = defaultRepoUnits
@ -88,7 +88,7 @@ func TestLoadUnitConfig(t *testing.T) {
setting.Repository.DefaultRepoUnits = []string{} setting.Repository.DefaultRepoUnits = []string{}
setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"} setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"}
assert.NoError(t, LoadUnitConfig()) assert.NoError(t, LoadUnitConfig())
assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits) assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnitsGet())
assert.ElementsMatch(t, []Type{TypeCode, TypePullRequests, TypeReleases, TypeWiki, TypePackages, TypeProjects, TypeActions}, DefaultRepoUnits) assert.ElementsMatch(t, []Type{TypeCode, TypePullRequests, TypeReleases, TypeWiki, TypePackages, TypeProjects, TypeActions}, DefaultRepoUnits)
assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits) assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
}) })

View file

@ -9,13 +9,15 @@ import (
"testing" "testing"
unit_model "code.gitea.io/gitea/models/unit" unit_model "code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/tests" "code.gitea.io/gitea/tests"
) )
func TestOrgProjectAccess(t *testing.T) { func TestOrgProjectAccess(t *testing.T) {
defer tests.PrepareTestEnv(t)() defer tests.PrepareTestEnv(t)()
defer test.MockVariableValue(&unit_model.DisabledRepoUnits, append(slices.Clone(unit_model.DisabledRepoUnits), unit_model.TypeProjects))()
disabledRepoUnits := unit_model.DisabledRepoUnitsGet()
unit_model.DisabledRepoUnitsSet(append(slices.Clone(disabledRepoUnits), unit_model.TypeProjects))
defer unit_model.DisabledRepoUnitsSet(disabledRepoUnits)
// repo project, 404 // repo project, 404
req := NewRequest(t, "GET", "/user2/repo1/projects") req := NewRequest(t, "GET", "/user2/repo1/projects")