Respect Co-authored-by trailers in the contributors graph

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++
+}

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"])
+	})
+
 }

tests/gitea-repositories-meta/user2/repo2.git/objects/26/442ad16af268ef4768a5a30db377e860f504a3

@@ -0,0 +1 @@
+x
uŽÁJ1E]ç+ÞLIÒf’ÈPJ…"¸vãî½äE'ÍRÐùzS\¹ps¹÷Â<C3B7>JÎsmLJV™A…„ÌnTí8*<2A>":oùE¶„{«G%}+V¾6Pr¯‰’²‰‚Œ>xCdÊE$vÉïµuVJ<56>·öQ*\J<>3V˜R)„õÄ_˜×…w¡ä#(«Œ¶Æi
ƒ¼Cýív<C3AD>+¼pÆÞ¸33LŸ÷µ1U<½gœ—pñºFlÏ%ó.G!žÊðëÁq ïG¸ÌÛç[<5B>)õJ½ý1?øçYa

tests/gitea-repositories-meta/user2/repo2.git/objects/ea/3b10c09130232e534cecfed470dbc0f8eb2eb3

@@ -0,0 +1,2 @@
+x
5х1─ PgNЯ┴ЁЁ╝nюh(	P#╞/▀и⌡·кЙ╟╜с▄CЦ╒хф°▓├.Ышb╞aтЬ°*Ц╔/T#╓
+┌в┘·.z▐ПZJЙж|╝.W

tests/gitea-repositories-meta/user2/repo2.git/refs/heads/branch-with-co-author

@@ -0,0 +1 @@
+26442ad16af268ef4768a5a30db377e860f504a3