Add draft comments to PatchLineCommentsUtil

I added the ability to read and write published comments from either
the notedb or the ReviewDb depending on the state of the
NotesMigration instance. Additionally, I modified all callers of
PatchLineCommentAccess that query for or edit comments to use the
corresponding methods in PatchLineCommentsUtil.

I added a new test to CommentsTest to test the reading and
writing of draft comments from both the notedb and the ReviewDb with
these new PatchLineCommentsUtil methods.

Finally, I added a full integration test for inline comments in
CommentsIT.java.

Change-Id: Iac4ea144497fe68d28c3e886b91c7698741d6615
This commit is contained in:
Yacob Yonas
2014-07-25 09:45:07 -07:00
parent 0223ebc8cf
commit 5020ff2fd0
24 changed files with 785 additions and 168 deletions

View File

@@ -0,0 +1,198 @@
// Copyright (C) 2014 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.acceptance.server.change;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.common.Comment;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.testutil.ConfigSuite;
import com.google.gson.reflect.TypeToken;
import org.apache.http.HttpStatus;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.Config;
import org.junit.Test;
import java.io.IOException;
import java.lang.reflect.Type;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class CommentsIT extends AbstractDaemonTest {
@ConfigSuite.Config
public static Config noteDbEnabled() {
Config cfg = new Config();
cfg.setBoolean("notedb", null, "write", true);
cfg.setBoolean("notedb", "comments", "read", true);
return cfg;
}
@Test
public void createDraft() throws GitAPIException, IOException {
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
ReviewInput.CommentInput comment = newCommentInfo(
"file1", Comment.Side.REVISION, 1, "comment 1");
addDraft(changeId, revId, comment);
Map<String, List<CommentInfo>> result = getDraftComments(changeId, revId);
assertEquals(1, result.size());
CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
assertCommentInfo(comment, actual);
}
@Test
public void postComment() throws RestApiException, Exception {
String file = "file";
String contents = "contents";
PushOneCommit push = pushFactory.create(db, admin.getIdent(),
"first subject", file, contents);
PushOneCommit.Result r = push.to(git, "refs/for/master");
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
ReviewInput input = new ReviewInput();
ReviewInput.CommentInput comment = newCommentInfo(
file, Comment.Side.REVISION, 1, "comment 1");
input.comments = new HashMap<String, List<ReviewInput.CommentInput>>();
input.comments.put(comment.path, Lists.newArrayList(comment));
revision(r).review(input);
Map<String, List<CommentInfo>> result = getPublishedComments(changeId, revId);
assertTrue(!result.isEmpty());
CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
assertCommentInfo(comment, actual);
}
@Test
public void putDraft() throws GitAPIException, IOException {
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
ReviewInput.CommentInput comment = newCommentInfo(
"file1", Comment.Side.REVISION, 1, "comment 1");
addDraft(changeId, revId, comment);
Map<String, List<CommentInfo>> result = getDraftComments(changeId, revId);
CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
assertCommentInfo(comment, actual);
String uuid = actual.id;
comment.message = "updated comment 1";
updateDraft(changeId, revId, comment, uuid);
result = getDraftComments(changeId, revId);
actual = Iterables.getOnlyElement(result.get(comment.path));
assertCommentInfo(comment, actual);
}
@Test
public void getDraft() throws GitAPIException, IOException {
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
ReviewInput.CommentInput comment = newCommentInfo(
"file1", Comment.Side.REVISION, 1, "comment 1");
CommentInfo returned = addDraft(changeId, revId, comment);
CommentInfo actual = getDraftComment(changeId, revId, returned.id);
assertCommentInfo(comment, actual);
}
@Test
public void deleteDraft() throws IOException, GitAPIException {
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
ReviewInput.CommentInput comment = newCommentInfo(
"file1", Comment.Side.REVISION, 1, "comment 1");
CommentInfo returned = addDraft(changeId, revId, comment);
deleteDraft(changeId, revId, returned.id);
Map<String, List<CommentInfo>> drafts = getDraftComments(changeId, revId);
assertTrue(drafts.isEmpty());
}
private CommentInfo addDraft(String changeId, String revId,
ReviewInput.CommentInput c) throws IOException {
RestResponse r = userSession.put(
"/changes/" + changeId + "/revisions/" + revId + "/drafts", c);
assertEquals(HttpStatus.SC_CREATED, r.getStatusCode());
return newGson().fromJson(r.getReader(), CommentInfo.class);
}
private void updateDraft(String changeId, String revId,
ReviewInput.CommentInput c, String uuid) throws IOException {
RestResponse r = userSession.put(
"/changes/" + changeId + "/revisions/" + revId + "/drafts/" + uuid, c);
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
}
private void deleteDraft(String changeId, String revId, String uuid)
throws IOException {
RestResponse r = userSession.delete(
"/changes/" + changeId + "/revisions/" + revId + "/drafts/" + uuid);
assertEquals(HttpStatus.SC_NO_CONTENT, r.getStatusCode());
}
private Map<String, List<CommentInfo>> getPublishedComments(String changeId,
String revId) throws IOException {
RestResponse r = userSession.get(
"/changes/" + changeId + "/revisions/" + revId + "/comments/");
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
Type mapType = new TypeToken<Map<String, List<CommentInfo>>>() {}.getType();
return newGson().fromJson(r.getReader(), mapType);
}
private Map<String, List<CommentInfo>> getDraftComments(String changeId,
String revId) throws IOException {
RestResponse r = userSession.get(
"/changes/" + changeId + "/revisions/" + revId + "/drafts/");
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
Type mapType = new TypeToken<Map<String, List<CommentInfo>>>() {}.getType();
return newGson().fromJson(r.getReader(), mapType);
}
private CommentInfo getDraftComment(String changeId, String revId,
String uuid) throws IOException {
RestResponse r = userSession.get(
"/changes/" + changeId + "/revisions/" + revId + "/drafts/" + uuid);
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
return newGson().fromJson(r.getReader(), CommentInfo.class);
}
private static void assertCommentInfo(ReviewInput.CommentInput expected,
CommentInfo actual) {
assertEquals(expected.line, actual.line);
assertEquals(expected.message, actual.message);
assertEquals(expected.inReplyTo, actual.inReplyTo);
if (actual.side == null) {
assertEquals(expected.side, Comment.Side.REVISION);
}
}
private ReviewInput.CommentInput newCommentInfo(String path,
Comment.Side side, int line, String message) {
ReviewInput.CommentInput input = new ReviewInput.CommentInput();
input.path = path;
input.side = side;
input.line = line;
input.message = message;
return input;
}
}

View File

