opendev/gerrit
Code Issues Proposed changes

DeleteBranches: Return 400 Bad Request if input is missing

Browse Source 
In the current implementation if the input is missing, or the list of
branches in the input is missing, it just creates an empty list. Then
the repository is opened, but nothing is done with it because there are
no branches to delete.

Instead, just return an error if the input or the list of branches is
not included in the request.

Change-Id: I7f694d590daf341b16000ec4c3f9b15690dc26cd
This commit is contained in:
David Pursehouse
2016-12-02 17:51:33 +09:00
parent a1bcf92a57
commit 36959f768e
2 changed files with 31 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java
View File

@@ -25,6 +25,7 @@ import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.api.projects.DeleteBranchesInput;
import com.google.gerrit.extensions.api.projects.ProjectApi;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.RefNames;
@@ -107,6 +108,31 @@ public class DeleteBranchesIT extends AbstractDaemonTest {
assertBranchesDeleted();
}
@Test
public void missingInput() throws Exception {
DeleteBranchesInput input = null;
exception.expect(BadRequestException.class);
exception.expectMessage("branches must be specified");
project().deleteBranches(input);
}
@Test
public void missingBranchList() throws Exception {
DeleteBranchesInput input = new DeleteBranchesInput();
exception.expect(BadRequestException.class);
exception.expectMessage("branches must be specified");
project().deleteBranches(input);
}
@Test
public void emptyBranchList() throws Exception {
DeleteBranchesInput input = new DeleteBranchesInput();
input.branches = Lists.newArrayList();
exception.expect(BadRequestException.class);
exception.expectMessage("branches must be specified");
project().deleteBranches(input);
}
private String errorMessageForBranches(List<String> branches) {
StringBuilder message = new StringBuilder();
for (String branch : branches) {

12
gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java
View File

@@ -16,10 +16,11 @@ package com.google.gerrit.server.project;
import static java.lang.String.format;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.api.projects.DeleteBranchesInput;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.server.IdentifiedUser;
@@ -72,13 +73,10 @@ public class DeleteBranches
@Override
public Response<?> apply(ProjectResource project, DeleteBranchesInput input)
throws OrmException, IOException, ResourceConflictException {
throws OrmException, IOException, RestApiException {
if (input == null) {
input = new DeleteBranchesInput();
}
if (input.branches == null) {
input.branches = Lists.newArrayListWithCapacity(1);
if (input == null || input.branches == null || input.branches.isEmpty()) {
throw new BadRequestException("branches must be specified");
}
try (Repository r = repoManager.openRepository(project.getNameKey())) {