Add oldObjectId and newObjectId to the GitReferenceUpdatedListener.Update

This is a first step towards creation of the "reviewnotes" plugin and
removing this code from the Gerrit core. The reviewnotes plugin will
listen on GitReferenceUpdated and will need old and new values in order
to know which commits become part of the changed refs history.

Change-Id: If5f3598adb209a9b473c5ea4a07bf8a95f838139
Signed-off-by: Sasa Zivkov <sasa.zivkov@sap.com>
This commit is contained in:
Sasa Zivkov 2012-12-06 10:27:00 +01:00 committed by Edwin Kempin
parent 57462b7766
commit c21f309f71
15 changed files with 69 additions and 25 deletions

View File

@ -23,6 +23,8 @@ import java.util.List;
public interface GitReferenceUpdatedListener { public interface GitReferenceUpdatedListener {
public interface Update { public interface Update {
String getRefName(); String getRefName();
String getOldObjectId();
String getNewObjectId();
} }
public interface Event { public interface Event {

View File

@ -145,7 +145,7 @@ class AddBranch extends Handler<AddBranchResult> {
case FAST_FORWARD: case FAST_FORWARD:
case NEW: case NEW:
case NO_CHANGE: case NO_CHANGE:
referenceUpdated.fire(name.getParentKey(), refname); referenceUpdated.fire(name.getParentKey(), u);
hooks.doRefUpdatedHook(name, u, identifiedUser.getAccount()); hooks.doRefUpdatedHook(name, u, identifiedUser.getAccount());
break; break;
case LOCK_FAILURE: case LOCK_FAILURE:

View File

@ -121,7 +121,7 @@ class DeleteBranches extends Handler<Set<Branch.NameKey>> {
case FAST_FORWARD: case FAST_FORWARD:
case FORCED: case FORCED:
deleted.add(branchKey); deleted.add(branchKey);
replication.fire(projectName, refname); replication.fire(projectName, u);
hooks.doRefUpdatedHook(branchKey, u, identifiedUser.getAccount()); hooks.doRefUpdatedHook(branchKey, u, identifiedUser.getAccount());
break; break;

View File

@ -279,7 +279,7 @@ public class ChangeUtil {
"Failed to create ref %s in %s: %s", ps.getRefName(), "Failed to create ref %s in %s: %s", ps.getRefName(),
change.getDest().getParentKey().get(), ru.getResult())); change.getDest().getParentKey().get(), ru.getResult()));
} }
replication.fire(change.getProject(), ru.getName()); replication.fire(change.getProject(), ru);
db.changes().beginTransaction(change.getId()); db.changes().beginTransaction(change.getId());
try { try {
@ -397,7 +397,7 @@ public class ChangeUtil {
"Failed to create ref %s in %s: %s", newPatchSet.getRefName(), "Failed to create ref %s in %s: %s", newPatchSet.getRefName(),
change.getDest().getParentKey().get(), ru.getResult())); change.getDest().getParentKey().get(), ru.getResult()));
} }
replication.fire(change.getProject(), ru.getName()); replication.fire(change.getProject(), ru);
db.changes().beginTransaction(change.getId()); db.changes().beginTransaction(change.getId());
try { try {
@ -507,7 +507,7 @@ public class ChangeUtil {
throw new IOException("Failed to delete ref " + patch.getRefName() + throw new IOException("Failed to delete ref " + patch.getRefName() +
" in " + repo.getDirectory() + ": " + update.getResult()); " in " + repo.getDirectory() + ": " + update.getResult());
} }
replication.fire(change.getProject(), update.getName()); replication.fire(change.getProject(), update);
} finally { } finally {
repo.close(); repo.close();
} }

View File

