opendev/gerrit
Code Issues Proposed changes

Use ObjectId instead of RevId throughout NoteDb comment code

Browse Source 
See Iff5644e2 context on removing RevId usages.

Change-Id: Ic586756132ace9d4d4c651294e4a1cf56cbb0536
This commit is contained in:
Dave Borowitz
2019-04-23 13:17:45 -07:00
parent aee58d9905
commit f2f03e989c
28 changed files with 230 additions and 228 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
java/com/google/gerrit/reviewdb/client/Comment.java
View File

@@ -21,6 +21,7 @@ import java.sql.Timestamp;
import java.util.Comparator; import java.util.Comparator;
import java.util.Objects; import java.util.Objects;
import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
/** /**
* This class represents inline comments in NoteDb. This means it determines the JSON format for * This class represents inline comments in NoteDb. This means it determines the JSON format for
@@ -196,8 +197,10 @@ public class Comment {
// Hex commit SHA1 of the commit of the patchset to which this comment applies. Other classes call // Hex commit SHA1 of the commit of the patchset to which this comment applies. Other classes call
// this "commitId", but this class uses the old ReviewDb term "revId", and this field name is // this "commitId", but this class uses the old ReviewDb term "revId", and this field name is
// serialized into JSON in NoteDb, so it can't easily be changed. // serialized into JSON in NoteDb, so it can't easily be changed. Callers do not access this field
public String revId; // directly, and instead use the public getter/setter that wraps an ObjectId.
private String revId;
public String serverId; public String serverId;
public boolean unresolved; public boolean unresolved;
@@ -254,8 +257,13 @@ public class Comment {
this.range = range != null ? range.asCommentRange() : null; this.range = range != null ? range.asCommentRange() : null;
} }
public void setRevId(@Nullable AnyObjectId revId) { @Nullable
this.revId = revId != null ? revId.name() : null; public ObjectId getCommitId() {
return revId != null ? ObjectId.fromString(revId) : null;
}
public void setCommitId(@Nullable AnyObjectId commitId) {
this.revId = commitId != null ? commitId.name() : null;
} }
public void setRealAuthor(Account.Id id) { public void setRealAuthor(Account.Id id) {

4
java/com/google/gerrit/reviewdb/client/PatchLineComment.java
View File

@@ -70,7 +70,7 @@ public final class PatchLineComment {
plc.setRange(new CommentRange(r.startLine, r.startChar, r.endLine, r.endChar)); plc.setRange(new CommentRange(r.startLine, r.startChar, r.endLine, r.endChar));
} }
plc.setTag(c.tag); plc.setTag(c.tag);
plc.setCommitId(ObjectId.fromString(c.revId)); plc.setCommitId(c.getCommitId());
plc.setStatus(status); plc.setStatus(status);
plc.setRealAuthor(c.getRealAuthor().getId()); plc.setRealAuthor(c.getRealAuthor().getId());
plc.setUnresolved(c.unresolved); plc.setUnresolved(c.unresolved);
@@ -268,7 +268,7 @@ public final class PatchLineComment {
message, message,
serverId, serverId,
unresolved); unresolved);
c.setRevId(commitId); c.setCommitId(commitId);
c.setRange(range); c.setRange(range);
c.lineNbr = lineNbr; c.lineNbr = lineNbr;
c.parentUuid = parentUuid; c.parentUuid = parentUuid;

12
java/com/google/gerrit/server/CommentsUtil.java
View File

@@ -306,22 +306,22 @@ public class CommentsUtil {
return sort(result); return sort(result);
} }
public static void setCommentRevId(Comment c, PatchListCache cache, Change change, PatchSet ps) public static void setCommentCommitId(Comment c, PatchListCache cache, Change change, PatchSet ps)
throws PatchListNotAvailableException { throws PatchListNotAvailableException {
checkArgument( checkArgument(
c.key.patchSetId == ps.getId().get(), c.key.patchSetId == ps.getId().get(),
"cannot set RevId for patch set %s on comment %s", "cannot set commit ID for patch set %s on comment %s",
ps.getId(), ps.getId(),
c); c);
if (c.revId == null) { if (c.getCommitId() == null) {
if (Side.fromShort(c.side) == Side.PARENT) { if (Side.fromShort(c.side) == Side.PARENT) {
if (c.side < 0) { if (c.side < 0) {
c.revId = ObjectId.toString(cache.getOldId(change, ps, -c.side)); c.setCommitId(cache.getOldId(change, ps, -c.side));
} else { } else {
c.revId = ObjectId.toString(cache.getOldId(change, ps, null)); c.setCommitId(cache.getOldId(change, ps, null));
} }
} else { } else {
c.revId = ps.getRevision().get(); c.setCommitId(ObjectId.fromString(ps.getRevision().get()));
} }
} }
} }

2
java/com/google/gerrit/server/PublishCommentUtil.java
View File

@@ -66,7 +66,7 @@ public class PublishCommentUtil {
// applies to X-Gerrit-RunAs, since modifying drafts via on_behalf_of is not allowed.) // applies to X-Gerrit-RunAs, since modifying drafts via on_behalf_of is not allowed.)
ctx.getUser().updateRealAccountId(d::setRealAuthor); ctx.getUser().updateRealAccountId(d::setRealAuthor);
try { try {
CommentsUtil.setCommentRevId(d, patchListCache, notes.getChange(), ps); CommentsUtil.setCommentCommitId(d, patchListCache, notes.getChange(), ps);
} catch (PatchListNotAvailableException e) { } catch (PatchListNotAvailableException e) {
throw new StorageException(e); throw new StorageException(e);
} }

2
java/com/google/gerrit/server/mail/receive/MailProcessor.java
View File

@@ -399,7 +399,7 @@ public class MailProcessor {
comment.range = mailComment.getInReplyTo().range; comment.range = mailComment.getInReplyTo().range;
comment.unresolved = mailComment.getInReplyTo().unresolved; comment.unresolved = mailComment.getInReplyTo().unresolved;
} }
CommentsUtil.setCommentRevId(comment, patchListCache, ctx.getChange(), patchSetForComment); CommentsUtil.setCommentCommitId(comment, patchListCache, ctx.getChange(), patchSetForComment);
return comment; return comment;
} }
} }

2
java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
View File

@@ -271,7 +271,7 @@ public abstract class AbstractChangeUpdate {
} }
protected void verifyComment(Comment c) { protected void verifyComment(Comment c) {
checkArgument(c.revId != null, "RevId required for comment: %s", c); checkArgument(c.getCommitId() != null, "commit ID required for comment: %s", c);
checkArgument( checkArgument(
c.author.getId().equals(getAccountId()), c.author.getId().equals(getAccountId()),
"The author for the following comment does not match the author of this %s (%s): %s", "The author for the following comment does not match the author of this %s (%s): %s",

25
java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
View File

@@ -26,7 +26,6 @@ import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.AllUsersName;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
@@ -74,13 +73,13 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
@AutoValue @AutoValue
abstract static class Key { abstract static class Key {
abstract String revId(); abstract ObjectId commitId();
abstract Comment.Key key(); abstract Comment.Key key();
} }
private static Key key(Comment c) { private static Key key(Comment c) {
return new AutoValue_ChangeDraftUpdate_Key(c.revId, c.key); return new AutoValue_ChangeDraftUpdate_Key(c.getCommitId(), c.key);
} }
private final AllUsersName draftsProject; private final AllUsersName draftsProject;
@@ -126,32 +125,32 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
delete.add(key(c)); delete.add(key(c));
} }
public void deleteComment(String revId, Comment.Key key) { public void deleteComment(ObjectId commitId, Comment.Key key) {
delete.add(new AutoValue_ChangeDraftUpdate_Key(revId, key)); delete.add(new AutoValue_ChangeDraftUpdate_Key(commitId, key));
} }
private CommitBuilder storeCommentsInNotes( private CommitBuilder storeCommentsInNotes(
RevWalk rw, ObjectInserter ins, ObjectId curr, CommitBuilder cb) RevWalk rw, ObjectInserter ins, ObjectId curr, CommitBuilder cb)
throws ConfigInvalidException, IOException { throws ConfigInvalidException, IOException {
RevisionNoteMap<ChangeRevisionNote> rnm = getRevisionNoteMap(rw, curr); RevisionNoteMap<ChangeRevisionNote> rnm = getRevisionNoteMap(rw, curr);
Set<RevId> updatedRevs = Sets.newHashSetWithExpectedSize(rnm.revisionNotes.size()); Set<ObjectId> updatedCommits = Sets.newHashSetWithExpectedSize(rnm.revisionNotes.size());
RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(rnm); RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(rnm);
for (Comment c : put) { for (Comment c : put) {
if (!delete.contains(key(c))) { if (!delete.contains(key(c))) {
cache.get(new RevId(c.revId)).putComment(c); cache.get(c.getCommitId()).putComment(c);
} }
} }
for (Key k : delete) { for (Key k : delete) {
cache.get(new RevId(k.revId())).deleteComment(k.key()); cache.get(k.commitId()).deleteComment(k.key());
} }
Map<RevId, RevisionNoteBuilder> builders = cache.getBuilders(); Map<ObjectId, RevisionNoteBuilder> builders = cache.getBuilders();
boolean touchedAnyRevs = false; boolean touchedAnyRevs = false;
boolean hasComments = false; boolean hasComments = false;
for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) { for (Map.Entry<ObjectId, RevisionNoteBuilder> e : builders.entrySet()) {
updatedRevs.add(e.getKey()); updatedCommits.add(e.getKey());
ObjectId id = ObjectId.fromString(e.getKey().get()); ObjectId id = e.getKey();
byte[] data = e.getValue().build(noteUtil.getChangeNoteJson()); byte[] data = e.getValue().build(noteUtil.getChangeNoteJson());
if (!Arrays.equals(data, e.getValue().baseRaw)) { if (!Arrays.equals(data, e.getValue().baseRaw)) {
touchedAnyRevs = true; touchedAnyRevs = true;
@@ -174,7 +173,7 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
// If we touched every revision and there are no comments left, tell the // If we touched every revision and there are no comments left, tell the
// caller to delete the entire ref. // caller to delete the entire ref.
boolean touchedAllRevs = updatedRevs.equals(rnm.revisionNotes.keySet()); boolean touchedAllRevs = updatedCommits.equals(rnm.revisionNotes.keySet());
if (touchedAllRevs && !hasComments) { if (touchedAllRevs && !hasComments) {
return null; return null;
} }

9
java/com/google/gerrit/server/notedb/ChangeNotes.java
View File

@@ -46,7 +46,6 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.client.RobotComment; import com.google.gerrit.reviewdb.client.RobotComment;
import com.google.gerrit.server.ReviewerByEmailSet; import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.ReviewerSet;
@@ -403,7 +402,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
} }
/** @return inline comments on each revision. */ /** @return inline comments on each revision. */
public ImmutableListMultimap<RevId, Comment> getComments() { public ImmutableListMultimap<ObjectId, Comment> getComments() {
return state.publishedComments(); return state.publishedComments();
} }
@@ -422,11 +421,11 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
return state.updateCount(); return state.updateCount();
} }
public ImmutableListMultimap<RevId, Comment> getDraftComments(Account.Id author) { public ImmutableListMultimap<ObjectId, Comment> getDraftComments(Account.Id author) {
return getDraftComments(author, null); return getDraftComments(author, null);
} }
public ImmutableListMultimap<RevId, Comment> getDraftComments( public ImmutableListMultimap<ObjectId, Comment> getDraftComments(
Account.Id author, @Nullable Ref ref) { Account.Id author, @Nullable Ref ref) {
loadDraftComments(author, ref); loadDraftComments(author, ref);
// Filter out any zombie draft comments. These are drafts that are also in // Filter out any zombie draft comments. These are drafts that are also in
@@ -437,7 +436,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
draftCommentNotes.getComments(), e -> !getCommentKeys().contains(e.getValue().key))); draftCommentNotes.getComments(), e -> !getCommentKeys().contains(e.getValue().key)));
} }
public ImmutableListMultimap<RevId, RobotComment> getRobotComments() { public ImmutableListMultimap<ObjectId, RobotComment> getRobotComments() {
loadRobotComments(); loadRobotComments();
return robotCommentNotes.getComments(); return robotCommentNotes.getComments();
} }

