Make readAll() resilient to exceptions reading a single file

filesystem/src/main/java/com/nytimes/android/external/fs3/FSAllReader.java

@@ -3,6 +3,8 @@
 import com.nytimes.android.external.fs3.filesystem.FileSystem;
 import com.nytimes.android.external.store3.base.DiskAllRead;
 
+import com.nytimes.android.external.store3.base.ReadResult;
+import com.nytimes.android.external.store3.base.ReadResultFactory;
 import java.io.FileNotFoundException;
 
 import javax.annotation.Nonnull;
@@ -15,7 +17,6 @@
  * FSReader is used when persisting from file system
  * PathResolver will be used in creating file system paths based on cache keys.
  * Make sure to have keys containing same data resolve to same "path"
- *
  */
 public class FSAllReader implements DiskAllRead {
     final FileSystem fileSystem;
@@ -24,6 +25,25 @@ public FSAllReader(FileSystem fileSystem) {
         this.fileSystem = fileSystem;
     }
 
+    @Nonnull
+    @Override
+    public Observable<ReadResult<BufferedSource>> safeReadAll(@Nonnull final String path) {
+        return Observable.defer(() -> {
+            Observable<ReadResult<BufferedSource>> bufferedSourceObservable = null;
+            try {
+                bufferedSourceObservable = Observable
+                    .fromIterable(fileSystem.list(path))
+                    .flatMap(s ->
+                        Observable.defer(() -> Observable.just(fileSystem.read(s)))
+                            .map(ReadResultFactory::createSuccessResult)
+                            .onErrorReturn(ReadResultFactory::createFailureResult));
+            } catch (FileNotFoundException e) {
+                throw Exceptions.propagate(e);
+            }
+            return bufferedSourceObservable;
+        });
+    }
+
     @Nonnull
     @Override
     public Observable<BufferedSource> readAll(@Nonnull final String path) throws FileNotFoundException {

filesystem/src/main/java/com/nytimes/android/external/fs3/SourceAllPersister.java

@@ -3,6 +3,7 @@
 
 import com.nytimes.android.external.fs3.filesystem.FileSystem;
 import com.nytimes.android.external.store3.base.AllPersister;
+import com.nytimes.android.external.store3.base.ReadResult;
 import com.nytimes.android.external.store3.base.impl.BarCode;
 
 import java.io.FileNotFoundException;
@@ -35,6 +36,12 @@ public SourceAllPersister(FileSystem fileSystem) {
         sourceFileWriter = new FSWriter<>(fileSystem,  new BarCodeReadAllPathResolver());
     }
 
+    @Nonnull
+    @Override
+    public Observable<ReadResult<BufferedSource>> safeReadAll(@Nonnull String path) {
+        return sourceFileAllReader.safeReadAll(path);
+    }
+
     @Nonnull
     @Override
     public Observable<BufferedSource> readAll(@Nonnull final String path) throws FileNotFoundException {

filesystem/src/test/java/com/nytimes/android/external/fs3/FSAllOperationTest.java

@@ -1,9 +1,11 @@
 package com.nytimes.android.external.fs3;
 
 
+import com.nytimes.android.external.fs3.filesystem.CrashOnReadFileSystem;
 import com.nytimes.android.external.fs3.filesystem.FileSystem;
 import com.nytimes.android.external.fs3.filesystem.FileSystemFactory;
 
+import com.nytimes.android.external.store3.base.ReadResult;
 import org.junit.Test;
 
 import java.io.ByteArrayInputStream;
@@ -45,6 +47,42 @@ public void readAll() throws IOException {
         assertThat(observable.blockingLast().readUtf8()).isEqualTo(CHALLAH_CHALLAH);
     }
 
+    @Test
+    public void safeReadAllWithCrash() throws IOException {
+        File tempDir = createTempDir();
+        FileSystem fileSystem = new CrashOnReadFileSystem(tempDir);
+
+        //write different data to File System for each barcode
+        fileSystem.write(FOLDER + "/key.txt", source(CHALLAH));
+        fileSystem.write(FOLDER + "/key_crash.txt", source(CHALLAH));
+        fileSystem.write(FOLDER + "/" + INNER_FOLDER + "/key2.txt", source(CHALLAH_CHALLAH));
+        FSAllReader reader = new FSAllReader(fileSystem);
+        //read back all values for the FOLDER
+        Observable<ReadResult<BufferedSource>> observable = reader.safeReadAll(FOLDER);
+        observable.test()
+            .assertValueAt(0, bufferedSourceReadResult ->
+                bufferedSourceReadResult.getResult().readUtf8().equals(CHALLAH))
+            .assertValueAt(1, bufferedSourceReadResult -> !bufferedSourceReadResult.isSuccess())
+            .assertValueAt(2, bufferedSourceReadResult ->
+                bufferedSourceReadResult.getResult().readUtf8().equals(CHALLAH_CHALLAH));
+    }
+
+    @Test(expected = RuntimeException.class)
+    public void readAllWithCrash() throws IOException {
+        File tempDir = createTempDir();
+        FileSystem fileSystem = new CrashOnReadFileSystem(tempDir);
+
+        //write different data to File System for each barcode
+        fileSystem.write(FOLDER + "/key.txt", source(CHALLAH));
+        fileSystem.write(FOLDER + "/key_crash.txt", source(CHALLAH));
+        fileSystem.write(FOLDER + "/" + INNER_FOLDER + "/key2.txt", source(CHALLAH_CHALLAH));
+        FSAllReader reader = new FSAllReader(fileSystem);
+        //read back all values for the FOLDER
+        Observable<BufferedSource> observable = reader.readAll(FOLDER);
+        assertThat(observable.blockingFirst().readUtf8()).isEqualTo(CHALLAH);
+        assertThat(observable.blockingLast().readUtf8()).isEqualTo(CHALLAH_CHALLAH);
+    }
+
     @Test
     public void deleteAll() throws IOException {
         File tempDir = createTempDir();

filesystem/src/test/java/com/nytimes/android/external/fs3/filesystem/CrashOnReadFileSystem.java

@@ -0,0 +1,23 @@
+package com.nytimes.android.external.fs3.filesystem;
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import javax.annotation.Nonnull;
+import okio.BufferedSource;
+
+public class CrashOnReadFileSystem extends FileSystemImpl {
+
+    public CrashOnReadFileSystem(@Nonnull File root) throws IOException {
+        super(root);
+    }
+
+    @Nonnull
+    @Override
+    public BufferedSource read(@Nonnull String path) throws FileNotFoundException {
+        if (path.contains("crash")) {
+            throw new FileNotFoundException();
+        }
+        return super.read(path);
+    }
+}

store/src/main/java/com/nytimes/android/external/store3/base/AllPersister.java

@@ -10,15 +10,26 @@
 import io.reactivex.Single;
 
 
-public interface AllPersister<Raw, Key> extends Persister<Raw, Key>, DiskAllRead, DiskAllErase {
+public interface AllPersister<Raw, Key> extends Persister<Raw, Key>, DiskAllRead<Raw>, DiskAllErase {
+
     /**
      * @param path to use to get data from persister
      *                If data is not available implementer needs to
-     *                throw an exception
+     *                check for failures in the emitted {@link ReadResult}
      */
+    @Nonnull
     @Override
+    Observable<ReadResult<Raw>> safeReadAll(@Nonnull final String path);
+
+    /**
+     * @deprecated Use {@link #safeReadAll(String)} instead
+     * @param path to use to get data from persister
+     *                If data is not available implementer needs to
+     *                throw an exception
+     */
     @Nonnull
-    Observable<Raw> readAll(@Nonnull final String path) throws FileNotFoundException;
+    @Override
+    Observable<Raw> readAll(@Nonnull String path) throws FileNotFoundException;
 
     /**
      * @param path to delete all the data in the the path.

store/src/main/java/com/nytimes/android/external/store3/base/DiskAllRead.java

@@ -8,7 +8,15 @@
 import io.reactivex.Observable;
 
 public interface DiskAllRead<Raw> {
+
+    /**
+     * Use {@link #safeReadAll(String)} instead
+     */
+    @Deprecated
     @Nonnull
     Observable<Raw> readAll(@Nonnull String path) throws FileNotFoundException;
+
+    @Nonnull
+    Observable<ReadResult<Raw>> safeReadAll(@Nonnull final String path);
 }
 

store/src/main/java/com/nytimes/android/external/store3/base/ReadResult.java

@@ -0,0 +1,27 @@
+package com.nytimes.android.external.store3.base;
+
+import javax.annotation.Nullable;
+
+public final class ReadResult<Raw> {
+    private final Raw raw;
+    private final Throwable throwable;
+
+    ReadResult(Raw raw, Throwable throwable) {
+        this.raw = raw;
+        this.throwable = throwable;
+    }
+
+    @Nullable
+    public Raw getResult() {
+        return raw;
+    }
+
+    @Nullable
+    public Throwable getThrowable() {
+        return throwable;
+    }
+
+    public boolean isSuccess() {
+        return raw != null;
+    }
+}

store/src/main/java/com/nytimes/android/external/store3/base/ReadResultFactory.java

@@ -0,0 +1,25 @@
+package com.nytimes.android.external.store3.base;
+
+import javax.annotation.Nonnull;
+
+public final class ReadResultFactory {
+
+    private ReadResultFactory() {
+    }
+
+    @Nonnull
+    public static <Raw> ReadResult<Raw> createFailureResult(@Nonnull Throwable throwable) {
+        if (throwable == null) {
+            throw new IllegalArgumentException("throwable cannot be null.");
+        }
+        return new ReadResult<>(null, throwable);
+    }
+
+    @Nonnull
+    public static <Raw> ReadResult<Raw> createSuccessResult(@Nonnull Raw result) {
+        if (result == null) {
+            throw new IllegalArgumentException("result cannot be null.");
+        }
+        return new ReadResult<>(result, null);
+    }
+}