From 270a2c7fa390bdd782cb59044775032fdb63dce9 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 26 Jan 2025 13:30:00 +0000 Subject: [PATCH] chore: remove usages of `sort.Sort` (#6689) improve language stats rounding: - Add tests (I had to omit some edge cases as the current method is non-determistic in some cases, due to random order of map access). - Document the algorithm used. - Lower the amount of calculations that need to be done. - Because of the aforementioned non-determistic don't use stable sort and instead regular sort. better sorting for `RepositoryList`: - Add testing - Use `slices.Sortfunc` instead of `sort.Sort`. - Remove the methods needed for `sort.Sort`. better git tag sorter: - Use `slices.SortFunc` instead of `sort.Sort`. - Remove `tagSorter` and its related methods. - Added testing. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6689 Reviewed-by: Earl Warren Co-authored-by: Gusted Co-committed-by: Gusted --- models/repo/language_stats.go | 34 ++++++++------- models/repo/language_stats_test.go | 66 ++++++++++++++++++++++++++++++ models/repo/repo_list.go | 12 ------ modules/git/repo_tag.go | 5 ++- modules/git/repo_tag_test.go | 3 ++ modules/git/tag.go | 21 ---------- routers/web/user/home.go | 5 ++- routers/web/user/home_test.go | 2 + 8 files changed, 97 insertions(+), 51 deletions(-) create mode 100644 models/repo/language_stats_test.go diff --git a/models/repo/language_stats.go b/models/repo/language_stats.go index 0bc0f1fb40..d44fea5375 100644 --- a/models/repo/language_stats.go +++ b/models/repo/language_stats.go @@ -4,9 +4,10 @@ package repo import ( + "cmp" "context" "math" - "sort" + "slices" "strings" "code.gitea.io/gitea/models/db" @@ -67,34 +68,37 @@ func (stats LanguageStatList) getLanguagePercentages() map[string]float32 { return langPerc } -// Rounds to 1 decimal point, target should be the expected sum of percs +// Use the quota method to round the percentages to one decimal place while +// keeping the sum of the percentages at 100%. func roundByLargestRemainder(percs map[string]float32, target float32) { + // Tracks the difference between the sum of percentage and 100%. leftToDistribute := int(target * 10) - keys := make([]string, 0, len(percs)) + type key struct { + language string + remainder float64 + } + keys := make([]key, 0, len(percs)) for k, v := range percs { - percs[k] = v * 10 - floored := math.Floor(float64(percs[k])) + floored, frac := math.Modf(float64(v * 10)) + percs[k] = float32(floored) leftToDistribute -= int(floored) - keys = append(keys, k) + keys = append(keys, key{language: k, remainder: frac}) } - // Sort the keys by the largest remainder - sort.SliceStable(keys, func(i, j int) bool { - _, remainderI := math.Modf(float64(percs[keys[i]])) - _, remainderJ := math.Modf(float64(percs[keys[j]])) - return remainderI > remainderJ + // Sort the fractional part in an ascending order. + slices.SortFunc(keys, func(b, a key) int { + return cmp.Compare(a.remainder, b.remainder) }) - // Increment the values in order of largest remainder + // As long as the sum of 100% is not reached, add 0.1% percentage. for _, k := range keys { - percs[k] = float32(math.Floor(float64(percs[k]))) if leftToDistribute > 0 { - percs[k]++ + percs[k.language]++ leftToDistribute-- } - percs[k] /= 10 + percs[k.language] /= 10 } } diff --git a/models/repo/language_stats_test.go b/models/repo/language_stats_test.go new file mode 100644 index 0000000000..dcfaeee6c9 --- /dev/null +++ b/models/repo/language_stats_test.go @@ -0,0 +1,66 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package repo + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestLanguagePercentages(t *testing.T) { + testCases := []struct { + input LanguageStatList + output map[string]float32 + }{ + { + []*LanguageStat{{Language: "Go", Size: 500}, {Language: "Rust", Size: 501}}, + map[string]float32{ + "Go": 50.0, + "Rust": 50.0, + }, + }, + { + []*LanguageStat{{Language: "Go", Size: 10}, {Language: "Rust", Size: 91}}, + map[string]float32{ + "Go": 9.9, + "Rust": 90.1, + }, + }, + { + []*LanguageStat{{Language: "Go", Size: 1}, {Language: "Rust", Size: 2}}, + map[string]float32{ + "Go": 33.3, + "Rust": 66.7, + }, + }, + { + []*LanguageStat{{Language: "Go", Size: 1}, {Language: "Rust", Size: 2}, {Language: "Shell", Size: 3}, {Language: "C#", Size: 4}, {Language: "Zig", Size: 5}, {Language: "Coq", Size: 6}, {Language: "Haskell", Size: 7}}, + map[string]float32{ + "Go": 3.6, + "Rust": 7.1, + "Shell": 10.7, + "C#": 14.3, + "Zig": 17.9, + "Coq": 21.4, + "Haskell": 25, + }, + }, + { + []*LanguageStat{{Language: "Go", Size: 1000}, {Language: "PHP", Size: 1}, {Language: "Java", Size: 1}}, + map[string]float32{ + "Go": 99.8, + "other": 0.2, + }, + }, + { + []*LanguageStat{}, + map[string]float32{}, + }, + } + + for _, testCase := range testCases { + assert.Equal(t, testCase.output, testCase.input.getLanguagePercentages()) + } +} diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index fc51f64f6a..693f8f12af 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -36,18 +36,6 @@ const RepositoryListDefaultPageSize = 64 // RepositoryList contains a list of repositories type RepositoryList []*Repository -func (repos RepositoryList) Len() int { - return len(repos) -} - -func (repos RepositoryList) Less(i, j int) bool { - return repos[i].FullName() < repos[j].FullName() -} - -func (repos RepositoryList) Swap(i, j int) { - repos[i], repos[j] = repos[j], repos[i] -} - // ValuesRepository converts a repository map to a list // FIXME: Remove in favor of maps.values when MIN_GO_VERSION >= 1.18 func ValuesRepository(m map[int64]*Repository) []*Repository { diff --git a/modules/git/repo_tag.go b/modules/git/repo_tag.go index 12b0c022cb..3b48b1fb9b 100644 --- a/modules/git/repo_tag.go +++ b/modules/git/repo_tag.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io" + "slices" "strings" "code.gitea.io/gitea/modules/git/foreachref" @@ -153,7 +154,9 @@ func (repo *Repository) GetTagInfos(page, pageSize int) ([]*Tag, int, error) { return nil, 0, fmt.Errorf("GetTagInfos: parse output: %w", err) } - sortTagsByTime(tags) + slices.SortFunc(tags, func(b, a *Tag) int { + return a.Tagger.When.Compare(b.Tagger.When) + }) tagsTotal := len(tags) if page != 0 { tags = util.PaginateSlice(tags, page, pageSize).([]*Tag) diff --git a/modules/git/repo_tag_test.go b/modules/git/repo_tag_test.go index 1cf420ad63..a4b13bf03d 100644 --- a/modules/git/repo_tag_test.go +++ b/modules/git/repo_tag_test.go @@ -6,6 +6,7 @@ package git import ( "path/filepath" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -30,9 +31,11 @@ func TestRepository_GetTags(t *testing.T) { assert.EqualValues(t, "signed-tag", tags[0].Name) assert.EqualValues(t, "36f97d9a96457e2bab511db30fe2db03893ebc64", tags[0].ID.String()) assert.EqualValues(t, "tag", tags[0].Type) + assert.EqualValues(t, time.Date(2022, time.November, 13, 16, 40, 20, 0, time.FixedZone("", 3600)), tags[0].Tagger.When) assert.EqualValues(t, "test", tags[1].Name) assert.EqualValues(t, "3ad28a9149a2864384548f3d17ed7f38014c9e8a", tags[1].ID.String()) assert.EqualValues(t, "tag", tags[1].Type) + assert.EqualValues(t, time.Date(2018, time.June, 16, 20, 13, 18, 0, time.FixedZone("", -25200)), tags[1].Tagger.When) } func TestRepository_GetTag(t *testing.T) { diff --git a/modules/git/tag.go b/modules/git/tag.go index 04f50e8db8..34ce8f6fc3 100644 --- a/modules/git/tag.go +++ b/modules/git/tag.go @@ -5,7 +5,6 @@ package git import ( "bytes" - "sort" "strings" api "code.gitea.io/gitea/modules/structs" @@ -107,23 +106,3 @@ l: return tag, nil } - -type tagSorter []*Tag - -func (ts tagSorter) Len() int { - return len([]*Tag(ts)) -} - -func (ts tagSorter) Less(i, j int) bool { - return []*Tag(ts)[i].Tagger.When.After([]*Tag(ts)[j].Tagger.When) -} - -func (ts tagSorter) Swap(i, j int) { - []*Tag(ts)[i], []*Tag(ts)[j] = []*Tag(ts)[j], []*Tag(ts)[i] -} - -// sortTagsByTime -func sortTagsByTime(tags []*Tag) { - sorter := tagSorter(tags) - sort.Sort(sorter) -} diff --git a/routers/web/user/home.go b/routers/web/user/home.go index c59dcf5c25..d67af29071 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -10,7 +10,6 @@ import ( "net/http" "regexp" "slices" - "sort" "strconv" "strings" @@ -242,7 +241,9 @@ func Milestones(ctx *context.Context) { ctx.ServerError("SearchRepositoryByCondition", err) return } - sort.Sort(showRepos) + slices.SortFunc(showRepos, func(a, b *repo_model.Repository) int { + return strings.Compare(a.FullName(), b.FullName()) + }) for i := 0; i < len(milestones); { for _, repo := range showRepos { diff --git a/routers/web/user/home_test.go b/routers/web/user/home_test.go index e1c8ca9a79..c09f609161 100644 --- a/routers/web/user/home_test.go +++ b/routers/web/user/home_test.go @@ -98,6 +98,8 @@ func TestMilestones(t *testing.T) { assert.EqualValues(t, 1, ctx.Data["Total"]) assert.Len(t, ctx.Data["Milestones"], 1) assert.Len(t, ctx.Data["Repos"], 2) // both repo 42 and 1 have milestones and both are owned by user 2 + assert.EqualValues(t, "user2/glob", ctx.Data["Repos"].(repo_model.RepositoryList)[0].FullName()) + assert.EqualValues(t, "user2/repo1", ctx.Data["Repos"].(repo_model.RepositoryList)[1].FullName()) } func TestMilestonesForSpecificRepo(t *testing.T) {