summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTomasz Zarna2013-07-30 08:42:54 (EDT)
committerTomasz Zarna2013-07-30 09:20:40 (EDT)
commitd1e4915a4bdd3ff69be0204bbfb8186c6bcc8d75 (patch)
tree9bc38482d804492bf330ad6ec125fb18cf45936a
parentbc7f37b72de00c7063556ce7285919c77ec59850 (diff)
downloadorg.eclipse.mylyn.reviews-d1e4915a4bdd3ff69be0204bbfb8186c6bcc8d75.zip
org.eclipse.mylyn.reviews-d1e4915a4bdd3ff69be0204bbfb8186c6bcc8d75.tar.gz
org.eclipse.mylyn.reviews-d1e4915a4bdd3ff69be0204bbfb8186c6bcc8d75.tar.bz2
395059: abandoned change doesn't provide info about approval typesrefs/changes/61/14961/2
Bug: 395059 Change-Id: I42e04e4574025a5faec7a2ee4b7d442977377aac 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/rest/ApprovalUtil.java13
-rw-r--r--org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactory.java29
-rw-r--r--org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactoryTest.java11
3 files changed, 25 insertions, 28 deletions
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 f82d576..2c92b5e 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
@@ -84,8 +84,7 @@ public final class ApprovalUtil {
BY_ID.put(IPCL.getCategory().getId().get(), IPCL);
}
- static ApprovalCategory findCategoryByName(String name) {
- name = name.replace('-', ' ');
+ private static ApprovalCategory findCategoryByName(String name) {
if (BY_NAME.containsKey(name)) {
return BY_NAME.get(name).getCategory();
}
@@ -93,8 +92,7 @@ public final class ApprovalUtil {
}
static ApprovalCategory findCategoryByNameWithDash(String name) {
- name = name.replace('-', ' ');
- return findCategoryByName(name);
+ return findCategoryByName(name.replace('-', ' '));
}
static ApprovalCategory.Id findCategoryIdByName(String name) {
@@ -106,11 +104,10 @@ public final class ApprovalUtil {
}
static ApprovalCategory.Id findCategoryIdByNameWithDash(String name) {
- name = name.replace('-', ' ');
- return findCategoryIdByName(name);
+ return findCategoryIdByName(name.replace('-', ' '));
}
- public static String findCategoryNameById(String id) {
+ static String findCategoryNameById(String id) {
if (BY_ID.containsKey(id)) {
return BY_ID.get(id).getCategory().getName();
}
@@ -121,7 +118,7 @@ public final class ApprovalUtil {
return name.replace(' ', '-');
}
- public static short parseShort(String s) {
+ 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$
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactory.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactory.java
index 7952bb5..e591f9c 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactory.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactory.java
@@ -55,6 +55,7 @@ import com.google.gerrit.common.data.ChangeInfo;
import com.google.gerrit.common.data.PatchSetDetail;
import com.google.gerrit.reviewdb.Account;
import com.google.gerrit.reviewdb.Change;
+import com.google.gerrit.reviewdb.Change.Status;
import com.google.gerrit.reviewdb.ChangeMessage;
import com.google.gerrit.reviewdb.PatchSetApproval;
import com.google.gerrit.reviewdb.UserIdentity;
@@ -134,7 +135,7 @@ public class GerritReviewRemoteFactory extends ReviewRemoteFactory<GerritChange,
} catch (GerritException e) {
throw GerritCorePlugin.getDefault()
.getConnector()
- .toCoreException(parent.getTaskRepository(), "Problem while retrieving Gerrit review.", e);
+ .toCoreException(parent.getTaskRepository(), "Problem while retrieving Gerrit review.", e); //$NON-NLS-1$
}
}
@@ -248,7 +249,7 @@ public class GerritReviewRemoteFactory extends ReviewRemoteFactory<GerritChange,
//We force a pull here, which is safe because there isn't any actual client API invocation
consumer.pull(true, new NullProgressMonitor());
} catch (CoreException e) {
- throw new RuntimeException("Internal Exception. Unexpected state.", e);
+ throw new RuntimeException("Internal Exception. Unexpected state.", e); //$NON-NLS-1$
}
if (patchIndex++ < oldPatchCount) {
consumer.release();
@@ -334,7 +335,7 @@ public class GerritReviewRemoteFactory extends ReviewRemoteFactory<GerritChange,
IUser reviewer = getGerritProvider().createUser(parent, detail.getAccounts(),
remoteApproval.getAccount());
if (reviewer == null) {
- throw new RuntimeException("Internal Error, no reviewer found for: " + remoteApproval.getAccount());
+ throw new RuntimeException("Internal Error, no reviewer found for: " + remoteApproval.getAccount()); //$NON-NLS-1$
}
IReviewerEntry reviewerEntry = review.getReviewerApprovals().get(reviewer);
if (reviewerEntry == null) {
@@ -366,18 +367,22 @@ public class GerritReviewRemoteFactory extends ReviewRemoteFactory<GerritChange,
for (Label label : record.getLabels()) {
IApprovalType approvalType = typeForName.get(label.getLabel());
if (approvalType == null) {
- throw new RuntimeException("Internal Error, no approval type found for: " + label.getLabel());
+ if (detail.getChange().getStatus() == Status.ABANDONED) {
+ // typeForName can be empty for an abandoned change as it no longer provides approval types info
+ continue;
+ }
+ throw new RuntimeException("Internal Error, no approval type found for: " + label.getLabel()); //$NON-NLS-1$
}
IRequirementEntry requirementEntry = IReviewsFactory.INSTANCE.createRequirementEntry();
- if (label.getStatus().equals("OK")) {
+ if (label.getStatus().equals("OK")) { //$NON-NLS-1$
requirementEntry.setStatus(RequirementStatus.SATISFIED);
- } else if (label.getStatus().equals("NEED")) {
+ } else if (label.getStatus().equals("NEED")) { //$NON-NLS-1$
requirementEntry.setStatus(RequirementStatus.NOT_SATISFIED);
- } else if (label.getStatus().equals("REJECT")) {
+ } else if (label.getStatus().equals("REJECT")) { //$NON-NLS-1$
requirementEntry.setStatus(RequirementStatus.REJECTED);
- } else if (label.getStatus().equals("MAY")) {
+ } else if (label.getStatus().equals("MAY")) { //$NON-NLS-1$
requirementEntry.setStatus(RequirementStatus.OPTIONAL);
- } else if (label.getStatus().equals("IMPOSSIBLE")) {
+ } else if (label.getStatus().equals("IMPOSSIBLE")) { //$NON-NLS-1$
requirementEntry.setStatus(RequirementStatus.ERROR);
}
if (label.getAppliedBy() != null) {
@@ -406,7 +411,7 @@ public class GerritReviewRemoteFactory extends ReviewRemoteFactory<GerritChange,
case ABANDONED:
return ReviewStatus.ABANDONED;
default:
- GerritCorePlugin.logError("Internal Error: unexpected change status: " + gerritStatus, new Exception());
+ GerritCorePlugin.logError("Internal Error: unexpected change status: " + gerritStatus, new Exception()); //$NON-NLS-1$
return null;
}
}
@@ -422,7 +427,7 @@ public class GerritReviewRemoteFactory extends ReviewRemoteFactory<GerritChange,
for (ChangeInfo remoteChange : remoteChanges) {
final IChange localChange = IReviewsFactory.INSTANCE.createChange();
localChange.setKey(remoteChange.getKey().get());
- localChange.setId(remoteChange.getId().get() + "");
+ localChange.setId(Integer.toString(remoteChange.getId().get()));
AccountInfo remoteOwner = detail.getAccounts().get(remoteChange.getOwner());
IUser owner = getGerritProvider().createUser(group, detail.getAccounts(), remoteOwner.getId());
localChange.setOwner(owner);
@@ -435,7 +440,7 @@ public class GerritReviewRemoteFactory extends ReviewRemoteFactory<GerritChange,
@Override
public String getRemoteKey(GerritChange remoteObject) {
- return remoteObject.getChangeDetail().getChange().getId().get() + "";
+ return Integer.toString(remoteObject.getChangeDetail().getChange().getId().get());
}
@Override
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 84423b8..59f52a9 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
@@ -385,14 +385,9 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
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());
- }
+ assertThat(getReview().getReviewerApprovals().isEmpty(), is(false));
+ assertThat(getReview().getReviewerApprovals().size(), is(1));
+ assertThat(getReview().getReviewerApprovals().get(0), nullValue());
}
private static Version getCurrentVersion() {