@ -333,7 +333,7 @@ public class RebaseChange {
newPatchSet.getRefName(), change.getDest().getParentKey().get(), newPatchSet.getRefName(), change.getDest().getParentKey().get(),
ru.getResult())); ru.getResult()));
} }
replication.fire(change.getProject(), ru.getName()); replication.fire(change.getProject(), ru);
db.changes().beginTransaction(change.getId()); db.changes().beginTransaction(change.getId());
try { try {

View File

@ -20,6 +20,9 @@ import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.inject.Inject; import com.google.inject.Inject;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.RefUpdate;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
@ -38,8 +41,16 @@ public class GitReferenceUpdated {
this.listeners = listeners; this.listeners = listeners;
} }
public void fire(Project.NameKey project, String ref) { public void fire(Project.NameKey project, RefUpdate refUpdate) {
Event event = new Event(project, ref); fire(project, refUpdate.getName(), refUpdate.getOldObjectId(),
refUpdate.getNewObjectId());
}
public void fire(Project.NameKey project, String ref,
ObjectId oldObjectId, ObjectId newObjectId) {
ObjectId o = oldObjectId != null ? oldObjectId : ObjectId.zeroId();
ObjectId n = newObjectId != null ? newObjectId : ObjectId.zeroId();
Event event = new Event(project, ref, o.name(), n.name());
for (GitReferenceUpdatedListener l : listeners) { for (GitReferenceUpdatedListener l : listeners) {
l.onGitReferenceUpdated(event); l.onGitReferenceUpdated(event);
} }
@ -48,10 +59,15 @@ public class GitReferenceUpdated {
private static class Event implements GitReferenceUpdatedListener.Event { private static class Event implements GitReferenceUpdatedListener.Event {
private final String projectName; private final String projectName;
private final String ref; private final String ref;
private final String oldObjectId;
private final String newObjectId;
Event(Project.NameKey project, String ref) { Event(Project.NameKey project, String ref,
String oldObjectId, String newObjectId) {
this.projectName = project.get(); this.projectName = project.get();
this.ref = ref; this.ref = ref;
this.oldObjectId = oldObjectId;
this.newObjectId = newObjectId;
} }
@Override @Override
@ -63,9 +79,20 @@ public class GitReferenceUpdated {
public List<GitReferenceUpdatedListener.Update> getUpdates() { public List<GitReferenceUpdatedListener.Update> getUpdates() {
GitReferenceUpdatedListener.Update update = GitReferenceUpdatedListener.Update update =
new GitReferenceUpdatedListener.Update() { new GitReferenceUpdatedListener.Update() {
@Override
public String getRefName() { public String getRefName() {
return ref; return ref;
} }
@Override
public String getOldObjectId() {
return oldObjectId;
}
@Override
public String getNewObjectId() {
return newObjectId;
}
}; };
return ImmutableList.of(update); return ImmutableList.of(update);
} }

View File

@ -202,7 +202,7 @@ public class CherryPick extends SubmitStrategy {
ru.getResult())); ru.getResult()));
} }
replication.fire(n.change.getProject(), ru.getName()); replication.fire(n.change.getProject(), ru);
newCommit.copyFrom(n); newCommit.copyFrom(n);
newCommit.statusCode = CommitMergeStatus.CLEAN_PICK; newCommit.statusCode = CommitMergeStatus.CLEAN_PICK;

View File

@ -686,7 +686,7 @@ public class MergeOp {
destProject.getProject().getDescription()); destProject.getProject().getDescription());
} }
replication.fire(destBranch.getParentKey(), branchUpdate.getName()); replication.fire(destBranch.getParentKey(), branchUpdate);
Account account = null; Account account = null;
final PatchSetApproval submitter = getSubmitter(db, mergeTip.patchsetId); final PatchSetApproval submitter = getSubmitter(db, mergeTip.patchsetId);

View File

@ -24,6 +24,7 @@ import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
import java.io.IOException; import java.io.IOException;
@ -129,7 +130,7 @@ public class MetaDataUpdate {
return commit; return commit;
} }
void replicate(String ref) { void replicate(RefUpdate ru) {
replication.fire(projectName, ref); replication.fire(projectName, ru);
} }
} }

View File

@ -261,7 +261,7 @@ public class NotesBranchUtil {
throw new IOException("Couldn't update " + notesBranch + ". " throw new IOException("Couldn't update " + notesBranch + ". "
+ result.name()); + result.name());
} else { } else {
gitRefUpdated.fire(project, notesBranch); gitRefUpdated.fire(project, refUpdate);
break; break;
} }
} }

View File

