Describe the bug Application crash after update Kotlin from 1.4.32 to 1.5.0-RC, no other changes in code.

Catch Exception:

E/AndroidRuntime: FATAL EXCEPTION: main
    Process: ru.superapplication.android.app.debug, PID: 18674
    java.lang.NoSuchMethodError: No static method getHours(I)D in class Lkotlin/time/DurationKt; or its super classes (declaration of 'kotlin.time.DurationKt' appears in /data/app/~~l_eMNOPYSnv8XUlDP7ycgA==/ru.superapplication.android.app.debug-ziiU-phO368T6tA7R1CseQ==/base.apk!classes33.dex)
        at com.dropbox.android.external.store4.StoreDefaults.<clinit>(StoreDefaults.kt:15)
        at com.dropbox.android.external.store4.RealStoreBuilder.<init>(StoreBuilder.kt:91)
        at com.dropbox.android.external.store4.StoreBuilder$Companion.from(StoreBuilder.kt:76)

Is your feature request related to a problem? Please describe. The documentation states "Another good use case for fresh() is when a user wants to pull to refresh." While this is correct, there is a drawback to using fresh() for pull to refresh events: We don't get the Loading or Error states from the StoreResponse. This may mean implementing additional logic around showing the user the loading state or error state in the UI when using both stream(), for the primary flow of data, and fresh() for the pull to refresh event.

Describe the solution you'd like I created the following extensions to be used in a pull refresh scenario and have found they work well. Since pull to refresh is common, perhaps these would be good to integrate into Store somehow?

/**
 * Use to trigger a network refresh for your Store. An example use-case is for pull to refresh functionality.
 * This is best used in conjunction with `Flow.collectRefreshFlow` to prevent this returned Flow from living
 * on longer than necessary and conflicting with your primary Store Flow.
 */
fun <Key : Any, Output : Any> Store<Key, Output>.refreshFlow(key: Key) = stream(StoreRequest.fresh(key))

/**
 * Helper to observe the loading state of a Store refresh call triggered via `Store.refreshFlow`. This Flow will
 * automatically be cancelled after the refresh is successful or has an error.
 *
 * @param checkLoadingState Lambda providing the current StoreResponse for the refresh (Loading, Error, or Data) allowing
 * you to decide when to show/hide your loading indicator.
 */
suspend fun <T> Flow<StoreResponse<T>>.collectRefreshFlow(checkLoadingState: suspend (StoreResponse<T>) -> Unit) {
    onEach { storeResponse ->
        checkLoadingState(storeResponse)
    }.first { storeResponse ->
        storeResponse.isData() || storeResponse.isError()
    }
}

Examples of the solution in use Below is an example of how refreshFlow is used alongside the main flow:

fun getUserDataFlow(): Flow<StoreResponse<List<UserData>>> {
   return myStore.stream(StoreRequest.cached(Key("123"), refresh = true))
}

fun refreshUserDataFlow(): Flow<StoreResponse<List<UserData>>> {
   return myStore.refreshFlow(Key("123"))
}

And finally an example of how collectRefreshFlow is used in a ViewModel alongside the main flow:

class MyViewModel {
   val userDataLiveData = userRepository.getUserDataFlow()
      .onEach { storeResponse ->
            setLoadingSpinnerVisibility(storeResponse)
            setEmptyStateVisibility(storeResponse)
       }
       .filterIsInstance<StoreResponse.Data<List<UserData>>>()
       .mapLatest { storeResponse ->
            storeResponse.dataOrNull() ?: emptyList()
       }
       .flowOn(Dispatchers.IO)
       .asLiveData().distinctUntilChanged()
   }

   fun onRetryLoadUserData() = viewModelScope.launch {
      if (networkIsConnected()) {
         userRepository.refreshUserDataFlow()
            .collectRefreshFlow { storeResponse -> setLoadingSpinnerVisibility(storeResponse) }
      }
   }
}

Additional context I feel these two extensions provide more power/flexibility in the pull to refresh scenario than simply calling the suspending fresh() method. Does it seem like these or something similar could fit into the Store API?

Resolves #14

This is a rewrite of the cache module in Kotlin. The new implementation supports sized-based and time-based (time-to-live and time-to-idle) evictions.

Cache and Builder APIs

interface Cache<in Key, Value> {

    /**
     * Returns the value associated with [key] in this cache, or null if there is no
     * cached value for [key].
     */
    fun get(key: Key): Value?

    /**
     * Returns the value associated with [key] in this cache if exists,
     * otherwise gets the value by invoking [loader], associates the value with [key] in the cache,
     * and returns the cached value.
     *
     * Note that if an unexpired value for the [key] is present by the time the [loader] returns
     * the new value, the existing value won't be replaced by the new value. Instead the existing
     * value will be returned.
     */
    fun get(key: Key, loader: () -> Value): Value

