Add StoreResult.NoNewData for empty fetchers. #194
Conversation
| // In the special case where the request is skipping memory cache but the | ||
| // fetcher returned no new data we actaully want to serve cache and SoT | ||
| // data. In this case we do check the cache. If the request did not skip | ||
| // memory and no new data was returned from the fetcher we do not emit | ||
| // from cache again to avoid a emitting twice from the cache. |
There was a problem hiding this comment.
This is where things get a little ugly. we give people no control over fallback but just implicitly assume cache + SoT
There was a problem hiding this comment.
Also, I think I missed the no SoT case. need to update
There was a problem hiding this comment.
this is tricky because cache does not necessarily have the latest local copy.
also when skip cache is true if we go back to cache it is really weird. if cache is skipped, shouldn't we just fall back to SoT (especially in this code path where we are already combining w/ SoT?)
store/src/test/java/com/dropbox/android/external/store4/impl/FlowStoreTest.kt
Show resolved
Hide resolved
store/src/main/java/com/dropbox/android/external/store4/StoreResponse.kt
Show resolved
Hide resolved
| // In the special case where the request is skipping memory cache but the | ||
| // fetcher returned no new data we actaully want to serve cache and SoT | ||
| // data. In this case we do check the cache. If the request did not skip | ||
| // memory and no new data was returned from the fetcher we do not emit | ||
| // from cache again to avoid a emitting twice from the cache. |
There was a problem hiding this comment.
this is tricky because cache does not necessarily have the latest local copy.
also when skip cache is true if we go back to cache it is really weird. if cache is skipped, shouldn't we just fall back to SoT (especially in this code path where we are already combining w/ SoT?)
yigit
left a comment
yigit
left a comment
There was a problem hiding this comment.
overall lgtm, just small nits.
store/src/main/java/com/dropbox/android/external/store4/impl/RealStore.kt
Show resolved
Hide resolved
store/src/main/java/com/dropbox/android/external/store4/StoreResponse.kt
Outdated
Show resolved
Hide resolved
store/src/main/java/com/dropbox/android/external/store4/StoreResponse.kt
Show resolved
Hide resolved
store/src/test/java/com/dropbox/android/external/store4/impl/FlowStoreTest.kt
Show resolved
Hide resolved
* Add StoreResult.NoNewData for empty fetchers. * handle no SoT case * update comments
…#194) * Add StoreResult.NoNewData for empty fetchers. * handle no SoT case * update comments
solves #185 by adding no new data modeling