Require at least one unit to be enabled (#24189)

Don't remember why the previous decision that `Code` and `Release` are
non-disable units globally. Since now every unit include `Code` could be
disabled, maybe we should have a new rule that the repo should have at
least one unit. So any unit could be disabled.

Fixes #20960
Fixes #7525

---------

Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: yp05327 <576951401@qq.com>

2 files changed, 70 insertions, 53 deletions

diff --git a/models/unit/unit.go b/models/unit/unit.go
index 3d5a8842cd..7cd679116f 100644
--- a/models/unit/unit.go
+++ b/models/unit/unit.go
@@ -4,6 +4,7 @@
 package unit
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 
@@ -106,12 +107,6 @@ var (
 		TypeExternalTracker,
 	}
 
-	// MustRepoUnits contains the units could not be disabled currently
-	MustRepoUnits = []Type{
-		TypeCode,
-		TypeReleases,
-	}
-
 	// DisabledRepoUnits contains the units that have been globally disabled
 	DisabledRepoUnits = []Type{}
 )
@@ -122,18 +117,13 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
 
 	// Use setting if not empty
 	if len(settingDefaultUnits) > 0 {
-		// MustRepoUnits required as default
-		units = make([]Type, len(MustRepoUnits))
-		copy(units, MustRepoUnits)
+		units = make([]Type, 0, len(settingDefaultUnits))
 		for _, settingUnit := range settingDefaultUnits {
 			if !settingUnit.CanBeDefault() {
 				log.Warn("Not allowed as default unit: %s", settingUnit.String())
 				continue
 			}
-			// MustRepoUnits already added
-			if settingUnit.CanDisable() {
-				units = append(units, settingUnit)
-			}
+			units = append(units, settingUnit)
 		}
 	}
 
@@ -150,30 +140,30 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
 }
 
 // LoadUnitConfig load units from settings
-func LoadUnitConfig() {
+func LoadUnitConfig() error {
 	var invalidKeys []string
 	DisabledRepoUnits, invalidKeys = FindUnitTypes(setting.Repository.DisabledRepoUnits...)
 	if len(invalidKeys) > 0 {
 		log.Warn("Invalid keys in disabled repo units: %s", strings.Join(invalidKeys, ", "))
 	}
-	// Check that must units are not disabled
-	for i, disabledU := range DisabledRepoUnits {
-		if !disabledU.CanDisable() {
-			log.Warn("Not allowed to global disable unit %s", disabledU.String())
-			DisabledRepoUnits = append(DisabledRepoUnits[:i], DisabledRepoUnits[i+1:]...)
-		}
-	}
 
 	setDefaultRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DefaultRepoUnits...)
 	if len(invalidKeys) > 0 {
 		log.Warn("Invalid keys in default repo units: %s", strings.Join(invalidKeys, ", "))
 	}
 	DefaultRepoUnits = validateDefaultRepoUnits(DefaultRepoUnits, setDefaultRepoUnits)
+	if len(DefaultRepoUnits) == 0 {
+		return errors.New("no default repository units found")
+	}
 	setDefaultForkRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DefaultForkRepoUnits...)
 	if len(invalidKeys) > 0 {
 		log.Warn("Invalid keys in default fork repo units: %s", strings.Join(invalidKeys, ", "))
 	}
 	DefaultForkRepoUnits = validateDefaultRepoUnits(DefaultForkRepoUnits, setDefaultForkRepoUnits)
+	if len(DefaultForkRepoUnits) == 0 {
+		return errors.New("no default fork repository units found")
+	}
+	return nil
 }
 
 // UnitGlobalDisabled checks if unit type is global disabled
@@ -186,16 +176,6 @@ func (u Type) UnitGlobalDisabled() bool {
 	return false
 }
 
