Accept in_reply_to to thread inline comments

The in_reply_to field supplies the id of another comment, enabling
threading. Because id is URL encoded to match a REST API we also
encode the in_reply_to field, allowing clients to correctly line up
messages by simple string equality.

Change-Id: Idc6af51873b9b8a12707c6f4df9d25b627e8b409
This commit is contained in:
Shawn O. Pearce
2012-11-23 19:46:40 -08:00
parent abaa4d9578
commit 04a17cfa75
6 changed files with 40 additions and 17 deletions

View File

@@ -185,4 +185,8 @@ public final class PatchLineComment {
public String getParentUuid() { public String getParentUuid() {
return parentUuid; return parentUuid;
} }
public void setParentUuid(String inReplyTo) {
parentUuid = inReplyTo;
}
} }

View File

@@ -25,6 +25,7 @@ import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.change.PutDraft.Input; import com.google.gerrit.server.change.PutDraft.Input;
import com.google.gerrit.server.util.Url;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -60,7 +61,7 @@ class CreateDraft implements RestModifyView<RevisionResource, Input> {
ChangeUtil.messageUUID(db.get())), ChangeUtil.messageUUID(db.get())),
in.line != null ? in.line : 0, in.line != null ? in.line : 0,
rsrc.getAuthorId(), rsrc.getAuthorId(),
null); Url.decode(in.inReplyTo));
c.setStatus(Status.DRAFT); c.setStatus(Status.DRAFT);
c.setSide(in.side == GetDraft.Side.PARENT ? (short) 0 : (short) 1); c.setSide(in.side == GetDraft.Side.PARENT ? (short) 0 : (short) 1);
c.setMessage(in.message.trim()); c.setMessage(in.message.trim());

View File

@@ -41,6 +41,7 @@ class GetDraft implements RestReadView<DraftResource> {
String path; String path;
Side side; Side side;
Integer line; Integer line;
String inReplyTo;
String message; String message;
Timestamp updated; Timestamp updated;
@@ -53,6 +54,7 @@ class GetDraft implements RestReadView<DraftResource> {
if (c.getLine() > 0) { if (c.getLine() > 0) {
line = c.getLine(); line = c.getLine();
} }
inReplyTo = Url.encode(c.getParentUuid());
message = Strings.emptyToNull(c.getMessage()); message = Strings.emptyToNull(c.getMessage());
updated = c.getWrittenOn(); updated = c.getWrittenOn();
} }

View File

@@ -39,6 +39,7 @@ import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.PostReview.Input; import com.google.gerrit.server.change.PostReview.Input;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.util.Url;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -85,6 +86,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
String id; String id;
GetDraft.Side side; GetDraft.Side side;
int line; int line;
String inReplyTo;
String message; String message;
} }
@@ -261,6 +263,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
for (Map.Entry<String, List<Comment>> ent : in.entrySet()) { for (Map.Entry<String, List<Comment>> ent : in.entrySet()) {
String path = ent.getKey(); String path = ent.getKey();
for (Comment c : ent.getValue()) { for (Comment c : ent.getValue()) {
String parent = Url.decode(c.inReplyTo);
PatchLineComment e = drafts.remove(c.id); PatchLineComment e = drafts.remove(c.id);
boolean create = e == null; boolean create = e == null;
if (create) { if (create) {
@@ -270,7 +273,9 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
ChangeUtil.messageUUID(db)), ChangeUtil.messageUUID(db)),
c.line, c.line,
rsrc.getAuthorId(), rsrc.getAuthorId(),
null); parent);
} else if (parent != null) {
e.setParentUuid(parent);
} }
e.setStatus(PatchLineComment.Status.PUBLISHED); e.setStatus(PatchLineComment.Status.PUBLISHED);
e.setWrittenOn(timestamp); e.setWrittenOn(timestamp);

View File

