Skip to content

Commit

Permalink
Merge pull request #89 from dalvizu/JENKINS-30801-2
Browse files Browse the repository at this point in the history 
[Fixed JENKINS-30801]
  • Loading branch information
@dalvizu
dalvizu committed Nov 20, 2015
2 parents 11ce036 + fb49ded commit c05598d
Show file tree
Hide file tree
Showing 6 changed files with 206 additions and 58 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Expand Up @@ -17,3 +17,4 @@ checkstyle-cachefile
*.iml
*.ipr
*.iws

7 changes: 6 additions & 1 deletion pom.xml
View file
Expand Up @@ -153,7 +153,12 @@
<version>18.0</version>
<scope>provided</scope>
</dependency>

<dependency>
<groupId>org.jenkins-ci</groupId>
<artifactId>test-annotations</artifactId>
<version>1.1</version>
<scope>test</scope>
</dependency>
</dependencies>


Expand Down
85 changes: 45 additions & 40 deletions src/main/java/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineView.java
View file
Expand Up @@ -60,6 +60,7 @@
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -405,31 +406,20 @@ public int triggerManualBuild(final Integer upstreamBuildNumber, final String tr
}

/**
* @param triggerProjectName
* the triggerProjectName
* @return the number of re-tried build
*/
@JavaScriptMethod
public int retryBuild(final String triggerProjectName) {
LOGGER.fine("Retrying build again: " + triggerProjectName); //$NON-NLS-1$
final AbstractProject<?, ?> triggerProject = (AbstractProject<?, ?>) super.getJob(triggerProjectName);
triggerProject.scheduleBuild(new MyUserIdCause());

return triggerProject.getNextBuildNumber();
}

/**
* Re-run a project, passing in the CauseActions from the previous completed {@link Run} so
* that the new run will appear in the same pipeline.
*
* @param externalizableId
* the externalizableId
* the externalizableId of the Run. See {@link Run#getExternalizableId()}
* @return the number of re-run build
*/
@JavaScriptMethod
public int rerunBuild(final String externalizableId) {
LOGGER.fine("Running build again: " + externalizableId); //$NON-NLS-1$
LOGGER.info("Running build again: " + externalizableId); //$NON-NLS-1$
final AbstractBuild<?, ?> triggerBuild = (AbstractBuild<?, ?>) Run.fromExternalizableId(externalizableId);
final AbstractProject<?, ?> triggerProject = triggerBuild.getProject();
final Future<?> future = triggerProject.scheduleBuild2(triggerProject.getQuietPeriod(), new MyUserIdCause(),
removeUserIdCauseActions(triggerBuild.getActions()));
filterActions(triggerBuild.getActions()));

