opendev/gerrit
Code Issues Proposed changes

ReceiveCommits: Update caller fullName after everything else

Browse Source 
We have a useful feature of populating the fullName field in the Account
for a caller based on a corresponding identity in the commits that they
push. However, this was awkwardly wedged into the middle of the loop
where commits are validated, hurting readability. It is also a side
effect that is not otherwise taken into account when determining whether
the operation succeeds, so a push that apparently fails might actually
have this observable side effect.

Change the loop to just keep track of the full name it discovers, and
perform this side effect after all other work.

Change-Id: I0ae4b5628d483ba2b842173b9dc8e10804edeb98
This commit is contained in:
Dave Borowitz
2017-08-31 09:36:05 -04:00
parent b1fad26e6f
commit 1ea05bdc99
1 changed files with 33 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
View File

@@ -362,6 +362,7 @@ class ReceiveCommits {
// Other settings populated during processing.
private MagicBranchInput magicBranch;
private boolean newChangeForAllNotInTarget;
private String setFullNameTo;
// Handles for outputting back over the wire to the end user.
private Task newProgress;
@@ -587,6 +588,9 @@ class ReceiveCommits {
}
}
// Update account info with details discovered during commit walking.
updateAccountInfo();
closeProgress.end();
commandProgress.end();
progress.end();
@@ -2694,7 +2698,7 @@ class ReceiveCommits {
return;
}
boolean defaultName = Strings.isNullOrEmpty(user.getAccount().getFullName());
boolean missingFullName = Strings.isNullOrEmpty(user.getAccount().getFullName());
RevWalk walk = rp.getRevWalk();
walk.reset();
walk.sort(RevSort.NONE);
@@ -2715,27 +2719,10 @@ class ReceiveCommits {
break;
}
if (defaultName && user.hasEmailAddress(c.getCommitterIdent().getEmailAddress())) {
try {
String committerName = c.getCommitterIdent().getName();
Account account =
accountsUpdate
.create()
.update(
user.getAccountId(),
a -> {
if (Strings.isNullOrEmpty(a.getFullName())) {
a.setFullName(committerName);
}
});
if (account != null && Strings.isNullOrEmpty(account.getFullName())) {
user.getAccount().setFullName(account.getFullName());
}
} catch (IOException | ConfigInvalidException e) {
logWarn("Cannot default full_name", e);
} finally {
defaultName = false;
}
if (missingFullName && user.hasEmailAddress(c.getCommitterIdent().getEmailAddress())) {
logDebug("Will update full name of caller");
setFullNameTo = c.getCommitterIdent().getName();
missingFullName = false;
}
}
logDebug("Validated {} new commits", i);
@@ -2880,6 +2867,30 @@ class ReceiveCommits {
}
}
private void updateAccountInfo() {
if (setFullNameTo == null) {
return;
}
logDebug("Updating full name of caller");
try {
Account account =
accountsUpdate
.create()
.update(
user.getAccountId(),
a -> {
if (Strings.isNullOrEmpty(a.getFullName())) {
a.setFullName(setFullNameTo);
}
});
if (account != null) {
user.getAccount().setFullName(account.getFullName());
}
} catch (IOException | ConfigInvalidException e) {
logWarn("Failed to update full name of caller", e);
}
}
private Map<Change.Key, ChangeNotes> openChangesByBranch(Branch.NameKey branch)
throws OrmException {
Map<Change.Key, ChangeNotes> r = new HashMap<>();