Fix fetchWithResult return value in case of fetch error (#382)

This PR fixes the issue #381, it contains a refactoring to return the `Result` on `fetchWithResult` instead of returning always `NETWORK` as source

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

@@ -26,7 +26,7 @@
  *                 Example usage:  @link
  */
 final class RealInternalStore<Raw, Parsed, Key> implements InternalStore<Parsed, Key> {
-    Cache<Key, Single<Parsed>> inFlightRequests;
+    Cache<Key, Single<Result<Parsed>>> inFlightRequests;
     Cache<Key, Maybe<Parsed>> memCache;
     StalePolicy stalePolicy;
     Persister<Raw, Key> persister;
@@ -188,7 +188,7 @@ public Single<Parsed> fetch(@Nonnull final Key key) {
     @Nonnull
     @Override
     public Single<Result<Parsed>> fetchWithResult(@Nonnull Key key) {
-        return fetch(key).map(Result::createFromNetwork);
+        return Single.defer(() -> fetchAndPersistResult(key));
     }
 
     /**
@@ -203,29 +203,40 @@ public Single<Result<Parsed>> fetchWithResult(@Nonnull Key key) {
      */
     @Nullable
     Single<Parsed> fetchAndPersist(@Nonnull final Key key) {
+        return fetchAndPersistResult(key).map(Result::value);
+    }
+
+    private Single<Result<Parsed>> fetchAndPersistResult(@Nonnull final Key key) {
         try {
-            return inFlightRequests.get(key, () -> response(key));
+            return inFlightRequests.get(key, () -> responseResult(key));
         } catch (ExecutionException e) {
             return Single.error(e);
         }
     }
 
     @Nonnull
     Single<Parsed> response(@Nonnull final Key key) {
+        return responseResult(key).map(Result::value);
+    }
+
+    @Nonnull
+    private Single<Result<Parsed>> responseResult(@Nonnull final Key key) {
         return fetcher()
                 .fetch(key)
                 .flatMap(raw -> persister()
                         .write(key, raw)
                         .flatMap(aBoolean -> readDisk(key).toSingle()))
+                .map(Result::createFromNetwork)
                 .onErrorResumeNext(throwable -> {
                     if (stalePolicy == StalePolicy.NETWORK_BEFORE_STALE) {
                         return readDisk(key)
                                 .switchIfEmpty(Maybe.<Parsed>error(throwable))
-                                .toSingle();
+                                .toSingle()
+                                .map(Result::createFromCache);
                     }
                     return Single.error(throwable);
                 })
-                .doOnSuccess(data -> notifySubscribers(data, key))
+                .doOnSuccess(data -> notifySubscribers(data.value(), key))
                 .doAfterTerminate(() -> inFlightRequests.invalidate(key))
                 .cache();
     }

store/src/test/java/com/nytimes/android/external/store3/StoreTest.java

@@ -14,6 +14,8 @@
 import org.junit.Test;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
+
+import java.io.IOException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import io.reactivex.Maybe;
@@ -99,6 +101,30 @@ public void testSimpleWithResult() {
         verify(fetcher, times(1)).fetch(barCode);
     }
 
+    @Test
+    public void testFetchWithResultReturnsCacheOnErrors() {
+
+        Store<String, BarCode> simpleStore = StoreBuilder.<String>barcode()
+            .persister(persister)
+            .fetcher(fetcher)
+            .networkBeforeStale()
+            .open();
+
+
+        when(fetcher.fetch(barCode))
+            .thenReturn(Single.error(new IOException()));
+
+        when(persister.read(barCode))
+            .thenReturn(Maybe.just(DISK));
+
+        when(persister.write(barCode, NETWORK))
+            .thenReturn(Single.just(true));
+
+        Result<String> result = simpleStore.fetchWithResult(barCode).blockingGet();
+
+        assertThat(result.source()).isEqualTo(Result.Source.CACHE);
+        assertThat(result.value()).isEqualTo(DISK);
+    }
 
     @Test
     public void testDoubleTap() {