opendev/gerrit
Code Issues Proposed changes

Merge changes I8c6ac497,Ibd14562d

Browse Source 
* changes:
  Remove unused classes to read/write legacy change notes
  Remove unused PatchLineComment class
This commit is contained in:
David Pursehouse
2019-10-17 08:46:32 +00:00
committed by Gerrit Code Review
parent d9ec549338 1dfc3e58e5
commit 68226928fb
25 changed files with 140 additions and 1145 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
java/com/google/gerrit/entities/Change.java
View File

@@ -44,7 +44,7 @@ import org.eclipse.jgit.lib.ObjectInserter;
* |
* +- {@link PatchSetApproval}: a +/- vote on the change's current state.
* |
* +- {@link PatchLineComment}: comment about a specific line
* +- {@link Comment}: comment about a specific line
* </pre>
*
* <p>
@@ -60,8 +60,8 @@ import org.eclipse.jgit.lib.ObjectInserter;
* commit can contain zero patches, if the merge has no conflicts, or has no impact other than to
* cut off a line of development.
*
* <p>Each PatchLineComment is a draft or a published comment about a single line of the associated
* file. These are the inline comment entities created by users as they perform a review.
* <p>Each Comment is a draft or a published comment about a single line of the associated file.
* These are the inline comment entities created by users as they perform a review.
*
* <p>When additional PatchSets appear under a change, these PatchSets reference <i>replacement</i>
* commits; alternative commits that could be made to the project instead of the original commit
@@ -76,10 +76,10 @@ import org.eclipse.jgit.lib.ObjectInserter;
* <h5>ChangeMessage</h5>
*
* <p>The ChangeMessage entity is a general free-form comment about the whole change, rather than
* PatchLineComment's file and line specific context. The ChangeMessage appears at the start of any
* email generated by Gerrit, and is shown on the change overview page, rather than in a
* file-specific context. Users often use this entity to describe general remarks about the overall
* concept proposed by the change.
* Comment's file and line specific context. The ChangeMessage appears at the start of any email
* generated by Gerrit, and is shown on the change overview page, rather than in a file-specific
* context. Users often use this entity to describe general remarks about the overall concept
* proposed by the change.
*
* <p>
*

44
java/com/google/gerrit/entities/Comment.java
View File

@@ -29,27 +29,33 @@ import org.eclipse.jgit.lib.ObjectId;
*
* <p>Changing fields in this class changes the storage format of inline comments in NoteDb and may
* require a corresponding data migration (adding new optional fields is generally okay).
*
* <p>{@link PatchLineComment} historically represented comments in ReviewDb. There are a few
* notable differences:
*
* <ul>
* <li>PatchLineComment knows the comment status (published or draft). For comments in NoteDb the
* status is determined by the branch in which they are stored (published comments are stored
* in the change meta ref; draft comments are store in refs/draft-comments branches in
* All-Users). Hence Comment doesn't need to contain the status, but the status is implicitly
* known by where the comments are read from.
* <li>PatchLineComment knows the change ID. For comments in NoteDb, the change ID is determined
* by the branch in which they are stored (the ref name contains the change ID). Hence Comment
* doesn't need to contain the change ID, but the change ID is implicitly known by where the
* comments are read from.
* </ul>
*
* <p>For all utility classes and middle layer functionality using Comment over PatchLineComment is
* preferred, as ReviewDb is gone so PatchLineComment is slated for deletion as well. This means
* Comment should be used everywhere.
*/
public class Comment {
public enum Status {
DRAFT('d'),
PUBLISHED('P');
private final char code;
Status(char c) {
code = c;
}
public char getCode() {
return code;
}
public static Status forCode(char c) {
for (Status s : Status.values()) {
if (s.code == c) {
return s;
}
}
return null;
}
}
public static class Key {
public String uuid;
public String filename;

288
java/com/google/gerrit/entities/PatchLineComment.java
View File

@@ -1,288 +0,0 @@
// Copyright (C) 2008 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.entities;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.Comment.Range;
import java.sql.Timestamp;
import java.util.Objects;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
/**
* A comment left by a user on a specific line of a {@link Patch}.
*
* <p>New APIs should not expose this class.
*
* @see Comment
*/
public final class PatchLineComment {
public static final char STATUS_DRAFT = 'd';
public static final char STATUS_PUBLISHED = 'P';
public enum Status {
DRAFT(STATUS_DRAFT),
PUBLISHED(STATUS_PUBLISHED);
private final char code;
Status(char c) {
code = c;
}
public char getCode() {
return code;
}
public static Status forCode(char c) {
for (Status s : Status.values()) {
if (s.code == c) {
return s;
}
}
return null;
}
}
protected Patch.Key patchKey;
protected String uuid;
/** Line number this comment applies to; it should display after the line. */
protected int lineNbr;
/** Who wrote this comment. */
protected Account.Id author;
/** When this comment was drafted. */
protected Timestamp writtenOn;
/** Current publication state of the comment; see {@link Status}. */
protected char status;
/** Which file is this comment; 0 is ancestor, 1 is new version. */
protected short side;
/** The text left by the user. */
@Nullable protected String message;
/** The parent of this comment, or null if this is the first comment on this line */
@Nullable protected String parentUuid;
@Nullable protected CommentRange range;
@Nullable protected String tag;
/** Real user that added this comment on behalf of the user recorded in {@link #author}. */
@Nullable protected Account.Id realAuthor;
/** True if this comment requires further action. */
protected boolean unresolved;
/** The ID of the commit to which this comment is referring. */
protected ObjectId commitId;
protected PatchLineComment() {}
public PatchLineComment(
Patch.Key patchKey, String uuid, int line, Account.Id a, String parentUuid, Timestamp when) {
this.patchKey = patchKey;
this.uuid = uuid;
lineNbr = line;
author = a;
setParentUuid(parentUuid);
setStatus(Status.DRAFT);
setWrittenOn(when);
}
public PatchLineComment(PatchLineComment o) {
patchKey = o.patchKey;
uuid = o.uuid;
lineNbr = o.lineNbr;
author = o.author;
realAuthor = o.realAuthor;
writtenOn = o.writtenOn;
status = o.status;
side = o.side;
message = o.message;
parentUuid = o.parentUuid;
commitId = o.commitId;
if (o.range != null) {
range =
new CommentRange(
o.range.getStartLine(),
o.range.getStartCharacter(),
o.range.getEndLine(),
o.range.getEndCharacter());
}
}
public PatchSet.Id getPatchSetId() {
return patchKey.patchSetId();
}
public int getLine() {
return lineNbr;
}
public void setLine(int line) {
lineNbr = line;
}
public Account.Id getAuthor() {
return author;
}
public Account.Id getRealAuthor() {
return realAuthor != null ? realAuthor : getAuthor();
}
public void setRealAuthor(Account.Id id) {
// Use null for same real author, as before the column was added.
realAuthor = Objects.equals(getAuthor(), id) ? null : id;
}
public Timestamp getWrittenOn() {
return writtenOn;
}
public Status getStatus() {
return Status.forCode(status);
}
public void setStatus(Status s) {
status = s.getCode();
}
public short getSide() {
return side;
}
public void setSide(short s) {
side = s;
}
public String getMessage() {
return message;
}
public void setMessage(String s) {
message = s;
}
public void setWrittenOn(Timestamp ts) {
writtenOn = ts;
}
public String getParentUuid() {
return parentUuid;
}
public void setParentUuid(String inReplyTo) {
parentUuid = inReplyTo;
}
public void setRange(Range r) {
if (r != null) {
range =
new CommentRange(
r.startLine, r.startCharacter,
r.endLine, r.endCharacter);
} else {
range = null;
}
}
public void setRange(CommentRange r) {
range = r;
}
public CommentRange getRange() {
return range;
}
public void setCommitId(AnyObjectId commitId) {
this.commitId = commitId.copy();
}
public ObjectId getCommitId() {
return commitId;
}
public void setTag(String tag) {
this.tag = tag;
}
public String getTag() {
return tag;
}
public void setUnresolved(Boolean unresolved) {
this.unresolved = unresolved;
}
public Boolean getUnresolved() {
return unresolved;
}
@Override
public boolean equals(Object o) {
if (o instanceof PatchLineComment) {
PatchLineComment c = (PatchLineComment) o;
return Objects.equals(patchKey, c.patchKey)
&& Objects.equals(uuid, c.uuid)
&& Objects.equals(lineNbr, c.getLine())
&& Objects.equals(author, c.getAuthor())
&& Objects.equals(writtenOn, c.getWrittenOn())
&& Objects.equals(status, c.getStatus().getCode())
&& Objects.equals(side, c.getSide())
&& Objects.equals(message, c.getMessage())
&& Objects.equals(parentUuid, c.getParentUuid())
&& Objects.equals(range, c.getRange())
&& Objects.equals(commitId, c.getCommitId())
&& Objects.equals(tag, c.getTag())
&& Objects.equals(unresolved, c.getUnresolved());
}
return false;
}
@Override
public int hashCode() {
return Objects.hash(patchKey, uuid);
}
@Override
public String toString() {
StringBuilder builder = new StringBuilder();
builder.append("PatchLineComment{");
builder.append("patchKey=").append(patchKey).append(',');
builder.append("uuid=").append(uuid).append(',');
builder.append("lineNbr=").append(lineNbr).append(',');
builder.append("author=").append(author.get()).append(',');
builder.append("realAuthor=").append(realAuthor != null ? realAuthor.get() : "").append(',');
builder.append("writtenOn=").append(writtenOn.toString()).append(',');
builder.append("status=").append(status).append(',');
builder.append("side=").append(side).append(',');
builder.append("message=").append(Objects.toString(message, "")).append(',');
builder.append("parentUuid=").append(Objects.toString(parentUuid, "")).append(',');
builder.append("range=").append(Objects.toString(range, "")).append(',');
builder.append("revId=").append(commitId != null ? commitId.name() : "").append(',');
builder.append("tag=").append(Objects.toString(tag, "")).append(',');
builder.append("unresolved=").append(unresolved);
builder.append('}');
return builder.toString();
}
}

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

@@ -26,7 +26,6 @@ import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.PatchLineComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.RobotComment;
@@ -258,8 +257,7 @@ public class CommentsUtil {
return sort(comments);
}
public void putComments(
ChangeUpdate update, PatchLineComment.Status status, Iterable<Comment> comments) {
public void putComments(ChangeUpdate update, Comment.Status status, Iterable<Comment> comments) {
for (Comment c : comments) {
update.putComment(status, c);
}

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

@@ -15,7 +15,7 @@
package com.google.gerrit.server;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.gerrit.entities.PatchLineComment.Status.PUBLISHED;
import static com.google.gerrit.entities.Comment.Status;
import static java.util.stream.Collectors.toSet;
import com.google.common.collect.ImmutableList;
@@ -80,7 +80,7 @@ public class PublishCommentUtil {
throw new StorageException(e);
}
}
commentsUtil.putComments(ctx.getUpdate(psId), PUBLISHED, draftComments);
commentsUtil.putComments(ctx.getUpdate(psId), Status.PUBLISHED, draftComments);
}
private static PatchSet.Id psId(ChangeNotes notes, Comment c) {

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

@@ -24,7 +24,6 @@ import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.PatchLineComment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
@@ -335,7 +334,7 @@ public class MailProcessor {
persistentCommentFromMailComment(ctx, c, targetPatchSetForComment(ctx, c, patchSet)));
}
commentsUtil.putComments(
ctx.getUpdate(ctx.getChange().currentPatchSetId()), Status.PUBLISHED, comments);
ctx.getUpdate(ctx.getChange().currentPatchSetId()), Comment.Status.PUBLISHED, comments);
return true;
}

