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 <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-02-15 14:55:10 +01:00
parent 7c69516b8d
commit 3ce9d7e6f4
7 changed files with 58 additions and 21 deletions

View File

@@ -225,16 +225,16 @@ public class ChangeJson {
public ChangeInfo format(Project.NameKey project, Change.Id id) public ChangeInfo format(Project.NameKey project, Change.Id id)
throws OrmException, NoSuchChangeException { throws OrmException, NoSuchChangeException {
Change c; ChangeNotes notes;
try { try {
c = notesFactory.createChecked(db.get(), project, id).getChange(); notes = notesFactory.createChecked(db.get(), project, id);
} catch (OrmException | NoSuchChangeException e) { } catch (OrmException | NoSuchChangeException e) {
if (!has(CHECK)) { if (!has(CHECK)) {
throw e; throw e;
} }
return checkOnly(changeDataFactory.create(db.get(), project, id)); 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 { public ChangeInfo format(ChangeData cd) throws OrmException {

View File

@@ -14,9 +14,9 @@
package com.google.gerrit.server.change; 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.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
@@ -32,5 +32,5 @@ public interface ChangeKindCache {
ChangeKind getChangeKind(ProjectState project, Repository repo, ChangeKind getChangeKind(ProjectState project, Repository repo,
ObjectId prior, ObjectId next); ObjectId prior, ObjectId next);
ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch); ChangeKind getChangeKind(ReviewDb db, ChangeNotes notes, PatchSet patch);
} }

View File

@@ -22,13 +22,13 @@ import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.Cache; import com.google.common.cache.Cache;
import com.google.common.cache.Weigher; import com.google.common.cache.Weigher;
import com.google.common.collect.FluentIterable; 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.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.cache.CacheModule; import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil; 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.ProjectCache;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
@@ -110,8 +110,9 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
} }
@Override @Override
public ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch) { public ChangeKind getChangeKind(ReviewDb db, ChangeNotes notes,
return getChangeKindInternal(this, db, change, patch, changeDataFactory, PatchSet patch) {
return getChangeKindInternal(this, db, notes, patch, changeDataFactory,
projectCache, repoManager); projectCache, repoManager);
} }
} }
@@ -326,15 +327,15 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
} }
@Override @Override
public ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch) { public ChangeKind getChangeKind(ReviewDb db, ChangeNotes notes, PatchSet patch) {
return getChangeKindInternal(this, db, change, patch, changeDataFactory, return getChangeKindInternal(this, db, notes, patch, changeDataFactory,
projectCache, repoManager); projectCache, repoManager);
} }
private static ChangeKind getChangeKindInternal( private static ChangeKind getChangeKindInternal(
ChangeKindCache cache, ChangeKindCache cache,
ReviewDb db, ReviewDb db,
Change change, ChangeNotes notes,
PatchSet patch, PatchSet patch,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
ProjectCache projectCache, 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 // Trivial case: if we're on the first patch, we don't need to open
// the repository. // the repository.
if (patch.getId().get() > 1) { if (patch.getId().get() > 1) {
try (Repository repo = repoManager.openRepository(change.getProject())) { try (Repository repo = repoManager.openRepository(notes.getProjectName())) {
ProjectState projectState = projectCache.checkedGet(change.getProject()); ProjectState projectState = projectCache.checkedGet(notes.getProjectName());
ChangeData cd = changeDataFactory.create(db, change); ChangeData cd = changeDataFactory.create(db, notes);
Collection<PatchSet> patchSetCollection = cd.patchSets(); Collection<PatchSet> patchSetCollection = cd.patchSets();
PatchSet priorPs = patch; PatchSet priorPs = patch;
for (PatchSet ps : patchSetCollection) { for (PatchSet ps : patchSetCollection) {
@@ -371,7 +372,7 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
} catch (IOException | OrmException e) { } catch (IOException | OrmException e) {
// Do nothing; assume we have a complex change // Do nothing; assume we have a complex change
log.warn("Unable to get change kind for patchSet " + patch.getPatchSetId() + log.warn("Unable to get change kind for patchSet " + patch.getPatchSetId() +
"of change " + change.getChangeId(), e); "of change " + notes.getChangeId(), e);
} }
} }
return kind; return kind;

View File

@@ -506,7 +506,7 @@ public class EventFactory {
p.sizeInsertions += pe.getInsertions(); p.sizeInsertions += pe.getInsertions();
} }
} }
p.kind = changeKindCache.getChangeKind(db, change, patchSet); p.kind = changeKindCache.getChangeKind(db, notes, patchSet);
} catch (IOException e) { } catch (IOException e) {
log.error("Cannot load patch set data for " + patchSet.getId(), e); log.error("Cannot load patch set data for " + patchSet.getId(), e);
} catch (PatchSetInfoNotAvailableException e) { } catch (PatchSetInfoNotAvailableException e) {

View File

@@ -29,7 +29,6 @@ import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors; 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.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.GerritServerConfig;
@@ -253,10 +252,9 @@ public class SiteIndexer {
ReviewDb db = schemaFactory.open()) { ReviewDb db = schemaFactory.open()) {
Map<String, Ref> refs = repo.getRefDatabase().getRefs(ALL); Map<String, Ref> refs = repo.getRefDatabase().getRefs(ALL);
for (ChangeNotes cn : notesFactory.scan(repo, db, project)) { for (ChangeNotes cn : notesFactory.scan(repo, db, project)) {
Change c = cn.getChange(); Ref r = refs.get(cn.getChange().currentPatchSetId().toRefName());
Ref r = refs.get(c.currentPatchSetId().toRefName());
if (r != null) { if (r != null) {
byId.put(r.getObjectId(), changeDataFactory.create(db, c)); byId.put(r.getObjectId(), changeDataFactory.create(db, cn));
} }
} }
new ProjectIndexer(indexer, new ProjectIndexer(indexer,

View File

@@ -276,6 +276,7 @@ public class ChangeData {
public interface Factory { public interface Factory {
ChangeData create(ReviewDb db, Project.NameKey project, Change.Id id); ChangeData create(ReviewDb db, Project.NameKey project, Change.Id id);
ChangeData create(ReviewDb db, Change c); ChangeData create(ReviewDb db, Change c);
ChangeData create(ReviewDb db, ChangeNotes cn);
ChangeData create(ReviewDb db, ChangeControl c); ChangeData create(ReviewDb db, ChangeControl c);
// TODO(dborowitz): Remove when deleting index schemas <27. // TODO(dborowitz): Remove when deleting index schemas <27.
@@ -411,6 +412,43 @@ public class ChangeData {
project = c.getProject(); 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 @AssistedInject
private ChangeData( private ChangeData(
GitRepositoryManager repoManager, GitRepositoryManager repoManager,

View File

@@ -215,7 +215,7 @@ public class InternalChangeQuery {
}), new Function<ChangeNotes, ChangeData>() { }), new Function<ChangeNotes, ChangeData>() {
@Override @Override
public ChangeData apply(ChangeNotes notes) { public ChangeData apply(ChangeNotes notes) {
return changeDataFactory.create(db, notes.getChange()); return changeDataFactory.create(db, notes);
} }
}); });
} }