Improve utils of slices (#22379)

- Move the file `compare.go` and `slice.go` to `slice.go`.
- Fix `ExistsInSlice`, it's buggy
  - It uses `sort.Search`, so it assumes that the input slice is sorted.
- It passes `func(i int) bool { return slice[i] == target })` to
`sort.Search`, that's incorrect, check the doc of `sort.Search`.
- Conbine `IsInt64InSlice(int64, []int64)` and `ExistsInSlice(string,
[]string)` to `SliceContains[T]([]T, T)`.
- Conbine `IsSliceInt64Eq([]int64, []int64)` and `IsEqualSlice([]string,
[]string)` to `SliceSortedEqual[T]([]T, T)`.
- Add `SliceEqual[T]([]T, T)` as a distinction from
`SliceSortedEqual[T]([]T, T)`.
- Redesign `RemoveIDFromList([]int64, int64) ([]int64, bool)` to
`SliceRemoveAll[T]([]T, T) []T`.
- Add `SliceContainsFunc[T]([]T, func(T) bool)` and
`SliceRemoveAllFunc[T]([]T, func(T) bool)` for general use.
- Add comments to explain why not `golang.org/x/exp/slices`.
- Add unit tests.

Author: wolfogre

cmd/admin.go

@@ -950,7 +950,7 @@ func parseSMTPConfig(c *cli.Context, conf *smtp.Source) error {
 	if c.IsSet("auth-type") {
 		conf.Auth = c.String("auth-type")
 		validAuthTypes := []string{"PLAIN", "LOGIN", "CRAM-MD5"}
-		if !contains(validAuthTypes, strings.ToUpper(c.String("auth-type"))) {
+		if !util.SliceContainsString(validAuthTypes, strings.ToUpper(c.String("auth-type"))) {
 			return errors.New("Auth must be one of PLAIN/LOGIN/CRAM-MD5")
 		}
 		conf.Auth = c.String("auth-type")

cmd/dump.go

@@ -409,15 +409,6 @@ func runDump(ctx *cli.Context) error {
 	return nil
 }
 
-func contains(slice []string, s string) bool {
-	for _, v := range slice {
-		if v == s {
-			return true
-		}
-	}
-	return false
-}
-
 // addRecursiveExclude zips absPath to specified insidePath inside writer excluding excludeAbsPath
 func addRecursiveExclude(w archiver.Writer, insidePath, absPath string, excludeAbsPath []string, verbose bool) error {
 	absPath, err := filepath.Abs(absPath)
@@ -438,7 +429,7 @@ func addRecursiveExclude(w archiver.Writer, insidePath, absPath string, excludeA
 		currentAbsPath := path.Join(absPath, file.Name())
 		currentInsidePath := path.Join(insidePath, file.Name())
 		if file.IsDir() {
-			if !contains(excludeAbsPath, currentAbsPath) {
+			if !util.SliceContainsString(excludeAbsPath, currentAbsPath) {
 				if err := addFile(w, currentInsidePath, currentAbsPath, false); err != nil {
 					return err
 				}

models/asymkey/ssh_key.go

@@ -409,14 +409,14 @@ func SynchronizePublicKeys(usr *user_model.User, s *auth.Source, sshPublicKeys [
 		sshKeySplit := strings.Split(v, " ")
 		if len(sshKeySplit) > 1 {
 			key := strings.Join(sshKeySplit[:2], " ")
-			if !util.ExistsInSlice(key, providedKeys) {
+			if !util.SliceContainsString(providedKeys, key) {
 				providedKeys = append(providedKeys, key)
 			}
 		}
 	}
 
 	// Check if Public Key sync is needed
-	if util.IsEqualSlice(giteaKeys, providedKeys) {
+	if util.SliceSortedEqual(giteaKeys, providedKeys) {
 		log.Trace("synchronizePublicKeys[%s]: Public Keys are already in sync for %s (Source:%v/DB:%v)", s.Name, usr.Name, len(providedKeys), len(giteaKeys))
 		return false
 	}
@@ -425,7 +425,7 @@ func SynchronizePublicKeys(usr *user_model.User, s *auth.Source, sshPublicKeys [
 	// Add new Public SSH Keys that doesn't already exist in DB
 	var newKeys []string
 	for _, key := range providedKeys {
-		if !util.ExistsInSlice(key, giteaKeys) {
+		if !util.SliceContainsString(giteaKeys, key) {
 			newKeys = append(newKeys, key)
 		}
 	}
@@ -436,7 +436,7 @@ func SynchronizePublicKeys(usr *user_model.User, s *auth.Source, sshPublicKeys [
 	// Mark keys from DB that no longer exist in the source for deletion
 	var giteaKeysToDelete []string
 	for _, giteaKey := range giteaKeys {
-		if !util.ExistsInSlice(giteaKey, providedKeys) {
+		if !util.SliceContainsString(providedKeys, giteaKey) {
 			log.Trace("synchronizePublicKeys[%s]: Marking Public SSH Key for deletion for user %s: %v", s.Name, usr.Name, giteaKey)
 			giteaKeysToDelete = append(giteaKeysToDelete, giteaKey)
 		}

models/auth/oauth2.go

@@ -69,13 +69,13 @@ func (app *OAuth2Application) ContainsRedirectURI(redirectURI string) bool {
 			if ip != nil && ip.IsLoopback() {
 				// strip port
 				uri.Host = uri.Hostname()
-				if util.IsStringInSlice(uri.String(), app.RedirectURIs, true) {
+				if util.SliceContainsString(app.RedirectURIs, uri.String(), true) {
 					return true
 				}
 			}
 		}
 	}
-	return util.IsStringInSlice(redirectURI, app.RedirectURIs, true)
+	return util.SliceContainsString(app.RedirectURIs, redirectURI, true)
 }
 
 // Base32 characters, but lowercased.

models/git/branches.go

@@ -342,7 +342,7 @@ func IsProtectedBranch(ctx context.Context, repoID int64, branchName string) (bo
 // updateApprovalWhitelist checks whether the user whitelist changed and returns a whitelist with
 // the users from newWhitelist which have explicit read or write access to the repo.
 func updateApprovalWhitelist(ctx context.Context, repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {
-	hasUsersChanged := !util.IsSliceInt64Eq(currentWhitelist, newWhitelist)
+	hasUsersChanged := !util.SliceSortedEqual(currentWhitelist, newWhitelist)
 	if !hasUsersChanged {
 		return currentWhitelist, nil
 	}
@@ -363,7 +363,7 @@ func updateApprovalWhitelist(ctx context.Context, repo *repo_model.Repository, c
 // updateUserWhitelist checks whether the user whitelist changed and returns a whitelist with
 // the users from newWhitelist which have write access to the repo.
 func updateUserWhitelist(ctx context.Context, repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {
-	hasUsersChanged := !util.IsSliceInt64Eq(currentWhitelist, newWhitelist)
+	hasUsersChanged := !util.SliceSortedEqual(currentWhitelist, newWhitelist)
 	if !hasUsersChanged {
 		return currentWhitelist, nil
 	}
@@ -392,7 +392,7 @@ func updateUserWhitelist(ctx context.Context, repo *repo_model.Repository, curre
 // updateTeamWhitelist checks whether the team whitelist changed and returns a whitelist with
 // the teams from newWhitelist which have write access to the repo.
 func updateTeamWhitelist(ctx context.Context, repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {
-	hasTeamsChanged := !util.IsSliceInt64Eq(currentWhitelist, newWhitelist)
+	hasTeamsChanged := !util.SliceSortedEqual(currentWhitelist, newWhitelist)
 	if !hasTeamsChanged {
 		return currentWhitelist, nil
 	}
@@ -404,7 +404,7 @@ func updateTeamWhitelist(ctx context.Context, repo *repo_model.Repository, curre
 
 	whitelist = make([]int64, 0, len(teams))
 	for i := range teams {
-		if util.IsInt64InSlice(teams[i].ID, newWhitelist) {
+		if util.SliceContains(newWhitelist, teams[i].ID) {
 			whitelist = append(whitelist, teams[i].ID)
 		}
 	}

models/issues/assignees.go

@@ -155,7 +155,7 @@ func MakeIDsFromAPIAssigneesToAdd(ctx context.Context, oneAssignee string, multi
 	var requestAssignees []string
 
 	// Keeping the old assigning method for compatibility reasons
-	if oneAssignee != "" && !util.IsStringInSlice(oneAssignee, multipleAssignees) {
+	if oneAssignee != "" && !util.SliceContainsString(multipleAssignees, oneAssignee) {
 		requestAssignees = append(requestAssignees, oneAssignee)
 	}
 

models/issues/issue.go

@@ -1529,7 +1529,7 @@ func IsUserParticipantsOfIssue(user *user_model.User, issue *Issue) bool {
 		log.Error(err.Error())
 		return false
 	}
-	return util.IsInt64InSlice(user.ID, userIDs)
+	return util.SliceContains(userIDs, user.ID)
 }
 
 // UpdateIssueMentions updates issue-user relations for mentioned users.
@@ -2023,7 +2023,7 @@ func (issue *Issue) GetParticipantIDsByIssue(ctx context.Context) ([]int64, erro
 		Find(&userIDs); err != nil {
 		return nil, fmt.Errorf("get poster IDs: %w", err)
 	}
-	if !util.IsInt64InSlice(issue.PosterID, userIDs) {
+	if !util.SliceContains(userIDs, issue.PosterID) {
 		return append(userIDs, issue.PosterID), nil
 	}
 	return userIDs, nil

models/org_team.go

@@ -398,20 +398,13 @@ func DeleteTeam(t *organization.Team) error {
 			return fmt.Errorf("findProtectedBranches: %w", err)
 		}
 		for _, p := range protections {
-			var matched1, matched2, matched3 bool
-			if len(p.WhitelistTeamIDs) != 0 {
-				p.WhitelistTeamIDs, matched1 = util.RemoveIDFromList(
-					p.WhitelistTeamIDs, t.ID)
-			}
-			if len(p.ApprovalsWhitelistTeamIDs) != 0 {
-				p.ApprovalsWhitelistTeamIDs, matched2 = util.RemoveIDFromList(
-					p.ApprovalsWhitelistTeamIDs, t.ID)
-			}
-			if len(p.MergeWhitelistTeamIDs) != 0 {
-				p.MergeWhitelistTeamIDs, matched3 = util.RemoveIDFromList(
-					p.MergeWhitelistTeamIDs, t.ID)
-			}
-			if matched1 || matched2 || matched3 {
+			lenIDs, lenApprovalIDs, lenMergeIDs := len(p.WhitelistTeamIDs), len(p.ApprovalsWhitelistTeamIDs), len(p.MergeWhitelistTeamIDs)
+			p.WhitelistTeamIDs = util.SliceRemoveAll(p.WhitelistTeamIDs, t.ID)
+			p.ApprovalsWhitelistTeamIDs = util.SliceRemoveAll(p.ApprovalsWhitelistTeamIDs, t.ID)
+			p.MergeWhitelistTeamIDs = util.SliceRemoveAll(p.MergeWhitelistTeamIDs, t.ID)
+			if lenIDs != len(p.WhitelistTeamIDs) ||
+				lenApprovalIDs != len(p.ApprovalsWhitelistTeamIDs) ||
+				lenMergeIDs != len(p.MergeWhitelistTeamIDs) {
 				if _, err = sess.ID(p.ID).Cols(
 					"whitelist_team_i_ds",
 					"merge_whitelist_team_i_ds",

models/user.go

@@ -141,20 +141,13 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
 				break
 			}
 			for _, p := range protections {
-				var matched1, matched2, matched3 bool
-				if len(p.WhitelistUserIDs) != 0 {
-					p.WhitelistUserIDs, matched1 = util.RemoveIDFromList(
-						p.WhitelistUserIDs, u.ID)
-				}
-				if len(p.ApprovalsWhitelistUserIDs) != 0 {
-					p.ApprovalsWhitelistUserIDs, matched2 = util.RemoveIDFromList(
-						p.ApprovalsWhitelistUserIDs, u.ID)
-				}
-				if len(p.MergeWhitelistUserIDs) != 0 {
-					p.MergeWhitelistUserIDs, matched3 = util.RemoveIDFromList(
-						p.MergeWhitelistUserIDs, u.ID)
-				}
-				if matched1 || matched2 || matched3 {
+				lenIDs, lenApprovalIDs, lenMergeIDs := len(p.WhitelistUserIDs), len(p.ApprovalsWhitelistUserIDs), len(p.MergeWhitelistUserIDs)
+				p.WhitelistUserIDs = util.SliceRemoveAll(p.WhitelistUserIDs, u.ID)
+				p.ApprovalsWhitelistUserIDs = util.SliceRemoveAll(p.ApprovalsWhitelistUserIDs, u.ID)
+				p.MergeWhitelistUserIDs = util.SliceRemoveAll(p.MergeWhitelistUserIDs, u.ID)
+				if lenIDs != len(p.WhitelistUserIDs) ||
+					lenApprovalIDs != len(p.ApprovalsWhitelistUserIDs) ||
+					lenMergeIDs != len(p.MergeWhitelistUserIDs) {
 					if _, err = e.ID(p.ID).Cols(
 						"whitelist_user_i_ds",
 						"merge_whitelist_user_i_ds",

modules/repository/init.go

@@ -170,7 +170,7 @@ func LoadRepoConfig() {
 			}
 
 			for _, f := range customFiles {
-				if !util.IsStringInSlice(f, files, true) {
+				if !util.SliceContainsString(files, f, true) {
 					files = append(files, f)
 				}
 			}
@@ -200,12 +200,12 @@ func LoadRepoConfig() {
 	// Filter out invalid names and promote preferred licenses.
 	sortedLicenses := make([]string, 0, len(Licenses))
 	for _, name := range setting.Repository.PreferredLicenses {
-		if util.IsStringInSlice(name, Licenses, true) {
+		if util.SliceContainsString(Licenses, name, true) {
 			sortedLicenses = append(sortedLicenses, name)
 		}
 	}
 	for _, name := range Licenses {
-		if !util.IsStringInSlice(name, setting.Repository.PreferredLicenses, true) {
+		if !util.SliceContainsString(setting.Repository.PreferredLicenses, name, true) {
 			sortedLicenses = append(sortedLicenses, name)
 		}
 	}