diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 07d39be555..bf40dda3c4 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -236,6 +236,7 @@ public abstract class AbstractDaemonTest { protected TestAccount user; protected TestRepository testRepo; protected String resourcePrefix; + protected Description description; @Inject private ChangeIndexCollection changeIndexes; @Inject private EventRecorder.Factory eventRecorderFactory; @@ -308,6 +309,7 @@ public abstract class AbstractDaemonTest { } protected void beforeTest(Description description) throws Exception { + this.description = description; GerritServer.Description classDesc = GerritServer.Description.forTestClass(description, configName); GerritServer.Description methodDesc = diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractNotificationTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractNotificationTest.java index 416d859fa5..a962c70c85 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractNotificationTest.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractNotificationTest.java @@ -24,9 +24,9 @@ import com.google.common.truth.Subject; import com.google.common.truth.SubjectFactory; import com.google.common.truth.Truth; import com.google.gerrit.common.Nullable; -import com.google.gerrit.extensions.api.changes.AddReviewerInput; -import com.google.gerrit.extensions.api.changes.AddReviewerResult; import com.google.gerrit.extensions.api.changes.RecipientType; +import com.google.gerrit.extensions.api.changes.ReviewInput; +import com.google.gerrit.extensions.api.changes.ReviewResult; import com.google.gerrit.extensions.api.projects.ConfigInput; import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy; @@ -38,11 +38,15 @@ import com.google.gerrit.server.mail.send.EmailHeader; import com.google.gerrit.server.mail.send.EmailHeader.AddressList; import com.google.gerrit.testutil.FakeEmailSender; import com.google.gerrit.testutil.FakeEmailSender.Message; +import java.io.IOException; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import org.eclipse.jgit.junit.TestRepository; +import org.junit.After; import org.junit.Before; public abstract class AbstractNotificationTest extends AbstractDaemonTest { @@ -69,6 +73,14 @@ public abstract class AbstractNotificationTest extends AbstractDaemonTest { } protected void setEmailStrategy(TestAccount account, EmailStrategy strategy) throws Exception { + setEmailStrategy(account, strategy, true); + } + + protected void setEmailStrategy(TestAccount account, EmailStrategy strategy, boolean record) + throws Exception { + if (record) { + accountsModifyingEmailStrategy.add(account); + } setApiUser(account); GeneralPreferencesInfo prefs = gApi.accounts().self().getPreferences(); prefs.emailStrategy = strategy; @@ -241,6 +253,20 @@ public abstract class AbstractNotificationTest extends AbstractDaemonTest { } } + private static final Map stagedUsers = new HashMap<>(); + + // TestAccount doesn't implement hashCode/equals, so this set is according + // to object identity. That's fine for our purposes. + private Set accountsModifyingEmailStrategy = new HashSet<>(); + + @After + public void resetEmailStrategies() throws Exception { + for (TestAccount account : accountsModifyingEmailStrategy) { + setEmailStrategy(account, EmailStrategy.ENABLED, false); + } + accountsModifyingEmailStrategy.clear(); + } + protected class StagedUsers { public final TestAccount owner; public final TestAccount author; @@ -248,6 +274,7 @@ public abstract class AbstractNotificationTest extends AbstractDaemonTest { public final TestAccount reviewer; public final TestAccount ccer; public final TestAccount starrer; + public final TestAccount assignee; public final TestAccount watchingProjectOwner; public final String reviewerByEmail = "reviewerByEmail@example.com"; public final String ccerByEmail = "ccByEmail@example.com"; @@ -255,31 +282,59 @@ public abstract class AbstractNotificationTest extends AbstractDaemonTest { private final Map accountsByEmail = new HashMap<>(); boolean supportReviewersByEmail; - StagedUsers(List watches) throws Exception { - owner = testAccount("owner"); - reviewer = testAccount("reviewer"); - author = testAccount("author"); - uploader = testAccount("uploader"); - ccer = testAccount("ccer"); - starrer = testAccount("starrer"); + private String usersCacheKey() { + return description.getClassName(); + } - watchingProjectOwner = testAccount("watchingProjectOwner", "Administrators"); - setApiUser(watchingProjectOwner); - watch(project.get(), pwi -> pwi.notifyNewChanges = true); + private TestAccount evictAndCopy(TestAccount account) throws IOException { + accountCache.evict(account.id); + return account; + } - for (NotifyType watch : watches) { - TestAccount watcher = testAccount(watch.toString()); - setApiUser(watcher); - watch( - project.get(), - pwi -> { - pwi.notifyAllComments = watch.equals(NotifyType.ALL_COMMENTS); - pwi.notifyAbandonedChanges = watch.equals(NotifyType.ABANDONED_CHANGES); - pwi.notifyNewChanges = watch.equals(NotifyType.NEW_CHANGES); - pwi.notifyNewPatchSets = watch.equals(NotifyType.NEW_PATCHSETS); - pwi.notifySubmittedChanges = watch.equals(NotifyType.SUBMITTED_CHANGES); - }); - watchers.put(watch, watcher); + public StagedUsers(List watches) throws Exception { + synchronized (stagedUsers) { + if (stagedUsers.containsKey(usersCacheKey())) { + StagedUsers existing = stagedUsers.get(usersCacheKey()); + owner = evictAndCopy(existing.owner); + author = evictAndCopy(existing.author); + uploader = evictAndCopy(existing.uploader); + reviewer = evictAndCopy(existing.reviewer); + ccer = evictAndCopy(existing.ccer); + starrer = evictAndCopy(existing.starrer); + assignee = evictAndCopy(existing.assignee); + watchingProjectOwner = evictAndCopy(existing.watchingProjectOwner); + watchers.putAll(existing.watchers); + return; + } + + owner = testAccount("owner"); + reviewer = testAccount("reviewer"); + author = testAccount("author"); + uploader = testAccount("uploader"); + ccer = testAccount("ccer"); + starrer = testAccount("starrer"); + assignee = testAccount("assignee"); + + watchingProjectOwner = testAccount("watchingProjectOwner", "Administrators"); + setApiUser(watchingProjectOwner); + watch(allProjects.get(), pwi -> pwi.notifyNewChanges = true); + + for (NotifyType watch : watches) { + TestAccount watcher = testAccount(watch.toString()); + setApiUser(watcher); + watch( + allProjects.get(), + pwi -> { + pwi.notifyAllComments = watch.equals(NotifyType.ALL_COMMENTS); + pwi.notifyAbandonedChanges = watch.equals(NotifyType.ABANDONED_CHANGES); + pwi.notifyNewChanges = watch.equals(NotifyType.NEW_CHANGES); + pwi.notifyNewPatchSets = watch.equals(NotifyType.NEW_PATCHSETS); + pwi.notifySubmittedChanges = watch.equals(NotifyType.SUBMITTED_CHANGES); + }); + watchers.put(watch, watcher); + } + + stagedUsers.put(usersCacheKey(), this); } } @@ -316,25 +371,23 @@ public abstract class AbstractNotificationTest extends AbstractDaemonTest { } protected void addReviewers(PushOneCommit.Result r) throws Exception { - AddReviewerInput in = new AddReviewerInput(); - in.reviewer = reviewer.email; - gApi.changes().id(r.getChangeId()).addReviewer(in); - - in.reviewer = ccer.email; - in.state = ReviewerState.CC; - gApi.changes().id(r.getChangeId()).addReviewer(in); - - in.reviewer = reviewerByEmail; - in.state = ReviewerState.REVIEWER; - AddReviewerResult result = gApi.changes().id(r.getChangeId()).addReviewer(in); - if (result.reviewers == null || result.reviewers.isEmpty()) { + ReviewInput in = + ReviewInput.noScore() + .reviewer(reviewer.email) + .reviewer(reviewerByEmail) + .reviewer(ccer.email, ReviewerState.CC, false) + .reviewer(ccerByEmail, ReviewerState.CC, false); + ReviewResult result = gApi.changes().id(r.getChangeId()).revision("current").review(in); + supportReviewersByEmail = true; + if (result.reviewers.values().stream().anyMatch(v -> v.error != null)) { supportReviewersByEmail = false; - } else { - supportReviewersByEmail = true; - in.reviewer = ccerByEmail; - in.state = ReviewerState.CC; - gApi.changes().id(r.getChangeId()).addReviewer(in); + in = + ReviewInput.noScore() + .reviewer(reviewer.email) + .reviewer(ccer.email, ReviewerState.CC, false); + result = gApi.changes().id(r.getChangeId()).revision("current").review(in); } + Truth.assertThat(result.reviewers.values().stream().allMatch(v -> v.error == null)).isTrue(); } } @@ -362,6 +415,7 @@ public abstract class AbstractNotificationTest extends AbstractDaemonTest { if (pushOptions != null) { ref = ref + '%' + Joiner.on(',').join(pushOptions); } + setApiUser(owner); repo = cloneProject(project, owner); PushOneCommit push = pushFactory.create(db, owner.getIdent(), repo); result = push.to(ref); @@ -381,7 +435,6 @@ public abstract class AbstractNotificationTest extends AbstractDaemonTest { } protected class StagedChange extends StagedPreChange { - StagedChange(String ref, List watches) throws Exception { super(ref, watches); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/CommentSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/CommentSenderIT.java index e78798f233..3123440d73 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/CommentSenderIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/CommentSenderIT.java @@ -27,7 +27,6 @@ import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.ReviewInput; -import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy; import org.junit.Test; @@ -35,8 +34,7 @@ public class CommentSenderIT extends AbstractNotificationTest { @Test public void commentOnReviewableChangeByOwner() throws Exception { StagedChange sc = stageReviewableChange(ALL_COMMENTS); - setApiUser(sc.owner); - review(sc.changeId, ENABLED); + review(sc.owner, sc.changeId, ENABLED); assertThat(sender) .sent("comment", sc) .notTo(sc.owner) @@ -50,8 +48,7 @@ public class CommentSenderIT extends AbstractNotificationTest { @Test public void commentOnReviewableChangeByReviewer() throws Exception { StagedChange sc = stageReviewableChange(ALL_COMMENTS); - setApiUser(sc.reviewer); - review(sc.changeId, ENABLED); + review(sc.reviewer, sc.changeId, ENABLED); assertThat(sender) .sent("comment", sc) .notTo(sc.reviewer) @@ -66,8 +63,7 @@ public class CommentSenderIT extends AbstractNotificationTest { @Test public void commentOnReviewableChangeByOwnerCcingSelf() throws Exception { StagedChange sc = stageReviewableChange(ALL_COMMENTS); - setApiUser(sc.owner); - review(sc.changeId, CC_ON_OWN_COMMENTS); + review(sc.owner, sc.changeId, CC_ON_OWN_COMMENTS); assertThat(sender) .sent("comment", sc) .to(sc.owner) @@ -81,8 +77,7 @@ public class CommentSenderIT extends AbstractNotificationTest { @Test public void commentOnReviewableChangeByReviewerCcingSelf() throws Exception { StagedChange sc = stageReviewableChange(ALL_COMMENTS); - setApiUser(sc.reviewer); - review(sc.changeId, CC_ON_OWN_COMMENTS); + review(sc.reviewer, sc.changeId, CC_ON_OWN_COMMENTS); assertThat(sender) .sent("comment", sc) .to(sc.owner) @@ -97,8 +92,7 @@ public class CommentSenderIT extends AbstractNotificationTest { public void commentOnReviewableChangeByOther() throws Exception { TestAccount other = accountCreator.create("other", "other@example.com", "other"); StagedChange sc = stageReviewableChange(ALL_COMMENTS); - setApiUser(other); - review(sc.changeId, ENABLED); + review(other, sc.changeId, ENABLED); assertThat(sender) .sent("comment", sc) .notTo(other) @@ -114,8 +108,7 @@ public class CommentSenderIT extends AbstractNotificationTest { public void commentOnReviewableChangeByOtherCcingSelf() throws Exception { TestAccount other = accountCreator.create("other", "other@example.com", "other"); StagedChange sc = stageReviewableChange(ALL_COMMENTS); - setApiUser(other); - review(sc.changeId, CC_ON_OWN_COMMENTS); + review(other, sc.changeId, CC_ON_OWN_COMMENTS); assertThat(sender) .sent("comment", sc) .to(sc.owner) @@ -129,7 +122,7 @@ public class CommentSenderIT extends AbstractNotificationTest { @Test public void commentOnReviewableChangeByOwnerNotifyOwnerReviewers() throws Exception { StagedChange sc = stageReviewableChange(ALL_COMMENTS); - review(sc.changeId, ENABLED, OWNER_REVIEWERS); + review(sc.owner, sc.changeId, ENABLED, OWNER_REVIEWERS); assertThat(sender) .sent("comment", sc) .to(sc.reviewerByEmail) // TODO(logan): This is unintentionally TO, should be CC. @@ -142,7 +135,7 @@ public class CommentSenderIT extends AbstractNotificationTest { @Test public void commentOnReviewableChangeByOwnerNotifyOwner() throws Exception { StagedChange sc = stageReviewableChange(ALL_COMMENTS); - review(sc.changeId, ENABLED, OWNER); + review(sc.owner, sc.changeId, ENABLED, OWNER); assertThat(sender).notSent(); } @@ -150,14 +143,14 @@ public class CommentSenderIT extends AbstractNotificationTest { public void commentOnReviewableChangeByOwnerCcingSelfNotifyOwner() throws Exception { StagedChange sc = stageReviewableChange(ALL_COMMENTS); setEmailStrategy(sc.owner, CC_ON_OWN_COMMENTS); - review(sc.changeId, ENABLED, OWNER); + review(sc.owner, sc.changeId, ENABLED, OWNER); assertThat(sender).notSent(); // TODO(logan): Why not send to owner? } @Test public void commentOnReviewableChangeByOwnerNotifyNone() throws Exception { StagedChange sc = stageReviewableChange(ALL_COMMENTS); - review(sc.changeId, ENABLED, NONE); + review(sc.owner, sc.changeId, ENABLED, NONE); assertThat(sender).notSent(); } @@ -165,28 +158,28 @@ public class CommentSenderIT extends AbstractNotificationTest { public void commentOnReviewableChangeByOwnerCcingSelfNotifyNone() throws Exception { StagedChange sc = stageReviewableChange(ALL_COMMENTS); setEmailStrategy(sc.owner, CC_ON_OWN_COMMENTS); - review(sc.changeId, ENABLED, NONE); + review(sc.owner, sc.changeId, ENABLED, NONE); assertThat(sender).notSent(); // TODO(logan): Why not send to owner? } @Test public void commentOnWipChangeByOwner() throws Exception { StagedChange sc = stageWipChange(ALL_COMMENTS); - review(sc.changeId, ENABLED); + review(sc.owner, sc.changeId, ENABLED); assertThat(sender).notSent(); } @Test public void commentOnWipChangeByOwnerCcingSelf() throws Exception { StagedChange sc = stageWipChange(ALL_COMMENTS); - review(sc.changeId, CC_ON_OWN_COMMENTS); + review(sc.owner, sc.changeId, CC_ON_OWN_COMMENTS); assertThat(sender).notSent(); } @Test public void commentOnWipChangeByOwnerNotifyAll() throws Exception { StagedChange sc = stageWipChange(ALL_COMMENTS); - review(sc.changeId, ENABLED, ALL); + review(sc.owner, sc.changeId, ENABLED, ALL); assertThat(sender) .sent("comment", sc) .notTo(sc.owner) @@ -200,19 +193,19 @@ public class CommentSenderIT extends AbstractNotificationTest { @Test public void commentOnReviewableWipChangeByOwner() throws Exception { StagedChange sc = stageReviewableWipChange(ALL_COMMENTS); - review(sc.changeId, ENABLED); + review(sc.owner, sc.changeId, ENABLED); assertThat(sender).notSent(); } - private void review(String changeId, EmailStrategy strategy) throws Exception { - review(changeId, strategy, null); + private void review(TestAccount account, String changeId, EmailStrategy strategy) + throws Exception { + review(account, changeId, strategy, null); } - private void review(String changeId, EmailStrategy strategy, @Nullable NotifyHandling notify) + private void review( + TestAccount account, String changeId, EmailStrategy strategy, @Nullable NotifyHandling notify) throws Exception { - GeneralPreferencesInfo prefs = gApi.accounts().self().getPreferences(); - prefs.emailStrategy = strategy; - gApi.accounts().self().setPreferences(prefs); + setEmailStrategy(account, strategy); ReviewInput in = ReviewInput.recommend(); in.notify = notify; gApi.changes().id(changeId).revision("current").review(in); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/SetAssigneeSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/SetAssigneeSenderIT.java index 2d3d272059..37c9d8b1f7 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/SetAssigneeSenderIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/SetAssigneeSenderIT.java @@ -35,51 +35,51 @@ public class SetAssigneeSenderIT extends AbstractNotificationTest { @Test public void setAssigneeOnReviewableChange() throws Exception { StagedChange sc = stageReviewableChange(); - assign(sc, sc.owner, assignee); + assign(sc, sc.owner, sc.assignee); assertThat(sender) .sent("setassignee", sc) .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer) .to(sc.reviewerByEmail) // TODO(logan): This is probably not intended! .cc(sc.ccerByEmail) // TODO(logan): This is probably not intended! - .to(assignee); + .to(sc.assignee); } @Test public void setAssigneeOnReviewableChangeByOwnerCcingSelf() throws Exception { StagedChange sc = stageReviewableChange(); - assign(sc, sc.owner, assignee, CC_ON_OWN_COMMENTS); + assign(sc, sc.owner, sc.assignee, CC_ON_OWN_COMMENTS); assertThat(sender) .sent("setassignee", sc) .notTo(sc.reviewer, sc.ccer, sc.starrer) .cc(sc.owner) .to(sc.reviewerByEmail) // TODO(logan): This is probably not intended! .cc(sc.ccerByEmail) // TODO(logan): This is probably not intended! - .to(assignee); + .to(sc.assignee); } @Test public void setAssigneeOnReviewableChangeByAdmin() throws Exception { StagedChange sc = stageReviewableChange(); - assign(sc, admin, assignee); + assign(sc, admin, sc.assignee); assertThat(sender) .sent("setassignee", sc) .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer, admin) .to(sc.reviewerByEmail) // TODO(logan): This is probably not intended! .cc(sc.ccerByEmail) // TODO(logan): This is probably not intended! - .to(assignee); + .to(sc.assignee); } @Test public void setAssigneeOnReviewableChangeByAdminCcingSelf() throws Exception { StagedChange sc = stageReviewableChange(); - assign(sc, admin, assignee, CC_ON_OWN_COMMENTS); + assign(sc, admin, sc.assignee, CC_ON_OWN_COMMENTS); assertThat(sender) .sent("setassignee", sc) .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer) .cc(admin) .to(sc.reviewerByEmail) // TODO(logan): This is probably not intended! .cc(sc.ccerByEmail) // TODO(logan): This is probably not intended! - .to(assignee); + .to(sc.assignee); } @Test @@ -89,7 +89,7 @@ public class SetAssigneeSenderIT extends AbstractNotificationTest { assign(sc, sc.owner, sc.owner); assertThat(sender) .sent("setassignee", sc) - .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer, assignee) + .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer, sc.assignee) .to(sc.reviewerByEmail) // TODO(logan): This is probably not intended! .cc(sc.ccerByEmail); // TODO(logan): This is probably not intended! } @@ -108,25 +108,25 @@ public class SetAssigneeSenderIT extends AbstractNotificationTest { TestAccount other = accountCreator.create("other", "other@example.com", "other"); assign(sc, sc.owner, other); sender.clear(); - assign(sc, sc.owner, assignee); + assign(sc, sc.owner, sc.assignee); assertThat(sender) .sent("setassignee", sc) .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer, other) .to(sc.reviewerByEmail) // TODO(logan): This is probably not intended! .cc(sc.ccerByEmail) // TODO(logan): This is probably not intended! - .to(assignee); + .to(sc.assignee); } @Test public void changeAssigneeToSelfOnReviewableChangeInNoteDb() throws Exception { assume().that(notesMigration.readChanges()).isTrue(); StagedChange sc = stageReviewableChange(); - assign(sc, sc.owner, assignee); + assign(sc, sc.owner, sc.assignee); sender.clear(); assign(sc, sc.owner, sc.owner); assertThat(sender) .sent("setassignee", sc) - .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer, assignee) + .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer, sc.assignee) .to(sc.reviewerByEmail) // TODO(logan): This is probably not intended! .cc(sc.ccerByEmail); // TODO(logan): This is probably not intended! } @@ -135,7 +135,7 @@ public class SetAssigneeSenderIT extends AbstractNotificationTest { public void changeAssigneeToSelfOnReviewableChangeInReviewDb() throws Exception { assume().that(notesMigration.readChanges()).isFalse(); StagedChange sc = stageReviewableChange(); - assign(sc, sc.owner, assignee); + assign(sc, sc.owner, sc.assignee); sender.clear(); assign(sc, sc.owner, sc.owner); assertThat(sender).notSent(); @@ -144,25 +144,25 @@ public class SetAssigneeSenderIT extends AbstractNotificationTest { @Test public void setAssigneeOnReviewableWipChange() throws Exception { StagedChange sc = stageReviewableWipChange(); - assign(sc, sc.owner, assignee); + assign(sc, sc.owner, sc.assignee); assertThat(sender) .sent("setassignee", sc) .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer) .to(sc.reviewerByEmail) // TODO(logan): This is probably not intended! .cc(sc.ccerByEmail) // TODO(logan): This is probably not intended! - .to(assignee); + .to(sc.assignee); } @Test public void setAssigneeOnWipChange() throws Exception { StagedChange sc = stageWipChange(); - assign(sc, sc.owner, assignee); + assign(sc, sc.owner, sc.assignee); assertThat(sender) .sent("setassignee", sc) .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer) .to(sc.reviewerByEmail) // TODO(logan): This is probably not intended! .cc(sc.ccerByEmail) // TODO(logan): This is probably not intended! - .to(assignee); + .to(sc.assignee); } private void assign(StagedChange sc, TestAccount by, TestAccount to) throws Exception { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 11330cb294..585c8e2775 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -60,6 +60,7 @@ import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.api.changes.SubmitInput; import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.DynamicMap.Entry; import com.google.gerrit.extensions.registration.DynamicSet; @@ -1358,8 +1359,12 @@ public class ReceiveCommits { this.draft = cmd.getRefName().startsWith(MagicBranch.NEW_DRAFT_CHANGE); this.labelTypes = labelTypes; this.notesMigration = notesMigration; + GeneralPreferencesInfo prefs = user.getAccount().getGeneralPreferencesInfo(); this.defaultPublishComments = - firstNonNull(user.getAccount().getGeneralPreferencesInfo().publishCommentsOnPush, false); + prefs != null + ? firstNonNull( + user.getAccount().getGeneralPreferencesInfo().publishCommentsOnPush, false) + : false; } MailRecipients getMailRecipients() {