3
java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
View File

@@ -50,7 +50,6 @@ public abstract class AbstractChangeNotes<T> {
public final ChangeNoteJson changeNoteJson;
public final GitRepositoryManager repoManager;
public final AllUsersName allUsers;
public final LegacyChangeNoteRead legacyChangeNoteRead;
public final NoteDbMetrics metrics;
public final String serverId;
@@ -64,14 +63,12 @@ public abstract class AbstractChangeNotes<T> {
GitRepositoryManager repoManager,
AllUsersName allUsers,
ChangeNoteJson changeNoteJson,
LegacyChangeNoteRead legacyChangeNoteRead,
NoteDbMetrics metrics,
Provider<ChangeNotesCache> cache,
@GerritServerId String serverId) {
this.failOnLoadForTest = new AtomicBoolean();
this.repoManager = repoManager;
this.allUsers = allUsers;
this.legacyChangeNoteRead = legacyChangeNoteRead;
this.changeNoteJson = changeNoteJson;
this.metrics = metrics;
this.cache = cache;

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

@@ -23,7 +23,6 @@ import com.google.common.collect.Sets;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.PatchLineComment;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
@@ -265,12 +264,7 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
// Even though reading from changes might not be enabled, we need to
// parse any existing revision notes so we can merge them.
return RevisionNoteMap.parse(
noteUtil.getChangeNoteJson(),
noteUtil.getLegacyChangeNoteRead(),
getId(),
rw.getObjectReader(),
noteMap,
PatchLineComment.Status.DRAFT);
noteUtil.getChangeNoteJson(), rw.getObjectReader(), noteMap, Comment.Status.DRAFT);
}
@Override

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

