summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTomasz Zarna2013-07-23 10:30:43 (EDT)
committerTomasz Zarna2013-07-26 15:24:50 (EDT)
commit433e5de744f11a0657eb5dd59f3de922eb617ab9 (patch)
tree2fb1aacb05c86ad9dd0e39fc5e0a59c9a3e4badb
parent31649c82814e6a54fa8159b40ffd81427d5687b0 (diff)
downloadorg.eclipse.mylyn.reviews-433e5de744f11a0657eb5dd59f3de922eb617ab9.zip
org.eclipse.mylyn.reviews-433e5de744f11a0657eb5dd59f3de922eb617ab9.tar.gz
org.eclipse.mylyn.reviews-433e5de744f11a0657eb5dd59f3de922eb617ab9.tar.bz2
395059: allow to submit a changerefs/changes/78/14778/5
Bug: 395059 Change-Id: I5c48f8c83affd3277abc5138f6f377c8d7a6b2cc Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=395059 Signed-off-by: Tomasz Zarna <tomasz.zarna@tasktop.com>
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java51
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClient.java32
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/SubmitInfo.java29
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/SubmitInput.java29
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/operations/SubmitRequest.java2
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/SubmitInfoTest.java38
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/SubmitInputTest.java35
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java8
-rw-r--r--org.eclipse.mylyn.gerrit.tests/testdata/SubmitInfo_merged.json4
-rw-r--r--org.eclipse.mylyn.gerrit.tests/testdata/SubmitInput_waitForMerge.json1
10 files changed, 206 insertions, 23 deletions
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java
index fc566a5..935f7b8 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient.java
@@ -17,6 +17,7 @@ package org.eclipse.mylyn.internal.gerrit.core.client;
import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.Type;
+import java.net.HttpURLConnection;
import java.net.URLEncoder;
import java.net.UnknownHostException;
import java.util.ArrayList;
@@ -36,6 +37,7 @@ import org.eclipse.mylyn.commons.net.AbstractWebLocation;
import org.eclipse.mylyn.commons.net.WebUtil;
import org.eclipse.mylyn.internal.gerrit.core.GerritCorePlugin;
import org.eclipse.mylyn.internal.gerrit.core.GerritUtil;
+import org.eclipse.mylyn.internal.gerrit.core.client.GerritHttpClient.ErrorHandler;
import org.eclipse.mylyn.internal.gerrit.core.client.GerritHttpClient.Request;
import org.eclipse.mylyn.internal.gerrit.core.client.GerritService.GerritRequest;
import org.eclipse.mylyn.internal.gerrit.core.client.compat.ChangeDetailService;
@@ -51,6 +53,8 @@ import org.eclipse.mylyn.internal.gerrit.core.client.rest.AbandonInput;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.ChangeInfo;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.ReviewInfo;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.ReviewInput;
+import org.eclipse.mylyn.internal.gerrit.core.client.rest.SubmitInfo;
+import org.eclipse.mylyn.internal.gerrit.core.client.rest.SubmitInput;
import org.eclipse.mylyn.internal.gerrit.core.remote.GerritRemoteFactoryProvider;
import org.eclipse.mylyn.reviews.core.model.IRepository;
import org.eclipse.mylyn.reviews.core.model.IReview;
@@ -255,7 +259,7 @@ public class GerritClient extends ReviewsClient {
});
} else {
final String uri = "/a/changes/" + id.getParentKey().get() + "/abandon"; //$NON-NLS-1$ //$NON-NLS-2$
- execute(uri, new AbandonInput(message), ChangeInfo.class, monitor);
+ execute(uri, new AbandonInput(message), ChangeInfo.class, null/*no error handler*/, monitor);
return getChangeDetail(id.getParentKey().get(), monitor);
}
}
@@ -486,7 +490,7 @@ public class GerritClient extends ReviewsClient {
});
} else {
final String uri = "/a/changes/" + id.getParentKey().get() + "/revisions/" + id.get() + "/review"; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
- execute(uri, new ReviewInput(message), ReviewInfo.class, monitor);
+ execute(uri, new ReviewInput(message), ReviewInfo.class, null /*no error handler*/, monitor);
}
}
@@ -692,15 +696,40 @@ public class GerritClient extends ReviewsClient {
});
}
- public ChangeDetail submit(String reviewId, int patchSetId, final String message, IProgressMonitor monitor)
- throws GerritException {
+ public ChangeDetail submit(String reviewId, int patchSetId, IProgressMonitor monitor) throws GerritException {
final PatchSet.Id id = new PatchSet.Id(new Change.Id(id(reviewId)), patchSetId);
- return execute(monitor, new Operation<ChangeDetail>() {
+ if (hasJsonRpcApi(monitor)) {
+ return execute(monitor, new Operation<ChangeDetail>() {
+ @Override
+ public void execute(IProgressMonitor monitor) throws GerritException {
+ getChangeManageService(monitor).submit(id, this);
+ }
+ });
+ } else {
+ return submitRest(id, monitor);
+ }
+ }
+
+ private ChangeDetail submitRest(PatchSet.Id id, IProgressMonitor monitor) throws GerritException {
+ final String uri = "/a/changes/" + id.getParentKey().get() + "/revisions/" + id.get() + "/submit"; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+ execute(uri, new SubmitInput(true), SubmitInfo.class, new ErrorHandler() {
@Override
- public void execute(IProgressMonitor monitor) throws GerritException {
- getChangeManageService(monitor).submit(id, this);
+ public void handleError(HttpMethodBase method) throws GerritException {
+ if (method.getStatusCode() == HttpURLConnection.HTTP_FORBIDDEN
+ && "submit not permitted\n".equals(getResponseBodyAsString(method))) { //$NON-NLS-1$
+ throw new GerritException("Cannot submit change"); //$NON-NLS-1$
+ }
}
- });
+
+ private String getResponseBodyAsString(HttpMethodBase method) {
+ try {
+ return method.getResponseBodyAsString();
+ } catch (IOException e) {
+ return null;
+ }
+ }
+ }, monitor);
+ return getChangeDetail(id.get(), monitor);
}
public List<GerritQueryResult> executeQuery(IProgressMonitor monitor, final String queryString)
@@ -895,13 +924,13 @@ public class GerritClient extends ReviewsClient {
}
}
- private <T> T execute(final String url, final Object input, final Type resultType, IProgressMonitor monitor)
- throws GerritException {
+ private <T> T execute(final String url, final Object input, final Type resultType, final ErrorHandler handler,
+ IProgressMonitor monitor) throws GerritException {
return execute(monitor, new Operation<T>() {
@Override
public void execute(IProgressMonitor monitor) throws GerritException {
try {
- setResult(client.<T> postRestRequest(url, input, resultType, monitor));
+ setResult(client.<T> postRestRequest(url, input, resultType, handler, monitor));
} catch (IOException e) {
throw new GerritException(e);
}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClient.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClient.java
index 9f3575e..f645554 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClient.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClient.java
@@ -64,9 +64,16 @@ public class GerritHttpClient {
public abstract T process(HttpMethodBase method) throws IOException;
+ public void handleError(HttpMethodBase method) throws GerritException {
+ // do nothing by default
+ }
+ }
+
+ public static interface ErrorHandler {
+ public void handleError(HttpMethodBase method) throws GerritException;
}
- private class JsonRequest extends Request<String> {
+ class JsonRequest extends Request<String> {
private final String serviceUri;
@@ -105,10 +112,13 @@ public class GerritHttpClient {
private final Type resultType;
- public RestRequest(final String serviceUri, final Object input, Type resultType) {
+ private final ErrorHandler errorHandler;
+
+ public RestRequest(final String serviceUri, final Object input, Type resultType, ErrorHandler handler) {
this.serviceUri = serviceUri;
this.input = input;
this.resultType = resultType;
+ this.errorHandler = handler;
}
@Override
@@ -129,6 +139,12 @@ public class GerritHttpClient {
return json.parseResponse(content, resultType);
}
+ @Override
+ public void handleError(HttpMethodBase method) throws GerritException {
+ if (errorHandler != null) {
+ errorHandler.handleError(method);
+ }
+ }
}
public static abstract class JsonEntity {
@@ -197,13 +213,13 @@ public class GerritHttpClient {
return execute(new JsonRequest(serviceUri, entity), monitor);
}
- public <T> T postRestRequest(final String serviceUri, final Object input, Type resultType, IProgressMonitor monitor)
- throws IOException, GerritException {
+ public <T> T postRestRequest(final String serviceUri, final Object input, Type resultType, ErrorHandler handler,
+ IProgressMonitor monitor) throws IOException, GerritException {
Assert.isNotNull(serviceUri, "REST Service URI must be not null."); //$NON-NLS-1$
Assert.isNotNull(input, "Input object must be not null."); //$NON-NLS-1$
Assert.isNotNull(resultType, "Output type must be not null."); //$NON-NLS-1$
- return execute(new RestRequest<T>(serviceUri, input, resultType), monitor);
+ return execute(new RestRequest<T>(serviceUri, input, resultType, handler), monitor);
}
public <T> T execute(Request<T> request, IProgressMonitor monitor) throws IOException, GerritException {
@@ -255,7 +271,11 @@ public class GerritHttpClient {
WebUtil.releaseConnection(method, monitor);
}
} else {
- WebUtil.releaseConnection(method, monitor);
+ try {
+ request.handleError(method);
+ } finally {
+ WebUtil.releaseConnection(method, monitor);
+ }
if (code == HttpURLConnection.HTTP_UNAUTHORIZED || code == HttpURLConnection.HTTP_FORBIDDEN) {
// login or re-authenticate due to an expired session
authenticate(openIdProvider, monitor);
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/SubmitInfo.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/SubmitInfo.java
new file mode 100644
index 0000000..5c5a57e
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/SubmitInfo.java
@@ -0,0 +1,29 @@
+/*******************************************************************************
+ * Copyright (c) 2013 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.internal.gerrit.core.client.rest;
+
+/**
+ * Data model object for <a
+ * href="https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#submit-info">SubmitInfo</a>.
+ */
+public class SubmitInfo {
+
+ public enum Status {
+ MERGED, SUBMITTED
+ }
+
+ private Status status;
+
+ public Status getStatus() {
+ return status;
+ }
+}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/SubmitInput.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/SubmitInput.java
new file mode 100644
index 0000000..7c4e42f
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/SubmitInput.java
@@ -0,0 +1,29 @@
+/*******************************************************************************
+ * Copyright (c) 2013 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.internal.gerrit.core.client.rest;
+
+/**
+ * Data model object for <a
+ * href="https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#submit-input">SubmitInput</a>.
+ */
+public class SubmitInput {
+
+ private final boolean wait_for_merge;
+
+ public SubmitInput(boolean waitForMerge) {
+ wait_for_merge = waitForMerge;
+ }
+
+ public boolean isWaitForMerge() {
+ return wait_for_merge;
+ }
+}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/operations/SubmitRequest.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/operations/SubmitRequest.java
index b2a2619..da7ff44 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/operations/SubmitRequest.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/operations/SubmitRequest.java
@@ -43,7 +43,7 @@ public class SubmitRequest extends AbstractRequest<ChangeDetail> {
@Override
protected ChangeDetail execute(GerritClient client, IProgressMonitor monitor) throws GerritException {
- return client.submit(getReviewId(), getPatchSetId(), getMessage(), monitor);
+ return client.submit(getReviewId(), getPatchSetId(), monitor);
}
}
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/SubmitInfoTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/SubmitInfoTest.java
new file mode 100644
index 0000000..73d901c
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/SubmitInfoTest.java
@@ -0,0 +1,38 @@
+/*******************************************************************************
+ * Copyright (c) 2013 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.gerrit.tests.core.client.rest;
+
+import java.io.File;
+import java.io.IOException;
+
+import junit.framework.TestCase;
+
+import org.eclipse.mylyn.commons.sdk.util.CommonTestUtil;
+import org.eclipse.mylyn.internal.gerrit.core.client.JSonSupport;
+import org.eclipse.mylyn.internal.gerrit.core.client.rest.SubmitInfo;
+import org.junit.Test;
+
+public class SubmitInfoTest extends TestCase {
+
+ @Test
+ public void testFromValid() throws IOException {
+ SubmitInfo submitInfo = parseFile("testdata/SubmitInfo_merged.json");
+
+ assertEquals(SubmitInfo.Status.MERGED, submitInfo.getStatus());
+ }
+
+ private SubmitInfo parseFile(String path) throws IOException {
+ File file = CommonTestUtil.getFile(this, path);
+ String content = CommonTestUtil.read(file);
+ return new JSonSupport().parseResponse(content, SubmitInfo.class);
+ }
+}
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/SubmitInputTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/SubmitInputTest.java
new file mode 100644
index 0000000..24cf25c
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/SubmitInputTest.java
@@ -0,0 +1,35 @@
+/*******************************************************************************
+ * Copyright (c) 2013 Tasktop Technologies and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.gerrit.tests.core.client.rest;
+
+import static org.eclipse.mylyn.commons.sdk.util.CommonTestUtil.getFile;
+import static org.eclipse.mylyn.commons.sdk.util.CommonTestUtil.read;
+import junit.framework.TestCase;
+
+import org.eclipse.mylyn.internal.gerrit.core.client.JSonSupport;
+import org.eclipse.mylyn.internal.gerrit.core.client.rest.SubmitInput;
+import org.junit.Test;
+
+public class SubmitInputTest extends TestCase {
+
+ @Test
+ public void testFromValid() throws Exception {
+ SubmitInput submitInput = new SubmitInput(true);
+ assertTrue(submitInput.isWaitForMerge());
+
+ String json = new JSonSupport().getGson().toJson(submitInput);
+
+ assertNotNull(json);
+ assertFalse(json.isEmpty());
+ assertEquals(read(getFile(this, "testdata/SubmitInput_waitForMerge.json")), json);
+ }
+}
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java
index 5c704ed..cb58f48 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java
@@ -17,6 +17,7 @@ import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.sameInstance;
+import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertThat;
import java.util.Arrays;
@@ -46,7 +47,6 @@ import org.eclipse.mylyn.reviews.core.model.IUser;
import org.eclipse.mylyn.reviews.core.model.RequirementStatus;
import org.eclipse.mylyn.reviews.core.model.ReviewStatus;
import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfConsumer;
-import org.eclipse.osgi.util.NLS;
import org.junit.Test;
import com.google.gerrit.common.data.ChangeDetail;
@@ -258,13 +258,11 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
}
public void testCannotSubmitChange() throws Exception {
- String message1 = "submit, time: " + System.currentTimeMillis(); //$NON-NLS-1$
try {
- reviewHarness.client.submit(reviewHarness.shortId, 1, message1, new NullProgressMonitor());
+ reviewHarness.client.submit(reviewHarness.shortId, 1, new NullProgressMonitor());
fail("Expected to fail when submitting a change without approvals");
} catch (GerritException e) {
- assertThat(e.getMessage(), is(NLS.bind(
- "Cannot submit change {0}: needs Verified; change {0}: needs Code-Review", reviewHarness.shortId)));
+ assertThat(e.getMessage(), startsWith("Cannot submit change"));
}
}
diff --git a/org.eclipse.mylyn.gerrit.tests/testdata/SubmitInfo_merged.json b/org.eclipse.mylyn.gerrit.tests/testdata/SubmitInfo_merged.json
new file mode 100644
index 0000000..f1fc0b7
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/testdata/SubmitInfo_merged.json
@@ -0,0 +1,4 @@
+)]}'
+{
+ "status": "MERGED"
+} \ No newline at end of file
diff --git a/org.eclipse.mylyn.gerrit.tests/testdata/SubmitInput_waitForMerge.json b/org.eclipse.mylyn.gerrit.tests/testdata/SubmitInput_waitForMerge.json
new file mode 100644
index 0000000..0d3f795
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/testdata/SubmitInput_waitForMerge.json
@@ -0,0 +1 @@
+{"wait_for_merge":true} \ No newline at end of file