opendev/gerrit
Code Issues Proposed changes

Factor out a CommentsUtil method to publish comments

Browse Source 
In addition to being reusable by other operaions, this has the side
benefit of allowing us to treat PUBLISH and PUBLISH_ALL_REVISIONS
identically at the point where they are published.

Change-Id: I0421608e7f8f2e10f915b5a2937ab8a0d10accb6
This commit is contained in:
Dave Borowitz
2017-04-27 06:00:38 -04:00
parent b782fc9279
commit 584506c05b
3 changed files with 61 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server;
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
import com.google.common.collect.ComparisonChain;
import com.google.common.collect.FluentIterable;
@@ -56,6 +57,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.stream.StreamSupport;
@@ -125,6 +127,8 @@ public class CommentsUtil {
private final GitRepositoryManager repoManager;
private final AllUsersName allUsers;
private final NotesMigration migration;
private final PatchListCache patchListCache;
private final PatchSetUtil psUtil;
private final String serverId;
@Inject
@@ -132,10 +136,14 @@ public class CommentsUtil {
GitRepositoryManager repoManager,
AllUsersName allUsers,
NotesMigration migration,
PatchListCache patchListCache,
PatchSetUtil psUtil,
@GerritServerId String serverId) {
this.repoManager = repoManager;
this.allUsers = allUsers;
this.migration = migration;
this.patchListCache = patchListCache;
this.psUtil = psUtil;
this.serverId = serverId;
}
@@ -500,4 +508,35 @@ public class CommentsUtil {
return COMMENT_ORDER.sortedCopy(
FluentIterable.from(comments).transform(plc -> plc.asComment(serverId)));
}
public void publish(
ChangeContext ctx, PatchSet.Id psId, Collection<Comment> drafts, @Nullable String tag)
throws OrmException {
ChangeNotes notes = ctx.getNotes();
checkArgument(notes != null);
if (drafts.isEmpty()) {
return;
}
Map<PatchSet.Id, PatchSet> patchSets =
psUtil.getAsMap(
ctx.getDb(), notes, drafts.stream().map(d -> psId(notes, d)).collect(toSet()));
for (Comment d : drafts) {
PatchSet ps = patchSets.get(psId(notes, d));
if (ps == null) {
throw new OrmException("patch set " + ps + " not found");
}
d.writtenOn = ctx.getWhen();
d.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);
setCommentRevId(d, patchListCache, notes.getChange(), ps);
}
putComments(ctx.getDb(), ctx.getUpdate(psId), PatchLineComment.Status.PUBLISHED, drafts);
}
private static PatchSet.Id psId(ChangeNotes notes, Comment c) {
return new PatchSet.Id(notes.getChangeId(), c.key.patchSetId);
}
}

24
gerrit-server/src/main/java/com/google/gerrit/server/PatchSetUtil.java
View File

@@ -16,11 +16,17 @@ package com.google.gerrit.server;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.gerrit.server.ChangeUtil.PS_ID_ORDER;
import static com.google.gerrit.server.notedb.PatchSetState.DRAFT;
import static com.google.gerrit.server.notedb.PatchSetState.PUBLISHED;
import static java.util.function.Function.identity;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId;
@@ -36,6 +42,7 @@ import java.io.IOException;
import java.sql.Timestamp;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -63,8 +70,7 @@ public class PatchSetUtil {
public ImmutableCollection<PatchSet> byChange(ReviewDb db, ChangeNotes notes)
throws OrmException {
if (!migration.readChanges()) {
return ChangeUtil.PS_ID_ORDER.immutableSortedCopy(
db.patchSets().byChange(notes.getChangeId()));
return PS_ID_ORDER.immutableSortedCopy(db.patchSets().byChange(notes.getChangeId()));
}
return notes.load().getPatchSets().values();
}
@@ -73,8 +79,7 @@ public class PatchSetUtil {
throws OrmException {
if (!migration.readChanges()) {
ImmutableMap.Builder<PatchSet.Id, PatchSet> result = ImmutableMap.builder();
for (PatchSet ps :
ChangeUtil.PS_ID_ORDER.sortedCopy(db.patchSets().byChange(notes.getChangeId()))) {
for (PatchSet ps : PS_ID_ORDER.sortedCopy(db.patchSets().byChange(notes.getChangeId()))) {
result.put(ps.getId(), ps);
}
return result.build();
@@ -82,6 +87,17 @@ public class PatchSetUtil {
return notes.load().getPatchSets();
}
public ImmutableMap<PatchSet.Id, PatchSet> getAsMap(
ReviewDb db, ChangeNotes notes, Set<PatchSet.Id> patchSetIds) throws OrmException {
if (!migration.readChanges()) {
patchSetIds = Sets.filter(patchSetIds, p -> p.getParentKey().equals(notes.getChangeId()));
return Streams.stream(db.patchSets().get(patchSetIds))
.sorted(PS_ID_ORDER)
.collect(toImmutableMap(PatchSet::getId, identity()));
}
return ImmutableMap.copyOf(Maps.filterKeys(notes.load().getPatchSets(), patchSetIds::contains));
}
public PatchSet insert(
ReviewDb db,
RevWalk rw,

39
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
View File

@@ -29,7 +29,6 @@ import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import com.google.auto.value.AutoValue;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
@@ -871,12 +870,9 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
toDel.addAll(drafts.values());
break;
case PUBLISH:
for (Comment e : drafts.values()) {
toPublish.add(publishComment(ctx, e, ps));
}
break;
case PUBLISH_ALL_REVISIONS:
publishAllRevisions(ctx, drafts, toPublish);
commentsUtil.publish(ctx, psId, drafts.values(), in.tag);
comments.addAll(drafts.values());
break;
}
ChangeUpdate u = ctx.getUpdate(psId);
@@ -1009,37 +1005,6 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
return labels;
}
private Comment publishComment(ChangeContext ctx, Comment c, PatchSet ps) throws OrmException {
c.writtenOn = ctx.getWhen();
c.tag = in.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(c::setRealAuthor);
setCommentRevId(c, patchListCache, ctx.getChange(), checkNotNull(ps));
return c;
}
private void publishAllRevisions(
ChangeContext ctx, Map<String, Comment> drafts, List<Comment> ups) throws OrmException {
boolean needOtherPatchSets = false;
for (Comment c : drafts.values()) {
if (c.key.patchSetId != psId.get()) {
needOtherPatchSets = true;
break;
}
}
Map<PatchSet.Id, PatchSet> patchSets =
needOtherPatchSets
? psUtil.byChangeAsMap(ctx.getDb(), ctx.getNotes())
: ImmutableMap.of(psId, ps);
for (Comment e : drafts.values()) {
ups.add(
publishComment(
ctx, e, patchSets.get(new PatchSet.Id(ctx.getChange().getId(), e.key.patchSetId))));
}
}
private Map<String, Short> getAllApprovals(
LabelTypes labelTypes, Map<String, Short> current, Map<String, Short> input) {
Map<String, Short> allApprovals = new HashMap<>();