ChangeKindCache: Take Project.NameKey instead of ProjectState

We used to read useContentMerge from the project settings, but this
was removed in I7ef9bda0. Using Project.NameKey instead of
ProjectState simplifies many callers, who no longer need to read from
the ProjectCache. It also makes it easier to write a fake
implementation of ChangeKindCacheImpl for small tests, since mocking
ProjectState is nontrivial.

Change-Id: If4e30db6d905e174f2b64d2d52ea2266de846445
This commit is contained in:
Dave Borowitz
2016-10-04 11:43:46 -04:00
parent 24406859c3
commit 424b0c680f
5 changed files with 22 additions and 42 deletions

View File

@@ -161,7 +161,8 @@ public class ApprovalCopier {
continue; continue;
} }
ChangeKind kind = changeKindCache.getChangeKind(project, repo, ChangeKind kind = changeKindCache.getChangeKind(
project.getProject().getNameKey(), repo,
ObjectId.fromString(priorPs.getRevision().get()), ObjectId.fromString(priorPs.getRevision().get()),
ObjectId.fromString(ps.getRevision().get())); ObjectId.fromString(ps.getRevision().get()));

View File

@@ -18,8 +18,8 @@ import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.ChangeKind; import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.reviewdb.client.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.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
@@ -32,7 +32,7 @@ import org.eclipse.jgit.lib.Repository;
* implementation changes, which might invalidate old entries). * implementation changes, which might invalidate old entries).
*/ */
public interface ChangeKindCache { public interface ChangeKindCache {
ChangeKind getChangeKind(ProjectState project, @Nullable Repository repo, ChangeKind getChangeKind(Project.NameKey project, @Nullable Repository repo,
ObjectId prior, ObjectId next); ObjectId prior, ObjectId next);
ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch); ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch);

View File

