410934: [regression] Draft review cannot be opened, synchronization

failed (force build)

Change-Id: I2a827c1b74e33f6f823211bc4deed171fc4748ed
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=410934

11 files changed, 65 insertions, 39 deletions

diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/GerritUtil.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/GerritUtil.java
index ea3315f3c..7fc655c43 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/GerritUtil.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/GerritUtil.java
@@ -27,7 +27,6 @@ import com.google.gerrit.common.data.AccountInfo;
 import com.google.gerrit.common.data.GerritConfig;
 import com.google.gerrit.reviewdb.Account;
 import com.google.gerrit.reviewdb.AccountGeneralPreferences.DownloadScheme;
-import com.google.gerrit.reviewdb.Change.Status;
 import com.google.gerrit.reviewdb.Project;
 
 /**
@@ -194,10 +193,4 @@ public class GerritUtil {
 		uriMap.put(DownloadScheme.ANON_GIT, getAnonGitCloneUri(repository, config, project));
 		return uriMap;
 	}
-
-	public static boolean isDraft(Status status) {
-		// DRAFT is not correctly parsed for ChangeInfo since Change.Status does not define the corresponding enum field 
-		return status == null;
-	}
-
 }
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/SubmitRecord.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/SubmitRecord.java
index e0555e68e..6bb060531 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/SubmitRecord.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/compat/SubmitRecord.java
@@ -11,6 +11,7 @@
 
 package org.eclipse.mylyn.internal.gerrit.core.client.compat;
 
+import java.util.Collections;
 import java.util.List;
 
 import com.google.gerrit.reviewdb.Account;
@@ -51,7 +52,7 @@ public class SubmitRecord {
 
 	String status;
 
-	private List<Label> labels;
+	private List<Label> labels = Collections.emptyList();;
 
 	public String getStatus() {
 		return status;
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/data/GerritQueryResult.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/data/GerritQueryResult.java
index 4674c9501..c87f1cefb 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/data/GerritQueryResult.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/data/GerritQueryResult.java
@@ -13,7 +13,8 @@ package org.eclipse.mylyn.internal.gerrit.core.client.data;
 
 import java.sql.Timestamp;
 
-import org.eclipse.mylyn.internal.gerrit.core.GerritUtil;
+import org.eclipse.mylyn.internal.gerrit.core.remote.GerritReviewRemoteFactory;
+import org.eclipse.mylyn.reviews.core.model.ReviewStatus;
 
 import com.google.gerrit.common.data.ChangeInfo;
 import com.google.gerrit.reviewdb.Change.Status;
@@ -47,7 +48,7 @@ public class GerritQueryResult {
 		setProject(changeInfo.getProject().getName());
 		setSubject(changeInfo.getSubject());
 		Status status = changeInfo.getStatus();
-		if (GerritUtil.isDraft(status)) {
+		if (GerritReviewRemoteFactory.getReviewStatus(status) == ReviewStatus.DRAFT) {
 			setStatus("DRAFT"); //$NON-NLS-1$
 		} else {
 			setStatus(status.toString());
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 51aa4627a..2dbe0b2d0 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
@@ -336,20 +336,26 @@ public class GerritReviewRemoteFactory extends ReviewRemoteFactory<GerritChange,
 			}
 		}
 
-		//State
-		switch (detail.getChange().getStatus()) {
+		review.setState(getReviewStatus(detail.getChange().getStatus()));
+	}
+
+	public static ReviewStatus getReviewStatus(com.google.gerrit.reviewdb.Change.Status gerritStatus) {
+		if (gerritStatus == null) {
+			// DRAFT is not correctly parsed for ChangeInfo since Change.Status does not define the corresponding enum field 
+			return ReviewStatus.DRAFT;
+		}
+		switch (gerritStatus) {
 		case NEW:
-			review.setState(ReviewStatus.NEW);
-			break;
+			return ReviewStatus.NEW;
 		case MERGED:
-			review.setState(ReviewStatus.MERGED);
-			break;
+			return ReviewStatus.MERGED;
 		case SUBMITTED:
-			review.setState(ReviewStatus.SUBMITTED);
-			break;
+			return ReviewStatus.SUBMITTED;
 		case ABANDONED:
-			review.setState(ReviewStatus.ABANDONED);
-			break;
+			return ReviewStatus.ABANDONED;
+		default:
+			GerritCorePlugin.logError("Internal Error: unexpected change status: " + gerritStatus, new Exception());
+			return null;
 		}
 	}
 
@@ -370,20 +376,7 @@ public class GerritReviewRemoteFactory extends ReviewRemoteFactory<GerritChange,
 			localChange.setOwner(owner);
 			localChange.setModificationDate(remoteChange.getLastUpdatedOn());
 			localChange.setSubject(remoteChange.getSubject());
-			switch (remoteChange.getStatus()) {
-			case NEW:
-				localChange.setState(ReviewStatus.NEW);
-				break;
-			case MERGED:
-				localChange.setState(ReviewStatus.MERGED);
-				break;
-			case SUBMITTED:
-				localChange.setState(ReviewStatus.SUBMITTED);
-				break;
-			case ABANDONED:
-				localChange.setState(ReviewStatus.ABANDONED);
-				break;
-			}
+			localChange.setState(getReviewStatus(remoteChange.getStatus()));
 			localChanges.add(localChange);
 		}
 	}
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 0647ae7db..9e1007495 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
@@ -47,6 +47,7 @@ import org.junit.Test;
 
 import com.google.gerrit.reviewdb.ApprovalCategory;
 import com.google.gerrit.reviewdb.ApprovalCategoryValue;
+import com.google.gerrit.reviewdb.Change.Status;
 
 /**
  * @author Miles Parker
@@ -72,6 +73,16 @@ public class GerritReviewRemoteFactoryTest extends GerritRemoteTest {
 	}
 
 	@Test
+	public void testReviewStatus() throws Exception {
+		assertThat(GerritReviewRemoteFactory.getReviewStatus(Status.ABANDONED), is(ReviewStatus.ABANDONED));
+		assertThat(GerritReviewRemoteFactory.getReviewStatus(Status.MERGED), is(ReviewStatus.MERGED));
+		assertThat(GerritReviewRemoteFactory.getReviewStatus(Status.NEW), is(ReviewStatus.NEW));
+		assertThat(GerritReviewRemoteFactory.getReviewStatus(Status.SUBMITTED), is(ReviewStatus.SUBMITTED));
+		//Test for drafts hack
+		assertThat(GerritReviewRemoteFactory.getReviewStatus(null), is(ReviewStatus.DRAFT));
+	}
+
+	@Test
 	public void testNewChange() throws Exception {
 		CommitCommand command2 = reviewHarness.git.commit()
 				.setAmend(true)
diff --git a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/editor/GerritReviewDetailSection.java b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/editor/GerritReviewDetailSection.java
index d26c28216..32c5c81e9 100644
--- a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/editor/GerritReviewDetailSection.java
+++ b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/editor/GerritReviewDetailSection.java
@@ -34,6 +34,6 @@ public class GerritReviewDetailSection extends ReviewDetailSection {
 
 	@Override
 	protected boolean canAddReviewers() {
-		return getReview().getState() == null || getReview().getState() == ReviewStatus.NEW;
+		return getReview().getState() == ReviewStatus.DRAFT || getReview().getState() == ReviewStatus.NEW;
 	}
 }
diff --git a/org.eclipse.mylyn.reviews.core/model/reviews.ecore b/org.eclipse.mylyn.reviews.core/model/reviews.ecore
index c419695d3..09131a92d 100644
--- a/org.eclipse.mylyn.reviews.core/model/reviews.ecore
+++ b/org.eclipse.mylyn.reviews.core/model/reviews.ecore
@@ -173,6 +173,7 @@
     <eLiterals name="Submitted" value="1" literal="SUBMITTED"/>
     <eLiterals name="Merged" value="2" literal="MERGED"/>
     <eLiterals name="Abandoned" value="3" literal="ABANDONED"/>
+    <eLiterals name="Draft" value="4" literal="DRAFT"/>
   </eClassifiers>
   <eClassifiers xsi:type="ecore:EDataType" name="IFileRevision" instanceClassName="org.eclipse.team.core.history.IFileRevision"
       serializable="false"/>
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/model/ReviewStatus.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/model/ReviewStatus.java
index e125cc005..4695728fa 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/model/ReviewStatus.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/model/ReviewStatus.java
@@ -56,7 +56,15 @@ public enum ReviewStatus implements InternalReviewStatus {
 	 * @generated
 	 * @ordered
 	 */