-// CanDisable checks if this unit type can be disabled.
-func (u *Type) CanDisable() bool {
-	for _, mu := range MustRepoUnits {
-		if *u == mu {
-			return false
-		}
-	}
-	return true
-}
-
 // CanBeDefault checks if the unit type can be a default repo unit
 func (u *Type) CanBeDefault() bool {
 	for _, nadU := range NotAllowedDefaultRepoUnits {
@@ -216,11 +196,6 @@ type Unit struct {
 	MaxAccessMode perm.AccessMode // The max access mode of the unit. i.e. Read means this unit can only be read.
 }
 
-// CanDisable returns if this unit could be disabled.
-func (u *Unit) CanDisable() bool {
-	return u.Type.CanDisable()
-}
-
 // IsLessThan compares order of two units
 func (u Unit) IsLessThan(unit Unit) bool {
 	if (u.Type == TypeExternalTracker || u.Type == TypeExternalWiki) && unit.Type != TypeExternalTracker && unit.Type != TypeExternalWiki {
diff --git a/models/unit/unit_test.go b/models/unit/unit_test.go
index 50d7817197..5c50a106bd 100644
--- a/models/unit/unit_test.go
+++ b/models/unit/unit_test.go
@@ -12,42 +12,84 @@ import (
 )
 
 func TestLoadUnitConfig(t *testing.T) {
-	defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
-		DisabledRepoUnits = disabledRepoUnits
-		DefaultRepoUnits = defaultRepoUnits
-		DefaultForkRepoUnits = defaultForkRepoUnits
-	}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
-	defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
-		setting.Repository.DisabledRepoUnits = disabledRepoUnits
-		setting.Repository.DefaultRepoUnits = defaultRepoUnits
-		setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
-	}(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
-
 	t.Run("regular", func(t *testing.T) {
+		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
+			DisabledRepoUnits = disabledRepoUnits
+			DefaultRepoUnits = defaultRepoUnits
+			DefaultForkRepoUnits = defaultForkRepoUnits
+		}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
+		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
+			setting.Repository.DisabledRepoUnits = disabledRepoUnits
+			setting.Repository.DefaultRepoUnits = defaultRepoUnits
+			setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
+		}(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
+
 		setting.Repository.DisabledRepoUnits = []string{"repo.issues"}
 		setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls"}
 		setting.Repository.DefaultForkRepoUnits = []string{"repo.releases"}
-		LoadUnitConfig()
+		assert.NoError(t, LoadUnitConfig())
 		assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
 		assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
-		assert.Equal(t, []Type{TypeCode, TypeReleases}, DefaultForkRepoUnits)
+		assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
 	})
 	t.Run("invalid", func(t *testing.T) {
+		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
+			DisabledRepoUnits = disabledRepoUnits
+			DefaultRepoUnits = defaultRepoUnits
+			DefaultForkRepoUnits = defaultForkRepoUnits
+		}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
+		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
+			setting.Repository.DisabledRepoUnits = disabledRepoUnits
+			setting.Repository.DefaultRepoUnits = defaultRepoUnits
+			setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
+		}(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
+
 		setting.Repository.DisabledRepoUnits = []string{"repo.issues", "invalid.1"}
 		setting.Repository.DefaultRepoUnits = []string{"repo.code", "invalid.2", "repo.releases", "repo.issues", "repo.pulls"}
 		setting.Repository.DefaultForkRepoUnits = []string{"invalid.3", "repo.releases"}
-		LoadUnitConfig()
+		assert.NoError(t, LoadUnitConfig())
 		assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
 		assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
-		assert.Equal(t, []Type{TypeCode, TypeReleases}, DefaultForkRepoUnits)
+		assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
 	})
 	t.Run("duplicate", func(t *testing.T) {
+		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
+			DisabledRepoUnits = disabledRepoUnits
+			DefaultRepoUnits = defaultRepoUnits
+			DefaultForkRepoUnits = defaultForkRepoUnits
+		}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
+		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
+			setting.Repository.DisabledRepoUnits = disabledRepoUnits
+			setting.Repository.DefaultRepoUnits = defaultRepoUnits
+			setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
+		}(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
+
 		setting.Repository.DisabledRepoUnits = []string{"repo.issues", "repo.issues"}
 		setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls", "repo.code"}
 		setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"}
-		LoadUnitConfig()
+		assert.NoError(t, LoadUnitConfig())
 		assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
 		assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
-		assert.Equal(t, []Type{TypeCode, TypeReleases}, DefaultForkRepoUnits)
+		assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
+	})
+	t.Run("empty_default", func(t *testing.T) {
+		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
+			DisabledRepoUnits = disabledRepoUnits
+			DefaultRepoUnits = defaultRepoUnits
+			DefaultForkRepoUnits = defaultForkRepoUnits
+		}(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
+		defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
+			setting.Repository.DisabledRepoUnits = disabledRepoUnits
+			setting.Repository.DefaultRepoUnits = defaultRepoUnits
+			setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
+		}(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
+
+		setting.Repository.DisabledRepoUnits = []string{"repo.issues", "repo.issues"}
+		setting.Repository.DefaultRepoUnits = []string{}
+		setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"}
+		assert.NoError(t, LoadUnitConfig())
+		assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
+		assert.ElementsMatch(t, []Type{TypeCode, TypePullRequests, TypeReleases, TypeWiki, TypePackages, TypeProjects}, DefaultRepoUnits)
+		assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
 	})
 }