EventFactory: Re-use revwalk for loading ChangeKindCache
We may have already parsed the commit with the already opened RevWalk or
we will later parse the commit with it. Either way, we can avoid
duplication of effort by passing it through to this cache.
This also skips attempting to re-open the repository and uses the
repoConfig every caller already has access to.
Release-Notes: skip
Change-Id: Ib6a5a6a44d64dab7b6da7d96c9b2a9b4500a26a2
diff --git a/java/com/google/gerrit/server/events/EventFactory.java b/java/com/google/gerrit/server/events/EventFactory.java
index 2827f59..c6c5617 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,24 @@
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,
+ ChangeData changeData,
LabelTypes labelTypes,
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);
}
ca.patchSets.add(psa);
if (includeFiles) {
- addPatchSetFileNames(psa, change, p);
+ addPatchSetFileNames(psa, changeData.change(), p);
}
}
}
@@ -441,13 +433,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 +471,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..ed24298 100644
--- a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
+++ b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
@@ -287,12 +287,13 @@
}
RevWalk rw = null;
+ Repository repo = null;
if (includePatchSets || includeCurrentPatchSet || includeDependencies) {
Project.NameKey p = d.change().getProject();
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);
@@ -302,11 +303,11 @@
if (includePatchSets) {
eventFactory.addPatchSets(
rw,
+ repo != null ? repo.getConfig() : repos.get(d.change().getProject()).getConfig(),
c,
- d.patchSets(),
includeApprovals ? d.conditionallyLoadApprovalsWithCopied().asMap() : null,
includeFiles,
- d.change(),
+ d,
labelTypes,
accountLoader);
}
@@ -314,7 +315,12 @@
if (includeCurrentPatchSet) {
PatchSet current = d.currentPatchSet();
if (current != null) {
- c.currentPatchSet = eventFactory.asPatchSetAttribute(rw, d.change(), current);
+ c.currentPatchSet =
+ eventFactory.asPatchSetAttribute(
+ rw,
+ repo != null ? repo.getConfig() : repos.get(d.change().getProject()).getConfig(),
+ d,
+ current);
eventFactory.addApprovals(
c.currentPatchSet, d.currentApprovals(), labelTypes, accountLoader);
@@ -332,11 +338,11 @@
if (includePatchSets) {
eventFactory.addPatchSets(
rw,
+ repo != null ? repo.getConfig() : repos.get(d.change().getProject()).getConfig(),
c,
- d.patchSets(),
includeApprovals ? d.approvals().asMap() : null,
includeFiles,
- d.change(),
+ d,
labelTypes,
accountLoader);
for (PatchSetAttribute attribute : c.patchSets) {
