diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java index 068293d54f..2e9d297cea 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java @@ -49,7 +49,10 @@ public class ReviewResult { public static class Error { public static enum Type { /** Not permitted to abandon this change. */ - ABANDON_NOT_PERMITTED + ABANDON_NOT_PERMITTED, + + /** Not permitted to restore this change. */ + RESTORE_NOT_PERMITTED } protected Type type; diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeManageServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeManageServiceImpl.java index 88945cb02c..f055498c8b 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeManageServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeManageServiceImpl.java @@ -24,7 +24,7 @@ import com.google.inject.Inject; class ChangeManageServiceImpl implements ChangeManageService { private final SubmitAction.Factory submitAction; private final AbandonChangeHandler.Factory abandonChangeHandlerFactory; - private final RestoreChange.Factory restoreChangeFactory; + private final RestoreChangeHandler.Factory restoreChangeHandlerFactory; private final RevertChange.Factory revertChangeFactory; private final PublishAction.Factory publishAction; private final DeleteDraftChange.Factory deleteDraftChangeFactory; @@ -32,13 +32,13 @@ class ChangeManageServiceImpl implements ChangeManageService { @Inject ChangeManageServiceImpl(final SubmitAction.Factory patchSetAction, final AbandonChangeHandler.Factory abandonChangeHandlerFactory, - final RestoreChange.Factory restoreChangeFactory, + final RestoreChangeHandler.Factory restoreChangeHandlerFactory, final RevertChange.Factory revertChangeFactory, final PublishAction.Factory publishAction, final DeleteDraftChange.Factory deleteDraftChangeFactory) { this.submitAction = patchSetAction; this.abandonChangeHandlerFactory = abandonChangeHandlerFactory; - this.restoreChangeFactory = restoreChangeFactory; + this.restoreChangeHandlerFactory = restoreChangeHandlerFactory; this.revertChangeFactory = revertChangeFactory; this.publishAction = publishAction; this.deleteDraftChangeFactory = deleteDraftChangeFactory; @@ -61,7 +61,7 @@ class ChangeManageServiceImpl implements ChangeManageService { public void restoreChange(final PatchSet.Id patchSetId, final String message, final AsyncCallback callback) { - restoreChangeFactory.create(patchSetId, message).to(callback); + restoreChangeHandlerFactory.create(patchSetId, message).to(callback); } public void publish(final PatchSet.Id patchSetId, diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java index d91b8e826b..42242930d3 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java @@ -29,7 +29,7 @@ public class ChangeModule extends RpcServletModule { @Override protected void configure() { factory(AbandonChangeHandler.Factory.class); - factory(RestoreChange.Factory.class); + factory(RestoreChangeHandler.Factory.class); factory(RevertChange.Factory.class); factory(ChangeDetailFactory.Factory.class); factory(IncludedInDetailFactory.Factory.class); 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/RestoreChangeHandler.java similarity index 56% rename from gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChange.java rename to gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChangeHandler.java index 2da7b2c6dc..667861a7d5 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/RestoreChangeHandler.java @@ -14,19 +14,14 @@ package com.google.gerrit.httpd.rpc.changedetail; -import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.data.ChangeDetail; +import com.google.gerrit.common.data.ReviewResult; import com.google.gerrit.common.errors.NoSuchEntityException; import com.google.gerrit.httpd.rpc.Handler; -import com.google.gerrit.reviewdb.Change; import com.google.gerrit.reviewdb.PatchSet; -import com.google.gerrit.reviewdb.ReviewDb; -import com.google.gerrit.server.ChangeUtil; -import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.changedetail.RestoreChange; 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; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gwtorm.client.OrmException; @@ -35,55 +30,39 @@ import com.google.inject.assistedinject.Assisted; import javax.annotation.Nullable; -class RestoreChange extends Handler { +class RestoreChangeHandler extends Handler { interface Factory { - RestoreChange create(PatchSet.Id patchSetId, String message); + RestoreChangeHandler create(PatchSet.Id patchSetId, String message); } - private final ChangeControl.Factory changeControlFactory; - private final ReviewDb db; - private final IdentifiedUser currentUser; - private final RestoredSender.Factory senderFactory; + private final RestoreChange.Factory restoreChangeFactory; private final ChangeDetailFactory.Factory changeDetailFactory; private final PatchSet.Id patchSetId; @Nullable private final String message; - private final ChangeHooks hooks; - @Inject - RestoreChange(final ChangeControl.Factory changeControlFactory, - final ReviewDb db, final IdentifiedUser currentUser, - final RestoredSender.Factory senderFactory, + RestoreChangeHandler(final RestoreChange.Factory restoreChangeFactory, final ChangeDetailFactory.Factory changeDetailFactory, @Assisted final PatchSet.Id patchSetId, - @Assisted @Nullable final String message, final ChangeHooks hooks) { - this.changeControlFactory = changeControlFactory; - this.db = db; - this.currentUser = currentUser; - this.senderFactory = senderFactory; + @Assisted @Nullable final String message) { + this.restoreChangeFactory = restoreChangeFactory; this.changeDetailFactory = changeDetailFactory; this.patchSetId = patchSetId; this.message = message; - this.hooks = hooks; } @Override public ChangeDetail call() throws NoSuchChangeException, OrmException, EmailException, NoSuchEntityException, InvalidChangeOperationException, PatchSetInfoNotAvailableException { - - final Change.Id changeId = patchSetId.getParentKey(); - final ChangeControl control = changeControlFactory.validateFor(changeId); - if (!control.canRestore()) { - throw new NoSuchChangeException(changeId); + final ReviewResult result = + restoreChangeFactory.create(patchSetId, message).call(); + if (result.getErrors().size() > 0) { + throw new NoSuchChangeException(result.getChangeId()); } - - ChangeUtil.restore(patchSetId, currentUser, message, db, senderFactory, - hooks); - - return changeDetailFactory.create(changeId).call(); + return changeDetailFactory.create(result.getChangeId()).call(); } } 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 450f8deca5..63c387fe16 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 @@ -33,7 +33,6 @@ import com.google.gerrit.server.git.MergeQueue; import com.google.gerrit.server.git.ReplicationQueue; import com.google.gerrit.server.mail.EmailException; import com.google.gerrit.server.mail.ReplyToChangeSender; -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; @@ -317,49 +316,6 @@ public class ChangeUtil { } } - public static void restore(final PatchSet.Id patchSetId, - final IdentifiedUser user, final String message, final ReviewDb db, - final RestoredSender.Factory senderFactory, - final ChangeHooks hooks) throws NoSuchChangeException, - InvalidChangeOperationException, EmailException, OrmException { - final Change.Id changeId = patchSetId.getParentKey(); - final PatchSet patch = db.patchSets().get(patchSetId); - if (patch == null) { - throw new NoSuchChangeException(changeId); - } - - final ChangeMessage cmsg = - new ChangeMessage(new ChangeMessage.Key(changeId, ChangeUtil - .messageUUID(db)), user.getAccountId(), patchSetId); - final StringBuilder msgBuf = - new StringBuilder("Patch Set " + patchSetId.get() + ": Restored"); - if (message != null && message.length() > 0) { - msgBuf.append("\n\n"); - msgBuf.append(message); - } - cmsg.setMessage(msgBuf.toString()); - - final Change updatedChange = db.changes().atomicUpdate(changeId, - new AtomicUpdate() { - @Override - public Change update(Change change) { - if (change.getStatus() == Change.Status.ABANDONED - && change.currentPatchSetId().equals(patchSetId)) { - change.setStatus(Change.Status.NEW); - ChangeUtil.updated(change); - return change; - } else { - return null; - } - } - }); - - updatedChange(db, user, updatedChange, cmsg, senderFactory, - "Change is not abandoned or patchset is not latest"); - - hooks.doChangeRestoreHook(updatedChange, user.getAccount(), message, db); - } - public static void publishDraftPatchSet(final ReviewDb db, final PatchSet.Id patchSetId) throws OrmException, NoSuchChangeException{ final Change.Id changeId = patchSetId.getParentKey(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RestoreChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RestoreChange.java new file mode 100644 index 0000000000..5df973bf6f --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RestoreChange.java @@ -0,0 +1,123 @@ +// Copyright (C) 2012 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.changedetail; + +import com.google.gerrit.common.ChangeHooks; +import com.google.gerrit.common.data.ReviewResult; +import com.google.gerrit.reviewdb.Change; +import com.google.gerrit.reviewdb.ChangeMessage; +import com.google.gerrit.reviewdb.PatchSet; +import com.google.gerrit.reviewdb.ReviewDb; +import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.mail.RestoredSender; +import com.google.gerrit.server.mail.EmailException; +import com.google.gerrit.server.project.InvalidChangeOperationException; +import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.project.ChangeControl; +import com.google.gwtorm.client.AtomicUpdate; +import com.google.gwtorm.client.OrmException; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; + +import java.util.concurrent.Callable; + +public class RestoreChange implements Callable { + + public interface Factory { + RestoreChange create(PatchSet.Id patchSetId, String changeComment); + } + + private final RestoredSender.Factory restoredSenderFactory; + private final ChangeControl.Factory changeControlFactory; + private final ReviewDb db; + private final IdentifiedUser currentUser; + private final ChangeHooks hooks; + + private final PatchSet.Id patchSetId; + private final String changeComment; + + @Inject + RestoreChange(final RestoredSender.Factory restoredSenderFactory, + final ChangeControl.Factory changeControlFactory, final ReviewDb db, + final IdentifiedUser currentUser, final ChangeHooks hooks, + @Assisted final PatchSet.Id patchSetId, + @Assisted final String changeComment) { + this.restoredSenderFactory = restoredSenderFactory; + this.changeControlFactory = changeControlFactory; + this.db = db; + this.currentUser = currentUser; + this.hooks = hooks; + + this.patchSetId = patchSetId; + this.changeComment = changeComment; + } + + @Override + public ReviewResult call() throws EmailException, + InvalidChangeOperationException, NoSuchChangeException, OrmException { + final ReviewResult result = new ReviewResult(); + + final Change.Id changeId = patchSetId.getParentKey(); + result.setChangeId(changeId); + final ChangeControl control = changeControlFactory.validateFor(changeId); + final PatchSet patch = db.patchSets().get(patchSetId); + if (!control.canRestore()) { + result.addError(new ReviewResult.Error( + ReviewResult.Error.Type.RESTORE_NOT_PERMITTED)); + } else if (patch == null) { + throw new NoSuchChangeException(changeId); + } else { + + // Create a message to accompany the restored change + final ChangeMessage cmsg = + new ChangeMessage(new ChangeMessage.Key(changeId, ChangeUtil + .messageUUID(db)), currentUser.getAccountId(), patchSetId); + final StringBuilder msgBuf = + new StringBuilder("Patch Set " + patchSetId.get() + ": Restored"); + if (changeComment != null && changeComment.length() > 0) { + msgBuf.append("\n\n"); + msgBuf.append(changeComment); + } + cmsg.setMessage(msgBuf.toString()); + + // Restore the change + final Change updatedChange = db.changes().atomicUpdate(changeId, + new AtomicUpdate() { + @Override + public Change update(Change change) { + if (change.getStatus() == Change.Status.ABANDONED + && change.currentPatchSetId().equals(patchSetId)) { + change.setStatus(Change.Status.NEW); + ChangeUtil.updated(change); + return change; + } else { + return null; + } + } + }); + + ChangeUtil.updatedChange( + db, currentUser, updatedChange, cmsg, restoredSenderFactory, + "Change is not abandoned or patchset is not latest"); + + hooks.doChangeRestoreHook(updatedChange, currentUser.getAccount(), + changeComment, db); + } + + return result; + } +} 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 1dc97b5fba..504dfb433c 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 @@ -28,6 +28,7 @@ import com.google.gerrit.server.account.PerformCreateGroup; import com.google.gerrit.server.account.PerformRenameGroup; import com.google.gerrit.server.account.VisibleGroups; import com.google.gerrit.server.changedetail.AbandonChange; +import com.google.gerrit.server.changedetail.RestoreChange; import com.google.gerrit.server.git.CreateCodeReviewNotes; import com.google.gerrit.server.git.MergeOp; import com.google.gerrit.server.git.MetaDataUpdate; @@ -88,6 +89,7 @@ public class GerritRequestModule extends FactoryModule { factory(ReplacePatchSetSender.Factory.class); factory(AbandonedSender.Factory.class); factory(RemoveReviewer.Factory.class); + factory(RestoreChange.Factory.class); factory(RestoredSender.Factory.class); factory(RevertedSender.Factory.class); factory(CommentSender.Factory.class); 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 f57a69bcf2..5fb0d06652 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 @@ -14,7 +14,6 @@ package com.google.gerrit.sshd.commands; -import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.data.ApprovalType; import com.google.gerrit.common.data.ApprovalTypes; import com.google.gerrit.common.data.ReviewResult; @@ -30,13 +29,12 @@ import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.changedetail.AbandonChange; +import com.google.gerrit.server.changedetail.RestoreChange; 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.mail.AbandonedSender; import com.google.gerrit.server.mail.EmailException; -import com.google.gerrit.server.mail.RestoredSender; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.patch.PublishComments; @@ -137,9 +135,6 @@ public class ReviewCommand extends BaseCommand { @Inject private AbandonChange.Factory abandonChangeFactory; - @Inject - private AbandonedSender.Factory abandonedSenderFactory; - @Inject private FunctionState.Factory functionStateFactory; @@ -147,10 +142,7 @@ public class ReviewCommand extends BaseCommand { private PublishComments.Factory publishCommentsFactory; @Inject - private RestoredSender.Factory restoredSenderFactory; - - @Inject - private ChangeHooks hooks; + private RestoreChange.Factory restoreChangeFactory; @Inject private GitRepositoryManager gitManager; @@ -278,15 +270,8 @@ public class ReviewCommand extends BaseCommand { if (abandonChange) { result = abandonChangeFactory.create(patchSetId, changeComment).call(); - } - - if (restoreChange) { - if (changeControl.canRestore()) { - ChangeUtil.restore(patchSetId, currentUser, changeComment, db, - restoredSenderFactory, hooks); - } else { - throw error("Not permitted to restore change"); - } + } else if (restoreChange) { + result = restoreChangeFactory.create(patchSetId, changeComment).call(); if (submitChange) { changeControl = changeControlFactory.validateFor(changeId); } @@ -379,6 +364,10 @@ public class ReviewCommand extends BaseCommand { switch (resultError.getType()) { case ABANDON_NOT_PERMITTED: writeError("error: not permitted to abandon change"); + break; + case RESTORE_NOT_PERMITTED: + writeError("error: not permitted to restore change"); + break; default: writeError("error: failure in review"); }