[JENKINS-39134] Deprecate injection

src/main/java/org/jenkinsci/plugins/workflow/steps/AbstractStepDescriptorImpl.java

@@ -9,7 +9,9 @@
 
 /**
  * @author Kohsuke Kawaguchi
+ * @deprecated Directly extend {@link StepDescriptor} and avoid Guice.
  */
+@Deprecated
 public abstract class AbstractStepDescriptorImpl extends StepDescriptor {
     private volatile transient Set<Class<?>> contextTypes;
 

src/main/java/org/jenkinsci/plugins/workflow/steps/AbstractStepExecutionImpl.java

@@ -16,7 +16,9 @@
  * make sure the {@link Step} is {@link Serializable} and do not mark it {@code transient}.
  * (For a {@link AbstractSynchronousStepExecution} these considerations are irrelevant.)
  * @author Kohsuke Kawaguchi
+ * @deprecated Directly extend {@link StepExecution} and avoid Guice.
  */
+@Deprecated
 public abstract class AbstractStepExecutionImpl extends StepExecution {
 
     protected AbstractStepExecutionImpl() {

src/main/java/org/jenkinsci/plugins/workflow/steps/AbstractStepImpl.java

@@ -1,33 +1,18 @@
 package org.jenkinsci.plugins.workflow.steps;
 
 import com.google.inject.Injector;
-import hudson.model.Describable;
-import hudson.model.Descriptor;
 import jenkins.model.Jenkins;
-import net.sf.json.JSONObject;
-import org.kohsuke.stapler.DataBoundConstructor;
-import org.kohsuke.stapler.DataBoundSetter;
-import org.kohsuke.stapler.StaplerRequest;
 
 import javax.annotation.Nullable;
 import javax.inject.Inject;
 
 /**
  * Partial convenient step implementation.
- *
- * <h2>Parameter injection</h2>
- * <p>
- * {@link Step} implementations are expected to follow the usual GUI-instantiable {@link Describable} pattern.
- * {@link AbstractStepImpl} comes with {@linkplain AbstractStepDescriptorImpl a partial implementation of StepDescriptor}
- * that automatically instantiate a Step subtype and perform {@link DataBoundConstructor}/{@link DataBoundSetter}
- * injections just like {@link Descriptor#newInstance(StaplerRequest, JSONObject)} does from JSON.
- *
- * <p>
- * In addition, fields and setter methods annotated with {@link StepContextParameter} will get its value
- * injected from {@link StepContext}.
- *
+ * Used with {@link AbstractStepDescriptorImpl} and {@link AbstractStepExecutionImpl}.
  * @author Kohsuke Kawaguchi
+ * @deprecated Directly extend {@link Step} and avoid Guice.
  */
+@Deprecated
 public abstract class AbstractStepImpl extends Step {
 
     /** Constructs a step execution automatically according to {@link AbstractStepDescriptorImpl#getExecutionType}. */

src/main/java/org/jenkinsci/plugins/workflow/steps/AbstractSynchronousNonBlockingStepExecution.java

@@ -2,11 +2,6 @@
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.security.ACL;
-import hudson.util.DaemonThreadFactory;
-import hudson.util.NamingThreadFactory;
-
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import javax.annotation.Nonnull;
 import jenkins.model.Jenkins;
@@ -17,14 +12,14 @@
  * Similar to {@link AbstractSynchronousStepExecution} (it executes synchronously too) but it does not block the CPS VM thread.
  * @see StepExecution
  * @param <T> the type of the return value (may be {@link Void})
+ * @deprecated Extend {@link SynchronousNonBlockingStepExecution} and avoid Guice.
  */
+@Deprecated
 public abstract class AbstractSynchronousNonBlockingStepExecution<T> extends AbstractStepExecutionImpl {
 
     private transient volatile Future<?> task;
     private transient String threadName;
 
-    private static ExecutorService executorService;
-
     protected AbstractSynchronousNonBlockingStepExecution() {
     }
 
@@ -42,7 +37,7 @@ protected AbstractSynchronousNonBlockingStepExecution(StepContext context) {
     @Override
     public final boolean start() throws Exception {
         final Authentication auth = Jenkins.getAuthentication();
-        task = getExecutorService().submit(new Runnable() {
+        task = SynchronousNonBlockingStepExecution.getExecutorService().submit(new Runnable() {
             @SuppressFBWarnings(value="SE_BAD_FIELD", justification="not serializing anything here")
             @Override public void run() {
                 try {
@@ -83,11 +78,4 @@ public void onResume() {
         }
     }
 
-    private static synchronized ExecutorService getExecutorService() {
-        if (executorService == null) {
-            executorService = Executors.newCachedThreadPool(new NamingThreadFactory(new DaemonThreadFactory(), "org.jenkinsci.plugins.workflow.steps.AbstractSynchronousNonBlockingStepExecution"));
-        }
-        return executorService;
-    }
-
 }

src/main/java/org/jenkinsci/plugins/workflow/steps/AbstractSynchronousStepExecution.java

@@ -15,7 +15,9 @@
  *
  * @param <T> the type of the return value (may be {@link Void})
  * @author Kohsuke Kawaguchi
+ * @deprecated Extend {@link SynchronousStepExecution} and avoid Guice.
  */
+@Deprecated
 public abstract class AbstractSynchronousStepExecution<T> extends AbstractStepExecutionImpl {
     private transient volatile Thread executing;
 

src/main/java/org/jenkinsci/plugins/workflow/steps/ContextParameterModule.java

@@ -10,7 +10,6 @@
 
 import javax.annotation.Nullable;
 import java.io.IOException;
-import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -20,7 +19,9 @@
 
 /**
  * @author Kohsuke Kawaguchi
+ * @deprecated Guice subsystem is deprecated.
  */
+@Deprecated
 class ContextParameterModule extends AbstractModule {
 
     private static final Logger LOGGER = Logger.getLogger(ContextParameterModule.class.getName());

src/main/java/org/jenkinsci/plugins/workflow/steps/MissingContextVariableException.java

@@ -47,7 +47,7 @@ public Class<?> getType() {
     /**
      * Is type in the given set, in consideration of the subtyping relationship?
      */
-    private boolean isIn(Set<Class<?>> classes) {
+    private boolean isIn(Set<? extends Class<?>> classes) {
         for (Class<?> t : classes)
             if (type.isAssignableFrom(t))
                 return true;

src/main/java/org/jenkinsci/plugins/workflow/steps/StepContextParameter.java

@@ -15,7 +15,9 @@
  *
  * @author Kohsuke Kawaguchi
  * @see AbstractStepImpl
+ * @deprecated Call {@link StepContext#get} as needed and avoid Guice.
  */
+@Deprecated
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.METHOD,ElementType.FIELD})
 @Documented

src/main/java/org/jenkinsci/plugins/workflow/steps/StepDescriptor.java

@@ -60,7 +60,7 @@ public abstract class StepDescriptor extends Descriptor<Step> {
      * (say in freestyle or in workflow).
      * @see StepContext#get(Class)
      */
-    public abstract Set<Class<?>> getRequiredContext();
+    public abstract Set<? extends Class<?>> getRequiredContext();
 
     /**
      * Returns the context {@link Step} adds/sets/modifies when executing a body.
@@ -76,7 +76,7 @@ public abstract class StepDescriptor extends Descriptor<Step> {
      *
      * @see MissingContextVariableException
      */
-    public Set<Class<?>> getProvidedContext() {
+    public Set<? extends Class<?>> getProvidedContext() {
         return Collections.emptySet();
     }
 

src/main/java/org/jenkinsci/plugins/workflow/steps/StepExecution.java

@@ -37,14 +37,16 @@ public abstract class StepExecution implements Serializable {
     /**
      * Default constructor used by injection.
      * @see AbstractStepImpl#start
+     * @deprecated Avoid Guice.
      */
+    @Deprecated
     protected StepExecution() {
     }
 
     /**
      * If manually created, {@link StepContext} must be passed in.
      */
-    protected StepExecution(StepContext context) {
+    protected StepExecution(@Nonnull StepContext context) {
         this.context = context;
     }
 

src/main/java/org/jenkinsci/plugins/workflow/steps/SynchronousNonBlockingStepExecution.java

@@ -0,0 +1,90 @@
+package org.jenkinsci.plugins.workflow.steps;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import hudson.security.ACL;
+import hudson.util.DaemonThreadFactory;
+import hudson.util.NamingThreadFactory;
+
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import javax.annotation.Nonnull;
+import jenkins.model.Jenkins;
+import jenkins.security.NotReallyRoleSensitiveCallable;
+import org.acegisecurity.Authentication;
+
+/**
+ * Similar to {@link SynchronousStepExecution} (it executes synchronously too) but it does not block the CPS VM thread.
+ * @see StepExecution
+ * @param <T> the type of the return value (may be {@link Void})
+ */
+public abstract class SynchronousNonBlockingStepExecution<T> extends StepExecution {
+
+    private transient volatile Future<?> task;
+    private transient String threadName;
+
+    private static ExecutorService executorService;
+
+    protected SynchronousNonBlockingStepExecution(@Nonnull StepContext context) {
+        super(context);
+    }
+
+    /**
+     * Meat of the execution.
+     *
+     * When this method returns, a step execution is over.
+     */
+    protected abstract T run() throws Exception;
+
+    @Override
+    public final boolean start() throws Exception {
+        final Authentication auth = Jenkins.getAuthentication();
+        task = getExecutorService().submit(new Runnable() {
+            @SuppressFBWarnings(value="SE_BAD_FIELD", justification="not serializing anything here")
+            @Override public void run() {
+                try {
+                    getContext().onSuccess(ACL.impersonate(auth, new NotReallyRoleSensitiveCallable<T, Exception>() {
+                        @Override public T call() throws Exception {
+                            threadName = Thread.currentThread().getName();
+                            return SynchronousNonBlockingStepExecution.this.run();
+                        }
+                    }));
+                } catch (Exception e) {
+                    getContext().onFailure(e);
+                }
+            }
+        });
+        return false;
+    }
+
+    /**
+     * If the computation is going synchronously, try to cancel that.
+     */
+    @Override
+    public void stop(Throwable cause) throws Exception {
+        if (task != null) {
+            task.cancel(true);
+        }
+    }
+
+    @Override
+    public void onResume() {
+        getContext().onFailure(new Exception("Resume after a restart not supported for non-blocking synchronous steps"));
+    }
+
+    @Override public @Nonnull String getStatus() {
+        if (threadName != null) {
+            return "running in thread: " + threadName;
+        } else {
+            return "not yet scheduled";
+        }
+    }
+
+    static synchronized ExecutorService getExecutorService() {
+        if (executorService == null) {
+            executorService = Executors.newCachedThreadPool(new NamingThreadFactory(new DaemonThreadFactory(), "org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution"));
+        }
+        return executorService;
+    }
+
+}

src/main/java/org/jenkinsci/plugins/workflow/steps/SynchronousStepExecution.java

@@ -0,0 +1,61 @@
+package org.jenkinsci.plugins.workflow.steps;
+
+import hudson.model.Executor;
+import hudson.model.Result;
+
+import static hudson.model.Result.ABORTED;
+import javax.annotation.Nonnull;
+
+/**
+ * {@link StepExecution} that always executes synchronously. This API should be used for short-lived tasks that
+ * return almost instantly.
+ *
+ * To call legacy Jenkins APIs which are potentially long-running and interruptible yet offer no asynchronous mode
+ * (for example because they block on a remoting call) use {@link SynchronousNonBlockingStepExecution}.
+ * Also note that long-lived tasks which do not need to run within a Java method call should use the more general {@link StepExecution}.
+ *
+ * @param <T> the type of the return value (may be {@link Void})
+ * @author Kohsuke Kawaguchi
+ */
+public abstract class SynchronousStepExecution<T> extends StepExecution {
+    private transient volatile Thread executing;
+
+    protected SynchronousStepExecution(@Nonnull StepContext context) {
+        super(context);
+    }
+
+    /**
+     * Meat of the execution.
+     *
+     * When this method returns, a step execution is over.
+     */
+    protected abstract T run() throws Exception;
+
+    @Override
+    public final boolean start() throws Exception {
+        executing = Thread.currentThread();
+        try {
+            getContext().onSuccess(run());
+        } catch (Throwable t) {
+            getContext().onFailure(t);
+        } finally {
+            executing = null;
+        }
+        return true;
+    }
+
+    /**
+     * If the computation is going synchronously, try to cancel that.
+     */
+    @Override
+    public void stop(Throwable cause) throws Exception {
+        Thread e = executing;   // capture
+        if (e!=null) {
+            if (e instanceof Executor) {
+                ((Executor) e).interrupt(ABORTED, new ExceptionCause(cause));
+            } else {
+                e.interrupt();
+            }
+        }
+    }
+}

src/test/java/org/jenkinsci/plugins/workflow/steps/AbstractStepImplTest.java

@@ -3,7 +3,6 @@
 import hudson.Extension;
 import hudson.model.Node;
 import jenkins.model.Jenkins;
-import org.codehaus.groovy.runtime.GStringImpl;
 import static org.junit.Assert.*;
 import org.junit.Before;
 import org.junit.Rule;
@@ -55,20 +54,6 @@ public void inject() throws Exception {
         b.start();
     }
 
-    /**
-     * Regular groovy method call coerces types, and we want to do the same here.
-     */
-    @Test
-    public void coercion() throws Exception {
-        Map r = new HashMap();
-        r.put("a",10L); // long -> int
-        r.put("b",new GStringImpl(new Object[]{1},new String[]{"pre","post"})); // GString -> String
-        BogusStep step = (BogusStep) d.newInstance(r);
-
-        assertEquals(step.a, 10);
-        assertEquals(step.b, "pre1post");
-    }
-
     public static class BogusStep extends AbstractStepImpl {
         int a;
         String b;