@ -589,7 +589,8 @@ public class ReceiveCommits {
// We only schedule direct refs updates for replication. // We only schedule direct refs updates for replication.
// Change refs are scheduled when they are created. // Change refs are scheduled when they are created.
// //
replication.fire(project.getNameKey(), c.getRefName()); replication.fire(project.getNameKey(), c.getRefName(),
c.getOldId(), c.getNewId());
hooks.doRefUpdatedHook( hooks.doRefUpdatedHook(
new Branch.NameKey(project.getNameKey(), c.getRefName()), new Branch.NameKey(project.getNameKey(), c.getRefName()),
c.getOldId(), c.getOldId(),
@ -1383,7 +1384,8 @@ public class ReceiveCommits {
} }
created = true; created = true;
replication.fire(project.getNameKey(), ps.getRefName()); replication.fire(project.getNameKey(), ps.getRefName(),
ObjectId.zeroId(), commit);
hooks.doPatchsetCreatedHook(change, ps, db); hooks.doPatchsetCreatedHook(change, ps, db);
workQueue.getDefaultQueue() workQueue.getDefaultQueue()
.submit(requestScopePropagator.wrap(new Runnable() { .submit(requestScopePropagator.wrap(new Runnable() {
@ -1745,7 +1747,8 @@ public class ReceiveCommits {
if (cmd.getResult() == NOT_ATTEMPTED) { if (cmd.getResult() == NOT_ATTEMPTED) {
cmd.execute(rp); cmd.execute(rp);
} }
replication.fire(project.getNameKey(), newPatchSet.getRefName()); replication.fire(project.getNameKey(), newPatchSet.getRefName(),
ObjectId.zeroId(), newCommit);
hooks.doPatchsetCreatedHook(change, newPatchSet, db); hooks.doPatchsetCreatedHook(change, newPatchSet, db);
if (mergedIntoRef != null) { if (mergedIntoRef != null) {
hooks.doChangeMergedHook( hooks.doChangeMergedHook(

View File

@ -337,7 +337,7 @@ public class SubmoduleOp {
switch (rfu.update()) { switch (rfu.update()) {
case NEW: case NEW:
case FAST_FORWARD: case FAST_FORWARD:
replication.fire(subscriber.getParentKey(), rfu.getName()); replication.fire(subscriber.getParentKey(), rfu);
// 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

@ -260,7 +260,7 @@ public abstract class VersionedMetaData {
switch (result) { switch (result) {
case NEW: case NEW:
revision = rw.parseCommit(ru.getNewObjectId()); revision = rw.parseCommit(ru.getNewObjectId());
update.replicate(ru.getName()); update.replicate(ru);
return revision; return revision;
default: default:
throw new IOException("Cannot update " + ru.getName() + " in " throw new IOException("Cannot update " + ru.getName() + " in "
@ -293,7 +293,7 @@ public abstract class VersionedMetaData {
case NEW: case NEW:
case FAST_FORWARD: case FAST_FORWARD:
revision = rw.parseCommit(ru.getNewObjectId()); revision = rw.parseCommit(ru.getNewObjectId());
update.replicate(ru.getName()); update.replicate(ru);
return revision; return revision;
default: default:

View File

@ -283,7 +283,7 @@ public class CreateProject {
final Result result = ru.update(); final Result result = ru.update();
switch (result) { switch (result) {
case NEW: case NEW:
referenceUpdated.fire(project, ref); referenceUpdated.fire(project, ru);
break; break;
default: { default: {
throw new IOException(String.format( throw new IOException(String.format(

View File

@ -14,10 +14,13 @@
package com.google.gerrit.server.git; package com.google.gerrit.server.git;
import static org.easymock.EasyMock.capture;
import static org.easymock.EasyMock.createStrictMock; import static org.easymock.EasyMock.createStrictMock;
import static org.easymock.EasyMock.eq;
import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay; 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 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;
@ -34,6 +37,7 @@ import com.google.gwtorm.server.SchemaFactory;
import com.google.gwtorm.server.StandardKeyEncoder; import com.google.gwtorm.server.StandardKeyEncoder;
import com.google.inject.Provider; import com.google.inject.Provider;
import org.easymock.Capture;
import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.dircache.DirCacheBuilder; import org.eclipse.jgit.dircache.DirCacheBuilder;
import org.eclipse.jgit.dircache.DirCacheEntry; import org.eclipse.jgit.dircache.DirCacheEntry;
@ -43,6 +47,7 @@ import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.revwalk.RevWalk;
@ -639,8 +644,9 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
expect(repoManager.openRepository(targetBranchNameKey.getParentKey())) expect(repoManager.openRepository(targetBranchNameKey.getParentKey()))
.andReturn(targetRepository); .andReturn(targetRepository);
replication.fire(targetBranchNameKey.getParentKey(), Capture<RefUpdate> ruCapture = new Capture<RefUpdate>();
targetBranchNameKey.get()); replication.fire(eq(targetBranchNameKey.getParentKey()),
capture(ruCapture));
expect(schema.submoduleSubscriptions()).andReturn(subscriptions); expect(schema.submoduleSubscriptions()).andReturn(subscriptions);
final ResultSet<SubmoduleSubscription> emptySubscriptions = final ResultSet<SubmoduleSubscription> emptySubscriptions =
@ -664,6 +670,8 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
submoduleOp.update(); submoduleOp.update();
doVerify(); doVerify();
RefUpdate ru = ruCapture.getValue();
assertEquals(ru.getName(), targetBranchNameKey.get());
} }
/** /**
@ -740,8 +748,9 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
expect(repoManager.openRepository(targetBranchNameKey.getParentKey())) expect(repoManager.openRepository(targetBranchNameKey.getParentKey()))
.andReturn(targetRepository); .andReturn(targetRepository);
replication.fire(targetBranchNameKey.getParentKey(), Capture<RefUpdate> ruCapture = new Capture<RefUpdate>();
targetBranchNameKey.get()); replication.fire(eq(targetBranchNameKey.getParentKey()),
capture(ruCapture));
expect(schema.submoduleSubscriptions()).andReturn(subscriptions); expect(schema.submoduleSubscriptions()).andReturn(subscriptions);
final ResultSet<SubmoduleSubscription> incorrectSubscriptions = final ResultSet<SubmoduleSubscription> incorrectSubscriptions =
@ -767,6 +776,8 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
submoduleOp.update(); submoduleOp.update();
doVerify(); doVerify();
RefUpdate ru = ruCapture.getValue();
assertEquals(ru.getName(), targetBranchNameKey.get());
} }
/** /**