Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix DraftPublished and ChangeMerged -> Replication
The Draft Published event is now handled correctly by ensuring that
Gerrit Trigger does not wait for a replicaton event from the slave.

The Change Merged event is now handled correctly by ensuring that we
wait for the replication event for the included patchset on the slave.

These use cases were covered:
- The user merges his/her change to master and there is no change in SHA1
  -- The replication has already occurred.
- The user merges his/her change to master and there is a change in SHA1
since there already other changes merged after the patchset was created.
  -- a new replication takes place and must be waited for.

[FIXED JENKINS-25047]
  • Loading branch information
escoheb committed Oct 14, 2014
1 parent bc9812e commit 8c7d0e8
Show file tree
Hide file tree
Showing 5 changed files with 277 additions and 86 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -62,7 +62,7 @@
<dependency>
<groupId>com.sonymobile.tools.gerrit</groupId>
<artifactId>gerrit-events</artifactId>
<version>2.2.0</version>
<version>2.4.0</version>
<!-- New source is here: https://github.com/sonyxperiadev/gerrit-events -->
</dependency>
<dependency>
Expand Down
Expand Up @@ -31,6 +31,7 @@
import hudson.model.queue.QueueTaskDispatcher;
import hudson.model.queue.CauseOfBlockage;

import java.util.Date;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
Expand All @@ -45,7 +46,10 @@
import com.sonymobile.tools.gerrit.gerritevents.GerritHandler;
import com.sonymobile.tools.gerrit.gerritevents.dto.GerritEvent;
import com.sonymobile.tools.gerrit.gerritevents.dto.RepositoryModifiedEvent;
import com.sonymobile.tools.gerrit.gerritevents.dto.attr.PatchSet;
import com.sonymobile.tools.gerrit.gerritevents.dto.events.ChangeBasedEvent;
import com.sonymobile.tools.gerrit.gerritevents.dto.events.RefReplicated;
import com.sonymobile.tools.gerrit.gerritevents.dto.events.RefUpdated;
import com.sonyericsson.hudson.plugins.gerrit.trigger.Messages;
import com.sonyericsson.hudson.plugins.gerrit.trigger.PluginImpl;
import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.GerritCause;
Expand All @@ -69,9 +73,9 @@ public class ReplicationQueueTaskDispatcher extends QueueTaskDispatcher implemen
*/
public ReplicationQueueTaskDispatcher() {
this(PluginImpl.getInstance().getHandler(),
ReplicationCache.Factory.createCache(
PluginImpl.getInstance().getPluginConfig().getReplicationCacheExpirationInMinutes(),
TimeUnit.MINUTES));
ReplicationCache.Factory.createCache(
PluginImpl.getInstance().getPluginConfig().getReplicationCacheExpirationInMinutes(),
TimeUnit.MINUTES));
}

