From 443f7d59f9606a9ec3ba331cd23d9675aa45c471 Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 24 Jan 2025 16:45:46 +0000 Subject: [PATCH] chore: teach `set` module about `iter.Seq` (#6676) - Add a new `Seq` function to the `Set` type, this returns an iterator over the values. - Convert some users of the `Values` method to allow for more optimal code. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6676 Reviewed-by: Earl Warren Co-authored-by: Gusted Co-committed-by: Gusted --- cmd/web.go | 2 +- models/user/user_test.go | 4 ++-- modules/assetfs/layered.go | 8 +++----- modules/container/set.go | 11 +++++++++++ modules/container/set_test.go | 9 +++++++++ modules/util/slice.go | 8 -------- routers/web/user/package.go | 20 ++++++++++---------- services/packages/alt/repository.go | 2 +- services/release/release.go | 2 +- 9 files changed, 38 insertions(+), 28 deletions(-) diff --git a/cmd/web.go b/cmd/web.go index 3fc64f7748..787411939c 100644 --- a/cmd/web.go +++ b/cmd/web.go @@ -195,7 +195,7 @@ func serveInstalled(ctx *cli.Context) error { publicFilesSet.Remove(".well-known") publicFilesSet.Remove("assets") publicFilesSet.Remove("robots.txt") - for _, fn := range publicFilesSet.Values() { + for fn := range publicFilesSet.Seq() { log.Error("Found legacy public asset %q in CustomPath. Please move it to %s/public/assets/%s", fn, setting.CustomPath, fn) } if _, err := os.Stat(filepath.Join(setting.CustomPath, "robots.txt")); err == nil { diff --git a/models/user/user_test.go b/models/user/user_test.go index df0c3856e9..2c8c1609fd 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -715,7 +715,7 @@ func TestDisabledUserFeatures(t *testing.T) { // no features should be disabled with a plain login type assert.LessOrEqual(t, user.LoginType, auth.Plain) assert.Empty(t, user_model.DisabledFeaturesWithLoginType(user).Values()) - for _, f := range testValues.Values() { + for f := range testValues.Seq() { assert.False(t, user_model.IsFeatureDisabledWithLoginType(user, f)) } @@ -724,7 +724,7 @@ func TestDisabledUserFeatures(t *testing.T) { // all features should be disabled assert.NotEmpty(t, user_model.DisabledFeaturesWithLoginType(user).Values()) - for _, f := range testValues.Values() { + for f := range testValues.Seq() { assert.True(t, user_model.IsFeatureDisabledWithLoginType(user, f)) } } diff --git a/modules/assetfs/layered.go b/modules/assetfs/layered.go index 9678d23ad6..9feabc3f8c 100644 --- a/modules/assetfs/layered.go +++ b/modules/assetfs/layered.go @@ -11,7 +11,7 @@ import ( "net/http" "os" "path/filepath" - "sort" + "slices" "time" "code.gitea.io/gitea/modules/container" @@ -143,8 +143,7 @@ func (l *LayeredFS) ListFiles(name string, fileMode ...bool) ([]string, error) { } } } - files := fileSet.Values() - sort.Strings(files) + files := slices.Sorted(fileSet.Seq()) return files, nil } @@ -184,8 +183,7 @@ func listAllFiles(layers []*Layer, name string, fileMode ...bool) ([]string, err if err := list(name); err != nil { return nil, err } - files := fileSet.Values() - sort.Strings(files) + files := slices.Sorted(fileSet.Seq()) return files, nil } diff --git a/modules/container/set.go b/modules/container/set.go index 2d654d0aee..70f837bc66 100644 --- a/modules/container/set.go +++ b/modules/container/set.go @@ -3,6 +3,11 @@ package container +import ( + "iter" + "maps" +) + type Set[T comparable] map[T]struct{} // SetOf creates a set and adds the specified elements to it. @@ -63,3 +68,9 @@ func (s Set[T]) Values() []T { } return keys } + +// Seq returns a iterator over the elements in the set. +// It returns a single-use iterator. +func (s Set[T]) Seq() iter.Seq[T] { + return maps.Keys(s) +} diff --git a/modules/container/set_test.go b/modules/container/set_test.go index 3cfbf7cc2c..e54e31a052 100644 --- a/modules/container/set_test.go +++ b/modules/container/set_test.go @@ -4,6 +4,7 @@ package container import ( + "slices" "testing" "github.com/stretchr/testify/assert" @@ -29,6 +30,14 @@ func TestSet(t *testing.T) { assert.True(t, s.Contains("key4")) assert.True(t, s.Contains("key5")) + values := s.Values() + called := 0 + for value := range s.Seq() { + called++ + assert.True(t, slices.Contains(values, value)) + } + assert.EqualValues(t, len(values), called) + s = SetOf("key6", "key7") assert.False(t, s.Contains("key1")) assert.True(t, s.Contains("key6")) diff --git a/modules/util/slice.go b/modules/util/slice.go index 9c878c24be..80c8e62f6f 100644 --- a/modules/util/slice.go +++ b/modules/util/slice.go @@ -4,7 +4,6 @@ package util import ( - "cmp" "slices" "strings" ) @@ -47,13 +46,6 @@ func SliceRemoveAll[T comparable](slice []T, target T) []T { return slices.DeleteFunc(slice, func(t T) bool { return t == target }) } -// Sorted returns the sorted slice -// Note: The parameter is sorted inline. -func Sorted[S ~[]E, E cmp.Ordered](values S) S { - slices.Sort(values) - return values -} - // TODO: Replace with "maps.Values" once available, current it only in golang.org/x/exp/maps but not in standard library func ValuesOfMap[K comparable, V any](m map[K]V) []V { values := make([]V, 0, len(m)) diff --git a/routers/web/user/package.go b/routers/web/user/package.go index 707c86db7a..70ea20d388 100644 --- a/routers/web/user/package.go +++ b/routers/web/user/package.go @@ -6,6 +6,7 @@ package user import ( "fmt" "net/http" + "slices" "code.gitea.io/gitea/models/db" org_model "code.gitea.io/gitea/models/organization" @@ -23,7 +24,6 @@ import ( debian_module "code.gitea.io/gitea/modules/packages/debian" rpm_module "code.gitea.io/gitea/modules/packages/rpm" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" packages_helper "code.gitea.io/gitea/routers/api/packages/helper" shared_user "code.gitea.io/gitea/routers/web/shared/user" @@ -200,9 +200,9 @@ func ViewPackageVersion(ctx *context.Context) { } } - ctx.Data["Branches"] = util.Sorted(branches.Values()) - ctx.Data["Repositories"] = util.Sorted(repositories.Values()) - ctx.Data["Architectures"] = util.Sorted(architectures.Values()) + ctx.Data["Branches"] = slices.Sorted(branches.Seq()) + ctx.Data["Repositories"] = slices.Sorted(repositories.Seq()) + ctx.Data["Architectures"] = slices.Sorted(architectures.Seq()) case packages_model.TypeArch: ctx.Data["SignMail"] = fmt.Sprintf("%s@noreply.%s", ctx.Package.Owner.Name, setting.Packages.RegistryHost) groups := make(container.Set[string]) @@ -213,7 +213,7 @@ func ViewPackageVersion(ctx *context.Context) { } } } - ctx.Data["Groups"] = util.Sorted(groups.Values()) + ctx.Data["Groups"] = slices.Sorted(groups.Seq()) case packages_model.TypeDebian: distributions := make(container.Set[string]) components := make(container.Set[string]) @@ -232,9 +232,9 @@ func ViewPackageVersion(ctx *context.Context) { } } - ctx.Data["Distributions"] = util.Sorted(distributions.Values()) - ctx.Data["Components"] = util.Sorted(components.Values()) - ctx.Data["Architectures"] = util.Sorted(architectures.Values()) + ctx.Data["Distributions"] = slices.Sorted(distributions.Seq()) + ctx.Data["Components"] = slices.Sorted(components.Seq()) + ctx.Data["Architectures"] = slices.Sorted(architectures.Seq()) case packages_model.TypeRpm, packages_model.TypeAlt: groups := make(container.Set[string]) architectures := make(container.Set[string]) @@ -250,8 +250,8 @@ func ViewPackageVersion(ctx *context.Context) { } } - ctx.Data["Groups"] = util.Sorted(groups.Values()) - ctx.Data["Architectures"] = util.Sorted(architectures.Values()) + ctx.Data["Groups"] = slices.Sorted(groups.Seq()) + ctx.Data["Architectures"] = slices.Sorted(architectures.Seq()) } var ( diff --git a/services/packages/alt/repository.go b/services/packages/alt/repository.go index 7b7951eebb..f49c435e64 100644 --- a/services/packages/alt/repository.go +++ b/services/packages/alt/repository.go @@ -711,7 +711,7 @@ func buildRelease(ctx context.Context, pv *packages_model.PackageVersion, pfs [] architectures.Add(pd.FileMetadata.Architecture) } - for architecture := range architectures { + for architecture := range architectures.Seq() { version := time.Now().Unix() label := setting.AppName data := fmt.Sprintf(`Archive: Alt Linux Team diff --git a/services/release/release.go b/services/release/release.go index 876514beec..b52e4b124e 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -372,7 +372,7 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo return err } - for _, uuid := range delAttachmentUUIDs.Values() { + for uuid := range delAttachmentUUIDs.Seq() { if err := storage.Attachments.Delete(repo_model.AttachmentRelativePath(uuid)); err != nil { // Even delete files failed, but the attachments has been removed from database, so we // should not return error but only record the error on logs.