@@ -170,7 +170,8 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
// quickly locate where they have pending drafts, and review them. // quickly locate where they have pending drafts, and review them.
// //
final Account.Id me = ((IdentifiedUser) user).getAccountId(); final Account.Id me = ((IdentifiedUser) user).getAccountId();
for (final PatchLineComment c : db.patchComments().draftByPatchSetAuthor(psIdNew, me)) { for (PatchLineComment c
: plcUtil.draftByPatchSetAuthor(db, psIdNew, me, notes)) {
final Patch p = byKey.get(c.getKey().getParentKey()); final Patch p = byKey.get(c.getKey().getParentKey());
if (p != null) { if (p != null) {
p.setDraftCount(p.getDraftCount() + 1); p.setDraftCount(p.getDraftCount() + 1);

View File

@@ -508,8 +508,8 @@ public class ChangeUtil {
ReviewDb db = this.db.get(); ReviewDb db = this.db.get();
db.accountPatchReviews().delete(db.accountPatchReviews().byPatchSet(patchSetId)); db.accountPatchReviews().delete(db.accountPatchReviews().byPatchSet(patchSetId));
db.changeMessages().delete(db.changeMessages().byPatchSet(patchSetId)); db.changeMessages().delete(db.changeMessages().byPatchSet(patchSetId));
db.patchComments().delete(db.patchComments().byPatchSet(patchSetId));
// No need to delete from notedb; draft patch sets will be filtered out. // No need to delete from notedb; draft patch sets will be filtered out.
db.patchComments().delete(db.patchComments().byPatchSet(patchSetId));
db.patchSetApprovals().delete(db.patchSetApprovals().byPatchSet(patchSetId)); db.patchSetApprovals().delete(db.patchSetApprovals().byPatchSet(patchSetId));
db.patchSetAncestors().delete(db.patchSetAncestors().byPatchSet(patchSetId)); db.patchSetAncestors().delete(db.patchSetAncestors().byPatchSet(patchSetId));

View File

@@ -12,25 +12,44 @@
// See the License for the specific language governing permissions and // See the License for the specific language governing permissions and
// limitations under the License. // limitations under the License.
package com.google.gerrit.server; package com.google.gerrit.server;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
import com.google.common.base.Predicate;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
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.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.AllUsersNameProvider;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.DraftCommentNotes;
import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Set;
/** /**
* Utility functions to manipulate PatchLineComments. * Utility functions to manipulate PatchLineComments.
@@ -40,45 +59,179 @@ import java.util.List;
*/ */
@Singleton @Singleton
public class PatchLineCommentsUtil { public class PatchLineCommentsUtil {
private final GitRepositoryManager repoManager;
private final AllUsersName allUsers;
private final DraftCommentNotes.Factory draftFactory;
private final NotesMigration migration; private final NotesMigration migration;
@VisibleForTesting @VisibleForTesting
@Inject @Inject
public PatchLineCommentsUtil(NotesMigration migration) { public PatchLineCommentsUtil(GitRepositoryManager repoManager,
AllUsersNameProvider allUsersProvider,
DraftCommentNotes.Factory draftFactory,
NotesMigration migration) {
this.repoManager = repoManager;
this.allUsers = allUsersProvider.get();
this.draftFactory = draftFactory;
this.migration = migration; this.migration = migration;
} }
public Optional<PatchLineComment> get(ReviewDb db, ChangeNotes notes,
PatchLineComment.Key key) throws OrmException {
if (!migration.readComments()) {
return Optional.fromNullable(db.patchComments().get(key));
}
for (PatchLineComment c : byChange(db, notes)) {
if (key.equals(c.getKey())) {
return Optional.of(c);
}
}
return Optional.absent();
}
public List<PatchLineComment> byChange(ReviewDb db,
ChangeNotes notes) throws OrmException {
if (!migration.readComments()) {
return db.patchComments().byChange(notes.getChangeId()).toList();
}
notes.load();
List<PatchLineComment> comments = Lists.newArrayList();
comments.addAll(notes.getBaseComments().values());
comments.addAll(notes.getPatchSetComments().values());
Iterable<String> filtered = getDraftRefs(notes.getChangeId());
for (String refName : filtered) {
Account.Id account = Account.Id.fromRefPart(refName);
if (account != null) {
comments.addAll(draftByChangeAuthor(db, notes, account));
}
}
return comments;
}
public List<PatchLineComment> byPatchSet(ReviewDb db,
ChangeNotes notes, PatchSet.Id psId) throws OrmException {
if (!migration.readComments()) {
return db.patchComments().byPatchSet(psId).toList();
}
List<PatchLineComment> comments = Lists.newArrayList();
comments.addAll(publishedByPatchSet(db, notes, psId));
Iterable<String> filtered = getDraftRefs(notes.getChangeId());
for (String refName : filtered) {
Account.Id account = Account.Id.fromRefPart(refName);
if (account != null) {
comments.addAll(draftByPatchSetAuthor(db, psId, account, notes));
}
}
return comments;
}
public List<PatchLineComment> publishedByChangeFile(ReviewDb db, public List<PatchLineComment> publishedByChangeFile(ReviewDb db,
ChangeNotes notes, Change.Id changeId, String file) throws OrmException { ChangeNotes notes, Change.Id changeId, String file) throws OrmException {
if (!migration.readPublishedComments()) { if (!migration.readComments()) {
return db.patchComments().publishedByChangeFile(changeId, file).toList(); return db.patchComments().publishedByChangeFile(changeId, file).toList();
} }
notes.load(); notes.load();
List<PatchLineComment> commentsOnFile = new ArrayList<PatchLineComment>(); List<PatchLineComment> comments = Lists.newArrayList();
// We must iterate through all comments to find the ones on this file. addCommentsOnFile(comments, notes.getBaseComments().values(), file);
addCommentsInFile(commentsOnFile, notes.getBaseComments().values(), file); addCommentsOnFile(comments, notes.getPatchSetComments().values(),
addCommentsInFile(commentsOnFile, notes.getPatchSetComments().values(),
file); file);
Collections.sort(comments, ChangeNotes.PatchLineCommentComparator);
Collections.sort(commentsOnFile, ChangeNotes.PatchLineCommentComparator); return comments;
return commentsOnFile;
} }
public List<PatchLineComment> publishedByPatchSet(ReviewDb db, public List<PatchLineComment> publishedByPatchSet(ReviewDb db,
ChangeNotes notes, PatchSet.Id psId) throws OrmException { ChangeNotes notes, PatchSet.Id psId) throws OrmException {
if (!migration.readPublishedComments()) { if (!migration.readComments()) {
return db.patchComments().publishedByPatchSet(psId).toList(); return db.patchComments().publishedByPatchSet(psId).toList();
} }
notes.load(); notes.load();
List<PatchLineComment> commentsOnPs = new ArrayList<PatchLineComment>(); List<PatchLineComment> comments = new ArrayList<PatchLineComment>();
commentsOnPs.addAll(notes.getPatchSetComments().get(psId)); comments.addAll(notes.getPatchSetComments().get(psId));
commentsOnPs.addAll(notes.getBaseComments().get(psId)); comments.addAll(notes.getBaseComments().get(psId));
return commentsOnPs; return comments;
} }
// TODO(yyonas): Delete drafts if they already existed. public List<PatchLineComment> draftByPatchSetAuthor(ReviewDb db,
public void addPublishedComments(ReviewDb db, ChangeUpdate update, PatchSet.Id psId, Account.Id author, ChangeNotes notes)
throws OrmException {
if (!migration.readComments()) {
return db.patchComments().draftByPatchSetAuthor(psId, author).toList();
}
List<PatchLineComment> comments = Lists.newArrayList();
comments.addAll(notes.getDraftBaseComments(author).row(psId).values());
comments.addAll(notes.getDraftPsComments(author).row(psId).values());
Collections.sort(comments, ChangeNotes.PatchLineCommentComparator);
return comments;
}
public List<PatchLineComment> draftByChangeFileAuthor(ReviewDb db,
ChangeNotes notes, String file, Account.Id author)
throws OrmException {
if (!migration.readComments()) {
return db.patchComments()
.draftByChangeFileAuthor(notes.getChangeId(), file, author)
.toList();
}
List<PatchLineComment> comments = Lists.newArrayList();
addCommentsOnFile(comments, notes.getDraftBaseComments(author).values(),
file);
addCommentsOnFile(comments, notes.getDraftPsComments(author).values(),
file);
Collections.sort(comments, ChangeNotes.PatchLineCommentComparator);
return comments;
}
public List<PatchLineComment> draftByChangeAuthor(ReviewDb db,
ChangeNotes notes, Account.Id author)
throws OrmException {
if (!migration.readComments()) {
return db.patchComments().byChange(notes.getChangeId()).toList();
}
List<PatchLineComment> comments = Lists.newArrayList();
comments.addAll(notes.getDraftBaseComments(author).values());
comments.addAll(notes.getDraftPsComments(author).values());
return comments;
}
public List<PatchLineComment> draftByAuthor(ReviewDb db,
Account.Id author) throws OrmException {
if (!migration.readComments()) {
return db.patchComments().draftByAuthor(author).toList();
}
Set<String> refNames =
getRefNamesAllUsers(RefNames.REFS_DRAFT_COMMENTS);
List<PatchLineComment> comments = Lists.newArrayList();
for (String refName : refNames) {
Account.Id id = Account.Id.fromRefPart(refName);
if (!author.equals(id)) {
continue;
}
Change.Id changeId = Change.Id.parse(refName);
DraftCommentNotes draftNotes =
draftFactory.create(changeId, author).load();
comments.addAll(draftNotes.getDraftBaseComments().values());
comments.addAll(draftNotes.getDraftPsComments().values());
}
return comments;
}
public void insertComments(ReviewDb db, ChangeUpdate update,
Iterable<PatchLineComment> comments) throws OrmException {
for (PatchLineComment c : comments) {
update.insertComment(c);
}
db.patchComments().insert(comments);
}
public void upsertComments(ReviewDb db, ChangeUpdate update,
Iterable<PatchLineComment> comments) throws OrmException { Iterable<PatchLineComment> comments) throws OrmException {
for (PatchLineComment c : comments) { for (PatchLineComment c : comments) {
update.upsertComment(c); update.upsertComment(c);
@@ -86,7 +239,23 @@ public class PatchLineCommentsUtil {
db.patchComments().upsert(comments); db.patchComments().upsert(comments);
} }
private static Collection<PatchLineComment> addCommentsInFile( public void updateComments(ReviewDb db, ChangeUpdate update,
Iterable<PatchLineComment> comments) throws OrmException {
for (PatchLineComment c : comments) {
update.updateComment(c);
}
db.patchComments().update(comments);
}
public void deleteComments(ReviewDb db, ChangeUpdate update,
Iterable<PatchLineComment> comments) throws OrmException {
for (PatchLineComment c : comments) {
update.deleteComment(c);
}
db.patchComments().delete(comments);
}
private static Collection<PatchLineComment> addCommentsOnFile(
Collection<PatchLineComment> commentsOnFile, Collection<PatchLineComment> commentsOnFile,
Collection<PatchLineComment> allComments, Collection<PatchLineComment> allComments,
String file) { String file) {
@@ -98,4 +267,49 @@ public class PatchLineCommentsUtil {
} }
return commentsOnFile; return commentsOnFile;
} }
public static void setCommentRevId(PatchLineComment c,
PatchListCache cache, Change change, PatchSet ps) throws OrmException {
if (c.getRevId() != null) {
return;
}
PatchList patchList;
try {
patchList = cache.get(change, ps);
} catch (PatchListNotAvailableException e) {
throw new OrmException(e);
}
c.setRevId((c.getSide() == (short) 0)
? new RevId(ObjectId.toString(patchList.getOldId()))
: new RevId(ObjectId.toString(patchList.getNewId())));
}
private Set<String> getRefNamesAllUsers(String prefix) throws OrmException {
Repository repo;
try {
repo = repoManager.openRepository(allUsers);
} catch (IOException e) {
throw new OrmException(e);
}
try {
RefDatabase refDb = repo.getRefDatabase();
return refDb.getRefs(prefix).keySet();
} catch (IOException e) {
throw new OrmException(e);
} finally {
repo.close();
}
}
private Iterable<String> getDraftRefs(final Change.Id changeId)
throws OrmException {
Set<String> refNames = getRefNamesAllUsers(RefNames.REFS_DRAFT_COMMENTS);
final String suffix = "-" + changeId.get();
return Iterables.filter(refNames, new Predicate<String>() {
@Override
public boolean apply(String input) {
return input.endsWith(suffix);
}
});
}
} }

View File

@@ -77,6 +77,7 @@ import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.ChangeMessagesUtil;
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.PatchLineCommentsUtil;
import com.google.gerrit.server.WebLinks; import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.account.AccountInfo; import com.google.gerrit.server.account.AccountInfo;
import com.google.gerrit.server.extensions.webui.UiActions; import com.google.gerrit.server.extensions.webui.UiActions;
@@ -127,6 +128,7 @@ public class ChangeJson {
private final Provider<WebLinks> webLinks; private final Provider<WebLinks> webLinks;
private final EnumSet<ListChangesOption> options; private final EnumSet<ListChangesOption> options;
private final ChangeMessagesUtil cmUtil; private final ChangeMessagesUtil cmUtil;
private final PatchLineCommentsUtil plcUtil;
private AccountInfo.Loader accountLoader; private AccountInfo.Loader accountLoader;
@@ -147,7 +149,8 @@ public class ChangeJson {
DynamicMap<RestView<ChangeResource>> changeViews, DynamicMap<RestView<ChangeResource>> changeViews,
Revisions revisions, Revisions revisions,
Provider<WebLinks> webLinks, Provider<WebLinks> webLinks,
ChangeMessagesUtil cmUtil) { ChangeMessagesUtil cmUtil,
PatchLineCommentsUtil plcUtil) {
this.db = db; this.db = db;
this.labelNormalizer = ln; this.labelNormalizer = ln;
this.userProvider = user; this.userProvider = user;
@@ -163,6 +166,7 @@ public class ChangeJson {
this.revisions = revisions; this.revisions = revisions;
this.webLinks = webLinks; this.webLinks = webLinks;
this.cmUtil = cmUtil; this.cmUtil = cmUtil;
this.plcUtil = plcUtil;
options = EnumSet.noneOf(ListChangesOption.class); options = EnumSet.noneOf(ListChangesOption.class);
} }
@@ -821,9 +825,8 @@ public class ChangeJson {
&& userProvider.get().isIdentifiedUser()) { && userProvider.get().isIdentifiedUser()) {
IdentifiedUser user = (IdentifiedUser)userProvider.get(); IdentifiedUser user = (IdentifiedUser)userProvider.get();
out.hasDraftComments = out.hasDraftComments =
db.get().patchComments() plcUtil.draftByPatchSetAuthor(db.get(), in.getId(),
.draftByPatchSetAuthor(in.getId(), user.getAccountId()) user.getAccountId(), ctl.getNotes()).iterator().hasNext()
.iterator().hasNext()
? true ? true
: null; : null;
} }

View File

@@ -14,6 +14,8 @@
package com.google.gerrit.server.change; package com.google.gerrit.server.change;
import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.gerrit.common.changes.Side; import com.google.gerrit.common.changes.Side;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -24,27 +26,41 @@ import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.change.PutDraft.Input; import com.google.gerrit.server.change.PutDraft.Input;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.util.TimeUtil; import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.Collections; import java.util.Collections;
@Singleton @Singleton
class CreateDraft implements RestModifyView<RevisionResource, Input> { class CreateDraft implements RestModifyView<RevisionResource, Input> {
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final ChangeUpdate.Factory updateFactory;
private final PatchLineCommentsUtil plcUtil;
private final PatchListCache patchListCache;
@Inject @Inject
CreateDraft(Provider<ReviewDb> db) { CreateDraft(Provider<ReviewDb> db,
ChangeUpdate.Factory updateFactory,
PatchLineCommentsUtil plcUtil,
PatchListCache patchListCache) {
this.db = db; this.db = db;
this.updateFactory = updateFactory;
this.plcUtil = plcUtil;
this.patchListCache = patchListCache;
} }
@Override @Override
public Response<CommentInfo> apply(RevisionResource rsrc, Input in) public Response<CommentInfo> apply(RevisionResource rsrc, Input in)
throws BadRequestException, OrmException { throws BadRequestException, OrmException, IOException {
if (Strings.isNullOrEmpty(in.path)) { if (Strings.isNullOrEmpty(in.path)) {
throw new BadRequestException("path must be non-empty"); throw new BadRequestException("path must be non-empty");
} else if (in.message == null || in.message.trim().isEmpty()) { } else if (in.message == null || in.message.trim().isEmpty()) {
@@ -59,15 +75,20 @@ class CreateDraft implements RestModifyView<RevisionResource, Input> {
? in.line ? in.line
: in.range != null ? in.range.getEndLine() : 0; : in.range != null ? in.range.getEndLine() : 0;
Timestamp now = TimeUtil.nowTs();
ChangeUpdate update = updateFactory.create(rsrc.getControl(), now);
PatchLineComment c = new PatchLineComment( PatchLineComment c = new PatchLineComment(
new PatchLineComment.Key( new PatchLineComment.Key(
new Patch.Key(rsrc.getPatchSet().getId(), in.path), new Patch.Key(rsrc.getPatchSet().getId(), in.path),
ChangeUtil.messageUUID(db.get())), ChangeUtil.messageUUID(db.get())),
line, rsrc.getAccountId(), Url.decode(in.inReplyTo), TimeUtil.nowTs()); line, rsrc.getAccountId(), Url.decode(in.inReplyTo), now);
c.setSide(in.side == Side.PARENT ? (short) 0 : (short) 1); c.setSide(in.side == Side.PARENT ? (short) 0 : (short) 1);
c.setMessage(in.message.trim()); c.setMessage(in.message.trim());
c.setRange(in.range); c.setRange(in.range);
db.get().patchComments().insert(Collections.singleton(c)); setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
plcUtil.insertComments(db.get(), update, Collections.singleton(c));
update.commit();
return Response.created(new CommentInfo(c, null)); return Response.created(new CommentInfo(c, null));
} }
} }

View File

@@ -14,15 +14,22 @@
package com.google.gerrit.server.change; package com.google.gerrit.server.change;
import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.change.DeleteDraft.Input; import com.google.gerrit.server.change.DeleteDraft.Input;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collections; import java.util.Collections;
@Singleton @Singleton
@@ -31,16 +38,30 @@ class DeleteDraft implements RestModifyView<DraftResource, Input> {
} }
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final PatchLineCommentsUtil plcUtil;
private final ChangeUpdate.Factory updateFactory;
private final PatchListCache patchListCache;
@Inject @Inject
DeleteDraft(Provider<ReviewDb> db) { DeleteDraft(Provider<ReviewDb> db,
PatchLineCommentsUtil plcUtil,
ChangeUpdate.Factory updateFactory,
PatchListCache patchListCache) {
this.db = db; this.db = db;
this.plcUtil = plcUtil;
this.updateFactory = updateFactory;
this.patchListCache = patchListCache;
} }
@Override @Override
public Response<CommentInfo> apply(DraftResource rsrc, Input input) public Response<CommentInfo> apply(DraftResource rsrc, Input input)
throws OrmException { throws OrmException, IOException {
db.get().patchComments().delete(Collections.singleton(rsrc.getComment())); ChangeUpdate update = updateFactory.create(rsrc.getControl());
PatchLineComment c = rsrc.getComment();
setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
plcUtil.deleteComments(db.get(), update, Collections.singleton(c));
update.commit();
return Response.none(); return Response.none();
} }
} }

View File

@@ -23,6 +23,7 @@ import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -34,16 +35,19 @@ class Drafts implements ChildCollection<RevisionResource, DraftResource> {
private final Provider<CurrentUser> user; private final Provider<CurrentUser> user;
private final ListDrafts list; private final ListDrafts list;
private final Provider<ReviewDb> dbProvider; private final Provider<ReviewDb> dbProvider;
private final PatchLineCommentsUtil plcUtil;
@Inject @Inject
Drafts(DynamicMap<RestView<DraftResource>> views, Drafts(DynamicMap<RestView<DraftResource>> views,
Provider<CurrentUser> user, Provider<CurrentUser> user,
ListDrafts list, ListDrafts list,
Provider<ReviewDb> dbProvider) { Provider<ReviewDb> dbProvider,
PatchLineCommentsUtil plcUtil) {
this.views = views; this.views = views;
this.user = user; this.user = user;
this.list = list; this.list = list;
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.plcUtil = plcUtil;
} }
@Override @Override
@@ -62,10 +66,8 @@ class Drafts implements ChildCollection<RevisionResource, DraftResource> {
throws ResourceNotFoundException, OrmException, AuthException { throws ResourceNotFoundException, OrmException, AuthException {
checkIdentifiedUser(); checkIdentifiedUser();
String uuid = id.get(); String uuid = id.get();
for (PatchLineComment c : dbProvider.get().patchComments() for (PatchLineComment c : plcUtil.draftByPatchSetAuthor(dbProvider.get(),
.draftByPatchSetAuthor( rev.getPatchSet().getId(), rev.getAccountId(), rev.getNotes())) {
rev.getPatchSet().getId(),
rev.getAccountId())) {
if (uuid.equals(c.getKey().get())) { if (uuid.equals(c.getKey().get())) {
return new DraftResource(rev, c); return new DraftResource(rev, c);
} }

View File

@@ -26,13 +26,10 @@ import com.google.inject.Singleton;
@Singleton @Singleton
class ListComments extends ListDrafts { class ListComments extends ListDrafts {
private final PatchLineCommentsUtil plcUtil;
@Inject @Inject
ListComments(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf, ListComments(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf,
PatchLineCommentsUtil plcUtil) { PatchLineCommentsUtil plcUtil) {
super(db, alf); super(db, alf, plcUtil);
this.plcUtil = plcUtil;
} }
@Override @Override

View File

@@ -22,6 +22,7 @@ import com.google.gerrit.common.changes.Side;
import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.account.AccountInfo; import com.google.gerrit.server.account.AccountInfo;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -36,20 +37,21 @@ import java.util.Map;
@Singleton @Singleton
class ListDrafts implements RestReadView<RevisionResource> { class ListDrafts implements RestReadView<RevisionResource> {
protected final Provider<ReviewDb> db; protected final Provider<ReviewDb> db;
protected final PatchLineCommentsUtil plcUtil;
private final AccountInfo.Loader.Factory accountLoaderFactory; private final AccountInfo.Loader.Factory accountLoaderFactory;
@Inject @Inject
ListDrafts(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf) { ListDrafts(Provider<ReviewDb> db, AccountInfo.Loader.Factory alf,
PatchLineCommentsUtil plcUtil) {
this.db = db; this.db = db;
this.accountLoaderFactory = alf; this.accountLoaderFactory = alf;
this.plcUtil = plcUtil;
} }
protected Iterable<PatchLineComment> listComments(RevisionResource rsrc) protected Iterable<PatchLineComment> listComments(RevisionResource rsrc)
throws OrmException { throws OrmException {
return db.get().patchComments() return plcUtil.draftByPatchSetAuthor(db.get(), rsrc.getPatchSet().getId(),
.draftByPatchSetAuthor( rsrc.getAccountId(), rsrc.getNotes());
rsrc.getPatchSet().getId(),
rsrc.getAccountId());
} }
protected boolean includeAuthorInfo() { protected boolean includeAuthorInfo() {

View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.change; package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
import com.google.common.base.Objects; import com.google.common.base.Objects;
import com.google.common.base.Strings; import com.google.common.base.Strings;
@@ -44,7 +45,6 @@ import com.google.gerrit.reviewdb.client.CommentRange;
import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchLineComment;
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.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.ChangeMessagesUtil;
@@ -54,9 +54,7 @@ import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.account.AccountsCollection; import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.index.ChangeIndexer; import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache; import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.util.LabelVote; import com.google.gerrit.server.util.LabelVote;
@@ -65,7 +63,6 @@ import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import org.eclipse.jgit.lib.ObjectId;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
@@ -346,15 +343,6 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
List<PatchLineComment> del = Lists.newArrayList(); List<PatchLineComment> del = Lists.newArrayList();
List<PatchLineComment> ups = Lists.newArrayList(); List<PatchLineComment> ups = Lists.newArrayList();
PatchList patchList = null;
try {
patchList = patchListCache.get(rsrc.getChange(), rsrc.getPatchSet());
} catch (PatchListNotAvailableException e) {
throw new OrmException("could not load PatchList for this patchset", e);
}
RevId patchSetCommit = new RevId(ObjectId.toString(patchList.getNewId()));
RevId baseCommit = new RevId(ObjectId.toString(patchList.getOldId()));
for (Map.Entry<String, List<CommentInput>> ent : in.entrySet()) { for (Map.Entry<String, List<CommentInput>> ent : in.entrySet()) {
String path = ent.getKey(); String path = ent.getKey();
for (CommentInput c : ent.getValue()) { for (CommentInput c : ent.getValue()) {
@@ -374,7 +362,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
e.setStatus(PatchLineComment.Status.PUBLISHED); e.setStatus(PatchLineComment.Status.PUBLISHED);
e.setWrittenOn(timestamp); e.setWrittenOn(timestamp);
e.setSide(c.side == Side.PARENT ? (short) 0 : (short) 1); e.setSide(c.side == Side.PARENT ? (short) 0 : (short) 1);
e.setRevId(c.side == Side.PARENT ? baseCommit : patchSetCommit); setCommentRevId(e, patchListCache, rsrc.getChange(),
rsrc.getPatchSet());
e.setMessage(c.message); e.setMessage(c.message);
if (c.range != null) { if (c.range != null) {
e.setRange(new CommentRange( e.setRange(new CommentRange(
@@ -398,13 +387,14 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
for (PatchLineComment e : drafts.values()) { for (PatchLineComment e : drafts.values()) {
e.setStatus(PatchLineComment.Status.PUBLISHED); e.setStatus(PatchLineComment.Status.PUBLISHED);
e.setWrittenOn(timestamp); e.setWrittenOn(timestamp);
e.setRevId(e.getSide() == (short) 0 ? baseCommit : patchSetCommit); setCommentRevId(e, patchListCache, rsrc.getChange(),
rsrc.getPatchSet());
ups.add(e); ups.add(e);
} }
break; break;
} }
db.get().patchComments().delete(del); plcUtil.deleteComments(db.get(), update, del);
plcUtil.addPublishedComments(db.get(), update, ups); plcUtil.upsertComments(db.get(), update, ups);
comments.addAll(ups); comments.addAll(ups);
return !del.isEmpty() || !ups.isEmpty(); return !del.isEmpty() || !ups.isEmpty();
} }
@@ -412,9 +402,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
private Map<String, PatchLineComment> scanDraftComments( private Map<String, PatchLineComment> scanDraftComments(
RevisionResource rsrc) throws OrmException { RevisionResource rsrc) throws OrmException {
Map<String, PatchLineComment> drafts = Maps.newHashMap(); Map<String, PatchLineComment> drafts = Maps.newHashMap();
for (PatchLineComment c : db.get().patchComments().draftByPatchSetAuthor( for (PatchLineComment c : plcUtil.draftByPatchSetAuthor(db.get(),
rsrc.getPatchSet().getId(), rsrc.getPatchSet().getId(), rsrc.getAccountId(), rsrc.getNotes())) {
rsrc.getAccountId())) {
drafts.put(c.getKey().get(), c); drafts.put(c.getKey().get(), c);
} }
return drafts; return drafts;

View File

@@ -14,6 +14,8 @@
package com.google.gerrit.server.change; package com.google.gerrit.server.change;
import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
import com.google.gerrit.common.changes.Side; import com.google.gerrit.common.changes.Side;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.DefaultInput; import com.google.gerrit.extensions.restapi.DefaultInput;
@@ -24,13 +26,17 @@ import com.google.gerrit.reviewdb.client.CommentRange;
import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.change.PutDraft.Input; import com.google.gerrit.server.change.PutDraft.Input;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.util.TimeUtil; import com.google.gerrit.server.util.TimeUtil;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp; import java.sql.Timestamp;
import java.util.Collections; import java.util.Collections;
@@ -51,17 +57,28 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final DeleteDraft delete; private final DeleteDraft delete;
private final PatchLineCommentsUtil plcUtil;
private final ChangeUpdate.Factory updateFactory;
private final PatchListCache patchListCache;
@Inject @Inject
PutDraft(Provider<ReviewDb> db, DeleteDraft delete) { PutDraft(Provider<ReviewDb> db,
DeleteDraft delete,
PatchLineCommentsUtil plcUtil,
ChangeUpdate.Factory updateFactory,
PatchListCache patchListCache) {
this.db = db; this.db = db;
this.delete = delete; this.delete = delete;
this.plcUtil = plcUtil;
this.updateFactory = updateFactory;
this.patchListCache = patchListCache;
} }
@Override @Override
public Response<CommentInfo> apply(DraftResource rsrc, Input in) throws public Response<CommentInfo> apply(DraftResource rsrc, Input in) throws
BadRequestException, OrmException { BadRequestException, OrmException, IOException {
PatchLineComment c = rsrc.getComment(); PatchLineComment c = rsrc.getComment();
ChangeUpdate update = updateFactory.create(rsrc.getControl());
if (in == null || in.message == null || in.message.trim().isEmpty()) { if (in == null || in.message == null || in.message.trim().isEmpty()) {
return delete.apply(rsrc, null); return delete.apply(rsrc, null);
} else if (in.id != null && !rsrc.getId().equals(in.id)) { } else if (in.id != null && !rsrc.getId().equals(in.id)) {
@@ -76,7 +93,8 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
&& !in.path.equals(c.getKey().getParentKey().getFileName())) { && !in.path.equals(c.getKey().getParentKey().getFileName())) {
// Updating the path alters the primary key, which isn't possible. // Updating the path alters the primary key, which isn't possible.
// Delete then recreate the comment instead of an update. // Delete then recreate the comment instead of an update.
db.get().patchComments().delete(Collections.singleton(c));
plcUtil.deleteComments(db.get(), update, Collections.singleton(c));
c = new PatchLineComment( c = new PatchLineComment(
new PatchLineComment.Key( new PatchLineComment.Key(
new Patch.Key(rsrc.getPatchSet().getId(), in.path), new Patch.Key(rsrc.getPatchSet().getId(), in.path),
@@ -84,10 +102,18 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
c.getLine(), c.getLine(),
rsrc.getAuthorId(), rsrc.getAuthorId(),
c.getParentUuid(), TimeUtil.nowTs()); c.getParentUuid(), TimeUtil.nowTs());
db.get().patchComments().insert(Collections.singleton(update(c, in))); setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
plcUtil.insertComments(db.get(), update,
Collections.singleton(update(c, in)));
} else { } else {
db.get().patchComments().update(Collections.singleton(update(c, in))); if (c.getRevId() == null) {
setCommentRevId(c, patchListCache, rsrc.getChange(),
rsrc.getPatchSet());
}
plcUtil.updateComments(db.get(), update,
Collections.singleton(update(c, in)));
} }
update.commit();
return Response.ok(new CommentInfo(c, null)); return Response.ok(new CommentInfo(c, null));
} }

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.mail; package com.google.gerrit.server.mail;
import com.google.common.base.Optional;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.common.collect.Ordering; import com.google.common.collect.Ordering;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
@@ -24,6 +25,7 @@ import com.google.gerrit.reviewdb.client.CommentRange;
import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.patch.PatchFile; import com.google.gerrit.server.patch.PatchFile;
import com.google.gerrit.server.patch.PatchList; import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -53,13 +55,16 @@ public class CommentSender extends ReplyToChangeSender {
private final NotifyHandling notify; private final NotifyHandling notify;
private List<PatchLineComment> inlineComments = Collections.emptyList(); private List<PatchLineComment> inlineComments = Collections.emptyList();
private final PatchLineCommentsUtil plcUtil;
@Inject @Inject
public CommentSender(EmailArguments ea, public CommentSender(EmailArguments ea,
@Assisted NotifyHandling notify, @Assisted NotifyHandling notify,
@Assisted Change c) { @Assisted Change c,
PatchLineCommentsUtil plcUtil) {
super(ea, c, "comment"); super(ea, c, "comment");
this.notify = notify; this.notify = notify;
this.plcUtil = plcUtil;
} }
public void setPatchLineComments(final List<PatchLineComment> plc) public void setPatchLineComments(final List<PatchLineComment> plc)
@@ -232,17 +237,19 @@ public class CommentSender extends ReplyToChangeSender {
private void appendQuotedParent(StringBuilder out, PatchLineComment child) { private void appendQuotedParent(StringBuilder out, PatchLineComment child) {
if (child.getParentUuid() != null) { if (child.getParentUuid() != null) {
PatchLineComment parent; Optional<PatchLineComment> parent;
PatchLineComment.Key key = new PatchLineComment.Key(
child.getKey().getParentKey(),
child.getParentUuid());
try { try {
parent = args.db.get().patchComments().get( parent = plcUtil.get(args.db.get(), changeData.notes(), key);
new PatchLineComment.Key(
child.getKey().getParentKey(),
child.getParentUuid()));
} catch (OrmException e) { } catch (OrmException e) {
parent = null; log.warn("Could not find the parent of this comment: "
+ child.toString());
parent = Optional.absent();
} }
if (parent != null) { if (parent.isPresent()) {
String msg = parent.getMessage().trim(); String msg = parent.get().getMessage().trim();
if (msg.length() > 75) { if (msg.length() > 75) {
msg = msg.substring(0, 75); msg = msg.substring(0, 75);
} }

View File

@@ -105,9 +105,11 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
verifyComment(c); verifyComment(c);
checkArgument(c.getStatus() == Status.DRAFT, checkArgument(c.getStatus() == Status.DRAFT,
"Cannot insert a published comment into a ChangeDraftUpdate"); "Cannot insert a published comment into a ChangeDraftUpdate");
checkArgument(!changeNotes.containsComment(c), if (migration.readComments()) {
"A comment already exists with the same key," checkArgument(!changeNotes.containsComment(c),
+ " so the following comment cannot be inserted: %s", c); "A comment already exists with the same key,"
+ " so the following comment cannot be inserted: %s", c);
}
upsertComments.add(c); upsertComments.add(c);
} }
@@ -122,16 +124,32 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
verifyComment(c); verifyComment(c);
checkArgument(c.getStatus() == Status.DRAFT, checkArgument(c.getStatus() == Status.DRAFT,
"Cannot update a published comment into a ChangeDraftUpdate"); "Cannot update a published comment into a ChangeDraftUpdate");
checkArgument(draftNotes.containsComment(c), // Here, we check to see if this comment existed previously as a draft.
"Cannot update this comment because it didn't exist previously"); // However, this could cause a race condition if there is a delete and an
// update operation happening concurrently (or two deletes) and they both
// believe that the comment exists. If a delete happens first, then
// the update will fail. However, this is an acceptable risk since the
// caller wanted the comment deleted anyways, so the end result will be the
// same either way.
if (migration.readComments()) {
checkArgument(draftNotes.containsComment(c),
"Cannot update this comment because it didn't exist previously");
}
upsertComments.add(c); upsertComments.add(c);
} }
public void deleteComment(PatchLineComment c) { public void deleteComment(PatchLineComment c) {
verifyComment(c); verifyComment(c);
checkArgument(draftNotes.containsComment(c), "Cannot delete this comment" // See the comment above about potential race condition.
+ " because it didn't previously exist as a draft"); if (migration.readComments()) {
deleteComments.add(c); checkArgument(draftNotes.containsComment(c), "Cannot delete this comment"
+ " because it didn't previously exist as a draft");
}
if (migration.write()) {
if (draftNotes.containsComment(c)) {
deleteComments.add(c);
}
}
} }
/** /**
@@ -151,7 +169,9 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
checkArgument(getCommentPsId(comment).equals(psId), checkArgument(getCommentPsId(comment).equals(psId),
"Comment on %s does not match configured patch set %s", "Comment on %s does not match configured patch set %s",
getCommentPsId(comment), psId); getCommentPsId(comment), psId);
checkArgument(comment.getRevId() != null); if (migration.write()) {
checkArgument(comment.getRevId() != null);
}
checkArgument(comment.getAuthor().equals(accountId), checkArgument(comment.getAuthor().equals(accountId),
"The author for the following comment does not match the author of" "The author for the following comment does not match the author of"
+ " this ChangeDraftUpdate (%s): %s", accountId, comment); + " this ChangeDraftUpdate (%s): %s", accountId, comment);
@@ -174,6 +194,8 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
Table<PatchSet.Id, String, PatchLineComment> psDrafts = Table<PatchSet.Id, String, PatchLineComment> psDrafts =
draftNotes.getDraftPsComments(); draftNotes.getDraftPsComments();
boolean draftsEmpty = baseDrafts.isEmpty() && psDrafts.isEmpty();
// There is no need to rewrite the note for one of the sides of the patch // There is no need to rewrite the note for one of the sides of the patch
// set if all of the modifications were made to the comments of one side, // set if all of the modifications were made to the comments of one side,
// so we set these flags to potentially save that extra work. // so we set these flags to potentially save that extra work.
@@ -219,7 +241,8 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
updateNoteMap(revisionSideChanged, noteMap, newPsDrafts, updateNoteMap(revisionSideChanged, noteMap, newPsDrafts,
psRevId); psRevId);
removedAllComments.set(baseDrafts.isEmpty() && psDrafts.isEmpty()); removedAllComments.set(
baseDrafts.isEmpty() && psDrafts.isEmpty() && !draftsEmpty);
return noteMap.writeTree(inserter); return noteMap.writeTree(inserter);
} }

View File

@@ -239,9 +239,11 @@ public class ChangeUpdate extends AbstractChangeUpdate {
if (notes == null) { if (notes == null) {
notes = getChangeNotes().load(); notes = getChangeNotes().load();
} }
checkArgument(!notes.containsComment(c), if (migration.readComments()) {
"A comment already exists with the same key as the following comment," checkArgument(!notes.containsComment(c),
+ " so we cannot insert this comment: %s", c); "A comment already exists with the same key as the following comment,"
+ " so we cannot insert this comment: %s", c);
}
if (c.getSide() == 0) { if (c.getSide() == 0) {
commentsForBase.add(c); commentsForBase.add(c);
} else { } else {
@@ -254,8 +256,19 @@ public class ChangeUpdate extends AbstractChangeUpdate {
draftUpdate.insertComment(c); draftUpdate.insertComment(c);
} }
private void upsertPublishedComment(PatchLineComment c) { private void upsertPublishedComment(PatchLineComment c) throws OrmException {
verifyComment(c); verifyComment(c);
if (notes == null) {
notes = getChangeNotes().load();
}
// This could allow callers to update a published comment if migration.write
// is on and migration.readComments is off because we will not be able to
// verify that the comment didn't already exist as a published comment
// since we don't have a ReviewDb.
if (migration.readComments()) {
checkArgument(!notes.containsCommentPublished(c),
"Cannot update a comment that has already been published and saved");
}
if (c.getSide() == 0) { if (c.getSide() == 0) {
commentsForBase.add(c); commentsForBase.add(c);
} else { } else {
@@ -273,8 +286,12 @@ public class ChangeUpdate extends AbstractChangeUpdate {
if (notes == null) { if (notes == null) {
notes = getChangeNotes().load(); notes = getChangeNotes().load();
} }
checkArgument(!notes.containsCommentPublished(c), // See comment above in upsertPublishedComment() about potential risk with
"Cannot update a comment that has already been published and saved"); // this check.
if (migration.readComments()) {
checkArgument(!notes.containsCommentPublished(c),
"Cannot update a comment that has already been published and saved");
}
if (c.getSide() == 0) { if (c.getSide() == 0) {
commentsForBase.add(c); commentsForBase.add(c);
} else { } else {
@@ -298,7 +315,6 @@ public class ChangeUpdate extends AbstractChangeUpdate {
draftUpdate.deleteCommentIfPresent(c); draftUpdate.deleteCommentIfPresent(c);
} }
private void createDraftUpdateIfNull(PatchLineComment c) throws OrmException { private void createDraftUpdateIfNull(PatchLineComment c) throws OrmException {
if (draftUpdate == null) { if (draftUpdate == null) {
draftUpdate = draftUpdateFactory.create(ctl, when); draftUpdate = draftUpdateFactory.create(ctl, when);

View File

@@ -36,14 +36,14 @@ public class NotesMigration {
cfg.setBoolean("notedb", null, "write", true); cfg.setBoolean("notedb", null, "write", true);
cfg.setBoolean("notedb", "patchSetApprovals", "read", true); cfg.setBoolean("notedb", "patchSetApprovals", "read", true);
cfg.setBoolean("notedb", "changeMessages", "read", true); cfg.setBoolean("notedb", "changeMessages", "read", true);
cfg.setBoolean("notedb", "publishedComments", "read", true); cfg.setBoolean("notedb", "comments", "read", true);
return new NotesMigration(cfg); return new NotesMigration(cfg);
} }
private final boolean write; private final boolean write;
private final boolean readPatchSetApprovals; private final boolean readPatchSetApprovals;
private final boolean readChangeMessages; private final boolean readChangeMessages;
private final boolean readPublishedComments; private final boolean readComments;
@Inject @Inject
NotesMigration(@GerritServerConfig Config cfg) { NotesMigration(@GerritServerConfig Config cfg) {
@@ -52,8 +52,8 @@ public class NotesMigration {
cfg.getBoolean("notedb", "patchSetApprovals", "read", false); cfg.getBoolean("notedb", "patchSetApprovals", "read", false);
readChangeMessages = readChangeMessages =
cfg.getBoolean("notedb", "changeMessages", "read", false); cfg.getBoolean("notedb", "changeMessages", "read", false);
readPublishedComments = readComments =
cfg.getBoolean("notedb", "publishedComments", "read", false); cfg.getBoolean("notedb", "comments", "read", false);
} }
public boolean write() { public boolean write() {
@@ -68,7 +68,7 @@ public class NotesMigration {
return readChangeMessages; return readChangeMessages;
} }
public boolean readPublishedComments() { public boolean readComments() {
return readPublishedComments; return readComments;
} }
} }

View File

@@ -338,7 +338,8 @@ public class PatchScriptFactory implements Callable<PatchScript> {
private void loadDrafts(final Map<Patch.Key, Patch> byKey, private void loadDrafts(final Map<Patch.Key, Patch> byKey,
final AccountInfoCacheFactory aic, final Account.Id me, final String file) final AccountInfoCacheFactory aic, final Account.Id me, final String file)
throws OrmException { throws OrmException {
for (PatchLineComment c : db.patchComments().draftByChangeFileAuthor(changeId, file, me)) { for (PatchLineComment c :
plcUtil.draftByChangeFileAuthor(db, control.getNotes(), file, me)) {
if (comments.include(c)) { if (comments.include(c)) {
aic.want(me); aic.want(me);
} }

View File

@@ -30,7 +30,8 @@ public class BasicChangeRewrites extends QueryRewriter<ChangeData> {
new InvalidProvider<ReviewDb>(), // new InvalidProvider<ReviewDb>(), //
new InvalidProvider<ChangeQueryRewriter>(), // new InvalidProvider<ChangeQueryRewriter>(), //
null, null, null, null, null, null, null, // null, null, null, null, null, null, null, //
null, null, null, null, null, null, null, null, null, null), null); null, null, null, null, null, null, null, null, null, null, null),
null);
private static final QueryRewriter.Definition<ChangeData, BasicChangeRewrites> mydef = private static final QueryRewriter.Definition<ChangeData, BasicChangeRewrites> mydef =
new QueryRewriter.Definition<ChangeData, BasicChangeRewrites>( new QueryRewriter.Definition<ChangeData, BasicChangeRewrites>(

View File

@@ -35,6 +35,7 @@ import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.ChangeMessagesUtil;
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.PatchLineCommentsUtil;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.notedb.NotesMigration;
@@ -154,7 +155,7 @@ public class ChangeData {
*/ */
static ChangeData createForTest(Change.Id id, int currentPatchSetId) { static ChangeData createForTest(Change.Id id, int currentPatchSetId) {
ChangeData cd = new ChangeData(null, null, null, null, null, ChangeData cd = new ChangeData(null, null, null, null, null,
null, null, null, null, id); null, null, null, null, null, id);
cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId)); cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId));
return cd; return cd;
} }
@@ -166,6 +167,7 @@ public class ChangeData {
private final ChangeNotes.Factory notesFactory; private final ChangeNotes.Factory notesFactory;
private final ApprovalsUtil approvalsUtil; private final ApprovalsUtil approvalsUtil;
private final ChangeMessagesUtil cmUtil; private final ChangeMessagesUtil cmUtil;
private final PatchLineCommentsUtil plcUtil;
private final PatchListCache patchListCache; private final PatchListCache patchListCache;
private final NotesMigration notesMigration; private final NotesMigration notesMigration;
private final Change.Id legacyId; private final Change.Id legacyId;
@@ -194,6 +196,7 @@ public class ChangeData {
ChangeNotes.Factory notesFactory, ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil, ChangeMessagesUtil cmUtil,
PatchLineCommentsUtil plcUtil,
PatchListCache patchListCache, PatchListCache patchListCache,
NotesMigration notesMigration, NotesMigration notesMigration,
@Assisted ReviewDb db, @Assisted ReviewDb db,
@@ -205,6 +208,7 @@ public class ChangeData {
this.notesFactory = notesFactory; this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
this.cmUtil = cmUtil; this.cmUtil = cmUtil;
this.plcUtil = plcUtil;
this.patchListCache = patchListCache; this.patchListCache = patchListCache;
this.notesMigration = notesMigration; this.notesMigration = notesMigration;
legacyId = id; legacyId = id;
@@ -218,6 +222,7 @@ public class ChangeData {
ChangeNotes.Factory notesFactory, ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil, ChangeMessagesUtil cmUtil,
PatchLineCommentsUtil plcUtil,
PatchListCache patchListCache, PatchListCache patchListCache,
NotesMigration notesMigration, NotesMigration notesMigration,
@Assisted ReviewDb db, @Assisted ReviewDb db,
@@ -229,6 +234,7 @@ public class ChangeData {
this.notesFactory = notesFactory; this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
this.cmUtil = cmUtil; this.cmUtil = cmUtil;
this.plcUtil = plcUtil;
this.patchListCache = patchListCache; this.patchListCache = patchListCache;
this.notesMigration = notesMigration; this.notesMigration = notesMigration;
legacyId = c.getId(); legacyId = c.getId();
@@ -243,6 +249,7 @@ public class ChangeData {
ChangeNotes.Factory notesFactory, ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil, ChangeMessagesUtil cmUtil,
PatchLineCommentsUtil plcUtil,
PatchListCache patchListCache, PatchListCache patchListCache,
NotesMigration notesMigration, NotesMigration notesMigration,
@Assisted ReviewDb db, @Assisted ReviewDb db,
@@ -254,6 +261,7 @@ public class ChangeData {
this.notesFactory = notesFactory; this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
this.cmUtil = cmUtil; this.cmUtil = cmUtil;
this.plcUtil = plcUtil;
this.patchListCache = patchListCache; this.patchListCache = patchListCache;
this.notesMigration = notesMigration; this.notesMigration = notesMigration;
legacyId = c.getChange().getId(); legacyId = c.getChange().getId();
@@ -522,7 +530,7 @@ public class ChangeData {
public Collection<PatchLineComment> comments() public Collection<PatchLineComment> comments()
throws OrmException { throws OrmException {
if (comments == null) { if (comments == null) {
comments = db.patchComments().byChange(legacyId).toList(); comments = plcUtil.byChange(db, notes());
} }
return comments; return comments;
} }

View File

@@ -25,6 +25,7 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
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.PatchLineCommentsUtil;
import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.CapabilityControl; import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupBackend;
@@ -146,6 +147,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
final CapabilityControl.Factory capabilityControlFactory; final CapabilityControl.Factory capabilityControlFactory;
final ChangeControl.GenericFactory changeControlGenericFactory; final ChangeControl.GenericFactory changeControlGenericFactory;
final ChangeData.Factory changeDataFactory; final ChangeData.Factory changeDataFactory;
final PatchLineCommentsUtil plcUtil;
final AccountResolver accountResolver; final AccountResolver accountResolver;
final GroupBackend groupBackend; final GroupBackend groupBackend;
final AllProjectsName allProjectsName; final AllProjectsName allProjectsName;
@@ -168,6 +170,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
CapabilityControl.Factory capabilityControlFactory, CapabilityControl.Factory capabilityControlFactory,
ChangeControl.GenericFactory changeControlGenericFactory, ChangeControl.GenericFactory changeControlGenericFactory,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
PatchLineCommentsUtil plcUtil,
AccountResolver accountResolver, AccountResolver accountResolver,
GroupBackend groupBackend, GroupBackend groupBackend,
AllProjectsName allProjectsName, AllProjectsName allProjectsName,
@@ -187,6 +190,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
this.capabilityControlFactory = capabilityControlFactory; this.capabilityControlFactory = capabilityControlFactory;
this.changeControlGenericFactory = changeControlGenericFactory; this.changeControlGenericFactory = changeControlGenericFactory;
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.plcUtil = plcUtil;
this.accountResolver = accountResolver; this.accountResolver = accountResolver;
this.groupBackend = groupBackend; this.groupBackend = groupBackend;
this.allProjectsName = allProjectsName; this.allProjectsName = allProjectsName;

View File

@@ -33,7 +33,8 @@ class HasDraftByPredicate extends OperatorPredicate<ChangeData> implements
private final Arguments args; private final Arguments args;
private final Account.Id accountId; private final Account.Id accountId;
HasDraftByPredicate(Arguments args, Account.Id accountId) { HasDraftByPredicate(Arguments args,
Account.Id accountId) {
super(ChangeQueryBuilder.FIELD_DRAFTBY, accountId.toString()); super(ChangeQueryBuilder.FIELD_DRAFTBY, accountId.toString());
this.args = args; this.args = args;
this.accountId = accountId; this.accountId = accountId;
@@ -41,20 +42,16 @@ class HasDraftByPredicate extends OperatorPredicate<ChangeData> implements
@Override @Override
public boolean match(final ChangeData object) throws OrmException { public boolean match(final ChangeData object) throws OrmException {
for (PatchLineComment c : object.comments()) { return !args.plcUtil
if (c.getStatus() == PatchLineComment.Status.DRAFT .draftByChangeAuthor(args.db.get(), object.notes(), accountId)
&& c.getAuthor().equals(accountId)) { .isEmpty();
return true;
}
}
return false;
} }
@Override @Override
public ResultSet<ChangeData> read() throws OrmException { public ResultSet<ChangeData> read() throws OrmException {
Set<Change.Id> ids = new HashSet<>(); Set<Change.Id> ids = new HashSet<>();
for (PatchLineComment sc : args.db.get().patchComments() for (PatchLineComment sc :
.draftByAuthor(accountId)) { args.plcUtil.draftByAuthor(args.db.get(), accountId)) {
ids.add(sc.getKey().getParentKey().getParentKey().getParentKey()); ids.add(sc.getKey().getParentKey().getParentKey().getParentKey());
} }

View File

@@ -43,6 +43,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.PatchLineCommentAccess; import com.google.gerrit.reviewdb.server.PatchLineCommentAccess;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchLineCommentsUtil; import com.google.gerrit.server.PatchLineCommentsUtil;
@@ -60,7 +61,6 @@ import com.google.gerrit.server.git.GitModule;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.util.TimeUtil; import com.google.gerrit.server.util.TimeUtil;
@@ -74,6 +74,8 @@ import com.google.gwtorm.server.ResultSet;
import com.google.inject.AbstractModule; import com.google.inject.AbstractModule;
import com.google.inject.Guice; import com.google.inject.Guice;
import com.google.inject.Injector; import com.google.inject.Injector;
import com.google.inject.Provides;
import com.google.inject.Singleton;
import com.google.inject.TypeLiteral; import com.google.inject.TypeLiteral;
import com.google.inject.util.Providers; import com.google.inject.util.Providers;
@@ -104,7 +106,7 @@ public class CommentsTest {
public static @GerritServerConfig Config noteDbEnabled() { public static @GerritServerConfig Config noteDbEnabled() {
@GerritServerConfig Config cfg = new Config(); @GerritServerConfig Config cfg = new Config();
cfg.setBoolean("notedb", null, "write", true); cfg.setBoolean("notedb", null, "write", true);
cfg.setBoolean("notedb", "publishedComments", "read", true); cfg.setBoolean("notedb", "comments", "read", true);
return cfg; return cfg;
} }
@@ -118,15 +120,23 @@ public class CommentsTest {
private PatchLineComment plc1; private PatchLineComment plc1;
private PatchLineComment plc2; private PatchLineComment plc2;
private PatchLineComment plc3; private PatchLineComment plc3;
private PatchLineComment plc4;
private PatchLineComment plc5;
private IdentifiedUser changeOwner; private IdentifiedUser changeOwner;
@Before @Before
public void setUp() throws Exception { public void setUp() throws Exception {
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
final DynamicMap<RestView<CommentResource>> views = final DynamicMap<RestView<CommentResource>> commentViews =
createMock(DynamicMap.class); createMock(DynamicMap.class);
final TypeLiteral<DynamicMap<RestView<CommentResource>>> viewsType = final TypeLiteral<DynamicMap<RestView<CommentResource>>> commentViewsType =
new TypeLiteral<DynamicMap<RestView<CommentResource>>>() {}; new TypeLiteral<DynamicMap<RestView<CommentResource>>>() {};
@SuppressWarnings("unchecked")
final DynamicMap<RestView<DraftResource>> draftViews =
createMock(DynamicMap.class);
final TypeLiteral<DynamicMap<RestView<DraftResource>>> draftViewsType =
new TypeLiteral<DynamicMap<RestView<DraftResource>>>() {};
final AccountInfo.Loader.Factory alf = final AccountInfo.Loader.Factory alf =
createMock(AccountInfo.Loader.Factory.class); createMock(AccountInfo.Loader.Factory.class);
final ReviewDb db = createMock(ReviewDb.class); final ReviewDb db = createMock(ReviewDb.class);
@@ -139,10 +149,23 @@ public class CommentsTest {
@SuppressWarnings("unused") @SuppressWarnings("unused")
InMemoryRepository repo = repoManager.createRepository(project); InMemoryRepository repo = repoManager.createRepository(project);
Account co = new Account(new Account.Id(1), TimeUtil.nowTs());
co.setFullName("Change Owner");
co.setPreferredEmail("change@owner.com");
accountCache.put(co);
final Account.Id ownerId = co.getId();
Account ou = new Account(new Account.Id(2), TimeUtil.nowTs());
ou.setFullName("Other Account");
ou.setPreferredEmail("other@account.com");
accountCache.put(ou);
Account.Id otherUserId = ou.getId();
AbstractModule mod = new AbstractModule() { AbstractModule mod = new AbstractModule() {
@Override @Override
protected void configure() { protected void configure() {
bind(viewsType).toInstance(views); bind(commentViewsType).toInstance(commentViews);
bind(draftViewsType).toInstance(draftViews);
bind(AccountInfo.Loader.Factory.class).toInstance(alf); bind(AccountInfo.Loader.Factory.class).toInstance(alf);
bind(ReviewDb.class).toProvider(Providers.<ReviewDb>of(db)); bind(ReviewDb.class).toProvider(Providers.<ReviewDb>of(db));
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(config); bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(config);
@@ -162,28 +185,16 @@ public class CommentsTest {
bind(PersonIdent.class).annotatedWith(GerritPersonIdent.class) bind(PersonIdent.class).annotatedWith(GerritPersonIdent.class)
.toInstance(serverIdent); .toInstance(serverIdent);
} }
@Provides
@Singleton
CurrentUser getCurrentUser(IdentifiedUser.GenericFactory userFactory) {
return userFactory.create(ownerId);
}
}; };
injector = Guice.createInjector(mod); injector = Guice.createInjector(mod);
NotesMigration migration = injector.getInstance(NotesMigration.class);
allUsers = injector.getInstance(AllUsersNameProvider.class);
repoManager.createRepository(allUsers.get());
plcUtil = new PatchLineCommentsUtil(migration);
Account co = new Account(new Account.Id(1), TimeUtil.nowTs());
co.setFullName("Change Owner");
co.setPreferredEmail("change@owner.com");
accountCache.put(co);
Account.Id ownerId = co.getId();
Account ou = new Account(new Account.Id(2), TimeUtil.nowTs());
ou.setFullName("Other Account");
ou.setPreferredEmail("other@account.com");
accountCache.put(ou);
Account.Id otherUserId = ou.getId();
IdentifiedUser.GenericFactory userFactory = IdentifiedUser.GenericFactory userFactory =
injector.getInstance(IdentifiedUser.GenericFactory.class); injector.getInstance(IdentifiedUser.GenericFactory.class);
changeOwner = userFactory.create(ownerId); changeOwner = userFactory.create(ownerId);
@@ -199,6 +210,10 @@ public class CommentsTest {
expect(alf.create(true)).andReturn(accountLoader).anyTimes(); expect(alf.create(true)).andReturn(accountLoader).anyTimes();
replay(accountLoader, alf); replay(accountLoader, alf);
allUsers = injector.getInstance(AllUsersNameProvider.class);
repoManager.createRepository(allUsers.get());
plcUtil = injector.getInstance(PatchLineCommentsUtil.class);
PatchLineCommentAccess plca = createMock(PatchLineCommentAccess.class); PatchLineCommentAccess plca = createMock(PatchLineCommentAccess.class);
expect(db.patchComments()).andReturn(plca).anyTimes(); expect(db.patchComments()).andReturn(plca).anyTimes();
@@ -221,32 +236,54 @@ public class CommentsTest {
"FileOne.txt", Side.PARENT, 3, ownerId, timeBase + 2000, "FileOne.txt", Side.PARENT, 3, ownerId, timeBase + 2000,
"First Parent Comment", new CommentRange(1, 2, 3, 4)); "First Parent Comment", new CommentRange(1, 2, 3, 4));
plc3.setRevId(new RevId("CDEFCDEFCDEFCDEFCDEFCDEFCDEFCDEFCDEFCDEF")); plc3.setRevId(new RevId("CDEFCDEFCDEFCDEFCDEFCDEFCDEFCDEFCDEFCDEF"));
plc4 = newPatchLineComment(psId2, "Comment4", null, "FileOne.txt",
Side.REVISION, 3, ownerId, timeBase + 3000, "Second Comment",
new CommentRange(1, 2, 3, 4), Status.DRAFT);
plc4.setRevId(new RevId("BCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDE"));
plc5 = newPatchLineComment(psId2, "Comment5", null, "FileOne.txt",
Side.REVISION, 5, ownerId, timeBase + 4000, "Third Comment",
new CommentRange(3, 4, 5, 6), Status.DRAFT);
plc5.setRevId(new RevId("BCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDEBCDE"));
List<PatchLineComment> commentsByOwner = Lists.newArrayList(); List<PatchLineComment> commentsByOwner = Lists.newArrayList();
commentsByOwner.add(plc1); commentsByOwner.add(plc1);
commentsByOwner.add(plc3); commentsByOwner.add(plc3);
List<PatchLineComment> commentsByReviewer = Lists.newArrayList(); List<PatchLineComment> commentsByReviewer = Lists.newArrayList();
commentsByReviewer.add(plc2); commentsByReviewer.add(plc2);
List<PatchLineComment> drafts = Lists.newArrayList();
drafts.add(plc4);
drafts.add(plc5);
plca.upsert(commentsByOwner); plca.upsert(commentsByOwner);
expectLastCall().anyTimes(); expectLastCall().anyTimes();
plca.upsert(commentsByReviewer); plca.upsert(commentsByReviewer);
expectLastCall().anyTimes(); expectLastCall().anyTimes();
plca.upsert(drafts);
expectLastCall().anyTimes();
expect(plca.publishedByPatchSet(psId1)) expect(plca.publishedByPatchSet(psId1))
.andAnswer(results(plc1, plc2, plc3)).anyTimes(); .andAnswer(results(plc1, plc2, plc3)).anyTimes();
expect(plca.publishedByPatchSet(psId2)) expect(plca.publishedByPatchSet(psId2))
.andAnswer(results()).anyTimes(); .andAnswer(results()).anyTimes();
expect(plca.draftByPatchSetAuthor(psId1, ownerId))
.andAnswer(results()).anyTimes();
expect(plca.draftByPatchSetAuthor(psId2, ownerId))
.andAnswer(results(plc4, plc5)).anyTimes();
replay(db, plca); replay(db, plca);
ChangeUpdate update = newUpdate(change, changeOwner); ChangeUpdate update = newUpdate(change, changeOwner);
update.setPatchSetId(psId1); update.setPatchSetId(psId1);
plcUtil.addPublishedComments(db, update, commentsByOwner); plcUtil.upsertComments(db, update, commentsByOwner);
update.commit(); update.commit();
update = newUpdate(change, otherUser); update = newUpdate(change, otherUser);
update.setPatchSetId(psId1); update.setPatchSetId(psId1);
plcUtil.addPublishedComments(db, update, commentsByReviewer); plcUtil.upsertComments(db, update, commentsByReviewer);
update.commit();
update = newUpdate(change, changeOwner);
update.setPatchSetId(psId2);
plcUtil.upsertComments(db, update, drafts);
update.commit(); update.commit();
ChangeControl ctl = stubChangeControl(change); ChangeControl ctl = stubChangeControl(change);
@@ -286,6 +323,17 @@ public class CommentsTest {
assertGetComment(injector, revRes1, null, "BadComment"); assertGetComment(injector, revRes1, null, "BadComment");
} }
@Test
public void testListDrafts() throws Exception {
// test ListDrafts for patch set 1
assertListDrafts(injector, revRes1,
Collections.<String, ArrayList<PatchLineComment>> emptyMap());
// test ListDrafts for patch set 2
assertListDrafts(injector, revRes2, ImmutableMap.of(
"FileOne.txt", Lists.newArrayList(plc4, plc5)));
}
private static IAnswer<ResultSet<PatchLineComment>> results( private static IAnswer<ResultSet<PatchLineComment>> results(
final PatchLineComment... comments) { final PatchLineComment... comments) {
return new IAnswer<ResultSet<PatchLineComment>>() { return new IAnswer<ResultSet<PatchLineComment>>() {
@@ -305,7 +353,7 @@ public class CommentsTest {
fail("Expected no comment"); fail("Expected no comment");
} }
CommentInfo actual = getComment.apply(commentRes); CommentInfo actual = getComment.apply(commentRes);
assertComment(expected, actual); assertComment(expected, actual, true);
} catch (ResourceNotFoundException e) { } catch (ResourceNotFoundException e) {
if (expected != null) { if (expected != null) {
fail("Expected to find comment"); fail("Expected to find comment");
@@ -330,17 +378,42 @@ public class CommentsTest {
assertNotNull(actualComments); assertNotNull(actualComments);
assertEquals(expectedComments.size(), actualComments.size()); assertEquals(expectedComments.size(), actualComments.size());
for (int i = 0; i < expectedComments.size(); i++) { for (int i = 0; i < expectedComments.size(); i++) {
assertComment(expectedComments.get(i), actualComments.get(i)); assertComment(expectedComments.get(i), actualComments.get(i), true);
} }
} }
} }
private static void assertComment(PatchLineComment plc, CommentInfo ci) { private static void assertListDrafts(Injector inj, RevisionResource res,
Map<String, ArrayList<PatchLineComment>> expected) throws Exception {
Drafts drafts = inj.getInstance(Drafts.class);
RestReadView<RevisionResource> listView =
(RestReadView<RevisionResource>) drafts.list();
@SuppressWarnings("unchecked")
Map<String, List<CommentInfo>> actual =
(Map<String, List<CommentInfo>>) listView.apply(res);
assertNotNull(actual);
assertEquals(expected.size(), actual.size());
assertEquals(expected.keySet(), actual.keySet());
for (Map.Entry<String, ArrayList<PatchLineComment>> entry : expected.entrySet()) {
List<PatchLineComment> expectedComments = entry.getValue();
List<CommentInfo> actualComments = actual.get(entry.getKey());
assertNotNull(actualComments);
assertEquals(expectedComments.size(), actualComments.size());
for (int i = 0; i < expectedComments.size(); i++) {
assertComment(expectedComments.get(i), actualComments.get(i), false);
}
}
}
private static void assertComment(PatchLineComment plc, CommentInfo ci,
boolean isPublished) {
assertEquals(plc.getKey().get(), ci.id); assertEquals(plc.getKey().get(), ci.id);
assertEquals(plc.getParentUuid(), ci.inReplyTo); assertEquals(plc.getParentUuid(), ci.inReplyTo);
assertEquals(plc.getMessage(), ci.message); assertEquals(plc.getMessage(), ci.message);
assertNotNull(ci.author); if (isPublished) {
assertEquals(plc.getAuthor(), ci.author._id); assertNotNull(ci.author);
assertEquals(plc.getAuthor(), ci.author._id);
}
assertEquals(plc.getLine(), (int) ci.line); assertEquals(plc.getLine(), (int) ci.line);
assertEquals(plc.getSide() == 0 ? Side.PARENT : Side.REVISION, assertEquals(plc.getSide() == 0 ? Side.PARENT : Side.REVISION,
Objects.firstNonNull(ci.side, Side.REVISION)); Objects.firstNonNull(ci.side, Side.REVISION));
@@ -351,7 +424,8 @@ public class CommentsTest {
private static PatchLineComment newPatchLineComment(PatchSet.Id psId, private static PatchLineComment newPatchLineComment(PatchSet.Id psId,
String uuid, String inReplyToUuid, String filename, Side side, int line, String uuid, String inReplyToUuid, String filename, Side side, int line,
Account.Id authorId, long millis, String message, CommentRange range) { Account.Id authorId, long millis, String message, CommentRange range,
PatchLineComment.Status status) {
Patch.Key p = new Patch.Key(psId, filename); Patch.Key p = new Patch.Key(psId, filename);
PatchLineComment.Key id = new PatchLineComment.Key(p, uuid); PatchLineComment.Key id = new PatchLineComment.Key(p, uuid);
PatchLineComment plc = PatchLineComment plc =
@@ -359,8 +433,15 @@ public class CommentsTest {
plc.setMessage(message); plc.setMessage(message);
plc.setRange(range); plc.setRange(range);
plc.setSide(side == Side.PARENT ? (short) 0 : (short) 1); plc.setSide(side == Side.PARENT ? (short) 0 : (short) 1);
plc.setStatus(Status.PUBLISHED); plc.setStatus(status);
plc.setWrittenOn(new Timestamp(millis)); plc.setWrittenOn(new Timestamp(millis));
return plc; return plc;
} }
private static PatchLineComment newPatchLineComment(PatchSet.Id psId,
String uuid, String inReplyToUuid, String filename, Side side, int line,
Account.Id authorId, long millis, String message, CommentRange range) {
return newPatchLineComment(psId, uuid, inReplyToUuid, filename, side, line,
authorId, millis, message, range, Status.PUBLISHED);
}
} }

View File

@@ -26,8 +26,8 @@ public class FakeQueryBuilder extends ChangeQueryBuilder {
new FakeQueryBuilder.Definition<>( new FakeQueryBuilder.Definition<>(
FakeQueryBuilder.class), FakeQueryBuilder.class),
new ChangeQueryBuilder.Arguments(null, null, null, null, null, null, new ChangeQueryBuilder.Arguments(null, null, null, null, null, null,
null, null, null, null, null, null, null, null, indexes, null, null, null, null, null, null, null, null, null, null, null, indexes, null,
null, null), null, null, null),
null); null);
} }

View File

@@ -895,7 +895,9 @@ public class ChangeNotesTest {
public void patchLineCommentNotesFormatSide1() throws Exception { public void patchLineCommentNotesFormatSide1() throws Exception {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser); ChangeUpdate update = newUpdate(c, otherUser);
String uuid = "uuid"; String uuid1 = "uuid1";
String uuid2 = "uuid2";
String uuid3 = "uuid3";
String message1 = "comment 1"; String message1 = "comment 1";
String message2 = "comment 2"; String message2 = "comment 2";
String message3 = "comment 3"; String message3 = "comment 3";
@@ -906,7 +908,7 @@ public class ChangeNotesTest {
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment comment1 = newPublishedPatchLineComment(psId, "file1", PatchLineComment comment1 = newPublishedPatchLineComment(psId, "file1",
uuid, range1, range1.getEndLine(), otherUser, null, time1, message1, uuid1, range1, range1.getEndLine(), otherUser, null, time1, message1,
(short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234"); (short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.upsertComment(comment1); update.upsertComment(comment1);
@@ -915,7 +917,7 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser); update = newUpdate(c, otherUser);
CommentRange range2 = new CommentRange(2, 1, 3, 1); CommentRange range2 = new CommentRange(2, 1, 3, 1);
PatchLineComment comment2 = newPublishedPatchLineComment(psId, "file1", PatchLineComment comment2 = newPublishedPatchLineComment(psId, "file1",
uuid, range2, range2.getEndLine(), otherUser, null, time2, message2, uuid2, range2, range2.getEndLine(), otherUser, null, time2, message2,
(short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234"); (short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.upsertComment(comment2); update.upsertComment(comment2);
@@ -924,7 +926,7 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser); update = newUpdate(c, otherUser);
CommentRange range3 = new CommentRange(3, 1, 4, 1); CommentRange range3 = new CommentRange(3, 1, 4, 1);
PatchLineComment comment3 = newPublishedPatchLineComment(psId, "file2", PatchLineComment comment3 = newPublishedPatchLineComment(psId, "file2",
uuid, range3, range3.getEndLine(), otherUser, null, time3, message3, uuid3, range3, range3.getEndLine(), otherUser, null, time3, message3,
(short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234"); (short) 1, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.upsertComment(comment3); update.upsertComment(comment3);
@@ -948,14 +950,14 @@ public class ChangeNotesTest {
+ "1:1-2:1\n" + "1:1-2:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n" + CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n"
+ "Author: Other Account <2@gerrit>\n" + "Author: Other Account <2@gerrit>\n"
+ "UUID: uuid\n" + "UUID: uuid1\n"
+ "Bytes: 9\n" + "Bytes: 9\n"
+ "comment 1\n" + "comment 1\n"
+ "\n" + "\n"
+ "2:1-3:1\n" + "2:1-3:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n" + CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n"
+ "Author: Other Account <2@gerrit>\n" + "Author: Other Account <2@gerrit>\n"
+ "UUID: uuid\n" + "UUID: uuid2\n"
+ "Bytes: 9\n" + "Bytes: 9\n"
+ "comment 2\n" + "comment 2\n"
+ "\n" + "\n"
@@ -964,7 +966,7 @@ public class ChangeNotesTest {
+ "3:1-4:1\n" + "3:1-4:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time3) + "\n" + CommentsInNotesUtil.formatTime(serverIdent, time3) + "\n"
+ "Author: Other Account <2@gerrit>\n" + "Author: Other Account <2@gerrit>\n"
+ "UUID: uuid\n" + "UUID: uuid3\n"
+ "Bytes: 9\n" + "Bytes: 9\n"
+ "comment 3\n" + "comment 3\n"
+ "\n", + "\n",
@@ -975,7 +977,8 @@ public class ChangeNotesTest {
public void patchLineCommentNotesFormatSide0() throws Exception { public void patchLineCommentNotesFormatSide0() throws Exception {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser); ChangeUpdate update = newUpdate(c, otherUser);
String uuid = "uuid"; String uuid1 = "uuid1";
String uuid2 = "uuid2";
String message1 = "comment 1"; String message1 = "comment 1";
String message2 = "comment 2"; String message2 = "comment 2";
CommentRange range1 = new CommentRange(1, 1, 2, 1); CommentRange range1 = new CommentRange(1, 1, 2, 1);
@@ -984,7 +987,7 @@ public class ChangeNotesTest {
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment comment1 = newPublishedPatchLineComment(psId, "file1", PatchLineComment comment1 = newPublishedPatchLineComment(psId, "file1",
uuid, range1, range1.getEndLine(), otherUser, null, time1, message1, uuid1, range1, range1.getEndLine(), otherUser, null, time1, message1,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234"); (short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.upsertComment(comment1); update.upsertComment(comment1);
@@ -993,7 +996,7 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser); update = newUpdate(c, otherUser);
CommentRange range2 = new CommentRange(2, 1, 3, 1); CommentRange range2 = new CommentRange(2, 1, 3, 1);
PatchLineComment comment2 = newPublishedPatchLineComment(psId, "file1", PatchLineComment comment2 = newPublishedPatchLineComment(psId, "file1",
uuid, range2, range2.getEndLine(), otherUser, null, time2, message2, uuid2, range2, range2.getEndLine(), otherUser, null, time2, message2,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234"); (short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.upsertComment(comment2); update.upsertComment(comment2);
@@ -1017,14 +1020,14 @@ public class ChangeNotesTest {
+ "1:1-2:1\n" + "1:1-2:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n" + CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n"
+ "Author: Other Account <2@gerrit>\n" + "Author: Other Account <2@gerrit>\n"
+ "UUID: uuid\n" + "UUID: uuid1\n"
+ "Bytes: 9\n" + "Bytes: 9\n"
+ "comment 1\n" + "comment 1\n"
+ "\n" + "\n"
+ "2:1-3:1\n" + "2:1-3:1\n"
+ CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n" + CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n"
+ "Author: Other Account <2@gerrit>\n" + "Author: Other Account <2@gerrit>\n"
+ "UUID: uuid\n" + "UUID: uuid2\n"
+ "Bytes: 9\n" + "Bytes: 9\n"
+ "comment 2\n" + "comment 2\n"
+ "\n", + "\n",
@@ -1037,7 +1040,8 @@ public class ChangeNotesTest {
throws Exception { throws Exception {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser); ChangeUpdate update = newUpdate(c, otherUser);
String uuid = "uuid"; String uuid1 = "uuid1";
String uuid2 = "uuid2";
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);
@@ -1045,7 +1049,7 @@ public class ChangeNotesTest {
PatchSet.Id psId = c.currentPatchSetId(); PatchSet.Id psId = c.currentPatchSetId();
PatchLineComment commentForBase = PatchLineComment commentForBase =
newPublishedPatchLineComment(psId, "filename", uuid, newPublishedPatchLineComment(psId, "filename", uuid1,
range, range.getEndLine(), otherUser, null, now, messageForBase, range, range.getEndLine(), otherUser, null, now, messageForBase,
(short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234"); (short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId); update.setPatchSetId(psId);
@@ -1054,7 +1058,7 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser); update = newUpdate(c, otherUser);
PatchLineComment commentForPS = PatchLineComment commentForPS =
newPublishedPatchLineComment(psId, "filename", uuid, newPublishedPatchLineComment(psId, "filename", uuid2,
range, range.getEndLine(), otherUser, null, now, messageForPS, range, range.getEndLine(), otherUser, null, now, messageForPS,
(short) 1, "abcd4567abcd4567abcd4567abcd4567abcd4567"); (short) 1, "abcd4567abcd4567abcd4567abcd4567abcd4567");
update.setPatchSetId(psId); update.setPatchSetId(psId);
@@ -1078,7 +1082,8 @@ public class ChangeNotesTest {
@Test @Test
public void patchLineCommentMultipleOnePatchsetOneFile() throws Exception { public void patchLineCommentMultipleOnePatchsetOneFile() throws Exception {
Change c = newChange(); Change c = newChange();
String uuid = "uuid"; String uuid1 = "uuid1";
String uuid2 = "uuid2";
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";
@@ -1088,7 +1093,7 @@ public class ChangeNotesTest {
Timestamp timeForComment1 = TimeUtil.nowTs(); Timestamp timeForComment1 = TimeUtil.nowTs();
Timestamp timeForComment2 = TimeUtil.nowTs(); Timestamp timeForComment2 = TimeUtil.nowTs();
PatchLineComment comment1 = newPublishedPatchLineComment(psId, filename, PatchLineComment comment1 = newPublishedPatchLineComment(psId, filename,
uuid, range, range.getEndLine(), otherUser, null, timeForComment1, uuid1, range, range.getEndLine(), otherUser, null, timeForComment1,
"comment 1", side, "abcd1234abcd1234abcd1234abcd1234abcd1234"); "comment 1", side, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.upsertComment(comment1); update.upsertComment(comment1);
@@ -1096,7 +1101,7 @@ public class ChangeNotesTest {
update = newUpdate(c, otherUser); update = newUpdate(c, otherUser);
PatchLineComment comment2 = newPublishedPatchLineComment(psId, filename, PatchLineComment comment2 = newPublishedPatchLineComment(psId, filename,
uuid, range, range.getEndLine(), otherUser, null, timeForComment2, uuid2, range, range.getEndLine(), otherUser, null, timeForComment2,
"comment 2", side, "abcd1234abcd1234abcd1234abcd1234abcd1234"); "comment 2", side, "abcd1234abcd1234abcd1234abcd1234abcd1234");
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.upsertComment(comment2); update.upsertComment(comment2);