From c60735a1242b8b06817c2fd0feeb9dc868750948 Mon Sep 17 00:00:00 2001 From: Akbashev Alexander Date: Tue, 22 Aug 2017 10:33:10 +0200 Subject: [PATCH] [JENKINS-29537] EnvironmentContributingAction compatible with Workflow (#2975) * [JENKINS-29537] EnvironmentContributingAction compatible with Workflow + Adds new method with default implementation in EnvironmentContributingAction to support Runs + Marks AbstractBuild as deprecated + Adds default implementation for deprecated method for backward compatiblity. * Tiny improvements in javadoc --- .../main/java/hudson/model/AbstractBuild.java | 2 +- .../model/EnvironmentContributingAction.java | 37 +++- .../java/hudson/model/ParametersAction.java | 5 +- core/src/main/java/hudson/model/Run.java | 3 + .../EnvironmentContributingActionTest.java | 159 ++++++++++++++++++ .../hudson/model/ParametersActionTest.java | 2 +- 6 files changed, 203 insertions(+), 5 deletions(-) create mode 100644 core/src/test/java/hudson/model/EnvironmentContributingActionTest.java diff --git a/core/src/main/java/hudson/model/AbstractBuild.java b/core/src/main/java/hudson/model/AbstractBuild.java index c8674fb028..2b7775c304 100644 --- a/core/src/main/java/hudson/model/AbstractBuild.java +++ b/core/src/main/java/hudson/model/AbstractBuild.java @@ -887,7 +887,7 @@ public abstract class AbstractBuild

,R extends Abs e.buildEnvVars(env); for (EnvironmentContributingAction a : getActions(EnvironmentContributingAction.class)) - a.buildEnvVars(this,env); + a.buildEnvVars(this,env,getBuiltOn()); EnvVars.resolve(env); diff --git a/core/src/main/java/hudson/model/EnvironmentContributingAction.java b/core/src/main/java/hudson/model/EnvironmentContributingAction.java index 761ed8ae97..55f5035965 100644 --- a/core/src/main/java/hudson/model/EnvironmentContributingAction.java +++ b/core/src/main/java/hudson/model/EnvironmentContributingAction.java @@ -24,9 +24,15 @@ package hudson.model; import hudson.EnvVars; +import hudson.Util; import hudson.model.Queue.Task; import hudson.tasks.Builder; import hudson.tasks.BuildWrapper; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.ProtectedExternally; + +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; /** * {@link Action} that contributes environment variables during a build. @@ -44,13 +50,42 @@ import hudson.tasks.BuildWrapper; * @see BuildWrapper */ public interface EnvironmentContributingAction extends Action { + /** + * Called by {@link Run} or {@link AbstractBuild} to allow plugins to contribute environment variables. + * + * @param run + * The calling build. Never null. + * @param node + * The node execute on. Can be {@code null} when the Run is not binded to the node, + * e.g. in Pipeline outside the {@code node() step} + * @param env + * Environment variables should be added to this map. + * @since TODO + */ + default void buildEnvVars(@Nonnull Run run, @Nonnull EnvVars env, @CheckForNull Node node) { + if (run instanceof AbstractBuild + && Util.isOverridden(EnvironmentContributingAction.class, + getClass(), "buildEnvVars", AbstractBuild.class, EnvVars.class)) { + buildEnvVars((AbstractBuild) run, env); + } + } + /** * Called by {@link AbstractBuild} to allow plugins to contribute environment variables. * + * @deprecated Use {@link #buildEnvVars(Run, EnvVars, Node)} instead + * * @param build * The calling build. Never null. * @param env * Environment variables should be added to this map. */ - void buildEnvVars(AbstractBuild build, EnvVars env); + @Deprecated + @Restricted(ProtectedExternally.class) + default void buildEnvVars(AbstractBuild build, EnvVars env) { + if (Util.isOverridden(EnvironmentContributingAction.class, + getClass(), "buildEnvVars", Run.class, EnvVars.class, Node.class)) { + buildEnvVars(build, env, build.getBuiltOn()); + } + } } diff --git a/core/src/main/java/hudson/model/ParametersAction.java b/core/src/main/java/hudson/model/ParametersAction.java index 6f7ecca9f8..86e687b17f 100644 --- a/core/src/main/java/hudson/model/ParametersAction.java +++ b/core/src/main/java/hudson/model/ParametersAction.java @@ -138,10 +138,11 @@ public class ParametersAction implements RunAction2, Iterable, Q } } - public void buildEnvVars(AbstractBuild build, EnvVars env) { + @Override + public void buildEnvVars(Run run, EnvVars env, Node node) { for (ParameterValue p : getParameters()) { if (p == null) continue; - p.buildEnvironment(build, env); + p.buildEnvironment(run, env); } } diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java index c467439347..46970882a2 100644 --- a/core/src/main/java/hudson/model/Run.java +++ b/core/src/main/java/hudson/model/Run.java @@ -2301,6 +2301,9 @@ public abstract class Run ,RunT extends Run run, EnvVars env, @CheckForNull Node node) { + wasCalled = true; + } + + boolean wasNewMethodCalled() { + return wasCalled; + } + } + + class OverrideAbstractBuild extends InvisibleAction implements EnvironmentContributingAction { + private boolean wasCalled = false; + + @Override + @SuppressWarnings("deprecation") + public void buildEnvVars(AbstractBuild abstractBuild, EnvVars envVars) { + wasCalled = true; + } + + boolean wasDeprecatedMethodCalled() { + return wasCalled; + } + } + + class OverrideBoth extends InvisibleAction implements EnvironmentContributingAction { + private boolean wasCalledAstractBuild = false; + private boolean wasCalledRun = false; + + @SuppressWarnings("deprecation") + @Override + public void buildEnvVars(AbstractBuild abstractBuild, EnvVars envVars) { + wasCalledAstractBuild = true; + } + + @Override + public void buildEnvVars(Run run, EnvVars env, @CheckForNull Node node) { + wasCalledRun = true; + } + + boolean wasDeprecatedMethodCalled() { + return wasCalledAstractBuild; + } + + boolean wasRunCalled() { + return wasCalledRun; + } + } + + private final EnvVars envVars = mock(EnvVars.class); + + @Test + public void testOverrideRunMethodAndCallNewMethod() throws Exception { + Run run = mock(Run.class); + Node node = mock(Node.class); + + OverrideRun overrideRun = new OverrideRun(); + overrideRun.buildEnvVars(run, envVars, node); + + assertTrue(overrideRun.wasNewMethodCalled()); + } + + /** + * If only non-deprecated method was overridden it would be executed even if someone would call deprecated method. + * @throws Exception if happens. + */ + @Test + @SuppressWarnings("deprecation") + public void testOverrideRunMethodAndCallDeprecatedMethod() throws Exception { + AbstractBuild abstractBuild = mock(AbstractBuild.class); + when(abstractBuild.getBuiltOn()).thenReturn(mock(Node.class)); + + OverrideRun overrideRun = new OverrideRun(); + overrideRun.buildEnvVars(abstractBuild, envVars); + + assertTrue(overrideRun.wasNewMethodCalled()); + } + + /** + * {@link AbstractBuild} should work as before. + * @throws Exception if happens. + */ + @Test + public void testOverrideAbstractBuildAndCallNewMethodWithAbstractBuild() throws Exception { + AbstractBuild abstractBuild = mock(AbstractBuild.class); + Node node = mock(Node.class); + + OverrideAbstractBuild action = new OverrideAbstractBuild(); + action.buildEnvVars(abstractBuild, envVars, node); + + assertTrue(action.wasDeprecatedMethodCalled()); + } + + /** + * {@link Run} should not execute method that was overridden for {@link AbstractBuild}. + * @throws Exception if happens. + */ + @Test + public void testOverrideAbstractBuildAndCallNewMethodWithRun() throws Exception { + Run run = mock(Run.class); + Node node = mock(Node.class); + + OverrideAbstractBuild action = new OverrideAbstractBuild(); + action.buildEnvVars(run, envVars, node); + + assertFalse(action.wasDeprecatedMethodCalled()); + } + + /** + * If someone wants to use overridden deprecated method, it would still work. + * @throws Exception if happens. + */ + @Test + public void testOverrideAbstractBuildAndCallDeprecatedMethod() throws Exception { + AbstractBuild abstractBuild = mock(AbstractBuild.class); + + OverrideAbstractBuild overrideRun = new OverrideAbstractBuild(); + overrideRun.buildEnvVars(abstractBuild, envVars); + + assertTrue(overrideRun.wasDeprecatedMethodCalled()); + } + + @Test + public void testOverrideBothAndCallNewMethod() throws Exception { + Run run = mock(Run.class); + Node node = mock(Node.class); + + OverrideBoth overrideRun = new OverrideBoth(); + overrideRun.buildEnvVars(run, envVars, node); + + assertTrue(overrideRun.wasRunCalled()); + } + + @Test + public void testOverrideBothAndCallDeprecatedMethod() throws Exception { + AbstractBuild abstractBuild = mock(AbstractBuild.class); + + OverrideBoth overrideRun = new OverrideBoth(); + overrideRun.buildEnvVars(abstractBuild, envVars); + + assertTrue(overrideRun.wasDeprecatedMethodCalled()); + } +} \ No newline at end of file diff --git a/core/src/test/java/hudson/model/ParametersActionTest.java b/core/src/test/java/hudson/model/ParametersActionTest.java index d9b6e63e69..182ead6ec1 100644 --- a/core/src/test/java/hudson/model/ParametersActionTest.java +++ b/core/src/test/java/hudson/model/ParametersActionTest.java @@ -105,7 +105,7 @@ public class ParametersActionTest { // Interaction with build EnvVars vars = new EnvVars(); - parametersAction.buildEnvVars(build, vars); + parametersAction.buildEnvVars(build, vars, build.getBuiltOn()); assertEquals(2, vars.size()); parametersAction.createVariableResolver(build); -- GitLab