OAK-11500: Remove usage of Guava io.Closeable (#2136)

* OAK-11500: Remove usage of Guava io.Closeable  (#2129)

* OAK-11500: Remove usage of Guava io.Closeable

* OAK-11500: Remove usage of Guava io.Closeable

* OAK-11500: fix change in MongoDocumentStore

* Revert "OAK-11500: fix change in MongoDocumentStore"

This reverts commit 91ce780.

* OAK-11500: fix change in MongoDocumentStore

* OAK-11500: fix change in AbstractSharedCachingDataStore

* OAK-11500: fix potentially problematic change in BlobIdTrackerClusterSharedTest

* OAK-11500: fix potentially problematic change in ActiveDeletedBlobCollectorFactory

* OAK-11500: simplify instances by using IOUtils.closeQuietly more

* OAK-11500: more use of try-with-resources, add comment about OAK-7662, avoid potential future NPEs

* OAK-11500: add comment about OAK-7662, avoid potential future NPEs

oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/AbstractSharedCachingDataStore.java

@@ -58,7 +58,6 @@
 
 import org.apache.jackrabbit.guava.common.base.Stopwatch;
 import org.apache.jackrabbit.guava.common.collect.Iterators;
-import org.apache.jackrabbit.guava.common.io.Closeables;
 import org.apache.jackrabbit.guava.common.util.concurrent.ListeningExecutorService;
 
 /**
@@ -327,15 +326,11 @@ public InputStream getStream() throws DataStoreException {
             try {
                 // If cache configured to 0 will return null
                 if (cached == null || !cached.exists()) {
-                    InputStream in = null;
-                    try {
-                        TransientFileFactory fileFactory = TransientFileFactory.getInstance();
-                        File tmpFile = fileFactory.createTransientFile("temp0cache", null, temp);
-                        in = backend.getRecord(getIdentifier()).getStream();
+                    TransientFileFactory fileFactory = TransientFileFactory.getInstance();
+                    File tmpFile = fileFactory.createTransientFile("temp0cache", null, temp);
+                    try (InputStream in = backend.getRecord(getIdentifier()).getStream()) {
                         copyInputStreamToFile(in, tmpFile);
                         return new LazyFileInputStream(tmpFile);
-                    } finally {
-                        Closeables.close(in, false);
                     }
                 } else {
                     return new FileInputStream(cached);

oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/FileCache.java

@@ -49,7 +49,6 @@
 
 import org.apache.jackrabbit.guava.common.base.Stopwatch;
 import org.apache.jackrabbit.guava.common.cache.AbstractCache;
-import org.apache.jackrabbit.guava.common.io.Closeables;
 
 /**
  */
@@ -110,18 +109,12 @@ public File load(String key) throws Exception {
                 if (cachedFile.exists()) {
                     return cachedFile;
                 } else {
-                    InputStream is = null;
-                    boolean threw = true;
                     long startNanos = System.nanoTime();
-                    try {
-                        is = loader.load(key);
+                    try (InputStream is = loader.load(key))  {
                         copyInputStreamToFile(is, cachedFile);
-                        threw = false;
                     } catch (Exception e) {
                         LOG.warn("Error reading object for id [{}] from backend", key, e);
                         throw e;
-                    } finally {
-                        Closeables.close(is, threw);
                     }
                     if (LOG.isDebugEnabled()) {
                         LOG.debug("Loaded file: {} in {}", key, (System.nanoTime() - startNanos) / 1_000_000);

oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/MarkSweepGarbageCollector.java

@@ -56,9 +56,9 @@
 
 import org.apache.commons.collections4.ListValuedMap;
 import org.apache.commons.collections4.multimap.ArrayListValuedHashMap;
+import org.apache.commons.io.IOUtils;
 import org.apache.jackrabbit.guava.common.base.Stopwatch;
 import org.apache.jackrabbit.guava.common.collect.Iterators;
-import org.apache.jackrabbit.guava.common.io.Closeables;
 import org.apache.jackrabbit.guava.common.util.concurrent.ListenableFutureTask;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.LineIterator;
@@ -303,14 +303,10 @@ public List<GarbageCollectionRepoStats> getStats() throws Exception {
                             stat.setStartTime(markers.get(uniqueSessionId).getLastModified());
                         }
 
-                        LineNumberReader reader = null;
-                        try {
-                            reader = new LineNumberReader(new InputStreamReader(refRec.getStream()));
+                        try (LineNumberReader reader = new LineNumberReader(new InputStreamReader(refRec.getStream()))) {
                             while (reader.readLine() != null) {
                             }
                             stat.setNumLines(reader.getLineNumber());
-                        } finally {
-                            Closeables.close(reader, true);
                         }
                     }
                 }
@@ -379,8 +375,16 @@ protected void markAndSweep(boolean markOnly, boolean forceBlobRetrieve) throws
             throw e;
         } finally {
             statsCollector.updateDuration(sw.elapsed(TimeUnit.MILLISECONDS), TimeUnit.MILLISECONDS);
+
+            // OAK-7662: retain output file when tracing
             if (!LOG.isTraceEnabled() && !traceOutput) {
-                Closeables.close(fs, threw);
+                try {
+                    IOUtils.close(fs);
+                } catch (IOException ioe) {
+                    if (!threw) {
+                        throw ioe;
+                    }
+                }
             }
         }
     }
@@ -727,7 +731,7 @@ public long checkConsistency(boolean markOnly) throws Exception {
 
             // Get size
             getBlobReferencesSize(fs, consistencyStats);
-            
+
             if (!markOnly) {
                 // Find all blobs available in the blob store
                 ListenableFutureTask<Integer> blobIdRetriever = ListenableFutureTask.create(new BlobIdRetriever(fs,
@@ -768,8 +772,15 @@ public long checkConsistency(boolean markOnly) throws Exception {
                 }
             }
         } finally {
+            // OAK-7662: retain output file when tracing
             if (!traceOutput && (!LOG.isTraceEnabled() && candidates == 0)) {
-                Closeables.close(fs, threw);
+                try {
+                    IOUtils.close(fs);
+                } catch (IOException ioe) {
+                    if (!threw) {
+                        throw ioe;
+                    }
+                }
             }
             sw.stop();
             consistencyStatsCollector.updateDuration(sw.elapsed(TimeUnit.MILLISECONDS), TimeUnit.MILLISECONDS);
@@ -1091,7 +1102,7 @@ void retrieve(GarbageCollectableBlobStore blobStore,
             } finally {
                 if (idsIter instanceof Closeable) {
                     try {
-                        Closeables.close((Closeable) idsIter, false);
+                        ((Closeable)idsIter).close();
                     } catch (Exception e) {
                         LOG.debug("Error closing iterator");
                     }

oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStore.java

@@ -79,7 +79,6 @@
 import org.slf4j.LoggerFactory;
 
 import org.apache.jackrabbit.guava.common.collect.Iterators;
-import org.apache.jackrabbit.guava.common.io.Closeables;
 
 /**
  * BlobStore wrapper for DataStore. Wraps Jackrabbit 2 DataStore and expose them as BlobStores
@@ -323,15 +322,13 @@ public String writeBlob(InputStream stream) throws IOException {
 
     @Override
     public String writeBlob(InputStream stream, BlobOptions options) throws IOException {
-        boolean threw = true;
-        try {
+        requireNonNull(stream);
+        try (stream) {
             long start = System.nanoTime();
 
-            requireNonNull(stream);
             DataRecord dr = writeStream(stream, options);
             String id = getBlobId(dr);
             updateTracker(id);
-            threw = false;
 
             stats.uploaded(System.nanoTime() - start, TimeUnit.NANOSECONDS, dr.getLength());
             stats.uploadCompleted(id);
@@ -340,10 +337,6 @@ public String writeBlob(InputStream stream, BlobOptions options) throws IOExcept
         } catch (DataStoreException e) {
             stats.uploadFailed();
             throw new IOException(e);
-        } finally {
-            //DataStore does not closes the stream internally
-            //So close the stream explicitly
-            Closeables.close(stream, threw);
         }
     }
 
@@ -364,15 +357,10 @@ public int readBlob(String encodedBlobId, long pos, byte[] buff, int off, int le
         //This is inefficient as repeated calls for same blobId would involve opening new Stream
         //instead clients should directly access the stream from DataRecord by special casing for
         //BlobStore which implements DataStore
-        InputStream stream = getInputStream(encodedBlobId);
-        boolean threw = true;
-        try {
+        try (InputStream stream = getInputStream(encodedBlobId)) {
             IOUtils.skipFully(stream, pos);
             int readCount = stream.read(buff, off, length);
-            threw = false;
             return readCount;
-        } finally {
-            Closeables.close(stream, threw);
         }
     }
 
@@ -439,13 +427,9 @@ public InputStream getInputStream(final String encodedBlobId) throws IOException
                     @Override
                     public byte[] call() throws Exception {
                         boolean threw = true;
-                        InputStream stream = getStream(blobId.blobId);
-                        try {
+                        try (InputStream stream = getStream(blobId.blobId)) {
                             byte[] result = IOUtils.toByteArray(stream);
-                            threw = false;
                             return result;
-                        } finally {
-                            Closeables.close(stream, threw);
                         }
                     }
                 });

oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/FSBackend.java

@@ -30,7 +30,6 @@
 import java.util.Properties;
 
 import org.apache.commons.lang3.StringUtils;
-import org.apache.jackrabbit.guava.common.io.Closeables;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.io.filefilter.FileFilterUtils;
@@ -189,12 +188,8 @@ public void addMetadataRecord(InputStream input, String name)
 
         try {
             File file = new File(fsPathDir, name);
-            FileOutputStream os = new FileOutputStream(file);
-            try {
+            try (input; FileOutputStream os = new FileOutputStream(file)) {
                 IOUtils.copyLarge(input, os);
-            } finally {
-                Closeables.close(os, true);
-                Closeables.close(input, true);
             }
         } catch (IOException e) {
             LOG.error("Exception while adding metadata record with name {}, {}",

oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/OakFileDataStore.java

@@ -33,7 +33,6 @@
 import java.util.Set;
 
 import org.apache.commons.lang3.StringUtils;
-import org.apache.jackrabbit.guava.common.io.Closeables;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.io.filefilter.FileFilterUtils;
@@ -139,12 +138,8 @@ public void addMetadataRecord(InputStream input, String name)
 
         try {
             File file = new File(getPath(), name);
-            FileOutputStream os = new FileOutputStream(file);
-            try {
+            try (input; FileOutputStream os = new FileOutputStream(file)) {
                 IOUtils.copyLarge(input, os);
-            } finally {
-                Closeables.close(os, true);
-                Closeables.close(input, true);
             }
         } catch (IOException e) {
             LOG.error("Exception while adding metadata record with name {}, {}",

oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/UploadStagingCacheTest.java

@@ -42,7 +42,6 @@
 import ch.qos.logback.classic.Level;
 
 import org.apache.jackrabbit.guava.common.collect.Iterators;
-import org.apache.jackrabbit.guava.common.io.Closeables;
 import org.apache.jackrabbit.guava.common.io.Closer;
 import org.apache.jackrabbit.guava.common.util.concurrent.Futures;
 import org.apache.jackrabbit.guava.common.util.concurrent.ListenableFuture;
@@ -693,7 +692,7 @@ public void testUpgradeCompromisedSerializedMap() throws IOException {
         // Create pre-upgrade load
         File home = folder.newFolder();
         File pendingUploadsFile = new File(home, DataStoreCacheUpgradeUtils.UPLOAD_MAP);
-        createGibberishLoad(home, pendingUploadsFile);
+        createGibberishLoad(pendingUploadsFile);
 
         LogCustomizer lc = LogCustomizer.forLogger(DataStoreCacheUpgradeUtils.class.getName())
             .filter(Level.WARN)
@@ -717,13 +716,9 @@ private void createUpgradeLoad(File home, File pendingUploadFile) throws IOExcep
     }
 
 
-    private void createGibberishLoad(File home, File pendingUploadFile) throws IOException {
-        BufferedWriter writer = null;
-        try {
-            writer = new BufferedWriter(new FileWriter(pendingUploadFile, StandardCharsets.UTF_8));
+    private void createGibberishLoad(File pendingUploadFile) throws IOException {
+        try (BufferedWriter writer = new BufferedWriter(new FileWriter(pendingUploadFile, StandardCharsets.UTF_8))) {
             FileIOUtils.writeAsLine(writer, "jerhgiuheirghoeoorqehgsjlwjpfkkwpkf", false);
-        } finally {
-            Closeables.close(writer, true);
         }
     }
 

oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTrackerClusterSharedTest.java

@@ -29,6 +29,7 @@
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 
+import org.apache.commons.io.IOUtils;
 import org.apache.jackrabbit.core.data.DataStoreException;
 import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser;
 import org.apache.jackrabbit.oak.plugins.blob.SharedDataStore;
@@ -41,7 +42,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static org.apache.jackrabbit.guava.common.io.Closeables.close;
 import static java.lang.String.valueOf;
 import static java.util.UUID.randomUUID;
 import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;
@@ -224,11 +224,11 @@ private static Set<String> retrieve(BlobTracker tracker) throws IOException {
         Set<String> retrieved = new HashSet<>();
         Iterator<String> iter = tracker.get();
         log.info("retrieving blob ids");
-        while(iter.hasNext()) {
+        while (iter.hasNext()) {
             retrieved.add(iter.next());
         }
         if (iter instanceof Closeable) {
-            close((Closeable)iter, true);
+            IOUtils.closeQuietly((Closeable)iter);
         }
         return retrieved;
     }

oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/FileIOUtils.java

@@ -20,7 +20,6 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.apache.commons.io.FileUtils.forceDelete;
 import static org.apache.commons.io.IOUtils.copyLarge;
-import static org.apache.jackrabbit.guava.common.io.Closeables.close;
 import static org.apache.jackrabbit.oak.commons.sort.EscapeUtils.escapeLineBreak;
 import static org.apache.jackrabbit.oak.commons.sort.EscapeUtils.unescapeLineBreaks;
 import static org.apache.jackrabbit.oak.commons.sort.ExternalSort.mergeSortedFiles;
@@ -140,25 +139,20 @@ public static File copy(InputStream stream) throws IOException {
      * @throws IOException
      */
     public static void append(List<File> files, File appendTo, boolean delete) throws IOException {
-        OutputStream appendStream = null;
-        boolean threw = true;
 
-        try {
-            appendStream = new BufferedOutputStream(new FileOutputStream(appendTo, true));
+        try (OutputStream appendStream = new BufferedOutputStream(new FileOutputStream(appendTo, true))) {
 
             for (File f : files) {
                 try (InputStream iStream = new FileInputStream(f)) {
                     copyLarge(iStream, appendStream);
                 }
             }
-            threw = false;
         } finally {
             if (delete) {
                 for (File f : files) {
                     f.delete();
                 }
             }
-            close(appendStream, threw);
         }
     }
 
@@ -227,11 +221,9 @@ public static int writeStrings(Iterator<String> iterator, File f, boolean escape
      */
     public static int writeStrings(Iterator<String> iterator, File f, boolean escape,
         @NotNull Function<String, String> transformer, @Nullable Logger logger, @Nullable String message) throws IOException {
-        BufferedWriter writer = new BufferedWriter(new FileWriter(f, UTF_8));
-        boolean threw = true;
 
         int count = 0;
-        try {
+        try (BufferedWriter writer = new BufferedWriter(new FileWriter(f, UTF_8))) {
             while (iterator.hasNext()) {
                 writeAsLine(writer, transformer.apply(iterator.next()), escape);
                 count++;
@@ -241,9 +233,6 @@ public static int writeStrings(Iterator<String> iterator, File f, boolean escape
                     }
                 }
             }
-            threw = false;
-        } finally {
-            close(writer, threw);
         }
         return count;
     }
@@ -257,23 +246,17 @@ public static int writeStrings(Iterator<String> iterator, File f, boolean escape
      * @throws IOException
      */
     public static Set<String> readStringsAsSet(InputStream stream, boolean unescape) throws IOException {
-        BufferedReader reader = null;
         Set<String> set = new HashSet<>();
-        boolean threw = true;
 
-        try {
-            reader = new BufferedReader(new InputStreamReader(stream, UTF_8));
-            String line  = null;
+        try (BufferedReader reader = new BufferedReader(new InputStreamReader(stream, UTF_8))) {
+            String line;
             while ((line = reader.readLine()) != null) {
                 if (unescape) {
                     set.add(unescapeLineBreaks(line));
                 } else {
                     set.add(line);
                 }
             }
-            threw = false;
-        } finally {
-            close(reader, threw);
         }
         return set;
     }