Merge "Fix: REST API allows to create draft change when drafts are disabled"
This commit is contained in:
@@ -15,17 +15,24 @@
|
|||||||
package com.google.gerrit.acceptance.rest.change;
|
package com.google.gerrit.acceptance.rest.change;
|
||||||
|
|
||||||
import static com.google.common.truth.Truth.assertThat;
|
import static com.google.common.truth.Truth.assertThat;
|
||||||
|
import static com.google.common.truth.TruthJUnit.assume;
|
||||||
|
|
||||||
import com.google.common.collect.Iterables;
|
import com.google.common.collect.Iterables;
|
||||||
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
||||||
import com.google.gerrit.acceptance.RestResponse;
|
import com.google.gerrit.acceptance.RestResponse;
|
||||||
import com.google.gerrit.extensions.client.ChangeStatus;
|
import com.google.gerrit.extensions.client.ChangeStatus;
|
||||||
import com.google.gerrit.extensions.common.ChangeInfo;
|
import com.google.gerrit.extensions.common.ChangeInfo;
|
||||||
|
import com.google.gerrit.testutil.ConfigSuite;
|
||||||
|
|
||||||
import org.apache.http.HttpStatus;
|
import org.apache.http.HttpStatus;
|
||||||
|
import org.eclipse.jgit.lib.Config;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
|
|
||||||
public class CreateChangeIT extends AbstractDaemonTest {
|
public class CreateChangeIT extends AbstractDaemonTest {
|
||||||
|
@ConfigSuite.Config
|
||||||
|
public static Config allowDraftsDisabled() {
|
||||||
|
return allowDraftsDisabledConfig();
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void createEmptyChange_MissingBranch() throws Exception {
|
public void createEmptyChange_MissingBranch() throws Exception {
|
||||||
@@ -61,9 +68,19 @@ public class CreateChangeIT extends AbstractDaemonTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void createDraftChange() throws Exception {
|
public void createDraftChange() throws Exception {
|
||||||
|
assume().that(isAllowDrafts()).isTrue();
|
||||||
assertChange(newChangeInfo(ChangeStatus.DRAFT));
|
assertChange(newChangeInfo(ChangeStatus.DRAFT));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void createDraftChangeNotAllowed() throws Exception {
|
||||||
|
assume().that(isAllowDrafts()).isFalse();
|
||||||
|
ChangeInfo ci = newChangeInfo(ChangeStatus.DRAFT);
|
||||||
|
RestResponse r = adminSession.post("/changes/", ci);
|
||||||
|
assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_METHOD_NOT_ALLOWED);
|
||||||
|
assertThat(r.getEntityContent()).contains("cannot upload drafts");
|
||||||
|
}
|
||||||
|
|
||||||
private ChangeInfo newChangeInfo(ChangeStatus status) {
|
private ChangeInfo newChangeInfo(ChangeStatus status) {
|
||||||
ChangeInfo in = new ChangeInfo();
|
ChangeInfo in = new ChangeInfo();
|
||||||
in.project = project.get();
|
in.project = project.get();
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ import com.google.gerrit.extensions.client.ChangeStatus;
|
|||||||
import com.google.gerrit.extensions.common.ChangeInfo;
|
import com.google.gerrit.extensions.common.ChangeInfo;
|
||||||
import com.google.gerrit.extensions.restapi.AuthException;
|
import com.google.gerrit.extensions.restapi.AuthException;
|
||||||
import com.google.gerrit.extensions.restapi.BadRequestException;
|
import com.google.gerrit.extensions.restapi.BadRequestException;
|
||||||
|
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
|
||||||
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
|
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
|
||||||
import com.google.gerrit.extensions.restapi.Response;
|
import com.google.gerrit.extensions.restapi.Response;
|
||||||
import com.google.gerrit.extensions.restapi.RestModifyView;
|
import com.google.gerrit.extensions.restapi.RestModifyView;
|
||||||
@@ -50,8 +51,10 @@ import com.google.gwtorm.server.OrmException;
|
|||||||
import com.google.inject.Inject;
|
import com.google.inject.Inject;
|
||||||
import com.google.inject.Provider;
|
import com.google.inject.Provider;
|
||||||
import com.google.inject.Singleton;
|
import com.google.inject.Singleton;
|
||||||
|
import com.google.gerrit.server.config.GerritServerConfig;
|
||||||
|
|
||||||
import org.eclipse.jgit.lib.CommitBuilder;
|
import org.eclipse.jgit.lib.CommitBuilder;
|
||||||
|
import org.eclipse.jgit.lib.Config;
|
||||||
import org.eclipse.jgit.lib.Constants;
|
import org.eclipse.jgit.lib.Constants;
|
||||||
import org.eclipse.jgit.lib.ObjectId;
|
import org.eclipse.jgit.lib.ObjectId;
|
||||||
import org.eclipse.jgit.lib.ObjectInserter;
|
import org.eclipse.jgit.lib.ObjectInserter;
|
||||||
@@ -83,6 +86,7 @@ public class CreateChange implements
|
|||||||
private final ChangeInserter.Factory changeInserterFactory;
|
private final ChangeInserter.Factory changeInserterFactory;
|
||||||
private final ChangeJson json;
|
private final ChangeJson json;
|
||||||
private final ChangeUtil changeUtil;
|
private final ChangeUtil changeUtil;
|
||||||
|
private final boolean allowDrafts;
|
||||||
|
|
||||||
@Inject
|
@Inject
|
||||||
CreateChange(Provider<ReviewDb> db,
|
CreateChange(Provider<ReviewDb> db,
|
||||||
@@ -93,7 +97,8 @@ public class CreateChange implements
|
|||||||
CommitValidators.Factory commitValidatorsFactory,
|
CommitValidators.Factory commitValidatorsFactory,
|
||||||
ChangeInserter.Factory changeInserterFactory,
|
ChangeInserter.Factory changeInserterFactory,
|
||||||
ChangeJson json,
|
ChangeJson json,
|
||||||
ChangeUtil changeUtil) {
|
ChangeUtil changeUtil,
|
||||||
|
@GerritServerConfig Config config) {
|
||||||
this.db = db;
|
this.db = db;
|
||||||
this.gitManager = gitManager;
|
this.gitManager = gitManager;
|
||||||
this.serverTimeZone = myIdent.getTimeZone();
|
this.serverTimeZone = myIdent.getTimeZone();
|
||||||
@@ -103,14 +108,15 @@ public class CreateChange implements
|
|||||||
this.changeInserterFactory = changeInserterFactory;
|
this.changeInserterFactory = changeInserterFactory;
|
||||||
this.json = json;
|
this.json = json;
|
||||||
this.changeUtil = changeUtil;
|
this.changeUtil = changeUtil;
|
||||||
|
this.allowDrafts = config.getBoolean("change", "allowDrafts", true);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Response<ChangeInfo> apply(TopLevelResource parent,
|
public Response<ChangeInfo> apply(TopLevelResource parent,
|
||||||
ChangeInfo input) throws AuthException, OrmException,
|
ChangeInfo input) throws AuthException, OrmException,
|
||||||
BadRequestException, UnprocessableEntityException, IOException,
|
BadRequestException, UnprocessableEntityException, IOException,
|
||||||
InvalidChangeOperationException, ResourceNotFoundException {
|
InvalidChangeOperationException, ResourceNotFoundException,
|
||||||
|
MethodNotAllowedException {
|
||||||
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");
|
||||||
}
|
}
|
||||||
@@ -128,6 +134,10 @@ public class CreateChange implements
|
|||||||
&& input.status != ChangeStatus.DRAFT) {
|
&& input.status != ChangeStatus.DRAFT) {
|
||||||
throw new BadRequestException("unsupported change status");
|
throw new BadRequestException("unsupported change status");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!allowDrafts && input.status == ChangeStatus.DRAFT) {
|
||||||
|
throw new MethodNotAllowedException("cannot upload drafts");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
String refName = input.branch;
|
String refName = input.branch;
|
||||||
|
|||||||
Reference in New Issue
Block a user