-	ABANDONED(3, "Abandoned", "ABANDONED"); //$NON-NLS-1$ //$NON-NLS-2$
+	ABANDONED(3, "Abandoned", "ABANDONED"), /**
+	 * The '<em><b>Draft</b></em>' literal object. <!-- begin-user-doc --> <!--
+	 * end-user-doc -->
+	 * 
+	 * @see #DRAFT_VALUE
+	 * @generated
+	 * @ordered
+	 */
+	DRAFT(4, "Draft", "DRAFT"); //$NON-NLS-1$ //$NON-NLS-2$
 
 	/**
 	 * The '<em><b>New</b></em>' literal value. <!-- begin-user-doc -->
@@ -115,11 +123,25 @@ public enum ReviewStatus implements InternalReviewStatus {
 	public static final int ABANDONED_VALUE = 3;
 
 	/**
+	 * The '<em><b>Draft</b></em>' literal value. <!-- begin-user-doc -->
+	 * <p>
+	 * If the meaning of '<em><b>Draft</b></em>' literal object isn't clear, there really should be more of a
+	 * description here...
+	 * </p>
+	 * <!-- end-user-doc -->
+	 * 
+	 * @see #DRAFT
+	 * @generated
+	 * @ordered
+	 */
+	public static final int DRAFT_VALUE = 4;
+
+	/**
 	 * An array of all the '<em><b>Review Status</b></em>' enumerators. <!-- begin-user-doc --> <!-- end-user-doc -->
 	 * 
 	 * @generated
 	 */
