MobileNativeFoundation / Store

A Kotlin Multiplatform library for building network-resilient applications
https://mobilenativefoundation.github.io/Store/
Apache License 2.0
3.16k stars 197 forks source link

Make it easier to work with lists #548

Closed matt-ramotar closed 1 year ago

matt-ramotar commented 1 year ago

TLDR

This PR delegates memory cache implementation and provides a hybrid cache with automatic list decomposition as a separate artifact. Proposed in #539

Summary

Purpose

The main goal of this PR is to delegate memory cache implementation and provide a hybrid cache with automatic list decomposition as a separate artifact​2​. However, there is a concern that enabling list decomposition may require changes to the Store, which could introduce breaking changes or separation of concerns​3​​4​.

Discussion

The contributors discussed whether a new store should be introduced, but the consensus was against this due to concerns over integration with local changes or fallback mechanisms. Instead, the suggestion is to enable memory cache delegation but default to Guava, which doesn't involve a breaking change and gives users the flexibility to work with lists as they choose​5​.

Code Changes

Regarding the code changes, a new cache implementation has been introduced, named MultiCache. This cache handles the storage and management of relationships among single items and collections, and it delegates cache storage and behavior to Guava caches. Functions include getting and putting individual items and collections, and invalidating items, collections, or all entries. It also maintains a map of item keys to collection keys to handle relationships​6​.

Other modifications include changes in GitHub actions for the build process and running tests​7​​8​, and an interface Identifiable has been added, which seems to provide a common interface for objects that can be identified by an ID​9​.

matt-ramotar commented 1 year ago

@digitalbuddha After writing this out I'm not sure there is a way to enable list decomposition without changing Store also. Rather than providing a cache, I am wondering if instead we should delegate to a cache like we do with SOT. Users would be able to add custom logic for getting and putting based on key for example

digitalbuddha commented 1 year ago

Sounds reasonable. Can we use the current one as a default?

matt-ramotar commented 1 year ago

Will sleep on this but thoughts so far - The more I think about this the more I think you are right that we should introduce a new store. Looking at Redux for example, its recommendation is to have separate slices of state. That makes sense to me. Personally I have handled this by building "single" stores and "collection" stores and wiring them together at the repository layer. My concern with delegating to a cache is separation of concerns and breaking changes. My concern with supporting through guidance on using two separate stores is performance costs from not sharing the memory cache.

matt-ramotar commented 1 year ago

@digitalbuddha I think we should enable memory cache delegation but default to Guava. I think we should not add a new store. The main concern I think we cannot overcome: The new store would not be able to integrate with local changes or fallback mechanisms because MutableStore and Superstore delegate to Store. Enabling delegation but defaulting to Guava is not a breaking change and opens up the possibility for users to work with lists as they choose. I think if a user wants to have a cache that is responsible for single items and collections, then that is their decision. Thoughts?

digitalbuddha commented 1 year ago

@digitalbuddha I think we should enable memory cache delegation but default to Guava. I think we should not add a new store. The main concern I think we cannot overcome: The new store would not be able to integrate with local changes or fallback mechanisms because MutableStore and Superstore delegate to Store. Enabling delegation but defaulting to Guava is not a breaking change and opens up the possibility for users to work with lists as they choose. I think if a user wants to have a cache that is responsible for single items and collections, then that is their decision. Thoughts?

Yup sounds good thanks for talking it through

matt-ramotar commented 1 year ago

Usage could look like this:

val freshCollectionRequest = StoreReadRequest.fresh(NoteKey.Collection("LIST_1")
val freshCollection: NoteData.Collection = store.stream(freshCollectionRequest).take(2).last().requireData()

val cachedSingleRequest = StoreReadRequest.cached(NoteKey.Single("NOTE_1"))
val cachedSingle: NoteData.Single = store.stream(cachedSingleRequest).first().requireData()