Close those file handles 😅 (#1398)

plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java

@@ -246,8 +246,6 @@ public class ConfigurationAsCode extends ManagementLink {
             return FormValidation.okWithMarkup("The configuration can be applied");
         } catch (ConfiguratorException | IllegalArgumentException e) {
             return FormValidation.error(e, e.getCause() == null ? e.getMessage() : e.getCause().getMessage());
-        } finally {
-            closeSources(yamlSources);
         }
     }
 
@@ -616,7 +614,6 @@ public class ConfigurationAsCode extends ManagementLink {
         lastTimeLoaded = System.currentTimeMillis();
         ConfigurationContext context = new ConfigurationContext(registry);
         configureWith(YamlUtils.loadFrom(sources, context), context);
-        closeSources(sources);
     }
 
     @Restricted(NoExternalUse.class)
@@ -632,16 +629,6 @@ public class ConfigurationAsCode extends ManagementLink {
         return checkWith(YamlUtils.loadFrom(sources, context), context);
     }
 
-    private void closeSources(List<YamlSource> sources) {
-        for (YamlSource source : sources) {
-            try {
-                source.close();
-            } catch (IOException e) {
-                LOGGER.log(Level.WARNING, "Failed to close YAML Source", e);
-            }
-        }
-    }
-
     /**
      * Recursive search for all {@link #YAML_FILES_PATTERN} in provided base path
      *

plugin/src/main/java/io/jenkins/plugins/casc/yaml/StreamReaderWithSource.java

@@ -1,6 +1,7 @@
 package io.jenkins.plugins.casc.yaml;
 
 import java.io.IOException;
+import java.io.Reader;
 import java.lang.reflect.Field;
 import org.yaml.snakeyaml.reader.StreamReader;
 
@@ -10,8 +11,8 @@ import org.yaml.snakeyaml.reader.StreamReader;
  */
 class StreamReaderWithSource extends StreamReader {
 
-    public StreamReaderWithSource(YamlSource source) throws IOException {
+    public StreamReaderWithSource(YamlSource source, Reader reader) throws IOException {
-        super(source.read());
+        super(reader);
         try {
             final Field f = StreamReader.class.getDeclaredField("name");
             f.setAccessible(true);

plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlReader.java

@@ -1,14 +0,0 @@
-package io.jenkins.plugins.casc.yaml;
-
-import java.io.IOException;
-import java.io.Reader;
-
-/**
- * @author <a href="mailto:nicolas.deloof@gmail.com">Nicolas De Loof</a>
- */
-@FunctionalInterface
-
-public interface YamlReader<T> {
-
-    Reader open(T source) throws IOException;
-}

plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlSource.java

@@ -1,86 +1,46 @@
 package io.jenkins.plugins.casc.yaml;
 
-import java.io.BufferedReader;
-import java.io.IOException;
 import java.io.InputStream;
-import java.io.InputStreamReader;
-import java.io.Reader;
-import java.net.URI;
-import java.net.URL;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import javax.servlet.http.HttpServletRequest;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
-
 /**
  * @author <a href="mailto:nicolas.deloof@gmail.com">Nicolas De Loof</a>
  */
 
-public class YamlSource<T> implements AutoCloseable {
+public class YamlSource<T> {
-
-    public static final YamlReader<String> READ_FROM_URL = config -> {
-        final URL url = URI.create(config).toURL();
-        return new InputStreamReader(url.openStream(), UTF_8);
-    };
-
-    public static final YamlReader<Path> READ_FROM_PATH = Files::newBufferedReader;
-
-    public static final YamlReader<InputStream> READ_FROM_INPUTSTREAM = in -> new InputStreamReader(in, UTF_8);
-
-    public static final YamlReader<HttpServletRequest> READ_FROM_REQUEST = req -> {
-        // TODO get encoding from req.getContentType()
-        return new InputStreamReader(req.getInputStream(), UTF_8);
-    };
 
     public final T source;
 
-    public final YamlReader<T> reader;
+    public YamlSource(T source) {
-
-    public YamlSource(T source, YamlReader<T> reader) {
         this.source = source;
-        this.reader = reader;
     }
 
     public static YamlSource<InputStream> of(InputStream in) {
-        return new YamlSource<>(in, READ_FROM_INPUTSTREAM);
+        return new YamlSource<>(in);
-    }
-
-    public static YamlSource<String> of(URL url) {
-        return new YamlSource<>(url.toExternalForm(), READ_FROM_URL);
     }
 
     public static YamlSource<String> of(String url) {
-        return new YamlSource<>(url, READ_FROM_URL);
+        return new YamlSource<>(url);
     }
 
     public static YamlSource<HttpServletRequest> of(HttpServletRequest req) {
-        return new YamlSource<>(req, YamlSource.READ_FROM_REQUEST);
+        return new YamlSource<>(req);
     }
 
     public static YamlSource<Path> of(Path path) {
-        return new YamlSource<>(path, YamlSource.READ_FROM_PATH);
+        return new YamlSource<>(path);
-    }
-
-    public Reader read() throws IOException {
-        return reader.open(source);
     }
 
     public String source() {
+        if (source instanceof HttpServletRequest) {
+            return ((HttpServletRequest) source).getPathInfo();
+        }
         return source.toString();
     }
 
     @Override
     public String toString() {
-        return "YamlSource: " + source;
+        return "YamlSource: " + source();
-    }
-
-    @Override
-    public void close() throws IOException {
-        if (reader instanceof BufferedReader) {
-            ((BufferedReader) reader).close();
-        } else if (reader instanceof InputStreamReader) {
-            ((InputStreamReader) reader).close();
-        }
     }
 }

plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlUtils.java

@@ -5,10 +5,17 @@ import io.jenkins.plugins.casc.ConfigurationContext;
 import io.jenkins.plugins.casc.ConfiguratorException;
 import io.jenkins.plugins.casc.model.Mapping;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
 import java.io.Reader;
+import java.net.URI;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.Iterator;
 import java.util.List;
 import java.util.logging.Logger;
+import javax.servlet.http.HttpServletRequest;
 import org.yaml.snakeyaml.LoaderOptions;
 import org.yaml.snakeyaml.composer.Composer;
 import org.yaml.snakeyaml.error.YAMLException;
@@ -21,6 +28,8 @@ import org.yaml.snakeyaml.nodes.SequenceNode;
 import org.yaml.snakeyaml.parser.ParserImpl;
 import org.yaml.snakeyaml.resolver.Resolver;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 /**
  * @author <a href="mailto:nicolas.deloof@gmail.com">Nicolas De Loof</a>
  */
@@ -29,13 +38,12 @@ public final class YamlUtils {
 
     public static final Logger LOGGER = Logger.getLogger(ConfigurationAsCode.class.getName());
 
-    public static Node merge(List<YamlSource> configs,
+    public static Node merge(List<YamlSource> sources,
         ConfigurationContext context) throws ConfiguratorException {
         Node root = null;
-        for (YamlSource source : configs) {
+        for (YamlSource<?> source : sources) {
-            try (Reader r = source.read()) {
+            try (Reader reader = reader(source)) {
-
+                final Node node = read(source, reader, context);
-                final Node node = read(source, context);
 
                 if (root == null) {
                     root = node;
@@ -52,11 +60,11 @@ public final class YamlUtils {
         return root;
     }
 
-    public static Node read(YamlSource source, ConfigurationContext context) throws IOException {
+    public static Node read(YamlSource source, Reader reader, ConfigurationContext context) throws IOException {
         LoaderOptions loaderOptions = new LoaderOptions();
         loaderOptions.setMaxAliasesForCollections(context.getYamlMaxAliasesForCollections());
         Composer composer = new Composer(
-            new ParserImpl(new StreamReaderWithSource(source)),
+            new ParserImpl(new StreamReaderWithSource(source, reader)),
             new Resolver(),
             loaderOptions);
         try {
@@ -72,6 +80,21 @@ public final class YamlUtils {
         }
     }
 
+    public static Reader reader(YamlSource<?> source) throws IOException {
+        Object src = source.source;
+        if (src instanceof String) {
+            final URL url = URI.create((String) src).toURL();
+            return new InputStreamReader(url.openStream(), UTF_8);
+        } else if (src instanceof InputStream) {
+            return new InputStreamReader((InputStream) src, UTF_8);
+        } else if (src instanceof HttpServletRequest) {
+            return new InputStreamReader(((HttpServletRequest) src).getInputStream(), UTF_8);
+        } else if (src instanceof Path) {
+            return Files.newBufferedReader((Path) src);
+        }
+        throw new IOException(String.format("Unknown %s", source));
+    }
+
     private static void merge(Node root, Node node, String source) throws ConfiguratorException {
         if (root.getNodeId() != node.getNodeId()) {
             // means one of those yaml file doesn't conform to JCasC schema

plugin/src/test/java/io/jenkins/plugins/casc/yaml/YamlSourceTest.java

@@ -1,10 +1,13 @@
 package io.jenkins.plugins.casc.yaml;
 
 import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.IOException;
 import java.io.InputStream;
-import java.net.MalformedURLException;
-import java.net.URL;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
+import javax.servlet.http.HttpServletRequest;
+import org.eclipse.jetty.server.Request;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
@@ -12,11 +15,11 @@ import static org.junit.Assert.assertEquals;
 public class YamlSourceTest {
 
     @Test
-    public void shouldHaveInformativeToStringForUrlSource() throws MalformedURLException {
+    public void shouldHaveInformativeToStringForUrlSource() throws IOException {
         //given
         String testUrl = "http://example.com/foo/bar";
         //and
-        YamlSource<String> yamlSource = YamlSource.of(new URL(testUrl));
+        YamlSource<String> yamlSource = YamlSource.of(testUrl);
         //expect
         assertEquals("YamlSource: " + testUrl, yamlSource.toString());
     }
@@ -31,4 +34,24 @@ public class YamlSourceTest {
         //expect
         assertEquals("YamlSource: " + testInputStreamToString, yamlSource.toString());
     }
+
+    @Test
+    public void shouldHaveInformativeToStringForPathSource() {
+        Path path = new File("./test").toPath();
+        String testPathToString = path.toString();
+        YamlSource<Path> yamlSource = YamlSource.of(path);
+        assertEquals("YamlSource: " + testPathToString, yamlSource.toString());
+    }
+
+    @Test
+    public void shouldHaveInformativeToStringForRequestSource() {
+        Request request = new Request(null, null) {
+            @Override
+            public String getPathInfo() {
+                return "/configuration-as-code/check";
+            }
+        };
+        YamlSource<HttpServletRequest> yamlSource = YamlSource.of(request);
+        assertEquals("YamlSource: /configuration-as-code/check", yamlSource.toString());
+    }
 }

test-harness/src/test/java/io/jenkins/plugins/casc/YamlReaderTest.java

@@ -0,0 +1,92 @@
+package io.jenkins.plugins.casc;
+
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.WebRequest;
+import io.jenkins.plugins.casc.misc.JenkinsConfiguredWithCodeRule;
+import io.jenkins.plugins.casc.yaml.YamlSource;
+import io.jenkins.plugins.casc.yaml.YamlUtils;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Paths;
+import java.text.MessageFormat;
+import jenkins.model.Jenkins;
+import org.junit.Rule;
+import org.junit.Test;
+
+import static com.gargoylesoftware.htmlunit.HttpMethod.POST;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertEquals;
+
+public class YamlReaderTest {
+
+    @Rule
+    public JenkinsConfiguredWithCodeRule j = new JenkinsConfiguredWithCodeRule();
+
+    @Test(expected = IOException.class)
+    public void unknownReader() throws IOException {
+        YamlUtils.reader(new YamlSource<>(new StringBuilder()));
+    }
+
+    @Test
+    public void folder() throws Exception {
+        String p = Paths.get(getClass().getResource("./folder").toURI()).toFile().getAbsolutePath();
+        ConfigurationAsCode.get().configure(p);
+        Jenkins jenkins = Jenkins.get();
+        assertEquals("configuration as code - JenkinsConfigTestFolder", jenkins.getSystemMessage());
+        assertEquals(10, jenkins.getQuietPeriod());
+    }
+
+    @Test
+    public void httpDoApply() throws Exception {
+        j.jenkins.setCrumbIssuer(null);
+
+        URL apiURL = new URL(MessageFormat.format(
+            "{0}configuration-as-code/apply",
+            j.getURL().toString()));
+        WebRequest request =
+            new WebRequest(apiURL, POST);
+        request.setCharset(StandardCharsets.UTF_8);
+        request.setRequestBody("jenkins:\n"
+            + "  systemMessage: \"configuration as code - JenkinsConfigTestHttpRequest\"\n"
+            + "  quietPeriod: 10");
+        int response = j.createWebClient().getPage(request).getWebResponse().getStatusCode();
+        assertThat(response, is(200));
+        Jenkins jenkins = Jenkins.get();
+        assertEquals("configuration as code - JenkinsConfigTestHttpRequest", jenkins.getSystemMessage());
+        assertEquals(10, jenkins.getQuietPeriod());
+    }
+
+    @Test
+    public void httpDoCheck() throws Exception {
+        j.jenkins.setCrumbIssuer(null);
+
+        URL apiURL = new URL(MessageFormat.format(
+            "{0}configuration-as-code/check",
+            j.getURL().toString()));
+        WebRequest request =
+            new WebRequest(apiURL, POST);
+        request.setCharset(StandardCharsets.UTF_8);
+        request.setRequestBody("jenkins:\n"
+            + "  systemMessage: \"configuration as code - JenkinsConfigTestHttpRequest\"\n"
+            + "  quietPeriod: 10");
+        int response = j.createWebClient().getPage(request).getWebResponse().getStatusCode();
+        assertThat(response, is(200));
+    }
+
+    @Test(expected = FailingHttpStatusCodeException.class)
+    public void httpDoCheckFailure() throws Exception {
+        j.jenkins.setCrumbIssuer(null);
+
+        URL apiURL = new URL(MessageFormat.format(
+            "{0}configuration-as-code/check",
+            j.getURL().toString()));
+        WebRequest request =
+            new WebRequest(apiURL, POST);
+        request.setCharset(StandardCharsets.UTF_8);
+        request.setRequestBody("jenkins:\n"
+            + "  systemMessage: {}");
+        j.createWebClient().getPage(request);
+    }
+}

test-harness/src/test/resources/io/jenkins/plugins/casc/folder/jenkins.yml

@@ -0,0 +1,2 @@
+jenkins:
+  systemMessage: "configuration as code - JenkinsConfigTestFolder"

test-harness/src/test/resources/io/jenkins/plugins/casc/folder/jenkins2.yml

@@ -0,0 +1,2 @@
+jenkins:
+  quietPeriod: 10