Merge pull request #50 from jglick/StepExecution.stop-JENKINS-26148

[JENKINS-26148] Using default implementation of StepExecution.stop
jenkinsci · Nov 6, 2017 · 1b1342d · 1b1342d
2 parents 2a03129 + e2eea4f
commit 1b1342d
Show file tree

Hide file tree

Showing 10 changed files with 7 additions and 46 deletions.
diff --git a/pom.xml b/pom.xml
@@ -28,7 +28,7 @@
     <parent>
         <groupId>org.jenkins-ci.plugins</groupId>
         <artifactId>plugin</artifactId>
-        <version>2.31</version>
+        <version>2.33</version>
         <relativePath />
     </parent>
     <groupId>org.jenkins-ci.plugins.workflow</groupId>
@@ -63,7 +63,7 @@
     </pluginRepositories>
     <properties>
         <jenkins.version>2.7.3</jenkins.version>
-        <workflow-step-api-plugin.version>2.12</workflow-step-api-plugin.version>
+        <workflow-step-api-plugin.version>2.13</workflow-step-api-plugin.version>
         <workflow-cps-plugin.version>2.32</workflow-cps-plugin.version>
         <workflow-support-plugin.version>2.14</workflow-support-plugin.version>
     </properties>

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java
@@ -84,10 +84,6 @@ public static final class Execution extends AbstractStepExecutionImpl {
             return false;
         }
 
-        @Override public void stop(Throwable cause) throws Exception {
-            // nothing to do
-        }
-
         @Override public void onResume() {}
 
         private static final class Callback extends BodyExecutionCallback {

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/CoreWrapperStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/CoreWrapperStep.java
@@ -92,10 +92,6 @@ public static final class Execution extends AbstractStepExecutionImpl {
             return false;
         }
 
-        @Override public void stop(Throwable cause) throws Exception {
-            // should be no need to do anything special (but verify in JENKINS-26148)
-        }
-
         @Override public void onResume() {}
 
     }

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java
@@ -89,10 +89,6 @@ public static class Execution extends AbstractStepExecutionImpl {
             return false;
         }
 
-        @Override public void stop(Throwable cause) throws Exception {
-            // should be no need to do anything special (but verify in JENKINS-26148)
-        }
-
         @Override public void onResume() {}
 
     }

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/PushdStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/PushdStep.java
@@ -80,7 +80,6 @@ public static class Execution extends AbstractStepExecutionImpl {
 
         @SuppressFBWarnings(value="SE_TRANSIENT_FIELD_NOT_RESTORED", justification="Only used when starting.")
         private transient final String path;
-        private BodyExecution body;
 
         Execution(String path, StepContext context) {
             super(context);
@@ -90,20 +89,14 @@ public static class Execution extends AbstractStepExecutionImpl {
         @Override public boolean start() throws Exception {
             FilePath dir = getContext().get(FilePath.class).child(path);
             getContext().get(TaskListener.class).getLogger().println("Running in " + dir);
-            body = getContext().newBodyInvoker()
+            getContext().newBodyInvoker()
                     .withContext(dir)
                     // Could use a dedicated BodyExecutionCallback here if we wished to print a message at the end ("Returning to ${cwd}"):
                     .withCallback(BodyExecutionCallback.wrap(getContext()))
                     .start();
             return false;
         }
 
-        @Override
-        public void stop(Throwable cause) throws Exception {
-            if (body!=null)
-                body.cancel(cause);
-        }
-
         @Override public void onResume() {}
 
         private static final long serialVersionUID = 1L;

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/RetryStepExecution.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/RetryStepExecution.java
@@ -13,7 +13,6 @@ public class RetryStepExecution extends AbstractStepExecutionImpl {
 
     @SuppressFBWarnings(value="SE_TRANSIENT_FIELD_NOT_RESTORED", justification="Only used when starting.")
     private transient final int count;
-    private volatile BodyExecution body;
 
     RetryStepExecution(int count, StepContext context) {
         super(context);
@@ -23,18 +22,12 @@ public class RetryStepExecution extends AbstractStepExecutionImpl {
     @Override
     public boolean start() throws Exception {
         StepContext context = getContext();
-        body = context.newBodyInvoker()
+        context.newBodyInvoker()
             .withCallback(new Callback(count))
             .start();
         return false;   // execution is asynchronous
     }
 
-    @Override
-    public void stop(Throwable cause) throws Exception {
-        if (body!=null)
-            body.cancel(cause);
-    }
-
     @Override public void onResume() {}
 
     private static class Callback extends BodyExecutionCallback {

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/SleepStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/SleepStep.java
@@ -111,7 +111,7 @@ private void setupTimer(long now) {
             if (task != null) {
                 task.cancel(false);
             }
-            getContext().onFailure(cause);
+            super.stop(cause);
         }
 
         @Override public void onResume() {

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/TimeoutStepExecution.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/TimeoutStepExecution.java
@@ -138,12 +138,6 @@ private void cancel() {
         }
     }
 
-    @Override
-    public void stop(Throwable cause) throws Exception {
-        if (body!=null)
-            body.cancel(cause);
-    }
-
     @Override public String getStatus() {
         if (killer == null) {
             return "killer task nowhere to be found";

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/WaitForConditionStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/WaitForConditionStep.java
@@ -48,7 +48,7 @@ public final class WaitForConditionStep extends Step {
     public static final class Execution extends AbstractStepExecutionImpl {
 
         private static final long serialVersionUID = 1;
-        private volatile BodyExecution body;
+        private volatile BodyExecution body; // TODO could be replaced with a simple boolean flag
         private transient volatile ScheduledFuture<?> task;
         /**
          * TODO JENKINS-26148 is there no cleaner way of finding the StepExecution that created a BodyExecutionCallback?
@@ -70,13 +70,10 @@ public static final class Execution extends AbstractStepExecutionImpl {
         }
 
         @Override public void stop(Throwable cause) throws Exception {
-            if (body != null) {
-                body.cancel(cause);
-            }
             if (task != null) {
                 task.cancel(false);
-                getContext().onFailure(cause);
             }
+            super.stop(cause);
         }
 
         @Override public void onResume() {

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/WithContextStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/WithContextStep.java
@@ -96,10 +96,6 @@ public static class Execution extends AbstractStepExecutionImpl {
             return false;
         }
 
-        @Override public void stop(Throwable cause) throws Exception {
-            getContext().onFailure(cause);
-        }
-
         @Override public void onResume() {}
 
     }