@@ -63,22 +63,13 @@ public class ChangeNoteUtil {
static final String UNRESOLVED = "Unresolved";
static final String TAG = FOOTER_TAG.getName();
private final LegacyChangeNoteRead legacyChangeNoteRead;
private final ChangeNoteJson changeNoteJson;
private final String serverId;
@Inject
public ChangeNoteUtil(
ChangeNoteJson changeNoteJson,
LegacyChangeNoteRead legacyChangeNoteRead,
@GerritServerId String serverId) {
public ChangeNoteUtil(ChangeNoteJson changeNoteJson, @GerritServerId String serverId) {
this.serverId = serverId;
this.changeNoteJson = changeNoteJson;
this.legacyChangeNoteRead = legacyChangeNoteRead;
}
public LegacyChangeNoteRead getLegacyChangeNoteRead() {
return legacyChangeNoteRead;
}
public ChangeNoteJson getChangeNoteJson() {

7
java/com/google/gerrit/server/notedb/ChangeNotesCache.java
View File

@@ -354,12 +354,7 @@ public class ChangeNotesCache {
"Load change notes for change %s of project %s", key.changeId(), key.project());
ChangeNotesParser parser =
new ChangeNotesParser(
key.changeId(),
key.id(),
walkSupplier.get(),
args.changeNoteJson,
args.legacyChangeNoteRead,
args.metrics);
key.changeId(), key.id(), walkSupplier.get(), args.changeNoteJson, args.metrics);
ChangeNotesState result = parser.parseAll();
// This assignment only happens if call() was actually called, which only
// happens when Cache#get(K, Callable<V>) incurs a cache miss.

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

@@ -61,7 +61,6 @@ import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.PatchLineComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.RefNames;
@@ -103,7 +102,6 @@ class ChangeNotesParser {
// Private final members initialized in the constructor.
private final ChangeNoteJson changeNoteJson;
private final LegacyChangeNoteRead legacyChangeNoteRead;
private final NoteDbMetrics metrics;
private final Change.Id id;
@@ -156,13 +154,11 @@ class ChangeNotesParser {
ObjectId tip,
ChangeNotesRevWalk walk,
ChangeNoteJson changeNoteJson,
LegacyChangeNoteRead legacyChangeNoteRead,
NoteDbMetrics metrics) {
this.id = changeId;
this.tip = tip;
this.walk = walk;
this.changeNoteJson = changeNoteJson;
this.legacyChangeNoteRead = legacyChangeNoteRead;
this.metrics = metrics;
approvals = new LinkedHashMap<>();
bufferedApprovals = new ArrayList<>();
@@ -444,7 +440,7 @@ class ChangeNotesParser {
return effectiveAccountId;
}
PersonIdent ident = RawParseUtils.parsePersonIdent(realUser);
return legacyChangeNoteRead.parseIdent(ident, id);
return parseIdent(ident);
}
private String parseTopic(ChangeNotesCommit commit) throws ConfigInvalidException {
@@ -576,7 +572,7 @@ class ChangeNotesParser {
parsedAssignee = Optional.empty();
} else {
PersonIdent ident = RawParseUtils.parsePersonIdent(assigneeValue);
parsedAssignee = Optional.ofNullable(legacyChangeNoteRead.parseIdent(ident, id));
parsedAssignee = Optional.ofNullable(parseIdent(ident));
}
assigneeUpdates.add(AssigneeStatusUpdate.create(ts, ownerId, parsedAssignee));
}
@@ -709,12 +705,7 @@ class ChangeNotesParser {
ChangeNotesCommit tipCommit = walk.parseCommit(tip);
revisionNoteMap =
RevisionNoteMap.parse(
changeNoteJson,
legacyChangeNoteRead,
id,
reader,
NoteMap.read(reader, tipCommit),
PatchLineComment.Status.PUBLISHED);
changeNoteJson, reader, NoteMap.read(reader, tipCommit), Comment.Status.PUBLISHED);
Map<ObjectId, ChangeRevisionNote> rns = revisionNoteMap.revisionNotes;
for (Map.Entry<ObjectId, ChangeRevisionNote> e : rns.entrySet()) {
@@ -773,7 +764,7 @@ class ChangeNotesParser {
labelVoteStr = line.substring(0, s);
PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1));
checkFooter(ident != null, FOOTER_LABEL, line);
effectiveAccountId = legacyChangeNoteRead.parseIdent(ident, id);
effectiveAccountId = parseIdent(ident);
} else {
labelVoteStr = line;
effectiveAccountId = committerId;
@@ -812,7 +803,7 @@ class ChangeNotesParser {
label = line.substring(1, s);
PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1));
checkFooter(ident != null, FOOTER_LABEL, line);
effectiveAccountId = legacyChangeNoteRead.parseIdent(ident, id);
effectiveAccountId = parseIdent(ident);
} else {
label = line.substring(1);
effectiveAccountId = committerId;
@@ -871,7 +862,7 @@ class ChangeNotesParser {
label.label = line.substring(c + 2, c2);
PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(c2 + 2));
checkFooter(ident != null, FOOTER_SUBMITTED_WITH, line);
label.appliedBy = legacyChangeNoteRead.parseIdent(ident, id);
label.appliedBy = parseIdent(ident);
} else {
label.label = line.substring(c + 2);
}
@@ -887,7 +878,7 @@ class ChangeNotesParser {
if (a.getName().equals(c.getName()) && a.getEmailAddress().equals(c.getEmailAddress())) {
return null;
}
return legacyChangeNoteRead.parseIdent(commit.getAuthorIdent(), id);
return parseIdent(commit.getAuthorIdent());
}
private void parseReviewer(Timestamp ts, ReviewerStateInternal state, String line)
@@ -896,7 +887,7 @@ class ChangeNotesParser {
if (ident == null) {
throw invalidFooter(state.getFooterKey(), line);
}
Account.Id accountId = legacyChangeNoteRead.parseIdent(ident, id);
Account.Id accountId = parseIdent(ident);
reviewerUpdates.add(ReviewerStatusUpdate.create(ts, ownerId, accountId, state));
if (!reviewers.containsRow(accountId)) {
reviewers.put(accountId, state, ts);
@@ -1096,4 +1087,10 @@ class ChangeNotesParser {
private ConfigInvalidException parseException(String fmt, Object... args) {
return ChangeNotes.parseException(id, fmt, args);
}
private Account.Id parseIdent(PersonIdent ident) throws ConfigInvalidException {
return NoteDbUtil.parseIdent(ident)
.orElseThrow(
() -> parseException("cannot retrieve account id: %s", ident.getEmailAddress()));
}
}

59
java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
View File

@@ -16,10 +16,7 @@ package com.google.gerrit.server.notedb;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.primitives.Bytes;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.PatchLineComment;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
@@ -30,30 +27,16 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.util.MutableInteger;
import org.eclipse.jgit.util.RawParseUtils;
class ChangeRevisionNote extends RevisionNote<Comment> {
private static final byte[] CERT_HEADER = "certificate version ".getBytes(UTF_8);
// See org.eclipse.jgit.transport.PushCertificateParser.END_SIGNATURE
private static final byte[] END_SIGNATURE = "-----END PGP SIGNATURE-----\n".getBytes(UTF_8);
private final ChangeNoteJson noteJson;
private final LegacyChangeNoteRead legacyChangeNoteRead;
private final Change.Id changeId;
private final PatchLineComment.Status status;
private final Comment.Status status;
private String pushCert;
ChangeRevisionNote(
ChangeNoteJson noteJson,
LegacyChangeNoteRead legacyChangeNoteRead,
Change.Id changeId,
ObjectReader reader,
ObjectId noteId,
PatchLineComment.Status status) {
ChangeNoteJson noteJson, ObjectReader reader, ObjectId noteId, Comment.Status status) {
super(reader, noteId);
this.legacyChangeNoteRead = legacyChangeNoteRead;
this.noteJson = noteJson;
this.changeId = changeId;
this.status = status;
}
@@ -67,29 +50,13 @@ class ChangeRevisionNote extends RevisionNote<Comment> {
MutableInteger p = new MutableInteger();
p.value = offset;
if (isJson(raw, p.value)) {
RevisionNoteData data = parseJson(noteJson, raw, p.value);
if (status == PatchLineComment.Status.PUBLISHED) {
pushCert = data.pushCert;
} else {
pushCert = null;
}
return data.comments;
}
if (status == PatchLineComment.Status.PUBLISHED) {
pushCert = parsePushCert(changeId, raw, p);
trimLeadingEmptyLines(raw, p);
RevisionNoteData data = parseJson(noteJson, raw, p.value);
if (status == Comment.Status.PUBLISHED) {
pushCert = data.pushCert;
} else {
pushCert = null;
}
List<Comment> comments = legacyChangeNoteRead.parseNote(raw, p, changeId);
comments.forEach(c -> c.legacyFormat = true);
return comments;
}
static boolean isJson(byte[] raw, int offset) {
return raw[offset] == '{' || raw[offset] == '[';
return data.comments;
}
private RevisionNoteData parseJson(ChangeNoteJson noteUtil, byte[] raw, int offset)
@@ -99,18 +66,4 @@ class ChangeRevisionNote extends RevisionNote<Comment> {
return noteUtil.getGson().fromJson(r, RevisionNoteData.class);
}
}
private static String parsePushCert(Change.Id changeId, byte[] bytes, MutableInteger p)
throws ConfigInvalidException {
if (RawParseUtils.match(bytes, p.value, CERT_HEADER) < 0) {
return null;
}
int end = Bytes.indexOf(bytes, END_SIGNATURE);
if (end < 0) {
throw ChangeNotes.parseException(changeId, "invalid push certificate in note");
}
int start = p.value;
p.value = end + END_SIGNATURE.length;
return new String(bytes, start, p.value, UTF_8);
}
}

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

@@ -53,7 +53,6 @@ import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.PatchLineComment;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RobotComment;
import com.google.gerrit.exceptions.StorageException;
@@ -273,10 +272,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
this.psDescription = psDescription;
}
public void putComment(PatchLineComment.Status status, Comment c) {
public void putComment(Comment.Status status, Comment c) {
verifyComment(c);
createDraftUpdateIfNull();
if (status == PatchLineComment.Status.DRAFT) {
if (status == Comment.Status.DRAFT) {
draftUpdate.putComment(c);
} else {
comments.add(c);
@@ -462,12 +461,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
// Even though reading from changes might not be enabled, we need to
// parse any existing revision notes so we can merge them.
return RevisionNoteMap.parse(
noteUtil.getChangeNoteJson(),
noteUtil.getLegacyChangeNoteRead(),
getId(),
rw.getObjectReader(),
noteMap,
PatchLineComment.Status.PUBLISHED);
noteUtil.getChangeNoteJson(), rw.getObjectReader(), noteMap, Comment.Status.PUBLISHED);
}
private void checkComments(

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

@@ -15,7 +15,7 @@
package com.google.gerrit.server.notedb;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.gerrit.entities.PatchLineComment.Status.PUBLISHED;
import static com.google.gerrit.entities.Comment.Status;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toMap;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
@@ -95,13 +95,13 @@ public class DeleteCommentRewriter implements NoteDbRewriter {
ObjectReader reader = revWalk.getObjectReader();
RevCommit newTipCommit = revWalk.next(); // The first commit will not be rewritten.
Map<String, Comment> parentComments =
getPublishedComments(noteUtil, changeId, reader, NoteMap.read(reader, newTipCommit));
getPublishedComments(noteUtil, reader, NoteMap.read(reader, newTipCommit));
boolean rewrite = false;
RevCommit originalCommit;
while ((originalCommit = revWalk.next()) != null) {
NoteMap noteMap = NoteMap.read(reader, originalCommit);
Map<String, Comment> currComments = getPublishedComments(noteUtil, changeId, reader, noteMap);
Map<String, Comment> currComments = getPublishedComments(noteUtil, reader, noteMap);
if (!rewrite && currComments.containsKey(uuid)) {
rewrite = true;
@@ -131,28 +131,18 @@ public class DeleteCommentRewriter implements NoteDbRewriter {
*/
@VisibleForTesting
public static Map<String, Comment> getPublishedComments(
ChangeNoteJson changeNoteJson,
LegacyChangeNoteRead legacyChangeNoteRead,
Change.Id changeId,
ObjectReader reader,
NoteMap noteMap)
ChangeNoteJson changeNoteJson, ObjectReader reader, NoteMap noteMap)
throws IOException, ConfigInvalidException {
return RevisionNoteMap.parse(
changeNoteJson, legacyChangeNoteRead, changeId, reader, noteMap, PUBLISHED)
.revisionNotes.values().stream()
return RevisionNoteMap.parse(changeNoteJson, reader, noteMap, Status.PUBLISHED).revisionNotes
.values().stream()
.flatMap(n -> n.getEntities().stream())
.collect(toMap(c -> c.key.uuid, Function.identity()));
}
public static Map<String, Comment> getPublishedComments(
ChangeNoteUtil noteUtil, Change.Id changeId, ObjectReader reader, NoteMap noteMap)
ChangeNoteUtil noteUtil, ObjectReader reader, NoteMap noteMap)
throws IOException, ConfigInvalidException {
return getPublishedComments(
noteUtil.getChangeNoteJson(),
noteUtil.getLegacyChangeNoteRead(),
changeId,
reader,
noteMap);
return getPublishedComments(noteUtil.getChangeNoteJson(), reader, noteMap);
}
/**
* Gets the comments put in by the current commit. The message of the target comment will be
@@ -215,11 +205,9 @@ public class DeleteCommentRewriter implements NoteDbRewriter {
RevisionNoteMap<ChangeRevisionNote> revNotesMap =
RevisionNoteMap.parse(
noteUtil.getChangeNoteJson(),
noteUtil.getLegacyChangeNoteRead(),
changeId,
reader,
NoteMap.read(reader, parentCommit),
PUBLISHED);
Status.PUBLISHED);
RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(revNotesMap);
for (Comment c : putInComments) {

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

@@ -27,7 +27,6 @@ import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.PatchLineComment;
import com.google.gerrit.entities.Project;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -121,12 +120,7 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
ObjectReader reader = handle.walk().getObjectReader();
revisionNoteMap =
RevisionNoteMap.parse(
args.changeNoteJson,
args.legacyChangeNoteRead,
getChangeId(),
reader,
NoteMap.read(reader, tipCommit),
PatchLineComment.Status.DRAFT);
args.changeNoteJson, reader, NoteMap.read(reader, tipCommit), Comment.Status.DRAFT);
ListMultimap<ObjectId, Comment> cs = MultimapBuilder.hashKeys().arrayListValues().build();
for (ChangeRevisionNote rn : revisionNoteMap.revisionNotes.values()) {
for (Comment c : rn.getEntities()) {

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

@@ -1,399 +0,0 @@
// Copyright (C) 2018 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.notedb;
import static com.google.gerrit.server.notedb.ChangeNotes.parseException;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.CommentRange;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.server.config.GerritServerId;
import com.google.inject.Inject;
import java.sql.Timestamp;
import java.text.ParseException;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.util.GitDateParser;
import org.eclipse.jgit.util.MutableInteger;
import org.eclipse.jgit.util.QuotedString;
import org.eclipse.jgit.util.RawParseUtils;
public class LegacyChangeNoteRead {
private final String serverId;
@Inject
public LegacyChangeNoteRead(@GerritServerId String serverId) {
this.serverId = serverId;
}
public Account.Id parseIdent(PersonIdent ident, Change.Id changeId)
throws ConfigInvalidException {
return NoteDbUtil.parseIdent(ident)
.orElseThrow(
() ->
parseException(
changeId, "cannot retrieve account id: %s", ident.getEmailAddress()));
}
private static boolean match(byte[] note, MutableInteger p, byte[] expected) {
int m = RawParseUtils.match(note, p.value, expected);
return m == p.value + expected.length;
}
public List<Comment> parseNote(byte[] note, MutableInteger p, Change.Id changeId)
throws ConfigInvalidException {
if (p.value >= note.length) {
return ImmutableList.of();
}
Set<Comment.Key> seen = new HashSet<>();
List<Comment> result = new ArrayList<>();
int sizeOfNote = note.length;
byte[] psb = ChangeNoteUtil.PATCH_SET.getBytes(UTF_8);
byte[] bpsb = ChangeNoteUtil.BASE_PATCH_SET.getBytes(UTF_8);
byte[] bpn = ChangeNoteUtil.PARENT_NUMBER.getBytes(UTF_8);
ObjectId commitId =
ObjectId.fromString(parseStringField(note, p, changeId, ChangeNoteUtil.REVISION));
String fileName = null;
PatchSet.Id psId = null;
boolean isForBase = false;
Integer parentNumber = null;
while (p.value < sizeOfNote) {
boolean matchPs = match(note, p, psb);
boolean matchBase = match(note, p, bpsb);
if (matchPs) {
fileName = null;
psId = parsePsId(note, p, changeId, ChangeNoteUtil.PATCH_SET);
isForBase = false;
} else if (matchBase) {
fileName = null;
psId = parsePsId(note, p, changeId, ChangeNoteUtil.BASE_PATCH_SET);
isForBase = true;
if (match(note, p, bpn)) {
parentNumber = parseParentNumber(note, p, changeId);
}
} else if (psId == null) {
throw parseException(
changeId,
"missing %s or %s header",
ChangeNoteUtil.PATCH_SET,
ChangeNoteUtil.BASE_PATCH_SET);
}
Comment c = parseComment(note, p, fileName, psId, commitId, isForBase, parentNumber);
fileName = c.key.filename;
if (!seen.add(c.key)) {
throw parseException(changeId, "multiple comments for %s in note", c.key);
}
result.add(c);
}
return result;
}
private Comment parseComment(
byte[] note,
MutableInteger curr,
String currentFileName,
PatchSet.Id psId,
ObjectId commitId,
boolean isForBase,
Integer parentNumber)
throws ConfigInvalidException {
Change.Id changeId = psId.changeId();
// Check if there is a new file.
boolean newFile =
(RawParseUtils.match(note, curr.value, ChangeNoteUtil.FILE.getBytes(UTF_8))) != -1;
if (newFile) {
// If so, parse the new file name.
currentFileName = parseFilename(note, curr, changeId);
} else if (currentFileName == null) {
throw parseException(changeId, "could not parse %s", ChangeNoteUtil.FILE);
}
CommentRange range = parseCommentRange(note, curr);
if (range == null) {
throw parseException(changeId, "could not parse %s", ChangeNoteUtil.COMMENT_RANGE);
}
Timestamp commentTime = parseTimestamp(note, curr, changeId);
Account.Id aId = parseAuthor(note, curr, changeId, ChangeNoteUtil.AUTHOR);
boolean hasRealAuthor =
(RawParseUtils.match(note, curr.value, ChangeNoteUtil.REAL_AUTHOR.getBytes(UTF_8))) != -1;
Account.Id raId = null;
if (hasRealAuthor) {
raId = parseAuthor(note, curr, changeId, ChangeNoteUtil.REAL_AUTHOR);
}
boolean hasParent =
(RawParseUtils.match(note, curr.value, ChangeNoteUtil.PARENT.getBytes(UTF_8))) != -1;
String parentUUID = null;
boolean unresolved = false;
if (hasParent) {
parentUUID = parseStringField(note, curr, changeId, ChangeNoteUtil.PARENT);
}
boolean hasUnresolved =
(RawParseUtils.match(note, curr.value, ChangeNoteUtil.UNRESOLVED.getBytes(UTF_8))) != -1;
if (hasUnresolved) {
unresolved = parseBooleanField(note, curr, changeId, ChangeNoteUtil.UNRESOLVED);
}
String uuid = parseStringField(note, curr, changeId, ChangeNoteUtil.UUID);
boolean hasTag =
(RawParseUtils.match(note, curr.value, ChangeNoteUtil.TAG.getBytes(UTF_8))) != -1;
String tag = null;
if (hasTag) {
tag = parseStringField(note, curr, changeId, ChangeNoteUtil.TAG);
}
int commentLength = parseCommentLength(note, curr, changeId);
String message = RawParseUtils.decode(UTF_8, note, curr.value, curr.value + commentLength);
checkResult(message, "message contents", changeId);
Comment c =
new Comment(
new Comment.Key(uuid, currentFileName, psId.get()),
aId,
commentTime,
isForBase ? (short) (parentNumber == null ? 0 : -parentNumber) : (short) 1,
message,
serverId,
unresolved);
c.lineNbr = range.getEndLine();
c.parentUuid = parentUUID;
c.tag = tag;
c.setCommitId(commitId);
if (raId != null) {
c.setRealAuthor(raId);
}
if (range.getStartCharacter() != -1) {
c.setRange(range);
}
curr.value = RawParseUtils.nextLF(note, curr.value + commentLength);
curr.value = RawParseUtils.nextLF(note, curr.value);
return c;
}
private static String parseStringField(
byte[] note, MutableInteger curr, Change.Id changeId, String fieldName)
throws ConfigInvalidException {
int endOfLine = RawParseUtils.nextLF(note, curr.value);
checkHeaderLineFormat(note, curr, fieldName, changeId);
int startOfField = RawParseUtils.endOfFooterLineKey(note, curr.value) + 2;
curr.value = endOfLine;
return RawParseUtils.decode(UTF_8, note, startOfField, endOfLine - 1);
}
/**
* @return a comment range. If the comment range line in the note only has one number, we return a
* CommentRange with that one number as the end line and the other fields as -1. If the
* comment range line in the note contains a whole comment range, then we return a
* CommentRange with all fields set. If the line is not correctly formatted, return null.
*/
private static CommentRange parseCommentRange(byte[] note, MutableInteger ptr) {
CommentRange range = new CommentRange(-1, -1, -1, -1);
int last = ptr.value;
int startLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
if (ptr.value == last) {
return null;
} else if (note[ptr.value] == '\n') {
range.setEndLine(startLine);
ptr.value += 1;
return range;
} else if (note[ptr.value] == ':') {
range.setStartLine(startLine);
ptr.value += 1;
} else {
return null;
}
last = ptr.value;
int startChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
if (ptr.value == last) {
return null;
} else if (note[ptr.value] == '-') {
range.setStartCharacter(startChar);
ptr.value += 1;
} else {
return null;
}
last = ptr.value;
int endLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
if (ptr.value == last) {
return null;
} else if (note[ptr.value] == ':') {
range.setEndLine(endLine);
ptr.value += 1;
} else {
return null;
}
last = ptr.value;
int endChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
if (ptr.value == last) {
return null;
} else if (note[ptr.value] == '\n') {
range.setEndCharacter(endChar);
ptr.value += 1;
} else {
return null;
}
return range;
}
private static PatchSet.Id parsePsId(
byte[] note, MutableInteger curr, Change.Id changeId, String fieldName)
throws ConfigInvalidException {
checkHeaderLineFormat(note, curr, fieldName, changeId);
int startOfPsId = RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
MutableInteger i = new MutableInteger();
int patchSetId = RawParseUtils.parseBase10(note, startOfPsId, i);
int endOfLine = RawParseUtils.nextLF(note, curr.value);
if (i.value != endOfLine - 1) {
throw parseException(changeId, "could not parse %s", fieldName);
}
checkResult(patchSetId, "patchset id", changeId);
curr.value = endOfLine;
return PatchSet.id(changeId, patchSetId);
}
private static Integer parseParentNumber(byte[] note, MutableInteger curr, Change.Id changeId)
throws ConfigInvalidException {
checkHeaderLineFormat(note, curr, ChangeNoteUtil.PARENT_NUMBER, changeId);
int start = RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
MutableInteger i = new MutableInteger();
int parentNumber = RawParseUtils.parseBase10(note, start, i);
int endOfLine = RawParseUtils.nextLF(note, curr.value);
if (i.value != endOfLine - 1) {
throw parseException(changeId, "could not parse %s", ChangeNoteUtil.PARENT_NUMBER);
}
checkResult(parentNumber, "parent number", changeId);
curr.value = endOfLine;
return Integer.valueOf(parentNumber);
}
private static String parseFilename(byte[] note, MutableInteger curr, Change.Id changeId)
throws ConfigInvalidException {
checkHeaderLineFormat(note, curr, ChangeNoteUtil.FILE, changeId);
int startOfFileName = RawParseUtils.endOfFooterLineKey(note, curr.value) + 2;
int endOfLine = RawParseUtils.nextLF(note, curr.value);
curr.value = endOfLine;
curr.value = RawParseUtils.nextLF(note, curr.value);
return QuotedString.GIT_PATH.dequote(
RawParseUtils.decode(UTF_8, note, startOfFileName, endOfLine - 1));
}
private static Timestamp parseTimestamp(byte[] note, MutableInteger curr, Change.Id changeId)
throws ConfigInvalidException {
int endOfLine = RawParseUtils.nextLF(note, curr.value);
Timestamp commentTime;
String dateString = RawParseUtils.decode(UTF_8, note, curr.value, endOfLine - 1);
try {
commentTime = new Timestamp(GitDateParser.parse(dateString, null, Locale.US).getTime());
} catch (ParseException e) {
throw new ConfigInvalidException("could not parse comment timestamp", e);
}
curr.value = endOfLine;
return checkResult(commentTime, "comment timestamp", changeId);
}
private Account.Id parseAuthor(
byte[] note, MutableInteger curr, Change.Id changeId, String fieldName)
throws ConfigInvalidException {
checkHeaderLineFormat(note, curr, fieldName, changeId);
int startOfAccountId = RawParseUtils.endOfFooterLineKey(note, curr.value) + 2;
PersonIdent ident = RawParseUtils.parsePersonIdent(note, startOfAccountId);
Account.Id aId = parseIdent(ident, changeId);
curr.value = RawParseUtils.nextLF(note, curr.value);
return checkResult(aId, fieldName, changeId);
}
private static int parseCommentLength(byte[] note, MutableInteger curr, Change.Id changeId)
throws ConfigInvalidException {
checkHeaderLineFormat(note, curr, ChangeNoteUtil.LENGTH, changeId);
int startOfLength = RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
MutableInteger i = new MutableInteger();
i.value = startOfLength;
int commentLength = RawParseUtils.parseBase10(note, startOfLength, i);
if (i.value == startOfLength) {
throw parseException(changeId, "could not parse %s", ChangeNoteUtil.LENGTH);
}
int endOfLine = RawParseUtils.nextLF(note, curr.value);
if (i.value != endOfLine - 1) {
throw parseException(changeId, "could not parse %s", ChangeNoteUtil.LENGTH);
}
curr.value = endOfLine;
return checkResult(commentLength, "comment length", changeId);
}
private boolean parseBooleanField(
byte[] note, MutableInteger curr, Change.Id changeId, String fieldName)
throws ConfigInvalidException {
String str = parseStringField(note, curr, changeId, fieldName);
if ("true".equalsIgnoreCase(str)) {
return true;
} else if ("false".equalsIgnoreCase(str)) {
return false;
}
throw parseException(changeId, "invalid boolean for %s: %s", fieldName, str);
}
private static <T> T checkResult(T o, String fieldName, Change.Id changeId)
throws ConfigInvalidException {
if (o == null) {
throw parseException(changeId, "could not parse %s", fieldName);
}
return o;
}
private static int checkResult(int i, String fieldName, Change.Id changeId)
throws ConfigInvalidException {
if (i <= 0) {
throw parseException(changeId, "could not parse %s", fieldName);
}
return i;
}
private static void checkHeaderLineFormat(
byte[] note, MutableInteger curr, String fieldName, Change.Id changeId)
throws ConfigInvalidException {
boolean correct = RawParseUtils.match(note, curr.value, fieldName.getBytes(UTF_8)) != -1;
int p = curr.value + fieldName.length();
correct &= (p < note.length && note[p] == ':');
p++;
correct &= (p < note.length && note[p] == ' ');
if (!correct) {
throw parseException(changeId, "could not parse %s", fieldName);
}
}
}

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

@@ -1,198 +0,0 @@
// Copyright (C) 2018 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.notedb;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.server.CommentsUtil.COMMENT_ORDER;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ListMultimap;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.GerritServerId;
import com.google.inject.Inject;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.sql.Timestamp;
import java.util.Date;
import java.util.List;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.util.QuotedString;
public class LegacyChangeNoteWrite {
private final PersonIdent serverIdent;
private final String serverId;
@Inject
public LegacyChangeNoteWrite(
@GerritPersonIdent PersonIdent serverIdent, @GerritServerId String serverId) {
this.serverIdent = serverIdent;
this.serverId = serverId;
}
public PersonIdent newIdent(Account.Id authorId, Date when, PersonIdent serverIdent) {
return new PersonIdent(
authorId.toString(), authorId.get() + "@" + serverId, when, serverIdent.getTimeZone());
}
@VisibleForTesting
public PersonIdent newIdent(Account author, Date when, PersonIdent serverIdent) {
return new PersonIdent(
author.toString(), author.id().get() + "@" + serverId, when, serverIdent.getTimeZone());
}
public String getServerId() {
return serverId;
}
private void appendHeaderField(PrintWriter writer, String field, String value) {
writer.print(field);
writer.print(": ");
writer.print(value);
writer.print('\n');
}
/**
* Build a note that contains the metadata for and the contents of all of the comments in the
* given comments.
*
* @param comments Comments to be written to the output stream, keyed by patch set ID; multiple
* patch sets are allowed since base revisions may be shared across patch sets. All of the
* comments must share the same commitId, and all the comments for a given patch set must have
* the same side.
* @param out output stream to write to.
*/
@UsedAt(UsedAt.Project.GOOGLE)
public void buildNote(ListMultimap<Integer, Comment> comments, OutputStream out) {
if (comments.isEmpty()) {
return;
}
ImmutableList<Integer> psIds = comments.keySet().stream().sorted().collect(toImmutableList());
OutputStreamWriter streamWriter = new OutputStreamWriter(out, UTF_8);
try (PrintWriter writer = new PrintWriter(streamWriter)) {
ObjectId commitId = comments.values().iterator().next().getCommitId();
String commitName = commitId.name();
appendHeaderField(writer, ChangeNoteUtil.REVISION, commitName);
for (int psId : psIds) {
List<Comment> psComments = COMMENT_ORDER.sortedCopy(comments.get(psId));
Comment first = psComments.get(0);
short side = first.side;
appendHeaderField(
writer,
side <= 0 ? ChangeNoteUtil.BASE_PATCH_SET : ChangeNoteUtil.PATCH_SET,
Integer.toString(psId));
if (side < 0) {
appendHeaderField(writer, ChangeNoteUtil.PARENT_NUMBER, Integer.toString(-side));
}
String currentFilename = null;
for (Comment c : psComments) {
checkArgument(
commitId.equals(c.getCommitId()),
"All comments being added must have all the same commitId. The "
+ "comment below does not have the same commitId as the others "
+ "(%s).\n%s",
commitId,
c);
checkArgument(
side == c.side,
"All comments being added must all have the same side. The "
+ "comment below does not have the same side as the others "
+ "(%s).\n%s",
side,
c);
String commentFilename = QuotedString.GIT_PATH.quote(c.key.filename);
if (!commentFilename.equals(currentFilename)) {
currentFilename = commentFilename;
writer.print("File: ");
writer.print(commentFilename);
writer.print("\n\n");
}
appendOneComment(writer, c);
}
}
}
}
private void appendOneComment(PrintWriter writer, Comment c) {
// The CommentRange field for a comment is allowed to be null. If it is
// null, then in the first line, we simply use the line number field for a
// comment instead. If it isn't null, we write the comment range itself.
Comment.Range range = c.range;
if (range != null) {
writer.print(range.startLine);
writer.print(':');
writer.print(range.startChar);
writer.print('-');
writer.print(range.endLine);
writer.print(':');
writer.print(range.endChar);
} else {
writer.print(c.lineNbr);
}
writer.print("\n");
writer.print(NoteDbUtil.formatTime(serverIdent, c.writtenOn));
writer.print("\n");
appendIdent(writer, ChangeNoteUtil.AUTHOR, c.author.getId(), c.writtenOn);
if (!c.getRealAuthor().equals(c.author)) {
appendIdent(writer, ChangeNoteUtil.REAL_AUTHOR, c.getRealAuthor().getId(), c.writtenOn);
}
String parent = c.parentUuid;
if (parent != null) {
appendHeaderField(writer, ChangeNoteUtil.PARENT, parent);
}
appendHeaderField(writer, ChangeNoteUtil.UNRESOLVED, Boolean.toString(c.unresolved));
appendHeaderField(writer, ChangeNoteUtil.UUID, c.key.uuid);
if (c.tag != null) {
appendHeaderField(writer, ChangeNoteUtil.TAG, c.tag);
}
byte[] messageBytes = c.message.getBytes(UTF_8);
appendHeaderField(writer, ChangeNoteUtil.LENGTH, Integer.toString(messageBytes.length));
writer.print(c.message);
writer.print("\n\n");
}
private void appendIdent(PrintWriter writer, String header, Account.Id id, Timestamp ts) {
PersonIdent ident = newIdent(id, ts, serverIdent);
StringBuilder name = new StringBuilder();
PersonIdent.appendSanitized(name, ident.getName());
name.append(" <");
PersonIdent.appendSanitized(name, ident.getEmailAddress());
name.append('>');
appendHeaderField(writer, header, name.toString());
}
}

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

@@ -15,9 +15,7 @@
package com.google.gerrit.server.notedb;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.PatchLineComment;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
@@ -32,18 +30,11 @@ class RevisionNoteMap<T extends RevisionNote<? extends Comment>> {
final ImmutableMap<ObjectId, T> revisionNotes;
static RevisionNoteMap<ChangeRevisionNote> parse(
ChangeNoteJson noteJson,
LegacyChangeNoteRead legacyChangeNoteRead,
Change.Id changeId,
ObjectReader reader,
NoteMap noteMap,
PatchLineComment.Status status)
ChangeNoteJson noteJson, ObjectReader reader, NoteMap noteMap, Comment.Status status)
throws ConfigInvalidException, IOException {
Map<ObjectId, ChangeRevisionNote> result = new HashMap<>();
for (Note note : noteMap) {
ChangeRevisionNote rn =
new ChangeRevisionNote(
noteJson, legacyChangeNoteRead, changeId, reader, note.getData(), status);
ChangeRevisionNote rn = new ChangeRevisionNote(noteJson, reader, note.getData(), status);
rn.parse();
result.put(note.copy(), rn);
}

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

@@ -18,7 +18,6 @@ import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
import com.google.common.base.Strings;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.PatchLineComment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.common.CommentInfo;
@@ -121,7 +120,8 @@ public class CreateDraftComment
setCommentCommitId(comment, patchListCache, ctx.getChange(), ps);
commentsUtil.putComments(ctx.getUpdate(psId), Status.DRAFT, Collections.singleton(comment));
commentsUtil.putComments(
ctx.getUpdate(psId), Comment.Status.DRAFT, Collections.singleton(comment));
return true;
}
}

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

@@ -49,7 +49,6 @@ import com.google.gerrit.entities.FixReplacement;
import com.google.gerrit.entities.FixSuggestion;
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.PatchLineComment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.RobotComment;
@@ -974,7 +973,7 @@ public class PostReview
break;
}
ChangeUpdate changeUpdate = ctx.getUpdate(psId);
commentsUtil.putComments(changeUpdate, Status.PUBLISHED, toPublish);
commentsUtil.putComments(changeUpdate, Comment.Status.PUBLISHED, toPublish);
comments.addAll(toPublish);
return !toPublish.isEmpty();
}

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

@@ -17,7 +17,6 @@ package com.google.gerrit.server.restapi.change;
import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.PatchLineComment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.common.CommentInfo;
@@ -140,7 +139,7 @@ public class PutDraftComment
}
setCommentCommitId(comment, patchListCache, ctx.getChange(), ps);
commentsUtil.putComments(
update, Status.DRAFT, Collections.singleton(update(comment, in, ctx.getWhen())));
update, Comment.Status.DRAFT, Collections.singleton(update(comment, in, ctx.getWhen())));
return true;
}
}

