Add missing call to ref-updated hook for submodule updates

Add call to ref-updated hook when a project ref is updated because of
submodule subscription.

Bug: issue 2831
Change-Id: I176ea044fe4bbb2d5abc596b3da6dcd0a8d3e12f
This commit is contained in:
Hugo Arès
2014-08-14 14:57:05 -04:00
committed by David Pursehouse
parent 4745a16a8b
commit 038b642cbe
4 changed files with 28 additions and 10 deletions

View File

@@ -648,14 +648,17 @@ public class MergeOp {
private void fireRefUpdated(RefUpdate branchUpdate) { private void fireRefUpdated(RefUpdate branchUpdate) {
gitRefUpdated.fire(destBranch.getParentKey(), branchUpdate); gitRefUpdated.fire(destBranch.getParentKey(), branchUpdate);
hooks.doRefUpdatedHook(destBranch, branchUpdate, getAccount(mergeTip));
}
private Account getAccount(CodeReviewCommit codeReviewCommit) {
Account account = null; Account account = null;
PatchSetApproval submitter = approvalsUtil.getSubmitter( PatchSetApproval submitter = approvalsUtil.getSubmitter(
db, mergeTip.notes(), mergeTip.getPatchsetId()); db, codeReviewCommit.notes(), codeReviewCommit.getPatchsetId());
if (submitter != null) { if (submitter != null) {
account = accountCache.get(submitter.getAccountId()).getAccount(); account = accountCache.get(submitter.getAccountId()).getAccount();
} }
hooks.doRefUpdatedHook(destBranch, branchUpdate, account); return account;
} }
private void updateChangeStatus(final List<Change> submitted) { private void updateChangeStatus(final List<Change> submitted) {
@@ -719,7 +722,8 @@ public class MergeOp {
if (mergeTip != null && (branchTip == null || branchTip != mergeTip)) { if (mergeTip != null && (branchTip == null || branchTip != mergeTip)) {
SubmoduleOp subOp = SubmoduleOp subOp =
subOpFactory.create(destBranch, mergeTip, rw, repo, subOpFactory.create(destBranch, mergeTip, rw, repo,
destProject.getProject(), submitted, commits); destProject.getProject(), submitted, commits,
getAccount(mergeTip));
try { try {
subOp.update(); subOp.update();
} catch (SubmoduleException e) { } catch (SubmoduleException e) {

View File

@@ -2258,7 +2258,8 @@ public class ReceiveCommits {
subOpFactory.create( subOpFactory.create(
new Branch.NameKey(project.getNameKey(), cmd.getRefName()), new Branch.NameKey(project.getNameKey(), cmd.getRefName()),
codeReviewCommit, rw, repo, project, new ArrayList<Change>(), codeReviewCommit, rw, repo, project, new ArrayList<Change>(),
new HashMap<Change.Id, CodeReviewCommit>()); new HashMap<Change.Id, CodeReviewCommit>(),
currentUser.getAccount());
subOp.update(); subOp.update();
} catch (InsertException e) { } catch (InsertException e) {
log.error("Can't insert patchset", e); log.error("Can't insert patchset", e);

View File

@@ -14,7 +14,9 @@
package com.google.gerrit.server.git; package com.google.gerrit.server.git;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
@@ -67,7 +69,7 @@ public class SubmoduleOp {
public interface Factory { public interface Factory {
SubmoduleOp create(Branch.NameKey destBranch, RevCommit mergeTip, SubmoduleOp create(Branch.NameKey destBranch, RevCommit mergeTip,
RevWalk rw, Repository db, Project destProject, List<Change> submitted, RevWalk rw, Repository db, Project destProject, List<Change> submitted,
Map<Change.Id, CodeReviewCommit> commits); Map<Change.Id, CodeReviewCommit> commits, Account account);
} }
private static final Logger log = LoggerFactory.getLogger(SubmoduleOp.class); private static final Logger log = LoggerFactory.getLogger(SubmoduleOp.class);
@@ -87,6 +89,8 @@ public class SubmoduleOp {
private final GitReferenceUpdated gitRefUpdated; private final GitReferenceUpdated gitRefUpdated;
private final SchemaFactory<ReviewDb> schemaFactory; private final SchemaFactory<ReviewDb> schemaFactory;
private final Set<Branch.NameKey> updatedSubscribers; private final Set<Branch.NameKey> updatedSubscribers;
private final Account account;
private final ChangeHooks changeHooks;
@Inject @Inject
public SubmoduleOp(@Assisted final Branch.NameKey destBranch, public SubmoduleOp(@Assisted final Branch.NameKey destBranch,
@@ -96,7 +100,8 @@ public class SubmoduleOp {
@Assisted Project destProject, @Assisted List<Change> submitted, @Assisted Project destProject, @Assisted List<Change> submitted,
@Assisted final Map<Change.Id, CodeReviewCommit> commits, @Assisted final Map<Change.Id, CodeReviewCommit> commits,
@GerritPersonIdent final PersonIdent myIdent, @GerritPersonIdent final PersonIdent myIdent,
GitRepositoryManager repoManager, GitReferenceUpdated gitRefUpdated) { GitRepositoryManager repoManager, GitReferenceUpdated gitRefUpdated,
@Assisted Account account, ChangeHooks changeHooks) {
this.destBranch = destBranch; this.destBranch = destBranch;
this.mergeTip = mergeTip; this.mergeTip = mergeTip;
this.rw = rw; this.rw = rw;
@@ -109,6 +114,8 @@ public class SubmoduleOp {
this.myIdent = myIdent; this.myIdent = myIdent;
this.repoManager = repoManager; this.repoManager = repoManager;
this.gitRefUpdated = gitRefUpdated; this.gitRefUpdated = gitRefUpdated;
this.account = account;
this.changeHooks = changeHooks;
updatedSubscribers = new HashSet<Branch.NameKey>(); updatedSubscribers = new HashSet<Branch.NameKey>();
} }
@@ -341,6 +348,7 @@ public class SubmoduleOp {
case NEW: case NEW:
case FAST_FORWARD: case FAST_FORWARD:
gitRefUpdated.fire(subscriber.getParentKey(), rfu); gitRefUpdated.fire(subscriber.getParentKey(), rfu);
changeHooks.doRefUpdatedHook(subscriber, rfu, account);
// TODO since this is performed "in the background" no mail will be // TODO since this is performed "in the background" no mail will be
// sent to inform users about the updated branch // sent to inform users about the updated branch
break; break;

View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.git; package com.google.gerrit.server.git;
import static org.easymock.EasyMock.capture; import static org.easymock.EasyMock.capture;
import static org.easymock.EasyMock.createNiceMock;
import static org.easymock.EasyMock.createStrictMock; import static org.easymock.EasyMock.createStrictMock;
import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.eq;
import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.expect;
@@ -22,6 +23,7 @@ import static org.easymock.EasyMock.replay;
import static org.easymock.EasyMock.verify; import static org.easymock.EasyMock.verify;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
@@ -79,6 +81,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
private Provider<String> urlProvider; private Provider<String> urlProvider;
private GitRepositoryManager repoManager; private GitRepositoryManager repoManager;
private GitReferenceUpdated gitRefUpdated; private GitReferenceUpdated gitRefUpdated;
private ChangeHooks changeHooks;
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
@Override @Override
@@ -92,6 +95,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
urlProvider = createStrictMock(Provider.class); urlProvider = createStrictMock(Provider.class);
repoManager = createStrictMock(GitRepositoryManager.class); repoManager = createStrictMock(GitRepositoryManager.class);
gitRefUpdated = createStrictMock(GitReferenceUpdated.class); gitRefUpdated = createStrictMock(GitReferenceUpdated.class);
changeHooks = createNiceMock(ChangeHooks.class);
} }
private void doReplay() { private void doReplay() {
@@ -138,7 +142,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
final SubmoduleOp submoduleOp = final SubmoduleOp submoduleOp =
new SubmoduleOp(branchNameKey, mergeTip, new RevWalk(realDb), urlProvider, new SubmoduleOp(branchNameKey, mergeTip, new RevWalk(realDb), urlProvider,
schemaFactory, realDb, null, new ArrayList<Change>(), null, null, schemaFactory, realDb, null, new ArrayList<Change>(), null, null,
null, null); null, null, null, null);
submoduleOp.update(); submoduleOp.update();
@@ -665,7 +669,8 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk( new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk(
sourceRepository), urlProvider, schemaFactory, sourceRepository, sourceRepository), urlProvider, schemaFactory, sourceRepository,
new Project(sourceBranchNameKey.getParentKey()), submitted, new Project(sourceBranchNameKey.getParentKey()), submitted,
mergedCommits, myIdent, repoManager, gitRefUpdated); mergedCommits, myIdent, repoManager, gitRefUpdated, null,
changeHooks);
submoduleOp.update(); submoduleOp.update();
@@ -770,7 +775,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk( new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk(
sourceRepository), urlProvider, schemaFactory, sourceRepository, sourceRepository), urlProvider, schemaFactory, sourceRepository,
new Project(sourceBranchNameKey.getParentKey()), submitted, new Project(sourceBranchNameKey.getParentKey()), submitted,
mergedCommits, myIdent, repoManager, gitRefUpdated); mergedCommits, myIdent, repoManager, gitRefUpdated, null, changeHooks);
submoduleOp.update(); submoduleOp.update();
@@ -927,7 +932,7 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
new SubmoduleOp(mergedBranch, mergeTip, new RevWalk(realDb), new SubmoduleOp(mergedBranch, mergeTip, new RevWalk(realDb),
urlProvider, schemaFactory, realDb, new Project(mergedBranch urlProvider, schemaFactory, realDb, new Project(mergedBranch
.getParentKey()), new ArrayList<Change>(), null, null, .getParentKey()), new ArrayList<Change>(), null, null,
repoManager, null); repoManager, null, null, null);
submoduleOp.update(); submoduleOp.update();
} }