opendev/gerrit
Code Issues Proposed changes

CreateChange: move logic for "isPrivate" to #checkAndSanitizeChangeInput

Browse Source 
Change-Id: Ia384c293c229c3d398c2bb68c64d434191a561db
This commit is contained in:
Changcheng Xiao
2019-01-28 14:03:20 +01:00
parent a2fc847e73
commit 97ab6d8f7c
2 changed files with 18 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
java/com/google/gerrit/server/restapi/change/CreateChange.java
View File

@@ -165,12 +165,6 @@ public class CreateChange
checkAndSanitizeChangeInput(input); checkAndSanitizeChangeInput(input);
ProjectResource rsrc = projectsCollection.parse(input.project); ProjectResource rsrc = projectsCollection.parse(input.project);
boolean privateByDefault = rsrc.getProjectState().is(BooleanProjectConfig.PRIVATE_BY_DEFAULT);
boolean isPrivate = input.isPrivate == null ? privateByDefault : input.isPrivate;
if (isPrivate && disablePrivateChanges) {
throw new MethodNotAllowedException("private changes are disabled");
}
contributorAgreements.check(rsrc.getNameKey(), rsrc.getUser()); contributorAgreements.check(rsrc.getNameKey(), rsrc.getUser());
@@ -292,7 +286,7 @@ public class CreateChange
ChangeInserter ins = changeInserterFactory.create(changeId, c, refName); ChangeInserter ins = changeInserterFactory.create(changeId, c, refName);
ins.setMessage(String.format("Uploaded patch set %s.", ins.getPatchSetId().get())); ins.setMessage(String.format("Uploaded patch set %s.", ins.getPatchSetId().get()));
ins.setTopic(input.topic); ins.setTopic(input.topic);
ins.setPrivate(isPrivate); ins.setPrivate(input.isPrivate);
ins.setWorkInProgress(isWorkInProgress); ins.setWorkInProgress(isWorkInProgress);
ins.setGroups(groups); ins.setGroups(groups);
ins.setNotify(input.notify); ins.setNotify(input.notify);
@@ -318,7 +312,8 @@ public class CreateChange
* ChangeInput} object so that it can be reused directly by follow-up code. * ChangeInput} object so that it can be reused directly by follow-up code.
* @throws BadRequestException if the input is not legal. * @throws BadRequestException if the input is not legal.
*/ */
private static void checkAndSanitizeChangeInput(ChangeInput input) throws BadRequestException { private void checkAndSanitizeChangeInput(ChangeInput input)
throws RestApiException, PermissionBackendException, IOException {
if (Strings.isNullOrEmpty(input.project)) { if (Strings.isNullOrEmpty(input.project)) {
throw new BadRequestException("project must be non-empty"); throw new BadRequestException("project must be non-empty");
} }
@@ -345,6 +340,16 @@ public class CreateChange
if (input.baseChange != null && input.baseCommit != null) { if (input.baseChange != null && input.baseCommit != null) {
throw new BadRequestException("only provide one of base_change or base_commit"); throw new BadRequestException("only provide one of base_change or base_commit");
} }
ProjectResource projectResource = projectsCollection.parse(input.project);
// Checks whether the change to be created should be a private change.
boolean privateByDefault =
projectResource.getProjectState().is(BooleanProjectConfig.PRIVATE_BY_DEFAULT);
boolean isPrivate = input.isPrivate == null ? privateByDefault : input.isPrivate;
if (isPrivate && disablePrivateChanges) {
throw new MethodNotAllowedException("private changes are disabled");
}
input.isPrivate = isPrivate;
} }
private static RevCommit newCommit( private static RevCommit newCommit(

6
javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
View File

@@ -487,7 +487,11 @@ public class CreateChangeIT extends AbstractDaemonTest {
assertThat(out.subject).isEqualTo(in.subject.split("\n")[0]); assertThat(out.subject).isEqualTo(in.subject.split("\n")[0]);
assertThat(out.topic).isEqualTo(in.topic); assertThat(out.topic).isEqualTo(in.topic);
assertThat(out.status).isEqualTo(in.status); assertThat(out.status).isEqualTo(in.status);
assertThat(out.isPrivate).isEqualTo(in.isPrivate); if (in.isPrivate) {
assertThat(out.isPrivate).isTrue();
} else {
assertThat(out.isPrivate).isNull();
}
assertThat(out.workInProgress).isEqualTo(in.workInProgress); assertThat(out.workInProgress).isEqualTo(in.workInProgress);
assertThat(out.revisions).hasSize(1); assertThat(out.revisions).hasSize(1);
assertThat(out.submitted).isNull(); assertThat(out.submitted).isNull();