opendev/gerrit
Code Issues Proposed changes

CreateMergePatchSet: Do not fail with ISE if given merge strategy is invalid

Browse Source 
That's wrong user input and should result in '400 Bad Request'. The
handling of the InvalidMergeStrategyException in CreateMergePatchSet is
consistent with the handling of the same exception in CreateChange.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Iaf1cdbf00108191f7cf3d4d9497b0ca5e4ba0c34
This commit is contained in:
Edwin Kempin
2020-01-30 09:32:54 +01:00
committed by David Pursehouse
parent a8679a7b70
commit 1709abb6ac
2 changed files with 43 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
View File

@@ -21,6 +21,7 @@ import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.InvalidMergeStrategyException;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.MergeInput;
@@ -194,6 +195,8 @@ public class CreateMergePatchSet
ChangeJson json = jsonFactory.create(ListChangesOption.CURRENT_REVISION);
return Response.ok(json.format(psInserter.getChange()));
} catch (InvalidMergeStrategyException e) {
throw new BadRequestException(e.getMessage());
}
}

40
javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@@ -3332,6 +3332,46 @@ public class ChangeIT extends AbstractDaemonTest {
.isEqualTo(expectedParent);
}
@Test
public void createMergePatchSetWithUnupportedMergeStrategy() throws Exception {
RevCommit initialHead = projectOperations.project(project).getHead("master");
createBranch("dev");
// create a change for master
String changeId = createChange().getChangeId();
String fileName = "shared.txt";
String sourceSubject = "source change";
String sourceContent = "source content";
String targetSubject = "target change";
String targetContent = "target content";
testRepo.reset(initialHead);
PushOneCommit.Result currentMaster =
pushFactory
.create(admin.newIdent(), testRepo, targetSubject, fileName, targetContent)
.to("refs/heads/master");
currentMaster.assertOkStatus();
// push a commit into dev branch
testRepo.reset(initialHead);
PushOneCommit.Result changeA =
pushFactory
.create(user.newIdent(), testRepo, sourceSubject, fileName, sourceContent)
.to("refs/heads/dev");
changeA.assertOkStatus();
MergeInput mergeInput = new MergeInput();
mergeInput.source = "dev";
mergeInput.strategy = "unsupported-strategy";
MergePatchSetInput in = new MergePatchSetInput();
in.merge = mergeInput;
in.subject = "update change by merge ps2";
BadRequestException ex =
assertThrows(
BadRequestException.class, () -> gApi.changes().id(changeId).createMergePatchSet(in));
assertThat(ex).hasMessageThat().isEqualTo("invalid merge strategy: " + mergeInput.strategy);
}
private MergePatchSetInput createMergePatchSetInput(String baseChange) {
MergeInput mergeInput = new MergeInput();
mergeInput.source = "foo";