Merge branch 'dev' into dev-flex

Author: br648

src/main/java/com/conveyal/datatools/common/utils/CloseableHttpURLConnection.java

@@ -0,0 +1,28 @@
+package com.conveyal.datatools.common.utils;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.net.HttpURLConnection;
+import java.net.URL;
+
+/**
+ * A wrapper around HttpURLConnection that implements Closeable so it can be used in try-with-resources blocks.
+ */
+public class CloseableHttpURLConnection implements Closeable {
+    private final HttpURLConnection connection;
+
+    public CloseableHttpURLConnection(URL url) throws IOException {
+        this.connection = (HttpURLConnection) url.openConnection();
+    }
+
+    public HttpURLConnection getConnection() {
+        return connection;
+    }
+
+    @Override
+    public void close() {
+        if (connection != null) {
+            connection.disconnect();
+        }
+    }
+}

src/main/java/com/conveyal/datatools/common/utils/SparkUtils.java

@@ -258,13 +258,12 @@ public static void logRequestOrResponse(
      * multipart form handling code also caches the request body.
      */
     public static void copyRequestStreamIntoFile(Request req, File file) {
-        try {
+        try (
             ServletInputStream inputStream = ((ServletRequestWrapper) req.raw()).getRequest().getInputStream();
             FileOutputStream fileOutputStream = new FileOutputStream(file);
+        ) {
             // Guava's ByteStreams.copy uses a 4k buffer (no need to wrap output stream), but does not close streams.
             ByteStreams.copy(inputStream, fileOutputStream);
-            fileOutputStream.close();
-            inputStream.close();
             if (file.length() == 0) {
                 // Throw IO exception to be caught and returned to user via halt.
                 throw new IOException("No file found in request body.");
@@ -299,10 +298,13 @@ public static String uploadMultipartRequestBodyToS3(Request req, String uploadTy
 
             extension = "." + part.getContentType().split("/", 0)[1];
             tempFile = File.createTempFile(part.getName() + "_" + uploadType, extension);
-            InputStream inputStream;
-            inputStream = part.getInputStream();
-            FileOutputStream out = new FileOutputStream(tempFile);
-            IOUtils.copy(inputStream, out);
+
+            try (
+                InputStream inputStream = part.getInputStream();
+                FileOutputStream out = new FileOutputStream(tempFile)
+            ) {
+                IOUtils.copy(inputStream, out);
+            }
         } catch (IOException | ServletException e) {
             e.printStackTrace();
             logMessageAndHalt(req, 400, "Unable to read uploaded file");

src/main/java/com/conveyal/datatools/editor/jobs/ExportSnapshotToGTFSJob.java

@@ -82,8 +82,8 @@ public void jobLogic() {
             }
             LOG.info("Storing snapshot GTFS at {}", S3Utils.getDefaultBucketUriForKey(s3Key));
         } else {
-            try {
-                File gtfsFile = FeedVersion.feedStore.newFeed(filename, new FileInputStream(tempFile), null);
+            try (FileInputStream fileInputStream = new FileInputStream(tempFile)) {
+                File gtfsFile = FeedVersion.feedStore.newFeed(filename, fileInputStream, null);
                 if (isNewVersion) feedVersion.assignGtfsFileAttributes(gtfsFile);
             } catch (IOException e) {
                 status.fail(String.format("Could not store feed for snapshot %s", snapshot.id), e);

src/main/java/com/conveyal/datatools/editor/utils/ClassLoaderSerializer.java

@@ -19,15 +19,20 @@ public class ClassLoaderSerializer implements Serializer<Object>, Serializable {
 
     @Override
     public void serialize(DataOutput out, Object value) throws IOException {
-        ObjectOutputStream out2 = new ObjectOutputStream((OutputStream) out);
-        out2.writeObject(value);
-        out2.flush();
+        try (ObjectOutputStream out2 = new ObjectOutputStream((OutputStream) out)) {
+            out2.writeObject(value);
+            out2.flush();
+        }
     }
 
     @Override
     public Object deserialize(DataInput in, int available) throws IOException {
-        try {
-            ObjectInputStream in2 = new ClassLoaderObjectInputStream(Thread.currentThread().getContextClassLoader(), (InputStream) in);
+        try (
+            ObjectInputStream in2 = new ClassLoaderObjectInputStream(
+                Thread.currentThread().getContextClassLoader(),
+                (InputStream) in
+            )
+        ) {
             return in2.readObject();
         } catch (ClassNotFoundException e) {
             throw new IOException(e);

src/main/java/com/conveyal/datatools/editor/utils/DirectoryZip.java

@@ -1,6 +1,5 @@
 package com.conveyal.datatools.editor.utils;
 
-import java.io.Closeable;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
@@ -18,35 +17,34 @@ public class DirectoryZip {
 
     public static void zip(File directory, File zipfile) throws IOException {
         URI base = directory.toURI();
-        Deque<File> queue = new LinkedList<File>();
+        Deque<File> queue = new LinkedList<>();
         queue.push(directory);
-        OutputStream out = new FileOutputStream(zipfile);
-        Closeable res = out;
-        try {
-          ZipOutputStream zout = new ZipOutputStream(out);
-          res = zout;
-          while (!queue.isEmpty()) {
-            directory = queue.pop();
-            for (File kid : directory.listFiles()) {
-              String name = base.relativize(kid.toURI()).getPath();
-              if (kid.isDirectory()) {
-                queue.push(kid);
-                name = name.endsWith("/") ? name : name + "/";
-                zout.putNextEntry(new ZipEntry(name));
-              } else {
-                zout.putNextEntry(new ZipEntry(name));
-                copy(kid, zout);
-                zout.closeEntry();
-              }
+
+        try (
+            FileOutputStream out = new FileOutputStream(zipfile);
+            ZipOutputStream zout = new ZipOutputStream(out)
+        ) {
+            while (!queue.isEmpty()) {
+                directory = queue.pop();
+                for (File kid : directory.listFiles()) {
+                    String name = base.relativize(kid.toURI()).getPath();
+                    if (kid.isDirectory()) {
+                        queue.push(kid);
+                        name = name.endsWith("/") ? name : name + "/";
+                        zout.putNextEntry(new ZipEntry(name));
+                    } else {
+                        zout.putNextEntry(new ZipEntry(name));
+                        copy(kid, zout);
+                    }
+                    // Explicitly close directory entry to ensure ZIP structure is valid; omitting this may corrupt the
+                    // archive.
+                    zout.closeEntry();
+                }
             }
-          }
-        } finally {
-          res.close();
         }
-      }
+    }
 
-
-     private static void copy(InputStream in, OutputStream out) throws IOException {
+    private static void copy(InputStream in, OutputStream out) throws IOException {
         byte[] buffer = new byte[1024];
         while (true) {
           int readCount = in.read(buffer);
@@ -57,22 +55,9 @@ private static void copy(InputStream in, OutputStream out) throws IOException {
         }
       }
 
-     private static void copy(File file, OutputStream out) throws IOException {
-        InputStream in = new FileInputStream(file);
-        try {
-          copy(in, out);
-        } finally {
-          in.close();
+    private static void copy(File file, OutputStream out) throws IOException {
+        try (InputStream in = new FileInputStream(file)) {
+            copy(in, out);
         }
-      }
-
-      private static void copy(InputStream in, File file) throws IOException {
-        OutputStream out = new FileOutputStream(file);
-        try {
-          copy(in, out);
-        } finally {
-          out.close();
-        }
-      }
-
+    }
 }

src/main/java/com/conveyal/datatools/manager/DataManager.java

@@ -159,9 +159,10 @@ static void initializeApplication(String[] args, boolean initScheduledTasks) thr
      */
     private static void loadProperties() {
         final Properties projectProperties = new Properties();
-        InputStream projectPropertiesInputStream =
-            DataManager.class.getClassLoader().getResourceAsStream(".properties");
-        try {
+        try (
+            InputStream projectPropertiesInputStream =
+                DataManager.class.getClassLoader().getResourceAsStream(".properties");
+        ) {
             projectProperties.load(projectPropertiesInputStream);
             repoUrl = projectProperties.getProperty("repo_url");
         } catch (IOException e) {
@@ -170,9 +171,10 @@ private static void loadProperties() {
         }
 
         final Properties gitProperties = new Properties();
-        try {
+        try (
             InputStream gitPropertiesInputStream =
                 DataManager.class.getClassLoader().getResourceAsStream("git.properties");
+        ) {
             gitProperties.load(gitPropertiesInputStream);
             commit = gitProperties.getProperty("git.commit.id");
         } catch (Exception e) {
@@ -484,24 +486,28 @@ private static void registerExternalResources() {
      * default configuration file locations. Config fields are retrieved with getConfigProperty.
      */
     private static void loadConfig(String[] args) throws IOException {
-        FileInputStream envConfigStream;
-        FileInputStream serverConfigStream;
+        File envFile;
+        File serverFile;
 
         if (args.length == 0) {
             LOG.warn("Using default env.yml: {}", DEFAULT_ENV);
             LOG.warn("Using default server.yml: {}", DEFAULT_CONFIG);
-            envConfigStream = new FileInputStream(new File(DEFAULT_ENV));
-            serverConfigStream = new FileInputStream(new File(DEFAULT_CONFIG));
-        }
-        else {
+            envFile = new File(DEFAULT_ENV);
+            serverFile = new File(DEFAULT_CONFIG);
+        } else {
             LOG.info("Loading env.yml: {}", args[0]);
             LOG.info("Loading server.yml: {}", args[1]);
-            envConfigStream = new FileInputStream(new File(args[0]));
-            serverConfigStream = new FileInputStream(new File(args[1]));
+            envFile = new File(args[0]);
+            serverFile = new File(args[1]);
         }
 
-        envConfig = yamlMapper.readTree(envConfigStream);
-        serverConfig = yamlMapper.readTree(serverConfigStream);
+        try (
+            FileInputStream envConfigStream = new FileInputStream(envFile);
+            FileInputStream serverConfigStream = new FileInputStream(serverFile)
+        ) {
+            envConfig = yamlMapper.readTree(envConfigStream);
+            serverConfig = yamlMapper.readTree(serverConfigStream);
+        }
     }
 
     /**

src/main/java/com/conveyal/datatools/manager/controllers/api/DeploymentController.java

@@ -159,6 +159,7 @@ private static FileInputStream downloadDeployment (Request req, Response res) th
         File temp = File.createTempFile("deployment", ".zip");
         // just include GTFS, not any of the ancillary information
         deployment.dump(temp, false, false, false);
+        // File input stream must be closed by calling method.
         FileInputStream fis = new FileInputStream(temp);
         String cleanName = deployment.name.replaceAll("[^a-zA-Z0-9]", "");
         res.type("application/zip");

src/main/java/com/conveyal/datatools/manager/controllers/api/GtfsPlusController.java

@@ -100,10 +100,9 @@ private static HttpServletResponse getGtfsPlusFromGtfs(String feedVersionId, Req
             gtfsPlusTables.add(tableNode.get("name").asText());
         }
 
-        try {
+        try (ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(gtfsPlusFile))) {
             // create a new zip file to only contain the GTFS+ tables
             gtfsPlusFile = File.createTempFile(version.id + "_gtfsplus", ".zip");
-            ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(gtfsPlusFile));
 
             // iterate through the existing GTFS file, copying any GTFS+ tables
             ZipFile gtfsFile = new ZipFile(version.retrieveGtfsFile());
@@ -116,15 +115,14 @@ private static HttpServletResponse getGtfsPlusFromGtfs(String feedVersionId, Req
                 // create a new empty ZipEntry and copy the contents
                 ZipEntry newEntry = new ZipEntry(entry.getName());
                 zos.putNextEntry(newEntry);
-                InputStream in = gtfsFile.getInputStream(entry);
-                while (0 < in.available()){
-                    int read = in.read(buffer);
-                    zos.write(buffer,0,read);
+                try (InputStream in = gtfsFile.getInputStream(entry)) {
+                    while (0 < in.available()) {
+                        int read = in.read(buffer);
+                        zos.write(buffer, 0, read);
+                    }
                 }
-                in.close();
                 zos.closeEntry();
             }
-            zos.close();
         } catch (IOException e) {
             logMessageAndHalt(req, 500, "An error occurred while trying to create a gtfs file", e);
         }
@@ -177,10 +175,9 @@ private static String publishGtfsPlusFile(Request req, Response res) {
 
         File newFeed = null;
 
-        try {
+        try (ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(newFeed))) {
             // First, create a new zip file to only contain the GTFS+ tables
             newFeed = File.createTempFile(feedVersionId + "_new", ".zip");
-            ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(newFeed));
 
             // Next, iterate through the existing GTFS file, copying all non-GTFS+ tables.
             ZipFile gtfsFile = new ZipFile(feedVersion.retrieveGtfsFile());
@@ -194,12 +191,12 @@ private static String publishGtfsPlusFile(Request req, Response res) {
                 // create a new empty ZipEntry and copy the contents
                 ZipEntry newEntry = new ZipEntry(entry.getName());
                 zos.putNextEntry(newEntry);
-                InputStream in = gtfsFile.getInputStream(entry);
-                while (0 < in.available()){
-                    int read = in.read(buffer);
-                    zos.write(buffer,0,read);
+                try (InputStream in = gtfsFile.getInputStream(entry)) {
+                    while (0 < in.available()) {
+                        int read = in.read(buffer);
+                        zos.write(buffer, 0, read);
+                    }
                 }
-                in.close();
                 zos.closeEntry();
             }
 
@@ -211,23 +208,22 @@ private static String publishGtfsPlusFile(Request req, Response res) {
 
                 ZipEntry newEntry = new ZipEntry(entry.getName());
                 zos.putNextEntry(newEntry);
-                InputStream in = plusZipFile.getInputStream(entry);
-                while (0 < in.available()){
-                    int read = in.read(buffer);
-                    zos.write(buffer,0,read);
+                try (InputStream in = plusZipFile.getInputStream(entry)) {
+                    while (0 < in.available()) {
+                        int read = in.read(buffer);
+                        zos.write(buffer, 0, read);
+                    }
                 }
-                in.close();
                 zos.closeEntry();
             }
-            zos.close();
         } catch (IOException e) {
             logMessageAndHalt(req, 500, "Error creating combined GTFS/GTFS+ file", e);
         }
         // Create a new feed version to represent the published GTFS+.
         FeedVersion newFeedVersion = new FeedVersion(feedVersion.parentFeedSource(), PRODUCED_IN_HOUSE_GTFS_PLUS);
         File newGtfsFile = null;
-        try {
-            newGtfsFile = newFeedVersion.newGtfsFile(new FileInputStream(newFeed));
+        try (FileInputStream fis = new FileInputStream(newFeed)) {
+            newGtfsFile = newFeedVersion.newGtfsFile(fis);
         } catch (IOException e) {
             e.printStackTrace();
             logMessageAndHalt(req, 500, "Error reading GTFS file input stream", e);