@@ -24,6 +24,8 @@ import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.GetDraft.Side; import com.google.gerrit.server.change.GetDraft.Side;
import com.google.gerrit.server.change.PutDraft.Input; import com.google.gerrit.server.change.PutDraft.Input;
import com.google.gerrit.server.util.Url;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -37,6 +39,7 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
String path; String path;
Side side; Side side;
Integer line; Integer line;
String inReplyTo;
Timestamp updated; // Accepted but ignored. Timestamp updated; // Accepted but ignored.
@DefaultInput @DefaultInput
@@ -58,9 +61,9 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
} }
@Override @Override
public Object apply(DraftResource rsrc, Input in) public Object apply(DraftResource rsrc, Input in) throws AuthException,
throws AuthException, BadRequestException, ResourceConflictException, BadRequestException, ResourceConflictException, OrmException {
Exception { PatchLineComment c = rsrc.getComment();
if (in == null || in.message == null || in.message.trim().isEmpty()) { if (in == null || in.message == null || in.message.trim().isEmpty()) {
return delete.get().apply(rsrc, null); return delete.get().apply(rsrc, null);
} else if (in.kind != null && !"gerritcodereview#comment".equals(in.kind)) { } else if (in.kind != null && !"gerritcodereview#comment".equals(in.kind)) {
@@ -69,20 +72,19 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
throw new BadRequestException("line must be >= 0"); throw new BadRequestException("line must be >= 0");
} }
PatchLineComment c = rsrc.getComment();
if (in.path != null if (in.path != null
&& !in.path.equals(c.getKey().getParentKey().getFileName())) { && !in.path.equals(c.getKey().getParentKey().getFileName())) {
// Updating the path alters the primary key, which isn't possible. // Updating the path alters the primary key, which isn't possible.
// Delete then recreate the comment instead of an update. // Delete then recreate the comment instead of an update.
db.get().patchComments().delete(Collections.singleton(c)); db.get().patchComments().delete(Collections.singleton(c));
c = update(new PatchLineComment( c = new PatchLineComment(
new PatchLineComment.Key( new PatchLineComment.Key(
new Patch.Key(rsrc.getPatchSet().getId(), in.path), new Patch.Key(rsrc.getPatchSet().getId(), in.path),
c.getKey().get()), c.getKey().get()),
c.getLine(), c.getLine(),
rsrc.getAuthorId(), rsrc.getAuthorId(),
c.getParentUuid()), in); c.getParentUuid());
db.get().patchComments().insert(Collections.singleton(c)); db.get().patchComments().insert(Collections.singleton(update(c, in)));
} else { } else {
db.get().patchComments().update(Collections.singleton(update(c, in))); db.get().patchComments().update(Collections.singleton(update(c, in)));
} }
@@ -96,6 +98,9 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
if (in.line != null) { if (in.line != null) {
e.setLine(in.line); e.setLine(in.line);
} }
if (in.inReplyTo != null) {
e.setParentUuid(Url.decode(in.inReplyTo));
}
e.setMessage(in.message.trim()); e.setMessage(in.message.trim());
e.updated(); e.updated();
return e; return e;

View File

@@ -38,21 +38,27 @@ public final class Url {
* @return a string with all invalid URL characters escaped. * @return a string with all invalid URL characters escaped.
*/ */
public static String encode(String component) { public static String encode(String component) {
if (component != null) {
try { try {
return URLEncoder.encode(component, "UTF-8"); return URLEncoder.encode(component, "UTF-8");
} catch (UnsupportedEncodingException e) { } catch (UnsupportedEncodingException e) {
throw new RuntimeException("JVM must support UTF-8", e); throw new RuntimeException("JVM must support UTF-8", e);
} }
} }
return null;
}
/** Decode a URL encoded string, e.g. from {@code "%2F"} to {@code "/"}. */ /** Decode a URL encoded string, e.g. from {@code "%2F"} to {@code "/"}. */
public static String decode(String str) { public static String decode(String str) {
if (str != null) {
try { try {
return URLDecoder.decode(str, "UTF-8"); return URLDecoder.decode(str, "UTF-8");
} catch (UnsupportedEncodingException e) { } catch (UnsupportedEncodingException e) {
throw new RuntimeException("JVM must support UTF-8", e); throw new RuntimeException("JVM must support UTF-8", e);
} }
} }
return null;
}
private Url() { private Url() {
} }