467049: deleting just-created drafts does not work

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

10 files changed, 28 insertions, 55 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 a26984838..88ab4534c 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
@@ -23,7 +23,6 @@ import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
-import java.lang.reflect.Type;
 import java.net.HttpURLConnection;
 import java.net.URLEncoder;
 import java.net.UnknownHostException;
@@ -117,7 +116,6 @@ import com.google.gerrit.reviewdb.ContributorAgreement;
 import com.google.gerrit.reviewdb.Patch;
 import com.google.gerrit.reviewdb.Patch.ChangeType;
 import com.google.gerrit.reviewdb.Patch.Key;
-import com.google.gerrit.reviewdb.PatchLineComment;
 import com.google.gerrit.reviewdb.PatchSet;
 import com.google.gerrit.reviewdb.PatchSet.Id;
 import com.google.gerrit.reviewdb.Project;
@@ -173,7 +171,7 @@ public abstract class GerritClient extends ReviewsClient {
 	public abstract PatchSetPublishDetailX getPatchSetPublishDetail(final PatchSet.Id id, IProgressMonitor monitor)
 			throws GerritException;
 
-	public abstract PatchLineComment saveDraft(Key patchKey, String message, int line, short side, String parentUuid,
+	public abstract CommentInput saveDraft(Key patchKey, String message, int line, short side, String parentUuid,
 			String uuid, IProgressMonitor monitor) throws GerritException;
 
 	public abstract VoidResult deleteDraft(Key patchkey, String uuid, IProgressMonitor monitor) throws GerritException;
@@ -554,15 +552,6 @@ public abstract class GerritClient extends ReviewsClient {
 		return clazz.cast(service);
 	}
 
-	protected PatchLineComment createDraftComment(Patch.Key patchKey, String message, int line, short side,
-			String parentUuid, String uuid, IProgressMonitor monitor) throws GerritException {
-		PatchLineComment.Key id = new PatchLineComment.Key(patchKey, uuid);
-		final PatchLineComment comment = new PatchLineComment(id, line, getAccount(monitor).getId(), parentUuid);
-		comment.setMessage(message);
-		comment.setSide(side);
-		return comment;
-	}
-
 	protected List<ReviewerInfo> listReviewers(final int reviewId, IProgressMonitor monitor) throws GerritException {
 		final String uri = "/changes/" + reviewId + "/reviewers/"; //$NON-NLS-1$ //$NON-NLS-2$
 		TypeToken<List<ReviewerInfo>> reviewersListType = new TypeToken<List<ReviewerInfo>>() {
@@ -591,7 +580,8 @@ public abstract class GerritClient extends ReviewsClient {
 		ReviewerResult reviewerResult = new ReviewerResult();
 		for (final String reviewerId : reviewers) {
 			try {
-				AddReviewerResult addReviewerResult = restClient.executePostRestRequest(uri, new ReviewerInput(reviewerId), AddReviewerResult.class, null, monitor);
+				AddReviewerResult addReviewerResult = restClient.executePostRestRequest(uri, new ReviewerInput(
+						reviewerId), AddReviewerResult.class, null, monitor);
 				reviewerInfos.addAll(addReviewerResult.getReviewers());
 			} catch (GerritHttpException e) {
 				if (e.getResponseCode() == HttpStatus.SC_UNPROCESSABLE_ENTITY) {
@@ -695,7 +685,8 @@ public abstract class GerritClient extends ReviewsClient {
 				if (starred) {
 					restClient.executePutRestRequest(uri, req, ToggleStarRequest.class, createErrorHandler(), monitor);
 				} else {
-					restClient.executeDeleteRestRequest(uri, req, ToggleStarRequest.class, createErrorHandler(), monitor);
+					restClient.executeDeleteRestRequest(uri, req, ToggleStarRequest.class, createErrorHandler(),
+							monitor);
 				}
 			}
 		});
@@ -726,7 +717,7 @@ public abstract class GerritClient extends ReviewsClient {
 		ChangeInfo28 changeInfo28 = null;
 		try {
 			changeInfo28 = restClient.executeGetRestRequest("/changes/" + Integer.toString(reviewId) //$NON-NLS-1$
-			+ "/?o=ALL_REVISIONS&o=CURRENT_ACTIONS&o=ALL_COMMITS", ChangeInfo28.class, monitor); //$NON-NLS-1$
+					+ "/?o=ALL_REVISIONS&o=CURRENT_ACTIONS&o=ALL_COMMITS", ChangeInfo28.class, monitor); //$NON-NLS-1$
 		} catch (GerritException e) {
 			StatusHandler.log(new Status(IStatus.ERROR, GerritCorePlugin.PLUGIN_ID, e.getMessage(), e));
 		}
@@ -829,7 +820,7 @@ public abstract class GerritClient extends ReviewsClient {
 		reviewInput.setApprovals(approvals);
 		final String uri = "/a/changes/" + id.getParentKey().get() + "/revisions/" + id.get() + "/review"; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
 		restClient.executePostRestRequest(uri, reviewInput, ReviewInfo.class, new ErrorHandler() {
-		
+
 			@Override
 			public void handleError(HttpMethodBase method) throws GerritException {
 				if (method.getStatusCode() == HttpURLConnection.HTTP_FORBIDDEN) {
@@ -839,7 +830,7 @@ public abstract class GerritClient extends ReviewsClient {
 					}
 				}
 			}
-		
+
 			private String getResponseBodyAsString(HttpMethodBase method) {
 				try {
 					String msg = method.getResponseBodyAsString();
@@ -945,7 +936,7 @@ public abstract class GerritClient extends ReviewsClient {
 					throw new GerritException(NLS.bind("Cannot submit change: {0}", errorMsg)); //$NON-NLS-1$
 				}
 			}
-		
+
 			private String getResponseBodyAsString(HttpMethodBase method) {
 				try {
 					return method.getResponseBodyAsString();
@@ -953,12 +944,12 @@ public abstract class GerritClient extends ReviewsClient {
 					return null;
 				}
 			}
-		
+
 			private boolean isNotPermitted(HttpMethodBase method, String msg) {
 				return method.getStatusCode() == HttpURLConnection.HTTP_FORBIDDEN
 						&& "submit not permitted\n".equals(msg); //$NON-NLS-1$
 			}
-		
+
 			private boolean isConflict(HttpMethodBase method) {
 				return method.getStatusCode() == HttpURLConnection.HTTP_CONFLICT;
 			}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient29.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient29.java
index 651a56f0d..d6509f14e 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient29.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritClient29.java
@@ -55,7 +55,6 @@ import com.google.gerrit.reviewdb.Branch;
 import com.google.gerrit.reviewdb.Change;
 import com.google.gerrit.reviewdb.ChangeMessage;
 import com.google.gerrit.reviewdb.Patch.Key;
-import com.google.gerrit.reviewdb.PatchLineComment;
 import com.google.gerrit.reviewdb.PatchSet;
 import com.google.gerrit.reviewdb.PatchSetInfo;
 import com.google.gerrit.reviewdb.Project;
@@ -400,9 +399,8 @@ public class GerritClient29 extends GerritClient {
 	}
 
 	@Override
-	public PatchLineComment saveDraft(Key patchKey, String message, int line, short side, String parentUuid,
-			String uuid, IProgressMonitor monitor) throws GerritException {
-		final PatchLineComment comment = createDraftComment(patchKey, message, line, side, parentUuid, uuid, monitor);
+	public CommentInput saveDraft(Key patchKey, String message, int line, short side, String parentUuid, String uuid,
+			IProgressMonitor monitor) throws GerritException {
 		if (uuid == null) {
 			uuid = ""; //$NON-NLS-1$
 		}
@@ -422,9 +420,7 @@ public class GerritClient29 extends GerritClient {
 			uri = uri.concat("/" + uuid); //$NON-NLS-1$
 		}
 
-		getRestClient().executePutRestRequest(uri, commentInput, CommentInput.class, null, monitor);
-
-		return comment;
+		return getRestClient().executePutRestRequest(uri, commentInput, CommentInput.class, null, monitor);
 	}
 
 	private void getAdditionalChangeInfo(int reviewId, ChangeDetailX changeDetail, IProgressMonitor monitor) {
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ActionInfo.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ActionInfo.java
index c259377c4..60feff3a1 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ActionInfo.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ActionInfo.java
@@ -16,13 +16,6 @@ package org.eclipse.mylyn.internal.gerrit.core.client.rest;
  * href="https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#action-info">ActionInfo</a>.
  */
 public class ActionInfo {
-	// e.g. "gerritcodereview#change"
-	private String method;
-
-	private String label;
-
-	private String title;
-
 	private boolean enabled;
 
 	public boolean getEnabled() {
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeMessageInfo.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeMessageInfo.java
index bd432cac4..ce81acec0 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeMessageInfo.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/ChangeMessageInfo.java
@@ -20,16 +20,12 @@ import java.sql.Timestamp;
  */
 public class ChangeMessageInfo {
 
-	private String id;
-
 	private AccountInfo author;
 
 	private Timestamp date;
 
 	private String message;
 
-	private Integer _revisionNumber;
-
 	public String getMesssage() {
 		return message;
 	}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/CommitInfo.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/CommitInfo.java
index e66141f21..b22940f3d 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/CommitInfo.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/CommitInfo.java
@@ -11,7 +11,6 @@
  ******************************************************************************/
 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#commit-info">CommitInfo</a>.
@@ -24,8 +23,6 @@ public class CommitInfo {
 
 	private GitPersonalInfo author;
 
-	private GitPersonalInfo committer;
-
 	private String subject;
 
 	private String message;
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/GitPersonalInfo.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/GitPersonalInfo.java
index d20e65235..0bfc85651 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/GitPersonalInfo.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/rest/GitPersonalInfo.java
@@ -22,10 +22,6 @@ public class GitPersonalInfo {
 
 	private String email;
 
-	private String date;
-
-	private int tz;
-
 	public String getName() {
 		return name;
 	}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/operations/SaveDraftRequest.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/operations/SaveDraftRequest.java
index 27ddea275..2c0dadd40 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/operations/SaveDraftRequest.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/operations/SaveDraftRequest.java
@@ -15,15 +15,15 @@ package org.eclipse.mylyn.internal.gerrit.core.operations;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.mylyn.internal.gerrit.core.client.GerritClient;
 import org.eclipse.mylyn.internal.gerrit.core.client.GerritException;
+import org.eclipse.mylyn.internal.gerrit.core.client.rest.CommentInput;
 
 import com.google.gerrit.reviewdb.Patch.Key;
-import com.google.gerrit.reviewdb.PatchLineComment;
 
 /**
  * @author Steffen Pingel
  * @author Guy Perron
  */
-public class SaveDraftRequest extends AbstractRequest<PatchLineComment> {
+public class SaveDraftRequest extends AbstractRequest<CommentInput> {
 
 	private final Key patchKey;
 
@@ -64,7 +64,7 @@ public class SaveDraftRequest extends AbstractRequest<PatchLineComment> {
 	}
 
 	@Override
-	protected PatchLineComment execute(GerritClient client, IProgressMonitor monitor) throws GerritException {
+	protected CommentInput execute(GerritClient client, IProgressMonitor monitor) throws GerritException {
 		return client.saveDraft(getPatchKey(), getMessage(), getLine(), getSide(), getParentUuid(), getUuid(), monitor);
 	}
 
diff --git a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java
index c5a793559..cb56587c6 100644
--- a/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java
+++ b/org.eclipse.mylyn.gerrit.tests/src/org/eclipse/mylyn/internal/gerrit/core/remote/PatchSetRemoteFactoryTest.java
@@ -40,6 +40,7 @@ import org.eclipse.mylyn.internal.gerrit.core.client.GerritClient;
 import org.eclipse.mylyn.internal.gerrit.core.client.GerritException;
 import org.eclipse.mylyn.internal.gerrit.core.client.PatchSetContent;
 import org.eclipse.mylyn.internal.gerrit.core.client.compat.PatchScriptX;
+import org.eclipse.mylyn.internal.gerrit.core.client.rest.CommentInput;
 import org.eclipse.mylyn.reviews.core.model.IComment;
 import org.eclipse.mylyn.reviews.core.model.IFileItem;
 import org.eclipse.mylyn.reviews.core.model.IFileVersion;
@@ -262,8 +263,8 @@ public class PatchSetRemoteFactoryTest extends GerritRemoteTest {
 		assertThat(commentFile.getAllComments().size(), is(0));
 
 		String id = commentFile.getReference();
-		reviewHarness.getClient().saveDraft(Patch.Key.parse(id), "Line 2 Comment", 2, (short) 1, null, null,
-				new NullProgressMonitor());
+		CommentInput commentInput = reviewHarness.getClient().saveDraft(Patch.Key.parse(id), "Line 2 Comment", 2,
+				(short) 1, null, null, new NullProgressMonitor());
 		patchSetObserver.retrieve(false);
 		patchSetObserver.waitForResponse();
 
@@ -271,6 +272,7 @@ public class PatchSetRemoteFactoryTest extends GerritRemoteTest {
 		List<IComment> allComments = commentFile.getAllComments();
 		assertThat(allComments.size(), is(1));
 		IComment fileComment = allComments.get(0);
+		assertThat("saveDraft returned wrong comment id", fileComment.getId(), is(commentInput.getId()));
 		assertThat(fileComment, notNullValue());
 		assertThat(fileComment.isDraft(), is(true));
 		assertThat(fileComment.getAuthor().getDisplayName(), is("tests"));
diff --git a/org.eclipse.mylyn.gerrit.ui/.classpath b/org.eclipse.mylyn.gerrit.ui/.classpath
index 2d567e659..6b2ca7162 100644
--- a/org.eclipse.mylyn.gerrit.ui/.classpath
+++ b/org.eclipse.mylyn.gerrit.ui/.classpath
@@ -12,6 +12,8 @@
 			<accessrule kind="accessible" pattern="org/eclipse/mylyn/internal/reviews/ui/providers/**"/>
 			<accessrule kind="accessible" pattern="org/eclipse/mylyn/internal/reviews/ui/compare/**"/>
 			<accessrule kind="accessible" pattern="org/eclipse/mylyn/internal/tasks/ui/editors/**"/>
+			<accessrule kind="accessible" pattern="org/eclipse/mylyn/internal/gerrit/core/client/rest/*"/>
+			<accessrule kind="accessible" pattern="org/eclipse/egit/**"/>
 		</accessrules>
 	</classpathentry>
 	<classpathentry kind="src" path="src"/>
diff --git a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/GerritReviewBehavior.java b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/GerritReviewBehavior.java
index 093be541f..cea8b8406 100644
--- a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/GerritReviewBehavior.java
+++ b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/GerritReviewBehavior.java
@@ -18,6 +18,7 @@ import org.eclipse.core.runtime.IStatus;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.mylyn.internal.gerrit.core.GerritOperationFactory;
 import org.eclipse.mylyn.internal.gerrit.core.Messages;
+import org.eclipse.mylyn.internal.gerrit.core.client.rest.CommentInput;
 import org.eclipse.mylyn.internal.gerrit.core.operations.DiscardDraftRequest;
 import org.eclipse.mylyn.internal.gerrit.core.operations.GerritOperation;
 import org.eclipse.mylyn.internal.gerrit.core.operations.SaveDraftRequest;
@@ -34,7 +35,6 @@ import org.eclipse.team.core.history.IFileRevision;
 
 import com.google.common.base.Strings;
 import com.google.gerrit.reviewdb.Patch;
-import com.google.gerrit.reviewdb.PatchLineComment;
 import com.google.gwtjsonrpc.client.VoidResult;
 
 /**
@@ -81,12 +81,12 @@ public class GerritReviewBehavior extends ReviewBehavior {
 						Strings.emptyToNull(comment.getId()));
 				request.setMessage(comment.getDescription());
 
-				GerritOperation<PatchLineComment> operation = getOperationFactory().createOperation(getTask(), request);
+				GerritOperation<CommentInput> operation = getOperationFactory().createOperation(getTask(), request);
 				IStatus status = operation.run(monitor);
-				PatchLineComment patchLineComment = operation.getOperationResult();
+				CommentInput commentInput = operation.getOperationResult();
 				// save the value of uuid, and keep it with the comment
-				if (patchLineComment != null && patchLineComment.getKey() != null) {
-					comment.setId(patchLineComment.getKey().get());
+				if (commentInput != null) {
+					comment.setId(commentInput.getId());
 				}
 				return status;
 			}