diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java index ef3faa5555..2bd093d2c8 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java @@ -106,7 +106,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { public void onEvent(Event event) { if (event instanceof ChangeMergedEvent) { ChangeMergedEvent changeMergedEvent = (ChangeMergedEvent) event; - mergeResults.put(changeMergedEvent.change.number, + mergeResults.put(changeMergedEvent.change.get().number, changeMergedEvent.newRev); } } 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 d9934f604a..8e02cc7211 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 @@ -14,6 +14,10 @@ package com.google.gerrit.common; +import com.google.common.base.Optional; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; +import com.google.common.base.Throwables; import com.google.common.collect.Sets; import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.google.gerrit.common.data.LabelType; @@ -34,8 +38,11 @@ import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.config.AnonymousCowardName; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; +import com.google.gerrit.server.data.AccountAttribute; import com.google.gerrit.server.data.ApprovalAttribute; +import com.google.gerrit.server.data.ChangeAttribute; import com.google.gerrit.server.data.PatchSetAttribute; +import com.google.gerrit.server.data.RefUpdateAttribute; import com.google.gerrit.server.events.ChangeAbandonedEvent; import com.google.gerrit.server.events.ChangeMergedEvent; import com.google.gerrit.server.events.ChangeRestoredEvent; @@ -174,46 +181,46 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, private final DynamicSet unrestrictedListeners; /** Path of the new patchset hook. */ - private final Path patchsetCreatedHook; + private final Optional patchsetCreatedHook; /** Path of the draft published hook. */ - private final Path draftPublishedHook; + private final Optional draftPublishedHook; /** Path of the new comments hook. */ - private final Path commentAddedHook; + private final Optional commentAddedHook; /** Path of the change merged hook. */ - private final Path changeMergedHook; + private final Optional changeMergedHook; /** Path of the merge failed hook. */ - private final Path mergeFailedHook; + private final Optional mergeFailedHook; /** Path of the change abandoned hook. */ - private final Path changeAbandonedHook; + private final Optional changeAbandonedHook; /** Path of the change restored hook. */ - private final Path changeRestoredHook; + private final Optional changeRestoredHook; /** Path of the ref updated hook. */ - private final Path refUpdatedHook; + private final Optional refUpdatedHook; /** Path of the reviewer added hook. */ - private final Path reviewerAddedHook; + private final Optional reviewerAddedHook; /** Path of the topic changed hook. */ - private final Path topicChangedHook; + private final Optional topicChangedHook; /** Path of the cla signed hook. */ - private final Path claSignedHook; + private final Optional claSignedHook; /** Path of the update hook. */ - private final Path refUpdateHook; + private final Optional refUpdateHook; /** Path of the hashtags changed hook */ - private final Path hashtagsChangedHook; + private final Optional hashtagsChangedHook; /** Path of the project created hook. */ - private final Path projectCreatedHook; + private final Optional projectCreatedHook; private final String anonymousCowardName; @@ -297,10 +304,11 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, .build()); } - private static Path hook(Config config, Path path, String name) { + private static Optional hook(Config config, Path path, String name) { String setting = name.replace("-", "") + "hook"; String value = config.getString("hooks", null, setting); - return path.resolve(value != null ? value : name); + Path p = path.resolve(value != null ? value : name); + return Files.exists(p) ? Optional.of(p) : Optional.absent(); } @Override @@ -335,16 +343,6 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, } } - private PatchSetAttribute asPatchSetAttribute(Change change, - PatchSet patchSet, ReviewDb db) throws OrmException { - try (Repository repo = repoManager.openRepository(change.getProject()); - RevWalk revWalk = new RevWalk(repo)) { - return eventFactory.asPatchSetAttribute(db, revWalk, patchSet); - } catch (IOException e) { - throw new OrmException(e); - } - } - /** * Fire the update hook * @@ -352,6 +350,9 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, @Override public HookResult doRefUpdateHook(Project project, String refname, Account uploader, ObjectId oldId, ObjectId newId) { + if (!refUpdateHook.isPresent()) { + return null; + } List args = new ArrayList<>(); addArg(args, "--project", project.getName()); @@ -368,8 +369,13 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, ProjectCreatedEvent event = new ProjectCreatedEvent(); event.projectName = project.get(); event.headName = headName; + fireEvent(project, event); + if (!projectCreatedHook.isPresent()) { + return; + } + List args = new ArrayList<>(); addArg(args, "--project", project.get()); addArg(args, "--head", headName); @@ -382,32 +388,42 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, * * @param change The change itself. * @param patchSet The Patchset that was created. + * @param db Review database. * @throws OrmException */ @Override - public void doPatchsetCreatedHook(Change change, PatchSet patchSet, - ReviewDb db) throws OrmException { + public void doPatchsetCreatedHook(Change change, + PatchSet patchSet, ReviewDb db) throws OrmException { PatchSetCreatedEvent event = new PatchSetCreatedEvent(); - AccountState uploader = accountCache.get(patchSet.getUploader()); - AccountState owner = accountCache.get(change.getOwner()); + Supplier uploader = + getAccountSupplier(patchSet.getUploader()); + Supplier owner = getAccountSupplier(change.getOwner()); + + event.change = changeAttributeSupplier(change); + event.patchSet = patchSetAttributeSupplier(change, patchSet); + event.uploader = accountAttributeSupplier(uploader); - event.change = eventFactory.asChangeAttribute(db, change); - event.patchSet = asPatchSetAttribute(change, patchSet, db); - event.uploader = eventFactory.asAccountAttribute(uploader.getAccount()); fireEvent(change, event, db); + if (!patchsetCreatedHook.isPresent()) { + return; + } + List args = new ArrayList<>(); - addArg(args, "--change", event.change.id); + + ChangeAttribute c = event.change.get(); + PatchSetAttribute ps = event.patchSet.get(); + addArg(args, "--change", c.id); addArg(args, "--is-draft", String.valueOf(patchSet.isDraft())); - addArg(args, "--kind", String.valueOf(event.patchSet.kind)); - addArg(args, "--change-url", event.change.url); - addArg(args, "--change-owner", getDisplayName(owner.getAccount())); - addArg(args, "--project", event.change.project); - addArg(args, "--branch", event.change.branch); - addArg(args, "--topic", event.change.topic); - addArg(args, "--uploader", getDisplayName(uploader.getAccount())); - addArg(args, "--commit", event.patchSet.revision); - addArg(args, "--patchset", event.patchSet.number); + addArg(args, "--kind", String.valueOf(ps.kind)); + addArg(args, "--change-url", c.url); + addArg(args, "--change-owner", getDisplayName(owner.get().getAccount())); + addArg(args, "--project", c.project); + addArg(args, "--branch", c.branch); + addArg(args, "--topic", c.topic); + addArg(args, "--uploader", getDisplayName(uploader.get().getAccount())); + addArg(args, "--commit", ps.revision); + addArg(args, "--patchset", ps.number); runHook(change.getProject(), patchsetCreatedHook, args); } @@ -416,62 +432,88 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, public void doDraftPublishedHook(Change change, PatchSet patchSet, ReviewDb db) throws OrmException { DraftPublishedEvent event = new DraftPublishedEvent(); - AccountState uploader = accountCache.get(patchSet.getUploader()); - AccountState owner = accountCache.get(change.getOwner()); + Supplier uploader = + getAccountSupplier(patchSet.getUploader()); + Supplier owner = getAccountSupplier(change.getOwner()); + + event.change = changeAttributeSupplier(change); + event.patchSet = patchSetAttributeSupplier(change, patchSet); + event.uploader = accountAttributeSupplier(uploader); - event.change = eventFactory.asChangeAttribute(db, change); - event.patchSet = asPatchSetAttribute(change, patchSet, db); - event.uploader = eventFactory.asAccountAttribute(uploader.getAccount()); fireEvent(change, event, db); + if (!draftPublishedHook.isPresent()) { + return; + } + List args = new ArrayList<>(); - addArg(args, "--change", event.change.id); - addArg(args, "--change-url", event.change.url); - addArg(args, "--change-owner", getDisplayName(owner.getAccount())); - addArg(args, "--project", event.change.project); - addArg(args, "--branch", event.change.branch); - addArg(args, "--topic", event.change.topic); - addArg(args, "--uploader", getDisplayName(uploader.getAccount())); - addArg(args, "--commit", event.patchSet.revision); - addArg(args, "--patchset", event.patchSet.number); + ChangeAttribute c = event.change.get(); + PatchSetAttribute ps = event.patchSet.get(); + + addArg(args, "--change", c.id); + addArg(args, "--change-url", c.url); + addArg(args, "--change-owner", getDisplayName(owner.get().getAccount())); + addArg(args, "--project", c.project); + addArg(args, "--branch", c.branch); + addArg(args, "--topic", c.topic); + addArg(args, "--uploader", getDisplayName(uploader.get().getAccount())); + addArg(args, "--commit", ps.revision); + addArg(args, "--patchset", ps.number); runHook(change.getProject(), draftPublishedHook, args); } @Override - public void doCommentAddedHook(Change change, Account account, - PatchSet patchSet, String comment, Map approvals, + public void doCommentAddedHook(final Change change, Account account, + PatchSet patchSet, String comment, final Map approvals, ReviewDb db) throws OrmException { CommentAddedEvent event = new CommentAddedEvent(); - AccountState owner = accountCache.get(change.getOwner()); + Supplier owner = getAccountSupplier(change.getOwner()); - event.change = eventFactory.asChangeAttribute(db, change); - event.author = eventFactory.asAccountAttribute(account); - event.patchSet = asPatchSetAttribute(change, patchSet, db); + event.change = changeAttributeSupplier(change); + event.author = accountAttributeSupplier(account); + event.patchSet = patchSetAttributeSupplier(change, patchSet); event.comment = comment; - - LabelTypes labelTypes = projectCache.get(change.getProject()).getLabelTypes(); - if (approvals.size() > 0) { - event.approvals = new ApprovalAttribute[approvals.size()]; - int i = 0; - for (Map.Entry approval : approvals.entrySet()) { - event.approvals[i++] = getApprovalAttribute(labelTypes, approval); - } - } + event.approvals = Suppliers.memoize( + new Supplier() { + @Override + public ApprovalAttribute[] get() { + LabelTypes labelTypes = projectCache.get( + change.getProject()).getLabelTypes(); + if (approvals.size() > 0) { + ApprovalAttribute[] r = new ApprovalAttribute[approvals.size()]; + int i = 0; + for (Map.Entry approval : approvals.entrySet()) { + r[i++] = getApprovalAttribute(labelTypes, approval); + } + return r; + } + return null; + } + }); fireEvent(change, event, db); + if (!commentAddedHook.isPresent()) { + return; + } + List args = new ArrayList<>(); - addArg(args, "--change", event.change.id); + ChangeAttribute c = event.change.get(); + PatchSetAttribute ps = event.patchSet.get(); + + addArg(args, "--change", c.id); addArg(args, "--is-draft", patchSet.isDraft() ? "true" : "false"); - addArg(args, "--change-url", event.change.url); - addArg(args, "--change-owner", getDisplayName(owner.getAccount())); - addArg(args, "--project", event.change.project); - addArg(args, "--branch", event.change.branch); - addArg(args, "--topic", event.change.topic); + addArg(args, "--change-url", c.url); + addArg(args, "--change-owner", getDisplayName(owner.get().getAccount())); + addArg(args, "--project", c.project); + addArg(args, "--branch", c.branch); + addArg(args, "--topic", c.topic); addArg(args, "--author", getDisplayName(account)); - addArg(args, "--commit", event.patchSet.revision); + addArg(args, "--commit", ps.revision); addArg(args, "--comment", comment == null ? "" : comment); + LabelTypes labelTypes = projectCache.get( + change.getProject()).getLabelTypes(); for (Map.Entry approval : approvals.entrySet()) { LabelType lt = labelTypes.byLabel(approval.getKey()); if (lt != null) { @@ -487,23 +529,31 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, PatchSet patchSet, ReviewDb db, String mergeResultRev) throws OrmException { ChangeMergedEvent event = new ChangeMergedEvent(); - AccountState owner = accountCache.get(change.getOwner()); + Supplier owner = getAccountSupplier(change.getOwner()); - event.change = eventFactory.asChangeAttribute(db, change); - event.submitter = eventFactory.asAccountAttribute(account); - event.patchSet = asPatchSetAttribute(change, patchSet, db); + event.change = changeAttributeSupplier(change); + event.submitter = accountAttributeSupplier(account); + event.patchSet = patchSetAttributeSupplier(change, patchSet); event.newRev = mergeResultRev; + fireEvent(change, event, db); + if (!changeMergedHook.isPresent()) { + return; + } + List args = new ArrayList<>(); - addArg(args, "--change", event.change.id); - addArg(args, "--change-url", event.change.url); - addArg(args, "--change-owner", getDisplayName(owner.getAccount())); - addArg(args, "--project", event.change.project); - addArg(args, "--branch", event.change.branch); - addArg(args, "--topic", event.change.topic); + ChangeAttribute c = event.change.get(); + PatchSetAttribute ps = event.patchSet.get(); + + addArg(args, "--change", c.id); + addArg(args, "--change-url", c.url); + addArg(args, "--change-owner", getDisplayName(owner.get().getAccount())); + addArg(args, "--project", c.project); + addArg(args, "--branch", c.branch); + addArg(args, "--topic", c.topic); addArg(args, "--submitter", getDisplayName(account)); - addArg(args, "--commit", event.patchSet.revision); + addArg(args, "--commit", ps.revision); addArg(args, "--newrev", mergeResultRev); runHook(change.getProject(), changeMergedHook, args); @@ -514,23 +564,31 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, PatchSet patchSet, String reason, ReviewDb db) throws OrmException { MergeFailedEvent event = new MergeFailedEvent(); - AccountState owner = accountCache.get(change.getOwner()); + Supplier owner = getAccountSupplier(change.getOwner()); - event.change = eventFactory.asChangeAttribute(db, change); - event.submitter = eventFactory.asAccountAttribute(account); - event.patchSet = asPatchSetAttribute(change, patchSet, db); + event.change = changeAttributeSupplier(change); + event.submitter = accountAttributeSupplier(account); + event.patchSet = patchSetAttributeSupplier(change, patchSet); event.reason = reason; + fireEvent(change, event, db); + if (!mergeFailedHook.isPresent()) { + return; + } + List args = new ArrayList<>(); - addArg(args, "--change", event.change.id); - addArg(args, "--change-url", event.change.url); - addArg(args, "--change-owner", getDisplayName(owner.getAccount())); - addArg(args, "--project", event.change.project); - addArg(args, "--branch", event.change.branch); - addArg(args, "--topic", event.change.topic); + ChangeAttribute c = event.change.get(); + PatchSetAttribute ps = event.patchSet.get(); + + addArg(args, "--change", c.id); + addArg(args, "--change-url", c.url); + addArg(args, "--change-owner", getDisplayName(owner.get().getAccount())); + addArg(args, "--project", c.project); + addArg(args, "--branch", c.branch); + addArg(args, "--topic", c.topic); addArg(args, "--submitter", getDisplayName(account)); - addArg(args, "--commit", event.patchSet.revision); + addArg(args, "--commit", ps.revision); addArg(args, "--reason", reason == null ? "" : reason); runHook(change.getProject(), mergeFailedHook, args); @@ -543,21 +601,29 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, ChangeAbandonedEvent event = new ChangeAbandonedEvent(); AccountState owner = accountCache.get(change.getOwner()); - event.change = eventFactory.asChangeAttribute(db, change); - event.abandoner = eventFactory.asAccountAttribute(account); - event.patchSet = asPatchSetAttribute(change, patchSet, db); + event.change = changeAttributeSupplier(change); + event.abandoner = accountAttributeSupplier(account); + event.patchSet = patchSetAttributeSupplier(change, patchSet); event.reason = reason; + fireEvent(change, event, db); + if (!changeAbandonedHook.isPresent()) { + return; + } + List args = new ArrayList<>(); - addArg(args, "--change", event.change.id); - addArg(args, "--change-url", event.change.url); + ChangeAttribute c = event.change.get(); + PatchSetAttribute ps = event.patchSet.get(); + + addArg(args, "--change", c.id); + addArg(args, "--change-url", c.url); addArg(args, "--change-owner", getDisplayName(owner.getAccount())); - addArg(args, "--project", event.change.project); - addArg(args, "--branch", event.change.branch); - addArg(args, "--topic", event.change.topic); + addArg(args, "--project", c.project); + addArg(args, "--branch", c.branch); + addArg(args, "--topic", c.topic); addArg(args, "--abandoner", getDisplayName(account)); - addArg(args, "--commit", event.patchSet.revision); + addArg(args, "--commit", ps.revision); addArg(args, "--reason", reason == null ? "" : reason); runHook(change.getProject(), changeAbandonedHook, args); @@ -570,21 +636,29 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, ChangeRestoredEvent event = new ChangeRestoredEvent(); AccountState owner = accountCache.get(change.getOwner()); - event.change = eventFactory.asChangeAttribute(db, change); - event.restorer = eventFactory.asAccountAttribute(account); - event.patchSet = asPatchSetAttribute(change, patchSet, db); + event.change = changeAttributeSupplier(change); + event.restorer = accountAttributeSupplier(account); + event.patchSet = patchSetAttributeSupplier(change, patchSet); event.reason = reason; + fireEvent(change, event, db); + if (!changeRestoredHook.isPresent()) { + return; + } + List args = new ArrayList<>(); - addArg(args, "--change", event.change.id); - addArg(args, "--change-url", event.change.url); + ChangeAttribute c = event.change.get(); + PatchSetAttribute ps = event.patchSet.get(); + + addArg(args, "--change", c.id); + addArg(args, "--change-url", c.url); addArg(args, "--change-owner", getDisplayName(owner.getAccount())); - addArg(args, "--project", event.change.project); - addArg(args, "--branch", event.change.branch); - addArg(args, "--topic", event.change.topic); + addArg(args, "--project", c.project); + addArg(args, "--branch", c.branch); + addArg(args, "--topic", c.topic); addArg(args, "--restorer", getDisplayName(account)); - addArg(args, "--commit", event.patchSet.revision); + addArg(args, "--commit", ps.revision); addArg(args, "--reason", reason == null ? "" : reason); runHook(change.getProject(), changeRestoredHook, args); @@ -598,21 +672,33 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, } @Override - public void doRefUpdatedHook(Branch.NameKey refName, ObjectId oldId, - ObjectId newId, Account account) { + public void doRefUpdatedHook(final Branch.NameKey refName, + final ObjectId oldId, final ObjectId newId, Account account) { RefUpdatedEvent event = new RefUpdatedEvent(); if (account != null) { - event.submitter = eventFactory.asAccountAttribute(account); + event.submitter = accountAttributeSupplier(account); } - event.refUpdate = eventFactory.asRefUpdateAttribute(oldId, newId, refName); + event.refUpdate = Suppliers.memoize( + new Supplier() { + @Override + public RefUpdateAttribute get() { + return eventFactory.asRefUpdateAttribute(oldId, newId, refName); + } + }); + fireEvent(refName, event); + if (!refUpdatedHook.isPresent()) { + return; + } + List args = new ArrayList<>(); - addArg(args, "--oldrev", event.refUpdate.oldRev); - addArg(args, "--newrev", event.refUpdate.newRev); - addArg(args, "--refname", event.refUpdate.refName); - addArg(args, "--project", event.refUpdate.project); + RefUpdateAttribute r = event.refUpdate.get(); + addArg(args, "--oldrev", r.oldRev); + addArg(args, "--newrev", r.newRev); + addArg(args, "--refname", r.refName); + addArg(args, "--project", r.project); if (account != null) { addArg(args, "--submitter", getDisplayName(account)); } @@ -624,19 +710,26 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, public void doReviewerAddedHook(Change change, Account account, PatchSet patchSet, ReviewDb db) throws OrmException { ReviewerAddedEvent event = new ReviewerAddedEvent(); - AccountState owner = accountCache.get(change.getOwner()); + Supplier owner = getAccountSupplier(change.getOwner()); + + event.change = changeAttributeSupplier(change); + event.patchSet = null;//patchSetAttributeSupplier(change, patchSet, db); + event.reviewer = null;//accountAttributeSupplier(account); - event.change = eventFactory.asChangeAttribute(db, change); - event.patchSet = asPatchSetAttribute(change, patchSet, db); - event.reviewer = eventFactory.asAccountAttribute(account); fireEvent(change, event, db); + if (!reviewerAddedHook.isPresent()) { + return; + } + List args = new ArrayList<>(); - addArg(args, "--change", event.change.id); - addArg(args, "--change-url", event.change.url); - addArg(args, "--change-owner", getDisplayName(owner.getAccount())); - addArg(args, "--project", event.change.project); - addArg(args, "--branch", event.change.branch); + ChangeAttribute c = event.change.get(); + + addArg(args, "--change", c.id); + addArg(args, "--change-url", c.url); + addArg(args, "--change-owner", getDisplayName(owner.get().getAccount())); + addArg(args, "--project", c.project); + addArg(args, "--branch", c.branch); addArg(args, "--reviewer", getDisplayName(account)); runHook(change.getProject(), reviewerAddedHook, args); @@ -649,19 +742,26 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, TopicChangedEvent event = new TopicChangedEvent(); AccountState owner = accountCache.get(change.getOwner()); - event.change = eventFactory.asChangeAttribute(db, change); - event.changer = eventFactory.asAccountAttribute(account); + event.change = changeAttributeSupplier(change); + event.changer = accountAttributeSupplier(account); event.oldTopic = oldTopic; + fireEvent(change, event, db); + if (!topicChangedHook.isPresent()) { + return; + } + List args = new ArrayList<>(); - addArg(args, "--change", event.change.id); + ChangeAttribute c = event.change.get(); + + addArg(args, "--change", c.id); addArg(args, "--change-owner", getDisplayName(owner.getAccount())); - addArg(args, "--project", event.change.project); - addArg(args, "--branch", event.change.branch); + addArg(args, "--project", c.project); + addArg(args, "--branch", c.branch); addArg(args, "--changer", getDisplayName(account)); addArg(args, "--old-topic", oldTopic); - addArg(args, "--new-topic", event.change.topic); + addArg(args, "--new-topic", c.topic); runHook(change.getProject(), topicChangedHook, args); } @@ -681,19 +781,25 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, HashtagsChangedEvent event = new HashtagsChangedEvent(); AccountState owner = accountCache.get(change.getOwner()); - event.change = eventFactory.asChangeAttribute(db, change); - event.editor = eventFactory.asAccountAttribute(account); + event.change = changeAttributeSupplier(change); + event.editor = accountAttributeSupplier(account); event.hashtags = hashtagArray(hashtags); event.added = hashtagArray(added); event.removed = hashtagArray(removed); fireEvent(change, event, db); + if (!hashtagsChangedHook.isPresent()) { + return; + } + List args = new ArrayList<>(); - addArg(args, "--change", event.change.id); + ChangeAttribute c = event.change.get(); + + addArg(args, "--change", c.id); addArg(args, "--change-owner", getDisplayName(owner.getAccount())); - addArg(args, "--project", event.change.project); - addArg(args, "--branch", event.change.branch); + addArg(args, "--project", c.project); + addArg(args, "--branch", c.branch); addArg(args, "--editor", getDisplayName(account)); if (hashtags != null) { for (String hashtag : hashtags) { @@ -715,6 +821,10 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, @Override public void doClaSignupHook(Account account, String claName) { + if (!claSignedHook.isPresent()) { + return; + } + if (account != null) { List args = new ArrayList<>(); addArg(args, "--submitter", getDisplayName(account)); @@ -736,6 +846,67 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, fireEvent(branchName, event); } + private Supplier getAccountSupplier( + final Account.Id account) { + return Suppliers.memoize( + new Supplier() { + @Override + public AccountState get() { + return accountCache.get(account); + } + }); + } + + private Supplier accountAttributeSupplier( + final Supplier s) { + return Suppliers.memoize( + new Supplier() { + @Override + public AccountAttribute get() { + return eventFactory.asAccountAttribute(s.get().getAccount()); + } + }); + } + + private Supplier accountAttributeSupplier( + final Account account) { + return Suppliers.memoize( + new Supplier() { + @Override + public AccountAttribute get() { + return eventFactory.asAccountAttribute(account); + } + }); + } + + private Supplier patchSetAttributeSupplier( + final Change change, final PatchSet patchSet) { + return Suppliers.memoize( + new Supplier() { + @Override + public PatchSetAttribute get() { + try (Repository repo + = repoManager.openRepository(change.getProject()); + RevWalk revWalk = new RevWalk(repo)) { + return eventFactory.asPatchSetAttribute(revWalk, patchSet); + } catch (IOException e) { + throw Throwables.propagate(e); + } + } + }); + } + + private Supplier changeAttributeSupplier( + final Change change) { + return Suppliers.memoize( + new Supplier() { + @Override + public ChangeAttribute get() { + return eventFactory.asChangeAttribute(change); + } + }); + } + private void fireEventForUnrestrictedListeners(com.google.gerrit.server.events.Event event) { for (EventListener listener : unrestrictedListeners) { listener.onEvent(event); @@ -851,27 +1022,27 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, * @param hook the hook to execute. * @param args Arguments to use to run the hook. */ - private synchronized void runHook(Project.NameKey project, Path hook, + private synchronized void runHook(Project.NameKey project, Optional hook, List args) { - if (project != null && Files.exists(hook)) { - hookQueue.execute(new AsyncHookTask(project, hook, args)); + if (project != null && hook.isPresent()) { + hookQueue.execute(new AsyncHookTask(project, hook.get(), args)); } } - private synchronized void runHook(Path hook, List args) { - if (Files.exists(hook)) { - hookQueue.execute(new AsyncHookTask(null, hook, args)); + private synchronized void runHook(Optional hook, List args) { + if (hook.isPresent()) { + hookQueue.execute(new AsyncHookTask(null, hook.get(), args)); } } private HookResult runSyncHook(Project.NameKey project, - Path hook, List args) { + Optional hook, List args) { - if (!Files.exists(hook)) { + if (!hook.isPresent()) { return null; } - SyncHookTask syncHook = new SyncHookTask(project, hook, args); + SyncHookTask syncHook = new SyncHookTask(project, hook.get(), args); FutureTask task = new FutureTask<>(syncHook); syncHookThreadPool.execute(task); @@ -881,10 +1052,10 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, try { return task.get(syncHookTimeout, TimeUnit.SECONDS); } catch (TimeoutException e) { - message = "Synchronous hook timed out " + hook.toAbsolutePath(); + message = "Synchronous hook timed out " + hook.get().toAbsolutePath(); log.error(message); } catch (Exception e) { - message = "Error running hook " + hook.toAbsolutePath(); + message = "Error running hook " + hook.get().toAbsolutePath(); log.error(message, e); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeAbandonedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeAbandonedEvent.java index c285a82bcd..ea13d1f6b6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeAbandonedEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeAbandonedEvent.java @@ -14,10 +14,11 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.server.data.AccountAttribute; public class ChangeAbandonedEvent extends PatchSetEvent { - public AccountAttribute abandoner; + public Supplier abandoner; public String reason; public ChangeAbandonedEvent() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeEvent.java index f083b7ab80..85cc998c4d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeEvent.java @@ -14,13 +14,14 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.data.ChangeAttribute; public abstract class ChangeEvent extends RefEvent { - public ChangeAttribute change; + public Supplier change; protected ChangeEvent(String type) { super(type); @@ -28,15 +29,15 @@ public abstract class ChangeEvent extends RefEvent { @Override public Project.NameKey getProjectNameKey() { - return new Project.NameKey(change.project); + return new Project.NameKey(change.get().project); } @Override public String getRefName() { - return RefNames.fullName(change.branch); + return RefNames.fullName(change.get().branch); } public Change.Key getChangeKey() { - return new Change.Key(change.id); + return new Change.Key(change.get().id); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeMergedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeMergedEvent.java index 41a95cb0f7..d762f0fe91 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeMergedEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeMergedEvent.java @@ -14,10 +14,11 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.server.data.AccountAttribute; public class ChangeMergedEvent extends PatchSetEvent { - public AccountAttribute submitter; + public Supplier submitter; public String newRev; public ChangeMergedEvent() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeRestoredEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeRestoredEvent.java index a575a42cee..f045b8f15b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeRestoredEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeRestoredEvent.java @@ -14,10 +14,11 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.server.data.AccountAttribute; public class ChangeRestoredEvent extends PatchSetEvent { - public AccountAttribute restorer; + public Supplier restorer; public String reason; public ChangeRestoredEvent () { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/CommentAddedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/CommentAddedEvent.java index 4391dfbdb9..9f0e0aa144 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/CommentAddedEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/CommentAddedEvent.java @@ -14,12 +14,13 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.server.data.AccountAttribute; import com.google.gerrit.server.data.ApprovalAttribute; public class CommentAddedEvent extends PatchSetEvent { - public AccountAttribute author; - public ApprovalAttribute[] approvals; + public Supplier author; + public Supplier approvals; public String comment; public CommentAddedEvent() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/DraftPublishedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/DraftPublishedEvent.java index 5db628f673..f27f0f1cae 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/DraftPublishedEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/DraftPublishedEvent.java @@ -14,10 +14,11 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.server.data.AccountAttribute; public class DraftPublishedEvent extends PatchSetEvent { - public AccountAttribute uploader; + public Supplier uploader; public DraftPublishedEvent() { super("draft-published"); 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 d52f8316d8..f160248070 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 @@ -61,6 +61,7 @@ import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -92,6 +93,7 @@ public class EventFactory { private final ApprovalsUtil approvalsUtil; private final ChangeKindCache changeKindCache; private final Provider queryProvider; + private final SchemaFactory schema; @Inject EventFactory(AccountCache accountCache, @@ -102,7 +104,8 @@ public class EventFactory { ChangeData.Factory changeDataFactory, ApprovalsUtil approvalsUtil, ChangeKindCache changeKindCache, - Provider queryProvider) { + Provider queryProvider, + SchemaFactory schema) { this.accountCache = accountCache; this.urlProvider = urlProvider; this.patchListCache = patchListCache; @@ -112,6 +115,7 @@ public class EventFactory { this.approvalsUtil = approvalsUtil; this.changeKindCache = changeKindCache; this.queryProvider = queryProvider; + this.schema = schema; } /** @@ -121,6 +125,23 @@ public class EventFactory { * @param change * @return object suitable for serialization to JSON */ + public ChangeAttribute asChangeAttribute(Change change) { + try (ReviewDb db = schema.open()) { + return asChangeAttribute(db, change); + } catch (OrmException e) { + log.error("Cannot open database connection", e); + return new ChangeAttribute(); + } + } + + /** + * Create a ChangeAttribute for the given change suitable for serialization to + * JSON. + * + * @param db Review database + * @param change + * @return object suitable for serialization to JSON + */ public ChangeAttribute asChangeAttribute(ReviewDb db, Change change) { ChangeAttribute a = new ChangeAttribute(); a.project = change.getProject().get(); @@ -429,6 +450,19 @@ public class EventFactory { * @param patchSet * @return object suitable for serialization to JSON */ + public PatchSetAttribute asPatchSetAttribute(RevWalk revWalk, + PatchSet patchSet) { + return asPatchSetAttribute(revWalk, patchSet); + } + + /** + * Create a PatchSetAttribute for the given patchset suitable for + * serialization to JSON. + * + * @param db Review database + * @param patchSet + * @return object suitable for serialization to JSON + */ public PatchSetAttribute asPatchSetAttribute(ReviewDb db, RevWalk revWalk, PatchSet patchSet) { PatchSetAttribute p = new PatchSetAttribute(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/HashtagsChangedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/HashtagsChangedEvent.java index c5919e50c9..28c6e4dead 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/HashtagsChangedEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/HashtagsChangedEvent.java @@ -14,10 +14,11 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.server.data.AccountAttribute; public class HashtagsChangedEvent extends ChangeEvent { - public AccountAttribute editor; + public Supplier editor; public String[] added; public String[] removed; public String[] hashtags; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/MergeFailedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/MergeFailedEvent.java index 75cbcb028c..a7d8c457df 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/MergeFailedEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/MergeFailedEvent.java @@ -14,10 +14,11 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.server.data.AccountAttribute; public class MergeFailedEvent extends PatchSetEvent { - public AccountAttribute submitter; + public Supplier submitter; public String reason; public MergeFailedEvent() { 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 e4685939d5..4f7e3e73d5 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,10 +14,11 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.server.data.AccountAttribute; public class PatchSetCreatedEvent extends PatchSetEvent { - public AccountAttribute uploader; + public Supplier uploader; public PatchSetCreatedEvent() { super("patchset-created"); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/PatchSetEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/PatchSetEvent.java index cdaf601843..8d0b079b2a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/PatchSetEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/PatchSetEvent.java @@ -14,10 +14,11 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.server.data.PatchSetAttribute; public class PatchSetEvent extends ChangeEvent { - public PatchSetAttribute patchSet; + public Supplier patchSet; protected PatchSetEvent(String type) { super(type); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/RefUpdatedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/RefUpdatedEvent.java index e5039ff425..f2a72b8eec 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/RefUpdatedEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/RefUpdatedEvent.java @@ -14,13 +14,14 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.data.AccountAttribute; import com.google.gerrit.server.data.RefUpdateAttribute; public class RefUpdatedEvent extends RefEvent { - public AccountAttribute submitter; - public RefUpdateAttribute refUpdate; + public Supplier submitter; + public Supplier refUpdate; public RefUpdatedEvent() { super("ref-updated"); @@ -28,11 +29,11 @@ public class RefUpdatedEvent extends RefEvent { @Override public Project.NameKey getProjectNameKey() { - return new Project.NameKey(refUpdate.project); + return new Project.NameKey(refUpdate.get().project); } @Override public String getRefName() { - return refUpdate.refName; + return refUpdate.get().refName; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/ReviewerAddedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/ReviewerAddedEvent.java index b016bd9de5..483f68ee72 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/ReviewerAddedEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/ReviewerAddedEvent.java @@ -14,10 +14,11 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.server.data.AccountAttribute; public class ReviewerAddedEvent extends PatchSetEvent { - public AccountAttribute reviewer; + public Supplier reviewer; public ReviewerAddedEvent() { super("reviewer-added"); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/TopicChangedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/TopicChangedEvent.java index 7bb334fa7e..d038edd36a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/TopicChangedEvent.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/TopicChangedEvent.java @@ -14,10 +14,11 @@ package com.google.gerrit.server.events; +import com.google.common.base.Supplier; import com.google.gerrit.server.data.AccountAttribute; public class TopicChangedEvent extends ChangeEvent { - public AccountAttribute changer; + public Supplier changer; public String oldTopic; public TopicChangedEvent() { diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StreamEvents.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StreamEvents.java index 379f1b92b9..91757585ef 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StreamEvents.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StreamEvents.java @@ -17,6 +17,7 @@ package com.google.gerrit.sshd.commands; import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER; import static java.nio.charset.StandardCharsets.UTF_8; +import com.google.common.base.Supplier; import com.google.gerrit.common.EventListener; import com.google.gerrit.common.EventSource; import com.google.gerrit.common.data.GlobalCapability; @@ -30,12 +31,17 @@ import com.google.gerrit.sshd.BaseCommand; import com.google.gerrit.sshd.CommandMetaData; import com.google.gerrit.sshd.StreamCommandExecutor; import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.JsonElement; +import com.google.gson.JsonSerializationContext; +import com.google.gson.JsonSerializer; import com.google.inject.Inject; import org.apache.sshd.server.Environment; import java.io.IOException; import java.io.PrintWriter; +import java.lang.reflect.Type; import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; @@ -63,7 +69,7 @@ final class StreamEvents extends BaseCommand { private final LinkedBlockingQueue queue = new LinkedBlockingQueue<>(MAX_EVENTS); - private final Gson gson = new Gson(); + private Gson gson; /** Special event to notify clients they missed other events. */ private static final class DroppedOutputEvent extends Event { @@ -139,6 +145,10 @@ final class StreamEvents extends BaseCommand { stdout = toPrintWriter(out); source.addEventListener(listener, currentUser); + + gson = new GsonBuilder() + .registerTypeAdapter(Supplier.class, new SupplierSerializer()) + .create(); } @Override @@ -247,4 +257,13 @@ final class StreamEvents extends BaseCommand { stdout.flush(); } } + + private static class SupplierSerializer + implements JsonSerializer> { + @Override + public JsonElement serialize(Supplier src, Type typeOfSrc, + JsonSerializationContext context) { + return context.serialize(src.get()); + } + } }