4
javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
View File

@@ -1058,10 +1058,10 @@ public class CommentsIT extends AbstractDaemonTest {
Map<String, com.google.gerrit.entities.Comment> commentMapBefore =
DeleteCommentRewriter.getPublishedComments(
noteUtil, changeId, reader, NoteMap.read(reader, commitBefore));
noteUtil, reader, NoteMap.read(reader, commitBefore));
Map<String, com.google.gerrit.entities.Comment> commentMapAfter =
DeleteCommentRewriter.getPublishedComments(
noteUtil, changeId, reader, NoteMap.read(reader, commitAfter));
noteUtil, reader, NoteMap.read(reader, commitAfter));
if (commentMapBefore.containsKey(targetCommentUuid)) {
assertThat(commentMapAfter).containsKey(targetCommentUuid);

4
javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
View File

@@ -571,8 +571,6 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
private ChangeNotesParser newParser(ObjectId tip) throws Exception {
walk.reset();
ChangeNoteJson changeNoteJson = injector.getInstance(ChangeNoteJson.class);
LegacyChangeNoteRead reader = injector.getInstance(LegacyChangeNoteRead.class);
return new ChangeNotesParser(
newChange().getId(), tip, walk, changeNoteJson, reader, args.metrics);
return new ChangeNotesParser(newChange().getId(), tip, walk, changeNoteJson, args.metrics);
}
}

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

