From a6ec09622c512f1c0978ff3cb08293ba19e31f7a Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Wed, 16 Oct 2019 14:17:56 +0200 Subject: [PATCH] PublishCommentUtil.publish: Fix exception message if patch set not found If a patch set from a draft comment was not found, the exception message was always "patch set null not found". Where it says 'null' we expect the ID of the patch set that was not found, instead the code used the patch set (not the patch set ID) for the message and since the patch set was not found, it is always null. While we are here, improve the name of some variables. Signed-off-by: Edwin Kempin Change-Id: I8f5e7b1d878ee3851af313eb1629daf62247c109 --- .../gerrit/server/PublishCommentUtil.java | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/java/com/google/gerrit/server/PublishCommentUtil.java b/java/com/google/gerrit/server/PublishCommentUtil.java index 6ebebcf994..f55bdf60bf 100644 --- a/java/com/google/gerrit/server/PublishCommentUtil.java +++ b/java/com/google/gerrit/server/PublishCommentUtil.java @@ -51,32 +51,36 @@ public class PublishCommentUtil { } public void publish( - ChangeContext ctx, PatchSet.Id psId, Collection drafts, @Nullable String tag) { + ChangeContext ctx, + PatchSet.Id psId, + Collection draftComments, + @Nullable String tag) { ChangeNotes notes = ctx.getNotes(); checkArgument(notes != null); - if (drafts.isEmpty()) { + if (draftComments.isEmpty()) { return; } Map patchSets = - psUtil.getAsMap(notes, drafts.stream().map(d -> psId(notes, d)).collect(toSet())); - for (Comment d : drafts) { - PatchSet ps = patchSets.get(psId(notes, d)); + psUtil.getAsMap(notes, draftComments.stream().map(d -> psId(notes, d)).collect(toSet())); + for (Comment draftComment : draftComments) { + PatchSet.Id psIdOfDraftComment = psId(notes, draftComment); + PatchSet ps = patchSets.get(psIdOfDraftComment); if (ps == null) { - throw new StorageException("patch set " + ps + " not found"); + throw new StorageException("patch set " + psIdOfDraftComment + " not found"); } - d.writtenOn = ctx.getWhen(); - d.tag = tag; + draftComment.writtenOn = ctx.getWhen(); + draftComment.tag = tag; // Draft may have been created by a different real user; copy the current real user. (Only // applies to X-Gerrit-RunAs, since modifying drafts via on_behalf_of is not allowed.) - ctx.getUser().updateRealAccountId(d::setRealAuthor); + ctx.getUser().updateRealAccountId(draftComment::setRealAuthor); try { - CommentsUtil.setCommentCommitId(d, patchListCache, notes.getChange(), ps); + CommentsUtil.setCommentCommitId(draftComment, patchListCache, notes.getChange(), ps); } catch (PatchListNotAvailableException e) { throw new StorageException(e); } } - commentsUtil.putComments(ctx.getUpdate(psId), PUBLISHED, drafts); + commentsUtil.putComments(ctx.getUpdate(psId), PUBLISHED, draftComments); } private static PatchSet.Id psId(ChangeNotes notes, Comment c) {