Merge changes Iad263b16,If74c346e,I5682bf9b
* changes:
CommentsUtil: Don't use the diff cache to look up the parent commit
Cover some more behaviors of ported comments with tests
Ignore comments on non-existing patchsets when porting
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 6b89d67..09741aa 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -182,20 +182,6 @@
+
By default 1.
-[[asyncPostUpdate]]
-=== Section asyncPostUpdate
-
-[[asyncPostUpdate.threadPoolSize]]asyncPostUpdate.threadPoolSize::
-+
-Maximum size of thread pool in which async post updates are sent out.
-+
-When set to 0, a direct executor is used.
-+
-When unset, use link:#sendemail[sendemail.threadPoolSize] threadPoolSize. When both
-are unset, use the default.
-+
-By default, 1.
-
[[auth]]
=== Section auth
@@ -884,8 +870,6 @@
The check if they are is cheap and always happens on the thread that
inquires for a cached value.
+
-When set to 0, a direct executor is used.
-+
Defaults to 2.
==== [[cache_names]]Standard Caches
@@ -4490,7 +4474,11 @@
an error occurs.
[[sendemail.threadPoolSize]]sendemail.threadPoolSize::
-Deprecated. Replaced with link:#asyncPostUpdate.threadPoolSize[asyncPostUpdate.threadPoolSize]
++
+Maximum size of thread pool in which the review comments
+notifications are sent out asynchronously.
++
+By default, 1.
[[sendemail.from]]sendemail.from::
+
diff --git a/java/com/google/gerrit/acceptance/AbstractNotificationTest.java b/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
index 32bc992..452df67 100644
--- a/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
@@ -48,7 +48,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import org.eclipse.jgit.junit.TestRepository;
@@ -98,7 +97,7 @@
protected static class FakeEmailSenderSubject extends Subject {
private final FakeEmailSender fakeEmailSender;
- private Optional<Message> message;
+ private Message message;
private StagedUsers users;
private Map<RecipientType, List<String>> recipients = new HashMap<>();
private Set<String> accountedFor = new HashSet<>();
@@ -117,29 +116,35 @@
}
public FakeEmailSenderSubject sent(String messageType, StagedUsers users) {
- fakeEmailSender.readOneMessage();
- message =
- fakeEmailSender.getMessages().stream()
- .filter(
- m ->
- m.headers()
- .get("X-Gerrit-MessageType")
- .equals(new EmailHeader.String(messageType)))
- .findFirst();
- if (!message.isPresent()) {
- failWithoutActual(
- fact(String.format("expected message of type %s", messageType), "not sent"));
+ message = fakeEmailSender.nextMessage();
+ if (message == null) {
+ failWithoutActual(fact("expected message", "not sent"));
}
recipients = new HashMap<>();
- recipients.put(TO, parseAddresses(message.get(), "To"));
- recipients.put(CC, parseAddresses(message.get(), "Cc"));
+ recipients.put(TO, parseAddresses(message, "To"));
+ recipients.put(CC, parseAddresses(message, "Cc"));
recipients.put(
BCC,
- message.get().rcpt().stream()
+ message.rcpt().stream()
.map(Address::email)
.filter(e -> !recipients.get(TO).contains(e) && !recipients.get(CC).contains(e))
.collect(toList()));
this.users = users;
+ if (!message.headers().containsKey("X-Gerrit-MessageType")) {
+ failWithoutActual(
+ fact("expected to have message sent with", "X-Gerrit-MessageType header"));
+ }
+ EmailHeader header = message.headers().get("X-Gerrit-MessageType");
+ if (!header.equals(new EmailHeader.String(messageType))) {
+ failWithoutActual(
+ fact("expected message of type", messageType),
+ fact(
+ "actual",
+ header instanceof EmailHeader.String
+ ? ((EmailHeader.String) header).getString()
+ : header));
+ }
+
return this;
}
diff --git a/java/com/google/gerrit/server/PublishCommentsOp.java b/java/com/google/gerrit/server/PublishCommentsOp.java
index e03c244..358ce92 100644
--- a/java/com/google/gerrit/server/PublishCommentsOp.java
+++ b/java/com/google/gerrit/server/PublishCommentsOp.java
@@ -28,7 +28,6 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.CommentsRejectedException;
@@ -47,7 +46,7 @@
* <p>This class uses the {@link PublishCommentUtil} to publish draft comments and fires the
* necessary event for this.
*/
-public class PublishCommentsOp implements BatchUpdateOp, AsyncPostUpdateOp {
+public class PublishCommentsOp implements BatchUpdateOp {
private final PatchSetUtil psUtil;
private final ChangeNotes.Factory changeNotesFactory;
private final ChangeMessagesUtil cmUtil;
@@ -62,8 +61,6 @@
private List<HumanComment> comments = new ArrayList<>();
private ChangeMessage message;
private IdentifiedUser user;
- private ChangeNotes changeNotes;
- private PatchSet patchset;
public interface Factory {
PublishCommentsOp create(PatchSet.Id psId, Project.NameKey projectNameKey);
@@ -112,24 +109,11 @@
@Override
public void postUpdate(Context ctx) {
- changeNotes = changeNotesFactory.createChecked(projectNameKey, psId.changeId());
- patchset = psUtil.get(changeNotes, psId);
-
- commentAdded.fire(
- changeNotes.getChange(),
- patchset,
- ctx.getAccount(),
- message.getMessage(),
- ImmutableMap.of(),
- ImmutableMap.of(),
- ctx.getWhen());
- }
-
- @Override
- public void asyncPostUpdate(Context ctx) {
if (message == null || comments.isEmpty()) {
return;
}
+ ChangeNotes changeNotes = changeNotesFactory.createChecked(projectNameKey, psId.changeId());
+ PatchSet ps = psUtil.get(changeNotes, psId);
NotifyResolver.Result notify = ctx.getNotify(changeNotes.getChangeId());
if (notify.shouldNotify()) {
RepoView repoView;
@@ -140,10 +124,17 @@
String.format("Repository %s not found", ctx.getProject().get()), ex);
}
email
- .create(
- notify, changeNotes, patchset, user, message, comments, null, labelDelta, repoView)
- .send();
+ .create(notify, changeNotes, ps, user, message, comments, null, labelDelta, repoView)
+ .sendAsync();
}
+ commentAdded.fire(
+ changeNotes.getChange(),
+ ps,
+ ctx.getAccount(),
+ message.getMessage(),
+ ImmutableMap.of(),
+ ImmutableMap.of(),
+ ctx.getWhen());
}
private boolean insertMessage(ChangeContext ctx, ChangeUpdate changeUpdate) {
diff --git a/java/com/google/gerrit/server/change/AbandonOp.java b/java/com/google/gerrit/server/change/AbandonOp.java
index e0e489b..6c39ed0 100644
--- a/java/com/google/gerrit/server/change/AbandonOp.java
+++ b/java/com/google/gerrit/server/change/AbandonOp.java
@@ -30,14 +30,13 @@
import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.mail.send.ReplyToChangeSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
-public class AbandonOp implements BatchUpdateOp, AsyncPostUpdateOp {
+public class AbandonOp implements BatchUpdateOp {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final AbandonedSender.Factory abandonedSenderFactory;
@@ -52,7 +51,6 @@
private Change change;
private PatchSet patchSet;
private ChangeMessage message;
- private NotifyResolver.Result notify;
public interface Factory {
AbandonOp create(
@@ -98,7 +96,6 @@
update.setStatus(change.getStatus());
message = newMessage(ctx);
cmUtil.addChangeMessage(update, message);
- notify = ctx.getNotify(change.getId());
return true;
}
@@ -115,11 +112,7 @@
@Override
public void postUpdate(Context ctx) {
- changeAbandoned.fire(change, patchSet, accountState, msgTxt, ctx.getWhen(), notify.handling());
- }
-
- @Override
- public void asyncPostUpdate(Context ctx) {
+ NotifyResolver.Result notify = ctx.getNotify(change.getId());
try {
ReplyToChangeSender emailSender =
abandonedSenderFactory.create(ctx.getProject(), change.getId());
@@ -134,5 +127,6 @@
} catch (Exception e) {
logger.atSevere().withCause(e).log("Cannot email update for change %s", change.getId());
}
+ changeAbandoned.fire(change, patchSet, accountState, msgTxt, ctx.getWhen(), notify.handling());
}
}
diff --git a/java/com/google/gerrit/server/change/AddReviewersEmail.java b/java/com/google/gerrit/server/change/AddReviewersEmail.java
index 5e48353..4a3f638 100644
--- a/java/com/google/gerrit/server/change/AddReviewersEmail.java
+++ b/java/com/google/gerrit/server/change/AddReviewersEmail.java
@@ -23,27 +23,34 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.mail.send.AddReviewerSender;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.Collection;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
@Singleton
public class AddReviewersEmail {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final AddReviewerSender.Factory addReviewerSenderFactory;
+ private final ExecutorService sendEmailsExecutor;
private final MessageIdGenerator messageIdGenerator;
@Inject
AddReviewersEmail(
- AddReviewerSender.Factory addReviewerSenderFactory, MessageIdGenerator messageIdGenerator) {
+ AddReviewerSender.Factory addReviewerSenderFactory,
+ @SendEmailExecutor ExecutorService sendEmailsExecutor,
+ MessageIdGenerator messageIdGenerator) {
this.addReviewerSenderFactory = addReviewerSenderFactory;
+ this.sendEmailsExecutor = sendEmailsExecutor;
this.messageIdGenerator = messageIdGenerator;
}
- public void emailReviewers(
+ public void emailReviewersAsync(
IdentifiedUser user,
Change change,
Collection<Account.Id> added,
@@ -71,20 +78,27 @@
ImmutableList<Address> immutableAddedByEmail = ImmutableList.copyOf(addedByEmail);
ImmutableList<Address> immutableCopiedByEmail = ImmutableList.copyOf(copiedByEmail);
- try {
- AddReviewerSender emailSender = addReviewerSenderFactory.create(projectNameKey, cId);
- emailSender.setNotify(notify);
- emailSender.setFrom(userId);
- emailSender.addReviewers(immutableToMail);
- emailSender.addReviewersByEmail(immutableAddedByEmail);
- emailSender.addExtraCC(immutableToCopy);
- emailSender.addExtraCCByEmail(immutableCopiedByEmail);
- emailSender.setMessageId(
- messageIdGenerator.fromChangeUpdate(change.getProject(), change.currentPatchSetId()));
- emailSender.send();
- } catch (Exception err) {
- logger.atSevere().withCause(err).log(
- "Cannot send email to new reviewers of change %s", change.getId());
- }
+ @SuppressWarnings("unused")
+ Future<?> possiblyIgnoredError =
+ sendEmailsExecutor.submit(
+ () -> {
+ try {
+ AddReviewerSender emailSender =
+ addReviewerSenderFactory.create(projectNameKey, cId);
+ emailSender.setNotify(notify);
+ emailSender.setFrom(userId);
+ emailSender.addReviewers(immutableToMail);
+ emailSender.addReviewersByEmail(immutableAddedByEmail);
+ emailSender.addExtraCC(immutableToCopy);
+ emailSender.addExtraCCByEmail(immutableCopiedByEmail);
+ emailSender.setMessageId(
+ messageIdGenerator.fromChangeUpdate(
+ change.getProject(), change.currentPatchSetId()));
+ emailSender.send();
+ } catch (Exception err) {
+ logger.atSevere().withCause(err).log(
+ "Cannot send email to new reviewers of change %s", change.getId());
+ }
+ });
}
}
diff --git a/java/com/google/gerrit/server/change/AddReviewersOp.java b/java/com/google/gerrit/server/change/AddReviewersOp.java
index fd57722..ff8e5c6 100644
--- a/java/com/google/gerrit/server/change/AddReviewersOp.java
+++ b/java/com/google/gerrit/server/change/AddReviewersOp.java
@@ -42,7 +42,6 @@
import com.google.gerrit.server.extensions.events.ReviewerAdded;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -53,7 +52,7 @@
import java.util.List;
import java.util.Set;
-public class AddReviewersOp implements BatchUpdateOp, AsyncPostUpdateOp {
+public class AddReviewersOp implements BatchUpdateOp {
public interface Factory {
/**
@@ -239,7 +238,7 @@
}
@Override
- public void postUpdate(Context ctx) {
+ public void postUpdate(Context ctx) throws Exception {
opResult =
Result.builder()
.setAddedReviewers(addedReviewers)
@@ -247,20 +246,8 @@
.setAddedCCs(addedCCs)
.setAddedCCsByEmail(addedCCsByEmail)
.build();
- if (!addedReviewers.isEmpty()) {
- List<AccountState> reviewers =
- addedReviewers.stream()
- .map(r -> accountCache.get(r.accountId()))
- .flatMap(Streams::stream)
- .collect(toList());
- reviewerAdded.fire(change, patchSet, reviewers, ctx.getAccount(), ctx.getWhen());
- }
- }
-
- @Override
- public void asyncPostUpdate(Context ctx) {
if (sendEmail) {
- addReviewersEmail.emailReviewers(
+ addReviewersEmail.emailReviewersAsync(
ctx.getUser().asIdentifiedUser(),
change,
Lists.transform(addedReviewers, PatchSetApproval::accountId),
@@ -269,6 +256,14 @@
addedCCsByEmail,
ctx.getNotify(change.getId()));
}
+ if (!addedReviewers.isEmpty()) {
+ List<AccountState> reviewers =
+ addedReviewers.stream()
+ .map(r -> accountCache.get(r.accountId()))
+ .flatMap(Streams::stream)
+ .collect(toList());
+ reviewerAdded.fire(change, patchSet, reviewers, ctx.getAccount(), ctx.getWhen());
+ }
}
public Result getResult() {
diff --git a/java/com/google/gerrit/server/change/AddToAttentionSetOp.java b/java/com/google/gerrit/server/change/AddToAttentionSetOp.java
index 31147fc..8053b30 100644
--- a/java/com/google/gerrit/server/change/AddToAttentionSetOp.java
+++ b/java/com/google/gerrit/server/change/AddToAttentionSetOp.java
@@ -25,7 +25,6 @@
import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -35,7 +34,7 @@
import java.io.IOException;
/** Add a specified user to the attention set. */
-public class AddToAttentionSetOp implements BatchUpdateOp, AsyncPostUpdateOp {
+public class AddToAttentionSetOp implements BatchUpdateOp {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
@@ -102,7 +101,7 @@
}
@Override
- public void asyncPostUpdate(Context ctx) {
+ public void postUpdate(Context ctx) {
if (!notify) {
return;
}
@@ -115,7 +114,7 @@
reason,
messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), change.currentPatchSetId()),
attentionUserId)
- .send();
+ .sendAsync();
} catch (IOException e) {
logger.atSevere().withCause(e).log(e.getMessage(), change.getId());
}
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index 2f95a5c..a086cb1 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -52,6 +52,7 @@
import com.google.gerrit.server.change.ReviewerAdder.InternalAddReviewerInput;
import com.google.gerrit.server.change.ReviewerAdder.ReviewerAddition;
import com.google.gerrit.server.change.ReviewerAdder.ReviewerAdditionList;
+import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.extensions.events.CommentAdded;
@@ -68,12 +69,12 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.ssh.NoSshInfo;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.InsertChangeOp;
import com.google.gerrit.server.update.RepoContext;
import com.google.gerrit.server.util.CommitMessageUtil;
+import com.google.gerrit.server.util.RequestScopePropagator;
import com.google.gerrit.server.validators.ValidationException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -83,13 +84,15 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
-public class ChangeInserter implements InsertChangeOp, AsyncPostUpdateOp {
+public class ChangeInserter implements InsertChangeOp {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
@@ -103,6 +106,7 @@
private final ApprovalsUtil approvalsUtil;
private final ChangeMessagesUtil cmUtil;
private final CreateChangeSender.Factory createChangeSenderFactory;
+ private final ExecutorService sendEmailExecutor;
private final CommitValidators.Factory commitValidatorsFactory;
private final RevisionCreated revisionCreated;
private final CommentAdded commentAdded;
@@ -126,6 +130,7 @@
private List<String> groups = Collections.emptyList();
private boolean validate = true;
private Map<String, Short> approvals;
+ private RequestScopePropagator requestScopePropagator;
private boolean fireRevisionCreated;
private boolean sendMail;
private boolean updateRef;
@@ -141,7 +146,6 @@
private String pushCert;
private ProjectState projectState;
private ReviewerAdditionList reviewerAdditions;
- private NotifyResolver.Result notify;
@Inject
ChangeInserter(
@@ -152,6 +156,7 @@
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
CreateChangeSender.Factory createChangeSenderFactory,
+ @SendEmailExecutor ExecutorService sendEmailExecutor,
CommitValidators.Factory commitValidatorsFactory,
CommentAdded commentAdded,
RevisionCreated revisionCreated,
@@ -168,6 +173,7 @@
this.approvalsUtil = approvalsUtil;
this.cmUtil = cmUtil;
this.createChangeSenderFactory = createChangeSenderFactory;
+ this.sendEmailExecutor = sendEmailExecutor;
this.commitValidatorsFactory = commitValidatorsFactory;
this.revisionCreated = revisionCreated;
this.commentAdded = commentAdded;
@@ -309,6 +315,11 @@
return this;
}
+ public ChangeInserter setRequestScopePropagator(RequestScopePropagator r) {
+ this.requestScopePropagator = r;
+ return this;
+ }
+
public ChangeInserter setRevertOf(Change.Id revertOf) {
this.revertOf = revertOf;
return this;
@@ -446,13 +457,57 @@
ChangeMessagesUtil.uploadedPatchSetTag(workInProgress));
cmUtil.addChangeMessage(update, changeMessage);
}
- notify = ctx.getNotify(change.getId());
return true;
}
@Override
- public void postUpdate(Context ctx) {
+ public void postUpdate(Context ctx) throws Exception {
reviewerAdditions.postUpdate(ctx);
+ NotifyResolver.Result notify = ctx.getNotify(change.getId());
+ if (sendMail && notify.shouldNotify()) {
+ Runnable sender =
+ new Runnable() {
+ @Override
+ public void run() {
+ try {
+ CreateChangeSender emailSender =
+ createChangeSenderFactory.create(change.getProject(), change.getId());
+ emailSender.setFrom(change.getOwner());
+ emailSender.setPatchSet(patchSet, patchSetInfo);
+ emailSender.setNotify(notify);
+ emailSender.addReviewers(
+ reviewerAdditions.flattenResults(AddReviewersOp.Result::addedReviewers).stream()
+ .map(PatchSetApproval::accountId)
+ .collect(toImmutableSet()));
+ emailSender.addReviewersByEmail(
+ reviewerAdditions.flattenResults(AddReviewersOp.Result::addedReviewersByEmail));
+ emailSender.addExtraCC(
+ reviewerAdditions.flattenResults(AddReviewersOp.Result::addedCCs));
+ emailSender.addExtraCCByEmail(
+ reviewerAdditions.flattenResults(AddReviewersOp.Result::addedCCsByEmail));
+ emailSender.setMessageId(
+ messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), patchSet.id()));
+ emailSender.send();
+ } catch (Exception e) {
+ logger.atSevere().withCause(e).log(
+ "Cannot send email for new change %s", change.getId());
+ }
+ }
+
+ @Override
+ public String toString() {
+ return "send-email newchange";
+ }
+ };
+ if (requestScopePropagator != null) {
+ @SuppressWarnings("unused")
+ Future<?> possiblyIgnoredError =
+ sendEmailExecutor.submit(requestScopePropagator.wrap(sender));
+ } else {
+ sender.run();
+ }
+ }
+
/* For labels that are not set in this operation, show the "current" value
* of 0, and no oldValue as the value was not modified by this operation.
* For labels that are set in this operation, the value was modified, so
@@ -480,34 +535,6 @@
}
}
- @Override
- public void asyncPostUpdate(Context ctx) {
- reviewerAdditions.asyncPostUpdate(ctx);
- if (sendMail && notify.shouldNotify()) {
- try {
- CreateChangeSender emailSender =
- createChangeSenderFactory.create(change.getProject(), change.getId());
- emailSender.setFrom(change.getOwner());
- emailSender.setPatchSet(patchSet, patchSetInfo);
- emailSender.setNotify(notify);
- emailSender.addReviewers(
- reviewerAdditions.flattenResults(AddReviewersOp.Result::addedReviewers).stream()
- .map(PatchSetApproval::accountId)
- .collect(toImmutableSet()));
- emailSender.addReviewersByEmail(
- reviewerAdditions.flattenResults(AddReviewersOp.Result::addedReviewersByEmail));
- emailSender.addExtraCC(reviewerAdditions.flattenResults(AddReviewersOp.Result::addedCCs));
- emailSender.addExtraCCByEmail(
- reviewerAdditions.flattenResults(AddReviewersOp.Result::addedCCsByEmail));
- emailSender.setMessageId(
- messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), patchSet.id()));
- emailSender.send();
- } catch (Exception e) {
- logger.atSevere().withCause(e).log("Cannot send email for new change %s", change.getId());
- }
- }
- }
-
private void validate(RepoContext ctx) throws IOException, ResourceConflictException {
if (!validate) {
return;
diff --git a/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java b/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java
index d9ede58..255e13a 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewerByEmailOp.java
@@ -22,7 +22,6 @@
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.mail.send.DeleteReviewerSender;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -30,7 +29,7 @@
import com.google.inject.assistedinject.Assisted;
import java.util.Collections;
-public class DeleteReviewerByEmailOp implements BatchUpdateOp, AsyncPostUpdateOp {
+public class DeleteReviewerByEmailOp implements BatchUpdateOp {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
@@ -74,7 +73,7 @@
}
@Override
- public void asyncPostUpdate(Context ctx) {
+ public void postUpdate(Context ctx) {
try {
NotifyResolver.Result notify = ctx.getNotify(change.getId());
if (!notify.shouldNotify()) {
diff --git a/java/com/google/gerrit/server/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
index de11e07..07cb04f 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewerOp.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
@@ -43,7 +43,6 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.RemoveReviewerControl;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -56,7 +55,7 @@
import java.util.HashMap;
import java.util.Map;
-public class DeleteReviewerOp implements BatchUpdateOp, AsyncPostUpdateOp {
+public class DeleteReviewerOp implements BatchUpdateOp {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
@@ -77,12 +76,11 @@
private final AccountState reviewer;
private final DeleteReviewerInput input;
- private ChangeMessage changeMessage;
- private Change currChange;
- private PatchSet currPs;
- private Map<String, Short> newApprovals = new HashMap<>();
- private Map<String, Short> oldApprovals = new HashMap<>();
- private NotifyResolver.Result notify;
+ ChangeMessage changeMessage;
+ Change currChange;
+ PatchSet currPs;
+ Map<String, Short> newApprovals = new HashMap<>();
+ Map<String, Short> oldApprovals = new HashMap<>();
@Inject
DeleteReviewerOp(
@@ -167,27 +165,13 @@
changeMessage =
ChangeMessagesUtil.newMessage(ctx, msg.toString(), ChangeMessagesUtil.TAG_DELETE_REVIEWER);
cmUtil.addChangeMessage(update, changeMessage);
- notify = ctx.getNotify(currChange.getId());
return true;
}
@Override
public void postUpdate(Context ctx) {
- reviewerDeleted.fire(
- currChange,
- currPs,
- reviewer,
- ctx.getAccount(),
- changeMessage.getMessage(),
- newApprovals,
- oldApprovals,
- notify.handling(),
- ctx.getWhen());
- }
-
- @Override
- public void asyncPostUpdate(Context ctx) {
+ NotifyResolver.Result notify = ctx.getNotify(currChange.getId());
if (input.notify == null
&& currChange.isWorkInProgress()
&& !oldApprovals.isEmpty()
@@ -203,6 +187,16 @@
} catch (Exception err) {
logger.atSevere().withCause(err).log("Cannot email update for change %s", currChange.getId());
}
+ reviewerDeleted.fire(
+ currChange,
+ currPs,
+ reviewer,
+ ctx.getAccount(),
+ changeMessage.getMessage(),
+ newApprovals,
+ oldApprovals,
+ notify.handling(),
+ ctx.getWhen());
}
private Iterable<PatchSetApproval> approvals(ChangeContext ctx, Account.Id accountId) {
diff --git a/java/com/google/gerrit/server/change/EmailReviewComments.java b/java/com/google/gerrit/server/change/EmailReviewComments.java
index 309b041..cacfbe7 100644
--- a/java/com/google/gerrit/server/change/EmailReviewComments.java
+++ b/java/com/google/gerrit/server/change/EmailReviewComments.java
@@ -21,18 +21,24 @@
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.mail.send.CommentSender;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.update.RepoView;
import com.google.gerrit.server.util.LabelVote;
+import com.google.gerrit.server.util.RequestContext;
+import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
-public class EmailReviewComments {
+public class EmailReviewComments implements Runnable, RequestContext {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
@@ -65,8 +71,10 @@
RepoView repoView);
}
+ private final ExecutorService sendEmailsExecutor;
private final PatchSetInfoFactory patchSetInfoFactory;
private final CommentSender.Factory commentSenderFactory;
+ private final ThreadLocalRequestContext requestContext;
private final MessageIdGenerator messageIdGenerator;
private final NotifyResolver.Result notify;
@@ -81,8 +89,10 @@
@Inject
EmailReviewComments(
+ @SendEmailExecutor ExecutorService executor,
PatchSetInfoFactory patchSetInfoFactory,
CommentSender.Factory commentSenderFactory,
+ ThreadLocalRequestContext requestContext,
MessageIdGenerator messageIdGenerator,
@Assisted NotifyResolver.Result notify,
@Assisted ChangeNotes notes,
@@ -93,8 +103,10 @@
@Nullable @Assisted String patchSetComment,
@Assisted List<LabelVote> labels,
@Assisted RepoView repoView) {
+ this.sendEmailsExecutor = executor;
this.patchSetInfoFactory = patchSetInfoFactory;
this.commentSenderFactory = commentSenderFactory;
+ this.requestContext = requestContext;
this.messageIdGenerator = messageIdGenerator;
this.notify = notify;
this.notes = notes;
@@ -107,7 +119,14 @@
this.repoView = repoView;
}
- public void send() {
+ public void sendAsync() {
+ @SuppressWarnings("unused")
+ Future<?> possiblyIgnoredError = sendEmailsExecutor.submit(this);
+ }
+
+ @Override
+ public void run() {
+ RequestContext old = requestContext.setContext(this);
try {
CommentSender emailSender =
commentSenderFactory.create(notes.getProjectName(), notes.getChangeId());
@@ -124,6 +143,18 @@
emailSender.send();
} catch (Exception e) {
logger.atSevere().withCause(e).log("Cannot email comments for %s", patchSet.id());
+ } finally {
+ requestContext.setContext(old);
}
}
+
+ @Override
+ public String toString() {
+ return "send-email comments";
+ }
+
+ @Override
+ public CurrentUser getUser() {
+ return user.getRealUser();
+ }
}
diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java
index 4502408..882352d 100644
--- a/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -49,7 +49,6 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.ssh.NoSshInfo;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -63,7 +62,7 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.transport.ReceiveCommand;
-public class PatchSetInserter implements BatchUpdateOp, AsyncPostUpdateOp {
+public class PatchSetInserter implements BatchUpdateOp {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
@@ -110,7 +109,6 @@
private ChangeMessage changeMessage;
private ReviewerSet oldReviewers;
private boolean oldWorkInProgressState;
- private NotifyResolver.Result notify;
@Inject
public PatchSetInserter(
@@ -281,12 +279,12 @@
throw new BadRequestException(ex.getMessage());
}
}
- notify = ctx.getNotify(change.getId());
return true;
}
@Override
- public void asyncPostUpdate(Context ctx) {
+ public void postUpdate(Context ctx) {
+ NotifyResolver.Result notify = ctx.getNotify(change.getId());
if (notify.shouldNotify() && sendEmail) {
requireNonNull(changeMessage);
try {
@@ -306,10 +304,7 @@
"Cannot send email for new patch set on change %s", change.getId());
}
}
- }
- @Override
- public void postUpdate(Context ctx) {
if (fireRevisionCreated) {
revisionCreated.fire(change, patchSet, ctx.getAccount(), ctx.getWhen(), notify);
}
diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java
index a8cc75a..231359b 100644
--- a/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -34,7 +34,6 @@
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -49,7 +48,7 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
-public class RebaseChangeOp implements BatchUpdateOp, AsyncPostUpdateOp {
+public class RebaseChangeOp implements BatchUpdateOp {
public interface Factory {
RebaseChangeOp create(ChangeNotes notes, PatchSet originalPatchSet, ObjectId baseCommitId);
}
@@ -222,11 +221,6 @@
patchSetInserter.postUpdate(ctx);
}
- @Override
- public void asyncPostUpdate(Context ctx) {
- patchSetInserter.asyncPostUpdate(ctx);
- }
-
public RevCommit getRebasedCommit() {
checkState(rebasedCommit != null, "getRebasedCommit() only valid after updateRepo");
return rebasedCommit;
diff --git a/java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java b/java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java
index 7f4e0e3..e532409 100644
--- a/java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java
+++ b/java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java
@@ -26,7 +26,6 @@
import com.google.gerrit.server.mail.send.RemoveFromAttentionSetSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -37,7 +36,7 @@
import java.util.Optional;
/** Remove a specified user from the attention set. */
-public class RemoveFromAttentionSetOp implements BatchUpdateOp, AsyncPostUpdateOp {
+public class RemoveFromAttentionSetOp implements BatchUpdateOp {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
@@ -102,7 +101,7 @@
}
@Override
- public void asyncPostUpdate(Context ctx) {
+ public void postUpdate(Context ctx) {
if (!notify) {
return;
}
@@ -115,7 +114,7 @@
reason,
messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), change.currentPatchSetId()),
attentionUserId)
- .send();
+ .sendAsync();
} catch (IOException e) {
logger.atSevere().withCause(e).log(e.getMessage(), change.getId());
}
diff --git a/java/com/google/gerrit/server/change/ReviewerAdder.java b/java/com/google/gerrit/server/change/ReviewerAdder.java
index 3638bf2..3d986d2 100644
--- a/java/com/google/gerrit/server/change/ReviewerAdder.java
+++ b/java/com/google/gerrit/server/change/ReviewerAdder.java
@@ -587,7 +587,7 @@
}
}
- public void postUpdate(Context ctx) {
+ public void postUpdate(Context ctx) throws Exception {
for (ReviewerAddition addition : additions()) {
if (addition.op != null) {
addition.op.postUpdate(ctx);
@@ -595,14 +595,6 @@
}
}
- public void asyncPostUpdate(Context ctx) {
- for (ReviewerAddition addition : additions()) {
- if (addition.op != null) {
- addition.op.asyncPostUpdate(ctx);
- }
- }
- }
-
public <T> ImmutableSet<T> flattenResults(
Function<AddReviewersOp.Result, ? extends Collection<T>> func) {
additions()
diff --git a/java/com/google/gerrit/server/change/SetAssigneeOp.java b/java/com/google/gerrit/server/change/SetAssigneeOp.java
index c2e36f5..411c9b6 100644
--- a/java/com/google/gerrit/server/change/SetAssigneeOp.java
+++ b/java/com/google/gerrit/server/change/SetAssigneeOp.java
@@ -28,7 +28,6 @@
import com.google.gerrit.server.mail.send.SetAssigneeSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.plugincontext.PluginSetContext;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -38,7 +37,7 @@
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
-public class SetAssigneeOp implements BatchUpdateOp, AsyncPostUpdateOp {
+public class SetAssigneeOp implements BatchUpdateOp {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
@@ -121,7 +120,7 @@
}
@Override
- public void asyncPostUpdate(Context ctx) {
+ public void postUpdate(Context ctx) {
try {
SetAssigneeSender emailSender =
setAssigneeSenderFactory.create(
@@ -134,10 +133,6 @@
logger.atSevere().withCause(err).log(
"Cannot send email to new assignee of change %s", change.getId());
}
- }
-
- @Override
- public void postUpdate(Context ctx) {
assigneeChanged.fire(
change, ctx.getAccount(), oldAssignee != null ? oldAssignee.state() : null, ctx.getWhen());
}
diff --git a/java/com/google/gerrit/server/change/WorkInProgressOp.java b/java/com/google/gerrit/server/change/WorkInProgressOp.java
index 99f7adb..f0ebb80 100644
--- a/java/com/google/gerrit/server/change/WorkInProgressOp.java
+++ b/java/com/google/gerrit/server/change/WorkInProgressOp.java
@@ -28,7 +28,6 @@
import com.google.gerrit.server.extensions.events.WorkInProgressStateChanged;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -38,7 +37,7 @@
import java.io.IOException;
/* Set work in progress or ready for review state on a change */
-public class WorkInProgressOp implements BatchUpdateOp, AsyncPostUpdateOp {
+public class WorkInProgressOp implements BatchUpdateOp {
public static class Input extends InputWithMessage {
@Nullable public NotifyHandling notify;
@@ -127,7 +126,8 @@
}
@Override
- public void asyncPostUpdate(Context ctx) {
+ public void postUpdate(Context ctx) {
+ stateChanged.fire(change, ps, ctx.getAccount(), ctx.getWhen());
NotifyResolver.Result notify = ctx.getNotify(change.getId());
if (workInProgress
|| notify.handling().compareTo(NotifyHandling.OWNER_REVIEWERS) < 0
@@ -152,11 +152,6 @@
cmsg.getMessage(),
ImmutableList.of(),
repoView)
- .send();
- }
-
- @Override
- public void postUpdate(Context ctx) {
- stateChanged.fire(change, ps, ctx.getAccount(), ctx.getWhen());
+ .sendAsync();
}
}
diff --git a/java/com/google/gerrit/server/config/AsyncPostUpdateExecutor.java b/java/com/google/gerrit/server/config/SendEmailExecutor.java
similarity index 90%
rename from java/com/google/gerrit/server/config/AsyncPostUpdateExecutor.java
rename to java/com/google/gerrit/server/config/SendEmailExecutor.java
index 7951b8c..cf90cbf 100644
--- a/java/com/google/gerrit/server/config/AsyncPostUpdateExecutor.java
+++ b/java/com/google/gerrit/server/config/SendEmailExecutor.java
@@ -1,4 +1,4 @@
-// Copyright (C) 2020 The Android Open Source Project
+// Copyright (C) 2014 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@@ -23,4 +23,4 @@
/** Marker on the global {@link ScheduledThreadPoolExecutor} used to send email. */
@Retention(RUNTIME)
@BindingAnnotation
-public @interface AsyncPostUpdateExecutor {}
+public @interface SendEmailExecutor {}
diff --git a/java/com/google/gerrit/server/config/SysExecutorModule.java b/java/com/google/gerrit/server/config/SysExecutorModule.java
index 5caf6fe..ea45b12 100644
--- a/java/com/google/gerrit/server/config/SysExecutorModule.java
+++ b/java/com/google/gerrit/server/config/SysExecutorModule.java
@@ -51,20 +51,14 @@
@Provides
@Singleton
- @AsyncPostUpdateExecutor
+ @SendEmailExecutor
public ExecutorService provideSendEmailExecutor(
@GerritServerConfig Config config, WorkQueue queues) {
- // sendemail.threadPoolSize is deprecated and overridden by asyncPostUpdate.threadPoolSize.
- int poolSize =
- config.getInt(
- "asyncPostUpdate",
- null,
- "threadPoolSize",
- config.getInt("sendemail", null, "threadPoolSize", 1));
+ int poolSize = config.getInt("sendemail", null, "threadPoolSize", 1);
if (poolSize == 0) {
return newDirectExecutorService();
}
- return queues.createQueue(poolSize, "AsyncPostUpdate", true);
+ return queues.createQueue(poolSize, "SendEmail", true);
}
@Provides
diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java
index 67c7f01..47cbd60 100644
--- a/java/com/google/gerrit/server/git/CommitUtil.java
+++ b/java/com/google/gerrit/server/git/CommitUtil.java
@@ -45,7 +45,6 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.notedb.Sequences;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -301,7 +300,7 @@
return changeId;
}
- private class NotifyOp implements BatchUpdateOp, AsyncPostUpdateOp {
+ private class NotifyOp implements BatchUpdateOp {
private final Change change;
private final ChangeInserter ins;
@@ -311,12 +310,8 @@
}
@Override
- public void postUpdate(Context ctx) {
+ public void postUpdate(Context ctx) throws Exception {
changeReverted.fire(change, ins.getChange(), ctx.getWhen());
- }
-
- @Override
- public void asyncPostUpdate(Context ctx) {
try {
RevertedSender emailSender = revertedSenderFactory.create(ctx.getProject(), change.getId());
emailSender.setFrom(ctx.getAccountId());
diff --git a/java/com/google/gerrit/server/git/MergedByPushOp.java b/java/com/google/gerrit/server/git/MergedByPushOp.java
index 13625bc..40e2730 100644
--- a/java/com/google/gerrit/server/git/MergedByPushOp.java
+++ b/java/com/google/gerrit/server/git/MergedByPushOp.java
@@ -25,19 +25,22 @@
import com.google.gerrit.entities.SubmissionId;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.extensions.events.ChangeMerged;
import com.google.gerrit.server.mail.send.MergedSender;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
+import com.google.gerrit.server.util.RequestScopePropagator;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -49,21 +52,24 @@
* <p>When we find a change corresponding to a commit that is pushed to a branch directly, we close
* the change. This class marks the change as merged, and sends out the email notification.
*/
-public class MergedByPushOp implements BatchUpdateOp, AsyncPostUpdateOp {
+public class MergedByPushOp implements BatchUpdateOp {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
MergedByPushOp create(
+ RequestScopePropagator requestScopePropagator,
PatchSet.Id psId,
@Assisted SubmissionId submissionId,
@Assisted("refName") String refName,
@Assisted("mergeResultRevId") String mergeResultRevId);
}
+ private final RequestScopePropagator requestScopePropagator;
private final PatchSetInfoFactory patchSetInfoFactory;
private final ChangeMessagesUtil cmUtil;
private final MergedSender.Factory mergedSenderFactory;
private final PatchSetUtil psUtil;
+ private final ExecutorService sendEmailExecutor;
private final ChangeMerged changeMerged;
private final MessageIdGenerator messageIdGenerator;
@@ -84,8 +90,10 @@
ChangeMessagesUtil cmUtil,
MergedSender.Factory mergedSenderFactory,
PatchSetUtil psUtil,
+ @SendEmailExecutor ExecutorService sendEmailExecutor,
ChangeMerged changeMerged,
MessageIdGenerator messageIdGenerator,
+ @Assisted RequestScopePropagator requestScopePropagator,
@Assisted PatchSet.Id psId,
@Assisted SubmissionId submissionId,
@Assisted("refName") String refName,
@@ -94,8 +102,10 @@
this.cmUtil = cmUtil;
this.mergedSenderFactory = mergedSenderFactory;
this.psUtil = psUtil;
+ this.sendEmailExecutor = sendEmailExecutor;
this.changeMerged = changeMerged;
this.messageIdGenerator = messageIdGenerator;
+ this.requestScopePropagator = requestScopePropagator;
this.submissionId = submissionId;
this.psId = psId;
this.refName = refName;
@@ -171,27 +181,36 @@
if (!correctBranch) {
return;
}
+ @SuppressWarnings("unused") // Runnable already handles errors
+ Future<?> possiblyIgnoredError =
+ sendEmailExecutor.submit(
+ requestScopePropagator.wrap(
+ new Runnable() {
+ @Override
+ public void run() {
+ try {
+ MergedSender emailSender =
+ mergedSenderFactory.create(ctx.getProject(), psId.changeId());
+ emailSender.setFrom(ctx.getAccountId());
+ emailSender.setPatchSet(patchSet, info);
+ emailSender.setMessageId(
+ messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), patchSet.id()));
+ emailSender.send();
+ } catch (Exception e) {
+ logger.atSevere().withCause(e).log(
+ "Cannot send email for submitted patch set %s", psId);
+ }
+ }
+
+ @Override
+ public String toString() {
+ return "send-email merged";
+ }
+ }));
+
changeMerged.fire(change, patchSet, ctx.getAccount(), mergeResultRevId, ctx.getWhen());
}
- @Override
- public void asyncPostUpdate(Context ctx) {
- if (!correctBranch) {
- return;
- }
-
- try {
- MergedSender emailSender = mergedSenderFactory.create(ctx.getProject(), psId.changeId());
- emailSender.setFrom(ctx.getAccountId());
- emailSender.setPatchSet(patchSet, info);
- emailSender.setMessageId(
- messageIdGenerator.fromChangeUpdate(ctx.getRepoView(), patchSet.id()));
- emailSender.send();
- } catch (Exception e) {
- logger.atSevere().withCause(e).log("Cannot send email for submitted patch set %s", psId);
- }
- }
-
private PatchSetInfo getPatchSetInfo(ChangeContext ctx) throws IOException {
RevWalk rw = ctx.getRevWalk();
RevCommit commit = rw.parseCommit(requireNonNull(patchSet).commitId());
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index e44f422..015ed0b 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -175,6 +175,7 @@
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.LabelVote;
import com.google.gerrit.server.util.MagicBranch;
+import com.google.gerrit.server.util.RequestScopePropagator;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.server.validators.ValidationException;
import com.google.gerrit.util.cli.CmdLineParser;
@@ -338,6 +339,7 @@
private final PluginSetContext<RequestListener> requestListeners;
private final PublishCommentsOp.Factory publishCommentsOp;
private final RetryHelper retryHelper;
+ private final RequestScopePropagator requestScopePropagator;
private final Sequences seq;
private final SetHashtagsOp.Factory hashtagsFactory;
private final SubmoduleOp.Factory subOpFactory;
@@ -419,6 +421,7 @@
ReplaceOp.Factory replaceOpFactory,
PluginSetContext<RequestListener> requestListeners,
RetryHelper retryHelper,
+ RequestScopePropagator requestScopePropagator,
Sequences seq,
SetHashtagsOp.Factory hashtagsFactory,
SubmoduleOp.Factory subOpFactory,
@@ -467,6 +470,7 @@
this.replaceOpFactory = replaceOpFactory;
this.requestListeners = requestListeners;
this.retryHelper = retryHelper;
+ this.requestScopePropagator = requestScopePropagator;
this.seq = seq;
this.subOpFactory = subOpFactory;
this.tagCache = tagCache;
@@ -2582,6 +2586,7 @@
magicBranch.getCombinedCcs(fromFooters))
.setApprovals(approvals)
.setMessage(msg.toString())
+ .setRequestScopePropagator(requestScopePropagator)
.setSendMail(true)
.setPatchSetDescription(magicBranch.message));
if (!magicBranch.hashtags.isEmpty()) {
@@ -3008,20 +3013,22 @@
RevCommit priorCommit = revisions.inverse().get(priorPatchSet);
replaceOp =
- replaceOpFactory.create(
- projectState,
- notes.getChange().getDest(),
- checkMergedInto,
- checkMergedInto ? inputCommand.getNewId().name() : null,
- priorPatchSet,
- priorCommit,
- psId,
- newCommit,
- info,
- groups,
- magicBranch,
- receivePack.getPushCertificate(),
- notes.getChange());
+ replaceOpFactory
+ .create(
+ projectState,
+ notes.getChange().getDest(),
+ checkMergedInto,
+ checkMergedInto ? inputCommand.getNewId().name() : null,
+ priorPatchSet,
+ priorCommit,
+ psId,
+ newCommit,
+ info,
+ groups,
+ magicBranch,
+ receivePack.getPushCertificate(),
+ notes.getChange())
+ .setRequestScopePropagator(requestScopePropagator);
bu.addOp(notes.getChangeId(), replaceOp);
if (progress != null) {
bu.addOp(notes.getChangeId(), new ChangeProgressOp(progress));
@@ -3302,7 +3309,11 @@
bu.addOp(
psId.changeId(),
mergedByPushOpFactory.create(
- psId, submissionId, refName, newTip.getId().getName()));
+ requestScopePropagator,
+ psId,
+ submissionId,
+ refName,
+ newTip.getId().getName()));
continue COMMIT;
}
}
@@ -3346,7 +3357,12 @@
bu.addOp(
id,
mergedByPushOpFactory
- .create(req.psId, submissionId, refName, newTip.getId().getName())
+ .create(
+ requestScopePropagator,
+ req.psId,
+ submissionId,
+ refName,
+ newTip.getId().getName())
.setPatchSetProvider(req.replaceOp::getPatchSet));
bu.addOp(id, new ChangeProgressOp(progress));
ids.add(id);
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index 87266f3..ce62d7a 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -56,6 +56,7 @@
import com.google.gerrit.server.change.ReviewerAdder.InternalAddReviewerInput;
import com.google.gerrit.server.change.ReviewerAdder.ReviewerAddition;
import com.google.gerrit.server.change.ReviewerAdder.ReviewerAdditionList;
+import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.extensions.events.CommentAdded;
import com.google.gerrit.server.extensions.events.RevisionCreated;
@@ -70,11 +71,11 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.RepoContext;
+import com.google.gerrit.server.util.RequestScopePropagator;
import com.google.gerrit.server.validators.ValidationException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -85,6 +86,8 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
import java.util.stream.Stream;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
@@ -93,7 +96,7 @@
import org.eclipse.jgit.transport.PushCertificate;
import org.eclipse.jgit.transport.ReceiveCommand;
-public class ReplaceOp implements BatchUpdateOp, AsyncPostUpdateOp {
+public class ReplaceOp implements BatchUpdateOp {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
@@ -120,6 +123,7 @@
private final ChangeData.Factory changeDataFactory;
private final ChangeKindCache changeKindCache;
private final ChangeMessagesUtil cmUtil;
+ private final ExecutorService sendEmailExecutor;
private final RevisionCreated revisionCreated;
private final CommentAdded commentAdded;
private final MergedByPushOp.Factory mergedByPushOpFactory;
@@ -153,6 +157,7 @@
private ChangeMessage msg;
private String rejectMessage;
private MergedByPushOp mergedByPushOp;
+ private RequestScopePropagator requestScopePropagator;
private ReviewerAdditionList reviewerAdditions;
private MailRecipients oldRecipients;
@@ -169,6 +174,7 @@
PatchSetUtil psUtil,
ReplacePatchSetSender.Factory replacePatchSetFactory,
ProjectCache projectCache,
+ @SendEmailExecutor ExecutorService sendEmailExecutor,
ReviewerAdder reviewerAdder,
Change change,
MessageIdGenerator messageIdGenerator,
@@ -196,6 +202,7 @@
this.psUtil = psUtil;
this.replacePatchSetFactory = replacePatchSetFactory;
this.projectCache = projectCache;
+ this.sendEmailExecutor = sendEmailExecutor;
this.reviewerAdder = reviewerAdder;
this.change = change;
this.messageIdGenerator = messageIdGenerator;
@@ -232,7 +239,11 @@
if (mergedInto != null) {
mergedByPushOp =
mergedByPushOpFactory.create(
- patchSetId, new SubmissionId(change), mergedInto, mergeResultRevId);
+ requestScopePropagator,
+ patchSetId,
+ new SubmissionId(change),
+ mergedInto,
+ mergeResultRevId);
}
}
@@ -482,8 +493,18 @@
}
@Override
- public void postUpdate(Context ctx) {
+ public void postUpdate(Context ctx) throws Exception {
reviewerAdditions.postUpdate(ctx);
+ if (changeKind != ChangeKind.TRIVIAL_REBASE) {
+ // TODO(dborowitz): Merge email templates so we only have to send one.
+ Runnable e = new ReplaceEmailTask(ctx);
+ if (requestScopePropagator != null) {
+ @SuppressWarnings("unused")
+ Future<?> possiblyIgnoredError = sendEmailExecutor.submit(requestScopePropagator.wrap(e));
+ } else {
+ e.run();
+ }
+ }
NotifyResolver.Result notify = ctx.getNotify(notes.getChangeId());
revisionCreated.fire(notes.getChange(), newPatchSet, ctx.getAccount(), ctx.getWhen(), notify);
try {
@@ -496,11 +517,15 @@
}
}
- @Override
- public void asyncPostUpdate(Context ctx) {
- reviewerAdditions.asyncPostUpdate(ctx);
- if (changeKind != ChangeKind.TRIVIAL_REBASE) {
- // TODO(dborowitz): Merge email templates so we only have to send one.
+ private class ReplaceEmailTask implements Runnable {
+ private final Context ctx;
+
+ private ReplaceEmailTask(Context ctx) {
+ this.ctx = ctx;
+ }
+
+ @Override
+ public void run() {
try {
ReplacePatchSetSender emailSender =
replacePatchSetFactory.create(projectState.getNameKey(), notes.getChangeId());
@@ -528,8 +553,10 @@
"Cannot send email for new patch set %s", newPatchSet.id());
}
}
- if (mergedByPushOp != null) {
- mergedByPushOp.asyncPostUpdate(ctx);
+
+ @Override
+ public String toString() {
+ return "send-email newpatchset";
}
}
@@ -586,6 +613,11 @@
return cmd;
}
+ public ReplaceOp setRequestScopePropagator(RequestScopePropagator requestScopePropagator) {
+ this.requestScopePropagator = requestScopePropagator;
+ return this;
+ }
+
private static String findMergedInto(Context ctx, String first, RevCommit commit) {
try {
RevWalk rw = ctx.getRevWalk();
diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index c983c09..df38118 100644
--- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -62,7 +62,6 @@
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -310,7 +309,7 @@
}
}
- private class Op implements BatchUpdateOp, AsyncPostUpdateOp {
+ private class Op implements BatchUpdateOp {
private final PatchSet.Id psId;
private final List<MailComment> parsedComments;
private final String tag;
@@ -354,26 +353,6 @@
@Override
public void postUpdate(Context ctx) throws Exception {
- // Get previous approvals from this user
- Map<String, Short> approvals = new HashMap<>();
- approvalsUtil
- .byPatchSetUser(
- notes, psId, ctx.getAccountId(), ctx.getRevWalk(), ctx.getRepoView().getConfig())
- .forEach(a -> approvals.put(a.label(), a.value()));
- // Fire Gerrit event. Note that approvals can't be granted via email, so old and new approvals
- // are always the same here.
- commentAdded.fire(
- notes.getChange(),
- patchSet,
- ctx.getAccount(),
- changeMessage.getMessage(),
- approvals,
- approvals,
- ctx.getWhen());
- }
-
- @Override
- public void asyncPostUpdate(Context ctx) throws Exception {
String patchSetComment = null;
if (parsedComments.get(0).getType() == MailComment.CommentType.CHANGE_MESSAGE) {
patchSetComment = parsedComments.get(0).getMessage();
@@ -390,7 +369,23 @@
patchSetComment,
ImmutableList.of(),
ctx.getRepoView())
- .send();
+ .sendAsync();
+ // Get previous approvals from this user
+ Map<String, Short> approvals = new HashMap<>();
+ approvalsUtil
+ .byPatchSetUser(
+ notes, psId, ctx.getAccountId(), ctx.getRevWalk(), ctx.getRepoView().getConfig())
+ .forEach(a -> approvals.put(a.label(), a.value()));
+ // Fire Gerrit event. Note that approvals can't be granted via email, so old and new approvals
+ // are always the same here.
+ commentAdded.fire(
+ notes.getChange(),
+ patchSet,
+ ctx.getAccount(),
+ changeMessage.getMessage(),
+ approvals,
+ approvals,
+ ctx.getWhen());
}
private ChangeMessage generateChangeMessage(ChangeContext ctx) {
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteVote.java b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
index 22c3567..4b813df 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
@@ -50,7 +50,6 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.RemoveReviewerControl;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -147,7 +146,7 @@
return Response.none();
}
- private class Op implements BatchUpdateOp, AsyncPostUpdateOp {
+ private class Op implements BatchUpdateOp {
private final ProjectState projectState;
private final AccountState accountState;
private final String label;
@@ -222,30 +221,17 @@
@Override
public void postUpdate(Context ctx) {
- voteDeleted.fire(
- change,
- ps,
- accountState,
- newApprovals,
- oldApprovals,
- input.notify,
- changeMessage.getMessage(),
- ctx.getIdentifiedUser().state(),
- ctx.getWhen());
- }
-
- @Override
- public void asyncPostUpdate(Context ctx) {
if (changeMessage == null) {
return;
}
+ IdentifiedUser user = ctx.getIdentifiedUser();
try {
NotifyResolver.Result notify = ctx.getNotify(change.getId());
if (notify.shouldNotify()) {
ReplyToChangeSender emailSender =
deleteVoteSenderFactory.create(ctx.getProject(), change.getId());
- emailSender.setFrom(ctx.getIdentifiedUser().getAccountId());
+ emailSender.setFrom(user.getAccountId());
emailSender.setChangeMessage(changeMessage.getMessage(), ctx.getWhen());
emailSender.setNotify(notify);
emailSender.setMessageId(
@@ -255,6 +241,17 @@
} catch (Exception e) {
logger.atSevere().withCause(e).log("Cannot email update for change %s", change.getId());
}
+
+ voteDeleted.fire(
+ change,
+ ps,
+ accountState,
+ newApprovals,
+ oldApprovals,
+ input.notify,
+ changeMessage.getMessage(),
+ user.state(),
+ ctx.getWhen());
}
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 843bfd2..3367ca6 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -119,7 +119,6 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -448,7 +447,7 @@
ccByEmail.addAll(addition.reviewersByEmail);
}
}
- addReviewersEmail.emailReviewers(
+ addReviewersEmail.emailReviewersAsync(
user.asIdentifiedUser(), change, to, cc, toByEmail, ccByEmail, notify);
}
}
@@ -870,7 +869,7 @@
abstract Comment.Range range();
}
- private class Op implements BatchUpdateOp, AsyncPostUpdateOp {
+ private class Op implements BatchUpdateOp {
private final ProjectState projectState;
private final PatchSet.Id psId;
private final ReviewInput in;
@@ -916,7 +915,7 @@
}
@Override
- public void asyncPostUpdate(Context ctx) {
+ public void postUpdate(Context ctx) {
if (message == null) {
return;
}
@@ -934,20 +933,12 @@
in.message,
labelDelta,
ctx.getRepoView())
- .send();
+ .sendAsync();
} catch (IOException ex) {
throw new StorageException(
String.format("Repository %s not found", ctx.getProject().get()), ex);
}
}
- }
-
- @Override
- public void postUpdate(Context ctx) {
- if (message == null) {
- return;
- }
-
commentAdded.fire(
notes.getChange(),
ps,
diff --git a/java/com/google/gerrit/server/restapi/change/Restore.java b/java/com/google/gerrit/server/restapi/change/Restore.java
index 26b9d4c..7faf8e0 100644
--- a/java/com/google/gerrit/server/restapi/change/Restore.java
+++ b/java/com/google/gerrit/server/restapi/change/Restore.java
@@ -44,7 +44,6 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -109,7 +108,7 @@
return Response.ok(json.noOptions().format(op.change));
}
- private class Op implements BatchUpdateOp, AsyncPostUpdateOp {
+ private class Op implements BatchUpdateOp {
private final RestoreInput input;
private Change change;
@@ -150,12 +149,6 @@
@Override
public void postUpdate(Context ctx) {
- changeRestored.fire(
- change, patchSet, ctx.getAccount(), Strings.emptyToNull(input.message), ctx.getWhen());
- }
-
- @Override
- public void asyncPostUpdate(Context ctx) {
try {
ReplyToChangeSender emailSender =
restoredSenderFactory.create(ctx.getProject(), change.getId());
@@ -167,6 +160,8 @@
} catch (Exception e) {
logger.atSevere().withCause(e).log("Cannot email update for change %s", change.getId());
}
+ changeRestored.fire(
+ change, patchSet, ctx.getAccount(), Strings.emptyToNull(input.message), ctx.getWhen());
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index d128186..ca39a57 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -72,7 +72,6 @@
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.restapi.change.CherryPickChange.Result;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -601,7 +600,7 @@
}
}
- private class NotifyOp implements BatchUpdateOp, AsyncPostUpdateOp {
+ private class NotifyOp implements BatchUpdateOp {
private final Change change;
private final Change.Id revertChangeId;
@@ -611,13 +610,9 @@
}
@Override
- public void postUpdate(Context ctx) {
+ public void postUpdate(Context ctx) throws Exception {
changeReverted.fire(
change, changeNotesFactory.createChecked(revertChangeId).getChange(), ctx.getWhen());
- }
-
- @Override
- public void asyncPostUpdate(Context ctx) {
try {
RevertedSender emailSender = revertedSenderFactory.create(ctx.getProject(), change.getId());
emailSender.setFrom(ctx.getAccountId());
diff --git a/java/com/google/gerrit/server/submit/EmailMerge.java b/java/com/google/gerrit/server/submit/EmailMerge.java
index c7671b0..4efa4c8 100644
--- a/java/com/google/gerrit/server/submit/EmailMerge.java
+++ b/java/com/google/gerrit/server/submit/EmailMerge.java
@@ -19,14 +19,22 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.NotifyResolver;
+import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.mail.send.MergedSender;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.update.RepoView;
+import com.google.gerrit.server.util.RequestContext;
+import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.inject.Inject;
+import com.google.inject.OutOfScopeException;
import com.google.inject.assistedinject.Assisted;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
-class EmailMerge {
+class EmailMerge implements Runnable, RequestContext {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
interface Factory {
@@ -38,7 +46,10 @@
RepoView repoView);
}
+ private final ExecutorService sendEmailsExecutor;
private final MergedSender.Factory mergedSenderFactory;
+ private final ThreadLocalRequestContext requestContext;
+ private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final MessageIdGenerator messageIdGenerator;
private final Project.NameKey project;
@@ -49,14 +60,20 @@
@Inject
EmailMerge(
+ @SendEmailExecutor ExecutorService executor,
MergedSender.Factory mergedSenderFactory,
+ ThreadLocalRequestContext requestContext,
+ IdentifiedUser.GenericFactory identifiedUserFactory,
MessageIdGenerator messageIdGenerator,
@Assisted Project.NameKey project,
@Assisted Change change,
@Assisted @Nullable Account.Id submitter,
@Assisted NotifyResolver.Result notify,
@Assisted RepoView repoView) {
+ this.sendEmailsExecutor = executor;
this.mergedSenderFactory = mergedSenderFactory;
+ this.requestContext = requestContext;
+ this.identifiedUserFactory = identifiedUserFactory;
this.messageIdGenerator = messageIdGenerator;
this.project = project;
this.change = change;
@@ -65,7 +82,14 @@
this.repoView = repoView;
}
- public void send() {
+ void sendAsync() {
+ @SuppressWarnings("unused")
+ Future<?> possiblyIgnoredError = sendEmailsExecutor.submit(this);
+ }
+
+ @Override
+ public void run() {
+ RequestContext old = requestContext.setContext(this);
try {
MergedSender emailSender = mergedSenderFactory.create(project, change.getId());
if (submitter != null) {
@@ -77,6 +101,21 @@
emailSender.send();
} catch (Exception e) {
logger.atSevere().withCause(e).log("Cannot email merged notification for %s", change.getId());
+ } finally {
+ requestContext.setContext(old);
}
}
+
+ @Override
+ public String toString() {
+ return "send-email merged";
+ }
+
+ @Override
+ public CurrentUser getUser() {
+ if (submitter != null) {
+ return identifiedUserFactory.create(submitter).getRealUser();
+ }
+ throw new OutOfScopeException("No user on email thread");
+ }
}
diff --git a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
index 3629640..edc3725 100644
--- a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
+++ b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
@@ -275,13 +275,6 @@
rebaseOp.postUpdate(ctx);
}
}
-
- @Override
- public void asyncPostUpdateImpl(Context ctx) {
- if (rebaseOp != null) {
- rebaseOp.asyncPostUpdate(ctx);
- }
- }
}
private class MergeIfNecessaryOp extends SubmitStrategyOp {
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
index 201eba8..3cc566b 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
@@ -43,7 +43,6 @@
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.update.AsyncPostUpdateOp;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -61,7 +60,7 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.ReceiveCommand;
-abstract class SubmitStrategyOp implements BatchUpdateOp, AsyncPostUpdateOp {
+abstract class SubmitStrategyOp implements BatchUpdateOp {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
protected final SubmitStrategy.Arguments args;
@@ -484,22 +483,8 @@
}
}
- if (mergeResultRev != null && !args.dryrun) {
- args.changeMerged.fire(
- updatedChange,
- mergedPatchSet,
- args.accountCache.get(submitter.accountId()).orElse(null),
- args.mergeTip.getCurrentTip().name(),
- ctx.getWhen());
- }
- }
-
- /**
- * Assume the change must have been merged at this point, otherwise we would have failed in one of
- * the other steps in postUpdate (which is done prior to this method).
- */
- @Override
- public final void asyncPostUpdate(Context ctx) {
+ // Assume the change must have been merged at this point, otherwise we would
+ // have failed fast in one of the other steps.
try {
args.mergedSenderFactory
.create(
@@ -508,11 +493,18 @@
submitter.accountId(),
ctx.getNotify(getId()),
ctx.getRepoView())
- .send();
+ .sendAsync();
} catch (Exception e) {
logger.atSevere().withCause(e).log("Cannot email merged notification for %s", getId());
}
- asyncPostUpdateImpl(ctx);
+ if (mergeResultRev != null && !args.dryrun) {
+ args.changeMerged.fire(
+ updatedChange,
+ mergedPatchSet,
+ args.accountCache.get(submitter.accountId()).orElse(null),
+ args.mergeTip.getCurrentTip().name(),
+ ctx.getWhen());
+ }
}
/**
@@ -537,12 +529,6 @@
protected void postUpdateImpl(Context ctx) throws Exception {}
/**
- * @see #asyncPostUpdate(Context)
- * @param ctx
- */
- protected void asyncPostUpdateImpl(Context ctx) {}
-
- /**
* Amend the commit with gitlink update
*
* @param commit
diff --git a/java/com/google/gerrit/server/update/AsyncPostUpdateOp.java b/java/com/google/gerrit/server/update/AsyncPostUpdateOp.java
deleted file mode 100644
index 9a989b7..0000000
--- a/java/com/google/gerrit/server/update/AsyncPostUpdateOp.java
+++ /dev/null
@@ -1,38 +0,0 @@
-// Copyright (C) 2020 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.update;
-
-import com.google.gerrit.server.config.SysExecutorModule;
-
-/** Base interface for operations performed asynchronously as part of a {@link BatchUpdate}. */
-public interface AsyncPostUpdateOp {
-
- /**
- * Override this method to do something after the update e.g. send emails. This method will be
- * invoked asynchronously, and when invoked, the invoking method will not wait for the async
- * updates to finish. This method will be called after {@link BatchUpdateOp} operations and {@link
- * RepoOnlyOp} are finished.
- *
- * <p>The maximum amount of threads in the thread pool is decided by
- * asyncPostUpdate.threadPoolSize. When asyncPostUpdate.threadPoolSize is not specified, the
- * deprecated sendemail.threadPoolSize is used (see {@link
- * SysExecutorModule#provideSendEmailExecutor}). This is the case for legacy reasons, since in the
- * past only some emails were sent async (and sendemail.threadPoolSize) was used, and now all
- * emails (and possibly others) are done async, so asyncPostUpdate.threadPoolSize is used.
- *
- * @param ctx context
- */
- default void asyncPostUpdate(Context ctx) throws Exception {}
-}
diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java
index 2fdc124..166e88d 100644
--- a/java/com/google/gerrit/server/update/BatchUpdate.java
+++ b/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -47,7 +47,6 @@
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.change.NotifyResolver;
-import com.google.gerrit.server.config.AsyncPostUpdateExecutor;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.validators.OnSubmitValidators;
@@ -63,7 +62,6 @@
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.NoSuchRefException;
-import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.assistedinject.Assisted;
@@ -77,7 +75,6 @@
import java.util.Optional;
import java.util.TimeZone;
import java.util.TreeMap;
-import java.util.concurrent.ExecutorService;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
@@ -335,8 +332,6 @@
private final NoteDbUpdateManager.Factory updateManagerFactory;
private final ChangeIndexer indexer;
private final GitReferenceUpdated gitRefUpdated;
- private final ThreadLocalRequestContext requestContext;
- private final ExecutorService executorService;
private final Project.NameKey project;
private final CurrentUser user;
@@ -365,8 +360,6 @@
NoteDbUpdateManager.Factory updateManagerFactory,
ChangeIndexer indexer,
GitReferenceUpdated gitRefUpdated,
- ThreadLocalRequestContext requestContext,
- @AsyncPostUpdateExecutor ExecutorService executorService,
@Assisted Project.NameKey project,
@Assisted CurrentUser user,
@Assisted Timestamp when) {
@@ -376,8 +369,6 @@
this.updateManagerFactory = updateManagerFactory;
this.indexer = indexer;
this.gitRefUpdated = gitRefUpdated;
- this.requestContext = requestContext;
- this.executorService = executorService;
this.project = project;
this.user = user;
this.when = when;
@@ -646,40 +637,23 @@
return new ChangeContextImpl(notes);
}
- private void executePostOps() {
+ private void executePostOps() throws Exception {
ContextImpl ctx = new ContextImpl();
for (BatchUpdateOp op : ops.values()) {
- postUpdate(ctx, op);
- if (op instanceof AsyncPostUpdateOp) {
- asyncPostUpdate(ctx, ((AsyncPostUpdateOp) op));
+ try (TraceContext.TraceTimer ignored =
+ TraceContext.newTimer(op.getClass().getSimpleName() + "#postUpdate", Metadata.empty())) {
+ op.postUpdate(ctx);
}
}
for (RepoOnlyOp op : repoOnlyOps) {
- postUpdate(ctx, op);
- if (op instanceof AsyncPostUpdateOp) {
- asyncPostUpdate(ctx, ((AsyncPostUpdateOp) op));
+ try (TraceContext.TraceTimer ignored =
+ TraceContext.newTimer(op.getClass().getSimpleName() + "#postUpdate", Metadata.empty())) {
+ op.postUpdate(ctx);
}
}
}
- /** Invoke the postUpdate methods synchronously. */
- private void postUpdate(ContextImpl ctx, RepoOnlyOp op) {
- try (TraceContext.TraceTimer ignored =
- TraceContext.newTimer(op.getClass().getSimpleName() + "#postUpdate", Metadata.empty())) {
- op.postUpdate(ctx);
- } catch (Exception ex) {
- logDebug(
- String.format(
- "postUpdate for project %s failed for user %s", ctx.getProject(), ctx.getUser()));
- }
- }
-
- /** Invoke the asyncPostUpdate methods asynchronously. */
- private void asyncPostUpdate(ContextImpl ctx, AsyncPostUpdateOp op) {
- executorService.execute(new ExecuteAsyncPostUpdate(op, ctx, user, requestContext));
- }
-
private static void logDebug(String msg) {
// Only log if there is a requestId assigned, since those are the
// expensive/complicated requests like MergeOp. Doing it every time would be
diff --git a/java/com/google/gerrit/server/update/ExecuteAsyncPostUpdate.java b/java/com/google/gerrit/server/update/ExecuteAsyncPostUpdate.java
deleted file mode 100644
index e640ab1..0000000
--- a/java/com/google/gerrit/server/update/ExecuteAsyncPostUpdate.java
+++ /dev/null
@@ -1,65 +0,0 @@
-// Copyright (C) 2020 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.update;
-
-import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.util.RequestContext;
-import com.google.gerrit.server.util.ThreadLocalRequestContext;
-
-/** Executes {@link AsyncPostUpdateOp#asyncPostUpdate(Context)} on a specific op, asynchronously. */
-public class ExecuteAsyncPostUpdate implements Runnable, RequestContext {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
- private final AsyncPostUpdateOp op;
- private final Context ctx;
- private final CurrentUser user;
- private final ThreadLocalRequestContext threadLocalRequestContext;
-
- ExecuteAsyncPostUpdate(
- AsyncPostUpdateOp op,
- Context ctx,
- CurrentUser user,
- ThreadLocalRequestContext threadLocalRequestContext) {
- this.op = op;
- this.ctx = ctx;
- this.user = user;
- this.threadLocalRequestContext = threadLocalRequestContext;
- }
-
- @Override
- public void run() {
- RequestContext old = threadLocalRequestContext.setContext(this);
- try {
- op.asyncPostUpdate(ctx);
- } catch (Exception e) {
- logger.atSevere().withCause(e).log(
- "Cannot perform async post update for repo %s and user %s",
- ctx.getProject(), ctx.getAccount().account().getName());
- } finally {
- threadLocalRequestContext.setContext(old);
- }
- }
-
- @Override
- public String toString() {
- return "async-post-update";
- }
-
- @Override
- public CurrentUser getUser() {
- return user;
- }
-}
diff --git a/java/com/google/gerrit/server/update/RepoOnlyOp.java b/java/com/google/gerrit/server/update/RepoOnlyOp.java
index f9b41c4..7e9c47e 100644
--- a/java/com/google/gerrit/server/update/RepoOnlyOp.java
+++ b/java/com/google/gerrit/server/update/RepoOnlyOp.java
@@ -30,11 +30,10 @@
default void updateRepo(RepoContext ctx) throws Exception {}
/**
- * Override this method to do something after the update e.g. run hooks. This method will
- * <strong>NOT</strong> be invoked asynchronously. This method will be finished before {@link
- * AsyncPostUpdateOp#asyncPostUpdate} is called.
+ * Override this method to do something after the update e.g. send email or run hooks
*
* @param ctx context
*/
+ // TODO(dborowitz): Support async operations?
default void postUpdate(Context ctx) throws Exception {}
}
diff --git a/java/com/google/gerrit/server/util/AttentionSetEmail.java b/java/com/google/gerrit/server/util/AttentionSetEmail.java
index 2b09c49..56b1dda 100644
--- a/java/com/google/gerrit/server/util/AttentionSetEmail.java
+++ b/java/com/google/gerrit/server/util/AttentionSetEmail.java
@@ -17,7 +17,9 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.mail.send.AddToAttentionSetSender;
import com.google.gerrit.server.mail.send.AttentionSetSender;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
@@ -25,8 +27,10 @@
import com.google.gerrit.server.update.Context;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
-public class AttentionSetEmail {
+public class AttentionSetEmail implements Runnable, RequestContext {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
@@ -51,6 +55,7 @@
Account.Id attentionUserId);
}
+ private ExecutorService sendEmailsExecutor;
private AttentionSetSender sender;
private Context ctx;
private Change change;
@@ -61,12 +66,14 @@
@Inject
AttentionSetEmail(
+ @SendEmailExecutor ExecutorService executor,
@Assisted AttentionSetSender sender,
@Assisted Context ctx,
@Assisted Change change,
@Assisted String reason,
@Assisted MessageIdGenerator.MessageId messageId,
@Assisted Account.Id attentionUserId) {
+ this.sendEmailsExecutor = executor;
this.sender = sender;
this.ctx = ctx;
this.change = change;
@@ -75,7 +82,13 @@
this.attentionUserId = attentionUserId;
}
- public void send() {
+ public void sendAsync() {
+ @SuppressWarnings("unused")
+ Future<?> possiblyIgnoredError = sendEmailsExecutor.submit(this);
+ }
+
+ @Override
+ public void run() {
try {
AccountState accountState =
ctx.getUser().isIdentifiedUser() ? ctx.getUser().asIdentifiedUser().state() : null;
@@ -91,4 +104,14 @@
logger.atSevere().withCause(e).log("Cannot email update for change %s", change.getId());
}
}
+
+ @Override
+ public String toString() {
+ return "send-email comments";
+ }
+
+ @Override
+ public CurrentUser getUser() {
+ return ctx.getUser();
+ }
}
diff --git a/java/com/google/gerrit/testing/FakeEmailSender.java b/java/com/google/gerrit/testing/FakeEmailSender.java
index f73020a..fec9b27 100644
--- a/java/com/google/gerrit/testing/FakeEmailSender.java
+++ b/java/com/google/gerrit/testing/FakeEmailSender.java
@@ -128,7 +128,6 @@
}
public synchronized @Nullable Message peekMessage() {
- waitForEmails();
if (messagesRead >= messages.size()) {
return null;
}
@@ -136,14 +135,9 @@
}
public synchronized @Nullable Message nextMessage() {
- waitForEmails();
Message msg = peekMessage();
- readOneMessage();
- return msg;
- }
-
- public synchronized void readOneMessage() {
messagesRead++;
+ return msg;
}
public ImmutableList<Message> getMessages() {
@@ -166,7 +160,7 @@
// a single thread in tests (tricky because most callers just use the
// default executor).
for (WorkQueue.Task<?> task : workQueue.getTasks()) {
- if (task.toString().contains("async-post-update")) {
+ if (task.toString().contains("send-email")) {
try {
task.get();
} catch (ExecutionException | InterruptedException e) {
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index 693fd90..6c9fbed 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -47,7 +47,6 @@
import com.google.gerrit.server.config.AllUsersNameProvider;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
-import com.google.gerrit.server.config.AsyncPostUpdateExecutor;
import com.google.gerrit.server.config.CanonicalWebUrlModule;
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
import com.google.gerrit.server.config.DefaultUrlFormatter;
@@ -59,6 +58,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.config.GerritServerIdProvider;
+import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.config.SitePath;
import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.config.TrackingFootersProvider;
@@ -279,7 +279,7 @@
@Provides
@Singleton
- @AsyncPostUpdateExecutor
+ @SendEmailExecutor
public ExecutorService createSendEmailExecutor() {
return newDirectExecutorService();
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 0bfd525..d4affb7 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -284,11 +284,6 @@
String fileName = "a_new_file.txt";
String fileContent = "First line\nSecond line\n";
PushOneCommit.Result result = createChange("Add a file", fileName, fileContent);
-
- // Emails are sent here async which triggers cache hits, so we must wait until those email are
- // actually sent.
- sender.clear();
-
String triplet = project.get() + "~master~" + result.getChangeId();
CacheStats startPatch = cloneStats(fileCache.stats());
CacheStats startIntra = cloneStats(intraCache.stats());
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java b/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java
index aafe9b9..23bcdec 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractSubmitOnPush.java
@@ -412,17 +412,17 @@
sender.clear();
result = pushTo(pushSpec + ",submit,notify=" + NotifyHandling.OWNER_REVIEWERS);
result.assertOkStatus();
- assertThatEmailsForChangeCreationAndSubmitWereSent(result.getChangeId(), user, null);
+ assertThatEmailsForChangeCreationAndSubmitWereSent(user, null);
sender.clear();
result = pushTo(pushSpec + ",submit,notify=" + NotifyHandling.ALL);
result.assertOkStatus();
- assertThatEmailsForChangeCreationAndSubmitWereSent(result.getChangeId(), user, null);
+ assertThatEmailsForChangeCreationAndSubmitWereSent(user, null);
sender.clear();
result = pushTo(pushSpec + ",submit"); // default is notify = ALL
result.assertOkStatus();
- assertThatEmailsForChangeCreationAndSubmitWereSent(result.getChangeId(), user, null);
+ assertThatEmailsForChangeCreationAndSubmitWereSent(user, null);
}
@Test
@@ -442,22 +442,19 @@
PushOneCommit.Result result =
pushTo(pushSpec + ",submit,notify=" + NotifyHandling.NONE + ",notify-to=" + user2.email());
result.assertOkStatus();
- assertThatEmailsForChangeCreationAndSubmitWereSent(
- result.getChangeId(), user2, RecipientType.TO);
+ assertThatEmailsForChangeCreationAndSubmitWereSent(user2, RecipientType.TO);
sender.clear();
result =
pushTo(pushSpec + ",submit,notify=" + NotifyHandling.NONE + ",notify-cc=" + user2.email());
result.assertOkStatus();
- assertThatEmailsForChangeCreationAndSubmitWereSent(
- result.getChangeId(), user2, RecipientType.CC);
+ assertThatEmailsForChangeCreationAndSubmitWereSent(user2, RecipientType.CC);
sender.clear();
result =
pushTo(pushSpec + ",submit,notify=" + NotifyHandling.NONE + ",notify-bcc=" + user2.email());
result.assertOkStatus();
- assertThatEmailsForChangeCreationAndSubmitWereSent(
- result.getChangeId(), user2, RecipientType.BCC);
+ assertThatEmailsForChangeCreationAndSubmitWereSent(user2, RecipientType.BCC);
}
private PatchSetApproval getSubmitter(PatchSet.Id patchSetId) throws Exception {
@@ -518,15 +515,15 @@
* sent as "To" and sometimes can be sent as "Cc".
*/
private void assertThatEmailsForChangeCreationAndSubmitWereSent(
- String changeId, TestAccount expected, @Nullable RecipientType expectedRecipientType) {
+ TestAccount expected, @Nullable RecipientType expectedRecipientType) {
String expectedEmail = expected.email();
String expectedFullName = expected.fullName();
Address expectedAddress = Address.create(expectedFullName, expectedEmail);
assertThat(sender.getMessages()).hasSize(2);
- Message message = Iterables.getOnlyElement(sender.getMessages(changeId, "newchange"));
+ Message message = sender.getMessages().get(0);
assertThat(message.body().contains("review")).isTrue();
assertAddress(message, expectedAddress, expectedRecipientType);
- message = Iterables.getOnlyElement(sender.getMessages(changeId, "merged"));
+ message = sender.getMessages().get(1);
assertThat(message.rcpt()).containsExactly(expectedAddress);
assertAddress(message, expectedAddress, expectedRecipientType);
assertThat(message.body().contains("submitted")).isTrue();
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
index e040860..a6bd5eb 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
@@ -28,7 +28,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
@@ -399,13 +398,13 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(2);
- Message m = Iterables.getOnlyElement(sender.getMessages(r.getChangeId(), "comment"));
+ Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getNameEmail(), observer.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has posted comments on this change.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertThat(m.body()).contains("Patch Set 1: Code-Review+2");
- m = Iterables.getOnlyElement(sender.getMessages(r.getChangeId(), "newchange"));
+ m = messages.get(1);
assertThat(m.rcpt()).containsExactly(user.getNameEmail(), observer.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
diff --git a/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java b/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
index 7570ce9..1a01184 100644
--- a/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsCommentValidationIT.java
@@ -124,14 +124,12 @@
List<FakeEmailSender.Message> messages = sender.getMessages();
assertThat(messages).hasSize(2);
- FakeEmailSender.Message newPatchsetMessage =
- Iterables.getOnlyElement(sender.getMessages(changeId, "newpatchset"));
+ FakeEmailSender.Message newPatchsetMessage = messages.get(0);
assertThat(newPatchsetMessage.body()).contains("new patch set");
assertThat(newPatchsetMessage.headers().get("Message-ID").toString())
.doesNotContain("EmailReviewComments");
- FakeEmailSender.Message newCommentsMessage =
- Iterables.getOnlyElement(sender.getMessages(changeId, "comment"));
+ FakeEmailSender.Message newCommentsMessage = messages.get(1);
assertThat(newCommentsMessage.body()).contains("has posted comments on this change");
assertThat(newCommentsMessage.headers().get("Message-ID").toString())
.contains("EmailReviewComments");
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
index 7bcd833..b75c26d 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
@@ -127,9 +127,9 @@
/** @override */
created() {
super.created();
- this.addEventListener('content-change', e =>
- this._handleContentChange(e as CustomEvent<{value: string}>)
- );
+ this.addEventListener('content-change', e => {
+ this._handleContentChange(e as CustomEvent<{value: string}>);
+ });
}
/** @override */
diff --git a/polygerrit-ui/app/styles/shared-styles.ts b/polygerrit-ui/app/styles/shared-styles.ts
index 3b738a4..fbc62e2 100644
--- a/polygerrit-ui/app/styles/shared-styles.ts
+++ b/polygerrit-ui/app/styles/shared-styles.ts
@@ -135,7 +135,7 @@
/* Stopgap solution until we remove hidden$ attributes. */
- :host[hidden],
+ :host([hidden]),
[hidden] {
display: none !important;
}
