From c40227e06445e8bd255fb81a451cd9cbff08d57f Mon Sep 17 00:00:00 2001 From: Robert Sandell Date: Tue, 17 May 2016 23:09:11 +0200 Subject: [PATCH 01/10] First fix all tests that should still succeed on 2.x Mostly due to more mocking and some legacy use of Hudson --- pom.xml | 1 + .../gerritnotifier/ParameterExpander.java | 7 +-- .../trigger/hudsontrigger/GerritCause.java | 5 +- .../ParameterExpanderParameterizedTest.java | 17 +++++- ...arameterExpanderSkipVoteParameterTest.java | 17 +++++- .../gerritnotifier/ParameterExpanderTest.java | 54 +++++++++---------- .../hudsontrigger/GerritProjectListTest.java | 12 +++++ .../hudsontrigger/GerritTriggerTest.java | 16 +++--- .../data/TriggerContextConverterTest.java | 43 ++++++++++++++- 9 files changed, 131 insertions(+), 41 deletions(-) diff --git a/pom.xml b/pom.xml index f350d751a..d71e10892 100644 --- a/pom.xml +++ b/pom.xml @@ -54,6 +54,7 @@ 1.609.3 + 6 UTF-8 false diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java index 78f916847..7b89a26fe 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java @@ -48,6 +48,7 @@ import java.util.HashMap; import java.util.Map; +import jenkins.model.Jenkins; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -72,14 +73,14 @@ public class ParameterExpander { private static final Logger logger = LoggerFactory.getLogger(ParameterExpander.class); private IGerritHudsonTriggerConfig config; - private Hudson hudson; + private Jenkins hudson; /** * Constructor. * @param config the global config. * @param hudson the Hudson instance. */ - public ParameterExpander(IGerritHudsonTriggerConfig config, Hudson hudson) { + public ParameterExpander(IGerritHudsonTriggerConfig config, Jenkins hudson) { this.config = config; this.hudson = hudson; } @@ -89,7 +90,7 @@ public ParameterExpander(IGerritHudsonTriggerConfig config, Hudson hudson) { * @param config the global config. */ public ParameterExpander(IGerritHudsonTriggerConfig config) { - this(config, Hudson.getInstance()); + this(config, Jenkins.getInstance()); } /** diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritCause.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritCause.java index cc65aa130..207c3c528 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritCause.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritCause.java @@ -233,7 +233,10 @@ public boolean equals(Object o) { @Override public int hashCode() { - int result = tEvent.hashCode(); + int result = 1; + if (tEvent != null) { + result = tEvent.hashCode(); + } result = 31 * result + (silentMode ? 1 : 0); return result; } diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderParameterizedTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderParameterizedTest.java index 21dad18be..ff13390f2 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderParameterizedTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderParameterizedTest.java @@ -30,10 +30,16 @@ import hudson.model.Result; +import jenkins.model.Jenkins; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.powermock.modules.junit4.PowerMockRunnerDelegate; import java.util.Collection; import java.util.LinkedList; @@ -51,7 +57,9 @@ * and {@link ParameterExpander#getVerifiedValue(hudson.model.Result, GerritTrigger)} * @author Robert Sandell <robert.sandell@sonyericsson.com> */ -@RunWith(Parameterized.class) +@RunWith(PowerMockRunner.class) +@PowerMockRunnerDelegate(Parameterized.class) +@PrepareForTest(Jenkins.class) public class ParameterExpanderParameterizedTest { private TestParameters parameters; @@ -64,6 +72,13 @@ public ParameterExpanderParameterizedTest(TestParameters parameters) { this.parameters = parameters; } + @Before + public void setup() { + PowerMockito.mockStatic(Jenkins.class); + Jenkins jenkins = PowerMockito.mock(Jenkins.class); + PowerMockito.when(Jenkins.getInstance()).thenReturn(jenkins); + } + /** * test. */ diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java index c28617e21..f1d5c9297 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java @@ -30,9 +30,15 @@ import hudson.model.Result; +import jenkins.model.Jenkins; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.powermock.modules.junit4.PowerMockRunnerDelegate; import java.util.Collection; import java.util.LinkedList; @@ -51,7 +57,9 @@ * * @author Robert Sandell <robert.sandell@sonymobile.com> */ -@RunWith(Parameterized.class) +@RunWith(PowerMockRunner.class) +@PowerMockRunnerDelegate(Parameterized.class) +@PrepareForTest(Jenkins.class) public class ParameterExpanderSkipVoteParameterTest { private TestParameter parameter; @@ -65,6 +73,13 @@ public ParameterExpanderSkipVoteParameterTest(TestParameter parameter) { this.parameter = parameter; } + @Before + public void setup() { + PowerMockito.mockStatic(Jenkins.class); + Jenkins jenkins = PowerMockito.mock(Jenkins.class); + PowerMockito.when(Jenkins.getInstance()).thenReturn(jenkins); + } + /** * Tests that {@link ParameterExpander#getMinimumCodeReviewValue(BuildMemory.MemoryImprint, boolean)} * returns {@link TestParameter#expectedCodeReview}. diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java index ca4b3affc..58cf9285b 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java @@ -41,16 +41,17 @@ import hudson.model.TaskListener; import hudson.model.AbstractBuild; import hudson.model.AbstractProject; -import hudson.model.Hudson; import java.io.IOException; import java.util.Arrays; import java.util.LinkedList; import java.util.List; +import jenkins.model.Jenkins; import org.hamcrest.Description; import org.hamcrest.Matcher; import org.hamcrest.TypeSafeMatcher; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.powermock.api.mockito.PowerMockito; @@ -73,9 +74,19 @@ * @author Robert Sandell <robert.sandell@sonyericsson.com> */ @RunWith(PowerMockRunner.class) -@PrepareForTest({ Hudson.class, GerritMessageProvider.class }) +@PrepareForTest({ Jenkins.class, GerritMessageProvider.class }) public class ParameterExpanderTest { + private Jenkins jenkins; + + @Before + public void setup() { + PowerMockito.mockStatic(Jenkins.class); + jenkins = PowerMockito.mock(Jenkins.class); + PowerMockito.when(Jenkins.getInstance()).thenReturn(jenkins); + when(jenkins.getRootUrl()).thenReturn("http://localhost/"); + } + /** * test. * @throws Exception Exception @@ -96,8 +107,6 @@ public void testGetBuildStartedCommand() throws Exception { BuildsStartedStats stats = Setup.createBuildStartedStats(event); IGerritHudsonTriggerConfig config = Setup.createConfig(); - Hudson hudson = PowerMockito.mock(Hudson.class); - when(hudson.getRootUrl()).thenReturn("http://localhost/"); PowerMockito.mockStatic(GerritMessageProvider.class); List messageProviderExtensionList = new LinkedList(); @@ -105,7 +114,7 @@ public void testGetBuildStartedCommand() throws Exception { messageProviderExtensionList.add(new GerritMessageProviderExtensionReturnNull()); when(GerritMessageProvider.all()).thenReturn(messageProviderExtensionList); - ParameterExpander instance = new ParameterExpander(config, hudson); + ParameterExpander instance = new ParameterExpander(config, jenkins); final String expectedRefSpec = StringUtil.makeRefSpec(event); @@ -132,7 +141,7 @@ public void testGetBuildStartedCommand() throws Exception { public void testGetMinimumVerifiedValue() { IGerritHudsonTriggerConfig config = Setup.createConfig(); - ParameterExpander instance = new ParameterExpander(config); + ParameterExpander instance = new ParameterExpander(config, jenkins); MemoryImprint memoryImprint = mock(MemoryImprint.class); MemoryImprint.Entry[] entries = new MemoryImprint.Entry[4]; @@ -172,7 +181,7 @@ public void testGetMinimumVerifiedValue() { public void testGetMinimumCodeReviewValue() { IGerritHudsonTriggerConfig config = Setup.createConfig(); - ParameterExpander instance = new ParameterExpander(config); + ParameterExpander instance = new ParameterExpander(config, jenkins); MemoryImprint memoryImprint = mock(MemoryImprint.class); MemoryImprint.Entry[] entries = new MemoryImprint.Entry[4]; @@ -213,7 +222,7 @@ public void testGetMinimumCodeReviewValue() { public void testGetMinimumCodeReviewValueOneUnstableSkipped() { IGerritHudsonTriggerConfig config = Setup.createConfig(); - ParameterExpander instance = new ParameterExpander(config); + ParameterExpander instance = new ParameterExpander(config, jenkins); MemoryImprint memoryImprint = mock(MemoryImprint.class); MemoryImprint.Entry[] entries = new MemoryImprint.Entry[3]; @@ -248,7 +257,7 @@ public void testGetMinimumCodeReviewValueOneUnstableSkipped() { public void testGetMinimumCodeReviewValueOneSuccessfulSkipped() { IGerritHudsonTriggerConfig config = Setup.createConfig(); - ParameterExpander instance = new ParameterExpander(config); + ParameterExpander instance = new ParameterExpander(config, jenkins); MemoryImprint memoryImprint = mock(MemoryImprint.class); MemoryImprint.Entry[] entries = new MemoryImprint.Entry[1]; @@ -274,7 +283,7 @@ public void testGetMinimumCodeReviewValueOneSuccessfulSkipped() { public void testGetMinimumCodeReviewValueForOneJobOverridenBuildSuccessful() { IGerritHudsonTriggerConfig config = Setup.createConfigWithCodeReviewsNull(); - ParameterExpander instance = new ParameterExpander(config); + ParameterExpander instance = new ParameterExpander(config, jenkins); MemoryImprint memoryImprint = mock(MemoryImprint.class); MemoryImprint.Entry[] entries = new MemoryImprint.Entry[2]; @@ -304,7 +313,7 @@ public void testGetMinimumCodeReviewValueForOneJobOverridenBuildSuccessful() { public void testGetMinimumCodeReviewValueForOneJobOverridenBuildFailed() { IGerritHudsonTriggerConfig config = Setup.createConfigWithCodeReviewsNull(); - ParameterExpander instance = new ParameterExpander(config); + ParameterExpander instance = new ParameterExpander(config, jenkins); MemoryImprint memoryImprint = mock(MemoryImprint.class); MemoryImprint.Entry[] entries = new MemoryImprint.Entry[2]; @@ -334,7 +343,7 @@ public void testGetMinimumCodeReviewValueForOneJobOverridenBuildFailed() { public void testGetMinimumCodeReviewValueForOneJobOverridenMixed() { IGerritHudsonTriggerConfig config = Setup.createConfigWithCodeReviewsNull(); - ParameterExpander instance = new ParameterExpander(config); + ParameterExpander instance = new ParameterExpander(config, jenkins); MemoryImprint memoryImprint = mock(MemoryImprint.class); MemoryImprint.Entry[] entries = new MemoryImprint.Entry[2]; @@ -528,9 +537,6 @@ public void tryGetBuildCompletedCommandEventWithResults(String customUrl, String IGerritHudsonTriggerConfig config = Setup.createConfig(); - Hudson hudson = PowerMockito.mock(Hudson.class); - when(hudson.getRootUrl()).thenReturn("http://localhost/"); - TaskListener taskListener = mock(TaskListener.class); GerritTrigger trigger = mock(GerritTrigger.class); @@ -544,7 +550,7 @@ public void tryGetBuildCompletedCommandEventWithResults(String customUrl, String for (int i = 0; i < expectedBuildResults.length; i++) { EnvVars env = Setup.createEnvVars(); AbstractBuild r = Setup.createBuild(project, taskListener, env); - env.put("BUILD_URL", hudson.getRootUrl() + r.getUrl()); + env.put("BUILD_URL", jenkins.getRootUrl() + r.getUrl()); when(r.getResult()).thenReturn(expectedBuildResults[i]); entries[i] = Setup.createImprintEntry(project, r); } @@ -568,7 +574,7 @@ public void tryGetBuildCompletedCommandEventWithResults(String customUrl, String messageProviderExtensionList.add(new GerritMessageProviderExtensionReturnNull()); when(GerritMessageProvider.all()).thenReturn(messageProviderExtensionList); - ParameterExpander instance = new ParameterExpander(config, hudson); + ParameterExpander instance = new ParameterExpander(config, jenkins); String result = instance.getBuildCompletedCommand(memoryImprint, taskListener); System.out.println("Result: " + result); @@ -611,9 +617,6 @@ public void testBuildStatsWithUnsuccessfulMessage() throws Exception { public void tryBuildStatsFailureCommand(String unsuccessfulMessage, String expectedBuildStats) throws Exception { IGerritHudsonTriggerConfig config = Setup.createConfig(); - Hudson hudson = PowerMockito.mock(Hudson.class); - when(hudson.getRootUrl()).thenReturn("http://localhost/"); - TaskListener taskListener = mock(TaskListener.class); GerritTrigger trigger = mock(GerritTrigger.class); @@ -624,7 +627,7 @@ public void tryBuildStatsFailureCommand(String unsuccessfulMessage, String expec EnvVars env = Setup.createEnvVars(); AbstractBuild r = Setup.createBuild(project, taskListener, env); - env.put("BUILD_URL", hudson.getRootUrl() + r.getUrl()); + env.put("BUILD_URL", jenkins.getRootUrl() + r.getUrl()); when(r.getResult()).thenReturn(Result.FAILURE); @@ -653,7 +656,7 @@ public void tryBuildStatsFailureCommand(String unsuccessfulMessage, String expec messageProviderExtensionList.add(new GerritMessageProviderExtensionReturnNull()); when(GerritMessageProvider.all()).thenReturn(messageProviderExtensionList); - ParameterExpander instance = new ParameterExpander(config, hudson); + ParameterExpander instance = new ParameterExpander(config, jenkins); String result = instance.getBuildCompletedCommand(memoryImprint, taskListener); System.out.println("Result: " + result); @@ -670,9 +673,6 @@ public void tryBuildStatsFailureCommand(String unsuccessfulMessage, String expec public void getBuildStatsFailureCommandWithNullsForCodeReviewValues() throws Exception { IGerritHudsonTriggerConfig config = Setup.createConfigWithCodeReviewsNull(); - Hudson hudson = PowerMockito.mock(Hudson.class); - when(hudson.getRootUrl()).thenReturn("http://localhost/"); - TaskListener taskListener = mock(TaskListener.class); GerritTrigger trigger = mock(GerritTrigger.class); @@ -684,7 +684,7 @@ public void getBuildStatsFailureCommandWithNullsForCodeReviewValues() throws Exc EnvVars env = Setup.createEnvVars(); AbstractBuild r = Setup.createBuild(project, taskListener, env); - env.put("BUILD_URL", hudson.getRootUrl() + r.getUrl()); + env.put("BUILD_URL", jenkins.getRootUrl() + r.getUrl()); when(r.getResult()).thenReturn(Result.FAILURE); @@ -709,7 +709,7 @@ public void getBuildStatsFailureCommandWithNullsForCodeReviewValues() throws Exc messageProviderExtensionList.add(new GerritMessageProviderExtensionReturnNull()); when(GerritMessageProvider.all()).thenReturn(messageProviderExtensionList); - ParameterExpander instance = new ParameterExpander(config, hudson); + ParameterExpander instance = new ParameterExpander(config, jenkins); String result = instance.getBuildCompletedCommand(memoryImprint, taskListener); System.out.println("Result: " + result); diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritProjectListTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritProjectListTest.java index 3dfecc8de..0a2168f29 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritProjectListTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritProjectListTest.java @@ -30,9 +30,14 @@ import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data.GerritProject; import com.sonyericsson.hudson.plugins.gerrit.trigger.mock.Setup; +import jenkins.model.Jenkins; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; import static org.junit.Assert.assertEquals; @@ -46,6 +51,8 @@ /** * Tests for {@link GerritProjectListTest}. */ +@RunWith(PowerMockRunner.class) +@PrepareForTest(Jenkins.class) public class GerritProjectListTest { /** @@ -88,6 +95,11 @@ private GerritTrigger createGerritTrigger( */ @Before public void createProjectsAndTriggers() throws Exception { + + PowerMockito.mockStatic(Jenkins.class); + Jenkins jenkins = PowerMockito.mock(Jenkins.class); + PowerMockito.when(Jenkins.getInstance()).thenReturn(jenkins); + GerritProject gP1 = createGerritProject("test/project1", CompareType.PLAIN); GerritProject gP2 = createGerritProject("test/project2", CompareType.PLAIN); GerritProject gP3 = createGerritProject("test/project3", CompareType.PLAIN); diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java index f6d8bbed6..5bb02cf07 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java @@ -264,6 +264,7 @@ public void testScheduleWithAverageBuildScheduleDelay() { */ @Test public void testProjectRename() throws Exception { + mockConfig(); mockPluginConfig(0); // we'll make AbstractProject return different names over time final String[] name = new String[1]; @@ -1127,7 +1128,7 @@ public void describeTo(Description description) { */ @Test public void testCreateParametersWhenTriggerWithEscapeQuotesOn() { - + mockConfig(); String stringWithQuotes = "Fixed \" the thing to make \" some thing fun"; String stringWithQuotesEscaped = "Fixed \\\" the thing to make \\\" some thing fun"; String stringWithoutQuotes = "Fixed the thing to make some thing fun"; @@ -1187,7 +1188,7 @@ public void testCreateParametersWhenTriggerWithEscapeQuotesOn() { */ @Test public void testCreateParametersWhenTriggerWithEscapeQuotesOff() { - + mockConfig(); String stringWithQuotes = "Fixed \" the thing to make \" some thing fun"; String stringWithoutQuotes = "Fixed the thing to make some thing fun"; @@ -1247,7 +1248,7 @@ public void testCreateParametersWhenTriggerWithEscapeQuotesOff() { */ @Test public void testCreateParametersWhenTriggerWithReadableMessageOn() { - + mockConfig(); String stringReadable = "This is human readable message"; //prepare AbstractProject object @@ -1295,7 +1296,7 @@ public void testCreateParametersWhenTriggerWithReadableMessageOn() { */ @Test public void testCreateParametersWhenTriggerWithReadableMessageOff() { - + mockConfig(); String stringReadable = "This is human readable message"; String stringEncoded = "VGhpcyBpcyBodW1hbiByZWFkYWJsZSBtZXNzYWdl"; @@ -1553,9 +1554,10 @@ public String toString() { @Test public void shouldReturnEmptySlaveListWhenGerritServerNotFound() { // setup - PowerMockito.mockStatic(PluginImpl.class); + mockConfig(); + /*PowerMockito.mockStatic(PluginImpl.class); PluginImpl pluginMock = mock(PluginImpl.class); - when(PluginImpl.getInstance()).thenReturn(pluginMock); + when(PluginImpl.getInstance()).thenReturn(pluginMock);*/ GerritTrigger gerritTrigger = Setup.createDefaultTrigger(null); // actual test @@ -1570,6 +1572,7 @@ public void shouldReturnEmptySlaveListWhenGerritServerNotFound() { */ @Test public void shouldReturnEmptySlaveListWhenNotConfigured() { + mockConfig(); IGerritHudsonTriggerConfig configMock = setupSeverConfigMock(); GerritTrigger gerritTrigger = Setup.createDefaultTrigger(null); @@ -1738,6 +1741,7 @@ public void testDependencyValidationThreeProjectsInvolved() { * @return the ReplicationConfig mock */ private ReplicationConfig setupReplicationConfigMock() { + mockConfig(); IGerritHudsonTriggerConfig configMock = setupSeverConfigMock(); ReplicationConfig replicationConfigMock = mock(ReplicationConfig.class); when(configMock.getReplicationConfig()).thenReturn(replicationConfigMock); diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/TriggerContextConverterTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/TriggerContextConverterTest.java index 655191808..b3f65444d 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/TriggerContextConverterTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/TriggerContextConverterTest.java @@ -34,13 +34,26 @@ import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.actions.RetriggerAction; import com.sonyericsson.hudson.plugins.gerrit.trigger.mock.Setup; import com.thoughtworks.xstream.XStream; +import hudson.ExtensionList; +import hudson.diagnosis.OldDataMonitor; +import hudson.matrix.MatrixProject; import hudson.matrix.MatrixRun; +import hudson.model.AbstractItem; import hudson.model.Cause; +import hudson.model.Saveable; import hudson.util.XStream2; +import jenkins.model.Jenkins; +import jenkins.model.TransientActionFactory; +import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; import org.powermock.reflect.Whitebox; +import java.util.Collections; import java.util.LinkedList; import java.util.List; @@ -52,7 +65,16 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; - +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyCollection; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.same; +import static org.powermock.api.mockito.PowerMockito.doNothing; +import static org.powermock.api.mockito.PowerMockito.doReturn; +import static org.powermock.api.mockito.PowerMockito.mock; +import static org.powermock.api.mockito.PowerMockito.mockStatic; +import static org.powermock.api.mockito.PowerMockito.when; /** @@ -60,9 +82,21 @@ * * @author Robert Sandell <robert.sandell@sonyericsson.com> */ +@RunWith(PowerMockRunner.class) +@PrepareForTest({Jenkins.class, OldDataMonitor.class, ExtensionList.class}) public class TriggerContextConverterTest { + private Jenkins jenkins; //CS IGNORE MagicNumber FOR NEXT 500 LINES. REASON: test data. + + @Before + public void setup() { + PowerMockito.mockStatic(Jenkins.class); + jenkins = mock(Jenkins.class); + when(Jenkins.getInstance()).thenReturn(jenkins); + when(jenkins.getFullName()).thenReturn(""); + } + //CS IGNORE LineLength FOR NEXT 4 LINES. REASON: Javadoc. /** @@ -536,13 +570,18 @@ public void testUnmarshalOldData2() throws Exception { */ @Test public void testUnmarshalOldMatrixBuild() throws Exception { + PowerMockito.mockStatic(OldDataMonitor.class); + doNothing().when(OldDataMonitor.class, "report", any(Saveable.class), anyCollection()); XStream xStream = new XStream2(); xStream.registerConverter(new TriggerContextConverter()); xStream.alias("matrix-run", MatrixRun.class); Object obj = xStream.fromXML(getClass().getResourceAsStream("matrix_build.xml")); assertTrue(obj instanceof MatrixRun); MatrixRun run = (MatrixRun)obj; - + mockStatic(ExtensionList.class); + ExtensionList listMock = mock(ExtensionList.class); + doReturn(Collections.emptyList().iterator()).when(listMock).iterator(); + doReturn(listMock).when(ExtensionList.class, "lookup", same(TransientActionFactory.class)); Cause.UpstreamCause upCause = run.getCause(Cause.UpstreamCause.class); List upstreamCauses = Whitebox.getInternalState(upCause, "upstreamCauses"); GerritCause cause = (GerritCause)upstreamCauses.get(0); From 3116820af823b0a2ee3613b03a5d0d2ca591e754 Mon Sep 17 00:00:00 2001 From: Robert Sandell Date: Wed, 18 May 2016 16:33:36 +0200 Subject: [PATCH 02/10] [JENKINS-34753] Provide safe parameters to ParametersAction And fixed some checkstyle errors in the tests --- .../gerritnotifier/ParameterExpander.java | 1 - .../trigger/hudsontrigger/EventListener.java | 13 +++++++++++++ .../hudsontrigger/GerritTriggerParameters.java | 16 ++++++++++++++++ .../ParameterExpanderParameterizedTest.java | 3 +++ .../ParameterExpanderSkipVoteParameterTest.java | 3 +++ .../gerritnotifier/ParameterExpanderTest.java | 3 +++ .../data/TriggerContextConverterTest.java | 11 +++++------ 7 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java index 7b89a26fe..a115fa806 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java @@ -40,7 +40,6 @@ import hudson.model.Result; import hudson.model.Run; import hudson.model.TaskListener; -import hudson.model.Hudson; import java.util.Arrays; import java.util.Collections; diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java index 6e365fb41..0f83787c0 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java @@ -48,7 +48,9 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +import java.lang.reflect.Constructor; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.concurrent.Future; @@ -251,6 +253,17 @@ protected Job asJob() { protected ParametersAction createParameters(GerritTriggeredEvent event, Job project) { List parameters = getDefaultParametersValues(project); setOrCreateParameters(event, project, parameters); + try { + Constructor constructor = ParametersAction.class.getConstructor(List.class, + Collection.class); + return constructor.newInstance(parameters, GerritTriggerParameters.getNamesSet()); + } catch (NoSuchMethodException e) { + logger.debug("Running on an old core before safe parameters, we are safe.", e); + } catch (IllegalAccessException e) { + logger.warn("Running on a core with safe parameters fix available, but not allowed to specify them", e); + } catch (Exception e) { + logger.warn("Running on a core with safe parameters fix available, but failed to provide them", e); + } return new ParametersAction(parameters); } diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerParameters.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerParameters.java index ed17bc850..276305fd5 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerParameters.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerParameters.java @@ -50,6 +50,8 @@ import java.lang.reflect.Constructor; import java.nio.charset.Charset; import java.util.List; +import java.util.Set; +import java.util.TreeSet; /** * The parameters to add to a build. @@ -217,6 +219,20 @@ public enum GerritTriggerParameters { private static final Logger logger = LoggerFactory.getLogger(GerritTriggerParameters.class); + /** + * A set of all the declared parameter names. + * @return the names of the parameters + * @see #values() + * @see #name() + */ + public static Set getNamesSet() { + Set names = new TreeSet(); + for (GerritTriggerParameters p : GerritTriggerParameters.values()) { + names.add(p.name()); + } + return names; + } + /** * Creates a {@link hudson.model.ParameterValue} and adds it to the provided list. * If the parameter with the same name already exists in the list it will be replaced by the new parameter, diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderParameterizedTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderParameterizedTest.java index ff13390f2..587faad8a 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderParameterizedTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderParameterizedTest.java @@ -72,6 +72,9 @@ public ParameterExpanderParameterizedTest(TestParameters parameters) { this.parameters = parameters; } + /** + * Mock Jenkins. + */ @Before public void setup() { PowerMockito.mockStatic(Jenkins.class); diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java index f1d5c9297..c313bfd15 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java @@ -73,6 +73,9 @@ public ParameterExpanderSkipVoteParameterTest(TestParameter parameter) { this.parameter = parameter; } + /** + * Mock Jenkins. + */ @Before public void setup() { PowerMockito.mockStatic(Jenkins.class); diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java index 58cf9285b..0fcaad741 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java @@ -79,6 +79,9 @@ public class ParameterExpanderTest { private Jenkins jenkins; + /** + * Mock Jenkins. + */ @Before public void setup() { PowerMockito.mockStatic(Jenkins.class); diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/TriggerContextConverterTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/TriggerContextConverterTest.java index b3f65444d..056c72442 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/TriggerContextConverterTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/data/TriggerContextConverterTest.java @@ -36,9 +36,7 @@ import com.thoughtworks.xstream.XStream; import hudson.ExtensionList; import hudson.diagnosis.OldDataMonitor; -import hudson.matrix.MatrixProject; import hudson.matrix.MatrixRun; -import hudson.model.AbstractItem; import hudson.model.Cause; import hudson.model.Saveable; import hudson.util.XStream2; @@ -67,8 +65,6 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyCollection; -import static org.mockito.Matchers.anyString; -import static org.mockito.Matchers.eq; import static org.mockito.Matchers.same; import static org.powermock.api.mockito.PowerMockito.doNothing; import static org.powermock.api.mockito.PowerMockito.doReturn; @@ -83,12 +79,15 @@ * @author Robert Sandell <robert.sandell@sonyericsson.com> */ @RunWith(PowerMockRunner.class) -@PrepareForTest({Jenkins.class, OldDataMonitor.class, ExtensionList.class}) +@PrepareForTest({Jenkins.class, OldDataMonitor.class, ExtensionList.class }) public class TriggerContextConverterTest { private Jenkins jenkins; - //CS IGNORE MagicNumber FOR NEXT 500 LINES. REASON: test data. + //CS IGNORE MagicNumber FOR NEXT 600 LINES. REASON: test data. + /** + * Mock Jenkins. + */ @Before public void setup() { PowerMockito.mockStatic(Jenkins.class); From 1d084be511df4cf92cbf2454daaa5caeee8f3592 Mon Sep 17 00:00:00 2001 From: Robert Sandell Date: Wed, 18 May 2016 17:18:12 +0200 Subject: [PATCH 03/10] More selective log warning --- .../gerrit/trigger/hudsontrigger/EventListener.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java index 0f83787c0..a203339a1 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java @@ -41,6 +41,7 @@ import hudson.model.ParameterValue; import hudson.model.ParametersAction; import hudson.model.ParametersDefinitionProperty; +import hudson.util.VersionNumber; import jenkins.model.Jenkins; import jenkins.model.ParameterizedJobMixIn; import org.slf4j.Logger; @@ -258,7 +259,16 @@ protected ParametersAction createParameters(GerritTriggeredEvent event, Job proj Collection.class); return constructor.newInstance(parameters, GerritTriggerParameters.getNamesSet()); } catch (NoSuchMethodException e) { - logger.debug("Running on an old core before safe parameters, we are safe.", e); + VersionNumber version = Jenkins.getVersion(); + if (version.isNewerThan(new VersionNumber("2.2")) + || (version.isNewerThan(new VersionNumber("1.651.1")) + && version.isOlderThan(new VersionNumber("1.651.300")))) { + logger.warn("Running on a core with SECURITY-170 fixed but no way to specify safe parameters.\n" + + "You should consider upgrading to > 2.5 or set the appropriate startup parameters", + e); + } else { + logger.debug("Running on an old core before safe parameters, we should be safe.", e); + } } catch (IllegalAccessException e) { logger.warn("Running on a core with safe parameters fix available, but not allowed to specify them", e); } catch (Exception e) { From 9f8f4f7b73489897982d4129b66c04196c264c0c Mon Sep 17 00:00:00 2001 From: Robert Sandell Date: Wed, 18 May 2016 17:26:15 +0200 Subject: [PATCH 04/10] Remove some commented code --- .../gerrit/trigger/hudsontrigger/GerritTriggerTest.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java index 5bb02cf07..84177b39a 100644 --- a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java @@ -1555,9 +1555,6 @@ public String toString() { public void shouldReturnEmptySlaveListWhenGerritServerNotFound() { // setup mockConfig(); - /*PowerMockito.mockStatic(PluginImpl.class); - PluginImpl pluginMock = mock(PluginImpl.class); - when(PluginImpl.getInstance()).thenReturn(pluginMock);*/ GerritTrigger gerritTrigger = Setup.createDefaultTrigger(null); // actual test From 305b009f8eea6ccee86b2b9786ebaf7acb2d96a0 Mon Sep 17 00:00:00 2001 From: Robert Sandell Date: Wed, 18 May 2016 17:45:53 +0200 Subject: [PATCH 05/10] One more hudson to jenkins rename --- .../trigger/gerritnotifier/ParameterExpander.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java index a115fa806..0946f7644 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java @@ -72,16 +72,16 @@ public class ParameterExpander { private static final Logger logger = LoggerFactory.getLogger(ParameterExpander.class); private IGerritHudsonTriggerConfig config; - private Jenkins hudson; + private Jenkins jenkins; /** * Constructor. * @param config the global config. - * @param hudson the Hudson instance. + * @param jenkins the Hudson instance. */ - public ParameterExpander(IGerritHudsonTriggerConfig config, Jenkins hudson) { + public ParameterExpander(IGerritHudsonTriggerConfig config, Jenkins jenkins) { this.config = config; - this.hudson = hudson; + this.jenkins = jenkins; } /** @@ -240,7 +240,7 @@ private Map createStandardParameters(Run r, GerritTriggeredEvent } } if (r != null) { - map.put("BUILDURL", hudson.getRootUrl() + r.getUrl()); + map.put("BUILDURL", jenkins.getRootUrl() + r.getUrl()); } map.put("VERIFIED", String.valueOf(verified)); map.put("CODE_REVIEW", String.valueOf(codeReview)); @@ -554,7 +554,7 @@ public String getBuildCompletedCommand(MemoryImprint memoryImprint, TaskListener private String createBuildsStats(MemoryImprint memoryImprint, TaskListener listener, Map parameters) { StringBuilder str = new StringBuilder(""); - final String rootUrl = hudson.getRootUrl(); + final String rootUrl = jenkins.getRootUrl(); String unsuccessfulMessage = null; From 5a6fd2143e0e80714b92ca2a3584006d34908242 Mon Sep 17 00:00:00 2001 From: Robert Sandell Date: Thu, 19 May 2016 12:08:44 +0200 Subject: [PATCH 06/10] Guarding for null version --- .../gerrit/trigger/hudsontrigger/EventListener.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java index a203339a1..fd52a5482 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java @@ -260,9 +260,15 @@ protected ParametersAction createParameters(GerritTriggeredEvent event, Job proj return constructor.newInstance(parameters, GerritTriggerParameters.getNamesSet()); } catch (NoSuchMethodException e) { VersionNumber version = Jenkins.getVersion(); - if (version.isNewerThan(new VersionNumber("2.2")) - || (version.isNewerThan(new VersionNumber("1.651.1")) - && version.isOlderThan(new VersionNumber("1.651.300")))) { + if (version != null + && ( + version.isNewerThan(new VersionNumber("2.2")) + || ( + version.isNewerThan(new VersionNumber("1.651.1")) + && version.isOlderThan(new VersionNumber("1.651.300")) + ) + ) + ) { logger.warn("Running on a core with SECURITY-170 fixed but no way to specify safe parameters.\n" + "You should consider upgrading to > 2.5 or set the appropriate startup parameters", e); From c013c5b4eb338023ae9b41a3d4f83fef9647d4f1 Mon Sep 17 00:00:00 2001 From: Robert Sandell Date: Thu, 19 May 2016 15:27:57 +0200 Subject: [PATCH 07/10] Missed a checkstyle warning --- .../plugins/gerrit/trigger/hudsontrigger/EventListener.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java index fd52a5482..4f5f823b0 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java @@ -269,8 +269,8 @@ protected ParametersAction createParameters(GerritTriggeredEvent event, Job proj ) ) ) { - logger.warn("Running on a core with SECURITY-170 fixed but no way to specify safe parameters.\n" + - "You should consider upgrading to > 2.5 or set the appropriate startup parameters", + logger.warn("Running on a core with SECURITY-170 fixed but no way to specify safe parameters.\n" + + "You should consider upgrading to > 2.5 or set the appropriate startup parameters", e); } else { logger.debug("Running on an old core before safe parameters, we should be safe.", e); From 6b4d78be770ba09959dc30c70db19606b8183a46 Mon Sep 17 00:00:00 2001 From: Robert Sandell Date: Mon, 23 May 2016 14:12:24 +0200 Subject: [PATCH 08/10] Inspect ParametersAction for better logging --- .../trigger/hudsontrigger/EventListener.java | 150 ++++++++++++++++-- 1 file changed, 136 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java index 4f5f823b0..2d841043d 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java @@ -41,18 +41,21 @@ import hudson.model.ParameterValue; import hudson.model.ParametersAction; import hudson.model.ParametersDefinitionProperty; -import hudson.util.VersionNumber; import jenkins.model.Jenkins; import jenkins.model.ParameterizedJobMixIn; +import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Locale; import java.util.concurrent.Future; import static com.sonyericsson.hudson.plugins.gerrit.trigger.PluginImpl.getServerConfig; @@ -259,19 +262,29 @@ protected ParametersAction createParameters(GerritTriggeredEvent event, Job proj Collection.class); return constructor.newInstance(parameters, GerritTriggerParameters.getNamesSet()); } catch (NoSuchMethodException e) { - VersionNumber version = Jenkins.getVersion(); - if (version != null - && ( - version.isNewerThan(new VersionNumber("2.2")) - || ( - version.isNewerThan(new VersionNumber("1.651.1")) - && version.isOlderThan(new VersionNumber("1.651.300")) - ) - ) - ) { - logger.warn("Running on a core with SECURITY-170 fixed but no way to specify safe parameters.\n" - + "You should consider upgrading to > 2.5 or set the appropriate startup parameters", - e); + ParametersActionInspection inspection = getParametersInspection(); + if (inspection.isHasSafeParameterConfig()) { + StringBuilder txt = new StringBuilder( + "Running on a core with SECURITY-170 fixed but no direct way for Gerrit Trigger" + + " to self-specify safe parameters."); + txt.append(" You should consider upgrading to a new Jenkins core version."); + if (inspection.isKeepUndefinedParameters()) { + txt.append("\n.keepSafeParameters is set so the trigger should behave normally."); + } else if (inspection.isSafeParametersSet()) { + txt.append("\nAll Gerrit related parameters are set in .safeParameters"); + txt.append(" so the trigger should behave normally."); + } else { + txt.append("No overriding system properties appears to be set,"); + txt.append(" your builds might not work as expected.\n"); + txt.append("See https://wiki.jenkins-ci.org/display/SECURITY/Jenkins+Security+Advisory+2016-05-11"); + txt.append(" for information."); + } + logger.warn(txt.toString()); + } else if (inspection.isInspectionFailure()) { + logger.warn("Failed to inspect ParametersAction to determine " + + "if we can behave normally around SECURITY-170.\nSee " + + "https://wiki.jenkins-ci.org/display/SECURITY/Jenkins+Security+Advisory+2016-05-11" + + " for information."); } else { logger.debug("Running on an old core before safe parameters, we should be safe.", e); } @@ -361,4 +374,113 @@ public int hashCode() { public boolean equals(Object obj) { return obj instanceof EventListener && ((EventListener)obj).job.equals(job); } + + /** + * Inspects {@link ParametersAction} to see what kind of capabilities it has in regards to SECURITY-170. + * Assuming the safeParameters constructor could not be found. + * + * @return the inspection result + * @see #createParameters(GerritTriggeredEvent, Job) + */ + private static synchronized ParametersActionInspection getParametersInspection() { + if (parametersInspectionCache == null) { + parametersInspectionCache = new ParametersActionInspection(); + } + return parametersInspectionCache; + } + + /** + * Stored cache of the inspection. + * @see #getParametersInspection() + */ + private static volatile ParametersActionInspection parametersInspectionCache = null; + + /** + * Data structure with information regarding what kind of capabilities {@link ParametersAction} has. + * @see #getParametersInspection() + * @see #createParameters(GerritTriggeredEvent, Job) + */ + private static class ParametersActionInspection { + private static final Class KLASS = ParametersAction.class; + private boolean inspectionFailure; + private boolean safeParametersSet = false; + private boolean keepUndefinedParameters = false; + private boolean hasSafeParameterConfig = false; + + /** + * Constructor that performs the inspection. + */ + ParametersActionInspection() { + try { + for (Field field : KLASS.getDeclaredFields()) { + if (Modifier.isStatic(field.getModifiers()) + && ( + field.getName().equals("KEEP_UNDEFINED_PARAMETERS_SYSTEM_PROPERTY_NAME") + || field.getName().equals("SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME") + ) + ) { + this.hasSafeParameterConfig = true; + break; + } + } + if (hasSafeParameterConfig) { + if (Boolean.getBoolean(KLASS.getName() + ".keepUndefinedParameters")) { + this.keepUndefinedParameters = true; + } + String safeParameters = System.getProperty(KLASS.getName() + ".safeParameters"); + if (!StringUtils.isBlank(safeParameters)) { + safeParameters = safeParameters.toUpperCase(Locale.ENGLISH); + boolean declared = true; + for (GerritTriggerParameters parameter : GerritTriggerParameters.values()) { + if (!safeParameters.contains(parameter.name())) { + declared = false; + break; + } + } + this.safeParametersSet = declared; + } else { + this.safeParametersSet = false; + } + } + this.inspectionFailure = false; + } catch (Exception e) { + this.inspectionFailure = true; + } + } + + /** + * If the system property .safeParameters is set and contains all Gerrit related parameters. + * @return true if so. + */ + boolean isSafeParametersSet() { + return safeParametersSet; + } + + /** + * If the system property ..keepSafeParameters is set and set to true. + * + * @return true if so. + */ + boolean isKeepUndefinedParameters() { + return keepUndefinedParameters; + } + + /** + * If any of the constant fields regarding safeParameters are declared in {@link ParametersAction}. + * + * @return true if so. + */ + boolean isHasSafeParameterConfig() { + return hasSafeParameterConfig; + } + + /** + * If there was an exception when inspecting the class. + * + * @return true if so. + */ + public boolean isInspectionFailure() { + return inspectionFailure; + } + } } From 559060945d2ecf9b98abdf95d118deb88a2a3463 Mon Sep 17 00:00:00 2001 From: Robert Sandell Date: Mon, 23 May 2016 14:16:08 +0200 Subject: [PATCH 09/10] Check inspection error first --- .../trigger/hudsontrigger/EventListener.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java index 2d841043d..2a7740f80 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java @@ -263,15 +263,20 @@ protected ParametersAction createParameters(GerritTriggeredEvent event, Job proj return constructor.newInstance(parameters, GerritTriggerParameters.getNamesSet()); } catch (NoSuchMethodException e) { ParametersActionInspection inspection = getParametersInspection(); - if (inspection.isHasSafeParameterConfig()) { + if (inspection.isInspectionFailure()) { + logger.warn("Failed to inspect ParametersAction to determine " + + "if we can behave normally around SECURITY-170.\nSee " + + "https://wiki.jenkins-ci.org/display/SECURITY/Jenkins+Security+Advisory+2016-05-11" + + " for information."); + } else if (inspection.isHasSafeParameterConfig()) { StringBuilder txt = new StringBuilder( "Running on a core with SECURITY-170 fixed but no direct way for Gerrit Trigger" + " to self-specify safe parameters."); - txt.append(" You should consider upgrading to a new Jenkins core version."); + txt.append(" You should consider upgrading to a new Jenkins core version.\n"); if (inspection.isKeepUndefinedParameters()) { - txt.append("\n.keepSafeParameters is set so the trigger should behave normally."); + txt.append(".keepSafeParameters is set so the trigger should behave normally."); } else if (inspection.isSafeParametersSet()) { - txt.append("\nAll Gerrit related parameters are set in .safeParameters"); + txt.append("All Gerrit related parameters are set in .safeParameters"); txt.append(" so the trigger should behave normally."); } else { txt.append("No overriding system properties appears to be set,"); @@ -280,11 +285,6 @@ protected ParametersAction createParameters(GerritTriggeredEvent event, Job proj txt.append(" for information."); } logger.warn(txt.toString()); - } else if (inspection.isInspectionFailure()) { - logger.warn("Failed to inspect ParametersAction to determine " - + "if we can behave normally around SECURITY-170.\nSee " - + "https://wiki.jenkins-ci.org/display/SECURITY/Jenkins+Security+Advisory+2016-05-11" - + " for information."); } else { logger.debug("Running on an old core before safe parameters, we should be safe.", e); } From d8878ba6ff19f38c8520b60bf875c15830cef8a8 Mon Sep 17 00:00:00 2001 From: Robert Sandell Date: Thu, 26 May 2016 17:11:11 +0200 Subject: [PATCH 10/10] Corrected log messages and added to the javadoc --- .../gerrit/trigger/hudsontrigger/EventListener.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java index 2a7740f80..d0bf5ffaf 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/EventListener.java @@ -249,6 +249,10 @@ protected Job asJob() { /** * Creates a ParameterAction and fills it with the project's default parameters + the Standard Gerrit parameters. + * If running on a core version that let's us specify safeParameters for the ParameterAction + * the Gerrit specific parameters will be specified in the safeParameters list in addition to anything the admin + * might have set. + * A warning will be printed to the log if that is not possible but SECURITY-170 appears to be in effect. * * @param event the event. * @param project the project. @@ -274,7 +278,7 @@ protected ParametersAction createParameters(GerritTriggeredEvent event, Job proj + " to self-specify safe parameters."); txt.append(" You should consider upgrading to a new Jenkins core version.\n"); if (inspection.isKeepUndefinedParameters()) { - txt.append(".keepSafeParameters is set so the trigger should behave normally."); + txt.append(".keepUndefinedParameters is set so the trigger should behave normally."); } else if (inspection.isSafeParametersSet()) { txt.append("All Gerrit related parameters are set in .safeParameters"); txt.append(" so the trigger should behave normally."); @@ -457,7 +461,7 @@ boolean isSafeParametersSet() { } /** - * If the system property ..keepSafeParameters is set and set to true. + * If the system property .keepUndefinedParameters is set and set to true. * * @return true if so. */