Refactor TextFile.lines()

core/src/main/java/hudson/util/TextFile.java

@@ -23,25 +23,23 @@
  */
 package hudson.util;
 
-import com.google.common.collect.*;
+import edu.umd.cs.findbugs.annotations.CreatesObligation;
 
 import hudson.Util;
+import jenkins.util.io.LinesStream;
 
 import java.nio.file.Files;
-import java.nio.file.InvalidPathException;
 import javax.annotation.Nonnull;
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
-import java.io.InputStreamReader;
 import java.io.PrintWriter;
 import java.io.RandomAccessFile;
 import java.io.Reader;
 import java.io.StringWriter;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
-import java.util.Iterator;
 
 /**
  * Represents a text file.
@@ -51,9 +49,10 @@
  * @author Kohsuke Kawaguchi
  */
 public class TextFile {
-    public final File file;
 
-    public TextFile(File file) {
+    public final @Nonnull File file;
+
+    public TextFile(@Nonnull File file) {
         this.file = file;
     }
 
@@ -80,36 +79,16 @@ public String read() throws IOException {
     }
 
     /**
-     * Parse text file line by line.
+     * Creates a new {@link jenkins.util.io.LinesStream} of the file.
+     * <p>
+     * Note: The caller is responsible for closing the returned
+     * <code>LinesStream</code>.
+     * @throws IOException if the file cannot be converted to a
+     * {@link java.nio.file.Path} or if the file cannot be opened for reading
      */
-    public Iterable<String> lines() {
-        return new Iterable<String>() {
-            @Override
-            public Iterator<String> iterator() {
-                try {
-                    final BufferedReader in = new BufferedReader(new InputStreamReader(
-                            Files.newInputStream(file.toPath()),"UTF-8"));
-
-                    return new AbstractIterator<String>() {
-                        @Override
-                        protected String computeNext() {
-                            try {
-                                String r = in.readLine();
-                                if (r==null) {
-                                    in.close();
-                                    return endOfData();
-                                }
-                                return r;
-                            } catch (IOException e) {
-                                throw new RuntimeException(e);
-                            }
-                        }
-                    };
-                } catch (IOException | InvalidPathException e) {
-                    throw new RuntimeException(e);
-                }
-            }
-        };
+    @CreatesObligation
+    public @Nonnull LinesStream lines() throws IOException {
+        return new LinesStream(Util.fileToPath(file));
     }
 
     /**

core/src/main/java/jenkins/security/s2m/ConfigFile.java

@@ -3,6 +3,7 @@
 import hudson.CopyOnWrite;
 import hudson.util.TextFile;
 import jenkins.model.Jenkins;
+import jenkins.util.io.LinesStream;
 
 import java.io.BufferedReader;
 import java.io.File;
@@ -26,15 +27,42 @@ public ConfigFile(File file) {
     protected abstract COL create();
     protected abstract COL readOnly(COL base);
 
-    public synchronized void load() {
+    /**
+     * Loads the configuration from the configuration file.
+     * <p>
+     * This method is equivalent to {@link #load2()}, except that any
+     * {@link java.io.IOException} that occurs is wrapped as a
+     * {@link java.lang.RuntimeException}.
+     * <p>
+     * This method exists for source compatibility. Users should call
+     * {@link #load2()} instead.
+     * @deprecated use {@link #load2()} instead.
+     */
+    @Deprecated
+    public void load() {
+        try {
+            load2();
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    /**
+     * Loads the configuration from the configuration file.
+     * @throws IOException if the configuration file could not be read.
+     * @since TODO
+     */
+    public synchronized void load2() throws IOException {
         COL result = create();
 
         if (exists()) {
-            for (String line : lines()) {
-                if (line.startsWith("#")) continue;   // comment
-                T r = parse(line);
-                if (r != null)
-                    result.add(r);
+            try (LinesStream stream = lines()) {
+                for (String line : stream) {
+                    if (line.startsWith("#")) continue;   // comment
+                    T r = parse(line);
+                    if (r != null)
+                        result.add(r);
+                }
             }
         }
 
@@ -63,7 +91,7 @@ public synchronized void set(String newContent) throws IOException {
         Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER);
 
         write(newContent);
-        load();
+        load2();
     }
 
     public synchronized void append(String additional) throws IOException {
@@ -79,8 +107,9 @@ public COL get() {
         // load upon the first use
         if (parsed==null) {
             synchronized (this) {
-                if (parsed==null)
+                if (parsed==null) {
                     load();
+                }
             }
         }
         return parsed;

core/src/main/java/jenkins/util/io/LinesStream.java

@@ -0,0 +1,114 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2018 Daniel Trebbien.
+ *
+ * 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 jenkins.util.io;
+
+import com.google.common.collect.AbstractIterator;
+
+import edu.umd.cs.findbugs.annotations.CleanupObligation;
+import edu.umd.cs.findbugs.annotations.DischargesObligation;
+
+import java.io.BufferedReader;
+import java.io.Closeable;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Iterator;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+/**
+ * Represents a stream over the lines of a text file.
+ * <p>
+ * Although <code>LinesStream</code> implements {@link java.lang.Iterable}, it
+ * is intended to be first used to initialize a resource in a try-with-resources
+ * statement and then iterated, as in:
+ * <pre>
+ *  try (LinesStream stream = new LinesStream(...)) {
+ *      for (String line : stream) {
+ *          ...
+ *      }
+ *  }
+ * </pre>
+ * This pattern ensures that the underlying file handle is closed properly.
+ * <p>
+ * Like {@link java.nio.file.DirectoryStream}, <code>LinesStream</code> supports
+ * creating at most one <code>Iterator</code>. Invoking {@link #iterator()} to
+ * obtain a second or subsequent <code>Iterator</code> throws
+ * <code>IllegalStateException</code>.
+ *
+ * @since TODO
+ */
+@CleanupObligation
+public class LinesStream implements Closeable, Iterable<String> {
+
+    private final @Nonnull BufferedReader in;
+    private transient @Nullable Iterator<String> iterator;
+
+    /**
+     * Opens the text file at <code>path</code> for reading using charset
+     * {@link java.nio.charset.StandardCharsets#UTF_8}.
+     * @param path Path to the file to open for reading.
+     * @throws IOException if the file at <code>path</code> cannot be opened for
+     * reading.
+     */
+    public LinesStream(@Nonnull Path path) throws IOException {
+        in = Files.newBufferedReader(path); // uses UTF-8 by default
+    }
+
+    @DischargesObligation
+    @Override
+    public void close() throws IOException {
+        in.close();
+    }
+
+    @Override
+    public Iterator<String> iterator() {
+        if (iterator!=null)
+            throw new IllegalStateException("Only one Iterator can be created.");
+
+        iterator = new AbstractIterator<String>() {
+            @Override
+            protected String computeNext() {
+                try {
+                    String r = in.readLine();
+                    if (r==null) {
+                        // Calling close() here helps ensure that the file
+                        // handle is closed even when LinesStream is being used
+                        // incorrectly, where it is iterated over without being
+                        // used to initialize a resource of a try-with-resources
+                        // statement.
+                        in.close();
+                        return endOfData();
+                    }
+                    return r;
+                } catch (IOException e) {
+                    throw new RuntimeException(e);
+                }
+            }
+        };
+
+        return iterator;
+    }
+}