diff --git a/modules/util/commit_trailers.go b/modules/util/commit_trailers.go new file mode 100644 index 0000000000..9c6547d0cd --- /dev/null +++ b/modules/util/commit_trailers.go @@ -0,0 +1,42 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package util + +import ( + "errors" + "net/mail" + "strings" +) + +var ErrInvalidCommitTrailerValueSyntax = errors.New("syntax error occurred while parsing a commit trailer value") + +// ParseCommitTrailerValueWithAuthor parses a commit trailer value that contains author data. +// Note that it only parses the value and does not consider the trailer key i.e. we just +// parse something like the following: +// +// Foo Bar +func ParseCommitTrailerValueWithAuthor(value string) (name, email string, err error) { + value = strings.TrimSpace(value) + if !strings.HasSuffix(value, ">") { + return "", "", ErrInvalidCommitTrailerValueSyntax + } + + closedBracketIdx := len(value) - 1 + openBracketIdx := strings.LastIndex(value, "<") + if openBracketIdx == -1 { + return "", "", ErrInvalidCommitTrailerValueSyntax + } + + email = value[openBracketIdx+1 : closedBracketIdx] + if _, err := mail.ParseAddress(email); err != nil { + return "", "", ErrInvalidCommitTrailerValueSyntax + } + + name = strings.TrimSpace(value[:openBracketIdx]) + if len(name) == 0 { + return "", "", ErrInvalidCommitTrailerValueSyntax + } + + return name, email, nil +} diff --git a/modules/util/commit_trailers_test.go b/modules/util/commit_trailers_test.go new file mode 100644 index 0000000000..fe9b64d896 --- /dev/null +++ b/modules/util/commit_trailers_test.go @@ -0,0 +1,40 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package util + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseCommitTrailerValueWithAuthor(t *testing.T) { + cases := []struct { + input string + shouldBeError bool + expectedName string + expectedEmail string + }{ + {"Foo Bar ", true, "", ""}, + {"Foo Bar <>", true, "", ""}, + {"Foo Bar ", true, "", ""}, + {"", true, "", ""}, + {" ", true, "", ""}, + {"Foo Bar ", false, "Foo Bar", "foobar@example.com"}, + {" Foo Bar ", false, "Foo Bar", "foobar@example.com"}, + // Account for edge case where name contains an open bracket. + {" Foo < Bar ", false, "Foo < Bar", "foobar@example.com"}, + } + + for n, c := range cases { + name, email, err := ParseCommitTrailerValueWithAuthor(c.input) + if c.shouldBeError { + assert.Error(t, err, "case %d should be a syntax error", n) + } else { + assert.Equal(t, c.expectedName, name, "case %d should have correct name", n) + assert.Equal(t, c.expectedEmail, email, "case %d should have correct email", n) + } + } +} diff --git a/services/repository/contributors_graph.go b/services/repository/contributors_graph.go index b0748f8ee3..41c9c4f8ab 100644 --- a/services/repository/contributors_graph.go +++ b/services/repository/contributors_graph.go @@ -23,6 +23,7 @@ import ( "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" ) const ( @@ -53,10 +54,11 @@ type ContributorData struct { Weeks map[int64]*WeekData `json:"weeks"` } -// ExtendedCommitStats contains information for commit stats with author data +// ExtendedCommitStats contains information for commit stats with both author and coauthors data type ExtendedCommitStats struct { - Author *api.CommitUser `json:"author"` - Stats *api.CommitStats `json:"stats"` + Author *api.CommitUser `json:"author"` + CoAuthors []*api.CommitUser `json:"co_authors"` + Stats *api.CommitStats `json:"stats"` } const layout = time.DateOnly @@ -125,8 +127,7 @@ func getExtendedCommitStats(repo *git.Repository, revision string /*, limit int _ = stdoutWriter.Close() }() - gitCmd := git.NewCommand(repo.Ctx, "log", "--shortstat", "--no-merges", "--pretty=format:---%n%aN%n%aE%n%as", "--reverse") - // AddOptionFormat("--max-count=%d", limit) + gitCmd := git.NewCommand(repo.Ctx, "log", "--shortstat", "--no-merges", "--pretty=format:---%n%aN%n%aE%n%as%n%(trailers:key=Co-authored-by,valueonly=true)", "--reverse") gitCmd.AddDynamicArguments(baseCommit.ID.String()) var extendedCommitStats []*ExtendedCommitStats @@ -150,6 +151,30 @@ func getExtendedCommitStats(repo *git.Repository, revision string /*, limit int authorEmail := strings.TrimSpace(scanner.Text()) scanner.Scan() date := strings.TrimSpace(scanner.Text()) + + var coAuthors []*api.CommitUser + emailSet := map[string]bool{} + for scanner.Scan() { + line := scanner.Text() + if line == "" { + // There should be an empty line before we read the commit stats line. + break + } + coAuthorName, coAuthorEmail, err := util.ParseCommitTrailerValueWithAuthor(line) + if err != nil { + continue + } + if _, exists := emailSet[coAuthorEmail]; exists { + continue + } + emailSet[coAuthorEmail] = true + coAuthor := &api.CommitUser{ + Identity: api.Identity{Name: coAuthorName, Email: coAuthorEmail}, + Date: date, + } + coAuthors = append(coAuthors, coAuthor) + } + scanner.Scan() stats := strings.TrimSpace(scanner.Text()) if authorName == "" || authorEmail == "" || date == "" || stats == "" { @@ -184,7 +209,8 @@ func getExtendedCommitStats(repo *git.Repository, revision string /*, limit int }, Date: date, }, - Stats: &commitStats, + CoAuthors: coAuthors, + Stats: &commitStats, } extendedCommitStats = append(extendedCommitStats, res) } @@ -222,8 +248,6 @@ func generateContributorStats(genDone chan struct{}, cache cache.StringCache, ca return } - layout := time.DateOnly - unknownUserAvatarLink := user_model.NewGhostUser().AvatarLinkWithSize(ctx, 0) contributorsCommitStats := make(map[string]*ContributorData) contributorsCommitStats["total"] = &ContributorData{ @@ -237,67 +261,16 @@ func generateContributorStats(genDone chan struct{}, cache cache.StringCache, ca if len(userEmail) == 0 { continue } - u, _ := user_model.GetUserByEmail(ctx, userEmail) - if u != nil { - // update userEmail with user's primary email address so - // that different mail addresses will linked to same account - userEmail = u.GetEmail() - } - // duplicated logic - if _, ok := contributorsCommitStats[userEmail]; !ok { - if u == nil { - avatarLink := avatars.GenerateEmailAvatarFastLink(ctx, userEmail, 0) - if avatarLink == "" { - avatarLink = unknownUserAvatarLink - } - contributorsCommitStats[userEmail] = &ContributorData{ - Name: v.Author.Name, - AvatarLink: avatarLink, - Weeks: make(map[int64]*WeekData), - } - } else { - contributorsCommitStats[userEmail] = &ContributorData{ - Name: u.DisplayName(), - Login: u.LowerName, - AvatarLink: u.AvatarLinkWithSize(ctx, 0), - HomeLink: u.HomeLink(), - Weeks: make(map[int64]*WeekData), - } - } - } - // Update user statistics - user := contributorsCommitStats[userEmail] - startingOfWeek, _ := findLastSundayBeforeDate(v.Author.Date) - val, _ := time.Parse(layout, startingOfWeek) - week := val.UnixMilli() + authorData := getContributorData(ctx, contributorsCommitStats, v.Author, unknownUserAvatarLink) + date := v.Author.Date + stats := v.Stats + updateUserAndOverallStats(stats, date, authorData, total, false) - if user.Weeks[week] == nil { - user.Weeks[week] = &WeekData{ - Additions: 0, - Deletions: 0, - Commits: 0, - Week: week, - } + for _, coAuthor := range v.CoAuthors { + coAuthorData := getContributorData(ctx, contributorsCommitStats, coAuthor, unknownUserAvatarLink) + updateUserAndOverallStats(stats, date, coAuthorData, total, true) } - if total.Weeks[week] == nil { - total.Weeks[week] = &WeekData{ - Additions: 0, - Deletions: 0, - Commits: 0, - Week: week, - } - } - user.Weeks[week].Additions += v.Stats.Additions - user.Weeks[week].Deletions += v.Stats.Deletions - user.Weeks[week].Commits++ - user.TotalCommits++ - - // Update overall statistics - total.Weeks[week].Additions += v.Stats.Additions - total.Weeks[week].Deletions += v.Stats.Deletions - total.Weeks[week].Commits++ - total.TotalCommits++ } _ = cache.PutJSON(cacheKey, contributorsCommitStats, contributorStatsCacheTimeout) @@ -306,3 +279,77 @@ func generateContributorStats(genDone chan struct{}, cache cache.StringCache, ca genDone <- struct{}{} } } + +func getContributorData(ctx context.Context, contributorsCommitStats map[string]*ContributorData, user *api.CommitUser, defaultUserAvatarLink string) *ContributorData { + userEmail := user.Email + u, _ := user_model.GetUserByEmail(ctx, userEmail) + if u != nil { + // update userEmail with user's primary email address so + // that different mail addresses will linked to same account + userEmail = u.GetEmail() + } + + if _, ok := contributorsCommitStats[userEmail]; !ok { + if u == nil { + avatarLink := avatars.GenerateEmailAvatarFastLink(ctx, userEmail, 0) + if avatarLink == "" { + avatarLink = defaultUserAvatarLink + } + contributorsCommitStats[userEmail] = &ContributorData{ + Name: user.Name, + AvatarLink: avatarLink, + Weeks: make(map[int64]*WeekData), + } + } else { + contributorsCommitStats[userEmail] = &ContributorData{ + Name: u.DisplayName(), + Login: u.LowerName, + AvatarLink: u.AvatarLinkWithSize(ctx, 0), + HomeLink: u.HomeLink(), + Weeks: make(map[int64]*WeekData), + } + } + } + return contributorsCommitStats[userEmail] +} + +func updateUserAndOverallStats(stats *api.CommitStats, commitDate string, user, total *ContributorData, isCoAuthor bool) { + startingOfWeek, _ := findLastSundayBeforeDate(commitDate) + + val, _ := time.Parse(layout, startingOfWeek) + week := val.UnixMilli() + + if user.Weeks[week] == nil { + user.Weeks[week] = &WeekData{ + Additions: 0, + Deletions: 0, + Commits: 0, + Week: week, + } + } + if total.Weeks[week] == nil { + total.Weeks[week] = &WeekData{ + Additions: 0, + Deletions: 0, + Commits: 0, + Week: week, + } + } + // Update user statistics + user.Weeks[week].Additions += stats.Additions + user.Weeks[week].Deletions += stats.Deletions + user.Weeks[week].Commits++ + user.TotalCommits++ + + if isCoAuthor { + // We would have or will count these additions/deletions/commits already when we encounter the original + // author of the commit. Let's avoid this duplication. + return + } + + // Update overall statistics + total.Weeks[week].Additions += stats.Additions + total.Weeks[week].Deletions += stats.Deletions + total.Weeks[week].Commits++ + total.TotalCommits++ +} diff --git a/services/repository/contributors_graph_test.go b/services/repository/contributors_graph_test.go index f22c115276..b2df72fbfc 100644 --- a/services/repository/contributors_graph_test.go +++ b/services/repository/contributors_graph_test.go @@ -20,66 +20,240 @@ func TestRepository_ContributorsGraph(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) assert.NoError(t, repo.LoadOwner(db.DefaultContext)) - mockCache, err := cache.NewStringCache(setting.Cache{}) - assert.NoError(t, err) - generateContributorStats(nil, mockCache, "key", repo, "404ref") - var data map[string]*ContributorData - _, getErr := mockCache.GetJSON("key", &data) - assert.NotNil(t, getErr) - assert.ErrorContains(t, getErr.ToError(), "object does not exist") + t.Run("non-existent revision", func(t *testing.T) { + mockCache, err := cache.NewStringCache(setting.Cache{}) + assert.NoError(t, err) + generateContributorStats(nil, mockCache, "key", repo, "404ref") + var data map[string]*ContributorData + _, getErr := mockCache.GetJSON("key", &data) + assert.NotNil(t, getErr) + assert.ErrorContains(t, getErr.ToError(), "object does not exist") + }) + t.Run("generate contributor stats", func(t *testing.T) { + mockCache, err := cache.NewStringCache(setting.Cache{}) + assert.NoError(t, err) + generateContributorStats(nil, mockCache, "key", repo, "master") + var data map[string]*ContributorData + exist, _ := mockCache.GetJSON("key", &data) + assert.True(t, exist) + var keys []string + for k := range data { + keys = append(keys, k) + } + slices.Sort(keys) + assert.EqualValues(t, []string{ + "ethantkoenig@gmail.com", + "jimmy.praet@telenet.be", + "jon@allspice.io", + "total", // generated summary + }, keys) - generateContributorStats(nil, mockCache, "key2", repo, "master") - exist, _ := mockCache.GetJSON("key2", &data) - assert.True(t, exist) - var keys []string - for k := range data { - keys = append(keys, k) - } - slices.Sort(keys) - assert.EqualValues(t, []string{ - "ethantkoenig@gmail.com", - "jimmy.praet@telenet.be", - "jon@allspice.io", - "total", // generated summary - }, keys) + assert.EqualValues(t, &ContributorData{ + Name: "Ethan Koenig", + AvatarLink: "https://secure.gravatar.com/avatar/b42fb195faa8c61b8d88abfefe30e9e3?d=identicon", + TotalCommits: 1, + Weeks: map[int64]*WeekData{ + 1511654400000: { + Week: 1511654400000, // sunday 2017-11-26 + Additions: 3, + Deletions: 0, + Commits: 1, + }, + }, + }, data["ethantkoenig@gmail.com"]) + assert.EqualValues(t, &ContributorData{ + Name: "Total", + AvatarLink: "", + TotalCommits: 3, + Weeks: map[int64]*WeekData{ + 1511654400000: { + Week: 1511654400000, // sunday 2017-11-26 (2017-11-26 20:31:18 -0800) + Additions: 3, + Deletions: 0, + Commits: 1, + }, + 1607817600000: { + Week: 1607817600000, // sunday 2020-12-13 (2020-12-15 15:23:11 -0500) + Additions: 10, + Deletions: 0, + Commits: 1, + }, + 1624752000000: { + Week: 1624752000000, // sunday 2021-06-27 (2021-06-29 21:54:09 +0200) + Additions: 2, + Deletions: 0, + Commits: 1, + }, + }, + }, data["total"]) + }) + t.Run("generate contributor stats with co-authored commit", func(t *testing.T) { + mockCache, err := cache.NewStringCache(setting.Cache{}) + assert.NoError(t, err) + generateContributorStats(nil, mockCache, "key", repo, "branch-with-co-author") + var data map[string]*ContributorData + exist, _ := mockCache.GetJSON("key", &data) + assert.True(t, exist) + var keys []string + for k := range data { + keys = append(keys, k) + } + slices.Sort(keys) + assert.EqualValues(t, []string{ + "ethantkoenig@gmail.com", + "fizzbuzz@example.com", + "foobar@example.com", + "jimmy.praet@telenet.be", + "jon@allspice.io", + "total", + }, keys) - assert.EqualValues(t, &ContributorData{ - Name: "Ethan Koenig", - AvatarLink: "https://secure.gravatar.com/avatar/b42fb195faa8c61b8d88abfefe30e9e3?d=identicon", - TotalCommits: 1, - Weeks: map[int64]*WeekData{ - 1511654400000: { - Week: 1511654400000, // sunday 2017-11-26 - Additions: 3, - Deletions: 0, - Commits: 1, + // make sure we can see the author of the commit + assert.EqualValues(t, &ContributorData{ + Name: "Foo Bar", + AvatarLink: "https://secure.gravatar.com/avatar/0d4907cea9d97688aa7a5e722d742f71?d=identicon", + TotalCommits: 1, + Weeks: map[int64]*WeekData{ + 1714867200000: { + Week: 1714867200000, // sunday 2024-05-05 + Additions: 1, + Deletions: 1, + Commits: 1, + }, }, - }, - }, data["ethantkoenig@gmail.com"]) - assert.EqualValues(t, &ContributorData{ - Name: "Total", - AvatarLink: "", - TotalCommits: 3, - Weeks: map[int64]*WeekData{ - 1511654400000: { - Week: 1511654400000, // sunday 2017-11-26 (2017-11-26 20:31:18 -0800) - Additions: 3, - Deletions: 0, - Commits: 1, + }, data["foobar@example.com"]) + + // make sure that we can also see the co-author + assert.EqualValues(t, &ContributorData{ + Name: "Fizz Buzz", + AvatarLink: "https://secure.gravatar.com/avatar/474e3516254f43b2337011c4ac4de421?d=identicon", + TotalCommits: 1, + Weeks: map[int64]*WeekData{ + 1714867200000: { + Week: 1714867200000, // sunday 2024-05-05 + Additions: 1, + Deletions: 1, + Commits: 1, + }, }, - 1607817600000: { - Week: 1607817600000, // sunday 2020-12-13 (2020-12-15 15:23:11 -0500) - Additions: 10, - Deletions: 0, - Commits: 1, + }, data["fizzbuzz@example.com"]) + + // let's also make sure we don't duplicate the additions/deletions/commits counts in the overall stats that week + assert.EqualValues(t, &ContributorData{ + Name: "Total", + AvatarLink: "", + TotalCommits: 4, + Weeks: map[int64]*WeekData{ + 1714867200000: { + Week: 1714867200000, // sunday 2024-05-05 + Additions: 1, + Deletions: 1, + Commits: 1, + }, + 1511654400000: { + Week: 1511654400000, // sunday 2017-11-26 + Additions: 3, + Deletions: 0, + Commits: 1, + }, + 1607817600000: { + Week: 1607817600000, // sunday 2020-12-13 + Additions: 10, + Deletions: 0, + Commits: 1, + }, + 1624752000000: { + Week: 1624752000000, // sunday 2021-06-27 + Additions: 2, + Deletions: 0, + Commits: 1, + }, }, - 1624752000000: { - Week: 1624752000000, // sunday 2021-06-27 (2021-06-29 21:54:09 +0200) - Additions: 2, - Deletions: 0, - Commits: 1, + }, data["total"]) + }) + t.Run("generate contributor stats with commit that has duplicate co-authored lines", func(t *testing.T) { + mockCache, err := cache.NewStringCache(setting.Cache{}) + assert.NoError(t, err) + generateContributorStats(nil, mockCache, "key", repo, "branch-with-duplicated-co-author-entries") + var data map[string]*ContributorData + exist, _ := mockCache.GetJSON("key", &data) + assert.True(t, exist) + var keys []string + for k := range data { + keys = append(keys, k) + } + slices.Sort(keys) + assert.EqualValues(t, []string{ + "ethantkoenig@gmail.com", + "fizzbuzz@example.com", + "foobar@example.com", + "jimmy.praet@telenet.be", + "jon@allspice.io", + "total", + }, keys) + + // make sure we can see the author of the commit + assert.EqualValues(t, &ContributorData{ + Name: "Foo Bar", + AvatarLink: "https://secure.gravatar.com/avatar/0d4907cea9d97688aa7a5e722d742f71?d=identicon", + TotalCommits: 1, + Weeks: map[int64]*WeekData{ + 1715472000000: { + Week: 1715472000000, // sunday 2024-05-12 + Additions: 1, + Deletions: 0, + Commits: 1, + }, }, - }, - }, data["total"]) + }, data["foobar@example.com"]) + + // make sure that we can also see the co-author and that we don't see duplicated additions/deletions/commits + assert.EqualValues(t, &ContributorData{ + Name: "Fizz Buzz", + AvatarLink: "https://secure.gravatar.com/avatar/474e3516254f43b2337011c4ac4de421?d=identicon", + TotalCommits: 1, + Weeks: map[int64]*WeekData{ + 1715472000000: { + Week: 1715472000000, // sunday 2024-05-12 + Additions: 1, + Deletions: 0, + Commits: 1, + }, + }, + }, data["fizzbuzz@example.com"]) + + // let's also make sure we don't duplicate the additions/deletions/commits counts in the overall stats that week + assert.EqualValues(t, &ContributorData{ + Name: "Total", + AvatarLink: "", + TotalCommits: 4, + Weeks: map[int64]*WeekData{ + 1715472000000: { + Week: 1715472000000, // sunday 2024-05-12 + Additions: 1, + Deletions: 0, + Commits: 1, + }, + 1511654400000: { + Week: 1511654400000, // sunday 2017-11-26 + Additions: 3, + Deletions: 0, + Commits: 1, + }, + 1607817600000: { + Week: 1607817600000, // sunday 2020-12-13 + Additions: 10, + Deletions: 0, + Commits: 1, + }, + 1624752000000: { + Week: 1624752000000, // sunday 2021-06-27 + Additions: 2, + Deletions: 0, + Commits: 1, + }, + }, + }, data["total"]) + }) } diff --git a/tests/gitea-repositories-meta/user2/repo2.git/objects/1c/faee861da76612fda8951b94bde7ba3726109f b/tests/gitea-repositories-meta/user2/repo2.git/objects/1c/faee861da76612fda8951b94bde7ba3726109f new file mode 100644 index 0000000000..a87c676a68 Binary files /dev/null and b/tests/gitea-repositories-meta/user2/repo2.git/objects/1c/faee861da76612fda8951b94bde7ba3726109f differ diff --git a/tests/gitea-repositories-meta/user2/repo2.git/objects/26/442ad16af268ef4768a5a30db377e860f504a3 b/tests/gitea-repositories-meta/user2/repo2.git/objects/26/442ad16af268ef4768a5a30db377e860f504a3 new file mode 100644 index 0000000000..4645515be7 --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo2.git/objects/26/442ad16af268ef4768a5a30db377e860f504a3 @@ -0,0 +1 @@ +xuJ1E]+LIfPJ"vE'RzS\psJsmLJVAnT8*":oE{G%}+V6Pr>xCdE$vuVJQ*\J3VR)_ׅw#(i Cv+p޸33L1U