Merge pull request #4761 from linuxfoundation/unicron-4760-update-co-authors-lookup-algorithm

Update co-authors searching algorithm - support lookup easyCLA DynamoDB database by emails

Author: lukaszgryglicki

CO_AUTHORS.md

@@ -155,7 +155,7 @@ If it does, the backend will process the co-authors as follows, assume trailer v
 
 - Second we check if email is in format `[email protected]`. If it is we use username part as GitHub username/login and fetch the user from GitHub API. If the user is found, we use that user as co-author.
 
-- Third we lookup for email using GitHub API. If the user is found, we use that user as co-author.
+- Third we lookup for email using GitHub API. If the user is found, we use that user as co-author. If not we lookup for this user in EasyCLA database by email (first by primary/LF email and then by other emails). If found, we use that user as co-author.
 
 - Finally we use the name part for `name <email>`. If the name matches the GitHub username pattern (alphanumeric characters or hyphens, must be 3–39 characters long, cannot start or end with a hyphen, and cannot contain consecutive hyphens), then we lookup using the GitHub API assuming that this name is a GitHub username/login (this is the case for some bots). If the user is found, we use that user as co-author.
 

cla-backend-go/github/github_repository.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	log "github.com/linuxfoundation/easycla/cla-backend-go/logging"
+	"github.com/linuxfoundation/easycla/cla-backend-go/users"
 	"github.com/linuxfoundation/easycla/cla-backend-go/utils"
 	"github.com/sirupsen/logrus"
 
@@ -327,13 +328,15 @@ func GetCoAuthorsFromCommit(
 		commitMessage := commit.GetCommit().GetMessage()
 		// log.WithFields(f).Debugf("commit message: %s", commitMessage)
 
-		re := regexp.MustCompile(`(?i)co-authored-by: (.*) <(.*)>`)
+		re := regexp.MustCompile(`(?i)co-authored-by:\s*(.+?)\s*<([^<>]+)>`)
 		matches := re.FindAllStringSubmatch(commitMessage, -1)
 		for _, match := range matches {
 			name := strings.TrimSpace(match[1])
-			email := strings.TrimSpace(match[2])
-			coAuthors = append(coAuthors, [2]string{name, email})
-			log.WithFields(f).Debugf("found co-author: name: %s, email: %s", name, email)
+			email := strings.ToLower(strings.TrimSpace(match[2]))
+			if name != "" && email != "" {
+				coAuthors = append(coAuthors, [2]string{name, email})
+				log.WithFields(f).Debugf("found co-author: name: %s, email: %s", name, email)
+			}
 		}
 	}
 	return coAuthors
