Merge changes I5c65740d,I919cf96d,I8dc8281e,I770c7281,I3d98e55f, ...
* changes:
OutputStreamQuery: Clone PSA for currentPatchSet
OutputStreamQuery: Use AccountLoader for current patch set
OutputStreamQuery: Nest RevWalk using options under existing condition
OutputStreamQuery: Lazy load labelTypes
OutputStreamQuery: Add patchset comments when adding patchsets
OutputStreamQuery: Remove duplicate addPatchSets()
EventFactory: Re-use revwalk for loading ChangeKindCache
diff --git a/java/com/google/gerrit/server/data/PatchSetAttribute.java b/java/com/google/gerrit/server/data/PatchSetAttribute.java
index dc47057..3f7c8e4 100644
--- a/java/com/google/gerrit/server/data/PatchSetAttribute.java
+++ b/java/com/google/gerrit/server/data/PatchSetAttribute.java
@@ -17,7 +17,7 @@
import com.google.gerrit.extensions.client.ChangeKind;
import java.util.List;
-public class PatchSetAttribute {
+public class PatchSetAttribute implements Cloneable {
public int number;
public String revision;
public List<String> parents;
@@ -32,4 +32,12 @@
public List<PatchAttribute> files;
public int sizeInsertions;
public int sizeDeletions;
+
+ public PatchSetAttribute shallowClone() {
+ try {
+ return (PatchSetAttribute) super.clone();
+ } catch (CloneNotSupportedException e) {
+ throw new AssertionError(e);
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/events/EventFactory.java b/java/com/google/gerrit/server/events/EventFactory.java
index 2827f59..ac69120 100644
--- a/java/com/google/gerrit/server/events/EventFactory.java
+++ b/java/com/google/gerrit/server/events/EventFactory.java
@@ -76,6 +76,7 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -358,33 +359,23 @@
public void addPatchSets(
RevWalk revWalk,
+ Config repoConfig,
ChangeAttribute ca,
- Collection<PatchSet> ps,
- Map<PatchSet.Id, Collection<PatchSetApproval>> approvals,
- LabelTypes labelTypes,
- AccountAttributeLoader accountLoader) {
- addPatchSets(revWalk, ca, ps, approvals, false, null, labelTypes, accountLoader);
- }
-
- public void addPatchSets(
- RevWalk revWalk,
- ChangeAttribute ca,
- Collection<PatchSet> ps,
Map<PatchSet.Id, Collection<PatchSetApproval>> approvals,
boolean includeFiles,
- Change change,
- LabelTypes labelTypes,
+ ChangeData changeData,
AccountAttributeLoader accountLoader) {
- if (!ps.isEmpty()) {
- ca.patchSets = new ArrayList<>(ps.size());
- for (PatchSet p : ps) {
- PatchSetAttribute psa = asPatchSetAttribute(revWalk, change, p, accountLoader);
+ if (!changeData.patchSets().isEmpty()) {
+ ca.patchSets = new ArrayList<>(changeData.patchSets().size());
+ for (PatchSet p : changeData.patchSets()) {
+ PatchSetAttribute psa =
+ asPatchSetAttribute(revWalk, repoConfig, changeData, p, accountLoader);
if (approvals != null) {
- addApprovals(psa, p.id(), approvals, labelTypes, accountLoader);
+ addApprovals(psa, p.id(), approvals, changeData.getLabelTypes(), accountLoader);
}
ca.patchSets.add(psa);
if (includeFiles) {
- addPatchSetFileNames(psa, change, p);
+ addPatchSetFileNames(psa, changeData.change(), p);
}
}
}
@@ -441,13 +432,18 @@
}
}
- public PatchSetAttribute asPatchSetAttribute(RevWalk revWalk, Change change, PatchSet patchSet) {
- return asPatchSetAttribute(revWalk, change, patchSet, null);
+ public PatchSetAttribute asPatchSetAttribute(
+ RevWalk revWalk, Config repoConfig, ChangeData changeData, PatchSet patchSet) {
+ return asPatchSetAttribute(revWalk, repoConfig, changeData, patchSet, null);
}
/** Create a PatchSetAttribute for the given patchset suitable for serialization to JSON. */
public PatchSetAttribute asPatchSetAttribute(
- RevWalk revWalk, Change change, PatchSet patchSet, AccountAttributeLoader accountLoader) {
+ RevWalk revWalk,
+ Config repoConfig,
+ ChangeData changeData,
+ PatchSet patchSet,
+ AccountAttributeLoader accountLoader) {
PatchSetAttribute p = new PatchSetAttribute();
p.revision = patchSet.commitId().name();
p.number = patchSet.number();
@@ -474,12 +470,12 @@
Map<String, FileDiffOutput> modifiedFiles =
diffOperations.listModifiedFilesAgainstParent(
- change.getProject(), patchSet.commitId(), /* parentNum= */ 0, DiffOptions.DEFAULTS);
+ changeData.project(), patchSet.commitId(), /* parentNum= */ 0, DiffOptions.DEFAULTS);
for (FileDiffOutput fileDiff : modifiedFiles.values()) {
p.sizeDeletions += fileDiff.deletions();
p.sizeInsertions += fileDiff.insertions();
}
- p.kind = changeKindCache.getChangeKind(change, patchSet);
+ p.kind = changeKindCache.getChangeKind(revWalk, repoConfig, changeData, patchSet);
} catch (IOException | StorageException e) {
logger.atSevere().withCause(e).log("Cannot load patch set data for %s", patchSet.id());
} catch (DiffNotAvailableException e) {
diff --git a/java/com/google/gerrit/server/events/StreamEventsApiListener.java b/java/com/google/gerrit/server/events/StreamEventsApiListener.java
index 89aebde..66e894c 100644
--- a/java/com/google/gerrit/server/events/StreamEventsApiListener.java
+++ b/java/com/google/gerrit/server/events/StreamEventsApiListener.java
@@ -65,6 +65,7 @@
import com.google.gerrit.server.plugincontext.PluginItemContext;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -149,6 +150,7 @@
private final PatchSetUtil psUtil;
private final ChangeNotes.Factory changeNotesFactory;
private final boolean enableDraftCommentEvents;
+ private final ChangeData.Factory changeDataFactory;
private final String gerritInstanceId;
@@ -161,7 +163,8 @@
PatchSetUtil psUtil,
ChangeNotes.Factory changeNotesFactory,
@GerritServerConfig Config config,
- @Nullable @GerritInstanceId String gerritInstanceId) {
+ @Nullable @GerritInstanceId String gerritInstanceId,
+ ChangeData.Factory changeDataFactory) {
this.dispatcher = dispatcher;
this.eventFactory = eventFactory;
this.projectCache = projectCache;
@@ -171,6 +174,7 @@
this.enableDraftCommentEvents =
config.getBoolean("event", "stream-events", "enableDraftCommentEvents", false);
this.gerritInstanceId = gerritInstanceId;
+ this.changeDataFactory = changeDataFactory;
}
private ChangeNotes getNotes(ChangeInfo info) {
@@ -206,12 +210,13 @@
}
private Supplier<PatchSetAttribute> patchSetAttributeSupplier(
- final Change change, PatchSet patchSet) {
+ final ChangeData changeData, PatchSet patchSet) {
return Suppliers.memoize(
() -> {
- try (Repository repo = repoManager.openRepository(change.getProject());
+ try (Repository repo = repoManager.openRepository(changeData.change().getProject());
RevWalk revWalk = new RevWalk(repo)) {
- return eventFactory.asPatchSetAttribute(revWalk, change, patchSet);
+ return eventFactory.asPatchSetAttribute(
+ revWalk, repo.getConfig(), changeData, patchSet);
} catch (IOException e) {
throw new RuntimeException(e);
}
@@ -301,7 +306,7 @@
PatchSetCreatedEvent event = new PatchSetCreatedEvent(change);
event.change = changeAttributeSupplier(change, notes);
- event.patchSet = patchSetAttributeSupplier(change, patchSet);
+ event.patchSet = patchSetAttributeSupplier(changeDataFactory.create(notes), patchSet);
event.uploader = accountAttributeSupplier(ev.getWho());
dispatcher.run(d -> d.postEvent(change, event));
@@ -317,7 +322,8 @@
Change change = notes.getChange();
ReviewerDeletedEvent event = new ReviewerDeletedEvent(change);
event.change = changeAttributeSupplier(change, notes);
- event.patchSet = patchSetAttributeSupplier(change, psUtil.current(notes));
+ event.patchSet =
+ patchSetAttributeSupplier(changeDataFactory.create(notes), psUtil.current(notes));
event.reviewer = accountAttributeSupplier(ev.getReviewer());
event.remover = accountAttributeSupplier(ev.getWho());
event.comment = ev.getComment();
@@ -338,7 +344,8 @@
ReviewerAddedEvent event = new ReviewerAddedEvent(change);
event.change = changeAttributeSupplier(change, notes);
- event.patchSet = patchSetAttributeSupplier(change, psUtil.current(notes));
+ event.patchSet =
+ patchSetAttributeSupplier(changeDataFactory.create(notes), psUtil.current(notes));
event.adder = accountAttributeSupplier(ev.getWho());
for (AccountInfo reviewer : ev.getReviewers()) {
event.reviewer = accountAttributeSupplier(reviewer);
@@ -466,7 +473,7 @@
event.change = changeAttributeSupplier(change, notes);
event.author = accountAttributeSupplier(ev.getWho());
- event.patchSet = patchSetAttributeSupplier(change, ps);
+ event.patchSet = patchSetAttributeSupplier(changeDataFactory.create(notes), ps);
event.comment = ev.getComment();
event.approvals = approvalsAttributeSupplier(change, ev.getApprovals(), ev.getOldApprovals());
@@ -485,7 +492,8 @@
event.change = changeAttributeSupplier(change, notes);
event.restorer = accountAttributeSupplier(ev.getWho());
- event.patchSet = patchSetAttributeSupplier(change, psUtil.current(notes));
+ event.patchSet =
+ patchSetAttributeSupplier(changeDataFactory.create(notes), psUtil.current(notes));
event.reason = ev.getReason();
dispatcher.run(d -> d.postEvent(change, event));
@@ -503,7 +511,8 @@
event.change = changeAttributeSupplier(change, notes);
event.submitter = accountAttributeSupplier(ev.getWho());
- event.patchSet = patchSetAttributeSupplier(change, psUtil.current(notes));
+ event.patchSet =
+ patchSetAttributeSupplier(changeDataFactory.create(notes), psUtil.current(notes));
event.newRev = ev.getNewRevisionId();
dispatcher.run(d -> d.postEvent(change, event));
@@ -521,7 +530,8 @@
event.change = changeAttributeSupplier(change, notes);
event.abandoner = accountAttributeSupplier(ev.getWho());
- event.patchSet = patchSetAttributeSupplier(change, psUtil.current(notes));
+ event.patchSet =
+ patchSetAttributeSupplier(changeDataFactory.create(notes), psUtil.current(notes));
event.reason = ev.getReason();
dispatcher.run(d -> d.postEvent(change, event));
@@ -540,7 +550,7 @@
event.change = changeAttributeSupplier(change, notes);
event.changer = accountAttributeSupplier(ev.getWho());
- event.patchSet = patchSetAttributeSupplier(change, patchSet);
+ event.patchSet = patchSetAttributeSupplier(changeDataFactory.create(notes), patchSet);
dispatcher.run(d -> d.postEvent(change, event));
} catch (StorageException e) {
@@ -558,7 +568,7 @@
event.change = changeAttributeSupplier(change, notes);
event.changer = accountAttributeSupplier(ev.getWho());
- event.patchSet = patchSetAttributeSupplier(change, patchSet);
+ event.patchSet = patchSetAttributeSupplier(changeDataFactory.create(notes), patchSet);
dispatcher.run(d -> d.postEvent(change, event));
} catch (StorageException e) {
@@ -574,7 +584,8 @@
VoteDeletedEvent event = new VoteDeletedEvent(change);
event.change = changeAttributeSupplier(change, notes);
- event.patchSet = patchSetAttributeSupplier(change, psUtil.current(notes));
+ event.patchSet =
+ patchSetAttributeSupplier(changeDataFactory.create(notes), psUtil.current(notes));
event.comment = ev.getMessage();
event.reviewer = accountAttributeSupplier(ev.getReviewer());
event.remover = accountAttributeSupplier(ev.getWho());
diff --git a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
index d3b5605..0fd9c0e 100644
--- a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
+++ b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
@@ -22,7 +22,6 @@
import com.google.common.collect.Lists;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.LabelTypes;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
@@ -262,7 +261,6 @@
Map<Project.NameKey, RevWalk> revWalks,
AccountAttributeLoader accountLoader)
throws IOException {
- LabelTypes labelTypes = d.getLabelTypes();
ChangeAttribute c = eventFactory.asChangeAttribute(d.change(), accountLoader);
c.hashtags = Lists.newArrayList(d.hashtags());
eventFactory.extend(c, d.change());
@@ -286,67 +284,71 @@
eventFactory.addCommitMessage(c, d.commitMessage());
}
- RevWalk rw = null;
if (includePatchSets || includeCurrentPatchSet || includeDependencies) {
Project.NameKey p = d.change().getProject();
- rw = revWalks.get(p);
+ Repository repo;
+ RevWalk rw = revWalks.get(p);
// Cache and reuse repos and revwalks.
if (rw == null) {
- Repository repo = repoManager.openRepository(p);
+ repo = repoManager.openRepository(p);
checkState(repos.put(p, repo) == null);
rw = new RevWalk(repo);
revWalks.put(p, rw);
+ } else {
+ repo = repos.get(p);
}
- }
- if (includePatchSets) {
- eventFactory.addPatchSets(
- rw,
- c,
- d.patchSets(),
- includeApprovals ? d.conditionallyLoadApprovalsWithCopied().asMap() : null,
- includeFiles,
- d.change(),
- labelTypes,
- accountLoader);
- }
-
- if (includeCurrentPatchSet) {
- PatchSet current = d.currentPatchSet();
- if (current != null) {
- c.currentPatchSet = eventFactory.asPatchSetAttribute(rw, d.change(), current);
- eventFactory.addApprovals(
- c.currentPatchSet, d.currentApprovals(), labelTypes, accountLoader);
-
- if (includeFiles) {
- eventFactory.addPatchSetFileNames(c.currentPatchSet, d.change(), d.currentPatchSet());
- }
+ if (includePatchSets) {
+ eventFactory.addPatchSets(
+ rw,
+ repo.getConfig(),
+ c,
+ includeApprovals ? d.conditionallyLoadApprovalsWithCopied().asMap() : null,
+ includeFiles,
+ d,
+ accountLoader);
if (includeComments) {
- eventFactory.addPatchSetComments(c.currentPatchSet, d.publishedComments(), accountLoader);
+ for (PatchSetAttribute attribute : c.patchSets) {
+ eventFactory.addPatchSetComments(attribute, d.publishedComments(), accountLoader);
+ }
}
}
+
+ if (includeCurrentPatchSet) {
+ PatchSet current = d.currentPatchSet();
+ if (current != null) {
+ if (includePatchSets) {
+ for (PatchSetAttribute attribute : c.patchSets) {
+ if (attribute.number == current.number()) {
+ c.currentPatchSet = attribute.shallowClone();
+ // approvals will be populated later using different logic than --patch-sets uses
+ c.currentPatchSet.approvals = null;
+ break;
+ }
+ }
+ } else {
+ c.currentPatchSet =
+ eventFactory.asPatchSetAttribute(rw, repo.getConfig(), d, current, accountLoader);
+ if (includeFiles) {
+ eventFactory.addPatchSetFileNames(c.currentPatchSet, d.change(), d.currentPatchSet());
+ }
+ if (includeComments) {
+ eventFactory.addPatchSetComments(
+ c.currentPatchSet, d.publishedComments(), accountLoader);
+ }
+ }
+ eventFactory.addApprovals(
+ c.currentPatchSet, d.currentApprovals(), d.getLabelTypes(), accountLoader);
+ }
+ }
+
+ if (includeDependencies) {
+ eventFactory.addDependencies(rw, c, d.change(), d.currentPatchSet());
+ }
}
if (includeComments) {
eventFactory.addComments(c, d.messages(), accountLoader);
- if (includePatchSets) {
- eventFactory.addPatchSets(
- rw,
- c,
- d.patchSets(),
- includeApprovals ? d.approvals().asMap() : null,
- includeFiles,
- d.change(),
- labelTypes,
- accountLoader);
- for (PatchSetAttribute attribute : c.patchSets) {
- eventFactory.addPatchSetComments(attribute, d.publishedComments(), accountLoader);
- }
- }
- }
-
- if (includeDependencies) {
- eventFactory.addDependencies(rw, c, d.change(), d.currentPatchSet());
}
ImmutableList<PluginDefinedInfo> pluginInfos = pluginInfosByChange.get(d.getId());
