Skip to content

Commit

Permalink
Fix data-race during testing (#30999) (#31024)
Browse files Browse the repository at this point in the history 
Backport #30999 by wxiaoguang

Fix #30992

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
  • Loading branch information
@GiteaBot @wxiaoguang
GiteaBot and wxiaoguang committed May 20, 2024
1 parent 8446caa commit d17bfad
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 20 deletions.
26 changes: 20 additions & 6 deletions models/unit/unit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"strings"
"sync/atomic"

"code.gitea.io/gitea/models/perm"
"code.gitea.io/gitea/modules/container"
Expand Down Expand Up @@ -106,10 +107,23 @@ var (
TypeExternalTracker,
}

// DisabledRepoUnits contains the units that have been globally disabled
DisabledRepoUnits = []Type{}
disabledRepoUnitsAtomic atomic.Pointer[[]Type] // the units that have been globally disabled
)

// 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
func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
units := defaultUnits
Expand All @@ -127,7 +141,7 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
}

// Remove disabled units
for _, disabledUnit := range DisabledRepoUnits {
for _, disabledUnit := range DisabledRepoUnitsGet() {
for i, unit := range units {
if unit == disabledUnit {
units = append(units[:i], units[i+1:]...)
Expand All @@ -140,11 +154,11 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {

// LoadUnitConfig load units from settings
func LoadUnitConfig() error {
var invalidKeys []string
DisabledRepoUnits, invalidKeys = FindUnitTypes(setting.Repository.DisabledRepoUnits...)
disabledRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DisabledRepoUnits...)
if len(invalidKeys) > 0 {
log.Warn("Invalid keys in disabled repo units: %s", strings.Join(invalidKeys, ", "))
}
DisabledRepoUnitsSet(disabledRepoUnits)

setDefaultRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DefaultRepoUnits...)
if len(invalidKeys) > 0 {
Expand All @@ -167,7 +181,7 @@ func LoadUnitConfig() error {

// UnitGlobalDisabled checks if unit type is global disabled
func (u Type) UnitGlobalDisabled() bool {
for _, ud := range DisabledRepoUnits {
for _, ud := range DisabledRepoUnitsGet() {
if u == ud {
return true
}
Expand Down
24 changes: 12 additions & 12 deletions models/unit/unit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import (
func TestLoadUnitConfig(t *testing.T) {
t.Run("regular", func(t *testing.T) {
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
DisabledRepoUnits = disabledRepoUnits
DisabledRepoUnitsSet(disabledRepoUnits)
DefaultRepoUnits = defaultRepoUnits
DefaultForkRepoUnits = defaultForkRepoUnits
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
}(DisabledRepoUnitsGet(), DefaultRepoUnits, DefaultForkRepoUnits)
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
setting.Repository.DisabledRepoUnits = disabledRepoUnits
setting.Repository.DefaultRepoUnits = defaultRepoUnits
Expand All @@ -28,16 +28,16 @@ func TestLoadUnitConfig(t *testing.T) {
setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls"}
setting.Repository.DefaultForkRepoUnits = []string{"repo.releases"}
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{TypeReleases}, DefaultForkRepoUnits)
})
t.Run("invalid", func(t *testing.T) {
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
DisabledRepoUnits = disabledRepoUnits
DisabledRepoUnitsSet(disabledRepoUnits)
DefaultRepoUnits = defaultRepoUnits
DefaultForkRepoUnits = defaultForkRepoUnits
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
}(DisabledRepoUnitsGet(), DefaultRepoUnits, DefaultForkRepoUnits)
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
setting.Repository.DisabledRepoUnits = disabledRepoUnits
setting.Repository.DefaultRepoUnits = defaultRepoUnits
Expand All @@ -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.DefaultForkRepoUnits = []string{"invalid.3", "repo.releases"}
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{TypeReleases}, DefaultForkRepoUnits)
})
t.Run("duplicate", func(t *testing.T) {
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
DisabledRepoUnits = disabledRepoUnits
DisabledRepoUnitsSet(disabledRepoUnits)
DefaultRepoUnits = defaultRepoUnits
DefaultForkRepoUnits = defaultForkRepoUnits
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
}(DisabledRepoUnitsGet(), DefaultRepoUnits, DefaultForkRepoUnits)
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
setting.Repository.DisabledRepoUnits = disabledRepoUnits
setting.Repository.DefaultRepoUnits = defaultRepoUnits
Expand All @@ -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.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"}
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{TypeReleases}, DefaultForkRepoUnits)
})
t.Run("empty_default", func(t *testing.T) {
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
DisabledRepoUnits = disabledRepoUnits
DisabledRepoUnitsSet(disabledRepoUnits)
DefaultRepoUnits = defaultRepoUnits
DefaultForkRepoUnits = defaultForkRepoUnits
}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
}(DisabledRepoUnitsGet(), DefaultRepoUnits, DefaultForkRepoUnits)
defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
setting.Repository.DisabledRepoUnits = disabledRepoUnits
setting.Repository.DefaultRepoUnits = defaultRepoUnits
Expand All @@ -88,7 +88,7 @@ func TestLoadUnitConfig(t *testing.T) {
setting.Repository.DefaultRepoUnits = []string{}
setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"}
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.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
})
Expand Down
6 changes: 4 additions & 2 deletions tests/integration/org_project_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ import (
"testing"

unit_model "code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/tests"
)

func TestOrgProjectAccess(t *testing.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
req := NewRequest(t, "GET", "/user2/repo1/projects")
Expand Down

0 comments on commit d17bfad

Please sign in to comment.