@@ -343,6 +346,7 @@ func GetCoAuthorsFromCommit(
 func ExpandWithCoAuthors(
 	ctx context.Context,
 	client *github.Client,
+	usersService users.Service,
 	commit *github.RepositoryCommit,
 	pr int,
 	installationID int64,
@@ -355,7 +359,7 @@ func ExpandWithCoAuthors(
 	coAuthors := GetCoAuthorsFromCommit(ctx, commit)
 	log.WithFields(f).Debugf("co-authors found: %s", coAuthors)
 	for _, coAuthor := range coAuthors {
-		summary := GetCoAuthorCommits(ctx, client, coAuthor, commit, pr, installationID)
+		summary := GetCoAuthorCommits(ctx, client, usersService, coAuthor, commit, pr, installationID)
 		*commitAuthors = append(*commitAuthors, summary)
 	}
 }
@@ -374,9 +378,11 @@ func IsValidGitHubUsername(username string) bool {
 	return true
 }
 
+//nolint:gocyclo // complexity is acceptable for now
 func GetCoAuthorCommits(
 	ctx context.Context,
 	client *github.Client,
+	usersService users.Service,
 	coAuthor [2]string,
 	commit *github.RepositoryCommit,
 	pr int,
@@ -398,8 +404,10 @@ func GetCoAuthorCommits(
 	)
 	name = strings.TrimSpace(coAuthor[0])
 	email = strings.TrimSpace(coAuthor[1])
+	lName := strings.ToLower(name)
 
-	if cachedUser, ok := GithubUserCache.Get([2]string{name, email}); ok {
+	cacheKey := [2]string{lName, email}
+	if cachedUser, ok := GithubUserCache.Get(cacheKey); ok {
 		log.WithFields(f).Debugf("GitHub user found in cache for name/email: %s/%s: %+v", name, email, cachedUser)
 		var summary *UserCommitSummary
 		if cachedUser != nil {
@@ -454,19 +462,63 @@ func GetCoAuthorCommits(
 		}
 	}
 
-	// 3. Try to find user by email
+	// 3. Try to find user by email via GitHub APIs
 	if user == nil {
 		user, err = SearchGithubUserByEmail(ctx, client, email)
 		if err != nil {
-			log.WithFields(f).Debugf("Co-author GitHub user not found via email %s: %v (error: %v)", email, coAuthor, err)
+			log.WithFields(f).Debugf("Co-author GitHub user not found via github email %s: %v (error: %v)", email, coAuthor, err)
 			user = nil
 		}
 	}
 
+	//	3b. Try to find user by email in our database
+	if user == nil {
+		var githubID string
+		dbUsers, err2 := usersService.GetUsersByLFEmail(email)
+		if err2 == nil {
+			for _, dbUser := range dbUsers {
+				if dbUser.GithubID != "" {
+					githubID = dbUser.GithubID
+					// log.WithFields(f).Debugf("FOUND githubID.1 = %s", githubID)
+					break
+				}
+			}
+		} else {
+			log.WithFields(f).Debugf("Co-author GitHub user not found via lf email %s: %v (error: %v)", email, coAuthor, err2)
+		}
+		if githubID == "" {
+			dbUsers, err2 := usersService.GetUsersByEmail(email)
+			if err2 == nil {
+				for _, dbUser := range dbUsers {
+					if dbUser.GithubID != "" {
+						githubID = dbUser.GithubID
+						// log.WithFields(f).Debugf("FOUND githubID.2 = %s", githubID)
+						break
+					}
+				}
+			} else {
+				log.WithFields(f).Debugf("Co-author GitHub user not found via emails %s: %v (error: %v)", email, coAuthor, err2)
+			}
+		}
+		if githubID != "" {
+			githubIDInt, err2 := strconv.ParseInt(githubID, 10, 64)
+			if err2 != nil {
+				log.WithFields(f).Debugf("Co-author GitHub user not found via lf email %s, wrong GitHub ID: %s: %v (error: %v)", email, githubID, coAuthor, err2)
+			} else {
+				user, err = GetGithubUserByID(ctx, client, githubIDInt)
+				if err != nil {
+					log.WithFields(f).Debugf("Error fetching user by ID %d: %v", githubIDInt, err)
+					user = nil
+				}
+				// log.WithFields(f).Debugf("FOUND user = (%s, %d, %s, %s)", *user.Login, *user.ID, *user.Name, *user.Email)
+			}
+		}
+	}
+
 	// 4. Last resort - try to find by name=login
-	if user == nil && IsValidGitHubUsername(name) {
+	if user == nil && IsValidGitHubUsername(lName) {
 		// Note that Co-authored-by: name <email> is not actually a GitHub login but rather a name - but we are trying hard to find a GitHub profile
-		user, err = GetGithubUserByLogin(ctx, client, name)
+		user, err = GetGithubUserByLogin(ctx, client, lName)
 		if err != nil {
 			log.WithFields(f).Debugf("Co-author GitHub user not found via name=login=%s: %v (error: %v)", name, coAuthor, err)
 			user = nil
@@ -512,11 +564,11 @@ func GetCoAuthorCommits(
 		log.WithFields(f).Debugf("Co-author GitHub user details not found: %v", coAuthor)
 	}
 
-	GithubUserCache.Set([2]string{name, email}, user)
+	GithubUserCache.Set(cacheKey, user)
 	return summary
 }
 
-func GetPullRequestCommitAuthors(ctx context.Context, installationID int64, pullRequestID int, owner, repo string, withCoAuthors bool) ([]*UserCommitSummary, *string, error) {
+func GetPullRequestCommitAuthors(ctx context.Context, usersService users.Service, installationID int64, pullRequestID int, owner, repo string, withCoAuthors bool) ([]*UserCommitSummary, *string, error) {
 	f := logrus.Fields{
 		"functionName":  "github.github_repository.GetPullRequestCommitAuthors",
 		"pullRequestID": pullRequestID,
@@ -571,7 +623,7 @@ func GetPullRequestCommitAuthors(ctx context.Context, installationID int64, pull
 			Authorized:   false,
 		})
 		if withCoAuthors {
-			ExpandWithCoAuthors(ctx, client, commit, pullRequestID, installationID, &userCommitSummary)
+			ExpandWithCoAuthors(ctx, client, usersService, commit, pullRequestID, installationID, &userCommitSummary)
 		}
 	}
 

cla-backend-go/signatures/service.go

@@ -1023,7 +1023,7 @@ func (s service) updateChangeRequest(ctx context.Context, ghOrg *models.GithubOr
 	// Fetch committers
 	withCoAuthors := github.IsCoAuthorsEnabledForRepo(ghOrg.EnableCoAuthors, gitHubRepoName)
 	log.WithFields(f).Debugf("fetching commit authors for PR: %d using repository owner: %s, repo: %s", pullRequestID, gitHubOrgName, gitHubRepoName)
-	authors, latestSHA, authorsErr := github.GetPullRequestCommitAuthors(ctx, ghOrg.OrganizationInstallationID, int(pullRequestID), gitHubOrgName, gitHubRepoName, withCoAuthors)
+	authors, latestSHA, authorsErr := github.GetPullRequestCommitAuthors(ctx, s.usersService, ghOrg.OrganizationInstallationID, int(pullRequestID), gitHubOrgName, gitHubRepoName, withCoAuthors)
 	if authorsErr != nil {
 		log.WithFields(f).WithError(authorsErr).Warnf("unable to get commit authors for %s/%s for PR: %d", gitHubOrgName, gitHubRepoName, pullRequestID)
 		return authorsErr

cla-backend-go/users/mocks/mock_repo.go

@@ -42,6 +42,7 @@ type UserRepository interface {
 	GetUserByExternalID(userExternalID string) (*models.User, error)
 	GetUserByUserName(userName string, fullMatch bool) (*models.User, error)
 	GetUserByEmail(userEmail string) (*models.User, error)
+	GetUsersByLFEmail(userEmail string) ([]*models.User, error)
 	GetUserByGitHubID(gitHubID string) (*models.User, error)
 	GetUserByGitHubUsername(gitHubUsername string) (*models.User, error)
 	GetUserByGitlabID(gitlabID int) (*models.User, error)
@@ -125,7 +126,7 @@ func (repo repository) CreateUser(user *models.User) (*models.User, error) {
 
 	if user.LfEmail != "" {
 		attributes["lf_email"] = &dynamodb.AttributeValue{
-			S: aws.String(user.LfEmail.String()),
+			S: aws.String(strings.ToLower(user.LfEmail.String())),
 		}
 	}
 
@@ -340,6 +341,7 @@ func (repo repository) Save(user *models.UserUpdate) (*models.User, error) {
 		"tableName":        repo.tableName,
 	}
 
+	user.LfEmail = strings.ToLower(user.LfEmail)
 	var oldUserModel *models.User
 	var err error
 	oldUserModel, err = repo.getUserByUpdateModel(user)
@@ -881,12 +883,76 @@ func (repo repository) GetUsersByEmail(userEmail string) ([]*models.User, error)
 	return users, nil
 }
 
+// GetUsersByLFEmail fetches the user record by email
+func (repo repository) GetUsersByLFEmail(userEmail string) ([]*models.User, error) {
+	f := logrus.Fields{
+		"functionName": "users.repository.GetUsersByLFEmail",
+		"userEmail":    userEmail,
+	}
+	userEmail = strings.ToLower(userEmail)
+	// This is the key we want to match
+	condition := expression.Key("lf_email").Equal(expression.Value(userEmail))
+
+	// These are the columns we want returned
+	projection := buildUserProjection()
+
+	// Use the nice builder to create the expression
+	expr, err := expression.NewBuilder().WithKeyCondition(condition).WithProjection(projection).Build()
+	if err != nil {
+		log.WithFields(f).WithError(err).Warnf("error building expression for lf_email : %s, error: %v", userEmail, err)
+		return []*models.User{}, err
+	}
+
+	// Assemble the query input parameters
+	queryInput := &dynamodb.QueryInput{
+		ExpressionAttributeNames:  expr.Names(),
+		ExpressionAttributeValues: expr.Values(),
+		KeyConditionExpression:    expr.KeyCondition(),
+		ProjectionExpression:      expr.Projection(),
+		TableName:                 aws.String(repo.tableName),
+		IndexName:                 aws.String("lf-email-index"),
+	}
+
+	// Make the DynamoDB Query API call
+	result, err := repo.dynamoDBClient.Query(queryInput)
+	if err != nil {
+		log.WithFields(f).WithError(err).Warnf("error retrieving user by lf_email: %s, error: %+v", userEmail, err)
+		return []*models.User{}, err
+	}
+
+	// The user model
+	var dbUserModels []DBUser
+
+	err = dynamodbattribute.UnmarshalListOfMaps(result.Items, &dbUserModels)
+	if err != nil {
+		log.WithFields(f).WithError(err).Warnf("error unmarshalling user record from database for lf_email: %s, error: %+v", userEmail, err)
+		return []*models.User{}, err
+	}
+
+	if len(dbUserModels) == 0 {
+		return []*models.User{}, &utils.UserNotFound{
+			Message:   fmt.Sprintf("user not found when searching by lf email: %s", userEmail),
+			UserLFID:  "",
+			UserName:  "",
+			UserEmail: userEmail,
+			Err:       nil,
+		}
+	}
+
+	usrs := make([]*models.User, 0, len(dbUserModels))
+	for _, dbUser := range dbUserModels {
+		usrs = append(usrs, convertDBUserModel(dbUser))
+	}
+	return usrs, nil
+}
+
 // GetUserByEmail fetches the user record by email
 func (repo repository) GetUserByEmail(userEmail string) (*models.User, error) {
 	f := logrus.Fields{
 		"functionName": "users.repository.GetUserByEmail",
 		"userEmail":    userEmail,
 	}
+	userEmail = strings.ToLower(userEmail)
 	// This is the key we want to match
 	condition := expression.Key("lf_email").Equal(expression.Value(userEmail))
 
@@ -1325,7 +1391,7 @@ func convertDBUserModel(user DBUser) *models.User {
 		UserID:         user.UserID,
 		UserExternalID: user.UserExternalID,
 		Admin:          user.Admin,
-		LfEmail:        strfmt.Email(user.LFEmail),
+		LfEmail:        strfmt.Email(strings.ToLower(user.LFEmail)),
 		LfSub:          user.LFSub,
 		LfUsername:     user.LFUsername,
 		DateCreated:    user.DateCreated,

cla-backend-go/users/repository.go

@@ -22,6 +22,8 @@ type Service interface {
 	GetUserByLFUserName(lfUserName string) (*models.User, error)
 	GetUserByUserName(userName string, fullMatch bool) (*models.User, error)
 	GetUserByEmail(userEmail string) (*models.User, error)
+	GetUsersByEmail(userEmail string) ([]*models.User, error)
+	GetUsersByLFEmail(userEmail string) ([]*models.User, error)
 	GetUserByGitHubID(gitHubID string) (*models.User, error)
 	GetUserByGitHubUsername(gitlabUsername string) (*models.User, error)
 	GetUserByGitlabID(gitHubID int) (*models.User, error)
@@ -156,6 +158,22 @@ func (s service) GetUserByEmail(userEmail string) (*models.User, error) {
 	return s.repo.GetUserByEmail(userEmail)
 }
 
+// GetUsersByEmail fetches the users by email
+func (s service) GetUsersByEmail(userEmail string) ([]*models.User, error) {
+	if userEmail == "" {
+		return nil, errors.New("userEmail is empty")
+	}
+	return s.repo.GetUsersByEmail(userEmail)
+}
+
+// GetUsersByLFEmail fetches the users by email
+func (s service) GetUsersByLFEmail(userEmail string) ([]*models.User, error) {
+	if userEmail == "" {
+		return nil, errors.New("userEmail is empty")
+	}
+	return s.repo.GetUsersByLFEmail(userEmail)
+}
+
 // GetUserByGitHubID fetches the user by GitHub ID
 func (s service) GetUserByGitHubID(gitHubID string) (*models.User, error) {
 	if gitHubID == "" {

cla-backend-go/users/service.go

@@ -61,7 +61,7 @@ func (s service) updateChangeRequest(ctx context.Context, installationID, reposi
 		withCoAuthors = github.IsCoAuthorsEnabledForRepo(ghOrg.EnableCoAuthors, gitHubRepoName)
 	}
 	log.WithFields(f).Debugf("fetching commit authors for PR: %d using repository owner: %s, repo: %s", pullRequestID, gitHubOrgName, gitHubRepoName)
-	authors, latestSHA, authorsErr := github.GetPullRequestCommitAuthors(ctx, installationID, int(pullRequestID), gitHubOrgName, gitHubRepoName, withCoAuthors)
+	authors, latestSHA, authorsErr := github.GetPullRequestCommitAuthors(ctx, s.userService, installationID, int(pullRequestID), gitHubOrgName, gitHubRepoName, withCoAuthors)
 	if authorsErr != nil {
 		log.WithFields(f).WithError(authorsErr).Warnf("unable to get commit authors for %s/%s for PR: %d", gitHubOrgName, gitHubRepoName, pullRequestID)
 		return authorsErr