/**
Expand Down Expand Up @@ -106,7 +110,7 @@ public CauseOfBlockage canRun(Item item) {
return null;
} else {
logger.trace("item id {} is still waiting replication to {} gerrit slaves, ", itemId,
blockedItem.slavesWaitingFor.size());
blockedItem.slavesWaitingFor.size());
return new WaitingForReplication(blockedItem.slavesWaitingFor.values());
}
} else {
Expand All @@ -131,11 +135,11 @@ private void updateFromReplicationCache(BlockedItem blockedItem) {
Iterator<GerritSlave> it = blockedItem.slavesWaitingFor.values().iterator();
while (it.hasNext()) {
RefReplicated refReplicated = replicationCache.getIfPresent(blockedItem.gerritServer,
blockedItem.gerritProject, blockedItem.ref, it.next().getHost());
blockedItem.gerritProject, blockedItem.ref, it.next().getHost());
if (refReplicated != null) {
blockedItem.processRefReplicatedEvent(refReplicated);
logger.trace("processed a replication event from the cache, remaining number of events waiting for: "
+ blockedItem.slavesWaitingFor.size());
+ blockedItem.slavesWaitingFor.size());
}
}
}
Expand All @@ -151,7 +155,7 @@ private BlockedItem getBlockedItem(Item item) {
return null;
}
if (gerritCause.getEvent() != null && gerritCause.getEvent() instanceof RepositoryModifiedEvent
&& item.task instanceof AbstractProject<?, ?>) {
&& item.task instanceof AbstractProject<?, ?>) {

if (replicationCache.isExpired(gerritCause.getEvent().getReceivedOn())) {
return null;
Expand All @@ -173,14 +177,38 @@ private BlockedItem getBlockedItem(Item item) {
List<GerritSlave> slaves = gerritTrigger.gerritSlavesToWaitFor(gerritServer);
if (!slaves.isEmpty()) {
RepositoryModifiedEvent repositoryModifiedEvent = (RepositoryModifiedEvent)gerritCause.getEvent();
if (repositoryModifiedEvent.getModifiedProject() != null
&& repositoryModifiedEvent.getModifiedRef() != null) {
return new BlockedItem(repositoryModifiedEvent.getModifiedProject(),

if (repositoryModifiedEvent.getModifiedProject() == null
|| repositoryModifiedEvent.getModifiedRef() == null) {
return null;
}

Date createdOnDate = null;
if (repositoryModifiedEvent instanceof ChangeBasedEvent) {
PatchSet patchset = ((ChangeBasedEvent)repositoryModifiedEvent).getPatchSet();
createdOnDate = patchset.getCreatedOn();
}

if (createdOnDate != null && replicationCache.isExpired(createdOnDate.getTime())) {
return null;
}

boolean useTimestampWhenProcessingRefReplicatedEvent = false;
// we only need to perform a timestamp check if
// we are looking at a RefUpdated event.
// The reason for this is due to the fact that the ref
// is not unique for RefUpdated events and we therefore
// *need* to compare timestamps to ensure we use the
// correct event.
if (gerritCause.getEvent() instanceof RefUpdated) {
useTimestampWhenProcessingRefReplicatedEvent = true;
}
return new BlockedItem(repositoryModifiedEvent.getModifiedProject(),
repositoryModifiedEvent.getModifiedRef(),
gerritServer,
slaves,
gerritCause.getEvent().getReceivedOn());
}
gerritCause.getEvent().getReceivedOn(),
useTimestampWhenProcessingRefReplicatedEvent);
}
}
return null;
Expand All @@ -202,7 +230,7 @@ private GerritCause getGerritCause(Item item) {

@Override
public void gerritEvent(GerritEvent event) {
//not interested in the other events, only RefReplicated
//not interested in the other events, only RefReplicated
}

/**
Expand Down Expand Up @@ -238,6 +266,7 @@ private static class BlockedItem {
private boolean canRun = false;
private long eventTimeStamp;
private String replicationFailedMessage;
private boolean useTimestampWhenProcessingRefReplicatedEvent = false;

/**
* Standard constructor.
Expand All @@ -246,9 +275,11 @@ private static class BlockedItem {
* @param gerritServer The gerrit server
* @param gerritSlaves The gerrit slaves
* @param eventTimeStamp The original event time stamp.
* @param useTimestampWhenProcessingRefReplicatedEvent Enable use of timestamp for deciding to
* process refreplicated event.
*/
public BlockedItem(String gerritProject, String ref, String gerritServer, List<GerritSlave> gerritSlaves,
long eventTimeStamp) {
long eventTimeStamp, boolean useTimestampWhenProcessingRefReplicatedEvent) {
this.gerritProject = gerritProject;
this.ref = ref;
this.gerritServer = gerritServer;
Expand All @@ -257,6 +288,7 @@ public BlockedItem(String gerritProject, String ref, String gerritServer, List<G
slavesWaitingFor.put(gerritSlave.getHost(), gerritSlave);
}
this.eventTimeStamp = eventTimeStamp;
this.useTimestampWhenProcessingRefReplicatedEvent = useTimestampWhenProcessingRefReplicatedEvent;
}

