opendev/gerrit
Code Issues Proposed changes

Use AccountCache#maybeGet instead of AccountCache#get when firing events

Browse Source 
AccountCache#get returns an empty AccountState to represent a missing
account. The empty AccountState has neither a full name, a preferred
email nor a user name. This means in case of a missing account we
created an empty AccountAttribute. An empty AccountAttribute is no
better than no AccountAttribute. Actually it seems that it was the
intention to use no AccountAttribute in case of a missing account since
null is used if account state is null. Only by use of AccountCache#get
account state was never null.

Change-Id: Ie16c1a9c7521db19de7bf79ef4a9493e1c3c48b2
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-01-26 15:53:19 +01:00
parent 4b813deb92
commit a09712aab6
2 changed files with 7 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java
View File

@@ -556,7 +556,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
args.changeMerged.fire(
updatedChange,
mergedPatchSet,
args.accountCache.get(submitter.getAccountId()),
args.accountCache.maybeGet(submitter.getAccountId()).orElse(null),
args.mergeTip.getCurrentTip().name(),
ctx.getWhen());
}

7
java/com/google/gerrit/server/restapi/change/PostReviewersOp.java
View File

@@ -23,6 +23,7 @@ import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Streams;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
@@ -204,7 +205,11 @@ public class PostReviewersOp implements BatchUpdateOp {
accountsToNotify);
if (!addedReviewers.isEmpty()) {
List<AccountState> reviewers =
addedReviewers.stream().map(r -> accountCache.get(r.getAccountId())).collect(toList());
addedReviewers
.stream()
.map(r -> accountCache.maybeGet(r.getAccountId()))
.flatMap(Streams::stream)
.collect(toList());
reviewerAdded.fire(rsrc.getChange(), patchSet, reviewers, ctx.getAccount(), ctx.getWhen());
}
}