@@ -43,7 +43,6 @@ import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.CommentRange;
import com.google.gerrit.entities.PatchLineComment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.exceptions.StorageException;
@@ -75,7 +74,6 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
@Inject private DraftCommentNotes.Factory draftNotesFactory;
@Inject private ChangeNoteJson changeNoteJson;
@Inject private LegacyChangeNoteRead legacyChangeNoteRead;
@Inject private @GerritServerId String serverId;
@@ -121,7 +119,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
RevCommit commit = tr.commit().message("PS2").create();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putComment(
Status.PUBLISHED,
Comment.Status.PUBLISHED,
newComment(
c.currentPatchSetId(),
"a.txt",
@@ -183,7 +181,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
RevCommit commit = tr.commit().message("PS2").create();
update = newUpdate(c, changeOwner);
update.putComment(
Status.PUBLISHED,
Comment.Status.PUBLISHED,
newComment(
c.currentPatchSetId(),
"a.txt",
@@ -1091,7 +1089,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update.putApproval("Code-Review", (short) 1);
update.setChangeMessage("This is a message");
update.putComment(
Status.PUBLISHED,
Comment.Status.PUBLISHED,
newComment(
c.currentPatchSetId(),
"a.txt",
@@ -1191,7 +1189,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update.setPatchSetId(psId2);
Timestamp ts = TimeUtil.nowTs();
update.putComment(
Status.PUBLISHED,
Comment.Status.PUBLISHED,
newComment(
psId2,
"a.txt",
@@ -1276,7 +1274,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
false);
update1.setPatchSetId(psId);
update1.putComment(Status.PUBLISHED, comment1);
update1.putComment(Comment.Status.PUBLISHED, comment1);
updateManager.add(update1);
ChangeUpdate update2 = newUpdate(c, otherUser);
@@ -1298,12 +1296,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
try (ChangeNotesRevWalk rw = ChangeNotesCommit.newRevWalk(repo)) {
ChangeNotesParser notesWithComments =
new ChangeNotesParser(
c.getId(),
commitWithComments.copy(),
rw,
changeNoteJson,
legacyChangeNoteRead,
args.metrics);
c.getId(), commitWithComments.copy(), rw, changeNoteJson, args.metrics);
ChangeNotesState state = notesWithComments.parseAll();
assertThat(state.approvals()).isEmpty();
assertThat(state.publishedComments()).hasSize(1);
@@ -1312,12 +1305,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
try (ChangeNotesRevWalk rw = ChangeNotesCommit.newRevWalk(repo)) {
ChangeNotesParser notesWithApprovals =
new ChangeNotesParser(
c.getId(),
commitWithApprovals.copy(),
rw,
changeNoteJson,
legacyChangeNoteRead,
args.metrics);
c.getId(), commitWithApprovals.copy(), rw, changeNoteJson, args.metrics);
ChangeNotesState state = notesWithApprovals.parseAll();
assertThat(state.approvals()).hasSize(1);
@@ -1500,7 +1488,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId,
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1530,7 +1518,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId,
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1560,7 +1548,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId,
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1590,7 +1578,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId,
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1659,9 +1647,9 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(psId2);
update.putComment(Status.PUBLISHED, comment3);
update.putComment(Status.PUBLISHED, comment2);
update.putComment(Status.PUBLISHED, comment1);
update.putComment(Comment.Status.PUBLISHED, comment3);
update.putComment(Comment.Status.PUBLISHED, comment2);
update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1701,7 +1689,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
false);
comment.setRealAuthor(changeOwner.getAccountId());
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1739,7 +1727,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1777,7 +1765,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId1,
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, commentForBase);
update.putComment(Comment.Status.PUBLISHED, commentForBase);
update.commit();
update = newUpdate(c, otherUser);
@@ -1796,7 +1784,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId2,
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, commentForPS);
update.putComment(Comment.Status.PUBLISHED, commentForPS);
update.commit();
assertThat(newNotes(c).getComments())
@@ -1835,7 +1823,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId,
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment1);
update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
update = newUpdate(c, otherUser);
@@ -1854,7 +1842,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId,
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment2);
update.putComment(Comment.Status.PUBLISHED, comment2);
update.commit();
assertThat(newNotes(c).getComments())
@@ -1893,7 +1881,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId,
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment1);
update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
update = newUpdate(c, otherUser);
@@ -1912,7 +1900,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId,
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment2);
update.putComment(Comment.Status.PUBLISHED, comment2);
update.commit();
assertThat(newNotes(c).getComments())
@@ -1951,7 +1939,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId1,
false);
update.setPatchSetId(ps1);
update.putComment(Status.PUBLISHED, comment1);
update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
incrementPatchSet(c);
@@ -1974,7 +1962,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId2,
false);
update.setPatchSetId(ps2);
update.putComment(Status.PUBLISHED, comment2);
update.putComment(Comment.Status.PUBLISHED, comment2);
update.commit();
assertThat(newNotes(c).getComments())
@@ -2011,7 +1999,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId,
false);
update.setPatchSetId(ps1);
update.putComment(Status.DRAFT, comment1);
update.putComment(Comment.Status.DRAFT, comment1);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2021,7 +2009,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
update.putComment(Status.PUBLISHED, comment1);
update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
notes = newNotes(c);
@@ -2074,8 +2062,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
side,
commitId,
false);
update.putComment(Status.DRAFT, comment1);
update.putComment(Status.DRAFT, comment2);
update.putComment(Comment.Status.DRAFT, comment1);
update.putComment(Comment.Status.DRAFT, comment2);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2090,7 +2078,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
// Publish first draft.
update = newUpdate(c, otherUser);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment1);
update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
notes = newNotes(c);
@@ -2145,8 +2133,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId2,
false);
update.putComment(Status.DRAFT, baseComment);
update.putComment(Status.DRAFT, psComment);
update.putComment(Comment.Status.DRAFT, baseComment);
update.putComment(Comment.Status.DRAFT, psComment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2161,8 +2149,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update = newUpdate(c, otherUser);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, baseComment);
update.putComment(Status.PUBLISHED, psComment);
update.putComment(Comment.Status.PUBLISHED, baseComment);
update.putComment(Comment.Status.PUBLISHED, psComment);
update.commit();
notes = newNotes(c);
@@ -2201,7 +2189,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId,
false);
update.setPatchSetId(psId);
update.putComment(Status.DRAFT, comment);
update.putComment(Comment.Status.DRAFT, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2246,7 +2234,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId1,
false);
update.setPatchSetId(ps1);
update.putComment(Status.DRAFT, comment1);
update.putComment(Comment.Status.DRAFT, comment1);
update.commit();
incrementPatchSet(c);
@@ -2269,7 +2257,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId2,
false);
update.setPatchSetId(ps2);
update.putComment(Status.DRAFT, comment2);
update.putComment(Comment.Status.DRAFT, comment2);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2313,7 +2301,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
side,
commitId,
false);
update.putComment(Status.PUBLISHED, comment);
update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
assertThat(repo.exactRef(changeMetaRef(c.getId()))).isNotNull();
@@ -2346,7 +2334,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
side,
commitId,
false);
update.putComment(Status.DRAFT, draft);
update.putComment(Comment.Status.DRAFT, draft);
update.commit();
String draftRef = refsDraftComments(c.getId(), otherUser.getAccountId());
@@ -2368,7 +2356,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
side,
commitId,
false);
update.putComment(Status.PUBLISHED, pub);
update.putComment(Comment.Status.PUBLISHED, pub);
update.commit();
assertThat(exactRefAllUsers(draftRef)).isEqualTo(old);
@@ -2399,7 +2387,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId,
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
assertThat(newNotes(c).getComments())
@@ -2431,7 +2419,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
commitId,
false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
assertThat(newNotes(c).getComments())
@@ -2483,8 +2471,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
side,
commitId2,
false);
update.putComment(Status.DRAFT, comment1);
update.putComment(Status.DRAFT, comment2);
update.putComment(Comment.Status.DRAFT, comment1);
update.putComment(Comment.Status.DRAFT, comment2);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2493,8 +2481,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update = newUpdate(c, otherUser);
update.setPatchSetId(ps2);
update.putComment(Status.PUBLISHED, comment1);
update.putComment(Status.PUBLISHED, comment2);
update.putComment(Comment.Status.PUBLISHED, comment1);
update.putComment(Comment.Status.PUBLISHED, comment2);
update.commit();
notes = newNotes(c);
@@ -2541,8 +2529,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
side,
commitId1,
false);
update.putComment(Status.DRAFT, comment1);
update.putComment(Status.DRAFT, comment2);
update.putComment(Comment.Status.DRAFT, comment1);
update.putComment(Comment.Status.DRAFT, comment2);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2552,7 +2540,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
update.putComment(Status.PUBLISHED, comment2);
update.putComment(Comment.Status.PUBLISHED, comment2);
update.commit();
notes = newNotes(c);
@@ -2614,8 +2602,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
side,
commitId1,
false);
update.putComment(Status.DRAFT, comment1);
update.putComment(Status.DRAFT, comment2);
update.putComment(Comment.Status.DRAFT, comment1);
update.putComment(Comment.Status.DRAFT, comment2);
update.commit();
String refName = refsDraftComments(c.getId(), otherUserId);
@@ -2623,7 +2611,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
update.putComment(Status.PUBLISHED, comment2);
update.putComment(Comment.Status.PUBLISHED, comment2);
update.commit();
assertThat(exactRefAllUsers(refName)).isNotNull();
assertThat(exactRefAllUsers(refName)).isNotEqualTo(oldDraftId);
@@ -2649,7 +2637,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
update.putComment(Status.PUBLISHED, comment1);
update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
// Updating an unrelated comment causes the zombie comment to get fixed up.
@@ -2677,7 +2665,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
(short) 1,
commitId,
false);
update1.putComment(Status.PUBLISHED, comment1);
update1.putComment(Comment.Status.PUBLISHED, comment1);
ChangeUpdate update2 = newUpdate(c, otherUser);
Comment comment2 =
@@ -2694,7 +2682,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
(short) 1,
commitId,
false);
update2.putComment(Status.PUBLISHED, comment2);
update2.putComment(Comment.Status.PUBLISHED, comment2);
try (NoteDbUpdateManager manager = updateManagerFactory.create(project)) {
manager.add(update1);
@@ -2751,7 +2739,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
(short) 1,
ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
false);
update.putComment(Status.PUBLISHED, comment);
update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
notes = newNotes(c);

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