/**
Expand All @@ -275,7 +307,7 @@ public boolean canRunWithTimeoutCheck() {
// check if any Gerrit Slave reached its timeout
for (GerritSlave slave : slavesWaitingFor.values()) {
if (slave.getTimeoutInSeconds() != GerritSlave.DISABLED_TIMEOUT_VALUE
&& TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis() - eventTimeStamp) > slave
&& TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis() - eventTimeStamp) > slave
.getTimeoutInSeconds()) {
replicationFailedMessage = Messages.WaitingForReplicationTimeout(ref, slave.getName());
return true;
Expand All @@ -294,13 +326,17 @@ public void processRefReplicatedEvent(RefReplicated refReplicated) {
return;
}
if (gerritProject.equals(refReplicated.getProject())
&& gerritServer.equals(refReplicated.getProvider().getName())
&& ref.equals(refReplicated.getRef())
&& slavesWaitingFor.containsKey(refReplicated.getTargetNode())
&& eventTimeStamp < refReplicated.getReceivedOn()) {
&& gerritServer.equals(refReplicated.getProvider().getName())
&& ref.equals(refReplicated.getRef())
&& slavesWaitingFor.containsKey(refReplicated.getTargetNode())) {

if (useTimestampWhenProcessingRefReplicatedEvent
&& (!(eventTimeStamp < refReplicated.getReceivedOn()))) {
return;
}
if (refReplicated.getStatus().equals(RefReplicated.FAILED_STATUS)) {
replicationFailedMessage = Messages.ReplicationFailed(ref,
slavesWaitingFor.get(refReplicated.getTargetNode()).getName());
slavesWaitingFor.get(refReplicated.getTargetNode()).getName());
slavesWaitingFor.clear();
} else {
slavesWaitingFor.remove(refReplicated.getTargetNode());
Expand Down
Expand Up @@ -275,7 +275,7 @@ public void testGetBuildCompletedCommandSuccessful() throws IOException, Interru
"\n\nhttp://localhost/test/console : SUCCESS");
}

/**
/**
* Same test as {@link #testGetBuildCompletedCommandSuccessful()}, but with ChangeAbandoned event instead.
*
* @throws IOException IOException
Expand Down Expand Up @@ -357,7 +357,8 @@ public void tryGetBuildCompletedCommandSuccessfulChangeAbandoned(String customUr
*/
public void tryGetBuildCompletedCommandSuccessfulChangeMerged(String customUrl, String expectedBuildsStats)
throws IOException, InterruptedException {
tryGetBuildCompletedCommandSuccessfulEvent(customUrl, expectedBuildsStats, Setup.createChangeMerged(), 0, 0);
tryGetBuildCompletedCommandSuccessfulEvent(customUrl, expectedBuildsStats,
Setup.createChangeMerged(), 0, 0);
}

