Store icon indicating copy to clipboard operation
Store copied to clipboard

High level design for write support

Open eyalgu opened this issue 6 years ago • 8 comments

eyalgu avatar Dec 17 '19 18:12 eyalgu

proposed data flow: https://www.lucidchart.com/invitations/accept/264350eb-27f2-43c9-bf50-2ef7f33e44df

eyalgu avatar Dec 17 '19 23:12 eyalgu

i'm actually not sure if this needs to go through store. I initially though this as just a thing which maintains your writes, and works w/ the same database (source of truth) that Store uses. There might be options for better integrations though but i don't think they need to be coupled.

Part of having support for single source of truth in store is making Store compatible with anything else that has such support.

A key requirement to make this consistent is the ability to change modification log in the same transaction as the source of truth. Otherwise, it is not feasible to be resilient w/o adding payload to the mutable objects (or to the source of truth).

I can try to push the code i had about local modification log thing to provide more background on the thinking. (which was the basis for the offline first design doc)

yigit avatar Dec 17 '19 23:12 yigit

I personally would vote for adding update support to Store. It feels like a missing core usecase of a repository pattern. Given how disjoined the read support and the write support are I can also see us doing it in a separate class/library though.

Maybe you can take me through the history of why you decided to move from exposing SourceOfTruth to exposing 3 individual functions. If we have users provide one impl of source of truth rather than individual function it would be easy to extend that interface to have user provide transactional update support for writes.

eyalgu avatar Dec 18 '19 22:12 eyalgu

My vote is for it to be part of main artifact but we to not be a part of RealStore, instead builder can return a RealUpdatingStore which can wrap a RealStore. As for the SourceOfTruth as single or multiple functions @yigit might remember, I don't feel strongly one way or another on that one. Historically Store use to have interfaces like RecordProvider or Clearable that user can add to a Persister to make a ClearingPersister or RecordPersister. I found that to be a flexible api allowing RealStore to operate differently based on the type of persister.

digitalbuddha avatar Dec 19 '19 01:12 digitalbuddha

yea there is no real reason for it to be 3 methods instead of SOurceOfTruth, it is just some API decision left TBD. Coupling with store might be nice for trivial use cases but it won't cover all and will be a limiting factor. If we implement it independently, we can still provide first class support for it, or Store can depend on it but I initially envisioned it as an independent artifact.

One major use case is when the change includes multiple stores. This is also having support for a true SourceOfTruth comes handy. For example, if it is a messaging app, i might have two store: Store<Conversation>, Store<Message>. When User marks a conversation as read, it is possibly a write in multiple tables (stores) that needs to run in a single transaction in the database. As store works well w/ an observable database, this will just work on the store side (for reads). For writes, developer would use the LocalModificationLog in a combined path for all of their local writes (both user writes and data coming from server) so that local modifications can be re-applied. Sorry this example may not be good (had 8 hrs of meetings today :/) but hopefully it conveys the idea where writes might be modfying multiple entities that are represented as different store instances.

yigit avatar Dec 19 '19 02:12 yigit

Ok lets assume we're going to support write from a separate entry point (let's call it WriteStore for until we come up with a better name).

There are 2 general options here:

  1. We support local update by recording the modification in a separate table and keep the value table unmodified until the update is confirmed by the server
  2. We support local update by immediately applying the local modification and update the source of truth's value table. We roll back in case of a conflict (which means we still need to record the local modifications so that we can re-apply them on the new base).

option 1 is simpler to implement with one caveat: The (read) Store source of truth now needs to track both the value table and the local modifications table to that it can emit reduce(baseValue, localModificaitonsList) every time a new local modification is committed locally.

I can't quite put my finger on what makes me uncomfortable with option 2 above, aside from the issue that we will have a complex source-of-truth not consistent with the server situation that can probably lead to subtle bugs down the line.

The high level API for such a store would be something like:

interface LocalModification<DiskValue> {
    fun apply(value: DiskValue): DiskValue
}

class Update<Key, DiskValue>(
    val key: Key,
    val baseValue: DiskValue,
    val modification: LocalModification<DiskValue>
)

interface WriteStore {
    suspend fun <Key, DiskValue> update(update: Update<Key, DiskValue>)
    suspend fun update(updates: List<Update<Any, Any>>) // for bulk updates
}

class RemoteUpdateConfirmation<Key, NetworkValue>(key: Key, newValue: NetworkValue, appliedMofidicaitonsCount: Int)

interface GlobalSourceOfTruth {

    fun <Key, DiskValue> readValue(key: Key): Flow<DiskValue> // reduce(baseValue, modificationList)
    suspend fun <Key, NetworkValue> writeValue(key: Key, value: NetworkValue)
    fun <Key, DiskValue>supportsDeleteValue(): Boolean
    suspend fun <Key, DiskValue> deleteValue(key: Key)

    suspend fun <Key, DiskValue> writeLocalModification(key: Key, localModification: LocalModification<DiskValue>)
    suspend fun <Key, DiskValue> readLocalModifications(key: Key): List<LocalModification<DiskValue>>
    suspend fun <Key, NetworkValue> confirmLocalModificationsAplied(confirmation: RemoteUpdateConfirmation<Key, NetworkValue>)

    suspend fun readLocalModificationsBulk(keys: List<Pair<Class<*>, Any>>): List<Pair<Any, LocalModification<Any>>>
    suspend fun recordLocalModificationsApliedBulk(confirmations: List<RemoteUpdateConfirmation<Any, Any>>)
}

typealias Updater<Key, DiskValue, NetworkValue> = suspend (Key, DiskValue) -> NetworkValue

Thoughts?

eyalgu avatar Dec 20 '19 00:12 eyalgu

I was thinking of going w/ option 2. To do offline first, app has to handle conflicts w/ the server. It is normal to make you uncomfortable, it is an uncomfortable situation :) The problem w/ not writing writes into database is that it makes it impossible to run queries properly. e.g. if I marked a message as read in a chat app, it needs to be marked as read in the database so that a query like select count(*) from messages where read= 1 can work properly.

For the API design, in my opinion, it is better to design local modifications as custom objects rather than the whole class.

For instance, this is some code from my app where i was playing w/ this pattern:

db.withTransaction {
            modificationRegistry.enqueueUpdateTask(
                taskId, UpdateTaskArgs(
                    title = title,
                    tags = tags.map { it.id },
                    localId = localTaskId
                )
            )
            db.taskDao().updateTitleAndTags(taskId, title, tags)
        }
        enqueueWorker()

In a transaction, it both saves enough information about the change and also modifies the database. Same modification is run over any incoming data from network as well so that if any local modification has not arrived to the server yet, it gets re-applied before it is saved to database. This ensures having locally latest data always in database.

yigit avatar Dec 20 '19 17:12 yigit

OK I think that could work. We'll maintained to concept of baseValue which could be updated locally and also the local modification log. in case of conflict we will fetch the new baseValue from the server, attempt to reapply the updates and do everything else the same. Updating the original table means we have no side-effects between reads and writes so we can create a completely separate entry point for write

eyalgu avatar Dec 20 '19 19:12 eyalgu