Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-22665] [FIXED JENKINS-19755] Changed MyUserIdCause to …
…not include the whole User object serialized.
  • Loading branch information
patbos committed Mar 12, 2015
1 parent a8cd14c commit bd77518
Show file tree
Hide file tree
Showing 6 changed files with 305 additions and 119 deletions.
Expand Up @@ -33,7 +33,6 @@
import hudson.model.Item;
import hudson.model.ItemGroup;
import hudson.model.ParameterValue;
import hudson.model.TaskListener;
import hudson.model.TopLevelItem;
import hudson.model.AbstractBuild;
import hudson.model.AbstractProject;
Expand All @@ -45,7 +44,6 @@
import hudson.model.Hudson;
import hudson.model.ParametersAction;
import hudson.model.Run;
import hudson.model.User;
import hudson.model.View;
import hudson.model.ViewDescriptor;
import hudson.plugins.parameterizedtrigger.AbstractBuildParameters;
Expand Down Expand Up @@ -151,71 +149,12 @@ public class BuildPipelineView extends View {
private static final Logger LOGGER = Logger.getLogger(BuildPipelineView.class.getName());

/**
* An instance of {@link Cause.UserIdCause} related to the current user. Must be transient, or xstream will include it in the
* serialization
* An instance of {@link Cause.UserIdCause} related to the current user.
* Just kept for backwards comparability.
* @deprecated Use Cause.UserIdCause instead
*/
private class MyUserIdCause extends Cause.UserIdCause {
/**
* user
*/
private User user;

/**
*
*/
public MyUserIdCause() {
try {
// this block can generate a CyclicGraphDetector.CycleDetectedException
// in cases that I haven't quite figured out yet
// also an org.acegisecurity.AccessDeniedException when the user
// is not logged in
user = Hudson.getInstance().getMe();
} catch (final Exception e) {
// do nothing
LOGGER.fine(e.getMessage());
}
}

@Override
public String getUserId() {
return (null == user) ? null : user.getId();
}

@Override
public String getUserName() {
return (null == user) ? null : user.getDisplayName();
}

@Override
public String toString() {
return getUserName();
}

@Override
public int hashCode() {
if (getUserId() == null) {
return super.hashCode();
} else {
return getUserId().hashCode();
}
}

@Override
public boolean equals(final Object o) {
if (null == o) {
return false;
}
if (!(o instanceof Cause.UserIdCause)) {
return false;
}

return hashCode() == o.hashCode();
}

@Override
public void print(final TaskListener listener) {
// do nothing
}
@Deprecated
private static class MyUserIdCause extends Cause.UserIdCause {

This comment has been minimized.

Copy link
@daniel-beck

daniel-beck Mar 22, 2015

Member

This commit made the previously inner class into a static nested class. This may have caused JENKINS-27537.

This comment has been minimized.

Copy link
@daniel-beck

daniel-beck Mar 28, 2015

Member

This comment has been minimized.

Copy link
@patbos

patbos Mar 28, 2015

Author

Making the class non static would fix the problem?

This comment has been minimized.

Copy link
@daniel-beck

daniel-beck Mar 28, 2015

Member

Possibly (with unloadable old data since the fields are gone). I haven't had time to try it.

This comment has been minimized.

Copy link
@KostyaSha

KostyaSha Apr 6, 2015

Member

@patbos could you create test case and propose fix?

This comment has been minimized.

Copy link
@patbos

patbos Apr 6, 2015

Author

@daniel-beck Is it possible to create some kind of converter who converts all au.com.centrumsystems.hudson.plugin.buildpipeline.BuildPipelineView$MyUserIdCause to hudson.model.Cause$UserIdCause in the build.xml:s?

This comment has been minimized.

Copy link
@KostyaSha

KostyaSha Apr 6, 2015

Member

You need use Listener for conversion afair

This comment has been minimized.

Copy link
@daniel-beck

daniel-beck Apr 6, 2015

Member

Using readResolve() may work, when loading a BuildPipelineView$MyUserIdCause it can replace that with a Cause$UserIdCause.

This comment has been minimized.

Copy link
@KostyaSha

KostyaSha Apr 9, 2015

Member

Yes, should work @patbos could you do it?

This comment has been minimized.

Copy link
@patbos

patbos Apr 12, 2015

Author

@KostyaSha Will have a look at it.

This comment has been minimized.

Copy link
@KostyaSha

KostyaSha Apr 12, 2015

Member

I spent 2 days trying reproduce this bug, but still have no luck

This comment has been minimized.

Copy link
@KostyaSha

KostyaSha Apr 28, 2015

Member

removing static helps, but <outer-class> with hudson still presents. Also on 1.565.3 build and the whole history can disappear if any build will refer to not existed user permissions. Such fix jenkinsci/matrix-auth-plugin#1 probably can help.

}

/**
Expand Down Expand Up @@ -505,7 +444,7 @@ private int triggerBuild(final AbstractProject<?, ?> triggerProject, final Abstr
final Action buildParametersAction) {
LOGGER.fine("Triggering build for project: " + triggerProject.getFullDisplayName()); //$NON-NLS-1$
final List<Action> buildActions = new ArrayList<Action>();
final CauseAction causeAction = new CauseAction(new MyUserIdCause());
final CauseAction causeAction = new CauseAction(new UserIdCause());
if (upstreamBuild != null) {
causeAction.getCauses().add(new Cause.UpstreamCause((Run<?, ?>) upstreamBuild));
}
Expand Down
Expand Up @@ -25,6 +25,7 @@
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;
Expand All @@ -41,33 +42,35 @@

import jenkins.model.Jenkins;

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

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

import static org.junit.Assert.*;

/**
* Test Build Pipeline View
*
* @author Centrum Systems
*
*/
public class BuildPipelineViewTest extends HudsonTestCase {
public class BuildPipelineViewTest {

@Rule
public JenkinsRule jenkins = new JenkinsRule();

@Override
@Before
public void setUp() throws Exception {
super.setUp();
}

@Test
public void testGetSelectedProject() throws IOException {
final String bpViewName = "MyTestView";
final String bpViewTitle = "MyTestViewTitle";
final String proj1 = "Proj1";
final String noOfBuilds = "5";
createFreeStyleProject(proj1);
jenkins.createFreeStyleProject(proj1);

// Test a valid case
DownstreamProjectGridBuilder gridBuilder = new DownstreamProjectGridBuilder(proj1);
Expand All @@ -91,7 +94,7 @@ public void testHasBuildPermission() throws IOException {
final String bpViewTitle = "MyTestViewTitle";
final String proj1 = "Proj1";
final String noOfBuilds = "5";
final FreeStyleProject project1 = createFreeStyleProject(proj1);
final FreeStyleProject project1 = jenkins.createFreeStyleProject(proj1);

final BuildPipelineView testView = new BuildPipelineView(bpViewName, bpViewTitle,
new DownstreamProjectGridBuilder(proj1), noOfBuilds, false, null);
Expand All @@ -104,7 +107,7 @@ public void testTriggerOnlyLatestJob() throws IOException {
final String bpViewTitle = "MyTestViewTitle";
final String proj1 = "Proj1";
final String noOfBuilds = "5";
createFreeStyleProject(proj1);
jenkins.createFreeStyleProject(proj1);

// True
BuildPipelineView testView = new BuildPipelineView(bpViewName, bpViewTitle, new DownstreamProjectGridBuilder(proj1), noOfBuilds, true, null);
Expand All @@ -121,7 +124,7 @@ public void testAlwaysAllowManualTrigger() throws IOException {
final String bpViewTitle = "MyTestViewTitle";
final String proj1 = "Proj1";
final String noOfBuilds = "5";
createFreeStyleProject(proj1);
jenkins.createFreeStyleProject(proj1);

// True
BuildPipelineView testView = new BuildPipelineView(bpViewName, bpViewTitle, new DownstreamProjectGridBuilder(proj1), noOfBuilds, true, true, false, false, false, 2, null, null);
Expand All @@ -138,7 +141,7 @@ public void testShowPipelineDefinitionHeader() throws IOException {
final String bpViewTitle = "MyTestViewTitle";
final String proj1 = "Proj1";
final String noOfBuilds = "5";
createFreeStyleProject(proj1);
jenkins.createFreeStyleProject(proj1);

// True
BuildPipelineView testView = new BuildPipelineView(bpViewName, bpViewTitle, new DownstreamProjectGridBuilder(proj1), noOfBuilds, true, false, false, false, true, 2, null, null);
Expand All @@ -155,7 +158,7 @@ public void testShowPipelineParameters() throws IOException {
final String bpViewTitle = "MyTestViewTitle";
final String proj1 = "Proj1";
final String noOfBuilds = "5";
createFreeStyleProject(proj1);
jenkins.createFreeStyleProject(proj1);

// True
BuildPipelineView testView = new BuildPipelineView(bpViewName, bpViewTitle, new DownstreamProjectGridBuilder(proj1), noOfBuilds, true, false, true, false, false, 2, null, null);
Expand All @@ -172,7 +175,7 @@ public void testShowPipelineParametersInHeaders() throws IOException {
final String bpViewTitle = "MyTestViewTitle";
final String proj1 = "Proj1";
final String noOfBuilds = "5";
createFreeStyleProject(proj1);
jenkins.createFreeStyleProject(proj1);

BuildPipelineView testView = new BuildPipelineView(bpViewName, bpViewTitle, new DownstreamProjectGridBuilder(proj1), noOfBuilds, true, false, true, true, false, 2, null, null);
assertTrue("Failed to set ShowPipelineParametersInHeaders flag", testView.isShowPipelineParametersInHeaders());
Expand All @@ -188,8 +191,8 @@ public void testHasDownstreamProjects() throws IOException {
final String proj1 = "Proj1";
final String proj2 = "Proj2";
final String noOfBuilds = "5";
final FreeStyleProject project1 = createFreeStyleProject(proj1);
final FreeStyleProject project2 = createFreeStyleProject(proj2);
final FreeStyleProject project1 = jenkins.createFreeStyleProject(proj1);
final FreeStyleProject project2 = jenkins.createFreeStyleProject(proj2);

// Add project2 as a post build action: build other project
project1.getPublishersList().add(new BuildPipelineTrigger(proj2, null));
Expand All @@ -212,8 +215,8 @@ public void testGetDownstreamProjects() throws IOException {
final String proj1 = "Proj1";
final String proj2 = "Proj2";
final String noOfBuilds = "5";
final FreeStyleProject project1 = createFreeStyleProject(proj1);
final FreeStyleProject project2 = createFreeStyleProject(proj2);
final FreeStyleProject project1 = jenkins.createFreeStyleProject(proj1);
final FreeStyleProject project2 = jenkins.createFreeStyleProject(proj2);

// Add project2 as a post build action: build other project
project1.getPublishersList().add(new BuildPipelineTrigger(proj2, null));
Expand All @@ -238,9 +241,9 @@ public void testGetBuildPipelineForm() throws Exception {
final String proj3 = "Proj3";
FreeStyleBuild build1;
final String noOfBuilds = "5";
final FreeStyleProject project1 = createFreeStyleProject(proj1);
final FreeStyleProject project2 = createFreeStyleProject(proj2);
final FreeStyleProject project3 = createFreeStyleProject(proj3);
final FreeStyleProject project1 = jenkins.createFreeStyleProject(proj1);
final FreeStyleProject project2 = jenkins.createFreeStyleProject(proj2);
final FreeStyleProject project3 = jenkins.createFreeStyleProject(proj3);

// Add project2 as a post build action: build other project
project1.getPublishersList().add(new BuildPipelineTrigger(proj2, null));
Expand All @@ -251,8 +254,8 @@ public void testGetBuildPipelineForm() throws Exception {
Hudson.getInstance().rebuildDependencyGraph();

// Build project1
build1 = buildAndAssertSuccess(project1);
waitUntilNoActivity();
build1 = jenkins.buildAndAssertSuccess(project1);
jenkins.waitUntilNoActivity();

// Test a valid case
final BuildPipelineView testView = BuildPipelineViewFactory.getBuildPipelineView(bpViewName, bpViewTitle, new DownstreamProjectGridBuilder(proj1), noOfBuilds, false);
Expand All @@ -262,13 +265,13 @@ public void testGetBuildPipelineForm() throws Exception {
final List<Action> buildActions = new ArrayList<Action>();
project2.scheduleBuild(0, upstreamCause,
buildActions.toArray(new Action[buildActions.size()]));
waitUntilNoActivity();
jenkins.waitUntilNoActivity();

upstreamCause = new hudson.model.Cause.UpstreamCause(
(Run<?, ?>) project2.getBuildByNumber(1));
project3.scheduleBuild(0, upstreamCause,
buildActions.toArray(new Action[buildActions.size()]));
waitUntilNoActivity();
jenkins.waitUntilNoActivity();

final BuildPipelineForm testForm = testView.getBuildPipelineForm();

Expand All @@ -289,7 +292,7 @@ public void testOnJobRenamed() throws IOException {
final String proj2 = "Proj2";
final String proj3 = "Proj3";
final String noOfBuilds = "5";
final FreeStyleProject project1 = createFreeStyleProject(proj1);
final FreeStyleProject project1 = jenkins.createFreeStyleProject(proj1);

// Add project2 as a post build action: build other project
project1.getPublishersList().add(new BuildPipelineTrigger(proj2, null));
Expand All @@ -312,7 +315,7 @@ public void testGetItems() throws IOException {
final String bpViewTitle = "MyTestViewTitle";
final String proj1 = "Proj1";
final String noOfBuilds = "5";
final FreeStyleProject project1 = createFreeStyleProject(proj1);
final FreeStyleProject project1 = jenkins.createFreeStyleProject(proj1);

final BuildPipelineView testView = new BuildPipelineView(bpViewName, bpViewTitle,
new DownstreamProjectGridBuilder(proj1), noOfBuilds, false, null);
Expand All @@ -321,7 +324,7 @@ public void testGetItems() throws IOException {
assertTrue(testView.getItems().contains(item1));

final String proj2 = "Proj2";
final FreeStyleProject project2 = createFreeStyleProject(proj2);
final FreeStyleProject project2 = jenkins.createFreeStyleProject(proj2);
TopLevelItem item2 = Jenkins.getInstance().getItem(proj2);
assertNotNull(item2);
assertFalse(testView.getItems().contains(item2));
Expand All @@ -341,6 +344,18 @@ bpViewTitle, new DownstreamProjectGridBuilder(proj1),
assertTrue(testView.hasPermission(Permission.READ));
}

@Test
@LocalData
@Bug(19755)
public void testMyUserIdCauseConversion() throws Exception {
FreeStyleProject projectB = (FreeStyleProject) jenkins.getInstance().getItem("B");
FreeStyleBuild buildB = projectB.getBuildByNumber(1);
assertNotNull(buildB);
Cause.UserIdCause cause = buildB.getCause(Cause.UserIdCause.class);
assertNotNull(cause);
assertEquals("bill", cause.getUserId());
}

/**
* This is a factory to create an instance of the class under test. This
* helps to avoid a NPE in View.java when calling getOwnerItemGroup and it's
Expand Down

0 comments on commit bd77518

Please sign in to comment.