/**
Expand Down Expand Up @@ -386,9 +387,9 @@ public void tryGetBuildCompletedCommandSuccessfulChangeRestored(String customUrl
* @throws InterruptedException if so.
*/
public void tryGetBuildCompletedCommandSuccessfulEvent(String customUrl, String expectedBuildsStats,
GerritTriggeredEvent event, int expectedVerifiedVote,
int expectedCodeReviewVote)
throws IOException, InterruptedException {
GerritTriggeredEvent event, int expectedVerifiedVote,
int expectedCodeReviewVote)
throws IOException, InterruptedException {

IGerritHudsonTriggerConfig config = Setup.createConfig();

Expand Down
Expand Up @@ -29,13 +29,15 @@
import com.sonymobile.tools.gerrit.gerritevents.dto.attr.Change;
import com.sonymobile.tools.gerrit.gerritevents.dto.attr.PatchSet;
import com.sonymobile.tools.gerrit.gerritevents.dto.attr.Provider;
import com.sonymobile.tools.gerrit.gerritevents.dto.attr.RefUpdate;
import com.sonymobile.tools.gerrit.gerritevents.dto.events.ChangeAbandoned;
import com.sonymobile.tools.gerrit.gerritevents.dto.events.ChangeMerged;
import com.sonymobile.tools.gerrit.gerritevents.dto.events.ChangeRestored;
import com.sonymobile.tools.gerrit.gerritevents.dto.events.CommentAdded;
import com.sonymobile.tools.gerrit.gerritevents.dto.events.DraftPublished;
import com.sonymobile.tools.gerrit.gerritevents.dto.events.PatchsetCreated;
import com.sonymobile.tools.gerrit.gerritevents.dto.events.RefReplicated;
import com.sonymobile.tools.gerrit.gerritevents.dto.events.RefUpdated;
import com.sonyericsson.hudson.plugins.gerrit.trigger.VerdictCategory;
import com.sonyericsson.hudson.plugins.gerrit.trigger.PluginImpl;
import com.sonyericsson.hudson.plugins.gerrit.trigger.events.ManualPatchsetCreated;
Expand All @@ -60,6 +62,7 @@

import java.io.IOException;
import java.util.Collections;
import java.util.Date;
import java.util.LinkedList;
import java.util.List;

Expand Down Expand Up @@ -121,6 +124,30 @@ public static PatchsetCreated createPatchsetCreated(String serverName) {
return createPatchsetCreated(serverName, "project", "ref");
}

/**
* Create a new RefUpdated event with given data.
* @param serverName The server name.
* @param project The project.
* @param ref The ref.
* @return a RefUpdated event
*/
public static RefUpdated createRefUpdated(String serverName, String project, String ref) {
RefUpdated event = new RefUpdated();
Account account = new Account();
account.setEmail("email@domain.com");
account.setName("Name");
event.setAccount(account);
event.setProvider(new Provider(serverName, "gerrit", "29418", "ssh", "http://gerrit/", "1"));

RefUpdate refUpdate = new RefUpdate();
refUpdate.setNewRev("2");
refUpdate.setOldRev("1");
refUpdate.setProject(project);
refUpdate.setRefName(ref);
event.setRefUpdate(refUpdate);

return event;
}
/**
* Create a new patchset created event with the given data.
* @param serverName The server name
Expand Down Expand Up @@ -191,6 +218,8 @@ public static DraftPublished createDraftPublished() {
patch.setNumber("1");
patch.setRevision("9999");
event.setPatchset(patch);
patch.setRef("ref");
event.setProvider(new Provider(PluginImpl.DEFAULT_SERVER_NAME, "gerrit", "29418", "ssh", "http://gerrit/", "1"));
return event;
}

Expand Down Expand Up @@ -219,6 +248,17 @@ public static ChangeAbandoned createChangeAbandoned() {
return event;
}

/**
* Gives you a ChangeMerged mock.
* @param serverName The server name
* @param project The project
* @param ref The ref
* @return ChangeMerged mock.
*/
public static ChangeMerged createChangeMerged(String serverName, String project, String ref) {
return createChangeMergedWithPatchSetDate(serverName, project, ref, new Date());
}

/**
* Gives you a ChangeMerged mock.
* @return ChangeMerged mock.
Expand All @@ -244,6 +284,38 @@ public static ChangeMerged createChangeMerged() {
return event;
}

/**
* Gives you a ChangeMerged mock.
* @param serverName The server name
* @param project The project
* @param ref The ref
* @param date The patchset's createdOn date
* @return ChangeMerged mock.
*/
public static ChangeMerged createChangeMergedWithPatchSetDate(String serverName, String project,
String ref, Date date) {
ChangeMerged event = new ChangeMerged();
Change change = new Change();
change.setBranch("branch");
change.setId("Iddaaddaa123456789");
change.setNumber("1000");
Account account = new Account();
account.setEmail("email@domain.com");
account.setName("Name");
change.setOwner(account);
change.setProject(project);
change.setSubject("subject");
change.setUrl("http://gerrit/1000");
event.setChange(change);
PatchSet patch = new PatchSet();
patch.setNumber("1");
patch.setRevision("9999");
event.setPatchset(patch);
patch.setRef(ref);
patch.setCreatedOn(date);
event.setProvider(new Provider(serverName, "gerrit", "29418", "ssh", "http://gerrit/", "1"));
return event;
}
/**
* Gives you a ChangeRestored mock.
* @return ChangeRestored mock.
Expand Down Expand Up @@ -503,9 +575,9 @@ public static MemoryImprint.Entry createAndSetupMemoryImprintEntry(GerritTrigger
* @see com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.GerritTrigger#getSkipVote()
*/
public static MemoryImprint.Entry createAndSetupMemoryImprintEntry(Result result,
int resultsCodeReviewVote,
int resultsVerifiedVote,
boolean shouldSkip) {
int resultsCodeReviewVote,
int resultsVerifiedVote,
boolean shouldSkip) {
GerritTrigger trigger = mock(GerritTrigger.class);
SkipVote skipVote = null;
if (result == Result.SUCCESS) {
Expand Down Expand Up @@ -572,7 +644,7 @@ public static AbstractBuild createBuild(AbstractProject project, TaskListener ta
* @return a RefReplicated event
*/
public static RefReplicated createRefReplicatedEvent(String project, String ref, String server, String slave,
String status) {
String status) {
RefReplicated refReplicated = new RefReplicated();
refReplicated.setProject(project);
refReplicated.setProvider(new Provider(server, null, null, null, null, null));
Expand Down

0 comments on commit 8c7d0e8

Please sign in to comment.