Merge pull request #245 from jglick/s2m

Remove agent-to-controller calls

src/main/java/io/jenkins/plugins/coverage/detector/AntPathReportDetector.java

@@ -5,18 +5,14 @@
 import hudson.FilePath;
 import hudson.model.Run;
 import hudson.model.TaskListener;
-import hudson.remoting.VirtualChannel;
 import io.jenkins.plugins.coverage.adapter.CoverageAdapterDescriptor;
 import io.jenkins.plugins.coverage.exception.CoverageException;
-import jenkins.MasterToSlaveFileCallable;
 import org.jenkinsci.Symbol;
 import org.kohsuke.stapler.DataBoundConstructor;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
-import java.util.stream.Collectors;
 
 public class AntPathReportDetector extends ReportDetector {
 
@@ -30,12 +26,7 @@ public AntPathReportDetector(String path) {
     @Override
     public List<FilePath> findFiles(Run<?, ?> run, FilePath workspace, TaskListener listener) throws CoverageException {
         try {
-            return Arrays.stream(workspace.act(new MasterToSlaveFileCallable<FilePath[]>() {
-                @Override
-                public FilePath[] invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
-                    return workspace.list(path);
-                }
-            })).collect(Collectors.toList());
+            return Arrays.asList(workspace.list(path));
         } catch (IOException | InterruptedException e) {
             throw new CoverageException(e);
         }

src/main/java/io/jenkins/plugins/coverage/source/DefaultSourceFileResolver.java

@@ -82,7 +82,7 @@ public void resolveSourceFiles(Run<?, ?> run, FilePath workspace, TaskListener l
         final Map<String, FilePath> sourceFileMapping = createSourceFileMapping(workspace, listener);
 
         paints.forEach((sourceFilePath, paint) -> {
-            final FilePath buildDirSourceFile = new FilePath(new File(runRootDir, DEFAULT_SOURCE_CODE_STORE_DIRECTORY + sanitizeFilename(sourceFilePath)));
+            final File buildDirSourceFile = new File(runRootDir, DEFAULT_SOURCE_CODE_STORE_DIRECTORY + sanitizeFilename(sourceFilePath));
 
             try {
                 listener.getLogger().printf("Starting copy source file %s. %n", sourceFilePath);
@@ -92,17 +92,13 @@ public void resolveSourceFiles(Run<?, ?> run, FilePath workspace, TaskListener l
                     possibleParentPaths = Collections.emptySet();
                 }
 
-                final boolean copiedSucceed = workspace.act(new SourceFilePainter(
+                FileUtils.write(buildDirSourceFile, workspace.act(new SourceFilePainter(
                         sourceFilePath,
                         paint,
-                        buildDirSourceFile,
                         possibleParentPaths,
                         sourceFileMapping
-                ));
-                if (copiedSucceed) {
-                    listener.getLogger().printf("Copied %s. %n", sourceFilePath);
-
-                }
+                )), StandardCharsets.UTF_8);
+                listener.getLogger().printf("Copied %s. %n", sourceFilePath);
 
             } catch (IOException | InterruptedException e) {
                 listener.getLogger().println(e.getMessage());
@@ -152,44 +148,39 @@ public ListBoxModel doFillLevelItems() {
         }
     }
 
-    private static class SourceFilePainter extends MasterToSlaveFileCallable<Boolean> {
+    private static class SourceFilePainter extends MasterToSlaveFileCallable<String> {
         private static final long serialVersionUID = 6548573019315830249L;
 
         private final String sourceFilePath;
         private final Set<String> possiblePaths;
         private final CoveragePaint paint;
-        private final FilePath destination;
         private final Map<String, FilePath> sourceFileMapping;
 
         SourceFilePainter(
                 @NonNull String sourceFilePath,
                 @NonNull CoveragePaint paint,
-                @NonNull FilePath destination,
                 @NonNull Set<String> possiblePaths,
                 @NonNull Map<String, FilePath> sourceFileMapping
         ) {
             this.sourceFilePath = sourceFilePath;
             this.paint = paint;
-            this.destination = destination;
             this.possiblePaths = possiblePaths;
             this.sourceFileMapping = sourceFileMapping;
         }
 
         @Override
-        public Boolean invoke(File workspace, VirtualChannel channel) throws IOException {
+        public String invoke(File workspace, VirtualChannel channel) throws IOException {
             FilePath sourceFile = tryFindSourceFile(workspace);
             if (sourceFile == null) {
                 throw new IOException(
                         String.format("Unable to find source file %s in workspace %s", sourceFilePath, workspace.getAbsolutePath()));
             }
 
             try {
-                paintSourceCode(sourceFile, paint, destination);
+                return paintSourceCode(sourceFile, paint);
             } catch (CoverageException e) {
                 throw new IOException(e);
             }
-
-            return true;
         }
 
         private FilePath tryFindSourceFile(File workspace) {
@@ -239,8 +230,8 @@ private boolean isValidSourceFile(File sourceFile) {
             return sourceFile.exists() && sourceFile.isFile() && sourceFile.canRead();
         }
 
-        private void paintSourceCode(FilePath source, CoveragePaint paint, FilePath canvas) throws CoverageException {
-            try (BufferedWriter output = new BufferedWriter(new OutputStreamWriter(canvas.write(), StandardCharsets.UTF_8));
+        private String paintSourceCode(FilePath source, CoveragePaint paint) throws CoverageException {
+            try (StringWriter output = new StringWriter();
                  BufferedReader input = new BufferedReader(new InputStreamReader(source.read(), StandardCharsets.UTF_8))) {
                 int line = 0;
                 String content;
@@ -276,6 +267,7 @@ private void paintSourceCode(FilePath source, CoveragePaint paint, FilePath canv
                 }
 
                 paint.setTotalLines(line);
+                return output.toString();
             } catch (IOException | InterruptedException e) {
                 throw new CoverageException(e);
             }

src/test/java/io/jenkins/plugins/coverage/BlockSlaveToMasterFileCallable.java

@@ -0,0 +1,52 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2021 CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package io.jenkins.plugins.coverage;
+
+import edu.umd.cs.findbugs.annotations.Nullable;
+import hudson.Extension;
+import hudson.remoting.ChannelBuilder;
+import java.io.File;
+import jenkins.ReflectiveFilePathFilter;
+import jenkins.SlaveToMasterFileCallable;
+import jenkins.security.ChannelConfigurator;
+
+/**
+ * Prevents all {@link SlaveToMasterFileCallable}s from running during tests, to make sure we do not rely on them.
+ */
+public class BlockSlaveToMasterFileCallable extends ReflectiveFilePathFilter {
+
+    @Override protected boolean op(String name, File path) throws SecurityException {
+        throw new SecurityException("refusing to " + name + " on " + path);
+    }
+
+    @Extension public static class ChannelConfiguratorImpl extends ChannelConfigurator {
+
+        @Override public void onChannelBuilding(ChannelBuilder builder, @Nullable Object context) {
+            new BlockSlaveToMasterFileCallable().installTo(builder, 1000); // higher priority than, say, AdminFilePathFilter or even DefaultFilePathFilter
+        }
+
+    }
+
+}

src/test/java/io/jenkins/plugins/coverage/CoveragePublisherPipelineTest.java

@@ -4,8 +4,10 @@
 import java.util.Objects;
 
 import org.junit.Assert;
+import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.BuildWatcher;
 import org.jvnet.hudson.test.JenkinsRule;
 
 import com.google.common.collect.Lists;
@@ -15,6 +17,7 @@
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import hudson.FilePath;
 import hudson.model.Result;
+import hudson.slaves.DumbSlave;
 
 import io.jenkins.plugins.coverage.adapter.CoberturaReportAdapter;
 import io.jenkins.plugins.coverage.adapter.JacocoReportAdapter;
@@ -24,6 +27,8 @@
 
 public class CoveragePublisherPipelineTest {
 
+    @ClassRule
+    public static BuildWatcher bw = new BuildWatcher();
 
     @Rule
     public JenkinsRule j = new JenkinsRule();
@@ -317,14 +322,17 @@ public void testAbsolutePathSourceFile() throws Exception {
 
     @Test
     public void testRelativePathSourceFile() throws Exception {
+        DumbSlave agent = j.createOnlineSlave();
+
         CoverageScriptedPipelineScriptBuilder builder = CoverageScriptedPipelineScriptBuilder.builder()
+                .onAgent(agent)
                 .setEnableSourceFileResolver(true);
 
         CoberturaReportAdapter adapter = new CoberturaReportAdapter("cobertura-coverage.xml");
         builder.addAdapter(adapter);
 
         WorkflowJob project = j.createProject(WorkflowJob.class, "coverage-pipeline-test");
-        FilePath workspace = j.jenkins.getWorkspaceFor(project);
+        FilePath workspace = agent.getWorkspaceFor(project);
 
         Objects.requireNonNull(workspace)
                 .child("cobertura-coverage.xml")

src/test/java/io/jenkins/plugins/coverage/CoverageScriptedPipelineScriptBuilder.java

@@ -1,6 +1,7 @@
 package io.jenkins.plugins.coverage;
 
 import hudson.model.Descriptor;
+import hudson.model.Slave;
 import io.jenkins.plugins.coverage.adapter.CoverageAdapter;
 import io.jenkins.plugins.coverage.adapter.CoverageReportAdapter;
 import io.jenkins.plugins.coverage.detector.AntPathReportDetector;
@@ -21,6 +22,7 @@ public class CoverageScriptedPipelineScriptBuilder {
     private boolean applyThresholdRecursively;
 
     private boolean enableSourceFileResolver;
+    private Slave agent;
 
     private CoverageScriptedPipelineScriptBuilder() {
     }
@@ -40,10 +42,18 @@ public CoverageScriptedPipelineScriptBuilder addGlobalThreshold(Threshold thresh
         return this;
     }
 
+    public CoverageScriptedPipelineScriptBuilder onAgent(Slave slave) {
+        this.agent = slave;
+        return this;
+    }
 
     public String build() {
         StringBuilder sb = new StringBuilder();
-        sb.append("node {")
+        sb.append("node");
+        if (agent != null) {
+            sb.append("('").append(agent.getNodeName()).append("')");
+        }
+        sb.append(" {")
                 .append("publishCoverage(");
 
         sb.append("failUnhealthy:").append(failUnhealthy);