From 3ce9d7e6f4c58b79ddf85a29fcf03948a2e61a09 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Mon, 15 Feb 2016 14:55:10 +0100 Subject: [PATCH] Where available pass ChangeNotes (instead of Change) into ChangeData The callers that have already loaded the change notes should pass them into ChangeData so that later they don't need to be loaded again. Change-Id: I1c2fbda87be4974b26cc5c7202b174c3057451e9 Signed-off-by: Edwin Kempin --- .../gerrit/server/change/ChangeJson.java | 6 +-- .../gerrit/server/change/ChangeKindCache.java | 4 +- .../server/change/ChangeKindCacheImpl.java | 21 +++++----- .../gerrit/server/events/EventFactory.java | 2 +- .../gerrit/server/index/SiteIndexer.java | 6 +-- .../server/query/change/ChangeData.java | 38 +++++++++++++++++++ .../query/change/InternalChangeQuery.java | 2 +- 7 files changed, 58 insertions(+), 21 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index 3fdca9d7bb..0f68ee27d4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java @@ -225,16 +225,16 @@ public class ChangeJson { public ChangeInfo format(Project.NameKey project, Change.Id id) throws OrmException, NoSuchChangeException { - Change c; + ChangeNotes notes; try { - c = notesFactory.createChecked(db.get(), project, id).getChange(); + notes = notesFactory.createChecked(db.get(), project, id); } catch (OrmException | NoSuchChangeException e) { if (!has(CHECK)) { throw e; } return checkOnly(changeDataFactory.create(db.get(), project, id)); } - return format(changeDataFactory.create(db.get(), c)); + return format(changeDataFactory.create(db.get(), notes)); } public ChangeInfo format(ChangeData cd) throws OrmException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCache.java index 5148f9ac56..b8f7fbca4f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCache.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCache.java @@ -14,9 +14,9 @@ package com.google.gerrit.server.change; -import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.ProjectState; import org.eclipse.jgit.lib.ObjectId; @@ -32,5 +32,5 @@ public interface ChangeKindCache { ChangeKind getChangeKind(ProjectState project, Repository repo, ObjectId prior, ObjectId next); - ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch); + ChangeKind getChangeKind(ReviewDb db, ChangeNotes notes, PatchSet patch); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java index 24e0f91d4d..8843004622 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java @@ -22,13 +22,13 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.cache.Cache; import com.google.common.cache.Weigher; import com.google.common.collect.FluentIterable; -import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.cache.CacheModule; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MergeUtil; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.query.change.ChangeData; @@ -110,8 +110,9 @@ public class ChangeKindCacheImpl implements ChangeKindCache { } @Override - public ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch) { - return getChangeKindInternal(this, db, change, patch, changeDataFactory, + public ChangeKind getChangeKind(ReviewDb db, ChangeNotes notes, + PatchSet patch) { + return getChangeKindInternal(this, db, notes, patch, changeDataFactory, projectCache, repoManager); } } @@ -326,15 +327,15 @@ public class ChangeKindCacheImpl implements ChangeKindCache { } @Override - public ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch) { - return getChangeKindInternal(this, db, change, patch, changeDataFactory, + public ChangeKind getChangeKind(ReviewDb db, ChangeNotes notes, PatchSet patch) { + return getChangeKindInternal(this, db, notes, patch, changeDataFactory, projectCache, repoManager); } private static ChangeKind getChangeKindInternal( ChangeKindCache cache, ReviewDb db, - Change change, + ChangeNotes notes, PatchSet patch, ChangeData.Factory changeDataFactory, ProjectCache projectCache, @@ -344,9 +345,9 @@ public class ChangeKindCacheImpl implements ChangeKindCache { // Trivial case: if we're on the first patch, we don't need to open // the repository. if (patch.getId().get() > 1) { - try (Repository repo = repoManager.openRepository(change.getProject())) { - ProjectState projectState = projectCache.checkedGet(change.getProject()); - ChangeData cd = changeDataFactory.create(db, change); + try (Repository repo = repoManager.openRepository(notes.getProjectName())) { + ProjectState projectState = projectCache.checkedGet(notes.getProjectName()); + ChangeData cd = changeDataFactory.create(db, notes); Collection patchSetCollection = cd.patchSets(); PatchSet priorPs = patch; for (PatchSet ps : patchSetCollection) { @@ -371,7 +372,7 @@ public class ChangeKindCacheImpl implements ChangeKindCache { } catch (IOException | OrmException e) { // Do nothing; assume we have a complex change log.warn("Unable to get change kind for patchSet " + patch.getPatchSetId() + - "of change " + change.getChangeId(), e); + "of change " + notes.getChangeId(), e); } } return kind; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java index 50d35c0969..d5cb04e0df 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java @@ -506,7 +506,7 @@ public class EventFactory { p.sizeInsertions += pe.getInsertions(); } } - p.kind = changeKindCache.getChangeKind(db, change, patchSet); + p.kind = changeKindCache.getChangeKind(db, notes, patchSet); } catch (IOException e) { log.error("Cannot load patch set data for " + patchSet.getId(), e); } catch (PatchSetInfoNotAvailableException e) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java index 3b03901532..a6cb12f1e6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java @@ -29,7 +29,6 @@ import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; -import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.config.GerritServerConfig; @@ -253,10 +252,9 @@ public class SiteIndexer { ReviewDb db = schemaFactory.open()) { Map refs = repo.getRefDatabase().getRefs(ALL); for (ChangeNotes cn : notesFactory.scan(repo, db, project)) { - Change c = cn.getChange(); - Ref r = refs.get(c.currentPatchSetId().toRefName()); + Ref r = refs.get(cn.getChange().currentPatchSetId().toRefName()); if (r != null) { - byId.put(r.getObjectId(), changeDataFactory.create(db, c)); + byId.put(r.getObjectId(), changeDataFactory.create(db, cn)); } } new ProjectIndexer(indexer, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java index 3844608e55..aa12b91854 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java @@ -276,6 +276,7 @@ public class ChangeData { public interface Factory { ChangeData create(ReviewDb db, Project.NameKey project, Change.Id id); ChangeData create(ReviewDb db, Change c); + ChangeData create(ReviewDb db, ChangeNotes cn); ChangeData create(ReviewDb db, ChangeControl c); // TODO(dborowitz): Remove when deleting index schemas <27. @@ -411,6 +412,43 @@ public class ChangeData { project = c.getProject(); } + @AssistedInject + private ChangeData( + GitRepositoryManager repoManager, + ChangeControl.GenericFactory changeControlFactory, + IdentifiedUser.GenericFactory userFactory, + ProjectCache projectCache, + MergeUtil.Factory mergeUtilFactory, + ChangeNotes.Factory notesFactory, + ApprovalsUtil approvalsUtil, + ChangeMessagesUtil cmUtil, + PatchLineCommentsUtil plcUtil, + PatchSetUtil psUtil, + PatchListCache patchListCache, + NotesMigration notesMigration, + MergeabilityCache mergeabilityCache, + @Assisted ReviewDb db, + @Assisted ChangeNotes cn) { + this.db = db; + this.repoManager = repoManager; + this.changeControlFactory = changeControlFactory; + this.userFactory = userFactory; + this.projectCache = projectCache; + this.mergeUtilFactory = mergeUtilFactory; + this.notesFactory = notesFactory; + this.approvalsUtil = approvalsUtil; + this.cmUtil = cmUtil; + this.plcUtil = plcUtil; + this.psUtil = psUtil; + this.patchListCache = patchListCache; + this.notesMigration = notesMigration; + this.mergeabilityCache = mergeabilityCache; + legacyId = cn.getChangeId(); + change = cn.getChange(); + project = cn.getProjectName(); + notes = cn; + } + @AssistedInject private ChangeData( GitRepositoryManager repoManager, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java index 9ff1d267b1..d02e58b3f3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java @@ -215,7 +215,7 @@ public class InternalChangeQuery { }), new Function() { @Override public ChangeData apply(ChangeNotes notes) { - return changeDataFactory.create(db, notes.getChange()); + return changeDataFactory.create(db, notes); } }); }