diff --git a/java/com/google/gerrit/server/CommentsUtil.java b/java/com/google/gerrit/server/CommentsUtil.java index 99dfbbb1a2..b3812a0870 100644 --- a/java/com/google/gerrit/server/CommentsUtil.java +++ b/java/com/google/gerrit/server/CommentsUtil.java @@ -16,16 +16,12 @@ package com.google.gerrit.server; import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.gerrit.reviewdb.client.PatchLineComment.Status.PUBLISHED; import static java.util.stream.Collectors.toList; import com.google.common.collect.ComparisonChain; import com.google.common.collect.FluentIterable; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Ordering; -import com.google.common.collect.Streams; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.client.Side; import com.google.gerrit.extensions.common.CommentInfo; @@ -35,31 +31,23 @@ import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Comment; import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchLineComment; -import com.google.gerrit.reviewdb.client.PatchLineComment.Status; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RobotComment; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.reviewdb.server.ReviewDbUtil; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.GerritServerId; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeUpdate; -import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage; -import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.patch.PatchListCache; import com.google.gerrit.server.patch.PatchListNotAvailableException; -import com.google.gerrit.server.update.BatchUpdateReviewDb; import com.google.gerrit.server.update.ChangeContext; import com.google.gwtorm.server.OrmException; -import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.Optional; import org.eclipse.jgit.lib.BatchRefUpdate; @@ -70,12 +58,7 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.ReceiveCommand; -/** - * Utility functions to manipulate Comments. - * - *

These methods either query for and update Comments in the NoteDb or ReviewDb, depending on the - * state of the NotesMigration. - */ +/** Utility functions to manipulate Comments. */ @Singleton public class CommentsUtil { public static final Ordering COMMENT_ORDER = @@ -127,18 +110,13 @@ public class CommentsUtil { private final GitRepositoryManager repoManager; private final AllUsersName allUsers; - private final NotesMigration migration; private final String serverId; @Inject CommentsUtil( - GitRepositoryManager repoManager, - AllUsersName allUsers, - NotesMigration migration, - @GerritServerId String serverId) { + GitRepositoryManager repoManager, AllUsersName allUsers, @GerritServerId String serverId) { this.repoManager = repoManager; this.allUsers = allUsers; - this.migration = migration; this.serverId = serverId; } @@ -158,7 +136,7 @@ public class CommentsUtil { } else { // Inherit unresolved value from inReplyTo comment if not specified. Comment.Key key = new Comment.Key(parentUuid, path, psId.patchSetId); - Optional parent = getPublished(ctx.getDb(), ctx.getNotes(), key); + Optional parent = getPublished(ctx.getNotes(), key); if (!parent.isPresent()) { throw new UnprocessableEntityException("Invalid parentUuid supplied for comment"); } @@ -201,119 +179,64 @@ public class CommentsUtil { return c; } - public Optional getPublished(ReviewDb db, ChangeNotes notes, Comment.Key key) - throws OrmException { - if (!migration.readChanges()) { - return getReviewDb(db, notes, key); - } - return publishedByChange(db, notes).stream().filter(c -> key.equals(c.key)).findFirst(); + public Optional getPublished(ChangeNotes notes, Comment.Key key) throws OrmException { + return publishedByChange(notes).stream().filter(c -> key.equals(c.key)).findFirst(); } - public Optional getDraft( - ReviewDb db, ChangeNotes notes, IdentifiedUser user, Comment.Key key) throws OrmException { - if (!migration.readChanges()) { - Optional c = getReviewDb(db, notes, key); - if (c.isPresent() && !c.get().author.getId().equals(user.getAccountId())) { - throw new OrmException( - String.format( - "Expected draft %s to belong to account %s, but it belongs to %s", - key, user.getAccountId(), c.get().author.getId())); - } - return c; - } - return draftByChangeAuthor(db, notes, user.getAccountId()) + public Optional getDraft(ChangeNotes notes, IdentifiedUser user, Comment.Key key) + throws OrmException { + return draftByChangeAuthor(notes, user.getAccountId()) .stream() .filter(c -> key.equals(c.key)) .findFirst(); } - private Optional getReviewDb(ReviewDb db, ChangeNotes notes, Comment.Key key) - throws OrmException { - return Optional.ofNullable( - db.patchComments().get(PatchLineComment.Key.from(notes.getChangeId(), key))) - .map(plc -> plc.asComment(serverId)); - } - - public List publishedByChange(ReviewDb db, ChangeNotes notes) throws OrmException { - if (!migration.readChanges()) { - return sort(byCommentStatus(db.patchComments().byChange(notes.getChangeId()), PUBLISHED)); - } - + public List publishedByChange(ChangeNotes notes) throws OrmException { notes.load(); return sort(Lists.newArrayList(notes.getComments().values())); } public List robotCommentsByChange(ChangeNotes notes) throws OrmException { - if (!migration.readChanges()) { - return ImmutableList.of(); - } - notes.load(); return sort(Lists.newArrayList(notes.getRobotComments().values())); } - public List draftByChange(ReviewDb db, ChangeNotes notes) throws OrmException { - if (!migration.readChanges()) { - return sort(byCommentStatus(db.patchComments().byChange(notes.getChangeId()), Status.DRAFT)); - } - + public List draftByChange(ChangeNotes notes) throws OrmException { List comments = new ArrayList<>(); for (Ref ref : getDraftRefs(notes.getChangeId())) { Account.Id account = Account.Id.fromRefSuffix(ref.getName()); if (account != null) { - comments.addAll(draftByChangeAuthor(db, notes, account)); + comments.addAll(draftByChangeAuthor(notes, account)); } } return sort(comments); } - private List byCommentStatus( - ResultSet comments, PatchLineComment.Status status) { - return toComments( - serverId, Lists.newArrayList(Iterables.filter(comments, c -> c.getStatus() == status))); - } - - public List byPatchSet(ReviewDb db, ChangeNotes notes, PatchSet.Id psId) - throws OrmException { - if (!migration.readChanges()) { - return sort(toComments(serverId, db.patchComments().byPatchSet(psId).toList())); - } + public List byPatchSet(ChangeNotes notes, PatchSet.Id psId) throws OrmException { List comments = new ArrayList<>(); - comments.addAll(publishedByPatchSet(db, notes, psId)); + comments.addAll(publishedByPatchSet(notes, psId)); for (Ref ref : getDraftRefs(notes.getChangeId())) { Account.Id account = Account.Id.fromRefSuffix(ref.getName()); if (account != null) { - comments.addAll(draftByPatchSetAuthor(db, psId, account, notes)); + comments.addAll(draftByPatchSetAuthor(psId, account, notes)); } } return sort(comments); } - public List publishedByChangeFile( - ReviewDb db, ChangeNotes notes, Change.Id changeId, String file) throws OrmException { - if (!migration.readChanges()) { - return sort( - toComments(serverId, db.patchComments().publishedByChangeFile(changeId, file).toList())); - } + public List publishedByChangeFile(ChangeNotes notes, String file) throws OrmException { return commentsOnFile(notes.load().getComments().values(), file); } - public List publishedByPatchSet(ReviewDb db, ChangeNotes notes, PatchSet.Id psId) + public List publishedByPatchSet(ChangeNotes notes, PatchSet.Id psId) throws OrmException { - if (!migration.readChanges()) { - return removeCommentsOnAncestorOfCommitMessage( - sort(toComments(serverId, db.patchComments().publishedByPatchSet(psId).toList()))); - } return removeCommentsOnAncestorOfCommitMessage( commentsOnPatchSet(notes.load().getComments().values(), psId)); } public List robotCommentsByPatchSet(ChangeNotes notes, PatchSet.Id psId) throws OrmException { - if (!migration.readChanges()) { - return ImmutableList.of(); - } return commentsOnPatchSet(notes.load().getRobotComments().values(), psId); } @@ -330,49 +253,28 @@ public class CommentsUtil { .collect(toList()); } - public List draftByPatchSetAuthor( - ReviewDb db, PatchSet.Id psId, Account.Id author, ChangeNotes notes) throws OrmException { - if (!migration.readChanges()) { - return sort( - toComments(serverId, db.patchComments().draftByPatchSetAuthor(psId, author).toList())); - } + public List draftByPatchSetAuthor(PatchSet.Id psId, Account.Id author, ChangeNotes notes) + throws OrmException { return commentsOnPatchSet(notes.load().getDraftComments(author).values(), psId); } - public List draftByChangeFileAuthor( - ReviewDb db, ChangeNotes notes, String file, Account.Id author) throws OrmException { - if (!migration.readChanges()) { - return sort( - toComments( - serverId, - db.patchComments() - .draftByChangeFileAuthor(notes.getChangeId(), file, author) - .toList())); - } + public List draftByChangeFileAuthor(ChangeNotes notes, String file, Account.Id author) + throws OrmException { return commentsOnFile(notes.load().getDraftComments(author).values(), file); } - public List draftByChangeAuthor(ReviewDb db, ChangeNotes notes, Account.Id author) + public List draftByChangeAuthor(ChangeNotes notes, Account.Id author) throws OrmException { - if (!migration.readChanges()) { - return Streams.stream(db.patchComments().draftByAuthor(author)) - .filter(c -> c.getPatchSetId().getParentKey().equals(notes.getChangeId())) - .map(plc -> plc.asComment(serverId)) - .sorted(COMMENT_ORDER) - .collect(toList()); - } List comments = new ArrayList<>(); comments.addAll(notes.getDraftComments(author).values()); return sort(comments); } public void putComments( - ReviewDb db, ChangeUpdate update, PatchLineComment.Status status, Iterable comments) - throws OrmException { + ChangeUpdate update, PatchLineComment.Status status, Iterable comments) { for (Comment c : comments) { update.putComment(status, c); } - db.patchComments().upsert(toPatchLineComments(update.getId(), status, comments)); } public void putRobotComments(ChangeUpdate update, Iterable comments) { @@ -381,37 +283,14 @@ public class CommentsUtil { } } - public void deleteComments(ReviewDb db, ChangeUpdate update, Iterable comments) - throws OrmException { + public void deleteComments(ChangeUpdate update, Iterable comments) { for (Comment c : comments) { update.deleteComment(c); } - db.patchComments() - .delete(toPatchLineComments(update.getId(), PatchLineComment.Status.DRAFT, comments)); } public void deleteCommentByRewritingHistory( - ReviewDb db, ChangeUpdate update, Comment.Key commentKey, PatchSet.Id psId, String newMessage) - throws OrmException { - if (PrimaryStorage.of(update.getChange()).equals(PrimaryStorage.REVIEW_DB)) { - PatchLineComment.Key key = - new PatchLineComment.Key(new Patch.Key(psId, commentKey.filename), commentKey.uuid); - - if (db instanceof BatchUpdateReviewDb) { - db = ((BatchUpdateReviewDb) db).unsafeGetDelegate(); - } - db = ReviewDbUtil.unwrapDb(db); - - PatchLineComment patchLineComment = db.patchComments().get(key); - - if (!patchLineComment.getStatus().equals(PUBLISHED)) { - throw new OrmException(String.format("comment %s is not published", key)); - } - - patchLineComment.setMessage(newMessage); - db.patchComments().upsert(Collections.singleton(patchLineComment)); - } - + ChangeUpdate update, Comment.Key commentKey, String newMessage) { update.deleteCommentByRewritingHistory(commentKey.uuid, newMessage); } diff --git a/java/com/google/gerrit/server/PublishCommentUtil.java b/java/com/google/gerrit/server/PublishCommentUtil.java index a90f3e7365..9d5ecb435d 100644 --- a/java/com/google/gerrit/server/PublishCommentUtil.java +++ b/java/com/google/gerrit/server/PublishCommentUtil.java @@ -74,7 +74,7 @@ public class PublishCommentUtil { throw new OrmException(e); } } - commentsUtil.putComments(ctx.getDb(), ctx.getUpdate(psId), PUBLISHED, drafts); + commentsUtil.putComments(ctx.getUpdate(psId), PUBLISHED, drafts); } private static PatchSet.Id psId(ChangeNotes notes, Comment c) { diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java index a580cf6170..34189b5c8b 100644 --- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java +++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java @@ -512,7 +512,7 @@ public class ReplaceOp implements BatchUpdateOp { private List publishComments(ChangeContext ctx, boolean workInProgress) throws OrmException { List comments = - commentsUtil.draftByChangeAuthor(ctx.getDb(), ctx.getNotes(), ctx.getUser().getAccountId()); + commentsUtil.draftByChangeAuthor(ctx.getNotes(), ctx.getUser().getAccountId()); publishCommentUtil.publish( ctx, patchSetId, comments, ChangeMessagesUtil.uploadedPatchSetTag(workInProgress)); return comments; diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java index 262e82bc5c..775fa35e0e 100644 --- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java +++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java @@ -301,10 +301,7 @@ public class MailProcessor { persistentCommentFromMailComment(ctx, c, targetPatchSetForComment(ctx, c, patchSet))); } commentsUtil.putComments( - ctx.getDb(), - ctx.getUpdate(ctx.getChange().currentPatchSetId()), - Status.PUBLISHED, - comments); + ctx.getUpdate(ctx.getChange().currentPatchSetId()), Status.PUBLISHED, comments); return true; } diff --git a/java/com/google/gerrit/server/mail/send/CommentSender.java b/java/com/google/gerrit/server/mail/send/CommentSender.java index e810397a3a..c8aa259be0 100644 --- a/java/com/google/gerrit/server/mail/send/CommentSender.java +++ b/java/com/google/gerrit/server/mail/send/CommentSender.java @@ -316,7 +316,7 @@ public class CommentSender extends ReplyToChangeSender { Comment.Key key = new Comment.Key(child.parentUuid, child.key.filename, child.key.patchSetId); try { - return commentsUtil.getPublished(args.db.get(), changeData.notes(), key); + return commentsUtil.getPublished(changeData.notes(), key); } catch (OrmException e) { logger.atWarning().log("Could not find the parent of this comment: %s", child); return Optional.empty(); diff --git a/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/java/com/google/gerrit/server/patch/PatchScriptFactory.java index b1e0e3cf68..dca5098c58 100644 --- a/java/com/google/gerrit/server/patch/PatchScriptFactory.java +++ b/java/com/google/gerrit/server/patch/PatchScriptFactory.java @@ -391,7 +391,7 @@ public class PatchScriptFactory implements Callable { } private void loadPublished(Map byKey, String file) throws OrmException { - for (Comment c : commentsUtil.publishedByChangeFile(db, notes, changeId, file)) { + for (Comment c : commentsUtil.publishedByChangeFile(notes, file)) { comments.include(notes.getChangeId(), c); PatchSet.Id psId = new PatchSet.Id(notes.getChangeId(), c.key.patchSetId); Patch.Key pKey = new Patch.Key(psId, c.key.filename); @@ -404,7 +404,7 @@ public class PatchScriptFactory implements Callable { private void loadDrafts(Map byKey, Account.Id me, String file) throws OrmException { - for (Comment c : commentsUtil.draftByChangeFileAuthor(db, notes, file, me)) { + for (Comment c : commentsUtil.draftByChangeFileAuthor(notes, file, me)) { comments.include(notes.getChangeId(), c); PatchSet.Id psId = new PatchSet.Id(notes.getChangeId(), c.key.patchSetId); Patch.Key pKey = new Patch.Key(psId, c.key.filename); diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java index b129fc48bf..3d0fed98df 100644 --- a/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/java/com/google/gerrit/server/query/change/ChangeData.java @@ -858,7 +858,7 @@ public class ChangeData { if (!lazyLoad) { return Collections.emptyList(); } - publishedComments = commentsUtil.publishedByChange(db, notes()); + publishedComments = commentsUtil.publishedByChange(notes()); } return publishedComments; } @@ -1097,7 +1097,7 @@ public class ChangeData { } } } else { - for (Comment sc : commentsUtil.draftByChange(db, notes())) { + for (Comment sc : commentsUtil.draftByChange(notes())) { draftsByUser.put(sc.author.getId(), null); } } diff --git a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java index 108ee0e04f..3c553a1075 100644 --- a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java +++ b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java @@ -184,12 +184,12 @@ public class DeleteDraftComments throws OrmException, PatchListNotAvailableException, PermissionBackendException { ImmutableList.Builder comments = ImmutableList.builder(); boolean dirty = false; - for (Comment c : commentsUtil.draftByChangeAuthor(ctx.getDb(), ctx.getNotes(), accountId)) { + for (Comment c : commentsUtil.draftByChangeAuthor(ctx.getNotes(), accountId)) { dirty = true; PatchSet.Id psId = new PatchSet.Id(ctx.getChange().getId(), c.key.patchSetId); setCommentRevId( c, patchListCache, ctx.getChange(), psUtil.get(ctx.getDb(), ctx.getNotes(), psId)); - commentsUtil.deleteComments(ctx.getDb(), ctx.getUpdate(psId), Collections.singleton(c)); + commentsUtil.deleteComments(ctx.getUpdate(psId), Collections.singleton(c)); comments.add(commentFormatter.format(c)); } if (dirty) { diff --git a/java/com/google/gerrit/server/restapi/change/Comments.java b/java/com/google/gerrit/server/restapi/change/Comments.java index f563cc620c..22f376be98 100644 --- a/java/com/google/gerrit/server/restapi/change/Comments.java +++ b/java/com/google/gerrit/server/restapi/change/Comments.java @@ -20,32 +20,27 @@ import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.reviewdb.client.Comment; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CommentsUtil; import com.google.gerrit.server.change.CommentResource; import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.Singleton; @Singleton public class Comments implements ChildCollection { private final DynamicMap> views; private final ListRevisionComments list; - private final Provider dbProvider; private final CommentsUtil commentsUtil; @Inject Comments( DynamicMap> views, ListRevisionComments list, - Provider dbProvider, CommentsUtil commentsUtil) { this.views = views; this.list = list; - this.dbProvider = dbProvider; this.commentsUtil = commentsUtil; } @@ -65,8 +60,7 @@ public class Comments implements ChildCollection changeComments = commentsUtil.publishedByChange(dbProvider.get(), updatedNotes); + List changeComments = commentsUtil.publishedByChange(updatedNotes); Optional updatedComment = changeComments.stream().filter(c -> c.key.equals(rsrc.getComment().key)).findFirst(); if (!updatedComment.isPresent()) { @@ -127,14 +127,10 @@ public class DeleteComment @Override public boolean updateChange(ChangeContext ctx) - throws ResourceConflictException, OrmException, ResourceNotFoundException { + throws ResourceConflictException, ResourceNotFoundException { PatchSet.Id psId = ctx.getChange().currentPatchSetId(); commentsUtil.deleteCommentByRewritingHistory( - ctx.getDb(), - ctx.getUpdate(psId), - rsrc.getComment().key, - rsrc.getPatchSet().getId(), - newMessage); + ctx.getUpdate(psId), rsrc.getComment().key, newMessage); return true; } } diff --git a/java/com/google/gerrit/server/restapi/change/DeleteDraftComment.java b/java/com/google/gerrit/server/restapi/change/DeleteDraftComment.java index f8e3add7db..0d9629fa04 100644 --- a/java/com/google/gerrit/server/restapi/change/DeleteDraftComment.java +++ b/java/com/google/gerrit/server/restapi/change/DeleteDraftComment.java @@ -91,7 +91,7 @@ public class DeleteDraftComment public boolean updateChange(ChangeContext ctx) throws ResourceNotFoundException, OrmException, PatchListNotAvailableException { Optional maybeComment = - commentsUtil.getDraft(ctx.getDb(), ctx.getNotes(), ctx.getIdentifiedUser(), key); + commentsUtil.getDraft(ctx.getNotes(), ctx.getIdentifiedUser(), key); if (!maybeComment.isPresent()) { return false; // Nothing to do. } @@ -102,7 +102,7 @@ public class DeleteDraftComment } Comment c = maybeComment.get(); setCommentRevId(c, patchListCache, ctx.getChange(), ps); - commentsUtil.deleteComments(ctx.getDb(), ctx.getUpdate(psId), Collections.singleton(c)); + commentsUtil.deleteComments(ctx.getUpdate(psId), Collections.singleton(c)); ctx.dontBumpLastUpdatedOn(); return true; } diff --git a/java/com/google/gerrit/server/restapi/change/DraftComments.java b/java/com/google/gerrit/server/restapi/change/DraftComments.java index b8e24a5408..9f06252d0f 100644 --- a/java/com/google/gerrit/server/restapi/change/DraftComments.java +++ b/java/com/google/gerrit/server/restapi/change/DraftComments.java @@ -21,7 +21,6 @@ import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.reviewdb.client.Comment; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CommentsUtil; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.change.DraftCommentResource; @@ -36,7 +35,6 @@ public class DraftComments implements ChildCollection> views; private final Provider user; private final ListRevisionDrafts list; - private final Provider dbProvider; private final CommentsUtil commentsUtil; @Inject @@ -44,12 +42,10 @@ public class DraftComments implements ChildCollection> views, Provider user, ListRevisionDrafts list, - Provider dbProvider, CommentsUtil commentsUtil) { this.views = views; this.user = user; this.list = list; - this.dbProvider = dbProvider; this.commentsUtil = commentsUtil; } @@ -71,7 +67,7 @@ public class DraftComments implements ChildCollection { .setFillAccounts(true) .setFillPatchSet(true) .newCommentFormatter() - .format(commentsUtil.publishedByChange(db.get(), cd.notes())); + .format(commentsUtil.publishedByChange(cd.notes())); } } diff --git a/java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java b/java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java index a524f6dec1..280277c287 100644 --- a/java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java +++ b/java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java @@ -57,7 +57,7 @@ public class ListChangeDrafts implements RestReadView { } ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes()); List drafts = - commentsUtil.draftByChangeAuthor(db.get(), cd.notes(), rsrc.getUser().getAccountId()); + commentsUtil.draftByChangeAuthor(cd.notes(), rsrc.getUser().getAccountId()); return commentJson .get() .setFillAccounts(false) diff --git a/java/com/google/gerrit/server/restapi/change/ListRevisionComments.java b/java/com/google/gerrit/server/restapi/change/ListRevisionComments.java index 964e56043f..f10d92bb83 100644 --- a/java/com/google/gerrit/server/restapi/change/ListRevisionComments.java +++ b/java/com/google/gerrit/server/restapi/change/ListRevisionComments.java @@ -15,7 +15,6 @@ package com.google.gerrit.server.restapi.change; import com.google.gerrit.reviewdb.client.Comment; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CommentsUtil; import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.notedb.ChangeNotes; @@ -27,9 +26,8 @@ import com.google.inject.Singleton; @Singleton public class ListRevisionComments extends ListRevisionDrafts { @Inject - ListRevisionComments( - Provider db, Provider commentJson, CommentsUtil commentsUtil) { - super(db, commentJson, commentsUtil); + ListRevisionComments(Provider commentJson, CommentsUtil commentsUtil) { + super(commentJson, commentsUtil); } @Override @@ -40,6 +38,6 @@ public class ListRevisionComments extends ListRevisionDrafts { @Override protected Iterable listComments(RevisionResource rsrc) throws OrmException { ChangeNotes notes = rsrc.getNotes(); - return commentsUtil.publishedByPatchSet(db.get(), notes, rsrc.getPatchSet().getId()); + return commentsUtil.publishedByPatchSet(notes, rsrc.getPatchSet().getId()); } } diff --git a/java/com/google/gerrit/server/restapi/change/ListRevisionDrafts.java b/java/com/google/gerrit/server/restapi/change/ListRevisionDrafts.java index dbd0ccf233..3df7e9c37c 100644 --- a/java/com/google/gerrit/server/restapi/change/ListRevisionDrafts.java +++ b/java/com/google/gerrit/server/restapi/change/ListRevisionDrafts.java @@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableList; import com.google.gerrit.extensions.common.CommentInfo; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.reviewdb.client.Comment; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CommentsUtil; import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.permissions.PermissionBackendException; @@ -31,21 +30,18 @@ import java.util.Map; @Singleton public class ListRevisionDrafts implements RestReadView { - protected final Provider db; protected final Provider commentJson; protected final CommentsUtil commentsUtil; @Inject - ListRevisionDrafts( - Provider db, Provider commentJson, CommentsUtil commentsUtil) { - this.db = db; + ListRevisionDrafts(Provider commentJson, CommentsUtil commentsUtil) { this.commentJson = commentJson; this.commentsUtil = commentsUtil; } protected Iterable listComments(RevisionResource rsrc) throws OrmException { return commentsUtil.draftByPatchSetAuthor( - db.get(), rsrc.getPatchSet().getId(), rsrc.getAccountId(), rsrc.getNotes()); + rsrc.getPatchSet().getId(), rsrc.getAccountId(), rsrc.getNotes()); } protected boolean includeAuthorInfo() { diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java index e40a991e8c..eece3bc8b0 100644 --- a/java/com/google/gerrit/server/restapi/change/PostReview.java +++ b/java/com/google/gerrit/server/restapi/change/PostReview.java @@ -990,7 +990,7 @@ public class PostReview break; } ChangeUpdate u = ctx.getUpdate(psId); - commentsUtil.putComments(ctx.getDb(), u, Status.PUBLISHED, toPublish); + commentsUtil.putComments(u, Status.PUBLISHED, toPublish); comments.addAll(toPublish); return !toPublish.isEmpty(); } @@ -1079,7 +1079,7 @@ public class PostReview private Set readExistingComments(ChangeContext ctx) throws OrmException { return commentsUtil - .publishedByChange(ctx.getDb(), ctx.getNotes()) + .publishedByChange(ctx.getNotes()) .stream() .map(CommentSetEntry::create) .collect(toSet()); @@ -1095,8 +1095,7 @@ public class PostReview private Map changeDrafts(ChangeContext ctx) throws OrmException { Map drafts = new HashMap<>(); - for (Comment c : - commentsUtil.draftByChangeAuthor(ctx.getDb(), ctx.getNotes(), user.getAccountId())) { + for (Comment c : commentsUtil.draftByChangeAuthor(ctx.getNotes(), user.getAccountId())) { c.tag = in.tag; drafts.put(c.key.uuid, c); } @@ -1106,8 +1105,7 @@ public class PostReview private Map patchSetDrafts(ChangeContext ctx) throws OrmException { Map drafts = new HashMap<>(); for (Comment c : - commentsUtil.draftByPatchSetAuthor( - ctx.getDb(), psId, user.getAccountId(), ctx.getNotes())) { + commentsUtil.draftByPatchSetAuthor(psId, user.getAccountId(), ctx.getNotes())) { drafts.put(c.key.uuid, c); } return drafts; diff --git a/java/com/google/gerrit/server/restapi/change/PutDraftComment.java b/java/com/google/gerrit/server/restapi/change/PutDraftComment.java index 72358bd6c8..dfa32bfcbe 100644 --- a/java/com/google/gerrit/server/restapi/change/PutDraftComment.java +++ b/java/com/google/gerrit/server/restapi/change/PutDraftComment.java @@ -118,7 +118,7 @@ public class PutDraftComment public boolean updateChange(ChangeContext ctx) throws ResourceNotFoundException, OrmException, PatchListNotAvailableException { Optional maybeComment = - commentsUtil.getDraft(ctx.getDb(), ctx.getNotes(), ctx.getIdentifiedUser(), key); + commentsUtil.getDraft(ctx.getNotes(), ctx.getIdentifiedUser(), key); if (!maybeComment.isPresent()) { // Disappeared out from under us. Can't easily fall back to insert, // because the input might be missing required fields. Just give up. @@ -141,15 +141,12 @@ public class PutDraftComment // Updating the path alters the primary key, which isn't possible. // Delete then recreate the comment instead of an update. - commentsUtil.deleteComments(ctx.getDb(), update, Collections.singleton(origComment)); + commentsUtil.deleteComments(update, Collections.singleton(origComment)); comment.key.filename = in.path; } setCommentRevId(comment, patchListCache, ctx.getChange(), ps); commentsUtil.putComments( - ctx.getDb(), - update, - Status.DRAFT, - Collections.singleton(update(comment, in, ctx.getWhen()))); + update, Status.DRAFT, Collections.singleton(update(comment, in, ctx.getWhen()))); ctx.dontBumpLastUpdatedOn(); return true; } diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java index 6e8febf6ee..94910204e8 100644 --- a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java @@ -216,7 +216,7 @@ public class ImpersonationIT extends AbstractDaemonTest { assertThat(psa.getRealAccountId()).isEqualTo(admin.id); ChangeData cd = r.getChange(); - Comment c = Iterables.getOnlyElement(commentsUtil.publishedByChange(db, cd.notes())); + Comment c = Iterables.getOnlyElement(commentsUtil.publishedByChange(cd.notes())); assertThat(c.message).isEqualTo(ci.message); assertThat(c.author.getId()).isEqualTo(user.id); assertThat(c.getRealAuthor().getId()).isEqualTo(admin.id); diff --git a/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java b/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java index b63adabf74..fbec5e6bcb 100644 --- a/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java +++ b/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java @@ -547,11 +547,9 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest { // TODO(dborowitz): Comparing collections directly would be much easier, but Comment doesn't // have a proper equals; switch to that when the issues with // https://gerrit-review.googlesource.com/c/gerrit/+/207013 are resolved. + assertCommentsEqual(commentsUtil.draftByChange(actual), commentsUtil.draftByChange(expected)); assertCommentsEqual( - commentsUtil.draftByChange(null, actual), commentsUtil.draftByChange(null, expected)); - assertCommentsEqual( - commentsUtil.publishedByChange(null, actual), - commentsUtil.publishedByChange(null, expected)); + commentsUtil.publishedByChange(actual), commentsUtil.publishedByChange(expected)); // Change metadata is equal. assertLogEqualExceptTrees(