Migrate CombinedCheckStateUpdatedSender to composition based classes
We are replacing inheritance with composition for email classes. This
change does the same for the emails generated by checks plugin.
Google-Bug-Id: b/273913864
Release-Notes: skip
Change-Id: If179443313a162d810366c9360f39885e46cf52c
diff --git a/java/com/google/gerrit/plugins/checks/ChecksUpdate.java b/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
index 0dae9a7..7ca7999 100644
--- a/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
@@ -27,14 +27,17 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.plugins.checks.Checks.GetCheckOptions;
import com.google.gerrit.plugins.checks.api.CombinedCheckState;
-import com.google.gerrit.plugins.checks.email.CombinedCheckStateUpdatedSender;
+import com.google.gerrit.plugins.checks.email.ChecksEmailModule.ChecksEmailFactories;
+import com.google.gerrit.plugins.checks.email.CombinedCheckStateUpdatedChangeEmailDecorator;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.UserInitiated;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.index.change.ChangeIndexer;
+import com.google.gerrit.server.mail.send.ChangeEmailNew;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
+import com.google.gerrit.server.mail.send.OutgoingEmailNew;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -62,7 +65,7 @@
private final ChecksStorageUpdate checksStorageUpdate;
private final CombinedCheckStateCache combinedCheckStateCache;
- private final CombinedCheckStateUpdatedSender.Factory combinedCheckStateUpdatedSenderFactory;
+ private final ChecksEmailFactories checksEmailFactories;
private final ChangeNotes.Factory notesFactory;
private final PatchSetUtil psUtil;
private final Checks checks;
@@ -77,7 +80,7 @@
ChecksUpdate(
@UserInitiated ChecksStorageUpdate checksStorageUpdate,
CombinedCheckStateCache combinedCheckStateCache,
- CombinedCheckStateUpdatedSender.Factory combinedCheckStateUpdatedSenderFactory,
+ ChecksEmailFactories checksEmailFactories,
ChangeNotes.Factory notesFactory,
PatchSetUtil psUtil,
Checks checks,
@@ -88,7 +91,7 @@
@Assisted IdentifiedUser currentUser) {
this.checksStorageUpdate = checksStorageUpdate;
this.combinedCheckStateCache = combinedCheckStateCache;
- this.combinedCheckStateUpdatedSenderFactory = combinedCheckStateUpdatedSenderFactory;
+ this.checksEmailFactories = checksEmailFactories;
this.notesFactory = notesFactory;
this.psUtil = psUtil;
this.checks = checks;
@@ -103,7 +106,7 @@
ChecksUpdate(
@ServerInitiated ChecksStorageUpdate checksStorageUpdate,
CombinedCheckStateCache combinedCheckStateCache,
- CombinedCheckStateUpdatedSender.Factory combinedCheckStateUpdatedSenderFactory,
+ ChecksEmailFactories checksEmailFactories,
ChangeNotes.Factory notesFactory,
PatchSetUtil psUtil,
Checks checks,
@@ -113,7 +116,7 @@
ChangeIndexer changeIndexer) {
this.checksStorageUpdate = checksStorageUpdate;
this.combinedCheckStateCache = combinedCheckStateCache;
- this.combinedCheckStateUpdatedSenderFactory = combinedCheckStateUpdatedSenderFactory;
+ this.checksEmailFactories = checksEmailFactories;
this.notesFactory = notesFactory;
this.psUtil = psUtil;
this.checks = checks;
@@ -206,18 +209,10 @@
NotifyResolver.Result notify = notifyResolver.resolve(notifyHandling, notifyDetails);
try {
- CombinedCheckStateUpdatedSender sender =
- combinedCheckStateUpdatedSenderFactory.create(
- checkKey.repository(), checkKey.patchSet().changeId());
-
- if (currentUser.isPresent()) {
- sender.setFrom(currentUser.get().getAccountId());
- }
-
- PatchSet patchSet = psUtil.get(changeNotes, checkKey.patchSet());
- sender.setPatchSet(patchSet);
- sender.setCombinedCheckState(oldCombinedCheckState, newCombinedCheckState);
- sender.setCheck(
+ CombinedCheckStateUpdatedChangeEmailDecorator checksEmailDecorator =
+ checksEmailFactories.createChecksEmailDecorator();
+ checksEmailDecorator.setCombinedCheckState(oldCombinedCheckState, newCombinedCheckState);
+ checksEmailDecorator.setCheck(
checkers
.getChecker(checkKey.checkerUuid())
.orElseThrow(
@@ -227,11 +222,20 @@
"checker %s of check %s not found",
checkKey.checkerUuid(), checkKey))),
updatedCheck);
- sender.setNotify(notify);
- sender.setChecksByChecker(getAllChecksByChecker(checkKey));
- sender.setMessageId(
+ checksEmailDecorator.setChecksByChecker(getAllChecksByChecker(checkKey));
+ ChangeEmailNew changeEmail =
+ checksEmailFactories.createChangeEmail(
+ checkKey.repository(), checkKey.patchSet().changeId(), checksEmailDecorator);
+ PatchSet patchSet = psUtil.get(changeNotes, checkKey.patchSet());
+ changeEmail.setPatchSet(patchSet);
+ OutgoingEmailNew outgoingEmail = checksEmailFactories.createEmail(changeEmail);
+ if (currentUser.isPresent()) {
+ outgoingEmail.setFrom(currentUser.get().getAccountId());
+ }
+ outgoingEmail.setNotify(notify);
+ outgoingEmail.setMessageId(
messageIdGenerator.fromChangeUpdate(checkKey.repository(), checkKey.patchSet()));
- sender.send();
+ outgoingEmail.send();
} catch (Exception e) {
logger.atSevere().withCause(e).log(
"Cannot email update for change %s", checkKey.patchSet().changeId());
diff --git a/java/com/google/gerrit/plugins/checks/email/ChecksEmailModule.java b/java/com/google/gerrit/plugins/checks/email/ChecksEmailModule.java
index 3b4e016..48b1df0 100644
--- a/java/com/google/gerrit/plugins/checks/email/ChecksEmailModule.java
+++ b/java/com/google/gerrit/plugins/checks/email/ChecksEmailModule.java
@@ -16,9 +16,17 @@
import static com.google.inject.Scopes.SINGLETON;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.mail.send.ChangeEmailNew;
+import com.google.gerrit.server.mail.send.ChangeEmailNewFactory;
+import com.google.gerrit.server.mail.send.EmailArguments;
import com.google.gerrit.server.mail.send.MailSoyTemplateProvider;
+import com.google.gerrit.server.mail.send.OutgoingEmailNew;
+import com.google.gerrit.server.mail.send.OutgoingEmailNewFactory;
+import com.google.inject.Inject;
public class ChecksEmailModule extends FactoryModule {
@Override
@@ -26,7 +34,36 @@
DynamicSet.bind(binder(), MailSoyTemplateProvider.class)
.to(ChecksMailSoyTemplateProvider.class)
.in(SINGLETON);
+ }
- factory(CombinedCheckStateUpdatedSender.Factory.class);
+ public static class ChecksEmailFactories {
+ private final EmailArguments args;
+ private final ChangeEmailNewFactory changeEmailFactory;
+ private final OutgoingEmailNewFactory outgoingEmailFactory;
+
+ @Inject
+ ChecksEmailFactories(
+ EmailArguments args,
+ ChangeEmailNewFactory changeEmailFactory,
+ OutgoingEmailNewFactory outgoingEmailFactory) {
+ this.args = args;
+ this.changeEmailFactory = changeEmailFactory;
+ this.outgoingEmailFactory = outgoingEmailFactory;
+ }
+
+ public CombinedCheckStateUpdatedChangeEmailDecorator createChecksEmailDecorator() {
+ return new CombinedCheckStateUpdatedChangeEmailDecorator();
+ }
+
+ public ChangeEmailNew createChangeEmail(
+ Project.NameKey project,
+ Change.Id changeId,
+ CombinedCheckStateUpdatedChangeEmailDecorator checksEmailDecorator) {
+ return changeEmailFactory.create(args.newChangeData(project, changeId), checksEmailDecorator);
+ }
+
+ public OutgoingEmailNew createEmail(ChangeEmailNew changeEmail) {
+ return outgoingEmailFactory.create("combinedCheckStateUpdate", changeEmail);
+ }
}
}
diff --git a/java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedSender.java b/java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedChangeEmailDecorator.java
similarity index 76%
rename from java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedSender.java
rename to java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedChangeEmailDecorator.java
index 69f7aa6..5855fdb 100644
--- a/java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedSender.java
+++ b/java/com/google/gerrit/plugins/checks/email/CombinedCheckStateUpdatedChangeEmailDecorator.java
@@ -22,41 +22,29 @@
import com.google.common.base.CaseFormat;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.NotifyConfig.NotifyType;
-import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.EmailException;
+import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.plugins.checks.Check;
import com.google.gerrit.plugins.checks.Checker;
import com.google.gerrit.plugins.checks.api.CheckState;
import com.google.gerrit.plugins.checks.api.CombinedCheckState;
-import com.google.gerrit.server.mail.send.ChangeEmail;
-import com.google.gerrit.server.mail.send.EmailArguments;
-import com.google.gerrit.server.mail.send.ReplyToChangeSender;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
+import com.google.gerrit.server.mail.send.ChangeEmailNew;
+import com.google.gerrit.server.mail.send.ChangeEmailNew.ChangeEmailDecorator;
+import com.google.gerrit.server.mail.send.OutgoingEmailNew;
import java.util.HashMap;
import java.util.Map;
/** Send notice about an update of the combined check state of a change. */
-public class CombinedCheckStateUpdatedSender extends ReplyToChangeSender {
- public interface Factory extends ReplyToChangeSender.Factory<CombinedCheckStateUpdatedSender> {
- @Override
- CombinedCheckStateUpdatedSender create(Project.NameKey project, Change.Id changeId);
- }
-
+public class CombinedCheckStateUpdatedChangeEmailDecorator implements ChangeEmailDecorator {
+ private OutgoingEmailNew email;
+ private ChangeEmailNew changeEmail;
private CombinedCheckState oldCombinedCheckState;
private CombinedCheckState newCombinedCheckState;
private Checker checker;
private Check check;
private ImmutableMap<Checker, Check> checksByChecker;
- @Inject
- public CombinedCheckStateUpdatedSender(
- EmailArguments args, @Assisted Project.NameKey project, @Assisted Change.Id changeId) {
- super(args, "combinedCheckStateUpdate", ChangeEmail.newChangeData(args, project, changeId));
- }
-
public void setCombinedCheckState(
CombinedCheckState oldCombinedCheckState, CombinedCheckState newCombinedCheckState) {
this.oldCombinedCheckState = requireNonNull(oldCombinedCheckState);
@@ -81,19 +69,26 @@
}
@Override
- protected void populateEmailContent() throws EmailException {
- super.populateEmailContent();
+ public void init(OutgoingEmailNew email, ChangeEmailNew changeEmail) {
+ this.email = email;
+ this.changeEmail = changeEmail;
+ changeEmail.markAsReply();
+ }
+
+ @Override
+ public void populateEmailContent() throws EmailException {
+ changeEmail.addAuthors(RecipientType.TO);
if (oldCombinedCheckState != null) {
- addSoyParam("oldCombinedCheckState", oldCombinedCheckState.name());
+ email.addSoyParam("oldCombinedCheckState", oldCombinedCheckState.name());
}
if (newCombinedCheckState != null) {
- addSoyParam("newCombinedCheckState", newCombinedCheckState.name());
+ email.addSoyParam("newCombinedCheckState", newCombinedCheckState.name());
}
if (checker != null && check != null) {
- addSoyParam("checker", getCheckerData(checker, check));
+ email.addSoyParam("checker", getCheckerData(checker, check));
}
if (checksByChecker != null) {
@@ -103,12 +98,17 @@
CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.LOWER_CAMEL, checkState.name()),
getCheckerDataForCheckState(checkState));
}
- addSoyParam("allCheckers", allCheckersData);
+ email.addSoyParam("allCheckers", allCheckersData);
}
- ccAllApprovals();
- bccStarredBy();
- includeWatchers(NotifyType.ALL_COMMENTS);
+ changeEmail.ccAllApprovals();
+ changeEmail.bccStarredBy();
+ changeEmail.includeWatchers(NotifyType.ALL_COMMENTS);
+
+ email.appendText(email.textTemplate("CombinedCheckStateUpdated"));
+ if (email.useHtml()) {
+ email.appendHtml(email.soyHtmlTemplate("CombinedCheckStateUpdatedHtml"));
+ }
}
/**
@@ -150,12 +150,4 @@
.map(e -> getCheckerData(e.getKey(), e.getValue()))
.collect(toImmutableList());
}
-
- @Override
- protected void formatChange() throws EmailException {
- appendText(textTemplate("CombinedCheckStateUpdated"));
- if (useHtml()) {
- appendHtml(soyHtmlTemplate("CombinedCheckStateUpdatedHtml"));
- }
- }
}