    /**
     * Associates [value] with [key] in this cache. If the cache previously contained a
     * value associated with [key], the old value is replaced by [value].
     */
    fun put(key: Key, value: Value)

    /**
     * Discards any cached value for key [key].
     */
    fun invalidate(key: Key)

    /**
     * Discards all entries in the cache.
     */
    fun invalidateAll()

    /**
     * Main entry point for creating a [Cache].
     */
    interface Builder {

        /**
         * Specifies that each entry should be automatically removed from the cache once a fixed duration
         * has elapsed after the entry's creation or the most recent replacement of its value.
         *
         * When [duration] is zero, the cache's max size will be set to 0
         * meaning no values will be cached.
         */
        fun expireAfterWrite(duration: Long, unit: TimeUnit): Builder

        /**
         * Specifies that each entry should be automatically removed from the cache once a fixed duration
         * has elapsed after the entry's creation, the most recent replacement of its value, or its last
         * access.
         *
         * When [duration] is zero, the cache's max size will be set to 0
         * meaning no values will be cached.
         */
        fun expireAfterAccess(duration: Long, unit: TimeUnit): Builder

        /**
         * Specifies the maximum number of entries the cache may contain.
         * Cache eviction policy is based on LRU - i.e. least recently accessed entries get evicted first.
         *
         * When [size] is 0, entries will be discarded immediately and no values will be cached.
         *
         * If not set, cache size will be unlimited.
         */
        fun maximumCacheSize(size: Long): Builder

        /**
         * Guides the allowed concurrent update operations. This is used as a hint for internal sizing,
         * actual concurrency will vary.
         *
         * If not set, default concurrency level is 16.
         */
        fun concurrencyLevel(concurrencyLevel: Int): Builder

        /**
         * Specifies [Clock] for this cache.
         *
         * This is useful for controlling time in tests
         * where a fake [Clock] implementation can be provided.
         *
         * A [SystemClock] will be used if not specified.
         */
        fun clock(clock: Clock): Builder

        /**
         * Builds a new instance of [Cache] with the specified configurations.
         */
        fun <K : Any, V : Any> build(): Cache<K, V>
    }
}

Note that currently only APIs required by RealStore are supported. Please let know if more APIs are required.

Usage

// build a new cache
val cache = Cache.Builder.newBuilder()
            .expireAfterWrite(30, TimeUnit.MINUTES)
            .expireAfterAccess(24, TimeUnit.HOURS)
            .maximumCacheSize(100)
            .build<Long, String>()

// cache a value
cache.put(1, "dog")

// get cached value by key
cache.get(1)

// get cached value by key, using the provided loader to compute and cache the value if none exists
val value = cache.get(2) { "cat" }

// remove a cached value by key
cache.invalidate(1)

// remove all values from cache
cache.invalidateAll()

Tests

./gradlew cache:test

100% Test Coverage:

Migration

All usages have been switched to use the new cache implementation.

I've also converted the remaining Java files to Kotlin, except MultiParserTest.java which is commented out.

Will update cache/README.md in a followup.

Exceptions aren't good. I think the Kotlin community is coalescing around returning a sealed class with error information instead of throwing an exception to indicate that a function has an error. That's what Store does with the StoreResponse class. However, a StoreResponse.Error object will only be returned if the fetcher throws an exception. Now the libraries commonly used by a fetcher, such as Retrofit or Ktor, throw exceptions on error (bad Ktor, no biscuit). But what if someone wants to use a library that returns a sealed class to indicate success or failure?

For example, if MyLibrary returns a MyLibraryResult.Success or a MyLibraryResult.Error, how should the fetcher or Store handle it?

• The app could define a custom exception which the fetcher throws when it receives an error, but we're trying to get away from exceptions. Introducing an exception between two libraries that don't use exceptions doesn't sound right.
• The fetcher could return the MyLibraryResult object (whether or not there was an error), but then Store doesn't know there was a failure. Also the persister has to deal with unwrapping the data while storing it, and handling any errors.
• There could be some direct way for the fetcher to tell Store there was a failure. The fetcher lambda could be an extension function on an interface, perhaps with a reportError() method. If this is called then the fetcher's return value is ignored and it is treated as an error.
• The fetcher returns a StoreResponse.Error object, which Store detects and treats the same as if an exception was thrown.
• The StoreBuilder functions, in addition to the key and output types, has a third type for errors. If the fetcher returns an object of this class, it treats it as an error. For example: StoreBuilder.fromNotFlow<String, Data, MyLibraryResult.Error>(). If the fetcher succeeds, it unwraps the data from the result class and returns it. If it fails, it returns an error object.

