Skip to content

Commit

Permalink
Improve utils of slices (#22379)
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
wolfogre authored Jan 11, 2023
1 parent dc5f2cf commit 477a1cc
Show file tree
Hide file tree
Showing 22 changed files with 228 additions and 182 deletions.
2 changes: 1 addition & 1 deletion cmd/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
11 changes: 1 addition & 10 deletions cmd/dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions models/asymkey/ssh_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
}
}
Expand All @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions models/auth/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions models/git/branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
}
Expand Down
2 changes: 1 addition & 1 deletion models/issues/assignees.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
4 changes: 2 additions & 2 deletions models/issues/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
21 changes: 7 additions & 14 deletions models/org_team.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
21 changes: 7 additions & 14 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 3 additions & 3 deletions modules/repository/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand Down
92 changes: 0 additions & 92 deletions modules/util/compare.go

This file was deleted.

Loading

0 comments on commit 477a1cc

Please sign in to comment.