Merge "Add change kind to PatchSetCreatedEvent"
diff --git a/Documentation/config-hooks.txt b/Documentation/config-hooks.txt
index 4635f80..8d58d36 100644
--- a/Documentation/config-hooks.txt
+++ b/Documentation/config-hooks.txt
@@ -41,9 +41,18 @@
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.
diff --git a/Documentation/json.txt b/Documentation/json.txt
index 680705c6..6ab35f9 100644
--- a/Documentation/json.txt
+++ b/Documentation/json.txt
@@ -115,6 +115,14 @@
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>>.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
index 1819272..257799a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
@@ -30,6 +30,7 @@
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 @@
private final AccountCache accountCache;
+ private final ChangeKindCache changeKindCache;
+
private final EventFactory eventFactory;
private final SitePaths sitePaths;
@@ -236,6 +239,7 @@
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 @@
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 @@
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);
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 0e0984d..7b55b63 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,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 @@
public interface ChangeKindCache {
public ChangeKind getChangeKind(ProjectState project, Repository repo,
ObjectId prior, ObjectId next);
+
+ public ChangeKind getChangeKind(ReviewDb db, Change change, 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 220dcb6..8e2b6ac 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
@@ -23,10 +23,17 @@
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.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 @@
@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 @@
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 @@
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 @@
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 @@
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;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/PatchSetCreatedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/PatchSetCreatedEvent.java
index fbaf4ef..10e0214 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/events/PatchSetCreatedEvent.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/PatchSetCreatedEvent.java
@@ -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 ChangeAttribute change;
public PatchSetAttribute patchSet;
public AccountAttribute uploader;
+ public ChangeKind kind;
}