You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by am...@apache.org on 2018/02/01 07:43:20 UTC

svn commit: r1822850 - in /jackrabbit/oak/trunk: oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/ oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/ oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/ ...

Author: amitj
Date: Thu Feb  1 07:43:20 2018
New Revision: 1822850

URL: http://svn.apache.org/viewvc?rev=1822850&view=rev
Log:
OAK-7223: Files could be kept partially in case of disconnection from backends

- Created a utility method in FileIOUtils#copyInputStreamToFile which delegates to IOUtils but deletes file on exception thus removing any potential partial files created.
- Using the new method in FileCache load.

Modified:
    jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/FileCache.java
    jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractDataStoreCacheTest.java
    jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/FileCacheTest.java
    jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/FileIOUtils.java
    jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/package-info.java
    jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/FileIOUtilsTest.java

Modified: jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/FileCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/FileCache.java?rev=1822850&r1=1822849&r2=1822850&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/FileCache.java (original)
+++ jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/FileCache.java Thu Feb  1 07:43:20 2018
@@ -51,8 +51,8 @@ import org.apache.jackrabbit.oak.commons
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static org.apache.commons.io.FileUtils.copyInputStreamToFile;
 import static org.apache.commons.io.FilenameUtils.normalizeNoEndSeparator;
+import static org.apache.jackrabbit.oak.commons.FileIOUtils.copyInputStreamToFile;
 
 /**
  */
@@ -137,6 +137,9 @@ public class FileCache extends AbstractC
                             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);
                         }

Modified: jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractDataStoreCacheTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractDataStoreCacheTest.java?rev=1822850&r1=1822849&r2=1822850&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractDataStoreCacheTest.java (original)
+++ jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractDataStoreCacheTest.java Thu Feb  1 07:43:20 2018
@@ -94,7 +94,7 @@ public class AbstractDataStoreCacheTest
 
 
     static class TestCacheLoader<S, I> extends CacheLoader<String, FileInputStream> {
-        private final File root;
+        protected final File root;
 
         public TestCacheLoader(File dir) {
             this.root = new File(dir, "datastore");
@@ -117,6 +117,46 @@ public class AbstractDataStoreCacheTest
         }
     }
 
+
+    /**
+     * Throws error after reading partially defined by max
+     */
+    private static class ErrorInputStream extends FileInputStream {
+        private long bytesread;
+        private long max;
+
+        ErrorInputStream(File file, long max) throws FileNotFoundException {
+            super(file);
+            this.max = max;
+        }
+
+        @Override
+        public int read(byte b[]) throws IOException {
+            bytesread += b.length;
+            if (bytesread > max) {
+                throw new IOException("Disconnected");
+            }
+            return super.read(b);
+        }
+    }
+
+
+    /**
+     * Test loader which uses the ErrorInputStream for load
+     */
+    static class TestErrorCacheLoader<S, I> extends TestCacheLoader<String, FileInputStream> {
+        private long max;
+
+        public TestErrorCacheLoader(File dir, long max) {
+            super(dir);
+            this.max = max;
+        }
+
+        @Override public FileInputStream load(@Nonnull String key) throws Exception {
+            return new ErrorInputStream(getFile(key, root), max);
+        }
+    }
+
     static class TestPoolExecutor extends ThreadPoolExecutor {
         private final CountDownLatch beforeLatch;
         private final CountDownLatch afterLatch;

Modified: jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/FileCacheTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/FileCacheTest.java?rev=1822850&r1=1822849&r2=1822850&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/FileCacheTest.java (original)
+++ jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/FileCacheTest.java Thu Feb  1 07:43:20 2018
@@ -38,6 +38,7 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
 import org.junit.rules.TestName;
 
@@ -59,6 +60,9 @@ public class FileCacheTest extends Abstr
     private Closer closer;
 
     @Rule
+    public ExpectedException expectedEx = ExpectedException.none();
+
+    @Rule
     public TemporaryFolder folder = new TemporaryFolder(new File("target"));
 
     @Rule
@@ -111,6 +115,25 @@ public class FileCacheTest extends Abstr
         LOG.info("Finished zeroCache");
     }
 
