Merge pull request #2156 from Georgetown-University-Libraries/ds3700m

DS-3700: MediaFilterServiceImpl forgot to close an input stream (for master)

dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterServiceImpl.java

@@ -310,12 +310,11 @@ public class MediaFilterServiceImpl implements MediaFilterService, InitializingB
         // get bitstream filename, calculate destination filename
         String newName = formatFilter.getFilteredName(source.getName());
 
-        Bitstream existingBitstream = null; // is there an existing rendition?
+        // check if destination bitstream exists
-        Bundle targetBundle = null; // bundle we're modifying
+        Bundle existingBundle = null;
-
+        Bitstream existingBitstream = null;
         List<Bundle> bundles = itemService.getBundles(item, formatFilter.getBundleName());
 
-        // check if destination bitstream exists
         if (bundles.size() > 0) {
             // only finds the last match (FIXME?)
             for (Bundle bundle : bundles) {
@@ -323,7 +322,7 @@ public class MediaFilterServiceImpl implements MediaFilterService, InitializingB
 
                 for (Bitstream bitstream : bitstreams) {
                     if (bitstream.getName().trim().equals(newName.trim())) {
-                        targetBundle = bundle;
+                        existingBundle = bundle;
                         existingBitstream = bitstream;
                     }
                 }
@@ -345,40 +344,41 @@ public class MediaFilterServiceImpl implements MediaFilterService, InitializingB
                                    + " (item: " + item.getHandle() + ")");
         }
 
-        InputStream destStream;
-        try {
         System.out.println("File: " + newName);
-            destStream = formatFilter.getDestinationStream(item, bitstreamService.retrieve(context, source), isVerbose);
+
+        // start filtering of the bitstream, using try with resource to close all InputStreams properly
+        try (
+                // get the source stream
+                InputStream srcStream = bitstreamService.retrieve(context, source);
+                // filter the source stream to produce the destination stream
+                // this is the hard work, check for OutOfMemoryErrors at the end of the try clause.
+                InputStream destStream = formatFilter.getDestinationStream(item, srcStream, isVerbose);
+        ) {
             if (destStream == null) {
                 if (!isQuiet) {
                     System.out.println("SKIPPED: bitstream " + source.getID()
                             + " (item: " + item.getHandle() + ") because filtering was unsuccessful");
                 }
-
-                return false;
-            }
-        } catch (OutOfMemoryError oome) {
-            System.out.println("!!! OutOfMemoryError !!!");
                 return false;
             }
 
-        // create new bundle if needed
+            Bundle targetBundle; // bundle we're modifying
             if (bundles.size() < 1) {
+                // create new bundle if needed
                 targetBundle = bundleService.create(context, item, formatFilter.getBundleName());
             } else {
-            // take the first match
+                // take the first match as we already looked out for the correct bundle name
                 targetBundle = bundles.get(0);
             }
 
+            // create bitstream to store the filter result
             Bitstream b = bitstreamService.create(context, targetBundle, destStream);
-
+            // set the name, source and description of the bitstream
-        // Now set the format and name of the bitstream
             b.setName(context, newName);
             b.setSource(context, "Written by FormatFilter " + formatFilter.getClass().getName() +
                     " on " + DCDate.getCurrent() + " (GMT).");
             b.setDescription(context, formatFilter.getDescription());
-
+            // Set the format of the bitstream
-        // Find the proper format
             BitstreamFormat bf = bitstreamFormatService.findByShortDescription(context,
                     formatFilter.getFormatString());
             bitstreamService.setFormat(context, b, bf);
@@ -398,10 +398,17 @@ public class MediaFilterServiceImpl implements MediaFilterService, InitializingB
                 authorizeService.inheritPolicies(context, source, b);
             }
 
+            //do post-processing of the generated bitstream
+            formatFilter.postProcessBitstream(context, item, b);
+
+        } catch (OutOfMemoryError oome) {
+            System.out.println("!!! OutOfMemoryError !!!");
+        }
+
         // fixme - set date?
         // we are overwriting, so remove old bitstream
         if (existingBitstream != null) {
-            bundleService.removeBitstream(context, targetBundle, existingBitstream);
+            bundleService.removeBitstream(context, existingBundle, existingBitstream);
         }
 
         if (!isQuiet) {
@@ -409,9 +416,6 @@ public class MediaFilterServiceImpl implements MediaFilterService, InitializingB
                                    + " (item: " + item.getHandle() + ") and created '" + newName + "'");
         }
 
-        //do post-processing of the generated bitstream
-        formatFilter.postProcessBitstream(context, item, b);
-
         return true;
     }
 

dspace-api/src/main/java/org/dspace/storage/bitstore/DSBitStoreService.java

@@ -117,25 +117,21 @@ public class DSBitStoreService implements BitStoreService {
             //Create the corresponding file and open it
             file.createNewFile();
 
+            try (
                     FileOutputStream fos = new FileOutputStream(file);
-
                     // Read through a digest input stream that will work out the MD5
-            DigestInputStream dis = null;
+                    DigestInputStream dis = new DigestInputStream(in, MessageDigest.getInstance(CSA));
-
+            ) {
-            try {
-                dis = new DigestInputStream(in, MessageDigest.getInstance(CSA));
-            } catch (NoSuchAlgorithmException nsae) {
-                // Should never happen
-                log.warn("Caught NoSuchAlgorithmException", nsae);
-            }
-
                 Utils.bufferedCopy(dis, fos);
-            fos.close();
                 in.close();
 
                 bitstream.setSizeBytes(file.length());
                 bitstream.setChecksum(Utils.toHex(dis.getMessageDigest().digest()));
                 bitstream.setChecksumAlgorithm(CSA);
+            } catch (NoSuchAlgorithmException nsae) {
+                // Should never happen
+                log.warn("Caught NoSuchAlgorithmException", nsae);
+            }
         } catch (Exception e) {
             log.error("put(" + bitstream.getInternalId() + ", inputstream)", e);
             throw new IOException(e);