summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTomasz Zarna2013-07-19 13:56:09 (EDT)
committer Tomasz Zarna2013-07-30 06:38:38 (EDT)
commiteaedac4a935d9e58aad072affd2d42d99517adbd (patch)
tree8dfce6bfbdf43b1a343f9fa1e397ed197ee9f646
parent32fcdb827173d44a0ba16522e22eeee39522c28c (diff)
downloadorg.eclipse.mylyn.reviews-eaedac4a935d9e58aad072affd2d42d99517adbd.zip
org.eclipse.mylyn.reviews-eaedac4a935d9e58aad072affd2d42d99517adbd.tar.gz
org.eclipse.mylyn.reviews-eaedac4a935d9e58aad072affd2d42d99517adbd.tar.bz2
395059: fix adding reviewers in Gerrit 2.6refs/changes/14/14714/7
Bug: 395059 Change-Id: Ib4a021ebc82bb12f9d075ae5d6b0a8468a3ea67a 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.java62
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/AddReviewerResult.java35
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ApprovalUtil.java9
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeInfo.java11
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ReviewerInfo.java57
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ReviewerInput.java33
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/AllGerritTests.java6
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/AddReviewerResultTest.java81
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ReviewerInfoTest.java103
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ReviewerInputTest.java60
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java92
-rw-r--r--org.eclipse.mylyn.gerrit.tests/testdata/AddReviewerResult_reviewers.json26
-rw-r--r--org.eclipse.mylyn.gerrit.tests/testdata/ReviewerInfo_JohnDoePlusTwo.json11
-rw-r--r--org.eclipse.mylyn.gerrit.tests/testdata/ReviewerInput_johnDoe.json1
14 files changed, 556 insertions, 31 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 10b2b05..a973b17 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
@@ -24,6 +24,7 @@ import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -34,6 +35,7 @@ import java.util.regex.Pattern;
import org.apache.commons.httpclient.Cookie;
import org.apache.commons.httpclient.HttpMethodBase;
import org.apache.commons.httpclient.methods.GetMethod;
+import org.eclipse.core.runtime.Assert;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.OperationCanceledException;
import org.eclipse.mylyn.commons.net.AbstractWebLocation;
@@ -53,12 +55,15 @@ import org.eclipse.mylyn.internal.gerrit.core.client.compat.ProjectAdminService;
import org.eclipse.mylyn.internal.gerrit.core.client.compat.ProjectDetailX;
import org.eclipse.mylyn.internal.gerrit.core.client.data.GerritQueryResult;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.AbandonInput;
+import org.eclipse.mylyn.internal.gerrit.core.client.rest.AddReviewerResult;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.ChangeInfo;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.CommentInfo;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.CommentInput;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.RestoreInput;
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.ReviewerInfo;
+import org.eclipse.mylyn.internal.gerrit.core.client.rest.ReviewerInput;
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;
@@ -72,6 +77,7 @@ import org.osgi.framework.Version;
import com.google.gerrit.common.data.AccountDashboardInfo;
import com.google.gerrit.common.data.AccountService;
+import com.google.gerrit.common.data.ApprovalDetail;
import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.common.data.ChangeListService;
import com.google.gerrit.common.data.GerritConfig;
@@ -546,27 +552,55 @@ public class GerritClient extends ReviewsClient {
public ReviewerResult addReviewers(String reviewId, final List<String> reviewers, IProgressMonitor monitor)
throws GerritException {
+ Assert.isLegal(reviewers != null, "reviewers cannot be null"); //$NON-NLS-1$
final Change.Id id = new Change.Id(id(reviewId));
- try {
- return execute(monitor, new Operation<ReviewerResult>() {
- @Override
- public void execute(IProgressMonitor monitor) throws GerritException {
- getPatchDetailService(monitor).addReviewers(id, reviewers, this);
- }
- });
- } catch (GerritException e) {
- // Gerrit 2.2
- String message = e.getMessage();
- if (message != null && message.contains("Error parsing request")) { //$NON-NLS-1$
+ if (hasJsonRpcApi(monitor)) {
+ try {
return execute(monitor, new Operation<ReviewerResult>() {
@Override
public void execute(IProgressMonitor monitor) throws GerritException {
- getPatchDetailService(monitor).addReviewers(id, reviewers, false, this);
+ getPatchDetailService(monitor).addReviewers(id, reviewers, this);
}
});
- } else {
- throw e;
+ } catch (GerritException e) {
+ // Gerrit 2.2
+ String message = e.getMessage();
+ if (message != null && message.contains("Error parsing request")) { //$NON-NLS-1$
+ return execute(monitor, new Operation<ReviewerResult>() {
+ @Override
+ public void execute(IProgressMonitor monitor) throws GerritException {
+ getPatchDetailService(monitor).addReviewers(id, reviewers, false, this);
+ }
+ });
+ } else {
+ throw e;
+ }
+ }
+ } else {
+ final String uri = "/a/changes/" + id.get() + "/reviewers"; //$NON-NLS-1$ //$NON-NLS-2$
+ Set<ReviewerInfo> reviewerInfos = new HashSet<ReviewerInfo>(reviewers.size());
+ ReviewerResult reviewerResult = new ReviewerResult();
+ for (final String reviewerId : reviewers) {
+ try {
+ AddReviewerResult addReviewerResult = executePostRestRequest(uri, new ReviewerInput(reviewerId),
+ AddReviewerResult.class, null /*no error handler*/, monitor);
+ reviewerInfos.addAll(addReviewerResult.getReviewers());
+ } catch (GerritHttpException e) {
+ if (e.getResponseCode() == 422 /*Unprocessable Entity*/) {
+ reviewerResult.addError(new ReviewerResult.Error(null /* no type*/, reviewerId));
+ }
+ }
+ }
+
+ ChangeDetail changeDetail = getChangeDetail(id.get(), monitor);
+
+ List<ApprovalDetail> approvalDetails = new ArrayList<ApprovalDetail>(reviewerInfos.size());
+ for (ReviewerInfo reviewerInfo : reviewerInfos) {
+ approvalDetails.add(reviewerInfo.toApprovalDetail(changeDetail.getCurrentPatchSet()));
}
+ changeDetail.setApprovals(approvalDetails);
+ reviewerResult.setChange(changeDetail);
+ return reviewerResult;
}
}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/AddReviewerResult.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/AddReviewerResult.java
new file mode 100644
index 0000000..e44f1d8
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/AddReviewerResult.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.internal.gerrit.core.client.rest;
+
+import java.util.List;
+
+/**
+ * Data model object for <a
+ * href="https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#add-reviewer-result"
+ * >AddReviewerResult</a>.
+ */
+public class AddReviewerResult {
+
+ private List<ReviewerInfo> reviewers;
+
+ private String error;
+
+ public List<ReviewerInfo> getReviewers() {
+ return reviewers;
+ }
+
+ public String getError() {
+ return error;
+ }
+
+}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ApprovalUtil.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ApprovalUtil.java
index b34433d..9db7ebf 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ApprovalUtil.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ApprovalUtil.java
@@ -98,6 +98,15 @@ public final class ApprovalUtil {
return null;
}
+ public static short parseShort(String s) {
+ s = s.trim();
+ // only Java7 handles a plus sign as indication of a positive value
+ if (s.startsWith("+")) { //$NON-NLS-1$
+ s = s.substring(1);
+ }
+ return Short.parseShort(s);
+ }
+
private ApprovalUtil() {
}
}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeInfo.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeInfo.java
index d4eabaf..47935f6 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeInfo.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeInfo.java
@@ -181,7 +181,7 @@ public class ChangeInfo {
List<ApprovalCategoryValue> valueList = new ArrayList<ApprovalCategoryValue>();
for (Entry<String, String> valueEntry : entry.getValue().getValues().entrySet()) {
valueList.add(new ApprovalCategoryValue(new ApprovalCategoryValue.Id(approvalCategoryId,
- parseShort(valueEntry.getKey())), valueEntry.getValue()));
+ ApprovalUtil.parseShort(valueEntry.getKey())), valueEntry.getValue()));
}
ApprovalType approvalType = new ApprovalType(approvalCategory, valueList);
result.add(approvalType);
@@ -189,13 +189,4 @@ public class ChangeInfo {
return result;
}
- private static short parseShort(String s) {
- s = s.trim();
- // only Java7 handles a plus sign as indication of a positive value
- if (s.startsWith("+")) { //$NON-NLS-1$
- s = s.substring(1);
- }
- return Short.parseShort(s);
- }
-
}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ReviewerInfo.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ReviewerInfo.java
new file mode 100644
index 0000000..2a808df
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ReviewerInfo.java
@@ -0,0 +1,57 @@
+/*******************************************************************************
+ * 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;
+
+import java.util.Map;
+import java.util.Map.Entry;
+
+import com.google.gerrit.common.data.ApprovalDetail;
+import com.google.gerrit.reviewdb.Account;
+import com.google.gerrit.reviewdb.ApprovalCategory;
+import com.google.gerrit.reviewdb.PatchSet;
+import com.google.gerrit.reviewdb.PatchSetApproval;
+
+/**
+ * Data model object for <a
+ * href="https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#reviewer-info">ReviewerInfo</a>.
+ */
+public class ReviewerInfo extends AccountInfo {
+
+ private final String kind = "gerritcodereview#reviewer"; //$NON-NLS-1$
+
+ private Map<String/*label name*/, String/*approval value*/> approvals;
+
+ public String getKind() {
+ return kind;
+ }
+
+ public Map<String, String> getApprovals() {
+ return approvals;
+ }
+
+ public ApprovalDetail toApprovalDetail(PatchSet currentPatchSet) {
+ Account.Id accountId = new Account.Id(getId());
+ ApprovalDetail approvalDetail = new ApprovalDetail(accountId);
+ Map<String, String> map = getApprovals();
+ if (map != null) {
+ for (Entry<String, String> entry : map.entrySet()) {
+ ApprovalCategory.Id categoryId = ApprovalUtil.findCategoryIdByName(entry.getKey().replace('-', ' '));
+ if (categoryId != null) {
+ PatchSetApproval patchSetApproval = new PatchSetApproval(new PatchSetApproval.Key(
+ currentPatchSet.getId(), accountId, categoryId), ApprovalUtil.parseShort(entry.getValue()));
+ approvalDetail.add(patchSetApproval);
+ }
+ }
+ }
+ return approvalDetail;
+ }
+}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ReviewerInput.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ReviewerInput.java
new file mode 100644
index 0000000..bb4ed19
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ReviewerInput.java
@@ -0,0 +1,33 @@
+/*******************************************************************************
+ * 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;
+
+import org.eclipse.core.runtime.Assert;
+
+/**
+ * Data model object for <a
+ * href="https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#reviewer-input">ReviewerInput</a>.
+ */
+public class ReviewerInput {
+
+ private final String reviewer;
+
+ public ReviewerInput(String reviewer) {
+ Assert.isLegal(reviewer != null);
+ Assert.isLegal(!reviewer.isEmpty());
+ this.reviewer = reviewer;
+ }
+
+ public String getReviewer() {
+ return reviewer;
+ }
+}
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/AllGerritTests.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/AllGerritTests.java
index 0b657b5..35a278c 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/AllGerritTests.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/AllGerritTests.java
@@ -25,10 +25,13 @@ import org.eclipse.mylyn.gerrit.tests.core.client.GerritVersionTest;
import org.eclipse.mylyn.gerrit.tests.core.client.OpenIdAuthenticationTest;
import org.eclipse.mylyn.gerrit.tests.core.client.rest.AbandonInputTest;
import org.eclipse.mylyn.gerrit.tests.core.client.rest.AccountInfoTest;
+import org.eclipse.mylyn.gerrit.tests.core.client.rest.AddReviewerResultTest;
import org.eclipse.mylyn.gerrit.tests.core.client.rest.ChangeInfoTest;
import org.eclipse.mylyn.gerrit.tests.core.client.rest.RestoreInputTest;
import org.eclipse.mylyn.gerrit.tests.core.client.rest.ReviewInfoTest;
import org.eclipse.mylyn.gerrit.tests.core.client.rest.ReviewInputTest;
+import org.eclipse.mylyn.gerrit.tests.core.client.rest.ReviewerInfoTest;
+import org.eclipse.mylyn.gerrit.tests.core.client.rest.ReviewerInputTest;
import org.eclipse.mylyn.gerrit.tests.core.client.rest.SubmitInfoTest;
import org.eclipse.mylyn.gerrit.tests.core.client.rest.SubmitInputTest;
import org.eclipse.mylyn.gerrit.tests.support.GerritFixture;
@@ -58,8 +61,11 @@ public class AllGerritTests {
suite.addTestSuite(GerritVersionTest.class);
suite.addTestSuite(AbandonInputTest.class);
suite.addTestSuite(AccountInfoTest.class);
+ suite.addTestSuite(AddReviewerResultTest.class);
suite.addTestSuite(ChangeInfoTest.class);
suite.addTestSuite(RestoreInputTest.class);
+ suite.addTestSuite(ReviewerInfoTest.class);
+ suite.addTestSuite(ReviewerInputTest.class);
suite.addTestSuite(ReviewInfoTest.class);
suite.addTestSuite(ReviewInputTest.class);
suite.addTestSuite(SubmitInfoTest.class);
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/AddReviewerResultTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/AddReviewerResultTest.java
new file mode 100644
index 0000000..f23aa28
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/AddReviewerResultTest.java
@@ -0,0 +1,81 @@
+/*******************************************************************************
+ * 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 java.util.List;
+
+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.AddReviewerResult;
+import org.eclipse.mylyn.internal.gerrit.core.client.rest.ReviewerInfo;
+import org.junit.Test;
+
+public class AddReviewerResultTest extends TestCase {
+ @Test
+ public void testFromEmptyJson() throws Exception {
+ AddReviewerResult addReviewerResult = parseFile("testdata/EmptyWithMagic.json");
+
+ assertNotNull(addReviewerResult);
+ assertNull(addReviewerResult.getError());
+ assertNull(addReviewerResult.getReviewers());
+ }
+
+ @Test
+ public void testFromInvalid() throws Exception {
+ AddReviewerResult addReviewerResult = parseFile("testdata/InvalidWithMagic.json");
+
+ assertNotNull(addReviewerResult);
+ assertNull(addReviewerResult.getError());
+ assertNull(addReviewerResult.getReviewers());
+ }
+
+ @Test
+ public void testFromValid() throws Exception {
+ AddReviewerResult addReviewerResult = parseFile("testdata/AddReviewerResult_reviewers.json");
+
+ assertNotNull(addReviewerResult);
+ assertNull(addReviewerResult.getError());
+ List<ReviewerInfo> reviewers = addReviewerResult.getReviewers();
+ assertNotNull(reviewers);
+ assertEquals(2, reviewers.size());
+ ReviewerInfo johnDoe = reviewers.get(0);
+ ReviewerInfo janeRoe = reviewers.get(1);
+ assertAccountInfo(johnDoe, 1000096, "John Doe", "john.doe@example.com");
+ assertAccountInfo(janeRoe, 1000097, "Jane Roe", "jane.roe@example.com");
+ assertApprovals(johnDoe, "+1", "+2");
+ assertApprovals(janeRoe, " 0", "-1");
+ }
+
+ private static void assertAccountInfo(ReviewerInfo reviewerInfo, int id, String name, String email) {
+ assertEquals(id, reviewerInfo.getId());
+ assertEquals(name, reviewerInfo.getName());
+ assertEquals(email, reviewerInfo.getEmail());
+ assertNull(reviewerInfo.getUsername());
+ }
+
+ private static void assertApprovals(ReviewerInfo reviewerInfo, String verified, String codeReview) {
+ assertNotNull(reviewerInfo.getApprovals());
+ assertEquals(2, reviewerInfo.getApprovals().size());
+ assertEquals(verified, reviewerInfo.getApprovals().get("Verified"));
+ assertEquals(codeReview, reviewerInfo.getApprovals().get("Code-Review"));
+ }
+
+ private AddReviewerResult parseFile(String path) throws IOException {
+ File file = CommonTestUtil.getFile(this, path);
+ String content = CommonTestUtil.read(file);
+ return new JSonSupport().parseResponse(content, AddReviewerResult.class);
+ }
+}
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ReviewerInfoTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ReviewerInfoTest.java
new file mode 100644
index 0000000..df46145
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ReviewerInfoTest.java
@@ -0,0 +1,103 @@
+/*******************************************************************************
+ * 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 java.util.Map;
+
+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.ApprovalUtil;
+import org.eclipse.mylyn.internal.gerrit.core.client.rest.ReviewerInfo;
+import org.junit.Test;
+
+import com.google.gerrit.common.data.ApprovalDetail;
+import com.google.gerrit.reviewdb.Account;
+import com.google.gerrit.reviewdb.ApprovalCategory;
+import com.google.gerrit.reviewdb.Change;
+import com.google.gerrit.reviewdb.PatchSet;
+import com.google.gerrit.reviewdb.PatchSetApproval;
+
+public class ReviewerInfoTest extends TestCase {
+ @Test
+ public void testFromEmptyJson() throws Exception {
+ ReviewerInfo reviewerInfo = parseFile("testdata/EmptyWithMagic.json");
+
+ assertNotNull(reviewerInfo);
+ // always available
+ assertEquals("gerritcodereview#reviewer", reviewerInfo.getKind());
+ assertNull(reviewerInfo.getApprovals());
+ // from AccountInfo
+ assertEquals(-1, reviewerInfo.getId());
+ assertNull(reviewerInfo.getName());
+ assertNull(reviewerInfo.getEmail());
+ assertNull(reviewerInfo.getUsername());
+ }
+
+ @Test
+ public void testFromInvalid() throws Exception {
+ ReviewerInfo reviewerInfo = parseFile("testdata/InvalidWithMagic.json");
+
+ assertNotNull(reviewerInfo);
+ // always available
+ assertEquals("gerritcodereview#reviewer", reviewerInfo.getKind());
+ assertNull(reviewerInfo.getApprovals());
+ // from AccountInfo
+ assertEquals(-1, reviewerInfo.getId());
+ assertNull(reviewerInfo.getName());
+ assertNull(reviewerInfo.getEmail());
+ assertNull(reviewerInfo.getUsername());
+ }
+
+ @Test
+ public void testFromValid() throws IOException {
+ ReviewerInfo reviewerInfo = parseFile("testdata/ReviewerInfo_JohnDoePlusTwo.json");
+
+ assertEquals("gerritcodereview#reviewer", reviewerInfo.getKind());
+ Map<String, String> approvals = reviewerInfo.getApprovals();
+ assertNotNull(approvals);
+ assertFalse(approvals.isEmpty());
+ assertEquals(2, approvals.size());
+ assertEquals("+1", approvals.get("Verified"));
+ assertEquals("+2", approvals.get("Code-Review"));
+ // from AccountInfo
+ assertEquals(1000096, reviewerInfo.getId());
+ assertEquals("John Doe", reviewerInfo.getName());
+ assertEquals("john.doe@example.com", reviewerInfo.getEmail());
+ assertNull(reviewerInfo.getUsername());
+
+ PatchSet dummyPatchSet = new PatchSet(new PatchSet.Id(new Change.Id(1), 1));
+ ApprovalDetail approvalDetail = reviewerInfo.toApprovalDetail(dummyPatchSet);
+ assertNotNull(approvalDetail);
+ assertEquals(new Account.Id(1000096), approvalDetail.getAccount());
+ Map<ApprovalCategory.Id, PatchSetApproval> approvalMap = approvalDetail.getApprovalMap();
+ assertNotNull(approvalMap);
+ assertEquals(2, approvalMap.size());
+ PatchSetApproval crvw = approvalMap.get(ApprovalUtil.CRVW.getCategory().getId());
+ assertNotNull(crvw);
+ assertEquals(1000096, crvw.getAccountId().get());
+ assertEquals(2, crvw.getValue());
+ PatchSetApproval vrif = approvalMap.get(ApprovalUtil.VRIF.getCategory().getId());
+ assertNotNull(vrif);
+ assertEquals(1000096, vrif.getAccountId().get());
+ assertEquals(1, vrif.getValue());
+ }
+
+ private ReviewerInfo parseFile(String path) throws IOException {
+ File file = CommonTestUtil.getFile(this, path);
+ String content = CommonTestUtil.read(file);
+ return new JSonSupport().parseResponse(content, ReviewerInfo.class);
+ }
+}
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ReviewerInputTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ReviewerInputTest.java
new file mode 100644
index 0000000..fa38f72
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/gerrit/tests/core/client/rest/ReviewerInputTest.java
@@ -0,0 +1,60 @@
+/*******************************************************************************
+ * 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.ReviewerInput;
+import org.junit.Test;
+
+public class ReviewerInputTest extends TestCase {
+ @Test
+ public void testFromNull() throws Exception {
+ try {
+ new ReviewerInput(null);
+ fail("Expected IllegalArgumentException");
+ } catch (IllegalArgumentException e) {
+ // expected
+ }
+ }
+
+ @Test
+ public void testFromEmpty() throws Exception {
+ try {
+ new ReviewerInput("");
+ fail("Expected IllegalArgumentException");
+ } catch (IllegalArgumentException e) {
+ // expected
+ }
+ }
+
+ @Test
+ public void testFromValid() throws Exception {
+ ReviewerInput reviewerInput = new ReviewerInput("john.doe@example.com");
+
+ String json = new JSonSupport().getGson().toJson(reviewerInput);
+
+ assertNotNull(json);
+ assertFalse(json.isEmpty());
+ assertEquals(readFile("testdata/ReviewerInput_johnDoe.json"), json);
+ }
+
+ private String readFile(String path) throws IOException {
+ File file = CommonTestUtil.getFile(this, path);
+ return CommonTestUtil.read(file);
+ }
+}
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 01a1c15..53fecc5 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
@@ -36,11 +36,13 @@ import org.apache.commons.lang.StringUtils;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jgit.api.CommitCommand;
import org.eclipse.mylyn.gerrit.tests.core.client.rest.ChangeInfoTest;
+import org.eclipse.mylyn.gerrit.tests.support.GerritFixture;
import org.eclipse.mylyn.gerrit.tests.support.GerritProject.CommitResult;
import org.eclipse.mylyn.internal.gerrit.core.client.GerritChange;
import org.eclipse.mylyn.internal.gerrit.core.client.GerritException;
import org.eclipse.mylyn.internal.gerrit.core.client.GerritVersion;
import org.eclipse.mylyn.internal.gerrit.core.client.compat.ChangeDetailX;
+import org.eclipse.mylyn.internal.gerrit.core.client.rest.ApprovalUtil;
import org.eclipse.mylyn.internal.gerrit.core.client.rest.ChangeInfo;
import org.eclipse.mylyn.reviews.core.model.IApprovalType;
import org.eclipse.mylyn.reviews.core.model.IChange;
@@ -55,12 +57,16 @@ 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.junit.Test;
+import org.osgi.framework.Version;
+import com.google.gerrit.common.data.ApprovalDetail;
import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.common.data.ReviewerResult;
+import com.google.gerrit.reviewdb.ApprovalCategory;
import com.google.gerrit.reviewdb.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.Change.Status;
import com.google.gerrit.reviewdb.ChangeMessage;
+import com.google.gerrit.reviewdb.PatchSetApproval;
/**
* @author Miles Parker
@@ -278,8 +284,8 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
try {
reviewHarness.client.addReviewers(reviewHarness.shortId, null, new NullProgressMonitor());
fail("Expected to fail when trying to add null reviewers");
- } catch (GerritException e) {
- assertThat(e.getMessage(), is("Internal Server Error"));
+ } catch (IllegalArgumentException e) {
+ assertThat(e.getMessage(), is("reviewers cannot be null"));
}
}
@@ -287,42 +293,114 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
public void testAddEmptyReviewers() throws Exception {
ReviewerResult reviewerResult = reviewHarness.client.addReviewers(reviewHarness.shortId,
Collections.<String> emptyList(), new NullProgressMonitor());
+ reviewHarness.consumer.retrieve(false);
+ reviewHarness.listener.waitForResponse(2, 2);
+
+ assertThat(reviewerResult, notNullValue());
assertThat(reviewerResult.getErrors().isEmpty(), is(true));
+ assertThat(reviewerResult.getChange().getApprovals().isEmpty(), is(true));
+ assertThat(getReview().getReviewerApprovals().isEmpty(), is(true));
}
@Test
public void testAddInvalidReviewers() throws Exception {
List<String> reviewers = Arrays.asList(new String[] { "foo" });
+
ReviewerResult reviewerResult = reviewHarness.client.addReviewers(reviewHarness.shortId, reviewers,
new NullProgressMonitor());
+ reviewHarness.consumer.retrieve(false);
+ reviewHarness.listener.waitForResponse(2, 2);
+
+ assertThat(reviewerResult, notNullValue());
assertThat(reviewerResult.getErrors().size(), is(1));
assertThat(reviewerResult.getErrors().get(0).getName(), is("foo"));
+ assertThat(reviewerResult.getErrors().get(0).getType(), nullValue());
+ assertThat(reviewerResult.getChange().getApprovals().isEmpty(), is(true));
+ assertThat(getReview().getReviewerApprovals().isEmpty(), is(true));
}
@Test
public void testAddSomeInvalidReviewers() throws Exception {
List<String> reviewers = Arrays.asList(new String[] { "tests", "foo" });
+
ReviewerResult reviewerResult = reviewHarness.client.addReviewers(reviewHarness.shortId, reviewers,
new NullProgressMonitor());
- assertThat(reviewerResult.getErrors().isEmpty(), is(false));
- assertThat(reviewerResult.getErrors().size(), is(1));
- assertThat(reviewerResult.getErrors().get(0).getName(), is("foo"));
+ reviewHarness.consumer.retrieve(false);
+ reviewHarness.listener.waitForResponse(2, 2);
+
+ assertReviewerResult(reviewerResult, "foo");
}
@Test
public void testAddReviewers() throws Exception {
+ assertThat(getReview().getReviewerApprovals().isEmpty(), is(true));
List<String> reviewers = Arrays.asList(new String[] { "tests" });
+
ReviewerResult reviewerResult = reviewHarness.client.addReviewers(reviewHarness.shortId, reviewers,
new NullProgressMonitor());
- assertThat(reviewerResult.getErrors().isEmpty(), is(true));
+ reviewHarness.consumer.retrieve(false);
+ reviewHarness.listener.waitForResponse(2, 2);
+
+ assertReviewerResult(reviewerResult, null);
}
@Test
public void testAddReviewersByEmail() throws Exception {
List<String> reviewers = Arrays.asList(new String[] { "tests@mylyn.eclipse.org" });
+
ReviewerResult reviewerResult = reviewHarness.client.addReviewers(reviewHarness.shortId, reviewers,
new NullProgressMonitor());
- assertThat(reviewerResult.getErrors().isEmpty(), is(true));
+ reviewHarness.consumer.retrieve(false);
+ reviewHarness.listener.waitForResponse(2, 2);
+
+ assertReviewerResult(reviewerResult, null);
+ }
+
+ private void assertReviewerResult(ReviewerResult reviewerResult, String nameInErrors) {
+ assertThat(reviewerResult, notNullValue());
+
+ assertThat(reviewerResult.getErrors().isEmpty(), is(nameInErrors == null));
+ if (nameInErrors != null) {
+ assertThat(reviewerResult.getErrors().size(), is(1));
+ assertThat(reviewerResult.getErrors().get(0).getName(), is(nameInErrors));
+ assertThat(reviewerResult.getErrors().get(0).getType(), nullValue());
+ }
+
+ List<ApprovalDetail> approvals = reviewerResult.getChange().getApprovals();
+ assertThat(approvals.isEmpty(), is(false));
+ assertThat(approvals.size(), is(1));
+ assertThat(approvals.get(0).getAccount().get(), is(1000001));
+
+ Map<ApprovalCategory.Id, PatchSetApproval> approvalMap = approvals.get(0).getApprovalMap();
+ assertThat(approvalMap, notNullValue());
+ assertThat(approvalMap.isEmpty(), is(false));
+ assertThat(approvalMap.size(), is(1));
+
+ PatchSetApproval crvw = approvalMap.get(ApprovalUtil.CRVW.getCategory().getId());
+ assertThat(crvw, notNullValue());
+ assertThat(crvw.getAccountId().get(), is(1000001));
+ assertThat(crvw.getValue(), is((short) 0));
+ assertThat(crvw.getGranted(), notNullValue());
+ assertThat(crvw.getPatchSetId(), notNullValue());
+ assertThat(crvw.getPatchSetId().get(), is(1));
+ assertThat(crvw.getPatchSetId().getParentKey().get(), is(Integer.parseInt(getReview().getId())));
+
+ // TODO: empty map vs map with null key
+ if (GerritVersion.isVersion26OrLater(getCurrentVersion())) {
+ assertThat(getReview().getReviewerApprovals().isEmpty(), is(true));
+ } else {
+ assertThat(getReview().getReviewerApprovals().isEmpty(), is(false));
+ assertThat(getReview().getReviewerApprovals().size(), is(1));
+ assertThat(getReview().getReviewerApprovals().get(0), nullValue());
+ }
+ }
+
+ private static Version getCurrentVersion() {
+ String version = GerritFixture.current().getSimpleInfo();
+ if (version.indexOf('/') != -1) {
+ version = version.substring(0, version.indexOf('/'));
+ }
+ return GerritVersion.parseGerritVersion(version);
}
@Test
diff --git a/org.eclipse.mylyn.gerrit.tests/testdata/AddReviewerResult_reviewers.json b/org.eclipse.mylyn.gerrit.tests/testdata/AddReviewerResult_reviewers.json
new file mode 100644
index 0000000..10a44ad
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/testdata/AddReviewerResult_reviewers.json
@@ -0,0 +1,26 @@
+)]}'
+{
+ "reviewers" :
+ [
+ {
+ "kind": "gerritcodereview#reviewer",
+ "approvals": {
+ "Verified": "+1",
+ "Code-Review": "+2"
+ },
+ "_account_id": 1000096,
+ "name": "John Doe",
+ "email": "john.doe@example.com"
+ },
+ {
+ "kind": "gerritcodereview#reviewer",
+ "approvals": {
+ "Verified": " 0",
+ "Code-Review": "-1"
+ },
+ "_account_id": 1000097,
+ "name": "Jane Roe",
+ "email": "jane.roe@example.com"
+ }
+ ]
+} \ No newline at end of file
diff --git a/org.eclipse.mylyn.gerrit.tests/testdata/ReviewerInfo_JohnDoePlusTwo.json b/org.eclipse.mylyn.gerrit.tests/testdata/ReviewerInfo_JohnDoePlusTwo.json
new file mode 100644
index 0000000..c552923
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/testdata/ReviewerInfo_JohnDoePlusTwo.json
@@ -0,0 +1,11 @@
+)]}'
+{
+ "kind": "gerritcodereview#reviewer",
+ "approvals": {
+ "Verified": "+1",
+ "Code-Review": "+2"
+ },
+ "_account_id": 1000096,
+ "name": "John Doe",
+ "email": "john.doe@example.com"
+}
diff --git a/org.eclipse.mylyn.gerrit.tests/testdata/ReviewerInput_johnDoe.json b/org.eclipse.mylyn.gerrit.tests/testdata/ReviewerInput_johnDoe.json
new file mode 100644
index 0000000..228c747
--- /dev/null
+++ b/org.eclipse.mylyn.gerrit.tests/testdata/ReviewerInput_johnDoe.json
@@ -0,0 +1 @@
+{"reviewer":"john.doe@example.com"} \ No newline at end of file