opendev/gerrit
Code Issues Proposed changes

Add change kind to PatchSetCreatedEvent

Browse Source 
Pass along an extra flag in the patchset-created hook to convey
the change kind.  This is also exposed in the PatchSetCreatedEvent
as the "kind" field.  Both follow the ChangeKind enum type, where a
change can be "REWORK", "NO_CODE_CHANGE", or "TRIVIAL_REBASE".

Bug: Issue 2634
Change-Id: Ie04d0282e42ab177810fafd857e90d21999cab28
This commit is contained in:
Doug Kelly 2014-05-15 11:54:44 -05:00 committed by Dave Borowitz
parent 310c4a16f2
commit 58ba20634f
6 changed files with 132 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
Documentation/config-hooks.txt
View File

@ -41,9 +41,18 @@ This is called whenever a patchset is created (this includes new
changes and drafts).
====
patchset-created --change <change id> --is-draft <boolean> --change-url <change url> --change-owner <change owner> --project <project name> --branch <branch> --topic <topic> --uploader <uploader> --commit <sha1> --patchset <patchset id>
patchset-created --change <change id> --is-draft <boolean> --kind <change kind> --change-url <change url> --change-owner <change owner> --project <project name> --branch <branch> --topic <topic> --uploader <uploader> --commit <sha1> --patchset <patchset id>
====
kind:: change kind represents the kind of change uploaded, also represented in link:json.html#patchSet[patchSet]
REWORK;; Nontrivial content changes.
TRIVIAL_REBASE;; Conflict-free merge between the new parent and the prior patch set.
NO_CODE_CHANGE;; No code changed; same tree and same parents.
=== draft-published
This is called whenever a draft change is published.

8
Documentation/json.txt
View File

@ -115,6 +115,14 @@ was created.
isDraft:: Whether or not the patch set is a draft patch set.
changeKind:: Kind of change uploaded.
REWORK;; Nontrivial content changes.
TRIVIAL_REBASE;; Conflict-free merge between the new parent and the prior patch set.
NO_CODE_CHANGE;; No code changed; same tree and same parents.
approvals:: The <<approval,approval attribute>> granted.
comments:: All comments for this patchset in <<patchsetcomment,patchsetComment attributes>>.

9
gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
View File

@ -30,6 +30,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.change.ChangeKindCache;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
@ -209,6 +210,8 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
private final AccountCache accountCache;
private final ChangeKindCache changeKindCache;
private final EventFactory eventFactory;
private final SitePaths sitePaths;
@ -236,6 +239,7 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
final SitePaths sitePath,
final ProjectCache projectCache,
final AccountCache accountCache,
final ChangeKindCache changeKindCache,
final EventFactory eventFactory,
final SitePaths sitePaths,
final DynamicSet<ChangeListener> unrestrictedListeners) {
@ -244,6 +248,7 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
this.hookQueue = queue.createQueue(1, "hook");
this.projectCache = projectCache;
this.accountCache = accountCache;
this.changeKindCache = changeKindCache;
this.eventFactory = eventFactory;
this.sitePaths = sitePath;
this.unrestrictedListeners = unrestrictedListeners;
@ -354,11 +359,13 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
event.change = eventFactory.asChangeAttribute(change);
event.patchSet = eventFactory.asPatchSetAttribute(patchSet);
event.uploader = eventFactory.asAccountAttribute(uploader.getAccount());
event.kind = changeKindCache.getChangeKind(db, change, patchSet);
fireEvent(change, event, db);
final List<String> args = new ArrayList<>();
addArg(args, "--change", event.change.id);
addArg(args, "--is-draft", patchSet.isDraft() ? "true" : "false");
addArg(args, "--is-draft", String.valueOf(patchSet.isDraft()));
addArg(args, "--kind", String.valueOf(event.kind));
addArg(args, "--change-url", event.change.url);
addArg(args, "--change-owner", getDisplayName(owner.getAccount()));
addArg(args, "--project", event.change.project);

5
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCache.java
View File

@ -14,6 +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.project.ProjectState;
import org.eclipse.jgit.lib.ObjectId;
@ -28,4 +31,6 @@ import org.eclipse.jgit.lib.Repository;
public interface ChangeKindCache {
public ChangeKind getChangeKind(ProjectState project, Repository repo,
ObjectId prior, ObjectId next);
public ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch);
}

101
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
View File

