From 62e4dad69c896e564da8502cb2f0db1cff5dacc1 Mon Sep 17 00:00:00 2001 From: Tomas Daarstad Date: Fri, 20 Apr 2012 10:09:44 +0200 Subject: Added restoredSender to use when restoring Change-Id: I95e803b9b7cd5d9d43601ec8d866fa80604e9c7f --- .../httpd/rpc/changedetail/RestoreChange.java | 10 ++--- .../gerrit/httpd/rpc/topic/RestoreTopic.java | 10 +++-- .../java/com/google/gerrit/server/ChangeUtil.java | 36 +++++++++-------- .../java/com/google/gerrit/server/TopicUtil.java | 6 +-- .../gerrit/server/config/GerritRequestModule.java | 2 + .../google/gerrit/server/git/ReceiveCommits.java | 16 ++++---- .../google/gerrit/server/mail/RestoredSender.java | 45 ++++++++++++++++++++++ .../com/google/gerrit/server/mail/Restored.vm | 44 +++++++++++++++++++++ .../google/gerrit/sshd/commands/ReviewCommand.java | 6 ++- 9 files changed, 139 insertions(+), 36 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java create mode 100644 gerrit-server/src/main/resources/com/google/gerrit/server/mail/Restored.vm diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChange.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChange.java index fa8785a134..1913c39cb8 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChange.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChange.java @@ -21,8 +21,8 @@ import com.google.gerrit.httpd.rpc.Handler; import com.google.gerrit.reviewdb.*; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.mail.AbandonedSender; import com.google.gerrit.server.mail.EmailException; +import com.google.gerrit.server.mail.RestoredSender; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.InvalidChangeOperationException; @@ -41,7 +41,7 @@ class RestoreChange extends Handler { private final ChangeControl.Factory changeControlFactory; private final ReviewDb db; private final IdentifiedUser currentUser; - private final AbandonedSender.Factory abandonedSenderFactory; + private final RestoredSender.Factory restoredSenderFactory; private final ChangeDetailFactory.Factory changeDetailFactory; private final PatchSet.Id patchSetId; @@ -53,14 +53,14 @@ class RestoreChange extends Handler { @Inject RestoreChange(final ChangeControl.Factory changeControlFactory, final ReviewDb db, final IdentifiedUser currentUser, - final AbandonedSender.Factory abandonedSenderFactory, + final RestoredSender.Factory restoredSenderFactory, final ChangeDetailFactory.Factory changeDetailFactory, @Assisted final PatchSet.Id patchSetId, @Assisted @Nullable final String message, final ChangeHookRunner hooks) { this.changeControlFactory = changeControlFactory; this.db = db; this.currentUser = currentUser; - this.abandonedSenderFactory = abandonedSenderFactory; + this.restoredSenderFactory = restoredSenderFactory; this.changeDetailFactory = changeDetailFactory; this.patchSetId = patchSetId; @@ -80,7 +80,7 @@ class RestoreChange extends Handler { } ChangeUtil.restore(patchSetId, currentUser, message, db, - abandonedSenderFactory, hooks); + restoredSenderFactory, hooks); return changeDetailFactory.create(changeId).call(); } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/topic/RestoreTopic.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/topic/RestoreTopic.java index 521cf05431..cdcb60f3d3 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/topic/RestoreTopic.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/topic/RestoreTopic.java @@ -18,11 +18,13 @@ import com.google.gerrit.common.ChangeHookRunner; import com.google.gerrit.common.data.TopicDetail; import com.google.gerrit.common.errors.NoSuchEntityException; import com.google.gerrit.httpd.rpc.Handler; -import com.google.gerrit.reviewdb.*; +import com.google.gerrit.reviewdb.ChangeSet; +import com.google.gerrit.reviewdb.ReviewDb; +import com.google.gerrit.reviewdb.Topic; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.TopicUtil; -import com.google.gerrit.server.mail.AbandonedSender; import com.google.gerrit.server.mail.EmailException; +import com.google.gerrit.server.mail.RestoredSender; import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchTopicException; @@ -41,7 +43,7 @@ class RestoreTopic extends Handler { private final TopicControl.Factory topicControlFactory; private final ReviewDb db; private final IdentifiedUser currentUser; - private final AbandonedSender.Factory restoredSenderFactory; + private final RestoredSender.Factory restoredSenderFactory; private final TopicDetailFactory.Factory topicDetailFactory; private final ChangeSet.Id changeSetId; @@ -53,7 +55,7 @@ class RestoreTopic extends Handler { @Inject RestoreTopic(final TopicControl.Factory topicControlFactory, final ReviewDb db, final IdentifiedUser currentUser, - final AbandonedSender.Factory restoredSenderFactory, + final RestoredSender.Factory restoredSenderFactory, final TopicDetailFactory.Factory topicDetailFactory, @Assisted final ChangeSet.Id changeSetId, @Assisted @Nullable final String message, final ChangeHookRunner hooks) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index 960af5c9a3..1c9f858194 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -14,13 +14,13 @@ package com.google.gerrit.server; -import static com.google.gerrit.reviewdb.ApprovalCategory.SUBMIT; import static com.google.gerrit.reviewdb.ApprovalCategory.STAGING; +import static com.google.gerrit.reviewdb.ApprovalCategory.SUBMIT; import com.google.gerrit.common.ChangeHookRunner; import com.google.gerrit.reviewdb.Account; -import com.google.gerrit.reviewdb.Branch; import com.google.gerrit.reviewdb.ApprovalCategory; +import com.google.gerrit.reviewdb.Branch; import com.google.gerrit.reviewdb.Change; import com.google.gerrit.reviewdb.ChangeMessage; import com.google.gerrit.reviewdb.ChangeSet; @@ -32,36 +32,36 @@ import com.google.gerrit.reviewdb.RevId; import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.reviewdb.Topic; import com.google.gerrit.reviewdb.TrackingId; -import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.config.TrackingFooter; import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MergeOp; import com.google.gerrit.server.git.MergeQueue; import com.google.gerrit.server.git.ReplicationQueue; -import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.git.StagingUtil; -import com.google.gerrit.server.project.NoSuchRefException; -import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; -import com.google.gerrit.server.project.InvalidChangeOperationException; -import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.mail.AbandonedSender; import com.google.gerrit.server.mail.EmailException; +import com.google.gerrit.server.mail.RestoredSender; import com.google.gerrit.server.mail.RevertedSender; +import com.google.gerrit.server.patch.PatchSetInfoFactory; +import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; +import com.google.gerrit.server.project.InvalidChangeOperationException; +import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.project.NoSuchRefException; import com.google.gwtorm.client.AtomicUpdate; import com.google.gwtorm.client.OrmConcurrencyException; import com.google.gwtorm.client.OrmException; -import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.Ref; -import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.CommitBuilder; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefUpdate; +import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.FooterLine; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; @@ -291,7 +291,7 @@ public class ChangeUtil { cm.setChangeMessage(cmsg); cm.send(); } else { - log.error("Abandoned sender factory is null!"); + log.error("Cannot send email when restoring a change belonging to topic."); } hooks.doChangeAbandonedHook(updatedChange, user.getAccount(), message); @@ -427,7 +427,7 @@ public class ChangeUtil { public static void restore(final PatchSet.Id patchSetId, final IdentifiedUser user, final String message, final ReviewDb db, - final AbandonedSender.Factory senderFactory, + final RestoredSender.Factory senderFactory, final ChangeHookRunner hooks) throws NoSuchChangeException, InvalidChangeOperationException, EmailException, OrmException { restore(patchSetId, user, message, db, senderFactory, hooks, true); @@ -435,7 +435,7 @@ public class ChangeUtil { public static void restore(final PatchSet.Id patchSetId, final IdentifiedUser user, final String message, final ReviewDb db, - final AbandonedSender.Factory senderFactory, + final RestoredSender.Factory senderFactory, final ChangeHookRunner hooks, final boolean sendMail) throws NoSuchChangeException, InvalidChangeOperationException, EmailException, OrmException { final Change.Id changeId = patchSetId.getParentKey(); @@ -484,11 +484,15 @@ public class ChangeUtil { } db.patchSetApprovals().update(approvals); - // Email the reviewers - final AbandonedSender cm = senderFactory.create(updatedChange); + if (senderFactory != null) { + // Email the reviewers + final RestoredSender cm = senderFactory.create(updatedChange); cm.setFrom(user.getAccountId()); cm.setChangeMessage(cmsg); cm.send(); + } else { + log.error("Cannot send email!"); + } hooks.doChangeRestoreHook(updatedChange, user.getAccount(), message); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/TopicUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/TopicUtil.java index 2befc26231..067cfe452d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/TopicUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/TopicUtil.java @@ -46,6 +46,7 @@ import com.google.gerrit.server.project.TopicControl; import com.google.gerrit.server.mail.AbandonedSender; import com.google.gerrit.server.mail.AddReviewerSender; import com.google.gerrit.server.mail.EmailException; +import com.google.gerrit.server.mail.RestoredSender; import com.google.gerrit.server.mail.RevertedSender; import com.google.gwtorm.client.AtomicUpdate; import com.google.gwtorm.client.OrmConcurrencyException; @@ -382,7 +383,7 @@ public static ChangeSetApproval createStagingApproval( public static void restore(final ChangeSet.Id changeSetId, final IdentifiedUser user, final String message, final ReviewDb db, - final AbandonedSender.Factory restoredSenderFactory, + final RestoredSender.Factory restoredSenderFactory, final ChangeHookRunner hooks) throws NoSuchChangeException, NoSuchTopicException, InvalidChangeOperationException, EmailException, OrmException { @@ -442,10 +443,9 @@ public static ChangeSetApproval createStagingApproval( } db.changeSetApprovals().update(approvals); - // TODO Topic support in AbandonedSender // Meanwhile, sending mails in "behalf" of the last change of the topic if (lastChange != null) { - final AbandonedSender cm = restoredSenderFactory.create(lastChange); + final RestoredSender cm = restoredSenderFactory.create(lastChange); cm.setFrom(user.getAccountId()); cm.setTopicMessage(tmsg); cm.send(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java index 42f145d2e0..49d670b6ba 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java @@ -38,6 +38,7 @@ import com.google.gerrit.server.mail.MergeFailSender; import com.google.gerrit.server.mail.MergedSender; import com.google.gerrit.server.mail.RegisterNewEmailSender; import com.google.gerrit.server.mail.ReplacePatchSetSender; +import com.google.gerrit.server.mail.RestoredSender; import com.google.gerrit.server.mail.RevertedSender; import com.google.gerrit.server.patch.PublishComments; import com.google.gerrit.server.project.ChangeControl; @@ -80,6 +81,7 @@ public class GerritRequestModule extends FactoryModule { factory(ReplacePatchSetSender.Factory.class); factory(AbandonedSender.Factory.class); factory(RevertedSender.Factory.class); + factory(RestoredSender.Factory.class); factory(CommentSender.Factory.class); factory(MergedSender.Factory.class); factory(MergeFailSender.Factory.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 629a160e09..922a1fc7a2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -29,11 +29,11 @@ import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.Branch; import com.google.gerrit.reviewdb.Change; import com.google.gerrit.reviewdb.ChangeMessage; -import com.google.gerrit.reviewdb.ContributorAgreement; import com.google.gerrit.reviewdb.ChangeSet; import com.google.gerrit.reviewdb.ChangeSetApproval; import com.google.gerrit.reviewdb.ChangeSetElement; import com.google.gerrit.reviewdb.ChangeSetInfo; +import com.google.gerrit.reviewdb.ContributorAgreement; import com.google.gerrit.reviewdb.PatchSet; import com.google.gerrit.reviewdb.PatchSetAncestor; import com.google.gerrit.reviewdb.PatchSetApproval; @@ -54,12 +54,13 @@ import com.google.gerrit.server.mail.CreateChangeSender; import com.google.gerrit.server.mail.EmailException; import com.google.gerrit.server.mail.MergedSender; import com.google.gerrit.server.mail.ReplacePatchSetSender; +import com.google.gerrit.server.mail.RestoredSender; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.NoSuchRefException; +import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.RefControl; @@ -90,8 +91,8 @@ import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.PostReceiveHook; import org.eclipse.jgit.transport.PreReceiveHook; import org.eclipse.jgit.transport.ReceiveCommand; -import org.eclipse.jgit.transport.ReceivePack; import org.eclipse.jgit.transport.ReceiveCommand.Result; +import org.eclipse.jgit.transport.ReceivePack; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -151,6 +152,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { private final CreateChangeSender.Factory createChangeSenderFactory; private final MergedSender.Factory mergedSenderFactory; private final ReplacePatchSetSender.Factory replacePatchSetFactory; + private final RestoredSender.Factory restoredSenderFactory; private final ReplicationQueue replication; private final PatchSetInfoFactory patchSetInfoFactory; private final ChangeHookRunner hooks; @@ -191,6 +193,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { final CreateChangeSender.Factory createChangeSenderFactory, final MergedSender.Factory mergedSenderFactory, final ReplacePatchSetSender.Factory replacePatchSetFactory, + final RestoredSender.Factory restoredSenderFactory, final ReplicationQueue replication, final PatchSetInfoFactory patchSetInfoFactory, final ChangeHookRunner hooks, final ProjectCache projectCache, @@ -209,6 +212,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { this.createChangeSenderFactory = createChangeSenderFactory; this.mergedSenderFactory = mergedSenderFactory; this.replacePatchSetFactory = replacePatchSetFactory; + this.restoredSenderFactory = restoredSenderFactory; this.replication = replication; this.patchSetInfoFactory = patchSetInfoFactory; this.hooks = hooks; @@ -1620,7 +1624,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { } try { ChangeUtil.restore(change.currentPatchSetId(), currentUser, - restoreMessage + request.topicId.get(), db, null, hooks, false); + restoreMessage + request.topicId.get(), db, restoredSenderFactory, hooks, false); } catch (NoSuchChangeException e) { reject(request.cmd, "change " + request.ontoChange + " not found"); return null; @@ -1629,9 +1633,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { + request.ontoChange); return null; } catch (EmailException e) { - // This should never happen, as it will never send any email - // - return null; + log.error("Cannot send email!", e); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java new file mode 100644 index 0000000000..482d6f71d9 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java @@ -0,0 +1,45 @@ +// Copyright (C) 2011 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.mail; + +import com.google.gerrit.reviewdb.Change; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; + +/** Send notice about a change being restored by its owner. */ +public class RestoredSender extends ReplyToChangeSender { + public static interface Factory { + RestoredSender create(Change change); + } + + @Inject + public RestoredSender(EmailArguments ea, @Assisted Change c) { + super(ea, c, "restore"); + } + + @Override + protected void init() throws EmailException { + super.init(); + + ccAllApprovals(); + bccStarredBy(); + bccWatchesNotifyAllComments(); + } + + @Override + protected void formatChange() throws EmailException { + appendText(velocifyFile("Restored.vm")); + } +} diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/Restored.vm b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/Restored.vm new file mode 100644 index 0000000000..afcbcc50ba --- /dev/null +++ b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/Restored.vm @@ -0,0 +1,44 @@ +## Copyright (C) 2011 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. +## +## +## Template Type: +## ------------- +## This is a velocity mail template, see: http://velocity.apache.org and the +## gerrit-docs:config-mail.txt for more info on modifying gerrit mail templates. +## +## Template File Names and extensions: +## ---------------------------------- +## Gerrit will use templates ending in ".vm" but will ignore templates ending +## in ".vm.example". If a .vm template does not exist, the default internal +## gerrit template which is the same as the .vm.example will be used. If you +## want to override the default template, copy the .vm.example file to a .vm +## file and edit it appropriately. +## +## This Template: +## -------------- +## The Restored.vm template will determine the contents of the email related +## to a change being restored. It is a ChangeEmail: see ChangeSubject.vm and +## ChangeFooter.vm. +## +$fromName has restored this change. + +Change subject: $change.subject +...................................................................... + + +#if ($coverLetter) +$coverLetter + +#end diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java index a50441d95b..d3cf572e9f 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java @@ -34,6 +34,7 @@ import com.google.gerrit.server.git.MergeQueue; import com.google.gerrit.server.git.StagingUtil; import com.google.gerrit.server.mail.AbandonedSender; import com.google.gerrit.server.mail.EmailException; +import com.google.gerrit.server.mail.RestoredSender; import com.google.gerrit.server.patch.PublishComments; import com.google.gerrit.server.project.CanSubmitResult; import com.google.gerrit.server.project.ChangeControl; @@ -129,6 +130,9 @@ public class ReviewCommand extends BaseCommand { @Inject private AbandonedSender.Factory abandonedSenderFactory; + @Inject + private RestoredSender.Factory restoredSenderFactory; + @Inject private FunctionState.Factory functionStateFactory; @@ -307,7 +311,7 @@ public class ReviewCommand extends BaseCommand { if (restoreChange) { if (changeControl.canRestore()) { ChangeUtil.restore(patchSetId, currentUser, changeComment, db, - abandonedSenderFactory, hooks); + restoredSenderFactory, hooks); } else { throw error("Not permitted to restore change"); } -- cgit v1.2.3