-	private static final ReviewStatus[] VALUES_ARRAY = new ReviewStatus[] { NEW, SUBMITTED, MERGED, ABANDONED, };
+	private static final ReviewStatus[] VALUES_ARRAY = new ReviewStatus[] { NEW, SUBMITTED, MERGED, ABANDONED, DRAFT, };
 
 	/**
 	 * A public read-only list of all the '<em><b>Review Status</b></em>' enumerators. <!-- begin-user-doc --> <!--
@@ -177,6 +199,8 @@ public enum ReviewStatus implements InternalReviewStatus {
 			return MERGED;
 		case ABANDONED_VALUE:
 			return ABANDONED;
+		case DRAFT_VALUE:
+			return DRAFT;
 		}
 		return null;
 	}
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/AbstractRemoteEmfFactory.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/AbstractRemoteEmfFactory.java
index 14c186588..62aabc1e2 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/AbstractRemoteEmfFactory.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/AbstractRemoteEmfFactory.java
@@ -193,7 +193,7 @@ public abstract class AbstractRemoteEmfFactory<EParentObjectType extends EObject
 		}
 		return consumer;
 	}
-	
+
 	class ObjectFinder implements Runnable {
 		EObjectType foundObject;
 
@@ -224,7 +224,7 @@ public abstract class AbstractRemoteEmfFactory<EParentObjectType extends EObject
 			}
 		}
 	}
-	
+
 	protected EObjectType open(EParentObjectType parentObject, LocalKeyType localKey) {
 		ObjectFinder finder = new ObjectFinder(parentObject, localKey);
 		getService().modelExec(finder, true);
diff --git a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/ReviewsPackage.java b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/ReviewsPackage.java
index 55544e6e2..73a41f262 100644
--- a/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/ReviewsPackage.java
+++ b/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/internal/core/model/ReviewsPackage.java
@@ -3647,6 +3647,7 @@ public class ReviewsPackage extends EPackageImpl {
 		addEEnumLiteral(reviewStatusEEnum, ReviewStatus.SUBMITTED);
 		addEEnumLiteral(reviewStatusEEnum, ReviewStatus.MERGED);
 		addEEnumLiteral(reviewStatusEEnum, ReviewStatus.ABANDONED);
+		addEEnumLiteral(reviewStatusEEnum, ReviewStatus.DRAFT);
 
 		// Initialize data types
 		initEDataType(iFileRevisionEDataType, IFileRevision.class,
diff --git a/org.eclipse.mylyn.reviews.edit/plugin.properties b/org.eclipse.mylyn.reviews.edit/plugin.properties
index 471b8b2f2..1f82617f4 100644
--- a/org.eclipse.mylyn.reviews.edit/plugin.properties
+++ b/org.eclipse.mylyn.reviews.edit/plugin.properties
@@ -137,3 +137,4 @@ _UI_ReviewStatus_New_literal = NEW
 _UI_ReviewStatus_Submitted_literal = SUBMITTED
 _UI_ReviewStatus_Merged_literal = MERGED
 _UI_ReviewStatus_Abandoned_literal = ABANDONED
+_UI_ReviewStatus_Draft_literal = DRAFT