From ea4099918def119912d9b276fd7bbd871c44f962 Mon Sep 17 00:00:00 2001 From: Kemal Zebari Date: Thu, 9 May 2024 11:32:18 -0700 Subject: [PATCH] Respect Co-authored-by trailers in the contributors graph --- services/repository/contributors_graph.go | 199 +++++++++++------ .../repository/contributors_graph_test.go | 204 +++++++++++++----- .../1c/faee861da76612fda8951b94bde7ba3726109f | Bin 0 -> 117 bytes .../26/442ad16af268ef4768a5a30db377e860f504a3 | 1 + .../ea/3b10c09130232e534cecfed470dbc0f8eb2eb3 | 2 + .../refs/heads/branch-with-co-author | 1 + 6 files changed, 285 insertions(+), 122 deletions(-) create mode 100644 tests/gitea-repositories-meta/user2/repo2.git/objects/1c/faee861da76612fda8951b94bde7ba3726109f create mode 100644 tests/gitea-repositories-meta/user2/repo2.git/objects/26/442ad16af268ef4768a5a30db377e860f504a3 create mode 100644 tests/gitea-repositories-meta/user2/repo2.git/objects/ea/3b10c09130232e534cecfed470dbc0f8eb2eb3 create mode 100644 tests/gitea-repositories-meta/user2/repo2.git/refs/heads/branch-with-co-author diff --git a/services/repository/contributors_graph.go b/services/repository/contributors_graph.go index b0748f8ee3..474c49f626 100644 --- a/services/repository/contributors_graph.go +++ b/services/repository/contributors_graph.go @@ -8,6 +8,7 @@ import ( "context" "errors" "fmt" + "net/mail" "os" "strconv" "strings" @@ -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,25 @@ 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 + for scanner.Scan() { + line := scanner.Text() + if line == "" { + // There should be an empty line before we read the commit stats line. + break + } + coAuthorEmail, coAuthorName, err := parseCoAuthorTrailerValue(line) + if err != nil { + continue + } + 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 +204,8 @@ func getExtendedCommitStats(repo *git.Repository, revision string /*, limit int }, Date: date, }, - Stats: &commitStats, + CoAuthors: coAuthors, + Stats: &commitStats, } extendedCommitStats = append(extendedCommitStats, res) } @@ -199,6 +220,31 @@ func getExtendedCommitStats(repo *git.Repository, revision string /*, limit int return extendedCommitStats, nil } +var errSyntax error = errors.New("syntax error occurred") + +func parseCoAuthorTrailerValue(value string) (email, name string, err error) { + value = strings.TrimSpace(value) + if !strings.HasSuffix(value, ">") { + return "", "", errSyntax + } + if openEmailBracketIdx := strings.LastIndex(value, "<"); openEmailBracketIdx == -1 { + return "", "", errSyntax + } + + parts := strings.Split(value, "<") + if len(parts) < 2 { + return "", "", errSyntax + } + + email = strings.TrimRight(parts[1], ">") + if _, err := mail.ParseAddress(email); err != nil { + return "", "", err + } + name = strings.TrimSpace(parts[0]) + + return email, name, nil +} + func generateContributorStats(genDone chan struct{}, cache cache.StringCache, cacheKey string, repo *repo_model.Repository, revision string) { ctx := graceful.GetManager().HammerContext() @@ -222,8 +268,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 +281,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 +299,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..d572d758c2 100644 --- a/services/repository/contributors_graph_test.go +++ b/services/repository/contributors_graph_test.go @@ -20,66 +20,158 @@ 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"]) + }) - 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, + 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) + + // 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"]) + }, 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 0000000000000000000000000000000000000000..a87c676a688f8e7c97b80c5c9cf9a97df35c1dd1 GIT binary patch literal 117 zcmV-*0E+*30V^p=O;s>7FlR6{FfcPQQSivmP1VayVR&UNaA2Z=vR<&yn}1gdZXfvZ zT5mH{Nls>7s$OwfIz#f*ZGH0ZcFXysR{PJ-UX-4tVZ#koQ<7R-qF0fd!=U!VY0p~o XUk(d*o1EVIPI2Dj*fv`L#2zjA6Y@DQ literal 0 HcmV?d00001 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