+    @Test
+    public void loadError() throws Exception {
+        LOG.info("Started loadError");
+
+        loader = new TestErrorCacheLoader<String, InputStream>(folder.newFolder(), 8192);
+        cache = FileCache.build(12 * 1024/* KB */, root, loader, null);
+        closer.register(cache);
+        createFile(0, loader, cache, folder, 12 * 1024);
+        try {
+            cache.get(ID_PREFIX + 0);
+        } catch (IOException e) {
+        }
+
+        expectedEx.expect(IOException.class);
+        cache.get(ID_PREFIX + 0);
+
+        LOG.info("Finished loadError");
+    }
+
     /**
      * Load and get from cache.
      * @throws Exception
@@ -503,9 +526,14 @@ public class FileCacheTest extends Abstr
         assertTrue(Files.equal(f, cached));
     }
 
+    private static File createFile(int seed, TestCacheLoader loader, FileCache cache, TemporaryFolder folder)
+        throws Exception {
+        return createFile(seed, loader, cache, folder, 4 * 1024);
+    }
+
     private static File createFile(int seed, TestCacheLoader loader, FileCache cache,
-        TemporaryFolder folder) throws Exception {
-        File f = copyToFile(randomStream(0, 4 * 1024),
+        TemporaryFolder folder, int size) throws Exception {
+        File f = copyToFile(randomStream(0, size),
             folder.newFile());
         loader.write(ID_PREFIX + seed, f);
         assertNull(cache.getIfPresent(ID_PREFIX + seed));

Modified: jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/FileIOUtils.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/FileIOUtils.java?rev=1822850&r1=1822849&r2=1822850&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/FileIOUtils.java (original)
+++ jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/FileIOUtils.java Thu Feb  1 07:43:20 2018
@@ -52,7 +52,6 @@ import static com.google.common.io.Files
 import static com.google.common.io.Files.move;
 import static com.google.common.io.Files.newWriter;
 import static java.io.File.createTempFile;
-import static org.apache.commons.io.FileUtils.copyInputStreamToFile;
 import static org.apache.commons.io.FileUtils.forceDelete;
 import static org.apache.commons.io.IOUtils.closeQuietly;
 import static org.apache.commons.io.IOUtils.copyLarge;
@@ -317,6 +316,26 @@ public final class FileIOUtils {
     }
 
     /**
+     *
+     * Copy the input stream to the given file. Delete the file in case of exception.
+     *
+     * @param source the input stream source
+     * @param destination the file to write to
+     * @throws IOException
+     */
+    public static void copyInputStreamToFile(final InputStream source, final File destination) throws IOException {
+        boolean success = false;
+        try {
+            FileUtils.copyInputStreamToFile(source, destination);
+            success = true;
+        } finally {
+            if (!success) {
+                forceDelete(destination);
+            }
+        }
+    }
+
+    /**
      * Decorates the given comparator and applies the function before delegating to the decorated
      * comparator.
      */

Modified: jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/package-info.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/package-info.java?rev=1822850&r1=1822849&r2=1822850&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/package-info.java (original)
+++ jackrabbit/oak/trunk/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/package-info.java Thu Feb  1 07:43:20 2018
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-@Version("1.0.0")
+@Version("1.1.0")
 package org.apache.jackrabbit.oak.commons;
 
-import org.osgi.annotation.versioning.Version;
\ No newline at end of file
+import org.osgi.annotation.versioning.Version;

Modified: jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/FileIOUtilsTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/FileIOUtilsTest.java?rev=1822850&r1=1822849&r2=1822850&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/FileIOUtilsTest.java (original)
+++ jackrabbit/oak/trunk/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/FileIOUtilsTest.java Thu Feb  1 07:43:20 2018
@@ -22,6 +22,7 @@ import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -49,7 +50,6 @@ import org.apache.jackrabbit.oak.commons
 import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
 
 import static com.google.common.base.Charsets.UTF_8;
@@ -398,6 +398,32 @@ public class FileIOUtilsTest {
         assertTrue(!f.exists());
     }
 
+    @Test
+    public void copyStreamToFile() throws Exception {
+        Set<String> added = newHashSet("a", "z", "e", "b");
+        File f = assertWrite(added.iterator(), false, added.size());
+
+        File f2 = folder.newFile();
+        FileIOUtils.copyInputStreamToFile(new FileInputStream(f), f2);
+        assertEquals(added, readStringsAsSet(new FileInputStream(f), false));
+        assertTrue(f.exists());
+    }
+
+    @Test
+    public void copyStreamToFileNoPartialCreation() throws Exception {
+        File f = folder.newFile();
+        FileIOUtils.copyInputStreamToFile(randomStream(12, 8192), f);
+
+        File f2 = folder.newFile();
+        try {
+            FileIOUtils.copyInputStreamToFile(new ErrorInputStream(f, 4096), f2);
+            Assert.fail("Should have failed with IOException");
+        } catch (Exception e) {}
+
+        assertTrue(f.exists());
+        assertTrue(!f2.exists());
+    }
+
     private static List<String> getLineBreakStrings() {
         return newArrayList("ab\nc\r", "ab\\z", "a\\\\z\nc",
             "/a", "/a/b\nc", "/a/b\rd", "/a/b\r\ne", "/a/c");
@@ -453,4 +479,26 @@ public class FileIOUtilsTest {
         r.nextBytes(data);
         return new ByteArrayInputStream(data);
     }
+
+    /**
+     * Throws error after reading partially defined by max
+     */
+    private static class ErrorInputStream extends FileInputStream {
+        private long bytesread;
+        private long max;
+
+        ErrorInputStream(File file, long max) throws FileNotFoundException {
+            super(file);
+            this.max = max;
+        }
+
+        @Override
+        public int read(byte b[]) throws IOException {
+            bytesread += b.length;
+            if (bytesread > max) {
+                throw new IOException("Disconnected");
+            }
+            return super.read(b);
+        }
+    }
 }