The StoreResponse class would need to be modified to handle this new kind of error. Maybe there are two error subclasses, one for exceptions and one for non-exceptions. Maybe StoreResponse.Error can contain either a Throwable or some other error class.

Let's review this simple example

  fun fetchFromCollection(keys: Set<MySerializedType>){
        someNonblockingScope.launch {
            for(key in keys) {
                myStore.steam(StoreRequest.cached(key, true)).collect {
                    if (it.origin == ResponseOrigin.Fetcher && it !is StoreResponse.Loading)
                        Log.d("CHECKING TRIGGER", "$key triggered")
                }
            }
        }
    }

the expected outcome should be that CHECKING TRIGGER will trigger once for each unique key even if fetchFromCollection executed several times (each time always unique keys)?

If I execute fetchFromCollection with one unique set of keys it will run as expected, but if I execute second time fetchFromCollection with another different set of keys it will trigger CHECKING TRIGGER condition more than once(new set of unique keys).

What I miss? Why it doesn't work as expected?

And then if execute fetchFromCollection three, four times and so on each time with new set of keys stream will just hang with Loading state forever.

I pushed this example project demonstrating the issue in full scale.

It seems that even if we set refresh flag to false in StoreRequest.cached, we still get the latest data from the fetcher. Is this the correct behavior?

Test code:

runBlocking {
    // Store with cache, without persister
    val store = StoreBuilder.fromNonFlow { key: Int -> key }.build()

    // Cache data
    store.fresh(1)

    // Stream data
    store.stream(StoreRequest.cached(1, refresh = false)).collect {
        println(it.toString())
    }
}

Output:

Data(value=1, origin=Cache)
Loading(origin=Fetcher)
Data(value=1, origin=Fetcher)

Describe the bug Duration API has been made stable in Kotlin 1.6.0, and some methods have been deleted. As result, the following error appears:

java.lang.NoSuchMethodError: No static method toDuration(DLjava/util/concurrent/TimeUnit;)J in class Lkotlin/time/DurationKt; or its super classes (declaration of 'kotlin.time.DurationKt' appears in /data/app/com.enki.Enki750g-dL2OkPW0zzwa1CQrnIAsnQ==/base.apk!classes22.dex)
        at com.dropbox.android.external.store4.MemoryPolicy.<clinit>(MemoryPolicy.kt:116)
        at com.dropbox.android.external.store4.StoreDefaults.<clinit>(StoreDefaults.kt:23)
        at com.dropbox.android.external.store4.RealStoreBuilder.<init>(StoreBuilder.kt:91)
        at com.dropbox.android.external.store4.StoreBuilder$Companion.from(StoreBuilder.kt:76)
        at com.webedia.food.config.ConfigManager.<init>(ConfigManager.kt:51)

To Reproduce

Create a store with the default options.

Expected behavior Should not crash

Infos

• Store Version: 4.0.2-KT15

Describe the bug Disclaimer: I'm no expert in coroutine scoping, so there's a great chance this is user error.
When I build a Store and declare a Scope that is not GlobalScope, I found that calling fetch on Store will leak my caller.

To Reproduce Here's a minimal activity that reproduces the issue. On config change (screen rotation), MainActivity is retained--- forever as far as I can tell. If I omit the scope argument on the Store builder or set the scope to global, I get the behavior I expect, which is that the fetch is cancelled and no scope is retained.

I tried following the ref chain down- it seems the scope assigned to FetcherController (which is the scope I passed into the builder) is ultimately retaining my Activity.

class MainActivity : AppCompatActivity() {
    var time: Long = 0
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)
        time = System.currentTimeMillis()

        lifecycleScope.launch {
            store.fresh(time)
        }
    }
}

object Repository {
    val store = StoreBuilder.from(
        fetcher = nonFlowFetcher<Long, Long> {
            delay(5000)
            FetcherResult.Data(it)        }
    ).scope(CoroutineScope(Dispatchers.Main)).build()
}

Expected behavior My expectation is that when I cancel the scope around my Store call (store.fresh()), I'd see a cancellation of any child jobs or Flows that Store has spawned for me, and I would not be retained by Store's scope.

** Additional context** Here's a heap dump! heap_dump_several_main_activities.txt

After reading #73 and the RealStore implementation, I realized it's effectively useless to set expireAfterAccess or expireAfterWrite on the MemoryPolicy when a sourceOfTruth is provided to the store, as the disk value will be immediately backfilled to the cache after cache expiration. From the user's perspective, they'll get the data from the disk instead of cache (slightly slower) when cache has expired but the fetcher won't be triggered (unless they bypass Store and manually delete the entry from disk).

This behavior is inconsistent to when sourceOfTruth == null, in which case cache expiry will trigger the fetcher (thanks to piggybackOnly).