AbstractBuild<?, ?> result = triggerBuild;
try {
Expand Down Expand Up @@ -584,43 +574,58 @@ private static ParametersAction mergeParameters(final ParametersAction base, fin
return new ParametersAction(params.values().toArray(new ParameterValue[params.size()]));
}


/**
* Checks whether the given {@link Action} contains a reference to a {@link UserIdCause} object.
* Filter out the list of actions so that it only includes {@link ParametersAction} and
* CauseActions, removing the UserIdAction from the CauseAction's list of Causes.
*
* We want to include CauseAction because that includes upstream cause actions, which
* are inherited in downstream builds.
*
* @param buildAction
* the action to check.
* @return <code>true</code> if the action has a reference to a userId cause.
* We do not want to inherit the UserId cause, because the user initiating a retry may
* be different than the user who originated the upstream build, and so should be
* re-identified.
*
* We do not want to inherit any other CauseAction because that will result in duplicating
* actions from publishers, and builders from previous builds corrupting the retriggered build.
*
* @param actions
* a collection of build actions.
* @return a collection of build actions with all UserId causes removed.
*/
private boolean isUserIdCauseAction(final Action buildAction) {
boolean retval = false;
if (buildAction instanceof CauseAction) {
for (final Cause cause : ((CauseAction) buildAction).getCauses()) {
if (cause instanceof UserIdCause) {
retval = true;
break;
private List<Action> filterActions(final List<Action> actions) {
final List<Action> retval = new ArrayList<Action>();
for (final Action action : actions) {
if (action instanceof CauseAction) {
final CauseAction causeAction = (CauseAction) action;
filterOutUserIdCause(causeAction);
if (!causeAction.getCauses().isEmpty()) {
retval.add(causeAction);
}
} else if (action instanceof ParametersAction) {
retval.add(action);
}
}
return retval;
}

/**
* Removes any UserId cause action from the given actions collection. This is used by downstream builds that inherit upstream actions.
* The downstream build can be initiated by another user that is different from the user who initiated the upstream build, so the
* downstream build needs to remove the old user action inherited from upstream, and add its own.
* Filter out {@link UserIdCause} from the given {@link CauseAction}.
*
* @param actions
* a collection of build actions.
* @return a collection of build actions with all UserId causes removed.
* We want to do this because re-run will want to contribute its own
* {@link UserIdCause}, not copy it from the previous run.
*
* @param causeAction
* the causeAction to remove UserIdCause from
*/
private List<Action> removeUserIdCauseActions(final List<Action> actions) {
final List<Action> retval = new ArrayList<Action>();
for (final Action action : actions) {
if (!isUserIdCauseAction(action)) {
retval.add(action);
private void filterOutUserIdCause(CauseAction causeAction) {
final Iterator<Cause> it = causeAction.getCauses().iterator();
while (it.hasNext()) {
final Cause cause = it.next();
if (cause instanceof UserIdCause) {
it.remove();
}
}
return retval;
}

/**
Expand Down
6 changes: 0 additions & 6 deletions src/main/webapp/js/build-pipeline.js
View file
Expand Up @@ -81,12 +81,6 @@ BuildPipeline.prototype = {
buildPipeline.updateNextBuildAndShowProgress(id, data.responseObject(), dependencyIds);
});
},
retryBuild : function(id, triggerProjectName, dependencyIds) {
var buildPipeline = this;
buildPipeline.viewProxy.retryBuild(triggerProjectName, function(data){
buildPipeline.updateNextBuildAndShowProgress(id, data.responseObject(), dependencyIds);
});
},
rerunBuild : function(id, buildExternalizableId, dependencyIds) {
var buildPipeline = this;
buildPipeline.viewProxy.rerunBuild(buildExternalizableId, function(data){
Expand Down
126 changes: 116 additions & 10 deletions src/test/java/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest.java
View file
Expand Up @@ -24,33 +24,32 @@
*/
package au.com.centrumsystems.hudson.plugin.buildpipeline;

import hudson.model.Action;
import hudson.model.Cause;
import hudson.model.FreeStyleBuild;
import hudson.model.ItemGroup;
import hudson.model.TopLevelItem;
import hudson.Launcher;
import hudson.model.*;

This comment has been minimized.

Sign in to view

Copy link
@recena

recena Nov 24, 2015

Please, avoid to use wildcard in import statement.
import hudson.model.Cause.UpstreamCause;
import hudson.model.FreeStyleProject;
import hudson.model.Hudson;
import hudson.model.Job;
import hudson.model.Run;
import hudson.security.Permission;

import java.io.IOException;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import hudson.triggers.SCMTrigger;
import jenkins.model.Jenkins;

import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Bug;

import au.com.centrumsystems.hudson.plugin.buildpipeline.trigger.BuildPipelineTrigger;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.TestBuilder;
import org.jvnet.hudson.test.recipes.LocalData;

import static org.junit.Assert.*;
import static org.mockito.Mockito.mock;

/**
* Test Build Pipeline View
Expand Down Expand Up @@ -256,7 +255,6 @@ public void testGetBuildPipelineForm() throws Exception {
// Build project1
build1 = jenkins.buildAndAssertSuccess(project1);
jenkins.waitUntilNoActivity();

// Test a valid case
final BuildPipelineView testView = BuildPipelineViewFactory.getBuildPipelineView(bpViewName, bpViewTitle, new DownstreamProjectGridBuilder(proj1), noOfBuilds, false);

Expand Down Expand Up @@ -374,4 +372,112 @@ public ItemGroup<? extends TopLevelItem> getOwnerItemGroup() {
};
}
}

@Test
@Issue("JENKINS-30801")
public void testRetriggerSuccessfulBuild() throws Exception {
final FreeStyleProject upstreamBuild = jenkins.createFreeStyleProject("upstream");
final FreeStyleProject downstreamBuild = jenkins.createFreeStyleProject("downstream");
upstreamBuild.getPublishersList().add(new BuildPipelineTrigger("downstream", null));
downstreamBuild.getBuildersList().add(new TestBuilder()
{

This comment has been minimized.

Sign in to view

Copy link
@recena

recena Nov 24, 2015

Better downstreamBuild.getBuildersList().add(new TestBuilder() { (in the same line)
@Override
public boolean perform(AbstractBuild<?, ?> abstractBuild, Launcher launcher, BuildListener buildListener)
throws InterruptedException, IOException
{

This comment has been minimized.

Sign in to view

Copy link
@recena

recena Nov 24, 2015

Ditto
abstractBuild.addAction(new MockAction());
return true;
}
});

// Important; we must do this step to ensure that the dependency graphs
// are updated
Hudson.getInstance().rebuildDependencyGraph();

// mock the upstream build as being caused by SCM trigger
Cause mockScmTriggerCause = new SCMTrigger.SCMTriggerCause("mock");
upstreamBuild.scheduleBuild2(0, mockScmTriggerCause);
jenkins.waitUntilNoActivity();

// mock trigget the downstream build as being triggered by upstream
ParametersAction parametersAction = new ParametersAction(
Arrays.asList((ParameterValue)new StringParameterValue("foo", "bar")));
UpstreamCause upstreamCause = new hudson.model.Cause.UpstreamCause(
(Run<?, ?>) upstreamBuild.getLastBuild());
downstreamBuild.scheduleBuild2(0, upstreamCause, parametersAction);
jenkins.waitUntilNoActivity();

BuildPipelineView pipeline = BuildPipelineViewFactory.getBuildPipelineView("pipeline", "",
new DownstreamProjectGridBuilder(upstreamBuild.getFullName()), "1", false);

jenkins.getInstance().addView(pipeline);
assertNotNull(downstreamBuild.getLastBuild());
// re-run the build as if we clicked re-run in the UI
pipeline.rerunBuild(downstreamBuild.getLastBuild().getExternalizableId());
jenkins.waitUntilNoActivity();

// MockAction is not copied from one run to another
assertEquals(1, downstreamBuild.getLastBuild().getActions(MockAction.class).size());
// upstream cause copied
assertEquals(1, downstreamBuild.getLastBuild().getCauses().size());
// parametersAction copied
assertNotNull(downstreamBuild.getLastBuild().getAction(ParametersAction.class));
StringParameterValue stringParam = (StringParameterValue) downstreamBuild.getLastBuild()
.getAction(ParametersAction.class).getParameter("foo");
assertEquals("bar", stringParam.value);
assertEquals(upstreamCause, downstreamBuild.getLastBuild().getCauses().get(0));
assertEquals(mockScmTriggerCause, upstreamCause.getUpstreamCauses().get(0));
}

@Test
public void testFilterUserIdCause() throws Exception {
final FreeStyleProject upstreamBuild = jenkins.createFreeStyleProject("upstream");
final FreeStyleProject downstreamBuild = jenkins.createFreeStyleProject("downstream");
upstreamBuild.getPublishersList().add(new BuildPipelineTrigger("downstream", null));
// Important; we must do this step to ensure that the dependency graphs
// are updated
Hudson.getInstance().rebuildDependencyGraph();
Cause mockUserIdCause = mock(Cause.UserIdCause.class);
upstreamBuild.scheduleBuild2(0, mockUserIdCause);
jenkins.waitUntilNoActivity();
UpstreamCause upstreamCause = new hudson.model.Cause.UpstreamCause(
(Run<?, ?>) upstreamBuild.getLastBuild());
downstreamBuild.scheduleBuild2(0, upstreamCause);
jenkins.waitUntilNoActivity();

BuildPipelineView pipeline = BuildPipelineViewFactory.getBuildPipelineView("pipeline", "",
new DownstreamProjectGridBuilder(upstreamBuild.getFullName()), "1", false);
jenkins.getInstance().addView(pipeline);
assertNotNull(downstreamBuild.getLastBuild());
// re-run the build as if we clicked re-run in the UI
pipeline.rerunBuild(upstreamBuild.getLastBuild().getExternalizableId());
jenkins.waitUntilNoActivity();
assertEquals(2, upstreamBuild.getBuilds().size());
assertNotNull(upstreamBuild.getLastBuild().getCause(Cause.UserIdCause.class));
assertNotSame(upstreamBuild.getLastBuild().getCause(Cause.UserIdCause.class),
mockUserIdCause);
}

public static class MockAction implements Action, Serializable {

private static final long serialVersionUID = 5677631606354259250L;

@Override
public String getIconFileName()
{
return null;
}

@Override
public String getDisplayName()
{
return null;
}

@Override
public String getUrlName()
{
return null;
}
}
}
39 changes: 38 additions & 1 deletion ...a/au/com/centrumsystems/hudson/plugin/buildpipeline/trigger/BuildPipelineTriggerTest.java
View file
Expand Up @@ -47,6 +47,7 @@
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Bug;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;

import static org.junit.Assert.*;
Expand Down Expand Up @@ -220,7 +221,8 @@ public void testGetBuilderConfigDescriptors() throws Exception {

@Test
@Bug(22665)
public void testManualTriggerCause() throws Exception {
public void testManualTriggerCause() throws Exception
{

This comment has been minimized.

Sign in to view

Copy link
@recena

recena Nov 24, 2015

Unrelated change. Please, don't change the current coding style guide.
FreeStyleProject projectA = jenkins.createFreeStyleProject("A");
FreeStyleProject projectB = jenkins.createFreeStyleProject("B");
projectA.getPublishersList().add(new BuildPipelineTrigger("B", null));
Expand All @@ -239,6 +241,41 @@ public void testManualTriggerCause() throws Exception {
assertNotNull(cause);
//Check that cause is of core class Cause.UserIdCause and not MyUserIdCause
assertEquals(Cause.UserIdCause.class.getName(), cause.getClass().getName());
Cause.UpstreamCause upstreamCause = build.getCause(Cause.UpstreamCause.class);
assertNotNull(upstreamCause);
}

@Test
@Issue("JENKINS-24883")
public void testReRunBuildPipelineTrigger()
throws Exception
{
FreeStyleProject projectA = jenkins.createFreeStyleProject("A");
FreeStyleProject projectB = jenkins.createFreeStyleProject("B");
projectA.getPublishersList().add(new BuildPipelineTrigger("B", null));
jenkins.getInstance().rebuildDependencyGraph();

BuildPipelineView view = new BuildPipelineView("Pipeline", "Title", new DownstreamProjectGridBuilder("A"), "1", false, "");

jenkins.buildAndAssertSuccess(projectA);

view.triggerManualBuild(1, "B", "A");
jenkins.waitUntilNoActivity();

assertNotNull(projectB.getLastBuild());
FreeStyleBuild build = projectB.getLastBuild();
Cause.UserIdCause cause = build.getCause(Cause.UserIdCause.class);
assertNotNull(cause);
//Check that cause is of core class Cause.UserIdCause and not MyUserIdCause
assertEquals(Cause.UserIdCause.class.getName(), cause.getClass().getName());
Cause.UpstreamCause upstreamCause = build.getCause(Cause.UpstreamCause.class);
assertNotNull(upstreamCause);

// re-triggering the build should preserve upstream context (JENKINS-24883
view.rerunBuild(projectB.getLastBuild().getExternalizableId());
jenkins.waitUntilNoActivity();
build = projectB.getLastBuild();
upstreamCause = build.getCause(Cause.UpstreamCause.class);
assertNotNull(upstreamCause);
}
}

0 comments on commit c05598d

Please sign in to comment.