8
java/com/google/gerrit/server/notedb/ChangeNotesParser.java
View File

@@ -134,7 +134,7 @@ class ChangeNotesParser {
private final List<Account.Id> allPastReviewers; private final List<Account.Id> allPastReviewers;
private final List<ReviewerStatusUpdate> reviewerUpdates; private final List<ReviewerStatusUpdate> reviewerUpdates;
private final List<SubmitRecord> submitRecords; private final List<SubmitRecord> submitRecords;
private final ListMultimap<RevId, Comment> comments; private final ListMultimap<ObjectId, Comment> comments;
private final Map<PatchSet.Id, PatchSet> patchSets; private final Map<PatchSet.Id, PatchSet> patchSets;
private final Set<PatchSet.Id> deletedPatchSets; private final Set<PatchSet.Id> deletedPatchSets;
private final Map<PatchSet.Id, PatchSetState> patchSetStates; private final Map<PatchSet.Id, PatchSetState> patchSetStates;
@@ -721,16 +721,16 @@ class ChangeNotesParser {
reader, reader,
NoteMap.read(reader, tipCommit), NoteMap.read(reader, tipCommit),
PatchLineComment.Status.PUBLISHED); PatchLineComment.Status.PUBLISHED);
Map<RevId, ChangeRevisionNote> rns = revisionNoteMap.revisionNotes; Map<ObjectId, ChangeRevisionNote> rns = revisionNoteMap.revisionNotes;
for (Map.Entry<RevId, ChangeRevisionNote> e : rns.entrySet()) { for (Map.Entry<ObjectId, ChangeRevisionNote> e : rns.entrySet()) {
for (Comment c : e.getValue().getEntities()) { for (Comment c : e.getValue().getEntities()) {
comments.put(e.getKey(), c); comments.put(e.getKey(), c);
} }
} }
for (PatchSet ps : patchSets.values()) { for (PatchSet ps : patchSets.values()) {
ChangeRevisionNote rn = rns.get(ps.getRevision()); ChangeRevisionNote rn = rns.get(ObjectId.fromString(ps.getRevision().get()));
if (rn != null && rn.getPushCert() != null) { if (rn != null && rn.getPushCert() != null) {
ps.setPushCertificate(rn.getPushCert()); ps.setPushCertificate(rn.getPushCert());
} }

9
java/com/google/gerrit/server/notedb/ChangeNotesState.java
View File

@@ -46,7 +46,6 @@ import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.converter.ChangeMessageProtoConverter; import com.google.gerrit.reviewdb.converter.ChangeMessageProtoConverter;
import com.google.gerrit.reviewdb.converter.PatchSetApprovalProtoConverter; import com.google.gerrit.reviewdb.converter.PatchSetApprovalProtoConverter;
import com.google.gerrit.reviewdb.converter.PatchSetProtoConverter; import com.google.gerrit.reviewdb.converter.PatchSetProtoConverter;
@@ -117,7 +116,7 @@ public abstract class ChangeNotesState {
List<ReviewerStatusUpdate> reviewerUpdates, List<ReviewerStatusUpdate> reviewerUpdates,
List<SubmitRecord> submitRecords, List<SubmitRecord> submitRecords,
List<ChangeMessage> changeMessages, List<ChangeMessage> changeMessages,
ListMultimap<RevId, Comment> publishedComments, ListMultimap<ObjectId, Comment> publishedComments,
boolean isPrivate, boolean isPrivate,
boolean workInProgress, boolean workInProgress,
boolean reviewStarted, boolean reviewStarted,
@@ -297,7 +296,7 @@ public abstract class ChangeNotesState {
abstract ImmutableList<ChangeMessage> changeMessages(); abstract ImmutableList<ChangeMessage> changeMessages();
abstract ImmutableListMultimap<RevId, Comment> publishedComments(); abstract ImmutableListMultimap<ObjectId, Comment> publishedComments();
abstract int updateCount(); abstract int updateCount();
@@ -401,7 +400,7 @@ public abstract class ChangeNotesState {
abstract Builder changeMessages(List<ChangeMessage> changeMessages); abstract Builder changeMessages(List<ChangeMessage> changeMessages);
abstract Builder publishedComments(ListMultimap<RevId, Comment> publishedComments); abstract Builder publishedComments(ListMultimap<ObjectId, Comment> publishedComments);
abstract Builder updateCount(int updateCount); abstract Builder updateCount(int updateCount);
@@ -581,7 +580,7 @@ public abstract class ChangeNotesState {
.publishedComments( .publishedComments(
proto.getPublishedCommentList().stream() proto.getPublishedCommentList().stream()
.map(r -> GSON.fromJson(r, Comment.class)) .map(r -> GSON.fromJson(r, Comment.class))
.collect(toImmutableListMultimap(c -> new RevId(c.revId), c -> c))) .collect(toImmutableListMultimap(Comment::getCommitId, c -> c)))
.updateCount(proto.getUpdateCount()); .updateCount(proto.getUpdateCount());
return b.build(); return b.build();
} }

16
java/com/google/gerrit/server/notedb/ChangeUpdate.java
View File

@@ -57,7 +57,6 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Comment; import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.client.RobotComment; import com.google.gerrit.reviewdb.client.RobotComment;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdent;
@@ -432,18 +431,18 @@ public class ChangeUpdate extends AbstractChangeUpdate {
RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(rnm); RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(rnm);
for (Comment c : comments) { for (Comment c : comments) {
c.tag = tag; c.tag = tag;
cache.get(new RevId(c.revId)).putComment(c); cache.get(c.getCommitId()).putComment(c);
} }
if (pushCert != null) { if (pushCert != null) {
checkState(commit != null); checkState(commit != null);
cache.get(new RevId(commit)).setPushCertificate(pushCert); cache.get(ObjectId.fromString(commit)).setPushCertificate(pushCert);
} }
Map<RevId, RevisionNoteBuilder> builders = cache.getBuilders(); Map<ObjectId, RevisionNoteBuilder> builders = cache.getBuilders();
checkComments(rnm.revisionNotes, builders); checkComments(rnm.revisionNotes, builders);
for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) { for (Map.Entry<ObjectId, RevisionNoteBuilder> e : builders.entrySet()) {
ObjectId data = inserter.insert(OBJ_BLOB, e.getValue().build(noteUtil.getChangeNoteJson())); ObjectId data = inserter.insert(OBJ_BLOB, e.getValue().build(noteUtil.getChangeNoteJson()));
rnm.noteMap.set(ObjectId.fromString(e.getKey().get()), data); rnm.noteMap.set(e.getKey(), data);
} }
return rnm.noteMap.writeTree(inserter); return rnm.noteMap.writeTree(inserter);
@@ -476,7 +475,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
} }
private void checkComments( private void checkComments(
Map<RevId, ChangeRevisionNote> existingNotes, Map<RevId, RevisionNoteBuilder> toUpdate) { Map<ObjectId, ChangeRevisionNote> existingNotes,
Map<ObjectId, RevisionNoteBuilder> toUpdate) {
// Prohibit various kinds of illegal operations on comments. // Prohibit various kinds of illegal operations on comments.
Set<Comment.Key> existing = new HashSet<>(); Set<Comment.Key> existing = new HashSet<>();
for (ChangeRevisionNote rn : existingNotes.values()) { for (ChangeRevisionNote rn : existingNotes.values()) {
@@ -498,7 +498,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
// separate commit. But note that we don't care much about the commit // separate commit. But note that we don't care much about the commit
// graph of the draft ref, particularly because the ref is completely // graph of the draft ref, particularly because the ref is completely
// deleted when all drafts are gone. // deleted when all drafts are gone.
draftUpdate.deleteComment(c.revId, c.key); draftUpdate.deleteComment(c.getCommitId(), c.key);
} }
} }
} }

11
java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
View File

