diff options
author | Tomasz Zarna | 2013-07-23 14:30:43 +0000 |
---|---|---|
committer | Tomasz Zarna | 2013-07-26 19:24:50 +0000 |
commit | 433e5de744f11a0657eb5dd59f3de922eb617ab9 (patch) | |
tree | 2fb1aacb05c86ad9dd0e39fc5e0a59c9a3e4badb | |
parent | 31649c82814e6a54fa8159b40ffd81427d5687b0 (diff) | |
download | org.eclipse.mylyn.reviews-433e5de744f11a0657eb5dd59f3de922eb617ab9.tar.gz org.eclipse.mylyn.reviews-433e5de744f11a0657eb5dd59f3de922eb617ab9.tar.xz org.eclipse.mylyn.reviews-433e5de744f11a0657eb5dd59f3de922eb617ab9.zip |
395059: allow to submit a change
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>
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 fc566a53a..935f7b8f8 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 9f3575e97..f64555402 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 000000000..5c5a57e09 --- /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 000000000..7c4e42fb3 --- /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 b2a2619ec..da7ff4423 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 000000000..73d901c3e --- /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 000000000..24cf25cbf --- /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 5c704edde..cb58f4875 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 000000000..f1fc0b720 --- /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 000000000..0d3f79537 --- /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 |