Should we try to make these 2 branches (with or without sourceOfTruth) behave consistently in terms of the effect of cache expiry?

I remember the old Store had a networkBeforeStale() API for triggering the fetcher if the disk entry is stale. Does it make sense to support this (on both memory cache and sourceOfTruth?)?

I understand that cache expiry and disk entry becoming stale could be interpreted as different concepts so unifying the behavior with or without sourceOfTruth might cause other confusions...

Non-working PR to show the proposed way of adding support for allowing the fetcher to communicate errors not via exceptions.

The main complication here is in the builder API. we already have 2 builders because we support a builder with and without source of truth (to change to generic type). If we went this way we would either need to support 4 builder or we will need to change the builder API to force providing the source of truth (and the new translator) in the builder's constructor

Describe the bug When a fetcher flow just completes without emitting any data the downstream flows will never complete. SourceOfTruth reader won't be called

To Reproduce fetcher = { key -> flow {} }

Expected behavior Store should detect when the fetcher flow completes and allow the rest of the process to continue as normal.

Store Version 4.0.0-alpha06

Additional context Fetchers flows that don't emit anything are extremely useful when manually dealing with caches that can't utilize the OkHttp cache.

Please see our contributing guidelines (contributing.md) primarily make sure to sign our cla as we cannot accept code externally without a signed cla

https://opensource.dropbox.com/cla/

    it looks like we can have a version of the converter that has:

fromNetworkToSOT fromSOTToCommon fromCommonToSOT

This would skip the need to make a converter going from network to common. Raises the question of whether that should be default as well. Why make someone pass a converter from network to common when they can give you a converter from network right to SOT

Originally posted by @digitalbuddha in https://github.com/MobileNativeFoundation/Store/pull/499#discussion_r1056335601

When migrating from Store4 to Store5, the below-mentioned crash happened in the Android platform.

java.lang.NoClassDefFoundError: Failed resolution of: Lkotlinx/atomicfu/AtomicFU;
        at org.mobilenativefoundation.store.store5.impl.SourceOfTruthWithBarrier.<init>(SourceOfTruthWithBarrier.kt:57)
        at org.mobilenativefoundation.store.store5.impl.RealStore.<init>(RealStore.kt:56)
        at org.mobilenativefoundation.store.store5.impl.RealStoreBuilder.build(RealStoreBuilder.kt:38)
Caused by: java.lang.ClassNotFoundException: Didn't find class "kotlinx.atomicfu.AtomicFU" on path

In the latest Store5, since the Kotlin version 1.7.21 is used, I have updated the Compose Compiler Version to 1.4.0-alpha02. But in the AtomicFU, they have mentioned turning on IR transformation by setting the below-mentioned properties in the gradle.properties file for Kotlin version >= 1.7.20.

kotlinx.atomicfu.enableJvmIrTransformation=true // for JVM IR transformation
kotlinx.atomicfu.enableJsIrTransformation=true // for JS IR transformation

But in the Store5 gradle.properties file, it was configured as mentioned below for the AtomicFU version 0.18.5.

kotlinx.atomicfu.enableJvmIrTransformation=false
kotlinx.atomicfu.enableJsIrTransformation=false
kotlin.js.compiler=ir

I'm not sure whether this might be the reason for the above-mentioned crash in the Android platform.

Problem

Android docs have guidance on conflict resolution once network is available. But they don’t consider cases of certain network services remaining unavailable. See https://developer.android.com/topic/architecture/data-layer/offline-first

Example

Imagine a user who can read from server but whose writes to server always fail. Normally we will eagerly resolve conflicts by pushing local writes before requesting latest values. But in this case the user can not push the local changes. We could:

1. Discard latest server value and continue to persist local changes until server write succeeds
2. Overwrite local changes with latest server value

Discussion (see #store-core)

@matt-ramotar - I think 1 makes the most sense. My preference is to defer to server. On first glance for me deferring to server meant overwriting with latest server value. However I think that is incorrect. I think deferring to server really means not discarding local changes until server receives them and makes a decision.

@digitalbuddha - Some folks might want destructive reads even when local writes exist.

Decision

Defer to server by default but enable configuration when:

• [x] Online + No Changes
• [x] Online + Changes
• [x] Offline + No Changes
• [x] Offline + Changes
• [ ] Can't Update + No Changes
• [ ] Can't Update + Changes
• [ ] Can't Read + No Changes
• [ ] Can't Read + Changes

    maybe move the noop logic to the converter

Originally posted by @digitalbuddha in https://github.com/MobileNativeFoundation/Store/pull/496#discussion_r1053357158

    I think eventually we should collapse to

Store.Request.Write and Store.Request.Read

same with the response

Originally posted by @digitalbuddha in https://github.com/MobileNativeFoundation/Store/pull/496#discussion_r1053355301