From 739cc3025da9724f3850ed88602f89ce625d44e9 Mon Sep 17 00:00:00 2001 From: Joseph Petersen Date: Sun, 24 May 2020 23:16:48 +0200 Subject: [PATCH] =?UTF-8?q?Close=20those=20file=20handles=20=F0=9F=98=85?= =?UTF-8?q?=20(#1398)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../plugins/casc/ConfigurationAsCode.java | 13 --- .../casc/yaml/StreamReaderWithSource.java | 5 +- .../jenkins/plugins/casc/yaml/YamlReader.java | 14 --- .../jenkins/plugins/casc/yaml/YamlSource.java | 60 ++---------- .../jenkins/plugins/casc/yaml/YamlUtils.java | 37 ++++++-- .../plugins/casc/yaml/YamlSourceTest.java | 31 ++++++- .../jenkins/plugins/casc/YamlReaderTest.java | 92 +++++++++++++++++++ .../jenkins/plugins/casc/folder/jenkins.yml | 2 + .../jenkins/plugins/casc/folder/jenkins2.yml | 2 + 9 files changed, 166 insertions(+), 90 deletions(-) delete mode 100644 plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlReader.java create mode 100644 test-harness/src/test/java/io/jenkins/plugins/casc/YamlReaderTest.java create mode 100644 test-harness/src/test/resources/io/jenkins/plugins/casc/folder/jenkins.yml create mode 100644 test-harness/src/test/resources/io/jenkins/plugins/casc/folder/jenkins2.yml diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java b/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java index 02b5fcf7..8d3f22c9 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java +++ b/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 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 * diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/yaml/StreamReaderWithSource.java b/plugin/src/main/java/io/jenkins/plugins/casc/yaml/StreamReaderWithSource.java index 3ad48525..0bcaecf6 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/yaml/StreamReaderWithSource.java +++ b/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 { - super(source.read()); + public StreamReaderWithSource(YamlSource source, Reader reader) throws IOException { + super(reader); try { final Field f = StreamReader.class.getDeclaredField("name"); f.setAccessible(true); diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlReader.java b/plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlReader.java deleted file mode 100644 index 410106ee..00000000 --- a/plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlReader.java +++ /dev/null @@ -1,14 +0,0 @@ -package io.jenkins.plugins.casc.yaml; - -import java.io.IOException; -import java.io.Reader; - -/** - * @author Nicolas De Loof - */ -@FunctionalInterface - -public interface YamlReader { - - Reader open(T source) throws IOException; -} diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlSource.java b/plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlSource.java index e84c43b3..fdbeb3f5 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlSource.java +++ b/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 Nicolas De Loof */ -public class YamlSource implements AutoCloseable { - - public static final YamlReader READ_FROM_URL = config -> { - final URL url = URI.create(config).toURL(); - return new InputStreamReader(url.openStream(), UTF_8); - }; - - public static final YamlReader READ_FROM_PATH = Files::newBufferedReader; - - public static final YamlReader READ_FROM_INPUTSTREAM = in -> new InputStreamReader(in, UTF_8); - - public static final YamlReader READ_FROM_REQUEST = req -> { - // TODO get encoding from req.getContentType() - return new InputStreamReader(req.getInputStream(), UTF_8); - }; +public class YamlSource { public final T source; - public final YamlReader reader; - - public YamlSource(T source, YamlReader reader) { + public YamlSource(T source) { this.source = source; - this.reader = reader; } public static YamlSource of(InputStream in) { - return new YamlSource<>(in, READ_FROM_INPUTSTREAM); - } - - public static YamlSource of(URL url) { - return new YamlSource<>(url.toExternalForm(), READ_FROM_URL); + return new YamlSource<>(in); } public static YamlSource of(String url) { - return new YamlSource<>(url, READ_FROM_URL); + return new YamlSource<>(url); } public static YamlSource of(HttpServletRequest req) { - return new YamlSource<>(req, YamlSource.READ_FROM_REQUEST); + return new YamlSource<>(req); } public static YamlSource of(Path path) { - return new YamlSource<>(path, YamlSource.READ_FROM_PATH); - } - - public Reader read() throws IOException { - return reader.open(source); + return new YamlSource<>(path); } public String source() { + if (source instanceof HttpServletRequest) { + return ((HttpServletRequest) source).getPathInfo(); + } return source.toString(); } @Override public String toString() { - return "YamlSource: " + source; - } - - @Override - public void close() throws IOException { - if (reader instanceof BufferedReader) { - ((BufferedReader) reader).close(); - } else if (reader instanceof InputStreamReader) { - ((InputStreamReader) reader).close(); - } + return "YamlSource: " + source(); } } diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlUtils.java b/plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlUtils.java index ea720c17..3b4f3f69 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlUtils.java +++ b/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 Nicolas De Loof */ @@ -29,13 +38,12 @@ public final class YamlUtils { public static final Logger LOGGER = Logger.getLogger(ConfigurationAsCode.class.getName()); - public static Node merge(List configs, + public static Node merge(List sources, ConfigurationContext context) throws ConfiguratorException { Node root = null; - for (YamlSource source : configs) { - try (Reader r = source.read()) { - - final Node node = read(source, context); + for (YamlSource source : sources) { + try (Reader reader = reader(source)) { + final Node node = read(source, reader, 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 diff --git a/plugin/src/test/java/io/jenkins/plugins/casc/yaml/YamlSourceTest.java b/plugin/src/test/java/io/jenkins/plugins/casc/yaml/YamlSourceTest.java index f918ea94..3c97366e 100644 --- a/plugin/src/test/java/io/jenkins/plugins/casc/yaml/YamlSourceTest.java +++ b/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 yamlSource = YamlSource.of(new URL(testUrl)); + YamlSource 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 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 yamlSource = YamlSource.of(request); + assertEquals("YamlSource: /configuration-as-code/check", yamlSource.toString()); + } } diff --git a/test-harness/src/test/java/io/jenkins/plugins/casc/YamlReaderTest.java b/test-harness/src/test/java/io/jenkins/plugins/casc/YamlReaderTest.java new file mode 100644 index 00000000..033b3750 --- /dev/null +++ b/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); + } +} diff --git a/test-harness/src/test/resources/io/jenkins/plugins/casc/folder/jenkins.yml b/test-harness/src/test/resources/io/jenkins/plugins/casc/folder/jenkins.yml new file mode 100644 index 00000000..c5799033 --- /dev/null +++ b/test-harness/src/test/resources/io/jenkins/plugins/casc/folder/jenkins.yml @@ -0,0 +1,2 @@ +jenkins: + systemMessage: "configuration as code - JenkinsConfigTestFolder" diff --git a/test-harness/src/test/resources/io/jenkins/plugins/casc/folder/jenkins2.yml b/test-harness/src/test/resources/io/jenkins/plugins/casc/folder/jenkins2.yml new file mode 100644 index 00000000..57f0b123 --- /dev/null +++ b/test-harness/src/test/resources/io/jenkins/plugins/casc/folder/jenkins2.yml @@ -0,0 +1,2 @@ +jenkins: + quietPeriod: 10