Merge "Reject restoring change if its destination branch does not exist"
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 001f9b4..28cf49b 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
@@ -76,7 +76,10 @@
NOT_A_DRAFT,
/** Error writing change to git repository */
- GIT_ERROR
+ GIT_ERROR,
+
+ /** The destination branch does not exist */
+ DEST_BRANCH_NOT_FOUND
}
protected Type type;
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AbandonChangeHandler.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AbandonChangeHandler.java
index 3aecb0c..4d1c1f5 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AbandonChangeHandler.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AbandonChangeHandler.java
@@ -28,6 +28,10 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+
+import java.io.IOException;
+
import javax.annotation.Nullable;
class AbandonChangeHandler extends Handler<ChangeDetail> {
@@ -58,7 +62,8 @@
@Override
public ChangeDetail call() throws NoSuchChangeException, OrmException,
EmailException, NoSuchEntityException, InvalidChangeOperationException,
- PatchSetInfoNotAvailableException {
+ PatchSetInfoNotAvailableException, RepositoryNotFoundException,
+ IOException {
final ReviewResult result =
abandonChangeFactory.create(patchSetId, message).call();
if (result.getErrors().size() > 0) {
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
index ab266f3..d765f39 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
@@ -33,8 +33,10 @@
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.ProjectUtil;
import com.google.gerrit.server.account.AccountInfoCacheFactory;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
@@ -46,8 +48,10 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Config;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
@@ -70,6 +74,7 @@
private final AccountInfoCacheFactory aic;
private final AnonymousUser anonymousUser;
private final ReviewDb db;
+ private final GitRepositoryManager repoManager;
private final Change.Id changeId;
@@ -84,6 +89,7 @@
ChangeDetailFactory(final ApprovalTypes approvalTypes,
final FunctionState.Factory functionState,
final PatchSetDetailFactory.Factory patchSetDetail, final ReviewDb db,
+ final GitRepositoryManager repoManager,
final ChangeControl.Factory changeControlFactory,
final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
final AnonymousUser anonymousUser,
@@ -94,6 +100,7 @@
this.functionState = functionState;
this.patchSetDetail = patchSetDetail;
this.db = db;
+ this.repoManager = repoManager;
this.changeControlFactory = changeControlFactory;
this.anonymousUser = anonymousUser;
this.aic = accountInfoCacheFactory.create();
@@ -106,7 +113,8 @@
@Override
public ChangeDetail call() throws OrmException, NoSuchEntityException,
- PatchSetInfoNotAvailableException, NoSuchChangeException {
+ PatchSetInfoNotAvailableException, NoSuchChangeException,
+ RepositoryNotFoundException, IOException {
control = changeControlFactory.validateFor(changeId);
final Change change = control.getChange();
final PatchSet patch = db.patchSets().get(change.currentPatchSetId());
@@ -122,7 +130,9 @@
detail.setCanAbandon(change.getStatus() != Change.Status.DRAFT && change.getStatus().isOpen() && control.canAbandon());
detail.setCanPublish(control.canPublish(db));
- detail.setCanRestore(change.getStatus() == Change.Status.ABANDONED && control.canRestore());
+ detail.setCanRestore(change.getStatus() == Change.Status.ABANDONED
+ && control.canRestore()
+ && ProjectUtil.branchExists(repoManager, change.getDest()));
detail.setCanDeleteDraft(control.canDeleteDraft(db));
detail.setStarred(control.getCurrentUser().getStarredChanges().contains(
changeId));
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PublishAction.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PublishAction.java
index 4ea279f..f57b29c 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PublishAction.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PublishAction.java
@@ -26,6 +26,10 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+
+import java.io.IOException;
+
class PublishAction extends Handler<ChangeDetail> {
interface Factory {
PublishAction create(PatchSet.Id patchSetId);
@@ -49,7 +53,7 @@
@Override
public ChangeDetail call() throws OrmException, NoSuchEntityException,
IllegalStateException, PatchSetInfoNotAvailableException,
- NoSuchChangeException {
+ NoSuchChangeException, RepositoryNotFoundException, IOException {
final ReviewResult result = publishFactory.create(patchSetId).call();
if (result.getErrors().size() > 0) {
throw new IllegalStateException("Cannot publish patchset");
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChangeHandler.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChangeHandler.java
index f018750..bcb03b0 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChangeHandler.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChangeHandler.java
@@ -28,6 +28,10 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+
+import java.io.IOException;
+
import javax.annotation.Nullable;
class RestoreChangeHandler extends Handler<ChangeDetail> {
@@ -57,7 +61,8 @@
@Override
public ChangeDetail call() throws NoSuchChangeException, OrmException,
EmailException, NoSuchEntityException, InvalidChangeOperationException,
- PatchSetInfoNotAvailableException {
+ PatchSetInfoNotAvailableException, RepositoryNotFoundException,
+ IOException {
final ReviewResult result =
restoreChangeFactory.create(patchSetId, message).call();
if (result.getErrors().size() > 0) {
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/SubmitAction.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/SubmitAction.java
index 80100ad..23b21d5 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/SubmitAction.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/SubmitAction.java
@@ -27,6 +27,10 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+
+import java.io.IOException;
+
class SubmitAction extends Handler<ChangeDetail> {
interface Factory {
SubmitAction create(PatchSet.Id patchSetId);
@@ -50,7 +54,8 @@
@Override
public ChangeDetail call() throws OrmException, NoSuchEntityException,
IllegalStateException, InvalidChangeOperationException,
- PatchSetInfoNotAvailableException, NoSuchChangeException {
+ PatchSetInfoNotAvailableException, NoSuchChangeException,
+ RepositoryNotFoundException, IOException {
final ReviewResult result =
submitFactory.create(patchSetId).call();
if (result.getErrors().size() > 0) {
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
index e90467e..40c9b84 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
@@ -52,6 +52,9 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+
+import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -166,6 +169,10 @@
throw new Failure(e);
} catch (PatchSetInfoNotAvailableException e) {
throw new Failure(e);
+ } catch (RepositoryNotFoundException e) {
+ throw new Failure(e);
+ } catch (IOException e) {
+ throw new Failure(e);
}
}
});
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ProjectUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ProjectUtil.java
new file mode 100644
index 0000000..8847f96
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ProjectUtil.java
@@ -0,0 +1,48 @@
+// 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;
+
+import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.server.git.GitRepositoryManager;
+
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.Repository;
+
+import java.io.IOException;
+
+public class ProjectUtil {
+
+ /**
+ * Checks whether the specified branch exists.
+ *
+ * @param repoManager Git repository manager to open the git repository
+ * @param branch the branch for which it should be checked if it exists
+ * @return <code>true</code> if the specified branch exists, otherwise
+ * <code>false</code>
+ * @throws RepositoryNotFoundException the repository of the branch's project
+ * does not exist.
+ * @throws IOException error while retrieving the branch from the repository.
+ */
+ public static boolean branchExists(final GitRepositoryManager repoManager,
+ final Branch.NameKey branch) throws RepositoryNotFoundException,
+ IOException {
+ final Repository repo = repoManager.openRepository(branch.getParentKey());
+ try {
+ return repo.getRef(branch.get()) != null;
+ } finally {
+ repo.close();
+ }
+ }
+}
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
index 7232755..7e52564 100644
--- 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
@@ -17,12 +17,15 @@
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.data.ReviewResult;
+import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.ProjectUtil;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.mail.EmailException;
import com.google.gerrit.server.mail.RestoredSender;
import com.google.gerrit.server.project.ChangeControl;
@@ -33,6 +36,9 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+
+import java.io.IOException;
import java.util.concurrent.Callable;
public class RestoreChange implements Callable<ReviewResult> {
@@ -44,6 +50,7 @@
private final RestoredSender.Factory restoredSenderFactory;
private final ChangeControl.Factory changeControlFactory;
private final ReviewDb db;
+ private final GitRepositoryManager repoManager;
private final IdentifiedUser currentUser;
private final ChangeHooks hooks;
@@ -53,12 +60,13 @@
@Inject
RestoreChange(final RestoredSender.Factory restoredSenderFactory,
final ChangeControl.Factory changeControlFactory, final ReviewDb db,
- final IdentifiedUser currentUser, final ChangeHooks hooks,
- @Assisted final PatchSet.Id patchSetId,
+ final GitRepositoryManager repoManager, 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.repoManager = repoManager;
this.currentUser = currentUser;
this.hooks = hooks;
@@ -68,56 +76,66 @@
@Override
public ReviewResult call() throws EmailException,
- InvalidChangeOperationException, NoSuchChangeException, OrmException {
+ InvalidChangeOperationException, NoSuchChangeException, OrmException,
+ RepositoryNotFoundException, IOException {
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<Change>() {
- @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;
}
+ final PatchSet patch = db.patchSets().get(patchSetId);
+ if (patch == null) {
+ throw new NoSuchChangeException(changeId);
+ }
+
+ final Branch.NameKey destBranch = control.getChange().getDest();
+ if (!ProjectUtil.branchExists(repoManager, destBranch)) {
+ result.addError(new ReviewResult.Error(
+ ReviewResult.Error.Type.DEST_BRANCH_NOT_FOUND, destBranch.get()));
+ return result;
+ }
+
+ // 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<Change>() {
+ @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-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 8c27da0..74782ed 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
@@ -39,6 +39,7 @@
import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.Option;
import org.slf4j.Logger;
@@ -180,8 +181,9 @@
}
}
- private void approveOne(final PatchSet.Id patchSetId) throws
- NoSuchChangeException, OrmException, EmailException, Failure {
+ private void approveOne(final PatchSet.Id patchSetId)
+ throws NoSuchChangeException, OrmException, EmailException, Failure,
+ RepositoryNotFoundException, IOException {
if (changeComment == null) {
changeComment = "";
@@ -261,6 +263,9 @@
case GIT_ERROR:
errMsg += "error writing change to git repository";
break;
+ case DEST_BRANCH_NOT_FOUND:
+ errMsg += "destination branch not found";
+ break;
default:
errMsg += "failure in review";
}