@@ -33,8 +33,6 @@ 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.InMemoryInserter; import com.google.gerrit.server.git.InMemoryInserter;
import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -85,7 +83,6 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
public static class NoCache implements ChangeKindCache { public static class NoCache implements ChangeKindCache {
private final boolean useRecursiveMerge; private final boolean useRecursiveMerge;
private final ChangeData.Factory changeDataFactory; private final ChangeData.Factory changeDataFactory;
private final ProjectCache projectCache;
private final GitRepositoryManager repoManager; private final GitRepositoryManager repoManager;
@@ -93,25 +90,21 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
NoCache( NoCache(
@GerritServerConfig Config serverConfig, @GerritServerConfig Config serverConfig,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
ProjectCache projectCache,
GitRepositoryManager repoManager) { GitRepositoryManager repoManager) {
this.useRecursiveMerge = MergeUtil.useRecursiveMerge(serverConfig); this.useRecursiveMerge = MergeUtil.useRecursiveMerge(serverConfig);
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.projectCache = projectCache;
this.repoManager = repoManager; this.repoManager = repoManager;
} }
@Override @Override
public ChangeKind getChangeKind(ProjectState project, public ChangeKind getChangeKind(Project.NameKey project,
@Nullable Repository repo, ObjectId prior, ObjectId next) { @Nullable Repository repo, ObjectId prior, ObjectId next) {
try { try {
Key key = new Key(prior, next, useRecursiveMerge); Key key = new Key(prior, next, useRecursiveMerge);
return new Loader( return new Loader(key, repoManager, project, repo).call();
key, repoManager, project.getProject().getNameKey(), repo)
.call();
} catch (IOException e) { } catch (IOException e) {
log.warn("Cannot check trivial rebase of new patch set " + next.name() log.warn("Cannot check trivial rebase of new patch set " + next.name()
+ " in " + project.getProject().getName(), e); + " in " + project, e);
return ChangeKind.REWORK; return ChangeKind.REWORK;
} }
} }
@@ -120,13 +113,13 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
public ChangeKind getChangeKind(ReviewDb db, Change change, public ChangeKind getChangeKind(ReviewDb db, Change change,
PatchSet patch) { PatchSet patch) {
return getChangeKindInternal(this, db, change, patch, changeDataFactory, return getChangeKindInternal(this, db, change, patch, changeDataFactory,
projectCache, repoManager); repoManager);
} }
@Override @Override
public ChangeKind getChangeKind(@Nullable Repository repo, ChangeData cd, public ChangeKind getChangeKind(@Nullable Repository repo, ChangeData cd,
PatchSet patch) { PatchSet patch) {
return getChangeKindInternal(this, repo, cd, patch, projectCache); return getChangeKindInternal(this, repo, cd, patch);
} }
} }
@@ -322,7 +315,6 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
private final Cache<Key, ChangeKind> cache; private final Cache<Key, ChangeKind> cache;
private final boolean useRecursiveMerge; private final boolean useRecursiveMerge;
private final ChangeData.Factory changeDataFactory; private final ChangeData.Factory changeDataFactory;
private final ProjectCache projectCache;
private final GitRepositoryManager repoManager; private final GitRepositoryManager repoManager;
@Inject @Inject
@@ -330,27 +322,22 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
@GerritServerConfig Config serverConfig, @GerritServerConfig Config serverConfig,
@Named(ID_CACHE) Cache<Key, ChangeKind> cache, @Named(ID_CACHE) Cache<Key, ChangeKind> cache,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
ProjectCache projectCache,
GitRepositoryManager repoManager) { GitRepositoryManager repoManager) {
this.cache = cache; this.cache = cache;
this.useRecursiveMerge = MergeUtil.useRecursiveMerge(serverConfig); this.useRecursiveMerge = MergeUtil.useRecursiveMerge(serverConfig);
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.projectCache = projectCache;
this.repoManager = repoManager; this.repoManager = repoManager;
} }
@Override @Override
public ChangeKind getChangeKind(ProjectState project, public ChangeKind getChangeKind(Project.NameKey project,
@Nullable Repository repo, ObjectId prior, ObjectId next) { @Nullable Repository repo, ObjectId prior, ObjectId next) {
try { try {
Key key = new Key(prior, next, useRecursiveMerge); Key key = new Key(prior, next, useRecursiveMerge);
return cache.get( return cache.get(key, new Loader(key, repoManager, project, repo));
key,
new Loader(
key, repoManager, project.getProject().getNameKey(), repo));
} catch (ExecutionException e) { } catch (ExecutionException e) {
log.warn("Cannot check trivial rebase of new patch set " + next.name() log.warn("Cannot check trivial rebase of new patch set " + next.name()
+ " in " + project.getProject().getName(), e); + " in " + project, e);
return ChangeKind.REWORK; return ChangeKind.REWORK;
} }
} }
@@ -358,27 +345,25 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
@Override @Override
public ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch) { public ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch) {
return getChangeKindInternal(this, db, change, patch, changeDataFactory, return getChangeKindInternal(this, db, change, patch, changeDataFactory,
projectCache, repoManager); repoManager);
} }
@Override @Override
public ChangeKind getChangeKind(@Nullable Repository repo, ChangeData cd, public ChangeKind getChangeKind(@Nullable Repository repo, ChangeData cd,
PatchSet patch) { PatchSet patch) {
return getChangeKindInternal(this, repo, cd, patch, projectCache); return getChangeKindInternal(this, repo, cd, patch);
} }
private static ChangeKind getChangeKindInternal( private static ChangeKind getChangeKindInternal(
ChangeKindCache cache, ChangeKindCache cache,
@Nullable Repository repo, @Nullable Repository repo,
ChangeData change, ChangeData change,
PatchSet patch, PatchSet patch) {
ProjectCache projectCache) {
ChangeKind kind = ChangeKind.REWORK; ChangeKind kind = ChangeKind.REWORK;
// Trivial case: if we're on the first patch, we don't need to use // Trivial case: if we're on the first patch, we don't need to use
// the repository. // the repository.
if (patch.getId().get() > 1) { if (patch.getId().get() > 1) {
try { try {
ProjectState projectState = projectCache.checkedGet(change.project());
Collection<PatchSet> patchSetCollection = change.patchSets(); Collection<PatchSet> patchSetCollection = change.patchSets();
PatchSet priorPs = patch; PatchSet priorPs = patch;
for (PatchSet ps : patchSetCollection) { for (PatchSet ps : patchSetCollection) {
@@ -396,11 +381,11 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
// and deletes the draft. // and deletes the draft.
if (priorPs != patch) { if (priorPs != patch) {
kind = kind =
cache.getChangeKind(projectState, repo, cache.getChangeKind(change.project(), repo,
ObjectId.fromString(priorPs.getRevision().get()), ObjectId.fromString(priorPs.getRevision().get()),
ObjectId.fromString(patch.getRevision().get())); ObjectId.fromString(patch.getRevision().get()));
} }
} catch (IOException | OrmException e) { } catch (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.getId(), e); "of change " + change.getId(), e);
@@ -415,7 +400,6 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
Change change, Change change,
PatchSet patch, PatchSet patch,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
ProjectCache projectCache,
GitRepositoryManager repoManager) { GitRepositoryManager repoManager) {
// TODO - dborowitz: add NEW_CHANGE type for default. // TODO - dborowitz: add NEW_CHANGE type for default.
ChangeKind kind = ChangeKind.REWORK; ChangeKind kind = ChangeKind.REWORK;
@@ -424,8 +408,7 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
if (patch.getId().get() > 1) { if (patch.getId().get() > 1) {
try (Repository repo = repoManager.openRepository(change.getProject())) { try (Repository repo = repoManager.openRepository(change.getProject())) {
kind = getChangeKindInternal(cache, repo, kind = getChangeKindInternal(cache, repo,
changeDataFactory.create(db, change), patch, changeDataFactory.create(db, change), patch);
projectCache);
} catch (IOException e) { } catch (IOException 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() +

View File

@@ -41,8 +41,6 @@ import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.index.change.ChangeIndexer; import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -71,7 +69,6 @@ public class ChangeEditUtil {
private final PatchSetInserter.Factory patchSetInserterFactory; private final PatchSetInserter.Factory patchSetInserterFactory;
private final ChangeControl.GenericFactory changeControlFactory; private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeIndexer indexer; private final ChangeIndexer indexer;
private final ProjectCache projectCache;
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final Provider<CurrentUser> user; private final Provider<CurrentUser> user;
private final ChangeKindCache changeKindCache; private final ChangeKindCache changeKindCache;
@@ -83,7 +80,6 @@ public class ChangeEditUtil {
PatchSetInserter.Factory patchSetInserterFactory, PatchSetInserter.Factory patchSetInserterFactory,
ChangeControl.GenericFactory changeControlFactory, ChangeControl.GenericFactory changeControlFactory,
ChangeIndexer indexer, ChangeIndexer indexer,
ProjectCache projectCache,
Provider<ReviewDb> db, Provider<ReviewDb> db,
Provider<CurrentUser> user, Provider<CurrentUser> user,
ChangeKindCache changeKindCache, ChangeKindCache changeKindCache,
@@ -93,7 +89,6 @@ public class ChangeEditUtil {
this.patchSetInserterFactory = patchSetInserterFactory; this.patchSetInserterFactory = patchSetInserterFactory;
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.indexer = indexer; this.indexer = indexer;
this.projectCache = projectCache;
this.db = db; this.db = db;
this.user = user; this.user = user;
this.changeKindCache = changeKindCache; this.changeKindCache = changeKindCache;
@@ -196,10 +191,10 @@ public class ChangeEditUtil {
.append(inserter.getPatchSetId().get()) .append(inserter.getPatchSetId().get())
.append(": "); .append(": ");
ProjectState project = projectCache.get(change.getDest().getParentKey());
// Previously checked that the base patch set is the current patch set. // Previously checked that the base patch set is the current patch set.
ObjectId prior = ObjectId.fromString(basePatchSet.getRevision().get()); ObjectId prior = ObjectId.fromString(basePatchSet.getRevision().get());
ChangeKind kind = changeKindCache.getChangeKind(project, repo, prior, squashed); ChangeKind kind = changeKindCache.getChangeKind(
change.getProject(), repo, prior, squashed);
if (kind == ChangeKind.NO_CODE_CHANGE) { if (kind == ChangeKind.NO_CODE_CHANGE) {
message.append("Commit message was updated."); message.append("Commit message was updated.");
} else { } else {

View File

@@ -193,7 +193,8 @@ public class ReplaceOp extends BatchUpdate.Op {
@Override @Override
public void updateRepo(RepoContext ctx) throws Exception { public void updateRepo(RepoContext ctx) throws Exception {
changeKind = changeKindCache.getChangeKind(projectControl.getProjectState(), changeKind = changeKindCache.getChangeKind(
projectControl.getProject().getNameKey(),
ctx.getRepository(), priorCommit, commit); ctx.getRepository(), priorCommit, commit);
if (checkMergedInto) { if (checkMergedInto) {