@ -23,10 +23,17 @@ import com.google.common.base.Objects;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.cache.Weigher;
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.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.Singleton;
@ -45,6 +52,7 @@ import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.util.Collection;
import java.util.concurrent.ExecutionException;
public class ChangeKindCacheImpl implements ChangeKindCache {
@ -69,11 +77,21 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
@VisibleForTesting
public static class NoCache implements ChangeKindCache {
private final boolean useRecursiveMerge;
private final ChangeData.Factory changeDataFactory;
private final ProjectCache projectCache;
private final GitRepositoryManager repoManager;
@Inject
NoCache(
@GerritServerConfig Config serverConfig) {
@GerritServerConfig Config serverConfig,
ChangeData.Factory changeDataFactory,
ProjectCache projectCache,
GitRepositoryManager repoManager) {
this.useRecursiveMerge = MergeUtil.useRecursiveMerge(serverConfig);
this.changeDataFactory = changeDataFactory;
this.projectCache = projectCache;
this.repoManager = repoManager;
}
@Override
@ -88,6 +106,12 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
return ChangeKind.REWORK;
}
}
@Override
public ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch) {
return getChangeKindInternal(this, db, change, patch, changeDataFactory,
projectCache, repoManager);
}
}
public static class Key implements Serializable {
@ -161,6 +185,10 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
private static class Loader extends CacheLoader<Key, ChangeKind> {
@Override
public ChangeKind load(Key key) throws IOException {
if (Objects.equal(key.prior, key.next)) {
return ChangeKind.NO_CODE_CHANGE;
}
RevWalk walk = new RevWalk(key.repo);
try {
RevCommit prior = walk.parseCommit(key.prior);
@ -224,13 +252,22 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
private final LoadingCache<Key, ChangeKind> cache;
private final boolean useRecursiveMerge;
private final ChangeData.Factory changeDataFactory;
private final ProjectCache projectCache;
private final GitRepositoryManager repoManager;
@Inject
ChangeKindCacheImpl(
@GerritServerConfig Config serverConfig,
@Named(ID_CACHE) LoadingCache<Key, ChangeKind> cache) {
@Named(ID_CACHE) LoadingCache<Key, ChangeKind> cache,
ChangeData.Factory changeDataFactory,
ProjectCache projectCache,
GitRepositoryManager repoManager) {
this.cache = cache;
this.useRecursiveMerge = MergeUtil.useRecursiveMerge(serverConfig);
this.changeDataFactory = changeDataFactory;
this.projectCache = projectCache;
this.repoManager = repoManager;
}
@Override
@ -244,4 +281,64 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
return ChangeKind.REWORK;
}
}
@Override
public ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch) {
return getChangeKindInternal(this, db, change, patch, changeDataFactory,
projectCache, repoManager);
}
private static ChangeKind getChangeKindInternal(
ChangeKindCache cache,
ReviewDb db,
Change change,
PatchSet patch,
ChangeData.Factory changeDataFactory,
ProjectCache projectCache,
GitRepositoryManager repoManager) {
Repository repo = null;
// TODO - dborowitz: add NEW_CHANGE type for default.
ChangeKind kind = ChangeKind.REWORK;
// Trivial case: if we're on the first patch, we don't need to open
// the repository.
if (patch.getId().get() > 1) {
try {
ProjectState projectState = projectCache.checkedGet(change.getProject());
repo = repoManager.openRepository(change.getProject());
ChangeData cd = changeDataFactory.create(db, change);
Collection<PatchSet> patchSetCollection = cd.patches();
PatchSet priorPs = patch;
for (PatchSet ps : patchSetCollection) {
if (ps.getId().get() < patch.getId().get() &&
(ps.getId().get() > priorPs.getId().get() ||
priorPs == patch)) {
// We only want the previous patch set, so walk until the last one
priorPs = ps;
}
}
// If we still think the previous patch is the current patch,
// we only have one patch set. Return the default.
// This can happen if a user creates a draft, uploads a second patch,
// and deletes the draft.
if (priorPs != patch) {
kind =
cache.getChangeKind(projectState, repo,
ObjectId.fromString(priorPs.getRevision().get()),
ObjectId.fromString(patch.getRevision().get()));
}
} 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);
} finally {
if (repo != null) {
repo.close();
}
}
}
return kind;
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/events/PatchSetCreatedEvent.java
View File

@ -14,6 +14,7 @@
package com.google.gerrit.server.events;
import com.google.gerrit.server.change.ChangeKind;
import com.google.gerrit.server.data.AccountAttribute;
import com.google.gerrit.server.data.ChangeAttribute;
import com.google.gerrit.server.data.PatchSetAttribute;
@ -23,4 +24,5 @@ public class PatchSetCreatedEvent extends ChangeEvent {
public ChangeAttribute change;
public PatchSetAttribute patchSet;
public AccountAttribute uploader;
public ChangeKind kind;
}