@@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.PatchLineComment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.server.util.time.TimeUtil;
import org.eclipse.jgit.lib.ObjectId;
@@ -32,7 +31,7 @@ public class DraftCommentNotesTest extends AbstractChangeNotesTest {
Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(c.currentPatchSetId());
update.putComment(Status.PUBLISHED, comment(c.currentPatchSetId()));
update.putComment(Comment.Status.PUBLISHED, comment(c.currentPatchSetId()));
update.commit();
assertThat(newNotes(c).getDraftComments(otherUserId)).isEmpty();
@@ -45,13 +44,13 @@ public class DraftCommentNotesTest extends AbstractChangeNotesTest {
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(c.currentPatchSetId());
update.putComment(Status.DRAFT, comment(c.currentPatchSetId()));
update.putComment(Comment.Status.DRAFT, comment(c.currentPatchSetId()));
update.commit();
assertThat(newNotes(c).getDraftComments(otherUserId)).hasSize(1);
assertableFanOutExecutor.assertInteractions(0);
update = newUpdate(c, otherUser);
update.putComment(Status.PUBLISHED, comment(c.currentPatchSetId()));
update.putComment(Comment.Status.PUBLISHED, comment(c.currentPatchSetId()));
update.commit();
assertThat(newNotes(c).getDraftComments(otherUserId)).isEmpty();
@@ -64,7 +63,7 @@ public class DraftCommentNotesTest extends AbstractChangeNotesTest {
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(c.currentPatchSetId());
update.putComment(Status.DRAFT, comment(c.currentPatchSetId()));
update.putComment(Comment.Status.DRAFT, comment(c.currentPatchSetId()));
update.commit();
ChangeNotes notes = newNotes(c);