@@ -24,7 +24,6 @@ import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Comment; import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
import java.io.IOException; import java.io.IOException;
@@ -224,16 +223,16 @@ public class DeleteCommentRewriter implements NoteDbRewriter {
RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(revNotesMap); RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(revNotesMap);
for (Comment c : putInComments) { for (Comment c : putInComments) {
cache.get(new RevId(c.revId)).putComment(c); cache.get(c.getCommitId()).putComment(c);
} }
for (Comment c : deletedComments) { for (Comment c : deletedComments) {
cache.get(new RevId(c.revId)).deleteComment(c.key); cache.get(c.getCommitId()).deleteComment(c.key);
} }
Map<RevId, RevisionNoteBuilder> builders = cache.getBuilders(); Map<ObjectId, RevisionNoteBuilder> builders = cache.getBuilders();
for (Map.Entry<RevId, RevisionNoteBuilder> entry : builders.entrySet()) { for (Map.Entry<ObjectId, RevisionNoteBuilder> entry : builders.entrySet()) {
ObjectId objectId = ObjectId.fromString(entry.getKey().get()); ObjectId objectId = entry.getKey();
byte[] data = entry.getValue().build(noteUtil.getChangeNoteJson()); byte[] data = entry.getValue().build(noteUtil.getChangeNoteJson());
if (data.length == 0) { if (data.length == 0) {
revNotesMap.noteMap.remove(objectId); revNotesMap.noteMap.remove(objectId);

9
java/com/google/gerrit/server/notedb/DraftCommentNotes.java
View File

@@ -29,7 +29,6 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Comment; import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject; import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException; import java.io.IOException;
@@ -52,7 +51,7 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
private final Account.Id author; private final Account.Id author;
private final Ref ref; private final Ref ref;
private ImmutableListMultimap<RevId, Comment> comments; private ImmutableListMultimap<ObjectId, Comment> comments;
private RevisionNoteMap<ChangeRevisionNote> revisionNoteMap; private RevisionNoteMap<ChangeRevisionNote> revisionNoteMap;
@AssistedInject @AssistedInject
@@ -82,7 +81,7 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
return author; return author;
} }
public ImmutableListMultimap<RevId, Comment> getComments() { public ImmutableListMultimap<ObjectId, Comment> getComments() {
return comments; return comments;
} }
@@ -128,10 +127,10 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
reader, reader,
NoteMap.read(reader, tipCommit), NoteMap.read(reader, tipCommit),
PatchLineComment.Status.DRAFT); PatchLineComment.Status.DRAFT);
ListMultimap<RevId, Comment> cs = MultimapBuilder.hashKeys().arrayListValues().build(); ListMultimap<ObjectId, Comment> cs = MultimapBuilder.hashKeys().arrayListValues().build();
for (ChangeRevisionNote rn : revisionNoteMap.revisionNotes.values()) { for (ChangeRevisionNote rn : revisionNoteMap.revisionNotes.values()) {
for (Comment c : rn.getEntities()) { for (Comment c : rn.getEntities()) {
cs.put(new RevId(c.revId), c); cs.put(c.getCommitId(), c);
} }
} }
comments = ImmutableListMultimap.copyOf(cs); comments = ImmutableListMultimap.copyOf(cs);

2
java/com/google/gerrit/server/notedb/LegacyChangeNoteRead.java
View File

@@ -190,7 +190,7 @@ public class LegacyChangeNoteRead {
c.lineNbr = range.getEndLine(); c.lineNbr = range.getEndLine();
c.parentUuid = parentUUID; c.parentUuid = parentUUID;
c.tag = tag; c.tag = tag;
c.setRevId(commitId); c.setCommitId(commitId);
if (raId != null) { if (raId != null) {
c.setRealAuthor(raId); c.setRealAuthor(raId);
} }

10
java/com/google/gerrit/server/notedb/LegacyChangeNoteWrite.java
View File

@@ -34,6 +34,7 @@ import java.io.PrintWriter;
import java.sql.Timestamp; import java.sql.Timestamp;
import java.util.Date; import java.util.Date;
import java.util.List; import java.util.List;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.util.QuotedString; import org.eclipse.jgit.util.QuotedString;
@@ -91,8 +92,9 @@ public class LegacyChangeNoteWrite {
OutputStreamWriter streamWriter = new OutputStreamWriter(out, UTF_8); OutputStreamWriter streamWriter = new OutputStreamWriter(out, UTF_8);
try (PrintWriter writer = new PrintWriter(streamWriter)) { try (PrintWriter writer = new PrintWriter(streamWriter)) {
String revId = comments.values().iterator().next().revId; ObjectId commitId = comments.values().iterator().next().getCommitId();
appendHeaderField(writer, ChangeNoteUtil.REVISION, revId); String commitName = commitId.name();
appendHeaderField(writer, ChangeNoteUtil.REVISION, commitName);
for (int psId : psIds) { for (int psId : psIds) {
List<Comment> psComments = COMMENT_ORDER.sortedCopy(comments.get(psId)); List<Comment> psComments = COMMENT_ORDER.sortedCopy(comments.get(psId));
@@ -111,11 +113,11 @@ public class LegacyChangeNoteWrite {
for (Comment c : psComments) { for (Comment c : psComments) {
checkArgument( checkArgument(
revId.equals(c.revId), commitId.equals(c.getCommitId()),
"All comments being added must have all the same RevId. The " "All comments being added must have all the same RevId. The "
+ "comment below does not have the same RevId as the others " + "comment below does not have the same RevId as the others "
+ "(%s).\n%s", + "(%s).\n%s",
revId, commitId,
c); c);
checkArgument( checkArgument(
side == c.side, side == c.side,

15
java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
View File

@@ -22,7 +22,6 @@ import com.google.common.collect.ListMultimap;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.common.collect.MultimapBuilder; import com.google.common.collect.MultimapBuilder;
import com.google.gerrit.reviewdb.client.Comment; import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.RevId;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
import java.io.IOException; import java.io.IOException;
import java.io.OutputStream; import java.io.OutputStream;
@@ -33,27 +32,29 @@ import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
class RevisionNoteBuilder { class RevisionNoteBuilder {
static class Cache { static class Cache {
private final RevisionNoteMap<? extends RevisionNote<? extends Comment>> revisionNoteMap; private final RevisionNoteMap<? extends RevisionNote<? extends Comment>> revisionNoteMap;
private final Map<RevId, RevisionNoteBuilder> builders; private final Map<ObjectId, RevisionNoteBuilder> builders;
Cache(RevisionNoteMap<? extends RevisionNote<? extends Comment>> revisionNoteMap) { Cache(RevisionNoteMap<? extends RevisionNote<? extends Comment>> revisionNoteMap) {
this.revisionNoteMap = revisionNoteMap; this.revisionNoteMap = revisionNoteMap;
this.builders = new HashMap<>(); this.builders = new HashMap<>();
} }
RevisionNoteBuilder get(RevId revId) { RevisionNoteBuilder get(AnyObjectId commitId) {
RevisionNoteBuilder b = builders.get(revId); RevisionNoteBuilder b = builders.get(commitId);
if (b == null) { if (b == null) {
b = new RevisionNoteBuilder(revisionNoteMap.revisionNotes.get(revId)); b = new RevisionNoteBuilder(revisionNoteMap.revisionNotes.get(commitId));
builders.put(revId, b); builders.put(commitId.copy(), b);
} }
return b; return b;
} }
Map<RevId, RevisionNoteBuilder> getBuilders() { Map<ObjectId, RevisionNoteBuilder> getBuilders() {
return Collections.unmodifiableMap(builders); return Collections.unmodifiableMap(builders);
} }
} }

14
java/com/google/gerrit/server/notedb/RevisionNoteMap.java
View File

@@ -18,18 +18,18 @@ import com.google.common.collect.ImmutableMap;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Comment; import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.RevId;
import java.io.IOException; import java.io.IOException;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.notes.Note; import org.eclipse.jgit.notes.Note;
import org.eclipse.jgit.notes.NoteMap; import org.eclipse.jgit.notes.NoteMap;
class RevisionNoteMap<T extends RevisionNote<? extends Comment>> { class RevisionNoteMap<T extends RevisionNote<? extends Comment>> {
final NoteMap noteMap; final NoteMap noteMap;
final ImmutableMap<RevId, T> revisionNotes; final ImmutableMap<ObjectId, T> revisionNotes;
static RevisionNoteMap<ChangeRevisionNote> parse( static RevisionNoteMap<ChangeRevisionNote> parse(
ChangeNoteJson noteJson, ChangeNoteJson noteJson,
@@ -39,13 +39,13 @@ class RevisionNoteMap<T extends RevisionNote<? extends Comment>> {
NoteMap noteMap, NoteMap noteMap,
PatchLineComment.Status status) PatchLineComment.Status status)
throws ConfigInvalidException, IOException { throws ConfigInvalidException, IOException {
Map<RevId, ChangeRevisionNote> result = new HashMap<>(); Map<ObjectId, ChangeRevisionNote> result = new HashMap<>();
for (Note note : noteMap) { for (Note note : noteMap) {
ChangeRevisionNote rn = ChangeRevisionNote rn =
new ChangeRevisionNote( new ChangeRevisionNote(
noteJson, legacyChangeNoteRead, changeId, reader, note.getData(), status); noteJson, legacyChangeNoteRead, changeId, reader, note.getData(), status);
rn.parse(); rn.parse();
result.put(new RevId(note.name()), rn); result.put(note.copy(), rn);
} }
return new RevisionNoteMap<>(noteMap, ImmutableMap.copyOf(result)); return new RevisionNoteMap<>(noteMap, ImmutableMap.copyOf(result));
} }
@@ -53,12 +53,12 @@ class RevisionNoteMap<T extends RevisionNote<? extends Comment>> {
static RevisionNoteMap<RobotCommentsRevisionNote> parseRobotComments( static RevisionNoteMap<RobotCommentsRevisionNote> parseRobotComments(
ChangeNoteJson changeNoteJson, ObjectReader reader, NoteMap noteMap) ChangeNoteJson changeNoteJson, ObjectReader reader, NoteMap noteMap)
throws ConfigInvalidException, IOException { throws ConfigInvalidException, IOException {
Map<RevId, RobotCommentsRevisionNote> result = new HashMap<>(); Map<ObjectId, RobotCommentsRevisionNote> result = new HashMap<>();
for (Note note : noteMap) { for (Note note : noteMap) {
RobotCommentsRevisionNote rn = RobotCommentsRevisionNote rn =
new RobotCommentsRevisionNote(changeNoteJson, reader, note.getData()); new RobotCommentsRevisionNote(changeNoteJson, reader, note.getData());
rn.parse(); rn.parse();
result.put(new RevId(note.name()), rn); result.put(note.copy(), rn);
} }
return new RevisionNoteMap<>(noteMap, ImmutableMap.copyOf(result)); return new RevisionNoteMap<>(noteMap, ImmutableMap.copyOf(result));
} }
@@ -67,7 +67,7 @@ class RevisionNoteMap<T extends RevisionNote<? extends Comment>> {
return new RevisionNoteMap<>(NoteMap.newEmptyMap(), ImmutableMap.of()); return new RevisionNoteMap<>(NoteMap.newEmptyMap(), ImmutableMap.of());
} }
private RevisionNoteMap(NoteMap noteMap, ImmutableMap<RevId, T> revisionNotes) { private RevisionNoteMap(NoteMap noteMap, ImmutableMap<ObjectId, T> revisionNotes) {
this.noteMap = noteMap; this.noteMap = noteMap;
this.revisionNotes = revisionNotes; this.revisionNotes = revisionNotes;
} }

9
java/com/google/gerrit/server/notedb/RobotCommentNotes.java
View File

@@ -22,7 +22,6 @@ import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.client.RobotComment; import com.google.gerrit.reviewdb.client.RobotComment;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
@@ -42,7 +41,7 @@ public class RobotCommentNotes extends AbstractChangeNotes<RobotCommentNotes> {
private final Change change; private final Change change;
private ImmutableListMultimap<RevId, RobotComment> comments; private ImmutableListMultimap<ObjectId, RobotComment> comments;
private RevisionNoteMap<RobotCommentsRevisionNote> revisionNoteMap; private RevisionNoteMap<RobotCommentsRevisionNote> revisionNoteMap;
private ObjectId metaId; private ObjectId metaId;
@@ -56,7 +55,7 @@ public class RobotCommentNotes extends AbstractChangeNotes<RobotCommentNotes> {
return revisionNoteMap; return revisionNoteMap;
} }
public ImmutableListMultimap<RevId, RobotComment> getComments() { public ImmutableListMultimap<ObjectId, RobotComment> getComments() {
return comments; return comments;
} }
@@ -95,10 +94,10 @@ public class RobotCommentNotes extends AbstractChangeNotes<RobotCommentNotes> {
revisionNoteMap = revisionNoteMap =
RevisionNoteMap.parseRobotComments( RevisionNoteMap.parseRobotComments(
args.changeNoteJson, reader, NoteMap.read(reader, tipCommit)); args.changeNoteJson, reader, NoteMap.read(reader, tipCommit));
ListMultimap<RevId, RobotComment> cs = MultimapBuilder.hashKeys().arrayListValues().build(); ListMultimap<ObjectId, RobotComment> cs = MultimapBuilder.hashKeys().arrayListValues().build();
for (RobotCommentsRevisionNote rn : revisionNoteMap.revisionNotes.values()) { for (RobotCommentsRevisionNote rn : revisionNoteMap.revisionNotes.values()) {
for (RobotComment c : rn.getEntities()) { for (RobotComment c : rn.getEntities()) {
cs.put(new RevId(c.revId), c); cs.put(c.getCommitId(), c);
} }
} }
comments = ImmutableListMultimap.copyOf(cs); comments = ImmutableListMultimap.copyOf(cs);

13
java/com/google/gerrit/server/notedb/RobotCommentUpdate.java
View File

@@ -23,7 +23,6 @@ import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.client.RobotComment; import com.google.gerrit.reviewdb.client.RobotComment;
import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdent;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
@@ -103,19 +102,19 @@ public class RobotCommentUpdate extends AbstractChangeUpdate {
RevWalk rw, ObjectInserter ins, ObjectId curr, CommitBuilder cb) RevWalk rw, ObjectInserter ins, ObjectId curr, CommitBuilder cb)
throws ConfigInvalidException, IOException { throws ConfigInvalidException, IOException {
RevisionNoteMap<RobotCommentsRevisionNote> rnm = getRevisionNoteMap(rw, curr); RevisionNoteMap<RobotCommentsRevisionNote> rnm = getRevisionNoteMap(rw, curr);
Set<RevId> updatedRevs = Sets.newHashSetWithExpectedSize(rnm.revisionNotes.size()); Set<ObjectId> updatedRevs = Sets.newHashSetWithExpectedSize(rnm.revisionNotes.size());
RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(rnm); RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(rnm);
for (RobotComment c : put) { for (RobotComment c : put) {
cache.get(new RevId(c.revId)).putComment(c); cache.get(c.getCommitId()).putComment(c);
} }
Map<RevId, RevisionNoteBuilder> builders = cache.getBuilders(); Map<ObjectId, RevisionNoteBuilder> builders = cache.getBuilders();
boolean touchedAnyRevs = false; boolean touchedAnyRevs = false;
boolean hasComments = false; boolean hasComments = false;
for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) { for (Map.Entry<ObjectId, RevisionNoteBuilder> e : builders.entrySet()) {
updatedRevs.add(e.getKey()); ObjectId id = e.getKey();
ObjectId id = ObjectId.fromString(e.getKey().get()); updatedRevs.add(id);
byte[] data = e.getValue().build(noteUtil); byte[] data = e.getValue().build(noteUtil);
if (!Arrays.equals(data, e.getValue().baseRaw)) { if (!Arrays.equals(data, e.getValue().baseRaw)) {
touchedAnyRevs = true; touchedAnyRevs = true;

4
java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
View File

@@ -15,7 +15,7 @@
package com.google.gerrit.server.restapi.account; package com.google.gerrit.server.restapi.account;
import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.server.CommentsUtil.setCommentRevId; import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
import com.google.common.base.CharMatcher; import com.google.common.base.CharMatcher;
import com.google.common.base.Strings; import com.google.common.base.Strings;
@@ -181,7 +181,7 @@ public class DeleteDraftComments
for (Comment c : commentsUtil.draftByChangeAuthor(ctx.getNotes(), accountId)) { for (Comment c : commentsUtil.draftByChangeAuthor(ctx.getNotes(), accountId)) {
dirty = true; dirty = true;
PatchSet.Id psId = PatchSet.id(ctx.getChange().getId(), c.key.patchSetId); PatchSet.Id psId = PatchSet.id(ctx.getChange().getId(), c.key.patchSetId);
setCommentRevId(c, patchListCache, ctx.getChange(), psUtil.get(ctx.getNotes(), psId)); setCommentCommitId(c, patchListCache, ctx.getChange(), psUtil.get(ctx.getNotes(), psId));
commentsUtil.deleteComments(ctx.getUpdate(psId), Collections.singleton(c)); commentsUtil.deleteComments(ctx.getUpdate(psId), Collections.singleton(c));
comments.add(commentFormatter.format(c)); comments.add(commentFormatter.format(c));
} }

4
java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
View File

@@ -14,7 +14,7 @@
package com.google.gerrit.server.restapi.change; package com.google.gerrit.server.restapi.change;
import static com.google.gerrit.server.CommentsUtil.setCommentRevId; import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.gerrit.extensions.api.changes.DraftInput; import com.google.gerrit.extensions.api.changes.DraftInput;
@@ -119,7 +119,7 @@ public class CreateDraftComment
comment.setLineNbrAndRange(in.line, in.range); comment.setLineNbrAndRange(in.line, in.range);
comment.tag = in.tag; comment.tag = in.tag;
setCommentRevId(comment, patchListCache, ctx.getChange(), ps); setCommentCommitId(comment, patchListCache, ctx.getChange(), ps);
commentsUtil.putComments(ctx.getUpdate(psId), Status.DRAFT, Collections.singleton(comment)); commentsUtil.putComments(ctx.getUpdate(psId), Status.DRAFT, Collections.singleton(comment));
return true; return true;

4
java/com/google/gerrit/server/restapi/change/DeleteDraftComment.java
View File

@@ -14,7 +14,7 @@
package com.google.gerrit.server.restapi.change; package com.google.gerrit.server.restapi.change;
import static com.google.gerrit.server.CommentsUtil.setCommentRevId; import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
import com.google.gerrit.extensions.common.CommentInfo; import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.Input; import com.google.gerrit.extensions.common.Input;
@@ -94,7 +94,7 @@ public class DeleteDraftComment
throw new ResourceNotFoundException("patch set not found: " + psId); throw new ResourceNotFoundException("patch set not found: " + psId);
} }
Comment c = maybeComment.get(); Comment c = maybeComment.get();
setCommentRevId(c, patchListCache, ctx.getChange(), ps); setCommentCommitId(c, patchListCache, ctx.getChange(), ps);
commentsUtil.deleteComments(ctx.getUpdate(psId), Collections.singleton(c)); commentsUtil.deleteComments(ctx.getUpdate(psId), Collections.singleton(c));
return true; return true;
} }

6
java/com/google/gerrit/server/restapi/change/PostReview.java
View File

@@ -16,7 +16,7 @@ package com.google.gerrit.server.restapi.change;
import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.CommentsUtil.setCommentRevId; import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER; import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import static com.google.gerrit.server.permissions.LabelPermission.ForUser.ON_BEHALF_OF; import static com.google.gerrit.server.permissions.LabelPermission.ForUser.ON_BEHALF_OF;
import static java.nio.charset.StandardCharsets.UTF_8; import static java.nio.charset.StandardCharsets.UTF_8;
@@ -921,7 +921,7 @@ public class PostReview
e.message = c.message; e.message = c.message;
} }
setCommentRevId(e, patchListCache, ctx.getChange(), ps); setCommentCommitId(e, patchListCache, ctx.getChange(), ps);
e.setLineNbrAndRange(c.line, c.range); e.setLineNbrAndRange(c.line, c.range);
e.tag = in.tag; e.tag = in.tag;
@@ -996,7 +996,7 @@ public class PostReview
robotComment.properties = robotCommentInput.properties; robotComment.properties = robotCommentInput.properties;
robotComment.setLineNbrAndRange(robotCommentInput.line, robotCommentInput.range); robotComment.setLineNbrAndRange(robotCommentInput.line, robotCommentInput.range);
robotComment.tag = in.tag; robotComment.tag = in.tag;
setCommentRevId(robotComment, patchListCache, ctx.getChange(), ps); setCommentCommitId(robotComment, patchListCache, ctx.getChange(), ps);
robotComment.fixSuggestions = createFixSuggestionsFromInput(robotCommentInput.fixSuggestions); robotComment.fixSuggestions = createFixSuggestionsFromInput(robotCommentInput.fixSuggestions);
return robotComment; return robotComment;
} }

4
java/com/google/gerrit/server/restapi/change/PutDraftComment.java
View File

@@ -14,7 +14,7 @@
package com.google.gerrit.server.restapi.change; package com.google.gerrit.server.restapi.change;
import static com.google.gerrit.server.CommentsUtil.setCommentRevId; import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
import com.google.gerrit.extensions.api.changes.DraftInput; import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.common.CommentInfo; import com.google.gerrit.extensions.common.CommentInfo;
@@ -138,7 +138,7 @@ public class PutDraftComment
commentsUtil.deleteComments(update, Collections.singleton(origComment)); commentsUtil.deleteComments(update, Collections.singleton(origComment));
comment.key.filename = in.path; comment.key.filename = in.path;
} }
setCommentRevId(comment, patchListCache, ctx.getChange(), ps); setCommentCommitId(comment, patchListCache, ctx.getChange(), ps);
commentsUtil.putComments( commentsUtil.putComments(
update, Status.DRAFT, Collections.singleton(update(comment, in, ctx.getWhen()))); update, Status.DRAFT, Collections.singleton(update(comment, in, ctx.getWhen())));
return true; return true;

5
javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
View File

@@ -66,6 +66,7 @@ import java.util.TimeZone;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.revwalk.RevWalk;
import org.junit.After; import org.junit.After;
@@ -247,7 +248,7 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
Timestamp t, Timestamp t,
String message, String message,
short side, short side,
String commitSHA1, ObjectId commitId,
boolean unresolved) { boolean unresolved) {
Comment c = Comment c =
new Comment( new Comment(
@@ -260,7 +261,7 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
unresolved); unresolved);
c.lineNbr = line; c.lineNbr = line;
c.parentUuid = parentUUID; c.parentUuid = parentUUID;
c.revId = commitSHA1; c.setCommitId(commitId);
c.setRange(range); c.setRange(range);
return c; return c;
} }

9
javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
View File

@@ -663,7 +663,7 @@ public class ChangeNotesStateTest extends GerritBaseTests {
"message 1", "message 1",
"serverId", "serverId",
false); false);
c1.setRevId(ObjectId.fromString("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")); c1.setCommitId(ObjectId.fromString("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"));
String c1Json = Serializer.GSON.toJson(c1); String c1Json = Serializer.GSON.toJson(c1);
Comment c2 = Comment c2 =
@@ -675,13 +675,12 @@ public class ChangeNotesStateTest extends GerritBaseTests {
"message 2", "message 2",
"serverId", "serverId",
true); true);
c2.setRevId(ObjectId.fromString("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")); c2.setCommitId(ObjectId.fromString("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"));
String c2Json = Serializer.GSON.toJson(c2); String c2Json = Serializer.GSON.toJson(c2);
assertRoundTrip( assertRoundTrip(
newBuilder() newBuilder()
.publishedComments( .publishedComments(ImmutableListMultimap.of(c2.getCommitId(), c2, c1.getCommitId(), c1))
ImmutableListMultimap.of(new RevId(c2.revId), c2, new RevId(c1.revId), c1))
.build(), .build(),
ChangeNotesStateProto.newBuilder() ChangeNotesStateProto.newBuilder()
.setMetaId(SHA_BYTES) .setMetaId(SHA_BYTES)
@@ -733,7 +732,7 @@ public class ChangeNotesStateTest extends GerritBaseTests {
.put("changeMessages", new TypeLiteral<ImmutableList<ChangeMessage>>() {}.getType()) .put("changeMessages", new TypeLiteral<ImmutableList<ChangeMessage>>() {}.getType())
.put( .put(
"publishedComments", "publishedComments",
new TypeLiteral<ImmutableListMultimap<RevId, Comment>>() {}.getType()) new TypeLiteral<ImmutableListMultimap<ObjectId, Comment>>() {}.getType())
.put("updateCount", int.class) .put("updateCount", int.class)
.build()); .build());
} }

231
javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
View File

@@ -47,7 +47,6 @@ import com.google.gerrit.reviewdb.client.CommentRange;
import com.google.gerrit.reviewdb.client.PatchLineComment.Status; import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.ReviewerSet;
@@ -132,14 +131,14 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
TimeUtil.nowTs(), TimeUtil.nowTs(),
"Comment", "Comment",
(short) 1, (short) 1,
commit.name(), commit,
false)); false));
update.setTag(tag); update.setTag(tag);
update.commit(); update.commit();
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
ImmutableListMultimap<RevId, Comment> comments = notes.getComments(); ImmutableListMultimap<ObjectId, Comment> comments = notes.getComments();
assertThat(comments).hasSize(1); assertThat(comments).hasSize(1);
assertThat(comments.entries().asList().get(0).getValue().tag).isEqualTo(tag); assertThat(comments.entries().asList().get(0).getValue().tag).isEqualTo(tag);
} }
@@ -194,7 +193,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
TimeUtil.nowTs(), TimeUtil.nowTs(),
"Comment", "Comment",
(short) 1, (short) 1,
commit.name(), commit,
false)); false));
update.setChangeMessage("coverage verification"); update.setChangeMessage("coverage verification");
update.setTag(coverageTag); update.setTag(coverageTag);
@@ -213,7 +212,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(approval.getTag()).isEqualTo(integrationTag); assertThat(approval.getTag()).isEqualTo(integrationTag);
assertThat(approval.getValue()).isEqualTo(-1); assertThat(approval.getValue()).isEqualTo(-1);
ImmutableListMultimap<RevId, Comment> comments = notes.getComments(); ImmutableListMultimap<ObjectId, Comment> comments = notes.getComments();
assertThat(comments).hasSize(1); assertThat(comments).hasSize(1);
assertThat(comments.entries().asList().get(0).getValue().tag).isEqualTo(coverageTag); assertThat(comments.entries().asList().get(0).getValue().tag).isEqualTo(coverageTag);
@@ -1066,7 +1065,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
TimeUtil.nowTs(), TimeUtil.nowTs(),
"Comment", "Comment",
(short) 1, (short) 1,
commit.name(), commit,
false)); false));
update.commit(); update.commit();
@@ -1166,7 +1165,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ts, ts,
"Comment", "Comment",
(short) 1, (short) 1,
commit.name(), commit,
false)); false));
update.commit(); update.commit();
@@ -1236,7 +1235,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
time1, time1,
message1, message1,
(short) 0, (short) 0,
"abcd1234abcd1234abcd1234abcd1234abcd1234", ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
false); false);
update1.setPatchSetId(psId); update1.setPatchSetId(psId);
update1.putComment(Status.PUBLISHED, comment1); update1.putComment(Status.PUBLISHED, comment1);
@@ -1446,7 +1445,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser); ChangeUpdate update = newUpdate(c, otherUser);
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
RevId revId = new RevId("abcd1234abcd1234abcd1234abcd1234abcd1234"); ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
Comment comment = Comment comment =
newComment( newComment(
@@ -1460,14 +1459,14 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
TimeUtil.nowTs(), TimeUtil.nowTs(),
"message", "message",
(short) 1, (short) 1,
revId.get(), commitId,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment); update.putComment(Status.PUBLISHED, comment);
update.commit(); update.commit();
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(revId, comment)); assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
} }
@Test @Test
@@ -1475,7 +1474,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser); ChangeUpdate update = newUpdate(c, otherUser);
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
RevId revId = new RevId("abcd1234abcd1234abcd1234abcd1234abcd1234"); ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
CommentRange range = new CommentRange(1, 0, 2, 0); CommentRange range = new CommentRange(1, 0, 2, 0);
Comment comment = Comment comment =
@@ -1490,14 +1489,14 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
TimeUtil.nowTs(), TimeUtil.nowTs(),
"message", "message",
(short) 1, (short) 1,
revId.get(), commitId,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment); update.putComment(Status.PUBLISHED, comment);
update.commit(); update.commit();
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(revId, comment)); assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
} }
@Test @Test
@@ -1505,7 +1504,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser); ChangeUpdate update = newUpdate(c, otherUser);
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
RevId revId = new RevId("abcd1234abcd1234abcd1234abcd1234abcd1234"); ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
CommentRange range = new CommentRange(0, 0, 0, 0); CommentRange range = new CommentRange(0, 0, 0, 0);
Comment comment = Comment comment =
@@ -1520,14 +1519,14 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
TimeUtil.nowTs(), TimeUtil.nowTs(),
"message", "message",
(short) 1, (short) 1,
revId.get(), commitId,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment); update.putComment(Status.PUBLISHED, comment);
update.commit(); update.commit();
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(revId, comment)); assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
} }
@Test @Test
@@ -1535,7 +1534,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser); ChangeUpdate update = newUpdate(c, otherUser);
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
RevId revId = new RevId("abcd1234abcd1234abcd1234abcd1234abcd1234"); ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
CommentRange range = new CommentRange(1, 2, 3, 4); CommentRange range = new CommentRange(1, 2, 3, 4);
Comment comment = Comment comment =
@@ -1550,14 +1549,14 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
TimeUtil.nowTs(), TimeUtil.nowTs(),
"message", "message",
(short) 1, (short) 1,
revId.get(), commitId,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment); update.putComment(Status.PUBLISHED, comment);
update.commit(); update.commit();
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(revId, comment)); assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
} }
@Test @Test
@@ -1575,7 +1574,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
CommentRange range1 = new CommentRange(1, 1, 2, 1); CommentRange range1 = new CommentRange(1, 1, 2, 1);
CommentRange range2 = new CommentRange(2, 1, 3, 1); CommentRange range2 = new CommentRange(2, 1, 3, 1);
Timestamp time = TimeUtil.nowTs(); Timestamp time = TimeUtil.nowTs();
RevId revId = new RevId("abcd1234abcd1234abcd1234abcd1234abcd1234"); ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
Comment comment1 = Comment comment1 =
newComment( newComment(
@@ -1589,7 +1588,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
time, time,
message1, message1,
(short) 0, (short) 0,
revId.get(), commitId,
false); false);
Comment comment2 = Comment comment2 =
newComment( newComment(
@@ -1603,7 +1602,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
time, time,
message2, message2,
(short) 0, (short) 0,
revId.get(), commitId,
false); false);
Comment comment3 = Comment comment3 =
newComment( newComment(
@@ -1617,7 +1616,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
time, time,
message3, message3,
(short) 0, (short) 0,
revId.get(), commitId,
false); false);
ChangeUpdate update = newUpdate(c, otherUser); ChangeUpdate update = newUpdate(c, otherUser);
@@ -1631,9 +1630,9 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(notes.getComments()) assertThat(notes.getComments())
.isEqualTo( .isEqualTo(
ImmutableListMultimap.of( ImmutableListMultimap.of(
revId, comment1, commitId, comment1,
revId, comment2, commitId, comment2,
revId, comment3)); commitId, comment3));
} }
@Test @Test
@@ -1646,7 +1645,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
Timestamp time = TimeUtil.nowTs(); Timestamp time = TimeUtil.nowTs();
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
RevId revId = new RevId("abcd1234abcd1234abcd1234abcd1234abcd1234"); ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
Comment comment = Comment comment =
newComment( newComment(
@@ -1660,7 +1659,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
time, time,
message, message,
(short) 1, (short) 1,
revId.get(), commitId,
false); false);
comment.setRealAuthor(changeOwner.getAccountId()); comment.setRealAuthor(changeOwner.getAccountId());
update.setPatchSetId(psId); update.setPatchSetId(psId);
@@ -1669,7 +1668,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(revId, comment)); assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
} }
@Test @Test
@@ -1699,7 +1698,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
time, time,
"comment", "comment",
(short) 1, (short) 1,
"abcd1234abcd1234abcd1234abcd1234abcd1234", ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment); update.putComment(Status.PUBLISHED, comment);
@@ -1708,7 +1707,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
assertThat(notes.getComments()) assertThat(notes.getComments())
.isEqualTo(ImmutableListMultimap.of(new RevId(comment.revId), comment)); .isEqualTo(ImmutableListMultimap.of(comment.getCommitId(), comment));
} }
@Test @Test
@@ -1717,8 +1716,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeUpdate update = newUpdate(c, otherUser); ChangeUpdate update = newUpdate(c, otherUser);
String uuid1 = "uuid1"; String uuid1 = "uuid1";
String uuid2 = "uuid2"; String uuid2 = "uuid2";
String rev1 = "abcd1234abcd1234abcd1234abcd1234abcd1234"; ObjectId commitId1 = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
String rev2 = "abcd4567abcd4567abcd4567abcd4567abcd4567"; ObjectId commitId2 = ObjectId.fromString("abcd4567abcd4567abcd4567abcd4567abcd4567");
String messageForBase = "comment for base"; String messageForBase = "comment for base";
String messageForPS = "comment for ps"; String messageForPS = "comment for ps";
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
@@ -1737,7 +1736,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
messageForBase, messageForBase,
(short) 0, (short) 0,
rev1, commitId1,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, commentForBase); update.putComment(Status.PUBLISHED, commentForBase);
@@ -1756,7 +1755,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
messageForPS, messageForPS,
(short) 1, (short) 1,
rev2, commitId2,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, commentForPS); update.putComment(Status.PUBLISHED, commentForPS);
@@ -1765,8 +1764,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(newNotes(c).getComments()) assertThat(newNotes(c).getComments())
.containsExactlyEntriesIn( .containsExactlyEntriesIn(
ImmutableListMultimap.of( ImmutableListMultimap.of(
new RevId(rev1), commentForBase, commitId1, commentForBase,
new RevId(rev2), commentForPS)); commitId2, commentForPS));
} }
@Test @Test
@@ -1774,7 +1773,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
Change c = newChange(); Change c = newChange();
String uuid1 = "uuid1"; String uuid1 = "uuid1";
String uuid2 = "uuid2"; String uuid2 = "uuid2";
String rev = "abcd1234abcd1234abcd1234abcd1234abcd1234"; ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
String filename = "filename"; String filename = "filename";
@@ -1795,7 +1794,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
timeForComment1, timeForComment1,
"comment 1", "comment 1",
side, side,
rev, commitId,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment1); update.putComment(Status.PUBLISHED, comment1);
@@ -1814,7 +1813,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
timeForComment2, timeForComment2,
"comment 2", "comment 2",
side, side,
rev, commitId,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment2); update.putComment(Status.PUBLISHED, comment2);
@@ -1823,8 +1822,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(newNotes(c).getComments()) assertThat(newNotes(c).getComments())
.containsExactlyEntriesIn( .containsExactlyEntriesIn(
ImmutableListMultimap.of( ImmutableListMultimap.of(
new RevId(rev), comment1, commitId, comment1,
new RevId(rev), comment2)) commitId, comment2))
.inOrder(); .inOrder();
} }
@@ -1832,7 +1831,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
public void patchLineCommentMultipleOnePatchsetMultipleFiles() throws Exception { public void patchLineCommentMultipleOnePatchsetMultipleFiles() throws Exception {
Change c = newChange(); Change c = newChange();
String uuid = "uuid"; String uuid = "uuid";
String rev = "abcd1234abcd1234abcd1234abcd1234abcd1234"; ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
String filename1 = "filename1"; String filename1 = "filename1";
@@ -1853,7 +1852,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment 1", "comment 1",
side, side,
rev, commitId,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment1); update.putComment(Status.PUBLISHED, comment1);
@@ -1872,7 +1871,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment 2", "comment 2",
side, side,
rev, commitId,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment2); update.putComment(Status.PUBLISHED, comment2);
@@ -1881,8 +1880,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(newNotes(c).getComments()) assertThat(newNotes(c).getComments())
.containsExactlyEntriesIn( .containsExactlyEntriesIn(
ImmutableListMultimap.of( ImmutableListMultimap.of(
new RevId(rev), comment1, commitId, comment1,
new RevId(rev), comment2)) commitId, comment2))
.inOrder(); .inOrder();
} }
@@ -1890,8 +1889,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
public void patchLineCommentMultiplePatchsets() throws Exception { public void patchLineCommentMultiplePatchsets() throws Exception {
Change c = newChange(); Change c = newChange();
String uuid = "uuid"; String uuid = "uuid";
String rev1 = "abcd1234abcd1234abcd1234abcd1234abcd1234"; ObjectId commitId1 = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
String rev2 = "abcd4567abcd4567abcd4567abcd4567abcd4567"; ObjectId commitId2 = ObjectId.fromString("abcd4567abcd4567abcd4567abcd4567abcd4567");
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id ps1 = c.currentPatchSetId(); PatchSet.Id ps1 = c.currentPatchSetId();
String filename = "filename1"; String filename = "filename1";
@@ -1911,7 +1910,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps1", "comment on ps1",
side, side,
rev1, commitId1,
false); false);
update.setPatchSetId(ps1); update.setPatchSetId(ps1);
update.putComment(Status.PUBLISHED, comment1); update.putComment(Status.PUBLISHED, comment1);
@@ -1934,7 +1933,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps2", "comment on ps2",
side, side,
rev2, commitId2,
false); false);
update.setPatchSetId(ps2); update.setPatchSetId(ps2);
update.putComment(Status.PUBLISHED, comment2); update.putComment(Status.PUBLISHED, comment2);
@@ -1943,15 +1942,15 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(newNotes(c).getComments()) assertThat(newNotes(c).getComments())
.containsExactlyEntriesIn( .containsExactlyEntriesIn(
ImmutableListMultimap.of( ImmutableListMultimap.of(
new RevId(rev1), comment1, commitId1, comment1,
new RevId(rev2), comment2)); commitId2, comment2));
} }
@Test @Test
public void patchLineCommentSingleDraftToPublished() throws Exception { public void patchLineCommentSingleDraftToPublished() throws Exception {
Change c = newChange(); Change c = newChange();
String uuid = "uuid"; String uuid = "uuid";
String rev = "abcd4567abcd4567abcd4567abcd4567abcd4567"; ObjectId commitId = ObjectId.fromString("abcd4567abcd4567abcd4567abcd4567abcd4567");
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id ps1 = c.currentPatchSetId(); PatchSet.Id ps1 = c.currentPatchSetId();
String filename = "filename1"; String filename = "filename1";
@@ -1971,7 +1970,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps1", "comment on ps1",
side, side,
rev, commitId,
false); false);
update.setPatchSetId(ps1); update.setPatchSetId(ps1);
update.putComment(Status.DRAFT, comment1); update.putComment(Status.DRAFT, comment1);
@@ -1979,7 +1978,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId)) assertThat(notes.getDraftComments(otherUserId))
.containsExactlyEntriesIn(ImmutableListMultimap.of(new RevId(rev), comment1)); .containsExactlyEntriesIn(ImmutableListMultimap.of(commitId, comment1));
assertThat(notes.getComments()).isEmpty(); assertThat(notes.getComments()).isEmpty();
update = newUpdate(c, otherUser); update = newUpdate(c, otherUser);
@@ -1990,7 +1989,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
notes = newNotes(c); notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId)).isEmpty(); assertThat(notes.getDraftComments(otherUserId)).isEmpty();
assertThat(notes.getComments()) assertThat(notes.getComments())
.containsExactlyEntriesIn(ImmutableListMultimap.of(new RevId(rev), comment1)); .containsExactlyEntriesIn(ImmutableListMultimap.of(commitId, comment1));
} }
@Test @Test
@@ -1998,7 +1997,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
Change c = newChange(); Change c = newChange();
String uuid1 = "uuid1"; String uuid1 = "uuid1";
String uuid2 = "uuid2"; String uuid2 = "uuid2";
String rev = "abcd4567abcd4567abcd4567abcd4567abcd4567"; ObjectId commitId = ObjectId.fromString("abcd4567abcd4567abcd4567abcd4567abcd4567");
CommentRange range1 = new CommentRange(1, 1, 2, 2); CommentRange range1 = new CommentRange(1, 1, 2, 2);
CommentRange range2 = new CommentRange(2, 2, 3, 3); CommentRange range2 = new CommentRange(2, 2, 3, 3);
String filename = "filename1"; String filename = "filename1";
@@ -2021,7 +2020,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps1", "comment on ps1",
side, side,
rev, commitId,
false); false);
Comment comment2 = Comment comment2 =
newComment( newComment(
@@ -2035,7 +2034,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"other on ps1", "other on ps1",
side, side,
rev, commitId,
false); false);
update.putComment(Status.DRAFT, comment1); update.putComment(Status.DRAFT, comment1);
update.putComment(Status.DRAFT, comment2); update.putComment(Status.DRAFT, comment2);
@@ -2045,8 +2044,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(notes.getDraftComments(otherUserId)) assertThat(notes.getDraftComments(otherUserId))
.containsExactlyEntriesIn( .containsExactlyEntriesIn(
ImmutableListMultimap.of( ImmutableListMultimap.of(
new RevId(rev), comment1, commitId, comment1,
new RevId(rev), comment2)) commitId, comment2))
.inOrder(); .inOrder();
assertThat(notes.getComments()).isEmpty(); assertThat(notes.getComments()).isEmpty();
@@ -2058,9 +2057,9 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
notes = newNotes(c); notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId)) assertThat(notes.getDraftComments(otherUserId))
.containsExactlyEntriesIn(ImmutableListMultimap.of(new RevId(rev), comment2)); .containsExactlyEntriesIn(ImmutableListMultimap.of(commitId, comment2));
assertThat(notes.getComments()) assertThat(notes.getComments())
.containsExactlyEntriesIn(ImmutableListMultimap.of(new RevId(rev), comment1)); .containsExactlyEntriesIn(ImmutableListMultimap.of(commitId, comment1));
} }
@Test @Test
@@ -2068,8 +2067,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
Change c = newChange(); Change c = newChange();
String uuid1 = "uuid1"; String uuid1 = "uuid1";
String uuid2 = "uuid2"; String uuid2 = "uuid2";
String rev1 = "abcd1234abcd1234abcd1234abcd1234abcd1234"; ObjectId commitId1 = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
String rev2 = "abcd4567abcd4567abcd4567abcd4567abcd4567"; ObjectId commitId2 = ObjectId.fromString("abcd4567abcd4567abcd4567abcd4567abcd4567");
CommentRange range1 = new CommentRange(1, 1, 2, 2); CommentRange range1 = new CommentRange(1, 1, 2, 2);
CommentRange range2 = new CommentRange(2, 2, 3, 3); CommentRange range2 = new CommentRange(2, 2, 3, 3);
String filename = "filename1"; String filename = "filename1";
@@ -2091,7 +2090,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on base", "comment on base",
(short) 0, (short) 0,
rev1, commitId1,
false); false);
Comment psComment = Comment psComment =
newComment( newComment(
@@ -2105,7 +2104,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps", "comment on ps",
(short) 1, (short) 1,
rev2, commitId2,
false); false);
update.putComment(Status.DRAFT, baseComment); update.putComment(Status.DRAFT, baseComment);
@@ -2116,8 +2115,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(notes.getDraftComments(otherUserId)) assertThat(notes.getDraftComments(otherUserId))
.containsExactlyEntriesIn( .containsExactlyEntriesIn(
ImmutableListMultimap.of( ImmutableListMultimap.of(
new RevId(rev1), baseComment, commitId1, baseComment,
new RevId(rev2), psComment)); commitId2, psComment));
assertThat(notes.getComments()).isEmpty(); assertThat(notes.getComments()).isEmpty();
// Publish both comments. // Publish both comments.
@@ -2133,16 +2132,15 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(notes.getComments()) assertThat(notes.getComments())
.containsExactlyEntriesIn( .containsExactlyEntriesIn(
ImmutableListMultimap.of( ImmutableListMultimap.of(
new RevId(rev1), baseComment, commitId1, baseComment,
new RevId(rev2), psComment)); commitId2, psComment));
} }
@Test @Test
public void patchLineCommentsDeleteAllDrafts() throws Exception { public void patchLineCommentsDeleteAllDrafts() throws Exception {
Change c = newChange(); Change c = newChange();
String uuid = "uuid"; String uuid = "uuid";
String rev = "abcd1234abcd1234abcd1234abcd1234abcd1234"; ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
ObjectId objId = ObjectId.fromString(rev);
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
String filename = "filename"; String filename = "filename";
@@ -2162,7 +2160,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps1", "comment on ps1",
side, side,
rev, commitId,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.DRAFT, comment); update.putComment(Status.DRAFT, comment);
@@ -2170,7 +2168,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId)).hasSize(1); assertThat(notes.getDraftComments(otherUserId)).hasSize(1);
assertThat(notes.getDraftCommentNotes().getNoteMap().contains(objId)).isTrue(); assertThat(notes.getDraftCommentNotes().getNoteMap().contains(commitId)).isTrue();
update = newUpdate(c, otherUser); update = newUpdate(c, otherUser);
update.setPatchSetId(psId); update.setPatchSetId(psId);
@@ -2186,10 +2184,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
public void patchLineCommentsDeleteAllDraftsForOneRevision() throws Exception { public void patchLineCommentsDeleteAllDraftsForOneRevision() throws Exception {
Change c = newChange(); Change c = newChange();
String uuid = "uuid"; String uuid = "uuid";
String rev1 = "abcd1234abcd1234abcd1234abcd1234abcd1234"; ObjectId commitId1 = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
String rev2 = "abcd4567abcd4567abcd4567abcd4567abcd4567"; ObjectId commitId2 = ObjectId.fromString("abcd4567abcd4567abcd4567abcd4567abcd4567");
ObjectId objId1 = ObjectId.fromString(rev1);
ObjectId objId2 = ObjectId.fromString(rev2);
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id ps1 = c.currentPatchSetId(); PatchSet.Id ps1 = c.currentPatchSetId();
String filename = "filename1"; String filename = "filename1";
@@ -2209,7 +2205,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps1", "comment on ps1",
side, side,
rev1, commitId1,
false); false);
update.setPatchSetId(ps1); update.setPatchSetId(ps1);
update.putComment(Status.DRAFT, comment1); update.putComment(Status.DRAFT, comment1);
@@ -2232,7 +2228,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps2", "comment on ps2",
side, side,
rev2, commitId2,
false); false);
update.setPatchSetId(ps2); update.setPatchSetId(ps2);
update.putComment(Status.DRAFT, comment2); update.putComment(Status.DRAFT, comment2);
@@ -2249,15 +2245,15 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
notes = newNotes(c); notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId)).hasSize(1); assertThat(notes.getDraftComments(otherUserId)).hasSize(1);
NoteMap noteMap = notes.getDraftCommentNotes().getNoteMap(); NoteMap noteMap = notes.getDraftCommentNotes().getNoteMap();
assertThat(noteMap.contains(objId1)).isTrue(); assertThat(noteMap.contains(commitId1)).isTrue();
assertThat(noteMap.contains(objId2)).isFalse(); assertThat(noteMap.contains(commitId2)).isFalse();
} }
@Test @Test
public void addingPublishedCommentDoesNotCreateNoOpCommitOnEmptyDraftRef() throws Exception { public void addingPublishedCommentDoesNotCreateNoOpCommitOnEmptyDraftRef() throws Exception {
Change c = newChange(); Change c = newChange();
String uuid = "uuid"; String uuid = "uuid";
String rev = "abcd4567abcd4567abcd4567abcd4567abcd4567"; ObjectId commitId = ObjectId.fromString("abcd4567abcd4567abcd4567abcd4567abcd4567");
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id ps1 = c.currentPatchSetId(); PatchSet.Id ps1 = c.currentPatchSetId();
String filename = "filename1"; String filename = "filename1";
@@ -2277,7 +2273,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps1", "comment on ps1",
side, side,
rev, commitId,
false); false);
update.putComment(Status.PUBLISHED, comment); update.putComment(Status.PUBLISHED, comment);
update.commit(); update.commit();
@@ -2290,7 +2286,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
@Test @Test
public void addingPublishedCommentDoesNotCreateNoOpCommitOnNonEmptyDraftRef() throws Exception { public void addingPublishedCommentDoesNotCreateNoOpCommitOnNonEmptyDraftRef() throws Exception {
Change c = newChange(); Change c = newChange();
String rev = "abcd4567abcd4567abcd4567abcd4567abcd4567"; ObjectId commitId = ObjectId.fromString("abcd4567abcd4567abcd4567abcd4567abcd4567");
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id ps1 = c.currentPatchSetId(); PatchSet.Id ps1 = c.currentPatchSetId();
String filename = "filename1"; String filename = "filename1";
@@ -2310,7 +2306,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"draft comment on ps1", "draft comment on ps1",
side, side,
rev, commitId,
false); false);
update.putComment(Status.DRAFT, draft); update.putComment(Status.DRAFT, draft);
update.commit(); update.commit();
@@ -2332,7 +2328,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps1", "comment on ps1",
side, side,
rev, commitId,
false); false);
update.putComment(Status.PUBLISHED, pub); update.putComment(Status.PUBLISHED, pub);
update.commit(); update.commit();
@@ -2345,7 +2341,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser); ChangeUpdate update = newUpdate(c, otherUser);
String uuid = "uuid"; String uuid = "uuid";
String rev = "abcd1234abcd1234abcd1234abcd1234abcd1234"; ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
String messageForBase = "comment for base"; String messageForBase = "comment for base";
Timestamp now = TimeUtil.nowTs(); Timestamp now = TimeUtil.nowTs();
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
@@ -2362,14 +2358,14 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
messageForBase, messageForBase,
(short) 0, (short) 0,
rev, commitId,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment); update.putComment(Status.PUBLISHED, comment);
update.commit(); update.commit();
assertThat(newNotes(c).getComments()) assertThat(newNotes(c).getComments())
.containsExactlyEntriesIn(ImmutableListMultimap.of(new RevId(rev), comment)); .containsExactlyEntriesIn(ImmutableListMultimap.of(commitId, comment));
} }
@Test @Test
@@ -2377,7 +2373,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser); ChangeUpdate update = newUpdate(c, otherUser);
String uuid = "uuid"; String uuid = "uuid";
String rev = "abcd1234abcd1234abcd1234abcd1234abcd1234"; ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
String messageForBase = "comment for base"; String messageForBase = "comment for base";
Timestamp now = TimeUtil.nowTs(); Timestamp now = TimeUtil.nowTs();
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
@@ -2394,22 +2390,22 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
messageForBase, messageForBase,
(short) 0, (short) 0,
rev, commitId,
false); false);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment); update.putComment(Status.PUBLISHED, comment);
update.commit(); update.commit();
assertThat(newNotes(c).getComments()) assertThat(newNotes(c).getComments())
.containsExactlyEntriesIn(ImmutableListMultimap.of(new RevId(rev), comment)); .containsExactlyEntriesIn(ImmutableListMultimap.of(commitId, comment));
} }
@Test @Test
public void putCommentsForMultipleRevisions() throws Exception { public void putCommentsForMultipleRevisions() throws Exception {
Change c = newChange(); Change c = newChange();
String uuid = "uuid"; String uuid = "uuid";
String rev1 = "abcd1234abcd1234abcd1234abcd1234abcd1234"; ObjectId commitId1 = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
String rev2 = "abcd4567abcd4567abcd4567abcd4567abcd4567"; ObjectId commitId2 = ObjectId.fromString("abcd4567abcd4567abcd4567abcd4567abcd4567");
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id ps1 = c.currentPatchSetId(); PatchSet.Id ps1 = c.currentPatchSetId();
String filename = "filename1"; String filename = "filename1";
@@ -2433,7 +2429,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps1", "comment on ps1",
side, side,
rev1, commitId1,
false); false);
Comment comment2 = Comment comment2 =
newComment( newComment(
@@ -2447,7 +2443,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps2", "comment on ps2",
side, side,
rev2, commitId2,
false); false);
update.putComment(Status.DRAFT, comment1); update.putComment(Status.DRAFT, comment1);
update.putComment(Status.DRAFT, comment2); update.putComment(Status.DRAFT, comment2);
@@ -2471,7 +2467,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
@Test @Test
public void publishSubsetOfCommentsOnRevision() throws Exception { public void publishSubsetOfCommentsOnRevision() throws Exception {
Change c = newChange(); Change c = newChange();
RevId rev1 = new RevId("abcd1234abcd1234abcd1234abcd1234abcd1234"); ObjectId commitId1 = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id ps1 = c.currentPatchSetId(); PatchSet.Id ps1 = c.currentPatchSetId();
short side = (short) 1; short side = (short) 1;
@@ -2491,7 +2487,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment1", "comment1",
side, side,
rev1.get(), commitId1,
false); false);
Comment comment2 = Comment comment2 =
newComment( newComment(
@@ -2505,14 +2501,15 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment2", "comment2",
side, side,
rev1.get(), commitId1,
false); false);
update.putComment(Status.DRAFT, comment1); update.putComment(Status.DRAFT, comment1);
update.putComment(Status.DRAFT, comment2); update.putComment(Status.DRAFT, comment2);
update.commit(); update.commit();
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId).get(rev1)).containsExactly(comment1, comment2); assertThat(notes.getDraftComments(otherUserId).get(commitId1))
.containsExactly(comment1, comment2);
assertThat(notes.getComments()).isEmpty(); assertThat(notes.getComments()).isEmpty();
update = newUpdate(c, otherUser); update = newUpdate(c, otherUser);
@@ -2521,8 +2518,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update.commit(); update.commit();
notes = newNotes(c); notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId).get(rev1)).containsExactly(comment1); assertThat(notes.getDraftComments(otherUserId).get(commitId1)).containsExactly(comment1);
assertThat(notes.getComments().get(rev1)).containsExactly(comment2); assertThat(notes.getComments().get(commitId1)).containsExactly(comment2);
} }
@Test @Test
@@ -2544,7 +2541,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
@Test @Test
public void filterOutAndFixUpZombieDraftComments() throws Exception { public void filterOutAndFixUpZombieDraftComments() throws Exception {
Change c = newChange(); Change c = newChange();
RevId rev1 = new RevId("abcd1234abcd1234abcd1234abcd1234abcd1234"); ObjectId commitId1 = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
PatchSet.Id ps1 = c.currentPatchSetId(); PatchSet.Id ps1 = c.currentPatchSetId();
short side = (short) 1; short side = (short) 1;
@@ -2563,7 +2560,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"comment on ps1", "comment on ps1",
side, side,
rev1.get(), commitId1,
false); false);
Comment comment2 = Comment comment2 =
newComment( newComment(
@@ -2577,7 +2574,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
now, now,
"another comment", "another comment",
side, side,
rev1.get(), commitId1,
false); false);
update.putComment(Status.DRAFT, comment1); update.putComment(Status.DRAFT, comment1);
update.putComment(Status.DRAFT, comment2); update.putComment(Status.DRAFT, comment2);
@@ -2605,12 +2602,12 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
// Looking at drafts directly shows the zombie comment. // Looking at drafts directly shows the zombie comment.
DraftCommentNotes draftNotes = draftNotesFactory.create(c.getId(), otherUserId); DraftCommentNotes draftNotes = draftNotesFactory.create(c.getId(), otherUserId);
assertThat(draftNotes.load().getComments().get(rev1)).containsExactly(comment1, comment2); assertThat(draftNotes.load().getComments().get(commitId1)).containsExactly(comment1, comment2);
// Zombie comment is filtered out of drafts via ChangeNotes. // Zombie comment is filtered out of drafts via ChangeNotes.
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId).get(rev1)).containsExactly(comment1); assertThat(notes.getDraftComments(otherUserId).get(commitId1)).containsExactly(comment1);
assertThat(notes.getComments().get(rev1)).containsExactly(comment2); assertThat(notes.getComments().get(commitId1)).containsExactly(comment2);
update = newUpdate(c, otherUser); update = newUpdate(c, otherUser);
update.setPatchSetId(ps1); update.setPatchSetId(ps1);
@@ -2625,7 +2622,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
public void updateCommentsInSequentialUpdates() throws Exception { public void updateCommentsInSequentialUpdates() throws Exception {
Change c = newChange(); Change c = newChange();
CommentRange range = new CommentRange(1, 1, 2, 1); CommentRange range = new CommentRange(1, 1, 2, 1);
String rev = "abcd1234abcd1234abcd1234abcd1234abcd1234"; ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
ChangeUpdate update1 = newUpdate(c, otherUser); ChangeUpdate update1 = newUpdate(c, otherUser);
Comment comment1 = Comment comment1 =
@@ -2640,7 +2637,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
new Timestamp(update1.getWhen().getTime()), new Timestamp(update1.getWhen().getTime()),
"comment 1", "comment 1",
(short) 1, (short) 1,
rev, commitId,
false); false);
update1.putComment(Status.PUBLISHED, comment1); update1.putComment(Status.PUBLISHED, comment1);
@@ -2657,7 +2654,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
new Timestamp(update2.getWhen().getTime()), new Timestamp(update2.getWhen().getTime()),
"comment 2", "comment 2",
(short) 1, (short) 1,
rev, commitId,
false); false);
update2.putComment(Status.PUBLISHED, comment2); update2.putComment(Status.PUBLISHED, comment2);
@@ -2668,7 +2665,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
} }
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
List<Comment> comments = notes.getComments().get(new RevId(rev)); List<Comment> comments = notes.getComments().get(commitId);
assertThat(comments).hasSize(2); assertThat(comments).hasSize(2);
assertThat(comments.get(0).message).isEqualTo("comment 1"); assertThat(comments.get(0).message).isEqualTo("comment 1");
assertThat(comments.get(1).message).isEqualTo("comment 2"); assertThat(comments.get(1).message).isEqualTo("comment 2");
@@ -2714,7 +2711,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
new Timestamp(update.getWhen().getTime()), new Timestamp(update.getWhen().getTime()),
"comment", "comment",
(short) 1, (short) 1,
"abcd1234abcd1234abcd1234abcd1234abcd1234", ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
false); false);
update.putComment(Status.PUBLISHED, comment); update.putComment(Status.PUBLISHED, comment);
update.commit(); update.commit();

3
javatests/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java
View File

@@ -25,6 +25,7 @@ import java.sql.Timestamp;
import java.time.ZonedDateTime; import java.time.ZonedDateTime;
import java.util.TimeZone; import java.util.TimeZone;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.After; import org.junit.After;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
@@ -160,7 +161,7 @@ public class CommentTimestampAdapterTest extends GerritBaseTests {
"serverId", "serverId",
false); false);
c.lineNbr = 1; c.lineNbr = 1;
c.revId = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"; c.setCommitId(ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
String json = gson.toJson(c); String json = gson.toJson(c);
assertThat(json).contains("\"writtenOn\": \"" + NON_DST_STR_TRUNC + "\","); assertThat(json).contains("\"writtenOn\": \"" + NON_DST_STR_TRUNC + "\",");